Menu
▾
▴
Re: [tipc-discussion] [RFC PATCH 1/2] tipc: fix large latency in smart Nagle streaming
|
From: Jon M. <jm...@re...> - 2020-05-04 16:04:10
|
On 5/4/20 7:28 AM, Tuong Lien wrote:
> Currently when a connection is in Nagle mode, we set the 'ack_required'
> bit in the last sending buffer and wait for the corresponding ACK prior
> to pushing more data. However, on the receiving side, the ACK is issued
> only when application actually gets
/gets/reads/
> the whole data. Even if part of the
> last buffer is received, we will not do the ACK as required.
Hmm, this sounds more like a bug than a weakness. Good that you caught it.
> This might
> cause an unnecessary delay since the receiver does not always fetch the
> message as fast as the sender, resulting in a large latency in the user
> message sending, which is [one RTT + the receiver processing time].
>
> The commit makes Nagle ACK as soon as possible i.e. when a message with
> the 'ack_required' arrives in the receiving side's stack even before it
> is processed / backlogged, put in the socket receive queue...
> This way, we can limit the streaming latency to one RTT as committed in
> Nagle mode.
>
> v2: optimize code
> v3: rebase to non debug
> v4: rename patch subject
>
> Signed-off-by: Tuong Lien <tuo...@de...>
> ---
> net/tipc/socket.c | 43 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 693e8902161e..4e71774528ad 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -1739,22 +1739,21 @@ static int tipc_sk_anc_data_recv(struct msghdr *m, struct sk_buff *skb,
> return 0;
> }
>
> -static void tipc_sk_send_ack(struct tipc_sock *tsk)
> +static struct sk_buff *tipc_sk_build_ack(struct tipc_sock *tsk)
> {
> struct sock *sk = &tsk->sk;
> - struct net *net = sock_net(sk);
> struct sk_buff *skb = NULL;
> struct tipc_msg *msg;
> u32 peer_port = tsk_peer_port(tsk);
> u32 dnode = tsk_peer_node(tsk);
>
> if (!tipc_sk_connected(sk))
> - return;
> + return NULL;
> skb = tipc_msg_create(CONN_MANAGER, CONN_ACK, INT_H_SIZE, 0,
> dnode, tsk_own_node(tsk), peer_port,
> tsk->portid, TIPC_OK);
> if (!skb)
> - return;
> + return NULL;
> msg = buf_msg(skb);
> msg_set_conn_ack(msg, tsk->rcv_unacked);
> tsk->rcv_unacked = 0;
> @@ -1764,7 +1763,20 @@ static void tipc_sk_send_ack(struct tipc_sock *tsk)
> tsk->rcv_win = tsk_adv_blocks(tsk->sk.sk_rcvbuf);
> msg_set_adv_win(msg, tsk->rcv_win);
> }
> - tipc_node_xmit_skb(net, skb, dnode, msg_link_selector(msg));
> +
> + return skb;
> +}
> +
> +static void tipc_sk_send_ack(struct tipc_sock *tsk)
> +{
> + struct sk_buff *skb;
> +
> + skb = tipc_sk_build_ack(tsk);
> + if (!skb)
> + return;
> +
> + tipc_node_xmit_skb(sock_net(&tsk->sk), skb, tsk_peer_node(tsk),
> + msg_link_selector(buf_msg(skb)));
> }
>
> static int tipc_wait_for_rcvmsg(struct socket *sock, long *timeop)
> @@ -1938,7 +1950,6 @@ static int tipc_recvstream(struct socket *sock, struct msghdr *m,
> bool peek = flags & MSG_PEEK;
> int offset, required, copy, copied = 0;
> int hlen, dlen, err, rc;
> - bool ack = false;
> long timeout;
>
> /* Catch invalid receive attempts */
> @@ -1983,7 +1994,6 @@ static int tipc_recvstream(struct socket *sock, struct msghdr *m,
>
> /* Copy data if msg ok, otherwise return error/partial data */
> if (likely(!err)) {
> - ack = msg_ack_required(hdr);
> offset = skb_cb->bytes_read;
> copy = min_t(int, dlen - offset, buflen - copied);
> rc = skb_copy_datagram_msg(skb, hlen + offset, m, copy);
> @@ -2011,7 +2021,7 @@ static int tipc_recvstream(struct socket *sock, struct msghdr *m,
>
> /* Send connection flow control advertisement when applicable */
> tsk->rcv_unacked += tsk_inc(tsk, hlen + dlen);
> - if (ack || tsk->rcv_unacked >= tsk->rcv_win / TIPC_ACK_RATE)
> + if (tsk->rcv_unacked >= tsk->rcv_win / TIPC_ACK_RATE)
> tipc_sk_send_ack(tsk);
>
> /* Exit if all requested data or FIN/error received */
> @@ -2105,9 +2115,11 @@ static void tipc_sk_proto_rcv(struct sock *sk,
> * tipc_sk_filter_connect - check incoming message for a connection-based socket
> * @tsk: TIPC socket
> * @skb: pointer to message buffer.
> + * @xmitq: for Nagle ACK if any
> * Returns true if message should be added to receive queue, false otherwise
> */
> -static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
> +static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff *skb,
> + struct sk_buff_head *xmitq)
> {
> struct sock *sk = &tsk->sk;
> struct net *net = sock_net(sk);
> @@ -2171,8 +2183,17 @@ static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
> if (!skb_queue_empty(&sk->sk_write_queue))
> tipc_sk_push_backlog(tsk);
> /* Accept only connection-based messages sent by peer */
> - if (likely(con_msg && !err && pport == oport && pnode == onode))
> + if (likely(con_msg && !err && pport == oport &&
> + pnode == onode)) {
> + if (msg_ack_required(hdr)) {
> + struct sk_buff *skb;
> +
> + skb = tipc_sk_build_ack(tsk);
> + if (skb)
> + __skb_queue_tail(xmitq, skb);
> + }
> return true;
> + }
> if (!tsk_peer_msg(tsk, hdr))
> return false;
> if (!err)
> @@ -2267,7 +2288,7 @@ static void tipc_sk_filter_rcv(struct sock *sk, struct sk_buff *skb,
> while ((skb = __skb_dequeue(&inputq))) {
> hdr = buf_msg(skb);
> limit = rcvbuf_limit(sk, skb);
> - if ((sk_conn && !tipc_sk_filter_connect(tsk, skb)) ||
> + if ((sk_conn && !tipc_sk_filter_connect(tsk, skb, xmitq)) ||
> (!sk_conn && msg_connected(hdr)) ||
> (!grp && msg_in_group(hdr)))
> err = TIPC_ERR_NO_PORT;
Acked-by: Jon Maloy <jm...@re...>
|
