Bug 2099689

Summary: [RFE] Investigate possible optimization of metadata restoration actions
Product: Red Hat Enterprise Linux Fast Datapath Reporter: Ilya Maximets <i.maximets>
Component: OVNAssignee: Numan Siddique <nusiddiq>
Status: NEW --- QA Contact: Jianlin Shi <jishi>
Severity: medium Docs Contact:
Priority: medium    
Version: FDP 22.ECC: ctrautma, jiji, lariel, mleitner, mmichels, nusiddiq
Target Milestone: ---Keywords: FutureFeature
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 2172625    

Description Ilya Maximets 2022-06-21 13:32:32 UTC
OVN pipeline consists of building blocks, a.k.a. OpenFlow tables.
Flows in some tables are written in a following way:

  match,actions=<save metadata>,<change metadata>,do something,<restore metadata>

This is done this way to be sure that if the flow from some other
table resubmits the packet to this one, after the resubmit the
metadata will stay the same as it was before.

However, if the resubmit to such a table ends up being the last
action, the actual <restore metadata> part is not really necessary
and can be omitted in the OpenFlow rule.

For example, table 64 contains several flows with the same action
list like this:

  actions=push:NXM_OF_IN_PORT[],load:0xffff->NXM_OF_IN_PORT[],resubmit(,65),pop:NXM_OF_IN_PORT[]

It's likely that resubmit to table 64 was the last action in
previous tables all the way up in the OpenFlow piepline.
In this case restoration of the input port value is not really
necessary.

Why it is important?

In case of table 64 actions, the resubmit to table 65 will likely
be the output to the br-ex bridge via the patch port.  Since the
resubmit(,65) is not the last action, OVS will emit a clone()
action while processing flows in the br-ex, if there are some
irreversible actions there.  So, the clone() will be emitted even
if the patch port processing is, in fact, the last meaningful
packet manipulation.  The clone() itself prevents the hardware
offloading, so it would be better if the input port metadata
restoration wasn't there in the first place allowing the HW offload
to happen.

This BZ is to investigate the possibility of not generating such
seemingly pointless metadata restoration actions on the OVN side.

The issue can be worked around on the OVS side, so if there is
no easy and safe way to fix it on OVN side, the OVS solution
should be preferred instead.