Re: [PATCH v9 net-next 1/3] filter: add Extended BPF interpreter and converter

[Alexei Starovoitov]

On Wed, Mar 12, 2014 at 12:22 PM, David Miller <davem@xxxxxxxxxxxxx> wrote:
> From: Alexei Starovoitov <ast@xxxxxxxxxxxx>
> Date: Mon, 10 Mar 2014 21:41:30 -0700
>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 1a869488b8ae..2c13d000389c 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -3054,6 +3054,7 @@ extern int              netdev_max_backlog;
>>  extern int           netdev_tstamp_prequeue;
>>  extern int           weight_p;
>>  extern int           bpf_jit_enable;
>> +extern int           bpf_ext_enable;
>
> I don't like seeing config ifdefs in C code, so please adjust the
> bpf_jit_enable declaration with something like:
>
> #ifdef CONFIG_BPF_JIT
> extern int              bpf_jit_enable;
> #else
> #define bpf_jit_enable  0
> #endif

agree. that's definitely cleaner.

>> +#ifdef CONFIG_64BIT
>> +             DL(BPF_STX, BPF_XADD, BPF_DW)
>> +#endif
>  ...
>> +#ifdef CONFIG_64BIT
>> +BPF_STX_BPF_XADD_BPF_DW: /* lock xadd *(u64 *)(A + insn->off) += X */
>> +     atomic64_add((u64)X, (atomic64_t *)(ulong)(A + insn->off));
>> +     CONT;
>> +#endif
>
> I'm not too happy about instructions which are only available on
> certain architectures.
>
> I'd rather see best effort emulation than wholesale lack of support.
>
> We have an implementation of 64-bit atomics for 32-bit systems or
> those without true atomic compare-and-saw etc. ops in lib/atomic64.c
> so the routines are always available.

Good point. Will do.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/