Thu, Aug 10, 2023 at 06:13:36PM CEST, kuba@kernel.org wrote:

On Thu, 10 Aug 2023 11:07:41 +0200 Jiri Pirko wrote:

quoted

quoted

+#ifdef __LITTLE_ENDIAN
+#define __GENL_PTR_LOW(byte)	((void *)(unsigned long)(byte))
+#else
+#define __GENL_PTR_LOW(byte)	\
+	((void *)((unsigned long)(byte) << (BITS_PER_LONG - 8)))
+#endif
+
+/**
+ * GENL_INFO_NTF() - define genl_info for notifications
+ * @__name: name of declared variable
+ * @__family: pointer to the genetlink family
+ * @__cmd: command to be used in the notification
+ */
+#define GENL_INFO_NTF(__name, __family, __cmd)			\
+	struct genl_info __name = {				\
+		.family = (__family),				\
+		.genlhdr = (void *)&(__name.user_ptr[0]),	\
+		.user_ptr[0] = __GENL_PTR_LOW(__cmd),		\  

Ugh. Took me some time to decypher what you do here. Having endian
specific code here seems quite odd to me. Why don't you have this as
static inline initializer function instead and use struct genlmsghdr
pointer to store cmd where it belong?

static inline void genl_info_ntf(struct genl_info *info,
				 const struct genl_family *family, u8 cmd)
}
	struct genlmsghdr *hdr = (void *) &info->user_ptr[0];

	info->family = family;
	info->genlhdr = hdr;
	hdr->cmd = cmd;
}

Nice! The endian magic is easily the nastiest part of this series.
I considered making genlhdr a struct (rather than a pointer) because
it's actually smaller than a pointer on 64b. But dunno, feels kinda
weird to have a copy of the struct and a pointer to nlh. Hence the
magic.

And I was trying to save the 2 LoC and provide the DEFINE_ style macro
but on second thought your init helper is cleaner. I don't like the
DEFINE_ shit myself. I'll just throw in a memset(0) in there, and maybe
add a verb to the name - genl_info_init_nft()?

Yep, looks fine to me. Thanks!