netdev - Re: [PATCH v2 net 1/2] net: dsa: sja1105: always enable the INCL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <756bda986e5b9946e2035926dc0370d14138fd20.camel@redhat.com>
Date: Thu, 29 Jun 2023 14:38:35 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>, Florian Fainelli
 <f.fainelli@...il.com>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 net 1/2] net: dsa: sja1105: always enable the
 INCL_SRCPT option

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