| Message ID | 20230303214846.410414-3-jan@3e8.eu |
|---|---|
| State | Superseded |
| Delegated to: | Sander Vanheule |
| Headers | show |
| Series | realtek: fix management of mdb entries | expand |
On 2023-03-03 at 22:48, Jan Hoffmann wrote: > This function returns the index of the allocated multicast group entry, > so the return value should be negative when no entry was allocated. > > Fixes: cde31976e375 ("realtek: Add support for Layer 2 Multicast") > Signed-off-by: Jan Hoffmann <jan@3e8.eu> > --- > > I'm not entirely sure about this patch. Seen individually, changing the > return code is obviously the right thing to do. However, this entire > branch is effectively dead code at the moment, as the only caller > already performs the same check before. > > That check should probably be removed everywhere, as it doesn't really > make sense to ignore mdb entries for LAG members. But I haven't tested > that yet, because link aggregation is totally broken right now. So a > proper cleanup will have to wait until link aggregation itself is fixed > (which I'm currently working on). I just realized that "is_lagmember" is only meant to be set for non-primary LAG ports (with the first added port being chosen as primary port), so this check actually makes a bit more sense. Deferring any possible cleanup until link aggregation works at all really seems like the best choice here. > > target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c | 2 +- > target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c | 2 +- > 2 files changed, 2 insertions(+), 2 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 fee63c36faa1..695453a75862 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 > @@ -983,7 +983,7 @@ static int rtl83xx_mc_group_alloc(struct rtl838x_switch_priv *priv, int port) > > if (priv->is_lagmember[port]) { > pr_info("%s: %d is lag slave. ignore\n", __func__, port); > - return 0; > + return -1; > } > > set_bit(mc_group, priv->mc_group_bm); > diff --git a/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c b/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c > index f9980ccacee1..09fdc38b55e7 100644 > --- a/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c > +++ b/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c > @@ -970,7 +970,7 @@ static int rtl83xx_mc_group_alloc(struct rtl838x_switch_priv *priv, int port) > > if (priv->is_lagmember[port]) { > pr_info("%s: %d is lag slave. ignore\n", __func__, port); > - return 0; > + return -1; > } > > set_bit(mc_group, priv->mc_group_bm);
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 fee63c36faa1..695453a75862 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 @@ -983,7 +983,7 @@ static int rtl83xx_mc_group_alloc(struct rtl838x_switch_priv *priv, int port) if (priv->is_lagmember[port]) { pr_info("%s: %d is lag slave. ignore\n", __func__, port); - return 0; + return -1; } set_bit(mc_group, priv->mc_group_bm); diff --git a/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c b/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c index f9980ccacee1..09fdc38b55e7 100644 --- a/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c +++ b/target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c @@ -970,7 +970,7 @@ static int rtl83xx_mc_group_alloc(struct rtl838x_switch_priv *priv, int port) if (priv->is_lagmember[port]) { pr_info("%s: %d is lag slave. ignore\n", __func__, port); - return 0; + return -1; } set_bit(mc_group, priv->mc_group_bm);
This function returns the index of the allocated multicast group entry, so the return value should be negative when no entry was allocated. Fixes: cde31976e375 ("realtek: Add support for Layer 2 Multicast") Signed-off-by: Jan Hoffmann <jan@3e8.eu> --- I'm not entirely sure about this patch. Seen individually, changing the return code is obviously the right thing to do. However, this entire branch is effectively dead code at the moment, as the only caller already performs the same check before. That check should probably be removed everywhere, as it doesn't really make sense to ignore mdb entries for LAG members. But I haven't tested that yet, because link aggregation is totally broken right now. So a proper cleanup will have to wait until link aggregation itself is fixed (which I'm currently working on). target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/dsa.c | 2 +- target/linux/realtek/files-5.15/drivers/net/dsa/rtl83xx/dsa.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)