Neil Horman wrote: >> We could do that yes, but it concerns me, as assigning the tsn in >>> sctp_outq_flush leaves us in a position where we assign tsn to chunks that might >>> get dropped prior to submission to the ip layer. Consider if we have a routing >>> table disruption, and we follow the no_route path in sctp_packet_transmit. In >>> that situation, we will discard chunks with tsns assigned, leaving a gap in our >>> stream. Unless we have a recovery path for that, I think the better option is >>> to wait to assign tsns until we are sure we can submit them to the ip layer >>> safely (where the transmitted queue can re-tranmit them if need be). If you can >>> explain the SACK case in a little more detail above, perhaps we can come up with >>> some logic to govern when it is and is not safe to call sctp_packet_transmit >>> from sctp_packet_transmit_chunk for data chunks. >> Assume that we have a number of queued chunks that add up to multiple MTUs >> all going to the same transport (typical case). They are currently gated by >> congestion window. >> >> A SACK arrives triggering a flush. With the proposed patch, once we fill a >> single MTU, the main loop in sctp_outq_flush will exist and we will transmit >> only a single packet and under-utilize our congesting window thus preventing >> future growth. With the old code, we had multiple packets sent out thus >> filling the congestion window. >> >> Another thing your patch didn't take into account is that every time we change >> the transport in sctp_outq_flush, we reset the packet, effectively marking it >> empty. You would end up leaking chunks if there was any queuing effects. >> >> If a transient routing problem happens and the packet fails to get sent, that's >> no different then a loss event in the network. It will get reported back as >> gaps or, if the failure is long term, it will be detected with HBs and >> retransmissions. So I don't see a problem of assigning TSNs when the DATA is >> added to the packet. We don't really want to do it any earlier though. >> > > Yeah, ok, heres a new version, instead of just skipping the packet transmit in > transmit_chunk, we instead simply assign a tsn in sctp_outq_flush, after we > dequeue a data chunk from the outq and do the normal expiration and invalid > stream checking. > > Regards > Neil > Hi Neil I don't think we can do that in sctp_outq_flush(). You can do it in either sctp_packet_append_data() or sctp_packet_append_chunk(). I think it will be cleaner in append_data(), but that's your choice. -vlad -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
