Re: libnftables extended API proposal

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Tue, Jan 02, 2018 at 07:02:19PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Dec 29, 2017 at 03:58:16PM +0100, Phil Sutter wrote:
> > On Thu, Dec 28, 2017 at 08:21:41PM +0100, Pablo Neira Ayuso wrote:
> > > On Sat, Dec 23, 2017 at 02:19:41PM +0100, Phil Sutter wrote:
[...]
> > > > But isn't the problem of keeping the API compatible comparable to
> > > > the problem of keeping the JSON representation compatible?
> > > 
> > > Well, keeping backward compatibility is always a burden to carry on.
> > > Even though, to me, JSON is as extensible as netlink is, ie. we can
> > > add new keys and deprecate things without breaking backward.
> > 
> > Yes, the format is very flexible. But how does that help with
> > compatibility? You'll have to support the deprecated attributes or JSON
> > without the new attributes either way, no?
> 
> Probably it's my subjective judging that maintaing json layout will be
> easier rather than a large bunch of APIs and internal object
> representations through getter/setter.
> 
> Anyway, these days, we expect people to use modern languages to build
> upper layers in the stack, right? And that seems to mix well with JSON.
> Again core infrastructure person here talking about upper layers, so
> take this lightly ;-).

Yeah, for firewalld at least JSON is not a disadvantage. Not sure about
projects written in C though, but that's a different kettle of fish. :)

> [...]
> > > Oh, I can help you on that. Although you're very much closer to
> > > firewalld usecases that I'm, so probably a draft from you on this
> > > would be nice ;-)
> > 
> > I went ahead and converted my local ruleset into JSON manually (until I
> > got bored), see attached ruleset.json. Then I wrote a schema to validate
> > it (also attached). Please let me know if you're OK with the base
> > format at least, i.e. everything except expressions and statements. Any
> > feedback on the few statements and expressions I put in there is highly
> > appreciated as well, of course! :)
> 
> Probably instead of having left and right, we can replace it by:
> 
> "match" : {
>         "key": {
>                 ...
>         },
>         "value": "lo"
> }
> 
> Then, allow to place expressions as "value" when it comes from a set
> lookup.

Yes, JSON Schema allows to define multiple possible types for an
attribute (see #/definitions/expression/val for instance). But I don't
follow regarding set lookup: There are other uses for an expression on
RHS as well, no?

> > > > > Regarding asynchronism between input and output, not sure I follow.
> > > > 
> > > > I am grepping through tests/py/*/*.t for pattern 'ok;':
> > > > 
> > > > - Adjacent ranges are combined.
> > > 
> > > We should disable this, there was some discussion on this recently.
> > > User should request this explicitly to happen through some knob.
> > 
> > I agree. While it's a nice idea, adding two adjacent ranges and later
> > wanting to delete one of them is troublesome. Do you have a name in mind
> > for that knob? Maybe something more generic which could cover other
> > cases of overly intelligent nft (in the future) as well?
> 
> Probably "coalesce" or "merge".

So you'd prefer a specific flag just for that feature? I had a more
generic one in mind, something like "optimize" for instance.

[...]
> > > > - meta iif "lo" accept;ok;iif "lo" accept
> > > >   -> Maybe less abstraction?
> > > 
> > > This is just dealing with something that is causing us problems, that
> > > is, iif is handled as primary key, so we cannot reuse it in the
> > > grammar given it results in shift-reduce conflicts.
> > 
> > The question is why allow both variants then? Since 'iif' is being used
> > as fib_flag as well, using 'iif' alone should be deprecated. Or is this
> > a case of backwards compatibility?
> 
> It was compromise solution, not to break syntax all of a sudden,
> allowing old and new ways for a little while. But this one, I think it
> was not add this.

I couldn't parse your last sentence here. :)

> > > > - tcp dport 22 iiftype ether ip daddr 1.2.3.4 ether saddr 00:0f:54:0c:11:4 accept ok;tcp dport 22 ether saddr 00:0f:54:0c:11:04 ip daddr 1.2.3.4 accept
> > > >   -> Here something is "optimized out", not sure if it should be kept in
> > > >   JSON.
> > > 
> > > This is testing redudant information, that we can remove given we can
> > > infer it.
> > 
> > Yeah, similar situation to the 'meta l4proto' one above. The (still)
> > tricky part is to communicate assigned handles back to the application.
> > Maybe we could return an exact copy of their JSON with just handle
> > properties added?
> 
> Yes, that should be fine. We already have the NLM_F_ECHO in place,
> just a matter of supporting json there.
> 
> BTW, a simple testsuite for this would be good too.

Sure! Maybe the existing data in tests/py could be reused (the *.payload
files at least).

> P.S: Happy new year BTW.

Thanks, likewise! :)

Cheers, Phil