Re: [dpdk-dev] [PATCH 01/10] ethdev: add a generic flow and new behavior switch to fdir

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-02-25 15:03, Rahul Lakkireddy:
> On Wednesday, February 02/24/16, 2016 at 14:17:58 -0800, Thomas Monjalon wrote:
> > > A raw flow provides a generic way for vendors to add their vendor
> > > specific input flow.
> > 
> > Please, "generic" and "vendor specific" in the same sentence.
> > It's obviously wrong.
> 
> I think this sentence is being mis-interpreted.
> What I intended to say is: the fields are generic so that any vendor can
> hook-in. The fields themselves are not vendor specific.

We are trying to push some features into fields of an API instead of
thinking how to make it simple.

> > > In our case, it is possible to match several flows
> > > in a single rule.  For example, it's possible to set an ethernet, vlan,
> > > ip and tcp/udp flows all in a single rule.  We can specify all of these
> > > flows in a single raw input flow, which can then be passed to cxgbe flow
> > > director to set the corresponding filter.
> > 
> > I feel we need to define what is an API.
> > If the application wants to call something specific to the NIC, why using
> > the ethdev API? You just have to include cxgbe.h.
> 
> Well, in that sense, flow-director is also very intel specific, no ?

Yes. I think the term "flow director" comes from Intel.

> What we are trying to do is make flow-director generic

So let's stop calling it flow director.
We are talking about filtering, right?

> and, we have been
> following the review comments on this. If there are better ideas on how
> to achieve this, we are open to suggestions/comments and are ready to
> re-do the series and re-submit also.

My first question: are you happy with the current API?
Do you understand the difference between RTE_ETH_FILTER_ETHERTYPE and
RTE_ETH_FILTER_FDIR with RTE_ETH_FLOW_L2_PAYLOAD?
Do you understand this structure?
enum rte_eth_fdir_status {
    RTE_ETH_FDIR_NO_REPORT_STATUS = 0, /**< Report nothing. */
    RTE_ETH_FDIR_REPORT_ID,            /**< Only report FD ID. */
    RTE_ETH_FDIR_REPORT_ID_FLEX_4,     /**< Report FD ID and 4 flex bytes. */
    RTE_ETH_FDIR_REPORT_FLEX_8,        /**< Report 8 flex bytes. */
};
These values?
enum rte_fdir_mode {
    RTE_FDIR_MODE_NONE      = 0, /**< Disable FDIR support. */
    RTE_FDIR_MODE_SIGNATURE,     /**< Enable FDIR signature filter mode. */
    RTE_FDIR_MODE_PERFECT,       /**< Enable FDIR perfect filter mode. */
    RTE_FDIR_MODE_PERFECT_MAC_VLAN, /**< Enable FDIR filter mode - MAC VLAN. */
    RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode - tunnel. */
};

>From my point of view, it is insane.
We have put the hardware complexity in the API.
And now you want to put some vendor specific data in some fields
like some black magic recipes.

Why is it so complex? We are talking about packet filtering, not rocket science!

> > I know the support of filters among NICs is really heterogeneous.
> > And the DPDK API are not yet generic enough. But please do not give up!
> > If the filtering API can be improved to support your cases, please do it.
> 
> I am not giving up. If there are better suggestions then, I am willing
> to re-do and re-submit the series.
> If the approach taken in RFC v1 series looks more promising then, I can
> re-surrect that also. However, I will need some direction over here so
> that it becomes generic and doesn't remain intel specific as it is now.

Yes the approach in the RFC was better in the sense it was describing the
fields. But honestly, I'd prefer thinking of filtering from scratch.

What is a hardware filter? (note there is no such doc yet)
It matches a packet with some criterias and take an action on it.
Simple.
Now details (it can take weeks or months to list every details).

A hardware implements a subset of the infinite capabilities.
So the API must provide a flag to check a rule/action capability without
really configuring it.

A matching rule must match every criterias or only some of them (AND/OR operators).
An action is associated to a matching rule.
There can be several matching rules on the same port (Chelsio case?).
Any packet field can be matched (we currently define some of them).

An action can be of different types:
- drop
- switch
- accept in any queue
- accept in a specific queue

Most of the rules give some values to match the fields.
The hash filtering (RSS) specify only some fields and a key to direct
packets in different queues.

Please, Intel, Chelsio and other vendors, tell what is wrong above
and let's start a sane discussion on hardware filtering.
More background:
The current API was designed by Intel when they were the only NIC vendor.
Now that there are 8 vendors with different capabilities and that FPGA should
bring even more capabilities, we should be able to build something more
generic while being usable.