Re: [PATCH 2/7] doc: fix/improve documentation of verdicts

[Florian Westphal <fw@strlen.de>]

Christoph Anton Mitterer <mail@christoph.anton.mitterer.name> wrote:
> What would you think about changing it roughly as follows (at this
> place, as well as perhaps in my new summary chapter about how
> evaluation works):
> - remove the rejects from next to the drops
> - add a paragraph which describes that there are verdict-like
>   statements, which a) do some extra stuff and b) cause one of the true
>   verdicts (drop/accept)
> - along with that add a list which explains which such statement causes
>   which verdict
> 
> What's IMO important from the user PoV is that for example at a
> sentence like:
> >*accept* and *drop*/*reject* are absolute verdicts, which immediately
> > terminate the evaluation of the current rule, i.e. even any later
> > statements of the current rule won’t get executed

Why not shorten it to "accept/drop/reject immediately terminate ..."

> The user doesn't want to have to read through the whole manpage to
> eventually realise that reject behaves effectively like drop here.
> And even if you refer to the REJECT section and mention it there, it
> makes things IMO more difficult to understand.

Ok.

> But back to verdict/verdict-like statements:
> Are jump/goto/return/etc. then true verdicts? I wouldn't guess so, yet
> they already are explained in the verdict section.

Yes, this is because of netfilter verdicts vs. nftables verdicts.

accept/drop are both, continue, goto, return the latter.

This is because netfilter core has no idea what a ruleset is;
nftables "plugs" into the netfilter core.

The nf_tables base hooks interpret the user policy and then
issue either NF_ACCEPT, NF_DROP, NF_QUEUE or NF_STOLEN.

(the "core" verdicts).  But I agree that this is an implementation
detail that is irrelevant to the man page and that continue, goto, etc.
should be called verdicts too.

> I'm not sure how much we win, by differentiating between these two, and
> even if we do so, how shall we call things like reject? "verdict like
> statements"? "statements that imply a verdict"?

What about "terminal statements"?
This is already used in the man page in several places.

> > 'reject' is also not the only statement that ends rule/basechain
> > evaluation,
> > other examples are redirect/dnat/snat/masquerade which will
> > internally
> > issue an accept verdict.  Or synproxy, which will drop internally to
> > consume the incoming packet.
> 
> Is there a complete list of all these which are verdict-like and what
> verdict they actually imply?

No.

net/bridge/netfilter/nft_reject_bridge.c:       regs->verdict.code = NF_DROP;
net/ipv4/netfilter/nft_reject_ipv4.c:   regs->verdict.code = NF_DROP;
net/ipv6/netfilter/nft_reject_ipv6.c:   regs->verdict.code = NF_DROP;
net/netfilter/nft_reject_inet.c:        regs->verdict.code = NF_DROP;
net/netfilter/nft_reject_netdev.c:      regs->verdict.code = NF_DROP;

These are "obvious", reject is a fancier drop.

net/netfilter/nft_compat.c:             regs->verdict.code = NF_ACCEPT;
net/netfilter/nft_compat.c:             regs->verdict.code = NF_DROP;

irrelevant for nftables

net/netfilter/nft_connlimit.c:          regs->verdict.code = NF_DROP;

Error handling only

net/netfilter/nft_ct.c:                 regs->verdict.code = NF_DROP;
net/netfilter/nft_ct.c:         regs->verdict.code = NF_DROP;
net/netfilter/nft_exthdr.c:     regs->verdict.code = NF_DROP;
net/netfilter/nft_fib_inet.c:   regs->verdict.code = NF_DROP;

Same, only errors

net/netfilter/nft_fwd_netdev.c: regs->verdict.code = NF_STOLEN;

Terminal (packet is redirected)

net/netfilter/nft_synproxy.c:                   regs->verdict.code = NF_DROP;
net/netfilter/nft_synproxy.c:                   regs->verdict.code = NF_STOLEN;
net/netfilter/nft_synproxy.c:           regs->verdict.code = NF_DROP;
net/netfilter/nft_synproxy.c:           regs->verdict.code = NF_STOLEN;

Also terminal, 3whs packets are dropped resp. stolen
for further processing.

> > Maybe the 'REJECT STATEMENT' section can be extended a little, but I
> > think its ok as-is.
> 
> Same solution as I'd propose above?!

Ok.

> > Maybe we should mention that 'return' is the implicit thing at the
> > end
> > of a user-created non-base chain?
> > 
> > Or do you think thats self-evident?
> 
> I do mention it in my summary chapter.
> 
> In the section on the verdicts you're referring to it follows
> implicitly from the description of the *jump* statement, but at least
> there I'd also think it's perhaps good to explicitly mention it in
> parentheses.

That seems fine.

> Whether we also mentioned it in the return description, I don't mind.
> We can, so to say as extra information, but it's IMO not strictly
> necessary. Because if someone looks up the book for the return
> statement he most likely wants to know what happens we issuing it - not
> which other things (he wasn't looking up) also behave in some cases
> like the statement he was looking up.

Ok.

> > > + In a regular chain that was called via *goto* or in a base chain,
> > > the *return*
> > > + verdict is equivalent to the base chain’s policy.
> > 
> > No, its not.
> > I think this warrants an example.
> > 
> > chain two { ... }
> > chain one {
> >  ...
> >  goto two
> >  ip saddr ..   # never matched
> > }
> > 
> > chain in {
> >  hook input type filter ...
> >  jump one
> >  ip saddr .. # evaluated for all packets not dropped/accepted yet
> > }
> > 
> > -> base chain calls 'one' and remembers this location
> > -> 'one' calls 'two', but doesn't place it on chain stack.
> > -> at the end of 'two' / on 'return', we resume after 'jump one', not
> >    after 'goto'.
> > 
> > The sentence wrt. base chain policy is valid in case 'chain in' would
> > contain 'goto one', as it doesn't remember the origin location,
> > end-of-one / return is equal to explicit 'return' from the base
> > chain.
> 
> Uff... okay... that makes things quite a bit harder to describe.

Yes, I think an example would be prudent, it is probably simpler
to unstand rather than describing the mechanism.

> So effectively that means, return (explicit or implicit) it not really
> equivalent to the policy, as claimed for at least base-chains in the
> current manpage:
> > return
> > Return from the current chain and continue evaluation at the next
> > rule in the last chain. If issued in a base chain, it isequivalent to
> > the base chain policy.
> But rather *if* there's nowhere to return to (like when it's called
> from a base-chain) *then* the policy is applied, right!?

Yes.

> > Maybe it should say that it will instead resume after the last jump
> > (if
> > there was any?, or not at all (base chain policy executes?)

Yes, you can also look into man iptables, --jump and --goto work the
same way in nftables.

> As far as I understand it now:
>   base --jump--> regularA --goto--> regularB
> then at the end of regularB or if return is called in it, while I don't
> return to regularA, I actually will return to the jump position in
> base, right?

Yes, rules in "regularB" are evaluated as if they would reside in
regularA, it resumes in the base chain.

> Similar in:
>   base --jump--> regularA --jump--> regularB --goto--> regularC
> I will not return to regularB, but will return to the jump position of
> regularA and then to that of base, right?

Yes.

> Very open for opinions, but I do think with that semantics it actually
> might be best to describe things with a call stack an give some
> examples.

Yes, I think examples are best here.