From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: "Jacob Keller" <jacob.e.keller@intel.com>,
"Andrew Lunn" <andrew@lunn.ch>,
"Vladimir Oltean" <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Tony Nguyen" <anthony.l.nguyen@intel.com>,
"Przemek Kitszel" <przemyslaw.kitszel@intel.com>,
"Saeed Mahameed" <saeedm@nvidia.com>,
"Leon Romanovsky" <leon@kernel.org>,
"Tariq Toukan" <tariqt@nvidia.com>,
"Bryan Whitehead" <bryan.whitehead@microchip.com>,
UNGLinuxDriver@microchip.com,
"Horatiu Vultur" <horatiu.vultur@microchip.com>,
"Paul Barker" <paul.barker.ct@bp.renesas.com>,
"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
"Richard Cochran" <richardcochran@gmail.com>,
"Heiner Kallweit" <hkallweit1@gmail.com>,
"Russell King" <linux@armlinux.org.uk>,
"Andrei Botila" <andrei.botila@oss.nxp.com>,
"Claudiu Manoil" <claudiu.manoil@nxp.com>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH net-next 0/2] net: ptp: driver opt-in for supported PTP ioctl flags
Date: Thu, 10 Apr 2025 11:07:16 +0100 [thread overview]
Message-ID: <454072e0-9fd8-4deb-a97e-b454d5e3fd5f@linux.dev> (raw)
In-Reply-To: <20250408-jk-supported-perout-flags-v1-0-d2f8e3df64f3@intel.com>
On 08/04/2025 21:55, Jacob Keller wrote:
> Both the PTP_EXTTS_REQUEST(2) and PTP_PEROUT_REQUEST(2) ioctls take flags
> from userspace to modify their behavior. Drivers are supposed to check
> these flags, rejecting requests for flags they do not support.
>
> Many drivers today do not check these flags, despite many attempts to
> squash individual drivers as these mistakes are discovered. Additionally,
> any new flags added can require updating every driver if their validation
> checks are poorly implemented.
>
> It is clear that driver authors will not reliably check for unsupported
> flags. The root of the issue is that drivers must essentially opt out of
> every flag, rather than opt in to the ones they support.
>
> Instead, lets introduce .supported_perout_flags and .supported_extts_flags
> to the ptp_clock_info structure. This is a pattern taken from several
> ethtool ioctls which enabled validation to move out of the drivers and into
> the shared ioctl handlers. This pattern has worked quite well and makes it
> much more difficult for drivers to accidentally accept flags they do not
> support.
>
> With this approach, drivers which do not set the supported fields will have
> the core automatically reject any request which has flags. Drivers must opt
> in to each flag they support by adding it to the list, with the sole
> exception being the PTP_ENABLE_FEATURE flag of the PTP_EXTTS_REQUEST ioctl
> since it is entirely handled by the ptp_chardev.c file.
>
> This change will ensure that all current and future drivers are safe for
> extension when we need to extend these ioctls.
>
> I opted to keep all the driver changes into one patch per ioctl type. The
> changes are relatively small and straight forward. Splitting it per-driver
> would make the series large, and also break flags between the introduction
> of the supported field and setting it in each driver.
>
> The non-Intel drivers are compile-tested only, and I would appreciate
> confirmation and testing from their respective maintainers. (It is also
> likely that I missed some of the driver authors especially for drivers
> which didn't make any checks at all and do not set either of the supported
> flags yet)
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
For the series:
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
WARNING: multiple messages have this Message-ID (diff)
From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: "Jacob Keller" <jacob.e.keller@intel.com>,
"Andrew Lunn" <andrew@lunn.ch>,
"Vladimir Oltean" <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Tony Nguyen" <anthony.l.nguyen@intel.com>,
"Przemek Kitszel" <przemyslaw.kitszel@intel.com>,
"Saeed Mahameed" <saeedm@nvidia.com>,
"Leon Romanovsky" <leon@kernel.org>,
"Tariq Toukan" <tariqt@nvidia.com>,
"Bryan Whitehead" <bryan.whitehead@microchip.com>,
UNGLinuxDriver@microchip.com,
"Horatiu Vultur" <horatiu.vultur@microchip.com>,
"Paul Barker" <paul.barker.ct@bp.renesas.com>,
"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
"Richard Cochran" <richardcochran@gmail.com>,
"Heiner Kallweit" <hkallweit1@gmail.com>,
"Russell King" <linux@armlinux.org.uk>,
"Andrei Botila" <andrei.botila@oss.nxp.com>,
"Claudiu Manoil" <claudiu.manoil@nxp.com>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH net-next 0/2] net: ptp: driver opt-in for supported PTP ioctl flags
Date: Thu, 10 Apr 2025 11:07:16 +0100 [thread overview]
Message-ID: <454072e0-9fd8-4deb-a97e-b454d5e3fd5f@linux.dev> (raw)
In-Reply-To: <20250408-jk-supported-perout-flags-v1-0-d2f8e3df64f3@intel.com>
On 08/04/2025 21:55, Jacob Keller wrote:
> Both the PTP_EXTTS_REQUEST(2) and PTP_PEROUT_REQUEST(2) ioctls take flags
> from userspace to modify their behavior. Drivers are supposed to check
> these flags, rejecting requests for flags they do not support.
>
> Many drivers today do not check these flags, despite many attempts to
> squash individual drivers as these mistakes are discovered. Additionally,
> any new flags added can require updating every driver if their validation
> checks are poorly implemented.
>
> It is clear that driver authors will not reliably check for unsupported
> flags. The root of the issue is that drivers must essentially opt out of
> every flag, rather than opt in to the ones they support.
>
> Instead, lets introduce .supported_perout_flags and .supported_extts_flags
> to the ptp_clock_info structure. This is a pattern taken from several
> ethtool ioctls which enabled validation to move out of the drivers and into
> the shared ioctl handlers. This pattern has worked quite well and makes it
> much more difficult for drivers to accidentally accept flags they do not
> support.
>
> With this approach, drivers which do not set the supported fields will have
> the core automatically reject any request which has flags. Drivers must opt
> in to each flag they support by adding it to the list, with the sole
> exception being the PTP_ENABLE_FEATURE flag of the PTP_EXTTS_REQUEST ioctl
> since it is entirely handled by the ptp_chardev.c file.
>
> This change will ensure that all current and future drivers are safe for
> extension when we need to extend these ioctls.
>
> I opted to keep all the driver changes into one patch per ioctl type. The
> changes are relatively small and straight forward. Splitting it per-driver
> would make the series large, and also break flags between the introduction
> of the supported field and setting it in each driver.
>
> The non-Intel drivers are compile-tested only, and I would appreciate
> confirmation and testing from their respective maintainers. (It is also
> likely that I missed some of the driver authors especially for drivers
> which didn't make any checks at all and do not set either of the supported
> flags yet)
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
For the series:
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
next prev parent reply other threads:[~2025-04-10 10:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-08 20:55 [Intel-wired-lan] [PATCH net-next 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Jacob Keller
2025-04-08 20:55 ` Jacob Keller
2025-04-08 20:55 ` [Intel-wired-lan] [PATCH net-next 1/2] net: ptp: introduce .supported_extts_flags to ptp_clock_info Jacob Keller
2025-04-08 20:55 ` Jacob Keller
2025-04-12 1:20 ` [Intel-wired-lan] " Jakub Kicinski
2025-04-12 1:20 ` Jakub Kicinski
2025-04-14 20:00 ` [Intel-wired-lan] " Jacob Keller
2025-04-14 20:00 ` Jacob Keller
2025-04-08 20:55 ` [Intel-wired-lan] [PATCH net-next 2/2] net: ptp: introduce .supported_perout_flags " Jacob Keller
2025-04-08 20:55 ` Jacob Keller
2025-04-10 10:07 ` Vadim Fedorenko [this message]
2025-04-10 10:07 ` [PATCH net-next 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Vadim Fedorenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=454072e0-9fd8-4deb-a97e-b454d5e3fd5f@linux.dev \
--to=vadim.fedorenko@linux.dev \
--cc=UNGLinuxDriver@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=andrei.botila@oss.nxp.com \
--cc=andrew@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=bryan.whitehead@microchip.com \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=horatiu.vultur@microchip.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=niklas.soderlund@ragnatech.se \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=paul.barker.ct@bp.renesas.com \
--cc=przemyslaw.kitszel@intel.com \
--cc=richardcochran@gmail.com \
--cc=saeedm@nvidia.com \
--cc=tariqt@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.