Hi,
On Mon, Jul 09, 2018 at 04:44:53PM +0200, Máté Eckl wrote:
[...]
> Example:
> nft> add table ip x
> nft> add chain ip x y { type filter hook prerouting priority raw; }
> nft> add chain ip x z { type filter hook prerouting priority mangle + 1; }
Nice stuff.
> nft> add chain ip x w { type filter hook prerouting priority dstnat - 5; }
I'd suggest prenat instead of dstnat, probably? I understand this is
leaking the definition we have in the kernel, but this is expected to
be used by non-programmers, so I wonder if we can offer a better tag
for this.
> nft> add chain ip x r { type filter hook prerouting priority filter + 10; }
> nft> add chain ip x t { type filter hook prerouting priority security; }
> nft> add chain ip x q { type filter hook prerouting priority srcnat + 11; }
I'd suggest postnat instead of srcnat.
BTW, srcnat only makes sense from postrouting, I think it would it be
possible to reject things that make no sense from there, like srcnat
with prerouting as in the example above.
More comments below.
> nft> add chain ip x h { type filter hook prerouting priority 15; }
> nft>
> nft> add flowtable ip x y { hook ingress priority filter + 5 ; devices =
> {enp0s31f6}; }
> nft>
> nft> add table arp x
> nft> add chain arp x y { type filter hook input priority filter + 5; }
> nft>
> nft> list ruleset
> table ip x {
> flowtable y {
> hook ingress priority filter + 5
> devices = { enp0s31f6 }
> }
>
> chain y {
> type filter hook prerouting priority raw; policy accept;
> }
>
> chain z {
> type filter hook prerouting priority mangle + 1; policy accept;
> }
>
> chain w {
> type filter hook prerouting priority dstnat - 5; policy accept;
> }
>
> chain r {
> type filter hook prerouting priority filter + 10; policy accept;
> }
>
> chain t {
> type filter hook prerouting priority security; policy accept;
> }
>
> chain q {
> type filter hook prerouting priority 111; policy accept;
> }
>
> chain h {
> type filter hook prerouting priority 15; policy accept;
> }
> }
> table arp x {
> chain y {
> type filter hook input priority filter + 5; policy accept;
> }
> }
> nft>
> nft> add chain ip x h { type filter hook prerouting priority first; }
> Error: 'first' is invalid for priority in this context.
> add chain ip x h { type filter hook prerouting priority first; }
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> nft> add chain arp x y { type filter hook input priority raw; }
> Error: 'raw' is invalid for priority in this context.
> add chain arp x y { type filter hook input priority raw; }
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> nft> add flowtable ip x y { hook ingress priority magle; devices =
> {enp0s31f6}; }
> Error: 'magle' is invalid for priority.
> add flowtable ip x y { hook ingress priority magle; devices = {enp0s31f6}; }
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Signed-off-by: Máté Eckl <eckl...@gmail.com>
> ---
> v4:
> - fix snat and dnat conflict with existing tokens
> - remove static char array from chain_prio2str
> - make numerical priority printing available via -nnn nft flag
> - add docs about priority names
> - check compatibility of standard prio names and table family
> - handle flowtables
>
> doc/nft.xml | 68 ++++++++++++++++++++++--
> include/rule.h | 5 ++
> src/evaluate.c | 56 ++++++++++++++++++++
> src/parser_bison.y | 36 ++++++++++---
> src/rule.c | 129 ++++++++++++++++++++++++++++++++++++++++++---
> src/scanner.l | 2 +
> 6 files changed, 281 insertions(+), 15 deletions(-)
>
> diff --git a/doc/nft.xml b/doc/nft.xml
> index dc93a8c..01cc1d1 100644
> --- a/doc/nft.xml
> +++ b/doc/nft.xml
> @@ -109,7 +109,7 @@ vi:ts=4 sw=4
> Show data numerically. When
> used once (the default behaviour), skip
> lookup of addresses to symbolic
> names. Use twice to also show Internet
> services (port numbers)
> numerically. Use three times to also show
> - protocols and UIDs/GIDs
> numerically.
> + protocols, UIDs/GIDs and
> priorities numerically.
> </para>
> </listitem>
> </varlistentry>
> @@ -856,10 +856,68 @@ add table inet mytable
> </itemizedlist>
> </para>
> <para>
> - The <literal>priority</literal> parameter accepts a
> signed integer value which specifies the order in which chains with same
> <literal>hook</literal> value are traversed. The ordering is ascending, i.e.
> lower priority values have precedence over higher ones.
> + The <literal>priority</literal> parameter accepts a
> signed integer value
> + or a standard priority name which specifies the order
> in which chains
> + with same <literal>hook</literal> value are traversed.
> The ordering is
> + ascending, i.e. lower priority values have precedence
> over higher ones.
> + <table frame="all">
> + <title>Standard priority names, values and
> families they are supported in</title>
> + <tgroup cols="3" align="left" colsep="1"
> rowsep="1">
> + <colspec colname="c1"/>
> + <colspec colname="c2" align="right"/>
> + <colspec colname="c3"/>
> + <thead>
> + <row>
> + <entry>Name</entry>
> + <entry>Value</entry>
> + <entry>Families</entry>
> + </row>
> + </thead>
> + <tbody>
> + <row>
> + <entry>raw</entry>
> + <entry>-300</entry>
> + <entry>ip, ip6,
> inet</entry>
> + </row>
> + <row>
> + <entry>mangle</entry>
> + <entry>-150</entry>
> + <entry>ip, ip6,
> inet</entry>
> + </row>
> + <row>
> + <entry>dstnat</entry>
> + <entry>-100</entry>
> + <entry>ip, ip6, inet,
> bridge</entry>
> + </row>
> + <row>
> + <entry>filter</entry>
> + <entry>0</entry>
> + <entry>ip, ip6, inet,
> arp, bridge, netdev</entry>
> + </row>
> + <row>
> + <entry>security</entry>
> + <entry>50</entry>
> + <entry>ip, ip6,
> inet</entry>
> + </row>
> + <row>
> + <entry>srcnat</entry>
> + <entry>100</entry>
> + <entry>ip, ip6, inet,
> bridge</entry>
> + </row>
> + </tbody>
> + </tgroup>
> + </table>
> + Basic arithmetic expressions (addition and
> substraction) can also
> + be achieved with these standard names to ease relative
> prioritizing,
> + eg. <literal>mangle - 5</literal> stands for
> <literal>-155</literal>.
> + Values will also be printid like this untill the value
> is not further
> + than 10 form the standard value.
> </para>
> <para>
> - Base chains also allow to set the chain's
> <literal>policy</literal>, i.e. what happens to packets not explicitly
> accepted or refused in contained rules. Supported policy values are
> <literal>accept</literal> (which is the default) or <literal>drop</literal>.
> + Base chains also allow to set the chain's
> <literal>policy</literal>, i.e.
> + what happens to packets not explicitly accepted or
> refused in contained
> + rules. Supported policy values are
> <literal>accept</literal> (which is
> + the default) or <literal>drop</literal>.
> </para>
> </refsect1>
>
> @@ -1379,6 +1437,10 @@ table inet filter {
> hybrid IPv4/IPv6 tables.
>
> When no address family is specified,
> <literal>ip</literal> is used by default.
> +
> + The <literal>priority</literal> can be a signed integer
> or <literal>filter</literal>
> + which stands for 0. Addition and substraction can be
> used to set relative priority
> + eg. filter + 5 equals to 5.
> </para>
>
> <variablelist>
> diff --git a/include/rule.h b/include/rule.h
> index 909ff36..cee9832 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -171,6 +171,7 @@ enum chain_flags {
> * @flags: chain flags
> * @hookstr: unified and human readable hook name (base chains)
> * @hooknum: hook number (base chains)
> + * @priostr: hook priority for standard priority name use (base chains)
> * @priority: hook priority (base chains)
> * @policy: default chain policy (base chains)
> * @type: chain type
> @@ -185,6 +186,7 @@ struct chain {
> uint32_t flags;
> const char *hookstr;
> unsigned int hooknum;
> + const char *priostr;
> int priority;
> int policy;
> const char *type;
> @@ -193,6 +195,8 @@ struct chain {
> struct list_head rules;
> };
>
> +#define STD_PRIO_BUFSIZE 100
> +extern int std_prio_lookup(const char *std_prio_name, int family);
> extern const char *chain_type_name_lookup(const char *name);
> extern const char *chain_hookname_lookup(const char *name);
> extern struct chain *chain_alloc(const char *name);
> @@ -357,6 +361,7 @@ struct flowtable {
> struct location location;
> const char * hookstr;
> unsigned int hooknum;
> + const char *priostr;
> int priority;
> const char **dev_array;
> struct expr *dev_expr;
> diff --git a/src/evaluate.c b/src/evaluate.c
> index c4ee3cc..c6de5e7 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -17,6 +17,7 @@
> #include <linux/netfilter.h>
> #include <linux/netfilter_arp.h>
> #include <linux/netfilter/nf_tables.h>
> +#include <linux/netfilter_ipv4.h>
> #include <netinet/ip_icmp.h>
> #include <netinet/icmp6.h>
> #include <net/ethernet.h>
> @@ -2868,6 +2869,46 @@ static int set_evaluate(struct eval_ctx *ctx, struct
> set *set)
> return 0;
> }
>
> +static bool parse_evaluate_priority(const char *priostr, int *prioptr,
> + int family) {
> + char *endptr = NULL, *numstart = NULL;
> + int priority;
> +
> + priority = (int)strtol(priostr, &endptr, 10);
> + /* prio_spec */
> + if (priostr != endptr) {
> + *prioptr = priority;
> + return true;
> + }
> +
> + /* STRING */
> + priority = std_prio_lookup(priostr, family);
> + if (priority != NF_IP_PRI_LAST) {
> + *prioptr = priority;
> + return true;
> + }
> +
> + /* STRUNG PLUS NUM | STRING DASH NUM */
> + numstart = strchr(priostr, '+');
> + if (!numstart)
> + numstart = strchr(priostr, '-');
> + if (numstart) {
> + char prioname[STD_PRIO_BUFSIZE];
> +
> + snprintf(prioname, numstart - priostr + 1, "%s", priostr);
> + priority = std_prio_lookup(prioname, family);
> + if (priority != NF_IP_PRI_LAST) {
> + priority += (int)strtol(numstart, &endptr, 10);
> + if (endptr == priostr + strlen(priostr)) {
> + *prioptr = priority;
> + return true;
> + }
> + }
> + }
> +
> + return false;
> +}
> +
> static uint32_t str2hooknum(uint32_t family, const char *hook);
>
> static int flowtable_evaluate(struct eval_ctx *ctx, struct flowtable *ft)
> @@ -2884,6 +2925,13 @@ static int flowtable_evaluate(struct eval_ctx *ctx,
> struct flowtable *ft)
> if (ft->hooknum == NF_INET_NUMHOOKS)
> return chain_error(ctx, ft, "invalid hook %s", ft->hookstr);
>
> + if (!ft->priostr)
> + return chain_error(ctx, ft,
> + "flowtable priority must be specified.");
> + if (!parse_evaluate_priority(ft->priostr, &ft->priority,
> NFPROTO_NETDEV))
> + return chain_error(ctx, ft, "'%s' is invalid priority.",
> + ft->priostr);
> +
> if (!ft->dev_expr)
> return chain_error(ctx, ft, "Unbound flowtable not allowed
> (must specify devices)");
>
> @@ -3038,6 +3086,14 @@ static int chain_evaluate(struct eval_ctx *ctx, struct
> chain *chain)
> chain->hookstr);
> }
>
> + if (!chain->priostr)
> + return chain_error(ctx, chain, "chain priority must be
> specified.");
> + if (!parse_evaluate_priority(chain->priostr, &chain->priority,
> + chain->handle.family))
> + return chain_error(ctx, chain,
> + "'%s' is invalid priority in this context.",
> + chain->priostr);
> +
> list_for_each_entry(rule, &chain->rules, list) {
> handle_merge(&rule->handle, &chain->handle);
> if (rule_evaluate(ctx, rule) < 0)
> diff --git a/src/parser_bison.y b/src/parser_bison.y
> index 98bfeba..2b7d7cc 100644
> --- a/src/parser_bison.y
> +++ b/src/parser_bison.y
> @@ -182,6 +182,8 @@ int nft_lex(void *, void *, void *);
> %token AT "@"
> %token VMAP "vmap"
>
> +%token PLUS "+"
> +
> %token INCLUDE "include"
> %token DEFINE "define"
> %token REDEFINE "redefine"
> @@ -522,6 +524,7 @@ int nft_lex(void *, void *, void *);
> %type <handle> set_spec setid_spec set_identifier
> flowtable_identifier obj_spec objid_spec obj_identifier
> %destructor { handle_free(&$$); } set_spec setid_spec set_identifier
> obj_spec objid_spec obj_identifier
> %type <val> family_spec family_spec_explicit chain_policy
> prio_spec
> +%type <string> str_prio_spec
>
> %type <string> dev_spec quota_unit
> %destructor { xfree($$); } dev_spec quota_unit
> @@ -1633,7 +1636,7 @@ flowtable_block_alloc : /* empty */
> flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
> | flowtable_block common_block
> | flowtable_block stmt_separator
> - | flowtable_block HOOK STRING
> PRIORITY prio_spec stmt_separator
> + | flowtable_block HOOK STRING
> PRIORITY str_prio_spec stmt_separator
> {
> $$->hookstr = chain_hookname_lookup($3);
> if ($$->hookstr == NULL) {
> @@ -1644,7 +1647,7 @@ flowtable_block : /* empty */ { $$ =
> $<flowtable>-1; }
> }
> xfree($3);
>
> - $$->priority = $5;
> + $$->priostr = $5;
> }
> | flowtable_block DEVICES '='
> flowtable_expr stmt_separator
> {
> @@ -1766,7 +1769,7 @@ type_identifier : STRING { $$ = $1; }
> | CLASSID { $$ = xstrdup("classid"); }
> ;
>
> -hook_spec : TYPE STRING HOOK
> STRING dev_spec PRIORITY prio_spec
> +hook_spec : TYPE STRING HOOK
> STRING dev_spec PRIORITY str_prio_spec
> {
> const char *chain_type =
> chain_type_name_lookup($2);
>
> @@ -1789,13 +1792,34 @@ hook_spec : TYPE STRING
> HOOK STRING dev_spec PRIORITY
> prio_spec
> xfree($4);
>
> $<chain>0->dev = $5;
> - $<chain>0->priority = $7;
> + $<chain>0->priostr = $7;
> $<chain>0->flags |= CHAIN_F_BASECHAIN;
> }
> ;
>
> -prio_spec : NUM { $$ = $1; }
> - | DASH NUM { $$ = -$2; }
> +str_prio_spec : prio_spec
> + {
> + char buf[STD_PRIO_BUFSIZE];
> + snprintf(buf, STD_PRIO_BUFSIZE, "%d", (int)$1);
> + $$ = xstrdup(buf);
> + }
> + | STRING { $$ = xstrdup($1); }
> + | STRING PLUS NUM
> + {
> + char buf[STD_PRIO_BUFSIZE];
> + snprintf(buf, STD_PRIO_BUFSIZE, "%s+%d",$1,
> (int)$3);
Could you store the string plus offset instead of building this
string that you need to parse again from the evaluation phase?
Probably you could reuse the existing priority integer field, then, if
the label is non-NULL, then it means the priority integer becomes an
offset.
Thanks for working on this !
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
