Re: [PATCH nf-next] netfilter: nf_meta: support for nexthop and nexthop6

["Anders K. Pedersen | Cohaesio" <akp@cohaesio.com>]

Hi Pablo,

On tir, 2016-09-20 at 17:28 +0200, Pablo Neira Ayuso wrote:
> Hi Anders,
> 
> On Wed, Sep 14, 2016 at 05:47:08AM +0000, Anders K. Pedersen |
> Cohaesio wrote:
> > Add meta support for IPv4 nexthop and IPv6 nexthop6 (i.e. the
> > directly
> > connected IP address that an outgoing packet is sent to), which can
> > be used
> > either for matching or accounting, eg.

> Florian is working on explicitly fib lookup expression, 

I had noticed his RFC before doing this, but thought that it wouldn't
be ideal for my use case, since 1) it might not give the route that is
actually used in case of ECMP, and 2) it would be less efficient to
perform an additional route lookup in stead of just reusing the one
that has to be performed anyway for routing the packet - especially on
a router with a full BGP feed (currently close to 600.000 routes).

> for the
> existing route attached to the skbuff, I think we can add
> nft_rt_ipv4.c, nft_rt_ipv6.c and nft_rt_inet.c expressions instead
> for
> this? One per family, have a look at nft_meta_bridge.c for reference,
> it should look similar.
> 
> I think many other rt fields could be useful with a valid usecase.
> 
> BTW, proposed syntax is:
> 
> # nft add rule filter postrouting \
> 	flow table acct { rt ip nexthop timeout 600s counter }
> 
> # nft add rule ip6 filter postrouting \
> 	flow table acct { rt ip6 nexthop timeout 600s counter }
> 
> Then, for the inet family:
> 
> # nft add rule inet filter postrouting \
> 	ether type ip flow table acct { rt ip nexthop timeout 600s
> counter }
> 
> This one should bail out if:
> 
> # nft add rule inet filter postrouting \
> 	ether type ip flow table acct { rt ip6 nexthop timeout 600s
> counter }
>                    ~~                      ^^^
> 
> they don't match, this is just a bit of code at
> nftables/src/evaluate.c
> 
> Thus, we pass an explicit NFTA_RT_FAMILY attribute to explicitly
> indicate the family type so we can use this from the inet table too.
> 
> You need to add a expr/rt.c expression to libnftnl, it is boiler
> plate
> code you can use meta expression as reference.
> 
> From nft, you have to add a new EXPR_RT, there will be code missing
> in
> evaluate.c, netlink_linearize.c and netlink_delinearize.c

...

> With the approach above, we will not need to check for pkt->pf !=
> NFPROTO_IPV4, given this will be checked from the _init() path of the
> expression.
> 
> It will be a bit more code though.
> 
> Would you have a look at this? Let me know, thanks!

I had an initial look at this and found that it would involve
modifying/creating the following files:

nf-next:
include/net/netfilter/nft_rt.h - new file based on nft_meta.h
include/uapi/linux/netfilter/nf_tables.h
 - new NFT_RT_* and NFTA_RT_* - based on what exists for nft_meta
net/netfilter/{Kconfig,Makefile} - new config NFT_RT and NFT_RT_INET
net/netfilter/nft_rt.c - new file based on nft_meta.c
net/netfilter/nft_rt_inet.c - new file based on nft_meta_bridge.c
net/ipv4/netfilter/{Kconfig,Makefile} - new config NFT_RT_IPV4
net/ipv4/netfilter/nft_rt_ipv4.c - new file based on nft_meta_bridge.c
net/ipv6/netfilter/{Kconfig,Makefile} - new config NFT_RT_IPV6
net/ipv6/netfilter/nft_rt_ipv6.c - new file based on nft_meta_bridge.c

libnftnl:
include/linux/netfilter/nf_tables.h - same as kernel changes
include/libnftnl/expr.h - add new NFTNL_EXPR_RT_* and NFT_EXPR_RT_*
src/expr/rt.c - new file based on meta.c
src/expr_ops.c - add references to expr_ops_rt
src/Makefile.am - add expr/rt.c

nftables:
doc/nft.xml - document new rt expression
include/linux/netfilter/nf_tables.h - same as kernel changes
include/expression.h - add new EXPR_RT
include/rt.h - new file based on meta.h
include/statement.h - add new STMT_RT and rt_stmt
src/evaluate.c - add handling of EXPR_RT and STMT_RT
src/netlink_linearize.c - add handling of EXPR_RT and STMT_RT
src/netlink_delinearize.c - add handling of EXPR_RT and STMT_RT
src/rt.c - new file based on meta.c
src/Makefile.am - add rt.c to nft_SOURCES
src/parser_bison.y - define new keywords and syntax
src/scanner.l - define new keywords

Does this seem right, or have I missed something?

It looks like quite a bit more code than my first attempt, but I can
give it a try. I don't know how much time I'll have for this, so it
will probably take some weeks to do.

Regards,
Anders K. Pedersen