On Sat, Sep 10, 2016 at 12:03:53AM +0800, Xin Long wrote:
> > I don't know, I still don't feel safe about it. I agree the socket lock
> > keeps
> > the state from changing during a single transmission, which makes the use
> > case
> > you are focused on correct.
> ok, :-)
>
> >
> > That said, have you considered the retransmit case? That is to say, if you
> > queue and flush the outq, and some packets fail delivery, and in the time
> > between the intial send and the expiration of the RTX timer (during which
> > the
> > socket lock will have been released), an event may occur which changes the
> > transport state, which will then be ignored with your patch.
> Sorry, I'm not sure if I got it.
>
> You mean "during which changes q->asoc->state", right ?
>
> This patch removes the check of q->asoc->state in sctp_outq_tail().
>
> sctp_outq_tail() is called for data only in:
> sctp_primitive_SEND -> sctp_do_sm -> sctp_cmd_send_msg ->
> sctp_cmd_interpreter -> sctp_cmd_send_msg() -> sctp_outq_tail()
>
> before calling sctp_primitive_SEND, hold sock lock first.
> then sctp_primitive_SEND choose FUNC according:
>
> #define TYPE_SCTP_PRIMITIVE_SEND {
> ....
>
> if asoc->state is unavailable, FUNC can't be sctp_cmd_send_msg,
> but sctp_sf_error_closed/sctp_sf_error_shutdown, sctp_outq_tail
> can't be called, either.
> I mean sctp_primitive_SEND do the same check for asoc->state
> already actually.
>
> so the code in sctp_outq_tail is redundant actually.
>
>
>
>
> >
> > Neil
> >
>
Ok, you've convinced me, thanks for taking the time to go through it
Acked-by: Neil Horman <[email protected]>
