On Tue, Nov 24, 2015 at 2:40 PM, Joe Stringer <j...@ovn.org> wrote: > On 23 November 2015 at 21:30, Pravin B Shelar <pshe...@nicira.com> wrote: >> Following patch adds support for lwtunnel to OVS datapath. >> With this change OVS datapath detect lwtunnel support and >> make use of new APIs if available. On older kernel where the >> support is not there the backported tunnel modules are used. >> These backported tunnel devices acts as lwtunnel devices. >> I tried to keep backported module same as upstream for easier >> bug-fix backport. Since STT and LISP are not upstream OVS >> always needs to use respective modules from tunnel compat layer. >> To make it work on kernel 4.3 I have converted STT and LISP >> modules to lwtunnel API model. >> >> lwtunnel make use of skb-dst to pass tunnel information to the >> tunnel module. On older kernel this is not possible. So the in >> case of old kernel metadata ref is stored in OVS_CB and direct >> call to tunnel transmit function is made by respective tunnel >> vport modules. Similarly on receive side tunnel recv directly >> call netdev-vport-receive to pass the skb to OVS. >> >> Major backported components include: >> Geneve, GRE, VXLAN, ip_tunnel, udp-tunnels GRO. >> >> Signed-off-by: Pravin B Shelar <pshe...@nicira.com> > > Jesse, I've provided some minor feedback below but overall it looks OK > to me. Did you have any thoughts on this?
I'm basically happy with this. It seems more or less impossible to review this in detail on every kernel but I looked at the overall structure and some key areas and it seems generally reasonable. Given that Joe did some compile testing on different kernels, I think that's good enough and we can work out any problems as we encounter them. One specific thing that I noticed is that I believe that there is a regression in regards to compatibility with GRE devices outside of OVS. We used to not register the compat GRE protocol handler until we actually created a GRE tunnel but now we do it immediately on OVS kernel load, which will block the upstream module from loading. I think that we need to reinstate the offloaded vlan checks for tunnels, since we don't have a real device layer to handle them here. Does this work if userspace adds tunnel ports to the datapath without going through the compat code? It seems like creation, receive, etc. should all be fine but transmit might be a problem since it will just use the normal dev_queue_xmit() instead of the specified transmit routine. Otherwise, I think the only accommodation that userspace needs to make for using the compat code is to try to create the device using the ovs_* type over RTNL. Most other things should work with the exception of tcpdump, offloading settings, and up/down (of those, up/down seems the most feasible, maybe we should try to make it work). Another general question that came to mind is whether it makes sense to have separate kernel modules for each device type (as they are upstream) vs. bundling them all into the OVS module. I guess bundling everything together avoids having to export interfaces that are really for compatibility but is there any other reason to do it this way? Finally, as Joe mentioned, there are some additional commits in 4.3 on top of this (besides the conntracking patches). One that I noticed is the egress tunnel info fixes, there are possibly others. I don't want to add more code to this patch but we should do a sweep of the change log as a follow up. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
