diff mbox series

[net-next,1/2] net: txgbe: Support to set UDP tunnel port

Message ID 20250410074456.321847-2-jiawenwu@trustnetic.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Implement udp tunnel port for txgbe | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
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 No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 163 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 6 this patch: 6
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2025-04-11--03-00 (tests: 900)

Commit Message

Jiawen Wu April 10, 2025, 7:44 a.m. UTC 
Tunnel types VXLAN/VXLAN_GPE/GENEVE are supported for txgbe devices. The
hardware supports to set only one port for each tunnel type.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 113 ++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |   8 ++
 2 files changed, 121 insertions(+)

Comments

Michal Kubiak April 11, 2025, 1:49 p.m. UTC | #1 
On Thu, Apr 10, 2025 at 03:44:55PM +0800, Jiawen Wu wrote:
> Tunnel types VXLAN/VXLAN_GPE/GENEVE are supported for txgbe devices. The
> hardware supports to set only one port for each tunnel type.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 113 ++++++++++++++++++
>  .../net/ethernet/wangxun/txgbe/txgbe_type.h   |   8 ++
>  2 files changed, 121 insertions(+)
> 
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h

[...]

> index 5937cbc6bd05..67ea81dfe786 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> @@ -88,6 +88,9 @@
>  /* Port cfg registers */
>  #define TXGBE_CFG_PORT_ST                       0x14404
>  #define TXGBE_CFG_PORT_ST_LINK_UP               BIT(0)
> +#define TXGBE_CFG_VXLAN                         0x14410
> +#define TXGBE_CFG_VXLAN_GPE                     0x14414
> +#define TXGBE_CFG_GENEVE                        0x14418
>  
>  /* I2C registers */
>  #define TXGBE_I2C_BASE                          0x14900
> @@ -359,6 +362,11 @@ struct txgbe {
>  	union txgbe_atr_input fdir_mask;
>  	int fdir_filter_count;
>  	spinlock_t fdir_perfect_lock; /* spinlock for FDIR */
> +
> +	/* tunnel port */
> +	__be16 vxlan_port;
> +	__be16 geneve_port;
> +	__be16 vxlan_gpe_port;

nitpick: Can these definitions be reordered to keep the consistent order in
         newly added code?
         (In all other places you have the order: VXLAN, GPE, GENEVE).

Thanks,
Michal

[...]
Jakub Kicinski April 11, 2025, 11:22 p.m. UTC | #2 
On Thu, 10 Apr 2025 15:44:55 +0800 Jiawen Wu wrote:
> +	if (ti->type != UDP_TUNNEL_TYPE_VXLAN &&
> +	    ti->type != UDP_TUNNEL_TYPE_VXLAN_GPE &&
> +	    ti->type != UDP_TUNNEL_TYPE_GENEVE)
> +		return -EINVAL;

Why are you doing this validation?

> +	switch (ti->type) {
> +	case UDP_TUNNEL_TYPE_VXLAN:
> +		if (txgbe->vxlan_port != ti->port) {
> +			wx_err(wx, "VXLAN port %d not found\n", ti->port);
> +			return -EINVAL;
> +		}

And what is this check for? Is the core code calling your driver with
incorrect info? Please don't do this sort of defensive programming.
This patch is >50% pointless checks :(

> +		txgbe->vxlan_port = 0;
> +		break;
> +	case UDP_TUNNEL_TYPE_VXLAN_GPE:
> +		if (txgbe->vxlan_gpe_port != ti->port) {
> +			wx_err(wx, "VXLAN-GPE port %d not found\n", ti->port);
> +			return -EINVAL;
> +		}
> +
> +		txgbe->vxlan_gpe_port = 0;
> +		break;
> +	case UDP_TUNNEL_TYPE_GENEVE:
> +		if (txgbe->geneve_port != ti->port) {
> +			wx_err(wx, "GENEVE port %d not found\n", ti->port);
> +			return -EINVAL;
> +		}
> +
> +		txgbe->geneve_port = 0;
> +		break;
> +	default:
> +		return -EINVAL;

Also pointless. Unless you can show me in the core how it could
possibly call your driver with a table type that you didn't declare as
supported.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index 6d9134a3ce4d..c984745504b4 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -8,6 +8,7 @@ 
 #include <linux/string.h>
 #include <linux/etherdevice.h>
 #include <linux/phylink.h>
+#include <net/udp_tunnel.h>
 #include <net/ip.h>
 #include <linux/if_vlan.h>
 
@@ -392,6 +393,8 @@  static int txgbe_open(struct net_device *netdev)
 
 	txgbe_up_complete(wx);
 
+	udp_tunnel_nic_reset_ntf(netdev);
+
 	return 0;
 
 err_free_irq:
@@ -537,6 +540,114 @@  void txgbe_do_reset(struct net_device *netdev)
 		txgbe_reset(wx);
 }
 
+static int txgbe_udp_tunnel_set(struct net_device *dev, unsigned int table,
+				unsigned int entry, struct udp_tunnel_info *ti)
+{
+	struct wx *wx = netdev_priv(dev);
+	struct txgbe *txgbe = wx->priv;
+
+	switch (ti->type) {
+	case UDP_TUNNEL_TYPE_VXLAN:
+		if (txgbe->vxlan_port == ti->port)
+			break;
+
+		if (txgbe->vxlan_port) {
+			wx_err(wx, "VXLAN port %d set, not adding port %d\n",
+			       txgbe->vxlan_port, ti->port);
+			return -EINVAL;
+		}
+
+		txgbe->vxlan_port = ti->port;
+		wr32(wx, TXGBE_CFG_VXLAN, ntohs(ti->port));
+		break;
+	case UDP_TUNNEL_TYPE_VXLAN_GPE:
+		if (txgbe->vxlan_gpe_port == ti->port)
+			break;
+
+		if (txgbe->vxlan_gpe_port) {
+			wx_err(wx, "VXLAN-GPE port %d set, not adding port %d\n",
+			       txgbe->vxlan_gpe_port, ti->port);
+			return -EINVAL;
+		}
+
+		txgbe->vxlan_gpe_port = ti->port;
+		wr32(wx, TXGBE_CFG_VXLAN_GPE, ntohs(ti->port));
+		break;
+	case UDP_TUNNEL_TYPE_GENEVE:
+		if (txgbe->geneve_port == ti->port)
+			break;
+
+		if (txgbe->geneve_port) {
+			wx_err(wx, "GENEVE port %d set, not adding port %d\n",
+			       txgbe->geneve_port, ti->port);
+			return -EINVAL;
+		}
+
+		txgbe->geneve_port = ti->port;
+		wr32(wx, TXGBE_CFG_GENEVE, ntohs(ti->port));
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int txgbe_udp_tunnel_unset(struct net_device *dev,
+				  unsigned int table, unsigned int entry,
+				  struct udp_tunnel_info *ti)
+{
+	struct wx *wx = netdev_priv(dev);
+	struct txgbe *txgbe = wx->priv;
+
+	if (ti->type != UDP_TUNNEL_TYPE_VXLAN &&
+	    ti->type != UDP_TUNNEL_TYPE_VXLAN_GPE &&
+	    ti->type != UDP_TUNNEL_TYPE_GENEVE)
+		return -EINVAL;
+
+	switch (ti->type) {
+	case UDP_TUNNEL_TYPE_VXLAN:
+		if (txgbe->vxlan_port != ti->port) {
+			wx_err(wx, "VXLAN port %d not found\n", ti->port);
+			return -EINVAL;
+		}
+
+		txgbe->vxlan_port = 0;
+		break;
+	case UDP_TUNNEL_TYPE_VXLAN_GPE:
+		if (txgbe->vxlan_gpe_port != ti->port) {
+			wx_err(wx, "VXLAN-GPE port %d not found\n", ti->port);
+			return -EINVAL;
+		}
+
+		txgbe->vxlan_gpe_port = 0;
+		break;
+	case UDP_TUNNEL_TYPE_GENEVE:
+		if (txgbe->geneve_port != ti->port) {
+			wx_err(wx, "GENEVE port %d not found\n", ti->port);
+			return -EINVAL;
+		}
+
+		txgbe->geneve_port = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct udp_tunnel_nic_info txgbe_udp_tunnels = {
+	.set_port	= txgbe_udp_tunnel_set,
+	.unset_port	= txgbe_udp_tunnel_unset,
+	.flags		= UDP_TUNNEL_NIC_INFO_MAY_SLEEP,
+	.tables		= {
+		{ .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN, },
+		{ .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN_GPE, },
+		{ .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_GENEVE, },
+	},
+};
+
 static const struct net_device_ops txgbe_netdev_ops = {
 	.ndo_open               = txgbe_open,
 	.ndo_stop               = txgbe_close,
@@ -632,6 +743,7 @@  static int txgbe_probe(struct pci_dev *pdev,
 	wx->driver_name = txgbe_driver_name;
 	txgbe_set_ethtool_ops(netdev);
 	netdev->netdev_ops = &txgbe_netdev_ops;
+	netdev->udp_tunnel_nic_info = &txgbe_udp_tunnels;
 
 	/* setup the private structure */
 	err = txgbe_sw_init(wx);
@@ -677,6 +789,7 @@  static int txgbe_probe(struct pci_dev *pdev,
 	netdev->features |= NETIF_F_HIGHDMA;
 	netdev->hw_features |= NETIF_F_GRO;
 	netdev->features |= NETIF_F_GRO;
+	netdev->features |= NETIF_F_RX_UDP_TUNNEL_PORT;
 
 	netdev->priv_flags |= IFF_UNICAST_FLT;
 	netdev->priv_flags |= IFF_SUPP_NOFCS;
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 5937cbc6bd05..67ea81dfe786 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -88,6 +88,9 @@ 
 /* Port cfg registers */
 #define TXGBE_CFG_PORT_ST                       0x14404
 #define TXGBE_CFG_PORT_ST_LINK_UP               BIT(0)
+#define TXGBE_CFG_VXLAN                         0x14410
+#define TXGBE_CFG_VXLAN_GPE                     0x14414
+#define TXGBE_CFG_GENEVE                        0x14418
 
 /* I2C registers */
 #define TXGBE_I2C_BASE                          0x14900
@@ -359,6 +362,11 @@  struct txgbe {
 	union txgbe_atr_input fdir_mask;
 	int fdir_filter_count;
 	spinlock_t fdir_perfect_lock; /* spinlock for FDIR */
+
+	/* tunnel port */
+	__be16 vxlan_port;
+	__be16 geneve_port;
+	__be16 vxlan_gpe_port;
 };
 
 #endif /* _TXGBE_TYPE_H_ */