bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: fix missing * in bpf.h
@ 2021-02-23 12:45 Hangbin Liu
  2021-02-23 13:14 ` [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description Hangbin Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Hangbin Liu @ 2021-02-23 12:45 UTC (permalink / raw)
  To: bpf; +Cc: netdev, Daniel Borkmann, Jesper Dangaard Brouer, Hangbin Liu

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 <liuhangbin@gmail.com>
---
 include/uapi/linux/bpf.h       | 2 +-
 tools/include/uapi/linux/bpf.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4c24daa43bac..46248f8e024b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3850,7 +3850,7 @@ union bpf_attr {
  *
  * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
  *	Description
-
+ *
  *		Check ctx packet size against exceeding MTU of net device (based
  *		on *ifindex*).  This helper will likely be used in combination
  *		with helpers that adjust/change the packet size.
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4c24daa43bac..46248f8e024b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3850,7 +3850,7 @@ union bpf_attr {
  *
  * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
  *	Description
-
+ *
  *		Check ctx packet size against exceeding MTU of net device (based
  *		on *ifindex*).  This helper will likely be used in combination
  *		with helpers that adjust/change the packet size.
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description
  2021-02-23 12:45 [PATCH bpf-next] bpf: fix missing * in bpf.h Hangbin Liu
@ 2021-02-23 13:14 ` Hangbin Liu
  2021-02-24  9:19   ` Jesper Dangaard Brouer
  2021-02-24 16:30   ` patchwork-bot+netdevbpf
  2021-02-23 14:43 ` [PATCH bpf-next] bpf: fix missing * in bpf.h Jesper Dangaard Brouer
  2021-02-24 16:00 ` patchwork-bot+netdevbpf
  2 siblings, 2 replies; 11+ messages in thread
From: Hangbin Liu @ 2021-02-23 13:14 UTC (permalink / raw)
  To: bpf; +Cc: netdev, Daniel Borkmann, Jesper Dangaard Brouer, Hangbin Liu

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 <liuhangbin@gmail.com>
---
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(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4c24daa43bac..79c893310492 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3850,7 +3850,6 @@ union bpf_attr {
  *
  * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
  *	Description
-
  *		Check ctx packet size against exceeding MTU of net device (based
  *		on *ifindex*).  This helper will likely be used in combination
  *		with helpers that adjust/change the packet size.
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4c24daa43bac..79c893310492 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3850,7 +3850,6 @@ union bpf_attr {
  *
  * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
  *	Description
-
  *		Check ctx packet size against exceeding MTU of net device (based
  *		on *ifindex*).  This helper will likely be used in combination
  *		with helpers that adjust/change the packet size.
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] bpf: fix missing * in bpf.h
  2021-02-23 12:45 [PATCH bpf-next] bpf: fix missing * in bpf.h Hangbin Liu
  2021-02-23 13:14 ` [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description Hangbin Liu
@ 2021-02-23 14:43 ` Jesper Dangaard Brouer
  2021-02-24 15:55   ` Daniel Borkmann
  2021-02-24 16:00 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2021-02-23 14:43 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: bpf, netdev, Daniel Borkmann, brouer

On Tue, 23 Feb 2021 20:45:54 +0800
Hangbin Liu <liuhangbin@gmail.com> 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 <liuhangbin@gmail.com>
> ---
>  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 <brouer@redhat.com>

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 -


> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4c24daa43bac..46248f8e024b 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3850,7 +3850,7 @@ union bpf_attr {
>   *
>   * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
>   *	Description
> -
> + *
>   *		Check ctx packet size against exceeding MTU of net device (based
>   *		on *ifindex*).  This helper will likely be used in combination
>   *		with helpers that adjust/change the packet size.
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 4c24daa43bac..46248f8e024b 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -3850,7 +3850,7 @@ union bpf_attr {
>   *
>   * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
>   *	Description
> -
> + *
>   *		Check ctx packet size against exceeding MTU of net device (based
>   *		on *ifindex*).  This helper will likely be used in combination
>   *		with helpers that adjust/change the packet size.



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description
  2021-02-23 13:14 ` [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description Hangbin Liu
@ 2021-02-24  9:19   ` Jesper Dangaard Brouer
  2021-02-24 16:30   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2021-02-24  9:19 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: bpf, netdev, Daniel Borkmann, brouer

On Tue, 23 Feb 2021 21:14:57 +0800
Hangbin Liu <liuhangbin@gmail.com> 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 <liuhangbin@gmail.com>
> ---
> 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(-)

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>


> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4c24daa43bac..79c893310492 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3850,7 +3850,6 @@ union bpf_attr {
>   *
>   * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
>   *	Description
> -
>   *		Check ctx packet size against exceeding MTU of net device (based
>   *		on *ifindex*).  This helper will likely be used in combination
>   *		with helpers that adjust/change the packet size.
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 4c24daa43bac..79c893310492 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -3850,7 +3850,6 @@ union bpf_attr {
>   *
>   * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
>   *	Description
> -
>   *		Check ctx packet size against exceeding MTU of net device (based
>   *		on *ifindex*).  This helper will likely be used in combination
>   *		with helpers that adjust/change the packet size.



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] bpf: fix missing * in bpf.h
  2021-02-23 14:43 ` [PATCH bpf-next] bpf: fix missing * in bpf.h Jesper Dangaard Brouer
@ 2021-02-24 15:55   ` Daniel Borkmann
  2021-02-24 18:59     ` Andrii Nakryiko
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Borkmann @ 2021-02-24 15:55 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Hangbin Liu; +Cc: bpf, netdev, andrii.nakryiko

On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
> On Tue, 23 Feb 2021 20:45:54 +0800
> Hangbin Liu <liuhangbin@gmail.com> 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 <liuhangbin@gmail.com>
>> ---
>>   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 <brouer@redhat.com>

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?

>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 4c24daa43bac..46248f8e024b 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -3850,7 +3850,7 @@ union bpf_attr {
>>    *
>>    * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
>>    *	Description
>> -
>> + *
>>    *		Check ctx packet size against exceeding MTU of net device (based
>>    *		on *ifindex*).  This helper will likely be used in combination
>>    *		with helpers that adjust/change the packet size.
>> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
>> index 4c24daa43bac..46248f8e024b 100644
>> --- a/tools/include/uapi/linux/bpf.h
>> +++ b/tools/include/uapi/linux/bpf.h
>> @@ -3850,7 +3850,7 @@ union bpf_attr {
>>    *
>>    * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
>>    *	Description
>> -
>> + *
>>    *		Check ctx packet size against exceeding MTU of net device (based
>>    *		on *ifindex*).  This helper will likely be used in combination
>>    *		with helpers that adjust/change the packet size.
> 
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] bpf: fix missing * in bpf.h
  2021-02-23 12:45 [PATCH bpf-next] bpf: fix missing * in bpf.h Hangbin Liu
  2021-02-23 13:14 ` [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description Hangbin Liu
  2021-02-23 14:43 ` [PATCH bpf-next] bpf: fix missing * in bpf.h Jesper Dangaard Brouer
@ 2021-02-24 16:00 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-24 16:00 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: bpf, netdev, daniel, brouer

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 <liuhangbin@gmail.com>
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpf: fix missing * in bpf.h
    https://git.kernel.org/bpf/bpf/c/f566aac4e053

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description
  2021-02-23 13:14 ` [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description Hangbin Liu
  2021-02-24  9:19   ` Jesper Dangaard Brouer
@ 2021-02-24 16:30   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-24 16:30 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: bpf, netdev, daniel, brouer

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 <liuhangbin@gmail.com>
> 
> [...]

Here is the summary with links:
  - [PATCHv2,bpf-next] bpf: remove blank line in bpf helper description
    https://git.kernel.org/bpf/bpf/c/a7c9c25a99bb

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] bpf: fix missing * in bpf.h
  2021-02-24 15:55   ` Daniel Borkmann
@ 2021-02-24 18:59     ` Andrii Nakryiko
  2021-02-26 16:50       ` Quentin Monnet
  0 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2021-02-24 18:59 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Jesper Dangaard Brouer, Hangbin Liu, bpf, Networking

On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
> > On Tue, 23 Feb 2021 20:45:54 +0800
> > Hangbin Liu <liuhangbin@gmail.com> 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 <liuhangbin@gmail.com>
> >> ---
> >>   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 <brouer@redhat.com>
>
> 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?

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.

>
> >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> >> index 4c24daa43bac..46248f8e024b 100644
> >> --- a/include/uapi/linux/bpf.h
> >> +++ b/include/uapi/linux/bpf.h
> >> @@ -3850,7 +3850,7 @@ union bpf_attr {
> >>    *
> >>    * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
> >>    * Description
> >> -
> >> + *
> >>    *         Check ctx packet size against exceeding MTU of net device (based
> >>    *         on *ifindex*).  This helper will likely be used in combination
> >>    *         with helpers that adjust/change the packet size.
> >> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> >> index 4c24daa43bac..46248f8e024b 100644
> >> --- a/tools/include/uapi/linux/bpf.h
> >> +++ b/tools/include/uapi/linux/bpf.h
> >> @@ -3850,7 +3850,7 @@ union bpf_attr {
> >>    *
> >>    * long bpf_check_mtu(void *ctx, u32 ifindex, u32 *mtu_len, s32 len_diff, u64 flags)
> >>    * Description
> >> -
> >> + *
> >>    *         Check ctx packet size against exceeding MTU of net device (based
> >>    *         on *ifindex*).  This helper will likely be used in combination
> >>    *         with helpers that adjust/change the packet size.
> >
> >
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] bpf: fix missing * in bpf.h
  2021-02-24 18:59     ` Andrii Nakryiko
@ 2021-02-26 16:50       ` Quentin Monnet
  2021-02-26 19:59         ` Andrii Nakryiko
  2021-03-02  1:31         ` Joe Stringer
  0 siblings, 2 replies; 11+ messages in thread
From: Quentin Monnet @ 2021-02-26 16:50 UTC (permalink / raw)
  To: Andrii Nakryiko, Daniel Borkmann
  Cc: Jesper Dangaard Brouer, Hangbin Liu, bpf, Networking, Joe Stringer

2021-02-24 10:59 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
>>> On Tue, 23 Feb 2021 20:45:54 +0800
>>> Hangbin Liu <liuhangbin@gmail.com> 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 <liuhangbin@gmail.com>
>>>> ---
>>>>   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 <brouer@redhat.com>
>>
>> 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?
> 
> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] bpf: fix missing * in bpf.h
  2021-02-26 16:50       ` Quentin Monnet
@ 2021-02-26 19:59         ` Andrii Nakryiko
  2021-03-02  1:31         ` Joe Stringer
  1 sibling, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2021-02-26 19:59 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Daniel Borkmann, Jesper Dangaard Brouer, Hangbin Liu, bpf,
	Networking, Joe Stringer

On Fri, Feb 26, 2021 at 8:50 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-02-24 10:59 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>
> >> On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
> >>> On Tue, 23 Feb 2021 20:45:54 +0800
> >>> Hangbin Liu <liuhangbin@gmail.com> 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 <liuhangbin@gmail.com>
> >>>> ---
> >>>>   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 <brouer@redhat.com>
> >>
> >> 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?
> >
> > 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).

Fine by me as well.


>
> [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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] bpf: fix missing * in bpf.h
  2021-02-26 16:50       ` Quentin Monnet
  2021-02-26 19:59         ` Andrii Nakryiko
@ 2021-03-02  1:31         ` Joe Stringer
  1 sibling, 0 replies; 11+ messages in thread
From: Joe Stringer @ 2021-03-02  1:31 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Andrii Nakryiko, Daniel Borkmann, Jesper Dangaard Brouer,
	Hangbin Liu, bpf, Networking, Joe Stringer

On Fri, Feb 26, 2021 at 8:51 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-02-24 10:59 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>
> >> On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
> >>> On Tue, 23 Feb 2021 20:45:54 +0800
> >>> Hangbin Liu <liuhangbin@gmail.com> 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 <liuhangbin@gmail.com>
> >>>> ---
> >>>>   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 <brouer@redhat.com>
> >>
> >> 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?
> >
> > 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:

diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
index e2ffac2b7695..74cdcc2bbf18 100755
--- a/scripts/bpf_doc.py
+++ b/scripts/bpf_doc.py
@@ -183,25 +183,51 @@ class HeaderParser(object):
         self.reader.readline()
         self.line = self.reader.readline()

+    def get_elem_count(self, target):
+        self.seek_to(target, 'Could not find symbol "%s"' % target)
+        end_re = re.compile('^$')
+        count = 0
+        while True:
+            capture = end_re.match(self.line)
+            if capture:
+                break
+            self.line = self.reader.readline()
+            count += 1
+
+        # The last line (either '};' or '/* */' doesn't count.
+        return count
+

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.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-03-02 10:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-23 12:45 [PATCH bpf-next] bpf: fix missing * in bpf.h Hangbin Liu
2021-02-23 13:14 ` [PATCHv2 bpf-next] bpf: remove blank line in bpf helper description Hangbin Liu
2021-02-24  9:19   ` Jesper Dangaard Brouer
2021-02-24 16:30   ` patchwork-bot+netdevbpf
2021-02-23 14:43 ` [PATCH bpf-next] bpf: fix missing * in bpf.h Jesper Dangaard Brouer
2021-02-24 15:55   ` Daniel Borkmann
2021-02-24 18:59     ` Andrii Nakryiko
2021-02-26 16:50       ` Quentin Monnet
2021-02-26 19:59         ` Andrii Nakryiko
2021-03-02  1:31         ` Joe Stringer
2021-02-24 16:00 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).