From: Hangbin Liu <hidden> Date: 2021-02-23 12:47:17
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
include/uapi/linux/bpf.h | 2 +-
tools/include/uapi/linux/bpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
From: Hangbin Liu <hidden> Date: 2021-02-23 13:16:19
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") added an
extra blank line in bpf helper description. This will make
bpf_helpers_doc.py stop building bpf_helper_defs.h immediately after
bpf_check_mtu, which will affect future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
v2: remove the blank line directly instead of adding a *
---
include/uapi/linux/bpf.h | 1 -
tools/include/uapi/linux/bpf.h | 1 -
2 files changed, 2 deletions(-)
On Tue, 23 Feb 2021 20:45:54 +0800
Hangbin Liu [off-list ref] wrote:
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
include/uapi/linux/bpf.h | 2 +-
tools/include/uapi/linux/bpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for fixing that!
Acked-by: Jesper Dangaard Brouer <redacted>
I though I had already fix that, but I must have missed or reintroduced
this, when I rolling back broken ideas in V13.
I usually run this command to check the man-page (before submitting):
./scripts/bpf_helpers_doc.py | rst2man | man -l -
On Tue, 23 Feb 2021 21:14:57 +0800
Hangbin Liu [off-list ref] wrote:
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") added an
extra blank line in bpf helper description. This will make
bpf_helpers_doc.py stop building bpf_helper_defs.h immediately after
bpf_check_mtu, which will affect future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
v2: remove the blank line directly instead of adding a *
---
include/uapi/linux/bpf.h | 1 -
tools/include/uapi/linux/bpf.h | 1 -
2 files changed, 2 deletions(-)
From: Daniel Borkmann <daniel@iogearbox.net> Date: 2021-02-24 16:09:14
On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
On Tue, 23 Feb 2021 20:45:54 +0800
Hangbin Liu [off-list ref] wrote:
quoted
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
include/uapi/linux/bpf.h | 2 +-
tools/include/uapi/linux/bpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for fixing that!
Acked-by: Jesper Dangaard Brouer <redacted>
Thanks guys, applied!
I though I had already fix that, but I must have missed or reintroduced
this, when I rolling back broken ideas in V13.
I usually run this command to check the man-page (before submitting):
./scripts/bpf_helpers_doc.py | rst2man | man -l -
[+ Andrii] maybe this could be included to run as part of CI to catch such
things in advance?
Hello:
This patch was applied to bpf/bpf.git (refs/heads/master):
On Tue, 23 Feb 2021 20:45:54 +0800 you wrote:
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
[...]
Hello:
This patch was applied to bpf/bpf.git (refs/heads/master):
On Tue, 23 Feb 2021 21:14:57 +0800 you wrote:
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") added an
extra blank line in bpf helper description. This will make
bpf_helpers_doc.py stop building bpf_helper_defs.h immediately after
bpf_check_mtu, which will affect future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
[...]
On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann [off-list ref] wrote:
On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
quoted
On Tue, 23 Feb 2021 20:45:54 +0800
Hangbin Liu [off-list ref] wrote:
quoted
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
include/uapi/linux/bpf.h | 2 +-
tools/include/uapi/linux/bpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for fixing that!
Acked-by: Jesper Dangaard Brouer <redacted>
Thanks guys, applied!
quoted
I though I had already fix that, but I must have missed or reintroduced
this, when I rolling back broken ideas in V13.
I usually run this command to check the man-page (before submitting):
./scripts/bpf_helpers_doc.py | rst2man | man -l -
[+ Andrii] maybe this could be included to run as part of CI to catch such
things in advance?
We do something like that as part of bpftool build, so there is no
reason we can't add this to selftests/bpf/Makefile as well.
On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann [off-list ref] wrote:
quoted
On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
quoted
On Tue, 23 Feb 2021 20:45:54 +0800
Hangbin Liu [off-list ref] wrote:
quoted
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
include/uapi/linux/bpf.h | 2 +-
tools/include/uapi/linux/bpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for fixing that!
Acked-by: Jesper Dangaard Brouer <redacted>
Thanks guys, applied!
quoted
I though I had already fix that, but I must have missed or reintroduced
this, when I rolling back broken ideas in V13.
I usually run this command to check the man-page (before submitting):
./scripts/bpf_helpers_doc.py | rst2man | man -l -
[+ Andrii] maybe this could be included to run as part of CI to catch such
things in advance?
We do something like that as part of bpftool build, so there is no
reason we can't add this to selftests/bpf/Makefile as well.
Hi, pretty sure this is the case already? [0]
This helps catching RST formatting issues, for example if a description
is using invalid markup, and reported by rst2man. My understanding is
that in the current case, the missing star simply ends the block for the
helpers documentation from the parser point of view, it's not considered
an error.
I see two possible workarounds:
1) Check that the number of helpers found ("len(self.helpers)") is equal
to the number of helpers in the file, but that requires knowing how many
helpers we have in the first place (e.g. parsing "__BPF_FUNC_MAPPER(FN)").
2) Add some ending tag to the documentation block, and make sure we
eventually reach it. This is probably a much simpler solution. I could
work on this (or sync with Joe (+Cc) who is also working on these bits
for documenting the bpf() syscall).
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/Makefile?h=v5.11#n189
Quentin
On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann [off-list ref] wrote:
quoted
On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
quoted
On Tue, 23 Feb 2021 20:45:54 +0800
Hangbin Liu [off-list ref] wrote:
quoted
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
include/uapi/linux/bpf.h | 2 +-
tools/include/uapi/linux/bpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for fixing that!
Acked-by: Jesper Dangaard Brouer <redacted>
Thanks guys, applied!
quoted
I though I had already fix that, but I must have missed or reintroduced
this, when I rolling back broken ideas in V13.
I usually run this command to check the man-page (before submitting):
./scripts/bpf_helpers_doc.py | rst2man | man -l -
[+ Andrii] maybe this could be included to run as part of CI to catch such
things in advance?
We do something like that as part of bpftool build, so there is no
reason we can't add this to selftests/bpf/Makefile as well.
Hi, pretty sure this is the case already? [0]
This helps catching RST formatting issues, for example if a description
is using invalid markup, and reported by rst2man. My understanding is
that in the current case, the missing star simply ends the block for the
helpers documentation from the parser point of view, it's not considered
an error.
I see two possible workarounds:
1) Check that the number of helpers found ("len(self.helpers)") is equal
to the number of helpers in the file, but that requires knowing how many
helpers we have in the first place (e.g. parsing "__BPF_FUNC_MAPPER(FN)").
It's a bit hacky, but you could also just count a number of '*
\tDescription' lines.
2) Add some ending tag to the documentation block, and make sure we
eventually reach it. This is probably a much simpler solution. I could
work on this (or sync with Joe (+Cc) who is also working on these bits
for documenting the bpf() syscall).
On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann [off-list ref] wrote:
quoted
On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
quoted
On Tue, 23 Feb 2021 20:45:54 +0800
Hangbin Liu [off-list ref] wrote:
quoted
Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
in bpf.h. This will make bpf_helpers_doc.py stop building
bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
future add functions.
Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
Signed-off-by: Hangbin Liu <redacted>
---
include/uapi/linux/bpf.h | 2 +-
tools/include/uapi/linux/bpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for fixing that!
Acked-by: Jesper Dangaard Brouer <redacted>
Thanks guys, applied!
quoted
I though I had already fix that, but I must have missed or reintroduced
this, when I rolling back broken ideas in V13.
I usually run this command to check the man-page (before submitting):
./scripts/bpf_helpers_doc.py | rst2man | man -l -
[+ Andrii] maybe this could be included to run as part of CI to catch such
things in advance?
We do something like that as part of bpftool build, so there is no
reason we can't add this to selftests/bpf/Makefile as well.
Hi, pretty sure this is the case already? [0]
This helps catching RST formatting issues, for example if a description
is using invalid markup, and reported by rst2man. My understanding is
that in the current case, the missing star simply ends the block for the
helpers documentation from the parser point of view, it's not considered
an error.
I see two possible workarounds:
1) Check that the number of helpers found ("len(self.helpers)") is equal
to the number of helpers in the file, but that requires knowing how many
helpers we have in the first place (e.g. parsing "__BPF_FUNC_MAPPER(FN)").
This is not so difficult as long as we stick to one symbol per line:
@@ -183,25 +183,51 @@ class HeaderParser(object):self.reader.readline()self.line=self.reader.readline()+defget_elem_count(self,target):+self.seek_to(target,'Could not find symbol "%s"'%target)+end_re=re.compile('^$')+count=0+whileTrue:+capture=end_re.match(self.line)+ifcapture:+break+self.line=self.reader.readline()+count+=1++# The last line (either '};' or '/* */' doesn't count.+returncount+
I can either roll this into my docs update v2, or hold onto it for
another dedicated patch fixup. Either way I'm trialing this out
locally to regression-test my own docs update PR and make sure I'm not
breaking one of the various output formats.