Hi Jakub,

-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Wednesday, 28 September, 2022 8:12 AM
To: Zulkifli, Muhammad Husaini <redacted>
Cc: intel-wired-lan@osuosl.org; netdev@vger.kernel.org;
davem@davemloft.net; edumazet@google.com; Gomes, Vinicius
[off-list ref]; Gunasekaran, Aravindhan
[off-list ref]; Ahmad Tarmizi, Noor Azura
[off-list ref]
Subject: Re: [PATCH v1 1/4] ethtool: Add new hwtstamp flag

On Tue, 27 Sep 2022 21:06:53 +0800 Muhammad Husaini Zulkifli wrote:

quoted

--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h

@@ -675,6 +675,7 @@ enum ethtool_link_ext_substate_module {
  * @ETH_SS_MSG_CLASSES: debug message class names
  * @ETH_SS_WOL_MODES: wake-on-lan modes
  * @ETH_SS_SOF_TIMESTAMPING: SOF_TIMESTAMPING_* flags
+ * @ETH_SS_HWTSTAMP_FLAG: timestamping flags
  * @ETH_SS_TS_TX_TYPES: timestamping Tx types
  * @ETH_SS_TS_RX_FILTERS: timestamping Rx filters
  * @ETH_SS_UDP_TUNNEL_TYPES: UDP tunnel types @@ -700,6 +701,7

@@

quoted

enum ethtool_stringset {
 	ETH_SS_MSG_CLASSES,
 	ETH_SS_WOL_MODES,
 	ETH_SS_SOF_TIMESTAMPING,
+	ETH_SS_HWTSTAMP_FLAG,
 	ETH_SS_TS_TX_TYPES,
 	ETH_SS_TS_RX_FILTERS,
 	ETH_SS_UDP_TUNNEL_TYPES,

@@ -1367,6 +1369,7 @@ struct ethtool_ts_info {
 	__u32	cmd;
 	__u32	so_timestamping;
 	__s32	phc_index;
+	__u32	flag;
 	__u32	tx_types;
 	__u32	tx_reserved[3];
 	__u32	rx_filters;

diff --git a/include/uapi/linux/ethtool_netlink.h

b/include/uapi/linux/ethtool_netlink.h
index 408a664fad59..58d073b5a6d2 100644

--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h

@@ -452,6 +452,7 @@ enum {
 	ETHTOOL_A_TSINFO_UNSPEC,
 	ETHTOOL_A_TSINFO_HEADER,			/* nest -

_A_HEADER_* */

quoted

 	ETHTOOL_A_TSINFO_TIMESTAMPING,			/* bitset */
+	ETHTOOL_A_TSINFO_FLAG,				/* bitset */
 	ETHTOOL_A_TSINFO_TX_TYPES,			/* bitset */
 	ETHTOOL_A_TSINFO_RX_FILTERS,			/* bitset */
 	ETHTOOL_A_TSINFO_PHC_INDEX,			/* u32 */

You can't add stuff into the middle of an enum or a struct in uAPI.
What's worse for the struct ethtool_ts_info you can't actually add anything
in, period. You can reuse reserved fields but even that requires extra
legwork. If the fields were not previously validated on input to the kernel (ie.
kernel didn't check they are zero) the ioctl path can't use them, because
some application may had been passing in garbage.

Noted. Will make the changes in V2 to move to the end.

Thanks!