* Re: [PATCH net-next] tcp: expose sk_state in tcp_retransmit_skb tracepoint
From: Yafang Shao @ 2018-09-24 12:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, David Miller, netdev, LKML
In-Reply-To: <375c389d-f391-5b72-bc52-9a82d61c6bd7@gmail.com>
On Mon, Sep 24, 2018 at 5:42 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 09/23/2018 12:49 PM, Yafang Shao wrote:
>> With sk_state, we can know whether this connection is in SYN_SENT state
>> or ESTBLISHED state.
>> The reason to distinguish between these two scenario is that the
>> retransmission in ESTABLISHED state always mean network congestion while
>> in SYN_SENT state it always mean server issue, i.e. the syn packet is
>> dropped due to syn backlog queue full.
>
> You mean, a packet drop on the remote peer ?
>
Yes, I mean drop on the remote peer.
> It could also be a packet drop in the network.
>
Yes of course.
> Your patch is good, but changelog is quite misleading.
Will modify the changelog.
Thanks
Yafang
^ permalink raw reply
* Re: [PATCH net] VSOCK: check sk state before receive
From: Jorgen S. Hansen @ 2018-09-24 7:00 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Stefan Hajnoczi, netdev@vger.kernel.org, David S. Miller
In-Reply-To: <20180922062744.GC24677@leo.usersys.redhat.com>
On Sep 22, 2018, at 8:27 AM, Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Fri, Sep 21, 2018 at 07:48:25AM +0000, Jorgen S. Hansen wrote:
>> Hi Hangbin,
>>
>> I finaly got to the bottom of this - the issue was indeed in the VMCI driver. The patch is posted here:
>>
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F9%2F21%2F326&data=02%7C01%7Cjhansen%40vmware.com%7C280a9c79e99248db3d1108d6205488de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636731944784367201&sdata=BEExXdB%2BF0SSh83Epa5fAuyoG4qn%2FAeM2J0wG2gM6GQ%3D&reserved=0
>>
>> I used your reproduce.log to test the fix. Thanks for discovering this issue.
>
> Hi Jorgen,
>
> Thanks for your patch. I built a test kernel with your fix, run my
> reproducer and syzkaller socket vnet test for a while. There is no such
> error. So I think your patch fixed this issue.
Great. Thanks a lot for trying out the patch.
> BTW, with FAULT_INJECTION enabled. I got another call trace:
The vhost_* stuff is for Virtio. Stefan would know better what is going on there.
> [ 251.166377] FAULT_INJECTION: forcing a failure.
> [ 251.178736] CPU: 15 PID: 10448 Comm: syz-executor7 Not tainted 4.19.0
> -rc4.syz.vnet+ #3
> [ 251.187577] Hardware name: Dell Inc. PowerEdge R730/0WCJNT, BIOS 2.1.5 04/11/2016
> [ 251.187578] Call Trace:
> [ 251.187586] dump_stack+0x8c/0xce
> [ 251.187594] should_fail+0x5dd/0x6b0
> [ 251.199932] ? fault_create_debugfs_attr+0x1d0/0x1d0
> [ 251.199937] __should_failslab+0xe8/0x120
> [ 251.199945] should_failslab+0xa/0x20
> [ 251.228430] kmem_cache_alloc_trace+0x43/0x1f0
> [ 251.233392] ? vhost_dev_set_owner+0x366/0x790 [vhost]
> [ 251.239129] vhost_dev_set_owner+0x366/0x790 [vhost]
> [ 251.244672] ? vhost_poll_wakeup+0xa0/0xa0 [vhost]
> [ 251.250018] ? kasan_unpoison_shadow+0x30/0x40
> [ 251.254978] ? vhost_worker+0x370/0x370 [vhost]
> [ 251.260035] ? kasan_kmalloc_large+0x71/0xe0
> [ 251.264799] ? kmalloc_order+0x54/0x60
> [ 251.268985] vhost_net_ioctl+0xc2e/0x14c0 [vhost_net]
> [ 251.274635] ? avc_ss_reset+0x150/0x150
> [ 251.278915] ? kstrtouint_from_user+0xe5/0x140
> [ 251.283876] ? handle_tx_kick+0x40/0x40 [vhost_net]
> [ 251.289320] ? save_stack+0x89/0xb0
> [ 251.293213] ? __kasan_slab_free+0x12e/0x180
> [ 251.297979] ? kmem_cache_free+0x7a/0x210
> [ 251.302452] ? putname+0xe2/0x120
> [ 251.306151] ? get_pid_task+0x6e/0x90
> [ 251.310238] ? proc_fail_nth_write+0x91/0x1c0
> [ 251.315100] ? map_files_get_link+0x3c0/0x3c0
> [ 251.319963] ? exit_robust_list+0x1c0/0x1c0
> [ 251.324633] ? __vfs_write+0xf7/0x6a0
> [ 251.328711] ? handle_tx_kick+0x40/0x40 [vhost_net]
> [ 251.334154] do_vfs_ioctl+0x1a5/0xfb0
> [ 251.338241] ? ioctl_preallocate+0x1c0/0x1c0
> [ 251.343009] ? selinux_file_ioctl+0x382/0x560
> [ 251.347872] ? selinux_capable+0x40/0x40
> [ 251.352250] ? __fget+0x211/0x2e0
> [ 251.355949] ? iterate_fd+0x1c0/0x1c0
> [ 251.360038] ? syscall_trace_enter+0x285/0xaa0
> [ 251.365011] ? security_file_ioctl+0x5d/0xb0
> [ 251.369776] ? selinux_capable+0x40/0x40
> [ 251.374153] ksys_ioctl+0x89/0xa0
> [ 251.377853] __x64_sys_ioctl+0x74/0xb0
> [ 251.382036] do_syscall_64+0xc3/0x390
> [ 251.386123] ? syscall_return_slowpath+0x14c/0x230
> [ 251.391473] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 251.397111] RIP: 0033:0x451b89
> [ 251.400519] Code: fc ff 48 81 c4 80 00 00 00 e9 f1 fe ff ff 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 0b 67 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> [ 251.421476] RSP: 002b:00007fc0d9673c48 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [ 251.429927] RAX: ffffffffffffffda RBX: 00007fc0d96746b4 RCX: 0000000000451b89
> [ 251.437889] RDX: 0000000000000000 RSI: 000000000000af01 RDI: 0000000000000003
> [ 251.445852] RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000
> [ 251.453815] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000004
> [ 251.461778] R13: 0000000000006450 R14: 00000000004d3090 R15: 00007fc0d9674700
>
> Thanks
> Hangbin
Thanks,
Jorgen
^ permalink raw reply
* Ping: [PATCH 0/3] xen-netback: hash mapping hanling adjustments
From: Jan Beulich @ 2018-09-24 7:43 UTC (permalink / raw)
To: davem; +Cc: Paul Durrant, Wei Liu, xen-devel, netdev
In-Reply-To: <20180911101623.3tuzvlyr7musocrg@zion.uk.xensource.com>
>>> On 11.09.18 at 12:16, <wei.liu2@citrix.com> wrote:
> On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
>> >>> On 28.08.18 at 16:54, wrote:
>> > First and foremost the fix for XSA-270. On top of that further changes
>> > which looked desirable to me while investigating that XSA.
>> >
>> > 1: fix input validation in xenvif_set_hash_mapping()
>> > 2: validate queue numbers in xenvif_set_hash_mapping()
>> > 3: handle page straddling in xenvif_set_hash_mapping()
>> >
>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> What is the way forward here? I've got R-b-s from Paul for all three
>> patches, and a minor change request on patch 2 from Wei. I'm not
>> really certain what to do in this case (hints appreciated), but could
>> at least the security fix (patch 1) be applied immediately?
>
> If you happen to resend, please make the adjustment; otherwise I'm fine
> with the patches as they are. I don't want to block useful things on
> cosmetic issues.
Dave? I notice none of the patches is in 4.19-rc5, not even the security
fix, the advisory for which had gone public over a month ago.
Jan
^ permalink raw reply
* Re: [PATCH net] xfrm: validate template mode
From: Steffen Klassert @ 2018-09-24 8:03 UTC (permalink / raw)
To: Sean Tranchetti; +Cc: netdev
In-Reply-To: <1537386896-5425-1-git-send-email-stranche@codeaurora.org>
On Wed, Sep 19, 2018 at 01:54:56PM -0600, Sean Tranchetti wrote:
> XFRM mode parameters passed as part of the user templates
> in the IP_XFRM_POLICY are never properly validated. Passing
> values other than valid XFRM modes can cause stack-out-of-bounds
> reads to occur later in the XFRM processing:
>
> [ 140.535608] ================================================================
> [ 140.543058] BUG: KASAN: stack-out-of-bounds in xfrm_state_find+0x17e4/0x1cc4
> [ 140.550306] Read of size 4 at addr ffffffc0238a7a58 by task repro/5148
> [ 140.557369]
> [ 140.558927] Call trace:
> [ 140.558936] dump_backtrace+0x0/0x388
> [ 140.558940] show_stack+0x24/0x30
> [ 140.558946] __dump_stack+0x24/0x2c
> [ 140.558949] dump_stack+0x8c/0xd0
> [ 140.558956] print_address_description+0x74/0x234
> [ 140.558960] kasan_report+0x240/0x264
> [ 140.558963] __asan_report_load4_noabort+0x2c/0x38
> [ 140.558967] xfrm_state_find+0x17e4/0x1cc4
> [ 140.558971] xfrm_resolve_and_create_bundle+0x40c/0x1fb8
> [ 140.558975] xfrm_lookup+0x238/0x1444
> [ 140.558977] xfrm_lookup_route+0x48/0x11c
> [ 140.558984] ip_route_output_flow+0x88/0xc4
> [ 140.558991] raw_sendmsg+0xa74/0x266c
> [ 140.558996] inet_sendmsg+0x258/0x3b0
> [ 140.559002] sock_sendmsg+0xbc/0xec
> [ 140.559005] SyS_sendto+0x3a8/0x5a8
> [ 140.559008] el0_svc_naked+0x34/0x38
> [ 140.559009]
> [ 140.592245] page dumped because: kasan: bad access detected
> [ 140.597981] page_owner info is not active (free page?)
> [ 140.603267]
> [ 140.653503] ================================================================
>
> Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
Applied to the ipsec tree, thanks a lot!
^ permalink raw reply
* Re: [PATCH 2/2] gpiolib: Fix array members of same chip processed separately
From: Linus Walleij @ 2018-09-24 8:13 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
David S. Miller, Dominik Brodowski, Greg KH, kishon,
Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Hartmut Knaack, Peter Meerwald, Jiri Slaby, Willy Tarreau,
Geert Uytterhoeven, Sebastien Bourdelin
In-Reply-To: <20180923235336.22148-3-jmkrzyszt@gmail.com>
On Mon, Sep 24, 2018 at 1:52 AM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> New code introduced by commit bf9346f5d47b ("gpiolib: Identify arrays
> matching GPIO hardware") forcibly tries to find an array member which
> has its array index number equal to its hardware pin number and set
> up an array info for possible fast bitmap processing of all arrray
> pins belonging to that chip which also satisfy that numbering rule.
>
> Depending on array content, it may happen that consecutive array
> members which belong to the same chip but don't have array indexes
> equal to their pin hardware numbers will be split into groups, some of
> them processed together via the fast bitmap path, and rest of them
> separetely. However, applications may expect all those pins being
> processed together with a single call to .set_multiple() chip callback,
> like that was done before the change.
>
> Limit applicability of fast bitmap processing path to cases where all
> pins of consecutive array members starting from 0 which belong to the
> same chip have their hardware numbers equal to their corresponding
> array indexes. That should still speed up processing of applications
> using whole GPIO banks as I/O ports, while not breaking simultaneous
> manipulation of consecutive pins of the same chip which don't follow
> the equal numbering rule.
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Patch applied!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] xsk: fix bug when trying to use both copy and zero-copy on one queue id
From: Magnus Karlsson @ 2018-09-24 8:37 UTC (permalink / raw)
To: jakub.kicinski
Cc: Y Song, Karlsson, Magnus, Björn Töpel, ast,
Daniel Borkmann, Network Development
In-Reply-To: <20180920074143.0add8cc4@cakuba.netronome.com>
On Thu, Sep 20, 2018 at 4:41 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Thu, 20 Sep 2018 11:17:17 +0200, Magnus Karlsson wrote:
> > On Wed, Sep 19, 2018 at 9:18 AM Magnus Karlsson wrote:
> > > On Wed, Sep 19, 2018 at 3:58 AM Jakub Kicinski wrote:
> > > >
> > > > On Tue, 18 Sep 2018 10:22:11 -0700, Y Song wrote:
> > > > > > +/* The umem is stored both in the _rx struct and the _tx struct as we do
> > > > > > + * not know if the device has more tx queues than rx, or the opposite.
> > > > > > + * This might also change during run time.
> > > > > > + */
> > > > > > +static void xdp_reg_umem_at_qid(struct net_device *dev, struct xdp_umem *umem,
> > > > > > + u16 queue_id)
> > > > > > +{
> > > > > > + if (queue_id < dev->real_num_rx_queues)
> > > > > > + dev->_rx[queue_id].umem = umem;
> > > > > > + if (queue_id < dev->real_num_tx_queues)
> > > > > > + dev->_tx[queue_id].umem = umem;
> > > > > > +}
> > > > > > +
> > > > > > +static struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev,
> > > > > > + u16 queue_id)
> > > > > > +{
> > > > > > + if (queue_id < dev->real_num_rx_queues)
> > > > > > + return dev->_rx[queue_id].umem;
> > > > > > + if (queue_id < dev->real_num_tx_queues)
> > > > > > + return dev->_tx[queue_id].umem;
> > > > > > +
> > > > > > + return NULL;
> > > > > > +}
> > > > > > +
> > > > > > +static void xdp_clear_umem_at_qid(struct net_device *dev, u16 queue_id)
> > > > > > +{
> > > > > > + /* Zero out the entry independent on how many queues are configured
> > > > > > + * at this point in time, as it might be used in the future.
> > > > > > + */
> > > > > > + if (queue_id < dev->num_rx_queues)
> > > > > > + dev->_rx[queue_id].umem = NULL;
> > > > > > + if (queue_id < dev->num_tx_queues)
> > > > > > + dev->_tx[queue_id].umem = NULL;
> > > > > > +}
> > > > > > +
> > > > >
> > > > > I am sure whether the following scenario can happen or not.
> > > > > Could you clarify?
> > > > > 1. suppose initially we have num_rx_queues = num_tx_queues = 10
> > > > > xdp_reg_umem_at_qid() set umem1 to queue_id = 8
> > > > > 2. num_tx_queues is changed to 5
> > > > > 3. xdp_clear_umem_at_qid() is called for queue_id = 8,
> > > > > and dev->_rx[8].umum = 0.
> > > > > 4. xdp_reg_umem_at_qid() is called gain to set for queue_id = 8
> > > > > dev->_rx[8].umem = umem2
> > > > > 5. num_tx_queues is changed to 10
> > > > > Now dev->_rx[8].umem != dev->_tx[8].umem, is this possible and is it
> > > > > a problem?
> > > >
> > > > Plus IIRC the check of qid vs real_num_[rt]x_queues in xsk_bind() is
> > > > not under rtnl_lock so it doesn't count for much. Why not do all the
> > > > checks against num_[rt]x_queues here, instead of real_..?
> > >
> > > You are correct, two separate rtnl_lock regions is broken. Will spin a
> > > v2 tomorrow when I am back in the office.
> > >
> > > Thanks Jakub for catching this. I really appreciate you reviewing my code.
> >
> > Sorry, forgot to answer your question about why real_num_ instead of
> > num_. If we used num_ instead, we would get a behavior where we can
> > open a socket on a queue id, let us say 10, that will not be active if
> > real_num_ is below 10. So no traffic will flow on this socket until we
> > issue an ethtool command to state that we have 10 or more queues
> > configured. While this behavior is sane (and consistent), I believe it
> > will lead to a more complicated driver implementation, break the
> > current uapi (since we will return an error if you try to bind to a
> > queue id that is not active at the moment) and I like my suggestion
> > below better :-).
> >
> > What I would like to suggest is that the current model at bind() is
> > kept, that is it will fail if you try to bind to a non-active queue
> > id. But we add some code in ethool_set_channels in net/core/ethtool.c
> > to check if you have an active af_xdp socket on a queue id and try to
> > make it inactive, we return an error and disallow the change. This
> > would IMHO be like not allowing a load module to be unloaded if
> > someone is using it or not allowing unmounting a file system if files
> > are in use. What do you think? If you think this is the right way to
> > go, I can implement this in a follow up patch or in this patch set if
> > that makes more sense, let me know. This suggestion would actually
> > make it simpler to implement zero-copy support in the driver. Some of
> > the complications we are having today is due to the fact that ethtool
> > can come in from the side and change things for us when an AF_XDP
> > socket is active on a queue id. And implementing zero copy support in
> > the driver needs to get simpler IMO.
> >
> > Please let me know what you think.
>
> I think I'd like that:
> https://patchwork.ozlabs.org/project/netdev/list/?series=57843&state=*
> in particular:
> https://patchwork.ozlabs.org/patch/949891/
> ;)
Perfect :-). I will drop the code I wrote to fix this and use yours in
my patch set. Please let me know if that does not work for you.
> If we fix the resize indeed you could stick to real_* but to be honest,
> I'm not entirely clear on what the advantage would be? The arrays are
> sized to num..queues, do you envision a use case where having
> xdp_get_umem_from_qid() return a NULL when asked for qid > real_..
> would help? Is there one for xdp_reg_umem_at_qid()
I just think that testing against real_ combined with not allowing to
remove queues that have an active af_xdp socket makes it consistent.
You cannot start an af_xdp socket on a non-active queue_id. You have
to make the queue active (increase real_num_ so it encompasses the
queue in question) first, then you can start your socket, and in order
to decrease real_num, you first have to remove the socket. AF_XDP
sockets are only on active queue ids. Should make the state handling
simpler, IMHO.
/Magnus
> I have no strong preference here, just wondering.
^ permalink raw reply
* Re: [PATCH net-next] dt-bindings: net: ravb: Add support for r8a7744 SoC
From: Simon Horman @ 2018-09-24 8:45 UTC (permalink / raw)
To: Biju Das
Cc: Sergei Shtylyov, Rob Herring, Mark Rutland, David S. Miller,
Geert Uytterhoeven, Fabrizio Castro, Jacopo Mondi,
Yoshihiro Shimoda, netdev@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org,
Chris Paterson
In-Reply-To: <OSBPR01MB21036782B7F7938C35D0D859B8120@OSBPR01MB2103.jpnprd01.prod.outlook.com>
On Fri, Sep 21, 2018 at 03:57:33PM +0000, Biju Das wrote:
> Hi Sergei,
>
> Thanks for the feedback.
>
> > Subject: Re: [PATCH net-next] dt-bindings: net: ravb: Add support for
> > r8a7744 SoC
> >
> > Hello!
> >
> > On 09/21/2018 05:25 PM, Biju Das wrote:
> >
> > > Document RZ/G1N (R8A7744) SoC bindings.
> > >
> > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >
> > > ---
> > > Documentation/devicetree/bindings/net/renesas,ravb.txt | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > > index da249b7..3530256 100644
> > > --- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > > +++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > > @@ -6,6 +6,7 @@ interface contains.
> > > Required properties:
> > > - compatible: Must contain one or more of the following:
> > > - "renesas,etheravb-r8a7743" for the R8A7743 SoC.
> > > + - "renesas,etheravb-r8a7744" for the R8A7744 SoC.
> >
> > Mmm, I thought it was decided to go with the full model #s, like with
> > R8A77470 below?
>
> For RZ/G1C case, there are 2 variants 1 with HDMI(R8A77470) and the other without HDMI(R8A77471).
> Current support is for R8A77470.
r8a7744 is consistent with other bindings for this SoC and my understanding
is that it is a) sufficient to differentiate this SoC and b) in keeping
with most other RZ/G1 SoCs (RZ/G1C being the exception).
In short, this seems fine to me.
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> > > - "renesas,etheravb-r8a7745" for the R8A7745 SoC.
> > > - "renesas,etheravb-r8a77470" for the R8A77470 SoC.
> > > - "renesas,etheravb-r8a7790" for the R8A7790 SoC.
^ permalink raw reply
* Re: [PATCH v2 net-next] ravb: remove tx buffer addr 4byte alilgnment restriction for R-Car Gen3
From: Simon Horman @ 2018-09-24 8:46 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: David Miller, Magnus Damm, netdev, linux-renesas-soc,
Kazuya Mizuguchi
In-Reply-To: <4b0144fc-d2f2-e687-eb15-4ad703f9c649@cogentembedded.com>
On Fri, Sep 21, 2018 at 07:14:02PM +0300, Sergei Shtylyov wrote:
> On 09/19/2018 11:06 AM, Simon Horman wrote:
>
> > From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> >
> > This patch sets from two descriptor to one descriptor because R-Car Gen3
> > does not have the 4 bytes alignment restriction of the transmission buffer.
> >
> > Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > v2 [Simon Horman]
> > * As per review by Sergi Shtylyov
>
> It's Sergei or (more correctly) Sergey. :-)
Sorry, that was completely unintentional.
>
> > - Use reverse xmas tree for variable declarations
> > - Use > rather than >= for conditions
> > - Dropped unnecessary parentheses
> > - Don't allocate memory for tx_align when it will not be used
> > - But, kept NUM_TX_DESC_GEN[23] as I see some value in
> > the self-documentation provided by these #defines
> [...]
>
> Better late than never (-:
>
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> MBR, Sergei
>
^ permalink raw reply
* [PATCH net] net: mvneta: fix the remaining Rx descriptor unmapping issues
From: Antoine Tenart @ 2018-09-24 14:56 UTC (permalink / raw)
To: davem, yelena
Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
stefanc, ymarkman, mw
With CONFIG_DMA_API_DEBUG enabled we get DMA unmapping warning in
various places of the mvneta driver, for example when putting down an
interface while traffic is passing through.
The issue is when using s/w buffer management, the Rx buffers are mapped
using dma_map_page but unmapped with dma_unmap_single. This patch fixes
this by using the right unmapping function.
Fixes: 562e2f467e71 ("net: mvneta: Improve the buffer allocation method for SWBM")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/marvell/mvneta.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 2db9708f2e24..b4ed7d394d07 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1890,8 +1890,8 @@ static void mvneta_rxq_drop_pkts(struct mvneta_port *pp,
if (!data || !(rx_desc->buf_phys_addr))
continue;
- dma_unmap_single(pp->dev->dev.parent, rx_desc->buf_phys_addr,
- MVNETA_RX_BUF_SIZE(pp->pkt_size), DMA_FROM_DEVICE);
+ dma_unmap_page(pp->dev->dev.parent, rx_desc->buf_phys_addr,
+ PAGE_SIZE, DMA_FROM_DEVICE);
__free_page(data);
}
}
@@ -2039,9 +2039,8 @@ static int mvneta_rx_swbm(struct napi_struct *napi,
frag_offset, frag_size,
PAGE_SIZE);
- dma_unmap_single(dev->dev.parent, phys_addr,
- PAGE_SIZE,
- DMA_FROM_DEVICE);
+ dma_unmap_page(dev->dev.parent, phys_addr,
+ PAGE_SIZE, DMA_FROM_DEVICE);
rxq->left_size -= frag_size;
}
--
2.17.1
^ permalink raw reply related
* [mainline][ppc][bnx2x] watchdog: CPU 80 self-detected hard LOCKUP @ opal_interrupt+0x28/0x70 when module load/unload
From: Abdul Haleem @ 2018-09-24 8:56 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-kernel, netdev, maurosr, mpe, manvanth, sim
Greeting's
bnx2x module load/unload test results in continuous hard LOCKUP trace on
my powerpc bare-metal running mainline 4.19.0-rc4 kernel
the instruction address points to:
0xc00000000009d048 is in opal_interrupt
(arch/powerpc/platforms/powernv/opal-irqchip.c:133).
128
129 static irqreturn_t opal_interrupt(int irq, void *data)
130 {
131 __be64 events;
132
133 opal_handle_interrupt(virq_to_hw(irq), &events);
134 last_outstanding_events = be64_to_cpu(events);
135 if (opal_have_pending_events())
136 opal_wake_poller();
137
trace:
bnx2x 0008:01:00.3 enP8p1s0f3: renamed from eth0
bnx2x 0008:01:00.3 enP8p1s0f3: using MSI-X IRQs: sp 297 fp[0] 299 ... fp[7] 306
bnx2x 0008:01:00.2 enP8p1s0f2: NIC Link is Up, 1000 Mbps full duplex, Flow control: none
bnx2x 0008:01:00.3 enP8p1s0f3: NIC Link is Up, 1000 Mbps full duplex, Flow control: none
bnx2x: QLogic 5771x/578xx 10/20-Gigabit Ethernet Driver bnx2x 1.712.30-0 (2014/02/10)
bnx2x 0008:01:00.0: msix capability found
bnx2x 0008:01:00.0: Using 64-bit DMA iommu bypass
bnx2x 0008:01:00.0: part number 0-0-0-0
bnx2x 0008:01:00.0: 32.000 Gb/s available PCIe bandwidth (5 GT/s x8 link)
bnx2x 0008:01:00.0 enP8p1s0f0: renamed from eth0
bnx2x 0008:01:00.1: msix capability found
bnx2x 0008:01:00.1: Using 64-bit DMA iommu bypass
bnx2x 0008:01:00.1: part number 0-0-0-0
bnx2x 0008:01:00.0 enP8p1s0f0: using MSI-X IRQs: sp 267 fp[0] 269 ... fp[7] 276
bnx2x 0008:01:00.0 enP8p1s0f0: NIC Link is Up, 10000 Mbps full duplex, Flow control: ON - receive & transmit
bnx2x 0008:01:00.1: 32.000 Gb/s available PCIe bandwidth (5 GT/s x8 link)
bnx2x 0008:01:00.1 enP8p1s0f1: renamed from eth0
bnx2x 0008:01:00.2: msix capability found
bnx2x 0008:01:00.2: Using 64-bit DMA iommu bypass
bnx2x 0008:01:00.2: part number 0-0-0-0
bnx2x 0008:01:00.1 enP8p1s0f1: using MSI-X IRQs: sp 277 fp[0] 279 ... fp[7] 286
bnx2x 0008:01:00.1 enP8p1s0f1: NIC Link is Up, 10000 Mbps full duplex, Flow control: ON - receive & transmit
watchdog: CPU 80 self-detected hard LOCKUP @ opal_interrupt+0x28/0x70
watchdog: CPU 80 TB:980794111093, last heartbeat TB:973959617200 (13348ms ago)
Modules linked in: bnx2x(+) iptable_mangle ipt_MASQUERADE iptable_nat
nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack nf_defrag_ipv4 ipt_REJECT
nf_reject_ipv4 xt_tcpudp tun bridge stp llc iptable_filter dm_mirror
dm_region_hash dm_log dm_service_time vmx_crypto powernv_rng rng_core
dm_multipath kvm_hv kvm binfmt_misc nfsd ip_tables x_tables autofs4 xfs
lpfc crc_t10dif crct10dif_generic nvme_fc nvme_fabrics mdio libcrc32c
nvme_core crct10dif_common [last unloaded: bnx2x]
CPU: 80 PID: 0 Comm: swapper/80 Not tainted 4.19.0-rc4-autotest-autotest #1
NIP: c00000000009d048 LR: c000000000092fd0 CTR: 0000000030032a00
REGS: c000003fff493d80 TRAP: 0900 Not tainted (4.19.0-rc4-autotest-autotest)
MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 48004042 XER: 00000000
CFAR: c000000000092fbc IRQMASK: 1
GPR00: 0000000030005128 c000003fff70f220 c0000000010ae500 0000000000000000
GPR04: 0000000048004042 c00000000009d048 9000000000009033 0000000000000090
GPR08: 0000000000000000 0000000000000000 c000000000092fe4 9000000000001003
GPR12: c000000000092fbc c000003fff7ff300 c000003c96c80c00 0000000000010000
GPR16: 0000000000000000 000000000000003c c000003c96c80800 c000003c96d00700
GPR20: 0000000000000001 0000000000000001 0000000000000002 0000000000000014
GPR24: c000001fe8741000 c000003fff70f330 0000000000000000 c000003ca947fb40
GPR28: 00000000092f47d0 0000000000000014 c000001fe8741000 c000001fe9860200
NIP [c00000000009d048] opal_interrupt+0x28/0x70
LR [c000000000092fd0] opal_return+0x14/0x48
Call Trace:
[c000003fff70f220] [c00000000009d048] opal_interrupt+0x28/0x70 (unreliable)
[c000003fff70f250] [c00000000016d890] __handle_irq_event_percpu+0x90/0x2d0
[c000003fff70f310] [c00000000016db00] handle_irq_event_percpu+0x30/0x90
[c000003fff70f350] [c00000000016dbc0] handle_irq_event+0x60/0xc0
[c000003fff70f380] [c000000000172d2c] handle_fasteoi_irq+0xbc/0x1f0
[c000003fff70f3b0] [c00000000016c084] generic_handle_irq+0x44/0x70
[c000003fff70f3d0] [c0000000000193cc] __do_irq+0x8c/0x200
[c000003fff70f440] [c000000000019640] do_IRQ+0x100/0x110
[c000003fff70f490] [c000000000008db8] hardware_interrupt_common+0x158/0x160
--- interrupt: 501 at fib_table_lookup+0xfc/0x600
LR = fib_validate_source+0x148/0x370
[c000003fff70f780] [0000000000000000] (null) (unreliable)
[c000003fff70f7e0] [c000000000959af8] fib_validate_source+0x148/0x370
[c000003fff70f8a0] [c0000000008fd664] ip_route_input_rcu+0x214/0x970
[c000003fff70f990] [c0000000008fdde0] ip_route_input_noref+0x20/0x30
[c000003fff70f9e0] [c000000000945e28] arp_process.constprop.14+0x3d8/0x8a0
[c000003fff70faf0] [c00000000089eb20] __netif_receive_skb_one_core+0x60/0x80
[c000003fff70fb30] [c0000000008a7d00] netif_receive_skb_internal+0x30/0x110
[c000003fff70fb70] [c0000000008a888c] napi_gro_receive+0x11c/0x1c0
[c000003fff70fbb0] [c000000000702afc] tg3_poll_work+0x5fc/0x1060
[c000003fff70fcb0] [c0000000007035b4] tg3_poll_msix+0x54/0x210
[c000003fff70fd00] [c0000000008a922c] net_rx_action+0x31c/0x470
[c000003fff70fe10] [c0000000009f5afc] __do_softirq+0x15c/0x3b4
[c000003fff70ff00] [c0000000000fddf0] irq_exit+0x100/0x120
[c000003fff70ff20] [c0000000000193d8] __do_irq+0x98/0x200
[c000003fff70ff90] [c00000000002af24] call_do_irq+0x14/0x24
[c000003ca947fa80] [c0000000000195d4] do_IRQ+0x94/0x110
[c000003ca947fad0] [c000000000008db8] hardware_interrupt_common+0x158/0x160
--- interrupt: 501 at replay_interrupt_return+0x0/0x4
LR = arch_local_irq_restore+0x84/0x90
[c000003ca947fdc0] [0000000000080000] 0x80000 (unreliable)
[c000003ca947fde0] [c000000000181f60] rcu_idle_exit+0xa0/0xd0
[c000003ca947fe30] [c000000000136d08] do_idle+0x1c8/0x3a0
[c000003ca947fec0] [c0000000001370b4] cpu_startup_entry+0x34/0x40
[c000003ca947fef0] [c0000000000467f4] start_secondary+0x4d4/0x520
[c000003ca947ff90] [c00000000000b270] start_secondary_prolog+0x10/0x14
Instruction dump:
60000000 60420000 3c4c0101 384214e0 7c0802a6 78630020 f8010010 f821ffd1
4bf7b901 60000000 38810020 4bff657d <60000000> 39010020 3d42ffed e94a5d28
watchdog: CPU 80 became unstuck TB:980802789270
CPU: 80 PID: 412 Comm: ksoftirqd/80 Not tainted 4.19.0-rc4-autotest-autotest #1
Call Trace:
[c000003ca96f7910] [c0000000009d4cec] dump_stack+0xb0/0xf4 (unreliable)
[c000003ca96f7950] [c00000000002f278] wd_smp_clear_cpu_pending+0x368/0x3f0
[c000003ca96f7a10] [c00000000002fa48] wd_timer_fn+0x78/0x3a0
[c000003ca96f7ad0] [c00000000018a3c0] call_timer_fn+0x50/0x1b0
[c000003ca96f7b50] [c00000000018a658] expire_timers+0x138/0x1e0
[c000003ca96f7bc0] [c00000000018a7c8] run_timer_softirq+0xc8/0x220
[c000003ca96f7c50] [c0000000009f5afc] __do_softirq+0x15c/0x3b4
[c000003ca96f7d40] [c0000000000fdab4] run_ksoftirqd+0x54/0x80
[c000003ca96f7d60] [c000000000126f10] smpboot_thread_fn+0x290/0x2a0
[c000003ca96f7dc0] [c0000000001215ac] kthread+0x15c/0x1a0
[c000003ca96f7e30] [c00000000000bdd4] ret_from_kernel_thread+0x5c/0x68
bnx2x 0008:01:00.2: 32.000 Gb/s available PCIe bandwidth (5 GT/s x8 link)
bnx2x 0008:01:00.2 enP8p1s0f2: renamed from eth0
bnx2x 0008:01:00.3: msix capability found
bnx2x 0008:01:00.3: Using 64-bit DMA iommu bypass
bnx2x 0008:01:00.3: part number 0-0-0-0
bnx2x 0008:01:00.3: 32.000 Gb/s available PCIe bandwidth (5 GT/s x8 link)
bnx2x 0008:01:00.3 enP8p1s0f3: renamed from eth0
--
Regard's
Abdul Haleem
IBM Linux Technology Centre
^ permalink raw reply
* Re: [PATCH net-next] dt-bindings: net: ravb: Add support for r8a7744 SoC
From: Geert Uytterhoeven @ 2018-09-24 9:26 UTC (permalink / raw)
To: Biju Das
Cc: Rob Herring, Mark Rutland, Sergei Shtylyov, David S. Miller,
Geert Uytterhoeven, Simon Horman, Fabrizio Castro, Jacopo Mondi,
Yoshihiro Shimoda, netdev, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Simon Horman, Chris Paterson
In-Reply-To: <1537539943-24712-1-git-send-email-biju.das@bp.renesas.com>
On Fri, Sep 21, 2018 at 4:32 PM Biju Das <biju.das@bp.renesas.com> wrote:
> Document RZ/G1N (R8A7744) SoC bindings.
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* pull-request: bpf 2018-09-24
From: Daniel Borkmann @ 2018-09-24 9:31 UTC (permalink / raw)
To: davem; +Cc: daniel, ast, netdev
Hi David,
The following pull-request contains BPF updates for your *net* tree.
The main changes are:
1) Several fixes for BPF sockmap to only allow sockets being attached in
ESTABLISHED state, from John.
2) Fix up the license to LGPL/BSD for the libc compat header which contains
fallback helpers that libbpf and bpftool is using, from Jakub.
Please consider pulling these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Thanks a lot!
----------------------------------------------------------------
The following changes since commit 69ba423d35c25ae06fa419ac1d4f6c4e2b74e412:
Merge branch 'ipv6-fix-issues-on-accessing-fib6_metrics' (2018-09-18 20:17:01 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
for you to fetch changes up to fad0c40fab14523ca381a30fef88b7c3266bef1c:
Merge branch 'bpf-sockmap-estab-fixes' (2018-09-22 02:46:43 +0200)
----------------------------------------------------------------
Daniel Borkmann (1):
Merge branch 'bpf-sockmap-estab-fixes'
Jakub Kicinski (1):
tools: bpf: fix license for a compat header file
John Fastabend (3):
bpf: sockmap only allow ESTABLISHED sock state
bpf: sockmap, fix transition through disconnect without close
bpf: test_maps, only support ESTABLISHED socks
kernel/bpf/sockmap.c | 91 +++++++++++++++++++++++++--------
tools/include/tools/libc_compat.h | 2 +-
tools/testing/selftests/bpf/test_maps.c | 10 ++--
3 files changed, 79 insertions(+), 24 deletions(-)
^ permalink raw reply
* Re: [PATCH net] net: mvneta: fix the remaining Rx descriptor unmapping issues
From: Gregory CLEMENT @ 2018-09-24 15:37 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, yelena, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180924145613.27307-1-antoine.tenart@bootlin.com>
Hi Antoine,
On lun., sept. 24 2018, Antoine Tenart <antoine.tenart@bootlin.com> wrote:
> With CONFIG_DMA_API_DEBUG enabled we get DMA unmapping warning in
> various places of the mvneta driver, for example when putting down an
> interface while traffic is passing through.
>
> The issue is when using s/w buffer management, the Rx buffers are mapped
> using dma_map_page but unmapped with dma_unmap_single. This patch fixes
> this by using the right unmapping function.
>
> Fixes: 562e2f467e71 ("net: mvneta: Improve the buffer allocation method for SWBM")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Given the clarification provided by Yelen, this patch looks good:
Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Thanks,
Gregory
> ---
> drivers/net/ethernet/marvell/mvneta.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 2db9708f2e24..b4ed7d394d07 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -1890,8 +1890,8 @@ static void mvneta_rxq_drop_pkts(struct mvneta_port *pp,
> if (!data || !(rx_desc->buf_phys_addr))
> continue;
>
> - dma_unmap_single(pp->dev->dev.parent, rx_desc->buf_phys_addr,
> - MVNETA_RX_BUF_SIZE(pp->pkt_size), DMA_FROM_DEVICE);
> + dma_unmap_page(pp->dev->dev.parent, rx_desc->buf_phys_addr,
> + PAGE_SIZE, DMA_FROM_DEVICE);
> __free_page(data);
> }
> }
> @@ -2039,9 +2039,8 @@ static int mvneta_rx_swbm(struct napi_struct *napi,
> frag_offset, frag_size,
> PAGE_SIZE);
>
> - dma_unmap_single(dev->dev.parent, phys_addr,
> - PAGE_SIZE,
> - DMA_FROM_DEVICE);
> + dma_unmap_page(dev->dev.parent, phys_addr,
> + PAGE_SIZE, DMA_FROM_DEVICE);
>
> rxq->left_size -= frag_size;
> }
> --
> 2.17.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
^ permalink raw reply
* Re: KMSAN: uninit-value in memcmp (2)
From: Dmitry Vyukov @ 2018-09-24 9:39 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Vladis Dronov, syzbot, syzkaller-bugs, David Miller, Eric Dumazet,
LKML, Networking, sunlianwen
In-Reply-To: <CAG_fn=WwmJONmipmqdZkNj8K0Z-ZRmEiCvG4qt_fHknGYQe5Cg@mail.gmail.com>
On Mon, Sep 24, 2018 at 8:53 AM, Alexander Potapenko <glider@google.com> wrote:
> On Mon, Sep 24, 2018 at 12:09 AM Vladis Dronov <vdronov@redhat.com> wrote:
>>
>> Hello, Dmirty,
>>
>> Thank you for the reply. Can we please, discuss this further?
> Hi Vladis,
>> > You can see on dashboard that the last crash
>> > for the second version (2) happened just few days ago. So this is a
>> > different bug.
> FWIW I've just double-checked that the reproducer provided by
> syzkaller in the original message still triggers the report from the
> original message in the latest KMSAN tree (which already contains the
> __hw_addr_add_ex() fix from April).
>> Well... yes and no. When I was looking at this bug (bug?id=088efeac32fd) I was looking
>> at the report at "2018/05/09 18:55" (https://syzkaller.appspot.com/text?tag=CrashReport&x=141b707b800000),
>> since it was the only report with a reproducer. This was my error.
>>
>> The error and the call trace in this report are:
>>
>> >>>
>> BUG: KMSAN: uninit-value in memcmp+0x119/0x180 lib/string.c:861
>> CPU: 0 PID: 38 Comm: kworker/0:1 Not tainted 4.17.0-rc3+ #88
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> Workqueue: ipv6_addrconf addrconf_dad_work
>> Call Trace:
>> __dump_stack lib/dump_stack.c:77 [inline]
>> dump_stack+0x185/0x1d0 lib/dump_stack.c:113
>> kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
>> __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
>> memcmp+0x119/0x180 lib/string.c:861
>> __hw_addr_add_ex net/core/dev_addr_lists.c:61 [inline]
>> __dev_mc_add+0x1fc/0x900 net/core/dev_addr_lists.c:670
>> dev_mc_add+0x6d/0x80 net/core/dev_addr_lists.c:687
>> igmp6_group_added+0x2db/0xa00 net/ipv6/mcast.c:662
>> ipv6_dev_mc_inc+0xe9e/0x1130 net/ipv6/mcast.c:914
>> addrconf_join_solict net/ipv6/addrconf.c:2103 [inline]
>> addrconf_dad_begin net/ipv6/addrconf.c:3853 [inline]
>> addrconf_dad_work+0x462/0x2a20 net/ipv6/addrconf.c:3979
>> process_one_work+0x12c6/0x1f60 kernel/workqueue.c:2145
>> worker_thread+0x113c/0x24f0 kernel/workqueue.c:2279
>> kthread+0x539/0x720 kernel/kthread.c:239
>> ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:412
>>
>> Local variable description: ----buf@igmp6_group_added
>> Variable was created at:
>> igmp6_group_added+0x4a/0xa00 net/ipv6/mcast.c:650
>> ipv6_dev_mc_inc+0xe9e/0x1130 net/ipv6/mcast.c:914
>> <<<
>>
>> It is the same like in bug?id=3887c0d99aecb27d085180c5222d245d08a30806
>> which, after some more test, made me believe these bugs are duplicate
>> and are fixed by the same commit.
>>
>> But let's look at another report at "2018/09/12 21:00"
>> (https://syzkaller.appspot.com/text?tag=CrashReport&x=14f99b71400000)
>> at the bug (bug?id=088efeac32fd), the one you've mentioned as
>> "the last crash for the second version (2) happened just few days ago".
>>
>> Its error and the call trace are completely different:
>>
>> >>>
>> BUG: KMSAN: uninit-value in memcmp+0x11d/0x180 lib/string.c:863
>> CPU: 0 PID: 6107 Comm: syz-executor4 Not tainted 4.19.0-rc3+ #45
>> 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+0x14b/0x190 lib/dump_stack.c:113
>> kmsan_report+0x183/0x2b0 mm/kmsan/kmsan.c:956
>> __msan_warning+0x70/0xc0 mm/kmsan/kmsan_instr.c:645
>> memcmp+0x11d/0x180 lib/string.c:863
>> dev_uc_add_excl+0x165/0x7b0 net/core/dev_addr_lists.c:464
>> ndo_dflt_fdb_add net/core/rtnetlink.c:3463 [inline]
>> rtnl_fdb_add+0x1081/0x1270 net/core/rtnetlink.c:3558
>> rtnetlink_rcv_msg+0xa0b/0x1530 net/core/rtnetlink.c:4715
>> netlink_rcv_skb+0x36e/0x5f0 net/netlink/af_netlink.c:2454
>> rtnetlink_rcv+0x50/0x60 net/core/rtnetlink.c:4733
>> netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
>> netlink_unicast+0x1638/0x1720 net/netlink/af_netlink.c:1343
>> netlink_sendmsg+0x1205/0x1290 net/netlink/af_netlink.c:1908
>> sock_sendmsg_nosec net/socket.c:621 [inline]
>> sock_sendmsg net/socket.c:631 [inline]
>> ...
>> Uninit was created at:
>> ...
>> slab_post_alloc_hook mm/slab.h:446 [inline]
>> slab_alloc_node mm/slub.c:2718 [inline]
>> __kmalloc_node_track_caller+0x9e7/0x1160 mm/slub.c:4351
>> __kmalloc_reserve net/core/skbuff.c:138 [inline]
>> __alloc_skb+0x2f5/0x9e0 net/core/skbuff.c:206
>> alloc_skb include/linux/skbuff.h:996 [inline]
>> netlink_alloc_large_skb net/netlink/af_netlink.c:1189 [inline]
>> netlink_sendmsg+0xb49/0x1290 net/netlink/af_netlink.c:1883
>> sock_sendmsg_nosec net/socket.c:621 [inline]
>> sock_sendmsg net/socket.c:631 [inline]
>> ___sys_sendmsg+0xe70/0x1290 net/socket.c:2114
>> <<<
>>
>> This is a different bug. How come these 2 different reports for 2 different
>> bugs have ended in the same syzkaller report (bug?id=088efeac32fd) ?
>
> I suspect this is because syzbot used the top stack frame as the
> report signature.
> There's a mechanism to ignore frames like memcmp() in the reports, not
> sure why didn't it work in this case (maybe it just wasn't in place at
> the time the report happened).
>> One bug is fixed by the "net: fix uninit-value in __hw_addr_add_ex()" commit,
>> the second one is not, but they are still in the same syzkaller report.
>>
>> This was the reason of my confusion. I'm not sure how to fix this. If it is possible,
>> probably we need to cancel/revoke "#syz fix: net: fix uninit-value in __hw_addr_add_ex()"
>> for this syzkaller report (bug?id=088efeac32fd). And then "split" it into 2 or
>> more different reports, but I'm not sure if this is possible.
>>
>> Probably, syzkaller needs to look deeper into the KMSAN reports to differentiate
>> KMSAN errors happening because of different reasons.
>>
>> > On Sun, Sep 23, 2018 at 6:02 PM, Vladis Dronov <vdronov@redhat.com> wrote:
>> > > #syz fix: net: fix uninit-value in __hw_addr_add_ex()
>> >
>> > Hi Vladis,
>> >
>> > This can be fixed with "net: fix uninit-value in __hw_addr_add_ex()".
>> > That commit landed in April, syzbot waited till the commit reached all
>> > tested trees, and then closed the bug.
>> > But the similar bug continued to happen, so syzbot created second
>> > version of this bug (2). You can see on dashboard that the last crash
>> > for the second version (2) happened just few days ago. So this is a
>> > different bug.
Precisely discriminating bugs (root causes) bases on crash text is
generally undecidable problem, even for humans. We even can have
literally equal crash texts, which are still different bugs. And we
can have significantly differently looking crash texts, which are
actually caused by the same root cause. syzbot extracts some
"identity" string for each crash and than uses that string to
discriminate crashes and sort them into bins. This identity string is
what you see in email subject and bug title on dashboard. This method
can have both false positives and false negatives, but works
reasonably well in most cases and looks like the best practical
option.
For this exact instance (memcmp) we actually improved the analysis
logic recently:
https://github.com/google/syzkaller/commit/0e29942f77715486995d996f80f82742812d75a2#diff-abe1515f011fad2659ff218f9eea9ae1
But this crash was analyzed and reported before the change. So if this
crash happens again it should be reported as "in __hw_addr_add_ex"
now.
Re __hw_addr_add_ex bug, as Alex noted the crash was detected _after_
the fixing commit went in. So it's something new and different and
can't be fixed by the older commit.
There are no general, single guideline as to what to do when several
different bugs glued together into a single bug. Fixing at least one
of them (any) in the context of the bug is good, fixing both is good
too. When/if a bug is closed, new occurrences of similar crashes (the
same identity string) will lead to creation of a new bug. So if we fix
only one and close the bug, eventually the second one will lead to a
new bug (won't be lost), now dedicated to this second crash.
Now syzbot thinks that this bug is fixed/closed:
https://syzkaller.appspot.com/bug?extid=d3402c47f680ff24b29c
There is specifically no "undo" functionality, because it's inherently
racy with creation of a new version of this bugs by new crashes. So if
of these crashes will happen again, syzbot will open new bugs (now
with better discriminated titles). We can wait for that. Or we can
submit new fixes without waiting for new syzbot bugs (adding
Reported-by to new commits referencing this bug should not do any
harm).
Hope this clarifies things a bit.
Thanks
^ permalink raw reply
* Re: [PATCH 0/2] gpiolib: Fix issues introduced by fast bitmap processing path
From: Marek Szyprowski @ 2018-09-24 9:43 UTC (permalink / raw)
To: Janusz Krzysztofik, Linus Walleij
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
David S. Miller, Dominik Brodowski, Greg Kroah-Hartman,
Kishon Vijay Abraham I, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
Jiri Slaby, Willy Tarreau, Geert Uytterhoeven
In-Reply-To: <20180923235336.22148-1-jmkrzyszt@gmail.com>
Hi Janusz,
On 2018-09-24 01:53, Janusz Krzysztofik wrote:
> While investigating possible reasons of GPIO fast bitmap processing
> related boot hang on Samsung Snow Chromebook, reported by Marek
> Szyprowski (thanks!), I've discovered one coding bug, addressed by
> PATCH 1/2 of this series, and one potential regression introduced at
> design level of the solution, hopefully fixed by PATCH 2/2. See
> commit messages for details.
>
> Janusz Krzysztofik (2):
> gpiolib: Fix missing updates of bitmap index
> gpiolib: Fix array members of same chip processed separately
>
> The fixes should resolve the boot hang observed by Marek, however the
> second change excludes that particular case from fast bitmap processing
> and restores the old behaviour.
I confirm, that the above 2 patches fixes boot issue on Samsung Snow
Chromebook with next-20180920.
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Hence, it is possible still another
> issue which have had an influence on that boot hang exists in the code.
> In order to fully verify the fix, it would have to be tested on a
> platform where an array of GPIO descriptors is used which starts from
> at least two consecutive pins of one GPIO chip in hardware order,
> starting ftom 0, followed by one or more pins belonging to other
> chip(s).
>
> In order to verify if separate calls to .set() chip callback for each
> pin instead of one call to .set_multiple() is actually the reason of
> boot hang on Samsung Snow Chromebook, the affected driver -
> drivers/mmc/core/pwrseq_simple.c - would have to be temporarily
> modified for testing purposes so it calls gpiod_set_value() for each
> pin instead of gpiod_set_array_value() for all of them. If that would
> also result in boot hang, we could be sure the issue was really the
> one addressed by the second fix. Marek, could you please try to
> perform such test?
Yes, I've just tested next-20180920 only with the first patch from this
patchset and the mentioned change to drivers/mmc/core/pwrseq_simple.c.
It boots fine, so indeed the issue is in handling of arrays of gpios.
Just to be sure I did it right, this is my change to the mentioned file:
diff --git a/drivers/mmc/core/pwrseq_simple.c
b/drivers/mmc/core/pwrseq_simple.c
index 7f882a2bb872..9397dc1f2e38 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -38,16 +38,11 @@ static void mmc_pwrseq_simple_set_gpios_value(struct
mmc_pwrseq_simple *pwrseq,
int value)
{
struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
+ int i;
- if (!IS_ERR(reset_gpios)) {
- DECLARE_BITMAP(values, BITS_PER_TYPE(value));
- int nvalues = reset_gpios->ndescs;
-
- values[0] = value;
-
- gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
- reset_gpios->info, values);
- }
+ if (!IS_ERR(reset_gpios))
+ for (i = 0; i < reset_gpios->ndescs; i++)
+ gpiod_set_value_cansleep(reset_gpios->desc[i], value);
}
static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply related
* [PATCH] qed: fix spelling mistake "toogle" -> "toggle"
From: Colin King @ 2018-09-24 15:51 UTC (permalink / raw)
To: Ariel Elior, everest-linux-l2, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Trivial fix to spelling mistake in DP_VERBOSE message
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/qlogic/qed/qed_rdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_rdma.c b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
index be941cfaa2d4..c71391b9c757 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
@@ -228,7 +228,7 @@ static int qed_rdma_alloc(struct qed_hwfn *p_hwfn,
num_cons, "Toggle");
if (rc) {
DP_VERBOSE(p_hwfn, QED_MSG_RDMA,
- "Failed to allocate toogle bits, rc = %d\n", rc);
+ "Failed to allocate toggle bits, rc = %d\n", rc);
goto free_cq_map;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next] net/tls: Fixed race condition in async encryption
From: Vakul Garg @ 2018-09-24 10:05 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, doronrk, Vakul Garg
On processors with multi-engine crypto accelerators, it is possible that
multiple records get encrypted in parallel and their encryption
completion is notified to different cpus in multicore processor. This
leads to the situation where tls_encrypt_done() starts executing in
parallel on different cores. In current implementation, encrypted
records are queued to tx_ready_list in tls_encrypt_done(). This requires
addition to linked list 'tx_ready_list' to be protected. As
tls_decrypt_done() could be executing in irq content, it is not possible
to protect linked list addition operation using a lock.
To fix the problem, we remove linked list addition operation from the
irq context. We do tx_ready_list addition/removal operation from
application context only and get rid of possible multiple access to
the linked list. Before starting encryption on the record, we add it to
the tail of tx_ready_list. To prevent tls_tx_records() from transmitting
it, we mark the record with a new flag 'tx_ready' in 'struct tls_rec'.
When record encryption gets completed, tls_encrypt_done() has to only
update the 'tx_ready' flag to true & linked list add operation is not
required.
The changed logic brings some other side benefits. Since the records
are always submitted in tls sequence number order for encryption, the
tx_ready_list always remains sorted and addition of new records to it
does not have to traverse the linked list.
Lastly, we renamed tx_ready_list in 'struct tls_sw_context_tx' to
'tx_list'. This is because now, the some of the records at the tail are
not ready to transmit.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
include/net/tls.h | 16 ++++-------
net/tls/tls_main.c | 4 +--
net/tls/tls_sw.c | 81 ++++++++++++++++++++----------------------------------
3 files changed, 37 insertions(+), 64 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index 3aa73e2d8823..1615fb5ea114 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -99,6 +99,7 @@ enum {
*/
struct tls_rec {
struct list_head list;
+ int tx_ready;
int tx_flags;
struct scatterlist sg_plaintext_data[MAX_SKB_FRAGS];
struct scatterlist sg_encrypted_data[MAX_SKB_FRAGS];
@@ -128,7 +129,7 @@ struct tls_sw_context_tx {
struct crypto_wait async_wait;
struct tx_work tx_work;
struct tls_rec *open_rec;
- struct list_head tx_ready_list;
+ struct list_head tx_list;
atomic_t encrypt_pending;
int async_notify;
@@ -220,7 +221,6 @@ struct tls_context {
struct scatterlist *partially_sent_record;
u16 partially_sent_offset;
- u64 tx_seq_number; /* Next TLS seqnum to be transmitted */
unsigned long flags;
bool in_tcp_sendpages;
@@ -341,21 +341,15 @@ static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx)
return tls_ctx->pending_open_record_frags;
}
-static inline bool is_tx_ready(struct tls_context *tls_ctx,
- struct tls_sw_context_tx *ctx)
+static inline bool is_tx_ready(struct tls_sw_context_tx *ctx)
{
struct tls_rec *rec;
- u64 seq;
- rec = list_first_entry(&ctx->tx_ready_list, struct tls_rec, list);
+ rec = list_first_entry(&ctx->tx_list, struct tls_rec, list);
if (!rec)
return false;
- seq = be64_to_cpup((const __be64 *)&rec->aad_space);
- if (seq == tls_ctx->tx_seq_number)
- return true;
- else
- return false;
+ return READ_ONCE(rec->tx_ready);
}
struct sk_buff *
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 06094de7a3d9..b428069a1b05 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -212,7 +212,7 @@ int tls_push_pending_closed_record(struct sock *sk,
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
if (tls_is_partially_sent_record(tls_ctx) ||
- !list_empty(&ctx->tx_ready_list))
+ !list_empty(&ctx->tx_list))
return tls_tx_records(sk, flags);
else
return tls_ctx->push_pending_record(sk, flags);
@@ -233,7 +233,7 @@ static void tls_write_space(struct sock *sk)
}
/* Schedule the transmission if tx list is ready */
- if (is_tx_ready(ctx, tx_ctx) && !sk->sk_write_pending) {
+ if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
/* Schedule the transmission */
if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask))
schedule_delayed_work(&tx_ctx->tx_work.work, 0);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index bcb24c498b84..d30d65bf0753 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -329,29 +329,6 @@ static void tls_free_both_sg(struct sock *sk)
&rec->sg_plaintext_size);
}
-static bool append_tx_ready_list(struct tls_context *tls_ctx,
- struct tls_sw_context_tx *ctx,
- struct tls_rec *enc_rec)
-{
- u64 new_seq = be64_to_cpup((const __be64 *)&enc_rec->aad_space);
- struct list_head *pos;
-
- /* Need to insert encrypted record in tx_ready_list sorted
- * as per sequence number. Traverse linked list from tail.
- */
- list_for_each_prev(pos, &ctx->tx_ready_list) {
- struct tls_rec *rec = (struct tls_rec *)pos;
- u64 seq = be64_to_cpup((const __be64 *)&rec->aad_space);
-
- if (new_seq > seq)
- break;
- }
-
- list_add((struct list_head *)&enc_rec->list, pos);
-
- return is_tx_ready(tls_ctx, ctx);
-}
-
int tls_tx_records(struct sock *sk, int flags)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
@@ -360,7 +337,7 @@ int tls_tx_records(struct sock *sk, int flags)
int tx_flags, rc = 0;
if (tls_is_partially_sent_record(tls_ctx)) {
- rec = list_first_entry(&ctx->tx_ready_list,
+ rec = list_first_entry(&ctx->tx_list,
struct tls_rec, list);
if (flags == -1)
@@ -373,18 +350,15 @@ int tls_tx_records(struct sock *sk, int flags)
goto tx_err;
/* Full record has been transmitted.
- * Remove the head of tx_ready_list
+ * Remove the head of tx_list
*/
- tls_ctx->tx_seq_number++;
list_del(&rec->list);
kfree(rec);
}
- /* Tx all ready records which have expected sequence number */
- list_for_each_entry_safe(rec, tmp, &ctx->tx_ready_list, list) {
- u64 seq = be64_to_cpup((const __be64 *)&rec->aad_space);
-
- if (seq == tls_ctx->tx_seq_number) {
+ /* Tx all ready records */
+ list_for_each_entry_safe(rec, tmp, &ctx->tx_list, list) {
+ if (READ_ONCE(rec->tx_ready)) {
if (flags == -1)
tx_flags = rec->tx_flags;
else
@@ -396,7 +370,6 @@ int tls_tx_records(struct sock *sk, int flags)
if (rc)
goto tx_err;
- tls_ctx->tx_seq_number++;
list_del(&rec->list);
kfree(rec);
} else {
@@ -446,9 +419,18 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err)
}
}
- /* Append the record in tx queue */
- if (rec)
- ready = append_tx_ready_list(tls_ctx, ctx, rec);
+ if (rec) {
+ struct tls_rec *first_rec;
+
+ /* Mark the record as ready for transmission */
+ smp_store_mb(rec->tx_ready, true);
+
+ /* If received record is at head of tx_list, schedule tx */
+ first_rec = list_first_entry(&ctx->tx_list,
+ struct tls_rec, list);
+ if (rec == first_rec)
+ ready = true;
+ }
pending = atomic_dec_return(&ctx->encrypt_pending);
@@ -484,6 +466,8 @@ static int tls_do_encryption(struct sock *sk,
aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
tls_encrypt_done, sk);
+ /* Add the record in tx_list */
+ list_add_tail((struct list_head *)&rec->list, &ctx->tx_list);
atomic_inc(&ctx->encrypt_pending);
rc = crypto_aead_encrypt(aead_req);
@@ -493,9 +477,12 @@ static int tls_do_encryption(struct sock *sk,
rec->sg_encrypted_data[0].length += tls_ctx->tx.prepend_size;
}
- /* Case of encryption failure */
- if (rc && rc != -EINPROGRESS)
+ if (!rc) {
+ WRITE_ONCE(rec->tx_ready, true);
+ } else if (rc != -EINPROGRESS) {
+ list_del(&rec->list);
return rc;
+ }
/* Unhook the record from context if encryption is not failure */
ctx->open_rec = NULL;
@@ -544,13 +531,7 @@ static int tls_push_record(struct sock *sk, int flags,
return rc;
}
- /* Put the record in tx_ready_list and start tx if permitted.
- * This happens only when encryption is not asynchronous.
- */
- if (append_tx_ready_list(tls_ctx, ctx, rec))
- return tls_tx_records(sk, flags);
-
- return 0;
+ return tls_tx_records(sk, flags);
}
static int tls_sw_push_pending_record(struct sock *sk, int flags)
@@ -1566,7 +1547,7 @@ void tls_sw_free_resources_tx(struct sock *sk)
/* Tx whatever records we can transmit and abandon the rest */
tls_tx_records(sk, -1);
- /* Free up un-sent records in tx_ready_list. First, free
+ /* Free up un-sent records in tx_list. First, free
* the partially sent record if any at head of tx_list.
*/
if (tls_ctx->partially_sent_record) {
@@ -1583,13 +1564,13 @@ void tls_sw_free_resources_tx(struct sock *sk)
tls_ctx->partially_sent_record = NULL;
- rec = list_first_entry(&ctx->tx_ready_list,
+ rec = list_first_entry(&ctx->tx_list,
struct tls_rec, list);
list_del(&rec->list);
kfree(rec);
}
- list_for_each_entry_safe(rec, tmp, &ctx->tx_ready_list, list) {
+ list_for_each_entry_safe(rec, tmp, &ctx->tx_list, list) {
free_sg(sk, rec->sg_encrypted_data,
&rec->sg_encrypted_num_elem,
&rec->sg_encrypted_size);
@@ -1633,7 +1614,7 @@ void tls_sw_free_resources_rx(struct sock *sk)
kfree(ctx);
}
-/* The work handler to transmitt the encrypted records in tx_ready_list */
+/* The work handler to transmitt the encrypted records in tx_list */
static void tx_work_handler(struct work_struct *work)
{
struct delayed_work *delayed_work = to_delayed_work(work);
@@ -1700,7 +1681,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
crypto_info = &ctx->crypto_send.info;
cctx = &ctx->tx;
aead = &sw_ctx_tx->aead_send;
- INIT_LIST_HEAD(&sw_ctx_tx->tx_ready_list);
+ INIT_LIST_HEAD(&sw_ctx_tx->tx_list);
INIT_DELAYED_WORK(&sw_ctx_tx->tx_work.work, tx_work_handler);
sw_ctx_tx->tx_work.sk = sk;
} else {
@@ -1789,8 +1770,6 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
sw_ctx_rx->sk_poll = sk->sk_socket->ops->poll;
strp_check_rcv(&sw_ctx_rx->strp);
- } else {
- ctx->tx_seq_number = be64_to_cpup((const __be64 *)rec_seq);
}
goto out;
--
2.13.6
^ permalink raw reply related
* [PATCH net-next] tls: Fixed uninitialised vars warning
From: Vakul Garg @ 2018-09-24 10:39 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, doronrk, Vakul Garg
In tls_sw_sendmsg() and tls_sw_sendpage(), it is possible that the
uninitialised variable 'ret' gets passed to sk_stream_error(). So
initialise local variable 'ret' to '0. The warnings were detected by
'smatch' tool.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index bcb24c498b84..102d84bdb2ab 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -709,7 +709,7 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
int record_room;
int num_zc = 0;
int orig_size;
- int ret;
+ int ret = 0;
if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL))
return -ENOTSUPP;
@@ -901,8 +901,8 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
int num_async = 0;
bool full_record;
int record_room;
+ int ret = 0;
bool eor;
- int ret;
if (flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL |
MSG_SENDPAGE_NOTLAST))
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next 0/2] net: mvpp2: Add txq to CPU mapping
From: David Miller @ 2018-09-24 17:01 UTC (permalink / raw)
To: maxime.chevallier
Cc: netdev, linux-kernel, antoine.tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180924091106.15094-1-maxime.chevallier@bootlin.com>
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Mon, 24 Sep 2018 11:11:04 +0200
> This short series adds XPS support to the mvpp2 driver, by mapping
> txqs and CPUs. This comes with a patch using round-robin scheduling
> for the HW to pick the next txq to transmit from, instead of the default
> fixed-priority scheduling.
Series applied, thanks Maxime.
^ permalink raw reply
* Re: [PATCH net] ip6_tunnel: be careful when accessing the inner header
From: Paolo Abeni @ 2018-09-24 11:03 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller,
Alexander Potapenko
In-Reply-To: <CAM_iQpX5ySGZqKTsaQO6WHaFLMqG6Q2ZAUT=ctGijL-7KU_1uA@mail.gmail.com>
On Fri, 2018-09-21 at 11:51 -0700, Cong Wang wrote:
> On Wed, Sep 19, 2018 at 6:04 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> > index 419960b0ba16..a0b6932c3afd 100644
> > --- a/net/ipv6/ip6_tunnel.c
> > +++ b/net/ipv6/ip6_tunnel.c
> > @@ -1234,7 +1234,7 @@ static inline int
> > ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
> > {
> > struct ip6_tnl *t = netdev_priv(dev);
> > - const struct iphdr *iph = ip_hdr(skb);
> > + const struct iphdr *iph;
> > int encap_limit = -1;
> > struct flowi6 fl6;
> > __u8 dsfield;
> > @@ -1242,6 +1242,11 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
> > u8 tproto;
> > int err;
> >
> > + /* ensure we can access the full inner ip header */
> > + if (!pskb_may_pull(skb, sizeof(struct iphdr)))
> > + return -1;
> > +
> > + iph = ip_hdr(skb);
>
> Hmm...
>
> How do IPv4 tunnels ensure they have the right inner header to access?
> ip_tunnel_xmit() uses skb_inner_network_header() to access inner header
> which doesn't have any check either AFAIK.
You are right, I think we need similar checks for ip_tunnel_xmit(),
too.
I'll try to cook a patch.
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: Make MLXSW_SP1_FWREV_MINOR a hard requirement
From: Petr Machata @ 2018-09-24 11:14 UTC (permalink / raw)
To: Ido Schimmel; +Cc: Andrew Lunn, Ido Schimmel, netdev, davem, jiri, mlxsw
In-Reply-To: <20180923222839.GA26312@splinter>
Ido Schimmel <idosch@idosch.org> writes:
> On Mon, Sep 24, 2018 at 12:04:17AM +0200, Andrew Lunn wrote:
>> But doesn't that mean you reflash the device with the minimum version,
>> when in fact there could be a much newer version in /lib/firmware?
>
> No, because we always enforce the latest version we post to
> linux-firmware. We try to keep firmware updates at a minimum, so if we
> decided to post a new version it's either because the driver now
> requires a feature from this version (which makes older versions
> incompatible) or because a critical bug was fixed in that version.
If you consider the case of an older driver and a recent FW update that
the driver didn't know about, then yes, such FW would be ignored. You
can flash it by hand of course (ethtool -f), as long as it's on the
right branch.
Petr
^ permalink raw reply
* [PATCH] wimax/i2400m: fix spelling mistake "unitialized" -> "uninitialized"
From: Colin King @ 2018-09-24 17:18 UTC (permalink / raw)
To: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Trivial fix to spelling mistake in ms_to_errno array of error messages
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wimax/i2400m/control.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/control.c b/drivers/net/wimax/i2400m/control.c
index 094cea775d0c..0475ad553707 100644
--- a/drivers/net/wimax/i2400m/control.c
+++ b/drivers/net/wimax/i2400m/control.c
@@ -257,7 +257,7 @@ static const struct
[I2400M_MS_ACCESSIBILITY_ERROR] = { "accesibility error", -EIO },
[I2400M_MS_BUSY] = { "busy", -EBUSY },
[I2400M_MS_CORRUPTED_TLV] = { "corrupted TLV", -EILSEQ },
- [I2400M_MS_UNINITIALIZED] = { "not unitialized", -EILSEQ },
+ [I2400M_MS_UNINITIALIZED] = { "not uninitialized", -EILSEQ },
[I2400M_MS_UNKNOWN_ERROR] = { "unknown error", -EIO },
[I2400M_MS_PRODUCTION_ERROR] = { "production error", -EIO },
[I2400M_MS_NO_RF] = { "no RF", -EIO },
--
2.17.1
^ permalink raw reply related
* [PATCH] net: qca_spi: Introduce write register verification
From: Stefan Wahren @ 2018-09-24 11:20 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, Stefan Wahren, Michael Heimpold
The SPI protocol for the QCA7000 doesn't have any fault detection.
In order to increase the drivers reliability in noisy environments,
we could implement a write verification inspired by the enc28j60.
This should avoid situations were the driver wrongly assumes the
receive interrupt is enabled and miss all incoming packets.
This function is disabled per default and can be controlled via module
parameter wr_verify.
Signed-off-by: Michael Heimpold <michael.heimpold@i2se.com>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/net/ethernet/qualcomm/qca_7k.c | 34 +++++++++++++++++++++++++++++--
drivers/net/ethernet/qualcomm/qca_7k.h | 2 +-
drivers/net/ethernet/qualcomm/qca_debug.c | 1 +
drivers/net/ethernet/qualcomm/qca_spi.c | 28 ++++++++++++++++++-------
drivers/net/ethernet/qualcomm/qca_spi.h | 1 +
5 files changed, 56 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c b/drivers/net/ethernet/qualcomm/qca_7k.c
index 6c8543f..4292c89 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k.c
@@ -81,8 +81,8 @@ qcaspi_read_register(struct qcaspi *qca, u16 reg, u16 *result)
return ret;
}
-int
-qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
+static int
+__qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
{
__be16 tx_data[2];
struct spi_transfer transfer[2];
@@ -117,3 +117,33 @@ qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
return ret;
}
+
+int
+qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value, int retry)
+{
+ int ret, i = 0;
+ u16 confirmed;
+
+ do {
+ ret = __qcaspi_write_register(qca, reg, value);
+ if (ret)
+ return ret;
+
+ if (!retry)
+ return 0;
+
+ ret = qcaspi_read_register(qca, reg, &confirmed);
+ if (ret)
+ return ret;
+
+ ret = confirmed != value;
+ if (!ret)
+ return 0;
+
+ i++;
+ qca->stats.write_verify_failed++;
+
+ } while (i <= retry);
+
+ return ret;
+}
diff --git a/drivers/net/ethernet/qualcomm/qca_7k.h b/drivers/net/ethernet/qualcomm/qca_7k.h
index 27124c2..356de8e 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k.h
@@ -66,6 +66,6 @@
void qcaspi_spi_error(struct qcaspi *qca);
int qcaspi_read_register(struct qcaspi *qca, u16 reg, u16 *result);
-int qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value);
+int qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value, int retry);
#endif /* _QCA_7K_H */
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c
index 51d89c8..a9f1bc0 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -60,6 +60,7 @@ static const char qcaspi_gstrings_stats[][ETH_GSTRING_LEN] = {
"Write buffer misses",
"Transmit ring full",
"SPI errors",
+ "Write verify errors",
};
#ifdef CONFIG_DEBUG_FS
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 66b775d..d531050 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -69,6 +69,12 @@ static int qcaspi_pluggable = QCASPI_PLUGGABLE_MIN;
module_param(qcaspi_pluggable, int, 0);
MODULE_PARM_DESC(qcaspi_pluggable, "Pluggable SPI connection (yes/no).");
+#define QCASPI_WRITE_VERIFY_MIN 0
+#define QCASPI_WRITE_VERIFY_MAX 3
+static int wr_verify = QCASPI_WRITE_VERIFY_MIN;
+module_param(wr_verify, int, 0);
+MODULE_PARM_DESC(wr_verify, "SPI register write verify trails. Use 0-3.");
+
#define QCASPI_TX_TIMEOUT (1 * HZ)
#define QCASPI_QCA7K_REBOOT_TIME_MS 1000
@@ -77,7 +83,7 @@ start_spi_intr_handling(struct qcaspi *qca, u16 *intr_cause)
{
*intr_cause = 0;
- qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0);
+ qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
qcaspi_read_register(qca, SPI_REG_INTR_CAUSE, intr_cause);
netdev_dbg(qca->net_dev, "interrupts: 0x%04x\n", *intr_cause);
}
@@ -90,8 +96,8 @@ end_spi_intr_handling(struct qcaspi *qca, u16 intr_cause)
SPI_INT_RDBUF_ERR |
SPI_INT_WRBUF_ERR);
- qcaspi_write_register(qca, SPI_REG_INTR_CAUSE, intr_cause);
- qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, intr_enable);
+ qcaspi_write_register(qca, SPI_REG_INTR_CAUSE, intr_cause, 0);
+ qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, intr_enable, wr_verify);
netdev_dbg(qca->net_dev, "acking int: 0x%04x\n", intr_cause);
}
@@ -239,7 +245,7 @@ qcaspi_tx_frame(struct qcaspi *qca, struct sk_buff *skb)
len = skb->len;
- qcaspi_write_register(qca, SPI_REG_BFR_SIZE, len);
+ qcaspi_write_register(qca, SPI_REG_BFR_SIZE, len, wr_verify);
if (qca->legacy_mode)
qcaspi_tx_cmd(qca, QCA7K_SPI_WRITE | QCA7K_SPI_EXTERNAL);
@@ -345,6 +351,7 @@ qcaspi_receive(struct qcaspi *qca)
/* Read the packet size. */
qcaspi_read_register(qca, SPI_REG_RDBUF_BYTE_AVA, &available);
+
netdev_dbg(net_dev, "qcaspi_receive: SPI_REG_RDBUF_BYTE_AVA: Value: %08x\n",
available);
@@ -353,7 +360,7 @@ qcaspi_receive(struct qcaspi *qca)
return -1;
}
- qcaspi_write_register(qca, SPI_REG_BFR_SIZE, available);
+ qcaspi_write_register(qca, SPI_REG_BFR_SIZE, available, wr_verify);
if (qca->legacy_mode)
qcaspi_tx_cmd(qca, QCA7K_SPI_READ | QCA7K_SPI_EXTERNAL);
@@ -524,7 +531,7 @@ qcaspi_qca7k_sync(struct qcaspi *qca, int event)
netdev_dbg(qca->net_dev, "sync: resetting device.\n");
qcaspi_read_register(qca, SPI_REG_SPI_CONFIG, &spi_config);
spi_config |= QCASPI_SLAVE_RESET_BIT;
- qcaspi_write_register(qca, SPI_REG_SPI_CONFIG, spi_config);
+ qcaspi_write_register(qca, SPI_REG_SPI_CONFIG, spi_config, 0);
qca->sync = QCASPI_SYNC_RESET;
qca->stats.trig_reset++;
@@ -684,7 +691,7 @@ qcaspi_netdev_close(struct net_device *dev)
netif_stop_queue(dev);
- qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0);
+ qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
free_irq(qca->spi_dev->irq, qca);
kthread_stop(qca->spi_thread);
@@ -904,6 +911,13 @@ qca_spi_probe(struct spi_device *spi)
return -EINVAL;
}
+ if (wr_verify < QCASPI_WRITE_VERIFY_MIN ||
+ wr_verify > QCASPI_WRITE_VERIFY_MAX) {
+ dev_err(&spi->dev, "Invalid write verify: %d\n",
+ wr_verify);
+ return -EINVAL;
+ }
+
dev_info(&spi->dev, "ver=%s, clkspeed=%d, burst_len=%d, pluggable=%d\n",
QCASPI_DRV_VERSION,
qcaspi_clkspeed,
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h b/drivers/net/ethernet/qualcomm/qca_spi.h
index fc0e987..2d2c497 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -73,6 +73,7 @@ struct qcaspi_stats {
u64 write_buf_miss;
u64 ring_full;
u64 spi_err;
+ u64 write_verify_failed;
};
struct qcaspi {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] wimax/i2400m: fix spelling mistake "unitialized" -> "uninitialized"
From: Sergei Shtylyov @ 2018-09-24 17:27 UTC (permalink / raw)
To: Colin King, Inaky Perez-Gonzalez, linux-wimax, David S . Miller,
netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180924171853.3294-1-colin.king@canonical.com>
On 09/24/2018 08:18 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in ms_to_errno array of error messages
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/wimax/i2400m/control.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wimax/i2400m/control.c b/drivers/net/wimax/i2400m/control.c
> index 094cea775d0c..0475ad553707 100644
> --- a/drivers/net/wimax/i2400m/control.c
> +++ b/drivers/net/wimax/i2400m/control.c
> @@ -257,7 +257,7 @@ static const struct
> [I2400M_MS_ACCESSIBILITY_ERROR] = { "accesibility error", -EIO },
> [I2400M_MS_BUSY] = { "busy", -EBUSY },
> [I2400M_MS_CORRUPTED_TLV] = { "corrupted TLV", -EILSEQ },
> - [I2400M_MS_UNINITIALIZED] = { "not unitialized", -EILSEQ },
> + [I2400M_MS_UNINITIALIZED] = { "not uninitialized", -EILSEQ },
I suspect they rather meant "not initialized".
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] wimax/i2400m: fix spelling mistake "unitialized" -> "uninitialized"
From: Colin Ian King @ 2018-09-24 17:35 UTC (permalink / raw)
To: Sergei Shtylyov, Inaky Perez-Gonzalez, linux-wimax,
David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <db8575a7-a4ab-3234-3e59-06cdeffe0f59@cogentembedded.com>
On 24/09/18 18:27, Sergei Shtylyov wrote:
> On 09/24/2018 08:18 PM, Colin King wrote:
>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Trivial fix to spelling mistake in ms_to_errno array of error messages
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/wimax/i2400m/control.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wimax/i2400m/control.c b/drivers/net/wimax/i2400m/control.c
>> index 094cea775d0c..0475ad553707 100644
>> --- a/drivers/net/wimax/i2400m/control.c
>> +++ b/drivers/net/wimax/i2400m/control.c
>> @@ -257,7 +257,7 @@ static const struct
>> [I2400M_MS_ACCESSIBILITY_ERROR] = { "accesibility error", -EIO },
>> [I2400M_MS_BUSY] = { "busy", -EBUSY },
>> [I2400M_MS_CORRUPTED_TLV] = { "corrupted TLV", -EILSEQ },
>> - [I2400M_MS_UNINITIALIZED] = { "not unitialized", -EILSEQ },
>> + [I2400M_MS_UNINITIALIZED] = { "not uninitialized", -EILSEQ },
>
> I suspect they rather meant "not initialized".
Of course, that makes far more sense. I send a V2.
>
> [...]
>
> MBR, Sergei
>
^ 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