All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY
@ 2009-09-04  6:38 Wei Yongjun
  2009-09-04 17:46 ` Vlad Yasevich
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Wei Yongjun @ 2009-09-04  6:38 UTC (permalink / raw)
  To: linux-sctp

This patch implement the sender side for SACK-IMMEDIATELY
extension.

  Section 4.1.  Sender Side Considerations

  Whenever the sender of a DATA chunk can benefit from the
  corresponding SACK chunk being sent back without delay, the sender
  MAY set the I-bit in the DATA chunk header.

  Reasons for setting the I-bit include

  o  The sender has not enough queued user data to send the remaining
     DATA chunks due to the Nagle algorithm.

  o  The sending of a DATA chunk fills the congestion or receiver
     window.

  o  The sender is in the SHUTDOWN-PENDING state.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 include/net/sctp/structs.h |    2 ++
 net/sctp/output.c          |   12 ++++++++++++
 net/sctp/outqueue.c        |   19 ++++++++++++++++++-
 3 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 42d00ce..54ac504 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -764,6 +764,7 @@ struct sctp_chunk {
 		has_asconf:1,		/* IN: have seen an asconf before */
 		tsn_missing_report:2,	/* Data chunk missing counter. */
 		fast_retransmit:2;	/* Is this chunk fast retransmitted? */
+	unsigned nagle_qlen;		/* Queued data length due to Nagle. */
 };
 
 void sctp_chunk_hold(struct sctp_chunk *);
@@ -826,6 +827,7 @@ struct sctp_packet {
 	    has_sack:1,		/* This packet contains a SACK chunk. */
 	    has_auth:1,		/* This packet contains an AUTH chunk */
 	    has_data:1,		/* This packet contains at least 1 DATA chunk */
+	    has_iflag:1,	/* This packet need I-Flag for last DATA chunk */
 	    ipfragok:1,		/* So let ip fragment this packet */
 	    malloced:1;		/* Is it malloced? */
 };
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 951f586..aad3ff5 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -75,6 +75,7 @@ static void sctp_packet_reset(struct sctp_packet *packet)
 	packet->has_cookie_echo = 0;
 	packet->has_sack = 0;
 	packet->has_data = 0;
+	packet->has_iflag = 0;
 	packet->has_auth = 0;
 	packet->ipfragok = 0;
 	packet->auth = NULL;
@@ -447,6 +448,10 @@ int sctp_packet_transmit(struct sctp_packet *packet)
 			} else
 				chunk->resent = 1;
 
+			if (packet->has_iflag &&
+			    list_empty(&packet->chunk_list))
+				chunk->chunk_hdr->flags |= SCTP_DATA_SACK_IMM;
+
 			has_data = 1;
 		}
 
@@ -743,6 +748,13 @@ static void sctp_packet_append_data(struct sctp_packet *packet,
 	else
 		rwnd = 0;
 
+	/* Reasons for setting the I-bit include
+	 * - The sending of a DATA chunk fills the congestion or
+	 *   receiver window.
+	 */
+	if (rwnd = 0 || transport->flight_size >= transport->cwnd)
+		packet->has_iflag = 1;
+
 	asoc->peer.rwnd = rwnd;
 	/* Has been accepted for transmission. */
 	if (!asoc->peer.prsctp_capable)
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index c9f20e2..1193169 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -995,10 +995,21 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
 			/* Add the chunk to the packet.  */
 			status = sctp_packet_transmit_chunk(packet, chunk, 0);
 
+			/* The sender is in the SHUTDOWN-PENDING state,
+			 * The sender MAY set the I-bit in the DATA
+			 * chunk header.
+			 */
+			if (asoc->state = SCTP_STATE_SHUTDOWN_PENDING)
+				packet->has_iflag = 1;
+
 			switch (status) {
+			case SCTP_XMIT_NAGLE_DELAY:
+				/* mark this chunks not be sent due to Nagle
+				 * algorithm
+				 */
+				chunk->nagle_qlen = q->out_qlen + 1;
 			case SCTP_XMIT_PMTU_FULL:
 			case SCTP_XMIT_RWND_FULL:
-			case SCTP_XMIT_NAGLE_DELAY:
 				/* We could not append this chunk, so put
 				 * the chunk back on the output queue.
 				 */
@@ -1011,6 +1022,12 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
 				break;
 
 			case SCTP_XMIT_OK:
+				/* The sender has not enough queued user data
+				 * to send the remaining DATA chunks due to the
+				 * Nagle algorithm.
+				 */
+				if (chunk->nagle_qlen = q->out_qlen + 1)
+					packet->has_iflag = 1;
 				break;
 
 			default:
-- 
1.6.2.2





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

* Re: [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY
  2009-09-04  6:38 [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY Wei Yongjun
@ 2009-09-04 17:46 ` Vlad Yasevich
  2009-09-08  7:10 ` Wei Yongjun
  2009-09-08 14:52 ` Vlad Yasevich
  2 siblings, 0 replies; 4+ messages in thread
From: Vlad Yasevich @ 2009-09-04 17:46 UTC (permalink / raw)
  To: linux-sctp

Wei Yongjun wrote:
> This patch implement the sender side for SACK-IMMEDIATELY
> extension.
> 
>   Section 4.1.  Sender Side Considerations
> 
>   Whenever the sender of a DATA chunk can benefit from the
>   corresponding SACK chunk being sent back without delay, the sender
>   MAY set the I-bit in the DATA chunk header.
> 
>   Reasons for setting the I-bit include
> 
>   o  The sender has not enough queued user data to send the remaining
>      DATA chunks due to the Nagle algorithm.
> 
>   o  The sending of a DATA chunk fills the congestion or receiver
>      window.

These reasons are still up for debate and BSD hasn't implemented them either.
For example, if the congestion window is filled, setting I bit is pointless
since either there are gaps and you'll get an immediate SACK anyway, or there
is sufficient DATA in flight that you'll get a SACK soon.

Setting if for the 0-window event is kind of useless too.

The Nagle algorithm is debatable as well.  With recent changes, Nagle doesn't
apply to large writes.  So the only time, this might be worth it is if
an application can't fill pathmtu within 200ms SACK timer.

> 
>   o  The sender is in the SHUTDOWN-PENDING state.

This is really the only valid state.  Also, when SCTP_EOF is set on the message.
This part is actually something we don't do right.  It should be possible
to write a message and set SCTP_EOF at the same time, but we don't support it
yet.

Also, please do the API patch along with it.  That's just exposing the flag
in sctp/user.h and honoring it.

Some more comments below.

> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> ---
>  include/net/sctp/structs.h |    2 ++
>  net/sctp/output.c          |   12 ++++++++++++
>  net/sctp/outqueue.c        |   19 ++++++++++++++++++-
>  3 files changed, 32 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 42d00ce..54ac504 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -764,6 +764,7 @@ struct sctp_chunk {
>  		has_asconf:1,		/* IN: have seen an asconf before */
>  		tsn_missing_report:2,	/* Data chunk missing counter. */
>  		fast_retransmit:2;	/* Is this chunk fast retransmitted? */
> +	unsigned nagle_qlen;		/* Queued data length due to Nagle. */
>  };
>  
>  void sctp_chunk_hold(struct sctp_chunk *);
> @@ -826,6 +827,7 @@ struct sctp_packet {
>  	    has_sack:1,		/* This packet contains a SACK chunk. */
>  	    has_auth:1,		/* This packet contains an AUTH chunk */
>  	    has_data:1,		/* This packet contains at least 1 DATA chunk */
> +	    has_iflag:1,	/* This packet need I-Flag for last DATA chunk */

You should be able to set this more then once per packet, so the packet flag
isn't the right way to do it.

>  	    ipfragok:1,		/* So let ip fragment this packet */
>  	    malloced:1;		/* Is it malloced? */
>  };
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 951f586..aad3ff5 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -75,6 +75,7 @@ static void sctp_packet_reset(struct sctp_packet *packet)
>  	packet->has_cookie_echo = 0;
>  	packet->has_sack = 0;
>  	packet->has_data = 0;
> +	packet->has_iflag = 0;
>  	packet->has_auth = 0;
>  	packet->ipfragok = 0;
>  	packet->auth = NULL;
> @@ -447,6 +448,10 @@ int sctp_packet_transmit(struct sctp_packet *packet)
>  			} else
>  				chunk->resent = 1;
>  
> +			if (packet->has_iflag &&
> +			    list_empty(&packet->chunk_list))
> +				chunk->chunk_hdr->flags |= SCTP_DATA_SACK_IMM;
> +

The setting of the bit should probably be done at chunk creation and when the
chunk is added to the packet (sctp_packet_append_data).

>  			has_data = 1;
>  		}
>  
> @@ -743,6 +748,13 @@ static void sctp_packet_append_data(struct sctp_packet *packet,
>  	else
>  		rwnd = 0;
>  
> +	/* Reasons for setting the I-bit include
> +	 * - The sending of a DATA chunk fills the congestion or
> +	 *   receiver window.
> +	 */
> +	if (rwnd = 0 || transport->flight_size >= transport->cwnd)
> +		packet->has_iflag = 1;
> +
>  	asoc->peer.rwnd = rwnd;
>  	/* Has been accepted for transmission. */
>  	if (!asoc->peer.prsctp_capable)
> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index c9f20e2..1193169 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -995,10 +995,21 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>  			/* Add the chunk to the packet.  */
>  			status = sctp_packet_transmit_chunk(packet, chunk, 0);
>  
> +			/* The sender is in the SHUTDOWN-PENDING state,
> +			 * The sender MAY set the I-bit in the DATA
> +			 * chunk header.
> +			 */
> +			if (asoc->state = SCTP_STATE_SHUTDOWN_PENDING)
> +				packet->has_iflag = 1;
> +

Set the bit per chunk when adding it to packet.

-vlad

>  			switch (status) {
> +			case SCTP_XMIT_NAGLE_DELAY:
> +				/* mark this chunks not be sent due to Nagle
> +				 * algorithm
> +				 */
> +				chunk->nagle_qlen = q->out_qlen + 1;
>  			case SCTP_XMIT_PMTU_FULL:
>  			case SCTP_XMIT_RWND_FULL:
> -			case SCTP_XMIT_NAGLE_DELAY:
>  				/* We could not append this chunk, so put
>  				 * the chunk back on the output queue.
>  				 */
> @@ -1011,6 +1022,12 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>  				break;
>  
>  			case SCTP_XMIT_OK:
> +				/* The sender has not enough queued user data
> +				 * to send the remaining DATA chunks due to the
> +				 * Nagle algorithm.
> +				 */
> +				if (chunk->nagle_qlen = q->out_qlen + 1)
> +					packet->has_iflag = 1;
>  				break;
>  
>  			default:


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

* Re: [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY
  2009-09-04  6:38 [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY Wei Yongjun
  2009-09-04 17:46 ` Vlad Yasevich
@ 2009-09-08  7:10 ` Wei Yongjun
  2009-09-08 14:52 ` Vlad Yasevich
  2 siblings, 0 replies; 4+ messages in thread
From: Wei Yongjun @ 2009-09-08  7:10 UTC (permalink / raw)
  To: linux-sctp

Vlad Yasevich wrote:
> Wei Yongjun wrote:
>   
>> This patch implement the sender side for SACK-IMMEDIATELY
>> extension.
>>
>>   Section 4.1.  Sender Side Considerations
>>
>>   Whenever the sender of a DATA chunk can benefit from the
>>   corresponding SACK chunk being sent back without delay, the sender
>>   MAY set the I-bit in the DATA chunk header.
>>
>>   Reasons for setting the I-bit include
>>
>>   o  The sender has not enough queued user data to send the remaining
>>      DATA chunks due to the Nagle algorithm.
>>
>>   o  The sending of a DATA chunk fills the congestion or receiver
>>      window.
>>     
>
> These reasons are still up for debate and BSD hasn't implemented them either.
> For example, if the congestion window is filled, setting I bit is pointless
> since either there are gaps and you'll get an immediate SACK anyway, or there
> is sufficient DATA in flight that you'll get a SACK soon.
>
> Setting if for the 0-window event is kind of useless too.
>
> The Nagle algorithm is debatable as well.  With recent changes, Nagle doesn't
> apply to large writes.  So the only time, this might be worth it is if
> an application can't fill pathmtu within 200ms SACK timer.
>   

I will remove those code in the next version.

>   
>>   o  The sender is in the SHUTDOWN-PENDING state.
>>     
>
> This is really the only valid state.  Also, when SCTP_EOF is set on the message.
> This part is actually something we don't do right.  It should be possible
> to write a message and set SCTP_EOF at the same time, but we don't support it
> yet.
>   

I will fix SCTP_EOF to enable write a message and set SCTP_EOF at the
same time.

> Also, please do the API patch along with it.  That's just exposing the flag
> in sctp/user.h and honoring it.
>   

Do you means add a new flag like SCTP_NODSACK or something else, and it can
be specified in sendmsg()'s arguments like SCTP_EOF?

Regards,

Wei Yongjun

> Some more comments below.
>
>   
>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>> ---
>>  include/net/sctp/structs.h |    2 ++
>>  net/sctp/output.c          |   12 ++++++++++++
>>  net/sctp/outqueue.c        |   19 ++++++++++++++++++-
>>  3 files changed, 32 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>> index 42d00ce..54ac504 100644
>> --- a/include/net/sctp/structs.h
>> +++ b/include/net/sctp/structs.h
>> @@ -764,6 +764,7 @@ struct sctp_chunk {
>>  		has_asconf:1,		/* IN: have seen an asconf before */
>>  		tsn_missing_report:2,	/* Data chunk missing counter. */
>>  		fast_retransmit:2;	/* Is this chunk fast retransmitted? */
>> +	unsigned nagle_qlen;		/* Queued data length due to Nagle. */
>>  };
>>  
>>  void sctp_chunk_hold(struct sctp_chunk *);
>> @@ -826,6 +827,7 @@ struct sctp_packet {
>>  	    has_sack:1,		/* This packet contains a SACK chunk. */
>>  	    has_auth:1,		/* This packet contains an AUTH chunk */
>>  	    has_data:1,		/* This packet contains at least 1 DATA chunk */
>> +	    has_iflag:1,	/* This packet need I-Flag for last DATA chunk */
>>     
>
> You should be able to set this more then once per packet, so the packet flag
> isn't the right way to do it.
>
>   
>>  	    ipfragok:1,		/* So let ip fragment this packet */
>>  	    malloced:1;		/* Is it malloced? */
>>  };
>> diff --git a/net/sctp/output.c b/net/sctp/output.c
>> index 951f586..aad3ff5 100644
>> --- a/net/sctp/output.c
>> +++ b/net/sctp/output.c
>> @@ -75,6 +75,7 @@ static void sctp_packet_reset(struct sctp_packet *packet)
>>  	packet->has_cookie_echo = 0;
>>  	packet->has_sack = 0;
>>  	packet->has_data = 0;
>> +	packet->has_iflag = 0;
>>  	packet->has_auth = 0;
>>  	packet->ipfragok = 0;
>>  	packet->auth = NULL;
>> @@ -447,6 +448,10 @@ int sctp_packet_transmit(struct sctp_packet *packet)
>>  			} else
>>  				chunk->resent = 1;
>>  
>> +			if (packet->has_iflag &&
>> +			    list_empty(&packet->chunk_list))
>> +				chunk->chunk_hdr->flags |= SCTP_DATA_SACK_IMM;
>> +
>>     
>
> The setting of the bit should probably be done at chunk creation and when the
> chunk is added to the packet (sctp_packet_append_data).
>
>   
>>  			has_data = 1;
>>  		}
>>  
>> @@ -743,6 +748,13 @@ static void sctp_packet_append_data(struct sctp_packet *packet,
>>  	else
>>  		rwnd = 0;
>>  
>> +	/* Reasons for setting the I-bit include
>> +	 * - The sending of a DATA chunk fills the congestion or
>> +	 *   receiver window.
>> +	 */
>> +	if (rwnd = 0 || transport->flight_size >= transport->cwnd)
>> +		packet->has_iflag = 1;
>> +
>>  	asoc->peer.rwnd = rwnd;
>>  	/* Has been accepted for transmission. */
>>  	if (!asoc->peer.prsctp_capable)
>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>> index c9f20e2..1193169 100644
>> --- a/net/sctp/outqueue.c
>> +++ b/net/sctp/outqueue.c
>> @@ -995,10 +995,21 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>>  			/* Add the chunk to the packet.  */
>>  			status = sctp_packet_transmit_chunk(packet, chunk, 0);
>>  
>> +			/* The sender is in the SHUTDOWN-PENDING state,
>> +			 * The sender MAY set the I-bit in the DATA
>> +			 * chunk header.
>> +			 */
>> +			if (asoc->state = SCTP_STATE_SHUTDOWN_PENDING)
>> +				packet->has_iflag = 1;
>> +
>>     
>
> Set the bit per chunk when adding it to packet.
>
> -vlad
>
>   
>>  			switch (status) {
>> +			case SCTP_XMIT_NAGLE_DELAY:
>> +				/* mark this chunks not be sent due to Nagle
>> +				 * algorithm
>> +				 */
>> +				chunk->nagle_qlen = q->out_qlen + 1;
>>  			case SCTP_XMIT_PMTU_FULL:
>>  			case SCTP_XMIT_RWND_FULL:
>> -			case SCTP_XMIT_NAGLE_DELAY:
>>  				/* We could not append this chunk, so put
>>  				 * the chunk back on the output queue.
>>  				 */
>> @@ -1011,6 +1022,12 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>>  				break;
>>  
>>  			case SCTP_XMIT_OK:
>> +				/* The sender has not enough queued user data
>> +				 * to send the remaining DATA chunks due to the
>> +				 * Nagle algorithm.
>> +				 */
>> +				if (chunk->nagle_qlen = q->out_qlen + 1)
>> +					packet->has_iflag = 1;
>>  				break;
>>  
>>  			default:
>>     
>
>
>
>
>   



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

* Re: [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY
  2009-09-04  6:38 [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY Wei Yongjun
  2009-09-04 17:46 ` Vlad Yasevich
  2009-09-08  7:10 ` Wei Yongjun
@ 2009-09-08 14:52 ` Vlad Yasevich
  2 siblings, 0 replies; 4+ messages in thread
From: Vlad Yasevich @ 2009-09-08 14:52 UTC (permalink / raw)
  To: linux-sctp

Wei Yongjun wrote:
> Vlad Yasevich wrote:
>> Wei Yongjun wrote:
>>   
>>> This patch implement the sender side for SACK-IMMEDIATELY
>>> extension.
>>>
>>>   Section 4.1.  Sender Side Considerations
>>>
>>>   Whenever the sender of a DATA chunk can benefit from the
>>>   corresponding SACK chunk being sent back without delay, the sender
>>>   MAY set the I-bit in the DATA chunk header.
>>>
>>>   Reasons for setting the I-bit include
>>>
>>>   o  The sender has not enough queued user data to send the remaining
>>>      DATA chunks due to the Nagle algorithm.
>>>
>>>   o  The sending of a DATA chunk fills the congestion or receiver
>>>      window.
>>>     
>> These reasons are still up for debate and BSD hasn't implemented them either.
>> For example, if the congestion window is filled, setting I bit is pointless
>> since either there are gaps and you'll get an immediate SACK anyway, or there
>> is sufficient DATA in flight that you'll get a SACK soon.
>>
>> Setting if for the 0-window event is kind of useless too.
>>
>> The Nagle algorithm is debatable as well.  With recent changes, Nagle doesn't
>> apply to large writes.  So the only time, this might be worth it is if
>> an application can't fill pathmtu within 200ms SACK timer.
>>   
> 
> I will remove those code in the next version.
> 
>>   
>>>   o  The sender is in the SHUTDOWN-PENDING state.
>>>     
>> This is really the only valid state.  Also, when SCTP_EOF is set on the message.
>> This part is actually something we don't do right.  It should be possible
>> to write a message and set SCTP_EOF at the same time, but we don't support it
>> yet.
>>   
> 
> I will fix SCTP_EOF to enable write a message and set SCTP_EOF at the
> same time.
> 
>> Also, please do the API patch along with it.  That's just exposing the flag
>> in sctp/user.h and honoring it.
>>   
> 
> Do you means add a new flag like SCTP_NODSACK or something else, and it can
> be specified in sendmsg()'s arguments like SCTP_EOF?

Yep.  See http://tools.ietf.org/html/draft-tuexen-tsvwg-sctp-sack-immediately-02.

-vlad

> 
> Regards,
> 
> Wei Yongjun
> 
>> Some more comments below.
>>
>>   
>>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>>> ---
>>>  include/net/sctp/structs.h |    2 ++
>>>  net/sctp/output.c          |   12 ++++++++++++
>>>  net/sctp/outqueue.c        |   19 ++++++++++++++++++-
>>>  3 files changed, 32 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>>> index 42d00ce..54ac504 100644
>>> --- a/include/net/sctp/structs.h
>>> +++ b/include/net/sctp/structs.h
>>> @@ -764,6 +764,7 @@ struct sctp_chunk {
>>>  		has_asconf:1,		/* IN: have seen an asconf before */
>>>  		tsn_missing_report:2,	/* Data chunk missing counter. */
>>>  		fast_retransmit:2;	/* Is this chunk fast retransmitted? */
>>> +	unsigned nagle_qlen;		/* Queued data length due to Nagle. */
>>>  };
>>>  
>>>  void sctp_chunk_hold(struct sctp_chunk *);
>>> @@ -826,6 +827,7 @@ struct sctp_packet {
>>>  	    has_sack:1,		/* This packet contains a SACK chunk. */
>>>  	    has_auth:1,		/* This packet contains an AUTH chunk */
>>>  	    has_data:1,		/* This packet contains at least 1 DATA chunk */
>>> +	    has_iflag:1,	/* This packet need I-Flag for last DATA chunk */
>>>     
>> You should be able to set this more then once per packet, so the packet flag
>> isn't the right way to do it.
>>
>>   
>>>  	    ipfragok:1,		/* So let ip fragment this packet */
>>>  	    malloced:1;		/* Is it malloced? */
>>>  };
>>> diff --git a/net/sctp/output.c b/net/sctp/output.c
>>> index 951f586..aad3ff5 100644
>>> --- a/net/sctp/output.c
>>> +++ b/net/sctp/output.c
>>> @@ -75,6 +75,7 @@ static void sctp_packet_reset(struct sctp_packet *packet)
>>>  	packet->has_cookie_echo = 0;
>>>  	packet->has_sack = 0;
>>>  	packet->has_data = 0;
>>> +	packet->has_iflag = 0;
>>>  	packet->has_auth = 0;
>>>  	packet->ipfragok = 0;
>>>  	packet->auth = NULL;
>>> @@ -447,6 +448,10 @@ int sctp_packet_transmit(struct sctp_packet *packet)
>>>  			} else
>>>  				chunk->resent = 1;
>>>  
>>> +			if (packet->has_iflag &&
>>> +			    list_empty(&packet->chunk_list))
>>> +				chunk->chunk_hdr->flags |= SCTP_DATA_SACK_IMM;
>>> +
>>>     
>> The setting of the bit should probably be done at chunk creation and when the
>> chunk is added to the packet (sctp_packet_append_data).
>>
>>   
>>>  			has_data = 1;
>>>  		}
>>>  
>>> @@ -743,6 +748,13 @@ static void sctp_packet_append_data(struct sctp_packet *packet,
>>>  	else
>>>  		rwnd = 0;
>>>  
>>> +	/* Reasons for setting the I-bit include
>>> +	 * - The sending of a DATA chunk fills the congestion or
>>> +	 *   receiver window.
>>> +	 */
>>> +	if (rwnd = 0 || transport->flight_size >= transport->cwnd)
>>> +		packet->has_iflag = 1;
>>> +
>>>  	asoc->peer.rwnd = rwnd;
>>>  	/* Has been accepted for transmission. */
>>>  	if (!asoc->peer.prsctp_capable)
>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>>> index c9f20e2..1193169 100644
>>> --- a/net/sctp/outqueue.c
>>> +++ b/net/sctp/outqueue.c
>>> @@ -995,10 +995,21 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>>>  			/* Add the chunk to the packet.  */
>>>  			status = sctp_packet_transmit_chunk(packet, chunk, 0);
>>>  
>>> +			/* The sender is in the SHUTDOWN-PENDING state,
>>> +			 * The sender MAY set the I-bit in the DATA
>>> +			 * chunk header.
>>> +			 */
>>> +			if (asoc->state = SCTP_STATE_SHUTDOWN_PENDING)
>>> +				packet->has_iflag = 1;
>>> +
>>>     
>> Set the bit per chunk when adding it to packet.
>>
>> -vlad
>>
>>   
>>>  			switch (status) {
>>> +			case SCTP_XMIT_NAGLE_DELAY:
>>> +				/* mark this chunks not be sent due to Nagle
>>> +				 * algorithm
>>> +				 */
>>> +				chunk->nagle_qlen = q->out_qlen + 1;
>>>  			case SCTP_XMIT_PMTU_FULL:
>>>  			case SCTP_XMIT_RWND_FULL:
>>> -			case SCTP_XMIT_NAGLE_DELAY:
>>>  				/* We could not append this chunk, so put
>>>  				 * the chunk back on the output queue.
>>>  				 */
>>> @@ -1011,6 +1022,12 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>>>  				break;
>>>  
>>>  			case SCTP_XMIT_OK:
>>> +				/* The sender has not enough queued user data
>>> +				 * to send the remaining DATA chunks due to the
>>> +				 * Nagle algorithm.
>>> +				 */
>>> +				if (chunk->nagle_qlen = q->out_qlen + 1)
>>> +					packet->has_iflag = 1;
>>>  				break;
>>>  
>>>  			default:
>>>     
>>
>>
>>
>>   
> 
> 


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

end of thread, other threads:[~2009-09-08 14:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-04  6:38 [PATCH 3/3] sctp: implement the sender side for SACK-IMMEDIATELY Wei Yongjun
2009-09-04 17:46 ` Vlad Yasevich
2009-09-08  7:10 ` Wei Yongjun
2009-09-08 14:52 ` Vlad Yasevich

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.