Re: [PATCH bpf-next v3 4/8] bpf: add documentation for eBPF helpers (23-32)

[Daniel Borkmann]

On 04/20/2018 08:54 PM, Quentin Monnet wrote:
> 2018-04-19 13:16 UTC+0200 ~ Daniel Borkmann 
>> On 04/17/2018 04:34 PM, Quentin Monnet wrote:
>>> 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 Daniel:
>>>
>>> - bpf_get_prandom_u32()
>>> - bpf_get_smp_processor_id()
>>> - bpf_get_cgroup_classid()
>>> - bpf_get_route_realm()
>>> - bpf_skb_load_bytes()
>>> - bpf_csum_diff()
>>> - bpf_skb_get_tunnel_opt()
>>> - bpf_skb_set_tunnel_opt()
>>> - bpf_skb_change_proto()
>>> - bpf_skb_change_type()
>>>
>>> v3:
>>> - bpf_get_prandom_u32(): Fix helper name :(. Add description, including
>>>   a note on the internal random state.
>>> - bpf_get_smp_processor_id(): Add description, including a note on the
>>>   processor id remaining stable during program run.
>>> - bpf_get_cgroup_classid(): State that CONFIG_CGROUP_NET_CLASSID is
>>>   required to use the helper. Add a reference to related documentation.
>>>   State that placing a task in net_cls controller disables cgroup-bpf.
>>> - bpf_get_route_realm(): State that CONFIG_CGROUP_NET_CLASSID is
>>>   required to use this helper.
>>> - bpf_skb_load_bytes(): Fix comment on current use cases for the helper.
>>>
>>> Cc: Daniel Borkmann 
>>> Signed-off-by: Quentin Monnet 
>>> ---
>>>  include/uapi/linux/bpf.h | 152 
>>> +++
>>>  1 file changed, 152 insertions(+)
>>>
>>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>>> index c59bf5b28164..d748f65a8f58 100644
>>> --- a/include/uapi/linux/bpf.h
>>> +++ b/include/uapi/linux/bpf.h
> 
> [...]
> 
>>> @@ -615,6 +632,27 @@ union bpf_attr {
>>>   * Return
>>>   * 0 on success, or a negative error in case of failure.
>>>   *
>>> + * u32 bpf_get_cgroup_classid(struct sk_buff *skb)
>>> + * Description
>>> + * Retrieve the classid for the current task, i.e. for the
>>> + * net_cls (network classifier) cgroup to which *skb* 
>>> belongs.
>>> + *
>>> + * This helper is only available is the kernel was 
>>> compiled with
>>> + * the **CONFIG_CGROUP_NET_CLASSID** configuration option 
>>> set to
>>> + * "**y**" or to "**m**".
>>> + *
>>> + * Note that placing a task into the net_cls controller 
>>> completely
>>> + * disables the execution of eBPF programs with the cgroup.
>>
>> I'm not sure I follow the above sentence, what do you mean by that?
> 
> Please see comment below.
> 
>> I would definitely also add here that this helper is limited to cgroups v1
>> only, and that it works on clsact TC egress hook but not the ingress one.
>>
>>> + * Also note that, in the above description, the "network
>>> + * classifier" cgroup does not designate a generic 
>>> classifier, but
>>> + * a particular mechanism that provides an interface to tag
>>> + * network packets with a specific class identifier. See 
>>> also the
>>
>> The "generic classifier" part is a bit strange to parse. I would probably
>> leave the first part out and explain that this provides a means to tag
>> packets based on a user-provided ID for all traffic coming from the tasks
>> belonging to the related cgroup.
> 
> In this paragraph and in the one above, I am trying to address Alexei
> comments to the RFC v2:
> 
> please add that kernel should be configured with
> CONFIG_NET_CLS_CGROUP=y|m
> and mention Documentation/cgroup-v1/net_cls.txt
> Otherwise 'network classifier' is way too generic.
> I'd also mention that placing a task into net_cls controller
> disables all of cgroup-bpf.
> 
> But I must admit I am struggling a bit on this helper. If I understand
> correctly, "network classifier" is too generic and could be confused
> with TC classifiers? Hence the attempt to avoid that in the second

I think if you just name it "net_cls cgroup" it should be good enough in case
the concern is that "network classifier" name would be misunderstood.

> paragraph... As for "placing a task into net_cls controller disables all
> of cgroup-bpf", again if I understand correctly, I think it refers to
> cgroup_sk_alloc_disable() being called in write_classid() in file
> net/core/netclassid_cgroup.c. But I am not familiar with it and cannot
> find a nice way to explain that in the doc :/.

Point here should be that there's cgroups v1 and v2 infra. Both are available
and you can use a mixture 

Re: [PATCH bpf-next v3 4/8] bpf: add documentation for eBPF helpers (23-32)

[Quentin Monnet]

2018-04-19 13:16 UTC+0200 ~ Daniel Borkmann 
> On 04/17/2018 04:34 PM, Quentin Monnet wrote:
>> 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 Daniel:
>>
>> - bpf_get_prandom_u32()
>> - bpf_get_smp_processor_id()
>> - bpf_get_cgroup_classid()
>> - bpf_get_route_realm()
>> - bpf_skb_load_bytes()
>> - bpf_csum_diff()
>> - bpf_skb_get_tunnel_opt()
>> - bpf_skb_set_tunnel_opt()
>> - bpf_skb_change_proto()
>> - bpf_skb_change_type()
>>
>> v3:
>> - bpf_get_prandom_u32(): Fix helper name :(. Add description, including
>>   a note on the internal random state.
>> - bpf_get_smp_processor_id(): Add description, including a note on the
>>   processor id remaining stable during program run.
>> - bpf_get_cgroup_classid(): State that CONFIG_CGROUP_NET_CLASSID is
>>   required to use the helper. Add a reference to related documentation.
>>   State that placing a task in net_cls controller disables cgroup-bpf.
>> - bpf_get_route_realm(): State that CONFIG_CGROUP_NET_CLASSID is
>>   required to use this helper.
>> - bpf_skb_load_bytes(): Fix comment on current use cases for the helper.
>>
>> Cc: Daniel Borkmann 
>> Signed-off-by: Quentin Monnet 
>> ---
>>  include/uapi/linux/bpf.h | 152 
>> +++
>>  1 file changed, 152 insertions(+)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index c59bf5b28164..d748f65a8f58 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h

[...]

>> @@ -615,6 +632,27 @@ union bpf_attr {
>>   *  Return
>>   *  0 on success, or a negative error in case of failure.
>>   *
>> + * u32 bpf_get_cgroup_classid(struct sk_buff *skb)
>> + *  Description
>> + *  Retrieve the classid for the current task, i.e. for the
>> + *  net_cls (network classifier) cgroup to which *skb* belongs.
>> + *
>> + *  This helper is only available is the kernel was compiled with
>> + *  the **CONFIG_CGROUP_NET_CLASSID** configuration option set to
>> + *  "**y**" or to "**m**".
>> + *
>> + *  Note that placing a task into the net_cls controller completely
>> + *  disables the execution of eBPF programs with the cgroup.
> 
> I'm not sure I follow the above sentence, what do you mean by that?

Please see comment below.

> I would definitely also add here that this helper is limited to cgroups v1
> only, and that it works on clsact TC egress hook but not the ingress one.
> 
>> + *  Also note that, in the above description, the "network
>> + *  classifier" cgroup does not designate a generic classifier, but
>> + *  a particular mechanism that provides an interface to tag
>> + *  network packets with a specific class identifier. See also the
> 
> The "generic classifier" part is a bit strange to parse. I would probably
> leave the first part out and explain that this provides a means to tag
> packets based on a user-provided ID for all traffic coming from the tasks
> belonging to the related cgroup.

In this paragraph and in the one above, I am trying to address Alexei
comments to the RFC v2:

please add that kernel should be configured with
CONFIG_NET_CLS_CGROUP=y|m
and mention Documentation/cgroup-v1/net_cls.txt
Otherwise 'network classifier' is way too generic.
I'd also mention that placing a task into net_cls controller
disables all of cgroup-bpf.

But I must admit I am struggling a bit on this helper. If I understand
correctly, "network classifier" is too generic and could be confused
with TC classifiers? Hence the attempt to avoid that in the second
paragraph... As for "placing a task into net_cls controller disables all
of cgroup-bpf", again if I understand correctly, I think it refers to
cgroup_sk_alloc_disable() being called in write_classid() in file
net/core/netclassid_cgroup.c. But I am not familiar with it and cannot
find a nice way to explain that in the doc :/.

>> + *  related kernel documentation, available from the Linux sources
>> + *  in file *Documentation/cgroup-v1/net_cls.txt*.
>> + *  Return
>> + *  The classid, or 0 for the default unconfigured classid.
>> + *
>>   * int bpf_skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 
>> vlan_tci)
>>   *  Description
>>   *  Push a *vlan_tci* (VLAN tag control information) of protocol
>> @@ -734,6 +772,16 @@ union bpf_attr {
>>   *  are **TC_ACT_REDIRECT** on success or **TC_ACT_SHOT** on
>>   *  

Re: [PATCH bpf-next v3 4/8] bpf: add documentation for eBPF helpers (23-32)

[Daniel Borkmann]

On 04/17/2018 04:34 PM, Quentin Monnet wrote:
> 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 Daniel:
> 
> - bpf_get_prandom_u32()
> - bpf_get_smp_processor_id()
> - bpf_get_cgroup_classid()
> - bpf_get_route_realm()
> - bpf_skb_load_bytes()
> - bpf_csum_diff()
> - bpf_skb_get_tunnel_opt()
> - bpf_skb_set_tunnel_opt()
> - bpf_skb_change_proto()
> - bpf_skb_change_type()
> 
> v3:
> - bpf_get_prandom_u32(): Fix helper name :(. Add description, including
>   a note on the internal random state.
> - bpf_get_smp_processor_id(): Add description, including a note on the
>   processor id remaining stable during program run.
> - bpf_get_cgroup_classid(): State that CONFIG_CGROUP_NET_CLASSID is
>   required to use the helper. Add a reference to related documentation.
>   State that placing a task in net_cls controller disables cgroup-bpf.
> - bpf_get_route_realm(): State that CONFIG_CGROUP_NET_CLASSID is
>   required to use this helper.
> - bpf_skb_load_bytes(): Fix comment on current use cases for the helper.
> 
> Cc: Daniel Borkmann 
> Signed-off-by: Quentin Monnet 
> ---
>  include/uapi/linux/bpf.h | 152 
> +++
>  1 file changed, 152 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c59bf5b28164..d748f65a8f58 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -483,6 +483,23 @@ union bpf_attr {
>   *   The number of bytes written to the buffer, or a negative error
>   *   in case of failure.
>   *
> + * u32 bpf_get_prandom_u32(void)
> + *   Description
> + *   Get a pseudo-random number. Note that this helper uses its own
> + *   pseudo-random internal state, and cannot be used to infer the
> + *   seed of other random functions in the kernel.

We should still add that this prng is not cryptographically secure.

> + *   Return
> + *   A random 32-bit unsigned value.
> + *
> + * u32 bpf_get_smp_processor_id(void)
> + *   Description
> + *   Get the SMP (Symmetric multiprocessing) processor id. Note that

Nit: s/Symmetric/symmetric/ ?

> + *   all programs run with preemption disabled, which means that the
> + *   SMP processor id is stable during all the execution of the
> + *   program.
> + *   Return
> + *   The SMP id of the processor running the program.
> + *
>   * int bpf_skb_store_bytes(struct sk_buff *skb, u32 offset, const void 
> *from, u32 len, u64 flags)
>   *   Description
>   *   Store *len* bytes from address *from* into the packet
> @@ -615,6 +632,27 @@ union bpf_attr {
>   *   Return
>   *   0 on success, or a negative error in case of failure.
>   *
> + * u32 bpf_get_cgroup_classid(struct sk_buff *skb)
> + *   Description
> + *   Retrieve the classid for the current task, i.e. for the
> + *   net_cls (network classifier) cgroup to which *skb* belongs.
> + *
> + *   This helper is only available is the kernel was compiled with
> + *   the **CONFIG_CGROUP_NET_CLASSID** configuration option set to
> + *   "**y**" or to "**m**".
> + *
> + *   Note that placing a task into the net_cls controller completely
> + *   disables the execution of eBPF programs with the cgroup.

I'm not sure I follow the above sentence, what do you mean by that?

I would definitely also add here that this helper is limited to cgroups v1
only, and that it works on clsact TC egress hook but not the ingress one.

> + *   Also note that, in the above description, the "network
> + *   classifier" cgroup does not designate a generic classifier, but
> + *   a particular mechanism that provides an interface to tag
> + *   network packets with a specific class identifier. See also the

The "generic classifier" part is a bit strange to parse. I would probably
leave the first part out and explain that this provides a means to tag
packets based on a user-provided ID for all traffic coming from the tasks
belonging to the related cgroup.

> + *   related kernel documentation, available from the Linux sources
> + *   in file *Documentation/cgroup-v1/net_cls.txt*.
> + *   Return
> + *   The classid, or 0 for the default unconfigured classid.
> + *
>   * int bpf_skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 
> vlan_tci)
>   *   Description
>   *   Push a *vlan_tci* (VLAN tag control information) of protocol
> @@ -734,6 

Re: [PATCH bpf-next v3 4/8] bpf: add documentation for eBPF helpers (23-32)

[Alexei Starovoitov]

On Tue, Apr 17, 2018 at 03:34:34PM +0100, Quentin Monnet wrote:
> 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 Daniel:
> 
> - bpf_get_prandom_u32()
> - bpf_get_smp_processor_id()
> - bpf_get_cgroup_classid()
> - bpf_get_route_realm()
> - bpf_skb_load_bytes()
> - bpf_csum_diff()
> - bpf_skb_get_tunnel_opt()
> - bpf_skb_set_tunnel_opt()
> - bpf_skb_change_proto()
> - bpf_skb_change_type()
> 
> v3:
> - bpf_get_prandom_u32(): Fix helper name :(. Add description, including
>   a note on the internal random state.
> - bpf_get_smp_processor_id(): Add description, including a note on the
>   processor id remaining stable during program run.
> - bpf_get_cgroup_classid(): State that CONFIG_CGROUP_NET_CLASSID is
>   required to use the helper. Add a reference to related documentation.
>   State that placing a task in net_cls controller disables cgroup-bpf.
> - bpf_get_route_realm(): State that CONFIG_CGROUP_NET_CLASSID is
>   required to use this helper.
> - bpf_skb_load_bytes(): Fix comment on current use cases for the helper.
> 
> Cc: Daniel Borkmann 
> Signed-off-by: Quentin Monnet 

Acked-by: Alexei Starovoitov 

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH bpf-next v3 4/8] bpf: add documentation for eBPF helpers (23-32)

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

- bpf_get_prandom_u32()
- bpf_get_smp_processor_id()
- bpf_get_cgroup_classid()
- bpf_get_route_realm()
- bpf_skb_load_bytes()
- bpf_csum_diff()
- bpf_skb_get_tunnel_opt()
- bpf_skb_set_tunnel_opt()
- bpf_skb_change_proto()
- bpf_skb_change_type()

v3:
- bpf_get_prandom_u32(): Fix helper name :(. Add description, including
  a note on the internal random state.
- bpf_get_smp_processor_id(): Add description, including a note on the
  processor id remaining stable during program run.
- bpf_get_cgroup_classid(): State that CONFIG_CGROUP_NET_CLASSID is
  required to use the helper. Add a reference to related documentation.
  State that placing a task in net_cls controller disables cgroup-bpf.
- bpf_get_route_realm(): State that CONFIG_CGROUP_NET_CLASSID is
  required to use this helper.
- bpf_skb_load_bytes(): Fix comment on current use cases for the helper.

Cc: Daniel Borkmann 
Signed-off-by: Quentin Monnet 
---
 include/uapi/linux/bpf.h | 152 +++
 1 file changed, 152 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c59bf5b28164..d748f65a8f58 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -483,6 +483,23 @@ union bpf_attr {
  * The number of bytes written to the buffer, or a negative error
  * in case of failure.
  *
+ * u32 bpf_get_prandom_u32(void)
+ * Description
+ * Get a pseudo-random number. Note that this helper uses its own
+ * pseudo-random internal state, and cannot be used to infer the
+ * seed of other random functions in the kernel.
+ * Return
+ * A random 32-bit unsigned value.
+ *
+ * u32 bpf_get_smp_processor_id(void)
+ * Description
+ * Get the SMP (Symmetric multiprocessing) processor id. Note that
+ * all programs run with preemption disabled, which means that the
+ * SMP processor id is stable during all the execution of the
+ * program.
+ * Return
+ * The SMP id of the processor running the program.
+ *
  * int bpf_skb_store_bytes(struct sk_buff *skb, u32 offset, const void *from, 
u32 len, u64 flags)
  * Description
  * Store *len* bytes from address *from* into the packet
@@ -615,6 +632,27 @@ union bpf_attr {
  * Return
  * 0 on success, or a negative error in case of failure.
  *
+ * u32 bpf_get_cgroup_classid(struct sk_buff *skb)
+ * Description
+ * Retrieve the classid for the current task, i.e. for the
+ * net_cls (network classifier) cgroup to which *skb* belongs.
+ *
+ * This helper is only available is the kernel was compiled with
+ * the **CONFIG_CGROUP_NET_CLASSID** configuration option set to
+ * "**y**" or to "**m**".
+ *
+ * Note that placing a task into the net_cls controller completely
+ * disables the execution of eBPF programs with the cgroup.
+ *
+ * Also note that, in the above description, the "network
+ * classifier" cgroup does not designate a generic classifier, but
+ * a particular mechanism that provides an interface to tag
+ * network packets with a specific class identifier. See also the
+ * related kernel documentation, available from the Linux sources
+ * in file *Documentation/cgroup-v1/net_cls.txt*.
+ * Return
+ * The classid, or 0 for the default unconfigured classid.
+ *
  * int bpf_skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
  * Description
  * Push a *vlan_tci* (VLAN tag control information) of protocol
@@ -734,6 +772,16 @@ union bpf_attr {
  * are **TC_ACT_REDIRECT** on success or **TC_ACT_SHOT** on
  * error.
  *
+ * u32 bpf_get_route_realm(struct sk_buff *skb)
+ * Description
+ * Retrieve the realm or the route, that is to say the
+ * **tclassid** field of the destination for the *skb*. This
+ * helper is available only if the kernel was compiled with
+ * **CONFIG_IP_ROUTE_CLASSID** configuration option.
+ * Return
+ * The realm of the route for the packet associated to *sdb*, or 0
+ * if none was found.
+ *
  * int bpf_perf_event_output(struct pt_reg *ctx, struct bpf_map *map, u64 
flags, void *data, u64 size)
  * Description
  *