On Tue, Feb 25, 2020 at 09:38:15PM +0100, Stefano Brivio wrote:
> On Tue, 25 Feb 2020 21:21:43 +0100
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
> > Hi Stefano,
> >
> > On Tue, Feb 25, 2020 at 03:34:35PM +0100, Stefano Brivio wrote:
> > [...]
> > > This is the problem Phil reported:
> > [...]
> > > Or also simply with:
> > >
> > > # nft add element t s '{ 20-30 . 40 }'
> > > # nft add element t s '{ 25-35 . 40 }'
> > >
> > > the second element is silently ignored. I'm returning -EEXIST from
> > > nft_pipapo_insert(), but nft_add_set_elem() clears it because NLM_F_EXCL
> > > is not set.
> > >
> > > Are you suggesting that this is consistent and therefore not a problem?
> >
> > NLM_F_EXCL !NLM_F_EXCL
> > exact match EEXIST 0 [*]
> > partial match EEXIST EEXIST
> >
> > The [*] case would allow for element timeout/expiration updates from
> > the control plane for exact matches.
>
> A-ha. I didn't even consider that.
>
> > Note that element updates are not
> > supported yet, so this check for !NLM_F_EXCL is a stub. I don't think
> > we should allow for updates on partial matches
> >
> > I think what it is missing is a error to report "partial match" from
> > pipapo. Then, the core translates this "partial match" error to EEXIST
> > whether NLM_F_EXCL is set or not.
>
> Yes, given what you explained, I also think it's the case.
>
> > Would this work for you?
>
> It would. I need to write a few more lines in nft_pipapo_insert(),
> because right now I don't have a special case for "entirely
> overlapping". Something on the lines of:
>
> dup = pipapo_get(net, set, start, genmask);
> if (PTR_ERR(dup) == -ENOENT) {
>
> --> compare start and end key for this entry with
> start and end key from 'ext'
>
> Let me know if you want me to post a patch with a placeholder for
> whatever you have in mind, or if I can help implementing this, etc.
Please, go ahead with the placeholder, it might be faster. I'll jump
on it.
