public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox•net>
To: Alexei Starovoitov <ast@plumgrid•com>, stephen@networkplumber•org
Cc: jiri@resnulli•us, tgraf@suug•ch, netdev@vger•kernel.org,
	Jamal Hadi Salim <jhs@mojatatu•com>
Subject: Re: [PATCH iproute2 -next] tc, bpf: finalize eBPF support for cls and act front-end
Date: Wed, 01 Apr 2015 10:48:31 +0200	[thread overview]
Message-ID: <551BB0DF.2050600@iogearbox.net> (raw)
In-Reply-To: <551B7F41.3080203@plumgrid.com>

On 04/01/2015 07:16 AM, Alexei Starovoitov wrote:
> On 3/30/15 3:35 PM, Daniel Borkmann wrote:
>> +static inline void act_update_drop_map(void)
>> +{
>> +    uint32_t *count, cpu = get_smp_processor_id();
>> +
>> +    count = bpf_map_lookup_elem(&map_drops, &cpu);
>> +    if (count)
>> +        __sync_fetch_and_add(count, 1);
>
> since this function demonstrates poor man's per-cpu counters we can use
> regular *count++ instead of atomic.
> Just picking nits :)

Okay, that's fine.

[...]
>> +/* tc ELF map ABI */
>> +struct bpf_elf_map {
>> +    uint32_t type;
>> +    uint32_t id;
>> +    uint32_t size_key;
>> +    uint32_t size_value;
>> +    uint32_t max_elem;
>> +};
>
> can you change 'id' being last, so that TC+bpf programs from
> samples/bpf/ that don't have 'id' concept can also be loaded?

Will do.

>> +#ifndef min
>> +# define min(x, y) ({            \
>> +    typeof(x) _min1 = (x);        \
>> +    typeof(y) _min2 = (y);        \
>> +    (void) (&_min1 == &_min2);    \
>> +    _min1 < _min2 ? _min1 : _min2; })
>> +#endif
> ...
>> +#ifndef min
>> +# define min(x, y) ({            \
>> +    typeof(x) _min1 = (x);        \
>> +    typeof(y) _min2 = (y);        \
>> +    (void) (&_min1 == &_min2);    \
>> +    _min1 < _min2 ? _min1 : _min2; })
>> +#endif
>
> two 'min' macros in different files?

Yes, I actually wanted to have a stand alone C file, but okay,
I can see to include from utils.h.

>> -static inline int bpf_open_object(const char *path, enum bpf_prog_type type)
>> +static inline int bpf_open_object(const char *path, enum bpf_prog_type type,
>> +                  const char *sec)
>>   {
>>       errno = ENOSYS;
>>       return -1;
>>   }
>
> while playing with it, I hit the ENOSYS first, since my user space
> is quite old. Can you add a human readable printf message here and other
> places, so we don't have to recompile tc with -g and run gdb on it?
> I think it will help distributions to configure dependencies of iproute2
> as well.

Will do and respin.

> Acked-by: Alexei Starovoitov <ast@plumgrid•com>
>
> Great work! Thanks a lot!

Thanks, Alexei!

  reply	other threads:[~2015-04-01  8:48 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-30 22:35 [PATCH iproute2 -next] tc, bpf: finalize eBPF support for cls and act front-end Daniel Borkmann
2015-04-01  5:16 ` Alexei Starovoitov
2015-04-01  8:48   ` Daniel Borkmann [this message]
2015-04-01 12:36 ` Jamal Hadi Salim
2015-04-01 14:13   ` Daniel Borkmann
2015-04-01 22:30     ` Thomas Graf
2015-04-08 11:58       ` Jamal Hadi Salim
2015-04-02  0:13 ` Hannes Frederic Sowa
2015-04-02  0:24   ` Daniel Borkmann
2015-04-02  0:29     ` Hannes Frederic Sowa
2015-04-02 10:19       ` Daniel Borkmann
2015-04-02 11:30         ` Hannes Frederic Sowa
2015-04-02 12:08           ` Daniel Borkmann
2015-04-02 16:14             ` Alexei Starovoitov
2015-04-02 18:38               ` Daniel Borkmann
2015-04-02 12:10           ` Thomas Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=551BB0DF.2050600@iogearbox.net \
    --to=daniel@iogearbox$(echo .)net \
    --cc=ast@plumgrid$(echo .)com \
    --cc=jhs@mojatatu$(echo .)com \
    --cc=jiri@resnulli$(echo .)us \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=stephen@networkplumber$(echo .)org \
    --cc=tgraf@suug$(echo .)ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox