diff mbox series

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

Message ID 20210929210349.130099-3-linus.walleij@linaro.org
State New
Headers show
Series RTL8366RB enhancements | expand

Commit Message

Linus Walleij Sept. 29, 2021, 9:03 p.m. UTC 
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(-)

-- 
2.31.1

Comments

Vladimir Oltean Sept. 29, 2021, 9:57 p.m. UTC | #1 
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

>
Alvin Šipraga Sept. 30, 2021, 9:09 a.m. UTC | #2 
On 9/29/21 11:57 PM, Vladimir Oltean wrote:
> 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?


Linus, are you sure these STORM_... registers are the right ones to 
control flooding? The doc from Realtek[1] talks briefly about this storm 
control feature, but it seems to be related to rate limiting, not actual 
flooding behaviour.

Just FYI, on the RTL8365MB there are similar storm control registers, 
but the vendor driver doesn't use them to control flooding. Flooding is 
controlled by a set of different registers which allow you to (1) flood, 
(2) flood to a specified portmask, (3) drop, or (4) trap. In the vendor 
driver those registers take names like 
RTL8367C_REG_UNKNOWN_UNICAST_DA_PORT_BEHAVE to control unicast flooding 
on a per-port basis. So there might be something similar for the '66RB.

Originally I thought "storm" was Realtek slang for "flood", but it seems 
that is not the case.

[1] 
https://cdn.jsdelivr.net/gh/libc0607/Realtek_switch_hacking@files/Realtek_Unmanaged_Switch_ProgrammingGuide.pdf

> 

>> + */

>> +#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

>>

>
Linus Walleij Oct. 4, 2021, 10:22 p.m. UTC | #3 
On Thu, Sep 30, 2021 at 11:09 AM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
> On 9/29/21 11:57 PM, Vladimir Oltean wrote:

> > 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.


> >> +/* 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?

>

> Linus, are you sure these STORM_... registers are the right ones to

> control flooding? The doc from Realtek[1] talks briefly about this storm

> control feature, but it seems to be related to rate limiting, not actual

> flooding behaviour.


You're probably right. I'll just drop this patch for now.

Yours,
Linus Walleij
diff mbox series

Patch

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.
+ */
+#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;
 }