Hi Pablo,
diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 7640290..3749069 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -37,6 +37,8 @@ enum nf_tables_msg_types {
NFT_MSG_NEWSETELEM,
NFT_MSG_GETSETELEM,
NFT_MSG_DELSETELEM,
+ NFT_MSG_COMMIT,
+ NFT_MSG_ABORT,
NFT_MSG_MAX,
};
@@ -85,12 +87,18 @@ enum nft_chain_attributes {
};
#define NFTA_CHAIN_MAX (__NFTA_CHAIN_MAX - 1)
+enum {
+ NFT_RULE_F_COMMIT = (1 << 0),
+ NFT_RULE_F_MASK = NFT_RULE_F_COMMIT,
+};
+
I guess you added this flags to add rules on non-transaction based
use-case, so it commits right away.
Wouldn't it be better to have no such flags in the API, but instead
keep track of an on going transaction in the struct nft_ctx (let's
have an internal flag here).
I.E: no on-going transaction -> we directly commit. If not, it's
handled as being part of the transaction.
I just took a look at nfnetlink however, it does not seem possible
to keep a data pointer per connection on the subsystem.
Wait, there is a field in struct sock that is meant for such usage.
It would require to change nf_tables_api.c only then.
Would it make sense or am I wrong with nft_ctx usage? (there would
be an issue: to free such context when the connection goes down, we
could abort an on-going transaction this way then)
We can define some container structure to store rules in the dirty
list:
struct nft_rule_update {
struct list_head head;
uint32_t nl_portid;
struct nft_rule *rule;
struct nft_table *table;
struct nft_chain *chain;
}
That should allows us to remove the struct list_head dirty_list in
struct nft_rule that I needed for this.
The nl_portid would be the netlink portid so we know what updates
belong to what netlink connection. Still I don't see how to get rid of
the commit flag.
Could you develop your idea?
I was exactly thinking about such external list. But it would be tighten
to the netlink connection more deeply: as a user data to the socket,
instead of storing the nl_portid.
I will explain below why.
To me iptables-nftables is a non-transaction based tool. There is no
point to start a transaction for one rule.
Btw it would then require a NFT_MSG_START or some sort, to start the
transaction based manipulation.
Let's say now you have a software, which require to do rules
manipulation, very often, which will be always connected through netlink
to nftables.
starts a transaction:
- manip 1
- manip 2
- ...
- manip n
Commit or Abort.
done.
Now, for whatever reason: the software crashes in the middle of a
transaction. When it restarts it has no idea what it was doing and why.
Here is why we should tight the transaction per netlink connection: if
the connections breaks, we abort right away. It's transparent.
It's consistent also with the fact that you raise a notification only
when the rule has been activated. Until it comes: no one knows about those
rules in the transaction but the one who owns the transaction.
We could do that via the struct sock { ... user_data ... }; related to
the netlink connection, storing the transaction list.
Now, no need of a flag: if the transaction list for the current netlink
connection is not NULL: you know you are on a transaction-based work, so
whatever manipulation comes: it will be part of the transaction. If it's
NULL, you do as usual: activating the rule right away.
* @list: used internally
* @rcu_head: used internally
+ * @net: net namespace that this chain belongs to
I would see that in another patch, even if it's a really tiny one.
(preceding this current one)
Moreover that we will have to full support the namespaces at some
point, right?
I need that net for the code in nft_do_chain_pktinfo added in this
patch. Probably it can be added to base chains only, would need to
check that.
Indeed. I missed that.
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
