* [PATCH net-next] iproute2: bridge: support vlan range
@ 2015-01-16 6:52 roopa
2015-01-18 1:35 ` Scott Feldman
0 siblings, 1 reply; 5+ messages in thread
From: roopa @ 2015-01-16 6:52 UTC (permalink / raw)
To: netdev, shemminger, vyasevic; +Cc: wkok
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds vlan range support to bridge command
using the newly added vinfo flags BRIDGE_VLAN_INFO_RANGE_BEGIN and
BRIDGE_VLAN_INFO_RANGE_END.
$bridge vlan show
port vlan ids
br0 1 PVID Egress Untagged
dummy0 1 PVID Egress Untagged
$bridge vlan add vid 10-15 dev dummy0
port vlan ids
br0 1 PVID Egress Untagged
dummy0 1 PVID Egress Untagged
10
11
12
13
14
15
$bridge vlan del vid 14 dev dummy0
$bridge vlan show
port vlan ids
br0 1 PVID Egress Untagged
dummy0 1 PVID Egress Untagged
10
11
12
13
15
$bridge vlan del vid 10-15 dev dummy0
$bridge vlan show
port vlan ids
br0 1 PVID Egress Untagged
dummy0 1 PVID Egress Untagged
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
---
bridge/vlan.c | 46 ++++++++++++++++++++++++++++++++++++++-------
include/linux/if_bridge.h | 2 ++
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 3bd7b0d..90b3b6b 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -32,6 +32,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
} req;
char *d = NULL;
short vid = -1;
+ short vid_end = -1;
struct rtattr *afspec;
struct bridge_vlan_info vinfo;
unsigned short flags = 0;
@@ -49,8 +50,18 @@ static int vlan_modify(int cmd, int argc, char **argv)
NEXT_ARG();
d = *argv;
} else if (strcmp(*argv, "vid") == 0) {
+ char *p;
NEXT_ARG();
- vid = atoi(*argv);
+ p = strchr(*argv, '-');
+ if (p) {
+ *p = '\0';
+ p++;
+ vinfo.vid = atoi(*argv);
+ vid_end = atoi(p);
+ vinfo.flags |= BRIDGE_VLAN_INFO_RANGE_BEGIN;
+ } else {
+ vinfo.vid = atoi(*argv);
+ }
} else if (strcmp(*argv, "self") == 0) {
flags |= BRIDGE_FLAGS_SELF;
} else if (strcmp(*argv, "master") == 0) {
@@ -67,7 +78,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
argc--; argv++;
}
- if (d == NULL || vid == -1) {
+ if (d == NULL || vinfo.vid == -1) {
fprintf(stderr, "Device and VLAN ID are required arguments.\n");
exit(-1);
}
@@ -78,20 +89,41 @@ static int vlan_modify(int cmd, int argc, char **argv)
return -1;
}
- if (vid >= 4096) {
- fprintf(stderr, "Invalid VLAN ID \"%hu\"\n", vid);
+ if (vinfo.vid >= 4096) {
+ fprintf(stderr, "Invalid VLAN ID \"%hu\"\n", vinfo.vid);
return -1;
}
- vinfo.vid = vid;
+ if (vinfo.flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
+ if (vid_end == -1 || vid_end >= 4096 || vinfo.vid >= vid_end) {
+ fprintf(stderr, "Invalid VLAN range \"%hu-%hu\"\n",
+ vinfo.vid, vid_end);
+ return -1;
+ }
+ if (vinfo.flags & BRIDGE_VLAN_INFO_PVID) {
+ fprintf(stderr,
+ "pvid cannot be configured for a vlan range\n");
+ return -1;
+ }
+ }
afspec = addattr_nest(&req.n, sizeof(req), IFLA_AF_SPEC);
if (flags)
addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags);
- addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
- sizeof(vinfo));
+ if (vid_end != -1) {
+ addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
+ sizeof(vinfo));
+ vinfo.flags &= ~BRIDGE_VLAN_INFO_RANGE_BEGIN;
+ vinfo.flags |= BRIDGE_VLAN_INFO_RANGE_END;
+ vinfo.vid = vid_end;
+ addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
+ sizeof(vinfo));
+ } else {
+ addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
+ sizeof(vinfo));
+ }
addattr_nest_end(&req.n, afspec);
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index ed6868e..e21a649 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -124,6 +124,8 @@ enum {
#define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
#define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
#define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
+#define BRIDGE_VLAN_INFO_RANGE_BEGIN (1<<3) /* VLAN is start of vlan range */
+#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
struct bridge_vlan_info {
__u16 flags;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] iproute2: bridge: support vlan range
2015-01-16 6:52 [PATCH net-next] iproute2: bridge: support vlan range roopa
@ 2015-01-18 1:35 ` Scott Feldman
2015-01-18 9:11 ` roopa
0 siblings, 1 reply; 5+ messages in thread
From: Scott Feldman @ 2015-01-18 1:35 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On Thu, Jan 15, 2015 at 10:52 PM, <roopa@cumulusnetworks.com> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch adds vlan range support to bridge command
> using the newly added vinfo flags BRIDGE_VLAN_INFO_RANGE_BEGIN and
> BRIDGE_VLAN_INFO_RANGE_END.
>
> $bridge vlan show
> port vlan ids
> br0 1 PVID Egress Untagged
>
> dummy0 1 PVID Egress Untagged
>
> $bridge vlan add vid 10-15 dev dummy0
> port vlan ids
> br0 1 PVID Egress Untagged
>
> dummy0 1 PVID Egress Untagged
> 10
> 11
> 12
> 13
> 14
> 15
>
> $bridge vlan del vid 14 dev dummy0
>
> $bridge vlan show
> port vlan ids
> br0 1 PVID Egress Untagged
>
> dummy0 1 PVID Egress Untagged
> 10
> 11
> 12
> 13
> 15
>
> $bridge vlan del vid 10-15 dev dummy0
>
> $bridge vlan show
> port vlan ids
> br0 1 PVID Egress Untagged
>
> dummy0 1 PVID Egress Untagged
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
> ---
> bridge/vlan.c | 46 ++++++++++++++++++++++++++++++++++++++-------
> include/linux/if_bridge.h | 2 ++
> 2 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/bridge/vlan.c b/bridge/vlan.c
> index 3bd7b0d..90b3b6b 100644
> --- a/bridge/vlan.c
> +++ b/bridge/vlan.c
> @@ -32,6 +32,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
> } req;
> char *d = NULL;
> short vid = -1;
> + short vid_end = -1;
> struct rtattr *afspec;
> struct bridge_vlan_info vinfo;
> unsigned short flags = 0;
> @@ -49,8 +50,18 @@ static int vlan_modify(int cmd, int argc, char **argv)
> NEXT_ARG();
> d = *argv;
> } else if (strcmp(*argv, "vid") == 0) {
> + char *p;
> NEXT_ARG();
> - vid = atoi(*argv);
> + p = strchr(*argv, '-');
> + if (p) {
> + *p = '\0';
> + p++;
> + vinfo.vid = atoi(*argv);
> + vid_end = atoi(p);
Is "vid 10-" same as "vid 10-0"?
Is "vid -15" same as "vid 0-15"?
What is "vid -"?
Does the "-" char mess up shells? I don't know the answer; just asking.
> + vinfo.flags |= BRIDGE_VLAN_INFO_RANGE_BEGIN;
> + } else {
> + vinfo.vid = atoi(*argv);
> + }
> } else if (strcmp(*argv, "self") == 0) {
> flags |= BRIDGE_FLAGS_SELF;
> } else if (strcmp(*argv, "master") == 0) {
> @@ -67,7 +78,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
> argc--; argv++;
> }
>
> - if (d == NULL || vid == -1) {
> + if (d == NULL || vinfo.vid == -1) {
Where was vinfo.vid initialized to -1? Maybe use vid rather than
vinfo.vid in the code above where parsing the arg, and continue using
vid and vid_end until final put of vinfo.
-scott
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] iproute2: bridge: support vlan range
2015-01-18 1:35 ` Scott Feldman
@ 2015-01-18 9:11 ` roopa
2015-01-18 17:44 ` Scott Feldman
0 siblings, 1 reply; 5+ messages in thread
From: roopa @ 2015-01-18 9:11 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On 1/17/15, 5:35 PM, Scott Feldman wrote:
> On Thu, Jan 15, 2015 at 10:52 PM, <roopa@cumulusnetworks.com> wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch adds vlan range support to bridge command
>> using the newly added vinfo flags BRIDGE_VLAN_INFO_RANGE_BEGIN and
>> BRIDGE_VLAN_INFO_RANGE_END.
>>
>> $bridge vlan show
>> port vlan ids
>> br0 1 PVID Egress Untagged
>>
>> dummy0 1 PVID Egress Untagged
>>
>> $bridge vlan add vid 10-15 dev dummy0
>> port vlan ids
>> br0 1 PVID Egress Untagged
>>
>> dummy0 1 PVID Egress Untagged
>> 10
>> 11
>> 12
>> 13
>> 14
>> 15
>>
>> $bridge vlan del vid 14 dev dummy0
>>
>> $bridge vlan show
>> port vlan ids
>> br0 1 PVID Egress Untagged
>>
>> dummy0 1 PVID Egress Untagged
>> 10
>> 11
>> 12
>> 13
>> 15
>>
>> $bridge vlan del vid 10-15 dev dummy0
>>
>> $bridge vlan show
>> port vlan ids
>> br0 1 PVID Egress Untagged
>>
>> dummy0 1 PVID Egress Untagged
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
>> ---
>> bridge/vlan.c | 46 ++++++++++++++++++++++++++++++++++++++-------
>> include/linux/if_bridge.h | 2 ++
>> 2 files changed, 41 insertions(+), 7 deletions(-)
>>
>> diff --git a/bridge/vlan.c b/bridge/vlan.c
>> index 3bd7b0d..90b3b6b 100644
>> --- a/bridge/vlan.c
>> +++ b/bridge/vlan.c
>> @@ -32,6 +32,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
>> } req;
>> char *d = NULL;
>> short vid = -1;
>> + short vid_end = -1;
>> struct rtattr *afspec;
>> struct bridge_vlan_info vinfo;
>> unsigned short flags = 0;
>> @@ -49,8 +50,18 @@ static int vlan_modify(int cmd, int argc, char **argv)
>> NEXT_ARG();
>> d = *argv;
>> } else if (strcmp(*argv, "vid") == 0) {
>> + char *p;
>> NEXT_ARG();
>> - vid = atoi(*argv);
>> + p = strchr(*argv, '-');
>> + if (p) {
>> + *p = '\0';
>> + p++;
>> + vinfo.vid = atoi(*argv);
>> + vid_end = atoi(p);
> Is "vid 10-" same as "vid 10-0"?
correct ..
# bridge vlan add vid 10- dev dummy0
Invalid VLAN range "10-0"
>
> Is "vid -15" same as "vid 0-15"?
correct
# bridge vlan add vid -10 dev dummy0
root@net-next:~/iproute2# bridge vlan show
port vlan ids
br0 1 PVID Egress Untagged
dummy0 0 PVID
1
2
3
4
5
6
7
8
9
10
dummy1 1 PVID Egress Untagged
maybe vlan 0 should not be allowed. Will check
>
> What is "vid -"?
>
> Does the "-" char mess up shells? I don't know the answer; just asking.
No it does not :)
# bridge vlan add vid -
Device and VLAN ID are required arguments.
>
>> + vinfo.flags |= BRIDGE_VLAN_INFO_RANGE_BEGIN;
>> + } else {
>> + vinfo.vid = atoi(*argv);
>> + }
>> } else if (strcmp(*argv, "self") == 0) {
>> flags |= BRIDGE_FLAGS_SELF;
>> } else if (strcmp(*argv, "master") == 0) {
>> @@ -67,7 +78,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
>> argc--; argv++;
>> }
>>
>> - if (d == NULL || vid == -1) {
>> + if (d == NULL || vinfo.vid == -1) {
> Where was vinfo.vid initialized to -1? Maybe use vid rather than
> vinfo.vid in the code above where parsing the arg, and continue using
> vid and vid_end until final put of vinfo.
>
There is already a "memset(&vinfo, 0, sizeof(vinfo));" in the code in
the beginning of the function.
And the code is already using vinfo for flags. That's the reason i
decided to use vinfo here.
Thanks,
Roopa
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] iproute2: bridge: support vlan range
2015-01-18 9:11 ` roopa
@ 2015-01-18 17:44 ` Scott Feldman
2015-01-19 3:06 ` roopa
0 siblings, 1 reply; 5+ messages in thread
From: Scott Feldman @ 2015-01-18 17:44 UTC (permalink / raw)
To: roopa; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On Sun, Jan 18, 2015 at 1:11 AM, roopa <roopa@cumulusnetworks.com> wrote:
> On 1/17/15, 5:35 PM, Scott Feldman wrote:
>>
>> On Thu, Jan 15, 2015 at 10:52 PM, <roopa@cumulusnetworks.com> wrote:
>>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This patch adds vlan range support to bridge command
>>> using the newly added vinfo flags BRIDGE_VLAN_INFO_RANGE_BEGIN and
>>> BRIDGE_VLAN_INFO_RANGE_END.
>>>
>
>>
>>> + vinfo.flags |=
>>> BRIDGE_VLAN_INFO_RANGE_BEGIN;
>>> + } else {
>>> + vinfo.vid = atoi(*argv);
>>> + }
>>> } else if (strcmp(*argv, "self") == 0) {
>>> flags |= BRIDGE_FLAGS_SELF;
>>> } else if (strcmp(*argv, "master") == 0) {
>>> @@ -67,7 +78,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
>>> argc--; argv++;
>>> }
>>>
>>> - if (d == NULL || vid == -1) {
>>> + if (d == NULL || vinfo.vid == -1) {
>>
>> Where was vinfo.vid initialized to -1? Maybe use vid rather than
>> vinfo.vid in the code above where parsing the arg, and continue using
>> vid and vid_end until final put of vinfo.
>>
> There is already a "memset(&vinfo, 0, sizeof(vinfo));" in the code in the
> beginning of the function.
That's the problem...vinfo.vid is initialized to 0, not -1, so
checking if vinfo.vid == -1 is always false.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] iproute2: bridge: support vlan range
2015-01-18 17:44 ` Scott Feldman
@ 2015-01-19 3:06 ` roopa
0 siblings, 0 replies; 5+ messages in thread
From: roopa @ 2015-01-19 3:06 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
On 1/18/15, 9:44 AM, Scott Feldman wrote:
> On Sun, Jan 18, 2015 at 1:11 AM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 1/17/15, 5:35 PM, Scott Feldman wrote:
>>> On Thu, Jan 15, 2015 at 10:52 PM, <roopa@cumulusnetworks.com> wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This patch adds vlan range support to bridge command
>>>> using the newly added vinfo flags BRIDGE_VLAN_INFO_RANGE_BEGIN and
>>>> BRIDGE_VLAN_INFO_RANGE_END.
>>>>
>>>> + vinfo.flags |=
>>>> BRIDGE_VLAN_INFO_RANGE_BEGIN;
>>>> + } else {
>>>> + vinfo.vid = atoi(*argv);
>>>> + }
>>>> } else if (strcmp(*argv, "self") == 0) {
>>>> flags |= BRIDGE_FLAGS_SELF;
>>>> } else if (strcmp(*argv, "master") == 0) {
>>>> @@ -67,7 +78,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
>>>> argc--; argv++;
>>>> }
>>>>
>>>> - if (d == NULL || vid == -1) {
>>>> + if (d == NULL || vinfo.vid == -1) {
>>> Where was vinfo.vid initialized to -1? Maybe use vid rather than
>>> vinfo.vid in the code above where parsing the arg, and continue using
>>> vid and vid_end until final put of vinfo.
>>>
>> There is already a "memset(&vinfo, 0, sizeof(vinfo));" in the code in the
>> beginning of the function.
> That's the problem...vinfo.vid is initialized to 0, not -1, so
> checking if vinfo.vid == -1 is always false.
ack, v2 coming ... thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-19 3:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-16 6:52 [PATCH net-next] iproute2: bridge: support vlan range roopa
2015-01-18 1:35 ` Scott Feldman
2015-01-18 9:11 ` roopa
2015-01-18 17:44 ` Scott Feldman
2015-01-19 3:06 ` roopa
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).