[ovs-dev] [PATCH v4] datapath: Add support for lwtunnel

[Jesse Gross]

On Wed, Nov 25, 2015 at 6:17 AM, Pravin Shelar <pshelar at nicira.com> wrote:
> On Wed, Nov 25, 2015 at 6:55 AM, Jesse Gross <jesse at kernel.org> wrote:
>> On Tue, Nov 24, 2015 at 2:40 PM, Joe Stringer <joe at ovn.org> wrote:
>>> On 23 November 2015 at 21:30, Pravin B Shelar <pshelar at 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 <pshelar at 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.
>>
> 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 mean the upstream ip_gre module. If it is loaded, it will cause OVS
module init to fail and conversely if OVS is already loaded (even
without any GRE tunnels) it will cause the ip_gre module to fail to
load.

>> 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.

What I was wondering about was whether we could try to detect
backported tunnel ports that are added to the datapath as normal
netdevices and redirect the send function.

However, maybe this isn't worth it for now. Userspace is still going
to need to be able to handle old upstream kernels using the compat
mechanism and I guess using that path for the out of tree module is
better than introducing a third one.

>> 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.

OK, that's fine.