Netdev List
 help / color / mirror / Atom feed
* Patch-o-match
From: Philip Prindeville @ 2010-08-15 18:14 UTC (permalink / raw)
  To: Netdev

  Hi.

I'd like to build our astlinux images (it's an automated build) with the geoip and TARPIT patches, but can't seem to figure out how.

The links from the Netfilter extensions HowTo seemed to be broken... and since I'm statically building images, I figure that just figuring out the appropriate patches and downloading them by once is probably easier than getting patch-o-matic installed and scripted...  How do I go about doing that?

Oh, maybe also the ROUTE target.  Seemed that it and --tee would be useful for implementing an FBI Jack for lawful intercept in our PBX.

I know how to intercept a particular SIP session, but not the associated SDP (RTP) session that goes with it.  Is there an easy way to do that as well?

Thanks.

-Philip


^ permalink raw reply

* [PATCH] tcp: allow effective reduction of TCP's rcv-buffer via setsockopt
From: Hagen Paul Pfeifer @ 2010-08-15 19:36 UTC (permalink / raw)
  To: netdev
  Cc: Hagen Paul Pfeifer, David S. Miller, Patrick McHardy,
	Eric Dumazet, Ilpo Järvinen

Via setsockopt it is possible to reduce the socket RX buffer
(SO_RCVBUF). TCP method to select the initial window and window scaling
option in tcp_select_initial_window() currently misbehaves and do not
consider a reduced RX socket buffer via setsockopt.

Even though the server's RX buffer is reduced via setsockopt() to 256
byte (Initial Window 384 byte => 256 * 2 - (256 * 2 / 4)) the window
scale option is still 7:

192.168.1.38.40676 > 78.47.222.210.5001: Flags [S], seq 2577214362, win 5840, options [mss 1460,sackOK,TS val 338417 ecr 0,nop,wscale 0], length 0
78.47.222.210.5001 > 192.168.1.38.40676: Flags [S.], seq 1570631029, ack 2577214363, win 384, options [mss 1452,sackOK,TS val 2435248895 ecr 338417,nop,wscale 7], length 0
192.168.1.38.40676 > 78.47.222.210.5001: Flags [.], ack 1, win 5840, options [nop,nop,TS val 338421 ecr 2435248895], length 0

Within tcp_select_initial_window() the original space argument - a
representation of the rx buffer size - is expanded during
tcp_select_initial_window(). Only sysctl_tcp_rmem[2], sysctl_rmem_max
and window_clamp are considered to calculate the initial window.

This patch adjust the window_clamp argument if the user explicitly
reduce the receive buffer.

Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_output.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index de3bd84..c605312 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2429,6 +2429,12 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
 		__u8 rcv_wscale;
 		/* Set this up on the first call only */
 		req->window_clamp = tp->window_clamp ? : dst_metric(dst, RTAX_WINDOW);
+
+		/* limit the window selection if the user enforce a smaller rx buffer */
+		if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+				(req->window_clamp > tcp_full_space(sk) || req->window_clamp == 0))
+			req->window_clamp = tcp_full_space(sk);
+
 		/* tcp_full_space because it is guaranteed to be the first packet */
 		tcp_select_initial_window(tcp_full_space(sk),
 			mss - (ireq->tstamp_ok ? TCPOLEN_TSTAMP_ALIGNED : 0),
@@ -2555,6 +2561,11 @@ static void tcp_connect_init(struct sock *sk)
 
 	tcp_initialize_rcv_mss(sk);
 
+	/* limit the window selection if the user enforce a smaller rx buffer */
+	if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+			(tp->window_clamp > tcp_full_space(sk) || tp->window_clamp == 0))
+		tp->window_clamp = tcp_full_space(sk);
+
 	tcp_select_initial_window(tcp_full_space(sk),
 				  tp->advmss - (tp->rx_opt.ts_recent_stamp ? tp->tcp_header_len - sizeof(struct tcphdr) : 0),
 				  &tp->rcv_wnd,
-- 
1.6.6.196.g1f735.dirty


^ permalink raw reply related

* [PATCH] netfilter: fix userspace header warning
From: Sam Ravnborg @ 2010-08-15 20:03 UTC (permalink / raw)
  To: Patrick McHardy, netdev, netfilter

"make headers_check" issued the following warning:

  CHECK   include/linux/netfilter (64 files)
usr/include/linux/netfilter/xt_ipvs.h:19: found __[us]{8,16,32,64} type without #include <linux/types.h>

Fix this by as suggested including linux/types.h.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
diff --git a/include/linux/netfilter/xt_ipvs.h b/include/linux/netfilter/xt_ipvs.h
index 1167aeb..eff34ac 100644
--- a/include/linux/netfilter/xt_ipvs.h
+++ b/include/linux/netfilter/xt_ipvs.h
@@ -1,6 +1,8 @@
 #ifndef _XT_IPVS_H
 #define _XT_IPVS_H
 
+#include <linux/types.h>
+
 enum {
 	XT_IPVS_IPVS_PROPERTY =	1 << 0, /* all other options imply this one */
 	XT_IPVS_PROTO =		1 << 1,

^ permalink raw reply related

* Re: via-rhine interrupts
From: Jarek Poplawski @ 2010-08-15 20:04 UTC (permalink / raw)
  To: =?UTF-8?B?SmFrdWIgUnXFvmnEjWth?=; +Cc: netdev
In-Reply-To: <AANLkTi=xfBSn=FS9=eR=NZa9QwrHAQwGc7Kh1-bUn3aj@mail.gmail.com>

Jakub Ružička wrote, On 29.07.2010 13:03:

> Hello,
Hi,

> the via-rhine driver powered cards generate a really big number of
> interrupts, almost one per packet (11429 interrupts for 8210 incoming
> and 2475 outgoing packets per second on full 100 Mbps load). This is
> observed on multiple different machines (embbed and desktop) and
> kernels (2.6.25 with and without NAPI, 2.6.30, 2.6.32 and 2.6.33). Do
> you have any idea why isn't the polling used or what can I try to find
> out what's wrong?
> 
> I have tested sending to/from the machines with nc and scp, measured
> interrups and load with atop. Few of these measurements on an embbed
> device (where the interrupt handling is a problem) are attached.

I've just tested it using a simplistic patch below, which skips
some napi receiving by doing it only every second jiffie (on even
ones), and I've got around 30% less interrupts from via-rhine,
which seems to suggest napi works OK, but there is too low
traffic (or too fast soft interrupt handling) to affect hard
interrupts. (Btw, probably CONFIG_HZ can matter here a bit too.
I tested with 1000.)

Cheers,
Jarek P.

--- (patch only for testing)

diff -Nurp a/net/core/dev.c b/net/core/dev.c
--- a/net/core/dev.c	2010-08-15 20:29:58.000000000 +0200
+++ b/net/core/dev.c	2010-08-15 21:15:04.000000000 +0200
@@ -3495,6 +3495,9 @@ static void net_rx_action(struct softirq
 		if (unlikely(budget <= 0 || time_after(jiffies, time_limit)))
 			goto softnet_break;
 
+		if (jiffies & 1)
+			goto softnet_break;
+
 		local_irq_enable();
 
 		/* Even though interrupts have been re-enabled, this


^ permalink raw reply

* Re: PROBLEM: raw sockets rewriting IP ID in rare cases.
From: David Miller @ 2010-08-15 20:13 UTC (permalink / raw)
  To: Morgon.J.Kanter; +Cc: netdev
In-Reply-To: <20100815135741.1iud2jvgw0wwg4kw@webmail.dartmouth.edu>

From: Morgon.J.Kanter@Dartmouth.edu
Date: Sun, 15 Aug 2010 13:57:41 -0400

> Is there a way to turn this behavior off, then, with raw sockets? Zero
> is the desired ID value.

No, there isn't.

^ permalink raw reply

* Re: PROBLEM: raw sockets rewriting IP ID in rare cases.
From: Eric Dumazet @ 2010-08-15 20:47 UTC (permalink / raw)
  To: David Miller; +Cc: Morgon.J.Kanter, netdev
In-Reply-To: <20100815.131359.191392588.davem@davemloft.net>

Le dimanche 15 août 2010 à 13:13 -0700, David Miller a écrit :
> From: Morgon.J.Kanter@Dartmouth.edu
> Date: Sun, 15 Aug 2010 13:57:41 -0400
> 
> > Is there a way to turn this behavior off, then, with raw sockets? Zero
> > is the desired ID value.
> 
> No, there isn't.

Just a note about IP_DF (Dont Fragment) bit.

If set, ID stay 0.

Not sure it can help Morgon.

Jiri Olsa added IP_NODEFRAG option some weeks ago (commit 7b2ff18e),
we probably could implement IP_NOIDENT option for RAW sockets ?




^ permalink raw reply

* Re: Using virtio as a physical (wire-level) transport
From: Rusty Russell @ 2010-08-16  0:19 UTC (permalink / raw)
  To: virtualization
  Cc: Alexander Graf, Ira W. Snyder, netdev@vger.kernel.org, Zang Roy,
	Michael S. Tsirkin
In-Reply-To: <4C336074-FC8C-4BDF-B945-5295133CDB38@suse.de>

On Sat, 14 Aug 2010 09:04:19 pm Alexander Graf wrote:
> 
> Am 06.08.2010 um 11:34 schrieb "Ira W. Snyder" <iws@ovro.caltech.edu>:
> > This problem is not limited to my new use of virtio. Virtio is
> > completely useless in a relatively common virtualization scenario:
> > x86 host with qemu-ppc guest. Or any other big endian guest system.
> 
> This one actually works because we know that we're building for a BE guest.
> But I agree that it's a mess and clearly a very incorrect design decision.

Yes, since you need to know the guest's endian to virtualize it, the
correct interpretation of the virtio ring seemed the least problem.  Perhaps
I went overboard in simplification here, but it seemed pure legacy.

If we did a virtio2, as has been suggested, it would be possible to address
this.  You could of course do a hack where you detect the ring endianness
the first time they use it (based on avail.flags, avail.index and the
descriptor it would be quite reliable in practice).

Cheers,
Rusty.

^ permalink raw reply

* Re: PROBLEM: raw sockets rewriting IP ID in rare cases.
From: Morgon.J.Kanter @ 2010-08-16  0:43 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1281905220.2942.57.camel@edumazet-laptop>

Quoting Eric Dumazet <eric.dumazet@gmail.com>:
> Le dimanche 15 août 2010 à 13:13 -0700, David Miller a écrit :
>> From: Morgon.J.Kanter@Dartmouth.edu
>> Date: Sun, 15 Aug 2010 13:57:41 -0400
>>
>> > Is there a way to turn this behavior off, then, with raw sockets? Zero
>> > is the desired ID value.
>>
>> No, there isn't.
>
> Just a note about IP_DF (Dont Fragment) bit.
>
> If set, ID stay 0.
>
> Not sure it can help Morgon.
>
> Jiri Olsa added IP_NODEFRAG option some weeks ago (commit 7b2ff18e),
> we probably could implement IP_NOIDENT option for RAW sockets ?

Thanks for the information. I was thinking of adding it in as a socket  
option as well, similar to that one you pointed out -- but seeing that  
makes me wonder, perhaps an "as-is" option would be more appropriate,  
instead of a different option for each field you want to leave as-is?

Thanks,
-- Morgon

^ permalink raw reply

* dccp test-tree [Patch 1/1] ccid-3: use per-route min-RTO and/or TCP default
From: Gerrit Renker @ 2010-08-16  5:19 UTC (permalink / raw)
  To: Leandro, Ivo Calado, dccp; +Cc: netdev

This patch relates to the similar one submitted last week for CCID-2.

It replaces the hardcoded (Kconfig) value of the minimum RTO value with
a reconfigurable, per-route value which falls back to the TCP minimum
RTO value.

Leando, Ivo, can you please have a look and indicate whether you are ok
with this? If yes I would like to also update the CCID-4 tree to make it
consistent with the test tree - for this reason the test tree has not yet
been updated.

>>>>>>>>>>>>>>>>>>>>>>> Patch <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
dccp ccid-3: use per-route RTO or TCP RTO as fallback

This makes the TCP (per-route) RTO value also available to CCID-3, hence it
is now possible to influence this value at runtime rather than, as before,
via Kconfig.

The same effect as the default Kconfig option of 100ms is now achieved by

> ip route replace to unicast 192.168.0.0/24 rto_min 100j dev eth0
 
(assuming HZ=1000).

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/ccids/Kconfig |   31 -------------------------------
 net/dccp/ccids/ccid3.c |    9 ++++-----
 2 files changed, 4 insertions(+), 36 deletions(-)

--- a/net/dccp/ccids/Kconfig
+++ b/net/dccp/ccids/Kconfig
@@ -47,37 +47,6 @@ config IP_DCCP_CCID3_DEBUG
 
 	  If in doubt, say N.
 
-config IP_DCCP_CCID3_RTO
-	  int "Use higher bound for nofeedback timer"
-	  default 100
-	  depends on IP_DCCP_CCID3 && EXPERIMENTAL
-	  ---help---
-	    Use higher lower bound for nofeedback timer expiration.
-
-	    The TFRC nofeedback timer normally expires after the maximum of 4
-	    RTTs and twice the current send interval (RFC 3448, 4.3). On LANs
-	    with a small RTT this can mean a high processing load and reduced
-	    performance, since then the nofeedback timer is triggered very
-	    frequently.
-
-	    This option enables to set a higher lower bound for the nofeedback
-	    value. Values in units of milliseconds can be set here.
-
-	    A value of 0 disables this feature by enforcing the value specified
-	    in RFC 3448. The following values have been suggested as bounds for
-	    experimental use:
-		* 16-20ms to match the typical multimedia inter-frame interval
-		* 100ms as a reasonable compromise [default]
-		* 1000ms corresponds to the lower TCP RTO bound (RFC 2988, 2.4)
-
-	    The default of 100ms is a compromise between a large value for
-	    efficient DCCP implementations, and a small value to avoid disrupting
-	    the network in times of congestion.
-
-	    The purpose of the nofeedback timer is to slow DCCP down when there
-	    is serious network congestion: experimenting with larger values should
-	    therefore not be performed on WANs.
-
 config IP_DCCP_TFRC_LIB
 	def_bool y if IP_DCCP_CCID3
 
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -461,12 +461,11 @@ done_computing_x:
 
 	/*
 	 * Update timeout interval for the nofeedback timer.
-	 * We use a configuration option to increase the lower bound.
-	 * This can help avoid triggering the nofeedback timer too
-	 * often ('spinning') on LANs with small RTTs.
+	 * We use the current (per-route) value of the TCP RTO as lower bound
+	 * to slow down rate-halving on networks with small RTTs (loopback,
+	 * running DCCP in a virtual machine, Gbit ethernet, etc).
 	 */
-	hc->tx_t_rto = max_t(u32, 4 * hc->tx_rtt, (CONFIG_IP_DCCP_CCID3_RTO *
-						       (USEC_PER_SEC / 1000)));
+	hc->tx_t_rto = max(4 * hc->tx_rtt, USEC_PER_SEC/HZ * tcp_rto_min(sk));
 	/*
 	 * Schedule no feedback timer to expire in
 	 * max(t_RTO, 2 * s/X)  =  max(t_RTO, 2 * t_ipi)

^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Johannes Berg @ 2010-08-16  5:25 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20100814.223722.116371746.davem@davemloft.net>

On Sat, 2010-08-14 at 22:37 -0700, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Fri, 13 Aug 2010 17:13:53 +0200
> 
> > On Fri, 2010-08-13 at 16:48 +0200, Eric Dumazet wrote:
> > 
> >> I assume reverting 1235f504aaba removes these errors ?
> > 
> > I haven't tried yet, but it only happened very recently and I didn't
> > find any other candidate -- the error always points to
> > wireless_send_event too.
> 
> Please test with the commit reverted and let us know if it helps.
> 
> The current situation is worse than what we were trying to fix
> in that commit, so if a revert fixes your problem then as Eric
> said we should do that first.

I haven't gotten around to it, but Kalle had been running into the same
issue and said reverting it fixed it:

http://article.gmane.org/gmane.linux.kernel.wireless.general/54492

johannes


^ permalink raw reply

* [Power Efficiency Proposal] Low Power on Media Disconnect
From: LionSky @ 2010-08-16  5:35 UTC (permalink / raw)
  To: netdev; +Cc: jie.yang, weiliang lin

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

It is a very typical scenario that users of laptop/Netbook experience
Internet over wireless instead of ethernet.
An obvious hint about that is user unplug-in the ethernet cable which
implies that he does not want to use the ethernet. So for power
efficiency, it is best to place ethernet adapter into a low power
state, such as C3hot, when ethernet cable is disconnected.
Battery life is very important for Laptop/Netbook. Currently, the
power consumption of active idle on a typical Netbook is about 5-6
Watt. If you placed the etherent adapter in C3hot state, about
0.5~1.0Watt power is saved which depneds on your H/W. This can prolong
battery life about 10%~20% which is a quite big power saving.
Now, Linux kernel supports runtime power management. It is not a hard
work to enable this feature on each ethernet adapter driver.

Here I attached a simple and prototype implementation which is
verified on a Netbook (ASUS E1005PE with Atheros AR8132 PCI-E Fast
Ethernet Controller).
1. When ethernet cable is disconnected, driver places ethernet adapter
into D3hot low power state. Just as CPU go to deep C-State.
2. When ethernet cable is connected again, driver places ethernet back
in D0 power state.

To further improve the power behavior of Linux on laptop/Netbook, a
proposal is that all ethernet drivers must support Low Power on Media
Disconnect.

Is it right? Welcome any comments.


Thanks
-Lionsky

[-- Attachment #2: patch.atl1c.rt.txt --]
[-- Type: text/plain, Size: 3854 bytes --]

--- atl1cold/atl1c_main.c	2010-08-04 16:21:59.863328895 +0800
+++ atl1cnew/atl1c_main.c	2010-08-16 11:06:42.217285358 +0800
@@ -20,6 +20,7 @@
  */
 
 #include "atl1c.h"
+#include <linux/pm_runtime.h>
 
 #define ATL1C_DRV_VERSION "1.0.0.2-NAPI"
 char atl1c_driver_name[] = "atl1c";
@@ -66,6 +67,8 @@
 static void atl1c_setup_mac_ctrl(struct atl1c_adapter *adapter);
 static void atl1c_clean_rx_irq(struct atl1c_adapter *adapter, u8 que,
 		   int *work_done, int work_to_do);
+static int atl1c_runtime_suspend(struct device *device);
+static int atl1c_runtime_resume(struct device *device);
 
 static const u16 atl1c_pay_load_size[] = {
 	128, 256, 512, 1024, 2048, 4096,
@@ -98,6 +101,9 @@
 static const u32 atl1c_default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
 	NETIF_MSG_LINK | NETIF_MSG_TIMER | NETIF_MSG_IFDOWN | NETIF_MSG_IFUP;
 
+/*a variable indicates current H/W power state*/
+static u16 atl1c_d3  = 1;
+
 /*
  * atl1c_init_pcie - init PCIE module
  */
@@ -240,8 +246,13 @@
 			atl1c_set_aspm(hw, false);
 		}
 		netif_carrier_off(netdev);
+
+		pm_runtime_put_noidle(&pdev->dev);
+		pm_schedule_suspend(&pdev->dev,0);
 	} else {
 		/* Link Up */
+		pm_request_resume(&pdev->dev);
+
 		hw->hibernate = false;
 		spin_lock_irqsave(&adapter->mdio_lock, flags);
 		err = atl1c_get_speed_and_duplex(hw, &speed, &duplex);
@@ -1576,6 +1587,21 @@
 }
 
 /*
+ * checkPoweState - If current power state is D3hot, resume it to D0 
+ * @pdev: pointer to a pci network interface device structure
+ */
+static inline int checkPowerState(struct pci_dev * pdev)
+{
+	if (unlikely(atl1c_d3)) {
+		pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, 0);
+		pdev->dev.power.runtime_status = 0;
+		atl1c_d3 = 0;
+		return 1;
+	}
+	return 0;
+}
+
+/*
  * atl1c_intr - Interrupt Handler
  * @irq: interrupt number
  * @data: pointer to a network interface device structure
@@ -1592,6 +1618,8 @@
 	u32 status;
 	u32 reg_data;
 
+	checkPowerState(pdev);
+
 	do {
 		AT_READ_REG(hw, REG_ISR, &reg_data);
 		status = reg_data & hw->intr_mask;
@@ -2703,6 +2731,12 @@
 	if (netif_msg_probe(adapter))
 		dev_info(&pdev->dev, "version %s\n", ATL1C_DRV_VERSION);
 	cards_found++;
+
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+	
+	pm_runtime_idle(&pdev->dev);
+
 	return 0;
 
 err_reset:
@@ -2832,6 +2866,57 @@
 	.resume = atl1c_io_resume,
 };
 
+/* Adding runtime power management support*/
+#ifdef CONFIG_PM
+
+//callback function definition
+static int atl1c_runtime_suspend(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+
+	pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, 3);
+	atl1c_d3 = 1;
+	dev_err(&pdev->dev, "[debug] runtime_suspend\n");
+
+	return 0; 
+}
+
+static int atl1c_runtime_resume(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+
+	pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, 0);
+	atl1c_d3 = 0;
+	dev_err(&pdev->dev, "[debug] runtime_resume\n");
+
+	return 0; 
+}
+
+static int atl1c_runtime_idle(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct atl1c_adapter *adapter = netdev_priv(dev);	
+
+	atl1c_check_link_status(adapter);
+
+	return 0; 
+}
+
+static const struct dev_pm_ops atl1c_pm_ops = {
+	.runtime_suspend = atl1c_runtime_suspend,
+	.runtime_resume = atl1c_runtime_resume,
+	.runtime_idle = atl1c_runtime_idle,
+};
+
+#define ATL1C_PM_OPS (&atl1c_pm_ops)
+
+#else /* !CONFIG_PM */
+
+#define ATL1C_PM_OPS	NULL
+
+#endif /* !CONFIG_PM */
+
 static struct pci_driver atl1c_driver = {
 	.name     = atl1c_driver_name,
 	.id_table = atl1c_pci_tbl,
@@ -2841,7 +2926,8 @@
 	.suspend  = atl1c_suspend,
 	.resume   = atl1c_resume,
 	.shutdown = atl1c_shutdown,
-	.err_handler = &atl1c_err_handler
+	.err_handler = &atl1c_err_handler,
+	.driver.pm = ATL1C_PM_OPS,
 };
 
 /*

^ permalink raw reply

* Re: [Power Efficiency Proposal] Low Power on Media Disconnect
From: Eric Dumazet @ 2010-08-16  6:08 UTC (permalink / raw)
  To: LionSky; +Cc: netdev, jie.yang
In-Reply-To: <AANLkTin3DNEPDA+vpY-WuSa8ia-sgEA8_sr9qOLpDHph@mail.gmail.com>

Le lundi 16 août 2010 à 13:35 +0800, LionSky a écrit :
> It is a very typical scenario that users of laptop/Netbook experience
> Internet over wireless instead of ethernet.
> An obvious hint about that is user unplug-in the ethernet cable which
> implies that he does not want to use the ethernet. So for power
> efficiency, it is best to place ethernet adapter into a low power
> state, such as C3hot, when ethernet cable is disconnected.
> Battery life is very important for Laptop/Netbook. Currently, the
> power consumption of active idle on a typical Netbook is about 5-6
> Watt. If you placed the etherent adapter in C3hot state, about
> 0.5~1.0Watt power is saved which depneds on your H/W. This can prolong
> battery life about 10%~20% which is a quite big power saving.
> Now, Linux kernel supports runtime power management. It is not a hard
> work to enable this feature on each ethernet adapter driver.
> 
> Here I attached a simple and prototype implementation which is
> verified on a Netbook (ASUS E1005PE with Atheros AR8132 PCI-E Fast
> Ethernet Controller).
> 1. When ethernet cable is disconnected, driver places ethernet adapter
> into D3hot low power state. Just as CPU go to deep C-State.
> 2. When ethernet cable is connected again, driver places ethernet back
> in D0 power state.
> 
> To further improve the power behavior of Linux on laptop/Netbook, a
> proposal is that all ethernet drivers must support Low Power on Media

 ... should support ...

> Disconnect.
> 
> Is it right? Welcome any comments.
> 

I cannot say if this patch is right on the power side.
(Would it be better to handle the ethernet device power switch from a
user policy daemon, instead of forcing the policy from the driver ?)
I dont know the implications of Low Power on Media disconnect.
At least, let ethtool be able to switch on or off this automatic
behavior...

But your patch introduces a shared (static actually) variable, atl1c_d3.
Are you sure a single state is enough to handle several ethernet ports ?




^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Eric Dumazet @ 2010-08-16  6:10 UTC (permalink / raw)
  To: Johannes Berg; +Cc: David Miller, netdev
In-Reply-To: <1281936351.3683.1.camel@jlt3.sipsolutions.net>

Le lundi 16 août 2010 à 07:25 +0200, Johannes Berg a écrit :

> I haven't gotten around to it, but Kalle had been running into the same
> issue and said reverting it fixed it:
> 
> http://article.gmane.org/gmane.linux.kernel.wireless.general/54492
> 

Thanks Johannes

Please David revert 1235f504.

We'll find another way to address the problem.




^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: David Miller @ 2010-08-16  6:21 UTC (permalink / raw)
  To: eric.dumazet; +Cc: johannes, netdev
In-Reply-To: <1281939002.2942.70.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 16 Aug 2010 08:10:02 +0200

> Le lundi 16 août 2010 à 07:25 +0200, Johannes Berg a écrit :
> 
>> I haven't gotten around to it, but Kalle had been running into the same
>> issue and said reverting it fixed it:
>> 
>> http://article.gmane.org/gmane.linux.kernel.wireless.general/54492
>> 
> 
> Thanks Johannes
> 
> Please David revert 1235f504.
> 
> We'll find another way to address the problem.

I will, thanks guys.

^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Johannes Berg @ 2010-08-16  6:22 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1281939002.2942.70.camel@edumazet-laptop>

On Mon, 2010-08-16 at 08:10 +0200, Eric Dumazet wrote:

> We'll find another way to address the problem.

For my understanding: The problem is that the save/restore of the
frag_list skb can race so a second reader will not see the frag_list skb
because it gets there while it's NULL. This happens with MSG_PEEK, or
with clones of the SKB since skb_shared_info is invariant across clones.

Correct?

johannes


^ permalink raw reply

* Re: PROBLEM: raw sockets rewriting IP ID in rare cases.
From: Eric Dumazet @ 2010-08-16  6:26 UTC (permalink / raw)
  To: David Miller, Michael Kerrisk
  Cc: Morgon.J.Kanter, netdev, Jiri Olsa, Stephen Hemminger
In-Reply-To: <1281905220.2942.57.camel@edumazet-laptop>

Le dimanche 15 août 2010 à 22:47 +0200, Eric Dumazet a écrit :

> Jiri Olsa added IP_NODEFRAG option some weeks ago (commit 7b2ff18e),
> we probably could implement IP_NOIDENT option for RAW sockets ?
> 
> 

Hmm, it seems we forgot to send to Michael some updates for
documentation.

http://www.kernel.org/doc/man-pages/online/pages/man7/ip.7.html

misses IP_FREEBIND, IP_IPSEC_POLICY, IP_XFRM_POLICY, IP_PASSSEC
IP_TRANSPARENT, IP_ORIGDSTADDR, IP_MINTTL and IP_NODEFRAG bits




^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Eric Dumazet @ 2010-08-16  6:29 UTC (permalink / raw)
  To: Johannes Berg; +Cc: David Miller, netdev
In-Reply-To: <1281939753.3683.5.camel@jlt3.sipsolutions.net>

Le lundi 16 août 2010 à 08:22 +0200, Johannes Berg a écrit :
> On Mon, 2010-08-16 at 08:10 +0200, Eric Dumazet wrote:
> 
> > We'll find another way to address the problem.
> 
> For my understanding: The problem is that the save/restore of the
> frag_list skb can race so a second reader will not see the frag_list skb
> because it gets there while it's NULL. This happens with MSG_PEEK, or
> with clones of the SKB since skb_shared_info is invariant across clones.
> 
> Correct?
> 

Yes

I believe we should have a mutual exclusion for the critical section.

(setting pointer to NULL, ...., setting pointer back to its orig value)




^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Johannes Berg @ 2010-08-16  6:31 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1281940172.2942.81.camel@edumazet-laptop>

On Mon, 2010-08-16 at 08:29 +0200, Eric Dumazet wrote:
> Le lundi 16 août 2010 à 08:22 +0200, Johannes Berg a écrit :
> > On Mon, 2010-08-16 at 08:10 +0200, Eric Dumazet wrote:
> > 
> > > We'll find another way to address the problem.
> > 
> > For my understanding: The problem is that the save/restore of the
> > frag_list skb can race so a second reader will not see the frag_list skb
> > because it gets there while it's NULL. This happens with MSG_PEEK, or
> > with clones of the SKB since skb_shared_info is invariant across clones.
> > 
> > Correct?
> > 
> 
> Yes
> 
> I believe we should have a mutual exclusion for the critical section.
> 
> (setting pointer to NULL, ...., setting pointer back to its orig value)

Yeah, that'd work, but I'm wondering now why I did this at all. The code
already restricts the data copying to skb->len, which will not include
the frag_list, so it should be OK to just leave it intact? I'll go and
look at that in more detail and try it.

johannes


^ permalink raw reply

* Re: [PATCH 2/2] platform: Facilitate the creation of pseudo-platform buses
From: Grant Likely @ 2010-08-16  6:43 UTC (permalink / raw)
  To: Moffett, Kyle D, Patrick Pannuto
  Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	magnus.damm@gmail.com, gregkh@suse.de, Kevin Hilman, Paul Mundt,
	Magnus Damm, Rafael J. Wysocki, Eric Miao, Dmitry Torokhov,
	netdev@vger.kernel.org, Kyle D Moffett
In-Reply-To: <3F978429-F916-42E5-8B36-6AC02DAC8CA2@boeing.com>

"Moffett, Kyle D" <Kyle.D.Moffett@boeing.com> wrote:

>On Aug 10, 2010, at 19:49, Patrick Pannuto wrote:
>> As SOCs become more popular, the desire to quickly define a simple,
>> but functional, bus type with only a few unique properties becomes
>> desirable. As they become more complicated, the ability to nest these
>> simple busses and otherwise orchestrate them to match the actual
>> topology also becomes desirable.
>>
>> EXAMPLE USAGE
>>
>> /arch/ARCH/MY_ARCH/my_bus.c:
>>
>> 	#include <linux/device.h>
>> 	#include <linux/platform_device.h>
>>
>> 	struct bus_type SOC_bus_type = {
>> 		.name = "SOC-bus-type",
>> 	};
>> 	EXPORT_SYMBOL_GPL(SOC_bus_type);
>>
>> 	struct platform_device SOC_bus1 = {
>> 		.name	 = "SOC-bus1",
>> 		.id		= -1,
>> 		.dev.bus = &SOC_bus_type;
>> 	};
>> 	EXPORT_SYMBOL_GPL(SOC_bus1);
>>
>> 	struct platform_device SOC_bus2 = {
>> 		.name	 = "SOC-bus2",
>> 		.id		= -2,
>> 		.dev.bus = &SOC_bus_type;
>> 	};
>> 	EXPORT_SYMBOL_GPL(SOC_bus2);
>>
>> 	static int __init SOC_bus_init(void)
>> 	{
>> 		int error;
>>
>> 		error = pseudo_platform_bus_register(&SOC_bus_type);
>> 		if (error)
>> 			return error;
>>
>> 		error = platform_device_register(&SOC_bus1);
>> 		if (error)
>> 			goto fail_bus1;
>>
>> 		error = platform_device_register(&SOC_bus2);
>> 		if (error)
>> 			goto fail_bus2;
>>
>> 		return error;
>>
>> 		/* platform_device_unregister(&SOC_bus2); */
>> fail_bus2:
>> 		platform_device_unregister(&SOC_bus1);
>> fail_bus1:
>> 		pseudo_platform_bus_unregister(&SOC_bus_type);
>>
>> 		return error;
>> 	}
>>
>> /drivers/my_driver.c:
>> 	static struct platform_driver my_driver = {
>> 		.driver = {
>> 			.name	= "my-driver",
>> 			.owner	= THIS_MODULE,
>> 			.bus	= &SOC_bus_type,
>> 		},
>> 	};
>>
>> /somewhere/my_device.c:
>> 	static struct platform_device my_device = {
>> 		.name		= "my-device",
>> 		.id		= -1,
>> 		.dev.bus	= &my_bus_type,
>> 		.dev.parent	= &SOC_bus1.dev,
>> 	};
>>
>> This will build a device tree that mirrors the actual system:
>>
>> /sys/bus
>> |-- SOC-bus-type
>> |   |-- devices
>> |   |   |-- SOC_bus1 -> ../../../devices/SOC_bus1
>> |   |   |-- SOC_bus2 -> ../../../devices/SOC_bus2
>> |   |   |-- my-device -> ../../../devices/SOC_bus1/my-device
>> |   |-- drivers
>> |   |   |-- my-driver
>>
>> /sys/devices
>> |-- SOC_bus1
>> |   |-- my-device
>> |-- SOC_bus2
>>
>> Driver can drive any device on the SOC, which is logical, without
>> actually being registered on multiple /bus_types/, even though the
>> devices may be on different /physical buses/ (which are actually
>> just devices).
>
>Hmm...
>
>To me this seems like a really painful implementation of what the OpenFirmware-esque "Flattened Device Tree" does on many embedded systems.

Patrick and Kevin have been trying to solve a different problem.  The
FDT is really good at describing the topology and interconnections of
the system, but it doesn't solve the problem of how the different bus
behaviour is implemented in the kernel.  They need a method of
registering devices that have subtly different behaviour from the
plain-vanilla platform bus.  Whether or not the device data originates
from the FDT is irrelevant, and the problem remains the same.

I'm not convinced (yet) that this is the right approach, and I'd like
to see a few sample drivers converted to the new approach.  Creating
new bus_types that "inherit" from the platform_bus is actually not a
bad idea, and it is an elegant way to change the behaviour (for
example, how power management is implemented) for devices connected in
a different way.

A problem with the approach that Kevin pointed out is that drivers
that need to work on both the platform_bus_type and the new
soc_bus_type must explicitly register themselves on both bus types.
There is no mechanism to allow drivers from one bus type to also be
made available to another bus type. Certainly it would be possible to
invent a mechanism, but the more I think about it, them more I think
it will be a bad idea.  The runtime-PM use-case that kicked this
discussion off makes the assumption that a driver will behave
identically when attached to either the platform_bus_type or the
soc_bus_type.  However, I think that in the general case that
assumption will prove to be false.  I strongly suspect that the new
bus type will turn out to be not as similar to the platform_bus_type
as originally assumed and that there will still be bus-type-specific
impact on device drivers (but I digress; this paragraph is more
directed to Patrick and Kevin, and doesn't address your comments).

More below...

>For example, to build an equivalent device tree using an OpenFirmware FDT file, I'd just use this:
>
>/dts-v1/;
>/ {
>	#address-cells = <1>;
>	#size-cells = <1>;
>	[...snip...]
>
>	soc-bus-1@fc000000 {
>		/* of_platform driver matches against this: */
>		compatible = "my-company-name,soc-bus-type";
>
>		/* Define base address and size of the bus */
>		reg = <0xfc000000 0x01000000>;
>		#address-cells = <1>;
>		#size-cells = <1>;
>
>		/*
>		 * Define logical memory mapping relative to the bus addr:
>		 * First field is the relative base address for children,
>		 * second field is the address in the parent's memory map,
>		 * third field is the size of the range.
>		 */
>		ranges = <0x0 0xfc000000 0x01000000>;
>
>		/* Now for sub-devices */
>		my-device@0x10000 {
>			compatible = "my-company-name,my-driver";
>			reg = <0x10000 0x100>; /* Address and size */
>		};
>	};
>
>	soc-bus-2@fd000000 {
>		/* of_platform driver matches against this: */
>		compatible = "my-company-name,soc-bus-type";
>
>		/* Define base address and size of the bus */
>		reg = <0xfd000000 0x01000000>;
>		#address-cells = <1>;
>		#size-cells = <1>;
>
>		/*
>		 * Define logical memory mapping relative to the bus addr:
>		 * First field is the relative base address for children,
>		 * second field is the address in the parent's memory map,
>		 * third field is the size of the range.
>		 */
>		ranges = <0x0 0xfd000000 0x01000000>;
>	};
>};
>
>If you don't need to actually do anything special at the bus level, you can just:
>	static struct of_device_id soc_bus_ids[] = {
>		{ .compatible = "soc-bus-type", },
>		{},
>	};
>	of_platform_bus_probe(NULL, &soc_bus_ids, NULL);
>
>Any of_platform driver that matches something on one of those busses is automatically probed.  Alternatively, if you need special bus behavior:
>
>	static struct of_device_id soc_bus_ids[] = {
>		{ .compatible = "soc-bus-type", },
>		{},
>	};
>	static struct of_platform_driver soc_bus_type = {
>		.name = "soc-bus-type",
>		.match_table = &soc_bus_ids,
>		.owner = THIS_MODULE,
>		.probe = mybus_probe,
>		.remove = mybus_remove,
>		.suspend = mybus_suspend,
>		.resume = mybus_resume,
>		.shutdown = mybus_shutdown,
>	};
>
>Then your .probe function actually registers a new OF bus.

Be careful!  This is actually a confusing example.  Linux already has
a concept of a bus_type, and it is not the same as what is shown in
this example.  Part of the problem is that Patrick's explanation leads
the reader to conflate two separate concepts; device topology and
bus_types (but I believe that Patrick understands the difference
between the two).  Topology is the tree of struct devices in the LInux
device model (specified by the .parent pointer).  It is represented by
the /sys/devices/* directory tree.

bus_type groups devices that use the same bus infrastructure, but it
says nothing about topology.  bus_types are represented by the
/sys/bus directory.  For example, a device on the i2c_bus_type must be
an i2c_device, is accessed with the i2c infrastructure, and can be
bound to an i2c_driver.  There can be multiple physical i2c busses in
a system, but all i2c_devices are grouped together in
/sys/bus/i2c/devices (as symlinks to the real location in
/sys/devices).

Using the name "soc-bus-type" in this example makes it easy to confuse
this of_platform_driver with an actual bus_type.

BTW, you'll probably be interested to know that as of about a week
ago, Linus pulled my tree which replaces of_platform_bus_type with the
regular platform_bus_type.  There no longer is any differentiation
between OF and non-OF devices.  Any device may have a pointer to an OF
device tree node regardless of bus_type.  Existing of_platform_drivers
do still work, but a shim is used to register them onto the platform
driver.  Next step is to convert existing of_platform_drivers (like
your example above) into normal platform_drivers.

>The best part is... all devices registered as "of_platform" devices can be used to support many entirely different board models from the exact same kernel.
>
>Fully commented and with actual physical addresses in, my FDT example is comparable to your sample code.  Furthermore, all of the error handling is automatically done, a bunch of device drivers are already ported over, and all the kinks regarding interrupts, etc are already taken care of.  I highly recommend taking a look to see if you can use the very nice existing OF bus code to solve your problem instead of writing yet another half-hard-coded platform bus type.
>
>Cheers,
>Kyle Moffett
>

^ permalink raw reply

* Re: PROBLEM: raw sockets rewriting IP ID in rare cases.
From: Jiri Olsa @ 2010-08-16  6:50 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Michael Kerrisk, Morgon.J.Kanter, netdev,
	Stephen Hemminger
In-Reply-To: <1281939985.2942.79.camel@edumazet-laptop>

On Mon, Aug 16, 2010 at 08:26:25AM +0200, Eric Dumazet wrote:
> Le dimanche 15 août 2010 à 22:47 +0200, Eric Dumazet a écrit :
> 
> > Jiri Olsa added IP_NODEFRAG option some weeks ago (commit 7b2ff18e),
> > we probably could implement IP_NOIDENT option for RAW sockets ?
> > 
> > 
> 
> Hmm, it seems we forgot to send to Michael some updates for
> documentation.
> 
> http://www.kernel.org/doc/man-pages/online/pages/man7/ip.7.html
> 
> misses IP_FREEBIND, IP_IPSEC_POLICY, IP_XFRM_POLICY, IP_PASSSEC
> IP_TRANSPARENT, IP_ORIGDSTADDR, IP_MINTTL and IP_NODEFRAG bits
> 
> 
> 

hi,
I've sent out the IP_NODEFRAG man bits together with the change
http://kerneltrap.org/mailarchive/linux-netdev/2010/6/26/6280033

jirka

^ permalink raw reply

* Re: [Power Efficiency Proposal] Low Power on Media Disconnect
From: LionSky @ 2010-08-16  7:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, jie.yang
In-Reply-To: <1281938880.2942.68.camel@edumazet-laptop>

Thanks Eric,
Yes, you are right. It is better to handle the ethernet device power
switch from a user policy daemon.
This is just a proposal about Low Power on Media Disconnect which
means when media, such as ethernet cable, is disconnected, the related
device should be placed in a low power state, such as D3hot.
Maybe we needs a smart implementation.

Thanks
-Lionsky

2010/8/16 Eric Dumazet <eric.dumazet@gmail.com>:
> Le lundi 16 août 2010 à 13:35 +0800, LionSky a écrit :
>> It is a very typical scenario that users of laptop/Netbook experience
>> Internet over wireless instead of ethernet.
>> An obvious hint about that is user unplug-in the ethernet cable which
>> implies that he does not want to use the ethernet. So for power
>> efficiency, it is best to place ethernet adapter into a low power
>> state, such as C3hot, when ethernet cable is disconnected.
>> Battery life is very important for Laptop/Netbook. Currently, the
>> power consumption of active idle on a typical Netbook is about 5-6
>> Watt. If you placed the etherent adapter in C3hot state, about
>> 0.5~1.0Watt power is saved which depneds on your H/W. This can prolong
>> battery life about 10%~20% which is a quite big power saving.
>> Now, Linux kernel supports runtime power management. It is not a hard
>> work to enable this feature on each ethernet adapter driver.
>>
>> Here I attached a simple and prototype implementation which is
>> verified on a Netbook (ASUS E1005PE with Atheros AR8132 PCI-E Fast
>> Ethernet Controller).
>> 1. When ethernet cable is disconnected, driver places ethernet adapter
>> into D3hot low power state. Just as CPU go to deep C-State.
>> 2. When ethernet cable is connected again, driver places ethernet back
>> in D0 power state.
>>
>> To further improve the power behavior of Linux on laptop/Netbook, a
>> proposal is that all ethernet drivers must support Low Power on Media
>
>  ... should support ...
>
>> Disconnect.
>>
>> Is it right? Welcome any comments.
>>
>
> I cannot say if this patch is right on the power side.
> (Would it be better to handle the ethernet device power switch from a
> user policy daemon, instead of forcing the policy from the driver ?)
> I dont know the implications of Low Power on Media disconnect.
> At least, let ethtool be able to switch on or off this automatic
> behavior...
>
> But your patch introduces a shared (static actually) variable, atl1c_d3.
> Are you sure a single state is enough to handle several ethernet ports ?
>
>
>
>

^ permalink raw reply

* [PATCH] netlink: fix compat recvmsg
From: Johannes Berg @ 2010-08-16  7:20 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Kalle Valo
In-Reply-To: <1281940172.2942.81.camel@edumazet-laptop>

From: Johannes Berg <johannes.berg@intel.com>

Since
commit 1dacc76d0014a034b8aca14237c127d7c19d7726
Author: Johannes Berg <johannes@sipsolutions.net>
Date:   Wed Jul 1 11:26:02 2009 +0000

    net/compat/wext: send different messages to compat tasks

we had a race condition when setting and then
restoring frag_list. Eric attempted to fix it,
but the fix created even worse problems.

However, the original motivation I had when I
added the code that turned out to be racy is
no longer clear to me, since we only copy up
to skb->len to userspace, which doesn't include
the frag_list length. As a result, not doing
any frag_list clearing and restoring avoids
the race condition, while not introducing any
other problems.

Additionally, while preparing this patch I found
that since none of the remaining netlink code is
really aware of the frag_list, we need to use the
original skb's information for packet information
and credentials. This fixes, for example, the
group information received by compat tasks.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: stable@kernel.org [2.6.31+, for 2.6.35 revert 1235f504aa]
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/netlink/af_netlink.c |   46 ++++++++++++++++------------------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

--- wireless-testing.orig/net/netlink/af_netlink.c	2010-08-16 08:33:42.000000000 +0200
+++ wireless-testing/net/netlink/af_netlink.c	2010-08-16 09:13:17.000000000 +0200
@@ -1406,7 +1406,7 @@ static int netlink_recvmsg(struct kiocb
 	struct netlink_sock *nlk = nlk_sk(sk);
 	int noblock = flags&MSG_DONTWAIT;
 	size_t copied;
-	struct sk_buff *skb, *frag __maybe_unused = NULL;
+	struct sk_buff *skb, *data_skb;
 	int err;
 
 	if (flags&MSG_OOB)
@@ -1418,45 +1418,35 @@ static int netlink_recvmsg(struct kiocb
 	if (skb == NULL)
 		goto out;
 
+	data_skb = skb;
+
 #ifdef CONFIG_COMPAT_NETLINK_MESSAGES
 	if (unlikely(skb_shinfo(skb)->frag_list)) {
-		bool need_compat = !!(flags & MSG_CMSG_COMPAT);
-
 		/*
-		 * If this skb has a frag_list, then here that means that
-		 * we will have to use the frag_list skb for compat tasks
-		 * and the regular skb for non-compat tasks.
+		 * If this skb has a frag_list, then here that means that we
+		 * will have to use the frag_list skb's data for compat tasks
+		 * and the regular skb's data for normal (non-compat) tasks.
 		 *
-		 * The skb might (and likely will) be cloned, so we can't
-		 * just reset frag_list and go on with things -- we need to
-		 * keep that. For the compat case that's easy -- simply get
-		 * a reference to the compat skb and free the regular one
-		 * including the frag. For the non-compat case, we need to
-		 * avoid sending the frag to the user -- so assign NULL but
-		 * restore it below before freeing the skb.
+		 * If we need to send the compat skb, assign it to the
+		 * 'data_skb' variable so that it will be used below for data
+		 * copying. We keep 'skb' for everything else, including
+		 * freeing both later.
 		 */
-		if (need_compat) {
-			struct sk_buff *compskb = skb_shinfo(skb)->frag_list;
-			skb_get(compskb);
-			kfree_skb(skb);
-			skb = compskb;
-		} else {
-			frag = skb_shinfo(skb)->frag_list;
-			skb_shinfo(skb)->frag_list = NULL;
-		}
+		if (flags & MSG_CMSG_COMPAT)
+			data_skb = skb_shinfo(skb)->frag_list;
 	}
 #endif
 
 	msg->msg_namelen = 0;
 
-	copied = skb->len;
+	copied = data_skb->len;
 	if (len < copied) {
 		msg->msg_flags |= MSG_TRUNC;
 		copied = len;
 	}
 
-	skb_reset_transport_header(skb);
-	err = skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
+	skb_reset_transport_header(data_skb);
+	err = skb_copy_datagram_iovec(data_skb, 0, msg->msg_iov, copied);
 
 	if (msg->msg_name) {
 		struct sockaddr_nl *addr = (struct sockaddr_nl *)msg->msg_name;
@@ -1476,11 +1466,7 @@ static int netlink_recvmsg(struct kiocb
 	}
 	siocb->scm->creds = *NETLINK_CREDS(skb);
 	if (flags & MSG_TRUNC)
-		copied = skb->len;
-
-#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
-	skb_shinfo(skb)->frag_list = frag;
-#endif
+		copied = data_skb->len;
 
 	skb_free_datagram(sk, skb);
 



^ permalink raw reply

* Re: tproxy related crash in inet_hashtables
From: Stephen Buck @ 2010-08-16  8:26 UTC (permalink / raw)
  To: David Miller
  Cc: eric.dumazet@gmail.com, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org, kaber@trash.net,
	hidden@sch.bme.hu
In-Reply-To: <20100814.221611.15248639.davem@davemloft.net>

On 15/08/10 15:16, David Miller wrote:
> From: Stephen Buck<stephen.buck@exinda.com>
> Date: Sat, 14 Aug 2010 14:35:21 +1000
>
>    
>> +		}
>> +		if (!node)
>> +			tb = inet_bind_bucket_create(table->bind_bucket_cachep,
>> +						     sock_net(sk), head, port);
>> +	}
>>   	sk_add_bind_node(child,&tb->owners);
>>      
> inet_bind_bucket_create() can fail due to memory allocation failure,
> you're therefore going to have to find a way to handle that.
>
> And in doing so you're going to find out that there is no easy
> way out of this code path in such an error, this code was
> absolutely not designed to be able to handle cases like this.
>    
You're right. I didn't cover the failure case. And no, it doesn't look easy.
I'll have another look at the problem when I get a chance.

Thanks for pointing it out.

^ permalink raw reply

* Re: GTSM and TCP accept problem
From: Eric Dumazet @ 2010-08-16  9:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Herbert Xu, netdev
In-Reply-To: <20100814214634.24a87715@s6510>

Le samedi 14 août 2010 à 21:46 -0400, Stephen Hemminger a écrit :
> The implementation of Generalized TTL Security has a problem
> on the TCP accept side. Since there can be different number of
> hops for each peer, the listener ends up doing
> 
>    socket
>    bind
>    set TTL to 255
>    listen
>    while (nfd = accept(peer)) {
>      info = lookup(peer)
>      set MINTTL to 255 - info->ttl_hops
> 
> 
> The problem is that a rogue peer can still do three way
> handshake causing the accept to succeed. But the rogue will
> leave a stuck connection that will then have to timeout.
> 
> The only ways I have come up to deal with this are:
>    * have short timeout on initial data (recycle faster)
>    * push peer;MINTTL table down into kernel (per socket)
>    * have BGP do this through iptables which is a non-starter
>      for a general application that needs to run on BSD, and
>      would mess up existing firewall rules.
> 
> Ideas welcome..
> --

Another idea would be to store the TTL of the SYN packet (or third
packet) and let application read it after accept(), allowing it to
reject the connection if it doesnt match expected TTL.




^ permalink raw reply

* Re: [GIT] Networking
From: Jarek Poplawski @ 2010-08-16  9:53 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, torvalds, akpm, netdev, linux-kernel,
	Patrick McHardy
In-Reply-To: <1281883637.2942.42.camel@edumazet-laptop>

Eric Dumazet wrote:
> Le dimanche 15 aou^t 2010 a` 12:55 +0200, Eric Dumazet a écrit :
...
> [PATCH] netfilter: {ip,ip6,arp}_tables: avoid lockdep false positive
> 
> After commit 24b36f019 (netfilter: {ip,ip6,arp}_tables: dont block
> bottom half more than necessary), lockdep can raise a warning
> because we attempt to lock a spinlock with BH enabled, while
> the same lock is usually locked by another cpu in a softirq context.

Btw, could you remind us how get_counters() are serialized (I guess
you can't have them on 2 cpus at the same time)?

Thanks,
Jarek P.

^ 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