Messages in this thread | | | Date | Mon, 3 Jul 2023 19:55:08 +0300 | From | Vladimir Oltean <> | Subject | Re: [PATCH net-next v2 6/7] net: dsa: vsc73xx: Add vlan filtering |
| |
On Thu, Jun 29, 2023 at 10:18:08PM +0200, Paweł Dembicki wrote: > niedz., 25 cze 2023 o 17:05 Vladimir Oltean <olteanv@gmail.com> napisał(a): > > 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? > > Because I want to forward tagged and untagged frames when > vlan_filtering is off. If I set VSC73XX_DOUBLE_VLAN_AWARE, I can put > all (tagged and untagged) traffic into the outer vlan, called by the > datasheet as "MAN space". In QinQ mode, it is possible to ignore what > goes from a particular port but it is possible to separate traffic > from different ports.
I think we may have some problem in finding common terminology.
Opening the manual and seeing the table "Customer Port Sample Configuration", I think it's indeed what you need. But I wouldn't call it "double VLAN aware". The port is actually configured to be VLAN *unaware* from the perspective of classification, and always encapsulate all packets in one more VLAN (the port PVID).
This switch's analyzer is always aware only of the outer VLAN header, and that's not "double VLAN aware" (it can perform no action based on the inner VLAN, if that exists), but it's fine for what is needed of it.
You might be mixing these with MAC_CFG::VLAN_AWR and MAC_CFG::VLAN_DBLAWR, which essentially are only there to allow single- and double-VLAN-tagged frames to be longer by 4 and 8 bytes, respectively, than the max frame size. I don't think that these 2 fields have any reason to depend upon the bridge VLAN awareness state of the port. They can be unconditionally enabled. After all, Linux only cares about MTU, and that is the size of the L2 payload, excluding any VLAN headers, if present.
I would suggest that if you exclude the MAC_CFG registers from vsc73xx_port_set_vlan_conf(), you end up with not as many VLAN awareness modes as you think. 2, to be precise: on or off. So you don't need the enum.
Also, AFAIU, I don't see a reason to modify CAT_VLAN_MISC::VLAN_KEEP_TAG_ENA from the value of 1 at all. You could always keep frames in the queue system with the VID attached, and strip that VID on egress, if necessary, via TXUPDCFG.
Not sure if you're noticed this, but drivers/net/ethernet/mscc/ and drivers/net/dsa/ocelot/ contain a driver for a newer generation of hardware than the VSC73xx, but many of the concepts apply. Maybe you can take a look at how some things were done there.
> > > + > > > + 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? > > I want to be sure that the table is cleared when vlan awareness is changed.
Yes, but why? That should specifically not be done, since there is no code in the kernel to replay the port_vlan_add() and tag_8021q_vlan_add() calls for you when the VLAN awareness state changes. If you delete the VLANs, they're gone, even though in software they're still there.
| |