Hi Pablo,
El 25 de julio de 2019 12:44:25 CEST, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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 <ffmancera@xxxxxxxxxx>
>> ---
>> 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! :-)
