On Thu, Apr 21, 2016 at 9:43 AM, Thadeu Lima de Souza Cascardo <casca...@redhat.com> wrote: > On Wed, Apr 20, 2016 at 11:38:31AM -0700, Jesse Gross wrote: >> One minor comment that I noticed on the patch itself - I don't know if >> the port mapping functions are handling IPsec variants of tunnels >> correctly in all situations (or maybe are handling them by accident). >> Since both IPsec and non-IPsec ports share the same dpif_port name, we >> might get the wrong class back and then it seems like we don't handle >> the IPsec class types when converting between names and types, which >> could itself be an independent bug. Can you take a look? > > Sure, I am not too familiar with the ipsec code. Looking at it, I don't see > anything really special about it. It just requires that some configuration is > available and that the ovs-monitor-ipsec daemon is running. When adding and > removing devices, the same procedure would still work. So, I don't see why we > would need to handle it specially, do you?
Nevermind, I just noticed that netdev_to_ovs_vport_type() handles GRE differently from the other tunnel types to account for the "ipsec_" prefix in the class name. I was concerned that we wouldn't properly translate IPsec netdev types into the Linux OVS vport type but that doesn't appear to be the case. > On the other hand, I noticed that VXLAN-GBP tunnels will have the problem of > sharing the same dpif_port name with non GBP tunnels. That means that tunnels > using different remote IPs with the same port all must be GBP or not GBP. If > we > used extensions for GPE, that would also apply there. Maybe we should just add > an infix gbp and gpe, resulting in vxlan_sys_gbp_4789 and vxlan_sys_gpe_4789. > How about that? I agree that this is a problem. Different extensions will need to run on different UDP port numbers since there's no way to otherwise know how to interpret the bits when the packet is first received. I think we need to block differing extensions on the same UDP port when the VXLAN ports are added to OVS, otherwise they'll silently be combined onto the same backer. Your idea of adding a tag indicating the extension type is nice for debugging (though you'll need to find a way to abbreviate the name so that it fits within the interface name length) but it's not strictly necessary: if extensions are running on different UDP ports then the backers will already be distinguished and if they are running on the same port then attempting to add the second one will silently fail at the kernel level. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
