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] src: allow update of net base w. meta l4proto icmpv6
Date: Fri, 24 Mar 2017 12:50:21 +0100	[thread overview]
Message-ID: <20170324115021.GA2515@salvia> (raw)
In-Reply-To: <20170322192252.GC8584@breakpoint.cc>

On Wed, Mar 22, 2017 at 08:22:52PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Wed, Mar 22, 2017 at 04:44:00PM +0100, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > Hm, I wonder why you need this new line in proto_inet_service:
> > > > 
> > > > +         PROTO_LINK(IPPROTO_ICMPV6,      &proto_icmp6),
> > > 
> > > meta_expr_pctx_update calls proto_find_upper(), without this
> > > that returns NULL and proto base is set to 'unknown'.
> > 
> > Oh right.
> > 
> > Will this still happen if you tell nft to generate the dependency
> > using meta l4proto instead of ip6 nexthdr?
> 
> Yes, tried with
> 
> src/nft add rule ip6 f i meta l4proto ipv6-icmp icmpv6 type nd-router-advert
>  <cmdline>:1:41-51: Error: conflicting protocols specified: unknown vs.  icmpv6
> 
> and this patch:
> 
> diff --git a/src/proto.c b/src/proto.c
> --- a/src/proto.c
> +++ b/src/proto.c
> @@ -707,7 +707,7 @@ const struct proto_desc proto_icmp6 = {
>  const struct proto_desc proto_ip6 = {
>         .name           = "ip6",
>         .base           = PROTO_BASE_NETWORK_HDR,
> -       .protocol_key   = IP6HDR_NEXTHDR,
> +       .protocol_key   = IP6HDR_INVALID,

In order spots, we just remove this line given IP6HDR_INVALID is zero.
I think this may be confusing to newcomers reading the code.

>         .protocols      = {
>                 PROTO_LINK(IPPROTO_ESP,         &proto_esp),
>                 PROTO_LINK(IPPROTO_AH,          &proto_ah),
> @@ -720,6 +720,7 @@ const struct proto_desc proto_ip6 = {
>                 PROTO_LINK(IPPROTO_ICMPV6,      &proto_icmp6),
>         },
>         .templates      = {
> +               [IP6HDR_INVALID]        = PROTO_META_TEMPLATE("nfproto", &inet_protocol_type, NFT_META_L4PROTO, 8),

We can just use NFT_META_L4PROTO all the time, so we use it from IPv4
too, right?

And use:

        [0]     = PROTO_META_TEMPLATE(...)

for consistency.

  reply	other threads:[~2017-03-24 11:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 18:54 [PATCH nft] src: allow update of net base w. meta l4proto icmpv6 Florian Westphal
2017-03-22 13:09 ` Pablo Neira Ayuso
2017-03-22 13:44   ` Florian Westphal
2017-03-22 15:29     ` Pablo Neira Ayuso
2017-03-22 15:32       ` Pablo Neira Ayuso
2017-03-22 15:44         ` Florian Westphal
2017-03-22 16:07           ` Pablo Neira Ayuso
2017-03-22 19:22             ` Florian Westphal
2017-03-24 11:50               ` Pablo Neira Ayuso [this message]
2017-03-24 12:21                 ` Florian Westphal
2017-03-29 10:13                   ` Pablo Neira Ayuso

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=20170324115021.GA2515@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).