Hi Pablo,

El 25 de julio de 2019 12:44:25 CEST, Pablo Neira Ayuso <pa...@netfilter.org> 
escribió:
>On Mon, Jul 22, 2019 at 06:02:39PM +0200, Fernando Fernandez Mancera
>wrote:
>> This patch introduces the use of nft input files variables in chain
>policy.
>> e.g.
>> 
>> define default_policy = "accept"
>> 
>> add table ip foo
>> add chain ip foo bar {type filter hook input priority filter; policy
>$default_policy}
>> 
>> table ip foo {
>>      chain bar {
>>              type filter hook input priority filter; policy accept;
>>      }
>> }
>> 
>> Signed-off-by: Fernando Fernandez Mancera <ffmanc...@riseup.net>
>> ---
>>  include/rule.h                                |  2 +-
>>  src/evaluate.c                                | 51
>+++++++++++++++++++
>>  src/json.c                                    |  5 +-
>>  src/mnl.c                                     |  9 ++--
>>  src/netlink.c                                 |  8 ++-
>>  src/parser_bison.y                            | 19 +++++--
>>  src/parser_json.c                             | 17 +++++--
>>  src/rule.c                                    | 13 +++--
>>  .../testcases/nft-f/0025policy_variable_0     | 17 +++++++
>>  .../testcases/nft-f/0026policy_variable_0     | 17 +++++++
>>  .../testcases/nft-f/0027policy_variable_1     | 18 +++++++
>>  .../testcases/nft-f/0028policy_variable_1     | 18 +++++++
>>  .../nft-f/dumps/0025policy_variable_0.nft     |  5 ++
>>  .../nft-f/dumps/0026policy_variable_0.nft     |  5 ++
>>  14 files changed, 186 insertions(+), 18 deletions(-)
>>  create mode 100755 tests/shell/testcases/nft-f/0025policy_variable_0
>>  create mode 100755 tests/shell/testcases/nft-f/0026policy_variable_0
>>  create mode 100755 tests/shell/testcases/nft-f/0027policy_variable_1
>>  create mode 100755 tests/shell/testcases/nft-f/0028policy_variable_1
>>  create mode 100644
>tests/shell/testcases/nft-f/dumps/0025policy_variable_0.nft
>>  create mode 100644
>tests/shell/testcases/nft-f/dumps/0026policy_variable_0.nft
>> 
>> diff --git a/include/rule.h b/include/rule.h
>> index c6e8716..b12165a 100644
>> --- a/include/rule.h
>> +++ b/include/rule.h
>> @@ -209,7 +209,7 @@ struct chain {
>>      const char              *hookstr;
>>      unsigned int            hooknum;
>>      struct prio_spec        priority;
>> -    int                     policy;
>> +    struct expr             *policy;
>>      const char              *type;
>>      const char              *dev;
>>      struct scope            scope;
>> diff --git a/src/evaluate.c b/src/evaluate.c
>> index d2faee8..4d8bfbf 100755
>> --- a/src/evaluate.c
>> +++ b/src/evaluate.c
>> @@ -3415,6 +3415,52 @@ static uint32_t str2hooknum(uint32_t family,
>const char *hook)
>>      return NF_INET_NUMHOOKS;
>>  }
>>  
>> +static bool evaluate_policy(struct eval_ctx *ctx, struct expr
>**policy)
>
>better rename static bool ...(..., struct expr **exprp)
>
>> +{
>> +    char policy_str[NFT_NAME_MAXLEN];
>> +    struct location loc;
>> +    int policy_num;
>
>so you can use 'int policy;'.
>
>> +    ctx->ectx.len = NFT_NAME_MAXLEN * BITS_PER_BYTE;
>> +    if (expr_evaluate(ctx, policy) < 0)
>> +            return false;
>
>probably cache this here:
>
>        expr = *exprp;
>
>so you don't need:
>
>        (*expr)->...
>
>in all this code below.
>
>> +    if ((*policy)->etype != EXPR_VALUE) {
>> +            expr_error(ctx->msgs, *policy, "%s is not a valid "
>> +                       "policy expression", expr_name(*policy));
>> +            return false;
>> +    }
>> +
>> +    if ((*policy)->dtype->type == TYPE_STRING) {
>> +            mpz_export_data(policy_str, (*policy)->value,
>> +                            BYTEORDER_HOST_ENDIAN,
>> +                            NFT_NAME_MAXLEN);
>> +            loc = (*policy)->location;
>> +            if (!strcmp(policy_str, "accept")) {
>> +                    expr_free(*policy);
>> +                    policy_num = NF_ACCEPT;
>> +                    *policy = constant_expr_alloc(&loc,
>> +                                                  &integer_type,
>> +                                                  BYTEORDER_HOST_ENDIAN,
>> +                                                  sizeof(int) * 
>> BITS_PER_BYTE,
>> +                                                  &policy_num);
>> +            } else if (!strcmp(policy_str, "drop")) {
>> +                    expr_free(*policy);
>> +                    policy_num = NF_DROP;
>> +                    *policy = constant_expr_alloc(&(*policy)->location,
>> +                                                  &integer_type,
>> +                                                  BYTEORDER_HOST_ENDIAN,
>> +                                                  sizeof(int) * 
>> BITS_PER_BYTE,
>> +                                                  &policy_num);
>
>Probably:
>
>               if (!strcmp(policy_str, "accept")) {
>                        policy = NF_ACCEPT;
>                else (!strmp(policy_str, "drop")) {
>                        policy = NF_DROP;
>                } else {
>                        ...
>                }
>
>                expr = constant_expr_alloc(...);
>
>so this code becomes shorter and easier to read.
>
>And I think you should do all this from the new policy_datype, not
>from the evaluation phase itself.
>
>Thanks.

I agree with everything except this part. There is no policy datatype. I could 
create it if you prefer it. Thanks! :-)

Reply via email to