On Thu, Feb 07, 2019 at 09:00:50PM +0100, Phil Sutter wrote:
> On Thu, Feb 07, 2019 at 07:29:10PM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Feb 07, 2019 at 05:48:53PM +0100, Florian Westphal wrote:
> > > Phil Sutter <[email protected]> wrote:
> > > > > What it should show instead:
> > > > > Bridge chain: FOOBAR, entries: 1, policy: RETURN
> > > > > -j DROP
> > > > > -d 01:02:03:04:05:06 -j CONTINUE
> > > > >
> > > > > (because thats whats the actual state -- the last rule is
> > > > > unreachable).
> > > >
> > > > Hmm. Yes, that's ugly. Also, if you perform a change to the ruleset in
> > > > that state (no matter what, e.g. just create another chain or add a rule
> > > > somewhere else), the policy rule will be moved to the end. Not sure how
> > > > we could handle this.
> > >
> > > I think in that case ebtables-nft should make policy be 'RETURN', i.e.,
> > > not re-add a new policy chain.
> > >
> > > I think we could even avoid the 'user comment' and just examine the last
> > > rule in the chain -- check if its unconditional DROP/ACCEPT and then
> > > handle that as the 'policy'.
> >
> > We can probably add UDATA_TYPE_EBTABLES_POLICY, so we don't need to
> > guess if this is an autogenerated policy rule at the end of non-base
> > chain. Just search for the rule with this flag. For nft this
> > autogenerated rule will be transparent.
>
> I just tried. There is a minor issue with
> nft_rule_to_iptables_command_state(): It assumes that if
> NFTNL_RULE_USERDATA is present, it is a comment. If I either drop that
> code (it is there only for backwards compatibility) or make it aware
> that NFTNL_RULE_USERDATA not necessarily contains a comment, things work
> fine.
>
> What do you think? Should I send a v3?
You could rename get_comment() to get_userdata() which returns an
ipt_userdata structure that looks like:
struct ipt_userdata {
const char *comment;
bool is_ebtables_policy;
};
that you can pass to get_userdata() to fetch things. You can probably
store this userdata in the command_state structure, so we don't need
to parse userdata over and over again from different spots.
But this is just a suggestion.
