| Message ID | 20230423032817.285371-1-glipus@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | New NDO methods ndo_hwtstamp_get/set | expand |
On Sat, Apr 22, 2023 at 09:28:17PM -0600, Maxim Georgiev wrote: > Considering the stackable nature of drivers there will be situations > where a driver implementing ndo_hwtstamp_get/set functions will have > to translate requests back to SIOCGHWTSTAMP/SIOCSHWTSTAMP IOCTLs > to pass them to lower level drivers that do not provide > ndo_hwtstamp_get/set callbacks. To simplify request translation in > such scenarios let's include a pointer to the original struct ifreq > to kernel_hwtstamp_config structure. > > Suggested-by: Jakub Kicinski <kuba@kernel.org> > Signed-off-by: Maxim Georgiev <glipus@gmail.com> > > Notes: > > Changes in V4: > - Introducing KERNEL_HWTSTAMP_FLAG_IFR_RESULT flag indicating that > the operation results are returned in the ifr referred by > struct kernel_hwtstamp_config instead of kernel_hwtstamp_config > glags/tx_type/rx_filter fields. > - Implementing generic_hwtstamp_set/set_lower() functions > which will be used by vlan, maxvlan, bond and potentially > other drivers translating ndo_hwtstamp_set/set calls to > lower level drivers. > --- > include/linux/net_tstamp.h | 7 ++++ > include/linux/netdevice.h | 6 +++ > net/core/dev_ioctl.c | 80 +++++++++++++++++++++++++++++++++++--- > 3 files changed, 87 insertions(+), 6 deletions(-) > > diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h > index 7c59824f43f5..5164dce3f9a0 100644 > --- a/include/linux/net_tstamp.h > +++ b/include/linux/net_tstamp.h > @@ -20,6 +20,13 @@ struct kernel_hwtstamp_config { > int flags; > int tx_type; > int rx_filter; > + struct ifreq *ifr; > + int kernel_flags; nit: ifr and kernel_flags should be added to the kdoc for this struct that appears immediately above it. > +}; > + > +/* possible values for kernel_hwtstamp_config->kernel_flags */ > +enum kernel_hwtstamp_flags { > + KERNEL_HWTSTAMP_FLAG_IFR_RESULT = (1 << 0), nit: maybe BIT(0) > }; > > static inline void hwtstamp_config_to_kernel(struct kernel_hwtstamp_config *kernel_cfg, ...
On Wed, Apr 26, 2023 at 1:59 PM Simon Horman <simon.horman@corigine.com> wrote: > > On Sat, Apr 22, 2023 at 09:28:17PM -0600, Maxim Georgiev wrote: > > Considering the stackable nature of drivers there will be situations > > where a driver implementing ndo_hwtstamp_get/set functions will have > > to translate requests back to SIOCGHWTSTAMP/SIOCSHWTSTAMP IOCTLs > > to pass them to lower level drivers that do not provide > > ndo_hwtstamp_get/set callbacks. To simplify request translation in > > such scenarios let's include a pointer to the original struct ifreq > > to kernel_hwtstamp_config structure. > > > > Suggested-by: Jakub Kicinski <kuba@kernel.org> > > Signed-off-by: Maxim Georgiev <glipus@gmail.com> > > > > Notes: > > > > Changes in V4: > > - Introducing KERNEL_HWTSTAMP_FLAG_IFR_RESULT flag indicating that > > the operation results are returned in the ifr referred by > > struct kernel_hwtstamp_config instead of kernel_hwtstamp_config > > glags/tx_type/rx_filter fields. > > - Implementing generic_hwtstamp_set/set_lower() functions > > which will be used by vlan, maxvlan, bond and potentially > > other drivers translating ndo_hwtstamp_set/set calls to > > lower level drivers. > > --- > > include/linux/net_tstamp.h | 7 ++++ > > include/linux/netdevice.h | 6 +++ > > net/core/dev_ioctl.c | 80 +++++++++++++++++++++++++++++++++++--- > > 3 files changed, 87 insertions(+), 6 deletions(-) > > > > diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h > > index 7c59824f43f5..5164dce3f9a0 100644 > > --- a/include/linux/net_tstamp.h > > +++ b/include/linux/net_tstamp.h > > @@ -20,6 +20,13 @@ struct kernel_hwtstamp_config { > > int flags; > > int tx_type; > > int rx_filter; > > + struct ifreq *ifr; > > + int kernel_flags; > > nit: ifr and kernel_flags should be added to the kdoc for this struct > that appears immediately above it. > > > +}; > > + > > +/* possible values for kernel_hwtstamp_config->kernel_flags */ > > +enum kernel_hwtstamp_flags { > > + KERNEL_HWTSTAMP_FLAG_IFR_RESULT = (1 << 0), > > nit: maybe BIT(0) > > > }; > > > > static inline void hwtstamp_config_to_kernel(struct kernel_hwtstamp_config *kernel_cfg, > > ... Thank you for the feedback! I'll update and resend the patch
diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h index 7c59824f43f5..5164dce3f9a0 100644 --- a/include/linux/net_tstamp.h +++ b/include/linux/net_tstamp.h @@ -20,6 +20,13 @@ struct kernel_hwtstamp_config { int flags; int tx_type; int rx_filter; + struct ifreq *ifr; + int kernel_flags; +}; + +/* possible values for kernel_hwtstamp_config->kernel_flags */ +enum kernel_hwtstamp_flags { + KERNEL_HWTSTAMP_FLAG_IFR_RESULT = (1 << 0), }; static inline void hwtstamp_config_to_kernel(struct kernel_hwtstamp_config *kernel_cfg, diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index ea10fb1dd6fc..40f4018b13f2 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3939,6 +3939,12 @@ int put_user_ifreq(struct ifreq *ifr, void __user *arg); int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, void __user *data, bool *need_copyout); int dev_ifconf(struct net *net, struct ifconf __user *ifc); +int generic_hwtstamp_set_lower(struct net_device *dev, + struct kernel_hwtstamp_config *kernel_cfg, + struct netlink_ext_ack *extack); +int generic_hwtstamp_get_lower(struct net_device *dev, + struct kernel_hwtstamp_config *kernel_cfg, + struct netlink_ext_ack *extack); int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *userdata); unsigned int dev_get_flags(const struct net_device *); int __dev_change_flags(struct net_device *dev, unsigned int flags, diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c index a157b9ab5237..da1d2391822f 100644 --- a/net/core/dev_ioctl.c +++ b/net/core/dev_ioctl.c @@ -265,14 +265,17 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr) if (!netif_device_present(dev)) return -ENODEV; + kernel_cfg.ifr = ifr; err = ops->ndo_hwtstamp_get(dev, &kernel_cfg, NULL); if (err) return err; - hwtstamp_config_from_kernel(&config, &kernel_cfg); + if (!(kernel_cfg.kernel_flags & KERNEL_HWTSTAMP_FLAG_IFR_RESULT)) { + hwtstamp_config_from_kernel(&config, &kernel_cfg); - if (copy_to_user(ifr->ifr_data, &config, sizeof(config))) - return -EFAULT; + if (copy_to_user(ifr->ifr_data, &config, sizeof(config))) + return -EFAULT; + } return 0; } @@ -289,6 +292,7 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr) return -EFAULT; hwtstamp_config_to_kernel(&kernel_cfg, &cfg); + kernel_cfg.ifr = ifr; err = net_hwtstamp_validate(&kernel_cfg); if (err) @@ -311,14 +315,78 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr) if (err) return err; - hwtstamp_config_from_kernel(&cfg, &kernel_cfg); + if (!(kernel_cfg.kernel_flags & KERNEL_HWTSTAMP_FLAG_IFR_RESULT)) { + hwtstamp_config_from_kernel(&cfg, &kernel_cfg); - if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg))) - return -EFAULT; + if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg))) + return -EFAULT; + } return 0; } +int generic_hwtstamp_set_lower(struct net_device *dev, + struct kernel_hwtstamp_config *kernel_cfg, + struct netlink_ext_ack *extack) +{ + const struct net_device_ops *ops = dev->netdev_ops; + struct ifreq ifrr; + int err; + + if (!netif_device_present(dev)) + return -ENODEV; + + if (ops->ndo_hwtstamp_set) { + kernel_cfg->kernel_flags &= ~KERNEL_HWTSTAMP_FLAG_IFR_RESULT; + err = ops->ndo_hwtstamp_set(dev, kernel_cfg, extack); + return err; + } + + if (!kernel_cfg->ifr) + return -EOPNOTSUPP; + + strscpy_pad(ifrr.ifr_name, dev->name, IFNAMSIZ); + ifrr.ifr_ifru = kernel_cfg->ifr->ifr_ifru; + err = dev_eth_ioctl(dev, &ifrr, SIOCSHWTSTAMP); + if (!err) { + kernel_cfg->ifr->ifr_ifru = ifrr.ifr_ifru; + kernel_cfg->kernel_flags |= KERNEL_HWTSTAMP_FLAG_IFR_RESULT; + } + return err; +} +EXPORT_SYMBOL(generic_hwtstamp_set_lower); + +int generic_hwtstamp_get_lower(struct net_device *dev, + struct kernel_hwtstamp_config *kernel_cfg, + struct netlink_ext_ack *extack) +{ + const struct net_device_ops *ops = dev->netdev_ops; + struct ifreq ifrr; + int err; + + if (!netif_device_present(dev)) + return -ENODEV; + + if (ops->ndo_hwtstamp_get) { + kernel_cfg->kernel_flags &= ~KERNEL_HWTSTAMP_FLAG_IFR_RESULT; + err = ops->ndo_hwtstamp_get(dev, kernel_cfg, extack); + return err; + } + + if (!kernel_cfg->ifr) + return -EOPNOTSUPP; + + strscpy_pad(ifrr.ifr_name, dev->name, IFNAMSIZ); + ifrr.ifr_ifru = kernel_cfg->ifr->ifr_ifru; + err = dev_eth_ioctl(dev, &ifrr, SIOCGHWTSTAMP); + if (!err) { + kernel_cfg->ifr->ifr_ifru = ifrr.ifr_ifru; + kernel_cfg->kernel_flags |= KERNEL_HWTSTAMP_FLAG_IFR_RESULT; + } + return err; +} +EXPORT_SYMBOL(generic_hwtstamp_get_lower); + static int dev_siocbond(struct net_device *dev, struct ifreq *ifr, unsigned int cmd) {
Considering the stackable nature of drivers there will be situations where a driver implementing ndo_hwtstamp_get/set functions will have to translate requests back to SIOCGHWTSTAMP/SIOCSHWTSTAMP IOCTLs to pass them to lower level drivers that do not provide ndo_hwtstamp_get/set callbacks. To simplify request translation in such scenarios let's include a pointer to the original struct ifreq to kernel_hwtstamp_config structure. Suggested-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Maxim Georgiev <glipus@gmail.com> Notes: Changes in V4: - Introducing KERNEL_HWTSTAMP_FLAG_IFR_RESULT flag indicating that the operation results are returned in the ifr referred by struct kernel_hwtstamp_config instead of kernel_hwtstamp_config glags/tx_type/rx_filter fields. - Implementing generic_hwtstamp_set/set_lower() functions which will be used by vlan, maxvlan, bond and potentially other drivers translating ndo_hwtstamp_set/set calls to lower level drivers. --- include/linux/net_tstamp.h | 7 ++++ include/linux/netdevice.h | 6 +++ net/core/dev_ioctl.c | 80 +++++++++++++++++++++++++++++++++++--- 3 files changed, 87 insertions(+), 6 deletions(-)