>-----Original Message-----
>From: Ilya Maximets <i.maxim...@ovn.org>
>Sent: Wednesday, 3 January 2024 19:12
>To: Eli Britstein <el...@nvidia.com>; ovs-discuss@openvswitch.org
>Cc: Maor Dickman <ma...@nvidia.com>; i.maxim...@ovn.org
>Subject: Re: [ovs-discuss] Match on header rewrite fields
>
>External email: Use caution opening links or attachments
>
>
>On 12/19/23 17:00, Eli Britstein via discuss wrote:
>> Hello,
>>
>> In case there is a header rewrite action, OVS implicitly adds match on the
>corresponding fields.
>>
>> For example:
>> ovs-ofctl add-flow br-int0
>'in_port=enp8s0f0,ip,actions=mod_dl_dst=00:11:22:33:44:55,enp8s0f0_0'
>>
>> The datapath flow will have an exact match on dl_dst field.
>>
>> Does someone know what is the logic behind this?
>
>Hi, Eli. The main reason for this, AFAIK, is that OVS is actually using the
>field
>value while trying to optimize the action later. If the action doesn't really
>change the value, the action will be omitted. In this case, if we do not have
>a
>match on the value, the action will not be executed for other packets as well.
>And that is a problem. It's also hard to debug such issues, because it depend
>on which packet hits OVS first.
>
>E.g. if the packet with dl_dst=00:11:22:33:44:55 hits OVS, OVS will install the
>following datapath flow:
>
> in_port(enp8s0f0),eth(dst=00:11:22:33:44:55), actions:enp8s0f0_0
>
>For a packet with dl_dst=00:11:22:33:44:77, it will be:
>
> in_port(enp8s0f0),eth(dst=00:11:22:33:44:77),
>actions:set(eth(dst=00:11:22:33:44:55)),enp8s0f0_0
>
>If we remove the match, the first flow will turn into:
>
> in_port(enp8s0f0), actions:enp8s0f0_0
>
>swallowing all the traffic without modifying the fields, if it happens to be
>installed first.
>
>There might be a room for optimization, e.g. by making sure the flow that
>doesn't have an action has a match, but the one that has an action, doesn't
>have a match. In this case we will end up with overlapping flows in the
>detapath. These are fine as long as the wider one has all the actions.
>However, this would need a very careful consideration, as it's hard to tell
>right
>away if the set of flows will be correct in the end or not.
>
>Let's see what will happen with a basic flow that changes 2
>fields:
>
> ip, actions:set_field:A->eth_src,set_field:B->eth_dst
>
> Packet eth(A,C) results in datapath flow:
> 1. eth(src=A), actions:set(eth(dst=B))
>
> Packet eth(C,B) results in datapath flow:
> 2. eth(dst=B), actions:set(eth(src=A))
>
> Packet eth(A,B) results in datapath flow:
> 3. eth(src=A,dst=B), actions:<none>
>
> Packet eth(C,D) results in datapath flow:
> 4. <none>, actions:set(eth(src=A,dst=B))
>
>After all the flows installed:
>Packet:
> eth(A,C) -> matches #1 -> set(eth(dst=B)) -> eth(A,B)
> matches #4 -> set(eth(src=A,dst=B)) -> eth(A,B)
>
> eth(A,B) -> matches all 4, ether way result will be eth(A,B)
>
> eth(C,B) -> matches #2 -> set(eth(src=A)) -> eth(A,B)
> matches #4 -> set(eth(src=A,dst=B)) -> eth(A,B)
>
> eth(C,D) -> matches #4 -> set(eth(src=A,dst=B)) -> eth(A,B)
>
>So, this might work, assuming datapath correctly handles overlapping flows,
>which it should. But it also becomes a bit more complex to process and adds a
>potential unnecessary overwrite of the fields that don't need it. The action
>execution performance is likely not a problem here. I'm not sure if having too
>many overlapping flows may cause performance issues for the datapath's
>packet classifier though. As seen above, we can have a lot of them with
>increasing number of fields.
>
>The action commit code also changed quite a bit since the logic was
>introduced, so there might be problems with this approach that I'm not aware
>of.
>
>Best regards, Ilya Maximets.
Hi Ilya,
Thanks for your input.
The optimization to avoid rewrite of the same value was done at [1]. Before
that #3 dp-flow created in your example was like this:
eth(src=A,dst=B), actions:set(eth(src=A,dst=B))
The question is what if we remove the match, but keep the actions, even if the
value is the same.
It means only #4 rule will be created.
The downside is that we will do the rewrite even if not needed (same values),
but on the other hand we decrease the number of dp-flows, consolidating them to
match more flows.
Can you point any functional issues with this approach?
Thanks,
Eli
[1] dbf4a92800d0 ("odp-util: Do not rewrite fields with the same values as
matched")
_______________________________________________
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
