netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup
@ 2011-03-09  8:43 Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 1/8] Phonet: fix NULL dereference on TX path with implicit source Rémi Denis-Courmont
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:43 UTC (permalink / raw)
  To: netdev

	Hello,

As "promised", here is the cleaned up and unified Phonet pipe code.
The same kernel can now support both old (e.g. Nokia N900) and new
(e.g. ST-Ericsson U8500 SoC) modems.

The following changes since commit 7b46ac4e77f3224a1befe032c77f1df31d1b42c4:

  inetpeer: Don't disable BH for initial fast RCU lookup. (2011-03-08 14:59:28 -0800)

are available in the git repository at:
  git://git.remlab.net/linux-phonet.git master

Rémi Denis-Courmont (8):
      Phonet: fix NULL dereference on TX path with implicit source
      Phonet: return an error when packet TX fails
      Phonet: correct pipe backlog callback return values
      Phonet: factor common code to send control messages
      Phonet: allocate sock from accept syscall rather than soft IRQ
      Phonet: provide pipe socket option to retrieve the pipe identifier
      Phonet: support active connection without pipe controller on modem
      Phonet: kill the ST-Ericsson pipe controller Kconfig

 Documentation/networking/phonet.txt |   67 ++---
 include/linux/phonet.h              |    4 +-
 include/net/phonet/pep.h            |    1 -
 net/phonet/Kconfig                  |   12 -
 net/phonet/af_phonet.c              |   13 +-
 net/phonet/pep.c                    |  682 ++++++++++++++---------------------
 net/phonet/socket.c                 |  112 +++----
 7 files changed, 352 insertions(+), 539 deletions(-)

-- 
Rémi Denis-Courmont
http://www.remlab.net/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/8] Phonet: fix NULL dereference on TX path with implicit source
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 2/8] Phonet: return an error when packet TX fails Rémi Denis-Courmont
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

The previous Phonet patch series introduced per-socket implicit
destination (i.e. connect()). In that case, the destination
socket address is NULL in the transmit function.
However commit a8059512b120362b15424f152b2548fe8b11bd0c
("Phonet: implement per-socket destination/peer address")
is incomplete and would trigger a NULL dereference.
(Fortunately, the code is not in released kernel, and in fact
 currently not reachable.)

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/af_phonet.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 30cc676..4706b77 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -262,10 +262,9 @@ int pn_skb_send(struct sock *sk, struct sk_buff *skb,
 	else if (phonet_address_lookup(net, daddr) == 0) {
 		dev = phonet_device_get(net);
 		skb->pkt_type = PACKET_LOOPBACK;
-	} else if (pn_sockaddr_get_object(target) == 0) {
+	} else if (dst == 0) {
 		/* Resource routing (small race until phonet_rcv()) */
-		struct sock *sk = pn_find_sock_by_res(net,
-							target->spn_resource);
+		struct sock *sk = pn_find_sock_by_res(net, res);
 		if (sk)	{
 			sock_put(sk);
 			dev = phonet_device_get(net);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/8] Phonet: return an error when packet TX fails
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 1/8] Phonet: fix NULL dereference on TX path with implicit source Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 3/8] Phonet: correct pipe backlog callback return values Rémi Denis-Courmont
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

Phonet assumes that packets are never dropped. We try our best to
avoid this situation. But lets return ENOBUFS if queueing to the
network device fails so that the caller knows things went wrong.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/af_phonet.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 4706b77..c6fffd9 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -195,11 +195,7 @@ static int pn_send(struct sk_buff *skb, struct net_device *dev,
 	if (skb->pkt_type == PACKET_LOOPBACK) {
 		skb_reset_mac_header(skb);
 		skb_orphan(skb);
-		if (irq)
-			netif_rx(skb);
-		else
-			netif_rx_ni(skb);
-		err = 0;
+		err = (irq ? netif_rx(skb) : netif_rx_ni(skb)) ? -ENOBUFS : 0;
 	} else {
 		err = dev_hard_header(skb, dev, ntohs(skb->protocol),
 					NULL, NULL, skb->len);
@@ -208,6 +204,8 @@ static int pn_send(struct sk_buff *skb, struct net_device *dev,
 			goto drop;
 		}
 		err = dev_queue_xmit(skb);
+		if (unlikely(err > 0))
+			err = net_xmit_errno(err);
 	}
 
 	return err;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/8] Phonet: correct pipe backlog callback return values
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 1/8] Phonet: fix NULL dereference on TX path with implicit source Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 2/8] Phonet: return an error when packet TX fails Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 4/8] Phonet: factor common code to send control messages Rémi Denis-Courmont
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

In some cases, the Phonet pipe backlog callbacks returned negative
errno instead of NET_RX_* values.

In other cases, NET_RX_DROP was returned for invalid packets, even
though it seems only intended for buffering problems (not for
deliberately discarded packets).

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/pep.c |   25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 875e86c..40952c7 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -522,7 +522,8 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
 		if (!pn_flow_safe(pn->rx_fc)) {
 			err = sock_queue_rcv_skb(sk, skb);
 			if (!err)
-				return 0;
+				return NET_RX_SUCCESS;
+			err = -ENOBUFS;
 			break;
 		}
 
@@ -575,7 +576,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 out:
 	kfree_skb(skb);
-	return err;
+	return (err == -ENOBUFS) ? NET_RX_DROP : NET_RX_SUCCESS;
 
 queue:
 	skb->dev = NULL;
@@ -584,7 +585,7 @@ queue:
 	skb_queue_tail(queue, skb);
 	if (!sock_flag(sk, SOCK_DEAD))
 		sk->sk_data_ready(sk, err);
-	return 0;
+	return NET_RX_SUCCESS;
 }
 
 /* Destroy connected sock. */
@@ -686,11 +687,6 @@ static int pep_connreq_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 	peer_type = hdr->other_pep_type << 8;
 
-	if (unlikely(sk->sk_state != TCP_LISTEN) || sk_acceptq_is_full(sk)) {
-		pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE);
-		return -ENOBUFS;
-	}
-
 	/* Parse sub-blocks (options) */
 	n_sb = hdr->data[4];
 	while (n_sb > 0) {
@@ -790,7 +786,6 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
 	struct sock *sknode;
 	struct pnpipehdr *hdr;
 	struct sockaddr_pn dst;
-	int err = NET_RX_SUCCESS;
 	u8 pipe_handle;
 
 	if (!pskb_may_pull(skb, sizeof(*hdr)))
@@ -814,18 +809,20 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
 		sock_put(sknode);
 		if (net_ratelimit())
 			printk(KERN_WARNING"Phonet unconnected PEP ignored");
-		err = NET_RX_DROP;
 		goto drop;
 	}
 
 	switch (hdr->message_id) {
 	case PNS_PEP_CONNECT_REQ:
-		err = pep_connreq_rcv(sk, skb);
+		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;
 
 #ifdef CONFIG_PHONET_PIPECTRLR
 	case PNS_PEP_CONNECT_RESP:
-		err = pep_connresp_rcv(sk, skb);
+		pep_connresp_rcv(sk, skb);
 		break;
 #endif
 
@@ -842,11 +839,11 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
 	case PNS_PEP_DISABLE_REQ:
 		/* invalid handle is not even allowed here! */
 	default:
-		err = NET_RX_DROP;
+		break;
 	}
 drop:
 	kfree_skb(skb);
-	return err;
+	return NET_RX_SUCCESS;
 }
 
 #ifndef CONFIG_PHONET_PIPECTRLR
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/8] Phonet: factor common code to send control messages
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
                   ` (2 preceding siblings ...)
  2011-03-09  8:44 ` [PATCH 3/8] Phonet: correct pipe backlog callback return values Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ Rémi Denis-Courmont
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

With the addition of the pipe controller, there is now quite a bit
of repetitive code for small signaling messages. Lets factor it.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/pep.c |  225 ++++++++++++++++++------------------------------------
 1 files changed, 73 insertions(+), 152 deletions(-)

diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 40952c7..610794a 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -77,24 +77,34 @@ static unsigned char *pep_get_sb(struct sk_buff *skb, u8 *ptype, u8 *plen,
 	return data;
 }
 
-static int pep_reply(struct sock *sk, struct sk_buff *oskb,
-			u8 code, const void *data, int len, gfp_t priority)
+static struct sk_buff *pep_alloc_skb(struct sock *sk, const void *payload,
+					int len, gfp_t priority)
+{
+	struct sk_buff *skb = alloc_skb(MAX_PNPIPE_HEADER + len, priority);
+	if (!skb)
+		return NULL;
+	skb_set_owner_w(skb, sk);
+
+	skb_reserve(skb, MAX_PNPIPE_HEADER);
+	__skb_put(skb, len);
+	skb_copy_to_linear_data(skb, payload, len);
+	__skb_push(skb, sizeof(struct pnpipehdr));
+	skb_reset_transport_header(skb);
+	return skb;
+}
+
+static int pep_reply(struct sock *sk, struct sk_buff *oskb, u8 code,
+			const void *data, int len, gfp_t priority)
 {
 	const struct pnpipehdr *oph = pnp_hdr(oskb);
 	struct pnpipehdr *ph;
 	struct sk_buff *skb;
 	struct sockaddr_pn peer;
 
-	skb = alloc_skb(MAX_PNPIPE_HEADER + len, priority);
+	skb = pep_alloc_skb(sk, data, len, priority);
 	if (!skb)
 		return -ENOMEM;
-	skb_set_owner_w(skb, sk);
 
-	skb_reserve(skb, MAX_PNPIPE_HEADER);
-	__skb_put(skb, len);
-	skb_copy_to_linear_data(skb, data, len);
-	__skb_push(skb, sizeof(*ph));
-	skb_reset_transport_header(skb);
 	ph = pnp_hdr(skb);
 	ph->utid = oph->utid;
 	ph->message_id = oph->message_id + 1; /* REQ -> RESP */
@@ -105,135 +115,69 @@ static int pep_reply(struct sock *sk, struct sk_buff *oskb,
 	return pn_skb_send(sk, skb, &peer);
 }
 
-#define PAD 0x00
-
-#ifdef CONFIG_PHONET_PIPECTRLR
-static int pipe_handler_send_req(struct sock *sk, u8 msg_id, gfp_t priority)
+static int pep_indicate(struct sock *sk, u8 id, u8 code,
+			const void *data, int len, gfp_t priority)
 {
-	int len;
+	struct pep_sock *pn = pep_sk(sk);
 	struct pnpipehdr *ph;
 	struct sk_buff *skb;
-	struct pep_sock *pn = pep_sk(sk);
-
-	static const u8 data[4] = {
-		PAD, PAD, PAD, PAD,
-	};
 
-	switch (msg_id) {
-	case PNS_PEP_CONNECT_REQ:
-		len = sizeof(data);
-		break;
-
-	case PNS_PEP_DISCONNECT_REQ:
-	case PNS_PEP_ENABLE_REQ:
-	case PNS_PEP_DISABLE_REQ:
-		len = 0;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
-	skb = alloc_skb(MAX_PNPIPE_HEADER + len, priority);
+	skb = pep_alloc_skb(sk, data, len, priority);
 	if (!skb)
 		return -ENOMEM;
-	skb_set_owner_w(skb, sk);
 
-	skb_reserve(skb, MAX_PNPIPE_HEADER);
-	if (len) {
-		__skb_put(skb, len);
-		skb_copy_to_linear_data(skb, data, len);
-	}
-	__skb_push(skb, sizeof(*ph));
-	skb_reset_transport_header(skb);
 	ph = pnp_hdr(skb);
-	ph->utid = msg_id; /* whatever */
-	ph->message_id = msg_id;
+	ph->utid = 0;
+	ph->message_id = id;
 	ph->pipe_handle = pn->pipe_handle;
-	ph->error_code = PN_PIPE_NO_ERROR;
-
+	ph->data[0] = code;
 	return pn_skb_send(sk, skb, NULL);
 }
 
-static int pipe_handler_send_created_ind(struct sock *sk, u8 msg_id)
+#define PAD 0x00
+
+#ifdef CONFIG_PHONET_PIPECTRLR
+static int pipe_handler_request(struct sock *sk, u8 id, u8 code,
+				const void *data, int len)
 {
-	int err_code;
+	struct pep_sock *pn = pep_sk(sk);
 	struct pnpipehdr *ph;
 	struct sk_buff *skb;
 
-	struct pep_sock *pn = pep_sk(sk);
-	static u8 data[4] = {
-		0x03, 0x04,
-	};
-	data[2] = pn->tx_fc;
-	data[3] = pn->rx_fc;
-
-	/*
-	 * actually, below is number of sub-blocks and not error code.
-	 * Pipe_created_ind message format does not have any
-	 * error code field. However, the Phonet stack will always send
-	 * an error code as part of pnpipehdr. So, use that err_code to
-	 * specify the number of sub-blocks.
-	 */
-	err_code = 0x01;
-
-	skb = alloc_skb(MAX_PNPIPE_HEADER + sizeof(data), GFP_ATOMIC);
+	skb = pep_alloc_skb(sk, data, len, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
-	skb_set_owner_w(skb, sk);
 
-	skb_reserve(skb, MAX_PNPIPE_HEADER);
-	__skb_put(skb, sizeof(data));
-	skb_copy_to_linear_data(skb, data, sizeof(data));
-	__skb_push(skb, sizeof(*ph));
-	skb_reset_transport_header(skb);
 	ph = pnp_hdr(skb);
-	ph->utid = 0;
-	ph->message_id = msg_id;
+	ph->utid = id; /* whatever */
+	ph->message_id = id;
 	ph->pipe_handle = pn->pipe_handle;
-	ph->error_code = err_code;
-
+	ph->data[0] = code;
 	return pn_skb_send(sk, skb, NULL);
 }
 
-static int pipe_handler_send_ind(struct sock *sk, u8 msg_id)
+static int pipe_handler_send_created_ind(struct sock *sk)
 {
-	int err_code;
-	struct pnpipehdr *ph;
-	struct sk_buff *skb;
 	struct pep_sock *pn = pep_sk(sk);
+	u8 data[4] = {
+		PN_PIPE_SB_NEGOTIATED_FC, pep_sb_size(2),
+		pn->tx_fc, pn->rx_fc,
+	};
 
-	/*
-	 * actually, below is a filler.
-	 * Pipe_enabled/disabled_ind message format does not have any
-	 * error code field. However, the Phonet stack will always send
-	 * an error code as part of pnpipehdr. So, use that err_code to
-	 * specify the filler value.
-	 */
-	err_code = 0x0;
-
-	skb = alloc_skb(MAX_PNPIPE_HEADER, GFP_ATOMIC);
-	if (!skb)
-		return -ENOMEM;
-	skb_set_owner_w(skb, sk);
-
-	skb_reserve(skb, MAX_PNPIPE_HEADER);
-	__skb_push(skb, sizeof(*ph));
-	skb_reset_transport_header(skb);
-	ph = pnp_hdr(skb);
-	ph->utid = 0;
-	ph->message_id = msg_id;
-	ph->pipe_handle = pn->pipe_handle;
-	ph->error_code = err_code;
+	return pep_indicate(sk, PNS_PIPE_CREATED_IND, 1 /* sub-blocks */,
+				data, 4, GFP_ATOMIC);
+}
 
-	return pn_skb_send(sk, skb, NULL);
+static int pipe_handler_send_ind(struct sock *sk, u8 id)
+{
+	return pep_indicate(sk, id, PAD, NULL, 0, GFP_ATOMIC);
 }
 
 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_send_req(sk, id, GFP_KERNEL);
+	return pipe_handler_request(sk, id, PAD, NULL, 0);
 }
 #endif
 
@@ -274,23 +218,21 @@ static int pep_ctrlreq_error(struct sock *sk, struct sk_buff *oskb, u8 code,
 	struct sk_buff *skb;
 	struct pnpipehdr *ph;
 	struct sockaddr_pn dst;
+	u8 data[4] = {
+		oph->data[0], /* PEP type */
+		code, /* error code, at an unusual offset */
+		PAD, PAD,
+	};
 
-	skb = alloc_skb(MAX_PNPIPE_HEADER + 4, priority);
+	skb = pep_alloc_skb(sk, data, 4, priority);
 	if (!skb)
 		return -ENOMEM;
-	skb_set_owner_w(skb, sk);
-
-	skb_reserve(skb, MAX_PHONET_HEADER);
-	ph = (struct pnpipehdr *)skb_put(skb, sizeof(*ph) + 4);
 
+	ph = pnp_hdr(skb);
 	ph->utid = oph->utid;
 	ph->message_id = PNS_PEP_CTRL_RESP;
 	ph->pipe_handle = oph->pipe_handle;
 	ph->data[0] = oph->data[1]; /* CTRL id */
-	ph->data[1] = oph->data[0]; /* PEP type */
-	ph->data[2] = code; /* error code, at an usual offset */
-	ph->data[3] = PAD;
-	ph->data[4] = PAD;
 
 	pn_skb_get_src_sockaddr(oskb, &dst);
 	return pn_skb_send(sk, skb, &dst);
@@ -298,34 +240,15 @@ static int pep_ctrlreq_error(struct sock *sk, struct sk_buff *oskb, u8 code,
 
 static int pipe_snd_status(struct sock *sk, u8 type, u8 status, gfp_t priority)
 {
-	struct pep_sock *pn = pep_sk(sk);
-	struct pnpipehdr *ph;
-	struct sk_buff *skb;
+	u8 data[4] = { type, PAD, PAD, status };
 
-	skb = alloc_skb(MAX_PNPIPE_HEADER + 4, priority);
-	if (!skb)
-		return -ENOMEM;
-	skb_set_owner_w(skb, sk);
-
-	skb_reserve(skb, MAX_PNPIPE_HEADER + 4);
-	__skb_push(skb, sizeof(*ph) + 4);
-	skb_reset_transport_header(skb);
-	ph = pnp_hdr(skb);
-	ph->utid = 0;
-	ph->message_id = PNS_PEP_STATUS_IND;
-	ph->pipe_handle = pn->pipe_handle;
-	ph->pep_type = PN_PEP_TYPE_COMMON;
-	ph->data[1] = type;
-	ph->data[2] = PAD;
-	ph->data[3] = PAD;
-	ph->data[4] = status;
-
-	return pn_skb_send(sk, skb, NULL);
+	return pep_indicate(sk, PNS_PEP_STATUS_IND, PN_PEP_TYPE_COMMON,
+				data, 4, priority);
 }
 
 /* Send our RX flow control information to the sender.
  * Socket must be locked. */
-static void pipe_grant_credits(struct sock *sk)
+static void pipe_grant_credits(struct sock *sk, gfp_t priority)
 {
 	struct pep_sock *pn = pep_sk(sk);
 
@@ -335,16 +258,16 @@ static void pipe_grant_credits(struct sock *sk)
 	case PN_LEGACY_FLOW_CONTROL: /* TODO */
 		break;
 	case PN_ONE_CREDIT_FLOW_CONTROL:
-		pipe_snd_status(sk, PN_PEP_IND_FLOW_CONTROL,
-				PEP_IND_READY, GFP_ATOMIC);
-		pn->rx_credits = 1;
+		if (pipe_snd_status(sk, PN_PEP_IND_FLOW_CONTROL,
+					PEP_IND_READY, priority) == 0)
+			pn->rx_credits = 1;
 		break;
 	case PN_MULTI_CREDIT_FLOW_CONTROL:
 		if ((pn->rx_credits + CREDITS_THR) > CREDITS_MAX)
 			break;
 		if (pipe_snd_status(sk, PN_PEP_IND_ID_MCFC_GRANT_CREDITS,
 					CREDITS_MAX - pn->rx_credits,
-					GFP_ATOMIC) == 0)
+					priority) == 0)
 			pn->rx_credits = CREDITS_MAX;
 		break;
 	}
@@ -474,7 +397,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
 		if (sk->sk_state == TCP_ESTABLISHED)
 			break; /* Nothing to do */
 		sk->sk_state = TCP_ESTABLISHED;
-		pipe_grant_credits(sk);
+		pipe_grant_credits(sk, GFP_ATOMIC);
 		break;
 #endif
 
@@ -561,7 +484,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
 		if (sk->sk_state == TCP_ESTABLISHED)
 			break; /* Nothing to do */
 		sk->sk_state = TCP_ESTABLISHED;
-		pipe_grant_credits(sk);
+		pipe_grant_credits(sk, GFP_ATOMIC);
 		break;
 
 	case PNS_PIPE_DISABLED_IND:
@@ -655,7 +578,7 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
 	pn->rx_credits = 0;
 	sk->sk_state_change(sk);
 
-	return pipe_handler_send_created_ind(sk, PNS_PIPE_CREATED_IND);
+	return pipe_handler_send_created_ind(sk);
 }
 #endif
 
@@ -853,19 +776,15 @@ static int pipe_do_remove(struct sock *sk)
 	struct pnpipehdr *ph;
 	struct sk_buff *skb;
 
-	skb = alloc_skb(MAX_PNPIPE_HEADER, GFP_KERNEL);
+	skb = pep_alloc_skb(sk, NULL, 0, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
 
-	skb_reserve(skb, MAX_PNPIPE_HEADER);
-	__skb_push(skb, sizeof(*ph));
-	skb_reset_transport_header(skb);
 	ph = pnp_hdr(skb);
 	ph->utid = 0;
 	ph->message_id = PNS_PIPE_REMOVE_REQ;
 	ph->pipe_handle = pn->pipe_handle;
 	ph->data[0] = PAD;
-
 	return pn_skb_send(sk, skb, NULL);
 }
 #endif
@@ -894,7 +813,7 @@ static void pep_sock_close(struct sock *sk, long timeout)
 		pipe_do_remove(sk);
 #else
 		/* send pep disconnect request */
-		pipe_handler_send_req(sk, PNS_PEP_DISCONNECT_REQ, GFP_KERNEL);
+		pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD, NULL, 0);
 		sk->sk_state = TCP_CLOSE;
 #endif
 	}
@@ -980,10 +899,12 @@ 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;
+	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);
-	return pipe_handler_send_req(sk, PNS_PEP_CONNECT_REQ, GFP_KERNEL);
+	return pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
+					PN_PIPE_DISABLE, data, 4);
 }
 #endif
 
@@ -1280,7 +1201,7 @@ struct sk_buff *pep_read(struct sock *sk)
 	struct sk_buff *skb = skb_dequeue(&sk->sk_receive_queue);
 
 	if (sk->sk_state == TCP_ESTABLISHED)
-		pipe_grant_credits(sk);
+		pipe_grant_credits(sk, GFP_ATOMIC);
 	return skb;
 }
 
@@ -1325,7 +1246,7 @@ static int pep_recvmsg(struct kiocb *iocb, struct sock *sk,
 	}
 
 	if (sk->sk_state == TCP_ESTABLISHED)
-		pipe_grant_credits(sk);
+		pipe_grant_credits(sk, GFP_KERNEL);
 	release_sock(sk);
 copy:
 	msg->msg_flags |= MSG_EOR;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
                   ` (3 preceding siblings ...)
  2011-03-09  8:44 ` [PATCH 4/8] Phonet: factor common code to send control messages Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier Rémi Denis-Courmont
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

This moves most of the accept logic to process context like other
socket stacks do. Then we can use a few more common socket helpers
and simplify a bit.

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	[flat|nested] 10+ messages in thread

* [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
                   ` (4 preceding siblings ...)
  2011-03-09  8:44 ` [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 7/8] Phonet: support active connection without pipe controller on modem Rémi Denis-Courmont
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

User-space sometimes needs this information. In particular, the GPRS
context or the AT commands pipe setups may use the pipe handle as a
reference.

This removes the settable pipe handle with CONFIG_PHONET_PIPECTRLR.
It did not handle error cases correctly. Furthermore, the kernel
*could* implement a smart scheme for allocating handles (if ever
needed), but userspace really cannot.

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	[flat|nested] 10+ messages in thread

* [PATCH 7/8] Phonet: support active connection without pipe controller on modem
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
                   ` (5 preceding siblings ...)
  2011-03-09  8:44 ` [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09  8:44 ` [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig Rémi Denis-Courmont
  2011-03-09 19:57 ` [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

This provides support for newer ISI modems with no need for the
earlier experimental compile-time alternative choice. With this,
we can now use the same kernel and userspace with both types of
modems.

This also avoids confusing two different and incompatible state
machines, actively connected vs accepted sockets, and adds
connection response error handling (processing "SYN/RST" of sorts).

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	[flat|nested] 10+ messages in thread

* [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
                   ` (6 preceding siblings ...)
  2011-03-09  8:44 ` [PATCH 7/8] Phonet: support active connection without pipe controller on modem Rémi Denis-Courmont
@ 2011-03-09  8:44 ` Rémi Denis-Courmont
  2011-03-09 19:57 ` [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-09  8:44 UTC (permalink / raw)
  To: netdev

This is now a run-time choice so that a single kernel can support both
old and new generation ISI modems. Support for manually enabling the
pipe flow is removed as it did not work properly, does not fit well
with the socket API, and I am not aware of any use at the moment.

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	[flat|nested] 10+ messages in thread

* Re: [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup
  2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
                   ` (7 preceding siblings ...)
  2011-03-09  8:44 ` [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig Rémi Denis-Courmont
@ 2011-03-09 19:57 ` David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2011-03-09 19:57 UTC (permalink / raw)
  To: remi.denis-courmont; +Cc: netdev

From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Wed, 9 Mar 2011 10:43:32 +0200

> 	Hello,
> 
> As "promised", here is the cleaned up and unified Phonet pipe code.
> The same kernel can now support both old (e.g. Nokia N900) and new
> (e.g. ST-Ericsson U8500 SoC) modems.
> 
> The following changes since commit 7b46ac4e77f3224a1befe032c77f1df31d1b42c4:
> 
>   inetpeer: Don't disable BH for initial fast RCU lookup. (2011-03-08 14:59:28 -0800)
> 
> are available in the git repository at:
>   git://git.remlab.net/linux-phonet.git master

Connects to that git server timeout, something is wrong.

sunset:/home/davem# tcpdump -i eth0 host git.remlab.net
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
11:53:55.290558 IP sunset.davemloft.net.davemloft.net.58504 > yop.chewa.net.git: Flags [S], seq 1785921252, win 4380, options [mss 1460,sackOK,TS val 437283616 ecr 0,nop,wscale 7], length 0
11:54:38.550165 IP sunset.davemloft.net.davemloft.net.51169 > yop.chewa.net.git: Flags [S], seq 3171701698, win 4380, options [mss 1460,sackOK,TS val 437287941 ecr 0,nop,wscale 7], length 0
11:54:41.550544 IP sunset.davemloft.net.davemloft.net.51169 > yop.chewa.net.git: Flags [S], seq 3171701698, win 4380, options [mss 1460,sackOK,TS val 437288242 ecr 0,nop,wscale 7], length 0
11:54:47.570532 IP sunset.davemloft.net.davemloft.net.51169 > yop.chewa.net.git: Flags [S], seq 3171701698, win 4380, options [mss 1460,sackOK,TS val 437288844 ecr 0,nop,wscale 7], length 0

Although web access works just fine.

So I'll apply the patches manually.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-03-09 19:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-09  8:43 [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 1/8] Phonet: fix NULL dereference on TX path with implicit source Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 2/8] Phonet: return an error when packet TX fails Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 3/8] Phonet: correct pipe backlog callback return values Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 4/8] Phonet: factor common code to send control messages Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 7/8] Phonet: support active connection without pipe controller on modem Rémi Denis-Courmont
2011-03-09  8:44 ` [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig Rémi Denis-Courmont
2011-03-09 19:57 ` [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).