Commit d8dd1578 authored by Neil Horman's avatar Neil Horman Committed by Vlad Yasevich

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: default avatarNeil Horman <nhorman@tuxdriver.com>
Reported-by: default avatarWilliam Reich <reich@ulticom.com>
Signed-off-by: default avatarVlad Yasevich <vladislav.yasevich@hp.com>
parent 46d5a808
...@@ -429,9 +429,7 @@ int sctp_packet_transmit(struct sctp_packet *packet) ...@@ -429,9 +429,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)
list_del_init(&chunk->list); list_del_init(&chunk->list);
if (sctp_chunk_is_data(chunk)) { if (sctp_chunk_is_data(chunk)) {
if (!chunk->has_tsn) { if (!chunk->resent) {
sctp_chunk_assign_ssn(chunk);
sctp_chunk_assign_tsn(chunk);
/* 6.3.1 C4) When data is in flight and when allowed /* 6.3.1 C4) When data is in flight and when allowed
* by rule C5, a new RTT measurement MUST be made each * by rule C5, a new RTT measurement MUST be made each
...@@ -444,7 +442,8 @@ int sctp_packet_transmit(struct sctp_packet *packet) ...@@ -444,7 +442,8 @@ int sctp_packet_transmit(struct sctp_packet *packet)
chunk->rtt_in_progress = 1; chunk->rtt_in_progress = 1;
tp->rto_pending = 1; tp->rto_pending = 1;
} }
} else }
chunk->resent = 1; chunk->resent = 1;
has_data = 1; has_data = 1;
...@@ -722,6 +721,8 @@ static void sctp_packet_append_data(struct sctp_packet *packet, ...@@ -722,6 +721,8 @@ static void sctp_packet_append_data(struct sctp_packet *packet,
/* Has been accepted for transmission. */ /* Has been accepted for transmission. */
if (!asoc->peer.prsctp_capable) if (!asoc->peer.prsctp_capable)
chunk->msg->can_abandon = 0; 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, static sctp_xmit_t sctp_packet_will_fit(struct sctp_packet *packet,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment