DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Gregory Etelson <getelson@nvidia.com>, orika@nvidia.com
Cc: ajit.khaparde@broadcom.com, andrew.rybchenko@oktetlabs.ru,
	dev@dpdk.org, jerinj@marvell.com, jerinjacobk@gmail.com,
	olivier.matz@6wind.com, thomas@monjalon.net,
	viacheslavo@nvidia.com, matan@nvidia.com, rasland@nvidia.com
Subject: Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add packet integrity checks
Date: Wed, 14 Apr 2021 14:31:40 +0100
Message-ID: <881ca2ff-2e2a-53d0-2668-e745694ff448@intel.com> (raw)
In-Reply-To: <dacce295-298f-4e07-bd83-d5691c84a72a@intel.com>

On 4/14/2021 2:27 PM, Ferruh Yigit wrote:
> On 4/14/2021 1:56 PM, Gregory Etelson wrote:
>> From: Ori Kam <orika@nvidia.com>
>>
>> Currently, DPDK application can offload the checksum check,
>> and report it in the mbuf.
>>
>> However, as more and more applications are offloading some or all
>> logic and action to the HW, there is a need to check the packet
>> integrity so the right decision can be taken.
>>
>> The application logic can be positive meaning if the packet is
>> valid jump / do  actions, or negative if packet is not valid
>> jump to SW / do actions (like drop)  a, and add default flow
>> (match all in low priority) that will direct the miss packet
>> to the miss path.
>>
>> Since currently rte_flow works in positive way the assumption is
>> that the positive way will be the common way in this case also.
>>
>> When thinking what is the best API to implement such feature,
>> we need to considure the following (in no specific order):
>> 1. API breakage.
>> 2. Simplicity.
>> 3. Performance.
>> 4. HW capabilities.
>> 5. rte_flow limitation.
>> 6. Flexibility.
>>
>> First option: Add integrity flags to each of the items.
>> For example add checksum_ok to ipv4 item.
>>
>> Pros:
>> 1. No new rte_flow item.
>> 2. Simple in the way that on each item the app can see
>> what checks are available.
>>
>> Cons:
>> 1. API breakage.
>> 2. increase number of flows, since app can't add global rule and
>>     must have dedicated flow for each of the flow combinations, for example
>>     matching on icmp traffic or UDP/TCP  traffic with IPv4 / IPv6 will
>>     result in 5 flows.
>>
>> Second option: dedicated item
>>
>> Pros:
>> 1. No API breakage, and there will be no for some time due to having
>>     extra space. (by using bits)
>> 2. Just one flow to support the icmp or UDP/TCP traffic with IPv4 /
>>     IPv6.
>> 3. Simplicity application can just look at one place to see all possible
>>     checks.
>> 4. Allow future support for more tests.
>>
>> Cons:
>> 1. New item, that holds number of fields from different items.
>>
>> For starter the following bits are suggested:
>> 1. packet_ok - means that all HW checks depending on packet layer have
>>     passed. This may mean that in some HW such flow should be splited to
>>     number of flows or fail.
>> 2. l2_ok - all check for layer 2 have passed.
>> 3. l3_ok - all check for layer 3 have passed. If packet doesn't have
>>     l3 layer this check should fail.
>> 4. l4_ok - all check for layer 4 have passed. If packet doesn't
>>     have l4 layer this check should fail.
>> 5. l2_crc_ok - the layer 2 crc is O.K.
>> 6. ipv4_csum_ok - IPv4 checksum is O.K. it is possible that the
>>     IPv4 checksum will be O.K. but the l3_ok will be 0. it is not
>>     possible that checksum will be 0 and the l3_ok will be 1.
>> 7. l4_csum_ok - layer 4 checksum is O.K.
>> 8. l3_len_OK - check that the reported layer 3 len is smaller than the
>>     frame len.
>>
>> Example of usage:
>> 1. check packets from all possible layers for integrity.
>>     flow create integrity spec packet_ok = 1 mask packet_ok = 1 .....
>>
>> 2. Check only packet with layer 4 (UDP / TCP)
>>     flow create integrity spec l3_ok = 1, l4_ok = 1 mask l3_ok = 1 l4_ok = 1
>>
>> Signed-off-by: Ori Kam <orika@nvidia.com>
>> ---
>>   doc/guides/prog_guide/rte_flow.rst     | 19 ++++++++++
>>   doc/guides/rel_notes/release_21_05.rst |  5 +++
>>   lib/librte_ethdev/rte_flow.h           | 48 ++++++++++++++++++++++++++
>>   3 files changed, 72 insertions(+)
>>
>> diff --git a/doc/guides/prog_guide/rte_flow.rst 
>> b/doc/guides/prog_guide/rte_flow.rst
>> index e1b93ecedf..4b8723b84c 100644
>> --- a/doc/guides/prog_guide/rte_flow.rst
>> +++ b/doc/guides/prog_guide/rte_flow.rst
>> @@ -1398,6 +1398,25 @@ Matches a eCPRI header.
>>   - ``hdr``: eCPRI header definition (``rte_ecpri.h``).
>>   - Default ``mask`` matches nothing, for all eCPRI messages.
>> +Item: ``PACKET_INTEGRITY_CHECKS``
>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +Matches packet integrity.
>> +Some devices require pre-enabling for this item before using it.
>> +
> 
> "pre-enabling" may not be clear enough, what about updating it slightly:
> 
> "Some devices require enabling integration checks in HW before using this flow 
> item."
> 

Indeed even with above it is not clear who should do the enabling, PMD or 
application, let me try again:

"For some devices application needs to enable integration checks in HW before 
using this flow item."


> For the record, the intention here is to highlight that if the requested 
> integration check is not enabled in HW, creating flow rule will fail.
> Application may need to enable the integration check in HW first.


  reply	other threads:[~2021-04-14 13:31 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 18:04 [dpdk-dev] [PATCH] " Ori Kam
2021-04-06  7:39 ` Jerin Jacob
2021-04-07 10:32   ` Ori Kam
2021-04-07 11:01     ` Jerin Jacob
2021-04-07 22:15       ` Ori Kam
2021-04-08  7:44         ` Jerin Jacob
2021-04-11  4:12           ` Ajit Khaparde
2021-04-11  6:03             ` Ori Kam
2021-04-13 15:16     ` [dpdk-dev] [PATCH v3 0/2] " Gregory Etelson
2021-04-13 15:16       ` [dpdk-dev] [PATCH v3 1/2] ethdev: " Gregory Etelson
2021-04-13 15:16       ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-13 17:15         ` Ferruh Yigit
2021-04-14 12:56     ` [dpdk-dev] [PATCH v4 0/2] add packet integrity checks Gregory Etelson
2021-04-14 12:56       ` [dpdk-dev] [PATCH v4 1/2] ethdev: " Gregory Etelson
2021-04-14 13:27         ` Ferruh Yigit
2021-04-14 13:31           ` Ferruh Yigit [this message]
2021-04-14 12:57       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-14 16:09     ` [dpdk-dev] [PATCH v5 0/2] add packet integrity checks Gregory Etelson
2021-04-14 16:09       ` [dpdk-dev] [PATCH v5 1/2] ethdev: " Gregory Etelson
2021-04-14 17:24         ` Ajit Khaparde
2021-04-15 15:10           ` Ori Kam
2021-04-15 15:25             ` Ajit Khaparde
2021-04-15 16:46         ` Thomas Monjalon
2021-04-16  7:43           ` Ori Kam
2021-04-18  8:15             ` Gregory Etelson
2021-04-18 18:00               ` Thomas Monjalon
2021-04-14 16:09       ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-14 16:26       ` [dpdk-dev] [PATCH v5 0/2] add packet integrity checks Ferruh Yigit
2021-04-18 15:51     ` [dpdk-dev] [PATCH v6 " Gregory Etelson
2021-04-18 15:51       ` [dpdk-dev] [PATCH v6 1/2] ethdev: " Gregory Etelson
2021-04-18 18:11         ` Thomas Monjalon
2021-04-18 19:24           ` Gregory Etelson
2021-04-18 21:30             ` Thomas Monjalon
2021-04-18 15:51       ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-19  8:29     ` [dpdk-dev] [PATCH v7 0/2] add packet integrity checks Gregory Etelson
2021-04-19  8:29       ` [dpdk-dev] [PATCH v7 1/2] ethdev: " Gregory Etelson
2021-04-19  8:47         ` Thomas Monjalon
2021-04-19  8:29       ` [dpdk-dev] [PATCH v7 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-19 11:20       ` [dpdk-dev] [PATCH v7 0/2] add packet integrity checks Ferruh Yigit
2021-04-19 12:08         ` Gregory Etelson
2021-04-19 12:44     ` [dpdk-dev] [PATCH v8 " Gregory Etelson
2021-04-19 12:44       ` [dpdk-dev] [PATCH v8 1/2] ethdev: " Gregory Etelson
2021-04-19 14:09         ` Ajit Khaparde
2021-04-19 16:34           ` Thomas Monjalon
2021-04-19 17:06             ` Ferruh Yigit
2021-04-19 12:44       ` [dpdk-dev] [PATCH v8 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-19 14:09         ` Ajit Khaparde
2021-04-08  8:04 ` [dpdk-dev] [PATCH] ethdev: add packet integrity checks Andrew Rybchenko
2021-04-08 11:39   ` Ori Kam
2021-04-09  8:08     ` Andrew Rybchenko
2021-04-11  6:42       ` Ori Kam
2021-04-11 17:30         ` Ori Kam
2021-04-11 17:34 ` [dpdk-dev] [PATCH v2 0/2] " Gregory Etelson
2021-04-11 17:34   ` [dpdk-dev] [PATCH v2 1/2] ethdev: " Gregory Etelson
2021-04-12 17:36     ` Ferruh Yigit
2021-04-12 19:26       ` Ori Kam
2021-04-12 23:31         ` Ferruh Yigit
2021-04-13  7:12           ` Ori Kam
2021-04-13  8:03             ` Ferruh Yigit
2021-04-13  8:18               ` Ori Kam
2021-04-13  8:30                 ` Ferruh Yigit
2021-04-13 10:21                   ` Ori Kam
2021-04-13 17:28                     ` Ferruh Yigit
2021-04-11 17:34   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-12 17:49     ` Ferruh Yigit
2021-04-13  7:53       ` Ori Kam
2021-04-13  8:14         ` Ferruh Yigit
2021-04-13 11:36           ` Ori Kam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=881ca2ff-2e2a-53d0-2668-e745694ff448@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=getelson@nvidia.com \
    --cc=jerinj@marvell.com \
    --cc=jerinjacobk@gmail.com \
    --cc=matan@nvidia.com \
    --cc=olivier.matz@6wind.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git