On Mon, Mar 28, 2022 at 1:40 AM Han Zhou <hz...@ovn.org> 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 <num...@ovn.org>

Numan

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

Reply via email to