netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* json_cmd_assoc and cmd
@ 2019-07-16 18:31 Pablo Neira Ayuso
  2019-07-16 19:02 ` Phil Sutter
  0 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-16 18:31 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

Hi Phil,

Why json_cmd_assoc is not placed in struct cmd instead? I mean, just
store the json_t *json in cmd?

This code could be simplified if you store this information in the
command object.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: json_cmd_assoc and cmd
  2019-07-16 18:31 json_cmd_assoc and cmd Pablo Neira Ayuso
@ 2019-07-16 19:02 ` Phil Sutter
  2019-07-16 19:39   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 15+ messages in thread
From: Phil Sutter @ 2019-07-16 19:02 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Hi Pablo,

On Tue, Jul 16, 2019 at 08:31:01PM +0200, Pablo Neira Ayuso wrote:
> Why json_cmd_assoc is not placed in struct cmd instead? I mean, just
> store the json_t *json in cmd?

The global list (json_cmd_list) is used in json_events_cb(). Unless I
miss something, the cmd list is not available from struct
netlink_mon_handler.

Maybe I could move struct cmds list head into struct nft_ctx?

Thanks, Phil

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: json_cmd_assoc and cmd
  2019-07-16 19:02 ` Phil Sutter
@ 2019-07-16 19:39   ` Pablo Neira Ayuso
  2019-07-18 12:37     ` Jeremy Sowden
  0 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-16 19:39 UTC (permalink / raw)
  To: Phil Sutter, netfilter-devel

On Tue, Jul 16, 2019 at 09:02:24PM +0200, Phil Sutter wrote:
> Hi Pablo,
>
> On Tue, Jul 16, 2019 at 08:31:01PM +0200, Pablo Neira Ayuso wrote:
> > Why json_cmd_assoc is not placed in struct cmd instead? I mean, just
> > store the json_t *json in cmd?
>
> The global list (json_cmd_list) is used in json_events_cb(). Unless I
> miss something, the cmd list is not available from struct
> netlink_mon_handler.

I see, thanks for explaining.

> Maybe I could move struct cmds list head into struct nft_ctx?

Probably place this in netlink_ctx?

We could also store num_cmds there too.

BTW, not directly related to this, but isn't this strange?

        list_for_each_entry(cmd, cmds, list) {
                memset(&ctx, 0, sizeof(ctx));
                ctx.msgs = msgs;
                ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
                ctx.batch = batch;
                ctx.nft = nft;
                init_list_head(&ctx.list);
                ret = do_command(&ctx, cmd);
                ...

ctx is reset over and over again. Then, recycled here:

                ret = mnl_batch_talk(&ctx, &err_list, num_cmds);

I wonder if we can get this better.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: json_cmd_assoc and cmd
  2019-07-16 19:39   ` Pablo Neira Ayuso
@ 2019-07-18 12:37     ` Jeremy Sowden
  2019-07-18 14:11       ` Phil Sutter
  2019-07-18 14:57       ` Pablo Neira Ayuso
  0 siblings, 2 replies; 15+ messages in thread
From: Jeremy Sowden @ 2019-07-18 12:37 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Phil Sutter, netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On 2019-07-16, at 21:39:03 +0200, Pablo Neira Ayuso wrote:
> BTW, not directly related to this, but isn't this strange?
>
>         list_for_each_entry(cmd, cmds, list) {
>                 memset(&ctx, 0, sizeof(ctx));
>                 ctx.msgs = msgs;
>                 ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
>                 ctx.batch = batch;
>                 ctx.nft = nft;
>                 init_list_head(&ctx.list);
>                 ret = do_command(&ctx, cmd);
>                 ...
>
> ctx is reset over and over again. Then, recycled here:
>
>                 ret = mnl_batch_talk(&ctx, &err_list, num_cmds);
>
> I wonder if we can get this better.

Something like this?

        ...
	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };
        ...

	ctx.batch = batch = mnl_batch_init();
	batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
	list_for_each_entry(cmd, cmds, list) {
		ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
		init_list_head(&ctx.list);
		ret = do_command(&ctx, cmd);
		...
	}

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: json_cmd_assoc and cmd
  2019-07-18 12:37     ` Jeremy Sowden
@ 2019-07-18 14:11       ` Phil Sutter
  2019-07-18 14:57       ` Pablo Neira Ayuso
  1 sibling, 0 replies; 15+ messages in thread
From: Phil Sutter @ 2019-07-18 14:11 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Pablo Neira Ayuso, netfilter-devel

Hi,

On Thu, Jul 18, 2019 at 01:37:04PM +0100, Jeremy Sowden wrote:
> On 2019-07-16, at 21:39:03 +0200, Pablo Neira Ayuso wrote:
> > BTW, not directly related to this, but isn't this strange?
> >
> >         list_for_each_entry(cmd, cmds, list) {
> >                 memset(&ctx, 0, sizeof(ctx));
> >                 ctx.msgs = msgs;
> >                 ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
> >                 ctx.batch = batch;
> >                 ctx.nft = nft;
> >                 init_list_head(&ctx.list);
> >                 ret = do_command(&ctx, cmd);
> >                 ...
> >
> > ctx is reset over and over again. Then, recycled here:
> >
> >                 ret = mnl_batch_talk(&ctx, &err_list, num_cmds);
> >
> > I wonder if we can get this better.
> 
> Something like this?
> 
>         ...
> 	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };
>         ...
> 
> 	ctx.batch = batch = mnl_batch_init();
> 	batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
> 	list_for_each_entry(cmd, cmds, list) {
> 		ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
> 		init_list_head(&ctx.list);
> 		ret = do_command(&ctx, cmd);
> 		...
> 	}

Yes, that at least simplifies the foreach loop a bit. I wonder though
if we could eliminate struct netlink_ctx altogether by moving pointers
into struct nft_ctx.

Pablo, do you think that's feasible?

Cheers, Phil

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: json_cmd_assoc and cmd
  2019-07-18 12:37     ` Jeremy Sowden
  2019-07-18 14:11       ` Phil Sutter
@ 2019-07-18 14:57       ` Pablo Neira Ayuso
  2019-07-18 15:07         ` Jeremy Sowden
  2019-07-18 21:05         ` [PATCH nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
  1 sibling, 2 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-18 14:57 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Phil Sutter, netfilter-devel

On Thu, Jul 18, 2019 at 01:37:04PM +0100, Jeremy Sowden wrote:
> On 2019-07-16, at 21:39:03 +0200, Pablo Neira Ayuso wrote:
> > BTW, not directly related to this, but isn't this strange?
> >
> >         list_for_each_entry(cmd, cmds, list) {
> >                 memset(&ctx, 0, sizeof(ctx));
> >                 ctx.msgs = msgs;
> >                 ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
> >                 ctx.batch = batch;
> >                 ctx.nft = nft;
> >                 init_list_head(&ctx.list);
> >                 ret = do_command(&ctx, cmd);
> >                 ...
> >
> > ctx is reset over and over again. Then, recycled here:
> >
> >                 ret = mnl_batch_talk(&ctx, &err_list, num_cmds);
> >
> > I wonder if we can get this better.
> 
> Something like this?

Yes, something like that would get things in better shape I think,
more comments below.

> 	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };
>         ...
> 
> 	ctx.batch = batch = mnl_batch_init();
> 	batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
> 	list_for_each_entry(cmd, cmds, list) {
> 		ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
> 		init_list_head(&ctx.list);

I think we don't need to re-initialize this list over and over again
(from what I see when doing: git grep "ctx->list").

This always does list_splice_tail_init() to attach the object list
where they belong.

You can probably add something like:

        if (!list_empty(&ctx->list))
                BUG("command list is not empty\n");

I would make a patch and run tests/shell and tests/py to check if what
I'm suggesting this fine :-)

> 		ret = do_command(&ctx, cmd);
> 		...

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: json_cmd_assoc and cmd
  2019-07-18 14:57       ` Pablo Neira Ayuso
@ 2019-07-18 15:07         ` Jeremy Sowden
  2019-07-18 21:05         ` [PATCH nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
  1 sibling, 0 replies; 15+ messages in thread
From: Jeremy Sowden @ 2019-07-18 15:07 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Phil Sutter, netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

On 2019-07-18, at 16:57:22 +0200, Pablo Neira Ayuso wrote:
> On Thu, Jul 18, 2019 at 01:37:04PM +0100, Jeremy Sowden wrote:
> > On 2019-07-16, at 21:39:03 +0200, Pablo Neira Ayuso wrote:
> > > BTW, not directly related to this, but isn't this strange?
> > >
> > >         list_for_each_entry(cmd, cmds, list) {
> > >                 memset(&ctx, 0, sizeof(ctx));
> > >                 ctx.msgs = msgs;
> > >                 ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
> > >                 ctx.batch = batch;
> > >                 ctx.nft = nft;
> > >                 init_list_head(&ctx.list);
> > >                 ret = do_command(&ctx, cmd);
> > >                 ...
> > >
> > > ctx is reset over and over again. Then, recycled here:
> > >
> > >                 ret = mnl_batch_talk(&ctx, &err_list, num_cmds);
> > >
> > > I wonder if we can get this better.
> >
> > Something like this?
>
> Yes, something like that would get things in better shape I think,
> more comments below.
>
> > 	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };
> >         ...
> >
> > 	ctx.batch = batch = mnl_batch_init();
> > 	batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
> > 	list_for_each_entry(cmd, cmds, list) {
> > 		ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
> > 		init_list_head(&ctx.list);
>
> I think we don't need to re-initialize this list over and over again
> (from what I see when doing: git grep "ctx->list").
>
> This always does list_splice_tail_init() to attach the object list
> where they belong.

Right.  Got it.

> You can probably add something like:
>
>         if (!list_empty(&ctx->list))
>                 BUG("command list is not empty\n");
>
> I would make a patch and run tests/shell and tests/py to check if what
> I'm suggesting this fine :-)

I've compiled the changes I outlined above and run `make check`.  Will
add the list changes and submit a patch once I've tested them.

> > 		ret = do_command(&ctx, cmd);
> > 		...

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop.
  2019-07-18 14:57       ` Pablo Neira Ayuso
  2019-07-18 15:07         ` Jeremy Sowden
@ 2019-07-18 21:05         ` Jeremy Sowden
  2019-07-19 10:32           ` Phil Sutter
  1 sibling, 1 reply; 15+ messages in thread
From: Jeremy Sowden @ 2019-07-18 21:05 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Pablo Neira Ayuso, Phil Sutter

Most members in the context doesn't change, so there is no need to
memset it and reassign most of its members on every iteration.  Moved
that code out of the loop.

Fixes: 49900d448ac9 ("libnftables: Move library stuff out of main.c")
Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/libnftables.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/libnftables.c b/src/libnftables.c
index 2f77a7709e2c..834ea661a146 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -23,7 +23,7 @@ static int nft_netlink(struct nft_ctx *nft,
 {
 	uint32_t batch_seqnum, seqnum = 0, num_cmds = 0;
 	struct nftnl_batch *batch;
-	struct netlink_ctx ctx;
+	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };
 	struct cmd *cmd;
 	struct mnl_err *err, *tmp;
 	LIST_HEAD(err_list);
@@ -32,16 +32,13 @@ static int nft_netlink(struct nft_ctx *nft,
 	if (list_empty(cmds))
 		return 0;
 
-	batch = mnl_batch_init();
+	init_list_head(&ctx.list);
+
+	ctx.batch = batch = mnl_batch_init();
 
 	batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
 	list_for_each_entry(cmd, cmds, list) {
-		memset(&ctx, 0, sizeof(ctx));
-		ctx.msgs = msgs;
 		ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
-		ctx.batch = batch;
-		ctx.nft = nft;
-		init_list_head(&ctx.list);
 		ret = do_command(&ctx, cmd);
 		if (ret < 0) {
 			netlink_io_error(&ctx, &cmd->location,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop.
  2019-07-18 21:05         ` [PATCH nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
@ 2019-07-19 10:32           ` Phil Sutter
  2019-07-19 11:10             ` [PATCH nft v2 0/2] netlink_ctx initialization fixes Jeremy Sowden
  0 siblings, 1 reply; 15+ messages in thread
From: Phil Sutter @ 2019-07-19 10:32 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Pablo Neira Ayuso

Hi,

On Thu, Jul 18, 2019 at 10:05:52PM +0100, Jeremy Sowden wrote:
> Most members in the context doesn't change, so there is no need to
> memset it and reassign most of its members on every iteration.  Moved
> that code out of the loop.
> 
> Fixes: 49900d448ac9 ("libnftables: Move library stuff out of main.c")

This commit merely moved the code from src/main.c into the current
location. I would rather point at a72315d2bad47 ("src: add rule batching
support"), which introduced the loop (and already contains some of the
pointless assignments). Note that commit is from 2013. :)

> Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/libnftables.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libnftables.c b/src/libnftables.c
> index 2f77a7709e2c..834ea661a146 100644
> --- a/src/libnftables.c
> +++ b/src/libnftables.c
> @@ -23,7 +23,7 @@ static int nft_netlink(struct nft_ctx *nft,
>  {
>  	uint32_t batch_seqnum, seqnum = 0, num_cmds = 0;
>  	struct nftnl_batch *batch;
> -	struct netlink_ctx ctx;
> +	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };

Use '.list = LIST_HEAD_INIT(ctx.list),' here. Refer to cache_update() in
src/rule.c for inspiration. :)

>  	struct cmd *cmd;
>  	struct mnl_err *err, *tmp;
>  	LIST_HEAD(err_list);
> @@ -32,16 +32,13 @@ static int nft_netlink(struct nft_ctx *nft,
>  	if (list_empty(cmds))
>  		return 0;
>  
> -	batch = mnl_batch_init();
> +	init_list_head(&ctx.list);

Drop this.

> +
> +	ctx.batch = batch = mnl_batch_init();

Move ctx.batch init into declaration as well. Drop dedicated 'batch'
variable and replace by 'ctx.batch'.

Thanks, Phil

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH nft v2 0/2] netlink_ctx initialization fixes.
  2019-07-19 10:32           ` Phil Sutter
@ 2019-07-19 11:10             ` Jeremy Sowden
  2019-07-19 11:10               ` [PATCH nft v2 1/2] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
                                 ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jeremy Sowden @ 2019-07-19 11:10 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Pablo Neira Ayuso, Phil Sutter

A couple of patches to tidy up initialization of a pair of netlink_ctx
variables.

Since v1 (based on Phil's feedback -- thanks, Phil):

 * Eliminated batch local variable from first patch.
 * Do initialization of .list and .batch into struct initializer from
   first patch.
 * Updated commit in "Fixes:" tag in first patch.
 * Added second patch.

Jeremy Sowden (2):
  libnftables: got rid of repeated initialization of netlink_ctx
    variable in loop.
  rule: removed duplicate member initializer.

 src/libnftables.c | 23 ++++++++++-------------
 src/rule.c        |  1 -
 2 files changed, 10 insertions(+), 14 deletions(-)

-- 
2.20.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH nft v2 1/2] libnftables: got rid of repeated initialization of netlink_ctx variable in loop.
  2019-07-19 11:10             ` [PATCH nft v2 0/2] netlink_ctx initialization fixes Jeremy Sowden
@ 2019-07-19 11:10               ` Jeremy Sowden
  2019-07-19 11:17                 ` Phil Sutter
  2019-07-19 11:10               ` [PATCH nft v2 2/2] rule: removed duplicate member initializer Jeremy Sowden
  2019-07-22 21:29               ` [PATCH nft v2 0/2] netlink_ctx initialization fixes Pablo Neira Ayuso
  2 siblings, 1 reply; 15+ messages in thread
From: Jeremy Sowden @ 2019-07-19 11:10 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Pablo Neira Ayuso, Phil Sutter

Most members in the context doesn't change, so there is no need to
memset it and reassign them on every iteration.  Moved that code out of
the loop.

Fixes: a72315d2bad4 ("src: add rule batching support")
Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/libnftables.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/libnftables.c b/src/libnftables.c
index 2f77a7709e2c..4a139c58b2b3 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -22,8 +22,12 @@ static int nft_netlink(struct nft_ctx *nft,
 		       struct mnl_socket *nf_sock)
 {
 	uint32_t batch_seqnum, seqnum = 0, num_cmds = 0;
-	struct nftnl_batch *batch;
-	struct netlink_ctx ctx;
+	struct netlink_ctx ctx = {
+		.nft  = nft,
+		.msgs = msgs,
+		.list = LIST_HEAD_INIT(ctx.list),
+		.batch = mnl_batch_init(),
+	};
 	struct cmd *cmd;
 	struct mnl_err *err, *tmp;
 	LIST_HEAD(err_list);
@@ -32,16 +36,9 @@ static int nft_netlink(struct nft_ctx *nft,
 	if (list_empty(cmds))
 		return 0;
 
-	batch = mnl_batch_init();
-
-	batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
+	batch_seqnum = mnl_batch_begin(ctx.batch, mnl_seqnum_alloc(&seqnum));
 	list_for_each_entry(cmd, cmds, list) {
-		memset(&ctx, 0, sizeof(ctx));
-		ctx.msgs = msgs;
 		ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
-		ctx.batch = batch;
-		ctx.nft = nft;
-		init_list_head(&ctx.list);
 		ret = do_command(&ctx, cmd);
 		if (ret < 0) {
 			netlink_io_error(&ctx, &cmd->location,
@@ -52,9 +49,9 @@ static int nft_netlink(struct nft_ctx *nft,
 		num_cmds++;
 	}
 	if (!nft->check)
-		mnl_batch_end(batch, mnl_seqnum_alloc(&seqnum));
+		mnl_batch_end(ctx.batch, mnl_seqnum_alloc(&seqnum));
 
-	if (!mnl_batch_ready(batch))
+	if (!mnl_batch_ready(ctx.batch))
 		goto out;
 
 	ret = mnl_batch_talk(&ctx, &err_list, num_cmds);
@@ -83,7 +80,7 @@ static int nft_netlink(struct nft_ctx *nft,
 		}
 	}
 out:
-	mnl_batch_reset(batch);
+	mnl_batch_reset(ctx.batch);
 	return ret;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH nft v2 2/2] rule: removed duplicate member initializer.
  2019-07-19 11:10             ` [PATCH nft v2 0/2] netlink_ctx initialization fixes Jeremy Sowden
  2019-07-19 11:10               ` [PATCH nft v2 1/2] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
@ 2019-07-19 11:10               ` Jeremy Sowden
  2019-07-19 11:17                 ` Phil Sutter
  2019-07-22 21:29               ` [PATCH nft v2 0/2] netlink_ctx initialization fixes Pablo Neira Ayuso
  2 siblings, 1 reply; 15+ messages in thread
From: Jeremy Sowden @ 2019-07-19 11:10 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Pablo Neira Ayuso, Phil Sutter

Initialization of a netlink_ctx included two initializers for .nft.
Removed one of them.

Fixes: 2dc07bcd7eaa ("src: pass struct nft_ctx through struct netlink_ctx")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/rule.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/rule.c b/src/rule.c
index b957b4571249..0ebe91e79a03 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -240,7 +240,6 @@ int cache_update(struct nft_ctx *nft, unsigned int flags, struct list_head *msgs
 		.list		= LIST_HEAD_INIT(ctx.list),
 		.nft		= nft,
 		.msgs		= msgs,
-		.nft		= nft,
 	};
 	struct nft_cache *cache = &nft->cache;
 	uint32_t genid, genid_stop;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH nft v2 2/2] rule: removed duplicate member initializer.
  2019-07-19 11:10               ` [PATCH nft v2 2/2] rule: removed duplicate member initializer Jeremy Sowden
@ 2019-07-19 11:17                 ` Phil Sutter
  0 siblings, 0 replies; 15+ messages in thread
From: Phil Sutter @ 2019-07-19 11:17 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Pablo Neira Ayuso

On Fri, Jul 19, 2019 at 12:10:10PM +0100, Jeremy Sowden wrote:
> Initialization of a netlink_ctx included two initializers for .nft.
> Removed one of them.

Oh, good catch! The fact that I stared ad this code ten minutes
ago and didn't see it speaks for itself. :)

> Fixes: 2dc07bcd7eaa ("src: pass struct nft_ctx through struct netlink_ctx")
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>

Acked-by: Phil Sutter <phil@nwl.cc>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH nft v2 1/2] libnftables: got rid of repeated initialization of netlink_ctx variable in loop.
  2019-07-19 11:10               ` [PATCH nft v2 1/2] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
@ 2019-07-19 11:17                 ` Phil Sutter
  0 siblings, 0 replies; 15+ messages in thread
From: Phil Sutter @ 2019-07-19 11:17 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Pablo Neira Ayuso

On Fri, Jul 19, 2019 at 12:10:09PM +0100, Jeremy Sowden wrote:
> Most members in the context doesn't change, so there is no need to
> memset it and reassign them on every iteration.  Moved that code out of
> the loop.
> 
> Fixes: a72315d2bad4 ("src: add rule batching support")
> Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>

Acked-by: Phil Sutter <phil@nwl.cc>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH nft v2 0/2] netlink_ctx initialization fixes.
  2019-07-19 11:10             ` [PATCH nft v2 0/2] netlink_ctx initialization fixes Jeremy Sowden
  2019-07-19 11:10               ` [PATCH nft v2 1/2] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
  2019-07-19 11:10               ` [PATCH nft v2 2/2] rule: removed duplicate member initializer Jeremy Sowden
@ 2019-07-22 21:29               ` Pablo Neira Ayuso
  2 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-22 21:29 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Phil Sutter

On Fri, Jul 19, 2019 at 12:10:08PM +0100, Jeremy Sowden wrote:
> A couple of patches to tidy up initialization of a pair of netlink_ctx
> variables.

Applied, thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-07-22 21:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-16 18:31 json_cmd_assoc and cmd Pablo Neira Ayuso
2019-07-16 19:02 ` Phil Sutter
2019-07-16 19:39   ` Pablo Neira Ayuso
2019-07-18 12:37     ` Jeremy Sowden
2019-07-18 14:11       ` Phil Sutter
2019-07-18 14:57       ` Pablo Neira Ayuso
2019-07-18 15:07         ` Jeremy Sowden
2019-07-18 21:05         ` [PATCH nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
2019-07-19 10:32           ` Phil Sutter
2019-07-19 11:10             ` [PATCH nft v2 0/2] netlink_ctx initialization fixes Jeremy Sowden
2019-07-19 11:10               ` [PATCH nft v2 1/2] libnftables: got rid of repeated initialization of netlink_ctx variable in loop Jeremy Sowden
2019-07-19 11:17                 ` Phil Sutter
2019-07-19 11:10               ` [PATCH nft v2 2/2] rule: removed duplicate member initializer Jeremy Sowden
2019-07-19 11:17                 ` Phil Sutter
2019-07-22 21:29               ` [PATCH nft v2 0/2] netlink_ctx initialization fixes 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).