netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Phil Sutter <phil@nwl.cc>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Florian Westphal <fw@strlen.de>, Eric Leblond <eric@regit.org>,
	netfilter-devel@vger.kernel.org
Subject: Re: [nft PATCH v2 2/2] src: get rid of printf
Date: Thu, 21 Sep 2017 19:05:58 +0200	[thread overview]
Message-ID: <20170921170558.GE15946@breakpoint.cc> (raw)
In-Reply-To: <20170921162108.GY30364@orbyte.nwl.cc>

Phil Sutter <phil@nwl.cc> wrote:
> On Thu, Sep 21, 2017 at 05:51:12PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Sep 21, 2017 at 05:43:09PM +0200, Florian Westphal wrote:
> > > Phil Sutter <phil@nwl.cc> wrote:
> > > > With the proposed implementation of nft_print(), this will be
> > > > problematic: nft_run_cmd_from_buffer() waits for the command to finish
> > > > before printing all the output at once. This obviously breaks monitor
> > > > which runs endlessly.
> > > > 
> > > > Maybe we should reference nft_print via a function pointer in
> > > > output_ctx? This would allow to configure a different nft_print
> > > > implementation which flushes the buffer to stdout immediately.
> > > > 
> > > > What do you think?
> > > 
> > > I wondered the same thing.
> > > 
> > > I have following issue: When using new typeof keyword then listing
> > > a set definition should output something like
> > > 
> > > set foo { type typeof(meta iifname) }
> > > 
> > > Because the kernel has no notion of 'meta iifname', we have to
> > > store this information in the kernel so we can read it back during
> > > delinearization.
> > > 
> > > One way to do this would be to store the 'meta iifname' string in the
> > > sets userdata.
> > > 
> > > For that, expr_print() would have to be able to print to a buffer
> > > (or we would need an expr_snprintf or something similar) to extract
> > > the convert struct *expr back to its original name.
> > > 
> > > The other solution would be to stash this in the expression
> > > during parsing but that seems weird as we need to be able to do such
> > > conversion anyway when printing the ruleset, so we merely need
> > > to make this accessible outside of plain printf() to stdout.
> > 
> > Probably the underlying problem is that the monitor code is that not
> > delinearing, ie. transforming from netlink to abstract syntax tree
> > (ast) before printing?
> 
> No, we are talking about different issues (but with a potential common
> solution):
> 
> Florian searches for a way to "convert" an expression from it's binary
> form (a struct expr) into a human readable string (here: "meta
> iifname"). We have this "converter" already in very limited form, namely
> expr_print(). Eric's patch changes that function to make use of
> nft_print(), which effectively allows to define a custom buffer to print
> into.
> 
> Florian, did I get this right?

Yes, thats right.

(Pablo, I do not NEED this function, but, as far is I understood
 you want nft to list

 set name { type typeof(ip saddr) }

back exactly like this, keeping the 'typeof'.  And AFAICS the best
way to do this is to store the human readable "ip saddr" as-is in the
userdata area and then just print that back.

If I misunderstood what you were saying, please clarify.

Ultimately something is needed to annotate in any case because in case
of 'meta iifname . ipv4_addr' we don't have anything to go on when deliarizing as
the string type lacks any length information and we don't have any rule
context from set definition to infer the needed key length information
of the interface name.

      reply	other threads:[~2017-09-21 17:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-03 22:03 [nft PATCH 0/2] libnftables preparation work Eric Leblond
2017-09-03 22:03 ` [nft PATCH 1/2] src: add flags fo nft_ctx_new Eric Leblond
2017-09-03 22:33   ` Pablo Neira Ayuso
2017-09-03 22:45     ` Pablo Neira Ayuso
2017-09-04  7:21       ` Eric Leblond
2017-09-03 22:03 ` [nft PATCH 2/2] src: get rid of printf Eric Leblond
2017-09-03 22:34   ` Pablo Neira Ayuso
2017-09-04  7:45     ` Eric Leblond
2017-09-04  7:55     ` [nft PATCH v2] libnftables preparation work Eric Leblond
2017-09-04  7:55       ` [nft PATCH v2 1/2] src: add flags fo nft_ctx_new Eric Leblond
2017-09-04 20:43         ` Pablo Neira Ayuso
2017-09-04  7:55       ` [nft PATCH v2 2/2] src: get rid of printf Eric Leblond
2017-09-04 20:43         ` Pablo Neira Ayuso
2017-09-04 20:53           ` Pablo Neira Ayuso
2017-09-04 21:23             ` Eric Leblond
2017-09-05 17:33               ` Pablo Neira Ayuso
2017-09-21 15:37             ` Phil Sutter
2017-09-21 15:43               ` Florian Westphal
2017-09-21 15:51                 ` Pablo Neira Ayuso
2017-09-21 16:21                   ` Phil Sutter
2017-09-21 17:05                     ` Florian Westphal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170921170558.GE15946@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=eric@regit.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=phil@nwl.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).