On Fri, Nov 06, 2015 at 04:10:49PM -0800, Jarno Rajahalme wrote:
> When modifying an existing datapath flow with recirculation actions,
> the references to old (if any) recirculation actions need to be freed,
> and references to new recirculation actions need to be stored.
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
> Acked-by: Joe Stringer <[email protected]>
Our coding style calls for a new-line after "void":
> +static void reval_op_init(struct ukey_op *op, enum reval_result result,
> + struct udpif *udpif, struct udpif_key *ukey,
> + struct recirc_refs *recircs,
> + struct ofpbuf *odp_actions)
Here, it wasn't obvious to me why the logic changed from only allocating
a recirc_id if we have a packet, to always allocating one (don't we
still need to reuse the recirc id from a previous translation?):
> @@ -3588,30 +3588,16 @@ compose_recirculate_action__(struct xlate_ctx *ctx,
> uint8_t table)
> .ofpacts = ctx->action_set.data,
> };
>
> - /* Only allocate recirculation ID if we have a packet. */
> - if (ctx->xin->packet) {
> - /* 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(&state);
> - if (!id) {
> - XLATE_REPORT_ERROR(ctx, "Failed to allocate recirculation id");
> - ctx->error = XLATE_NO_RECIRCULATION_CONTEXT;
> - return;
> - }
> - xlate_out_add_recirc(ctx->xout, id);
> - } else {
> - /* 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.
> - * If flow tables have changed sufficiently this can fail and we will
> - * delete the old datapath flow. */
> - id = recirc_find_id(&state);
> - if (!id) {
> - ctx->error = XLATE_NO_RECIRCULATION_CONTEXT;
> - return;
> - }
> + /* 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(&state);
> + if (!id) {
> + XLATE_REPORT_ERROR(ctx, "Failed to allocate recirculation id");
> + ctx->error = XLATE_NO_RECIRCULATION_CONTEXT;
> + return;
> }
> + recirc_refs_add(&ctx->xout->recircs, id);
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
