lkml.org 
[lkml]   [2012]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
    On Mon, Jul 23, 2012 at 10:30:34AM +0800, xufeng zhang wrote:
    > On 07/23/2012 08:49 AM, Neil Horman wrote:
    > >
    > >Not sure I understand how you came into this error. If we get an invalid
    > >stream, we issue an SCTP_REPORT_TSN side effect, followed by an SCTP_CMD_REPLY
    > >which sends the error chunk. The reply goes through
    > >sctp_outq_tail->sctp_outq_chunk->sctp_outq_transmit_chunk->sctp_outq_append_chunk.
    > >That last function checks to see if a sack is already part of the packet, and if
    > >there isn't one, appends one, using the updated tsn map.
    > Yes, you are right, but consider the invalid stream identifier's
    > DATA chunk is the first
    > DATA chunk in the association which will need SACK immediately.
    > Here is what I thought of the scenario:
    > sctp_sf_eat_data_6_2()
    > -->sctp_eat_data()
    > -->sctp_make_op_error()
    > -->sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(err))
    > -->sctp_outq_tail() /* First enqueue ERROR chunk */
    > -->sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE())
    > -->sctp_gen_sack()
    > -->sctp_make_sack()
    > -->sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
    > SCTP_CHUNK(sack))
    > -->sctp_outq_tail() /* Then enqueue SACK chunk */
    >
    > So SACK chunk is enqueued after ERROR chunk.
    Ah, I see. Since the ERROR and SACK chunks are both control chunks, and since
    we explicitly add the SACK to the control queue instead of going through the
    bundle path in sctp_packet_append_chunk the ordering gets wrong.

    Ok, so the problem makes sense. I think the soultion could be alot easier
    though. IIRC SACK chunks always live at the head of a packet, so why not just
    special case it in sctp_outq_tail? I.e. instead of doing a list_add_tail, in
    the else clause of sctp_outq_tail check the chunk_hdr->type to see if its
    SCTP_CID_SACK. If it is, use list_add_head rather than list_add_tail. I think
    that will fix up both the COOKIE_ECHO and ESTABLISHED cases, won't it? And then
    you won't have keep track of extra state in the packet configuration.

    Regards
    Neil



    \
     
     \ /
      Last update: 2012-07-23 15:01    [W:6.431 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site