[ovs-dev] [PATCHv2] dpif-netlink-rtnl: Fix ovs_geneve probing after restart.
William Tu
u9012063 at gmail.com
Thu Nov 2 19:09:31 UTC 2017
On Thu, Nov 2, 2017 at 10:32 AM, Eric Garver <e at erig.me> wrote:
> On Wed, Nov 01, 2017 at 12:35:40PM -0700, William Tu wrote:
>> When using the out-of-tree (openvswitch compat) geneve module,
>> the first time oot tunnel probing returns true (correct).
>> Without unloading the geneve module, if the userspace ovs-vswitchd
>> restarts, because the 'geneve_sys_6081' still exists, the probing
>> incorrectly returns false and loads the in-tree (upstream kernel)
>> geneve module.
>>
>> The patch fixes it by querying the geneve device's kind when exists.
>> The out-of-tree modules uses kind string as 'ovs_geneve', while the
>> in-tree module uses 'geneve'. To reproduce the issue, start the ovs
>> > /etc/init.d/openvswitch-switch start
>> > creat a bridge and attach a geneve port using out-of-tree geneve
>> > /etc/init.d/openvswitch-switch restart
>>
>> Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides used interface")
>> Signed-off-by: William Tu <u9012063 at gmail.com>
>> Cc: Eric Garver <e at erig.me>
>> Cc: Gurucharan Shetty <guru at ovn.org>
>> ---
>> v1->v2:
>> Add detection of existing module, instead of unconditionally
>> remote it and create.
>> ---
>> lib/dpif-netlink-rtnl.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
>> index 0c32e7d8ccb4..b19b862d308b 100644
>> --- a/lib/dpif-netlink-rtnl.c
>> +++ b/lib/dpif-netlink-rtnl.c
>> @@ -440,6 +440,7 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
>>
>> error = netdev_open("ovs-system-probe", "geneve", &netdev);
>> if (!error) {
>> + struct ofpbuf *reply;
>> const struct netdev_tunnel_config *tnl_cfg;
>>
>> tnl_cfg = netdev_get_tunnel_config(netdev);
>> @@ -448,6 +449,36 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
>> }
>>
>> name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
>> +
>> + /* The geneve module exists when ovs-vswitchd crashes
>> + * and restarts, handle the case here.
>> + */
>> + error = dpif_netlink_rtnl_getlink(name, &reply);
>> + if (!error) {
>> +
>> + struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
>> + struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
>> + const char *kind;
>> +
>> + nl_policy_parse(reply, NLMSG_HDRLEN + sizeof(struct ifinfomsg),
>> + rtlink_policy, rtlink,
>> + ARRAY_SIZE(rtlink_policy));
>> + nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy,
>> + linkinfo, ARRAY_SIZE(linkinfo_policy));
>
> Should check the return code of these two.
>
>> + kind = nl_attr_get_string(linkinfo[IFLA_INFO_KIND]);
>> +
>> + if (!strcmp(kind, "ovs_geneve")) {
>> + out_of_tree = true;
>> + goto exit;
>> + } else if (!strcmp(kind, "geneve")) {
>> + out_of_tree = false;
>> + goto exit;
>> + } else {
>> + VLOG_ABORT("Geneve tunnel device %s with kind %s"
>> + " not supported", name, kind);
>> + }
>
> Need to free reply somewhere, ofpbuf_delete(reply)
>
>> + }
>> +
>> error = dpif_netlink_rtnl_create(tnl_cfg, name, OVS_VPORT_TYPE_GENEVE,
>> "ovs_geneve",
>> (NLM_F_REQUEST | NLM_F_ACK
>> @@ -458,6 +489,10 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
>> }
>> out_of_tree = true;
>> }
>> + }
>> +
>> +exit:
>> + if (!error) {
>> netdev_close(netdev);
>> }
>
> This is not right. netdev_open() may succeed, but
> dpif_netlink_rtnl_create() can fail. Which means netdev will not be
> freed here.
Hi Eric,
Thanks for the feedback. I've sent v3 patch.
William
More information about the dev
mailing list