Re: [tipc-discussion] [net-next v3 1/1] tipc: add smart nagle feature

[Jon Maloy via tipc-discussion]

> -----Original Message-----
> From: Tuong Lien Tong <tuong.t.l...@dektech.com.au>
> Sent: 30-Oct-19 07:39
> To: Jon Maloy <jon.ma...@ericsson.com>; 'Jon Maloy' <ma...@donjonn.com>
> Cc: Mohan Krishna Ghanta Krishnamurthy 
> <mohan.krishna.ghanta.krishnamur...@ericsson.com>;
> parthasarathy.bhuvara...@gmail.com; Tung Quang Nguyen 
> <tung.q.ngu...@dektech.com.au>; Hoang
> Huu Le <hoang.h...@dektech.com.au>; Gordan Mihaljevic 
> <gordan.mihalje...@dektech.com.au>;
> ying....@windriver.com; tipc-discussion@lists.sourceforge.net
> Subject: RE: [net-next v3 1/1] tipc: add smart nagle feature
> 
> Hi Jon,
> 
> Please see below one comment from my side.
> 
> BR/Tuong
> 
> @@ -1437,16 +1492,17 @@ static int __tipc_sendstream(struct socket *sock,
> struct msghdr *m, size_t dlen)
>       struct sock *sk = sock->sk;
>       DECLARE_SOCKADDR(struct sockaddr_tipc *, dest, m->msg_name);
>       long timeout = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
> +     struct sk_buff_head *txq = &sk->sk_write_queue;
>       struct tipc_sock *tsk = tipc_sk(sk);
>       struct tipc_msg *hdr = &tsk->phdr;
>       struct net *net = sock_net(sk);
> -     struct sk_buff_head pkts;
>       u32 dnode = tsk_peer_node(tsk);
> +     int blocks = tsk->snd_backlog;
> +     int maxnagle = tsk->maxnagle;
> +     int maxpkt = tsk->max_pkt;
>       int send, sent = 0;
>       int rc = 0;
> 
> -     __skb_queue_head_init(&pkts);
> -
>       if (unlikely(dlen > INT_MAX))
>               return -EMSGSIZE;
> 
> @@ -1467,21 +1523,35 @@ static int __tipc_sendstream(struct socket *sock,
> struct msghdr *m, size_t dlen)
>                                        tipc_sk_connected(sk)));
>               if (unlikely(rc))
>                       break;
> -
>               send = min_t(size_t, dlen - sent, TIPC_MAX_USER_MSG_SIZE);
> -             rc = tipc_msg_build(hdr, m, sent, send, tsk->max_pkt,
> &pkts);
> -             if (unlikely(rc != send))
> -                     break;
> 
> -             trace_tipc_sk_sendstream(sk, skb_peek(&pkts),
> 
> [Tuong]: Should we set the 'blocks' here instead i.e. blocks =
> tsk->snd_backlog' as it can be changed if we have to release the sock &
> sleep in advance (e.g. tipc_wait_for_cond), also the 'while' statement can
> be re-run?

Yes, you are right. I'll fix this.

///jon

> 
> +             if (tsk->oneway++ >= 4 && send <= maxnagle) {
> +                     rc = tipc_msg_append(hdr, m, send, maxnagle, txq);
> +                     if (rc < 0)
> +                             break;
> +                     blocks += rc;
> +                     if (blocks <= 64 && tsk->expect_ack) {
> +                             tsk->snd_backlog = blocks;
> +                             sent += send;
> +                             break;
> +                     }
> +                     tsk->expect_ack = true;
> +             } else {
> +                     rc = tipc_msg_build(hdr, m, sent, send, maxpkt,
> txq);
> +                     if (unlikely(rc != send))
> +                             break;
> +                     blocks += tsk_inc(tsk, send + MIN_H_SIZE);
> +             }
> +             trace_tipc_sk_sendstream(sk, skb_peek(txq),
>                                        TIPC_DUMP_SK_SNDQ, " ");
> -             rc = tipc_node_xmit(net, &pkts, dnode, tsk->portid);
> +             rc = tipc_node_xmit(net, txq, dnode, tsk->portid);
>               if (unlikely(rc == -ELINKCONG)) {
>                       tsk->cong_link_cnt = 1;
>                       rc = 0;
>               }
>               if (likely(!rc)) {
> -                     tsk->snt_unacked += tsk_inc(tsk, send + MIN_H_SIZE);
> +                     tsk->snt_unacked += blocks;
> +                     tsk->snd_backlog = 0;
>                       sent += send;
>               }
>       } while (sent < dlen && !rc);
> @@ -1528,6 +1598,7 @@ static void tipc_sk_finish_conn(struct tipc_sock *tsk,
> u32 peer_port,
>       tipc_node_add_conn(net, peer_node, tsk->portid, peer_port);
>       tsk->max_pkt = tipc_node_get_mtu(net, peer_node, tsk->portid);
> 


_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion