Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: David Miller @ 2009-10-30 19:41 UTC (permalink / raw)
  To: ebiederm; +Cc: netdev, fubar
In-Reply-To: <m1tyxhwxah.fsf@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 29 Oct 2009 17:16:54 -0700

> I recently had it pointed out to me that the bonding driver does not
> work in a network namespace.  So I have simplified the bonding driver
> a bit, added support for ip link add and ip link del, and finally made
> the bonding driver work in multiple network namespaces.
> 
> The most note worthy change in the patchset is the addition of support
> in the networking core for registering a sysfs group for a device.
> 
> Using this in the bonding driver simplifies the code and removes a
> userspace race between actions triggered by the netlink event and the
> bonding sysfs attributes appearing.

I've tossed patches 1-7 into net-next-2.6, thanks Eric.

^ permalink raw reply

* Re: [PATCH] sky2: set carrier off in probe
From: David Miller @ 2009-10-30 19:28 UTC (permalink / raw)
  To: bphilips; +Cc: shemminger, netdev
In-Reply-To: <20091029235807.GF3228@jenkins.home.ifup.org>

From: Brandon Philips <bphilips@suse.de>
Date: Thu, 29 Oct 2009 16:58:07 -0700

> Before bringing up a sky2 interface up ethtool reports 
> "Link detected: yes". Do as ixgbe does and netif_carrier_off() on
> probe().
> 
> Signed-off-by: Brandon Philips <bphilips@suse.de>

Applied to net-2.6, thanks.

^ permalink raw reply

* Re: How to use gretap with bridge?
From: David Miller @ 2009-10-30 19:27 UTC (permalink / raw)
  To: shemminger; +Cc: herbert, nneul, netdev
In-Reply-To: <20091030093957.7128c596@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 30 Oct 2009 09:39:57 -0700

> On Fri, 30 Oct 2009 11:51:48 -0400
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
>> gre: Fix dev_addr clobbering for gretap
>> 
>> Nathan Neulinger noticed that gretap devices get their MAC address
>> from the local IP address, which results in invalid MAC addresses
>> half of the time.
>> 
>> This is because gretap is still using the tunnel netdev ops rather
>> than the correct tap netdev ops struct.
>> 
>> This patch also fixes changelink to not clobber the MAC address
>> for the gretap case.
>> 
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>> 
>> Thanks,
> 
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Applied to net-2.6, thanks!

^ permalink raw reply

* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: David Miller @ 2009-10-30 19:26 UTC (permalink / raw)
  To: eric.dumazet; +Cc: francis.moro, linux-kernel, netdev
In-Reply-To: <4AEB0059.1050400@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Oct 2009 16:03:53 +0100

> [PATCH take2] net: fix sk_forward_alloc corruption
> 
> On UDP sockets, we must call skb_free_datagram() with socket locked,
> or risk sk_forward_alloc corruption. This requirement is not respected
> in SUNRPC.
> 
> Add a convenient helper, skb_free_datagram_locked() and use it in SUNRPC
> 
> Reported-by: Francis Moreau <francis.moro@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

I've tentatively applied this to my net-2.6 tree, I won't
push it out until we have positive testing results.

^ permalink raw reply

* Re: [PATCH net-next 0/5] RDS: minor updates for net-next
From: David Miller @ 2009-10-30 19:23 UTC (permalink / raw)
  To: andy.grover; +Cc: netdev, rds-devel
In-Reply-To: <1256928893-17844-1-git-send-email-andy.grover@oracle.com>

From: Andy Grover <andy.grover@oracle.com>
Date: Fri, 30 Oct 2009 11:54:52 -0700

> These patches fix some small issues with RDS, please review and
> apply if ok.

Adding new socket options is not appropriate for -rc* series
submissions.

The changes that fix races and panics are fine, the rest is not.

Please respin your changes properly for net-2.6, and we can add the
socket option and do the other stuff in net-next-2.6

Thanks.

^ permalink raw reply

* Re: Connection tracking and vlan
From: Adayadil Thomas @ 2009-10-30 19:20 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Herbert Xu, netdev, Patrick McHardy
In-Reply-To: <4AEB06E6.6020206@gmail.com>

On Fri, Oct 30, 2009 at 11:31 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Very strange, this question about vlan looks like discussion we had
> yesterday (or the day before...) about interfaces (versus packet defragmentation)
>
> "IP conntracking" is about IP, and [V]LAN doesnt matter at all at this protocol level.
>
> Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
> include interface name/index

I am concerned about the following situation -

The linux device is configured as a bridge and is deployed between the
trunk ports
of 2 switches. In this situation the linux device will be seeing 802.1q packets
with vlan headers specifying the vlanids.

It is valid to have an environment where private IP addresses are duplicated
on different virtual LANs. i.e. it is valid to have machine A (
10.10.10.1) talking to
machine B (10.10.10.2) on vlan 1,
and
at the same time machine C ( 10.10.10.1) talking to machine D
(10.10.10.2) on vlan 2.

Since they are on different LANs (VLANs), there should not be any issues.

Now when VLANs are shared across switches, the trunk port will sent
802.1q tagged
packets between the switches. Imagine these packets when going through
the linux bridge.
The 802.1q header should identify the separate vlans by the vlan id.

If ip_conntrack does not consider vlans, it is possible that all 5
tuple are the same
and thus affect the connection tracking.

I hope I have described the scenario well. If not I can explain in a
more detailed fashion.

Thanks

^ permalink raw reply

* Re: [PATCH] sky2: set carrier off in probe
From: Herbert Xu @ 2009-10-30 19:13 UTC (permalink / raw)
  To: David Miller; +Cc: bphilips, shemminger, netdev
In-Reply-To: <20091030.112051.20510373.davem@davemloft.net>

On Fri, Oct 30, 2009 at 11:20:51AM -0700, David Miller wrote:
>
> Since many drivers (especially virtual software ones) do not manage
> carrier state and therefore that's why we start in state carrier on.
> 
> We've had this discussion a few times before, most recently with
> Rusty wrt. virtio :-)

You're right, modifying the driver is the best way.  Oh well,
at least it's easy to find them by searching for netif_carrier :)
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH net-next 0/5] RDS: minor updates for net-next
From: Andy Grover @ 2009-10-30 18:54 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel

These patches fix some small issues with RDS, please review and
apply if ok.

Thanks -- Regards -- Andy


^ permalink raw reply

* [PATCH 1/5] RDS: Add GET_MR_FOR_DEST sockopt
From: Andy Grover @ 2009-10-30 18:54 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1256928893-17844-1-git-send-email-andy.grover@oracle.com>

RDS currently supports a GET_MR sockopt to establish a
memory region (MR) for a chunk of memory. However, the fastreg
method ties a MR to a particular destination. The GET_MR_FOR_DEST
sockopt allows the remote machine to be specified, and thus
support for fastreg (aka FRWRs).

Note that this patch does *not* do all of this - it simply
implements the new sockopt in terms of the old one, so applications
can begin to use the new sockopt in preparation for cutover to
FRWRs.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 include/linux/rds.h |    8 ++++++++
 net/rds/af_rds.c    |    3 +++
 net/rds/rdma.c      |   24 ++++++++++++++++++++++++
 net/rds/rdma.h      |    1 +
 4 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/linux/rds.h b/include/linux/rds.h
index 89d46e1..cab4994 100644
--- a/include/linux/rds.h
+++ b/include/linux/rds.h
@@ -56,6 +56,7 @@
 /* deprecated: RDS_BARRIER 4 */
 #define RDS_RECVERR			5
 #define RDS_CONG_MONITOR		6
+#define RDS_GET_MR_FOR_DEST		7
 
 /*
  * Control message types for SOL_RDS.
@@ -224,6 +225,13 @@ struct rds_get_mr_args {
 	uint64_t	flags;
 };
 
+struct rds_get_mr_for_dest_args {
+	struct sockaddr_storage	dest_addr;
+	struct rds_iovec 	vec;
+	u_int64_t		cookie_addr;
+	uint64_t		flags;
+};
+
 struct rds_free_mr_args {
 	rds_rdma_cookie_t cookie;
 	u_int64_t	flags;
diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
index a202e5b..2b978dc 100644
--- a/net/rds/af_rds.c
+++ b/net/rds/af_rds.c
@@ -265,6 +265,9 @@ static int rds_setsockopt(struct socket *sock, int level, int optname,
 	case RDS_GET_MR:
 		ret = rds_get_mr(rs, optval, optlen);
 		break;
+	case RDS_GET_MR_FOR_DEST:
+		ret = rds_get_mr_for_dest(rs, optval, optlen);
+		break;
 	case RDS_FREE_MR:
 		ret = rds_free_mr(rs, optval, optlen);
 		break;
diff --git a/net/rds/rdma.c b/net/rds/rdma.c
index 8dc83d2..971b5a6 100644
--- a/net/rds/rdma.c
+++ b/net/rds/rdma.c
@@ -317,6 +317,30 @@ int rds_get_mr(struct rds_sock *rs, char __user *optval, int optlen)
 	return __rds_rdma_map(rs, &args, NULL, NULL);
 }
 
+int rds_get_mr_for_dest(struct rds_sock *rs, char __user *optval, int optlen)
+{
+	struct rds_get_mr_for_dest_args args;
+	struct rds_get_mr_args new_args;
+
+	if (optlen != sizeof(struct rds_get_mr_for_dest_args))
+		return -EINVAL;
+
+	if (copy_from_user(&args, (struct rds_get_mr_for_dest_args __user *)optval,
+			   sizeof(struct rds_get_mr_for_dest_args)))
+		return -EFAULT;
+
+	/*
+	 * Initially, just behave like get_mr().
+	 * TODO: Implement get_mr as wrapper around this
+	 *	 and deprecate it.
+	 */
+	new_args.vec = args.vec;
+	new_args.cookie_addr = args.cookie_addr;
+	new_args.flags = args.flags;
+
+	return __rds_rdma_map(rs, &new_args, NULL, NULL);
+}
+
 /*
  * Free the MR indicated by the given R_Key
  */
diff --git a/net/rds/rdma.h b/net/rds/rdma.h
index 4255120..909c398 100644
--- a/net/rds/rdma.h
+++ b/net/rds/rdma.h
@@ -61,6 +61,7 @@ static inline u32 rds_rdma_cookie_offset(rds_rdma_cookie_t cookie)
 }
 
 int rds_get_mr(struct rds_sock *rs, char __user *optval, int optlen);
+int rds_get_mr_for_dest(struct rds_sock *rs, char __user *optval, int optlen);
 int rds_free_mr(struct rds_sock *rs, char __user *optval, int optlen);
 void rds_rdma_drop_keys(struct rds_sock *rs);
 int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 2/5] RDS: Fix potential race around rds_i[bw]_allocation
From: Andy Grover @ 2009-10-30 18:51 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1256928717-17757-1-git-send-email-andy.grover@oracle.com>

>From Shin Hong:

"At rds_ib_recv_refill_one(), it first executes atomic_read(&rds_ib_allocation)
for if-condition checking,

and then executes atomic_inc(&rds_ib_allocation) if the condition was
not satisfied.

However, if any other code which updates rds_ib_allocation executes
between these two atomic operation executions,
it seems that it may result race condition. (especially when
rds_ib_allocation + 1 == rds_ib_sysctl_max_recv_allocation)"

This patch fixes this by using atomic_inc_unless to eliminate the
possibility of allocating more than rds_ib_sysctl_max_recv_allocation
and then decrementing the count if the allocation fails. It also
makes an identical change to the iwarp transport.

Reported-by: Shin Hong <hongshin@gmail.com>
Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 net/rds/ib_recv.c |    7 ++++---
 net/rds/iw_recv.c |    7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index cd7a6cf..2f009d3 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -143,15 +143,16 @@ static int rds_ib_recv_refill_one(struct rds_connection *conn,
 	int ret = -ENOMEM;
 
 	if (recv->r_ibinc == NULL) {
-		if (atomic_read(&rds_ib_allocation) >= rds_ib_sysctl_max_recv_allocation) {
+		if (!atomic_add_unless(&rds_ib_allocation, 1, rds_ib_sysctl_max_recv_allocation)) {
 			rds_ib_stats_inc(s_ib_rx_alloc_limit);
 			goto out;
 		}
 		recv->r_ibinc = kmem_cache_alloc(rds_ib_incoming_slab,
 						 kptr_gfp);
-		if (recv->r_ibinc == NULL)
+		if (recv->r_ibinc == NULL) {
+			atomic_dec(&rds_ib_allocation);
 			goto out;
-		atomic_inc(&rds_ib_allocation);
+		}
 		INIT_LIST_HEAD(&recv->r_ibinc->ii_frags);
 		rds_inc_init(&recv->r_ibinc->ii_inc, conn, conn->c_faddr);
 	}
diff --git a/net/rds/iw_recv.c b/net/rds/iw_recv.c
index 8683f5f..9f98150 100644
--- a/net/rds/iw_recv.c
+++ b/net/rds/iw_recv.c
@@ -143,15 +143,16 @@ static int rds_iw_recv_refill_one(struct rds_connection *conn,
 	int ret = -ENOMEM;
 
 	if (recv->r_iwinc == NULL) {
-		if (atomic_read(&rds_iw_allocation) >= rds_iw_sysctl_max_recv_allocation) {
+		if (!atomic_add_unless(&rds_iw_allocation, 1, rds_iw_sysctl_max_recv_allocation)) {
 			rds_iw_stats_inc(s_iw_rx_alloc_limit);
 			goto out;
 		}
 		recv->r_iwinc = kmem_cache_alloc(rds_iw_incoming_slab,
 						 kptr_gfp);
-		if (recv->r_iwinc == NULL)
+		if (recv->r_iwinc == NULL) {
+			atomic_dec(&rds_iw_allocation);
 			goto out;
-		atomic_inc(&rds_iw_allocation);
+		}
 		INIT_LIST_HEAD(&recv->r_iwinc->ii_frags);
 		rds_inc_init(&recv->r_iwinc->ii_inc, conn, conn->c_faddr);
 	}
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 5/5] RDS/IB+IW: Move recv processing to a tasklet
From: Andy Grover @ 2009-10-30 18:51 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1256928717-17757-1-git-send-email-andy.grover@oracle.com>

Move receive processing from event handler to a tasklet.
This should help prevent hangcheck timer from going off
when RDS is under heavy load.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 net/rds/ib.h      |    2 ++
 net/rds/ib_cm.c   |    2 ++
 net/rds/ib_recv.c |   28 ++++++++++++++++++++++------
 net/rds/iw.h      |    2 ++
 net/rds/iw_cm.c   |    2 ++
 net/rds/iw_recv.c |   28 ++++++++++++++++++++++------
 6 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/net/rds/ib.h b/net/rds/ib.h
index 1378b85..64df4e7 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -98,6 +98,7 @@ struct rds_ib_connection {
 	struct rds_ib_send_work *i_sends;
 
 	/* rx */
+	struct tasklet_struct	i_recv_tasklet;
 	struct mutex		i_recv_mutex;
 	struct rds_ib_work_ring	i_recv_ring;
 	struct rds_ib_incoming	*i_ibinc;
@@ -303,6 +304,7 @@ void rds_ib_inc_free(struct rds_incoming *inc);
 int rds_ib_inc_copy_to_user(struct rds_incoming *inc, struct iovec *iov,
 			     size_t size);
 void rds_ib_recv_cq_comp_handler(struct ib_cq *cq, void *context);
+void rds_ib_recv_tasklet_fn(unsigned long data);
 void rds_ib_recv_init_ring(struct rds_ib_connection *ic);
 void rds_ib_recv_clear_ring(struct rds_ib_connection *ic);
 void rds_ib_recv_init_ack(struct rds_ib_connection *ic);
diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index c2d372f..9d32069 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -694,6 +694,8 @@ int rds_ib_conn_alloc(struct rds_connection *conn, gfp_t gfp)
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&ic->ib_node);
+	tasklet_init(&ic->i_recv_tasklet, rds_ib_recv_tasklet_fn,
+		     (unsigned long) ic);
 	mutex_init(&ic->i_recv_mutex);
 #ifndef KERNEL_HAS_ATOMIC64
 	spin_lock_init(&ic->i_ack_lock);
diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index 2f009d3..fe5ab8c 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -825,17 +825,22 @@ void rds_ib_recv_cq_comp_handler(struct ib_cq *cq, void *context)
 {
 	struct rds_connection *conn = context;
 	struct rds_ib_connection *ic = conn->c_transport_data;
-	struct ib_wc wc;
-	struct rds_ib_ack_state state = { 0, };
-	struct rds_ib_recv_work *recv;
 
 	rdsdebug("conn %p cq %p\n", conn, cq);
 
 	rds_ib_stats_inc(s_ib_rx_cq_call);
 
-	ib_req_notify_cq(cq, IB_CQ_SOLICITED);
+	tasklet_schedule(&ic->i_recv_tasklet);
+}
 
-	while (ib_poll_cq(cq, 1, &wc) > 0) {
+static inline void rds_poll_cq(struct rds_ib_connection *ic,
+			       struct rds_ib_ack_state *state)
+{
+	struct rds_connection *conn = ic->conn;
+	struct ib_wc wc;
+	struct rds_ib_recv_work *recv;
+
+	while (ib_poll_cq(ic->i_recv_cq, 1, &wc) > 0) {
 		rdsdebug("wc wr_id 0x%llx status %u byte_len %u imm_data %u\n",
 			 (unsigned long long)wc.wr_id, wc.status, wc.byte_len,
 			 be32_to_cpu(wc.ex.imm_data));
@@ -853,7 +858,7 @@ void rds_ib_recv_cq_comp_handler(struct ib_cq *cq, void *context)
 		if (rds_conn_up(conn) || rds_conn_connecting(conn)) {
 			/* We expect errors as the qp is drained during shutdown */
 			if (wc.status == IB_WC_SUCCESS) {
-				rds_ib_process_recv(conn, recv, wc.byte_len, &state);
+				rds_ib_process_recv(conn, recv, wc.byte_len, state);
 			} else {
 				rds_ib_conn_error(conn, "recv completion on "
 				       "%pI4 had status %u, disconnecting and "
@@ -864,6 +869,17 @@ void rds_ib_recv_cq_comp_handler(struct ib_cq *cq, void *context)
 
 		rds_ib_ring_free(&ic->i_recv_ring, 1);
 	}
+}
+
+void rds_ib_recv_tasklet_fn(unsigned long data)
+{
+	struct rds_ib_connection *ic = (struct rds_ib_connection *) data;
+	struct rds_connection *conn = ic->conn;
+	struct rds_ib_ack_state state = { 0, };
+
+	rds_poll_cq(ic, &state);
+	ib_req_notify_cq(ic->i_recv_cq, IB_CQ_SOLICITED);
+	rds_poll_cq(ic, &state);
 
 	if (state.ack_next_valid)
 		rds_ib_set_ack(ic, state.ack_next, state.ack_required);
diff --git a/net/rds/iw.h b/net/rds/iw.h
index dd72b62..eef2f0c 100644
--- a/net/rds/iw.h
+++ b/net/rds/iw.h
@@ -119,6 +119,7 @@ struct rds_iw_connection {
 	struct rds_iw_send_work *i_sends;
 
 	/* rx */
+	struct tasklet_struct	i_recv_tasklet;
 	struct mutex		i_recv_mutex;
 	struct rds_iw_work_ring	i_recv_ring;
 	struct rds_iw_incoming	*i_iwinc;
@@ -330,6 +331,7 @@ void rds_iw_inc_free(struct rds_incoming *inc);
 int rds_iw_inc_copy_to_user(struct rds_incoming *inc, struct iovec *iov,
 			     size_t size);
 void rds_iw_recv_cq_comp_handler(struct ib_cq *cq, void *context);
+void rds_iw_recv_tasklet_fn(unsigned long data);
 void rds_iw_recv_init_ring(struct rds_iw_connection *ic);
 void rds_iw_recv_clear_ring(struct rds_iw_connection *ic);
 void rds_iw_recv_init_ack(struct rds_iw_connection *ic);
diff --git a/net/rds/iw_cm.c b/net/rds/iw_cm.c
index a416b0d..394cf6b 100644
--- a/net/rds/iw_cm.c
+++ b/net/rds/iw_cm.c
@@ -696,6 +696,8 @@ int rds_iw_conn_alloc(struct rds_connection *conn, gfp_t gfp)
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&ic->iw_node);
+	tasklet_init(&ic->i_recv_tasklet, rds_iw_recv_tasklet_fn,
+		     (unsigned long) ic);
 	mutex_init(&ic->i_recv_mutex);
 #ifndef KERNEL_HAS_ATOMIC64
 	spin_lock_init(&ic->i_ack_lock);
diff --git a/net/rds/iw_recv.c b/net/rds/iw_recv.c
index 9f98150..24fc53f 100644
--- a/net/rds/iw_recv.c
+++ b/net/rds/iw_recv.c
@@ -784,17 +784,22 @@ void rds_iw_recv_cq_comp_handler(struct ib_cq *cq, void *context)
 {
 	struct rds_connection *conn = context;
 	struct rds_iw_connection *ic = conn->c_transport_data;
-	struct ib_wc wc;
-	struct rds_iw_ack_state state = { 0, };
-	struct rds_iw_recv_work *recv;
 
 	rdsdebug("conn %p cq %p\n", conn, cq);
 
 	rds_iw_stats_inc(s_iw_rx_cq_call);
 
-	ib_req_notify_cq(cq, IB_CQ_SOLICITED);
+	tasklet_schedule(&ic->i_recv_tasklet);
+}
 
-	while (ib_poll_cq(cq, 1, &wc) > 0) {
+static inline void rds_poll_cq(struct rds_iw_connection *ic,
+			       struct rds_iw_ack_state *state)
+{
+	struct rds_connection *conn = ic->conn;
+	struct ib_wc wc;
+	struct rds_iw_recv_work *recv;
+
+	while (ib_poll_cq(ic->i_recv_cq, 1, &wc) > 0) {
 		rdsdebug("wc wr_id 0x%llx status %u byte_len %u imm_data %u\n",
 			 (unsigned long long)wc.wr_id, wc.status, wc.byte_len,
 			 be32_to_cpu(wc.ex.imm_data));
@@ -812,7 +817,7 @@ void rds_iw_recv_cq_comp_handler(struct ib_cq *cq, void *context)
 		if (rds_conn_up(conn) || rds_conn_connecting(conn)) {
 			/* We expect errors as the qp is drained during shutdown */
 			if (wc.status == IB_WC_SUCCESS) {
-				rds_iw_process_recv(conn, recv, wc.byte_len, &state);
+				rds_iw_process_recv(conn, recv, wc.byte_len, state);
 			} else {
 				rds_iw_conn_error(conn, "recv completion on "
 				       "%pI4 had status %u, disconnecting and "
@@ -823,6 +828,17 @@ void rds_iw_recv_cq_comp_handler(struct ib_cq *cq, void *context)
 
 		rds_iw_ring_free(&ic->i_recv_ring, 1);
 	}
+}
+
+void rds_iw_recv_tasklet_fn(unsigned long data)
+{
+	struct rds_iw_connection *ic = (struct rds_iw_connection *) data;
+	struct rds_connection *conn = ic->conn;
+	struct rds_iw_ack_state state = { 0, };
+
+	rds_poll_cq(ic, &state);
+	ib_req_notify_cq(ic->i_recv_cq, IB_CQ_SOLICITED);
+	rds_poll_cq(ic, &state);
 
 	if (state.ack_next_valid)
 		rds_iw_set_ack(ic, state.ack_next, state.ack_required);
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 4/5] RDS: Do not send congestion updates to loopback connections
From: Andy Grover @ 2009-10-30 18:51 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1256928717-17757-1-git-send-email-andy.grover@oracle.com>

This issue was discovered by HP's Pradeep and fixed in OFED
1.3, but not fixed in later versions, since the fix's implementation
was not immediately applyable to the later code. This patch should
do the trick for 1.4+ codebases.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 net/rds/cong.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/rds/cong.c b/net/rds/cong.c
index dd2711d..6d06cac 100644
--- a/net/rds/cong.c
+++ b/net/rds/cong.c
@@ -218,6 +218,8 @@ void rds_cong_queue_updates(struct rds_cong_map *map)
 	spin_lock_irqsave(&rds_cong_lock, flags);
 
 	list_for_each_entry(conn, &map->m_conn_list, c_map_item) {
+		if (conn->c_loopback)
+			continue;
 		if (!test_and_set_bit(0, &conn->c_map_queued)) {
 			rds_stats_inc(s_cong_update_queued);
 			queue_delayed_work(rds_wq, &conn->c_send_w, 0);
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 3/5] RDS: Fix panic on unload
From: Andy Grover @ 2009-10-30 18:51 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1256928717-17757-1-git-send-email-andy.grover@oracle.com>

Remove explicit destruction of passive connection when destroying
active end of the connection. The passive end is also on the
device's connection list, and will thus be cleaned up properly.
Panic was caused by trying to clean it up twice.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 net/rds/ib_rdma.c |    5 +----
 net/rds/iw_rdma.c |    5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/rds/ib_rdma.c b/net/rds/ib_rdma.c
index ef3ab5b..c5e9165 100644
--- a/net/rds/ib_rdma.c
+++ b/net/rds/ib_rdma.c
@@ -187,11 +187,8 @@ void __rds_ib_destroy_conns(struct list_head *list, spinlock_t *list_lock)
 	INIT_LIST_HEAD(list);
 	spin_unlock_irq(list_lock);
 
-	list_for_each_entry_safe(ic, _ic, &tmp_list, ib_node) {
-		if (ic->conn->c_passive)
-			rds_conn_destroy(ic->conn->c_passive);
+	list_for_each_entry_safe(ic, _ic, &tmp_list, ib_node)
 		rds_conn_destroy(ic->conn);
-	}
 }
 
 struct rds_ib_mr_pool *rds_ib_create_mr_pool(struct rds_ib_device *rds_ibdev)
diff --git a/net/rds/iw_rdma.c b/net/rds/iw_rdma.c
index de4a1b1..b25d785 100644
--- a/net/rds/iw_rdma.c
+++ b/net/rds/iw_rdma.c
@@ -245,11 +245,8 @@ void __rds_iw_destroy_conns(struct list_head *list, spinlock_t *list_lock)
 	INIT_LIST_HEAD(list);
 	spin_unlock_irq(list_lock);
 
-	list_for_each_entry_safe(ic, _ic, &tmp_list, iw_node) {
-		if (ic->conn->c_passive)
-			rds_conn_destroy(ic->conn->c_passive);
+	list_for_each_entry_safe(ic, _ic, &tmp_list, iw_node)
 		rds_conn_destroy(ic->conn);
-	}
 }
 
 static void rds_iw_set_scatterlist(struct rds_iw_scatterlist *sg,
-- 
1.6.3.3


^ permalink raw reply related

* Re: RFC: netdev: allow ethtool physical id to drop rtnl_lock
From: Stephen Hemminger @ 2009-10-30 18:30 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4AEB26BB.1050007@gmail.com>

On Fri, 30 Oct 2009 18:47:39 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Stephen Hemminger a écrit :
> > The ethtool operation to blink LED can take an indeterminately long time,
> > blocking out other operations (via rtnl_lock). This patch is an attempt
> > to work around the problem.
> > 
> > It does need more discussion, because it will mean that drivers that formerly
> > were protected from changes during blink aren't.  For example, user could
> > start device blinking, and then plug in cable causing change netlink event
> > to change state or pull cable and have device come down.
> > 
> > The other possibility is to do this on a driver by driver basis
> > which is more effort.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > 
> > 
> > --- a/net/core/ethtool.c	2009-10-30 10:27:23.621917624 -0700
> > +++ b/net/core/ethtool.c	2009-10-30 10:35:53.787670774 -0700
> > @@ -17,6 +17,7 @@
> >  #include <linux/errno.h>
> >  #include <linux/ethtool.h>
> >  #include <linux/netdevice.h>
> > +#include <linux/rtnetlink.h>
> >  #include <asm/uaccess.h>
> >  
> >  /*
> > @@ -781,6 +782,8 @@ static int ethtool_get_strings(struct ne
> >  static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
> >  {
> >  	struct ethtool_value id;
> > +	int err;
> > +	static int busy;
> >  
> >  	if (!dev->ethtool_ops->phys_id)
> >  		return -EOPNOTSUPP;
> > @@ -788,7 +791,21 @@ static int ethtool_phys_id(struct net_de
> >  	if (copy_from_user(&id, useraddr, sizeof(id)))
> >  		return -EFAULT;
> >  
> > -	return dev->ethtool_ops->phys_id(dev, id.data);
> > +	if (busy)
> > +		return -EBUSY;
> > +
> > +	/* This operation may take a long time, drop lock */
> > +	busy = 1;
> > +	dev_hold(dev);
> > +	rtnl_unlock();
> > +
> > +	err = dev->ethtool_ops->phys_id(dev, id.data);
> > +
> > +	rtnl_lock();
> > +	dev_put(dev);
> > +	busy = 0;
> > +
> > +	return err;
> >  }
> >  
> 
> It seems reasonable, but why have a global 'busy' flag, and not
> private to each netdev ?
> 

Because that is what old behaviour was (one blink at a time).


-- 

^ permalink raw reply

* Re: [PATCH] sky2: set carrier off in probe
From: David Miller @ 2009-10-30 18:20 UTC (permalink / raw)
  To: herbert; +Cc: bphilips, shemminger, netdev
In-Reply-To: <20091030181632.GA9530@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 30 Oct 2009 14:16:32 -0400

> On Fri, Oct 30, 2009 at 11:10:27AM -0700, David Miller wrote:
> . 
>> Because the driver must start in state with carrier off anyways,
>> so that we get the transition event when the device comes up
>> from link down to link up.
> 
> Well we want all drivers to start in state NOCARRIER.  However,
> a freshly allocated netdev has the NOCARRIER bit off.  This means
> every single driver has to set the NOCARRIER bit.
> 
> It would seem much easier to ensure that the NOCARRIER bit is set
> in a newly allocated netdev.

Since many drivers (especially virtual software ones) do not manage
carrier state and therefore that's why we start in state carrier on.

We've had this discussion a few times before, most recently with
Rusty wrt. virtio :-)

^ permalink raw reply

* Re: [PATCH] sky2: set carrier off in probe
From: Herbert Xu @ 2009-10-30 18:16 UTC (permalink / raw)
  To: David Miller; +Cc: bphilips, shemminger, netdev
In-Reply-To: <20091030.111027.234994529.davem@davemloft.net>

On Fri, Oct 30, 2009 at 11:10:27AM -0700, David Miller wrote:
. 
> Because the driver must start in state with carrier off anyways,
> so that we get the transition event when the device comes up
> from link down to link up.

Well we want all drivers to start in state NOCARRIER.  However,
a freshly allocated netdev has the NOCARRIER bit off.  This means
every single driver has to set the NOCARRIER bit.

It would seem much easier to ensure that the NOCARRIER bit is set
in a newly allocated netdev.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: William Allen Simpson @ 2009-10-30 18:11 UTC (permalink / raw)
  To: Linux Kernel Network Developers
In-Reply-To: <4AEAC763.4070200@gmail.com>

William Allen Simpson wrote:
> First of all, this is my first attempt at locks, so I'd like early review.
> 
> Secondly, scripts/checkpatch.pl tells me:
> 
> ERROR: do not initialise statics to 0 or NULL
> #95: FILE: net/ipv4/tcp.c:2977:
> +static struct tcp_cookie_secret *tcp_secret_generating = NULL;
> 
> They need to be NULL, and I'm not planning on exporting them, so what's
> the preferred mechanism?
> 
> (I've grep'd many other instances of statics = 0 or NULL, so I'm not 
> alone.)
> 
I've found a void __init tcp_init() that seems to be used to initialize
some things, so I've stuck the NULL in there.

Still would like somebody to assure me that I've used the locks correctly.


^ permalink raw reply

* Re: [PATCH 2/3] net: TCP thin linear timeouts
From: William Allen Simpson @ 2009-10-30 18:11 UTC (permalink / raw)
  To: Rick Jones
  Cc: apetlund, Ilpo Järvinen, Arnd Hannemann, Eric Dumazet,
	Netdev, LKML, shemminger, David Miller
In-Reply-To: <4AEB2362.5060601@hp.com>

Rick Jones wrote:
> apetlund@simula.no wrote:
>>> Just how thin can a thin stream be when a thin stream is found thin? 
>>> (to the cadence of "How much wood could a woodchuck chuck if a 
>>> woodchuck could chuck wood?")
> 
>>> Does a stream get so thin that a user's send could not be split into 
>>> four,
>>>  sub-MSS TCP segments?
>>
>>
>> That was a nifty idea: Anti-Nagle the segments to be able to trigger fast
>> retransmissions. I think it is possible.
>>
>> Besides using more resources on each send, this scheme will introduce the
>> need to delay parts of the segment, which is undesirable for
>> time-dependent applications (the intended target of the mechanisms).
>>
>> I think it would be fun to implement and play around with such a 
>> mechanism
>> to see the effects.
> 
> Indeed, it does feel a bit "anti-nagle" but at the same time, these thin 
> streams are supposed to be quite rare right?  I mean we have survived 20 
> odd years of congestion control and fast retransmission without it being 
> a big issue.
> 
> They are also supposed to not have terribly high bandwidth requirements 
> yes? Suppose that instead of an explicit "I promise to be thin" 
> setsockopt(), they instead set a Very Small (tm) in today's thinking 
> socket buffer size and the stack then picks the MSS to be no more than 
> 1/4 that size?  Or for that matter, assuming the permissions are 
> acceptable, the thin application makes a setsockopt(TCP_MAXSEG) call 
> such that the actual MSS is small enough to allow the send()'s to be 
> four (or more) segments.  And, if one wants to spin-away the anti-Nagle, 
> Nagle is defined by the send() being smaller than the MSS, so if the MSS 
> is smaller, it isn't anti-Nagle :)
> 
This is not a new idea.  Folks used to set the MSS really low for M$
Windows, so that their short little packets went over dialup links more
quickly and they saw a little bit more of their graphic as it crawled to
the screen.  Even though it was actually slower in total time, it "felt"
faster because of the continuing visual feedback.  It depended upon VJ
Header Prediction to keep the overhead down for the link.

These are/were called "TCP mice", and the result was routers and servers
being nibbled by mice.  Not pleasant.


> Further blue-skying...
> 
> If SACK were also enabled, it would seem that only loss of the last 
> segment in the "thin train" would be an issue?  Presumably, the thin 
> stream receiver would be in a position to detect this, perhaps with an 
> application-level timeout. Whether then it would suffice to allow the 
> receiving app to make a setsockopt() call to force an extra ACK or two 
> I'm not sure.  Perhaps if the thin-stream had a semi-aggressive 
> "heartbeat" going...
> 
Heartbeats are the usual solution for gaming.  Handles a host of
issues, including detection of clients that have become unreachable.

(No, these are not the same as TCP keep-alives.)

Beside my code in the field and widespread discussion, I know that Paul
Francis had several related papers a decade or so ago.  My memory is that
younger game coders weren't particularly avid readers....


> But it does seem that it should be possible to deal with this sort of 
> thing without having to make wholesale changes to TCP's RTO policies and 
> whatnot?
> 
Yep.

^ permalink raw reply

* Re: [PATCH] sky2: set carrier off in probe
From: David Miller @ 2009-10-30 18:11 UTC (permalink / raw)
  To: shemminger; +Cc: bphilips, netdev
In-Reply-To: <20091030083452.68ca227e@nehalam>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 30 Oct 2009 08:34:52 -0700

> Why not fix the problem in a generic way?

Drivers still need to make sure carrier is off when their
->open() routine runs, so that the transition event from
link down to link up occurs properly when the device is brought
up.

So mucking around with this carrier test will only hide the
bugs, not make things easier.

^ permalink raw reply

* Re: [PATCH] sky2: set carrier off in probe
From: David Miller @ 2009-10-30 18:10 UTC (permalink / raw)
  To: herbert; +Cc: bphilips, shemminger, netdev
In-Reply-To: <20091030155420.GA8345@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 30 Oct 2009 11:54:20 -0400

> David Miller <davem@davemloft.net> wrote:
>> 
>> I agree that we should report something consistent before interface
>> up and 'no' is probably the best.
>> 
>> I remember fixing something similar in other drivers a few months
>> ago.
> 
> Can't we do this in one spot rather than having every driver
> duplicate this?

It doesn't matter if we do.

Because the driver must start in state with carrier off anyways,
so that we get the transition event when the device comes up
from link down to link up.

So many things depend upon that link state transition event, that
we're not saving anything by just mucking with the carrier test.

^ permalink raw reply

* Re: RFC: netdev: allow ethtool physical id to drop rtnl_lock
From: Eric Dumazet @ 2009-10-30 17:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20091030104201.6bb03b23@nehalam>

Stephen Hemminger a écrit :
> The ethtool operation to blink LED can take an indeterminately long time,
> blocking out other operations (via rtnl_lock). This patch is an attempt
> to work around the problem.
> 
> It does need more discussion, because it will mean that drivers that formerly
> were protected from changes during blink aren't.  For example, user could
> start device blinking, and then plug in cable causing change netlink event
> to change state or pull cable and have device come down.
> 
> The other possibility is to do this on a driver by driver basis
> which is more effort.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> 
> --- a/net/core/ethtool.c	2009-10-30 10:27:23.621917624 -0700
> +++ b/net/core/ethtool.c	2009-10-30 10:35:53.787670774 -0700
> @@ -17,6 +17,7 @@
>  #include <linux/errno.h>
>  #include <linux/ethtool.h>
>  #include <linux/netdevice.h>
> +#include <linux/rtnetlink.h>
>  #include <asm/uaccess.h>
>  
>  /*
> @@ -781,6 +782,8 @@ static int ethtool_get_strings(struct ne
>  static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
>  {
>  	struct ethtool_value id;
> +	int err;
> +	static int busy;
>  
>  	if (!dev->ethtool_ops->phys_id)
>  		return -EOPNOTSUPP;
> @@ -788,7 +791,21 @@ static int ethtool_phys_id(struct net_de
>  	if (copy_from_user(&id, useraddr, sizeof(id)))
>  		return -EFAULT;
>  
> -	return dev->ethtool_ops->phys_id(dev, id.data);
> +	if (busy)
> +		return -EBUSY;
> +
> +	/* This operation may take a long time, drop lock */
> +	busy = 1;
> +	dev_hold(dev);
> +	rtnl_unlock();
> +
> +	err = dev->ethtool_ops->phys_id(dev, id.data);
> +
> +	rtnl_lock();
> +	dev_put(dev);
> +	busy = 0;
> +
> +	return err;
>  }
>  

It seems reasonable, but why have a global 'busy' flag, and not
private to each netdev ?


^ permalink raw reply

* RFC: netdev: allow ethtool physical id to drop rtnl_lock
From: Stephen Hemminger @ 2009-10-30 17:42 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

The ethtool operation to blink LED can take an indeterminately long time,
blocking out other operations (via rtnl_lock). This patch is an attempt
to work around the problem.

It does need more discussion, because it will mean that drivers that formerly
were protected from changes during blink aren't.  For example, user could
start device blinking, and then plug in cable causing change netlink event
to change state or pull cable and have device come down.

The other possibility is to do this on a driver by driver basis
which is more effort.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/net/core/ethtool.c	2009-10-30 10:27:23.621917624 -0700
+++ b/net/core/ethtool.c	2009-10-30 10:35:53.787670774 -0700
@@ -17,6 +17,7 @@
 #include <linux/errno.h>
 #include <linux/ethtool.h>
 #include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
 #include <asm/uaccess.h>
 
 /*
@@ -781,6 +782,8 @@ static int ethtool_get_strings(struct ne
 static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_value id;
+	int err;
+	static int busy;
 
 	if (!dev->ethtool_ops->phys_id)
 		return -EOPNOTSUPP;
@@ -788,7 +791,21 @@ static int ethtool_phys_id(struct net_de
 	if (copy_from_user(&id, useraddr, sizeof(id)))
 		return -EFAULT;
 
-	return dev->ethtool_ops->phys_id(dev, id.data);
+	if (busy)
+		return -EBUSY;
+
+	/* This operation may take a long time, drop lock */
+	busy = 1;
+	dev_hold(dev);
+	rtnl_unlock();
+
+	err = dev->ethtool_ops->phys_id(dev, id.data);
+
+	rtnl_lock();
+	dev_put(dev);
+	busy = 0;
+
+	return err;
 }
 
 static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)

^ permalink raw reply

* Re: [PATCH 2/3] net: TCP thin linear timeouts
From: Rick Jones @ 2009-10-30 17:33 UTC (permalink / raw)
  To: apetlund
  Cc: Ilpo Järvinen, Arnd Hannemann, Eric Dumazet, Netdev, LKML,
	shemminger, David Miller
In-Reply-To: <6980f83370cc081eb82dc2e0bd65bcf4.squirrel@webmail.uio.no>

apetlund@simula.no wrote:
>> Just how thin can a thin stream be when a thin stream is found thin? (to 
>> the cadence of "How much wood could a woodchuck chuck if a woodchuck could 
>> chuck wood?")

>> Does a stream get so thin that a user's send could not be split into four,
>>  sub-MSS TCP segments?
> 
> 
> That was a nifty idea: Anti-Nagle the segments to be able to trigger fast
> retransmissions. I think it is possible.
> 
> Besides using more resources on each send, this scheme will introduce the
> need to delay parts of the segment, which is undesirable for
> time-dependent applications (the intended target of the mechanisms).
> 
> I think it would be fun to implement and play around with such a mechanism
> to see the effects.

Indeed, it does feel a bit "anti-nagle" but at the same time, these thin streams 
are supposed to be quite rare right?  I mean we have survived 20 odd years of 
congestion control and fast retransmission without it being a big issue.

They are also supposed to not have terribly high bandwidth requirements yes? 
Suppose that instead of an explicit "I promise to be thin" setsockopt(), they 
instead set a Very Small (tm) in today's thinking socket buffer size and the 
stack then picks the MSS to be no more than 1/4 that size?  Or for that matter, 
assuming the permissions are acceptable, the thin application makes a 
setsockopt(TCP_MAXSEG) call such that the actual MSS is small enough to allow 
the send()'s to be four (or more) segments.  And, if one wants to spin-away the 
anti-Nagle, Nagle is defined by the send() being smaller than the MSS, so if the 
MSS is smaller, it isn't anti-Nagle :)

Further blue-skying...

If SACK were also enabled, it would seem that only loss of the last segment in 
the "thin train" would be an issue?  Presumably, the thin stream receiver would 
be in a position to detect this, perhaps with an application-level timeout. 
Whether then it would suffice to allow the receiving app to make a setsockopt() 
call to force an extra ACK or two I'm not sure.  Perhaps if the thin-stream had 
a semi-aggressive "heartbeat" going...

But it does seem that it should be possible to deal with this sort of thing 
without having to make wholesale changes to TCP's RTO policies and whatnot?

rick jones

^ permalink raw reply

* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: Greg KH @ 2009-10-30 17:13 UTC (permalink / raw)
  To: Matt Domsch
  Cc: dann frazier, Narendra_K, kay.sievers, linux-hotplug, netdev,
	Jordan_Hargrave, Charles_Rose, bhutchings
In-Reply-To: <20091030171003.GA7523@auslistsprd01.us.dell.com>

On Fri, Oct 30, 2009 at 12:10:03PM -0500, Matt Domsch wrote:
> ethN is fundamentally a nondeterministic namespace, and trying to
> enforce determinism on it is, from all my attempts, impossible.  Hence
> the desire to change the namespace.  But there can be many
> different naming policies one might want (including the
> nondeterministic ethN policy), and for all other types of devices this
> isn't a problem - we can have all the policies we want, in parallel.
> Only for network devices we can't.

So pick one in your installer and stick with it.  Doesn't seem that
complicated to me...

greg k-h

^ permalink raw reply

* [PATCH net-next-2.6] net: Introduce dev_get_by_name_rcu()
From: Eric Dumazet @ 2009-10-30 17:11 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

Some workloads hit dev_base_lock rwlock pretty hard.
We can use RCU lookups to avoid touching this rwlock
(and avoid touching netdevice refcount)

netdevices are already freed after a RCU grace period, so this patch
adds no penalty at device dismantle time.

However, it adds a synchronize_rcu() call in dev_change_name()

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/netdevice.h |    1 
 net/core/dev.c            |   49 +++++++++++++++++++++++++++++-------
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 193b637..86773e1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1115,6 +1115,7 @@ extern void		__dev_remove_pack(struct packet_type *pt);
 extern struct net_device	*dev_get_by_flags(struct net *net, unsigned short flags,
 						  unsigned short mask);
 extern struct net_device	*dev_get_by_name(struct net *net, const char *name);
+extern struct net_device	*dev_get_by_name_rcu(struct net *net, const char *name);
 extern struct net_device	*__dev_get_by_name(struct net *net, const char *name);
 extern int		dev_alloc_name(struct net_device *dev, const char *name);
 extern int		dev_open(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 94f42a1..f54d8b8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -213,7 +213,7 @@ static int list_netdevice(struct net_device *dev)
 
 	write_lock_bh(&dev_base_lock);
 	list_add_tail(&dev->dev_list, &net->dev_base_head);
-	hlist_add_head(&dev->name_hlist, dev_name_hash(net, dev->name));
+	hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
 	hlist_add_head_rcu(&dev->index_hlist,
 			   dev_index_hash(net, dev->ifindex));
 	write_unlock_bh(&dev_base_lock);
@@ -230,7 +230,7 @@ static void unlist_netdevice(struct net_device *dev)
 	/* Unlink dev from the device chain */
 	write_lock_bh(&dev_base_lock);
 	list_del(&dev->dev_list);
-	hlist_del(&dev->name_hlist);
+	hlist_del_rcu(&dev->name_hlist);
 	hlist_del_rcu(&dev->index_hlist);
 	write_unlock_bh(&dev_base_lock);
 }
@@ -599,6 +599,32 @@ struct net_device *__dev_get_by_name(struct net *net, const char *name)
 EXPORT_SYMBOL(__dev_get_by_name);
 
 /**
+ *	dev_get_by_name_rcu	- find a device by its name
+ *	@net: the applicable net namespace
+ *	@name: name to find
+ *
+ *	Find an interface by name.
+ *	If the name is found a pointer to the device is returned.
+ * 	If the name is not found then %NULL is returned.
+ *	The reference counters are not incremented so the caller must be
+ *	careful with locks. The caller must hold RCU lock.
+ */
+
+struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
+{
+	struct hlist_node *p;
+	struct net_device *dev;
+	struct hlist_head *head = dev_name_hash(net, name);
+
+	hlist_for_each_entry_rcu(dev, p, head, name_hlist)
+		if (!strncmp(dev->name, name, IFNAMSIZ))
+			return dev;
+
+	return NULL;
+}
+EXPORT_SYMBOL(dev_get_by_name_rcu);
+
+/**
  *	dev_get_by_name		- find a device by its name
  *	@net: the applicable net namespace
  *	@name: name to find
@@ -614,11 +640,11 @@ struct net_device *dev_get_by_name(struct net *net, const char *name)
 {
 	struct net_device *dev;
 
-	read_lock(&dev_base_lock);
-	dev = __dev_get_by_name(net, name);
+	rcu_read_lock();
+	dev = dev_get_by_name_rcu(net, name);
 	if (dev)
 		dev_hold(dev);
-	read_unlock(&dev_base_lock);
+	rcu_read_unlock();
 	return dev;
 }
 EXPORT_SYMBOL(dev_get_by_name);
@@ -960,7 +986,12 @@ rollback:
 
 	write_lock_bh(&dev_base_lock);
 	hlist_del(&dev->name_hlist);
-	hlist_add_head(&dev->name_hlist, dev_name_hash(net, dev->name));
+	write_unlock_bh(&dev_base_lock);
+
+	synchronize_rcu();
+
+	write_lock_bh(&dev_base_lock);
+	hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
 	write_unlock_bh(&dev_base_lock);
 
 	ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
@@ -1062,9 +1093,9 @@ void dev_load(struct net *net, const char *name)
 {
 	struct net_device *dev;
 
-	read_lock(&dev_base_lock);
-	dev = __dev_get_by_name(net, name);
-	read_unlock(&dev_base_lock);
+	rcu_read_lock();
+	dev = dev_get_by_name_rcu(net, name);
+	rcu_read_unlock();
 
 	if (!dev && capable(CAP_NET_ADMIN))
 		request_module("%s", name);


^ permalink raw reply related


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