diff mbox series

[nft,v4,1/7] src: Fix cache_flush() in cache_needs_more() logic

Message ID 20190528210323.14605-2-phil@nwl.cc
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series Cache update fix && intra-transaction rule references | expand

Commit Message

Phil Sutter May 28, 2019, 9:03 p.m. UTC
Commit 34a20645d54fa enabled cache updates depending on command causing
it. As a side-effect, this disabled measures in cache_flush() preventing
a later cache update. Re-establish this by setting cache->cmd in
addition to cache->genid after dropping cache entries.

While being at it, set cache->cmd in cache_release() as well. This
shouldn't be necessary since zeroing cache->genid should suffice for
cache_update(), but better be consistent (and future-proof) here.

Fixes: 34a20645d54fa ("src: update cache if cmd is more specific")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Adjust cache_release() as well, just to make sure.
---
 src/rule.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Eric Garver May 28, 2019, 9:32 p.m. UTC | #1
On Tue, May 28, 2019 at 11:03:17PM +0200, Phil Sutter wrote:
> Commit 34a20645d54fa enabled cache updates depending on command causing

Actual hash is eeda228c2d17. 

> it. As a side-effect, this disabled measures in cache_flush() preventing
> a later cache update. Re-establish this by setting cache->cmd in
> addition to cache->genid after dropping cache entries.
> 
> While being at it, set cache->cmd in cache_release() as well. This
> shouldn't be necessary since zeroing cache->genid should suffice for
> cache_update(), but better be consistent (and future-proof) here.
> 
> Fixes: 34a20645d54fa ("src: update cache if cmd is more specific")

Actual hash is eeda228c2d17. 

> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Adjust cache_release() as well, just to make sure.
> ---
>  src/rule.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/rule.c b/src/rule.c
> index 326edb5dd459a..966948cd7ae90 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -299,12 +299,15 @@ void cache_flush(struct nft_ctx *nft, enum cmd_ops cmd, struct list_head *msgs)
>  
>  	__cache_flush(&cache->list);
>  	cache->genid = mnl_genid_get(&ctx);
> +	cache->cmd = CMD_LIST;
>  }
>  
>  void cache_release(struct nft_cache *cache)
>  {
>  	__cache_flush(&cache->list);
>  	cache->genid = 0;
> +	cache->cmd = CMD_INVALID;
> +
>  }
>  
>  /* internal ID to uniquely identify a set in the batch */
> -- 
> 2.21.0

Otherwise looks good.
Phil Sutter May 28, 2019, 10:23 p.m. UTC | #2
On Tue, May 28, 2019 at 05:32:44PM -0400, Eric Garver wrote:
> On Tue, May 28, 2019 at 11:03:17PM +0200, Phil Sutter wrote:
> > Commit 34a20645d54fa enabled cache updates depending on command causing
> 
> Actual hash is eeda228c2d17. 

Oh, thanks for the catch! No idea where the other hash came from, but it
exists in my repository at least.

Thanks, Phil
diff mbox series

Patch

diff --git a/src/rule.c b/src/rule.c
index 326edb5dd459a..966948cd7ae90 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -299,12 +299,15 @@  void cache_flush(struct nft_ctx *nft, enum cmd_ops cmd, struct list_head *msgs)
 
 	__cache_flush(&cache->list);
 	cache->genid = mnl_genid_get(&ctx);
+	cache->cmd = CMD_LIST;
 }
 
 void cache_release(struct nft_cache *cache)
 {
 	__cache_flush(&cache->list);
 	cache->genid = 0;
+	cache->cmd = CMD_INVALID;
+
 }
 
 /* internal ID to uniquely identify a set in the batch */