From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 7DCBDA0540;
Mon, 13 Jul 2020 12:16:22 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 5B1141D5D3;
Mon, 13 Jul 2020 12:16:21 +0200 (CEST)
Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com
[148.163.129.52]) by dpdk.org (Postfix) with ESMTP id D35011D5D2
for <dev@dpdk.org>; Mon, 13 Jul 2020 12:16:19 +0200 (CEST)
Received: from mx1-us1.ppe-hosted.com (unknown [10.7.65.61])
by dispatch1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTP id
4325560089; Mon, 13 Jul 2020 10:16:19 +0000 (UTC)
Received: from us4-mdac16-53.ut7.mdlocal (unknown [10.7.66.24])
by mx1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTP id 41A578009E;
Mon, 13 Jul 2020 10:16:19 +0000 (UTC)
X-Virus-Scanned: Proofpoint Essentials engine
Received: from mx1-us1.ppe-hosted.com (unknown [10.7.66.33])
by mx1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 8383A80055;
Mon, 13 Jul 2020 10:16:18 +0000 (UTC)
Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits))
(No client certificate requested)
by mx1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 97A79A80061;
Mon, 13 Jul 2020 10:16:16 +0000 (UTC)
Received: from [192.168.38.17] (10.17.10.39) by ukex01.SolarFlarecom.com
(10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 13 Jul
2020 11:16:07 +0100
To: Andrey Vesnovaty <andreyv@mellanox.com>, Neil Horman
<nhorman@tuxdriver.com>, Thomas Monjalon <thomas@monjalon.net>, Ferruh Yigit
<ferruh.yigit@intel.com>, Andrew Rybchenko <arybchenko@solarflare.com>, "Ori
Kam" <orika@mellanox.com>
CC: "Kinsella, Ray" <mdr@ashroe.eu>, <dev@dpdk.org>, <jerinjacobk@gmail.com>,
<stephen@networkplumber.org>, <bruce.richardson@intel.com>,
<viacheslavo@mellanox.com>, <andrey.vesnovaty@gmail.com>
References: <AM6PR05MB51767B63C80143C187B37A41DB670@AM6PR05MB5176.eurprd05.prod.outlook.com>
<20200708204015.24429-2-andreyv@mellanox.com>
<a26e3f91-1819-3ed5-3aaa-36af590f367c@ashroe.eu>
From: Andrew Rybchenko <arybchenko@solarflare.com>
Autocrypt: addr=arybchenko@solarflare.com; keydata=
mQINBF2681gBEACbdTxu8eLL3UX2oAelsnK9GkeaJeUYSOHPJQpV7RL/iaIskqTwBRnhjXt7
j9UEwGA+omnOmqQMpeQTb/F9Ma2dYE+Hw4/t/1KVjxr3ehFaASvwR4fWJfO4e2l/Rk4rG6Yi
5r6CWU2y8su2654Fr8KFc+cMGOAgKoZTZHZsRy5lHpMlemeF+VZkv8L5sYJWPnsypgqlCG3h
v6lbtfZs+QqYbFH6bqoZwBAl5irmxywGR7ZJr1GLUZZ1lfdazSY8r6Vz0/Ip/KVxGu2uxo81
QCsAj0ZsQtwji9Sds/prTiPrIjx8Fc/tfbnAuVuPcnPbczwCJACzQr4q26XATL3kVuZhSBWh
4XfO/EAUuEq5AemUG5DDTM87g7Lp4eT9gMZB6P+rJwWPNWTiV3L7Cn+fO+l9mTPnOqdzBgDe
OaulKiNSft1o0DY4bGzOmM2ad2cZt0jfnbMPMTE9zsr6+RFa+M8Ct20o6U1MUE4vP6veErMK
of4kZ8PdoMM+Sq1hxMPNtlcVBSP9xMmdSZPlfDYI5VWosOceEcz7XZdjBJKdwKuz70V7eac4
ITSxgNFCTbeJ03zL2MR5s0IvD9ghISAwZ6ieCjU5UATn5+63qpD0nVNLsAdb/UpfvQcKAmvj
0fKlxu/PMVkjBa7/4cfNogYOhWDKUO+1pMaFwvb6/XTo6uMpfQARAQABtCxBbmRyZXcgUnli
Y2hlbmtvIDxhcnliY2hlbmtvQHNvbGFyZmxhcmUuY29tPokCVAQTAQoAPhYhBP6NPgcKRj/Y
X0yXQahue0sAy4m+BQJduvNYAhsDBQkB4TOABQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJ
EKhue0sAy4m+t3gP/j1MNc63CEozZo1IZ2UpVPAVWTYbLdPjIRdFqhlwvZYIgGIgIBk3ezKL
K0/oc4ZeIwL6wQ5+V24ahuXvvcxLlKxfbJ6lo2iQGC7GLGhsDG9Y2k6sW13/sTJB/XuR2yov
k5FtIgJ+aHa1PDZnepnGGOt9ka9n/Jzrc9WKYapOIIyLRe9U26ikoVgyqsD37PVeq5tLWHHA
NGTUKupe9G6DFWidxx0KzyMoWDTbW2AWYcEmV2eQsgRT094AZwLFN5ErfefYzsGdO8TAUU9X
YTiQN2MvP1pBxY/r0/5UfwV4UKBcR0S3ZvzyvrPoYER2Kxdf/qurx0Mn7StiCQ/JlNZb/GWQ
TQ7huduuZHNQKWm7ufbqvKSfbPYvfl3akj7Wl8/zXhYdLqb5mmK45HXrgYGEqPN53OnK2Ngx
IgYKEWr05KNv09097jLT5ONgYvszflqlLIzC4dV245g7ucuf9fYmsvmM1p/gFnOJBJL18YE5
P1fuGYNfLP+qp4WMiDqXlzaJfB4JcinyU49BXUj3Utd6f6sNBsO8YWcLbKBV9WmA324S3+wj
f4NPRp3A5E+6OmTVMLWire2ZvnYp3YvifUj1r8lhoZ2B2vKuWwiTlHOKYBEjnOQJQnqYZEF0
JQQ1xzVDBQKE01BPlA3vy6BGWe6I4psBVqMOB9lAev/H+xa4u6Z3uQINBF269JsBEAC2KB3W
8JES/fh74avN7LOSdK4QA7gFIUQ4egVL81KnxquLzzilABuOhmZf3Rq6rMHSM8xmUAWa7Dkt
YtzXStjEBI/uF0mAR3mMz1RcL2Wp+WD/15HjVpA7hPjXSEsWY0K2ymPerK4yrLcfFTHdMonY
JfuACCC9NtOZxrWHOJoUS+RT7AWk80q/6D2iwQ47/2dBTznVG+gSeHSes9l91TB09w6f9JX/
sT+Ud0NQfm7HJ7t2pmGI9O6Po/NLZsDogmnIpJp/WwYOZN9JK7u2FyX2UyRzR8jK42aJkRsh
DXs16Cc2/eYGakjrdO3x9a+RoxN7EuFtYhGR1PzMXdUiB5i+FyddYXkYUyO43QE/3VPA5l1v
TUOagzZq6aONsdNonGJkV3TIG3JmUNtM+D/+r6QKzmgoJ8w576JxEZI09I/ZFN+g7BnUmlMx
6Z3IUOXVX/SWfGFga0YajwajHz03IBhChEbYbbqndVhmshu2GFURxrfUPYWdDXEqkh+08a5U
Didia9jm2Opv4oE1e1TXAePyYJl/Zyps4Cv00GObAxibvMBQCUZQ+IBnNldRBOwXXRQV2xpx
P+9iO1VYA/QXn0KqRK+SH1JGRXbJYi42YFaW1gE0EU0fiR2Wb9pK+doNEjjOhlzUGuvOEAUS
+4m0m3dlfEvpCV9GMr7ERRpZzh9QkQARAQABiQI8BBgBCgAmFiEE/o0+BwpGP9hfTJdBqG57
SwDLib4FAl269JsCGwwFCQlmAYAACgkQqG57SwDLib7x6g//e+eCtNnJz7qFGbjWRJYNLCe5
gQwkhdyEGk4omr3VmjGj3z9kNFy/muh4pmHUngSAnnpwZggx14N4hhKf9y8G4Dwvsqa6b1zB
Jq/c4t/SBDtGW4M/E331N04PaQZpcrbTfp1KqHNknk2N7yOk4CcoLVuIZmA5tPguASV8aAfz
ZwhWAwn6vUEw9552eXEAnGFGDTCbyryNwzB5jtVQOEEDjTxcCkpcXMB45Tb1QUslRTu/sBAe
HhPCQSUcJHR+KOq+P6yKICGAr291PZd6Qc7C3UyE+A3pY/UfdEVWj0STBWx1qvYLaHLrI4O9
KXDgh7luLjZZafcueCaPYmNo4V2lmNb3+7S4TvqhoZS+wN+9ldRQ4gH3wmRZybN6Y/ZCqxol
RaZpE3AqdWsGvIgAkD0FpmtZNii9s2pnrhw0K6S4t4tYgXGTossxNSJUltfFQZdXM1xkZhtv
dBZuUEectbZWuviGvQXahOMuH2pM64mx2hpdZzPcI2beeJNHkAsGT2KcaMETgvtHUBFRlLVB
YxsUYz3UZmi2JSua4tbcGd6iWVN90eb8CxszYtivfpz6o2nPSjNwg0NaVGSHXjAK0tdByZ9t
SkwjC3tEPljVycRSDpbauogOiAkvjENfaPd/H26V5hY822kaclaKDAW6ZG9UKiMijcAgb9u5
CJoOyqE8aGS5Ag0EXbr1RwEQAMXZHbafqmZiu6Kudp+Filgdkj2/XJva5Elv3fLfpXvhVt0Y
if5Rzds3RpffoLQZk9nPwK8TbZFqNXPu7HSgg9AY7UdCM94WRFTkUCGKzbgiqGdXZ7Vyc8cy
teGW+BcdfQycDvjfy50T3fO4kJNVp2LDNdknPaZVe8HJ80Od63+9ksB6Ni+EijMkh6Uk3ulB
CSLnT4iFV57KgU2IsxOQVLnm+0bcsWMcCnGfphkY0yKP+aJ6MfmZkEeaDa7kf24N14ktg50m
vOGDitcxA/+XXQXOsOIDJx1VeidxYsQ2FfsKu1G8+G6ejuaLf4rV5MI/+B/tfLbbOdikM5PF
pxZVgTir9q13qHumMxdme7w5c7hybW412yWAe9TsrlXktFmFjRSFzAAxQhQSQxArS6db4oBk
yeYJ59mW52i4occkimPWSm/raSgdSM+0P6zdWUlxxj+r1qiLgCYvruzLNtp5Nts5tR/HRQjE
/ohQYaWDSVJEsc/4eGmgwzHzmvHtXeKkasn01381A1Lv3xwtpnfwERMAhxBZ8EGKEkc5gNdk
vIPhknnGgPXqKmE1aWu8LcHiY+RHAF8gYPCDMuwyzBYnbiosKcicuIUp0Fj8XIaPao6F+WTi
In4UOrqrYhsaCUvhVjsTBbNphGih9xbFJ8E+lkTLL8P3umtTcMPnpsB4xqcDABEBAAGJBHIE
GAEKACYWIQT+jT4HCkY/2F9Ml0GobntLAMuJvgUCXbr1RwIbAgUJCWYBgAJACRCobntLAMuJ
vsF0IAQZAQoAHRYhBNTYjdjWgdaEN5MrAN+9UR5r/4d3BQJduvVHAAoJEN+9UR5r/4d3EiQP
/3lyby6v49HTU94Q2Fn2Xat6uifR7kWE5SO/1pUwYzx6v+z5K2jqPgqUYmuNoejcGl0CTNhg
LbsxzUmAuf1OTAdE+ZYvOAjjKQhY4haxHc4enby/ltnHfWJYWJZ9UN5SsIQLvITvYu6rqthO
CYjpXJhwkj3ODmC9H1TrvjrBGc6i7CTnR8RCjMEwCs2LI2frHa4R6imViEr9ScMfUnzdABMQ
B0T5MOg8NX92/FRjTldU2KovG0ML9mSveSvVHAoEBLy4UIs5nEDdNiO1opJgKb5CXvWQugub
7AR52phNdKVdEB0S4tigJT4NalyTaPiUhFEm+CzZpMQDJ5E+/OowaPRfN4HeJX+c8sB+vUAZ
mkAaG75N+IEk5JKFK9Z+bBYgPgaBDFZYdWDB/TMH0ANt+KI5uYg0i12TB4M8pwKG1DEPUmWc
F2YpvB3jnbwzsOpSFiJOOlSs6nOB0Sb5GRtPOO3h6XGj+6mzQd6tcL63c9TrrUkjq7LDkxCz
SJ2hTYRC8WNX8Uw9skWo5728JNrXdazEYCenUWmYiKLNKLslXCFodUCRDh/sUiyqRwS7PHEA
LYC/UIWLMomI0Yvju3KA5v3RQVXhL+Gx2CzSj3GDz9xxGhJB2LfRfjzPbTR/Z27UpjCkd8z0
Ro3Ypmi1FLQwnRgoOKDbetTAIhugEShaLTITzJAP/iRDJCQsrZah5tE8oIl81qKEmBJEGcdt
HYikbpQe7ydcXhqTj7+IECa3O7azI5OhCxUH2jNyonJ/phUslHH2G1TTBZK8y4Hrx5RpuRNS
esn3P9uKu9DHqBAL7DMsCPwb2p1VNnapD72DBmRhzS/e6zS2R4+r9yNv03Hv7VCxKkmtE63H
qpS//qpjfrtsIcHAjnKDaDtL1LYCtHoweI+DOpKKULSAYp/JE6F8LNibPQ0/P3S5ZIJNC4QZ
uESjFOalJwFIqGQdkQB7ltRNJENLrHc+2jKGOuyFHm/Sbvp5EMGdaeQ0+u8CY0P+y6oXenwx
7WrJz/GvbNoFhJoJ6RzxCMQrFgxrssVZ7w5HcUj94lbnJ6osdYE/WpSd50B6jet6LKh5revg
u9XI9CoqsPQ1V4wKYYdllPuogCye7KNYNKuiiuSNpaF4gHq1ZWGArwZtWHjgc2v3LegOpRQF
SwOskMKmWsUyHIRMG1p8RpkBQTqY2rGSeUqPSvaqjT0nq+SUEM6qxEXD/2Wqri/X6bamuPDb
S0PkBvFD2+0zr5Bc2YkMGPBYPNGZiTp3UjmZlLfn3TiBKIC92jherY563CULjSsiBEJCOSvv
4VPLn5aAcfbCXJnE3IGCp/hPl50iQqu7BPOYBbWXeb9ptDjGCAThNxSz0WAXkmcjAFE8gdE6 Znk9
Message-ID: <9b45bb29-0757-d1ab-b95e-b4716947c6ce@solarflare.com>
Date: Mon, 13 Jul 2020 13:16:02 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
Thunderbird/68.9.0
MIME-Version: 1.0
In-Reply-To: <a26e3f91-1819-3ed5-3aaa-36af590f367c@ashroe.eu>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-Originating-IP: [10.17.10.39]
X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To
ukex01.SolarFlarecom.com (10.17.10.4)
X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.6.1012-25538.003
X-TM-AS-Result: No-21.187700-8.000000-10
X-TMASE-MatchedRID: IeZYkn8zfFp0nsJmoztmWPZvT2zYoYOwC/ExpXrHizxYjtHrjqa8D1/u
FVcjJsvfLuDcg1waLhfoszdb+c82BbU5q3I7howYjpyluct2Nr0Apu/OILCbuBd9OjZE/C4OlUc
xGfWk0G3RqP+FnPr152TLdwFqSualsaLhFLnhBw4qkSeDPauzr17OZ6hrwwnzSg8ufp5n3T5qEE
2MO1GebBCUQfhObnw9NyWntOhuJ46f80tdhYBdg4rkmrf0Igi/UzwbjSjBu8ps98Z8fG/6kT6ze
SLHcZq1MqqNAwlLCF3ExXUw9J4h+p9vFrcDXJOnalRqQPhHMT7xTZ6+JDIODbjOUXWmQ3OWJCpg
+RUZA9/xJxoTt7BPX81MZDclngtF3vqxXQgTXEUHRzaQbsazqCY6ALX8FNLO1y0aXF5eX+i0CmL
k+JEYOsEdglAiCXqgAra3yGxzcQ6dOdJ0aMSpfeLdprnA5EQREwOwAhdI3QNtw+n+iKWyyLXQPQ
H2e5fT06N+CrIpWzP/6GzjzPbZa3fQaGT4N/qzm2xpdM7TKZfr3E41VlKsfVezEE205dNv4Pc/c
e4FCb2vDUXvw65PTpUYmKy/IkulVIqGNM1Iazf6on3SPOQWYatzLEAeDOzCq2RzHFToRUjcru+P
SMhej+GgS4rOorYr0vwQHJ9bAGKwQEC6hpSor1K1YsOgGk9RSoCG4sefl8TJfDUN5trKvj5lm+V
DBI4Eqdy0seJH1kftgdqvl8Sn7LSw/pyY1ik4Pja3w1ExF8SCo8vMtMK/WTXSEULATMS5gdacrJ
RheXvPhf39UttKDux5dNvsT0LomGO7olRZK2OeAiCmPx4NwLTrdaH1ZWqC1B0Hk1Q1KyLgfCfWl
nNb/1cppCzPq+1UkGUtrowrXLg=
X-TM-AS-User-Approved-Sender: Yes
X-TM-AS-User-Blocked-Sender: No
X-TMASE-Result: 10--21.187700-8.000000
X-TMASE-Version: SMEX-12.5.0.1300-8.6.1012-25538.003
X-MDID: 1594635379-3Qq991lqz3TJ
Subject: Re: [dpdk-dev] [PATCH v2 1/6] ethdev: add flow shared action API
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
On 7/13/20 11:04 AM, Kinsella, Ray wrote:
>
>
> On 08/07/2020 21:40, Andrey Vesnovaty wrote:
>> From: Andrey Vesnovaty <andrey.vesnovaty@gmail.com>
>>
>> This commit introduces extension of DPDK flow action API enabling
>> sharing of single rte_flow_action in multiple flows. The API intended for
>> PMDs where multiple HW offloaded flows can reuse the same HW
>> essence/object representing flow action and modification of such an
>> essence/object effects all the rules using it.
>>
>> Motivation and example
>> ===
>> Adding or removing one or more queues to RSS used by multiple flow rules
>> imposes per rule toll for current DPDK flow API; the scenario requires
>> for each flow sharing cloned RSS action:
>> - call `rte_flow_destroy()`
>> - call `rte_flow_create()` with modified RSS action
>>
>> API for sharing action and its in-place update benefits:
>> - reduce the overhead of multiple RSS flow rules reconfiguration
It *allows* to reduce the overhead of multiple RSS flow rules
reconfiguration. I.e. it is not mandatory. PMD may do it in SW,
if HW does not support it. I see no problem to allow it.
Even if it is done in PMD, it is already an optimization since
applications have unified interface and internally it should
be cheaper to do it.
I'd consider to implement generic SW helper API for PMDs which
cannot have shared actions in HW, but would like to simulate it
in SW. It would allow to avoid the fallback in applications.
>> - optimize resource utilization by sharing action across of multiple
>> flows
>>
>> Change description
>> ===
>>
>> Shared action
>> ===
>> In order to represent flow action shared by multiple flows new action
>> type RTE_FLOW_ACTION_TYPE_SHARED is introduced (see `enum
>> rte_flow_action_type`).
>> Actually the introduced API decouples action from any specific flow and
>> enables sharing of single action by its handle across multiple flows.
>>
>> Shared action create/use/destroy
>> ===
>> Shared action may be reused by some or none flow rules at any given
>> moment, i.e. shared action reside outside of the context of any flow.
>> Shared action represent HW resources/objects used for action offloading
>> implementation.
>> API for shared action create (see `rte_flow_shared_action_create()`):
>> - should allocate HW resources and make related initializations required
>> for shared action implementation.
>> - make necessary preparations to maintain shared access to
>> the action resources, configuration and state.
>> API for shared action destroy (see `rte_flow_shared_action_destroy()`)
>> should release HW resources and make related cleanups required for shared
>> action implementation.
>>
>> In order to share some flow action reuse the handle of type
>> `struct rte_flow_shared_action` returned by
>> rte_flow_shared_action_create() as a `conf` field of
>> `struct rte_flow_action` (see "example" section).
>>
>> If some shared action not used by any flow rule all resources allocated
>> by the shared action can be released by rte_flow_shared_action_destroy()
>> (see "example" section). The shared action handle passed as argument to
>> destroy API should not be used any further i.e. result of the usage is
>> undefined.
May be it makes sense to implement housekeeping in ethdev
layer? I.e. guarantee consequency using reference counters etc.
Will applications benefit from it?
>>
>> Shared action re-configuration
>> ===
>> Shared action behavior defined by its configuration can be updated via
>> rte_flow_shared_action_update() (see "example" section). The shared
>> action update operation modifies HW related resources/objects allocated
>> on the action creation. The number of operations performed by the update
>> operation should not be dependent on number of flows sharing the related
>> action. On return of shared action update API action behavior should be
>> according to updated configuration for all flows sharing the action.
If shared action is simulated in SW, number of operations to do
reconfiguration will depend on a number of flow rules using it.
I think it is not a problem. So, *should not* used above is OK.
Consider:
"should not be dependent on" -> "should not depend on"
>>
>> Shared action query
>> ===
>> Provide separate API to query shared action sate (see
sate -> state
>> rte_flow_shared_action_update()). Taking a counter as an example: query
>> returns value aggregating all counter increments across all flow rules
>> sharing the counter.
>>
>> PMD support
>> ===
>> The support of introduced API is pure PMD specific design and
>> responsibility for each action type (see struct rte_flow_ops).
>>
>> testpmd
>> ===
>> In order to utilize introduced API testpmd cli may implement following
>> extension
>> create/update/destroy/query shared action accordingly
>>
>> flow shared_action create {port_id} [index] {action}
>> flow shared_action update {port_id} {index} {action}
>> flow shared_action destroy {port_id} {index}
>> flow shared_action query {port_id} {index}
>>
>> testpmd example
>> ===
>>
>> configure rss to queues 1 & 2
>>
>> testpmd> flow shared_action create 0 100 rss 1 2
>>
>> create flow rule utilizing shared action
>>
>> testpmd> flow create 0 ingress \
>> pattern eth dst is 0c:42:a1:15:fd:ac / ipv6 / tcp / end \
>> actions shared 100 end / end
>>
>> add 2 more queues
>>
>> testpmd> flow shared_action modify 0 100 rss 1 2 3 4
>>
>> example
>> ===
>>
>> struct rte_flow_action actions[2];
>> struct rte_flow_action action;
>> /* skipped: initialize action */
>> struct rte_flow_shared_action *handle = rte_flow_shared_action_create(
>> port_id, &action, &error);
It should be possible to have many actions in shared action.
I.e. similar to below, it should be an array terminated by
RTE_FLOW_ACTION_TYPE_END. It is unclear from the example
above.
>> actions[0].type = RTE_FLOW_ACTION_TYPE_SHARED;
>> actions[0].conf = handle;
>> actions[1].type = RTE_FLOW_ACTION_TYPE_END;
>> /* skipped: init attr0 & pattern0 args */
>> struct rte_flow *flow0 = rte_flow_create(port_id, &attr0, pattern0,
>> actions, error);
>> /* create more rules reusing shared action */
>> struct rte_flow *flow1 = rte_flow_create(port_id, &attr1, pattern1,
>> actions, error);
>> /* skipped: for flows 2 till N */
>> struct rte_flow *flowN = rte_flow_create(port_id, &attrN, patternN,
>> actions, error);
>> /* update shared action */
>> struct rte_flow_action updated_action;
>> /*
>> * skipped: initialize updated_action according to desired action
>> * configuration change
>> */
>> rte_flow_shared_action_update(port_id, handle, &updated_action, error);
>> /*
>> * from now on all flows 1 till N will act according to configuration of
>> * updated_action
>> */
>> /* skipped: destroy all flows 1 till N */
>> rte_flow_shared_action_destroy(port_id, handle, error);
>>
>> Signed-off-by: Andrey Vesnovaty <andreyv@mellanox.com>
>> ---
>> lib/librte_ethdev/rte_ethdev_version.map | 6 +
>> lib/librte_ethdev/rte_flow.c | 81 +++++++++++++
>> lib/librte_ethdev/rte_flow.h | 148 ++++++++++++++++++++++-
>> lib/librte_ethdev/rte_flow_driver.h | 22 ++++
>> 4 files changed, 256 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
>> index 7155056045..119d84976a 100644
>> --- a/lib/librte_ethdev/rte_ethdev_version.map
>> +++ b/lib/librte_ethdev/rte_ethdev_version.map
>> @@ -241,4 +241,10 @@ EXPERIMENTAL {
>> __rte_ethdev_trace_rx_burst;
>> __rte_ethdev_trace_tx_burst;
>> rte_flow_get_aged_flows;
>> +
>> + # added in 20.08
>> + rte_flow_shared_action_create;
>> + rte_flow_shared_action_destroy;
>> + rte_flow_shared_action_update;
>> + rte_flow_shared_action_query;
>> };
>> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
>> index 1685be5f73..0ac4d31a13 100644
>> --- a/lib/librte_ethdev/rte_flow.c
>> +++ b/lib/librte_ethdev/rte_flow.c
>> @@ -1250,3 +1250,84 @@ rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>> NULL, rte_strerror(ENOTSUP));
>> }
>> +
>> +struct rte_flow_shared_action *
>> +rte_flow_shared_action_create(uint16_t port_id,
>> + const struct rte_flow_action *action,
>> + struct rte_flow_error *error)
>> +{
>> + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>> + struct rte_flow_shared_action *shared_action;
>> + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
>> +
>> + if (unlikely(!ops))
>> + return NULL;
>> + if (likely(!!ops->shared_action_create)) {
>> + shared_action = ops->shared_action_create(dev, action, error);
>> + if (shared_action == NULL)
>> + flow_err(port_id, -rte_errno, error);
>> + return shared_action;
>> + }
>> + rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>> + NULL, rte_strerror(ENOSYS));
>> + return NULL;
>> +}
>> +
>> +int
>> +rte_flow_shared_action_destroy(uint16_t port_id,
>> + struct rte_flow_shared_action *action,
>> + struct rte_flow_error *error)
>> +{
>> + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>> + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
>> +
>> + if (unlikely(!ops))
>> + return -rte_errno;
>> + if (likely(!!ops->shared_action_destroy))
>> + return flow_err(port_id,
>> + ops->shared_action_destroy(dev, action, error),
>> + error);
>> + return rte_flow_error_set(error, ENOSYS,
>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>> + NULL, rte_strerror(ENOSYS));
>> +}
>> +
>> +int
>> +rte_flow_shared_action_update(uint16_t port_id,
>> + struct rte_flow_shared_action *action,
>> + const struct rte_flow_action *update,
>> + struct rte_flow_error *error)
>> +{
>> + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>> + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
>> +
>> + if (unlikely(!ops))
>> + return -rte_errno;
>> + if (likely(!!ops->shared_action_update))
>> + return flow_err(port_id, ops->shared_action_update(dev, action,
>> + update, error),
>> + error);
>> + return rte_flow_error_set(error, ENOSYS,
>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>> + NULL, rte_strerror(ENOSYS));
>> +}
>> +
>> +int
>> +rte_flow_shared_action_query(uint16_t port_id,
>> + const struct rte_flow_shared_action *action,
>> + void *data,
>> + struct rte_flow_error *error)
>> +{
>> + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>> + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
>> +
>> + if (unlikely(!ops))
>> + return -rte_errno;
>> + if (likely(!!ops->shared_action_query))
>> + return flow_err(port_id, ops->shared_action_query(dev, action,
>> + data, error),
>> + error);
>> + return rte_flow_error_set(error, ENOSYS,
>> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>> + NULL, rte_strerror(ENOSYS));
>> +}
>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>> index b0e4199192..257456b14a 100644
>> --- a/lib/librte_ethdev/rte_flow.h
>> +++ b/lib/librte_ethdev/rte_flow.h
>> @@ -1681,7 +1681,8 @@ enum rte_flow_action_type {
>> /**
>> * Enables counters for this flow rule.
>> *
>> - * These counters can be retrieved and reset through rte_flow_query(),
>> + * These counters can be retrieved and reset through rte_flow_query() or
>> + * rte_flow_shared_action_query() if the action provided via handle,
>> * see struct rte_flow_query_count.
>> *
>> * See struct rte_flow_action_count.
>> @@ -2099,6 +2100,14 @@ enum rte_flow_action_type {
>> * see enum RTE_ETH_EVENT_FLOW_AGED
>> */
>> RTE_FLOW_ACTION_TYPE_AGE,
>> +
>> + /**
>> + * Describes action shared a cross multiple flow rules.
Both options are possible, but I'd propose to stick to:
Describes -> Describe
Or even:
Use actions shared across multiple flow rules.
>> + *
>> + * Enables multiple rules reference the same action by handle (see
Enables -> Allow
>> + * struct rte_flow_shared_action).
>> + */
>> + RTE_FLOW_ACTION_TYPE_SHARED,
>> };
>>
>> /**
>> @@ -2660,6 +2669,20 @@ struct rte_flow_action_set_dscp {
>> uint8_t dscp;
>> };
>>
>> +
>> +/**
>> + * RTE_FLOW_ACTION_TYPE_SHARED
>> + *
>> + * Opaque type returned after successfully creating a shared action.
>> + *
>> + * This handle can be used to manage and query the related action:
>> + * - share it a cross multiple flow rules
>> + * - update action configuration
>> + * - query action data
>> + * - destroy action
>> + */
>> +struct rte_flow_shared_action;
>> +
>> /* Mbuf dynamic field offset for metadata. */
>> extern int32_t rte_flow_dynf_metadata_offs;
>>
>> @@ -3324,6 +3347,129 @@ int
>> rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
>> uint32_t nb_contexts, struct rte_flow_error *error);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Create shared action for reuse in multiple flow rules.
>> + *
>> + * @param[in] port_id
>> + * The port identifier of the Ethernet device.
>> + * @param[in] action
>> + * Action configuration for shared action creation.
>> + * @param[out] error
>> + * Perform verbose error reporting if not NULL. PMDs initialize this
>> + * structure in case of error only.
>> + * @return
>> + * A valid handle in case of success, NULL otherwise and rte_errno is set
>> + * to one of the error codes defined:
>> + * - (ENOSYS) if underlying device does not support this functionality.
>> + * - (EIO) if underlying device is removed.
>> + * - (EINVAL) if *action* invalid.
>> + * - (ENOTSUP) if *action* valid but unsupported.
>> + */
>> +__rte_experimental
>> +struct rte_flow_shared_action *
>> +rte_flow_shared_action_create(uint16_t port_id,
>> + const struct rte_flow_action *action,
>> + struct rte_flow_error *error);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Destroys the shared action by handle.
Destroys -> Destroy
>> + *
>> + * @param[in] port_id
>> + * The port identifier of the Ethernet device.
>> + * @param[in] action
>> + * Handle for the shared action to be destroyed.
>> + * @param[out] error
>> + * Perform verbose error reporting if not NULL. PMDs initialize this
>> + * structure in case of error only.
>> + * @return
>> + * - (0) if success.
>> + * - (-ENOSYS) if underlying device does not support this functionality.
>> + * - (-EIO) if underlying device is removed.
>> + * - (-ENOENT) if action pointed by *action* handle was not found.
>> + * - (-ETOOMANYREFS) if action pointed by *action* handle still used by one or
>> + * more rules
>> + * rte_errno is also set.
>> + */
>> +__rte_experimental
>> +int
>> +rte_flow_shared_action_destroy(uint16_t port_id,
>> + struct rte_flow_shared_action *action,
>> + struct rte_flow_error *error);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Updates inplace the shared action configuration pointed by *action* handle
Updates -> Update
inplace -> in-place
>> + * with the configuration provided as *update* argument.
>> + * The update of the shared action configuration effects all flow rules reusing
May be: reusing -> using
>> + * the action via handle.
The interesting question what to do, if some rule cannot have a
new action (i.e. new action is invalid for a rule).
May be it is better to highlight it.
>> + *
>> + * @param[in] port_id
>> + * The port identifier of the Ethernet device.
>> + * @param[in] action
>> + * Handle for the shared action to be updated.
>> + * @param[in] update
>> + * Action specification used to modify the action pointed by handle.
>> + * *update* should be of same type with the action pointed by the *action*
>> + * handle argument, otherwise considered as invalid.
Why? If it is a generic rule, it should be checked on a generic
ethdev layer, but I would not impose the limitation. If PMD
may change it, why generic interface specification should
restrict it.
>> + * @param[out] error
>> + * Perform verbose error reporting if not NULL. PMDs initialize this
>> + * structure in case of error only.
>> + * @return
>> + * - (0) if success.
>> + * - (-ENOSYS) if underlying device does not support this functionality.
>> + * - (-EIO) if underlying device is removed.
>> + * - (-EINVAL) if *update* invalid.
>> + * - (-ENOTSUP) if *update* valid but unsupported.
>> + * - (-ENOENT) if action pointed by *ctx* was not found.
>> + * rte_errno is also set.
>> + */
>> +__rte_experimental
>> +int
>> +rte_flow_shared_action_update(uint16_t port_id,
>> + struct rte_flow_shared_action *action,
>> + const struct rte_flow_action *update,
>> + struct rte_flow_error *error);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Query the shared action by handle.
>> + *
>> + * This function allows retrieving action-specific data such as counters.
"This function allows retrieving" -> Retrieve or Get or Query
>> + * Data is gathered by special action which may be present/referenced in
>> + * more than one flow rule definition.
>> + *
>> + * \see RTE_FLOW_ACTION_TYPE_COUNT
>> + *
>> + * @param port_id
>> + * Port identifier of Ethernet device.
>> + * @param[in] action
>> + * Handle for the shared action to query.
>> + * @param[in, out] data
>> + * Pointer to storage for the associated query data type.
>> + * @param[out] error
>> + * Perform verbose error reporting if not NULL. PMDs initialize this
>> + * structure in case of error only.
>> + *
>> + * @return
>> + * 0 on success, a negative errno value otherwise and rte_errno is set.
>> + */
>> +__rte_experimental
>> +int
>> +rte_flow_shared_action_query(uint16_t port_id,
>> + const struct rte_flow_shared_action *action,
>> + void *data,
>> + struct rte_flow_error *error);
>> +
>> #ifdef __cplusplus
>> }
>> #endif
>> diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
>> index 881cc469b7..a2cae1b53c 100644
>> --- a/lib/librte_ethdev/rte_flow_driver.h
>> +++ b/lib/librte_ethdev/rte_flow_driver.h
>> @@ -107,6 +107,28 @@ struct rte_flow_ops {
>> void **context,
>> uint32_t nb_contexts,
>> struct rte_flow_error *err);
>> + /** See rte_flow_shared_action_create() */
>> + struct rte_flow_shared_action *(*shared_action_create)
>> + (struct rte_eth_dev *dev,
>> + const struct rte_flow_action *action,
>> + struct rte_flow_error *error);
>> + /** See rte_flow_shared_action_destroy() */
>> + int (*shared_action_destroy)
>> + (struct rte_eth_dev *dev,
>> + struct rte_flow_shared_action *shared_action,
>> + struct rte_flow_error *error);
>> + /** See rte_flow_shared_action_update() */
>> + int (*shared_action_update)
>> + (struct rte_eth_dev *dev,
>> + struct rte_flow_shared_action *shared_action,
>> + const struct rte_flow_action *update,
>> + struct rte_flow_error *error);
>> + /** See rte_flow_shared_action_query() */
>> + int (*shared_action_query)
>> + (struct rte_eth_dev *dev,
>> + const struct rte_flow_shared_action *shared_action,
>> + void *data,
>> + struct rte_flow_error *error);
>> };
>>
>> /**
>>
>
> The modification of "struct rte_flow_ops", looks fine.
>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>