netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Florian Westphal <fw@strlen.de>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [PATCH nft 0/3] support ct/meta key lookups at runtime
Date: Thu, 27 Oct 2016 19:16:12 +0200	[thread overview]
Message-ID: <20161027171612.GA20304@salvia> (raw)
In-Reply-To: <20161027165843.GD23011@breakpoint.cc>

On Thu, Oct 27, 2016 at 06:58:43PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Oct 27, 2016 at 06:48:10PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Oct 27, 2016 at 12:36:05AM +0200, Florian Westphal wrote:
> > > > I have a patch series to add support of hash (skb hash) and sym hash
> > > > (symmetric skb hash) to nft_meta.
> > > > 
> > > > This series would allow extending meta without adding new hash/symhash
> > > > keywords in the scanner.
> > > > 
> > > > What do you think?
> > > 
> > > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > > 
> > > Only one suggestion, instead of:
> > > 
> > >  return error(loc, "Could not parse %s, known ct keys are: %s", str, buf);
> > > 
> > > Looking at current error reporting:
> > > 
> > >  # nft add rule x y ct
> > >  <cmdline>:1:18-18: Error: syntax error, unexpected newline
> > >  add rule x y ct
> > >                 ^
> > > Probably something like looks better:
> > > 
> > >  "syntax error, unexpected %s, known keys are %s"
> > 
> > Actually, if we follow this approach, we probably have to revisit all
> > other existing error messages...
> > 
> > Keep this as it is, we can revisit this later.
> 
> Are you sure?  I like the suggestion.

Go ahead with it then, we can just make a follow up patch to revisit
other existing spots. Probably we can add a new function so we don't
need to hardcode this everywhere in the code?

We will get one Outreachy student soon btw, we can hand over this task
to her.

      reply	other threads:[~2016-10-27 17:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-26 22:36 [PATCH nft 0/3] support ct/meta key lookups at runtime Florian Westphal
2016-10-26 22:36 ` [PATCH nft 1/3] utils: provide snprintf helper macro Florian Westphal
2016-10-26 22:36 ` [PATCH nft 2/3] ct: allow resolving ct keys at run time Florian Westphal
2016-10-26 22:36 ` [PATCH nft 3/3] meta: allow resolving meta " Florian Westphal
2016-10-27 16:48 ` [PATCH nft 0/3] support ct/meta key lookups at runtime Pablo Neira Ayuso
2016-10-27 16:51   ` Pablo Neira Ayuso
2016-10-27 16:58     ` Florian Westphal
2016-10-27 17:16       ` Pablo Neira Ayuso [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=20161027171612.GA20304@salvia \
    --to=pablo@netfilter.org \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    /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).