| Message ID | 20170906210839.26091-3-mmichels@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ovn: Add support for IPv6 load balancers | expand |
Hi Mark, On Wed, 6 Sep 2017 16:08:37 -0500 Mark Michelson <mmichels@redhat.com> wrote: > The ct_lb action previously assumed that any address arguments were > IPv4. This patch expands the parsing, formatting, and encoding of ct_lb > to be amenable to IPv6 addresses as well. > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > include/ovn/actions.h | 4 ++- > ovn/lib/actions.c | 99 ++++++++++++++++++++++++++++++++++++++++----------- > tests/ovn.at | 8 ++++- > 3 files changed, 89 insertions(+), 22 deletions(-) > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 0a04af7aa..ec0063efc 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -200,7 +200,9 @@ struct ovnact_ct_nat { > }; > > struct ovnact_ct_lb_dst { > - ovs_be32 ip; > + int family; > + struct in6_addr ipv6; > + ovs_be32 ipv4; > uint16_t port; > }; > Do you think it would make sense to put 'ipv6' and 'ipv4' in an anonymous union to emphasize the fact that we're using only one or the other? Similar to how it was done in flow_hash_symmetric_l4(): uint32_t flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis) { struct { union { ovs_be32 ipv4_addr; struct in6_addr ipv6_addr; }; ovs_be16 eth_type; ovs_be16 vlan_tci; ovs_be16 tp_port; struct eth_addr eth_addr; uint8_t ip_proto; } fields; /* ... */ } -Jakub > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c > index d0d73b69c..8d60529cb 100644 > --- a/ovn/lib/actions.c > +++ b/ovn/lib/actions.c > @@ -883,23 +883,63 @@ parse_ct_lb_action(struct action_context *ctx) > > if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { > while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { > - if (ctx->lexer->token.type != LEX_T_INTEGER > - || mf_subvalue_width(&ctx->lexer->token.value) > 32) { > - free(dsts); > - lexer_syntax_error(ctx->lexer, "expecting IPv4 address"); > - return; > - } > + struct ovnact_ct_lb_dst dst; > + if (lexer_match(ctx->lexer, LEX_T_LSQUARE)) { > + /* IPv6 address and port */ > + if (ctx->lexer->token.type != LEX_T_INTEGER > + || ctx->lexer->token.format != LEX_F_IPV6) { > + free(dsts); > + lexer_syntax_error(ctx->lexer, "expecting IPv6 address"); > + return; > + } > + dst.family = AF_INET6; > + dst.ipv6 = ctx->lexer->token.value.ipv6; > > - /* Parse IP. */ > - ovs_be32 ip = ctx->lexer->token.value.ipv4; > - lexer_get(ctx->lexer); > + lexer_get(ctx->lexer); > + if (!lexer_match(ctx->lexer, LEX_T_RSQUARE)) { > + free(dsts); > + lexer_syntax_error(ctx->lexer, "no closing square " > + "bracket"); > + return; > + } > + dst.port = 0; > + if (lexer_match(ctx->lexer, LEX_T_COLON) > + && !action_parse_port(ctx, &dst.port)) { > + free(dsts); > + return; > + } > + } else { > + if (ctx->lexer->token.type != LEX_T_INTEGER > + || (ctx->lexer->token.format != LEX_F_IPV4 > + && ctx->lexer->token.format != LEX_F_IPV6)) { > + free(dsts); > + lexer_syntax_error(ctx->lexer, "expecting IP address"); > + return; > + } > > - /* Parse optional port. */ > - uint16_t port = 0; > - if (lexer_match(ctx->lexer, LEX_T_COLON) > - && !action_parse_port(ctx, &port)) { > - free(dsts); > - return; > + /* Parse IP. */ > + struct ovnact_ct_lb_dst dst; > + if (ctx->lexer->token.format == LEX_F_IPV4) { > + dst.family = AF_INET; > + dst.ipv4 = ctx->lexer->token.value.ipv4; > + } else { > + dst.family = AF_INET6; > + dst.ipv6 = ctx->lexer->token.value.ipv6; > + } > + > + lexer_get(ctx->lexer); > + dst.port = 0; > + if (lexer_match(ctx->lexer, LEX_T_COLON)) { > + if (dst.family == AF_INET6) { > + free(dsts); > + lexer_syntax_error(ctx->lexer, "IPv6 address needs " > + "square brackets if port is included"); > + return; > + } else if (!action_parse_port(ctx, &dst.port)) { > + free(dsts); > + return; > + } > + } > } > lexer_match(ctx->lexer, LEX_T_COMMA); > > @@ -907,7 +947,7 @@ parse_ct_lb_action(struct action_context *ctx) > if (n_dsts >= allocated_dsts) { > dsts = x2nrealloc(dsts, &allocated_dsts, sizeof *dsts); > } > - dsts[n_dsts++] = (struct ovnact_ct_lb_dst) { ip, port }; > + dsts[n_dsts++] = dst; > } > } > > @@ -929,9 +969,19 @@ format_CT_LB(const struct ovnact_ct_lb *cl, struct ds *s) > } > > const struct ovnact_ct_lb_dst *dst = &cl->dsts[i]; > - ds_put_format(s, IP_FMT, IP_ARGS(dst->ip)); > - if (dst->port) { > - ds_put_format(s, ":%"PRIu16, dst->port); > + if (dst->family == AF_INET) { > + ds_put_format(s, IP_FMT, IP_ARGS(dst->ipv4)); > + if (dst->port) { > + ds_put_format(s, ":%"PRIu16, dst->port); > + } > + } else { > + if (dst->port) { > + ds_put_char(s, '['); > + } > + ipv6_format_addr(&dst->ipv6, s); > + if (dst->port) { > + ds_put_format(s, "]:%"PRIu16, dst->port); > + } > } > } > ds_put_char(s, ')'); > @@ -991,8 +1041,17 @@ encode_CT_LB(const struct ovnact_ct_lb *cl, > BUILD_ASSERT(MFF_LOG_DNAT_ZONE < MFF_REG0 + FLOW_N_REGS); > for (size_t bucket_id = 0; bucket_id < cl->n_dsts; bucket_id++) { > const struct ovnact_ct_lb_dst *dst = &cl->dsts[bucket_id]; > + char ip_addr[INET6_ADDRSTRLEN]; > + if (dst->family == AF_INET) { > + inet_ntop(AF_INET, &dst->ipv4, ip_addr, sizeof ip_addr); > + } else { > + inet_ntop(AF_INET6, &dst->ipv6, ip_addr, sizeof ip_addr); > + } > ds_put_format(&ds, ",bucket=bucket_id=%"PRIuSIZE",weight:100,actions=" > - "ct(nat(dst="IP_FMT, bucket_id, IP_ARGS(dst->ip)); > + "ct(nat(dst=%s%s%s", bucket_id, > + dst->family == AF_INET6 && dst->port ? "[" : "", > + ip_addr, > + dst->family == AF_INET6 && dst->port ? "]" : ""); > if (dst->port) { > ds_put_format(&ds, ":%"PRIu16, dst->port); > } > diff --git a/tests/ovn.at b/tests/ovn.at > index fb9fc7352..9118f2839 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -793,13 +793,19 @@ ct_lb(192.168.1.2, 192.168.1.3, ); > formats as ct_lb(192.168.1.2, 192.168.1.3); > encodes as group:2 > has prereqs ip > +ct_lb(fd0f::2, fd0f::3, ); > + formats as ct_lb(fd0f::2, fd0f::3); > + encodes as group:3 > + has prereqs ip > > ct_lb(192.168.1.2:); > Syntax error at `)' expecting port number. > ct_lb(192.168.1.2:123456); > Syntax error at `123456' expecting port number. > ct_lb(foo); > - Syntax error at `foo' expecting IPv4 address. > + Syntax error at `foo' expecting IP address. > +ct_lb([192.168.1.2]); > + Syntax error at `192.168.1.2' expecting IPv6 address. > > # ct_next > ct_next;
On Thu, Sep 7, 2017 at 8:12 AM Jakub Sitnicki <jkbs@redhat.com> wrote: > Hi Mark, > > On Wed, 6 Sep 2017 16:08:37 -0500 > Mark Michelson <mmichels@redhat.com> wrote: > > > The ct_lb action previously assumed that any address arguments were > > IPv4. This patch expands the parsing, formatting, and encoding of ct_lb > > to be amenable to IPv6 addresses as well. > > > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > > --- > > include/ovn/actions.h | 4 ++- > > ovn/lib/actions.c | 99 > ++++++++++++++++++++++++++++++++++++++++----------- > > tests/ovn.at | 8 ++++- > > 3 files changed, 89 insertions(+), 22 deletions(-) > > > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > > index 0a04af7aa..ec0063efc 100644 > > --- a/include/ovn/actions.h > > +++ b/include/ovn/actions.h > > @@ -200,7 +200,9 @@ struct ovnact_ct_nat { > > }; > > > > struct ovnact_ct_lb_dst { > > - ovs_be32 ip; > > + int family; > > + struct in6_addr ipv6; > > + ovs_be32 ipv4; > > uint16_t port; > > }; > > > > Do you think it would make sense to put 'ipv6' and 'ipv4' in an > anonymous union to emphasize the fact that we're using only one or the > other? Similar to how it was done in flow_hash_symmetric_l4(): > > Sure, I can do that. I think I'm going to wait for more feedback on the patchset before posting new revisions, though. Thanks for giving this a look! > uint32_t > flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis) > { > struct { > union { > ovs_be32 ipv4_addr; > struct in6_addr ipv6_addr; > }; > ovs_be16 eth_type; > ovs_be16 vlan_tci; > ovs_be16 tp_port; > struct eth_addr eth_addr; > uint8_t ip_proto; > } fields; > /* ... */ > } > > -Jakub > > > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c > > index d0d73b69c..8d60529cb 100644 > > --- a/ovn/lib/actions.c > > +++ b/ovn/lib/actions.c > > @@ -883,23 +883,63 @@ parse_ct_lb_action(struct action_context *ctx) > > > > if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { > > while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { > > - if (ctx->lexer->token.type != LEX_T_INTEGER > > - || mf_subvalue_width(&ctx->lexer->token.value) > 32) { > > - free(dsts); > > - lexer_syntax_error(ctx->lexer, "expecting IPv4 > address"); > > - return; > > - } > > + struct ovnact_ct_lb_dst dst; > > + if (lexer_match(ctx->lexer, LEX_T_LSQUARE)) { > > + /* IPv6 address and port */ > > + if (ctx->lexer->token.type != LEX_T_INTEGER > > + || ctx->lexer->token.format != LEX_F_IPV6) { > > + free(dsts); > > + lexer_syntax_error(ctx->lexer, "expecting IPv6 > address"); > > + return; > > + } > > + dst.family = AF_INET6; > > + dst.ipv6 = ctx->lexer->token.value.ipv6; > > > > - /* Parse IP. */ > > - ovs_be32 ip = ctx->lexer->token.value.ipv4; > > - lexer_get(ctx->lexer); > > + lexer_get(ctx->lexer); > > + if (!lexer_match(ctx->lexer, LEX_T_RSQUARE)) { > > + free(dsts); > > + lexer_syntax_error(ctx->lexer, "no closing square " > > + "bracket"); > > + return; > > + } > > + dst.port = 0; > > + if (lexer_match(ctx->lexer, LEX_T_COLON) > > + && !action_parse_port(ctx, &dst.port)) { > > + free(dsts); > > + return; > > + } > > + } else { > > + if (ctx->lexer->token.type != LEX_T_INTEGER > > + || (ctx->lexer->token.format != LEX_F_IPV4 > > + && ctx->lexer->token.format != LEX_F_IPV6)) { > > + free(dsts); > > + lexer_syntax_error(ctx->lexer, "expecting IP > address"); > > + return; > > + } > > > > - /* Parse optional port. */ > > - uint16_t port = 0; > > - if (lexer_match(ctx->lexer, LEX_T_COLON) > > - && !action_parse_port(ctx, &port)) { > > - free(dsts); > > - return; > > + /* Parse IP. */ > > + struct ovnact_ct_lb_dst dst; > > + if (ctx->lexer->token.format == LEX_F_IPV4) { > > + dst.family = AF_INET; > > + dst.ipv4 = ctx->lexer->token.value.ipv4; > > + } else { > > + dst.family = AF_INET6; > > + dst.ipv6 = ctx->lexer->token.value.ipv6; > > + } > > + > > + lexer_get(ctx->lexer); > > + dst.port = 0; > > + if (lexer_match(ctx->lexer, LEX_T_COLON)) { > > + if (dst.family == AF_INET6) { > > + free(dsts); > > + lexer_syntax_error(ctx->lexer, "IPv6 address > needs " > > + "square brackets if port is included"); > > + return; > > + } else if (!action_parse_port(ctx, &dst.port)) { > > + free(dsts); > > + return; > > + } > > + } > > } > > lexer_match(ctx->lexer, LEX_T_COMMA); > > > > @@ -907,7 +947,7 @@ parse_ct_lb_action(struct action_context *ctx) > > if (n_dsts >= allocated_dsts) { > > dsts = x2nrealloc(dsts, &allocated_dsts, sizeof *dsts); > > } > > - dsts[n_dsts++] = (struct ovnact_ct_lb_dst) { ip, port }; > > + dsts[n_dsts++] = dst; > > } > > } > > > > @@ -929,9 +969,19 @@ format_CT_LB(const struct ovnact_ct_lb *cl, struct > ds *s) > > } > > > > const struct ovnact_ct_lb_dst *dst = &cl->dsts[i]; > > - ds_put_format(s, IP_FMT, IP_ARGS(dst->ip)); > > - if (dst->port) { > > - ds_put_format(s, ":%"PRIu16, dst->port); > > + if (dst->family == AF_INET) { > > + ds_put_format(s, IP_FMT, IP_ARGS(dst->ipv4)); > > + if (dst->port) { > > + ds_put_format(s, ":%"PRIu16, dst->port); > > + } > > + } else { > > + if (dst->port) { > > + ds_put_char(s, '['); > > + } > > + ipv6_format_addr(&dst->ipv6, s); > > + if (dst->port) { > > + ds_put_format(s, "]:%"PRIu16, dst->port); > > + } > > } > > } > > ds_put_char(s, ')'); > > @@ -991,8 +1041,17 @@ encode_CT_LB(const struct ovnact_ct_lb *cl, > > BUILD_ASSERT(MFF_LOG_DNAT_ZONE < MFF_REG0 + FLOW_N_REGS); > > for (size_t bucket_id = 0; bucket_id < cl->n_dsts; bucket_id++) { > > const struct ovnact_ct_lb_dst *dst = &cl->dsts[bucket_id]; > > + char ip_addr[INET6_ADDRSTRLEN]; > > + if (dst->family == AF_INET) { > > + inet_ntop(AF_INET, &dst->ipv4, ip_addr, sizeof ip_addr); > > + } else { > > + inet_ntop(AF_INET6, &dst->ipv6, ip_addr, sizeof ip_addr); > > + } > > ds_put_format(&ds, > ",bucket=bucket_id=%"PRIuSIZE",weight:100,actions=" > > - "ct(nat(dst="IP_FMT, bucket_id, IP_ARGS(dst->ip)); > > + "ct(nat(dst=%s%s%s", bucket_id, > > + dst->family == AF_INET6 && dst->port ? "[" : "", > > + ip_addr, > > + dst->family == AF_INET6 && dst->port ? "]" : ""); > > if (dst->port) { > > ds_put_format(&ds, ":%"PRIu16, dst->port); > > } > > diff --git a/tests/ovn.at b/tests/ovn.at > > index fb9fc7352..9118f2839 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -793,13 +793,19 @@ ct_lb(192.168.1.2, 192.168.1.3, ); > > formats as ct_lb(192.168.1.2, 192.168.1.3); > > encodes as group:2 > > has prereqs ip > > +ct_lb(fd0f::2, fd0f::3, ); > > + formats as ct_lb(fd0f::2, fd0f::3); > > + encodes as group:3 > > + has prereqs ip > > > > ct_lb(192.168.1.2:); > > Syntax error at `)' expecting port number. > > ct_lb(192.168.1.2:123456); > > Syntax error at `123456' expecting port number. > > ct_lb(foo); > > - Syntax error at `foo' expecting IPv4 address. > > + Syntax error at `foo' expecting IP address. > > +ct_lb([192.168.1.2]); > > + Syntax error at `192.168.1.2' expecting IPv6 address. > > > > # ct_next > > ct_next; > >
diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 0a04af7aa..ec0063efc 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -200,7 +200,9 @@ struct ovnact_ct_nat { }; struct ovnact_ct_lb_dst { - ovs_be32 ip; + int family; + struct in6_addr ipv6; + ovs_be32 ipv4; uint16_t port; }; diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index d0d73b69c..8d60529cb 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -883,23 +883,63 @@ parse_ct_lb_action(struct action_context *ctx) if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { - if (ctx->lexer->token.type != LEX_T_INTEGER - || mf_subvalue_width(&ctx->lexer->token.value) > 32) { - free(dsts); - lexer_syntax_error(ctx->lexer, "expecting IPv4 address"); - return; - } + struct ovnact_ct_lb_dst dst; + if (lexer_match(ctx->lexer, LEX_T_LSQUARE)) { + /* IPv6 address and port */ + if (ctx->lexer->token.type != LEX_T_INTEGER + || ctx->lexer->token.format != LEX_F_IPV6) { + free(dsts); + lexer_syntax_error(ctx->lexer, "expecting IPv6 address"); + return; + } + dst.family = AF_INET6; + dst.ipv6 = ctx->lexer->token.value.ipv6; - /* Parse IP. */ - ovs_be32 ip = ctx->lexer->token.value.ipv4; - lexer_get(ctx->lexer); + lexer_get(ctx->lexer); + if (!lexer_match(ctx->lexer, LEX_T_RSQUARE)) { + free(dsts); + lexer_syntax_error(ctx->lexer, "no closing square " + "bracket"); + return; + } + dst.port = 0; + if (lexer_match(ctx->lexer, LEX_T_COLON) + && !action_parse_port(ctx, &dst.port)) { + free(dsts); + return; + } + } else { + if (ctx->lexer->token.type != LEX_T_INTEGER + || (ctx->lexer->token.format != LEX_F_IPV4 + && ctx->lexer->token.format != LEX_F_IPV6)) { + free(dsts); + lexer_syntax_error(ctx->lexer, "expecting IP address"); + return; + } - /* Parse optional port. */ - uint16_t port = 0; - if (lexer_match(ctx->lexer, LEX_T_COLON) - && !action_parse_port(ctx, &port)) { - free(dsts); - return; + /* Parse IP. */ + struct ovnact_ct_lb_dst dst; + if (ctx->lexer->token.format == LEX_F_IPV4) { + dst.family = AF_INET; + dst.ipv4 = ctx->lexer->token.value.ipv4; + } else { + dst.family = AF_INET6; + dst.ipv6 = ctx->lexer->token.value.ipv6; + } + + lexer_get(ctx->lexer); + dst.port = 0; + if (lexer_match(ctx->lexer, LEX_T_COLON)) { + if (dst.family == AF_INET6) { + free(dsts); + lexer_syntax_error(ctx->lexer, "IPv6 address needs " + "square brackets if port is included"); + return; + } else if (!action_parse_port(ctx, &dst.port)) { + free(dsts); + return; + } + } } lexer_match(ctx->lexer, LEX_T_COMMA); @@ -907,7 +947,7 @@ parse_ct_lb_action(struct action_context *ctx) if (n_dsts >= allocated_dsts) { dsts = x2nrealloc(dsts, &allocated_dsts, sizeof *dsts); } - dsts[n_dsts++] = (struct ovnact_ct_lb_dst) { ip, port }; + dsts[n_dsts++] = dst; } } @@ -929,9 +969,19 @@ format_CT_LB(const struct ovnact_ct_lb *cl, struct ds *s) } const struct ovnact_ct_lb_dst *dst = &cl->dsts[i]; - ds_put_format(s, IP_FMT, IP_ARGS(dst->ip)); - if (dst->port) { - ds_put_format(s, ":%"PRIu16, dst->port); + if (dst->family == AF_INET) { + ds_put_format(s, IP_FMT, IP_ARGS(dst->ipv4)); + if (dst->port) { + ds_put_format(s, ":%"PRIu16, dst->port); + } + } else { + if (dst->port) { + ds_put_char(s, '['); + } + ipv6_format_addr(&dst->ipv6, s); + if (dst->port) { + ds_put_format(s, "]:%"PRIu16, dst->port); + } } } ds_put_char(s, ')'); @@ -991,8 +1041,17 @@ encode_CT_LB(const struct ovnact_ct_lb *cl, BUILD_ASSERT(MFF_LOG_DNAT_ZONE < MFF_REG0 + FLOW_N_REGS); for (size_t bucket_id = 0; bucket_id < cl->n_dsts; bucket_id++) { const struct ovnact_ct_lb_dst *dst = &cl->dsts[bucket_id]; + char ip_addr[INET6_ADDRSTRLEN]; + if (dst->family == AF_INET) { + inet_ntop(AF_INET, &dst->ipv4, ip_addr, sizeof ip_addr); + } else { + inet_ntop(AF_INET6, &dst->ipv6, ip_addr, sizeof ip_addr); + } ds_put_format(&ds, ",bucket=bucket_id=%"PRIuSIZE",weight:100,actions=" - "ct(nat(dst="IP_FMT, bucket_id, IP_ARGS(dst->ip)); + "ct(nat(dst=%s%s%s", bucket_id, + dst->family == AF_INET6 && dst->port ? "[" : "", + ip_addr, + dst->family == AF_INET6 && dst->port ? "]" : ""); if (dst->port) { ds_put_format(&ds, ":%"PRIu16, dst->port); } diff --git a/tests/ovn.at b/tests/ovn.at index fb9fc7352..9118f2839 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -793,13 +793,19 @@ ct_lb(192.168.1.2, 192.168.1.3, ); formats as ct_lb(192.168.1.2, 192.168.1.3); encodes as group:2 has prereqs ip +ct_lb(fd0f::2, fd0f::3, ); + formats as ct_lb(fd0f::2, fd0f::3); + encodes as group:3 + has prereqs ip ct_lb(192.168.1.2:); Syntax error at `)' expecting port number. ct_lb(192.168.1.2:123456); Syntax error at `123456' expecting port number. ct_lb(foo); - Syntax error at `foo' expecting IPv4 address. + Syntax error at `foo' expecting IP address. +ct_lb([192.168.1.2]); + Syntax error at `192.168.1.2' expecting IPv6 address. # ct_next ct_next;
The ct_lb action previously assumed that any address arguments were IPv4. This patch expands the parsing, formatting, and encoding of ct_lb to be amenable to IPv6 addresses as well. Signed-off-by: Mark Michelson <mmichels@redhat.com> --- include/ovn/actions.h | 4 ++- ovn/lib/actions.c | 99 ++++++++++++++++++++++++++++++++++++++++----------- tests/ovn.at | 8 ++++- 3 files changed, 89 insertions(+), 22 deletions(-)