[net-next PATCH 00/15] Future-proof tunnel offload handlers

[net-next PATCH 00/15] Future-proof tunnel offload handlers

[Alexander Duyck]

This patch is meant to address two things.  First we are currently using
the ndo_add/del_vxlan_port calls with VXLAN-GPE tunnels and we cannot
really support that as it is likely to cause more harm than good since
VXLAN-GPE can support tunnels without a MAC address on the inner header.

As such we need to add a new offload to advertise this, but in doing so it
would mean introducing 3 new functions for the driver to request the ports,
and then for the tunnel to push the changes to add and delete the ports to
the device.  However instead of taking that approach I think it would be
much better if we just made one common function for fetching the ports, and
provided a generic means to push the tunnels to the device.  So in order to
make this work this patch set does several things.

First it merges the existing VXLAN and GENEVE functionality into one set of
functions and passes an enum in order to specify the type of tunnel we want
to offload.  By doing this we only have to extend this enum in the future
if we want to add additional types.

Second it goes through the drivers replacing all of the tunnel specific
offload calls with implementations that support the generic calls so that
we can drop the VXLAN and GENEVE specific calls entirely.

Finally I go through in the last patch and replace the VXLAN specific
offload request that was being used for VXLAN-GPE with one that specifies
if we want to offload VXLAN or VXLAN-GPE so that the hardware can decide if
it can actually support it or not.

I also ended up with some minor clean-up built into the driver patches for
this.  Most of it is to either fix misuse of build flags, specifying a type
to ignore instead of the type that should be used, or in the case of ixgbe
I actually moved a rtnl_lock/unlock in order to avoid taking it unless it
was actually needed.

---

Alexander Duyck (15):
      net: Combine GENEVE and VXLAN port offload notifiers into single functions
      net: Merge VXLAN and GENEVE push notifiers into a single notifier
      bnx2x: Move all UDP port notifiers to single function
      bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      benet: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      fm10k: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      i40e: Move all UDP port notifiers to single function
      ixgbe: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      mlx4_en: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      mlx5_en: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      nfp: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      qede: Move all UDP port notifiers to single function
      qlcnic: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port
      net: Remove deprecated tunnel specific UDP offload functions
      vxlan: Add new UDP encapsulation offload type for VXLAN-GPE


 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   |   96 ++++++-------
 drivers/net/ethernet/broadcom/bnxt/bnxt.c          |   22 ++-
 drivers/net/ethernet/emulex/benet/be_main.c        |   16 +-
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c    |   18 ++
 drivers/net/ethernet/intel/i40e/i40e_main.c        |  148 +++++---------------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |   36 +++--
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c     |   30 +++-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |   18 ++
 .../net/ethernet/netronome/nfp/nfp_net_common.c    |   18 ++
 drivers/net/ethernet/qlogic/qede/qede_main.c       |  113 ++++++++-------
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c   |   18 ++
 drivers/net/geneve.c                               |   50 +------
 drivers/net/vxlan.c                                |   56 ++------
 include/linux/netdevice.h                          |   19 +--
 include/net/geneve.h                               |    9 -
 include/net/udp_tunnel.h                           |   19 +++
 include/net/vxlan.h                                |    7 -
 net/ipv4/udp_tunnel.c                              |   52 +++++++
 18 files changed, 357 insertions(+), 388 deletions(-)

[net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Alexander Duyck]

This patch merges the GENEVE and VXLAN code so that both functions pass
through a shared code path.  This way we can start the effort of using a
single function on the network device drivers to handle both of these
tunnel offload types.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/geneve.c     |   48 ++++-------------------------
 drivers/net/vxlan.c      |   46 ++++-----------------------
 include/net/udp_tunnel.h |   12 +++++++
 net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 80 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index cadefe4fdaa2..f5ce41532cf4 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
 
 static void geneve_notify_add_rx_port(struct geneve_sock *gs)
 {
-	struct net_device *dev;
-	struct sock *sk = gs->sock->sk;
-	struct net *net = sock_net(sk);
-	sa_family_t sa_family = geneve_get_sk_family(gs);
-	__be16 port = inet_sk(sk)->inet_sport;
-
-	rcu_read_lock();
-	for_each_netdev_rcu(net, dev) {
-		if (dev->netdev_ops->ndo_add_geneve_port)
-			dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
-							     port);
-	}
-	rcu_read_unlock();
+	udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
 }
 
 static int geneve_hlen(struct genevehdr *gh)
@@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
 
 static void geneve_notify_del_rx_port(struct geneve_sock *gs)
 {
-	struct net_device *dev;
-	struct sock *sk = gs->sock->sk;
-	struct net *net = sock_net(sk);
-	sa_family_t sa_family = geneve_get_sk_family(gs);
-	__be16 port = inet_sk(sk)->inet_sport;
-
-	rcu_read_lock();
-	for_each_netdev_rcu(net, dev) {
-		if (dev->netdev_ops->ndo_del_geneve_port)
-			dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
-							     port);
-	}
-
-	rcu_read_unlock();
+	udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
 }
 
 static void __geneve_sock_release(struct geneve_sock *gs)
@@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
 	.name = "geneve",
 };
 
-/* Calls the ndo_add_geneve_port of the caller in order to
+/* Calls the ndo_add_udp_enc_port of the caller in order to
  * supply the listening GENEVE udp ports. Callers are expected
- * to implement the ndo_add_geneve_port.
+ * to implement the ndo_add_udp_enc_port.
  */
 static void geneve_push_rx_ports(struct net_device *dev)
 {
 	struct net *net = dev_net(dev);
 	struct geneve_net *gn = net_generic(net, geneve_net_id);
 	struct geneve_sock *gs;
-	sa_family_t sa_family;
-	struct sock *sk;
-	__be16 port;
-
-	if (!dev->netdev_ops->ndo_add_geneve_port)
-		return;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(gs, &gn->sock_list, list) {
-		sk = gs->sock->sk;
-		sa_family = sk->sk_family;
-		port = inet_sk(sk)->inet_sport;
-		dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
-	}
+	list_for_each_entry_rcu(gs, &gn->sock_list, list)
+		udp_tunnel_push_rx_port(dev, gs->sock,
+					UDP_ENC_OFFLOAD_TYPE_GENEVE);
 	rcu_read_unlock();
 }
 
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index f999db2f97b4..43f634282726 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
 /* Notify netdevs that UDP port started listening */
 static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
 {
-	struct net_device *dev;
-	struct sock *sk = vs->sock->sk;
-	struct net *net = sock_net(sk);
-	sa_family_t sa_family = vxlan_get_sk_family(vs);
-	__be16 port = inet_sk(sk)->inet_sport;
-
-	rcu_read_lock();
-	for_each_netdev_rcu(net, dev) {
-		if (dev->netdev_ops->ndo_add_vxlan_port)
-			dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
-							    port);
-	}
-	rcu_read_unlock();
+	udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
 }
 
 /* Notify netdevs that UDP port is no more listening */
 static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
 {
-	struct net_device *dev;
-	struct sock *sk = vs->sock->sk;
-	struct net *net = sock_net(sk);
-	sa_family_t sa_family = vxlan_get_sk_family(vs);
-	__be16 port = inet_sk(sk)->inet_sport;
-
-	rcu_read_lock();
-	for_each_netdev_rcu(net, dev) {
-		if (dev->netdev_ops->ndo_del_vxlan_port)
-			dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
-							    port);
-	}
-	rcu_read_unlock();
+	udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
 }
 
 /* Add new entry to forwarding table -- assumes lock held */
@@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
 	.name = "vxlan",
 };
 
-/* Calls the ndo_add_vxlan_port of the caller in order to
+/* Calls the ndo_add_udp_enc_port of the caller in order to
  * supply the listening VXLAN udp ports. Callers are expected
- * to implement the ndo_add_vxlan_port.
+ * to implement the ndo_add_udp_enc_port.
  */
 static void vxlan_push_rx_ports(struct net_device *dev)
 {
 	struct vxlan_sock *vs;
 	struct net *net = dev_net(dev);
 	struct vxlan_net *vn = net_generic(net, vxlan_net_id);
-	sa_family_t sa_family;
-	__be16 port;
 	unsigned int i;
 
-	if (!dev->netdev_ops->ndo_add_vxlan_port)
-		return;
-
 	spin_lock(&vn->sock_lock);
 	for (i = 0; i < PORT_HASH_SIZE; ++i) {
-		hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
-			port = inet_sk(vs->sock->sk)->inet_sport;
-			sa_family = vxlan_get_sk_family(vs);
-			dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
-							    port);
-		}
+		hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
+			udp_tunnel_push_rx_port(dev, vs->sock,
+						UDP_ENC_OFFLOAD_TYPE_VXLAN);
 	}
 	spin_unlock(&vn->sock_lock);
 }
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 9d14f707e534..704f931fd9ad 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
 void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
 			   struct udp_tunnel_sock_cfg *sock_cfg);
 
+/* List of offloadable UDP tunnel types */
+enum udp_enc_offloads {
+	UDP_ENC_OFFLOAD_TYPE_VXLAN,	/* RFC 7348 */
+	UDP_ENC_OFFLOAD_TYPE_GENEVE,	/* draft-ietf-nvo3-geneve */
+};
+
+/* Notify network devices of offloadable types */
+void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
+			     unsigned int type);
+void udp_tunnel_notify_add_rx_port(struct socket *sock, unsigned int type);
+void udp_tunnel_notify_del_rx_port(struct socket *sock, unsigned int type);
+
 /* Transmit the skb using UDP encapsulation. */
 void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
 			 __be32 src, __be32 dst, __u8 tos, __u8 ttl,
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
index 47f12c73d959..d575b70644e3 100644
--- a/net/ipv4/udp_tunnel.c
+++ b/net/ipv4/udp_tunnel.c
@@ -76,6 +76,83 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
 }
 EXPORT_SYMBOL_GPL(setup_udp_tunnel_sock);
 
+void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
+			     unsigned int type)
+{
+	struct sock *sk = sock->sk;
+	sa_family_t sa_family = sk->sk_family;
+	__be16 port = inet_sk(sk)->inet_sport;
+
+	switch (type) {
+	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
+		if (!dev->netdev_ops->ndo_add_vxlan_port)
+			break;
+
+		dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family, port);
+		break;
+	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
+		if (!dev->netdev_ops->ndo_add_geneve_port)
+			break;
+
+		dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
+		break;
+	default:
+		break;
+	}
+}
+EXPORT_SYMBOL_GPL(udp_tunnel_push_rx_port);
+
+/* Notify netdevs that UDP port started listening */
+void udp_tunnel_notify_add_rx_port(struct socket *sock, unsigned int type)
+{
+	struct net *net = sock_net(sock->sk);
+	struct net_device *dev;
+
+	rcu_read_lock();
+	for_each_netdev_rcu(net, dev)
+		udp_tunnel_push_rx_port(dev, sock, type);
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(udp_tunnel_notify_add_rx_port);
+
+static void udp_tunnel_pull_rx_port(struct net_device *dev,
+				    struct socket *sock, unsigned int type)
+{
+	struct sock *sk = sock->sk;
+	sa_family_t sa_family = sk->sk_family;
+	__be16 port = inet_sk(sk)->inet_sport;
+
+	switch (type) {
+	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
+		if (!dev->netdev_ops->ndo_del_vxlan_port)
+			break;
+
+		dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family, port);
+		break;
+	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
+		if (!dev->netdev_ops->ndo_del_geneve_port)
+			break;
+
+		dev->netdev_ops->ndo_del_geneve_port(dev, sa_family, port);
+		break;
+	default:
+		break;
+	}
+}
+
+/* Notify netdevs that UDP port is no more listening */
+void udp_tunnel_notify_del_rx_port(struct socket *sock, unsigned int type)
+{
+	struct net_device *dev;
+	struct net *net = sock_net(sock->sk);
+
+	rcu_read_lock();
+	for_each_netdev_rcu(net, dev)
+		udp_tunnel_pull_rx_port(dev, sock, type);
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(udp_tunnel_notify_del_rx_port);
+
 void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
 			 __be32 src, __be32 dst, __u8 tos, __u8 ttl,
 			 __be16 df, __be16 src_port, __be16 dst_port,

[net-next PATCH 02/15] net: Merge VXLAN and GENEVE push notifiers into a single notifier

[Alexander Duyck]

This patch merges the notifiers for VXLAN and GENEVE into a single UDP
encapsulation notifier.  The idea is that we will want to only have to make
one notifier call to receive the list of ports for VXLAN and GENEVE tunnels
that need to be offloaded.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/geneve.c      |    2 +-
 drivers/net/vxlan.c       |    2 +-
 include/linux/netdevice.h |   11 +++++++++--
 include/net/geneve.h      |    6 +-----
 include/net/udp_tunnel.h  |    6 ++++++
 include/net/vxlan.h       |    4 ++--
 net/ipv4/udp_tunnel.c     |   12 ++++++++++++
 7 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index f5ce41532cf4..f1d06bef1c55 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1508,7 +1508,7 @@ static int geneve_netdevice_event(struct notifier_block *unused,
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 
-	if (event == NETDEV_OFFLOAD_PUSH_GENEVE)
+	if (event == NETDEV_OFFLOAD_PUSH_UDP_ENC_OFFLOAD)
 		geneve_push_rx_ports(dev);
 
 	return NOTIFY_DONE;
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 43f634282726..72da056abdf4 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3266,7 +3266,7 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
 
 	if (event == NETDEV_UNREGISTER)
 		vxlan_handle_lowerdev_unregister(vn, dev);
-	else if (event == NETDEV_OFFLOAD_PUSH_VXLAN)
+	else if (event == NETDEV_OFFLOAD_PUSH_UDP_ENC_OFFLOAD)
 		vxlan_push_rx_ports(dev);
 
 	return NOTIFY_DONE;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d101e4d904ba..e959b6348f91 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1269,6 +1269,14 @@ struct net_device_ops {
 	void			(*ndo_del_geneve_port)(struct  net_device *dev,
 						       sa_family_t sa_family,
 						       __be16 port);
+	void			(*ndo_add_udp_enc_port)(struct  net_device *dev,
+						       sa_family_t sa_family,
+						       __be16 port,
+						       unsigned int type);
+	void			(*ndo_del_udp_enc_port)(struct  net_device *dev,
+						       sa_family_t sa_family,
+						       __be16 port,
+						       unsigned int type);
 	void*			(*ndo_dfwd_add_station)(struct net_device *pdev,
 							struct net_device *dev);
 	void			(*ndo_dfwd_del_station)(struct net_device *pdev,
@@ -2251,8 +2259,7 @@ struct netdev_lag_lower_state_info {
 #define NETDEV_BONDING_INFO	0x0019
 #define NETDEV_PRECHANGEUPPER	0x001A
 #define NETDEV_CHANGELOWERSTATE	0x001B
-#define NETDEV_OFFLOAD_PUSH_VXLAN	0x001C
-#define NETDEV_OFFLOAD_PUSH_GENEVE	0x001D
+#define NETDEV_OFFLOAD_PUSH_UDP_ENC_OFFLOAD	0x001C
 
 int register_netdevice_notifier(struct notifier_block *nb);
 int unregister_netdevice_notifier(struct notifier_block *nb);
diff --git a/include/net/geneve.h b/include/net/geneve.h
index cb544a530146..7638ec62c5e1 100644
--- a/include/net/geneve.h
+++ b/include/net/geneve.h
@@ -1,10 +1,7 @@
 #ifndef __NET_GENEVE_H
 #define __NET_GENEVE_H  1
 
-#ifdef CONFIG_INET
 #include <net/udp_tunnel.h>
-#endif
-
 
 /* Geneve Header:
  *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -64,8 +61,7 @@ struct genevehdr {
 
 static inline void geneve_get_rx_port(struct net_device *netdev)
 {
-	ASSERT_RTNL();
-	call_netdevice_notifiers(NETDEV_OFFLOAD_PUSH_GENEVE, netdev);
+	udp_tunnel_get_rx_port(netdev);
 }
 
 #ifdef CONFIG_INET
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 704f931fd9ad..9ea813740231 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -96,6 +96,12 @@ void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
 void udp_tunnel_notify_add_rx_port(struct socket *sock, unsigned int type);
 void udp_tunnel_notify_del_rx_port(struct socket *sock, unsigned int type);
 
+static inline void udp_tunnel_get_rx_port(struct net_device *dev)
+{
+	ASSERT_RTNL();
+	call_netdevice_notifiers(NETDEV_OFFLOAD_PUSH_UDP_ENC_OFFLOAD, dev);
+}
+
 /* Transmit the skb using UDP encapsulation. */
 void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
 			 __be32 src, __be32 dst, __u8 tos, __u8 ttl,
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index b8803165df91..2c4f8fcd5a3b 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -8,6 +8,7 @@
 #include <linux/netdevice.h>
 #include <linux/udp.h>
 #include <net/dst_metadata.h>
+#include <net/udp_tunnel.h>
 
 /* VXLAN protocol (RFC 7348) header:
  * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -394,8 +395,7 @@ static inline __be32 vxlan_compute_rco(unsigned int start, unsigned int offset)
 
 static inline void vxlan_get_rx_port(struct net_device *netdev)
 {
-	ASSERT_RTNL();
-	call_netdevice_notifiers(NETDEV_OFFLOAD_PUSH_VXLAN, netdev);
+	udp_tunnel_get_rx_port(netdev);
 }
 
 static inline unsigned short vxlan_get_sk_family(struct vxlan_sock *vs)
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
index d575b70644e3..a22677cd41d8 100644
--- a/net/ipv4/udp_tunnel.c
+++ b/net/ipv4/udp_tunnel.c
@@ -83,6 +83,12 @@ void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
 	sa_family_t sa_family = sk->sk_family;
 	__be16 port = inet_sk(sk)->inet_sport;
 
+	if (dev->netdev_ops->ndo_add_udp_enc_port) {
+		dev->netdev_ops->ndo_add_udp_enc_port(dev, sa_family,
+						      port, type);
+		return;
+	}
+
 	switch (type) {
 	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
 		if (!dev->netdev_ops->ndo_add_vxlan_port)
@@ -122,6 +128,12 @@ static void udp_tunnel_pull_rx_port(struct net_device *dev,
 	sa_family_t sa_family = sk->sk_family;
 	__be16 port = inet_sk(sk)->inet_sport;
 
+	if (dev->netdev_ops->ndo_del_udp_enc_port) {
+		dev->netdev_ops->ndo_del_udp_enc_port(dev, sa_family,
+						      port, type);
+		return;
+	}
+
 	switch (type) {
 	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
 		if (!dev->netdev_ops->ndo_del_vxlan_port)

[net-next PATCH 03/15] bnx2x: Move all UDP port notifiers to single function

[Alexander Duyck]

This patch goes through and combines the notifiers for VXLAN and GENEVE
into a single function for each action.  So there is now one combined
function for getting ports, one for adding the ports, and one for deleting
the ports.

While going through and updating this I also removed the use of IS_ENABLED
for the CONFIG_BNX2X_GENEVE checks and instead just checked for defined
since there is no option to have this as a module.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |   96 ++++++++++------------
 1 file changed, 44 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index a59d55e25d5f..8faef81aff89 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -59,7 +59,7 @@
 #include <linux/semaphore.h>
 #include <linux/stringify.h>
 #include <linux/vmalloc.h>
-#if IS_ENABLED(CONFIG_BNX2X_GENEVE)
+#ifdef CONFIG_BNX2X_GENEVE
 #include <net/geneve.h>
 #endif
 #include "bnx2x.h"
@@ -10076,7 +10076,7 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
 	}
 }
 
-#if defined(CONFIG_BNX2X_VXLAN) || IS_ENABLED(CONFIG_BNX2X_GENEVE)
+#if defined(CONFIG_BNX2X_VXLAN) || defined(CONFIG_BNX2X_GENEVE)
 static int bnx2x_udp_port_update(struct bnx2x *bp)
 {
 	struct bnx2x_func_switch_update_params *switch_update_params;
@@ -10177,46 +10177,53 @@ static void __bnx2x_del_udp_port(struct bnx2x *bp, u16 port,
 		DP(BNX2X_MSG_SP, "Deleted UDP tunnel [%d] port %d\n",
 		   type, port);
 }
-#endif
 
-#ifdef CONFIG_BNX2X_VXLAN
-static void bnx2x_add_vxlan_port(struct net_device *netdev,
-				 sa_family_t sa_family, __be16 port)
+static void bnx2x_add_udp_enc_port(struct net_device *netdev,
+				   sa_family_t sa_family, __be16 port,
+				   unsigned int type)
 {
 	struct bnx2x *bp = netdev_priv(netdev);
 	u16 t_port = ntohs(port);
 
-	__bnx2x_add_udp_port(bp, t_port, BNX2X_UDP_PORT_VXLAN);
+	switch (type) {
+#ifdef CONFIG_BNX2X_VXLAN
+	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
+		__bnx2x_add_udp_port(bp, t_port, BNX2X_UDP_PORT_VXLAN);
+		break;
+#endif
+#ifdef CONFIG_BNX2X_GENEVE
+	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
+		__bnx2x_add_udp_port(bp, t_port, BNX2X_UDP_PORT_GENEVE);
+		break;
+#endif
+	default:
+		break;
+	}
 }
 
-static void bnx2x_del_vxlan_port(struct net_device *netdev,
-				 sa_family_t sa_family, __be16 port)
+static void bnx2x_del_udp_enc_port(struct net_device *netdev,
+				   sa_family_t sa_family, __be16 port,
+				   unsigned int type)
 {
 	struct bnx2x *bp = netdev_priv(netdev);
 	u16 t_port = ntohs(port);
 
-	__bnx2x_del_udp_port(bp, t_port, BNX2X_UDP_PORT_VXLAN);
-}
+	switch (type) {
+#ifdef CONFIG_BNX2X_VXLAN
+	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
+		__bnx2x_del_udp_port(bp, t_port, BNX2X_UDP_PORT_VXLAN);
+		break;
 #endif
-
-#if IS_ENABLED(CONFIG_BNX2X_GENEVE)
-static void bnx2x_add_geneve_port(struct net_device *netdev,
-				  sa_family_t sa_family, __be16 port)
-{
-	struct bnx2x *bp = netdev_priv(netdev);
-	u16 t_port = ntohs(port);
-
-	__bnx2x_add_udp_port(bp, t_port, BNX2X_UDP_PORT_GENEVE);
+#ifdef CONFIG_BNX2X_GENEVE
+	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
+		__bnx2x_del_udp_port(bp, t_port, BNX2X_UDP_PORT_GENEVE);
+		break;
+#endif
+	default:
+		break;
+	}
 }
 
-static void bnx2x_del_geneve_port(struct net_device *netdev,
-				  sa_family_t sa_family, __be16 port)
-{
-	struct bnx2x *bp = netdev_priv(netdev);
-	u16 t_port = ntohs(port);
-
-	__bnx2x_del_udp_port(bp, t_port, BNX2X_UDP_PORT_GENEVE);
-}
 #endif
 
 static int bnx2x_close(struct net_device *dev);
@@ -10325,7 +10332,7 @@ sp_rtnl_not_reset:
 			       &bp->sp_rtnl_state))
 		bnx2x_update_mng_version(bp);
 
-#if defined(CONFIG_BNX2X_VXLAN) || IS_ENABLED(CONFIG_BNX2X_GENEVE)
+#if defined(CONFIG_BNX2X_VXLAN) || defined(CONFIG_BNX2X_GENEVE)
 	if (test_and_clear_bit(BNX2X_SP_RTNL_CHANGE_UDP_PORT,
 			       &bp->sp_rtnl_state)) {
 		if (bnx2x_udp_port_update(bp)) {
@@ -10335,17 +10342,12 @@ sp_rtnl_not_reset:
 			       BNX2X_UDP_PORT_MAX);
 		} else {
 			/* Since we don't store additional port information,
-			 * if no port is configured for any feature ask for
+			 * if no ports are configured for any feature ask for
 			 * information about currently configured ports.
 			 */
-#ifdef CONFIG_BNX2X_VXLAN
-			if (!bp->udp_tunnel_ports[BNX2X_UDP_PORT_VXLAN].count)
-				vxlan_get_rx_port(bp->dev);
-#endif
-#if IS_ENABLED(CONFIG_BNX2X_GENEVE)
-			if (!bp->udp_tunnel_ports[BNX2X_UDP_PORT_GENEVE].count)
-				geneve_get_rx_port(bp->dev);
-#endif
+			if (!bp->udp_tunnel_ports[BNX2X_UDP_PORT_VXLAN].count &&
+			    !bp->udp_tunnel_ports[BNX2X_UDP_PORT_GENEVE].count)
+				udp_tunnel_get_rx_port(bp->dev);
 		}
 	}
 #endif
@@ -12551,13 +12553,9 @@ static int bnx2x_open(struct net_device *dev)
 	if (rc)
 		return rc;
 
-#ifdef CONFIG_BNX2X_VXLAN
+#if defined(CONFIG_BNX2X_VXLAN) || defined(CONFIG_BNX2X_GENEVE)
 	if (IS_PF(bp))
-		vxlan_get_rx_port(dev);
-#endif
-#if IS_ENABLED(CONFIG_BNX2X_GENEVE)
-	if (IS_PF(bp))
-		geneve_get_rx_port(dev);
+		udp_tunnel_get_rx_port(dev);
 #endif
 
 	return 0;
@@ -13045,14 +13043,8 @@ static const struct net_device_ops bnx2x_netdev_ops = {
 	.ndo_get_phys_port_id	= bnx2x_get_phys_port_id,
 	.ndo_set_vf_link_state	= bnx2x_set_vf_link_state,
 	.ndo_features_check	= bnx2x_features_check,
-#ifdef CONFIG_BNX2X_VXLAN
-	.ndo_add_vxlan_port	= bnx2x_add_vxlan_port,
-	.ndo_del_vxlan_port	= bnx2x_del_vxlan_port,
-#endif
-#if IS_ENABLED(CONFIG_BNX2X_GENEVE)
-	.ndo_add_geneve_port	= bnx2x_add_geneve_port,
-	.ndo_del_geneve_port	= bnx2x_del_geneve_port,
-#endif
+	.ndo_add_udp_enc_port	= bnx2x_add_udp_enc_port,
+	.ndo_del_udp_enc_port	= bnx2x_del_udp_enc_port,
 };
 
 static int bnx2x_set_coherency_mask(struct bnx2x *bp)

[net-next PATCH 04/15] bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type if VXLAN we can maintain the same
functionality.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c |   22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index c777cde85ce4..ab55da46cb51 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5074,8 +5074,8 @@ static int __bnxt_open_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
 	}
 
 	if (irq_re_init) {
-#if defined(CONFIG_VXLAN) || defined(CONFIG_VXLAN_MODULE)
-		vxlan_get_rx_port(bp->dev);
+#if IS_ENABLED(CONFIG_VXLAN)
+		udp_tunnel_get_rx_port(bp->dev);
 #endif
 		if (!bnxt_hwrm_tunnel_dst_port_alloc(
 				bp, htons(0x17c1),
@@ -6049,16 +6049,19 @@ static void bnxt_cfg_ntp_filters(struct bnxt *bp)
 #endif /* CONFIG_RFS_ACCEL */
 
 static void bnxt_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
-				__be16 port)
+				__be16 port, unsigned int type)
 {
 	struct bnxt *bp = netdev_priv(dev);
 
-	if (!netif_running(dev))
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
 		return;
 
 	if (sa_family != AF_INET6 && sa_family != AF_INET)
 		return;
 
+	if (!netif_running(dev))
+		return;
+
 	if (bp->vxlan_port_cnt && bp->vxlan_port != port)
 		return;
 
@@ -6071,16 +6074,19 @@ static void bnxt_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
 }
 
 static void bnxt_del_vxlan_port(struct net_device *dev, sa_family_t sa_family,
-				__be16 port)
+				__be16 port, unsigned int type)
 {
 	struct bnxt *bp = netdev_priv(dev);
 
-	if (!netif_running(dev))
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
 		return;
 
 	if (sa_family != AF_INET6 && sa_family != AF_INET)
 		return;
 
+	if (!netif_running(dev))
+		return;
+
 	if (bp->vxlan_port_cnt && bp->vxlan_port == port) {
 		bp->vxlan_port_cnt--;
 
@@ -6119,8 +6125,8 @@ static const struct net_device_ops bnxt_netdev_ops = {
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	= bnxt_rx_flow_steer,
 #endif
-	.ndo_add_vxlan_port	= bnxt_add_vxlan_port,
-	.ndo_del_vxlan_port	= bnxt_del_vxlan_port,
+	.ndo_add_udp_enc_port	= bnxt_add_vxlan_port,
+	.ndo_del_udp_enc_port	= bnxt_del_vxlan_port,
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	.ndo_busy_poll		= bnxt_busy_poll,
 #endif

[net-next PATCH 05/15] benet: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type if VXLAN we can maintain the same
functionality.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 2451a47e88ab..f0ec15a7cd85 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3627,7 +3627,7 @@ static int be_open(struct net_device *netdev)
 	netif_tx_start_all_queues(netdev);
 #ifdef CONFIG_BE2NET_VXLAN
 	if (skyhawk_chip(adapter))
-		vxlan_get_rx_port(netdev);
+		udp_tunnel_get_rx_port(netdev);
 #endif
 
 	return 0;
@@ -4721,12 +4721,15 @@ static int be_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
  * until after all the tunnels are removed.
  */
 static void be_add_vxlan_port(struct net_device *netdev, sa_family_t sa_family,
-			      __be16 port)
+			      __be16 port, unsigned int type)
 {
 	struct be_adapter *adapter = netdev_priv(netdev);
 	struct device *dev = &adapter->pdev->dev;
 	int status;
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	if (lancer_chip(adapter) || BEx_chip(adapter) || be_is_mc(adapter))
 		return;
 
@@ -4775,10 +4778,13 @@ err:
 }
 
 static void be_del_vxlan_port(struct net_device *netdev, sa_family_t sa_family,
-			      __be16 port)
+			      __be16 port, unsigned int type)
 {
 	struct be_adapter *adapter = netdev_priv(netdev);
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	if (lancer_chip(adapter) || BEx_chip(adapter) || be_is_mc(adapter))
 		return;
 
@@ -4888,8 +4894,8 @@ static const struct net_device_ops be_netdev_ops = {
 	.ndo_busy_poll		= be_busy_poll,
 #endif
 #ifdef CONFIG_BE2NET_VXLAN
-	.ndo_add_vxlan_port	= be_add_vxlan_port,
-	.ndo_del_vxlan_port	= be_del_vxlan_port,
+	.ndo_add_udp_enc_port	= be_add_vxlan_port,
+	.ndo_del_udp_enc_port	= be_del_vxlan_port,
 	.ndo_features_check	= be_features_check,
 #endif
 	.ndo_get_phys_port_id   = be_get_phys_port_id,

[net-next PATCH 06/15] fm10k: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type if VXLAN we can maintain the same
functionality.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 2a08d3f5b6df..dd107af28e3d 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -436,6 +436,7 @@ static void fm10k_restore_vxlan_port(struct fm10k_intfc *interface)
  * @netdev: network interface device structure
  * @sa_family: Address family of new port
  * @port: port number used for VXLAN
+ * @type: Enumerated value specifying udp encapsulation type
  *
  * This function is called when a new VXLAN interface has added a new port
  * number to the range that is currently in use for VXLAN.  The new port
@@ -444,10 +445,13 @@ static void fm10k_restore_vxlan_port(struct fm10k_intfc *interface)
  * is always used as the VXLAN port number for offloads.
  **/
 static void fm10k_add_vxlan_port(struct net_device *dev,
-				 sa_family_t sa_family, __be16 port) {
+				 sa_family_t sa_family, __be16 port,
+				 unsigned int type) {
 	struct fm10k_intfc *interface = netdev_priv(dev);
 	struct fm10k_vxlan_port *vxlan_port;
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
 	/* only the PF supports configuring tunnels */
 	if (interface->hw.mac.type != fm10k_mac_pf)
 		return;
@@ -480,6 +484,7 @@ insert_tail:
  * @netdev: network interface device structure
  * @sa_family: Address family of freed port
  * @port: port number used for VXLAN
+ * @type: Enumerated value specifying udp encapsulation type
  *
  * This function is called when a new VXLAN interface has freed a port
  * number from the range that is currently in use for VXLAN.  The freed
@@ -487,10 +492,13 @@ insert_tail:
  * the port number for offloads.
  **/
 static void fm10k_del_vxlan_port(struct net_device *dev,
-				 sa_family_t sa_family, __be16 port) {
+				 sa_family_t sa_family, __be16 port,
+				 unsigned int type) {
 	struct fm10k_intfc *interface = netdev_priv(dev);
 	struct fm10k_vxlan_port *vxlan_port;
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
 	if (interface->hw.mac.type != fm10k_mac_pf)
 		return;
 
@@ -555,7 +563,7 @@ int fm10k_open(struct net_device *netdev)
 
 #ifdef CONFIG_FM10K_VXLAN
 	/* update VXLAN port configuration */
-	vxlan_get_rx_port(netdev);
+	udp_tunnel_get_rx_port(netdev);
 #endif
 
 	fm10k_up(interface);
@@ -1375,8 +1383,8 @@ static const struct net_device_ops fm10k_netdev_ops = {
 	.ndo_set_vf_vlan	= fm10k_ndo_set_vf_vlan,
 	.ndo_set_vf_rate	= fm10k_ndo_set_vf_bw,
 	.ndo_get_vf_config	= fm10k_ndo_get_vf_config,
-	.ndo_add_vxlan_port	= fm10k_add_vxlan_port,
-	.ndo_del_vxlan_port	= fm10k_del_vxlan_port,
+	.ndo_add_udp_enc_port	= fm10k_add_vxlan_port,
+	.ndo_del_udp_enc_port	= fm10k_del_vxlan_port,
 	.ndo_dfwd_add_station	= fm10k_dfwd_add_station,
 	.ndo_dfwd_del_station	= fm10k_dfwd_del_station,
 #ifdef CONFIG_NET_POLL_CONTROLLER

[net-next PATCH 07/15] i40e: Move all UDP port notifiers to single function

[Alexander Duyck]

This patch goes through and combines the notifiers for VXLAN and GENEVE
into a single function for each action.  So there is now one combined
function for getting ports, one for adding the ports, and one for deleting
the ports.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c |  148 ++++++---------------------
 1 file changed, 35 insertions(+), 113 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 68172759bf71..8695ef467c58 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5367,14 +5367,9 @@ int i40e_open(struct net_device *netdev)
 						       TCP_FLAG_CWR) >> 16);
 	wr32(&pf->hw, I40E_GLLAN_TSOMSK_L, be32_to_cpu(TCP_FLAG_CWR) >> 16);
 
-#ifdef CONFIG_I40E_VXLAN
-	vxlan_get_rx_port(netdev);
+#if defined(CONFIG_I40E_VXLAN) || defined(CONFIG_I40E_GENEVE)
+	udp_tunnel_get_rx_port(netdev);
 #endif
-#ifdef CONFIG_I40E_GENEVE
-	if (pf->flags & I40E_FLAG_GENEVE_OFFLOAD_CAPABLE)
-		geneve_get_rx_port(netdev);
-#endif
-
 	i40e_notify_client_of_netdev_open(vsi);
 
 	return 0;
@@ -8678,17 +8673,16 @@ static u8 i40e_get_udp_port_idx(struct i40e_pf *pf, __be16 port)
 	return i;
 }
 
-#endif
-
-#if IS_ENABLED(CONFIG_VXLAN)
 /**
- * i40e_add_vxlan_port - Get notifications about VXLAN ports that come up
+ * i40e_add_udp_enc_port - Get notifications about UDP tunnel ports that come up
  * @netdev: This physical port's netdev
- * @sa_family: Socket Family that VXLAN is notifying us about
- * @port: New UDP port number that VXLAN started listening to
+ * @sa_family: Socket Family that tunnel is notifying us about
+ * @port: New UDP port number that tunnel started listening to
+ * @type: Enumerated type specifying UDP encapsulation type to be offloaded
  **/
-static void i40e_add_vxlan_port(struct net_device *netdev,
-				sa_family_t sa_family, __be16 port)
+static void i40e_add_udp_enc_port(struct net_device *netdev,
+				  sa_family_t sa_family, __be16 port,
+				  unsigned int type)
 {
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
@@ -8700,7 +8694,7 @@ static void i40e_add_vxlan_port(struct net_device *netdev,
 
 	/* Check if port already exists */
 	if (idx < I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
-		netdev_info(netdev, "vxlan port %d already offloaded\n",
+		netdev_info(netdev, "port %d already offloaded\n",
 			    ntohs(port));
 		return;
 	}
@@ -8709,112 +8703,46 @@ static void i40e_add_vxlan_port(struct net_device *netdev,
 	next_idx = i40e_get_udp_port_idx(pf, 0);
 
 	if (next_idx == I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
-		netdev_info(netdev, "maximum number of vxlan UDP ports reached, not adding port %d\n",
-			    ntohs(port));
-		return;
-	}
-
-	/* New port: add it and mark its index in the bitmap */
-	pf->udp_ports[next_idx].index = port;
-	pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_VXLAN;
-	pf->pending_udp_bitmap |= BIT_ULL(next_idx);
-	pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
-}
-
-/**
- * i40e_del_vxlan_port - Get notifications about VXLAN ports that go away
- * @netdev: This physical port's netdev
- * @sa_family: Socket Family that VXLAN is notifying us about
- * @port: UDP port number that VXLAN stopped listening to
- **/
-static void i40e_del_vxlan_port(struct net_device *netdev,
-				sa_family_t sa_family, __be16 port)
-{
-	struct i40e_netdev_priv *np = netdev_priv(netdev);
-	struct i40e_vsi *vsi = np->vsi;
-	struct i40e_pf *pf = vsi->back;
-	u8 idx;
-
-	idx = i40e_get_udp_port_idx(pf, port);
-
-	/* Check if port already exists */
-	if (idx < I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
-		/* if port exists, set it to 0 (mark for deletion)
-		 * and make it pending
-		 */
-		pf->udp_ports[idx].index = 0;
-		pf->pending_udp_bitmap |= BIT_ULL(idx);
-		pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
-	} else {
-		netdev_warn(netdev, "vxlan port %d was not found, not deleting\n",
-			    ntohs(port));
-	}
-}
-#endif
-
-#if IS_ENABLED(CONFIG_GENEVE)
-/**
- * i40e_add_geneve_port - Get notifications about GENEVE ports that come up
- * @netdev: This physical port's netdev
- * @sa_family: Socket Family that GENEVE is notifying us about
- * @port: New UDP port number that GENEVE started listening to
- **/
-static void i40e_add_geneve_port(struct net_device *netdev,
-				 sa_family_t sa_family, __be16 port)
-{
-	struct i40e_netdev_priv *np = netdev_priv(netdev);
-	struct i40e_vsi *vsi = np->vsi;
-	struct i40e_pf *pf = vsi->back;
-	u8 next_idx;
-	u8 idx;
-
-	if (!(pf->flags & I40E_FLAG_GENEVE_OFFLOAD_CAPABLE))
-		return;
-
-	idx = i40e_get_udp_port_idx(pf, port);
-
-	/* Check if port already exists */
-	if (idx < I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
-		netdev_info(netdev, "udp port %d already offloaded\n",
+		netdev_info(netdev, "maximum number of offloaded UDP ports reached, not adding port %d\n",
 			    ntohs(port));
 		return;
 	}
 
-	/* Now check if there is space to add the new port */
-	next_idx = i40e_get_udp_port_idx(pf, 0);
-
-	if (next_idx == I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
-		netdev_info(netdev, "maximum number of UDP ports reached, not adding port %d\n",
-			    ntohs(port));
+	switch (type) {
+	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
+		pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_VXLAN;
+		break;
+	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
+		if (!(pf->flags & I40E_FLAG_GENEVE_OFFLOAD_CAPABLE))
+			return;
+		pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_NGE;
+		break;
+	default:
 		return;
 	}
 
 	/* New port: add it and mark its index in the bitmap */
 	pf->udp_ports[next_idx].index = port;
-	pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_NGE;
 	pf->pending_udp_bitmap |= BIT_ULL(next_idx);
 	pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
-
-	dev_info(&pf->pdev->dev, "adding geneve port %d\n", ntohs(port));
 }
 
 /**
- * i40e_del_geneve_port - Get notifications about GENEVE ports that go away
+ * i40e_del_udp_enc_port - Get notifications about UDP tunnel ports that go away
  * @netdev: This physical port's netdev
- * @sa_family: Socket Family that GENEVE is notifying us about
- * @port: UDP port number that GENEVE stopped listening to
+ * @sa_family: Socket Family that tunnel is notifying us about
+ * @port: UDP port number that tunnel stopped listening to
+ * @type: Enumerated type specifying UDP encapsulation type to be offloaded
  **/
-static void i40e_del_geneve_port(struct net_device *netdev,
-				 sa_family_t sa_family, __be16 port)
+static void i40e_del_udp_enc_port(struct net_device *netdev,
+				  sa_family_t sa_family, __be16 port,
+				  unsigned int type)
 {
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
 	u8 idx;
 
-	if (!(pf->flags & I40E_FLAG_GENEVE_OFFLOAD_CAPABLE))
-		return;
-
 	idx = i40e_get_udp_port_idx(pf, port);
 
 	/* Check if port already exists */
@@ -8825,13 +8753,11 @@ static void i40e_del_geneve_port(struct net_device *netdev,
 		pf->udp_ports[idx].index = 0;
 		pf->pending_udp_bitmap |= BIT_ULL(idx);
 		pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
-
-		dev_info(&pf->pdev->dev, "deleting geneve port %d\n",
-			 ntohs(port));
-	} else {
-		netdev_warn(netdev, "geneve port %d was not found, not deleting\n",
-			    ntohs(port));
+		return;
 	}
+
+	netdev_warn(netdev, "UDP port %d was not found, not deleting\n",
+		    ntohs(port));
 }
 #endif
 
@@ -9063,13 +8989,9 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_set_vf_link_state	= i40e_ndo_set_vf_link_state,
 	.ndo_set_vf_spoofchk	= i40e_ndo_set_vf_spoofchk,
 	.ndo_set_vf_trust	= i40e_ndo_set_vf_trust,
-#if IS_ENABLED(CONFIG_VXLAN)
-	.ndo_add_vxlan_port	= i40e_add_vxlan_port,
-	.ndo_del_vxlan_port	= i40e_del_vxlan_port,
-#endif
-#if IS_ENABLED(CONFIG_GENEVE)
-	.ndo_add_geneve_port	= i40e_add_geneve_port,
-	.ndo_del_geneve_port	= i40e_del_geneve_port,
+#if IS_ENABLED(CONFIG_VXLAN) || IS_ENABLED(CONFIG_GENEVE)
+	.ndo_add_udp_enc_port	= i40e_add_udp_enc_port,
+	.ndo_del_udp_enc_port	= i40e_del_udp_enc_port,
 #endif
 	.ndo_get_phys_port_id	= i40e_get_phys_port_id,
 	.ndo_fdb_add		= i40e_ndo_fdb_add,

[net-next PATCH 08/15] ixgbe: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type is VXLAN we can maintain the same
functionality.

In addition I updated the socket address family check so that instead of
excluding IPv6 we instead abort of type is not IPv4.  This makes much more
sense as we should only be supporting IPv4 outer addresses on this
hardware.

The last change is that I pulled the rtnl_lock/unlock into the conditional
statement for IXGBE_FLAG2_VXLAN_REREG_NEEDED.  The motivation behind this
is to avoid unneeded bouncing of the mutex which will just slow down the
handling of this call anyway.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   36 ++++++++++++++-----------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b488b52974f7..05f095509122 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6159,7 +6159,7 @@ int ixgbe_open(struct net_device *netdev)
 
 	ixgbe_clear_vxlan_port(adapter);
 #ifdef CONFIG_IXGBE_VXLAN
-	vxlan_get_rx_port(netdev);
+	udp_tunnel_get_rx_port(netdev);
 #endif
 
 	return 0;
@@ -7263,12 +7263,12 @@ static void ixgbe_service_task(struct work_struct *work)
 		return;
 	}
 #ifdef CONFIG_IXGBE_VXLAN
-	rtnl_lock();
 	if (adapter->flags2 & IXGBE_FLAG2_VXLAN_REREG_NEEDED) {
+		rtnl_lock();
 		adapter->flags2 &= ~IXGBE_FLAG2_VXLAN_REREG_NEEDED;
-		vxlan_get_rx_port(adapter->netdev);
+		udp_tunnel_get_rx_port(adapter->netdev);
+		rtnl_unlock();
 	}
-	rtnl_unlock();
 #endif /* CONFIG_IXGBE_VXLAN */
 	ixgbe_reset_subtask(adapter);
 	ixgbe_phy_interrupt_subtask(adapter);
@@ -8858,23 +8858,26 @@ static int ixgbe_set_features(struct net_device *netdev,
 	return 0;
 }
 
-#ifdef CONFIG_IXGBE_VXLAN
 /**
  * ixgbe_add_vxlan_port - Get notifications about VXLAN ports that come up
  * @dev: The port's netdev
  * @sa_family: Socket Family that VXLAN is notifiying us about
  * @port: New UDP port number that VXLAN started listening to
+ * @type: Enumerated type specifying UDP tunnel type
  **/
 static void ixgbe_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
-				 __be16 port)
+				 __be16 port, unsigned int type)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
 	struct ixgbe_hw *hw = &adapter->hw;
 
-	if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
+	if (sa_family != AF_INET)
 		return;
 
-	if (sa_family == AF_INET6)
+	if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
 		return;
 
 	if (adapter->vxlan_port == port)
@@ -8896,16 +8899,20 @@ static void ixgbe_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
  * @dev: The port's netdev
  * @sa_family: Socket Family that VXLAN is notifying us about
  * @port: UDP port number that VXLAN stopped listening to
+ * @type: Enumerated type specifying UDP tunnel type
  **/
 static void ixgbe_del_vxlan_port(struct net_device *dev, sa_family_t sa_family,
-				 __be16 port)
+				 __be16 port, unsigned int type)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
 
-	if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
+	if (sa_family != AF_INET)
 		return;
 
-	if (sa_family == AF_INET6)
+	if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
 		return;
 
 	if (adapter->vxlan_port != port) {
@@ -8917,7 +8924,6 @@ static void ixgbe_del_vxlan_port(struct net_device *dev, sa_family_t sa_family,
 	ixgbe_clear_vxlan_port(adapter);
 	adapter->flags2 |= IXGBE_FLAG2_VXLAN_REREG_NEEDED;
 }
-#endif /* CONFIG_IXGBE_VXLAN */
 
 static int ixgbe_ndo_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 			     struct net_device *dev,
@@ -9230,10 +9236,8 @@ static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_bridge_getlink	= ixgbe_ndo_bridge_getlink,
 	.ndo_dfwd_add_station	= ixgbe_fwd_add,
 	.ndo_dfwd_del_station	= ixgbe_fwd_del,
-#ifdef CONFIG_IXGBE_VXLAN
-	.ndo_add_vxlan_port	= ixgbe_add_vxlan_port,
-	.ndo_del_vxlan_port	= ixgbe_del_vxlan_port,
-#endif /* CONFIG_IXGBE_VXLAN */
+	.ndo_add_udp_enc_port	= ixgbe_add_vxlan_port,
+	.ndo_del_udp_enc_port	= ixgbe_del_vxlan_port,
 	.ndo_features_check	= ixgbe_features_check,
 };
 

[net-next PATCH 09/15] mlx4_en: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type is VXLAN we can maintain the same
functionality.

In addition I updated the socket address family check so that instead of
excluding IPv6 we instead abort of type is not IPv4.  This makes much more
sense as we should only be supporting IPv4 outer addresses on this
hardware.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   30 +++++++++++++++---------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 973391bfe286..d4d5fdffbcee 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1694,7 +1694,7 @@ int mlx4_en_start_port(struct net_device *dev)
 
 #ifdef CONFIG_MLX4_EN_VXLAN
 	if (priv->mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
-		vxlan_get_rx_port(dev);
+		udp_tunnel_get_rx_port(dev);
 #endif
 	priv->port_up = true;
 	netif_tx_start_all_queues(dev);
@@ -2392,15 +2392,19 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
 }
 
 static void mlx4_en_add_vxlan_port(struct  net_device *dev,
-				   sa_family_t sa_family, __be16 port)
+				   sa_family_t sa_family, __be16 port,
+				   unsigned int type)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	__be16 current_port;
 
-	if (priv->mdev->dev->caps.tunnel_offload_mode != MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
+	if (sa_family != AF_INET)
 		return;
 
-	if (sa_family == AF_INET6)
+	if (priv->mdev->dev->caps.tunnel_offload_mode != MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
 		return;
 
 	current_port = priv->vxlan_port;
@@ -2415,15 +2419,19 @@ static void mlx4_en_add_vxlan_port(struct  net_device *dev,
 }
 
 static void mlx4_en_del_vxlan_port(struct  net_device *dev,
-				   sa_family_t sa_family, __be16 port)
+				   sa_family_t sa_family, __be16 port,
+				   unsigned int type)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	__be16 current_port;
 
-	if (priv->mdev->dev->caps.tunnel_offload_mode != MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
+	if (sa_family != AF_INET)
 		return;
 
-	if (sa_family == AF_INET6)
+	if (priv->mdev->dev->caps.tunnel_offload_mode != MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
 		return;
 
 	current_port = priv->vxlan_port;
@@ -2507,8 +2515,8 @@ static const struct net_device_ops mlx4_netdev_ops = {
 #endif
 	.ndo_get_phys_port_id	= mlx4_en_get_phys_port_id,
 #ifdef CONFIG_MLX4_EN_VXLAN
-	.ndo_add_vxlan_port	= mlx4_en_add_vxlan_port,
-	.ndo_del_vxlan_port	= mlx4_en_del_vxlan_port,
+	.ndo_add_udp_enc_port	= mlx4_en_add_vxlan_port,
+	.ndo_del_udp_enc_port	= mlx4_en_del_vxlan_port,
 	.ndo_features_check	= mlx4_en_features_check,
 #endif
 	.ndo_set_tx_maxrate	= mlx4_en_set_tx_maxrate,
@@ -2545,8 +2553,8 @@ static const struct net_device_ops mlx4_netdev_ops_master = {
 #endif
 	.ndo_get_phys_port_id	= mlx4_en_get_phys_port_id,
 #ifdef CONFIG_MLX4_EN_VXLAN
-	.ndo_add_vxlan_port	= mlx4_en_add_vxlan_port,
-	.ndo_del_vxlan_port	= mlx4_en_del_vxlan_port,
+	.ndo_add_udp_enc_port	= mlx4_en_add_vxlan_port,
+	.ndo_del_udp_enc_port	= mlx4_en_del_vxlan_port,
 	.ndo_features_check	= mlx4_en_features_check,
 #endif
 	.ndo_set_tx_maxrate	= mlx4_en_set_tx_maxrate,

[net-next PATCH 10/15] mlx5_en: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type is VXLAN we can maintain the same
functionality.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index f5c8d5db25a8..217ea4ba1a29 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2520,10 +2520,14 @@ static int mlx5e_get_vf_stats(struct net_device *dev,
 }
 
 static void mlx5e_add_vxlan_port(struct net_device *netdev,
-				 sa_family_t sa_family, __be16 port)
+				 sa_family_t sa_family, __be16 port,
+				 unsigned int type)
 {
 	struct mlx5e_priv *priv = netdev_priv(netdev);
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	if (!mlx5e_vxlan_allowed(priv->mdev))
 		return;
 
@@ -2531,10 +2535,14 @@ static void mlx5e_add_vxlan_port(struct net_device *netdev,
 }
 
 static void mlx5e_del_vxlan_port(struct net_device *netdev,
-				 sa_family_t sa_family, __be16 port)
+				 sa_family_t sa_family, __be16 port,
+				 unsigned int type)
 {
 	struct mlx5e_priv *priv = netdev_priv(netdev);
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	if (!mlx5e_vxlan_allowed(priv->mdev))
 		return;
 
@@ -2624,8 +2632,8 @@ static const struct net_device_ops mlx5e_netdev_ops_sriov = {
 	.ndo_set_features        = mlx5e_set_features,
 	.ndo_change_mtu          = mlx5e_change_mtu,
 	.ndo_do_ioctl            = mlx5e_ioctl,
-	.ndo_add_vxlan_port      = mlx5e_add_vxlan_port,
-	.ndo_del_vxlan_port      = mlx5e_del_vxlan_port,
+	.ndo_add_udp_enc_port	 = mlx5e_add_vxlan_port,
+	.ndo_del_udp_enc_port	 = mlx5e_del_vxlan_port,
 	.ndo_features_check      = mlx5e_features_check,
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	 = mlx5e_rx_flow_steer,
@@ -3128,7 +3136,7 @@ static void *mlx5e_create_netdev(struct mlx5_core_dev *mdev)
 
 	if (mlx5e_vxlan_allowed(mdev)) {
 		rtnl_lock();
-		vxlan_get_rx_port(netdev);
+		udp_tunnel_get_rx_port(netdev);
 		rtnl_unlock();
 	}
 

[net-next PATCH 11/15] nfp: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type is VXLAN we can maintain the same
functionality.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 .../net/ethernet/netronome/nfp/nfp_net_common.c    |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index fa47c14c743a..5adddeda1e60 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -1979,7 +1979,7 @@ static int __nfp_net_set_config_and_enable(struct nfp_net *nn)
 	if (nn->ctrl & NFP_NET_CFG_CTRL_VXLAN) {
 		memset(&nn->vxlan_ports, 0, sizeof(nn->vxlan_ports));
 		memset(&nn->vxlan_usecnt, 0, sizeof(nn->vxlan_usecnt));
-		vxlan_get_rx_port(nn->netdev);
+		udp_tunnel_get_rx_port(nn->netdev);
 	}
 
 	return err;
@@ -2551,11 +2551,15 @@ static int nfp_net_find_vxlan_idx(struct nfp_net *nn, __be16 port)
 }
 
 static void nfp_net_add_vxlan_port(struct net_device *netdev,
-				   sa_family_t sa_family, __be16 port)
+				   sa_family_t sa_family, __be16 port,
+				   unsigned int type)
 {
 	struct nfp_net *nn = netdev_priv(netdev);
 	int idx;
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	idx = nfp_net_find_vxlan_idx(nn, port);
 	if (idx == -ENOSPC)
 		return;
@@ -2565,11 +2569,15 @@ static void nfp_net_add_vxlan_port(struct net_device *netdev,
 }
 
 static void nfp_net_del_vxlan_port(struct net_device *netdev,
-				   sa_family_t sa_family, __be16 port)
+				   sa_family_t sa_family, __be16 port,
+				   unsigned int type)
 {
 	struct nfp_net *nn = netdev_priv(netdev);
 	int idx;
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	idx = nfp_net_find_vxlan_idx(nn, port);
 	if (!nn->vxlan_usecnt[idx] || idx == -ENOSPC)
 		return;
@@ -2589,8 +2597,8 @@ static const struct net_device_ops nfp_net_netdev_ops = {
 	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_set_features	= nfp_net_set_features,
 	.ndo_features_check	= nfp_net_features_check,
-	.ndo_add_vxlan_port     = nfp_net_add_vxlan_port,
-	.ndo_del_vxlan_port     = nfp_net_del_vxlan_port,
+	.ndo_add_udp_enc_port	= nfp_net_add_vxlan_port,
+	.ndo_del_udp_enc_port	= nfp_net_del_vxlan_port,
 };
 
 /**

[net-next PATCH 12/15] qede: Move all UDP port notifiers to single function

[Alexander Duyck]

This patch goes through and combines the notifiers for VXLAN and GENEVE
into a single function for each action.  So there is now one combined
function for getting ports, one for adding the ports, and one for deleting
the ports.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/qlogic/qede/qede_main.c |  113 ++++++++++++++------------
 1 file changed, 59 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 423168ba7c98..c5d9e0f15ae9 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2112,72 +2112,84 @@ int qede_set_features(struct net_device *dev, netdev_features_t features)
 	return 0;
 }
 
-#ifdef CONFIG_QEDE_VXLAN
-static void qede_add_vxlan_port(struct net_device *dev,
-				sa_family_t sa_family, __be16 port)
+#if defined(CONFIG_QEDE_VXLAN) || defined(CONFIG_QEDE_GENEVE)
+static void qede_add_udp_enc_port(struct net_device *dev,
+				  sa_family_t sa_family, __be16 port,
+				  unsigned int type)
 {
 	struct qede_dev *edev = netdev_priv(dev);
 	u16 t_port = ntohs(port);
 
-	if (edev->vxlan_dst_port)
-		return;
+	switch (type) {
+#ifdef CONFIG_QEDE_VXLAN
+	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
+		if (edev->vxlan_dst_port)
+			return;
 
-	edev->vxlan_dst_port = t_port;
+		edev->vxlan_dst_port = t_port;
 
-	DP_VERBOSE(edev, QED_MSG_DEBUG, "Added vxlan port=%d", t_port);
+		DP_VERBOSE(edev, QED_MSG_DEBUG, "Added vxlan port=%d",
+			   t_port);
 
-	set_bit(QEDE_SP_VXLAN_PORT_CONFIG, &edev->sp_flags);
-	schedule_delayed_work(&edev->sp_task, 0);
-}
+		set_bit(QEDE_SP_VXLAN_PORT_CONFIG, &edev->sp_flags);
+		break;
+#endif
+#ifdef CONFIG_QEDE_GENEVE
+	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
+		if (edev->geneve_dst_port)
+			return;
 
-static void qede_del_vxlan_port(struct net_device *dev,
-				sa_family_t sa_family, __be16 port)
-{
-	struct qede_dev *edev = netdev_priv(dev);
-	u16 t_port = ntohs(port);
+		edev->geneve_dst_port = t_port;
 
-	if (t_port != edev->vxlan_dst_port)
+		DP_VERBOSE(edev, QED_MSG_DEBUG, "Added geneve port=%d",
+			   t_port);
+		set_bit(QEDE_SP_GENEVE_PORT_CONFIG, &edev->sp_flags);
+		break;
+#endif
+	default:
 		return;
+	}
 
-	edev->vxlan_dst_port = 0;
-
-	DP_VERBOSE(edev, QED_MSG_DEBUG, "Deleted vxlan port=%d", t_port);
-
-	set_bit(QEDE_SP_VXLAN_PORT_CONFIG, &edev->sp_flags);
 	schedule_delayed_work(&edev->sp_task, 0);
 }
-#endif
 
-#ifdef CONFIG_QEDE_GENEVE
-static void qede_add_geneve_port(struct net_device *dev,
-				 sa_family_t sa_family, __be16 port)
+static void qede_del_udp_enc_port(struct net_device *dev,
+				  sa_family_t sa_family, __be16 port,
+				  unsigned int type)
 {
 	struct qede_dev *edev = netdev_priv(dev);
 	u16 t_port = ntohs(port);
 
-	if (edev->geneve_dst_port)
-		return;
+	switch (type) {
+#ifdef CONFIG_QEDE_VXLAN
+	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
+		if (t_port != edev->vxlan_dst_port)
+			return;
 
-	edev->geneve_dst_port = t_port;
+		edev->vxlan_dst_port = 0;
 
-	DP_VERBOSE(edev, QED_MSG_DEBUG, "Added geneve port=%d", t_port);
-	set_bit(QEDE_SP_GENEVE_PORT_CONFIG, &edev->sp_flags);
-	schedule_delayed_work(&edev->sp_task, 0);
-}
+		DP_VERBOSE(edev, QED_MSG_DEBUG, "Deleted vxlan port=%d",
+			   t_port);
 
-static void qede_del_geneve_port(struct net_device *dev,
-				 sa_family_t sa_family, __be16 port)
-{
-	struct qede_dev *edev = netdev_priv(dev);
-	u16 t_port = ntohs(port);
+		set_bit(QEDE_SP_VXLAN_PORT_CONFIG, &edev->sp_flags);
+		break;
+#endif
+#ifdef CONFIG_QEDE_GENEVE
+	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
+		if (t_port != edev->geneve_dst_port)
+			return;
 
-	if (t_port != edev->geneve_dst_port)
-		return;
+		edev->geneve_dst_port = 0;
 
-	edev->geneve_dst_port = 0;
+		DP_VERBOSE(edev, QED_MSG_DEBUG, "Deleted geneve port=%d",
+			   t_port);
+		set_bit(QEDE_SP_GENEVE_PORT_CONFIG, &edev->sp_flags);
+		break;
+#endif
+	default:
+		return;
+	}
 
-	DP_VERBOSE(edev, QED_MSG_DEBUG, "Deleted geneve port=%d", t_port);
-	set_bit(QEDE_SP_GENEVE_PORT_CONFIG, &edev->sp_flags);
 	schedule_delayed_work(&edev->sp_task, 0);
 }
 #endif
@@ -2204,13 +2216,9 @@ static const struct net_device_ops qede_netdev_ops = {
 	.ndo_get_vf_config = qede_get_vf_config,
 	.ndo_set_vf_rate = qede_set_vf_rate,
 #endif
-#ifdef CONFIG_QEDE_VXLAN
-	.ndo_add_vxlan_port = qede_add_vxlan_port,
-	.ndo_del_vxlan_port = qede_del_vxlan_port,
-#endif
-#ifdef CONFIG_QEDE_GENEVE
-	.ndo_add_geneve_port = qede_add_geneve_port,
-	.ndo_del_geneve_port = qede_del_geneve_port,
+#if defined(CONFIG_QEDE_VXLAN) || defined(CONFIG_QEDE_GENEVE)
+	.ndo_add_udp_enc_port = qede_add_udp_enc_port,
+	.ndo_del_udp_enc_port = qede_del_udp_enc_port,
 #endif
 };
 
@@ -3579,11 +3587,8 @@ static int qede_open(struct net_device *ndev)
 	if (rc)
 		return rc;
 
-#ifdef CONFIG_QEDE_VXLAN
-	vxlan_get_rx_port(ndev);
-#endif
-#ifdef CONFIG_QEDE_GENEVE
-	geneve_get_rx_port(ndev);
+#if defined(CONFIG_QEDE_VXLAN) || defined(CONFIG_QEDE_GENEVE)
+	udp_tunnel_get_rx_port(ndev);
 #endif
 	return 0;
 }

[net-next PATCH 13/15] qlcnic: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alexander Duyck]

This change replaces the network device operations for adding or removing a
VXLAN port with operations that are more generically defined to be used for
any UDP offload port but provide a type.  As such by just adding a line to
verify that the offload type is VXLAN we can maintain the same
functionality.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 1c29105b6c36..e2840f55f09f 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -476,11 +476,15 @@ static int qlcnic_get_phys_port_id(struct net_device *netdev,
 
 #ifdef CONFIG_QLCNIC_VXLAN
 static void qlcnic_add_vxlan_port(struct net_device *netdev,
-				  sa_family_t sa_family, __be16 port)
+				  sa_family_t sa_family, __be16 port,
+				  unsigned int type)
 {
 	struct qlcnic_adapter *adapter = netdev_priv(netdev);
 	struct qlcnic_hardware_context *ahw = adapter->ahw;
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	/* Adapter supports only one VXLAN port. Use very first port
 	 * for enabling offload
 	 */
@@ -498,11 +502,15 @@ static void qlcnic_add_vxlan_port(struct net_device *netdev,
 }
 
 static void qlcnic_del_vxlan_port(struct net_device *netdev,
-				  sa_family_t sa_family, __be16 port)
+				  sa_family_t sa_family, __be16 port,
+				  unsigned int type)
 {
 	struct qlcnic_adapter *adapter = netdev_priv(netdev);
 	struct qlcnic_hardware_context *ahw = adapter->ahw;
 
+	if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
+		return;
+
 	if (!qlcnic_encap_rx_offload(adapter) || !ahw->vxlan_port_count ||
 	    (ahw->vxlan_port != ntohs(port)))
 		return;
@@ -540,8 +548,8 @@ static const struct net_device_ops qlcnic_netdev_ops = {
 	.ndo_fdb_dump		= qlcnic_fdb_dump,
 	.ndo_get_phys_port_id	= qlcnic_get_phys_port_id,
 #ifdef CONFIG_QLCNIC_VXLAN
-	.ndo_add_vxlan_port	= qlcnic_add_vxlan_port,
-	.ndo_del_vxlan_port	= qlcnic_del_vxlan_port,
+	.ndo_add_udp_enc_port	= qlcnic_add_vxlan_port,
+	.ndo_del_udp_enc_port	= qlcnic_del_vxlan_port,
 	.ndo_features_check	= qlcnic_features_check,
 #endif
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -2017,7 +2025,7 @@ qlcnic_attach(struct qlcnic_adapter *adapter)
 
 #ifdef CONFIG_QLCNIC_VXLAN
 	if (qlcnic_encap_rx_offload(adapter))
-		vxlan_get_rx_port(netdev);
+		udp_tunnel_get_rx_port(netdev);
 #endif
 
 	adapter->is_up = QLCNIC_ADAPTER_UP_MAGIC;

[net-next PATCH 14/15] net: Remove deprecated tunnel specific UDP offload functions

[Alexander Duyck]

Now that we have all the drivers using udp_tunnel_get_rx_ports,
ndo_add_udp_enc_rx_port, and ndo_del_udp_enc_rx_port we can drop the
function calls that were specific to VXLAN and GENEVE.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 include/linux/netdevice.h |   12 ------------
 include/net/geneve.h      |    5 -----
 include/net/vxlan.h       |    5 -----
 net/ipv4/udp_tunnel.c     |   41 ++---------------------------------------
 4 files changed, 2 insertions(+), 61 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e959b6348f91..ee3266d8aeaf 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1257,18 +1257,6 @@ struct net_device_ops {
 							struct netdev_phys_item_id *ppid);
 	int			(*ndo_get_phys_port_name)(struct net_device *dev,
 							  char *name, size_t len);
-	void			(*ndo_add_vxlan_port)(struct  net_device *dev,
-						      sa_family_t sa_family,
-						      __be16 port);
-	void			(*ndo_del_vxlan_port)(struct  net_device *dev,
-						      sa_family_t sa_family,
-						      __be16 port);
-	void			(*ndo_add_geneve_port)(struct  net_device *dev,
-						       sa_family_t sa_family,
-						       __be16 port);
-	void			(*ndo_del_geneve_port)(struct  net_device *dev,
-						       sa_family_t sa_family,
-						       __be16 port);
 	void			(*ndo_add_udp_enc_port)(struct  net_device *dev,
 						       sa_family_t sa_family,
 						       __be16 port,
diff --git a/include/net/geneve.h b/include/net/geneve.h
index 7638ec62c5e1..ec0327d4331b 100644
--- a/include/net/geneve.h
+++ b/include/net/geneve.h
@@ -59,11 +59,6 @@ struct genevehdr {
 	struct geneve_opt options[];
 };
 
-static inline void geneve_get_rx_port(struct net_device *netdev)
-{
-	udp_tunnel_get_rx_port(netdev);
-}
-
 #ifdef CONFIG_INET
 struct net_device *geneve_dev_create_fb(struct net *net, const char *name,
 					u8 name_assign_type, u16 dst_port);
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 2c4f8fcd5a3b..a958cbea8676 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -393,11 +393,6 @@ static inline __be32 vxlan_compute_rco(unsigned int start, unsigned int offset)
 	return vni_field;
 }
 
-static inline void vxlan_get_rx_port(struct net_device *netdev)
-{
-	udp_tunnel_get_rx_port(netdev);
-}
-
 static inline unsigned short vxlan_get_sk_family(struct vxlan_sock *vs)
 {
 	return vs->sock->sk->sk_family;
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
index a22677cd41d8..39dfe9e0e498 100644
--- a/net/ipv4/udp_tunnel.c
+++ b/net/ipv4/udp_tunnel.c
@@ -83,28 +83,10 @@ void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
 	sa_family_t sa_family = sk->sk_family;
 	__be16 port = inet_sk(sk)->inet_sport;
 
-	if (dev->netdev_ops->ndo_add_udp_enc_port) {
+	if (dev->netdev_ops->ndo_add_udp_enc_port)
 		dev->netdev_ops->ndo_add_udp_enc_port(dev, sa_family,
 						      port, type);
-		return;
-	}
 
-	switch (type) {
-	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
-		if (!dev->netdev_ops->ndo_add_vxlan_port)
-			break;
-
-		dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family, port);
-		break;
-	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
-		if (!dev->netdev_ops->ndo_add_geneve_port)
-			break;
-
-		dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
-		break;
-	default:
-		break;
-	}
 }
 EXPORT_SYMBOL_GPL(udp_tunnel_push_rx_port);
 
@@ -128,28 +110,9 @@ static void udp_tunnel_pull_rx_port(struct net_device *dev,
 	sa_family_t sa_family = sk->sk_family;
 	__be16 port = inet_sk(sk)->inet_sport;
 
-	if (dev->netdev_ops->ndo_del_udp_enc_port) {
+	if (dev->netdev_ops->ndo_del_udp_enc_port)
 		dev->netdev_ops->ndo_del_udp_enc_port(dev, sa_family,
 						      port, type);
-		return;
-	}
-
-	switch (type) {
-	case UDP_ENC_OFFLOAD_TYPE_VXLAN:
-		if (!dev->netdev_ops->ndo_del_vxlan_port)
-			break;
-
-		dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family, port);
-		break;
-	case UDP_ENC_OFFLOAD_TYPE_GENEVE:
-		if (!dev->netdev_ops->ndo_del_geneve_port)
-			break;
-
-		dev->netdev_ops->ndo_del_geneve_port(dev, sa_family, port);
-		break;
-	default:
-		break;
-	}
 }
 
 /* Notify netdevs that UDP port is no more listening */

[net-next PATCH 15/15] vxlan: Add new UDP encapsulation offload type for VXLAN-GPE

[Alexander Duyck]

The fact is VXLAN with Generic Protocol Extensions cannot be supported by
the same hardware parsers as that support VXLAN.  The protocol extensions
allow for things like a Next Protocol field which in turn allows for things
other than Ethernet to be passed over the tunnel.  Most existing parsers
will not know how to interpret this.

To resolve this I am giving VXLAN-GPE its own UDP encapsulation offload
type.  This way hardware that does support GPE can simply add this type to
the switch statement for VXLAN, and if they don't support it then this will
fix any issues where headers might be interpreted incorrectly.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/vxlan.c      |   12 ++++++++++--
 include/net/udp_tunnel.h |    1 +
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 72da056abdf4..501c9e4741ed 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -622,13 +622,19 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
 /* Notify netdevs that UDP port started listening */
 static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
 {
-	udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
+	udp_tunnel_notify_add_rx_port(vs->sock,
+				      (vs->flags & VXLAN_F_GPE) ?
+				      UDP_ENC_OFFLOAD_TYPE_VXLAN_GPE :
+				      UDP_ENC_OFFLOAD_TYPE_VXLAN);
 }
 
 /* Notify netdevs that UDP port is no more listening */
 static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
 {
-	udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
+	udp_tunnel_notify_del_rx_port(vs->sock,
+				      (vs->flags & VXLAN_F_GPE) ?
+				      UDP_ENC_OFFLOAD_TYPE_VXLAN_GPE :
+				      UDP_ENC_OFFLOAD_TYPE_VXLAN);
 }
 
 /* Add new entry to forwarding table -- assumes lock held */
@@ -2516,6 +2522,8 @@ static void vxlan_push_rx_ports(struct net_device *dev)
 	for (i = 0; i < PORT_HASH_SIZE; ++i) {
 		hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
 			udp_tunnel_push_rx_port(dev, vs->sock,
+						(vs->flags & VXLAN_F_GPE) ?
+						UDP_ENC_OFFLOAD_TYPE_VXLAN_GPE :
 						UDP_ENC_OFFLOAD_TYPE_VXLAN);
 	}
 	spin_unlock(&vn->sock_lock);
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 9ea813740231..cc85e9e53f3d 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -88,6 +88,7 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
 enum udp_enc_offloads {
 	UDP_ENC_OFFLOAD_TYPE_VXLAN,	/* RFC 7348 */
 	UDP_ENC_OFFLOAD_TYPE_GENEVE,	/* draft-ietf-nvo3-geneve */
+	UDP_ENC_OFFLOAD_TYPE_VXLAN_GPE,	/* draft-ietf-nvo3-vxlan-gpe */
 };
 
 /* Notify network devices of offloadable types */

Re: [net-next PATCH 02/15] net: Merge VXLAN and GENEVE push notifiers into a single notifier

[Hannes Frederic Sowa]

Hi Alex,

very cool series!

On 13.06.2016 19:48, Alexander Duyck wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d101e4d904ba..e959b6348f91 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1269,6 +1269,14 @@ struct net_device_ops {
>  	void			(*ndo_del_geneve_port)(struct  net_device *dev,
>  						       sa_family_t sa_family,
>  						       __be16 port);
> +	void			(*ndo_add_udp_enc_port)(struct  net_device *dev,
> +						       sa_family_t sa_family,
> +						       __be16 port,
> +						       unsigned int type);
> +	void			(*ndo_del_udp_enc_port)(struct  net_device *dev,
> +						       sa_family_t sa_family,
> +						       __be16 port,
> +						       unsigned int type);
>  	void*			(*ndo_dfwd_add_station)(struct net_device *pdev,
>  							struct net_device *dev);
>  	void			(*ndo_dfwd_del_station)(struct net_device *pdev,

What do you think about adding a struct as argument to
ndo_*_udp_enc_port? As a result we can much easier add new fields in
case future NICs allow us to e.g. specify a bound ip address?

Thanks,
Hannes

Re: [net-next PATCH 04/15] bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Jesse Gross]

On Mon, Jun 13, 2016 at 10:48 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This change replaces the network device operations for adding or removing a
> VXLAN port with operations that are more generically defined to be used for
> any UDP offload port but provide a type.  As such by just adding a line to
> verify that the offload type if VXLAN we can maintain the same
> functionality.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

It looks like bnxt actually has a hardcoded offload of the Geneve port
rather than using the registration (maybe it predated it?). It would
be nice to incorporate that into your unified handler as well.

I think you could actually just kill all of these
CONFIG_VXLAN/CONFIG_GENEVE checks (across all the drivers, not just
this one). They shouldn't be necessary any more now that there's no
longer strong linkage to the tunnel drivers.

Re: [Intel-wired-lan] [net-next PATCH 03/15] bnx2x: Move all UDP port notifiers to single function

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

Hi,

[auto build test ERROR on net/master]
[also build test ERROR on v4.7-rc3 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alexander-Duyck/Future-proof-tunnel-offload-handlers/20160614-020534
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:13046:26: error: 'bnx2x_add_udp_enc_port' undeclared here (not in a function)
     .ndo_add_udp_enc_port = bnx2x_add_udp_enc_port,
                             ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:13047:26: error: 'bnx2x_del_udp_enc_port' undeclared here (not in a function)
     .ndo_del_udp_enc_port = bnx2x_del_udp_enc_port,
                             ^~~~~~~~~~~~~~~~~~~~~~

vim +/bnx2x_add_udp_enc_port +13046 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c

 13040		.ndo_fcoe_get_wwn	= bnx2x_fcoe_get_wwn,
 13041	#endif
 13042	
 13043		.ndo_get_phys_port_id	= bnx2x_get_phys_port_id,
 13044		.ndo_set_vf_link_state	= bnx2x_set_vf_link_state,
 13045		.ndo_features_check	= bnx2x_features_check,
 13046		.ndo_add_udp_enc_port	= bnx2x_add_udp_enc_port,
 13047		.ndo_del_udp_enc_port	= bnx2x_del_udp_enc_port,
 13048	};
 13049	
 13050	static int bnx2x_set_coherency_mask(struct bnx2x *bp)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 37219 bytes --]

Re: [net-next PATCH 04/15] bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Hannes Frederic Sowa]

On 13.06.2016 20:41, Jesse Gross wrote:
> On Mon, Jun 13, 2016 at 10:48 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This change replaces the network device operations for adding or removing a
>> VXLAN port with operations that are more generically defined to be used for
>> any UDP offload port but provide a type.  As such by just adding a line to
>> verify that the offload type if VXLAN we can maintain the same
>> functionality.
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> 
> It looks like bnxt actually has a hardcoded offload of the Geneve port
> rather than using the registration (maybe it predated it?). It would
> be nice to incorporate that into your unified handler as well.
> 
> I think you could actually just kill all of these
> CONFIG_VXLAN/CONFIG_GENEVE checks (across all the drivers, not just
> this one). They shouldn't be necessary any more now that there's no
> longer strong linkage to the tunnel drivers.

Yes, agreed. We can remove the CONFIG_VXLAN and CONFIG_GENEVE stuff now.
But I think this can be a separate series.

Bye,
Hannes

Re: [net-next PATCH 04/15] bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Alex Duyck]

On Mon, Jun 13, 2016 at 12:14 PM, Hannes Frederic Sowa
<hannes@redhat.com> wrote:
> On 13.06.2016 20:41, Jesse Gross wrote:
>> On Mon, Jun 13, 2016 at 10:48 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>> This change replaces the network device operations for adding or removing a
>>> VXLAN port with operations that are more generically defined to be used for
>>> any UDP offload port but provide a type.  As such by just adding a line to
>>> verify that the offload type if VXLAN we can maintain the same
>>> functionality.
>>>
>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>>
>> It looks like bnxt actually has a hardcoded offload of the Geneve port
>> rather than using the registration (maybe it predated it?). It would
>> be nice to incorporate that into your unified handler as well.
>>
>> I think you could actually just kill all of these
>> CONFIG_VXLAN/CONFIG_GENEVE checks (across all the drivers, not just
>> this one). They shouldn't be necessary any more now that there's no
>> longer strong linkage to the tunnel drivers.
>
> Yes, agreed. We can remove the CONFIG_VXLAN and CONFIG_GENEVE stuff now.
> But I think this can be a separate series.

Actually it should be pretty easy to do all of this in the same
series.  I am directly effecting the code where it is wrapped up
anyway.  Dropping the defines will make this easier for me to test.

- Alex

Re: [net-next PATCH 04/15] bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[Michael Chan]

On Mon, Jun 13, 2016 at 11:41 AM, Jesse Gross <jesse@kernel.org> wrote:
> On Mon, Jun 13, 2016 at 10:48 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This change replaces the network device operations for adding or removing a
>> VXLAN port with operations that are more generically defined to be used for
>> any UDP offload port but provide a type.  As such by just adding a line to
>> verify that the offload type if VXLAN we can maintain the same
>> functionality.
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>
> It looks like bnxt actually has a hardcoded offload of the Geneve port
> rather than using the registration (maybe it predated it?).

It's not hardcoded.  The driver code just predated the ndo geneve calls.

>  It would
> be nice to incorporate that into your unified handler as well.
>
> I think you could actually just kill all of these
> CONFIG_VXLAN/CONFIG_GENEVE checks (across all the drivers, not just
> this one). They shouldn't be necessary any more now that there's no
> longer strong linkage to the tunnel drivers.

Agreed.

Re: [net-next PATCH 02/15] net: Merge VXLAN and GENEVE push notifiers into a single notifier

[Tom Herbert]

On Mon, Jun 13, 2016 at 10:57 AM, Hannes Frederic Sowa
<hannes@redhat.com> wrote:
> Hi Alex,
>
> very cool series!
>
> On 13.06.2016 19:48, Alexander Duyck wrote:
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index d101e4d904ba..e959b6348f91 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1269,6 +1269,14 @@ struct net_device_ops {
>>       void                    (*ndo_del_geneve_port)(struct  net_device *dev,
>>                                                      sa_family_t sa_family,
>>                                                      __be16 port);
>> +     void                    (*ndo_add_udp_enc_port)(struct  net_device *dev,
>> +                                                    sa_family_t sa_family,
>> +                                                    __be16 port,
>> +                                                    unsigned int type);
>> +     void                    (*ndo_del_udp_enc_port)(struct  net_device *dev,
>> +                                                    sa_family_t sa_family,
>> +                                                    __be16 port,
>> +                                                    unsigned int type);
>>       void*                   (*ndo_dfwd_add_station)(struct net_device *pdev,
>>                                                       struct net_device *dev);
>>       void                    (*ndo_dfwd_del_station)(struct net_device *pdev,
>
> What do you think about adding a struct as argument to
> ndo_*_udp_enc_port? As a result we can much easier add new fields in
> case future NICs allow us to e.g. specify a bound ip address?
>
This is why I think we should be using ntuple filtering for
encapsulation instead perpetuating the notion that encapsulation
offload can only be done based on receive ports.

Tom

> Thanks,
> Hannes
>

Re: [Intel-wired-lan] [net-next PATCH 04/15] bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3555 bytes --]

Hi,

[auto build test WARNING on net/master]
[also build test WARNING on v4.7-rc3 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alexander-Duyck/Future-proof-tunnel-offload-handlers/20160614-020534
config: x86_64-randconfig-x018-06140233 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/linux/list.h:4,
                    from include/linux/module.h:9,
                    from drivers/net/ethernet/broadcom/bnxt/bnxt.c:10:
   drivers/net/ethernet/broadcom/bnxt/bnxt.c: In function 'bnxt_add_vxlan_port':
   drivers/net/ethernet/broadcom/bnxt/bnxt.c:6056:14: error: 'UDP_ENC_OFFLOAD_TYPE_VXLAN' undeclared (first use in this function)
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
                 ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/net/ethernet/broadcom/bnxt/bnxt.c:6056:2: note: in expansion of macro 'if'
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
     ^~
   drivers/net/ethernet/broadcom/bnxt/bnxt.c:6056:14: note: each undeclared identifier is reported only once for each function it appears in
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
                 ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/net/ethernet/broadcom/bnxt/bnxt.c:6056:2: note: in expansion of macro 'if'
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
     ^~
   drivers/net/ethernet/broadcom/bnxt/bnxt.c: In function 'bnxt_del_vxlan_port':
   drivers/net/ethernet/broadcom/bnxt/bnxt.c:6081:14: error: 'UDP_ENC_OFFLOAD_TYPE_VXLAN' undeclared (first use in this function)
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
                 ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   drivers/net/ethernet/broadcom/bnxt/bnxt.c:6081:2: note: in expansion of macro 'if'
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
     ^~

vim +/if +6056 drivers/net/ethernet/broadcom/bnxt/bnxt.c

  6040			netdev_info(bp->dev, "Receive PF driver unload event!");
  6041	}
  6042	
  6043	#else
  6044	
  6045	static void bnxt_cfg_ntp_filters(struct bnxt *bp)
  6046	{
  6047	}
  6048	
  6049	#endif /* CONFIG_RFS_ACCEL */
  6050	
  6051	static void bnxt_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
  6052					__be16 port, unsigned int type)
  6053	{
  6054		struct bnxt *bp = netdev_priv(dev);
  6055	
> 6056		if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
  6057			return;
  6058	
  6059		if (sa_family != AF_INET6 && sa_family != AF_INET)
  6060			return;
  6061	
  6062		if (!netif_running(dev))
  6063			return;
  6064	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 28968 bytes --]

Re: [Intel-wired-lan] [net-next PATCH 04/15] bnxt: Replace ndo_add/del_vxlan_port with ndo_add/del_udp_enc_port

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]

Hi,

[auto build test ERROR on net/master]
[also build test ERROR on v4.7-rc3 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alexander-Duyck/Future-proof-tunnel-offload-handlers/20160614-020534
config: x86_64-randconfig-x014-06140233 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/net/ethernet/broadcom/bnxt/bnxt.c: In function 'bnxt_add_vxlan_port':
>> drivers/net/ethernet/broadcom/bnxt/bnxt.c:6056:14: error: 'UDP_ENC_OFFLOAD_TYPE_VXLAN' undeclared (first use in this function)
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/broadcom/bnxt/bnxt.c:6056:14: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/ethernet/broadcom/bnxt/bnxt.c: In function 'bnxt_del_vxlan_port':
   drivers/net/ethernet/broadcom/bnxt/bnxt.c:6081:14: error: 'UDP_ENC_OFFLOAD_TYPE_VXLAN' undeclared (first use in this function)
     if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/UDP_ENC_OFFLOAD_TYPE_VXLAN +6056 drivers/net/ethernet/broadcom/bnxt/bnxt.c

  6050	
  6051	static void bnxt_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
  6052					__be16 port, unsigned int type)
  6053	{
  6054		struct bnxt *bp = netdev_priv(dev);
  6055	
> 6056		if (type != UDP_ENC_OFFLOAD_TYPE_VXLAN)
  6057			return;
  6058	
  6059		if (sa_family != AF_INET6 && sa_family != AF_INET)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 31173 bytes --]

Re: [net-next PATCH 02/15] net: Merge VXLAN and GENEVE push notifiers into a single notifier

[Alexander Duyck]

On Mon, Jun 13, 2016 at 10:57 AM, Hannes Frederic Sowa
<hannes@redhat.com> wrote:
> Hi Alex,
>
> very cool series!
>
> On 13.06.2016 19:48, Alexander Duyck wrote:
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index d101e4d904ba..e959b6348f91 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1269,6 +1269,14 @@ struct net_device_ops {
>>       void                    (*ndo_del_geneve_port)(struct  net_device *dev,
>>                                                      sa_family_t sa_family,
>>                                                      __be16 port);
>> +     void                    (*ndo_add_udp_enc_port)(struct  net_device *dev,
>> +                                                    sa_family_t sa_family,
>> +                                                    __be16 port,
>> +                                                    unsigned int type);
>> +     void                    (*ndo_del_udp_enc_port)(struct  net_device *dev,
>> +                                                    sa_family_t sa_family,
>> +                                                    __be16 port,
>> +                                                    unsigned int type);
>>       void*                   (*ndo_dfwd_add_station)(struct net_device *pdev,
>>                                                       struct net_device *dev);
>>       void                    (*ndo_dfwd_del_station)(struct net_device *pdev,
>
> What do you think about adding a struct as argument to
> ndo_*_udp_enc_port? As a result we can much easier add new fields in
> case future NICs allow us to e.g. specify a bound ip address?

Actually that is probably a good idea.  Suggestions on the name are
welcome.  Otherwise I will try to come up with something in a bit as I
am currently going through and flushing out all the driver specific
VXLAN and GENEVE build flags.

- Alex

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Tom Herbert]

On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch merges the GENEVE and VXLAN code so that both functions pass
> through a shared code path.  This way we can start the effort of using a
> single function on the network device drivers to handle both of these
> tunnel offload types.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/geneve.c     |   48 ++++-------------------------
>  drivers/net/vxlan.c      |   46 ++++-----------------------
>  include/net/udp_tunnel.h |   12 +++++++
>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 103 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index cadefe4fdaa2..f5ce41532cf4 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>
>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>  {
> -       struct net_device *dev;
> -       struct sock *sk = gs->sock->sk;
> -       struct net *net = sock_net(sk);
> -       sa_family_t sa_family = geneve_get_sk_family(gs);
> -       __be16 port = inet_sk(sk)->inet_sport;
> -
> -       rcu_read_lock();
> -       for_each_netdev_rcu(net, dev) {
> -               if (dev->netdev_ops->ndo_add_geneve_port)
> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
> -                                                            port);
> -       }
> -       rcu_read_unlock();
> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>  }
>
>  static int geneve_hlen(struct genevehdr *gh)
> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>
>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>  {
> -       struct net_device *dev;
> -       struct sock *sk = gs->sock->sk;
> -       struct net *net = sock_net(sk);
> -       sa_family_t sa_family = geneve_get_sk_family(gs);
> -       __be16 port = inet_sk(sk)->inet_sport;
> -
> -       rcu_read_lock();
> -       for_each_netdev_rcu(net, dev) {
> -               if (dev->netdev_ops->ndo_del_geneve_port)
> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
> -                                                            port);
> -       }
> -
> -       rcu_read_unlock();
> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>  }
>
>  static void __geneve_sock_release(struct geneve_sock *gs)
> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>         .name = "geneve",
>  };
>
> -/* Calls the ndo_add_geneve_port of the caller in order to
> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>   * supply the listening GENEVE udp ports. Callers are expected
> - * to implement the ndo_add_geneve_port.
> + * to implement the ndo_add_udp_enc_port.
>   */
>  static void geneve_push_rx_ports(struct net_device *dev)
>  {
>         struct net *net = dev_net(dev);
>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>         struct geneve_sock *gs;
> -       sa_family_t sa_family;
> -       struct sock *sk;
> -       __be16 port;
> -
> -       if (!dev->netdev_ops->ndo_add_geneve_port)
> -               return;
>
>         rcu_read_lock();
> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
> -               sk = gs->sock->sk;
> -               sa_family = sk->sk_family;
> -               port = inet_sk(sk)->inet_sport;
> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
> -       }
> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
> +               udp_tunnel_push_rx_port(dev, gs->sock,
> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>         rcu_read_unlock();
>  }
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index f999db2f97b4..43f634282726 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>  /* Notify netdevs that UDP port started listening */
>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>  {
> -       struct net_device *dev;
> -       struct sock *sk = vs->sock->sk;
> -       struct net *net = sock_net(sk);
> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
> -       __be16 port = inet_sk(sk)->inet_sport;
> -
> -       rcu_read_lock();
> -       for_each_netdev_rcu(net, dev) {
> -               if (dev->netdev_ops->ndo_add_vxlan_port)
> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
> -                                                           port);
> -       }
> -       rcu_read_unlock();
> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>  }
>
>  /* Notify netdevs that UDP port is no more listening */
>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>  {
> -       struct net_device *dev;
> -       struct sock *sk = vs->sock->sk;
> -       struct net *net = sock_net(sk);
> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
> -       __be16 port = inet_sk(sk)->inet_sport;
> -
> -       rcu_read_lock();
> -       for_each_netdev_rcu(net, dev) {
> -               if (dev->netdev_ops->ndo_del_vxlan_port)
> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
> -                                                           port);
> -       }
> -       rcu_read_unlock();
> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>  }
>
>  /* Add new entry to forwarding table -- assumes lock held */
> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>         .name = "vxlan",
>  };
>
> -/* Calls the ndo_add_vxlan_port of the caller in order to
> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>   * supply the listening VXLAN udp ports. Callers are expected
> - * to implement the ndo_add_vxlan_port.
> + * to implement the ndo_add_udp_enc_port.
>   */
>  static void vxlan_push_rx_ports(struct net_device *dev)
>  {
>         struct vxlan_sock *vs;
>         struct net *net = dev_net(dev);
>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
> -       sa_family_t sa_family;
> -       __be16 port;
>         unsigned int i;
>
> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
> -               return;
> -
>         spin_lock(&vn->sock_lock);
>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
> -                       port = inet_sk(vs->sock->sk)->inet_sport;
> -                       sa_family = vxlan_get_sk_family(vs);
> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
> -                                                           port);
> -               }
> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
> +                       udp_tunnel_push_rx_port(dev, vs->sock,
> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>         }
>         spin_unlock(&vn->sock_lock);
>  }
> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
> index 9d14f707e534..704f931fd9ad 100644
> --- a/include/net/udp_tunnel.h
> +++ b/include/net/udp_tunnel.h
> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>                            struct udp_tunnel_sock_cfg *sock_cfg);
>
> +/* List of offloadable UDP tunnel types */
> +enum udp_enc_offloads {
> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
> +};
> +
We've already had a lot of discussion on this. The clear outcome from
netdev was that we need to support generic offloads and move away from
protocol specific offload. Generalizing the interface to allow vendors
to unnecessarily leak out protocol specific features undermines that
effort.

Tom

> +/* Notify network devices of offloadable types */
> +void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
> +                            unsigned int type);
> +void udp_tunnel_notify_add_rx_port(struct socket *sock, unsigned int type);
> +void udp_tunnel_notify_del_rx_port(struct socket *sock, unsigned int type);
> +
>  /* Transmit the skb using UDP encapsulation. */
>  void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
>                          __be32 src, __be32 dst, __u8 tos, __u8 ttl,
> diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
> index 47f12c73d959..d575b70644e3 100644
> --- a/net/ipv4/udp_tunnel.c
> +++ b/net/ipv4/udp_tunnel.c
> @@ -76,6 +76,83 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>  }
>  EXPORT_SYMBOL_GPL(setup_udp_tunnel_sock);
>
> +void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
> +                            unsigned int type)
> +{
> +       struct sock *sk = sock->sk;
> +       sa_family_t sa_family = sk->sk_family;
> +       __be16 port = inet_sk(sk)->inet_sport;
> +
> +       switch (type) {
> +       case UDP_ENC_OFFLOAD_TYPE_VXLAN:
> +               if (!dev->netdev_ops->ndo_add_vxlan_port)
> +                       break;
> +
> +               dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family, port);
> +               break;
> +       case UDP_ENC_OFFLOAD_TYPE_GENEVE:
> +               if (!dev->netdev_ops->ndo_add_geneve_port)
> +                       break;
> +
> +               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
> +               break;
> +       default:
> +               break;
> +       }
> +}
> +EXPORT_SYMBOL_GPL(udp_tunnel_push_rx_port);
> +
> +/* Notify netdevs that UDP port started listening */
> +void udp_tunnel_notify_add_rx_port(struct socket *sock, unsigned int type)
> +{
> +       struct net *net = sock_net(sock->sk);
> +       struct net_device *dev;
> +
> +       rcu_read_lock();
> +       for_each_netdev_rcu(net, dev)
> +               udp_tunnel_push_rx_port(dev, sock, type);
> +       rcu_read_unlock();
> +}
> +EXPORT_SYMBOL_GPL(udp_tunnel_notify_add_rx_port);
> +
> +static void udp_tunnel_pull_rx_port(struct net_device *dev,
> +                                   struct socket *sock, unsigned int type)
> +{
> +       struct sock *sk = sock->sk;
> +       sa_family_t sa_family = sk->sk_family;
> +       __be16 port = inet_sk(sk)->inet_sport;
> +
> +       switch (type) {
> +       case UDP_ENC_OFFLOAD_TYPE_VXLAN:
> +               if (!dev->netdev_ops->ndo_del_vxlan_port)
> +                       break;
> +
> +               dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family, port);
> +               break;
> +       case UDP_ENC_OFFLOAD_TYPE_GENEVE:
> +               if (!dev->netdev_ops->ndo_del_geneve_port)
> +                       break;
> +
> +               dev->netdev_ops->ndo_del_geneve_port(dev, sa_family, port);
> +               break;
> +       default:
> +               break;
> +       }
> +}
> +
> +/* Notify netdevs that UDP port is no more listening */
> +void udp_tunnel_notify_del_rx_port(struct socket *sock, unsigned int type)
> +{
> +       struct net_device *dev;
> +       struct net *net = sock_net(sock->sk);
> +
> +       rcu_read_lock();
> +       for_each_netdev_rcu(net, dev)
> +               udp_tunnel_pull_rx_port(dev, sock, type);
> +       rcu_read_unlock();
> +}
> +EXPORT_SYMBOL_GPL(udp_tunnel_notify_del_rx_port);
> +
>  void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
>                          __be32 src, __be32 dst, __u8 tos, __u8 ttl,
>                          __be16 df, __be16 src_port, __be16 dst_port,
>

Re: [Intel-wired-lan] [net-next PATCH 02/15] net: Merge VXLAN and GENEVE push notifiers into a single notifier

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]

Hi,

[auto build test ERROR on net/master]
[also build test ERROR on v4.7-rc3 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alexander-Duyck/Future-proof-tunnel-offload-handlers/20160614-020534
config: x86_64-randconfig-x015-06140233 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from include/net/vxlan.h:11:0,
                    from drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:53:
   include/net/udp_tunnel.h: In function 'udp_tunnel_handle_offloads':
>> include/net/udp_tunnel.h:130:9: error: implicit declaration of function 'iptunnel_handle_offloads' [-Werror=implicit-function-declaration]
     return iptunnel_handle_offloads(skb, type);
            ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/iptunnel_handle_offloads +130 include/net/udp_tunnel.h

c29a70d2 Pravin B Shelar 2015-08-26  124  				    int md_size);
c29a70d2 Pravin B Shelar 2015-08-26  125  
aed069df Alexander Duyck 2016-04-14  126  static inline int udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum)
6a93cc90 Andy Zhou       2014-09-16  127  {
6a93cc90 Andy Zhou       2014-09-16  128  	int type = udp_csum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
6a93cc90 Andy Zhou       2014-09-16  129  
6fa79666 Edward Cree     2016-02-11 @130  	return iptunnel_handle_offloads(skb, type);
6a93cc90 Andy Zhou       2014-09-16  131  }
6a93cc90 Andy Zhou       2014-09-16  132  
6a93cc90 Andy Zhou       2014-09-16  133  static inline void udp_tunnel_encap_enable(struct socket *sock)

:::::: The code at line 130 was first introduced by commit
:::::: 6fa79666e24d32be1b709f5269af41ed9e829e7e net: ip_tunnel: remove 'csum_help' argument to iptunnel_handle_offloads

:::::: TO: Edward Cree <ecree@solarflare.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 28051 bytes --]

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Alexander Duyck]

On Mon, Jun 13, 2016 at 12:55 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This patch merges the GENEVE and VXLAN code so that both functions pass
>> through a shared code path.  This way we can start the effort of using a
>> single function on the network device drivers to handle both of these
>> tunnel offload types.
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
>>  drivers/net/geneve.c     |   48 ++++-------------------------
>>  drivers/net/vxlan.c      |   46 ++++-----------------------
>>  include/net/udp_tunnel.h |   12 +++++++
>>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 103 insertions(+), 80 deletions(-)
>>
>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>> index cadefe4fdaa2..f5ce41532cf4 100644
>> --- a/drivers/net/geneve.c
>> +++ b/drivers/net/geneve.c
>> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>
>>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>  {
>> -       struct net_device *dev;
>> -       struct sock *sk = gs->sock->sk;
>> -       struct net *net = sock_net(sk);
>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>> -       __be16 port = inet_sk(sk)->inet_sport;
>> -
>> -       rcu_read_lock();
>> -       for_each_netdev_rcu(net, dev) {
>> -               if (dev->netdev_ops->ndo_add_geneve_port)
>> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>> -                                                            port);
>> -       }
>> -       rcu_read_unlock();
>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>  }
>>
>>  static int geneve_hlen(struct genevehdr *gh)
>> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>>
>>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>>  {
>> -       struct net_device *dev;
>> -       struct sock *sk = gs->sock->sk;
>> -       struct net *net = sock_net(sk);
>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>> -       __be16 port = inet_sk(sk)->inet_sport;
>> -
>> -       rcu_read_lock();
>> -       for_each_netdev_rcu(net, dev) {
>> -               if (dev->netdev_ops->ndo_del_geneve_port)
>> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
>> -                                                            port);
>> -       }
>> -
>> -       rcu_read_unlock();
>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>  }
>>
>>  static void __geneve_sock_release(struct geneve_sock *gs)
>> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>>         .name = "geneve",
>>  };
>>
>> -/* Calls the ndo_add_geneve_port of the caller in order to
>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>   * supply the listening GENEVE udp ports. Callers are expected
>> - * to implement the ndo_add_geneve_port.
>> + * to implement the ndo_add_udp_enc_port.
>>   */
>>  static void geneve_push_rx_ports(struct net_device *dev)
>>  {
>>         struct net *net = dev_net(dev);
>>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>>         struct geneve_sock *gs;
>> -       sa_family_t sa_family;
>> -       struct sock *sk;
>> -       __be16 port;
>> -
>> -       if (!dev->netdev_ops->ndo_add_geneve_port)
>> -               return;
>>
>>         rcu_read_lock();
>> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
>> -               sk = gs->sock->sk;
>> -               sa_family = sk->sk_family;
>> -               port = inet_sk(sk)->inet_sport;
>> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
>> -       }
>> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
>> +               udp_tunnel_push_rx_port(dev, gs->sock,
>> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>         rcu_read_unlock();
>>  }
>>
>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>> index f999db2f97b4..43f634282726 100644
>> --- a/drivers/net/vxlan.c
>> +++ b/drivers/net/vxlan.c
>> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>>  /* Notify netdevs that UDP port started listening */
>>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>>  {
>> -       struct net_device *dev;
>> -       struct sock *sk = vs->sock->sk;
>> -       struct net *net = sock_net(sk);
>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>> -       __be16 port = inet_sk(sk)->inet_sport;
>> -
>> -       rcu_read_lock();
>> -       for_each_netdev_rcu(net, dev) {
>> -               if (dev->netdev_ops->ndo_add_vxlan_port)
>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>> -                                                           port);
>> -       }
>> -       rcu_read_unlock();
>> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>  }
>>
>>  /* Notify netdevs that UDP port is no more listening */
>>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>>  {
>> -       struct net_device *dev;
>> -       struct sock *sk = vs->sock->sk;
>> -       struct net *net = sock_net(sk);
>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>> -       __be16 port = inet_sk(sk)->inet_sport;
>> -
>> -       rcu_read_lock();
>> -       for_each_netdev_rcu(net, dev) {
>> -               if (dev->netdev_ops->ndo_del_vxlan_port)
>> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>> -                                                           port);
>> -       }
>> -       rcu_read_unlock();
>> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>  }
>>
>>  /* Add new entry to forwarding table -- assumes lock held */
>> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>>         .name = "vxlan",
>>  };
>>
>> -/* Calls the ndo_add_vxlan_port of the caller in order to
>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>   * supply the listening VXLAN udp ports. Callers are expected
>> - * to implement the ndo_add_vxlan_port.
>> + * to implement the ndo_add_udp_enc_port.
>>   */
>>  static void vxlan_push_rx_ports(struct net_device *dev)
>>  {
>>         struct vxlan_sock *vs;
>>         struct net *net = dev_net(dev);
>>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>> -       sa_family_t sa_family;
>> -       __be16 port;
>>         unsigned int i;
>>
>> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
>> -               return;
>> -
>>         spin_lock(&vn->sock_lock);
>>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
>> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
>> -                       port = inet_sk(vs->sock->sk)->inet_sport;
>> -                       sa_family = vxlan_get_sk_family(vs);
>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>> -                                                           port);
>> -               }
>> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
>> +                       udp_tunnel_push_rx_port(dev, vs->sock,
>> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>         }
>>         spin_unlock(&vn->sock_lock);
>>  }
>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>> index 9d14f707e534..704f931fd9ad 100644
>> --- a/include/net/udp_tunnel.h
>> +++ b/include/net/udp_tunnel.h
>> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>>                            struct udp_tunnel_sock_cfg *sock_cfg);
>>
>> +/* List of offloadable UDP tunnel types */
>> +enum udp_enc_offloads {
>> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
>> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
>> +};
>> +
> We've already had a lot of discussion on this. The clear outcome from
> netdev was that we need to support generic offloads and move away from
> protocol specific offload. Generalizing the interface to allow vendors
> to unnecessarily leak out protocol specific features undermines that
> effort.

Then in turn we get dirty hacks like what we have right now where
VXLAN-GPE is attempting to reuse the VXLAN offload functions or
drivers that just hard-code GENEVE ports.

Going full obstructionist on this isn't going to work.  We need to be
able to support these type of offloads because the switch vendors are
going to force the NIC vendors to do so.  We will likely never be able
to convince Cisco to implement an outer transmit checksum on their
switches.  In order to make offloads work without the outer checksum
we will need to be able to parse the frames in order to be able to
validate the inner checksum values.

At this point what we need to focus on is being able to show that this
feature is essentially a crutch.  To that end I really think we need
to focus on enabling Tx checksum for all new UDP tunnels going forward
by default.  That way we get the full benefits of the offload before
we even have to start thinking about adding this kind of code.  It
isn't until we start having to deal with switches and other OSes that
cannot support the outer checksum on Tx that we need these kind of
workarounds in the hardware.

- Alex

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Tom Herbert]

On Mon, Jun 13, 2016 at 1:24 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, Jun 13, 2016 at 12:55 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>> This patch merges the GENEVE and VXLAN code so that both functions pass
>>> through a shared code path.  This way we can start the effort of using a
>>> single function on the network device drivers to handle both of these
>>> tunnel offload types.
>>>
>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>>> ---
>>>  drivers/net/geneve.c     |   48 ++++-------------------------
>>>  drivers/net/vxlan.c      |   46 ++++-----------------------
>>>  include/net/udp_tunnel.h |   12 +++++++
>>>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  4 files changed, 103 insertions(+), 80 deletions(-)
>>>
>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>> index cadefe4fdaa2..f5ce41532cf4 100644
>>> --- a/drivers/net/geneve.c
>>> +++ b/drivers/net/geneve.c
>>> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>>
>>>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>>  {
>>> -       struct net_device *dev;
>>> -       struct sock *sk = gs->sock->sk;
>>> -       struct net *net = sock_net(sk);
>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>> -
>>> -       rcu_read_lock();
>>> -       for_each_netdev_rcu(net, dev) {
>>> -               if (dev->netdev_ops->ndo_add_geneve_port)
>>> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>>> -                                                            port);
>>> -       }
>>> -       rcu_read_unlock();
>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>  }
>>>
>>>  static int geneve_hlen(struct genevehdr *gh)
>>> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>>>
>>>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>>>  {
>>> -       struct net_device *dev;
>>> -       struct sock *sk = gs->sock->sk;
>>> -       struct net *net = sock_net(sk);
>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>> -
>>> -       rcu_read_lock();
>>> -       for_each_netdev_rcu(net, dev) {
>>> -               if (dev->netdev_ops->ndo_del_geneve_port)
>>> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
>>> -                                                            port);
>>> -       }
>>> -
>>> -       rcu_read_unlock();
>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>  }
>>>
>>>  static void __geneve_sock_release(struct geneve_sock *gs)
>>> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>>>         .name = "geneve",
>>>  };
>>>
>>> -/* Calls the ndo_add_geneve_port of the caller in order to
>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>   * supply the listening GENEVE udp ports. Callers are expected
>>> - * to implement the ndo_add_geneve_port.
>>> + * to implement the ndo_add_udp_enc_port.
>>>   */
>>>  static void geneve_push_rx_ports(struct net_device *dev)
>>>  {
>>>         struct net *net = dev_net(dev);
>>>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>>>         struct geneve_sock *gs;
>>> -       sa_family_t sa_family;
>>> -       struct sock *sk;
>>> -       __be16 port;
>>> -
>>> -       if (!dev->netdev_ops->ndo_add_geneve_port)
>>> -               return;
>>>
>>>         rcu_read_lock();
>>> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
>>> -               sk = gs->sock->sk;
>>> -               sa_family = sk->sk_family;
>>> -               port = inet_sk(sk)->inet_sport;
>>> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
>>> -       }
>>> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
>>> +               udp_tunnel_push_rx_port(dev, gs->sock,
>>> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>         rcu_read_unlock();
>>>  }
>>>
>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>> index f999db2f97b4..43f634282726 100644
>>> --- a/drivers/net/vxlan.c
>>> +++ b/drivers/net/vxlan.c
>>> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>>>  /* Notify netdevs that UDP port started listening */
>>>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>>>  {
>>> -       struct net_device *dev;
>>> -       struct sock *sk = vs->sock->sk;
>>> -       struct net *net = sock_net(sk);
>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>> -
>>> -       rcu_read_lock();
>>> -       for_each_netdev_rcu(net, dev) {
>>> -               if (dev->netdev_ops->ndo_add_vxlan_port)
>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>> -                                                           port);
>>> -       }
>>> -       rcu_read_unlock();
>>> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>  }
>>>
>>>  /* Notify netdevs that UDP port is no more listening */
>>>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>>>  {
>>> -       struct net_device *dev;
>>> -       struct sock *sk = vs->sock->sk;
>>> -       struct net *net = sock_net(sk);
>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>> -
>>> -       rcu_read_lock();
>>> -       for_each_netdev_rcu(net, dev) {
>>> -               if (dev->netdev_ops->ndo_del_vxlan_port)
>>> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>>> -                                                           port);
>>> -       }
>>> -       rcu_read_unlock();
>>> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>  }
>>>
>>>  /* Add new entry to forwarding table -- assumes lock held */
>>> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>>>         .name = "vxlan",
>>>  };
>>>
>>> -/* Calls the ndo_add_vxlan_port of the caller in order to
>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>   * supply the listening VXLAN udp ports. Callers are expected
>>> - * to implement the ndo_add_vxlan_port.
>>> + * to implement the ndo_add_udp_enc_port.
>>>   */
>>>  static void vxlan_push_rx_ports(struct net_device *dev)
>>>  {
>>>         struct vxlan_sock *vs;
>>>         struct net *net = dev_net(dev);
>>>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>>> -       sa_family_t sa_family;
>>> -       __be16 port;
>>>         unsigned int i;
>>>
>>> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
>>> -               return;
>>> -
>>>         spin_lock(&vn->sock_lock);
>>>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
>>> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
>>> -                       port = inet_sk(vs->sock->sk)->inet_sport;
>>> -                       sa_family = vxlan_get_sk_family(vs);
>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>> -                                                           port);
>>> -               }
>>> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
>>> +                       udp_tunnel_push_rx_port(dev, vs->sock,
>>> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>         }
>>>         spin_unlock(&vn->sock_lock);
>>>  }
>>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>>> index 9d14f707e534..704f931fd9ad 100644
>>> --- a/include/net/udp_tunnel.h
>>> +++ b/include/net/udp_tunnel.h
>>> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>>>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>>>                            struct udp_tunnel_sock_cfg *sock_cfg);
>>>
>>> +/* List of offloadable UDP tunnel types */
>>> +enum udp_enc_offloads {
>>> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
>>> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
>>> +};
>>> +
>> We've already had a lot of discussion on this. The clear outcome from
>> netdev was that we need to support generic offloads and move away from
>> protocol specific offload. Generalizing the interface to allow vendors
>> to unnecessarily leak out protocol specific features undermines that
>> effort.
>
> Then in turn we get dirty hacks like what we have right now where
> VXLAN-GPE is attempting to reuse the VXLAN offload functions or
> drivers that just hard-code GENEVE ports.
>
> Going full obstructionist on this isn't going to work.  We need to be
> able to support these type of offloads because the switch vendors are
> going to force the NIC vendors to do so.  We will likely never be able
> to convince Cisco to implement an outer transmit checksum on their
> switches.  In order to make offloads work without the outer checksum
> we will need to be able to parse the frames in order to be able to
> validate the inner checksum values.
>
NIC vendors can support checksum-complete. This works with any form of
UDP encapsulation, and IP protocol (like extension headers), and other
form of tunneling we can dream up. That was the whole point of Dave's
keynote at netdev.

Tom

> At this point what we need to focus on is being able to show that this
> feature is essentially a crutch.  To that end I really think we need
> to focus on enabling Tx checksum for all new UDP tunnels going forward
> by default.  That way we get the full benefits of the offload before
> we even have to start thinking about adding this kind of code.  It
> isn't until we start having to deal with switches and other OSes that
> cannot support the outer checksum on Tx that we need these kind of
> workarounds in the hardware.
>
> - Alex

Re: [net-next PATCH 02/15] net: Merge VXLAN and GENEVE push notifiers into a single notifier

[Hannes Frederic Sowa]

On 13.06.2016 21:47, Alexander Duyck wrote:
> On Mon, Jun 13, 2016 at 10:57 AM, Hannes Frederic Sowa
> <hannes@redhat.com> wrote:
>> Hi Alex,
>>
>> very cool series!
>>
>> On 13.06.2016 19:48, Alexander Duyck wrote:
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index d101e4d904ba..e959b6348f91 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -1269,6 +1269,14 @@ struct net_device_ops {
>>>       void                    (*ndo_del_geneve_port)(struct  net_device *dev,
>>>                                                      sa_family_t sa_family,
>>>                                                      __be16 port);
>>> +     void                    (*ndo_add_udp_enc_port)(struct  net_device *dev,
>>> +                                                    sa_family_t sa_family,
>>> +                                                    __be16 port,
>>> +                                                    unsigned int type);
>>> +     void                    (*ndo_del_udp_enc_port)(struct  net_device *dev,
>>> +                                                    sa_family_t sa_family,
>>> +                                                    __be16 port,
>>> +                                                    unsigned int type);
>>>       void*                   (*ndo_dfwd_add_station)(struct net_device *pdev,
>>>                                                       struct net_device *dev);
>>>       void                    (*ndo_dfwd_del_station)(struct net_device *pdev,
>>
>> What do you think about adding a struct as argument to
>> ndo_*_udp_enc_port? As a result we can much easier add new fields in
>> case future NICs allow us to e.g. specify a bound ip address?
> 
> Actually that is probably a good idea.  Suggestions on the name are
> welcome.  Otherwise I will try to come up with something in a bit as I
> am currently going through and flushing out all the driver specific
> VXLAN and GENEVE build flags.

Hmmm... struct net_device_hw_offload, to be most generic? Maybe we can
even drop the udp_enc in the name and go completely generic:

int (*ndo_apply_offload)(..., struct hw_offload).

(enc reminded me too much at encryption)

Another idea, should we add error indications also for the future? We
can signal if a specific card was not able to enable offloading.
Different situations can be signaled: port list depleted, protocol
unsupported etc.

Might make sense for later postprocessing and signaling to user space.

Thanks,
Hannes

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Alexander Duyck]

On Mon, Jun 13, 2016 at 1:36 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, Jun 13, 2016 at 1:24 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, Jun 13, 2016 at 12:55 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>>> This patch merges the GENEVE and VXLAN code so that both functions pass
>>>> through a shared code path.  This way we can start the effort of using a
>>>> single function on the network device drivers to handle both of these
>>>> tunnel offload types.
>>>>
>>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>>>> ---
>>>>  drivers/net/geneve.c     |   48 ++++-------------------------
>>>>  drivers/net/vxlan.c      |   46 ++++-----------------------
>>>>  include/net/udp_tunnel.h |   12 +++++++
>>>>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>  4 files changed, 103 insertions(+), 80 deletions(-)
>>>>
>>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>>> index cadefe4fdaa2..f5ce41532cf4 100644
>>>> --- a/drivers/net/geneve.c
>>>> +++ b/drivers/net/geneve.c
>>>> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>>>
>>>>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>>>  {
>>>> -       struct net_device *dev;
>>>> -       struct sock *sk = gs->sock->sk;
>>>> -       struct net *net = sock_net(sk);
>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>> -
>>>> -       rcu_read_lock();
>>>> -       for_each_netdev_rcu(net, dev) {
>>>> -               if (dev->netdev_ops->ndo_add_geneve_port)
>>>> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>>>> -                                                            port);
>>>> -       }
>>>> -       rcu_read_unlock();
>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>  }
>>>>
>>>>  static int geneve_hlen(struct genevehdr *gh)
>>>> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>>>>
>>>>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>>>>  {
>>>> -       struct net_device *dev;
>>>> -       struct sock *sk = gs->sock->sk;
>>>> -       struct net *net = sock_net(sk);
>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>> -
>>>> -       rcu_read_lock();
>>>> -       for_each_netdev_rcu(net, dev) {
>>>> -               if (dev->netdev_ops->ndo_del_geneve_port)
>>>> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
>>>> -                                                            port);
>>>> -       }
>>>> -
>>>> -       rcu_read_unlock();
>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>  }
>>>>
>>>>  static void __geneve_sock_release(struct geneve_sock *gs)
>>>> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>>>>         .name = "geneve",
>>>>  };
>>>>
>>>> -/* Calls the ndo_add_geneve_port of the caller in order to
>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>   * supply the listening GENEVE udp ports. Callers are expected
>>>> - * to implement the ndo_add_geneve_port.
>>>> + * to implement the ndo_add_udp_enc_port.
>>>>   */
>>>>  static void geneve_push_rx_ports(struct net_device *dev)
>>>>  {
>>>>         struct net *net = dev_net(dev);
>>>>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>>>>         struct geneve_sock *gs;
>>>> -       sa_family_t sa_family;
>>>> -       struct sock *sk;
>>>> -       __be16 port;
>>>> -
>>>> -       if (!dev->netdev_ops->ndo_add_geneve_port)
>>>> -               return;
>>>>
>>>>         rcu_read_lock();
>>>> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
>>>> -               sk = gs->sock->sk;
>>>> -               sa_family = sk->sk_family;
>>>> -               port = inet_sk(sk)->inet_sport;
>>>> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
>>>> -       }
>>>> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
>>>> +               udp_tunnel_push_rx_port(dev, gs->sock,
>>>> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>         rcu_read_unlock();
>>>>  }
>>>>
>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>> index f999db2f97b4..43f634282726 100644
>>>> --- a/drivers/net/vxlan.c
>>>> +++ b/drivers/net/vxlan.c
>>>> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>>>>  /* Notify netdevs that UDP port started listening */
>>>>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>>>>  {
>>>> -       struct net_device *dev;
>>>> -       struct sock *sk = vs->sock->sk;
>>>> -       struct net *net = sock_net(sk);
>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>> -
>>>> -       rcu_read_lock();
>>>> -       for_each_netdev_rcu(net, dev) {
>>>> -               if (dev->netdev_ops->ndo_add_vxlan_port)
>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>> -                                                           port);
>>>> -       }
>>>> -       rcu_read_unlock();
>>>> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>  }
>>>>
>>>>  /* Notify netdevs that UDP port is no more listening */
>>>>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>>>>  {
>>>> -       struct net_device *dev;
>>>> -       struct sock *sk = vs->sock->sk;
>>>> -       struct net *net = sock_net(sk);
>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>> -
>>>> -       rcu_read_lock();
>>>> -       for_each_netdev_rcu(net, dev) {
>>>> -               if (dev->netdev_ops->ndo_del_vxlan_port)
>>>> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>>>> -                                                           port);
>>>> -       }
>>>> -       rcu_read_unlock();
>>>> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>  }
>>>>
>>>>  /* Add new entry to forwarding table -- assumes lock held */
>>>> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>>>>         .name = "vxlan",
>>>>  };
>>>>
>>>> -/* Calls the ndo_add_vxlan_port of the caller in order to
>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>   * supply the listening VXLAN udp ports. Callers are expected
>>>> - * to implement the ndo_add_vxlan_port.
>>>> + * to implement the ndo_add_udp_enc_port.
>>>>   */
>>>>  static void vxlan_push_rx_ports(struct net_device *dev)
>>>>  {
>>>>         struct vxlan_sock *vs;
>>>>         struct net *net = dev_net(dev);
>>>>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>>>> -       sa_family_t sa_family;
>>>> -       __be16 port;
>>>>         unsigned int i;
>>>>
>>>> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
>>>> -               return;
>>>> -
>>>>         spin_lock(&vn->sock_lock);
>>>>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
>>>> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
>>>> -                       port = inet_sk(vs->sock->sk)->inet_sport;
>>>> -                       sa_family = vxlan_get_sk_family(vs);
>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>> -                                                           port);
>>>> -               }
>>>> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
>>>> +                       udp_tunnel_push_rx_port(dev, vs->sock,
>>>> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>         }
>>>>         spin_unlock(&vn->sock_lock);
>>>>  }
>>>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>>>> index 9d14f707e534..704f931fd9ad 100644
>>>> --- a/include/net/udp_tunnel.h
>>>> +++ b/include/net/udp_tunnel.h
>>>> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>>>>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>>>>                            struct udp_tunnel_sock_cfg *sock_cfg);
>>>>
>>>> +/* List of offloadable UDP tunnel types */
>>>> +enum udp_enc_offloads {
>>>> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
>>>> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
>>>> +};
>>>> +
>>> We've already had a lot of discussion on this. The clear outcome from
>>> netdev was that we need to support generic offloads and move away from
>>> protocol specific offload. Generalizing the interface to allow vendors
>>> to unnecessarily leak out protocol specific features undermines that
>>> effort.
>>
>> Then in turn we get dirty hacks like what we have right now where
>> VXLAN-GPE is attempting to reuse the VXLAN offload functions or
>> drivers that just hard-code GENEVE ports.
>>
>> Going full obstructionist on this isn't going to work.  We need to be
>> able to support these type of offloads because the switch vendors are
>> going to force the NIC vendors to do so.  We will likely never be able
>> to convince Cisco to implement an outer transmit checksum on their
>> switches.  In order to make offloads work without the outer checksum
>> we will need to be able to parse the frames in order to be able to
>> validate the inner checksum values.
>>
> NIC vendors can support checksum-complete. This works with any form of
> UDP encapsulation, and IP protocol (like extension headers), and other
> form of tunneling we can dream up. That was the whole point of Dave's
> keynote at netdev.

Right.  That covers one tiny piece of the whole problem, but you are
holding out for hardware that may not be introduced for another 3 to 5
years.  The fact is trying to getting NIC vendors to support
checksum-complete is all well and good, but you seem to have forgotten
that NIC vendors are incredibly slow when it comes to implementing
anything.  In the meantime we will have the stuff that was already in
the pipeline coming out over the next several years.

How about the fact that we need to know that there is a tunnel there
if we want to do anything like try to parse the inner headers of a
given tunnel on Rx?  How do you propose to solve the RSS problem?
Enabling hashing on UDP source and destination port is okay-ish but
runs into the issue that the tunnel can be potentially fragmented.  In
addition any other fragmented UDP flows end up now being received with
potential out-of-order issues on the system.

There ends up being a number of reasons why you need to have this.
You can argue about checksum-complete all you want but in the end it
doesn't matter because simple things like flow identification and
steering will always be an issue regardless of how the checksum is
handled.

- Alex

Re: [net-next PATCH 02/15] net: Merge VXLAN and GENEVE push notifiers into a single notifier

[Alexander Duyck]

On Mon, Jun 13, 2016 at 2:08 PM, Hannes Frederic Sowa <hannes@redhat.com> wrote:
> On 13.06.2016 21:47, Alexander Duyck wrote:
>> On Mon, Jun 13, 2016 at 10:57 AM, Hannes Frederic Sowa
>> <hannes@redhat.com> wrote:
>>> Hi Alex,
>>>
>>> very cool series!
>>>
>>> On 13.06.2016 19:48, Alexander Duyck wrote:
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index d101e4d904ba..e959b6348f91 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -1269,6 +1269,14 @@ struct net_device_ops {
>>>>       void                    (*ndo_del_geneve_port)(struct  net_device *dev,
>>>>                                                      sa_family_t sa_family,
>>>>                                                      __be16 port);
>>>> +     void                    (*ndo_add_udp_enc_port)(struct  net_device *dev,
>>>> +                                                    sa_family_t sa_family,
>>>> +                                                    __be16 port,
>>>> +                                                    unsigned int type);
>>>> +     void                    (*ndo_del_udp_enc_port)(struct  net_device *dev,
>>>> +                                                    sa_family_t sa_family,
>>>> +                                                    __be16 port,
>>>> +                                                    unsigned int type);
>>>>       void*                   (*ndo_dfwd_add_station)(struct net_device *pdev,
>>>>                                                       struct net_device *dev);
>>>>       void                    (*ndo_dfwd_del_station)(struct net_device *pdev,
>>>
>>> What do you think about adding a struct as argument to
>>> ndo_*_udp_enc_port? As a result we can much easier add new fields in
>>> case future NICs allow us to e.g. specify a bound ip address?
>>
>> Actually that is probably a good idea.  Suggestions on the name are
>> welcome.  Otherwise I will try to come up with something in a bit as I
>> am currently going through and flushing out all the driver specific
>> VXLAN and GENEVE build flags.
>
> Hmmm... struct net_device_hw_offload, to be most generic? Maybe we can
> even drop the udp_enc in the name and go completely generic:
>
> int (*ndo_apply_offload)(..., struct hw_offload).

The only probably with generically using the offload keyword is it is
not very clear about what is going on.

For now I am just going with udp_enc_endpoint_info since that is
basically what we are passing.  Then I just use the pointer variable
ei for passing it back and forth between the functions.

> (enc reminded me too much at encryption)

I don't know.  In a way that isn't too far off since we are looking at
packet data buried inside of a UDP packet.  I thought it worked based
on the fact that we have hw_enc_features which is what is used to
indicate the hw features when a packet is encapsulated.  I could add a
few more letters and move things over to "encap" if you prefer.  It
just means adding 2 more letters.

> Another idea, should we add error indications also for the future? We
> can signal if a specific card was not able to enable offloading.
> Different situations can be signaled: port list depleted, protocol
> unsupported etc.
>
> Might make sense for later postprocessing and signaling to user space.

The problem is we are using a notifier type setup.  As such we cannot
really exit out if an error occurs on one of the ports.

- Alex

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Tom Herbert]

On Mon, Jun 13, 2016 at 2:51 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, Jun 13, 2016 at 1:36 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, Jun 13, 2016 at 1:24 PM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Mon, Jun 13, 2016 at 12:55 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>> On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>>>> This patch merges the GENEVE and VXLAN code so that both functions pass
>>>>> through a shared code path.  This way we can start the effort of using a
>>>>> single function on the network device drivers to handle both of these
>>>>> tunnel offload types.
>>>>>
>>>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>>>>> ---
>>>>>  drivers/net/geneve.c     |   48 ++++-------------------------
>>>>>  drivers/net/vxlan.c      |   46 ++++-----------------------
>>>>>  include/net/udp_tunnel.h |   12 +++++++
>>>>>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  4 files changed, 103 insertions(+), 80 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>>>> index cadefe4fdaa2..f5ce41532cf4 100644
>>>>> --- a/drivers/net/geneve.c
>>>>> +++ b/drivers/net/geneve.c
>>>>> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>>>>
>>>>>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>>>>  {
>>>>> -       struct net_device *dev;
>>>>> -       struct sock *sk = gs->sock->sk;
>>>>> -       struct net *net = sock_net(sk);
>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>> -
>>>>> -       rcu_read_lock();
>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>> -               if (dev->netdev_ops->ndo_add_geneve_port)
>>>>> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>>>>> -                                                            port);
>>>>> -       }
>>>>> -       rcu_read_unlock();
>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>  }
>>>>>
>>>>>  static int geneve_hlen(struct genevehdr *gh)
>>>>> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>>>>>
>>>>>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>>>>>  {
>>>>> -       struct net_device *dev;
>>>>> -       struct sock *sk = gs->sock->sk;
>>>>> -       struct net *net = sock_net(sk);
>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>> -
>>>>> -       rcu_read_lock();
>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>> -               if (dev->netdev_ops->ndo_del_geneve_port)
>>>>> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
>>>>> -                                                            port);
>>>>> -       }
>>>>> -
>>>>> -       rcu_read_unlock();
>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>  }
>>>>>
>>>>>  static void __geneve_sock_release(struct geneve_sock *gs)
>>>>> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>>>>>         .name = "geneve",
>>>>>  };
>>>>>
>>>>> -/* Calls the ndo_add_geneve_port of the caller in order to
>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>   * supply the listening GENEVE udp ports. Callers are expected
>>>>> - * to implement the ndo_add_geneve_port.
>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>   */
>>>>>  static void geneve_push_rx_ports(struct net_device *dev)
>>>>>  {
>>>>>         struct net *net = dev_net(dev);
>>>>>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>>>>>         struct geneve_sock *gs;
>>>>> -       sa_family_t sa_family;
>>>>> -       struct sock *sk;
>>>>> -       __be16 port;
>>>>> -
>>>>> -       if (!dev->netdev_ops->ndo_add_geneve_port)
>>>>> -               return;
>>>>>
>>>>>         rcu_read_lock();
>>>>> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
>>>>> -               sk = gs->sock->sk;
>>>>> -               sa_family = sk->sk_family;
>>>>> -               port = inet_sk(sk)->inet_sport;
>>>>> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
>>>>> -       }
>>>>> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
>>>>> +               udp_tunnel_push_rx_port(dev, gs->sock,
>>>>> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>         rcu_read_unlock();
>>>>>  }
>>>>>
>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>>> index f999db2f97b4..43f634282726 100644
>>>>> --- a/drivers/net/vxlan.c
>>>>> +++ b/drivers/net/vxlan.c
>>>>> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>>>>>  /* Notify netdevs that UDP port started listening */
>>>>>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>>>>>  {
>>>>> -       struct net_device *dev;
>>>>> -       struct sock *sk = vs->sock->sk;
>>>>> -       struct net *net = sock_net(sk);
>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>> -
>>>>> -       rcu_read_lock();
>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>> -               if (dev->netdev_ops->ndo_add_vxlan_port)
>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>> -                                                           port);
>>>>> -       }
>>>>> -       rcu_read_unlock();
>>>>> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>  }
>>>>>
>>>>>  /* Notify netdevs that UDP port is no more listening */
>>>>>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>>>>>  {
>>>>> -       struct net_device *dev;
>>>>> -       struct sock *sk = vs->sock->sk;
>>>>> -       struct net *net = sock_net(sk);
>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>> -
>>>>> -       rcu_read_lock();
>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>> -               if (dev->netdev_ops->ndo_del_vxlan_port)
>>>>> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>>>>> -                                                           port);
>>>>> -       }
>>>>> -       rcu_read_unlock();
>>>>> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>  }
>>>>>
>>>>>  /* Add new entry to forwarding table -- assumes lock held */
>>>>> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>>>>>         .name = "vxlan",
>>>>>  };
>>>>>
>>>>> -/* Calls the ndo_add_vxlan_port of the caller in order to
>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>   * supply the listening VXLAN udp ports. Callers are expected
>>>>> - * to implement the ndo_add_vxlan_port.
>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>   */
>>>>>  static void vxlan_push_rx_ports(struct net_device *dev)
>>>>>  {
>>>>>         struct vxlan_sock *vs;
>>>>>         struct net *net = dev_net(dev);
>>>>>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>>>>> -       sa_family_t sa_family;
>>>>> -       __be16 port;
>>>>>         unsigned int i;
>>>>>
>>>>> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
>>>>> -               return;
>>>>> -
>>>>>         spin_lock(&vn->sock_lock);
>>>>>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
>>>>> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
>>>>> -                       port = inet_sk(vs->sock->sk)->inet_sport;
>>>>> -                       sa_family = vxlan_get_sk_family(vs);
>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>> -                                                           port);
>>>>> -               }
>>>>> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
>>>>> +                       udp_tunnel_push_rx_port(dev, vs->sock,
>>>>> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>         }
>>>>>         spin_unlock(&vn->sock_lock);
>>>>>  }
>>>>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>>>>> index 9d14f707e534..704f931fd9ad 100644
>>>>> --- a/include/net/udp_tunnel.h
>>>>> +++ b/include/net/udp_tunnel.h
>>>>> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>>>>>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>>>>>                            struct udp_tunnel_sock_cfg *sock_cfg);
>>>>>
>>>>> +/* List of offloadable UDP tunnel types */
>>>>> +enum udp_enc_offloads {
>>>>> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
>>>>> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
>>>>> +};
>>>>> +
>>>> We've already had a lot of discussion on this. The clear outcome from
>>>> netdev was that we need to support generic offloads and move away from
>>>> protocol specific offload. Generalizing the interface to allow vendors
>>>> to unnecessarily leak out protocol specific features undermines that
>>>> effort.
>>>
>>> Then in turn we get dirty hacks like what we have right now where
>>> VXLAN-GPE is attempting to reuse the VXLAN offload functions or
>>> drivers that just hard-code GENEVE ports.
>>>
>>> Going full obstructionist on this isn't going to work.  We need to be
>>> able to support these type of offloads because the switch vendors are
>>> going to force the NIC vendors to do so.  We will likely never be able
>>> to convince Cisco to implement an outer transmit checksum on their
>>> switches.  In order to make offloads work without the outer checksum
>>> we will need to be able to parse the frames in order to be able to
>>> validate the inner checksum values.
>>>
>> NIC vendors can support checksum-complete. This works with any form of
>> UDP encapsulation, and IP protocol (like extension headers), and other
>> form of tunneling we can dream up. That was the whole point of Dave's
>> keynote at netdev.
>
> Right.  That covers one tiny piece of the whole problem, but you are
> holding out for hardware that may not be introduced for another 3 to 5
> years.  The fact is trying to getting NIC vendors to support
> checksum-complete is all well and good, but you seem to have forgotten
> that NIC vendors are incredibly slow when it comes to implementing
> anything.  In the meantime we will have the stuff that was already in
> the pipeline coming out over the next several years.
>
> How about the fact that we need to know that there is a tunnel there
> if we want to do anything like try to parse the inner headers of a
> given tunnel on Rx?  How do you propose to solve the RSS problem?

Solved by doing RSS and ECMP hash over 3-tuple of IP addresses and
IPv6 flow label (not ports). Non-zero flow labels will soon be widely
used over the Internet. IOS already is already setting them, Android
should pick up support in the next rebase, and the MS guys have
assured me that they will add support to next version of Windows. Like
a generic checksum offload, flow label works with an IP protocol,
extension header, fragmentation, UDP encapsulation, etc. With this
there is no reason for devices to parse L4 headers just to forward a
packet. HW vendors (both switches and NICs) are strongly encouraged to
support them.

> Enabling hashing on UDP source and destination port is okay-ish but
> runs into the issue that the tunnel can be potentially fragmented.  In
> addition any other fragmented UDP flows end up now being received with
> potential out-of-order issues on the system.
>
> There ends up being a number of reasons why you need to have this.
> You can argue about checksum-complete all you want but in the end it
> doesn't matter because simple things like flow identification and
> steering will always be an issue regardless of how the checksum is
> handled.
>
> - Alex

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Alexander Duyck]

On Mon, Jun 13, 2016 at 3:17 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, Jun 13, 2016 at 2:51 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, Jun 13, 2016 at 1:36 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> On Mon, Jun 13, 2016 at 1:24 PM, Alexander Duyck
>>> <alexander.duyck@gmail.com> wrote:
>>>> On Mon, Jun 13, 2016 at 12:55 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>> On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>>>>> This patch merges the GENEVE and VXLAN code so that both functions pass
>>>>>> through a shared code path.  This way we can start the effort of using a
>>>>>> single function on the network device drivers to handle both of these
>>>>>> tunnel offload types.
>>>>>>
>>>>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>>>>>> ---
>>>>>>  drivers/net/geneve.c     |   48 ++++-------------------------
>>>>>>  drivers/net/vxlan.c      |   46 ++++-----------------------
>>>>>>  include/net/udp_tunnel.h |   12 +++++++
>>>>>>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  4 files changed, 103 insertions(+), 80 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>>>>> index cadefe4fdaa2..f5ce41532cf4 100644
>>>>>> --- a/drivers/net/geneve.c
>>>>>> +++ b/drivers/net/geneve.c
>>>>>> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>>>>>
>>>>>>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>>>>>  {
>>>>>> -       struct net_device *dev;
>>>>>> -       struct sock *sk = gs->sock->sk;
>>>>>> -       struct net *net = sock_net(sk);
>>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>> -
>>>>>> -       rcu_read_lock();
>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>> -               if (dev->netdev_ops->ndo_add_geneve_port)
>>>>>> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>>>>>> -                                                            port);
>>>>>> -       }
>>>>>> -       rcu_read_unlock();
>>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>  }
>>>>>>
>>>>>>  static int geneve_hlen(struct genevehdr *gh)
>>>>>> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>>>>>>
>>>>>>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>>>>>>  {
>>>>>> -       struct net_device *dev;
>>>>>> -       struct sock *sk = gs->sock->sk;
>>>>>> -       struct net *net = sock_net(sk);
>>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>> -
>>>>>> -       rcu_read_lock();
>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>> -               if (dev->netdev_ops->ndo_del_geneve_port)
>>>>>> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
>>>>>> -                                                            port);
>>>>>> -       }
>>>>>> -
>>>>>> -       rcu_read_unlock();
>>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>  }
>>>>>>
>>>>>>  static void __geneve_sock_release(struct geneve_sock *gs)
>>>>>> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>>>>>>         .name = "geneve",
>>>>>>  };
>>>>>>
>>>>>> -/* Calls the ndo_add_geneve_port of the caller in order to
>>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>>   * supply the listening GENEVE udp ports. Callers are expected
>>>>>> - * to implement the ndo_add_geneve_port.
>>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>>   */
>>>>>>  static void geneve_push_rx_ports(struct net_device *dev)
>>>>>>  {
>>>>>>         struct net *net = dev_net(dev);
>>>>>>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>>>>>>         struct geneve_sock *gs;
>>>>>> -       sa_family_t sa_family;
>>>>>> -       struct sock *sk;
>>>>>> -       __be16 port;
>>>>>> -
>>>>>> -       if (!dev->netdev_ops->ndo_add_geneve_port)
>>>>>> -               return;
>>>>>>
>>>>>>         rcu_read_lock();
>>>>>> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
>>>>>> -               sk = gs->sock->sk;
>>>>>> -               sa_family = sk->sk_family;
>>>>>> -               port = inet_sk(sk)->inet_sport;
>>>>>> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
>>>>>> -       }
>>>>>> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
>>>>>> +               udp_tunnel_push_rx_port(dev, gs->sock,
>>>>>> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>         rcu_read_unlock();
>>>>>>  }
>>>>>>
>>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>>>> index f999db2f97b4..43f634282726 100644
>>>>>> --- a/drivers/net/vxlan.c
>>>>>> +++ b/drivers/net/vxlan.c
>>>>>> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>>>>>>  /* Notify netdevs that UDP port started listening */
>>>>>>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>>>>>>  {
>>>>>> -       struct net_device *dev;
>>>>>> -       struct sock *sk = vs->sock->sk;
>>>>>> -       struct net *net = sock_net(sk);
>>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>> -
>>>>>> -       rcu_read_lock();
>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>> -               if (dev->netdev_ops->ndo_add_vxlan_port)
>>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>>> -                                                           port);
>>>>>> -       }
>>>>>> -       rcu_read_unlock();
>>>>>> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>  }
>>>>>>
>>>>>>  /* Notify netdevs that UDP port is no more listening */
>>>>>>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>>>>>>  {
>>>>>> -       struct net_device *dev;
>>>>>> -       struct sock *sk = vs->sock->sk;
>>>>>> -       struct net *net = sock_net(sk);
>>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>> -
>>>>>> -       rcu_read_lock();
>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>> -               if (dev->netdev_ops->ndo_del_vxlan_port)
>>>>>> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>>>>>> -                                                           port);
>>>>>> -       }
>>>>>> -       rcu_read_unlock();
>>>>>> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>  }
>>>>>>
>>>>>>  /* Add new entry to forwarding table -- assumes lock held */
>>>>>> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>>>>>>         .name = "vxlan",
>>>>>>  };
>>>>>>
>>>>>> -/* Calls the ndo_add_vxlan_port of the caller in order to
>>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>>   * supply the listening VXLAN udp ports. Callers are expected
>>>>>> - * to implement the ndo_add_vxlan_port.
>>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>>   */
>>>>>>  static void vxlan_push_rx_ports(struct net_device *dev)
>>>>>>  {
>>>>>>         struct vxlan_sock *vs;
>>>>>>         struct net *net = dev_net(dev);
>>>>>>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>>>>>> -       sa_family_t sa_family;
>>>>>> -       __be16 port;
>>>>>>         unsigned int i;
>>>>>>
>>>>>> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
>>>>>> -               return;
>>>>>> -
>>>>>>         spin_lock(&vn->sock_lock);
>>>>>>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
>>>>>> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
>>>>>> -                       port = inet_sk(vs->sock->sk)->inet_sport;
>>>>>> -                       sa_family = vxlan_get_sk_family(vs);
>>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>>> -                                                           port);
>>>>>> -               }
>>>>>> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
>>>>>> +                       udp_tunnel_push_rx_port(dev, vs->sock,
>>>>>> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>         }
>>>>>>         spin_unlock(&vn->sock_lock);
>>>>>>  }
>>>>>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>>>>>> index 9d14f707e534..704f931fd9ad 100644
>>>>>> --- a/include/net/udp_tunnel.h
>>>>>> +++ b/include/net/udp_tunnel.h
>>>>>> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>>>>>>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>>>>>>                            struct udp_tunnel_sock_cfg *sock_cfg);
>>>>>>
>>>>>> +/* List of offloadable UDP tunnel types */
>>>>>> +enum udp_enc_offloads {
>>>>>> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
>>>>>> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
>>>>>> +};
>>>>>> +
>>>>> We've already had a lot of discussion on this. The clear outcome from
>>>>> netdev was that we need to support generic offloads and move away from
>>>>> protocol specific offload. Generalizing the interface to allow vendors
>>>>> to unnecessarily leak out protocol specific features undermines that
>>>>> effort.
>>>>
>>>> Then in turn we get dirty hacks like what we have right now where
>>>> VXLAN-GPE is attempting to reuse the VXLAN offload functions or
>>>> drivers that just hard-code GENEVE ports.
>>>>
>>>> Going full obstructionist on this isn't going to work.  We need to be
>>>> able to support these type of offloads because the switch vendors are
>>>> going to force the NIC vendors to do so.  We will likely never be able
>>>> to convince Cisco to implement an outer transmit checksum on their
>>>> switches.  In order to make offloads work without the outer checksum
>>>> we will need to be able to parse the frames in order to be able to
>>>> validate the inner checksum values.
>>>>
>>> NIC vendors can support checksum-complete. This works with any form of
>>> UDP encapsulation, and IP protocol (like extension headers), and other
>>> form of tunneling we can dream up. That was the whole point of Dave's
>>> keynote at netdev.
>>
>> Right.  That covers one tiny piece of the whole problem, but you are
>> holding out for hardware that may not be introduced for another 3 to 5
>> years.  The fact is trying to getting NIC vendors to support
>> checksum-complete is all well and good, but you seem to have forgotten
>> that NIC vendors are incredibly slow when it comes to implementing
>> anything.  In the meantime we will have the stuff that was already in
>> the pipeline coming out over the next several years.
>>
>> How about the fact that we need to know that there is a tunnel there
>> if we want to do anything like try to parse the inner headers of a
>> given tunnel on Rx?  How do you propose to solve the RSS problem?
>
> Solved by doing RSS and ECMP hash over 3-tuple of IP addresses and
> IPv6 flow label (not ports). Non-zero flow labels will soon be widely
> used over the Internet. IOS already is already setting them, Android
> should pick up support in the next rebase, and the MS guys have
> assured me that they will add support to next version of Windows. Like
> a generic checksum offload, flow label works with an IP protocol,
> extension header, fragmentation, UDP encapsulation, etc. With this
> there is no reason for devices to parse L4 headers just to forward a
> packet. HW vendors (both switches and NICs) are strongly encouraged to
> support them.

Right so with all this encouragement, where are we at on getting the
switches out there to support this?  The switches are what is pushing
most of the tunnel workloads and such, and it is the switches that are
going to be the painful interfaces for us to try to offload data from.

Also have you looked into the effect on regular traffic for trying to
deal with an extra IPv4 options header?  The Intel NICs should be able
to deal with it, at least for Tx, but my concern would be the NICs
that perform parsing based offloads and such.

It seems like we might be arguing past each other as I am focused on
what we can do right now, and it seems like you are arguing the point
that we need features that probably won't be enabled in the ecosystem
for 5 to 10 years depending on the hardware release cycles.  Hopefully
we can agree to disagree and can address this again when new features
start to become available.  At that time we could probably look at
deprecating the driver API for the UDP ports assuming we have other
means of resolving this available by then.

- Alex

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Tom Herbert]

On Mon, Jun 13, 2016 at 4:12 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, Jun 13, 2016 at 3:17 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, Jun 13, 2016 at 2:51 PM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Mon, Jun 13, 2016 at 1:36 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>> On Mon, Jun 13, 2016 at 1:24 PM, Alexander Duyck
>>>> <alexander.duyck@gmail.com> wrote:
>>>>> On Mon, Jun 13, 2016 at 12:55 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>> On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>>>>>> This patch merges the GENEVE and VXLAN code so that both functions pass
>>>>>>> through a shared code path.  This way we can start the effort of using a
>>>>>>> single function on the network device drivers to handle both of these
>>>>>>> tunnel offload types.
>>>>>>>
>>>>>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>>>>>>> ---
>>>>>>>  drivers/net/geneve.c     |   48 ++++-------------------------
>>>>>>>  drivers/net/vxlan.c      |   46 ++++-----------------------
>>>>>>>  include/net/udp_tunnel.h |   12 +++++++
>>>>>>>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>  4 files changed, 103 insertions(+), 80 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>>>>>> index cadefe4fdaa2..f5ce41532cf4 100644
>>>>>>> --- a/drivers/net/geneve.c
>>>>>>> +++ b/drivers/net/geneve.c
>>>>>>> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>>>>>>
>>>>>>>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>>>>>>  {
>>>>>>> -       struct net_device *dev;
>>>>>>> -       struct sock *sk = gs->sock->sk;
>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>> -
>>>>>>> -       rcu_read_lock();
>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>> -               if (dev->netdev_ops->ndo_add_geneve_port)
>>>>>>> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>>>>>>> -                                                            port);
>>>>>>> -       }
>>>>>>> -       rcu_read_unlock();
>>>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>>  }
>>>>>>>
>>>>>>>  static int geneve_hlen(struct genevehdr *gh)
>>>>>>> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>>>>>>>
>>>>>>>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>>>>>>>  {
>>>>>>> -       struct net_device *dev;
>>>>>>> -       struct sock *sk = gs->sock->sk;
>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>> -
>>>>>>> -       rcu_read_lock();
>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>> -               if (dev->netdev_ops->ndo_del_geneve_port)
>>>>>>> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
>>>>>>> -                                                            port);
>>>>>>> -       }
>>>>>>> -
>>>>>>> -       rcu_read_unlock();
>>>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>>  }
>>>>>>>
>>>>>>>  static void __geneve_sock_release(struct geneve_sock *gs)
>>>>>>> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>>>>>>>         .name = "geneve",
>>>>>>>  };
>>>>>>>
>>>>>>> -/* Calls the ndo_add_geneve_port of the caller in order to
>>>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>>>   * supply the listening GENEVE udp ports. Callers are expected
>>>>>>> - * to implement the ndo_add_geneve_port.
>>>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>>>   */
>>>>>>>  static void geneve_push_rx_ports(struct net_device *dev)
>>>>>>>  {
>>>>>>>         struct net *net = dev_net(dev);
>>>>>>>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>>>>>>>         struct geneve_sock *gs;
>>>>>>> -       sa_family_t sa_family;
>>>>>>> -       struct sock *sk;
>>>>>>> -       __be16 port;
>>>>>>> -
>>>>>>> -       if (!dev->netdev_ops->ndo_add_geneve_port)
>>>>>>> -               return;
>>>>>>>
>>>>>>>         rcu_read_lock();
>>>>>>> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
>>>>>>> -               sk = gs->sock->sk;
>>>>>>> -               sa_family = sk->sk_family;
>>>>>>> -               port = inet_sk(sk)->inet_sport;
>>>>>>> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
>>>>>>> -       }
>>>>>>> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
>>>>>>> +               udp_tunnel_push_rx_port(dev, gs->sock,
>>>>>>> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>>         rcu_read_unlock();
>>>>>>>  }
>>>>>>>
>>>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>>>>> index f999db2f97b4..43f634282726 100644
>>>>>>> --- a/drivers/net/vxlan.c
>>>>>>> +++ b/drivers/net/vxlan.c
>>>>>>> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>>>>>>>  /* Notify netdevs that UDP port started listening */
>>>>>>>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>>>>>>>  {
>>>>>>> -       struct net_device *dev;
>>>>>>> -       struct sock *sk = vs->sock->sk;
>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>> -
>>>>>>> -       rcu_read_lock();
>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>> -               if (dev->netdev_ops->ndo_add_vxlan_port)
>>>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>>>> -                                                           port);
>>>>>>> -       }
>>>>>>> -       rcu_read_unlock();
>>>>>>> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>>  }
>>>>>>>
>>>>>>>  /* Notify netdevs that UDP port is no more listening */
>>>>>>>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>>>>>>>  {
>>>>>>> -       struct net_device *dev;
>>>>>>> -       struct sock *sk = vs->sock->sk;
>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>> -
>>>>>>> -       rcu_read_lock();
>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>> -               if (dev->netdev_ops->ndo_del_vxlan_port)
>>>>>>> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>>>>>>> -                                                           port);
>>>>>>> -       }
>>>>>>> -       rcu_read_unlock();
>>>>>>> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>>  }
>>>>>>>
>>>>>>>  /* Add new entry to forwarding table -- assumes lock held */
>>>>>>> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>>>>>>>         .name = "vxlan",
>>>>>>>  };
>>>>>>>
>>>>>>> -/* Calls the ndo_add_vxlan_port of the caller in order to
>>>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>>>   * supply the listening VXLAN udp ports. Callers are expected
>>>>>>> - * to implement the ndo_add_vxlan_port.
>>>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>>>   */
>>>>>>>  static void vxlan_push_rx_ports(struct net_device *dev)
>>>>>>>  {
>>>>>>>         struct vxlan_sock *vs;
>>>>>>>         struct net *net = dev_net(dev);
>>>>>>>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>>>>>>> -       sa_family_t sa_family;
>>>>>>> -       __be16 port;
>>>>>>>         unsigned int i;
>>>>>>>
>>>>>>> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
>>>>>>> -               return;
>>>>>>> -
>>>>>>>         spin_lock(&vn->sock_lock);
>>>>>>>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
>>>>>>> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
>>>>>>> -                       port = inet_sk(vs->sock->sk)->inet_sport;
>>>>>>> -                       sa_family = vxlan_get_sk_family(vs);
>>>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>>>> -                                                           port);
>>>>>>> -               }
>>>>>>> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
>>>>>>> +                       udp_tunnel_push_rx_port(dev, vs->sock,
>>>>>>> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>>         }
>>>>>>>         spin_unlock(&vn->sock_lock);
>>>>>>>  }
>>>>>>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>>>>>>> index 9d14f707e534..704f931fd9ad 100644
>>>>>>> --- a/include/net/udp_tunnel.h
>>>>>>> +++ b/include/net/udp_tunnel.h
>>>>>>> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>>>>>>>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>>>>>>>                            struct udp_tunnel_sock_cfg *sock_cfg);
>>>>>>>
>>>>>>> +/* List of offloadable UDP tunnel types */
>>>>>>> +enum udp_enc_offloads {
>>>>>>> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
>>>>>>> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
>>>>>>> +};
>>>>>>> +
>>>>>> We've already had a lot of discussion on this. The clear outcome from
>>>>>> netdev was that we need to support generic offloads and move away from
>>>>>> protocol specific offload. Generalizing the interface to allow vendors
>>>>>> to unnecessarily leak out protocol specific features undermines that
>>>>>> effort.
>>>>>
>>>>> Then in turn we get dirty hacks like what we have right now where
>>>>> VXLAN-GPE is attempting to reuse the VXLAN offload functions or
>>>>> drivers that just hard-code GENEVE ports.
>>>>>
>>>>> Going full obstructionist on this isn't going to work.  We need to be
>>>>> able to support these type of offloads because the switch vendors are
>>>>> going to force the NIC vendors to do so.  We will likely never be able
>>>>> to convince Cisco to implement an outer transmit checksum on their
>>>>> switches.  In order to make offloads work without the outer checksum
>>>>> we will need to be able to parse the frames in order to be able to
>>>>> validate the inner checksum values.
>>>>>
>>>> NIC vendors can support checksum-complete. This works with any form of
>>>> UDP encapsulation, and IP protocol (like extension headers), and other
>>>> form of tunneling we can dream up. That was the whole point of Dave's
>>>> keynote at netdev.
>>>
>>> Right.  That covers one tiny piece of the whole problem, but you are
>>> holding out for hardware that may not be introduced for another 3 to 5
>>> years.  The fact is trying to getting NIC vendors to support
>>> checksum-complete is all well and good, but you seem to have forgotten
>>> that NIC vendors are incredibly slow when it comes to implementing
>>> anything.  In the meantime we will have the stuff that was already in
>>> the pipeline coming out over the next several years.
>>>
>>> How about the fact that we need to know that there is a tunnel there
>>> if we want to do anything like try to parse the inner headers of a
>>> given tunnel on Rx?  How do you propose to solve the RSS problem?
>>
>> Solved by doing RSS and ECMP hash over 3-tuple of IP addresses and
>> IPv6 flow label (not ports). Non-zero flow labels will soon be widely
>> used over the Internet. IOS already is already setting them, Android
>> should pick up support in the next rebase, and the MS guys have
>> assured me that they will add support to next version of Windows. Like
>> a generic checksum offload, flow label works with an IP protocol,
>> extension header, fragmentation, UDP encapsulation, etc. With this
>> there is no reason for devices to parse L4 headers just to forward a
>> packet. HW vendors (both switches and NICs) are strongly encouraged to
>> support them.
>
> Right so with all this encouragement, where are we at on getting the
> switches out there to support this?  The switches are what is pushing
> most of the tunnel workloads and such, and it is the switches that are
> going to be the painful interfaces for us to try to offload data from.
>
> Also have you looked into the effect on regular traffic for trying to
> deal with an extra IPv4 options header?  The Intel NICs should be able
> to deal with it, at least for Tx, but my concern would be the NICs
> that perform parsing based offloads and such.
>
> It seems like we might be arguing past each other as I am focused on
> what we can do right now, and it seems like you are arguing the point
> that we need features that probably won't be enabled in the ecosystem
> for 5 to 10 years depending on the hardware release cycles.  Hopefully
> we can agree to disagree and can address this again when new features
> start to become available.  At that time we could probably look at
> deprecating the driver API for the UDP ports assuming we have other
> means of resolving this available by then.
>
Alex,

I looked though the previous thread on protocol specific vs. generic
offloads in http://www.spinics.net/lists/netdev/msg354311.html and the
outcome from that. To quote davem:

"Doing anything other than providing 2's complement checksums in the
RX descriptor doesn't work.  We know this.

So we will not add to our core architecture and frameworks anything
that directly facilitates designs which we know are suboptimal.  And
protocol specific support for tunnel offloading is suboptimal and not
the way forward."

Unless Dave has changed his mind on this or you have new arguments or
new data, I really don't see point in participating in further
discussion on patches like this. Sorry.

Tom



> - Alex

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Alexander Duyck]

On Mon, Jun 13, 2016 at 5:28 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, Jun 13, 2016 at 4:12 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, Jun 13, 2016 at 3:17 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> On Mon, Jun 13, 2016 at 2:51 PM, Alexander Duyck
>>> <alexander.duyck@gmail.com> wrote:
>>>> On Mon, Jun 13, 2016 at 1:36 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>> On Mon, Jun 13, 2016 at 1:24 PM, Alexander Duyck
>>>>> <alexander.duyck@gmail.com> wrote:
>>>>>> On Mon, Jun 13, 2016 at 12:55 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>>> On Mon, Jun 13, 2016 at 10:47 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>>>>>>> This patch merges the GENEVE and VXLAN code so that both functions pass
>>>>>>>> through a shared code path.  This way we can start the effort of using a
>>>>>>>> single function on the network device drivers to handle both of these
>>>>>>>> tunnel offload types.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>>>>>>>> ---
>>>>>>>>  drivers/net/geneve.c     |   48 ++++-------------------------
>>>>>>>>  drivers/net/vxlan.c      |   46 ++++-----------------------
>>>>>>>>  include/net/udp_tunnel.h |   12 +++++++
>>>>>>>>  net/ipv4/udp_tunnel.c    |   77 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>  4 files changed, 103 insertions(+), 80 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>>>>>>> index cadefe4fdaa2..f5ce41532cf4 100644
>>>>>>>> --- a/drivers/net/geneve.c
>>>>>>>> +++ b/drivers/net/geneve.c
>>>>>>>> @@ -399,19 +399,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>>>>>>>
>>>>>>>>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>>>>>>>  {
>>>>>>>> -       struct net_device *dev;
>>>>>>>> -       struct sock *sk = gs->sock->sk;
>>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>>> -
>>>>>>>> -       rcu_read_lock();
>>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>>> -               if (dev->netdev_ops->ndo_add_geneve_port)
>>>>>>>> -                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>>>>>>>> -                                                            port);
>>>>>>>> -       }
>>>>>>>> -       rcu_read_unlock();
>>>>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  static int geneve_hlen(struct genevehdr *gh)
>>>>>>>> @@ -550,20 +538,7 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>>>>>>>>
>>>>>>>>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>>>>>>>>  {
>>>>>>>> -       struct net_device *dev;
>>>>>>>> -       struct sock *sk = gs->sock->sk;
>>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>>> -       sa_family_t sa_family = geneve_get_sk_family(gs);
>>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>>> -
>>>>>>>> -       rcu_read_lock();
>>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>>> -               if (dev->netdev_ops->ndo_del_geneve_port)
>>>>>>>> -                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
>>>>>>>> -                                                            port);
>>>>>>>> -       }
>>>>>>>> -
>>>>>>>> -       rcu_read_unlock();
>>>>>>>> +       udp_tunnel_notify_add_rx_port(gs->sock, UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  static void __geneve_sock_release(struct geneve_sock *gs)
>>>>>>>> @@ -1165,29 +1140,20 @@ static struct device_type geneve_type = {
>>>>>>>>         .name = "geneve",
>>>>>>>>  };
>>>>>>>>
>>>>>>>> -/* Calls the ndo_add_geneve_port of the caller in order to
>>>>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>>>>   * supply the listening GENEVE udp ports. Callers are expected
>>>>>>>> - * to implement the ndo_add_geneve_port.
>>>>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>>>>   */
>>>>>>>>  static void geneve_push_rx_ports(struct net_device *dev)
>>>>>>>>  {
>>>>>>>>         struct net *net = dev_net(dev);
>>>>>>>>         struct geneve_net *gn = net_generic(net, geneve_net_id);
>>>>>>>>         struct geneve_sock *gs;
>>>>>>>> -       sa_family_t sa_family;
>>>>>>>> -       struct sock *sk;
>>>>>>>> -       __be16 port;
>>>>>>>> -
>>>>>>>> -       if (!dev->netdev_ops->ndo_add_geneve_port)
>>>>>>>> -               return;
>>>>>>>>
>>>>>>>>         rcu_read_lock();
>>>>>>>> -       list_for_each_entry_rcu(gs, &gn->sock_list, list) {
>>>>>>>> -               sk = gs->sock->sk;
>>>>>>>> -               sa_family = sk->sk_family;
>>>>>>>> -               port = inet_sk(sk)->inet_sport;
>>>>>>>> -               dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
>>>>>>>> -       }
>>>>>>>> +       list_for_each_entry_rcu(gs, &gn->sock_list, list)
>>>>>>>> +               udp_tunnel_push_rx_port(dev, gs->sock,
>>>>>>>> +                                       UDP_ENC_OFFLOAD_TYPE_GENEVE);
>>>>>>>>         rcu_read_unlock();
>>>>>>>>  }
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>>>>>> index f999db2f97b4..43f634282726 100644
>>>>>>>> --- a/drivers/net/vxlan.c
>>>>>>>> +++ b/drivers/net/vxlan.c
>>>>>>>> @@ -622,37 +622,13 @@ static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
>>>>>>>>  /* Notify netdevs that UDP port started listening */
>>>>>>>>  static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>>>>>>>>  {
>>>>>>>> -       struct net_device *dev;
>>>>>>>> -       struct sock *sk = vs->sock->sk;
>>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>>> -
>>>>>>>> -       rcu_read_lock();
>>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>>> -               if (dev->netdev_ops->ndo_add_vxlan_port)
>>>>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>>>>> -                                                           port);
>>>>>>>> -       }
>>>>>>>> -       rcu_read_unlock();
>>>>>>>> +       udp_tunnel_notify_add_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  /* Notify netdevs that UDP port is no more listening */
>>>>>>>>  static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>>>>>>>>  {
>>>>>>>> -       struct net_device *dev;
>>>>>>>> -       struct sock *sk = vs->sock->sk;
>>>>>>>> -       struct net *net = sock_net(sk);
>>>>>>>> -       sa_family_t sa_family = vxlan_get_sk_family(vs);
>>>>>>>> -       __be16 port = inet_sk(sk)->inet_sport;
>>>>>>>> -
>>>>>>>> -       rcu_read_lock();
>>>>>>>> -       for_each_netdev_rcu(net, dev) {
>>>>>>>> -               if (dev->netdev_ops->ndo_del_vxlan_port)
>>>>>>>> -                       dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>>>>>>>> -                                                           port);
>>>>>>>> -       }
>>>>>>>> -       rcu_read_unlock();
>>>>>>>> +       udp_tunnel_notify_del_rx_port(vs->sock, UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  /* Add new entry to forwarding table -- assumes lock held */
>>>>>>>> @@ -2525,30 +2501,22 @@ static struct device_type vxlan_type = {
>>>>>>>>         .name = "vxlan",
>>>>>>>>  };
>>>>>>>>
>>>>>>>> -/* Calls the ndo_add_vxlan_port of the caller in order to
>>>>>>>> +/* Calls the ndo_add_udp_enc_port of the caller in order to
>>>>>>>>   * supply the listening VXLAN udp ports. Callers are expected
>>>>>>>> - * to implement the ndo_add_vxlan_port.
>>>>>>>> + * to implement the ndo_add_udp_enc_port.
>>>>>>>>   */
>>>>>>>>  static void vxlan_push_rx_ports(struct net_device *dev)
>>>>>>>>  {
>>>>>>>>         struct vxlan_sock *vs;
>>>>>>>>         struct net *net = dev_net(dev);
>>>>>>>>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>>>>>>>> -       sa_family_t sa_family;
>>>>>>>> -       __be16 port;
>>>>>>>>         unsigned int i;
>>>>>>>>
>>>>>>>> -       if (!dev->netdev_ops->ndo_add_vxlan_port)
>>>>>>>> -               return;
>>>>>>>> -
>>>>>>>>         spin_lock(&vn->sock_lock);
>>>>>>>>         for (i = 0; i < PORT_HASH_SIZE; ++i) {
>>>>>>>> -               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
>>>>>>>> -                       port = inet_sk(vs->sock->sk)->inet_sport;
>>>>>>>> -                       sa_family = vxlan_get_sk_family(vs);
>>>>>>>> -                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>>>>>>> -                                                           port);
>>>>>>>> -               }
>>>>>>>> +               hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist)
>>>>>>>> +                       udp_tunnel_push_rx_port(dev, vs->sock,
>>>>>>>> +                                               UDP_ENC_OFFLOAD_TYPE_VXLAN);
>>>>>>>>         }
>>>>>>>>         spin_unlock(&vn->sock_lock);
>>>>>>>>  }
>>>>>>>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>>>>>>>> index 9d14f707e534..704f931fd9ad 100644
>>>>>>>> --- a/include/net/udp_tunnel.h
>>>>>>>> +++ b/include/net/udp_tunnel.h
>>>>>>>> @@ -84,6 +84,18 @@ struct udp_tunnel_sock_cfg {
>>>>>>>>  void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
>>>>>>>>                            struct udp_tunnel_sock_cfg *sock_cfg);
>>>>>>>>
>>>>>>>> +/* List of offloadable UDP tunnel types */
>>>>>>>> +enum udp_enc_offloads {
>>>>>>>> +       UDP_ENC_OFFLOAD_TYPE_VXLAN,     /* RFC 7348 */
>>>>>>>> +       UDP_ENC_OFFLOAD_TYPE_GENEVE,    /* draft-ietf-nvo3-geneve */
>>>>>>>> +};
>>>>>>>> +
>>>>>>> We've already had a lot of discussion on this. The clear outcome from
>>>>>>> netdev was that we need to support generic offloads and move away from
>>>>>>> protocol specific offload. Generalizing the interface to allow vendors
>>>>>>> to unnecessarily leak out protocol specific features undermines that
>>>>>>> effort.
>>>>>>
>>>>>> Then in turn we get dirty hacks like what we have right now where
>>>>>> VXLAN-GPE is attempting to reuse the VXLAN offload functions or
>>>>>> drivers that just hard-code GENEVE ports.
>>>>>>
>>>>>> Going full obstructionist on this isn't going to work.  We need to be
>>>>>> able to support these type of offloads because the switch vendors are
>>>>>> going to force the NIC vendors to do so.  We will likely never be able
>>>>>> to convince Cisco to implement an outer transmit checksum on their
>>>>>> switches.  In order to make offloads work without the outer checksum
>>>>>> we will need to be able to parse the frames in order to be able to
>>>>>> validate the inner checksum values.
>>>>>>
>>>>> NIC vendors can support checksum-complete. This works with any form of
>>>>> UDP encapsulation, and IP protocol (like extension headers), and other
>>>>> form of tunneling we can dream up. That was the whole point of Dave's
>>>>> keynote at netdev.
>>>>
>>>> Right.  That covers one tiny piece of the whole problem, but you are
>>>> holding out for hardware that may not be introduced for another 3 to 5
>>>> years.  The fact is trying to getting NIC vendors to support
>>>> checksum-complete is all well and good, but you seem to have forgotten
>>>> that NIC vendors are incredibly slow when it comes to implementing
>>>> anything.  In the meantime we will have the stuff that was already in
>>>> the pipeline coming out over the next several years.
>>>>
>>>> How about the fact that we need to know that there is a tunnel there
>>>> if we want to do anything like try to parse the inner headers of a
>>>> given tunnel on Rx?  How do you propose to solve the RSS problem?
>>>
>>> Solved by doing RSS and ECMP hash over 3-tuple of IP addresses and
>>> IPv6 flow label (not ports). Non-zero flow labels will soon be widely
>>> used over the Internet. IOS already is already setting them, Android
>>> should pick up support in the next rebase, and the MS guys have
>>> assured me that they will add support to next version of Windows. Like
>>> a generic checksum offload, flow label works with an IP protocol,
>>> extension header, fragmentation, UDP encapsulation, etc. With this
>>> there is no reason for devices to parse L4 headers just to forward a
>>> packet. HW vendors (both switches and NICs) are strongly encouraged to
>>> support them.
>>
>> Right so with all this encouragement, where are we at on getting the
>> switches out there to support this?  The switches are what is pushing
>> most of the tunnel workloads and such, and it is the switches that are
>> going to be the painful interfaces for us to try to offload data from.
>>
>> Also have you looked into the effect on regular traffic for trying to
>> deal with an extra IPv4 options header?  The Intel NICs should be able
>> to deal with it, at least for Tx, but my concern would be the NICs
>> that perform parsing based offloads and such.
>>
>> It seems like we might be arguing past each other as I am focused on
>> what we can do right now, and it seems like you are arguing the point
>> that we need features that probably won't be enabled in the ecosystem
>> for 5 to 10 years depending on the hardware release cycles.  Hopefully
>> we can agree to disagree and can address this again when new features
>> start to become available.  At that time we could probably look at
>> deprecating the driver API for the UDP ports assuming we have other
>> means of resolving this available by then.
>>
> Alex,
>
> I looked though the previous thread on protocol specific vs. generic
> offloads in http://www.spinics.net/lists/netdev/msg354311.html and the
> outcome from that. To quote davem:
>
> "Doing anything other than providing 2's complement checksums in the
> RX descriptor doesn't work.  We know this.

For Rx checksum I would agree with you there.  Although I would argue
that we can solve much of this by just providing an outer UDP checksum
on Tx anyway.  The one that annoys me the most is the ones that end up
using this for Tx, but that is a topic for another time.

The problem is this goes beyond just the Rx checksum.  There end up
being a number of Rx offloads and the like that also need to figure
out where the inner headers start in order to handle things like MAC
based filtering, RSS, and the like.

> So we will not add to our core architecture and frameworks anything
> that directly facilitates designs which we know are suboptimal.  And
> protocol specific support for tunnel offloading is suboptimal and not
> the way forward."

Yes, but then we still ended up with the GENEVE offloads being
accepted anyway.  My concern is that we didn't spend enough time
talking about the implementation details of it because everything got
drowned out by the arguments for not accepting the offloads at all.
So we ended up just cloning what was already there for VXLAN which
ended up setting a bad precedent for how we should handle this going
forward.  It would be just as easy for us to stop any future additions
after my changes as it would the current code.  The only big
difference is that with my changes applied we can strip out all the
stupid defines and would likely be arguing over 1 or 2 patches rather
than a set of patches to accomplish the same task.

> Unless Dave has changed his mind on this or you have new arguments or
> new data, I really don't see point in participating in further
> discussion on patches like this. Sorry.

I'm not going to speculate on what Dave's opinion on this is.  I'll
wait to hear it from him.

My concern at this point is that we have several issues.  Specifically
we have VXLAN-GPE trying to pass itself off as VXLAN when it clearly
is not, and I know we are going to end up with somebody eventually
trying to push this feature into the kernel.  I know for a fact there
is hardware out there that already supports it.  I'm trying to get
ahead of this and define what the interface is supposed to look like
myself so that we don't end up with somebody unfamiliar with all this
trying to push it.  This way we can avoid having some hardware vendor
on a timeline trying to push it through quick as in the case of i40e,
or somebody trying to get around it by just hard coding it into their
driver like occurred with bnxt.

While I appreciate the opinion, outright refusing to enable the
existing offloads is counterproductive.  There are customers out there
that already have this hardware.  There are driver writers out there
who are going to have to enable these features one way or another.  If
we want to be obstructionists then I am sure they can just work around
us and write up out-of-tree drivers and use something like module
parameters to enable offloads on a specific port.  Most of these
implementations only seem to support one port anyway.  I just thought
it might be better to have this figured out in the kernel so that we
didn't end up creating a bigger mess than needed with each vendor
going off and doing their own out-of-tree implementation.

- Alex

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[David Miller]

From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Mon, 13 Jun 2016 19:50:02 -0700

> I'm not going to speculate on what Dave's opinion on this is.  I'll
> wait to hear it from him.
> 
> My concern at this point is that we have several issues.  Specifically
> we have VXLAN-GPE trying to pass itself off as VXLAN when it clearly
> is not, and I know we are going to end up with somebody eventually
> trying to push this feature into the kernel.  I know for a fact there
> is hardware out there that already supports it.  I'm trying to get
> ahead of this and define what the interface is supposed to look like
> myself so that we don't end up with somebody unfamiliar with all this
> trying to push it.  This way we can avoid having some hardware vendor
> on a timeline trying to push it through quick as in the case of i40e,
> or somebody trying to get around it by just hard coding it into their
> driver like occurred with bnxt.
> 
> While I appreciate the opinion, outright refusing to enable the
> existing offloads is counterproductive.  There are customers out there
> that already have this hardware.  There are driver writers out there
> who are going to have to enable these features one way or another.  If
> we want to be obstructionists then I am sure they can just work around
> us and write up out-of-tree drivers and use something like module
> parameters to enable offloads on a specific port.  Most of these
> implementations only seem to support one port anyway.  I just thought
> it might be better to have this figured out in the kernel so that we
> didn't end up creating a bigger mess than needed with each vendor
> going off and doing their own out-of-tree implementation.

My plan is to try and properly balance the two side of this situation.

Realistically, and Alex is right on this, we shoot ourselves in the
foot by not supporting offloads that exist in hardware now even if
they are not generic.

So I would encourage Alex to keep working on his patch set and to
keep working on the feedback he is given.

Thanks.

Re: [net-next PATCH 01/15] net: Combine GENEVE and VXLAN port offload notifiers into single functions

[Tom Herbert]

On Wed, Jun 15, 2016 at 12:22 AM, David Miller <davem@davemloft.net> wrote:
> From: Alexander Duyck <alexander.duyck@gmail.com>
> Date: Mon, 13 Jun 2016 19:50:02 -0700
>
>> I'm not going to speculate on what Dave's opinion on this is.  I'll
>> wait to hear it from him.
>>
>> My concern at this point is that we have several issues.  Specifically
>> we have VXLAN-GPE trying to pass itself off as VXLAN when it clearly
>> is not, and I know we are going to end up with somebody eventually
>> trying to push this feature into the kernel.  I know for a fact there
>> is hardware out there that already supports it.  I'm trying to get
>> ahead of this and define what the interface is supposed to look like
>> myself so that we don't end up with somebody unfamiliar with all this
>> trying to push it.  This way we can avoid having some hardware vendor
>> on a timeline trying to push it through quick as in the case of i40e,
>> or somebody trying to get around it by just hard coding it into their
>> driver like occurred with bnxt.
>>
>> While I appreciate the opinion, outright refusing to enable the
>> existing offloads is counterproductive.  There are customers out there
>> that already have this hardware.  There are driver writers out there
>> who are going to have to enable these features one way or another.  If
>> we want to be obstructionists then I am sure they can just work around
>> us and write up out-of-tree drivers and use something like module
>> parameters to enable offloads on a specific port.  Most of these
>> implementations only seem to support one port anyway.  I just thought
>> it might be better to have this figured out in the kernel so that we
>> didn't end up creating a bigger mess than needed with each vendor
>> going off and doing their own out-of-tree implementation.
>
> My plan is to try and properly balance the two side of this situation.
>
> Realistically, and Alex is right on this, we shoot ourselves in the
> foot by not supporting offloads that exist in hardware now even if
> they are not generic.
>
It was the interface that was rejected not any particular hardware
offload. We pointed out that n-tuple filtering is a much better
interface anyway allowing address binding and implementing offload for
new protocols without needed to change common header files. Besides,
if it's really just about VXLAN-GPE and that is the last of the
protocols in the three year HW pipeline then we can add another ndo
function specific for that.

Personally, I think the question of protocol specific offloads is moot
anyway. In the past six months the world since this issue came up the
world has moved forward. We have a lot better support for generic
offloads (Alex's patches on that), GRO for UDP is based on socket
lookup instead alternate lookup destination port, LCO has been
integrated, real support for ipxip6, extension headers being developed
that are going to be deployed, ILA allows virtualization without
encapsulation (no HW support needed), more IPv6 deployment in general,
MPLS/UDP is now an Internet standard, GRE/UDP will go to WGLC in IETF
soon, even more encapsulation protocols like IP/UDP have been
proposed, QUIC is being deployed and probably going to be a WG, first
TOU patches will soon be upstream, FD.io and DPDK are still making a
big fuss to bypass the kernel-- XDP is proving the kernel is not
problem, we now have first instances of truly programmable NICs using
BPF which is the path to truly generic offloads. This is really quite
an impressive list for sure! I don't see that protocol specific
offloads figure into much of this.

Tom

> So I would encourage Alex to keep working on his patch set and to
> keep working on the feedback he is given.
>
> Thanks.