Netdev List
 help / color / mirror / Atom feed
* [PATCH bpf-next v9 9/9] bpftool: support cgroup sockopt
From: Stanislav Fomichev @ 2019-06-27 20:38 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko,
	Martin Lau, Jakub Kicinski
In-Reply-To: <20190627203855.10515-1-sdf@google.com>

Support sockopt prog type and cgroup hooks in the bpftool.

Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Martin Lau <kafai@fb.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 7 +++++--
 tools/bpf/bpftool/Documentation/bpftool-prog.rst   | 3 ++-
 tools/bpf/bpftool/bash-completion/bpftool          | 9 ++++++---
 tools/bpf/bpftool/cgroup.c                         | 5 ++++-
 tools/bpf/bpftool/main.h                           | 1 +
 tools/bpf/bpftool/prog.c                           | 3 ++-
 6 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index d80fdde79c22..585f270c2d25 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -29,7 +29,8 @@ CGROUP COMMANDS
 |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
 |	*ATTACH_TYPE* := { **ingress** | **egress** | **sock_create** | **sock_ops** | **device** |
 |		**bind4** | **bind6** | **post_bind4** | **post_bind6** | **connect4** | **connect6** |
-|		**sendmsg4** | **sendmsg6** | **recvmsg4** | **recvmsg6** | **sysctl** }
+|		**sendmsg4** | **sendmsg6** | **recvmsg4** | **recvmsg6** | **sysctl** |
+|		**getsockopt** | **setsockopt** }
 |	*ATTACH_FLAGS* := { **multi** | **override** }
 
 DESCRIPTION
@@ -90,7 +91,9 @@ DESCRIPTION
                   an unconnected udp4 socket (since 5.2);
 		  **recvmsg6** call to recvfrom(2), recvmsg(2), recvmmsg(2) for
                   an unconnected udp6 socket (since 5.2);
-		  **sysctl** sysctl access (since 5.2).
+		  **sysctl** sysctl access (since 5.2);
+		  **getsockopt** call to getsockopt (since 5.3);
+		  **setsockopt** call to setsockopt (since 5.3).
 
 	**bpftool cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
 		  Detach *PROG* from the cgroup *CGROUP* and attach type
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 55dd06517a3b..1df637f85f94 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -40,7 +40,8 @@ PROG COMMANDS
 |		**lwt_seg6local** | **sockops** | **sk_skb** | **sk_msg** | **lirc_mode2** |
 |		**cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
 |		**cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6** |
-|		**cgroup/recvmsg4** | **cgroup/recvmsg6** | **cgroup/sysctl**
+|		**cgroup/recvmsg4** | **cgroup/recvmsg6** | **cgroup/sysctl** |
+|		**cgroup/getsockopt** | **cgroup/setsockopt**
 |	}
 |       *ATTACH_TYPE* := {
 |		**msg_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index a17e84c67498..ba37095e1f62 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -379,7 +379,8 @@ _bpftool()
                                 cgroup/sendmsg4 cgroup/sendmsg6 \
                                 cgroup/recvmsg4 cgroup/recvmsg6 \
                                 cgroup/post_bind4 cgroup/post_bind6 \
-                                cgroup/sysctl" -- \
+                                cgroup/sysctl cgroup/getsockopt \
+                                cgroup/setsockopt" -- \
                                                    "$cur" ) )
                             return 0
                             ;;
@@ -689,7 +690,8 @@ _bpftool()
                 attach|detach)
                     local ATTACH_TYPES='ingress egress sock_create sock_ops \
                         device bind4 bind6 post_bind4 post_bind6 connect4 \
-                        connect6 sendmsg4 sendmsg6 recvmsg4 recvmsg6 sysctl'
+                        connect6 sendmsg4 sendmsg6 recvmsg4 recvmsg6 sysctl \
+                        getsockopt setsockopt'
                     local ATTACH_FLAGS='multi override'
                     local PROG_TYPE='id pinned tag'
                     case $prev in
@@ -699,7 +701,8 @@ _bpftool()
                             ;;
                         ingress|egress|sock_create|sock_ops|device|bind4|bind6|\
                         post_bind4|post_bind6|connect4|connect6|sendmsg4|\
-                        sendmsg6|recvmsg4|recvmsg6|sysctl)
+                        sendmsg6|recvmsg4|recvmsg6|sysctl|getsockopt|\
+                        setsockopt)
                             COMPREPLY=( $( compgen -W "$PROG_TYPE" -- \
                                 "$cur" ) )
                             return 0
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 73ec8ea33fb4..390b89a224f1 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -26,7 +26,8 @@
 	"                        sock_ops | device | bind4 | bind6 |\n"	       \
 	"                        post_bind4 | post_bind6 | connect4 |\n"       \
 	"                        connect6 | sendmsg4 | sendmsg6 |\n"           \
-	"                        recvmsg4 | recvmsg6 | sysctl }"
+	"                        recvmsg4 | recvmsg6 | sysctl |\n"	       \
+	"                        getsockopt | setsockopt }"
 
 static const char * const attach_type_strings[] = {
 	[BPF_CGROUP_INET_INGRESS] = "ingress",
@@ -45,6 +46,8 @@ static const char * const attach_type_strings[] = {
 	[BPF_CGROUP_SYSCTL] = "sysctl",
 	[BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
 	[BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
+	[BPF_CGROUP_GETSOCKOPT] = "getsockopt",
+	[BPF_CGROUP_SETSOCKOPT] = "setsockopt",
 	[__MAX_BPF_ATTACH_TYPE] = NULL,
 };
 
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 28a2a5857e14..9c5d9c80f71e 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -74,6 +74,7 @@ static const char * const prog_type_name[] = {
 	[BPF_PROG_TYPE_SK_REUSEPORT]		= "sk_reuseport",
 	[BPF_PROG_TYPE_FLOW_DISSECTOR]		= "flow_dissector",
 	[BPF_PROG_TYPE_CGROUP_SYSCTL]		= "cgroup_sysctl",
+	[BPF_PROG_TYPE_CGROUP_SOCKOPT]		= "cgroup_sockopt",
 };
 
 extern const char * const map_type_name[];
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index f1a831f05010..9b0db5d14e31 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1071,7 +1071,8 @@ static int do_help(int argc, char **argv)
 		"                 cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
 		"                 cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
 		"                 cgroup/sendmsg4 | cgroup/sendmsg6 | cgroup/recvmsg4 |\n"
-		"                 cgroup/recvmsg6 }\n"
+		"                 cgroup/recvmsg6 | cgroup/getsockopt |\n"
+		"                 cgroup/setsockopt }\n"
 		"       ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
 		"                        flow_dissector }\n"
 		"       " HELP_SPEC_OPTIONS "\n"
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* Re: [PATCH 8/9] keys: Network namespace domain tag [ver #4]
From: Willem de Bruijn @ 2019-06-27 20:55 UTC (permalink / raw)
  To: David Howells
  Cc: Eric W. Biederman, keyrings, linux-cifs, linux-nfs,
	Network Development, linux-afs, dwalsh, vgoyal,
	linux-security-module, linux-fsdevel, LKML
In-Reply-To: <156096287188.28733.15342608110117616221.stgit@warthog.procyon.org.uk>

On Wed, Jun 19, 2019 at 12:49 PM David Howells <dhowells@redhat.com> wrote:
>
> Create key domain tags for network namespaces and make it possible to
> automatically tag keys that are used by networked services (e.g. AF_RXRPC,
> AFS, DNS) with the default network namespace if not set by the caller.
>
> This allows keys with the same description but in different namespaces to
> coexist within a keyring.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: netdev@vger.kernel.org
> cc: linux-nfs@vger.kernel.org
> cc: linux-cifs@vger.kernel.org
> cc: linux-afs@lists.infradead.org
> ---

> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 711b161505ac..076a75c73c9e 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -38,9 +38,16 @@ EXPORT_SYMBOL_GPL(net_namespace_list);
>  DECLARE_RWSEM(net_rwsem);
>  EXPORT_SYMBOL_GPL(net_rwsem);
>
> +#ifdef CONFIG_KEYS
> +static struct key_tag init_net_key_domain = { .usage = REFCOUNT_INIT(1) };
> +#endif
> +
>  struct net init_net = {
>         .count          = REFCOUNT_INIT(1),
>         .dev_base_head  = LIST_HEAD_INIT(init_net.dev_base_head),
> +#ifdef CONFIG_KEYS
> +       .key_domain     = &init_net_key_domain,
> +#endif
>  };
>  EXPORT_SYMBOL(init_net);
>
> @@ -386,10 +393,21 @@ static struct net *net_alloc(void)
>         if (!net)
>                 goto out_free;
>
> +#ifdef CONFIG_KEYS
> +       net->key_domain = kzalloc(sizeof(struct key_tag), GFP_KERNEL);
> +       if (!net->key_domain)
> +               goto out_free_2;
> +       refcount_set(&net->key_domain->usage, 1);
> +#endif
> +
>         rcu_assign_pointer(net->gen, ng);
>  out:
>         return net;
>
> +#ifdef CONFIG_KEYS
> +out_free_2:
> +       kmem_cache_free(net_cachep, net);

needs
            net = NULL;

to signal failure

> +#endif
>  out_free:
>         kfree(ng);
>         goto out;

Reported-by: syzbot <syzkaller@googlegroups.com>

BUG: KASAN: use-after-free in atomic_set
include/asm-generic/atomic-instrumented.h:44 [inline]
BUG: KASAN: use-after-free in refcount_set include/linux/refcount.h:32 [inline]
BUG: KASAN: use-after-free in copy_net_ns+0x1e8/0x431
net/core/net_namespace.c:466
Write of size 4 at addr ffff88809c9de080 by task syz-executor.1/12624

CPU: 1 PID: 12624 Comm: syz-executor.1 Not tainted 5.2.0-rc6-next-20190626 #23
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
 print_address_description.cold+0xd4/0x306 mm/kasan/report.c:351
 __kasan_report.cold+0x1b/0x36 mm/kasan/report.c:482
 kasan_report+0x12/0x17 mm/kasan/common.c:614
 check_memory_region_inline mm/kasan/generic.c:185 [inline]
 check_memory_region+0x123/0x190 mm/kasan/generic.c:191
 kasan_check_write+0x14/0x20 mm/kasan/common.c:100
 atomic_set include/asm-generic/atomic-instrumented.h:44 [inline]
 refcount_set include/linux/refcount.h:32 [inline]
 copy_net_ns+0x1e8/0x431 net/core/net_namespace.c:466
 create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103
 unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202
 ksys_unshare+0x444/0x980 kernel/fork.c:2828
 __do_sys_unshare kernel/fork.c:2896 [inline]
 __se_sys_unshare kernel/fork.c:2894 [inline]
 __x64_sys_unshare+0x31/0x40 kernel/fork.c:2894
 do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:301
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459519
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f2202261c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000110
RAX: ffffffffffffffda RBX: 00007f2202261c90 RCX: 0000000000459519
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000040000000
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f22022626d4
R13: 00000000004c8a2c R14: 00000000004df7d0 R15: 0000000000000006

Allocated by task 12624:
 save_stack+0x23/0x90 mm/kasan/common.c:71
 set_track mm/kasan/common.c:79 [inline]
 __kasan_kmalloc mm/kasan/common.c:489 [inline]
 __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:462
 kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:497
 slab_post_alloc_hook mm/slab.h:520 [inline]
 slab_alloc mm/slab.c:3320 [inline]
 kmem_cache_alloc+0x121/0x710 mm/slab.c:3484
 kmem_cache_zalloc include/linux/slab.h:737 [inline]
 net_alloc net/core/net_namespace.c:410 [inline]
 copy_net_ns+0xf1/0x431 net/core/net_namespace.c:461
 create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103
 unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202
 ksys_unshare+0x444/0x980 kernel/fork.c:2828
 __do_sys_unshare kernel/fork.c:2896 [inline]
 __se_sys_unshare kernel/fork.c:2894 [inline]
 __x64_sys_unshare+0x31/0x40 kernel/fork.c:2894
 do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:301
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 12624:
 save_stack+0x23/0x90 mm/kasan/common.c:71
 set_track mm/kasan/common.c:79 [inline]
 __kasan_slab_free+0x102/0x150 mm/kasan/common.c:451
 kasan_slab_free+0xe/0x10 mm/kasan/common.c:459
 __cache_free mm/slab.c:3426 [inline]
 kmem_cache_free+0x86/0x320 mm/slab.c:3694
 net_alloc net/core/net_namespace.c:427 [inline]
 copy_net_ns+0x3b1/0x431 net/core/net_namespace.c:461
 create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103
 unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202
 ksys_unshare+0x444/0x980 kernel/fork.c:2828
 __do_sys_unshare kernel/fork.c:2896 [inline]
 __se_sys_unshare kernel/fork.c:2894 [inline]
 __x64_sys_unshare+0x31/0x40 kernel/fork.c:2894
 do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:301
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

^ permalink raw reply

* Re: [PATCH net] net/ibmvnic: Report last valid speed and duplex values to ethtool
From: Thomas Falcon @ 2019-06-27 20:56 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, linuxppc-dev, bjking1, pradeep, dnbanerg
In-Reply-To: <20190627175715.GP27733@lunn.ch>


On 6/27/19 12:57 PM, Andrew Lunn wrote:
> On Thu, Jun 27, 2019 at 12:09:13PM -0500, Thomas Falcon wrote:
>> This patch resolves an issue with sensitive bonding modes
>> that require valid speed and duplex settings to function
>> properly. Currently, the adapter will report that device
>> speed and duplex is unknown if the communication link
>> with firmware is unavailable.
> Dumb question. If you cannot communicate with the firmware, isn't the
> device FUBAR? So setting the LACP port to disabled is the correct
> things to do.
>
>         Andrew
>
Yes, I think that is correct too.  The problem is that the link is only 
down temporarily.  In this case - we are testing with a pseries logical 
partition - the partition is migrated to another server. The driver must 
wait for a signal from the hypervisor to resume operation with the new 
device.  Once it resumes, we see that the device reboots and gets 
correct speed settings, but the port flag (AD_LACP_PORT_ENABLED) is 
still cleared.

Tom


^ permalink raw reply

* Re: [PATCH v2 bpf-next 1/3] libbpf: add perf buffer API
From: Daniel Borkmann @ 2019-06-27 21:04 UTC (permalink / raw)
  To: Andrii Nakryiko, andrii.nakryiko, ast, bpf, netdev, kernel-team
In-Reply-To: <20190626061235.602633-2-andriin@fb.com>

On 06/26/2019 08:12 AM, Andrii Nakryiko wrote:
> BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
> to user space for additional processing. libbpf already has very low-level API
> to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
> use and requires a lot of code to set everything up. This patch adds
> perf_buffer abstraction on top of it, abstracting setting up and polling
> per-CPU logic into simple and convenient API, similar to what BCC provides.
> 
> perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
> map entries. It accepts two user-provided callbacks: one for handling raw
> samples and one for get notifications of lost samples due to buffer overflow.
> 
> perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
> utilizing epoll instance.
> 
> perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
> 
> All APIs are not thread-safe. User should ensure proper locking/coordination if
> used in multi-threaded set up.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Aside from current feedback, this series generally looks great! Two small things:

> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 2382fbda4cbb..10f48103110a 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -170,13 +170,16 @@ LIBBPF_0.0.4 {
>  		btf_dump__dump_type;
>  		btf_dump__free;
>  		btf_dump__new;
> -		btf__parse_elf;
>  		bpf_object__load_xattr;
>  		bpf_program__attach_kprobe;
>  		bpf_program__attach_perf_event;
>  		bpf_program__attach_raw_tracepoint;
>  		bpf_program__attach_tracepoint;
>  		bpf_program__attach_uprobe;
> +		btf__parse_elf;
>  		libbpf_num_possible_cpus;
>  		libbpf_perf_event_disable_and_close;
> +		perf_buffer__free;
> +		perf_buffer__new;
> +		perf_buffer__poll;

We should prefix with libbpf_* given it's not strictly BPF-only and rather
helper function.

Also, we should convert bpftool (tools/bpf/bpftool/map_perf_ring.c) to make
use of these new helpers instead of open-coding there.

Thanks,
Daniel

^ permalink raw reply

* [PATCH net-next] r8169: improve handling VLAN tag
From: Heiner Kallweit @ 2019-06-27 21:06 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

The VLAN tag is stored in the descriptor in network byte order.
Using swab16 works on little endian host systems only. Better play safe
and use ntohs or htons respectively.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 48b8a90f7..b4df66bef 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1528,7 +1528,7 @@ static int rtl8169_set_features(struct net_device *dev,
 static inline u32 rtl8169_tx_vlan_tag(struct sk_buff *skb)
 {
 	return (skb_vlan_tag_present(skb)) ?
-		TxVlanTag | swab16(skb_vlan_tag_get(skb)) : 0x00;
+		TxVlanTag | htons(skb_vlan_tag_get(skb)) : 0x00;
 }
 
 static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
@@ -1536,7 +1536,8 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
 	u32 opts2 = le32_to_cpu(desc->opts2);
 
 	if (opts2 & RxVlanTag)
-		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), swab16(opts2 & 0xffff));
+		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+				       ntohs(opts2 & 0xffff));
 }
 
 static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next] r8169: consider that 32 Bit DMA is the default
From: Heiner Kallweit @ 2019-06-27 21:12 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

Documentation/DMA-API-HOWTO.txt states:
By default, the kernel assumes that your device can address 32-bits of
DMA addressing. For a 64-bit capable device, this needs to be increased,
and for a device with limitations, it needs to be decreased.

Therefore we don't need the 32 Bit DMA fallback configuration and can
remove it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index b4df66bef..1b5f125e1 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -6724,15 +6724,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->cp_cmd = RTL_R16(tp, CPlusCmd);
 
 	if (sizeof(dma_addr_t) > 4 && tp->mac_version >= RTL_GIGA_MAC_VER_18 &&
-	    !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
+	    !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)))
 		dev->features |= NETIF_F_HIGHDMA;
-	} else {
-		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
-		if (rc < 0) {
-			dev_err(&pdev->dev, "DMA configuration failed\n");
-			return rc;
-		}
-	}
 
 	rtl_init_rxcfg(tp);
 
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH v2 bpf-next 3/7] libbpf: add kprobe/uprobe attach API
From: Daniel Borkmann @ 2019-06-27 21:16 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Alexei Starovoitov, Stanislav Fomichev, bpf,
	Networking, Kernel Team
In-Reply-To: <CAEf4BzYy4Eorj0VxzArZg+V4muJCvDTX_VVfoouzZUcrBwTa1w@mail.gmail.com>

On 06/27/2019 12:15 AM, Andrii Nakryiko wrote:
> On Wed, Jun 26, 2019 at 7:25 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
[...]
>> What this boils down to is that this should get a proper abstraction, e.g. as
>> in struct libbpf_event which holds the event object. There should be helper
>> functions like libbpf_event_create_{kprobe,uprobe,tracepoint,raw_tracepoint} returning
>> such an struct libbpf_event object on success, and a single libbpf_event_destroy()
>> that does the event specific teardown. bpf_program__attach_event() can then take
>> care of only attaching the program to it. Having an object for this is also more
>> extensible than just a fd number. Nice thing is that this can also be completely
>> internal to libbpf.c as with struct bpf_program and other abstractions where we
>> don't expose the internals in the public header.
> 
> Yeah, I totally agree, I think this is a great idea! I don't
> particularly like "event" name, that seems very overloaded term. Do
> you mind if I call this "bpf_hook" instead of "libbpf_event"? I've
> always thought about these different points in the system to which one
> can attach BPF program as hooks exposed from kernel :)
> 
> Would it also make sense to do attaching to non-tracing hooks using
> the same mechanism (e.g., all the per-cgroup stuff, sysctl, etc)? Not
> sure how people do that today, will check to see how it's done, but I
> think nothing should conceptually prevent doing that using the same
> abstract bpf_hook way, right?

I think if we abstract it this way, then absolutely. If I grok the naming conventions
from the README right, then this would be under 'bpf_hook__' prefix. :)

Thanks,
Daniel

^ permalink raw reply

* [PATCH net-next] r8169: remove not needed call to dma_sync_single_for_device
From: Heiner Kallweit @ 2019-06-27 21:19 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

DMA_API_HOWTO.txt includes an example explaining when
dma_sync_single_for_device() is not needed, and that example matches
our use case. The buffer isn't changed by the CPU and direction is
DMA_FROM_DEVICE, so we can remove the call to
dma_sync_single_for_device().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 1b5f125e1..a73f25321 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5785,7 +5785,6 @@ static struct sk_buff *rtl8169_try_rx_copy(void *data,
 	skb = napi_alloc_skb(&tp->napi, pkt_size);
 	if (skb)
 		skb_copy_to_linear_data(skb, data, pkt_size);
-	dma_sync_single_for_device(d, addr, pkt_size, DMA_FROM_DEVICE);
 
 	return skb;
 }
-- 
2.22.0



^ permalink raw reply related

* Re: [PATCH 00/11] XDP unaligned chunk placement support
From: Jakub Kicinski @ 2019-06-27 21:25 UTC (permalink / raw)
  To: Laatz, Kevin
  Cc: Jonathan Lemon, netdev, ast, daniel, bjorn.topel, magnus.karlsson,
	bpf, intel-wired-lan, bruce.richardson, ciara.loftus
In-Reply-To: <ef7e9469-e7be-647b-8bb1-da29bc01fa2e@intel.com>

On Thu, 27 Jun 2019 12:14:50 +0100, Laatz, Kevin wrote:
> On the application side (xdpsock), we don't have to worry about the user 
> defined headroom, since it is 0, so we only need to account for the 
> XDP_PACKET_HEADROOM when computing the original address (in the default 
> scenario).

That assumes specific layout for the data inside the buffer.  Some NICs
will prepend information like timestamp to the packet, meaning the
packet would start at offset XDP_PACKET_HEADROOM + metadata len..

I think that's very limiting.  What is the challenge in providing
aligned addresses, exactly?

^ permalink raw reply

* [Patch net] netrom: fix a memory leak in nr_rx_frame()
From: Cong Wang @ 2019-06-27 21:30 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, syzbot+d6636a36d3c34bd88938

When the skb is associated with a new sock, just assigning
it to skb->sk is not sufficient, we have to set its destructor
to free the sock properly too.

Reported-by: syzbot+d6636a36d3c34bd88938@syzkaller.appspotmail.com
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/netrom/af_netrom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 86b87925ef34..96740d389377 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -869,7 +869,7 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
 	unsigned short frametype, flags, window, timeout;
 	int ret;
 
-	skb->sk = NULL;		/* Initially we don't know who it's for */
+	skb_orphan(skb);
 
 	/*
 	 *	skb->data points to the netrom frame start
@@ -968,6 +968,7 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
 	window = skb->data[20];
 
 	skb->sk             = make;
+	skb->destructor     = sock_efree;
 	make->sk_state	    = TCP_ESTABLISHED;
 
 	/* Fill in his circuit details */
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v2 bpf-next 3/7] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-06-27 21:42 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, Alexei Starovoitov, Stanislav Fomichev, bpf,
	Networking, Kernel Team
In-Reply-To: <44d3b02d-b0fb-b0cb-a0d3-e7dd4bde0b92@iogearbox.net>

On Thu, Jun 27, 2019 at 2:16 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 06/27/2019 12:15 AM, Andrii Nakryiko wrote:
> > On Wed, Jun 26, 2019 at 7:25 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> [...]
> >> What this boils down to is that this should get a proper abstraction, e.g. as
> >> in struct libbpf_event which holds the event object. There should be helper
> >> functions like libbpf_event_create_{kprobe,uprobe,tracepoint,raw_tracepoint} returning
> >> such an struct libbpf_event object on success, and a single libbpf_event_destroy()
> >> that does the event specific teardown. bpf_program__attach_event() can then take
> >> care of only attaching the program to it. Having an object for this is also more
> >> extensible than just a fd number. Nice thing is that this can also be completely
> >> internal to libbpf.c as with struct bpf_program and other abstractions where we
> >> don't expose the internals in the public header.
> >
> > Yeah, I totally agree, I think this is a great idea! I don't
> > particularly like "event" name, that seems very overloaded term. Do
> > you mind if I call this "bpf_hook" instead of "libbpf_event"? I've
> > always thought about these different points in the system to which one
> > can attach BPF program as hooks exposed from kernel :)
> >
> > Would it also make sense to do attaching to non-tracing hooks using
> > the same mechanism (e.g., all the per-cgroup stuff, sysctl, etc)? Not
> > sure how people do that today, will check to see how it's done, but I
> > think nothing should conceptually prevent doing that using the same
> > abstract bpf_hook way, right?
>
> I think if we abstract it this way, then absolutely. If I grok the naming conventions
> from the README right, then this would be under 'bpf_hook__' prefix. :)

Yeah, so this is what I had, API-wise:

struct bpf_hook;

LIBBPF_API struct bpf_hook *bpf_hook__new_perf_event(int pfd);
LIBBPF_API struct bpf_hook *bpf_hook__new_kprobe(bool retprobe,
                                                 const char *func_name);
LIBBPF_API struct bpf_hook *bpf_hook__new_uprobe(bool retprobe, pid_t pid,
                                                 const char *binary_path,
                                                 size_t func_offset);
LIBBPF_API struct bpf_hook *bpf_hook__new_tracepoint(const char *tp_category,
                                                     const char *tp_name);
LIBBPF_API struct bpf_hook *bpf_hook__new_raw_tracepoint(const char *tp_name);

LIBBPF_API int bpf_hook__attach_program(struct bpf_hook *hook,
                                        struct bpf_program *prog);
LIBBPF_API int bpf_hook__free(struct bpf_hook *hook);


You'd use bpf_hook_new_xxx to create struct bpf_hook, which then get's
attached using generic bpf_hook__attach_program and detached/freed
with generic bpf_hook__free.

But once I converted selftests, I realized that this generic
bpf_hook__attach_program is kind of unnecessary and is just a
boiler-plate extra function that everyone has to call. So now I'm
leaning towards a hybrid approach:

- bpf_program__attach_xxx will create some specific struct bpf_hook
*and* attach bpf_program to it;
- bpf_hook__free(struct bpf_hook *) would still be used to detach/free
resources, abstracting away specifics of detaching.

There are few benefits to this, I think:
1. Less error checking and clean up from caller: attach either
succeeds (and you'll have to eventually do bpf_hook__free) or not, and
then nothing needs to be cleaned up. With separate create/attach, if
create succeeds, but attach fails, you'll have to do extra
bpf_hook__free call. This bundling of create/attach does prevent
theoretical use case of having hook creation in one place and then
pass this generically into another place for attachment, but it seems
like a bit far-fetched corner use case, which can be implemented at
application level, if necessary.
2. bpf_program__attach has more context for helpful log messages, if
something goes wrong. E.g.,
bpf_program__attach_tracepoint(tp_category, tp_name), once it created
perf event FD for tracepoint, can discard tp_category and tp_name and
use only FD for attachment. But if attachment fails, we don't really
know which tracepoint failed to attach. To facilitate that, you'd need
to allocate/copy tp_category/tp_name (just in case for logging), which
is PITA. With bundled attach, you can log nice error with context
right there with no overhead.

This still allows to do cgroup/flow/sysctl/etc attachment in similar
uniform way (but that's for another set of patches).

Also, I renamed bpf_hook to bpf_link as it seems to convey connection
between connection point (hook) and bpf_program better. Alternative
might be bpf_assoc, I'll mention that as well in cover letter.

Anyways, I think it's better usability without losing anything
(realistically) in terms of flexibility for users. I'll post v2 later
today.


>
> Thanks,
> Daniel

^ permalink raw reply

* Re: [PATCH v2 bpf-next 1/3] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-06-27 21:45 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Networking, Kernel Team
In-Reply-To: <7de14b2b-a663-eed9-8f70-fb6bd5ea84d8@iogearbox.net>

On Thu, Jun 27, 2019 at 2:04 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 06/26/2019 08:12 AM, Andrii Nakryiko wrote:
> > BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
> > to user space for additional processing. libbpf already has very low-level API
> > to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
> > use and requires a lot of code to set everything up. This patch adds
> > perf_buffer abstraction on top of it, abstracting setting up and polling
> > per-CPU logic into simple and convenient API, similar to what BCC provides.
> >
> > perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
> > map entries. It accepts two user-provided callbacks: one for handling raw
> > samples and one for get notifications of lost samples due to buffer overflow.
> >
> > perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
> > utilizing epoll instance.
> >
> > perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
> >
> > All APIs are not thread-safe. User should ensure proper locking/coordination if
> > used in multi-threaded set up.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Aside from current feedback, this series generally looks great! Two small things:
>
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 2382fbda4cbb..10f48103110a 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -170,13 +170,16 @@ LIBBPF_0.0.4 {
> >               btf_dump__dump_type;
> >               btf_dump__free;
> >               btf_dump__new;
> > -             btf__parse_elf;
> >               bpf_object__load_xattr;
> >               bpf_program__attach_kprobe;
> >               bpf_program__attach_perf_event;
> >               bpf_program__attach_raw_tracepoint;
> >               bpf_program__attach_tracepoint;
> >               bpf_program__attach_uprobe;
> > +             btf__parse_elf;
> >               libbpf_num_possible_cpus;
> >               libbpf_perf_event_disable_and_close;
> > +             perf_buffer__free;
> > +             perf_buffer__new;
> > +             perf_buffer__poll;
>
> We should prefix with libbpf_* given it's not strictly BPF-only and rather
> helper function.

Well, perf_buffer is an object similar to `struct btf`, `struct
bpf_program`, etc. So it seems appropriate to follow this
"<object>__<method>" convention. Also, `struct libbpf_perf_buffer` and
`libbpf_perf_buffer__new` looks verbose and pretty ugly, IMO.

>
> Also, we should convert bpftool (tools/bpf/bpftool/map_perf_ring.c) to make
> use of these new helpers instead of open-coding there.

Yep, absolutely, will do that as well, thanks for pointing me there!

>
> Thanks,
> Daniel

^ permalink raw reply

* [PATCH v2 net-next 0/3] Better PHYLINK compliance for SJA1105 DSA
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
  To: linux, f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean

After discussing with Russell King, it appears this driver is making a
few confusions and not performing some checks for consistent operation.

Changes in v2:
- Removed redundant print in the phylink_validate callback (in 2/3).

Vladimir Oltean (3):
  net: dsa: sja1105: Don't check state->link in phylink_mac_config
  net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK
    reports
  net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK

 drivers/net/dsa/sja1105/sja1105_main.c | 56 +++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 2 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v2 net-next 1/3] net: dsa: sja1105: Don't check state->link in phylink_mac_config
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
  To: linux, f.fainelli, vivien.didelot, andrew, davem
  Cc: netdev, Vladimir Oltean, Russell King
In-Reply-To: <20190627214637.22366-1-olteanv@gmail.com>

It has been pointed out that PHYLINK can call mac_config only to update
the phy_interface_type and without knowing what the AN results are.

Experimentally, when this was observed to happen, state->link was also
unset, and therefore was used as a proxy to ignore this call. However it
is also suggested that state->link is undefined for this callback and
should not be relied upon.

So let the previously-dead codepath for SPEED_UNKNOWN be called, and
update the comment to make sure the MAC's behavior is sane.

Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index caebf76eaa3e..da1736093b06 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -715,7 +715,13 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
 
 	switch (speed_mbps) {
 	case SPEED_UNKNOWN:
-		/* No speed update requested */
+		/* PHYLINK called sja1105_mac_config() to inform us about
+		 * the state->interface, but AN has not completed and the
+		 * speed is not yet valid. UM10944.pdf says that setting
+		 * SJA1105_SPEED_AUTO at runtime disables the port, so that is
+		 * ok for power consumption in case AN will never complete -
+		 * otherwise PHYLINK should come back with a new update.
+		 */
 		speed = SJA1105_SPEED_AUTO;
 		break;
 	case SPEED_10:
@@ -766,9 +772,6 @@ static void sja1105_mac_config(struct dsa_switch *ds, int port,
 {
 	struct sja1105_private *priv = ds->priv;
 
-	if (!state->link)
-		return;
-
 	sja1105_adjust_port_config(priv, port, state->speed);
 }
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 net-next 2/3] net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK reports
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
  To: linux, f.fainelli, vivien.didelot, andrew, davem
  Cc: netdev, Vladimir Oltean, Russell King
In-Reply-To: <20190627214637.22366-1-olteanv@gmail.com>

PHYLINK being designed with PHYs in mind that can change MII protocol,
for correct operation it is necessary to ensure that the PHY interface
mode stays the same (otherwise clear the supported bit mask, as
required).

Because this is just a hypothetical situation for now, we don't bother
to check whether we could actually support the new PHY interface mode.
Actually we could modify the xMII table, reset the switch and send an
updated static configuration, but adding that would just be dead code.

Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 44 ++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index da1736093b06..b366b8e100f8 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -766,12 +766,46 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
 	return sja1105_clocking_setup_port(priv, port);
 }
 
+/* The SJA1105 MAC programming model is through the static config (the xMII
+ * Mode table cannot be dynamically reconfigured), and we have to program
+ * that early (earlier than PHYLINK calls us, anyway).
+ * So just error out in case the connected PHY attempts to change the initial
+ * system interface MII protocol from what is defined in the DT, at least for
+ * now.
+ */
+static bool sja1105_phy_mode_mismatch(struct sja1105_private *priv, int port,
+				      phy_interface_t interface)
+{
+	struct sja1105_xmii_params_entry *mii;
+	sja1105_phy_interface_t phy_mode;
+
+	mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
+	phy_mode = mii->xmii_mode[port];
+
+	switch (interface) {
+	case PHY_INTERFACE_MODE_MII:
+		return (phy_mode != XMII_MODE_MII);
+	case PHY_INTERFACE_MODE_RMII:
+		return (phy_mode != XMII_MODE_RMII);
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		return (phy_mode != XMII_MODE_RGMII);
+	default:
+		return true;
+	}
+}
+
 static void sja1105_mac_config(struct dsa_switch *ds, int port,
 			       unsigned int link_an_mode,
 			       const struct phylink_link_state *state)
 {
 	struct sja1105_private *priv = ds->priv;
 
+	if (sja1105_phy_mode_mismatch(priv, port, state->interface))
+		return;
+
 	sja1105_adjust_port_config(priv, port, state->speed);
 }
 
@@ -804,6 +838,16 @@ static void sja1105_phylink_validate(struct dsa_switch *ds, int port,
 
 	mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
 
+	/* include/linux/phylink.h says:
+	 *     When @state->interface is %PHY_INTERFACE_MODE_NA, phylink
+	 *     expects the MAC driver to return all supported link modes.
+	 */
+	if (state->interface != PHY_INTERFACE_MODE_NA &&
+	    sja1105_phy_mode_mismatch(priv, port, state->interface)) {
+		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+		return;
+	}
+
 	/* The MAC does not support pause frames, and also doesn't
 	 * support half-duplex traffic modes.
 	 */
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 net-next 3/3] net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
  To: linux, f.fainelli, vivien.didelot, andrew, davem
  Cc: netdev, Vladimir Oltean, Russell King
In-Reply-To: <20190627214637.22366-1-olteanv@gmail.com>

We need a better way to signal this, perhaps in phylink_validate, but
for now just print this error message as guidance for other people
looking at this driver's code while trying to rework PHYLINK.

Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index b366b8e100f8..32bf3a7cc3b6 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -806,6 +806,11 @@ static void sja1105_mac_config(struct dsa_switch *ds, int port,
 	if (sja1105_phy_mode_mismatch(priv, port, state->interface))
 		return;
 
+	if (link_an_mode == MLO_AN_INBAND) {
+		dev_err(ds->dev, "In-band AN not supported!\n");
+		return;
+	}
+
 	sja1105_adjust_port_config(priv, port, state->speed);
 }
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Daniel Borkmann @ 2019-06-27 21:55 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, netdev
  Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
	Jonathan Lemon
In-Reply-To: <156125626136.5209.14349225282974871197.stgit@alrua-x1>

On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
> 
> The bpf_redirect_map() helper used by XDP programs doesn't return any
> indication of whether it can successfully redirect to the map index it was
> given. Instead, BPF programs have to track this themselves, leading to
> programs using duplicate maps to track which entries are populated in the
> devmap.
> 
> This patch fixes this by moving the map lookup into the bpf_redirect_map()
> helper, which makes it possible to return failure to the eBPF program. The
> lower bits of the flags argument is used as the return code, which means
> that existing users who pass a '0' flag argument will get XDP_ABORTED.
> 
> With this, a BPF program can check the return code from the helper call and
> react by, for instance, substituting a different redirect. This works for
> any type of map used for redirect.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

Overall series looks good to me. Just very small things inline here & in the
other two patches:

[...]
> @@ -3750,9 +3742,16 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
>  {
>  	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
>  
> -	if (unlikely(flags))
> +	/* Lower bits of the flags are used as return code on lookup failure */
> +	if (unlikely(flags > XDP_TX))
>  		return XDP_ABORTED;
>  
> +	ri->item = __xdp_map_lookup_elem(map, ifindex);
> +	if (unlikely(!ri->item)) {
> +		WRITE_ONCE(ri->map, NULL);

This WRITE_ONCE() is not needed. We never set it before at this point.

> +		return flags;
> +	}
> +
>  	ri->ifindex = ifindex;
>  	ri->flags = flags;
>  	WRITE_ONCE(ri->map, map);
> 


^ permalink raw reply

* [PATCH 0/5] net: dsa: microchip: Further regmap cleanups
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
  To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
	Woojung Huh

This patchset cleans up KSZ9477 switch driver by replacing various
ad-hoc polling implementations and register RMW with regmap functions.

Each polling function is replaced separately to make it easier to review
and possibly bisect, but maybe the patches can be squashed.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>

Marek Vasut (5):
  net: dsa: microchip: Replace ad-hoc polling with regmap
  net: dsa: microchip: Replace ksz9477_wait_vlan_ctrl_ready polling with
    regmap
  net: dsa: microchip: Replace ksz9477_wait_alu_ready polling with
    regmap
  net: dsa: microchip: Replace ksz9477_wait_alu_sta_ready polling with
    regmap
  net: dsa: microchip: Replace bit RMW with regmap

 drivers/net/dsa/microchip/ksz9477.c    | 128 +++++++++----------------
 drivers/net/dsa/microchip/ksz_common.h |  14 ---
 2 files changed, 47 insertions(+), 95 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH 1/5] net: dsa: microchip: Replace ad-hoc polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
  To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
	Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>

Regmap provides polling function to poll for bits in a register,
use in instead of reimplementing it.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c    | 14 +++++---------
 drivers/net/dsa/microchip/ksz_common.h | 14 --------------
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 8f13dcc05a10..ece25f38e02a 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -263,12 +263,8 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
 static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
 			      u64 *cnt)
 {
-	struct ksz_poll_ctx ctx = {
-		.dev = dev,
-		.port = port,
-		.offset = REG_PORT_MIB_CTRL_STAT__4,
-	};
 	struct ksz_port *p = &dev->ports[port];
+	unsigned int val;
 	u32 data;
 	int ret;
 
@@ -278,11 +274,11 @@ static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
 	data |= (addr << MIB_COUNTER_INDEX_S);
 	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
 
-	ret = readx_poll_timeout(ksz_pread32_poll, &ctx, data,
-				 !(data & MIB_COUNTER_READ), 10, 1000);
-
+	ret = regmap_read_poll_timeout(dev->regmap[2],
+			PORT_CTRL_ADDR(port, REG_PORT_MIB_CTRL_STAT__4),
+			val, !(val & MIB_COUNTER_READ), 10, 1000);
 	/* failed to read MIB. get out of loop */
-	if (ret < 0) {
+	if (ret) {
 		dev_dbg(dev->dev, "Failed to get MIB\n");
 		return;
 	}
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 745318424f71..ee7096d8af07 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -119,20 +119,6 @@ static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
 	ksz_write32(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
-struct ksz_poll_ctx {
-	struct ksz_device *dev;
-	int port;
-	int offset;
-};
-
-static inline u32 ksz_pread32_poll(struct ksz_poll_ctx *ctx)
-{
-	u32 data;
-
-	ksz_pread32(ctx->dev, ctx->port, ctx->offset, &data);
-	return data;
-}
-
 /* Regmap tables generation */
 #define KSZ_SPI_OP_RD		3
 #define KSZ_SPI_OP_WR		2
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/5] net: dsa: microchip: Replace ksz9477_wait_vlan_ctrl_ready polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
  To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
	Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>

Regmap provides polling function to poll for bits in a register. This
function is another reimplementation of polling for bit being clear in
a register. Replace this with regmap polling function. Moreover, inline
the function parameters, as the function is never called with any other
parameter values than this one.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index ece25f38e02a..0aab00bf23b9 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -89,22 +89,12 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset,
 			   bits, set ? bits : 0);
 }
 
-static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton,
-					int timeout)
+static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev)
 {
-	u8 data;
-
-	do {
-		ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
+	unsigned int val;
 
-	return 0;
+	return regmap_read_poll_timeout(dev->regmap[0], REG_SW_VLAN_CTRL,
+					val, !(val & VLAN_START), 10, 1000);
 }
 
 static int ksz9477_get_vlan_table(struct ksz_device *dev, u16 vid,
@@ -118,8 +108,8 @@ static int ksz9477_get_vlan_table(struct ksz_device *dev, u16 vid,
 	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
 
 	/* wait to be cleared */
-	ret = ksz9477_wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
-	if (ret < 0) {
+	ret = ksz9477_wait_vlan_ctrl_ready(dev);
+	if (ret) {
 		dev_dbg(dev->dev, "Failed to read vlan table\n");
 		goto exit;
 	}
@@ -151,8 +141,8 @@ static int ksz9477_set_vlan_table(struct ksz_device *dev, u16 vid,
 	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
 
 	/* wait to be cleared */
-	ret = ksz9477_wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
-	if (ret < 0) {
+	ret = ksz9477_wait_vlan_ctrl_ready(dev);
+	if (ret) {
 		dev_dbg(dev->dev, "Failed to write vlan table\n");
 		goto exit;
 	}
-- 
2.20.1


^ permalink raw reply related

* [PATCH 4/5] net: dsa: microchip: Replace ksz9477_wait_alu_sta_ready polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
  To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
	Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>

Regmap provides polling function to poll for bits in a register. This
function is another reimplementation of polling for bit being clear in
a register. Replace this with regmap polling function. Moreover, inline
the function parameters, as the function is never called with any other
parameter values than this one.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 32 +++++++++++------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index e5b2f3e45db6..bfc44799854c 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -184,22 +184,14 @@ static int ksz9477_wait_alu_ready(struct ksz_device *dev)
 					val, !(val & ALU_START), 10, 1000);
 }
 
-static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev, u32 waiton,
-				      int timeout)
+static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev)
 {
-	u32 data;
-
-	do {
-		ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
+	unsigned int val;
 
-	return 0;
+	return regmap_read_poll_timeout(dev->regmap[2],
+					REG_SW_ALU_STAT_CTRL__4,
+					val, !(val & ALU_STAT_START),
+					10, 1000);
 }
 
 static int ksz9477_reset_switch(struct ksz_device *dev)
@@ -821,7 +813,7 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 		/* wait to be finished */
-		if (ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0) {
+		if (ksz9477_wait_alu_sta_ready(dev)) {
 			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 			goto exit;
 		}
@@ -862,7 +854,7 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 	/* wait to be finished */
-	if (ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0)
+	if (ksz9477_wait_alu_sta_ready(dev))
 		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 
 exit:
@@ -892,8 +884,8 @@ static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
 		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 		/* wait to be finished */
-		ret = ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
-		if (ret < 0) {
+		ret = ksz9477_wait_alu_sta_ready(dev);
+		if (ret) {
 			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 			goto exit;
 		}
@@ -934,8 +926,8 @@ static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
-	if (ret < 0)
+	ret = ksz9477_wait_alu_sta_ready(dev);
+	if (ret)
 		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 
 exit:
-- 
2.20.1


^ permalink raw reply related

* [PATCH 5/5] net: dsa: microchip: Replace bit RMW with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
  To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
	Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>

Regmap provides read-modify-write function to update bitfields in
registers. Replace ad-hoc read-modify-write with regmap_update_bits()
where applicable.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index bfc44799854c..a8c97f7a79b7 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -197,16 +197,14 @@ static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev)
 static int ksz9477_reset_switch(struct ksz_device *dev)
 {
 	u8 data8;
-	u16 data16;
 	u32 data32;
 
 	/* reset switch */
 	ksz_cfg(dev, REG_SW_OPERATION, SW_RESET, true);
 
 	/* turn off SPI DO Edge select */
-	ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
-	data8 &= ~SPI_AUTO_EDGE_DETECTION;
-	ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
+	regmap_update_bits(dev->regmap[0], REG_SW_GLOBAL_SERIAL_CTRL_0,
+			   SPI_AUTO_EDGE_DETECTION, 0);
 
 	/* default configuration */
 	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
@@ -220,10 +218,10 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
 	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
 
 	/* set broadcast storm protection 10% rate */
-	ksz_read16(dev, REG_SW_MAC_CTRL_2, &data16);
-	data16 &= ~BROADCAST_STORM_RATE;
-	data16 |= (BROADCAST_STORM_VALUE * BROADCAST_STORM_PROT_RATE) / 100;
-	ksz_write16(dev, REG_SW_MAC_CTRL_2, data16);
+	regmap_update_bits(dev->regmap[1], REG_SW_MAC_CTRL_2,
+			   BROADCAST_STORM_RATE,
+			   (BROADCAST_STORM_VALUE *
+			   BROADCAST_STORM_PROT_RATE) / 100);
 
 	if (dev->synclko_125)
 		ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1,
@@ -485,10 +483,10 @@ static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
 {
 	u8 data;
 
-	ksz_read8(dev, REG_SW_LUE_CTRL_2, &data);
-	data &= ~(SW_FLUSH_OPTION_M << SW_FLUSH_OPTION_S);
-	data |= (SW_FLUSH_OPTION_DYN_MAC << SW_FLUSH_OPTION_S);
-	ksz_write8(dev, REG_SW_LUE_CTRL_2, data);
+	regmap_update_bits(dev->regmap[0], REG_SW_LUE_CTRL_2,
+			   SW_FLUSH_OPTION_M << SW_FLUSH_OPTION_S,
+			   SW_FLUSH_OPTION_DYN_MAC << SW_FLUSH_OPTION_S);
+
 	if (port < dev->mib_port_cnt) {
 		/* flush individual port */
 		ksz_pread8(dev, port, P_STP_CTRL, &data);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 3/5] net: dsa: microchip: Replace ksz9477_wait_alu_ready polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
  To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
	Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>

Regmap provides polling function to poll for bits in a register. This
function is another reimplementation of polling for bit being clear in
a register. Replace this with regmap polling function. Moreover, inline
the function parameters, as the function is never called with any other
parameter values than this one.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 34 ++++++++++-------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 0aab00bf23b9..e5b2f3e45db6 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -176,22 +176,12 @@ static void ksz9477_write_table(struct ksz_device *dev, u32 *table)
 	ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]);
 }
 
-static int ksz9477_wait_alu_ready(struct ksz_device *dev, u32 waiton,
-				  int timeout)
+static int ksz9477_wait_alu_ready(struct ksz_device *dev)
 {
-	u32 data;
-
-	do {
-		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
+	unsigned int val;
 
-	return 0;
+	return regmap_read_poll_timeout(dev->regmap[2], REG_SW_ALU_CTRL__4,
+					val, !(val & ALU_START), 10, 1000);
 }
 
 static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev, u32 waiton,
@@ -633,8 +623,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0) {
+	ret = ksz9477_wait_alu_ready(dev);
+	if (ret) {
 		dev_dbg(dev->dev, "Failed to read ALU\n");
 		goto exit;
 	}
@@ -657,8 +647,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0)
+	ret = ksz9477_wait_alu_ready(dev);
+	if (ret)
 		dev_dbg(dev->dev, "Failed to write ALU\n");
 
 exit:
@@ -690,8 +680,8 @@ static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0) {
+	ret = ksz9477_wait_alu_ready(dev);
+	if (ret) {
 		dev_dbg(dev->dev, "Failed to read ALU\n");
 		goto exit;
 	}
@@ -724,8 +714,8 @@ static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0)
+	ret = ksz9477_wait_alu_ready(dev);
+	if (ret)
 		dev_dbg(dev->dev, "Failed to write ALU\n");
 
 exit:
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Ivan Khoronzhuk @ 2019-06-27 22:02 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: davem, grygorii.strashko, saeedm, leon, ast, linux-kernel,
	linux-omap, ilias.apalodimas, netdev, daniel, jakub.kicinski,
	john.fastabend
In-Reply-To: <20190627214317.237e5926@carbon>

Hi Jesper, thanks you remember about it.

>
>I don't think that "create" and "free" routines paring looks "more
>correct" together.
>
>Maybe we can scale back your solution(?), via creating a page_pool_get()
>and page_pool_put() API that can be used by your driver, to keep the
>page_pool object after a xdp_rxq_info_unreg() call.  Then you can use
>it for two xdp_rxq_info structs, and call page_pool_put() after you
>have unregistered both.
>
>The API would basically be:
>
>diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>index b366f59885c1..691ddacfb5a6 100644
>--- a/net/core/page_pool.c
>+++ b/net/core/page_pool.c
>@@ -357,6 +357,10 @@ static void __warn_in_flight(struct page_pool *pool)
> void __page_pool_free(struct page_pool *pool)
> {
>        WARN(pool->alloc.count, "API usage violation");
>+
>+       if (atomic_read(&pool->user_cnt) != 0)
>+               return;
>+
>        WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>
>        /* Can happen due to forced shutdown */
>@@ -372,6 +376,19 @@ void __page_pool_free(struct page_pool *pool)
> }
> EXPORT_SYMBOL(__page_pool_free);
>
>+void page_pool_put(struct page_pool *pool)
>+{
>+       if (!atomic_dec_and_test(&pool->user_cnt))
>+               __page_pool_free(pool);
>+}
>+EXPORT_SYMBOL(page_pool_put);
>+
>+void page_pool_get(struct page_pool *pool)
>+{
>+       atomic_inc(&pool->user_cnt);
>+}
>+EXPORT_SYMBOL(page_pool_get);
>+

I have another solution that doesn't touch page pool and adds modifications
to xdp allocator. As for me it looks better and work wider, I don't need to
think about this in the driver also.

It's supposed allocator works as before, no any changes to mlx5 and
page_pool API and its usage and seems like fits your requirements.
It still supposes that allocator runs under same napi softirq but allows
to reuse allocator.

I have not verified yet, but looks like:

diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
index 6a8cba6ea79a..995b21da2f27 100644
--- a/include/net/xdp_priv.h
+++ b/include/net/xdp_priv.h
@@ -18,6 +18,7 @@ struct xdp_mem_allocator {
 	struct rcu_head rcu;
 	struct delayed_work defer_wq;
 	unsigned long defer_warn;
+	unsigned long refcnt;
 };
 
 #endif /* __LINUX_NET_XDP_PRIV_H__ */
diff --git a/net/core/xdp.c b/net/core/xdp.c
index f98ab6b98674..6239483e3793 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -98,6 +98,12 @@ static bool __mem_id_disconnect(int id, bool force)
 		WARN(1, "Request remove non-existing id(%d), driver bug?", id);
 		return true;
 	}
+
+	if (--xa->refcnt) {
+		mutex_unlock(&mem_id_lock);
+		return true;
+	}
+
 	xa->disconnect_cnt++;
 
 	/* Detects in-flight packet-pages for page_pool */
@@ -312,6 +318,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
 	return true;
 }
 
+static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
+{
+	struct xdp_mem_allocator *xae, *xa == NULL;
+	struct rhashtable_iter iter;
+
+	mutex_lock(&mem_id_lock);
+	rhashtable_walk_enter(mem_id_ht, &iter);
+	do {
+		rhashtable_walk_start(&iter);
+
+		while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
+			if (xae->allocator == allocator) {
+				xae->refcnt++;
+				xa = xae;
+				break;
+			}
+		}
+
+		rhashtable_walk_stop(&iter);
+
+	} while (xae == ERR_PTR(-EAGAIN));
+	rhashtable_walk_exit(&iter);
+	mutex_unlock(&mem_id_lock);
+
+	return xa;
+}
+
 int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
 			       enum xdp_mem_type type, void *allocator)
 {
@@ -347,6 +380,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
 		}
 	}
 
+	if (xdp_allocator_get(allocator))
+		return 0;
+
 	xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
 	if (!xdp_alloc)
 		return -ENOMEM;
@@ -360,6 +396,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
 	xdp_rxq->mem.id = id;
 	xdp_alloc->mem  = xdp_rxq->mem;
 	xdp_alloc->allocator = allocator;
+	xdp_alloc->refcnt = 1;
 
 	/* Insert allocator into ID lookup table */
 	ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);


-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply related

* Re: WARNING in mark_lock
From: syzbot @ 2019-06-27 22:03 UTC (permalink / raw)
  To: ast, bpf, daniel, dvyukov, ebiggers, john.fastabend, linux-kernel,
	netdev, peterz, syzkaller-bugs, tglx
In-Reply-To: <0000000000005aedf1058c1bf7e8@google.com>

syzbot has bisected this bug to:

commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date:   Sat Jun 30 13:17:47 2018 +0000

     bpf: sockhash fix omitted bucket lock in sock_close

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1436e7e9a00000
start commit:   dc636f5d Add linux-next specific files for 20190620
git tree:       linux-next
final crash:    https://syzkaller.appspot.com/x/report.txt?x=1636e7e9a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1236e7e9a00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=99c104b0092a557b
dashboard link: https://syzkaller.appspot.com/bug?extid=a861f52659ae2596492b
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=110b24f6a00000

Reported-by: syzbot+a861f52659ae2596492b@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox