Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <756bda986e5b9946e2035926dc0370d14138fd20.camel@redhat.com>
Subject: Re: [PATCH v2 net 1/2] net: dsa: sja1105: always enable the
 INCL_SRCPT option
From:   Paolo Abeni <pabeni@redhat.com>
To:     Vladimir Oltean <vladimir.oltean@nxp.com>
Cc:     netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org
Date:   Thu, 29 Jun 2023 14:38:35 +0200
In-Reply-To: <20230629101950.7s3kagwvkzlnu7ao@skbuf>
References: <20230627094207.3385231-1-vladimir.oltean@nxp.com>
         <20230627094207.3385231-2-vladimir.oltean@nxp.com>
         <f494387c8d55d9b1d5a3e88beedeeb448f2e6cc3.camel@redhat.com>
         <20230629101950.7s3kagwvkzlnu7ao@skbuf>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

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.
> >=20
> > 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)
> >=20
> > It looks like that such frames will never reach the relevant
> > destination?
> >=20
> > Is such setup possible/relevant?
> >=20
> > Thanks,
> >=20
> > Paolo
> >=20
>=20
> They will have an incorrect MAC DA, with all the consequences of that.
>=20
> Given the fact that the incl_srcpt was enabled up until now for the
> vlan_filtering=3D1 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.
>=20
> 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