| 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
| ||
|
Message-ID: <20230625150552.sljrgm6rqodmefq5@skbuf>
Date: Sun, 25 Jun 2023 18:05:52 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Pawel Dembicki <paweldembicki@...il.com>
Cc: netdev@...r.kernel.org, Linus Walleij <linus.walleij@...aro.org>,
Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 6/7] net: dsa: vsc73xx: Add vlan filtering
On Sun, Jun 25, 2023 at 01:53:41PM +0200, Pawel Dembicki wrote:
> This patch implement vlan filtering for vsc73xx driver.
>
> After vlan filtering start, switch is reconfigured from QinQ to simple
> vlan aware mode. It's required, because VSC73XX chips haven't support
> for inner vlan tag filter.
>
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> Signed-off-by: Pawel Dembicki <paweldembicki@...il.com>
> ---
> v2:
> - no changes done
>
> drivers/net/dsa/vitesse-vsc73xx-core.c | 101 +++++++++++++++++++++++++
> 1 file changed, 101 insertions(+)
>
> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
> index 457eb7fddf4c..c946464489ab 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx-core.c
> +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
> @@ -1226,6 +1226,30 @@ static int vsc73xx_port_set_double_vlan_aware(struct dsa_switch *ds, int port)
> return ret;
> }
>
> +static int
> +vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port,
> + bool vlan_filtering, struct netlink_ext_ack *extack)
> +{
> + int ret, i;
> +
> + if (vlan_filtering) {
> + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_VLAN_AWARE);
> + } else {
> + if (port == CPU_PORT)
> + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_CPU_AWARE);
> + else
> + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_AWARE);
> + }
Why do you need ports to be double VLAN aware when vlan_filtering=0?
Isn't VLAN_TCI_IGNORE_ENA sufficient to ignore the 802.1Q header from
incoming packets, and set up the PVIDs of user ports as egress-tagged on
the CPU port?
> +
> + for (i = 0; i <= 3072; i++) {
> + ret = vsc73xx_port_update_vlan_table(ds, port, i, 0);
> + if (ret)
> + return ret;
> + }
What is the purpose of this?
> +
> + return ret;
> +}
> +
> static int vsc73xx_vlan_set_untagged(struct dsa_switch *ds, int port, u16 vid,
> bool port_vlan)
> {
> @@ -1304,6 +1328,80 @@ static int vsc73xx_vlan_set_pvid(struct dsa_switch *ds, int port, u16 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;
> + int ret;
> +
> + /* 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;
> + }
> +
> + if (untagged && port != CPU_PORT) {
> + ret = vsc73xx_vlan_set_untagged(ds, port, vlan->vid, true);
> + if (ret)
> + return ret;
> + }
> + if (pvid && port != CPU_PORT) {
Missing logic to change hardware PVID only while VLAN-aware, and to
restore the tag_8021q PVID when the bridge VLAN awareness gets disabled.
DSA does not resolve the conflicts on resources between .port_vlan_add()
and .tag_8021q_vlan_add(), the driver must do that.
> + ret = vsc73xx_vlan_set_pvid(ds, port, vlan->vid, true);
> + if (ret)
> + return ret;
> + }
> +
> + ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 1);
> +
> + return ret;
Style: return vsc73xx_port_update_vlan_table(...)
> +}
> +
> +static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_vlan *vlan)
> +{
> + struct vsc73xx *vsc = ds->priv;
> + u16 vlan_no;
> + int ret;
> + u32 val;
> +
> + ret =
> + vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 0);
Style: single line
> + if (ret)
> + return ret;
> +
> + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, &val);
> +
> + if (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA) {
> + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_TXUPDCFG, &val);
> + vlan_no = (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID) >>
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT;
> + if (vlan_no == vlan->vid) {
> + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_TXUPDCFG,
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA,
> + 0);
> + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_TXUPDCFG,
> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0);
> + }
> + }
> +
> + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, &val);
> + vlan_no = val & VSC73XX_CAT_PORT_VLAN_VLAN_VID;
> + if (vlan_no && vlan_no == vlan->vid) {
> + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> + VSC73XX_CAT_PORT_VLAN,
> + VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0);
As documented in Documentation/networking/switchdev.rst:
When the bridge has VLAN filtering enabled and a PVID is not configured on the
ingress port, untagged and 802.1p tagged packets must be dropped. When the bridge
has VLAN filtering enabled and a PVID exists on the ingress port, untagged and
priority-tagged packets must be accepted and forwarded according to the
bridge's port membership of the PVID VLAN. When the bridge has VLAN filtering
disabled, the presence/lack of a PVID should not influence the packet
forwarding decision.
Setting the hardware PVID to 0 when the bridge PVID is deleted sounds
like it accomplishes none of those.
> + }
> +
> + return 0;
> +}
> +
> static void vsc73xx_update_forwarding_map(struct vsc73xx *vsc)
> {
> int i;
> @@ -1524,6 +1622,9 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = {
> .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,
> .tag_8021q_vlan_add = vsc73xx_tag_8021q_vlan_add,
> .tag_8021q_vlan_del = vsc73xx_tag_8021q_vlan_del,
> };
> --
> 2.34.1
>
Powered by blists - more mailing lists