Hi Máté,
On Tue, Jun 19, 2018 at 11:50:24AM +0200, Máté Eckl wrote:
[...]
> diff --git a/src/parser_bison.y b/src/parser_bison.y
> index 98bfeba..d753fd9 100644
> --- a/src/parser_bison.y
> +++ b/src/parser_bison.y
> @@ -21,6 +21,7 @@
> #include <linux/netfilter/nf_conntrack_tuple_common.h>
> #include <linux/netfilter/nf_nat.h>
> #include <linux/netfilter/nf_log.h>
> +#include <linux/netfilter_ipv4.h>
> #include <netinet/ip_icmp.h>
> #include <netinet/icmp6.h>
> #include <libnftnl/common.h>
> @@ -182,6 +183,8 @@ int nft_lex(void *, void *, void *);
> %token AT "@"
> %token VMAP "vmap"
>
> +%token PLUS "+"
> +
> %token INCLUDE "include"
> %token DEFINE "define"
> %token REDEFINE "redefine"
> @@ -521,7 +524,7 @@ int nft_lex(void *, void *, void *);
> %destructor { handle_free(&$$); } table_spec tableid_spec chain_spec
> chainid_spec flowtable_spec chain_identifier ruleid_spec handle_spec
> position_spec rule_position ruleset_spec index_spec
> %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 <val> family_spec family_spec_explicit chain_policy
> prio_spec standard_prio
>
> %type <string> dev_spec quota_unit
> %destructor { xfree($$); } dev_spec quota_unit
> @@ -1794,8 +1797,23 @@ hook_spec : TYPE STRING
> HOOK STRING dev_spec PRIORITY
> prio_spec
> }
> ;
>
> -prio_spec : NUM { $$ = $1; }
> - | DASH NUM { $$ = -$2; }
> +prio_spec : NUM { $$ = $1; }
> + | DASH NUM { $$ = -$2; }
> + | standard_prio
> + | standard_prio PLUS NUM { $$ = $1 + $3; }
> + | standard_prio DASH NUM { $$ = $1 - $3; }
> + ;
> +
> +standard_prio : STRING
> + {
> + int tmp = chain_std_prio_lookup($1);
> + if (tmp == NF_IP_PRI_LAST) {
> + erec_queue(error(&@$, "priority name
> '%s' is invalid", $1),
> + state->msgs);
> + YYERROR;
> + }
> + $$ = tmp;
> + }
> ;
I think you could simplify this by allowing for 'standard_prio' to
consist of an empty string which evaluates to the value 0. Then
'prio_spec' would be just:
| prio_spec : standard_prio
| | standard_prio PLUS NUM { $$ = $1 + $3; }
| | standard_prio DASH NUM { $$ = $1 - $3; }
[...]
> + for (i = 0; i < sizeof(chain_std_prios) / sizeof(struct
> chain_prio_tag); ++i) {
Pablo already pointed this out, but one additional remark: There's no
need to define ARRAY_SIZE macro, simply use 'array_size()' defined in
include/utils.h.
[...]
> +static const char *chain_prio2str(int prio)
> +{
> + static char ret[256];
> + char offstr[20];
> + const int reach = 10;
> + size_t i;
> +
> + ret[0] = 0;
> + offstr[0] = 0;
> + for (i = 0; i < sizeof(chain_std_prios) / sizeof(struct
> chain_prio_tag); ++i) {
for (i = 0; i < array_size(chain_std_prios); i++) {
> + const int sdt_prio = chain_std_prios[i].val;
> + const char *sdt_prio_str = chain_std_prios[i].str;
> + if (prio >= sdt_prio - reach && prio <= sdt_prio + reach) {
if (abs(prio - sdt_prio) <= reach) {
> + int offset = prio - sdt_prio;
> + if (offset != 0) {
> + snprintf(offstr, sizeof(offstr), " %c %d",
> + offset > 0 ? '+' : '-', abs(offset));
> + }
> + sprintf(ret, "%s%s", sdt_prio_str, offstr);
strcpy(ret, sdt_prio_str);
if (offset > 0)
snprintf(ret + strlen(ret), "+ %d", offset);
else if (offset < 0)
snprintf(ret + strlen(ret), "- %d", -offset);
> + return ret;
> + }
> + }
> + sprintf(ret, "%d", prio);
> + return ret;
> +}
> +
> static void chain_print_declaration(const struct chain *chain,
> struct output_ctx *octx)
> {
> @@ -781,8 +832,8 @@ static void chain_print_declaration(const struct chain
> *chain,
> hooknum2str(chain->handle.family, chain->hooknum));
> if (chain->dev != NULL)
> nft_print(octx, " device %s", chain->dev);
> - nft_print(octx, " priority %d; policy %s;\n",
> - chain->priority, chain_policy2str(chain->policy));
> + nft_print(octx, " priority %s; policy %s;\n",
> + chain_prio2str(chain->priority),
> chain_policy2str(chain->policy));
Doesn't this exceed 80 column boundary?
> }
> }
>
> @@ -806,9 +857,9 @@ void chain_print_plain(const struct chain *chain, struct
> output_ctx *octx)
> chain->handle.table.name, chain->handle.chain.name);
>
> if (chain->flags & CHAIN_F_BASECHAIN) {
> - nft_print(octx, " { type %s hook %s priority %d; policy %s; }",
> + nft_print(octx, " { type %s hook %s priority %s; policy %s; }",
> chain->type, chain->hookstr,
> - chain->priority, chain_policy2str(chain->policy));
> + chain_prio2str(chain->priority),
> chain_policy2str(chain->policy));
Column boundary again?
Apart from that, LGTM!
Thanks, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
