[PATCH] Add netlink domain sockets support to socketutils

[Dmitry V. Levin]

On Fri, May 13, 2016 at 12:02:37PM +0000, Fabien Siron wrote:
> * socketutils.c: (print_sockeaddr_by_inode): Call netlink_print.
> (netlink_send_query, netlink_parse_response, netlink_print): New functions.
> ---
>  socketutils.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/socketutils.c b/socketutils.c
> index 8851198..9159842 100644
> --- a/socketutils.c
> +++ b/socketutils.c
> @@ -34,7 +34,9 @@
>  #include <linux/sock_diag.h>
>  #include <linux/inet_diag.h>
>  #include <linux/unix_diag.h>
> +#include <linux/netlink_diag.h>

This header first appeared in linux v3.10-rc1, but strace can be built
with older kernel headers.

In similar cases of linux/sock_diag.h, linux/inet_diag.h, and
linux/unix_diag.h we have stripped down replacement headers.

>  #include <linux/rtnetlink.h>
> +# include "xlat/netlink_protocols.h"

No need to indent.

>  
>  #if !defined NETLINK_SOCK_DIAG && defined NETLINK_INET_DIAG
>  # define NETLINK_SOCK_DIAG NETLINK_INET_DIAG
> @@ -335,6 +337,60 @@ unix_parse_response(const char *proto_name, const void *data,
>  }
>  
>  static bool
> +netlink_send_query(const int fd, const unsigned long inode)
> +{
> +	struct {
> +		const struct nlmsghdr nlh;
> +		const struct netlink_diag_req idr;

In inet_send_query, idr has type struct inet_diag_req_v2.
If you follow this naming scheme, idr should be called ndr.

> +	} req = {
> +		.nlh = {
> +			.nlmsg_len = sizeof(req),
> +			.nlmsg_type = SOCK_DIAG_BY_FAMILY,
> +			.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST
> +		},
> +		.idr = {
> +			.sdiag_family = AF_NETLINK,
> +			.sdiag_protocol = NDIAG_PROTO_ALL,
> +			.ndiag_ino = (unsigned) inode,

Isn't implicit cast enough?

BTW, inode lookup is not implemented anyway, so like in case of
unix_send_query, .ndiag_ino initialization has no practical purpose.

> +			.ndiag_show = NDIAG_SHOW_MEMINFO
> +		}
> +	};
> +	return send_query(fd, &req, sizeof(req));
> +}
> +
> +static int
> +netlink_parse_response(const char *proto_name, const void *data,
> +		    const int data_len, const unsigned long inode)
> +{
> +	const struct netlink_diag_msg *const diag_msg = data;
> +	const char *netlink_proto;
> +	char *details;
> +
> +	if (data_len < (int) NLMSG_LENGTH(sizeof(*diag_msg)))
> +		return -1;
> +	if (diag_msg->ndiag_ino != inode)
> +		return 0;
> +
> +	if (diag_msg->ndiag_family != AF_NETLINK)
> +		return -1;
> +
> +	netlink_proto = xlookup(netlink_protocols,
> +				diag_msg->ndiag_protocol);
> +
> +	if (netlink_proto) {
> +		if (asprintf(&details, "%s:[%s]", proto_name,
> +			    netlink_proto) < 0)

I've tried this with "ip l" command, and it printed
<NETLINK:[NETLINK_ROUTE]>, which looks redundant.

Wouldn't it be more useful to print just netlink_proto but also add more
netlink specific information, e.g. <NETLINK_ROUTE:[ndiag_portid]>?

> +			return -1;
> +	} else {
> +		if (asprintf(&details, "%s:[%u]", proto_name,
> +			     (unsigned) diag_msg->ndiag_protocol) < 0)
> +			return -1;
> +	}
> +
> +	return cache_and_print_inode_details(inode, details);
> +}
> +
> +static bool
>  unix_print(const int fd, const unsigned long inode)
>  {
>  	return unix_send_query(fd, inode)
> @@ -365,6 +421,14 @@ udp_v6_print(const int fd, const unsigned long inode)
>  	return inet_print(fd, AF_INET6, IPPROTO_UDP, inode, "UDPv6");
>  }
>  
> +static bool
> +netlink_print(const int fd, const unsigned long inode)
> +{
> +	return netlink_send_query(fd, inode)
> +		&& receive_responses(fd, inode, "NETLINK",
> +				     netlink_parse_response);
> +}
> +
>  /* Given an inode number of a socket, print out the details
>   * of the ip address and port. */
>  bool
> @@ -378,7 +442,8 @@ print_sockaddr_by_inode(const unsigned long inode, const char *const proto_name)
>  		{ "UDP", udp_v4_print },
>  		{ "TCPv6", tcp_v6_print },
>  		{ "UDPv6", udp_v6_print },
> -		{ "UNIX", unix_print }
> +		{ "UNIX", unix_print },
> +		{ "NETLINK", netlink_print }
>  	};
>  
>  	const int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_SOCK_DIAG);

This looks OK.  Could you add a test similar to already existing net-yy-*
tests (not necessarily with so many different syscalls)?


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