netdev - Re: [PATCH iproute2] vxlan: Add support for modifying vxlan device attributes

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] [day] [month] [year] [list]

Date:   Fri, 5 May 2017 09:47:03 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Girish Moodalbail <girish.moodalbail@...cle.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] vxlan: Add support for modifying vxlan device
 attributes

On Thu, 4 May 2017 17:26:23 -0700
Girish Moodalbail <girish.moodalbail@...cle.com> wrote:

> On 5/4/17 5:07 PM, Stephen Hemminger wrote:
> > On Thu,  4 May 2017 14:46:34 -0700
> > Girish Moodalbail <girish.moodalbail@...cle.com> wrote:
> >  
> >> Ability to change vxlan device attributes was added to kernel through
> >> commit 8bcdc4f3a20b ("vxlan: add changelink support"), however one
> >> cannot do the same through ip(8) command.  Changing the allowed vxlan
> >> device attributes using 'ip link set dev <vxlan_name> type vxlan
> >> <allowed_attributes>' currently fails with 'operation not supported'
> >> error.  This failure is due to the incorrect rtnetlink message
> >> construction for the 'ip link set' operation.
> >>
> >> The vxlan_parse_opt() callback function is called for parsing options
> >> for both 'ip link add' and 'ip link set'. For the 'add' case, we pass
> >> down default values for those attributes that were not provided as CLI
> >> options. However, for the 'set' case we should be only passing down the
> >> explicitly provided attributes and not any other (default) attributes.
> >>
> >> Signed-off-by: Girish Moodalbail <girish.moodalbail@...cle.com>
> >> ---  
> >
> > All these foo_set variables are ugly. This looks almost like machine
> > generated code. It doesn't read well.  
> 
> I thought about it, however I wasn't sure if refactoring that whole routine will 
> be well received so I decided to follow the current model that already existed 
> in iplink_vxlan.c. I will re-submit a patch cleaning up that whole routine.
> 
> thanks,
> ~Girish
> 

Thanks. This is one of those cases where something new gets added and additional
refactoring (that was overdue) is needed.