Re: [Patch net] mlx5: fixup checksum for ethernet padding
From: Eric Dumazet <hidden>
Date: 2018-11-28 10:48:10
On 11/27/2018 03:21 PM, Cong Wang wrote:
quoted hunk ↗ jump to hunk
When an ethernet frame is padded to meet the minimum ethernet frame size, the padding octets are not covered by the hardware checksum. Fortunately the padding octets are ususally zero's, which don't affect checksum. However, we have a switch which pads non-zero octets, this causes kernel hardware checksum fault repeatedly. Prior to commit 88078d98d1bb ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends"), skb checksum is forced to be CHECKSUM_NONE when padding is detected. After it, we have to keep skb->csum updated, like what we do for FCS. The logic is a bit complicated when dealing with both FCS and padding, so I wrap it up in a helper function mlx5e_csum_padding(). I tested this patch with RXFCS on and off, it works fine without any warning in both cases. Fixes: 88078d98d1bb ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends"), Cc: Saeed Mahameed <redacted> Cc: Eric Dumazet <edumazet@google.com> Signed-off-by: Cong Wang <redacted> --- .../net/ethernet/mellanox/mlx5/core/en_rx.c | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+)diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c index 16985ca3248d..93c18647ca74 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c@@ -732,6 +732,35 @@ static u8 get_ip_proto(struct sk_buff *skb, __be16 proto) ((struct ipv6hdr *)ip_p)->nexthdr; } +static void mlx5e_csum_padding(struct sk_buff *skb, int network_depth, + __be16 proto, bool has_fcs) +{ + u32 frame_len = has_fcs ? skb->len - ETH_FCS_LEN : skb->len; + void *ip_p = skb->data + network_depth; + u32 pad_offset, pad_len; + void *pad; + + if (likely(frame_len > ETH_ZLEN)) + return; +
But the padding might be added on normal packets (say 1000 bytes + 3 bytes of padding) ? The bug here is that mlx5 csum only includes the data in IP frame. I would simply force skb->ip_summed to CHECKSUM_NONE if any padding is detected. Otherwise, your patch needs more work when multiple frags are used (ie num_frags > 1 )