* [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop
From: Heiner Kallweit @ 2018-09-18 19:56 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller
Cc: netdev@vger.kernel.org, Geert Uytterhoeven
In-Reply-To: <fc18a6b5-5022-ac2b-9e68-584b4c28bb71@gmail.com>
phy_stop() may be called e.g. when suspending, therefore all needed
actions should be performed synchronously. Therefore add a synchronous
call to the state machine.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c78203b25..f5bb6a7a8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -861,6 +861,8 @@ void phy_stop(struct phy_device *phydev)
out_unlock:
mutex_unlock(&phydev->lock);
+ phy_state_machine(&phydev->state_queue.work);
+
/* Cannot call flush_scheduled_work() here as desired because
* of rtnl_lock(), but PHY_HALTED shall guarantee phy_change()
* will not reenable interrupts.
--
2.19.0
^ permalink raw reply related
* Re: [PATCH net-next v5 07/20] zinc: Poly1305 generic C implementations and selftest
From: Jason A. Donenfeld @ 2018-09-19 1:35 UTC (permalink / raw)
To: Eric Biggers
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <20180919005054.GC74746@gmail.com>
On Wed, Sep 19, 2018 at 2:50 AM Eric Biggers <ebiggers@kernel.org> wrote:
> Hardcoding the 'input' array to 600 bytes forces the full amount of space to be
> reserved in the kernel image for every test vector. Also, if anyone adds a
> longer test vector they will need to remember to increase the value.
>
> It should be a const pointer instead, like the test vectors in crypto/testmgr.h.
I know. The agony. This has been really annoying me. I originally did
it the right way, but removed it last week, when I noticed that gcc
failed to put it in the initconst section:
https://git.zx2c4.com/WireGuard/commit/?id=f4698d20f13946afc6ce99e98685ba3f9adc4474
Even changing the (u8[]){ ... } into a (const u8[]){ ... } or even
into a const string literal does not do the trick. It makes it into
the constant data section with const, but it does not make it into the
initconst section. What a bummer.
I went asking about this on the gcc mailing list, to see if there was
just some aspect of C that I had overlooked:
https://gcc.gnu.org/ml/gcc/2018-09/msg00043.html So far, it looks like
we're SOL. I could probably make some macros to do this in a .S file
but... that's pretty unacceptably gross. Or I could define lots and
lots of variables in __initconst, and then connect them all together
at the end, but that's pretty gross too. Or I could have all this data
in one variable and record offsets into it, but that's even more
atrocious.
So I think it comes down to these two non-ugly options:
- We use fixed sized buffers, waste a lot of space, and be happy that
it's cleared from memory immediately after init/insertion anyway, so
it's not actually wasting ram.
- We use const string literals / constant compound literals, save
space on disk, but not benefit from having it cleared from memory
after init/insertion.
Of these, which would you prefer? I can see the argument both ways,
but in the end opted for the first. Or perhaps you have a better third
option?
Jason
^ permalink raw reply
* Re: [PATCH] net: phy: don't reschedule state machine when PHY is halted
From: Florian Fainelli @ 2018-09-18 20:02 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <d32d4101-03cb-df57-66d2-29e9a17fc2c5@gmail.com>
On 09/18/2018 12:12 PM, Heiner Kallweit wrote:
> I think I've seen a similar or same patch before, not sure why it
> didn't make it yet. When being in state PHY_HALTED we don't have to
> reschedule the state machine, phy_start() will start it again.
Yes, this is conceptually the same patch as as this one:
https://patchwork.ozlabs.org/patch/830415/
I prefer your version, though the comment in the original patch explains
why.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/net/phy/phy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 1ee25877c..c78203b25 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1123,7 +1123,7 @@ void phy_state_machine(struct work_struct *work)
> * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
> * between states from phy_mac_interrupt()
> */
> - if (phy_polling_mode(phydev))
> + if (phy_polling_mode(phydev) && old_state != PHY_HALTED)
> queue_delayed_work(system_power_efficient_wq, &phydev->state_queue,
> PHY_STATE_TIME * HZ);
> }
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v5 07/20] zinc: Poly1305 generic C implementations and selftest
From: Jason A. Donenfeld @ 2018-09-19 1:39 UTC (permalink / raw)
To: Eric Biggers
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <20180919005054.GC74746@gmail.com>
> > + const size_t num = ctx->num % POLY1305_BLOCK_SIZE;
> 0 <= ctx->num < POLY1305_BLOCK_SIZE, so no need to mod by POLY1305_BLOCK_SIZE.
> > + size_t num = ctx->num % POLY1305_BLOCK_SIZE;
> Same here.
I know, but I was having a hard time convincing gcc-8 of that
invariant, and it was warning me. Perhaps this is something they
fixed, though, between 8.1 and 8.2 though. I'll check back and adjust
accordingly.
^ permalink raw reply
* Re: [PATCH net-next v5 07/20] zinc: Poly1305 generic C implementations and selftest
From: Jason A. Donenfeld @ 2018-09-19 1:41 UTC (permalink / raw)
To: Eric Biggers
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <CAHmME9rab92gc68tGX384eGR71q+7JtovpxBLSYrYNHA7L5zFw@mail.gmail.com>
On Wed, Sep 19, 2018 at 3:39 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> > > + const size_t num = ctx->num % POLY1305_BLOCK_SIZE;
> > 0 <= ctx->num < POLY1305_BLOCK_SIZE, so no need to mod by POLY1305_BLOCK_SIZE.
> > > + size_t num = ctx->num % POLY1305_BLOCK_SIZE;
> > Same here.
>
> I know, but I was having a hard time convincing gcc-8 of that
> invariant, and it was warning me. Perhaps this is something they
> fixed, though, between 8.1 and 8.2 though. I'll check back and adjust
> accordingly.
This was changed here:
https://git.zx2c4.com/WireGuard/commit/?id=37f114a73ba37219b00a66f0a51219a696599745
I can't reproduce with 8.2 anymore, so perhaps I should remove it now.
Unless you'd like to avoid a warning on old compilers. Since there's
no difference in speed, probably we should avoid the 8.1 warning and
leave it be?
^ permalink raw reply
* Re: [PATCH] net: phy: don't reschedule state machine when PHY is halted
From: Heiner Kallweit @ 2018-09-18 20:17 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <faeb4977-10e9-96aa-ce70-7fc637d117cb@gmail.com>
On 18.09.2018 22:02, Florian Fainelli wrote:
> On 09/18/2018 12:12 PM, Heiner Kallweit wrote:
>> I think I've seen a similar or same patch before, not sure why it
>> didn't make it yet. When being in state PHY_HALTED we don't have to
>> reschedule the state machine, phy_start() will start it again.
>
> Yes, this is conceptually the same patch as as this one:
>
> https://patchwork.ozlabs.org/patch/830415/
>
Thanks for the link, this is what I was referring to.
> I prefer your version, though the comment in the original patch explains
> why.
>
To be sure I understand you correctly:
You're fine with the patch as is or would you prefer to add a comment
like in the original patch ?
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/net/phy/phy.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 1ee25877c..c78203b25 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -1123,7 +1123,7 @@ void phy_state_machine(struct work_struct *work)
>> * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
>> * between states from phy_mac_interrupt()
>> */
>> - if (phy_polling_mode(phydev))
>> + if (phy_polling_mode(phydev) && old_state != PHY_HALTED)
>> queue_delayed_work(system_power_efficient_wq, &phydev->state_queue,
>> PHY_STATE_TIME * HZ);
>> }
>>
>
>
^ permalink raw reply
* Re: [Patch net-next] ipv4: initialize ra_mutex in inet_init_net()
From: Cong Wang @ 2018-09-18 20:17 UTC (permalink / raw)
To: Kirill Tkhai; +Cc: Linux Kernel Network Developers
In-Reply-To: <be6bbcbd-0817-691a-78b8-74b9b5b0a8b0@virtuozzo.com>
On Mon, Sep 17, 2018 at 12:25 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> In inet_init() the order of registration is:
>
> ip_mr_init();
> init_inet_pernet_ops();
>
> This means, ipmr_net_ops pernet operations are before af_inet_ops
> in pernet_list. So, there is a theoretical probability, sometimes
> in the future, we will have a problem during a fail of net initialization.
>
> Say,
>
> setup_net():
> ipmr_net_ops->init() returns 0
> xxx->init() returns error
> and then we do:
> ipmr_net_ops->exit(),
>
> which could touch ra_mutex (theoretically).
How could ra_mutex be touched in this scenario?
ra_mutex is only used in ip_ra_control() which is called
only by {get,set}sockopt(). I don't see anything related
to netns exit() path here.
^ permalink raw reply
* [PATCH bpf-next] flow_dissector: fix build failure without CONFIG_NET
From: Willem de Bruijn @ 2018-09-18 20:20 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, rdunlap, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
If boolean CONFIG_BPF_SYSCALL is enabled, kernel/bpf/syscall.c will
call flow_dissector functions from net/core/flow_dissector.c.
This causes this build failure if CONFIG_NET is disabled:
kernel/bpf/syscall.o: In function `__x64_sys_bpf':
syscall.c:(.text+0x3278): undefined reference to
`skb_flow_dissector_bpf_prog_attach'
syscall.c:(.text+0x3310): undefined reference to
`skb_flow_dissector_bpf_prog_detach'
kernel/bpf/syscall.o:(.rodata+0x3f0): undefined reference to
`flow_dissector_prog_ops'
kernel/bpf/verifier.o:(.rodata+0x250): undefined reference to
`flow_dissector_verifier_ops'
Analogous to other optional BPF program types in syscall.c, add stubs
if the relevant functions are not compiled and move the BPF_PROG_TYPE
definition in the #ifdef CONFIG_NET block.
Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/linux/bpf_types.h | 2 +-
include/linux/skbuff.h | 13 +++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 22083712dd18..c9bd6fb765b0 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -16,6 +16,7 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_SEG6LOCAL, lwt_seg6local)
BPF_PROG_TYPE(BPF_PROG_TYPE_SOCK_OPS, sock_ops)
BPF_PROG_TYPE(BPF_PROG_TYPE_SK_SKB, sk_skb)
BPF_PROG_TYPE(BPF_PROG_TYPE_SK_MSG, sk_msg)
+BPF_PROG_TYPE(BPF_PROG_TYPE_FLOW_DISSECTOR, flow_dissector)
#endif
#ifdef CONFIG_BPF_EVENTS
BPF_PROG_TYPE(BPF_PROG_TYPE_KPROBE, kprobe)
@@ -32,7 +33,6 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
#ifdef CONFIG_INET
BPF_PROG_TYPE(BPF_PROG_TYPE_SK_REUSEPORT, sk_reuseport)
#endif
-BPF_PROG_TYPE(BPF_PROG_TYPE_FLOW_DISSECTOR, flow_dissector)
BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ce0e863f02a2..76be85ea392a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1194,10 +1194,23 @@ void skb_flow_dissector_init(struct flow_dissector *flow_dissector,
const struct flow_dissector_key *key,
unsigned int key_count);
+#ifdef CONFIG_NET
int skb_flow_dissector_bpf_prog_attach(const union bpf_attr *attr,
struct bpf_prog *prog);
int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr);
+#else
+static inline int skb_flow_dissector_bpf_prog_attach(const union bpf_attr *attr,
+ struct bpf_prog *prog)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr)
+{
+ return -EOPNOTSUPP;
+}
+#endif
bool __skb_flow_dissect(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
--
2.19.0.397.gdd90340f6a-goog
^ permalink raw reply related
* Re: [PATCH net-next v5 20/20] net: WireGuard secure network tunnel
From: Jason A. Donenfeld @ 2018-09-19 2:04 UTC (permalink / raw)
To: Andrew Lunn
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <20180918233411.GB17466@lunn.ch>
Hi Andrew,
On Wed, Sep 19, 2018 at 1:34 AM Andrew Lunn <andrew@lunn.ch> wrote:
> I see this BUG_ON() is still here. It really needs to be removed. It
> does not look like you need to crash the kernel here. Can you add in a
> test of len >= 128, do a WARN and then return. I think you then leak
> some memory, but i would much prefer that to a crashed machine.
Sure, I'll change it to that.
Regards,
Jason
^ permalink raw reply
* Re: pegged softirq and NAPI race (?)
From: Song Liu @ 2018-09-18 20:37 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexei Starovoitov, netdev, Jeff Kirsher, Alexander Duyck,
michael.chan@broadcom.com, Kernel Team
In-Reply-To: <CANn89iJTEMjYepKbr-8pmk0i03d9D+CfDFLPx+J=fqZivDJ9zQ@mail.gmail.com>
> On Sep 18, 2018, at 11:17 AM, Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Sep 18, 2018 at 10:51 AM Alexei Starovoitov <ast@fb.com> wrote:
>>
>> On 9/18/18 6:45 AM, Eric Dumazet wrote:
>>> On Tue, Sep 18, 2018 at 1:41 AM Song Liu <songliubraving@fb.com> wrote:
>>>>
>>>> We are debugging this issue that netconsole message triggers pegged softirq
>>>> (ksoftirqd taking 100% CPU for many seconds). We found this issue in
>>>> production with both bnxt and ixgbe, on a 4.11 based kernel. This is easily
>>>> reproducible with ixgbe on 4.11, and latest net/net-next (see [1] for more
>>>> detail).
>>>>
>>>> After debugging for some time, we found that this issue is likely related
>>>> to 39e6c8208d7b ("net: solve a NAPI race"). After reverting this commit,
>>>> the steps described in [1] cannot reproduce the issue on ixgbe. Reverting
>>>> this commit also reduces the chances we hit the issue with bnxt (it still
>>>> happens with a lower rate).
>>>>
>>>> I tried to fix this issue with relaxed variant (or older version) of
>>>> napi_schedule_prep() in netpoll, just like the one on napi_watchdog().
>>>> However, my tests do not always go as expected.
>>>>
>>>> Please share your comments/suggestions on which direction shall we try
>>>> to fix this.
>>>>
>>>> Thanks in advance!
>>>> Song
>>>>
>>>>
>>>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_netdev_msg522328.html&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=i6WobKxbeG3slzHSIOxTVtYIJw7qjCE6S0spDTKL-J4&m=iSaOapj1kxjhGYLgQr0Qd8mQCzVdobmgT1L4JwFvzxs&s=lCEhrz6wQJUUaJOkxFmtOszAgkf3Jh4reX_i1GbI5RI&e=
>>>
>>> You have not traced ixgbe to understand why driver hits
>>> "clean_complete=false" all the time ?
>>
>> Eric,
>>
>> I'm looking at commit 39e6c8208d7b and wondering that it's doing
>> clear_bit(NAPI_STATE_MISSED,..);
>> for busy_poll_stop(), but not for netpoll.
>> Can that be an issue?
>>
>> and then something like below is needed:
>> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
>> index 57557a6a950c..a848be6b503c 100644
>> --- a/net/core/netpoll.c
>> +++ b/net/core/netpoll.c
>> @@ -172,6 +172,7 @@ static void poll_one_napi(struct napi_struct *napi)
>> trace_napi_poll(napi, work, 0);
>>
>> clear_bit(NAPI_STATE_NPSVC, &napi->state);
>> + clear_bit(NAPI_STATE_MISSED, &napi->state);
>> }
>
>
> NAPI_STATE_MISSED should only be cleared under strict circumstances.
>
> The clear in busy_poll_stop() is an optimization really (as explained
> in the comment)
>
> It is cleared when napi_complete_done() is eventually called, but if
> ixgbe always handle 64 RX frames in its poll function,
> napi_complete_done() will not be called. The bug is in ixgbe,
> pretending its poll function should be called forever.
Looks like a patch like the following fixes the issue for ixgbe. But I
cannot explain it yet.
Does this ring a bell?
Thanks,
Song
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 787c84fb20dd..51611f799dae 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3059,11 +3059,14 @@ static irqreturn_t ixgbe_msix_other(int irq, void *data)
static irqreturn_t ixgbe_msix_clean_rings(int irq, void *data)
{
struct ixgbe_q_vector *q_vector = data;
+ struct napi_struct *napi = &q_vector->napi;
/* EIAM disabled interrupts (on this vector) for us */
- if (q_vector->rx.ring || q_vector->tx.ring)
- napi_schedule_irqoff(&q_vector->napi);
+ if ((q_vector->rx.ring || q_vector->tx.ring) &&
+ !napi_disable_pending(napi) &&
+ !test_and_set_bit(NAPI_STATE_SCHED, &napi->state))
+ __napi_schedule_irqoff(napi);
return IRQ_HANDLED;
}
^ permalink raw reply related
* Re: [PATCH net-next v5 04/20] zinc: ChaCha20 x86_64 implementation
From: Jason A. Donenfeld @ 2018-09-19 2:14 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson, Andy Polyakov, mingo, X86 ML
In-Reply-To: <alpine.DEB.2.21.1809190017050.1468@nanos.tec.linutronix.de>
Hi Thomas,
On Wed, Sep 19, 2018 at 12:30 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> I'm a bit confused by this SOB chain. So above you write that it's from
> Andy Polakovs implementation and Samuel did the changes. But here it seems
> you are the main author. If Samuel just did some modifications then you
> want to use the Co-developed-by tag along with his SOB.
Thanks, I'll use that tag.
>
> Also I'd recommend to add a Originally-by or Based-on-code-from: Andy
> Polyakov tag. Both are not formal tags but widely in use for attributions.
Great idea.
>
> > +++ b/lib/zinc/chacha20/chacha20-x86_64-glue.h
> > @@ -0,0 +1,100 @@
> > +/* SPDX-License-Identifier: MIT
>
> Please put that into a separate one liner comment. Also this should be
> 'GPL-2.0[+] or MIT' I think.
I had that originally, but changed it to just MIT, since MIT is a
subset of GPL-2.0. And looking at tree repo, it appears this is what
others do too.
Jason
^ permalink raw reply
* Re: [bpf-next, v4 1/5] flow_dissector: implements flow dissector BPF hook
From: Willem de Bruijn @ 2018-09-18 20:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Petar Penkov, Network Development, David Miller,
Alexei Starovoitov, Daniel Borkmann, simon.horman, ecree,
songliubraving, Tom Herbert, Petar Penkov, Willem de Bruijn
In-Reply-To: <57de8ffd-e832-27b8-dede-f9ef76306470@gmail.com>
On Tue, Sep 18, 2018 at 2:09 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/14/2018 07:46 AM, Petar Penkov wrote:
> > From: Petar Penkov <ppenkov@google.com>
> >
> > Adds a hook for programs of type BPF_PROG_TYPE_FLOW_DISSECTOR and
> > attach type BPF_FLOW_DISSECTOR that is executed in the flow dissector
> > path. The BPF program is per-network namespace
>
> ...
>
> >
> > + rcu_read_lock();
> > + attached = skb ? rcu_dereference(dev_net(skb->dev)->flow_dissector_prog)
> > + : NULL;
>
>
> Some skbs have a NULL skb->dev, so we are going to crash here.
Ai. Thanks, Eric.
For these cases, we can probably infer the netns from sk->sk_net. That
is assuming that all skbs have at least one of skb->sk or skb->dev
set.
^ permalink raw reply
* [PATCH net 0/2] ipv6: fix issues on accessing fib6_metrics
From: Wei Wang @ 2018-09-18 20:44 UTC (permalink / raw)
To: David Miller, netdev
Cc: Eric Dumazet, David Ahern, Cong Wang, Sabrina Dubroca, Wei Wang
From: Wei Wang <weiwan@google.com>
The latest fix on the memory leak of fib6_metrics still causes
use-after-free.
This patch series first revert the previous fix and propose a new fix
that is more inline with ipv4 logic and is tested to fix the
use-after-free issue reported.
Wei Wang (2):
Revert "ipv6: fix double refcount of fib6_metrics"
ipv6: fix memory leak on dst->_metrics
net/ipv6/route.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
--
2.19.0.397.gdd90340f6a-goog
^ permalink raw reply
* [PATCH net 1/2] Revert "ipv6: fix double refcount of fib6_metrics"
From: Wei Wang @ 2018-09-18 20:44 UTC (permalink / raw)
To: David Miller, netdev
Cc: Eric Dumazet, David Ahern, Cong Wang, Sabrina Dubroca, Wei Wang
In-Reply-To: <20180918204500.106240-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
This reverts commit e70a3aad44cc8b24986687ffc98c4a4f6ecf25ea.
This change causes use-after-free on dst->_metrics.
The crash trace looks like this:
[ 97.763269] BUG: KASAN: use-after-free in ip6_mtu+0x116/0x140
[ 97.769038] Read of size 4 at addr ffff881781d2cf84 by task svw_NetThreadEv/8801
[ 97.777954] CPU: 76 PID: 8801 Comm: svw_NetThreadEv Not tainted 4.15.0-smp-DEV #11
[ 97.777956] Hardware name: Default string Default string/Indus_QC_02, BIOS 5.46.4 03/29/2018
[ 97.777957] Call Trace:
[ 97.777971] [<ffffffff895709db>] dump_stack+0x4d/0x72
[ 97.777985] [<ffffffff881651df>] print_address_description+0x6f/0x260
[ 97.777997] [<ffffffff88165747>] kasan_report+0x257/0x370
[ 97.778001] [<ffffffff894488e6>] ? ip6_mtu+0x116/0x140
[ 97.778004] [<ffffffff881658b9>] __asan_report_load4_noabort+0x19/0x20
[ 97.778008] [<ffffffff894488e6>] ip6_mtu+0x116/0x140
[ 97.778013] [<ffffffff892bb91e>] tcp_current_mss+0x12e/0x280
[ 97.778016] [<ffffffff892bb7f0>] ? tcp_mtu_to_mss+0x2d0/0x2d0
[ 97.778022] [<ffffffff887b45b8>] ? depot_save_stack+0x138/0x4a0
[ 97.778037] [<ffffffff87c38985>] ? __mmdrop+0x145/0x1f0
[ 97.778040] [<ffffffff881643b1>] ? save_stack+0xb1/0xd0
[ 97.778046] [<ffffffff89264c82>] tcp_send_mss+0x22/0x220
[ 97.778059] [<ffffffff89273a49>] tcp_sendmsg_locked+0x4f9/0x39f0
[ 97.778062] [<ffffffff881642b4>] ? kasan_check_write+0x14/0x20
[ 97.778066] [<ffffffff89273550>] ? tcp_sendpage+0x60/0x60
[ 97.778070] [<ffffffff881cb359>] ? rw_copy_check_uvector+0x69/0x280
[ 97.778075] [<ffffffff8873c65f>] ? import_iovec+0x9f/0x430
[ 97.778078] [<ffffffff88164be7>] ? kasan_slab_free+0x87/0xc0
[ 97.778082] [<ffffffff8873c5c0>] ? memzero_page+0x140/0x140
[ 97.778085] [<ffffffff881642b4>] ? kasan_check_write+0x14/0x20
[ 97.778088] [<ffffffff89276f6c>] tcp_sendmsg+0x2c/0x50
[ 97.778092] [<ffffffff89276f6c>] ? tcp_sendmsg+0x2c/0x50
[ 97.778098] [<ffffffff89352d43>] inet_sendmsg+0x103/0x480
[ 97.778102] [<ffffffff89352c40>] ? inet_gso_segment+0x15b0/0x15b0
[ 97.778105] [<ffffffff890294da>] sock_sendmsg+0xba/0xf0
[ 97.778108] [<ffffffff8902ab6a>] ___sys_sendmsg+0x6ca/0x8e0
[ 97.778113] [<ffffffff87dccac1>] ? hrtimer_try_to_cancel+0x71/0x3b0
[ 97.778116] [<ffffffff8902a4a0>] ? copy_msghdr_from_user+0x3d0/0x3d0
[ 97.778119] [<ffffffff881646d1>] ? memset+0x31/0x40
[ 97.778123] [<ffffffff87a0cff5>] ? schedule_hrtimeout_range_clock+0x165/0x380
[ 97.778127] [<ffffffff87a0ce90>] ? hrtimer_nanosleep_restart+0x250/0x250
[ 97.778130] [<ffffffff87dcc700>] ? __hrtimer_init+0x180/0x180
[ 97.778133] [<ffffffff87dd1f82>] ? ktime_get_ts64+0x172/0x200
[ 97.778137] [<ffffffff8822b8ec>] ? __fget_light+0x8c/0x2f0
[ 97.778141] [<ffffffff8902d5c6>] __sys_sendmsg+0xe6/0x190
[ 97.778144] [<ffffffff8902d5c6>] ? __sys_sendmsg+0xe6/0x190
[ 97.778147] [<ffffffff8902d4e0>] ? SyS_shutdown+0x20/0x20
[ 97.778152] [<ffffffff87cd4370>] ? wake_up_q+0xe0/0xe0
[ 97.778155] [<ffffffff8902d670>] ? __sys_sendmsg+0x190/0x190
[ 97.778158] [<ffffffff8902d683>] SyS_sendmsg+0x13/0x20
[ 97.778162] [<ffffffff87a1600c>] do_syscall_64+0x2ac/0x430
[ 97.778166] [<ffffffff87c17515>] ? do_page_fault+0x35/0x3d0
[ 97.778171] [<ffffffff8960131f>] ? page_fault+0x2f/0x50
[ 97.778174] [<ffffffff89600071>] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[ 97.778177] RIP: 0033:0x7f83fa36000d
[ 97.778178] RSP: 002b:00007f83ef9229e0 EFLAGS: 00000293 ORIG_RAX: 000000000000002e
[ 97.778180] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007f83fa36000d
[ 97.778182] RDX: 0000000000004000 RSI: 00007f83ef922f00 RDI: 0000000000000036
[ 97.778183] RBP: 00007f83ef923040 R08: 00007f83ef9231f8 R09: 00007f83ef923168
[ 97.778184] R10: 0000000000000000 R11: 0000000000000293 R12: 00007f83f69c5b40
[ 97.778185] R13: 000000000000001c R14: 0000000000000001 R15: 0000000000004000
[ 97.779684] Allocated by task 5919:
[ 97.783185] save_stack+0x46/0xd0
[ 97.783187] kasan_kmalloc+0xad/0xe0
[ 97.783189] kmem_cache_alloc_trace+0xdf/0x580
[ 97.783190] ip6_convert_metrics.isra.79+0x7e/0x190
[ 97.783192] ip6_route_info_create+0x60a/0x2480
[ 97.783193] ip6_route_add+0x1d/0x80
[ 97.783195] inet6_rtm_newroute+0xdd/0xf0
[ 97.783198] rtnetlink_rcv_msg+0x641/0xb10
[ 97.783200] netlink_rcv_skb+0x27b/0x3e0
[ 97.783202] rtnetlink_rcv+0x15/0x20
[ 97.783203] netlink_unicast+0x4be/0x720
[ 97.783204] netlink_sendmsg+0x7bc/0xbf0
[ 97.783205] sock_sendmsg+0xba/0xf0
[ 97.783207] ___sys_sendmsg+0x6ca/0x8e0
[ 97.783208] __sys_sendmsg+0xe6/0x190
[ 97.783209] SyS_sendmsg+0x13/0x20
[ 97.783211] do_syscall_64+0x2ac/0x430
[ 97.783213] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[ 97.784709] Freed by task 0:
[ 97.785056] knetbase: Error: /proc/sys/net/core/txcs_enable does not exist
[ 97.794497] save_stack+0x46/0xd0
[ 97.794499] kasan_slab_free+0x71/0xc0
[ 97.794500] kfree+0x7c/0xf0
[ 97.794501] fib6_info_destroy_rcu+0x24f/0x310
[ 97.794504] rcu_process_callbacks+0x38b/0x1730
[ 97.794506] __do_softirq+0x1c8/0x5d0
Reported-by: John Sperbeck <jsperbeck@google.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/route.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 480a79f47c52..b5d3e6b294ab 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -976,6 +976,10 @@ static void rt6_set_from(struct rt6_info *rt, struct fib6_info *from)
rt->rt6i_flags &= ~RTF_EXPIRES;
rcu_assign_pointer(rt->from, from);
dst_init_metrics(&rt->dst, from->fib6_metrics->metrics, true);
+ if (from->fib6_metrics != &dst_default_metrics) {
+ rt->dst._metrics |= DST_METRICS_REFCOUNTED;
+ refcount_inc(&from->fib6_metrics->refcnt);
+ }
}
/* Caller must already hold reference to @ort */
--
2.19.0.397.gdd90340f6a-goog
^ permalink raw reply related
* [PATCH net 2/2] ipv6: fix memory leak on dst->_metrics
From: Wei Wang @ 2018-09-18 20:45 UTC (permalink / raw)
To: David Miller, netdev
Cc: Eric Dumazet, David Ahern, Cong Wang, Sabrina Dubroca, Wei Wang
In-Reply-To: <20180918204500.106240-2-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
When dst->_metrics and f6i->fib6_metrics share the same memory, both
take reference count on the dst_metrics structure. However, when dst is
destroyed, ip6_dst_destroy() only invokes dst_destroy_metrics_generic()
which does not take care of READONLY metrics and does not release refcnt.
This causes memory leak.
Similar to ipv4 logic, the fix is to properly release refcnt and free
the memory space pointed by dst->_metrics if refcnt becomes 0.
Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
Reported-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/route.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b5d3e6b294ab..826b14de7dbb 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -364,11 +364,14 @@ EXPORT_SYMBOL(ip6_dst_alloc);
static void ip6_dst_destroy(struct dst_entry *dst)
{
+ struct dst_metrics *p = (struct dst_metrics *)DST_METRICS_PTR(dst);
struct rt6_info *rt = (struct rt6_info *)dst;
struct fib6_info *from;
struct inet6_dev *idev;
- dst_destroy_metrics_generic(dst);
+ if (p != &dst_default_metrics && refcount_dec_and_test(&p->refcnt))
+ kfree(p);
+
rt6_uncached_list_del(rt);
idev = rt->rt6i_idev;
--
2.19.0.397.gdd90340f6a-goog
^ permalink raw reply related
* Re: pegged softirq and NAPI race (?)
From: Eric Dumazet @ 2018-09-18 21:13 UTC (permalink / raw)
To: songliubraving
Cc: Alexei Starovoitov, netdev, Jeff Kirsher, Alexander Duyck,
michael.chan, kernel-team
In-Reply-To: <0FD562CC-CDE9-43C8-9623-B42AC7A208C8@fb.com>
On Tue, Sep 18, 2018 at 1:37 PM Song Liu <songliubraving@fb.com> wrote:
>
> Looks like a patch like the following fixes the issue for ixgbe. But I
> cannot explain it yet.
>
> Does this ring a bell?
I dunno, it looks like the NIC is generating an interrupt while it should not,
and constantly sets NAPI_STATE_MISSED.
Or maybe we need to properly check napi_complete_done() return value.
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index d3e72d0f66ef428b08e4bd88508e05b734bc43a4..c4c565c982a98a5891603cedcdcb72dc1c401813
100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -1773,8 +1773,8 @@ ixgb_clean(struct napi_struct *napi, int budget)
ixgb_clean_rx_irq(adapter, &work_done, budget);
/* If budget not fully consumed, exit the polling mode */
- if (work_done < budget) {
- napi_complete_done(napi, work_done);
+ if (work_done < budget &&
+ napi_complete_done(napi, work_done)) {
if (!test_bit(__IXGB_DOWN, &adapter->flags))
ixgb_irq_enable(adapter);
}
^ permalink raw reply
* Re: [PATCH 0/2] NFC fixes for 4.19-final
From: David Miller @ 2018-09-19 2:54 UTC (permalink / raw)
To: gregkh
Cc: sameo, netdev, allen.pais, keescook, surenb, linux-wireless,
linux-kernel
In-Reply-To: <20180917135141.28328-1-gregkh@linuxfoundation.org>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Mon, 17 Sep 2018 15:51:39 +0200
> For some reason, these two NFC patches fell through the cracks. It
> looks like sometimes NFC patches go through the wireless tree, and
> other times, through the networking tree directly.
>
> I don't care which way they go, but they should get merged through some
> way, or, I can take them in my device tree, but that feels a bit odd to
> me.
I'll take these directly, thanks Greg.
^ permalink raw reply
* Re: pegged softirq and NAPI race (?)
From: Eric Dumazet @ 2018-09-18 21:21 UTC (permalink / raw)
To: Eric Dumazet, songliubraving
Cc: Alexei Starovoitov, netdev, Jeff Kirsher, Alexander Duyck,
michael.chan, kernel-team
In-Reply-To: <CANn89i+kg=tE9aRYoFb+9ws35CCJJQz9Yc9i=9w=vDj=Epvj=A@mail.gmail.com>
On 09/18/2018 02:13 PM, Eric Dumazet wrote:
> On Tue, Sep 18, 2018 at 1:37 PM Song Liu <songliubraving@fb.com> wrote:
>>
>
>> Looks like a patch like the following fixes the issue for ixgbe. But I
>> cannot explain it yet.
>>
>> Does this ring a bell?
>
> I dunno, it looks like the NIC is generating an interrupt while it should not,
> and constantly sets NAPI_STATE_MISSED.
>
> Or maybe we need to properly check napi_complete_done() return value.
>
> diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> index d3e72d0f66ef428b08e4bd88508e05b734bc43a4..c4c565c982a98a5891603cedcdcb72dc1c401813
> 100644
> --- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> +++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> @@ -1773,8 +1773,8 @@ ixgb_clean(struct napi_struct *napi, int budget)
> ixgb_clean_rx_irq(adapter, &work_done, budget);
>
> /* If budget not fully consumed, exit the polling mode */
> - if (work_done < budget) {
> - napi_complete_done(napi, work_done);
> + if (work_done < budget &&
> + napi_complete_done(napi, work_done)) {
> if (!test_bit(__IXGB_DOWN, &adapter->flags))
> ixgb_irq_enable(adapter);
> }
>
ixgbe patch would be :
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 604282f03d236e4358fc91e64d8ba00a9b36cb8c..80d00aecb6e3e3e950ce6309bfe3639953dd73d9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3196,12 +3196,12 @@ int ixgbe_poll(struct napi_struct *napi, int budget)
return budget;
/* all work done, exit the polling mode */
- napi_complete_done(napi, work_done);
- if (adapter->rx_itr_setting & 1)
- ixgbe_set_itr(q_vector);
- if (!test_bit(__IXGBE_DOWN, &adapter->state))
- ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector->v_idx));
-
+ if (napi_complete_done(napi, work_done)) {
+ if (adapter->rx_itr_setting & 1)
+ ixgbe_set_itr(q_vector);
+ if (!test_bit(__IXGBE_DOWN, &adapter->state))
+ ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector->v_idx));
+ }
return min(work_done, budget - 1);
}
^ permalink raw reply related
* Re: pegged softirq and NAPI race (?)
From: Song Liu @ 2018-09-18 21:21 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexei Starovoitov, netdev, Jeff Kirsher, Alexander Duyck,
michael.chan@broadcom.com, Kernel Team
In-Reply-To: <CANn89i+kg=tE9aRYoFb+9ws35CCJJQz9Yc9i=9w=vDj=Epvj=A@mail.gmail.com>
> On Sep 18, 2018, at 2:13 PM, Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Sep 18, 2018 at 1:37 PM Song Liu <songliubraving@fb.com> wrote:
>>
>
>> Looks like a patch like the following fixes the issue for ixgbe. But I
>> cannot explain it yet.
>>
>> Does this ring a bell?
>
> I dunno, it looks like the NIC is generating an interrupt while it should not,
> and constantly sets NAPI_STATE_MISSED.
>
> Or maybe we need to properly check napi_complete_done() return value.
>
> diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> index d3e72d0f66ef428b08e4bd88508e05b734bc43a4..c4c565c982a98a5891603cedcdcb72dc1c401813
> 100644
> --- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> +++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> @@ -1773,8 +1773,8 @@ ixgb_clean(struct napi_struct *napi, int budget)
> ixgb_clean_rx_irq(adapter, &work_done, budget);
>
> /* If budget not fully consumed, exit the polling mode */
> - if (work_done < budget) {
> - napi_complete_done(napi, work_done);
> + if (work_done < budget &&
> + napi_complete_done(napi, work_done)) {
> if (!test_bit(__IXGB_DOWN, &adapter->flags))
> ixgb_irq_enable(adapter);
> }
Thanks Eric!
I was looking at exactly this part. :) And it seems working!
I will run a bigger test and update shortly.
Best,
Song
^ permalink raw reply
* Re: pegged softirq and NAPI race (?)
From: Eric Dumazet @ 2018-09-18 21:25 UTC (permalink / raw)
To: songliubraving
Cc: Alexei Starovoitov, netdev, Jeff Kirsher, Alexander Duyck,
michael.chan, kernel-team
In-Reply-To: <117A22DB-A4CF-4FFB-AD08-A187E9F6EEB3@fb.com>
On Tue, Sep 18, 2018 at 2:22 PM Song Liu <songliubraving@fb.com> wrote:
>
> Thanks Eric!
>
> I was looking at exactly this part. :) And it seems working!
And this should also make busy polling a bit faster as well, avoiding
enabling/receiving interrupts in the busy loop.
>
> I will run a bigger test and update shortly.
^ permalink raw reply
* Re: pegged softirq and NAPI race (?)
From: Florian Fainelli @ 2018-09-18 21:25 UTC (permalink / raw)
To: Eric Dumazet, songliubraving
Cc: Alexei Starovoitov, netdev, Jeff Kirsher, Alexander Duyck,
michael.chan, kernel-team
In-Reply-To: <CANn89i+kg=tE9aRYoFb+9ws35CCJJQz9Yc9i=9w=vDj=Epvj=A@mail.gmail.com>
On 09/18/2018 02:13 PM, Eric Dumazet wrote:
> On Tue, Sep 18, 2018 at 1:37 PM Song Liu <songliubraving@fb.com> wrote:
>>
>
>> Looks like a patch like the following fixes the issue for ixgbe. But I
>> cannot explain it yet.
>>
>> Does this ring a bell?
>
> I dunno, it looks like the NIC is generating an interrupt while it should not,
> and constantly sets NAPI_STATE_MISSED.
>
> Or maybe we need to properly check napi_complete_done() return value.
>
> diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> index d3e72d0f66ef428b08e4bd88508e05b734bc43a4..c4c565c982a98a5891603cedcdcb72dc1c401813
> 100644
> --- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> +++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> @@ -1773,8 +1773,8 @@ ixgb_clean(struct napi_struct *napi, int budget)
> ixgb_clean_rx_irq(adapter, &work_done, budget);
>
> /* If budget not fully consumed, exit the polling mode */
> - if (work_done < budget) {
> - napi_complete_done(napi, work_done);
> + if (work_done < budget &&
> + napi_complete_done(napi, work_done)) {
> if (!test_bit(__IXGB_DOWN, &adapter->flags))
> ixgb_irq_enable(adapter);
> }
This would not be the only driver doing this unfortunately... should we
add a __must_check annotation to help catch those (mis)uses? Though that
could cause false positives for drivers using NAPI to clean their TX ring.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] net: hns3: fix return type of ndo_start_xmit function
From: David Miller @ 2018-09-19 3:01 UTC (permalink / raw)
To: yuehaibing; +Cc: yisen.zhuang, salil.mehta, linux-kernel, netdev
In-Reply-To: <20180918060943.25208-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 18 Sep 2018 14:09:43 +0800
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, also the implementation in this
> driver has returns 'netdev_tx_t' value, so just change the function
> return type to netdev_tx_t.
>
> Found by coccinelle.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: cavium: fix return type of ndo_start_xmit function
From: David Miller @ 2018-09-19 3:03 UTC (permalink / raw)
To: yuehaibing
Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
alexander.sverdlin, david.daney, linux-kernel, netdev
In-Reply-To: <20180918061905.25000-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 18 Sep 2018 14:19:05 +0800
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, so make sure the implementation in
> this driver has returns 'netdev_tx_t' value, and change the function
> return type to netdev_tx_t.
>
> Found by coccinelle.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: pegged softirq and NAPI race (?)
From: Eric Dumazet @ 2018-09-18 21:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: songliubraving, Alexei Starovoitov, netdev, Jeff Kirsher,
Alexander Duyck, michael.chan, kernel-team
In-Reply-To: <09e7e3f8-dee8-71ea-7e57-4d0c92dcf13b@gmail.com>
On Tue, Sep 18, 2018 at 2:25 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
> This would not be the only driver doing this unfortunately... should we
> add a __must_check annotation to help catch those (mis)uses? Though that
> could cause false positives for drivers using NAPI to clean their TX ring.
>
Well, before adding __must_check we would need to cook a (big) patch
series to change all occurrences.
Not clear why netpoll is the trigger ?
^ permalink raw reply
* Re: [PATCH net-next] net: ibm: fix return type of ndo_start_xmit function
From: David Miller @ 2018-09-19 3:03 UTC (permalink / raw)
To: yuehaibing
Cc: dougmill, benh, paulus, mpe, tlfalcon, jallen, ivan, chunkeey,
keescook, linux-kernel, netdev, linuxppc-dev
In-Reply-To: <20180918063547.25644-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 18 Sep 2018 14:35:47 +0800
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, so make sure the implementation in
> this driver has returns 'netdev_tx_t' value, and change the function
> return type to netdev_tx_t.
>
> Found by coccinelle.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox