There is time between the physical removal of the device until
sub-device PMDs get a RMV interrupt. At this time DPDK PMDs and
applications still don't know about the removal and may call sub-device
control operation which should return an error.
In previous code this error is reported to the application contrary to
fail-safe principle that the app should not be aware of device removal.
Define a removal error that each sub-device PMD should return in case
of an error caused by removal event; The special error is -ENODEV.
Add an error check in each relevant control command error flow and
prevent an error report to application when its value is -ENODEV.
Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD")
Fixes: b737a1e ("net/failsafe: support flow API")
Cc: sta...@dpdk.org
Signed-off-by: Matan Azrad <ma...@mellanox.com>
---
doc/guides/nics/fail_safe.rst | 7 +++++++
doc/guides/prog_guide/env_abstraction_layer.rst | 3 +++
drivers/net/failsafe/failsafe_flow.c | 16 +++++++++------
drivers/net/failsafe/failsafe_ops.c | 27 ++++++++++++++++---------
drivers/net/failsafe/failsafe_private.h | 8 ++++++++
5 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/doc/guides/nics/fail_safe.rst b/doc/guides/nics/fail_safe.rst
index c4e3d2e..5023fc4 100644
--- a/doc/guides/nics/fail_safe.rst
+++ b/doc/guides/nics/fail_safe.rst
@@ -193,6 +193,13 @@ any time. The fail-safe PMD will register a callback for
such event and react
accordingly. It will try to safely stop, close and uninit the sub-device having
emitted this event, allowing it to free its eventual resources.
+When fail-safe PMD gets -ENODEV error from control command sent to removable
+sub-devices, it assumes that the error reason is device removal. In this case
+fail-safe returns success value to application. The PMD controlling the
+sub-device is still responsible to emit a removal event (RMV) in addition to
+returning -ENODEV from control operations after the device has been physically
+removed. Only the reception of this event unregisters it on the fail-safe side.
+
Fail-safe glossary
------------------
diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst
b/doc/guides/prog_guide/env_abstraction_layer.rst
index 4775eb3..bd2fd87 100644
--- a/doc/guides/prog_guide/env_abstraction_layer.rst
+++ b/doc/guides/prog_guide/env_abstraction_layer.rst
@@ -213,6 +213,9 @@ device having emitted a Device Removal Event. In such case,
calling
callback. Care must be taken not to close the device from the interrupt handler
context. It is necessary to reschedule such closing operation.
+Unsuccessful control operations (for those that return errors) may return
+-ENODEV after the device is physically unplugged.
+
Blacklisting
~~~~~~~~~~~~
diff --git a/drivers/net/failsafe/failsafe_flow.c
b/drivers/net/failsafe/failsafe_flow.c
index 153ceee..ce9b769 100644
--- a/drivers/net/failsafe/failsafe_flow.c
+++ b/drivers/net/failsafe/failsafe_flow.c
@@ -87,7 +87,7 @@
DEBUG("Calling rte_flow_validate on sub_device %d", i);
ret = rte_flow_validate(PORT_ID(sdev),
attr, patterns, actions, error);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_flow_validate failed for
sub_device %d"
" with error %d", i, ret);
return ret;
@@ -111,7 +111,8 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
flow->flows[i] = rte_flow_create(PORT_ID(sdev),
attr, patterns, actions, error);
- if (flow->flows[i] == NULL) {
+ if (flow->flows[i] == NULL &&
+ !SUBDEV_REMOVED(sdev, -rte_errno)) {
ERROR("Failed to create flow on sub_device %d",
i);
goto err;
@@ -150,7 +151,7 @@
continue;
local_ret = rte_flow_destroy(PORT_ID(sdev),
flow->flows[i], error);
- if (local_ret) {
+ if (local_ret && !SUBDEV_REMOVED(sdev, local_ret)) {
ERROR("Failed to destroy flow on sub_device %d: %d",
i, local_ret);
if (ret == 0)
@@ -175,7 +176,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_flow_flush on sub_device %d", i);
ret = rte_flow_flush(PORT_ID(sdev), error);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_flow_flush failed for sub_device
%d"
" with error %d", i, ret);
return ret;
@@ -199,8 +200,11 @@
sdev = TX_SUBDEV(dev);
if (sdev != NULL) {
- return rte_flow_query(PORT_ID(sdev),
+ int ret = rte_flow_query(PORT_ID(sdev),
flow->flows[SUB_ID(sdev)], type, arg, error);
+
+ if (!SUBDEV_REMOVED(sdev, ret))
+ return ret;
}
WARN("No active sub_device to query about its flow");
return -1;
@@ -223,7 +227,7 @@
WARN("flow isolation mode of sub_device %d in
incoherent state.",
i);
ret = rte_flow_isolate(PORT_ID(sdev), set, error);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_flow_isolate failed for sub_device
%d"
" with error %d", i, ret);
return ret;
diff --git a/drivers/net/failsafe/failsafe_ops.c
b/drivers/net/failsafe/failsafe_ops.c
index f460551..cc7ab7f 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -314,7 +314,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_set_link_up on sub_device %d", i);
ret = rte_eth_dev_set_link_up(PORT_ID(sdev));
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_eth_dev_set_link_up failed for
sub_device %d"
" with error %d", i, ret);
return ret;
@@ -333,7 +333,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_set_link_down on sub_device %d", i);
ret = rte_eth_dev_set_link_down(PORT_ID(sdev));
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_eth_dev_set_link_down failed for
sub_device %d"
" with error %d", i, ret);
return ret;
@@ -418,7 +418,7 @@
rx_queue_id,
nb_rx_desc, socket_id,
rx_conf, mb_pool);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("RX queue setup failed for sub_device %d", i);
goto free_rxq;
}
@@ -484,7 +484,7 @@
tx_queue_id,
nb_tx_desc, socket_id,
tx_conf);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("TX queue setup failed for sub_device %d", i);
goto free_txq;
}
@@ -563,7 +563,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling link_update on sub_device %d", i);
ret = (SUBOPS(sdev, link_update))(ETH(sdev), wait_to_complete);
- if (ret && ret != -1) {
+ if (ret && ret != -1 && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Link update failed for sub_device %d with error
%d",
i, ret);
return ret;
@@ -587,6 +587,7 @@
fs_stats_get(struct rte_eth_dev *dev,
struct rte_eth_stats *stats)
{
+ struct rte_eth_stats backup;
struct sub_device *sdev;
uint8_t i;
int ret;
@@ -596,14 +597,20 @@
struct rte_eth_stats *snapshot = &sdev->stats_snapshot.stats;
uint64_t *timestamp = &sdev->stats_snapshot.timestamp;
+ rte_memcpy(&backup, snapshot, sizeof(backup));
ret = rte_eth_stats_get(PORT_ID(sdev), snapshot);
if (ret) {
+ if (SUBDEV_REMOVED(sdev, ret)) {
+ rte_memcpy(snapshot, &backup, sizeof(backup));
+ goto inc;
+ }
ERROR("Operation rte_eth_stats_get failed for
sub_device %d with error %d",
i, ret);
*timestamp = 0;
return ret;
}
*timestamp = rte_rdtsc();
+inc:
failsafe_stats_increment(stats, snapshot);
}
return 0;
@@ -716,7 +723,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_set_mtu on sub_device %d", i);
ret = rte_eth_dev_set_mtu(PORT_ID(sdev), mtu);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_eth_dev_set_mtu failed for
sub_device %d with error %d",
i, ret);
return ret;
@@ -735,7 +742,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_vlan_filter on sub_device %d", i);
ret = rte_eth_dev_vlan_filter(PORT_ID(sdev), vlan_id, on);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_eth_dev_vlan_filter failed for
sub_device %d"
" with error %d", i, ret);
return ret;
@@ -769,7 +776,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_flow_ctrl_set on sub_device %d", i);
ret = rte_eth_dev_flow_ctrl_set(PORT_ID(sdev), fc_conf);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_eth_dev_flow_ctrl_set failed for
sub_device %d"
" with error %d", i, ret);
return ret;
@@ -806,7 +813,7 @@
RTE_ASSERT(index < FAILSAFE_MAX_ETHADDR);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
ret = rte_eth_dev_mac_addr_add(PORT_ID(sdev), mac_addr, vmdq);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_eth_dev_mac_addr_add failed for
sub_device %"
PRIu8 " with error %d", i, ret);
return ret;
@@ -848,7 +855,7 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_filter_ctrl on sub_device %d", i);
ret = rte_eth_dev_filter_ctrl(PORT_ID(sdev), type, op, arg);
- if (ret) {
+ if (ret && !SUBDEV_REMOVED(sdev, ret)) {
ERROR("Operation rte_eth_dev_filter_ctrl failed for
sub_device %d"
" with error %d", i, ret);
return ret;
diff --git a/drivers/net/failsafe/failsafe_private.h
b/drivers/net/failsafe/failsafe_private.h
index d81cc3c..ee81b70 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -262,6 +262,14 @@ int failsafe_eth_lsc_event_callback(uint16_t port_id,
(ETH(s)->dev_ops->ops)
/**
+ * s: (struct sub_device *)
+ * e: (int) error
+ */
+#define SUBDEV_REMOVED(s, e) \
+ (s->remove || \
+ (((e) == -ENODEV) && (ETH(s)->data->dev_flags & RTE_ETH_DEV_INTR_RMV)))
+
+/**
* Atomic guard
*/
--
1.8.3.1
