On 05/24/2018 02:53 PM, Saeed Mahameed wrote:
> From: Eran Ben Elisha <era...@mellanox.com>
>
> When RXFCS feature is enabled, the HW do not strip the FCS data,
> however it is not present in the checksum calculated by the HW.
>
> Fix that by manually calculating the FCS checksum and adding it to the SKB
> checksum field.
>
> Add helper function to find the FCS data for all SKB forms (linear,
> one fragment or more).
>
> Fixes: 102722fc6832 ("net/mlx5e: Add support for RXFCS feature flag")
> Signed-off-by: Eran Ben Elisha <era...@mellanox.com>
> Signed-off-by: Saeed Mahameed <sae...@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/en_rx.c | 42 +++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 176645762e49..1ff0b0e93804 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -615,6 +615,45 @@ static inline bool is_last_ethertype_ip(struct sk_buff
> *skb, int *network_depth)
> return (ethertype == htons(ETH_P_IP) || ethertype == htons(ETH_P_IPV6));
> }
>
> +static __be32 mlx5e_get_fcs(struct sk_buff *skb)
> +{
> + int last_frag_sz, bytes_in_prev, nr_frags;
> + u8 *fcs_p1, *fcs_p2;
> + skb_frag_t *last_frag;
> + __be32 fcs_bytes;
> +
> + if (!skb_is_nonlinear(skb))
> + return *(__be32 *)(skb->data + skb->len - ETH_FCS_LEN);
> +
> + nr_frags = skb_shinfo(skb)->nr_frags;
> + last_frag = &skb_shinfo(skb)->frags[nr_frags - 1];
> + last_frag_sz = skb_frag_size(last_frag);
> +
> + /* If all FCS data is in last frag */
> + if (last_frag_sz >= ETH_FCS_LEN)
> + return *(__be32 *)(skb_frag_address(last_frag) +
> + last_frag_sz - ETH_FCS_LEN);
> +
> + fcs_p2 = (u8 *)skb_frag_address(last_frag);
> + bytes_in_prev = ETH_FCS_LEN - last_frag_sz;
> +
> + /* Find where the other part of the FCS is - Linear or another frag */
> + if (nr_frags == 1) {
> + fcs_p1 = skb_tail_pointer(skb);
> + } else {
> + skb_frag_t *prev_frag = &skb_shinfo(skb)->frags[nr_frags - 2];
> +
> + fcs_p1 = skb_frag_address(prev_frag) +
> + skb_frag_size(prev_frag);
> + }
> + fcs_p1 -= bytes_in_prev;
> +
> + memcpy(&fcs_bytes, fcs_p1, bytes_in_prev);
> + memcpy(((u8 *)&fcs_bytes) + bytes_in_prev, fcs_p2, last_frag_sz);
> +
> + return fcs_bytes;
> +}
>
Oh well, this is so ugly, why isn't skb_header_pointer() used ?
Untested patch :
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index
94224c22ecc310a87b6715051e335446f29bec03..11129e3a50d6f3b9a49861a99023541720bbcbe4
100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -713,43 +713,12 @@ static inline void mlx5e_enable_ecn(struct mlx5e_rq *rq,
struct sk_buff *skb)
rq->stats->ecn_mark += !!rc;
}
-static __be32 mlx5e_get_fcs(struct sk_buff *skb)
+static __be32 mlx5e_get_fcs(const struct sk_buff *skb)
{
- int last_frag_sz, bytes_in_prev, nr_frags;
- u8 *fcs_p1, *fcs_p2;
- skb_frag_t *last_frag;
__be32 fcs_bytes;
- if (!skb_is_nonlinear(skb))
- return *(__be32 *)(skb->data + skb->len - ETH_FCS_LEN);
-
- nr_frags = skb_shinfo(skb)->nr_frags;
- last_frag = &skb_shinfo(skb)->frags[nr_frags - 1];
- last_frag_sz = skb_frag_size(last_frag);
-
- /* If all FCS data is in last frag */
- if (last_frag_sz >= ETH_FCS_LEN)
- return *(__be32 *)(skb_frag_address(last_frag) +
- last_frag_sz - ETH_FCS_LEN);
-
- fcs_p2 = (u8 *)skb_frag_address(last_frag);
- bytes_in_prev = ETH_FCS_LEN - last_frag_sz;
-
- /* Find where the other part of the FCS is - Linear or another frag */
- if (nr_frags == 1) {
- fcs_p1 = skb_tail_pointer(skb);
- } else {
- skb_frag_t *prev_frag = &skb_shinfo(skb)->frags[nr_frags - 2];
-
- fcs_p1 = skb_frag_address(prev_frag) +
- skb_frag_size(prev_frag);
- }
- fcs_p1 -= bytes_in_prev;
-
- memcpy(&fcs_bytes, fcs_p1, bytes_in_prev);
- memcpy(((u8 *)&fcs_bytes) + bytes_in_prev, fcs_p2, last_frag_sz);
-
- return fcs_bytes;
+ return *(__be32 *)skb_header_pointer(skb, skb->len - ETH_FCS_LEN,
+ ETH_FCS_LEN, &fcs_bytes);
}
