[PATCH v5] Print ip and port associated with descriptor with -yy

[Dmitry V. Levin]

On Mon, Aug 11, 2014 at 02:21:01PM +0530, zubin.mithra at gmail.com wrote:
[...]
> diff --git a/configure.ac b/configure.ac
> index 9aeb3a6..21dcc0e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -228,8 +228,11 @@ AC_CHECK_HEADERS(m4_normalize([
>  	inttypes.h
>  	ioctls.h
>  	linux/capability.h
> +	linux/inet_diag.h
> +	linux/netlink.h
>  	linux/perf_event.h
>  	linux/ptrace.h
> +	linux/sock_diag.h
>  	linux/utsname.h
>  	mqueue.h
>  	netinet/sctp.h
> @@ -258,6 +261,8 @@ AC_CHECK_HEADERS([netinet/tcp.h netinet/udp.h],,, [#include <netinet/in.h>])
>  
>  AC_CHECK_MEMBERS([struct msghdr.msg_control],,, [#include <sys/socket.h>])
>  
> +AC_CHECK_TYPES([struct inet_diag_req_v2],,, [#include <linux/inet_diag.h])

It has to be <linux/inet_diag.h>, otherwise HAVE_STRUCT_INET_DIAG_REQ_V2
is never going to be defined.

[...]
> diff --git a/socketutils.c b/socketutils.c
> new file mode 100644
> index 0000000..79e4d07
> --- /dev/null
> +++ b/socketutils.c
> @@ -0,0 +1,179 @@
> +#include "defs.h"
> +#include <netinet/in.h>
> +#include <sys/socket.h>
> +#include <arpa/inet.h>
> +#include <netlink/netlink.h>

What is <netlink/netlink.h>?  I suppose this header is not needed.

> +#ifdef HAVE_LINUX_NETLINK_H
> +# include <linux/netlink.h>
> +#endif

You are conditionally include <linux/netlink.h> here, but later
unconditionally use constants defined in that header.

I think you can safely assume that <linux/netlink.h> is available.
The only feasible alternative is to define dummy printsockdetails() when
<linux/netlink.h> is not available.

> +#ifdef HAVE_LINUX_SOCK_DIAG_H
> +# include <linux/sock_diag.h>
> +#else
> +# define SOCK_DIAG_BY_FAMILY 20
> +  struct sock_diag_req {
> +	  __u8    sdiag_family;
> +	  __u8    sdiag_protocol;
> +  };
> +#endif

We do not use kernel specific types, please use uint8_t instead.
Since you have to define SOCK_DIAG_BY_FAMILY and sock_diag_req anyway
(<linux/sock_diag.h> may not be available before linux 3.3),
why not just create linux/sock_diag.h file?

> +#ifdef HAVE_LINUX_INET_DIAG_H
> +# include <linux/inet_diag.h>
> +#else
> +  struct inet_diag_sockid {
> +	  __be16  idiag_sport;
> +	  __be16  idiag_dport;
> +	  __be32  idiag_src[4];
> +	  __be32  idiag_dst[4];
> +	  __u32   idiag_if;
> +	  __u32   idiag_cookie[2];
> +  #define INET_DIAG_NOCOOKIE (~0U)
> +  };
> +#endif
> +
> +/* Not all linux/inet_diag.h have v2 */
> +#ifndef HAVE_STRUCT_INET_DIAG_REQ_V2
> +  struct inet_diag_req_v2 {
> +	  __u8	sdiag_family;
> +	  __u8	sdiag_protocol;
> +	  __u8	idiag_ext;
> +	  __u8	pad;
> +	  __u32	idiag_states;
> +	  struct inet_diag_sockid id;
> +  };
> +#endif

All that was said about <linux/sock_diag.h> is also applicable to this case.

> +int
> +parse_response(struct inet_diag_msg *diag_msg, int inodenr)

This function has to be static.
diag_msg could be declared const.
inodenr has to be the same type all over the code.

> +{
> +	char remote_addr_buf[INET6_ADDRSTRLEN];
> +	int rport;

rport has to be uint16_t.

> +	char *ret;

This has to be const.

> +
> +	if (diag_msg->idiag_inode != inodenr)
> +		return -1;
> +
> +	switch(diag_msg->idiag_family) {
> +		case AF_INET:
> +			ret = inet_ntop(diag_msg->idiag_family,
> +			  (struct in_addr*) &(diag_msg->id.idiag_dst),
> +			  remote_addr_buf, INET_ADDRSTRLEN);
> +			break;
> +		case AF_INET6:
> +			ret = inet_ntop(diag_msg->idiag_family,
> +			  (struct in_addr*) &(diag_msg->id.idiag_dst),
> +			  remote_addr_buf, INET6_ADDRSTRLEN);
> +			break;
> +		default:
> +			return -1;
> +	}
> +
> +	if (!ret)
> +		return -1;
> +
> +	if (remote_addr_buf[0] == 0)
> +		return -1;

These two inet_ntop calls differ by one socklen_t argument, so
you could write this shorter:

	socklen_t size;
	switch(diag_msg->idiag_family) {
		case AF_INET:
			size = INET_ADDRSTRLEN;
			break;
		case AF_INET6:
			size = INET6_ADDRSTRLEN;
			break;
		default:
			return -1;
	}
	if (!inet_ntop(diag_msg->idiag_family, diag_msg->id.idiag_dst,
		       remote_addr_buf, size) || !remote_addr_buf[0])
		return -1;

> +	rport = ntohs(diag_msg->id.idiag_dport);
> +	tprintf("%s:%d", remote_addr_buf, rport);

This has to be %u.

> +	return 0;
> +}
> +
> +int
> +send_query(int sockfd, int proto, int family)
> +{
> +	struct msghdr msg;
> +	struct nlmsghdr nlh;
> +	struct inet_diag_req_v2 conn_req;
> +	struct sockaddr_nl sa;
> +	struct iovec iov[2];
> +
> +	memset(&msg, 0, sizeof(msg));
> +	memset(&sa, 0, sizeof(sa));
> +	memset(&nlh, 0, sizeof(nlh));
> +	memset(&conn_req, 0, sizeof(conn_req));
> +
> +	sa.nl_family = AF_NETLINK;
> +	conn_req.sdiag_family = family;
> +	conn_req.sdiag_protocol = proto;
> +	conn_req.idiag_states = -1;
> +
> +	nlh.nlmsg_len = NLMSG_LENGTH(sizeof(conn_req));
> +	nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST;
> +
> +	nlh.nlmsg_type = SOCK_DIAG_BY_FAMILY;
> +	iov[0].iov_base = (void*) &nlh;
> +	iov[0].iov_len = sizeof(nlh);
> +	iov[1].iov_base = (void*) &conn_req;
> +	iov[1].iov_len = sizeof(conn_req);
> +
> +	msg.msg_name = (void*) &sa;
> +	msg.msg_namelen = sizeof(sa);
> +	msg.msg_iov = iov;
> +	msg.msg_iovlen = 2;
> +
> +	return sendmsg(sockfd, &msg, 0);
> +}
> +
> +int
> +receive_responses(int sockfd, int inodenr)
> +{
> +	char recv_buf[SOCKET_BUFFER_SIZE];
> +	struct nlmsghdr *nlh;
> +	struct inet_diag_msg *diag_msg;
> +	int numbytes = 0;
> +	while (1) {
> +		numbytes = recv(sockfd, recv_buf, sizeof(recv_buf), 0);
> +		nlh = (struct nlmsghdr*) recv_buf;
> +
> +		while (NLMSG_OK(nlh, numbytes)) {
> +			if (nlh->nlmsg_type == NLMSG_DONE)
> +				return -1;
> +
> +			else if (nlh->nlmsg_type == NLMSG_ERROR)
> +				return -1;

I tested this patch on a x86_64 system with a fresh kernel.
It always returns NLMSG_ERROR.
I suppose something in send_query() is wrong.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140821/84f3fb06/attachment.bin>