Hi Partha, Forget about my previous comment. On a closer look, I find that your patch is correct. (But you should still rephrase.) Both patches acked by: me
///jon > -----Original Message----- > From: Jon Maloy [mailto:jon.ma...@ericsson.com] > Sent: Friday, February 24, 2017 09:51 AM > To: Parthasarathy Bhuvaragan <parthasarathy.bhuvara...@ericsson.com>; > tipc-discussion@lists.sourceforge.net; Ying Xue <ying....@windriver.com>; > pbut...@sonusnet.com > Subject: Re: [tipc-discussion] [PATCH net v1 1/2] tipc: fix socket flow > control > errors > > > > > -----Original Message----- > > From: Parthasarathy Bhuvaragan > > Sent: Friday, February 24, 2017 08:51 AM > > To: tipc-discussion@lists.sourceforge.net; Jon Maloy > > <jon.ma...@ericsson.com>; Ying Xue <ying....@windriver.com>; > > pbut...@sonusnet.com > > Subject: [PATCH net v1 1/2] tipc: fix socket flow control errors > > > > In this commit, we fix the following two errors: > > 1. In tipc_send_stream(), fix the return value during congestion > > when the send is partially successful. Until now, we return -1 > > instead of returning the partial sent bytes. > > 2. In tipc_recv_stream(), we update the rcv_unack not based on the > > message size, but on sz. Usually they are the same, but in cases > > where the socket receivers buffer is smaller than the incoming > > message, these two parameters differ greatly. > > I was first confused by this, but I assume you with 'socket receive buffer' in > this case mean the read buffer given by the user when he does read(), not > sk->sk_rcvbuf as I first thought. I suggest you rephrase this. > Worse, I don't think this fix is correct. Now, you will add the full > msg_data_sz() at each iteration, something that will lead to a skew in the > other direction, -the reader will now count in *more* blocks than the > sender, eventually leading the sender's snt_unacked to step around (and > become 64k since it is a u16) and in practice disabling the flow control > altogether. I believe the correction must still be based on 'sz', but > compensating for the fact that tsk_inc() is non-linear, i.e., it adds an > extra > block at each iteration, which I think is the real problem you have here. I > will > also need to spend some thought on the impact on the legacy flow control, > which must still work. > > Regards > ///jon > > > This introduces a > > slack in accounting leading to permanent congestion. In this > > commit, we perform accounting always based on the incoming message. > > > > Signed-off-by: Parthasarathy Bhuvaragan > > <parthasarathy.bhuvara...@ericsson.com> > > --- > > net/tipc/socket.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/net/tipc/socket.c b/net/tipc/socket.c index > > 6b09a778cc71..79e628cd08a9 100644 > > --- a/net/tipc/socket.c > > +++ b/net/tipc/socket.c > > @@ -1080,7 +1080,7 @@ static int __tipc_sendstream(struct socket > > *sock, struct msghdr *m, size_t dlen) > > } > > } while (sent < dlen && !rc); > > > > - return rc ? rc : sent; > > + return sent ? sent : rc; > > } > > > > /** > > @@ -1481,16 +1481,15 @@ static int tipc_recv_stream(struct socket > > *sock, struct msghdr *m, > > if (unlikely(flags & MSG_PEEK)) > > goto exit; > > > > - tsk->rcv_unacked += tsk_inc(tsk, hlen + sz); > > + tsk->rcv_unacked += tsk_inc(tsk, hlen + msg_data_sz(msg)); > > if (unlikely(tsk->rcv_unacked >= (tsk->rcv_win / 4))) > > tipc_sk_send_ack(tsk); > > tsk_advance_rx_queue(sk); > > > > /* Loop around if more data is required */ > > - if ((sz_copied < buf_len) && /* didn't get all requested data */ > > + if ((!err) && (sz_copied < buf_len) && > > (!skb_queue_empty(&sk->sk_receive_queue) || > > - (sz_copied < target)) && /* and more is ready or required */ > > - (!err)) /* and haven't reached a FIN */ > > + (sz_copied < target))) > > goto restart; > > > > exit: > > -- > > 2.1.4 > > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most engaging > tech sites, SlashDot.org! http://sdm.link/slashdot > _______________________________________________ > tipc-discussion mailing list > tipc-discussion@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tipc-discussion ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
