Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
From: David Miller @ 2013-09-04 18:13 UTC (permalink / raw)
  To: hannes; +Cc: michele, netdev, yoshfuji, sasha.levin
In-Reply-To: <20130903001331.GA8820@order.stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 3 Sep 2013 02:13:31 +0200

> Hi!
> 
> On Mon, Sep 02, 2013 at 10:31:28PM +0100, Michele Baldessari wrote:
>> with the latest linux master git tree from Linus
>> (248d296d6d9df384996c2ed95676b367d876d48c - 2 Sep) I can reproduceably oops 
>> the kernel with the following commands:
>> ip addrlabel flush
>> ip addrlabel add prefix ::1/128              label 0
>> ip addrlabel add prefix ::/0                 label 1
> 
> Thanks for the report! This patch should fix this issue:
> 
> [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
> 
> Commit b67bfe0d42cac56c512dd5da4b1b347a23f4b70a ("hlist: drop
> the node parameter from iterators") changed the behavior of
> hlist_for_each_entry_safe to leave the p argument NULL.
> 
> Fix this up by tracking the last argument.
> 
> Reported-by: Michele Baldessari <michele@acksyn.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Applied.

^ permalink raw reply

* Re: [PATCH] net: fec: fix the error to get the previous BD entry
From: David Miller @ 2013-09-04 18:13 UTC (permalink / raw)
  To: B38611; +Cc: b20596, netdev, bhutchings, stephen
In-Reply-To: <1378176078-10380-1-git-send-email-B38611@freescale.com>

From: Fugang Duan <B38611@freescale.com>
Date: Tue, 3 Sep 2013 10:41:18 +0800

> Bug: error to get the previous BD entry. When the current BD
> is the first BD, the previous BD entry must be the last BD,
> not "bdp - 1" in current logic.
 ...
> Reviewed-by: Li Frank <B20596@freescale.com>
> Signed-off-by: Fugang Duan  <B38611@freescale.com>

Applied.

^ permalink raw reply

* [PATCH] icplus: Use netif_running to determine device state
From: Jon Mason @ 2013-09-04 18:26 UTC (permalink / raw)
  To: netdev; +Cc: Francois Romieu, Sorbica Shieh

Remove the __LINK_STATE_START check to verify the device is running, in
favor of netif_running().  netif_running() performs the same check of
__LINK_STATE_START, so the code should behave the same.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Sorbica Shieh <sorbica@icplus.com.tw>
---
 drivers/net/ethernet/icplus/ipg.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/icplus/ipg.c b/drivers/net/ethernet/icplus/ipg.c
index 1fde90b..bdf5023 100644
--- a/drivers/net/ethernet/icplus/ipg.c
+++ b/drivers/net/ethernet/icplus/ipg.c
@@ -1004,7 +1004,7 @@ static struct net_device_stats *ipg_nic_get_stats(struct net_device *dev)
 	/* Check to see if the NIC has been initialized via nic_open,
 	 * before trying to read statistic registers.
 	 */
-	if (!test_bit(__LINK_STATE_START, &dev->state))
+	if (!netif_running(dev))
 		return &sp->stats;
 
 	sp->stats.rx_packets += ipg_r32(IPG_FRAMESRCVDOK);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH RFC] solos-pci: Fix BUG() with shared skb
From: David Miller @ 2013-09-04 18:30 UTC (permalink / raw)
  To: dwmw2; +Cc: netdev, simon, linux-atm-general, nathan
In-Reply-To: <1378223125.4210.11.camel@i7.infradead.org>

From: David Woodhouse <dwmw2@infradead.org>
Date: Tue, 03 Sep 2013 16:45:25 +0100

> Simon reported this BUG():
> 
>  kernel BUG at net/core/skbuff.c:1065!
>  Call Trace:
>   [<f9b7c12c>] ? pppoatm_send+0x3f/0x1a0 [pppoatm]
>   [<f8751797>] psend+0xa9/0x14a [solos_pci]
>   [<f9b7c248>] pppoatm_send+0x15b/0x1a0 [pppoatm]
>   [<f8a2f77d>] ppp_push+0x76/0x533 [ppp_generic]
> 
> (Rest of backtrace at http://s85.org/mn0aOxMN ― the skb appears to be
> IPv6, forwarded from another interface over PPPoATM.)
> 
> I wasn't expecting to see shared skbs in the ATM driver's ->send()
> function. Is this the right fix?

skb_realloc_headroom() should do everything you need.

This is what ethernet drivers do to prepend custom headers
when skb_headroom() is not large enough.

For example, see drivers/net/ethernet/sun/niu.c:niu_start_xmit().
There, the driver is attempting to prepend a TX descriptor to the
SKB.

If the SKB is shared, skb_realloc_headroom() will do the clone
for you if necessary.

^ permalink raw reply

* Re: [PATCH v5] ipv6:introduce function to find route for redirect
From: Hannes Frederic Sowa @ 2013-09-04 18:36 UTC (permalink / raw)
  To: Duan Jiong; +Cc: davem, netdev
In-Reply-To: <52271D15.1000806@cn.fujitsu.com>

On Wed, Sep 04, 2013 at 07:44:21PM +0800, Duan Jiong wrote:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> 
> RFC 4861 says that the IP source address of the Redirect is the
> same as the current first-hop router for the specified ICMP
> Destination Address, so the gateway should be taken into
> consideration when we find the route for redirect.
> 
> There was once a check in commit
> a6279458c534d01ccc39498aba61c93083ee0372 ("NDISC: Search over
> all possible rules on receipt of redirect.") and the check
> went away in commit b94f1c0904da9b8bf031667afc48080ba7c3e8c9
> ("ipv6: Use icmpv6_notify() to propagate redirect, instead of
> rt6_redirect()").
> 
> The bug is only "exploitable" on layer-2 because the source
> address of the redirect is checked to be a valid link-local
> address but it makes spoofing a lot easier in the same L2
> domain nonetheless.
> 
> Thanks very much for Hannes's help.
> 
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>

Tested and looks good!

Thanks!

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

^ permalink raw reply

* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: David Miller @ 2013-09-04 18:41 UTC (permalink / raw)
  To: tgraf; +Cc: netdev, eric.dumazet, hannes, swarren, festevam
In-Reply-To: <e4600dac657e1160da7a5e7758dcb973b616a10e.1378207925.git.tgraf@suug.ch>

From: Thomas Graf <tgraf@suug.ch>
Date: Tue,  3 Sep 2013 13:37:01 +0200

> Allocating skbs when sending out neighbour discovery messages
> currently uses sock_alloc_send_skb() based on a per net namespace
> socket and thus share a socket wmem buffer space.
> 
> If a netdevice is temporarily unable to transmit due to carrier
> loss or for other reasons, the queued up ndisc messages will cosnume
> all of the wmem space and will thus prevent from any more skbs to
> be allocated even for netdevices that are able to transmit packets.
> 
> The number of neighbour discovery messages sent is very limited,
> use of alloc_skb() bypasses the socket wmem buffer size enforcement
> while the manual call to skb_set_owner_w() maintains the socket
> reference needed for the IPv6 output path.
> 
> This patch has orginally been posted by Eric Dumazet in a modified
> form.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

Applied, and queued up for -stable along with Cong Wang's ndisc_send_rs()
locking change.

Thanks.

^ permalink raw reply

* Re: [PATCH v2 1/2] atm: nicstar: re-use native mac_pton() helper
From: David Miller @ 2013-09-04 18:42 UTC (permalink / raw)
  To: andriy.shevchenko; +Cc: chas, linux-atm-general, netdev, andy.shevchenko
In-Reply-To: <1378210424-14781-1-git-send-email-andriy.shevchenko@linux.intel.com>

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Tue,  3 Sep 2013 15:13:43 +0300

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> There is a nice helper to parse MAC. Let's use it and remove custom
> implementation.
> 
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2 2/2] atm: he: print MAC via %pM
From: David Miller @ 2013-09-04 18:42 UTC (permalink / raw)
  To: andriy.shevchenko; +Cc: chas, linux-atm-general, netdev
In-Reply-To: <1378210424-14781-2-git-send-email-andriy.shevchenko@linux.intel.com>

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Tue,  3 Sep 2013 15:13:44 +0300

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] vxlan: Optimize vxlan rcv
From: David Miller @ 2013-09-04 18:42 UTC (permalink / raw)
  To: pshelar; +Cc: netdev, stephen
In-Reply-To: <1378226684-7751-1-git-send-email-pshelar@nicira.com>

From: Pravin B Shelar <pshelar@nicira.com>
Date: Tue,  3 Sep 2013 09:44:44 -0700

> vxlan-udp-recv function lookup vxlan_sock struct on every packet
> recv by using udp-port number. we can use sk->sk_user_data to
> store vxlan_sock and avoid lookup.
> I have open coded rcu-api to store and read vxlan_sock from
> sk_user_data to avoid sparse warning as sk_user_data is not
> __rcu pointer.
> 
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] tcp: better comments for RTO initiallization
From: David Miller @ 2013-09-04 18:42 UTC (permalink / raw)
  To: ycheng; +Cc: ncardwell, edumazet, netdev
In-Reply-To: <1378242875-5172-1-git-send-email-ycheng@google.com>

From: Yuchung Cheng <ycheng@google.com>
Date: Tue,  3 Sep 2013 14:14:35 -0700

> Commit 1b7fdd2ab585("tcp: do not use cached RTT for RTT estimation")
> removes important comments on how RTO is initialized and updated.
> Hopefully this patch puts those information back.
> 
> Signed-off-by: Yuchung Cheng <ycheng@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH] xen-netback: count number required slots for an skb more carefully
From: David Miller @ 2013-09-04 18:48 UTC (permalink / raw)
  To: david.vrabel
  Cc: xen-devel, konrad.wilk, boris.ostrovsky, ian.campbell, netdev
In-Reply-To: <1378229390-12919-1-git-send-email-david.vrabel@citrix.com>


Can you guys just post the patch you want applied once this thread
moves closer to a consensus?  Thanks.

^ permalink raw reply

* Re: [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
From: Hannes Frederic Sowa @ 2013-09-04 18:51 UTC (permalink / raw)
  To: David Miller; +Cc: michele, netdev, yoshfuji, sasha.levin
In-Reply-To: <20130904.141301.41040920905693536.davem@davemloft.net>

On Wed, Sep 04, 2013 at 02:13:01PM -0400, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Tue, 3 Sep 2013 02:13:31 +0200
> 
> > Hi!
> > 
> > On Mon, Sep 02, 2013 at 10:31:28PM +0100, Michele Baldessari wrote:
> >> with the latest linux master git tree from Linus
> >> (248d296d6d9df384996c2ed95676b367d876d48c - 2 Sep) I can reproduceably oops 
> >> the kernel with the following commands:
> >> ip addrlabel flush
> >> ip addrlabel add prefix ::1/128              label 0
> >> ip addrlabel add prefix ::/0                 label 1
> > 
> > Thanks for the report! This patch should fix this issue:
> > 
> > [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
> > 
> > Commit b67bfe0d42cac56c512dd5da4b1b347a23f4b70a ("hlist: drop
> > the node parameter from iterators") changed the behavior of
> > hlist_for_each_entry_safe to leave the p argument NULL.
> > 
> > Fix this up by tracking the last argument.
> > 
> > Reported-by: Michele Baldessari <michele@acksyn.org>
> > Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> > Cc: Sasha Levin <sasha.levin@oracle.com>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> 
> Applied.

Sorry I forgot to mention that this patch is also applicable to solve
this issue in the longterm 3.10 kernel. Maybe this should go to stable?

Thanks,

  Hannes

^ permalink raw reply

* Re: [PATCH net-next v2 0/8] IPv6 MLD updates
From: David Miller @ 2013-09-04 18:54 UTC (permalink / raw)
  To: dborkman; +Cc: netdev
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

From: Daniel Borkmann <dborkman@redhat.com>
Date: Wed,  4 Sep 2013 00:19:36 +0200

> This set contains the non-RFC version of the previous submission entitled
> "[RFC PATCH net-next 0/7] IPv6 MLD updates". Most importantly it contains
> a fix for MLDv1/v2 switchback timeout where hosts currently are switching
> back from v1 compat mode to normal v2 too early (i.e. switchback time was
> <= 30secs instead of >= 260secs on default), and the set also contains a
> patch that allows for v2-only mode as per RFC recommendation. The rest is
> related to cleanups that make the code more readable resp. maintainable.

Series applied, thanks in particular for the 'net' merge conflict resolution
as that helps me a lot.

^ permalink raw reply

* Re: [PATCH] sh_eth: fix napi_{en|dis}able() calls racing against interrupts
From: David Miller @ 2013-09-04 18:56 UTC (permalink / raw)
  To: sergei.shtylyov; +Cc: netdev, nobuhiro.iwamatsu.yj, linux-sh
In-Reply-To: <201309040241.27834.sergei.shtylyov@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 4 Sep 2013 02:41:27 +0400

> While implementing NAPI for the driver, I overlooked the race conditions where
> interrupt  handler might have called napi_schedule_prep() before napi_enable()
> was called or after napi_disable() was called. If RX interrupt happens, this
> would cause the endless interrupts and messages like:
> 
> sh-eth eth0: ignoring interrupt, status 0x00040000, mask 0x01ff009f.
> 
> The interrupt wouldn't even be masked by the kernel eventually since the handler
> would return IRQ_HANDLED all the time.
> 
> As a fix, move napi_enable() call before request_irq() call and napi_disable()
> call after free_irq() call.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Applied and queued up for -stable, thanks!

^ permalink raw reply

* Re: [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: David Miller @ 2013-09-04 18:57 UTC (permalink / raw)
  To: dborkman; +Cc: netdev, eric.dumazet
In-Reply-To: <1378229352-9779-1-git-send-email-dborkman@redhat.com>

From: Daniel Borkmann <dborkman@redhat.com>
Date: Tue,  3 Sep 2013 19:29:12 +0200

> In tcp_v6_do_rcv() code, when processing pkt options, we soley work
> on our skb clone opt_skb that we've created earlier before entering
> tcp_rcv_established() on our way. However, only in condition ...
> 
>   if (np->rxopt.bits.rxtclass)
>     np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
> 
> ... we work on skb itself. As we extract every other information out
> of opt_skb in ipv6_pktoptions path, this seems wrong, since skb can
> already be released by tcp_rcv_established() earlier on. When we try
> to access it in ipv6_hdr(), we will dereference freed skb.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
From: David Miller @ 2013-09-04 18:58 UTC (permalink / raw)
  To: hannes; +Cc: michele, netdev, yoshfuji, sasha.levin
In-Reply-To: <20130904185145.GC13136@order.stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Wed, 4 Sep 2013 20:51:45 +0200

> Sorry I forgot to mention that this patch is also applicable to solve
> this issue in the longterm 3.10 kernel. Maybe this should go to stable?

I did queue it up for -stable already.

^ permalink raw reply

* Re: [PATCH 2/2] sh_eth: add device tree support
From: Sergei Shtylyov @ 2013-09-04 19:42 UTC (permalink / raw)
  To: Kumar Gala
  Cc: netdev, rob.herring, pawel.moll, mark.rutland, swarren,
	ian.campbell, grant.likely, devicetree, nobuhiro.iwamatsu.yj,
	linux-sh, rob, linux-doc
In-Reply-To: <E4C90BAA-A89F-4C7C-B186-12C2DF7E6F31@codeaurora.org>

Hello.

On 09/04/2013 02:08 AM, Kumar Gala wrote:

>> Add support of the device tree probing for the Renesas SH-Mobile SoCs.

>> This work is loosely based on an original patch by Nobuhiro Iwamatsu
>> <nobuhiro.iwamatsu.yj@renesas.com>.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> ---
>> This patch is against Dave's 'net-next.git' repo.

>> Documentation/devicetree/bindings/net/sh_eth.txt |   40 +++++++++++++
>> drivers/net/ethernet/renesas/sh_eth.c            |   66 ++++++++++++++++++++++-
>> 2 files changed, 105 insertions(+), 1 deletion(-)

>> Index: net-next/Documentation/devicetree/bindings/net/sh_eth.txt
>> ===================================================================
>> --- /dev/null
>> +++ net-next/Documentation/devicetree/bindings/net/sh_eth.txt
>> @@ -0,0 +1,40 @@
>> +* Renesas Electronics SH EtherMAC
>> +
>> +This file provides information on what the device node for the SH EtherMAC
>> +interface contains.
>> +
>> +Required properties:
>> +- compatible: "renesas,gether-r8a7740" if the device is a part of R8A7740 SoC.
>> +	      "renesas,ether-r8a7779" if the device is a part of R8A7778/9 SoCs.
>> +	      "renesas,ether-r8a7790" if the device is a part of R8A7790/1 SoCs.

> Curious, 7779 (vs 7778) but 7790 (vs 7791).

    Support for R8A7779 appeared in Linux before support for R8A7778 (probably 
the chips appeared on the market in the same order). The same can be said 
about R8A7790 -- it appeared first; R8A7791 isn't yet supported by Linux (it 
should be in 3.13). R8A7779/90 are high end chips, R8A7778/91 are middle class 
chips. Anyway, by request of Magnus Damm, I'll have to use the individual 
"compatible" properties.

>> +- reg: offset and length of the register set for the device; if the device has
>> +       TSU registers, you need to specify two register sets here.
>> +- interrupt-parent: the phandle for the interrupt controller that services
>> +		    interrupts for this device.
>> +- interrupts: interrupt mapping for the interrupt source.
>> +- phy-mode: string, operation mode of the PHY interface (a string that
>> +	    of_get_phy_mode() can understand).
>> +- phy-handle: phandle of the PHY device.
>> +

> Should add something about requiring a phy subnode

    OK.

>> +Optional properties:
>> +- local-mac-address: 6 bytes, MAC address.

> What's the assumption if local-mac-address is not specified?

    The driver will try to read the chip's MAC address registers hoping to 
find an address left there programmed by U-Boot. If no valid address is found 
(usual case on R8A7778/9 development boards which have an extra network chip), 
the random one will be used.

>> +- renesas,no-ether-link: specify when a board does not provide a proper LINK
>> +			 signal.

> How is this different from the fixed link concept we have?

    Here the chip-specific LINK signal is meant which is supposed to go from a 
PHY to the SoC. Sometimes it's wired from the PHY's link/active LED output or 
just absent on the board, so we aren't able to use it for the link detection.
In this case, phylib's notification is used.

>> +- renesas,ether-link-active-low: specify when the LINK signal is active-low.
>> +
>> +Example (Armadillo800EVA board):
>> +
>> +	ethernet@e9a00000 {
>> +		compatible = "renesas,gether-r8a7740";
>> +		reg = <0xe9a00000 0x800>, <0xe9a01800 0x800>;
>> +		interrupt-parent = <&gic>;
>> +		interrupts = <0 142 0x4>;
>> +		phy-mode = "mii";
>> +		phy-handle = <&phy0>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		phy0: ethernet-phy@0 {
>> +			reg = <0>;
>> +		};
>> +	};

> - k

WBR, Sergei


^ permalink raw reply

* Re: [PATCH RFC] solos-pci: Fix BUG() with shared skb
From: David Woodhouse @ 2013-09-04 20:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, simon, linux-atm-general, nathan
In-Reply-To: <20130904.143056.1558570610941220565.davem@davemloft.net>

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

On Wed, 2013-09-04 at 14:30 -0400, David Miller wrote:
> skb_realloc_headroom() should do everything you need.

Great, thanks! Something like this then... ? 

Do I really need the truesize check? And if so, is there a better way to
handle the ATM accounting? It just *happens* to be the case that the the
br2684_pop() and pppoatm_pop() functions don't mind being bypassed in
this fashion, and we should probably get away with tweaking the core ATM
accounting directly like this. Doesn't make me happy though...

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 32784d1..4492c0f 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -1145,19 +1145,19 @@ static int psend(struct atm_vcc *vcc, struct sk_buff *skb)
 		return 0;
 	}
 
-	if (!skb_clone_writable(skb, sizeof(*header))) {
-		int expand_by = 0;
-		int ret;
-
-		if (skb_headroom(skb) < sizeof(*header))
-			expand_by = sizeof(*header) - skb_headroom(skb);
-
-		ret = pskb_expand_head(skb, expand_by, 0, GFP_ATOMIC);
-		if (ret) {
-			dev_warn(&card->dev->dev, "pskb_expand_head failed.\n");
-			solos_pop(vcc, skb);
-			return ret;
-		}
+	if (skb_headroom(skb) < sizeof(*header)) {
+		struct sk_buff *nskb;
+
+		nskb = skb_realloc_headroom(skb, sizeof(*header));
+		if (!nskb) {
+			solos_pop(vcc, skb);
+			return -ENOMEM;
+		}
+		if (skb->truesize != nskb->truesize)
+			atm_force_charge(vcc, nskb->truesize - skb->truesize);
+ 
+		dev_kfree_skb_any(skb);
+		skb = nskb;
 	}
 
 	header = (void *)skb_push(skb, sizeof(*header));


-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]

^ permalink raw reply related

* [PATCH] net/core: Order-3 frag allocator causes SWIOTLB bouncing under Xen
From: Zoltan Kiss @ 2013-09-04 20:47 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, David S. Miller, Eric Dumazet, Neil Horman,
	Li Zefan, Eliezer Tamir, netdev, linux-kernel, malcolm.crossley,
	david.vrabel, xen-devel
  Cc: Zoltan Kiss

THIS PATCH IS NOT INTENDED TO BE UPSTREAMED, IT HAS ONLY INFORMING PURPOSES!

I've noticed a performance regression with upstream kernels when used as Dom0
under Xen. The classic kernel can utilize the whole bandwidth of a 10G NIC
(ca. 9.3 Gbps), but upstream can reach only ca. 7 Gbps. I found that it
happens because SWIOTLB has to do double buffering. The per task frag
allocator introduced in 5640f7 creates 32 kb frags, which are not contiguous
in mfn space.
This patch provides a workaround by going back to the old way. The possible
ideas came up to solve this:

* make sure Dom0 memory is contiguous: it sounds trivial, but doesn't work with
driver domains, and there are lots of situations where this is not possible.
* use PVH Dom0: so we will have IOMMU. In the future sometime.
* use IOMMU with PV Dom0: this seems to happen earlier.

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
 net/core/sock.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 2c097c5..854a0ea 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1812,7 +1812,7 @@ struct sk_buff *sock_alloc_send_skb(struct sock *sk, unsigned long size,
 EXPORT_SYMBOL(sock_alloc_send_skb);
 
 /* On 32bit arches, an skb frag is limited to 2^15 */
-#define SKB_FRAG_PAGE_ORDER	get_order(32768)
+#define SKB_FRAG_PAGE_ORDER	get_order(4096)
 
 bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
 {

^ permalink raw reply related

* Re: [PATCH] net/core: Order-3 frag allocator causes SWIOTLB bouncing under Xen
From: Eric Dumazet @ 2013-09-04 21:00 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: Konrad Rzeszutek Wilk, David S. Miller, Eric Dumazet, Neil Horman,
	Li Zefan, Eliezer Tamir, netdev, linux-kernel, malcolm.crossley,
	david.vrabel, xen-devel
In-Reply-To: <1378327638-23956-1-git-send-email-zoltan.kiss@citrix.com>

On Wed, 2013-09-04 at 21:47 +0100, Zoltan Kiss wrote:
> THIS PATCH IS NOT INTENDED TO BE UPSTREAMED, IT HAS ONLY INFORMING PURPOSES!
> 
> I've noticed a performance regression with upstream kernels when used as Dom0
> under Xen. The classic kernel can utilize the whole bandwidth of a 10G NIC
> (ca. 9.3 Gbps), but upstream can reach only ca. 7 Gbps. I found that it
> happens because SWIOTLB has to do double buffering. The per task frag
> allocator introduced in 5640f7 creates 32 kb frags, which are not contiguous
> in mfn space.
> This patch provides a workaround by going back to the old way. The possible
> ideas came up to solve this:
> 
> * make sure Dom0 memory is contiguous: it sounds trivial, but doesn't work with
> driver domains, and there are lots of situations where this is not possible.
> * use PVH Dom0: so we will have IOMMU. In the future sometime.
> * use IOMMU with PV Dom0: this seems to happen earlier.
> 
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> ---
>  net/core/sock.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 2c097c5..854a0ea 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1812,7 +1812,7 @@ struct sk_buff *sock_alloc_send_skb(struct sock *sk, unsigned long size,
>  EXPORT_SYMBOL(sock_alloc_send_skb);
>  
>  /* On 32bit arches, an skb frag is limited to 2^15 */
> -#define SKB_FRAG_PAGE_ORDER	get_order(32768)
> +#define SKB_FRAG_PAGE_ORDER	get_order(4096)
>  

Well, this hack is not new...

We have dev->gso_max_size and dev->gso_max_segs

We also have in net-next sk_pacing_rate and dynamic TSO sizing.

Maybe you could add proper infrastructure to deal with Xen limitations.

^ permalink raw reply

* Re: linux-next: Tree for Sep 4 (netfilter: xt_TPROXY)
From: Florian Westphal @ 2013-09-04 21:09 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Stephen Rothwell, linux-next, linux-kernel, netfilter-devel,
	netdev@vger.kernel.org
In-Reply-To: <52277212.5020006@infradead.org>

Randy Dunlap <rdunlap@infradead.org> wrote:
> On 09/04/13 01:13, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Please do not add any code for v3.13 to your linux-next included branches
> > until after v3.12-rc1 is released.
> > 
> > Changes since 20130902:
> > 
> 
> on x86_64:
> 
> when CONFIG_IPV6=m
> and CONFIG_NETFILTER_XT_TARGET_TPROXY=y:
> 
> net/built-in.o: In function `tproxy_tg6_v1':
> xt_TPROXY.c:(.text+0x5dc05): undefined reference to `udp6_lib_lookup'
> xt_TPROXY.c:(.text+0x5e32f): undefined reference to `udp6_lib_lookup'
> xt_TPROXY.c:(.text+0x5e432): undefined reference to `udp6_lib_lookup'
> net/built-in.o: In function `tproxy_tg_init':
> xt_TPROXY.c:(.init.text+0x1540): undefined reference to `nf_defrag_ipv6_enable'

Thanks for reporting.

I can reproduce same error with 3.10.6 stable tree, so its not
a recent problem.

As always, the tempting solution is to just forbid TPROXY=y with IPV6=m but
it would be better to get rid of the ipv6 link time deps..

^ permalink raw reply

* Re: [PATCH] net/core: Order-3 frag allocator causes SWIOTLB bouncing under Xen
From: Konrad Rzeszutek Wilk @ 2013-09-04 21:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Zoltan Kiss, David S. Miller, Eric Dumazet, Neil Horman, Li Zefan,
	Eliezer Tamir, netdev, linux-kernel, malcolm.crossley,
	david.vrabel, xen-devel
In-Reply-To: <1378328440.7360.124.camel@edumazet-glaptop>

On Wed, Sep 04, 2013 at 02:00:40PM -0700, Eric Dumazet wrote:
> On Wed, 2013-09-04 at 21:47 +0100, Zoltan Kiss wrote:
> > THIS PATCH IS NOT INTENDED TO BE UPSTREAMED, IT HAS ONLY INFORMING PURPOSES!
> > 
> > I've noticed a performance regression with upstream kernels when used as Dom0
> > under Xen. The classic kernel can utilize the whole bandwidth of a 10G NIC
> > (ca. 9.3 Gbps), but upstream can reach only ca. 7 Gbps. I found that it
> > happens because SWIOTLB has to do double buffering. The per task frag
> > allocator introduced in 5640f7 creates 32 kb frags, which are not contiguous
> > in mfn space.
> > This patch provides a workaround by going back to the old way. The possible
> > ideas came up to solve this:
> > 
> > * make sure Dom0 memory is contiguous: it sounds trivial, but doesn't work with
> > driver domains, and there are lots of situations where this is not possible.
> > * use PVH Dom0: so we will have IOMMU. In the future sometime.
> > * use IOMMU with PV Dom0: this seems to happen earlier.
> > 
> > Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> > ---
> >  net/core/sock.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index 2c097c5..854a0ea 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -1812,7 +1812,7 @@ struct sk_buff *sock_alloc_send_skb(struct sock *sk, unsigned long size,
> >  EXPORT_SYMBOL(sock_alloc_send_skb);
> >  
> >  /* On 32bit arches, an skb frag is limited to 2^15 */
> > -#define SKB_FRAG_PAGE_ORDER	get_order(32768)
> > +#define SKB_FRAG_PAGE_ORDER	get_order(4096)
> >  
> 
> Well, this hack is not new...
> 
> We have dev->gso_max_size and dev->gso_max_segs
> 
> We also have in net-next sk_pacing_rate and dynamic TSO sizing.
> 
> Maybe you could add proper infrastructure to deal with Xen limitations.

I think Ian posted at some point an sysctl patch for that (more for
debugging that anything else). And it kind
of stalled: http://lists.xen.org/archives/html/xen-devel/2012-10/msg01832.html

Is that what you mean by proper infrastructure ?

Oh wait, did you mean via dev and not the whole system wide sysctl?

> 
> 
> 

^ permalink raw reply

* [PATCH net] vxlan: Fix kernel panic on device delete.
From: Pravin B Shelar @ 2013-09-04 21:12 UTC (permalink / raw)
  To: netdev; +Cc: Pravin B Shelar

On vxlan device create if socket create fails vxlan device is not
added to hash table. Therefore we need to check if device
is in hashtable before we delete it from hlist.
Following patch avoid the crash. net-next already has this fix.

---8<---
BUG: unable to handle kernel NULL pointer dereference at           (null)
IP: [<ffffffffa05f9ca7>] vxlan_dellink+0x77/0xf0 [vxlan]
PGD 42b2d9067 PUD 42e04c067 PMD 0
Oops: 0002 [#1] SMP
Modules linked in: vxlan(-)
Hardware name: Dell Inc. PowerEdge R620/0KCKR5, BIOS 1.4.8 10/25/2012
task: ffff88042ecf8760 ti: ffff88042f106000 task.ti: ffff88042f106000
RIP: 0010:[<ffffffffa05f9ca7>]  [<ffffffffa05f9ca7>]
vxlan_dellink+0x77/0xf0 [vxlan]
RSP: 0018:ffff88042f107e28  EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff88082af08000 RCX: ffff88083fd80000
RDX: 0000000000000000 RSI: ffff88042f107e58 RDI: ffff88042e12f810
RBP: ffff88042f107e48 R08: ffffffff8166eca0 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffff88082af087c0
R13: ffff88042e12f000 R14: ffff88042f107e58 R15: ffff88042f107e58
FS:  00007f4ed2de7700(0000) GS:ffff88043fc80000(0000)
knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000042e076000 CR4: 00000000000407e0
Stack:
 ffff88082af08000 ffffffff81654848 ffffffffa05fb4e0 ffffffff81654780
 ffff88042f107e98 ffffffff813b9c7a ffff88042f107e58 ffff88042f107e58
 ffff88042f107e88 ffffffffa05fb4e0 ffffffffa05fb780 ffff88042f107f18
Call Trace:
 [<ffffffff813b9c7a>] __rtnl_link_unregister+0xca/0xd0
 [<ffffffff813bb0e9>] rtnl_link_unregister+0x19/0x30
 [<ffffffffa05faa4c>] vxlan_cleanup_module+0x10/0x2f [vxlan]
 [<ffffffff81099fef>] SyS_delete_module+0x1cf/0x2c0
 [<ffffffff8146c069>] ? do_page_fault+0x9/0x10
 [<ffffffff8146f012>] system_call_fastpath+0x16/0x1b
Code: 4d 85 ed 0f 84 95 00 00 00 4c 8d a7 c0 07 00 00 49 8d bd 10 08 00
00 e8 28 e8 e6 e0 48 8b 83 c0 07 00 00 49 8b 54 24 08 48 85 c0 <48> 89
02 74 04 48 89 50 08 49 b8 00 02 20 00 00 00 ad de 4d 89
RIP  [<ffffffffa05f9ca7>] vxlan_dellink+0x77/0xf0 [vxlan]
 RSP <ffff88042f107e28>
CR2: 0000000000000000

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 drivers/net/vxlan.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 767f7af..2c15f6c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1794,7 +1794,8 @@ static void vxlan_dellink(struct net_device *dev, struct list_head *head)
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 
 	spin_lock(&vn->sock_lock);
-	hlist_del_rcu(&vxlan->hlist);
+	if (!hlist_unhashed(&vxlan->hlist))
+		hlist_del_rcu(&vxlan->hlist);
 	spin_unlock(&vn->sock_lock);
 
 	list_del(&vxlan->next);
-- 
1.7.1

^ permalink raw reply related

* [PATCH] can: pcan_usb_core: fix memory leak on failure paths in peak_usb_start()
From: Alexey Khoroshilov @ 2013-09-04 21:46 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Alexey Khoroshilov, Wolfgang Grandegger, linux-can, netdev,
	linux-kernel, ldv-project

Tx and rx urbs are not deallocated if something goes wrong in peak_usb_start().
The patch fixes error handling to deallocate all the resources.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/net/can/usb/peak_usb/pcan_usb_core.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index a0f647f..0b7a4c3 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -463,7 +463,7 @@ static int peak_usb_start(struct peak_usb_device *dev)
 	if (i < PCAN_USB_MAX_TX_URBS) {
 		if (i == 0) {
 			netdev_err(netdev, "couldn't setup any tx URB\n");
-			return err;
+			goto err_tx;
 		}
 
 		netdev_warn(netdev, "tx performance may be slow\n");
@@ -472,7 +472,7 @@ static int peak_usb_start(struct peak_usb_device *dev)
 	if (dev->adapter->dev_start) {
 		err = dev->adapter->dev_start(dev);
 		if (err)
-			goto failed;
+			goto err_adapter;
 	}
 
 	dev->state |= PCAN_USB_STATE_STARTED;
@@ -481,19 +481,26 @@ static int peak_usb_start(struct peak_usb_device *dev)
 	if (dev->adapter->dev_set_bus) {
 		err = dev->adapter->dev_set_bus(dev, 1);
 		if (err)
-			goto failed;
+			goto err_adapter;
 	}
 
 	dev->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	return 0;
 
-failed:
+err_adapter:
 	if (err == -ENODEV)
 		netif_device_detach(dev->netdev);
 
 	netdev_warn(netdev, "couldn't submit control: %d\n", err);
 
+	for (i = 0; i < PCAN_USB_MAX_TX_URBS; i++) {
+		usb_free_urb(dev->tx_contexts[i].urb);
+		dev->tx_contexts[i].urb = NULL;
+	}
+err_tx:
+	usb_kill_anchored_urbs(&dev->rx_submitted);
+
 	return err;
 }
 
-- 
1.8.1.2


^ permalink raw reply related

* Re: [PATCH RFC] solos-pci: Fix BUG() with shared skb
From: David Miller @ 2013-09-04 21:51 UTC (permalink / raw)
  To: dwmw2; +Cc: netdev, simon, linux-atm-general, nathan
In-Reply-To: <1378327275.2627.28.camel@shinybook.infradead.org>

From: David Woodhouse <dwmw2@infradead.org>
Date: Wed, 04 Sep 2013 21:41:15 +0100

> On Wed, 2013-09-04 at 14:30 -0400, David Miller wrote:
>> skb_realloc_headroom() should do everything you need.
> 
> Great, thanks! Something like this then... ? 
> 
> Do I really need the truesize check? And if so, is there a better way to
> handle the ATM accounting? It just *happens* to be the case that the the
> br2684_pop() and pppoatm_pop() functions don't mind being bypassed in
> this fashion, and we should probably get away with tweaking the core ATM
> accounting directly like this. Doesn't make me happy though...
 ...
> +		nskb = skb_realloc_headroom(skb, sizeof(*header));
> +		if (!nskb) {
> +			solos_pop(vcc, skb);
> +			return -ENOMEM;
> +		}
> +		if (skb->truesize != nskb->truesize)
> +			atm_force_charge(vcc, nskb->truesize - skb->truesize);

My understanding is that truesize will not be changed by calls to
skb_realloc_headroom(), because if it did then every ethernet driver
would be screwed as the skb->truesize adjustment would corrupt socket
memory accounting.

The only thing you need to be mindful is that after the
skb_realloc_headroom() call all skb data pointers change, and
therefore things like packet pointers are now stale and need to be
recalculated.

^ permalink raw reply


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