[Bridge] [PATCH net-next] bridge: Set the pvid for untaged packet before prerouting

[Nikolay Aleksandrov]

On 10/06/2019 12:44, wenxu at ucloud.cn wrote:
> From: wenxu <wenxu at ucloud.cn>
> 
> bridge vlan add dev veth1 vid 200 pvid untagged
> bridge vlan add dev veth2 vid 200 pvid untagged
> 
> nft add table bridge firewall
> nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
> nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
> 
> As above set the bridge port with pvid, the received packet don't contain
> the vlan tag which means the packet should belong to vlan 200 through pvid.
> User can do conntrack base base on vlan id and map the vlan id to zone id
> in the prerouting hook.
> 
> Signed-off-by: wenxu <wenxu at ucloud.cn>
> ---
>  net/bridge/br_input.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Nacked-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>

Hi,
I don't think this is a good idea for a few reasons:
- duplicating code (pvid insertion by __allowed_ingress)
- adding 2 new tests in the fast path (even 3 in the vlan filtering case)
- issue can be solved with current state by using different config

Why do you need the vid to be set when you can assume that all the traffic from
that port belongs to the pvid vlan ? In this case you can match the port ifindex
for example and associate the zones based on that. Another approach - you can
insert the vlan by tc's vlan action on ingress, you'll get the same effect.

Overall this looks like an issue solvable by different config instead of adding new tests
in the fast path and increasing the complexity of the bridge input code for little value.

Cheers,
 Nik