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

[Hangbin Liu <liuhangbin@gmail.com>]

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