On Thu, Feb 07, 2019 at 04:19:51PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Feb 05, 2019 at 05:01:43PM +0100, Phil Sutter wrote:
> > Legacy ebtables supports policies for user-defined chains - and what's
> > worse, they default to ACCEPT unlike anywhere else. So lack of support
> > for this braindead feature in ebtables-nft is actually a change of
> > behaviour which very likely affects all ebtables users out there.
> >
> > The solution implemented here uses an implicit (and transparent) last
> > rule in all user-defined ebtables-nft chains with policy other than
> > RETURN. This rule is identified by an nft comment
> > "XTABLES_EB_INTERNAL_POLICY_RULE" (since commit ccf154d7420c0 ("xtables:
> > Don't use native nftables comments") nft comments are not used
> > otherwise).
> >
> > To minimize interference with existing code, this policy rule is removed
> > from chains during cache population and the policy is saved in
> > NFTNL_CHAIN_POLICY attribute. When committing changes to the kernel,
> > nft_commit() traverses through the list of chains and (re-)creates
> > policy rules if required.
> >
> > In ebtables-nft-restore, table flushes are problematic. To avoid weird
> > kernel error responses, introduce a custom 'table_flush' callback which
> > removes any pending policy rule add/remove jobs prior to creating the
> > NFT_COMPAT_TABLE_FLUSH one.
> >
> > I've hidden all this mess behind checks for h->family, so hopefully
> > impact on {ip,ip6,arp}tables-nft should be negligible.
>
> LGTM.
>
> One nitpick, nft list ruleset shows:
>
> ether type 0x0000 counter packets 0 bytes 0 accept comment "XTABLES_EB_INTERNAL_POLICY_RULE"
> ^^^^^^^^^^^^^^^^^
Another thing to consider is if we should add a new specific type for
this. I mean, a new TLV, on top (or instead) of the existing visible
rule comment.
We should probably place UDATA_TYPE_* in libnftnl, so they are public
and both nft and iptables can reuse those definitions.
Thanks!
