From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5FE9CC43381 for ; Thu, 14 Feb 2019 11:10:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 304F420838 for ; Thu, 14 Feb 2019 11:10:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728000AbfBNLKI (ORCPT ); Thu, 14 Feb 2019 06:10:08 -0500 Received: from mail-wm1-f65.google.com ([209.85.128.65]:56101 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727330AbfBNLKI (ORCPT ); Thu, 14 Feb 2019 06:10:08 -0500 Received: by mail-wm1-f65.google.com with SMTP id r17so5775519wmh.5 for ; Thu, 14 Feb 2019 03:10:07 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=FODdj7bYc5Te5uB8rvGJsaKzJPoUPDEBmvYdSy0QOtc=; b=f9YCpm+Yzq78htkjK3kJ4OnbCVm+3zp1S4IjMOmR4QrZqsW1TafFoDdXt7cbP/ZqEX TYgDmt+cL+HQLDOT40DIW3gYfKmTe+L+vIxGZe1KSKD5zAMKsqNzWmx1tFUfmhctSEZ2 ZKce3re/h+djNdtq3Cm3u/CcdzMLUNzkG9sEEDCaGoautprewW1ixNhE57o74SO7sR9V ob9f1ik1nlhvrgWklvRmqaSLGSEhk+jXHcrJ9951ls1gtQpo715sqXJlnp3tKYfUjC7P 5dDnRdCB7xNPpFZNfMTPaqxsp5jm1vCiTAIhNNoVhJ7srxzR+Js0E+H6Sx4Y8SSUX+mD 0cDg== X-Gm-Message-State: AHQUAubuKbLGToV4b3Ldey8nBtvNmTTlu6fcUa78khsseHVixgVsd/Bq 5aMNDApgRSICb4IPabCIaYMcEw== X-Google-Smtp-Source: AHgI3IYfNnFHjyySLMpBZZlGQpSAcDyomkBAxyARrwNtJRY/Gqg56RHoSm0wiUn7AL6cv7kFZBtAvw== X-Received: by 2002:a7b:c08a:: with SMTP id r10mr2218765wmh.112.1550142606413; Thu, 14 Feb 2019 03:10:06 -0800 (PST) Received: from localhost.localdomain (nat-pool-mxp-t.redhat.com. [149.6.153.186]) by smtp.gmail.com with ESMTPSA id i13sm3598373wrm.86.2019.02.14.03.10.05 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 14 Feb 2019 03:10:05 -0800 (PST) Date: Thu, 14 Feb 2019 12:10:03 +0100 From: Lorenzo Bianconi To: Petr Machata Cc: "netdev@vger.kernel.org" , "davem@davemloft.net" , "kuznet@ms2.inr.ac.ru" , "yoshfuji@linux-ipv6.org" , lucien.xin@gmail.com Subject: Re: [PATCH net-next] net: ip6_gre: Give ERSPAN a fill_info link op of its own Message-ID: <20190214111002.GB16752@localhost.localdomain> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org > In commit c706863bc890 ("net: ip6_gre: always reports o_key to > userspace"), ip6gre and ip6gretap tunnels started reporting a TUNNEL_KEY > output flag even if one was not configured at the device. > > When an okey-less ip6gre or ip6gretap netdevice is created, it initially > encapsulates the packets without okey. But any configuration change > (even a non-change such as setting TOS to an already-configured value) > then causes the okey flag from the reported configuration to be > circulated back to actual configuration. From that point on, the device > encapsulates packets with output key of 0. > > The intention was to implement this behavior for ERSPAN devices, not for > all ip6gre devices. The ERSPAN netdevice should really have its own > fill_info callback. Add one. Hi Petr, I was assuming erspan_ver is set just for erspan tunnels. In particular I guess the issue is due to the default erspan_ver configuration done in ip6gre_netlink_parms (commit 84581bdae9587). What about adding a routine to set erspan_ver and moving it in ip6erspan_newlink/ip6erspan_changelink? In this way erspan_ver will be defined just for erspan tunnels. Moreover do we have a similar issue for IFLA_GRE_ERSPAN_INDEX in ip6gre_fill_info? Something like: diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index 65a4f96dc462..bb525abd860e 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -1719,6 +1719,24 @@ static int ip6erspan_tap_validate(struct nlattr *tb[], struct nlattr *data[], return 0; } +static void ip6erspan_set_version(struct nlattr *data[], + struct __ip6_tnl_parm *parms) +{ + parms->erspan_ver = 1; + if (data[IFLA_GRE_ERSPAN_VER]) + parms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]); + + if (parms->erspan_ver == 1) { + if (data[IFLA_GRE_ERSPAN_INDEX]) + parms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]); + } else if (parms->erspan_ver == 2) { + if (data[IFLA_GRE_ERSPAN_DIR]) + parms->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]); + if (data[IFLA_GRE_ERSPAN_HWID]) + parms->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]); + } +} + static void ip6gre_netlink_parms(struct nlattr *data[], struct __ip6_tnl_parm *parms) { @@ -1767,20 +1785,6 @@ static void ip6gre_netlink_parms(struct nlattr *data[], if (data[IFLA_GRE_COLLECT_METADATA]) parms->collect_md = true; - - parms->erspan_ver = 1; - if (data[IFLA_GRE_ERSPAN_VER]) - parms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]); - - if (parms->erspan_ver == 1) { - if (data[IFLA_GRE_ERSPAN_INDEX]) - parms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]); - } else if (parms->erspan_ver == 2) { - if (data[IFLA_GRE_ERSPAN_DIR]) - parms->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]); - if (data[IFLA_GRE_ERSPAN_HWID]) - parms->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]); - } } static int ip6gre_tap_init(struct net_device *dev) @@ -2203,6 +2207,7 @@ static int ip6erspan_newlink(struct net *src_net, struct net_device *dev, int err; ip6gre_netlink_parms(data, &nt->parms); + ip6erspan_set_version(data, &nt->parms); ign = net_generic(net, ip6gre_net_id); if (nt->parms.collect_md) { @@ -2248,6 +2253,7 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[], if (IS_ERR(t)) return PTR_ERR(t); + ip6erspan_set_version(data, &p); ip6gre_tunnel_unlink_md(ign, t); ip6gre_tunnel_unlink(ign, t); ip6erspan_tnl_change(t, &p, !tb[IFLA_MTU]); Does it fix reported issue? Regards, Lorenzo > > Fixes: c706863bc890 ("net: ip6_gre: always reports o_key to userspace") > CC: Lorenzo Bianconi > Signed-off-by: Petr Machata > --- > net/ipv6/ip6_gre.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c > index 65a4f96dc462..0a6087cffe54 100644 > --- a/net/ipv6/ip6_gre.c > +++ b/net/ipv6/ip6_gre.c > @@ -2094,15 +2094,13 @@ static size_t ip6gre_get_size(const struct net_device *dev) > 0; > } > > -static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev) > +static int __ip6gre_fill_info(struct sk_buff *skb, > + const struct net_device *dev, > + __be16 base_o_flags) > { > struct ip6_tnl *t = netdev_priv(dev); > struct __ip6_tnl_parm *p = &t->parms; > - __be16 o_flags = p->o_flags; > - > - if ((p->erspan_ver == 1 || p->erspan_ver == 2) && > - !p->collect_md) > - o_flags |= TUNNEL_KEY; > + __be16 o_flags = p->o_flags | base_o_flags; > > if (nla_put_u32(skb, IFLA_GRE_LINK, p->link) || > nla_put_be16(skb, IFLA_GRE_IFLAGS, > @@ -2155,6 +2153,11 @@ static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev) > return -EMSGSIZE; > } > > +static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev) > +{ > + return __ip6gre_fill_info(skb, dev, 0); > +} > + > static const struct nla_policy ip6gre_policy[IFLA_GRE_MAX + 1] = { > [IFLA_GRE_LINK] = { .type = NLA_U32 }, > [IFLA_GRE_IFLAGS] = { .type = NLA_U16 }, > @@ -2256,6 +2259,20 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[], > return 0; > } > > +static int ip6erspan_fill_info(struct sk_buff *skb, > + const struct net_device *dev) > +{ > + struct ip6_tnl *t = netdev_priv(dev); > + struct __ip6_tnl_parm *p = &t->parms; > + __be16 base_o_flags = 0; > + > + if ((p->erspan_ver == 1 || p->erspan_ver == 2) && > + !p->collect_md) > + base_o_flags |= TUNNEL_KEY; > + > + return __ip6gre_fill_info(skb, dev, base_o_flags); > +} > + > static struct rtnl_link_ops ip6gre_link_ops __read_mostly = { > .kind = "ip6gre", > .maxtype = IFLA_GRE_MAX, > @@ -2295,7 +2312,7 @@ static struct rtnl_link_ops ip6erspan_tap_ops __read_mostly = { > .newlink = ip6erspan_newlink, > .changelink = ip6erspan_changelink, > .get_size = ip6gre_get_size, > - .fill_info = ip6gre_fill_info, > + .fill_info = ip6erspan_fill_info, > .get_link_net = ip6_tnl_get_link_net, > }; > > -- > 2.4.11 >