diff mbox series

[iproute2-next] Update tc-bpf.8 man page examples

Message ID CA+i7u5oa4aN9yufQJ04MqCpUcW_F-AxzjatZx1Ef-Ad-wEo62g@mail.gmail.com
State Accepted
Delegated to: David Ahern
Headers show
Series [iproute2-next] Update tc-bpf.8 man page examples | expand

Commit Message

Lucas Siba April 20, 2019, 7:06 p.m. UTC
From: Lucas Siba @ 2019-04-20 11:40 UTC
To: netdev

This patch updates the tc-bpf.8 example application for changes to the
struct bpf_elf_map definition. In it's current form, things compile, but
the resulting object file is rejected by the verifier when attempting to
load it through tc.

Signed-off-by: Lucas Siba <lucas.siba@gmail.com>
---
 man/man8/tc-bpf.8 | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

--
In a future patch, it might be worth changing the man page to refer to
"bpf_elf.h" and "uapi/linux/bpf.h" for the definition of structures and
constants, as they may be changed again without the man page being updated.

Comments

David Ahern April 26, 2019, 5:40 p.m. UTC | #1
On 4/20/19 1:06 PM, Lucas Siba wrote:
> From: Lucas Siba @ 2019-04-20 11:40 UTC
> To: netdev
> 
> This patch updates the tc-bpf.8 example application for changes to the
> struct bpf_elf_map definition. In it's current form, things compile, but
> the resulting object file is rejected by the verifier when attempting to
> load it through tc.
> 
> Signed-off-by: Lucas Siba <lucas.siba@gmail.com>
> ---
>  man/man8/tc-bpf.8 | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Daniel: you agree with this change?

> 
> diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
> index b2f9344f..b6cfeab9 100644
> --- a/man/man8/tc-bpf.8
> +++ b/man/man8/tc-bpf.8
> @@ -597,6 +597,8 @@ struct bpf_elf_map __section("maps") map_stats = {
>          .size_key       =       sizeof(uint32_t),
>          .size_value     =       sizeof(struct tuple),
>          .max_elem       =       BPF_MAX_MARK,
> +        .pinning        =       PIN_GLOBAL_NS,
> +        .flags          =       0,
>  };
> 
>  static inline void cls_update_stats(const struct __sk_buff *skb,
> @@ -709,13 +711,22 @@ in both examples was:
>  #define likely(x) __builtin_expect(!!(x), 1)
>  #define unlikely(x) __builtin_expect(!!(x), 0)
> 
> -/* Used map structure */
> +/* Object pinning settings */
> +#define PIN_NONE       0
> +#define PIN_OBJECT_NS  1
> +#define PIN_GLOBAL_NS  2
> +
> +/* ELF map definition */
>  struct bpf_elf_map {
>      __u32 type;
>      __u32 size_key;
>      __u32 size_value;
>      __u32 max_elem;
> +    __u32 flags;
>      __u32 id;
> +    __u32 pinning;
> +    __u32 inner_id;
> +    __u32 inner_idx;
>  };
> 
>  /* Some used BPF function calls. */
> --
> In a future patch, it might be worth changing the man page to refer to
> "bpf_elf.h" and "uapi/linux/bpf.h" for the definition of structures and
> constants, as they may be changed again without the man page being updated.
>
Daniel Borkmann April 26, 2019, 8:34 p.m. UTC | #2
On 04/26/2019 07:40 PM, David Ahern wrote:
> On 4/20/19 1:06 PM, Lucas Siba wrote:
>> From: Lucas Siba @ 2019-04-20 11:40 UTC
>> To: netdev
>>
>> This patch updates the tc-bpf.8 example application for changes to the
>> struct bpf_elf_map definition. In it's current form, things compile, but
>> the resulting object file is rejected by the verifier when attempting to
>> load it through tc.
>>
>> Signed-off-by: Lucas Siba <lucas.siba@gmail.com>
>> ---
>>  man/man8/tc-bpf.8 | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> Daniel: you agree with this change?

Yeah, looks good.

>> diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
>> index b2f9344f..b6cfeab9 100644
>> --- a/man/man8/tc-bpf.8
>> +++ b/man/man8/tc-bpf.8
>> @@ -597,6 +597,8 @@ struct bpf_elf_map __section("maps") map_stats = {
>>          .size_key       =       sizeof(uint32_t),
>>          .size_value     =       sizeof(struct tuple),
>>          .max_elem       =       BPF_MAX_MARK,
>> +        .pinning        =       PIN_GLOBAL_NS,
>> +        .flags          =       0,

If 0 anyway, it doesn't need to be explicitly initialized. David can
probably fix it up while applying.

>>  };
>>
>>  static inline void cls_update_stats(const struct __sk_buff *skb,
>> @@ -709,13 +711,22 @@ in both examples was:
>>  #define likely(x) __builtin_expect(!!(x), 1)
>>  #define unlikely(x) __builtin_expect(!!(x), 0)
>>
>> -/* Used map structure */
>> +/* Object pinning settings */
>> +#define PIN_NONE       0
>> +#define PIN_OBJECT_NS  1
>> +#define PIN_GLOBAL_NS  2
>> +
>> +/* ELF map definition */
>>  struct bpf_elf_map {
>>      __u32 type;
>>      __u32 size_key;
>>      __u32 size_value;
>>      __u32 max_elem;
>> +    __u32 flags;
>>      __u32 id;
>> +    __u32 pinning;
>> +    __u32 inner_id;
>> +    __u32 inner_idx;
>>  };
>>
>>  /* Some used BPF function calls. */
>> --
>> In a future patch, it might be worth changing the man page to refer to
>> "bpf_elf.h" and "uapi/linux/bpf.h" for the definition of structures and
>> constants, as they may be changed again without the man page being updated.

Yep, also tc-bpf.8 is super outdated in general unfortunately. :/

Thanks,
Daniel
David Ahern April 26, 2019, 9:06 p.m. UTC | #3
On 4/26/19 2:34 PM, Daniel Borkmann wrote:
> On 04/26/2019 07:40 PM, David Ahern wrote:
>> On 4/20/19 1:06 PM, Lucas Siba wrote:
>>> From: Lucas Siba @ 2019-04-20 11:40 UTC
>>> To: netdev
>>>
>>> This patch updates the tc-bpf.8 example application for changes to the
>>> struct bpf_elf_map definition. In it's current form, things compile, but
>>> the resulting object file is rejected by the verifier when attempting to
>>> load it through tc.
>>>
>>> Signed-off-by: Lucas Siba <lucas.siba@gmail.com>
>>> ---
>>>  man/man8/tc-bpf.8 | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> Daniel: you agree with this change?
> 
> Yeah, looks good.
> 
>>> diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
>>> index b2f9344f..b6cfeab9 100644
>>> --- a/man/man8/tc-bpf.8
>>> +++ b/man/man8/tc-bpf.8
>>> @@ -597,6 +597,8 @@ struct bpf_elf_map __section("maps") map_stats = {
>>>          .size_key       =       sizeof(uint32_t),
>>>          .size_value     =       sizeof(struct tuple),
>>>          .max_elem       =       BPF_MAX_MARK,
>>> +        .pinning        =       PIN_GLOBAL_NS,
>>> +        .flags          =       0,
> 
> If 0 anyway, it doesn't need to be explicitly initialized. David can
> probably fix it up while applying.
> 

Thanks, Daniel. Removed the flags initialization and applied to
iproute2-next.
diff mbox series

Patch

diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
index b2f9344f..b6cfeab9 100644
--- a/man/man8/tc-bpf.8
+++ b/man/man8/tc-bpf.8
@@ -597,6 +597,8 @@  struct bpf_elf_map __section("maps") map_stats = {
         .size_key       =       sizeof(uint32_t),
         .size_value     =       sizeof(struct tuple),
         .max_elem       =       BPF_MAX_MARK,
+        .pinning        =       PIN_GLOBAL_NS,
+        .flags          =       0,
 };

 static inline void cls_update_stats(const struct __sk_buff *skb,
@@ -709,13 +711,22 @@  in both examples was:
 #define likely(x) __builtin_expect(!!(x), 1)
 #define unlikely(x) __builtin_expect(!!(x), 0)

-/* Used map structure */
+/* Object pinning settings */
+#define PIN_NONE       0
+#define PIN_OBJECT_NS  1
+#define PIN_GLOBAL_NS  2
+
+/* ELF map definition */
 struct bpf_elf_map {
     __u32 type;
     __u32 size_key;
     __u32 size_value;
     __u32 max_elem;
+    __u32 flags;
     __u32 id;
+    __u32 pinning;
+    __u32 inner_id;
+    __u32 inner_idx;
 };

 /* Some used BPF function calls. */