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

[Guru Shetty]

On 10 November 2017 at 07:54, Eric Garver <e at erig.me> wrote:

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

This was observed in a real world case.
1. Take a RHEL7.3 host
1. Upgrade OVS RHEL packages from 2.7 to 2.8 (including kernel module
package).
2. Upgrade RHEL7.3 to RHEL7.4

When iptables gets upgraded in step 3, it calls:
/usr/libexec/iptables/ip6tables.init stop

That script tries to remove all kernel modules that are dependent on
conntrack. So it tries to remove openvswitch and vport-geneve. openvswitch
does not get removed as it has a reference count. But a 'modprobe -r
vport-geneve' will remove "geneve", which in turn deletes the geneve device
leaving the tunnels down.



> >
> > 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>
> > ---
> [...]
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>