| 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
| ||
|
Date: Fri, 4 Aug 2023 09:21:28 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>, Maxim Georgiev <glipus@...il.com>,
Horatiu Vultur <horatiu.vultur@...rochip.com>, Köry Maincent <kory.maincent@...tlin.com>,
Maxime Chevallier <maxime.chevallier@...tlin.com>, Richard Cochran <richardcochran@...il.com>,
Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
Gerhard Engleder <gerhard@...leder-embedded.com>, Hangbin Liu <liuhangbin@...il.com>,
Russell King <linux@...linux.org.uk>, Heiner Kallweit <hkallweit1@...il.com>,
Jacob Keller <jacob.e.keller@...el.com>, Jay Vosburgh <j.vosburgh@...il.com>,
Andy Gospodarek <andy@...yhouse.net>, Wei Fang <wei.fang@....com>,
Shenwei Wang <shenwei.wang@....com>, Clark Wang <xiaoning.wang@....com>,
NXP Linux Team <linux-imx@....com>, UNGLinuxDriver@...rochip.com,
Lars Povlsen <lars.povlsen@...rochip.com>, Steen Hegelund <Steen.Hegelund@...rochip.com>,
Daniel Machon <daniel.machon@...rochip.com>, Simon Horman <simon.horman@...igine.com>,
Casper Andersson <casper.casan@...il.com>, Sergey Organov <sorganov@...il.com>,
Michal Kubecek <mkubecek@...e.cz>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 net-next 12/12] net: remove phy_has_hwtstamp() ->
phy_mii_ioctl() decision from converted drivers
On Tue, Aug 1, 2023 at 4:29 PM Vladimir Oltean <vladimir.oltean@....com> wrote:
>
> It is desirable that the new .ndo_hwtstamp_set() API gives more
> uniformity, less overhead and future flexibility w.r.t. the PHY
> timestamping behavior.
>
> Currently there are some drivers which allow PHY timestamping through
> the procedure mentioned in Documentation/networking/timestamping.rst.
> They don't do anything locally if phy_has_hwtstamp() is set, except for
> lan966x which installs PTP packet traps.
>
> Centralize that behavior in a new dev_set_hwtstamp_phylib() code
> function, which calls either phy_mii_ioctl() for the phylib PHY,
> or .ndo_hwtstamp_set() of the netdev, based on a single policy
> (currently simplistic: phy_has_hwtstamp()).
>
> Any driver converted to .ndo_hwtstamp_set() will automatically opt into
> the centralized phylib timestamping policy. Unconverted drivers still
> get to choose whether they let the PHY handle timestamping or not.
>
> Netdev drivers with integrated PHY drivers that don't use phylib
> presumably don't set dev->phydev, and those will always see
> HWTSTAMP_SOURCE_NETDEV requests even when converted. The timestamping
> policy will remain 100% up to them.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> Tested-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> ---
> +/**
> + * dev_set_hwtstamp_phylib() - Change hardware timestamping of NIC
> + * or of attached phylib PHY
> + * @dev: Network device
> + * @cfg: Timestamping configuration structure
> + * @extack: Netlink extended ack message structure, for error reporting
> + *
> + * Helper for enforcing a common policy that phylib timestamping, if available,
> + * should take precedence in front of hardware timestamping provided by the
> + * netdev. If the netdev driver needs to perform specific actions even for PHY
> + * timestamping to work properly (a switch port must trap the timestamped
> + * frames and not forward them), it must set IFF_SEE_ALL_HWTSTAMP_REQUESTS in
> + * dev->priv_flags.
> + */
> +static int dev_set_hwtstamp_phylib(struct net_device *dev,
> + struct kernel_hwtstamp_config *cfg,
> + struct netlink_ext_ack *extack)
> +{
> + const struct net_device_ops *ops = dev->netdev_ops;
> + bool phy_ts = phy_has_hwtstamp(dev->phydev);
> + struct kernel_hwtstamp_config old_cfg = {};
> + bool changed = false;
> + int err;
> +
> + cfg->source = phy_ts ? HWTSTAMP_SOURCE_PHYLIB : HWTSTAMP_SOURCE_NETDEV;
> +
> + if (!phy_ts || (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
> + err = ops->ndo_hwtstamp_get(dev, &old_cfg);
old_cfg.ifr is NULL at this point.
This causes a crash later in generic_hwtstamp_ioctl_lower()
ifrr.ifr_ifru = kernel_cfg->ifr->ifr_ifru;
> + if (err)
> + return err;
> +
> + err = ops->ndo_hwtstamp_set(dev, cfg, extack);
> + if (err) {
> + if (extack->_msg)
> + netdev_err(dev, "%s\n", extack->_msg);
> + return err;
> + }
> +
> + changed = kernel_hwtstamp_config_changed(&old_cfg, cfg);
> + }
> +
> + if (phy_ts) {
> + err = phy_hwtstamp_set(dev->phydev, cfg, extack);
> + if (err) {
> + if (changed)
> + ops->ndo_hwtstamp_set(dev, &old_cfg, NULL);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
> {
> const struct net_device_ops *ops = dev->netdev_ops;
> @@ -314,12 +392,9 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
> if (!netif_device_present(dev))
> return -ENODEV;
>
> - err = ops->ndo_hwtstamp_set(dev, &kernel_cfg, &extack);
> - if (err) {
> - if (extack._msg)
> - netdev_err(dev, "%s\n", extack._msg);
> + err = dev_set_hwtstamp_phylib(dev, &kernel_cfg, &extack);
> + if (err)
> return err;
> - }
>
> /* The driver may have modified the configuration, so copy the
> * updated version of it back to user space
> @@ -362,7 +437,7 @@ int generic_hwtstamp_get_lower(struct net_device *dev,
> return -ENODEV;
>
> if (ops->ndo_hwtstamp_get)
> - return ops->ndo_hwtstamp_get(dev, kernel_cfg);
> + return dev_get_hwtstamp_phylib(dev, kernel_cfg);
>
> /* Legacy path: unconverted lower driver */
> return generic_hwtstamp_ioctl_lower(dev, SIOCGHWTSTAMP, kernel_cfg);
> @@ -379,7 +454,7 @@ int generic_hwtstamp_set_lower(struct net_device *dev,
> return -ENODEV;
>
> if (ops->ndo_hwtstamp_set)
> - return ops->ndo_hwtstamp_set(dev, kernel_cfg, extack);
> + return dev_set_hwtstamp_phylib(dev, kernel_cfg, extack);
>
> /* Legacy path: unconverted lower driver */
> return generic_hwtstamp_ioctl_lower(dev, SIOCSHWTSTAMP, kernel_cfg);
> --
> 2.34.1
>
Powered by blists - more mailing lists