Re: [tipc-discussion] [PATCHv2 net-next] tipc: keep the skb in rcv queue until the whole data is read

[Jon M. <jm...@re...>]

On 7/5/21 11:57 PM, maloy wrote:
>
>
>
>
> Sent from my Galaxy
>
>
> -------- Original message --------
> From: Xin Long <luc...@gm...>
> Date: 2021-06-30 20:21 (GMT-05:00)
> To: Jon Maloy <jm...@re...>, Erin Shepherd 
> <eri...@e4...>, tip...@li...
> Subject: [tipc-discussion] [PATCHv2 net-next] tipc: keep the skb in 
> rcv queue until the whole data is read
>
> Currently, when userspace reads a datagram with a buffer that is
> smaller than this datagram, the data will be truncated and only
> part of it can be received by users. It doesn't seem right that
> users don't know the datagram size and have to use a huge buffer
> to read it to avoid the truncation.
>
> This patch to fix it by keeping the skb in rcv queue until the
> whole data is read by users. Only the last msg of the datagram
> will be marked with MSG_EOR, just as TCP/SCTP does.
>
> Note that this will work as above only when MSG_EOR is set in the
> flags parameter of recvmsg(), so that it won't break any old user
> applications.
>
> v1->v2:
>   - To enable this only when the flags with MSG_EOR is passed into
>     recvmsg() to fix the compatibility isssue as Erin noticed.
>
> Signed-off-by: Xin Long <luc...@gm...>
> ---
> net/tipc/socket.c | 36 +++++++++++++++++++++++++++---------
> 1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 34a97ea36cc8..9b0b311c7ec1 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -1880,6 +1880,7 @@ static int tipc_recvmsg(struct socket *sock, 
> struct msghdr *m,
> bool connected = !tipc_sk_type_connectionless(sk);
> struct tipc_sock *tsk = tipc_sk(sk);
> int rc, err, hlen, dlen, copy;
> + struct tipc_skb_cb *skb_cb;
> struct sk_buff_head xmitq;
> struct tipc_msg *hdr;
> struct sk_buff *skb;
> @@ -1903,6 +1904,7 @@ static int tipc_recvmsg(struct socket *sock, 
> struct msghdr *m,
> if (unlikely(rc))
> goto exit;
> skb = skb_peek(&sk->sk_receive_queue);
> + skb_cb = TIPC_SKB_CB(skb);
> hdr = buf_msg(skb);
> dlen = msg_data_sz(hdr);
> hlen = msg_hdr_sz(hdr);
> @@ -1922,18 +1924,33 @@ static int tipc_recvmsg(struct socket *sock, 
> struct msghdr *m,
>
> /* Capture data if non-error msg, otherwise just set return value */
> if (likely(!err)) {
> - copy = min_t(int, dlen, buflen);
> - if (unlikely(copy != dlen))
> - m->msg_flags |= MSG_TRUNC;
> - rc = skb_copy_datagram_msg(skb, hlen, m, copy);
> + int offset = skb_cb->bytes_read;
> +
> + copy = min_t(int, dlen - offset, buflen);
> + rc = skb_copy_datagram_msg(skb, hlen + offset, m, copy);
> + if (unlikely(rc))
> + goto exit;
> + if (unlikely(offset + copy < dlen)) {
> + if (flags & MSG_EOR) {
> + if (!(flags & MSG_PEEK))
> + skb_cb->bytes_read = offset + copy;
> + } else {
> + m->msg_flags |= MSG_TRUNC;
> + skb_cb->bytes_read = 0;
> + }
> + } else {
> + if (flags & MSG_EOR)
> + m->msg_flags |= MSG_EOR;
> + skb_cb->bytes_read = 0;
> + }
> } else {
> copy = 0;
> rc = 0;
> - if (err != TIPC_CONN_SHUTDOWN && connected && !m->msg_control)
> + if (err != TIPC_CONN_SHUTDOWN && connected && !m->msg_control) {
> rc = -ECONNRESET;
> + goto exit;
> + }
> }
> - if (unlikely(rc))
> - goto exit;
>
> /* Mark message as group event if applicable */
> if (unlikely(grp_evt)) {
> @@ -1956,9 +1973,10 @@ static int tipc_recvmsg(struct socket *sock, 
> struct msghdr *m,
> tipc_node_distr_xmit(sock_net(sk), &xmitq);
> }
>
> - tsk_advance_rx_queue(sk);
> + if (!skb_cb->bytes_read)
> + tsk_advance_rx_queue(sk);
>
> - if (likely(!connected))
> + if (likely(!connected) || skb_cb->bytes_read)
> goto exit;
>
> /* Send connection flow control advertisement when applicable */
> -- 
> 2.27.0
>
>
> Signed-off-by: Jon Maloy <jm...@re...>
Acked-by, of course ;-)
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion