From: roopa <roopa@cumulusnetworks•com>
To: Scott Feldman <sfeldma@gmail•com>
Cc: Netdev <netdev@vger•kernel.org>,
shemminger@vyatta•com,
"vyasevic@redhat•com" <vyasevic@redhat•com>,
Wilson Kok <wkok@cumulusnetworks•com>
Subject: Re: [PATCH net-next] iproute2: bridge: support vlan range
Date: Sun, 18 Jan 2015 01:11:54 -0800 [thread overview]
Message-ID: <54BB78DA.6060208@cumulusnetworks.com> (raw)
In-Reply-To: <CAE4R7bA2VHepe8CCYuubH8teXMvRry10jnpsisYq+7Qv+gXzHQ@mail.gmail.com>
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
next prev parent reply other threads:[~2015-01-18 9:11 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2015-01-18 17:44 ` Scott Feldman
2015-01-19 3:06 ` roopa
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=54BB78DA.6060208@cumulusnetworks.com \
--to=roopa@cumulusnetworks$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sfeldma@gmail$(echo .)com \
--cc=shemminger@vyatta$(echo .)com \
--cc=vyasevic@redhat$(echo .)com \
--cc=wkok@cumulusnetworks$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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