From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id DE76BA0A02; Wed, 7 Apr 2021 13:02:09 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BCC214069F; Wed, 7 Apr 2021 13:02:09 +0200 (CEST) Received: from mail-il1-f175.google.com (mail-il1-f175.google.com [209.85.166.175]) by mails.dpdk.org (Postfix) with ESMTP id 442594013F for ; Wed, 7 Apr 2021 13:02:09 +0200 (CEST) Received: by mail-il1-f175.google.com with SMTP id 6so6168869ilt.9 for ; Wed, 07 Apr 2021 04:02:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=2b6ukZI5tyDwB/5tu1Tc1zK7E4+JiQvs1Sia+upl0DQ=; b=FVx9ZT05KYm3uQP2tdn6ThlWNhV5qu4xZinbHfgmjUBMTEM+TJkqk88UPk7q6y0pvl 1oAOpD/NLpUPKyXnpS/aNvyua6sBm/tKEPBzY2/KZWGLKjYOTu4IjplKNMWp71vG+HwL NSu/fLYLgbO5PO2cWDtKFXJfk6ZmS+jnN/I4cU4bzGVpm02oEpMMqO32uU0JhdcktZFT 05viqAFMT5cfJKG20BdZVt1zQWKIc2jbham2XO/OrZyvojnjePU6cZU0v+s4rn6VbhtW xZylmE0wyOQqulxNBba62SOplGE9BVp12qIuC2kBudN7vW0+Ctqc9ETzKb9vhnBwlcD0 hzvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2b6ukZI5tyDwB/5tu1Tc1zK7E4+JiQvs1Sia+upl0DQ=; b=a6Viq6B47Kz/w/eFgpng5Qcta2Cn3zxMXYs11ziJ/X4zk2iZM61DiwLyR7auiKhm/t 9wehWjbo+xYtCvOgE7iCYbyyn7z64x9OjtZpxXGrE9jiftFeekuPrWnWrT/gP/6LAG3w Zm1tO0Nmwmm7IIAiOlSSZjjqrYtiXQtTPXSlW/0d8KalF+G2SIkiyVAxbaeeseevIokg C6l2AbTEE6XlFLO8lFmbT8KfI10qG970HomiLf/7MtB7f44UQKbVwEl1/u6jvFxBTM6h b0xYXT4F8o0/JGRse3QiVf1KK8vaC7pc+7FTQSCdk6O3fl/yEKPvNqBUofR0oQiB3QJB w36g== X-Gm-Message-State: AOAM530WnbVX8wdVfm1bwcBC/pu/gL4iLK792wqHU0ecDMk1UbHuHtD9 yQQOKLTQK1pQbycySOacYvaVNBVgQ1AidcR/JvI= X-Google-Smtp-Source: ABdhPJzGc6IylcCzHHTj89OczTbTpoj8PXO03xY9nqei6tq3mcDguSjyN1+e+sNfYHMRERHV0aj4ei1Sxef5pAMcBgk= X-Received: by 2002:a92:c566:: with SMTP id b6mr1311524ilj.162.1617793328611; Wed, 07 Apr 2021 04:02:08 -0700 (PDT) MIME-Version: 1.0 References: <1617645874-105139-1-git-send-email-orika@nvidia.com> In-Reply-To: From: Jerin Jacob Date: Wed, 7 Apr 2021 16:31:52 +0530 Message-ID: To: Ori Kam Cc: Ajit Khaparde , Andrew Rybchenko , Ferruh Yigit , NBU-Contact-Thomas Monjalon , dpdk-dev , Jerin Jacob , Olivier Matz , Slava Ovsiienko Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] ethdev: add packet integrity checks X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Apr 7, 2021 at 4:02 PM Ori Kam wrote: > > Hi Jerin, Hi Ori, > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Tuesday, April 6, 2021 10:40 AM > > To: Ori Kam > > Subject: Re: [dpdk-dev] [PATCH] ethdev: add packet integrity checks > > > > On Mon, Apr 5, 2021 at 11:35 PM Ori Kam wrote: > > > > > > 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 currenlty rte_flow works in positive way the assumtion is > > > that the postive 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. Flexability. > > > > > > Alteast in Marvell HW integrity checks are functions of the Ethdev Rx > > queue attribute. > > Not sure about other Vendor HW. > > I'm not sure what do you mean? What I meant is, What will be the preferred way to configure the feature? ie. Is it as ethdev Rx offload or rte_flow? I think, in order to decide that, we need to know, how most of the other HW express this feature? > This is the idea of the patch, to allow application to route the packet > before getting to the Rx, > In any case all items support is dependent on HW capabilities. > > > > > > > > > > > > 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 flor layer 2 have passed. > > > 3. l3_ok - all check flor layer 2 have passed. If packet doens't have > > > l3 layer this check shoudl fail. > > > 4. l4_ok - all check flor layer 2 have passed. If packet doesn't > > > have l4 layer this check should fail. > > > 5. l2_crc_ok - the layer 2 crc is O.K. it is possible that the crc will > > > be O.K. but the l3_ok will be 0. it is not possible that l2_crc_ok will > > > be 0 and the l3_ok will be 0. > > > 6. ipv4_csum_ok - IPv4 checksum is O.K. > > > 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 > > > packet 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 > > > --- > > > doc/guides/prog_guide/rte_flow.rst | 19 ++++++++++++++++ > > > lib/librte_ethdev/rte_flow.h | 46 > > ++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 65 insertions(+) > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst > > b/doc/guides/prog_guide/rte_flow.rst > > > index aec2ba1..58b116e 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. > > > + > > > +- ``level``: the encapsulation level that should be checked. level 0 means the > > > + default PMD mode (Can be inner most / outermost). value of 1 means > > outermost > > > + and higher value means inner header. See also RSS level. > > > +- ``packet_ok``: All HW packet integrity checks have passed based on the > > max > > > + layer of the packet. > > > + layer of the packet. > > > +- ``l2_ok``: all layer 2 HW integrity checks passed. > > > +- ``l3_ok``: all layer 3 HW integrity checks passed. > > > +- ``l4_ok``: all layer 3 HW integrity checks passed. > > > +- ``l2_crc_ok``: layer 2 crc check passed. > > > +- ``ipv4_csum_ok``: ipv4 checksum check passed. > > > +- ``l4_csum_ok``: layer 4 checksum check passed. > > > +- ``l3_len_ok``: the layer 3 len is smaller than the packet len. > > > + > > > Actions > > > ~~~~~~~ > > > > > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h > > > index 6cc5713..f6888a1 100644 > > > --- a/lib/librte_ethdev/rte_flow.h > > > +++ b/lib/librte_ethdev/rte_flow.h > > > @@ -551,6 +551,15 @@ enum rte_flow_item_type { > > > * See struct rte_flow_item_geneve_opt > > > */ > > > RTE_FLOW_ITEM_TYPE_GENEVE_OPT, > > > + > > > + /** > > > + * [META] > > > + * > > > + * Matches on packet integrity. > > > + * > > > + * See struct rte_flow_item_packet_integrity_checks. > > > + */ > > > + RTE_FLOW_ITEM_TYPE_PACKET_INTEGRITY_CHECKS, > > > }; > > > > > > /** > > > @@ -1685,6 +1694,43 @@ struct rte_flow_item_geneve_opt { > > > }; > > > #endif > > > > > > +struct rte_flow_item_packet_integrity_checks { > > > + uint32_t level; > > > + /**< Packet encapsulation level the item should apply to. > > > + * @see rte_flow_action_rss > > > + */ > > > +RTE_STD_C11 > > > + union { > > > + struct { > > > + uint64_t packet_ok:1; > > > + /** The packet is valid after passing all HW checks. */ > > > + uint64_t l2_ok:1; > > > + /**< L2 layer is valid after passing all HW checks. */ > > > + uint64_t l3_ok:1; > > > + /**< L3 layer is valid after passing all HW checks. */ > > > + uint64_t l4_ok:1; > > > + /**< L4 layer is valid after passing all HW checks. */ > > > + uint64_t l2_crc_ok:1; > > > + /**< L2 layer checksum is valid. */ > > > + uint64_t ipv4_csum_ok:1; > > > + /**< L3 layer checksum is valid. */ > > > + uint64_t l4_csum_ok:1; > > > + /**< L4 layer checksum is valid. */ > > > + uint64_t l3_len_ok:1; > > > + /**< The l3 len is smaller than the packet len. */ > > > + uint64_t reserved:56; > > > + }; > > > + uint64_t value; > > > + }; > > > +}; > > > + > > > +#ifndef __cplusplus > > > +static const struct rte_flow_item_sanity_checks > > > + rte_flow_item_sanity_checks_mask = { > > > + .value = 0, > > > +}; > > > +#endif > > > + > > > /** > > > * Matching pattern item definition. > > > * > > > -- > > > 1.8.3.1 > > > > > Best, > Ori