Hi Vladimir,
On Thu, 29 Aug 2019 14:50:14 +0300, Vladimir Oltean <olte...@gmail.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
