Re: [dpdk-dev] [PATCH] ethdev: add packet integrity checks

[Ori Kam <orika@nvidia.com>]

Hi Jerin,

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, April 6, 2021 10:40 AM
> To: Ori Kam <orika@nvidia.com>
> Subject: Re: [dpdk-dev] [PATCH] ethdev: add packet integrity checks
> 
> On Mon, Apr 5, 2021 at 11:35 PM Ori Kam <orika@nvidia.com> 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?
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 <orika@nvidia.com>
> > ---
> >  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