Update code generation for bpf_helper_defs.h by adding
__attribute__((nomerge)) for a set of helper functions to prevent some
verifier unfriendly compiler optimizations.
This addresses a recent mailing list thread [1].
There Zhongqiu Duan and Yonghong Song discussed a C program as below:
if (data_end - data > 1024) {
bpf_for_each_map_elem(&map1, cb, &cb_data, 0);
} else {
bpf_for_each_map_elem(&map2, cb, &cb_data, 0);
}
Which was converted by clang to something like this:
if (data_end - data > 1024)
tmp = &map1;
else
tmp = &map2;
bpf_for_each_map_elem(tmp, cb, &cb_data, 0);
Which in turn triggered verification error, because
verifier.c:record_func_map() requires a single map address for each
bpf_for_each_map_elem() call.
In fact, this is a requirement for the following helpers:
- bpf_tail_call
- bpf_map_lookup_elem
- bpf_map_update_elem
- bpf_map_delete_elem
- bpf_map_push_elem
- bpf_map_pop_elem
- bpf_map_peek_elem
- bpf_for_each_map_elem
- bpf_redirect_map
- bpf_map_lookup_percpu_elem
I had an off-list discussion with Yonghong where we agreed that clang
attribute 'nomerge' (see [2]) could be used to prevent the
optimization hitting in [1]. However, currently 'nomerge' applies only
to functions and statements, hence I submitted change requests [3],
[4] to allow specifying 'nomerge' for function pointers as well.
The patch below updates bpf_helper_defs.h generation by adding a
definition of __nomerge macro, and using this macro in definitions of
relevant helpers.
The generated code looks as follows:
/* This is auto-generated file. See bpf_doc.py for details. */
#if __has_attribute(nomerge)
#define __nomerge __attribute__((nomerge))
#else
#define __nomerge
#endif
/* Forward declarations of BPF structs */
...
static long (*bpf_for_each_map_elem)(void *map, ...) __nomerge = (void *) 164;
...
(In non-RFC version the macro definition would have to be updated to
check for supported clang version).
Does community agree with such approach?
[1] https://lore.kernel.org/bpf/03bdf90f-f374-1e67-69d6-76dd9c8318a4@xxxxxxxx/
[2] https://clang.llvm.org/docs/AttributeReference.html#nomerge
[3] https://reviews.llvm.org/D152986
[4] https://reviews.llvm.org/D152987
---
scripts/bpf_doc.py | 37 ++++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
index eaae2ce78381..dbd4893c793e 100755
--- a/scripts/bpf_doc.py
+++ b/scripts/bpf_doc.py
@@ -777,14 +777,33 @@ class PrinterHelpers(Printer):
'bpf_get_socket_cookie',
'bpf_sk_assign',
]
+ # Helpers that need __nomerge attribute
+ nomerge_helpers = set([
+ "bpf_tail_call",
+ "bpf_map_lookup_elem",
+ "bpf_map_update_elem",
+ "bpf_map_delete_elem",
+ "bpf_map_push_elem",
+ "bpf_map_pop_elem",
+ "bpf_map_peek_elem",
+ "bpf_for_each_map_elem",
+ "bpf_redirect_map",
+ "bpf_map_lookup_percpu_elem"
+ ])
+
+ macros = '''\
+#if __has_attribute(nomerge)
+#define __nomerge __attribute__((nomerge))
+#else
+#define __nomerge
+#endif'''
def print_header(self):
- header = '''\
-/* This is auto-generated file. See bpf_doc.py for details. */
-
-/* Forward declarations of BPF structs */'''
-
- print(header)
+ print('/* This is auto-generated file. See bpf_doc.py for details. */')
+ print()
+ print(self.macros)
+ print()
+ print('/* Forward declarations of BPF structs */')
for fwd in self.type_fwds:
print('%s;' % fwd)
print('')
@@ -846,7 +865,11 @@ class PrinterHelpers(Printer):
comma = ', '
print(one_arg, end='')
- print(') = (void *) %d;' % helper.enum_val)
+ print(')', end='')
+ if proto['name'] in self.nomerge_helpers:
+ print(' __nomerge', end='')
+
+ print(' = (void *) %d;' % helper.enum_val)
print('')
###############################################################################
--
2.40.1
