[PATCH v3 0/8] conntrack: save output format

[PATCH v3 0/8] conntrack: save output format

[Mikhail Sennikovsky]

Hi Pablo & all,

As discussed, here is the updated version of the "save output format"
patches.
The main changes since the last series:
1. Adopting upon the latest master changes from Pablo
2. Line parsing logic is taken from the xtables as Pablo suggested
3. The changes made more granular.

Thanks & regards,
Mikhail

Mikhail Sennikovsky (8):
  conntrack: reset optind in do_parse
  conntrack: move global options to struct ct_cmd
  conntrack: per-command entries counters
  conntrack: introduce ct_cmd_list
  conntrack: accept commands from file
  conntrack.8: man update for --load-file support
  tests: saving and loading ct entries, save format
  tests: conntrack -L/-D ip family filtering

 conntrack.8                         |   8 +
 src/conntrack.c                     | 546 +++++++++++++++++++++-------
 tests/conntrack/test-conntrack.c    |  84 ++++-
 tests/conntrack/testsuite/08stdin   |  80 ++++
 tests/conntrack/testsuite/09dumpopt | 147 ++++++++
 5 files changed, 726 insertions(+), 139 deletions(-)
 create mode 100644 tests/conntrack/testsuite/08stdin
 create mode 100644 tests/conntrack/testsuite/09dumpopt

-- 
2.25.1

[PATCH v3 1/8] conntrack: reset optind in do_parse

[Mikhail Sennikovsky]

As a multicommand support preparation reset optind
for the case do_parse is called multiple times

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 src/conntrack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/conntrack.c b/src/conntrack.c
index 987d936..c582d86 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -2798,6 +2798,8 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 
 	/* disable explicit missing arguments error output from getopt_long */
 	opterr = 0;
+	/* reset optind, for the case do_parse is called multiple times */
+	optind = 0;
 
 	while ((c = getopt_long(argc, argv, getopt_str, opts, NULL)) != -1) {
 	switch(c) {
-- 
2.25.1

Re: [PATCH v3 1/8] conntrack: reset optind in do_parse

[Pablo Neira Ayuso]

On Fri, Jan 29, 2021 at 10:24:45PM +0100, Mikhail Sennikovsky wrote:
> As a multicommand support preparation reset optind
> for the case do_parse is called multiple times

Patch looks good, I'd suggest to collapse it to 4/8 and 5/8 in the
next round.

Thanks.

Re: [PATCH v3 1/8] conntrack: reset optind in do_parse

[Mikhail Sennikovsky]

Hi Pablo,

Sure, so taking into account your comments to 3/8 and 4/8 I collapse
1-5/8 in a single commit, correct?

Regards,
Mikhail

On Mon, 15 Mar 2021 at 18:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Fri, Jan 29, 2021 at 10:24:45PM +0100, Mikhail Sennikovsky wrote:
> > As a multicommand support preparation reset optind
> > for the case do_parse is called multiple times
>
> Patch looks good, I'd suggest to collapse it to 4/8 and 5/8 in the
> next round.
>
> Thanks.

Re: [PATCH v3 1/8] conntrack: reset optind in do_parse

[Pablo Neira Ayuso]

On Wed, Mar 17, 2021 at 07:31:18PM +0100, Mikhail Sennikovsky wrote:
> Hi Pablo,
> 
> Sure, so taking into account your comments to 3/8 and 4/8 I collapse
> 1-5/8 in a single commit, correct?

Yes please. My understanding is that they belong to the same logical
changes, that is, add batch support.

So this new "batch support" for conntrack is all new code that can be
added at once IMO.

Thanks.

[PATCH v3 2/8] conntrack: move global options to struct ct_cmd

[Mikhail Sennikovsky]

As a multicommand support preparation options must be done
per-command es well.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 src/conntrack.c | 205 ++++++++++++++++++++++++++----------------------
 1 file changed, 113 insertions(+), 92 deletions(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index c582d86..a090542 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -600,7 +600,6 @@ static unsigned int addr_valid_flags[ADDR_VALID_FLAGS_MAX] = {
 
 static LIST_HEAD(proto_list);
 
-static unsigned int options;
 static struct nfct_labelmap *labelmap;
 static int filter_family;
 
@@ -1460,6 +1459,19 @@ static void usage(const char *prog)
 
 static unsigned int output_mask;
 
+struct ct_cmd {
+	struct list_head list_entry;
+	unsigned int	command;
+	unsigned int	cmd;
+	int             options;
+	unsigned int	type;
+	unsigned int	event_mask;
+	int		family;
+	int		protonum;
+	size_t		socketbuffersize;
+	struct ct_tmpl	tmpl;
+};
+
 static int
 filter_label(const struct nf_conntrack *ct, const struct ct_tmpl *tmpl)
 {
@@ -1480,24 +1492,27 @@ filter_label(const struct nf_conntrack *ct, const struct ct_tmpl *tmpl)
 }
 
 static int
-filter_mark(const struct nf_conntrack *ct, const struct ct_tmpl *tmpl)
+filter_mark(const struct ct_cmd *cmd,
+		const struct nf_conntrack *ct, const struct ct_tmpl *tmpl)
 {
-	if ((options & CT_OPT_MARK) &&
+	if ((cmd->options & CT_OPT_MARK) &&
 	     !mark_cmp(&tmpl->mark, ct))
 		return 1;
 	return 0;
 }
 
 static int 
-filter_nat(const struct nf_conntrack *obj, const struct nf_conntrack *ct)
+filter_nat(const struct ct_cmd *cmd,
+		const struct nf_conntrack *obj,
+		const struct nf_conntrack *ct)
 {
-	int check_srcnat = options & CT_OPT_SRC_NAT ? 1 : 0;
-	int check_dstnat = options & CT_OPT_DST_NAT ? 1 : 0;
+	int check_srcnat = cmd->options & CT_OPT_SRC_NAT ? 1 : 0;
+	int check_dstnat = cmd->options & CT_OPT_DST_NAT ? 1 : 0;
 	int has_srcnat = 0, has_dstnat = 0;
 	uint32_t ip;
 	uint16_t port;
 
-	if (options & CT_OPT_ANY_NAT)
+	if (cmd->options & CT_OPT_ANY_NAT)
 		check_srcnat = check_dstnat = 1;
 
 	if (check_srcnat) {
@@ -1560,13 +1575,14 @@ filter_nat(const struct nf_conntrack *obj, const struct nf_conntrack *ct)
 		     nfct_getobjopt(ct, NFCT_GOPT_IS_DPAT)))
 			has_dstnat = 1;
 	}
-	if (options & CT_OPT_ANY_NAT)
+	if (cmd->options & CT_OPT_ANY_NAT)
 		return !(has_srcnat || has_dstnat);
-	else if ((options & CT_OPT_SRC_NAT) && (options & CT_OPT_DST_NAT))
+	else if ((cmd->options & CT_OPT_SRC_NAT)
+			&& (cmd->options & CT_OPT_DST_NAT))
 		return !(has_srcnat && has_dstnat);
-	else if (options & CT_OPT_SRC_NAT)
+	else if (cmd->options & CT_OPT_SRC_NAT)
 		return !has_srcnat;
-	else if (options & CT_OPT_DST_NAT)
+	else if (cmd->options & CT_OPT_DST_NAT)
 		return !has_dstnat;
 
 	return 0;
@@ -1614,14 +1630,14 @@ nfct_filter_network_direction(const struct nf_conntrack *ct, enum ct_direction d
 }
 
 static int
-filter_network(const struct nf_conntrack *ct)
+filter_network(const struct ct_cmd *cmd, const struct nf_conntrack *ct)
 {
-	if (options & CT_OPT_MASK_SRC) {
+	if (cmd->options & CT_OPT_MASK_SRC) {
 		if (nfct_filter_network_direction(ct, DIR_SRC))
 			return 1;
 	}
 
-	if (options & CT_OPT_MASK_DST) {
+	if (cmd->options & CT_OPT_MASK_DST) {
 		if (nfct_filter_network_direction(ct, DIR_DST))
 			return 1;
 	}
@@ -1629,16 +1645,18 @@ filter_network(const struct nf_conntrack *ct)
 }
 
 static int
-nfct_filter(struct nf_conntrack *obj, struct nf_conntrack *ct,
+nfct_filter(const struct ct_cmd *cmd,
+		const struct nf_conntrack *obj,
+		struct nf_conntrack *ct,
 	    const struct ct_tmpl *tmpl)
 {
-	if (filter_nat(obj, ct) ||
-	    filter_mark(ct, tmpl) ||
+	if (filter_nat(cmd, obj, ct) ||
+	    filter_mark(cmd, ct, tmpl) ||
 	    filter_label(ct, tmpl) ||
-	    filter_network(ct))
+	    filter_network(cmd, ct))
 		return 1;
 
-	if (options & CT_COMPARISON &&
+	if (cmd->options & CT_COMPARISON &&
 	    !nfct_cmp(obj, ct, NFCT_CMP_ALL | NFCT_CMP_MASK))
 		return 1;
 
@@ -1843,7 +1861,8 @@ static int event_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nfgenmsg *nfh = mnl_nlmsg_get_payload(nlh);
 	unsigned int op_type = NFCT_O_DEFAULT;
-	struct nf_conntrack *obj = data;
+	struct ct_cmd *cmd = data;
+	struct nf_conntrack *obj = cmd->tmpl.ct;
 	enum nf_conntrack_msg_type type;
 	unsigned int op_flags = 0;
 	struct nf_conntrack *ct;
@@ -1874,7 +1893,7 @@ static int event_cb(const struct nlmsghdr *nlh, void *data)
 
 	if ((filter_family != AF_UNSPEC &&
 	     filter_family != nfh->nfgen_family) ||
-	    nfct_filter(obj, ct, cur_tmpl))
+	    nfct_filter(cmd, obj, ct, cur_tmpl))
 		goto out;
 
 	if (output_mask & _O_SAVE) {
@@ -1930,11 +1949,12 @@ static int dump_cb(enum nf_conntrack_msg_type type,
 		   void *data)
 {
 	char buf[1024];
-	struct nf_conntrack *obj = data;
+	struct ct_cmd *cmd = data;
+	struct nf_conntrack *obj = cmd->tmpl.ct;
 	unsigned int op_type = NFCT_O_DEFAULT;
 	unsigned int op_flags = 0;
 
-	if (nfct_filter(obj, ct, cur_tmpl))
+	if (nfct_filter(cmd, obj, ct, cur_tmpl))
 		return NFCT_CB_CONTINUE;
 
 	if (output_mask & _O_SAVE) {
@@ -1972,11 +1992,12 @@ static int delete_cb(enum nf_conntrack_msg_type type,
 {
 	int res;
 	char buf[1024];
-	struct nf_conntrack *obj = data;
+	struct ct_cmd *cmd = data;
+	struct nf_conntrack *obj = cmd->tmpl.ct;
 	unsigned int op_type = NFCT_O_DEFAULT;
 	unsigned int op_flags = 0;
 
-	if (nfct_filter(obj, ct, cur_tmpl))
+	if (nfct_filter(cmd, obj, ct, cur_tmpl))
 		return NFCT_CB_CONTINUE;
 
 	res = nfct_query(ith, NFCT_Q_DESTROY, ct);
@@ -2033,20 +2054,23 @@ done:
 	return NFCT_CB_CONTINUE;
 }
 
-static void copy_mark(struct nf_conntrack *tmp,
+static void copy_mark(const struct ct_cmd *cmd,
+		      struct nf_conntrack *tmp,
 		      const struct nf_conntrack *ct,
 		      const struct u32_mask *m)
 {
-	if (options & CT_OPT_MARK) {
+	if (cmd->options & CT_OPT_MARK) {
 		uint32_t mark = nfct_get_attr_u32(ct, ATTR_MARK);
 		mark = (mark & ~m->mask) ^ m->value;
 		nfct_set_attr_u32(tmp, ATTR_MARK, mark);
 	}
 }
 
-static void copy_status(struct nf_conntrack *tmp, const struct nf_conntrack *ct)
+static void copy_status(const struct ct_cmd *cmd,
+		      struct nf_conntrack *tmp,
+		      const struct nf_conntrack *ct)
 {
-	if (options & CT_OPT_STATUS) {
+	if (cmd->options & CT_OPT_STATUS) {
 		/* copy existing flags, we only allow setting them. */
 		uint32_t status = nfct_get_attr_u32(ct, ATTR_STATUS);
 		status |= nfct_get_attr_u32(tmp, ATTR_STATUS);
@@ -2062,19 +2086,21 @@ static struct nfct_bitmask *xnfct_bitmask_clone(const struct nfct_bitmask *a)
 	return b;
 }
 
-static void copy_label(struct nf_conntrack *tmp, const struct nf_conntrack *ct,
-		       const struct ct_tmpl *tmpl)
+static void copy_label(const struct ct_cmd *cmd,
+		      struct nf_conntrack *tmp,
+			  const struct nf_conntrack *ct,
+		      const struct ct_tmpl *tmpl)
 {
 	struct nfct_bitmask *ctb, *newmask;
 	unsigned int i;
 
-	if ((options & (CT_OPT_ADD_LABEL|CT_OPT_DEL_LABEL)) == 0)
+	if ((cmd->options & (CT_OPT_ADD_LABEL|CT_OPT_DEL_LABEL)) == 0)
 		return;
 
 	nfct_copy_attr(tmp, ct, ATTR_CONNLABELS);
 	ctb = (void *) nfct_get_attr(tmp, ATTR_CONNLABELS);
 
-	if (options & CT_OPT_ADD_LABEL) {
+	if (cmd->options & CT_OPT_ADD_LABEL) {
 		if (ctb == NULL) {
 			nfct_set_attr(tmp, ATTR_CONNLABELS,
 					xnfct_bitmask_clone(tmpl->label_modify));
@@ -2126,20 +2152,21 @@ static int update_cb(enum nf_conntrack_msg_type type,
 		     void *data)
 {
 	int res;
-	struct nf_conntrack *obj = data, *tmp;
+	struct ct_cmd *cmd = data;
+	struct nf_conntrack *obj = cmd->tmpl.ct, *tmp;
 
-	if (filter_nat(obj, ct) ||
+	if (filter_nat(cmd, obj, ct) ||
 	    filter_label(ct, cur_tmpl) ||
-	    filter_network(ct))
+	    filter_network(cmd, ct))
 		return NFCT_CB_CONTINUE;
 
 	if (nfct_attr_is_set(obj, ATTR_ID) && nfct_attr_is_set(ct, ATTR_ID) &&
 	    nfct_get_attr_u32(obj, ATTR_ID) != nfct_get_attr_u32(ct, ATTR_ID))
 	    	return NFCT_CB_CONTINUE;
 
-	if (options & CT_OPT_TUPLE_ORIG && !nfct_cmp(obj, ct, NFCT_CMP_ORIG))
+	if (cmd->options & CT_OPT_TUPLE_ORIG && !nfct_cmp(obj, ct, NFCT_CMP_ORIG))
 		return NFCT_CB_CONTINUE;
-	if (options & CT_OPT_TUPLE_REPL && !nfct_cmp(obj, ct, NFCT_CMP_REPL))
+	if (cmd->options & CT_OPT_TUPLE_REPL && !nfct_cmp(obj, ct, NFCT_CMP_REPL))
 		return NFCT_CB_CONTINUE;
 
 	tmp = nfct_new();
@@ -2148,9 +2175,9 @@ static int update_cb(enum nf_conntrack_msg_type type,
 
 	nfct_copy(tmp, ct, NFCT_CP_ORIG);
 	nfct_copy(tmp, obj, NFCT_CP_META);
-	copy_mark(tmp, ct, &cur_tmpl->mark);
-	copy_status(tmp, ct);
-	copy_label(tmp, ct, cur_tmpl);
+	copy_mark(cmd, tmp, ct, &cur_tmpl->mark);
+	copy_status(cmd, tmp, ct);
+	copy_label(cmd, tmp, ct, cur_tmpl);
 
 	/* do not send NFCT_Q_UPDATE if ct appears unchanged */
 	if (nfct_cmp(tmp, ct, NFCT_CMP_ALL | NFCT_CMP_MASK)) {
@@ -2613,23 +2640,23 @@ nfct_network_attr_prepare(const int family, enum ct_direction dir,
 }
 
 static void
-nfct_filter_init(const int family, const struct ct_tmpl *tmpl)
+nfct_filter_init(struct ct_cmd *cmd)
 {
-	filter_family = family;
-	if (options & CT_OPT_MASK_SRC) {
-		assert(family != AF_UNSPEC);
-		if (!(options & CT_OPT_ORIG_SRC))
+	filter_family = cmd->family;
+	if (cmd->options & CT_OPT_MASK_SRC) {
+		assert(cmd->family != AF_UNSPEC);
+		if (!(cmd->options & CT_OPT_ORIG_SRC))
 			exit_error(PARAMETER_PROBLEM,
 			           "Can't use --mask-src without --src");
-		nfct_network_attr_prepare(family, DIR_SRC, tmpl);
+		nfct_network_attr_prepare(cmd->family, DIR_SRC, &cmd->tmpl);
 	}
 
-	if (options & CT_OPT_MASK_DST) {
-		assert(family != AF_UNSPEC);
-		if (!(options & CT_OPT_ORIG_DST))
+	if (cmd->options & CT_OPT_MASK_DST) {
+		assert(cmd->family != AF_UNSPEC);
+		if (!(cmd->options & CT_OPT_ORIG_DST))
 			exit_error(PARAMETER_PROBLEM,
 			           "Can't use --mask-dst without --dst");
-		nfct_network_attr_prepare(family, DIR_DST, tmpl);
+		nfct_network_attr_prepare(cmd->family, DIR_DST, &cmd->tmpl);
 	}
 }
 
@@ -2696,16 +2723,19 @@ nfct_set_addr_only(const int opt, struct nf_conntrack *ct, union ct_address *ad,
 }
 
 static void
-nfct_set_addr_opt(const int opt, struct nf_conntrack *ct, union ct_address *ad,
-		  const int l3protonum)
+nfct_set_addr_opt(unsigned int *options,
+			 const int opt,
+			 struct nf_conntrack *ct,
+			 union ct_address *ad,
+			 const int l3protonum)
 {
-	options |= opt2type[opt];
+	*options |= opt2type[opt];
 	nfct_set_addr_only(opt, ct, ad, l3protonum);
 	nfct_set_attr_u8(ct, opt2attr[opt], l3protonum);
 }
 
 static void
-nfct_parse_addr_from_opt(const int opt, const char *arg,
+nfct_parse_addr_from_opt(unsigned int *options, const int opt, const char *arg,
 			 struct nf_conntrack *ct,
 			 struct nf_conntrack *ctmask,
 			 union ct_address *ad, int *family)
@@ -2728,7 +2758,7 @@ nfct_parse_addr_from_opt(const int opt, const char *arg,
 		           "Invalid netmask");
 	}
 
-	nfct_set_addr_opt(opt, ct, ad, l3protonum);
+	nfct_set_addr_opt(options, opt, ct, ad, l3protonum);
 
 	/* bail if we don't have a netmask to set*/
 	if (mask == -1 || !maskopt || ctmask == NULL)
@@ -2747,7 +2777,7 @@ nfct_parse_addr_from_opt(const int opt, const char *arg,
 		break;
 	}
 
-	nfct_set_addr_opt(maskopt, ctmask, ad, l3protonum);
+	nfct_set_addr_opt(options, maskopt, ctmask, ad, l3protonum);
 }
 
 static void
@@ -2768,23 +2798,13 @@ nfct_set_nat_details(const int opt, struct nf_conntrack *ct,
 
 }
 
-struct ct_cmd {
-	unsigned int	command;
-	unsigned int	cmd;
-	unsigned int	type;
-	unsigned int	event_mask;
-	int		family;
-	int		protonum;
-	size_t		socketbuffersize;
-	struct ct_tmpl	tmpl;
-};
-
 static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 {
 	unsigned int type = 0, event_mask = 0, l4flags = 0, status = 0;
 	int protonum = 0, family = AF_UNSPEC;
 	size_t socketbuffersize = 0;
 	unsigned int command = 0;
+	unsigned int options = 0;
 	struct ct_tmpl *tmpl;
 	int res = 0, partial;
 	union ct_address ad;
@@ -2851,17 +2871,17 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 		case 'd':
 		case 'r':
 		case 'q':
-			nfct_parse_addr_from_opt(c, optarg, tmpl->ct,
+			nfct_parse_addr_from_opt(&options, c, optarg, tmpl->ct,
 						 tmpl->mask, &ad, &family);
 			break;
 		case '[':
 		case ']':
-			nfct_parse_addr_from_opt(c, optarg, tmpl->exptuple,
+			nfct_parse_addr_from_opt(&options, c, optarg, tmpl->exptuple,
 						 tmpl->mask, &ad, &family);
 			break;
 		case '{':
 		case '}':
-			nfct_parse_addr_from_opt(c, optarg, tmpl->mask,
+			nfct_parse_addr_from_opt(&options, c, optarg, tmpl->mask,
 						 NULL, &ad, &family);
 			break;
 		case 'p':
@@ -2919,7 +2939,7 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 				split_address_and_port(optional_arg,
 						       &nat_address,
 						       &port_str);
-				nfct_parse_addr_from_opt(c, nat_address,
+				nfct_parse_addr_from_opt(&options, c, nat_address,
 							 tmpl->ct, NULL,
 							 &ad, &family);
 				if (c == 'j') {
@@ -3078,6 +3098,7 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 
 	ct_cmd->command = command;
 	ct_cmd->cmd = cmd;
+	ct_cmd->options = options;
 	ct_cmd->family = family;
 	ct_cmd->type = type;
 	ct_cmd->protonum = protonum;
@@ -3118,14 +3139,14 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		if (!cth)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
-		if (options & CT_COMPARISON && 
-		    options & CT_OPT_ZERO)
+		if (cmd->options & CT_COMPARISON &&
+			cmd->options & CT_OPT_ZERO)
 			exit_error(PARAMETER_PROBLEM, "Can't use -z with "
 						      "filtering parameters");
 
-		nfct_filter_init(cmd->family, &cmd->tmpl);
+		nfct_filter_init(cmd);
 
-		nfct_callback_register(cth, NFCT_T_ALL, dump_cb, cmd->tmpl.ct);
+		nfct_callback_register(cth, NFCT_T_ALL, dump_cb, cmd);
 
 		filter_dump = nfct_filter_dump_create();
 		if (filter_dump == NULL)
@@ -3140,7 +3161,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 					     NFCT_FILTER_DUMP_L3NUM,
 					     cmd->family);
 
-		if (options & CT_OPT_ZERO)
+		if (cmd->options & CT_OPT_ZERO)
 			res = nfct_query(cth, NFCT_Q_DUMP_FILTER_RESET,
 					filter_dump);
 		else
@@ -3172,15 +3193,15 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		break;
 
 	case CT_CREATE:
-		if ((options & CT_OPT_ORIG) && !(options & CT_OPT_REPL))
+		if ((cmd->options & CT_OPT_ORIG) && !(cmd->options & CT_OPT_REPL))
 			nfct_setobjopt(cmd->tmpl.ct, NFCT_SOPT_SETUP_REPLY);
-		else if (!(options & CT_OPT_ORIG) && (options & CT_OPT_REPL))
+		else if (!(cmd->options & CT_OPT_ORIG) && (cmd->options & CT_OPT_REPL))
 			nfct_setobjopt(cmd->tmpl.ct, NFCT_SOPT_SETUP_ORIGINAL);
 
-		if (options & CT_OPT_MARK)
+		if (cmd->options & CT_OPT_MARK)
 			nfct_set_attr_u32(cmd->tmpl.ct, ATTR_MARK, cmd->tmpl.mark.value);
 
-		if (options & CT_OPT_ADD_LABEL)
+		if (cmd->options & CT_OPT_ADD_LABEL)
 			nfct_set_attr(cmd->tmpl.ct, ATTR_CONNLABELS,
 					xnfct_bitmask_clone(cmd->tmpl.label_modify));
 
@@ -3214,9 +3235,9 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		if (!cth || !ith)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
-		nfct_filter_init(cmd->family, &cmd->tmpl);
+		nfct_filter_init(cmd);
 
-		nfct_callback_register(cth, NFCT_T_ALL, update_cb, cmd->tmpl.ct);
+		nfct_callback_register(cth, NFCT_T_ALL, update_cb, cmd);
 
 		res = nfct_query(cth, NFCT_Q_DUMP, &cmd->family);
 		nfct_close(ith);
@@ -3229,9 +3250,9 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		if (!cth || !ith)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
-		nfct_filter_init(cmd->family, &cmd->tmpl);
+		nfct_filter_init(cmd);
 
-		nfct_callback_register(cth, NFCT_T_ALL, delete_cb, cmd->tmpl.ct);
+		nfct_callback_register(cth, NFCT_T_ALL, delete_cb, cmd);
 
 		filter_dump = nfct_filter_dump_create();
 		if (filter_dump == NULL)
@@ -3270,7 +3291,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		if (!cth)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
-		nfct_callback_register(cth, NFCT_T_ALL, dump_cb, cmd->tmpl.ct);
+		nfct_callback_register(cth, NFCT_T_ALL, dump_cb, cmd);
 		res = nfct_query(cth, NFCT_Q_GET, cmd->tmpl.ct);
 		nfct_close(cth);
 		break;
@@ -3308,7 +3329,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		break;
 
 	case CT_EVENT:
-		if (options & CT_OPT_EVENT_MASK) {
+		if (cmd->options & CT_OPT_EVENT_MASK) {
 			unsigned int nl_events = 0;
 
 			if (cmd->event_mask & CT_EVENT_F_NEW)
@@ -3328,7 +3349,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		if (res < 0)
 			exit_error(OTHER_PROBLEM, "Can't open netlink socket");
 
-		if (options & CT_OPT_BUFFERSIZE) {
+		if (cmd->options & CT_OPT_BUFFERSIZE) {
 			size_t socketbuffersize = cmd->socketbuffersize;
 
 			socklen_t socklen = sizeof(socketbuffersize);
@@ -3350,7 +3371,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 					socketbuffersize);
 		}
 
-		nfct_filter_init(cmd->family, &cmd->tmpl);
+		nfct_filter_init(cmd);
 
 		signal(SIGINT, event_sighandler);
 		signal(SIGTERM, event_sighandler);
@@ -3375,13 +3396,13 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 					   strerror(errno));
 				break;
 			}
-			res = mnl_cb_run(buf, res, 0, 0, event_cb, cmd->tmpl.ct);
+			res = mnl_cb_run(buf, res, 0, 0, event_cb, cmd);
 		}
 		mnl_socket_close(sock.mnl);
 		break;
 
 	case EXP_EVENT:
-		if (options & CT_OPT_EVENT_MASK) {
+		if (cmd->options & CT_OPT_EVENT_MASK) {
 			unsigned int nl_events = 0;
 
 			if (cmd->event_mask & CT_EVENT_F_NEW)
@@ -3496,7 +3517,7 @@ try_proc:
 		break;
 	case CT_HELP:
 		usage(progname);
-		if (options & CT_OPT_PROTO)
+		if (cmd->options & CT_OPT_PROTO)
 			extension_help(h, cmd->protonum);
 		break;
 	default:
-- 
2.25.1

[PATCH v3 3/8] conntrack: per-command entries counters

[Mikhail Sennikovsky]

As a multicommand support preparation entry counters need
to be made per-command as well, e.g. for the case -D and -I
can be executed in a single batch, and we want to have separate
counters for them.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 src/conntrack.c | 117 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 81 insertions(+), 36 deletions(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index a090542..4783825 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -1663,8 +1663,48 @@ nfct_filter(const struct ct_cmd *cmd,
 	return 0;
 }
 
-static int counter;
 static int dump_xml_header_done = 1;
+static unsigned int cmd_executed = 0;
+static const unsigned int cmd_no_entries_ok = 0
+						| CT_LIST
+						| EXP_LIST
+						;
+static unsigned int cmd_counters[NUMBER_OF_CMD];
+
+static int
+print_cmd_counters(void)
+{
+	int i, ret = EXIT_FAILURE;
+
+	if (!cmd_executed)
+		return EXIT_SUCCESS;
+
+	for (i = 0;
+		i < (int)(sizeof(cmd_counters) / sizeof(cmd_counters[0]));
+		++i) {
+		if (cmd_executed & 1 << i) {
+			if (exit_msg[i][0]) {
+				fprintf(stderr, "%s v%s (conntrack-tools): ",
+							PROGNAME, VERSION);
+				fprintf(stderr, exit_msg[i], cmd_counters[i]);
+			}
+			/*
+			 * If there is at least one command which is supposed
+			 * to return success, EXIT_SUCCESS is returned.
+			 * I.e. for the --load-file case this would ensure that
+			 * e.g. -D followed by a series of -I's
+			 * would return success in case there are no entries
+			 * to be deleted with the -D command preceding the -I's
+			 */
+			if (!exit_msg[i][0]
+					|| cmd_counters[i] != 0
+					|| cmd_no_entries_ok & 1 << i)
+				ret &= EXIT_SUCCESS;
+		}
+	}
+	return ret;
+}
+
 
 static void __attribute__((noreturn))
 event_sighandler(int s)
@@ -1674,8 +1714,7 @@ event_sighandler(int s)
 		fflush(stdout);
 	}
 
-	fprintf(stderr, "%s v%s (conntrack-tools): ", PROGNAME, VERSION);
-	fprintf(stderr, "%d flow events have been shown.\n", counter);
+	print_cmd_counters();
 	mnl_socket_close(sock.mnl);
 	exit(0);
 }
@@ -1688,8 +1727,7 @@ exp_event_sighandler(int s)
 		fflush(stdout);
 	}
 
-	fprintf(stderr, "%s v%s (conntrack-tools): ", PROGNAME, VERSION);
-	fprintf(stderr, "%d expectation events have been shown.\n", counter);
+	print_cmd_counters();
 	nfct_close(cth);
 	exit(0);
 }
@@ -1938,7 +1976,7 @@ done:
 	}
 	fflush(stdout);
 
-	counter++;
+	cmd_counters[cmd->cmd]++;
 out:
 	nfct_destroy(ct);
 	return MNL_CB_OK;
@@ -1981,7 +2019,7 @@ static int dump_cb(enum nf_conntrack_msg_type type,
 done:
 	printf("%s\n", buf);
 
-	counter++;
+	cmd_counters[cmd->cmd]++;
 
 	return NFCT_CB_CONTINUE;
 }
@@ -2022,7 +2060,7 @@ static int delete_cb(enum nf_conntrack_msg_type type,
 done:
 	printf("%s\n", buf);
 
-	counter++;
+	cmd_counters[cmd->cmd]++;
 
 	return NFCT_CB_CONTINUE;
 }
@@ -2207,7 +2245,7 @@ static int update_cb(enum nf_conntrack_msg_type type,
 	nfct_destroy(tmp);
 	nfct_callback_unregister(ith);
 
-	counter++;
+	cmd_counters[cmd->cmd]++;
 
 	return NFCT_CB_CONTINUE;
 }
@@ -2217,6 +2255,7 @@ static int dump_exp_cb(enum nf_conntrack_msg_type type,
 		      void *data)
 {
 	char buf[1024];
+	struct ct_cmd *cmd = data;
 	unsigned int op_type = NFCT_O_DEFAULT;
 	unsigned int op_flags = 0;
 
@@ -2239,7 +2278,7 @@ static int dump_exp_cb(enum nf_conntrack_msg_type type,
 
 	nfexp_snprintf(buf,sizeof(buf), exp, NFCT_T_UNKNOWN, op_type, op_flags);
 	printf("%s\n", buf);
-	counter++;
+	cmd_counters[cmd->cmd]++;
 
 	return NFCT_CB_CONTINUE;
 }
@@ -2248,6 +2287,7 @@ static int event_exp_cb(enum nf_conntrack_msg_type type,
 			struct nf_expect *exp, void *data)
 {
 	char buf[1024];
+	struct ct_cmd *cmd = data;
 	unsigned int op_type = NFCT_O_DEFAULT;
 	unsigned int op_flags = 0;
 
@@ -2271,7 +2311,7 @@ static int event_exp_cb(enum nf_conntrack_msg_type type,
 	nfexp_snprintf(buf,sizeof(buf), exp, type, op_type, op_flags);
 	printf("%s\n", buf);
 	fflush(stdout);
-	counter++;
+	cmd_counters[cmd->cmd]++;
 
 	return NFCT_CB_CONTINUE;
 }
@@ -2280,7 +2320,9 @@ static int count_exp_cb(enum nf_conntrack_msg_type type,
 			struct nf_expect *exp,
 			void *data)
 {
-	counter++;
+	struct ct_cmd *cmd = data;
+
+	cmd_counters[cmd->cmd]++;
 	return NFCT_CB_CONTINUE;
 }
 
@@ -2396,7 +2438,9 @@ static void nfct_mnl_socket_close(void)
 }
 
 static int
-nfct_mnl_dump(uint16_t subsys, uint16_t type, mnl_cb_t cb, uint8_t family)
+nfct_mnl_dump(uint16_t subsys, uint16_t type,
+		      mnl_cb_t cb, void *data,
+		      uint8_t family)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nlmsghdr *nlh;
@@ -2411,7 +2455,7 @@ nfct_mnl_dump(uint16_t subsys, uint16_t type, mnl_cb_t cb, uint8_t family)
 	res = mnl_socket_recvfrom(sock.mnl, buf, sizeof(buf));
 	while (res > 0) {
 		res = mnl_cb_run(buf, res, nlh->nlmsg_seq, sock.portid,
-					 cb, NULL);
+					 cb, data);
 		if (res <= MNL_CB_STOP)
 			break;
 
@@ -2581,6 +2625,7 @@ static int mnl_nfct_dump_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nf_conntrack *ct;
 	char buf[4096];
+	struct ct_cmd *cmd = data;
 
 	ct = nfct_new();
 	if (ct == NULL)
@@ -2593,7 +2638,7 @@ static int mnl_nfct_dump_cb(const struct nlmsghdr *nlh, void *data)
 
 	nfct_destroy(ct);
 
-	counter++;
+	cmd_counters[cmd->cmd]++;
 
 	return MNL_CB_OK;
 }
@@ -3051,6 +3096,10 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 		}
 	}
 
+	if (!command)
+		/* invalid args */
+		exit_error(PARAMETER_PROBLEM, "invalid cmd line syntax");
+
 	/* default family only for the following commands */
 	if (family == AF_UNSPEC) {
 		switch (command) {
@@ -3106,11 +3155,14 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 	ct_cmd->socketbuffersize = socketbuffersize;
 }
 
-static int do_command_ct(const char *progname, struct ct_cmd *cmd)
+static void do_command_ct(const char *progname, struct ct_cmd *cmd)
 {
 	struct nfct_filter_dump *filter_dump;
 	int res = 0;
 
+	assert(cmd->cmd < sizeof(cmd_counters) / sizeof(cmd_counters[0]));
+	cmd_executed |= cmd->command;
+
 	switch(cmd->command) {
 	case CT_LIST:
 		if (cmd->type == CT_TABLE_DYING) {
@@ -3119,7 +3171,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 
 			res = nfct_mnl_dump(NFNL_SUBSYS_CTNETLINK,
 					    IPCTNL_MSG_CT_GET_DYING,
-					    mnl_nfct_dump_cb, cmd->family);
+					    mnl_nfct_dump_cb, cmd, cmd->family);
 
 			nfct_mnl_socket_close();
 			break;
@@ -3129,7 +3181,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 
 			res = nfct_mnl_dump(NFNL_SUBSYS_CTNETLINK,
 					    IPCTNL_MSG_CT_GET_UNCONFIRMED,
-					    mnl_nfct_dump_cb, cmd->family);
+					    mnl_nfct_dump_cb, cmd, cmd->family);
 
 			nfct_mnl_socket_close();
 			break;
@@ -3182,7 +3234,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		if (!cth)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
-		nfexp_callback_register(cth, NFCT_T_ALL, dump_exp_cb, NULL);
+		nfexp_callback_register(cth, NFCT_T_ALL, dump_exp_cb, cmd);
 		res = nfexp_query(cth, NFCT_Q_DUMP, &cmd->family);
 		nfct_close(cth);
 
@@ -3211,7 +3263,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 
 		res = nfct_query(cth, NFCT_Q_CREATE, cmd->tmpl.ct);
 		if (res != -1)
-			counter++;
+			cmd_counters[cmd->cmd]++;
 		nfct_close(cth);
 		break;
 
@@ -3303,7 +3355,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 		if (!cth)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
-		nfexp_callback_register(cth, NFCT_T_ALL, dump_exp_cb, NULL);
+		nfexp_callback_register(cth, NFCT_T_ALL, dump_exp_cb, cmd);
 		res = nfexp_query(cth, NFCT_Q_GET, cmd->tmpl.exp);
 		nfct_close(cth);
 		break;
@@ -3424,7 +3476,7 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 		signal(SIGINT, exp_event_sighandler);
 		signal(SIGTERM, exp_event_sighandler);
-		nfexp_callback_register(cth, NFCT_T_ALL, event_exp_cb, NULL);
+		nfexp_callback_register(cth, NFCT_T_ALL, event_exp_cb, cmd);
 		res = nfexp_catch(cth);
 		nfct_close(cth);
 		break;
@@ -3468,10 +3520,10 @@ try_proc_count:
 		if (!cth)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
-		nfexp_callback_register(cth, NFCT_T_ALL, count_exp_cb, NULL);
+		nfexp_callback_register(cth, NFCT_T_ALL, count_exp_cb, cmd);
 		res = nfexp_query(cth, NFCT_Q_DUMP, &cmd->family);
 		nfct_close(cth);
-		printf("%d\n", counter);
+		printf("%d\n", cmd_counters[cmd->cmd]);
 		break;
 	case CT_STATS:
 		/* If we fail with netlink, fall back to /proc to ensure
@@ -3482,7 +3534,7 @@ try_proc_count:
 
 		res = nfct_mnl_dump(NFNL_SUBSYS_CTNETLINK,
 				    IPCTNL_MSG_CT_GET_STATS_CPU,
-				    nfct_stats_cb, AF_UNSPEC);
+				    nfct_stats_cb, NULL, AF_UNSPEC);
 
 		nfct_mnl_socket_close();
 
@@ -3501,7 +3553,7 @@ try_proc_count:
 
 		res = nfct_mnl_dump(NFNL_SUBSYS_CTNETLINK_EXP,
 				    IPCTNL_MSG_EXP_GET_STATS_CPU,
-				    nfexp_stats_cb, AF_UNSPEC);
+				    nfexp_stats_cb, NULL, AF_UNSPEC);
 
 		nfct_mnl_socket_close();
 
@@ -3533,15 +3585,6 @@ try_proc:
 	free_options();
 	if (labelmap)
 		nfct_labelmap_destroy(labelmap);
-
-	if (cmd->command && exit_msg[cmd->cmd][0]) {
-		fprintf(stderr, "%s v%s (conntrack-tools): ",PROGNAME,VERSION);
-		fprintf(stderr, exit_msg[cmd->cmd], counter);
-		if (counter == 0 && !(cmd->command & (CT_LIST | EXP_LIST)))
-			return EXIT_FAILURE;
-	}
-
-	return EXIT_SUCCESS;
 }
 
 int main(int argc, char *argv[])
@@ -3560,5 +3603,7 @@ int main(int argc, char *argv[])
 
 	do_parse(cmd, argc, argv);
 
-	return do_command_ct(argv[0], cmd);
+	do_command_ct(argv[0], cmd);
+
+	return print_cmd_counters();
 }
-- 
2.25.1

Re: [PATCH v3 3/8] conntrack: per-command entries counters

[Pablo Neira Ayuso]

Hi Mikhail,

On Fri, Jan 29, 2021 at 10:24:47PM +0100, Mikhail Sennikovsky wrote:
> As a multicommand support preparation entry counters need
> to be made per-command as well, e.g. for the case -D and -I
> can be executed in a single batch, and we want to have separate
> counters for them.

How do you plan to use the counters? -F provides no stats though.

It should be possible to do some pretty print for stats.

There is also the -I and -D cases, which might fail. In that case,
this should probably stop processing on failure?

I sent another round of patches based on your that gets things closer
to the batch support.

Re: [PATCH v3 3/8] conntrack: per-command entries counters

[Mikhail Sennikovsky]

Hi Pablo,

On Mon, 15 Mar 2021 at 18:12, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi Mikhail,
>
> On Fri, Jan 29, 2021 at 10:24:47PM +0100, Mikhail Sennikovsky wrote:
> > As a multicommand support preparation entry counters need
> > to be made per-command as well, e.g. for the case -D and -I
> > can be executed in a single batch, and we want to have separate
> > counters for them.
>
> How do you plan to use the counters? -F provides no stats though.
Those counters are used to print the number of affected entries for
each command "type" executed.
I.e. prior to the "--load-file" support it was only possible to have a
single command for each conntrack tool invocation,
so a global counter used to print the stats message like
"conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created."
was sufficient.

With the --load-file/-R command support it is possible to have
multiple command types
in a single conntrack tool invocation, e.g. both -I and -D commands as
in example below.

echo "-D -w 123
-I -w 123 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state
LISTEN -u SEEN_REPLY -t 50 " | conntrack -R -

The per-command counters functionality added here makes it possible to print
those stats info for each command "type" separately.
So as a result of the above command something the following would be printed:

conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created.
conntrack v1.4.6 (conntrack-tools): 3 flow entries have been deleted.

Following your request to make the changes more granular, I moved this
functionality
to this separate "preparation" commit.

>
> It should be possible to do some pretty print for stats.
>
> There is also the -I and -D cases, which might fail. In that case,
> this should probably stop processing on failure?
Are you talking about error handling processing ct_cmd entries?
The way it is done currently is that each failure would result in
exit_error to happen.
This logic actually stays unchanged.

>
> I sent another round of patches based on your that gets things closer
> to the batch support.
Thanks, I'll have a look into them.

Regards,
Mikhail

Re: [PATCH v3 3/8] conntrack: per-command entries counters

[Pablo Neira Ayuso]

Hi Mikhail,

On Wed, Mar 17, 2021 at 07:20:55PM +0100, Mikhail Sennikovsky wrote:
> Hi Pablo,
> 
> On Mon, 15 Mar 2021 at 18:12, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > Hi Mikhail,
> >
> > On Fri, Jan 29, 2021 at 10:24:47PM +0100, Mikhail Sennikovsky wrote:
> > > As a multicommand support preparation entry counters need
> > > to be made per-command as well, e.g. for the case -D and -I
> > > can be executed in a single batch, and we want to have separate
> > > counters for them.
> >
> > How do you plan to use the counters? -F provides no stats though.
> Those counters are used to print the number of affected entries for
> each command "type" executed.
> I.e. prior to the "--load-file" support it was only possible to have a
> single command for each conntrack tool invocation,
> so a global counter used to print the stats message like
> "conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created."
> was sufficient.
> 
> With the --load-file/-R command support it is possible to have
> multiple command types
> in a single conntrack tool invocation, e.g. both -I and -D commands as
> in example below.
> 
> echo "-D -w 123
> -I -w 123 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state
> LISTEN -u SEEN_REPLY -t 50 " | conntrack -R -
> 
> The per-command counters functionality added here makes it possible to print
> those stats info for each command "type" separately.
> So as a result of the above command something the following would be printed:
> 
> conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created.
> conntrack v1.4.6 (conntrack-tools): 3 flow entries have been deleted.
> 
> Following your request to make the changes more granular, I moved this
> functionality to this separate "preparation" commit.
>
> > It should be possible to do some pretty print for stats.

I think it should be possible to do some pretty print, something like:

        conntrack v1.4.6 (conntrack-tools)
        Line 1-3: 3 flow entries have been created.
        Line 4: 1 flow entries have been deleted.
        ...

One possibility is that we skip the pretty print by now, ie. you
rebase your patch on top of conntrack-tools, get it merged upstream.
Then incrementally we look at adding the pretty print for stats.

> > There is also the -I and -D cases, which might fail. In that case,
> > this should probably stop processing on failure?
>
> Are you talking about error handling processing ct_cmd entries?
> The way it is done currently is that each failure would result in
> exit_error to happen.
> This logic actually stays unchanged.

So the batch processing stops on the first error, right?

> > I sent another round of patches based on your that gets things closer
> > to the batch support.
>
> Thanks, I'll have a look into them.

I have pushed them out, any mistake please let me know I'll fix it.

Thanks.

Re: [PATCH v3 3/8] conntrack: per-command entries counters

[Mikhail Sennikovsky]

Hi Pablo,

On Wed, 24 Mar 2021 at 12:24, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi Mikhail,
>
> On Wed, Mar 17, 2021 at 07:20:55PM +0100, Mikhail Sennikovsky wrote:
> > Hi Pablo,
> >
> > On Mon, 15 Mar 2021 at 18:12, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >
> > > Hi Mikhail,
> > >
> > > On Fri, Jan 29, 2021 at 10:24:47PM +0100, Mikhail Sennikovsky wrote:
> > > > As a multicommand support preparation entry counters need
> > > > to be made per-command as well, e.g. for the case -D and -I
> > > > can be executed in a single batch, and we want to have separate
> > > > counters for them.
> > >
> > > How do you plan to use the counters? -F provides no stats though.
> > Those counters are used to print the number of affected entries for
> > each command "type" executed.
> > I.e. prior to the "--load-file" support it was only possible to have a
> > single command for each conntrack tool invocation,
> > so a global counter used to print the stats message like
> > "conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created."
> > was sufficient.
> >
> > With the --load-file/-R command support it is possible to have
> > multiple command types
> > in a single conntrack tool invocation, e.g. both -I and -D commands as
> > in example below.
> >
> > echo "-D -w 123
> > -I -w 123 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state
> > LISTEN -u SEEN_REPLY -t 50 " | conntrack -R -
> >
> > The per-command counters functionality added here makes it possible to print
> > those stats info for each command "type" separately.
> > So as a result of the above command something the following would be printed:
> >
> > conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created.
> > conntrack v1.4.6 (conntrack-tools): 3 flow entries have been deleted.
> >
> > Following your request to make the changes more granular, I moved this
> > functionality to this separate "preparation" commit.
> >
> > > It should be possible to do some pretty print for stats.
>
> I think it should be possible to do some pretty print, something like:
>
>         conntrack v1.4.6 (conntrack-tools)
>         Line 1-3: 3 flow entries have been created.
>         Line 4: 1 flow entries have been deleted.
>         ...
>
> One possibility is that we skip the pretty print by now, ie. you
> rebase your patch on top of conntrack-tools, get it merged upstream.
> Then incrementally we look at adding the pretty print for stats.
Agreed.

>
> > > There is also the -I and -D cases, which might fail. In that case,
> > > this should probably stop processing on failure?
> >
> > Are you talking about error handling processing ct_cmd entries?
> > The way it is done currently is that each failure would result in
> > exit_error to happen.
> > This logic actually stays unchanged.
>
> So the batch processing stops on the first error, right?
Yes. As I mentioned, this is the easiest thing to do currently, as it
does not require any code changes.
LAter on we could add an option/switch to proceed on failure if
everyone finds it useful.

>
> > > I sent another round of patches based on your that gets things closer
> > > to the batch support.
> >
> > Thanks, I'll have a look into them.
>
> I have pushed them out, any mistake please let me know I'll fix it.
Great!

>
> Thanks.
Thanks as well)
Mikhail

[PATCH v3 4/8] conntrack: introduce ct_cmd_list

[Mikhail Sennikovsky]

As a multicommand support preparation, add support for the
ct_cmd_list, which represents a list of ct_cmd elements.
Currently only a single entry generated from the command line
arguments is created.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 src/conntrack.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index 4783825..1719ca9 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -598,6 +598,19 @@ static unsigned int addr_valid_flags[ADDR_VALID_FLAGS_MAX] = {
 	CT_OPT_REPL_SRC | CT_OPT_REPL_DST,
 };
 
+#define CT_COMMANDS_LOAD_FILE_ALLOWED ( 0 \
+						| CT_CREATE       \
+						| CT_UPDATE_BIT   \
+						| CT_DELETE       \
+						| CT_FLUSH        \
+						| EXP_CREATE      \
+						| EXP_DELETE      \
+						| EXP_FLUSH       \
+						)
+
+static unsigned int cmd_allowed = ~0;
+
+
 static LIST_HEAD(proto_list);
 
 static struct nfct_labelmap *labelmap;
@@ -1250,6 +1263,9 @@ add_command(unsigned int *cmd, const int newcmd)
 {
 	if (*cmd)
 		exit_error(PARAMETER_PROBLEM, "Invalid commands combination");
+	if (!(cmd_allowed & newcmd))
+		exit_error(PARAMETER_PROBLEM,
+				"Command can not be used in the current mode");
 	*cmd |= newcmd;
 }
 
@@ -1472,6 +1488,10 @@ struct ct_cmd {
 	struct ct_tmpl	tmpl;
 };
 
+struct ct_cmd_list {
+	struct list_head list;
+};
+
 static int
 filter_label(const struct nf_conntrack *ct, const struct ct_tmpl *tmpl)
 {
@@ -3155,6 +3175,19 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
 	ct_cmd->socketbuffersize = socketbuffersize;
 }
 
+static struct ct_cmd *ct_cmd_create(int argc, char *argv[])
+{
+	struct ct_cmd *ct_cmd;
+
+	ct_cmd = calloc(1, sizeof(*ct_cmd));
+	if (!ct_cmd)
+		exit_error(OTHER_PROBLEM, "cmd alloc failed!!");
+
+	do_parse(ct_cmd, argc, argv);
+
+	return ct_cmd;
+}
+
 static void do_command_ct(const char *progname, struct ct_cmd *cmd)
 {
 	struct nfct_filter_dump *filter_dump;
@@ -3587,9 +3620,37 @@ try_proc:
 		nfct_labelmap_destroy(labelmap);
 }
 
+static void ct_cmd_list_init(struct ct_cmd_list *list)
+{
+	memset(list, 0, sizeof(*list));
+	INIT_LIST_HEAD(&list->list);
+}
+
+static void ct_cmd_list_add(struct ct_cmd_list *list, struct ct_cmd *cmd)
+{
+	list_add_tail(&cmd->list_entry, &list->list);
+}
+
+static void ct_cmd_list_apply(struct ct_cmd_list *list, const char *progname)
+{
+	struct ct_cmd *cmd, *tmp;
+
+	list_for_each_entry_safe(cmd, tmp, &list->list, list_entry) {
+		list_del(&cmd->list_entry);
+		do_command_ct(progname, cmd);
+		free(cmd);
+	}
+}
+
+static void ct_cmd_list_parse_argv(struct ct_cmd_list *list,
+		int argc, char *argv[])
+{
+	ct_cmd_list_add(list, ct_cmd_create(argc, argv));
+}
+
 int main(int argc, char *argv[])
 {
-	struct ct_cmd _cmd = {}, *cmd = &_cmd;
+	struct ct_cmd_list list;
 
 	register_tcp();
 	register_udp();
@@ -3601,9 +3662,11 @@ int main(int argc, char *argv[])
 	register_gre();
 	register_unknown();
 
-	do_parse(cmd, argc, argv);
+	ct_cmd_list_init(&list);
+
+	ct_cmd_list_parse_argv(&list, argc, argv);
 
-	do_command_ct(argv[0], cmd);
+	ct_cmd_list_apply(&list, argv[0]);
 
 	return print_cmd_counters();
 }
-- 
2.25.1

Re: [PATCH v3 4/8] conntrack: introduce ct_cmd_list

[Pablo Neira Ayuso]

On Fri, Jan 29, 2021 at 10:24:48PM +0100, Mikhail Sennikovsky wrote:
> As a multicommand support preparation, add support for the
> ct_cmd_list, which represents a list of ct_cmd elements.
> Currently only a single entry generated from the command line
> arguments is created.
> 
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
> ---
>  src/conntrack.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 66 insertions(+), 3 deletions(-)
> 
> diff --git a/src/conntrack.c b/src/conntrack.c
> index 4783825..1719ca9 100644
> --- a/src/conntrack.c
> +++ b/src/conntrack.c
> @@ -598,6 +598,19 @@ static unsigned int addr_valid_flags[ADDR_VALID_FLAGS_MAX] = {
>  	CT_OPT_REPL_SRC | CT_OPT_REPL_DST,
>  };
>  
> +#define CT_COMMANDS_LOAD_FILE_ALLOWED ( 0 \
> +						| CT_CREATE       \
> +						| CT_UPDATE_BIT   \

This should CT_UPDATE.

> +						| CT_DELETE       \
> +						| CT_FLUSH        \
> +						| EXP_CREATE      \
> +						| EXP_DELETE      \
> +						| EXP_FLUSH       \

Do you need expectations too? The expectation support for the
conntrack command line tool is limited IIRC.

I would probably collapse patch 4/8 and 5/8, it should be easy to
review, it all basically new code to support for the batching mode.

Re: [PATCH v3 4/8] conntrack: introduce ct_cmd_list

[Mikhail Sennikovsky]

Hi Pablo,

On Mon, 15 Mar 2021 at 18:17, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Fri, Jan 29, 2021 at 10:24:48PM +0100, Mikhail Sennikovsky wrote:
> > As a multicommand support preparation, add support for the
> > ct_cmd_list, which represents a list of ct_cmd elements.
> > Currently only a single entry generated from the command line
> > arguments is created.
> >
> > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
> > ---
> >  src/conntrack.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 66 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/conntrack.c b/src/conntrack.c
> > index 4783825..1719ca9 100644
> > --- a/src/conntrack.c
> > +++ b/src/conntrack.c
> > @@ -598,6 +598,19 @@ static unsigned int addr_valid_flags[ADDR_VALID_FLAGS_MAX] = {
> >       CT_OPT_REPL_SRC | CT_OPT_REPL_DST,
> >  };
> >
> > +#define CT_COMMANDS_LOAD_FILE_ALLOWED ( 0 \
> > +                                             | CT_CREATE       \
> > +                                             | CT_UPDATE_BIT   \
>
> This should CT_UPDATE.
>
> > +                                             | CT_DELETE       \
> > +                                             | CT_FLUSH        \
> > +                                             | EXP_CREATE      \
> > +                                             | EXP_DELETE      \
> > +                                             | EXP_FLUSH       \
>
> Do you need expectations too? The expectation support for the
> conntrack command line tool is limited IIRC.
Actually I do not need expectations, and I agree they can be removed for now.

> I would probably collapse patch 4/8 and 5/8, it should be easy to
> review, it all basically new code to support for the batching mode.
I could squash the 3/ 4/ and 5/8 for sure.
Again the goal was to make the changes more granular and easier for
review, since all these parts are independent.
So the 3/, 4/ and 5/8 are kind of "preparation" commits for the "real"
--load-file functionality.
If you say it's better to squash them, I can surely do it.

Regards,
Mikhail

Re: [PATCH v3 4/8] conntrack: introduce ct_cmd_list

[Pablo Neira Ayuso]

On Wed, Mar 17, 2021 at 07:28:59PM +0100, Mikhail Sennikovsky wrote:
[...]
> > > +                                             | CT_DELETE       \
> > > +                                             | CT_FLUSH        \
> > > +                                             | EXP_CREATE      \
> > > +                                             | EXP_DELETE      \
> > > +                                             | EXP_FLUSH       \
> >
> > Do you need expectations too? The expectation support for the
> > conntrack command line tool is limited IIRC.
>
> Actually I do not need expectations, and I agree they can be removed for now.

Yes, let's skip expectations until someone has a usecase for this.

Thanks.

[PATCH v3 5/8] conntrack: accept commands from file

[Mikhail Sennikovsky]

This commit implements the --load-file option which
allows processing conntrack commands stored in file.
Most often this would be used as a counter-part for the
-o save option, which outputs conntrack entries
in the format of the conntrack tool options.
This could be useful when one needs to add/update/delete a large
set of ct entries with a single conntrack tool invocation.

Expected syntax is "conntrack --load-file file".
If "-" is given as a file name, stdin is used.
No other commands or options are allowed to be specified
in conjunction with the --load-file command.
It is however possible to specify multiple --load-file file pairs.

Example:
Copy all entries from ct zone 11 to ct zone 12:

conntrack -L -w 11 -o save | sed "s/-w 11/-w 12/g" | \
        conntrack --load-file -

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 src/conntrack.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 162 insertions(+), 1 deletion(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index 1719ca9..97357b0 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -3648,6 +3648,167 @@ static void ct_cmd_list_parse_argv(struct ct_cmd_list *list,
 	ct_cmd_list_add(list, ct_cmd_create(argc, argv));
 }
 
+#define MAX_ARGC	255
+struct argv_store {
+	int argc;
+	char *argv[MAX_ARGC];
+	int argvattr[MAX_ARGC];
+};
+
+/* function adding one argument to store, updating argc
+ * returns if argument added, does not return otherwise */
+static void add_argv(struct argv_store *store, const char *what, int quoted)
+{
+	if (store->argc + 1 >= MAX_ARGC)
+		exit_error(PARAMETER_PROBLEM,
+			      "Parser cannot handle more arguments\n");
+	if (!what)
+		exit_error(PARAMETER_PROBLEM,
+			      "Trying to store NULL argument\n");
+
+	store->argv[store->argc] = strdup(what);
+	store->argvattr[store->argc] = quoted;
+	store->argv[++store->argc] = NULL;
+}
+
+static void free_argv(struct argv_store *store)
+{
+	while (store->argc) {
+		store->argc--;
+		free(store->argv[store->argc]);
+		store->argvattr[store->argc] = 0;
+	}
+}
+
+struct ct_param_buf {
+	char	buffer[1024];
+	int 	len;
+};
+
+static void add_param(struct ct_param_buf *param, const char *curchar)
+{
+	param->buffer[param->len++] = *curchar;
+	if (param->len >= (int)sizeof(param->buffer))
+		exit_error(PARAMETER_PROBLEM, "Parameter too long!");
+}
+
+static void add_param_to_argv(struct argv_store *store, char *parsestart)
+{
+	int quote_open = 0, escaped = 0, quoted = 0;
+	struct ct_param_buf param = {};
+	char *curchar;
+
+	/* After fighting with strtok enough, here's now
+	 * a 'real' parser. According to Rusty I'm now no
+	 * longer a real hacker, but I can live with that */
+
+	for (curchar = parsestart; *curchar; curchar++) {
+		if (quote_open) {
+			if (escaped) {
+				add_param(&param, curchar);
+				escaped = 0;
+				continue;
+			} else if (*curchar == '\\') {
+				escaped = 1;
+				continue;
+			} else if (*curchar == '"') {
+				quote_open = 0;
+			} else {
+				add_param(&param, curchar);
+				continue;
+			}
+		} else {
+			if (*curchar == '"') {
+				quote_open = 1;
+				quoted = 1;
+				continue;
+			}
+		}
+
+		switch (*curchar) {
+		case '"':
+			break;
+		case ' ':
+		case '\t':
+		case '\n':
+			if (!param.len) {
+				/* two spaces? */
+				continue;
+			}
+			break;
+		default:
+			/* regular character, copy to buffer */
+			add_param(&param, curchar);
+			continue;
+		}
+
+		param.buffer[param.len] = '\0';
+		add_argv(store, param.buffer, quoted);
+		param.len = 0;
+		quoted = 0;
+	}
+	if (param.len) {
+		param.buffer[param.len] = '\0';
+		add_argv(store, param.buffer, 0);
+	}
+}
+
+static void ct_cmd_list_parse_line(struct ct_cmd_list *list,
+		const char *progname, char *buffer)
+{
+	struct argv_store store = {};
+
+	/* skip prepended tabs and spaces */
+	for (; *buffer == ' ' || *buffer == '\t'; buffer++);
+
+	if (buffer[0] == '\n'
+			|| buffer[0] == '#')
+		return;
+
+	add_argv(&store, progname, false);
+
+	add_param_to_argv(&store, buffer);
+
+	ct_cmd_list_parse_argv(list, store.argc, store.argv);
+
+	free_argv(&store);
+}
+
+static void ct_cmd_list_parse_file(struct ct_cmd_list *list,
+			   const char *progname,
+			   const char *file_name)
+{
+	char buffer[10240] = {};
+	FILE *file;
+
+	if (!strcmp(file_name, "-"))
+		file_name = "/dev/stdin";
+
+	file = fopen(file_name, "r");
+	if (!file)
+		exit_error(PARAMETER_PROBLEM, NULL,
+					   "Failed to open file %s for reading", file_name);
+
+	while (fgets(buffer, sizeof(buffer), file))
+		ct_cmd_list_parse_line(list, progname, buffer);
+}
+
+static void ct_cmd_list_parse(struct ct_cmd_list *list, int argc, char *argv[])
+{
+	if (argc > 2
+			&& (!strcmp(argv[1], "-R")
+			|| !strcmp(argv[1], "--load-file"))) {
+		int i;
+
+		cmd_allowed = CT_COMMANDS_LOAD_FILE_ALLOWED;
+
+		for (i = 2; i < argc; ++i)
+			ct_cmd_list_parse_file(list, argv[0], argv[i]);
+		return;
+	}
+	ct_cmd_list_parse_argv(list, argc, argv);
+}
+
 int main(int argc, char *argv[])
 {
 	struct ct_cmd_list list;
@@ -3664,7 +3825,7 @@ int main(int argc, char *argv[])
 
 	ct_cmd_list_init(&list);
 
-	ct_cmd_list_parse_argv(&list, argc, argv);
+	ct_cmd_list_parse(&list, argc, argv);
 
 	ct_cmd_list_apply(&list, argv[0]);
 
-- 
2.25.1

[PATCH v3 6/8] conntrack.8: man update for --load-file support

[Mikhail Sennikovsky]

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 conntrack.8 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/conntrack.8 b/conntrack.8
index 898daae..a14cca6 100644
--- a/conntrack.8
+++ b/conntrack.8
@@ -23,6 +23,8 @@ conntrack \- command line interface for netfilter connection tracking
 .BR "conntrack -C [table]"
 .br
 .BR "conntrack -S "
+.br
+.BR "conntrack -R file"
 .SH DESCRIPTION
 The \fBconntrack\fP utilty provides a full featured userspace interface to the
 Netfilter connection tracking system that is intended to replace the old
@@ -102,6 +104,9 @@ Show the table counter.
 .TP
 .BI "-S, --stats "
 Show the in-kernel connection tracking system statistics.
+.TP
+.BI "-R, --load-file "
+Load entries from a given file. To read from stdin, "\-" should be specified.
 
 .SS PARAMETERS
 .TP
@@ -394,6 +399,9 @@ Delete all flow whose source address is 1.2.3.4
 .TP
 .B conntrack \-U \-s 1.2.3.4 \-m 1
 Set connmark to 1 of all the flows whose source address is 1.2.3.4
+.TP
+.B conntrack -L -w 11 -o save | sed "s/-w 11/-w 12/g" | conntrack --load-file -
+Copy all entries from ct zone 11 to ct zone 12
 
 .SH BUGS
 Please, report them to netfilter-devel@vger.kernel.org or file a bug in
-- 
2.25.1

[PATCH v3 7/8] tests: saving and loading ct entries, save format

[Mikhail Sennikovsky]

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 tests/conntrack/test-conntrack.c    | 84 ++++++++++++++++++++++++-----
 tests/conntrack/testsuite/08stdin   | 80 +++++++++++++++++++++++++++
 tests/conntrack/testsuite/09dumpopt | 77 ++++++++++++++++++++++++++
 3 files changed, 229 insertions(+), 12 deletions(-)
 create mode 100644 tests/conntrack/testsuite/08stdin
 create mode 100644 tests/conntrack/testsuite/09dumpopt

diff --git a/tests/conntrack/test-conntrack.c b/tests/conntrack/test-conntrack.c
index 76ab051..372e025 100644
--- a/tests/conntrack/test-conntrack.c
+++ b/tests/conntrack/test-conntrack.c
@@ -28,6 +28,23 @@ int main()
 	struct dirent *dent;
 	char file[1024];
 	int i,n;
+	char cmd_buf[1024 * 8];
+	int i_cmd_buf = 0;
+	char cmd, cur_cmd = 0;
+	char *cmd_opt;
+
+#define cmd_strappend(_s) do { \
+	char * pos = stpncpy(cmd_buf + i_cmd_buf, _s, sizeof(cmd_buf) - i_cmd_buf); \
+	i_cmd_buf = pos - cmd_buf; \
+	if (i_cmd_buf == sizeof(cmd_buf)) { \
+		printf("buffer full!\n"); \
+		exit(EXIT_FAILURE); \
+	} \
+} while (0)
+
+#define cmd_reset() do { \
+		i_cmd_buf = 0; \
+} while (0)
 
 	n = scandir("testsuite", &dents, NULL, alphasort);
 
@@ -48,9 +65,7 @@ int main()
 		}
 
 		while (fgets(buf, sizeof(buf), fp)) {
-			char tmp[1024] = CT_PROG, *res;
-			tmp[strlen(CT_PROG)] = ' ';
-
+			char *res;
 			line++;
 
 			if (buf[0] == '#' || buf[0] == ' ')
@@ -63,27 +78,72 @@ int main()
 				exit(EXIT_FAILURE);
 			}
 			*res = '\0';
-			res+=2;
+			res++;
+			for (; *res == ' ' || *res == '\t'; res++);
+			cmd = res[0];
+			cmd_opt = &res[1];
+			for (; *cmd_opt == ' ' || *cmd_opt == '\t'; cmd_opt++);
+			res = strchr(cmd_opt, '\n');
+			if (res)
+				*res = '\0';
+
+			if (cur_cmd && cmd != cur_cmd) {
+				/* complete current multi-line command */
+				switch (cur_cmd) {
+				case '\n':
+					cmd_strappend("\" | ");
+					break;
+				default:
+					printf("Internal Error: unexpected multiline command %c",
+							cur_cmd);
+					exit(EXIT_FAILURE);
+					break;
+				}
+
+				cur_cmd = 0;
+			}
+
+			switch (cmd) {
+			case '\n':
+				if (!cur_cmd) {
+					cmd_strappend("echo \"");
+					cur_cmd = cmd;
+				} else
+					cmd_strappend("\n");
+				cmd_strappend(buf);
+				continue;
+			default:
+				cmd_strappend(CT_PROG);
+				cmd_strappend(" ");
+				cmd_strappend(buf);
+				if (cmd == '|') {
+					cmd_strappend(" | ");
+					if (cmd_opt[0]) {
+						cmd_strappend("sed \"");
+						cmd_strappend(cmd_opt);
+						cmd_strappend("\" | ");
+					}
+					continue;
+				}
+				cmd_reset();
+				break;
+			}
 
-			strcpy(tmp + strlen(CT_PROG) + 1, buf);
-			printf("(%d) Executing: %s\n", line, tmp);
+			printf("(%d) Executing: %s\n", line, cmd_buf);
 
 			fflush(stdout);
-			ret = system(tmp);
+			ret = system(cmd_buf);
 
 			if (WIFEXITED(ret) &&
 			    WEXITSTATUS(ret) == EXIT_SUCCESS) {
-			    	if (res[0] == 'O' &&
-				    res[1] == 'K')
+				if (cmd == 'O')
 					ok++;
 				else {
 					bad++;
 					printf("^----- BAD\n");
 				}
 			} else {
-				if (res[0] == 'B' &&
-				    res[1] == 'A' &&
-				    res[2] == 'D')
+				if (cmd == 'B')
 					ok++;
 				else {
 					bad++;
diff --git a/tests/conntrack/testsuite/08stdin b/tests/conntrack/testsuite/08stdin
new file mode 100644
index 0000000..38f3b8b
--- /dev/null
+++ b/tests/conntrack/testsuite/08stdin
@@ -0,0 +1,80 @@
+# create
+# create a conntrack
+-I -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+# create from reply
+-I -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ;
+# create a v6 conntrack
+-I -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+# creae icmp ping request entry
+-I -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ;
+-R - ; OK
+# create again
+-I -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
+# make sure create again with stdio mode fails as well
+-I -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+-R - ; BAD
+-I -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ;
+-R - ; BAD
+# empty lines are ignored
+;
+-I -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+-R - ; BAD
+# spaces or tabs are ignored as well
+  ;
+		;
+-I -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ;
+-R - ; BAD
+# delete
+-D -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 ;
+# empty lines should be just ignored
+;
+;
+# delete reverse
+-D -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 ;
+# empty lines with spaces or tabs should be ignored as well
+ ;
+	;
+		;
+  ;
+	    ;
+	    	    	;
+# delete v6 conntrack
+-D -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 ;
+# delete icmp ping request entry
+-D -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ;
+;
+;
+-R - ; OK
+# create again - should succeed now
+-I -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; OK
+-I -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ; OK
+-I -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; OK
+-I -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# delete again (for cleanup)
+-D -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 ;
+-D -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 ;
+-D -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 ;
+-D -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ;
+;
+-R - ; OK
+# delete no entries - should return err
+-D -w 123 ; BAD
+# delete no entries via stdin - should return err as well
+-D -w 123 ;
+-R - ; BAD
+# delete no entries in parallel with adding entries via stdin - should succeed
+# -D and -I should work in parallel
+-D -w 123 ;
+-I -w 123 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+-R - ; OK
+# now deleting entries should return success
+-D -w 123 ;
+-R - ; OK
+# should fail again
+-D -w 123 ;
+-R - ; BAD
+# validate it via standard command line way
+-D -w 123 ; BAD
\ No newline at end of file
diff --git a/tests/conntrack/testsuite/09dumpopt b/tests/conntrack/testsuite/09dumpopt
new file mode 100644
index 0000000..0d5d9d4
--- /dev/null
+++ b/tests/conntrack/testsuite/09dumpopt
@@ -0,0 +1,77 @@
+# test opts output for -L
+# create
+# create a conntrack
+-I -w 10 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+# create from reply
+-I -w 10 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ;
+# create a v6 conntrack
+-I -w 10 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+# creae icmp ping request entry
+-I -w 10 -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ;
+-R - ; OK
+# copy ipv4 bits to zone 11
+-L -w 10 -o save -f ipv4 ; |s/-w 10/-w 11/g
+-R - ; OK
+# copy ipv6 bits to zone 11
+-L -w 10 -o save -f ipv6 ; |s/-w 10/-w 11/g
+-R - ; OK
+# create again in zone 11
+-I -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -w 11 -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
+# delete new entries
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 ; OK
+# delete reverse
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 ; OK
+# delete v6 conntrack
+-D -w 11-s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 ; OK
+# delete icmp ping request entry
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# delete old entries
+-D -w 10 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 ; OK
+# delete reverse
+-D -w 10 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 ; OK
+# delete v6 conntrack
+-D -w 10-s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 ; OK
+# delete icmp ping request entry
+-D -w 10 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+#
+# now test opts output for -D
+# create entries again
+# create a conntrack
+-I -w 10 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+# create from reply
+-I -w 10 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ;
+# create a v6 conntrack
+-I -w 10 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ;
+# creae icmp ping request entry
+-I -w 10 -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ;
+-R - ; OK
+# move ipv4 bits to zone 11
+-D -w 10 -o save -f ipv4 ; |s/-w 10/-w 11/g; s/-D /-I /g
+-R - ; OK
+# move ipv6 bits to zone 11
+-D -w 10 -o save -f ipv6 ; |s/-w 10/-w 11/g; s/-D /-I /g
+-R - ; OK
+# create again in zone 11
+-I -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; BAD
+-I -w 11 -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
+# delete new entries
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 ; OK
+# delete reverse
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 ; OK
+# delete v6 conntrack
+-D -w 11-s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 ; OK
+# delete icmp ping request entry
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# delete old entries
+-D -w 10 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 ; BAD
+# delete reverse
+-D -w 10 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 ; BAD
+# delete v6 conntrack
+-D -w 10-s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 ; BAD
+# delete icmp ping request entry
+-D -w 10 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
\ No newline at end of file
-- 
2.25.1

[PATCH v3 8/8] tests: conntrack -L/-D ip family filtering

[Mikhail Sennikovsky]

Tests to cover conntrack -L and conntrack -D with and w/o
family (-f) specfied.

conntrack -L and contnrack -D shold list/delete
both IPv4 and IPv6 entries if no family is specified,
and should ony display the corresponding entries if
the family is given.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 tests/conntrack/testsuite/09dumpopt | 72 ++++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/tests/conntrack/testsuite/09dumpopt b/tests/conntrack/testsuite/09dumpopt
index 0d5d9d4..447590b 100644
--- a/tests/conntrack/testsuite/09dumpopt
+++ b/tests/conntrack/testsuite/09dumpopt
@@ -74,4 +74,74 @@
 # delete v6 conntrack
 -D -w 10-s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 ; BAD
 # delete icmp ping request entry
--D -w 10 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
\ No newline at end of file
+-D -w 10 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
+#
+# Additional tests to check that family attribute is treated properly
+# for -L and -D commands
+# namely:
+# - if family (-f) is given - only entries of the given family are dumped/deleted
+# - if no family is given - entries of both ipv4 and ipv6 families are dumped/deleted
+# First create some ipv4 and ipv6 entries
+-I -w 10 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; OK
+-I -w 10 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY -t 50 ; OK
+-I -w 10 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; OK
+-I -w 10 -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# dump all entries to zone 11
+-L -w 10 -o save; |s/-w 10/-w 11/g
+-R - ; OK
+# ensure that both ipv4 and ipv6 entries get copied (delete for each of them should succeed)
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY ; OK
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY ; OK
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; OK
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# dump only ipv4 entries to zone 11
+-L -w 10 -o save -f ipv4; |s/-w 10/-w 11/g
+-R - ; OK
+# ensure that only ipv4 entries get copied (delete only for ipv4 entries should succeed)
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; OK
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY; OK
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# dump only ipv6 entries to zone 11
+-L -w 10 -o save -f ipv6; |s/-w 10/-w 11/g
+-R - ; OK
+# ensure that only ipv6 entries get copied (delete only for ipv6 entries should succeed)
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; OK
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
+# now test deleting w/ and /o family specified
+# for simplicity do it by re-creating entries in zone 11
+# by copying ezisting entries from zone 10 into it
+# re-create entries in ct zone 11
+-L -w 10 -o save; |s/-w 10/-w 11/g
+-R - ; OK
+# delete all entries in zone 11
+-D -w 11 ; OK
+# both ipv4 and ipv6 should be deleted
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
+# re-create entries in ct zone 11
+-L -w 10 -o save; |s/-w 10/-w 11/g
+-R - ; OK
+# delete only ipv4 entries in zone 11
+-D -w 11 -f ipv4 ; OK
+# ipv6 should remain
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; OK
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; BAD
+ # re-create entries in ct zone 11
+-L -w 10 -o save; |s/-w 10/-w 11/g
+-R - ; OK
+# delete only ipv6 entries in zone 11
+-D -w 11 -f ipv6 ; OK
+# ipv4 should remain
+-D -w 11 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; OK
+-D -w 11 -r 2.2.2.2 -q 1.1.1.1 -p tcp --reply-port-src 11 --reply-port-dst 21 --state LISTEN -u SEEN_REPLY; OK
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY; BAD
+-D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# clean up after yourself
+-D -w 10 ; OK
-- 
2.25.1