On Tue, May 17, 2022 at 09:01:56AM +0200, Maxime Chevallier wrote: > Hi Vlad, > > On Sat, 14 May 2022 22:40:03 +0000 > Vladimir Oltean <vladimir.oltean@xxxxxxx> wrote: > > > On Sat, May 14, 2022 at 05:06:53PM +0200, Maxime Chevallier wrote: > > > This tagging protocol is designed for the situation where the link > > > between the MAC and the Switch is designed such that the Destination > > > Port, which is usually embedded in some part of the Ethernet > > > Header, is sent out-of-band, and isn't present at all in the > > > Ethernet frame. > > > > > > This can happen when the MAC and Switch are tightly integrated on an > > > SoC, as is the case with the Qualcomm IPQ4019 for example, where > > > the DSA tag is inserted directly into the DMA descriptors. In that > > > case, the MAC driver is responsible for sending the tag to the > > > switch using the out-of-band medium. To do so, the MAC driver needs > > > to have the information of the destination port for that skb. > > > > > > This out-of-band tagging protocol is using the very beggining of > > > the skb headroom to store the tag. The drawback of this approch is > > > that the headroom isn't initialized upon allocating it, therefore > > > we have a chance that the garbage data that lies there at > > > allocation time actually ressembles a valid oob tag. This is only > > > problematic if we are sending/receiving traffic on the master port, > > > which isn't a valid DSA use-case from the beggining. When dealing > > > from traffic to/from a slave port, then the oob tag will be > > > initialized properly by the tagger or the mac driver through the > > > use of the dsa_oob_tag_push() call. > > > > > > Signed-off-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx> > > > --- > > > > Why put the DSA pseudo-header at skb->head rather than push it using > > skb_push()? I thought you were going to check for the presence of a > > DSA header using something like skb->mac_len == ETH_HLEN + tag len, > > but right now it sounds like treating garbage in the headroom as a > > valid DSA tag is indeed a potential problem. If you can't sort that > > out using information from the header offsets alone, maybe an skb > > extension is required? > > Indeed, I thought of that, the main reason is that pushing/poping in > itself is not enough, you also have to move the whole mac_header to > leave room for the tag, and then re-set it in it's original location. > There's nothing wrong with this, but it looked a bit cumbersome just to > insert a dummy tag that gets removed rightaway. Does that make sense ? You're thinking about inserting a header before the EtherType. But what has been said was to _prepend_ a header, i.e. put it before the Ethernet MAC DA. That way you don't need to move the Ethernet header. But anyway, too much talk for mostly nothing, see below. > But yes I would really like to get a way to know wether the tag is > there or not, I'll dig a bit more to see if I can find a way to get > this info from the various skb offsets in a reliable way. Without an skb extension, this seems like an impossible task to me (which should also answer Florian's request for feedback on the proposal to share skb->cb with GRO, the qdisc, and whomever else there might be in the path between the DSA master and the switch).
