| Message ID | 20240811195649.2360966-1-paweldembicki@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: vsc73xx: implement FDB operations | expand |
On Sun, Aug 11, 2024 at 09:56:49PM +0200, Pawel Dembicki wrote: > This commit introduces implementations of three functions: > .port_fdb_dump > .port_fdb_add > .port_fdb_del > > The FDB database organization is the same as in other old Vitesse chips: > It has 2048 rows and 4 columns (buckets). The row index is calculated by > the hash function 'vsc73xx_calc_hash' and the FDB entry must be placed > exactly into row[hash]. The chip selects the row number by itself. > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > --- > drivers/net/dsa/vitesse-vsc73xx-core.c | 302 +++++++++++++++++++++++++ > drivers/net/dsa/vitesse-vsc73xx.h | 2 + > 2 files changed, 304 insertions(+) > > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c > index a82b550a9e40..7da1641b8bab 100644 > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c > @@ -46,6 +46,8 @@ > #define VSC73XX_BLOCK_MII_EXTERNAL 0x1 /* External MDIO subblock */ > > #define CPU_PORT 6 /* CPU port */ > +#define VSC73XX_NUM_FDB_RECORDS 2048 > +#define VSC73XX_NUM_BUCKETS 4 > > /* MAC Block registers */ > #define VSC73XX_MAC_CFG 0x00 > @@ -197,6 +199,21 @@ > #define VSC73XX_SRCMASKS_MIRROR BIT(26) > #define VSC73XX_SRCMASKS_PORTS_MASK GENMASK(7, 0) > > +#define VSC73XX_MACHDATA_VID GENMASK(27, 16) > +#define VSC73XX_MACHDATA_VID_SHIFT 16 > +#define VSC73XX_MACHDATA_MAC0_SHIFT 8 > +#define VSC73XX_MACHDATA_MAC1_SHIFT 0 > +#define VSC73XX_MACLDATA_MAC2_SHIFT 24 > +#define VSC73XX_MACLDATA_MAC3_SHIFT 16 > +#define VSC73XX_MACLDATA_MAC4_SHIFT 8 > +#define VSC73XX_MACLDATA_MAC5_SHIFT 0 > +#define VSC73XX_MAC_BYTE_MASK GENMASK(7, 0) > + > +#define VSC73XX_MACTINDX_SHADOW BIT(13) > +#define VSC73XX_MACTINDX_BUCKET_MASK GENMASK(12, 11) > +#define VSC73XX_MACTINDX_BUCKET_MASK_SHIFT 11 > +#define VSC73XX_MACTINDX_INDEX_MASK GENMASK(10, 0) > + > #define VSC73XX_MACACCESS_CPU_COPY BIT(14) > #define VSC73XX_MACACCESS_FWD_KILL BIT(13) > #define VSC73XX_MACACCESS_IGNORE_VLAN BIT(12) > @@ -204,6 +221,7 @@ > #define VSC73XX_MACACCESS_VALID BIT(10) > #define VSC73XX_MACACCESS_LOCKED BIT(9) > #define VSC73XX_MACACCESS_DEST_IDX_MASK GENMASK(8, 3) > +#define VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT 3 > #define VSC73XX_MACACCESS_CMD_MASK GENMASK(2, 0) > #define VSC73XX_MACACCESS_CMD_IDLE 0 > #define VSC73XX_MACACCESS_CMD_LEARN 1 > @@ -329,6 +347,13 @@ struct vsc73xx_counter { > const char *name; > }; > > +struct vsc73xx_fdb { > + u16 vid; > + u8 port; > + u8 mac[6]; > + bool valid; > +}; > + > /* Counters are named according to the MIB standards where applicable. > * Some counters are custom, non-standard. The standard counters are > * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex > @@ -1829,6 +1854,278 @@ static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port, > vsc73xx_refresh_fwd_map(ds, port, state); > } > > +static u16 vsc73xx_calc_hash(const unsigned char *addr, u16 vid) > +{ > + /* VID 5-0, MAC 47-44 */ > + u16 hash = ((vid & GENMASK(5, 0)) << 4) | (addr[0] >> 4); > + > + /* MAC 43-33 */ > + hash ^= ((addr[0] & GENMASK(3, 0)) << 7) | (addr[1] >> 1); > + /* MAC 32-22 */ > + hash ^= ((addr[1] & BIT(0)) << 10) | (addr[2] << 2) | (addr[3] >> 6); > + /* MAC 21-11 */ > + hash ^= ((addr[3] & GENMASK(5, 0)) << 5) | (addr[4] >> 3); > + /* MAC 10-0 */ > + hash ^= ((addr[4] & GENMASK(2, 0)) << 8) | addr[5]; In this function and throughout this patchset I see a lot of mask nd shift operations, sometimes combined, here and elsewhere in this patchset. It seems likely that using FIELD_PREP, in conjunction with GENMASK and BIT, would be appropriate instead. If combined with #defines for the GENMASK and BIT constants with meaningful names this may significantly improve readability. > + > + return hash; > +} ...
On Sun, Aug 11, 2024 at 09:56:49PM +0200, Pawel Dembicki wrote: > This commit introduces implementations of three functions: > .port_fdb_dump > .port_fdb_add > .port_fdb_del > > The FDB database organization is the same as in other old Vitesse chips: > It has 2048 rows and 4 columns (buckets). The row index is calculated by > the hash function 'vsc73xx_calc_hash' and the FDB entry must be placed > exactly into row[hash]. The chip selects the row number by itself. You mean "selects the bucket" maybe? > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > --- > drivers/net/dsa/vitesse-vsc73xx-core.c | 302 +++++++++++++++++++++++++ > drivers/net/dsa/vitesse-vsc73xx.h | 2 + > 2 files changed, 304 insertions(+) > > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c > index a82b550a9e40..7da1641b8bab 100644 > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c > @@ -46,6 +46,8 @@ > #define VSC73XX_BLOCK_MII_EXTERNAL 0x1 /* External MDIO subblock */ > > #define CPU_PORT 6 /* CPU port */ > +#define VSC73XX_NUM_FDB_RECORDS 2048 Terminology issue perhaps, but do you call a "record" as something that holds 1 FDB entry, or 4? There should be 2048 * 4 records, and 2048 "rows"? There's also vsc73xx_port_read_mac_table_entry(), which calls an FDB "entry" an array of 4 addresses. Do you have a consistent name for a switch data structure that holds a single address? > +#define VSC73XX_NUM_BUCKETS 4 > > /* MAC Block registers */ > #define VSC73XX_MAC_CFG 0x00 > @@ -197,6 +199,21 @@ > #define VSC73XX_SRCMASKS_MIRROR BIT(26) > #define VSC73XX_SRCMASKS_PORTS_MASK GENMASK(7, 0) > > +#define VSC73XX_MACHDATA_VID GENMASK(27, 16) > +#define VSC73XX_MACHDATA_VID_SHIFT 16 > +#define VSC73XX_MACHDATA_MAC0_SHIFT 8 > +#define VSC73XX_MACHDATA_MAC1_SHIFT 0 > +#define VSC73XX_MACLDATA_MAC2_SHIFT 24 > +#define VSC73XX_MACLDATA_MAC3_SHIFT 16 > +#define VSC73XX_MACLDATA_MAC4_SHIFT 8 > +#define VSC73XX_MACLDATA_MAC5_SHIFT 0 > +#define VSC73XX_MAC_BYTE_MASK GENMASK(7, 0) > + > +#define VSC73XX_MACTINDX_SHADOW BIT(13) > +#define VSC73XX_MACTINDX_BUCKET_MASK GENMASK(12, 11) > +#define VSC73XX_MACTINDX_BUCKET_MASK_SHIFT 11 > +#define VSC73XX_MACTINDX_INDEX_MASK GENMASK(10, 0) > + > #define VSC73XX_MACACCESS_CPU_COPY BIT(14) > #define VSC73XX_MACACCESS_FWD_KILL BIT(13) > #define VSC73XX_MACACCESS_IGNORE_VLAN BIT(12) > @@ -204,6 +221,7 @@ > #define VSC73XX_MACACCESS_VALID BIT(10) > #define VSC73XX_MACACCESS_LOCKED BIT(9) > #define VSC73XX_MACACCESS_DEST_IDX_MASK GENMASK(8, 3) > +#define VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT 3 > #define VSC73XX_MACACCESS_CMD_MASK GENMASK(2, 0) > #define VSC73XX_MACACCESS_CMD_IDLE 0 > #define VSC73XX_MACACCESS_CMD_LEARN 1 > @@ -329,6 +347,13 @@ struct vsc73xx_counter { > const char *name; > }; > > +struct vsc73xx_fdb { > + u16 vid; > + u8 port; > + u8 mac[6]; u8 mac[ETH_ALEN] > + bool valid; > +}; > + > /* Counters are named according to the MIB standards where applicable. > * Some counters are custom, non-standard. The standard counters are > * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex > @@ -1829,6 +1854,278 @@ static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port, > vsc73xx_refresh_fwd_map(ds, port, state); > } > > +static u16 vsc73xx_calc_hash(const unsigned char *addr, u16 vid) > +{ > + /* VID 5-0, MAC 47-44 */ > + u16 hash = ((vid & GENMASK(5, 0)) << 4) | (addr[0] >> 4); > + > + /* MAC 43-33 */ > + hash ^= ((addr[0] & GENMASK(3, 0)) << 7) | (addr[1] >> 1); > + /* MAC 32-22 */ > + hash ^= ((addr[1] & BIT(0)) << 10) | (addr[2] << 2) | (addr[3] >> 6); > + /* MAC 21-11 */ > + hash ^= ((addr[3] & GENMASK(5, 0)) << 5) | (addr[4] >> 3); > + /* MAC 10-0 */ > + hash ^= ((addr[4] & GENMASK(2, 0)) << 8) | addr[5]; > + > + return hash; > +} > + > +static int > +vsc73xx_port_wait_for_mac_table_cmd(struct vsc73xx *vsc) > +{ > + int ret, err; > + u32 val; > + > + ret = read_poll_timeout(vsc73xx_read, err, > + err < 0 || > + ((val & VSC73XX_MACACCESS_CMD_MASK) == > + VSC73XX_MACACCESS_CMD_IDLE), > + VSC73XX_POLL_SLEEP_US, VSC73XX_POLL_TIMEOUT_US, > + false, vsc, VSC73XX_BLOCK_ANALYZER, > + 0, VSC73XX_MACACCESS, &val); > + if (ret) > + return ret; > + return err; > +} > + > +static int vsc73xx_port_read_mac_table_entry(struct vsc73xx *vsc, u16 index, > + struct vsc73xx_fdb *fdb) > +{ > + int ret, i; > + u32 val; > + > + if (!fdb) > + return -EINVAL; > + if (index >= VSC73XX_NUM_FDB_RECORDS) > + return -EINVAL; > + > + for (i = 0; i < VSC73XX_NUM_BUCKETS; i++) { > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, > + (i ? 0 : VSC73XX_MACTINDX_SHADOW) | > + i << VSC73XX_MACTINDX_BUCKET_MASK_SHIFT | > + index); Could you check for error codes from vsc73xx_read() and vsc73xx_write() as well? This is applicable to the entire patch. > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > + if (ret) > + return ret; > + > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, > + VSC73XX_MACACCESS, > + VSC73XX_MACACCESS_CMD_MASK, > + VSC73XX_MACACCESS_CMD_READ_ENTRY); > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > + if (ret) > + return ret; > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACACCESS, > + &val); > + fdb[i].valid = val & VSC73XX_MACACCESS_VALID; > + if (!fdb[i].valid) > + continue; > + > + fdb[i].port = (val & VSC73XX_MACACCESS_DEST_IDX_MASK) >> > + VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT; > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACHDATA, > + &val); > + fdb[i].vid = (val & VSC73XX_MACHDATA_VID) >> > + VSC73XX_MACHDATA_VID_SHIFT; > + fdb[i].mac[0] = (val >> VSC73XX_MACHDATA_MAC0_SHIFT) & > + VSC73XX_MAC_BYTE_MASK; > + fdb[i].mac[1] = (val >> VSC73XX_MACHDATA_MAC1_SHIFT) & > + VSC73XX_MAC_BYTE_MASK; > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACLDATA, > + &val); > + fdb[i].mac[2] = (val >> VSC73XX_MACLDATA_MAC2_SHIFT) & > + VSC73XX_MAC_BYTE_MASK; > + fdb[i].mac[3] = (val >> VSC73XX_MACLDATA_MAC3_SHIFT) & > + VSC73XX_MAC_BYTE_MASK; > + fdb[i].mac[4] = (val >> VSC73XX_MACLDATA_MAC4_SHIFT) & > + VSC73XX_MAC_BYTE_MASK; > + fdb[i].mac[5] = (val >> VSC73XX_MACLDATA_MAC5_SHIFT) & > + VSC73XX_MAC_BYTE_MASK; > + } > + > + return ret; > +} > + > +static void > +vsc73xx_fdb_insert_mac(struct vsc73xx *vsc, const unsigned char *addr, u16 vid) > +{ > + u32 val; > + > + val = (vid << VSC73XX_MACHDATA_VID_SHIFT) & VSC73XX_MACHDATA_VID; > + val |= (addr[0] << VSC73XX_MACHDATA_MAC0_SHIFT); > + val |= (addr[1] << VSC73XX_MACHDATA_MAC1_SHIFT); > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACHDATA, val); > + > + val = (addr[2] << VSC73XX_MACLDATA_MAC2_SHIFT); > + val |= (addr[3] << VSC73XX_MACLDATA_MAC3_SHIFT); > + val |= (addr[4] << VSC73XX_MACLDATA_MAC4_SHIFT); > + val |= (addr[5] << VSC73XX_MACLDATA_MAC5_SHIFT); > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACLDATA, val); > +} > + > +static int vsc73xx_fdb_del_entry(struct vsc73xx *vsc, int port, > + const unsigned char *addr, u16 vid) > +{ > + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; > + u16 hash = vsc73xx_calc_hash(addr, vid); > + int bucket, ret; > + > + mutex_lock(&vsc->fdb_lock); > + > + ret = vsc73xx_port_read_mac_table_entry(vsc, hash, fdb); > + if (ret) > + goto err; > + > + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { > + if (fdb[bucket].valid && fdb[bucket].port == port && > + !memcmp(addr, fdb[bucket].mac, ETH_ALEN)) ether_addr_equal() > + break; > + } > + > + if (bucket == VSC73XX_NUM_BUCKETS) { > + /* Can't find MAC in MAC table */ > + ret = -ENODATA; > + goto err; > + } > + > + vsc73xx_fdb_insert_mac(vsc, addr, vid); > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, hash); > + > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > + if (ret) > + goto err; > + > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACACCESS, > + VSC73XX_MACACCESS_CMD_MASK, > + VSC73XX_MACACCESS_CMD_FORGET); > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > +err: > + mutex_unlock(&vsc->fdb_lock); > + return ret; > +} > + > +static int vsc73xx_fdb_add_entry(struct vsc73xx *vsc, int port, > + const unsigned char *addr, u16 vid) > +{ > + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; > + u16 hash = vsc73xx_calc_hash(addr, vid); > + int bucket, ret; > + u32 val; > + > + mutex_lock(&vsc->fdb_lock); > + > + vsc73xx_port_read_mac_table_entry(vsc, hash, fdb); > + > + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { > + if (!fdb[bucket].valid) > + break; > + } > + > + if (bucket == VSC73XX_NUM_BUCKETS) { > + /* Bucket is full */ > + ret = -EOVERFLOW; > + goto err; > + } > + > + vsc73xx_fdb_insert_mac(vsc, addr, vid); > + > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, hash); > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > + if (ret) > + goto err; > + > + val = (port << VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT) & > + VSC73XX_MACACCESS_DEST_IDX_MASK; > + > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, > + VSC73XX_MACACCESS, > + VSC73XX_MACACCESS_VALID | > + VSC73XX_MACACCESS_CMD_MASK | > + VSC73XX_MACACCESS_DEST_IDX_MASK | > + VSC73XX_MACACCESS_LOCKED, > + VSC73XX_MACACCESS_VALID | > + VSC73XX_MACACCESS_CMD_LEARN | > + VSC73XX_MACACCESS_LOCKED | val); > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > + > +err: > + mutex_unlock(&vsc->fdb_lock); > + return ret; > +} > + > +static int vsc73xx_fdb_add(struct dsa_switch *ds, int port, > + const unsigned char *addr, u16 vid, struct dsa_db db) > +{ > + struct vsc73xx *vsc = ds->priv; > + > + if (!vid) { > + switch (db.type) { > + case DSA_DB_PORT: > + vid = dsa_tag_8021q_standalone_vid(db.dp); > + break; > + case DSA_DB_BRIDGE: > + vid = dsa_tag_8021q_bridge_vid(db.bridge.num); I appreciate the intention, but if you don't set ds->fdb_isolation (which you don't, although I believe the driver satisfies the documented requirements), db.bridge.num will always be passed as 0 in the .port_fdb_add() and .port_fdb_del() methods. Thus, dsa_tag_8021q_bridge_vid(0) will always be different than what dsa_tag_8021q_bridge_join() selects as VLAN-unaware bridge PVID for your ports. The FDB entry will be in a different VLAN than what your switch classifies the packets to. This means it won't match. Assuming this went through a reasonable round of testing (add bridge FDB entry towards expected port, make sure it isn't sent to others) and this issue was not noticed, then maybe the switch performs shared VLAN learning? Case in which, if you can't configure it to independent VLAN learning, it does not pass the ds->fdb_isolation requirements, plus the entire dance of picking a proper VID is pointless, as any chosen VID would have the same behavior. > + break; > + default: > + return -EOPNOTSUPP; > + } > + } > + > + return vsc73xx_fdb_add_entry(vsc, port, addr, vid); > +} > + > +static int vsc73xx_fdb_del(struct dsa_switch *ds, int port, > + const unsigned char *addr, u16 vid, struct dsa_db db) > +{ > + struct vsc73xx *vsc = ds->priv; > + > + if (!vid) { > + switch (db.type) { > + case DSA_DB_PORT: > + vid = dsa_tag_8021q_standalone_vid(db.dp); > + break; > + case DSA_DB_BRIDGE: > + vid = dsa_tag_8021q_bridge_vid(db.bridge.num); > + break; > + default: > + return -EOPNOTSUPP; > + } > + } > + > + return vsc73xx_fdb_del_entry(vsc, port, addr, vid); > +} > + > +static int vsc73xx_port_fdb_dump(struct dsa_switch *ds, > + int port, dsa_fdb_dump_cb_t *cb, void *data) > +{ > + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; > + struct vsc73xx *vsc = ds->priv; > + u16 i, bucket; > + > + mutex_lock(&vsc->fdb_lock); > + > + for (i = 0; i < VSC73XX_NUM_FDB_RECORDS; i++) { > + vsc73xx_port_read_mac_table_entry(vsc, i, fdb); > + > + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { > + if (!fdb[bucket].valid || fdb[bucket].port != port) > + continue; > + > + /* We need to hide dsa_8021q VLANs from the user */ > + if (vid_is_dsa_8021q(fdb[bucket].vid)) > + fdb[bucket].vid = 0; > + cb(fdb[bucket].mac, fdb[bucket].vid, false, data); "cb" is actually dsa_user_port_fdb_do_dump(). It can return -EMSGSIZE when the netlink skb is full, and it is very important that you propagate that to the caller: err = cb(); if (err) goto unlock; otherwise, you might notice that large FDB dumps will have missing FDB entries. > + } > + } > + > + mutex_unlock(&vsc->fdb_lock); > + return 0; > +} > + > static const struct phylink_mac_ops vsc73xx_phylink_mac_ops = { > .mac_config = vsc73xx_mac_config, > .mac_link_down = vsc73xx_mac_link_down, > @@ -1851,6 +2148,9 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { > .port_bridge_join = dsa_tag_8021q_bridge_join, > .port_bridge_leave = dsa_tag_8021q_bridge_leave, > .port_change_mtu = vsc73xx_change_mtu, > + .port_fdb_add = vsc73xx_fdb_add, > + .port_fdb_del = vsc73xx_fdb_del, > + .port_fdb_dump = vsc73xx_port_fdb_dump, > .port_max_mtu = vsc73xx_get_max_mtu, > .port_stp_state_set = vsc73xx_port_stp_state_set, > .port_vlan_filtering = vsc73xx_port_vlan_filtering, > @@ -1981,6 +2281,8 @@ int vsc73xx_probe(struct vsc73xx *vsc) > return -ENODEV; > } > > + mutex_init(&vsc->fdb_lock); > + > eth_random_addr(vsc->addr); > dev_info(vsc->dev, > "MAC for control frames: %02X:%02X:%02X:%02X:%02X:%02X\n", > diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h > index 3ca579acc798..a36ca607671e 100644 > --- a/drivers/net/dsa/vitesse-vsc73xx.h > +++ b/drivers/net/dsa/vitesse-vsc73xx.h > @@ -45,6 +45,7 @@ struct vsc73xx_portinfo { > * @vlans: List of configured vlans. Contains port mask and untagged status of > * every vlan configured in port vlan operation. It doesn't cover tag_8021q > * vlans. > + * @fdb_lock: Mutex protects fdb access > */ > struct vsc73xx { > struct device *dev; > @@ -57,6 +58,7 @@ struct vsc73xx { > void *priv; > struct vsc73xx_portinfo portinfo[VSC73XX_MAX_NUM_PORTS]; > struct list_head vlans; > + struct mutex fdb_lock; /* protects fdb access */ Redundant comment since it's already in the kernel-doc? > }; > > /** > -- > 2.34.1 >
wt., 13 sie 2024 o 13:34 Vladimir Oltean <olteanv@gmail.com> napisał(a): > > On Sun, Aug 11, 2024 at 09:56:49PM +0200, Pawel Dembicki wrote: > > This commit introduces implementations of three functions: > > .port_fdb_dump > > .port_fdb_add > > .port_fdb_del > > > > The FDB database organization is the same as in other old Vitesse chips: > > It has 2048 rows and 4 columns (buckets). The row index is calculated by > > the hash function 'vsc73xx_calc_hash' and the FDB entry must be placed > > exactly into row[hash]. The chip selects the row number by itself. > > You mean "selects the bucket" maybe? > > > > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > > --- > > drivers/net/dsa/vitesse-vsc73xx-core.c | 302 +++++++++++++++++++++++++ > > drivers/net/dsa/vitesse-vsc73xx.h | 2 + > > 2 files changed, 304 insertions(+) > > > > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c > > index a82b550a9e40..7da1641b8bab 100644 > > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c > > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c > > @@ -46,6 +46,8 @@ > > #define VSC73XX_BLOCK_MII_EXTERNAL 0x1 /* External MDIO subblock */ > > > > #define CPU_PORT 6 /* CPU port */ > > +#define VSC73XX_NUM_FDB_RECORDS 2048 > > Terminology issue perhaps, but do you call a "record" as something that > holds 1 FDB entry, or 4? There should be 2048 * 4 records, and 2048 "rows"? > > There's also vsc73xx_port_read_mac_table_entry(), which calls an FDB > "entry" an array of 4 addresses. Do you have a consistent name for a > switch data structure that holds a single address? > > > +#define VSC73XX_NUM_BUCKETS 4 > > > > /* MAC Block registers */ > > #define VSC73XX_MAC_CFG 0x00 > > @@ -197,6 +199,21 @@ > > #define VSC73XX_SRCMASKS_MIRROR BIT(26) > > #define VSC73XX_SRCMASKS_PORTS_MASK GENMASK(7, 0) > > > > +#define VSC73XX_MACHDATA_VID GENMASK(27, 16) > > +#define VSC73XX_MACHDATA_VID_SHIFT 16 > > +#define VSC73XX_MACHDATA_MAC0_SHIFT 8 > > +#define VSC73XX_MACHDATA_MAC1_SHIFT 0 > > +#define VSC73XX_MACLDATA_MAC2_SHIFT 24 > > +#define VSC73XX_MACLDATA_MAC3_SHIFT 16 > > +#define VSC73XX_MACLDATA_MAC4_SHIFT 8 > > +#define VSC73XX_MACLDATA_MAC5_SHIFT 0 > > +#define VSC73XX_MAC_BYTE_MASK GENMASK(7, 0) > > + > > +#define VSC73XX_MACTINDX_SHADOW BIT(13) > > +#define VSC73XX_MACTINDX_BUCKET_MASK GENMASK(12, 11) > > +#define VSC73XX_MACTINDX_BUCKET_MASK_SHIFT 11 > > +#define VSC73XX_MACTINDX_INDEX_MASK GENMASK(10, 0) > > + > > #define VSC73XX_MACACCESS_CPU_COPY BIT(14) > > #define VSC73XX_MACACCESS_FWD_KILL BIT(13) > > #define VSC73XX_MACACCESS_IGNORE_VLAN BIT(12) > > @@ -204,6 +221,7 @@ > > #define VSC73XX_MACACCESS_VALID BIT(10) > > #define VSC73XX_MACACCESS_LOCKED BIT(9) > > #define VSC73XX_MACACCESS_DEST_IDX_MASK GENMASK(8, 3) > > +#define VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT 3 > > #define VSC73XX_MACACCESS_CMD_MASK GENMASK(2, 0) > > #define VSC73XX_MACACCESS_CMD_IDLE 0 > > #define VSC73XX_MACACCESS_CMD_LEARN 1 > > @@ -329,6 +347,13 @@ struct vsc73xx_counter { > > const char *name; > > }; > > > > +struct vsc73xx_fdb { > > + u16 vid; > > + u8 port; > > + u8 mac[6]; > > u8 mac[ETH_ALEN] > > > + bool valid; > > +}; > > + > > /* Counters are named according to the MIB standards where applicable. > > * Some counters are custom, non-standard. The standard counters are > > * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex > > @@ -1829,6 +1854,278 @@ static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port, > > vsc73xx_refresh_fwd_map(ds, port, state); > > } > > > > +static u16 vsc73xx_calc_hash(const unsigned char *addr, u16 vid) > > +{ > > + /* VID 5-0, MAC 47-44 */ > > + u16 hash = ((vid & GENMASK(5, 0)) << 4) | (addr[0] >> 4); > > + > > + /* MAC 43-33 */ > > + hash ^= ((addr[0] & GENMASK(3, 0)) << 7) | (addr[1] >> 1); > > + /* MAC 32-22 */ > > + hash ^= ((addr[1] & BIT(0)) << 10) | (addr[2] << 2) | (addr[3] >> 6); > > + /* MAC 21-11 */ > > + hash ^= ((addr[3] & GENMASK(5, 0)) << 5) | (addr[4] >> 3); > > + /* MAC 10-0 */ > > + hash ^= ((addr[4] & GENMASK(2, 0)) << 8) | addr[5]; > > + > > + return hash; > > +} > > + > > +static int > > +vsc73xx_port_wait_for_mac_table_cmd(struct vsc73xx *vsc) > > +{ > > + int ret, err; > > + u32 val; > > + > > + ret = read_poll_timeout(vsc73xx_read, err, > > + err < 0 || > > + ((val & VSC73XX_MACACCESS_CMD_MASK) == > > + VSC73XX_MACACCESS_CMD_IDLE), > > + VSC73XX_POLL_SLEEP_US, VSC73XX_POLL_TIMEOUT_US, > > + false, vsc, VSC73XX_BLOCK_ANALYZER, > > + 0, VSC73XX_MACACCESS, &val); > > + if (ret) > > + return ret; > > + return err; > > +} > > + > > +static int vsc73xx_port_read_mac_table_entry(struct vsc73xx *vsc, u16 index, > > + struct vsc73xx_fdb *fdb) > > +{ > > + int ret, i; > > + u32 val; > > + > > + if (!fdb) > > + return -EINVAL; > > + if (index >= VSC73XX_NUM_FDB_RECORDS) > > + return -EINVAL; > > + > > + for (i = 0; i < VSC73XX_NUM_BUCKETS; i++) { > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, > > + (i ? 0 : VSC73XX_MACTINDX_SHADOW) | > > + i << VSC73XX_MACTINDX_BUCKET_MASK_SHIFT | > > + index); > > Could you check for error codes from vsc73xx_read() and vsc73xx_write() > as well? This is applicable to the entire patch. > > > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > > + if (ret) > > + return ret; > > + > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, > > + VSC73XX_MACACCESS, > > + VSC73XX_MACACCESS_CMD_MASK, > > + VSC73XX_MACACCESS_CMD_READ_ENTRY); > > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > > + if (ret) > > + return ret; > > + > > + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACACCESS, > > + &val); > > + fdb[i].valid = val & VSC73XX_MACACCESS_VALID; > > + if (!fdb[i].valid) > > + continue; > > + > > + fdb[i].port = (val & VSC73XX_MACACCESS_DEST_IDX_MASK) >> > > + VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT; > > + > > + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACHDATA, > > + &val); > > + fdb[i].vid = (val & VSC73XX_MACHDATA_VID) >> > > + VSC73XX_MACHDATA_VID_SHIFT; > > + fdb[i].mac[0] = (val >> VSC73XX_MACHDATA_MAC0_SHIFT) & > > + VSC73XX_MAC_BYTE_MASK; > > + fdb[i].mac[1] = (val >> VSC73XX_MACHDATA_MAC1_SHIFT) & > > + VSC73XX_MAC_BYTE_MASK; > > + > > + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACLDATA, > > + &val); > > + fdb[i].mac[2] = (val >> VSC73XX_MACLDATA_MAC2_SHIFT) & > > + VSC73XX_MAC_BYTE_MASK; > > + fdb[i].mac[3] = (val >> VSC73XX_MACLDATA_MAC3_SHIFT) & > > + VSC73XX_MAC_BYTE_MASK; > > + fdb[i].mac[4] = (val >> VSC73XX_MACLDATA_MAC4_SHIFT) & > > + VSC73XX_MAC_BYTE_MASK; > > + fdb[i].mac[5] = (val >> VSC73XX_MACLDATA_MAC5_SHIFT) & > > + VSC73XX_MAC_BYTE_MASK; > > + } > > + > > + return ret; > > +} > > + > > +static void > > +vsc73xx_fdb_insert_mac(struct vsc73xx *vsc, const unsigned char *addr, u16 vid) > > +{ > > + u32 val; > > + > > + val = (vid << VSC73XX_MACHDATA_VID_SHIFT) & VSC73XX_MACHDATA_VID; > > + val |= (addr[0] << VSC73XX_MACHDATA_MAC0_SHIFT); > > + val |= (addr[1] << VSC73XX_MACHDATA_MAC1_SHIFT); > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACHDATA, val); > > + > > + val = (addr[2] << VSC73XX_MACLDATA_MAC2_SHIFT); > > + val |= (addr[3] << VSC73XX_MACLDATA_MAC3_SHIFT); > > + val |= (addr[4] << VSC73XX_MACLDATA_MAC4_SHIFT); > > + val |= (addr[5] << VSC73XX_MACLDATA_MAC5_SHIFT); > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACLDATA, val); > > +} > > + > > +static int vsc73xx_fdb_del_entry(struct vsc73xx *vsc, int port, > > + const unsigned char *addr, u16 vid) > > +{ > > + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; > > + u16 hash = vsc73xx_calc_hash(addr, vid); > > + int bucket, ret; > > + > > + mutex_lock(&vsc->fdb_lock); > > + > > + ret = vsc73xx_port_read_mac_table_entry(vsc, hash, fdb); > > + if (ret) > > + goto err; > > + > > + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { > > + if (fdb[bucket].valid && fdb[bucket].port == port && > > + !memcmp(addr, fdb[bucket].mac, ETH_ALEN)) > > ether_addr_equal() > > > + break; > > + } > > + > > + if (bucket == VSC73XX_NUM_BUCKETS) { > > + /* Can't find MAC in MAC table */ > > + ret = -ENODATA; > > + goto err; > > + } > > + > > + vsc73xx_fdb_insert_mac(vsc, addr, vid); > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, hash); > > + > > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > > + if (ret) > > + goto err; > > + > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACACCESS, > > + VSC73XX_MACACCESS_CMD_MASK, > > + VSC73XX_MACACCESS_CMD_FORGET); > > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > > +err: > > + mutex_unlock(&vsc->fdb_lock); > > + return ret; > > +} > > + > > +static int vsc73xx_fdb_add_entry(struct vsc73xx *vsc, int port, > > + const unsigned char *addr, u16 vid) > > +{ > > + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; > > + u16 hash = vsc73xx_calc_hash(addr, vid); > > + int bucket, ret; > > + u32 val; > > + > > + mutex_lock(&vsc->fdb_lock); > > + > > + vsc73xx_port_read_mac_table_entry(vsc, hash, fdb); > > + > > + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { > > + if (!fdb[bucket].valid) > > + break; > > + } > > + > > + if (bucket == VSC73XX_NUM_BUCKETS) { > > + /* Bucket is full */ > > + ret = -EOVERFLOW; > > + goto err; > > + } > > + > > + vsc73xx_fdb_insert_mac(vsc, addr, vid); > > + > > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, hash); > > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > > + if (ret) > > + goto err; > > + > > + val = (port << VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT) & > > + VSC73XX_MACACCESS_DEST_IDX_MASK; > > + > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, > > + VSC73XX_MACACCESS, > > + VSC73XX_MACACCESS_VALID | > > + VSC73XX_MACACCESS_CMD_MASK | > > + VSC73XX_MACACCESS_DEST_IDX_MASK | > > + VSC73XX_MACACCESS_LOCKED, > > + VSC73XX_MACACCESS_VALID | > > + VSC73XX_MACACCESS_CMD_LEARN | > > + VSC73XX_MACACCESS_LOCKED | val); > > + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); > > + > > +err: > > + mutex_unlock(&vsc->fdb_lock); > > + return ret; > > +} > > + > > +static int vsc73xx_fdb_add(struct dsa_switch *ds, int port, > > + const unsigned char *addr, u16 vid, struct dsa_db db) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + > > + if (!vid) { > > + switch (db.type) { > > + case DSA_DB_PORT: > > + vid = dsa_tag_8021q_standalone_vid(db.dp); > > + break; > > + case DSA_DB_BRIDGE: > > + vid = dsa_tag_8021q_bridge_vid(db.bridge.num); > > I appreciate the intention, but if you don't set ds->fdb_isolation > (which you don't, although I believe the driver satisfies the documented > requirements), db.bridge.num will always be passed as 0 in the > .port_fdb_add() and .port_fdb_del() methods. Thus, dsa_tag_8021q_bridge_vid(0) > will always be different than what dsa_tag_8021q_bridge_join() selects > as VLAN-unaware bridge PVID for your ports. The FDB entry will be in a > different VLAN than what your switch classifies the packets to. This > means it won't match. > > Assuming this went through a reasonable round of testing (add bridge FDB > entry towards expected port, make sure it isn't sent to others) and this > issue was not noticed, then maybe the switch performs shared VLAN learning? > Case in which, if you can't configure it to independent VLAN learning, > it does not pass the ds->fdb_isolation requirements, plus the entire > dance of picking a proper VID is pointless, as any chosen VID would have > the same behavior. > ds->fdb_isolation was missed in this patch accidentally. Vsc73xx has enabled independent learning by default. > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + } > > + > > + return vsc73xx_fdb_add_entry(vsc, port, addr, vid); > > +} > > + > > +static int vsc73xx_fdb_del(struct dsa_switch *ds, int port, > > + const unsigned char *addr, u16 vid, struct dsa_db db) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + > > + if (!vid) { > > + switch (db.type) { > > + case DSA_DB_PORT: > > + vid = dsa_tag_8021q_standalone_vid(db.dp); > > + break; > > + case DSA_DB_BRIDGE: > > + vid = dsa_tag_8021q_bridge_vid(db.bridge.num); > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + } > > + > > + return vsc73xx_fdb_del_entry(vsc, port, addr, vid); > > +} > > + > > +static int vsc73xx_port_fdb_dump(struct dsa_switch *ds, > > + int port, dsa_fdb_dump_cb_t *cb, void *data) > > +{ > > + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; > > + struct vsc73xx *vsc = ds->priv; > > + u16 i, bucket; > > + > > + mutex_lock(&vsc->fdb_lock); > > + > > + for (i = 0; i < VSC73XX_NUM_FDB_RECORDS; i++) { > > + vsc73xx_port_read_mac_table_entry(vsc, i, fdb); > > + > > + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { > > + if (!fdb[bucket].valid || fdb[bucket].port != port) > > + continue; > > + > > + /* We need to hide dsa_8021q VLANs from the user */ > > + if (vid_is_dsa_8021q(fdb[bucket].vid)) > > + fdb[bucket].vid = 0; > > + cb(fdb[bucket].mac, fdb[bucket].vid, false, data); > > "cb" is actually dsa_user_port_fdb_do_dump(). It can return -EMSGSIZE > when the netlink skb is full, and it is very important that you > propagate that to the caller: > > err = cb(); > if (err) > goto unlock; > > otherwise, you might notice that large FDB dumps will have missing FDB entries. > > > + } > > + } > > + > > + mutex_unlock(&vsc->fdb_lock); > > + return 0; > > +} > > + > > static const struct phylink_mac_ops vsc73xx_phylink_mac_ops = { > > .mac_config = vsc73xx_mac_config, > > .mac_link_down = vsc73xx_mac_link_down, > > @@ -1851,6 +2148,9 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { > > .port_bridge_join = dsa_tag_8021q_bridge_join, > > .port_bridge_leave = dsa_tag_8021q_bridge_leave, > > .port_change_mtu = vsc73xx_change_mtu, > > + .port_fdb_add = vsc73xx_fdb_add, > > + .port_fdb_del = vsc73xx_fdb_del, > > + .port_fdb_dump = vsc73xx_port_fdb_dump, > > .port_max_mtu = vsc73xx_get_max_mtu, > > .port_stp_state_set = vsc73xx_port_stp_state_set, > > .port_vlan_filtering = vsc73xx_port_vlan_filtering, > > @@ -1981,6 +2281,8 @@ int vsc73xx_probe(struct vsc73xx *vsc) > > return -ENODEV; > > } > > > > + mutex_init(&vsc->fdb_lock); > > + > > eth_random_addr(vsc->addr); > > dev_info(vsc->dev, > > "MAC for control frames: %02X:%02X:%02X:%02X:%02X:%02X\n", > > diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h > > index 3ca579acc798..a36ca607671e 100644 > > --- a/drivers/net/dsa/vitesse-vsc73xx.h > > +++ b/drivers/net/dsa/vitesse-vsc73xx.h > > @@ -45,6 +45,7 @@ struct vsc73xx_portinfo { > > * @vlans: List of configured vlans. Contains port mask and untagged status of > > * every vlan configured in port vlan operation. It doesn't cover tag_8021q > > * vlans. > > + * @fdb_lock: Mutex protects fdb access > > */ > > struct vsc73xx { > > struct device *dev; > > @@ -57,6 +58,7 @@ struct vsc73xx { > > void *priv; > > struct vsc73xx_portinfo portinfo[VSC73XX_MAX_NUM_PORTS]; > > struct list_head vlans; > > + struct mutex fdb_lock; /* protects fdb access */ > > Redundant comment since it's already in the kernel-doc? > > > }; > > > > /** > > -- > > 2.34.1 > >
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c index a82b550a9e40..7da1641b8bab 100644 --- a/drivers/net/dsa/vitesse-vsc73xx-core.c +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c @@ -46,6 +46,8 @@ #define VSC73XX_BLOCK_MII_EXTERNAL 0x1 /* External MDIO subblock */ #define CPU_PORT 6 /* CPU port */ +#define VSC73XX_NUM_FDB_RECORDS 2048 +#define VSC73XX_NUM_BUCKETS 4 /* MAC Block registers */ #define VSC73XX_MAC_CFG 0x00 @@ -197,6 +199,21 @@ #define VSC73XX_SRCMASKS_MIRROR BIT(26) #define VSC73XX_SRCMASKS_PORTS_MASK GENMASK(7, 0) +#define VSC73XX_MACHDATA_VID GENMASK(27, 16) +#define VSC73XX_MACHDATA_VID_SHIFT 16 +#define VSC73XX_MACHDATA_MAC0_SHIFT 8 +#define VSC73XX_MACHDATA_MAC1_SHIFT 0 +#define VSC73XX_MACLDATA_MAC2_SHIFT 24 +#define VSC73XX_MACLDATA_MAC3_SHIFT 16 +#define VSC73XX_MACLDATA_MAC4_SHIFT 8 +#define VSC73XX_MACLDATA_MAC5_SHIFT 0 +#define VSC73XX_MAC_BYTE_MASK GENMASK(7, 0) + +#define VSC73XX_MACTINDX_SHADOW BIT(13) +#define VSC73XX_MACTINDX_BUCKET_MASK GENMASK(12, 11) +#define VSC73XX_MACTINDX_BUCKET_MASK_SHIFT 11 +#define VSC73XX_MACTINDX_INDEX_MASK GENMASK(10, 0) + #define VSC73XX_MACACCESS_CPU_COPY BIT(14) #define VSC73XX_MACACCESS_FWD_KILL BIT(13) #define VSC73XX_MACACCESS_IGNORE_VLAN BIT(12) @@ -204,6 +221,7 @@ #define VSC73XX_MACACCESS_VALID BIT(10) #define VSC73XX_MACACCESS_LOCKED BIT(9) #define VSC73XX_MACACCESS_DEST_IDX_MASK GENMASK(8, 3) +#define VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT 3 #define VSC73XX_MACACCESS_CMD_MASK GENMASK(2, 0) #define VSC73XX_MACACCESS_CMD_IDLE 0 #define VSC73XX_MACACCESS_CMD_LEARN 1 @@ -329,6 +347,13 @@ struct vsc73xx_counter { const char *name; }; +struct vsc73xx_fdb { + u16 vid; + u8 port; + u8 mac[6]; + bool valid; +}; + /* Counters are named according to the MIB standards where applicable. * Some counters are custom, non-standard. The standard counters are * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex @@ -1829,6 +1854,278 @@ static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port, vsc73xx_refresh_fwd_map(ds, port, state); } +static u16 vsc73xx_calc_hash(const unsigned char *addr, u16 vid) +{ + /* VID 5-0, MAC 47-44 */ + u16 hash = ((vid & GENMASK(5, 0)) << 4) | (addr[0] >> 4); + + /* MAC 43-33 */ + hash ^= ((addr[0] & GENMASK(3, 0)) << 7) | (addr[1] >> 1); + /* MAC 32-22 */ + hash ^= ((addr[1] & BIT(0)) << 10) | (addr[2] << 2) | (addr[3] >> 6); + /* MAC 21-11 */ + hash ^= ((addr[3] & GENMASK(5, 0)) << 5) | (addr[4] >> 3); + /* MAC 10-0 */ + hash ^= ((addr[4] & GENMASK(2, 0)) << 8) | addr[5]; + + return hash; +} + +static int +vsc73xx_port_wait_for_mac_table_cmd(struct vsc73xx *vsc) +{ + int ret, err; + u32 val; + + ret = read_poll_timeout(vsc73xx_read, err, + err < 0 || + ((val & VSC73XX_MACACCESS_CMD_MASK) == + VSC73XX_MACACCESS_CMD_IDLE), + VSC73XX_POLL_SLEEP_US, VSC73XX_POLL_TIMEOUT_US, + false, vsc, VSC73XX_BLOCK_ANALYZER, + 0, VSC73XX_MACACCESS, &val); + if (ret) + return ret; + return err; +} + +static int vsc73xx_port_read_mac_table_entry(struct vsc73xx *vsc, u16 index, + struct vsc73xx_fdb *fdb) +{ + int ret, i; + u32 val; + + if (!fdb) + return -EINVAL; + if (index >= VSC73XX_NUM_FDB_RECORDS) + return -EINVAL; + + for (i = 0; i < VSC73XX_NUM_BUCKETS; i++) { + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, + (i ? 0 : VSC73XX_MACTINDX_SHADOW) | + i << VSC73XX_MACTINDX_BUCKET_MASK_SHIFT | + index); + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); + if (ret) + return ret; + + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, + VSC73XX_MACACCESS, + VSC73XX_MACACCESS_CMD_MASK, + VSC73XX_MACACCESS_CMD_READ_ENTRY); + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); + if (ret) + return ret; + + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACACCESS, + &val); + fdb[i].valid = val & VSC73XX_MACACCESS_VALID; + if (!fdb[i].valid) + continue; + + fdb[i].port = (val & VSC73XX_MACACCESS_DEST_IDX_MASK) >> + VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT; + + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACHDATA, + &val); + fdb[i].vid = (val & VSC73XX_MACHDATA_VID) >> + VSC73XX_MACHDATA_VID_SHIFT; + fdb[i].mac[0] = (val >> VSC73XX_MACHDATA_MAC0_SHIFT) & + VSC73XX_MAC_BYTE_MASK; + fdb[i].mac[1] = (val >> VSC73XX_MACHDATA_MAC1_SHIFT) & + VSC73XX_MAC_BYTE_MASK; + + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACLDATA, + &val); + fdb[i].mac[2] = (val >> VSC73XX_MACLDATA_MAC2_SHIFT) & + VSC73XX_MAC_BYTE_MASK; + fdb[i].mac[3] = (val >> VSC73XX_MACLDATA_MAC3_SHIFT) & + VSC73XX_MAC_BYTE_MASK; + fdb[i].mac[4] = (val >> VSC73XX_MACLDATA_MAC4_SHIFT) & + VSC73XX_MAC_BYTE_MASK; + fdb[i].mac[5] = (val >> VSC73XX_MACLDATA_MAC5_SHIFT) & + VSC73XX_MAC_BYTE_MASK; + } + + return ret; +} + +static void +vsc73xx_fdb_insert_mac(struct vsc73xx *vsc, const unsigned char *addr, u16 vid) +{ + u32 val; + + val = (vid << VSC73XX_MACHDATA_VID_SHIFT) & VSC73XX_MACHDATA_VID; + val |= (addr[0] << VSC73XX_MACHDATA_MAC0_SHIFT); + val |= (addr[1] << VSC73XX_MACHDATA_MAC1_SHIFT); + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACHDATA, val); + + val = (addr[2] << VSC73XX_MACLDATA_MAC2_SHIFT); + val |= (addr[3] << VSC73XX_MACLDATA_MAC3_SHIFT); + val |= (addr[4] << VSC73XX_MACLDATA_MAC4_SHIFT); + val |= (addr[5] << VSC73XX_MACLDATA_MAC5_SHIFT); + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACLDATA, val); +} + +static int vsc73xx_fdb_del_entry(struct vsc73xx *vsc, int port, + const unsigned char *addr, u16 vid) +{ + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; + u16 hash = vsc73xx_calc_hash(addr, vid); + int bucket, ret; + + mutex_lock(&vsc->fdb_lock); + + ret = vsc73xx_port_read_mac_table_entry(vsc, hash, fdb); + if (ret) + goto err; + + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { + if (fdb[bucket].valid && fdb[bucket].port == port && + !memcmp(addr, fdb[bucket].mac, ETH_ALEN)) + break; + } + + if (bucket == VSC73XX_NUM_BUCKETS) { + /* Can't find MAC in MAC table */ + ret = -ENODATA; + goto err; + } + + vsc73xx_fdb_insert_mac(vsc, addr, vid); + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, hash); + + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); + if (ret) + goto err; + + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACACCESS, + VSC73XX_MACACCESS_CMD_MASK, + VSC73XX_MACACCESS_CMD_FORGET); + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); +err: + mutex_unlock(&vsc->fdb_lock); + return ret; +} + +static int vsc73xx_fdb_add_entry(struct vsc73xx *vsc, int port, + const unsigned char *addr, u16 vid) +{ + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; + u16 hash = vsc73xx_calc_hash(addr, vid); + int bucket, ret; + u32 val; + + mutex_lock(&vsc->fdb_lock); + + vsc73xx_port_read_mac_table_entry(vsc, hash, fdb); + + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { + if (!fdb[bucket].valid) + break; + } + + if (bucket == VSC73XX_NUM_BUCKETS) { + /* Bucket is full */ + ret = -EOVERFLOW; + goto err; + } + + vsc73xx_fdb_insert_mac(vsc, addr, vid); + + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_MACTINDX, hash); + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); + if (ret) + goto err; + + val = (port << VSC73XX_MACACCESS_DEST_IDX_MASK_SHIFT) & + VSC73XX_MACACCESS_DEST_IDX_MASK; + + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, + VSC73XX_MACACCESS, + VSC73XX_MACACCESS_VALID | + VSC73XX_MACACCESS_CMD_MASK | + VSC73XX_MACACCESS_DEST_IDX_MASK | + VSC73XX_MACACCESS_LOCKED, + VSC73XX_MACACCESS_VALID | + VSC73XX_MACACCESS_CMD_LEARN | + VSC73XX_MACACCESS_LOCKED | val); + ret = vsc73xx_port_wait_for_mac_table_cmd(vsc); + +err: + mutex_unlock(&vsc->fdb_lock); + return ret; +} + +static int vsc73xx_fdb_add(struct dsa_switch *ds, int port, + const unsigned char *addr, u16 vid, struct dsa_db db) +{ + struct vsc73xx *vsc = ds->priv; + + if (!vid) { + switch (db.type) { + case DSA_DB_PORT: + vid = dsa_tag_8021q_standalone_vid(db.dp); + break; + case DSA_DB_BRIDGE: + vid = dsa_tag_8021q_bridge_vid(db.bridge.num); + break; + default: + return -EOPNOTSUPP; + } + } + + return vsc73xx_fdb_add_entry(vsc, port, addr, vid); +} + +static int vsc73xx_fdb_del(struct dsa_switch *ds, int port, + const unsigned char *addr, u16 vid, struct dsa_db db) +{ + struct vsc73xx *vsc = ds->priv; + + if (!vid) { + switch (db.type) { + case DSA_DB_PORT: + vid = dsa_tag_8021q_standalone_vid(db.dp); + break; + case DSA_DB_BRIDGE: + vid = dsa_tag_8021q_bridge_vid(db.bridge.num); + break; + default: + return -EOPNOTSUPP; + } + } + + return vsc73xx_fdb_del_entry(vsc, port, addr, vid); +} + +static int vsc73xx_port_fdb_dump(struct dsa_switch *ds, + int port, dsa_fdb_dump_cb_t *cb, void *data) +{ + struct vsc73xx_fdb fdb[VSC73XX_NUM_BUCKETS]; + struct vsc73xx *vsc = ds->priv; + u16 i, bucket; + + mutex_lock(&vsc->fdb_lock); + + for (i = 0; i < VSC73XX_NUM_FDB_RECORDS; i++) { + vsc73xx_port_read_mac_table_entry(vsc, i, fdb); + + for (bucket = 0; bucket < VSC73XX_NUM_BUCKETS; bucket++) { + if (!fdb[bucket].valid || fdb[bucket].port != port) + continue; + + /* We need to hide dsa_8021q VLANs from the user */ + if (vid_is_dsa_8021q(fdb[bucket].vid)) + fdb[bucket].vid = 0; + cb(fdb[bucket].mac, fdb[bucket].vid, false, data); + } + } + + mutex_unlock(&vsc->fdb_lock); + return 0; +} + static const struct phylink_mac_ops vsc73xx_phylink_mac_ops = { .mac_config = vsc73xx_mac_config, .mac_link_down = vsc73xx_mac_link_down, @@ -1851,6 +2148,9 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { .port_bridge_join = dsa_tag_8021q_bridge_join, .port_bridge_leave = dsa_tag_8021q_bridge_leave, .port_change_mtu = vsc73xx_change_mtu, + .port_fdb_add = vsc73xx_fdb_add, + .port_fdb_del = vsc73xx_fdb_del, + .port_fdb_dump = vsc73xx_port_fdb_dump, .port_max_mtu = vsc73xx_get_max_mtu, .port_stp_state_set = vsc73xx_port_stp_state_set, .port_vlan_filtering = vsc73xx_port_vlan_filtering, @@ -1981,6 +2281,8 @@ int vsc73xx_probe(struct vsc73xx *vsc) return -ENODEV; } + mutex_init(&vsc->fdb_lock); + eth_random_addr(vsc->addr); dev_info(vsc->dev, "MAC for control frames: %02X:%02X:%02X:%02X:%02X:%02X\n", diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h index 3ca579acc798..a36ca607671e 100644 --- a/drivers/net/dsa/vitesse-vsc73xx.h +++ b/drivers/net/dsa/vitesse-vsc73xx.h @@ -45,6 +45,7 @@ struct vsc73xx_portinfo { * @vlans: List of configured vlans. Contains port mask and untagged status of * every vlan configured in port vlan operation. It doesn't cover tag_8021q * vlans. + * @fdb_lock: Mutex protects fdb access */ struct vsc73xx { struct device *dev; @@ -57,6 +58,7 @@ struct vsc73xx { void *priv; struct vsc73xx_portinfo portinfo[VSC73XX_MAX_NUM_PORTS]; struct list_head vlans; + struct mutex fdb_lock; /* protects fdb access */ }; /**
This commit introduces implementations of three functions: .port_fdb_dump .port_fdb_add .port_fdb_del The FDB database organization is the same as in other old Vitesse chips: It has 2048 rows and 4 columns (buckets). The row index is calculated by the hash function 'vsc73xx_calc_hash' and the FDB entry must be placed exactly into row[hash]. The chip selects the row number by itself. Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> --- drivers/net/dsa/vitesse-vsc73xx-core.c | 302 +++++++++++++++++++++++++ drivers/net/dsa/vitesse-vsc73xx.h | 2 + 2 files changed, 304 insertions(+)