[dpdk-dev] [RFC] ethdev: add sanity packet checks

[Ori Kam]

Hi

> -----Original Message-----
> From: dev <dev-bounces at dpdk.org> On Behalf Of Thomas Monjalon
> Sent: Tuesday, March 9, 2021 11:11 AM
> Subject: Re: [dpdk-dev] [RFC] ethdev: add sanity packet checks
> 
> 09/03/2021 10:01, Andrew Rybchenko:
> > On 2/28/21 10:48 PM, Ori Kam wrote:
> > > Currently, DPDK application can offload the checksum check,
> > > and report it in the mbuf.
> > >
> > > However, this approach doesn't work if the traffic
> > > is offloaded and should not arrive to the application.
> > >
> > > This commit introduces rte flow item that enables
> > > matching on the checksum of the L3 and L4 layers,
> > > in addition to other checks that can determine if
> > > the packet is valid.
> > > some of those tests can be packet len, data len,
> > > unsupported flags, and so on.
> > >
> > > The full check is HW dependent.
> > >
> > > Signed-off-by: Ori Kam <orika at nvidia.com>
> >
> > In general, I strongly dislike the approach. If such checks are required,
> > it must be done per item basis. I.e. we should add non-header boolean
> > flags to IPv4, TCP, UDP etc items. E.g.
> >
> > struct rte_flow_item_ipv4 {
> >         struct rte_ipv4_hdr hdr; /**< IPv4 header definition. */
> >         bool hdr_checksum_valid;
> > };
> >
> > Also it will allow to filter by packets with invalid checksum as well and
> > redirect to dedicated Rx path or drop and/or count etc.
> 
> +1
> 
I'm not sure I understand your comment, also according to the proposed
RFC we can redirect to the correct path.

> I think the only drawback of this solution is for HW giving a global
> check status without knowing which header is valid or invalid.
> 
Like Thomas remark the main drawback with adding the valid to each
of the items is that, it forces the application to have detected rule per
each type, which will make the SW logic more complex.
Also this may have performance impact on the packets and on the
number of flows.