[dpdk-dev] [PATCH 0/1] proposed minor change in rte_flow_validate semantics

Adrien Mazarguil adrien.mazarguil at 6wind.com
Fri Mar 24 10:46:33 CET 2017


Hi John,

On Thu, Mar 23, 2017 at 07:36:58PM -0700, John Daley wrote:
> Hi,
> 
> In implementing rte_flow_validate() for the Cisco enic, I got to wondering
> if the semantics might be slightly off given how I see apps using it.
> 
> Please forgive me if this has already been discussed, but during runtime, I
> can't see any reason why rte_flow_validate() would be called when
> rte_flow_create() returns the same errors as validate, but also creates
> the filter if all is good (calling validate first is just extra overhead).
> What are the use cases for calling validate when the app is up and running?

In short (I'll elaborate below), to get a rough idea of the capabilities of
a given port at initialization time. One should use rte_flow_validate() with
nonspecific rules to determine if the PMD understands them at all *given the
circumstances* (the current configuration state of the device).

Nothing prevents one from using it just before calling rte_flow_create(),
however doing so is indeed redundant.

> I see rte_flow_validate() being run at startup for answering these 2 questions:
> 1.) Given enough resources, is the device capable of handling flows
> a, b, c, ..,  which the app may want to create during runtime?

Yes, breaking this down means all the following:

1. Recognize all pattern items and actions.
2. Make sure items are properly stacked and actions can be combined.
3. Check for pattern item specification structures (e.g. limits, masks,
   ranges, and so on).
4. Check for actions configuration structures (e.g. target queue exists).

Depending on the flow rule, 3. and 4. may have to check the current device
configuration in order to validate the rule.

> 2.) How many concurrent flows of type a, b, c, .. can the device handle? To
> answer this app needs to do a bunch of rte_flow_create()'s (followed by a bunch of rte_flow_destroy()s).

Hehe, this is the brute force approach, actually the rte_flow API in its
current form is not supposed to answer this question, and I don't think we
should document it that way (note you could use rte_flow_flush() to destroy
all flows at once faster).

Some devices (like mlx5) do not really have an upper limit on the number of
possible flows one can configure, they're basically limited by the available
memory on the host system.

> The answer to these questions will allow the app to decide beforehand which
> filters to offload (if any) and set up function pointers or whatever to run
> as efficiently as possible on the target device.
> 
> rte_flow_validate() has comments that imply it should be run on the fly,like
> "the flow rule is validated against its current configuration state and the
> returned value should be considered valid by the caller for that state only".
> The only real way to implement this is for enic (and I think other nics) is
> to actually do a flow create/destroy within rte_flow_validate(). But, see
> paragraph 2 above- why bother? I looked at the i40e and mlx5 implementations
> and I didn't see any calls to the nics to check a flow against current state
> so they might not be implemented as the comment imply they should be either.

Then you're probably right there is an issue with the documentation, it's
not the intent (more below).

> If there are no use cases for calling validate at runtime,  I think we ought
> to change the semantics of rte_flow_validate() a little to define success to
> mean that the device will accept the flow IF there are enough resources,
> rather than it will accept the flow given the current state. We can safely
> remove the -EEXIST return code since no other drivers set them yet. This
> makes the function easier to implement correctly without trying to do
> something that's not useful anyway.
> 
> The following patch just changes comments but I think that is all that is
> required.

rte_flow_validate() and rte_flow_create() share the same error codes to make
their implementation simpler. rte_flow_validate() basically exposes the
validation part necessarily present in rte_flow_create().

EEXIST and other error codes *may* be returned by rte_flow_validate(), but
this does not force PMDs to check possible collisions during the validation
step, I'll take documentation is not clear enough regarding this.

So to be completely clear, rte_flow_validate() already does *not* guarantee
a rule can be created afterward, only that rte_flow_create() will
understand that rule.

The other reason "configuration state" is mentioned in the documentation
(besides target queues should exist) is that existing rules may have
switched the device in a mode that does not allow different rule types.

If after initialization, matching TCP, UDP and ICMP is possible, creating a
UDP rule might subsequently prevent the creation of otherwise valid TCP and
ICMP rules. rte_flow_validate() should (but is not forced to) check for
that.

What do you think about keeping the defined error codes as is and merging
somehow my above statements in the documentation instead?

-- 
Adrien Mazarguil
6WIND


More information about the dev mailing list