* [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
@ 2015-01-26 2:26 roopa
2015-01-26 4:35 ` Scott Feldman
0 siblings, 1 reply; 6+ messages in thread
From: roopa @ 2015-01-26 2:26 UTC (permalink / raw)
To: netdev, shemminger, vyasevic; +Cc: wkok, sfeldma
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Introduce new option -c[ompressvlans] to request
vlan ranges from kernel
(pls suggest better option names if this does not look ok)
$bridge vlan show
port vlan ids
dummy0 1 PVID Egress Untagged
dummy1 1 PVID Egress Untagged
2
3
4
5
6
7
9
10
12
br0 1 PVID Egress Untagged
$bridge help
Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }
where OBJECT := { link | fdb | mdb | vlan | monitor }
OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |
-o[neline] | -t[imestamp] | -n[etns] name |
-c[ompressvlans] }
$bridge -c vlan show
port vlan ids
dummy0 1 PVID Egress Untagged
dummy1 1 PVID Egress Untagged
2-7
9-10
12
br0 1 PVID Egress Untagged
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
bridge/br_common.h | 1 +
bridge/bridge.c | 6 +++++-
bridge/vlan.c | 11 +++++++++--
include/linux/rtnetlink.h | 1 +
4 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/bridge/br_common.h b/bridge/br_common.h
index 12fce3e..169a162 100644
--- a/bridge/br_common.h
+++ b/bridge/br_common.h
@@ -16,4 +16,5 @@ extern int preferred_family;
extern int show_stats;
extern int show_details;
extern int timestamp;
+extern int compress_vlans;
extern struct rtnl_handle rth;
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 5fcc552..88469ca 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -21,6 +21,7 @@ int resolve_hosts;
int oneline = 0;
int show_stats;
int show_details;
+int compress_vlans;
int timestamp;
char * _SL_ = NULL;
@@ -32,7 +33,8 @@ static void usage(void)
"Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
"where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
" OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
-" -o[neline] | -t[imestamp] | -n[etns] name }\n");
+" -o[neline] | -t[imestamp] | -n[etns] name |\n"
+" -c[ompressvlans] }\n");
exit(-1);
}
@@ -117,6 +119,8 @@ main(int argc, char **argv)
NEXT_ARG();
if (netns_switch(argv[1]))
exit(-1);
+ } else if (matches(opt, "-compressvlans") == 0) {
+ ++compress_vlans;
} else {
fprintf(stderr, "Option \"%s\" is unknown, try \"bridge help\".\n", opt);
exit(-1);
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 88992e6..9f6c84e 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -182,7 +182,12 @@ static int print_vlan(const struct sockaddr_nl *who,
continue;
vinfo = RTA_DATA(i);
- fprintf(fp, "\t %hu", vinfo->vid);
+ if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END)
+ fprintf(fp, "-%hu", vinfo->vid);
+ else
+ fprintf(fp, "\t %hu", vinfo->vid);
+ if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN)
+ continue;
if (vinfo->flags & BRIDGE_VLAN_INFO_PVID)
fprintf(fp, " PVID");
if (vinfo->flags & BRIDGE_VLAN_INFO_UNTAGGED)
@@ -218,7 +223,9 @@ static int vlan_show(int argc, char **argv)
}
if (rtnl_wilddump_req_filter(&rth, PF_BRIDGE, RTM_GETLINK,
- RTEXT_FILTER_BRVLAN) < 0) {
+ (compress_vlans ?
+ RTEXT_FILTER_BRVLAN_COMPRESSED :
+ RTEXT_FILTER_BRVLAN)) < 0) {
perror("Cannont send dump request");
exit(1);
}
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 9aa5c2f..19eadb1 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -632,6 +632,7 @@ struct tcamsg {
/* New extended info filters for IFLA_EXT_MASK */
#define RTEXT_FILTER_VF (1 << 0)
#define RTEXT_FILTER_BRVLAN (1 << 1)
+#define RTEXT_FILTER_BRVLAN_COMPRESSED (1 << 2)
/* End of information exported to user level */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
2015-01-26 2:26 [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges roopa
@ 2015-01-26 4:35 ` Scott Feldman
2015-01-26 5:18 ` roopa
0 siblings, 1 reply; 6+ messages in thread
From: Scott Feldman @ 2015-01-26 4:35 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
> diff --git a/bridge/bridge.c b/bridge/bridge.c
> index 5fcc552..88469ca 100644
> --- a/bridge/bridge.c
> +++ b/bridge/bridge.c
> @@ -21,6 +21,7 @@ int resolve_hosts;
> int oneline = 0;
> int show_stats;
> int show_details;
> +int compress_vlans;
> int timestamp;
> char * _SL_ = NULL;
>
> @@ -32,7 +33,8 @@ static void usage(void)
> "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
> "where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
> " OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
> -" -o[neline] | -t[imestamp] | -n[etns] name }\n");
> +" -o[neline] | -t[imestamp] | -n[etns] name |\n"
> +" -c[ompressvlans] }\n");
This option is awkward at this level since it's specific to bridge vlan cmd.
Is it even necessary? The real user of compressed vlans isn't the
"bridge show" cmd; just keep "bridge show" simple, listing each vlan
per line.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
2015-01-26 4:35 ` Scott Feldman
@ 2015-01-26 5:18 ` roopa
2015-01-26 5:57 ` Scott Feldman
0 siblings, 1 reply; 6+ messages in thread
From: roopa @ 2015-01-26 5:18 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On 1/25/15, 8:35 PM, Scott Feldman wrote:
>> diff --git a/bridge/bridge.c b/bridge/bridge.c
>> index 5fcc552..88469ca 100644
>> --- a/bridge/bridge.c
>> +++ b/bridge/bridge.c
>> @@ -21,6 +21,7 @@ int resolve_hosts;
>> int oneline = 0;
>> int show_stats;
>> int show_details;
>> +int compress_vlans;
>> int timestamp;
>> char * _SL_ = NULL;
>>
>> @@ -32,7 +33,8 @@ static void usage(void)
>> "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>> "where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>> " OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
>> -" -o[neline] | -t[imestamp] | -n[etns] name }\n");
>> +" -o[neline] | -t[imestamp] | -n[etns] name |\n"
>> +" -c[ompressvlans] }\n");
> This option is awkward at this level since it's specific to bridge vlan cmd.
>
> Is it even necessary? The real user of compressed vlans isn't the
> "bridge show" cmd; just keep "bridge show" simple, listing each vlan
> per line.
without ranges it does get a bit difficult (or rather long) to read if
there are many vlans (it was also a comment I received on v2).
All the available options whether they apply to the immediate
sub-command or not are at the toplevel command today.
Hence the position (Not a big fan of the current name. suggestions welcome).
I would have loved to have the default output print ranges, If that is
acceptable. But, that can break new iproute2 on older kernels (If that
is something we should care about).
other option is keyword argument like maybe 'bridge vlan show
compressed' or 'bridge vlan show ranges'
Thanks,
Roopa
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
2015-01-26 5:18 ` roopa
@ 2015-01-26 5:57 ` Scott Feldman
2015-01-26 6:09 ` roopa
0 siblings, 1 reply; 6+ messages in thread
From: Scott Feldman @ 2015-01-26 5:57 UTC (permalink / raw)
To: roopa; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On Sun, Jan 25, 2015 at 9:18 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 1/25/15, 8:35 PM, Scott Feldman wrote:
>>>
>>> diff --git a/bridge/bridge.c b/bridge/bridge.c
>>> index 5fcc552..88469ca 100644
>>> --- a/bridge/bridge.c
>>> +++ b/bridge/bridge.c
>>> @@ -21,6 +21,7 @@ int resolve_hosts;
>>> int oneline = 0;
>>> int show_stats;
>>> int show_details;
>>> +int compress_vlans;
>>> int timestamp;
>>> char * _SL_ = NULL;
>>>
>>> @@ -32,7 +33,8 @@ static void usage(void)
>>> "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>>> "where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>>> " OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
>>> -" -o[neline] | -t[imestamp] | -n[etns] name }\n");
>>> +" -o[neline] | -t[imestamp] | -n[etns] name |\n"
>>> +" -c[ompressvlans] }\n");
>>
>> This option is awkward at this level since it's specific to bridge vlan
>> cmd.
>>
>> Is it even necessary? The real user of compressed vlans isn't the
>> "bridge show" cmd; just keep "bridge show" simple, listing each vlan
>> per line.
>
> without ranges it does get a bit difficult (or rather long) to read if there
> are many vlans (it was also a comment I received on v2).
>
> All the available options whether they apply to the immediate sub-command or
> not are at the toplevel command today.
> Hence the position (Not a big fan of the current name. suggestions welcome).
>
> I would have loved to have the default output print ranges, If that is
> acceptable. But, that can break new iproute2 on older kernels (If that is
> something we should care about).
>
> other option is keyword argument like maybe 'bridge vlan show compressed' or
> 'bridge vlan show ranges'
How about try RTEXT_FILTER_BRVLAN_COMPRESSED first, and if that fails,
try RTEXT_FILTER_BRVLAN, and don't add -c option?
-scott
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
2015-01-26 5:57 ` Scott Feldman
@ 2015-01-26 6:09 ` roopa
2015-01-26 18:57 ` roopa
0 siblings, 1 reply; 6+ messages in thread
From: roopa @ 2015-01-26 6:09 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On 1/25/15, 9:57 PM, Scott Feldman wrote:
> On Sun, Jan 25, 2015 at 9:18 PM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 1/25/15, 8:35 PM, Scott Feldman wrote:
>>>> diff --git a/bridge/bridge.c b/bridge/bridge.c
>>>> index 5fcc552..88469ca 100644
>>>> --- a/bridge/bridge.c
>>>> +++ b/bridge/bridge.c
>>>> @@ -21,6 +21,7 @@ int resolve_hosts;
>>>> int oneline = 0;
>>>> int show_stats;
>>>> int show_details;
>>>> +int compress_vlans;
>>>> int timestamp;
>>>> char * _SL_ = NULL;
>>>>
>>>> @@ -32,7 +33,8 @@ static void usage(void)
>>>> "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>>>> "where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>>>> " OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
>>>> -" -o[neline] | -t[imestamp] | -n[etns] name }\n");
>>>> +" -o[neline] | -t[imestamp] | -n[etns] name |\n"
>>>> +" -c[ompressvlans] }\n");
>>> This option is awkward at this level since it's specific to bridge vlan
>>> cmd.
>>>
>>> Is it even necessary? The real user of compressed vlans isn't the
>>> "bridge show" cmd; just keep "bridge show" simple, listing each vlan
>>> per line.
>> without ranges it does get a bit difficult (or rather long) to read if there
>> are many vlans (it was also a comment I received on v2).
>>
>> All the available options whether they apply to the immediate sub-command or
>> not are at the toplevel command today.
>> Hence the position (Not a big fan of the current name. suggestions welcome).
>>
>> I would have loved to have the default output print ranges, If that is
>> acceptable. But, that can break new iproute2 on older kernels (If that is
>> something we should care about).
>>
>> other option is keyword argument like maybe 'bridge vlan show compressed' or
>> 'bridge vlan show ranges'
> How about try RTEXT_FILTER_BRVLAN_COMPRESSED first, and if that fails,
> try RTEXT_FILTER_BRVLAN, and don't add -c option?
>
>
That would have been nice, except that it will not fail. It will pass
but not return any vlans.
Better would be to send both flags. I think that will work. New kernel
looks at RTEXT_FILTER_BRVLAN_COMPRESSED first.
v4 soon ....after some testing ... ;)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
2015-01-26 6:09 ` roopa
@ 2015-01-26 18:57 ` roopa
0 siblings, 0 replies; 6+ messages in thread
From: roopa @ 2015-01-26 18:57 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On 1/25/15, 10:09 PM, roopa wrote:
> On 1/25/15, 9:57 PM, Scott Feldman wrote:
>> On Sun, Jan 25, 2015 at 9:18 PM, roopa <roopa@cumulusnetworks.com>
>> wrote:
>>> On 1/25/15, 8:35 PM, Scott Feldman wrote:
>>>>> diff --git a/bridge/bridge.c b/bridge/bridge.c
>>>>> index 5fcc552..88469ca 100644
>>>>> --- a/bridge/bridge.c
>>>>> +++ b/bridge/bridge.c
>>>>> @@ -21,6 +21,7 @@ int resolve_hosts;
>>>>> int oneline = 0;
>>>>> int show_stats;
>>>>> int show_details;
>>>>> +int compress_vlans;
>>>>> int timestamp;
>>>>> char * _SL_ = NULL;
>>>>>
>>>>> @@ -32,7 +33,8 @@ static void usage(void)
>>>>> "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>>>>> "where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>>>>> " OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
>>>>> -" -o[neline] | -t[imestamp] | -n[etns] name
>>>>> }\n");
>>>>> +" -o[neline] | -t[imestamp] | -n[etns] name |\n"
>>>>> +" -c[ompressvlans] }\n");
>>>> This option is awkward at this level since it's specific to bridge
>>>> vlan
>>>> cmd.
>>>>
>>>> Is it even necessary? The real user of compressed vlans isn't the
>>>> "bridge show" cmd; just keep "bridge show" simple, listing each vlan
>>>> per line.
>>> without ranges it does get a bit difficult (or rather long) to read
>>> if there
>>> are many vlans (it was also a comment I received on v2).
>>>
>>> All the available options whether they apply to the immediate
>>> sub-command or
>>> not are at the toplevel command today.
>>> Hence the position (Not a big fan of the current name. suggestions
>>> welcome).
>>>
>>> I would have loved to have the default output print ranges, If that is
>>> acceptable. But, that can break new iproute2 on older kernels (If
>>> that is
>>> something we should care about).
>>>
>>> other option is keyword argument like maybe 'bridge vlan show
>>> compressed' or
>>> 'bridge vlan show ranges'
>> How about try RTEXT_FILTER_BRVLAN_COMPRESSED first, and if that fails,
>> try RTEXT_FILTER_BRVLAN, and don't add -c option?
>>
>>
> That would have been nice, except that it will not fail. It will pass
> but not return any vlans.
>
> Better would be to send both flags. I think that will work. New kernel
> looks at RTEXT_FILTER_BRVLAN_COMPRESSED first.
> v4 soon ....after some testing ... ;)
This still means i will end up changing the default/current output into
vlan ranges on newer kernels. If that is acceptable then i will resubmit
with v4.
If not, v3 looks ok to me so far.
thanks,
Roopa
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-01-26 18:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-26 2:26 [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges roopa
2015-01-26 4:35 ` Scott Feldman
2015-01-26 5:18 ` roopa
2015-01-26 5:57 ` Scott Feldman
2015-01-26 6:09 ` roopa
2015-01-26 18:57 ` roopa
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.