Messages in this thread | | | Subject | Re: [PATCH v2 net 1/2] net: dsa: sja1105: always enable the INCL_SRCPT option | From | Paolo Abeni <> | Date | Thu, 29 Jun 2023 14:38:35 +0200 |
| |
On Thu, 2023-06-29 at 13:19 +0300, Vladimir Oltean wrote: > 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.
Ok, the above sounds like a good enough reply to me.
> 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.
[I'm mixing replies to your 2 emails here, I hope overall this is still human parsable ;) ]
Quickly skimming over the patch you shared I *think* it could be -net material, too. Given the mentioned lack of complains for the potential issue, I think it could be a follow-up to this series.
I'm applying it right now.
Thanks!
Paolo
| |