diff mbox series

[1/2] realtek: set up L2 table entries properly

Message ID 20221021215427.3719844-2-jan@3e8.eu
State Superseded
Delegated to: Sander Vanheule
Headers show
Series realtek: fix L2 entry setup and learning on CPU port | expand

Commit Message

Jan Hoffmann Oct. 21, 2022, 9:54 p.m. UTC
Initialize the data structure using memset to avoid the possibility of
writing garbage values to the hardware.

Always set a valid entry type, which should fix writing unicast entries
on RTL930x.

For unicast entries, set the is_static flag to prevent the switch from
aging them out.

Also set the rvid field for unicast entries. This is not strictly
necessary, as the switch fills it in automatically from a non-zero vid.
However, this makes the code consistent with multicast entry setup.

While at it, reorder the statements and fix some style issues (double
space, comma instead of semicolon at end of statement). Also remove the
unneeded priv parameter and debug print for the multicast entry setup
function.

Fixes: cde31976e37 ("realtek: Add support for Layer 2 Multicast")
Signed-off-by: Jan Hoffmann <jan@3e8.eu>
---
 .../files-5.10/drivers/net/dsa/rtl83xx/dsa.c  | 29 ++++++++++++-------
 1 file changed, 19 insertions(+), 10 deletions(-)

Comments

Sander Vanheule Oct. 23, 2022, 6:24 p.m. UTC | #1
Hi Jan,

On Fri, 2022-10-21 at 23:54 +0200, Jan Hoffmann wrote:
> Initialize the data structure using memset to avoid the possibility of
> writing garbage values to the hardware.
> 
> Always set a valid entry type, which should fix writing unicast entries
> on RTL930x.
> 
> For unicast entries, set the is_static flag to prevent the switch from
> aging them out.
> 
> Also set the rvid field for unicast entries. This is not strictly
> necessary, as the switch fills it in automatically from a non-zero vid.
> However, this makes the code consistent with multicast entry setup.
> 
> While at it, reorder the statements and fix some style issues (double
> space, comma instead of semicolon at end of statement). Also remove the
> unneeded priv parameter and debug print for the multicast entry setup
> function.
> 
> Fixes: cde31976e37 ("realtek: Add support for Layer 2 Multicast")
> Signed-off-by: Jan Hoffmann <jan@3e8.eu>
> ---
>  .../files-5.10/drivers/net/dsa/rtl83xx/dsa.c  | 29 ++++++++++++-------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c
> b/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c
> index 3ff1a96ed680..45b489afa124 100644
> --- a/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c
> +++ b/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c
> @@ -1559,23 +1559,32 @@ static void dump_l2_entry(struct rtl838x_l2_entry *e)
>  
>  static void rtl83xx_setup_l2_uc_entry(struct rtl838x_l2_entry *e, int port,
> int vid, u64 mac)
>  {
> -       e->is_ip_mc = e->is_ipv6_mc  = false;
> +       memset(e, 0, sizeof(struct rtl838x_l2_entry));

memset(e, 0, sizeof(*e)) doesn't require one to edit this line if the struct is
renamed. memset(var, 0, sizeof(struct var_type)) also appears to be less popular
(in the kernel) from a quick grep, but still widely used. Fine either way, I
suppose.

Otherwise this patch looks good to me.

Best,
Sander
diff mbox series

Patch

diff --git a/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c b/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c
index 3ff1a96ed680..45b489afa124 100644
--- a/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c
+++ b/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c
@@ -1559,23 +1559,32 @@  static void dump_l2_entry(struct rtl838x_l2_entry *e)
 
 static void rtl83xx_setup_l2_uc_entry(struct rtl838x_l2_entry *e, int port, int vid, u64 mac)
 {
-	e->is_ip_mc = e->is_ipv6_mc  = false;
+	memset(e, 0, sizeof(struct rtl838x_l2_entry));
+
+	e->type = L2_UNICAST;
 	e->valid = true;
+
 	e->age = 3;
-	e->port = port,
-	e->vid = vid;
+	e->is_static = true;
+
+	e->port = port;
+
+	e->rvid = e->vid = vid;
+	e->is_ip_mc = e->is_ipv6_mc = false;
 	u64_to_ether_addr(mac, e->mac);
 }
 
-static void rtl83xx_setup_l2_mc_entry(struct rtl838x_switch_priv *priv,
-				      struct rtl838x_l2_entry *e, int vid, u64 mac, int mc_group)
+static void rtl83xx_setup_l2_mc_entry(struct rtl838x_l2_entry *e, int vid, u64 mac, int mc_group)
 {
-	e->is_ip_mc = e->is_ipv6_mc  = false;
+	memset(e, 0, sizeof(struct rtl838x_l2_entry));
+
+	e->type = L2_MULTICAST;
 	e->valid = true;
+
 	e->mc_portmask_index = mc_group;
-	e->type = L2_MULTICAST;
+
 	e->rvid = e->vid = vid;
-	pr_debug("%s: vid: %d, rvid: %d\n", __func__, e->vid, e->rvid);
+	e->is_ip_mc = e->is_ipv6_mc = false;
 	u64_to_ether_addr(mac, e->mac);
 }
 
@@ -1815,7 +1824,7 @@  static void rtl83xx_port_mdb_add(struct dsa_switch *ds, int port,
 				err = -ENOTSUPP;
 				goto out;
 			}
-			rtl83xx_setup_l2_mc_entry(priv, &e, vid, mac, mc_group);
+			rtl83xx_setup_l2_mc_entry(&e, vid, mac, mc_group);
 			priv->r->write_l2_entry_using_hash(idx >> 2, idx & 0x3, &e);
 		}
 		goto out;
@@ -1836,7 +1845,7 @@  static void rtl83xx_port_mdb_add(struct dsa_switch *ds, int port,
 				err = -ENOTSUPP;
 				goto out;
 			}
-			rtl83xx_setup_l2_mc_entry(priv, &e, vid, mac, mc_group);
+			rtl83xx_setup_l2_mc_entry(&e, vid, mac, mc_group);
 			priv->r->write_cam(idx, &e);
 		}
 		goto out;