On Thu, Jun 16, 2016 at 03:53:12PM -0700, Jesse Gross wrote: > 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.
The routing code just receives notifications for all system interfaces and opens them up with the NULL type, which is the system type. In the specific case of vport types, they are not directly opened or shouldn't be by other code. For the purpose of the routing code, using a different type could even prevent it from using it as intended, that is, to get the address list. That said, I see the potential for problems the other way around, but I am not sure yet what is the best fix. We should warn or at least print a debug message when a netdev is opened with a different type than the one currently opened. I can see some potential ways to cause lots of confusion, like having a vxlan0 interface on the system (be it a vxlan interface or not) and create a vxlan0 port/interface with type=vxlan. Not sure how to fix that problem. In this specific patchset, I think the best way forward is fixing it in netdev_get_type_from_name for now. Cascardo. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
