From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [Patch net] mlx5: fixup checksum for ethernet padding Date: Tue, 27 Nov 2018 15:48:35 -0800 Message-ID: <0b5b3894-161f-c604-12bc-ecde341d7776@gmail.com> References: <20181127232142.7561-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Saeed Mahameed , Eric Dumazet To: Cong Wang , netdev@vger.kernel.org Return-path: Received: from mail-pl1-f196.google.com ([209.85.214.196]:43114 "EHLO mail-pl1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726345AbeK1KsK (ORCPT ); Wed, 28 Nov 2018 05:48:10 -0500 Received: by mail-pl1-f196.google.com with SMTP id gn14so16408981plb.10 for ; Tue, 27 Nov 2018 15:48:37 -0800 (PST) In-Reply-To: <20181127232142.7561-1-xiyou.wangcong@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/27/2018 03:21 PM, Cong Wang wrote: > 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 > Cc: Eric Dumazet > Signed-off-by: Cong Wang > --- > .../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 )