All of lore.kernel.org
 help / color / mirror / Atom feed
* [iproute2] concern regarding the color option usage in "bridge" & "tc" cmd
@ 2021-08-29  9:49 Gokul Sivakumar
  2021-09-01  3:41 ` David Ahern
  0 siblings, 1 reply; 3+ messages in thread
From: Gokul Sivakumar @ 2021-08-29  9:49 UTC (permalink / raw)
  To: Stephen Hemminger, David Ahern; +Cc: netdev

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [iproute2] concern regarding the color option usage in "bridge" & "tc" cmd
  2021-08-29  9:49 [iproute2] concern regarding the color option usage in "bridge" & "tc" cmd Gokul Sivakumar
@ 2021-09-01  3:41 ` David Ahern
  2021-09-21 17:22   ` Gokul Sivakumar
  0 siblings, 1 reply; 3+ messages in thread
From: David Ahern @ 2021-09-01  3:41 UTC (permalink / raw)
  To: Gokul Sivakumar, Stephen Hemminger, David Ahern, Roopa Prabhu; +Cc: netdev

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [iproute2] concern regarding the color option usage in "bridge" & "tc" cmd
  2021-09-01  3:41 ` David Ahern
@ 2021-09-21 17:22   ` Gokul Sivakumar
  0 siblings, 0 replies; 3+ messages in thread
From: Gokul Sivakumar @ 2021-09-21 17:22 UTC (permalink / raw)
  To: David Ahern; +Cc: Stephen Hemminger, David Ahern, Roopa Prabhu, netdev

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-21 17:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-29  9:49 [iproute2] concern regarding the color option usage in "bridge" & "tc" cmd Gokul Sivakumar
2021-09-01  3:41 ` David Ahern
2021-09-21 17:22   ` Gokul Sivakumar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.