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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 13 Apr 2019 22:58:23 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     f.fainelli@...il.com, vivien.didelot@...il.com, andrew@...n.ch,
        davem@...emloft.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, georg.waibel@...sor-technik.de
Subject: Re: [PATCH v3 net-next 15/24] net: dsa: sja1105: Add support for FDB
 and MDB management

Sat, Apr 13, 2019 at 03:28:13AM CEST, olteanv@...il.com 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@...il.com>
>Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
>---
>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.