| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 Mar 2024 15:51:11 -0800
From: Florian Fainelli <f.fainelli@...il.com>
To: Pawel Dembicki <paweldembicki@...il.com>, netdev@...r.kernel.org
Cc: Linus Walleij <linus.walleij@...aro.org>, Simon Horman
<horms@...nel.org>, Andrew Lunn <andrew@...n.ch>,
Vladimir Oltean <olteanv@...il.com>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Claudiu Manoil <claudiu.manoil@....com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
UNGLinuxDriver@...rochip.com, Russell King <linux@...linux.org.uk>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v6 07/16] net: dsa: vsc73xx: Add vlan filtering
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@...il.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.
--
Florian
Powered by blists - more mailing lists