patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH 1/1] net/failsafe: link_update request crashing at boot
@ 2021-10-21 11:51 vipul.ashri
  2021-10-21 21:42 ` [dpdk-stable] [PATCH v2] " vipul.ashri
  2022-02-14 13:44 ` vipul.ashri
  0 siblings, 2 replies; 13+ messages in thread
From: vipul.ashri @ 2021-10-21 11:51 UTC (permalink / raw)
  To: grive; +Cc: dev, Vipul Ashri, stable

From: Vipul Ashri <vipul.ashri@oracle.com>

failsafe crashed while sending early link_update request during
boot time initialization.
Based on debugging we found failsafe device was good but sub-
devices were progressing towards initialization and SUBOPS macro
where expanding macro gives [partial_dev]->dev_ops->link_update()
execution of which triggered crash because dev_ops==0. similar
crash seen at failsafe_eth_dev_close()

Failsafe driver need a separate check for subdevices similar to
"RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);" which is
called to almost every eth_dev function.

Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD")
Cc: stable@dpdk.org
Signed-off-by: Vipul Ashri <vipul.ashri@oracle.com>
---
 drivers/net/failsafe/failsafe_ops.c     | 45 +++++++++++++++++++++++--
 drivers/net/failsafe/failsafe_private.h |  6 ++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index d0030af061..f0c5e40fd5 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -371,7 +371,8 @@ fs_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
 		close(rxq->event_fd);
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
 		if (ETH(sdev)->data->rx_queues != NULL &&
-		    ETH(sdev)->data->rx_queues[rxq->qid] != NULL)
+			ETH(sdev)->data->rx_queues[rxq->qid] != NULL &&
+			SUBDEV_VALID_PORTID(sdev)) {
 			SUBOPS(sdev, rx_queue_release)(ETH(sdev), rxq->qid);
 	}
 	dev->data->rx_queues[rxq->qid] = NULL;
@@ -405,6 +406,12 @@ fs_rx_queue_setup(struct rte_eth_dev *dev,
 	fs_lock(dev, 0);
 	if (rx_conf->rx_deferred_start) {
 		FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
+			if (!SUBDEV_VALID_PORTID(sdev)) {
+				ERROR("fs_rx_queue_setup: Invalid sub-device "
+					"port_id=%u\n", PORT_ID(sdev));
+				fs_unlock(dev, 0);
+				return -ENODEV;
+			}
 			if (SUBOPS(sdev, rx_queue_start) == NULL) {
 				ERROR("Rx queue deferred start is not "
 					"supported for subdevice %d", i);
@@ -548,7 +555,8 @@ fs_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
 	fs_lock(dev, 0);
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
 		if (ETH(sdev)->data->tx_queues != NULL &&
-		    ETH(sdev)->data->tx_queues[txq->qid] != NULL)
+			ETH(sdev)->data->tx_queues[txq->qid] != NULL &&
+			SUBDEV_VALID_PORTID(sdev)) {
 			SUBOPS(sdev, tx_queue_release)(ETH(sdev), txq->qid);
 	}
 	dev->data->tx_queues[txq->qid] = NULL;
@@ -571,6 +579,12 @@ fs_tx_queue_setup(struct rte_eth_dev *dev,
 	fs_lock(dev, 0);
 	if (tx_conf->tx_deferred_start) {
 		FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
+			if (!SUBDEV_VALID_PORTID(sdev)) {
+				ERROR("fs_tx_queue_setup: Invalid sub-device "
+					"port_id=%u\n", PORT_ID(sdev));
+				fs_unlock(dev, 0);
+				return -ENODEV;
+			}
 			if (SUBOPS(sdev, tx_queue_start) == NULL) {
 				ERROR("Tx queue deferred start is not "
 					"supported for subdevice %d", i);
@@ -645,6 +659,12 @@ failsafe_eth_dev_close(struct rte_eth_dev *dev)
 	fs_lock(dev, 0);
 	failsafe_hotplug_alarm_cancel(dev);
 	if (PRIV(dev)->state == DEV_STARTED) {
+		if (!rte_eth_dev_is_valid_port(dev->data->port_id)) {
+			ERROR("failsafe_eth_dev_close: Invalid sub-device "
+				"port_id=%u\n", dev->data->port_id);
+			fs_unlock(dev, 0);
+			return -ENODEV;
+		}
 		ret = dev->dev_ops->dev_stop(dev);
 		if (ret != 0) {
 			fs_unlock(dev, 0);
@@ -827,6 +847,12 @@ fs_link_update(struct rte_eth_dev *dev,
 
 	fs_lock(dev, 0);
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		if (!SUBDEV_VALID_PORTID(sdev)) {
+			ERROR("fs_link_update: Invalid Sub-device "
+				"port_id=%u\n", PORT_ID(sdev));
+			fs_unlock(dev, 0);
+			return -ENODEV;
+		}
 		DEBUG("Calling link_update on sub_device %d", i);
 		ret = (SUBOPS(sdev, link_update))(ETH(sdev), wait_to_complete);
 		if (ret && ret != -1 && sdev->remove == 0 &&
@@ -1251,6 +1277,15 @@ fs_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 		goto unlock;
 	}
 	edev = ETH(sdev);
+
+	if (!SUBDEV_VALID_PORTID(sdev)) {
+		ERROR("fs_dev_supported_ptypes_get: "
+			"Invalid TX_SUBDEV port_id=%u\n", PORT_ID(sdev));
+		rte_errno = -ENODEV;
+		ret = NULL;
+		goto unlock;
+	}
+
 	/* ENOTSUP: counts as no supported ptypes */
 	if (SUBOPS(sdev, dev_supported_ptypes_get) == NULL) {
 		ret = NULL;
@@ -1326,6 +1361,12 @@ fs_flow_ctrl_get(struct rte_eth_dev *dev,
 		ret = 0;
 		goto unlock;
 	}
+	if (!SUBDEV_VALID_PORTID(sdev)) {
+		ERROR("fs_flow_ctrl_get_get: Invalid TX_SUBDEV "
+			"port_id=%u\n", PORT_ID(sdev));
+		ret = -ENODEV;
+		goto unlock;
+	}
 	if (SUBOPS(sdev, flow_ctrl_get) == NULL) {
 		ret = -ENOTSUP;
 		goto unlock;
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index cd39d103c6..95eb31f9e6 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -308,6 +308,12 @@ extern int failsafe_mac_from_arg;
 	 : (PRIV(dev)->subs[PRIV(dev)->subs_tx].state < DEV_PROBED ? NULL \
 	 : &PRIV(dev)->subs[PRIV(dev)->subs_tx]))
 
+/**
+ * check for fail-safe sub-device valid port
+ */
+#define SUBDEV_VALID_PORTID(s) \
+    rte_eth_dev_is_valid_port(PORT_ID(s))
+
 /**
  * s:   (struct sub_device *)
  * ops: (struct eth_dev_ops) member
-- 
2.28.0.windows.1


^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH v2] net/failsafe: link_update request crashing at boot
@ 2022-02-14 13:09 Vipul Ashri
  2022-02-14 16:54 ` Stephen Hemminger
  0 siblings, 1 reply; 13+ messages in thread
From: Vipul Ashri @ 2022-02-14 13:09 UTC (permalink / raw)
  To: Gaëtan Rivet, dev; +Cc: stable, Madhuker Mythri

[-- Attachment #1: Type: text/plain, Size: 9314 bytes --]

Hi Gaetan



Sorry for very late reply, we were busy working on 21.11 integration.

Although we have adopted this code internally for us but I am sharing the patch to opensource for community benefit.



This is specific case of AZURE setup with our very customized complex environment.

Let me share the logs with traceback first

SECONDARY PROCESS

timestamp=1633598184

TCZ0.0.0 Cycle 152 (Build 1832)

signal 11 (Segmentation fault), address is 0x31117bbce6c8 from 0x47d08b1



[bt]: ( 1) _Z18snprintf_backtraceRPciiP9siginfo_tPv      (+   0xf4) - sp = 0x7fffef3fd110, ip = 0x3acdc54

[bt]: ( 2) _Z13crit_err_hdlriP9siginfo_tPv               (+  0x159) - sp = 0x7fffef3fdc20, ip = 0x3acdf29

[bt]: ( 3) _ZN13SignalAdapter12handleSignalEiP9siginfo_tPv (+  0x104) - sp = 0x7fffef3fdf00, ip = 0x274d4c4

[bt]: ( 4) _L_unlock_18                                  (+   0x2c) - sp = 0x7fffef3fdf80, ip = 0x7ffff7bce630

[bt]: ( 5) rte_eth_dev_attach_secondary                  (+   0x21) - sp = 0x7fffef3fec50, ip = 0x47d08b1

[bt]: ( 6) rte_eth_from_ring                             (+ 0x3438) - sp = 0x7fffef3fec80, ip = 0x4e49da8

[bt]: ( 7) _init                                         (+ 0xa1b8) - sp = 0x7fffef3feec0, ip = 0x12e0368

[bt]: ( 8) local_dev_probe                               (+   0xac) - sp = 0x7fffef3feef0, ip = 0x478fd2c

[bt]: ( 9) rte_uuid_unparse                              (+  0x274) - sp = 0x7fffef3fef30, ip = 0x47a3e94

[bt]: (10) rte_eal_vfio_get_vf_token                     (+   0xd7) - sp = 0x7fffef3ff110, ip = 0x47b04b7

[bt]: (11) eal_hugepage_info_read                        (+  0x602) - sp = 0x7fffef3ff170, ip = 0x47b2cd2

[bt]: (12) start_thread                                  (+   0xc5) - sp = 0x7fffef3ff220, ip = 0x7ffff7bc6ea5

[bt]: (13) clone                                         (+   0x6d) - sp = 0x7fffef3ff2c0, ip = 0x7ffff004096d

EAL: Fail to recv reply for request /var/run/dpdk/oracusbc/mp_socket:eal_dev_mp_request

EAL: Cannot send request to primary

EAL: Failed to send hotplug request to primary

net_failsafe: Failed to probe devargs net_tap_vsc0

EAL: Fail to recv reply for request /var/run/dpdk/oracusbc/mp_socket:eal_dev_mp_request

EAL: Cannot send request to primary

EAL: Failed to send hotplug request to primary

net_failsafe: Failed to probe devargs net_tap_vsc1

EAL: No legacy callbacks, legacy socket not created

EAL: Drop mp reply: eal_dev_mp_request



PRIMARY PROCESS

timestamp=1633598196

TCZ0.0.0 Cycle 152 (Build 1832)

signal 11 (Segmentation fault), address is 0x38 from 0x9d8fbe



[bt]: ( 1) _Z18snprintf_backtraceRPciiP9siginfo_tPv      (+   0xf4) - sp = 0x7fffecf41150, ip = 0x100dd44

[bt]: ( 2) _Z13crit_err_hdlriP9siginfo_tPv               (+  0x159) - sp = 0x7fffecf41c60, ip = 0x100e019

[bt]: ( 3) _ZN13SignalAdapter12handleSignalEiP9siginfo_tPv (+  0x104) - sp = 0x7fffecf41f40, ip = 0xff4894

[bt]: ( 4) _L_unlock_18                                  (+   0x2c) - sp = 0x7fffecf41fc0, ip = 0x7ffff61d9630

[bt]: ( 5) failsafe_eth_dev_close                        (+  0x65e) - sp = 0x7fffecf42c90, ip = 0x9d8fbe

[bt]: ( 6) rte_eth_link_get_nowait                       (+   0x6a) - sp = 0x7fffecf42cf0, ip = 0x62fa0a

[bt]: ( 7) _ZN11StatsThread9statsLoopEP10CustomObject      (+  0x33e) - sp = 0x7fffecf42d20, ip = 0xedea2e

[bt]: ( 8) _ZN11StatsThread9statsLoopEP10CustomObject      (+  0x8dc) - sp = 0x7fffecf42d90, ip = 0xedefcc

[bt]: ( 9) ThreadFunction                                (+   0xe6) - sp = 0x7fffecf42db0, ip = 0x7ffff6b477e6

[bt]: (10) start_thread                                  (+   0xc5) - sp = 0x7fffecf42de0, ip = 0x7ffff61d1ea5

[bt]: (11) clone                                         (+   0x6d) - sp = 0x7fffecf42e80, ip = 0x7ffff0a6b96d







DPDK 20.11.2

core mask is 00000000000000000000000000004000

DPDK Custom Process initialized with 2 ports

the min max TxQ is maxTxQueues 16

Using 1 RxQs for port 0 (# F-core=1)

Using 1 RxQs for port 3 (# F-core=1)

Core 14 (port=0, rxQ=0) kni_ring=(nil)

Core 14 (port=3, rxQ=0) kni_ring=(nil)

Core 14 txN = 0

Thread for core 14 using ring from usbc of 0x31117b29bb00

Ring size must be powers of 2, adjusting from 8196 to 16384

Thread for core 14 using ring from MEDIA of 0x31117b27b840

Encaps Memory Zone= 48044 sizeof encaps = 60

Trace Memory Zone= 272

Policy Memory Zone= 8196 sizeof policy = 240

link status for port 0 is 1

link status for port 3 is 1

PORT 0 supports 16 rx queues and 16 tx queues (driver_name = net_failsafe, driver_type = 16)

PORT 0 is polling for link-change, interrupts disabled

[DPDK] tap_flow_create(): Kernel refused TC filter rule creation (17): File exists

[DPDK] net_failsafe: Failed to create flow on sub_device 1

add_flow(): create() fails for port 0; Reason: overlapping rules or Kernel too old for flower support

Error adding broadcast flow

PORT 3 supports 16 rx queues and 16 tx queues (driver_name = net_failsafe, driver_type = 16)

PORT 3 is polling for link-change, interrupts disabled

[DPDK] EAL: Failed to hotplug add device on primary

[DPDK] tap_flow_create(): Kernel refused TC filter rule creation (17): File exists

[DPDK] net_failsafe: Failed to create flow on sub_device 1

add_flow(): create() fails for port 3; Reason: overlapping rules or Kernel too old for flower support

Error adding broadcast flow

Cmd Thread is available

Capture object initialized

init :Stats Thread is available

ifLinkUpdate: Sending OperStatus for port=0 stat=1

ifLinkUpdate: Port 0 Link Change - speed 40000 Mbps - full-duplex

[DPDK] EAL: Fail to recv reply for request /var/run/dpdk/oracusbc/mp_socket_2934_298e9db8d1:eal_dev_mp_request

[DPDK] EAL: rte_mp_request_sync failed

[DPDK] EAL: Failed to send hotplug request to secondary

[DPDK] EAL: Fail to recv reply for request /var/run/dpdk/oracusbc/mp_socket_2934_298e9db8d1:eal_dev_mp_request

[DPDK] EAL: rte_mp_request_sync failed

[DPDK] EAL: Failed to hotplug add device on primary

[DPDK] Invalid port_id=2

[DPDK] net_failsafe: Operation rte_eth_stats_get failed for sub_device 1 with error -19



There is some race at secondary process and primary got crashed because its data-structures and partially filled.

Let me know if you need GDB analysis, I can share with next reply if you are still unsatisfied. GDB analysis will be bigger.

Thanks!



Regards

Vipul



-----Original Message-----
From: Gaëtan Rivet <grive@u256.net>
Sent: Monday, November 22, 2021 3:53 PM
To: Vipul Ashri <vipul.ashri@oracle.com>; dev@dpdk.org
Cc: stable@dpdk.org
Subject: [External] : Re: [PATCH v2] net/failsafe: link_update request crashing at boot



On Thu, Oct 21, 2021, at 23:42, vipul.ashri@oracle.com<mailto:vipul.ashri@oracle.com> wrote:

> From: Vipul Ashri <vipul.ashri@oracle.com<mailto:vipul.ashri@oracle.com>>

>

> failsafe crashed while sending early link_update request during boot

> time initialization.

> Based on debugging we found failsafe device was good but sub- devices

> were progressing towards initialization and SUBOPS macro where

> expanding macro gives [partial_dev]->dev_ops->link_update()

> execution of which triggered crash because dev_ops==0. similar crash

> seen at failsafe_eth_dev_close()

>

> Failsafe driver need a separate check for subdevices similar to

> "RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);" which is called

> to almost every eth_dev function.

>

> Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD")

> Cc: stable@dpdk.org<mailto:stable@dpdk.org>

> Signed-off-by: Vipul Ashri <vipul.ashri@oracle.com<mailto:vipul.ashri@oracle.com>>



Hello Vipul,



I'm sorry for the delay, I missed your fix on the mailing list.



IIUC, the issue is that failsafe finished init and received an ethdev operation call, but one of its sub-device, although marked DEV_ACTIVE, has its eth_dev->dev_ops field NULL.



It is really surprising to me, because there aren't many ways for a sub-device to become DEV_ACTIVE.



The only two ways are



  * by executing 'fs_dev_configure()', which will first execute

    rte_eth_dev_configure() on the sub-device, and on error would

    stop *without* setting DEV_ACTIVE.

    rte_eth_dev_configure() will itself execute

    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV), so it would

    return negative errno and fs_dev_configure() would abort.



  * by executing 'fs_dev_remove()' and the sub-device was 'DEV_STARTED'

    to begin with, then it is retrograded to DEV_ACTIVE once stopped.



So I don't understand yet how it is possible for a sub-device to become DEV_ACTIVE while its eth_dev->dev_ops are NULL. It seems more like a bug, memory corruption or just an unexpected execution pattern.



Could describe in more detail the execution?

In particular, setting the EAL log-level to debug with the option:

' --log-level pmd.net.failsafe:debug '

for example while using testpmd or your DPDK app.

It should show ethdev level accesses to the sub-devices, and error values.



Best regards,

--

Gaetan Rivet

[-- Attachment #2: Type: text/html, Size: 20571 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-10-17 16:43 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 11:51 [dpdk-stable] [PATCH 1/1] net/failsafe: link_update request crashing at boot vipul.ashri
2021-10-21 21:42 ` [dpdk-stable] [PATCH v2] " vipul.ashri
2021-11-22  9:36   ` [dpdk-dev] " Ferruh Yigit
2021-11-22 10:23   ` Gaëtan Rivet
2022-02-14 14:47     ` vipul.ashri
2022-02-15 13:07     ` vipul.ashri
2022-02-15 16:24     ` Vipul Ashri
2022-02-15 16:46       ` Vipul Ashri
2023-10-17 16:43         ` Stephen Hemminger
2023-07-07  9:35       ` Ferruh Yigit
2022-02-14 13:44 ` vipul.ashri
2022-02-14 13:09 Vipul Ashri
2022-02-14 16:54 ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).