>-----Original Message-----
>From: Ilya Maximets <i.maxim...@ovn.org>
>Sent: Thursday, 20 March 2025 17:02
>To: Eli Britstein <el...@nvidia.com>; Roi Dayan <r...@nvidia.com>;
>d...@openvswitch.org
>Cc: i.maxim...@ovn.org; Maor Dickman <ma...@nvidia.com>
>Subject: Re: [ovs-dev] [PATCH 0/3] Support tunnel-void action
>
>External email: Use caution opening links or attachments
>
>
>On 3/20/25 15:29, Eli Britstein wrote:
>>
>>
>>> -----Original Message-----
>>> From: Ilya Maximets <i.maxim...@ovn.org>
>>> Sent: Thursday, 20 March 2025 14:59
>>> To: Roi Dayan <r...@nvidia.com>; d...@openvswitch.org
>>> Cc: Maor Dickman <ma...@nvidia.com>; Eli Britstein
>>> <el...@nvidia.com>; i.maxim...@ovn.org
>>> Subject: Re: [ovs-dev] [PATCH 0/3] Support tunnel-void action
>>>
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 3/20/25 10:26, Roi Dayan via dev wrote:
>>>> Upon tunnel output failure, due to routing failure for example, add
>>>> a tunnel-void action. This action does not do anything, but it
>>>> appears in the dp-flows for better visibility for that case.
>>>
>>> Hi, Eli. Thanks for the set!
>>>
>>> I see the issue and agree that we should have better visibility for
>>> it, but we shouldn't add new actions. We have an explicit drop
>>> action exactly for cases like this. So, instead of a new action, we
>>> need to add a new xlate_error type and generate an explicit drop
>>> action (if supported by the datapath) with this error as a drop
>>> reason. And we already have counting of such drops with coverage
>>> counters in the userspace datapath, we'll just need to add a new one for a
>new drop reason. See dp_update_drop_action_counter() function.
>>>
>>> Note: new xlate errors should be added to the end of the enum.
>>
>> Hi Ilya,
>>
>> The "tnl-void" is not a "drop", but a "no-op" action. Only if there are no
>other actions, it is translated to drop.
>> In other cases, it is not.
>> For example, consider a flow that follows this openflow (that fails routing):
>> In_port=<P1>,actions=<vxlan0>,<P2>
>>
>> With the absence of "tnl-void", the datapath flow is simply
>> in_port=<P1>,actions=<P2> I omit the other default matches, just to
>explain.
>>
>> With those patches, the datapath flow will be
>> in_port=<P1>,actions=tnl_void,<P2>
>
>If the tunnel is configured properly datapath actions will be:
>
> actions=clone(tnl_push(),<out-port>),<P2>
>
>If not, it will be:
>
> actions=clone(drop(reason)),<P2>.
>
>Tunnel push changes the packet, so the clone is necessary. Drop also
>"changes" the packet, so yes, the clone is necessary as well.
>
>So, it's a very similar action structure. Optimizing for a flow translation
>failure
>by not including the clone() in the drop case doesn't seem to make a lot of
>sense. If needed, we may optimize the clone(drop) in the datapth while
>executing the action, so it will also mostly be a no-op, though it seems a
>little
>unnecessary.
Thanks for that approach. See
https://patchwork.ozlabs.org/project/openvswitch/list/?series=449757
>
>Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
