Re: [PATCH nftables 6/6] src: add trace support to nft monitor mode

[Florian Westphal <fw@strlen.de>]

Patrick McHardy <kaber@trash.net> wrote:
> > +static void trace_print_if(const struct nftnl_trace *nlt, uint16_t attr, const char *str)
> > +{
> > +	char __name[IFNAMSIZ];
> > +	const char *ifname;
> > +
> > +        if (!nftnl_trace_is_set(nlt, attr))
> > +		return;
> > +
> > +	ifname = nft_if_indextoname(nftnl_trace_get_u32(nlt, attr), __name);
> > +	if (ifname)
> > +		printf(" %s %s", str, ifname);
> > +	else
> > +		printf(" %s %d", str, nftnl_trace_get_u32(nlt, attr));
> > +}
> 
> A lot of the other trace attributes are not used so far. I'm wondering if
> you intend to add special print functions for them as well.

All of the trace attributes are used, most of the use is limited to
libnftnl though.

Wrt. to iif/oif, I do the printing for it in nftables to take advantage
of the iif/oif idx<->name cache.

I would have preferred to just stick it into libnftnl like most of the
other rest BUT then you either have the additional name translation overhead
or just the index number...

> An alternative would be to use our internal datatypes, IOW parse the
> attributes, associate the values with an internal type and use the regular
> printing functions. The benefit would be fully consistent output, also
> with respect to output options like numerical output.

Yes, right now virtually all of the printing is in libnftnl
(called from nftables via nftnl_trace_fprintf() ).

> > +static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
> > +{
> > +	struct table *t;
> > +	struct set *s;
> > +	struct netlink_mon_handler monhandler;
> > +
> > +	monhandler.cache_needed = need_cache(cmd);
> >  	if (monhandler.cache_needed) {
> > +		struct rule *rule, *nrule;
> > +		struct chain *chain;
> > +		int ret;
> > +
> >  		list_for_each_entry(t, &table_list, list) {
> >  			list_for_each_entry(s, &t->sets, list)
> >  				s->init = set_expr_alloc(&cmd->location);
> > +
> > +			if (!(cmd->monitor->flags & (1 << NFT_MSG_TRACE)))
> > +				continue;
> > +
> > +			/* When tracing we'd like to translate the rule handle
> > +			 * we receive in the trace messages to the actual rule
> > +			 * struct to print that out.  Populate rule cache now.
> > +			 */
> 
> Tracing might be a long running operation. The cache can go out of sync, might
> be better to do a lookup on demand.

Hmm, okay.