Re: [PATCHv3 net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAG_BONDED_PHC_INDEX

On Tue, 28 Dec 2021 08:00:50 -0800 Richard Cochran wrote:

On Tue, Dec 28, 2021 at 07:15:28AM -0800, Jakub Kicinski wrote:

quoted

On Tue, 28 Dec 2021 15:01:13 +0800 Hangbin Liu wrote:  

quoted

When implement the user space support for this feature. I realized that
we can't use the new flag directly as the user space tool needs to have
backward compatibility. Because run the new tool with this flag enabled
on old kernel will get -EINVAL error. And we also could not use #ifdef
directly as HWTSTAMP_FLAG_BONDED_PHC_INDEX is a enum.

Do you think if we could add a #define in linux/net_tstamp.h like

#define HWTSTAMP_FLAGS_SUPPORT 1

So that the user space tool could use it like

#ifdef HWTSTAMP_FLAGS_SUPPORT
       cfg->flags = HWTSTAMP_FLAG_BONDED_PHC_INDEX;
#endif  

We could set it on SIOCGHWTSTAMP to let user space know that it's
necessary for a given netdev.  

What about adding matching #defines into the enum declaration?

enum hwtstamp_flags {
	HWTSTAMP_FLAG_BONDED_PHC_INDEX = (1<<0),
#define HWTSTAMP_FLAG_BONDED_PHC_INDEX (1<<0)
};

IIRC I have seen this pattern used in the kernel, but ATM I can't find any example :(

That's still just a compile-time fix, if the user space binary 
is distributed in binary form (distro package) there is no knowing
on which kernel versions it will run. I think runtime probing will
be necessary.

If we want the define it should be to the enum name:

What about adding matching #defines into the enum declaration?

enum hwtstamp_flags {
	HWTSTAMP_FLAG_BONDED_PHC_INDEX = (1<<0),
#define HWTSTAMP_FLAG_BONDED_PHC_INDEX HWTSTAMP_FLAG_BONDED_PHC_INDEX
};

Examples in include/uapi/linux/rtnetlink.h