From: Alexei Starovoitov <ast@plumgrid•com>
To: Daniel Borkmann <daniel@iogearbox•net>, 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: Tue, 31 Mar 2015 22:16:49 -0700 [thread overview]
Message-ID: <551B7F41.3080203@plumgrid.com> (raw)
In-Reply-To: <d9a577418790f99130c1cb20dc01cf50c2d6a7da.1427754287.git.daniel@iogearbox.net>
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 :)
> +__section("action-rand") int act_rand_main(struct __sk_buff *skb)
> +{
> + /* Sorry, we're near event horizon ... */
> + if ((get_prandom_u32() & 3) == 0) {
> + act_update_drop_map();
> + return TC_ACT_SHOT;
> + }
didn't get the joke. Is it April 1st yet? :)
> +/* 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?
> +#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?
> -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.
Acked-by: Alexei Starovoitov <ast@plumgrid•com>
Great work! Thanks a lot!
next prev parent reply other threads:[~2015-04-01 5:16 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 [this message]
2015-04-01 8:48 ` Daniel Borkmann
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=551B7F41.3080203@plumgrid.com \
--to=ast@plumgrid$(echo .)com \
--cc=daniel@iogearbox$(echo .)net \
--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