Message-ID: <4cfeb80e-8799-44dc-9b8b-7b7e0e329541@redhat.com>
Date: Wed, 19 Mar 2025 22:21:55 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Jacob Keller <jacob.e.keller@...el.com>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Richard Cochran
 <richardcochran@...il.com>, Ruud Bos <kernel.hbk@...il.com>,
 Paul Barker <paul.barker.ct@...renesas.com>,
 Niklas Söderlund <niklas.soderlund@...natech.se>,
 Bryan Whitehead <bryan.whitehead@...rochip.com>,
 UNGLinuxDriver@...rochip.com,
 Florian Fainelli <florian.fainelli@...adcom.com>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>, Andrew Lunn <andrew@...n.ch>,
 Heiner Kallweit <hkallweit1@...il.com>, Russell King
 <linux@...linux.org.uk>, Jonathan Lemon <jonathan.lemon@...il.com>,
 Lasse Johnsen <l@...johnsen.me>, Vadim Fedorenko
 <vadim.fedorenko@...ux.dev>,
 Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH net v2 0/5] net: ptp: fix egregious supported flag checks

On 3/12/25 11:15 PM, Jacob Keller wrote:
> In preparation for adding .supported_extts_flags and
> .supported_perout_flags to the ptp_clock_info structure, fix a couple of
> places where drivers get existing flag gets grossly incorrect.
> 
> The igb driver claims 82580 supports strictly validating PTP_RISING_EDGE
> and PTP_FALLING_EDGE, but doesn't actually check the flags. Fix the driver
> to require that the request match both edges, as this is implied by the
> datasheet description.
> 
> The renesas driver also claims to support strict flag checking, but does
> not actually check the flags either. I do not have the data sheet for this
> device, so I do not know what edge it timestamps. For simplicity, just
> reject all requests with PTP_STRICT_FLAGS. This essentially prevents the
> PTP_EXTTS_REQUEST2 ioctl from working. Updating to correctly validate the
> flags will require someone who has the hardware to confirm the behavior.
> 
> The lan743x driver supports (and strictly validates) that the request is
> either PTP_RISING_EDGE or PTP_FALLING_EDGE but not both. However, it does
> not check the flags are one of the known valid flags. Thus, requests for
> PTP_EXT_OFF (and any future flag) will be accepted and misinterpreted. Add
> the appropriate check to reject unsupported PTP_EXT_OFF requests and future
> proof against new flags.
> 
> The broadcom PHY driver checks that PTP_PEROUT_PHASE is not set. This
> appears to be an attempt at rejecting unsupported flags. It is not robust
> against flag additions such as the PTP_PEROUT_ONE_SHOT, or anything added
> in the future. Fix this by instead checking against the negation of the
> supported PTP_PEROUT_DUTY_CYCLE instead.
> 
> The ptp_ocp driver supports PTP_PEROUT_PHASE and PTP_PEROUT_DUTY_CYCLE, but
> does not check unsupported flags. Add the appropriate check to ensure
> PTP_PEROUT_ONE_SHOT and any future flags are rejected as unsupported.
> 
> These are changes compile-tested, but I do not have hardware to validate the
> behavior.
> 
> There are a number of other drivers which enable periodic output or
> external timestamp requests, but which do not check flags at all. We could
> go through each of these drivers one-by-one and meticulously add a flag
> check. Instead, these drivers will be covered only by the upcoming
> .supported_extts_flags and .supported_perout_flags checks in a net-next
> series.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>

I admit I'm the most significant source of latency, but this series is
IMHO a bit risky to land this late in the cycle in the net tree,
especially considering the lack of H/W testing for the BCM phy.

What about applying this to net-next instead?

Thanks,

Paolo

Powered by blists - more mailing lists