Date: Thu, 7 Sep 2023 14:08:15 -0500
From: "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, Junfeng Guo <junfeng.guo@...el.com>
CC: <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>, <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 9/5/2023 5:37 PM, Jakub Kicinski 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 is specific to the ice driver implementation. A package called 
Dynamic device personalization(DDP) is loaded by the driver during probe 
time to configure the HW pipeline.
Today, the driver has a non-scalable implementation that needs to be 
changed whenever the DDP package extends the capabilities supported by 
our HW pipeline. We will re-word the problem statement in the next revision.

> 
>> 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.

This link shows an early version of a user of this patch series
	https://lore.kernel.org/intel-wired-lan/20230818064703.154183-1-junfeng.guo@intel.com/

This API is planned to be exposed to VF drivers via virtchnl interface 
to pass raw training packets and masks. The VF using this API can only 
steer RX traffic directed that VF to its own queues.

> 
>> 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.

I think the usage of 'parser library' is misleading. This is not a 
generic library, but specific to ice and enables the driver to learn the 
hw parser capabilities from the DDP package that is downloaded to HW. 
This information along with the raw packet/mask is used to figure out 
all the metadata required to add a filter rule.

> 
>> 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.
> 
Will do more internal reviews before submitting the next revision.

- Sridhar & Jesse

Powered by blists - more mailing lists