Re: [PATCH net-next] switchdev: enforce no pvid flag in vlan ranges

[Ido Schimmel <idosch@mellanox.com>]

Mon, Oct 12, 2015 at 08:36:25PM IDT, vivien.didelot@savoirfairelinux.com wrote:
>Hi guys,
>
>On Oct. Monday 12 (42) 02:01 PM, Nikolay Aleksandrov wrote:
>> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> 
>> We shouldn't allow BRIDGE_VLAN_INFO_PVID flag in VLAN ranges.
>> 
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.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@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html