All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Y.b. Lu" <yangbo.lu@nxp.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"mptcp@lists.linux.dev" <mptcp@lists.linux.dev>,
	Richard Cochran <richardcochran@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Mat Martineau <mathew.j.martineau@linux.intel.com>,
	Matthieu Baerts <matthieu.baerts@tessares.net>,
	Shuah Khan <shuah@kernel.org>, Michal Kubecek <mkubecek@suse.cz>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>, Rui Sousa <rui.sousa@nxp.com>,
	Sebastien Laveze <sebastien.laveze@nxp.com>
Subject: RE: [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks
Date: Tue, 22 Jun 2021 10:10:05 +0000	[thread overview]
Message-ID: <DB7PR04MB5017C6E0D6223B4D9B80EA1DF8099@DB7PR04MB5017.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20210615124911.15e64ce9@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: 2021年6月16日 3:49
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; Richard Cochran
> <richardcochran@gmail.com>; David S . Miller <davem@davemloft.net>; Mat
> Martineau <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>
> Subject: Re: [net-next, v3, 05/10] ethtool: add a new command for getting PHC
> virtual clocks
> 
> On Tue, 15 Jun 2021 17:45:12 +0800 Yangbo Lu wrote:
> > Add an interface for getting PHC (PTP Hardware Clock) virtual clocks,
> > which are based on PHC physical clock providing hardware timestamp to
> > network packets.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> 
> > diff --git a/include/uapi/linux/ethtool.h
> > b/include/uapi/linux/ethtool.h index cfef6b08169a..0fb04f945767 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -17,6 +17,7 @@
> >  #include <linux/const.h>
> >  #include <linux/types.h>
> >  #include <linux/if_ether.h>
> > +#include <linux/ptp_clock.h>
> >
> >  #ifndef __KERNEL__
> >  #include <limits.h> /* for INT_MAX */ @@ -1341,6 +1342,18 @@ struct
> > ethtool_ts_info {
> >  	__u32	rx_reserved[3];
> >  };
> >
> > +/**
> > + * struct ethtool_phc_vclocks - holds a device's PTP virtual clocks
> > + * @cmd: command number = %ETHTOOL_GET_PHC_VCLOCKS
> > + * @num: number of PTP vclocks
> > + * @index: all index values of PTP vclocks  */ struct
> > +ethtool_phc_vclocks {
> > +	__u32	cmd;
> > +	__u8	num;
> > +	__s32	index[PTP_MAX_VCLOCKS];
> > +};
> > +
> >  /*
> >   * %ETHTOOL_SFEATURES changes features present in features[].valid to
> the
> >   * values of corresponding bits in features[].requested. Bits in
> > .requested @@ -1552,6 +1565,7 @@ enum ethtool_fec_config_bits {
> >  #define ETHTOOL_PHY_STUNABLE	0x0000004f /* Set PHY tunable
> configuration */
> >  #define ETHTOOL_GFECPARAM	0x00000050 /* Get FEC settings */
> >  #define ETHTOOL_SFECPARAM	0x00000051 /* Set FEC settings */
> > +#define ETHTOOL_GET_PHC_VCLOCKS	0x00000052 /* Get PHC virtual
> clocks info */
> 
> We don't add new IOCTL commands, only netlink API is going to be extended.
> Please remove the IOCTL interface & uAPI.

Will remove. Thanks.

> 
> >  /* compatibility with older code */
> >  #define SPARC_ETH_GSET		ETHTOOL_GSET
> 
> > +/* PHC VCLOCKS */
> > +
> > +enum {
> > +	ETHTOOL_A_PHC_VCLOCKS_UNSPEC,
> > +	ETHTOOL_A_PHC_VCLOCKS_HEADER,			/* nest - _A_HEADER_*
> */
> > +	ETHTOOL_A_PHC_VCLOCKS_NUM,			/* u8 */
> 
> u32, no need to limit yourself, the netlink attribute is rounded up to 4B
> anyway.

Get it. Will use u32.

> 
> > +	ETHTOOL_A_PHC_VCLOCKS_INDEX,			/* s32 */
> 
> This is an array, AFAICT, not a single s32.

Will fix. Thanks.

> 
> > +
> > +	/* add new constants above here */
> > +	__ETHTOOL_A_PHC_VCLOCKS_CNT,
> > +	ETHTOOL_A_PHC_VCLOCKS_MAX = (__ETHTOOL_A_PHC_VCLOCKS_CNT -
> 1) };
> > +
> >  /* CABLE TEST */
> >
> >  enum {
> 
> > +static int phc_vclocks_fill_reply(struct sk_buff *skb,
> > +				  const struct ethnl_req_info *req_base,
> > +				  const struct ethnl_reply_data *reply_base) {
> > +	const struct phc_vclocks_reply_data *data =
> > +		PHC_VCLOCKS_REPDATA(reply_base);
> > +	const struct ethtool_phc_vclocks *phc_vclocks = &data->phc_vclocks;
> > +
> > +	if (phc_vclocks->num <= 0)
> > +		return 0;
> > +
> > +	if (nla_put_u32(skb, ETHTOOL_A_PHC_VCLOCKS_NUM, phc_vclocks->num)
> ||
> > +	    nla_put(skb, ETHTOOL_A_PHC_VCLOCKS_INDEX,
> > +		    sizeof(phc_vclocks->index), phc_vclocks->index))
> 
> Looks like you'll report the whole array, why not just num?

Will report just num. Thanks.

> 
> > +		return -EMSGSIZE;
> > +
> > +	return 0;
> > +}
> > +
> > +const struct ethnl_request_ops ethnl_phc_vclocks_request_ops = {
> > +	.request_cmd		= ETHTOOL_MSG_PHC_VCLOCKS_GET,
> > +	.reply_cmd		= ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY,
> > +	.hdr_attr		= ETHTOOL_A_PHC_VCLOCKS_HEADER,
> > +	.req_info_size		= sizeof(struct phc_vclocks_req_info),
> > +	.reply_data_size	= sizeof(struct phc_vclocks_reply_data),
> > +
> > +	.prepare_data		= phc_vclocks_prepare_data,
> > +	.reply_size		= phc_vclocks_reply_size,
> > +	.fill_reply		= phc_vclocks_fill_reply,
> > +};


  reply	other threads:[~2021-06-22 10:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15  9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
2021-06-15  9:45 ` [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework Yangbo Lu
2021-06-17 17:32   ` Richard Cochran
2021-06-22 10:17     ` Y.b. Lu
2021-06-15  9:45 ` [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion Yangbo Lu
2021-06-17 17:42   ` Richard Cochran
2021-06-22 10:18     ` Y.b. Lu
2021-06-17 18:27   ` Richard Cochran
2021-06-22 10:35     ` Y.b. Lu
2021-06-25 11:09       ` Y.b. Lu
2021-06-19  4:06   ` Richard Cochran
2021-06-22 10:39     ` Y.b. Lu
2021-06-15  9:45 ` [net-next, v3, 03/10] ptp: track available ptp vclocks information Yangbo Lu
2021-06-15  9:45 ` [net-next, v3, 04/10] ptp: add kernel API ptp_get_vclocks_index() Yangbo Lu
2021-06-15  9:45 ` [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks Yangbo Lu
2021-06-15 19:49   ` Jakub Kicinski
2021-06-22 10:10     ` Y.b. Lu [this message]
2021-06-15 23:24   ` Michal Kubecek
2021-06-22 10:10     ` Y.b. Lu
2021-06-15  9:45 ` [net-next, v3, 06/10] ptp: add kernel API ptp_convert_timestamp() Yangbo Lu
2021-06-15  9:45 ` [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding Yangbo Lu
2021-06-16  1:00   ` Mat Martineau
2021-06-22 10:14     ` Y.b. Lu
2021-06-16 10:27   ` kernel test robot
2021-06-16 10:27     ` kernel test robot
2021-06-16 10:39   ` kernel test robot
2021-06-16 10:39     ` kernel test robot
2021-06-15  9:45 ` [net-next, v3, 08/10] net: socket: support hardware timestamp conversion to PHC bound Yangbo Lu
2021-06-15  9:45 ` [net-next, v3, 09/10] selftests/net: timestamping: support binding PHC Yangbo Lu
2021-06-15  9:45 ` [net-next, v3, 10/10] MAINTAINERS: add entry for PTP virtual clock driver Yangbo Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB7PR04MB5017C6E0D6223B4D9B80EA1DF8099@DB7PR04MB5017.eurprd04.prod.outlook.com \
    --to=yangbo.lu@nxp.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mathew.j.martineau@linux.intel.com \
    --cc=matthieu.baerts@tessares.net \
    --cc=mkubecek@suse.cz \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=rui.sousa@nxp.com \
    --cc=sebastien.laveze@nxp.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.