On Wed, Nov 25, 2015 at 6:55 AM, Jesse Gross <[email protected]> wrote: > On Tue, Nov 24, 2015 at 2:40 PM, Joe Stringer <[email protected]> wrote: >> On 23 November 2015 at 21:30, Pravin B Shelar <[email protected]> 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 <[email protected]> >> >> 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. > Which upstream module are you referring to? In the patch gre cisco protocol is registered which only blocks ovs vport-gre module. I do not think we can insmod upstream OVS and out of tree OVS modules at same time anyways.
> 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. > ok. > 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). > I can only enable it for kernel which support metadata-dst. I have fixed in the patch. In case of older kernel, I can not trust the skb-cb. > 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? > I am not sure if I can make it as module. Since few symbols depends on OVS and OVS also needs symbols from individual tunnel modules. There is circular dependency between there modules. > 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. sure I am planing on doing it. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
