Re: [nft PATCH 4/7] cli: Use nft_run_cmd_from_buffer()

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Fri, Oct 20, 2017 at 07:10:18PM +0200, Phil Sutter wrote:
> On Fri, Oct 20, 2017 at 02:15:34PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Oct 19, 2017 at 10:18:44AM +0200, Phil Sutter wrote:
> > > This simplifies CLI code and allows to reduce libnftables API by not
> > > exporting nft_run().
> > > 
> > > Since nft_run_cmd_from_buffer() takes care of scanner initialization and
> > > libmnl socket passed to cli_init() is present as nft_ctx field as well,
> > > signature of cli_init() can be reduced to just take nft_ctx pointer as
> > > single argument.
> > 
> > libmnl socket is indeed in nft_ctx, but we're planning a mode that
> > allows to expose the mnl_socket for advanced handling. In that
> > scenario, nft->nf_sock will be null.
> > 
> > So I would prefer we don't do changes that we have to undo once the
> > advanced API is in place.
> 
> IMHO this doesn't contradict what the patch does. Right now we only have
> the "simple API", and the patch changes src/cli.c to use just that. CLI
> code doesn't need anything which is not fulfilled by simple API at this
> point, so I'd say changing it to use advanced API should be done when we
> implement features (e.g. transaction control) there.
> 
> What do you think?

I have no strong objection against this, I just would like we don't
lose track of the high level API, and that one will need to expose the
netlink socket. So all these calls we will end up needed the nf_sock
parameter again at some point.

I don't have any strong opinion against this, just an observation.