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

[William Tu]

On Mon, Nov 13, 2017 at 5:01 AM, Pravin Shelar <pshelar at ovn.org> wrote:
> On Sat, Nov 11, 2017 at 11:53 PM, Guru Shetty <guru at ovn.org> wrote:
>> On 10 November 2017 at 22:54, Gregory Rose <gvrose8192 at gmail.com> wrote:
>>
>>> On 11/11/2017 9:57 AM, William Tu wrote:
>>>
>>>> yes, this is an artificial dependency. Another way I'm thinking is for
>>>>>> ovs-vswitchd
>>>>>>   to hold the geneve.ko dependency instead of openvswitch.ko, when user
>>>>>> creates
>>>>>> a geneve device. Is there a way to do that through rtnetlink or at
>>>>>> dpif_netlink_rtnl_create()?
>>>>>>
>>>>> It should be vport-netdev doing the magic. It creates the linkage with
>>>>> netdev_master_upper_dev_link(), but apparently that is not enough to
>>>>> bump the kernel module refcnt and thus prevent unloading. Maybe you can
>>>>> dig into why that is. AFAICS, it should be linking openvswitch.ko and
>>>>> and geneve.ko with that call.
>>>>>
>>>> Sure, will take a look.
>>>>
>>>
>>> I'm extremely uncomfortable with this approach of manually bumping a
>>> driver refcnt.
>>> I worry we're going to get into a situation where the driver won't
>>> *unload*.  In a properly
>>> configured system this shouldn't be necessary.  Driver dependencies
>>> shouldn't have to
>>> be manually configured.
>>>
>> Hypothetically, let us assume that we use only upstream "geneve". Once we
>> create tunnel via it, it shouldn't be possible to remove it. But right now,
>> we can. And I think that is wrong.
>>
>
> That is always been behavior of the port lifecycle. If you remove the
> driver of a device, port is removed from kernel datapath and the
> device is deleted from kernel networking stack. For example you can
> remove intel NIC driver, or veth module even if there is a
> corresponding port in OVS bridge. I am not sure why tunnel device is
> special case.

agree. in this case we allow the driver to be unloaded and since openvswitch.ko
registers the NETDEV_UNREGISTER notifier, the ovs-vswtichd gets notified
that the port is removed.

I also check Eric's suggestion about the upper dev approach
(netdev_master_upper_dev_link) and although the tunnel port is the lower dev,
having an upper device does not prevent us from uploading the lower
device's driver.

Since this is triggered by upgrading OVS using ovs-ctl force-reload-kmod,
Guru has submitted another approach:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/340835.html

Regards,
William