netdev - Re: [PATCH net-next] geneve: fix ttl inherit type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180929150323.GJ24677@leo.usersys.redhat.com>
Date:   Sat, 29 Sep 2018 23:03:23 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
        Stephen Hemminger <stephen@...workplumber.org>,
        David Ahern <dsahern@...il.com>, Phil Sutter <phil@....cc>
Subject: Re: [PATCH net-next] geneve: fix ttl inherit type

On Sat, Sep 29, 2018 at 01:46:19AM +0200, Michal Kubecek wrote:
> Is it desirable to switch to a flag? If I read geneve_changelink() and
> geneve_nl2info() correctly, it allows you to set the ttl_inherit flag
> for an existing device but doesn't allow you to clear it. With NLA_U8,
> you could distinguish three cases: set the flag (non-zero value), clear
> the flag (zero value) and preserve current state (attribute not
> present).
> 
> The same problem exists for vxlan but vxlan code intentionally disallows
> changing the flag value for an existing device (I'm not sure if it's
> because it's really impossible or just due to limits of the interface).
> Unfortunately it has been already released with NLA_FLAG in 4.18,
> AFAICS, so we have to live with it. But it's not too late for geneve.
> 
> Michal Kubecek

Hi michal,

I thought about the vxlan issue and agree with you. TTL inherit is a way
to define the ttl number we should use. It also should be able to be changed
as the normal ttl. How about enabling clear ttl inherit flag like:

--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3303,13 +3303,11 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
        if (data[IFLA_VXLAN_TOS])
                conf->tos  = nla_get_u8(data[IFLA_VXLAN_TOS]);

-       if (data[IFLA_VXLAN_TTL])
-               conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
-
        if (data[IFLA_VXLAN_TTL_INHERIT]) {
-               if (changelink)
-                       return -EOPNOTSUPP;
                conf->flags |= VXLAN_F_TTL_INHERIT;
+       } else if (data[IFLA_VXLAN_TTL]) {
+               conf->flags &= ~VXLAN_F_TTL_INHERIT;
+               conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
        }


Before this fix, we disabled changing it after creating vxlan. And with this fix
we can set/unset it. I think this should not be a usage break. What do you think?

Thanks
Hangbin