On Tue, Apr 24, 2018 at 08:49:33PM +0200, Phil Sutter wrote:
> Hi,
>
> On Tue, Apr 24, 2018 at 05:51:39PM +0200, Pablo Neira Ayuso wrote:
> > 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?
>
> How about using 'handle <handle>' for that? It would be clear what's
> expected, and it is consistent with replace and delete commands.
I would prefer using "handle <handle>" for symmetry with the
delete/replace verbs.
> I still think allowing users to specify an absolute position to insert a
> rule at would be nice to have, despite the potential problems that come
> with it. Though I guess reusing 'position' keyword for it is not an
> option due to backwards compatibility, right?
Unfortunately, the meaning of "position" changing would be pretty
unexpected.
> My idea was to implement that absolute position support in userspace by
> fetching the current ruleset and translating user-supplied position
> index into a handle to pass to the kernel.
>
> What do you think?
>
> Cheers, Phil
> --
> 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
--
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
