netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
To: Benjamin Poirier <bpoirier@cumulusnetworks.com>,
	Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH iproute2 1/7] bridge: Use the same flag names in input and output
Date: Thu, 30 Apr 2020 03:58:51 +0300	[thread overview]
Message-ID: <a2fc6935-555f-b881-1ff2-f824bd08d6ae@cumulusnetworks.com> (raw)
In-Reply-To: <20200430002216.GA76853@f3>

On 4/30/20 3:22 AM, Benjamin Poirier wrote:
> On 2020-04-29 08:12 -0700, Roopa Prabhu wrote:
>> On Mon, Apr 27, 2020 at 4:51 PM Benjamin Poirier
>> <bpoirier@cumulusnetworks.com> wrote:
>>>
>>> Output the same names for vlan flags as the ones accepted in command input.
>>>
>>> Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
>>> ---
>>
>> Benjamin, It's a good change,  but this will break existing users ?.
> 
> Nikolay voiced the same concern. The current output looks like
> 
> ben@f3:~$ bridge vlan
> port    vlan ids
> br0     None
> tap0     1 PVID Egress Untagged
> 
> tap1     1 PVID Egress Untagged
> 
> docker0  1 PVID Egress Untagged
> 
> ben@f3:~$
> 
> "PVID Egress Untagged" look like 3 flags to me. Anything we can do to
> improve it?
> 

Put a "," after PVID ? :-)
The bigger problem is that "Egress Untagged" is also used as a flag in the json output.
Anyone parsing that and looking at the flags would be broken. In addition
this has been described in many of HowTos and docs over the years.

I'd just drop this change.

Thanks,
  Nik


  reply	other threads:[~2020-04-30  0:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-27 23:50 [PATCH iproute2 0/7] bridge vlan output fixes Benjamin Poirier
2020-04-27 23:50 ` [PATCH iproute2 1/7] bridge: Use the same flag names in input and output Benjamin Poirier
2020-04-29 15:12   ` Roopa Prabhu
2020-04-30  0:22     ` Benjamin Poirier
2020-04-30  0:58       ` Nikolay Aleksandrov [this message]
2020-04-27 23:50 ` [PATCH iproute2 2/7] bridge: Use consistent column names in vlan output Benjamin Poirier
2020-04-27 23:50 ` [PATCH iproute2 3/7] bridge: Fix typo Benjamin Poirier
2020-04-27 23:50 ` [PATCH iproute2 4/7] bridge: Fix output with empty vlan lists Benjamin Poirier
2020-04-27 23:50 ` [PATCH iproute2 5/7] json_print: Return number of characters printed Benjamin Poirier
2020-04-27 23:50 ` [PATCH iproute2 6/7] bridge: Align output columns Benjamin Poirier
2020-04-27 23:50 ` [PATCH iproute2 7/7] Replace open-coded instances of print_nl() Benjamin Poirier
2020-04-30  5:35 ` [PATCH iproute2 0/7] bridge vlan output fixes Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a2fc6935-555f-b881-1ff2-f824bd08d6ae@cumulusnetworks.com \
    --to=nikolay@cumulusnetworks.com \
    --cc=bpoirier@cumulusnetworks.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).