From: Vladislav Yasevich <vladislav.yasevich@hp•com>
To: netdev@vger•kernel.org, davem@davemloft•net,
Wei Yongjun <yjwei@cn•fujitsu.com>,
Sridhar Samudrala <sri@us•ibm.com>,
linux-sctp@vger•kernel.org
Subject: Re: [PATCH] sctp: ABORT if receive queue is not empty while closing socket
Date: Thu, 30 Jun 2011 10:11:06 -0400 [thread overview]
Message-ID: <4E0C83FA.2090909@hp.com> (raw)
In-Reply-To: <20110630133122.GB24074@canuck.infradead.org>
On 06/30/2011 09:31 AM, Thomas Graf wrote:
> On Wed, Jun 29, 2011 at 12:14:41PM -0400, Vladislav Yasevich wrote:
>> Right. The lack of ABORT from the receive of data is a bug. I was trying to point out
>> that instead of modified the sender of data to send the ABORT, you modify the receiver
>> to send the ABORT when it is being closed while having data queued.
>
> Is this what you had in mind?
Almost. It could really be a simple true/false condition about recvqueue or inqueue
being non-empty. If that's the case, trigger abort.
-vlad
>
> Trigger user ABORT when a socket is closed which has skbs sitting on
> the receive queue. If data was lost, there is no point in doing a
> graceful shutdown. This is consistent with TCP behaviour.
>
> This also resolves the situation when a receiver cannot reopen its rwnd
> and the sender continues retransmission attempts indefinitely before
> initiating the shutdown.
>
> Signed-off-by: Thomas Graf <tgraf@infradead•org>
>
> diff --git a/include/net/sctp/ulpevent.h b/include/net/sctp/ulpevent.h
> index 99b027b..ca4693b 100644
> --- a/include/net/sctp/ulpevent.h
> +++ b/include/net/sctp/ulpevent.h
> @@ -80,7 +80,7 @@ static inline struct sctp_ulpevent *sctp_skb2event(struct sk_buff *skb)
>
> void sctp_ulpevent_free(struct sctp_ulpevent *);
> int sctp_ulpevent_is_notification(const struct sctp_ulpevent *);
> -void sctp_queue_purge_ulpevents(struct sk_buff_head *list);
> +unsigned int sctp_queue_purge_ulpevents(struct sk_buff_head *list);
>
> struct sctp_ulpevent *sctp_ulpevent_make_assoc_change(
> const struct sctp_association *asoc,
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 6766913..958253a 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1384,6 +1384,7 @@ SCTP_STATIC void sctp_close(struct sock *sk, long timeout)
> struct sctp_endpoint *ep;
> struct sctp_association *asoc;
> struct list_head *pos, *temp;
> + unsigned int data_was_unread;
>
> SCTP_DEBUG_PRINTK("sctp_close(sk: 0x%p, timeout:%ld)\n", sk, timeout);
>
> @@ -1393,6 +1394,10 @@ SCTP_STATIC void sctp_close(struct sock *sk, long timeout)
>
> ep = sctp_sk(sk)->ep;
>
> + /* Clean up any skbs sitting on the receive queue. */
> + data_was_unread = sctp_queue_purge_ulpevents(&sk->sk_receive_queue);
> + data_was_unread += sctp_queue_purge_ulpevents(&sctp_sk(sk)->pd_lobby);
> +
> /* Walk all associations on an endpoint. */
> list_for_each_safe(pos, temp, &ep->asocs) {
> asoc = list_entry(pos, struct sctp_association, asocs);
> @@ -1410,7 +1415,8 @@ SCTP_STATIC void sctp_close(struct sock *sk, long timeout)
> }
> }
>
> - if (sock_flag(sk, SOCK_LINGER) && !sk->sk_lingertime) {
> + if (data_was_unread ||
> + (sock_flag(sk, SOCK_LINGER) && !sk->sk_lingertime)) {
> struct sctp_chunk *chunk;
>
> chunk = sctp_make_abort_user(asoc, NULL, 0);
> @@ -1420,10 +1426,6 @@ SCTP_STATIC void sctp_close(struct sock *sk, long timeout)
> sctp_primitive_SHUTDOWN(asoc, NULL);
> }
>
> - /* Clean up any skbs sitting on the receive queue. */
> - sctp_queue_purge_ulpevents(&sk->sk_receive_queue);
> - sctp_queue_purge_ulpevents(&sctp_sk(sk)->pd_lobby);
> -
> /* On a TCP-style socket, block for at most linger_time if set. */
> if (sctp_style(sk, TCP) && timeout)
> sctp_wait_for_close(sk, timeout);
> diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
> index e70e5fc..aab3184 100644
> --- a/net/sctp/ulpevent.c
> +++ b/net/sctp/ulpevent.c
> @@ -1081,9 +1081,19 @@ void sctp_ulpevent_free(struct sctp_ulpevent *event)
> }
>
> /* Purge the skb lists holding ulpevents. */
> -void sctp_queue_purge_ulpevents(struct sk_buff_head *list)
> +unsigned int sctp_queue_purge_ulpevents(struct sk_buff_head *list)
> {
> struct sk_buff *skb;
> - while ((skb = skb_dequeue(list)) != NULL)
> + unsigned int data_unread = 0;
> +
> + while ((skb = skb_dequeue(list)) != NULL) {
> + struct sctp_ulpevent *event = sctp_skb2event(skb);
> +
> + if (!sctp_ulpevent_is_notification(event))
> + data_unread += skb->len;
> +
> sctp_ulpevent_free(sctp_skb2event(skb));
> + }
> +
> + return data_unread;
> }
>
next prev parent reply other threads:[~2011-06-30 14:11 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-29 13:57 [PATCH] sctp: Enforce maximum retransmissions during shutdown Thomas Graf
2011-06-29 14:20 ` Vladislav Yasevich
2011-06-29 14:36 ` Thomas Graf
2011-06-29 14:58 ` Vladislav Yasevich
2011-06-29 15:48 ` Thomas Graf
2011-06-29 16:14 ` Vladislav Yasevich
2011-06-30 8:49 ` Thomas Graf
2011-06-30 14:08 ` Vladislav Yasevich
2011-06-30 16:17 ` Thomas Graf
2011-07-04 13:50 ` [PATCHv2] sctp: Enforce retransmission limit " Thomas Graf
2011-07-06 7:24 ` David Miller
2011-07-06 12:15 ` Neil Horman
2011-07-06 13:16 ` Thomas Graf
2011-07-06 14:19 ` Neil Horman
2011-07-06 13:42 ` Vladislav Yasevich
2011-07-06 14:18 ` Thomas Graf
2011-07-06 14:31 ` Vladislav Yasevich
2011-07-06 15:49 ` Thomas Graf
2011-07-06 16:23 ` Vladislav Yasevich
2011-07-06 21:58 ` Thomas Graf
2011-07-07 10:28 ` [PATCHv3] " Thomas Graf
2011-07-07 13:36 ` Vladislav Yasevich
2011-07-07 21:09 ` David Miller
2011-06-30 13:31 ` [PATCH] sctp: ABORT if receive queue is not empty while closing socket Thomas Graf
2011-06-30 14:11 ` Vladislav Yasevich [this message]
2011-06-30 16:19 ` Thomas Graf
2011-06-30 16:27 ` Vladislav Yasevich
2011-07-08 10:57 ` [PATCHv2] " Thomas Graf
2011-07-08 13:49 ` Vladislav Yasevich
2011-07-08 14:29 ` Thomas Graf
2011-07-08 14:37 ` [PATCHv3] sctp: ABORT if receive, reassmbly, or reodering " Thomas Graf
2011-07-08 16:37 ` David Miller
2011-07-08 16:43 ` Vladislav Yasevich
2011-07-08 16:53 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E0C83FA.2090909@hp.com \
--to=vladislav.yasevich@hp$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=linux-sctp@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sri@us$(echo .)ibm.com \
--cc=yjwei@cn$(echo .)fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox