netdev - Re: [net-next 14/16] i40e: enable PTP

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]

Date:	Fri, 10 Jan 2014 15:55:05 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
CC:	<davem@...emloft.net>, Jacob Keller <jacob.e.keller@...el.com>,
	<netdev@...r.kernel.org>, <gospo@...hat.com>,
	<sassmann@...hat.com>, Richard Cochran <richardcochran@...il.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>
Subject: Re: [net-next 14/16] i40e: enable PTP

On Fri, 2014-01-10 at 00:58 -0800, Jeff Kirsher wrote:
> From: Jacob Keller <jacob.e.keller@...el.com>
> 
> New feature: Enable PTP support in the i40e driver.
> 
> Change-ID: I6a8e799f582705191f9583afb1b9231a8db96cc8
> Cc: Richard Cochran <richardcochran@...il.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
> Tested-by: Kavindya Deegala <kavindya.s.deegala@...el.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> ---
>  drivers/net/ethernet/intel/i40e/Makefile       |   2 +
>  drivers/net/ethernet/intel/i40e/i40e.h         |  24 +
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  33 +-
>  drivers/net/ethernet/intel/i40e/i40e_main.c    |  45 +-
>  drivers/net/ethernet/intel/i40e/i40e_ptp.c     | 640 +++++++++++++++++++++++++
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c    |  53 ++
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h    |   3 +
>  7 files changed, 798 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/net/ethernet/intel/i40e/i40e_ptp.c

This is missing a Kconfig update to select PTP_1588_CLOCK.

[...]
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -1698,6 +1698,25 @@ static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
>  }
>  
>  /**
> + * i40e_ioctl - Access the hwtstamp interface
> + * @netdev: network interface device structure
> + * @ifr: interface request data
> + * @cmd: ioctl command
> + **/
> +int i40e_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
> +{
> +	struct i40e_netdev_priv *np = netdev_priv(netdev);
> +	struct i40e_pf *pf = np->vsi->back;
> +
> +	switch (cmd) {
> +	case SIOCSHWTSTAMP:
> +		return i40e_ptp_hwtstamp_ioctl(pf, ifr, cmd);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}

Please implement SIOCGHWTSTAMP as well.

[...]
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
[...]
> +void i40e_ptp_init(struct i40e_pf *pf)
> +{
> +	struct i40e_hw *hw = &pf->hw;
> +	struct net_device *netdev = pf->vsi[pf->lan_vsi]->netdev;
> +
> +	snprintf(pf->ptp_caps.name, 16, "%pm", netdev->dev_addr);
[...]

Use sizeof(), not a magic number.  Also the clock name should perhaps be
the driver name, not the MAC address.

Is the clock shared between multiple PFs or are there independent clocks
for each PF?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html