* [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command @ 2019-06-14 12:36 Pablo Neira Ayuso 2019-06-14 12:54 ` Phil Sutter 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2019-06-14 12:36 UTC (permalink / raw) To: netfilter-devel; +Cc: fw, phil __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink dump to populate the cache. This patch is a workaround until we have a better infrastructure to track the state of the cache objects. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- v2: check for __CMD_FLUSH_RULESET before dumping tables via netlink. include/rule.h | 1 + src/cache.c | 3 +++ src/rule.c | 3 +++ 3 files changed, 7 insertions(+) diff --git a/include/rule.h b/include/rule.h index dd9df9ec6dd8..b41825d000d6 100644 --- a/include/rule.h +++ b/include/rule.h @@ -462,6 +462,7 @@ enum cmd_ops { CMD_EXPORT, CMD_MONITOR, CMD_DESCRIBE, + __CMD_FLUSH_RULESET, }; /** diff --git a/src/cache.c b/src/cache.c index 532ef425906a..d7153f6f6b8f 100644 --- a/src/cache.c +++ b/src/cache.c @@ -54,6 +54,9 @@ static unsigned int evaluate_cache_flush(struct cmd *cmd) unsigned int completeness = CMD_INVALID; switch (cmd->obj) { + case CMD_OBJ_RULESET: + completeness = __CMD_FLUSH_RULESET; + break; case CMD_OBJ_SET: case CMD_OBJ_MAP: case CMD_OBJ_METER: diff --git a/src/rule.c b/src/rule.c index 8de5aa62b94f..0c0fd07ec70c 100644 --- a/src/rule.c +++ b/src/rule.c @@ -210,6 +210,9 @@ static int cache_init(struct netlink_ctx *ctx, enum cmd_ops cmd) }; int ret; + if (cmd == __CMD_FLUSH_RULESET) + return 0; + ret = cache_init_tables(ctx, &handle, &ctx->nft->cache); if (ret < 0) return ret; -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command 2019-06-14 12:36 [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command Pablo Neira Ayuso @ 2019-06-14 12:54 ` Phil Sutter 2019-06-14 12:59 ` Pablo Neira Ayuso 0 siblings, 1 reply; 6+ messages in thread From: Phil Sutter @ 2019-06-14 12:54 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw Hi Pablo, On Fri, Jun 14, 2019 at 02:36:30PM +0200, Pablo Neira Ayuso wrote: > __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink > dump to populate the cache. This patch is a workaround until we have a > better infrastructure to track the state of the cache objects. I assumed the problem wouldn't exist anymore since we're populating the cache just once. Can you maybe elaborate a bit on the problem you're trying to solve with that workaround? Cheers, Phil ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command 2019-06-14 12:54 ` Phil Sutter @ 2019-06-14 12:59 ` Pablo Neira Ayuso 2019-06-14 13:04 ` Pablo Neira Ayuso 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2019-06-14 12:59 UTC (permalink / raw) To: Phil Sutter, netfilter-devel, fw On Fri, Jun 14, 2019 at 02:54:32PM +0200, Phil Sutter wrote: > Hi Pablo, > > On Fri, Jun 14, 2019 at 02:36:30PM +0200, Pablo Neira Ayuso wrote: > > __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink > > dump to populate the cache. This patch is a workaround until we have a > > better infrastructure to track the state of the cache objects. > > I assumed the problem wouldn't exist anymore since we're populating the > cache just once. Can you maybe elaborate a bit on the problem you're > trying to solve with that workaround? If nft segfaults to dump the cache, 'nft flush ruleset' will not work since it always fetches the cache, it will segfault too. The flush ruleset command was still dumping the cache before this patch. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command 2019-06-14 12:59 ` Pablo Neira Ayuso @ 2019-06-14 13:04 ` Pablo Neira Ayuso 2019-06-14 13:41 ` Phil Sutter 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2019-06-14 13:04 UTC (permalink / raw) To: Phil Sutter, netfilter-devel, fw On Fri, Jun 14, 2019 at 02:59:10PM +0200, Pablo Neira Ayuso wrote: > On Fri, Jun 14, 2019 at 02:54:32PM +0200, Phil Sutter wrote: > > Hi Pablo, > > > > On Fri, Jun 14, 2019 at 02:36:30PM +0200, Pablo Neira Ayuso wrote: > > > __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink > > > dump to populate the cache. This patch is a workaround until we have a > > > better infrastructure to track the state of the cache objects. > > > > I assumed the problem wouldn't exist anymore since we're populating the > > cache just once. Can you maybe elaborate a bit on the problem you're > > trying to solve with that workaround? > > If nft segfaults to dump the cache, 'nft flush ruleset' will not work > since it always fetches the cache, it will segfault too. > > The flush ruleset command was still dumping the cache before this > patch. In general, we still need to improve the cache logic, to make it finer grain. Now that we have a single point to populate the cache, things will get more simple. We need to replace the cache command level to cache flags or our own cache level definitions. The existing approach that uses of commands to define the cache level completeness has its own limitations. We can discuss this during the NFWS :-). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command 2019-06-14 13:04 ` Pablo Neira Ayuso @ 2019-06-14 13:41 ` Phil Sutter 2019-06-17 13:07 ` Pablo Neira Ayuso 0 siblings, 1 reply; 6+ messages in thread From: Phil Sutter @ 2019-06-14 13:41 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw Hi Pablo, On Fri, Jun 14, 2019 at 03:04:38PM +0200, Pablo Neira Ayuso wrote: > On Fri, Jun 14, 2019 at 02:59:10PM +0200, Pablo Neira Ayuso wrote: > > On Fri, Jun 14, 2019 at 02:54:32PM +0200, Phil Sutter wrote: > > > Hi Pablo, > > > > > > On Fri, Jun 14, 2019 at 02:36:30PM +0200, Pablo Neira Ayuso wrote: > > > > __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink > > > > dump to populate the cache. This patch is a workaround until we have a > > > > better infrastructure to track the state of the cache objects. > > > > > > I assumed the problem wouldn't exist anymore since we're populating the > > > cache just once. Can you maybe elaborate a bit on the problem you're > > > trying to solve with that workaround? > > > > If nft segfaults to dump the cache, 'nft flush ruleset' will not work > > since it always fetches the cache, it will segfault too. > > > > The flush ruleset command was still dumping the cache before this > > patch. > > In general, we still need to improve the cache logic, to make it finer > grain. Now that we have a single point to populate the cache, things > will get more simple. We need to replace the cache command level to > cache flags or our own cache level definitions. The existing approach > that uses of commands to define the cache level completeness has its > own limitations. We can discuss this during the NFWS :-). Yes, I had the same thought already. It is quite unintuitive how we link cache completeness to certain commands. :) Regarding your problem, maybe cache_update() should exit immediately if passed cmd is CMD_INVALID? Unless I miss something, if cache_evaluate() returns that value, we don't need a cache at all. Cheers, Phil ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command 2019-06-14 13:41 ` Phil Sutter @ 2019-06-17 13:07 ` Pablo Neira Ayuso 0 siblings, 0 replies; 6+ messages in thread From: Pablo Neira Ayuso @ 2019-06-17 13:07 UTC (permalink / raw) To: Phil Sutter, netfilter-devel, fw Hi Phil, On Fri, Jun 14, 2019 at 03:41:24PM +0200, Phil Sutter wrote: [...] > On Fri, Jun 14, 2019 at 03:04:38PM +0200, Pablo Neira Ayuso wrote: > > On Fri, Jun 14, 2019 at 02:59:10PM +0200, Pablo Neira Ayuso wrote: > > > On Fri, Jun 14, 2019 at 02:54:32PM +0200, Phil Sutter wrote: > > > > Hi Pablo, > > > > > > > > On Fri, Jun 14, 2019 at 02:36:30PM +0200, Pablo Neira Ayuso wrote: > > > > > __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink > > > > > dump to populate the cache. This patch is a workaround until we have a > > > > > better infrastructure to track the state of the cache objects. > > > > > > > > I assumed the problem wouldn't exist anymore since we're populating the > > > > cache just once. Can you maybe elaborate a bit on the problem you're > > > > trying to solve with that workaround? > > > > > > If nft segfaults to dump the cache, 'nft flush ruleset' will not work > > > since it always fetches the cache, it will segfault too. > > > > > > The flush ruleset command was still dumping the cache before this > > > patch. > > > > In general, we still need to improve the cache logic, to make it finer > > grain. Now that we have a single point to populate the cache, things > > will get more simple. We need to replace the cache command level to > > cache flags or our own cache level definitions. The existing approach > > that uses of commands to define the cache level completeness has its > > own limitations. We can discuss this during the NFWS :-). > > Yes, I had the same thought already. It is quite unintuitive how we link > cache completeness to certain commands. :) Just sent a follow up patch to introduce cache level flags [1]. The existing approach is rather conservative in what we fetch from the kernel (sometimes more than needed) but it should be possible to review this logic incrementally. > Regarding your problem, maybe cache_update() should exit immediately if > passed cmd is CMD_INVALID? Unless I miss something, if cache_evaluate() > returns that value, we don't need a cache at all. Problem is that CMD_INVALID does not mean empty cache. With the new cache level infrastructure, now there is a NFT_CACHE_EMPTY that provides the semantics this workaround was providing in a clearer way I think. [1] https://patchwork.ozlabs.org/patch/1116973/ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-17 13:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-14 12:36 [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command Pablo Neira Ayuso 2019-06-14 12:54 ` Phil Sutter 2019-06-14 12:59 ` Pablo Neira Ayuso 2019-06-14 13:04 ` Pablo Neira Ayuso 2019-06-14 13:41 ` Phil Sutter 2019-06-17 13:07 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).