Re: [PATCH bpf-next v3 8/8] bpf: add documentation for eBPF helpers (58-64)

[Jesper Dangaard Brouer]

On Thu, 19 Apr 2018 13:44:41 +0100
Quentin Monnet  wrote:

> 2018-04-18 17:43 UTC+0200 ~ Jesper Dangaard Brouer 
> > On Wed, 18 Apr 2018 15:09:41 +0100
> > Quentin Monnet  wrote:
> >   
> >> 2018-04-18 15:34 UTC+0200 ~ Jesper Dangaard Brouer   
> >>> On Tue, 17 Apr 2018 15:34:38 +0100
> >>> Quentin Monnet  wrote:
> >>> 
>  diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>  index 350459c583de..3d329538498f 100644
>  --- a/include/uapi/linux/bpf.h
>  +++ b/include/uapi/linux/bpf.h
>  @@ -1276,6 +1276,50 @@ union bpf_attr {
>    *  Return
>    *  0 on success, or a negative error in case of failure.
>    *
>  + * int bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
>  + *  Description
>  + *  Redirect the packet to the endpoint referenced by *map* 
>  at
>  + *  index *key*. Depending on its type, his *map* can 
>  contain
> >>> ^^^
> >>>
> >>> "his" -> "this"
> >>
> >> Thanks!
> >>  
>  + *  references to net devices (for forwarding packets 
>  through other
>  + *  ports), or to CPUs (for redirecting XDP frames to 
>  another CPU;
>  + *  but this is only implemented for native XDP (with driver
>  + *  support) as of this writing).
>  + *
>  + *  All values for *flags* are reserved for future usage, 
>  and must
>  + *  be left at zero.
>  + *  Return
>  + *  **XDP_REDIRECT** on success, or **XDP_ABORT** on error.
>  + *
> >>>
> >>> "XDP_ABORT" -> "XDP_ABORTED"
> >>
> >> Ouch. And I did the same for bpf_redirect(). Thanks for the catch.
> >>  
> >>>
> >>> I don't know if it's worth mentioning in the doc/man-page; that for XDP
> >>> using bpf_redirect_map() is a HUGE performance advantage, compared to
> >>> the bpf_redirect() call ?
> >>
> >> It seems worth to me. How would you simply explain the reason for this
> >> difference?  
> > 
> > The basic reason is "bulking effect", as devmap avoids the NIC
> > tailptr/doorbell update on every packet... how to write that in a doc
> > format?
> > 
> > I wrote about why XDP_REDIRECT with maps are smart here:
> >  
> > http://vger.kernel.org/netconf2017_files/XDP_devel_update_NetConf2017_Seoul.pdf
> > 
> > Using maps for redirect, hopefully makes XDP_REDIRECT the last driver
> > XDP action code we need.  As new types of redirect can be introduced
> > without driver changes. See that AF_XDP also uses a map.
> > 
> > It is more subtle, but maps also function as a sorting step. Imagine
> > your XDP program need to redirect out different interfaces (or CPUs in
> > cpumap case), and packets arrive intermixed.  Packets get sorted into
> > the different map indexes, and the xdp_do_flush_map() will trigger the
> > flush operation.
> > 
> > 
> > Happened to have an i40e NIC benchmark setup, and ran a single flow pktgen 
> > test.
> > 
> > Results with 'xdp_redirect_map'
> >  13589297 pps (13,589,297) 
> > 
> > Results with 'xdp_redirect' NOT using devmap:
> >   7567575 pps (7,567,575)
> > 
> > Just to point out the performance benefit of devmap...  
> 
> 
> Thanks for those details! This is an impressive change in performance
> indeed.
> 
> I think I will just keep it simple for the documentation. I will add the
> following for bpf_redirect_map():
> 
> When used to redirect packets to net devices, this helper
> provides a high performance increase over **bpf_redirect**\ ().
> This is due to various implementation details of the underlying
> mechanisms, one of which is the fact that **bpf_redirect_map**\ ()
> tries to send packet as a "bulk" to the device.
> 
> And also append the following to bpf_redirect():
> 
> The same effect can be attained with the more generic
> **bpf_redirect_map**\ (), which requires specific maps
> to be used but offers better performance.

This sounds good to me! :-)

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

Re: [PATCH bpf-next v3 8/8] bpf: add documentation for eBPF helpers (58-64)

[Quentin Monnet]

2018-04-18 17:43 UTC+0200 ~ Jesper Dangaard Brouer 
> On Wed, 18 Apr 2018 15:09:41 +0100
> Quentin Monnet  wrote:
> 
>> 2018-04-18 15:34 UTC+0200 ~ Jesper Dangaard Brouer 
>>> On Tue, 17 Apr 2018 15:34:38 +0100
>>> Quentin Monnet  wrote:
>>>   
 diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
 index 350459c583de..3d329538498f 100644
 --- a/include/uapi/linux/bpf.h
 +++ b/include/uapi/linux/bpf.h
 @@ -1276,6 +1276,50 @@ union bpf_attr {
   *Return
   *0 on success, or a negative error in case of failure.
   *
 + * int bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
 + *Description
 + *Redirect the packet to the endpoint referenced by *map* 
 at
 + *index *key*. Depending on its type, his *map* can 
 contain  
>>> ^^^
>>>
>>> "his" -> "this"  
>>
>> Thanks!
>>
 + *references to net devices (for forwarding packets 
 through other
 + *ports), or to CPUs (for redirecting XDP frames to 
 another CPU;
 + *but this is only implemented for native XDP (with driver
 + *support) as of this writing).
 + *
 + *All values for *flags* are reserved for future usage, 
 and must
 + *be left at zero.
 + *Return
 + ***XDP_REDIRECT** on success, or **XDP_ABORT** on error.
 + *  
>>>
>>> "XDP_ABORT" -> "XDP_ABORTED"  
>>
>> Ouch. And I did the same for bpf_redirect(). Thanks for the catch.
>>
>>>
>>> I don't know if it's worth mentioning in the doc/man-page; that for XDP
>>> using bpf_redirect_map() is a HUGE performance advantage, compared to
>>> the bpf_redirect() call ?  
>>
>> It seems worth to me. How would you simply explain the reason for this
>> difference?
> 
> The basic reason is "bulking effect", as devmap avoids the NIC
> tailptr/doorbell update on every packet... how to write that in a doc
> format?
> 
> I wrote about why XDP_REDIRECT with maps are smart here:
>  
> http://vger.kernel.org/netconf2017_files/XDP_devel_update_NetConf2017_Seoul.pdf
> 
> Using maps for redirect, hopefully makes XDP_REDIRECT the last driver
> XDP action code we need.  As new types of redirect can be introduced
> without driver changes. See that AF_XDP also uses a map.
> 
> It is more subtle, but maps also function as a sorting step. Imagine
> your XDP program need to redirect out different interfaces (or CPUs in
> cpumap case), and packets arrive intermixed.  Packets get sorted into
> the different map indexes, and the xdp_do_flush_map() will trigger the
> flush operation.
> 
> 
> Happened to have an i40e NIC benchmark setup, and ran a single flow pktgen 
> test.
> 
> Results with 'xdp_redirect_map'
>  13589297 pps (13,589,297) 
> 
> Results with 'xdp_redirect' NOT using devmap:
>   7567575 pps (7,567,575)
> 
> Just to point out the performance benefit of devmap...


Thanks for those details! This is an impressive change in performance
indeed.

I think I will just keep it simple for the documentation. I will add the
following for bpf_redirect_map():

When used to redirect packets to net devices, this helper
provides a high performance increase over **bpf_redirect**\ ().
This is due to various implementation details of the underlying
mechanisms, one of which is the fact that **bpf_redirect_map**\ ()
tries to send packet as a "bulk" to the device.

And also append the following to bpf_redirect():

The same effect can be attained with the more generic
**bpf_redirect_map**\ (), which requires specific maps
to be used but offers better performance.

Best,
Quentin

Re: [PATCH bpf-next v3 8/8] bpf: add documentation for eBPF helpers (58-64)

[Jesper Dangaard Brouer]

On Wed, 18 Apr 2018 15:09:41 +0100
Quentin Monnet  wrote:

> 2018-04-18 15:34 UTC+0200 ~ Jesper Dangaard Brouer 
> > On Tue, 17 Apr 2018 15:34:38 +0100
> > Quentin Monnet  wrote:
> >   
> >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> >> index 350459c583de..3d329538498f 100644
> >> --- a/include/uapi/linux/bpf.h
> >> +++ b/include/uapi/linux/bpf.h
> >> @@ -1276,6 +1276,50 @@ union bpf_attr {
> >>   *Return
> >>   *0 on success, or a negative error in case of failure.
> >>   *
> >> + * int bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
> >> + *Description
> >> + *Redirect the packet to the endpoint referenced by *map* 
> >> at
> >> + *index *key*. Depending on its type, his *map* can 
> >> contain  
> > ^^^
> > 
> > "his" -> "this"  
> 
> Thanks!
> 
> >> + *references to net devices (for forwarding packets 
> >> through other
> >> + *ports), or to CPUs (for redirecting XDP frames to 
> >> another CPU;
> >> + *but this is only implemented for native XDP (with driver
> >> + *support) as of this writing).
> >> + *
> >> + *All values for *flags* are reserved for future usage, 
> >> and must
> >> + *be left at zero.
> >> + *Return
> >> + ***XDP_REDIRECT** on success, or **XDP_ABORT** on error.
> >> + *  
> > 
> > "XDP_ABORT" -> "XDP_ABORTED"  
> 
> Ouch. And I did the same for bpf_redirect(). Thanks for the catch.
> 
> > 
> > I don't know if it's worth mentioning in the doc/man-page; that for XDP
> > using bpf_redirect_map() is a HUGE performance advantage, compared to
> > the bpf_redirect() call ?  
> 
> It seems worth to me. How would you simply explain the reason for this
> difference?

The basic reason is "bulking effect", as devmap avoids the NIC
tailptr/doorbell update on every packet... how to write that in a doc
format?

I wrote about why XDP_REDIRECT with maps are smart here:
 http://vger.kernel.org/netconf2017_files/XDP_devel_update_NetConf2017_Seoul.pdf

Using maps for redirect, hopefully makes XDP_REDIRECT the last driver
XDP action code we need.  As new types of redirect can be introduced
without driver changes. See that AF_XDP also uses a map.

It is more subtle, but maps also function as a sorting step. Imagine
your XDP program need to redirect out different interfaces (or CPUs in
cpumap case), and packets arrive intermixed.  Packets get sorted into
the different map indexes, and the xdp_do_flush_map() will trigger the
flush operation.


Happened to have an i40e NIC benchmark setup, and ran a single flow pktgen test.

Results with 'xdp_redirect_map'
 13589297 pps (13,589,297) 

Results with 'xdp_redirect' NOT using devmap:
  7567575 pps (7,567,575)

Just to point out the performance benefit of devmap...

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

Re: [PATCH bpf-next v3 8/8] bpf: add documentation for eBPF helpers (58-64)

[Quentin Monnet]

2018-04-18 15:34 UTC+0200 ~ Jesper Dangaard Brouer 
> On Tue, 17 Apr 2018 15:34:38 +0100
> Quentin Monnet  wrote:
> 
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 350459c583de..3d329538498f 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -1276,6 +1276,50 @@ union bpf_attr {
>>   *  Return
>>   *  0 on success, or a negative error in case of failure.
>>   *
>> + * int bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
>> + *  Description
>> + *  Redirect the packet to the endpoint referenced by *map* at
>> + *  index *key*. Depending on its type, his *map* can contain
> ^^^
> 
> "his" -> "this"

Thanks!

>> + *  references to net devices (for forwarding packets through other
>> + *  ports), or to CPUs (for redirecting XDP frames to another CPU;
>> + *  but this is only implemented for native XDP (with driver
>> + *  support) as of this writing).
>> + *
>> + *  All values for *flags* are reserved for future usage, and must
>> + *  be left at zero.
>> + *  Return
>> + *  **XDP_REDIRECT** on success, or **XDP_ABORT** on error.
>> + *
> 
> "XDP_ABORT" -> "XDP_ABORTED"

Ouch. And I did the same for bpf_redirect(). Thanks for the catch.

> 
> I don't know if it's worth mentioning in the doc/man-page; that for XDP
> using bpf_redirect_map() is a HUGE performance advantage, compared to
> the bpf_redirect() call ?

It seems worth to me. How would you simply explain the reason for this
difference?

Quentin

Re: [PATCH bpf-next v3 8/8] bpf: add documentation for eBPF helpers (58-64)

[Jesper Dangaard Brouer]

On Tue, 17 Apr 2018 15:34:38 +0100
Quentin Monnet  wrote:

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 350459c583de..3d329538498f 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1276,6 +1276,50 @@ union bpf_attr {
>   *   Return
>   *   0 on success, or a negative error in case of failure.
>   *
> + * int bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
> + *   Description
> + *   Redirect the packet to the endpoint referenced by *map* at
> + *   index *key*. Depending on its type, his *map* can contain
^^^

"his" -> "this"

> + *   references to net devices (for forwarding packets through other
> + *   ports), or to CPUs (for redirecting XDP frames to another CPU;
> + *   but this is only implemented for native XDP (with driver
> + *   support) as of this writing).
> + *
> + *   All values for *flags* are reserved for future usage, and must
> + *   be left at zero.
> + *   Return
> + *   **XDP_REDIRECT** on success, or **XDP_ABORT** on error.
> + *

"XDP_ABORT" -> "XDP_ABORTED"

I don't know if it's worth mentioning in the doc/man-page; that for XDP
using bpf_redirect_map() is a HUGE performance advantage, compared to
the bpf_redirect() call ?

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

[PATCH bpf-next v3 8/8] bpf: add documentation for eBPF helpers (58-64)

[Quentin Monnet]

Add documentation for eBPF helper functions to bpf.h user header file.
This documentation can be parsed with the Python script provided in
another commit of the patch series, in order to provide a RST document
that can later be converted into a man page.

The objective is to make the documentation easily understandable and
accessible to all eBPF developers, including beginners.

This patch contains descriptions for the following helper functions, all
written by John:

- bpf_redirect_map()
- bpf_sk_redirect_map()
- bpf_sock_map_update()
- bpf_msg_redirect_map()
- bpf_msg_apply_bytes()
- bpf_msg_cork_bytes()
- bpf_msg_pull_data()

v3:
- bpf_sk_redirect_map(): Improve description of BPF_F_INGRESS flag.
- bpf_msg_redirect_map(): Improve description of BPF_F_INGRESS flag.
- bpf_redirect_map(): Fix note on CPU redirection, not fully implemented
  for generic XDP but supported on native XDP.

Cc: Jesper Dangaard Brouer 
Cc: John Fastabend 
Signed-off-by: Quentin Monnet 
---
 include/uapi/linux/bpf.h | 140 +++
 1 file changed, 140 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 350459c583de..3d329538498f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1276,6 +1276,50 @@ union bpf_attr {
  * Return
  * 0 on success, or a negative error in case of failure.
  *
+ * int bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
+ * Description
+ * Redirect the packet to the endpoint referenced by *map* at
+ * index *key*. Depending on its type, his *map* can contain
+ * references to net devices (for forwarding packets through other
+ * ports), or to CPUs (for redirecting XDP frames to another CPU;
+ * but this is only implemented for native XDP (with driver
+ * support) as of this writing).
+ *
+ * All values for *flags* are reserved for future usage, and must
+ * be left at zero.
+ * Return
+ * **XDP_REDIRECT** on success, or **XDP_ABORT** on error.
+ *
+ * int bpf_sk_redirect_map(struct bpf_map *map, u32 key, u64 flags)
+ * Description
+ * Redirect the packet to the socket referenced by *map* (of type
+ * **BPF_MAP_TYPE_SOCKMAP**) at index *key*. Both ingress and
+ * egress interfaces can be used for redirection. The
+ * **BPF_F_INGRESS** value in *flags* is used to make the
+ * distinction (ingress path is selected if the flag is present,
+ * egress path otherwise). This is the only flag supported for now.
+ * Return
+ * **SK_PASS** on success, or **SK_DROP** on error.
+ *
+ * int bpf_sock_map_update(struct bpf_sock_ops_kern *skops, struct bpf_map 
*map, void *key, u64 flags)
+ * Description
+ * Add an entry to, or update a *map* referencing sockets. The
+ * *skops* is used as a new value for the entry associated to
+ * *key*. *flags* is one of:
+ *
+ * **BPF_NOEXIST**
+ * The entry for *key* must not exist in the map.
+ * **BPF_EXIST**
+ * The entry for *key* must already exist in the map.
+ * **BPF_ANY**
+ * No condition on the existence of the entry for *key*.
+ *
+ * If the *map* has eBPF programs (parser and verdict), those will
+ * be inherited by the socket being added. If the socket is
+ * already attached to eBPF programs, this results in an error.
+ * Return
+ * 0 on success, or a negative error in case of failure.
+ *
  * int bpf_xdp_adjust_meta(struct xdp_buff *xdp_md, int delta)
  * Description
  * Adjust the address pointed by *xdp_md*\ **->data_meta** by
@@ -1443,6 +1487,102 @@ union bpf_attr {
  * be set is returned (which comes down to 0 if all bits were set
  * as required).
  *
+ * int bpf_msg_redirect_map(struct sk_msg_buff *msg, struct bpf_map *map, u32 
key, u64 flags)
+ * Description
+ * This helper is used in programs implementing policies at the
+ * socket level. If the message *msg* is allowed to pass (i.e. if
+ * the verdict eBPF program returns **SK_PASS**), redirect it to
+ * the socket referenced by *map* (of type
+ * **BPF_MAP_TYPE_SOCKMAP**) at index *key*. Both ingress and
+ * egress interfaces can be used for redirection. The
+ * **BPF_F_INGRESS** value in *flags* is used to make the
+ * distinction (ingress path is selected if the flag is present,
+ * egress path otherwise). This is the only flag supported for now.
+ * Return
+ * **SK_PASS** on success, or **SK_DROP** on error.
+ *
+ * int bpf_msg_apply_bytes(struct sk_msg_buff