* [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
@ 2011-03-08 13:56 ` Rémi Denis-Courmont
2011-03-08 19:29 ` David Miller
2011-03-08 13:57 ` [PATCH 2/8] Phonet: return an error when skb TX fails Rémi Denis-Courmont
` (6 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:56 UTC (permalink / raw)
To: netdev
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] 14+ messages in thread
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
2011-03-08 13:56 ` [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c Rémi Denis-Courmont
@ 2011-03-08 19:29 ` David Miller
2011-03-08 20:59 ` Rémi Denis-Courmont
0 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2011-03-08 19:29 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Tue, 8 Mar 2011 15:56:59 +0200
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
You don't even read what I tell you, which makes reviewing your work
insanely frustrating.
I said:
Reference other changes like a man, by mentioned the SHA1 ID and
giving the commit message header line inside of ("") right
afterwards. :-)
I didn't say to put the SHA1 reference in the subject line, it's
so messy there. Put it in the commit message. Do you see other
people putting commit ID references in the commit header line?
Also, I said explicitly to reference the commit like this:
$(SHA1_ID) ("commit message header line")
so in this case it would be:
a8059512b120362b15424f152b2548fe8b11bd0c ("Phonet: implement
per-socket destination/peer address")
because when commits get backported that SHA1_ID might be different
in the tree it gets backported to, so you have to provide that
textual reference to the commit so that in such a case it can still
be matched up.
It also irks me that you persisted to provide the most terse possible
one-line commit message. Have a conversation in your commit message,
this isn't for you it's for other people trying to understand your
work.
The commit message header line just gives a top-level description.
Details like the reasoning, reference to relevant commits, etc.
belong in the commit message contents.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
2011-03-08 19:29 ` David Miller
@ 2011-03-08 20:59 ` Rémi Denis-Courmont
2011-03-08 21:09 ` David Miller
0 siblings, 1 reply; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 20:59 UTC (permalink / raw)
To: netdev
Hello,
On March 8th 2011 21:29:23 David Miller, you wrote :
> You don't even read what I tell you, which makes reviewing your work
> insanely frustrating.
Fair enough.
You were the one complaining about the Nokia mail server being insanely
broken (in private email by the way). So I have to read stuff at home, and
implement it the next day at work. Informations do get lost at night. I don't
need to mention I don't like this is frustrating for us Nokians as well.
(...)
> It also irks me that you persisted to provide the most terse possible
> one-line commit message. Have a conversation in your commit message,
> this isn't for you it's for other people trying to understand your
> work.
I'm trying to fix the terrible mess that ST-Ericsson left of the Phonet stack
here. This wouldn't be an issue if the crap hadn't been merged, or if I had
been given enough time to voice my concerns.
Just one WTF? out of many:
static u8 data[4] = {
0x03, 0x04,
};
data[2] = pn->tx_fc;
data[3] = pn->rx_fc;
(Those coffee-deprivated, note the 'static' keyword.) I wonder if the guy ever
wrote C (not C++) before he came to kernel.org and left as quickly.
I am totally fine with strictness on your part. I do genuinely believe it
improves code quality and such, and do try to apply similar patterns when I am
the reviewer. So I present my apologizes for my series of unintended
violations of The Process.
I just wish you'd been as strict back then. Nobody would be having this
pointless non-technical discussion... Well... I'll fix it tomorrow because I
don't want that kind of crap in the Linux kernel anyway.
Thanks every one you for your cooperation.
--
Rémi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
2011-03-08 20:59 ` Rémi Denis-Courmont
@ 2011-03-08 21:09 ` David Miller
2011-03-08 21:30 ` Rémi Denis-Courmont
0 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2011-03-08 21:09 UTC (permalink / raw)
To: remi; +Cc: netdev
From: "Rémi Denis-Courmont" <remi@remlab.net>
Date: Tue, 8 Mar 2011 22:59:41 +0200
> I just wish you'd been as strict back then.
The only thing worse that putting crappy code into the tree is having
patches rot in patchwork because the maintainer isn't responsive at
reviewing things.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
2011-03-08 21:09 ` David Miller
@ 2011-03-08 21:30 ` Rémi Denis-Courmont
2011-03-08 21:34 ` David Miller
0 siblings, 1 reply; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 21:30 UTC (permalink / raw)
To: netdev
Le mardi 8 mars 2011 23:09:26 David Miller, vous avez écrit :
> From: "Rémi Denis-Courmont" <remi@remlab.net>
> Date: Tue, 8 Mar 2011 22:59:41 +0200
>
> > I just wish you'd been as strict back then.
>
> The only thing worse that putting crappy code into the tree is having
> patches rot in patchwork because the maintainer isn't responsive at
> reviewing things.
Patch submitted:
Date: Mon, 27 Sep 2010 10:37:59 +0530
Patch merged:
Date: Mon, 27 Sep 2010 21:50:59 -0700
So the patches had been "rotting" for 0 days 23:13:00. Come on.
--
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
2011-03-08 21:30 ` Rémi Denis-Courmont
@ 2011-03-08 21:34 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2011-03-08 21:34 UTC (permalink / raw)
To: remi; +Cc: netdev
From: "Rémi Denis-Courmont" <remi@remlab.net>
Date: Tue, 8 Mar 2011 23:30:31 +0200
> Le mardi 8 mars 2011 23:09:26 David Miller, vous avez écrit :
>> From: "Rémi Denis-Courmont" <remi@remlab.net>
>> Date: Tue, 8 Mar 2011 22:59:41 +0200
>>
>> > I just wish you'd been as strict back then.
>>
>> The only thing worse that putting crappy code into the tree is having
>> patches rot in patchwork because the maintainer isn't responsive at
>> reviewing things.
>
> Patch submitted:
> Date: Mon, 27 Sep 2010 10:37:59 +0530
>
> Patch merged:
> Date: Mon, 27 Sep 2010 21:50:59 -0700
>
> So the patches had been "rotting" for 0 days 23:13:00. Come on.
I gave you nearly a full day.
If I do my daily email scan twice, and see the same patch on both
occaisions without a review I am very likely to merge the thing
or poke the maintainer.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/8] Phonet: return an error when skb TX fails
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
2011-03-08 13:56 ` [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c Rémi Denis-Courmont
@ 2011-03-08 13:57 ` Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 3/8] Phonet: correct pipe backlog callback return values Rémi Denis-Courmont
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
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] 14+ messages in thread
* [PATCH 3/8] Phonet: correct pipe backlog callback return values
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
2011-03-08 13:56 ` [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 2/8] Phonet: return an error when skb TX fails Rémi Denis-Courmont
@ 2011-03-08 13:57 ` Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 4/8] Phonet: factor common code to send control messages Rémi Denis-Courmont
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In some cases, the Phonet pipe backlog callbacks returned negative
errno instead. In some other cases, NET_RX_DROP was returned even
though there was no buffering issue.
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] 14+ messages in thread
* [PATCH 4/8] Phonet: factor common code to send control messages
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
` (2 preceding siblings ...)
2011-03-08 13:57 ` [PATCH 3/8] Phonet: correct pipe backlog callback return values Rémi Denis-Courmont
@ 2011-03-08 13:57 ` Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ Rémi Denis-Courmont
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
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] 14+ messages in thread
* [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
` (3 preceding siblings ...)
2011-03-08 13:57 ` [PATCH 4/8] Phonet: factor common code to send control messages Rémi Denis-Courmont
@ 2011-03-08 13:57 ` Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier Rémi Denis-Courmont
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
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 [flat|nested] 14+ messages in thread
* [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
` (4 preceding siblings ...)
2011-03-08 13:57 ` [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ Rémi Denis-Courmont
@ 2011-03-08 13:57 ` Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 7/8] Phonet: support active connection without pipe controller on modem Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig Rémi Denis-Courmont
7 siblings, 0 replies; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
User-space sometimes needs this information.
This replaces the buggy code in CONFIG_PHONET_PIPECTRLR case:
this was never used and error cases not handled correctly.
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] 14+ messages in thread
* [PATCH 7/8] Phonet: support active connection without pipe controller on modem
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
` (5 preceding siblings ...)
2011-03-08 13:57 ` [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier Rémi Denis-Courmont
@ 2011-03-08 13:57 ` Rémi Denis-Courmont
2011-03-08 13:57 ` [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig Rémi Denis-Courmont
7 siblings, 0 replies; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:57 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] 14+ messages in thread
* [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig
2011-03-08 13:56 [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup Rémi Denis-Courmont
` (6 preceding siblings ...)
2011-03-08 13:57 ` [PATCH 7/8] Phonet: support active connection without pipe controller on modem Rémi Denis-Courmont
@ 2011-03-08 13:57 ` Rémi Denis-Courmont
7 siblings, 0 replies; 14+ messages in thread
From: Rémi Denis-Courmont @ 2011-03-08 13:57 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.
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] 14+ messages in thread