[dpdk-dev,1/1] ethdev: don't consider device state when validating flows

Message ID 20170324023659.28099-2-johndale@cisco.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

John Daley (johndale) March 24, 2017, 2:36 a.m. UTC
PMDs only consider if a flow would be accepted or not the assuming the
device had all it's resources available to it. Since state is not
considered, -EEXIST and -EBUSY return codes no longer make sense and
are removed. Also clarify the -ENOMEM has nothig to do with device
resouces, only host resources needed rte_flow_validate().

Signed-off-by: John Daley <johndale@cisco.com>
---
 lib/librte_ether/rte_flow.h | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)
  

Comments

Thomas Monjalon April 6, 2017, 8:50 p.m. UTC | #1
Ping,
any progress on this patch?


2017-03-23 19:36, John Daley:
> PMDs only consider if a flow would be accepted or not the assuming the
> device had all it's resources available to it. Since state is not
> considered, -EEXIST and -EBUSY return codes no longer make sense and
> are removed. Also clarify the -ENOMEM has nothig to do with device
> resouces, only host resources needed rte_flow_validate().
> 
> Signed-off-by: John Daley <johndale@cisco.com>
  
John Daley (johndale) April 6, 2017, 10:41 p.m. UTC | #2
Adrien,

Here is another crack at the comments for rte_flow_validate. Does this
capture what you were explaining to me?

I'm still not crazy about multiple meanings for EEXIST or ENOMEM since
it makes them unusable by apps, but at least the comments try to explain it. In 17.08 what about having PMDs indicate if they support flow
collision and resouce checking, or drop those return codes all together?

cheers,
john

John Daley (1):
  ethdev: fix flow validate comments

 lib/librte_ether/rte_flow.h | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
  

Patch

diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index 171a5698e..16846449d 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -932,15 +932,10 @@  struct rte_flow_error {
 /**
  * Check whether a flow rule can be created on a given port.
  *
- * While this function has no effect on the target device, 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 returned value is guaranteed to remain valid only as long as no
- * successful calls to rte_flow_create() or rte_flow_destroy() are made in
- * the meantime and no device parameter affecting flow rules in any way are
- * modified, due to possible collisions or resource limitations (although in
- * such cases EINVAL should not be returned).
+ * The flow rule is validated against the target device. There is no check
+ * against the current state of the device- creating the flow could still
+ * fail due to a lack of resources on the device. This function has no effect
+ * on the target device.
  *
  * @param port_id
  *   Port identifier of Ethernet device.
@@ -965,13 +960,7 @@  struct rte_flow_error {
  *   -ENOTSUP: valid but unsupported rule specification (e.g. partial
  *   bit-masks are unsupported).
  *
- *   -EEXIST: collision with an existing rule.
- *
- *   -ENOMEM: not enough resources.
- *
- *   -EBUSY: action cannot be performed due to busy device resources, may
- *   succeed if the affected queues or even the entire port are in a stopped
- *   state (see rte_eth_dev_rx_queue_stop() and rte_eth_dev_stop()).
+ *   -ENOMEM: not enough host resources to execute this funtion.
  */
 int
 rte_flow_validate(uint8_t port_id,