commit 91dc281a82ea ("src: rework singleton interval transformation to reduce memory consumption") duplicates singleton interval elements when the netlink message gets full, this results in spurious EEXIST errors when creating many elements in a set. This patch extends the existing test to cover for this bug. Signed-off-by: Pablo Neira Ayuso --- .../maps/0004interval_map_create_once_0 | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/shell/testcases/maps/0004interval_map_create_once_0 b/tests/shell/testcases/maps/0004interval_map_create_once_0 index 7d3825596c49..965da2d0663a 100755 --- a/tests/shell/testcases/maps/0004interval_map_create_once_0 +++ b/tests/shell/testcases/maps/0004interval_map_create_once_0 @@ -41,6 +41,29 @@ generate_test() { echo -e "$elements" } +echo "add table x +add map x y { type ipv4_addr : ipv4_addr; flags interval; } +create element x y $(generate_add)" > $tmpfile + +set -e +$NFT -f $tmpfile + +EXPECTED="table ip x { + map y { + type ipv4_addr : ipv4_addr + flags interval + elements = { "$(generate_test)" } + } +}" +GET=$($NFT list ruleset) +if [ "$EXPECTED" != "$GET" ] ; then + $DIFF -u <(echo "$EXPECTED") <(echo "$GET") + exit 1 +fi + +$NFT flush ruleset + +# now try with add element command echo "add table x add map x y { type ipv4_addr : ipv4_addr; flags interval; } add element x y $(generate_add)" > $tmpfile -- 2.47.3 The rework to reduce memory consumption has introduced a bug that result in spurious EEXIST with large batches. The code that tracks the start and end elements of the interval can add the same element twice to the batch. This works with the add element command, since it ignores EEXIST error, but it breaks the the create element command. Update this codepath to ensure both sides of the interval fit into the netlink message, otherwise, trim the netlink message to remove them. So the next netlink message includes the elements that represent the interval that could not fit. Fixes: 91dc281a82ea ("src: rework singleton interval transformation to reduce memory consumption") Signed-off-by: Pablo Neira Ayuso --- src/mnl.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/mnl.c b/src/mnl.c index 0a445189da82..eee0a33ceaeb 100644 --- a/src/mnl.c +++ b/src/mnl.c @@ -1880,16 +1880,17 @@ static int set_elem_cb(const struct nlmsghdr *nlh, void *data) static bool mnl_nft_attr_nest_overflow(struct nlmsghdr *nlh, const struct nlattr *from, - const struct nlattr *to) + const struct nlattr *to, + unsigned int nest_len) { - int len = (void *)to + to->nla_len - (void *)from; + int len = (void *)to + nest_len - (void *)from; /* The attribute length field is 16 bits long, thus the maximum payload * that an attribute can convey is UINT16_MAX. In case of overflow, * discard the last attribute that did not fit into the nest. */ if (len > UINT16_MAX) { - nlh->nlmsg_len -= to->nla_len; + nlh->nlmsg_len -= nest_len; return true; } return false; @@ -1955,8 +1956,9 @@ static int mnl_nft_setelem_batch(const struct nftnl_set *nls, struct cmd *cmd, struct netlink_ctx *ctx) { struct nftnl_set_elem *nlse, *nlse_high = NULL; + struct nlattr *nest1, *nest2, *nest3; struct expr *expr = NULL, *next; - struct nlattr *nest1, *nest2; + unsigned int nest_len = 0; struct nlmsghdr *nlh; int i = 0; @@ -1998,33 +2000,35 @@ next: else next = NULL; - if (!nlse_high) { - nlse = alloc_nftnl_setelem_interval(set, init, expr, next, &nlse_high); - } else { - nlse = nlse_high; - nlse_high = NULL; - } + nlse = alloc_nftnl_setelem_interval(set, init, expr, next, &nlse_high); } else { nlse = alloc_nftnl_setelem(init, expr); } cmd_add_loc(cmd, nlh, &expr->location); - /* remain with this element, range high still needs to be added. */ - if (nlse_high) - expr = list_prev_entry(expr, list); - nest2 = mnl_attr_nest_start(nlh, ++i); nftnl_set_elem_nlmsg_build_payload(nlh, nlse); mnl_attr_nest_end(nlh, nest2); netlink_dump_setelem(nlse, ctx); nftnl_set_elem_free(nlse); - if (mnl_nft_attr_nest_overflow(nlh, nest1, nest2)) { - if (nlse_high) { - nftnl_set_elem_free(nlse_high); - nlse_high = NULL; - } + + nest_len = nest2->nla_len; + + if (nlse_high) { + nest3 = mnl_attr_nest_start(nlh, ++i); + nftnl_set_elem_nlmsg_build_payload(nlh, nlse_high); + mnl_attr_nest_end(nlh, nest3); + + netlink_dump_setelem(nlse_high, ctx); + nftnl_set_elem_free(nlse_high); + nlse_high = NULL; + + nest_len += nest3->nla_len; + } + + if (mnl_nft_attr_nest_overflow(nlh, nest1, nest2, nest_len)) { mnl_attr_nest_end(nlh, nest1); mnl_nft_batch_continue(batch); mnl_seqnum_inc(seqnum); -- 2.47.3