Message-ID: <CALx6S34B_BvkNuqALCCT+2V2dL8rwr9n_DnRfevjkW4UwMF=pw@mail.gmail.com>
Date: Tue, 5 Sep 2023 16:05:09 -0700
From: Tom Herbert <tom@...bertland.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Junfeng Guo <junfeng.guo@...el.com>, intel-wired-lan@...ts.osuosl.org, 
	netdev@...r.kernel.org, anthony.l.nguyen@...el.com, 
	jesse.brandeburg@...el.com, qi.z.zhang@...el.com, ivecera@...hat.com, 
	sridhar.samudrala@...el.com, horms@...nel.org, edumazet@...gle.com, 
	davem@...emloft.net, pabeni@...hat.com
Subject: Re: [PATCH iwl-next v9 00/15] Introduce the Parser Library

On Tue, Sep 5, 2023 at 3:37 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Mon,  4 Sep 2023 10:14:40 +0800 Junfeng Guo wrote:
> > Current software architecture for flow filtering offloading limited
> > the capability of Intel Ethernet 800 Series Dynamic Device
> > Personalization (DDP) Package. The flow filtering offloading in the
> > driver is enabled based on the naming parsers, each flow pattern is
> > represented by a protocol header stack. And there are multiple layers
> > (e.g., virtchnl) to maintain their own enum/macro/structure
> > to represent a protocol header (IP, TCP, UDP ...), thus the extra
> > parsers to verify if a pattern is supported by hardware or not as
> > well as the extra converters that to translate represents between
> > different layers. Every time a new protocol/field is requested to be
> > supported, the corresponding logic for the parsers and the converters
> > needs to be modified accordingly. Thus, huge & redundant efforts are
> > required to support the increasing flow filtering offloading features,
> > especially for the tunnel types flow filtering.
>
> Are you talking about problems internal to ICE or the flower interface?
>
> > This patch set provides a way for applications to send down training
> > packets & masks (in binary) to the driver. Then these binary data
> > would be used by the driver to generate certain data that are needed
> > to create a filter rule in the filtering stage of switch/RSS/FDIR.
>
> What's the API for the user? I see a whole bunch of functions added here
> which never get called.
>
> > Note that the impact of a malicious rule in the raw packet filter is
> > limited to performance rather than functionality. It may affect the
> > performance of the workload, similar to other limitations in FDIR/RSS
> > on AVF. For example, there is no resource boundary for VF FDIR/RSS
> > rules, so one malicious VF could potentially make other VFs
> > inefficient in offloading.
> >
> > The parser library is expected to include boundary checks to prevent
> > critical errors such as infinite loops or segmentation faults.
> > However, only implementing and validating the parser emulator in a
> > sandbox environment (like ebpf) presents a challenge.
> >
> > The idea is to make the driver be able to learn from the DDP package
> > directly to understand how the hardware parser works (i.e., the
> > Parser Library), so that it can process on the raw training packet
> > (in binary) directly and create the filter rule accordingly.
>
> No idea what this means in terms of the larger networking stack.
>

Yes, creating an elaborate mechanism that is only usable for one
vendor, e.g. a feature of DDP, really isn't very helpful. Parsing is a
very common operation in the networking stack, and if there's
something with the vanglorious name of "Parser Library" really should
start off as being a common, foundational, vendor agnostic library to
solve the larger problem and provide the most utility. The common
components would define consistent user and kernel interfaces for
parser offload, interfaces into the NIC drivers would be defined to
allow different vendors to implement parser offload in their devices.

The concepts in kParser patch "net/kparser: add kParser" were aligned
with what the backend of Parser Library might be. That path introduced
iproute commands to program an in kernel parser extensible to support
arbitrary protocols (including constructs like TLVs, flag fields, and
now even nested TLVs). It is quite conceivable that these commands
could be sent to the device to achieve programmable parser offload.

Tom


> > Based on this Parser Library, the raw flow filtering of
> > switch/RSS/FDIR could be enabled to allow new flow filtering
> > offloading features to be supported without any driver changes (only
> > need to update the DDP package).
>
> Sounds like you are talking about some vague "vision" rather than
> the code you're actually posting.
>
> Given that you've posted 5 versions of this to netdev and got no
> notable comments, please don't CC netdev on the next version
> until you get some reviews inside Intel. Stuff like:
>
> +#define ICE_ERR_NOT_IMPL               -1
>
> should get caught by internal review.
>

Powered by blists - more mailing lists