On 8/29/22 12:04, Roi Dayan via dev wrote:
>
>
> On 2022-07-27 8:58 AM, Ales Musil wrote:
>> On Tue, Jul 26, 2022 at 2:24 PM Eelco Chaudron <[email protected]> wrote:
>>
>>>
>>>
>>> On 26 Jul 2022, at 14:03, Ales Musil wrote:
>>>
>>>> On Tue, Jul 26, 2022 at 11:40 AM Eelco Chaudron <[email protected]>
>>> wrote:
>>>>
>>>>> When OFPROTO non-reversible actions are translated to data plane
>>>>> actions, the only thing looked at is if there are more actions
>>>>> pending. If this is the case, the action is encapsulated in a
>>>>> clone().
>>>>>
>>>>> This could lead to unnecessary clones if no meaningful data
>>>>> plane actions are added. For example, the register pop in the
>>>>> included test case.
>>>>>
>>>>> The best solution would probably be to build the full action
>>>>> path and determine if the clone is needed. However, this would
>>>>> be a huge change in the existing design, so for now, we just try
>>>>> to optimize the generated datapath flow. We can revisit this
>>>>> later, as some of the pending CT issues might need this rework.
>>>>>
>>>>> Fixes: feee58b9587f ("ofproto-dpif-xlate: Keep track of the last
>>> action")
>>>>> Fixes: dadd8357f224 ("ofproto-dpif: Fix issue with non-reversible
>>> actions
>>>>> on a patch ports.")
>>>>> Signed-off-by: Eelco Chaudron <[email protected]>
>>>>> ---
>>>
>>>>
>>>> Hi,
>>>>
>>>> not sure how much is my review valuable, but:
>>>>
>>>> Acked-by: Ales Musil <[email protected]>
>>>>
>>>
>>> Thanks Ales! Where you able to run some OVN tests with this change, as it
>>> might be useful to know that it does not break OVN ;)
>>>
>>> //Eelco
>>>
>>>
>> I've run the basic set that we have in the repo and that was fine, I've
>> also started to look into a long term solution for this.
>>
>> Regards,
>> Ales
>>
>
> Hi,
>
> we are also running some ovs/ovn tests with this patch.
> we didn't encounter breaking issues with current tests we have.
>
> Thanks,
> Roi
Thanks, Roi, Ales and Eelco! Applied to master and 3.0.
It doesn't apply to earlier branches due to missing test
fixes from commit:
c8bff848aeca ("ofproto-dpif-xlate: No clone when tunnel push is last action.")
But I'm not sure if we actually need to backport further.
Let me know if it is needed.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
