Re: [Qemu-devel] [RFC 6/6] virtio-net: rss: Add bpf filter

[Sameeh Jubran]

On Mon, Sep 3, 2018 at 2:54 PM, Daniel P. Berrangé <address@hidden> wrote:
> On Thu, Aug 30, 2018 at 05:27:08PM +0300, Sameeh Jubran wrote:
>> From: Sameeh Jubran <address@hidden>
>>
>> Signed-off-by: Sameeh Jubran <address@hidden>
>> ---
>>  hw/net/rss_bpf_insns.h       | 3992 
>> ++++++++++++++++++++++++++++++++++++++++++
>>  hw/net/rss_tap_bpf.h         |   40 +
>>  hw/net/rss_tap_bpf_program.c |  175 ++
>>  hw/net/virtio-net.c          |   99 +-
>>  4 files changed, 4305 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/net/rss_bpf_insns.h
>>  create mode 100644 hw/net/rss_tap_bpf.h
>>  create mode 100644 hw/net/rss_tap_bpf_program.c
>>
>> diff --git a/hw/net/rss_bpf_insns.h b/hw/net/rss_bpf_insns.h
>> new file mode 100644
>> index 0000000000..1a92110b8d
>> --- /dev/null
>> +++ b/hw/net/rss_bpf_insns.h
>> @@ -0,0 +1,3992 @@
>> +/*
>> + * RSS ebpf instructions for virtio-net
>> + *
>> + * Copyright (c) 2018 RedHat.
>
> Why copyright RedHat ?
>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include <linux/bpf.h>
>> +
>> +#ifndef BPF_RSS_INSNS
>> +#define BPF_RSS_INSNS
>> +
>> +/* bpf_insn array matching l3_l4 section. see tap_bpf_program.c file */
>> +struct bpf_insn l3_l4_hash_insns[] = {
>> +{0xbf , 0x6 , 0x1 , 0x0000 , 0x00000000},
>> +{0x28 , 0x0 , 0x0 , 0x0000 , 0x0000000c},
>> +{0xbf , 0x8 , 0x0 , 0x0000 , 0x00000000},
>
> [snip]
>
>> +};
>
> This massive array is presumably an auto-generated content.
>
> We shouldn't be storing this in GIT. We need to store the
> original preferred source format, and providing makefile
> rules to generate it.
>
>
>> +
>> +#endif
>> diff --git a/hw/net/rss_tap_bpf.h b/hw/net/rss_tap_bpf.h
>> new file mode 100644
>> index 0000000000..54b88cfb76
>> --- /dev/null
>> +++ b/hw/net/rss_tap_bpf.h
>> @@ -0,0 +1,40 @@
>> +/*
>> + * RSS ebpf header for virtio-net
>> + *
>> + * Copyright (c) 2018 RedHat.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + * This code is heavily based on the following bpf code from dpdk
>> + * https://git.dpdk.org/dpdk/tree/drivers/net/tap/
>
> There are alot of files in this directory, with varying different
> copyright claims on them, while you're claiming Red Hat copyright.
> This looks dubious.
>
>> + *
>> + */
>> +
>> +#ifndef RSS_TAP_BPF_H
>> +#define RSS_TAP_BPF_H
>> +
>> +/* hashed fields for RSS */
>> +enum hash_field {
>> +  HASH_FIELD_IPV4_L3,  /* IPv4 src/dst addr */
>> +  HASH_FIELD_IPV4_L3_L4,  /* IPv4 src/dst addr + L4 src/dst ports */
>> +  HASH_FIELD_IPV6_L3,  /* IPv6 src/dst addr */
>> +  HASH_FIELD_IPV6_L3_L4,  /* IPv6 src/dst addr + L4 src/dst ports */
>> +  HASH_FIELD_L2_SRC,  /* Ethernet src addr */
>> +  HASH_FIELD_L2_DST,  /* Ethernet dst addr */
>> +  HASH_FIELD_L3_SRC,  /* L3 src addr */
>> +  HASH_FIELD_L3_DST,  /* L3 dst addr */
>> +  HASH_FIELD_L4_SRC,  /* TCP/UDP src ports */
>> +  HASH_FIELD_L4_DST,  /* TCP/UDP dst ports */
>> +};
>> +
>> +struct rss_key {
>> +  __u32 hash_fields;
>> +  __u32 nb_queues;
>> +    __u32 *indirection_table;
>> +    __u32 indirection_table_size;
>> +  __u8 *key;
>> +  __u32 key_size;
>> +} __attribute__((packed));
>> +
>> +#endif
>> diff --git a/hw/net/rss_tap_bpf_program.c b/hw/net/rss_tap_bpf_program.c
>> new file mode 100644
>> index 0000000000..2744436e86
>> --- /dev/null
>> +++ b/hw/net/rss_tap_bpf_program.c
>> @@ -0,0 +1,175 @@
>> +/*
>> + * RSS ebpf code for virtio-net
>> + *
>> + * Copyright (c) 2018 RedHat.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + * This code is heavily based on the following bpf code from dpdk
>> + * https://git.dpdk.org/dpdk/tree/drivers/net/tap/tap_bpf_program.c
>
> That file says
>
>    * Copyright 2017 Mellanox Technologies, Ltd
>
> while you are claiming RedHat copyright. That can't be right attribution
> if this is indeed a derived work.
I am not an expert when it comes to licensing, I gave credits Mellanox
for the work but I have introduced some new changes as well to suit my
usage.
Moreover the license for the original code is 3BSD or GPL, I have no
idea how this is possible and which license I should take into
account?
Anyways I'd love some guidance on this topic so I can give the
appropriate credit where it is needed.

Thanks!
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



-- 
Respectfully,
Sameeh Jubran
Linkedin
Software Engineer @ Daynix.