public inbox for linux-man@vger.kernel.org
 help / color / mirror / Atom feed
* Typo in the man7 bpf-helpers page
@ 2023-01-31 10:03 Zexuan Luo
  2023-01-31 11:00 ` Alejandro Colomar
  0 siblings, 1 reply; 8+ messages in thread
From: Zexuan Luo @ 2023-01-31 10:03 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: linux-man

Hello Colomar,

I just found a potential bug in the bpf-helpers page.

Under the https://www.man7.org/linux/man-pages/man7/bpf-helpers.7.html:

```
       u64 bpf_get_socket_cookie(struct sk_buff *skb)

              Description
                     If the struct sk_buff pointed by skb has a known
                     socket, retrieve the cookie (generated by the
                     kernel) of this socket.  If no cookie has been set
                     yet, generate a new cookie. Once generated, the
                     socket cookie remains stable for the life of the
                     socket. This helper can be useful for monitoring
                     per socket networking traffic statistics as it
                     provides a global socket identifier that can be
                     assumed unique.

              Return A 8-byte long non-decreasing number on success, or
                     0 if the socket field is missing inside skb.

       u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)

              Description
                     Equivalent to bpf_get_socket_cookie() helper that
                     accepts skb, but gets socket from struct
                     bpf_sock_addr context.

              Return A 8-byte long non-decreasing number.

       u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)

              Description
                     Equivalent to bpf_get_socket_cookie() helper that
                     accepts skb, but gets socket from struct
                     bpf_sock_ops context.

              Return A 8-byte long non-decreasing number.
```

The function bpf_get_socket_cookie repeats three times. The second one
should be bpf_get_socket_cookie_addr and the third one should be
bpf_get_socket_cookie_ops.

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

* Re: Typo in the man7 bpf-helpers page
  2023-01-31 10:03 Typo in the man7 bpf-helpers page Zexuan Luo
@ 2023-01-31 11:00 ` Alejandro Colomar
  2023-01-31 11:40   ` Alejandro Colomar
  0 siblings, 1 reply; 8+ messages in thread
From: Alejandro Colomar @ 2023-01-31 11:00 UTC (permalink / raw)
  To: Zexuan Luo, Quentin Monnet, bpf; +Cc: linux-man, Alejandro Colomar


[-- Attachment #1.1: Type: text/plain, Size: 2565 bytes --]

Hi Zexuan, Quentin,

On 1/31/23 11:03, Zexuan Luo wrote:
> Hello Colomar,
> 
> I just found a potential bug in the bpf-helpers page.

Thanks for reporting bugs :)

> 
> Under the https://www.man7.org/linux/man-pages/man7/bpf-helpers.7.html:

This page is generated from the Linux kernel sources.  I've CCed Quentin and the 
BPF list so they can check it there.

BTW, I'm refreshing the page now.

Quentin, I realized in the diff that there is some inconsistency in the number 
of spaces after a sentence-ending period.  Could you please use two spaces for 
that?  It's especially important for groff(1), which will render it differently. 
  However, it's not a big issue, so don't feel urged to do that.

Cheers,

Alex

> 
> ```
>         u64 bpf_get_socket_cookie(struct sk_buff *skb)
> 
>                Description
>                       If the struct sk_buff pointed by skb has a known
>                       socket, retrieve the cookie (generated by the
>                       kernel) of this socket.  If no cookie has been set
>                       yet, generate a new cookie. Once generated, the
>                       socket cookie remains stable for the life of the
>                       socket. This helper can be useful for monitoring
>                       per socket networking traffic statistics as it
>                       provides a global socket identifier that can be
>                       assumed unique.
> 
>                Return A 8-byte long non-decreasing number on success, or
>                       0 if the socket field is missing inside skb.
> 
>         u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
> 
>                Description
>                       Equivalent to bpf_get_socket_cookie() helper that
>                       accepts skb, but gets socket from struct
>                       bpf_sock_addr context.
> 
>                Return A 8-byte long non-decreasing number.
> 
>         u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
> 
>                Description
>                       Equivalent to bpf_get_socket_cookie() helper that
>                       accepts skb, but gets socket from struct
>                       bpf_sock_ops context.
> 
>                Return A 8-byte long non-decreasing number.
> ```
> 
> The function bpf_get_socket_cookie repeats three times. The second one
> should be bpf_get_socket_cookie_addr and the third one should be
> bpf_get_socket_cookie_ops.

-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Typo in the man7 bpf-helpers page
  2023-01-31 11:00 ` Alejandro Colomar
@ 2023-01-31 11:40   ` Alejandro Colomar
  2023-01-31 11:51     ` Alejandro Colomar
  2023-01-31 12:02     ` Quentin Monnet
  0 siblings, 2 replies; 8+ messages in thread
From: Alejandro Colomar @ 2023-01-31 11:40 UTC (permalink / raw)
  To: Zexuan Luo, bpf, Quentin Monnet; +Cc: linux-man, Alejandro Colomar


[-- Attachment #1.1: Type: text/plain, Size: 2648 bytes --]

[Resend with Quentin's right address, I hope]

Hi Zexuan, Quentin,

On 1/31/23 11:03, Zexuan Luo wrote:
 > Hello Colomar,
 >
 > I just found a potential bug in the bpf-helpers page.

Thanks for reporting bugs :)

 >
 > Under the https://www.man7.org/linux/man-pages/man7/bpf-helpers.7.html:

This page is generated from the Linux kernel sources.  I've CCed Quentin and the 
BPF list so they can check it there.

BTW, I'm refreshing the page now.

Quentin, I realized in the diff that there is some inconsistency in the number 
of spaces after a sentence-ending period.  Could you please use two spaces for 
that?  It's especially important for groff(1), which will render it differently. 
   However, it's not a big issue, so don't feel urged to do that.

Cheers,

Alex

 >
 > ```
 >         u64 bpf_get_socket_cookie(struct sk_buff *skb)
 >
 >                Description
 >                       If the struct sk_buff pointed by skb has a known
 >                       socket, retrieve the cookie (generated by the
 >                       kernel) of this socket.  If no cookie has been set
 >                       yet, generate a new cookie. Once generated, the
 >                       socket cookie remains stable for the life of the
 >                       socket. This helper can be useful for monitoring
 >                       per socket networking traffic statistics as it
 >                       provides a global socket identifier that can be
 >                       assumed unique.
 >
 >                Return A 8-byte long non-decreasing number on success, or
 >                       0 if the socket field is missing inside skb.
 >
 >         u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
 >
 >                Description
 >                       Equivalent to bpf_get_socket_cookie() helper that
 >                       accepts skb, but gets socket from struct
 >                       bpf_sock_addr context.
 >
 >                Return A 8-byte long non-decreasing number.
 >
 >         u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
 >
 >                Description
 >                       Equivalent to bpf_get_socket_cookie() helper that
 >                       accepts skb, but gets socket from struct
 >                       bpf_sock_ops context.
 >
 >                Return A 8-byte long non-decreasing number.
 > ```
 >
 > The function bpf_get_socket_cookie repeats three times. The second one
 > should be bpf_get_socket_cookie_addr and the third one should be
 > bpf_get_socket_cookie_ops.

-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Typo in the man7 bpf-helpers page
  2023-01-31 11:40   ` Alejandro Colomar
@ 2023-01-31 11:51     ` Alejandro Colomar
  2023-01-31 12:02     ` Quentin Monnet
  1 sibling, 0 replies; 8+ messages in thread
From: Alejandro Colomar @ 2023-01-31 11:51 UTC (permalink / raw)
  To: bpf, Quentin Monnet; +Cc: linux-man, Alejandro Colomar, Zexuan Luo


[-- Attachment #1.1: Type: text/plain, Size: 3542 bytes --]

Hi Quentin,

On 1/31/23 12:40, Alejandro Colomar wrote:
> [Resend with Quentin's right address, I hope]
> 
> Hi Zexuan, Quentin,
> 
> On 1/31/23 11:03, Zexuan Luo wrote:
>  > Hello Colomar,
>  >
>  > I just found a potential bug in the bpf-helpers page.
> 
> Thanks for reporting bugs :)
> 
>  >
>  > Under the https://www.man7.org/linux/man-pages/man7/bpf-helpers.7.html:
> 
> This page is generated from the Linux kernel sources.  I've CCed Quentin and the 
> BPF list so they can check it there.
> 
> BTW, I'm refreshing the page now.
> 
> Quentin, I realized in the diff that there is some inconsistency in the number 
> of spaces after a sentence-ending period.  Could you please use two spaces for 
> that?  It's especially important for groff(1), which will render it differently. 
>    However, it's not a big issue, so don't feel urged to do that.

I also found some trailing whitespace.  I'm not sure if it's necessary for rst, 
but in man(7) it causes trailing whitespace.  grep for '^\.\. $'  (and maybe 
there are others that I didn't notice.

Cheers,

Alex

> 
> Cheers,
> 
> Alex
> 
>  >
>  > ```
>  >         u64 bpf_get_socket_cookie(struct sk_buff *skb)
>  >
>  >                Description
>  >                       If the struct sk_buff pointed by skb has a known
>  >                       socket, retrieve the cookie (generated by the
>  >                       kernel) of this socket.  If no cookie has been set
>  >                       yet, generate a new cookie. Once generated, the
>  >                       socket cookie remains stable for the life of the
>  >                       socket. This helper can be useful for monitoring
>  >                       per socket networking traffic statistics as it
>  >                       provides a global socket identifier that can be
>  >                       assumed unique.
>  >
>  >                Return A 8-byte long non-decreasing number on success, or
>  >                       0 if the socket field is missing inside skb.
>  >
>  >         u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
>  >
>  >                Description
>  >                       Equivalent to bpf_get_socket_cookie() helper that
>  >                       accepts skb, but gets socket from struct
>  >                       bpf_sock_addr context.
>  >
>  >                Return A 8-byte long non-decreasing number.
>  >
>  >         u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
>  >
>  >                Description
>  >                       Equivalent to bpf_get_socket_cookie() helper that
>  >                       accepts skb, but gets socket from struct
>  >                       bpf_sock_ops context.
>  >
>  >                Return A 8-byte long non-decreasing number.
>  > ```
>  >
>  > The function bpf_get_socket_cookie repeats three times. The second one
>  > should be bpf_get_socket_cookie_addr and the third one should be
>  > bpf_get_socket_cookie_ops.
> 

-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Typo in the man7 bpf-helpers page
  2023-01-31 11:40   ` Alejandro Colomar
  2023-01-31 11:51     ` Alejandro Colomar
@ 2023-01-31 12:02     ` Quentin Monnet
  2023-01-31 14:36       ` Zexuan Luo
  1 sibling, 1 reply; 8+ messages in thread
From: Quentin Monnet @ 2023-01-31 12:02 UTC (permalink / raw)
  To: Alejandro Colomar, Zexuan Luo, bpf; +Cc: linux-man, Alejandro Colomar

2023-01-31 12:40 UTC+0100 ~ Alejandro Colomar <alx.manpages@gmail.com>
> [Resend with Quentin's right address, I hope]
> 
> Hi Zexuan, Quentin,
> 
> On 1/31/23 11:03, Zexuan Luo wrote:
>> Hello Colomar,
>>
>> I just found a potential bug in the bpf-helpers page.
> 
> Thanks for reporting bugs :)
> 
>>
>> Under the https://www.man7.org/linux/man-pages/man7/bpf-helpers.7.html:
> 
> This page is generated from the Linux kernel sources.  I've CCed Quentin
> and the BPF list so they can check it there.
> 

Hi Alejandro, Zexuan,
Thanks for the report! Happy to take fixes, however, see below...

> BTW, I'm refreshing the page now.
> 

Great, thank you!

> Quentin, I realized in the diff that there is some inconsistency in the
> number of spaces after a sentence-ending period.  Could you please use
> two spaces for that?  It's especially important for groff(1), which will
> render it differently.   However, it's not a big issue, so don't feel
> urged to do that.

Yes, you mentioned that in the past and this is on my list. As you can
see, I haven't felt urged so far indeed :). But it's still on my mind
for the next time I take a look at this doc for typos etc.

> 
> Cheers,
> 
> Alex
> 
>>
>> ```
>>         u64 bpf_get_socket_cookie(struct sk_buff *skb)
>>
>>                Description
>>                       If the struct sk_buff pointed by skb has a known
>>                       socket, retrieve the cookie (generated by the
>>                       kernel) of this socket.  If no cookie has been set
>>                       yet, generate a new cookie. Once generated, the
>>                       socket cookie remains stable for the life of the
>>                       socket. This helper can be useful for monitoring
>>                       per socket networking traffic statistics as it
>>                       provides a global socket identifier that can be
>>                       assumed unique.
>>
>>                Return A 8-byte long non-decreasing number on success, or
>>                       0 if the socket field is missing inside skb.
>>
>>         u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
>>
>>                Description
>>                       Equivalent to bpf_get_socket_cookie() helper that
>>                       accepts skb, but gets socket from struct
>>                       bpf_sock_addr context.
>>
>>                Return A 8-byte long non-decreasing number.
>>
>>         u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
>>
>>                Description
>>                       Equivalent to bpf_get_socket_cookie() helper that
>>                       accepts skb, but gets socket from struct
>>                       bpf_sock_ops context.
>>
>>                Return A 8-byte long non-decreasing number.
>> ```
>>
>> The function bpf_get_socket_cookie repeats three times. The second one
>> should be bpf_get_socket_cookie_addr and the third one should be
>> bpf_get_socket_cookie_ops.
> 

No, I don't think there is anything wrong with that. I suppose you mean
bpf_get_socket_cookie_sock_(addr|ops) (the functions you mentioned don't
exist), but the four variants of the helper just have the same name, and
take different objects for their context. There is no risk of collision
because they are each associated to distinct eBPF program types.

Please see also commit d692f1138a4b ("bpf: Support bpf_get_socket_cookie
in more prog types"): "It doesn't introduce new helpers. Instead it
reuses same helper name bpf_get_socket_cookie() but adds support to this
helper to accept `struct bpf_sock_addr` and `struct bpf_sock_ops`.".

Thanks,
Quentin

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

* Re: Typo in the man7 bpf-helpers page
  2023-01-31 12:02     ` Quentin Monnet
@ 2023-01-31 14:36       ` Zexuan Luo
  2023-01-31 14:45         ` Zexuan Luo
  2023-01-31 14:51         ` Quentin Monnet
  0 siblings, 2 replies; 8+ messages in thread
From: Zexuan Luo @ 2023-01-31 14:36 UTC (permalink / raw)
  To: Quentin Monnet; +Cc: Alejandro Colomar, bpf, linux-man, Alejandro Colomar

My bad!

> No, I don't think there is anything wrong with that. I suppose you mean
bpf_get_socket_cookie_sock_(ad
dr|ops) (the functions you mentioned don't
exist), but the four variants of the helper just have the same name, and
take different objects for their context.

Yes! I made a mistake in the function names in the first email. Thank
you for pointing that out.

I am an eBPF newbie and I am learning it currently. AFAIK, language C
doesn't support function overriding via different parameters.
So how do these four functions co-exist?

Some naive search in the kernel code leads me to:
https://elixir.bootlin.com/linux/v6.2-rc6/source/net/core/filter.c#L4919
```
static const struct bpf_func_proto bpf_get_socket_cookie_sock_addr_proto = {
    .func        = bpf_get_socket_cookie_sock_addr,
    .gpl_only    = false,
    .ret_type    = RET_INTEGER,
    .arg1_type    = ARG_PTR_TO_CTX,
};
```

https://elixir.bootlin.com/linux/v6.2-rc6/source/net/core/filter.c#L4955
```
static const struct bpf_func_proto bpf_get_socket_cookie_sock_ops_proto = {
        .func           = bpf_get_socket_cookie_sock_ops,
        .gpl_only       = false,
        .ret_type       = RET_INTEGER,
        .arg1_type      = ARG_PTR_TO_CTX,
};
```

It seems that the function definitions are quite real...

Quentin Monnet <quentin@isovalent.com> 于2023年1月31日周二 20:02写道:

>
> 2023-01-31 12:40 UTC+0100 ~ Alejandro Colomar <alx.manpages@gmail.com>
> > [Resend with Quentin's right address, I hope]
> >
> > Hi Zexuan, Quentin,
> >
> > On 1/31/23 11:03, Zexuan Luo wrote:
> >> Hello Colomar,
> >>
> >> I just found a potential bug in the bpf-helpers page.
> >
> > Thanks for reporting bugs :)
> >
> >>
> >> Under the https://www.man7.org/linux/man-pages/man7/bpf-helpers.7.html:
> >
> > This page is generated from the Linux kernel sources.  I've CCed Quentin
> > and the BPF list so they can check it there.
> >
>
> Hi Alejandro, Zexuan,
> Thanks for the report! Happy to take fixes, however, see below...
>
> > BTW, I'm refreshing the page now.
> >
>
> Great, thank you!
>
> > Quentin, I realized in the diff that there is some inconsistency in the
> > number of spaces after a sentence-ending period.  Could you please use
> > two spaces for that?  It's especially important for groff(1), which will
> > render it differently.   However, it's not a big issue, so don't feel
> > urged to do that.
>
> Yes, you mentioned that in the past and this is on my list. As you can
> see, I haven't felt urged so far indeed :). But it's still on my mind
> for the next time I take a look at this doc for typos etc.
>
> >
> > Cheers,
> >
> > Alex
> >
> >>
> >> ```
> >>         u64 bpf_get_socket_cookie(struct sk_buff *skb)
> >>
> >>                Description
> >>                       If the struct sk_buff pointed by skb has a known
> >>                       socket, retrieve the cookie (generated by the
> >>                       kernel) of this socket.  If no cookie has been set
> >>                       yet, generate a new cookie. Once generated, the
> >>                       socket cookie remains stable for the life of the
> >>                       socket. This helper can be useful for monitoring
> >>                       per socket networking traffic statistics as it
> >>                       provides a global socket identifier that can be
> >>                       assumed unique.
> >>
> >>                Return A 8-byte long non-decreasing number on success, or
> >>                       0 if the socket field is missing inside skb.
> >>
> >>         u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
> >>
> >>                Description
> >>                       Equivalent to bpf_get_socket_cookie() helper that
> >>                       accepts skb, but gets socket from struct
> >>                       bpf_sock_addr context.
> >>
> >>                Return A 8-byte long non-decreasing number.
> >>
> >>         u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
> >>
> >>                Description
> >>                       Equivalent to bpf_get_socket_cookie() helper that
> >>                       accepts skb, but gets socket from struct
> >>                       bpf_sock_ops context.
> >>
> >>                Return A 8-byte long non-decreasing number.
> >> ```
> >>
> >> The function bpf_get_socket_cookie repeats three times. The second one
> >> should be bpf_get_socket_cookie_addr and the third one should be
> >> bpf_get_socket_cookie_ops.
> >
>
> No, I don't think there is anything wrong with that. I suppose you mean
> bpf_get_socket_cookie_sock_(addr|ops) (the functions you mentioned don't
> exist), but the four variants of the helper just have the same name, and
> take different objects for their context. There is no risk of collision
> because they are each associated to distinct eBPF program types.
>
> Please see also commit d692f1138a4b ("bpf: Support bpf_get_socket_cookie
> in more prog types"): "It doesn't introduce new helpers. Instead it
> reuses same helper name bpf_get_socket_cookie() but adds support to this
> helper to accept `struct bpf_sock_addr` and `struct bpf_sock_ops`.".
>
> Thanks,
> Quentin

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

* Re: Typo in the man7 bpf-helpers page
  2023-01-31 14:36       ` Zexuan Luo
@ 2023-01-31 14:45         ` Zexuan Luo
  2023-01-31 14:51         ` Quentin Monnet
  1 sibling, 0 replies; 8+ messages in thread
From: Zexuan Luo @ 2023-01-31 14:45 UTC (permalink / raw)
  To: Quentin Monnet; +Cc: Alejandro Colomar, bpf, linux-man, Alejandro Colomar

Hi Quentin,
After reading https://github.com/torvalds/linux/commit/d692f1138a4b,
now I figure out that only BPF_FUNC_get_socket_cookie is exposed.
The same name (function id BPF_FUNC_get_socket_cookie) will be
resolved to a different function (bpf_get_socket_cookie_sock_ops and
the others) according to the scope.

Thanks for your explanation!

Zexuan Luo <spacewanderlzx@gmail.com> 于2023年1月31日周二 22:36写道:
>
> My bad!
>
> > No, I don't think there is anything wrong with that. I suppose you mean
> bpf_get_socket_cookie_sock_(ad
> dr|ops) (the functions you mentioned don't
> exist), but the four variants of the helper just have the same name, and
> take different objects for their context.
>
> Yes! I made a mistake in the function names in the first email. Thank
> you for pointing that out.
>
> I am an eBPF newbie and I am learning it currently. AFAIK, language C
> doesn't support function overriding via different parameters.
> So how do these four functions co-exist?
>
> Some naive search in the kernel code leads me to:
> https://elixir.bootlin.com/linux/v6.2-rc6/source/net/core/filter.c#L4919
> ```
> static const struct bpf_func_proto bpf_get_socket_cookie_sock_addr_proto = {
>     .func        = bpf_get_socket_cookie_sock_addr,
>     .gpl_only    = false,
>     .ret_type    = RET_INTEGER,
>     .arg1_type    = ARG_PTR_TO_CTX,
> };
> ```
>
> https://elixir.bootlin.com/linux/v6.2-rc6/source/net/core/filter.c#L4955
> ```
> static const struct bpf_func_proto bpf_get_socket_cookie_sock_ops_proto = {
>         .func           = bpf_get_socket_cookie_sock_ops,
>         .gpl_only       = false,
>         .ret_type       = RET_INTEGER,
>         .arg1_type      = ARG_PTR_TO_CTX,
> };
> ```
>
> It seems that the function definitions are quite real...
>
> Quentin Monnet <quentin@isovalent.com> 于2023年1月31日周二 20:02写道:
>
> >
> > 2023-01-31 12:40 UTC+0100 ~ Alejandro Colomar <alx.manpages@gmail.com>
> > > [Resend with Quentin's right address, I hope]
> > >
> > > Hi Zexuan, Quentin,
> > >
> > > On 1/31/23 11:03, Zexuan Luo wrote:
> > >> Hello Colomar,
> > >>
> > >> I just found a potential bug in the bpf-helpers page.
> > >
> > > Thanks for reporting bugs :)
> > >
> > >>
> > >> Under the https://www.man7.org/linux/man-pages/man7/bpf-helpers.7.html:
> > >
> > > This page is generated from the Linux kernel sources.  I've CCed Quentin
> > > and the BPF list so they can check it there.
> > >
> >
> > Hi Alejandro, Zexuan,
> > Thanks for the report! Happy to take fixes, however, see below...
> >
> > > BTW, I'm refreshing the page now.
> > >
> >
> > Great, thank you!
> >
> > > Quentin, I realized in the diff that there is some inconsistency in the
> > > number of spaces after a sentence-ending period.  Could you please use
> > > two spaces for that?  It's especially important for groff(1), which will
> > > render it differently.   However, it's not a big issue, so don't feel
> > > urged to do that.
> >
> > Yes, you mentioned that in the past and this is on my list. As you can
> > see, I haven't felt urged so far indeed :). But it's still on my mind
> > for the next time I take a look at this doc for typos etc.
> >
> > >
> > > Cheers,
> > >
> > > Alex
> > >
> > >>
> > >> ```
> > >>         u64 bpf_get_socket_cookie(struct sk_buff *skb)
> > >>
> > >>                Description
> > >>                       If the struct sk_buff pointed by skb has a known
> > >>                       socket, retrieve the cookie (generated by the
> > >>                       kernel) of this socket.  If no cookie has been set
> > >>                       yet, generate a new cookie. Once generated, the
> > >>                       socket cookie remains stable for the life of the
> > >>                       socket. This helper can be useful for monitoring
> > >>                       per socket networking traffic statistics as it
> > >>                       provides a global socket identifier that can be
> > >>                       assumed unique.
> > >>
> > >>                Return A 8-byte long non-decreasing number on success, or
> > >>                       0 if the socket field is missing inside skb.
> > >>
> > >>         u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
> > >>
> > >>                Description
> > >>                       Equivalent to bpf_get_socket_cookie() helper that
> > >>                       accepts skb, but gets socket from struct
> > >>                       bpf_sock_addr context.
> > >>
> > >>                Return A 8-byte long non-decreasing number.
> > >>
> > >>         u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
> > >>
> > >>                Description
> > >>                       Equivalent to bpf_get_socket_cookie() helper that
> > >>                       accepts skb, but gets socket from struct
> > >>                       bpf_sock_ops context.
> > >>
> > >>                Return A 8-byte long non-decreasing number.
> > >> ```
> > >>
> > >> The function bpf_get_socket_cookie repeats three times. The second one
> > >> should be bpf_get_socket_cookie_addr and the third one should be
> > >> bpf_get_socket_cookie_ops.
> > >
> >
> > No, I don't think there is anything wrong with that. I suppose you mean
> > bpf_get_socket_cookie_sock_(addr|ops) (the functions you mentioned don't
> > exist), but the four variants of the helper just have the same name, and
> > take different objects for their context. There is no risk of collision
> > because they are each associated to distinct eBPF program types.
> >
> > Please see also commit d692f1138a4b ("bpf: Support bpf_get_socket_cookie
> > in more prog types"): "It doesn't introduce new helpers. Instead it
> > reuses same helper name bpf_get_socket_cookie() but adds support to this
> > helper to accept `struct bpf_sock_addr` and `struct bpf_sock_ops`.".
> >
> > Thanks,
> > Quentin

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

* Re: Typo in the man7 bpf-helpers page
  2023-01-31 14:36       ` Zexuan Luo
  2023-01-31 14:45         ` Zexuan Luo
@ 2023-01-31 14:51         ` Quentin Monnet
  1 sibling, 0 replies; 8+ messages in thread
From: Quentin Monnet @ 2023-01-31 14:51 UTC (permalink / raw)
  To: Zexuan Luo; +Cc: Alejandro Colomar, bpf, linux-man, Alejandro Colomar

2023-01-31 22:36 UTC+0800 ~ Zexuan Luo <spacewanderlzx@gmail.com>
> My bad!
> 
>> No, I don't think there is anything wrong with that. I suppose you mean
> bpf_get_socket_cookie_sock_(ad
> dr|ops) (the functions you mentioned don't
> exist), but the four variants of the helper just have the same name, and
> take different objects for their context.
> 
> Yes! I made a mistake in the function names in the first email. Thank
> you for pointing that out.
> 
> I am an eBPF newbie and I am learning it currently. AFAIK, language C
> doesn't support function overriding via different parameters.
> So how do these four functions co-exist?

User space header file knows only about a single
bpf_get_socket_cookie(), which takes a "void *". See:
https://github.com/libbpf/libbpf/blob/v1.1.0/src/bpf_helper_defs.h#L1154

It compiles into the eBPF insruction "call 46", where 46 is the number
associated to these helpers. When your program loads, the verifier finds
out, depending on program type, what function should be called for that
number (see e.g. tc_cls_act_func_proto() in net/core/filter.c, where
BPF_FUNC_get_socket_cookie (46) returns &bpf_get_socket_cookie_proto,
whereas sock_addr_func_proto(), applied to programs of different types,
will return &bpf_get_socket_cookie_sock_addr_proto instead).

> 
> Some naive search in the kernel code leads me to:
> https://elixir.bootlin.com/linux/v6.2-rc6/source/net/core/filter.c#L4919
> ```
> static const struct bpf_func_proto bpf_get_socket_cookie_sock_addr_proto = {
>     .func        = bpf_get_socket_cookie_sock_addr,
>     .gpl_only    = false,
>     .ret_type    = RET_INTEGER,
>     .arg1_type    = ARG_PTR_TO_CTX,
> };
> ```
> 
> https://elixir.bootlin.com/linux/v6.2-rc6/source/net/core/filter.c#L4955
> ```
> static const struct bpf_func_proto bpf_get_socket_cookie_sock_ops_proto = {
>         .func           = bpf_get_socket_cookie_sock_ops,
>         .gpl_only       = false,
>         .ret_type       = RET_INTEGER,
>         .arg1_type      = ARG_PTR_TO_CTX,
> };
> ```
> 
> It seems that the function definitions are quite real...

Yes, but "bpf_get_socket_cookie_addr" (from your message) !=
"bpf_get_socket_cookie_sock_addr".

Quentin

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

end of thread, other threads:[~2023-01-31 14:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-31 10:03 Typo in the man7 bpf-helpers page Zexuan Luo
2023-01-31 11:00 ` Alejandro Colomar
2023-01-31 11:40   ` Alejandro Colomar
2023-01-31 11:51     ` Alejandro Colomar
2023-01-31 12:02     ` Quentin Monnet
2023-01-31 14:36       ` Zexuan Luo
2023-01-31 14:45         ` Zexuan Luo
2023-01-31 14:51         ` Quentin Monnet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox