[ovs-dev] [PATCH v8] datapath: Add support for VXLAN tunnels to Open vSwitch

[Jesse Gross]

On Wed, Dec 5, 2012 at 1:06 PM, Kyle Mestery <kmestery at cisco.com> wrote:
> Add support for VXLAN tunnels to Open vSwitch. Add support
> for setting the destination UDP port on a per-port basis.
> This is done by adding a "dst_port" parameter to the port
> configuration. This is only applicable currently to VXLAN
> tunnels.
>
> Please note this currently does not implement any sort of multicast
> learning. With this patch, VXLAN tunnels must be configured similar
> to GRE tunnels (e.g. point to point). A subsequent patch will implement
> a VXLAN control plane in userspace to handle multicast learning.
>
> This patch set is based on one posted by Ben Pfaff on Oct. 12, 2011
> to the ovs-dev mailing list:
>
> http://openvswitch.org/pipermail/dev/2011-October/012051.html
>
> The patch has been maintained, updated, and freshened by me and a
> version of it is available at the following github repository:
>
> https://github.com/mestery/ovs-vxlan/tree/vxlan
>
> I've tested this patch with multiple VXLAN tunnels between hosts
> using different UDP port numbers. Performance is on par (though
> slightly faster) than comparable GRE tunnels.
>
> See the following IETF draft for additional information about VXLAN:
> http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02
>
> Signed-off-by: Kyle Mestery <kmestery at cisco.com>

Applied, thanks Kyle!

I noticed a couple of minor things at the last minute and decided to
just address them directly rather than go through another round.
Here's the patch that I applied, I hope that's OK:

diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
index 0b727bb..137e5d8 100644
--- a/datapath/vport-vxlan.c
+++ b/datapath/vport-vxlan.c
@@ -61,7 +61,6 @@ static inline int vxlan_hdr_len(const struct
tnl_mutable_config *m
  * struct vxlan_port - Keeps track of open UDP ports
  * @list: list element.
  * @port: The UDP port number in network byte order.
- * @net: The net namespace this port is assosciated with.
  * @socket: The socket created for this port number.
  * @count: How many ports are using this socket/port.
  */
@@ -240,8 +239,7 @@ static void vxlan_tunnel_release(struct vxlan_port
*vxlan_port)
                kfree(vxlan_port);
        }
 }
-static int vxlan_tunnel_setup(struct net *net, const char *name,
-                             struct nlattr *options,
+static int vxlan_tunnel_setup(struct net *net, struct nlattr *options,
                              struct vxlan_port **vxport)
 {
        struct nlattr *a;
@@ -277,7 +275,7 @@ static int vxlan_tunnel_setup(struct net *net,
const char *name,
        vxlan_port = kzalloc(sizeof(struct vxlan_port), GFP_KERNEL);
        if (!vxlan_port) {
                err = -ENOMEM;
-               goto error;
+               goto out;
        }

        vxlan_port->port = htons(dst_port);
@@ -292,10 +290,8 @@ static int vxlan_tunnel_setup(struct net *net,
const char *name
        goto out;

 error:
-       if (vxlan_port) {
-               list_del(&vxlan_port->list);
-               kfree(vxlan_port);
-       }
+       list_del(&vxlan_port->list);
+       kfree(vxlan_port);
 out:
        return err;
 }
@@ -303,7 +299,6 @@ out:
 static int vxlan_set_options(struct vport *vport, struct nlattr *options)
 {
        int err;
-       const char *vname = vport->ops->get_name(vport);
        struct net *net = ovs_dp_get_net(vport->dp);
        struct tnl_vport *tnl_vport = tnl_vport_priv(vport);
        struct tnl_mutable_config *config;
@@ -314,7 +309,7 @@ static int vxlan_set_options(struct vport *vport,
struct nlattr

        old_port = vxlan_port_exists(net, config->dst_port);

-       err = vxlan_tunnel_setup(net, vname, options, &vxlan_port);
+       err = vxlan_tunnel_setup(net, options, &vxlan_port);
        if (err)
                goto out;

@@ -359,8 +354,8 @@ static struct vport *vxlan_tnl_create(const struct
vport_parms *
        struct vport *vport;
        struct vxlan_port *vxlan_port = NULL;

-       err = vxlan_tunnel_setup(ovs_dp_get_net(parms->dp), parms->name,
-                                               parms->options, &vxlan_port);
+       err = vxlan_tunnel_setup(ovs_dp_get_net(parms->dp), parms->options,
+                                &vxlan_port);
        if (err)
                return ERR_PTR(err);

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index eadba27..1863d82 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -857,7 +857,9 @@ unparse_tunnel_config(const char *name OVS_UNUSED,
const char *t

     if (a[OVS_TUNNEL_ATTR_DST_PORT]) {
         uint16_t dst_port = nl_attr_get_u16(a[OVS_TUNNEL_ATTR_DST_PORT]);
-        smap_add_format(args, "dst_port", "%d", dst_port);
+        if (dst_port != VXLAN_DST_PORT) {
+            smap_add_format(args, "dst_port", "%d", dst_port);
+        }
     }

     if (flags & TNL_F_CSUM) {

The changes are:
 * Remove a couple areas of dead comments/code.
 * Simplify error handler in vxlan_tunnel_setup().  At first I thought
I saw a bug here; it turns out that it's OK after all but I decided to
keep the simplified version to make it easier to read.
 * Only print the VXLAN port if a non-standard port is used - we don't
usually print out options if the defaults are used.

Thanks again for all your work!