On Thu, Jun 16, 2016 at 12:40 PM, Thadeu Lima de Souza Cascardo <casca...@redhat.com> wrote: > On Wed, Jun 08, 2016 at 03:21:58PM -0300, Thadeu Lima de Souza Cascardo wrote: >> This series adds support for the creation of tunnels using the rtnetlink >> interface. This will open the possibility for new features and flags on those >> vports without the need to change vport compatibility code. >> >> Support for STT and LISP have not been added because these are not upstream >> yet, >> so we don't know how the interface will be like upstream. And there are no >> features in the current drivers right now we could make use of. >> >> We are able to set the MTU to UINT16_MAX since it is not restricted by the >> driver during newlink. >> >> Thadeu Lima de Souza Cascardo (5): >> netdev: get device type from vport prefix if not found >> dpif-netlink: break out code to add compat and non-compat vports >> dpif-netlink: add VXLAN creation support >> dpif-netlink: add GRE creation support >> dpif-netlink: add GENEVE creation support >> >> lib/dpif-netlink.c | 526 >> ++++++++++++++++++++++++++++++++++++++++++++++++----- >> lib/netdev.c | 18 ++ >> 2 files changed, 494 insertions(+), 50 deletions(-) > > I found what is causing the issues found by Jesse and Flavio. > > The reason we have the first patch in the series is because we can't identify > the port as vxlan otherwise. It is identified as system. In that case, the > port > would not be removed, and that would cause the problems you see during > restarts > and when removing all ports for that vport. > > But it happens that if a netdev has been opened for that name and no type has > been used, things will break, as its type will be detected as system. > > So it happens that the route table opens it and keeps a reference when > inserting > it into the tnl port map. Not sure why it didn't happen to me in the past. It > could be explained by a change after a rebase, but I couldn't find what commit > would have broken it. > > Well, there are plenty of ways to fix it. One line of possible changes > involves > the routing table: prevent vports from being considered in tnl ports or the > route table, or preventing routes with only LLAs routes to go to the tnl > ports. > > The other line is making sure we will detect the correct type no matter what: > we can either always open such devices with their appropriate type, doing the > vport name to type mapping before open, or in netdev_get_type_from_name always > prefer the vport type before trying to find an existing netdev. > > I think the last solution would be best. First, any later changes on routing > table or in case anyone calls netdev_open for these names for any other reason > won't break it. Second, doing it on netdev_open would cause other changes, > like > the netdev_class used would be different, and even the route table code would > have problems, since get_addr is not supported for vport types.
I definitely agree that it is better to fix the netdev code rather than the routing table. Just trying to avoid this situation would almost certainly mean that it would break at some point in the future. However, I'm not sure that it's really a good idea to have the same device open as netdevs with different types (if I'm understanding your proposal correctly). I think we should try to ensure that we always have the right netdev class when we are dealing with devices, otherwise I suspect that we'll have weird corner cases in the future. If the routing table code isn't aware of some types of devices and doesn't handle them properly then that seems like an appropriate thing to fix within the routing code. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
