[PATCH 2/6] realtek: don't return 0 from rtl83xx_mc_group_alloc on failure

Jan Hoffmann jan at 3e8.eu
Fri Mar 3 16:05:05 PST 2023


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 at 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);



More information about the openwrt-devel mailing list