Hi Pablo,
On Thu, Oct 17, 2019 at 03:34:55PM +0200, Pablo Neira Ayuso wrote:
> If --echo is passed, then the cache already contains the commands that
> have been sent to the kernel. However, anonymous sets are an exception
> since the cache needs to be updated in this case.
>
> Remove the old cache logic from the monitor code that has been replaced
> by 01e5c6f0ed03 ("src: add cache level flags").
>
> Signed-off-by: Pablo Neira Ayuso <[email protected]>
This is a nice solution, thanks! A few nits below:
[...]
> diff --git a/src/monitor.c b/src/monitor.c
> index 40c381149cda..b7b00d7b1343 100644
> --- a/src/monitor.c
> +++ b/src/monitor.c
> @@ -609,6 +609,12 @@ static void netlink_events_cache_addset(struct
> netlink_mon_handler *monh,
> goto out;
> }
>
> + if (nft_output_echo(&monh->ctx->nft->output) &&
> + !(s->flags & NFT_SET_ANONYMOUS)) {
There is set_is_anonymous(), set and element printing callbacks use it
as well.
[...]
> @@ -636,6 +642,10 @@ static void netlink_events_cache_addsetelem(struct
> netlink_mon_handler *monh,
> goto out;
> }
>
> + if (nft_output_echo(&monh->ctx->nft->output) &&
> + !(set->flags & NFT_SET_ANONYMOUS))
Same here.
[...]
> @@ -744,7 +754,8 @@ out:
> static void netlink_events_cache_update(struct netlink_mon_handler *monh,
> const struct nlmsghdr *nlh, int type)
> {
> - if (!monh->cache_needed)
> + if (nft_output_echo(&monh->ctx->nft->output) &&
> + type != NFT_MSG_NEWSET && type != NFT_MSG_NEWSETELEM)
> return;
I would use switch() here, like so:
| if (nft_output_echo(&monh->ctx->nft->output)) {
| switch (type) {
| case NFT_MSG_NEWSET:
| case NFT_MSG_NEWSETELEM:
| break;
| default:
| return;
| }
| }
it emphasises that code is filtering for certain event types. Up to you,
I'm fine with both.
Thanks, Phil
