Re: [PATCH net v3 1/3] net: Make USO depend on CSUM offload

[Jakub Sitnicki <jakub@cloudflare.com>]

On Wed, Aug 07, 2024 at 10:29 PM -04, Willem de Bruijn wrote:
> Jakub Sitnicki wrote:
>> UDP segmentation offload inherently depends on checksum offload. It should
>> not be possible to disable checksum offload while leaving USO enabled.
>> Enforce this dependency in code.
>> 
>> There is a single tx-udp-segmentation feature flag to indicate support for
>> both IPv4/6, hence the devices wishing to support USO must offer checksum
>> offload for both IP versions.
>> 
>> Fixes: 83aa025f535f ("udp: add gso support to virtual devices")
>
> Was this not introduced by removing the CHECKSUM_PARTIAL check in
> udp_send_skb?

Makes sense. It only became a problem after that change. Will update.

>
>> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
>> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
>> ---
>>  net/core/dev.c | 27 ++++++++++++++++++---------
>>  1 file changed, 18 insertions(+), 9 deletions(-)
>> 
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 751d9b70e6ad..dfb12164b35d 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -9912,6 +9912,16 @@ static void netdev_sync_lower_features(struct net_device *upper,
>>  	}
>>  }
>>  
>> +#define IP_CSUM_MASK (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)
>> +
>
> Perhaps NETIF_F_IP_CSUM_MASK in netdev_features.h right below
>
> #define NETIF_F_CSUM_MASK
>
> Then again, for a stable patch we want a small patch. Then I'd define
> as a constant netdev_features_t inside the function scope.
>
> Minor: still prefix with netdev_ even though it's a static function?


Will apply all feedback. Thanks.

[...]