netlink: add NLM_F_NONREC flag for deletion requests

In the last NFWS in Faro, Portugal, we discussed that netlink is lacking the semantics to request non recursive deletions, ie. do not delete an object iff it has child objects that hang from this parent object that the user requests to be deleted.

We need this new flag to solve a problem for the iptables-compat backward compatibility utility, that runs iptables commands using the existing nf_tables netlink interface. Specifically, custom chains in iptables cannot be deleted if there are rules in it, however, nf_tables allows to remove any chain that is populated with content. To sort out this asymmetry, iptables-compat userspace sets this new NLM_F_NONREC flag to obtain the same semantics that iptables provides.

This new flag should only be used for deletion requests. Note this new flag value overlaps with the existing:

* NLM_F_ROOT for get requests.
* NLM_F_REPLACE for new requests.

However, those flags should not ever be used in deletion requests.

---
@David: Please, acknowledge this if you think this is fine so I can
take this into the nf-next tree, given patches 4/5 and 5/5 depend on this. Thanks a lot!

 include/uapi/linux/netlink.h | 3 +++
 1 file changed, 3 insertions(+)

index f4fc9c9e123d..e8af60a7c56d 100644
@@ -69,6 +69,9 @@ struct nlmsghdr {
 #define NLM_F_CREATE	0x400	/* Create, if it does not exist	*/
 #define NLM_F_APPEND	0x800	/* Add to end of list		*/
 
+/* Modifiers to DELETE request */
+#define NLM_F_NONREC	0x100	/* Do not delete recursively	*/
+
 /* Flags for ACK message */
 #define NLM_F_CAPPED	0x100	/* request was capped */
 #define NLM_F_ACK_TLVS	0x200	/* extended ACK TVLs were included */

2.1.4

Bail out if user requests non-recursive deletion for tables and sets. This new flags tells nf_tables netlink interface to reject deletions if tables and sets have content.

---
 net/netfilter/nf_tables_api.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

index 8b86acbb9770..2ea043e5b344 100644
@@ -860,6 +860,10 @@ static int nf_tables_deltable(struct net *net, struct sock *nlsk,
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
+	if (nlh->nlmsg_flags & NLM_F_NONREC &&
+	    table->use > 0)
+		return -EBUSY;
+
 	ctx.afi = afi;
 	ctx.table = table;
 

@@ -3225,7 +3229,9 @@ static int nf_tables_delset(struct net *net, struct sock *nlsk,
 	set = nf_tables_set_lookup(ctx.table, nla[NFTA_SET_NAME], genmask);
 	if (IS_ERR(set))
 		return PTR_ERR(set);
-	if (!list_empty(&set->bindings))
+
+	if (!list_empty(&set->bindings) ||
+	    (nlh->nlmsg_flags & NLM_F_NONREC && atomic_read(&set->nelems) > 0))
 		return -EBUSY;
 
 	return nft_delset(&ctx, set);

2.1.4

This patch sorts out an asymmetry in deletions. Currently, table and set deletion commands come with an implicit content flush on deletion. However, chain deletion results in -EBUSY if there is content in this chain, so no implicit flush happens. So you have to send a flush command in first place to delete chains, this is inconsistent and it can be annoying in terms of user experience.

This patch uses the new NLM_F_NONREC flag to request non-recursive chain deletion, ie. if the chain to be removed contains rules, then this returns EBUSY. This problem was discussed during the NFWS'17 in Faro, Portugal. In iptables, you hit -EBUSY if you try to delete a chain that contains rules, so you have to flush first before you can remove anything. Since iptables-compat uses the nf_tables netlink interface, it has to use the NLM_F_NONREC flag from userspace to retain the original iptables semantics, ie. bail out on removing chains that contain rules.

---
 net/netfilter/nf_tables_api.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

index 2ea043e5b344..5e2cfdbd51bd 100644
@@ -1617,8 +1617,11 @@ static int nf_tables_delchain(struct net *net, struct sock *nlsk,
 	struct nft_af_info *afi;
 	struct nft_table *table;
 	struct nft_chain *chain;
+	struct nft_rule *rule;
 	int family = nfmsg->nfgen_family;
 	struct nft_ctx ctx;
+	u32 use;
+	int err;
 
 	afi = nf_tables_afinfo_lookup(net, family, false);
 	if (IS_ERR(afi))

@@ -1631,11 +1634,30 @@ static int nf_tables_delchain(struct net *net, struct sock *nlsk,
 	chain = nf_tables_chain_lookup(table, nla[NFTA_CHAIN_NAME], genmask);
 	if (IS_ERR(chain))
 		return PTR_ERR(chain);
-	if (chain->use > 0)
+
+	if (nlh->nlmsg_flags & NLM_F_NONREC &&
+	    chain->use > 0)
 		return -EBUSY;
 
 	nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
 
+	use = chain->use;
+	list_for_each_entry(rule, &chain->rules, list) {
+		if (!nft_is_active_next(net, rule))
+			continue;
+		use--;
+
+		err = nft_delrule(&ctx, rule);
+		if (err < 0)
+			return err;
+	}
+
+	/* There are rules and elements that are still holding references to us,
+	 * we cannot do a recursive removal in this case.
+	 */
+	if (use > 0)
+		return -EBUSY;
+
 	return nft_delchain(&ctx);
 }
 

2.1.4

I only see patches 3, 4, and 5 of this series.

If this is meant for net-next inclusion, you'll have to submit it such that I see the entire series on netdev and thus in patchwork.

Thanks.

I'm posting this new NLM_F_NONREC for acknowledgment, if possible. I have a few more patches that follow up so I can take them through nf-next in the next batch.

But I can just re-send this through your net-next tree, as you prefer.