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

[Radharapu, Rakesh]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx>
> Sent: Sunday, March 16, 2025 7:35 AM
> To: Radharapu, Rakesh <Rakesh.Radharapu@xxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; 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.
>
>
> 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?

Yes, Thanks for your reply. I'm making the recommended modification
to the driver and will also monitor performance numbers.
If everything goes well this RFC patch can be dropped.

Regards,
Rakesh