| 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: <CA+h21hq=VVw0p0OjGaPx2-c4FE1ge-STRVHYZ6P62c-+_xW0nw@mail.gmail.com>
Date: Thu, 29 Aug 2019 14:50:14 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Florian Fainelli <f.fainelli@...il.com>,
Vivien Didelot <vivien.didelot@...il.com>,
Andrew Lunn <andrew@...n.ch>, Ido Schimmel <idosch@...sch.org>,
Roopa Prabhu <roopa@...ulusnetworks.com>,
nikolay@...ulusnetworks.com,
"David S. Miller" <davem@...emloft.net>
Cc: netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net-next 2/2] net: dsa: tag_8021q: Restore bridge VLANs
when enabling vlan_filtering
Vivien,
On Sun, 25 Aug 2019 at 21:46, Vladimir Oltean <olteanv@...il.com> wrote:
>
> The bridge core assumes that enabling/disabling vlan_filtering will
> translate into the simple toggling of a flag for switchdev drivers.
>
> That is clearly not the case for sja1105, which alters the VLAN table
> and the pvids in order to obtain port separation in standalone mode.
>
> There are 2 parts to the issue.
>
> First, tag_8021q changes the pvid to a unique per-port rx_vid for frame
> identification. But we need to disable tag_8021q when vlan_filtering
> kicks in, and at that point, the VLAN configured as pvid will have to be
> removed from the filtering table of the ports. With an invalid pvid, the
> ports will drop all traffic. Since the bridge will not call any vlan
> operation through switchdev after enabling vlan_filtering, we need to
> ensure we're in a functional state ourselves. Hence read the pvid that
> the bridge is aware of, and program that into our ports.
>
> Secondly, tag_8021q uses the 1024-3071 range privately in
> vlan_filtering=0 mode. Had the user installed one of these VLANs during
> a previous vlan_filtering=1 session, then upon the next tag_8021q
> cleanup for vlan_filtering to kick in again, VLANs in that range will
> get deleted unconditionally, hence breaking user expectation. So when
> deleting the VLANs, check if the bridge had knowledge about them, and if
> it did, re-apply the settings. Wrap this logic inside a
> dsa_8021q_vid_apply helper function to reduce code duplication.
>
> Signed-off-by: Vladimir Oltean <olteanv@...il.com>
> ---
> net/dsa/tag_8021q.c | 91 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 71 insertions(+), 20 deletions(-)
>
> diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
> index 67a1bc635a7b..81f943e365b9 100644
> --- a/net/dsa/tag_8021q.c
> +++ b/net/dsa/tag_8021q.c
> @@ -93,6 +93,68 @@ int dsa_8021q_rx_source_port(u16 vid)
> }
> EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
>
> +static int dsa_8021q_restore_pvid(struct dsa_switch *ds, int port)
> +{
> + struct bridge_vlan_info vinfo;
> + struct net_device *slave;
> + u16 pvid;
> + int err;
> +
> + if (!dsa_is_user_port(ds, port))
> + return 0;
> +
> + slave = ds->ports[port].slave;
> +
> + err = br_vlan_get_pvid(slave, &pvid);
> + if (err < 0) {
> + dev_err(ds->dev, "Couldn't determine bridge PVID\n");
> + return err;
> + }
> +
> + err = br_vlan_get_info(slave, pvid, &vinfo);
> + if (err < 0) {
> + dev_err(ds->dev, "Couldn't determine PVID attributes\n");
> + return err;
> + }
> +
> + return dsa_port_vid_add(&ds->ports[port], pvid, vinfo.flags);
If the bridge had installed a dsa_8021q VLAN here, I need to use the
dsa_slave_vid_add logic to restore it. The dsa_8021q flags on the CPU
port are "ingress tagged", but that may not be the case for the bridge
VLAN.
Should I expose dsa_slave_vlan_add in dsa_priv.h, or should I just
open-code another dsa_port_vid_add for dp->cpu_dp, duplicating a bit
of code from dsa_slave_vlan_add?
> +}
> +
> +/* If @enabled is true, installs @vid with @flags into the switch port's HW
> + * filter.
> + * If @enabled is false, deletes @vid (ignores @flags) from the port. Had the
> + * user explicitly configured this @vid through the bridge core, then the @vid
> + * is installed again, but this time with the flags from the bridge layer.
> + */
> +static int dsa_8021q_vid_apply(struct dsa_switch *ds, int port, u16 vid,
> + u16 flags, bool enabled)
> +{
> + struct dsa_port *dp = &ds->ports[port];
> + struct bridge_vlan_info vinfo;
> + int err;
> +
> + if (enabled)
> + return dsa_port_vid_add(dp, vid, flags);
> +
> + err = dsa_port_vid_del(dp, vid);
> + if (err < 0)
> + return err;
> +
> + /* Nothing to restore from the bridge for a non-user port */
> + if (!dsa_is_user_port(ds, port))
> + return 0;
> +
> + err = br_vlan_get_info(dp->slave, vid, &vinfo);
> + /* Couldn't determine bridge attributes for this vid,
> + * it means the bridge had not configured it.
> + */
> + if (err < 0)
> + return 0;
> +
> + /* Restore the VID from the bridge */
> + return dsa_port_vid_add(dp, vid, vinfo.flags);
> +}
> +
> /* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
> * front-panel switch port (here swp0).
> *
> @@ -148,8 +210,6 @@ EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
> int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> {
> int upstream = dsa_upstream_port(ds, port);
> - struct dsa_port *dp = &ds->ports[port];
> - struct dsa_port *upstream_dp = &ds->ports[upstream];
> u16 rx_vid = dsa_8021q_rx_vid(ds, port);
> u16 tx_vid = dsa_8021q_tx_vid(ds, port);
> int i, err;
> @@ -166,7 +226,6 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> * restrictions, so there are no concerns about leaking traffic.
> */
> for (i = 0; i < ds->num_ports; i++) {
> - struct dsa_port *other_dp = &ds->ports[i];
> u16 flags;
>
> if (i == upstream)
> @@ -179,10 +238,7 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> /* The RX VID is a regular VLAN on all others */
> flags = BRIDGE_VLAN_INFO_UNTAGGED;
>
> - if (enabled)
> - err = dsa_port_vid_add(other_dp, rx_vid, flags);
> - else
> - err = dsa_port_vid_del(other_dp, rx_vid);
> + err = dsa_8021q_vid_apply(ds, i, rx_vid, flags, enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply RX VID %d to port %d: %d\n",
> rx_vid, port, err);
> @@ -193,10 +249,7 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> /* CPU port needs to see this port's RX VID
> * as tagged egress.
> */
> - if (enabled)
> - err = dsa_port_vid_add(upstream_dp, rx_vid, 0);
> - else
> - err = dsa_port_vid_del(upstream_dp, rx_vid);
> + err = dsa_8021q_vid_apply(ds, upstream, rx_vid, 0, enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply RX VID %d to port %d: %d\n",
> rx_vid, port, err);
> @@ -204,26 +257,24 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> }
>
> /* Finally apply the TX VID on this port and on the CPU port */
> - if (enabled)
> - err = dsa_port_vid_add(dp, tx_vid, BRIDGE_VLAN_INFO_UNTAGGED);
> - else
> - err = dsa_port_vid_del(dp, tx_vid);
> + err = dsa_8021q_vid_apply(ds, port, tx_vid, BRIDGE_VLAN_INFO_UNTAGGED,
> + enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply TX VID %d on port %d: %d\n",
> tx_vid, port, err);
> return err;
> }
> - if (enabled)
> - err = dsa_port_vid_add(upstream_dp, tx_vid, 0);
> - else
> - err = dsa_port_vid_del(upstream_dp, tx_vid);
> + err = dsa_8021q_vid_apply(ds, upstream, tx_vid, 0, enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply TX VID %d on port %d: %d\n",
> tx_vid, upstream, err);
> return err;
> }
>
> - return 0;
> + if (!enabled)
> + err = dsa_8021q_restore_pvid(ds, port);
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(dsa_port_setup_8021q_tagging);
>
> --
> 2.17.1
>
Thanks,
-Vladimir
Powered by blists - more mailing lists