Re: [PATCH v2 net 1/2] net: dsa: sja1105: always enable the INCL_SRCPT option

[Vladimir Oltean]

On Thu, Jun 29, 2023 at 11:36:38AM +0200, Paolo Abeni wrote:
> > The big drawback with INCL_SRCPT is that it makes it impossible to
> > distinguish between an original MAC DA of 01:80:C2:XX:YY:ZZ and
> > 01:80:C2:AA:BB:ZZ, because the tagger just patches MAC DA bytes 3 and 4
> > with zeroes. Only if PTP RX timestamping is enabled, the switch will
> > generate a META follow-up frame containing the RX timestamp and the
> > original bytes 3 and 4 of the MAC DA. Those will be used to patch up the
> > original packet. Nonetheless, in the absence of PTP RX timestamping, we
> > have to live with this limitation, since it is more important to have
> > the more precise source port information for link-local traffic.
> 
> What if 2 different DSA are under the same linux bridge, so that the
> host has to forward in S/W the received frames? (and DA is incomplete)
> 
> It looks like that such frames will never reach the relevant
> destination?
> 
> Is such setup possible/relevant?
> 
> Thanks,
> 
> Paolo
>

They will have an incorrect MAC DA, with all the consequences of that.

Given the fact that the incl_srcpt was enabled up until now for the
vlan_filtering=1 bridge case only, this was already possible to see.
However it was never reported to me as being a problem, unlike what
is being fixed here.

I see no other escape than to unconditionally enable the send_meta
options as well, so that the overwritten MAC DA bytes can always be
reconstructed from the upcoming meta frames, even though the RX
timestamp (main payload of those meta frames) may or may not be useful.
Doing that might also have the benefit that it simplifies the code,
removing the need for tagger_data->rxtstamp_set_state() and
tagger_data->rxtstamp_get_state(), because with that simplification,
the tagger will always expect meta frames.

Because of the lack of complaints, I was considering that as net-next
material though.