netdev - Re: [PATCH v2 net-next 2/2] net: dsa: tag_8021q: Restore bridge VLANs when enabling vlan

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190829100257.GB8933@t480s.localdomain>
Date:   Thu, 29 Aug 2019 10:02:57 -0400
From:   Vivien Didelot <vivien.didelot@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Florian Fainelli <f.fainelli@...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>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net-next 2/2] net: dsa: tag_8021q: Restore bridge VLANs
 when enabling vlan_filtering

Hi Vladimir,

On Thu, 29 Aug 2019 14:50:14 +0300, Vladimir Oltean <olteanv@...il.com> wrote:
> > +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?

dsa_slave_* functions are the entry points for operations performed on the
net_device structures exposed to userspace. Using them elsewhere seems wrong.

dsa_port_* functions scope any dsa_port structure regardless its type though,
so I'd suggest duplicating a bit of code in tag_8021q.c to implement this
specific use case, until we figure out something nice to factorize.


Thank you,

	Vivien