| Message ID | 20230404101842.1382986-3-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 57795412a447812553a8b61bbd968d06a54a41b4 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: ksz8: Enhance static MAC table operations and error handling | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 18 this patch: 18 |
| netdev/cc_maintainers | success | CCed 10 of 10 maintainers |
| netdev/build_clang | success | Errors and warnings before: 18 this patch: 18 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 18 this patch: 18 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 36 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Apr 04, 2023 at 12:18:37PM +0200, Oleksij Rempel wrote: > Add support for add/del_fdb operations and utilize the refactored static > MAC table code. This resolves kernel warnings caused by the lack of fdb > add function support in the current driver. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- Side note, I wonder if it's so simple, why this was not done in e66f840c08a2 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")?
On Tue, Apr 04, 2023 at 02:31:24PM +0300, Vladimir Oltean wrote: > On Tue, Apr 04, 2023 at 12:18:37PM +0200, Oleksij Rempel wrote: > > Add support for add/del_fdb operations and utilize the refactored static > > MAC table code. This resolves kernel warnings caused by the lack of fdb > > add function support in the current driver. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > Side note, I wonder if it's so simple, why this was not done in > e66f840c08a2 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")? If I compare KSZ879CLX and KSZ8873MLL datasheets, i do not see direct answer. The only reason I can imagine is the size of static MAC table. All KSZ88xx and KSZ87xx variants have only 8 entries. One is already used for STP (even if STP is not enabled, can be optimized). If BRIDGE_VLAN compiled, each local address will be configured 2 times. So, depending on system configuration the static MAC table will full very soon. I tested this patch on KSZ8873. Without this patch, if we do not send any thing from CPU port, local MAC addresses will be forgotten by the dynamic MAC table. Sending packets to a local MAC address from swp0 will flood packets to CPU and swp1. With this patch, packets fill be forwarded only to CPU - as expected. Regards, Oleksij
On Tue, Apr 04, 2023 at 02:19:11PM +0200, Oleksij Rempel wrote: > If I compare KSZ879CLX and KSZ8873MLL datasheets, i do not see direct > answer. The only reason I can imagine is the size of static MAC table. > All KSZ88xx and KSZ87xx variants have only 8 entries. One is already > used for STP (even if STP is not enabled, can be optimized). If > BRIDGE_VLAN compiled, each local address will be configured 2 times. > So, depending on system configuration the static MAC table will full > very soon. Yikes. KSZ8765 has num_statics = 8 and port_cnt = 5 (so 4 user ports I assume). So if all 4 user ports had their own MAC address, it would simply not be possible to put them under a VLAN-aware bridge, since that would consume 2 BR_FDB_LOCAL entries for each port, so the static MAC table would be full even without taking the bridge's MAC address into consideration. Even with CONFIG_BRIDGE_VLAN_FILTERING turned off or with the bridge option vlan_default_pvid = 0, this would still consume 4 BR_FDB_LOCAL entries + one for the bridge's MAC address + 1 for STP, leaving only 2 entries usable for *both* bridge fdb, *and* bridge mdb. I haven't opened the datasheets of these chips. Is it possible to use the dynamic MAC table to store static(-ish) entries?
On Tue, Apr 04, 2023 at 03:50:02PM +0300, Vladimir Oltean wrote: > On Tue, Apr 04, 2023 at 02:19:11PM +0200, Oleksij Rempel wrote: > > If I compare KSZ879CLX and KSZ8873MLL datasheets, i do not see direct > > answer. The only reason I can imagine is the size of static MAC table. > > All KSZ88xx and KSZ87xx variants have only 8 entries. One is already > > used for STP (even if STP is not enabled, can be optimized). If > > BRIDGE_VLAN compiled, each local address will be configured 2 times. > > So, depending on system configuration the static MAC table will full > > very soon. > > Yikes. KSZ8765 has num_statics = 8 and port_cnt = 5 (so 4 user ports I > assume). So if all 4 user ports had their own MAC address, it would > simply not be possible to put them under a VLAN-aware bridge, since that > would consume 2 BR_FDB_LOCAL entries for each port, so the static MAC > table would be full even without taking the bridge's MAC address into > consideration. > > Even with CONFIG_BRIDGE_VLAN_FILTERING turned off or with the bridge > option vlan_default_pvid = 0, this would still consume 4 BR_FDB_LOCAL > entries + one for the bridge's MAC address + 1 for STP, leaving only 2 > entries usable for *both* bridge fdb, *and* bridge mdb. > > I haven't opened the datasheets of these chips. Is it possible to use > the dynamic MAC table to store static(-ish) entries? According to KSZ8795CLX datasheet, dynamic MAC table is read-only. But there is Access Control Lists (ACL) with 16 entries. It is possible created a forwarding rule with match against DST MAC address. Beside, I'm working right now on KSZ9477 tc-flower support based on ACL implementation. Regards, Oleksij
diff --git a/drivers/net/dsa/microchip/ksz8.h b/drivers/net/dsa/microchip/ksz8.h index 9bb19764fa33..ad2c3a72a576 100644 --- a/drivers/net/dsa/microchip/ksz8.h +++ b/drivers/net/dsa/microchip/ksz8.h @@ -32,6 +32,10 @@ void ksz8_freeze_mib(struct ksz_device *dev, int port, bool freeze); void ksz8_port_init_cnt(struct ksz_device *dev, int port); int ksz8_fdb_dump(struct ksz_device *dev, int port, dsa_fdb_dump_cb_t *cb, void *data); +int ksz8_fdb_add(struct ksz_device *dev, int port, const unsigned char *addr, + u16 vid, struct dsa_db db); +int ksz8_fdb_del(struct ksz_device *dev, int port, const unsigned char *addr, + u16 vid, struct dsa_db db); int ksz8_mdb_add(struct ksz_device *dev, int port, const struct switchdev_obj_port_mdb *mdb, struct dsa_db db); int ksz8_mdb_del(struct ksz_device *dev, int port, diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c index 97a6c5516673..ee68e166fc44 100644 --- a/drivers/net/dsa/microchip/ksz8795.c +++ b/drivers/net/dsa/microchip/ksz8795.c @@ -1073,6 +1073,18 @@ int ksz8_mdb_del(struct ksz_device *dev, int port, return ksz8_del_sta_mac(dev, port, mdb->addr, mdb->vid); } +int ksz8_fdb_add(struct ksz_device *dev, int port, const unsigned char *addr, + u16 vid, struct dsa_db db) +{ + return ksz8_add_sta_mac(dev, port, addr, vid); +} + +int ksz8_fdb_del(struct ksz_device *dev, int port, const unsigned char *addr, + u16 vid, struct dsa_db db) +{ + return ksz8_del_sta_mac(dev, port, addr, vid); +} + int ksz8_port_vlan_filtering(struct ksz_device *dev, int port, bool flag, struct netlink_ext_ack *extack) { diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 93131347ad98..6e19ad70c671 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -200,6 +200,8 @@ static const struct ksz_dev_ops ksz8_dev_ops = { .freeze_mib = ksz8_freeze_mib, .port_init_cnt = ksz8_port_init_cnt, .fdb_dump = ksz8_fdb_dump, + .fdb_add = ksz8_fdb_add, + .fdb_del = ksz8_fdb_del, .mdb_add = ksz8_mdb_add, .mdb_del = ksz8_mdb_del, .vlan_filtering = ksz8_port_vlan_filtering,
Add support for add/del_fdb operations and utilize the refactored static MAC table code. This resolves kernel warnings caused by the lack of fdb add function support in the current driver. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz8.h | 4 ++++ drivers/net/dsa/microchip/ksz8795.c | 12 ++++++++++++ drivers/net/dsa/microchip/ksz_common.c | 2 ++ 3 files changed, 18 insertions(+)