On Sun, Jun 27, 2021 at 3:44 PM Erin Shepherd <erin.sheph...@e43.eu> wrote:
>
> Xin Long <lucien....@gmail.com> writes:
>
> > 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.
>
> I agree that the current behavior is suboptimal, but:
>
> * Isn't this the same behavior that other datagram socket types
> exhibit? It seems like this would make TIPC behave inconsistently
> compared to other transports
Yes, SCTP.
Do you see any reliable datagram transports not doing this?
> * Isn't this a compatibility break with existing software? Particularly
> existing software will not expect to receive trailers of overlong
> datagrams
I talked to Jon about this, he seems okay with this.
>
> It feels like this behavior should be activated either with a
> setsockopt(2) call or a new MSG_* flag passed to recv
Anyway, It may not be worth a new sockopt.
I'm thinking to pass MSG_EOR into sendmsg:
sendmsg(MSG_EOR).
to indicate we don't want the truncating msg.
When the msg flag returns with no MSG_EOR, it means there's more data to read.
Thanks.
>
> - Erin
>
> > Signed-off-by: Xin Long <lucien....@gmail.com>
> > ---
> > net/tipc/socket.c | 30 +++++++++++++++++++++---------
> > 1 file changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> > index 34a97ea36cc8..504e59838b8b 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,27 @@ 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_PEEK))
> > + skb_cb->bytes_read = offset + copy;
> > + } else {
> > + 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 +1967,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
> >
> >
> >
> > _______________________________________________
> > tipc-discussion mailing list
> > tipc-discussion@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/tipc-discussion
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
