Netdev List
 help / color / mirror / Atom feed
* [PATCH 14/15] RDS/IW: Remove dead code
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

In iWARP code, node_type will always be RNIC

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

diff --git a/net/rds/iw.c b/net/rds/iw.c
index 2e6b495..f5e9a29 100644
--- a/net/rds/iw.c
+++ b/net/rds/iw.c
@@ -89,15 +89,10 @@ void rds_iw_add_one(struct ib_device *device)
 		goto free_dev;
 
 	if (!rds_iwdev->dma_local_lkey) {
-		if (device->node_type != RDMA_NODE_RNIC) {
-			rds_iwdev->mr = ib_get_dma_mr(rds_iwdev->pd,
-						IB_ACCESS_LOCAL_WRITE);
-		} else {
-			rds_iwdev->mr = ib_get_dma_mr(rds_iwdev->pd,
-						IB_ACCESS_REMOTE_READ |
-						IB_ACCESS_REMOTE_WRITE |
-						IB_ACCESS_LOCAL_WRITE);
-		}
+		rds_iwdev->mr = ib_get_dma_mr(rds_iwdev->pd,
+					IB_ACCESS_REMOTE_READ |
+					IB_ACCESS_REMOTE_WRITE |
+					IB_ACCESS_LOCAL_WRITE);
 		if (IS_ERR(rds_iwdev->mr))
 			goto err_pd;
 	} else
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 04/15] RDS/IB: Fix printk to indicate remote IP, not local
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

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

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 1eb0c29..f621086 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -111,7 +111,7 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
 	}
 
 	printk(KERN_NOTICE "RDS/IB: connected to %pI4 version %u.%u%s\n",
-			&conn->c_laddr,
+			&conn->c_faddr,
 			RDS_PROTOCOL_MAJOR(conn->c_version),
 			RDS_PROTOCOL_MINOR(conn->c_version),
 			ic->i_flowctl ? ", flow control" : "");
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 15/15] RDS: Refactor end of __conn_create for readability
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

Add a comment for what's going on. Remove negative logic.
I find this much easier to understand quickly, although
there are a few lines duplicated.

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

diff --git a/net/rds/connection.c b/net/rds/connection.c
index 605fe3f..b420a20 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -126,7 +126,7 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
 				       struct rds_transport *trans, gfp_t gfp,
 				       int is_outgoing)
 {
-	struct rds_connection *conn, *tmp, *parent = NULL;
+	struct rds_connection *conn, *parent = NULL;
 	struct hlist_head *head = rds_conn_bucket(laddr, faddr);
 	unsigned long flags;
 	int ret;
@@ -210,26 +210,40 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
 	  trans->t_name ? trans->t_name : "[unknown]",
 	  is_outgoing ? "(outgoing)" : "");
 
+	/*
+	 * Since we ran without holding the conn lock, someone could
+	 * have created the same conn (either normal or passive) in the
+	 * interim. We check while holding the lock. If we won, we complete
+	 * init and return our conn. If we lost, we rollback and return the
+	 * other one.
+	 */
 	spin_lock_irqsave(&rds_conn_lock, flags);
-	if (parent == NULL) {
-		tmp = rds_conn_lookup(head, laddr, faddr, trans);
-		if (tmp == NULL)
-			hlist_add_head(&conn->c_hash_node, head);
-	} else {
-		tmp = parent->c_passive;
-		if (!tmp)
+	if (parent) {
+		/* Creating passive conn */
+		if (parent->c_passive) {
+			trans->conn_free(conn->c_transport_data);
+			kmem_cache_free(rds_conn_slab, conn);
+			conn = parent->c_passive;
+		} else {
 			parent->c_passive = conn;
-	}
-
-	if (tmp) {
-		trans->conn_free(conn->c_transport_data);
-		kmem_cache_free(rds_conn_slab, conn);
-		conn = tmp;
+			rds_cong_add_conn(conn);
+			rds_conn_count++;
+		}
 	} else {
-		rds_cong_add_conn(conn);
-		rds_conn_count++;
+		/* Creating normal conn */
+		struct rds_connection *found;
+
+		found = rds_conn_lookup(head, laddr, faddr, trans);
+		if (found) {
+			trans->conn_free(conn->c_transport_data);
+			kmem_cache_free(rds_conn_slab, conn);
+			conn = found;
+		} else {
+			hlist_add_head(&conn->c_hash_node, head);
+			rds_cong_add_conn(conn);
+			rds_conn_count++;
+		}
 	}
-
 	spin_unlock_irqrestore(&rds_conn_lock, flags);
 
 out:
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 09/15] RDS/IB: Disable flow control in sysctl and explain why
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

Backwards compatibility with rds 3.0 causes protocol-
based flow control to be disabled as a side-effect.

I don't want to pull out FC support from the IB transport
but I do want to document and keep the sysctl consistent
if possible.

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

diff --git a/net/rds/ib_sysctl.c b/net/rds/ib_sysctl.c
index d87830d..84b5ffc 100644
--- a/net/rds/ib_sysctl.c
+++ b/net/rds/ib_sysctl.c
@@ -53,7 +53,17 @@ unsigned long rds_ib_sysctl_max_unsig_bytes = (16 << 20);
 static unsigned long rds_ib_sysctl_max_unsig_bytes_min = 1;
 static unsigned long rds_ib_sysctl_max_unsig_bytes_max = ~0UL;
 
-unsigned int rds_ib_sysctl_flow_control = 1;
+/*
+ * This sysctl does nothing.
+ *
+ * Backwards compatibility with RDS 3.0 wire protocol
+ * disables initial FC credit exchange.
+ * If it's ever possible to drop 3.0 support,
+ * setting this to 1 and moving init/refill of send/recv
+ * rings from ib_cm_connect_complete() back into ib_setup_qp()
+ * will cause credits to be added before protocol negotiation.
+ */
+unsigned int rds_ib_sysctl_flow_control = 0;
 
 ctl_table rds_ib_sysctl_table[] = {
 	{
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 01/15] RDS: Set retry_count to 2 and make modifiable via modparam
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

This will be default cause IB connections to failover faster,
but allow a longer retry count to be used if desired.

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

diff --git a/net/rds/ib.c b/net/rds/ib.c
index b9bcd32..27abdd3 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -43,11 +43,14 @@
 
 unsigned int fmr_pool_size = RDS_FMR_POOL_SIZE;
 unsigned int fmr_message_size = RDS_FMR_SIZE + 1; /* +1 allows for unaligned MRs */
+unsigned int rds_ib_retry_count = RDS_IB_DEFAULT_RETRY_COUNT;
 
 module_param(fmr_pool_size, int, 0444);
 MODULE_PARM_DESC(fmr_pool_size, " Max number of fmr per HCA");
 module_param(fmr_message_size, int, 0444);
 MODULE_PARM_DESC(fmr_message_size, " Max size of a RDMA transfer");
+module_param(rds_ib_retry_count, int, 0444);
+MODULE_PARM_DESC(rds_ib_retry_count, " Number of hw retries before reporting an error");
 
 struct list_head rds_ib_devices;
 
diff --git a/net/rds/ib.h b/net/rds/ib.h
index 455ae73..420afb9 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -15,6 +15,8 @@
 #define RDS_IB_DEFAULT_RECV_WR		1024
 #define RDS_IB_DEFAULT_SEND_WR		256
 
+#define RDS_IB_DEFAULT_RETRY_COUNT	2
+
 #define RDS_IB_SUPPORTED_PROTOCOLS	0x00000003	/* minor versions supported */
 
 extern struct list_head rds_ib_devices;
@@ -247,6 +249,7 @@ extern struct ib_client rds_ib_client;
 
 extern unsigned int fmr_pool_size;
 extern unsigned int fmr_message_size;
+extern unsigned int rds_ib_retry_count;
 
 extern spinlock_t ib_nodev_conns_lock;
 extern struct list_head ib_nodev_conns;
diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index f8e40e1..605c032 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -145,7 +145,7 @@ static void rds_ib_cm_fill_conn_param(struct rds_connection *conn,
 	/* XXX tune these? */
 	conn_param->responder_resources = 1;
 	conn_param->initiator_depth = 1;
-	conn_param->retry_count = 7;
+	conn_param->retry_count = min_t(unsigned int, rds_ib_retry_count, 7);
 	conn_param->rnr_retry_count = 7;
 
 	if (dp) {
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 03/15] RDS/IB: Handle connections using RDS 3.0 wire protocol
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

The big differences between RDS 3.0 and 3.1 are protocol-level
flow control, and with 3.1 the header is in front of the data. The header
always ends up in the header buffer, and the data goes in the data page.

In 3.0 our "header" is a trailer, and will end up either in the data
page, the header buffer, or split across the two. Since 3.1 is backwards-
compatible with 3.0, we need to continue to support these cases. This
patch does that -- if using RDS 3.0 wire protocol, it will copy the header
from wherever it ended up into the header buffer.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 net/rds/ib.h      |   12 ++++++++++--
 net/rds/ib_cm.c   |    9 ++++++---
 net/rds/ib_recv.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/net/rds/ib.h b/net/rds/ib.h
index 420afb9..c0de7af 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -358,17 +358,25 @@ extern ctl_table rds_ib_sysctl_table[];
 /*
  * Helper functions for getting/setting the header and data SGEs in
  * RDS packets (not RDMA)
+ *
+ * From version 3.1 onwards, header is in front of data in the sge.
  */
 static inline struct ib_sge *
 rds_ib_header_sge(struct rds_ib_connection *ic, struct ib_sge *sge)
 {
-	return &sge[0];
+	if (ic->conn->c_version > RDS_PROTOCOL_3_0)
+		return &sge[0];
+	else
+		return &sge[1];
 }
 
 static inline struct ib_sge *
 rds_ib_data_sge(struct rds_ib_connection *ic, struct ib_sge *sge)
 {
-	return &sge[1];
+	if (ic->conn->c_version > RDS_PROTOCOL_3_0)
+		return &sge[1];
+	else
+		return &sge[0];
 }
 
 #endif
diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 0964ac5..1eb0c29 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -101,10 +101,13 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
 	if (event->param.conn.private_data_len >= sizeof(*dp)) {
 		dp = event->param.conn.private_data;
 
-		rds_ib_set_protocol(conn,
+		/* make sure it isn't empty data */
+		if (dp->dp_protocol_major) {
+			rds_ib_set_protocol(conn,
 				RDS_PROTOCOL(dp->dp_protocol_major,
-					dp->dp_protocol_minor));
-		rds_ib_set_flow_control(conn, be32_to_cpu(dp->dp_credit));
+				dp->dp_protocol_minor));
+			rds_ib_set_flow_control(conn, be32_to_cpu(dp->dp_credit));
+		}
 	}
 
 	printk(KERN_NOTICE "RDS/IB: connected to %pI4 version %u.%u%s\n",
diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index 5709bad..28bdcdc 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -555,6 +555,47 @@ u64 rds_ib_piggyb_ack(struct rds_ib_connection *ic)
 	return rds_ib_get_ack(ic);
 }
 
+static struct rds_header *rds_ib_get_header(struct rds_connection *conn,
+					    struct rds_ib_recv_work *recv,
+					    u32 data_len)
+{
+	struct rds_ib_connection *ic = conn->c_transport_data;
+	void *hdr_buff = &ic->i_recv_hdrs[recv - ic->i_recvs];
+	void *addr;
+	u32 misplaced_hdr_bytes;
+
+	/*
+	 * Support header at the front (RDS 3.1+) as well as header-at-end.
+	 *
+	 * Cases:
+	 * 1) header all in header buff (great!)
+	 * 2) header all in data page (copy all to header buff)
+	 * 3) header split across hdr buf + data page
+	 *    (move bit in hdr buff to end before copying other bit from data page)
+	 */
+	if (conn->c_version > RDS_PROTOCOL_3_0 || data_len == RDS_FRAG_SIZE)
+	        return hdr_buff;
+
+	if (data_len <= (RDS_FRAG_SIZE - sizeof(struct rds_header))) {
+		addr = kmap_atomic(recv->r_frag->f_page, KM_SOFTIRQ0);
+		memcpy(hdr_buff,
+		       addr + recv->r_frag->f_offset + data_len,
+		       sizeof(struct rds_header));
+		kunmap_atomic(addr, KM_SOFTIRQ0);
+		return hdr_buff;
+	}
+
+	misplaced_hdr_bytes = (sizeof(struct rds_header) - (RDS_FRAG_SIZE - data_len));
+
+	memmove(hdr_buff + misplaced_hdr_bytes, hdr_buff, misplaced_hdr_bytes);
+
+	addr = kmap_atomic(recv->r_frag->f_page, KM_SOFTIRQ0);
+	memcpy(hdr_buff, addr + recv->r_frag->f_offset + data_len,
+	       sizeof(struct rds_header) - misplaced_hdr_bytes);
+	kunmap_atomic(addr, KM_SOFTIRQ0);
+	return hdr_buff;
+}
+
 /*
  * It's kind of lame that we're copying from the posted receive pages into
  * long-lived bitmaps.  We could have posted the bitmaps and rdma written into
@@ -667,7 +708,7 @@ static void rds_ib_process_recv(struct rds_connection *conn,
 	}
 	byte_len -= sizeof(struct rds_header);
 
-	ihdr = &ic->i_recv_hdrs[recv - ic->i_recvs];
+	ihdr = rds_ib_get_header(conn, recv, byte_len);
 
 	/* Validate the checksum. */
 	if (!rds_message_verify_checksum(ihdr)) {
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 13/15] RDS/IW: Remove page_shift variable from iwarp transport
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

The existing code treated page_shift as a variable, when in fact we
always want to have the fastreg page size be the same as the arch's
page size -- and it is, so this doesn't need to be a variable.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 net/rds/iw.c      |    2 --
 net/rds/iw.h      |    1 -
 net/rds/iw_rdma.c |   28 ++++++++++------------------
 net/rds/iw_send.c |    2 +-
 4 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/net/rds/iw.c b/net/rds/iw.c
index d16e1cb..2e6b495 100644
--- a/net/rds/iw.c
+++ b/net/rds/iw.c
@@ -83,8 +83,6 @@ void rds_iw_add_one(struct ib_device *device)
 	rds_iwdev->max_wrs = dev_attr->max_qp_wr;
 	rds_iwdev->max_sge = min(dev_attr->max_sge, RDS_IW_MAX_SGE);
 
-	rds_iwdev->page_shift = max(PAGE_SHIFT, ffs(dev_attr->page_size_cap) - 1);
-
 	rds_iwdev->dev = device;
 	rds_iwdev->pd = ib_alloc_pd(device);
 	if (IS_ERR(rds_iwdev->pd))
diff --git a/net/rds/iw.h b/net/rds/iw.h
index 0715dde..dd72b62 100644
--- a/net/rds/iw.h
+++ b/net/rds/iw.h
@@ -181,7 +181,6 @@ struct rds_iw_device {
 	struct ib_pd		*pd;
 	struct ib_mr		*mr;
 	struct rds_iw_mr_pool	*mr_pool;
-	int			page_shift;
 	int			max_sge;
 	unsigned int		max_wrs;
 	unsigned int		dma_local_lkey:1;
diff --git a/net/rds/iw_rdma.c b/net/rds/iw_rdma.c
index dcdb37d..de4a1b1 100644
--- a/net/rds/iw_rdma.c
+++ b/net/rds/iw_rdma.c
@@ -263,18 +263,12 @@ static void rds_iw_set_scatterlist(struct rds_iw_scatterlist *sg,
 }
 
 static u64 *rds_iw_map_scatterlist(struct rds_iw_device *rds_iwdev,
-			struct rds_iw_scatterlist *sg,
-			unsigned int dma_page_shift)
+			struct rds_iw_scatterlist *sg)
 {
 	struct ib_device *dev = rds_iwdev->dev;
 	u64 *dma_pages = NULL;
-	u64 dma_mask;
-	unsigned int dma_page_size;
 	int i, j, ret;
 
-	dma_page_size = 1 << dma_page_shift;
-	dma_mask = dma_page_size - 1;
-
 	WARN_ON(sg->dma_len);
 
 	sg->dma_len = ib_dma_map_sg(dev, sg->list, sg->len, DMA_BIDIRECTIONAL);
@@ -295,18 +289,18 @@ static u64 *rds_iw_map_scatterlist(struct rds_iw_device *rds_iwdev,
 		sg->bytes += dma_len;
 
 		end_addr = dma_addr + dma_len;
-		if (dma_addr & dma_mask) {
+		if (dma_addr & PAGE_MASK) {
 			if (i > 0)
 				goto out_unmap;
-			dma_addr &= ~dma_mask;
+			dma_addr &= ~PAGE_MASK;
 		}
-		if (end_addr & dma_mask) {
+		if (end_addr & PAGE_MASK) {
 			if (i < sg->dma_len - 1)
 				goto out_unmap;
-			end_addr = (end_addr + dma_mask) & ~dma_mask;
+			end_addr = (end_addr + PAGE_MASK) & ~PAGE_MASK;
 		}
 
-		sg->dma_npages += (end_addr - dma_addr) >> dma_page_shift;
+		sg->dma_npages += (end_addr - dma_addr) >> PAGE_SHIFT;
 	}
 
 	/* Now gather the dma addrs into one list */
@@ -325,8 +319,8 @@ static u64 *rds_iw_map_scatterlist(struct rds_iw_device *rds_iwdev,
 		u64 end_addr;
 
 		end_addr = dma_addr + dma_len;
-		dma_addr &= ~dma_mask;
-		for (; dma_addr < end_addr; dma_addr += dma_page_size)
+		dma_addr &= ~PAGE_MASK;
+		for (; dma_addr < end_addr; dma_addr += PAGE_SIZE)
 			dma_pages[j++] = dma_addr;
 		BUG_ON(j > sg->dma_npages);
 	}
@@ -727,7 +721,7 @@ static int rds_iw_rdma_build_fastreg(struct rds_iw_mapping *mapping)
 	f_wr.wr.fast_reg.rkey = mapping->m_rkey;
 	f_wr.wr.fast_reg.page_list = ibmr->page_list;
 	f_wr.wr.fast_reg.page_list_len = mapping->m_sg.dma_len;
-	f_wr.wr.fast_reg.page_shift = ibmr->device->page_shift;
+	f_wr.wr.fast_reg.page_shift = PAGE_SHIFT;
 	f_wr.wr.fast_reg.access_flags = IB_ACCESS_LOCAL_WRITE |
 				IB_ACCESS_REMOTE_READ |
 				IB_ACCESS_REMOTE_WRITE;
@@ -780,9 +774,7 @@ static int rds_iw_map_fastreg(struct rds_iw_mr_pool *pool,
 
 	rds_iw_set_scatterlist(&mapping->m_sg, sg, sg_len);
 
-	dma_pages = rds_iw_map_scatterlist(rds_iwdev,
-				&mapping->m_sg,
-				rds_iwdev->page_shift);
+	dma_pages = rds_iw_map_scatterlist(rds_iwdev, &mapping->m_sg);
 	if (IS_ERR(dma_pages)) {
 		ret = PTR_ERR(dma_pages);
 		dma_pages = NULL;
diff --git a/net/rds/iw_send.c b/net/rds/iw_send.c
index 44a6a05..1f5abe3 100644
--- a/net/rds/iw_send.c
+++ b/net/rds/iw_send.c
@@ -779,7 +779,7 @@ static void rds_iw_build_send_fastreg(struct rds_iw_device *rds_iwdev, struct rd
 	send->s_wr.wr.fast_reg.rkey = send->s_mr->rkey;
 	send->s_wr.wr.fast_reg.page_list = send->s_page_list;
 	send->s_wr.wr.fast_reg.page_list_len = nent;
-	send->s_wr.wr.fast_reg.page_shift = rds_iwdev->page_shift;
+	send->s_wr.wr.fast_reg.page_shift = PAGE_SHIFT;
 	send->s_wr.wr.fast_reg.access_flags = IB_ACCESS_REMOTE_WRITE;
 	send->s_wr.wr.fast_reg.iova_start = sg_addr;
 
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 02/15] RDS/IB: Improve RDS protocol version checking
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

RDS on IB uses privdata to do protocol version negotiation. Apparently
the IB stack will return a larger privdata buffer than the struct we were
expecting. Just to be extra-sure, this patch adds some checks in this area.

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

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 605c032..0964ac5 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -98,7 +98,7 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
 	struct ib_qp_attr qp_attr;
 	int err;
 
-	if (event->param.conn.private_data_len) {
+	if (event->param.conn.private_data_len >= sizeof(*dp)) {
 		dp = event->param.conn.private_data;
 
 		rds_ib_set_protocol(conn,
@@ -344,19 +344,32 @@ out:
 	return ret;
 }
 
-static u32 rds_ib_protocol_compatible(const struct rds_ib_connect_private *dp)
+static u32 rds_ib_protocol_compatible(struct rdma_cm_event *event)
 {
+	const struct rds_ib_connect_private *dp = event->param.conn.private_data;
 	u16 common;
 	u32 version = 0;
 
-	/* rdma_cm private data is odd - when there is any private data in the
+	/*
+	 * rdma_cm private data is odd - when there is any private data in the
 	 * request, we will be given a pretty large buffer without telling us the
 	 * original size. The only way to tell the difference is by looking at
 	 * the contents, which are initialized to zero.
 	 * If the protocol version fields aren't set, this is a connection attempt
 	 * from an older version. This could could be 3.0 or 2.0 - we can't tell.
-	 * We really should have changed this for OFED 1.3 :-( */
-	if (dp->dp_protocol_major == 0)
+	 * We really should have changed this for OFED 1.3 :-(
+	 */
+
+	/* Be paranoid. RDS always has privdata */
+	if (!event->param.conn.private_data_len) {
+		printk(KERN_NOTICE "RDS incoming connection has no private data, "
+			"rejecting\n");
+		return 0;
+	}
+
+	/* Even if len is crap *now* I still want to check it. -ASG */
+	if (event->param.conn.private_data_len < sizeof (*dp)
+	    || dp->dp_protocol_major == 0)
 		return RDS_PROTOCOL_3_0;
 
 	common = be16_to_cpu(dp->dp_protocol_minor_mask) & RDS_IB_SUPPORTED_PROTOCOLS;
@@ -388,7 +401,7 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
 	int err, destroy = 1;
 
 	/* Check whether the remote protocol version matches ours. */
-	version = rds_ib_protocol_compatible(dp);
+	version = rds_ib_protocol_compatible(event);
 	if (!version)
 		goto out;
 
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 10/15] RDS/IB: Drop connection when a fatal QP event is received
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

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

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 0ad749c..c2d372f 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -203,9 +203,9 @@ static void rds_ib_qp_event_handler(struct ib_event *event, void *data)
 		rdma_notify(ic->i_cm_id, IB_EVENT_COMM_EST);
 		break;
 	default:
-		printk(KERN_WARNING "RDS/ib: unhandled QP event %u "
-		       "on connection to %pI4\n", event->event,
-		       &conn->c_faddr);
+		rds_ib_conn_error(conn, "RDS/IB: Fatal QP Event %u "
+			"- connection %pI4->%pI4, reconnecting\n",
+			event->event, &conn->c_laddr, &conn->c_faddr);
 		break;
 	}
 }
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 1/3] netxen: fix context deletion sequence
From: Dhananjay Phadke @ 2009-07-18  1:27 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1247880428-4895-1-git-send-email-dhananjay@netxen.com>

o Use D3 reset context deletion for NX2031, it cleans up
  more resources in the firmware.
o Release rx buffers after hardware context has been reset.
o Delete tx context after rx context, some firmware control
  commands are sent on tx context, so it should be the last
  to go.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic.h      |    1 +
 drivers/net/netxen/netxen_nic_ctx.c  |   13 +++++++------
 drivers/net/netxen/netxen_nic_main.c |    2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index e1cdba7..9fa71fa 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -210,6 +210,7 @@
 #define NETXEN_CTX_SIGNATURE	0xdee0
 #define NETXEN_CTX_SIGNATURE_V2	0x0002dee0
 #define NETXEN_CTX_RESET	0xbad0
+#define NETXEN_CTX_D3_RESET	0xacc0
 #define NETXEN_RCV_PRODUCER(ringid)	(ringid)
 
 #define PHAN_PEG_RCV_INITIALIZED	0xff01
diff --git a/drivers/net/netxen/netxen_nic_ctx.c b/drivers/net/netxen/netxen_nic_ctx.c
index 4754f5c..9f8ae47 100644
--- a/drivers/net/netxen/netxen_nic_ctx.c
+++ b/drivers/net/netxen/netxen_nic_ctx.c
@@ -684,10 +684,8 @@ int netxen_alloc_hw_resources(struct netxen_adapter *adapter)
 			goto err_out_free;
 	} else {
 		err = netxen_init_old_ctx(adapter);
-		if (err) {
-			netxen_free_hw_resources(adapter);
-			return err;
-		}
+		if (err)
+			goto err_out_free;
 	}
 
 	return 0;
@@ -708,15 +706,18 @@ void netxen_free_hw_resources(struct netxen_adapter *adapter)
 	int port = adapter->portnum;
 
 	if (adapter->fw_major >= 4) {
-		nx_fw_cmd_destroy_tx_ctx(adapter);
 		nx_fw_cmd_destroy_rx_ctx(adapter);
+		nx_fw_cmd_destroy_tx_ctx(adapter);
 	} else {
 		netxen_api_lock(adapter);
 		NXWR32(adapter, CRB_CTX_SIGNATURE_REG(port),
-				NETXEN_CTX_RESET | port);
+				NETXEN_CTX_D3_RESET | port);
 		netxen_api_unlock(adapter);
 	}
 
+	/* Allow dma queues to drain after context reset */
+	msleep(20);
+
 	recv_ctx = &adapter->recv_ctx;
 
 	if (recv_ctx->hwctx != NULL) {
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 27539dd..9a7c4c8 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -943,8 +943,8 @@ err_out_free_sw:
 static void
 netxen_nic_detach(struct netxen_adapter *adapter)
 {
-	netxen_release_rx_buffers(adapter);
 	netxen_free_hw_resources(adapter);
+	netxen_release_rx_buffers(adapter);
 	netxen_nic_free_irq(adapter);
 	netxen_free_sw_resources(adapter);
 
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH 0/3] netxen bug fixes
From: Dhananjay Phadke @ 2009-07-18  1:27 UTC (permalink / raw)
  To: davem; +Cc: netdev

Dave,

Set of 3 critical bug fixes for 2.6.31, including a fix for
intermittant deadlock on dev close.

Please apply.

Thanks,
	Dhananjay



^ permalink raw reply

* [PATCH 2/3] netxen: fix deadlock on dev close
From: Dhananjay Phadke @ 2009-07-18  1:27 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1247880428-4895-1-git-send-email-dhananjay@netxen.com>

netxen: fix deadlock on dev close

The tx ring accounting fix in commit cb2107be43d2fc5eadec58b92b
("netxen: fix tx ring accounting") introduced intermittent
deadlock when inteface is going down.

This was possibly combined effect of speculative tx pause,
calling netif_tx_lock instead of queue lock and unclean
synchronization with napi which could end up unmasking
interrupt.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic.h      |    2 ++
 drivers/net/netxen/netxen_nic_hw.c   |    9 +++++----
 drivers/net/netxen/netxen_nic_init.c |    5 +++--
 drivers/net/netxen/netxen_nic_main.c |   20 +++++++++++++-------
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 9fa71fa..f86e050 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -774,6 +774,8 @@ struct nx_host_tx_ring {
 	u32 crb_cmd_consumer;
 	u32 num_desc;
 
+	struct netdev_queue *txq;
+
 	struct netxen_cmd_buffer *cmd_buf_arr;
 	struct cmd_desc_type0 *desc_head;
 	dma_addr_t phys_addr;
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index ce3b89d..b9123d4 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -461,13 +461,14 @@ netxen_send_cmd_descs(struct netxen_adapter *adapter,
 	i = 0;
 
 	tx_ring = adapter->tx_ring;
-	netif_tx_lock_bh(adapter->netdev);
+	__netif_tx_lock_bh(tx_ring->txq);
 
 	producer = tx_ring->producer;
 	consumer = tx_ring->sw_consumer;
 
-	if (nr_desc >= find_diff_among(producer, consumer, tx_ring->num_desc)) {
-		netif_tx_unlock_bh(adapter->netdev);
+	if (nr_desc >= netxen_tx_avail(tx_ring)) {
+		netif_tx_stop_queue(tx_ring->txq);
+		__netif_tx_unlock_bh(tx_ring->txq);
 		return -EBUSY;
 	}
 
@@ -490,7 +491,7 @@ netxen_send_cmd_descs(struct netxen_adapter *adapter,
 
 	netxen_nic_update_cmd_producer(adapter, tx_ring);
 
-	netif_tx_unlock_bh(adapter->netdev);
+	__netif_tx_unlock_bh(tx_ring->txq);
 
 	return 0;
 }
diff --git a/drivers/net/netxen/netxen_nic_init.c b/drivers/net/netxen/netxen_nic_init.c
index b899bd5..5d3343e 100644
--- a/drivers/net/netxen/netxen_nic_init.c
+++ b/drivers/net/netxen/netxen_nic_init.c
@@ -214,6 +214,7 @@ int netxen_alloc_sw_resources(struct netxen_adapter *adapter)
 	adapter->tx_ring = tx_ring;
 
 	tx_ring->num_desc = adapter->num_txd;
+	tx_ring->txq = netdev_get_tx_queue(netdev, 0);
 
 	cmd_buf_arr = vmalloc(TX_BUFF_RINGSIZE(tx_ring));
 	if (cmd_buf_arr == NULL) {
@@ -1400,10 +1401,10 @@ int netxen_process_cmd_ring(struct netxen_adapter *adapter)
 		smp_mb();
 
 		if (netif_queue_stopped(netdev) && netif_carrier_ok(netdev)) {
-			netif_tx_lock(netdev);
+			__netif_tx_lock(tx_ring->txq, smp_processor_id());
 			if (netxen_tx_avail(tx_ring) > TX_STOP_THRESH)
 				netif_wake_queue(netdev);
-			netif_tx_unlock(netdev);
+			__netif_tx_unlock(tx_ring->txq);
 		}
 	}
 	/*
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 9a7c4c8..370c52f 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -215,9 +215,9 @@ netxen_napi_disable(struct netxen_adapter *adapter)
 
 	for (ring = 0; ring < adapter->max_sds_rings; ring++) {
 		sds_ring = &recv_ctx->sds_rings[ring];
-		napi_disable(&sds_ring->napi);
 		netxen_nic_disable_int(sds_ring);
-		synchronize_irq(sds_ring->irq);
+		napi_synchronize(&sds_ring->napi);
+		napi_disable(&sds_ring->napi);
 	}
 }
 
@@ -833,11 +833,11 @@ netxen_nic_up(struct netxen_adapter *adapter, struct net_device *netdev)
 
 	adapter->ahw.linkup = 0;
 
-	netxen_napi_enable(adapter);
-
 	if (adapter->max_sds_rings > 1)
 		netxen_config_rss(adapter, 1);
 
+	netxen_napi_enable(adapter);
+
 	if (adapter->capabilities & NX_FW_CAPABILITY_LINK_NOTIFICATION)
 		netxen_linkevent_request(adapter, 1);
 	else
@@ -851,8 +851,9 @@ netxen_nic_up(struct netxen_adapter *adapter, struct net_device *netdev)
 static void
 netxen_nic_down(struct netxen_adapter *adapter, struct net_device *netdev)
 {
+	spin_lock(&adapter->tx_clean_lock);
 	netif_carrier_off(netdev);
-	netif_stop_queue(netdev);
+	netif_tx_disable(netdev);
 
 	if (adapter->stop_port)
 		adapter->stop_port(adapter);
@@ -863,9 +864,10 @@ netxen_nic_down(struct netxen_adapter *adapter, struct net_device *netdev)
 	netxen_napi_disable(adapter);
 
 	netxen_release_tx_buffers(adapter);
+	spin_unlock(&adapter->tx_clean_lock);
 
-	FLUSH_SCHEDULED_WORK();
 	del_timer_sync(&adapter->watchdog_timer);
+	FLUSH_SCHEDULED_WORK();
 }
 
 
@@ -1645,6 +1647,9 @@ static void netxen_tx_timeout_task(struct work_struct *work)
 	struct netxen_adapter *adapter =
 		container_of(work, struct netxen_adapter, tx_timeout_task);
 
+	if (!netif_running(adapter->netdev))
+		return;
+
 	printk(KERN_ERR "%s %s: transmit timeout, resetting.\n",
 	       netxen_nic_driver_name, adapter->netdev->name);
 
@@ -1757,7 +1762,8 @@ static int netxen_nic_poll(struct napi_struct *napi, int budget)
 
 	if ((work_done < budget) && tx_complete) {
 		napi_complete(&sds_ring->napi);
-		netxen_nic_enable_int(sds_ring);
+		if (netif_running(adapter->netdev))
+			netxen_nic_enable_int(sds_ring);
 	}
 
 	return work_done;
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH 3/3] netxen: fix thermal check and shutdown
From: Dhananjay Phadke @ 2009-07-18  1:27 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1247880428-4895-1-git-send-email-dhananjay@netxen.com>

Check temperature for all PCI functions, that can allow
graceful shutdown of all interfaces on the overheated card.

Old code was only monitoring temperature for function 0 only.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic_main.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 370c52f..637ac8b 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -1535,10 +1535,12 @@ static int netxen_nic_check_temp(struct netxen_adapter *adapter)
 		printk(KERN_ALERT
 		       "%s: Device temperature %d degrees C exceeds"
 		       " maximum allowed. Hardware has been shut down.\n",
-		       netxen_nic_driver_name, temp_val);
+		       netdev->name, temp_val);
+
+		netif_device_detach(netdev);
+		netxen_nic_down(adapter, netdev);
+		netxen_nic_detach(adapter);
 
-		netif_carrier_off(netdev);
-		netif_stop_queue(netdev);
 		rv = 1;
 	} else if (temp_state == NX_TEMP_WARN) {
 		if (adapter->temp == NX_TEMP_NORMAL) {
@@ -1546,13 +1548,13 @@ static int netxen_nic_check_temp(struct netxen_adapter *adapter)
 			       "%s: Device temperature %d degrees C "
 			       "exceeds operating range."
 			       " Immediate action needed.\n",
-			       netxen_nic_driver_name, temp_val);
+			       netdev->name, temp_val);
 		}
 	} else {
 		if (adapter->temp == NX_TEMP_WARN) {
 			printk(KERN_INFO
 			       "%s: Device temperature is now %d degrees C"
-			       " in normal range.\n", netxen_nic_driver_name,
+			       " in normal range.\n", netdev->name,
 			       temp_val);
 		}
 	}
@@ -1625,7 +1627,7 @@ void netxen_watchdog_task(struct work_struct *work)
 	struct netxen_adapter *adapter =
 		container_of(work, struct netxen_adapter, watchdog_task);
 
-	if ((adapter->portnum  == 0) && netxen_nic_check_temp(adapter))
+	if (netxen_nic_check_temp(adapter))
 		return;
 
 	if (!adapter->has_link_events)
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH 12/15] RDS/IB: Always use PAGE_SIZE for FMR page size
From: Andy Grover @ 2009-07-17 23:13 UTC (permalink / raw)
  To: netdev; +Cc: rds-devel
In-Reply-To: <1247872416-17834-1-git-send-email-andy.grover@oracle.com>

While FMRs allow significant flexibility in what size of pages they can use,
we really just want FMR pages to match CPU page size. Roland says we can
count on this always being supported, so this simplifies things.

Signed-off-by: Andy Grover <andy.grover@oracle.com>
---
 net/rds/ib.c      |    3 ---
 net/rds/ib.h      |    3 ---
 net/rds/ib_rdma.c |   12 ++++++------
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/net/rds/ib.c b/net/rds/ib.c
index 27abdd3..868559a 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -85,9 +85,6 @@ void rds_ib_add_one(struct ib_device *device)
 	rds_ibdev->max_wrs = dev_attr->max_qp_wr;
 	rds_ibdev->max_sge = min(dev_attr->max_sge, RDS_IB_MAX_SGE);
 
-	rds_ibdev->fmr_page_shift = max(9, ffs(dev_attr->page_size_cap) - 1);
-	rds_ibdev->fmr_page_size  = 1 << rds_ibdev->fmr_page_shift;
-	rds_ibdev->fmr_page_mask  = ~((u64) rds_ibdev->fmr_page_size - 1);
 	rds_ibdev->fmr_max_remaps = dev_attr->max_map_per_fmr?: 32;
 	rds_ibdev->max_fmrs = dev_attr->max_fmr ?
 			min_t(unsigned int, dev_attr->max_fmr, fmr_pool_size) :
diff --git a/net/rds/ib.h b/net/rds/ib.h
index c0de7af..1378b85 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -159,9 +159,6 @@ struct rds_ib_device {
 	struct ib_pd		*pd;
 	struct ib_mr		*mr;
 	struct rds_ib_mr_pool	*mr_pool;
-	int			fmr_page_shift;
-	int			fmr_page_size;
-	u64			fmr_page_mask;
 	unsigned int		fmr_max_remaps;
 	unsigned int		max_fmrs;
 	int			max_sge;
diff --git a/net/rds/ib_rdma.c b/net/rds/ib_rdma.c
index 81033af..ef3ab5b 100644
--- a/net/rds/ib_rdma.c
+++ b/net/rds/ib_rdma.c
@@ -211,7 +211,7 @@ struct rds_ib_mr_pool *rds_ib_create_mr_pool(struct rds_ib_device *rds_ibdev)
 
 	pool->fmr_attr.max_pages = fmr_message_size;
 	pool->fmr_attr.max_maps = rds_ibdev->fmr_max_remaps;
-	pool->fmr_attr.page_shift = rds_ibdev->fmr_page_shift;
+	pool->fmr_attr.page_shift = PAGE_SHIFT;
 	pool->max_free_pinned = rds_ibdev->max_fmrs * fmr_message_size / 4;
 
 	/* We never allow more than max_items MRs to be allocated.
@@ -349,13 +349,13 @@ static int rds_ib_map_fmr(struct rds_ib_device *rds_ibdev, struct rds_ib_mr *ibm
 		unsigned int dma_len = ib_sg_dma_len(dev, &scat[i]);
 		u64 dma_addr = ib_sg_dma_address(dev, &scat[i]);
 
-		if (dma_addr & ~rds_ibdev->fmr_page_mask) {
+		if (dma_addr & ~PAGE_MASK) {
 			if (i > 0)
 				return -EINVAL;
 			else
 				++page_cnt;
 		}
-		if ((dma_addr + dma_len) & ~rds_ibdev->fmr_page_mask) {
+		if ((dma_addr + dma_len) & ~PAGE_MASK) {
 			if (i < sg_dma_len - 1)
 				return -EINVAL;
 			else
@@ -365,7 +365,7 @@ static int rds_ib_map_fmr(struct rds_ib_device *rds_ibdev, struct rds_ib_mr *ibm
 		len += dma_len;
 	}
 
-	page_cnt += len >> rds_ibdev->fmr_page_shift;
+	page_cnt += len >> PAGE_SHIFT;
 	if (page_cnt > fmr_message_size)
 		return -EINVAL;
 
@@ -378,9 +378,9 @@ static int rds_ib_map_fmr(struct rds_ib_device *rds_ibdev, struct rds_ib_mr *ibm
 		unsigned int dma_len = ib_sg_dma_len(dev, &scat[i]);
 		u64 dma_addr = ib_sg_dma_address(dev, &scat[i]);
 
-		for (j = 0; j < dma_len; j += rds_ibdev->fmr_page_size)
+		for (j = 0; j < dma_len; j += PAGE_SIZE)
 			dma_pages[page_cnt++] =
-				(dma_addr & rds_ibdev->fmr_page_mask) + j;
+				(dma_addr & PAGE_MASK) + j;
 	}
 
 	ret = ib_map_phys_fmr(ibmr->fmr,
-- 
1.6.0.4


^ permalink raw reply related

* Re: [PATCH v2] Receive Packet Steering
From: David Miller @ 2009-07-18  3:54 UTC (permalink / raw)
  To: therbert; +Cc: netdev
In-Reply-To: <65634d660907171259l2a3a31fel549176274201bd66@mail.gmail.com>

From: Tom Herbert <therbert@google.com>
Date: Fri, 17 Jul 2009 12:59:31 -0700

> eth_type_trans does not need to even be called assuming that the
> packet type information can be inferred from the RX descriptor.

Great assumption :-)

We investigated doing that in the past but it's racey and can't be
made to work in all situations.

^ permalink raw reply

* Re: [PATCH] TCP: Add comments to (near) all functions in tcp_output.c
From: Ilpo Järvinen @ 2009-07-18  6:38 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Netdev, David Miller
In-Reply-To: <20090717204004.GA516@basil.fritz.box>

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

On Fri, 17 Jul 2009, Andi Kleen wrote:

> TCP: Add comments to (near) all functions in tcp_output.c
> 
> While looking for something else I spent some time adding
> one liner comments to the tcp_output.c functions that
> didn't have any. That makes the comments more consistent.
> 
> I hope I documented everything right.
> 
> No code changes.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  net/ipv4/tcp_output.c |   49 ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 44 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
> ===================================================================
> --- linux-2.6.31-rc3-ak.orig/net/ipv4/tcp_output.c
> +++ linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
> @@ -59,6 +59,8 @@ int sysctl_tcp_base_mss __read_mostly =
>  /* By default, RFC2861 behavior.  */
>  int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
>  
> +/* Account for new data that has been sent to the network.
> + */
>  static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -142,6 +144,8 @@ static void tcp_cwnd_restart(struct sock
>  	tp->snd_cwnd_used = 0;
>  }
>  
> +/* Congestion state accounting after a packet has been sent.
> + */
>  static void tcp_event_data_sent(struct tcp_sock *tp,
>  				struct sk_buff *skb, struct sock *sk)
>  {
> @@ -276,6 +280,8 @@ static u16 tcp_select_window(struct sock
>  	return new_win;
>  }
>  
> +/* Packet ECN state for a SYN-ACK
> + */
>  static inline void TCP_ECN_send_synack(struct tcp_sock *tp, struct sk_buff *skb)
>  {
>  	TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_CWR;
> @@ -283,6 +289,8 @@ static inline void TCP_ECN_send_synack(s
>  		TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_ECE;
>  }
>  
> +/* Packet ECN state for a SYN.
> + */
>  static inline void TCP_ECN_send_syn(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -301,6 +309,9 @@ TCP_ECN_make_synack(struct request_sock
>  		th->ece = 1;
>  }
>  
> +/* Set up ECN state for a packet on a ESTABLISHED socket that is about to
> + * be sent.
> + */
>  static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb,
>  				int tcp_header_len)
>  {
> @@ -362,7 +373,9 @@ struct tcp_out_options {
>  	__u32 tsval, tsecr;	/* need to include OPTION_TS */
>  };
>  
> -/* Beware: Something in the Internet is very sensitive to the ordering of
> +/* Write previously computed TCP options to the packet.
> + *
> + * Beware: Something in the Internet is very sensitive to the ordering of
>   * TCP options, we learned this through the hard way, so be careful here.
>   * Luckily we can at least blame others for their non-compliance but from
>   * inter-operatibility perspective it seems that we're somewhat stuck with
> @@ -445,6 +458,8 @@ static void tcp_options_write(__be32 *pt
>  	}
>  }
>  
> +/* Compute TCP options for SYN packets and store them in the socket.
> + */

These three (this and two below) calculate stuff not into socket but into 
a tcp_out_options struct in stack, and besides that they also find out 
the space that is needed for those options (the returned int).

>  static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb,
>  				struct tcp_out_options *opts,
>  				struct tcp_md5sig_key **md5) {
> @@ -493,6 +508,8 @@ static unsigned tcp_syn_options(struct s
>  	return size;
>  }
>  
> +/* Set up TCP options for SYN-ACKs and store them in the socket.
> + */
>  static unsigned tcp_synack_options(struct sock *sk,
>  				   struct request_sock *req,
>  				   unsigned mss, struct sk_buff *skb,
> @@ -541,6 +558,8 @@ static unsigned tcp_synack_options(struc
>  	return size;
>  }
>  
> +/* Compute TCP options for ESTABLISHED sockets and store them in the socket.
> + */
>  static unsigned tcp_established_options(struct sock *sk, struct sk_buff *skb,
>  					struct tcp_out_options *opts,
>  					struct tcp_md5sig_key **md5) {
> @@ -705,7 +724,7 @@ static int tcp_transmit_skb(struct sock
>  	return net_xmit_eval(err);
>  }
>  
> -/* This routine just queue's the buffer
> +/* This routine just queues the buffer
>   *
>   * NOTE: probe0 timer is not checked, do not forget tcp_push_pending_frames,
>   * otherwise socket can stall.
> @@ -909,6 +928,8 @@ static void __pskb_trim_head(struct sk_b
>  	skb->len = skb->data_len;
>  }
>  
> +/* Remove acked data from a packet in the transmit queue.
> + */
>  int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
>  {
>  	if (skb_cloned(skb) && pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
> @@ -937,7 +958,9 @@ int tcp_trim_head(struct sock *sk, struc
>  	return 0;
>  }
>  
> -/* Not accounting for SACKs here. */
> +/* Calculate MSS
> + * Not accounting for SACKs here.
> + */
>  int tcp_mtu_to_mss(struct sock *sk, int pmtu)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -1143,7 +1166,8 @@ static inline unsigned int tcp_cwnd_test
>  	return 0;
>  }
>  
> -/* This must be invoked the first time we consider transmitting
> +/* Intialize TSO state.

... of skb. ?

Not some some socket or whatever global state.

> + * This must be invoked the first time we consider transmitting
>   * SKB onto the wire.
>   */
>  static int tcp_init_tso_segs(struct sock *sk, struct sk_buff *skb,
> @@ -1242,6 +1266,8 @@ static unsigned int tcp_snd_test(struct
>  	return cwnd_quota;
>  }
>  
> +/* Test if sending is allowed right now.
> + */
>  int tcp_may_send_now(struct sock *sk)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -1378,6 +1404,9 @@ send_now:
>  }
>  
>  /* Create a new MTU probe if we are ready.
> + * MTU probe is trying to increase the path MTU again after PMTU
> + * discovery initially shrunk the PMTU. This discovers routing
> + * changes.

I don't particularly like that "initially shrunk" part here. Honestly I 
don't even know what that refers to :-) and mtu probe may well work from 
1500 upwards too if your path is able... I mean that no shrunking 
occurred in that scenario?!?

>   * Returns 0 if we should wait to probe (no cwnd available),
>   *         1 if a probe was sent,
>   *         -1 otherwise
> @@ -1808,6 +1837,9 @@ static int tcp_can_collapse(struct sock
>  	return 1;
>  }
>  
> +/* Collapse packets in the retransmit queue to make them
> + * fit into the MSS/MTU and minimize packets on the wire.

small packets? collapsed up to MSS/MTU?

> + */
>  static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
>  				     int space)
>  {
> @@ -1957,6 +1989,9 @@ int tcp_retransmit_skb(struct sock *sk,
>  	return err;
>  }
>  
> +/* Check if we forward retransmits are possible in the current
> + * window/congestion state.
> + */
>  static int tcp_can_forward_retransmit(struct sock *sk)
>  {
>  	const struct inet_connection_sock *icsk = inet_csk(sk);
> @@ -2145,7 +2180,8 @@ void tcp_send_active_reset(struct sock *
>  	TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
>  }
>  
> -/* WARNING: This routine must only be called when we have already sent
> +/* Send a crossed SYN-ACK during socket establishment.
> + * WARNING: This routine must only be called when we have already sent
>   * a SYN packet that crossed the incoming SYN that caused this routine
>   * to get called. If this assumption fails then the initial rcv_wnd
>   * and rcv_wscale values will not be correct.
> @@ -2493,6 +2529,9 @@ static int tcp_xmit_probe_skb(struct soc
>  	return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC);
>  }
>  
> +/* Initiate keepalive or window probe from timer.
> + * Retransmits the data on the head of the retransmit queue.

skb = tcp_send_head(sk)? ...That's not head of the retransmit queue but 
the first unsent segment. ...And it may send just a probe packet(s) if no 
data is available.

> + */
>  int tcp_write_wakeup(struct sock *sk)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);

Other than those mentioned,

Acked-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>


-- 
 i.

^ permalink raw reply

* Re: Achieved 10Gbit/s bidirectional routing
From: Jesper Dangaard Brouer @ 2009-07-18  7:14 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Bill Fink, netdev@vger.kernel.org, David S. Miller, Robert Olsson,
	Waskiewicz Jr, Peter P, Ronciak, John, jesse.brandeburg,
	Stephen Hemminger, Linux Kernel Mailing List
In-Reply-To: <20090717203546.GA31259@1wt.eu>

On Fri, 2009-07-17 at 22:35 +0200, Willy Tarreau wrote:
> On Thu, Jul 16, 2009 at 11:38:27AM -0400, Bill Fink wrote:
> > On Thu, 16 Jul 2009, Jesper Dangaard Brouer wrote:
> > 
> > > On Wed, 2009-07-15 at 23:22 -0400, Bill Fink wrote:
> > > > On Wed, 15 Jul 2009, Jesper Dangaard Brouer wrote:
> > > > 
> > > > > I'm giving a talk at LinuxCon, about 10Gbit/s routing on standard
> > > > > hardware running Linux.
> > > > > 
> > > > >   http://linuxcon.linuxfoundation.org/meetings/1585
> > > > >   https://events.linuxfoundation.org/lc09o17
> > > > > 
> > > > > I'm getting some really good 10Gbit/s bidirectional routing results
> > > > > with Intels latest 82599 chip. (I got two pre-release engineering
> > > > > samples directly from Intel, thanks Peter)
> > > > > 
> > > > > Using a Core i7-920, and tuning the memory according to the RAMs
> > > > > X.M.P. settings DDR3-1600MHz, notice this also increases the QPI to
> > > > > 6.4GT/s.  (Motherboard P6T6 WS revolution)
> > > > > 
> > > > > With big 1514 bytes packets, I can basically do 10Gbit/s wirespeed
> > > > > bidirectional routing.
> > > > > 
> > > > > Notice bidirectional routing means that we actually has to move approx
> > > > > 40Gbit/s through memory and in-and-out of the interfaces.
> > > > > 
> > > > > Formatted quick view using 'ifstat -b'
> > > > > 
> > > > >   eth31-in   eth31-out   eth32-in  eth32-out
> > > > >     9.57  +    9.52  +     9.51 +     9.60  = 38.20 Gbit/s
> > > > >     9.60  +    9.55  +     9.52 +     9.62  = 38.29 Gbit/s
> > > > >     9.61  +    9.53  +     9.52 +     9.62  = 38.28 Gbit/s
> > > > >     9.61  +    9.53  +     9.54 +     9.62  = 38.30 Gbit/s
> > > > > 
> > > > > [Adding an extra NIC]
> > > > > 
> > > > > Another observation is that I'm hitting some kind of bottleneck on the
> > > > > PCI-express switch.  Adding an extra NIC in a PCIe slot connected to
> > > > > the same PCIe switch, does not scale beyond 40Gbit/s collective
> > > > > throughput.
> > > 
> > > Correcting my self, according to Bill's info below.
> > > 
> > > It does not scale when adding an extra NIC to the same NVIDIA NF200 PCIe
> > > switch chip (reason explained below by Bill)
> > > 
> > >  
> > > > > But, I happened to have a special motherboard ASUS P6T6 WS revolution,
> > > > > which has an additional PCIe switch chip NVIDIA's NF200.
> > > > > 
> > > > > Connecting two dual port 10GbE NICs via two different PCI-express
> > > > > switch chips, makes things scale again!  I have achieved a collective
> > > > > throughput of 66.25 Gbit/s.  This results is also influenced by my
> > > > > pktgen machines cannot keep up, and I'm getting closer to the memory
> > > > > bandwidth limits.
> > > > > 
> > > > > FYI: I found a really good reference explaining the PCI-express
> > > > > architecture, written by Intel:
> > > > > 
> > > > >  http://download.intel.com/design/intarch/papers/321071.pdf
> > > > > 
> > > > > I'm not sure how to explain the PCI-express chip bottleneck I'm
> > > > > seeing, but my guess is that I'm limited by the number of outstanding
> > > > > packets/DMA-transfers and the latency for the DMA operations.
> > > > > 
> > > > > Does any one have datasheets on the X58 and NVIDIA's NF200 PCI-express
> > > > > chips, that can tell me the number of outstanding transfers they
> > > > > support?
> > > > 
> > > > We've achieved 70 Gbps aggregate unidirectional TCP performance from
> > > > one P6T6 based system to another.  We figured out in our case that
> > > > we were being limited by the interconnect between the Intel X58 and
> > > > Nvidia N200 chips.  The first 2 PCIe 2.0 slots are directly off the
> > > > Intel X58 and get the full 40 Gbps throughput from the dual-port
> > > > Myricom 10-GigE NICs we have installed in them.  But the other
> > > > 3 PCIe 2.0 slots are on the Nvidia N200 chip, and I discovered
> > > > through googling that the link between the X58 and N200 chips
> > > > only operates at PCIe x16 _1.0_ speed, which limits the possible
> > > > aggregate throughput of the last 3 PCIe 2.0 slots to only 32 Gbps.
> > > 
> > > This definitly explains the bottlenecks I have seen! Thanks!
> > > 
> > > Yes, it seems to scale when installing the two NICs in the first two
> > > slots, both connected to the X58.  If overclocking the RAM and CPU a
> > > bit, I can match my pktgen machines speed which gives a collective
> > > throughput of 67.95 Gbit/s.
> > > 
> > >    eth33          eth34          eth31         eth32
> > >  in     out     in     out     in    out     in    out 
> > > 7.54 + 9.58  + 9.56 + 7.56  + 7.33 + 9.53 + 9.50 + 7.35  = 67.95 Gbit/s
> > > 
> > > Now I just need a faster generator machine, to find the next bottleneck ;-)
> > > 
> > > 
> > > > This was clearly seen in our nuttcp testing:
> > > > 
> > > > [root@i7raid-1 ~]# ./nuttcp-6.2.6 -In2 -xc0/0 -p5001 192.168.1.11 & ./nuttcp-6.2.6 -In3 -xc0/0 -p5002 192.168.2.11 & ./nuttcp-6.2.6 -In4 -xc1/1 -p5003 192.168.3.11 & ./nuttcp-6.2.6 -In5 -xc1/1 -p5004 192.168.4.11 & ./nuttcp-6.2.6 -In6 -xc2/2 -p5005 192.168.5.11 & ./nuttcp-6.2.6 -In7 -xc2/2 -p5006 192.168.6.11 & ./nuttcp-6.2.6 -In8 -xc3/3 -p5007 192.168.7.11 & ./nuttcp-6.2.6 -In9 -xc3/3 -p5008 192.168.8.11
> > > > n2: 11505.2648 MB /  10.09 sec = 9566.2298 Mbps 37 %TX 55 %RX 0 retrans 0.10 msRTT
> > > > n3: 11727.4489 MB /  10.02 sec = 9815.7570 Mbps 39 %TX 44 %RX 0 retrans 0.10 msRTT
> > > > n4: 11770.1250 MB /  10.07 sec = 9803.9901 Mbps 39 %TX 51 %RX 0 retrans 0.10 msRTT
> > > > n5: 11837.9320 MB /  10.05 sec = 9876.5725 Mbps 39 %TX 47 %RX 0 retrans 0.10 msRTT
> > > > n6:  9096.8125 MB /  10.09 sec = 7559.3310 Mbps 30 %TX 32 %RX 0 retrans 0.10 msRTT
> > > > n7:  9100.1211 MB /  10.10 sec = 7559.7790 Mbps 30 %TX 44 %RX 0 retrans 0.10 msRTT
> > > > n8:  9095.6179 MB /  10.10 sec = 7557.9983 Mbps 31 %TX 33 %RX 0 retrans 0.10 msRTT
> > > > n9:  9075.5472 MB /  10.08 sec = 7551.0234 Mbps 31 %TX 33 %RX 0 retrans 0.11 msRTT
> > > > 
> > > > This used 4 dual-port Myricom 10-GigE NICs.  We also tested with
> > > > a fifth dual-port 10-GigE NIC, but the aggregate throughput stayed
> > > > at about 70 Gbps, due to the performance bottleneck between the
> > > > X58 and N200 chips.
> > > 
> > > This is also very excellent results!
> > > 
> > > Thanks a lot Bill !!!
> > 
> > We also achieved nearly 80 Gbps in bidirectional TCP tests (40 Gbps
> > simultaneously in each direction):
> > 
> > [root@i7raid-1 ~]# ./nuttcp-6.2.6 -In2 -xc0/0 -p5001 192.168.1.11 & ./nuttcp-6.2.6 -In3 -r -xc0/0 -p5002 192.168.2.11 & ./nuttcp-6.2.6 -In4 -xc1/1 -p5003 192.168.3.11 & ./nuttcp-6.2.6 -In5 -r -xc1/1 -p5004 192.168.4.11 & ./nuttcp-6.2.6 -In6 -xc2/2 -p5005 192.168.5.11 & ./nuttcp-6.2.6 -In7 -r -xc2/2 -p5006 192.168.6.11 & ./nuttcp-6.2.6 -In8 -xc3/3 -p5007 192.168.7.11 & ./nuttcp-6.2.6 -In9 -r -xc3/3 -p5008 192.168.8.11                                    
> > n2: 11542.6250 MB /  10.07 sec = 9619.9920 Mbps 44 %TX 51 %RX 0 retrans 0.12 msRTT                                                                      
> > n3: 11543.7143 MB /  10.06 sec = 9622.2153 Mbps 41 %TX 49 %RX 0 retrans 0.15 msRTT                                                   
> > n4: 11622.8125 MB /  10.05 sec = 9701.0296 Mbps 43 %TX 51 %RX 0 retrans 0.10 msRTT                                                                      
> > n5: 11523.6875 MB /  10.03 sec = 9638.8883 Mbps 43 %TX 50 %RX 0 retrans 0.15 msRTT                                                                      
> > n6: 11608.0141 MB /  10.04 sec = 9695.7388 Mbps 43 %TX 50 %RX 0 retrans 0.10 msRTT                                                                      
> > n7: 11580.1250 MB /  10.04 sec = 9679.3910 Mbps 43 %TX 50 %RX 0 retrans 0.13 msRTT                                                                      
> > n8: 11608.0000 MB /  10.06 sec = 9678.7596 Mbps 42 %TX 50 %RX 0 retrans 0.10 msRTT                                                                      
> > n9: 11553.3750 MB /  10.05 sec = 9643.7296 Mbps 45 %TX 50 %RX 0 retrans 0.11 msRTT                                                                      
> > 
> > This was using 2 dual-port 10-GigE NICs in the first two PCIe 2.0 slots.
> > We are using an Intel i7 965 quad-core 3.2 GHz Nehalem processor
> > (overclocked to 3.4 GHz) and 2000 MHz DDR3 memory.  Adding an additional
> > dual-port 10-GigE NIC on the Nvidia N200 chip does only marginally
> > better, as it appears we are basically CPU limited at this point for
> > this test (the sum of the TX and RX CPU utilization for each pair of
> > 10-GigE interfaces is about 93%).
> 
> Hey guys, those are really nice numbers. Since TCP splicing appeared in the
> kernel (once we got it fixed), I achieved 10 Gbps of HTTP proxying using
> haproxy with very low CPU usage (about 20% of a Core2Duo 2.66 GHz).

Nice, but I think we have a bug with the measured CPU usage.  Eric
Dumazet did a fix, but also pointed out that in a later mail, at I seem
like it not fixed completely yet...

> Before buying the machines, I had been wandering around with the NICs
> donated by Myricom in order to try to find a machine capable of supporting
> this. My conclusion was that a lot of machines had difficulties getting
> above 3.5, 4.7 and 6.5 Gbps of output traffic (those 3 numbers were always
> the same, depending on the chipsets). There clearly was a bandwidth
> limitation imposed by the chipset.
> 
> So I waited for the X38 and AM780FX chipsets to become available and
> bought 3 machines (1 C2D, 1 AMD X2, 1 AMD X4). Those ones have no problem
> with 10 Gbps of forwarded traffic (20 Gbps of total bus bandwidth), even
> with 1500 bytes frames, but I don't know how high they can go, maybe
> they will saturate slightly above.

My experience is also that the AMDs can easily do 10Gbit/s forwarding,
but doing bidirectional they suffer...


> Unfortunately, I only have 5 NICs in 3 machines and no switch (and CX4
> is hard to find these days), so I'm probably stuck at 10 Gbps max.

We are a fiber company, so I'm using our spare 10G optics, but I'm
limited by our supply of SFP+ currently.

I'll be getting two 6 port 10GbE NIC using PCIe2 x16 82599, in august,
so it will be interesting how high we can go! :-)

> Interestingly, I had the impression that forwarding data with TCP
> splicing costs less CPU than IP forwarding, because the NICs can do
> LRO.
> 
> Also, I know a french service provider who uses haproxy on Core i7
> machines and who has already reached 5 Gbps of sustained traffic
> with recent intel dual-port NICs (though I'm not sure exactly which
> ones). This is with very little CPU usage too, less than 2-3% user
> and 15% system+softirq. On previous machines (quad core xeons), it
> was impossible to go beyond 3 Gbps, it looked like the chipset was
> the limitating factor too (though I don't precisely remember which
> one it was).
> 
> I really blamed the NICs because this guys machine was about 4 times
> more powerful than mine, but apparently it was just a chipset issue.
> 
> I also happen to have a customer who recently received a few Sun NXGE,
> mounted in Sun x2100-m2 using an nvidia chipset which I tested OK at
> 10 Gbps with my myri10GE NICs. I'll try to see if I can run some tests
> there, as Davem once said those NICs are really good too.

The Sun NIU NIC has to use several hardware queues to achieve 10GbE.
Currently using these as generators, and thats one of my limiting
factors.

> All in all, I find it really cool that our beloved OS scales that
> well with the hardware :-)

Yes, its really amazing how well the Linux net stack scales.  I think
the primary thanks for this efford goes to DaveMs multiqueue changes and
Eric Dumazet's tuning.

ps. I'll offline untill tuesday.
-- 
Med venlig hilsen / Best regards
  Jesper Brouer
  ComX Networks A/S
  Linux Network developer
  Cand. Scient Datalog / MSc.
  Author of http://adsl-optimizer.dk
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* iproute2 : unambiguous reading.
From: sebastien @ 2009-07-18  9:43 UTC (permalink / raw)
  To: netdev, netfilter

Hi,

	First, thanks for all job done on that community in particular, and the
others free ones in a more general way.

	I don't know where should be the right place of this remark and put
this in netdev and netfilter vger's mailing list : hope this will be
usefull.



Legacy :

	We, at my house work, have a non-free software which can be reach from
network : client/server architecture. I will call it "network soft".

	For obscure reasons, we have 2 networks and 2 adsl box for this
purpose : let's say that network1 is a 192.168.0.0/24 with an adsl modem
gateway (gateway1 : 192.168.0.1) and network2 is a 192.168.1.0/24 with
another adsl modem gateway (gateway2 : 192.168.1.1).

	Network1 and network2 are Virtualy Laned by an hp switch and acts like
2 physically separed networks : all is ethernet however.

	"Network Soft" is on network1. People who wants to use it from network2
must reach network1 via gateway1 and gateway2, using outside network1
and network2 links. Thoses who are familar with "Fernand Renaud" jokes
will understand the "22 à Asnières" situation.



Changes to the legacy :

	We are not allowed to make an unique network, so I took a PII with two
ethernet cards and put a linux box on that 600 BogoMips !

	Two Ip adress on the PII box : one for network1 (eth0 : 192.168.0.2)
and one for network2 (eth1 : 192.168.1.2).

	I read howtos and other material and found that the maintainer of
Debian didn't compile the routing ease in iptable and says that this is
for iproute2.

	Ok, I read more about this and go on. All I want is that packets from
network2 and destined to "network soft" and packets from "network soft"
and destined to network2 goes across the ethernet cards on the PII box.
All other packets are respectively send to the two gateways : gateway1
for network1 and gateway2 for network2.



_*_ INFORMATION _*_ :

	Saying to the PII box that all traffic from network1 to outside world
uses gateway1 and that all traffic from network2 to outside world uses
gateway2 was really easy and simple. I played with default route on
clients on network1 and network2 and two rules in iproute2 : one for
network1 and another for network2, less than ten minutes !

	I surprisingly spent many hours to join the to networks via the PII
box. All I have to say is that Packets from "network soft" and destined
to network2 must go accross from ethernet1 in network1 to ethernet2 in
network2 and reciprocaly for packets from network2 destined to "network
soft". So, why  that wasted time ? What does I forgot ?


	I re-read many times iproute2 man pages and It didn't work as I
expected. I first make two routing commands :

	ip route from 192.168.0.0/24 to 192.168.1.0/24 via 192.168.1.2 table
network1

	ip route from 192.168.1.0/24 to 192.168.0.0/24 via 192.168.0.2 table
network2

	Which does _not_ work. I supposed that linux was able to see that "from
192.168.0.0/24 to 192.168.1.0/24" concerning all packets, whatever they
came in ethernet1 (network1) or in ethernet2 (network2), adjusting the
route when according to cause 192.168.0.2 is on ethernet1 and
192.168.1.2 is on ethernet2.

	I added dev after the from/to directive but the device was conserved
for the next via argument and ip says that there were no such device so
I added after "via" the name of the concerned interface, and says
something like : 

	ip route from 192.168.0.0/24 to 192.168.1.0/24 via 192.168.1.2 dev eth1
table network1

	ip route from 192.168.1.0/24 to 192.168.0.0/24 via 192.168.0.2 dev eth0
table network2

	Which doesn't worked too. Ip considers that we are talking about the
same device for the two arguments from/to and via !


	This fixed correctly what I wanted :

	ip route from 192.168.0.0/24 to 192.168.1.0/24 dev eth0 via 192.168.1.2
dev eth1 table network1

	ip route from 192.168.1.0/24 to 192.168.0.0/24 dev eth1 via 192.168.0.2
dev eth0 table network2

	When repeating the name of the device, it works : this is not well
documented in man pages or other text.



NOTE :
	One can see that eth0 is the ethernet1 in network1 and eth1 is
ethernet2 in network2.



DISCUSSION :

	It's probably an evidence for you hackers that we must repeat the name
for the device in order to get accross one of it to another one but it
was not an evidence for me.

	My remark goes to be better documented of that way of thinking : what
are your opinions about that ?

See ya.
Best regards.



^ permalink raw reply

* [PATCH] TCP: Add comments to (near) all functions in tcp_output.c v2
From: Andi Kleen @ 2009-07-18 10:31 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: Andi Kleen, Netdev, David Miller
In-Reply-To: <Pine.LNX.4.64.0907180919270.31457@melkinkari.cs.Helsinki.FI>


Here's a updated version incorporating feedback.
-Andi

---

TCP: Add comments to (near) all functions in tcp_output.c v2

While looking for something else I spent some time adding
one liner comments to the tcp_output.c functions that
didn't have any. That makes the comments more consistent.

I hope I documented everything right.

No code changes.

v2: Incorporated feedback from Ilpo.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

---
 net/ipv4/tcp_output.c |   51 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 5 deletions(-)

Index: linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
===================================================================
--- linux-2.6.31-rc3-ak.orig/net/ipv4/tcp_output.c
+++ linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
@@ -59,6 +59,8 @@ int sysctl_tcp_base_mss __read_mostly =
 /* By default, RFC2861 behavior.  */
 int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
 
+/* Account for new data that has been sent to the network.
+ */
 static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -142,6 +144,8 @@ static void tcp_cwnd_restart(struct sock
 	tp->snd_cwnd_used = 0;
 }
 
+/* Congestion state accounting after a packet has been sent.
+ */
 static void tcp_event_data_sent(struct tcp_sock *tp,
 				struct sk_buff *skb, struct sock *sk)
 {
@@ -276,6 +280,8 @@ static u16 tcp_select_window(struct sock
 	return new_win;
 }
 
+/* Packet ECN state for a SYN-ACK
+ */
 static inline void TCP_ECN_send_synack(struct tcp_sock *tp, struct sk_buff *skb)
 {
 	TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_CWR;
@@ -283,6 +289,8 @@ static inline void TCP_ECN_send_synack(s
 		TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_ECE;
 }
 
+/* Packet ECN state for a SYN.
+ */
 static inline void TCP_ECN_send_syn(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -301,6 +309,9 @@ TCP_ECN_make_synack(struct request_sock
 		th->ece = 1;
 }
 
+/* Set up ECN state for a packet on a ESTABLISHED socket that is about to
+ * be sent.
+ */
 static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb,
 				int tcp_header_len)
 {
@@ -362,7 +373,9 @@ struct tcp_out_options {
 	__u32 tsval, tsecr;	/* need to include OPTION_TS */
 };
 
-/* Beware: Something in the Internet is very sensitive to the ordering of
+/* Write previously computed TCP options to the packet.
+ *
+ * Beware: Something in the Internet is very sensitive to the ordering of
  * TCP options, we learned this through the hard way, so be careful here.
  * Luckily we can at least blame others for their non-compliance but from
  * inter-operatibility perspective it seems that we're somewhat stuck with
@@ -445,6 +458,9 @@ static void tcp_options_write(__be32 *pt
 	}
 }
 
+/* Compute TCP options for SYN packets. This is not the final
+ * network wire format yet.
+ */
 static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb,
 				struct tcp_out_options *opts,
 				struct tcp_md5sig_key **md5) {
@@ -493,6 +509,8 @@ static unsigned tcp_syn_options(struct s
 	return size;
 }
 
+/* Set up TCP options for SYN-ACKs.
+ */
 static unsigned tcp_synack_options(struct sock *sk,
 				   struct request_sock *req,
 				   unsigned mss, struct sk_buff *skb,
@@ -541,6 +559,9 @@ static unsigned tcp_synack_options(struc
 	return size;
 }
 
+/* Compute TCP options for ESTABLISHED sockets. This is not the
+ * final wire format yet.
+ */
 static unsigned tcp_established_options(struct sock *sk, struct sk_buff *skb,
 					struct tcp_out_options *opts,
 					struct tcp_md5sig_key **md5) {
@@ -705,7 +726,7 @@ static int tcp_transmit_skb(struct sock
 	return net_xmit_eval(err);
 }
 
-/* This routine just queue's the buffer
+/* This routine just queues the buffer
  *
  * NOTE: probe0 timer is not checked, do not forget tcp_push_pending_frames,
  * otherwise socket can stall.
@@ -909,6 +930,8 @@ static void __pskb_trim_head(struct sk_b
 	skb->len = skb->data_len;
 }
 
+/* Remove acked data from a packet in the transmit queue.
+ */
 int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
 {
 	if (skb_cloned(skb) && pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
@@ -937,7 +960,9 @@ int tcp_trim_head(struct sock *sk, struc
 	return 0;
 }
 
-/* Not accounting for SACKs here. */
+/* Calculate MSS
+ * Not accounting for SACKs here.
+ */
 int tcp_mtu_to_mss(struct sock *sk, int pmtu)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -1143,7 +1168,8 @@ static inline unsigned int tcp_cwnd_test
 	return 0;
 }
 
-/* This must be invoked the first time we consider transmitting
+/* Intialize TSO state of a skb.
+ * This must be invoked the first time we consider transmitting
  * SKB onto the wire.
  */
 static int tcp_init_tso_segs(struct sock *sk, struct sk_buff *skb,
@@ -1242,6 +1268,8 @@ static unsigned int tcp_snd_test(struct
 	return cwnd_quota;
 }
 
+/* Test if sending is allowed right now.
+ */
 int tcp_may_send_now(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -1378,6 +1406,10 @@ send_now:
 }
 
 /* Create a new MTU probe if we are ready.
+ * MTU probe is regularly attempting to increase the path MTU by
+ * deliberately sending larger packets.  This discovers routing
+ * changes resulting in larger path MTUs.
+ *
  * Returns 0 if we should wait to probe (no cwnd available),
  *         1 if a probe was sent,
  *         -1 otherwise
@@ -1808,6 +1840,9 @@ static int tcp_can_collapse(struct sock
 	return 1;
 }
 
+/* Collapse packets in the retransmit queue to make to create
+ * less packets on the wire. This is only done on retransmission.
+ */
 static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
 				     int space)
 {
@@ -1957,6 +1992,9 @@ int tcp_retransmit_skb(struct sock *sk,
 	return err;
 }
 
+/* Check if we forward retransmits are possible in the current
+ * window/congestion state.
+ */
 static int tcp_can_forward_retransmit(struct sock *sk)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -2145,7 +2183,8 @@ void tcp_send_active_reset(struct sock *
 	TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
 }
 
-/* WARNING: This routine must only be called when we have already sent
+/* Send a crossed SYN-ACK during socket establishment.
+ * WARNING: This routine must only be called when we have already sent
  * a SYN packet that crossed the incoming SYN that caused this routine
  * to get called. If this assumption fails then the initial rcv_wnd
  * and rcv_wscale values will not be correct.
@@ -2493,6 +2532,8 @@ static int tcp_xmit_probe_skb(struct soc
 	return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC);
 }
 
+/* Initiate keepalive or window probe from timer.
+ */
 int tcp_write_wakeup(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);

^ permalink raw reply

* Re: [PATCH] check spinlock_t/rwlock_t argument type on non-SMP builds
From: Ingo Molnar @ 2009-07-18 12:14 UTC (permalink / raw)
  To: Dave; +Cc: Peter Zijlstra, Andrew Morton, linux-kernel, netdev
In-Reply-To: <4A4E55A9.7090001@gmail.com>


* Dave <kilroyd@googlemail.com> wrote:

> Ingo Molnar wrote:
> > * David Kilroy <kilroyd@googlemail.com> wrote:
> > 
> >> When writing code for UP without CONFIG_DEBUG_SPINLOCK it's easy 
> >> to get the first argument to the spinlock/rwlock functions wrong. 
> >> This is because the parameter is not actually used in this 
> >> configuration.
> >>
> >> Typically you will only find out it's wrong
> >>  * by rebuilding with CONFIG_SMP or CONFIG_DEBUG_SPINLOCK
> >>  * after you've submitted your beautiful patch series.
> >>
> >> The first means a long wait, and the latter is a bit late.
> >>
> >> Add typechecking on the first argument of these macro functions. 
> >> Note that since the typecheck now references the variable, the 
> >> explicit read is redundant and can be removed.
> >>
> >> This change causes compiler warnings in net/ipv4/route.c, as this 
> >> passes NULL as the first argument in the UP configuration. Simply 
> >> cast this.
> > 
> > Wondering - can the wrappers be moved from CPP land to C land by 
> > turning them into inlines? (i havent checked all usages so there 
> > might be some surprises, but by and large it ought to be 
> > possible.)
> 
> I thought about doing it that way. I decided not to because I 
> suspected it would be harder to verify that the behaviour is 
> unchanged.

These things break noisily if they are wrong so i wouldnt be worried 
about that aspect.

> Also the _lock_irqsave functions output to the flags parameter 
> (which isn't a pointer) so that has to remain a macro.

Do we still need it? I remember it was originally due to some 
sparc32-ness, but meanwhile that's fixed in Sparc so we can 
generally pass irq flags around at will.

> If you'd really rather an inline version, I can spend some time 
> looking into it.

Would be nice.

	Ingo

^ permalink raw reply

* [1/1] connector maintainer/mail update.
From: Evgeniy Polyakov @ 2009-07-18 14:25 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Mike Frysinger, David Miller, netdev
In-Reply-To: <20090717223307.d86a5551.akpm@linux-foundation.org>

Please apply.

Signed-off-by: Evgeniy Polyakov <zbr@ioremap.net>

diff --git a/Documentation/connector/cn_test.c b/Documentation/connector/cn_test.c
index 6977c17..6344f7b 100644
--- a/Documentation/connector/cn_test.c
+++ b/Documentation/connector/cn_test.c
@@ -1,7 +1,7 @@
 /*
  * 	cn_test.c
  * 
- * 2004-2005 Copyright (c) Evgeniy Polyakov <johnpol@2ka.mipt.ru>
+ * 2004+ Copyright (c) Evgeniy Polyakov <zbr@ioremap.net>
  * All rights reserved.
  * 
  * This program is free software; you can redistribute it and/or modify
@@ -187,5 +187,5 @@ module_init(cn_test_init);
 module_exit(cn_test_fini);
 
 MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Evgeniy Polyakov <johnpol@2ka.mipt.ru>");
+MODULE_AUTHOR("Evgeniy Polyakov <zbr@ioremap.net>");
 MODULE_DESCRIPTION("Connector's test module");
diff --git a/Documentation/connector/ucon.c b/Documentation/connector/ucon.c
index d738cde..c5092ad 100644
--- a/Documentation/connector/ucon.c
+++ b/Documentation/connector/ucon.c
@@ -1,7 +1,7 @@
 /*
  * 	ucon.c
  *
- * Copyright (c) 2004+ Evgeniy Polyakov <johnpol@2ka.mipt.ru>
+ * Copyright (c) 2004+ Evgeniy Polyakov <zbr@ioremap.net>
  *
  *
  * This program is free software; you can redistribute it and/or modify
diff --git a/MAINTAINERS b/MAINTAINERS
index c944d61..764ce3b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1518,6 +1518,13 @@ S:	Supported
 F:	fs/configfs/
 F:	include/linux/configfs.h
 
+CONNECTOR
+P:	Evgeniy Polyakov
+M:	zbr@ioremap.net
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/connector/
+
 CONTROL GROUPS (CGROUPS)
 P:	Paul Menage
 M:	menage@google.com
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index c769ef2..b4ad84d 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -1,7 +1,7 @@
 /*
  * 	cn_queue.c
  *
- * 2004-2005 Copyright (c) Evgeniy Polyakov <johnpol@2ka.mipt.ru>
+ * 2004+ Copyright (c) Evgeniy Polyakov <zbr@ioremap.net>
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index fd336c5..b135281 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -1,7 +1,7 @@
 /*
  * 	connector.c
  *
- * 2004-2005 Copyright (c) Evgeniy Polyakov <johnpol@2ka.mipt.ru>
+ * 2004+ Copyright (c) Evgeniy Polyakov <zbr@ioremap.net>
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -33,7 +33,7 @@
 #include <net/sock.h>
 
 MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Evgeniy Polyakov <johnpol@2ka.mipt.ru>");
+MODULE_AUTHOR("Evgeniy Polyakov <zbr@ioremap.net>");
 MODULE_DESCRIPTION("Generic userspace <-> kernelspace connector.");
 
 static u32 cn_idx = CN_IDX_CONNECTOR;


-- 
	Evgeniy Polyakov

^ permalink raw reply related

* Re: [PATCH v2 0/4] net: Revive fixed link support
From: Anton Vorontsov @ 2009-07-18 18:04 UTC (permalink / raw)
  To: Grant Likely; +Cc: davem, afleming, netdev, linuxppc-dev, leoli
In-Reply-To: <20090717065220.15652.93331.stgit@localhost.localdomain>

On Fri, Jul 17, 2009 at 01:31:25AM -0600, Grant Likely wrote:
[...]
> Part of the problem I think is that the phylib code merges two separate
> constructs; the construct of an MDIO bus (on which many device may
> reside, not all of them PHYs), and the construct of an MII link whose
> speed and configuration need to be manipulated.  I've run into problems
> myself on how best to handle things like Ethernet switches which
> definitely do not behave like PHYs and the phylib state machine cannot
> be used on them.  It seems to me that the whole 'dummy phy' approach
> is just an artifact of the phylib model not being quite right yet.

Yep. With a bit of phylib rework we can remove all the MDIO emulation
stuff from phy/fixed.c driver, and leave there just speed/duplex/pause
assignments.

Though, I still believe that we should avoid two code paths in the
drivers. One of the code paths will be constantly broken if we do so.

> I
> want to investigate the possibility of separating the two concepts, but
> that will require a fair bit of thought and experimentation.

That would be great indeed.

[...]
> Anton, once again I don't have hardware to test this, so I rely on you
> to tell be if I screwed it up.  It has been compile tested.

Works fine here, thanks!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [1/1] connector maintainer/mail update.
From: Mike Frysinger @ 2009-07-18 18:35 UTC (permalink / raw)
  To: Evgeniy Polyakov; +Cc: Andrew Morton, David Miller, netdev
In-Reply-To: <20090718142533.GA1948@ioremap.net>

On Sat, Jul 18, 2009 at 10:25, Evgeniy Polyakov wrote:
> - * 2004-2005 Copyright (c) Evgeniy Polyakov <johnpol@2ka.mipt.ru>
> + * 2004+ Copyright (c) Evgeniy Polyakov <zbr@ioremap.net>

i dont think open-ended copyrights are acceptable.  copyrights arent
something that extend forever which is why they need explicit updating
(i.e. you'd want 2004-<last year you made a change>).

> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> +CONNECTOR
> +P:     Evgeniy Polyakov
> +M:     zbr@ioremap.net
> +L:     netdev@vger.kernel.org
> +S:     Maintained
> +F:     drivers/connector/

thanks
-mike

^ permalink raw reply

* Re: [PATCH v2 0/4] net: Revive fixed link support
From: Grant Likely @ 2009-07-18 18:37 UTC (permalink / raw)
  To: avorontsov; +Cc: davem, afleming, netdev, linuxppc-dev, leoli
In-Reply-To: <20090718180448.GA3252@oksana.dev.rtsoft.ru>

On Sat, Jul 18, 2009 at 12:04 PM, Anton
Vorontsov<avorontsov@ru.mvista.com> wrote:
> On Fri, Jul 17, 2009 at 01:31:25AM -0600, Grant Likely wrote:
> [...]
>> Part of the problem I think is that the phylib code merges two separate
>> constructs; the construct of an MDIO bus (on which many device may
>> reside, not all of them PHYs), and the construct of an MII link whose
>> speed and configuration need to be manipulated.  I've run into problems
>> myself on how best to handle things like Ethernet switches which
>> definitely do not behave like PHYs and the phylib state machine cannot
>> be used on them.  It seems to me that the whole 'dummy phy' approach
>> is just an artifact of the phylib model not being quite right yet.
>
> Yep. With a bit of phylib rework we can remove all the MDIO emulation
> stuff from phy/fixed.c driver, and leave there just speed/duplex/pause
> assignments.
>
> Though, I still believe that we should avoid two code paths in the
> drivers. One of the code paths will be constantly broken if we do so.

Yes, I agree.  Splitting the concepts also has the added advantage
that non-phy devices will have an interface to manipulate the link
speed without modifying drivers.

>> Anton, once again I don't have hardware to test this, so I rely on you
>> to tell be if I screwed it up.  It has been compile tested.
>
> Works fine here, thanks!

Awesome.  Dave, can you please pick up this series?

Thanks,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [1/1] connector maintainer/mail update.
From: Evgeniy Polyakov @ 2009-07-18 18:43 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: Andrew Morton, David Miller, netdev
In-Reply-To: <8bd0f97a0907181135w27c8c56al636aefc6bf7b09f8@mail.gmail.com>

On Sat, Jul 18, 2009 at 02:35:55PM -0400, Mike Frysinger (vapier.adi@gmail.com) wrote:
> On Sat, Jul 18, 2009 at 10:25, Evgeniy Polyakov wrote:
> > - * 2004-2005 Copyright (c) Evgeniy Polyakov <johnpol@2ka.mipt.ru>
> > + * 2004+ Copyright (c) Evgeniy Polyakov <zbr@ioremap.net>
> 
> i dont think open-ended copyrights are acceptable.  copyrights arent
> something that extend forever which is why they need explicit updating
> (i.e. you'd want 2004-<last year you made a change>).

It can be always replaced back if I will step aside.

-- 
	Evgeniy Polyakov

^ 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