Re: [PATCH v3 net-next 15/24] net: dsa: sja1105: Add support for FDB and MDB management

[Jiri Pirko]

Sat, Apr 13, 2019 at 03:28:13AM CEST, olteanv@xxxxxxxxx wrote:
>Currently only the (more difficult) first generation E/T series is
>supported. Here the TCAM is only 4-way associative, and to know where
>the hardware will search for a FDB entry, we need to perform the same
>hash algorithm in order to install the entry in the correct bin.
>
>On P/Q/R/S, the TCAM should be fully associative. However the SPI
>command interface is different, and because I don't have access to a
>new-generation device at the moment, support for it is TODO.
>
>Signed-off-by: Vladimir Oltean <olteanv@xxxxxxxxx>
>Reviewed-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
>---
>Changes from v3:
>None
>
>Changes from v2:
>None
>
> drivers/net/dsa/sja1105/sja1105.h             |   2 +
> .../net/dsa/sja1105/sja1105_dynamic_config.c  |  40 ++++
> drivers/net/dsa/sja1105/sja1105_main.c        | 193 ++++++++++++++++++
> 3 files changed, 235 insertions(+)
>
>diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
>index ef555dd385a3..4c9df44a4478 100644
>--- a/drivers/net/dsa/sja1105/sja1105.h
>+++ b/drivers/net/dsa/sja1105/sja1105.h
>@@ -129,6 +129,8 @@ int sja1105_dynamic_config_write(struct sja1105_private *priv,
> 				 enum sja1105_blk_idx blk_idx,
> 				 int index, void *entry, bool keep);
> 
>+u8 sja1105_fdb_hash(struct sja1105_private *priv, const u8 *addr, u16 vid);
>+
> /* Common implementations for the static and dynamic configs */
> size_t sja1105_l2_forwarding_entry_packing(void *buf, void *entry_ptr,
> 					   enum packing_op op);
>diff --git a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
>index 74c3a00d453c..0aeda6868c27 100644
>--- a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
>+++ b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
>@@ -462,3 +462,43 @@ int sja1105_dynamic_config_write(struct sja1105_private *priv,
> 
> 	return 0;
> }
>+
>+static u8 crc8_add(u8 crc, u8 byte, u8 poly)

Prefix please.


>+{
>+	int i;
>+
>+	for (i = 0; i < 8; i++) {
>+		if ((crc ^ byte) & (1 << 7)) {
>+			crc <<= 1;
>+			crc ^= poly;
>+		} else {
>+			crc <<= 1;
>+		}
>+		byte <<= 1;
>+	}
>+	return crc;
>+}

[...]


> 
>+#define fdb(bin, index) \
>+	((bin) * SJA1105ET_FDB_BIN_SIZE + (index))
>+#define is_bin_index_valid(i) \
>+	((i) >= 0 && (i) < SJA1105ET_FDB_BIN_SIZE)

Please use prefixes and sane names. Also, consider using functions
instead of macros.


[...]


>+
>+#undef fdb
>+#undef is_bin_index_valid

Odd.