[iproute2] concern regarding the color option usage in "bridge" & "tc" cmd

[iproute2] concern regarding the color option usage in "bridge" & "tc" cmd

[Gokul Sivakumar]

Hi Stephen, David,

Recently I have added a commit 82149efee9 ("bridge: reorder cmd line arg parsing
to let "-c" detected as "color" option") in iproute2 tree bridge.c which aligns
the behaviour of the "bridge" cmd with the "bridge" man page description w.r.t
the color option usage. Now I have stumbled upon a commit f38e278b8446 ("bridge:
make -c match -compressvlans first instead of -color") that was added back in
2018 which says that "there are apps and network interface managers out there
that are already using -c to prepresent compressed vlans".

So after finding the commit f38e278b8446, now I think the man page should have
fixed instead of changing the bridge.c to align the behaviour of the "bridge" cmd
with the man page. Do you think we can revert the bridge.c changes 82149efee9,
so that the "bridge" cmd detects "-c" as "-compressedvlans" instead of "-color"?

If we are reverting the commit 82149efee9, then "-c" will be detected as
"-compressedvlans" and I will send out a patch to change the "bridge" man page
to reflect the new "bridge" cmd behaviour. If we are not reverting the commit
82149efee9, then "-c" will be detected as "-color" and I will send a out a patch
to change the "bridge" cmd help menu to reflect the current "bridge" cmd behaviour.
Please share your thoughts.

And also regarding the "tc" cmd, in the man/man8/tc.8 man page, the "-c" option
is mentioned to be used as a shorthand option for "-color", but instead it is
detected as "-conf". So here also, we need to decide between fixing the man page
and fixing the "tc" cmd behaviour w.r.t to color option usage.

I understand that "matches()" gives a lot of trouble and I see that you both are
now preferring full "strcmp()" over "matches()" for newly added cmd line options.

Thanks
Gokul

Re: [iproute2] concern regarding the color option usage in "bridge" & "tc" cmd

[David Ahern]

On 8/29/21 2:49 AM, Gokul Sivakumar wrote:
> Hi Stephen, David,
> 
> Recently I have added a commit 82149efee9 ("bridge: reorder cmd line arg parsing
> to let "-c" detected as "color" option") in iproute2 tree bridge.c which aligns
> the behaviour of the "bridge" cmd with the "bridge" man page description w.r.t
> the color option usage. Now I have stumbled upon a commit f38e278b8446 ("bridge:
> make -c match -compressvlans first instead of -color") that was added back in
> 2018 which says that "there are apps and network interface managers out there
> that are already using -c to prepresent compressed vlans".
> 
> So after finding the commit f38e278b8446, now I think the man page should have
> fixed instead of changing the bridge.c to align the behaviour of the "bridge" cmd
> with the man page. Do you think we can revert the bridge.c changes 82149efee9,
> so that the "bridge" cmd detects "-c" as "-compressedvlans" instead of "-color"?
> 
> If we are reverting the commit 82149efee9, then "-c" will be detected as
> "-compressedvlans" and I will send out a patch to change the "bridge" man page
> to reflect the new "bridge" cmd behaviour. If we are not reverting the commit
> 82149efee9, then "-c" will be detected as "-color" and I will send a out a patch
> to change the "bridge" cmd help menu to reflect the current "bridge" cmd behaviour.
> Please share your thoughts.
> 
> And also regarding the "tc" cmd, in the man/man8/tc.8 man page, the "-c" option
> is mentioned to be used as a shorthand option for "-color", but instead it is
> detected as "-conf". So here also, we need to decide between fixing the man page
> and fixing the "tc" cmd behaviour w.r.t to color option usage.
> 
> I understand that "matches()" gives a lot of trouble and I see that you both are
> now preferring full "strcmp()" over "matches()" for newly added cmd line options.
> 


Stephen: This should be reverted for 5.14 release given the change in
behavior. I will take a wild guess that ifupdown2 is the interface
manager that will notice.

Re: [iproute2] concern regarding the color option usage in "bridge" & "tc" cmd

[Gokul Sivakumar]

On Tue, Aug 31, 2021 at 08:41:01PM -0700, David Ahern wrote:
> On 8/29/21 2:49 AM, Gokul Sivakumar wrote:
> > Hi Stephen, David,
> > 
> > Recently I have added a commit 82149efee9 ("bridge: reorder cmd line arg parsing
> > to let "-c" detected as "color" option") in iproute2 tree bridge.c which aligns
> > the behaviour of the "bridge" cmd with the "bridge" man page description w.r.t
> > the color option usage. Now I have stumbled upon a commit f38e278b8446 ("bridge:
> > make -c match -compressvlans first instead of -color") that was added back in
> > 2018 which says that "there are apps and network interface managers out there
> > that are already using -c to prepresent compressed vlans".
> > 
> > So after finding the commit f38e278b8446, now I think the man page should have
> > fixed instead of changing the bridge.c to align the behaviour of the "bridge" cmd
> > with the man page. Do you think we can revert the bridge.c changes 82149efee9,
> > so that the "bridge" cmd detects "-c" as "-compressedvlans" instead of "-color"?
> > 
> > If we are reverting the commit 82149efee9, then "-c" will be detected as
> > "-compressedvlans" and I will send out a patch to change the "bridge" man page
> > to reflect the new "bridge" cmd behaviour. If we are not reverting the commit
> > 82149efee9, then "-c" will be detected as "-color" and I will send a out a patch
> > to change the "bridge" cmd help menu to reflect the current "bridge" cmd behaviour.
> > Please share your thoughts.
> > 
> > And also regarding the "tc" cmd, in the man/man8/tc.8 man page, the "-c" option
> > is mentioned to be used as a shorthand option for "-color", but instead it is
> > detected as "-conf". So here also, we need to decide between fixing the man page
> > and fixing the "tc" cmd behaviour w.r.t to color option usage.
> > 
> > I understand that "matches()" gives a lot of trouble and I see that you both are
> > now preferring full "strcmp()" over "matches()" for newly added cmd line options.
> > 
> 
> 
> Stephen: This should be reverted for 5.14 release given the change in
> behavior. I will take a wild guess that ifupdown2 is the interface
> manager that will notice.

Thanks David.

Hi Stephen, I see that the iproute2 5.14 got released before reverting the
commit 82149efee9. I would like to bring my concern regarding "the color
option usage" to your notice in case if you have missed this email thread
earlier.

Gokul