All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlad Yasevich <vyasevich@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>
Subject: Re: [PATCH net-next] sctp: Add partially support for MSG_MORE to SCTP.
Date: Fri, 20 Jun 2014 22:10:45 +0000	[thread overview]
Message-ID: <53A4B165.6050504@gmail.com> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1725FB90@AcuExch.aculab.com>

On 06/20/2014 12:24 PM, David Laight wrote:
> If MSG_MORE is set then buffer sends as if Nagle were enabled.
> The first data chunk is still sent on its own, but subsequent chunks
> will be bundled and full packets sent.
> Full MSG_MORE support would require a timeout (preferably configurable
> per-socket) to send the last chunk(s), instead of sending them
> when there is nothing outstanding.
> 

Instead of using 1 and 2, can you define them as flags please

Thanks
-vlad

> Signed-off-by: David Laight <david.laight@aculab.com>
> ---
>  net/sctp/output.c |  7 ++++++-
>  net/sctp/socket.c | 14 +++++++++++++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 0f4d15f..9486229 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -690,8 +690,13 @@ static sctp_xmit_t sctp_packet_can_append_data(struct sctp_packet *packet,
>  	 * Inhibit the sending of new chunks when new outgoing data arrives
>  	 * if any previously transmitted data on the connection remains
>  	 * unacknowledged.
> +	 * If nodelay is clear or MSG_MORE is set then perform the Nagle delay.
> +	 * (MSG_MORE from the last sendmsg is saved as 'nodelay & 2'.)
> +	 * This is a partial implementation of MSG_MORE since MSG_MORE should
> +	 * also delay the first packet. However that would need a timeout be
> +	 * added to force the data to be finally sent.
>  	 */
> -	if (!sctp_sk(asoc->base.sk)->nodelay && sctp_packet_empty(packet) &&
> +	if (sctp_sk(asoc->base.sk)->nodelay != 1 && sctp_packet_empty(packet) &&
>  	    inflight && sctp_state(asoc, ESTABLISHED)) {
>  		unsigned int max = transport->pathmtu - packet->overhead;
>  		unsigned int len = chunk->skb->len + q->out_qlen;
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index fee06b9..484c34e 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1927,6 +1927,18 @@ static int sctp_sendmsg(struct kiocb *iocb, struct sock *sk,
>  		pr_debug("%s: we associated primitively\n", __func__);
>  	}
>  
> +	/* Setting MSG_MORE currently has the same effect as enabling Nagle.
> +	 * This means that the user can't force bundling of the first two data
> +	 * chunks.  It does mean that all the data chunks will be sent
> +	 * without an extra timer.
> +	 * It is enough to save the last value since any data sent with
> +	 * MSG_MORE clear will already have been sent (subject to flow control).
> +	 */
> +	if (msg->msg_flags & MSG_MORE)
> +		sp->nodelay |= 2;
> +	else
> +		sp->nodelay &= ~2;
> +
>  	/* Break the message into multiple chunks of maximum size. */
>  	datamsg = sctp_datamsg_from_user(asoc, sinfo, msg, msg_len);
>  	if (IS_ERR(datamsg)) {
> @@ -5020,7 +5032,7 @@ static int sctp_getsockopt_nodelay(struct sock *sk, int len,
>  		return -EINVAL;
>  
>  	len = sizeof(int);
> -	val = (sctp_sk(sk)->nodelay = 1);
> +	val = sctp_sk(sk)->nodelay & 1;
>  	if (put_user(len, optlen))
>  		return -EFAULT;
>  	if (copy_to_user(optval, &val, len))
> 


WARNING: multiple messages have this Message-ID (diff)
From: Vlad Yasevich <vyasevich@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>
Subject: Re: [PATCH net-next] sctp: Add partially support for MSG_MORE to SCTP.
Date: Fri, 20 Jun 2014 18:10:45 -0400	[thread overview]
Message-ID: <53A4B165.6050504@gmail.com> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1725FB90@AcuExch.aculab.com>

On 06/20/2014 12:24 PM, David Laight wrote:
> If MSG_MORE is set then buffer sends as if Nagle were enabled.
> The first data chunk is still sent on its own, but subsequent chunks
> will be bundled and full packets sent.
> Full MSG_MORE support would require a timeout (preferably configurable
> per-socket) to send the last chunk(s), instead of sending them
> when there is nothing outstanding.
> 

Instead of using 1 and 2, can you define them as flags please

Thanks
-vlad

> Signed-off-by: David Laight <david.laight@aculab.com>
> ---
>  net/sctp/output.c |  7 ++++++-
>  net/sctp/socket.c | 14 +++++++++++++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 0f4d15f..9486229 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -690,8 +690,13 @@ static sctp_xmit_t sctp_packet_can_append_data(struct sctp_packet *packet,
>  	 * Inhibit the sending of new chunks when new outgoing data arrives
>  	 * if any previously transmitted data on the connection remains
>  	 * unacknowledged.
> +	 * If nodelay is clear or MSG_MORE is set then perform the Nagle delay.
> +	 * (MSG_MORE from the last sendmsg is saved as 'nodelay & 2'.)
> +	 * This is a partial implementation of MSG_MORE since MSG_MORE should
> +	 * also delay the first packet. However that would need a timeout be
> +	 * added to force the data to be finally sent.
>  	 */
> -	if (!sctp_sk(asoc->base.sk)->nodelay && sctp_packet_empty(packet) &&
> +	if (sctp_sk(asoc->base.sk)->nodelay != 1 && sctp_packet_empty(packet) &&
>  	    inflight && sctp_state(asoc, ESTABLISHED)) {
>  		unsigned int max = transport->pathmtu - packet->overhead;
>  		unsigned int len = chunk->skb->len + q->out_qlen;
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index fee06b9..484c34e 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1927,6 +1927,18 @@ static int sctp_sendmsg(struct kiocb *iocb, struct sock *sk,
>  		pr_debug("%s: we associated primitively\n", __func__);
>  	}
>  
> +	/* Setting MSG_MORE currently has the same effect as enabling Nagle.
> +	 * This means that the user can't force bundling of the first two data
> +	 * chunks.  It does mean that all the data chunks will be sent
> +	 * without an extra timer.
> +	 * It is enough to save the last value since any data sent with
> +	 * MSG_MORE clear will already have been sent (subject to flow control).
> +	 */
> +	if (msg->msg_flags & MSG_MORE)
> +		sp->nodelay |= 2;
> +	else
> +		sp->nodelay &= ~2;
> +
>  	/* Break the message into multiple chunks of maximum size. */
>  	datamsg = sctp_datamsg_from_user(asoc, sinfo, msg, msg_len);
>  	if (IS_ERR(datamsg)) {
> @@ -5020,7 +5032,7 @@ static int sctp_getsockopt_nodelay(struct sock *sk, int len,
>  		return -EINVAL;
>  
>  	len = sizeof(int);
> -	val = (sctp_sk(sk)->nodelay == 1);
> +	val = sctp_sk(sk)->nodelay & 1;
>  	if (put_user(len, optlen))
>  		return -EFAULT;
>  	if (copy_to_user(optval, &val, len))
> 

  reply	other threads:[~2014-06-20 22:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-20 16:24 [PATCH net-next] sctp: Add partially support for MSG_MORE to SCTP David Laight
2014-06-20 16:24 ` David Laight
2014-06-20 22:10 ` Vlad Yasevich [this message]
2014-06-20 22:10   ` Vlad Yasevich
2014-06-23 14:27   ` David Laight
2014-06-25 10:28     ` Daniel Borkmann
2014-06-25 10:28       ` Daniel Borkmann

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=53A4B165.6050504@gmail.com \
    --to=vyasevich@gmail.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=linux-sctp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.