* Re: [bpf-next V2 PATCH 0/2] Implement sample code for XDP cpumap IP-pair load-balancing
From: Daniel Borkmann @ 2018-08-10 14:10 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev
Cc: victor, eric, Daniel Borkmann, Alexei Starovoitov, jhsiao
In-Reply-To: <153390254528.10825.9125109091513926669.stgit@firesoul>
On 08/10/2018 02:02 PM, Jesper Dangaard Brouer wrote:
> Background: cpumap moves the SKB allocation out of the driver code,
> and instead allocate it on the remote CPU, and invokes the regular
> kernel network stack with the newly allocated SKB.
>
> The idea behind the XDP CPU redirect feature, is to use XDP as a
> load-balancer step in-front of regular kernel network stack. But the
> current sample code does not provide a good example of this. Part of
> the reason is that, I have implemented this as part of Suricata XDP
> load-balancer.
>
> Given this is the most frequent feature request I get. This patchset
> implement the same XDP load-balancing as Suricata does, which is a
> symmetric hash based on the IP-pairs + L4-protocol.
>
> The expected setup for the use-case is to reduce the number of NIC RX
> queues via ethtool (as XDP can handle more per core), and via
> smp_affinity assign these RX queues to a set of CPUs, which will be
> handling RX packets. The CPUs that runs the regular network stack is
> supplied to the sample xdp_redirect_cpu tool by specifying
> the --cpu option multiple times on the cmdline.
>
> I do note that cpumap SKB creation is not feature complete yet, and
> more work is coming. E.g. given GRO is not implemented yet, do expect
> TCP workloads to be slower. My measurements do indicate UDP workloads
> are faster.
Applied to bpf-next, thanks Jesper!
^ permalink raw reply
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
From: Daniel Borkmann @ 2018-08-10 14:10 UTC (permalink / raw)
To: Björn Töpel, ast, netdev, ap420073, brouer
Cc: Björn Töpel, magnus.karlsson, magnus.karlsson, kafai
In-Reply-To: <20180810092802.5948-1-bjorn.topel@gmail.com>
On 08/10/2018 11:28 AM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
>
> The reverted commit adds a WARN to check against NULL entries in the
> mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> driver) fast path is required to make a paired
> xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> addition, a driver using a different allocation scheme than the
> default MEM_TYPE_PAGE_SHARED is required to additionally call
> xdp_rxq_info_reg_mem_model.
>
> For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> that the mem_id_ht rhashtable has a properly inserted allocator id. If
> not, this would be a driver bug. A NULL pointer kernel OOPS is
> preferred to the WARN.
>
> Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Given the last bpf pr went out yesterday night, I've applied this to
bpf-next (worst case we can just route it via stable), thanks!
^ permalink raw reply
* [PATCH net-next 1/1] tc: Update README and add config
From: Keara Leibovitz @ 2018-08-10 14:09 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, xiyou.wangcong, jiri, lucasb, Keara Leibovitz
Updated README.
Added config file that contains the minimum required features enabled to
run the tests currently present in the kernel.
This must be updated when new unittests are created and require their own
modules.
Signed-off-by: Keara Leibovitz <kleib@mojatatu.com>
---
tools/testing/selftests/tc-testing/README | 16 +++++++----
tools/testing/selftests/tc-testing/config | 48 +++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 5 deletions(-)
create mode 100644 tools/testing/selftests/tc-testing/config
diff --git a/tools/testing/selftests/tc-testing/README b/tools/testing/selftests/tc-testing/README
index 3a0336782d2d..49a6f8c3fdae 100644
--- a/tools/testing/selftests/tc-testing/README
+++ b/tools/testing/selftests/tc-testing/README
@@ -17,6 +17,10 @@ REQUIREMENTS
* The kernel must have veth support available, as a veth pair is created
prior to running the tests.
+* The kernel must have the appropriate infrastructure enabled to run all tdc
+ unit tests. See the config file in this directory for minimum required
+ features. As new tests will be added, config options list will be updated.
+
* All tc-related features being tested must be built in or available as
modules. To check what is required in current setup run:
./tdc.py -c
@@ -109,8 +113,8 @@ COMMAND LINE ARGUMENTS
Run tdc.py -h to see the full list of available arguments.
usage: tdc.py [-h] [-p PATH] [-D DIR [DIR ...]] [-f FILE [FILE ...]]
- [-c [CATG [CATG ...]]] [-e ID [ID ...]] [-l] [-s] [-i] [-v]
- [-d DEVICE] [-n NS] [-V]
+ [-c [CATG [CATG ...]]] [-e ID [ID ...]] [-l] [-s] [-i] [-v] [-N]
+ [-d DEVICE] [-P] [-n] [-V]
Linux TC unit tests
@@ -118,8 +122,10 @@ optional arguments:
-h, --help show this help message and exit
-p PATH, --path PATH The full path to the tc executable to use
-v, --verbose Show the commands that are being run
+ -N, --notap Suppress tap results for command under test
-d DEVICE, --device DEVICE
Execute the test case in flower category
+ -P, --pause Pause execution just before post-suite stage
selection:
select which test cases: files plus directories; filtered by categories
@@ -146,10 +152,10 @@ action:
-i, --id Generate ID numbers for new test cases
netns:
- options for nsPlugin(run commands in net namespace)
+ options for nsPlugin (run commands in net namespace)
- -n NS, --namespace NS
- Run commands in namespace NS
+ -n, --namespace
+ Run commands in namespace as specified in tdc_config.py
valgrind:
options for valgrindPlugin (run command under test under Valgrind)
diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
new file mode 100644
index 000000000000..203302065458
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/config
@@ -0,0 +1,48 @@
+CONFIG_NET_SCHED=y
+
+#
+# Queueing/Scheduling
+#
+CONFIG_NET_SCH_PRIO=m
+CONFIG_NET_SCH_INGRESS=m
+
+#
+# Classification
+#
+CONFIG_NET_CLS=y
+CONFIG_NET_CLS_FW=m
+CONFIG_NET_CLS_U32=m
+CONFIG_CLS_U32_PERF=y
+CONFIG_CLS_U32_MARK=y
+CONFIG_NET_EMATCH=y
+CONFIG_NET_EMATCH_STACK=32
+CONFIG_NET_EMATCH_CMP=m
+CONFIG_NET_EMATCH_NBYTE=m
+CONFIG_NET_EMATCH_U32=m
+CONFIG_NET_EMATCH_META=m
+CONFIG_NET_EMATCH_TEXT=m
+CONFIG_NET_EMATCH_IPSET=m
+CONFIG_NET_EMATCH_IPT=m
+CONFIG_NET_CLS_ACT=y
+CONFIG_NET_ACT_POLICE=m
+CONFIG_NET_ACT_GACT=m
+CONFIG_GACT_PROB=y
+CONFIG_NET_ACT_MIRRED=m
+CONFIG_NET_ACT_SAMPLE=m
+CONFIG_NET_ACT_IPT=m
+CONFIG_NET_ACT_NAT=m
+CONFIG_NET_ACT_PEDIT=m
+CONFIG_NET_ACT_SIMP=m
+CONFIG_NET_ACT_SKBEDIT=m
+CONFIG_NET_ACT_CSUM=m
+CONFIG_NET_ACT_VLAN=m
+CONFIG_NET_ACT_BPF=m
+CONFIG_NET_ACT_CONNMARK=m
+CONFIG_NET_ACT_SKBMOD=m
+CONFIG_NET_ACT_IFE=m
+CONFIG_NET_ACT_TUNNEL_KEY=m
+CONFIG_NET_IFE_SKBMARK=m
+CONFIG_NET_IFE_SKBPRIO=m
+CONFIG_NET_IFE_SKBTCINDEX=m
+CONFIG_NET_CLS_IND=y
+CONFIG_NET_SCH_FIFO=y
--
2.7.4
^ permalink raw reply related
* Re: KMSAN: uninit-value in ip_tunnel_xmit (2)
From: William Tu @ 2018-08-10 16:20 UTC (permalink / raw)
To: syzbot
Cc: David Miller, Alexey Kuznetsov, LKML,
Linux Kernel Network Developers, syzkaller-bugs,
Hideaki YOSHIFUJI
In-Reply-To: <0000000000005012b605731594e3@google.com>
On Fri, Aug 10, 2018 at 7:30 AM, syzbot
<syzbot+4a2c52677a8a1aa283cb@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 80ecacc456c1 kmsan: fix CONFIG_KMSAN=n build
> git tree: https://github.com/google/kmsan.git/master
> console output: https://syzkaller.appspot.com/x/log.txt?x=163b7794400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=93d57043084eee38
> dashboard link: https://syzkaller.appspot.com/bug?extid=4a2c52677a8a1aa283cb
> compiler: clang version 7.0.0 (trunk 334104)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=17cec9dc400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=145dcf0c400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+4a2c52677a8a1aa283cb@syzkaller.appspotmail.com
>
> IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> 8021q: adding VLAN 0 to HW filter on device team0
> ==================================================================
> BUG: KMSAN: uninit-value in ip_tunnel_xmit+0x63a/0x3b10 net/ipv4/ip_tunnel.c:645
It complains about
644 dst = tnl_params->daddr;
645 if (dst == 0) {
I guess at __gre_tunnel_init, we have to zero the tunne->parms.iph.
I will test it to see if the issue go away.
Thanks
William
> CPU: 1 PID: 4603 Comm: syz-executor712 Not tainted 4.18.0-rc4+ #27
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x185/0x1e0 lib/dump_stack.c:113
> kmsan_report+0x195/0x2c0 mm/kmsan/kmsan.c:982
> __msan_warning+0x7d/0xe0 mm/kmsan/kmsan_instr.c:645
> ip_tunnel_xmit+0x63a/0x3b10 net/ipv4/ip_tunnel.c:645
> __gre_xmit net/ipv4/ip_gre.c:449 [inline]
> ipgre_xmit+0xe16/0xef0 net/ipv4/ip_gre.c:701
> __netdev_start_xmit include/linux/netdevice.h:4148 [inline]
> netdev_start_xmit include/linux/netdevice.h:4157 [inline]
> xmit_one net/core/dev.c:3034 [inline]
> dev_hard_start_xmit+0x60f/0xcc0 net/core/dev.c:3050
> __dev_queue_xmit+0x3060/0x3c70 net/core/dev.c:3569
> dev_queue_xmit+0x4b/0x60 net/core/dev.c:3602
> packet_snd net/packet/af_packet.c:2919 [inline]
> packet_sendmsg+0x8469/0x9010 net/packet/af_packet.c:2944
> sock_sendmsg_nosec net/socket.c:641 [inline]
> sock_sendmsg net/socket.c:651 [inline]
> ___sys_sendmsg+0xed9/0x1350 net/socket.c:2125
> __sys_sendmsg net/socket.c:2163 [inline]
> __do_sys_sendmsg net/socket.c:2172 [inline]
> __se_sys_sendmsg net/socket.c:2170 [inline]
> __x64_sys_sendmsg+0x3b0/0x520 net/socket.c:2170
> do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x63/0xe7
> RIP: 0033:0x4412f9
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffdc77f10d8 EFLAGS: 00000213 ORIG_RAX: 000000000000002e
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004412f9
> RDX: 0000000000000000 RSI: 0000000020000600 RDI: 0000000000000003
> RBP: 00000000006cc018 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000020 R11: 0000000000000213 R12: 0000000000402260
> R13: 00000000004022f0 R14: 0000000000000000 R15: 0000000000000000
>
> Uninit was created at:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
> kmsan_internal_poison_shadow+0xc8/0x1d0 mm/kmsan/kmsan.c:181
> kmsan_kmalloc+0xa1/0x120 mm/kmsan/kmsan_hooks.c:91
> kmsan_slab_alloc+0x10/0x20 mm/kmsan/kmsan_hooks.c:100
> slab_post_alloc_hook mm/slab.h:446 [inline]
> slab_alloc_node mm/slub.c:2720 [inline]
> __kmalloc_node_track_caller+0xb48/0x11d0 mm/slub.c:4353
> __kmalloc_reserve net/core/skbuff.c:138 [inline]
> __alloc_skb+0x2cb/0x9e0 net/core/skbuff.c:206
> alloc_skb include/linux/skbuff.h:988 [inline]
> alloc_skb_with_frags+0x1e6/0xb80 net/core/skbuff.c:5271
> sock_alloc_send_pskb+0xb59/0x11e0 net/core/sock.c:2095
> packet_alloc_skb net/packet/af_packet.c:2776 [inline]
> packet_snd net/packet/af_packet.c:2867 [inline]
> packet_sendmsg+0x68cb/0x9010 net/packet/af_packet.c:2944
> sock_sendmsg_nosec net/socket.c:641 [inline]
> sock_sendmsg net/socket.c:651 [inline]
> ___sys_sendmsg+0xed9/0x1350 net/socket.c:2125
> __sys_sendmsg net/socket.c:2163 [inline]
> __do_sys_sendmsg net/socket.c:2172 [inline]
> __se_sys_sendmsg net/socket.c:2170 [inline]
> __x64_sys_sendmsg+0x3b0/0x520 net/socket.c:2170
> do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x63/0xe7
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* this here for you
From: Kelly @ 2018-08-10 10:33 UTC (permalink / raw)
To: netdev
Your photos need editing. We can do it for you.
We do editing for e-commerce photos, jewelries images and portrait photos
etc.
This will include cutout and clipping path etc , also retouching if needed.
Let;s know if you want to send photos for working.
We can do test on your photos.
Thanks,
Kelly
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Daniel Borkmann @ 2018-08-10 13:30 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: Alexei Starovoitov, ast, netdev, yhs
In-Reply-To: <20180810125450.qsurzvu4zuy6niqt@kafai-mbp.dhcp.thefacebook.com>
On 08/10/2018 02:54 PM, Martin KaFai Lau wrote:
> On Fri, Aug 10, 2018 at 09:55:35AM +0200, Daniel Borkmann wrote:
>> On 08/10/2018 04:13 AM, Alexei Starovoitov wrote:
>>> On Fri, Aug 10, 2018 at 12:43:20AM +0200, Daniel Borkmann wrote:
>>>> On 08/09/2018 11:44 PM, Alexei Starovoitov wrote:
>>>>> On Thu, Aug 09, 2018 at 11:30:52PM +0200, Daniel Borkmann wrote:
>>>>>> On 08/09/2018 11:14 PM, Alexei Starovoitov wrote:
>>>>>>> On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
>>>>>>>> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
>>>>>>>> the basic arraymap") enabled support for BTF and dumping via
>>>>>>>> BPF fs for arraymap. However, both can be decoupled from each
>>>>>>>> other such that all BPF maps can be supported for attaching
>>>>>>>> BTF key/value information, while not all maps necessarily
>>>>>>>> need to dump via map_seq_show_elem() callback.
>>>>>>>>
>>>>>>>> The check in array_map_check_btf() can be generalized as
>>>>>>>> ultimatively the key and value size is the only contraint
>>>>>>>> that needs to match for the map. The fact that the key needs
>>>>>>>> to be of type int is optional; it could be any data type as
>>>>>>>> long as it matches the 4 byte key size, just like hash table
>>>>>>>> key or others could be of any data type as well.
>>>>>>>>
>>>>>>>> Minimal example of a hash table dump which then works out
>>>>>>>> of the box for bpftool:
>>>>>>>>
>>>>>>>> # bpftool map dump id 19
>>>>>>>> [{
>>>>>>>> "key": {
>>>>>>>> "": {
>>>>>>>> "vip": 0,
>>>>>>>> "vipv6": []
>>>>>>>> },
>>>>>>>> "port": 0,
>>>>>>>> "family": 0,
>>>>>>>> "proto": 0
>>>>>>>> },
>>>>>>>> "value": {
>>>>>>>> "flags": 0,
>>>>>>>> "vip_num": 0
>>>>>>>> }
>>>>>>>> }
>>>>>>>> ]
>>>>>>>>
>>>>>>>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>>>>>>>> Cc: Yonghong Song <yhs@fb.com>
>>>>>>>> ---
>>>>>>>> include/linux/bpf.h | 4 +---
>>>>>>>> kernel/bpf/arraymap.c | 27 ---------------------------
>>>>>>>> kernel/bpf/inode.c | 3 ++-
>>>>>>>> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
>>>>>>>> 4 files changed, 23 insertions(+), 35 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>>>>>>> index cd8790d..eb76e8e 100644
>>>>>>>> --- a/include/linux/bpf.h
>>>>>>>> +++ b/include/linux/bpf.h
>>>>>>>> @@ -48,8 +48,6 @@ struct bpf_map_ops {
>>>>>>>> u32 (*map_fd_sys_lookup_elem)(void *ptr);
>>>>>>>> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
>>>>>>>> struct seq_file *m);
>>>>>>>> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
>>>>>>>> - u32 key_type_id, u32 value_type_id);
>>>>>>>> };
>>>>>>>>
>>>>>>>> struct bpf_map {
>>>>>>>> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
>>>>>>>>
>>>>>>>> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
>>>>>>>> {
>>>>>>>> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
>>>>>>>> + return map->btf && map->ops->map_seq_show_elem;
>>>>>>>> }
>>>>>>>>
>>>>>>>> extern const struct bpf_map_ops bpf_map_offload_ops;
>>>>>>>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>>>>>>>> index 2aa55d030..67f0bdf 100644
>>>>>>>> --- a/kernel/bpf/arraymap.c
>>>>>>>> +++ b/kernel/bpf/arraymap.c
>>>>>>>> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
>>>>>>>> rcu_read_unlock();
>>>>>>>> }
>>>>>>>>
>>>>>>>> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
>>>>>>>> - u32 btf_key_id, u32 btf_value_id)
>>>>>>>> -{
>>>>>>>> - const struct btf_type *key_type, *value_type;
>>>>>>>> - u32 key_size, value_size;
>>>>>>>> - u32 int_data;
>>>>>>>> -
>>>>>>>> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
>>>>>>>> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
>>>>>>>> - return -EINVAL;
>>>>>>>> -
>>>>>>>> - int_data = *(u32 *)(key_type + 1);
>>>>>>>> - /* bpf array can only take a u32 key. This check makes
>>>>>>>> - * sure that the btf matches the attr used during map_create.
>>>>>>>> - */
>>>>>>>> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
>>>>>>>> - BTF_INT_OFFSET(int_data))
>>>>>>>> - return -EINVAL;
>>>>>>>
>>>>>>> I think most of these checks are still necessary for array type.
>>>>>>> Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
>>>>>>> is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
>>>>>>
>>>>>> Hmm, so on 64 bit archs BTF_KIND_PTR would get rejected for array,
>>>>>> on 32 bit it may be allowed due to sizeof(void *) == 4. BTF_KIND_ARRAY
>>>>>> could be array of u8 foo[4], for example, or u16 foo[2]. But how would
>>>>>> it ultimately be different from e.g. having 'struct a' versus 'struct b'
>>>>>> where both are of same size and while actual key has 'struct a', the one
>>>>>> who writes the prog resp. loads the BTF into the kernel would lie about
>>>>>> it stating it's of type 'struct b' instead? It's basically trusting the
>>>>>> app that it advertised sane key types which kernel is propagating back.
>>>>>
>>>>> for hash map - yes. the kernel cannot yet catch the lie that
>>>>> key == 'struct a' that user said in BTF is not what program used
>>>>> (which used 'struct b' of the same size).
>>>>> Eventually we will annotate all load/store in the program and will
>>>>> make sure that memory access match what BTF said.
>>>>
>>>> But in that case, would you reject the program? E.g. from prog point of
>>>> view, it's just a buffer of x bytes, so key could be casted to different
>>>> struct/types potentially and used for lookup; similar with value if you
>>>> would go the route to annotate all access into it. I don't think this
>>>> serves as a security feature (since you might as well just load the prog
>>>> without BTF just fine), but it can be used to help verifier to perform
>>>> rewrites like in tracing for implicit bpf_probe_read() based on member
>>>> access. But also in that case, if you might have e.g. stale or wrong BTF
>>>> data e.g. of the whole kernel or some application it would follow/walk
>>>> that one instead. But such user error would be "acceptable" since it serves
>>>> as a hint, roughly similar to (explicitly) walking the data structures
>>>> based on the headers today, you do have better control in terms of header
>>>> mismatches in that you can ship the BTF directly from the build, but there's
>>>> still no guarantee in that sense that you "verified" that these bytes
>>>> originally were mapped to struct foo somewhere in a C program.
>>>
>>> I wouldn't view such key checks as safety feature, but rather
>>> as honesty check. Meaning that user space shouldn't be able to cheat
>>> by passing completely bogus BTF into the kernel that only
>>> statisfies single size check.
>>
>> Ok, meaning, we agree that BTF is passed as a hint to the kernel. It is
>> a 'should not cheat' requirement but not 'cannot cheat', meaning if it
>> was the latter, BTF is core part of the verifier's safety analysis /
>> checks, but as it's optional on top of it (well, due to compatibility it
>> kind of needs to be anyway), it doesn't affect kernel's safety in
>> relation to what the program is doing internally.
>>
>>> Say, BTF has a key that is a 4 byte struct { char a,b,c,d; };
>>> but program operates with it as u32. I very much would like
>>> the verifier to notice and reject such program, since if C code
>>> has struct { char a,b,c,d; }; the compiler won't generate u32 access
>>> to it unless C code type casts, but then llvm will warn. So for u32
>>> to legitimately appear in the generated code the struct should be:
>>> union {
>>> struct { char a,b,c,d;}
>>> u32 e;
>>> };
>>> Narrow access (like u8 load/store in the bpf prog form u32 BTF field)
>>> is ok, since that's normal compiler optimization, but any other
>>> field/size mismatch the verifier should reject to prevent cheating.
>>>
>>> In other words even proprietary bpf programs should not be able to cheat.
>>> If they provide BTF to the kernel, it should correspond exactly to the program.
>>> That's the key to _trusted_ introspection.
>>
>> But then wouldn't this artificially force users into programming /
>> thinking style that verifier dictates upon them similar as we have
>> today as opposed to further moving away from it to allow more C-style
>> programs to be accepted?
>>
>> But even if that is the goal, it is still used as a hint today, e.g.
>> even for an array I could tell the compiler that the key is '__u32'
>> for BTF sake while using the underlying key differently (e.g. as struct,
>> enum, etc) in order to pass the array_map_check_btf() check in the
>> kernel. Those type of programs would still need to be accepted due
>> to compatibility reasons. Same if we only test on size match in other
>> maps, it's nothing different. I don't see how verifier would start
>> enforcing programs to be rejected based on access patterns on the
>> types; while it might work for newly added program types that this
>> could be enforced, it cannot for all the many existing program types,
>> but I also don't really think it needs to be. Unless it is declared
>> a safety feature for future program types.
>>
>>> Admin that ssh-es into the box and operates with bpftool should be
>>> certain that the map introspection represent the real situation of
>>> the program. If proprietary prog is paranoid about layout of map
>>> it shouldn't be using BTF, but if it does, BTF should match.
>>> In the future I'd like to enfore availability of BTF for
>>> new program types.
>>
>> Sure, and in vast majority / normal cases it will represent the real
>> situation. Do you mistrust DWARF debugging data that gets shipped with
>> your distro, for example? pahole and other tools on top of it? It is
>> pretty much the same situation. Given gdb example below, does gdb do
>> any verification of the binary in order to analyze load/store patterns
>> and whether it matches with what sits in DWARF as types? It would just
>> as well map the types into memory and dump it as pretty print instead.
>> And that's okay, the one thing that needs to be verified for correctness
>> resp. validity is the debugging format itself so it can be used for
>> that.
> I think decoupling map_seq_show_elem() requirement from BTF support
> during map_create() is ok. There is plan to do that going forward if
> it ever hits some map types that is difficult to do map_seq_show_elem().
> However, I think most of the maps should have pretty straight
> forward seq_show implementation considering most of the
> heavy lifting has already been done in btf_*_seq_show()
> and bpffs_map_seq_ops.
I think potentially this could also be made generic for majority of maps
in that you do the lookup and then the key/value btf_type_seq_show()
dump from it under RCU.
> Regarding one general map_check_btf() for all maps' purpose,
> I think not all bpf maps are behaving the same.
>
> There are some property that the kernel implies on a particular
> map type. Some of them even have kernel generated value in it.
> For example, map-in-map, the lookup_elem() value is a map's id
> which must be an integer generated by kernel's idr_alloc_cyclic().
> Treating it as enum/struct/array...etc seems wrong.
> Hence, I think each map type should be able to provide
> its own map_check_btf(). If a map does not have
> specific key/value requirement, it can use the default one which
> is the min requirement for kernel safety reason.
Yeah, fair point, I think potentially for the in-kernel generated ones at
some point they could provide their own in-kernel BTF description. Ok,
I'll rework to have a default and stricter map specific override if present
which those in-kernel ones will reject then.
Thanks,
Daniel
^ permalink raw reply
* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Alan Cox @ 2018-08-10 15:57 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: Michal Kubeček, Konstantin Böhm, shess, pieter.robyns,
linux-kernel@vger.kernel.org>, , Xue Liu, Ken Yu,
Michael Röder, Stefan Schmidt, Rob Herring, lora,
Alexander Graf, Jan Jongboom, Janus Piwek, Jon Ortego, devicetree,
Jiri Pirko, Hasnain Virk, Daniele Comel, Marcel Holtmann,
linux-spi, Mark Brown, Dollar Chen, Brian Ray
In-Reply-To: <CAC=mGzjn4Kc-q9NFwX6NhU4V_thnzBP8SHtFQKyRbpSKR=yx3w@mail.gmail.com>
> Except saving power, mitigating the wireless signal conflict on the
> air is one of the reasons.
If the device level is always receiving when not transmitting it has no
effect on this. The act of listening does not harm other traffic.
> The sleep/idle/stop mitigate the unconcerned RF signals or messages.
At the physical level it's irrelevant. If we are receiving then we might
hear more things we later discard. It's not running on a tiny
microcontroller so the extra CPU cycles are not going to kill us.
> > How do you plan to deal with routing if you've got multiple devices ?
>
> For LoRaWAN, it is a star topology.
No the question was much more how you plan to deal with it in the OS. If
for example I want to open a LORA connection to something, then there
needs to be a proper process to figure out where the target is and how to
get traffic to them.
I guess it's best phrased as
- What does a struct sockaddr_lora look like
- How does the kernel decide which interface it goes out of (if any), and
if it loops back
remembering we might only be talking to a hub, or we might even be a
virtualized LORA interface where we are pretending to be some kind of
sensor and feeding it back.
Long term yes I think Alexander is right the inevitable fate of all
networks is to become a link layer in order to transmit IP frames 8)
Alan
^ permalink raw reply
* did you check it
From: Kelly @ 2018-08-10 11:02 UTC (permalink / raw)
To: netdev
Your photos need editing. We can do it for you.
We do editing for e-commerce photos, jewelries images and portrait photos
etc.
This will include cutout and clipping path etc , also retouching if needed.
Let;s know if you want to send photos for working.
We can do test on your photos.
Thanks,
Kelly
^ permalink raw reply
* Re: Error running AF_XDP sample application
From: Konrad Djimeli @ 2018-08-10 13:21 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, netdev-owner
In-Reply-To: <23a938260df3326eb036207f6e193f1b@igalia.com>
On 2018-08-10 11:58, Konrad Djimeli wrote:
> On 2018-08-10 03:51, Jakub Kicinski wrote:
>> On Thu, 09 Aug 2018 18:18:08 +0200, kdjimeli wrote:
>>> Hello,
>>>
>>> I have been trying to test a sample AF_XDP program, but I have been
>>> experiencing some issues.
>>> After building the sample code
>>> https://github.com/torvalds/linux/tree/master/samples/bpf,
>>> when running the xdpsock binary, I get the errors
>>> "libbpf: failed to create map (name: 'xsks_map'): Invalid argument"
>>> "libbpf: failed to load object './xdpsock_kern.o"
>>>
>>> I tried to figure out the cause of the error but all I know is that it
>>> occurs at line 910 with the function
>>> call "bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd)".
>>>
>>> Please I would like to inquire what could be a possible for this error.
>>
>> which kernel version are you running?
>
> My kernel version is 4.18.0-rc8+. I cloned it from
> https://github.com/torvalds/linux before building a running.
>
> My commit head(git show-ref --head) is at
> 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc HEAD
> 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc refs/heads/master
> 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc refs/remotes/origin/HEAD
> 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc refs/remotes/origin/master
> ...
>
>
> I also applied the patch https://patchwork.ozlabs.org/patch/949884/
> (samples: bpf: convert xdpsock_user.c to libbpf ), as the error was
> initially in the form show below:
> "failed to create a map: 22 Invalid argument"
> "ERROR: load_bpf_file"
>
> Thanks
> Konrad
Also other sample applications that make use of other bpf maps, such as
BPF_MAP_TYPE_CPUMAP in xdp_redirect_cpu work fine. But the application
with BPF_MAP_TYPE_XSKMAP fails producing the error mentioned above.
Thanks
Konrad
^ permalink raw reply
* [PATCH net-next 1/1] net/smc: send response to test link signal
From: Ursula Braun @ 2018-08-10 15:45 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
With SMC-D z/OS sends a test link signal every 10 seconds. Linux is
supposed to answer, otherwise the SMC-D connection breaks.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_ism.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index cfade7fdcc6d..e36f21ce7252 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -184,6 +184,37 @@ struct smc_ism_event_work {
struct smcd_event event;
};
+#define ISM_EVENT_REQUEST 0x0001
+#define ISM_EVENT_RESPONSE 0x0002
+#define ISM_EVENT_REQUEST_IR 0x00000001
+#define ISM_EVENT_CODE_TESTLINK 0x83
+
+static void smcd_handle_sw_event(struct smc_ism_event_work *wrk)
+{
+ union {
+ u64 info;
+ struct {
+ u32 uid;
+ unsigned short vlanid;
+ u16 code;
+ };
+ } ev_info;
+
+ switch (wrk->event.code) {
+ case ISM_EVENT_CODE_TESTLINK: /* Activity timer */
+ ev_info.info = wrk->event.info;
+ if (ev_info.code == ISM_EVENT_REQUEST) {
+ ev_info.code = ISM_EVENT_RESPONSE;
+ wrk->smcd->ops->signal_event(wrk->smcd,
+ wrk->event.tok,
+ ISM_EVENT_REQUEST_IR,
+ ISM_EVENT_CODE_TESTLINK,
+ ev_info.info);
+ }
+ break;
+ }
+}
+
/* worker for SMC-D events */
static void smc_ism_event_work(struct work_struct *work)
{
@@ -196,6 +227,9 @@ static void smc_ism_event_work(struct work_struct *work)
break;
case ISM_EVENT_DMB:
break;
+ case ISM_EVENT_SWR: /* Software defined event */
+ smcd_handle_sw_event(wrk);
+ break;
}
kfree(wrk);
}
--
2.16.4
^ permalink raw reply related
* [PATCH] chtls: fix null dereference chtls_free_uld()
From: Ganesh Goudar @ 2018-08-10 12:57 UTC (permalink / raw)
To: herbert, linux-crypto
Cc: netdev, davem, indranil, dt, Ganesh Goudar, Atul Gupta
call chtls_free_uld() only for the initialized cdev,
this fixes NULL dereference in chtls_free_uld()
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
drivers/crypto/chelsio/chtls/chtls.h | 5 +++++
drivers/crypto/chelsio/chtls/chtls_main.c | 7 +++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/chelsio/chtls/chtls.h b/drivers/crypto/chelsio/chtls/chtls.h
index a53a0e6..7725b6e 100644
--- a/drivers/crypto/chelsio/chtls/chtls.h
+++ b/drivers/crypto/chelsio/chtls/chtls.h
@@ -96,6 +96,10 @@ enum csk_flags {
CSK_CONN_INLINE, /* Connection on HW */
};
+enum chtls_cdev_state {
+ CHTLS_CDEV_STATE_UP = 1
+};
+
struct listen_ctx {
struct sock *lsk;
struct chtls_dev *cdev;
@@ -146,6 +150,7 @@ struct chtls_dev {
unsigned int send_page_order;
int max_host_sndbuf;
struct key_map kmap;
+ unsigned int cdev_state;
};
struct chtls_hws {
diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c b/drivers/crypto/chelsio/chtls/chtls_main.c
index 9b07f91..f59b044 100644
--- a/drivers/crypto/chelsio/chtls/chtls_main.c
+++ b/drivers/crypto/chelsio/chtls/chtls_main.c
@@ -160,6 +160,7 @@ static void chtls_register_dev(struct chtls_dev *cdev)
tlsdev->hash = chtls_create_hash;
tlsdev->unhash = chtls_destroy_hash;
tls_register_device(&cdev->tlsdev);
+ cdev->cdev_state = CHTLS_CDEV_STATE_UP;
}
static void chtls_unregister_dev(struct chtls_dev *cdev)
@@ -281,8 +282,10 @@ static void chtls_free_all_uld(void)
struct chtls_dev *cdev, *tmp;
mutex_lock(&cdev_mutex);
- list_for_each_entry_safe(cdev, tmp, &cdev_list, list)
- chtls_free_uld(cdev);
+ list_for_each_entry_safe(cdev, tmp, &cdev_list, list) {
+ if (cdev->cdev_state == CHTLS_CDEV_STATE_UP)
+ chtls_free_uld(cdev);
+ }
mutex_unlock(&cdev_mutex);
}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Martin KaFai Lau @ 2018-08-10 12:54 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Alexei Starovoitov, ast, netdev, yhs
In-Reply-To: <ffe4dcf6-ac9a-ca24-fd65-c4bf453a4721@iogearbox.net>
On Fri, Aug 10, 2018 at 09:55:35AM +0200, Daniel Borkmann wrote:
> On 08/10/2018 04:13 AM, Alexei Starovoitov wrote:
> > On Fri, Aug 10, 2018 at 12:43:20AM +0200, Daniel Borkmann wrote:
> >> On 08/09/2018 11:44 PM, Alexei Starovoitov wrote:
> >>> On Thu, Aug 09, 2018 at 11:30:52PM +0200, Daniel Borkmann wrote:
> >>>> On 08/09/2018 11:14 PM, Alexei Starovoitov wrote:
> >>>>> On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
> >>>>>> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
> >>>>>> the basic arraymap") enabled support for BTF and dumping via
> >>>>>> BPF fs for arraymap. However, both can be decoupled from each
> >>>>>> other such that all BPF maps can be supported for attaching
> >>>>>> BTF key/value information, while not all maps necessarily
> >>>>>> need to dump via map_seq_show_elem() callback.
> >>>>>>
> >>>>>> The check in array_map_check_btf() can be generalized as
> >>>>>> ultimatively the key and value size is the only contraint
> >>>>>> that needs to match for the map. The fact that the key needs
> >>>>>> to be of type int is optional; it could be any data type as
> >>>>>> long as it matches the 4 byte key size, just like hash table
> >>>>>> key or others could be of any data type as well.
> >>>>>>
> >>>>>> Minimal example of a hash table dump which then works out
> >>>>>> of the box for bpftool:
> >>>>>>
> >>>>>> # bpftool map dump id 19
> >>>>>> [{
> >>>>>> "key": {
> >>>>>> "": {
> >>>>>> "vip": 0,
> >>>>>> "vipv6": []
> >>>>>> },
> >>>>>> "port": 0,
> >>>>>> "family": 0,
> >>>>>> "proto": 0
> >>>>>> },
> >>>>>> "value": {
> >>>>>> "flags": 0,
> >>>>>> "vip_num": 0
> >>>>>> }
> >>>>>> }
> >>>>>> ]
> >>>>>>
> >>>>>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> >>>>>> Cc: Yonghong Song <yhs@fb.com>
> >>>>>> ---
> >>>>>> include/linux/bpf.h | 4 +---
> >>>>>> kernel/bpf/arraymap.c | 27 ---------------------------
> >>>>>> kernel/bpf/inode.c | 3 ++-
> >>>>>> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
> >>>>>> 4 files changed, 23 insertions(+), 35 deletions(-)
> >>>>>>
> >>>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >>>>>> index cd8790d..eb76e8e 100644
> >>>>>> --- a/include/linux/bpf.h
> >>>>>> +++ b/include/linux/bpf.h
> >>>>>> @@ -48,8 +48,6 @@ struct bpf_map_ops {
> >>>>>> u32 (*map_fd_sys_lookup_elem)(void *ptr);
> >>>>>> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
> >>>>>> struct seq_file *m);
> >>>>>> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
> >>>>>> - u32 key_type_id, u32 value_type_id);
> >>>>>> };
> >>>>>>
> >>>>>> struct bpf_map {
> >>>>>> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
> >>>>>>
> >>>>>> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
> >>>>>> {
> >>>>>> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
> >>>>>> + return map->btf && map->ops->map_seq_show_elem;
> >>>>>> }
> >>>>>>
> >>>>>> extern const struct bpf_map_ops bpf_map_offload_ops;
> >>>>>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> >>>>>> index 2aa55d030..67f0bdf 100644
> >>>>>> --- a/kernel/bpf/arraymap.c
> >>>>>> +++ b/kernel/bpf/arraymap.c
> >>>>>> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
> >>>>>> rcu_read_unlock();
> >>>>>> }
> >>>>>>
> >>>>>> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
> >>>>>> - u32 btf_key_id, u32 btf_value_id)
> >>>>>> -{
> >>>>>> - const struct btf_type *key_type, *value_type;
> >>>>>> - u32 key_size, value_size;
> >>>>>> - u32 int_data;
> >>>>>> -
> >>>>>> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
> >>>>>> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
> >>>>>> - return -EINVAL;
> >>>>>> -
> >>>>>> - int_data = *(u32 *)(key_type + 1);
> >>>>>> - /* bpf array can only take a u32 key. This check makes
> >>>>>> - * sure that the btf matches the attr used during map_create.
> >>>>>> - */
> >>>>>> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
> >>>>>> - BTF_INT_OFFSET(int_data))
> >>>>>> - return -EINVAL;
> >>>>>
> >>>>> I think most of these checks are still necessary for array type.
> >>>>> Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
> >>>>> is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
> >>>>
> >>>> Hmm, so on 64 bit archs BTF_KIND_PTR would get rejected for array,
> >>>> on 32 bit it may be allowed due to sizeof(void *) == 4. BTF_KIND_ARRAY
> >>>> could be array of u8 foo[4], for example, or u16 foo[2]. But how would
> >>>> it ultimately be different from e.g. having 'struct a' versus 'struct b'
> >>>> where both are of same size and while actual key has 'struct a', the one
> >>>> who writes the prog resp. loads the BTF into the kernel would lie about
> >>>> it stating it's of type 'struct b' instead? It's basically trusting the
> >>>> app that it advertised sane key types which kernel is propagating back.
> >>>
> >>> for hash map - yes. the kernel cannot yet catch the lie that
> >>> key == 'struct a' that user said in BTF is not what program used
> >>> (which used 'struct b' of the same size).
> >>> Eventually we will annotate all load/store in the program and will
> >>> make sure that memory access match what BTF said.
> >>
> >> But in that case, would you reject the program? E.g. from prog point of
> >> view, it's just a buffer of x bytes, so key could be casted to different
> >> struct/types potentially and used for lookup; similar with value if you
> >> would go the route to annotate all access into it. I don't think this
> >> serves as a security feature (since you might as well just load the prog
> >> without BTF just fine), but it can be used to help verifier to perform
> >> rewrites like in tracing for implicit bpf_probe_read() based on member
> >> access. But also in that case, if you might have e.g. stale or wrong BTF
> >> data e.g. of the whole kernel or some application it would follow/walk
> >> that one instead. But such user error would be "acceptable" since it serves
> >> as a hint, roughly similar to (explicitly) walking the data structures
> >> based on the headers today, you do have better control in terms of header
> >> mismatches in that you can ship the BTF directly from the build, but there's
> >> still no guarantee in that sense that you "verified" that these bytes
> >> originally were mapped to struct foo somewhere in a C program.
> >
> > I wouldn't view such key checks as safety feature, but rather
> > as honesty check. Meaning that user space shouldn't be able to cheat
> > by passing completely bogus BTF into the kernel that only
> > statisfies single size check.
>
> Ok, meaning, we agree that BTF is passed as a hint to the kernel. It is
> a 'should not cheat' requirement but not 'cannot cheat', meaning if it
> was the latter, BTF is core part of the verifier's safety analysis /
> checks, but as it's optional on top of it (well, due to compatibility it
> kind of needs to be anyway), it doesn't affect kernel's safety in
> relation to what the program is doing internally.
>
> > Say, BTF has a key that is a 4 byte struct { char a,b,c,d; };
> > but program operates with it as u32. I very much would like
> > the verifier to notice and reject such program, since if C code
> > has struct { char a,b,c,d; }; the compiler won't generate u32 access
> > to it unless C code type casts, but then llvm will warn. So for u32
> > to legitimately appear in the generated code the struct should be:
> > union {
> > struct { char a,b,c,d;}
> > u32 e;
> > };
> > Narrow access (like u8 load/store in the bpf prog form u32 BTF field)
> > is ok, since that's normal compiler optimization, but any other
> > field/size mismatch the verifier should reject to prevent cheating.
> >
> > In other words even proprietary bpf programs should not be able to cheat.
> > If they provide BTF to the kernel, it should correspond exactly to the program.
> > That's the key to _trusted_ introspection.
>
> But then wouldn't this artificially force users into programming /
> thinking style that verifier dictates upon them similar as we have
> today as opposed to further moving away from it to allow more C-style
> programs to be accepted?
>
> But even if that is the goal, it is still used as a hint today, e.g.
> even for an array I could tell the compiler that the key is '__u32'
> for BTF sake while using the underlying key differently (e.g. as struct,
> enum, etc) in order to pass the array_map_check_btf() check in the
> kernel. Those type of programs would still need to be accepted due
> to compatibility reasons. Same if we only test on size match in other
> maps, it's nothing different. I don't see how verifier would start
> enforcing programs to be rejected based on access patterns on the
> types; while it might work for newly added program types that this
> could be enforced, it cannot for all the many existing program types,
> but I also don't really think it needs to be. Unless it is declared
> a safety feature for future program types.
>
> > Admin that ssh-es into the box and operates with bpftool should be
> > certain that the map introspection represent the real situation of
> > the program. If proprietary prog is paranoid about layout of map
> > it shouldn't be using BTF, but if it does, BTF should match.
> > In the future I'd like to enfore availability of BTF for
> > new program types.
>
> Sure, and in vast majority / normal cases it will represent the real
> situation. Do you mistrust DWARF debugging data that gets shipped with
> your distro, for example? pahole and other tools on top of it? It is
> pretty much the same situation. Given gdb example below, does gdb do
> any verification of the binary in order to analyze load/store patterns
> and whether it matches with what sits in DWARF as types? It would just
> as well map the types into memory and dump it as pretty print instead.
> And that's okay, the one thing that needs to be verified for correctness
> resp. validity is the debugging format itself so it can be used for
> that.
I think decoupling map_seq_show_elem() requirement from BTF support
during map_create() is ok. There is plan to do that going forward if
it ever hits some map types that is difficult to do map_seq_show_elem().
However, I think most of the maps should have pretty straight
forward seq_show implementation considering most of the
heavy lifting has already been done in btf_*_seq_show()
and bpffs_map_seq_ops.
Regarding one general map_check_btf() for all maps' purpose,
I think not all bpf maps are behaving the same.
There are some property that the kernel implies on a particular
map type. Some of them even have kernel generated value in it.
For example, map-in-map, the lookup_elem() value is a map's id
which must be an integer generated by kernel's idr_alloc_cyclic().
Treating it as enum/struct/array...etc seems wrong.
Hence, I think each map type should be able to provide
its own map_check_btf(). If a map does not have
specific key/value requirement, it can use the default one which
is the min requirement for kernel safety reason.
The map can provide a more strict requirement first and relax it
later when use cases come up.
>
> >>> For array we can catch the lie today that key is not 4 byte int,
> >>> since it matters from pretty printing point of view.
> >>> If it's PTR or ARRAY or STRUCT, the printer will go nuts.
> >>
> >> In that case, would you enforce a hash map key size of 4 also to INT-only
> >> instead of e.g. allowing STRUCT and various others?
> >
> > hash map key of 4 bytes can be anything and printing is ideally
> > done similar to the way gdb prints stl c++:
> > #include <map>
> >
> > struct K {
> > int a, b;
> > };
> > struct V {
> > int c, d;
> > };
> > int operator<(const K& a, const K& b) {return a.a < b.a;}
> > std::map<K, V> m;
> >
> > int main()
> > {
> > K k1 = {1, 2};
> > K k2 = {3, 4};
> > V v1 = {5, 6};
> > V v2 = {7, 8};
> > m[k1] = v1;
> > m[k2] = v2;
> > ...
> > (gdb) p m
> > $3 = std::map with 2 elements = {[{a = 1, b = 2}] = {c = 5, d = 6}, [{a = 3, b = 4}] = {c = 7, d = 8}}
> > (gdb) set print pretty on
> > (gdb) p m
> > $4 = std::map with 2 elements = {
> > [{
> > a = 1,
> > b = 2
> > }] = {
> > c = 5,
> > d = 6
> > },
> > [{
> > a = 3,
> > b = 4
> > }] = {
> > c = 7,
> > d = 8
> > }
> > }
>
^ permalink raw reply
* Re: [PATCH] mlxsw: remove unused arrays mlxsw_i2c_driver_name and mlxsw_pci_driver_name
From: Ido Schimmel @ 2018-08-10 15:23 UTC (permalink / raw)
To: Colin King
Cc: Jiri Pirko, David S . Miller, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20180810075328.26400-1-colin.king@canonical.com>
On Fri, Aug 10, 2018 at 08:53:28AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Arrays mlxsw_i2c_driver_name and mlxsw_pci_driver_name are defined
> but never used hence they are redundant and can be removed.
>
> Cleans up clang warnings:
> warning: 'mlxsw_i2c_driver_name' defined but not used
> warning: 'mlxsw_pci_driver_name' defined but not used
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Thanks!
^ permalink raw reply
* [PATCH v2 bpf-next] BPF: helpers: New helper to obtain namespace data from current task
From: Carlos Neira @ 2018-08-10 12:41 UTC (permalink / raw)
To: netdev; +Cc: ebiederm, quentin.monnet, ys114321, brouer, Carlos Neira
This helper obtains the active namespace from current and returns pid, tgid,
device and namespace id as seen from that namespace, allowing to instrument
a process inside a container.
Device is read from /proc/self/ns/pid, as in the future it's possible that
different pid_ns files may belong to different devices, according
to the discussion between Eric Biederman and Yonghong in 2017 linux plumbers
conference.
Currently bpf_get_current_pid_tgid(), is used to do pid filtering in bcc's
scripts but this helper returns the pid as seen by the root namespace which is
fine when a bcc script is not executed inside a container.
When the process of interest is inside a container, pid filtering will not work
if bpf_get_current_pid_tgid() is used. This helper addresses this limitation
returning the pid as it's seen by the current namespace where the script is
executing.
This helper has the same use cases as bpf_get_current_pid_tgid() as it can be
used to do pid filtering even inside a container.
For example a bcc script using bpf_get_current_pid_tgid() (tools/funccount.py):
u32 pid = bpf_get_current_pid_tgid() >> 32;
if (pid != <pid_arg_passed_in>)
return 0;
Could be modified to use bpf_get_current_pidns_info() as follows:
struct bpf_pidns pidns;
bpf_get_current_pidns_info(&pidns, sizeof(struct bpf_pidns));
u32 pid = pidns.tgid;
u32 nsid = pidns.nsid;
if ((pid != <pid_arg_passed_in>) && (nsid != <nsid_arg_passed_in>))
return 0;
To find out the name PID namespace id of a process, you could use this command:
$ ps -h -o pidns -p <pid_of_interest>
Or this other command:
$ ls -Li /proc/<pid_of_interest>/ns/pid
Signed-off-by: Carlos Antonio Neira Bustos <cneirabustos@gmail.com>
---
include/linux/bpf.h | 1 +
include/uapi/linux/bpf.h | 24 +++++++++++-
kernel/bpf/core.c | 1 +
kernel/bpf/helpers.c | 64 +++++++++++++++++++++++++++++++
kernel/trace/bpf_trace.c | 2 +
samples/bpf/Makefile | 3 ++
samples/bpf/trace_ns_info_user.c | 35 +++++++++++++++++
samples/bpf/trace_ns_info_user_kern.c | 45 ++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 24 +++++++++++-
tools/testing/selftests/bpf/bpf_helpers.h | 3 ++
10 files changed, 200 insertions(+), 2 deletions(-)
create mode 100644 samples/bpf/trace_ns_info_user.c
create mode 100644 samples/bpf/trace_ns_info_user_kern.c
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index cd8790d2c6ed..3f4b999f7c99 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -787,6 +787,7 @@ extern const struct bpf_func_proto bpf_get_stack_proto;
extern const struct bpf_func_proto bpf_sock_map_update_proto;
extern const struct bpf_func_proto bpf_sock_hash_update_proto;
extern const struct bpf_func_proto bpf_get_current_cgroup_id_proto;
+extern const struct bpf_func_proto bpf_get_current_pidns_info_proto;
extern const struct bpf_func_proto bpf_get_local_storage_proto;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index dd5758dc35d3..8462f9881465 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2113,6 +2113,18 @@ union bpf_attr {
* the shared data.
* Return
* Pointer to the local storage area.
+ *
+ * int bpf_get_current_pidns_info(struct bpf_pidns_info *pidns, u32 size_of_pidns)
+ * Description
+ * Copies into *pidns* pid, namespace id and tgid as seen by the
+ * current namespace and also device from /proc/self/ns/pid.
+ * *size_of_pidns* must be the size of *pidns*
+ *
+ * This helper is used when pid filtering is needed inside a
+ * container as bpf_get_current_tgid() helper returns always the
+ * pid id as seen by the root namespace.
+ * Return
+ * 0 on success -EINVAL on error.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2196,7 +2208,8 @@ union bpf_attr {
FN(rc_keydown), \
FN(skb_cgroup_id), \
FN(get_current_cgroup_id), \
- FN(get_local_storage),
+ FN(get_local_storage), \
+ FN(get_current_pidns_info),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -2724,4 +2737,13 @@ enum bpf_task_fd_type {
BPF_FD_TYPE_URETPROBE, /* filename + offset */
};
+/* helper bpf_get_current_pidns_info will store the following
+ * data, dev will contain major/minor from /proc/self/ns/pid.
+ */
+struct bpf_pidns_info {
+ __u32 dev;
+ __u32 nsid;
+ __u32 tgid;
+ __u32 pid;
+};
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 4d09e610777f..98ce53ce2ea6 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1796,6 +1796,7 @@ const struct bpf_func_proto bpf_sock_map_update_proto __weak;
const struct bpf_func_proto bpf_sock_hash_update_proto __weak;
const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_get_current_pidns_info __weak;
const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
{
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1991466b8327..d06d723b9cff 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -18,6 +18,9 @@
#include <linux/sched.h>
#include <linux/uidgid.h>
#include <linux/filter.h>
+#include <linux/pid_namespace.h>
+#include <linux/major.h>
+#include <linux/stat.h>
/* If kernel subsystem is allowing eBPF programs to call this function,
* inside its own verifier_ops->get_func_proto() callback it should return
@@ -214,3 +217,64 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
.arg2_type = ARG_ANYTHING,
};
#endif
+
+BPF_CALL_2(bpf_get_current_pidns_info, struct bpf_pidns_info *, pidns_info, u32,
+ size)
+{
+ const char *ppath = "/proc/self/ns/pid";
+ struct pid_namespace *pidns = NULL;
+ mm_segment_t oldsegfs;
+ struct kstat stat;
+ pid_t tgid = 0;
+ pid_t pid = 0;
+ int res = 0;
+
+ if (unlikely(size != sizeof(struct bpf_pidns_info)))
+ goto clear;
+
+ pidns = task_active_pid_ns(current);
+
+ if (unlikely(!pidns))
+ goto clear;
+
+ pidns_info->nsid = pidns->ns.inum;
+ pid = task_pid_nr_ns(current, pidns);
+
+ if (unlikely(!pid))
+ goto clear;
+
+ tgid = task_tgid_nr_ns(current, pidns);
+
+ if (unlikely(!tgid))
+ goto clear;
+
+ pidns_info->tgid = (u32) tgid;
+ pidns_info->pid = (u32) pid;
+
+ oldsegfs = get_fs();
+ set_fs(KERNEL_DS);
+ res = vfs_stat((const char __user *)ppath, &stat);
+ set_fs(oldsegfs);
+
+ if (unlikely(res))
+ goto clear;
+
+ pidns_info->dev = stat.dev;
+
+ return 0;
+
+clear:
+ if (pidns_info)
+ memset((void *)pidns, 0, (size_t) size);
+
+ return -EINVAL;
+}
+
+const struct bpf_func_proto bpf_get_current_pidns_info_proto = {
+ .func = bpf_get_current_pidns_info,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg2_type = ARG_CONST_SIZE,
+};
+
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0ae6829804bc..f70be29e49ab 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -568,6 +568,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_get_current_cgroup_id:
return &bpf_get_current_cgroup_id_proto;
#endif
+ case BPF_FUNC_get_current_pidns_info:
+ return &bpf_get_current_pidns_info_proto;
default:
return NULL;
}
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index f88d5683d6ee..fdcde00554ce 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -53,6 +53,7 @@ hostprogs-y += xdpsock
hostprogs-y += xdp_fwd
hostprogs-y += task_fd_query
hostprogs-y += xdp_sample_pkts
+hostprogs-y += trace_ns_info
# Libbpf dependencies
LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
@@ -109,6 +110,7 @@ xdpsock-objs := xdpsock_user.o
xdp_fwd-objs := xdp_fwd_user.o
task_fd_query-objs := bpf_load.o task_fd_query_user.o $(TRACE_HELPERS)
xdp_sample_pkts-objs := xdp_sample_pkts_user.o $(TRACE_HELPERS)
+trace_ns_info-objs := bpf_load.o trace_ns_info_user.o
# Tell kbuild to always build the programs
always := $(hostprogs-y)
@@ -166,6 +168,7 @@ always += xdpsock_kern.o
always += xdp_fwd_kern.o
always += task_fd_query_kern.o
always += xdp_sample_pkts_kern.o
+always += trace_ns_info_user_kern.o
HOSTCFLAGS += -I$(objtree)/usr/include
HOSTCFLAGS += -I$(srctree)/tools/lib/
diff --git a/samples/bpf/trace_ns_info_user.c b/samples/bpf/trace_ns_info_user.c
new file mode 100644
index 000000000000..e06d08db6f30
--- /dev/null
+++ b/samples/bpf/trace_ns_info_user.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018 Carlos Neira cneirabustos@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+
+#include <stdio.h>
+#include <linux/bpf.h>
+#include <unistd.h>
+#include "bpf/libbpf.h"
+#include "bpf_load.h"
+
+/* This code was taken verbatim from tracex1_user.c, it's used
+ * to exercize bpf_get_current_pidns_info() helper call.
+ */
+int main(int ac, char **argv)
+{
+ FILE *f;
+ char filename[256];
+
+ snprintf(filename, sizeof(filename), "%s_user_kern.o", argv[0]);
+ printf("loading %s\n", filename);
+
+ if (load_bpf_file(filename)) {
+ printf("%s", bpf_log_buf);
+ return 1;
+ }
+
+ f = popen("taskset 1 ping localhost", "r");
+ (void) f;
+ read_trace_pipe();
+ return 0;
+}
diff --git a/samples/bpf/trace_ns_info_user_kern.c b/samples/bpf/trace_ns_info_user_kern.c
new file mode 100644
index 000000000000..ceaf3e83c8e7
--- /dev/null
+++ b/samples/bpf/trace_ns_info_user_kern.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018 Carlos Neira cneirabustos@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/version.h>
+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+
+typedef __u64 u64;
+typedef __u32 u32;
+
+
+/* kprobe is NOT a stable ABI
+ * kernel functions can be removed, renamed or completely change semantics.
+ * Number of arguments and their positions can change, etc.
+ * In such case this bpf+kprobe example will no longer be meaningful
+ */
+
+/* This will call bpf_get_current_pidns_info() to display pid and ns values
+ * as seen by the current namespace, on the far left you will see the pid as
+ * seen as by the root namespace.
+ */
+
+SEC("kprobe/__netif_receive_skb_core")
+int bpf_prog1(struct pt_regs *ctx)
+{
+ char fmt[] = "nsid:%u, dev: %u, pid:%u\n";
+ struct bpf_pidns_info nsinfo;
+ int ok = 0;
+
+ ok = bpf_get_current_pidns_info(&nsinfo, sizeof(nsinfo));
+ if (ok == 0)
+ bpf_trace_printk(fmt, sizeof(fmt), (u32)nsinfo.nsid,
+ (u32) nsinfo.dev, (u32)nsinfo.pid);
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index dd5758dc35d3..8462f9881465 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2113,6 +2113,18 @@ union bpf_attr {
* the shared data.
* Return
* Pointer to the local storage area.
+ *
+ * int bpf_get_current_pidns_info(struct bpf_pidns_info *pidns, u32 size_of_pidns)
+ * Description
+ * Copies into *pidns* pid, namespace id and tgid as seen by the
+ * current namespace and also device from /proc/self/ns/pid.
+ * *size_of_pidns* must be the size of *pidns*
+ *
+ * This helper is used when pid filtering is needed inside a
+ * container as bpf_get_current_tgid() helper returns always the
+ * pid id as seen by the root namespace.
+ * Return
+ * 0 on success -EINVAL on error.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2196,7 +2208,8 @@ union bpf_attr {
FN(rc_keydown), \
FN(skb_cgroup_id), \
FN(get_current_cgroup_id), \
- FN(get_local_storage),
+ FN(get_local_storage), \
+ FN(get_current_pidns_info),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -2724,4 +2737,13 @@ enum bpf_task_fd_type {
BPF_FD_TYPE_URETPROBE, /* filename + offset */
};
+/* helper bpf_get_current_pidns_info will store the following
+ * data, dev will contain major/minor from /proc/self/ns/pid.
+ */
+struct bpf_pidns_info {
+ __u32 dev;
+ __u32 nsid;
+ __u32 tgid;
+ __u32 pid;
+};
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index cb9fcfbc9307..c3060f3284e0 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -137,6 +137,9 @@ static unsigned long long (*bpf_get_current_cgroup_id)(void) =
(void *) BPF_FUNC_get_current_cgroup_id;
static void *(*bpf_get_local_storage)(void *map, unsigned long long flags) =
(void *) BPF_FUNC_get_local_storage;
+static int (*bpf_get_current_pidns_info)(struct bpf_pidns_info *buf,
+ unsigned int buf_size) =
+ (void *) BPF_FUNC_get_current_pidns_info;
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
--
2.11.0
^ permalink raw reply related
* [PATCH] ICMPv6: Add icmp_echo_ignore_all support for ICMPv6
From: Virgile Jarry @ 2018-08-10 14:36 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
linux-kernel
Preventing the kernel from responding to ICMP Echo Requests messages
can be useful in several ways. The sysctl parameter
'icmp_echo_ignore_all' can be used to prevent the kernel from
responding to IPv4 ICMP echo requests. For IPv6 pings, such
a sysctl kernel parameter did not exist.
Add the ability to prevent the kernel from responding to IPv6
ICMP echo requests through the use of the following sysctl
parameter : /proc/sys/net/ipv6/icmp/echo_ignore_all.
Update the documentation to reflect this change.
Signed-off-by: Virgile Jarry <virgile@acceis.fr>
---
Documentation/networking/ip-sysctl.txt | 5 +++++
include/net/netns/ipv6.h | 1 +
net/ipv6/af_inet6.c | 1 +
net/ipv6/icmp.c | 16 +++++++++++++---
4 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index ce8fbf5..5042a13 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1863,6 +1863,11 @@ ratelimit - INTEGER
otherwise the minimal space between responses in milliseconds.
Default: 1000
+echo_ignore_all - BOOLEAN
+ If set non-zero, then the kernel will ignore all ICMP ECHO
+ requests sent to it over the IPv6 protocol.
+ Default: 0
+
xfrm6_gc_thresh - INTEGER
The threshold at which we will start garbage collecting for IPv6
destination cache entries. At twice this value the system will
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 762ac99..f0e396a 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -32,6 +32,7 @@ struct netns_sysctl_ipv6 {
int flowlabel_consistency;
int auto_flowlabels;
int icmpv6_time;
+ int icmpv6_echo_ignore_all;
int anycast_src_echo_reply;
int ip_nonlocal_bind;
int fwmark_reflect;
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 9ed0eae..d0033b3 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -833,6 +833,7 @@ static int __net_init inet6_net_init(struct net *net)
net->ipv6.sysctl.bindv6only = 0;
net->ipv6.sysctl.icmpv6_time = 1*HZ;
+ net->ipv6.sysctl.icmpv6_echo_ignore_all = 0;
net->ipv6.sysctl.flowlabel_consistency = 1;
net->ipv6.sysctl.auto_flowlabels = IP6_DEFAULT_AUTO_FLOW_LABELS;
net->ipv6.sysctl.idgen_retries = 3;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index ef2505a..1d869f1 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -800,6 +800,7 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
static int icmpv6_rcv(struct sk_buff *skb)
{
+ struct net *net = dev_net(skb->dev);
struct net_device *dev = skb->dev;
struct inet6_dev *idev = __in6_dev_get(dev);
const struct in6_addr *saddr, *daddr;
@@ -849,7 +850,8 @@ static int icmpv6_rcv(struct sk_buff *skb)
switch (type) {
case ICMPV6_ECHO_REQUEST:
- icmpv6_echo_reply(skb);
+ if (!net->ipv6.sysctl.icmpv6_echo_ignore_all)
+ icmpv6_echo_reply(skb);
break;
case ICMPV6_ECHO_REPLY:
@@ -1110,6 +1112,13 @@ static struct ctl_table ipv6_icmp_table_template[] = {
.mode = 0644,
.proc_handler = proc_dointvec_ms_jiffies,
},
+ {
+ .procname = "echo_ignore_all",
+ .data = &init_net.ipv6.sysctl.icmpv6_echo_ignore_all,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
{ },
};
@@ -1121,9 +1130,10 @@ struct ctl_table * __net_init ipv6_icmp_sysctl_init(struct net *net)
sizeof(ipv6_icmp_table_template),
GFP_KERNEL);
- if (table)
+ if (table) {
table[0].data = &net->ipv6.sysctl.icmpv6_time;
-
+ table[1].data = &net->ipv6.sysctl.icmpv6_echo_ignore_all;
+ }
return table;
}
#endif
--
2.7.4
^ permalink raw reply related
* [bpf-next V2 PATCH 2/2] samples/bpf: xdp_redirect_cpu load balance like Suricata
From: Jesper Dangaard Brouer @ 2018-08-10 12:03 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer
Cc: victor, eric, Daniel Borkmann, Alexei Starovoitov, jhsiao
In-Reply-To: <153390254528.10825.9125109091513926669.stgit@firesoul>
From: Jesper Dangaard Brouer <brouer@redhat.com>
This implement XDP CPU redirection load-balancing across available
CPUs, based on the hashing IP-pairs + L4-protocol. This equivalent to
xdp-cpu-redirect feature in Suricata, which is inspired by the
Suricata 'ippair' hashing code.
An important property is that the hashing is flow symmetric, meaning
that if the source and destination gets swapped then the selected CPU
will remain the same. This is helps locality by placing both directions
of a flows on the same CPU, in a forwarding/routing scenario.
The hashing INITVAL (15485863 the 10^6th prime number) was fairly
arbitrary choosen, but experiments with kernel tree pktgen scripts
(pktgen_sample04_many_flows.sh +pktgen_sample05_flow_per_thread.sh)
showed this improved the distribution.
This patch also change the default loaded XDP program to be this
load-balancer. As based on different user feedback, this seems to be
the expected behavior of the sample xdp_redirect_cpu.
Link: https://github.com/OISF/suricata/commit/796ec08dd7a63
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
samples/bpf/xdp_redirect_cpu_kern.c | 103 +++++++++++++++++++++++++++++++++++
samples/bpf/xdp_redirect_cpu_user.c | 4 +
2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/samples/bpf/xdp_redirect_cpu_kern.c b/samples/bpf/xdp_redirect_cpu_kern.c
index 8cb703671b04..081ef4bb4fe3 100644
--- a/samples/bpf/xdp_redirect_cpu_kern.c
+++ b/samples/bpf/xdp_redirect_cpu_kern.c
@@ -13,6 +13,7 @@
#include <uapi/linux/bpf.h>
#include "bpf_helpers.h"
+#include "hash_func01.h"
#define MAX_CPUS 12 /* WARNING - sync with _user.c */
@@ -461,6 +462,108 @@ int xdp_prognum4_ddos_filter_pktgen(struct xdp_md *ctx)
return bpf_redirect_map(&cpu_map, cpu_dest, 0);
}
+/* Hashing initval */
+#define INITVAL 15485863
+
+static __always_inline
+u32 get_ipv4_hash_ip_pair(struct xdp_md *ctx, u64 nh_off)
+{
+ void *data_end = (void *)(long)ctx->data_end;
+ void *data = (void *)(long)ctx->data;
+ struct iphdr *iph = data + nh_off;
+ u32 cpu_hash;
+
+ if (iph + 1 > data_end)
+ return 0;
+
+ cpu_hash = iph->saddr + iph->daddr;
+ cpu_hash = SuperFastHash((char *)&cpu_hash, 4, INITVAL + iph->protocol);
+
+ return cpu_hash;
+}
+
+static __always_inline
+u32 get_ipv6_hash_ip_pair(struct xdp_md *ctx, u64 nh_off)
+{
+ void *data_end = (void *)(long)ctx->data_end;
+ void *data = (void *)(long)ctx->data;
+ struct ipv6hdr *ip6h = data + nh_off;
+ u32 cpu_hash;
+
+ if (ip6h + 1 > data_end)
+ return 0;
+
+ cpu_hash = ip6h->saddr.s6_addr32[0] + ip6h->daddr.s6_addr32[0];
+ cpu_hash += ip6h->saddr.s6_addr32[1] + ip6h->daddr.s6_addr32[1];
+ cpu_hash += ip6h->saddr.s6_addr32[2] + ip6h->daddr.s6_addr32[2];
+ cpu_hash += ip6h->saddr.s6_addr32[3] + ip6h->daddr.s6_addr32[3];
+ cpu_hash = SuperFastHash((char *)&cpu_hash, 4, INITVAL + ip6h->nexthdr);
+
+ return cpu_hash;
+}
+
+/* Load-Balance traffic based on hashing IP-addrs + L4-proto. The
+ * hashing scheme is symmetric, meaning swapping IP src/dest still hit
+ * same CPU.
+ */
+SEC("xdp_cpu_map5_lb_hash_ip_pairs")
+int xdp_prognum5_lb_hash_ip_pairs(struct xdp_md *ctx)
+{
+ void *data_end = (void *)(long)ctx->data_end;
+ void *data = (void *)(long)ctx->data;
+ struct ethhdr *eth = data;
+ u8 ip_proto = IPPROTO_UDP;
+ struct datarec *rec;
+ u16 eth_proto = 0;
+ u64 l3_offset = 0;
+ u32 cpu_dest = 0;
+ u32 cpu_idx = 0;
+ u32 *cpu_lookup;
+ u32 *cpu_max;
+ u32 cpu_hash;
+ u32 key = 0;
+
+ /* Count RX packet in map */
+ rec = bpf_map_lookup_elem(&rx_cnt, &key);
+ if (!rec)
+ return XDP_ABORTED;
+ rec->processed++;
+
+ cpu_max = bpf_map_lookup_elem(&cpus_count, &key);
+ if (!cpu_max)
+ return XDP_ABORTED;
+
+ if (!(parse_eth(eth, data_end, ð_proto, &l3_offset)))
+ return XDP_PASS; /* Just skip */
+
+ /* Hash for IPv4 and IPv6 */
+ switch (eth_proto) {
+ case ETH_P_IP:
+ cpu_hash = get_ipv4_hash_ip_pair(ctx, l3_offset);
+ break;
+ case ETH_P_IPV6:
+ cpu_hash = get_ipv6_hash_ip_pair(ctx, l3_offset);
+ break;
+ case ETH_P_ARP: /* ARP packet handled on CPU idx 0 */
+ default:
+ cpu_hash = 0;
+ }
+
+ /* Choose CPU based on hash */
+ cpu_idx = cpu_hash % *cpu_max;
+
+ cpu_lookup = bpf_map_lookup_elem(&cpus_available, &cpu_idx);
+ if (!cpu_lookup)
+ return XDP_ABORTED;
+ cpu_dest = *cpu_lookup;
+
+ if (cpu_dest >= MAX_CPUS) {
+ rec->issue++;
+ return XDP_ABORTED;
+ }
+
+ return bpf_redirect_map(&cpu_map, cpu_dest, 0);
+}
char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index f6efaefd485b..007710d7c748 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -22,7 +22,7 @@ static const char *__doc__ =
#define MAX_CPUS 12 /* WARNING - sync with _kern.c */
/* How many xdp_progs are defined in _kern.c */
-#define MAX_PROG 5
+#define MAX_PROG 6
/* Wanted to get rid of bpf_load.h and fake-"libbpf.h" (and instead
* use bpf/libbpf.h), but cannot as (currently) needed for XDP
@@ -567,7 +567,7 @@ int main(int argc, char **argv)
int added_cpus = 0;
int longindex = 0;
int interval = 2;
- int prog_num = 0;
+ int prog_num = 5;
int add_cpu = -1;
__u32 qsize;
int opt;
^ permalink raw reply related
* [bpf-next V2 PATCH 1/2] samples/bpf: add Paul Hsieh's (LGPL 2.1) hash function SuperFastHash
From: Jesper Dangaard Brouer @ 2018-08-10 12:02 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer
Cc: victor, eric, Daniel Borkmann, Alexei Starovoitov, jhsiao
In-Reply-To: <153390254528.10825.9125109091513926669.stgit@firesoul>
Adjusted function call API to take an initval. This allow the API
user to set the initial value, as a seed. This could also be used for
inputting the previous hash.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
samples/bpf/hash_func01.h | 55 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 samples/bpf/hash_func01.h
diff --git a/samples/bpf/hash_func01.h b/samples/bpf/hash_func01.h
new file mode 100644
index 000000000000..38255812e376
--- /dev/null
+++ b/samples/bpf/hash_func01.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: LGPL-2.1
+ *
+ * Based on Paul Hsieh's (LGPG 2.1) hash function
+ * From: http://www.azillionmonkeys.com/qed/hash.html
+ */
+
+#define get16bits(d) (*((const __u16 *) (d)))
+
+static __always_inline
+__u32 SuperFastHash (const char *data, int len, __u32 initval) {
+ __u32 hash = initval;
+ __u32 tmp;
+ int rem;
+
+ if (len <= 0 || data == NULL) return 0;
+
+ rem = len & 3;
+ len >>= 2;
+
+ /* Main loop */
+#pragma clang loop unroll(full)
+ for (;len > 0; len--) {
+ hash += get16bits (data);
+ tmp = (get16bits (data+2) << 11) ^ hash;
+ hash = (hash << 16) ^ tmp;
+ data += 2*sizeof (__u16);
+ hash += hash >> 11;
+ }
+
+ /* Handle end cases */
+ switch (rem) {
+ case 3: hash += get16bits (data);
+ hash ^= hash << 16;
+ hash ^= ((signed char)data[sizeof (__u16)]) << 18;
+ hash += hash >> 11;
+ break;
+ case 2: hash += get16bits (data);
+ hash ^= hash << 11;
+ hash += hash >> 17;
+ break;
+ case 1: hash += (signed char)*data;
+ hash ^= hash << 10;
+ hash += hash >> 1;
+ }
+
+ /* Force "avalanching" of final 127 bits */
+ hash ^= hash << 3;
+ hash += hash >> 5;
+ hash ^= hash << 4;
+ hash += hash >> 17;
+ hash ^= hash << 25;
+ hash += hash >> 6;
+
+ return hash;
+}
^ permalink raw reply related
* [bpf-next V2 PATCH 0/2] Implement sample code for XDP cpumap IP-pair load-balancing
From: Jesper Dangaard Brouer @ 2018-08-10 12:02 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer
Cc: victor, eric, Daniel Borkmann, Alexei Starovoitov, jhsiao
Background: cpumap moves the SKB allocation out of the driver code,
and instead allocate it on the remote CPU, and invokes the regular
kernel network stack with the newly allocated SKB.
The idea behind the XDP CPU redirect feature, is to use XDP as a
load-balancer step in-front of regular kernel network stack. But the
current sample code does not provide a good example of this. Part of
the reason is that, I have implemented this as part of Suricata XDP
load-balancer.
Given this is the most frequent feature request I get. This patchset
implement the same XDP load-balancing as Suricata does, which is a
symmetric hash based on the IP-pairs + L4-protocol.
The expected setup for the use-case is to reduce the number of NIC RX
queues via ethtool (as XDP can handle more per core), and via
smp_affinity assign these RX queues to a set of CPUs, which will be
handling RX packets. The CPUs that runs the regular network stack is
supplied to the sample xdp_redirect_cpu tool by specifying
the --cpu option multiple times on the cmdline.
I do note that cpumap SKB creation is not feature complete yet, and
more work is coming. E.g. given GRO is not implemented yet, do expect
TCP workloads to be slower. My measurements do indicate UDP workloads
are faster.
---
Jesper Dangaard Brouer (2):
samples/bpf: add Paul Hsieh's (LGPL 2.1) hash function SuperFastHash
samples/bpf: xdp_redirect_cpu load balance like Suricata
samples/bpf/hash_func01.h | 55 +++++++++++++++++++
samples/bpf/xdp_redirect_cpu_kern.c | 103 +++++++++++++++++++++++++++++++++++
samples/bpf/xdp_redirect_cpu_user.c | 4 +
3 files changed, 160 insertions(+), 2 deletions(-)
create mode 100644 samples/bpf/hash_func01.h
^ permalink raw reply
* [PATCH iproute2/net-next v2] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
From: Eelco Chaudron @ 2018-08-10 11:59 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen
Add support for showing hardware specific counters to easy
troubleshooting hardware offload.
$ tc -s filter show dev enp3s0np0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
eth_type ipv4
dst_ip 2.0.0.0
src_ip 1.0.0.0
ip_flags nofrag
in_hw
action order 1: mirred (Egress Redirect to device eth1) stolen
index 1 ref 1 bind 1 installed 0 sec used 0 sec
Action statistics:
Sent 534884742 bytes 8915697 pkt (dropped 0, overlimits 0 requeues 0)
Sent software 187542 bytes 4077 pkt
Sent hardware 534697200 bytes 8911620 pkt
backlog 0b 0p requeues 0
cookie 89173e6a44447001becfd486bda17e29
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
v2:
* Removed unnecessary initialization
* Made not displaying of missing TCA_STATS_BASIC_HW more obvious
* Use _SL_ macro for single line output
include/uapi/linux/gen_stats.h | 1 +
tc/tc_util.c | 41 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
diff --git a/include/uapi/linux/gen_stats.h b/include/uapi/linux/gen_stats.h
index 24a861c..065408e 100644
--- a/include/uapi/linux/gen_stats.h
+++ b/include/uapi/linux/gen_stats.h
@@ -12,6 +12,7 @@ enum {
TCA_STATS_APP,
TCA_STATS_RATE_EST64,
TCA_STATS_PAD,
+ TCA_STATS_BASIC_HW,
__TCA_STATS_MAX,
};
#define TCA_STATS_MAX (__TCA_STATS_MAX - 1)
diff --git a/tc/tc_util.c b/tc/tc_util.c
index d757852..5a1bbf2 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -800,6 +800,44 @@ void print_tm(FILE *f, const struct tcf_t *tm)
}
}
+static void print_tcstats_basic_hw(struct rtattr **tbs, char *prefix)
+{
+ struct gnet_stats_basic bs_hw;
+
+ if (!tbs[TCA_STATS_BASIC_HW])
+ return;
+
+ memcpy(&bs_hw, RTA_DATA(tbs[TCA_STATS_BASIC_HW]),
+ MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC_HW]), sizeof(bs_hw)));
+
+ if (bs_hw.bytes == 0 && bs_hw.packets == 0)
+ return;
+
+ if (tbs[TCA_STATS_BASIC]) {
+ struct gnet_stats_basic bs;
+
+ memcpy(&bs, RTA_DATA(tbs[TCA_STATS_BASIC]),
+ MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC]),
+ sizeof(bs)));
+
+ if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
+ print_string(PRINT_FP, NULL, "%s", _SL_);
+ print_string(PRINT_FP, NULL, "%s", prefix);
+ print_lluint(PRINT_ANY, "sw_bytes",
+ "Sent software %llu bytes",
+ bs.bytes - bs_hw.bytes);
+ print_uint(PRINT_ANY, "sw_packets", " %u pkt",
+ bs.packets - bs_hw.packets);
+ }
+ }
+
+ print_string(PRINT_FP, NULL, "%s", _SL_);
+ print_string(PRINT_FP, NULL, "%s", prefix);
+ print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
+ bs_hw.bytes);
+ print_uint(PRINT_ANY, "hw_packets", " %u pkt", bs_hw.packets);
+}
+
void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtattr **xstats)
{
SPRINT_BUF(b1);
@@ -826,6 +864,9 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtat
print_uint(PRINT_ANY, "requeues", " requeues %u) ", q.requeues);
}
+ if (tbs[TCA_STATS_BASIC_HW])
+ print_tcstats_basic_hw(tbs, prefix);
+
if (tbs[TCA_STATS_RATE_EST64]) {
struct gnet_stats_rate_est64 re = {0};
^ permalink raw reply related
* Re: [BUG] bpf: syscall: a possible sleep-in-atomic-context bug in map_update_elem()
From: Daniel Borkmann @ 2018-08-10 14:22 UTC (permalink / raw)
To: Jia-Ju Bai, ast; +Cc: netdev, Linux Kernel Mailing List, brouer
In-Reply-To: <65830741-bf35-4d32-e365-c32fc17c25cb@gmail.com>
On 08/10/2018 04:07 PM, Jia-Ju Bai wrote:
> The kernel may sleep with holding a rcu read lock.
>
> The function call paths (from bottom to top) in Linux-4.16 are:
>
> [FUNC] kmalloc(GFP_KERNEL)
> kernel/kthread.c, 283: kmalloc in __kthread_create_on_node
> kernel/kthread.c, 365: __kthread_create_on_node in kthread_create_on_node
> kernel/bpf/cpumap.c, 368: kthread_create_on_node in __cpu_map_entry_alloc
> kernel/bpf/cpumap.c, 490: __cpu_map_entry_alloc in cpu_map_update_elem
> kernel/bpf/syscall.c, 724: [FUNC_PTR]cpu_map_update_elem in map_update_elem
> kernel/bpf/syscall.c, 723: rcu_read_lock in map_update_elem
>
> Note that [FUNC_PTR] means a function pointer call is used.
>
> I do not find a good way to fix it, so I only report.
> This is found by my static analysis tool (DSAC).
Thanks for the report Jia-Ju! In the map_update_elem() from syscall
path there's a check map->map_type == BPF_MAP_TYPE_CPUMAP, where we
call the cpumap's map->ops->map_update_elem() while /not/ being under
rcu_read_lock() as in other cases, so looks okay to me. Could you point
out the case for being under rcu_read_lock() more specifically which
the tool found?
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: William Tu @ 2018-08-10 14:11 UTC (permalink / raw)
To: Stefano Brivio
Cc: <dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org>,
Justin Pettit, netdev, Jiri Benc, Joe Stringer
In-Reply-To: <20180804024324.4d900b5e@epycfail>
On Fri, Aug 3, 2018 at 5:43 PM, Stefano Brivio <sbrivio-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 3 Aug 2018 16:01:08 -0700
> Ben Pfaff <blp-LZ6Gd1LRuIk@public.gmane.org> wrote:
>
> > I think that a simple mechanism for fairness is fine. The direction
> > of extensibility that makes me anxious is how to decide what matters
> > for fairness. So far, we've talked about per-vport fairness. That
> > works pretty well for packets coming in from virtual interfaces where
> > each vport represents a separate VM.
>
> Yes, right, that's the case where we have significant issues currently.
>
> > It does not work well if the traffic filling your queues all comes
> > from a single physical port because some source of traffic is sending
> > traffic at a high rate. In that case, you'll do a lot better if you
> > do fairness based on the source 5-tuple. But if you're doing network
> > virtualization, then the outer source 5-tuples won't necessarily vary
> > much and you'd be better off looking at the VNI and maybe some Geneve
> > TLV options and maybe the inner 5-tuple...
>
> Sure, I see what you mean now. That looks entirely doable if we
> abstract the round-robin bucket selection out of the current patch.
>
> > I would be very pleased if we could integrate a simple mechanism for
> > fairness, based for now on some simple criteria like the source port,
> > but thinking ahead to how we could later make it gracefully extensible
> > to consider more general and possibly customizable criteria.
>
> We could change the patch so that instead of just using the vport for
> round-robin queue insertion, we generalise that and use "buckets"
> instead of vports, and have a set of possible functions that are called
> instead of using port_no directly in ovs_dp_upcall_queue_roundrobin(),
> making this configurable via netlink, per datapath.
>
> We could implement selection based on source port or a hash on the
> source 5-tuple, and the relevant bits of
> ovs_dp_upcall_queue_roundrobin() would look like this:
>
> static int ovs_dp_upcall_queue_roundrobin(struct datapath *dp,
> struct dp_upcall_info *upcall)
> {
>
> [...]
>
> list_for_each_entry(pos, head, list) {
> int bucket = dp->rr_select(pos);
>
> /* Count per-bucket upcalls. */
> if (dp->upcalls.count[bucket] == U8_MAX) {
> err = -ENOSPC;
> goto out_clear;
> }
> dp->upcalls.count[bucket]++;
>
> if (bucket == upcall->bucket) {
> /* Another upcall for the same bucket: move
> insertion
> * point here, keep looking for insertion
> condition to
> * be still met further on.
> */
> find_next = true;
> here = pos;
> continue;
> }
>
> count = dp->upcalls.count[bucket];
> if (find_next && dp->upcalls.count[bucket] >= count) {
> /* Insertion condition met: no need to look
> further,
> * unless another upcall for the same port occurs
> later.
> */
> find_next = false;
> here = pos;
> }
> }
>
> [...]
>
> }
>
> and implementations for dp->rr_select() would look like:
>
> int rr_select_vport(struct dp_upcall_info *upcall)
> {
> return upcall->port_no;
> }
>
> int rr_select_srcport(struct dp_upcall_info *upcall)
> {
> /* look up source port from upcall->skb... */
> }
>
> And we could then easily extend this to use BPF with maps one day.
>
>
Hi Stefano,
If you want to experiment with BPF, Joe and I have some prototype.
We implemented the upcall mechanism using BPF perf event helper function
https://github.com/williamtu/ovs-ebpf/blob/master/bpf/datapath.c#L62
And there are threads polling the perf ring buffer to receive packets from
BPF.
https://github.com/williamtu/ovs-ebpf/blob/master/lib/perf-event.c#L232
If I follow the discussion correctly, before upcall, you need to queue
packets based
on different configurations (vport/hash/vni/5-tuple/...) and queue to
different buckets
when congestion happens. In this case, you probably needs a BPF map to
enqueue/dequeue
the packet. BPF queue map is not supported yet, but there is patch
available:
[iovisor-dev] [RFC PATCH 1/3] bpf: add bpf queue map
So how to enqueue and dequeue packets depends on user's BPF implementation.
This allows fairness scheme to be extensible.
Regards,
William
This is for clarity by the way, but I guess we should avoid indirect
> calls in the final implementation.
>
> What do you think?
>
> --
> Stefano
>
^ permalink raw reply
* [BUG] bpf: syscall: a possible sleep-in-atomic-context bug in map_update_elem()
From: Jia-Ju Bai @ 2018-08-10 14:07 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, Linux Kernel Mailing List
The kernel may sleep with holding a rcu read lock.
The function call paths (from bottom to top) in Linux-4.16 are:
[FUNC] kmalloc(GFP_KERNEL)
kernel/kthread.c, 283: kmalloc in __kthread_create_on_node
kernel/kthread.c, 365: __kthread_create_on_node in kthread_create_on_node
kernel/bpf/cpumap.c, 368: kthread_create_on_node in __cpu_map_entry_alloc
kernel/bpf/cpumap.c, 490: __cpu_map_entry_alloc in cpu_map_update_elem
kernel/bpf/syscall.c, 724: [FUNC_PTR]cpu_map_update_elem in map_update_elem
kernel/bpf/syscall.c, 723: rcu_read_lock in map_update_elem
Note that [FUNC_PTR] means a function pointer call is used.
I do not find a good way to fix it, so I only report.
This is found by my static analysis tool (DSAC).
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH lora-next 01/10] net: lora: sx1301: add register, bit-fields, and helpers for regmap
From: Andreas Färber @ 2018-08-10 11:37 UTC (permalink / raw)
To: Ben Whitten, Ben Whitten
Cc: starnight@g.ncu.edu.tw, hasnain.virk@arm.com,
netdev@vger.kernel.org, Xue Liu, Sebastian Heß, Yannick Lanz
In-Reply-To: <BY1PR02MB111463B66B79C5008905E011E7250@BY1PR02MB1114.namprd02.prod.outlook.com>
Am 09.08.2018 um 15:23 schrieb Ben Whitten:
>> BTW we'll need this regmap conversion for the picoGW_hal,
>> so once we
>> have a working SPI regmap driver, we'll need to split out the
>> SPI bits,
>> similar to sx125x.
>
> I am unfamiliar with the picoGW_hal, do they expose the sx1301
> as a device on a regmap_bus then?
It uses an MCU implementing a USB CDC interface that implements UART
commands corresponding to the individual SPI read and write modes like
single and burst. That reference design is for SX1308, but some vendors
appear to be using it for SX1301, too.
https://github.com/Lora-net/picoGW_mcu
So I believe I'll need to implement a regmap_bus in a new serdev driver
to support it, once I gain access to such an implementation.
But there's other general serdev problems with USB not limited to picoGW
that I'll need to start a thread about on linux-serial/linux-usb...
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* REPLY AS SOON AS POSSIBLE
From: Dr Chien Direktor von Hang Seng @ 2018-08-10 6:42 UTC (permalink / raw)
To: Recipients
I am Vice Chairman of Hang Seng Bank, I have Important Matter to Discuss with you concerning my late client. Died without a NEXT OF KIN. Send me your private email for full details information. email me at E-Mail: draymdm@gmail.com
Regards
Mr.Fung
^ permalink raw reply
* Re: [Query]: DSA Understanding
From: Lad, Prabhakar @ 2018-08-10 11:26 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20180809172334.GA25449@lunn.ch>
Hi Andrew,
On Thu, Aug 9, 2018 at 6:23 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Its coming from the switch lan4 I have attached the png, where
> > C4:F3:12:08:FE:7F is
> > the mac of lan4, which is broadcast to ff:ff:ff:ff:ff:ff, which is
> > causing rx counter on
> > PC to go up.
>
> So, big packets are making it from the switch to the PC. But the small
> ARP packets are not.
>
> This is what Florian was suggesting.
>
> ARP packets are smaller than 64 bytes, which is the minimum packet
> size for Ethernet. Any packets smaller than 64 bytes are called runt
> packets. They have to be padded upto 64 bytes in order to make them
> valid. Otherwise the destination, or any switch along the path, might
> throw them away.
>
> What could be happening is that the CSPW driver or hardware is padding
> the packet to 64 bytes. But that packet has a DSA header in it. The
> switch removes the header, recalculate the checksum and sends the
> packet. It is now either 4 or 8 bytes smaller, depending on what DSA
> header was used. It then becomes a runt packet.
>
Thank you for the clarification, this really helped me out.
> Florian had to fix this problem recently.
>
> http://patchwork.ozlabs.org/patch/836534/
>
But seems like this patch was never accepted, instead
brcm_tag_xmit_ll() does it if I am understanding it correctly.
similarly to this ksz_xmit() is taking care of padding.
> You probably need something similar for the cpsw.
>
looking at the tag_ksz.c in xmit function this is taken care of
/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
* ---------------------------------------------------------------------------
* DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
* ---------------------------------------------------------------------------
* tag0 : Prioritization (not used now)
* tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
*
* For Egress (KSZ -> Host), 1 byte is added before FCS.
* ---------------------------------------------------------------------------
* DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
* ---------------------------------------------------------------------------
* tag0 : zero-based value represents port
* (eg, 0x00=port1, 0x02=port3, 0x06=port7)
*/
#define KSZ_INGRESS_TAG_LEN 2
#define KSZ_EGRESS_TAG_LEN 1
static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct sk_buff *nskb;
int padlen;
u8 *tag;
padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
/* Let dsa_slave_xmit() free skb */
if (__skb_put_padto(skb, skb->len + padlen, false))
return NULL;
nskb = skb;
} else {
nskb = alloc_skb(NET_IP_ALIGN + skb->len +
padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
if (!nskb)
return NULL;
skb_reserve(nskb, NET_IP_ALIGN);
skb_reset_mac_header(nskb);
skb_set_network_header(nskb,
skb_network_header(skb) - skb->head);
skb_set_transport_header(nskb,
skb_transport_header(skb) - skb->head);
skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
/* Let skb_put_padto() free nskb, and let dsa_slave_xmit() free
* skb
*/
if (skb_put_padto(nskb, nskb->len + padlen))
return NULL;
consume_skb(skb);
}
tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
tag[0] = 0;
tag[1] = 1 << p->dp->index; /* destination port */
return nskb;
}
Cheers,
--Prabhakar Lad
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox