One comment see below.
On Wed, May 22, 2019 at 05:30:35PM +0200, Phil Sutter wrote:
> @@ -3237,10 +3222,62 @@ static int rule_evaluate(struct eval_ctx *ctx, struct
> rule *rule)
> return -1;
> }
>
> - if (rule->handle.index.id &&
> - rule_translate_index(ctx, rule))
> - return -1;
> + table = table_lookup(&rule->handle, &ctx->nft->cache);
> + if (!table)
> + return table_not_found(ctx);
> +
> + chain = chain_lookup(table, &rule->handle);
> + if (!chain)
> + return chain_not_found(ctx);
>
> + if (rule->handle.index.id) {
> + ref = rule_lookup_by_index(chain, rule->handle.index.id);
> + if (!ref)
> + return cmd_error(ctx, &rule->handle.index.location,
> + "Could not process rule: %s",
> + strerror(ENOENT));
> +
> + link_rules(rule, ref);
> + } else if (rule->handle.handle.id) {
> + ref = rule_lookup(chain, rule->handle.handle.id);
> + if (!ref)
> + return cmd_error(ctx, &rule->handle.handle.location,
> + "Could not process rule: %s",
> + strerror(ENOENT));
> + } else if (rule->handle.position.id) {
> + ref = rule_lookup(chain, rule->handle.position.id);
> + if (!ref)
> + return cmd_error(ctx, &rule->handle.position.location,
> + "Could not process rule: %s",
> + strerror(ENOENT));
> + }
> +
Nitpick: Probably move this code below into a function, something
like rule_cache_update().
> + switch (op) {
> + case CMD_INSERT:
> + rule_get(rule);
> + if (ref)
> + list_add_tail(&rule->list, &ref->list);
> + else
> + list_add(&rule->list, &chain->rules);
> + break;
> + case CMD_ADD:
> + rule_get(rule);
> + if (ref)
> + list_add(&rule->list, &ref->list);
> + else
> + list_add_tail(&rule->list, &chain->rules);
> + break;
> + case CMD_REPLACE:
> + rule_get(rule);
> + list_add(&rule->list, &ref->list);
> + /* fall through */
> + case CMD_DELETE:
> + list_del(&ref->list);
> + rule_free(ref);
> + break;
> + default:
> + break;
> + }
> return 0;
> }
Not related to this patch, but now that we have the cache completeness
logic, I think we need a flag to specify that cache has been modified.
If that is a problem, I can just apply this patch and we fix that
use-case I'm refering to later on.
