Message ID | 20200407180124.19169-1-ydahhrk@gmail.com |
---|---|
State | RFC |
Delegated to: | Pablo Neira |
Headers | show |
Series | [libnftnl,1/2] expr: add jool support | expand |
On Tue, Apr 7, 2020 at 8:03 PM Alberto Leiva Popper <ydahhrk@gmail.com> wrote: > > Jool statements are used to send packets to the Jool kernel module, > which is an IP/ICMP translator: www.jool.mx > > Sample usage: > > modprobe jool > jool instance add "name" --iptables -6 64:ff9b::/96 > sudo nft add rule inet table1 chain1 jool nat64 "name" > Hi Alberto, Looking at the code, the pool4db is pretty much an adaptation of what conntrack already does. So, why not to put the efforts in extending conntrack to support NAT64/NAT46 ? This way, the support of this natting is likely to be included in the kernel vanilla and just configure it with just one rule: sudo nft add rule inet table1 chain1 dnat 64 64:ff9b::/96 One more thing, it seems that jool only supports PREROUTING, is that right? Cheers.
> Looking at the code, the pool4db is pretty much an adaptation of what > conntrack already does. So, why not to put the efforts in extending > conntrack to support NAT64/NAT46 ? Ok, please don't take this as an aggressively defensive gesture, but I feel like this is an unfair question. If I provide a ready and simple but effective means to bridge our projects I feel like it befalls on you to justify why you wish to commit to the far more troublesome course of action. Merging the projects seems to me like several (if not many) months worth of development and testing, little of which would be made in benefit of our users. (No real functionality would be added, and some functionality might be dropped--eg. atomic configuration, session synchronization.) I mean I get that you want to avoid some duplicate functionality, but is this really a more important use of my time than, say, adding MAP-T support? ([0]) > This way, the support of this natting is likely to be included in the > kernel vanilla and just configure it with just one rule: > > sudo nft add rule inet table1 chain1 dnat 64 64:ff9b::/96 Ok, but I don't think an IP translator is *meant* to be configured in a single line. Particularly in the case of NAT46. How do you populate a large EAM table ([1]) on a line? If your translator instance is defined entirely in a rule matched by IPv6 packets, how do you tell the corresponding IPv4 rule to refer to the same instance? It is my humble opinion that some level of separation between nftables rules and translator instances is clean design. > One more thing, it seems that jool only supports PREROUTING, is that right? Yes, although this might presently only be because nobody has asked elsewhat. I tried adding LOCAL_OUT support some years ago and forgot to write down the problems that prevented me from succeeding. I can give it another shot if this is important for you. Cheers, Alberto [0] https://tools.ietf.org/html/rfc7599 [1] https://jool.mx/en/eamt.html On Wed, Apr 8, 2020 at 2:22 PM Laura Garcia <nevola@gmail.com> wrote: > > On Tue, Apr 7, 2020 at 8:03 PM Alberto Leiva Popper <ydahhrk@gmail.com> wrote: > > > > Jool statements are used to send packets to the Jool kernel module, > > which is an IP/ICMP translator: www.jool.mx > > > > Sample usage: > > > > modprobe jool > > jool instance add "name" --iptables -6 64:ff9b::/96 > > sudo nft add rule inet table1 chain1 jool nat64 "name" > > > > Hi Alberto, > > Looking at the code, the pool4db is pretty much an adaptation of what > conntrack already does. So, why not to put the efforts in extending > conntrack to support NAT64/NAT46 ? > > This way, the support of this natting is likely to be included in the > kernel vanilla and just configure it with just one rule: > > sudo nft add rule inet table1 chain1 dnat 64 64:ff9b::/96 > > One more thing, it seems that jool only supports PREROUTING, is that right? > > Cheers.
Ok, my point of view has changed. Sorry; I'm trying to reconcile the interests of different parties. Here's the thing: Creating a bridge between nftables and Jool, and merging Jool into nftables, are (from my perspective) separate tasks. The former is simple, the latter is longterm. My current pull request attempts to address the former, but not the latter. The former task is [0], and the latter is [1]. According to Jool's feature survey ([2]), a majority of my opinionated users want the nftables/Jool bridge, another majority wants me to implement MAP-T. Unfortunately, I lack the number of users who want me to perform a formal merge between nftables and Jool, but I presume that it has diminished ever since Jool was packaged for different distributions. (Since installation is no longer a daunting task.) However, I do acknowledge that merging Jool into nftables is the right course of action, and I would like to commit in this direction. So I propose the following order of events: 1. Merge the bridge into nftables. This will give a temporary early solution for those who want it. 2. Implement MAP-T. 3. Merge Jool into nftables. 4. Deprecate the bridge, and eventually remove it. Is this an acceptable compromise? Alberto [0] https://github.com/NICMx/Jool/issues/285 [1] https://github.com/NICMx/Jool/issues/273 [2] https://docs.google.com/forms/d/e/1FAIpQLSe_9_wBttFGd9aJ7lKXiJvIN7wWZm_C6yy3gU0Ttepha275nQ/viewanalytics On Wed, Apr 15, 2020 at 4:41 PM Alberto Leiva <ydahhrk@gmail.com> wrote: > > > Looking at the code, the pool4db is pretty much an adaptation of what > > conntrack already does. So, why not to put the efforts in extending > > conntrack to support NAT64/NAT46 ? > > Ok, please don't take this as an aggressively defensive gesture, but I > feel like this is an unfair question. > > If I provide a ready and simple but effective means to bridge our > projects I feel like it befalls on you to justify why you wish to > commit to the far more troublesome course of action. > > Merging the projects seems to me like several (if not many) months > worth of development and testing, little of which would be made in > benefit of our users. (No real functionality would be added, and some > functionality might be dropped--eg. atomic configuration, session > synchronization.) > > I mean I get that you want to avoid some duplicate functionality, but > is this really a more important use of my time than, say, adding MAP-T > support? ([0]) > > > This way, the support of this natting is likely to be included in the > > kernel vanilla and just configure it with just one rule: > > > > sudo nft add rule inet table1 chain1 dnat 64 64:ff9b::/96 > > Ok, but I don't think an IP translator is *meant* to be configured in > a single line. Particularly in the case of NAT46. How do you populate > a large EAM table ([1]) on a line? If your translator instance is > defined entirely in a rule matched by IPv6 packets, how do you tell > the corresponding IPv4 rule to refer to the same instance? > > It is my humble opinion that some level of separation between nftables > rules and translator instances is clean design. > > > One more thing, it seems that jool only supports PREROUTING, is that right? > > Yes, although this might presently only be because nobody has asked elsewhat. > > I tried adding LOCAL_OUT support some years ago and forgot to write > down the problems that prevented me from succeeding. I can give it > another shot if this is important for you. > > Cheers, > Alberto > > [0] https://tools.ietf.org/html/rfc7599 > [1] https://jool.mx/en/eamt.html > > On Wed, Apr 8, 2020 at 2:22 PM Laura Garcia <nevola@gmail.com> wrote: > > > > On Tue, Apr 7, 2020 at 8:03 PM Alberto Leiva Popper <ydahhrk@gmail.com> wrote: > > > > > > Jool statements are used to send packets to the Jool kernel module, > > > which is an IP/ICMP translator: www.jool.mx > > > > > > Sample usage: > > > > > > modprobe jool > > > jool instance add "name" --iptables -6 64:ff9b::/96 > > > sudo nft add rule inet table1 chain1 jool nat64 "name" > > > > > > > Hi Alberto, > > > > Looking at the code, the pool4db is pretty much an adaptation of what > > conntrack already does. So, why not to put the efforts in extending > > conntrack to support NAT64/NAT46 ? > > > > This way, the support of this natting is likely to be included in the > > kernel vanilla and just configure it with just one rule: > > > > sudo nft add rule inet table1 chain1 dnat 64 64:ff9b::/96 > > > > One more thing, it seems that jool only supports PREROUTING, is that right? > > > > Cheers.
On Wed, Apr 15, 2020 at 11:41 PM Alberto Leiva <ydahhrk@gmail.com> wrote: > > > Looking at the code, the pool4db is pretty much an adaptation of what > > conntrack already does. So, why not to put the efforts in extending > > conntrack to support NAT64/NAT46 ? > > Ok, please don't take this as an aggressively defensive gesture, but I > feel like this is an unfair question. > Sorry, but I don't get your point. What I meant is that both pool4db and conntrack are natting machines, so extending conntrack (which is already integrated in the kernel) with what pool4db does could be a good way to go. Anyway, please let me come back to the technical discussion. > If I provide a ready and simple but effective means to bridge our > projects I feel like it befalls on you to justify why you wish to > commit to the far more troublesome course of action. > > Merging the projects seems to me like several (if not many) months > worth of development and testing, little of which would be made in > benefit of our users. (No real functionality would be added, and some > functionality might be dropped--eg. atomic configuration, session > synchronization.) > Atomic configuration is already supported in nftables and extending conntrack all the security functionalities and session replication with conntrackd will be also available. > I mean I get that you want to avoid some duplicate functionality, but > is this really a more important use of my time than, say, adding MAP-T > support? ([0]) > > > This way, the support of this natting is likely to be included in the > > kernel vanilla and just configure it with just one rule: > > > > sudo nft add rule inet table1 chain1 dnat 64 64:ff9b::/96 > > Ok, but I don't think an IP translator is *meant* to be configured in > a single line. Particularly in the case of NAT46. How do you populate > a large EAM table ([1]) on a line? If your translator instance is > defined entirely in a rule matched by IPv6 packets, how do you tell > the corresponding IPv4 rule to refer to the same instance? > nft supports maps generation from user space, so something like this could be configured: table inet my_table { map my_eamt { type ipv4_addr : ipv6_addr; flags interval; elements = { 192.0.2.1/32 : 2001:db8:aaaa::5/128, 198.51.100.0/24 : 2001:db8:bbbb::/120, 203.0.113.8/29 : 2001:db8:cccc::/125 } } } And then, use this map to perform the nat rule: nft add rule inet my_table my_chain snat ip saddr to @my_eamt Currently, the map structure doesn't work cause the second item should be a singleton, but probably it can be fixed easily. > It is my humble opinion that some level of separation between nftables > rules and translator instances is clean design. > My humble opinion is that this model will be hard to accept after the great efforts done with nftables that joins different commands that were used in the age of iptables. > > One more thing, it seems that jool only supports PREROUTING, is that right? > > Yes, although this might presently only be because nobody has asked elsewhat. > > I tried adding LOCAL_OUT support some years ago and forgot to write > down the problems that prevented me from succeeding. I can give it > another shot if this is important for you. > My concern is that this can break the normalization of having source nat in the postrouting instead of in the prerouting phase. Note that integrating a new feature must ensure not breaking other subsystems. Cheers.
Ok. This looks doable. I expect to run into trouble along the way, but I don't have any more objections for now. Did you receive my second mail from that day? (https://marc.info/?l=netfilter-devel&m=158700165716521&w=2) I won't hold it against you if you refuse the bridge, I just need something to tell my users. On Sat, Apr 18, 2020 at 1:25 PM Laura Garcia <nevola@gmail.com> wrote: > > On Wed, Apr 15, 2020 at 11:41 PM Alberto Leiva <ydahhrk@gmail.com> wrote: > > > > > Looking at the code, the pool4db is pretty much an adaptation of what > > > conntrack already does. So, why not to put the efforts in extending > > > conntrack to support NAT64/NAT46 ? > > > > Ok, please don't take this as an aggressively defensive gesture, but I > > feel like this is an unfair question. > > > > Sorry, but I don't get your point. What I meant is that both pool4db > and conntrack are natting machines, so extending conntrack (which is > already integrated in the kernel) with what pool4db does could be a > good way to go. > > Anyway, please let me come back to the technical discussion. > > > If I provide a ready and simple but effective means to bridge our > > projects I feel like it befalls on you to justify why you wish to > > commit to the far more troublesome course of action. > > > > Merging the projects seems to me like several (if not many) months > > worth of development and testing, little of which would be made in > > benefit of our users. (No real functionality would be added, and some > > functionality might be dropped--eg. atomic configuration, session > > synchronization.) > > > > Atomic configuration is already supported in nftables and extending > conntrack all the security functionalities and session replication > with conntrackd will be also available. > > > I mean I get that you want to avoid some duplicate functionality, but > > is this really a more important use of my time than, say, adding MAP-T > > support? ([0]) > > > > > This way, the support of this natting is likely to be included in the > > > kernel vanilla and just configure it with just one rule: > > > > > > sudo nft add rule inet table1 chain1 dnat 64 64:ff9b::/96 > > > > Ok, but I don't think an IP translator is *meant* to be configured in > > a single line. Particularly in the case of NAT46. How do you populate > > a large EAM table ([1]) on a line? If your translator instance is > > defined entirely in a rule matched by IPv6 packets, how do you tell > > the corresponding IPv4 rule to refer to the same instance? > > > > nft supports maps generation from user space, so something like this > could be configured: > > table inet my_table { > map my_eamt { > type ipv4_addr : ipv6_addr; > flags interval; > elements = { 192.0.2.1/32 : 2001:db8:aaaa::5/128, > 198.51.100.0/24 : 2001:db8:bbbb::/120, > 203.0.113.8/29 : 2001:db8:cccc::/125 } > } > } > > And then, use this map to perform the nat rule: > > nft add rule inet my_table my_chain snat ip saddr to @my_eamt > > Currently, the map structure doesn't work cause the second item should > be a singleton, but probably it can be fixed easily. > > > It is my humble opinion that some level of separation between nftables > > rules and translator instances is clean design. > > > > My humble opinion is that this model will be hard to accept after the > great efforts done with nftables that joins different commands that > were used in the age of iptables. > > > > One more thing, it seems that jool only supports PREROUTING, is that right? > > > > Yes, although this might presently only be because nobody has asked elsewhat. > > > > I tried adding LOCAL_OUT support some years ago and forgot to write > > down the problems that prevented me from succeeding. I can give it > > another shot if this is important for you. > > > > My concern is that this can break the normalization of having source > nat in the postrouting instead of in the prerouting phase. Note that > integrating a new feature must ensure not breaking other subsystems. > > Cheers.
On Tue, Apr 28, 2020 at 2:29 AM Alberto Leiva <ydahhrk@gmail.com> wrote: > > Ok. This looks doable. I expect to run into trouble along the way, but > I don't have any more objections for now. > > Did you receive my second mail from that day? > (https://marc.info/?l=netfilter-devel&m=158700165716521&w=2) > I won't hold it against you if you refuse the bridge, I just need > something to tell my users. > Hi Alberto, Please don't get me wrong, I'm not refusing anything. Just expressing my opinion. The coreteam is who had to decide. I truly want NAT64/46 to be implemented in Netfilter but in a definitive way, not something temporary. I got your second email but I had nothing to add. I consider it very unlikely to integrate an API that you already know that could be obsolete anytime soon. Cheers.
Ok, understood. Thank you. On Tue, Apr 28, 2020 at 3:10 AM Laura Garcia <nevola@gmail.com> wrote: > > On Tue, Apr 28, 2020 at 2:29 AM Alberto Leiva <ydahhrk@gmail.com> wrote: > > > > Ok. This looks doable. I expect to run into trouble along the way, but > > I don't have any more objections for now. > > > > Did you receive my second mail from that day? > > (https://marc.info/?l=netfilter-devel&m=158700165716521&w=2) > > I won't hold it against you if you refuse the bridge, I just need > > something to tell my users. > > > > Hi Alberto, > > Please don't get me wrong, I'm not refusing anything. Just expressing > my opinion. The coreteam is who had to decide. > > I truly want NAT64/46 to be implemented in Netfilter but in a > definitive way, not something temporary. > > I got your second email but I had nothing to add. I consider it very > unlikely to integrate an API that you already know that could be > obsolete anytime soon. > > Cheers.
diff --git a/include/statement.h b/include/statement.h index 8fb459ca..8f4cb0fd 100644 --- a/include/statement.h +++ b/include/statement.h @@ -253,6 +253,18 @@ struct xt_stmt { extern struct stmt *xt_stmt_alloc(const struct location *loc); +enum nft_jool_type { + NFT_JOOL_SIIT = (1 << 0), + NFT_JOOL_NAT64 = (1 << 1), +}; + +struct jool_stmt { + enum nft_jool_type type; + const char *instance; +}; + +extern struct stmt *jool_stmt_alloc(const struct location *loc); + /** * enum stmt_types - statement types * @@ -281,6 +293,7 @@ extern struct stmt *xt_stmt_alloc(const struct location *loc); * @STMT_CONNLIMIT: connection limit statement * @STMT_MAP: map statement * @STMT_SYNPROXY: synproxy statement + * @STMT_JOOL: Jool statement */ enum stmt_types { STMT_INVALID, @@ -309,6 +322,7 @@ enum stmt_types { STMT_CONNLIMIT, STMT_MAP, STMT_SYNPROXY, + STMT_JOOL, }; /** @@ -374,6 +388,7 @@ struct stmt { struct flow_stmt flow; struct map_stmt map; struct synproxy_stmt synproxy; + struct jool_stmt jool; }; }; diff --git a/src/evaluate.c b/src/evaluate.c index fcc79386..592a3cc8 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -3353,6 +3353,7 @@ int stmt_evaluate(struct eval_ctx *ctx, struct stmt *stmt) case STMT_QUOTA: case STMT_NOTRACK: case STMT_FLOW_OFFLOAD: + case STMT_JOOL: return 0; case STMT_EXPRESSION: return stmt_evaluate_expr(ctx, stmt); diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 79efda12..58a802f8 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -1106,6 +1106,20 @@ out_err: xfree(stmt); } +static void netlink_parse_jool(struct netlink_parse_ctx *ctx, + const struct location *loc, + const struct nftnl_expr *nle) +{ + struct stmt *stmt; + + stmt = jool_stmt_alloc(loc); + stmt->jool.type = nftnl_expr_get_u8(nle, NFTNL_EXPR_JOOL_TYPE); + stmt->jool.instance = xstrdup(nftnl_expr_get_str(nle, + NFTNL_EXPR_JOOL_INSTANCE)); + + ctx->stmt = stmt; +} + static void netlink_parse_synproxy(struct netlink_parse_ctx *ctx, const struct location *loc, const struct nftnl_expr *nle) @@ -1594,6 +1608,7 @@ static const struct { { .name = "flow_offload", .parse = netlink_parse_flow_offload }, { .name = "xfrm", .parse = netlink_parse_xfrm }, { .name = "synproxy", .parse = netlink_parse_synproxy }, + { .name = "jool", .parse = netlink_parse_jool }, }; static int netlink_parse_expr(const struct nftnl_expr *nle, diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c index e70e63b3..c0597f39 100644 --- a/src/netlink_linearize.c +++ b/src/netlink_linearize.c @@ -1434,6 +1434,19 @@ static void netlink_gen_meter_stmt(struct netlink_linearize_ctx *ctx, nftnl_rule_add_expr(ctx->nlr, nle); } +static void netlink_gen_jool_stmt(struct netlink_linearize_ctx *ctx, + const struct stmt *stmt) +{ + struct nftnl_expr *nle; + + nle = alloc_nft_expr("jool"); + + nftnl_expr_set_u8(nle, NFTNL_EXPR_JOOL_TYPE, stmt->jool.type); + nftnl_expr_set_str(nle, NFTNL_EXPR_JOOL_INSTANCE, stmt->jool.instance); + + nftnl_rule_add_expr(ctx->nlr, nle); +} + static void netlink_gen_stmt(struct netlink_linearize_ctx *ctx, const struct stmt *stmt) { @@ -1487,6 +1500,8 @@ static void netlink_gen_stmt(struct netlink_linearize_ctx *ctx, return netlink_gen_objref_stmt(ctx, stmt); case STMT_MAP: return netlink_gen_map_stmt(ctx, stmt); + case STMT_JOOL: + return netlink_gen_jool_stmt(ctx, stmt); default: BUG("unknown statement type %s\n", stmt->ops->name); } diff --git a/src/parser_bison.y b/src/parser_bison.y index 3e8d6bd6..0999172c 100644 --- a/src/parser_bison.y +++ b/src/parser_bison.y @@ -523,6 +523,8 @@ int nft_lex(void *, void *, void *); %token FULLY_RANDOM "fully-random" %token PERSISTENT "persistent" +%token JOOL "jool" + %token QUEUE "queue" %token QUEUENUM "num" %token BYPASS "bypass" @@ -642,6 +644,8 @@ int nft_lex(void *, void *, void *); %destructor { stmt_free($$); } tproxy_stmt %type <stmt> synproxy_stmt synproxy_stmt_alloc %destructor { stmt_free($$); } synproxy_stmt synproxy_stmt_alloc +%type <stmt> jool_stmt jool_stmt_alloc +%destructor { stmt_free($$); } jool_stmt jool_stmt_alloc %type <stmt> queue_stmt queue_stmt_alloc @@ -2492,6 +2496,7 @@ stmt : verdict_stmt | set_stmt | map_stmt | synproxy_stmt + | jool_stmt ; verdict_stmt : verdict_expr @@ -3014,6 +3019,32 @@ synproxy_sack : /* empty */ { $$ = 0; } } ; +jool_stmt : jool_stmt_alloc jool_opts + ; + +jool_stmt_alloc : JOOL + { + $$ = jool_stmt_alloc(&@$); + } + ; + +jool_opts : string string + { + if (!strcmp("siit", $1)) + $<stmt>0->jool.type = NFT_JOOL_SIIT; + else if (!strcmp("nat64", $1)) + $<stmt>0->jool.type = NFT_JOOL_NAT64; + else { + erec_queue(error(&@1, "invalid jool type (expected siit or nat64)"), + state->msgs); + xfree($1); + YYERROR; + } + xfree($1); + $<stmt>0->jool.instance = $2; + } + ; + primary_stmt_expr : symbol_expr { $$ = $1; } | integer_expr { $$ = $1; } | boolean_expr { $$ = $1; } diff --git a/src/scanner.l b/src/scanner.l index 45699c85..9e8d637c 100644 --- a/src/scanner.l +++ b/src/scanner.l @@ -375,6 +375,8 @@ addrstring ({macaddr}|{ip4addr}|{ip6addr}) "fully-random" { return FULLY_RANDOM; } "persistent" { return PERSISTENT; } +"jool" { return JOOL; } + "ll" { return LL_HDR; } "nh" { return NETWORK_HDR; } "th" { return TRANSPORT_HDR; } diff --git a/src/statement.c b/src/statement.c index 182edac8..4ec27658 100644 --- a/src/statement.c +++ b/src/statement.c @@ -942,3 +942,36 @@ struct stmt *synproxy_stmt_alloc(const struct location *loc) { return stmt_alloc(loc, &synproxy_stmt_ops); } + +static void jool_stmt_print(const struct stmt *stmt, struct output_ctx *octx) +{ + const char *type = "<unknown>"; + + switch (stmt->jool.type) { + case NFT_JOOL_SIIT: + type = "siit"; + break; + case NFT_JOOL_NAT64: + type = "nat64"; + break; + } + + nft_print(octx, "jool \"%s\" \"%s\"", type, stmt->jool.instance); +} + +static void jool_stmt_destroy(struct stmt *stmt) +{ + xfree(stmt->jool.instance); +} + +static const struct stmt_ops jool_stmt_ops = { + .type = STMT_JOOL, + .name = "jool", + .print = jool_stmt_print, + .destroy = jool_stmt_destroy, +}; + +struct stmt *jool_stmt_alloc(const struct location *loc) +{ + return stmt_alloc(loc, &jool_stmt_ops); +}
Jool statements are used to send packets to the Jool kernel module, which is an IP/ICMP translator: www.jool.mx Sample usage: modprobe jool jool instance add "name" --iptables -6 64:ff9b::/96 sudo nft add rule inet table1 chain1 jool nat64 "name" This feature was requested in Jool's bug tracker: https://github.com/NICMx/Jool/issues/285 Signed-off-by: Alberto Leiva Popper <ydahhrk@gmail.com> --- include/statement.h | 15 +++++++++++++++ src/evaluate.c | 1 + src/netlink_delinearize.c | 15 +++++++++++++++ src/netlink_linearize.c | 15 +++++++++++++++ src/parser_bison.y | 31 +++++++++++++++++++++++++++++++ src/scanner.l | 2 ++ src/statement.c | 33 +++++++++++++++++++++++++++++++++ 7 files changed, 112 insertions(+)