Re: [PATCH net 4/5] net: wangxun: change NETIF_F_HW_VLAN_STAG_* to fixed features

[Simon Horman <horms@kernel.org>]

On Thu, Apr 25, 2024 at 02:53:24PM +0800, Jiawen Wu wrote:
> On Fri, April 19, 2024 2:59 AM, Simon Horman wrote:
> > On Tue, Apr 16, 2024 at 02:29:51PM +0800, Jiawen Wu wrote:
> > > Because the hardware doesn't support the configuration of VLAN STAG,
> > > remove NETIF_F_HW_VLAN_STAG_* in netdev->features, and set their state
> > > to be consistent with NETIF_F_HW_VLAN_CTAG_*.
> > >
> > > Fixes: 6670f1ece2c8 ("net: txgbe: Add netdev features support")
> > > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > 
> > Hi Jiawen Wu,
> > 
> > I am having trouble reconciling "hardware doesn't support the configuration
> > of VLAN STAG" with both "set their state to be consistent with
> > NETIF_F_HW_VLAN_CTAG_*" and the code changes.
> > 
> > Is the problem here not that VLAN STAGs aren't supported by
> > the HW, but rather that the HW requires that corresponding
> > CTAG and STAG configuration always matches?
> > 
> > I.e, the HW requires:
> > 
> >   f & NETIF_F_HW_VLAN_CTAG_FILTER == f & NETIF_F_HW_VLAN_STAG_FILTER
> >   f & NETIF_F_HW_VLAN_CTAG_RX     == f & NETIF_F_HW_VLAN_STAG_RX
> >   f & NETIF_F_HW_VLAN_CTAG_TX     == f & NETIF_F_HW_VLAN_STAG_TX
> > 
> > If so, I wonder if only the wx_fix_features() portion of
> > this patch is required.
> 
> You are right. I need to set their state to be consistent in wx_fix_features(),
> this patch is missing the case when STAG changes.

Yes, agreed. The case where STAG changes also occurred to me after I sent
my previous email. Sorry for forgetting on follow-up on that.