RxRPC: 3 issues found in my example code
Tim Smith
tim at electronghost.co.uk
Sat Apr 26 08:14:35 PDT 2014
On Friday 25 April 2014 19:10:13 Tim Smith wrote:
> On Friday 25 April 2014 09:51:35 Leon Liu wrote:
> > ====================================
> > Issues 1:
> > ====================================
> > when I send out a call data larger than 1500(aka: MTU size), the first
> > data
> > snippet was received by receiver successfully but failed for following
> > snippets. No problem if call data is less than MTU size.
>
> This is almost certainly my fault. I suspect removing the spinlocks has
> introduced a race.
>
> I will take a look at this over the weekend.
I've reproduced something very similar to your first report, and the attached
hack fixes it locally for me. If it fixes it for you, I will make something
less ugly.
--
Tim Smith <tim at electronghost.co.uk>
England Prevails
-------------- next part --------------
Fix the rxrpc connection creation race in a hacky and not-proper way.
From: Tim Smith <tim at electronghost.co.uk>
---
net/rxrpc/ar-accept.c | 37 +++++++------------------------------
net/rxrpc/ar-input.c | 5 +++--
net/rxrpc/ar-internal.h | 2 +-
net/rxrpc/ar-local.c | 4 ++--
4 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/net/rxrpc/ar-accept.c b/net/rxrpc/ar-accept.c
index 6d79310..e995922 100644
--- a/net/rxrpc/ar-accept.c
+++ b/net/rxrpc/ar-accept.c
@@ -70,7 +70,7 @@ static int rxrpc_busy(struct rxrpc_local *local, struct sockaddr_rxrpc *srx,
* accept an incoming call that needs peer, transport and/or connection setting
* up
*/
-static int rxrpc_accept_incoming_call(struct rxrpc_local *local,
+static int _rxrpc_accept_incoming_call(struct rxrpc_local *local,
struct rxrpc_sock *rx,
struct sk_buff *skb,
struct sockaddr_rxrpc *srx)
@@ -204,38 +204,15 @@ error_nofree:
* accept incoming calls that need peer, transport and/or connection setting up
* - the packets we get are all incoming client DATA packets that have seq == 1
*/
-void rxrpc_accept_incoming_calls(struct work_struct *work)
+void rxrpc_accept_incoming_call(struct rxrpc_local *local, struct sk_buff *skb)
{
- struct rxrpc_local *local =
- container_of(work, struct rxrpc_local, acceptor);
struct rxrpc_skb_priv *sp;
struct sockaddr_rxrpc srx;
struct rxrpc_sock *rx;
- struct sk_buff *skb;
__be16 service_id;
int ret;
_enter("%d", local->debug_id);
-
- read_lock_bh(&rxrpc_local_lock);
- if (atomic_read(&local->usage) > 0)
- rxrpc_get_local(local);
- else
- local = NULL;
- read_unlock_bh(&rxrpc_local_lock);
- if (!local) {
- _leave(" [local dead]");
- return;
- }
-
-process_next_packet:
- skb = skb_dequeue(&local->accept_queue);
- if (!skb) {
- rxrpc_put_local(local);
- _leave("\n");
- return;
- }
-
_net("incoming call skb %p", skb);
sp = rxrpc_skb(skb);
@@ -274,7 +251,7 @@ found_service:
sock_hold(&rx->sk);
read_unlock_bh(&local->services_lock);
- ret = rxrpc_accept_incoming_call(local, rx, skb, &srx);
+ ret = _rxrpc_accept_incoming_call(local, rx, skb, &srx);
if (ret < 0)
sk_acceptq_removed(&rx->sk);
sock_put(&rx->sk);
@@ -282,7 +259,7 @@ found_service:
case -ECONNRESET: /* old calls are ignored */
case -ECONNABORTED: /* aborted calls are reaborted or ignored */
case 0:
- goto process_next_packet;
+ return;
case -ECONNREFUSED:
goto invalid_service;
case -EBUSY:
@@ -298,18 +275,18 @@ backlog_full:
busy:
rxrpc_busy(local, &srx, &sp->hdr);
rxrpc_free_skb(skb);
- goto process_next_packet;
+ return;
invalid_service:
skb->priority = RX_INVALID_OPERATION;
rxrpc_reject_packet(local, skb);
- goto process_next_packet;
+ return;
/* can't change connection security type mid-flow */
security_mismatch:
skb->priority = RX_PROTOCOL_ERROR;
rxrpc_reject_packet(local, skb);
- goto process_next_packet;
+ return;
}
/*
diff --git a/net/rxrpc/ar-input.c b/net/rxrpc/ar-input.c
index 7374264..0ca899c 100644
--- a/net/rxrpc/ar-input.c
+++ b/net/rxrpc/ar-input.c
@@ -747,8 +747,9 @@ cant_route_call:
sp->hdr.type == RXRPC_PACKET_TYPE_DATA) {
if (sp->hdr.seq == cpu_to_be32(1)) {
_debug("first packet");
- skb_queue_tail(&local->accept_queue, skb);
- rxrpc_queue_work(&local->acceptor);
+// skb_queue_tail(&local->accept_queue, skb);
+// rxrpc_queue_work(&local->acceptor);
+ rxrpc_accept_incoming_call(local, skb);
rxrpc_put_local(local);
_leave(" [incoming]");
return;
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index c831d44..f1de5a0 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -437,7 +437,7 @@ extern struct workqueue_struct *rxrpc_workqueue;
/*
* ar-accept.c
*/
-void rxrpc_accept_incoming_calls(struct work_struct *);
+void rxrpc_accept_incoming_call(struct rxrpc_local *local, struct sk_buff *skb);
struct rxrpc_call *rxrpc_accept_call(struct rxrpc_sock *, unsigned long);
int rxrpc_reject_call(struct rxrpc_sock *);
diff --git a/net/rxrpc/ar-local.c b/net/rxrpc/ar-local.c
index 87f7135..ae922af 100644
--- a/net/rxrpc/ar-local.c
+++ b/net/rxrpc/ar-local.c
@@ -35,7 +35,7 @@ struct rxrpc_local *rxrpc_alloc_local(struct sockaddr_rxrpc *srx)
local = kzalloc(sizeof(struct rxrpc_local), GFP_KERNEL);
if (local) {
INIT_WORK(&local->destroyer, &rxrpc_destroy_local);
- INIT_WORK(&local->acceptor, &rxrpc_accept_incoming_calls);
+// INIT_WORK(&local->acceptor, &rxrpc_accept_incoming_calls);
INIT_WORK(&local->rejecter, &rxrpc_reject_packets);
INIT_LIST_HEAD(&local->services);
INIT_LIST_HEAD(&local->link);
@@ -262,7 +262,7 @@ static void rxrpc_destroy_local(struct work_struct *work)
downgrade_write(&rxrpc_local_sem);
ASSERT(list_empty(&local->services));
- ASSERT(!work_pending(&local->acceptor));
+// ASSERT(!work_pending(&local->acceptor));
ASSERT(!work_pending(&local->rejecter));
/* finish cleaning up the local descriptor */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-afs/attachments/20140426/4ad3fabd/attachment.sig>
More information about the linux-afs
mailing list