Netdev List
 help / color / mirror / Atom feed
* Re: question about netif_rx
From: David Shwatrz @ 2013-08-13  6:53 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Francois Romieu, grant.likely, rob.herring, netdev, linux-kernel,
	devicetree
In-Reply-To: <alpine.DEB.2.02.1308130828550.1997@localhost6.localdomain6>

Hello,
Sorry. I still don't understand what checksum has to do with it.
Does GRO depends on Rx/Tx checksum ? I don't think so.

In the napi_gro_receive() we check that the device supports
NETIF_F_GRO, but I don't see that we inspect checksum or that
NETIF_F_GRO is depends on checksum.

Can you please explain how checsum offload is related ?

rgs
David

On Tue, Aug 13, 2013 at 9:29 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Tue, 13 Aug 2013, Francois Romieu wrote:
>
>> Julia Lawall <julia.lawall@lip6.fr> :
>> > François Romieu <romieu@fr.zoreil.com> :
>> [...]
>> > > Can you send a netif_receive_skb replacement patch for it ?
>> >
>> > Just to be sure, I just replace netif_rx by netif_receive_skb, nothing
>> > else?
>>
>> Yes. It should imho be fine with a comment incluing your analysis and a
>> few words about the current state of checksum offloading support.
>
> I wouldn't know what to say about the checksum part.  It is not supported
> so I should use netif_receive_skb?
>
> thanks,
> julia

^ permalink raw reply

* Re: [PATCH 2/2] ipv6: fix checkpatch errors in net/ipv6/addrconf.c
From: Joe Perches @ 2013-08-13  6:30 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, Netdev
In-Reply-To: <5209CEEC.2050505@huawei.com>

On Tue, 2013-08-13 at 14:15 +0800, Ding Tianhong wrote:
> ERROR: Macros with complex values should be enclosed in parenthesis
> +#define ADBG(x) printk x
[]
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
[]
> @@ -99,7 +99,7 @@
[]
> -#define ADBG(x) printk x
> +#define ADBG(x) (printk x)

A better way to fix this would be
to convert the uses of ADBG and remove
the superfluous parentheses.

Something like the below but it might
even better to remove the KERN_<LEVEL>s and
just use pr_debug instead of ADBG...
---
 net/ipv6/addrconf.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 7fd8572..1286a7d 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -99,9 +99,9 @@
 #define ACONF_DEBUG 2
 
 #if ACONF_DEBUG >= 3
-#define ADBG(x) printk x
+#define ADBG(fmt, ...) printk(fmt, ##__VA_ARGS__)
 #else
-#define ADBG(x)
+#define ADBG(fmt, ...) do { if (0) printk(fmt, ##__VA_ARGS__); } while (0)
 #endif
 
 #define	INFINITY_LIFE_TIME	0xFFFFFFFF
@@ -369,9 +369,9 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 	dev_hold(dev);
 
 	if (snmp6_alloc_dev(ndev) < 0) {
-		ADBG((KERN_WARNING
-			"%s: cannot allocate memory for statistics; dev=%s.\n",
-			__func__, dev->name));
+		ADBG(KERN_WARNING
+		     "%s: cannot allocate memory for statistics; dev=%s\n",
+		     __func__, dev->name);
 		neigh_parms_release(&nd_tbl, ndev->nd_parms);
 		dev_put(dev);
 		kfree(ndev);
@@ -379,9 +379,8 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 	}
 
 	if (snmp6_register_dev(ndev) < 0) {
-		ADBG((KERN_WARNING
-			"%s: cannot create /proc/net/dev_snmp6/%s\n",
-			__func__, dev->name));
+		ADBG(KERN_WARNING "%s: cannot create /proc/net/dev_snmp6/%s\n",
+		     __func__, dev->name);
 		neigh_parms_release(&nd_tbl, ndev->nd_parms);
 		ndev->dead = 1;
 		in6_dev_finish_destroy(ndev);
@@ -844,7 +843,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 
 	/* Ignore adding duplicate addresses on an interface */
 	if (ipv6_chk_same_addr(dev_net(idev->dev), addr, idev->dev)) {
-		ADBG(("ipv6_add_addr: already assigned\n"));
+		ADBG("ipv6_add_addr: already assigned\n");
 		err = -EEXIST;
 		goto out;
 	}
@@ -852,7 +851,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 	ifa = kzalloc(sizeof(struct inet6_ifaddr), GFP_ATOMIC);
 
 	if (ifa == NULL) {
-		ADBG(("ipv6_add_addr: malloc failed\n"));
+		ADBG("ipv6_add_addr: malloc failed\n");
 		err = -ENOBUFS;
 		goto out;
 	}
@@ -2052,7 +2051,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 	pinfo = (struct prefix_info *) opt;
 
 	if (len < sizeof(struct prefix_info)) {
-		ADBG(("addrconf: prefix option too short\n"));
+		ADBG("addrconf: prefix option too short\n");
 		return;
 	}
 
@@ -3632,8 +3631,8 @@ restart:
 	if (time_before(next_sched, jiffies + ADDRCONF_TIMER_FUZZ_MAX))
 		next_sched = jiffies + ADDRCONF_TIMER_FUZZ_MAX;
 
-	ADBG((KERN_DEBUG "now = %lu, schedule = %lu, rounded schedule = %lu => %lu\n",
-	      now, next, next_sec, next_sched));
+	ADBG(KERN_DEBUG "now = %lu, schedule = %lu, rounded schedule = %lu => %lu\n",
+	     now, next, next_sec, next_sched);
 
 	addr_chk_timer.expires = next_sched;
 	add_timer(&addr_chk_timer);

^ permalink raw reply related

* Re: question about netif_rx
From: Julia Lawall @ 2013-08-13  6:29 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Julia Lawall, grant.likely, rob.herring, netdev, linux-kernel,
	devicetree
In-Reply-To: <20130813052036.GA11640@electric-eye.fr.zoreil.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 565 bytes --]

On Tue, 13 Aug 2013, Francois Romieu wrote:

> Julia Lawall <julia.lawall@lip6.fr> :
> > François Romieu <romieu@fr.zoreil.com> :
> [...]
> > > Can you send a netif_receive_skb replacement patch for it ?
> > 
> > Just to be sure, I just replace netif_rx by netif_receive_skb, nothing
> > else?
> 
> Yes. It should imho be fine with a comment incluing your analysis and a
> few words about the current state of checksum offloading support.

I wouldn't know what to say about the checksum part.  It is not supported 
so I should use netif_receive_skb?

thanks,
julia

^ permalink raw reply

* Re: question about netif_rx
From: Francois Romieu @ 2013-08-13  6:28 UTC (permalink / raw)
  To: David Shwatrz
  Cc: Julia Lawall, grant.likely, rob.herring, netdev, linux-kernel,
	devicetree
In-Reply-To: <CAJJAcoe+GpRYqHyu1xctRaUnPXf4CP8=PTt=svJzJFtBjfqxrg@mail.gmail.com>

David Shwatrz <dshwatrz@gmail.com> :
[...]
> what is the current state of checksum offloading support has to do
> with it ? maybe you meant current state of NAPI support ?

netif_receive_skb vs napi_gro_receive choice.

-- 
Ueimor

^ permalink raw reply

* [PATCH 1/1] net/e1000e: Avoid kernel crash during shutdown
From: Li Zhang @ 2013-08-13  6:27 UTC (permalink / raw)
  To: jeffrey.t.kirsher, jesse.brandeburg, davem, netdev; +Cc: shangw

While doing shutdown on the PCI device, the corresponding callback
function e1000e_shutdown() is trying to clear those correctable
errors on the upstream P2P bridge. Unfortunately, we don't have
the upstream P2P bridge under some cases (e.g. PCI-passthrou for
KVM on Power). That leads to kernel crash eventually.

The patch adds one more check on that to avoid kernel crash.

Signed-off-by: Li Zhang <zhlcindy@gmail.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 77f81cb..232b2e8 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5999,11 +5999,18 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime)
 	 * correctable error when the MAC transitions from D0 to D3.  To
 	 * prevent this we need to mask off the correctable errors on the
 	 * downstream port of the pci-e switch.
+	 *
+	 * We don't have the associated upstream bridge while assigning
+	 * the PCI device into guest. For example, the KVM on power is
+	 * one of the cases.
 	 */
 	if (adapter->flags & FLAG_IS_QUAD_PORT) {
 		struct pci_dev *us_dev = pdev->bus->self;
 		u16 devctl;
 
+		if (!us_dev)
+			return 0;
+
 		pcie_capability_read_word(us_dev, PCI_EXP_DEVCTL, &devctl);
 		pcie_capability_write_word(us_dev, PCI_EXP_DEVCTL,
 					   (devctl & ~PCI_EXP_DEVCTL_CERE));
-- 
1.8.1.4

^ permalink raw reply related

* Re: ipsec smp scalability and cpu use fairness (softirqs)
From: Timo Teras @ 2013-08-13  6:23 UTC (permalink / raw)
  To: Andrew Collins; +Cc: netdev
In-Reply-To: <CAKTPYJTVhvF=215eS0xriEdUyyCLaC+zzja4zPrG668ALzERyw@mail.gmail.com>

On Mon, 12 Aug 2013 15:58:41 -0600
Andrew Collins <bsderandrew@gmail.com> wrote:

> On Mon, Aug 12, 2013 at 7:01 AM, Timo Teras <timo.teras@iki.fi> wrote:
> > 1. Single core systems that are going out of cpu power, are
> > overwhelmed in uncontrollable manner. As softirq is doing the heavy
> > lifting, the user land processes are starved first. This can cause
> > userland IKE daemon to starve and lose tunnels when it is unable to
> > answer liveliness checks. The quick workaround is to setup traffic
> > shaping for the encrypted traffic.
> 
> Which kernel version are you on?  I've found I've had better behavior
> since:
>
> commit c10d73671ad30f54692f7f69f0e09e75d3a8926a
> Author: Eric Dumazet <edumazet@google.com>
> Date:   Thu Jan 10 15:26:34 2013 -0800
> 
>     softirq: reduce latencies
> 
> as it bails from lengthy softirq processing much earlier, along with

The user process starvation observations are originally from 3.3/3.4
kernels, and I have not retested properly yet with newer ones.
Currently starting upgrades to 3.10. That commit looks like it will
directly fix mostly single core starvation issues.

I think netdev_budget mostly affects latencies for other softirqs,
since the rx softirq will be practically always on during the stress.

And it can still cause problems that encrypted and non-encrypted
packets still go through same queues. This means that when we are out
cpu, we can start dropping even non-encrypted packets early.

> tuning "netdev_budget" to avoid cycling for too long in the NAPI poll.
> 
> > 2. On multicore (6-12 cores) systems, it would appear that it is not
> > easy to distribute the ipsec to multiple cores. as softirq is
> > sticky to the cpu where it was raised. The ipsec
> > decryption/encryption is done synchronously in the napi poll loop,
> > and the throughput is limited by one cpu. If the NIC supports
> > multiple queues and balancing with ESP SPI, we can use that to get
> > some parallelism.
> 
> Although it's highly usecase dependent, I've had good luck using
> RPS.  I'm testing as an ipsec router however, not with an endpoint
> on the host itself, so it processes nearly all ipsec traffic in
> receive context.

Yes, RPS will help on many scenarios but not all. The flow dissector
knows only IP/TCP/UDP/GRE, but not ESP. So as long as traffic is
distributed between different IP-addresses, it is distributed. But if I
have lot of traffic between two nodes either with different ESP SPI
(different gatewayed subnets), or even with same SPI, then it won't.

For my scenario it will be usually even same SPI. So even if flow
dissector learns ESP and uses SPI in hash, I'd need a way to balance
traffic to multiple SAs.

I guess the place where I'd want to see the distribution to cores is
crypto_aead_*() calls. In fact, it seems there's code infracture
already for it: crypto/cryptd.c. Seems it needs to be manually
configured and only few places e.g. aesni gcm parts use it.

I'm wondering if it'd make sense to patch net/xfrm/xfrm_algo.c to use
cryptd? Or at least have a Kconfig or sysctl option make it do so.

- Timo

^ permalink raw reply

* [PATCH 1/2] ipv6: do not disable temp_address when reaching max_address
From: Ding Tianhong @ 2013-08-13  6:14 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Netdev, Patrick McHardy

A LAN user can remotely disable temporary address which may lead
to privacy violatins and information disclosure.

The reason is that the linux kernel uses the 'ipv6.max_addresses'
option to specify how many ipv6 addresses and interface may have.
The 'ipv6.regen_max_retry' (default value 3) option specifies
how many times the kernel will try to create a new address.

But the kernel is not distinguish between the event of reaching
max_addresses for an interface and failing to generate a new address.
the kernel disable the temporary address after regenerate a new
address 'regen_max_retry' times.

According RFC4941 3.3.7:

---------------------------------------

If DAD indicates the address is already in use,
the node must generate a new randomized interface
identifier as described in section 3.2 above, and
repeat the previous steps as appropriate up to
TEMP_IDGEN_RETRIES times.

If after TEMP_IDGEN_RETRIES consecutive attempts no
non-unique address was generated, the node must log
a system error and must not attempt to generate
temporary address for that interface.

------------------------------------------

RFC4941 3.3.7 specifies that disabling the temp_address must happen
upon the address is already in use, not reach the max_address,
So we have to check the return err and distinguish the correct retry path.

This fixes CVE-2013-0343

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
Tested-by: Wang Weidong <wangweidong1@huawei.com>
Cc: David S. Miller <davem@davemloft.net>
---
 net/ipv6/addrconf.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index da4241c..99f9fd5 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1134,10 +1134,27 @@ retry:
 	if (IS_ERR_OR_NULL(ift)) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
-		pr_info("%s: retry temporary address regeneration\n", __func__);
-		tmpaddr = &addr;
-		write_lock(&idev->lock);
-		goto retry;
+
+		/* According RFC4941 3.3.7:
+		 * If DAD indicates the address is already in use,
+		 * the node must generate a new randomized interface
+		 * identifier as described in section 3.2 above, and
+		 * repeat the previous steps as appropriate up to
+		 * TEMP_IDGEN_RETRIES times.
+		 * If after TEMP_IDGEN_RETRIES consecutive attempts no
+		 * non-unique address was generated, the node must log
+		 * a system error and must not attempt to generate
+		 * temporary address for that interface.
+		 * So we have to check the return err and distinguish
+		 * the correct retry path.
+		 */
+		if (PTR_ERR(ift) == -EEXIST) {
+			pr_info("%s: retry temporary address regeneration\n", __func__);
+			tmpaddr = &addr;
+			write_lock(&idev->lock);
+			goto retry;
+		} else
+			goto out;
 	}
 
 	spin_lock_bh(&ift->lock);
-- 
1.8.2.1

^ permalink raw reply related

* [PATCH 2/2] ipv6: fix checkpatch errors in net/ipv6/addrconf.c
From: Ding Tianhong @ 2013-08-13  6:15 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, Netdev

ERROR: Macros with complex values should be enclosed in parenthesis
+#define ADBG(x) printk x

ERROR: code indent should use tabs where possible
+^I                idev->cnf.dad_transmits *$

ERROR: code indent should use tabs where possible
+^I                idev->nd_parms->retrans_time / HZ;$

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 net/ipv6/addrconf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 99f9fd5..5aaf931 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -99,7 +99,7 @@
 #define ACONF_DEBUG 2
 
 #if ACONF_DEBUG >= 3
-#define ADBG(x) printk x
+#define ADBG(x) (printk x)
 #else
 #define ADBG(x)
 #endif
@@ -1105,8 +1105,8 @@ retry:
 	spin_unlock_bh(&ifp->lock);
 
 	regen_advance = idev->cnf.regen_max_retry *
-	                idev->cnf.dad_transmits *
-	                idev->nd_parms->retrans_time / HZ;
+			idev->cnf.dad_transmits *
+			idev->nd_parms->retrans_time / HZ;
 	write_unlock(&idev->lock);
 
 	/* A temporary address is created only if this calculated Preferred
-- 
1.8.2.1

^ permalink raw reply related

* Re: question about netif_rx
From: David Shwatrz @ 2013-08-13  5:45 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Julia Lawall, grant.likely, rob.herring, netdev, linux-kernel,
	devicetree
In-Reply-To: <20130813052036.GA11640@electric-eye.fr.zoreil.com>

Hello,
what is the current state of checksum offloading support has to do
with it ? maybe you meant current state of NAPI support ?

regards,
DS

On Tue, Aug 13, 2013 at 8:20 AM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Julia Lawall <julia.lawall@lip6.fr> :
>> François Romieu <romieu@fr.zoreil.com> :
> [...]
>> > Can you send a netif_receive_skb replacement patch for it ?
>>
>> Just to be sure, I just replace netif_rx by netif_receive_skb, nothing
>> else?
>
> Yes. It should imho be fine with a comment incluing your analysis and a
> few words about the current state of checksum offloading support.
>
> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: question about netif_rx
From: Francois Romieu @ 2013-08-13  5:20 UTC (permalink / raw)
  To: Julia Lawall; +Cc: grant.likely, rob.herring, netdev, linux-kernel, devicetree
In-Reply-To: <alpine.DEB.2.02.1308111811440.2072@localhost6.localdomain6>

Julia Lawall <julia.lawall@lip6.fr> :
> François Romieu <romieu@fr.zoreil.com> :
[...]
> > Can you send a netif_receive_skb replacement patch for it ?
> 
> Just to be sure, I just replace netif_rx by netif_receive_skb, nothing
> else?

Yes. It should imho be fine with a comment incluing your analysis and a
few words about the current state of checksum offloading support.

-- 
Ueimor

^ permalink raw reply

* Re: Where are OVS VXLAN patches?
From: Cong Wang @ 2013-08-13  5:16 UTC (permalink / raw)
  To: Jesse Gross; +Cc: David S. Miller, netdev, Pravin B Shelar, Stephen Hemminger
In-Reply-To: <CAEP_g=-JQUZaLUhkMWQ6p8iYn8jqb_0tZOq=x+kv0Xip3dUA7A@mail.gmail.com>

On Mon, 2013-08-12 at 09:59 -0700, Jesse Gross wrote:
> On Mon, Aug 12, 2013 at 12:07 AM, Cong Wang <amwang@redhat.com> wrote:
> > Hello, David
> >
> > I am wondering where are the patches for OVS VXLAN support from Pravin?
> > v7 were posted one week ago:
> > http://marc.info/?l=linux-netdev&m=137572485116601&w=2
> >
> > and no one complains about them, so I think they are ready for merge?
> >
> > But they still do not appear in net-next, nor I can find them in vxlan
> > tree of Stephen or openvswitch tree of Jesse. What's more, I even can't
> > find them in netdev patchwork [1].
> 
> I believe that Stephen is planning on taking the first part of the
> series through his VXLAN tree.

I can't find them here:
https://git.kernel.org/cgit/linux/kernel/git/shemminger/vxlan-next.git

Maybe Stephen hasn't pushed them.

Thanks.

^ permalink raw reply

* Re: [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
From: David Miller @ 2013-08-13  5:14 UTC (permalink / raw)
  To: vyasevich; +Cc: dborkman, netdev, linux-sctp
In-Reply-To: <5204FFDD.5060703@gmail.com>

From: Vlad Yasevich <vyasevich@gmail.com>
Date: Fri, 09 Aug 2013 10:42:37 -0400

> On 08/09/2013 10:25 AM, Daniel Borkmann wrote:
>> Probably this one is quite unlikely to be triggered, but it's more
>> safe
>> to do the call_rcu() at the end after we have dropped the reference on
>> the asoc and freed sctp packet chunks. The reason why is because in
>> sctp_transport_destroy_rcu() the transport is being kfree()'d, and if
>> we're unlucky enough we could run into corrupted pointers. Probably
>> that's more of theoretical nature, but it's safer to have this simple
>> fix.
>>
>> Introduced by commit 8c98653f ("sctp: sctp_close: fix release of
>> bindings
>> for deferred call_rcu's"). I also did the 8c98653f regression test and
>> it's fine that way.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> 
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH net] net: sctp: sctp_assoc_control_transport: fix MTU size in SCTP_PF state
From: David Miller @ 2013-08-13  5:12 UTC (permalink / raw)
  To: vyasevich; +Cc: dborkman, kheiss, netdev, linux-sctp, nhorman
In-Reply-To: <5204F121.7080609@gmail.com>

From: Vlad Yasevich <vyasevich@gmail.com>
Date: Fri, 09 Aug 2013 09:39:45 -0400

> On 08/09/2013 09:09 AM, Daniel Borkmann wrote:
>> The SCTP Quick failover draft [1] section 5.1, point 5 says that the
>> cwnd
>> should be 1 MTU. So, instead of 1, set it to 1 MTU.
>>
>>    [1] https://tools.ietf.org/html/draft-nishida-tsvwg-sctp-failover-05
>>
>> Reported-by: Karl Heiss <kheiss@gmail.com>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> Cc: Neil Horman <nhorman@tuxdriver.com>
 ...
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] stmmac: fix init_dma_desc_rings() to handle errors
From: David Miller @ 2013-08-13  5:11 UTC (permalink / raw)
  To: b.zolnierkie; +Cc: peppe.cavallaro, netdev
In-Reply-To: <1392383.ZcinuGX9SF@amdc1032>

From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Date: Fri, 09 Aug 2013 14:02:08 +0200

> In stmmac_init_rx_buffers():
> * add missing handling of dma_map_single() error
> * remove superfluous unlikely() optimization while at it
> 
> Add stmmac_free_rx_buffers() helper and use it in dma_free_rx_skbufs().
> 
> In init_dma_desc_rings():
> * add missing handling of kmalloc_array() errors
> * fix handling of dma_alloc_coherent() and stmmac_init_rx_buffers() errors
> * make function return an error value on error and 0 on success
> 
> In stmmac_open():
> * add handling of init_dma_desc_rings() return value
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 3/4] drivers/net: enic: Make ASIC information available to USNIC
From: Nishank Trivedi @ 2013-08-13  5:10 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Stephen Hemminger, Neel Patel, netdev, Christian Benvenuti,
	Upinder Malhi (umalhi)
In-Reply-To: <1376151205.32005.2.camel@deadeye.wl.decadent.org.uk>

On 8/10/13 9:13 AM, Ben Hutchings wrote:
> On Fri, 2013-08-09 at 15:21 -0700, Stephen Hemminger wrote:
>> On Fri,  9 Aug 2013 11:12:20 -0700
>> Neel Patel <neepatel@cisco.com> wrote:
>>
>>> This patch provides asic information via ethtool.
> [...]
>>> --- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
>>> +++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
>>> @@ -19,6 +19,7 @@
>>>   #include <linux/netdevice.h>
>>>   #include <linux/ethtool.h>
>>>
>>> +#include "driver_utils.h"
>>>   #include "enic_res.h"
>>>   #include "enic.h"
>>>   #include "enic_dev.h"
>>> @@ -116,6 +117,9 @@ static void enic_get_drvinfo(struct net_device *netdev,
>>>   		sizeof(drvinfo->fw_version));
>>>   	strlcpy(drvinfo->bus_info, pci_name(enic->pdev),
>>>   		sizeof(drvinfo->bus_info));
>>> +	memset(drvinfo->reserved1, 0, sizeof(drvinfo->reserved1));
>>> +	driver_encode_asic_info(drvinfo->reserved1, sizeof(drvinfo->reserved1),
>>> +		fw_info->asic_type, fw_info->asic_rev);
>>>   }
>>
>> If you want to use a reserved field, then make it a first class citizen.
>> Rename it to asic_info, make sure the result is okay for other drivers
>> and add send patch so Ben can make it part of normal ethtool support.
>>
>> Otherwise, this code is likely to break when someone else actually unreserves
>> that field.
>
> Right.  I bet this is redundant with the IDs that lspci can show,
> anyway.

Thanks Stephen, Ben for your input, they are valid points. Neel would 
send a new patch series minus 3/4 for now.

While you are right that lspci or sysfs can be used to get same info, we 
were trying to use asic info (encoded with type and version) within 
drvinfo so as to use one string to achieve same effect as reading PCI 
subsystem id and revision explicitly. Instead of going to different tool 
(lspci), ethtool would be enough to unqiuely identify the device. Asic 
version along with already existing firmware version, driver version, 
etc seems natural.

Thanks,
nishank

^ permalink raw reply

* Re: [PATCH v2 3.11] genetlink: fix family dump race
From: David Miller @ 2013-08-13  5:06 UTC (permalink / raw)
  To: johannes
  Cc: linux-wireless, netdev, tgraf, andrei.otcheretianski, ilan.peer,
	stable, pshelar
In-Reply-To: <1376297697.11514.8.camel@jlt4.sipsolutions.net>

From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 12 Aug 2013 10:54:57 +0200

> I think for the current code the fix would still be correct, I can
> change the commit message if you like.

I agree, please update the commit message and resubmit.

> For backporting, we'll have to check which tree has Pravin's change
> and which doesn't and change this accordingly, I suppose.

This is really tricky, because we'd need to also backport the fixes
for the regressions added by Pravin's change such as:

50754d2188b04a679a249fb57751542643a436e0
c74f2b2678f40b80265dd53556f1f778c8e1823f

^ permalink raw reply

* Re: [PATCH net v5 1/5] bnx2x: protect different statistics flows
From: David Miller @ 2013-08-13  4:24 UTC (permalink / raw)
  To: stephen; +Cc: dmitry, netdev, eilong, ariele
In-Reply-To: <20130812171208.7dde2569@nehalam.linuxnetplumber.net>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 12 Aug 2013 17:12:08 -0700

> On Tue, 13 Aug 2013 02:24:59 +0300
> "Dmitry Kravkov" <dmitry@broadcom.com> wrote:
> 
>> +	bool					stats_started;
>> +	struct semaphore			stats_sema;
> 
> Is there a reason to use a counting semaphore? Do you expect it to
> be held across user process boundary? or want count > 1?
> 
> Use of semaphores as a locking primitive is discouraged,
> instead us a mutex.

Would you please look at the feedback I gave these guys to the
previous iteration of these changes?

They were using custom locking primitives and semaphores gave
the best approximation to the semantics they were looking for.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next] tcp: reset reordering est. selectively on timeout
From: Neal Cardwell @ 2013-08-13  4:16 UTC (permalink / raw)
  To: Yuchung Cheng; +Cc: David Miller, Eric Dumazet, Matt Mathis, Netdev
In-Reply-To: <1376350885-32407-1-git-send-email-ycheng@google.com>

On Mon, Aug 12, 2013 at 7:41 PM, Yuchung Cheng <ycheng@google.com> wrote:
> On timeout the TCP sender unconditionally reset the estimated
> degree of network reordering (tp->reordering). This idea behind
> is that the estimate is too large to trigger fast recovery
> (e.g., due to a IP path change).
>
> But for example if the sender only had 2 packets outstanding,
> then a timeout doesn't tell much about reordering. A sender that
> learns about reordering on big writes and lose packets on small
> writes will end up falsely retransmit again and again, especially
> when reordering is more likely on big writes.
>
> Therefore the sender should only suspects tp->reordering is too
> high if it could have gone into fast recovery with the (lower)
> default estimate.
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> ---
>  net/ipv4/tcp_input.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Acked-by: Neal Cardwell <ncardwell@google.com>

neal

^ permalink raw reply

* Re: BUG cxgb3: Check and handle the dma mapping errors
From: Alexey Kardashevskiy @ 2013-08-13  2:42 UTC (permalink / raw)
  To: Divy Le ray
  Cc: Jay Fenlason, Santosh Rastapur, David S. Miller,
	netdev@vger.kernel.org, Linux Kernel Mailing List
In-Reply-To: <52032ED4.2030202@ozlabs.ru>

Ping, anyone?


On 08/08/2013 03:38 PM, Alexey Kardashevskiy wrote:
> On 08/08/2013 02:55 AM, Divy Le ray wrote:
>> On 08/05/2013 11:41 AM, Jay Fenlason wrote:
>>> On Mon, Aug 05, 2013 at 12:59:04PM +1000, Alexey Kardashevskiy wrote:
>>>> Hi!
>>>>
>>>> Recently I started getting multiple errors like this:
>>>>
>>>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>>>> c000001fbdaaa882 npages 1
>>>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>>>> c000001fbdaaa882 npages 1
>>>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>>>> c000001fbdaaa882 npages 1
>>>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>>>> c000001fbdaaa882 npages 1
>>>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>>>> c000001fbdaaa882 npages 1
>>>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>>>> c000001fbdaaa882 npages 1
>>>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>>>> c000001fbdaaa882 npages 1
>>>> ... and so on
>>>>
>>>> This is all happening on a PPC64 "powernv" platform machine. To trigger the
>>>> error state, it is enough to _flood_ ping CXGB3 card from another machine
>>>> (which has Emulex 10Gb NIC + Cisco switch). Just do "ping -f 172.20.1.2"
>>>> and wait 10-15 seconds.
>>>>
>>>>
>>>> The messages are coming from arch/powerpc/kernel/iommu.c and basically
>>>> mean that the driver requested more pages than the DMA window has which is
>>>> normally 1GB (there could be another possible source of errors -
>>>> ppc_md.tce_build callback - but on powernv platform it always succeeds).
>>>>
>>>>
>>>> The patch after which it broke is:
>>>> commit f83331bab149e29fa2c49cf102c0cd8c3f1ce9f9
>>>> Author: Santosh Rastapur <santosh@chelsio.com>
>>>> Date:   Tue May 21 04:21:29 2013 +0000
>>>> cxgb3: Check and handle the dma mapping errors
>>>>
>>>> Any quick ideas? Thanks!
>>> That patch adds error checking to detect failed dma mapping requests.
>>> Before it, the code always assumed that dma mapping requests succeded,
>>> whether they actually do or not, so the fact that the older kernel
>>> does not log errors only means that the failures are being ignored,
>>> and any appearance of working is through pure luck.  The machine could
>>> have just crashed at that point.
>>>
>>> What is the observed behavior of the system by the machine initiating
>>> the ping flood?  Do the older and newer kernels differ in the
>>> percentage of pings that do not receive replies?  O the newer kernel,
>>> when the mapping errors are detected, the packet that it is trying to
>>> transmit is dropped, but I'm not at all sure what happens on the older
>>> kernel after the dma mapping fails.  As I mentioned earlier, I'm
>>> surprised it does not crash.  Perhaps the folks from Chelsio have a
>>> better idea what happens after a dma mapping error is ignored?
>>
>> Hi,
>>
>> It should definitely not be ignored. It should not happen this reliably
>> either.
>> I wonder if we are not hitting a leak of iommu entries.
> 
> Yes we do. I did some more tests with socklib from here
> http://junkcode.samba.org/ftp/unpacked/junkcode/socklib/
> 
> The test is basically sock_source sending packets to sock_sink. If block
> size is >=512 bytes, there is no leak, if I set packet size to <=256 bytes,
> it starts leaking, smaller block size means faster leak. The type of the
> other adapter does not really matter, can be the same Emulex adapter.
> 
> I am attaching a small patch which I made in order to detect the leak.
> Without the patch, no leak happens, I double checked.
> 
> 


-- 
Alexey

^ permalink raw reply

* [PATCH net] xfrm: introduce helper for safe determination of mtu
From: Hannes Frederic Sowa @ 2013-08-13  2:35 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert

skb->sk socket can be of AF_INET or AF_INET6 address family. Thus we
always have to make sure we a referring to the correct interpretation
of skb->sk.

We only depend on header defines to query the mtu, so we don't introduce
a new dependency to ipv6 by this change.

Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/net/route.h     |  8 ++++++++
 include/net/xfrm.h      | 12 ++++++++++++
 net/ipv4/ip_output.c    |  8 --------
 net/ipv4/xfrm4_output.c |  4 +---
 net/ipv6/xfrm6_output.c |  5 ++++-
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/include/net/route.h b/include/net/route.h
index 2ea40c1..afdeeb5 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -317,4 +317,12 @@ static inline int ip4_dst_hoplimit(const struct dst_entry *dst)
 	return hoplimit;
 }
 
+static inline int ip_skb_dst_mtu(struct sk_buff *skb)
+{
+	struct inet_sock *inet = skb->sk ? inet_sk(skb->sk) : NULL;
+
+	return (inet && inet->pmtudisc == IP_PMTUDISC_PROBE) ?
+	       skb_dst(skb)->dev->mtu : dst_mtu(skb_dst(skb));
+}
+
 #endif	/* _ROUTE_H */
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index e823786..b41d2d1 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -20,6 +20,7 @@
 #include <net/route.h>
 #include <net/ipv6.h>
 #include <net/ip6_fib.h>
+#include <net/ip6_route.h>
 #include <net/flow.h>
 
 #include <linux/interrupt.h>
@@ -1723,4 +1724,15 @@ static inline int xfrm_mark_put(struct sk_buff *skb, const struct xfrm_mark *m)
 	return ret;
 }
 
+static inline int xfrm_skb_dst_mtu(struct sk_buff *skb)
+{
+	struct sock *sk = skb->sk;
+
+	if (sk && sk->sk_family == AF_INET6)
+		return ip6_skb_dst_mtu(skb);
+	else if (sk && sk->sk_family == AF_INET)
+		return ip_skb_dst_mtu(skb);
+	return dst_mtu(skb_dst(skb));
+}
+
 #endif	/* _NET_XFRM_H */
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 4bcabf3..9ee17e3 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -211,14 +211,6 @@ static inline int ip_finish_output2(struct sk_buff *skb)
 	return -EINVAL;
 }
 
-static inline int ip_skb_dst_mtu(struct sk_buff *skb)
-{
-	struct inet_sock *inet = skb->sk ? inet_sk(skb->sk) : NULL;
-
-	return (inet && inet->pmtudisc == IP_PMTUDISC_PROBE) ?
-	       skb_dst(skb)->dev->mtu : dst_mtu(skb_dst(skb));
-}
-
 static int ip_finish_output(struct sk_buff *skb)
 {
 #if defined(CONFIG_NETFILTER) && defined(CONFIG_XFRM)
diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 7a5491f..80baf4a 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -21,7 +21,6 @@
 static int xfrm4_tunnel_check_size(struct sk_buff *skb)
 {
 	int mtu, ret = 0;
-	struct dst_entry *dst;
 
 	if (IPCB(skb)->flags & IPSKB_XFRM_TUNNEL_SIZE)
 		goto out;
@@ -29,8 +28,7 @@ static int xfrm4_tunnel_check_size(struct sk_buff *skb)
 	if (!(ip_hdr(skb)->frag_off & htons(IP_DF)) || skb->local_df)
 		goto out;
 
-	dst = skb_dst(skb);
-	mtu = dst_mtu(dst);
+	mtu = xfrm_skb_dst_mtu(skb);
 	if (skb->len > mtu) {
 		if (skb->sk)
 			xfrm_local_error(skb, mtu);
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index b64fff3..3ac5ab2 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -138,7 +138,10 @@ static int __xfrm6_output(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb_dst(skb);
 	struct xfrm_state *x = dst->xfrm;
-	int mtu = ip6_skb_dst_mtu(skb);
+	int mtu = xfrm_skb_dst_mtu(skb);
+
+	if (mtu < IPV6_MIN_MTU)
+		mtu = IPV6_MIN_MTU;
 
 	if (skb->len > mtu && xfrm6_local_dontfrag(skb)) {
 		xfrm6_local_rxpmtu(skb, mtu);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next v2] xfrm: make local error reporting more robust
From: Hannes Frederic Sowa @ 2013-08-13  1:56 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: Eric Dumazet, netdev, vi0oss
In-Reply-To: <20130812055414.GK25511@secunet.com>

On Mon, Aug 12, 2013 at 07:54:14AM +0200, Steffen Klassert wrote:
> On Sat, Aug 10, 2013 at 06:16:29PM +0200, Hannes Frederic Sowa wrote:
> > 
> > Seems skb->encapsulated helps, but I still have to wire it up for the ipv6
> > tunnels.
> > 
> > I just prototyped this patch, but I fear I now introduced a dependency
> > from core xfrm to ipv6, which I would like to have prevented (this would
> > even happen if I put xfrm_local_error in a header file). Is this actually
> > a problem? I fear so. The other way would be to put the local_error
> > handler as function pointers somewhere reachable from struct sock.
> > 
> 
> Maybe we should put a local_error() function pointer to struct
> xfrm_state_afinfo and call it via inner_mode->afinfo->local_error().
> 
> This should always call the right local_error function and we
> would not need to touch generic networking code to fix it.

Sorry, had to do a v2, because I missed two more unsafe skb->sk dereferences.
I will post a further one (unsafe determination of mtu) as a seperate patch
(needs its own commit message).

[PATCH net-next v2] xfrm: make local error reporting more robust

In xfrm4 and xfrm6 we need to take care about sockets of the other
address family. This could happen because a 6in4 or 4in6 tunnel could
get protected by ipsec.

Because we don't want to have a run-time dependency on ipv6 when only
using ipv4 xfrm we have to embed a pointer to the correct local_error
function in xfrm_state_afinet and look it up when returning an error
depending on the socket address family.

Thanks to vi0ss for the great bug report:
<https://bugzilla.kernel.org/show_bug.cgi?id=58691>

v2:
a) fix two more unsafe interpretations of skb->sk as ipv6 socket
   (xfrm6_local_dontfrag and __xfrm6_output)

Reported-by: <vi0oss@gmail.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/net/xfrm.h      |  6 ++++++
 net/ipv4/xfrm4_output.c | 12 ++++++++++--
 net/ipv4/xfrm4_state.c  |  1 +
 net/ipv6/xfrm6_output.c | 10 ++++++----
 net/ipv6/xfrm6_state.c  |  1 +
 net/xfrm/xfrm_output.c  | 12 ++++++++++++
 net/xfrm/xfrm_state.c   |  7 ++-----
 7 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 94ce082..e823786 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -341,10 +341,13 @@ struct xfrm_state_afinfo {
 						  struct sk_buff *skb);
 	int			(*transport_finish)(struct sk_buff *skb,
 						    int async);
+	void			(*local_error)(struct sk_buff *skb, u32 mtu);
 };
 
 extern int xfrm_state_register_afinfo(struct xfrm_state_afinfo *afinfo);
 extern int xfrm_state_unregister_afinfo(struct xfrm_state_afinfo *afinfo);
+extern struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family);
+extern void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo);
 
 extern void xfrm_state_delete_tunnel(struct xfrm_state *x);
 
@@ -1477,6 +1480,7 @@ extern int xfrm_input_resume(struct sk_buff *skb, int nexthdr);
 extern int xfrm_output_resume(struct sk_buff *skb, int err);
 extern int xfrm_output(struct sk_buff *skb);
 extern int xfrm_inner_extract_output(struct xfrm_state *x, struct sk_buff *skb);
+extern void xfrm_local_error(struct sk_buff *skb, int mtu);
 extern int xfrm4_extract_header(struct sk_buff *skb);
 extern int xfrm4_extract_input(struct xfrm_state *x, struct sk_buff *skb);
 extern int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
@@ -1497,6 +1501,7 @@ extern int xfrm4_tunnel_register(struct xfrm_tunnel *handler, unsigned short fam
 extern int xfrm4_tunnel_deregister(struct xfrm_tunnel *handler, unsigned short family);
 extern int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel *handler);
 extern int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel *handler);
+extern void xfrm4_local_error(struct sk_buff *skb, u32 mtu);
 extern int xfrm6_extract_header(struct sk_buff *skb);
 extern int xfrm6_extract_input(struct xfrm_state *x, struct sk_buff *skb);
 extern int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi);
@@ -1514,6 +1519,7 @@ extern int xfrm6_output(struct sk_buff *skb);
 extern int xfrm6_output_finish(struct sk_buff *skb);
 extern int xfrm6_find_1stfragopt(struct xfrm_state *x, struct sk_buff *skb,
 				 u8 **prevhdr);
+extern void xfrm6_local_error(struct sk_buff *skb, u32 mtu);
 
 #ifdef CONFIG_XFRM
 extern int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb);
diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 327a617..7a5491f 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -33,8 +33,7 @@ static int xfrm4_tunnel_check_size(struct sk_buff *skb)
 	mtu = dst_mtu(dst);
 	if (skb->len > mtu) {
 		if (skb->sk)
-			ip_local_error(skb->sk, EMSGSIZE, ip_hdr(skb)->daddr,
-				       inet_sk(skb->sk)->inet_dport, mtu);
+			xfrm_local_error(skb, mtu);
 		else
 			icmp_send(skb, ICMP_DEST_UNREACH,
 				  ICMP_FRAG_NEEDED, htonl(mtu));
@@ -99,3 +98,12 @@ int xfrm4_output(struct sk_buff *skb)
 			    x->outer_mode->afinfo->output_finish,
 			    !(IPCB(skb)->flags & IPSKB_REROUTED));
 }
+
+void xfrm4_local_error(struct sk_buff *skb, u32 mtu)
+{
+	struct iphdr *hdr;
+
+	hdr = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb);
+	ip_local_error(skb->sk, EMSGSIZE, hdr->daddr,
+		       inet_sk(skb->sk)->inet_dport, mtu);
+}
diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c
index 9258e75..0b2a064 100644
--- a/net/ipv4/xfrm4_state.c
+++ b/net/ipv4/xfrm4_state.c
@@ -83,6 +83,7 @@ static struct xfrm_state_afinfo xfrm4_state_afinfo = {
 	.extract_input		= xfrm4_extract_input,
 	.extract_output		= xfrm4_extract_output,
 	.transport_finish	= xfrm4_transport_finish,
+	.local_error		= xfrm4_local_error,
 };
 
 void __init xfrm4_state_init(void)
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index 8755a30..b64fff3 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -34,8 +34,10 @@ static int xfrm6_local_dontfrag(struct sk_buff *skb)
 	struct sock *sk = skb->sk;
 
 	if (sk) {
-		proto = sk->sk_protocol;
+		if (sk->sk_family != AF_INET6)
+			return 0;
 
+		proto = sk->sk_protocol;
 		if (proto == IPPROTO_UDP || proto == IPPROTO_RAW)
 			return inet6_sk(sk)->dontfrag;
 	}
@@ -54,7 +56,7 @@ static void xfrm6_local_rxpmtu(struct sk_buff *skb, u32 mtu)
 	ipv6_local_rxpmtu(sk, &fl6, mtu);
 }
 
-static void xfrm6_local_error(struct sk_buff *skb, u32 mtu)
+void xfrm6_local_error(struct sk_buff *skb, u32 mtu)
 {
 	struct flowi6 fl6;
 	struct sock *sk = skb->sk;
@@ -80,7 +82,7 @@ static int xfrm6_tunnel_check_size(struct sk_buff *skb)
 		if (xfrm6_local_dontfrag(skb))
 			xfrm6_local_rxpmtu(skb, mtu);
 		else if (skb->sk)
-			xfrm6_local_error(skb, mtu);
+			xfrm_local_error(skb, mtu);
 		else
 			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		ret = -EMSGSIZE;
@@ -142,7 +144,7 @@ static int __xfrm6_output(struct sk_buff *skb)
 		xfrm6_local_rxpmtu(skb, mtu);
 		return -EMSGSIZE;
 	} else if (!skb->local_df && skb->len > mtu && skb->sk) {
-		xfrm6_local_error(skb, mtu);
+		xfrm_local_error(skb, mtu);
 		return -EMSGSIZE;
 	}
 
diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
index d8c70b8..3fc9701 100644
--- a/net/ipv6/xfrm6_state.c
+++ b/net/ipv6/xfrm6_state.c
@@ -183,6 +183,7 @@ static struct xfrm_state_afinfo xfrm6_state_afinfo = {
 	.extract_input		= xfrm6_extract_input,
 	.extract_output		= xfrm6_extract_output,
 	.transport_finish	= xfrm6_transport_finish,
+	.local_error		= xfrm6_local_error,
 };
 
 int __init xfrm6_state_init(void)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index eb4a842..c445a2f 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -214,5 +214,17 @@ int xfrm_inner_extract_output(struct xfrm_state *x, struct sk_buff *skb)
 	return inner_mode->afinfo->extract_output(x, skb);
 }
 
+void xfrm_local_error(struct sk_buff *skb, int mtu)
+{
+	struct xfrm_state_afinfo *afinfo;
+
+	afinfo = xfrm_state_get_afinfo(skb->sk->sk_family);
+	if (!afinfo)
+		return;
+
+	afinfo->local_error(skb, mtu);
+	xfrm_state_put_afinfo(afinfo);
+}
+
 EXPORT_SYMBOL_GPL(xfrm_output);
 EXPORT_SYMBOL_GPL(xfrm_inner_extract_output);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 78f66fa..54c0acd 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -39,9 +39,6 @@ static DEFINE_SPINLOCK(xfrm_state_lock);
 
 static unsigned int xfrm_state_hashmax __read_mostly = 1 * 1024 * 1024;
 
-static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family);
-static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo);
-
 static inline unsigned int xfrm_dst_hash(struct net *net,
 					 const xfrm_address_t *daddr,
 					 const xfrm_address_t *saddr,
@@ -1860,7 +1857,7 @@ int xfrm_state_unregister_afinfo(struct xfrm_state_afinfo *afinfo)
 }
 EXPORT_SYMBOL(xfrm_state_unregister_afinfo);
 
-static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family)
+struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family)
 {
 	struct xfrm_state_afinfo *afinfo;
 	if (unlikely(family >= NPROTO))
@@ -1872,7 +1869,7 @@ static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family)
 	return afinfo;
 }
 
-static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo)
+void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo)
 {
 	rcu_read_unlock();
 }
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH 8/8] net: davinci_mdio: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-13  1:23 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: David Miller, mugunthanvnm, bigeasy, prabhakar.csengg, b-liu,
	netdev, LKML, Li Zefan
In-Reply-To: <520926CB.4020500@cogentembedded.com>

On 2013/8/13 2:17, Sergei Shtylyov wrote:
> On 08/12/2013 05:25 PM, Libo Chen wrote:
> 
>> Use the wrapper functions for getting and setting the driver data using
>> platform_device instead of using dev_{get,set}_drvdata() with &pdev->dev,
>> so we can directly pass a struct platform_device.
> 
>> Signed-off-by: Libo Chen <libo.chen@huawei.com>
>> ---
>>   drivers/net/ethernet/ti/davinci_mdio.c |    4 +---
>>   1 files changed, 1 insertions(+), 3 deletions(-)
> 
>> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
>> index 16ddfc3..2249b2c 100644
>> --- a/drivers/net/ethernet/ti/davinci_mdio.c
>> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> [...]
>> @@ -434,7 +433,6 @@ static int davinci_mdio_remove(struct platform_device *pdev)
>>       pm_runtime_put_sync(&pdev->dev);
>>       pm_runtime_disable(&pdev->dev);
>>
>> -    dev_set_drvdata(dev, NULL);
> 
>    Doesn't get described as well. I think you're mixing two types of changes in one patchset.
> 
> WBR, Sergei
> 

thanks for your attention!


Libo

> 
> 

^ permalink raw reply

* Re: [PATCH 8/8] net: davinci_mdio: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-13  1:22 UTC (permalink / raw)
  To: David Miller
  Cc: sergei.shtylyov, mugunthanvnm, bigeasy, prabhakar.csengg, b-liu,
	netdev, linux-kernel, lizefan
In-Reply-To: <20130812.130413.1869715009441960165.davem@davemloft.net>

On 2013/8/13 4:04, David Miller wrote:
> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Date: Mon, 12 Aug 2013 22:17:47 +0400
> 
>>    Doesn't get described as well. I think you're mixing two types of
>>    changes in one patchset.
> 
> Agreed, you're going to have to redo this entire patch set.
> 

okay, I will redo!

^ permalink raw reply

* Re: [PATCH net-next] bonding: lacp_port_id setting for 802.3ad
From: Ding Tianhong @ 2013-08-13  1:07 UTC (permalink / raw)
  To: Krisztian Ivancso; +Cc: netdev
In-Reply-To: <5208C4BA.9020605@ivancso.net>

On 2013/8/12 19:19, Krisztian Ivancso wrote:
> The patch adds port id setting feature by lacp_port_id module
> parameter for 802.3ad ports.
> 
> Adding more linux boxes to a link aggregation makes possible to do
> load-balancing by using port channel load balancing of switch
> (eg. by source-dest-ip or src-ip).
> 
> It's essential to set the same MAC address to bonding interface and
> to set different port ids on different linux boxes for a
> working solution.
> 
> Possible use cases:
> 1. Redundant DNS servers with same IP address
> 2. Reverse proxies with same IP address
> 3. "multi-master" LVS
> 
> Beside redundancy it provides scalability, scalability depends on
> hardware capability.
> E.g. a LAG port with 4 members - linux LVS servers - using a Cisco
> switch/router splits traffic to ports equally by src IP (2-2-2-2).
> If a link fails, switch forwards traffic to 3 switch ports (3-3-3).
> It means you can utilize overall bandwith as much as 66% with full
> redundancy. It's better with 16% than using 2 master-slave LVS and
> you use just 1 IP and can handle 100% more peak traffic.
> 
> Using this feature with vPC (virtual portchannel) or similar solution
> it provides a physically redundant service from network devices
> to servers using the same IP address.
> 
> 
>>From 472fffa5a8f170daed9e4cc677af8e2560b86be2 Mon Sep 17 00:00:00 2001
> From: Krisztian Ivancso <github-ivan@ivancso.net>
> Date: Sun, 11 Aug 2013 20:30:44 +0200
> Subject: [PATCH net-next] bonding: lacp_port_id setting for 802.3ad ports
> 
> By setting this parameter to different values on different hosts
> it's possible to add more Linux boxes to the same Link Aggregation.
> 

I think the port id was made by negotiation,  if the negotiation failed, the bonding slave
could not get the port id you gave, which lie on the hardware message.

^ permalink raw reply

* [PATCH net-next] xfrm: make local error reporting more robust
From: Hannes Frederic Sowa @ 2013-08-13  0:48 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: Eric Dumazet, netdev, vi0oss
In-Reply-To: <20130812055414.GK25511@secunet.com>

On Mon, Aug 12, 2013 at 07:54:14AM +0200, Steffen Klassert wrote:
> On Sat, Aug 10, 2013 at 06:16:29PM +0200, Hannes Frederic Sowa wrote:
> > 
> > Seems skb->encapsulated helps, but I still have to wire it up for the ipv6
> > tunnels.
> > 
> > I just prototyped this patch, but I fear I now introduced a dependency
> > from core xfrm to ipv6, which I would like to have prevented (this would
> > even happen if I put xfrm_local_error in a header file). Is this actually
> > a problem? I fear so. The other way would be to put the local_error
> > handler as function pointers somewhere reachable from struct sock.
> > 
> 
> Maybe we should put a local_error() function pointer to struct
> xfrm_state_afinfo and call it via inner_mode->afinfo->local_error().
> 
> This should always call the right local_error function and we
> would not need to touch generic networking code to fix it.

afinfo was a great hint. But I could not use inner_mode, because it
would still point to the wrong afinfo instance.

I tested this patch with vi0ss's great test script:
<https://gist.github.com/vi/5640512>

I will look at the skb->encapsulation for IPv6 logic again, as soon
as I understand the side effects when I introduce this. This will then
correct the destination address when sending back ipv6 errors.

Steffen, if you are fine with this patch, do you think it is a candidate
for stable?

[PATCH net-next] xfrm: make local error reporting more robust

In xfrm4 and xfrm6 we need to take care about sockets of the other
address family. This could happen because a 6in4 or 4in6 tunnel could
get protected by ipsec.

Because we don't want to have a run-time dependency on ipv6 when only
using ipv4 xfrm we have to embed a pointer to the correct local_error
function in xfrm_state_afinet and look it up when returning an error
depending on the socket address family.

Thanks to vi0ss for the great bug report:
<https://bugzilla.kernel.org/show_bug.cgi?id=58691>

Reported-by: <vi0oss@gmail.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/net/xfrm.h      |  6 ++++++
 net/ipv4/xfrm4_output.c | 12 ++++++++++--
 net/ipv4/xfrm4_state.c  |  1 +
 net/ipv6/xfrm6_output.c |  4 ++--
 net/ipv6/xfrm6_state.c  |  1 +
 net/xfrm/xfrm_output.c  | 12 ++++++++++++
 net/xfrm/xfrm_state.c   |  7 ++-----
 7 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 94ce082..e823786 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -341,10 +341,13 @@ struct xfrm_state_afinfo {
 						  struct sk_buff *skb);
 	int			(*transport_finish)(struct sk_buff *skb,
 						    int async);
+	void			(*local_error)(struct sk_buff *skb, u32 mtu);
 };
 
 extern int xfrm_state_register_afinfo(struct xfrm_state_afinfo *afinfo);
 extern int xfrm_state_unregister_afinfo(struct xfrm_state_afinfo *afinfo);
+extern struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family);
+extern void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo);
 
 extern void xfrm_state_delete_tunnel(struct xfrm_state *x);
 
@@ -1477,6 +1480,7 @@ extern int xfrm_input_resume(struct sk_buff *skb, int nexthdr);
 extern int xfrm_output_resume(struct sk_buff *skb, int err);
 extern int xfrm_output(struct sk_buff *skb);
 extern int xfrm_inner_extract_output(struct xfrm_state *x, struct sk_buff *skb);
+extern void xfrm_local_error(struct sk_buff *skb, int mtu);
 extern int xfrm4_extract_header(struct sk_buff *skb);
 extern int xfrm4_extract_input(struct xfrm_state *x, struct sk_buff *skb);
 extern int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
@@ -1497,6 +1501,7 @@ extern int xfrm4_tunnel_register(struct xfrm_tunnel *handler, unsigned short fam
 extern int xfrm4_tunnel_deregister(struct xfrm_tunnel *handler, unsigned short family);
 extern int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel *handler);
 extern int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel *handler);
+extern void xfrm4_local_error(struct sk_buff *skb, u32 mtu);
 extern int xfrm6_extract_header(struct sk_buff *skb);
 extern int xfrm6_extract_input(struct xfrm_state *x, struct sk_buff *skb);
 extern int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi);
@@ -1514,6 +1519,7 @@ extern int xfrm6_output(struct sk_buff *skb);
 extern int xfrm6_output_finish(struct sk_buff *skb);
 extern int xfrm6_find_1stfragopt(struct xfrm_state *x, struct sk_buff *skb,
 				 u8 **prevhdr);
+extern void xfrm6_local_error(struct sk_buff *skb, u32 mtu);
 
 #ifdef CONFIG_XFRM
 extern int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb);
diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 327a617..7a5491f 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -33,8 +33,7 @@ static int xfrm4_tunnel_check_size(struct sk_buff *skb)
 	mtu = dst_mtu(dst);
 	if (skb->len > mtu) {
 		if (skb->sk)
-			ip_local_error(skb->sk, EMSGSIZE, ip_hdr(skb)->daddr,
-				       inet_sk(skb->sk)->inet_dport, mtu);
+			xfrm_local_error(skb, mtu);
 		else
 			icmp_send(skb, ICMP_DEST_UNREACH,
 				  ICMP_FRAG_NEEDED, htonl(mtu));
@@ -99,3 +98,12 @@ int xfrm4_output(struct sk_buff *skb)
 			    x->outer_mode->afinfo->output_finish,
 			    !(IPCB(skb)->flags & IPSKB_REROUTED));
 }
+
+void xfrm4_local_error(struct sk_buff *skb, u32 mtu)
+{
+	struct iphdr *hdr;
+
+	hdr = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb);
+	ip_local_error(skb->sk, EMSGSIZE, hdr->daddr,
+		       inet_sk(skb->sk)->inet_dport, mtu);
+}
diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c
index 9258e75..0b2a064 100644
--- a/net/ipv4/xfrm4_state.c
+++ b/net/ipv4/xfrm4_state.c
@@ -83,6 +83,7 @@ static struct xfrm_state_afinfo xfrm4_state_afinfo = {
 	.extract_input		= xfrm4_extract_input,
 	.extract_output		= xfrm4_extract_output,
 	.transport_finish	= xfrm4_transport_finish,
+	.local_error		= xfrm4_local_error,
 };
 
 void __init xfrm4_state_init(void)
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index 8755a30..f6f9110 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -54,7 +54,7 @@ static void xfrm6_local_rxpmtu(struct sk_buff *skb, u32 mtu)
 	ipv6_local_rxpmtu(sk, &fl6, mtu);
 }
 
-static void xfrm6_local_error(struct sk_buff *skb, u32 mtu)
+void xfrm6_local_error(struct sk_buff *skb, u32 mtu)
 {
 	struct flowi6 fl6;
 	struct sock *sk = skb->sk;
@@ -80,7 +80,7 @@ static int xfrm6_tunnel_check_size(struct sk_buff *skb)
 		if (xfrm6_local_dontfrag(skb))
 			xfrm6_local_rxpmtu(skb, mtu);
 		else if (skb->sk)
-			xfrm6_local_error(skb, mtu);
+			xfrm_local_error(skb, mtu);
 		else
 			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		ret = -EMSGSIZE;
diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
index d8c70b8..3fc9701 100644
--- a/net/ipv6/xfrm6_state.c
+++ b/net/ipv6/xfrm6_state.c
@@ -183,6 +183,7 @@ static struct xfrm_state_afinfo xfrm6_state_afinfo = {
 	.extract_input		= xfrm6_extract_input,
 	.extract_output		= xfrm6_extract_output,
 	.transport_finish	= xfrm6_transport_finish,
+	.local_error		= xfrm6_local_error,
 };
 
 int __init xfrm6_state_init(void)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index eb4a842..c445a2f 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -214,5 +214,17 @@ int xfrm_inner_extract_output(struct xfrm_state *x, struct sk_buff *skb)
 	return inner_mode->afinfo->extract_output(x, skb);
 }
 
+void xfrm_local_error(struct sk_buff *skb, int mtu)
+{
+	struct xfrm_state_afinfo *afinfo;
+
+	afinfo = xfrm_state_get_afinfo(skb->sk->sk_family);
+	if (!afinfo)
+		return;
+
+	afinfo->local_error(skb, mtu);
+	xfrm_state_put_afinfo(afinfo);
+}
+
 EXPORT_SYMBOL_GPL(xfrm_output);
 EXPORT_SYMBOL_GPL(xfrm_inner_extract_output);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 78f66fa..54c0acd 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -39,9 +39,6 @@ static DEFINE_SPINLOCK(xfrm_state_lock);
 
 static unsigned int xfrm_state_hashmax __read_mostly = 1 * 1024 * 1024;
 
-static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family);
-static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo);
-
 static inline unsigned int xfrm_dst_hash(struct net *net,
 					 const xfrm_address_t *daddr,
 					 const xfrm_address_t *saddr,
@@ -1860,7 +1857,7 @@ int xfrm_state_unregister_afinfo(struct xfrm_state_afinfo *afinfo)
 }
 EXPORT_SYMBOL(xfrm_state_unregister_afinfo);
 
-static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family)
+struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family)
 {
 	struct xfrm_state_afinfo *afinfo;
 	if (unlikely(family >= NPROTO))
@@ -1872,7 +1869,7 @@ static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family)
 	return afinfo;
 }
 
-static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo)
+void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo)
 {
 	rcu_read_unlock();
 }
-- 
1.8.3.1

^ 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