diff options
author | Neil Horman <nhorman@tuxdriver.com> | 2009-11-23 23:54:00 +0300 |
---|---|---|
committer | Vlad Yasevich <vladislav.yasevich@hp.com> | 2009-11-23 23:54:00 +0300 |
commit | d8dd15781dd621c5ceab79083f4c5112787863f5 (patch) | |
tree | 49ee8bfc01d2ded7130a0c17298ca7a9db998fa4 | |
parent | 46d5a808558181e03a4760d2188cc9879445738a (diff) | |
download | linux-d8dd15781dd621c5ceab79083f4c5112787863f5.tar.xz |
sctp: Fix mis-ordering of user space data when multihoming in use
Recently had a bug reported to me, in which the user was sending
packets with a payload containing a sequence number. The packets
were getting delivered in order according the chunk TSN values, but
the sequence values in the payload were arriving out of order. At
first I thought it must be an application error, but we eventually
found it to be a problem on the transmit side in the sctp stack.
The conditions for the error are that multihoming must be in use,
and it helps if each transport has a different pmtu. The problem
occurs in sctp_outq_flush. Basically we dequeue packets from the
data queue, and attempt to append them to the orrered packet for a
given transport. After we append a data chunk we add the trasport
to the end of a list of transports to have their packets sent at
the end of sctp_outq_flush. The problem occurs when a data chunks
fills up a offered packet on a transport. The function that does
the appending (sctp_packet_transmit_chunk), will try to call
sctp_packet_transmit on the full packet, and then append the chunk
to a new packet. This call to sctp_packet_transmit, sends that
packet ahead of the others that may be queued in the transport_list
in sctp_outq_flush. The result is that frames that were sent in one
order from the user space sending application get re-ordered prior
to tsn assignment in sctp_packet_transmit, resulting in mis-sequencing
of data payloads, even though tsn ordering is correct.
The fix is to change where we assign a tsn. By doing this earlier,
we are then free to place chunks in packets, whatever way we
see fit and the protocol will make sure to do all the appropriate
re-ordering on receive as is needed.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Reported-by: William Reich <reich@ulticom.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
-rw-r--r-- | net/sctp/output.c | 25 |
1 files changed, 13 insertions, 12 deletions
diff --git a/net/sctp/output.c b/net/sctp/output.c index b210d2077e28..7c5589363433 100644 --- a/net/sctp/output.c +++ b/net/sctp/output.c @@ -429,23 +429,22 @@ int sctp_packet_transmit(struct sctp_packet *packet) list_del_init(&chunk->list); if (sctp_chunk_is_data(chunk)) { - if (!chunk->has_tsn) { - sctp_chunk_assign_ssn(chunk); - sctp_chunk_assign_tsn(chunk); - - /* 6.3.1 C4) When data is in flight and when allowed - * by rule C5, a new RTT measurement MUST be made each - * round trip. Furthermore, new RTT measurements - * SHOULD be made no more than once per round-trip - * for a given destination transport address. - */ + if (!chunk->resent) { + + /* 6.3.1 C4) When data is in flight and when allowed + * by rule C5, a new RTT measurement MUST be made each + * round trip. Furthermore, new RTT measurements + * SHOULD be made no more than once per round-trip + * for a given destination transport address. + */ if (!tp->rto_pending) { chunk->rtt_in_progress = 1; tp->rto_pending = 1; } - } else - chunk->resent = 1; + } + + chunk->resent = 1; has_data = 1; } @@ -722,6 +721,8 @@ static void sctp_packet_append_data(struct sctp_packet *packet, /* Has been accepted for transmission. */ if (!asoc->peer.prsctp_capable) chunk->msg->can_abandon = 0; + sctp_chunk_assign_tsn(chunk); + sctp_chunk_assign_ssn(chunk); } static sctp_xmit_t sctp_packet_will_fit(struct sctp_packet *packet, |