Re: [nf-next PATCH] net: nftables: Make rule position deterministic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 20, 2018 at 09:21:12AM -0400, Eric Garver wrote:
> On Fri, Apr 20, 2018 at 12:00:54PM +0200, Jan Engelhardt wrote:
> > 
> > On Friday 2018-04-20 10:47, Pablo Neira Ayuso wrote:
> > >> -	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),
> > >
> > >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.
> > 
> > That is absolutely understandable, but then the handle, which I take works like
> > a uuid or an sql auto_increment, should be sent with a "handle" label, and not
> > be advertised as a "position" or" index" IMO, since these two sort of convey a
> > hole-free sequence.
> 
> I think this is the real issue. IMO, "position" implies a counting
> order.
> Something like:
> 
>     "insert rule .. before <handle> .."
>     "add rule .. after <handle> .."
> 
> would be much more intuitive.

Yes, the before and after sounds more intuitive.

Still, insert and add already comes with semantics (inherited from
iptables) indicating before or after.

Probably we can use 'at' instead of 'position'?

        insert rule ... at <handle>
        add rule ...    at <handle>

Although this still denotes position?

> The nft man page is also misleading.
> 
> 	RULES
> 		   [add | insert] rule [family] table chain [position position] statement...
> 		   replace rule [family] table chain handle handle statement...
> 		   delete rule [family] table chain handle handle
> 
> delete/replace use <handle> where the handle ID should go, but add/insert use
> <position>. This seems to imply they're different types.

Right, Florian just fixed this.
--
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



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux