[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 8 Aug 2017 10:46:06 -0700
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Tonghao Zhang <xiangxia.m.yue@...il.com>
Cc: Eric Dumazet <eric.dumazet@...il.com>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Willem de Bruijn <willemb@...gle.com>,
Pravin B Shelar <pshelar@....org>
Subject: Re: [PATCH] net: Reduce skb_warn_bad_offload() noise.
>> @@ -2670,6 +2670,7 @@ static inline bool skb_needs_check(struct
>> sk_buff *skb, bool tx_path)
>> {
>> if (tx_path)
>> return skb->ip_summed != CHECKSUM_PARTIAL &&
>> + skb->ip_summed != CHECKSUM_UNNECESSARY &&
>> skb->ip_summed != CHECKSUM_NONE;
>
> Good catch. Only, the CHECKSUM_NONE case was added specifically to
> work around this UFO issue on the tx path in commit 6e7bc478c9a0
> ("net: skb_needs_check() accepts CHECKSUM_NONE for tx"). If we change
> the value generated by UFO, we can remove that statement, so
>
> + skb->ip_summed != CHECKSUM_UNNECESSARY;
> - skb->ip_summed != CHECKSUM_NONE;
>
> Else the entire check becomes a NOOP. These are the only three valid
> states on tx. With very few codepaths generating CHECKSUM_UNNECESSARY
> to begin with, it arguably already is practically a NOOP. I need to
> look more closely what the statement is intended to protect against,
> before we relax it even further.
On transmit, packets entering skb_gso_segment are expected to always
have ip_summed CHECKSUM_PARTIAL. This check was added to track down
unexpected exceptions in commit 67fd1a731ff1 ("net: Add debug info to
track down GSO checksum bug").
Only when called for the second time, after skb_mac_gso_segment, do we
have to possibly handle the case where the GSO layer computes the
checksum and changes ip_summed.
Since this only goes into 4.11 to 4.13, making two separate
skb_needs_check variants for these two call sites seems overkill. I
will send the simple fix to convert CHECKSUM_NONE to
CHECKSUM_UNNECESSARY.
As a side effect of removing UFO in 4.14-rc1, we can also revert
commit 6e7bc478c9a0 ("net: skb_needs_check() accepts CHECKSUM_NONE for
tx") in net-next.
Powered by blists - more mailing lists