On Thu, Sep 5, 2024 at 4:34 PM Vadim Fedorenko
[off-list ref] wrote:

On 05/09/2024 09:24, Jason Xing wrote:

quoted

Hello Vadim,

On Wed, Sep 4, 2024 at 7:32 PM Vadim Fedorenko [off-list ref] wrote:
[...]

quoted

diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index a2c66b3d7f0f..1c38536350e7 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h

@@ -38,6 +38,13 @@ enum {
                                  SOF_TIMESTAMPING_LAST
  };

+/*
+ * The highest bit of sk_tsflags is reserved for kernel-internal
+ * SOCKCM_FLAG_TS_OPT_ID. This check is to control that SOF_TIMESTAMPING*
+ * values do not reach this reserved area

I wonder if we can add the above description which is quite useful in
enum{} like this:

diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index a2c66b3d7f0f..2314fccaf51d 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h

@@ -13,7 +13,12 @@
  #include <linux/types.h>
  #include <linux/socket.h>   /* for SO_TIMESTAMPING */

-/* SO_TIMESTAMPING flags */
+/* SO_TIMESTAMPING flags
+ *
+ * The highest bit of sk_tsflags is reserved for kernel-internal
+ * SOCKCM_FLAG_TS_OPT_ID.
+ * SOCKCM_FLAG_TS_OPT_ID = (1 << 31),
+ */
  enum {
         SOF_TIMESTAMPING_TX_HARDWARE = (1<<0),
         SOF_TIMESTAMPING_TX_SOFTWARE = (1<<1),

to explicitly remind the developers not to touch 1<<31 field. Or else,
it can be very hard to trace who occupied the highest field in the
future at the first glance, I think.

[...]

That's a bit contradictory to Willem's comment about not exposing
implementation details to UAPI headers, which I think makes sense.

Oh, well, I missed checking the filename... it's an uapi file, sorry
for the noise :(

I will move the comment to the definition area of SOCKCM_FLAG_TS_OPT_ID
and will try to add meaningful message to BUILD_BUG_ON() to make it
easier for developers to understand the problem.

Great! Thanks!