Netdev List
 help / color / mirror / Atom feed
* [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ
From: Rémi Denis-Courmont @ 2011-03-04 10:00 UTC (permalink / raw)
  To: netdev
In-Reply-To: <201103031322.37184.remi.denis-courmont@nokia.com>

This moves most of the accept logic to process context, and simplifies
the code a bit at the same time.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 include/net/phonet/pep.h |    1 -
 net/phonet/pep.c         |  284 +++++++++++++++++++---------------------------
 net/phonet/socket.c      |   10 +-
 3 files changed, 121 insertions(+), 174 deletions(-)

diff --git a/include/net/phonet/pep.h b/include/net/phonet/pep.h
index 38eed1b..b669fe6 100644
--- a/include/net/phonet/pep.h
+++ b/include/net/phonet/pep.h
@@ -28,7 +28,6 @@ struct pep_sock {
 
 	/* XXX: union-ify listening vs connected stuff ? */
 	/* Listening socket stuff: */
-	struct hlist_head	ackq;
 	struct hlist_head	hlist;
 
 	/* Connected socket stuff: */
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 610794a..c0fab4c 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -42,7 +42,7 @@
  * TCP_ESTABLISHED	connected pipe in enabled state
  *
  * pep_sock locking:
- *  - sk_state, ackq, hlist: sock lock needed
+ *  - sk_state, hlist: sock lock needed
  *  - listener: read only
  *  - pipe_handle: read only
  */
@@ -202,11 +202,12 @@ static int pep_accept_conn(struct sock *sk, struct sk_buff *skb)
 				GFP_KERNEL);
 }
 
-static int pep_reject_conn(struct sock *sk, struct sk_buff *skb, u8 code)
+static int pep_reject_conn(struct sock *sk, struct sk_buff *skb, u8 code,
+				gfp_t priority)
 {
 	static const u8 data[4] = { PAD, PAD, PAD, 0 /* sub-blocks */ };
 	WARN_ON(code == PN_PIPE_NO_ERROR);
-	return pep_reply(sk, skb, code, data, sizeof(data), GFP_ATOMIC);
+	return pep_reply(sk, skb, code, data, sizeof(data), priority);
 }
 
 /* Control requests are not sent by the pipe service and have a specific
@@ -365,7 +366,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
 
 	switch (hdr->message_id) {
 	case PNS_PEP_CONNECT_REQ:
-		pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE);
+		pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE, GFP_ATOMIC);
 		break;
 
 	case PNS_PEP_DISCONNECT_REQ:
@@ -574,7 +575,6 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
 
 	sk->sk_state = TCP_SYN_RECV;
 	sk->sk_backlog_rcv = pipe_do_rcv;
-	sk->sk_destruct = pipe_destruct;
 	pn->rx_credits = 0;
 	sk->sk_state_change(sk);
 
@@ -582,96 +582,6 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
 }
 #endif
 
-static int pep_connreq_rcv(struct sock *sk, struct sk_buff *skb)
-{
-	struct sock *newsk;
-	struct pep_sock *newpn, *pn = pep_sk(sk);
-	struct pnpipehdr *hdr;
-	struct sockaddr_pn dst, src;
-	u16 peer_type;
-	u8 pipe_handle, enabled, n_sb;
-	u8 aligned = 0;
-
-	if (!pskb_pull(skb, sizeof(*hdr) + 4))
-		return -EINVAL;
-
-	hdr = pnp_hdr(skb);
-	pipe_handle = hdr->pipe_handle;
-	switch (hdr->state_after_connect) {
-	case PN_PIPE_DISABLE:
-		enabled = 0;
-		break;
-	case PN_PIPE_ENABLE:
-		enabled = 1;
-		break;
-	default:
-		pep_reject_conn(sk, skb, PN_PIPE_ERR_INVALID_PARAM);
-		return -EINVAL;
-	}
-	peer_type = hdr->other_pep_type << 8;
-
-	/* Parse sub-blocks (options) */
-	n_sb = hdr->data[4];
-	while (n_sb > 0) {
-		u8 type, buf[1], len = sizeof(buf);
-		const u8 *data = pep_get_sb(skb, &type, &len, buf);
-
-		if (data == NULL)
-			return -EINVAL;
-		switch (type) {
-		case PN_PIPE_SB_CONNECT_REQ_PEP_SUB_TYPE:
-			if (len < 1)
-				return -EINVAL;
-			peer_type = (peer_type & 0xff00) | data[0];
-			break;
-		case PN_PIPE_SB_ALIGNED_DATA:
-			aligned = data[0] != 0;
-			break;
-		}
-		n_sb--;
-	}
-
-	skb = skb_clone(skb, GFP_ATOMIC);
-	if (!skb)
-		return -ENOMEM;
-
-	/* Create a new to-be-accepted sock */
-	newsk = sk_alloc(sock_net(sk), PF_PHONET, GFP_ATOMIC, sk->sk_prot);
-	if (!newsk) {
-		kfree_skb(skb);
-		return -ENOMEM;
-	}
-	sock_init_data(NULL, newsk);
-	newsk->sk_state = TCP_SYN_RECV;
-	newsk->sk_backlog_rcv = pipe_do_rcv;
-	newsk->sk_protocol = sk->sk_protocol;
-	newsk->sk_destruct = pipe_destruct;
-
-	newpn = pep_sk(newsk);
-	pn_skb_get_dst_sockaddr(skb, &dst);
-	pn_skb_get_src_sockaddr(skb, &src);
-	newpn->pn_sk.sobject = pn_sockaddr_get_object(&dst);
-	newpn->pn_sk.dobject = pn_sockaddr_get_object(&src);
-	newpn->pn_sk.resource = pn_sockaddr_get_resource(&dst);
-	skb_queue_head_init(&newpn->ctrlreq_queue);
-	newpn->pipe_handle = pipe_handle;
-	atomic_set(&newpn->tx_credits, 0);
-	newpn->peer_type = peer_type;
-	newpn->rx_credits = 0;
-	newpn->rx_fc = newpn->tx_fc = PN_LEGACY_FLOW_CONTROL;
-	newpn->init_enable = enabled;
-	newpn->aligned = aligned;
-
-	BUG_ON(!skb_queue_empty(&newsk->sk_receive_queue));
-	skb_queue_head(&newsk->sk_receive_queue, skb);
-	if (!sock_flag(sk, SOCK_DEAD))
-		sk->sk_data_ready(sk, 0);
-
-	sk_acceptq_added(sk);
-	sk_add_node(newsk, &pn->ackq);
-	return 0;
-}
-
 /* Listening sock must be locked */
 static struct sock *pep_find_pipe(const struct hlist_head *hlist,
 					const struct sockaddr_pn *dst,
@@ -726,22 +636,18 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
 	if (sknode)
 		return sk_receive_skb(sknode, skb, 1);
 
-	/* Look for a pipe handle pending accept */
-	sknode = pep_find_pipe(&pn->ackq, &dst, pipe_handle);
-	if (sknode) {
-		sock_put(sknode);
-		if (net_ratelimit())
-			printk(KERN_WARNING"Phonet unconnected PEP ignored");
-		goto drop;
-	}
-
 	switch (hdr->message_id) {
 	case PNS_PEP_CONNECT_REQ:
-		if (sk->sk_state == TCP_LISTEN && !sk_acceptq_is_full(sk))
-			pep_connreq_rcv(sk, skb);
-		else
-			pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE);
-		break;
+		if (sk->sk_state != TCP_LISTEN || sk_acceptq_is_full(sk)) {
+			pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE,
+					GFP_ATOMIC);
+			break;
+		}
+		skb_queue_head(&sk->sk_receive_queue, skb);
+		sk_acceptq_added(sk);
+		if (!sock_flag(sk, SOCK_DEAD))
+			sk->sk_data_ready(sk, 0);
+		return NET_RX_SUCCESS;
 
 #ifdef CONFIG_PHONET_PIPECTRLR
 	case PNS_PEP_CONNECT_RESP:
@@ -799,24 +705,16 @@ static void pep_sock_close(struct sock *sk, long timeout)
 	sk_common_release(sk);
 
 	lock_sock(sk);
-	if (sk->sk_state == TCP_LISTEN) {
-		/* Destroy the listen queue */
-		struct sock *sknode;
-		struct hlist_node *p, *n;
-
-		sk_for_each_safe(sknode, p, n, &pn->ackq)
-			sk_del_node_init(sknode);
-		sk->sk_state = TCP_CLOSE;
-	} else if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
+	if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
 #ifndef CONFIG_PHONET_PIPECTRLR
 		/* Forcefully remove dangling Phonet pipe */
 		pipe_do_remove(sk);
 #else
 		/* send pep disconnect request */
 		pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD, NULL, 0);
-		sk->sk_state = TCP_CLOSE;
 #endif
 	}
+	sk->sk_state = TCP_CLOSE;
 
 	ifindex = pn->ifindex;
 	pn->ifindex = 0;
@@ -827,69 +725,121 @@ static void pep_sock_close(struct sock *sk, long timeout)
 	sock_put(sk);
 }
 
-static int pep_wait_connreq(struct sock *sk, int noblock)
+static struct sock *pep_sock_accept(struct sock *sk, int flags, int *errp)
 {
-	struct task_struct *tsk = current;
-	struct pep_sock *pn = pep_sk(sk);
-	long timeo = sock_rcvtimeo(sk, noblock);
-
-	for (;;) {
-		DEFINE_WAIT(wait);
+	struct pep_sock *pn = pep_sk(sk), *newpn;
+	struct sock *newsk = NULL;
+	struct sk_buff *skb;
+	struct pnpipehdr *hdr;
+	struct sockaddr_pn dst, src;
+	int err;
+	u16 peer_type;
+	u8 pipe_handle, enabled, n_sb;
+	u8 aligned = 0;
 
-		if (sk->sk_state != TCP_LISTEN)
-			return -EINVAL;
-		if (!hlist_empty(&pn->ackq))
-			break;
-		if (!timeo)
-			return -EWOULDBLOCK;
-		if (signal_pending(tsk))
-			return sock_intr_errno(timeo);
+	skb = skb_recv_datagram(sk, 0, flags & O_NONBLOCK, errp);
+	if (!skb)
+		return NULL;
 
-		prepare_to_wait_exclusive(sk_sleep(sk), &wait,
-						TASK_INTERRUPTIBLE);
-		release_sock(sk);
-		timeo = schedule_timeout(timeo);
-		lock_sock(sk);
-		finish_wait(sk_sleep(sk), &wait);
+	lock_sock(sk);
+	if (sk->sk_state != TCP_LISTEN) {
+		err = -EINVAL;
+		goto drop;
 	}
+	sk_acceptq_removed(sk);
 
-	return 0;
-}
+	err = -EPROTO;
+	if (!pskb_may_pull(skb, sizeof(*hdr) + 4))
+		goto drop;
 
-static struct sock *pep_sock_accept(struct sock *sk, int flags, int *errp)
-{
-	struct pep_sock *pn = pep_sk(sk);
-	struct sock *newsk = NULL;
-	struct sk_buff *oskb;
-	int err;
+	hdr = pnp_hdr(skb);
+	pipe_handle = hdr->pipe_handle;
+	switch (hdr->state_after_connect) {
+	case PN_PIPE_DISABLE:
+		enabled = 0;
+		break;
+	case PN_PIPE_ENABLE:
+		enabled = 1;
+		break;
+	default:
+		pep_reject_conn(sk, skb, PN_PIPE_ERR_INVALID_PARAM,
+				GFP_KERNEL);
+		goto drop;
+	}
+	peer_type = hdr->other_pep_type << 8;
 
-	lock_sock(sk);
-	err = pep_wait_connreq(sk, flags & O_NONBLOCK);
-	if (err)
-		goto out;
+	/* Parse sub-blocks (options) */
+	n_sb = hdr->data[4];
+	while (n_sb > 0) {
+		u8 type, buf[1], len = sizeof(buf);
+		const u8 *data = pep_get_sb(skb, &type, &len, buf);
 
-	newsk = __sk_head(&pn->ackq);
+		if (data == NULL)
+			goto drop;
+		switch (type) {
+		case PN_PIPE_SB_CONNECT_REQ_PEP_SUB_TYPE:
+			if (len < 1)
+				goto drop;
+			peer_type = (peer_type & 0xff00) | data[0];
+			break;
+		case PN_PIPE_SB_ALIGNED_DATA:
+			aligned = data[0] != 0;
+			break;
+		}
+		n_sb--;
+	}
 
-	oskb = skb_dequeue(&newsk->sk_receive_queue);
-	err = pep_accept_conn(newsk, oskb);
-	if (err) {
-		skb_queue_head(&newsk->sk_receive_queue, oskb);
+	/* Check for duplicate pipe handle */
+	newsk = pep_find_pipe(&pn->hlist, &dst, pipe_handle);
+	if (unlikely(newsk)) {
+		__sock_put(newsk);
 		newsk = NULL;
-		goto out;
+		pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE, GFP_KERNEL);
+		goto drop;
+	}
+
+	/* Create a new to-be-accepted sock */
+	newsk = sk_alloc(sock_net(sk), PF_PHONET, GFP_KERNEL, sk->sk_prot);
+	if (!newsk) {
+		pep_reject_conn(sk, skb, PN_PIPE_ERR_OVERLOAD, GFP_KERNEL);
+		err = -ENOBUFS;
+		goto drop;
 	}
-	kfree_skb(oskb);
 
+	sock_init_data(NULL, newsk);
+	newsk->sk_state = TCP_SYN_RECV;
+	newsk->sk_backlog_rcv = pipe_do_rcv;
+	newsk->sk_protocol = sk->sk_protocol;
+	newsk->sk_destruct = pipe_destruct;
+
+	newpn = pep_sk(newsk);
+	pn_skb_get_dst_sockaddr(skb, &dst);
+	pn_skb_get_src_sockaddr(skb, &src);
+	newpn->pn_sk.sobject = pn_sockaddr_get_object(&dst);
+	newpn->pn_sk.dobject = pn_sockaddr_get_object(&src);
+	newpn->pn_sk.resource = pn_sockaddr_get_resource(&dst);
 	sock_hold(sk);
-	pep_sk(newsk)->listener = sk;
+	newpn->listener = sk;
+	skb_queue_head_init(&newpn->ctrlreq_queue);
+	newpn->pipe_handle = pipe_handle;
+	atomic_set(&newpn->tx_credits, 0);
+	newpn->ifindex = 0;
+	newpn->peer_type = peer_type;
+	newpn->rx_credits = 0;
+	newpn->rx_fc = newpn->tx_fc = PN_LEGACY_FLOW_CONTROL;
+	newpn->init_enable = enabled;
+	newpn->aligned = aligned;
 
-	sock_hold(newsk);
-	sk_del_node_init(newsk);
-	sk_acceptq_removed(sk);
+	err = pep_accept_conn(newsk, skb);
+	if (err) {
+		sock_put(newsk);
+		newsk = NULL;
+		goto drop;
+	}
 	sk_add_node(newsk, &pn->hlist);
-	__sock_put(newsk);
-
-out:
+drop:
 	release_sock(sk);
+	kfree_skb(skb);
 	*errp = err;
 	return newsk;
 }
@@ -937,7 +887,7 @@ static int pep_init(struct sock *sk)
 {
 	struct pep_sock *pn = pep_sk(sk);
 
-	INIT_HLIST_HEAD(&pn->ackq);
+	sk->sk_destruct = pipe_destruct;
 	INIT_HLIST_HEAD(&pn->hlist);
 	skb_queue_head_init(&pn->ctrlreq_queue);
 	pn->pipe_handle = PN_PIPE_INVALID_HANDLE;
diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 65a0333..1eccfc3 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -327,6 +327,9 @@ static int pn_socket_accept(struct socket *sock, struct socket *newsock,
 	struct sock *newsk;
 	int err;
 
+	if (unlikely(sk->sk_state != TCP_LISTEN))
+		return -EINVAL;
+
 	newsk = sk->sk_prot->accept(sk, flags, &err);
 	if (!newsk)
 		return err;
@@ -363,13 +366,8 @@ static unsigned int pn_socket_poll(struct file *file, struct socket *sock,
 
 	poll_wait(file, sk_sleep(sk), wait);
 
-	switch (sk->sk_state) {
-	case TCP_LISTEN:
-		return hlist_empty(&pn->ackq) ? 0 : POLLIN;
-	case TCP_CLOSE:
+	if (sk->sk_state == TCP_CLOSE)
 		return POLLERR;
-	}
-
 	if (!skb_queue_empty(&sk->sk_receive_queue))
 		mask |= POLLIN | POLLRDNORM;
 	if (!skb_queue_empty(&pn->ctrlreq_queue))
-- 
1.7.1


^ permalink raw reply related

* [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig
From: Rémi Denis-Courmont @ 2011-03-04 10:00 UTC (permalink / raw)
  To: netdev
In-Reply-To: <201103031322.37184.remi.denis-courmont@nokia.com>

This is now a run-time choice so that a single kernel can support both
old and new generation ISI modems.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 Documentation/networking/phonet.txt |   13 -------------
 include/linux/phonet.h              |    2 --
 net/phonet/Kconfig                  |   12 ------------
 net/phonet/pep.c                    |   25 -------------------------
 4 files changed, 0 insertions(+), 52 deletions(-)

diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index 3d12779..8100358 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -205,19 +205,6 @@ The pipe protocol provides two socket options at the SOL_PNPIPE level:
     socket descriptors that are already connected or being connected.
 
 
-Phonet Pipe-controller Implementation
--------------------------------------
-
-Phonet Pipe-controller is enabled by selecting the CONFIG_PHONET_PIPECTRLR
-Kconfig option.
-
-The implementation adds socket options at SOL_PNPIPE level:
-
-  PNPIPE_ENABLE accepts one integer value (int). If set to zero, the pipe
-    is disabled. If the value is non-zero, the pipe is enabled. If the pipe
-    is not (yet) connected, ENOTCONN is error is returned.
-
-
 Authors
 -------
 
diff --git a/include/linux/phonet.h b/include/linux/phonet.h
index 32a0965..6fb1384 100644
--- a/include/linux/phonet.h
+++ b/include/linux/phonet.h
@@ -37,8 +37,6 @@
 #define PNPIPE_ENCAP		1
 #define PNPIPE_IFINDEX		2
 #define PNPIPE_HANDLE		3
-#define PNPIPE_ENABLE           4
-/* unused slot */
 
 #define PNADDR_ANY		0
 #define PNADDR_BROADCAST	0xFC
diff --git a/net/phonet/Kconfig b/net/phonet/Kconfig
index 0d9b8a2..6ec7d55 100644
--- a/net/phonet/Kconfig
+++ b/net/phonet/Kconfig
@@ -14,15 +14,3 @@ config PHONET
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called phonet. If unsure, say N.
-
-config PHONET_PIPECTRLR
-	bool "Phonet Pipe Controller (EXPERIMENTAL)"
-	depends on PHONET && EXPERIMENTAL
-	default N
-	help
-	  The Pipe Controller implementation in Phonet stack to support Pipe
-	  data with Nokia Slim modems like WG2.5 used on ST-Ericsson U8500
-	  platform.
-
-	  This option is incompatible with older Nokia modems.
-	  Say N here unless you really know what you are doing.
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 671effb..68e635f 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -167,15 +167,6 @@ static int pipe_handler_send_created_ind(struct sock *sk)
 				data, 4, GFP_ATOMIC);
 }
 
-#ifdef CONFIG_PHONET_PIPECTRLR
-static int pipe_handler_enable_pipe(struct sock *sk, int enable)
-{
-	u8 id = enable ? PNS_PEP_ENABLE_REQ : PNS_PEP_DISABLE_REQ;
-
-	return pipe_handler_request(sk, id, PAD, NULL, 0);
-}
-#endif
-
 static int pep_accept_conn(struct sock *sk, struct sk_buff *skb)
 {
 	static const u8 data[20] = {
@@ -968,16 +959,6 @@ static int pep_setsockopt(struct sock *sk, int level, int optname,
 		}
 		goto out_norel;
 
-#ifdef CONFIG_PHONET_PIPECTRLR
-	case PNPIPE_ENABLE:
-		if ((1 << sk->sk_state) & ~(TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
-			err = -ENOTCONN;
-			break;
-		}
-		err = pipe_handler_enable_pipe(sk, val);
-		break;
-#endif
-
 	default:
 		err = -ENOPROTOOPT;
 	}
@@ -1013,12 +994,6 @@ static int pep_getsockopt(struct sock *sk, int level, int optname,
 			return -EINVAL;
 		break;
 
-#ifdef CONFIG_PHONET_PIPECTRLR
-	case PNPIPE_ENABLE:
-		val = sk->sk_state == TCP_ESTABLISHED;
-		break;
-#endif
-
 	default:
 		return -ENOPROTOOPT;
 	}
-- 
1.7.1


^ permalink raw reply related

* [PATCH 7/8] Phonet: support active connection without pipe controller on modem
From: Rémi Denis-Courmont @ 2011-03-04 10:00 UTC (permalink / raw)
  To: netdev
In-Reply-To: <201103031322.37184.remi.denis-courmont@nokia.com>

This provides support for newer ISI modems with no need for the
earlier experimental compile-time alternative choice. This also fixes
a number of remaining bugs in the old ST-E code.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 Documentation/networking/phonet.txt |   53 +++++------
 net/phonet/pep.c                    |  172 ++++++++++++++++++++--------------
 net/phonet/socket.c                 |  102 ++++++++-------------
 3 files changed, 165 insertions(+), 162 deletions(-)

diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index cacac96..3d12779 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -154,9 +154,28 @@ connections, one per accept()'d socket.
     write(cfd, msg, msglen);
   }
 
-Connections are established between two endpoints by a "third party"
-application. This means that both endpoints are passive; so connect()
-is not possible.
+Connections are traditionally established between two endpoints by a
+"third party" application. This means that both endpoints are passive.
+
+
+As of Linux kernel version 2.6.39, it is also possible to connect
+two endpoints directly, using connect() on the active side. This is
+intended to support the newer Nokia Wireless Modem API, as found in
+e.g. the Nokia Slim Modem in the ST-Ericsson U8500 platform:
+
+  struct sockaddr_spn spn;
+  int fd;
+
+  fd = socket(PF_PHONET, SOCK_SEQPACKET, PN_PROTO_PIPE);
+  memset(&spn, 0, sizeof(spn));
+  spn.spn_family = AF_PHONET;
+  spn.spn_obj = ...;
+  spn.spn_dev = ...;
+  spn.spn_resource = 0xD9;
+  connect(fd, (struct sockaddr *)&spn, sizeof(spn));
+  /* normal I/O here ... */
+  close(fd);
+
 
 WARNING:
 When polling a connected pipe socket for writability, there is an
@@ -189,17 +208,8 @@ The pipe protocol provides two socket options at the SOL_PNPIPE level:
 Phonet Pipe-controller Implementation
 -------------------------------------
 
-Phonet Pipe-controller is enabled by selecting the CONFIG_PHONET_PIPECTRLR Kconfig
-option. It is useful when communicating with those Nokia Modems which do not
-implement Pipe controller in them e.g. Nokia Slim Modem used in ST-Ericsson
-U8500 platform.
-
-The implementation is based on the Data Connection Establishment Sequence
-depicted in 'Nokia Wireless Modem API - Wireless_modem_user_guide.pdf'
-document.
-
-It allows a phonet sequenced socket (host-pep) to initiate a Pipe connection
-between itself and a remote pipe-end point (e.g. modem).
+Phonet Pipe-controller is enabled by selecting the CONFIG_PHONET_PIPECTRLR
+Kconfig option.
 
 The implementation adds socket options at SOL_PNPIPE level:
 
@@ -207,21 +217,6 @@ The implementation adds socket options at SOL_PNPIPE level:
     is disabled. If the value is non-zero, the pipe is enabled. If the pipe
     is not (yet) connected, ENOTCONN is error is returned.
 
-The implementation also adds socket 'connect'. On calling the 'connect', pipe
-will be created between the source socket and the destination, and the pipe
-state will be set to PIPE_DISABLED.
-
-After a pipe has been created and enabled successfully, the Pipe data can be
-exchanged between the host-pep and remote-pep (modem).
-
-User-space would typically follow below sequence with Pipe controller:-
--socket
--bind
--setsockopt for PNPIPE_PIPE_HANDLE
--connect
--setsockopt for PNPIPE_ENCAP_IP
--setsockopt for PNPIPE_ENABLE
-
 
 Authors
 -------
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index abfb795..671effb 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -136,7 +136,6 @@ static int pep_indicate(struct sock *sk, u8 id, u8 code,
 
 #define PAD 0x00
 
-#ifdef CONFIG_PHONET_PIPECTRLR
 static int pipe_handler_request(struct sock *sk, u8 id, u8 code,
 				const void *data, int len)
 {
@@ -168,11 +167,7 @@ static int pipe_handler_send_created_ind(struct sock *sk)
 				data, 4, GFP_ATOMIC);
 }
 
-static int pipe_handler_send_ind(struct sock *sk, u8 id)
-{
-	return pep_indicate(sk, id, PAD, NULL, 0, GFP_ATOMIC);
-}
-
+#ifdef CONFIG_PHONET_PIPECTRLR
 static int pipe_handler_enable_pipe(struct sock *sk, int enable)
 {
 	u8 id = enable ? PNS_PEP_ENABLE_REQ : PNS_PEP_DISABLE_REQ;
@@ -376,32 +371,11 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
 			sk->sk_state_change(sk);
 		break;
 
-#ifdef CONFIG_PHONET_PIPECTRLR
-	case PNS_PEP_DISCONNECT_RESP:
-		sk->sk_state = TCP_CLOSE;
-		break;
-#endif
-
 	case PNS_PEP_ENABLE_REQ:
 		/* Wait for PNS_PIPE_(ENABLED|REDIRECTED)_IND */
 		pep_reply(sk, skb, PN_PIPE_NO_ERROR, NULL, 0, GFP_ATOMIC);
 		break;
 
-#ifdef CONFIG_PHONET_PIPECTRLR
-	case PNS_PEP_ENABLE_RESP:
-		pipe_handler_send_ind(sk, PNS_PIPE_ENABLED_IND);
-
-		if (!pn_flow_safe(pn->tx_fc)) {
-			atomic_set(&pn->tx_credits, 1);
-			sk->sk_write_space(sk);
-		}
-		if (sk->sk_state == TCP_ESTABLISHED)
-			break; /* Nothing to do */
-		sk->sk_state = TCP_ESTABLISHED;
-		pipe_grant_credits(sk, GFP_ATOMIC);
-		break;
-#endif
-
 	case PNS_PEP_RESET_REQ:
 		switch (hdr->state_after_reset) {
 		case PN_PIPE_DISABLE:
@@ -420,15 +394,6 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
 		pep_reply(sk, skb, PN_PIPE_NO_ERROR, NULL, 0, GFP_ATOMIC);
 		break;
 
-#ifdef CONFIG_PHONET_PIPECTRLR
-	case PNS_PEP_DISABLE_RESP:
-		atomic_set(&pn->tx_credits, 0);
-		pipe_handler_send_ind(sk, PNS_PIPE_DISABLED_IND);
-		sk->sk_state = TCP_SYN_RECV;
-		pn->rx_credits = 0;
-		break;
-#endif
-
 	case PNS_PEP_CTRL_REQ:
 		if (skb_queue_len(&pn->ctrlreq_queue) >= PNPIPE_CTRLREQ_MAX) {
 			atomic_inc(&sk->sk_drops);
@@ -521,7 +486,6 @@ static void pipe_destruct(struct sock *sk)
 	skb_queue_purge(&pn->ctrlreq_queue);
 }
 
-#ifdef CONFIG_PHONET_PIPECTRLR
 static u8 pipe_negotiate_fc(const u8 *fcs, unsigned n)
 {
 	unsigned i;
@@ -546,6 +510,8 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
 		return -EINVAL;
 
 	hdr = pnp_hdr(skb);
+	if (hdr->error_code != PN_PIPE_NO_ERROR)
+		return -ECONNREFUSED;
 
 	/* Parse sub-blocks */
 	n_sb = hdr->data[4];
@@ -573,14 +539,74 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
 		n_sb--;
 	}
 
-	sk->sk_state = TCP_SYN_RECV;
-	sk->sk_backlog_rcv = pipe_do_rcv;
-	pn->rx_credits = 0;
-	sk->sk_state_change(sk);
-
 	return pipe_handler_send_created_ind(sk);
 }
-#endif
+
+/* Queue an skb to an actively connected sock.
+ * Socket lock must be held. */
+static int pipe_handler_do_rcv(struct sock *sk, struct sk_buff *skb)
+{
+	struct pep_sock *pn = pep_sk(sk);
+	struct pnpipehdr *hdr = pnp_hdr(skb);
+	int err = NET_RX_SUCCESS;
+
+	switch (hdr->message_id) {
+	case PNS_PIPE_ALIGNED_DATA:
+		__skb_pull(skb, 1);
+		/* fall through */
+	case PNS_PIPE_DATA:
+		__skb_pull(skb, 3); /* Pipe data header */
+		if (!pn_flow_safe(pn->rx_fc)) {
+			err = sock_queue_rcv_skb(sk, skb);
+			if (!err)
+				return NET_RX_SUCCESS;
+			err = NET_RX_DROP;
+			break;
+		}
+
+		if (pn->rx_credits == 0) {
+			atomic_inc(&sk->sk_drops);
+			err = NET_RX_DROP;
+			break;
+		}
+		pn->rx_credits--;
+		skb->dev = NULL;
+		skb_set_owner_r(skb, sk);
+		err = skb->len;
+		skb_queue_tail(&sk->sk_receive_queue, skb);
+		if (!sock_flag(sk, SOCK_DEAD))
+			sk->sk_data_ready(sk, err);
+		return NET_RX_SUCCESS;
+
+	case PNS_PEP_CONNECT_RESP:
+		if (sk->sk_state != TCP_SYN_SENT)
+			break;
+		if (!sock_flag(sk, SOCK_DEAD))
+			sk->sk_state_change(sk);
+		if (pep_connresp_rcv(sk, skb)) {
+			sk->sk_state = TCP_CLOSE_WAIT;
+			break;
+		}
+
+		sk->sk_state = TCP_ESTABLISHED;
+		if (!pn_flow_safe(pn->tx_fc)) {
+			atomic_set(&pn->tx_credits, 1);
+			sk->sk_write_space(sk);
+		}
+		pipe_grant_credits(sk, GFP_ATOMIC);
+		break;
+
+	case PNS_PEP_DISCONNECT_RESP:
+		/* sock should already be dead, nothing to do */
+		break;
+
+	case PNS_PEP_STATUS_IND:
+		pipe_rcv_status(sk, skb);
+		break;
+	}
+	kfree_skb(skb);
+	return err;
+}
 
 /* Listening sock must be locked */
 static struct sock *pep_find_pipe(const struct hlist_head *hlist,
@@ -649,12 +675,6 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
 			sk->sk_data_ready(sk, 0);
 		return NET_RX_SUCCESS;
 
-#ifdef CONFIG_PHONET_PIPECTRLR
-	case PNS_PEP_CONNECT_RESP:
-		pep_connresp_rcv(sk, skb);
-		break;
-#endif
-
 	case PNS_PEP_DISCONNECT_REQ:
 		pep_reply(sk, skb, PN_PIPE_NO_ERROR, NULL, 0, GFP_ATOMIC);
 		break;
@@ -667,15 +687,19 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
 	case PNS_PEP_ENABLE_REQ:
 	case PNS_PEP_DISABLE_REQ:
 		/* invalid handle is not even allowed here! */
-	default:
 		break;
+
+	default:
+		if ((1 << sk->sk_state)
+				& ~(TCPF_CLOSE|TCPF_LISTEN|TCPF_CLOSE_WAIT))
+			/* actively connected socket */
+			return pipe_handler_do_rcv(sk, skb);
 	}
 drop:
 	kfree_skb(skb);
 	return NET_RX_SUCCESS;
 }
 
-#ifndef CONFIG_PHONET_PIPECTRLR
 static int pipe_do_remove(struct sock *sk)
 {
 	struct pep_sock *pn = pep_sk(sk);
@@ -693,7 +717,6 @@ static int pipe_do_remove(struct sock *sk)
 	ph->data[0] = PAD;
 	return pn_skb_send(sk, skb, NULL);
 }
-#endif
 
 /* associated socket ceases to exist */
 static void pep_sock_close(struct sock *sk, long timeout)
@@ -706,13 +729,12 @@ static void pep_sock_close(struct sock *sk, long timeout)
 
 	lock_sock(sk);
 	if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
-#ifndef CONFIG_PHONET_PIPECTRLR
-		/* Forcefully remove dangling Phonet pipe */
-		pipe_do_remove(sk);
-#else
-		/* send pep disconnect request */
-		pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD, NULL, 0);
-#endif
+		if (sk->sk_backlog_rcv == pipe_do_rcv)
+			/* Forcefully remove dangling Phonet pipe */
+			pipe_do_remove(sk);
+		else
+			pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD,
+						NULL, 0);
 	}
 	sk->sk_state = TCP_CLOSE;
 
@@ -844,20 +866,22 @@ drop:
 	return newsk;
 }
 
-#ifdef CONFIG_PHONET_PIPECTRLR
 static int pep_sock_connect(struct sock *sk, struct sockaddr *addr, int len)
 {
 	struct pep_sock *pn = pep_sk(sk);
-	const struct sockaddr_pn *spn = (struct sockaddr_pn *)addr;
+	int err;
 	u8 data[4] = { 0 /* sub-blocks */, PAD, PAD, PAD };
 
-	pn->pn_sk.dobject = pn_sockaddr_get_object(spn);
-	pn->pn_sk.resource = pn_sockaddr_get_resource(spn);
 	pn->pipe_handle = 1; /* anything but INVALID_HANDLE */
-	return pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
-					PN_PIPE_DISABLE, data, 4);
+	err = pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
+					PN_PIPE_ENABLE, data, 4);
+	if (err) {
+		pn->pipe_handle = PN_PIPE_INVALID_HANDLE;
+		return err;
+	}
+	sk->sk_state = TCP_SYN_SENT;
+	return 0;
 }
-#endif
 
 static int pep_ioctl(struct sock *sk, int cmd, unsigned long arg)
 {
@@ -890,8 +914,16 @@ static int pep_init(struct sock *sk)
 
 	sk->sk_destruct = pipe_destruct;
 	INIT_HLIST_HEAD(&pn->hlist);
+	pn->listener = NULL;
 	skb_queue_head_init(&pn->ctrlreq_queue);
+	atomic_set(&pn->tx_credits, 0);
+	pn->ifindex = 0;
+	pn->peer_type = 0;
 	pn->pipe_handle = PN_PIPE_INVALID_HANDLE;
+	pn->rx_credits = 0;
+	pn->rx_fc = pn->tx_fc = PN_LEGACY_FLOW_CONTROL;
+	pn->init_enable = 1;
+	pn->aligned = 0;
 	return 0;
 }
 
@@ -1219,9 +1251,9 @@ static void pep_sock_unhash(struct sock *sk)
 
 	lock_sock(sk);
 
-#ifndef CONFIG_PHONET_PIPECTRLR
-	if ((1 << sk->sk_state) & ~(TCPF_CLOSE|TCPF_LISTEN)) {
+	if (pn->listener != NULL) {
 		skparent = pn->listener;
+		pn->listener = NULL;
 		release_sock(sk);
 
 		pn = pep_sk(skparent);
@@ -1229,7 +1261,7 @@ static void pep_sock_unhash(struct sock *sk)
 		sk_del_node_init(sk);
 		sk = skparent;
 	}
-#endif
+
 	/* Unhash a listening sock only when it is closed
 	 * and all of its active connected pipes are closed. */
 	if (hlist_empty(&pn->hlist))
@@ -1243,9 +1275,7 @@ static void pep_sock_unhash(struct sock *sk)
 static struct proto pep_proto = {
 	.close		= pep_sock_close,
 	.accept		= pep_sock_accept,
-#ifdef CONFIG_PHONET_PIPECTRLR
 	.connect	= pep_sock_connect,
-#endif
 	.ioctl		= pep_ioctl,
 	.init		= pep_init,
 	.setsockopt	= pep_setsockopt,
diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 1eccfc3..b1adafa 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -225,15 +225,18 @@ static int pn_socket_autobind(struct socket *sock)
 	return 0; /* socket was already bound */
 }
 
-#ifdef CONFIG_PHONET_PIPECTRLR
 static int pn_socket_connect(struct socket *sock, struct sockaddr *addr,
 		int len, int flags)
 {
 	struct sock *sk = sock->sk;
+	struct pn_sock *pn = pn_sk(sk);
 	struct sockaddr_pn *spn = (struct sockaddr_pn *)addr;
-	long timeo;
+	struct task_struct *tsk = current;
+	long timeo = sock_rcvtimeo(sk, flags & O_NONBLOCK);
 	int err;
 
+	if (pn_socket_autobind(sock))
+		return -ENOBUFS;
 	if (len < sizeof(struct sockaddr_pn))
 		return -EINVAL;
 	if (spn->spn_family != AF_PHONET)
@@ -243,82 +246,61 @@ static int pn_socket_connect(struct socket *sock, struct sockaddr *addr,
 
 	switch (sock->state) {
 	case SS_UNCONNECTED:
-		sk->sk_state = TCP_CLOSE;
-		break;
-	case SS_CONNECTING:
-		switch (sk->sk_state) {
-		case TCP_SYN_RECV:
-			sock->state = SS_CONNECTED;
-			err = -EISCONN;
-			goto out;
-		case TCP_CLOSE:
-			err = -EALREADY;
-			if (flags & O_NONBLOCK)
-				goto out;
-			goto wait_connect;
-		}
-		break;
-	case SS_CONNECTED:
-		switch (sk->sk_state) {
-		case TCP_SYN_RECV:
+		if (sk->sk_state != TCP_CLOSE) {
 			err = -EISCONN;
 			goto out;
-		case TCP_CLOSE:
-			sock->state = SS_UNCONNECTED;
-			break;
 		}
 		break;
-	case SS_DISCONNECTING:
-	case SS_FREE:
-		break;
+	case SS_CONNECTING:
+		err = -EALREADY;
+		goto out;
+	default:
+		err = -EISCONN;
+		goto out;
 	}
-	sk->sk_state = TCP_CLOSE;
-	sk_stream_kill_queues(sk);
 
+	pn->dobject = pn_sockaddr_get_object(spn);
+	pn->resource = pn_sockaddr_get_resource(spn);
 	sock->state = SS_CONNECTING;
+
 	err = sk->sk_prot->connect(sk, addr, len);
-	if (err < 0) {
+	if (err) {
 		sock->state = SS_UNCONNECTED;
-		sk->sk_state = TCP_CLOSE;
+		pn->dobject = 0;
 		goto out;
 	}
 
-	err = -EINPROGRESS;
-wait_connect:
-	if (sk->sk_state != TCP_SYN_RECV && (flags & O_NONBLOCK))
-		goto out;
-
-	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
-	release_sock(sk);
+	while (sk->sk_state == TCP_SYN_SENT) {
+		DEFINE_WAIT(wait);
 
-	err = -ERESTARTSYS;
-	timeo = wait_event_interruptible_timeout(*sk_sleep(sk),
-			sk->sk_state != TCP_CLOSE,
-			timeo);
-
-	lock_sock(sk);
-	if (timeo < 0)
-		goto out; /* -ERESTARTSYS */
-
-	err = -ETIMEDOUT;
-	if (timeo == 0 && sk->sk_state != TCP_SYN_RECV)
-		goto out;
+		if (!timeo) {
+			err = -EINPROGRESS;
+			goto out;
+		}
+		if (signal_pending(tsk)) {
+			err = sock_intr_errno(timeo);
+			goto out;
+		}
 
-	if (sk->sk_state != TCP_SYN_RECV) {
-		sock->state = SS_UNCONNECTED;
-		err = sock_error(sk);
-		if (!err)
-			err = -ECONNREFUSED;
-		goto out;
+		prepare_to_wait_exclusive(sk_sleep(sk), &wait,
+						TASK_INTERRUPTIBLE);
+		release_sock(sk);
+		timeo = schedule_timeout(timeo);
+		lock_sock(sk);
+		finish_wait(sk_sleep(sk), &wait);
 	}
-	sock->state = SS_CONNECTED;
-	err = 0;
 
+	if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED))
+		err = 0;
+	else if (sk->sk_state == TCP_CLOSE_WAIT)
+		err = -ECONNRESET;
+	else
+		err = -ECONNREFUSED;
+	sock->state = err ? SS_UNCONNECTED : SS_CONNECTED;
 out:
 	release_sock(sk);
 	return err;
 }
-#endif
 
 static int pn_socket_accept(struct socket *sock, struct socket *newsock,
 				int flags)
@@ -486,11 +468,7 @@ const struct proto_ops phonet_stream_ops = {
 	.owner		= THIS_MODULE,
 	.release	= pn_socket_release,
 	.bind		= pn_socket_bind,
-#ifdef CONFIG_PHONET_PIPECTRLR
 	.connect	= pn_socket_connect,
-#else
-	.connect	= sock_no_connect,
-#endif
 	.socketpair	= sock_no_socketpair,
 	.accept		= pn_socket_accept,
 	.getname	= pn_socket_getname,
-- 
1.7.1


^ permalink raw reply related

* [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier
From: Rémi Denis-Courmont @ 2011-03-04 10:00 UTC (permalink / raw)
  To: netdev
In-Reply-To: <201103031322.37184.remi.denis-courmont@nokia.com>

User-space sometimes needs this information.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 Documentation/networking/phonet.txt |    7 ++++---
 include/linux/phonet.h              |    2 +-
 net/phonet/pep.c                    |   15 +++++++--------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index 24ad2ad..cacac96 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -181,6 +181,10 @@ The pipe protocol provides two socket options at the SOL_PNPIPE level:
     interface index of the network interface created by PNPIPE_ENCAP,
     or zero if encapsulation is off.
 
+  PNPIPE_HANDLE is a read-only integer value. It contains the underlying
+    identifier ("pipe handle") of the pipe. This is only defined for
+    socket descriptors that are already connected or being connected.
+
 
 Phonet Pipe-controller Implementation
 -------------------------------------
@@ -199,9 +203,6 @@ between itself and a remote pipe-end point (e.g. modem).
 
 The implementation adds socket options at SOL_PNPIPE level:
 
- PNPIPE_PIPE_HANDLE
-	It accepts an integer argument for setting value of pipe handle.
-
   PNPIPE_ENABLE accepts one integer value (int). If set to zero, the pipe
     is disabled. If the value is non-zero, the pipe is enabled. If the pipe
     is not (yet) connected, ENOTCONN is error is returned.
diff --git a/include/linux/phonet.h b/include/linux/phonet.h
index 26c8df7..32a0965 100644
--- a/include/linux/phonet.h
+++ b/include/linux/phonet.h
@@ -36,7 +36,7 @@
 /* Socket options for SOL_PNPIPE level */
 #define PNPIPE_ENCAP		1
 #define PNPIPE_IFINDEX		2
-#define PNPIPE_PIPE_HANDLE	3
+#define PNPIPE_HANDLE		3
 #define PNPIPE_ENABLE           4
 /* unused slot */
 
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index c0fab4c..abfb795 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -853,6 +853,7 @@ static int pep_sock_connect(struct sock *sk, struct sockaddr *addr, int len)
 
 	pn->pn_sk.dobject = pn_sockaddr_get_object(spn);
 	pn->pn_sk.resource = pn_sockaddr_get_resource(spn);
+	pn->pipe_handle = 1; /* anything but INVALID_HANDLE */
 	return pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
 					PN_PIPE_DISABLE, data, 4);
 }
@@ -909,14 +910,6 @@ static int pep_setsockopt(struct sock *sk, int level, int optname,
 
 	lock_sock(sk);
 	switch (optname) {
-#ifdef CONFIG_PHONET_PIPECTRLR
-	case PNPIPE_PIPE_HANDLE:
-		if (val) {
-			pn->pipe_handle = val;
-			break;
-		}
-#endif
-
 	case PNPIPE_ENCAP:
 		if (val && val != PNPIPE_ENCAP_IP) {
 			err = -EINVAL;
@@ -982,6 +975,12 @@ static int pep_getsockopt(struct sock *sk, int level, int optname,
 		val = pn->ifindex;
 		break;
 
+	case PNPIPE_HANDLE:
+		val = pn->pipe_handle;
+		if (val == PN_PIPE_INVALID_HANDLE)
+			return -EINVAL;
+		break;
+
 #ifdef CONFIG_PHONET_PIPECTRLR
 	case PNPIPE_ENABLE:
 		val = sk->sk_state == TCP_ESTABLISHED;
-- 
1.7.1


^ permalink raw reply related

* [PATCH v2] tcp: ioctl type SIOCOUTQNSD returns amount of data not sent
From: Steffen Sledz @ 2011-03-04 10:20 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy,
	Mario Schuknecht, Steffen Sledz
In-Reply-To: <201103041057.53671.arnd@arndb.de>

From: Mario Schuknecht <m.schuknecht@dresearch.de>

In contrast to SIOCOUTQ which returns the amount of data sent
but not yet acknowledged plus data not yet sent this patch only
returns the data not sent.

For various methods of live streaming bitrate control it may
be helpful to know how much data are in the tcp outqueue are
not sent yet.

Signed-off-by: Mario Schuknecht <m.schuknecht@dresearch.de>
Signed-off-by: Steffen Sledz <sledz@dresearch.de>
---
 arch/alpha/include/asm/ioctls.h   |    3 ++-
 arch/mips/include/asm/ioctls.h    |    3 ++-
 arch/parisc/include/asm/ioctls.h  |    3 ++-
 arch/powerpc/include/asm/ioctls.h |    3 ++-
 arch/sh/include/asm/ioctls.h      |    3 ++-
 arch/sparc/include/asm/ioctls.h   |    3 ++-
 arch/xtensa/include/asm/ioctls.h  |    3 ++-
 include/asm-generic/ioctls.h      |    3 ++-
 include/linux/sockios.h           |    1 +
 net/ipv4/tcp.c                    |    9 +++++++++
 10 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/arch/alpha/include/asm/ioctls.h b/arch/alpha/include/asm/ioctls.h
index 80e1cee..5ceb2de 100644
--- a/arch/alpha/include/asm/ioctls.h
+++ b/arch/alpha/include/asm/ioctls.h
@@ -35,12 +35,13 @@
 #define TIOCGWINSZ	_IOR('t', 104, struct winsize)
 #define	TIOCSTART	_IO('t', 110)		/* start output, like ^Q */
 #define	TIOCSTOP	_IO('t', 111)		/* stop output, like ^S */
-#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size */
+#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size (not send + not acked) */
 
 #define TIOCGLTC	_IOR('t', 116, struct ltchars)
 #define TIOCSLTC	_IOW('t', 117, struct ltchars)
 #define TIOCSPGRP	_IOW('t', 118, int)
 #define TIOCGPGRP	_IOR('t', 119, int)
+#define TIOCOUTQNSD     _IOR('t', 120, int)     /* output queue size (not send only) */
 
 #define TIOCEXCL	0x540C
 #define TIOCNXCL	0x540D
diff --git a/arch/mips/include/asm/ioctls.h b/arch/mips/include/asm/ioctls.h
index 92403c3..dd8d8c5 100644
--- a/arch/mips/include/asm/ioctls.h
+++ b/arch/mips/include/asm/ioctls.h
@@ -27,7 +27,8 @@
 
 #define TIOCEXCL	0x740d		/* set exclusive use of tty */
 #define TIOCNXCL	0x740e		/* reset exclusive use of tty */
-#define TIOCOUTQ	0x7472		/* output queue size */
+#define TIOCOUTQ	0x7472		/* output queue size (not send + not acked) */
+#define TIOCOUTQNSD	0x7473		/* output queue size (not send only) */
 #define TIOCSTI		0x5472		/* simulate terminal input */
 #define TIOCMGET	0x741d		/* get all modem bits */
 #define TIOCMBIS	0x741b		/* bis modem bits */
diff --git a/arch/parisc/include/asm/ioctls.h b/arch/parisc/include/asm/ioctls.h
index 054ec06..1cf1681 100644
--- a/arch/parisc/include/asm/ioctls.h
+++ b/arch/parisc/include/asm/ioctls.h
@@ -21,7 +21,7 @@
 #define TIOCSCTTY	0x540E
 #define TIOCGPGRP	_IOR('T', 30, int)
 #define TIOCSPGRP	_IOW('T', 29, int)
-#define TIOCOUTQ	0x5411
+#define TIOCOUTQ	0x5411 /* output queue size (not send + not acked) */
 #define TIOCSTI		0x5412
 #define TIOCGWINSZ	0x5413
 #define TIOCSWINSZ	0x5414
@@ -55,6 +55,7 @@
 #define TIOCGDEV	_IOR('T',0x32, int)  /* Get primary device node of /dev/console */
 #define TIOCSIG		_IOW('T',0x36, int)  /* Generate signal on Pty slave */
 #define TIOCVHANGUP	0x5437
+#define TIOCOUTQNSD	0x5438 /* output queue size (not send only) */
 
 #define FIONCLEX	0x5450  /* these numbers need to be adjusted. */
 #define FIOCLEX		0x5451
diff --git a/arch/powerpc/include/asm/ioctls.h b/arch/powerpc/include/asm/ioctls.h
index e9b7887..ae1b21e 100644
--- a/arch/powerpc/include/asm/ioctls.h
+++ b/arch/powerpc/include/asm/ioctls.h
@@ -35,12 +35,13 @@
 #define TIOCGWINSZ	_IOR('t', 104, struct winsize)
 #define	TIOCSTART	_IO('t', 110)		/* start output, like ^Q */
 #define	TIOCSTOP	_IO('t', 111)		/* stop output, like ^S */
-#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size */
+#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size (not send + not acked) */
 
 #define TIOCGLTC	_IOR('t', 116, struct ltchars)
 #define TIOCSLTC	_IOW('t', 117, struct ltchars)
 #define TIOCSPGRP	_IOW('t', 118, int)
 #define TIOCGPGRP	_IOR('t', 119, int)
+#define TIOCOUTQNSD     _IOR('t', 120, int)     /* output queue size (not send only) */
 
 #define TIOCEXCL	0x540C
 #define TIOCNXCL	0x540D
diff --git a/arch/sh/include/asm/ioctls.h b/arch/sh/include/asm/ioctls.h
index a6769f3..f0bd75a 100644
--- a/arch/sh/include/asm/ioctls.h
+++ b/arch/sh/include/asm/ioctls.h
@@ -29,7 +29,8 @@
 #define TIOCGWINSZ	0x80087468	/* _IOR('t', 104, struct winsize) */
 #define	TIOCSTART	_IO('t', 110)		/* start output, like ^Q */
 #define	TIOCSTOP	_IO('t', 111)		/* stop output, like ^S */
-#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size */
+#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size (not send + not acked) */
+#define TIOCOUTQNSD     _IOR('t', 116, int)     /* output queue size (not send only) */
 
 #define TIOCSPGRP	_IOW('t', 118, int)
 #define TIOCGPGRP	_IOR('t', 119, int)
diff --git a/arch/sparc/include/asm/ioctls.h b/arch/sparc/include/asm/ioctls.h
index 28d0c8b..184acc3 100644
--- a/arch/sparc/include/asm/ioctls.h
+++ b/arch/sparc/include/asm/ioctls.h
@@ -60,7 +60,7 @@
 #define TIOCPKT		_IOW('t', 112, int)
 #define TIOCNOTTY	_IO('t', 113)
 #define TIOCSTI		_IOW('t', 114, char)
-#define TIOCOUTQ	_IOR('t', 115, int)
+#define TIOCOUTQ	_IOR('t', 115, int) /* output queue size (not send + not acked) */
 #define __TIOCGLTC        _IOR('t', 116, struct ltchars) /* SunOS Specific */
 #define __TIOCSLTC        _IOW('t', 117, struct ltchars) /* SunOS Specific */
 /* 118 is the non-posix setpgrp tty ioctl */
@@ -83,6 +83,7 @@
 #define TIOCGPTN	_IOR('t', 134, unsigned int) /* Get Pty Number */
 #define TIOCSPTLCK	_IOW('t', 135, int) /* Lock/unlock PTY */
 #define TIOCSIG		_IOW('t', 136, int) /* Generate signal on Pty slave */
+#define TIOCOUTQNSD	_IOW('t', 137, int) /* output queue size (not send only) */
 
 /* Little f */
 #define FIOCLEX		_IO('f', 1)
diff --git a/arch/xtensa/include/asm/ioctls.h b/arch/xtensa/include/asm/ioctls.h
index fd1d136..5222d8c 100644
--- a/arch/xtensa/include/asm/ioctls.h
+++ b/arch/xtensa/include/asm/ioctls.h
@@ -41,7 +41,8 @@
 #define TIOCGWINSZ	_IOR('t', 104, struct winsize)
 #define	TIOCSTART	_IO('t', 110)		/* start output, like ^Q */
 #define	TIOCSTOP	_IO('t', 111)		/* stop output, like ^S */
-#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size */
+#define TIOCOUTQ        _IOR('t', 115, int)     /* output queue size (not send + not acked) */
+#define TIOCOUTQNSD     _IOR('t', 116, int)     /* output queue size (not send only) */
 
 #define TIOCSPGRP	_IOW('t', 118, int)
 #define TIOCGPGRP	_IOR('t', 119, int)
diff --git a/include/asm-generic/ioctls.h b/include/asm-generic/ioctls.h
index 199975f..231e492 100644
--- a/include/asm-generic/ioctls.h
+++ b/include/asm-generic/ioctls.h
@@ -32,7 +32,7 @@
 #define TIOCSCTTY	0x540E
 #define TIOCGPGRP	0x540F
 #define TIOCSPGRP	0x5410
-#define TIOCOUTQ	0x5411
+#define TIOCOUTQ	0x5411 /* output queue size (not send + not acked) */
 #define TIOCSTI		0x5412
 #define TIOCGWINSZ	0x5413
 #define TIOCSWINSZ	0x5414
@@ -74,6 +74,7 @@
 #define TCSETXW		0x5435
 #define TIOCSIG		_IOW('T', 0x36, int)  /* pty: generate signal */
 #define TIOCVHANGUP	0x5437
+#define TIOCOUTQNSD	0x5438 /* output queue size (not send only) */
 
 #define FIONCLEX	0x5450
 #define FIOCLEX		0x5451
diff --git a/include/linux/sockios.h b/include/linux/sockios.h
index 241f179..4c5ca47 100644
--- a/include/linux/sockios.h
+++ b/include/linux/sockios.h
@@ -23,6 +23,7 @@
 /* Linux-specific socket ioctls */
 #define SIOCINQ		FIONREAD
 #define SIOCOUTQ	TIOCOUTQ
+#define SIOCOUTQNSD	TIOCOUTQNSD
 
 /* Routing table calls. */
 #define SIOCADDRT	0x890B		/* add routing table entry	*/
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index a17a5a7..b22d450 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -505,6 +505,15 @@ int tcp_ioctl(struct sock *sk, int cmd, unsigned long arg)
 		else
 			answ = tp->write_seq - tp->snd_una;
 		break;
+	case SIOCOUTQNSD:
+		if (sk->sk_state == TCP_LISTEN)
+			return -EINVAL;
+
+		if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV))
+			answ = 0;
+		else
+			answ = tp->write_seq - tp->snd_nxt;
+		break;
 	default:
 		return -ENOIOCTLCMD;
 	}
-- 
1.7.1


^ permalink raw reply related

* Re: [RFC] Keep track of interrupt URB status and resubmit in bh if necessary
From: Indrek Peri @ 2011-03-04 11:01 UTC (permalink / raw)
  To: Paul Stewart; +Cc: netdev@vger.kernel.org, dbrownell@users.sourceforge.org
In-Reply-To: <20110303055828.073E520218@glenhelen.mtv.corp.google.com>


To my understanding usbnet has design questions, like EVENT_DEV_ASLEEP,
device states and events. In struct "flags" member holds an event to
execute deferred work. Actually, in usbnet_bh, EVENT_DEV_ASLEEP is
really needed in if-statement. Without that, in selective suspend
case driver crashes.

Another problem was that "suspend" removed interrupt URB. I tought in
the same way as Paul, I added insertion of interrupt URB. Paul added
it in usbnet_bh and I in "resume". I do not see a problem to add
interrupt URB in "resume". My patch had a typo, in "resume" we
should use GFP_ATOMIC in usb_submit_urb. Now is the question is
this good design? My interpretation of David is that driver needs
"resume driver" transition where RX and INT URBs are activated.

I guess that usbnet needs a redesign and rewriting. 
usbnet is used by many USB-Ethernet devices that actually do not
use selective suspend.

BR, Indrek



On 03/03/2011 06:45 AM, Paul Stewart wrote:
> It appears that a patch from Indrek Peri similar to the one below
> without resolution.  I'm new to this problem (suspend-resume causing
> interrupt URBs to stop delivering) and am curious about what the correct
> solution would should like.  Before becoming aware of this thread, I
> just added a "usb_submit_urb" of "dev->interrupt" into "usbnet_resume()"
> and that worked to solve the issue I was having.  Apparently this isn't
> the correct solution though, from David's response to Indrek.  So, I'm
> curious about what the right code should be.
> 
> I'll note is that submitting the interrupt URB seems fairly benign.  If
> we are in a situation where we should not have sent an URB (e.g, the
> netif wasn't running) intr_complete correctly handles this case and does
> not re-submit the URB, so at most we get one "rogue" interrupt URB after
> resume-from-suspend.  The only nasty thing is that this URB should
> probably not be submitted from interrupt, which the resume function
> almost certainly is.  I'm guessing this is part of why David NAKed
> Indrek's patch.  Am I correct?
> 
> Does something like the patch below seem like a resonable solution?
> 
> Cc: David Brownell <dbrownell@users.sourceforge.org>
> Cc: Indrek Peri <Indrek.Peri@Ericsson.com>
> Signed-off-by: Paul Stewart <pstew@google.com>
> ---
>  drivers/net/usb/usbnet.c   |   21 +++++++++++++++++++--
>  include/linux/usb/usbnet.h |    1 +
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 02d25c7..bc6a8e0 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -471,6 +471,8 @@ static void intr_complete (struct urb *urb)
>  	struct usbnet	*dev = urb->context;
>  	int		status = urb->status;
>  
> +	dev->interrupt_urb_running = 0;
> +
>  	switch (status) {
>  	/* success */
>  	case 0:
> @@ -497,7 +499,9 @@ static void intr_complete (struct urb *urb)
>  
>  	memset(urb->transfer_buffer, 0, urb->transfer_buffer_length);
>  	status = usb_submit_urb (urb, GFP_ATOMIC);
> -	if (status != 0 && netif_msg_timer (dev))
> +	if (status == 0)
> +		dev->interrupt_urb_running = 1;
> +	else if (netif_msg_timer (dev))
>  		deverr(dev, "intr resubmit --> %d", status);
>  }
>  
> @@ -580,6 +584,7 @@ static int usbnet_stop (struct net_device *net)
>  	remove_wait_queue (&unlink_wakeup, &wait);
>  
>  	usb_kill_urb(dev->interrupt);
> +	dev->interrupt_urb_running = 0;
>  
>  	/* deferred work (task, timer, softirq) must also stop.
>  	 * can't flush_scheduled_work() until we drop rtnl (later),
> @@ -640,7 +645,8 @@ static int usbnet_open (struct net_device *net)
>  			if (netif_msg_ifup (dev))
>  				deverr (dev, "intr submit %d", retval);
>  			goto done;
> -		}
> +		} else
> +			dev->interrupt_urb_running = 1;
>  	}
>  
>  	netif_start_queue (net);
> @@ -1065,6 +1071,17 @@ static void usbnet_bh (unsigned long param)
>  		if (dev->txq.qlen < TX_QLEN (dev))
>  			netif_wake_queue (dev->net);
>  	}
> +
> +	// Do we need to re-enable interrupt URBs?
> +	if (netif_running (dev->net) &&
> +	    netif_device_present (dev->net) &&
> +	    dev->interrupt_urb_running == 0) {
> +		usb_submit_urb (dev->interrupt, GFP_KERNEL);
> +
> +		/* Unconditionally mark as running so we don't retry */
> +		dev->interrupt_urb_running = 1;
> +	}
> +
>  }
>  
>  
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index ba09fe8..1b8ed8a 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -49,6 +49,7 @@ struct usbnet {
>  	u32			hard_mtu;	/* count any extra framing */
>  	size_t			rx_urb_size;	/* size for rx urbs */
>  	struct mii_if_info	mii;
> +	int			interrupt_urb_running;
>  
>  	/* various kinds of pending driver work */
>  	struct sk_buff_head	rxq;


^ permalink raw reply

* [PATCH] s2io: fix uninitialized compile warning
From: Shan Wei @ 2011-03-04 11:23 UTC (permalink / raw)
  To: ramkrishna.vepa, sivakumar.subramani, sreenivasa.honnur,
	jon.mason, netdev


drivers/net/s2io.c:7559: warning: ‘tcp_len’ may be used uninitialized in this function

Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 drivers/net/s2io.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 39c17ce..2ad6364 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -7556,7 +7556,7 @@ static int rx_osm_handler(struct ring_info *ring_data, struct RxD_t * rxdp)
 			 */
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			if (ring_data->lro) {
-				u32 tcp_len;
+				u32 tcp_len = 0;
 				u8 *tcp;
 				int ret = 0;
 
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH 0/3] [RFC] Implement multiqueue (RX & TX) virtio-net
From: Krishna Kumar2 @ 2011-03-04 12:22 UTC (permalink / raw)
  To: habanero
  Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, mst, netdev,
	rusty
In-Reply-To: <1299178884.13202.50.camel@localhost.localdomain>

Andrew Theurer <habanero@linux.vnet.ibm.com> wrote on 03/04/2011 12:31:24
AM:

Hi Andrew,

> > _______________________________________________________________________
> >                 TCP: Guest -> Local Host (TCP_STREAM)
> >                  TCP: Local Host -> Guest (TCP_MAERTS)
> >             UDP: Local Host -> Guest (UDP_STREAM)
>
>
> Any reason why the tests don't include a guest-to-guest on same host, or
> on different hosts?  Seems like those would be a lot more common that
> guest-to/from-localhost.

This was missing in my test plan, but good point. I will run
these tests also and send the results soon.

Thanks,

- KK


^ permalink raw reply

* Re: [PATCH v2] tcp: ioctl type SIOCOUTQNSD returns amount of data not sent
From: Steffen Sledz @ 2011-03-04 12:26 UTC (permalink / raw)
  To: Alan Cox
  Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, Mario Schuknecht, Steffen Sledz
In-Reply-To: <1299234031-13937-1-git-send-email-sledz@dresearch.de>

Am 04.03.2011 12:49, schrieb Alan Cox:
> ...
>>> Signed-off-by: Mario Schuknecht <m.schuknecht@dresearch.de>
>>> ---
>>>  include/asm-generic/ioctls.h |    1 +
>>>  include/linux/sockios.h      |    1 +
>>>  net/ipv4/tcp.c               |    9 +++++++++
>>>  3 files changed, 11 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/asm-generic/ioctls.h b/include/asm-generic/ioctls.h
>>> index a321665..b87115b 100644
>>> --- a/include/asm-generic/ioctls.h
>>> +++ b/include/asm-generic/ioctls.h
>>> @@ -72,6 +72,7 @@
>>>  #define TCSETXF		0x5434
>>>  #define TCSETXW		0x5435
>>>  #define TIOCSIG		_IOW('T', 0x36, int)  /* pty: generate signal */
>>> +#define TIOCOUTQNSD	0x5437
>
> These are tty not socket ioctl codes. For various historical reasons some
> of them got sort of emulated by the socket layer in Unix and thus in
> Linux.
>
> If you plan to add one to Linux then please post the accompanying *tty
> layer* patches and reasoning, otherwise if it is socket only please stick
> to the socket ioctl.

So we should define the new SIOCOUTQNSD in sockios.h only and not similiar to SIOCOUTQ using TIOCOUTQ. Right?

Steffen

-- 
DResearch Fahrzeugelektronik GmbH
Otto-Schmirgal-Str. 3, 10319 Berlin, Germany
Tel: +49 30 515932-237 mailto:sledz@DResearch.de
Fax: +49 30 515932-299
Geschäftsführer: Dr. Michael Weber, Werner Mögle;
Amtsgericht Berlin Charlottenburg; HRB 130120 B;
Ust.-IDNr. DE273952058

^ permalink raw reply

* Re: [PATCH v2] tcp: ioctl type SIOCOUTQNSD returns amount of data not sent
From: Alan Cox @ 2011-03-04 12:39 UTC (permalink / raw)
  To: Steffen Sledz
  Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, Mario Schuknecht
In-Reply-To: <4D70DA8D.9070603@dresearch.de>

> > If you plan to add one to Linux then please post the accompanying *tty
> > layer* patches and reasoning, otherwise if it is socket only please stick
> > to the socket ioctl.
> 
> So we should define the new SIOCOUTQNSD in sockios.h only and not similiar to SIOCOUTQ using TIOCOUTQ. Right?

Right

Alan

^ permalink raw reply

* Re: Jetway JAD3RTLANG, Daughter Board, 3x GigaBit LAN does not work correctly
From: Markus Feldmann @ 2011-03-04 12:43 UTC (permalink / raw)
  To: netdev
In-Reply-To: <ikovkn$bn8$1@dough.gmane.org>

Here is the dmesg:
http://pastebin.com/PVJdq5hE


^ permalink raw reply

* Re: [PATCH] sctp: do not mark chunk abandoned if peer has no PRSCTP capable
From: Vlad Yasevich @ 2011-03-04 13:08 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: netdev@vger.kernel.org, lksctp, David Miller
In-Reply-To: <4D707442.1090903@cn.fujitsu.com>

On Fri, 2011-03-04 at 13:10 +0800, Wei Yongjun wrote:
> > On 03/02/2011 11:20 PM, Wei Yongjun wrote:
> >> Chunk is marked abandoned if the chunk is expires, and it not be
> >> retransmited even if the peer has no PRSCTP capable, but the peer
> >> will still wait for retransmit it to update CTSN.
> >> This patch disable mark chunk abandoned if peer has no PRSCTP
> >> capable.
> >>
> >> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> >> ---
> >>  net/sctp/chunk.c |    3 +++
> >>  1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> >> index 6c85564..0d4832d 100644
> >> --- a/net/sctp/chunk.c
> >> +++ b/net/sctp/chunk.c
> >> @@ -347,6 +347,9 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
> >>  {
> >>  	struct sctp_datamsg *msg = chunk->msg;
> >>  
> >> +	if (!chunk->asoc->peer.prsctp_capable)
> >> +		return 0;
> >> +
> >>  	if (!msg->can_abandon)
> >>  		return 0;
> >>  
> > The trouble is that timetolive can be set on a message independent of Partial Reliability.
> > The difference in behavior is that when PR can't be used, a chunk can only be abandoned
> > if it has not yet been transmitted.  With PR enabled, the chunk can be abandoned at any time.
> >
> > So, you can't blindly disallow abandonment. 
> 
> But, how can we do PR if peer has no PRSCTP capable?

That's already taken care of.  We report the message as unsent.  When
PRSCTP is disabled, you may only abandon messages/chunks that have not
been transmitted (assigned a TSN).

> Return error to application in sendmsg()? If so, how we check this if there is no asoc?

How would you have data without an association?

-vlad

> 
> 
> > -vlad
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 




^ permalink raw reply

* Re: Jetway JAD3RTLANG, Daughter Board, 3x GigaBit LAN does not work correctly
From: Markus Feldmann @ 2011-03-04 13:17 UTC (permalink / raw)
  To: netdev
In-Reply-To: <ikqmod$hul$1@dough.gmane.org>

Yesterday my daughterboard made trouble, today it works. I can change 
the ethernet port or connect two clients at the same time. The dmesg 
from today is showing the working state. I will post a dmesg when it 
does not work.

regards Markus


^ permalink raw reply

* Re: [PATCH] sctp: fix the fast retransmit limit
From: Vlad Yasevich @ 2011-03-04 14:09 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: David Miller, netdev@vger.kernel.org, lksctp, Mingyuan Zhu,
	Neil Horman
In-Reply-To: <4D707CB6.8020502@cn.fujitsu.com>

On Fri, 2011-03-04 at 13:46 +0800, Wei Yongjun wrote:
> > On 03/03/2011 04:29 AM, Wei Yongjun wrote:
> >> If chunk is still lost after fast retransmit, SCTP stack will
> >> never allow the second fast retransmit of this chunk, even if
> >> the peer need we do this. This chunk will be retransmit until
> >> the rtx timeout. This limit is introduce by the following patch:
> >>   sctp: reduce memory footprint of sctp_chunk structure
> >>   (c226ef9b83694311327f3ab0036c6de9c22e9daf)
> >>
> >> This patch revert this limit and removed useless SCTP_DONT_FRTX.
> > NACK.  Please read the spec and how fast recovery is specified.
> 
> 
> RFC 7.2.4. Fast Retransmit on Gap Reports said:
> 5) Mark the DATA chunk(s) as being fast retransmitted and thus
> ineligible for a subsequent Fast Retransmit. Those TSNs marked
> for retransmission due to the Fast-Retransmit algorithm that did
> not fit in the sent datagram carrying K other TSNs are also
> marked as ineligible for a subsequent Fast Retransmit. However,
> as they are marked for retransmission they will be retransmitted
> later on as soon as cwnd allows.
> 
> So we can treat this chunk can be Fast Retransmit again if not in
> fast recovery?

Right, but you are in Fast Recovery until Cumulative TSN moves
past fast recovery exit point.  In other words, you can't exit
fast recovery until the missing chunk has been acknowledged.

The algorithm states that you can Fast Retransmit a chunk once.
If the Fast RTX chunk is lost, you revert to time-outs for that chunk.

Now, there are some improvements to this algorithm that have been
researched, but not standardized.  One such improvement is to allow
subsequent FAST-RTXs once the New TSN in SACK reaches the fast recovery
exit point (meaning, that all in-flight data has been SACKed, but the
chunk is still missing).  Once that happens, you can start counting
misses again and FAST-RTX when 3 more misses occur.  This will improve
the performance of FAST-RTX.

The reason we don't automatically allow additional miss indications to
count is because you may have a lot of chunks in flight.  That means
that you end up doing FAST-RTX too often.  The idea is to allow the
retransmitted chunk to get there.

-vlad
> 
> > -vlad
> >
> >> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> >> ---
> >>  include/net/sctp/structs.h |    1 -
> >>  net/sctp/outqueue.c        |    4 ++--
> >>  2 files changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> >> index cc9185c..82a0f84 100644
> >> --- a/include/net/sctp/structs.h
> >> +++ b/include/net/sctp/structs.h
> >> @@ -751,7 +751,6 @@ struct sctp_chunk {
> >>  
> >>  #define SCTP_CAN_FRTX 0x0
> >>  #define SCTP_NEED_FRTX 0x1
> >> -#define SCTP_DONT_FRTX 0x2
> >>  	__u16	rtt_in_progress:1,	/* This chunk used for RTT calc? */
> >>  		has_tsn:1,		/* Does this chunk have a TSN yet? */
> >>  		has_ssn:1,		/* Does this chunk have a SSN yet? */
> >> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> >> index 8c6d379..7ed5862 100644
> >> --- a/net/sctp/outqueue.c
> >> +++ b/net/sctp/outqueue.c
> >> @@ -657,7 +657,7 @@ redo:
> >>  			 * after it is retransmitted.
> >>  			 */
> >>  			if (chunk->fast_retransmit == SCTP_NEED_FRTX)
> >> -				chunk->fast_retransmit = SCTP_DONT_FRTX;
> >> +				chunk->fast_retransmit = SCTP_CAN_FRTX;
> >>  
> >>  			q->empty = 0;
> >>  			break;
> >> @@ -679,7 +679,7 @@ redo:
> >>  	if (rtx_timeout || fast_rtx) {
> >>  		list_for_each_entry(chunk1, lqueue, transmitted_list) {
> >>  			if (chunk1->fast_retransmit == SCTP_NEED_FRTX)
> >> -				chunk1->fast_retransmit = SCTP_DONT_FRTX;
> >> +				chunk1->fast_retransmit = SCTP_CAN_FRTX;
> >>  		}
> >>  	}
> >>  
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 




^ permalink raw reply

* [PATCH net-next-2.6] inetpeer: seqlock optimization
From: Eric Dumazet @ 2011-03-04 15:09 UTC (permalink / raw)
  To: David Miller; +Cc: xiaosuo, netdev
In-Reply-To: <20110303.003229.229764095.davem@davemloft.net>

Le jeudi 03 mars 2011 à 00:32 -0800, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 03 Mar 2011 08:39:37 +0100
> 
> > Le mercredi 02 mars 2011 à 22:42 -0800, David Miller a écrit :
> >> Actually, back to the original topic, I wonder how bad it is to simply
> >> elide the recheck in the create==0 case anyways.  Except for the ipv4
> >> fragmentation wraparound protection values, perfect inetpeer finding
> >> is not necessary for correctness.  And IPv4 fragmentation always calls
> >> inetpeer with create!=0.
> > 
> > We could use a seqlock, to detect that a writer might have changed
> > things while we did our RCU lookup ?
> 
> That would certainly work.

Here is a patch to implement this idea.

Thanks !

[PATCH net-next-2.6] inetpeer: seqlock optimization

David noticed :

------------------
Eric, I was profiling the non-routing-cache case and something that
stuck out is the case of calling inet_getpeer() with create==0.

If an entry is not found, we have to redo the lookup under a spinlock
to make certain that a concurrent writer rebalancing the tree does
not "hide" an existing entry from us.

This makes the case of a create==0 lookup for a not-present entry
really expensive.  It is on the order of 600 cpu cycles on my
Niagara2.

I added a hack to not do the relookup under the lock when create==0
and it now costs less than 300 cycles.

This is now a pretty common operation with the way we handle COW'd
metrics, so I think it's definitely worth optimizing.
-----------------

One solution is to use a seqlock instead of a spinlock to protect struct
inet_peer_base.

After a failed avl tree lookup, we can easily detect if a writer did
some changes during our lookup. Taking the lock and redo the lookup is
only necessary in this case.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/inetpeer.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 48f8d45..7fd9fab 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -81,19 +81,19 @@ static const struct inet_peer peer_fake_node = {
 
 struct inet_peer_base {
 	struct inet_peer __rcu *root;
-	spinlock_t	lock;
+	seqlock_t	lock;
 	int		total;
 };
 
 static struct inet_peer_base v4_peers = {
 	.root		= peer_avl_empty_rcu,
-	.lock		= __SPIN_LOCK_UNLOCKED(v4_peers.lock),
+	.lock		= __SEQLOCK_UNLOCKED(v4_peers.lock),
 	.total		= 0,
 };
 
 static struct inet_peer_base v6_peers = {
 	.root		= peer_avl_empty_rcu,
-	.lock		= __SPIN_LOCK_UNLOCKED(v6_peers.lock),
+	.lock		= __SEQLOCK_UNLOCKED(v6_peers.lock),
 	.total		= 0,
 };
 
@@ -372,7 +372,7 @@ static void unlink_from_pool(struct inet_peer *p, struct inet_peer_base *base)
 
 	do_free = 0;
 
-	spin_lock_bh(&base->lock);
+	write_seqlock_bh(&base->lock);
 	/* Check the reference counter.  It was artificially incremented by 1
 	 * in cleanup() function to prevent sudden disappearing.  If we can
 	 * atomically (because of lockless readers) take this last reference,
@@ -409,7 +409,7 @@ static void unlink_from_pool(struct inet_peer *p, struct inet_peer_base *base)
 		base->total--;
 		do_free = 1;
 	}
-	spin_unlock_bh(&base->lock);
+	write_sequnlock_bh(&base->lock);
 
 	if (do_free)
 		call_rcu_bh(&p->rcu, inetpeer_free_rcu);
@@ -477,12 +477,16 @@ struct inet_peer *inet_getpeer(struct inetpeer_addr *daddr, int create)
 	struct inet_peer __rcu **stack[PEER_MAXDEPTH], ***stackptr;
 	struct inet_peer_base *base = family_to_base(daddr->family);
 	struct inet_peer *p;
+	unsigned int sequence;
+	int invalidated;
 
 	/* Look up for the address quickly, lockless.
 	 * Because of a concurrent writer, we might not find an existing entry.
 	 */
 	rcu_read_lock_bh();
+	sequence = read_seqbegin(&base->lock);
 	p = lookup_rcu_bh(daddr, base);
+	invalidated = read_seqretry(&base->lock, sequence);
 	rcu_read_unlock_bh();
 
 	if (p) {
@@ -493,14 +497,18 @@ struct inet_peer *inet_getpeer(struct inetpeer_addr *daddr, int create)
 		return p;
 	}
 
+	/* If no writer did a change during our lookup, we can return early. */
+	if (!create && !invalidated)
+		return NULL;
+
 	/* retry an exact lookup, taking the lock before.
 	 * At least, nodes should be hot in our cache.
 	 */
-	spin_lock_bh(&base->lock);
+	write_seqlock_bh(&base->lock);
 	p = lookup(daddr, stack, base);
 	if (p != peer_avl_empty) {
 		atomic_inc(&p->refcnt);
-		spin_unlock_bh(&base->lock);
+		write_sequnlock_bh(&base->lock);
 		/* Remove the entry from unused list if it was there. */
 		unlink_from_unused(p);
 		return p;
@@ -524,7 +532,7 @@ struct inet_peer *inet_getpeer(struct inetpeer_addr *daddr, int create)
 		link_to_pool(p, base);
 		base->total++;
 	}
-	spin_unlock_bh(&base->lock);
+	write_sequnlock_bh(&base->lock);
 
 	if (base->total >= inet_peer_threshold)
 		/* Remove one less-recently-used entry. */



^ permalink raw reply related

* Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Narendra_K @ 2011-03-04 16:28 UTC (permalink / raw)
  To: linux-pci, linux-hotplug
  Cc: netdev, mjg, Matt_Domsch, Charles_Rose, Jordan_Hargrave,
	Shyam_Iyer, sfr
In-Reply-To: <20110302172508.GA2794@fedora14-r610.oslab.blr.amer.dell.com>

On Wed, Mar 02, 2011 at 10:34:17PM +0530, K, Narendra wrote:
> On Wed, Feb 23, 2011 at 06:06:42PM +0530, K, Narendra wrote:
> > Hello,
> >
> > This patch exports ACPI _DSM provided firmware instance number and
> > string name to sysfs.
> >
> > V1 -> V2:
> > The attribute 'index' is changed to 'acpi_index' as the semantics of
> > SMBIOS provided device type instance and ACPI _DSM provided firmware
> > instance number are different.
> >
> > V2 -> V3:
> > Matthew Garrett pointed out that 'sysfs_create_groups' does return an
> > error when there are no ACPI _DSM attributes available and because of
> > that the fallback to SMBIOS will not happen. As a result SMBIOS provided
> > attributes are not created. This version of the patch addresses the issue.
> >
> 
> V3 -> V4:
> Select NLS if (DMI || ACPI) in drivers/pci/Kconfig to prevent build
> breakage from an 'allmodconfig'
> 
> Matthew,
> Thanks for the suggestion.
> 
> From: Narendra K <narendra_k@dell.com>
> Subject: [PATCH] Export ACPI _DSM provided firmware instance number and string to sysfs
>

Hi Jesse,

Does Version 4 of the patch look good for inclusion ? Please let us know
if there are any concerns.

With regards,
Narendra K 

^ permalink raw reply

* Re: [PATCH] net: Enter net/ipv6/ even if CONFIG_IPV6=n
From: Randy Dunlap @ 2011-03-04 17:01 UTC (permalink / raw)
  To: davem, Patrick McHardy, Stephen Rothwell, netdev, linux-next
In-Reply-To: <20110304093507.GG10761@canuck.infradead.org>

On 03/04/11 01:35, Thomas Graf wrote:
> exthdrs_core.c and addrconf_core.c in net/ipv6/ contain bits which
> must be made available even if IPv6 is disabled.
> 
> net/ipv6/Makefile already correctly includes them if CONFIG_IPV6=n
> but net/Makefile prevents entering the subdirectory.
> 
> Signed-off-by: Thomas Graf <tgraf@infradead.org>

Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

Thanks.

> 
> diff --git a/net/Makefile b/net/Makefile
> index a3330eb..a51d946 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -19,9 +19,7 @@ obj-$(CONFIG_NETFILTER)		+= netfilter/
>  obj-$(CONFIG_INET)		+= ipv4/
>  obj-$(CONFIG_XFRM)		+= xfrm/
>  obj-$(CONFIG_UNIX)		+= unix/
> -ifneq ($(CONFIG_IPV6),)
> -obj-y				+= ipv6/
> -endif
> +obj-$(CONFIG_NET)		+= ipv6/
>  obj-$(CONFIG_PACKET)		+= packet/
>  obj-$(CONFIG_NET_KEY)		+= key/
>  obj-$(CONFIG_BRIDGE)		+= bridge/


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* Re: [PATCH] sched: QFQ - quick fair queue scheduler (v4)
From: Stephen Hemminger @ 2011-03-04 17:17 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Fabio Checconi, Luigi Rizzo, netdev
In-Reply-To: <1299221426.2547.47.camel@edumazet-laptop>

On Fri, 04 Mar 2011 07:50:26 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le jeudi 03 mars 2011 à 16:30 -0800, Stephen Hemminger a écrit :
> > This is an implementation of the Quick Fair Queue scheduler developed
> > by Fabio Checconi. The same algorithm is already implemented in ipfw
> > in FreeBSD. Fabio had an earlier version developed on Linux, I just
> > cleaned it up.  Thanks to Eric Dumazet for doing the testing and
> > finding bugs.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > 
> > ---
> > v4 - change slots[] to hlist from simple linked list
> > 
> >  include/linux/pkt_sched.h |   15 
> >  net/sched/Kconfig         |   11 
> >  net/sched/Makefile        |    1 
> >  net/sched/sch_qfq.c       | 1133 ++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 1160 insertions(+)
> 
> Thanks
> 
> Still crashing hard here in qfq_reset_qdisc(), when packets are present
> in queues.
> 
> Probably hlist_for_each_entry_safe() is needed, since
> qfq_deactivate_class() is called ...
> 
> Also rename cl->next to cl->hnode so that following is clearer ?
> 
> -	hlist_for_each_entry(cl, n, &grp->slots[j], next) {
> +	hlist_for_each_entry_safe(cl, n, next, &grp->slots[j], hnode) {
> 
> Fabio, any idea why everything is blocked after a few packets for me ?
> 
> Here is script to reproduce the problem :
> 
> # cat qfq_setup.sh
> modprobe dummy
> 
> ifconfig dummy0 10.2.2.254 netmask 255.255.255.0 up
> 
> for i in `seq 1 16`
> do
>  arp -H ether -i dummy0 -s 10.2.2.$i 00:00:0c:07:ac:$(printf %02x $i)
> done
> 
> DEV=dummy0
> RATE="rate 40Mbit"
> TNETS="10.2.2.0/25"
> ALLOT="allot 20000"
> 
> tc qdisc del dev dummy0 root 2>/dev/null
> 
> tc qdisc add dev $DEV root handle 1: cbq avpkt 1000 rate 1000Mbit \
> 	bandwidth 1000Mbit
> tc class add dev $DEV parent 1: classid 1:1 \
> 	est 1sec 8sec cbq allot 10000 mpu 64 \
> 	rate 1000Mbit prio 1 avpkt 1500 bounded
> 
> # output to test nets :  40 Mbit limit
> tc class add dev $DEV parent 1:1 classid 1:11 \
> 	est 1sec 8sec cbq $ALLOT mpu 64      \
> 	$RATE prio 2 avpkt 1400 bounded
> 
> tc qdisc add dev $DEV parent 1:11 handle 11:  \
> 	est 1sec 8sec qfq
> 
> tc filter add dev $DEV protocol ip parent 11: handle 3 \
> 	flow hash keys rxhash divisor 8
> 
> for i in `seq 1 8`
> do
>  classid=11:$(printf %x $i)
>  tc class add dev $DEV classid $classid qfq 
>  tc qdisc add dev $DEV parent $classid pfifo limit 30
> done
> 
> 
> for privnet in $TNETS
> do
> 	tc filter add dev $DEV parent 1: protocol ip prio 100 u32 \
> 		match ip dst $privnet flowid 1:11
> done
> 
> tc filter add dev $DEV parent 1: protocol ip prio 100 u32 \
> 	match ip protocol 0 0x00 flowid 1:1
> 
> 
> iperf -u -c 10.2.2.1 -P 32 -l 50
> 
> 
> ------------------------------------------------------------------------------
> 
> After a run I get only 5 packets sent, 240 queued in backlog, and all
> other packets dropped.
> 
> # tc -s -d qdisc show dev dummy0
> qdisc cbq 1: root refcnt 2 rate 1000Mbit cell 8b (bounded,isolated) prio
> no-transmit/8 weight 1000Mbit allot 1514b 
> level 2 ewma 5 avpkt 1000b maxidle 0us 
>  Sent 460 bytes 5 pkt (dropped 198800, overlimits 199043 requeues 0) 
>  backlog 0b 240p requeues 0 
>   borrowed 0 overactions 0 avgidle 125 undertime 0
> qdisc qfq 11: parent 1:11 
>  Sent 460 bytes 5 pkt (dropped 198800, overlimits 0 requeues 0) 
>  rate 0bit 0pps backlog 0b 240p requeues 0 
> qdisc pfifo 8011: parent 11:1 limit 30p
>  Sent 0 bytes 0 pkt (dropped 6178, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> qdisc pfifo 8012: parent 11:2 limit 30p
>  Sent 92 bytes 1 pkt (dropped 37048, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> qdisc pfifo 8013: parent 11:3 limit 30p
>  Sent 0 bytes 0 pkt (dropped 24856, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> qdisc pfifo 8014: parent 11:4 limit 30p
>  Sent 276 bytes 3 pkt (dropped 37358, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> qdisc pfifo 8015: parent 11:5 limit 30p
>  Sent 0 bytes 0 pkt (dropped 24934, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> qdisc pfifo 8016: parent 11:6 limit 30p
>  Sent 0 bytes 0 pkt (dropped 24882, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> qdisc pfifo 8017: parent 11:7 limit 30p
>  Sent 0 bytes 0 pkt (dropped 12328, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> qdisc pfifo 8018: parent 11:8 limit 30p
>  Sent 92 bytes 1 pkt (dropped 31216, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> 
> 
> and :
> 
> # tc -s -d class show dev dummy0
> class cbq 1:11 parent 1:1 leaf 11: rate 40000Kbit cell 128b mpu 64b
> (bounded) prio 2/2 weight 40000Kbit allot 20000b 
> level 0 ewma 5 avpkt 1400b maxidle 0us 
>  Sent 460 bytes 5 pkt (dropped 198800, overlimits 15 requeues 0) 
>  rate 0bit 0pps backlog 0b 240p requeues 0 
>   borrowed 0 overactions 3 avgidle -225 undertime -6.46702e+07
> class cbq 1: root rate 1000Mbit cell 8b (bounded,isolated) prio
> no-transmit/8 weight 1000Mbit allot 1514b 
> level 2 ewma 5 avpkt 1000b maxidle 0us 
>  Sent 460 bytes 5 pkt (dropped 0, overlimits 0 requeues 0) 
>  backlog 0b 0p requeues 0 
>   borrowed 0 overactions 0 avgidle 125 undertime 0
> class cbq 1:1 parent 1: rate 1000Mbit cell 64b mpu 64b (bounded) prio
> 1/1 weight 1000Mbit allot 10000b 
> level 1 ewma 5 avpkt 1500b maxidle 0us 
>  Sent 460 bytes 5 pkt (dropped 0, overlimits 0 requeues 0) 
>  rate 0bit 0pps backlog 0b 0p requeues 0 
>   borrowed 0 overactions 0 avgidle 125 undertime 0
> class qfq 11:1 root leaf 8011: weight 1 maxpkt 2048 
>  Sent 2760 bytes 30 pkt (dropped 6178, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> class qfq 11:2 root leaf 8012: weight 1 maxpkt 2048 
>  Sent 2852 bytes 31 pkt (dropped 37048, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> class qfq 11:3 root leaf 8013: weight 1 maxpkt 2048 
>  Sent 2760 bytes 30 pkt (dropped 24856, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> class qfq 11:4 root leaf 8014: weight 1 maxpkt 2048 
>  Sent 3036 bytes 33 pkt (dropped 37358, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> class qfq 11:5 root leaf 8015: weight 1 maxpkt 2048 
>  Sent 2760 bytes 30 pkt (dropped 24934, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> class qfq 11:6 root leaf 8016: weight 1 maxpkt 2048 
>  Sent 2760 bytes 30 pkt (dropped 24882, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> class qfq 11:7 root leaf 8017: weight 1 maxpkt 2048 
>  Sent 2760 bytes 30 pkt (dropped 12328, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> class qfq 11:8 root leaf 8018: weight 1 maxpkt 2048 
>  Sent 2852 bytes 31 pkt (dropped 31216, overlimits 0 requeues 0) 
>  backlog 2760b 30p requeues 0 
> 
> 
> 
> 
> Then this crashes :
> 
> # tc qdisc del dev dummy0 root

hlist_for_each_entry_safe fixes that.

Can you reproduce without putting cbq on the root?

-- 

^ permalink raw reply

* Re: [PATCH] sched: QFQ - quick fair queue scheduler (v4)
From: Stephen Hemminger @ 2011-03-04 17:18 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Fabio Checconi, Luigi Rizzo, netdev
In-Reply-To: <1299221426.2547.47.camel@edumazet-laptop>

On Fri, 04 Mar 2011 07:50:26 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le jeudi 03 mars 2011 à 16:30 -0800, Stephen Hemminger a écrit :
> > This is an implementation of the Quick Fair Queue scheduler developed
> > by Fabio Checconi. The same algorithm is already implemented in ipfw
> > in FreeBSD. Fabio had an earlier version developed on Linux, I just
> > cleaned it up.  Thanks to Eric Dumazet for doing the testing and
> > finding bugs.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > 
> > ---
> > v4 - change slots[] to hlist from simple linked list
> > 
> >  include/linux/pkt_sched.h |   15 
> >  net/sched/Kconfig         |   11 
> >  net/sched/Makefile        |    1 
> >  net/sched/sch_qfq.c       | 1133 ++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 1160 insertions(+)
> 
> Thanks
> 
> Still crashing hard here in qfq_reset_qdisc(), when packets are present
> in queues.
> 
> Probably hlist_for_each_entry_safe() is needed, since
> qfq_deactivate_class() is called ...
> 
> Also rename cl->next to cl->hnode so that following is clearer ?
> 
> -	hlist_for_each_entry(cl, n, &grp->slots[j], next) {
> +	hlist_for_each_entry_safe(cl, n, next, &grp->slots[j], hnode) {
> 
> Fabio, any idea why everything is blocked after a few packets for me ?
> 
> Here is script to reproduce the problem :
> 
> # cat qfq_setup.sh
> modprobe dummy
> 
> ifconfig dummy0 10.2.2.254 netmask 255.255.255.0 up
> 
> for i in `seq 1 16`
> do
>  arp -H ether -i dummy0 -s 10.2.2.$i 00:00:0c:07:ac:$(printf %02x $i)
> done
> 
> DEV=dummy0
> RATE="rate 40Mbit"
> TNETS="10.2.2.0/25"
> ALLOT="allot 20000"
> 
> tc qdisc del dev dummy0 root 2>/dev/null
> 
> tc qdisc add dev $DEV root handle 1: cbq avpkt 1000 rate 1000Mbit \
> 	bandwidth 1000Mbit

QFQ is non work conserving, it may choose to send a smaller packet
ahead of a larger packet in other flow...


-- 

^ permalink raw reply

* QFQ debugfs
From: Stephen Hemminger @ 2011-03-04 17:20 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Fabio Checconi, Luigi Rizzo, netdev
In-Reply-To: <1299221426.2547.47.camel@edumazet-laptop>

This is quick hack to put a debugfs hook on:
  debug/qfq/<device>

dumps internal state

---
 net/sched/sch_qfq.c |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

--- a/net/sched/sch_qfq.c	2011-03-04 08:24:39.138728524 -0800
+++ b/net/sched/sch_qfq.c	2011-03-04 08:44:10.889972934 -0800
@@ -14,6 +14,7 @@
 #include <linux/errno.h>
 #include <linux/netdevice.h>
 #include <linux/pkt_sched.h>
+#include <linux/debugfs.h>
 #include <net/sch_generic.h>
 #include <net/pkt_sched.h>
 #include <net/pkt_cls.h>
@@ -144,6 +145,7 @@ struct qfq_group {
 struct qfq_sched {
 	struct tcf_proto *filter_list;
 	struct Qdisc_class_hash clhash;
+	struct dentry	     *debugfs;
 
 	u64		V;		/* Precise virtual time. */
 	u32		wsum;		/* weight sum */
@@ -1025,6 +1027,55 @@ static unsigned int qfq_drop(struct Qdis
 	return 0;
 }
 
+static struct dentry *qfq_debug;
+
+static int qfq_debug_show(struct seq_file *seq, void *v)
+{
+	struct Qdisc *sch = seq->private;
+	const struct qfq_sched *q = qdisc_priv(sch);
+	unsigned int i, j;
+
+	seq_printf(seq, "V=%llu wsum=%u\n", q->V, q->wsum);
+	for (i = 0; i < QFQ_MAX_STATE; i++)
+		seq_printf(seq, "%lx%c", q->bitmaps[i],
+			   (i & 3) == 3 ? '\n' : ' ');
+
+	for (i = 0; i <= QFQ_MAX_INDEX; i++) {
+		const struct qfq_group *grp = &q->groups[i];
+		seq_printf(seq, "%d: S=%llu F=%llu shift=%u index=%u front=%u full=%#lx\n",
+			   i, (unsigned long long)grp->S,
+			   (unsigned long long)grp->F,
+			   grp->slot_shift, grp->index,
+			   grp->front, grp->full_slots);
+
+		for (j = 0; j < QFQ_MAX_SLOTS; j++) {
+			const struct qfq_class *cl;
+			struct hlist_node *n;
+
+			hlist_for_each_entry(cl, n, &grp->slots[j], next) {
+				seq_printf(seq, "  %d: S=%llu F=%llu inv_w=%u lmax=%u\n",
+					   j, cl->S, cl->F,
+					   cl->inv_w, cl->lmax);
+			}
+		}
+	}
+	return 0;
+}
+
+static int qfq_debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, qfq_debug_show, inode->i_private);
+}
+
+static const struct file_operations qfq_debug_fops = {
+	.owner		= THIS_MODULE,
+	.open		= qfq_debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+
 static int qfq_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
@@ -1042,6 +1093,9 @@ static int qfq_init_qdisc(struct Qdisc *
 			INIT_HLIST_HEAD(&grp->slots[j]);
 	}
 
+	q->debugfs = debugfs_create_file(sch->dev_queue->dev->name,
+					 S_IRUGO, qfq_debug, sch,
+					 &qfq_debug_fops);
 	return 0;
 }
 
@@ -1121,11 +1175,16 @@ static struct Qdisc_ops qfq_qdisc_ops __
 
 static int __init qfq_init(void)
 {
+	qfq_debug = debugfs_create_dir("qfq", NULL);
+
 	return register_qdisc(&qfq_qdisc_ops);
 }
 
 static void __exit qfq_exit(void)
 {
+	if (qfq_debug)
+		debugfs_remove(qfq_debug);
+
 	unregister_qdisc(&qfq_qdisc_ops);
 }
 

^ permalink raw reply

* mii_bus->read return checking in phy_device.c
From: Florian Fainelli @ 2011-03-04 17:25 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Andy Fleming

Hello Andy,

While debugging a PHY probing issue with the au1000_eth, I stumbled upon this 
in drivers/net/phy/phy_device.c:

        phy_reg = bus->read(bus, addr, MII_PHYSID1);                                                  
                                   
        if (phy_reg < 0)                        
                return -EIO;

most drivers implement phylib's mdio_read callback by simply returning the 
contents of their MDIO register after a readl, ioread ... which is unsigned. 
Would not it rather make sense to check for phy_reg <= 0 instead?

This can lead for instance to believing that a PHY is present at a wrong 
address because the MDIO read function returns 0 for that particular register, 
which is logical because no PHY is present at that address.

I am asking in case I just miss something.

Thank you.
--
Florian

^ permalink raw reply

* Re: [PATCH 1/2 net-next][v2] bonding: fix incorrect transmit queue offset
From: Jay Vosburgh @ 2011-03-04 17:37 UTC (permalink / raw)
  To: Phil Oester; +Cc: Andy Gospodarek, David Miller, bhutchings, netdev
In-Reply-To: <20110302014009.GA2045@linuxace.com>

Phil Oester <kernel@linuxace.com> wrote:

>On Tue, Mar 01, 2011 at 10:31:36AM -0500, Andy Gospodarek wrote:
>> > The patch works as expected.  Do we have any agreement on a final version?
>> >
>>
>> Thanks for the testing, Phil.
>>
>> I'm in favor of this patch as it does alert the admin that bonding may
>> not have enough default queues, but it is not as verbose (backtrace et
>> al) and likely to create bug reports as a message from WARN_ON.
>> +             if (net_ratelimit())
>> +                     pr_warning("%s selects invalid tx queue %d.  Consider"
>> +                                " setting module option tx_queues > %d.",
>> +                                dev->name, txq, dev->real_num_tx_queues);
>
>It is unclear why we need to alert the admin to this situation (repeatedly).  
>Say the incoming nic has 32 queues, and is headed out a bond (with 16).
>With your patch, we will log 50% of the time, no?  What benefit is this
>log spew?
>
>While WARN_ONCE may be a bit extreme due to the backtrace, perhaps we
>should at least throw a 'static bool warned' variable in there to lessen
>the nuisance?

	I'm also concerned that the log messages will be excessive.

	Should we instead create a bonding driver-private ethtool
statistics and count these events that way?

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* [Patch] ariadne: fix possible null dereference
From: j223yang @ 2011-03-04 17:46 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-kernel

This patch fixes bugzilla #13853:
https://bugzilla.kernel.org/show_bug.cgi?id=13853

The patch removes dereference of 'dev' after testing for NULL.
The source code ariadne.c uses spaces instead of tabs, so the patch
uses spaces too.

Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
---
 drivers/net/ariadne.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ariadne.c b/drivers/net/ariadne.c
--- a/drivers/net/ariadne.c
+++ b/drivers/net/ariadne.c
@@ -420,7 +420,7 @@ static inline void ariadne_reset(struct
 static irqreturn_t ariadne_interrupt(int irq, void *data)
 {
     struct net_device *dev = (struct net_device *)data;
-    volatile struct Am79C960 *lance = (struct Am79C960*)dev->base_addr;
+    volatile struct Am79C960 *lance;
     struct ariadne_private *priv;
     int csr0, boguscnt;
     int handled = 0;
@@ -430,6 +430,7 @@ static irqreturn_t ariadne_interrupt(int
        return IRQ_NONE;
     }

+    lance = (struct Am79C960 *)dev->base_addr;
     lance->RAP = CSR0;                 /* PCnet-ISA Controller Status */

     if (!(lance->RDP & INTR))          /* Check if any interrupt has been */

^ permalink raw reply

* Re: [RFC] Keep track of interrupt URB status and resubmit in bh if necessary
From: Paul Stewart @ 2011-03-04 18:01 UTC (permalink / raw)
  To: Indrek Peri; +Cc: netdev@vger.kernel.org, David Brownell
In-Reply-To: <4D70C699.8000606@Ericsson.com>

Thanks for helping clear this up a bit.  I'm still a little foggy on
the details, especially when it comes to selective suspend (although I
currently don't work with a system that is actively using it, so I
might not see these issues).

On Fri, Mar 4, 2011 at 3:01 AM, Indrek Peri <Indrek.Peri@ericsson.com> wrote:
>
> To my understanding usbnet has design questions, like EVENT_DEV_ASLEEP,
> device states and events. In struct "flags" member holds an event to
> execute deferred work. Actually, in usbnet_bh, EVENT_DEV_ASLEEP is
> really needed in if-statement. Without that, in selective suspend
> case driver crashes.
>
> Another problem was that "suspend" removed interrupt URB. I tought in
> the same way as Paul, I added insertion of interrupt URB. Paul added
> it in usbnet_bh and I in "resume". I do not see a problem to add
> interrupt URB in "resume". My patch had a typo, in "resume" we
> should use GFP_ATOMIC in usb_submit_urb. Now is the question is
> this good design? My interpretation of David is that driver needs
> "resume driver" transition where RX and INT URBs are activated.

My interpretation of his statement "shouldn't this be just another
part of the "resume driver" action, like refilling the RX urb queue?"
was implicitly asking "shouldn't you move this to usbnet_bh, which is
where the RX urb queue is refilled?"  This is why I moved things
there.  Hopefully David can shed some light on it (now that I think
I've typed in his email address correctly. :-)

Now, I am also seeing what appear to be RX stalls with some
probability after suspend/resume, so it seems even with the current bh
there is still work to be done...

> I guess that usbnet needs a redesign and rewriting.
> usbnet is used by many USB-Ethernet devices that actually do not
> use selective suspend.
>
> BR, Indrek
>
>
>
> On 03/03/2011 06:45 AM, Paul Stewart wrote:
>> It appears that a patch from Indrek Peri similar to the one below
>> without resolution.  I'm new to this problem (suspend-resume causing
>> interrupt URBs to stop delivering) and am curious about what the correct
>> solution would should like.  Before becoming aware of this thread, I
>> just added a "usb_submit_urb" of "dev->interrupt" into "usbnet_resume()"
>> and that worked to solve the issue I was having.  Apparently this isn't
>> the correct solution though, from David's response to Indrek.  So, I'm
>> curious about what the right code should be.
>>
>> I'll note is that submitting the interrupt URB seems fairly benign.  If
>> we are in a situation where we should not have sent an URB (e.g, the
>> netif wasn't running) intr_complete correctly handles this case and does
>> not re-submit the URB, so at most we get one "rogue" interrupt URB after
>> resume-from-suspend.  The only nasty thing is that this URB should
>> probably not be submitted from interrupt, which the resume function
>> almost certainly is.  I'm guessing this is part of why David NAKed
>> Indrek's patch.  Am I correct?
>>
>> Does something like the patch below seem like a resonable solution?
>>
>> Cc: David Brownell <dbrownell@users.sourceforge.org>
>> Cc: Indrek Peri <Indrek.Peri@Ericsson.com>
>> Signed-off-by: Paul Stewart <pstew@google.com>
>> ---
>>  drivers/net/usb/usbnet.c   |   21 +++++++++++++++++++--
>>  include/linux/usb/usbnet.h |    1 +
>>  2 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
>> index 02d25c7..bc6a8e0 100644
>> --- a/drivers/net/usb/usbnet.c
>> +++ b/drivers/net/usb/usbnet.c
>> @@ -471,6 +471,8 @@ static void intr_complete (struct urb *urb)
>>       struct usbnet   *dev = urb->context;
>>       int             status = urb->status;
>>
>> +     dev->interrupt_urb_running = 0;
>> +
>>       switch (status) {
>>       /* success */
>>       case 0:
>> @@ -497,7 +499,9 @@ static void intr_complete (struct urb *urb)
>>
>>       memset(urb->transfer_buffer, 0, urb->transfer_buffer_length);
>>       status = usb_submit_urb (urb, GFP_ATOMIC);
>> -     if (status != 0 && netif_msg_timer (dev))
>> +     if (status == 0)
>> +             dev->interrupt_urb_running = 1;
>> +     else if (netif_msg_timer (dev))
>>               deverr(dev, "intr resubmit --> %d", status);
>>  }
>>
>> @@ -580,6 +584,7 @@ static int usbnet_stop (struct net_device *net)
>>       remove_wait_queue (&unlink_wakeup, &wait);
>>
>>       usb_kill_urb(dev->interrupt);
>> +     dev->interrupt_urb_running = 0;
>>
>>       /* deferred work (task, timer, softirq) must also stop.
>>        * can't flush_scheduled_work() until we drop rtnl (later),
>> @@ -640,7 +645,8 @@ static int usbnet_open (struct net_device *net)
>>                       if (netif_msg_ifup (dev))
>>                               deverr (dev, "intr submit %d", retval);
>>                       goto done;
>> -             }
>> +             } else
>> +                     dev->interrupt_urb_running = 1;
>>       }
>>
>>       netif_start_queue (net);
>> @@ -1065,6 +1071,17 @@ static void usbnet_bh (unsigned long param)
>>               if (dev->txq.qlen < TX_QLEN (dev))
>>                       netif_wake_queue (dev->net);
>>       }
>> +
>> +     // Do we need to re-enable interrupt URBs?
>> +     if (netif_running (dev->net) &&
>> +         netif_device_present (dev->net) &&
>> +         dev->interrupt_urb_running == 0) {
>> +             usb_submit_urb (dev->interrupt, GFP_KERNEL);
>> +
>> +             /* Unconditionally mark as running so we don't retry */
>> +             dev->interrupt_urb_running = 1;
>> +     }
>> +
>>  }
>>
>>
>> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
>> index ba09fe8..1b8ed8a 100644
>> --- a/include/linux/usb/usbnet.h
>> +++ b/include/linux/usb/usbnet.h
>> @@ -49,6 +49,7 @@ struct usbnet {
>>       u32                     hard_mtu;       /* count any extra framing */
>>       size_t                  rx_urb_size;    /* size for rx urbs */
>>       struct mii_if_info      mii;
>> +     int                     interrupt_urb_running;
>>
>>       /* various kinds of pending driver work */
>>       struct sk_buff_head     rxq;
>
>

^ permalink raw reply

* Re: [PATCH] e1000: power off PHY after reset when interface is down
From: Prasanna Panchamukhi @ 2011-03-04 18:03 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: Allan, Bruce W, Pieper, Jeffrey E,
	e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <AANLkTim-T99r+npYWCLH8ry+hbtCNBOM4WUev8soaoM6@mail.gmail.com>

On 03/04/2011 12:19 AM, Jeff Kirsher wrote:
> On Wed, Feb 23, 2011 at 16:02, Jeff Kirsher<jeffrey.t.kirsher@intel.com>  wrote:
>> On Tue, 2011-02-22 at 17:25 -0800, prasanna.panchamukhi@riverbed.com
>> wrote:
>>> From: Prasanna S. Panchamukhi<prasanna.panchamukhi@riverbed.com>
>>>
>>> Some Phys supported by the e1000 driver do not remain powered off
>>> across
>>> a reset of the device when the interface is down, e.g. on 82546.
>>> This patch powers down (only when WoL is disabled) the PHY after reset
>>> if
>>> the interface is down and ethtool diagnostics are not currently
>>> running.
>>>
>>> Similar problem was see on 82571 controller and was fixed in e1000e
>>> driver
>>> by Bruce Allan.
>>> Please refer commit 31dbe5b4ac6fca72dec946e4d0fa7f0913f1d9b1 for
>>> details.
>>>
>>> Signed-off-by: Prasanna S. Panchamukhi
>>> <prasanna.panchamukhi@riverbed.com>
>>> ---
>>>   drivers/net/e1000/e1000_ethtool.c |   27 +++++++++++++++++++--------
>>>   drivers/net/e1000/e1000_main.c    |    7 +++++++
>>>   2 files changed, 26 insertions(+), 8 deletions(-)
>> Thanks Prasanna!  I have added the patch to my queue of e1000 patches.
>>
> Prasanna-
> Here is what we found during validating your patch:
>
> The behavior of 82546 device(s) seems to be identical with/without this patch
> applied. 82546GB (LOM), dev_id 1079 powers down (with wol disabled) after
> ifdown, but powers back up after approx. 10 seconds. 82546EB (NIC), dev_id
> 1010 powers down (with wol disabled) after ifdown. Both of the above
> behaviors are the same with and without the patch applied. Also, if this patch
> DID work as expected, it should print a message after a reset, such as "Cannot
> restart autonegotiation: Resource temporarily unavailable", which would mirror
> the behavior of e1000e.
>
Hi Jeff,

Below is the test case we run verify this fix:

$ ethtool -s eth0 wol d
$ ifconfig eth0 up
$ mii-tool eth0
eth0: negotiated 100baseTx-FD, link ok
$ ifconfig eth0 down
$ mii-tool eth0
eth0: no link
$ ethtool -s eth0 autoneg on  (doesn't really matter what we do here)
$ mii-tool eth0
eth0: negotiated 100baseTx-FD, link ok (this should be: eth0: no link)

I will re-run the test&  check if does not fix this.

Thanks
Prasanna




^ 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