diff mbox series

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

Message ID 20240416062952.14196-5-jiawenwu@trustnetic.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Wangxun fixes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 926 this patch: 926
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 937 this patch: 937
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 937 this patch: 937
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 71 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 28 this patch: 28
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-16--12-00 (tests: 957)

Commit Message

Jiawen Wu April 16, 2024, 6:29 a.m. UTC 
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>
---
 drivers/net/ethernet/wangxun/libwx/wx_lib.c   | 22 +++++++++++++++++++
 drivers/net/ethernet/wangxun/libwx/wx_lib.h   |  2 ++
 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c |  6 ++++-
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  5 ++++-
 4 files changed, 33 insertions(+), 2 deletions(-)

Comments

Simon Horman April 18, 2024, 6:58 p.m. UTC | #1 
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.

...
Jiawen Wu April 25, 2024, 6:53 a.m. UTC | #2 
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.
Simon Horman April 26, 2024, 7:07 a.m. UTC | #3 
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.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
index 5c511feb97f2..40612cd29f1b 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
@@ -2701,6 +2701,28 @@  int wx_set_features(struct net_device *netdev, netdev_features_t features)
 }
 EXPORT_SYMBOL(wx_set_features);
 
+netdev_features_t wx_fix_features(struct net_device *netdev,
+				  netdev_features_t features)
+{
+	if (features & NETIF_F_HW_VLAN_CTAG_FILTER)
+		features |= NETIF_F_HW_VLAN_STAG_FILTER;
+	else
+		features &= ~NETIF_F_HW_VLAN_STAG_FILTER;
+
+	if (features & NETIF_F_HW_VLAN_CTAG_RX)
+		features |= NETIF_F_HW_VLAN_STAG_RX;
+	else
+		features &= ~NETIF_F_HW_VLAN_STAG_RX;
+
+	if (features & NETIF_F_HW_VLAN_CTAG_TX)
+		features |= NETIF_F_HW_VLAN_STAG_TX;
+	else
+		features &= ~NETIF_F_HW_VLAN_STAG_TX;
+
+	return features;
+}
+EXPORT_SYMBOL(wx_fix_features);
+
 void wx_set_ring(struct wx *wx, u32 new_tx_count,
 		 u32 new_rx_count, struct wx_ring *temp_ring)
 {
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.h b/drivers/net/ethernet/wangxun/libwx/wx_lib.h
index ec909e876720..c41b29ea812f 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.h
@@ -30,6 +30,8 @@  int wx_setup_resources(struct wx *wx);
 void wx_get_stats64(struct net_device *netdev,
 		    struct rtnl_link_stats64 *stats);
 int wx_set_features(struct net_device *netdev, netdev_features_t features);
+netdev_features_t wx_fix_features(struct net_device *netdev,
+				  netdev_features_t features);
 void wx_set_ring(struct wx *wx, u32 new_tx_count,
 		 u32 new_rx_count, struct wx_ring *temp_ring);
 
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index fdd6b4f70b7a..12e47a1056d7 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -499,6 +499,7 @@  static const struct net_device_ops ngbe_netdev_ops = {
 	.ndo_start_xmit         = wx_xmit_frame,
 	.ndo_set_rx_mode        = wx_set_rx_mode,
 	.ndo_set_features       = wx_set_features,
+	.ndo_fix_features       = wx_fix_features,
 	.ndo_validate_addr      = eth_validate_addr,
 	.ndo_set_mac_address    = wx_set_mac,
 	.ndo_get_stats64        = wx_get_stats64,
@@ -584,7 +585,10 @@  static int ngbe_probe(struct pci_dev *pdev,
 			   NETIF_F_RXHASH | NETIF_F_RXCSUM;
 	netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_TSO_MANGLEID;
 	netdev->vlan_features |= netdev->features;
-	netdev->features |= NETIF_F_IPV6_CSUM | NETIF_F_VLAN_FEATURES;
+	netdev->features |= NETIF_F_IPV6_CSUM;
+	netdev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+			    NETIF_F_HW_VLAN_CTAG_RX |
+			    NETIF_F_HW_VLAN_CTAG_TX;
 	/* copy netdev features into list of user selectable features */
 	netdev->hw_features |= netdev->features | NETIF_F_RXALL;
 	netdev->hw_features |= NETIF_F_NTUPLE | NETIF_F_HW_TC;
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index bd4624d14ca0..af0c548e52b0 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -428,6 +428,7 @@  static const struct net_device_ops txgbe_netdev_ops = {
 	.ndo_start_xmit         = wx_xmit_frame,
 	.ndo_set_rx_mode        = wx_set_rx_mode,
 	.ndo_set_features       = wx_set_features,
+	.ndo_fix_features       = wx_fix_features,
 	.ndo_validate_addr      = eth_validate_addr,
 	.ndo_set_mac_address    = wx_set_mac,
 	.ndo_get_stats64        = wx_get_stats64,
@@ -547,7 +548,9 @@  static int txgbe_probe(struct pci_dev *pdev,
 	netdev->features |= NETIF_F_SCTP_CRC;
 	netdev->vlan_features |= netdev->features | NETIF_F_TSO_MANGLEID;
 	netdev->hw_enc_features |= netdev->vlan_features;
-	netdev->features |= NETIF_F_VLAN_FEATURES;
+	netdev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+			    NETIF_F_HW_VLAN_CTAG_RX |
+			    NETIF_F_HW_VLAN_CTAG_TX;
 	/* copy netdev features into list of user selectable features */
 	netdev->hw_features |= netdev->features | NETIF_F_RXALL;
 	netdev->hw_features |= NETIF_F_NTUPLE | NETIF_F_HW_TC;