Re: [PATCH net-next 0/4] Fix broken PTP over IP on Ocelot switches

From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Po Liu <po.liu@nxp.com>, "David S. Miller" <davem@davemloft.net>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Antoine Tenart <atenart@kernel.org>,
	"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
	"Y.B. Lu" <yangbo.lu@nxp.com>, Rui Sousa <rui.sousa@nxp.com>,
	Richard Cochran <richardcochran@gmail.com>,
	"Allan W . Nielsen" <allan.nielsen@microchip.com>
Subject: Re: [PATCH net-next 0/4] Fix broken PTP over IP on Ocelot switches
Date: Fri, 26 Nov 2021 09:55:00 +0000	[thread overview]
Message-ID: <20211126095500.tkcctzfh5zp2nluc@skbuf> (raw)
In-Reply-To: <20211125190101.63f1f0a2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Thu, Nov 25, 2021 at 07:01:01PM -0800, Jakub Kicinski wrote:
> On Thu, 25 Nov 2021 23:45:21 +0000 Vladimir Oltean wrote:
> > On Fri, Nov 26, 2021 at 01:21:14AM +0200, Vladimir Oltean wrote:
> > > Po Liu reported recently that timestamping PTP over IPv4 is broken using
> > > the felix driver on NXP LS1028A. This has been known for a while, of
> > > course, since it has always been broken. The reason is because IP PTP
> > > packets are currently treated as unknown IP multicast, which is not
> > > flooded to the CPU port in the ocelot driver design, so packets don't
> > > reach the ptp4l program.
> > >
> > > The series solves the problem by installing packet traps per port when
> > > the timestamping ioctl is called, depending on the RX filter selected
> > > (L2, L4 or both).
> >
> > I don't know why I targeted these patches to "net-next". Habit I guess.
> > Nonetheless, they apply equally well to "net", can they be considered
> > for merging there without me resending?
>
> Only patch 1 looks like a fix, tho? Patch 4 seems to fall into
> the "this never worked and doesn't cause a crash" category.
>
> I'm hoping to send a PR tomorrow, so if you resend quickly it
> will be in net-next soon.

It's true that a lot of work went into ocelot_vcap.c in order to make it
safely usable for traps outside of the tc-flower offload, and I
understand that you need to draw the line somewhere. But on the other
hand, this is fixing very real problems that are bothering real users.
Patch 1, not so much, it popped up as a result of discussions and
looking at code. None of the bugs fixed here cause a crash, it's just
that things don't work as expected. Technically, a user could still set
up the appropriate traps via tc-flower and PTP would work, but they'd
have to know that they need to, in the first place. So I would still be
very appreciative if all 4 patches would be considered for inclusion
into "net". I'm not expecting them to be backported very far, of course,
but as long as they reach at least v5.15 I'm happy.