| Message ID | 20211119152847.18118-1-fw@strlen.de |
|---|---|
| Headers | show |
| Series | mptcp subtype option match support | expand |
On Fri, Nov 19, 2021 at 04:28:39PM +0100, Florian Westphal wrote: > This series adds 'tcp option mptcp subtype' matching to nft. LGTM. > 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. I wanted to look into this for a different usecase. > 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? Thanks.
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. Without this patch, we're asked to find a matching 1-byte field (which does not exist). Whats different vs. payload expressions here to require annotations? > > 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.
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. > Without this patch, we're asked to find a matching 1-byte field > (which does not exist). > > Whats different vs. payload expressions here to require annotations? > > > > 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?
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.
On Wed, Dec 01, 2021 at 12:30:10PM +0100, Florian Westphal wrote: > 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. Please add a comment and push out this. > '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). This might require to extend the dependency infrastructure. > > > > > 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. Sounds good, please consider the typeof path for set/map too.
This series adds 'tcp option mptcp subtype' matching to nft. Because the subtype is only 4 bits in size the exthdr delinearization needs a fixup to remove the binop added by the evaluation step. 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. Florian Westphal (8): tcpopt: remove KIND keyword scanner: add tcp flex scope parser: split tcp option rules tcpopt: add md5sig, fastopen and mptcp options tests: py: add test cases for md5sig, fastopen and mptcp mnemonics mptcp: add subtype matching exthdr: fix tcpopt_find_template to use length after mask adjustment tests: py: add tcp subtype match test cases doc/payload-expression.txt | 29 ++++--- include/parser.h | 1 + include/tcpopt.h | 13 ++- src/exthdr.c | 46 +++++----- src/parser_bison.y | 108 +++++++++++++++++------ src/scanner.l | 22 +++-- src/tcpopt.c | 38 +++++++- tests/py/any/tcpopt.t | 21 +++-- tests/py/any/tcpopt.t.json | 159 +++++++++++++++++++++++++--------- tests/py/any/tcpopt.t.payload | 64 ++++++++++---- 10 files changed, 360 insertions(+), 141 deletions(-)