From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hangbin Liu Subject: Re: [PATCH net-next] geneve: fix ttl inherit type Date: Sat, 29 Sep 2018 23:03:23 +0800 Message-ID: <20180929150323.GJ24677@leo.usersys.redhat.com> References: <1538096998-20937-1-git-send-email-liuhangbin@gmail.com> <20180928234619.GA17323@unicorn.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, David Miller , Stephen Hemminger , David Ahern , Phil Sutter To: Michal Kubecek Return-path: Received: from mail-pf1-f196.google.com ([209.85.210.196]:34359 "EHLO mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728278AbeI2VcT (ORCPT ); Sat, 29 Sep 2018 17:32:19 -0400 Received: by mail-pf1-f196.google.com with SMTP id k19-v6so6260174pfi.1 for ; Sat, 29 Sep 2018 08:03:34 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20180928234619.GA17323@unicorn.suse.cz> Sender: netdev-owner@vger.kernel.org List-ID: 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