Re: [PATCH bpf-next v7 1/1] bpf, docs: document BPF_MAP_TYPE_ARRAY

[Donald Hunter <donald.hunter@gmail.com>]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Fri, Oct 7, 2022 at 9:39 AM Donald Hunter <donald.hunter@gmail.com> wrote:
>
> Actually all values are rounded up to 8.
> Maybe we should say that all array elements are aligned to 8
> instead of values are rounded?
> Because values_size=4 stays as 4 from bpf prog pov.
> The progs cannot access bytes 5,6,7,8 though that memory is consumed.

Agreed, I will reword to mention alignment instead of rounding.

>> +When using a ``BPF_MAP_TYPE_PERCPU_ARRAY`` the ``bpf_map_update_elem()`` and
>> +``bpf_map_lookup_elem()`` helpers automatically access the hash slot for the
>> +current CPU.
>
> hash slot?
> the copy paste went wrong? :)

Good catch.

>> +    int bpf_prog(struct __sk_buff *skb)
>> +    {
>> +            int index = load_byte(skb,
>> +                                  ETH_HLEN + offsetof(struct iphdr, protocol));
>> +            long *value;
>
> Please avoid using deprecated instructions like load_byte in examples.

Will rewrite to use bpf_skb_load_bytes.

>> +    int create_array() {
>> +            int fd;
>> +            LIBBPF_OPTS(bpf_map_create_opts, opts, .map_flags = BPF_F_MMAPABLE);
>
> Add empty line pls.

Ack.

>> +    int lookup(int fd) {
>> +            __u32 index = 42;
>> +            long value;
>> +            int ret = bpf_map_lookup_elem(fd, &index, &value);
>
> Empty line pls.
> Or better yet do 'int ret;'
> and ret = bpf_map... on a separate line.

Ack.

>> +    int lookup(int fd) {
>> +            int ncpus = libbpf_num_possible_cpus();
>> +            __u32 index = 42, j;
>> +            long values[ncpus];
>> +            int ret = bpf_map_lookup_elem(fd, &index, &values);
>
> same here.

Ack. Thansk for the review!