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

Dong Jun dongj at dtdream.com
Sat Jan 7 10:08:15 UTC 2017


On 2017/1/7 17:14, Dong Jun wrote:
> I tested my 
> issue(https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326936.html)
> withpatch serial v3 1-4 (http://patchwork.ozlabs.org/patch/712028/).
It sounds a little vague, i mean all four patches from 1/4 to 4/4.
>
> The issue has been resolved. Gateway NAT and floating ip NAT both 
> worked well and conntrack
> flows were completed as well.
> Thank you.
>
>
> On 2017/1/6 20:24, Numan Siddique wrote:
>>
>>
>> On Fri, Jan 6, 2017 at 9:52 AM, Ben Pfaff <blp at ovn.org 
>> <mailto:blp at ovn.org>> wrote:
>>
>>     On Thu, Jan 05, 2017 at 05:54:46PM -0800, Jarno Rajahalme wrote:
>>     >
>>     > > On Jan 5, 2017, at 4:28 PM, Ben Pfaff <blp at ovn.org
>>     <mailto:blp at ovn.org>> wrote:
>>     > >
>>     > > 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.
>>     > >
>>     > > 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.
>>     >
>>     > I think the rationale is that ‘was_mpls’ is used to track the
>>     validity
>>     > of the flow key across an MPLS POP operation. Since the flow 
>> key is
>>     > saved and restored, we should save and restore ‘was_mpls’ as well.
>>
>>     OK.  I did send a patch that adds that.  Do you want to make a
>>     suggestion for the commit message?  I may not fully understand the
>>     issue
>>     yet, so I don't think the commit message is very good.
>>
>>     Here's the patch:
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327207.html
>> <https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327207.html>
>>
>>     > >>   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.
>>     >
>>     > I think saving and restoring is needed, but I’m not so sure of
>>     > clearing these. However, it seems that there is no way for the
>>     action
>>     > set to be executed within the clone, so I guess it does not 
>> matter.
>>
>>     I guess that it would also be a clean design, and consistent with
>>     my new
>>     ct_clear action, to not clear them but instead to start from a
>>     copy and
>>     allow for clearing them explicitly within the clone.
>>
>>     There is already an instruction to clear the action set, so we
>>     wouldn't
>>     need to add anything.  I think that the action set can only affect
>>     what
>>     happens inside the clone in terms of matches or actions based on the
>>     actset_output field, though.
>>
>>     I'm not sure of the value of an action to clear the stack, so I'd be
>>     inclined to hold off on that until we think of one.
>>
>>     I'll revise my patch to work this way.
>>
>>     > It would be good to add these changes to the documentation as 
>> well.
>>
>>     My patch does update the documentation on this point.
>>
>>
>> ​Thanks Ben for all the fixes. We are in middle of performance 
>> testing with the version of ovn-controller which creates patch ports 
>> for router ports.
>>
>> Once this is done, we will be able to test with the patches you have 
>> proposed.
>>
>> ​Dong Jun- May be if you want to test these patches and I see if it 
>> resolves the issues which you had posted.
>>>>
>> Thanks
>> Numan
>>
>>
>>     _______________________________________________
>>     dev mailing list
>>     dev at openvswitch.org <mailto:dev at openvswitch.org>
>>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>>
>>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



More information about the dev mailing list