| Message ID | 20210830214859.403100-5-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | RTL8366RB improvements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | fail | ERROR: code indent should use tabs where possible WARNING: line length of 81 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: please, no spaces at the start of a line |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Mon, Aug 30, 2021 at 11:48:58PM +0200, Linus Walleij wrote: > Now that we have implemented bridge flag handling we can easily > support flood (storm) 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: DENG Qingfang <dqfext@gmail.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - New patch > --- > drivers/net/dsa/rtl8366rb.c | 38 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c > index 2cadd3e57e8b..4cb0e336ce6b 100644 > --- a/drivers/net/dsa/rtl8366rb.c > +++ b/drivers/net/dsa/rtl8366rb.c > @@ -149,6 +149,11 @@ > > #define RTL8366RB_VLAN_INGRESS_CTRL2_REG 0x037f > > +#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 > @@ -1158,7 +1163,8 @@ rtl8366rb_port_pre_bridge_flags(struct dsa_switch *ds, int port, > 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)) Spaces instead of tabs here? > return -EINVAL; > > return 0; > @@ -1180,6 +1186,36 @@ 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; > + } > + > + /* Enable/disable both types of unicast floods */ > + if (flags.mask & BR_FLOOD) { > + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNDA_CTRL, > + BIT(port), > + (flags.val & BR_FLOOD) ? BIT(port) : 0); > + if (ret) > + return ret; > + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNMC_CTRL, > + BIT(port), > + (flags.val & BR_FLOOD) ? BIT(port) : 0); > + if (ret) > + return ret; UNDA and UNMC? > + } > + > return 0; > } > > -- > 2.31.1 >
On 8/30/21 11:48 PM, Linus Walleij wrote: > Now that we have implemented bridge flag handling we can easily > support flood (storm) 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: DENG Qingfang <dqfext@gmail.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk> > ChangeLog v1->v2: > - New patch > --- > drivers/net/dsa/rtl8366rb.c | 38 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c > index 2cadd3e57e8b..4cb0e336ce6b 100644 > --- a/drivers/net/dsa/rtl8366rb.c > +++ b/drivers/net/dsa/rtl8366rb.c > @@ -149,6 +149,11 @@ > > #define RTL8366RB_VLAN_INGRESS_CTRL2_REG 0x037f > > +#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 > @@ -1158,7 +1163,8 @@ rtl8366rb_port_pre_bridge_flags(struct dsa_switch *ds, int port, > struct netlink_ext_ack *extack) > { > /* We support enabling/disabling learning */ Perhaps you can remove this comment altogether, since we support more things now, and it is self evident anyway. > - if (flags.mask & ~(BR_LEARNING)) > + if (flags.mask & ~(BR_LEARNING | BR_BCAST_FLOOD | > + BR_MCAST_FLOOD | BR_FLOOD)) > return -EINVAL; > > return 0; > @@ -1180,6 +1186,36 @@ 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; > + } > + > + /* Enable/disable both types of unicast floods */ > + if (flags.mask & BR_FLOOD) { > + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNDA_CTRL, > + BIT(port), > + (flags.val & BR_FLOOD) ? BIT(port) : 0); > + if (ret) > + return ret; > + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNMC_CTRL, > + BIT(port), > + (flags.val & BR_FLOOD) ? BIT(port) : 0); > + if (ret) > + return ret; > + } > + > return 0; > } > >
On 8/31/21 12:43 AM, Vladimir Oltean wrote: > On Mon, Aug 30, 2021 at 11:48:58PM +0200, Linus Walleij wrote: >> Now that we have implemented bridge flag handling we can easily >> support flood (storm) 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: DENG Qingfang <dqfext@gmail.com> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> >> --- >> ChangeLog v1->v2: >> - New patch >> --- >> drivers/net/dsa/rtl8366rb.c | 38 ++++++++++++++++++++++++++++++++++++- >> 1 file changed, 37 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c >> index 2cadd3e57e8b..4cb0e336ce6b 100644 >> --- a/drivers/net/dsa/rtl8366rb.c >> +++ b/drivers/net/dsa/rtl8366rb.c >> @@ -149,6 +149,11 @@ >> >> #define RTL8366RB_VLAN_INGRESS_CTRL2_REG 0x037f >> >> +#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 >> @@ -1158,7 +1163,8 @@ rtl8366rb_port_pre_bridge_flags(struct dsa_switch *ds, int port, >> 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)) > > Spaces instead of tabs here? > >> return -EINVAL; >> >> return 0; >> @@ -1180,6 +1186,36 @@ 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; >> + } >> + >> + /* Enable/disable both types of unicast floods */ >> + if (flags.mask & BR_FLOOD) { >> + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNDA_CTRL, >> + BIT(port), >> + (flags.val & BR_FLOOD) ? BIT(port) : 0); >> + if (ret) >> + return ret; >> + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNMC_CTRL, >> + BIT(port), >> + (flags.val & BR_FLOOD) ? BIT(port) : 0); >> + if (ret) >> + return ret; > > UNDA and UNMC? Ah, I was also fooled by this. UN is not unicast. It means "unknown destination address" and "unknown multicast address". So, UNMC should go under BR_MCAST_FLOOD? > >> + } >> + >> return 0; >> } >> >> -- >> 2.31.1 >> >
On 8/30/2021 2:48 PM, Linus Walleij wrote: > Now that we have implemented bridge flag handling we can easily > support flood (storm) control as well so let's do it. storm control is usually defined by switch vendors about defining an acceptable bit rate for a certain type of traffic (unicast, multicast, broadcast) whereas the flags you are controlling here are about essential bridge functional, regardless of the bitrate, I would just drop that mention of (storm) altogether in your v3.
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c index 2cadd3e57e8b..4cb0e336ce6b 100644 --- a/drivers/net/dsa/rtl8366rb.c +++ b/drivers/net/dsa/rtl8366rb.c @@ -149,6 +149,11 @@ #define RTL8366RB_VLAN_INGRESS_CTRL2_REG 0x037f +#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 @@ -1158,7 +1163,8 @@ rtl8366rb_port_pre_bridge_flags(struct dsa_switch *ds, int port, 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; @@ -1180,6 +1186,36 @@ 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; + } + + /* Enable/disable both types of unicast floods */ + if (flags.mask & BR_FLOOD) { + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNDA_CTRL, + BIT(port), + (flags.val & BR_FLOOD) ? BIT(port) : 0); + if (ret) + return ret; + ret = regmap_update_bits(smi->map, RTL8366RB_STORM_UNMC_CTRL, + BIT(port), + (flags.val & BR_FLOOD) ? BIT(port) : 0); + if (ret) + return ret; + } + return 0; }
Now that we have implemented bridge flag handling we can easily support flood (storm) 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: DENG Qingfang <dqfext@gmail.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - New patch --- drivers/net/dsa/rtl8366rb.c | 38 ++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-)