[dpdk-stable] patch 'ethdev: fix invalid configuration after failure' has been queued to stable release 18.08.1
Kevin Traynor
ktraynor at redhat.com
Thu Nov 29 14:20:25 CET 2018
Hi,
FYI, your patch has been queued to stable release 18.08.1
Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 12/08/18. So please
shout if anyone has objections.
Also note that after the patch there's a diff of the upstream commit vs the patch applied
to the branch. If the code is different (ie: not only metadata diffs), due for example to
a change in context or macro names, please double check it.
Thanks.
Kevin Traynor
---
>From 3be9c9bdfb09b4f25d3e26ddf56856be2b24e7ed Mon Sep 17 00:00:00 2001
From: Wenzhuo Lu <wenzhuo.lu at intel.com>
Date: Tue, 13 Nov 2018 11:12:36 +0000
Subject: [PATCH] ethdev: fix invalid configuration after failure
[ upstream commit aa28ec5d27b0ead28877081b30ccf0b74a16bbcd ]
The new configuration is stored during the rte_eth_dev_configure() API
but the API may fail. After failure stored configuration will be
invalid since it is not fully applied to the device.
We better roll the configuration back after failure.
Fixes: af75078fece3 ("first public release")
Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit at intel.com>
Reviewed-by: Andrew Rybchenko <arybchenko at solarflare.com>
---
lib/librte_ethdev/rte_ethdev.c | 49 +++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 07335a1d6..85e80b313 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1020,6 +1020,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
struct rte_eth_dev *dev;
struct rte_eth_dev_info dev_info;
+ struct rte_eth_conf orig_conf;
struct rte_eth_conf local_conf = *dev_conf;
int diag;
+ int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
@@ -1068,4 +1070,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
}
+ /* Store original config, as rollback required on failure */
+ memcpy(&orig_conf, &dev->data->dev_conf, sizeof(dev->data->dev_conf));
+
/* Copy the dev_conf parameter into the dev structure */
memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf));
@@ -1079,5 +1084,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%u nb_rx_queues=%u > %u\n",
port_id, nb_rx_q, dev_info.max_rx_queues);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
@@ -1085,5 +1091,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%u nb_tx_queues=%u > %u\n",
port_id, nb_tx_q, dev_info.max_tx_queues);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
@@ -1093,5 +1100,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Driver %s does not support lsc\n",
dev->device->driver->name);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
if ((dev_conf->intr_conf.rmv == 1) &&
@@ -1099,5 +1107,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Driver %s does not support rmv\n",
dev->device->driver->name);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
@@ -1112,5 +1121,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, dev_conf->rxmode.max_rx_pkt_len,
dev_info.max_rx_pktlen);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
} else if (dev_conf->rxmode.max_rx_pkt_len < ETHER_MIN_LEN) {
RTE_ETHDEV_LOG(ERR,
@@ -1118,5 +1128,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, dev_conf->rxmode.max_rx_pkt_len,
(unsigned)ETHER_MIN_LEN);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
} else {
@@ -1137,5 +1148,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
dev_info.rx_offload_capa,
__func__);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
@@ -1147,5 +1159,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
dev_info.tx_offload_capa,
__func__);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
@@ -1166,5 +1179,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, dev_conf->rx_adv_conf.rss_conf.rss_hf,
dev_info.flow_type_rss_offloads);
- return -EINVAL;
+ ret = -EINVAL;
+ goto rollback;
}
@@ -1177,5 +1191,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
"Port%u rte_eth_dev_rx_queue_config = %d\n",
port_id, diag);
- return diag;
+ ret = diag;
+ goto rollback;
}
@@ -1186,5 +1201,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, diag);
rte_eth_dev_rx_queue_config(dev, 0);
- return diag;
+ ret = diag;
+ goto rollback;
}
@@ -1195,5 +1211,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
rte_eth_dev_rx_queue_config(dev, 0);
rte_eth_dev_tx_queue_config(dev, 0);
- return eth_err(port_id, diag);
+ ret = eth_err(port_id, diag);
+ goto rollback;
}
@@ -1205,8 +1222,14 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
rte_eth_dev_rx_queue_config(dev, 0);
rte_eth_dev_tx_queue_config(dev, 0);
- return eth_err(port_id, diag);
+ ret = eth_err(port_id, diag);
+ goto rollback;
}
return 0;
+
+rollback:
+ memcpy(&dev->data->dev_conf, &orig_conf, sizeof(dev->data->dev_conf));
+
+ return ret;
}
--
2.19.0
---
Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- - 2018-11-29 13:11:35.660236544 +0000
+++ 0024-ethdev-fix-invalid-configuration-after-failure.patch 2018-11-29 13:11:34.000000000 +0000
@@ -1,8 +1,10 @@
-From aa28ec5d27b0ead28877081b30ccf0b74a16bbcd Mon Sep 17 00:00:00 2001
+From 3be9c9bdfb09b4f25d3e26ddf56856be2b24e7ed Mon Sep 17 00:00:00 2001
From: Wenzhuo Lu <wenzhuo.lu at intel.com>
Date: Tue, 13 Nov 2018 11:12:36 +0000
Subject: [PATCH] ethdev: fix invalid configuration after failure
+[ upstream commit aa28ec5d27b0ead28877081b30ccf0b74a16bbcd ]
+
The new configuration is stored during the rte_eth_dev_configure() API
but the API may fail. After failure stored configuration will be
invalid since it is not fully applied to the device.
@@ -10,7 +12,6 @@
We better roll the configuration back after failure.
Fixes: af75078fece3 ("first public release")
-Cc: stable at dpdk.org
Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit at intel.com>
@@ -20,10 +21,10 @@
1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
-index 8eaa5fcc7..04dff1f5e 100644
+index 07335a1d6..85e80b313 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
-@@ -1093,6 +1093,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1020,6 +1020,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
struct rte_eth_dev *dev;
struct rte_eth_dev_info dev_info;
+ struct rte_eth_conf orig_conf;
@@ -32,7 +33,7 @@
+ int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
-@@ -1141,4 +1143,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1068,4 +1070,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
}
+ /* Store original config, as rollback required on failure */
@@ -40,7 +41,7 @@
+
/* Copy the dev_conf parameter into the dev structure */
memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf));
-@@ -1152,5 +1157,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1079,5 +1084,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%u nb_rx_queues=%u > %u\n",
port_id, nb_rx_q, dev_info.max_rx_queues);
- return -EINVAL;
@@ -48,7 +49,7 @@
+ goto rollback;
}
-@@ -1158,5 +1164,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1085,5 +1091,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%u nb_tx_queues=%u > %u\n",
port_id, nb_tx_q, dev_info.max_tx_queues);
- return -EINVAL;
@@ -56,7 +57,7 @@
+ goto rollback;
}
-@@ -1166,5 +1173,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1093,5 +1100,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Driver %s does not support lsc\n",
dev->device->driver->name);
- return -EINVAL;
@@ -64,7 +65,7 @@
+ goto rollback;
}
if ((dev_conf->intr_conf.rmv == 1) &&
-@@ -1172,5 +1180,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1099,5 +1107,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
RTE_ETHDEV_LOG(ERR, "Driver %s does not support rmv\n",
dev->device->driver->name);
- return -EINVAL;
@@ -72,7 +73,7 @@
+ goto rollback;
}
-@@ -1185,5 +1194,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1112,5 +1121,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, dev_conf->rxmode.max_rx_pkt_len,
dev_info.max_rx_pktlen);
- return -EINVAL;
@@ -80,7 +81,7 @@
+ goto rollback;
} else if (dev_conf->rxmode.max_rx_pkt_len < ETHER_MIN_LEN) {
RTE_ETHDEV_LOG(ERR,
-@@ -1191,5 +1201,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1118,5 +1128,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, dev_conf->rxmode.max_rx_pkt_len,
(unsigned)ETHER_MIN_LEN);
- return -EINVAL;
@@ -88,7 +89,7 @@
+ goto rollback;
}
} else {
-@@ -1210,5 +1221,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1137,5 +1148,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
dev_info.rx_offload_capa,
__func__);
- return -EINVAL;
@@ -96,7 +97,7 @@
+ goto rollback;
}
if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
-@@ -1220,5 +1232,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1147,5 +1159,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
dev_info.tx_offload_capa,
__func__);
- return -EINVAL;
@@ -104,7 +105,7 @@
+ goto rollback;
}
-@@ -1231,5 +1244,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1166,5 +1179,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, dev_conf->rx_adv_conf.rss_conf.rss_hf,
dev_info.flow_type_rss_offloads);
- return -EINVAL;
@@ -112,7 +113,7 @@
+ goto rollback;
}
-@@ -1242,5 +1256,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1177,5 +1191,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
"Port%u rte_eth_dev_rx_queue_config = %d\n",
port_id, diag);
- return diag;
@@ -120,7 +121,7 @@
+ goto rollback;
}
-@@ -1251,5 +1266,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1186,5 +1201,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
port_id, diag);
rte_eth_dev_rx_queue_config(dev, 0);
- return diag;
@@ -128,7 +129,7 @@
+ goto rollback;
}
-@@ -1260,5 +1276,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1195,5 +1211,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
rte_eth_dev_rx_queue_config(dev, 0);
rte_eth_dev_tx_queue_config(dev, 0);
- return eth_err(port_id, diag);
@@ -136,7 +137,7 @@
+ goto rollback;
}
-@@ -1270,8 +1287,14 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+@@ -1205,8 +1222,14 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
rte_eth_dev_rx_queue_config(dev, 0);
rte_eth_dev_tx_queue_config(dev, 0);
- return eth_err(port_id, diag);
More information about the stable
mailing list