| Message ID | 20210824165253.1691315-1-dqfext@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1ca8a193cade7f49801cc79e20d5f2a123991cdf |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: mt7530: manually set up VLAN ID 0 | 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 13 of 13 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 | warning | WARNING: line length of 82 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Wed, Aug 25, 2021 at 12:52:52AM +0800, DENG Qingfang wrote: > The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After > the blamed commit, VLAN ID 0 won't be set up anymore, breaking software > bridging fallback on VLAN-unaware bridges. > > Manually set up VLAN ID 0 to fix this. > > Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed") > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- I understand that this is how you noticed the issue, but please remember that one can always compile a kernel with CONFIG_VLAN_8021Q=n. So the issue predates my patch by much longer. You might reconsider the Fixes: tag in light of this, maybe the patch needs to be sent to stable. > drivers/net/dsa/mt7530.c | 25 +++++++++++++++++++++++++ > drivers/net/dsa/mt7530.h | 2 ++ > 2 files changed, 27 insertions(+) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index d757d9dcba51..d0cba2d1cd68 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -1599,6 +1599,21 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid, > mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, vid); > } > > +static int > +mt7530_setup_vlan0(struct mt7530_priv *priv) > +{ > + u32 val; > + > + /* Validate the entry with independent learning, keep the original > + * ingress tag attribute. > + */ > + val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) | FID_BRIDGED? > + VLAN_VALID; > + mt7530_write(priv, MT7530_VAWD1, val); > + > + return mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, 0); > +} > + > static int > mt7530_port_vlan_add(struct dsa_switch *ds, int port, > const struct switchdev_obj_port_vlan *vlan, > @@ -2174,6 +2189,11 @@ mt7530_setup(struct dsa_switch *ds) > PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); > } > > + /* Setup VLAN ID 0 for VLAN-unaware bridges */ > + ret = mt7530_setup_vlan0(priv); > + if (ret) > + return ret; > + > /* Setup port 5 */ > priv->p5_intf_sel = P5_DISABLED; > interface = PHY_INTERFACE_MODE_NA; > @@ -2346,6 +2366,11 @@ mt7531_setup(struct dsa_switch *ds) > PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); > } > > + /* Setup VLAN ID 0 for VLAN-unaware bridges */ > + ret = mt7530_setup_vlan0(priv); > + if (ret) > + return ret; > + > ds->assisted_learning_on_cpu_port = true; > ds->mtu_enforcement_ingress = true; > > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h > index fe4cd2ac26d0..91508e2feef9 100644 > --- a/drivers/net/dsa/mt7530.h > +++ b/drivers/net/dsa/mt7530.h > @@ -145,6 +145,8 @@ enum mt7530_vlan_cmd { > #define PORT_STAG BIT(31) > /* Independent VLAN Learning */ > #define IVL_MAC BIT(30) > +/* Egress Tag Consistent */ > +#define EG_CON BIT(29) > /* Per VLAN Egress Tag Control */ > #define VTAG_EN BIT(28) > /* VLAN Member Control */ > -- > 2.25.1 >
On Tue, Aug 24, 2021 at 07:57:42PM +0300, Vladimir Oltean wrote: > > +static int > > +mt7530_setup_vlan0(struct mt7530_priv *priv) > > +{ > > + u32 val; > > + > > + /* Validate the entry with independent learning, keep the original > > + * ingress tag attribute. > > + */ > > + val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) | > > FID_BRIDGED? yes, FID_BRIDGED. Please confirm that the Fixes: tag is the one you intend. The patch appears fine otherwise.
On Tue, Aug 24, 2021 at 07:57:42PM +0300, Vladimir Oltean wrote: > I understand that this is how you noticed the issue, but please remember > that one can always compile a kernel with CONFIG_VLAN_8021Q=n. So the > issue predates my patch by much longer. You might reconsider the Fixes: > tag in light of this, maybe the patch needs to be sent to stable. Okay. So the Fixes tag should be 6087175b7991, which initially adds the software fallback support for mt7530. > > > +static int > > +mt7530_setup_vlan0(struct mt7530_priv *priv) > > +{ > > + u32 val; > > + > > + /* Validate the entry with independent learning, keep the original > > + * ingress tag attribute. > > + */ > > + val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) | > > FID_BRIDGED? What's wrong with that? > > > + VLAN_VALID; > > + mt7530_write(priv, MT7530_VAWD1, val); > > + > > + return mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, 0); > > +} >
On Wed, Aug 25, 2021 at 01:32:37AM +0800, DENG Qingfang wrote: > On Tue, Aug 24, 2021 at 07:57:42PM +0300, Vladimir Oltean wrote: > > I understand that this is how you noticed the issue, but please remember > > that one can always compile a kernel with CONFIG_VLAN_8021Q=n. So the > > issue predates my patch by much longer. You might reconsider the Fixes: > > tag in light of this, maybe the patch needs to be sent to stable. > > Okay. So the Fixes tag should be 6087175b7991, which initially adds the > software fallback support for mt7530. Ok. Did the old code not need VLAN 0 for VLAN-unaware ports, or are you saying that since the VLAN table lookup was bypassed completely in the old code, 'no VLAN 0' was an inconsequential error? I think it's the latter. Just wanted to make sure. So that means, either this Fixes: tag or the other, the patch still belongs to net-next. From my side you shouldn't need to resend. Reviewed-by: Vladimir Oltean <olteanv@gmail.com> > > > +static int > > > +mt7530_setup_vlan0(struct mt7530_priv *priv) > > > +{ > > > + u32 val; > > > + > > > + /* Validate the entry with independent learning, keep the original > > > + * ingress tag attribute. > > > + */ > > > + val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) | > > > > FID_BRIDGED? > > What's wrong with that? Nothing, I had a senior moment and I forgot how mt7530 sets up things.
On 8/24/2021 6:52 PM, DENG Qingfang wrote: > The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After > the blamed commit, VLAN ID 0 won't be set up anymore, breaking software > bridging fallback on VLAN-unaware bridges. > > Manually set up VLAN ID 0 to fix this. > > Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed") > Signed-off-by: DENG Qingfang <dqfext@gmail.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On Tue, Aug 24, 2021 at 08:37:14PM +0300, Vladimir Oltean wrote: > On Wed, Aug 25, 2021 at 01:32:37AM +0800, DENG Qingfang wrote: > > Okay. So the Fixes tag should be 6087175b7991, which initially adds the > > software fallback support for mt7530. > > Ok. Did the old code not need VLAN 0 for VLAN-unaware ports, or are you > saying that since the VLAN table lookup was bypassed completely in the > old code, 'no VLAN 0' was an inconsequential error? > > I think it's the latter. Just wanted to make sure. So that means, either > this Fixes: tag or the other, the patch still belongs to net-next. From > my side you shouldn't need to resend. You're right. The old code does not use VLAN table lookup for VLAN-unaware ports, and the current code set VLAN-unaware ports to fallback mode so missing VLAN 0 will only make them fallback to SVL. > > Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Wed, 25 Aug 2021 00:52:52 +0800 you wrote: > The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After > the blamed commit, VLAN ID 0 won't be set up anymore, breaking software > bridging fallback on VLAN-unaware bridges. > > Manually set up VLAN ID 0 to fix this. > > Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed") > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > > [...] Here is the summary with links: - [net-next] net: dsa: mt7530: manually set up VLAN ID 0 https://git.kernel.org/netdev/net-next/c/1ca8a193cade You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index d757d9dcba51..d0cba2d1cd68 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -1599,6 +1599,21 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid, mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, vid); } +static int +mt7530_setup_vlan0(struct mt7530_priv *priv) +{ + u32 val; + + /* Validate the entry with independent learning, keep the original + * ingress tag attribute. + */ + val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) | + VLAN_VALID; + mt7530_write(priv, MT7530_VAWD1, val); + + return mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, 0); +} + static int mt7530_port_vlan_add(struct dsa_switch *ds, int port, const struct switchdev_obj_port_vlan *vlan, @@ -2174,6 +2189,11 @@ mt7530_setup(struct dsa_switch *ds) PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); } + /* Setup VLAN ID 0 for VLAN-unaware bridges */ + ret = mt7530_setup_vlan0(priv); + if (ret) + return ret; + /* Setup port 5 */ priv->p5_intf_sel = P5_DISABLED; interface = PHY_INTERFACE_MODE_NA; @@ -2346,6 +2366,11 @@ mt7531_setup(struct dsa_switch *ds) PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); } + /* Setup VLAN ID 0 for VLAN-unaware bridges */ + ret = mt7530_setup_vlan0(priv); + if (ret) + return ret; + ds->assisted_learning_on_cpu_port = true; ds->mtu_enforcement_ingress = true; diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h index fe4cd2ac26d0..91508e2feef9 100644 --- a/drivers/net/dsa/mt7530.h +++ b/drivers/net/dsa/mt7530.h @@ -145,6 +145,8 @@ enum mt7530_vlan_cmd { #define PORT_STAG BIT(31) /* Independent VLAN Learning */ #define IVL_MAC BIT(30) +/* Egress Tag Consistent */ +#define EG_CON BIT(29) /* Per VLAN Egress Tag Control */ #define VTAG_EN BIT(28) /* VLAN Member Control */
The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After the blamed commit, VLAN ID 0 won't be set up anymore, breaking software bridging fallback on VLAN-unaware bridges. Manually set up VLAN ID 0 to fix this. Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed") Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- drivers/net/dsa/mt7530.c | 25 +++++++++++++++++++++++++ drivers/net/dsa/mt7530.h | 2 ++ 2 files changed, 27 insertions(+)