On Sun, Aug 20, 2017 at 04:06:25PM +0800, JingPiao Chen wrote:
> On Sun, Aug 20, 2017 at 01:33:53AM +0300, Dmitry V. Levin wrote:
> > On Sat, Aug 19, 2017 at 09:50:10AM +0800, JingPiao Chen wrote:
> > > * configure.ac (AC_CHECK_HEADERS): Add linux/if_link.h.
> > > (AC_CHECK_TYPES): Check for struct rtnl_link_stats64 in linux/if_link.h.
> > > (AC_CHECK_MEMBERS): Check for rx_nohandler field
> > > in struct rtnl_link_stats and struct rtnl_link_stats64.
> > > * rtnl_link.c: Include <arpa/inet.h>, <linux/if_arp.h>,
> > > <linux/if_link.h> and <linux/netdevice.h>.
> > > (min_ifla_address_len, ifla_address_default_decoder,
> > > ifla_address_type_specific_decoder,
> > > decode_ifla_address, decode_rtnl_link_stats,
> > > decode_rtnl_link_ifmap, decode_rtnl_link_stats64,
> > > print_item_id, decode_ifla_phys_item_id): New functions.
> > > (decode_ifla_phys_item_id): New array.
> >
> > How could decode_ifla_phys_item_id be both a function and an array?
> >
> > > (decode_ifinfomsg): Use it.
> > > ---
> [...]
> > > +static bool
> > > +decode_rtnl_link_stats(struct tcb *const tcp,
> > > + const kernel_ulong_t addr,
> > > + const unsigned int len,
> > > + const void *const opaque_data)
> > > +{
> > > + struct rtnl_link_stats st;
> > > +
> > > + if (len < sizeof(st))
> > > + return false;
> >
> > The kernel may not transfer struct rtnl_link_stats.rx_nohandler despite
> > the latter being defined by linux/if_link.h, e.g. if the kernel uses
> > an older version of struct rtnl_link_stats.
> > The minimal size is therefore not sizeof(struct rtnl_link_stats)
> > but offsetofend(struct rtnl_link_stats, tx_compressed).
> >
> > Likewise, with struct rtnl_link_stats64.rx_nohandler.
>
> You means:
>
> struct rtnl_link_stats st;
> const unsigned int sizeof_stat =
> offsetofend(struct rtnl_link_stats, tx_compressed);
>
> if (len < sizeof_stat)
> return false;
> else if (!umoven_or_printaddr(tcp, addr, sizeof_stat, &st)) {
> ...
>
> I can not understand where I can know kernel not transfer
> struct rtnl_link_stats.rx_nohandler.
This can happen, for example, when kernel headers used to build strace
are newer when the kernel itself.
> > > +static bool
> > > +print_item_id(struct tcb *const tcp, void *const elem_buf,
> > > + const size_t elem_size, void *const opaque_data)
> > > +{
> > > + unsigned int *const count = opaque_data;
> > > +
> > > + /* MAX_PHYS_ITEM_ID_LEN = 32 */
> > > + if ((*count)++ >= 32) {
> > > + tprints("...");
> > > + return false;
> > > + }
> > > +
> > > + tprintf("%" PRIu8, *(uint8_t *) elem_buf);
>
> I changed:
> tprintf("%02x", *(uint8_t *) elem_buf);
>
> > > +
> > > + return true;
> > > +}
> > > +
> > > +static bool
> > > +decode_ifla_phys_item_id(struct tcb *const tcp,
> > > + const kernel_ulong_t addr,
> > > + const unsigned int len,
> > > + const void *const opaque_data)
> > > +{
> > > + uint8_t id;
> > > + unsigned int count = 0;
> > > +
> > > + print_array(tcp, addr, len, &id, sizeof(id),
> > > + umoven_or_printaddr, print_item_id, &count);
> >
> > I'm not sure it's the best way to decode struct netdev_phys_item_id.id.
>
> Or I can reference iproute2/lib/utils.c: hexstring_n2a
> Now: [xx, xx, xx, xx, xx]
> hexstring_n2a: xx xx xx xx xx xx
AFAICT, hexstring_n2a outputs xxxxxxxxxxxx.
> strace print xx xx xx xx xx xx seems strange, here space as delimiter,
> easy to misunderstand it is in memory.
> what do think?
I wonder wouldn't it be better just to use
printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX)?
> > > +static const nla_decoder_t ifinfomsg_nla_decoders[] = {
> > > + [IFLA_ADDRESS] = decode_ifla_address,
> > > + [IFLA_BROADCAST] = decode_ifla_address,
> > > + [IFLA_IFNAME] = decode_nla_str,
> > > + [IFLA_MTU] = decode_nla_u32,
> > > + [IFLA_LINK] = decode_nla_u32,
> > > + [IFLA_QDISC] = decode_nla_str,
> > > + [IFLA_STATS] = decode_rtnl_link_stats,
> > > + [IFLA_COST] = NULL,
> > > + [IFLA_PRIORITY] = NULL,
> > > + [IFLA_MASTER] = decode_nla_u32,
> > > + [IFLA_WIRELESS] = NULL,
> >
> > Here a parser of struct iw_event is expected.
> >
> > > + [IFLA_PROTINFO] = NULL,
> >
> > This one seems to be used in the kernel.
> >
> > > + [IFLA_TXQLEN] = decode_nla_u32,
> > > + [IFLA_MAP] = decode_rtnl_link_ifmap,
> > > + [IFLA_WEIGHT] = decode_nla_u32,
> > > + [IFLA_OPERSTATE] = decode_nla_u8,
> > > + [IFLA_LINKMODE] = decode_nla_u8,
> > > + [IFLA_LINKINFO] = NULL,
> >
> > This one also seems to be used in the kernel.
> >
> > > + [IFLA_NET_NS_PID] = decode_nla_u32,
> > > + [IFLA_IFALIAS] = decode_nla_str,
> > > + [IFLA_NUM_VF] = decode_nla_u32,
> > > + [IFLA_VFINFO_LIST] = NULL,
> >
> > Likewise.
> >
> > > + [IFLA_STATS64] = decode_rtnl_link_stats64,
> > > + [IFLA_VF_PORTS] = NULL,
> >
> > Likewise.
> >
> > > + [IFLA_PORT_SELF] = NULL,
> >
> > Likewise.
> >
> > > + [IFLA_AF_SPEC] = NULL,
> >
> > Likewise.
> >
> > > + [IFLA_GROUP] = decode_nla_u32,
> > > + [IFLA_NET_NS_FD] = decode_nla_u32,
> > > + [IFLA_EXT_MASK] = decode_nla_u32,
> > > + [IFLA_PROMISCUITY] = decode_nla_u32,
> > > + [IFLA_NUM_TX_QUEUES] = decode_nla_u32,
> > > + [IFLA_NUM_RX_QUEUES] = decode_nla_u32,
> > > + [IFLA_CARRIER] = decode_nla_u8,
> > > + [IFLA_PHYS_PORT_ID] = decode_ifla_phys_item_id,
> > > + [IFLA_CARRIER_CHANGES] = decode_nla_u32,
> > > + [IFLA_PHYS_SWITCH_ID] = decode_ifla_phys_item_id,
> > > + [IFLA_LINK_NETNSID] = decode_nla_s32,
> > > + [IFLA_PHYS_PORT_NAME] = decode_nla_str,
> > > + [IFLA_PROTO_DOWN] = decode_nla_u8,
> > > + [IFLA_GSO_MAX_SEGS] = decode_nla_u32,
> > > + [IFLA_GSO_MAX_SIZE] = decode_nla_u32,
> > > + [IFLA_PAD] = NULL,
> > > + [IFLA_XDP] = NULL,
> >
> > Likewise.
>
> You means that I should decode all the attributes?
There are few attributes that are not used in the kernel,
for those NULL is fine but a comment, e.g. /* unused */,
would be nice to have added, too.
> Decode all the attributes in one commit the patch will very big,
There is definitely no need to do it all in a single commit.
> and I'm not familiar with some attribute data, e.g. struct iw_event.
> So need a lot of time.
How much is a lot? ;)
--
ldv
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
