Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-18 Thread 'Marcelo Ricardo Leitner'
On Wed, Aug 17, 2016 at 09:01:38AM +, David Laight wrote: > From: Marcelo Ricardo Leitner > > Sent: 16 August 2016 18:25 > ... > > > That doesn't seem a good idea. > > > You don't want to abort the association if there is a transient > > > memory allocation failure. > > > You also can't drop da

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-17 Thread Xin Long
>> > If letting the application see ENOMEM errors, and sctp has to drop this >> > chunk, instead of retransmiting the ENOMEM chunk, but the ENOMEM >> > chunk may not be the chunk from current msg, as it flush all the queue. >> > even if users get an ENOMEM error, they may re-send a chunk that is s

RE: [PATCH net] sctp: fix a success return may hide an error

2016-08-17 Thread David Laight
From: Marcelo Ricardo Leitner > Sent: 16 August 2016 18:25 ... > > That doesn't seem a good idea. > > You don't want to abort the association if there is a transient > > memory allocation failure. > > You also can't drop data chunks. > > From a system-wise POV, this behavior - to free the new asoc

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-16 Thread Marcelo Ricardo Leitner
On Tue, Aug 16, 2016 at 03:33:30PM -0300, Marcelo Ricardo Leitner wrote: > On Wed, Aug 17, 2016 at 02:24:19AM +0800, Xin Long wrote: > > >> > This err returns back to sctp_sendmsg, there sctp will abort asoc. > > > > > > That's not right I think. sctp_sendmsg will only free the asoc if it was > > >

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-16 Thread Marcelo Ricardo Leitner
On Wed, Aug 17, 2016 at 02:24:19AM +0800, Xin Long wrote: > >> > This err returns back to sctp_sendmsg, there sctp will abort asoc. > > > > That's not right I think. sctp_sendmsg will only free the asoc if it was > > created to send that specific chunk. And in this case, this change > > should have

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-16 Thread Xin Long
>> > This err returns back to sctp_sendmsg, there sctp will abort asoc. > > That's not right I think. sctp_sendmsg will only free the asoc if it was > created to send that specific chunk. And in this case, this change > should have no effect as it can't have sctp_outq_flush() touching > several tra

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-16 Thread Marcelo Ricardo Leitner
On Tue, Aug 16, 2016 at 04:01:50PM +, David Laight wrote: > From: Xin Long > > Sent: 16 August 2016 12:34 > > > > >> Both sctp_outq_flush_rtx and sctp_packet_transmit can ONLY > > >> return one error (-ENOMEM), as sctp_outq_flush_rtx also calls > > >> sctp_packet_transmit. > > > > > > What is t

RE: [PATCH net] sctp: fix a success return may hide an error

2016-08-16 Thread David Laight
From: Xin Long > Sent: 16 August 2016 12:34 > > >> Both sctp_outq_flush_rtx and sctp_packet_transmit can ONLY > >> return one error (-ENOMEM), as sctp_outq_flush_rtx also calls > >> sctp_packet_transmit. > > > > What is the effect of the error? > > If it is 'just' equivalent to a lost ethernet pack

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-16 Thread Xin Long
>> >> [1] >> Both sctp_outq_flush_rtx and sctp_packet_transmit can ONLY >> return one error (-ENOMEM), as sctp_outq_flush_rtx also calls >> sctp_packet_transmit. > > What is the effect of the error? > If it is 'just' equivalent to a lost ethernet packet (and the skb (etc) > is freed) then the proto

RE: [PATCH net] sctp: fix a success return may hide an error

2016-08-16 Thread David Laight
From: Xin Long > Sent: 13 August 2016 08:48 > > > > This style of error handling is dangerous. The first error can be > > lost. > > > > For example, if sctp_outq_flush_rtx() earlier in this function returns > > an error, it will be lost if any invocation of the function > > sctp_packet_transmit()

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-13 Thread Xin Long
> > This style of error handling is dangerous. The first error can be > lost. > > For example, if sctp_outq_flush_rtx() earlier in this function returns > an error, it will be lost if any invocation of the function > sctp_packet_transmit() at the end function signals an error. > > I think you shou

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-12 Thread David Miller
From: Xin Long Date: Thu, 11 Aug 2016 20:52:58 +0800 > Now in the end of sctp_outq_flush, sctp calls sctp_packet_transmit > in a loop. The return of current sctp_packet_transmit always covers > the prior one's. If the last call of sctp_packet_transmit return a > success, it may hide the error tha

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-11 Thread Neil Horman
On Thu, Aug 11, 2016 at 08:52:58PM +0800, Xin Long wrote: > Now in the end of sctp_outq_flush, sctp calls sctp_packet_transmit > in a loop. The return of current sctp_packet_transmit always covers > the prior one's. If the last call of sctp_packet_transmit return a > success, it may hide the error

Re: [PATCH net] sctp: fix a success return may hide an error

2016-08-11 Thread Marcelo Ricardo Leitner
On Thu, Aug 11, 2016 at 08:52:58PM +0800, Xin Long wrote: > Now in the end of sctp_outq_flush, sctp calls sctp_packet_transmit > in a loop. The return of current sctp_packet_transmit always covers > the prior one's. If the last call of sctp_packet_transmit return a > success, it may hide the error