Re: [PATCH net-next 3/4] net: dsa: tag_ocelot: use a short prefix on both ingress and egress

[Florian Fainelli <f.fainelli@gmail.com>]

On 5/11/2020 1:20 PM, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> There are 2 goals that we follow:
> 
> - Reduce the header size
> - Make the header size equal between RX and TX
> 
> The issue that required long prefix on RX was the fact that the ocelot
> DSA tag, being put before Ethernet as it is, would overlap with the area
> that a DSA master uses for RX filtering (destination MAC address
> mainly).
> 
> Now that we can ask DSA to put the master in promiscuous mode, in theory
> we could remove the prefix altogether and call it a day, but it looks
> like we can't. Using no prefix on ingress, some packets (such as ICMP)
> would be received, while others (such as PTP) would not be received.
> This is because the DSA master we use (enetc) triggers parse errors
> ("MAC rx frame errors") presumably because it sees Ethernet frames with
> a bad length. And indeed, when using no prefix, the EtherType (bytes
> 12-13 of the frame, bits 96-111) falls over the REW_VAL field from the
> extraction header, aka the PTP timestamp.
> 
> When turning the short (32-bit) prefix on, the EtherType overlaps with
> bits 64-79 of the extraction header, which are a reserved area
> transmitted as zero by the switch. The packets are not dropped by the
> DSA master with a short prefix. Actually, the frames look like this in
> tcpdump (below is a PTP frame, with an extra dsa_8021q tag - dadb 0482 -
> added by a downstream sja1105).
> 
> 89:0c:a9:f2:01:00 > 88:80:00:0a:00:1d, 802.3, length 0: LLC, \
> 	dsap Unknown (0x10) Individual, ssap ProWay NM (0x0e) Response, \
> 	ctrl 0x0004: Information, send seq 2, rcv seq 0, \
> 	Flags [Response], length 78
> 
> 0x0000:  8880 000a 001d 890c a9f2 0100 0000 100f  ................
> 0x0010:  0400 0000 0180 c200 000e 001f 7b63 0248  ............{c.H
> 0x0020:  dadb 0482 88f7 1202 0036 0000 0000 0000  .........6......
> 0x0030:  0000 0000 0000 0000 0000 001f 7bff fe63  ............{..c
> 0x0040:  0248 0001 1f81 0500 0000 0000 0000 0000  .H..............
> 0x0050:  0000 0000 0000 0000 0000 0000            ............
> 
> So the short prefix is our new default: we've shortened our RX frames by
> 12 octets, increased TX by 4, and headers are now equal between RX and
> TX. Note that we still need promiscuous mode for the DSA master to not
> drop it.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian