| Message ID | 20220729151733.6032-5-arun.ramadoss@microchip.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: vlan configuration for bridge_vlan_unaware ports | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 11 of 11 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 314 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Fri, Jul 29, 2022 at 08:47:33PM +0530, Arun Ramadoss wrote: > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c > index 516fb9d35c87..8a5583b1f2f4 100644 > --- a/drivers/net/dsa/microchip/ksz_common.c > +++ b/drivers/net/dsa/microchip/ksz_common.c > @@ -161,6 +161,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = { > .vlan_filtering = ksz8_port_vlan_filtering, > .vlan_add = ksz8_port_vlan_add, > .vlan_del = ksz8_port_vlan_del, > + .drop_untagged = ksz8_port_enable_pvid, You'll have to explain this one. What impact does PVID insertion on KSZ8 have upon dropping/not dropping untagged packets? This patch is saying that when untagged packets should be dropped, PVID insertion should be enabled, and when untagged packets should be accepted, PVID insertion should be disabled. How come? > .mirror_add = ksz8_port_mirror_add, > .mirror_del = ksz8_port_mirror_del, > .get_caps = ksz8_get_caps, > @@ -187,6 +188,7 @@ static const struct ksz_dev_ops ksz9477_dev_ops = { > .vlan_filtering = ksz9477_port_vlan_filtering, > .vlan_add = ksz9477_port_vlan_add, > .vlan_del = ksz9477_port_vlan_del, > + .drop_untagged = ksz9477_port_drop_untagged, > .mirror_add = ksz9477_port_mirror_add, > .mirror_del = ksz9477_port_mirror_del, > .get_caps = ksz9477_get_caps, > @@ -220,6 +222,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = { > .vlan_filtering = ksz9477_port_vlan_filtering, > .vlan_add = ksz9477_port_vlan_add, > .vlan_del = ksz9477_port_vlan_del, > + .drop_untagged = ksz9477_port_drop_untagged, > .mirror_add = ksz9477_port_mirror_add, > .mirror_del = ksz9477_port_mirror_del, > .get_caps = lan937x_phylink_get_caps, > @@ -1254,6 +1257,9 @@ static int ksz_enable_port(struct dsa_switch *ds, int port, > { > struct ksz_device *dev = ds->priv; > > + dev->dev_ops->vlan_add(dev, port, KSZ_DEFAULT_VLAN, > + BRIDGE_VLAN_INFO_UNTAGGED); > + How many times can this be executed before the VLAN add operation fails due to the VLAN already being present on the port? I notice you're ignoring the return code. Wouldn't it be better to do this at port_setup() time instead? (side note, the PVID for standalone mode can be added at port_setup time. The PVID to use for bridges in VLAN-unaware mode can be allocated at port_bridge_join time) > if (!dsa_is_user_port(ds, port)) > return 0; > > +static int ksz_commit_pvid(struct dsa_switch *ds, int port) > +{ > + struct dsa_port *dp = dsa_to_port(ds, port); > + struct net_device *br = dsa_port_bridge_dev_get(dp); > + struct ksz_device *dev = ds->priv; > + u16 pvid = KSZ_DEFAULT_VLAN; > + bool drop_untagged = false; > + struct ksz_port *p; > + > + p = &dev->ports[port]; > + > + if (br && br_vlan_enabled(br)) { > + pvid = p->bridge_pvid.vid; > + drop_untagged = !p->bridge_pvid.valid; > + } This is better in the sense that it resolves the need for the configure_vlan_while_not_filtering hack. But standalone and VLAN-unaware bridge ports still share the same PVID. Even more so, standalone ports have address learning enabled, which will poison the address database of VLAN-unaware bridge ports (and of other standalone ports): https://patchwork.kernel.org/project/netdevbpf/patch/20220802002636.3963025-1-vladimir.oltean@nxp.com/ Are you going to do further work in this area? > + > + ksz_set_pvid(dev, port, pvid); > + > + if (dev->dev_ops->drop_untagged) > + dev->dev_ops->drop_untagged(dev, port, drop_untagged); > + > + return 0; > +}
On Tue, 2022-08-02 at 13:59 +0300, Vladimir Oltean wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Fri, Jul 29, 2022 at 08:47:33PM +0530, Arun Ramadoss wrote: > > diff --git a/drivers/net/dsa/microchip/ksz_common.c > > b/drivers/net/dsa/microchip/ksz_common.c > > index 516fb9d35c87..8a5583b1f2f4 100644 > > --- a/drivers/net/dsa/microchip/ksz_common.c > > +++ b/drivers/net/dsa/microchip/ksz_common.c > > @@ -161,6 +161,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = > > { > > .vlan_filtering = ksz8_port_vlan_filtering, > > .vlan_add = ksz8_port_vlan_add, > > .vlan_del = ksz8_port_vlan_del, > > + .drop_untagged = ksz8_port_enable_pvid, > > You'll have to explain this one. What impact does PVID insertion on > KSZ8 > have upon dropping/not dropping untagged packets? This patch is > saying > that when untagged packets should be dropped, PVID insertion should > be > enabled, and when untagged packets should be accepted, PVID insertion > should be disabled. How come? Its my mistake. I referred KSZ87xx datasheet but I couldn't find the register for the dropping the untagged packet. If that is the case, shall I remove the dropping of untagged packet feature from the ksz8 switches? > > > .mirror_add = ksz8_port_mirror_add, > > .mirror_del = ksz8_port_mirror_del, > > .get_caps = ksz8_get_caps, > > @@ -187,6 +188,7 @@ static const struct ksz_dev_ops ksz9477_dev_ops > > = { > > .vlan_filtering = ksz9477_port_vlan_filtering, > > .vlan_add = ksz9477_port_vlan_add, > > .vlan_del = ksz9477_port_vlan_del, > > + .drop_untagged = ksz9477_port_drop_untagged, > > .mirror_add = ksz9477_port_mirror_add, > > .mirror_del = ksz9477_port_mirror_del, > > .get_caps = ksz9477_get_caps, > > @@ -220,6 +222,7 @@ static const struct ksz_dev_ops lan937x_dev_ops > > = { > > .vlan_filtering = ksz9477_port_vlan_filtering, > > .vlan_add = ksz9477_port_vlan_add, > > .vlan_del = ksz9477_port_vlan_del, > > + .drop_untagged = ksz9477_port_drop_untagged, > > .mirror_add = ksz9477_port_mirror_add, > > .mirror_del = ksz9477_port_mirror_del, > > .get_caps = lan937x_phylink_get_caps, > > @@ -1254,6 +1257,9 @@ static int ksz_enable_port(struct dsa_switch > > *ds, int port, > > { > > struct ksz_device *dev = ds->priv; > > > > + dev->dev_ops->vlan_add(dev, port, KSZ_DEFAULT_VLAN, > > + BRIDGE_VLAN_INFO_UNTAGGED); > > + > > How many times can this be executed before the VLAN add operation > fails > due to the VLAN already being present on the port? I notice you're > ignoring the return code. Wouldn't it be better to do this at > port_setup() time instead? > > (side note, the PVID for standalone mode can be added at port_setup > time. The PVID to use for bridges in VLAN-unaware mode can be > allocated > at port_bridge_join time) Ok. I will add in port_bridge_join function for bridge vlan_aware ports. > > > if (!dsa_is_user_port(ds, port)) > > return 0; > > > > +static int ksz_commit_pvid(struct dsa_switch *ds, int port) > > +{ > > + struct dsa_port *dp = dsa_to_port(ds, port); > > + struct net_device *br = dsa_port_bridge_dev_get(dp); > > + struct ksz_device *dev = ds->priv; > > + u16 pvid = KSZ_DEFAULT_VLAN; > > + bool drop_untagged = false; > > + struct ksz_port *p; > > + > > + p = &dev->ports[port]; > > + > > + if (br && br_vlan_enabled(br)) { > > + pvid = p->bridge_pvid.vid; > > + drop_untagged = !p->bridge_pvid.valid; > > + } > > This is better in the sense that it resolves the need for the > configure_vlan_while_not_filtering hack. But standalone and VLAN- > unaware > bridge ports still share the same PVID. Even more so, standalone > ports > have address learning enabled, which will poison the address database > of > VLAN-unaware bridge ports (and of other standalone ports): > https://patchwork.kernel.org/project/netdevbpf/patch/20220802002636.3963025-1-vladimir.oltean@nxp.com/ > > Are you going to do further work in this area? For now, I thought I can fix the issue for bridge vlan unaware port. I have few other patch series to be submitted like gPTP, tc commands. If standalone port fix also needed for your patch series I can work on it otherwise I can take up later stage. > > > + > > + ksz_set_pvid(dev, port, pvid); > > + > > + if (dev->dev_ops->drop_untagged) > > + dev->dev_ops->drop_untagged(dev, port, > > drop_untagged); > > + > > + return 0; > > +}
On Tue, Aug 02, 2022 at 02:40:09PM +0000, Arun.Ramadoss@microchip.com wrote: > On Tue, 2022-08-02 at 13:59 +0300, Vladimir Oltean wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > know the content is safe > > > > On Fri, Jul 29, 2022 at 08:47:33PM +0530, Arun Ramadoss wrote: > > > diff --git a/drivers/net/dsa/microchip/ksz_common.c > > > b/drivers/net/dsa/microchip/ksz_common.c > > > index 516fb9d35c87..8a5583b1f2f4 100644 > > > --- a/drivers/net/dsa/microchip/ksz_common.c > > > +++ b/drivers/net/dsa/microchip/ksz_common.c > > > @@ -161,6 +161,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = > > > { > > > .vlan_filtering = ksz8_port_vlan_filtering, > > > .vlan_add = ksz8_port_vlan_add, > > > .vlan_del = ksz8_port_vlan_del, > > > + .drop_untagged = ksz8_port_enable_pvid, > > > > You'll have to explain this one. What impact does PVID insertion on KSZ8 > > have upon dropping/not dropping untagged packets? This patch is saying > > that when untagged packets should be dropped, PVID insertion should be > > enabled, and when untagged packets should be accepted, PVID insertion > > should be disabled. How come? > > Its my mistake. I referred KSZ87xx datasheet but I couldn't find the > register for the dropping the untagged packet. If that is the case, > shall I remove the dropping of untagged packet feature from the ksz8 > switches? You'll have to see how KSZ8 behaves when the ingress port is configured with a PVID (through REG_PORT_CTRL_VID) which isn't present in the VLAN table. If untagged packets are dropped, that's your "drop untagged" setting. Some other switches will not do this, and accept untagged packets even if the VLAN table doesn't contain an entry for the PVID (or doesn't have this port as a member of that VLAN), but have a separate knob for dropping untagged traffic. > > This is better in the sense that it resolves the need for the > > configure_vlan_while_not_filtering hack. But standalone and VLAN-unaware > > bridge ports still share the same PVID. Even more so, standalone ports > > have address learning enabled, which will poison the address database of > > VLAN-unaware bridge ports (and of other standalone ports): > > > https://patchwork.kernel.org/project/netdevbpf/patch/20220802002636.3963025-1-vladimir.oltean@nxp.com/ > > > > Are you going to do further work in this area? > > For now, I thought I can fix the issue for bridge vlan unaware port. I > have few other patch series to be submitted like gPTP, tc commands. If > standalone port fix also needed for your patch series I can work on it > otherwise I can take up later stage. I think the most imperative thing for you to do is to make sure you are not introducing regressions with the port default VID change - this can be done by running the bridge related selftests (and making them pass). Something which I forgot to mention is that normally, I'd expect a change of VLAN-unaware PVID to also need a change in the way that VLAN-unaware FDB entries are added (other drivers need to remap vid=0 from port_fdb_add() to the PVID that they use for that VLAN-unaware bridge, in your case 4095, for those FDB entries to continue matching properly). However, I see that currently, ksz9477_fdb_add() sets the "USE FID" bit only for VLAN-aware FDB entries (vid != 0), which leaves me with more questions than answers. It isn't very well explained what it means to not use FID: let's say there are 2 entries in the static address table, one has "USE FID"=false, and the other has "USE FID"=true and FID=127, and a packet is received which is classified to FID 127. On which entry will this packet match? The bridge driver gives you all FDB entries at once (VLAN-aware and VLAN-unaware), so if the USE_FID=false entries that the ksz9477 driver uses for VLAN-unaware mode will shadow the VLAN-aware FDB entries, this is going to be a problem. Also, the way in which the ksz9477 driver translates a 12-bit VID into a 7-bit FID also has me incredibly confused (FID is vlan->vid & VLAN_FID_M, or otherwise said, a simple truncation). This means that your VLAN-unaware PVID of 4095 uses a FID of 127, which is also the same FID as VLANs 127, 255, 383 etc, right? So there is potentially still full address database leakage between VLAN-unaware and VLAN-aware bridges. I think this phrase from the documentation is under-appreciated in understanding how the hardware works: | Table 4-8 details the forwarding and discarding actions that are taken | for the various VLAN scenarios. The first entry in the table is | explained by the fact that VLAN Table lookup is enabled even when 802.1Q | VLAN is not enabled. The last part ("VLAN Table lookup is enabled even when 802.1Q VLAN is not enabled") is what makes it so that the PVID of the port must be present in the VLAN table or otherwise you get packet drops. In turn, if the VLAN table is being looked up, it means that regardless of whether the switch is VLAN-unaware or not, the VID will be transformed into a 7-bit FID. I want that the FID that is being used for standalone ports and VLAN-unaware bridges (127) to be a fully conscious decision, with the implications understood, and not just something done for me to shut up. There is a risk here that you may think things are fine and work on other features, but things are not fine at all. And in this area, standalone ports/bridge VLANs/ FDB entries/FIDs are very inter-related things. When you change one, you may find that the entire scheme needs to be re-thought.
diff --git a/drivers/net/dsa/microchip/ksz8.h b/drivers/net/dsa/microchip/ksz8.h index 6529f2e2426a..c5b0ab031427 100644 --- a/drivers/net/dsa/microchip/ksz8.h +++ b/drivers/net/dsa/microchip/ksz8.h @@ -42,6 +42,7 @@ int ksz8_port_vlan_add(struct ksz_device *dev, int port, u16 vlan_vid, u16 flags); int ksz8_port_vlan_del(struct ksz_device *dev, int port, const struct switchdev_obj_port_vlan *vlan); +void ksz8_port_enable_pvid(struct ksz_device *dev, int port, bool state); int ksz8_port_mirror_add(struct ksz_device *dev, int port, struct dsa_mall_mirror_tc_entry *mirror, bool ingress, struct netlink_ext_ack *extack); diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c index b8843697c5a5..16709949d079 100644 --- a/drivers/net/dsa/microchip/ksz8795.c +++ b/drivers/net/dsa/microchip/ksz8795.c @@ -952,7 +952,7 @@ int ksz8_port_vlan_filtering(struct ksz_device *dev, int port, bool flag, return 0; } -static void ksz8_port_enable_pvid(struct ksz_device *dev, int port, bool state) +void ksz8_port_enable_pvid(struct ksz_device *dev, int port, bool state) { if (ksz_is_ksz88x3(dev)) { ksz_cfg(dev, REG_SW_INSERT_SRC_PVID, @@ -967,8 +967,8 @@ int ksz8_port_vlan_add(struct ksz_device *dev, int port, u16 vlan_vid, { bool untagged = flags & BRIDGE_VLAN_INFO_UNTAGGED; struct ksz_port *p = &dev->ports[port]; - u16 data, new_pvid = 0; u8 fid, member, valid; + u16 data; if (ksz_is_ksz88x3(dev)) return -ENOTSUPP; @@ -1015,32 +1015,18 @@ int ksz8_port_vlan_add(struct ksz_device *dev, int port, u16 vlan_vid, ksz8_to_vlan(dev, fid, member, valid, &data); ksz8_w_vlan_table(dev, vlan_vid, data); - /* change PVID */ - if (flags & BRIDGE_VLAN_INFO_PVID) - new_pvid = vlan_vid; - - if (new_pvid) { - - ksz_set_pvid(dev, port, new_pvid); - - ksz8_port_enable_pvid(dev, port, true); - } - return 0; } int ksz8_port_vlan_del(struct ksz_device *dev, int port, const struct switchdev_obj_port_vlan *vlan) { - u16 data, pvid; + u16 data; u8 fid, member, valid; if (ksz_is_ksz88x3(dev)) return -ENOTSUPP; - ksz_get_pvid(dev, port, &pvid); - pvid = pvid & 0xFFF; - ksz8_r_vlan_table(dev, vlan->vid, &data); ksz8_from_vlan(dev, data, &fid, &member, &valid); @@ -1055,9 +1041,6 @@ int ksz8_port_vlan_del(struct ksz_device *dev, int port, ksz8_to_vlan(dev, fid, member, valid, &data); ksz8_w_vlan_table(dev, vlan->vid, data); - if (pvid == vlan->vid) - ksz8_port_enable_pvid(dev, port, false); - return 0; } diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index a43a581520fb..b423aebe4473 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -354,6 +354,12 @@ void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port) } } +void ksz9477_port_drop_untagged(struct ksz_device *dev, int port, bool state) +{ + ksz_port_cfg(dev, port, REG_PORT_MRI_MAC_CTRL, PORT_DROP_NON_VLAN, + state); +} + int ksz9477_port_vlan_filtering(struct ksz_device *dev, int port, bool flag, struct netlink_ext_ack *extack) { @@ -394,10 +400,6 @@ int ksz9477_port_vlan_add(struct ksz_device *dev, int port, u16 vlan_vid, if (err) return err; - /* change PVID */ - if (flags & BRIDGE_VLAN_INFO_PVID) - ksz_set_pvid(dev, port, vlan_vid); - return 0; } @@ -406,10 +408,6 @@ int ksz9477_port_vlan_del(struct ksz_device *dev, int port, { bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; u32 vlan_table[3]; - u16 pvid; - - ksz_get_pvid(dev, port, &pvid); - pvid = pvid & 0xFFF; if (ksz9477_get_vlan_table(dev, vlan->vid, vlan_table)) { dev_dbg(dev->dev, "Failed to get vlan table\n"); @@ -418,9 +416,6 @@ int ksz9477_port_vlan_del(struct ksz_device *dev, int port, vlan_table[2] &= ~BIT(port); - if (pvid == vlan->vid) - pvid = 1; - if (untagged) vlan_table[1] &= ~BIT(port); @@ -429,8 +424,6 @@ int ksz9477_port_vlan_del(struct ksz_device *dev, int port, return -ETIMEDOUT; } - ksz_set_pvid(dev, port, pvid); - return 0; } diff --git a/drivers/net/dsa/microchip/ksz9477.h b/drivers/net/dsa/microchip/ksz9477.h index 30a1fff9b8ec..2bd88319a2c0 100644 --- a/drivers/net/dsa/microchip/ksz9477.h +++ b/drivers/net/dsa/microchip/ksz9477.h @@ -28,6 +28,7 @@ int ksz9477_port_vlan_filtering(struct ksz_device *dev, int port, int ksz9477_port_vlan_add(struct ksz_device *dev, int port, u16 vid, u16 flags); int ksz9477_port_vlan_del(struct ksz_device *dev, int port, const struct switchdev_obj_port_vlan *vlan); +void ksz9477_port_drop_untagged(struct ksz_device *dev, int port, bool state); int ksz9477_port_mirror_add(struct ksz_device *dev, int port, struct dsa_mall_mirror_tc_entry *mirror, bool ingress, struct netlink_ext_ack *extack); diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 516fb9d35c87..8a5583b1f2f4 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -161,6 +161,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = { .vlan_filtering = ksz8_port_vlan_filtering, .vlan_add = ksz8_port_vlan_add, .vlan_del = ksz8_port_vlan_del, + .drop_untagged = ksz8_port_enable_pvid, .mirror_add = ksz8_port_mirror_add, .mirror_del = ksz8_port_mirror_del, .get_caps = ksz8_get_caps, @@ -187,6 +188,7 @@ static const struct ksz_dev_ops ksz9477_dev_ops = { .vlan_filtering = ksz9477_port_vlan_filtering, .vlan_add = ksz9477_port_vlan_add, .vlan_del = ksz9477_port_vlan_del, + .drop_untagged = ksz9477_port_drop_untagged, .mirror_add = ksz9477_port_mirror_add, .mirror_del = ksz9477_port_mirror_del, .get_caps = ksz9477_get_caps, @@ -220,6 +222,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = { .vlan_filtering = ksz9477_port_vlan_filtering, .vlan_add = ksz9477_port_vlan_add, .vlan_del = ksz9477_port_vlan_del, + .drop_untagged = ksz9477_port_drop_untagged, .mirror_add = ksz9477_port_mirror_add, .mirror_del = ksz9477_port_mirror_del, .get_caps = lan937x_phylink_get_caps, @@ -1254,6 +1257,9 @@ static int ksz_enable_port(struct dsa_switch *ds, int port, { struct ksz_device *dev = ds->priv; + dev->dev_ops->vlan_add(dev, port, KSZ_DEFAULT_VLAN, + BRIDGE_VLAN_INFO_UNTAGGED); + if (!dsa_is_user_port(ds, port)) return 0; @@ -1335,7 +1341,7 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds, return proto; } -void ksz_get_pvid(struct ksz_device *dev, int port, u16 *pvid) +static void ksz_get_pvid(struct ksz_device *dev, int port, u16 *pvid) { const u16 *regs = dev->info->regs; u16 val; @@ -1345,45 +1351,110 @@ void ksz_get_pvid(struct ksz_device *dev, int port, u16 *pvid) *pvid = val & VLAN_VID_MASK; } -void ksz_set_pvid(struct ksz_device *dev, int port, u16 pvid) +static void ksz_set_pvid(struct ksz_device *dev, int port, u16 pvid) { const u16 *regs = dev->info->regs; ksz_prmw16(dev, port, regs[P_DEFAULT_PVID], VLAN_VID_MASK, pvid); } +static int ksz_commit_pvid(struct dsa_switch *ds, int port) +{ + struct dsa_port *dp = dsa_to_port(ds, port); + struct net_device *br = dsa_port_bridge_dev_get(dp); + struct ksz_device *dev = ds->priv; + u16 pvid = KSZ_DEFAULT_VLAN; + bool drop_untagged = false; + struct ksz_port *p; + + p = &dev->ports[port]; + + if (br && br_vlan_enabled(br)) { + pvid = p->bridge_pvid.vid; + drop_untagged = !p->bridge_pvid.valid; + } + + ksz_set_pvid(dev, port, pvid); + + if (dev->dev_ops->drop_untagged) + dev->dev_ops->drop_untagged(dev, port, drop_untagged); + + return 0; +} + static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port, bool flag, struct netlink_ext_ack *extack) { struct ksz_device *dev = ds->priv; + int ret; if (!dev->dev_ops->vlan_filtering) return -EOPNOTSUPP; - return dev->dev_ops->vlan_filtering(dev, port, flag, extack); + ret = dev->dev_ops->vlan_filtering(dev, port, flag, extack); + if (ret) + return ret; + + return ksz_commit_pvid(ds, port); } static int ksz_port_vlan_add(struct dsa_switch *ds, int port, const struct switchdev_obj_port_vlan *vlan, struct netlink_ext_ack *extack) { + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; struct ksz_device *dev = ds->priv; + struct ksz_port *p; + int ret; + + p = &dev->ports[port]; if (!dev->dev_ops->vlan_add) return -EOPNOTSUPP; - return dev->dev_ops->vlan_add(dev, port, vlan->vid, vlan->flags); + ret = dev->dev_ops->vlan_add(dev, port, vlan->vid, vlan->flags); + if (ret) + return ret; + + if (pvid) { + p->bridge_pvid.vid = vlan->vid; + p->bridge_pvid.valid = true; + } else if (vlan->vid && p->bridge_pvid.vid == vlan->vid) { + /* The old pvid was reinstalled as a non-pvid VLAN */ + p->bridge_pvid.valid = false; + } + + return ksz_commit_pvid(ds, port); } static int ksz_port_vlan_del(struct dsa_switch *ds, int port, const struct switchdev_obj_port_vlan *vlan) { struct ksz_device *dev = ds->priv; + struct ksz_port *p; + u16 pvid; + int ret; + + p = &dev->ports[port]; if (!dev->dev_ops->vlan_del) return -EOPNOTSUPP; - return dev->dev_ops->vlan_del(dev, port, vlan); + ksz_get_pvid(dev, port, &pvid); + + ret = dev->dev_ops->vlan_del(dev, port, vlan); + if (ret) + return ret; + + if (vlan->vid == pvid) { + p->bridge_pvid.valid = false; + + ret = ksz_commit_pvid(ds, port); + if (ret) + return ret; + } + + return 0; } static int ksz_port_mirror_add(struct dsa_switch *ds, int port, diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index 3bcd4e20bfaa..3d7eb170e3ec 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -15,6 +15,7 @@ #include <net/dsa.h> #define KSZ_MAX_NUM_PORTS 8 +#define KSZ_DEFAULT_VLAN (VLAN_N_VID - 1) struct vlan_table { u32 table[3]; @@ -63,6 +64,11 @@ struct ksz_chip_data { bool internal_phy[KSZ_MAX_NUM_PORTS]; }; +struct ksz_vlan { + u16 vid; + bool valid; +}; + struct ksz_port { bool remove_tag; /* Remove Tag flag set, for ksz8795 only */ int stp_state; @@ -81,6 +87,7 @@ struct ksz_port { u16 max_frame; u32 rgmii_tx_val; u32 rgmii_rx_val; + struct ksz_vlan bridge_pvid; }; struct ksz_device { @@ -271,6 +278,7 @@ struct ksz_dev_ops { int (*vlan_add)(struct ksz_device *dev, int port, u16 vid, u16 flags); int (*vlan_del)(struct ksz_device *dev, int port, const struct switchdev_obj_port_vlan *vlan); + void (*drop_untagged)(struct ksz_device *dev, int port, bool untagged); int (*mirror_add)(struct ksz_device *dev, int port, struct dsa_mall_mirror_tc_entry *mirror, bool ingress, struct netlink_ext_ack *extack); @@ -320,8 +328,6 @@ void ksz_port_stp_state_set(struct dsa_switch *ds, int port, u8 state); bool ksz_get_gbit(struct ksz_device *dev, int port); phy_interface_t ksz_get_xmii(struct ksz_device *dev, int port, bool gbit); extern const struct ksz_chip_data ksz_switch_chips[]; -void ksz_get_pvid(struct ksz_device *dev, int port, u16 *pvid); -void ksz_set_pvid(struct ksz_device *dev, int port, u16 pvid); /* Common register access functions */
To remove ds->configure_vlan_while_not_filtering, for the bridge vlan unaware the private pvid of 4095 is used. The bridged vlan pvid is stored in the ksz port structure and it is used only when vlan_filtering is enabled. If vlan_filtering is disabled then private pvid is used. Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com> --- drivers/net/dsa/microchip/ksz8.h | 1 + drivers/net/dsa/microchip/ksz8795.c | 23 +------- drivers/net/dsa/microchip/ksz9477.c | 19 ++---- drivers/net/dsa/microchip/ksz9477.h | 1 + drivers/net/dsa/microchip/ksz_common.c | 81 ++++++++++++++++++++++++-- drivers/net/dsa/microchip/ksz_common.h | 10 +++- 6 files changed, 95 insertions(+), 40 deletions(-)