On 25.11, Florian Westphal wrote:
> Patrick McHardy <kaber@xxxxxxxxx> wrote:
> >
> > The ordering appears to be a bit random (or I'm missing something), I'd
> > expect basic information like TABLE/CHAIN/RULE_HANDLE to be at the beginning
> > (in that order), similar for TYPE, VERDICT etc.
>
> NFTA_TRACE_UNSPEC,
> NFTA_TRACE_TABLE,
> NFTA_TRACE_CHAIN,
> NFTA_TRACE_RULE_HANDLE,
> NFTA_TRACE_TYPE,
> NFTA_TRACE_VERDICT,
> NFTA_TRACE_DEV_TYPE,
> NFTA_TRACE_ID,
> NFTA_TRACE_LL_HEADER,
> NFTA_TRACE_NETWORK_HEADER,
> NFTA_TRACE_TRANSPORT_HEADER,
> NFTA_TRACE_TRANSPORT_PROTO,
> NFTA_TRACE_IIF,
> NFTA_TRACE_OIF,
> NFTA_TRACE_MARK,
> NFTA_TRACE_VLAN_TAG,
>
> better? If not, please just tell me what ordering you'd prefer and
> thats what I'll use.
Yep looks good.
> > > + if (nla_put_be32(skb, NFTA_TRACE_ID, htonl(hash32_ptr(pkt->skb))))
>
> > This could lead to duplicate IDs quite quickly. I can't think of any other
> > values where we know for sure they will stay constant while the skb is within
> > netfilter. How about combining this with a per CPU counter or something
> > similar?
>
> I would not mind, it would result in ID change though for nfqueue case.
> If thats ok I'll use a pcpu counter that gets incremented when nft_meta
> sets skb->nftrace=1 and use __this_cpu_read(trace_id) instead (if we
> have pcpu id counter, is there any point in also using skb address...?)
Not sure, I guess not.
> > > + switch (type) {
> > > + case NFT_TRACETYPE_POLICY:
> > > + case NFT_TRACETYPE_RETURN:
> > > + case NFT_TRACETYPE_RULE:
> > > + if (nla_put_be32(skb, NFTA_TRACE_VERDICT, htonl(verdict)))
> > > + goto nla_put_failure;
> >
> > It would be nice to have the full verdict available, IOW also the jump target.
> >
> > You could simply pass struct nft_verdict to this function and adapt
> > nft_verdict_dump() to take the attribute value as parameter.
>
> Will add NFTA_TRACE_JUMP/NFTA_TRACE_GOTO for that, ok?
Any reason not to use the standard verdict encoding? We even have an almost
ready to use function for that.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
