Thank you very much for testing. I applied all four patches to master.
On Sat, Jan 07, 2017 at 06:08:15PM +0800, Dong Jun wrote: > > 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 <b...@ovn.org > >><mailto:b...@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 <b...@ovn.org > >> <mailto:b...@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 > >> d...@openvswitch.org <mailto:d...@openvswitch.org> > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >><https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > >> > >> > > > >_______________________________________________ > >dev mailing list > >d...@openvswitch.org > >https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
