On Fri, Nov 10, 2017 at 7:54 AM, Eric Garver <[email protected]> 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.
>
>>
>> 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?

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()?

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

Yes, we will need to address vxlan and gre later.

Thanks for the comments.
William
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to