hi Phil,
On Thu, Apr 19, 2018 at 09:14:56PM +0200, Phil Sutter wrote:
> Making a rule's position be the previous rule's handle (or zero if it's
> the first rule in it's chain) is confusing at least.
>
> Change this to what a user would expect, namely an index of the rule in
> it's chain.
>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> net/netfilter/nf_tables_api.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 8e9ce40392085..945df0827e5a0 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1976,6 +1976,18 @@ static const struct nla_policy nft_rule_policy[NFTA_RULE_MAX + 1] = {
> [NFTA_RULE_ID] = { .type = NLA_U32 },
> };
>
> +static int rule_get_position(const struct nft_rule *rule,
> + const struct list_head *start)
> +{
> + int pos = 0;
> +
> + while (rule->list.prev != start) {
> + rule = list_prev_entry(rule, list);
> + pos++;
> + }
> + return pos;
> +}
> +
> static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
> u32 portid, u32 seq, int event,
> u32 flags, int family,
> @@ -1987,7 +1999,6 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
> struct nfgenmsg *nfmsg;
> const struct nft_expr *expr, *next;
> struct nlattr *list;
> - const struct nft_rule *prule;
> u16 type = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
>
> nlh = nlmsg_put(skb, portid, seq, type, sizeof(struct nfgenmsg), flags);
> @@ -2007,10 +2018,10 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
> NFTA_RULE_PAD))
> goto nla_put_failure;
>
> - if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) {
> - prule = list_prev_entry(rule, list);
> - if (nla_put_be64(skb, NFTA_RULE_POSITION,
> - cpu_to_be64(prule->handle),
> + if ((event != NFT_MSG_DELRULE)) {
> + int pos = rule_get_position(rule, &chain->rules);
> +
> + if (nla_put_be64(skb, NFTA_RULE_POSITION, cpu_to_be64(pos),
I understand the goal of this is to find a 1:1 mapping with iptables,
but this is actually solving a design problem in the iptables interface.
Rule counting to locate rule position is prone to races, the use of the
handle to identify the position to replace rules help us make sure that
we replace the right rule with several concurrent nft.
--
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
