On Tue, Apr 19, 2016 at 04:25:32PM -0700, Jesse Gross wrote:
> On Mon, Apr 18, 2016 at 2:57 AM, Thadeu Lima de Souza Cascardo
> <casca...@redhat.com> wrote:
> > On Fri, Apr 15, 2016 at 08:36:51PM -0700, Jesse Gross wrote:
> >> On Fri, Apr 15, 2016 at 2:30 PM, Thadeu Lima de Souza Cascardo
> >> <casca...@redhat.com> wrote:
> >> > Hi, this is an RFC patch (could probably be 2 patches - more below), that
> >> > creates VXLAN devices in userspace and adds them as netdev vports,
> >> > instead of
> >> > using the vxlan vport code.
> >> >
> >> > The reason for this change is that it has been mentioned in the past
> >> > that some
> >> > of the vport code should be considered compat code, that is, it won't
> >> > receive
> >> > new features or flags.
> >>
> >> Thanks for looking at this. I agree that this is definitely a
> >> necessary direction.
> >>
> >> > First is the creation of the device under a switch/case. I tried to make
> >> > this a
> >> > netdev_class function, overriding the netdev class by
> >> > dpif_port_open_type. That
> >> > required exporting a lot of the vport functions. I can still do it that
> >> > way, if
> >> > that's preferrable, but this seems more simple.
> >>
> >> Can you give some examples of what would need to be exported? It would
> >> be nice to just set the open function for each type but if it is
> >> really horrible we can live with the switch statement.
> >>
> >
> > netdev_vport_{alloc, construct, destruct, dealloc}, get_tunnel_config,
> > set_tunnel_config, get_netdev_tunnel_config.
> >
> > We could also do it the other way around, write this code in netdev-vport.c
> > and
> > export one or two functions from netdev-linux or dpif-netlink, and the
> > create
> > function per tunnel type.
>
> OK, thanks, I understand now. I think what you have currently is
> probably the best solution for the time being.
>
> >> > The other problem is during port_del, that since we don't have a netdev,
> >> > the
> >> > dpif_port type would not be vxlan, and deciding whether to remove it or
> >> > not
> >> > could not be made. In fact, this is one of the main reasons why this is
> >> > an RFC.
> >> >
> >> > The solution here is to decide on the type by the device's name, and
> >> > there is
> >> > even a function for this, though it looks up for the netdev's already
> >> > created,
> >> > those that probably come from the database. However, when OVS starts, it
> >> > removes
> >> > ports from the datapath, and that information is not available, and may
> >> > not even
> >> > be. In fact, since the dpif_port has a different name because it's a
> >> > vport, it
> >> > won't match any netdev at all. So, I did the opposite of getting the
> >> > type from
> >> > the dpif_port name, ie, if it contains vxlan_sys, it's of vxlan type.
> >> > Even if we
> >> > make this more strict and use the port, we could still be in conflict
> >> > with a
> >> > vxlan device created by the user with such name. This should have been
> >> > one patch
> >> > by itself.
> >>
> >> What about using the driver name exposed through ethtool? I believe
> >> that all of the tunnel and internal devices expose this in a
> >> consistent way - i.e. a VXLAN device can be queried and get back the
> >> string "vxlan". Any driver other than the ones that we recognize can
> >> be assumed to be OVS_VPORT_TYPE_NETDEV.
> >>
> >
> > I don't see how this address the case when the user adds a vxlan interface
> > created by the system. Like:
> >
> > ip link add vxlan_sys_4789 type vxlan id 10 remote 192.168.123.1 dstport
> > 4789
> > ovs-vsctl add-port br0 vxlan_sys_4789
> >
> > Its driver would be vxlan. That goes to vxlan0 too.
>
> I think we can check the combination of device type and the options
> that are set on it (the same as what we need to do after we create the
> device). If all of those match then it doesn't matter whether we added
> it previously or the user added it - the device will work exactly the
> same either way. If there isn't a match then we should bail out
> without adding the port. This is pretty similar to the behavior of the
> current compat code in the kernel (in that case if a port already
> exists with the same name, it just aborts).
>
As I pointed out in another message, vxlan_sys* is a reserved name, if
any port like that is added to the database, it's going to be rejected
by vswitchd. The same goes for other vports. So, I think it's sufficient
to check this name, then set the type appropriately, and depending on
the type, try to remove the interface. So vxlan0 is not going to be
removed, but vxlan_sys_4789 will.
> Two unresolved issues in my mind:
> * How do we handle cleaning up ports (including in cases where
> userspace crashes)? In the kernel case, the port will stick around
> until either the module is removed or the port is explicitly deleted.
> * How do we handle upgrade where the new version of OVS needs options
> that are different from the previous version? This is basically a
> special version of the port being created by someone else but we
> should be able to handle the differences. Depending on how good a job
> we do at cleaning up the port, this might not be an issue.
OVS already does a good job at cleaning up ports when it starts. In
open_dpif_backer, any port not belonging to init_ofp_ports will be
removed from the datapath. As I implemented the code, those vxlan_sys
devices are going to be deleted as well after removal from the datapath,
and only if they were present in the datapath in the first place. That
means the port won't stick around in the case of a crash, and neither in
the case of upgrades.
Cascardo.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
