Re: [PATCH net-next 2/4 v4] net: dsa: rtl8366rb: Support flood control

Vladimir Oltean
2021-09-29T21:57:56Z

On Wed, Sep 29, 2021 at 11:03:47PM +0200, Linus Walleij wrote:
> Now that we have implemented bridge flag handling we can easily
> support flood control as well so let's do it.
>
> Cc: Vladimir Oltean <olteanv@gmail.com>
> Cc: Alvin Šipraga <alsi@bang-olufsen.dk>
> Cc: Mauri Sandberg <sandberg@mailfence.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: DENG Qingfang <dqfext@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v3->v4:
> - No changes, rebased on the other patches.
> ChangeLog v2->v3:
> - Move the UNMC under the multicast setting as it is related to
> multicast to unknown address.
> - Add some more registers from the API, unfortunately we don't
> know how to make use of them.
> - Use tabs for indentation in copypaste bug.
> - Since we don't know how to make the elaborate storm control
> work just mention flood control in the message.
> ChangeLog v1->v2:
> - New patch
> ---
> drivers/net/dsa/rtl8366rb.c | 55 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
> index b3056064b937..52e750ea790e 100644
> --- a/drivers/net/dsa/rtl8366rb.c
> +++ b/drivers/net/dsa/rtl8366rb.c
> @@ -164,6 +164,26 @@
> */
> #define RTL8366RB_VLAN_INGRESS_CTRL2_REG 0x037f
>
> +/* Storm registers are for flood control
> + *
> + * 02e2 and 02e3 are defined in the header for the RTL8366RB API
> + * but there are no usage examples. The implementation only activates
> + * the filter per port in the CTRL registers.

The "filter" word bothers me a bit.
Are these settings applied on ingress or on egress? If you have
RTL8366RB_STORM_BC_CTRL == BIT(0) | BIT(1), and a broadcast packet is
received on port 2, then

(a) is it received or dropped?
(b) is it forwarded to port 0 and 1?
(c) is it forwarded to port 3?

> + */
> +#define RTL8366RB_STORM_FILTERING_1_REG 0x02e2
> +#define RTL8366RB_STORM_FILTERING_PERIOD_BIT BIT(0)
> +#define RTL8366RB_STORM_FILTERING_PERIOD_MSK GENMASK(1, 0)
> +#define RTL8366RB_STORM_FILTERING_COUNT_BIT BIT(1)
> +#define RTL8366RB_STORM_FILTERING_COUNT_MSK GENMASK(3, 2)
> +#define RTL8366RB_STORM_FILTERING_BC_BIT BIT(5)
> +#define RTL8366RB_STORM_FILTERING_2_REG 0x02e3
> +#define RTL8366RB_STORM_FILTERING_MC_BIT BIT(0)
> +#define RTL8366RB_STORM_FILTERING_UNDA_BIT BIT(5)
> +#define RTL8366RB_STORM_BC_CTRL 0x03e0
> +#define RTL8366RB_STORM_MC_CTRL 0x03e1
> +#define RTL8366RB_STORM_UNDA_CTRL 0x03e2
> +#define RTL8366RB_STORM_UNMC_CTRL 0x03e3
> +
> /* LED control registers */
> #define RTL8366RB_LED_BLINKRATE_REG 0x0430
> #define RTL8366RB_LED_BLINKRATE_MASK 0x0007
> @@ -1282,8 +1302,8 @@ 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))
> + if (flags.mask & ~(BR_LEARNING | BR_BCAST_FLOOD |
> + BR_MCAST_FLOOD | BR_FLOOD))
> return -EINVAL;
>
> return 0;
> @@ -1305,6 +1325,37 @@ rtl8366rb_port_bridge_flags(struct dsa_switch *ds, int port,
> return ret;
> }
>
> + if (flags.mask & BR_BCAST_FLOOD) {
> + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_BC_CTRL,
> + BIT(port),
> + (flags.val & BR_BCAST_FLOOD) ? BIT(port) : 0);
> + if (ret)
> + return ret;
> + }
> +
> + if (flags.mask & BR_MCAST_FLOOD) {
> + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_MC_CTRL,
> + BIT(port),
> + (flags.val & BR_MCAST_FLOOD) ? BIT(port) : 0);
> + if (ret)
> + return ret;
> + /* UNMC = Unknown multicast address */
> + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNMC_CTRL,
> + BIT(port),
> + (flags.val & BR_FLOOD) ? BIT(port) : 0);
> + if (ret)
> + return ret;
> + }
> +
> + if (flags.mask & BR_FLOOD) {
> + /* UNDA = Unknown destination address */
> + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNDA_CTRL,
> + BIT(port),
> + (flags.val & BR_FLOOD) ? BIT(port) : 0);
> + if (ret)
> + return ret;
> + }
> +
> return 0;
> }
>
> --
> 2.31.1
>