RE: [RFC PATCH net-next] net: Modify CSUM capability check for USO

[Willem de Bruijn]

Radharapu, Rakesh wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> > -----Original Message-----
> > From: Paolo Abeni <pabeni@xxxxxxxxxx>
> > Sent: Wednesday, March 12, 2025 9:44 PM
> > To: Radharapu, Rakesh <Rakesh.Radharapu@xxxxxxx>; git (AMD-Xilinx)
> > <git@xxxxxxx>; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> > kuba@xxxxxxxxxx; horms@xxxxxxxxxx; kuniyu@xxxxxxxxxx;
> > bigeasy@xxxxxxxxxxxxx
> > Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Katakam, Harini
> > <harini.katakam@xxxxxxx>; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@xxxxxxx>; Simek, Michal
> > <michal.simek@xxxxxxx>
> > Subject: Re: [RFC PATCH net-next] net: Modify CSUM capability check for USO
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > On 3/12/25 12:54 PM, Radharapu Rakesh wrote:
> > >  net/core/dev.c | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/net/core/dev.c b/net/core/dev.c index
> > > 1cb134ff7327..a22f8f6e2ed1 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -10465,11 +10465,13 @@ static void
> > > netdev_sync_lower_features(struct net_device *upper,
> > >
> > >  static bool netdev_has_ip_or_hw_csum(netdev_features_t features)  {
> > > -     netdev_features_t ip_csum_mask = NETIF_F_IP_CSUM |
> > NETIF_F_IPV6_CSUM;
> > > -     bool ip_csum = (features & ip_csum_mask) == ip_csum_mask;
> > > +     netdev_features_t ipv4_csum_mask = NETIF_F_IP_CSUM;
> > > +     netdev_features_t ipv6_csum_mask = NETIF_F_IPV6_CSUM;
> > > +     bool ipv4_csum = (features & ipv4_csum_mask) == ipv4_csum_mask;
> > > +     bool ipv6_csum = (features & ipv6_csum_mask) == ipv6_csum_mask;
> > >       bool hw_csum = features & NETIF_F_HW_CSUM;
> > >
> > > -     return ip_csum || hw_csum;
> > > +     return ipv4_csum || ipv6_csum || hw_csum;
> > >  }
> >
> > The above will additionally affect TLS offload, and will likely break i.e. USO
> > over IPv6 traffic landing on devices supporting only USO over IPv4, unless
> > such devices additionally implement a suitable ndo_features_check().
> >
> > Such situation will be quite bug prone, I'm unsure we want this kind of change
> > - even without looking at the TLS side of it.
> >
> > /P
> Thanks for your review. I understand that this will lead to an issue.
> We have a device that supports only IPv4 CSUM and are unable to enable the
> USO feature because of this check. Can you please let me know if splitting
> GSO feature for IPv4 and IPv6 would be helpful? That way corresponding
> CSUM offloads can be checked. But this would be a major change.
> Will appreciate any other suggestions.

Splitting NETIF_F_GSO_UDP_L4 would incur significant churn.

Since this is a limitation of a specific device, can you instead
advertise the feature, but for IPv6 packets drop the flag in
ndo_features_check?