[PATCH nft] doc: extend description of fib expression

[PATCH nft] doc: extend description of fib expression

[Florian Westphal]

Describe the input keys and the result types.
Mention which input keys are mandatory and which keys are mutually
exclusive.

Describe which hooks can be used with the various lookup modifiers
and extend the examples with more information on fib expression
capabilities.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1663
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 doc/primary-expression.txt | 75 ++++++++++++++++++++++++++++++++------
 1 file changed, 63 insertions(+), 12 deletions(-)

diff --git a/doc/primary-expression.txt b/doc/primary-expression.txt
index 782494bda6f3..1cc06f3466b8 100644
--- a/doc/primary-expression.txt
+++ b/doc/primary-expression.txt
@@ -310,17 +310,48 @@ table inet x {
 FIB EXPRESSIONS
 ~~~~~~~~~~~~~~~
 [verse]
-*fib* {*saddr* | *daddr* | *mark* | *iif* | *oif*} [*.* ...] {*oif* | *oifname* | *type*}
+*fib* 'FIB_TUPLE' 'FIB_RESULT'
+'FIB_TUPLE' := { *saddr* | *daddr*} [ *.* { *iif* | *oif* } *.* *mark* ]
+'FIB_RESULT'  := { *oif* | *oifname* | *type* }
 
-A fib expression queries the fib (forwarding information base) to obtain
-information such as the output interface index a particular address would use.
-The input is a tuple of elements that is used as input to the fib lookup
-functions.
 
-.fib expression specific types
+A fib expression queries the fib (forwarding information base) to obtain information
+such as the output interface index.
+
+The first arguments to the *fib* expression are the input keys to be passed to the fib lookup function.
+One of *saddr* or *daddr* is mandatory, they are also mutually exclusive.
+
+*mark*, *iif* and *oif* keywords are optional modifiers to influence the search result, see
+the *FIB_TUPLE* keyword table below for a description.
+The *iif* and *oif* tuple keywords are also mutually exclusive.
+
+The last argument to the *fib* expression is the desired result type.
+
+*oif* asks to obtain the interface index that would be used to send packets to the packets source
+(*saddr* key) or destination (*daddr* key).  If no routing entry is found, the returned interface
+index is 0.
+
+*oifname* is like *oif*, but it fills the interface name instead.  This is useful to check dynamic
+interfaces such as ppp devices.  If no entry is found, an empty interface name is returned.
+
+*type* returns the address type such as unicast or multicast.
+
+.FIB_TUPLE keywords
 [options="header"]
 |==================
-|Keyword| Description| Type
+|flag| Description
+|daddr| Perform a normal route lookup: search fib for route to the *destination address* of the packet.
+|saddr| Perform a reverse route lookup: search the fib for route to the *source address* of the packet.
+|mark | consider the packet mark (nfmark) when querying the fib.
+|iif  | fail fib lookup unless route exists and its output interface is identical to the packets input interface
+|oif  | fail fib lookup unless route exists and its output interface is identical to the packets output interface.  This flag can only be used with the *type* result.
+
+|=======================
+
+.FIB_RESULT keywords
+[options="header"]
+|==================
+|Keyword| Description| Result Type
 |oif|
 Output interface index|
 integer (32 bit)
@@ -334,20 +365,40 @@ fib_addrtype
 
 Use *nft* *describe* *fib_addrtype* to get a list of all address types.
 
+The *oif* and *oifname* result is only valid in the *prerouting*, *input* and *forward* hooks.
+The *type* can be queried from any one of *prerouting*, *input*, *forward* *output* and *postrouting*.
+
+For *type*, the presence of the *iif* keyword in the 'FIB_TUPLE' modifiers restrict the available
+hooks to those where the packet is associated with an incoming interface, i.e. *prerouting*, *input* and *forward*.
+Likewise, the *oif* keyword in the 'FIB_TUPLE' modifier list will limit the available hooks to
+*forward*, *output* and *postrouting*.
+
 .Using fib expressions
 ----------------------
 # drop packets without a reverse path
 filter prerouting fib saddr . iif oif missing drop
 
-In this example, 'saddr . iif' looks up routing information based on the source address and the input interface.
-oif picks the output interface index from the routing information.
+In this example, 'saddr . iif' looks up a route to the *source address* of the packet and restricts matching
+results to the interface that the packet arrived on, then stores the output interface index from the obtained
+fib route result.
+
 If no route was found for the source address/input interface combination, the output interface index is zero.
-In case the input interface is specified as part of the input key, the output interface index is always the same as the input interface index or zero.
-If only 'saddr oif' is given, then oif can be any interface index or zero.
+Hence, this rule will drop all packets that do not have a strict reverse path (hypothetical reply packet
+would be sent via the interface the tested packet arrived on).
+
+If only 'saddr oif' is used as the input key, then this rule would only drop packets where the fib cannot
+find a route. In most setups this will never drop packets because the default route is returned.
 
-# drop packets to address not configured on incoming interface
+# drop packets if the destination ip address is not configured on the incoming interface
 filter prerouting fib daddr . iif type != { local, broadcast, multicast } drop
 
+This queries the fib based on the current packets' destination address and the incoming interface.
+
+If the packet is sent to a unicast address that is configured on a different interface, then the packet
+will be dropped as such an address would be classified as 'unicast' type.
+Without the 'iif' modifier, any address configured on the local machine is 'local', and unicast addresses
+not configured on any interface would return the type 'unicast'.
+
 # perform lookup in a specific 'blackhole' table (0xdead, needs ip appropriate ip rule)
 filter prerouting meta mark set 0xdead fib daddr . mark type vmap { blackhole : drop, prohibit : jump prohibited, unreachable : drop }
 ----------------------
-- 
2.45.2

Re: [PATCH nft] doc: extend description of fib expression

[Pablo Neira Ayuso]

Hi Florian,

Thanks, this is an improvement, a few comments.

On Thu, Oct 10, 2024 at 03:37:42PM +0200, Florian Westphal wrote:
> +The last argument to the *fib* expression is the desired result type.
> +
> +*oif* asks to obtain the interface index that would be used to send packets to the packets source
> +(*saddr* key) or destination (*daddr* key).  If no routing entry is found, the returned interface
> +index is 0.
> +
> +*oifname* is like *oif*, but it fills the interface name instead.  This is useful to check dynamic
> +interfaces such as ppp devices.  If no entry is found, an empty interface name is returned.
> +
> +*type* returns the address type such as unicast or multicast.
> +
> +.FIB_TUPLE keywords
>  [options="header"]
>  |==================
> -|Keyword| Description| Type
> +|flag| Description
> +|daddr| Perform a normal route lookup: search fib for route to the *destination address* of the packet.
> +|saddr| Perform a reverse route lookup: search the fib for route to the *source address* of the packet.
> +|mark | consider the packet mark (nfmark) when querying the fib.
> +|iif  | fail fib lookup unless route exists and its output interface is identical to the packets input interface

maybe easier to understand?

           if fib lookups provides a route then check its output interface is identical to the packets *input* interface.

> +|oif  | fail fib lookup unless route exists and its output interface is identical to the packets output interface.

           if fib lookups provides a route then check its output interface is identical to the packets *output* interface.

> This flag can only be used with the *type* result.

Are you sure 'oif' can only be used with type? I can see NFTA_FIB_F_OIF is available in nft_fib4_eval()

        if (priv->flags & NFTA_FIB_F_OIF)
                oif = nft_out(pkt);
        else if (priv->flags & NFTA_FIB_F_IIF)
                oif = nft_in(pkt);
        else
                oif = NULL;

One more comment below.

> +|=======================
> +
> +.FIB_RESULT keywords
> +[options="header"]
> +|==================
> +|Keyword| Description| Result Type
>  |oif|
>  Output interface index|
>  integer (32 bit)
> @@ -334,20 +365,40 @@ fib_addrtype
>  
>  Use *nft* *describe* *fib_addrtype* to get a list of all address types.
>  
> +The *oif* and *oifname* result is only valid in the *prerouting*, *input* and *forward* hooks.
> +The *type* can be queried from any one of *prerouting*, *input*, *forward* *output* and *postrouting*.
> +
> +For *type*, the presence of the *iif* keyword in the 'FIB_TUPLE' modifiers restrict the available
> +hooks to those where the packet is associated with an incoming interface, i.e. *prerouting*, *input* and *forward*.
> +Likewise, the *oif* keyword in the 'FIB_TUPLE' modifier list will limit the available hooks to
> +*forward*, *output* and *postrouting*.
> +
>  .Using fib expressions
>  ----------------------
>  # drop packets without a reverse path
>  filter prerouting fib saddr . iif oif missing drop
>  
> -In this example, 'saddr . iif' looks up routing information based on the source address and the input interface.
> -oif picks the output interface index from the routing information.
> +In this example, 'saddr . iif' looks up a route to the *source address* of the packet and restricts matching
> +results to the interface that the packet arrived on, then stores the output interface index from the obtained
> +fib route result.
>
>  If no route was found for the source address/input interface combination, the output interface index is zero.
> -In case the input interface is specified as part of the input key, the output interface index is always the same as the input interface index or zero.
> -If only 'saddr oif' is given, then oif can be any interface index or zero.
> +Hence, this rule will drop all packets that do not have a strict reverse path (hypothetical reply packet
> +would be sent via the interface the tested packet arrived on).
> +
> +If only 'saddr oif' is used as the input key, then this rule would only drop packets where the fib cannot
> +find a route. In most setups this will never drop packets because the default route is returned.
>  
> -# drop packets to address not configured on incoming interface
> +# drop packets if the destination ip address is not configured on the incoming interface
>  filter prerouting fib daddr . iif type != { local, broadcast, multicast } drop

I don't see a table in the manpage possible return values of fib type
lookups, I mean:

static const struct symbol_table addrtype_tbl = {
        .base           = BASE_DECIMAL,
        .symbols        = {
                SYMBOL("unspec",        RTN_UNSPEC),
                SYMBOL("unicast",       RTN_UNICAST),
                SYMBOL("local",         RTN_LOCAL),
                SYMBOL("broadcast",     RTN_BROADCAST),
                SYMBOL("anycast",       RTN_ANYCAST),
                SYMBOL("multicast",     RTN_MULTICAST),
                SYMBOL("blackhole",     RTN_BLACKHOLE),
                SYMBOL("unreachable",   RTN_UNREACHABLE),
                SYMBOL("prohibit",      RTN_PROHIBIT),
                SYMBOL_LIST_END
        }
};

Thanks.

Re: [PATCH nft] doc: extend description of fib expression

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > -|Keyword| Description| Type
> > +|flag| Description
> > +|daddr| Perform a normal route lookup: search fib for route to the *destination address* of the packet.
> > +|saddr| Perform a reverse route lookup: search the fib for route to the *source address* of the packet.
> > +|mark | consider the packet mark (nfmark) when querying the fib.
> > +|iif  | fail fib lookup unless route exists and its output interface is identical to the packets input interface
> 
> maybe easier to understand?
> 
>            if fib lookups provides a route then check its output interface is identical to the packets *input* interface.
> 
> > +|oif  | fail fib lookup unless route exists and its output interface is identical to the packets output interface.
> 
>            if fib lookups provides a route then check its output interface is identical to the packets *output* interface.

Its better, updated, thanks.

> > This flag can only be used with the *type* result.
> 
> Are you sure 'oif' can only be used with type? I can see NFTA_FIB_F_OIF is available in nft_fib4_eval()
> 
>         if (priv->flags & NFTA_FIB_F_OIF)
>                 oif = nft_out(pkt);
>         else if (priv->flags & NFTA_FIB_F_IIF)
>                 oif = nft_in(pkt);
>         else
>                 oif = NULL;

Seems to be dead code.  nft_fib_init() has:
        switch (priv->result) {
        case NFT_FIB_RESULT_OIF:
                if (priv->flags & NFTA_FIB_F_OIF)
                        return -EINVAL;
                len = sizeof(int);
                break;
        case NFT_FIB_RESULT_OIFNAME:
                if (priv->flags & NFTA_FIB_F_OIF)
                        return -EINVAL;
                len = IFNAMSIZ;


since _OIF and _OIFNAME was restricted to prerouting, nf hookfn has NULL
output interface, so there is nothing we could compare against.

Now its available in forward too so it could be selectively relaxed for
this, but, what is the use case?

Do a RPF in forward, then we need to compare vs. incoming interface.

But for outgoing interface, we'd do a normal route lookup, but the stack
already did that for us (as packet is already being forwarded).

So what would be the desired outcome for a 'fib daddr . oif' check?

Re: [PATCH nft] doc: extend description of fib expression

[Pablo Neira Ayuso]

On Fri, Oct 18, 2024 at 02:08:25PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > -|Keyword| Description| Type
> > > +|flag| Description
> > > +|daddr| Perform a normal route lookup: search fib for route to the *destination address* of the packet.
> > > +|saddr| Perform a reverse route lookup: search the fib for route to the *source address* of the packet.
> > > +|mark | consider the packet mark (nfmark) when querying the fib.
> > > +|iif  | fail fib lookup unless route exists and its output interface is identical to the packets input interface
> > 
> > maybe easier to understand?
> > 
> >            if fib lookups provides a route then check its output interface is identical to the packets *input* interface.
> > 
> > > +|oif  | fail fib lookup unless route exists and its output interface is identical to the packets output interface.
> > 
> >            if fib lookups provides a route then check its output interface is identical to the packets *output* interface.
> 
> Its better, updated, thanks.
> 
> > > This flag can only be used with the *type* result.
> > 
> > Are you sure 'oif' can only be used with type? I can see NFTA_FIB_F_OIF is available in nft_fib4_eval()
> > 
> >         if (priv->flags & NFTA_FIB_F_OIF)
> >                 oif = nft_out(pkt);
> >         else if (priv->flags & NFTA_FIB_F_IIF)
> >                 oif = nft_in(pkt);
> >         else
> >                 oif = NULL;
> 
> Seems to be dead code.  nft_fib_init() has:
>         switch (priv->result) {
>         case NFT_FIB_RESULT_OIF:
>                 if (priv->flags & NFTA_FIB_F_OIF)
>                         return -EINVAL;
>                 len = sizeof(int);
>                 break;
>         case NFT_FIB_RESULT_OIFNAME:
>                 if (priv->flags & NFTA_FIB_F_OIF)
>                         return -EINVAL;
>                 len = IFNAMSIZ;
> 
> 
> since _OIF and _OIFNAME was restricted to prerouting, nf hookfn has NULL
> output interface, so there is nothing we could compare against.
> 
> Now its available in forward too so it could be selectively relaxed for
> this, but, what is the use case?
> 
> Do a RPF in forward, then we need to compare vs. incoming interface.

This is for an esoteric scenario: Policy-based routing using input
interface as key. The fib rule for RPF does not work from prerouting
because iif cannot be inferred, there is no way to know if route in
the reverse direction exists until the route lookup for this direction
is done.

commit be8be04e5ddb9842d4ff2c1e4eaeec6ca801c573
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Thu Mar 31 17:14:47 2022 +0200

    netfilter: nft_fib: reverse path filter for policy-based routing on iif

    If policy-based routing using the iif selector is used, then the fib
    expression fails to look up for the reverse path from the prerouting
    hook because the input interface cannot be inferred. In order to support
    this scenario, extend the fib expression to allow to use after the route
    lookup, from the forward hook.

> But for outgoing interface, we'd do a normal route lookup, but the stack
> already did that for us (as packet is already being forwarded).
>
> So what would be the desired outcome for a 'fib daddr . oif' check?

Hm, this always evaluates true from forward and any later hook.

I missing now, what is the point of . oif in general?

Re: [PATCH nft] doc: extend description of fib expression

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote> > since _OIF and _OIFNAME was restricted to prerouting, nf hookfn has NULL
> > output interface, so there is nothing we could compare against.
> > 
> > Now its available in forward too so it could be selectively relaxed for
> > this, but, what is the use case?
> > 
> > Do a RPF in forward, then we need to compare vs. incoming interface.
> 
> This is for an esoteric scenario: Policy-based routing using input
> interface as key. The fib rule for RPF does not work from prerouting
> because iif cannot be inferred, there is no way to know if route in
> the reverse direction exists until the route lookup for this direction
> is done.

Yes, that internally sets fibs iif to the oif.

> > But for outgoing interface, we'd do a normal route lookup, but the stack
> > already did that for us (as packet is already being forwarded).
> >
> > So what would be the desired outcome for a 'fib daddr . oif' check?
> 
> Hm, this always evaluates true from forward and any later hook.
> 
> I missing now, what is the point of . oif in general?

Its for use with the 'type' output, i.e. consult fib to determine
the type of the daddr (multicast, broadcast etc).

I don't see an application for the fib case, with exception
of the 'rpf lookup in forward' case.

Re: [PATCH nft] doc: extend description of fib expression

[Pablo Neira Ayuso]

On Thu, Oct 24, 2024 at 12:41:03PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote> > since _OIF and _OIFNAME was restricted to prerouting, nf hookfn has NULL
> > > output interface, so there is nothing we could compare against.
> > > 
> > > Now its available in forward too so it could be selectively relaxed for
> > > this, but, what is the use case?
> > > 
> > > Do a RPF in forward, then we need to compare vs. incoming interface.
> > 
> > This is for an esoteric scenario: Policy-based routing using input
> > interface as key. The fib rule for RPF does not work from prerouting
> > because iif cannot be inferred, there is no way to know if route in
> > the reverse direction exists until the route lookup for this direction
> > is done.
> 
> Yes, that internally sets fibs iif to the oif.

Yes, oif is used for the reverse lookup as iif.

> > > But for outgoing interface, we'd do a normal route lookup, but the stack
> > > already did that for us (as packet is already being forwarded).
> > >
> > > So what would be the desired outcome for a 'fib daddr . oif' check?
> > 
> > Hm, this always evaluates true from forward and any later hook.
> > 
> > I missing now, what is the point of . oif in general?
> 
> Its for use with the 'type' output, i.e. consult fib to determine
> the type of the daddr (multicast, broadcast etc).

Is it possible to use skb_dst for this case to safe the fib lookup?
This is not possible because it depends on the fib tuple, correct?

> I don't see an application for the fib case, with exception
> of the 'rpf lookup in forward' case.

OK.

New patch round to address my silly nitpicks?

Thanks