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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP5jrPE3wpVBHvyS-C4PN71QgKXrA5GVsa+D=RSaBOjEKnD2vw@mail.gmail.com>
Date:   Wed, 26 Apr 2023 22:00:43 -0600
From:   Max Georgiev <glipus@...il.com>
To:     Köry Maincent <kory.maincent@...tlin.com>
Cc:     kuba@...nel.org, netdev@...r.kernel.org,
        maxime.chevallier@...tlin.com, vladimir.oltean@....com,
        vadim.fedorenko@...ux.dev, richardcochran@...il.com,
        gerhard@...leder-embedded.com, thomas.petazzoni@...tlin.com
Subject: Re: [RFC PATCH v4 0/5] New NDO methods ndo_hwtstamp_get/set

On Wed, Apr 26, 2023 at 10:58 AM Köry Maincent
<kory.maincent@...tlin.com> wrote:
>
> From: Kory Maincent <kory.maincent@...tlin.com>
>
> You patch series work on my side with the macb MAC controller and this
> patch.
> I don't know if you are waiting for more reviews but it seems good enough
> to drop the RFC tag.
>
> ---
>
>  drivers/net/ethernet/cadence/macb.h      | 10 ++++++--
>  drivers/net/ethernet/cadence/macb_main.c | 15 ++++--------
>  drivers/net/ethernet/cadence/macb_ptp.c  | 30 ++++++++++++++----------
>  3 files changed, 29 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index cfbdd0022764..bc73b080093e 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1350,8 +1350,14 @@ static inline void gem_ptp_do_rxstamp(struct macb *bp, struct sk_buff *skb, stru
>
>         gem_ptp_rxstamp(bp, skb, desc);
>  }
> -int gem_get_hwtst(struct net_device *dev, struct ifreq *rq);
> -int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd);
> +
> +int gem_get_hwtst(struct net_device *dev,
> +                 struct kernel_hwtstamp_config *kernel_config,
> +                 struct netlink_ext_ack *extack);
> +int gem_set_hwtst(struct net_device *dev,
> +                 struct kernel_hwtstamp_config *kernel_config,
> +                 struct netlink_ext_ack *extack);
> +
>  #else
>  static inline void gem_ptp_init(struct net_device *ndev) { }
>  static inline void gem_ptp_remove(struct net_device *ndev) { }
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 45f63df5bdc4..c1d65be88835 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -3402,8 +3402,6 @@ static struct macb_ptp_info gem_ptp_info = {
>         .get_ptp_max_adj = gem_get_ptp_max_adj,
>         .get_tsu_rate    = gem_get_tsu_rate,
>         .get_ts_info     = gem_get_ts_info,
> -       .get_hwtst       = gem_get_hwtst,
> -       .set_hwtst       = gem_set_hwtst,
>  };
>  #endif
>
> @@ -3764,15 +3762,6 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>         if (!netif_running(dev))
>                 return -EINVAL;
>
> -       if (!phy_has_hwtstamp(dev->phydev) && bp->ptp_info) {
> -               switch (cmd) {
> -               case SIOCSHWTSTAMP:
> -                       return bp->ptp_info->set_hwtst(dev, rq, cmd);
> -               case SIOCGHWTSTAMP:
> -                       return bp->ptp_info->get_hwtst(dev, rq);
> -               }
> -       }
> -
>         return phylink_mii_ioctl(bp->phylink, rq, cmd);
>  }
>
> @@ -3875,6 +3864,10 @@ static const struct net_device_ops macb_netdev_ops = {
>  #endif
>         .ndo_set_features       = macb_set_features,
>         .ndo_features_check     = macb_features_check,
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> +       .ndo_hwtstamp_get       = gem_get_hwtst,
> +       .ndo_hwtstamp_set       = gem_set_hwtst,
> +#endif
>  };
>
>  /* Configure peripheral capabilities according to device tree
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
> index 51d26fa190d7..eddacc5df435 100644
> --- a/drivers/net/ethernet/cadence/macb_ptp.c
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c
> @@ -374,19 +374,22 @@ static int gem_ptp_set_ts_mode(struct macb *bp,
>         return 0;
>  }
>
> -int gem_get_hwtst(struct net_device *dev, struct ifreq *rq)
> +int gem_get_hwtst(struct net_device *dev,
> +                 struct kernel_hwtstamp_config *kernel_config,
> +                 struct netlink_ext_ack *extack)
>  {
>         struct hwtstamp_config *tstamp_config;
>         struct macb *bp = netdev_priv(dev);
>
> +       if (phy_has_hwtstamp(dev->phydev))
> +               return phylink_mii_ioctl(bp->phylink, kernel_config->ifr, SIOCGHWTSTAMP);
> +
>         tstamp_config = &bp->tstamp_config;
>         if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
>                 return -EOPNOTSUPP;
>
> -       if (copy_to_user(rq->ifr_data, tstamp_config, sizeof(*tstamp_config)))
> -               return -EFAULT;
> -       else
> -               return 0;
> +       hwtstamp_config_to_kernel(kernel_config, tstamp_config);
> +       return 0;
>  }
>
>  static void gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
> @@ -401,7 +404,9 @@ static void gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
>                 macb_writel(bp, NCR, reg_val & ~MACB_BIT(OSSMODE));
>  }
>
> -int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
> +int gem_set_hwtst(struct net_device *dev,
> +                 struct kernel_hwtstamp_config *kernel_config,
> +                 struct netlink_ext_ack *extack)
>  {
>         enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
>         enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
> @@ -409,13 +414,14 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
>         struct macb *bp = netdev_priv(dev);
>         u32 regval;
>
> +       if (phy_has_hwtstamp(dev->phydev))
> +               return phylink_mii_ioctl(bp->phylink, kernel_config->ifr, SIOCSHWTSTAMP);
> +
>         tstamp_config = &bp->tstamp_config;
>         if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
>                 return -EOPNOTSUPP;
>
> -       if (copy_from_user(tstamp_config, ifr->ifr_data,
> -                          sizeof(*tstamp_config)))
> -               return -EFAULT;
> +       hwtstamp_config_from_kernel(tstamp_config, kernel_config);
>
>         switch (tstamp_config->tx_type) {
>         case HWTSTAMP_TX_OFF:
> @@ -466,9 +472,7 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
>         if (gem_ptp_set_ts_mode(bp, tx_bd_control, rx_bd_control) != 0)
>                 return -ERANGE;
>
> -       if (copy_to_user(ifr->ifr_data, tstamp_config, sizeof(*tstamp_config)))
> -               return -EFAULT;
> -       else
> -               return 0;
> +       hwtstamp_config_to_kernel(kernel_config, tstamp_config);
> +       return 0;
>  }
>
> --
> 2.25.1
>

Thank you for giving it a try!
I'll drop the RFC tag starting from the next iteration.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ