[ovs-dev] [PATCH ovn 4/5] actions: Add new actions chk_lb_hairpin, chk_lb_hairpin_reply and ct_snat_to_vip.
Mark Michelson
mmichels at redhat.com
Fri Oct 23 20:48:57 UTC 2020
On 10/21/20 3:25 AM, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
>
> The action - chk_lb_hairpin checks if the packet destined to a load balancer VIP
> is to be hairpinned back to the same destination and if so, sets the destination register
> bit to 1.
>
> The action - chk_lb_hairpin_reply checks if the packet is a reply of the hairpinned
> packet. If so, it sets the destination register bit to 1.
>
> The action ct_snat_to_vip snats the source IP to the load balancer VIP if chk_lb_hairpin()
> returned true.
>
> These actions will be used in the upcoming patch by ovn-northd in the hairpin logical flows.
> This helps in reducing lots of hairpin logical flows.
>
> Signed-off-by: Numan Siddique <numans at ovn.org>
> ---
> controller/lflow.c | 3 ++
> include/ovn/actions.h | 15 ++++--
> lib/actions.c | 115 +++++++++++++++++++++++++++++++++++++++---
> ovn-sb.xml | 37 ++++++++++++++
> tests/ovn.at | 39 ++++++++++++++
> tests/test-ovn.c | 3 ++
> utilities/ovn-trace.c | 65 +++++++++++++++++++++++-
> 7 files changed, 265 insertions(+), 12 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index 77da4a5198..75a8cd9e13 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -698,6 +698,9 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
> .output_ptable = output_ptable,
> .mac_bind_ptable = OFTABLE_MAC_BINDING,
> .mac_lookup_ptable = OFTABLE_MAC_LOOKUP,
> + .lb_hairpin_ptable = OFTABLE_CHK_LB_HAIRPIN,
> + .lb_hairpin_reply_ptable = OFTABLE_CHK_LB_HAIRPIN_REPLY,
> + .ct_snat_vip_ptable = OFTABLE_CT_SNAT_FOR_VIP,
> };
> ovnacts_encode(ovnacts->data, ovnacts->size, &ep, &ofpacts);
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index b4e5acabb9..32f9c53dfc 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -83,7 +83,7 @@ struct ovn_extend_table;
> OVNACT(PUT_DHCPV4_OPTS, ovnact_put_opts) \
> OVNACT(PUT_DHCPV6_OPTS, ovnact_put_opts) \
> OVNACT(SET_QUEUE, ovnact_set_queue) \
> - OVNACT(DNS_LOOKUP, ovnact_dns_lookup) \
> + OVNACT(DNS_LOOKUP, ovnact_result) \
> OVNACT(LOG, ovnact_log) \
> OVNACT(PUT_ND_RA_OPTS, ovnact_put_opts) \
> OVNACT(ND_NS, ovnact_nest) \
> @@ -97,6 +97,9 @@ struct ovn_extend_table;
> OVNACT(DHCP6_REPLY, ovnact_null) \
> OVNACT(ICMP6_ERROR, ovnact_nest) \
> OVNACT(REJECT, ovnact_nest) \
> + OVNACT(CHK_LB_HAIRPIN, ovnact_result) \
> + OVNACT(CHK_LB_HAIRPIN_REPLY, ovnact_result) \
> + OVNACT(CT_SNAT_TO_VIP, ovnact_null) \
>
> /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */
> enum OVS_PACKED_ENUM ovnact_type {
> @@ -338,8 +341,8 @@ struct ovnact_set_queue {
> uint16_t queue_id;
> };
>
> -/* OVNACT_DNS_LOOKUP. */
> -struct ovnact_dns_lookup {
> +/* OVNACT_DNS_LOOKUP, OVNACT_CHK_LB_HAIRPIN, OVNACT_CHK_LB_HAIRPIN_REPLY. */
> +struct ovnact_result {
> struct ovnact ovnact;
> struct expr_field dst; /* 1-bit destination field. */
> };
> @@ -727,6 +730,12 @@ struct ovnact_encode_params {
> resubmit. */
> uint8_t mac_lookup_ptable; /* OpenFlow table for
> 'lookup_arp'/'lookup_nd' to resubmit. */
> + uint8_t lb_hairpin_ptable; /* OpenFlow table for
> + * 'chk_lb_hairpin' to resubmit. */
> + uint8_t lb_hairpin_reply_ptable; /* OpenFlow table for
> + * 'chk_lb_hairpin_reply' to resubmit. */
> + uint8_t ct_snat_vip_ptable; /* OpenFlow table for
> + * 'ct_snat_to_vip' to resubmit. */
> };
>
> void ovnacts_encode(const struct ovnact[], size_t ovnacts_len,
> diff --git a/lib/actions.c b/lib/actions.c
> index 1e1bdeff24..d9ee063017 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -2634,13 +2634,14 @@ ovnact_set_queue_free(struct ovnact_set_queue *a OVS_UNUSED)
> }
>
> static void
> -parse_dns_lookup(struct action_context *ctx, const struct expr_field *dst,
> - struct ovnact_dns_lookup *dl)
> +parse_ovnact_result(struct action_context *ctx, const char *name,
> + const char *prereq, const struct expr_field *dst,
> + struct ovnact_result *res)
> {
> lexer_get(ctx->lexer); /* Skip dns_lookup. */
This comment needs to be updated.
> lexer_get(ctx->lexer); /* Skip '('. */
> if (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
> - lexer_error(ctx->lexer, "dns_lookup doesn't take any parameters");
> + lexer_error(ctx->lexer, "%s doesn't take any parameters", name);
> return;
> }
> /* Validate that the destination is a 1-bit, modifiable field. */
> @@ -2650,19 +2651,29 @@ parse_dns_lookup(struct action_context *ctx, const struct expr_field *dst,
> free(error);
> return;
> }
> - dl->dst = *dst;
> - add_prerequisite(ctx, "udp");
> + res->dst = *dst;
> +
> + if (prereq) {
> + add_prerequisite(ctx, prereq);
> + }
> +}
> +
> +static void
> +parse_dns_lookup(struct action_context *ctx, const struct expr_field *dst,
> + struct ovnact_result *dl)
> +{
> + parse_ovnact_result(ctx, "dns_lookup", "udp", dst, dl);
> }
>
> static void
> -format_DNS_LOOKUP(const struct ovnact_dns_lookup *dl, struct ds *s)
> +format_DNS_LOOKUP(const struct ovnact_result *dl, struct ds *s)
> {
> expr_field_format(&dl->dst, s);
> ds_put_cstr(s, " = dns_lookup();");
> }
>
> static void
> -encode_DNS_LOOKUP(const struct ovnact_dns_lookup *dl,
> +encode_DNS_LOOKUP(const struct ovnact_result *dl,
> const struct ovnact_encode_params *ep OVS_UNUSED,
> struct ofpbuf *ofpacts)
> {
> @@ -2679,7 +2690,7 @@ encode_DNS_LOOKUP(const struct ovnact_dns_lookup *dl,
>
>
> static void
> -ovnact_dns_lookup_free(struct ovnact_dns_lookup *dl OVS_UNUSED)
> +ovnact_result_free(struct ovnact_result *dl OVS_UNUSED)
> {
> }
>
> @@ -3451,6 +3462,84 @@ ovnact_fwd_group_free(struct ovnact_fwd_group *fwd_group)
> free(fwd_group->child_ports);
> }
>
> +static void
> +parse_chk_lb_hairpin(struct action_context *ctx, const struct expr_field *dst,
> + struct ovnact_result *res)
> +{
> + parse_ovnact_result(ctx, "chk_lb_hairpin", NULL, dst, res);
> +}
> +
> +static void
> +parse_chk_lb_hairpin_reply(struct action_context *ctx,
> + const struct expr_field *dst,
> + struct ovnact_result *res)
> +{
> + parse_ovnact_result(ctx, "chk_lb_hairpin_reply", NULL, dst, res);
> +}
> +
> +
> +static void
> +format_CHK_LB_HAIRPIN(const struct ovnact_result *res, struct ds *s)
> +{
> + expr_field_format(&res->dst, s);
> + ds_put_cstr(s, " = chk_lb_hairpin();");
> +}
> +
> +static void
> +format_CHK_LB_HAIRPIN_REPLY(const struct ovnact_result *res, struct ds *s)
> +{
> + expr_field_format(&res->dst, s);
> + ds_put_cstr(s, " = chk_lb_hairpin_reply();");
> +}
> +
> +static void
> +encode_CHK_LB_HAIRPIN(const struct ovnact_result *res,
> + const struct ovnact_encode_params *ep,
> + struct ofpbuf *ofpacts)
> +{
> + struct mf_subfield dst = expr_resolve_field(&res->dst);
> + ovs_assert(dst.field);
> + put_load(0, MFF_LOG_FLAGS, MLF_LOOKUP_LB_HAIRPIN_BIT, 1, ofpacts);
> + emit_resubmit(ofpacts, ep->lb_hairpin_ptable);
> +
> + struct ofpact_reg_move *orm = ofpact_put_REG_MOVE(ofpacts);
> + orm->dst = dst;
> + orm->src.field = mf_from_id(MFF_LOG_FLAGS);
> + orm->src.ofs = MLF_LOOKUP_LB_HAIRPIN_BIT;
> + orm->src.n_bits = 1;
> +}
> +
> +static void
> +encode_CHK_LB_HAIRPIN_REPLY(const struct ovnact_result *res,
> + const struct ovnact_encode_params *ep,
> + struct ofpbuf *ofpacts)
> +{
> + struct mf_subfield dst = expr_resolve_field(&res->dst);
> + ovs_assert(dst.field);
> + put_load(0, MFF_LOG_FLAGS, MLF_LOOKUP_LB_HAIRPIN_BIT, 1, ofpacts);
> + emit_resubmit(ofpacts, ep->lb_hairpin_reply_ptable);
> +
> + struct ofpact_reg_move *orm = ofpact_put_REG_MOVE(ofpacts);
> + orm->dst = dst;
> + orm->src.field = mf_from_id(MFF_LOG_FLAGS);
> + orm->src.ofs = MLF_LOOKUP_LB_HAIRPIN_BIT;
> + orm->src.n_bits = 1;
> +}
encode_CHK_LB_HAIRPIN and encode_CHK_LB_HAIRPIN_REPLY are exactly the
same except for the resubmit destination. It seems like they could be
refactored to use a common function.
> +
> +static void
> +format_CT_SNAT_TO_VIP(const struct ovnact_null *null OVS_UNUSED, struct ds *s)
> +{
> + ds_put_cstr(s, "ct_snat_to_vip;");
> +}
> +
> +static void
> +encode_CT_SNAT_TO_VIP(const struct ovnact_null *null OVS_UNUSED,
> + const struct ovnact_encode_params *ep,
> + struct ofpbuf *ofpacts)
> +{
> + emit_resubmit(ofpacts, ep->ct_snat_vip_ptable);
> +}
> +
> /* Parses an assignment or exchange or put_dhcp_opts action. */
> static void
> parse_set_action(struct action_context *ctx)
> @@ -3503,6 +3592,14 @@ parse_set_action(struct action_context *ctx)
> && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
> parse_lookup_mac_bind_ip(ctx, &lhs, 128,
> ovnact_put_LOOKUP_ND_IP(ctx->ovnacts));
> + } else if (!strcmp(ctx->lexer->token.s, "chk_lb_hairpin")
> + && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
> + parse_chk_lb_hairpin(ctx, &lhs,
> + ovnact_put_CHK_LB_HAIRPIN(ctx->ovnacts));
> + } else if (!strcmp(ctx->lexer->token.s, "chk_lb_hairpin_reply")
> + && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
> + parse_chk_lb_hairpin_reply(
> + ctx, &lhs, ovnact_put_CHK_LB_HAIRPIN_REPLY(ctx->ovnacts));
> } else {
> parse_assignment_action(ctx, false, &lhs);
> }
> @@ -3589,6 +3686,8 @@ parse_action(struct action_context *ctx)
> ovnact_put_DHCP6_REPLY(ctx->ovnacts);
> } else if (lexer_match_id(ctx->lexer, "reject")) {
> parse_REJECT(ctx);
> + } else if (lexer_match_id(ctx->lexer, "ct_snat_to_vip")) {
> + ovnact_put_CT_SNAT_TO_VIP(ctx->ovnacts);
> } else {
> lexer_syntax_error(ctx->lexer, "expecting action");
> }
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 8638fa7eb4..b87791b93c 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2325,6 +2325,43 @@ tcp.flags = RST;
> Delegation Router and managed IPv6 Prefix delegation state machine
> </p>
> </dd>
> +
> + <dt><code><var>R</var> = chk_lb_hairpin();</code></dt>
> + <dd>
> + <p>
> + This action checks if the packet under consideration was destined
> + to a load balancer VIP and it is hairpinned, i.e., after load
> + balancing the destination IP matches the source IP. If it is so,
> + then the 1-bit destination register <var>R</var> is set to 1.
> + </p>
> + </dd>
> +
> + <dt><code><var>R</var> = chk_lb_hairpin_reply();</code></dt>
> + <dd>
> + <p>
> + This action checks if the packet under consideration is from
> + one of the backend IP of a load balancer VIP and the destination IP
> + is the load balancer VIP. If it is so, then the 1-bit destination
> + register <var>R</var> is set to 1.
> + </p>
> + </dd>
> +
> + <dt><code><var>R</var> = ct_snat_to_vip;</code></dt>
> + <dd>
> + <p>
> + This action sends the packet through the SNAT zone to change the
> + source IP address of the packet to the load balancer VIP if the
> + original destination IP was load balancer VIP and commits the
> + connection. This action applies successfully only for the
> + hairpinned traffic i.e if the action <code>chk_lb_hairpin</code>
> + returned success. This action doesn't take any arguments and it
> + determines the SNAT IP internally.
> +
> + The packet is not automatically sent to the next table. The caller
> + has to execute the <code>next;</code> action explicitly after this
> + action to advance the packet to the next stage.
> + </p>
> + </dd>
> </dl>
> </column>
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 8480e0ee1d..ad99ce9908 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1716,6 +1716,45 @@ fwd_group(liveness="false", childports="eth0", "lsp1");
> handle_dhcpv6_reply;
> encodes as controller(userdata=00.00.00.13.00.00.00.00)
>
> +# chk_lb_hairpin
> +reg0[0] = chk_lb_hairpin();
> + encodes as set_field:0/0x80->reg10,resubmit(,68),move:NXM_NX_REG10[7]->NXM_NX_XXREG0[96]
> +
> +reg2[2] = chk_lb_hairpin();
> + encodes as set_field:0/0x80->reg10,resubmit(,68),move:NXM_NX_REG10[7]->NXM_NX_XXREG0[34]
> +
> +reg0 = chk_lb_hairpin();
> + Cannot use 32-bit field reg0[0..31] where 1-bit field is required.
> +
> +reg0[0] = chk_lb_hairpin(foo);
> + chk_lb_hairpin doesn't take any parameters
> +
> +chk_lb_hairpin;
> + Syntax error at `chk_lb_hairpin' expecting action.
> +
> +# chk_lb_hairpin_reply
> +reg0[0] = chk_lb_hairpin_reply();
> + encodes as set_field:0/0x80->reg10,resubmit(,69),move:NXM_NX_REG10[7]->NXM_NX_XXREG0[96]
> +
> +reg2[2..3] = chk_lb_hairpin_reply();
> + Cannot use 2-bit field reg2[2..3] where 1-bit field is required.
> +
> +reg0 = chk_lb_hairpin_reply();
> + Cannot use 32-bit field reg0[0..31] where 1-bit field is required.
> +
> +reg0[0] = chk_lb_hairpin_reply(foo);
> + chk_lb_hairpin_reply doesn't take any parameters
> +
> +chk_lb_hairpin_reply;
> + Syntax error at `chk_lb_hairpin_reply' expecting action.
> +
> +# ct_snat_to_vip
> +ct_snat_to_vip;
> + encodes as resubmit(,70)
> +
> +ct_snat_to_vip(foo);
> + Syntax error at `(' expecting `;'.
> +
> # Miscellaneous negative tests.
> ;
> Syntax error at `;'.
> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
> index d94ab025d9..13642f4c88 100644
> --- a/tests/test-ovn.c
> +++ b/tests/test-ovn.c
> @@ -1341,6 +1341,9 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED)
> .output_ptable = OFTABLE_SAVE_INPORT,
> .mac_bind_ptable = OFTABLE_MAC_BINDING,
> .mac_lookup_ptable = OFTABLE_MAC_LOOKUP,
> + .lb_hairpin_ptable = OFTABLE_CHK_LB_HAIRPIN,
> + .lb_hairpin_reply_ptable = OFTABLE_CHK_LB_HAIRPIN_REPLY,
> + .ct_snat_vip_ptable = OFTABLE_CT_SNAT_FOR_VIP,
> };
> struct ofpbuf ofpacts;
> ofpbuf_init(&ofpacts, 0);
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index db5bb301e8..3368f5cc27 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -1990,7 +1990,7 @@ execute_next(const struct ovnact_next *next,
>
>
> static void
> -execute_dns_lookup(const struct ovnact_dns_lookup *dl, struct flow *uflow,
> +execute_dns_lookup(const struct ovnact_result *dl, struct flow *uflow,
> struct ovs_list *super)
> {
> struct mf_subfield sf = expr_resolve_field(&dl->dst);
> @@ -2222,6 +2222,57 @@ execute_ovnfield_load(const struct ovnact_load *load,
> }
> }
>
> +static void
> +execute_chk_lb_hairpin(const struct ovnact_result *dl, struct flow *uflow,
> + struct ovs_list *super)
> +{
> + int family = (uflow->dl_type == htons(ETH_TYPE_IP) ? AF_INET
> + : uflow->dl_type == htons(ETH_TYPE_IPV6) ? AF_INET6
> + : AF_UNSPEC);
> + uint8_t res = 0;
> + if (family != AF_UNSPEC && uflow->ct_state & CS_DST_NAT) {
> + if (family == AF_INET) {
> + res = (uflow->nw_src == uflow->nw_dst) ? 1 : 0;
> + } else {
> + res = ipv6_addr_equals(&uflow->ipv6_src, &uflow->ipv6_dst) ? 1 : 0;
> + }
> + }
> +
> + struct mf_subfield sf = expr_resolve_field(&dl->dst);
> + union mf_subvalue sv = { .u8_val = res };
> + mf_write_subfield_flow(&sf, &sv, uflow);
> +
> + struct ds s = DS_EMPTY_INITIALIZER;
> + expr_field_format(&dl->dst, &s);
> + ovntrace_node_append(super, OVNTRACE_NODE_MODIFY,
> + "%s = %d", ds_cstr(&s), res);
> + ds_destroy(&s);
> +}
> +
> +static void
> +execute_chk_lb_hairpin_reply(const struct ovnact_result *dl,
> + struct flow *uflow,
> + struct ovs_list *super)
> +{
> + struct mf_subfield sf = expr_resolve_field(&dl->dst);
> + union mf_subvalue sv = { .u8_val = 0 };
> + mf_write_subfield_flow(&sf, &sv, uflow);
> + ovntrace_node_append(super, OVNTRACE_NODE_ERROR,
> + "*** chk_lb_hairpin_reply action not implemented");
> + struct ds s = DS_EMPTY_INITIALIZER;
> + expr_field_format(&dl->dst, &s);
> + ovntrace_node_append(super, OVNTRACE_NODE_MODIFY,
> + "%s = 0", ds_cstr(&s));
> + ds_destroy(&s);
> +}
> +
> +static void
> +execute_ct_snat_to_vip(struct flow *uflow OVS_UNUSED, struct ovs_list *super)
> +{
> + ovntrace_node_append(super, OVNTRACE_NODE_ERROR,
> + "*** ct_snat_to_vip action not implemented");
> +}
> +
> static void
> trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
> const struct ovntrace_datapath *dp, struct flow *uflow,
> @@ -2438,6 +2489,18 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
> pipeline, super);
> break;
>
> + case OVNACT_CHK_LB_HAIRPIN:
> + execute_chk_lb_hairpin(ovnact_get_CHK_LB_HAIRPIN(a), uflow, super);
> + break;
> +
> + case OVNACT_CHK_LB_HAIRPIN_REPLY:
> + execute_chk_lb_hairpin_reply(ovnact_get_CHK_LB_HAIRPIN_REPLY(a),
> + uflow, super);
> + break;
> + case OVNACT_CT_SNAT_TO_VIP:
> + execute_ct_snat_to_vip(uflow, super);
> + break;
> +
> case OVNACT_TRIGGER_EVENT:
> break;
>
>
More information about the dev
mailing list