From: Eyal Birger <eyal.birger@gmail.com>
To: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
haoluo@google.com, jolsa@kernel.org
Cc: bpf@vger.kernel.org, Eyal Birger <eyal.birger@gmail.com>
Subject: [PATCH bpf-next] bpf/scripts: use helper enum value instead of relying on comment order
Date: Fri, 19 Aug 2022 12:12:44 +0300 [thread overview]
Message-ID: <20220819091244.1001962-1-eyal.birger@gmail.com> (raw)
The helper value is ABI as defined by enum bpf_func_id.
As bpf_helper_defs.h is used for the userpace part, it must be consistent
with this enum.
Before this change, the enumerated value was derived from the comment
order, which assumes comments are always appended, however, there doesn't
seem to be an enforcement anywhere for maintaining a strict order.
When adding new helpers it is very puzzling when the userspace application
breaks in weird places if the comment is inserted instead of appended -
because the generated helper ABI is incorrect and shifted.
This commit attempts to ease this by always using bpf_func_id order as
the helper value.
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
scripts/bpf_doc.py | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
index dfb260de17a8..7797aa032eca 100755
--- a/scripts/bpf_doc.py
+++ b/scripts/bpf_doc.py
@@ -88,7 +88,7 @@ class HeaderParser(object):
self.helpers = []
self.commands = []
self.desc_unique_helpers = set()
- self.define_unique_helpers = []
+ self.define_unique_helpers = {}
self.desc_syscalls = []
self.enum_syscalls = []
@@ -245,24 +245,24 @@ class HeaderParser(object):
break
def parse_define_helpers(self):
- # Parse the number of FN(...) in #define __BPF_FUNC_MAPPER to compare
- # later with the number of unique function names present in description.
+ # Parse FN(...) in #define __BPF_FUNC_MAPPER to compare later with the
+ # number of unique function names present in description and use the
+ # correct enumeration value.
# Note: seek_to(..) discards the first line below the target search text,
# resulting in FN(unspec) being skipped and not added to self.define_unique_helpers.
self.seek_to('#define __BPF_FUNC_MAPPER(FN)',
'Could not find start of eBPF helper definition list')
# Searches for either one or more FN(\w+) defines or a backslash for newline
- p = re.compile('\s*(FN\(\w+\))+|\\\\')
- fn_defines_str = ''
+ p = re.compile('\s*FN\((\w+)\)+|\\\\')
+ i = 1 # 'unspec' is skipped as mentioned above
while True:
capture = p.match(self.line)
if capture:
- fn_defines_str += self.line
+ self.define_unique_helpers[capture.expand(r'bpf_\1')] = i
+ i += 1
else:
break
self.line = self.reader.readline()
- # Find the number of occurences of FN(\w+)
- self.define_unique_helpers = re.findall('FN\(\w+\)', fn_defines_str)
def run(self):
self.parse_desc_syscall()
@@ -573,6 +573,7 @@ class PrinterHelpers(Printer):
def __init__(self, parser):
self.elements = parser.helpers
self.elem_number_check(parser.desc_unique_helpers, parser.define_unique_helpers, 'helper', '__BPF_FUNC_MAPPER')
+ self.define_unique_helpers = parser.define_unique_helpers
type_fwds = [
'struct bpf_fib_lookup',
@@ -761,7 +762,7 @@ class PrinterHelpers(Printer):
comma = ', '
print(one_arg, end='')
- print(') = (void *) %d;' % len(self.seen_helpers))
+ print(') = (void *) %d;' % self.define_unique_helpers[proto['name']])
print('')
###############################################################################
--
2.34.1
next reply other threads:[~2022-08-19 9:13 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-19 9:12 Eyal Birger [this message]
2022-08-23 20:49 ` [PATCH bpf-next] bpf/scripts: use helper enum value instead of relying on comment order Quentin Monnet
2022-08-23 23:05 ` Eyal Birger
2022-08-24 8:44 ` Quentin Monnet
2022-08-24 9:04 ` Eyal Birger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220819091244.1001962-1-eyal.birger@gmail.com \
--to=eyal.birger@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=martin.lau@linux.dev \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox