diff mbox series

[net,v1] netlink: optimize the NMLSG_OK macro

Message ID 20240912024018.8117-1-atlas.yu@canonical.com (mailing list archive)
State Rejected
Delegated to: Netdev Maintainers
Headers show
Series [net,v1] netlink: optimize the NMLSG_OK macro | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 57 this patch: 57
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/build_clang success Errors and warnings before: 109 this patch: 109
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4778 this patch: 4778
netdev/checkpatch fail ERROR: space required after that ',' (ctx:VxV)
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-09-12--15-00 (tests: 764)

Commit Message

Atlas Yu Sept. 12, 2024, 2:40 a.m. UTC
When nlmsg_len >= sizeof(hdr) and nlmsg_len <= len are true, we can set
up an inequation: len >= nlmsg_len >= sizeof(hdr), which makes checking
len >= sizeof(hdr) useless.

gcc -O3 cannot optimize ok1 to generate the same instructions as ok2 on
x86_64 (not investigated on other architectures).
  int ok1(int a, int b, int c) { return a >= b && c >= b && c <= a; }
  int ok2(int a, int b, int c) { return c >= b && c <= a; }

Signed-off-by: Atlas Yu <atlas.yu@canonical.com>
---
 include/uapi/linux/netlink.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Kuniyuki Iwashima Sept. 12, 2024, 5:58 a.m. UTC | #1
From: Atlas Yu <atlas.yu@canonical.com>
Date: Thu, 12 Sep 2024 10:40:18 +0800
> When nlmsg_len >= sizeof(hdr) and nlmsg_len <= len are true, we can set
> up an inequation: len >= nlmsg_len >= sizeof(hdr), which makes checking
> len >= sizeof(hdr) useless.
> 
> gcc -O3 cannot optimize ok1 to generate the same instructions as ok2 on
> x86_64 (not investigated on other architectures).
>   int ok1(int a, int b, int c) { return a >= b && c >= b && c <= a; }
>   int ok2(int a, int b, int c) { return c >= b && c <= a; }
> 
> Signed-off-by: Atlas Yu <atlas.yu@canonical.com>
> ---
>  include/uapi/linux/netlink.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
> index f87aaf28a649..85dcfa6b33af 100644
> --- a/include/uapi/linux/netlink.h
> +++ b/include/uapi/linux/netlink.h
> @@ -104,8 +104,7 @@ struct nlmsghdr {
>  #define NLMSG_NEXT(nlh,len)	 ((len) -= NLMSG_ALIGN((nlh)->nlmsg_len), \
>  				  (struct nlmsghdr *)(((char *)(nlh)) + \
>  				  NLMSG_ALIGN((nlh)->nlmsg_len)))
> -#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
> -			   (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
> +#define NLMSG_OK(nlh,len) ((nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \

If len < sizeof(struct nlmsghdr)
(more specifically, offsetof(struct nlmsghdr, nlmsg_len)),
you can't access nlmsg_len; it returns a garbage.


>  			   (nlh)->nlmsg_len <= (len))
>  #define NLMSG_PAYLOAD(nlh,len) ((nlh)->nlmsg_len - NLMSG_SPACE((len)))
>  
> -- 
> 2.43.0
>
diff mbox series

Patch

diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index f87aaf28a649..85dcfa6b33af 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -104,8 +104,7 @@  struct nlmsghdr {
 #define NLMSG_NEXT(nlh,len)	 ((len) -= NLMSG_ALIGN((nlh)->nlmsg_len), \
 				  (struct nlmsghdr *)(((char *)(nlh)) + \
 				  NLMSG_ALIGN((nlh)->nlmsg_len)))
-#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
-			   (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
+#define NLMSG_OK(nlh,len) ((nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
 			   (nlh)->nlmsg_len <= (len))
 #define NLMSG_PAYLOAD(nlh,len) ((nlh)->nlmsg_len - NLMSG_SPACE((len)))