[ovs-dev] Issues with the use of the clone action for resubmission to the pipeline

Ben Pfaff blp at ovn.org
Fri Jan 6 00:28:50 UTC 2017


On Tue, Jan 03, 2017 at 02:55:19AM -0800, Mickey Spiegel wrote:
> One of the motivations for clone is to use it as a lightweight way to
> resubmit to an earlier table at the beginning of the pipeline, without
> incurring all of the overhead associated with openflow patch ports.
> One such usage is in OVN, where a recent patch set replaced the
> use of openflow patch ports with clone, for OVN patch ports within
> the same bridge (br-int).
>
> Over the holidays, some issues arose related to this usage of clone
> (see the thread ovn ping from VM to external gateway IP failed).

Thanks for bringing this up.  I had overlooked these questions and this
issue.

> While investigating these issues, I noticed some significant
> differences between the way flow and other context information
> is saved and restored when using openflow patch ports, versus
> the way it is done with clone. At least one such difference, with
> regard to conntrack, is causing failures.

What's the difference that is causing failures?  I do not know about it,
so it is a high priority to fix it.  Is it already fixed, or do we need
to do something about it?

> I would classify the differences (those for which there is no current
> workaround by specifying nested actions within the clone) into two
> categories:
> 1. State that is kept outside of flow, which is currently saved,
>    cleared, and restored when using openflow patch ports.
>    This includes:
>    ctx->wc->masks.tunnel

I think that clone should not save and restore this.  The situation is
different from a patch port because clone does not change the ingress
port.  The patch port code has the following comment:

        /* Since this packet came in on a patch port (from the perspective of
         * the peer bridge), it cannot have useful tunnel information. As a
         * result, any wildcards generated on that tunnel also cannot be valid.
         * The tunnel wildcards must be restored to their original version since
         * the peer bridge uses a separate tunnel metadata table and therefore
         * any generated wildcards will be garbage in the context of our
         * metadata table. */
        ctx->wc->masks.tunnel = old_flow_tnl_wc;

>    ctx->conntracked

I guess that we should save and restore this since we're saving and
restoring the conntrack metadata.  I've written up a patch.

>    ctx->was_mpls

I do not think that that this is a correctness issue, but it's a nice
optimization to save and restore this.  I've written up a patch.

>    ctx->xin->tables_version (not an issue if bridge does not change)

clone doesn't change the bridge, so this shouldn't matter.

>    ctx->stack
>    ctx->action_set

I think it's cleanest if a clone starts off with both of these empty and
saves and restores them.  I've written up a patch.

>    ctx->xbridge (not an issue if bridge does not change)
>    ctx->mirrors (not an issue if bridge does not change)

clone doesn't change the bridge, so these shouldn't matter.

> 2. State that resides inside the flow, but for which there is no
>    workaround to clear or reset the fields. This includes:
>    flow->tunnel ?

Like the tunnel mask, this doesn't matter for clone, because clone
doesn't change the ingress port.

>    flow->tunnel.metadata.tab (not an issue if bridge does not change)

clone doesn't change the bridge, so this shouldn't matter.

>    flow->ct_state (read only)
>    flow->ct_mark (restricted so can only be set in ct action)
>    flow->ct_label (restricted so can only be set in ct action)

It's not obvious to me that clone should always clear these.  However,
if it saves and restores them, then we can support clearing them by
adding a ct_clear action.  I've written up a patch.

I sent out a series that addresses these issues.  Probably, we also need
to modify OVN to insert a "ct_clear" action into its use of "clone", but
my series doesn't do that.

The series starts here:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327204.html


More information about the dev mailing list