On 5/23/22 20:07, Han Zhou wrote: > On Tue, Mar 29, 2022 at 3:06 PM Han Zhou <[email protected]> wrote: >> >> >> >> On Tue, Mar 29, 2022 at 2:20 PM Numan Siddique <[email protected]> wrote: >>> >>> On Mon, Mar 28, 2022 at 1:40 AM Han Zhou <[email protected]> wrote: >>>> >>>> Some NICs support HW offloading for datapath flows, but masked access > to >>>> the 128-bit ct_label field may prevent a flow being offloaded due to > HW >>>> limitations. OVN's use of ct_label currently includes: >>>> - ct_label.blocked (1 bit) >>>> - ct_label.natted (1 bit) >>>> - ct_label.ecmp_reply_port (16 bits) >>>> - ct_label.ecmp_reply_eth (48 bits) >>>> - ct_label.label (32 bits) >>>> >>>> This patch moves the bits blocked, natted and ecmp_reply_port to use >>>> ct_mark (18 bits in total among the 32-bit ct_mark), and keep the rest >>>> of the fields in ct_label: >>>> - ct_mark.blocked (1 bit) >>>> - ct_mark.natted (1 bit) >>>> - ct_mark.ecmp_reply_port (16 bits) >>>> - ct_label.ecmp_reply_eth (48 bits) >>>> - ct_label.label (32 bits) >>>> >>>> This would allow HW offloading to work for most of the cases. >>>> >>>> For ct_label.ecmp_reply_eth, the flow matching it still uses masked >>>> access, but it doesn't matter because the flow is for new connections >>>> and requires ct_commit in its actions, so it wouldn't be offloaded >>>> anyway for those NICs. There is a flow for established connections > that >>>> would access the masked field in the actions, while in this patch it >>>> avoids masked access by using a register xxreg1 to temporarily read > the >>>> whole ct_label, and then use masked access to xxreg1 to read the > actual >>>> value. >>>> >>>> The only exception is for ct_label.label, there is a flow that matches >>>> the masked field for ACL logging of reply direction. This patch cannot >>>> avoid the masked access to ct_label in this case. This flow is enabled >>>> only for the feature "log-related". So offloading may still not work > for >>>> some NICs when an ACL is configured with a label and with > "log-related" >>>> enabled. >>>> >>>> There are no other flows relying on masked ct_label match, but it's >>>> worth noting that the LB hairpin related flows using ct_label.natted >>>> which were hardcoded directly in ovn-controller are still kept to > avoid >>>> traffic breaking during upgrading. It relies on the >>>> northd-internal-version to internally determine if it is currently >>>> upgrading from a version that requires the ct_label flows being >>>> kept, and automatically removes the flows when northd-internal-version >>>> is up-to-date. >>>> >>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1957786 >>> >>> >>> Thanks for the patch series. >>> >>> For the entire series: >>> >>> Acked-by: Numan Siddique <[email protected]> >>> >>> Numan >>> >> Thanks Numan! I applied the series to main. >> > > As discussed in the last OVN meetings, I backported this series to > branch-22.03. > It was also requested to backport to 21.12, but @Dumitru Ceara > <[email protected]> volunteered to work on that branch. Please note that > there will be conflict primarily because "log-related" was not supported in > 21.12.
I also just posted the 21.12 backport: https://patchwork.ozlabs.org/project/ovn/list/?series=301671&state=* https://mail.openvswitch.org/pipermail/ovs-dev/2022-May/394245.html Thanks, Dumitru > > Thanks, > Han > >> Han >> >>>> >>>> v1 -> v2: Fixed two system test cases. >>>> v2 -> v3: >>>> - Addressed Numan's comment regarding hairpin flow upgrading > problem by >>>> keeping ct_label flows together with ct_mark flows for hairpin, > and >>>> provided an option to disable ct_label after upgrading. >>>> - Moved a misplaced chunk from patch2 to patch5 to fix related > system tests. >>>> v3 -> v4: >>>> - Avoid the extra option to disable ct_label after upgrading for > hairpin >>>> flows. Instead, reuse the north-internal-ver to determine > automatically. >>>> For this to work, a new patch is added to handle SB_Global > changes in I-P >>>> engine of ovn-controller. >>>> >>>> Han Zhou (6): >>>> ovn-sb.xml: Fix ct_lb documentation. >>>> actions: Add action ct_lb_mark. >>>> actions: Add stack push and pop actions. >>>> ovn-northd: Improve the doc and tests for ecmp-symmetric-reply. >>>> ovn-controller: Handle SB_Global:options:northd_internal_version in >>>> I-P engine. >>>> Use ct_mark for masked access to make flows HW-offloading friendly. >>>> >>>> NEWS | 2 + >>>> controller/lflow.c | 33 ++- >>>> controller/lflow.h | 1 + >>>> controller/ovn-controller.c | 79 ++++++ >>>> include/ovn/actions.h | 11 +- >>>> include/ovn/logical-fields.h | 3 + >>>> lib/actions.c | 128 ++++++++- >>>> lib/logical-fields.c | 17 +- >>>> lib/ovn-util.c | 25 +- >>>> lib/ovn-util.h | 4 + >>>> northd/northd.c | 107 ++++--- >>>> northd/ovn-northd.8.xml | 59 ++-- >>>> ovn-sb.xml | 54 +++- >>>> tests/ovn-controller.at | 46 +++ >>>> tests/ovn-northd.at | 526 > ++++++++++++++++++----------------- >>>> tests/ovn.at | 208 +++++++------- >>>> tests/system-ovn.at | 178 ++++++------ >>>> utilities/ovn-trace.c | 72 ++++- >>>> 18 files changed, 1017 insertions(+), 536 deletions(-) >>>> >>>> -- >>>> 2.30.2 >>>> >>>> _______________________________________________ >>>> dev mailing list >>>> [email protected] >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
