Re: [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload()

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Rongguang Wei wrote:
> Willem de Bruijn wrote:
> > Rongguang Wei wrote:
> >> From: Rongguang Wei <weirongguang@kylinos.cn>
> >>
> >> The USO features in set_offload() incorrectly uses feature_mask and
> >> features argument.
> >>
> >> The USO feature was written to the local features variable instead of
> >> feature_mask. All other offload bits (TSO, TSO_ECN) are stored in
> >> feature_mask which becomes tap->tap_features and is used by
> >> tap_handle_frame() for GSO segmentation. Without NETIF_F_GSO_UDP_L4
> >> in tap->tap_features, making USO on tap effectively non-functional.
> >>
> >> Keeping the USO handling inside the TUN_F_CSUM block avoids enabling
> >> GRO/LRO when userspace requests USO without CSUM.
> >>
> >> Fixes: 399e0827642f ("driver/net/tun: Added features for USO.")
> >> Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
> >> Reviewed-by: Willem de Bruijn <willemb@google.com>
> > 
> > Reviewed-by: Willem de Bruijn <willemb@google.com>
> > 
> > Substantially changed patch, please don't keep Reviewed-by tags across
> > non-trivial changes. That said, I do agree, so adding it again.
> > 
> >> ---
> >> v2: Fix more incorrect variable in USO check.
> >>     https://lore.kernel.org/netdev/20260806163949.2807698-1-kuba@kernel.org/
> >> v1: https://lore.kernel.org/netdev/20260805072121.117472-1-clementwei90@163.com/
> >> ---
> >>  drivers/net/tap.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> >> index 5d2d34d24ce8..d4ca2fee538b 100644
> >> --- a/drivers/net/tap.c
> >> +++ b/drivers/net/tap.c
> >> @@ -883,7 +883,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
> >>  
> >>  		/* TODO: for now USO4 and USO6 should work simultaneously */
> >>  		if ((arg & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
> >> -			features |= NETIF_F_GSO_UDP_L4;
> >> +			feature_mask |= NETIF_F_GSO_UDP_L4;
> > 
> > Drivers/net/tun seems to have this same issue.
> > 
> > Do you want to take a look or shall I?
> > 
> Hi,
> I have check the code in drivers/net/tun.c.
> In tun.c, the tun's all offload bits go into features argument and
> I think tun.c does not have this issue.

Oh indeed. Thanks for checking.

To repeat, this patch LGTM.