Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 0/4] test_rhashtable: add test case for rhl table
From: David Miller @ 2017-09-19 21:03 UTC (permalink / raw)
  To: fw; +Cc: netdev
In-Reply-To: <20170918210711.10202-1-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Mon, 18 Sep 2017 23:07:07 +0200

> Add a test case for the rhlist interface.
> 
> While at it, cleanup current rhashtable test a bit and add a check
> for max_size support.

The kbuild test robot complained about a very large on-stack allocation
added by these changes, please address that.

Thanks.

^ permalink raw reply

* Re: [PATCH] tcp: avoid bogus warning in tcp_clean_rtx_queue
From: David Miller @ 2017-09-19 21:02 UTC (permalink / raw)
  To: arnd
  Cc: kuznet, yoshfuji, edumazet, ncardwell, ycheng, soheil, fw, netdev,
	linux-kernel
In-Reply-To: <20170918204855.170920-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 18 Sep 2017 22:48:47 +0200

> gcc-4.9 warns that it cannot trace the state of the 'last_ackt'
> variable since the change to the TCP timestamping code, when
> CONFIG_PROFILE_ANNOTATED_BRANCHES is set:
> 
> net/ipv4/tcp_input.c: In function 'tcp_clean_rtx_queue':
> include/net/tcp.h:757:23: error: 'last_ackt' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> Other gcc versions, both older and newer do now show this
> warning. Removing the 'likely' annotation makes it go away,
> and has no effect on the object code without
> CONFIG_PROFILE_ANNOTATED_BRANCHES, as tested with gcc-4.9
> and gcc-7.1.1, so this seems to be a safe workaround.
> 
> Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

This reaches the limits at which I am willing to work around compiler
stuff.

What cpu did you test the object code generation upon and does that
cpu have branch prediction hints in the target you are building for?

^ permalink raw reply

* Re: cross namespace interface notification for tun devices
From: Jason A. Donenfeld @ 2017-09-19 21:02 UTC (permalink / raw)
  To: Cong Wang; +Cc: Netdev, Mathias
In-Reply-To: <CAM_iQpVvDNRbtj_47_3XXa_ezyrT=a_VzDtK5SCR9a4_2Gqw9w@mail.gmail.com>

On Tue, Sep 19, 2017 at 10:40 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> By "notification" I assume you mean netlink notification.

Yes, netlink notification.

> The question is why does the process in A still care about
> the device sitting in B?
>
> Also, the process should be able to receive a last notification
> on IFF_UP|IFF_RUNNING before device is finally moved to B.
> After this point, it should not have any relation to netns A
> any more, like the device were completely gone.

That's very clearly not the case with a tun device. Tun devices work
by letting a userspace process control the inputs (ndo_start_xmit) and
outputs (netif_rx) of the actual network device. This controlling
userspace process needs to know when its own interface that it
controls goes up and down. In the kernel, we can do this by just
checking dev->flags&IFF_UP, and receive notifications on ndo_open and
ndo_stop. In userspace, the controlling process looses the ability to
receive notifications like ndo_open/ndo_stop when the interface is
moved to a new namespace. After the interface is moved to a namespace,
the process will still control inputs and ouputs (ndo_start_xmit and
netif_rx), but it will no longer receive netlink notifications for the
equivalent of ndo_open and ndo_stop. This is problematic.

^ permalink raw reply

* Re: [PATCH net-next 0/3] Implement delete for BPF LPM trie
From: David Miller @ 2017-09-19 20:55 UTC (permalink / raw)
  To: kraigatgoog; +Cc: daniel, ast, daniel, netdev
In-Reply-To: <20170918193057.37644-1-kraigatgoog@gmail.com>

From: Craig Gallek <kraigatgoog@gmail.com>
Date: Mon, 18 Sep 2017 15:30:54 -0400

> This was previously left as a TODO.  Add the implementation and
> extend the test to cover it.

Series applied, thanks.

^ permalink raw reply

* Re: [RFC PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
From: Doug Anderson @ 2017-09-19 20:53 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Guenter Roeck, Grant Grundler, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, netdev
In-Reply-To: <1505853476.15836.9.camel@suse.com>

Hi,

On Tue, Sep 19, 2017 at 1:37 PM, Oliver Neukum <oneukum@suse.com> wrote:
> Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson:
>> In general when you've got a flag communicating that "something needs
>> to be done" you want to clear that flag _before_ doing the task.  If
>> you clear the flag _after_ doing the task you end up with the risk
>> that this will happen:
>>
>> 1. Requester sets flag saying task A needs to be done.
>> 2. Worker comes and stars doing task A.
>> 3. Worker finishes task A but hasn't yet cleared the flag.
>> 4. Requester wants to set flag saying task A needs to be done again.
>> 5. Worker clears the flag without doing anything.
>>
>> Let's make the usbnet codebase consistently clear the flag _before_ it
>> does the requested work.  That way if there's another request to do
>> the work while the work is already in progress it won't be lost.
>>
>> NOTES:
>> - No known bugs are fixed by this; it's just found by code inspection.
>
> Hi,
>
> unfortunately the patch is wrong. The flags must be cleared only
> in case the handler is successful. That is not guaranteed.
>
>         Regards
>                 Oliver
>
> NACK

OK, thanks for reviewing!  I definitely wasn't super confident about
the patch (hence the RFC).

Do you think that the races I identified are possible to hit?  In
other words: should I try to rework the patch somehow or just drop it?
 Originally I had the patch setting the flags back to true in the
failure cases, but then I convinced myself that wasn't needed.  I can
certainly go back and try it that way...

-Doug

^ permalink raw reply

* Re: [PATCH net-next 2/7] kobject: copy env blob in one go
From: Cong Wang @ 2017-09-19 20:52 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric W . Biederman, Eric Dumazet
In-Reply-To: <20170918190733.26272-3-edumazet@google.com>

On Mon, Sep 18, 2017 at 12:07 PM, Eric Dumazet <edumazet@google.com> wrote:
> +                       scratch = skb_put(skb, env->buflen);
> +                       memcpy(scratch, env->buf, env->buflen);

skb_put_data()

^ permalink raw reply

* Re: [PATCH] vsock: vmci: Remove unneeded linux/miscdevice.h include
From: David Miller @ 2017-09-19 20:52 UTC (permalink / raw)
  To: clabbe.montjoie; +Cc: netdev, linux-kernel
In-Reply-To: <20170918181855.32211-1-clabbe.montjoie@gmail.com>

From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Mon, 18 Sep 2017 20:18:55 +0200

> net/vmw_vsock/vmci_transport.c does not use any miscdevice so this patch
> remove this unnecessary inclusion.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Applied, thank you.

^ permalink raw reply

* Re: [RFC PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
From: Guenter Roeck @ 2017-09-19 20:51 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Douglas Anderson, Guenter Roeck, Grant Grundler, linux-kernel,
	linux-usb, netdev
In-Reply-To: <1505853476.15836.9.camel@suse.com>

On Tue, Sep 19, 2017 at 1:37 PM, Oliver Neukum <oneukum@suse.com> wrote:
> Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson:
>> In general when you've got a flag communicating that "something needs
>> to be done" you want to clear that flag _before_ doing the task.  If
>> you clear the flag _after_ doing the task you end up with the risk
>> that this will happen:
>>
>> 1. Requester sets flag saying task A needs to be done.
>> 2. Worker comes and stars doing task A.
>> 3. Worker finishes task A but hasn't yet cleared the flag.
>> 4. Requester wants to set flag saying task A needs to be done again.
>> 5. Worker clears the flag without doing anything.
>>
>> Let's make the usbnet codebase consistently clear the flag _before_ it
>> does the requested work.  That way if there's another request to do
>> the work while the work is already in progress it won't be lost.
>>
>> NOTES:
>> - No known bugs are fixed by this; it's just found by code inspection.
>
> Hi,
>
> unfortunately the patch is wrong. The flags must be cleared only
> in case the handler is successful. That is not guaranteed.
>

Just out of curiosity, what is the retry mechanism ? Whenever a new,
possibly unrelated, event is scheduled ?

Thanks,
Guenter

^ permalink raw reply

* Re: [PATCH net] bpf: do not disable/enable BH in bpf_map_free_id()
From: Daniel Borkmann @ 2017-09-19 20:51 UTC (permalink / raw)
  To: Eric Dumazet, David Miller; +Cc: Martin KaFai Lau, Alexei Starovoitov, netdev
In-Reply-To: <1505837759.29839.64.camel@edumazet-glaptop3.roam.corp.google.com>

On 09/19/2017 06:15 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller reported following splat [1]
>
> Since hard irq are disabled by the caller, bpf_map_free_id()
> should not try to enable/disable BH.
>
> Another solution would be to change htab_map_delete_elem() to
> defer the free_htab_elem() call after
> raw_spin_unlock_irqrestore(&b->lock, flags), but this might be not
> enough to cover other code paths.
>
> [1]
> WARNING: CPU: 1 PID: 8052 at kernel/softirq.c:161 __local_bh_enable_ip
> +0x1e/0x160 kernel/softirq.c:161
> Kernel panic - not syncing: panic_on_warn set ...
>
> CPU: 1 PID: 8052 Comm: syz-executor1 Not tainted 4.13.0-next-20170915+
> #23
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>   __dump_stack lib/dump_stack.c:16 [inline]
>   dump_stack+0x194/0x257 lib/dump_stack.c:52
>   panic+0x1e4/0x417 kernel/panic.c:181
>   __warn+0x1c4/0x1d9 kernel/panic.c:542
>   report_bug+0x211/0x2d0 lib/bug.c:183
>   fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:178
>   do_trap_no_signal arch/x86/kernel/traps.c:212 [inline]
>   do_trap+0x260/0x390 arch/x86/kernel/traps.c:261
>   do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:298
>   do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:311
>   invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:905
> RIP: 0010:__local_bh_enable_ip+0x1e/0x160 kernel/softirq.c:161
> RSP: 0018:ffff8801cdcd7748 EFLAGS: 00010046
> RAX: 0000000000000082 RBX: 0000000000000201 RCX: 0000000000000000
> RDX: 1ffffffff0b5933c RSI: 0000000000000201 RDI: ffffffff85ac99e0
> RBP: ffff8801cdcd7758 R08: ffffffff85b87158 R09: 1ffff10039b9aec6
> R10: ffff8801c99f24c0 R11: 0000000000000002 R12: ffffffff817b0b47
> R13: dffffc0000000000 R14: ffff8801cdcd77e8 R15: 0000000000000001
>   __raw_spin_unlock_bh include/linux/spinlock_api_smp.h:176 [inline]
>   _raw_spin_unlock_bh+0x30/0x40 kernel/locking/spinlock.c:207
>   spin_unlock_bh include/linux/spinlock.h:361 [inline]
>   bpf_map_free_id kernel/bpf/syscall.c:197 [inline]
>   __bpf_map_put+0x267/0x320 kernel/bpf/syscall.c:227
>   bpf_map_put+0x1a/0x20 kernel/bpf/syscall.c:235
>   bpf_map_fd_put_ptr+0x15/0x20 kernel/bpf/map_in_map.c:96
>   free_htab_elem+0xc3/0x1b0 kernel/bpf/hashtab.c:658
>   htab_map_delete_elem+0x74d/0x970 kernel/bpf/hashtab.c:1063
>   map_delete_elem kernel/bpf/syscall.c:633 [inline]
>   SYSC_bpf kernel/bpf/syscall.c:1479 [inline]
>   SyS_bpf+0x2188/0x46a0 kernel/bpf/syscall.c:1451
>   entry_SYSCALL_64_fastpath+0x1f/0xbe
>
> Fixes: f3f1c054c288 ("bpf: Introduce bpf_map ID")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Martin KaFai Lau <kafai@fb.com>

Thanks for the fix, Eric!

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply

* Re: [PATCH 0/3] fix reuseaddr regression
From: David Miller @ 2017-09-19 20:50 UTC (permalink / raw)
  To: josef; +Cc: netdev, linux-kernel, crobinso, labbott, kernel-team
In-Reply-To: <1505752137-15522-1-git-send-email-jbacik@fb.com>

From: josef@toxicpanda.com
Date: Mon, 18 Sep 2017 12:28:54 -0400

> I introduced a regression when reworking the fastreuse port stuff that allows
> bind conflicts to occur once a reuseaddr socket successfully opens on an
> existing tb.  The root cause is I reversed an if statement which caused us to
> set the tb as if there were no owners on the socket if there were, which
> obviously is not correct.
> 
> Dave I have follow up patches that will add a selftest for this case and I ran
> the other reuseport related tests as well.  These need to go in pretty quickly
> as it breaks kvm, I've marked them for stable.  Sorry for the regression,

First, please fix your "From: " field so that it actually has your full
name rather than just your email address.  This matter when I apply
your patches.

Second, remove the stable CC:.  For networking changes, you simply ask
me to queue the changes up for -stable.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next] net: mvpp2: remove useless goto
From: David Miller @ 2017-09-19 20:49 UTC (permalink / raw)
  To: antoine.tenart
  Cc: andrew, gregory.clement, thomas.petazzoni, miquel.raynal, nadavh,
	linux, linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170918133651.29556-1-antoine.tenart@free-electrons.com>

From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Mon, 18 Sep 2017 15:36:51 +0200

> Remove a goto in the PPv2 tx function which jumps to the next line
> anyway. This is a cosmetic commit.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net] net: phy: Kconfig: Fix PHY infrastructure menu in menuconfig
From: David Miller @ 2017-09-19 20:47 UTC (permalink / raw)
  To: jbrunet; +Cc: andrew, f.fainelli, netdev, linux-kernel, rmk+kernel
In-Reply-To: <20170918125920.10298-1-jbrunet@baylibre.com>

From: Jerome Brunet <jbrunet@baylibre.com>
Date: Mon, 18 Sep 2017 14:59:20 +0200

> Since the integration of PHYLINK, the configuration option which
> used to be under the PHY infrastructure menu in menuconfig ended
> up one level up (the network device driver section)
> 
> By placing PHYLINK option right after PHYLIB entry, it broke the
> way Kconfig used to build the menu. See kconfig-language.txt, section
> "Menu structure", 2nd method.
> 
> This is fixed by placing the PHYLINK option just before PHYLIB.
> 
> Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Applied, thank you.

^ permalink raw reply

* Re: cross namespace interface notification for tun devices
From: Cong Wang @ 2017-09-19 20:40 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Netdev, Mathias
In-Reply-To: <CAHmME9pD_kees+xrjTeWXgi26UdhbMVgvoddbCyueTZkbguvAg@mail.gmail.com>

On Mon, Sep 18, 2017 at 11:47 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hey guys,
>
> It's possible to create a tun device in a process in namespace A and
> then move that interface to namespace B. The controlling process in A
> needs to receive notifications on when the interface is brought up or
> down. It can receive these notifications via netlink while the
> interface lives in A but not when it moves to B.

By "notification" I assume you mean netlink notification.

>
> Any tricks or APIs to get around this?

The question is why does the process in A still care about
the device sitting in B?

Also, the process should be able to receive a last notification
on IFF_UP|IFF_RUNNING before device is finally moved to B.
After this point, it should not have any relation to netns A
any more, like the device were completely gone.

^ permalink raw reply

* Re: [PATCH net-next v4 3/4] bpf: add helper bpf_perf_prog_read_value
From: Daniel Borkmann @ 2017-09-19 20:40 UTC (permalink / raw)
  To: Yonghong Song, peterz, rostedt, ast, netdev; +Cc: kernel-team
In-Reply-To: <20170919070413.3838201-4-yhs@fb.com>

On 09/19/2017 09:04 AM, Yonghong Song wrote:
[...]
>   #ifdef CONFIG_CGROUP_PERF
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 2c68b9e..ba77022 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -590,6 +590,13 @@ union bpf_attr {
>    *     @buf: buf to fill
>    *     @buf_size: size of the buf
>    *     Return: 0 on success or negative error code
> + *
> + * int bpf_perf_prog_read_value(ctx, buf, buf_size)
> + *     read perf prog attached perf event counter and enabled/running time
> + *     @ctx: pointer to ctx
> + *     @buf: buf to fill
> + *     @buf_size: size of the buf
> + *     Return : 0 on success or negative error code
>    */
>   #define __BPF_FUNC_MAPPER(FN)		\
>   	FN(unspec),			\
> @@ -647,6 +654,7 @@ union bpf_attr {
>   	FN(sk_redirect_map),		\
>   	FN(sock_map_update),		\
>   	FN(perf_event_read_value),		\
> +	FN(perf_prog_read_value),		\

(Same here.)

>
>   /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>    * function eBPF program intends to call
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 2d5bbe5..d039086 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8081,6 +8081,7 @@ static void bpf_overflow_handler(struct perf_event *event,
>   	struct bpf_perf_event_data_kern ctx = {
>   		.data = data,
>   		.regs = regs,
> +		.event = event,
>   	};
>   	int ret = 0;
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 39ce5d9..596b5c9 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -603,6 +603,18 @@ BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,
>   			       flags, 0, 0);
>   }
>
> +BPF_CALL_3(bpf_perf_prog_read_value_tp, void *, ctx, struct bpf_perf_event_value *,
> +	buf, u32, size)

Nit: indent

> +{
> +	struct bpf_perf_event_data_kern *kctx = (struct bpf_perf_event_data_kern *)ctx;

Why having the arg as void * and have this detour instead of having
struct bpf_perf_event_data_kern * right in the helper signature as
argument?

> +	if (size != sizeof(struct bpf_perf_event_value))

unlikely()

> +		return -EINVAL;
> +
> +	return perf_event_read_local(kctx->event, &buf->counter, &buf->enabled,
> +				     &buf->running);
> +}

bpf_perf_prog_read_value_proto_tp would go right underneath here,
and bpf_get_stackid_proto_tp below the previous helper above.

>   static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
>   	.func		= bpf_get_stackid_tp,
>   	.gpl_only	= true,
> @@ -612,6 +624,15 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
>   	.arg3_type	= ARG_ANYTHING,
>   };
>
> +static const struct bpf_func_proto bpf_perf_prog_read_value_proto_tp = {
> +         .func           = bpf_perf_prog_read_value_tp,
> +         .gpl_only       = true,
> +         .ret_type       = RET_INTEGER,
> +         .arg1_type      = ARG_PTR_TO_CTX,
> +         .arg2_type      = ARG_PTR_TO_UNINIT_MEM,

Same on error path.

> +         .arg3_type      = ARG_CONST_SIZE,
> +};
> +
>   static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
>   {
>   	switch (func_id) {
> @@ -619,6 +640,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
>   		return &bpf_perf_event_output_proto_tp;
>   	case BPF_FUNC_get_stackid:
>   		return &bpf_get_stackid_proto_tp;
> +	case BPF_FUNC_perf_prog_read_value:
> +		return &bpf_perf_prog_read_value_proto_tp;
>   	default:
>   		return tracing_func_proto(func_id);
>   	}
>

^ permalink raw reply

* [PATCH] ath9k: make const array reg_hole_list static, reduces object code size
From: Colin King @ 2017-09-19 20:40 UTC (permalink / raw)
  To: QCA ath9k Development, Kalle Valo, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Don't populate the read-only array reg_hole_list on the stack, instead make
it static.  Makes the object code smaller by over 200 bytes:

Before:
   text	   data	    bss	    dec	    hex	filename
  57518	  15248	      0	  72766	  11c3e	debug.o

After:
   text	   data	    bss	    dec	    hex	filename
  57218	  15344	      0	  72562	  11b72	debug.o

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/ath/ath9k/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 01fa30117288..5a0a05abd51a 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -916,7 +916,7 @@ static int open_file_regdump(struct inode *inode, struct file *file)
 	u8 *buf;
 	int i, j = 0;
 	unsigned long num_regs, regdump_len, max_reg_offset;
-	const struct reg_hole {
+	static const struct reg_hole {
 		u32 start;
 		u32 end;
 	} reg_hole_list[] = {
-- 
2.14.1

^ permalink raw reply related

* Re: [RFC PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
From: Oliver Neukum @ 2017-09-19 20:37 UTC (permalink / raw)
  To: Douglas Anderson; +Cc: groeck, grundler, linux-kernel, linux-usb, netdev
In-Reply-To: <20170919161522.995-2-dianders@chromium.org>

Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson:
> In general when you've got a flag communicating that "something needs
> to be done" you want to clear that flag _before_ doing the task.  If
> you clear the flag _after_ doing the task you end up with the risk
> that this will happen:
> 
> 1. Requester sets flag saying task A needs to be done.
> 2. Worker comes and stars doing task A.
> 3. Worker finishes task A but hasn't yet cleared the flag.
> 4. Requester wants to set flag saying task A needs to be done again.
> 5. Worker clears the flag without doing anything.
> 
> Let's make the usbnet codebase consistently clear the flag _before_ it
> does the requested work.  That way if there's another request to do
> the work while the work is already in progress it won't be lost.
> 
> NOTES:
> - No known bugs are fixed by this; it's just found by code inspection.

Hi,

unfortunately the patch is wrong. The flags must be cleared only
in case the handler is successful. That is not guaranteed.

	Regards
		Oliver

NACK

^ permalink raw reply

* Re: [RFC PATCH 1/3] usbnet: Get rid of spammy usbnet "kevent X may have been dropped"
From: Oliver Neukum @ 2017-09-19 20:36 UTC (permalink / raw)
  To: Douglas Anderson; +Cc: groeck, grundler, linux-kernel, linux-usb, netdev
In-Reply-To: <20170919161522.995-1-dianders@chromium.org>

Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson:
> 
> ALSO NOTE: If somehow some of the types of work need to be repeated if
> usbnet_defer_kevent() is called multiple times then that should be
> quite easy to accomplish without dropping any work on the floor.  We
> can just keep an atomic count for that type of work and add a loop
> into usbnet_deferred_kevent().

Thanks for doing this, it is overdue.

	Regards
		Oliver

> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Oliver Neukum <oneukum@suse.com>

^ permalink raw reply

* Re: [PATCH net-next v4 1/4] bpf: add helper bpf_perf_event_read_value for perf event array map
From: Daniel Borkmann @ 2017-09-19 20:33 UTC (permalink / raw)
  To: Yonghong Song, peterz, rostedt, ast, netdev; +Cc: kernel-team
In-Reply-To: <20170919070413.3838201-2-yhs@fb.com>

On 09/19/2017 09:04 AM, Yonghong Song wrote:
[...]

Just some minor things, but a bit scattered.

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 43ab5c4..2c68b9e 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -582,6 +582,14 @@ union bpf_attr {
>    *	@map: pointer to sockmap to update
>    *	@key: key to insert/update sock in map
>    *	@flags: same flags as map update elem
> + *
> + * int bpf_perf_event_read_value(map, flags, buf, buf_size)
> + *     read perf event counter value and perf event enabled/running time
> + *     @map: pointer to perf_event_array map
> + *     @flags: index of event in the map or bitmask flags
> + *     @buf: buf to fill
> + *     @buf_size: size of the buf
> + *     Return: 0 on success or negative error code
>    */
>   #define __BPF_FUNC_MAPPER(FN)		\
>   	FN(unspec),			\
> @@ -638,6 +646,7 @@ union bpf_attr {
>   	FN(redirect_map),		\
>   	FN(sk_redirect_map),		\
>   	FN(sock_map_update),		\
> +	FN(perf_event_read_value),		\

Nit: can you indent the '\' so it aligns with the rest

>   /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>    * function eBPF program intends to call
> @@ -681,7 +690,8 @@ enum bpf_func_id {
>   #define BPF_F_ZERO_CSUM_TX		(1ULL << 1)
>   #define BPF_F_DONT_FRAGMENT		(1ULL << 2)
>
> -/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
> +/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
> + * BPF_FUNC_perf_event_read_value flags. */

Nit: comment style

>   #define BPF_F_INDEX_MASK		0xffffffffULL
>   #define BPF_F_CURRENT_CPU		BPF_F_INDEX_MASK
>   /* BPF_FUNC_perf_event_output for sk_buff input context. */
> @@ -864,4 +874,10 @@ enum {
>   #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
>   #define TCP_BPF_SNDCWND_CLAMP	1002	/* Set sndcwnd_clamp */
>
> +struct bpf_perf_event_value {
> +	__u64 counter;
> +	__u64 enabled;
> +	__u64 running;
> +};
> +
[...]
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 3e691b7..2d5bbe5 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3684,10 +3684,12 @@ static inline u64 perf_event_count(struct perf_event *event)
>    *     will not be local and we cannot read them atomically
>    *   - must not have a pmu::count method
>    */
> -int perf_event_read_local(struct perf_event *event, u64 *value)
> +int perf_event_read_local(struct perf_event *event, u64 *value,
> +			  u64 *enabled, u64 *running)
>   {
>   	unsigned long flags;
>   	int ret = 0;
> +	u64 now;
>
>   	/*
>   	 * Disabling interrupts avoids all counter scheduling (context
> @@ -3718,14 +3720,21 @@ int perf_event_read_local(struct perf_event *event, u64 *value)
>   		goto out;
>   	}
>
> +	now = event->shadow_ctx_time + perf_clock();
> +	if (enabled)
> +		*enabled = now - event->tstamp_enabled;
>   	/*
>   	 * If the event is currently on this CPU, its either a per-task event,
>   	 * or local to this CPU. Furthermore it means its ACTIVE (otherwise
>   	 * oncpu == -1).
>   	 */
> -	if (event->oncpu == smp_processor_id())
> +	if (event->oncpu == smp_processor_id()) {
>   		event->pmu->read(event);
> -
> +		if (running)
> +			*running = now - event->tstamp_running;
> +	} else if (running) {
> +		*running = event->total_time_running;
> +	}
>   	*value = local64_read(&event->count);
>   out:
>   	local_irq_restore(flags);
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index dc498b6..39ce5d9 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -255,13 +255,13 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
>   	return &bpf_trace_printk_proto;
>   }
>
> -BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
> -{
> +static __always_inline int
> +get_map_perf_counter(struct bpf_map *map, u64 flags,
> +		u64 *value, u64 *enabled, u64 *running) {

Nit: coding style

>   	struct bpf_array *array = container_of(map, struct bpf_array, map);
>   	unsigned int cpu = smp_processor_id();
>   	u64 index = flags & BPF_F_INDEX_MASK;
>   	struct bpf_event_entry *ee;
> -	u64 value = 0;
>   	int err;
>
>   	if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
> @@ -275,7 +275,17 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
>   	if (!ee)
>   		return -ENOENT;
>
> -	err = perf_event_read_local(ee->event, &value);
> +	err = perf_event_read_local(ee->event, value, enabled, running);
> +	return err;

err can be removed entirely then.

   return perf_event_read_local(ee->event, value, enabled, running);

> +}
> +
> +

Nit: Two newlines slipped in.

> +BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
> +{
> +	u64 value = 0;
> +	int err;
> +
> +	err = get_map_perf_counter(map, flags, &value, NULL, NULL);
>   	/*
>   	 * this api is ugly since we miss [-22..-2] range of valid
>   	 * counter values, but that's uapi
> @@ -285,6 +295,20 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
>   	return value;
>   }

Can you also move the bpf_perf_event_read_proto definition right
underneath here as we have with all other helpers?

> +BPF_CALL_4(bpf_perf_event_read_value, struct bpf_map *, map, u64, flags,
> +	struct bpf_perf_event_value *, buf, u32, size)

Nit: indent

> +{
> +	int err;
> +
> +	if (unlikely(size != sizeof(struct bpf_perf_event_value)))
> +		return -EINVAL;
> +	err = get_map_perf_counter(map, flags, &buf->counter, &buf->enabled,
> +                            &buf->running);

^ This is only indented with spaces.

> +	if (err)
> +		return err;
> +	return 0;

Also here err can be removed entirely, make it
less convoluted:

BPF_CALL_4(bpf_perf_event_read_value, struct bpf_map *, map, u64, flags,
           struct bpf_perf_event_value *, eval, u32, size)
{
        if (unlikely(size != sizeof(struct bpf_perf_event_value)))
                return -EINVAL;

        return get_map_perf_counter(map, flags, &eval->counter, &eval->enabled,
                                    &eval->running);
}

> +}
> +
>   static const struct bpf_func_proto bpf_perf_event_read_proto = {
>   	.func		= bpf_perf_event_read,
>   	.gpl_only	= true,
> @@ -293,6 +317,16 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
>   	.arg2_type	= ARG_ANYTHING,
>   };
>
> +static const struct bpf_func_proto bpf_perf_event_read_value_proto = {
> +	.func		= bpf_perf_event_read_value,
> +	.gpl_only	= true,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_CONST_MAP_PTR,
> +	.arg2_type	= ARG_ANYTHING,
> +	.arg3_type	= ARG_PTR_TO_UNINIT_MEM,

If you do that, then error paths need to memset the region, e.g.
see bpf_skb_get_tunnel_opt() and bpf_skb_get_tunnel_key() helpers
which operate similarly.

> +	.arg4_type	= ARG_CONST_SIZE,
> +};
> +
>   static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
>
>   static __always_inline u64
> @@ -499,6 +533,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
>   		return &bpf_perf_event_output_proto;
>   	case BPF_FUNC_get_stackid:
>   		return &bpf_get_stackid_proto;
> +	case BPF_FUNC_perf_event_read_value:
> +		return &bpf_perf_event_read_value_proto;
>   	default:
>   		return tracing_func_proto(func_id);
>   	}
>

^ permalink raw reply

* [PATCH net-next] net_sched: no need to free qdisc in RCU callback
From: Cong Wang @ 2017-09-19 20:15 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, Eric Dumazet

gen estimator has been rewritten in commit 1c0d32fde5bd
("net_sched: gen_estimator: complete rewrite of rate estimators"),
the caller no longer needs to wait for a grace period. So this
patch gets rid of it.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/sch_generic.h |  1 -
 net/sched/sch_generic.c   | 10 ++--------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 135f5a2dd931..684d8ed27eaa 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -93,7 +93,6 @@ struct Qdisc {
 	unsigned long		state;
 	struct Qdisc            *next_sched;
 	struct sk_buff		*skb_bad_txq;
-	struct rcu_head		rcu_head;
 	int			padded;
 	refcount_t		refcnt;
 
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 92237e75dbbc..1fb0c754b7fd 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -688,10 +688,8 @@ void qdisc_reset(struct Qdisc *qdisc)
 }
 EXPORT_SYMBOL(qdisc_reset);
 
-static void qdisc_rcu_free(struct rcu_head *head)
+static void qdisc_free(struct Qdisc *qdisc)
 {
-	struct Qdisc *qdisc = container_of(head, struct Qdisc, rcu_head);
-
 	if (qdisc_is_percpu_stats(qdisc)) {
 		free_percpu(qdisc->cpu_bstats);
 		free_percpu(qdisc->cpu_qstats);
@@ -724,11 +722,7 @@ void qdisc_destroy(struct Qdisc *qdisc)
 
 	kfree_skb_list(qdisc->gso_skb);
 	kfree_skb(qdisc->skb_bad_txq);
-	/*
-	 * gen_estimator est_timer() might access qdisc->q.lock,
-	 * wait a RCU grace period before freeing qdisc.
-	 */
-	call_rcu(&qdisc->rcu_head, qdisc_rcu_free);
+	qdisc_free(qdisc);
 }
 EXPORT_SYMBOL(qdisc_destroy);
 
-- 
2.13.0

^ permalink raw reply related

* Re: [PATCH net-next 0/4] net: dsa: move master ethtool code
From: Florian Fainelli @ 2017-09-19 20:04 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-1-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> The DSA core overrides the master device's ethtool_ops structure so that
> it can inject statistics and such of its dedicated switch CPU port.
> 
> This ethtool code is currently called on unnecessary conditions or
> before the master interface and its switch CPU port get wired up.
> This patchset fixes this.
> 
> Similarly to slave.c where the DSA slave net_device is the entry point
> of the dsa_slave_* functions, this patchset also isolates the master's
> ethtool code in a new master.c file, where the DSA master net_device is
> the entry point of the dsa_master_* functions.
> 
> This is a first step towards better control of the master device and
> support for multiple CPU ports.

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

* ethtool -S eth0 -> switch port CPU stats are still correctly overlayed
* ethtool -s gphy wol g -> both switch port and CPU port correctly
enable WoL
* ethtool -i eth0 -> driver still reports correct information

Thanks!

> 
> Vivien Didelot (4):
>   net: dsa: remove copy of master ethtool_ops
>   net: dsa: setup master ethtool unconditionally
>   net: dsa: setup master ethtool after dsa_ptr
>   net: dsa: move master ethtool code
> 
>  include/net/dsa.h  |   1 -
>  net/dsa/Makefile   |   2 +-
>  net/dsa/dsa.c      |  28 -------------
>  net/dsa/dsa2.c     |  18 ++++----
>  net/dsa/dsa_priv.h |   7 ++--
>  net/dsa/legacy.c   |  10 ++---
>  net/dsa/master.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/dsa/slave.c    |  80 -----------------------------------
>  8 files changed, 136 insertions(+), 130 deletions(-)
>  create mode 100644 net/dsa/master.c
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH 2/3] selftests: actually run the various net selftests
From: Shuah Khan @ 2017-09-19 20:00 UTC (permalink / raw)
  To: Willem de Bruijn, Josef Bacik
  Cc: Willem de Bruijn, Josef Bacik, David S. Miller, LKML,
	linux-kselftest, Network Development, Shuah Khan, Shuah Khan
In-Reply-To: <CAF=yD-JewUiQso6XbDEAjC+gP=yK=czsdujYmcu5=jjXTsweDA@mail.gmail.com>

On 09/19/2017 12:14 PM, Willem de Bruijn wrote:
> On Tue, Sep 19, 2017 at 9:34 AM, Josef Bacik <josef@toxicpanda.com> wrote:
>> On Mon, Sep 18, 2017 at 04:14:41PM -0600, Shuah Khan wrote:
>>> On 09/18/2017 11:32 AM, josef@toxicpanda.com wrote:
>>>> From: Josef Bacik <jbacik@fb.com>
>>>>
>>>> These self tests are just self contained binaries, they are not run by
>>>> any of the scripts in the directory.  This means they need to be marked
>>>> with TEST_GEN_PROGS to actually be run, not TEST_GEN_FILES.
>>>>
>>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>>>> ---
>>>>  tools/testing/selftests/net/Makefile | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
>>>> index 3df542c84610..45a4e77a47c4 100644
>>>> --- a/tools/testing/selftests/net/Makefile
>>>> +++ b/tools/testing/selftests/net/Makefile
>>>> @@ -6,8 +6,8 @@ CFLAGS += -I../../../../usr/include/
>>>>  TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
>>>>  TEST_GEN_FILES =  socket
>>>>  TEST_GEN_FILES += psock_fanout psock_tpacket
>>>> -TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>>>> -TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
>>>> +TEST_GEN_PROGS += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>>>> +TEST_GEN_PROGS += reuseport_dualstack msg_zerocopy reuseaddr_conflict
>>>
>>> Hmm. I see msg_zerocopy.sh for running msg_zerocopy. msg_zerocopy should
>>> still stay in TEST_GEN_FILES and msg_zerocopy.sh needs to be added to
>>> TEST_PROGS so it runs.
>>>
>>
>> Actually the shell script requires arguments, it doesn't just run the test.
>> I'll fix this to just omit the test for now as it's not setup to run properly.
>>
>> Willem, could you follow up with a patch so that the zero copy test is run
>> properly the way you envision it running?  You need to make sure that
>>
>> make -C tools/testing/selftests TARGETS=net run_tests
>>
>> actually runs your zero copy test the way you expect it to, otherwise it's just
>> sitting there collecting dust.  Thanks,
> 
> Will do.
> 
> In its current state, this test is really only meant to be run manually.
> It demonstrates the API and outputs some information on stderr.
> 
> Zerocopy itself requires a two-host test. The feature is expressly
> disabled over loopback.

Running this manually is [perfectly fine. TEST_GEN_FILES is the right
place for it as TEST_GEN_FILES will get built and installed, but won't
be run bt lib.mk. No changes needed.

> 
> But I can make this a pass/fail tests that exercises the interface
> and notification channel and verifies that data was copied. It will
> be a bit more work than just changing the default invocation of
> msg_zerocopy.sh
> 

No need to make changes as this test is intended to be run manually.
Josef's v2 doesn't change the location of msg_zerocopy. We are all set.

Thanks for clearing this up.

-- Shuah

^ permalink raw reply

* Re: [PATCH net-next 1/3] bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
From: Daniel Mack @ 2017-09-19 19:48 UTC (permalink / raw)
  To: Daniel Borkmann, Craig Gallek, Alexei Starovoitov
  Cc: David S . Miller, netdev
In-Reply-To: <59C141DC.9060200@iogearbox.net>

Hi,

Thanks for working on this, Craig!

On 09/19/2017 06:12 PM, Daniel Borkmann wrote:
> On 09/19/2017 05:08 PM, Craig Gallek wrote:
>> On Mon, Sep 18, 2017 at 6:53 PM, Alexei Starovoitov <ast@fb.com> wrote:
>>> On 9/18/17 12:30 PM, Craig Gallek wrote:
> [...]
>>>> +
>>>> +               next_bit = extract_bit(key->data, node->prefixlen);
>>>> +               /* If we hit a node that has more than one child or is a
>>>> valid
>>>> +                * prefix itself, do not remove it. Reset the root of the
>>>> trim
>>>> +                * path to its descendant on our path.
>>>> +                */
>>>> +               if (!(node->flags & LPM_TREE_NODE_FLAG_IM) ||
>>>> +                   (node->child[0] && node->child[1]))
>>>> +                       trim = &node->child[next_bit];
>>>> +               node = rcu_dereference_protected(
>>>> +                       node->child[next_bit],
>>>> lockdep_is_held(&trie->lock));
>>>> +       }
>>>> +
>>>> +       if (!node || node->prefixlen != key->prefixlen ||
>>>> +           (node->flags & LPM_TREE_NODE_FLAG_IM)) {
>>>> +               ret = -ENOENT;
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       trie->n_entries--;
>>>> +
>>>> +       /* If the node we are removing is not a leaf node, simply mark it
>>>> +        * as intermediate and we are done.
>>>> +        */
>>>> +       if (rcu_access_pointer(node->child[0]) ||
>>>> +           rcu_access_pointer(node->child[1])) {
>>>> +               node->flags |= LPM_TREE_NODE_FLAG_IM;
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       /* trim should now point to the slot holding the start of a path
>>>> from
>>>> +        * zero or more intermediate nodes to our leaf node for deletion.
>>>> +        */
>>>> +       while ((node = rcu_dereference_protected(
>>>> +                       *trim, lockdep_is_held(&trie->lock)))) {
>>>> +               RCU_INIT_POINTER(*trim, NULL);
>>>> +               trim = rcu_access_pointer(node->child[0]) ?
>>>> +                       &node->child[0] :
>>>> +                       &node->child[1];
>>>> +               kfree_rcu(node, rcu);
>>>
>>> can it be that some of the nodes this loop walks have
>>> both child[0] and [1] ?
>> No, the loop above will push trim down the walk every time it
>> encounters a node with two children.  The only other trim assignment
>> is the initial trim = &trie->root.  But the only time we would skip
>> the assignment in the loop is if the node being removed is the root.
>> If the root had multiple children and is being removed, it would be
>> handled by the case that turns the node into an intermediate node
>> rather than walking the trim path freeing things.
> 
> Looks good to me. We should probably still merge nodes once we turn
> a real node into an im which just has a single child attached to it;
> parent can be im or real node. Thus, we don't need to traverse this
> extra one on lookup.

Right, but only if the the parent of the node allows us to do that,
because the 'next bit' in the lookup key has to match the slot index.

To illustrate, consider the following trie with no IM nodes:

                      +----------------+
                      |       (1)  (R) |
                      | 192.168.0.0/16 |
                      |    value: 1    |
                      |   [0]    [1]   |
                      +----------------+
                           |      |
            +----------------+  +------------------+
            |       (2)      |  |        (3)       |
            | 192.168.0.0/23 |  | 192.168.128.0/24 |
            |    value: 2    |  |     value: 3     |
            |   [0]    [1]   |  |    [0]    [1]    |
            +----------------+  +------------------+
                        |
                      +----------------+
                      |       (4)      |
                      | 192.168.1.0/24 |
                      |     value: 4   |
                      |   [0]    [1]   |
                      +----------------+

If you now try to delete (2), the node has to stay around because (3)
and (4) share the same value in bit 17 (1). If, however, (4) had a
prefix of 192.168.0.0/24, then (2) should be removed completely, and (4)
should be directly attached to (1) as child[0].

With this implementation, a situation in which multiple IM nodes appear
in a chain cannot emerge. And that again should make your trimming
algorithm simpler, because you only need to find an exact match, and
then handle three distinct cases:

a) the node as 0 children: simply remove it and nullify the pointer in
the parent

b) the node has 1 child: apply logic I described above

c) the node has 2 children: turn the node into an IM


Makes sense?


Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH net-next 4/4] net: dsa: move master ethtool code
From: Florian Fainelli @ 2017-09-19 19:56 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-5-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:57 AM, Vivien Didelot wrote:
> DSA overrides the master device ethtool ops, so that it can inject stats
> from its dedicated switch CPU port as well.
> 
> The related code is currently split in dsa.c and slave.c, but it only
> scopes the master net device. Move it to a new master.c DSA core file.
> 
> This file will be later extented with master net device specific code.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 1/4] net: dsa: remove copy of master ethtool_ops
From: Florian Fainelli @ 2017-09-19 19:55 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-2-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> There is no need to store a copy of the master ethtool ops, storing the
> original pointer in DSA and the new one in the master netdev itself is
> enough.
> 
> In the meantime, set orig_ethtool_ops to NULL when restoring the master
> ethtool ops and check the presence of the master original ethtool ops as
> well as its needed functions before calling them.

I clearly like memcpy() too much, this looks good:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [5/5] e1000e: Avoid receiver overrun interrupt bursts
From: Benjamin Poirier @ 2017-09-19 19:41 UTC (permalink / raw)
  To: Philip Prindeville
  Cc: Jeff Kirsher, netdev, intel-wired-lan, linux-kernel,
	Lennart Sorensen
In-Reply-To: <DDD41C67-CDF2-4443-9F49-0DC5489D9243@redfish-solutions.com>

On 2017/09/19 12:38, Philip Prindeville wrote:
> Hi.
> 
> We’ve been running this patchset (all 5) for about as long as they’ve been under review… about 2 months.  And in a burn-in lab with heavy traffic.
> 
> We’ve not seen a single link-flap in hundreds of ours of saturated traffic.
> 
> Would love to see some resolution soon on this as we don’t want to ship a release with unsanctioned patches.
> 
> Is there an estimate on when that might be?

The patches have been added to Jeff Kirsher's next-queue tree. I guess
they will be submitted for v4.15 which might be released in early
2018...
http://phb-crystal-ball.org/

^ 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