On Dec 5, 2012, at 8:13 PM, Jesse Gross <[email protected]> wrote: > On Wed, Dec 5, 2012 at 1:06 PM, Kyle Mestery <[email protected]> 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 <[email protected]> > > 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!
Awesome, glad to have this in the tree! And thank you for all your work reviewing and providing positive feedback. Thanks, Kyle _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
