GSoC 2017 Introduction: Netlink socket parsers

[Dmitry V. Levin]

On Mon, Mar 27, 2017 at 10:40:05PM +0800, JingPiao Chen wrote:
> 2017-03-27 6:16 GMT+08:00 Dmitry V. Levin <ldv at altlinux.org> wrote:
> >On Sun, Mar 19, 2017 at 12:25:11PM +0800, JingPiao Chen wrote:
> >> Hello strace-developers:
> >> My name is JingPiao Chen. I'am a Computer Science sophomore from
> >> Guangdong Pharmaceutical University. I am interested in Netlink socket
> >> parsers suggested by Gabriel Laskar and Dmitry V. Levin. I am familiar
> >> with C, git, shell script, and gdb.
> >>
> >> My current work base on the code by Fabien Siron
> >> (https://github.com/Saruta/strace/tree/netlink)
> >
> >btw, there was a short conversation on #strace a month ago about this code:
> >https://strace.io/logs/strace/2017-02-28
> >
> >> The work I do:
> >> (master:https://github.com/ppiao/strace/tree/master)
> >> 1.Merge it into latest main tree.
> >> 2.Fix coding style.
> >> 3.Rewrite decode_netlink_flags() and decode_netlink_type().
> >> 4.Rewrite and rename decode_inet_diag_sockid().
> >> 5.Rewrite decode_nlattr_data().
> >> 6.Decode some nested attributes.
> >
> >Why have you squashed all commits made by Fabien into a single commit?
> >Do you think it's easier to review a big commit rather than a series of
> >small commits?
> 
> No, I don't know a big commit not easy to review.

Sorry, I don't understand.  Do you mean that you haven't seen a commit big
enough to make review a herculean job?

> I think all this commits
> implement enhance netlink socket decode.
> Do you think separate three commits, or can you give me some advice?

The general idea is that every logically separate change goes
to an individual commit.
The netlink branch made by Fabien follows this approach.
Can you follow it, too?

> 1.Add support for NLMSG_ERROR and NLMSG_DONE.
> 2.Introduce getfdnlproto and enhanced decoding of struct nlmsghdr
> nlmsg_type and nlmsg_flags.
> 3.Introduce a dummy netlink attributes parser and decoding of
> NETLINK_SOCK_DIAG protocol.

Sorry, I don't understand, is it something you've done already,
or are you going to implement this?

> >> (netlink branch:https://github.com/ppiao/strace/tree/netlink)
> >> 7.Rewrite getfdnlproto().
> >> 8.Do not use optional_data in struct print_iovec_config,
> >> when call print_iovec, pass tcp->u_arg[0] as fd.
> >> Is this right? Like this:
> >>
> >> diff --git a/io.c b/io.c
> >> index a4f6b8e..6b43d7d 100644
> >> --- a/io.c
> >> +++ b/io.c
> >> @@ -94,7 +94,7 @@ print_iovec(struct tcb *tcp, void *elem_buf, size_t
> >> elem_size, void *data)
> >>   len = c->data_size;
> >>   if (c->data_size != (kernel_ulong_t) -1)
> >>   c->data_size -= len;
> >> - decode_netlink(tcp, iov[0], iov[1]);
> >> + decode_netlink(tcp, tcp->u_arg[0], iov[0], iov[1]);
> >
> >Why tcp->u_arg[0]?  It doesn't look correct.
> 
> Because send, sendto, recv, recvfrom, sendmsg, sendmmsg, recvmsg and
> recvmmsg
> the first argument are fd.

Are you absolutely sure that in *all* cases the descriptor is passed
via the 1st syscall argument?

The common rule is not to make such assumptions unless you are 100% sure.

> >> I want to know the goal of Netlink socket parsers project.
> >
> >Obviously, the goal is to implement parsers for assigned netlink families.
> I upload a draft of my proposal[1]. Please provide some feedback.
> Thank you.
> 
> [1]
> https://docs.google.com/document/d/15cVdpdBP6jzEGP74tSG18ymi278_P2uv8MhRus-CuPI/edit?usp=sharing

Why do you want to rewrite getfdnlproto?
What's wrong with optional_data?


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