Buffered commands are just ... buffered.

Calculate the free buffer size when adding the existing attributes at the buffered
commands. If the buffer is full, cancel the unfinished nested attribute and commit
the previously buffered commands. Then restart with the current buffered command.
Thus we can get rid of the ugly maxsize parameter of the set types.

3 files changed, 82 insertions, 129 deletions

diff --git a/include/libipset/types.h b/include/libipset/types.h
index f1847aa..f9c8f2d 100644
--- a/include/libipset/types.h
+++ b/include/libipset/types.h
@@ -58,13 +58,6 @@ enum {
 	IPSET_KERNEL_OK,
 };
 
-/* Max sizes for aggregated ADD (and DEL) commands */
-enum {
-	IPSET_MAXSIZE_INET,
-	IPSET_MAXSIZE_INET6,
-	IPSET_MAXSIZE_MAX,
-};
-
 /* How element parts are parsed */
 struct ipset_elem {
 	ipset_parsefn parse;			/* elem parser function */
@@ -88,7 +81,6 @@ struct ipset_type {
 	const struct ipset_arg *args[IPSET_CADT_MAX]; /* create/ADT args besides elem */
 	uint64_t mandatory[IPSET_CADT_MAX];	/* create/ADT mandatory flags */
 	uint64_t full[IPSET_CADT_MAX];		/* full args flags */
-	size_t maxsize[IPSET_MAXSIZE_MAX];	/* max sizes */
 	const char *usage;			/* terse usage */
 	void (*usagefn)(void);			/* additional usage */
 
diff --git a/lib/session.c b/lib/session.c
index ba4e458..babd881 100644
--- a/lib/session.c
+++ b/lib/session.c
@@ -1311,10 +1311,13 @@ callback_noop(const struct nlmsghdr *nlh UNUSED, void *data UNUSED)
  * Build and send messages
  */
 
-static inline void
+static inline int
 open_nested(struct ipset_session *session, struct nlmsghdr *nlh, int attr)
 {
+	if (nlh->nlmsg_len + MNL_ATTR_HDRLEN > session->bufsize)
+		return 1;
 	session->nested[session->nestid++] = mnl_attr_nest_start(nlh, attr);
+	return 0;
 }
 
 static inline void
@@ -1348,8 +1351,11 @@ attr_len(const struct ipset_attr_policy *attr, uint8_t family, uint16_t *flags)
 	}
 }
 
+#define BUFFER_FULL(bufsize, nlmsg_len, nestlen, attrlen)	\
+(nlmsg_len + nestlen + MNL_ATTR_HDRLEN + MNL_ALIGN(alen) + MNL_ALIGN(sizeof(struct nlmsgerr)) > bufsize)
+
 static int
-rawdata2attr(struct nlmsghdr *nlh,
+rawdata2attr(struct ipset_session *session, struct nlmsghdr *nlh,
 	     const void *d, int type, uint8_t family,
 	     const struct ipset_attr_policy attrs[])
 {
@@ -1359,15 +1365,16 @@ rawdata2attr(struct nlmsghdr *nlh,
 
 
 	attr = &attrs[type];
-	alen = attr_len(attr, family, &flags);
-
-	switch (attr->type) {
-	case MNL_TYPE_NESTED: {
+	if (attr->type == MNL_TYPE_NESTED) {
 		/* IP addresses */
-		struct nlattr *nested = mnl_attr_nest_start(nlh, type);
+		struct nlattr *nested;
 		int atype = family == AF_INET ? IPSET_ATTR_IPADDR_IPV4
 					      : IPSET_ATTR_IPADDR_IPV6;
 
+		alen = attr_len(attr, family, &flags);
+		if (BUFFER_FULL(session->bufsize, nlh->nlmsg_len, MNL_ATTR_HDRLEN, alen))
+			return 1;
+		nested = mnl_attr_nest_start(nlh, type);
 		D("family: %s", family == AF_INET ? "INET" :
 				family == AF_INET6 ? "INET6" : "UNSPEC");
 		mnl_attr_put(nlh, atype | flags, alen, d);
@@ -1375,6 +1382,12 @@ rawdata2attr(struct nlmsghdr *nlh,
 		
 		return 0;
 	}
+
+	alen = attr_len(attr, family, &flags);
+	if (BUFFER_FULL(session->bufsize, nlh->nlmsg_len, 0, alen))
+		return 1;
+
+	switch (attr->type) {
 	case MNL_TYPE_U32: {
 		uint32_t value = htonl(*(const uint32_t *)d);
 		
@@ -1397,66 +1410,52 @@ rawdata2attr(struct nlmsghdr *nlh,
 }
 
 static int
-data2attr(struct nlmsghdr *nlh,
+data2attr(struct ipset_session *session, struct nlmsghdr *nlh,
 	  struct ipset_data *data, int type, uint8_t family,
 	  const struct ipset_attr_policy attrs[])
 {
 	const struct ipset_attr_policy *attr = &attrs[type];
 
-	return rawdata2attr(nlh, ipset_data_get(data, attr->opt),
+	return rawdata2attr(session, nlh, ipset_data_get(data, attr->opt),
 			    type, family, attrs);
 }
 
 #define ADDATTR_PROTOCOL(nlh)						\
 	mnl_attr_put_u8(nlh, IPSET_ATTR_PROTOCOL, IPSET_PROTOCOL)
 
-#define ADDATTR(nlh, data, type, family, attrs)				\
-	data2attr(nlh, data, type, family, attrs)
+#define ADDATTR(session, nlh, data, type, family, attrs)		\
+	data2attr(session, nlh, data, type, family, attrs)
 
-#define ADDATTR_SETNAME(nlh, data)					\
-	data2attr(nlh, data, IPSET_ATTR_SETNAME, AF_INET, cmd_attrs)
+#define ADDATTR_SETNAME(session, nlh, data)				\
+	data2attr(session, nlh, data, IPSET_ATTR_SETNAME, AF_INET, cmd_attrs)
 
-#define ADDATTR_IF(nlh, data, type, family, attrs)			\
-	if (ipset_data_test(data, attrs[type].opt)) {			\
-		D("addattr if: %u", attrs[type].opt);			\
-		data2attr(nlh, data, type, family, attrs); }
+#define ADDATTR_IF(session, nlh, data, type, family, attrs)		\
+	ipset_data_test(data, attrs[type].opt) ? 			\
+		data2attr(session, nlh, data, type, family, attrs) : 0
 
-#define ADDATTR_RAW(nlh, data, type, attrs)				\
-	rawdata2attr(nlh, data, type, AF_INET, attrs)
+#define ADDATTR_RAW(session, nlh, data, type, attrs)			\
+	rawdata2attr(session, nlh, data, type, AF_INET, attrs)
 
 static void
-addattr_create(struct nlmsghdr *nlh, struct ipset_data *data, uint8_t family)
+addattr_create(struct ipset_session *session,
+	       struct nlmsghdr *nlh, struct ipset_data *data, uint8_t family)
 {
 	int i;
 
 	for (i = IPSET_ATTR_UNSPEC + 1; i <= IPSET_ATTR_CREATE_MAX; i++)
-		ADDATTR_IF(nlh, data, i, family, create_attrs);
+		ADDATTR_IF(session, nlh, data, i, family, create_attrs);
 }
 
-static void
-addattr_adt(struct nlmsghdr *nlh, struct ipset_data *data, uint8_t family)
+static int
+addattr_adt(struct ipset_session *session,
+	    struct nlmsghdr *nlh, struct ipset_data *data, uint8_t family)
 {
 	int i;
 	
 	for (i = IPSET_ATTR_UNSPEC + 1; i <= IPSET_ATTR_ADT_MAX; i++)
-		ADDATTR_IF(nlh, data, i, family, adt_attrs);
-}
-
-static inline bool
-buffer_full(struct ipset_session *session)
-{
-	struct nlmsghdr *nlh;
-	const struct ipset_type *type;
-	int sizeid;
-
-	nlh = (struct nlmsghdr *) session->buffer;
-	type = ipset_data_get(session->data, IPSET_OPT_TYPE);
-	sizeid = !!(ipset_data_family(session->data) != AF_INET);
-
-	/* Current size + ADD/DEL max size + nlsmgerr must fit into buffer */
-	return session->bufsize < nlh->nlmsg_len
-				  + type->maxsize[sizeid]
-				  + MNL_ALIGN(sizeof(struct nlmsgerr));
+		if (ADDATTR_IF(session, nlh, data, i, family, adt_attrs))
+			return 1;
+	return 0;
 }
 
 #define PRIVATE_MSG_BUFLEN	256
@@ -1483,16 +1482,16 @@ build_send_private_msg(struct ipset_session *session, enum ipset_cmd cmd)
 			return ipset_err(session,
 				"Invalid internal HEADER command: "
 				"missing setname");
-		ADDATTR_SETNAME(nlh, data);
+		ADDATTR_SETNAME(session, nlh, data);
 		break;
 	case IPSET_CMD_TYPE:
 		if (!ipset_data_test(data, IPSET_OPT_TYPENAME))
 			return ipset_err(session,
 				"Invalid internal TYPE command: "
 				"missing settype");
-		ADDATTR(nlh, data, IPSET_ATTR_TYPENAME, AF_INET, cmd_attrs);
+		ADDATTR(session, nlh, data, IPSET_ATTR_TYPENAME, AF_INET, cmd_attrs);
 		if (ipset_data_test(data, IPSET_OPT_FAMILY))
-			ADDATTR(nlh, data, IPSET_ATTR_FAMILY, AF_INET, cmd_attrs);
+			ADDATTR(session, nlh, data, IPSET_ATTR_FAMILY, AF_INET, cmd_attrs);
 		else
 			/* bitmap:port and list:set types */
 			mnl_attr_put_u8(nlh, IPSET_ATTR_FAMILY, AF_UNSPEC);
@@ -1517,12 +1516,9 @@ may_aggregate_ad(struct ipset_session *session, struct ipset_data *data,
 	return (cmd == IPSET_CMD_ADD || cmd == IPSET_CMD_DEL)
 	       && cmd == session->cmd
 	       && STREQ(ipset_data_setname(data), session->saved_setname);
-} 
-
-int
-build_msg(struct ipset_session *session, bool aggregate);
+}
 
-int
+static int
 build_msg(struct ipset_session *session, bool aggregate)
 {
 	struct nlmsghdr *nlh = (struct nlmsghdr *) session->buffer;
@@ -1556,18 +1552,18 @@ build_msg(struct ipset_session *session, bool aggregate)
 		type = ipset_data_get(data, IPSET_OPT_TYPE);
 		/* Core attributes:
 		 * setname, typename, revision, family, flags (optional) */
-		ADDATTR_SETNAME(nlh, data);
-		ADDATTR(nlh, data, IPSET_ATTR_TYPENAME, AF_INET, cmd_attrs);
-		ADDATTR_RAW(nlh, &type->revision,
+		ADDATTR_SETNAME(session, nlh, data);
+		ADDATTR(session, nlh, data, IPSET_ATTR_TYPENAME, AF_INET, cmd_attrs);
+		ADDATTR_RAW(session, nlh, &type->revision,
 			    IPSET_ATTR_REVISION, cmd_attrs);
 		D("family: %u, type family %u",
 		  ipset_data_family(data), type->family);
-		ADDATTR(nlh, data, IPSET_ATTR_FAMILY, AF_INET, cmd_attrs);
+		ADDATTR(session, nlh, data, IPSET_ATTR_FAMILY, AF_INET, cmd_attrs);
 
 		/* Type-specific create attributes */
 		D("call open_nested");
 		open_nested(session, nlh, IPSET_ATTR_DATA);
-		addattr_create(nlh, data, type->family);
+		addattr_create(session, nlh, data, type->family);
 		D("call close_nested");
 		close_nested(session, nlh);
 		break;
@@ -1577,7 +1573,7 @@ build_msg(struct ipset_session *session, bool aggregate)
 	case IPSET_CMD_LIST:
 	case IPSET_CMD_SAVE:
 		if (ipset_data_test(data, IPSET_SETNAME))
-			ADDATTR_SETNAME(nlh, data);
+			ADDATTR_SETNAME(session, nlh, data);
 		break;
 	case IPSET_CMD_RENAME:
 	case IPSET_CMD_SWAP:
@@ -1589,8 +1585,8 @@ build_msg(struct ipset_session *session, bool aggregate)
 			return ipset_err(session,
 				"Invalid %s command: missing to-setname",
 				session->cmd == IPSET_CMD_SWAP ? "swap" : "rename");
-		ADDATTR_SETNAME(nlh, data);
-		ADDATTR_RAW(nlh, ipset_data_get(data, IPSET_OPT_SETNAME2),
+		ADDATTR_SETNAME(session, nlh, data);
+		ADDATTR_RAW(session, nlh, ipset_data_get(data, IPSET_OPT_SETNAME2),
 			    IPSET_ATTR_SETNAME2, cmd_attrs);
 		break;
 	case IPSET_CMD_ADD:
@@ -1610,10 +1606,10 @@ build_msg(struct ipset_session *session, bool aggregate)
 					session->cmd == IPSET_CMD_ADD ? "add" : "del");
 
 			/* Core options: setname */
-			ADDATTR_SETNAME(nlh, data);
+			ADDATTR_SETNAME(session, nlh, data);
 			if (session->lineno != 0) {
 				/* Restore mode */
-				ADDATTR_RAW(nlh, &session->lineno,
+				ADDATTR_RAW(session, nlh, &session->lineno,
 					    IPSET_ATTR_LINENO, cmd_attrs);
 				open_nested(session, nlh, IPSET_ATTR_ADT);
 			}
@@ -1621,10 +1617,18 @@ build_msg(struct ipset_session *session, bool aggregate)
 		type = ipset_data_get(data, IPSET_OPT_TYPE);
 		D("family: %u, type family %u",
 		  ipset_data_family(data), type->family);
-		open_nested(session, nlh, IPSET_ATTR_DATA);
-		addattr_adt(nlh, data, ipset_data_family(data));
-		ADDATTR_RAW(nlh, &session->lineno,
-			    IPSET_ATTR_LINENO, cmd_attrs);
+		if (open_nested(session, nlh, IPSET_ATTR_DATA)) {
+			D("open_nested failed");
+			return 1;
+		}
+		if (addattr_adt(session, nlh, data, ipset_data_family(data))
+		    || ADDATTR_RAW(session, nlh, &session->lineno,
+				   IPSET_ATTR_LINENO, cmd_attrs)) {
+			/* Cancel last, unfinished nested attribute */
+			mnl_attr_nest_cancel(nlh, session->nested[session->nestid-1]);
+			session->nested[--session->nestid] = NULL;
+			return 1;
+		}
 		close_nested(session, nlh);
 		break;
 	}
@@ -1645,9 +1649,9 @@ build_msg(struct ipset_session *session, bool aggregate)
 		type = ipset_data_get(data, IPSET_OPT_TYPE);
 		D("family: %u, type family %u",
 		  ipset_data_family(data), type->family);
-		ADDATTR_SETNAME(nlh, data);
+		ADDATTR_SETNAME(session, nlh, data);
 		open_nested(session, nlh, IPSET_ATTR_DATA);
-		addattr_adt(nlh, data, ipset_data_family(data));
+		addattr_adt(session, nlh, data, ipset_data_family(data));
 		close_nested(session, nlh);
 		break;
 	}
@@ -1766,15 +1770,9 @@ ipset_cmd(struct ipset_session *session, enum ipset_cmd cmd, uint32_t lineno)
 	if (cmd == IPSET_CMD_TYPE || cmd == IPSET_CMD_HEADER) 
 		return build_send_private_msg(session, cmd);
 	
-	/* Check buffer and aggregatable commands */
-	if (session->lineno != 0) {
-		if (may_aggregate_ad(session, data, cmd))
-			aggregate = true;
-		if (!aggregate || buffer_full(session)) {
-		    	 if (ipset_commit(session) < 0)
-				goto cleanup;
-		}
-	}
+	/* Check aggregatable commands */
+	if (session->lineno != 0)
+		aggregate = may_aggregate_ad(session, data, cmd);
 
 	/* Real command: update lineno too */
 	session->cmd = cmd;
@@ -1791,7 +1789,16 @@ ipset_cmd(struct ipset_session *session, enum ipset_cmd cmd, uint32_t lineno)
 
 	D("next: build_msg");
 	/* Build new message or append buffered commands */
-	if (build_msg(session, aggregate) < 0)
+	ret = build_msg(session, aggregate);
+	D("build_msg returned %u", ret);
+	if (ret > 0) {
+		/* Buffer is full, send buffered commands */
+		if (ipset_commit(session) < 0)
+			goto cleanup;
+		ret = build_msg(session, false);
+		D("build_msg 2 returned %u", ret);
+	}
+	if (ret < 0)
 		goto cleanup;
 	D("past: build_msg");
 
diff --git a/lib/types.c b/lib/types.c
index 3d9b034..3034477 100644
--- a/lib/types.c
+++ b/lib/types.c
@@ -426,35 +426,6 @@ ipset_type_check(struct ipset_session *session)
 	return match;
 }
 
-static void
-type_max_size(struct ipset_type *type, uint8_t family)
-{
-	int sizeid;
-	enum ipset_opt opt;
-	size_t max = 0;
-
-	sizeid = family == AF_INET ? IPSET_MAXSIZE_INET : IPSET_MAXSIZE_INET6;
-	for (opt = IPSET_OPT_NONE + 1; opt < IPSET_OPT_MAX; opt++) {
-		if (!(IPSET_FLAG(opt) & IPSET_ADT_FLAGS))
-			continue;
-		if (!(IPSET_FLAG(opt) & type->full[IPSET_ADD]))
-			continue;
-		max += MNL_ALIGN(ipset_data_sizeof(opt, family))
-			+ MNL_ATTR_HDRLEN;
-		switch (opt) {
-		case IPSET_OPT_IP:
-		case IPSET_OPT_IP_TO:
-		case IPSET_OPT_IP2:
-			/* Nested attributes */
-			max += MNL_ATTR_HDRLEN;
-			break;
-		default:
-			break;
-		}
-	}
-	type->maxsize[sizeid] = max;
-}
-
 /**
  * ipset_type_add - add (register) a userspace set type
  * @type: pointer to the set type structure
@@ -471,23 +442,6 @@ ipset_type_add(struct ipset_type *type)
 
 	assert(type);
 
-	/* Fill out max sizes */
-	switch (type->family) {
-	case AF_UNSPEC:
-	case AF_INET:
-		type_max_size(type, AF_INET);
-		break;
-	case AF_INET6:
-		type_max_size(type, AF_INET6);
-		break;
-	case AF_INET46:
-		type_max_size(type, AF_INET);
-		type_max_size(type, AF_INET6);
-		break;
-	default:
-		return -1;
-	}
-
 	/* Add to the list: higher revision numbers first */
 	for (t = typelist, prev = NULL; t != NULL; t = t->next) {
 		if (STREQ(t->name, type->name)) {