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.
> > 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.
But... wait a minute. We don't support adding devices name as vxlan_sys*. Such
names are reserved. I think that means we could probably rely on the names.
> > Jiri has suggested that we require users to create the interfaces
> > themselves, by
> > using whatever method their OS has, and add them as netdev devices. That
> > would
> > still require fetching some of the configuration from the device in order to
> > make it properly work with flow-based tunnels. In fact, if we set the
> > remote or
> > local IP addresses on those devices, this would require multiple interfaces
> > instead of only one just to be able to specify the same level of
> > configuration
> > as ovsdb allows us to. And that still requires some proper changes in order
> > to
> > support flow-based tunnels (calling tnl_port_add/tnl_port_del with proper
> > configuration, for example).
>
> I'm not too excited about this. It seems like it would be a regression
> - currently OVSDB allows remote creation of tunnels, so it seems like
> this would break existing setups if it also requires users to
> explicitly create tunnel devices on the host ahead of time.
Agreed. That's a very good reason not to go this path. And realizing we already
reserve the names, I am more comfortable relying on only that.
>
> One comment on the patch itself - it's possible that the device that
> is being created might not support all of the necessary options that
> we are passing to it. For example, the original VXLAN driver as merged
> into the kernel didn't support COLLECT_METADATA. We'll need to check
> that the device that was created supports what we need and fallback to
> the old model otherwise.
>
That should be taken care of. We would get EINVAL, and that's why I return
EOPNOTSUPP if that's the case. Then, the code falls back to the compat mode.
> I presume that you will convert all of the vport types over once we
> have settled on the right model?
Sure.
Thanks for the review and help.
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
