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. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
