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