netdev - Re: [PATCH net-next 1/4 v4] net: dsa: rtl8366rb: Support disabling learning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9c620f87-884f-dd85-3d29-df8861131516@bang-olufsen.dk>
Date:   Thu, 30 Sep 2021 10:45:12 +0000
From:   Alvin Šipraga <ALSI@...g-olufsen.dk>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Mauri Sandberg <sandberg@...lfence.com>,
        DENG Qingfang <dqfext@...il.com>
Subject: Re: [PATCH net-next 1/4 v4] net: dsa: rtl8366rb: Support disabling
 learning

Hi Linus,

On 9/29/21 11:03 PM, Linus Walleij wrote:
> The RTL8366RB hardware supports disabling learning per-port
> so let's make use of this feature. Rename some unfortunately
> named registers in the process.

Since you have implemented bridge offloading and you are now disabling 
learning on the CPU port by default, will this mean that all ingress 
frames on a user port with DA behind the CPU port will be flooded by the 
switch to all ports in the bridge, as well as the CPU port? It seems 
that will be the case if now the switch can't learn the SA of frames 
coming from the CPU.

Following your discussion with Vladimir [1], did you come to a 
conclusion on how you will handle this?

	Alvin

[1] https://lore.kernel.org/netdev/20210908210939.cwwnwgj3p67qvsrh@skbuf/

> 
> Suggested-by: Vladimir Oltean <olteanv@...il.com>
> Cc: Alvin Šipraga <alsi@...g-olufsen.dk>
> Cc: Mauri Sandberg <sandberg@...lfence.com>
> Cc: Florian Fainelli <f.fainelli@...il.com>
> Cc: DENG Qingfang <dqfext@...il.com>
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---
> ChangeLog v3->v4:
> - No changes, rebased on other patches.
> ChangeLog v2->v3:
> - Disable learning by default, learning will be turned
>    on selectively using the callback.
> ChangeLog v1->v2:
> - New patch suggested by Vladimir.
> ---
>   drivers/net/dsa/rtl8366rb.c | 50 ++++++++++++++++++++++++++++++++-----
>   1 file changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
> index bb9d017c2f9f..b3056064b937 100644
> --- a/drivers/net/dsa/rtl8366rb.c
> +++ b/drivers/net/dsa/rtl8366rb.c
> @@ -14,6 +14,7 @@
>   
>   #include <linux/bitops.h>
>   #include <linux/etherdevice.h>
> +#include <linux/if_bridge.h>
>   #include <linux/interrupt.h>
>   #include <linux/irqdomain.h>
>   #include <linux/irqchip/chained_irq.h>
> @@ -42,9 +43,12 @@
>   /* Port Enable Control register */
>   #define RTL8366RB_PECR				0x0001
>   
> -/* Switch Security Control registers */
> -#define RTL8366RB_SSCR0				0x0002
> -#define RTL8366RB_SSCR1				0x0003
> +/* Switch per-port learning disablement register */
> +#define RTL8366RB_PORT_LEARNDIS_CTRL		0x0002
> +
> +/* Security control, actually aging register */
> +#define RTL8366RB_SECURITY_CTRL			0x0003
> +
>   #define RTL8366RB_SSCR2				0x0004
>   #define RTL8366RB_SSCR2_DROP_UNKNOWN_DA		BIT(0)
>   
> @@ -927,13 +931,14 @@ static int rtl8366rb_setup(struct dsa_switch *ds)
>   		/* layer 2 size, see rtl8366rb_change_mtu() */
>   		rb->max_mtu[i] = 1532;
>   
> -	/* Enable learning for all ports */
> -	ret = regmap_write(smi->map, RTL8366RB_SSCR0, 0);
> +	/* Disable learning for all ports */
> +	ret = regmap_write(smi->map, RTL8366RB_PORT_LEARNDIS_CTRL,
> +			   RTL8366RB_PORT_ALL);
>   	if (ret)
>   		return ret;
>   
>   	/* Enable auto ageing for all ports */
> -	ret = regmap_write(smi->map, RTL8366RB_SSCR1, 0);
> +	ret = regmap_write(smi->map, RTL8366RB_SECURITY_CTRL, 0);
>   	if (ret)
>   		return ret;
>   
> @@ -1272,6 +1277,37 @@ static int rtl8366rb_vlan_filtering(struct dsa_switch *ds, int port,
>   	return ret;
>   }
>   
> +static int
> +rtl8366rb_port_pre_bridge_flags(struct dsa_switch *ds, int port,
> +				struct switchdev_brport_flags flags,
> +				struct netlink_ext_ack *extack)
> +{
> +	/* We support enabling/disabling learning */
> +	if (flags.mask & ~(BR_LEARNING))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int
> +rtl8366rb_port_bridge_flags(struct dsa_switch *ds, int port,
> +			    struct switchdev_brport_flags flags,
> +			    struct netlink_ext_ack *extack)
> +{
> +	struct realtek_smi *smi = ds->priv;
> +	int ret;
> +
> +	if (flags.mask & BR_LEARNING) {
> +		ret = regmap_update_bits(smi->map, RTL8366RB_PORT_LEARNDIS_CTRL,
> +					 BIT(port),
> +					 (flags.val & BR_LEARNING) ? 0 : BIT(port));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static int rtl8366rb_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
>   {
>   	struct realtek_smi *smi = ds->priv;
> @@ -1682,6 +1718,8 @@ static const struct dsa_switch_ops rtl8366rb_switch_ops = {
>   	.port_vlan_del = rtl8366_vlan_del,
>   	.port_enable = rtl8366rb_port_enable,
>   	.port_disable = rtl8366rb_port_disable,
> +	.port_pre_bridge_flags = rtl8366rb_port_pre_bridge_flags,
> +	.port_bridge_flags = rtl8366rb_port_bridge_flags,
>   	.port_change_mtu = rtl8366rb_change_mtu,
>   	.port_max_mtu = rtl8366rb_max_mtu,
>   };
>