Re: [PATCH net-next 4/6 v6] net: dsa: rtl8366rb: Fix off-by-one bug

[Vladimir Oltean <olteanv@gmail.com>]

On Sat, Sep 25, 2021 at 03:23:09PM +0200, Linus Walleij wrote:
> The max VLAN number with non-4K VLAN activated is 15, and the
> range is 0..15. Not 16.
> 
> The impact should be low since we by default have 4K VLAN and
> thus have 4095 VLANs to play with in this switch. There will
> not be a problem unless the code is rewritten to only use
> 16 VLANs.
> 
> Fixes: d8652956cf37 ("net: dsa: realtek-smi: Add Realtek SMI driver")
> Cc: Vladimir Oltean <olteanv@gmail.com>
> Cc: Mauri Sandberg <sandberg@mailfence.com>
> Cc: Alvin Šipraga <alsi@bang-olufsen.dk>
> Cc: DENG Qingfang <dqfext@gmail.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v5->v6:
> - No changes just resending with the rest of the
>   patches.
> ChangeLog v4->v5:
> - Add some more text describing that this is not a critical bug.
> - Add Fixes tag
> ChangeLog v1->v4:
> - New patch for a bug discovered when fixing the other issues.
> ---
>  drivers/net/dsa/rtl8366rb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
> index 2c66a0c2ee50..6f25ee57069d 100644
> --- a/drivers/net/dsa/rtl8366rb.c
> +++ b/drivers/net/dsa/rtl8366rb.c
> @@ -1450,7 +1450,7 @@ static int rtl8366rb_set_mc_index(struct realtek_smi *smi, int port, int index)
>  
>  static bool rtl8366rb_is_vlan_valid(struct realtek_smi *smi, unsigned int vlan)
>  {
> -	unsigned int max = RTL8366RB_NUM_VLANS;
> +	unsigned int max = RTL8366RB_NUM_VLANS - 1;
>  
>  	if (smi->vlan4k_enabled)
>  		max = RTL8366RB_NUM_VIDS - 1;

Personally I would have preferred to make this "max" = RTL8366RB_NUM_VIDS,
and then the comparison right below:

	if (vlan >= max)
		return false;

because it's easier than to carry around "- 1" everywhere.
Anyway, this works just as well.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>