Skip to content

Commit

Permalink
ovn: Don't require clearing inport to hair-pin packets.
Browse files Browse the repository at this point in the history 
Introduce the "flags.loopback" symbol to allow packets to be sent back
on their ingress ports.  Previously, one needed to clear "inport" to
hair-pin packets, but this made "inport" not available for future
matching.  This approach should be more intuitive, but it will also be
needed in future patches.

This patch also removes functionality from the OVN expression library
that clears the OpenFlow ingress port when the logical input port is
zeroed.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
@justinpettit
justinpettit committed Jul 29, 2016
1 parent 6fdb7cd commit bf14349
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 68 deletions.
8 changes: 7 additions & 1 deletion ovn/controller/lflow.c
View file
Expand Up @@ -81,6 +81,12 @@ lflow_init(void)
expr_symtab_add_field(&symtab, "xxreg0", MFF_XXREG0, NULL, false);
expr_symtab_add_field(&symtab, "xxreg1", MFF_XXREG1, NULL, false);

/* Flags used in logical to physical transformation. */
expr_symtab_add_field(&symtab, "flags", MFF_LOG_FLAGS, NULL, false);
char flags_str[16];
snprintf(flags_str, sizeof flags_str, "flags[%d]", MLF_ALLOW_LOOPBACK_BIT);
expr_symtab_add_subfield(&symtab, "flags.loopback", NULL, flags_str);

/* Connection tracking state. */
expr_symtab_add_field(&symtab, "ct_mark", MFF_CT_MARK, NULL, false);
expr_symtab_add_field(&symtab, "ct_label", MFF_CT_LABEL, NULL, false);
Expand Down Expand Up @@ -494,7 +500,7 @@ consider_logical_flow(const struct lport_index *lports,
uint8_t ptable = first_ptable + lflow->table_id;
uint8_t output_ptable = (ingress
? OFTABLE_REMOTE_OUTPUT
: OFTABLE_LOG_TO_PHY);
: OFTABLE_SAVE_INPORT);

/* Translate OVN actions into OpenFlow actions.
*
Expand Down
7 changes: 4 additions & 3 deletions ovn/controller/lflow.h
View file
Expand Up @@ -51,10 +51,11 @@ struct uuid;
#define OFTABLE_LOG_INGRESS_PIPELINE 16 /* First of LOG_PIPELINE_LEN tables. */
#define OFTABLE_REMOTE_OUTPUT 32
#define OFTABLE_LOCAL_OUTPUT 33
#define OFTABLE_DROP_LOOPBACK 34
#define OFTABLE_CHECK_LOOPBACK 34
#define OFTABLE_LOG_EGRESS_PIPELINE 48 /* First of LOG_PIPELINE_LEN tables. */
#define OFTABLE_LOG_TO_PHY 64
#define OFTABLE_MAC_BINDING 65
#define OFTABLE_SAVE_INPORT 64
#define OFTABLE_LOG_TO_PHY 65
#define OFTABLE_MAC_BINDING 66

/* The number of tables for the ingress and egress pipelines. */
#define LOG_PIPELINE_LEN 16
Expand Down
46 changes: 40 additions & 6 deletions ovn/controller/physical.c
View file
Expand Up @@ -375,23 +375,47 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
}

/* Resubmit to table 34. */
put_resubmit(OFTABLE_DROP_LOOPBACK, ofpacts_p);
put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT, 100,
&match, ofpacts_p, &binding->header_.uuid);

/* Table 34, Priority 100.
* =======================
*
* Drop packets whose logical inport and outport are the same. */
* Drop packets whose logical inport and outport are the same
* and the MLF_ALLOW_LOOPBACK flag is not set. */
match_init_catchall(&match);
ofpbuf_clear(ofpacts_p);
match_set_metadata(&match, htonll(dp_key));
match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
0, MLF_ALLOW_LOOPBACK);
match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, port_key);
match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, port_key);
ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 100,
ofctrl_add_flow(OFTABLE_CHECK_LOOPBACK, 100,
&match, ofpacts_p, &binding->header_.uuid);

/* Table 64, Priority 100.
* =======================
*
* If the packet is supposed to hair-pin because the "loopback"
* flag is set, temporarily set the in_port to zero, resubmit to
* table 65 for logical-to-physical translation, then restore
* the port number. */
match_init_catchall(&match);
ofpbuf_clear(ofpacts_p);
match_set_metadata(&match, htonll(dp_key));
match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
MLF_ALLOW_LOOPBACK, MLF_ALLOW_LOOPBACK);
match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, port_key);

put_stack(MFF_IN_PORT, ofpact_put_STACK_PUSH(ofpacts_p));
put_load(0, MFF_IN_PORT, 0, 16, ofpacts_p);
put_resubmit(OFTABLE_LOG_TO_PHY, ofpacts_p);
put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(ofpacts_p));
ofctrl_add_flow(OFTABLE_SAVE_INPORT, 100,
&match, ofpacts_p, &binding->header_.uuid);

/* Table 65, Priority 100.
* =======================
*
* Deliver the packet to the local vif. */
Expand Down Expand Up @@ -523,12 +547,12 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
if (!strcmp(port->type, "patch")) {
put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32,
remote_ofpacts_p);
put_resubmit(OFTABLE_DROP_LOOPBACK, remote_ofpacts_p);
put_resubmit(OFTABLE_CHECK_LOOPBACK, remote_ofpacts_p);
} else if (simap_contains(&localvif_to_ofport,
(port->parent_port && *port->parent_port)
? port->parent_port : port->logical_port)) {
put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, ofpacts_p);
put_resubmit(OFTABLE_DROP_LOOPBACK, ofpacts_p);
put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
} else if (port->chassis && !get_localnet_port(local_datapaths,
mc->datapath->tunnel_key)) {
/* Add remote chassis only when localnet port not exist,
Expand Down Expand Up @@ -888,7 +912,17 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
MFF_LOG_REGS;
#undef MFF_LOG_REGS
put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, &ofpacts);
ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts, hc_uuid);
ofctrl_add_flow(OFTABLE_CHECK_LOOPBACK, 0, &match, &ofpacts, hc_uuid);

/* Table 64, Priority 0.
* =======================
*
* Resubmit packets that do not have the MLF_ALLOW_LOOPBACK flag set
* to table 65 for logical-to-physical translation. */
match_init_catchall(&match);
ofpbuf_clear(&ofpacts);
put_resubmit(OFTABLE_LOG_TO_PHY, &ofpacts);
ofctrl_add_flow(OFTABLE_SAVE_INPORT, 0, &match, &ofpacts, hc_uuid);

ofpbuf_uninit(&ofpacts);

Expand Down
10 changes: 0 additions & 10 deletions ovn/lib/expr.c
View file
Expand Up @@ -2872,16 +2872,6 @@ parse_assignment(struct lexer *lexer, struct expr_field *dst,
bitwise_put(port, &sf->value,
sf->field->n_bytes, 0, sf->field->n_bits);
bitwise_one(&sf->mask, sf->field->n_bytes, 0, sf->field->n_bits);

/* If the logical input port is being zeroed, clear the OpenFlow
* ingress port also, to allow a packet to be sent back to its
* origin. */
if (!port && sf->field->id == MFF_LOG_INPORT) {
sf = ofpact_put_SET_FIELD(ofpacts);
sf->field = mf_from_id(MFF_IN_PORT);
bitwise_one(&sf->mask,
sf->field->n_bytes, 0, sf->field->n_bits);
}
}

exit_destroy_cs:
Expand Down
10 changes: 10 additions & 0 deletions ovn/lib/logical-fields.h
View file
Expand Up @@ -18,11 +18,21 @@

#include "openvswitch/meta-flow.h"

enum {
MLF_ALLOW_LOOPBACK_BIT = 0
};

enum {
MLF_ALLOW_LOOPBACK = (1 << MLF_ALLOW_LOOPBACK_BIT) /* Allow outputting
back to inport. */
};

/* Logical fields.
*
* These values are documented in ovn-architecture(7), please update the
* documentation if you change any of them. */
#define MFF_LOG_DATAPATH MFF_METADATA /* Logical datapath (64 bits). */
#define MFF_LOG_FLAGS MFF_REG10 /* One of MLF_* (32 bits). */
#define MFF_LOG_DNAT_ZONE MFF_REG11 /* conntrack dnat zone for gateway router
* (32 bits). */
#define MFF_LOG_SNAT_ZONE MFF_REG12 /* conntrack snat zone for gateway router
Expand Down
26 changes: 13 additions & 13 deletions ovn/northd/ovn-northd.8.xml
View file
Expand Up @@ -441,7 +441,7 @@ arp.sha = <var>E</var>;
arp.tpa = arp.spa;
arp.spa = <var>A</var>;
outport = inport;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
output;
</pre>

Expand All @@ -467,7 +467,7 @@ nd_na {
nd.target = <var>A</var>;
nd.tll = <var>E</var>;
outport = inport;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
output;
};
</pre>
Expand Down Expand Up @@ -546,7 +546,7 @@ ip4.src = <var>S</var>;
udp.src = 67;
udp.dst = 68;
outport = <var>P</var>;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
output;
</pre>

Expand Down Expand Up @@ -772,7 +772,7 @@ output;
ip4.dst &lt;-&gt; ip4.src;
ip.ttl = 255;
icmp4.type = 0;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
next;
</pre>

Expand All @@ -784,7 +784,7 @@ next;
ip6.dst &lt;-&gt; ip6.src;
ip.ttl = 255;
icmp6.type = 129;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
next;
</pre>
</li>
Expand Down Expand Up @@ -812,7 +812,7 @@ arp.sha = <var>E</var>;
arp.tpa = arp.spa;
arp.spa = <var>A</var>;
outport = <var>P</var>;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
output;
</pre>
</li>
Expand All @@ -837,7 +837,7 @@ arp.sha = <var>E</var>;
arp.tpa = arp.spa;
arp.spa = <var>A</var>;
outport = <var>P</var>;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
output;
</pre>
</li>
Expand Down Expand Up @@ -872,7 +872,7 @@ nd_na {
nd.target = <var>A</var>;
nd.tll = <var>E</var>;
outport = inport;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
output;
};
</pre>
Expand Down Expand Up @@ -1032,13 +1032,13 @@ icmp4 {
For each configuration in the OVN Northbound database, that asks
to change the destination IP address of a packet from <var>A</var> to
<var>B</var>, a priority-100 flow matches <code>ip &amp;&amp;
ip4.dst == <var>A</var></code> with an action <code>inport = "";
ct_dnat(<var>B</var>);</code>.
ip4.dst == <var>A</var></code> with an action
<code>flags.loopback = 1; ct_dnat(<var>B</var>);</code>.
</p>

<p>
For all IP packets of a Gateway router, a priority-50 flow with an
action <code>inport = ""; ct_dnat;</code>.
action <code>flags.loopback = 1; ct_dnat;</code>.
</p>

<p>
Expand Down Expand Up @@ -1084,7 +1084,7 @@ reg0 = <var>G</var>;
reg1 = <var>A</var>;
eth.src = <var>E</var>;
outport = <var>P</var>;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
next;
</pre>

Expand Down Expand Up @@ -1117,7 +1117,7 @@ xxreg0 = <var>G</var>;
xxreg1 = <var>A</var>;
eth.src = <var>E</var>;
outport = <var>P</var>;
inport = ""; /* Allow sending out inport. */
flags.loopback = 1;
next;
</pre>

Expand Down
24 changes: 12 additions & 12 deletions ovn/northd/ovn-northd.c
View file
Expand Up @@ -1802,8 +1802,8 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,

ds_put_format(response_action, "eth.dst = eth.src; eth.src = %s; "
"ip4.dst = "IP_FMT"; ip4.src = %s; udp.src = 67; "
"udp.dst = 68; outport = inport; inport = \"\";"
" /* Allow sending out inport. */ output;",
"udp.dst = 68; outport = inport; flags.loopback = 1; "
"output;",
server_mac, IP_ARGS(offer_ip), server_ip);

smap_destroy(&dhcpv4_options);
Expand Down Expand Up @@ -2470,7 +2470,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
"arp.tpa = arp.spa; "
"arp.spa = %s; "
"outport = inport; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"output;",
op->lsp_addrs[i].ea_s, op->lsp_addrs[i].ea_s,
op->lsp_addrs[i].ipv4_addrs[j].addr_s);
Expand All @@ -2497,7 +2497,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
"nd.target = %s; "
"nd.tll = %s; "
"outport = inport; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"output; "
"};",
op->lsp_addrs[i].ea_s,
Expand Down Expand Up @@ -2793,7 +2793,7 @@ add_route(struct hmap *lflows, const struct ovn_port *op,
"%sreg1 = %s; "
"eth.src = %s; "
"outport = %s; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"next;",
is_ipv4 ? "" : "xx",
lrp_addr_s,
Expand Down Expand Up @@ -3074,7 +3074,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
"ip4.dst <-> ip4.src; "
"ip.ttl = 255; "
"icmp4.type = 0; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"next; ");
ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
ds_cstr(&match), ds_cstr(&actions));
Expand All @@ -3098,7 +3098,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
"arp.tpa = arp.spa; "
"arp.spa = %s; "
"outport = %s; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"output;",
op->lrp_networks.ea_s,
op->lrp_networks.ea_s,
Expand Down Expand Up @@ -3147,7 +3147,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
"arp.tpa = arp.spa; "
"arp.spa = "IP_FMT"; "
"outport = %s; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"output;",
op->lrp_networks.ea_s,
op->lrp_networks.ea_s,
Expand Down Expand Up @@ -3216,7 +3216,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
"ip6.dst <-> ip6.src; "
"ip.ttl = 255; "
"icmp6.type = 129; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"next; ");
ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
ds_cstr(&match), ds_cstr(&actions));
Expand Down Expand Up @@ -3249,7 +3249,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
"nd.target = %s; "
"nd.tll = %s; "
"outport = inport; "
"inport = \"\"; /* Allow sending out inport. */ "
"flags.loopback = 1; "
"output; "
"};",
op->lrp_networks.ea_s,
Expand Down Expand Up @@ -3345,7 +3345,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
ds_clear(&match);
ds_put_format(&match, "ip && ip4.dst == %s", nat->external_ip);
ds_clear(&actions);
ds_put_format(&actions,"inport = \"\"; ct_dnat(%s);",
ds_put_format(&actions,"flags.loopback = 1; ct_dnat(%s);",
nat->logical_ip);
ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100,
ds_cstr(&match), ds_cstr(&actions));
Expand Down Expand Up @@ -3385,7 +3385,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
* back the new destination IP address that is needed for
* routing in the openflow pipeline. */
ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 50,
"ip", "inport = \"\"; ct_dnat;");
"ip", "flags.loopback = 1; ct_dnat;");
}

/* Logical router ingress table 4: IP Routing.
Expand Down

0 comments on commit bf14349

Please sign in to comment.