| Message ID | 20221111211020.543540-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ed1fe1bebe18884b11e5536b5ac42e3a48960835 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: dsa: make dsa_master_ioctl() see through port_hwtstamp_get() shims | expand |
Hi Vladimir, On Fri, Nov 11, 2022 at 6:10 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > There are multi-generational drivers like mv88e6xxx which have code like > this: > > int mv88e6xxx_port_hwtstamp_get(struct dsa_switch *ds, int port, > struct ifreq *ifr) > { > if (!chip->info->ptp_support) > return -EOPNOTSUPP; > > ... > } > > DSA wants to deny PTP timestamping on the master if the switch supports > timestamping too. However it currently relies on the presence of the > port_hwtstamp_get() callback to determine PTP capability, and this > clearly does not work in that case (method is present but returns > -EOPNOTSUPP). > > We should not deny PTP on the DSA master for those switches which truly > do not support hardware timestamping. > > Create a dsa_port_supports_hwtstamp() method which actually probes for > support by calling port_hwtstamp_get() and seeing whether that returned > -EOPNOTSUPP or not. > > Fixes: f685e609a301 ("net: dsa: Deny PTP on master if switch supports it") > Link: https://patchwork.kernel.org/project/netdevbpf/patch/20221110124345.3901389-1-festevam@gmail.com/ > Reported-by: Fabio Estevam <festevam@gmail.com> > Reported-by: Steffen Bätz <steffen@innosonix.de> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Thanks for your fix! Tested-by: Fabio Estevam <festevam@denx.de>
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Fri, 11 Nov 2022 23:10:20 +0200 you wrote: > There are multi-generational drivers like mv88e6xxx which have code like > this: > > int mv88e6xxx_port_hwtstamp_get(struct dsa_switch *ds, int port, > struct ifreq *ifr) > { > if (!chip->info->ptp_support) > return -EOPNOTSUPP; > > [...] Here is the summary with links: - [net] net: dsa: make dsa_master_ioctl() see through port_hwtstamp_get() shims https://git.kernel.org/netdev/net/c/ed1fe1bebe18 You are awesome, thank you!
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 6e65c7ffd6f3..71e9707d11d4 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -210,6 +210,7 @@ static inline struct net_device *dsa_master_find_slave(struct net_device *dev, extern struct rtnl_link_ops dsa_link_ops __read_mostly; /* port.c */ +bool dsa_port_supports_hwtstamp(struct dsa_port *dp, struct ifreq *ifr); void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp, const struct dsa_device_ops *tag_ops); int dsa_port_set_state(struct dsa_port *dp, u8 state, bool do_fast_age); diff --git a/net/dsa/master.c b/net/dsa/master.c index 40367ab41cf8..421de166515f 100644 --- a/net/dsa/master.c +++ b/net/dsa/master.c @@ -204,8 +204,7 @@ static int dsa_master_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) * switch in the tree that is PTP capable. */ list_for_each_entry(dp, &dst->ports, list) - if (dp->ds->ops->port_hwtstamp_get || - dp->ds->ops->port_hwtstamp_set) + if (dsa_port_supports_hwtstamp(dp, ifr)) return -EBUSY; break; } diff --git a/net/dsa/port.c b/net/dsa/port.c index 208168276995..750fe68d9b2a 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -110,6 +110,22 @@ static bool dsa_port_can_configure_learning(struct dsa_port *dp) return !err; } +bool dsa_port_supports_hwtstamp(struct dsa_port *dp, struct ifreq *ifr) +{ + struct dsa_switch *ds = dp->ds; + int err; + + if (!ds->ops->port_hwtstamp_get || !ds->ops->port_hwtstamp_set) + return false; + + /* "See through" shim implementations of the "get" method. + * This will clobber the ifreq structure, but we will either return an + * error, or the master will overwrite it with proper values. + */ + err = ds->ops->port_hwtstamp_get(ds, dp->index, ifr); + return err != -EOPNOTSUPP; +} + int dsa_port_set_state(struct dsa_port *dp, u8 state, bool do_fast_age) { struct dsa_switch *ds = dp->ds;
There are multi-generational drivers like mv88e6xxx which have code like this: int mv88e6xxx_port_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr) { if (!chip->info->ptp_support) return -EOPNOTSUPP; ... } DSA wants to deny PTP timestamping on the master if the switch supports timestamping too. However it currently relies on the presence of the port_hwtstamp_get() callback to determine PTP capability, and this clearly does not work in that case (method is present but returns -EOPNOTSUPP). We should not deny PTP on the DSA master for those switches which truly do not support hardware timestamping. Create a dsa_port_supports_hwtstamp() method which actually probes for support by calling port_hwtstamp_get() and seeing whether that returned -EOPNOTSUPP or not. Fixes: f685e609a301 ("net: dsa: Deny PTP on master if switch supports it") Link: https://patchwork.kernel.org/project/netdevbpf/patch/20221110124345.3901389-1-festevam@gmail.com/ Reported-by: Fabio Estevam <festevam@gmail.com> Reported-by: Steffen Bätz <steffen@innosonix.de> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- net/dsa/dsa_priv.h | 1 + net/dsa/master.c | 3 +-- net/dsa/port.c | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-)