All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>, netfilter-devel@vger.kernel.org
Subject: Re: [PATCH nft 0/8] mptcp subtype option match support
Date: Wed, 1 Dec 2021 12:30:10 +0100	[thread overview]
Message-ID: <20211201113010.GC2315@breakpoint.cc> (raw)
In-Reply-To: <Yaabbty7g8XyeMs/@salvia>

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Nov 23, 2021 at 02:37:42PM +0100, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Fri, Nov 19, 2021 at 04:28:39PM +0100, Florian Westphal wrote:
> > > > Because the subtype is only 4 bits in size the exthdr
> > > > delinearization needs a fixup to remove the binop added by the
> > > > evaluation step.
> > > 
> > > By the bitwise operation to take the 4 bits you can infer this refers to
> > > mptcp, but it might be good to store in the rule userdata area that this
> > > expression refers to mptcp as a suggestion to userspace when
> > > delinearizing the rule.
> > 
> > Why?  Userspace has all info: its a tcp option, we have the tcp option
> > number (mptcp) and a 4 bit field which matches the template field.
> > 
> > There is no guesswork here.
> 
> OK, thanks for explaining.

I can add a commet before pushing this out.

All mptcp suboptions have this 4bit identifier at the same location,
so

'tcp option == mptcp' always allows to take those 4 bit to identify
the next subheader.

The only caveat is that some suboptions have different sizes depending
on the option length field, this will get more complicated once matching
on those is added (we will need to take moer dependencies into account).

> > > > One remaining usablility problem is the lack of mnemonics for the
> > > > subtype, i.e. something like:
> > > > 
> > > > static const struct symbol_table mptcp_subtype_tbl = {
> > > >        .base           = BASE_DECIMAL,
> > > >        .symbols        = {
> > > >                SYMBOL("mp-capable",    0),
> > > >                SYMBOL("mp-join",       1),
> > > >                SYMBOL("dss",           2),
> > > >                SYMBOL("add-addr",      3),
> > > >                SYMBOL("remove-addr",   4),
> > > >                SYMBOL("mp-prio",       5),
> > > >                SYMBOL("mp-fail",       6),
> > > >                SYMBOL("mp-fastclose",  7),
> > > >                SYMBOL("mp-tcprst",     8),
> > > >                SYMBOL_LIST_END
> > > >        },
> > > > 
> > > > ... but this would need addition of yet another data type.
> > > >
> > > > Use of implicit/context-dependent symbol table would
> > > > be preferrable, I will look into this next.
> > > 
> > > Could you develop your idea?
> > 
> > No idea, I have not thought about it at all.  I do not want
> > to add a new data type for this.
> > 
> > One way is to just extend the scanner with even more keywords.
> > I tought I could annotate the eval context with 'saw mptcp'
> > and then use that when trying to resolve the symbol expressions.
> > 
> > No idea if that works and no idea how much hassle that is.
> 
> You can probably use a string in the parser for these types, then
> invoke the symbol_table lookup from the parser to fetch the value?

Current solution I'm working on adds a phony integer type, similar
to the 'hex output' one.

Delinearization needs to do some extra steps to override the integer
again though.

I will share some draft patches soon.

  reply	other threads:[~2021-12-01 11:30 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-19 15:28 [PATCH nft 0/8] mptcp subtype option match support Florian Westphal
2021-11-19 15:28 ` [PATCH nft 1/8] tcpopt: remove KIND keyword Florian Westphal
2021-11-19 15:28 ` [PATCH nft 2/8] scanner: add tcp flex scope Florian Westphal
2021-11-19 15:28 ` [PATCH nft 3/8] parser: split tcp option rules Florian Westphal
2021-11-19 15:28 ` [PATCH nft 4/8] tcpopt: add md5sig, fastopen and mptcp options Florian Westphal
2021-11-19 15:28 ` [PATCH nft 5/8] tests: py: add test cases for md5sig, fastopen and mptcp mnemonics Florian Westphal
2021-11-19 15:28 ` [PATCH nft 6/8] mptcp: add subtype matching Florian Westphal
2021-11-19 15:28 ` [PATCH nft 7/8] exthdr: fix tcpopt_find_template to use length after mask adjustment Florian Westphal
2021-11-19 15:28 ` [PATCH nft 8/8] tests: py: add tcp subtype match test cases Florian Westphal
2021-11-23 13:16 ` [PATCH nft 0/8] mptcp subtype option match support Pablo Neira Ayuso
2021-11-23 13:37   ` Florian Westphal
2021-11-30 21:45     ` Pablo Neira Ayuso
2021-12-01 11:30       ` Florian Westphal [this message]
2021-12-02 21:42         ` 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=20211201113010.GC2315@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.