linux-kernel - 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: <20230629114601.guhv5l7jz6t4cmrw@skbuf>
Date:   Thu, 29 Jun 2023 14:46:01 +0300
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Paolo Abeni <pabeni@...hat.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, Jun 29, 2023 at 01:19:50PM +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.
> 
> 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.

This is the patch that would avoid that. Not sure whether I should
include it as part of the "net" series or if it's better to retarget
it to net-next. It has an interesting diffstat of "5 files changed,
7 insertions(+), 93 deletions(-)", so there's that.

The patch is attached because I'm trying not to post code that patchwork
would think it needs to process.

View attachment "0001-net-dsa-sja1105-always-enable-the-send_meta-options.patch" of type "text/x-diff" (10102 bytes)