[ovs-dev] [PATCH] datapath: compat: fix kernel module reference count.

[Eric Garver]

On Fri, Nov 10, 2017 at 05:36:25AM -0800, William Tu wrote:
> Before we introduce dpif-netlink-rtnl, creating a tunnel device
> depends on openvswitch/vport kernel module calling kernel's tunnel API.
> Example call sequence:
>  1) ovs_vport_add (openvswitch.ko)
>  2) geneve_create (vport_geneve.ko)
>  3) geneve_tnl_create (vport_geneve.ko)
>  4) geneve_dev_create_fb (geneve.ko)
> As a result, module dependency/refcnt is created: module openvswitch.ko
> references vport_geneve.ko and vport_geneve.ko depends on geneve.ko.
> 
> After the dpif-netlink-rtnl, a tunnel device can be created by using
> rtnetlink, so the creation of fb device comes from the ovs-vswitchd
> instead of going through OVS kernel modules.  This breaks the module
> dependency between 1) and 2).  As a result, when ovs-vswitchd is running,
> the 'lsmod' shows:
>   geneve                 27326  1 vport_geneve
>   vport_geneve           12560  0
>   openvswitch           172469  1 vport_geneve

Which is correct. vport_geneve is not using geneve. If anything is using
geneve, it's vport_netdev. But it already provides the proper linkage
via netdev_master_upper_dev_link().

> Problems can be cerated by doing  `rmmod vport_geneve` then
> `rmmod geneve`, when ovs-vswitchd expects geneve module should already
> be loaded.

Seems like this is not something that can happen by accident, but
requires the user to do something they shouldn't.

> 
> The patch introduces 1) load the upstream geneve.ko at init time
> from openvswitch.ko and 2) grap the reference count of geneve.
> This adds the dependency: openvswitch.ko needs geneve.ko, so that
> geneve won't be unloaded accidentally.  As a result, `lsmod` shows
>   geneve                 20254  2 vport_geneve
>   vport_geneve            1744  0
>   openvswitch           159846  1 vport_geneve
> 
> Note that when choosing to use upstream tunnel, we don't actually need
> vport_geneve, so the patch increment refcnt of geneve instead of
> vport_geneve.

I don't think inserting an artificial dependency is the right answer.
Although I'm not sure what problem you're trying to solve. Can you help
me understand the issue here?

This patch also only address GENEVE, what about VXLAN and GRE?

> 
> Fixes: b6d6830d29e4 ("dpif-netlink-rtnl: Support GENEVE creation")
> Signed-off-by: William Tu <u9012063 at gmail.com>
> Cc: Eric Garver <e at erig.me>
> ---
[...]