Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] htb: improved accuracy at high rates
From: Rick Jones @ 2012-10-22 17:35 UTC (permalink / raw)
  To: Vimal; +Cc: davem, eric.dumazet, Jamal Hadi Salim, netdev
In-Reply-To: <CAK3Ji116nZ79csrq9GJ7C5e3zLPJina1r2+VasdVRsdY7n+3ww@mail.gmail.com>

On 10/19/2012 05:51 PM, Vimal wrote:
> On 19 October 2012 16:52, Rick Jones <rick.jones2@hp.com> wrote:
>>
>> First some netperf/operational kinds of questions:
>>
>> Did it really take 20 concurrent netperf UDP_STREAM tests to get to those
>> rates?  And why UDP_STREAM rather than TCP_STREAM?
>
> Nope, even 1 netperf was sufficient.  Before I couldn't get TCP_STREAM
> to send small byte packets, but I checked my script now and I forgot
> to enable TCP_NODELAY + send buffer size (-s $size).
>
> With one tcp sender I am unable to reach the 1Gb/s limit (only
> ~100Mb/s) even with a lot of CPU to spare, which indicates that the
> test is limited by e2e latency.  With 10 connections, I could get only
> 800Mb/s, and with 20 connections it went to 1160Mb/s, which violates
> the 1Gb/s limit set.

Were you explicitly constraining the TCP socketbuffer/window via 
test-specific -s and -S options?  Or was this a system with little 
enough memory that the upper limit for the TCP socket/window autotuning 
wasn't the somewhat common, which would have been sufficient to cover a 
rather large RTT,  The results of the TCP_RR tests below suggest there 
was actually very little e2e latency... which suggests something else 
was holding-back the TCP performance.  Perhaps lost packets?

Or does this suggest the need for an htb_codel?-)  If your system can 
have rrdtool installed on it, and you are indeed using a contemporary 
version of netperf, you can run the bloat.sh script from doc/examples 
and get an idea of how much bufferbloat there is in the setup.

If you are using a "current" version of netperf, you can use the omni 
output selectors to have netperf emit the number of TCP retransmissions 
on the data connection during the test.  Otherwise, if the netperf tests 
are the only things running at the time, you can take a snapshot of 
netstat -s output before and after the test and run it through something 
like beforeafter, or the other script I keep forgetting the name of :(

>> Which reported throughput was used from the UDP_STREAM tests - send side or
>> receive side?
>
> Send side.

Given the nature of UDP and that netperf makes no attempt to compensate 
for that, it would be rather better to use receive-side throughout.  The 
receive side throughput is known to have made it through everything. 
The send side throughput is only that which didn't report an error in 
the sendto() call.  And lack of error on the sendto() call does not 
guarantee a successful transmission on the wire.  Even under Linux with 
intra-stack flow-control.

>> Is there much/any change in service demand on a netperf test?  That is what
>> is the service demand of a mumble_STREAM test running through the old HTB
>> versus the new HTB?  And/or the performance of a TCP_RR test (both
>> transactions per second and service demand per transaction) before vs after.
>>
>
> At 1Gb/s with just one TCP_STREAM:
> With old HTB:
> Sdem local: 0.548us/KB, Sdem remote: 1.426us/KB.
>
> With new HTB:
> Sdem local: 0.598us/KB, Sdem remote: 1.089us/KB.

Presumably the receive side service demand should have remained 
unaffected by the HTB changes.  That it changed by 40% suggests there 
wasn't actually all that much stability - at least not on the receive 
side.  Was there a large change in throughput for the single-stream?

That there was a 9% increase in sending service demand is a bit 
troubling, and at least slightly at odds with the little to no change in 
the sending service demand for the TCP_RR tests below.  I suppose that 
the "timing" nature of having things like small sends and TCP_NODELAY 
set can introduce too many variables.

One other way to skin the cat of "what does it do to sending service 
demand would be to stick with TCP_RR and walk-up the request size. 
Without a burst mode enabled, and just the one transaction in flight at 
one time, TCP_NODELAY on/off should be a don't care.  So, something 
along the lines of:

HDR="-P 1"
for r in 1 4 16 64 256 1024 4096 16384 65535
do
netperf $HDR -H <remote> -t TCP_RR -c -C -l 30 -- -r ${r},1
HDR="-P 0"
done

> TCP_RR: 1b req/response consumed very little bandwidth (~12Mb/s)

That is why, by default, it reports a transaction per second rate and 
not a bandwidth :)

> old HTB at 1Gb/s
> Sdem local: 14.738us/trans, Sdem remote: 11.485us/Tran, latency: 41.622us/Tran.
>
> new HTB at 1Gb/s
> Sdem local: 14.505us/trans, Sdem remote: 11.440us/Tran, latency: 41.709us/Tran.
>
> With multiple tests, these values are fairly stable. :)

Those do look like they were within the noise level.

Out of mostly idle curiousity, just what sort of system was being used 
for the testing?  CPU, bitness, memory etc.

happy benchmarking,

rick jones

^ permalink raw reply

* Re: [PATCH v2] phy: add AT803x driver
From: David Miller @ 2012-10-22 17:55 UTC (permalink / raw)
  To: ujhelyi.m; +Cc: netdev, daniel, rd
In-Reply-To: <CAJKycQkiUXi2dcUziztHBkEtT6aJ6NnPAVgDw=pYpqJuF_1s9g@mail.gmail.com>

From: Matus Ujhelyi <ujhelyi.m@gmail.com>
Date: Mon, 22 Oct 2012 12:20:26 +0200

> do you have any feedback on the new version of patch (see below) ?

It's applied to my tree for days, what more do you want?

^ permalink raw reply

* Re: [PATCH net-next 1/2] r8169: enable ALDPS for power saving
From: David Miller @ 2012-10-22 17:59 UTC (permalink / raw)
  To: hayeswang; +Cc: romieu, netdev, linux-kernel, jean
In-Reply-To: <1350893153-18320-1-git-send-email-hayeswang@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Mon, 22 Oct 2012 16:05:53 +0800

> Enable ALDPS function to save power when link down. Note that the
> feature should be set after the other PHY settings. And the firmware
> is necessary. Don't enable it without loading the firmware.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

If there are two patches in this series, which the Subject line
implies, where is the second patch?

^ permalink raw reply

* Re: Re: A reliable kernel panic (3.6.2) and system crash when visiting a particular website
From: Alan Stern @ 2012-10-22 18:01 UTC (permalink / raw)
  To: Artem S. Tashkinov
  Cc: zonque, bp, pavel, linux-kernel, netdev, security, linux-media,
	linux-usb, alsa-devel
In-Reply-To: <1985645001.39510.1350927037793.JavaMail.mail@webmail15>

On Mon, 22 Oct 2012, Artem S. Tashkinov wrote:

> OK, here's what the kernel prints with your patch:
> 
> usb 6.1.4: ep 86 list del corruption prev: e5103b54 e5103a94 e51039d4
> 
> A small delay before I got thousands of list_del corruption messages would
> have been nice, but I managed to catch the message anyway.

All right.  Here's a new patch, which will print more information and
will provide a 10-second delay.

For this to be useful, you should capture a usbmon trace at the same
time.  The relevant entries will show up in the trace shortly before
_and_ shortly after the error message appears.

Alan Stern

P.S.: It will help if you unplug as many of the other USB devices as
possible before running this test.



Index: usb-3.6/drivers/usb/core/hcd.c
===================================================================
--- usb-3.6.orig/drivers/usb/core/hcd.c
+++ usb-3.6/drivers/usb/core/hcd.c
@@ -1083,6 +1083,8 @@ EXPORT_SYMBOL_GPL(usb_calc_bus_time);
 
 /*-------------------------------------------------------------------------*/
 
+static bool list_error;
+
 /**
  * usb_hcd_link_urb_to_ep - add an URB to its endpoint queue
  * @hcd: host controller to which @urb was submitted
@@ -1193,6 +1195,25 @@ void usb_hcd_unlink_urb_from_ep(struct u
 {
 	/* clear all state linking urb to this dev (and hcd) */
 	spin_lock(&hcd_urb_list_lock);
+	{
+		struct list_head *cur = &urb->urb_list;
+		struct list_head *prev = cur->prev;
+		struct list_head *next = cur->next;
+
+		if (prev->next != cur && !list_error) {
+			list_error = true;
+			dev_err(&urb->dev->dev,
+				"ep %x list del corruption prev: %p %p %p %p %p\n",
+				urb->ep->desc.bEndpointAddress,
+				cur, prev, prev->next, next, next->prev);
+			dev_err(&urb->dev->dev,
+				"head %p urb %p urbprev %p urbnext %p\n",
+				&urb->ep->urb_list, urb,
+				list_entry(prev, struct urb, urb_list),
+				list_entry(next, struct urb, urb_list));
+			mdelay(10000);
+		}
+	}
 	list_del_init(&urb->urb_list);
 	spin_unlock(&hcd_urb_list_lock);
 }

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Brian Haley @ 2012-10-22 18:04 UTC (permalink / raw)
  To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <508123AC.5080208@parallels.com>

On 10/19/2012 05:55 AM, Pavel Emelyanov wrote:
> The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but
> cannot be get via sockopt API. The only way we can find the device id a
> socket is bound to is via sock-diag interface. But the diag works only on
> hashed sockets, while the opt in question can be set for yet unhashed one.
> 
> That said, in order to know what device a socket is bound to (we do want
> to know this in checkpoint-restore project) I propose to make this option
> getsockopt-able and report the respective device index.
> 
> Another solution to the problem might be to teach the sock-diag reporting
> info on unhashed sockets. Should I go this way instead?
> 
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
> 
> ---
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8a146cf..c49412c 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
>  	case SO_NOFCS:
>  		v.val = sock_flag(sk, SOCK_NOFCS);
>  		break;
> +	case SO_BINDTODEVICE:
> +		v.val = sk->sk_bound_dev_if;
> +		break;
>  	default:
>  		return -ENOPROTOOPT;
>  	}

Doesn't this make the set and get non-symmetrical?  For example, setsockopt()
would take "eth0", but getsockopt() would return 2.  The following patch would
return a string, or -ENODEV if not set.

-Brian

---

Change getsockopt(SO_BINDTODEVICE) to be symmetrical with setsockopt() by
returning the interface name as a string.

Signed-off-by: Brian Haley <brian.haley@hp.com>

diff --git a/net/core/sock.c b/net/core/sock.c
index c49412c..69b9d92 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -505,7 +505,8 @@ struct dst_entry *sk_dst_check(struct sock *sk, u32 cookie)
 }
 EXPORT_SYMBOL(sk_dst_check);

-static int sock_bindtodevice(struct sock *sk, char __user *optval, int optlen)
+static int sock_setbindtodevice(struct sock *sk, char __user *optval,
+				int optlen)
 {
 	int ret = -ENOPROTOOPT;
 #ifdef CONFIG_NETDEVICES
@@ -562,6 +563,49 @@ out:
 	return ret;
 }

+static int sock_getbindtodevice(struct sock *sk, char __user *optval,
+				int __user *optlen, int len)
+{
+	int ret = -ENOPROTOOPT;
+#ifdef CONFIG_NETDEVICES
+	struct net *net = sock_net(sk);
+	struct net_device *dev;
+	char devname[IFNAMSIZ];
+
+	ret = 0;
+	if (sk->sk_bound_dev_if == 0)
+		goto out;
+
+	ret = -EINVAL;
+	if (len < IFNAMSIZ)
+		goto out;
+	if (len > IFNAMSIZ)
+		len = IFNAMSIZ;
+
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
+	if (dev)
+		strcpy(dev->name, devname);
+	rcu_read_unlock();
+	ret = -ENODEV;
+	if (!dev)
+		goto out;
+
+	ret = -EFAULT;
+	if (copy_to_user(optval, devname, len))
+		goto out;
+
+	if (put_user(len, optlen))
+		goto out;
+
+	ret = 0;
+
+out:
+#endif
+
+	return ret;
+}
+
 static inline void sock_valbool_flag(struct sock *sk, int bit, int valbool)
 {
 	if (valbool)
@@ -589,7 +633,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 	 */

 	if (optname == SO_BINDTODEVICE)
-		return sock_bindtodevice(sk, optval, optlen);
+		return sock_setbindtodevice(sk, optval, optlen);

 	if (optlen < sizeof(int))
 		return -EINVAL;
@@ -1074,9 +1118,10 @@ int sock_getsockopt(struct socket *sock, int level, int
optname,
 	case SO_NOFCS:
 		v.val = sock_flag(sk, SOCK_NOFCS);
 		break;
+
 	case SO_BINDTODEVICE:
-		v.val = sk->sk_bound_dev_if;
-		break;
+		return sock_getbindtodevice(sk, optval, optlen, len);
+
 	default:
 		return -ENOPROTOOPT;
 	}

^ permalink raw reply related

* [PATCH v3 0/4] make cadence ethernet drivers build on any architecture
From: Joachim Eastwood @ 2012-10-22 18:45 UTC (permalink / raw)
  To: nicolas.ferre, davem, hskinnemoen, egtvedt, plagnioj, bgat
  Cc: netdev, linux-arm-kernel, Joachim Eastwood

This series makes the at91_ether and macb driver build-able on any architecture.

Patch 1 fixes a integer truncated warning in macb, which will show up when compiling for 64-bit, before enabling macb driver for other architectures.

Patch 2 removes the HAVE_NET_MACB from the cadence Kconfig and this allow macb to be built on any architecture. The macb driver requires no other modifications.

Patch 4 moves a bootloader quirk for CSB337 from at91_ether into platform data as a flag. This allow us to remove the last mach include and build this driver on any architecture. It also make it easier to share the address setup between at91_ether and macb.


The next series will make the at91_ether driver use address setting and statistics from macb. There is also a clean up of print outs plus some comment/style fixes.

Patch log:
v3 - Include integer truncated warning fix for macb
v2 - Use macb platform data to support reversed ethernet address and squash HAVE_NET_MACB removal patches.

Joachim Eastwood (4):
  net/macb: fix truncate warnings
  net/cadence: get rid of HAVE_NET_MACB
  net/at91_ether: select MACB in Kconfig
  net/at91_ether: add pdata flag for reverse Eth addr

 arch/arm/mach-at91/Kconfig                | 4 ----
 arch/arm/mach-at91/board-csb337.c         | 2 ++
 arch/avr32/Kconfig                        | 1 -
 drivers/net/ethernet/cadence/Kconfig      | 8 +-------
 drivers/net/ethernet/cadence/Makefile     | 2 +-
 drivers/net/ethernet/cadence/at91_ether.c | 5 ++---
 drivers/net/ethernet/cadence/macb.c       | 8 ++++----
 include/linux/platform_data/macb.h        | 1 +
 8 files changed, 11 insertions(+), 20 deletions(-)

-- 
1.7.12.4

^ permalink raw reply

* [PATCH v3 1/4] net/macb: fix truncate warnings
From: Joachim Eastwood @ 2012-10-22 18:45 UTC (permalink / raw)
  To: nicolas.ferre, davem, hskinnemoen, egtvedt, plagnioj, bgat
  Cc: netdev, linux-arm-kernel, Joachim Eastwood
In-Reply-To: <1350931534-8416-1-git-send-email-manabian@gmail.com>

When building macb on x86_64 the following warnings show up:
  drivers/net/ethernet/cadence/macb.c: In function macb_interrupt:
  drivers/net/ethernet/cadence/macb.c:556:4: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  drivers/net/ethernet/cadence/macb.c: In function macb_reset_hw:
  drivers/net/ethernet/cadence/macb.c:792:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  drivers/net/ethernet/cadence/macb.c:793:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  drivers/net/ethernet/cadence/macb.c:796:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]

Use -1 insted of ~0UL, as done in other places in the driver,
to silence these warnings.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 drivers/net/ethernet/cadence/macb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 6c84a11..6a4f499 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -553,7 +553,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 	while (status) {
 		/* close possible race with dev_close */
 		if (unlikely(!netif_running(dev))) {
-			macb_writel(bp, IDR, ~0UL);
+			macb_writel(bp, IDR, -1);
 			break;
 		}
 
@@ -789,11 +789,11 @@ static void macb_reset_hw(struct macb *bp)
 	macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
 
 	/* Clear all status flags */
-	macb_writel(bp, TSR, ~0UL);
-	macb_writel(bp, RSR, ~0UL);
+	macb_writel(bp, TSR, -1);
+	macb_writel(bp, RSR, -1);
 
 	/* Disable all interrupts */
-	macb_writel(bp, IDR, ~0UL);
+	macb_writel(bp, IDR, -1);
 	macb_readl(bp, ISR);
 }
 
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH v3 2/4] net/cadence: get rid of HAVE_NET_MACB
From: Joachim Eastwood @ 2012-10-22 18:45 UTC (permalink / raw)
  To: nicolas.ferre, davem, hskinnemoen, egtvedt, plagnioj, bgat
  Cc: netdev, linux-arm-kernel, Joachim Eastwood
In-Reply-To: <1350931534-8416-1-git-send-email-manabian@gmail.com>

macb is a platform driver and there is nothing that prevents
this driver from being built on non-ARM/AVR32 platforms.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/mach-at91/Kconfig           | 4 ----
 arch/avr32/Kconfig                   | 1 -
 drivers/net/ethernet/cadence/Kconfig | 5 -----
 3 files changed, 10 deletions(-)

diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index d846b6e..72020fb 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -45,7 +45,6 @@ config SOC_AT91RM9200
 config SOC_AT91SAM9260
 	bool "AT91SAM9260, AT91SAM9XE or AT91SAM9G20"
 	select HAVE_AT91_DBGU0
-	select HAVE_NET_MACB
 	select SOC_AT91SAM9
 	help
 	  Select this if you are using one of Atmel's AT91SAM9260, AT91SAM9XE
@@ -63,7 +62,6 @@ config SOC_AT91SAM9263
 	bool "AT91SAM9263"
 	select HAVE_AT91_DBGU1
 	select HAVE_FB_ATMEL
-	select HAVE_NET_MACB
 	select SOC_AT91SAM9
 
 config SOC_AT91SAM9RL
@@ -76,7 +74,6 @@ config SOC_AT91SAM9G45
 	bool "AT91SAM9G45 or AT91SAM9M10 families"
 	select HAVE_AT91_DBGU1
 	select HAVE_FB_ATMEL
-	select HAVE_NET_MACB
 	select SOC_AT91SAM9
 	help
 	  Select this if you are using one of Atmel's AT91SAM9G45 family SoC.
@@ -86,7 +83,6 @@ config SOC_AT91SAM9X5
 	bool "AT91SAM9x5 family"
 	select HAVE_AT91_DBGU0
 	select HAVE_FB_ATMEL
-	select HAVE_NET_MACB
 	select SOC_AT91SAM9
 	help
 	  Select this if you are using one of Atmel's AT91SAM9x5 family SoC.
diff --git a/arch/avr32/Kconfig b/arch/avr32/Kconfig
index 06e73bf..09f9fa8 100644
--- a/arch/avr32/Kconfig
+++ b/arch/avr32/Kconfig
@@ -80,7 +80,6 @@ config PLATFORM_AT32AP
 	select ARCH_REQUIRE_GPIOLIB
 	select GENERIC_ALLOCATOR
 	select HAVE_FB_ATMEL
-	select HAVE_NET_MACB
 
 #
 # CPU types
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 57f78abe..5d1ea30 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -2,13 +2,9 @@
 # Atmel device configuration
 #
 
-config HAVE_NET_MACB
-	bool
-
 config NET_CADENCE
 	bool "Cadence devices"
 	default y
-	depends on HAVE_NET_MACB || (ARM && ARCH_AT91RM9200)
 	---help---
 	  If you have a network (Ethernet) card belonging to this class, say Y.
 	  Make sure you know the name of your card. Read the Ethernet-HOWTO,
@@ -34,7 +30,6 @@ config ARM_AT91_ETHER
 
 config MACB
 	tristate "Cadence MACB/GEM support"
-	depends on HAVE_NET_MACB
 	select PHYLIB
 	---help---
 	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH v3 3/4] net/at91_ether: select MACB in Kconfig
From: Joachim Eastwood @ 2012-10-22 18:45 UTC (permalink / raw)
  To: nicolas.ferre, davem, hskinnemoen, egtvedt, plagnioj, bgat
  Cc: netdev, linux-arm-kernel, Joachim Eastwood
In-Reply-To: <1350931534-8416-1-git-send-email-manabian@gmail.com>

Now that HAVE_NET_MACB is gone let's just select MACB to
satisfy the dependecies in at91_ether.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 drivers/net/ethernet/cadence/Kconfig  | 2 +-
 drivers/net/ethernet/cadence/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 5d1ea30..f6d0956 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -23,7 +23,7 @@ config ARM_AT91_ETHER
 	tristate "AT91RM9200 Ethernet support"
 	depends on ARM && ARCH_AT91RM9200
 	select NET_CORE
-	select PHYLIB
+	select MACB
 	---help---
 	  If you wish to compile a kernel for the AT91RM9200 and enable
 	  ethernet support, then you should always answer Y to this.
diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
index 798b1e0..9068b83 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,5 +2,5 @@
 # Makefile for the Atmel network device drivers.
 #
 
-obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o macb.o
+obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o
 obj-$(CONFIG_MACB) += macb.o
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH v3 4/4] net/at91_ether: add pdata flag for reverse Eth addr
From: Joachim Eastwood @ 2012-10-22 18:45 UTC (permalink / raw)
  To: nicolas.ferre, davem, hskinnemoen, egtvedt, plagnioj, bgat
  Cc: netdev, linux-arm-kernel, Joachim Eastwood
In-Reply-To: <1350931534-8416-1-git-send-email-manabian@gmail.com>

This will allow us to remove the last mach include from at91_ether
and also make it easier to share address setup with macb.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/mach-at91/board-csb337.c         | 2 ++
 drivers/net/ethernet/cadence/Kconfig      | 1 -
 drivers/net/ethernet/cadence/at91_ether.c | 5 ++---
 include/linux/platform_data/macb.h        | 1 +
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-at91/board-csb337.c b/arch/arm/mach-at91/board-csb337.c
index 3e37437..aa9b320 100644
--- a/arch/arm/mach-at91/board-csb337.c
+++ b/arch/arm/mach-at91/board-csb337.c
@@ -53,6 +53,8 @@ static void __init csb337_init_early(void)
 static struct macb_platform_data __initdata csb337_eth_data = {
 	.phy_irq_pin	= AT91_PIN_PC2,
 	.is_rmii	= 0,
+	/* The CSB337 bootloader stores the MAC the wrong-way around */
+	.rev_eth_addr	= 1,
 };
 
 static struct at91_usbh_data __initdata csb337_usbh_data = {
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index f6d0956..40172d1 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -21,7 +21,6 @@ if NET_CADENCE
 
 config ARM_AT91_ETHER
 	tristate "AT91RM9200 Ethernet support"
-	depends on ARM && ARCH_AT91RM9200
 	select NET_CORE
 	select MACB
 	---help---
diff --git a/drivers/net/ethernet/cadence/at91_ether.c b/drivers/net/ethernet/cadence/at91_ether.c
index 375d272..b92815a 100644
--- a/drivers/net/ethernet/cadence/at91_ether.c
+++ b/drivers/net/ethernet/cadence/at91_ether.c
@@ -32,8 +32,6 @@
 #include <linux/phy.h>
 #include <linux/io.h>
 
-#include <asm/mach-types.h>
-
 #include "macb.h"
 
 #define DRV_NAME	"at91_ether"
@@ -61,9 +59,10 @@
 
 static short __init unpack_mac_address(struct net_device *dev, unsigned int hi, unsigned int lo)
 {
+	struct macb *lp = netdev_priv(dev);
 	char addr[6];
 
-	if (machine_is_csb337()) {
+	if (lp->board_data.rev_eth_addr) {
 		addr[5] = (lo & 0xff);			/* The CSB337 bootloader stores the MAC the wrong-way around */
 		addr[4] = (lo & 0xff00) >> 8;
 		addr[3] = (lo & 0xff0000) >> 16;
diff --git a/include/linux/platform_data/macb.h b/include/linux/platform_data/macb.h
index b081c72..044a124 100644
--- a/include/linux/platform_data/macb.h
+++ b/include/linux/platform_data/macb.h
@@ -12,6 +12,7 @@ struct macb_platform_data {
 	u32		phy_mask;
 	int		phy_irq_pin;	/* PHY IRQ */
 	u8		is_rmii;	/* using RMII interface? */
+	u8		rev_eth_addr;	/* reverse Ethernet address byte order */
 };
 
 #endif /* __MACB_PDATA_H__ */
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH] net: fix secpath kmemleak
From: Eric Dumazet @ 2012-10-22 19:03 UTC (permalink / raw)
  To: Mike Kazantsev, David Miller; +Cc: Paul Moore, netdev, linux-mm
In-Reply-To: <1350926647.8609.1006.camel@edumazet-glaptop>

From: Eric Dumazet <edumazet@google.com>

Mike Kazantsev found 3.5 kernels and beyond were leaking memory,
and tracked the faulty commit to a1c7fff7e18f59e (net:
netdev_alloc_skb() use build_skb()

While this commit seems fine, it uncovered a bug introduced
in commit bad43ca8325 (net: introduce skb_try_coalesce()), in function
kfree_skb_partial() :

If head is stolen, we free the sk_buff,
without removing references on secpath (skb->sp).

So IPsec + IP defrag/reassembly (using skb coalescing), or
TCP coalescing could leak secpath objects.

Fix this bug by calling skb_release_head_state(skb) to properly
release all possible references to linked objects.

Reported-by: Mike Kazantsev <mk.fraggod@gmail.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Bisected-by: Mike Kazantsev <mk.fraggod@gmail.com>
Tested-by: Mike Kazantsev <mk.fraggod@gmail.com>
---
It seems TCP stack could immediately release secpath references instead
of waiting skb are eaten by consumer, thats will be a followup patch.

 net/core/skbuff.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 6e04b1f..4007c14 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3379,10 +3379,12 @@ EXPORT_SYMBOL(__skb_warn_lro_forwarding);
 
 void kfree_skb_partial(struct sk_buff *skb, bool head_stolen)
 {
-	if (head_stolen)
+	if (head_stolen) {
+		skb_release_head_state(skb);
 		kmem_cache_free(skbuff_head_cache, skb);
-	else
+	} else {
 		__kfree_skb(skb);
+	}
 }
 EXPORT_SYMBOL(kfree_skb_partial);
 



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: [PATCH net-next] tcp: RFC 5961 5.2 Blind Data Injection Attack Mitigation
From: David Miller @ 2012-10-22 19:11 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, ncardwell, ycheng, hkchu
In-Reply-To: <1350885431.8609.250.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 22 Oct 2012 07:57:11 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> RFC 5961 5.2 [Blind Data Injection Attack].[Mitigation]
> 
>   All TCP stacks MAY implement the following mitigation.  TCP stacks
>   that implement this mitigation MUST add an additional input check to
>   any incoming segment.  The ACK value is considered acceptable only if
>   it is in the range of ((SND.UNA - MAX.SND.WND) <= SEG.ACK <=
>   SND.NXT).  All incoming segments whose ACK value doesn't satisfy the
>   above condition MUST be discarded and an ACK sent back.
> 
> Move tcp_send_challenge_ack() before tcp_ack() to avoid a forward
> declaration.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] tcp: speedup SIOCINQ ioctl
From: David Miller @ 2012-10-22 19:11 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1350886016.8609.266.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 22 Oct 2012 08:06:56 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> SIOCINQ can use the lock_sock_fast() version to avoid double acquisition
> of socket lock.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] ipv4: 16 slots in initial fib_info hash table
From: David Miller @ 2012-10-22 19:11 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1350886329.8609.276.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 22 Oct 2012 08:12:09 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> A small host typically needs ~10 fib_info structures, so create initial
> hash table with 16 slots instead of only one. This removes potential
> false sharing and reallocs/rehashes (1->2->4->8->16)
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH] tcp: add SYN/data info to TCP_INFO
From: David Miller @ 2012-10-22 19:16 UTC (permalink / raw)
  To: ycheng; +Cc: ncardwell, edumazet, hkchu, netdev
In-Reply-To: <1350695684-26962-1-git-send-email-ycheng@google.com>

From: Yuchung Cheng <ycheng@google.com>
Date: Fri, 19 Oct 2012 18:14:44 -0700

> Add a bit TCPI_OPT_SYN_DATA (32) to the socket option TCP_INFO:tcpi_options.
> It's set if the data in SYN (sent or received) is acked by SYN-ACK. Server or
> client application can use this information to check Fast Open success rate.
> 
> Signed-off-by: Yuchung Cheng <ycheng@google.com>

Applied to 'net'

^ permalink raw reply

* Re: [PATCH] qla3xxx: Ensure request/response queue addr writes to the registers
From: David Miller @ 2012-10-22 19:17 UTC (permalink / raw)
  To: joe.jin
  Cc: jitendra.kalsaria, ron.mercer, Linux-Driver, akpm, netdev,
	linux-kernel, greg.marsden
In-Reply-To: <50849604.9050109@oracle.com>

From: Joe Jin <joe.jin@oracle.com>
Date: Mon, 22 Oct 2012 08:40:36 +0800

> Before use the request and response queue addr, make sure it has wrote
> to the registers.
> 
> Signed-off-by: Joe Jin <joe.jin@oracle.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: fix secpath kmemleak
From: David Miller @ 2012-10-22 19:17 UTC (permalink / raw)
  To: eric.dumazet; +Cc: mk.fraggod, paul, netdev, linux-mm
In-Reply-To: <1350932620.8609.1142.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 22 Oct 2012 21:03:40 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> Mike Kazantsev found 3.5 kernels and beyond were leaking memory,
> and tracked the faulty commit to a1c7fff7e18f59e (net:
> netdev_alloc_skb() use build_skb()
> 
> While this commit seems fine, it uncovered a bug introduced
> in commit bad43ca8325 (net: introduce skb_try_coalesce()), in function
> kfree_skb_partial() :
> 
> If head is stolen, we free the sk_buff,
> without removing references on secpath (skb->sp).
> 
> So IPsec + IP defrag/reassembly (using skb coalescing), or
> TCP coalescing could leak secpath objects.
> 
> Fix this bug by calling skb_release_head_state(skb) to properly
> release all possible references to linked objects.
> 
> Reported-by: Mike Kazantsev <mk.fraggod@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Bisected-by: Mike Kazantsev <mk.fraggod@gmail.com>
> Tested-by: Mike Kazantsev <mk.fraggod@gmail.com>

Applied and queued up for -stable, thanks!

> It seems TCP stack could immediately release secpath references instead
> of waiting skb are eaten by consumer, thats will be a followup patch.

Indeed.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [RFC PATCH v2 4/6] net/core: apply pm_runtime_set_memalloc_noio on network devices
From: Alan Stern @ 2012-10-22 19:18 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-kernel, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
	Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
	netdev, linux-usb, linux-pm, linux-mm, Eric Dumazet,
	David Decotigny, Tom Herbert, Ingo Molnar
In-Reply-To: <1350894794-1494-5-git-send-email-ming.lei@canonical.com>

On Mon, 22 Oct 2012, Ming Lei wrote:

> Deadlock might be caused by allocating memory with GFP_KERNEL in
> runtime_resume callback of network devices in iSCSI situation, so
> mark network devices and its ancestor as 'memalloc_noio_resume'
> with the introduced pm_runtime_set_memalloc_noio().

Is this really needed?  Even with iSCSI, doesn't register_disk() have
to be called for the underlying block device?  And given your 3/6
patch, wouldn't that mark the network device?

Alan Stern

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [nf-next PATCH v2] ipvs: fix build errors related to config option combinations
From: Jesper Dangaard Brouer @ 2012-10-22 19:22 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Simon Horman
  Cc: Jesper Dangaard Brouer, fengguang.wu, yuanhan.liu, netdev,
	lvs-devel, netfilter-devel, Hans Schillstrom

Fix two build error introduced by commit 63dca2c0:
 "ipvs: Fix faulty IPv6 extension header handling in IPVS"

First build error was fairly trivial and can occur, when
CONFIG_IP_VS_IPV6 is disabled.

The second build error was tricky, and can occur when deselecting
both all Netfilter and IPVS, but selecting CONFIG_IPV6.  This is
caused by "kernel/sysctl_binary.c" including "net/ip_vs.h", which
includes "linux/netfilter_ipv6/ip6_tables.h" causing include
of "include/linux/netfilter/x_tables.h" which then cannot find
the typedef nf_hookfn.

Fix this by only including "linux/netfilter_ipv6/ip6_tables.h" in
case of CONFIG_IP_VS_IPV6 as its already used to guard the usage
of ipv6_find_hdr().

Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Reported-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---

 include/net/ip_vs.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index a681ad6..68c69d5 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -22,7 +22,7 @@
 #include <linux/ip.h>
 #include <linux/ipv6.h>			/* for struct ipv6hdr */
 #include <net/ipv6.h>
-#if IS_ENABLED(CONFIG_IPV6)
+#if IS_ENABLED(CONFIG_IP_VS_IPV6)
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #endif
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
@@ -212,8 +212,9 @@ ip_vs_fill_iph_addr_only(int af, const struct sk_buff *skb,
 			(struct ipv6hdr *)skb_network_header(skb);
 		iphdr->saddr.in6 = iph->saddr;
 		iphdr->daddr.in6 = iph->daddr;
-	} else {
+	} else
 #endif
+	{
 		const struct iphdr *iph =
 			(struct iphdr *)skb_network_header(skb);
 		iphdr->saddr.ip = iph->saddr;


^ permalink raw reply related

* Re: [nf-next PATCH] ipvs: fix build error when CONFIG_IP_VS_IPV6 is disabled
From: Jesper Dangaard Brouer @ 2012-10-22 19:24 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Jesper Dangaard Brouer, Simon Horman, fengguang.wu, yuanhan.liu,
	netdev, lvs-devel, netfilter-devel
In-Reply-To: <20121022163034.GA6698@1984>

On Mon, 2012-10-22 at 18:30 +0200, Pablo Neira Ayuso wrote:
> On Mon, Oct 22, 2012 at 05:39:43PM +0200, Jesper Dangaard Brouer wrote:
> > 
> > Hi Pablo,
> > 
> > Argh - there is one more CONFIG option combi that can cause issues.
> > 
> >  *So hold off on this patch*
> > 
> > Which is related to including linux/netfilter_ipv6/ip6_tables.h
> > under CONFIG_IPV6 when CONFIG_NETFILTER is disabled, as
> > kernel/sysctl_binary.c include net/ip_vs.h.  I'll send a new patch
> > soon!
> 
> Ok, wait for it.

Posted with subj.:
 [nf-next PATCH v2] ipvs: fix build errors related to config option combinations

(Again, I'm very sorry for letting this slip through...)



^ permalink raw reply

* Re: [PATCH net-next 1/2] r8169: enable ALDPS for power saving
From: Francois Romieu @ 2012-10-22 19:27 UTC (permalink / raw)
  To: Hayes Wang; +Cc: netdev, linux-kernel, jean
In-Reply-To: <1350893153-18320-1-git-send-email-hayeswang@realtek.com>

Hayes Wang <hayeswang@realtek.com> :
> Enable ALDPS function to save power when link down. Note that the
> feature should be set after the other PHY settings. And the firmware
> is necessary. Don't enable it without loading the firmware.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 46 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index e7ff886..ba29e4d 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -687,6 +687,7 @@ enum features {
>  	RTL_FEATURE_WOL		= (1 << 0),
>  	RTL_FEATURE_MSI		= (1 << 1),
>  	RTL_FEATURE_GMII	= (1 << 2),
> +	RTL_FEATURE_EXTENDED	= (1 << 3),

Is there a specific reason why it is not named RTL_FEATURE_ALDPS ?

RTL_FEATURE_EXTENDED is anything but enlightning.

>  };
>  
>  struct rtl8169_counters {
> @@ -2394,8 +2395,10 @@ static void rtl_apply_firmware(struct rtl8169_private *tp)
>  	struct rtl_fw *rtl_fw = tp->rtl_fw;
>  
>  	/* TODO: release firmware once rtl_phy_write_fw signals failures. */
> -	if (!IS_ERR_OR_NULL(rtl_fw))
> +	if (!IS_ERR_OR_NULL(rtl_fw)) {
>  		rtl_phy_write_fw(tp, rtl_fw);
> +		tp->features |= RTL_FEATURE_EXTENDED;
> +	}


>  }
>  
>  static void rtl_apply_firmware_cond(struct rtl8169_private *tp, u8 reg, u16 val)
> @@ -3178,6 +3181,12 @@ static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp)
>  	rtl_w1w0_phy(tp, 0x19, 0x0000, 0x0001);
>  	rtl_w1w0_phy(tp, 0x10, 0x0000, 0x0400);
>  	rtl_writephy(tp, 0x1f, 0x0000);
> +
> +	/* ALDPS enable */
> +	if (tp->features & RTL_FEATURE_EXTENDED) {
> +		rtl_writephy(tp, 0x1f, 0x0000);
> +		rtl_w1w0_phy(tp, 0x15, 0x1000, 0x0000);
> +	}


This same code fragment will be repeated 3 extra times. The patch duplicates
a different fragment 3 times and the driver already repeats the disable
ALDPS sequence 3 times.

What about some factoring out frenzy ? :o)

[...]
> @@ -6391,6 +6433,8 @@ static void rtl8169_net_suspend(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
>  
> +	tp->features &= ~RTL_FEATURE_EXTENDED;
> +

The commit message does not explain this part.

What are you trying to achieve ?


After this patch the driver would look like:
1. disable ALDPS before setting firmware (unmodified by patch)
   RTL_GIGA_MAC_VER_29 "RTL8105e"
   RTL_GIGA_MAC_VER_30 "RTL8105e"
   RTL_GIGA_MAC_VER_37 "RTL8402"
   RTL_GIGA_MAC_VER_39 "RTL8106e"

2. apply_firmware (unmodified by patch)
   RTL_GIGA_MAC_VER_25 "RTL8168d/8111d"
   RTL_GIGA_MAC_VER_26 "RTL8168d/8111d"
   RTL_GIGA_MAC_VER_29 "RTL8105e"
   RTL_GIGA_MAC_VER_30 "RTL8105e"
   RTL_GIGA_MAC_VER_32 "RTL8168e/8111e"
   RTL_GIGA_MAC_VER_33 "RTL8168e/8111e"
   RTL_GIGA_MAC_VER_34 "RTL8168evl/8111evl"
   RTL_GIGA_MAC_VER_35 "RTL8168f/8111f"
   RTL_GIGA_MAC_VER_36 "RTL8168f/8111f"
   RTL_GIGA_MAC_VER_37 "RTL8402"
   RTL_GIGA_MAC_VER_38 "RTL8411"
   RTL_GIGA_MAC_VER_39 "RTL8106e"
   RTL_GIGA_MAC_VER_40 "RTL8168g/8111g"

3. enable ALDPS after firmware
   RTL_GIGA_MAC_VER_34 "RTL8168evl/8111evl"
   RTL_GIGA_MAC_VER_35 "RTL8168f/8111f"
   RTL_GIGA_MAC_VER_36 "RTL8168f/8111f"
   RTL_GIGA_MAC_VER_37 "RTL8402"
   RTL_GIGA_MAC_VER_38 "RTL8411"
   RTL_GIGA_MAC_VER_39 "RTL8106e"
   RTL_GIGA_MAC_VER_40 "RTL8168g/8111g"

The disable/enable ALDPS code is not trivially balanced.

Do we exactly perform the required ALDPS operations ? Nothing more,
nothing less ?

-- 
Ueimor

^ permalink raw reply

* Re: listen(2) backlog changes in or around Linux 3.1?
From: Vijay Subramanian @ 2012-10-22 20:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Vijay Subramanian, enh, Venkat Venkatsubra, netdev
In-Reply-To: <1350642562.2293.411.camel@edumazet-glaptop>


>
> If we send a SYNACK, then receive the ACK from client, and the acceptq
> is full, we should reset the connexion. Right now we have kind of stupid
> situation, were we drop the ACK, and leave the REQ in the SYN_RECV
> state, so we retransmit SYNACKS.
>


It seems the third ack is remembered in inet_rsk(req)->acked in
tcp_check_req(). However, because of the order in which the tests are performed, 
server stills retransmits the synack needlessly. Following patch 
(for review) prevents this synack retransmission if third ack has been 
received.

The request_sock will expire in around 30 seconds and will be dropped if it does
not move into accept_queue by then.  Maybe we should also call 
req->rsk_ops->send_reset(sk,skb); 
when the request_sock expires and is dropped?


net/ipv4/inet_connection_sock.c |    5 ++---
  1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index d34ce29..4e8e52e 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -598,9 +598,8 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
                                                &expire, &resend);
                                 req->rsk_ops->syn_ack_timeout(parent, req);
                                 if (!expire &&
-                                   (!resend ||
-                                    !req->rsk_ops->rtx_syn_ack(parent, req, NULL) ||
-                                    inet_rsk(req)->acked)) {
+                                   (!resend || inet_rsk(req)->acked ||
+                                    !req->rsk_ops->rtx_syn_ack(parent, req, NULL))) {
                                         unsigned long timeo;

                                         if (req->retrans++ == 0)

Thanks,
Vijay

^ permalink raw reply related

* Re: listen(2) backlog changes in or around Linux 3.1?
From: Eric Dumazet @ 2012-10-22 20:08 UTC (permalink / raw)
  To: Vijay Subramanian; +Cc: enh, Venkat Venkatsubra, netdev
In-Reply-To: <alpine.DEB.2.00.1210221251520.1684@cleese>

On Mon, 2012-10-22 at 13:00 -0700, Vijay Subramanian wrote:
> >
> > If we send a SYNACK, then receive the ACK from client, and the acceptq
> > is full, we should reset the connexion. Right now we have kind of stupid
> > situation, were we drop the ACK, and leave the REQ in the SYN_RECV
> > state, so we retransmit SYNACKS.
> >
> 
> 
> It seems the third ack is remembered in inet_rsk(req)->acked in
> tcp_check_req(). However, because of the order in which the tests are performed, 
> server stills retransmits the synack needlessly. Following patch 
> (for review) prevents this synack retransmission if third ack has been 
> received.
> 
> The request_sock will expire in around 30 seconds and will be dropped if it does
> not move into accept_queue by then.  Maybe we should also call 
> req->rsk_ops->send_reset(sk,skb); 
> when the request_sock expires and is dropped?
> 

Not sure its needed, and we are under stress.

> 
> net/ipv4/inet_connection_sock.c |    5 ++---
>   1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index d34ce29..4e8e52e 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -598,9 +598,8 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
>                                                 &expire, &resend);
>                                  req->rsk_ops->syn_ack_timeout(parent, req);
>                                  if (!expire &&
> -                                   (!resend ||
> -                                    !req->rsk_ops->rtx_syn_ack(parent, req, NULL) ||
> -                                    inet_rsk(req)->acked)) {
> +                                   (!resend || inet_rsk(req)->acked ||
> +                                    !req->rsk_ops->rtx_syn_ack(parent, req, NULL))) {
>                                          unsigned long timeo;
> 
>                                          if (req->retrans++ == 0)

I wonder then if we dont need to retransmit the synack when req moves
into accept_queue then ?

Or else how the client can 'knows' it can send data to server ?

All these facilities sound very complex and not really usable by clients
(ie users not willing to wait more than few seconds anyway)

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Pavel Emelyanov @ 2012-10-22 20:28 UTC (permalink / raw)
  To: Brian Haley; +Cc: Linux Netdev List, David Miller
In-Reply-To: <50858ABD.2000206@hp.com>

On 10/22/2012 10:04 PM, Brian Haley wrote:
> On 10/19/2012 05:55 AM, Pavel Emelyanov wrote:
>> The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but
>> cannot be get via sockopt API. The only way we can find the device id a
>> socket is bound to is via sock-diag interface. But the diag works only on
>> hashed sockets, while the opt in question can be set for yet unhashed one.
>>
>> That said, in order to know what device a socket is bound to (we do want
>> to know this in checkpoint-restore project) I propose to make this option
>> getsockopt-able and report the respective device index.
>>
>> Another solution to the problem might be to teach the sock-diag reporting
>> info on unhashed sockets. Should I go this way instead?
>>
>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>>
>> ---
>>
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 8a146cf..c49412c 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
>>  	case SO_NOFCS:
>>  		v.val = sock_flag(sk, SOCK_NOFCS);
>>  		break;
>> +	case SO_BINDTODEVICE:
>> +		v.val = sk->sk_bound_dev_if;
>> +		break;
>>  	default:
>>  		return -ENOPROTOOPT;
>>  	}
> 
> Doesn't this make the set and get non-symmetrical?  For example, setsockopt()
> would take "eth0", but getsockopt() would return 2.

It will, but since device name and index are two equal device "IDs" I assumed
it would be OK.

However, some comments inline.

> The following patch would return a string, or -ENODEV if not set.
> 
> -Brian
> 
> ---
> 
> Change getsockopt(SO_BINDTODEVICE) to be symmetrical with setsockopt() by
> returning the interface name as a string.
> 
> Signed-off-by: Brian Haley <brian.haley@hp.com>
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index c49412c..69b9d92 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -505,7 +505,8 @@ struct dst_entry *sk_dst_check(struct sock *sk, u32 cookie)
>  }
>  EXPORT_SYMBOL(sk_dst_check);
> 
> -static int sock_bindtodevice(struct sock *sk, char __user *optval, int optlen)
> +static int sock_setbindtodevice(struct sock *sk, char __user *optval,
> +				int optlen)
>  {
>  	int ret = -ENOPROTOOPT;
>  #ifdef CONFIG_NETDEVICES
> @@ -562,6 +563,49 @@ out:
>  	return ret;
>  }
> 
> +static int sock_getbindtodevice(struct sock *sk, char __user *optval,
> +				int __user *optlen, int len)
> +{
> +	int ret = -ENOPROTOOPT;
> +#ifdef CONFIG_NETDEVICES
> +	struct net *net = sock_net(sk);
> +	struct net_device *dev;
> +	char devname[IFNAMSIZ];
> +
> +	ret = 0;
> +	if (sk->sk_bound_dev_if == 0)
> +		goto out;

It will return 0 if device is not set, thus making it impossible to detect
this situation.

> +	ret = -EINVAL;
> +	if (len < IFNAMSIZ)
> +		goto out;
> +	if (len > IFNAMSIZ)
> +		len = IFNAMSIZ;
> +
> +	rcu_read_lock();
> +	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
> +	if (dev)
> +		strcpy(dev->name, devname);
> +	rcu_read_unlock();
> +	ret = -ENODEV;
> +	if (!dev)
> +		goto out;
> +
> +	ret = -EFAULT;
> +	if (copy_to_user(optval, devname, len))
> +		goto out;
> +
> +	if (put_user(len, optlen))
> +		goto out;

What's the point in reporting IFNAMSIZ to the userspace always, taking
into account that this constant is exported there anyway?

> +	ret = 0;
> +
> +out:
> +#endif
> +
> +	return ret;
> +}
> +
>  static inline void sock_valbool_flag(struct sock *sk, int bit, int valbool)
>  {
>  	if (valbool)
> @@ -589,7 +633,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>  	 */
> 
>  	if (optname == SO_BINDTODEVICE)
> -		return sock_bindtodevice(sk, optval, optlen);
> +		return sock_setbindtodevice(sk, optval, optlen);
> 
>  	if (optlen < sizeof(int))
>  		return -EINVAL;
> @@ -1074,9 +1118,10 @@ int sock_getsockopt(struct socket *sock, int level, int
> optname,
>  	case SO_NOFCS:
>  		v.val = sock_flag(sk, SOCK_NOFCS);
>  		break;
> +
>  	case SO_BINDTODEVICE:
> -		v.val = sk->sk_bound_dev_if;
> -		break;
> +		return sock_getbindtodevice(sk, optval, optlen, len);
> +
>  	default:
>  		return -ENOPROTOOPT;
>  	}
> --
> 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: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Eric Dumazet @ 2012-10-22 20:45 UTC (permalink / raw)
  To: Brian Haley; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <50858ABD.2000206@hp.com>

On Mon, 2012-10-22 at 14:04 -0400, Brian Haley wrote:

> +	char devname[IFNAMSIZ];
> +
> +	ret = 0;
> +	if (sk->sk_bound_dev_if == 0)
> +		goto out;
> +
> +	ret = -EINVAL;
> +	if (len < IFNAMSIZ)
> +		goto out;
> +	if (len > IFNAMSIZ)
> +		len = IFNAMSIZ;
> +
> +	rcu_read_lock();
> +	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
> +	if (dev)
> +		strcpy(dev->name, devname);
> +	rcu_read_unlock();
> +	ret = -ENODEV;

You probably meant

	strcpy(devname, dev->name)

By the way, this is not really safe in case device is renamed

^ 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