[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151013083111.GA1432@colbert.mtl.com>
Date: Tue, 13 Oct 2015 11:31:52 +0300
From: Ido Schimmel <idosch@...lanox.com>
To: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
CC: Nikolay Aleksandrov <razor@...ckwall.org>,
<netdev@...r.kernel.org>, <sfeldma@...il.com>, <jiri@...nulli.us>,
<davem@...emloft.net>,
"Nikolay Aleksandrov" <nikolay@...ulusnetworks.com>,
<eladr@...lanox.com>
Subject: Re: [PATCH net-next] switchdev: enforce no pvid flag in vlan ranges
Mon, Oct 12, 2015 at 08:36:25PM IDT, vivien.didelot@...oirfairelinux.com wrote:
>Hi guys,
>
>On Oct. Monday 12 (42) 02:01 PM, Nikolay Aleksandrov wrote:
>> From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>>
>> We shouldn't allow BRIDGE_VLAN_INFO_PVID flag in VLAN ranges.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>> ---
>> net/switchdev/switchdev.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>> index 6e4a4f9ad927..256c596de896 100644
>> --- a/net/switchdev/switchdev.c
>> +++ b/net/switchdev/switchdev.c
>> @@ -720,6 +720,9 @@ static int switchdev_port_br_afspec(struct net_device *dev,
>> if (vlan.vid_begin)
>> return -EINVAL;
>> vlan.vid_begin = vinfo->vid;
>> + /* don't allow range of pvids */
>> + if (vlan.flags & BRIDGE_VLAN_INFO_PVID)
>> + return -EINVAL;
>> } else if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END) {
>> if (!vlan.vid_begin)
>> return -EINVAL;
>> --
>> 2.4.3
>>
>
>Yes the patch looks good, but it is a minor check though. I hope the
>subject of this thread is making sense.
>
>VLAN ranges seem to have been included for an UX purpose (so commands
>look like Cisco IOS). We don't want to change any existing interface, so
>we pushed that down to drivers, with the only valid reason that, maybe
>one day, an hardware can be capable of programming a range on a per-port
>basis.
Hi,
That's actually what we are doing in mlxsw. We can do up to 256 entries in
one go. We've yet to submit this part.
>
>So what happens is that we'll add some code to fix and check non-sense
>(e.g. range + PVID) in switchdev, bridge, and I'm sure we are missing
>other spots.
>
>Sorry for being insistent, but this still doesn't look right to me.
>
>It seems like we are bloating bridge, switchdev and drivers for the only
>reason to maintain a kernel support for something like:
>
> # for i in $(seq 100 4000); do bridge vlan add vid $i dev swp0; done
>
>Thanks,
>-v
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@...r.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists