[PATCH] Add netlink domain sockets support to socketutils

Fabien Siron fabien.siron at epita.fr
Sun May 15 10:24:22 UTC 2016


Quoting Dmitry V. Levin (2016-05-14 23:27:36)
> On Fri, May 13, 2016 at 12:02:37PM +0000, Fabien Siron wrote:
> >  #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.

Ah yes, I didn't notice them.

> 
> >  #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.

Do you suggest to remove .ndiag_ino initialization?

> 
> > ...
> 
> 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]>?

I agree.

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

Cheers,
--
Fabien Siron




More information about the Strace-devel mailing list