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.
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.
Thanks.
Eric.
--
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
