> -----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
