[PATCH v5 4/4] Netlink: handle multi netlink messages

[Dmitry V. Levin]

On Mon, Jun 20, 2016 at 04:06:45PM +0000, Fabien Siron wrote:
> This commit handles the case where there are several messages in the buffer.
> This is very useful to some protocols like SOCK_DIAG.
> 
> * netlink.c (nlmsg_fetch, nlmsg_next): New functions.
> (decode_netlink_msg): New function.
> (decode_netlink): Call decode_netlink_msg().
> * tests/netlink_parsing.c (send_query): Adapt test.
> ---
>  netlink.c               | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  tests/netlink_parsing.c |  8 ++++----
>  2 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/netlink.c b/netlink.c
> index c43f6e7..fecc413 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -31,8 +31,34 @@
>  #include "xlat/netlink_flags.h"
>  #include "xlat/netlink_types.h"
>  
> -void
> -decode_netlink(struct tcb *tcp, unsigned long addr, unsigned long size)
> +/* since our target is not in the same process, here are some utils for nlmsg */
> +static int
> +nlmsg_fetch(struct tcb *tcp, struct nlmsghdr *nlmsghdr, unsigned long addr,
> +	    unsigned long len) {
> +	if (len < sizeof(struct nlmsghdr)) {
> +		if (len != 0)
> +			printstr(tcp, addr, len);
> +		return 0;
> +	}
> +
> +	if (umove_or_printaddr(tcp, addr, nlmsghdr) == -1)
> +		return 0;
> +
> +	return nlmsghdr->nlmsg_len >= sizeof(struct nlmsghdr) &&
> +		nlmsghdr->nlmsg_len <= len;
> +}
> +
> +static unsigned long
> +nlmsg_next(struct nlmsghdr *nlmsghdr, unsigned long addr, unsigned long *len) {
> +	*len -= NLMSG_ALIGN(nlmsghdr->nlmsg_len);
> +
> +	return (unsigned long)
> +		((char *)(addr) + NLMSG_ALIGN(nlmsghdr->nlmsg_len));

What's the use of these casts to (char *) and back?

What's going to happen when
- NLMSG_ALIGN(nlmsghdr->nlmsg_len) == 0
- NLMSG_ALIGN(nlmsghdr->nlmsg_len) > len?

> +}
> +
> +static void
> +decode_netlink_msg(struct tcb *tcp, unsigned long addr,
> +			    unsigned long size)
>  {
>  	struct nlmsghdr nlmsghdr;
>  
> @@ -62,3 +88,18 @@ decode_netlink(struct tcb *tcp, unsigned long addr, unsigned long size)
>  
>  	tprints("}");
>  }
> +
> +void
> +decode_netlink(struct tcb *tcp, unsigned long addr, unsigned long total_size) {
> +	struct nlmsghdr nlmsghdr;
> +	unsigned long size = total_size;
> +
> +	for (; nlmsg_fetch(tcp, &nlmsghdr, addr, size);
> +	     addr = nlmsg_next(&nlmsghdr, addr, &size)) {
> +		if (size != total_size)
> +			tprints(", ");
> +		tprints("{");
> +		decode_netlink_msg(tcp, addr, size);
> +		tprints("}");

Square brackets are more appropriate for this sequence of structures,
one before the first element and one after the last.  No need to enclose
each element in additional brackets.  I'd say if there is only one
element, no need to enclose it in additional brackets, too.

Should there be a limit on the number of elements printed?

> +	}
> +}
> diff --git a/tests/netlink_parsing.c b/tests/netlink_parsing.c
> index 3e7c983..61957cd 100644
> --- a/tests/netlink_parsing.c
> +++ b/tests/netlink_parsing.c
> @@ -61,8 +61,8 @@ send_query(const int fd)
>  	    (unsigned) sizeof(req))
>  		perror_msg_and_skip("sendto");
>  
> -	printf("sendto(%d, {{len=%u, type=NLMSG_NOOP, flags=NLM_F_REQUEST|0x%x"
> -	       ", seq=0, pid=0}, \"abcd\"}, %u, MSG_DONTWAIT, NULL, 0) = %u\n",
> +	printf("sendto(%d, {{{len=%u, type=NLMSG_NOOP, flags=NLM_F_REQUEST|0x%x"
> +	       ", seq=0, pid=0}, \"abcd\"}}, %u, MSG_DONTWAIT, NULL, 0) = %u\n",

Could you extend the test to check how a sequence is printed, please?


-- 
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/20160621/ff2cf61d/attachment.bin>