>-----Original Message-----
>From: David Marchand <david.march...@redhat.com>
>Sent: Wednesday, 11 January 2023 10:32
>To: Eli Britstein <el...@nvidia.com>; Ivan Malov <ivan.ma...@oktetlabs.ru>
>Cc: Ilya Maximets <i.maxim...@ovn.org>; d...@openvswitch.org; Stephen
>Hemminger <step...@networkplumber.org>; Ori Kam <or...@nvidia.com>;
>Maxime Coquelin <maxime.coque...@redhat.com>; Ian Stokes
><ian.sto...@intel.com>; Andrew Rybchenko
><andrew.rybche...@oktetlabs.ru>
>Subject: Re: [PATCH v3 0/3] Rework the usage of DPDK transfer flow offloads
>
>External email: Use caution opening links or attachments
>
>
>On Wed, Jan 11, 2023 at 9:16 AM Eli Britstein <el...@nvidia.com> wrote:
>>
>>
>>
>> >-----Original Message-----
>> >From: David Marchand <david.march...@redhat.com>
>> >Sent: Wednesday, 11 January 2023 9:59
>> >To: Ivan Malov <ivan.ma...@oktetlabs.ru>
>> >Cc: Ilya Maximets <i.maxim...@ovn.org>; Eli Britstein
>> ><el...@nvidia.com>; d...@openvswitch.org; Stephen Hemminger
>> ><step...@networkplumber.org>; Ori Kam <or...@nvidia.com>; Maxime
>> >Coquelin <maxime.coque...@redhat.com>; Ian Stokes
>> ><ian.sto...@intel.com>; Andrew Rybchenko
>> ><andrew.rybche...@oktetlabs.ru>
>> >Subject: Re: [PATCH v3 0/3] Rework the usage of DPDK transfer flow
>> >offloads
>> >
>> >External email: Use caution opening links or attachments
>> >
>> >
>> >Hi Ivan,
>> >
>> >On Fri, Jul 22, 2022 at 12:02 PM Ivan Malov <ivan.ma...@oktetlabs.ru>
>wrote:
>> >>
>> >> Hi Ilya, Eli,
>> >>
>> >> Many thanks for your feedback. Please see below.
>> >>
>> >> On Wed, 20 Jul 2022, Ilya Maximets wrote:
>> >>
>> >> > On 7/20/22 14:18, Ivan Malov wrote:
>> >> >> DPDK has got support for offloads involving assignment of
>> >> >> per-packet Rx metadata (flag, mark, tunnel ID and the likes).
>> >> >> However, delivery of such metadata from the NIC to the PMD might
>> >> >> need to be negotiated in advance. API [1] addresses that
>> >> >> problem. Make
>> >OvS invoke this API.
>> >> >>
>> >> >> Another problem is how flow rules with attribute "transfer"
>> >> >> refer to embedded switch ports by means of their representors.
>> >> >> Action PORT_ID has proved ambiguous: it refers to a DPDK ethdev,
>> >> >> but in fact steers packets to the entity represented by the
>> >> >> ethdev, in example,
>> >to a VF.
>> >> >> The problem is addressed by [2]. Use the solution in OvS accordingly.
>> >> >>
>> >> >> In addition, [2] and [3] address filtering traffic by input
>> >> >> ports of the embedded switch. In the suggested approach,
>> >> >> "transfer" rules are managed via the only ethdev with sufficient
>> >> >> privileges, whilst match criteria include an explicit item to indicate
>> >> >> the
>desired input port.
>> >> >> Revisit OvS support for "transfer" rules to follow the said approach.
>> >> >>
>> >> >> The following tests have been considered so far:
>> >> >> - build check with the current dpdk-next-net;
>> >> >> - running "make check" for every patch;
>> >> >> - tunnel offload demo with net/sfc PMD.
>> >> >>
>> >> >> [1]
>> >> >>
>>
>>https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fma%
>2
>>
>>F&data=05%7C01%7Celibr%40nvidia.com%7C655010753f9c45488f9e08daf3ae
>4c5
>>
>>2%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C63809022718993219
>7%7CU
>>
>>nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
>I6Ik1h
>>
>>aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=CZvqD1p6gHMqgVhyF
>uizcskk6Pwd
>> >GD3N4SLgr27DeZE%3D&reserved=0
>> >> >> ils.dpdk.org%2Farchives%2Fdev%2F2021-
>> >October%2F224291.html&data=05%
>> >> >>
>>
>>7C01%7Celibr%40nvidia.com%7C9f047907e3d54640a11f08daf3a9b52a%7C430
>8
>> >> >>
>>
>>3d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638090207473941055%7CUnk
>n
>> >own
>> >> >>
>>
>>%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
>W
>> >wi
>> >> >>
>>
>>LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=OwaqM%2BGfklo73Lb%2BJI
>p
>> >5vcH0DH
>> >> >> F%2FL6PZCbrvEgupwWc%3D&reserved=0 [2]
>> >> >>
>>
>>https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fma%
>2
>>
>>F&data=05%7C01%7Celibr%40nvidia.com%7C655010753f9c45488f9e08daf3ae
>4c5
>>
>>2%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C63809022718993219
>7%7CU
>>
>>nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
>I6Ik1h
>>
>>aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=CZvqD1p6gHMqgVhyF
>uizcskk6Pwd
>> >GD3N4SLgr27DeZE%3D&reserved=0
>> >> >> ils.dpdk.org%2Farchives%2Fdev%2F2021-
>> >October%2F224620.html&data=05%
>> >> >>
>>
>>7C01%7Celibr%40nvidia.com%7C9f047907e3d54640a11f08daf3a9b52a%7C430
>8
>> >> >>
>>
>>3d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638090207473941055%7CUnk
>n
>> >own
>> >> >>
>>
>>%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
>W
>> >wi
>> >> >>
>>
>>LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WF%2FsuI3pSCjMcHetlKbJz
>e5
>> >xFon8
>> >> >> cnj%2F4A4YCd9QwWg%3D&reserved=0 [3]
>> >> >>
>>
>>https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fma%
>2
>>
>>F&data=05%7C01%7Celibr%40nvidia.com%7C655010753f9c45488f9e08daf3ae
>4c5
>>
>>2%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C63809022718993219
>7%7CU
>>
>>nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
>I6Ik1h
>>
>>aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=CZvqD1p6gHMqgVhyF
>uizcskk6Pwd
>> >GD3N4SLgr27DeZE%3D&reserved=0
>> >> >> ils.dpdk.org%2Farchives%2Fdev%2F2021-
>> >October%2F225081.html&data=05%
>> >> >>
>>
>>7C01%7Celibr%40nvidia.com%7C9f047907e3d54640a11f08daf3a9b52a%7C430
>8
>> >> >>
>>
>>3d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638090207473941055%7CUnk
>n
>> >own
>> >> >>
>>
>>%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
>W
>> >wi
>> >> >>
>>
>>LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=AP07kpKyl3DPp01Rui7G7AIq
>> >%2B4SH
>> >> >> YrnlOAU30lNP%2BXg%3D&reserved=0
>> >> >> ---
>> >> >> v1 -> v2: amendments to care about proxy detach and port ID
>> >> >> logging
>> >> >> v2 -> v3: a minor adjustment in the cover letter's subject line
>> >> >>
>> >> >> Ivan Malov (3):
>> >> >> netdev-dpdk: negotiate delivery of per-packet Rx metadata
>> >> >> netdev-offload-dpdk: replace action PORT_ID with
>REPRESENTED_PORT
>> >> >> netdev-offload-dpdk: use flow transfer proxy mechanism
>> >> >>
>> >> >> lib/netdev-dpdk.c | 141
>+++++++++++++++++++++++++++++++++-
>> >----
>> >> >> lib/netdev-dpdk.h | 4 +-
>> >> >> lib/netdev-offload-dpdk.c | 91 +++++++++++++++++++++---
>> >> >> 3 files changed, 209 insertions(+), 27 deletions(-)
>> >> >>
>> >> >
>> >> > Hi. Since this patch set contains use of DPDK's experimental
>> >> > API, it needs to be based on dpdk-latest branch and have 'dpdk-latest'
>> >> > in the subject prefix. OVS doesn't normally use experimental API.
>> >> > The tunnel offload support was an exception, because it required
>> >> > significant re-work of the generic code that wasn't feasible to
>> >> > carry in the dpdk-latest branch.
>> >>
>> >> Very useful information indeed.
>> >>
>> >> >
>> >> > CC: Ian, dpdk-latest branch might need some rebase.
>> >> >
>> >> > Also, please, don't send new versions in reply to previous one.
>> >> > It's only required for dpdk-dev list for some reason, but
>> >> > prohibited or frown upon in every other list that I know of.
>> >> > It's very inconvenient to work this way.
>> >>
>> >> Thanks for the hint. I should've known.
>> >>
>> >> >
>> >> > For the patch set itself, a brief look over current state of DPDK
>> >> > shows that not all drivers that support PORT_ID support the
>> >> > REPRESENTED_PORT action as well. I see at least dpaa2 and cnxk
>> >> > that doesn't. So, this patch set essentially breaks offloading
>> >> > for these NICs, IIUC. Yet another rte_flow inconsistency.
>> >> > Any plans for addressing that?
>> >> >
>> >> > Best regards, Ilya Maximets.
>> >> >
>> >>
>> >> This observation of yours is correct. The REPRESENTED_PORT commit is:
>> >>
>> >> commit 88caad251c8de3a84e353b0b2a27014bc303df87
>> >> Author: Ivan Malov <ivan.ma...@oktetlabs.ru>
>> >> Date: Wed Oct 13 20:34:40 2021 +0300
>> >>
>> >> ethdev: add represented port action to flow API
>> >>
>> >> While the PORT_ID commits in dpaa2 and cnxk, respectively, are:
>> >>
>> >> commit 028d1dfd183043db03eb3d726579b18219c02624
>> >> Author: Jun Yang <jun.y...@nxp.com>
>> >> Date: Wed Oct 6 22:31:23 2021 +0530
>> >>
>> >> net/dpaa2: support Tx flow redirection action
>> >>
>> >> commit 00ea15e7a39617903ad39a9b89280a028ec7783a
>> >> Author: Satheesh Paul <psathe...@marvell.com>
>> >> Date: Thu Oct 21 10:41:15 2021 +0530
>> >>
>> >> net/cnxk: support port ID flow action
>> >>
>> >> Indeed, I might've lost at least one driver from my view when
>> >> sending the REPRESENTED_PORT series updating the capable PMDs.
>> >> And in the case of cnxk, perhaps maintainers were busy enough and
>> >> thus did not spot the PORT_ID deprecation being under way.
>> >>
>> >> So yes, the problem exists, and many thanks for noticing that.
>> >>
>> >> Looking into it, I see that we've had a deprecation notice in DPDK
>> >> for almost a year already:
>> >>
>> >> * ethdev: Items and actions ``PF``, ``VF``, ``PHY_PORT``, ``PORT_ID`` are
>> >> deprecated as hard-to-use / ambiguous and will be removed in
>> >> DPDK
>> >22.11.
>> >>
>> >> * ethdev: The use of attributes ``ingress`` / ``egress`` in "transfer"
>> >> flows
>> >> is deprecated as ambiguous with respect to the embedded switch.
>> >> The use
>> >of
>> >> these attributes will become invalid starting from DPDK 22.11.
>> >>
>> >> So, based on that, I devise the following plan:
>> >>
>> >> 1) I will send a patch to make the new items / actions non-experimental.
>> >> 2) I will send two patches to add REPRESENTED_PORT to dpaa2 and cnxk.
>> >> 3) I will dispose of the deprecated items / actions and the notice.
>> >> Doing so will also remove "experimental" from transfer proxy
>> >> as well as from Rx metadata configuration API.
>> >> 4) When (1-3) have been done, I will re-send the OvS patch series, but
>> >> this time without ALLOW_EXPERIMENTAL guards. The idea is that OvS,
>> >> in its next release, should pick DPDK 22.11 stable release, that
>> >> is, the use of Rx metadata, flow proxy and new items / actions
>> >> should not be experimental by that time any longer.
>> >>
>> >> However, if the patch sets (like this one) can be applied earlier
>> >> in OvS, then it might be handy to preserve ALLOW_EXPERIMENTAL tags.
>> >> Then they can be removed later on, when OvS is switched to DPDK 22.11.
>> >> But don't have a strong opinion on that.
>> >
>> >- You can rebsae this series now, since experimental tag checks can
>> >be removed.
>> >
>> >- I was doing a quick check on dpdk offload in OVS.
>> >Unless I missed something, full offloading is broken because of the
>> >use of mixed ingress and transfer attributes.
>> >Unfortunately, the issue on ingress attribute is still present with
>> >this series applied.
>> >Do you have a fix for this too?
>> @@ -2242,7 +2242,7 @@
>> struct nlattr *nl_actions,
>> size_t actions_len) {
>> - const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
>> + const struct rte_flow_attr flow_attr = { .transfer = 1 };
>> struct flow_actions actions = {
>> .actions = NULL,
>> .cnt = 0,
>
>Well, that's what I did as a hack, and it seemed enough.
>Should this simple fix go as is, do we need Ivan series as well?
Ivan's series is orthogonal to this.
This line fix should be part of the adaptations to DPDK 22.11.
>
>
>--
>David Marchand
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
