2024-04-03 12:51:16

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v2 3/8] net: dsa: microchip: ksz8: Refactor ksz8_fdb_dump()

Refactor ksz8_fdb_dump() to address potential issues:
- Limit the number of iterations to avoid endless loops.
- Handle error codes returned by ksz8_r_dyn_mac_table(), with
an exception for -ENXIO when no more dynamic entries are detected.

Reviewed-by: Vladimir Oltean <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
changes v2:
- move variable declaration out of "for" scope
- rework "if (port != src_port); continue" to to "if (port == src_port);
code"
---
drivers/net/dsa/microchip/ksz8795.c | 29 ++++++++++++++-----------
drivers/net/dsa/microchip/ksz8795_reg.h | 1 +
2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index e407111db6637..c93eb351ab3d5 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1191,27 +1191,30 @@ void ksz8_flush_dyn_mac_table(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 ret = 0;
- u16 i = 0;
- u16 entries = 0;
- u8 fid;
- u8 src_port;
u8 mac[ETH_ALEN];
+ u8 src_port, fid;
+ u16 entries = 0;
+ int ret, i;

- do {
+ for (i = 0; i < KSZ8_DYN_MAC_ENTRIES; i++) {
ret = ksz8_r_dyn_mac_table(dev, i, mac, &fid, &src_port,
&entries);
- if (!ret && port == src_port) {
+ if (ret == -ENXIO)
+ return 0;
+ if (ret)
+ return ret;
+
+ if (i >= entries)
+ return 0;
+
+ if (port == src_port) {
ret = cb(mac, fid, false, data);
if (ret)
- break;
+ return ret;
}
- i++;
- } while (i < entries);
- if (i >= entries)
- ret = 0;
+ }

- return ret;
+ return 0;
}

static int ksz8_add_sta_mac(struct ksz_device *dev, int port,
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 7c9341ef73b03..0d13a6e29b0e6 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -794,5 +794,6 @@
#define TAIL_TAG_LOOKUP BIT(7)

#define FID_ENTRIES 128
+#define KSZ8_DYN_MAC_ENTRIES 1024

#endif
--
2.39.2



2024-04-03 13:03:43

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v2 3/8] net: dsa: microchip: ksz8: Refactor ksz8_fdb_dump()



On 4/3/2024 5:50 AM, Oleksij Rempel wrote:
> Refactor ksz8_fdb_dump() to address potential issues:
> - Limit the number of iterations to avoid endless loops.
> - Handle error codes returned by ksz8_r_dyn_mac_table(), with
> an exception for -ENXIO when no more dynamic entries are detected.
>
> Reviewed-by: Vladimir Oltean <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2024-04-04 03:19:06

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [PATCH net-next v2 3/8] net: dsa: microchip: ksz8: Refactor ksz8_fdb_dump()

Hi Oleksij,

On Wed, 2024-04-03 at 14:50 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Refactor ksz8_fdb_dump() to address potential issues:
> - Limit the number of iterations to avoid endless loops.
> - Handle error codes returned by ksz8_r_dyn_mac_table(), with
> an exception for -ENXIO when no more dynamic entries are detected.
>
> Reviewed-by: Vladimir Oltean <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>

I believe Signed-off-by tag should be first followed by the
Acked/reviewed tag in the order who reviewed it to indicate
chronological order. Correct me if I am wrong.

Acked-by: Arun Ramadoss <[email protected]>