Messages in this thread | | | From | 运辉崔 <> | Date | Fri, 6 Jan 2023 11:24:18 +0800 | Subject | Re: [External] Re: [PATCH v2] sock: add tracepoint for send recv length |
| |
On Thu, Jan 5, 2023 at 10:08 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Thu, 5 Jan 2023 18:00:14 +0800 > Yunhui Cui <cuiyunhui@bytedance.com> wrote: > > > @@ -715,6 +716,10 @@ static inline int sock_sendmsg_nosec(struct socket *sock, struct msghdr *msg) > > inet_sendmsg, sock, msg, > > msg_data_left(msg)); > > BUG_ON(ret == -EIOCBQUEUED); > > + > > + trace_sock_sendmsg_length(sock->sk, sock->sk->sk_family, > > + sock->sk->sk_protocol, > > + ret > 0 ? ret : 0, ret > 0 ? 0 : ret, 0); > > The length and error seem confusing. Basically, length is ret > 0 and error > is ret < 0, right? > > That means we don't need both. We could simply have: > > > + TP_ARGS(sk, family, protocol, length, error, flags), > > TP_ARGS(sk, family, protocol, ret, flags) >
Hi Steve, thank you for your advice,i'll modify it on v3.
> > > > @@ -992,9 +997,17 @@ INDIRECT_CALLABLE_DECLARE(int inet6_recvmsg(struct socket *, struct msghdr *, > > static inline int sock_recvmsg_nosec(struct socket *sock, struct msghdr *msg, > > int flags) > > { > > - return INDIRECT_CALL_INET(sock->ops->recvmsg, inet6_recvmsg, > > + int ret = INDIRECT_CALL_INET(sock->ops->recvmsg, inet6_recvmsg, > > inet_recvmsg, sock, msg, msg_data_left(msg), > > flags); > > + > > + trace_sock_recvmsg_length(sock->sk, sock->sk->sk_family, > > + sock->sk->sk_protocol, > > + (ret > 0 && !(flags & MSG_PEEK)) ? > > + ret : 0, > > + (ret > 0 && !(flags & MSG_PEEK)) ? 0 : ret, > > Since both length and error are zero when flags has MSG_PEEK set: > > trace_sock_recvmsg_length(sock->sk, sock->sk->sk_family, > sock->sk->sk_protocol, > !(flags & MSG_PEEK) ? ret : 0, > > -- Steve
Regardless of whether the MSG_PEEK flag is set or not, it is possible to return -errno, but based on your suggestion, I plan to modify it like this:
trace_sock_recvmsg_length(sock->sk, sock->sk->sk_family, sock->sk->sk_protocol, !(flags & MSG_PEEK) ? ret : (ret < 0 ? ret : 0),
what do you think?
Thanks, Yunhui
| |