> In a case, there is also warn info. The test topo is shown as below.

Thanks for testing the patch, Tonghao.

> The warn info is shown as below [1]. If we change the CHECKSUM_NONE to
> CHECKSUM_UNNECESSARY in the udp4_ufo_fragment().
> and we should add a check in skb_needs_check() when outputting a packet.
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 416137c..8fe12a7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -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.

The patch will need the same UFO change in ipv6_ufo_fragment.

I also had to verify that it is indeed correct to use
CHECKSUM_UNNECESSARY for this case. It is not commonly used as alias
for CHECKSUM_NONE on the tx path. And indeed does not carry the exact
same meaning. It was defined as "no need to checksum" on tx for
protocols that skb_checksum_help does not support:

  http://patchwork.ozlabs.org/patch/146567/

Given that, it should be fine to use in this case.

Reply via email to