When traversing from one bridge to another via a patch port, we should drop conntrack metadata from the OpenFlow pipeline. This should only affect the subsequent processing on the destination bridge (and not continued processing, resubmits, etc. on the original bridge).
If conntrack is executed for the current bridge, then recirculation should make the conntrack fields available for matching. In all other cases, the conntrack fields should be zeroed during upcall processing. CC: Jarno Rajahalme <[email protected]> Signed-off-by: Joe Stringer <[email protected]> --- This should be folded into earlier conntrack patches, but I have left it as an independent patch for now to aid review. --- lib/ofp-actions.h | 5 +++ ofproto/ofproto-dpif-rid.c | 80 +++++++++++++++++++++++------------------- ofproto/ofproto-dpif-rid.h | 15 ++++---- ofproto/ofproto-dpif-xlate.c | 46 ++++++++++++++++++++---- 4 files changed, 97 insertions(+), 49 deletions(-) diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h index 01958c2..0063b60 100644 --- a/lib/ofp-actions.h +++ b/lib/ofp-actions.h @@ -748,6 +748,11 @@ struct ofpact_unroll_xlate { /* Metadata in xlate context, visible to controller via PACKET_INs. */ uint8_t rule_table_id; /* 0xFF if none. */ ovs_be64 rule_cookie; /* OVS_BE64_MAX if none. */ + + /* Whether conntrack was executed prior to recirculation. If so, related + * fields may be made available post-recirculation, until peer traversal + * or subsequent conntrack execution. */ + bool conntracked; }; /* Converting OpenFlow to ofpacts. */ diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index f1b3bdc..c539e0a 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -125,9 +125,9 @@ recirc_id_node_find(uint32_t id) static uint32_t recirc_metadata_hash(struct ofproto_dpif *ofproto, uint8_t table_id, - struct recirc_metadata *md, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *ofpacts) + bool conntrack, struct recirc_metadata *md, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *ofpacts) { uint32_t hash; @@ -135,6 +135,7 @@ recirc_metadata_hash(struct ofproto_dpif *ofproto, uint8_t table_id, hash = hash_pointer(ofproto, 0); hash = hash_int(table_id, hash); + hash = hash_boolean(conntrack, hash); hash = hash_words64((const uint64_t *)md, sizeof *md / sizeof(uint64_t), hash); if (stack && stack->size != 0) { @@ -153,12 +154,13 @@ recirc_metadata_hash(struct ofproto_dpif *ofproto, uint8_t table_id, static bool recirc_metadata_equal(const struct recirc_id_node *node, struct ofproto_dpif *ofproto, uint8_t table_id, - struct recirc_metadata *md, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *ofpacts) + bool conntrack, struct recirc_metadata *md, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *ofpacts) { return node->ofproto == ofproto && node->table_id == table_id + && node->conntrack == conntrack && !memcmp(&node->metadata, md, sizeof *md) && ((!node->stack && (!stack || stack->size == 0)) || (node->stack && stack && ofpbuf_equal(node->stack, stack))) @@ -171,15 +173,17 @@ recirc_metadata_equal(const struct recirc_id_node *node, * state, caller should take a reference. */ static struct recirc_id_node * recirc_find_equal(struct ofproto_dpif *ofproto, uint8_t table_id, - struct recirc_metadata *md, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *ofpacts, uint32_t hash) + bool conntrack, struct recirc_metadata *md, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *ofpacts, + uint32_t hash) { struct recirc_id_node *node; CMAP_FOR_EACH_WITH_HASH(node, metadata_node, hash, &metadata_map) { - if (recirc_metadata_equal(node, ofproto, table_id, md, stack, - action_set_len, ofpacts_len, ofpacts)) { + if (recirc_metadata_equal(node, ofproto, table_id, conntrack, md, + stack, action_set_len, ofpacts_len, + ofpacts)) { return node; } } @@ -188,15 +192,16 @@ recirc_find_equal(struct ofproto_dpif *ofproto, uint8_t table_id, static struct recirc_id_node * recirc_ref_equal(struct ofproto_dpif *ofproto, uint8_t table_id, - struct recirc_metadata *md, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *ofpacts, uint32_t hash) + bool conntrack, struct recirc_metadata *md, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *ofpacts, + uint32_t hash) { struct recirc_id_node *node; do { - node = recirc_find_equal(ofproto, table_id, md, stack, action_set_len, - ofpacts_len, ofpacts, hash); + node = recirc_find_equal(ofproto, table_id, conntrack, md, stack, + action_set_len, ofpacts_len, ofpacts, hash); /* Try again if the node was released before we get the reference. */ } while (node && !ovs_refcount_try_ref_rcu(&node->refcount)); @@ -210,9 +215,10 @@ recirc_ref_equal(struct ofproto_dpif *ofproto, uint8_t table_id, * hash is recomputed if it is passed in as 0. */ static struct recirc_id_node * recirc_alloc_id__(struct ofproto_dpif *ofproto, uint8_t table_id, - struct recirc_metadata *md, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *ofpacts, uint32_t hash) + bool conntrack, struct recirc_metadata *md, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *ofpacts, + uint32_t hash) { struct recirc_id_node *node = xzalloc(sizeof *node + OFPACT_ALIGN(ofpacts_len)); @@ -221,6 +227,7 @@ recirc_alloc_id__(struct ofproto_dpif *ofproto, uint8_t table_id, node->ofproto = ofproto; node->table_id = table_id; + node->conntrack = conntrack; memcpy(&node->metadata, md, sizeof node->metadata); node->stack = (stack && stack->size) ? ofpbuf_clone(stack) : NULL; node->action_set_len = action_set_len; @@ -254,7 +261,7 @@ recirc_alloc_id__(struct ofproto_dpif *ofproto, uint8_t table_id, /* Look up an existing ID for the given flow's metadata and optional actions. */ uint32_t -recirc_find_id(struct ofproto_dpif *ofproto, uint8_t table_id, +recirc_find_id(struct ofproto_dpif *ofproto, uint8_t table_id, bool conntrack, struct recirc_metadata *md, struct ofpbuf *stack, uint32_t action_set_len, uint32_t ofpacts_len, const struct ofpact *ofpacts) @@ -263,10 +270,10 @@ recirc_find_id(struct ofproto_dpif *ofproto, uint8_t table_id, struct recirc_id_node *node; uint32_t hash; - hash = recirc_metadata_hash(ofproto, table_id, md, stack, action_set_len, - ofpacts_len, ofpacts); - node = recirc_find_equal(ofproto, table_id, md, stack, action_set_len, - ofpacts_len, ofpacts, hash); + hash = recirc_metadata_hash(ofproto, table_id, conntrack, md, stack, + action_set_len, ofpacts_len, ofpacts); + node = recirc_find_equal(ofproto, table_id, conntrack, md, stack, + action_set_len, ofpacts_len, ofpacts, hash); return node ? node->id : 0; } @@ -275,25 +282,25 @@ recirc_find_id(struct ofproto_dpif *ofproto, uint8_t table_id, optional actions. */ uint32_t recirc_alloc_id_ctx(struct ofproto_dpif *ofproto, uint8_t table_id, - struct recirc_metadata *md, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *ofpacts) + bool conntrack, struct recirc_metadata *md, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *ofpacts) { struct recirc_id_node *node; uint32_t hash; /* Look up an existing ID. */ - hash = recirc_metadata_hash(ofproto, table_id, md, stack, action_set_len, - ofpacts_len, ofpacts); - node = recirc_ref_equal(ofproto, table_id, md, stack, action_set_len, - ofpacts_len, ofpacts, hash); + hash = recirc_metadata_hash(ofproto, table_id, conntrack, md, stack, + action_set_len,ofpacts_len, ofpacts); + node = recirc_ref_equal(ofproto, table_id, conntrack, md, stack, + action_set_len, ofpacts_len, ofpacts, hash); /* Allocate a new recirc ID if needed. */ if (!node) { ovs_assert(action_set_len <= ofpacts_len); - node = recirc_alloc_id__(ofproto, table_id, md, stack, action_set_len, - ofpacts_len, ofpacts, hash); + node = recirc_alloc_id__(ofproto, table_id, conntrack, md, stack, + action_set_len, ofpacts_len, ofpacts, hash); } return node->id; @@ -309,9 +316,10 @@ recirc_alloc_id(struct ofproto_dpif *ofproto) memset(&md, 0, sizeof md); md.in_port = OFPP_NONE; - hash = recirc_metadata_hash(ofproto, TBL_INTERNAL, &md, NULL, 0, 0, NULL); - node = recirc_alloc_id__(ofproto, TBL_INTERNAL, &md, NULL, 0, 0, NULL, - hash); + hash = recirc_metadata_hash(ofproto, TBL_INTERNAL, false, &md, NULL, 0, 0, + NULL); + node = recirc_alloc_id__(ofproto, TBL_INTERNAL, false, &md, NULL, 0, 0, + NULL, hash); return node->id; } diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index dc533ce..db77144 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -138,6 +138,9 @@ struct recirc_id_node { /* Initial table for post-recirculation processing. */ uint8_t table_id; + /* True if conntracking occurred prior to recirculation. */ + bool conntrack; + /* Pipeline context for post-recirculation processing. */ struct ofproto_dpif *ofproto; /* Post-recirculation bridge. */ struct recirc_metadata metadata; /* Flow metadata. */ @@ -157,13 +160,13 @@ void recirc_init(void); uint32_t recirc_alloc_id(struct ofproto_dpif *); uint32_t recirc_alloc_id_ctx(struct ofproto_dpif *, uint8_t table_id, - struct recirc_metadata *, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *); + bool conntrack, struct recirc_metadata *, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *); uint32_t recirc_find_id(struct ofproto_dpif *, uint8_t table_id, - struct recirc_metadata *, struct ofpbuf *stack, - uint32_t action_set_len, uint32_t ofpacts_len, - const struct ofpact *); + bool conntrack, struct recirc_metadata *, + struct ofpbuf *stack, uint32_t action_set_len, + uint32_t ofpacts_len, const struct ofpact *); void recirc_free_id(uint32_t recirc_id); void recirc_free_ofproto(struct ofproto_dpif *, const char *ofproto_name); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 5561410..9a9b4a6 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -283,6 +283,11 @@ struct xlate_ctx { * the MPLS label stack that was originally present. */ bool was_mpls; + /* True if conntrack has been performed on this packet during processing + * on the current bridge. This is used to determine whether conntrack + * state from the datapath should be honored after recirculation. */ + bool conntracked; + /* OpenFlow 1.1+ action set. * * 'action_set' accumulates "struct ofpact"s added by OFPACT_WRITE_ACTIONS. @@ -2714,6 +2719,15 @@ build_tunnel_send(const struct xlate_ctx *ctx, const struct xport *xport, } static void +clear_conntrack(struct flow *flow) +{ + flow->conn_state = 0; + flow->conn_zone = 0; + flow->conn_mark = 0; + memset(&flow->conn_label, 0, sizeof flow->conn_label); +} + +static void compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, const struct xlate_bond_recirc *xr, bool check_stp) { @@ -2777,6 +2791,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, const struct xport *peer = xport->peer; struct flow old_flow = ctx->xin->flow; bool old_was_mpls = ctx->was_mpls; + bool old_conntracked = ctx->conntracked; enum slow_path_reason special; struct ofpbuf old_stack = ctx->stack; union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)]; @@ -2792,6 +2807,10 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, memset(flow->regs, 0, sizeof flow->regs); flow->actset_output = OFPP_UNSET; + /* Drop conntrack metadata when traversing a peer. */ + clear_conntrack(flow); + ctx->conntracked = false; + special = process_special(ctx, &ctx->xin->flow, peer, ctx->xin->packet); if (special) { @@ -2842,6 +2861,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, * bridge. */ ctx->was_mpls = old_was_mpls; + ctx->conntracked = old_conntracked; + /* The fact that the peer bridge exits (for any reason) does not mean * that the original bridge should exit. Specifically, if the peer * bridge recirculates (which typically modifies the packet), the @@ -3483,8 +3504,9 @@ compose_recirculate_action__(struct xlate_ctx *ctx, struct ofpbuf *stack, /* Allocate a unique recirc id for the given metadata state in the * flow. The life-cycle of this recirc id is managed by associating it * with the udpif key ('ukey') created for each new datapath flow. */ - id = recirc_alloc_id_ctx(ctx->xbridge->ofproto, 0, &md, stack, - action_offset, ofpacts_len, ofpacts); + id = recirc_alloc_id_ctx(ctx->xbridge->ofproto, 0, ctx->conntracked, + &md, stack, action_offset, ofpacts_len, + ofpacts); if (!id) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); VLOG_ERR_RL(&rl, "Failed to allocate recirculation id"); @@ -3495,8 +3517,8 @@ compose_recirculate_action__(struct xlate_ctx *ctx, struct ofpbuf *stack, /* Look up an existing recirc id for the given metadata state in the * flow. No new reference is taken, as the ID is RCU protected and is * only required temporarily for verification. */ - id = recirc_find_id(ctx->xbridge->ofproto, 0, &md, stack, - action_offset, ofpacts_len, ofpacts); + id = recirc_find_id(ctx->xbridge->ofproto, 0, ctx->conntracked, &md, + stack, action_offset, ofpacts_len, ofpacts); /* We let zero 'id' to be used in the RECIRC action below, which will * fail all revalidations as zero is not a valid recirculation ID. */ } @@ -3983,14 +4005,16 @@ recirc_put_unroll_xlate(struct xlate_ctx *ctx) /* Restore the table_id and rule cookie for a potential PACKET * IN if needed. */ - if (!unroll || - (ctx->table_id != unroll->rule_table_id - || ctx->rule_cookie != unroll->rule_cookie)) { + if (!unroll + || ctx->table_id != unroll->rule_table_id + || ctx->rule_cookie != unroll->rule_cookie + || ctx->conntracked) { ctx->last_unroll_offset = ctx->action_set.size; unroll = ofpact_put_UNROLL_XLATE(&ctx->action_set); unroll->rule_table_id = ctx->table_id; unroll->rule_cookie = ctx->rule_cookie; + unroll->conntracked = ctx->conntracked; } } @@ -4113,6 +4137,9 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc) put_connhelper(odp_actions, ofc); nl_msg_end_nested(odp_actions, ct_offset); + /* Use conn_* fields from datapath during recirculation upcall. */ + ctx->conntracked = true; + if (ofc->flags & NX_CT_F_RECIRC) { compose_recirculate_action__(ctx, NULL, 0, 0, NULL); } @@ -4436,6 +4463,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, /* Restore translation context data that was stored earlier. */ ctx->table_id = unroll->rule_table_id; ctx->rule_cookie = unroll->rule_cookie; + ctx->conntracked = unroll->conntracked; break; } case OFPACT_FIN_TIMEOUT: @@ -4848,6 +4876,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) * only if there are no post-recirculation actions. */ ctx.table_id = recirc->table_id; + if (!recirc->conntrack) { + clear_conntrack(flow); + } + /* Restore pipeline metadata. May change flow's in_port and other * metadata to the values that existed when recirculation was * triggered. */ -- 1.7.10.4 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
