[net-next 1/2] vxlan: Notify drivers for listening UDP port changes

[net-next 1/2] vxlan: Notify drivers for listening UDP port changes

[Jeff Kirsher]

From: Joseph Gasparakis <joseph.gasparakis@intel.com>

This patch adds two more ndo ops: ndo_add_rx_vxlan_port() and
ndo_del_rx_vxlan_port().

Drivers can get notifications through the above functions about changes
of the UDP listening port of VXLAN. Also, when physical ports come up,
now they can call vxlan_get_rx_port() in order to obtain the port number(s)
of the existing VXLAN interface in case they already up before them.

This information about the listening UDP port would be used for VXLAN
related offloads.

A big thank you to John Fastabend (john.r.fastabend@intel.com) for his input
and his suggestions on this patch set.

CC: John Fastabend <john.r.fastabend@intel.com>
CC: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/vxlan.c       | 53 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/netdevice.h | 16 ++++++++++++++
 include/net/vxlan.h       |  1 +
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 3b21aca..2b4ce79 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -456,6 +456,32 @@ static int vxlan_fdb_append(struct vxlan_fdb *f,
 	return 1;
 }
 
+/* Notify netdevs that UDP port started listening */
+static void vxlan_notify_add_rx_port(struct net *net, __be16 port)
+{
+	struct net_device *dev;
+
+	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, htons(port));
+	}
+	rcu_read_unlock();
+}
+
+/* Notify netdevs that UDP port is no more listening */
+static void vxlan_notify_del_rx_port(struct net *net, __be16 port)
+{
+	struct net_device *dev;
+
+	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, htons(port));
+	}
+	rcu_read_unlock();
+}
+
 /* Add new entry to forwarding table -- assumes lock held */
 static int vxlan_fdb_create(struct vxlan_dev *vxlan,
 			    const u8 *mac, __be32 ip,
@@ -797,13 +823,15 @@ static void vxlan_sock_hold(struct vxlan_sock *vs)
 
 void vxlan_sock_release(struct vxlan_sock *vs)
 {
-	struct vxlan_net *vn = net_generic(sock_net(vs->sock->sk), vxlan_net_id);
+	struct net *net = sock_net(vs->sock->sk);
+	struct vxlan_net *vn = net_generic(net, vxlan_net_id);
 
 	if (!atomic_dec_and_test(&vs->refcnt))
 		return;
 
 	spin_lock(&vn->sock_lock);
 	hlist_del_rcu(&vs->hlist);
+	vxlan_notify_del_rx_port(net, inet_sk(vs->sock->sk)->inet_sport);
 	spin_unlock(&vn->sock_lock);
 
 	queue_work(vxlan_wq, &vs->del_work);
@@ -1543,6 +1571,28 @@ static struct device_type vxlan_type = {
 	.name = "vxlan",
 };
 
+/* Calls the ndo_add_vxlan_port of the caller in order to
+ * supply the listening VXLAN udp ports.
+ */
+void vxlan_get_rx_port(struct net_device *dev)
+{
+	struct vxlan_sock *vs;
+	struct net *net = dev_net(dev);
+	u16 port;
+	int i;
+
+	if (!dev || !dev->netdev_ops || !dev->netdev_ops->ndo_add_vxlan_port)
+		return;
+
+	for (i = 0; i < PORT_HASH_SIZE; ++i) {
+		hlist_for_each_entry_rcu(vs, vs_head(net, i), hlist) {
+			port = htons(inet_sk(vs->sock->sk)->inet_sport);
+			dev->netdev_ops->ndo_add_vxlan_port(dev, port);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(vxlan_get_rx_port);
+
 /* Initialize the device structure. */
 static void vxlan_setup(struct net_device *dev)
 {
@@ -1723,6 +1773,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
 	inet_sk(sk)->mc_loop = 0;
 	spin_lock(&vn->sock_lock);
 	hlist_add_head_rcu(&vs->hlist, vs_head(net, port));
+	vxlan_notify_add_rx_port(net, port);
 	spin_unlock(&vn->sock_lock);
 
 	/* Mark socket as an encapsulation socket. */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 077363d..f56b141 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -948,6 +948,18 @@ struct netdev_phys_port_id {
  *	Called to get ID of physical port of this device. If driver does
  *	not implement this, it is assumed that the hw is not able to have
  *	multiple net devices on single physical port.
+ *
+ * int (*ndo_add_vxlan_port)(struct  net_device *dev,
+ *			      __u16 port);
+ *	Called by vxlan to notiy a driver about the UDP port that vxlan
+ *	is listnening to. It is called only when a new port starts listening.
+ *	The operation is protected by the vxlan_net->sock_lock.
+ *
+ * int (*ndo_del_vxlan_port)(struct  net_device *dev,
+ *			    __u16 port);
+ *	Called by vxlan to notify the driver about a UDP port of vxlan
+ *	that is not listening anymore. The operation is protected by
+ *	the vxlan_net->sock_lock.
  */
 struct net_device_ops {
 	int			(*ndo_init)(struct net_device *dev);
@@ -1078,6 +1090,10 @@ struct net_device_ops {
 						      bool new_carrier);
 	int			(*ndo_get_phys_port_id)(struct net_device *dev,
 							struct netdev_phys_port_id *ppid);
+	int			(*ndo_add_vxlan_port)(struct  net_device *dev,
+						      __u16 port);
+	int			(*ndo_del_vxlan_port)(struct  net_device *dev,
+						      __u16 port);
 };
 
 /*
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index ad342e3..a0dc497 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -36,4 +36,5 @@ int vxlan_xmit_skb(struct net *net, struct vxlan_sock *vs,
 
 __be16 vxlan_src_port(__u16 port_min, __u16 port_max, struct sk_buff *skb);
 
+void vxlan_get_rx_port(struct net_device *netdev);
 #endif
-- 
1.8.3.1

[net-next 2/2] ixgbe: Get and display the notifications from changes of the Rx vxlan UDP port

[Jeff Kirsher]

From: Joseph Gasparakis <joseph.gasparakis@intel.com>

This RFC patch showcases how drivers can use the changes in
"vxlan: Notify drivers for listening UDP port changes", and its
sole purpose is to help people test the changes introduced there.
This is not meant to be submitted for inclusion in the kernel.

CC: Don Skidmore <donald.c.skidmore@intel.com>
CC: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 128d6b8..93a3af2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -47,6 +47,9 @@
 #include <linux/if_bridge.h>
 #include <linux/prefetch.h>
 #include <scsi/fc/fc_fcoe.h>
+#ifdef CONFIG_VXLAN_MODULE
+#include <net/vxlan.h>
+#endif
 
 #include "ixgbe.h"
 #include "ixgbe_common.h"
@@ -5174,6 +5177,9 @@ static int ixgbe_open(struct net_device *netdev)
 
 	ixgbe_up_complete(adapter);
 
+#ifdef CONFIG_VXLAN_MODULE
+	vxlan_get_rx_port(netdev);
+#endif
 	return 0;
 
 err_set_queues:
@@ -7283,6 +7289,21 @@ static int ixgbe_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode);
 }
 
+#ifdef CONFIG_VXLAN_MODULE
+static int ixgbe_add_vxlan_port(struct net_device *netdev,
+				__u16 port)
+{
+	netdev_info(netdev, ">>>>> Adding VXLAN port %d\n", port);
+	return 0;
+}
+
+static int ixgbe_del_vxlan_port(struct net_device *netdev,
+				__u16 port)
+{
+	netdev_info(netdev, "<<<<< Removing VXLAN port %d\n", port);
+	return 0;
+}
+#endif
 static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_open		= ixgbe_open,
 	.ndo_stop		= ixgbe_close,
@@ -7327,6 +7348,10 @@ static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_fdb_add		= ixgbe_ndo_fdb_add,
 	.ndo_bridge_setlink	= ixgbe_ndo_bridge_setlink,
 	.ndo_bridge_getlink	= ixgbe_ndo_bridge_getlink,
+#ifdef CONFIG_VXLAN_MODULE
+	.ndo_add_vxlan_port	= ixgbe_add_vxlan_port,
+	.ndo_del_vxlan_port	= ixgbe_del_vxlan_port,
+#endif
 };
 
 /**
-- 
1.8.3.1

Re: [net-next 2/2] ixgbe: Get and display the notifications from changes of the Rx vxlan UDP port

[Jeff Kirsher]

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

On Tue, 2013-08-27 at 21:46 -0700, Jeff Kirsher wrote:
> From: Joseph Gasparakis <joseph.gasparakis@intel.com>
> 
> This RFC patch showcases how drivers can use the changes in
> "vxlan: Notify drivers for listening UDP port changes", and its
> sole purpose is to help people test the changes introduced there.
> This is not meant to be submitted for inclusion in the kernel.
> 
> CC: Don Skidmore <donald.c.skidmore@intel.com>
> CC: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25
> +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)

Just to be clear, this patch is an RFC.  I missed putting it in the
patch title.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [net-next 1/2] vxlan: Notify drivers for listening UDP port changes

[Cong Wang]

On Wed, 28 Aug 2013 at 04:46 GMT, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> From: Joseph Gasparakis <joseph.gasparakis@intel.com>
>
> This patch adds two more ndo ops: ndo_add_rx_vxlan_port() and
> ndo_del_rx_vxlan_port().
>
> Drivers can get notifications through the above functions about changes
> of the UDP listening port of VXLAN. Also, when physical ports come up,
> now they can call vxlan_get_rx_port() in order to obtain the port number(s)
> of the existing VXLAN interface in case they already up before them.
>
> This information about the listening UDP port would be used for VXLAN
> related offloads.
>

Since this is a notification, how about adding a new NETDEV_* event
rather than adding two new ndo_* ops?

Re: [net-next 1/2] vxlan: Notify drivers for listening UDP port changes

[Ben Hutchings]

On Wed, 2013-08-28 at 07:47 +0000, Cong Wang wrote:
> On Wed, 28 Aug 2013 at 04:46 GMT, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> > From: Joseph Gasparakis <joseph.gasparakis@intel.com>
> >
> > This patch adds two more ndo ops: ndo_add_rx_vxlan_port() and
> > ndo_del_rx_vxlan_port().
> >
> > Drivers can get notifications through the above functions about changes
> > of the UDP listening port of VXLAN. Also, when physical ports come up,
> > now they can call vxlan_get_rx_port() in order to obtain the port number(s)
> > of the existing VXLAN interface in case they already up before them.
> >
> > This information about the listening UDP port would be used for VXLAN
> > related offloads.
> >
> 
> Since this is a notification, how about adding a new NETDEV_* event
> rather than adding two new ndo_* ops?

Since this is a change at the netns (rather than net device) level, it
makes more logical sense to send a single notification for each change.
But any driver that needs to know about this would then need to iterate
over its own devices (limited to the given netns) - which means writing
the same iteration in each driver that supports this (with added
filtering by driver), or adding a list of devices to each driver.  So
practically it seems better to do this iteration in the core as
proposed.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [net-next 1/2] vxlan: Notify drivers for listening UDP port changes

[John Fastabend]

On 8/27/2013 9:46 PM, Jeff Kirsher wrote:
> From: Joseph Gasparakis <joseph.gasparakis@intel.com>
>
> This patch adds two more ndo ops: ndo_add_rx_vxlan_port() and
> ndo_del_rx_vxlan_port().
>
> Drivers can get notifications through the above functions about changes
> of the UDP listening port of VXLAN. Also, when physical ports come up,
> now they can call vxlan_get_rx_port() in order to obtain the port number(s)
> of the existing VXLAN interface in case they already up before them.
>
> This information about the listening UDP port would be used for VXLAN
> related offloads.

[...]

>   /* Add new entry to forwarding table -- assumes lock held */
>   static int vxlan_fdb_create(struct vxlan_dev *vxlan,
>   			    const u8 *mac, __be32 ip,
> @@ -797,13 +823,15 @@ static void vxlan_sock_hold(struct vxlan_sock *vs)
>
>   void vxlan_sock_release(struct vxlan_sock *vs)
>   {
> -	struct vxlan_net *vn = net_generic(sock_net(vs->sock->sk), vxlan_net_id);
> +	struct net *net = sock_net(vs->sock->sk);
> +	struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>
>   	if (!atomic_dec_and_test(&vs->refcnt))
>   		return;
>
>   	spin_lock(&vn->sock_lock);
>   	hlist_del_rcu(&vs->hlist);
> +	vxlan_notify_del_rx_port(net, inet_sk(vs->sock->sk)->inet_sport);
>   	spin_unlock(&vn->sock_lock);

Both the del and add port are protected by sock_lock serializing the
operations.

>
>   	queue_work(vxlan_wq, &vs->del_work);
> @@ -1543,6 +1571,28 @@ static struct device_type vxlan_type = {
>   	.name = "vxlan",
>   };
>
> +/* Calls the ndo_add_vxlan_port of the caller in order to
> + * supply the listening VXLAN udp ports.
> + */
> +void vxlan_get_rx_port(struct net_device *dev)
> +{
> +	struct vxlan_sock *vs;
> +	struct net *net = dev_net(dev);
> +	u16 port;
> +	int i;
> +
> +	if (!dev || !dev->netdev_ops || !dev->netdev_ops->ndo_add_vxlan_port)
> +		return;
> +
> +	for (i = 0; i < PORT_HASH_SIZE; ++i) {
> +		hlist_for_each_entry_rcu(vs, vs_head(net, i), hlist) {
> +			port = htons(inet_sk(vs->sock->sk)->inet_sport);
> +			dev->netdev_ops->ndo_add_vxlan_port(dev, port);

However this list walk occurs without the sock_lock. Looks like you
could delete a port and then subsequently add it here if you had really
"good" timing.

Then it would be deleted from the vxlan list but pushed into hardware.
Probably not a terrible scenario but it would waste hardware resources.

I suspect you want to lock this list traversal with the sock_lock as
well.

> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(vxlan_get_rx_port);

Re: [net-next 1/2] vxlan: Notify drivers for listening UDP port changes

[Joseph Gasparakis]

On Wed, 28 Aug 2013, John Fastabend wrote:

> On 8/27/2013 9:46 PM, Jeff Kirsher wrote:
> > From: Joseph Gasparakis <joseph.gasparakis@intel.com>
> >
> > This patch adds two more ndo ops: ndo_add_rx_vxlan_port() and
> > ndo_del_rx_vxlan_port().
> >
> > Drivers can get notifications through the above functions about changes
> > of the UDP listening port of VXLAN. Also, when physical ports come up,
> > now they can call vxlan_get_rx_port() in order to obtain the port number(s)
> > of the existing VXLAN interface in case they already up before them.
> >
> > This information about the listening UDP port would be used for VXLAN
> > related offloads.
> 
> [...]
> 
> >   /* Add new entry to forwarding table -- assumes lock held */
> >   static int vxlan_fdb_create(struct vxlan_dev *vxlan,
> >   			    const u8 *mac, __be32 ip,
> > @@ -797,13 +823,15 @@ static void vxlan_sock_hold(struct vxlan_sock *vs)
> >
> >   void vxlan_sock_release(struct vxlan_sock *vs)
> >   {
> > -	struct vxlan_net *vn = net_generic(sock_net(vs->sock->sk), vxlan_net_id);
> > +	struct net *net = sock_net(vs->sock->sk);
> > +	struct vxlan_net *vn = net_generic(net, vxlan_net_id);
> >
> >   	if (!atomic_dec_and_test(&vs->refcnt))
> >   		return;
> >
> >   	spin_lock(&vn->sock_lock);
> >   	hlist_del_rcu(&vs->hlist);
> > +	vxlan_notify_del_rx_port(net, inet_sk(vs->sock->sk)->inet_sport);
> >   	spin_unlock(&vn->sock_lock);
> 
> Both the del and add port are protected by sock_lock serializing the
> operations.
> 
> >
> >   	queue_work(vxlan_wq, &vs->del_work);
> > @@ -1543,6 +1571,28 @@ static struct device_type vxlan_type = {
> >   	.name = "vxlan",
> >   };
> >
> > +/* Calls the ndo_add_vxlan_port of the caller in order to
> > + * supply the listening VXLAN udp ports.
> > + */
> > +void vxlan_get_rx_port(struct net_device *dev)
> > +{
> > +	struct vxlan_sock *vs;
> > +	struct net *net = dev_net(dev);
> > +	u16 port;
> > +	int i;
> > +
> > +	if (!dev || !dev->netdev_ops || !dev->netdev_ops->ndo_add_vxlan_port)
> > +		return;
> > +
> > +	for (i = 0; i < PORT_HASH_SIZE; ++i) {
> > +		hlist_for_each_entry_rcu(vs, vs_head(net, i), hlist) {
> > +			port = htons(inet_sk(vs->sock->sk)->inet_sport);
> > +			dev->netdev_ops->ndo_add_vxlan_port(dev, port);
> 
> However this list walk occurs without the sock_lock. Looks like you
> could delete a port and then subsequently add it here if you had really
> "good" timing.
> 
> Then it would be deleted from the vxlan list but pushed into hardware.
> Probably not a terrible scenario but it would waste hardware resources.
> 
> I suspect you want to lock this list traversal with the sock_lock as
> well.
> 

Yes, you are right. I will wait a little bit more to gather more comments 
and will push a v2 fixing this issue.

> > +		}
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(vxlan_get_rx_port);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>