nl_bridge: dont add fdb entry to neighbors without vlan

[rubensfig]

Fixes bisdn/basebox/issues/272.

Description

272 > [] We assume that all fdb entries come assigned to vlan. rtnl_neigh_get_vlan() then returns -1 because vid is unset, which we then pass on as vid, which is obviously not a valid VID.

Motivation and Context

Avoids

baseboxd[1279]: I1008 08:25:06.064443  1289 nl_bridge.cc:695] add_neigh_to_fdb: add mac=<caddress_ll 00:00:00:01:00:01 >
baseboxd[1279]:  to bridge swbridge on port=7 vlan=4294967295, permanent=1
baseboxd[1279]: I1008 08:25:06.064512  1289 nl_bridge.cc:698] add_neigh_to_fdb: object: bridge dev port7 lladdr 00:00:00:01:00:01 swbridge <noarp,permanent,self,master>
baseboxd[1279]: baseboxd: ../../git/lib/rofl_ofdpa_fm_driver.cpp:2023: rofl::openflow::cofflowmod rofl::openflow::rofl_ofdpa_fm_driver::add_bridging_unicast_vlan(uint8_t, uint32_t, uint16_t, const cmacaddr&, bool, bool): Assertion `vid < 0x1000' failed.

How Has This Been Tested?

With the following script.

#!/usr/bin/env bash
BRIDGE_NAME=swbridge

BNG_DP_PORT=port7
BNG_VID=100
BNG_DOWNSTREAM_MAC=00:00:00:01:00:01

## Delete and add bridge
ip link del $BRIDGE_NAME
ip link add $BRIDGE_NAME type bridge vlan_filtering 1 vlan_default_pvid 0

## VLANs that are processed by the bridge
bridge vlan add vid $BNG_VID dev $BRIDGE_NAME self

## Add ports to bridge
ip link set $BNG_DP_PORT master $BRIDGE_NAME # BNG data plane

## Add VLANs to ports
bridge vlan add vid $BNG_VID dev $BNG_DP_PORT pvid egress untagged

## Add forwarding entry
bridge fdb add $BNG_DOWNSTREAM_MAC dev $BNG_DP_PORT

nl_bridge.cc:683] add_neigh_to_fdb: vlan not set, skipping add neigh=bridge dev port2 lladdr 00:00:00:01:00:01 swbridge <noarp,permanent,self>
...
nl_bridge.cc:707] add_neigh_to_fdb: object: bridge dev port2 lladdr 0c:c4:7a:9c:27:d1 vlan 100 swbridge <reachable,master,ext_learned>

This entry will be configured in Linux, but will not be translated into the ASIC.

bridge fdb
00:00:00:01:00:01 dev port2 self permanent

flowtable 50
Table ID 50 (Bridging):   Retrieving all entries. Max entries = 24575, Current entries = 1.
--  vlanId:mask = 0x1064:0x1fff (VLAN 100) destMac:mask = 0000.0000.0000:0000.0000.0000 | GoTo = 60 (ACL Policy) groupId = 0x40640064 outPort = 0 (Physical)  | priority = 2 hard_time = 0 idle_time = 0 cookie = 27

[rubensfig]

updated the comment

[KanjiMonster]

ip link add $BRIDGE_NAME type bridge vlan_filtering 1 vlan_default_pvid 0

Please do the test with a bridge with vlan defined.

[KanjiMonster]

ip link add $BRIDGE_NAME type bridge vlan_filtering 1 vlan_default_pvid 0

Please do the test with a bridge with vlan defined.

You did, but something is weird.

If you do that you should get two fdb entries, one without vid, and one with the pvid (of 100).

E.g. if I do the following on the ag7648 with the default docker bridge:

agema-ag7648:~$ bridge vlan show
port              vlan-id  
docker0           1 PVID Egress Untagged
agema-ag7648:~$ sudo bridge fdb add 00:00:00:01:00:01 dev docker0
agema-ag7648:~$ bridge fdb | grep docker0 | grep 00:00:00:01:00:01
00:00:00:01:00:01 dev docker0 vlan 1 master docker0 permanent
00:00:00:01:00:01 dev docker0 master docker0 permanent

I have two fdb entries.

[KanjiMonster]

Okay, the difference is master vs self. When adding the fdb entry with master, an entry will be created for all vlans as well.

Reading about it, self means local to the device without notifying the master device about it (i.e. the bridge), while master means the bridge will be aware of it.

So I'm wondering if we should ignore self fdb entries in general.