| Message ID | 20240301221641.159542-8-paweldembicki@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: vsc73xx: Make vsc73xx usable | expand |
On 3/1/24 14:16, Pawel Dembicki wrote: > This patch implements VLAN filtering for the vsc73xx driver. > > After starting VLAN filtering, the switch is reconfigured from QinQ to > a simple VLAN aware mode. This is required because VSC73XX chips do not > support inner VLAN tag filtering. > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > --- [snip] > +static size_t > +vsc73xx_bridge_vlan_num_untagged(struct vsc73xx *vsc, int port, u16 ignored_vid) > +{ > + struct vsc73xx_bridge_vlan *vlan; > + size_t num_untagged = 0; > + > + list_for_each_entry(vlan, &vsc->vlans, list) > + if ((vlan->portmask & BIT(port)) && > + (vlan->untagged & BIT(port)) && > + vlan->vid != ignored_vid) > + num_untagged++; > + > + return num_untagged; > +} You always use both helpers at the same time, so I would suggest returning num_tagged and num_untagged by reference to have a single linked list lookup. > + > +static u16 vsc73xx_find_first_vlan_untagged(struct vsc73xx *vsc, int port) > +{ > + struct vsc73xx_bridge_vlan *vlan; > + > + list_for_each_entry(vlan, &vsc->vlans, list) > + if ((vlan->portmask & BIT(port)) && > + (vlan->untagged & BIT(port))) > + return vlan->vid; > + > + return VLAN_N_VID; > +} > + > +static int > +vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port, > + bool vlan_filtering, struct netlink_ext_ack *extack) > +{ > + enum vsc73xx_port_vlan_conf port_vlan_conf = VSC73XX_VLAN_IGNORE; > + struct vsc73xx *vsc = ds->priv; > + bool store_untagged = false; > + bool store_pvid = false; > + u16 vid, vlan_untagged; > + > + /* The swap processed below is required because vsc73xx is using > + * tag_8021q. When vlan_filtering is disabled, tag_8021q uses > + * pvid/untagged vlans for port recognition. The values configured for > + * vlans < 3072 are stored in storage table. When vlan_filtering is > + * enabled, we need to restore pvid/untagged from storage and keep > + * values used for tag_8021q. > + */ > + if (vlan_filtering) { > + /* Use VLAN_N_VID to count all vlans */ > + size_t num_untagged = > + vsc73xx_bridge_vlan_num_untagged(vsc, port, VLAN_N_VID); > + > + port_vlan_conf = (num_untagged > 1) ? > + VSC73XX_VLAN_FILTER_UNTAG_ALL : > + VSC73XX_VLAN_FILTER; > + > + vlan_untagged = vsc73xx_find_first_vlan_untagged(vsc, port); > + if (vlan_untagged < VLAN_N_VID) { > + store_untagged = vsc73xx_port_get_untagged(vsc, port, > + &vid, > + false); > + vsc73xx_vlan_change_untagged(vsc, port, vlan_untagged, > + true, false); > + vsc->untagged_storage[port] = store_untagged ? > + vid : VLAN_N_VID; > + } > + } else { > + vsc73xx_vlan_change_untagged(vsc, port, > + vsc->untagged_storage[port], > + vsc->untagged_storage[port] < > + VLAN_N_VID, false); > + } > + > + vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); > + > + store_pvid = vsc73xx_port_get_pvid(vsc, port, &vid, false); > + vsc73xx_vlan_change_pvid(vsc, port, vsc->pvid_storage[port], > + vsc->pvid_storage[port] < VLAN_N_VID, false); > + vsc->pvid_storage[port] = store_pvid ? vid : VLAN_N_VID; > + > + return 0; > +} > + > +static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan, > + struct netlink_ext_ack *extack) > +{ > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; > + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; > + struct dsa_port *dp = dsa_to_port(ds, port); > + struct vsc73xx_bridge_vlan *vsc73xx_vlan; > + size_t num_tagged, num_untagged; > + struct vsc73xx *vsc = ds->priv; > + int ret; > + u16 vid; > + > + /* Be sure to deny alterations to the configuration done by tag_8021q. > + */ > + if (vid_is_dsa_8021q(vlan->vid)) { > + NL_SET_ERR_MSG_MOD(extack, > + "Range 3072-4095 reserved for dsa_8021q operation"); > + return -EBUSY; > + } > + > + /* The processed vlan->vid is excluded from the search because the VLAN > + * can be re-added with a different set of flags, so it's easiest to > + * ignore its old flags from the VLAN database software copy. > + */ > + num_tagged = vsc73xx_bridge_vlan_num_tagged(vsc, port, vlan->vid); > + num_untagged = vsc73xx_bridge_vlan_num_untagged(vsc, port, vlan->vid); > + > + /* VSC73XX allow only three untagged states: none, one or all */ > + if ((untagged && num_tagged > 0 && num_untagged > 0) || > + (!untagged && num_untagged > 1)) { > + NL_SET_ERR_MSG_MOD(extack, > + "Port can have only none, one or all untagged vlan"); > + return -EBUSY; > + } > + > + vsc73xx_vlan = vsc73xx_bridge_vlan_find(vsc, vlan->vid); > + > + if (!vsc73xx_vlan) { > + vsc73xx_vlan = kzalloc(sizeof(*vsc73xx_vlan), GFP_KERNEL); > + if (!vsc73xx_vlan) > + return -ENOMEM; > + > + vsc73xx_vlan->vid = vlan->vid; > + vsc73xx_vlan->portmask = BIT(port); > + vsc73xx_vlan->untagged = untagged ? BIT(port) : 0; > + > + INIT_LIST_HEAD(&vsc73xx_vlan->list); > + list_add_tail(&vsc73xx_vlan->list, &vsc->vlans); > + } else { > + vsc73xx_vlan->portmask |= BIT(port); > + > + if (untagged) > + vsc73xx_vlan->untagged |= BIT(port); > + else > + vsc73xx_vlan->untagged &= ~BIT(port); These assignments should be working even when you have a freshly allocated VLAN entry, so you can just re-factor this a bit and have a common set of assignments applying to an existing or freshly allocated VLAN entry? > + } > + > + /* CPU port must be always tagged because port separation is based on > + * tag_8021q. > + */ > + if (port != CPU_PORT) { Please reduce indentation here. Have to admit the logic is a bit hard to follow, but that is also because of my lack of understanding of the requirements surrounding the use of tag_8021q.
On Fri, Mar 01, 2024 at 11:16:29PM +0100, Pawel Dembicki wrote: > +static int > +vsc73xx_write_vlan_table_entry(struct vsc73xx *vsc, u16 vid, u8 portmap) > +{ > + int ret; > + > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANTIDX, vid); > + > + ret = vsc73xx_wait_for_vlan_table_cmd(vsc); > + if (ret) > + return ret; > + > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK | > + VSC73XX_VLANACCESS_VLAN_SRC_CHECK | > + VSC73XX_VLANACCESS_VLAN_PORT_MASK, > + VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY | > + VSC73XX_VLANACCESS_VLAN_SRC_CHECK | > + (portmap << VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT) > + ); Close the parenthesis on the previous line. > + > + return vsc73xx_wait_for_vlan_table_cmd(vsc); > +} > + > +static int > +vsc73xx_update_vlan_table(struct vsc73xx *vsc, int port, u16 vid, bool set) > +{ > + u8 portmap; > + int ret; > + > + ret = vsc73xx_read_vlan_table_entry(vsc, vid, &portmap); > + if (ret) > + return ret; > + > + if (set) > + portmap |= BIT(port); > + else > + portmap &= ~BIT(port); > + > + return vsc73xx_write_vlan_table_entry(vsc, vid, portmap); > +} > + > static int vsc73xx_setup(struct dsa_switch *ds) > { > struct vsc73xx *vsc = ds->priv; > @@ -598,7 +714,7 @@ static int vsc73xx_setup(struct dsa_switch *ds) > VSC73XX_MACACCESS, > VSC73XX_MACACCESS_CMD_CLEAR_TABLE); > > - /* Clear VLAN table */ > + /* Set VLAN table to default values */ > vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, > VSC73XX_VLANACCESS, > VSC73XX_VLANACCESS_VLAN_TBL_CMD_CLEAR_TABLE); > @@ -627,6 +743,9 @@ static int vsc73xx_setup(struct dsa_switch *ds) > vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GMIIDELAY, > VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS | > VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS); > + /* Ingess VLAN reception mask (table 145) */ > + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANMASK, > + 0x5f); It would have been nice for this to be expressed more symbolically than the magic number 0x5f. Also, is it correct for vsc7398 (8 ports)? > /* IP multicast flood mask (table 144) */ > vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_IFLODMSK, > 0xff); > @@ -639,6 +758,12 @@ static int vsc73xx_setup(struct dsa_switch *ds) > > udelay(4); > > + /* Clear VLAN table */ > + for (i = 0; i < VLAN_N_VID; i++) > + vsc73xx_write_vlan_table_entry(vsc, i, 0); > + > + INIT_LIST_HEAD(&vsc->vlans); > + > return 0; > } > > @@ -1029,6 +1154,443 @@ static void vsc73xx_phylink_get_caps(struct dsa_switch *dsa, int port, > config->mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000; > } > > +static void > +vsc73xx_set_vlan_conf(struct vsc73xx *vsc, int port, > + enum vsc73xx_port_vlan_conf port_vlan_conf) > +{ > + u32 val = 0; > + > + if (port_vlan_conf == VSC73XX_VLAN_IGNORE) > + val = VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA | > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA; > + > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC, > + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA | > + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA, val); > + > + val = (port_vlan_conf == VSC73XX_VLAN_FILTER) ? > + VSC73XX_TXUPDCFG_TX_INSERT_TAG : 0; > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, > + VSC73XX_TXUPDCFG_TX_INSERT_TAG, val); > +} > + > +static int > +vsc73xx_vlan_change_untagged(struct vsc73xx *vsc, int port, u16 vid, bool set, > + bool operate_on_storage) > +{ > + u32 val = 0; > + > + if (operate_on_storage) { > + vsc->untagged_storage[port] = set ? vid : VLAN_N_VID; > + return 0; > + } > + > + if (set) > + val = VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA | > + ((vid << VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT) & > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID); > + > + return vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > + VSC73XX_TXUPDCFG, > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA | > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, val); > +} > + > +static int vsc73xx_vlan_change_pvid(struct vsc73xx *vsc, int port, u16 vid, > + bool set, bool operate_on_storage) > +{ > + int ret; > + u32 val; > + > + if (operate_on_storage) { > + vsc->pvid_storage[port] = set ? vid : VLAN_N_VID; > + return 0; > + } > + > + val = set ? 0 : VSC73XX_CAT_DROP_UNTAGGED_ENA; > + vid = set ? vid : 0; Why overwrite "vid" to 0 if deleting the PVID? Does it even matter, since you're setting VSC73XX_CAT_DROP_UNTAGGED_ENA? Or is it just due to the weird calling convention, where "vid" does not even matter when set==false, but some callers pass weird values like VLAN_N_VID, and you want to avoid programming the HW with that? If so, see the more detailed suggestion below. > + > + ret = vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > + VSC73XX_CAT_DROP, > + VSC73XX_CAT_DROP_UNTAGGED_ENA, val); > + if (ret) > + return ret; > + > + return vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > + VSC73XX_CAT_PORT_VLAN, > + VSC73XX_CAT_PORT_VLAN_VLAN_VID, > + vid & VSC73XX_CAT_PORT_VLAN_VLAN_VID); > +} > + > +static bool vsc73xx_port_get_pvid(struct vsc73xx *vsc, int port, u16 *vid, > + bool operate_on_storage) > +{ > + u32 val; > + > + if (operate_on_storage) { > + if (vsc->pvid_storage[port] < VLAN_N_VID) { > + *vid = vsc->pvid_storage[port]; > + return true; > + } > + return false; > + } > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP, &val); > + if (val & VSC73XX_CAT_DROP_UNTAGGED_ENA) > + return false; > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, &val); > + *vid = val & VSC73XX_CAT_PORT_VLAN_VLAN_VID; > + > + return true; > +} > + > +static bool vsc73xx_tag_8021q_active(struct dsa_port *dp) > +{ > + return !dsa_port_is_vlan_filtering(dp); > +} This and vsc73xx_port_get_untagged() are a bit misplaced in terms of ordering. I guess you'd want the functions which handle the untagged VLAN to be grouped together, then the functions which handle the PVID, then the rest. > + > +static bool vsc73xx_port_get_untagged(struct vsc73xx *vsc, int port, u16 *vid, > + bool operate_on_storage) > +{ > + u32 val; > + > + if (operate_on_storage) { > + if (vsc->untagged_storage[port] < VLAN_N_VID) { > + *vid = vsc->untagged_storage[port]; > + return true; > + } > + return false; > + } > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, &val); > + if (!(val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA)) > + return false; > + > + *vid = (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID) >> > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT; > + > + return true; > +} > + > +static struct vsc73xx_bridge_vlan * > +vsc73xx_bridge_vlan_find(struct vsc73xx *vsc, u16 vid) > +{ > + struct vsc73xx_bridge_vlan *vlan; > + > + list_for_each_entry(vlan, &vsc->vlans, list) > + if (vlan->vid == vid) > + return vlan; > + > + return NULL; > +} > + > +static size_t > +vsc73xx_bridge_vlan_num_tagged(struct vsc73xx *vsc, int port, u16 ignored_vid) > +{ > + struct vsc73xx_bridge_vlan *vlan; > + size_t num_tagged = 0; > + > + list_for_each_entry(vlan, &vsc->vlans, list) > + if ((vlan->portmask & BIT(port)) && > + !(vlan->untagged & BIT(port)) && > + vlan->vid != ignored_vid) > + num_tagged++; > + > + return num_tagged; > +} > + > +static size_t > +vsc73xx_bridge_vlan_num_untagged(struct vsc73xx *vsc, int port, u16 ignored_vid) > +{ > + struct vsc73xx_bridge_vlan *vlan; > + size_t num_untagged = 0; > + > + list_for_each_entry(vlan, &vsc->vlans, list) > + if ((vlan->portmask & BIT(port)) && > + (vlan->untagged & BIT(port)) && > + vlan->vid != ignored_vid) > + num_untagged++; > + > + return num_untagged; > +} > + > +static u16 vsc73xx_find_first_vlan_untagged(struct vsc73xx *vsc, int port) > +{ > + struct vsc73xx_bridge_vlan *vlan; > + > + list_for_each_entry(vlan, &vsc->vlans, list) > + if ((vlan->portmask & BIT(port)) && > + (vlan->untagged & BIT(port))) > + return vlan->vid; > + > + return VLAN_N_VID; > +} > + > +static int > +vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port, > + bool vlan_filtering, struct netlink_ext_ack *extack) > +{ > + enum vsc73xx_port_vlan_conf port_vlan_conf = VSC73XX_VLAN_IGNORE; > + struct vsc73xx *vsc = ds->priv; > + bool store_untagged = false; > + bool store_pvid = false; > + u16 vid, vlan_untagged; > + > + /* The swap processed below is required because vsc73xx is using > + * tag_8021q. When vlan_filtering is disabled, tag_8021q uses > + * pvid/untagged vlans for port recognition. The values configured for > + * vlans < 3072 are stored in storage table. When vlan_filtering is > + * enabled, we need to restore pvid/untagged from storage and keep > + * values used for tag_8021q. > + */ > + if (vlan_filtering) { > + /* Use VLAN_N_VID to count all vlans */ > + size_t num_untagged = > + vsc73xx_bridge_vlan_num_untagged(vsc, port, VLAN_N_VID); > + > + port_vlan_conf = (num_untagged > 1) ? > + VSC73XX_VLAN_FILTER_UNTAG_ALL : > + VSC73XX_VLAN_FILTER; > + > + vlan_untagged = vsc73xx_find_first_vlan_untagged(vsc, port); > + if (vlan_untagged < VLAN_N_VID) { > + store_untagged = vsc73xx_port_get_untagged(vsc, port, A single space before =. > + &vid, > + false); > + vsc73xx_vlan_change_untagged(vsc, port, vlan_untagged, > + true, false); > + vsc->untagged_storage[port] = store_untagged ? > + vid : VLAN_N_VID; > + } > + } else { > + vsc73xx_vlan_change_untagged(vsc, port, > + vsc->untagged_storage[port], > + vsc->untagged_storage[port] < > + VLAN_N_VID, false); > + } > + > + vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); > + > + store_pvid = vsc73xx_port_get_pvid(vsc, port, &vid, false); > + vsc73xx_vlan_change_pvid(vsc, port, vsc->pvid_storage[port], > + vsc->pvid_storage[port] < VLAN_N_VID, false); > + vsc->pvid_storage[port] = store_pvid ? vid : VLAN_N_VID; > + > + return 0; > +} > + > +static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan, > + struct netlink_ext_ack *extack) > +{ > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; > + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; > + struct dsa_port *dp = dsa_to_port(ds, port); > + struct vsc73xx_bridge_vlan *vsc73xx_vlan; > + size_t num_tagged, num_untagged; > + struct vsc73xx *vsc = ds->priv; > + int ret; > + u16 vid; > + > + /* Be sure to deny alterations to the configuration done by tag_8021q. > + */ > + if (vid_is_dsa_8021q(vlan->vid)) { > + NL_SET_ERR_MSG_MOD(extack, > + "Range 3072-4095 reserved for dsa_8021q operation"); > + return -EBUSY; > + } > + > + /* The processed vlan->vid is excluded from the search because the VLAN > + * can be re-added with a different set of flags, so it's easiest to > + * ignore its old flags from the VLAN database software copy. > + */ > + num_tagged = vsc73xx_bridge_vlan_num_tagged(vsc, port, vlan->vid); > + num_untagged = vsc73xx_bridge_vlan_num_untagged(vsc, port, vlan->vid); > + > + /* VSC73XX allow only three untagged states: none, one or all */ > + if ((untagged && num_tagged > 0 && num_untagged > 0) || > + (!untagged && num_untagged > 1)) { > + NL_SET_ERR_MSG_MOD(extack, > + "Port can have only none, one or all untagged vlan"); > + return -EBUSY; > + } > + > + vsc73xx_vlan = vsc73xx_bridge_vlan_find(vsc, vlan->vid); > + > + if (!vsc73xx_vlan) { > + vsc73xx_vlan = kzalloc(sizeof(*vsc73xx_vlan), GFP_KERNEL); > + if (!vsc73xx_vlan) > + return -ENOMEM; > + > + vsc73xx_vlan->vid = vlan->vid; > + vsc73xx_vlan->portmask = BIT(port); > + vsc73xx_vlan->untagged = untagged ? BIT(port) : 0; > + > + INIT_LIST_HEAD(&vsc73xx_vlan->list); > + list_add_tail(&vsc73xx_vlan->list, &vsc->vlans); > + } else { > + vsc73xx_vlan->portmask |= BIT(port); > + > + if (untagged) > + vsc73xx_vlan->untagged |= BIT(port); > + else > + vsc73xx_vlan->untagged &= ~BIT(port); > + } > + > + /* CPU port must be always tagged because port separation is based on > + * tag_8021q. > + */ > + if (port != CPU_PORT) { > + bool operate_on_storage = vsc73xx_tag_8021q_active(dp); > + > + if (!operate_on_storage) { > + enum vsc73xx_port_vlan_conf port_vlan_conf = > + VSC73XX_VLAN_FILTER; > + > + if (num_tagged == 0 && untagged) > + port_vlan_conf = VSC73XX_VLAN_FILTER_UNTAG_ALL; > + vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); > + > + if (port_vlan_conf == VSC73XX_VLAN_FILTER) { > + if (untagged) { > + ret = vsc73xx_vlan_change_untagged(vsc, > + port, > + vlan->vid, > + true, > + false); > + if (ret) > + goto err; > + } else if (num_untagged == 1) { > + vid = vsc73xx_find_first_vlan_untagged(vsc, > + port); > + ret = vsc73xx_vlan_change_untagged(vsc, > + port, > + vid, > + true, > + false); > + if (ret) > + goto err; > + } > + } > + } > + > + if (pvid) { > + ret = vsc73xx_vlan_change_pvid(vsc, port, vlan->vid, > + true, > + operate_on_storage); > + if (ret) > + goto err; > + } else if (vsc73xx_port_get_pvid(vsc, port, &vid, false) && > + vid == vlan->vid) { > + vsc73xx_vlan_change_pvid(vsc, port, 0, false, false); > + } else if (vsc->pvid_storage[port] == vlan->vid) { > + vsc73xx_vlan_change_pvid(vsc, port, 0, false, true); > + } > + } > + > + ret = vsc73xx_update_vlan_table(vsc, port, vlan->vid, true); > + if (!ret) > + return 0; > +err: > + list_del(&vsc73xx_vlan->list); > + kfree(vsc73xx_vlan); > + return ret; > +} > + > +static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan) > +{ > + struct vsc73xx_bridge_vlan *vsc73xx_vlan; > + size_t num_tagged, num_untagged; > + struct vsc73xx *vsc = ds->priv; > + bool operate_on_storage; > + int ret; > + u16 vid; > + > + num_tagged = vsc73xx_bridge_vlan_num_tagged(vsc, port, vlan->vid); > + num_untagged = vsc73xx_bridge_vlan_num_untagged(vsc, port, vlan->vid); > + > + ret = vsc73xx_update_vlan_table(vsc, port, vlan->vid, false); > + if (ret) > + return ret; > + > + operate_on_storage = vsc73xx_tag_8021q_active(dsa_to_port(ds, port)); > + > + if (!operate_on_storage) { > + enum vsc73xx_port_vlan_conf port_vlan_conf = > + VSC73XX_VLAN_FILTER; > + > + if (num_tagged == 0) > + port_vlan_conf = VSC73XX_VLAN_FILTER_UNTAG_ALL; > + vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); > + > + if (num_untagged <= 1) { > + vid = vsc73xx_find_first_vlan_untagged(vsc, port); > + vsc73xx_vlan_change_untagged(vsc, port, vid, > + num_untagged, false); > + } > + } else if (vsc->untagged_storage[port] == vlan->vid) { > + vsc73xx_vlan_change_untagged(vsc, port, 0, false, true); The fact that the "vid" argument (here 0) is ignored when "set" is false makes the calling convention confusing, and thus the caller itself. I would have probably written 2 wrappers on top of vsc73xx_vlan_change_untagged() as follows: - vsc73xx_vlan_set_untagged(), which eliminates the "set" argument from callers (implicitly true) - vsc73xx_vlan_del_untagged(), which eliminates the "set" argument (implicitly false) and the "vid" argument (unused) Same for vsc73xx_vlan_change_pvid(). This will force you to eliminate the complex expressions given to the "set" argument, like "vsc->untagged_storage[port] < VLAN_N_VID", and transform them into "if (vsc->untagged_storage[port] < VLAN_N_VID) set() else del()", which is probably a good thing in terms of readability. Something like this: diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c index 05dbeec8eb63..464d74c891d7 100644 --- a/drivers/net/dsa/vitesse-vsc73xx-core.c +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c @@ -1205,6 +1205,20 @@ vsc73xx_vlan_change_untagged(struct vsc73xx *vsc, int port, u16 vid, bool set, VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, val); } +static int vsc73xx_vlan_set_untagged(struct vsc73xx *vsc, int port, u16 vid, + bool operate_on_storage) +{ + return vsc73xx_vlan_change_untagged(vsc, port, vid, true, + operate_on_storage); +} + +static int vsc73xx_vlan_del_untagged(struct vsc73xx *vsc, int port, + bool operate_on_storage) +{ + return vsc73xx_vlan_change_untagged(vsc, port, 0, false, + operate_on_storage); +} + static int vsc73xx_vlan_change_pvid(struct vsc73xx *vsc, int port, u16 vid, bool set, bool operate_on_storage) { @@ -1231,6 +1245,20 @@ static int vsc73xx_vlan_change_pvid(struct vsc73xx *vsc, int port, u16 vid, vid & VSC73XX_CAT_PORT_VLAN_VLAN_VID); } +static int vsc73xx_vlan_set_pvid(struct vsc73xx *vsc, int port, u16 vid, + bool operate_on_storage) +{ + return vsc73xx_vlan_change_pvid(vsc, port, vid, true, + operate_on_storage); +} + +static int vsc73xx_vlan_del_pvid(struct vsc73xx *vsc, int port, + bool operate_on_storage) +{ + return vsc73xx_vlan_change_pvid(vsc, port, 0, false, + operate_on_storage); +} + static bool vsc73xx_port_get_pvid(struct vsc73xx *vsc, int port, u16 *vid, bool operate_on_storage) { @@ -1367,23 +1395,25 @@ vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port, store_untagged = vsc73xx_port_get_untagged(vsc, port, &vid, false); - vsc73xx_vlan_change_untagged(vsc, port, vlan_untagged, - true, false); + vsc73xx_vlan_set_untagged(vsc, port, vlan_untagged, + false); vsc->untagged_storage[port] = store_untagged ? vid : VLAN_N_VID; } + } else if (vsc->untagged_storage[port] < VLAN_N_VID) { + vsc73xx_vlan_set_untagged(vsc, port, vsc->untagged_storage[port], + false); } else { - vsc73xx_vlan_change_untagged(vsc, port, - vsc->untagged_storage[port], - vsc->untagged_storage[port] < - VLAN_N_VID, false); + vsc73xx_vlan_del_untagged(vsc, port, false); } vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); store_pvid = vsc73xx_port_get_pvid(vsc, port, &vid, false); - vsc73xx_vlan_change_pvid(vsc, port, vsc->pvid_storage[port], - vsc->pvid_storage[port] < VLAN_N_VID, false); + if (vsc->pvid_storage[port] < VLAN_N_VID) + vsc73xx_vlan_set_pvid(vsc, port, vsc->pvid_storage[port], false); + else + vsc73xx_vlan_del_pvid(vsc, port, false); vsc->pvid_storage[port] = store_pvid ? vid : VLAN_N_VID; return 0; @@ -1463,21 +1493,16 @@ static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, if (port_vlan_conf == VSC73XX_VLAN_FILTER) { if (untagged) { - ret = vsc73xx_vlan_change_untagged(vsc, - port, - vlan->vid, - true, - false); + ret = vsc73xx_vlan_set_untagged(vsc, port, + vlan->vid, + false); if (ret) goto err; } else if (num_untagged == 1) { vid = vsc73xx_find_first_vlan_untagged(vsc, port); - ret = vsc73xx_vlan_change_untagged(vsc, - port, - vid, - true, - false); + ret = vsc73xx_vlan_set_untagged(vsc, port, + vid, false); if (ret) goto err; } @@ -1485,16 +1510,15 @@ static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, } if (pvid) { - ret = vsc73xx_vlan_change_pvid(vsc, port, vlan->vid, - true, - operate_on_storage); + ret = vsc73xx_vlan_set_pvid(vsc, port, vlan->vid, + operate_on_storage); if (ret) goto err; } else if (vsc73xx_port_get_pvid(vsc, port, &vid, false) && vid == vlan->vid) { - vsc73xx_vlan_change_pvid(vsc, port, 0, false, false); + vsc73xx_vlan_del_pvid(vsc, port, false); } else if (vsc->pvid_storage[port] == vlan->vid) { - vsc73xx_vlan_change_pvid(vsc, port, 0, false, true); + vsc73xx_vlan_del_pvid(vsc, port, true); } } @@ -1534,19 +1558,20 @@ static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port, port_vlan_conf = VSC73XX_VLAN_FILTER_UNTAG_ALL; vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); - if (num_untagged <= 1) { + if (num_untagged == 1) { vid = vsc73xx_find_first_vlan_untagged(vsc, port); - vsc73xx_vlan_change_untagged(vsc, port, vid, - num_untagged, false); + vsc73xx_vlan_set_untagged(vsc, port, vid, false); + } else if (num_untagged == 0) { + vsc73xx_vlan_del_untagged(vsc, port, false); } } else if (vsc->untagged_storage[port] == vlan->vid) { - vsc73xx_vlan_change_untagged(vsc, port, 0, false, true); + vsc73xx_vlan_del_untagged(vsc, port, true); } if (vsc73xx_port_get_pvid(vsc, port, &vid, false) && vid == vlan->vid) - vsc73xx_vlan_change_pvid(vsc, port, 0, false, false); + vsc73xx_vlan_del_pvid(vsc, port, false); else if (vsc->pvid_storage[port] == vlan->vid) - vsc73xx_vlan_change_pvid(vsc, port, 0, false, true); + vsc73xx_vlan_del_pvid(vsc, port, true); vsc73xx_vlan = vsc73xx_bridge_vlan_find(vsc, vlan->vid); @@ -1574,11 +1599,9 @@ static int vsc73xx_tag_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid, operate_on_storage = !vsc73xx_tag_8021q_active(dsa_to_port(ds, port)); if (untagged) - vsc73xx_vlan_change_untagged(vsc, port, vid, true, - operate_on_storage); + vsc73xx_vlan_set_untagged(vsc, port, vid, operate_on_storage); if (pvid) - vsc73xx_vlan_change_pvid(vsc, port, vid, true, - operate_on_storage); + vsc73xx_vlan_set_pvid(vsc, port, vid, operate_on_storage); return vsc73xx_update_vlan_table(vsc, port, vid, true); } > + } > + > + if (vsc73xx_port_get_pvid(vsc, port, &vid, false) && vid == vlan->vid) > + vsc73xx_vlan_change_pvid(vsc, port, 0, false, false); > + else if (vsc->pvid_storage[port] == vlan->vid) > + vsc73xx_vlan_change_pvid(vsc, port, 0, false, true); > + > + vsc73xx_vlan = vsc73xx_bridge_vlan_find(vsc, vlan->vid); > + > + if (vsc73xx_vlan) { > + vsc73xx_vlan->portmask &= ~BIT(port); > + > + if (vsc73xx_vlan->portmask) > + return 0; > + > + list_del(&vsc73xx_vlan->list); > + kfree(vsc73xx_vlan); > + } > + > + return 0; > +} > + > +static int vsc73xx_port_setup(struct dsa_switch *ds, int port) > +{ > + struct vsc73xx *vsc = ds->priv; > + > + /* Those bits are responsible for MTU only. Kernel take care about MTU, takes care > + * let's enable +8 bytes frame length unconditionally. > + */ > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, > + VSC73XX_MAC_CFG_VLAN_AWR | > + VSC73XX_MAC_CFG_VLAN_DBLAWR, > + VSC73XX_MAC_CFG_VLAN_AWR | > + VSC73XX_MAC_CFG_VLAN_DBLAWR); > + > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP, > + VSC73XX_CAT_DROP_TAGGED_ENA | > + VSC73XX_CAT_DROP_UNTAGGED_ENA, > + VSC73XX_CAT_DROP_UNTAGGED_ENA); > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA | > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0); > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, > + VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0); > + > + if (port == CPU_PORT) > + vsc73xx_set_vlan_conf(vsc, port, VSC73XX_VLAN_FILTER); > + else > + vsc73xx_set_vlan_conf(vsc, port, VSC73XX_VLAN_IGNORE); > + > + /* Initially, there is no backup VLAN configuration to keep track of, so > + * configure the storage values out of range > + */ > + vsc->pvid_storage[port] = VLAN_N_VID; > + vsc->untagged_storage[port] = VLAN_N_VID; > + > + return 0; > +} > + > static void vsc73xx_refresh_fwd_map(struct dsa_switch *ds, int port, u8 state) > { > struct dsa_port *other_dp, *dp = dsa_to_port(ds, port); > @@ -1123,11 +1685,15 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { > .get_strings = vsc73xx_get_strings, > .get_ethtool_stats = vsc73xx_get_ethtool_stats, > .get_sset_count = vsc73xx_get_sset_count, > + .port_setup = vsc73xx_port_setup, > .port_enable = vsc73xx_port_enable, > .port_disable = vsc73xx_port_disable, > .port_change_mtu = vsc73xx_change_mtu, > .port_max_mtu = vsc73xx_get_max_mtu, > .port_stp_state_set = vsc73xx_port_stp_state_set, > + .port_vlan_filtering = vsc73xx_port_vlan_filtering, > + .port_vlan_add = vsc73xx_port_vlan_add, > + .port_vlan_del = vsc73xx_port_vlan_del, > .phylink_get_caps = vsc73xx_phylink_get_caps, > }; > > diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h > index e7b08599a625..facc50f1e320 100644 > --- a/drivers/net/dsa/vitesse-vsc73xx.h > +++ b/drivers/net/dsa/vitesse-vsc73xx.h > @@ -25,6 +25,17 @@ > * @addr: MAC address used in flow control frames > * @ops: Structure with hardware-dependent operations > * @priv: Pointer to the configuration interface structure > + * @pvid_storage: Storage table with PVID configured for other state of > + * vlan_filtering. It has two alternating roles: it stores the PVID when > + * configured by the bridge but VLAN filtering is off, and it stores the > + * PVID necessary for tag_8021q operation when bridge VLAN filtering is > + * enabled. > + * @untagged_storage: Storage table with eggres untagged VLAN configured for egress > + * other state of vlan_filtering.Keep VID necessary for tag8021q operations Space between "vlan_filtering." and "Keep". tag_8021q > + * when vlan filtering is enabled. > + * @vlans: List of configured vlans. Contains port mask and untagged status of > + * every vlan configured in port vlan operation. It doesn't cover tag_8021q > + * vlans. > */ > struct vsc73xx { > struct device *dev; > @@ -35,6 +46,9 @@ struct vsc73xx { > u8 addr[ETH_ALEN]; > const struct vsc73xx_ops *ops; > void *priv; > + u16 pvid_storage[VSC73XX_MAX_NUM_PORTS]; > + u16 untagged_storage[VSC73XX_MAX_NUM_PORTS]; > + struct list_head vlans; > }; > > /** > @@ -49,6 +63,21 @@ struct vsc73xx_ops { > u32 val); > }; > > +/** > + * struct vsc73xx_bridge_vlan - VSC73xx driver structure which keeps vlan > + * database copy > + * @vid: VLAN number > + * @portmask: each bit represends one port represents > + * @untagged: each bit represends one port configured with @vid untagged represents > + * @list: list structure > + */ > +struct vsc73xx_bridge_vlan { > + u16 vid; > + u8 portmask; > + u8 untagged; > + struct list_head list; > +}; > + > int vsc73xx_is_addr_valid(u8 block, u8 subblock); > int vsc73xx_probe(struct vsc73xx *vsc); > void vsc73xx_remove(struct vsc73xx *vsc); > -- > 2.34.1 >
On Tue, Mar 05, 2024 at 03:51:11PM -0800, Florian Fainelli wrote: > On 3/1/24 14:16, Pawel Dembicki wrote: > > This patch implements VLAN filtering for the vsc73xx driver. > > > > After starting VLAN filtering, the switch is reconfigured from QinQ to > > a simple VLAN aware mode. This is required because VSC73XX chips do not > > support inner VLAN tag filtering. > > > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > > --- > > [snip] > > > +static size_t > > +vsc73xx_bridge_vlan_num_untagged(struct vsc73xx *vsc, int port, u16 ignored_vid) > > +{ > > + struct vsc73xx_bridge_vlan *vlan; > > + size_t num_untagged = 0; > > + > > + list_for_each_entry(vlan, &vsc->vlans, list) > > + if ((vlan->portmask & BIT(port)) && > > + (vlan->untagged & BIT(port)) && > > + vlan->vid != ignored_vid) > > + num_untagged++; > > + > > + return num_untagged; > > +} > > You always use both helpers at the same time, so I would suggest returning > num_tagged and num_untagged by reference to have a single linked list > lookup. vsc73xx_port_vlan_filtering() calls vsc73xx_bridge_vlan_num_untagged() but not vsc73xx_bridge_vlan_num_tagged(). Doing as you suggest, while keeping the code otherwise the same, would imply adding a dummy num_tagged variable in vlan_filtering(). Though I agree it is generally confusing, because "port_vlan_conf" is assigned based on inconsistent conditions in vsc73xx_port_vlan_filtering() vs vsc73xx_port_vlan_add() vs vsc73xx_port_vlan_del(). Namely the number of tagged VLANs is taken into consideration only on vlan_add(), and of remaining tagged VLANs on vlan_del(), respectively. Maybe something like this could help? struct vsc73xx_vlan_summary { size_t num_tagged; size_t num_untagged; }; static void vsc73xx_bridge_vlan_summary(struct vsc73xx *vsc, int port, struct vsc73xx_vlan_summary *summary, u16 ignored_vid) { size_t num_tagged = 0, num_untagged = 0; struct vsc73xx_bridge_vlan *vlan; list_for_each_entry(vlan, &vsc->vlans, list) { if (!(vlan->portmask & BIT(port)) || vlan->vid == ignored_vid) continue; if (vlan->untagged & BIT(port)) num_untagged++; else num_tagged++; } summary->num_untagged = num_untagged; summary->num_tagged = num_tagged; } .. and use what you need from the summary. > > + } > > + > > + /* CPU port must be always tagged because port separation is based on > > + * tag_8021q. > > + */ > > + if (port != CPU_PORT) { > > Please reduce indentation here. > > Have to admit the logic is a bit hard to follow, but that is also because of > my lack of understanding of the requirements surrounding the use of > tag_8021q. It's not only that. The code is also a bit hard on the brain for me. An alternative coding pattern would be to observe that certain hardware registers (the egress-untagged VLAN, the PVID) depend on a constellation of N input variables (the bridge VLAN filtering state, the tag_8021q active state, the bridge VLAN table). So, to make the code easier to follow and to ensure correctness, in theory a central function could be written, which embeds the same invariant logic of determining what to program the registers with, depending on the N inputs. This invariant function is called from every place that modifies any of the N inputs. What Paweł did here was to have slightly different code paths for modifying the hardware registers, each code path adjusted slightly on the state change transition of individual inputs. This was a design choice on which I commented very early on, stating that it's unusual but that I can go along with it. It is probably very ingrained with the choice of the untagged_storage[] and pvid_storage[] arrays, the logic of swapping the storage with the hardware at VLAN filtering state change, and thus very hard to change at this stage of development.
pt., 8 mar 2024 o 14:09 Vladimir Oltean <olteanv@gmail.com> napisał(a): > > On Tue, Mar 05, 2024 at 03:51:11PM -0800, Florian Fainelli wrote: > > On 3/1/24 14:16, Pawel Dembicki wrote: > > > This patch implements VLAN filtering for the vsc73xx driver. > > > > > > After starting VLAN filtering, the switch is reconfigured from QinQ to > > > a simple VLAN aware mode. This is required because VSC73XX chips do not > > > support inner VLAN tag filtering. > > > > > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > > > --- > > > > [snip] > > > > [snip] > > > > Have to admit the logic is a bit hard to follow, but that is also because of > > my lack of understanding of the requirements surrounding the use of > > tag_8021q. > > It's not only that. The code is also a bit hard on the brain for me. > > An alternative coding pattern would be to observe that certain hardware > registers (the egress-untagged VLAN, the PVID) depend on a constellation > of N input variables (the bridge VLAN filtering state, the tag_8021q > active state, the bridge VLAN table). So, to make the code easier to > follow and to ensure correctness, in theory a central function could be > written, which embeds the same invariant logic of determining what to > program the registers with, depending on the N inputs. This invariant > function is called from every place that modifies any of the N inputs. > > What Paweł did here was to have slightly different code paths for > modifying the hardware registers, each code path adjusted slightly on > the state change transition of individual inputs. > > This was a design choice on which I commented very early on, stating > that it's unusual but that I can go along with it. It is probably very > ingrained with the choice of the untagged_storage[] and pvid_storage[] > arrays, the logic of swapping the storage with the hardware at VLAN > filtering state change, and thus very hard to change at this stage of > development. I have to admit that it wasn't an optimal implementation. I focused on the wrong priorities, which led me astray. I prepared v7 and I tried to maximize simplicity. I hope it will be more acceptable than the v6 version.
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c index d1e84a9a83d1..c643f445f026 100644 --- a/drivers/net/dsa/vitesse-vsc73xx-core.c +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c @@ -22,9 +22,11 @@ #include <linux/of_mdio.h> #include <linux/bitops.h> #include <linux/if_bridge.h> +#include <linux/if_vlan.h> #include <linux/etherdevice.h> #include <linux/gpio/consumer.h> #include <linux/gpio/driver.h> +#include <linux/dsa/8021q.h> #include <linux/random.h> #include <net/dsa.h> @@ -62,6 +64,8 @@ #define VSC73XX_CAT_DROP 0x6e #define VSC73XX_CAT_PR_MISC_L2 0x6f #define VSC73XX_CAT_PR_USR_PRIO 0x75 +#define VSC73XX_CAT_VLAN_MISC 0x79 +#define VSC73XX_CAT_PORT_VLAN 0x7a #define VSC73XX_Q_MISC_CONF 0xdf /* MAC_CFG register bits */ @@ -122,6 +126,17 @@ #define VSC73XX_ADVPORTM_IO_LOOPBACK BIT(1) #define VSC73XX_ADVPORTM_HOST_LOOPBACK BIT(0) +/* TXUPDCFG transmit modify setup bits */ +#define VSC73XX_TXUPDCFG_DSCP_REWR_MODE GENMASK(20, 19) +#define VSC73XX_TXUPDCFG_DSCP_REWR_ENA BIT(18) +#define VSC73XX_TXUPDCFG_TX_INT_TO_USRPRIO_ENA BIT(17) +#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID GENMASK(15, 4) +#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA BIT(3) +#define VSC73XX_TXUPDCFG_TX_UPDATE_CRC_CPU_ENA BIT(1) +#define VSC73XX_TXUPDCFG_TX_INSERT_TAG BIT(0) + +#define VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT 4 + /* CAT_DROP categorizer frame dropping register bits */ #define VSC73XX_CAT_DROP_DROP_MC_SMAC_ENA BIT(6) #define VSC73XX_CAT_DROP_FWD_CTRL_ENA BIT(4) @@ -135,6 +150,15 @@ #define VSC73XX_Q_MISC_CONF_EARLY_TX_512 (1 << 1) #define VSC73XX_Q_MISC_CONF_MAC_PAUSE_MODE BIT(0) +/* CAT_VLAN_MISC categorizer VLAN miscellaneous bits */ +#define VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA BIT(8) +#define VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA BIT(7) + +/* CAT_PORT_VLAN categorizer port VLAN */ +#define VSC73XX_CAT_PORT_VLAN_VLAN_CFI BIT(15) +#define VSC73XX_CAT_PORT_VLAN_VLAN_USR_PRIO GENMASK(14, 12) +#define VSC73XX_CAT_PORT_VLAN_VLAN_VID GENMASK(11, 0) + /* Frame analyzer block 2 registers */ #define VSC73XX_STORMLIMIT 0x02 #define VSC73XX_ADVLEARN 0x03 @@ -189,7 +213,8 @@ #define VSC73XX_VLANACCESS_VLAN_MIRROR BIT(29) #define VSC73XX_VLANACCESS_VLAN_SRC_CHECK BIT(28) #define VSC73XX_VLANACCESS_VLAN_PORT_MASK GENMASK(9, 2) -#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK GENMASK(2, 0) +#define VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT 2 +#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK GENMASK(1, 0) #define VSC73XX_VLANACCESS_VLAN_TBL_CMD_IDLE 0 #define VSC73XX_VLANACCESS_VLAN_TBL_CMD_READ_ENTRY 1 #define VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY 2 @@ -347,6 +372,12 @@ static const struct vsc73xx_counter vsc73xx_tx_counters[] = { { 29, "TxQoSClass3" }, /* non-standard counter */ }; +enum vsc73xx_port_vlan_conf { + VSC73XX_VLAN_FILTER, + VSC73XX_VLAN_FILTER_UNTAG_ALL, + VSC73XX_VLAN_IGNORE, +}; + int vsc73xx_is_addr_valid(u8 block, u8 subblock) { switch (block) { @@ -564,6 +595,91 @@ static enum dsa_tag_protocol vsc73xx_get_tag_protocol(struct dsa_switch *ds, return DSA_TAG_PROTO_NONE; } +static int vsc73xx_wait_for_vlan_table_cmd(struct vsc73xx *vsc) +{ + int ret, err; + u32 val; + + ret = read_poll_timeout(vsc73xx_read, err, + err < 0 || + ((val & VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK) == + VSC73XX_VLANACCESS_VLAN_TBL_CMD_IDLE), + VSC73XX_POLL_SLEEP_US, VSC73XX_POLL_TIMEOUT_US, + false, vsc, VSC73XX_BLOCK_ANALYZER, + 0, VSC73XX_VLANACCESS, &val); + if (ret) + return ret; + return err; +} + +static int +vsc73xx_read_vlan_table_entry(struct vsc73xx *vsc, u16 vid, u8 *portmap) +{ + u32 val; + int ret; + + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANTIDX, vid); + + ret = vsc73xx_wait_for_vlan_table_cmd(vsc); + if (ret) + return ret; + + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, + VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK, + VSC73XX_VLANACCESS_VLAN_TBL_CMD_READ_ENTRY); + + ret = vsc73xx_wait_for_vlan_table_cmd(vsc); + if (ret) + return ret; + + vsc73xx_read(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, &val); + *portmap = (val & VSC73XX_VLANACCESS_VLAN_PORT_MASK) >> + VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT; + + return 0; +} + +static int +vsc73xx_write_vlan_table_entry(struct vsc73xx *vsc, u16 vid, u8 portmap) +{ + int ret; + + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANTIDX, vid); + + ret = vsc73xx_wait_for_vlan_table_cmd(vsc); + if (ret) + return ret; + + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, + VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK | + VSC73XX_VLANACCESS_VLAN_SRC_CHECK | + VSC73XX_VLANACCESS_VLAN_PORT_MASK, + VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY | + VSC73XX_VLANACCESS_VLAN_SRC_CHECK | + (portmap << VSC73XX_VLANACCESS_VLAN_PORT_MASK_SHIFT) + ); + + return vsc73xx_wait_for_vlan_table_cmd(vsc); +} + +static int +vsc73xx_update_vlan_table(struct vsc73xx *vsc, int port, u16 vid, bool set) +{ + u8 portmap; + int ret; + + ret = vsc73xx_read_vlan_table_entry(vsc, vid, &portmap); + if (ret) + return ret; + + if (set) + portmap |= BIT(port); + else + portmap &= ~BIT(port); + + return vsc73xx_write_vlan_table_entry(vsc, vid, portmap); +} + static int vsc73xx_setup(struct dsa_switch *ds) { struct vsc73xx *vsc = ds->priv; @@ -598,7 +714,7 @@ static int vsc73xx_setup(struct dsa_switch *ds) VSC73XX_MACACCESS, VSC73XX_MACACCESS_CMD_CLEAR_TABLE); - /* Clear VLAN table */ + /* Set VLAN table to default values */ vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANACCESS, VSC73XX_VLANACCESS_VLAN_TBL_CMD_CLEAR_TABLE); @@ -627,6 +743,9 @@ static int vsc73xx_setup(struct dsa_switch *ds) vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GMIIDELAY, VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS | VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS); + /* Ingess VLAN reception mask (table 145) */ + vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_VLANMASK, + 0x5f); /* IP multicast flood mask (table 144) */ vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_IFLODMSK, 0xff); @@ -639,6 +758,12 @@ static int vsc73xx_setup(struct dsa_switch *ds) udelay(4); + /* Clear VLAN table */ + for (i = 0; i < VLAN_N_VID; i++) + vsc73xx_write_vlan_table_entry(vsc, i, 0); + + INIT_LIST_HEAD(&vsc->vlans); + return 0; } @@ -1029,6 +1154,443 @@ static void vsc73xx_phylink_get_caps(struct dsa_switch *dsa, int port, config->mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000; } +static void +vsc73xx_set_vlan_conf(struct vsc73xx *vsc, int port, + enum vsc73xx_port_vlan_conf port_vlan_conf) +{ + u32 val = 0; + + if (port_vlan_conf == VSC73XX_VLAN_IGNORE) + val = VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA | + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA; + + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_VLAN_MISC, + VSC73XX_CAT_VLAN_MISC_VLAN_TCI_IGNORE_ENA | + VSC73XX_CAT_VLAN_MISC_VLAN_KEEP_TAG_ENA, val); + + val = (port_vlan_conf == VSC73XX_VLAN_FILTER) ? + VSC73XX_TXUPDCFG_TX_INSERT_TAG : 0; + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, + VSC73XX_TXUPDCFG_TX_INSERT_TAG, val); +} + +static int +vsc73xx_vlan_change_untagged(struct vsc73xx *vsc, int port, u16 vid, bool set, + bool operate_on_storage) +{ + u32 val = 0; + + if (operate_on_storage) { + vsc->untagged_storage[port] = set ? vid : VLAN_N_VID; + return 0; + } + + if (set) + val = VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA | + ((vid << VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT) & + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID); + + return vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, + VSC73XX_TXUPDCFG, + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA | + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, val); +} + +static int vsc73xx_vlan_change_pvid(struct vsc73xx *vsc, int port, u16 vid, + bool set, bool operate_on_storage) +{ + int ret; + u32 val; + + if (operate_on_storage) { + vsc->pvid_storage[port] = set ? vid : VLAN_N_VID; + return 0; + } + + val = set ? 0 : VSC73XX_CAT_DROP_UNTAGGED_ENA; + vid = set ? vid : 0; + + ret = vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, + VSC73XX_CAT_DROP, + VSC73XX_CAT_DROP_UNTAGGED_ENA, val); + if (ret) + return ret; + + return vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, + VSC73XX_CAT_PORT_VLAN, + VSC73XX_CAT_PORT_VLAN_VLAN_VID, + vid & VSC73XX_CAT_PORT_VLAN_VLAN_VID); +} + +static bool vsc73xx_port_get_pvid(struct vsc73xx *vsc, int port, u16 *vid, + bool operate_on_storage) +{ + u32 val; + + if (operate_on_storage) { + if (vsc->pvid_storage[port] < VLAN_N_VID) { + *vid = vsc->pvid_storage[port]; + return true; + } + return false; + } + + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP, &val); + if (val & VSC73XX_CAT_DROP_UNTAGGED_ENA) + return false; + + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, &val); + *vid = val & VSC73XX_CAT_PORT_VLAN_VLAN_VID; + + return true; +} + +static bool vsc73xx_tag_8021q_active(struct dsa_port *dp) +{ + return !dsa_port_is_vlan_filtering(dp); +} + +static bool vsc73xx_port_get_untagged(struct vsc73xx *vsc, int port, u16 *vid, + bool operate_on_storage) +{ + u32 val; + + if (operate_on_storage) { + if (vsc->untagged_storage[port] < VLAN_N_VID) { + *vid = vsc->untagged_storage[port]; + return true; + } + return false; + } + + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, &val); + if (!(val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA)) + return false; + + *vid = (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID) >> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT; + + return true; +} + +static struct vsc73xx_bridge_vlan * +vsc73xx_bridge_vlan_find(struct vsc73xx *vsc, u16 vid) +{ + struct vsc73xx_bridge_vlan *vlan; + + list_for_each_entry(vlan, &vsc->vlans, list) + if (vlan->vid == vid) + return vlan; + + return NULL; +} + +static size_t +vsc73xx_bridge_vlan_num_tagged(struct vsc73xx *vsc, int port, u16 ignored_vid) +{ + struct vsc73xx_bridge_vlan *vlan; + size_t num_tagged = 0; + + list_for_each_entry(vlan, &vsc->vlans, list) + if ((vlan->portmask & BIT(port)) && + !(vlan->untagged & BIT(port)) && + vlan->vid != ignored_vid) + num_tagged++; + + return num_tagged; +} + +static size_t +vsc73xx_bridge_vlan_num_untagged(struct vsc73xx *vsc, int port, u16 ignored_vid) +{ + struct vsc73xx_bridge_vlan *vlan; + size_t num_untagged = 0; + + list_for_each_entry(vlan, &vsc->vlans, list) + if ((vlan->portmask & BIT(port)) && + (vlan->untagged & BIT(port)) && + vlan->vid != ignored_vid) + num_untagged++; + + return num_untagged; +} + +static u16 vsc73xx_find_first_vlan_untagged(struct vsc73xx *vsc, int port) +{ + struct vsc73xx_bridge_vlan *vlan; + + list_for_each_entry(vlan, &vsc->vlans, list) + if ((vlan->portmask & BIT(port)) && + (vlan->untagged & BIT(port))) + return vlan->vid; + + return VLAN_N_VID; +} + +static int +vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port, + bool vlan_filtering, struct netlink_ext_ack *extack) +{ + enum vsc73xx_port_vlan_conf port_vlan_conf = VSC73XX_VLAN_IGNORE; + struct vsc73xx *vsc = ds->priv; + bool store_untagged = false; + bool store_pvid = false; + u16 vid, vlan_untagged; + + /* The swap processed below is required because vsc73xx is using + * tag_8021q. When vlan_filtering is disabled, tag_8021q uses + * pvid/untagged vlans for port recognition. The values configured for + * vlans < 3072 are stored in storage table. When vlan_filtering is + * enabled, we need to restore pvid/untagged from storage and keep + * values used for tag_8021q. + */ + if (vlan_filtering) { + /* Use VLAN_N_VID to count all vlans */ + size_t num_untagged = + vsc73xx_bridge_vlan_num_untagged(vsc, port, VLAN_N_VID); + + port_vlan_conf = (num_untagged > 1) ? + VSC73XX_VLAN_FILTER_UNTAG_ALL : + VSC73XX_VLAN_FILTER; + + vlan_untagged = vsc73xx_find_first_vlan_untagged(vsc, port); + if (vlan_untagged < VLAN_N_VID) { + store_untagged = vsc73xx_port_get_untagged(vsc, port, + &vid, + false); + vsc73xx_vlan_change_untagged(vsc, port, vlan_untagged, + true, false); + vsc->untagged_storage[port] = store_untagged ? + vid : VLAN_N_VID; + } + } else { + vsc73xx_vlan_change_untagged(vsc, port, + vsc->untagged_storage[port], + vsc->untagged_storage[port] < + VLAN_N_VID, false); + } + + vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); + + store_pvid = vsc73xx_port_get_pvid(vsc, port, &vid, false); + vsc73xx_vlan_change_pvid(vsc, port, vsc->pvid_storage[port], + vsc->pvid_storage[port] < VLAN_N_VID, false); + vsc->pvid_storage[port] = store_pvid ? vid : VLAN_N_VID; + + return 0; +} + +static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_vlan *vlan, + struct netlink_ext_ack *extack) +{ + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; + struct dsa_port *dp = dsa_to_port(ds, port); + struct vsc73xx_bridge_vlan *vsc73xx_vlan; + size_t num_tagged, num_untagged; + struct vsc73xx *vsc = ds->priv; + int ret; + u16 vid; + + /* Be sure to deny alterations to the configuration done by tag_8021q. + */ + if (vid_is_dsa_8021q(vlan->vid)) { + NL_SET_ERR_MSG_MOD(extack, + "Range 3072-4095 reserved for dsa_8021q operation"); + return -EBUSY; + } + + /* The processed vlan->vid is excluded from the search because the VLAN + * can be re-added with a different set of flags, so it's easiest to + * ignore its old flags from the VLAN database software copy. + */ + num_tagged = vsc73xx_bridge_vlan_num_tagged(vsc, port, vlan->vid); + num_untagged = vsc73xx_bridge_vlan_num_untagged(vsc, port, vlan->vid); + + /* VSC73XX allow only three untagged states: none, one or all */ + if ((untagged && num_tagged > 0 && num_untagged > 0) || + (!untagged && num_untagged > 1)) { + NL_SET_ERR_MSG_MOD(extack, + "Port can have only none, one or all untagged vlan"); + return -EBUSY; + } + + vsc73xx_vlan = vsc73xx_bridge_vlan_find(vsc, vlan->vid); + + if (!vsc73xx_vlan) { + vsc73xx_vlan = kzalloc(sizeof(*vsc73xx_vlan), GFP_KERNEL); + if (!vsc73xx_vlan) + return -ENOMEM; + + vsc73xx_vlan->vid = vlan->vid; + vsc73xx_vlan->portmask = BIT(port); + vsc73xx_vlan->untagged = untagged ? BIT(port) : 0; + + INIT_LIST_HEAD(&vsc73xx_vlan->list); + list_add_tail(&vsc73xx_vlan->list, &vsc->vlans); + } else { + vsc73xx_vlan->portmask |= BIT(port); + + if (untagged) + vsc73xx_vlan->untagged |= BIT(port); + else + vsc73xx_vlan->untagged &= ~BIT(port); + } + + /* CPU port must be always tagged because port separation is based on + * tag_8021q. + */ + if (port != CPU_PORT) { + bool operate_on_storage = vsc73xx_tag_8021q_active(dp); + + if (!operate_on_storage) { + enum vsc73xx_port_vlan_conf port_vlan_conf = + VSC73XX_VLAN_FILTER; + + if (num_tagged == 0 && untagged) + port_vlan_conf = VSC73XX_VLAN_FILTER_UNTAG_ALL; + vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); + + if (port_vlan_conf == VSC73XX_VLAN_FILTER) { + if (untagged) { + ret = vsc73xx_vlan_change_untagged(vsc, + port, + vlan->vid, + true, + false); + if (ret) + goto err; + } else if (num_untagged == 1) { + vid = vsc73xx_find_first_vlan_untagged(vsc, + port); + ret = vsc73xx_vlan_change_untagged(vsc, + port, + vid, + true, + false); + if (ret) + goto err; + } + } + } + + if (pvid) { + ret = vsc73xx_vlan_change_pvid(vsc, port, vlan->vid, + true, + operate_on_storage); + if (ret) + goto err; + } else if (vsc73xx_port_get_pvid(vsc, port, &vid, false) && + vid == vlan->vid) { + vsc73xx_vlan_change_pvid(vsc, port, 0, false, false); + } else if (vsc->pvid_storage[port] == vlan->vid) { + vsc73xx_vlan_change_pvid(vsc, port, 0, false, true); + } + } + + ret = vsc73xx_update_vlan_table(vsc, port, vlan->vid, true); + if (!ret) + return 0; +err: + list_del(&vsc73xx_vlan->list); + kfree(vsc73xx_vlan); + return ret; +} + +static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_vlan *vlan) +{ + struct vsc73xx_bridge_vlan *vsc73xx_vlan; + size_t num_tagged, num_untagged; + struct vsc73xx *vsc = ds->priv; + bool operate_on_storage; + int ret; + u16 vid; + + num_tagged = vsc73xx_bridge_vlan_num_tagged(vsc, port, vlan->vid); + num_untagged = vsc73xx_bridge_vlan_num_untagged(vsc, port, vlan->vid); + + ret = vsc73xx_update_vlan_table(vsc, port, vlan->vid, false); + if (ret) + return ret; + + operate_on_storage = vsc73xx_tag_8021q_active(dsa_to_port(ds, port)); + + if (!operate_on_storage) { + enum vsc73xx_port_vlan_conf port_vlan_conf = + VSC73XX_VLAN_FILTER; + + if (num_tagged == 0) + port_vlan_conf = VSC73XX_VLAN_FILTER_UNTAG_ALL; + vsc73xx_set_vlan_conf(vsc, port, port_vlan_conf); + + if (num_untagged <= 1) { + vid = vsc73xx_find_first_vlan_untagged(vsc, port); + vsc73xx_vlan_change_untagged(vsc, port, vid, + num_untagged, false); + } + } else if (vsc->untagged_storage[port] == vlan->vid) { + vsc73xx_vlan_change_untagged(vsc, port, 0, false, true); + } + + if (vsc73xx_port_get_pvid(vsc, port, &vid, false) && vid == vlan->vid) + vsc73xx_vlan_change_pvid(vsc, port, 0, false, false); + else if (vsc->pvid_storage[port] == vlan->vid) + vsc73xx_vlan_change_pvid(vsc, port, 0, false, true); + + vsc73xx_vlan = vsc73xx_bridge_vlan_find(vsc, vlan->vid); + + if (vsc73xx_vlan) { + vsc73xx_vlan->portmask &= ~BIT(port); + + if (vsc73xx_vlan->portmask) + return 0; + + list_del(&vsc73xx_vlan->list); + kfree(vsc73xx_vlan); + } + + return 0; +} + +static int vsc73xx_port_setup(struct dsa_switch *ds, int port) +{ + struct vsc73xx *vsc = ds->priv; + + /* Those bits are responsible for MTU only. Kernel take care about MTU, + * let's enable +8 bytes frame length unconditionally. + */ + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, + VSC73XX_MAC_CFG_VLAN_AWR | + VSC73XX_MAC_CFG_VLAN_DBLAWR, + VSC73XX_MAC_CFG_VLAN_AWR | + VSC73XX_MAC_CFG_VLAN_DBLAWR); + + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_DROP, + VSC73XX_CAT_DROP_TAGGED_ENA | + VSC73XX_CAT_DROP_UNTAGGED_ENA, + VSC73XX_CAT_DROP_UNTAGGED_ENA); + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA | + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0); + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, + VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0); + + if (port == CPU_PORT) + vsc73xx_set_vlan_conf(vsc, port, VSC73XX_VLAN_FILTER); + else + vsc73xx_set_vlan_conf(vsc, port, VSC73XX_VLAN_IGNORE); + + /* Initially, there is no backup VLAN configuration to keep track of, so + * configure the storage values out of range + */ + vsc->pvid_storage[port] = VLAN_N_VID; + vsc->untagged_storage[port] = VLAN_N_VID; + + return 0; +} + static void vsc73xx_refresh_fwd_map(struct dsa_switch *ds, int port, u8 state) { struct dsa_port *other_dp, *dp = dsa_to_port(ds, port); @@ -1123,11 +1685,15 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { .get_strings = vsc73xx_get_strings, .get_ethtool_stats = vsc73xx_get_ethtool_stats, .get_sset_count = vsc73xx_get_sset_count, + .port_setup = vsc73xx_port_setup, .port_enable = vsc73xx_port_enable, .port_disable = vsc73xx_port_disable, .port_change_mtu = vsc73xx_change_mtu, .port_max_mtu = vsc73xx_get_max_mtu, .port_stp_state_set = vsc73xx_port_stp_state_set, + .port_vlan_filtering = vsc73xx_port_vlan_filtering, + .port_vlan_add = vsc73xx_port_vlan_add, + .port_vlan_del = vsc73xx_port_vlan_del, .phylink_get_caps = vsc73xx_phylink_get_caps, }; diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h index e7b08599a625..facc50f1e320 100644 --- a/drivers/net/dsa/vitesse-vsc73xx.h +++ b/drivers/net/dsa/vitesse-vsc73xx.h @@ -25,6 +25,17 @@ * @addr: MAC address used in flow control frames * @ops: Structure with hardware-dependent operations * @priv: Pointer to the configuration interface structure + * @pvid_storage: Storage table with PVID configured for other state of + * vlan_filtering. It has two alternating roles: it stores the PVID when + * configured by the bridge but VLAN filtering is off, and it stores the + * PVID necessary for tag_8021q operation when bridge VLAN filtering is + * enabled. + * @untagged_storage: Storage table with eggres untagged VLAN configured for + * other state of vlan_filtering.Keep VID necessary for tag8021q operations + * when vlan filtering is enabled. + * @vlans: List of configured vlans. Contains port mask and untagged status of + * every vlan configured in port vlan operation. It doesn't cover tag_8021q + * vlans. */ struct vsc73xx { struct device *dev; @@ -35,6 +46,9 @@ struct vsc73xx { u8 addr[ETH_ALEN]; const struct vsc73xx_ops *ops; void *priv; + u16 pvid_storage[VSC73XX_MAX_NUM_PORTS]; + u16 untagged_storage[VSC73XX_MAX_NUM_PORTS]; + struct list_head vlans; }; /** @@ -49,6 +63,21 @@ struct vsc73xx_ops { u32 val); }; +/** + * struct vsc73xx_bridge_vlan - VSC73xx driver structure which keeps vlan + * database copy + * @vid: VLAN number + * @portmask: each bit represends one port + * @untagged: each bit represends one port configured with @vid untagged + * @list: list structure + */ +struct vsc73xx_bridge_vlan { + u16 vid; + u8 portmask; + u8 untagged; + struct list_head list; +}; + int vsc73xx_is_addr_valid(u8 block, u8 subblock); int vsc73xx_probe(struct vsc73xx *vsc); void vsc73xx_remove(struct vsc73xx *vsc);
This patch implements VLAN filtering for the vsc73xx driver. After starting VLAN filtering, the switch is reconfigured from QinQ to a simple VLAN aware mode. This is required because VSC73XX chips do not support inner VLAN tag filtering. Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> --- v6: - resend only v5: - fix possible leak in 'vsc73xx_port_vlan_add' - use proper variable in statement from 'vsc73xx_port_vlan_filtering' - change 'vlan_no' name to 'vid' - codding style improvements - comment improvements - handle return of 'vsc73xx_update_bits' - reduce I/O operations - use 'size_t' for counting variables v4: - reworked most of conditional register configs - simplified port_vlan function - move vlan table clearing from port_setup to setup - pvid configuration simplified (now kernel take care about no of pvids per port) - port vlans are stored in list now - introduce implementation of all untagged vlans state - many minor changes v3: - reworked all vlan commits - added storage variables for pvid and untagged vlans - move length extender settings to port setup - remove vlan table cleaning in wrong places v2: - no changes done drivers/net/dsa/vitesse-vsc73xx-core.c | 570 ++++++++++++++++++++++++- drivers/net/dsa/vitesse-vsc73xx.h | 29 ++ 2 files changed, 597 insertions(+), 2 deletions(-)