libnftables, next steps

libnftables, next steps

[Phil Sutter]

Hi!

I rebased Eric's libnftables patch series onto current master to get an
overview of what's still missing (and what I could work on :). Here's
what I collected:

* Implement application accessible batch support.
  -> This basically splits nft_run() into stages.
  -> I would change nft_run_cmd_from_*() to use this internally.
  -> Do we want this in the early library version or is this going to be
     part of the 'advanced API' to add later?

* Add erec_free_list().
  -> This becomes handy if the application wants to drop erec list
     without printing it (erec_print_list() clears the list while
     traversing it).
  -> No use for this if we only export nft_run_cmd_from_*() functions.

* Create src/nftables_common.c and include/nftables_common.h to hold
  nft_run() and nft_netlink().
  -> Is this meant as the (not exported) high-level library backend?
  -> If batch support is implemented, these could be removed after
     changing nft_run_cmd_from_*() and cli_complete() to use it.

* Move library routines from src/main.c into src/libnftables.c and
  create include/nftables/nftables.h to hold the signatures.

* Introduce the library (i.e., generate libnftables.so).

Some additional thoughts:

* Should we support different output streams for debug and/or error
  messages?

* Should we reuse src/erec.c for regular output as well? (This probably
  needs a 'print immediately' switch for monitor mode, though.)

Feedback highly appreciated, of course! Should I start with moving the
library stuff into libnftables.{c,h} so we get an impression of what the
API will look like?

Cheers, Phil

Re: libnftables, next steps

[Pablo Neira Ayuso]

Hi Phil,

Sorry it took a while to get back to you.

On Thu, Oct 05, 2017 at 12:51:52AM +0200, Phil Sutter wrote:
> Hi!
> 
> I rebased Eric's libnftables patch series onto current master to get an
> overview of what's still missing (and what I could work on :). Here's
> what I collected:
> 
> * Implement application accessible batch support.
>   -> This basically splits nft_run() into stages.
>   -> I would change nft_run_cmd_from_*() to use this internally.
>   -> Do we want this in the early library version or is this going to be
>      part of the 'advanced API' to add later?

I would leave this behind. Let's just start with the most simple API,
then we move on.

> * Add erec_free_list().
>   -> This becomes handy if the application wants to drop erec list
>      without printing it (erec_print_list() clears the list while
>      traversing it).
>
>   -> No use for this if we only export nft_run_cmd_from_*() functions.

OK, so this is part of the advanced API then.

> * Create src/nftables_common.c and include/nftables_common.h to hold
>   nft_run() and nft_netlink().

Why not just place this in src/libnftables.c?

>   -> Is this meant as the (not exported) high-level library backend?
>   -> If batch support is implemented, these could be removed after
>      changing nft_run_cmd_from_*() and cli_complete() to use it.
> 
> * Move library routines from src/main.c into src/libnftables.c and
>   create include/nftables/nftables.h to hold the signatures.
> 
> * Introduce the library (i.e., generate libnftables.so).
> 
> Some additional thoughts:
> 
> * Should we support different output streams for debug and/or error
>   messages?

What usecase you have in mind for this?

> * Should we reuse src/erec.c for regular output as well? (This probably
>   needs a 'print immediately' switch for monitor mode, though.)

Again, same question.

> Feedback highly appreciated, of course! Should I start with moving the
> library stuff into libnftables.{c,h} so we get an impression of what the
> API will look like?

I think Eric doesn't have time at this stage, so if you can take his
patches, revamp and resubmit, that would be great.

Thanks!

Re: libnftables, next steps

[Phil Sutter]

Hi Pablo,

On Mon, Oct 16, 2017 at 12:19:51PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Oct 05, 2017 at 12:51:52AM +0200, Phil Sutter wrote:
[...]
> > * Create src/nftables_common.c and include/nftables_common.h to hold
> >   nft_run() and nft_netlink().
> 
> Why not just place this in src/libnftables.c?

Sounds reasonable. I was merely wondering why Eric chose to distribute
things this way.

[..]
> > Some additional thoughts:
> > 
> > * Should we support different output streams for debug and/or error
> >   messages?
> 
> What usecase you have in mind for this?

This was just me speculating about advanced API features. Assuming that
in future, applications would be able to replicate nft_run_cmd_from_*()
functions in their own code, it might be useful for them to configure
different streams for debug/error messages to treat them differently
from "regular" output. But I guess we best just ignore that until
someone actually provides a use-case for it.

> > * Should we reuse src/erec.c for regular output as well? (This probably
> >   needs a 'print immediately' switch for monitor mode, though.)
> 
> Again, same question.

This was just an idea as well, no actual use-case in mind. The nice
thing about error records is that they may be typed (erec->type) and
separate messages stay separate. Right now, applications get everything
in a single buffer and using erec would allow them to distinguish
between different messages without parsing for newlines or the like. But
yeah, maybe just keep in mind for later.

> > Feedback highly appreciated, of course! Should I start with moving the
> > library stuff into libnftables.{c,h} so we get an impression of what the
> > API will look like?
> 
> I think Eric doesn't have time at this stage, so if you can take his
> patches, revamp and resubmit, that would be great.

Will do, thanks for your feedback!

Cheers, Phil