* Re: [PATCH v2 bpf-next] bpf: cgroup: Fix build error without CONFIG_NET
From: Daniel Borkmann @ 2019-07-08 15:26 UTC (permalink / raw)
To: YueHaibing, ast, kafai, songliubraving, yhs, sdf
Cc: linux-kernel, netdev, bpf
In-Reply-To: <20190703082630.51104-1-yuehaibing@huawei.com>
On 07/03/2019 10:26 AM, YueHaibing wrote:
> If CONFIG_NET is not set and CONFIG_CGROUP_BPF=y,
> gcc building fails:
>
> kernel/bpf/cgroup.o: In function `cg_sockopt_func_proto':
> cgroup.c:(.text+0x237e): undefined reference to `bpf_sk_storage_get_proto'
> cgroup.c:(.text+0x2394): undefined reference to `bpf_sk_storage_delete_proto'
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_getsockopt':
> (.text+0x2a1f): undefined reference to `lock_sock_nested'
> (.text+0x2ca2): undefined reference to `release_sock'
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_setsockopt':
> (.text+0x3006): undefined reference to `lock_sock_nested'
> (.text+0x32bb): undefined reference to `release_sock'
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
> Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next 1/2] bpf: skip sockopt hooks without CONFIG_NET
From: Daniel Borkmann @ 2019-07-08 15:26 UTC (permalink / raw)
To: Yonghong Song, Arnd Bergmann, Alexei Starovoitov
Cc: Andrii Nakryiko, Martin Lau, Stanislav Fomichev, Song Liu,
Mauricio Vasquez B, Roman Gushchin, Matt Mullins,
Willem de Bruijn, Andrey Ignatov, netdev@vger.kernel.org,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <0e7cf1b5-579f-5fcd-0966-8760148b00de@fb.com>
On 07/08/2019 05:06 PM, Yonghong Song wrote:
> On 7/8/19 5:57 AM, Arnd Bergmann wrote:
>> When CONFIG_NET is disabled, we get a link error:
>>
>> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_setsockopt':
>> cgroup.c:(.text+0x3010): undefined reference to `lock_sock_nested'
>> cgroup.c:(.text+0x3258): undefined reference to `release_sock'
>> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_getsockopt':
>> cgroup.c:(.text+0x3568): undefined reference to `lock_sock_nested'
>> cgroup.c:(.text+0x3870): undefined reference to `release_sock'
>> kernel/bpf/cgroup.o: In function `cg_sockopt_func_proto':
>> cgroup.c:(.text+0x41d8): undefined reference to `bpf_sk_storage_delete_proto'
>>
>> None of this code is useful in this configuration anyway, so we can
>> simply hide it in an appropriate #ifdef.
>>
>> Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> FYI.
>
> There is already a patch to fix the same issue,
> https://lore.kernel.org/bpf/e9e489fe-feec-a211-82aa-5df0c6a308d1@huawei.com/T/#t
>
> which has been acked and not merged yet.
Done now, and I've also applied patch 2/2 from here, thanks.
^ permalink raw reply
* Re: [PATCH bpf-next 0/3] xdp: Add devmap_hash map type
From: Toke Høiland-Jørgensen @ 2019-07-08 15:40 UTC (permalink / raw)
To: Jonathan Lemon
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <53906C87-8AF9-4048-8CA0-AE38C023AEF7@flugsvamp.com>
"Jonathan Lemon" <jlemon@flugsvamp.com> writes:
> On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
>
>> This series adds a new map type, devmap_hash, that works like the
>> existing
>> devmap type, but using a hash-based indexing scheme. This is useful
>> for the use
>> case where a devmap is indexed by ifindex (for instance for use with
>> the routing
>> table lookup helper). For this use case, the regular devmap needs to
>> be sized
>> after the maximum ifindex number, not the number of devices in it. A
>> hash-based
>> indexing scheme makes it possible to size the map after the number of
>> devices it
>> should contain instead.
>
> This device hash map is sized at NETDEV_HASHENTRIES == 2^8 == 256. Is
> this actually smaller than an array? What ifindex values are you
> seeing?
Well, not in all cases, certainly. But machines with lots of virtual
interfaces (e.g., container hosts) can easily exceed that. Also, for a
devmap we charge the full size of max_entries * struct bpf_dtab_netdev
towards the locked memory cost on map creation. And since sizeof(struct
bpf_dtab_netdev) is 64, the size of the hashmap only corresponds to 32
entries...
But more importantly, it's a UI issue: Say you want to create a simple
program that uses the fib_lookup helper (something like the xdp_fwd
example under samples/bpf/). You know that you only want to route
between a couple of interfaces, so you naturally create a devmap that
can hold, say, 8 entries (just to be sure). This works fine on your
initial test, where the machine only has a couple of physical interfaces
brought up at boot. But then you try to run the same program on your
production server, where the interfaces you need to use just happen to
have ifindexes higher than 8, and now it breaks for no discernible
reason. Or even worse, if you remove and re-add an interface, you may no
longer be able to insert it into your map because the ifindex changed...
-Toke
^ permalink raw reply
* WARNING in __mark_chain_precision
From: syzbot @ 2019-07-08 15:27 UTC (permalink / raw)
To: ast, bcrl, bpf, daniel, davem, hawk, jakub.kicinski,
john.fastabend, kafai, linux-aio, linux-fsdevel, linux-kernel,
netdev, songliubraving, syzkaller-bugs, torvalds, viro,
xdp-newbies, yhs
Hello,
syzbot found the following crash on:
HEAD commit: e5a3e259 Merge branch 'bpf-tcp-rtt-hook'
git tree: bpf-next
console output: https://syzkaller.appspot.com/x/log.txt?x=14190c2da00000
kernel config: https://syzkaller.appspot.com/x/.config?x=dd16b8dc9d0d210c
dashboard link: https://syzkaller.appspot.com/bug?extid=4da3ff23081bafe74fc2
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1409ce0da00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17226a0da00000
The bug was bisected to:
commit b53119f13a04879c3bf502828d99d13726639ead
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu Mar 7 01:22:54 2019 +0000
pin iocb through aio.
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11427b8ba00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=13427b8ba00000
console output: https://syzkaller.appspot.com/x/log.txt?x=15427b8ba00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4da3ff23081bafe74fc2@syzkaller.appspotmail.com
Fixes: b53119f13a04 ("pin iocb through aio.")
------------[ cut here ]------------
verifier backtracking bug
WARNING: CPU: 0 PID: 9104 at kernel/bpf/verifier.c:1785
__mark_chain_precision+0x19bb/0x1ee0 kernel/bpf/verifier.c:1785
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 9104 Comm: syz-executor284 Not tainted 5.2.0-rc5+ #34
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+0x172/0x1f0 lib/dump_stack.c:113
panic+0x2cb/0x744 kernel/panic.c:219
__warn.cold+0x20/0x4d kernel/panic.c:576
report_bug+0x263/0x2b0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:272
do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:291
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:986
RIP: 0010:__mark_chain_precision+0x19bb/0x1ee0 kernel/bpf/verifier.c:1785
Code: 08 31 ff 89 de e8 95 ba f2 ff 84 db 0f 85 ce fe ff ff e8 48 b9 f2 ff
48 c7 c7 e0 44 91 87 c6 05 1c 15 1f 08 01 e8 03 f1 c4 ff <0f> 0b 41 bc f2
ff ff ff e9 af fe ff ff e8 d3 3c 2c 00 e9 c2 e7 ff
RSP: 0018:ffff88809f04f380 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815ad926 RDI: ffffed1013e09e62
RBP: ffff88809f04f4d0 R08: ffff88809987a540 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffff88809af0c280 R14: 0000000000000001 R15: ffff88809f65cb00
mark_chain_precision kernel/bpf/verifier.c:1822 [inline]
check_cond_jmp_op+0xcd8/0x3c30 kernel/bpf/verifier.c:5842
do_check+0x60f4/0x8a20 kernel/bpf/verifier.c:7782
bpf_check+0x6f99/0x9950 kernel/bpf/verifier.c:9293
bpf_prog_load+0xe68/0x1670 kernel/bpf/syscall.c:1698
__do_sys_bpf+0xa20/0x42d0 kernel/bpf/syscall.c:2849
__se_sys_bpf kernel/bpf/syscall.c:2808 [inline]
__x64_sys_bpf+0x73/0xb0 kernel/bpf/syscall.c:2808
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440369
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 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffca85a2fa8 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440369
RDX: 0000000000000048 RSI: 0000000020000200 RDI: 0000000000000005
RBP: 00000000006ca018 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000ffffffff R11: 0000000000000246 R12: 0000000000401bf0
R13: 0000000000401c80 R14: 0000000000000000 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
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#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH net-next v5 1/4] net/sched: Introduce action ct
From: Florian Westphal @ 2019-07-08 15:28 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Paul Blakey, Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
netdev, David Miller, Aaron Conole, Zhike Wang, Rony Efraim,
nst-kernel, John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <20190708134208.GD3390@localhost.localdomain>
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote:
> > + } else { /* NFPROTO_IPV6 */
> > + enum ip6_defrag_users user = IP6_DEFRAG_CONNTRACK_IN + zone;
> > +
> > + memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));
> > + err = nf_ct_frag6_gather(net, skb, user);
>
> This doesn't build without IPv6 enabled.
> ERROR: "nf_ct_frag6_gather" [net/sched/act_ct.ko] undefined!
>
> We need to (copy and pasted):
>
> @@ -179,7 +179,9 @@ static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
> local_bh_enable();
> if (err && err != -EINPROGRESS)
> goto out_free;
> - } else { /* NFPROTO_IPV6 */
> + }
> +#if IS_ENABLED(IPV6)
> + else { /* NFPROTO_IPV6 */
> enum ip6_defrag_users user = IP6_DEFRAG_CONNTRACK_IN + zone;
Good catch, but it should be
#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
just like ovs conntrack.c ,
^ permalink raw reply
* Re: [PATCH bpf-next] tools: bpftool: add completion for bpftool prog "loadall"
From: Daniel Borkmann @ 2019-07-08 15:27 UTC (permalink / raw)
To: Quentin Monnet, Alexei Starovoitov; +Cc: bpf, netdev, oss-drivers
In-Reply-To: <20190708130546.7518-1-quentin.monnet@netronome.com>
On 07/08/2019 03:05 PM, Quentin Monnet wrote:
> Bash completion for proposing the "loadall" subcommand is missing. Let's
> add it to the completion script.
>
> Add a specific case to propose "load" and "loadall" for completing:
>
> $ bpftool prog load
> ^ cursor is here
>
> Otherwise, completion considers that $command is in load|loadall and
> starts making related completions (file or directory names, as the
> number of words on the command line is below 6), when the only suggested
> keywords should be "load" and "loadall" until one has been picked and a
> space entered after that to move to the next word.
>
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Applied, thanks!
^ permalink raw reply
* [ANNOUNCE] iproute2 5.2
From: Stephen Hemminger @ 2019-07-08 15:26 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
The 5.2 kernel has been released, and therefore time for another
update to iproute2.
Not a lot of big new features in this release. Just the usual array of
small fixes across the board.
Download:
https://www.kernel.org/pub/linux/utils/net/iproute2/iproute2-5.2.0.tar.gz
Repository for upcoming release:
git://git.kernel.org/pub/scm/network/iproute2/iproute2.git
And future release (net-next):
git://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git
Thanks for all the contributions.
Report problems (or enhancements) to the netdev@vger.kernel.org mailing list.
---
Andrea Claudi (6):
Makefile: use make -C
ip address: do not set nodad option for IPv4 addresses
ip address: do not set home option for IPv4 addresses
ip address: do not set mngtmpaddr option for IPv4 addresses
man: tc-netem.8: fix URL for netem page
tc: netem: fix r parameter in Bernoulli loss model
Baruch Siach (2):
devlink: fix format string warning for 32bit targets
devlink: fix libc and kernel headers collision
David Ahern (5):
Update kernel headers
Update kernel headers
Update kernel headers
uapi: wrap SIOCGSTAMP and SIOCGSTAMPNS in ifndef
Update kernel headers
Davide Caratti (2):
man: tc-skbedit.8: document 'inheritdsfield'
tc: simple: don't hardcode the control action
Eyal Birger (1):
tc: adjust xtables_match and xtables_target to changes in recent iptables
Gal Pressman (1):
rdma: Update node type strings
Hangbin Liu (1):
ip/iptoken: fix dump error when ipv6 disabled
Hoang Le (3):
tipc: add link broadcast set method and ratio
tipc: add link broadcast get
tipc: add link broadcast man page
Ido Schimmel (2):
ipneigh: Print neighbour offload indication
devlink: Increase column size for larger shared buffers
Josh Hunt (1):
ss: add option to print socket information on one line
Kristian Evensen (1):
ip fou: Support binding FOU ports
Lucas Siba 2019-04-20 11:40 UTC (1):
Update tc-bpf.8 man page examples
Lukasz Czapnik (1):
tc: flower: fix port value truncation
Mahesh Bandewar (1):
ip6tunnel: fix 'ip -6 {show|change} dev <name>' cmds
Matteo Croce (4):
ip: reset netns after each command in batch mode
netns: switch netns in the child when executing commands
ip vrf: use hook to change VRF in the child
netns: make netns_{save,restore} static
Michael Forney (1):
ipmroute: Prevent overlapping storage of `filter` global
Mike Manning (1):
iplink_vlan: add support for VLAN bridge binding flag
Moshe Shemesh (1):
devlink: mnlg: Catch returned error value of dumpit commands
Nicolas Dichtel (3):
lib: suppress error msg when filling the cache
iplink: don't try to get ll addr len when creating an iface
ip monitor: display interfaces from all groups
Nikolay Aleksandrov (2):
iplink: bridge: add support for vlan_stats_per_port
bridge: mdb: restore text output format
Paolo Abeni (2):
tc: add support for plug qdisc
m_mirred: don't bail if the control action is missing
Parav Pandit (1):
devlink: Increase bus, device buffer size to 64 bytes
Pete Morici (1):
Add support for configuring MACsec gcm-aes-256 cipher type.
Roman Mashak (1):
tc: Fix binding of gact action by index.
Stefano Brivio (1):
iproute: Set flags and attributes on dump to get IPv6 cached routes to be flushed
Stephen Hemminger (14):
uapi: update to elf-em header
uapi: add include/linux/net.h
uapi: update headers to import asm-generic/sockios.h
mailmap: add myself
mailmap: map David's mail address
uapi: add sockios.h
uapi: merge bpf.h from 5.2
rdma: update uapi headers
man: fix macaddr section of ip-link
uapi: minor upstream btf.h header change
testsuite: intent if/else in Makefile
uapi: update headers and add if_link.h and if_infiniband.h
devlink: replace print macros with functions
vv5.2.0
Steve Wise (4):
Add .mailmap file
rdma: add helper rd_sendrecv_msg()
rdma: add 'link add/delete' commands
rdma: man page update for link add/delete
Tomasz Torcz (1):
ss: in --numeric mode, print raw numbers for data rates
Vinicius Costa Gomes (2):
taprio: Add support for changing schedules
taprio: Add support for cycle_time and cycle_time_extension
^ permalink raw reply
* Re: [PATCH v7 bpf-next 0/5] libbpf: add perf buffer abstraction and API
From: Daniel Borkmann @ 2019-07-08 15:23 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, bpf, netdev, ast, kernel-team
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>
On 07/06/2019 08:06 PM, Andrii Nakryiko wrote:
> This patchset adds a high-level API for setting up and polling perf buffers
> associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
> described in corresponding commit.
>
> Patch #1 adds a set of APIs to set up and work with perf buffer.
> Patch #2 enhances libbpf to support auto-setting PERF_EVENT_ARRAY map size.
> Patch #3 adds test.
> Patch #4 converts bpftool map event_pipe to new API.
> Patch #5 updates README to mention perf_buffer_ prefix.
>
> v6->v7:
> - __x64_ syscall prefix (Yonghong);
> v5->v6:
> - fix C99 for loop variable initialization usage (Yonghong);
> v4->v5:
> - initialize perf_buffer_raw_opts in bpftool map event_pipe (Jakub);
> - add perf_buffer_ to README;
> v3->v4:
> - fixed bpftool event_pipe cmd error handling (Jakub);
> v2->v3:
> - added perf_buffer__new_raw for more low-level control;
> - converted bpftool map event_pipe to new API (Daniel);
> - fixed bug with error handling in create_maps (Song);
> v1->v2:
> - add auto-sizing of PERF_EVENT_ARRAY maps;
>
> Andrii Nakryiko (5):
> libbpf: add perf buffer API
> libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
> selftests/bpf: test perf buffer API
> tools/bpftool: switch map event_pipe to libbpf's perf_buffer
> libbpf: add perf_buffer_ prefix to README
>
> tools/bpf/bpftool/map_perf_ring.c | 201 +++------
> tools/lib/bpf/README.rst | 3 +-
> tools/lib/bpf/libbpf.c | 397 +++++++++++++++++-
> tools/lib/bpf/libbpf.h | 49 +++
> tools/lib/bpf/libbpf.map | 4 +
> .../selftests/bpf/prog_tests/perf_buffer.c | 100 +++++
> .../selftests/bpf/progs/test_perf_buffer.c | 25 ++
> 7 files changed, 634 insertions(+), 145 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] xdp: Refactor devmap allocation code for reuse
From: Toke Høiland-Jørgensen @ 2019-07-08 15:19 UTC (permalink / raw)
To: Jonathan Lemon
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <A2ABED10-8475-4878-93DF-F16D106FC33D@flugsvamp.com>
"Jonathan Lemon" <jlemon@flugsvamp.com> writes:
> On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
>
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> The subsequent patch to add a new devmap sub-type can re-use much of
>> the
>> initialisation and allocation code, so refactor it into separate
>> functions.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>> kernel/bpf/devmap.c | 137
>> +++++++++++++++++++++++++++++++--------------------
>> 1 file changed, 84 insertions(+), 53 deletions(-)
>>
>> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
>> index d83cf8ccc872..a2fe16362129 100644
>> --- a/kernel/bpf/devmap.c
>> +++ b/kernel/bpf/devmap.c
>> @@ -60,7 +60,7 @@ struct xdp_bulk_queue {
>> struct bpf_dtab_netdev {
>> struct net_device *dev; /* must be first member, due to tracepoint
>> */
>> struct bpf_dtab *dtab;
>> - unsigned int bit;
>> + unsigned int idx; /* keep track of map index for tracepoint */
>> struct xdp_bulk_queue __percpu *bulkq;
>> struct rcu_head rcu;
>> };
>> @@ -75,28 +75,22 @@ struct bpf_dtab {
>> static DEFINE_SPINLOCK(dev_map_lock);
>> static LIST_HEAD(dev_map_list);
>>
>> -static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
>> +static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr
>> *attr,
>> + bool check_memlock)
>
> This check_memlock parameter appears to be unused.
Ah yes, good catch! That was left over from when the patch set also
contained the "default map" stuff. Will fix.
-Toke
^ permalink raw reply
* Re: [PATCH iproute2 1/2] tc: added mask parameter in skbedit action
From: Stephen Hemminger @ 2019-07-08 15:15 UTC (permalink / raw)
To: Roman Mashak; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1562195132-9829-1-git-send-email-mrv@mojatatu.com>
On Wed, 3 Jul 2019 19:05:31 -0400
Roman Mashak <mrv@mojatatu.com> wrote:
> + if (tb[TCA_SKBEDIT_MASK]) {
> + print_uint(PRINT_ANY, "mask", "/0x%x",
> + rta_getattr_u32(tb[TCA_SKBEDIT_MASK]));
Why not print in hex with.
print_hex(PRINT_ANY, "mask", "/%#x",
rta_getattr_u32(tb[TCA_SKBEDIT_MASK]));
^ permalink raw reply
* Re: [PATCH iproute2] ip-route: fix json formatting for metrics
From: Andrea Claudi @ 2019-07-08 15:14 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, David Ahern
In-Reply-To: <f1535e547aa6da8216ca2a0da7c06b645a132929.1562578533.git.aclaudi@redhat.com>
On Mon, Jul 8, 2019 at 11:38 AM Andrea Claudi <aclaudi@redhat.com> wrote:
>
> Setting metrics for routes currently lead to non-parsable
> json output. For example:
>
> $ ip link add type dummy
> $ ip route add 192.168.2.0 dev dummy0 metric 100 mtu 1000 rto_min 3
> $ ip -j route | jq
> parse error: ':' not as part of an object at line 1, column 319
>
> Fixing this opening a json object in the metrics array and using
> print_string() instead of fprintf().
>
> This is the output for the above commands applying this patch:
>
> $ ip -j route | jq
> [
> {
> "dst": "192.168.2.0",
> "dev": "dummy0",
> "scope": "link",
> "metric": 100,
> "flags": [],
> "metrics": [
> {
> "mtu": 1000,
> "rto_min": 3
> }
> ]
> }
> ]
>
> Fixes: 663c3cb23103f ("iproute: implement JSON and color output")
> Fixes: 968272e791710 ("iproute: refactor metrics print")
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> ---
> ip/iproute.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 1669e0138259e..2f9b612b0b506 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -578,6 +578,7 @@ static void print_rta_metrics(FILE *fp, const struct rtattr *rta)
> int i;
>
> open_json_array(PRINT_JSON, "metrics");
> + open_json_object(NULL);
>
> parse_rtattr(mxrta, RTAX_MAX, RTA_DATA(rta), RTA_PAYLOAD(rta));
>
> @@ -611,7 +612,7 @@ static void print_rta_metrics(FILE *fp, const struct rtattr *rta)
> print_rtax_features(fp, val);
> break;
> default:
> - fprintf(fp, "%u ", val);
> + print_uint(PRINT_ANY, mx_names[i], "%u ", val);
> break;
>
> case RTAX_RTT:
> @@ -639,6 +640,7 @@ static void print_rta_metrics(FILE *fp, const struct rtattr *rta)
> }
> }
>
> + close_json_object();
> close_json_array(PRINT_JSON, NULL);
> }
>
> --
> 2.20.1
>
Sorry, I forgot to add:
Reported-by: Frank Hofmann <fhofmann@cloudflare.com>
Regards,
Andrea
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
From: Anton Protopopov @ 2019-07-08 15:11 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
netdev, bpf, linux-kernel, andriin
In-Reply-To: <734dd45a-95b0-a7fd-9e1d-0535ef4d3e12@iogearbox.net>
пт, 5 июл. 2019 г. в 17:44, Daniel Borkmann <daniel@iogearbox.net>:
>
> On 07/05/2019 10:44 PM, Anton Protopopov wrote:
> > Add a new API bpf_object__reuse_maps() which can be used to replace all maps in
> > an object by maps pinned to a directory provided in the path argument. Namely,
> > each map M in the object will be replaced by a map pinned to path/M.name.
> >
> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > ---
> > tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/libbpf.h | 2 ++
> > tools/lib/bpf/libbpf.map | 1 +
> > 3 files changed, 37 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 4907997289e9..84c9e8f7bfd3 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
> > return 0;
> > }
> >
> > +int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)
> > +{
> > + struct bpf_map *map;
> > +
> > + if (!obj)
> > + return -ENOENT;
> > +
> > + if (!path)
> > + return -EINVAL;
> > +
> > + bpf_object__for_each_map(map, obj) {
> > + int len, err;
> > + int pinned_map_fd;
> > + char buf[PATH_MAX];
>
> We'd need to skip the case of bpf_map__is_internal(map) since they are always
> recreated for the given object.
>
> > + len = snprintf(buf, PATH_MAX, "%s/%s", path, bpf_map__name(map));
> > + if (len < 0) {
> > + return -EINVAL;
> > + } else if (len >= PATH_MAX) {
> > + return -ENAMETOOLONG;
> > + }
> > +
> > + pinned_map_fd = bpf_obj_get(buf);
> > + if (pinned_map_fd < 0)
> > + return pinned_map_fd;
>
> Should we rather have a new map definition attribute that tells to reuse
> the map if it's pinned in bpf fs, and if not, we create it and later on
> pin it? This is what iproute2 is doing and which we're making use of heavily.
What do you think about adding a new generic field, say load_flags,
to the bpf_map_def structure and a particular flag, say LOAD_F_STICKY
for this purpose? And it will be cleared for internal maps, so we will skip
them as well.
> In bpf_object__reuse_maps() bailing out if bpf_obj_get() fails is perhaps
> too limiting for a generic API as new version of an object file may contain
> new maps which are not yet present in bpf fs at that point.
How permissive should it be? Is it ok to just print a warning on any
bpf_obj_get()
failure? Or does it make sense to skip some specific error (ENOENT) and reject
on other errors?
>
> > + err = bpf_map__reuse_fd(map, pinned_map_fd);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
> > {
> > struct bpf_program *prog;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index d639f47e3110..7fe465a1be76 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -82,6 +82,8 @@ int bpf_object__variable_offset(const struct bpf_object *obj, const char *name,
> > LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
> > LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
> > const char *path);
> > +LIBBPF_API int bpf_object__reuse_maps(struct bpf_object *obj,
> > + const char *path);
> > LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
> > const char *path);
> > LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 2c6d835620d2..66a30be6696c 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -172,5 +172,6 @@ LIBBPF_0.0.4 {
> > btf_dump__new;
> > btf__parse_elf;
> > bpf_object__load_xattr;
> > + bpf_object__reuse_maps;
> > libbpf_num_possible_cpus;
> > } LIBBPF_0.0.3;
> >
>
^ permalink raw reply
* Re: [PATCH net-next 1/2] bpf: skip sockopt hooks without CONFIG_NET
From: Yonghong Song @ 2019-07-08 15:06 UTC (permalink / raw)
To: Arnd Bergmann, Alexei Starovoitov, Daniel Borkmann
Cc: Andrii Nakryiko, Martin Lau, Stanislav Fomichev, Song Liu,
Mauricio Vasquez B, Roman Gushchin, Matt Mullins,
Willem de Bruijn, Andrey Ignatov, netdev@vger.kernel.org,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190708125733.3944836-1-arnd@arndb.de>
On 7/8/19 5:57 AM, Arnd Bergmann wrote:
> When CONFIG_NET is disabled, we get a link error:
>
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_setsockopt':
> cgroup.c:(.text+0x3010): undefined reference to `lock_sock_nested'
> cgroup.c:(.text+0x3258): undefined reference to `release_sock'
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_getsockopt':
> cgroup.c:(.text+0x3568): undefined reference to `lock_sock_nested'
> cgroup.c:(.text+0x3870): undefined reference to `release_sock'
> kernel/bpf/cgroup.o: In function `cg_sockopt_func_proto':
> cgroup.c:(.text+0x41d8): undefined reference to `bpf_sk_storage_delete_proto'
>
> None of this code is useful in this configuration anyway, so we can
> simply hide it in an appropriate #ifdef.
>
> Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
FYI.
There is already a patch to fix the same issue,
https://lore.kernel.org/bpf/e9e489fe-feec-a211-82aa-5df0c6a308d1@huawei.com/T/#t
which has been acked and not merged yet.
> ---
> include/linux/bpf_types.h | 2 ++
> kernel/bpf/cgroup.c | 6 ++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index eec5aeeeaf92..3c7222b2db96 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -30,8 +30,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, raw_tracepoint_writable)
> #ifdef CONFIG_CGROUP_BPF
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SYSCTL, cg_sysctl)
> +#ifdef CONFIG_NET
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCKOPT, cg_sockopt)
> #endif
> +#endif
> #ifdef CONFIG_BPF_LIRC_MODE2
> BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
> #endif
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 76fa0076f20d..7be44460bd93 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -590,6 +590,7 @@ int cgroup_bpf_prog_query(const union bpf_attr *attr,
> return ret;
> }
>
> +#ifdef CONFIG_NET
> /**
> * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering
> * @sk: The socket sending or receiving traffic
> @@ -750,6 +751,7 @@ int __cgroup_bpf_run_filter_sock_ops(struct sock *sk,
> return ret == 1 ? 0 : -EPERM;
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_sock_ops);
> +#endif
>
> int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
> short access, enum bpf_attach_type type)
> @@ -939,6 +941,7 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_sysctl);
>
> +#ifdef CONFIG_NET
> static bool __cgroup_bpf_prog_array_is_empty(struct cgroup *cgrp,
> enum bpf_attach_type attach_type)
> {
> @@ -1120,6 +1123,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
> return ret;
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_getsockopt);
> +#endif
>
> static ssize_t sysctl_cpy_dir(const struct ctl_dir *dir, char **bufp,
> size_t *lenp)
> @@ -1382,6 +1386,7 @@ const struct bpf_verifier_ops cg_sysctl_verifier_ops = {
> const struct bpf_prog_ops cg_sysctl_prog_ops = {
> };
>
> +#ifdef CONFIG_NET
> static const struct bpf_func_proto *
> cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> @@ -1531,3 +1536,4 @@ const struct bpf_verifier_ops cg_sockopt_verifier_ops = {
>
> const struct bpf_prog_ops cg_sockopt_prog_ops = {
> };
> +#endif
>
^ permalink raw reply
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Matt Hart @ 2019-07-08 15:11 UTC (permalink / raw)
To: andriin; +Cc: andrii.nakryiko, ast, bpf, daniel, kernel-team, netdev, sdf
Hi all,
I bisected a perf build error on ARMv7 to this patch:
libbpf.c: In function ‘perf_event_open_probe’:
libbpf.c:4112:17: error: cast from pointer to integer of different
size [-Werror=pointer-to-int-cast]
attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
^
Is this a known issue?
^ permalink raw reply
* Re: [PATCH bpf-next 0/3] xdp: Add devmap_hash map type
From: Jonathan Lemon @ 2019-07-08 15:10 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156234940798.2378.9008707939063611210.stgit@alrua-x1>
On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
> This series adds a new map type, devmap_hash, that works like the
> existing
> devmap type, but using a hash-based indexing scheme. This is useful
> for the use
> case where a devmap is indexed by ifindex (for instance for use with
> the routing
> table lookup helper). For this use case, the regular devmap needs to
> be sized
> after the maximum ifindex number, not the number of devices in it. A
> hash-based
> indexing scheme makes it possible to size the map after the number of
> devices it
> should contain instead.
This device hash map is sized at NETDEV_HASHENTRIES == 2^8 == 256. Is
this actually
smaller than an array? What ifindex values are you seeing?
--
Jonathan
>
> This was previously part of my patch series that also turned the
> regular
> bpf_redirect() helper into a map-based one; for this series I just
> pulled out
> the patches that introduced the new map type.
>
> Changelog:
>
> Changes to these patches since the previous series:
>
> - Rebase on top of the other devmap changes (makes this one simpler!)
> - Don't enforce key==val, but allow arbitrary indexes.
> - Rename the type to devmap_hash to reflect the fact that it's just a
> hashmap now.
>
> ---
>
> Toke Høiland-Jørgensen (3):
> include/bpf.h: Remove map_insert_ctx() stubs
> xdp: Refactor devmap allocation code for reuse
> xdp: Add devmap_hash map type for looking up devices by hashed
> index
>
>
> include/linux/bpf.h | 11 -
> include/linux/bpf_types.h | 1
> include/trace/events/xdp.h | 3
> include/uapi/linux/bpf.h | 7 -
> kernel/bpf/devmap.c | 325
> ++++++++++++++++++++++++++-----
> kernel/bpf/verifier.c | 2
> net/core/filter.c | 9 +
> tools/bpf/bpftool/map.c | 1
> tools/include/uapi/linux/bpf.h | 7 -
> tools/lib/bpf/libbpf_probes.c | 1
> tools/testing/selftests/bpf/test_maps.c | 16 ++
> 11 files changed, 316 insertions(+), 67 deletions(-)
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] xdp: Refactor devmap allocation code for reuse
From: Jonathan Lemon @ 2019-07-08 15:06 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156234940841.2378.6629890565300526702.stgit@alrua-x1>
On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> The subsequent patch to add a new devmap sub-type can re-use much of
> the
> initialisation and allocation code, so refactor it into separate
> functions.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
> kernel/bpf/devmap.c | 137
> +++++++++++++++++++++++++++++++--------------------
> 1 file changed, 84 insertions(+), 53 deletions(-)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index d83cf8ccc872..a2fe16362129 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -60,7 +60,7 @@ struct xdp_bulk_queue {
> struct bpf_dtab_netdev {
> struct net_device *dev; /* must be first member, due to tracepoint
> */
> struct bpf_dtab *dtab;
> - unsigned int bit;
> + unsigned int idx; /* keep track of map index for tracepoint */
> struct xdp_bulk_queue __percpu *bulkq;
> struct rcu_head rcu;
> };
> @@ -75,28 +75,22 @@ struct bpf_dtab {
> static DEFINE_SPINLOCK(dev_map_lock);
> static LIST_HEAD(dev_map_list);
>
> -static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
> +static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr
> *attr,
> + bool check_memlock)
This check_memlock parameter appears to be unused.
--
Jonathan
> {
> - struct bpf_dtab *dtab;
> int err, cpu;
> u64 cost;
>
> - if (!capable(CAP_NET_ADMIN))
> - return ERR_PTR(-EPERM);
> -
> /* check sanity of attributes */
> if (attr->max_entries == 0 || attr->key_size != 4 ||
> attr->value_size != 4 || attr->map_flags &
> ~DEV_CREATE_FLAG_MASK)
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
>
> /* Lookup returns a pointer straight to dev->ifindex, so make sure
> the
> * verifier prevents writes from the BPF side
> */
> attr->map_flags |= BPF_F_RDONLY_PROG;
>
> - dtab = kzalloc(sizeof(*dtab), GFP_USER);
> - if (!dtab)
> - return ERR_PTR(-ENOMEM);
>
> bpf_map_init_from_attr(&dtab->map, attr);
>
> @@ -107,9 +101,7 @@ static struct bpf_map *dev_map_alloc(union
> bpf_attr *attr)
> /* if map size is larger than memlock limit, reject it */
> err = bpf_map_charge_init(&dtab->map.memory, cost);
> if (err)
> - goto free_dtab;
> -
> - err = -ENOMEM;
> + return -EINVAL;
>
> dtab->flush_list = alloc_percpu(struct list_head);
> if (!dtab->flush_list)
> @@ -124,19 +116,38 @@ static struct bpf_map *dev_map_alloc(union
> bpf_attr *attr)
> if (!dtab->netdev_map)
> goto free_percpu;
>
> - spin_lock(&dev_map_lock);
> - list_add_tail_rcu(&dtab->list, &dev_map_list);
> - spin_unlock(&dev_map_lock);
> -
> - return &dtab->map;
> + return 0;
>
> free_percpu:
> free_percpu(dtab->flush_list);
> free_charge:
> bpf_map_charge_finish(&dtab->map.memory);
> -free_dtab:
> - kfree(dtab);
> - return ERR_PTR(err);
> + return -ENOMEM;
> +}
> +
> +static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
> +{
> + struct bpf_dtab *dtab;
> + int err;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return ERR_PTR(-EPERM);
> +
> + dtab = kzalloc(sizeof(*dtab), GFP_USER);
> + if (!dtab)
> + return ERR_PTR(-ENOMEM);
> +
> + err = dev_map_init_map(dtab, attr, true);
> + if (err) {
> + kfree(dtab);
> + return ERR_PTR(err);
> + }
> +
> + spin_lock(&dev_map_lock);
> + list_add_tail_rcu(&dtab->list, &dev_map_list);
> + spin_unlock(&dev_map_lock);
> +
> + return &dtab->map;
> }
>
> static void dev_map_free(struct bpf_map *map)
> @@ -235,7 +246,7 @@ static int bq_xmit_all(struct xdp_bulk_queue *bq,
> u32 flags,
> out:
> bq->count = 0;
>
> - trace_xdp_devmap_xmit(&obj->dtab->map, obj->bit,
> + trace_xdp_devmap_xmit(&obj->dtab->map, obj->idx,
> sent, drops, bq->dev_rx, dev, err);
> bq->dev_rx = NULL;
> __list_del_clearprev(&bq->flush_node);
> @@ -412,17 +423,52 @@ static int dev_map_delete_elem(struct bpf_map
> *map, void *key)
> return 0;
> }
>
> -static int dev_map_update_elem(struct bpf_map *map, void *key, void
> *value,
> - u64 map_flags)
> +static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
> + struct bpf_dtab *dtab,
> + u32 ifindex,
> + unsigned int idx)
> {
> - struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> - struct net *net = current->nsproxy->net_ns;
> gfp_t gfp = GFP_ATOMIC | __GFP_NOWARN;
> + struct bpf_dtab_netdev *dev;
> + struct xdp_bulk_queue *bq;
> + int cpu;
> +
> + dev = kmalloc_node(sizeof(*dev), gfp, dtab->map.numa_node);
> + if (!dev)
> + return ERR_PTR(-ENOMEM);
> +
> + dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
> + sizeof(void *), gfp);
> + if (!dev->bulkq) {
> + kfree(dev);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + for_each_possible_cpu(cpu) {
> + bq = per_cpu_ptr(dev->bulkq, cpu);
> + bq->obj = dev;
> + }
> +
> + dev->dev = dev_get_by_index(net, ifindex);
> + if (!dev->dev) {
> + free_percpu(dev->bulkq);
> + kfree(dev);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + dev->idx = idx;
> + dev->dtab = dtab;
> +
> + return dev;
> +}
> +
> +static int __dev_map_update_elem(struct net *net, struct bpf_map
> *map,
> + void *key, void *value, u64 map_flags)
> +{
> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> struct bpf_dtab_netdev *dev, *old_dev;
> u32 ifindex = *(u32 *)value;
> - struct xdp_bulk_queue *bq;
> u32 i = *(u32 *)key;
> - int cpu;
>
> if (unlikely(map_flags > BPF_EXIST))
> return -EINVAL;
> @@ -434,31 +480,9 @@ static int dev_map_update_elem(struct bpf_map
> *map, void *key, void *value,
> if (!ifindex) {
> dev = NULL;
> } else {
> - dev = kmalloc_node(sizeof(*dev), gfp, map->numa_node);
> - if (!dev)
> - return -ENOMEM;
> -
> - dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
> - sizeof(void *), gfp);
> - if (!dev->bulkq) {
> - kfree(dev);
> - return -ENOMEM;
> - }
> -
> - for_each_possible_cpu(cpu) {
> - bq = per_cpu_ptr(dev->bulkq, cpu);
> - bq->obj = dev;
> - }
> -
> - dev->dev = dev_get_by_index(net, ifindex);
> - if (!dev->dev) {
> - free_percpu(dev->bulkq);
> - kfree(dev);
> - return -EINVAL;
> - }
> -
> - dev->bit = i;
> - dev->dtab = dtab;
> + dev = __dev_map_alloc_node(net, dtab, ifindex, i);
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> }
>
> /* Use call_rcu() here to ensure rcu critical sections have
> completed
> @@ -472,6 +496,13 @@ static int dev_map_update_elem(struct bpf_map
> *map, void *key, void *value,
> return 0;
> }
>
> +static int dev_map_update_elem(struct bpf_map *map, void *key, void
> *value,
> + u64 map_flags)
> +{
> + return __dev_map_update_elem(current->nsproxy->net_ns,
> + map, key, value, map_flags);
> +}
> +
> const struct bpf_map_ops dev_map_ops = {
> .map_alloc = dev_map_alloc,
> .map_free = dev_map_free,
^ permalink raw reply
* Re: [PATCH bpf-next v2 4/6] xdp: Add devmap_hash map type for looking up devices by hashed index
From: Y Song @ 2019-07-08 15:01 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <874l3w26lb.fsf@toke.dk>
On Mon, Jul 8, 2019 at 2:55 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Y Song <ys114321@gmail.com> writes:
>
> > On Sat, Jul 6, 2019 at 1:48 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> >>
> >> A common pattern when using xdp_redirect_map() is to create a device map
> >> where the lookup key is simply ifindex. Because device maps are arrays,
> >> this leaves holes in the map, and the map has to be sized to fit the
> >> largest ifindex, regardless of how many devices actually are actually
> >> needed in the map.
> >>
> >> This patch adds a second type of device map where the key is looked up
> >> using a hashmap, instead of being used as an array index. This allows maps
> >> to be densely packed, so they can be smaller.
> >>
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> >> ---
> >> include/linux/bpf.h | 7 ++
> >> include/linux/bpf_types.h | 1
> >> include/trace/events/xdp.h | 3 -
> >> kernel/bpf/devmap.c | 192 ++++++++++++++++++++++++++++++++++++++++++++
> >> kernel/bpf/verifier.c | 2
> >> net/core/filter.c | 9 ++
> >> 6 files changed, 211 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >> index bfdb54dd2ad1..f9a506147c8a 100644
> >> --- a/include/linux/bpf.h
> >> +++ b/include/linux/bpf.h
> >> @@ -713,6 +713,7 @@ struct xdp_buff;
> >> struct sk_buff;
> >>
> >> struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
> >> +struct bpf_dtab_netdev *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key);
> >> void __dev_map_flush(struct bpf_map *map);
> >> int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
> >> struct net_device *dev_rx);
> >> @@ -799,6 +800,12 @@ static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
> >> return NULL;
> >> }
> >>
> >> +static inline struct net_device *__dev_map_hash_lookup_elem(struct bpf_map *map,
> >> + u32 key)
> >> +{
> >> + return NULL;
> >> +}
> >> +
> >> static inline void __dev_map_flush(struct bpf_map *map)
> >> {
> >> }
> >> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> >> index eec5aeeeaf92..36a9c2325176 100644
> >> --- a/include/linux/bpf_types.h
> >> +++ b/include/linux/bpf_types.h
> >> @@ -62,6 +62,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
> >> #ifdef CONFIG_NET
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
> >> +BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP_HASH, dev_map_hash_ops)
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
> >> #if defined(CONFIG_BPF_STREAM_PARSER)
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
> >> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
> >> index 68899fdc985b..8c8420230a10 100644
> >> --- a/include/trace/events/xdp.h
> >> +++ b/include/trace/events/xdp.h
> >> @@ -175,7 +175,8 @@ struct _bpf_dtab_netdev {
> >> #endif /* __DEVMAP_OBJ_TYPE */
> >>
> >> #define devmap_ifindex(fwd, map) \
> >> - ((map->map_type == BPF_MAP_TYPE_DEVMAP) ? \
> >> + ((map->map_type == BPF_MAP_TYPE_DEVMAP || \
> >> + map->map_type == BPF_MAP_TYPE_DEVMAP_HASH) ? \
> >> ((struct _bpf_dtab_netdev *)fwd)->dev->ifindex : 0)
> >>
> >> #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
> >> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> >> index a2fe16362129..341af02f049d 100644
> >> --- a/kernel/bpf/devmap.c
> >> +++ b/kernel/bpf/devmap.c
> >> @@ -37,6 +37,12 @@
> >> * notifier hook walks the map we know that new dev references can not be
> >> * added by the user because core infrastructure ensures dev_get_by_index()
> >> * calls will fail at this point.
> >> + *
> >> + * The devmap_hash type is a map type which interprets keys as ifindexes and
> >> + * indexes these using a hashmap. This allows maps that use ifindex as key to be
> >> + * densely packed instead of having holes in the lookup array for unused
> >> + * ifindexes. The setup and packet enqueue/send code is shared between the two
> >> + * types of devmap; only the lookup and insertion is different.
> >> */
> >> #include <linux/bpf.h>
> >> #include <net/xdp.h>
> >> @@ -59,6 +65,7 @@ struct xdp_bulk_queue {
> >>
> >> struct bpf_dtab_netdev {
> >> struct net_device *dev; /* must be first member, due to tracepoint */
> >> + struct hlist_node index_hlist;
> >> struct bpf_dtab *dtab;
> >> unsigned int idx; /* keep track of map index for tracepoint */
> >> struct xdp_bulk_queue __percpu *bulkq;
> >> @@ -70,11 +77,29 @@ struct bpf_dtab {
> >> struct bpf_dtab_netdev **netdev_map;
> >> struct list_head __percpu *flush_list;
> >> struct list_head list;
> >> +
> >> + /* these are only used for DEVMAP_HASH type maps */
> >> + unsigned int items;
> >> + struct hlist_head *dev_index_head;
> >> + spinlock_t index_lock;
> >> };
> >>
> >> static DEFINE_SPINLOCK(dev_map_lock);
> >> static LIST_HEAD(dev_map_list);
> >>
> >> +static struct hlist_head *dev_map_create_hash(void)
> >> +{
> >> + int i;
> >> + struct hlist_head *hash;
> >> +
> >> + hash = kmalloc_array(NETDEV_HASHENTRIES, sizeof(*hash), GFP_KERNEL);
> >> + if (hash != NULL)
> >> + for (i = 0; i < NETDEV_HASHENTRIES; i++)
> >> + INIT_HLIST_HEAD(&hash[i]);
> >> +
> >> + return hash;
> >> +}
> >> +
> >> static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> >> bool check_memlock)
> >> {
> >> @@ -98,6 +123,9 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> >> cost = (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
> >> cost += sizeof(struct list_head) * num_possible_cpus();
> >>
> >> + if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH)
> >> + cost += sizeof(struct hlist_head) * NETDEV_HASHENTRIES;
> >> +
> >> /* if map size is larger than memlock limit, reject it */
> >> err = bpf_map_charge_init(&dtab->map.memory, cost);
> >> if (err)
> >> @@ -116,8 +144,18 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> >> if (!dtab->netdev_map)
> >> goto free_percpu;
> >>
> >> + if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
> >> + dtab->dev_index_head = dev_map_create_hash();
> >> + if (!dtab->dev_index_head)
> >> + goto free_map_area;
> >> +
> >> + spin_lock_init(&dtab->index_lock);
> >> + }
> >> +
> >> return 0;
> >>
> >> +free_map_area:
> >> + bpf_map_area_free(dtab->netdev_map);
> >> free_percpu:
> >> free_percpu(dtab->flush_list);
> >> free_charge:
> >> @@ -199,6 +237,7 @@ static void dev_map_free(struct bpf_map *map)
> >>
> >> free_percpu(dtab->flush_list);
> >> bpf_map_area_free(dtab->netdev_map);
> >> + kfree(dtab->dev_index_head);
> >> kfree(dtab);
> >> }
> >>
> >> @@ -219,6 +258,70 @@ static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
> >> return 0;
> >> }
> >>
> >> +static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
> >> + int idx)
> >> +{
> >> + return &dtab->dev_index_head[idx & (NETDEV_HASHENTRIES - 1)];
> >> +}
> >> +
> >> +struct bpf_dtab_netdev *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key)
> >> +{
> >> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> >> + struct hlist_head *head = dev_map_index_hash(dtab, key);
> >> + struct bpf_dtab_netdev *dev;
> >> +
> >> + hlist_for_each_entry_rcu(dev, head, index_hlist)
> >> + if (dev->idx == key)
> >> + return dev;
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> +static int dev_map_hash_get_next_key(struct bpf_map *map, void *key,
> >> + void *next_key)
> >> +{
> >> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> >> + u32 idx, *next = next_key;
> >> + struct bpf_dtab_netdev *dev, *next_dev;
> >> + struct hlist_head *head;
> >> + int i = 0;
> >> +
> >> + if (!key)
> >> + goto find_first;
> >> +
> >> + idx = *(u32 *)key;
> >> +
> >> + dev = __dev_map_hash_lookup_elem(map, idx);
> >> + if (!dev)
> >> + goto find_first;
> >> +
> >> + next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(&dev->index_hlist)),
> >> + struct bpf_dtab_netdev, index_hlist);
> >
> > Just want to get a better understanding. Why do you want
> > hlist_entry_safe instead of hlist_entry?
>
> Erm, because the entry might not exist? The _safe variant just checks
> the list pointer before casting to the containing struct.
hlist_entry_safe is certainly correct. Just want to better understand when
the entry might not exist. By looking at hashtab.c implementation, it has
the same possible-null-entry assumption, so we should be ok here.
>
> > Also, maybe rcu_dereference instead of rcu_dereference_raw?
>
> Well, the _raw() variant comes from the equivalent function in
> hashtab.c, where I originally nicked most of this function, so think it
> makes more sense to keep them the same.
Okay.
>
> > dev_map_hash_get_next_key() is called in syscall.c within rcu_read_lock region.
> >
> >> +
> >> + if (next_dev) {
> >> + *next = next_dev->idx;
> >> + return 0;
> >> + }
> >> +
> >> + i = idx & (NETDEV_HASHENTRIES - 1);
> >> + i++;
> >> +
> >> + find_first:
> >> + for (; i < NETDEV_HASHENTRIES; i++) {
> >> + head = dev_map_index_hash(dtab, i);
> >> +
> >> + next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),
> >> + struct bpf_dtab_netdev,
> >> + index_hlist);
> >
> > ditto. The same question as the above.
> >
> >> + if (next_dev) {
> >> + *next = next_dev->idx;
> >> + return 0;
> >> + }
> >> + }
> >> +
> >> + return -ENOENT;
> >> +}
> >> +
> >> static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags,
> >> bool in_napi_ctx)
> >> {
> >> @@ -374,6 +477,15 @@ static void *dev_map_lookup_elem(struct bpf_map *map, void *key)
> >> return dev ? &dev->ifindex : NULL;
> >> }
> >>
> >> +static void *dev_map_hash_lookup_elem(struct bpf_map *map, void *key)
> >> +{
> >> + struct bpf_dtab_netdev *obj = __dev_map_hash_lookup_elem(map,
> >> + *(u32 *)key);
> >> + struct net_device *dev = obj ? obj->dev : NULL;
> >
> > When obj->dev could be NULL?
>
> Never. This could just as well be written as
>
> return obj ? obj->dev->ifindex : NULL;
>
> but writing it this way keeps it consistent with the existing
> dev_map_lookup_elem() function.
Okay then in order to be consistent with the existing code base.
Thanks for all the explanation!
^ permalink raw reply
* [PATCH net,v2] nfc: fix potential illegal memory access
From: Yang Wei @ 2019-07-08 14:57 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: davem, tglx, albin_yang
The frags_q is not properly initialized, it may result in illegal memory
access when conn_info is NULL.
The "goto free_exit" should be replaced by "goto exit".
Signed-off-by: Yang Wei <albin_yang@163.com>
---
net/nfc/nci/data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/nfc/nci/data.c b/net/nfc/nci/data.c
index 0a0c265..ce3382b 100644
--- a/net/nfc/nci/data.c
+++ b/net/nfc/nci/data.c
@@ -107,7 +107,7 @@ static int nci_queue_tx_data_frags(struct nci_dev *ndev,
conn_info = nci_get_conn_info_by_conn_id(ndev, conn_id);
if (!conn_info) {
rc = -EPROTO;
- goto free_exit;
+ goto exit;
}
__skb_queue_head_init(&frags_q);
--
2.7.4
^ permalink raw reply related
* Re: i.mx6ul with DSA in multi chip addressing mode - no MDIO access
From: Andrew Lunn @ 2019-07-08 14:57 UTC (permalink / raw)
To: Benjamin Beckmeyer; +Cc: netdev
In-Reply-To: <5e35a41c-be0e-efd4-cb69-cf5c860b872e@eks-engel.de>
> Hi Andrew,
> I got it working a little bit better. When I'm fast enough I can read
> the registers I want but it isn't a solution.
Why do you need to read registers?
What you actually might be interested in is the debugfs patches in
Viviens github tree.
> Here is an output of the tracing even with my custom accesses.
> mii -i 2 0 0x9b60; mii -i 2 1
> phyid:2, reg:0x01 -> 0xc801
>
> Do you know how to delete EEInt bit? It is always one. And now all
> accesses coming from the kworker thread. Maybe this is your polling
> function?
EEInt should clear on read for older chips. For 6390, it might be
necessary to read global 2, register 0x13, index 03.
> I view the INT pin on an oscilloscope but it never changed. So maybe
> this is the problem. We just soldered a pull-up to that pin but it
> still never changend. Maybe you have an idea?
The EEInt bit is probably masked. So it will not generate in
interrupt.
> So what I think is, because of the EEInt bit is never set back to one
> i will poll it as fast as possible.
Is it forever looping in mv88e6xxx_g1_irq_thread_work? Or is it the
polling code, mv88e6xxx_irq_poll() firing every 100ms?
Andrew
^ permalink raw reply
* Re: [PATCH net-next 3/4] bnxt_en: optimized XDP_REDIRECT support
From: Ilias Apalodimas @ 2019-07-08 14:51 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: Michael Chan, davem, netdev, hawk, ast, ivan.khoronzhuk
In-Reply-To: <20190708142606.GF87269@C02RW35GFVH8.dhcp.broadcom.net>
Hi Andy,
> On Mon, Jul 08, 2019 at 11:28:03AM +0300, Ilias Apalodimas wrote:
> > Thanks Andy, Michael
> >
> > > + if (event & BNXT_REDIRECT_EVENT)
> > > + xdp_do_flush_map();
> > > +
> > > if (event & BNXT_TX_EVENT) {
> > > struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
> > > u16 prod = txr->tx_prod;
> > > @@ -2254,9 +2257,23 @@ static void bnxt_free_tx_skbs(struct bnxt *bp)
> > >
> > > for (j = 0; j < max_idx;) {
> > > struct bnxt_sw_tx_bd *tx_buf = &txr->tx_buf_ring[j];
> > > - struct sk_buff *skb = tx_buf->skb;
> > > + struct sk_buff *skb;
> > > int k, last;
> > >
> > > + if (i < bp->tx_nr_rings_xdp &&
> > > + tx_buf->action == XDP_REDIRECT) {
> > > + dma_unmap_single(&pdev->dev,
> > > + dma_unmap_addr(tx_buf, mapping),
> > > + dma_unmap_len(tx_buf, len),
> > > + PCI_DMA_TODEVICE);
> > > + xdp_return_frame(tx_buf->xdpf);
> > > + tx_buf->action = 0;
> > > + tx_buf->xdpf = NULL;
> > > + j++;
> > > + continue;
> > > + }
> > > +
> >
> > Can't see the whole file here and maybe i am missing something, but since you
> > optimize for that and start using page_pool, XDP_TX will be a re-synced (and
> > not remapped) buffer that can be returned to the pool and resynced for
> > device usage.
> > Is that happening later on the tx clean function?
>
> Take a look at the way we treat the buffers in bnxt_rx_xdp() when we
> receive them and then in bnxt_tx_int_xdp() when the transmits have
> completed (for XDP_TX and XDP_REDIRECT). I think we are doing what is
> proper with respect to mapping vs sync for both cases, but I would be
> fine to be corrected.
>
Yea seems to be doing the right thing,
XDP_TX syncs correctly and reuses with bnxt_reuse_rx_data() right?
This might be a bit confusing for someone reading the driver on the first time,
probably because you'll end up with 2 ways of recycling buffers.
Once a buffers get freed on the XDP path it's either fed back to the pool, so
the next requested buffer get served from the pools cache (ndo_xdp_xmit case in
the patch). If the buffer is used for XDP_TX is's synced correctly but recycled
via bnxt_reuse_rx_data() right? Since you are moving to page pool please
consider having a common approach towards the recycling path. I understand that
means tracking buffers types and make sure you do the right thing on 'tx clean'.
I've done something similar on the netsec driver and i do think this might be a
good thing to add on page_pool API
Again this isn't a blocker at least for me but you already have the buffer type
(via tx_buf->action)
> >
> > > + skb = tx_buf->skb;
> > > if (!skb) {
> > > j++;
> > > continue;
> > > @@ -2517,6 +2534,13 @@ static int bnxt_alloc_rx_rings(struct bnxt *bp)
> > > if (rc < 0)
> > > return rc;
> > >
> > > + rc = xdp_rxq_info_reg_mem_model(&rxr->xdp_rxq,
> > > + MEM_TYPE_PAGE_SHARED, NULL);
> > > + if (rc) {
> > > + xdp_rxq_info_unreg(&rxr->xdp_rxq);
> >
> > I think you can use page_pool_free directly here (and pge_pool_destroy once
> > Ivan's patchset gets nerged), that's what mlx5 does iirc. Can we keep that
> > common please?
>
> That's an easy change, I can do that.
>
> >
> > If Ivan's patch get merged please note you'll have to explicitly
> > page_pool_destroy, after calling xdp_rxq_info_unreg() in the general unregister
> > case (not the error habdling here). Sorry for the confusion this might bring!
>
> Funny enough the driver was basically doing that until page_pool_destroy
> was removed (these patches are not new). I saw last week there was
> discussion to add it back, but I did not want to wait to get this on the
> list before that was resolved.
Fair enough
>
> This path works as expected with the code in the tree today so it seemed
> like the correct approach to post something that is working, right? :-)
Yes.
It will continue to work even if you dont change the call in the future.
This is more a 'let's not spread the code' attempt, but removing and re-adding
page_pool_destroy() was/is our mess. We might as well live with the
consequences!
>
> >
> > > + return rc;
> > > + }
> > > +
> > > rc = bnxt_alloc_ring(bp, &ring->ring_mem);
> > > if (rc)
> > > return rc;
> > > @@ -10233,6 +10257,7 @@ static const struct net_device_ops bnxt_netdev_ops = {
> > [...]
> >
Thanks!
/Ilias
^ permalink raw reply
* Re: [PATCH] ath10k: work around uninitialized vht_pfr variable
From: Nathan Chancellor @ 2019-07-08 14:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Kalle Valo, Miaoqing Pan, David S. Miller, Rakesh Pillai,
Brian Norris, Balaji Pothunoori, Wen Gong, Pradeep kumar Chitrapu,
Sriram R, ath10k, linux-wireless, netdev, linux-kernel,
clang-built-linux
In-Reply-To: <20190708125050.3689133-1-arnd@arndb.de>
On Mon, Jul 08, 2019 at 02:50:06PM +0200, Arnd Bergmann wrote:
> As clang points out, the vht_pfr is assigned to a struct member
> without being initialized in one case:
>
> drivers/net/wireless/ath/ath10k/mac.c:7528:7: error: variable 'vht_pfr' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7551:20: note: uninitialized use occurs here
> arvif->vht_pfr = vht_pfr;
> ^~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7528:3: note: remove the 'if' if its condition is always true
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7483:12: note: initialize the variable 'vht_pfr' to silence this warning
> u8 vht_pfr;
>
> Add an explicit but probably incorrect initialization here.
> I suspect we want a better fix here, but chose this approach to
> illustrate the issue.
Yup, I reached out to the maintainers when this issue first cropped up,
should have taken your approach though.
https://lore.kernel.org/lkml/20190702181837.GA118849@archlinux-epyc/
Initializing to zero is better than uninitialized.
>
> Fixes: 8b97b055dc9d ("ath10k: fix failure to set multiple fixed rate")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v5 2/5] devlink: Return physical port fields only for applicable port flavours
From: Jiri Pirko @ 2019-07-08 14:44 UTC (permalink / raw)
To: Parav Pandit; +Cc: netdev, jiri, saeedm, jakub.kicinski
In-Reply-To: <20190708041549.56601-3-parav@mellanox.com>
Mon, Jul 08, 2019 at 06:15:46AM CEST, parav@mellanox.com wrote:
>Physical port number and split group fields are applicable only to
>physical port flavours such as PHYSICAL, CPU and DSA.
>Hence limit returning those values in netlink response to such port
>flavours.
>
>Signed-off-by: Parav Pandit <parav@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v5 1/5] devlink: Refactor physical port attributes
From: Jiri Pirko @ 2019-07-08 14:44 UTC (permalink / raw)
To: Parav Pandit; +Cc: netdev, jiri, saeedm, jakub.kicinski
In-Reply-To: <20190708041549.56601-2-parav@mellanox.com>
Mon, Jul 08, 2019 at 06:15:45AM CEST, parav@mellanox.com wrote:
>To support additional devlink port flavours and to support few common
>and few different port attributes, move physical port attributes to a
>different structure.
>
>Signed-off-by: Parav Pandit <parav@mellanox.com>
>---
>Changelog:
>v4->v5:
> - Addressed comments from Jiri.
> - Moved check for physical port flavours check to separate patch.
>v3->v4:
> - Addressed comments from Jiri.
> - Renamed phys_port to physical to be consistent with pci_pf.
> - Removed port_number from __devlink_port_attrs_set and moved
> assigment to caller function.
> - Used capital letter while moving old comment to new structure.
> - Removed helper function is_devlink_phy_port_num_supported().
>v2->v3:
> - Address comments from Jakub.
> - Made port_number and split_port_number applicable only to
> physical port flavours by having in union.
>v1->v2:
> - Limited port_num attribute to physical ports
> - Updated PCI PF attribute set API to not have port_number
>---
> include/net/devlink.h | 13 ++++++++--
> net/core/devlink.c | 59 ++++++++++++++++++++++++++++---------------
> 2 files changed, 50 insertions(+), 22 deletions(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 6625ea068d5e..c79a1370867a 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -38,14 +38,23 @@ struct devlink {
> char priv[0] __aligned(NETDEV_ALIGN);
> };
>
>+struct devlink_port_phys_attrs {
>+ u32 port_number; /* Same value as "split group".
>+ * A physical port which is visible to the user
>+ * for a given port flavour.
>+ */
>+ u32 split_subport_number;
>+};
>+
> struct devlink_port_attrs {
> u8 set:1,
> split:1,
> switch_port:1;
> enum devlink_port_flavour flavour;
>- u32 port_number; /* same value as "split group" */
>- u32 split_subport_number;
> struct netdev_phys_item_id switch_id;
>+ union {
>+ struct devlink_port_phys_attrs physical;
You can shorten this to just "phys". Would be better.
With that
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* [PATCH net-next] net: dsa: vsc73xx: Fix Kconfig warning and build errors
From: YueHaibing @ 2019-07-08 14:42 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, davem, paweldembicki
Cc: linux-kernel, netdev, YueHaibing
Fix Kconfig dependency warning and subsequent build errors
caused by OF is not set:
WARNING: unmet direct dependencies detected for NET_DSA_VITESSE_VSC73XX
Depends on [n]: NETDEVICES [=y] && HAVE_NET_DSA [=y] && OF [=n] && NET_DSA [=m]
Selected by [m]:
- NET_DSA_VITESSE_VSC73XX_PLATFORM [=m] && NETDEVICES [=y] && HAVE_NET_DSA [=y] && HAS_IOMEM [=y]
Move OF and NET_DSA dependencies to NET_DSA_VITESSE_VSC73XX/
NET_DSA_VITESSE_VSC73XX_PLATFORM to fix this.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 631e83bf7c0e ("net: dsa: vsc73xx: add support for parallel mode")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/dsa/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index cf9dbd1..e28c209 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -100,8 +100,6 @@ config NET_DSA_SMSC_LAN9303_MDIO
config NET_DSA_VITESSE_VSC73XX
tristate
- depends on OF
- depends on NET_DSA
select FIXED_PHY
select VITESSE_PHY
select GPIOLIB
@@ -112,6 +110,7 @@ config NET_DSA_VITESSE_VSC73XX
config NET_DSA_VITESSE_VSC73XX_SPI
tristate "Vitesse VSC7385/7388/7395/7398 SPI mode support"
depends on SPI
+ depends on OF && NET_DSA
select NET_DSA_VITESSE_VSC73XX
---help---
This enables support for the Vitesse VSC7385, VSC7388, VSC7395
@@ -120,6 +119,7 @@ config NET_DSA_VITESSE_VSC73XX_SPI
config NET_DSA_VITESSE_VSC73XX_PLATFORM
tristate "Vitesse VSC7385/7388/7395/7398 Platform mode support"
depends on HAS_IOMEM
+ depends on OF && NET_DSA
select NET_DSA_VITESSE_VSC73XX
---help---
This enables support for the Vitesse VSC7385, VSC7388, VSC7395
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Jiri Pirko @ 2019-07-08 14:40 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190708122251.GB24474@unicorn.suse.cz>
Mon, Jul 08, 2019 at 02:22:51PM CEST, mkubecek@suse.cz wrote:
>On Wed, Jul 03, 2019 at 12:04:35PM +0200, Jiri Pirko wrote:
>> Tue, Jul 02, 2019 at 06:34:37PM CEST, mkubecek@suse.cz wrote:
>> >On Tue, Jul 02, 2019 at 03:05:15PM +0200, Jiri Pirko wrote:
>> >> Tue, Jul 02, 2019 at 01:50:04PM CEST, mkubecek@suse.cz wrote:
>> >> >+/**
>> >> >+ * ethnl_is_privileged() - check if request has sufficient privileges
>> >> >+ * @skb: skb with client request
>> >> >+ *
>> >> >+ * Checks if client request has CAP_NET_ADMIN in its netns. Unlike the flags
>> >> >+ * in genl_ops, this allows finer access control, e.g. allowing or denying
>> >> >+ * the request based on its contents or witholding only part of the data
>> >> >+ * from unprivileged users.
>> >> >+ *
>> >> >+ * Return: true if request is privileged, false if not
>> >> >+ */
>> >> >+static inline bool ethnl_is_privileged(struct sk_buff *skb)
>> >>
>> >> I wonder why you need this helper. Genetlink uses
>> >> ops->flags & GENL_ADMIN_PERM for this.
>> >
>> >It's explained in the function description. Sometimes we need finer
>> >control than by request message type. An example is the WoL password:
>> >ETHTOOL_GWOL is privileged because of it but I believe there si no
>> >reason why unprivileged user couldn't see enabled WoL modes, we can
>> >simply omit the password for him. (Also, it allows to combine query for
>> >WoL settings with other unprivileged settings.)
>>
>> Why can't we have rather:
>> ETHTOOL_WOL_GET for all
>> ETHTOOL_WOL_PASSWORD_GET with GENL_ADMIN_PERM
>> ?
>> Better to stick with what we have in gennetlink rather then to bend the
>> implementation from the very beginning I think.
>
>We can. But it would also mean two separate SET requests (or breaking
>the rule that _GET_REPLY, _SET and _NTF share the layout). That would be
>unfortunate as ethtool_ops callback does not actually allow setting only
>the modes so that the ETHTOOL_MSG_WOL_SET request (which would have to
>go first as many drivers ignore .sopass if WAKE_MAGICSECURE is not set)
>would have to pass a different password (most likely just leaving what
>->get_wol() put there) and that password would be actually set until the
>second request arrives. There goes the idea of getting rid of ioctl
>interface raciness...
I understand. That is my concern, not to bring baggage from ioclt :/
>
>I would rather see returning to WoL modes not being visible to
>unprivileged users than that (even if there is no actual reason for it).
>Anyway, shortening the series left WoL settings out if the first part so
>that I can split this out for now and leave the discussion for when we
>get to WoL one day.
Fine.
>
>> >> >+/**
>> >> >+ * ethnl_reply_header_size() - total size of reply header
>> >> >+ *
>> >> >+ * This is an upper estimate so that we do not need to hold RTNL lock longer
>> >> >+ * than necessary (to prevent rename between size estimate and composing the
>> >>
>> >> I guess this description is not relevant anymore. I don't see why to
>> >> hold rtnl mutex for this function...
>> >
>> >You don't need it for this function, it's the other way around: unless
>> >you hold RTNL lock for the whole time covering both checking needed
>> >message size and filling the message - and we don't - the device could
>> >be renamed in between. Thus if we returned size based on current device
>> >name, it might not be sufficient at the time the header is filled.
>> >That's why this function returns maximum possible size (which is
>> >actually a constant).
>>
>> I suggest to avoid the description. It is misleading. Perhaps something
>> to have in a patch description but not here in code.
>
>The reason I put the comment there was to prevent someone "optimizing"
>the helper by using strlen() later. Maybe something shorter and more to
>the point, e.g.
>
> Using IFNAMSIZ is faster and prevents a race if the device is renamed
> before we fill the name into skb.
>
>?
Sounds good, thanks!
>
>Michal
^ 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