Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] bpf: fix bug in eBPF verifier
From: David Miller @ 2014-10-22  1:44 UTC (permalink / raw)
  To: ast; +Cc: hannes, dborkman, netdev, linux-kernel
In-Reply-To: <1413842097-4380-1-git-send-email-ast@plumgrid.com>

From: Alexei Starovoitov <ast@plumgrid.com>
Date: Mon, 20 Oct 2014 14:54:57 -0700

> while comparing for verifier state equivalency the comparison
> was missing a check for uninitialized register.
> Make sure it does so and add a testcase.
> 
> Fixes: f1bca824dabb ("bpf: add search pruning optimization to verifier")
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 1/2] xfrm: fix a potential use after free in xfrm4_policy.c
From: David Miller @ 2014-10-22  1:42 UTC (permalink / raw)
  To: roy.qing.li; +Cc: netdev, steffen.klassert
In-Reply-To: <1413794954-16967-1-git-send-email-roy.qing.li@gmail.com>

From: roy.qing.li@gmail.com
Date: Mon, 20 Oct 2014 16:49:13 +0800

> From: Li RongQing <roy.qing.li@gmail.com>
> 
> pskb_may_pull() maybe change skb->data and make xprth pointer oboslete,
> so recompute the xprth
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>

Please don't use macros that hide uses of local variables.

That is almost as bad as hiding control flow inside of
a macro.

^ permalink raw reply

* Re: [PATCH] drivers: net: xgene: Add missing initialization in xgene_enet_ecc_init()
From: David Miller @ 2014-10-22  1:41 UTC (permalink / raw)
  To: geert; +Cc: isubramanian, kchudgar, netdev, linux-kernel
In-Reply-To: <1413792496-8558-1-git-send-email-geert@linux-m68k.org>

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Mon, 20 Oct 2014 10:08:16 +0200

> drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c: In function ‘xgene_enet_ecc_init’:
> drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:126: warning: ‘data’ may be used uninitialized in this function
> 
> Depending on the arbitrary value on the stack, the loop may terminate
> too early, and cause a bogus -ENODEV failure.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Please, use a do { } while(0) loop to fix this, thanks.

^ permalink raw reply

* Re: [PATCH] netlink: don't copy over empty attribute data
From: David Miller @ 2014-10-22  1:39 UTC (permalink / raw)
  To: sasha.levin; +Cc: a.ryabinin, pablo, mschmidt, akpm, linux-kernel, netdev
In-Reply-To: <1413924669-26732-1-git-send-email-sasha.levin@oracle.com>

From: Sasha Levin <sasha.levin@oracle.com>
Date: Tue, 21 Oct 2014 16:51:09 -0400

> netlink uses empty data to seperate different levels. However, we still
> try to copy that data from a NULL ptr using memcpy, which is an undefined
> behaviour.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

This isn't a POSIX C library, this it the Linux kernel, and as such
we can make sure none of our memcpy() implementations try to access
any bytes if the given length is NULL.

And to be quite honest, there is no benefit whatsoever nor even the
possibility of using that "undefined behavior" flexibility to do
anthing. This is because every memcpy() implementation must be sure
not to access past the end of either source or destination buffer.

And the one and only way to do that is to respect the length.

I'm not applying this, because the basis for which this is claimed
to be a bug fix is quite bogus in my opinion.

Sorry.

^ permalink raw reply

* Re: [PATCH] netlink: Re-add locking to netlink_lookup() and seq walker
From: David Miller @ 2014-10-22  1:36 UTC (permalink / raw)
  To: tgraf; +Cc: davem, netdev, linux-kernel, eric.dumazet
In-Reply-To: <14101d52c64e0d52176ca0d4b8888f8cf20ab9b0.1413921873.git.tgraf@suug.ch>

From: Thomas Graf <tgraf@suug.ch>
Date: Tue, 21 Oct 2014 22:05:38 +0200

> The synchronize_rcu() in netlink_release() introduces unacceptable
> latency. Reintroduce minimal lookup so we can drop the
> synchronize_rcu() until socket destruction has been RCUfied.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
> Reported-and-tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

Applied and queued up for -stable.

Please use an appropriate "Fixes: " tag in the future, this is
especially important for an issue as serious as this one.

Thanks.

^ permalink raw reply

* RE: Urgent Order
From: Parker Nick @ 2014-10-22  7:37 UTC (permalink / raw)


Hello,

Please send me a price list and quotation for your products we want to import from your country this is a very large order for long term import and urgent request.

Thank you in advance for your prompt confirmation.
Regards,

Parker Nick

Equipos de Elevaci n , S.A. de C.V.Av. Texcoco #35,
54070 Tlalnepantla , Edo . de M xico
M xico
Tels.+27840117769

^ permalink raw reply

* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
From: Sowmini Varadhan @ 2014-10-22  0:16 UTC (permalink / raw)
  To: Dave Kleikamp; +Cc: Julian Calaby, David S. Miller, netdev, sparclinux
In-Reply-To: <5446F291.1030400@oracle.com>

On (10/21/14 18:56), Dave Kleikamp wrote:
> > 
> > Is gcc not smart enough to know that this variable isn't used before
> > it's set? (I assume it isn't used elsewhere in this function)
> 
> It probably assumes in_softirq() might evaluate differently in the each
> case.

yes, that's what I suspected too. I suppose it is possible
from the compiler's point of view that something in between 
might change the result of in_softirq() so that we may be 
using an uninit variable in the second call.

anyway, the warning was annoying, and would only numb the
user into ignoring other real issues, so I figured I might
as well silence the warning.

--Sowmini


^ permalink raw reply

* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
From: Dave Kleikamp @ 2014-10-21 23:56 UTC (permalink / raw)
  To: Julian Calaby, Sowmini Varadhan; +Cc: David S. Miller, netdev, sparclinux
In-Reply-To: <CAGRGNgXvKJBhOE1z3=_gZkE=X94HnoAvTN6Zv5G1h8JuFfS8Ng@mail.gmail.com>

On 10/21/2014 05:35 PM, Julian Calaby wrote:
> Hi Sowmini,
> 
> On Wed, Oct 22, 2014 at 1:16 AM, Sowmini Varadhan
> <sowmini.varadhan@oracle.com> wrote:
>> For NAPIfied drivers , there is no need to
>> synchronize by doing irqsave/restore on vio.lock in the I/O
>> path.
>>
>> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
>> ---
>>  arch/sparc/kernel/viohs.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
>> index 7ef081a..d731586 100644
>> --- a/arch/sparc/kernel/viohs.c
>> +++ b/arch/sparc/kernel/viohs.c
>> @@ -747,10 +747,11 @@ EXPORT_SYMBOL(vio_ldc_free);
>>
>>  void vio_port_up(struct vio_driver_state *vio)
>>  {
>> -       unsigned long flags;
>> +       unsigned long flags = 0;
> 
> Is gcc not smart enough to know that this variable isn't used before
> it's set? (I assume it isn't used elsewhere in this function)

It probably assumes in_softirq() might evaluate differently in the each
case.

> 
>>         int err, state;
>>
>> -       spin_lock_irqsave(&vio->lock, flags);
>> +       if (!in_softirq())
>> +               spin_lock_irqsave(&vio->lock, flags);
>>
>>         state = ldc_state(vio->lp);
>>
>> @@ -777,7 +778,8 @@ void vio_port_up(struct vio_driver_state *vio)
>>                 mod_timer(&vio->timer, expires);
>>         }
>>
>> -       spin_unlock_irqrestore(&vio->lock, flags);
>> +       if (!in_softirq())
>> +               spin_unlock_irqrestore(&vio->lock, flags);
>>  }
>>  EXPORT_SYMBOL(vio_port_up);
> 
> Thanks,
> 

^ permalink raw reply

* IPv6 UFO for VMs
From: Ben Hutchings @ 2014-10-21 23:44 UTC (permalink / raw)
  To: netdev; +Cc: Hannes Frederic Sowa, virtualization


[-- Attachment #1.1: Type: text/plain, Size: 1303 bytes --]

There are several ways that VMs can take advantage of UFO and get the
host to do fragmentation for them:

drivers/net/macvtap.c:                  gso_type = SKB_GSO_UDP;
drivers/net/tun.c:                      skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
drivers/net/virtio_net.c:                       skb_shinfo(skb)->gso_type = SKB_GSO_UDP;

Our implementation of UFO for IPv6 does:

		fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
		fptr->nexthdr = nexthdr;
		fptr->reserved = 0;
		fptr->identification = skb_shinfo(skb)->ip6_frag_id;

which assumes ip6_frag_id has been set.  That's only true if the local
stack constructed the skb; otherwise it appears we get zero.

This seems to be a regression as a result of:

commit 916e4cf46d0204806c062c8c6c4d1f633852c5b6
Author: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date:   Fri Feb 21 02:55:35 2014 +0100

    ipv6: reuse ip6_frag_id from ip6_ufo_append_data

However, that change seems reasonable - we *shouldn't* be choosing IDs
for any other stack.  Any paravirt net driver that can use IPv6 UFO
needs to have some way of passing a fragmentation ID to put in
skb_shared_info::ip6_frag_id.

Ben.

-- 
Ben Hutchings
For every action, there is an equal and opposite criticism. - Harrison

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [stable request <= 3.11] net/mlx4_en: Fix BlueFlame race
From: Cong Wang @ 2014-10-21 23:15 UTC (permalink / raw)
  To: Vinson Lee
  Cc: David S. Miller, Amir Vadai, Or Gerlitz, Jack Morgenstein,
	Eugenia Emantayev, Matan Barak, netdev
In-Reply-To: <CAHTgTXVn1479XCoh1-j=Xghtad3zA-hf0za5r_AyoaKuKeFZ2g@mail.gmail.com>

On Sat, Oct 18, 2014 at 2:14 PM, Vinson Lee <vlee@twopensource.com> wrote:
> Hi.
>
> Please consider backporting upstream commit
> 2d4b646613d6b12175b017aca18113945af1faf3 "net/mlx4_en: Fix BlueFlame
> race" to stable kernels <= 3.11.
>

David, could you take care of it if you have time? It fixes a real
bug in production. :)

Thanks a lot!

^ permalink raw reply

* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
From: Julian Calaby @ 2014-10-21 22:42 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: David S. Miller, netdev, sparclinux
In-Reply-To: <20141021223952.GA26724@oracle.com>

Hi Sowmini,

On Wed, Oct 22, 2014 at 9:39 AM, Sowmini Varadhan
<sowmini.varadhan@oracle.com> wrote:
> On (10/22/14 09:35), Julian Calaby wrote:
>> >  void vio_port_up(struct vio_driver_state *vio)
>> >  {
>> > -       unsigned long flags;
>> > +       unsigned long flags = 0;
>>
>> Is gcc not smart enough to know that this variable isn't used before
>> it's set? (I assume it isn't used elsewhere in this function)
>
> No, it's not used elsewhere in the function, and yes, I too was
> surprised by the build warning, which is why I initialized it
> as above.

Ok, fair enough then.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply

* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
From: Sowmini Varadhan @ 2014-10-21 22:39 UTC (permalink / raw)
  To: Julian Calaby; +Cc: David S. Miller, netdev, sparclinux
In-Reply-To: <CAGRGNgXvKJBhOE1z3=_gZkE=X94HnoAvTN6Zv5G1h8JuFfS8Ng@mail.gmail.com>

On (10/22/14 09:35), Julian Calaby wrote:
> >  void vio_port_up(struct vio_driver_state *vio)
> >  {
> > -       unsigned long flags;
> > +       unsigned long flags = 0;
> 
> Is gcc not smart enough to know that this variable isn't used before
> it's set? (I assume it isn't used elsewhere in this function)

No, it's not used elsewhere in the function, and yes, I too was
surprised by the build warning, which is why I initialized it
as above.

--Sowmini


^ permalink raw reply

* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
From: Julian Calaby @ 2014-10-21 22:35 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: David S. Miller, netdev, sparclinux
In-Reply-To: <20141021141647.GF15405@oracle.com>

Hi Sowmini,

On Wed, Oct 22, 2014 at 1:16 AM, Sowmini Varadhan
<sowmini.varadhan@oracle.com> wrote:
> For NAPIfied drivers , there is no need to
> synchronize by doing irqsave/restore on vio.lock in the I/O
> path.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
>  arch/sparc/kernel/viohs.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
> index 7ef081a..d731586 100644
> --- a/arch/sparc/kernel/viohs.c
> +++ b/arch/sparc/kernel/viohs.c
> @@ -747,10 +747,11 @@ EXPORT_SYMBOL(vio_ldc_free);
>
>  void vio_port_up(struct vio_driver_state *vio)
>  {
> -       unsigned long flags;
> +       unsigned long flags = 0;

Is gcc not smart enough to know that this variable isn't used before
it's set? (I assume it isn't used elsewhere in this function)

>         int err, state;
>
> -       spin_lock_irqsave(&vio->lock, flags);
> +       if (!in_softirq())
> +               spin_lock_irqsave(&vio->lock, flags);
>
>         state = ldc_state(vio->lp);
>
> @@ -777,7 +778,8 @@ void vio_port_up(struct vio_driver_state *vio)
>                 mod_timer(&vio->timer, expires);
>         }
>
> -       spin_unlock_irqrestore(&vio->lock, flags);
> +       if (!in_softirq())
> +               spin_unlock_irqrestore(&vio->lock, flags);
>  }
>  EXPORT_SYMBOL(vio_port_up);

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply

* Re: [PATCH] Documentation: ptp: Fix build failure on MIPS cross builds
From: Peter Foley @ 2014-10-21 22:04 UTC (permalink / raw)
  To: Richard Cochran
  Cc: David Daney, David Miller, markos.chandras, linux-mips, corbet,
	netdev, linux-doc@vger.kernel.org, LKML
In-Reply-To: <20141021182757.GA3960@localhost.localdomain>

On Tue, Oct 21, 2014 at 2:27 PM, Richard Cochran
<richardcochran@gmail.com> wrote:
> On Tue, Oct 21, 2014 at 09:58:51AM -0700, David Daney wrote:
>> What I don't understand is why we are using hostprogs in this
>> Makefile.  Isn't this a program that would run on the target, not
>> the build host?
>
> Yes.
>
> Peter, could you please fix it?


The intention of these changes was to generate more compiliation
coverage for code in Documentation/
The underlying issue is that this doesn't work for cross-compiling
because kbuild doesn't have cross-compile support for userspace code.
I submitted a patch to disable building Documentation when
cross-compiling, as the consensus in the thread that resulted in that
patch (https://lkml.org/lkml/2014/10/8/510) was that implementing
targetprogs in kbuild was not currently worth it.
I can try to take a crack at adding targetprogs support, but I'm
rather busy right now, so it may take a little while.

Thanks,

Peter

^ permalink raw reply

* Re: [PATCH 0/9] Marvell PXA168 libphy handling and Berlin Ethernet
From: Florian Fainelli @ 2014-10-21 22:03 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: David S. Miller, Antoine Ténart, Eric Miao, Haojian Zhuang,
	linux-arm-kernel, netdev, devicetree, linux-kernel
In-Reply-To: <1413881627-21639-1-git-send-email-sebastian.hesselbarth@gmail.com>

Hi Sebastian,

On 10/21/2014 01:53 AM, Sebastian Hesselbarth wrote:
> This patch series deals with a removing a IP feature that can be found
> on all currently supported Marvell Ethernet IP (pxa168_eth, mv643xx_eth,
> mvneta). The MAC IP allows to automatically perform PHY auto-negotiation
> without software interaction.
> 
> However, this feature (a) fundamentally clashes with the way libphy works
> and (b) is unable to deal with quirky PHYs that require special treatment.
> In this series, pxa168_eth driver is rewritten to completely disable that
> feature and properly deal with libphy provided PHYs.
> 
> This is the real patch set after an RFT sent earlier. Unfortunately, there
> was no testing from MMP/gplug but there was for BG2Q. However, it still
> would be great to get a Tested-by on gplug. Also, this patch set is now
> rebased on v3.18-rc1. As usual, a branch on top of v3.18-rc1 can be found at
> 
> git://git.infradead.org/users/hesselba/linux-berlin.git devel/bg2-bg2cd-eth-v1
> 
> Patches 1-5 should go through David's net tree, I'll pick up the DT patches
> 6-9.
> 
> Compared to the RFT, there have been some changes:
> - added phy-connection-type property to BG2Q PHY DT node
> - bail out from pxa168_eth_adjust_link when there is no change in
>   PHY parameters. Also, add a call to phy_print_status.
> 
> Patch 1 adds support for Marvell 88E3016 FastEthernet PHY that is also
> integrated in Marvell Berlin BG2/BG2CD SoCs.
> 
> Patch 2 allows to pass phy_interface_t on pxa168_eth platform_data that
> is only used by mach-mmp/gplug. From the board setup, I guessed gplug's
> PHY is connected via RMII. The patch still isn't even compile tested.
> 
> Patches 3-5 prepare proper libphy handling and finally remove all in-driver
> PHY mangling related to the feature explained above.
> 
> Patches 6-9 add corresponding ethernet DT nodes to BG2, BG2CD, add a
> phy-connection-type property to BG2Q and enable ethernet on BG2-based Sony
> NSZ-GS7. I have tested all this on GS7 successfully with ip=dhcp on 100M FD.

Besides the misplacement of the 'phy-connection-type' as reported by
Sergei, this looks good to me:

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

Thanks!

> 
> Antoine Ténart (1):
>   ARM: berlin: Add phy-connection-type to BG2Q PHY
> 
> Sebastian Hesselbarth (8):
>   phy: marvell: Add support for 88E3016 FastEthernet PHY
>   net: pxa168_eth: Provide phy_interface mode on platform_data
>   net: pxa168_eth: Prepare proper libphy handling
>   net: pxa168_eth: Remove HW auto-negotiaion
>   net: pxa168_eth: Remove in-driver PHY mangling
>   ARM: berlin: Add BG2 ethernet DT nodes
>   ARM: berlin: Add BG2CD ethernet DT nodes
>   ARM: berlin: Enable ethernet on Sony NSZ-GS7
> 
>  arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts |   2 +
>  arch/arm/boot/dts/berlin2.dtsi             |  36 +++++
>  arch/arm/boot/dts/berlin2cd.dtsi           |  36 +++++
>  arch/arm/boot/dts/berlin2q.dtsi            |   1 +
>  arch/arm/mach-mmp/gplugd.c                 |   2 +
>  drivers/net/ethernet/marvell/pxa168_eth.c  | 248 ++++++++++++-----------------
>  drivers/net/phy/marvell.c                  |  46 ++++++
>  include/linux/marvell_phy.h                |   1 +
>  include/linux/pxa168_eth.h                 |   1 +
>  9 files changed, 225 insertions(+), 148 deletions(-)
> 
> ---
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: "Antoine Ténart" <antoine.tenart@free-electrons.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Eric Miao <eric.y.miao@gmail.com>
> Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: netdev@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 

^ permalink raw reply

* Re: [PATCH] net: typhoon: Remove redundant casts
From: David Dillow @ 2014-10-21 21:29 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: netdev, linux-kernel
In-Reply-To: <1413903103-3047-1-git-send-email-linux@rasmusvillemoes.dk>

On Tue, 2014-10-21 at 16:51 +0200, Rasmus Villemoes wrote:
> Both image_data and typhoon_fw->data are const u8*, so the cast to u8*
> is unnecessary and confusing.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Acked-by: David Dillow <dave@thedillows.org>

^ permalink raw reply

* RE: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send()
From: Haiyang Zhang @ 2014-10-21 21:25 UTC (permalink / raw)
  To: Sitsofe Wheeler, Long Li
  Cc: David Miller, olaf@aepfle.de, netdev@vger.kernel.org,
	jasowang@redhat.com, linux-kernel@vger.kernel.org,
	apw@canonical.com, devel@linuxdriverproject.org
In-Reply-To: <CALjAwxj1jHUugt+vtJL7duXfTpdoxmCrwTzRmNHGgOdj+Ju3Aw@mail.gmail.com>



> -----Original Message-----
> From: Sitsofe Wheeler [mailto:sitsofe@gmail.com]
> Sent: Tuesday, October 21, 2014 5:16 PM
> To: Long Li
> Cc: David Miller; olaf@aepfle.de; netdev@vger.kernel.org;
> jasowang@redhat.com; linux-kernel@vger.kernel.org; apw@canonical.com;
> devel@linuxdriverproject.org; Haiyang Zhang
> Subject: Re: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send()
> 
> On 21 October 2014 18:13, Long Li <longli@microsoft.com> wrote:
> > Thanks Sitsofe. This should have been fixed by this patch:
> > http://git.kernel.org/cgit/linux/kernel/git/next/linux-
> next.git/commit/?id=f88e67149f97d73c704d6fe6f492edde97463025
> >
> > Can you give it a try?
> 
> Ah this one went mainline a few days ago so I've already been trying
> it. Yes it resolves my problem (that system is no longer oopsing on
> startup). I'd have added my Tested-by but it's already been committed
> :-)

Thank you for the test!

- Haiyang

^ permalink raw reply

* Re: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send()
From: Sitsofe Wheeler @ 2014-10-21 21:16 UTC (permalink / raw)
  To: Long Li
  Cc: David Miller, olaf@aepfle.de, netdev@vger.kernel.org,
	jasowang@redhat.com, linux-kernel@vger.kernel.org,
	apw@canonical.com, devel@linuxdriverproject.org, Haiyang Zhang
In-Reply-To: <1413911624662.86334@microsoft.com>

On 21 October 2014 18:13, Long Li <longli@microsoft.com> wrote:
> Thanks Sitsofe. This should have been fixed by this patch:
> http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=f88e67149f97d73c704d6fe6f492edde97463025
>
> Can you give it a try?

Ah this one went mainline a few days ago so I've already been trying
it. Yes it resolves my problem (that system is no longer oopsing on
startup). I'd have added my Tested-by but it's already been committed
:-)

What helped you narrow it down?

-- 
Sitsofe | http://sucs.org/~sits/

^ permalink raw reply

* Re: localed stuck in recent 3.18 git in copy_net_ns?
From: Kevin Fenzi @ 2014-10-21 21:12 UTC (permalink / raw)
  To: netdev, linux-kernel
In-Reply-To: <20141020145359.565fe5e6@voldemort.scrye.com>

[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]

On Mon, 20 Oct 2014 14:53:59 -0600
Kevin Fenzi <kevin@scrye.com> wrote:

> On Mon, 20 Oct 2014 16:43:26 -0400
> Dave Jones <davej@redhat.com> wrote:
> 
> > I've seen similar soft lockup traces from the sys_unshare path when
> > running my fuzz tester.  It seems that if you create enough network
> > namespaces, it can take a huge amount of time for them to be
> > iterated. (Running trinity with '-c unshare' you can see the slow
> > down happen. In some cases, it takes so long that the watchdog
> > process kills it -- though the SIGKILL won't get delivered until
> > the unshare() completes)
> > 
> > Any idea what this machine had been doing prior to this that may
> > have involved creating lots of namespaces ?
> 
> That was right after boot. ;) 
> 
> This is my main rawhide running laptop.
> 
> A 'ip netns list' shows nothing.

Some more information: 

The problem started between: 

v3.17-7872-g5ff0b9e1a1da and v3.17-8307-gf1d0d14120a8

(I can try and do a bisect, but have to head out on a trip tomorrow)

In all the kernels with the problem, there is a kworker process in D. 

sysrq-t says: 
                                            Showing all locks held in the system:
Oct 21 15:06:31 voldemort.scrye.com kernel: 4 locks held by kworker/u16:0/6:
Oct 21 15:06:31 voldemort.scrye.com kernel:  #0:  ("%s""netns"){.+.+.+}, at: [<ffffffff810ccbff>] process_one_work+0x17f/0x850
Oct 21 15:06:31 voldemort.scrye.com kernel:  #1:  (net_cleanup_work){+.+.+.}, at: [<ffffffff810ccbff>] process_one_work+0x17f/0x850
Oct 21 15:06:31 voldemort.scrye.com kernel:  #2:  (net_mutex){+.+.+.}, at: [<ffffffff817069fc>] cleanup_net+0x8c/0x1f0
Oct 21 15:06:31 voldemort.scrye.com kernel:  #3:
(rcu_sched_state.barrier_mutex){+.+...}, at: [<ffffffff8112a395>]
_rcu_barrier+0x35/0x200

On first running any of the systemd units that use PrivateNetwork, then
run ok, but they are also set to timeout after a minute. On sucessive
runs they hang in D also.

kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/5] stmmac: pci: use managed resources
From: Sergei Shtylyov @ 2014-10-21 20:55 UTC (permalink / raw)
  To: Andy Shevchenko, Giuseppe Cavallaro, netdev, Kweh Hock Leong,
	David S. Miller, Vince Bridgers
In-Reply-To: <1413909333-16380-3-git-send-email-andriy.shevchenko@linux.intel.com>

Hello.

On 10/21/2014 08:35 PM, Andy Shevchenko wrote:

> Migrate pci driver to managed resources to reduce boilerplate error handling
> code.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 46 +++++-------------------
>   1 file changed, 8 insertions(+), 38 deletions(-)

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 5459a4e..f8d4ce2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -65,45 +65,29 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
>   			    const struct pci_device_id *id)
>   {
>   	int ret = 0;
> -	void __iomem *addr = NULL;
>   	struct stmmac_priv *priv = NULL;
> -	int i;
> +	int pci_bar = 0;

    I don't see this variable changing anywhere...

>   	/* Enable pci device */
> -	ret = pci_enable_device(pdev);
> +	ret = pcim_enable_device(pdev);
>   	if (ret) {
>   		pr_err("%s : ERROR: failed to enable %s device\n", __func__,
>   		       pci_name(pdev));
>   		return ret;
>   	}
> -	if (pci_request_regions(pdev, STMMAC_RESOURCE_NAME)) {
> -		pr_err("%s: ERROR: failed to get PCI region\n", __func__);
> -		ret = -ENODEV;
> -		goto err_out_req_reg_failed;
> -	}
> +	ret = pcim_iomap_regions(pdev, BIT(pci_bar), pci_name(pdev));
> +	if (ret)
> +		return ret;
>
> -	/* Get the base address of device */
> -	for (i = 0; i <= 5; i++) {
> -		if (pci_resource_len(pdev, i) == 0)
> -			continue;
> -		addr = pci_iomap(pdev, i, 0);
> -		if (addr == NULL) {
> -			pr_err("%s: ERROR: cannot map register memory aborting",
> -			       __func__);
> -			ret = -EIO;
> -			goto err_out_map_failed;
> -		}
> -		break;
> -	}

    It's not an equivalent change: the old code mapped a first existing BAR, 
you always map BAR0. Are you sure that's what you meant? If so, wouldn't hurt 
to describe this in the changelog...

WBR, Sergei

^ permalink raw reply

* [PATCH] netlink: don't copy over empty attribute data
From: Sasha Levin @ 2014-10-21 20:51 UTC (permalink / raw)
  To: davem; +Cc: a.ryabinin, pablo, mschmidt, akpm, linux-kernel, netdev,
	Sasha Levin

netlink uses empty data to seperate different levels. However, we still
try to copy that data from a NULL ptr using memcpy, which is an undefined
behaviour.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 lib/nlattr.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/nlattr.c b/lib/nlattr.c
index 9c3e85f..6512b43 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -430,7 +430,8 @@ void __nla_put(struct sk_buff *skb, int attrtype, int attrlen,
 	struct nlattr *nla;
 
 	nla = __nla_reserve(skb, attrtype, attrlen);
-	memcpy(nla_data(nla), data, attrlen);
+	if (data)
+		memcpy(nla_data(nla), data, attrlen);
 }
 EXPORT_SYMBOL(__nla_put);
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/5] stmmac: pci: use managed resources
From: Andy Shevchenko @ 2014-10-21 16:35 UTC (permalink / raw)
  To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S. Miller,
	Vince Bridgers
  Cc: Andy Shevchenko
In-Reply-To: <1413909333-16380-1-git-send-email-andriy.shevchenko@linux.intel.com>

Migrate pci driver to managed resources to reduce boilerplate error handling
code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 46 +++++-------------------
 1 file changed, 8 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 5459a4e..f8d4ce2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -65,45 +65,29 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
 	int ret = 0;
-	void __iomem *addr = NULL;
 	struct stmmac_priv *priv = NULL;
-	int i;
+	int pci_bar = 0;
 
 	/* Enable pci device */
-	ret = pci_enable_device(pdev);
+	ret = pcim_enable_device(pdev);
 	if (ret) {
 		pr_err("%s : ERROR: failed to enable %s device\n", __func__,
 		       pci_name(pdev));
 		return ret;
 	}
-	if (pci_request_regions(pdev, STMMAC_RESOURCE_NAME)) {
-		pr_err("%s: ERROR: failed to get PCI region\n", __func__);
-		ret = -ENODEV;
-		goto err_out_req_reg_failed;
-	}
+	ret = pcim_iomap_regions(pdev, BIT(pci_bar), pci_name(pdev));
+	if (ret)
+		return ret;
 
-	/* Get the base address of device */
-	for (i = 0; i <= 5; i++) {
-		if (pci_resource_len(pdev, i) == 0)
-			continue;
-		addr = pci_iomap(pdev, i, 0);
-		if (addr == NULL) {
-			pr_err("%s: ERROR: cannot map register memory aborting",
-			       __func__);
-			ret = -EIO;
-			goto err_out_map_failed;
-		}
-		break;
-	}
 	pci_set_master(pdev);
 
 	stmmac_default_data();
 
-	priv = stmmac_dvr_probe(&(pdev->dev), &plat_dat, addr);
+	priv = stmmac_dvr_probe(&pdev->dev, &plat_dat,
+				pcim_iomap_table(pdev)[pci_bar]);
 	if (IS_ERR(priv)) {
 		pr_err("%s: main driver probe failed", __func__);
-		ret = PTR_ERR(priv);
-		goto err_out;
+		return PTR_ERR(priv);
 	}
 	priv->dev->irq = pdev->irq;
 	priv->wol_irq = pdev->irq;
@@ -113,15 +97,6 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 	pr_debug("STMMAC platform driver registration completed");
 
 	return 0;
-
-err_out:
-	pci_clear_master(pdev);
-err_out_map_failed:
-	pci_release_regions(pdev);
-err_out_req_reg_failed:
-	pci_disable_device(pdev);
-
-	return ret;
 }
 
 /**
@@ -134,13 +109,8 @@ err_out_req_reg_failed:
 static void stmmac_pci_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
-	struct stmmac_priv *priv = netdev_priv(ndev);
 
 	stmmac_dvr_remove(ndev);
-
-	pci_iounmap(pdev, priv->ioaddr);
-	pci_release_regions(pdev);
-	pci_disable_device(pdev);
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.1.1

^ permalink raw reply related

* [PATCH 5/5] stmmac: pci: remove FSF address
From: Andy Shevchenko @ 2014-10-21 16:35 UTC (permalink / raw)
  To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S. Miller,
	Vince Bridgers
  Cc: Andy Shevchenko
In-Reply-To: <1413909333-16380-1-git-send-email-andriy.shevchenko@linux.intel.com>

The FSF address is subject to change, thus remove it from the file.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 22cb5ff..95eb195 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -12,10 +12,6 @@
   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
   more details.
 
-  You should have received a copy of the GNU General Public License along with
-  this program; if not, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".
 
-- 
2.1.1

^ permalink raw reply related

* Re: [PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove
From: Fabian Frederick @ 2014-10-21 20:05 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, netdev, linux-kernel, David S. Miller,
	John W. Linville
In-Reply-To: <1413918366.1998.10.camel@jlt4.sipsolutions.net>



> On 21 October 2014 at 21:06 Johannes Berg <johannes@sipsolutions.net> wrote:
>
>
> On Tue, 2014-10-21 at 18:20 +0200, Fabian Frederick wrote:
> > Fix checkpatch warnings:
> >
> >     WARNING: debugfs_remove(NULL) is safe this check is probably not
> >required
>
> I'll apply this; however, I think that checkpatch is a just tool, and
> the commit message should reflect why you're changing the code.
> Presumably you're not doing it to make the tool happy, but to address an
> issue that the tool pointed out, so I think in most cases the commit
> message should state the former, not the latter.
>
> Note that in this particular case the NULL check check could be there to
> avoid a memory write (which can be significant depending on the context)
> so blindly doing what the tool suggested wouldn't always be a good idea.
>
> johannes
>

Thanks Johannes,

Maybe you can replace commit message with:
"
This patch removes NULL check before debugfs_remove.
That function already does that check and is
only called during key management so we can add some memory writes.
"

I can also resubmit patch if necessary.

Regards,
Fabian

^ permalink raw reply

* [PATCH] netlink: Re-add locking to netlink_lookup() and seq walker
From: Thomas Graf @ 2014-10-21 20:05 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, David S. Miller, Eric Dumazet

The synchronize_rcu() in netlink_release() introduces unacceptable
latency. Reintroduce minimal lookup so we can drop the
synchronize_rcu() until socket destruction has been RCUfied.

Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
Reported-and-tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 net/netlink/af_netlink.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7a186e7..f1de72d 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -96,6 +96,14 @@ static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
 static int netlink_dump(struct sock *sk);
 static void netlink_skb_destructor(struct sk_buff *skb);
 
+/* nl_table locking explained:
+ * Lookup and traversal are protected with nl_sk_hash_lock or nl_table_lock
+ * combined with an RCU read-side lock. Insertion and removal are protected
+ * with nl_sk_hash_lock while using RCU list modification primitives and may
+ * run in parallel to nl_table_lock protected lookups. Destruction of the
+ * Netlink socket may only occur *after* nl_table_lock has been acquired
+ * either during or after the socket has been removed from the list.
+ */
 DEFINE_RWLOCK(nl_table_lock);
 EXPORT_SYMBOL_GPL(nl_table_lock);
 static atomic_t nl_table_users = ATOMIC_INIT(0);
@@ -109,10 +117,10 @@ EXPORT_SYMBOL_GPL(nl_sk_hash_lock);
 static int lockdep_nl_sk_hash_is_held(void)
 {
 #ifdef CONFIG_LOCKDEP
-	return (debug_locks) ? lockdep_is_held(&nl_sk_hash_lock) : 1;
-#else
-	return 1;
+	if (debug_locks)
+		return lockdep_is_held(&nl_sk_hash_lock) || lockdep_is_held(&nl_table_lock);
 #endif
+	return 1;
 }
 
 static ATOMIC_NOTIFIER_HEAD(netlink_chain);
@@ -1028,11 +1036,13 @@ static struct sock *netlink_lookup(struct net *net, int protocol, u32 portid)
 	struct netlink_table *table = &nl_table[protocol];
 	struct sock *sk;
 
+	read_lock(&nl_table_lock);
 	rcu_read_lock();
 	sk = __netlink_lookup(table, portid, net);
 	if (sk)
 		sock_hold(sk);
 	rcu_read_unlock();
+	read_unlock(&nl_table_lock);
 
 	return sk;
 }
@@ -1257,9 +1267,6 @@ static int netlink_release(struct socket *sock)
 	}
 	netlink_table_ungrab();
 
-	/* Wait for readers to complete */
-	synchronize_net();
-
 	kfree(nlk->groups);
 	nlk->groups = NULL;
 
@@ -1281,6 +1288,7 @@ static int netlink_autobind(struct socket *sock)
 
 retry:
 	cond_resched();
+	netlink_table_grab();
 	rcu_read_lock();
 	if (__netlink_lookup(table, portid, net)) {
 		/* Bind collision, search negative portid values. */
@@ -1288,9 +1296,11 @@ retry:
 		if (rover > -4097)
 			rover = -4097;
 		rcu_read_unlock();
+		netlink_table_ungrab();
 		goto retry;
 	}
 	rcu_read_unlock();
+	netlink_table_ungrab();
 
 	err = netlink_insert(sk, net, portid);
 	if (err == -EADDRINUSE)
@@ -2921,14 +2931,16 @@ static struct sock *netlink_seq_socket_idx(struct seq_file *seq, loff_t pos)
 }
 
 static void *netlink_seq_start(struct seq_file *seq, loff_t *pos)
-	__acquires(RCU)
+	__acquires(nl_table_lock) __acquires(RCU)
 {
+	read_lock(&nl_table_lock);
 	rcu_read_lock();
 	return *pos ? netlink_seq_socket_idx(seq, *pos - 1) : SEQ_START_TOKEN;
 }
 
 static void *netlink_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
+	struct rhashtable *ht;
 	struct netlink_sock *nlk;
 	struct nl_seq_iter *iter;
 	struct net *net;
@@ -2943,19 +2955,19 @@ static void *netlink_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 	iter = seq->private;
 	nlk = v;
 
-	rht_for_each_entry_rcu(nlk, nlk->node.next, node)
+	i = iter->link;
+	ht = &nl_table[i].hash;
+	rht_for_each_entry(nlk, nlk->node.next, ht, node)
 		if (net_eq(sock_net((struct sock *)nlk), net))
 			return nlk;
 
-	i = iter->link;
 	j = iter->hash_idx + 1;
 
 	do {
-		struct rhashtable *ht = &nl_table[i].hash;
 		const struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
 
 		for (; j < tbl->size; j++) {
-			rht_for_each_entry_rcu(nlk, tbl->buckets[j], node) {
+			rht_for_each_entry(nlk, tbl->buckets[j], ht, node) {
 				if (net_eq(sock_net((struct sock *)nlk), net)) {
 					iter->link = i;
 					iter->hash_idx = j;
@@ -2971,9 +2983,10 @@ static void *netlink_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void netlink_seq_stop(struct seq_file *seq, void *v)
-	__releases(RCU)
+	__releases(RCU) __releases(nl_table_lock)
 {
 	rcu_read_unlock();
+	read_unlock(&nl_table_lock);
 }
 
 
-- 
1.9.3

^ permalink raw reply related


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