net/failsafe: Fix crash due to in-valid sub-device port id

Message ID 20221116121121.1969-1-madhuker.mythri@oracle.com (mailing list archive)
State New
Delegated to: Ferruh Yigit
Headers
Series net/failsafe: Fix crash due to in-valid sub-device port id |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Madhuker Mythri Nov. 16, 2022, 12:11 p.m. UTC
  From: Madhuker Mythri <madhuker.mythri@oracle.com>

Crash occurring while the DPDK secondary processes trying to probe the
tap-device, where tap-device is a sub-device of Fail-safe device.
Some-times we get in-valid sub-devices(with the in-valid port-id's),
due to which the IPC communication does not get response and causes the
communication failures b/w primary/secondary process.
So, need to validate the sub-device(tap) while secondary process probe in
the Fail-safe PMD, to avoid such issues.

Bugzilla Id: 1116

Signed-off-by: Madhuker Mythri <madhuker.mythri@oracle.com>
---
 drivers/net/failsafe/failsafe.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Ferruh Yigit Dec. 7, 2022, 5:21 p.m. UTC | #1
Unaddressed
On 11/16/2022 12:11 PM, madhuker.mythri@oracle.com wrote:
> From: Madhuker Mythri <madhuker.mythri@oracle.com>
> 
> Crash occurring while the DPDK secondary processes trying to probe the
> tap-device, where tap-device is a sub-device of Fail-safe device.
> Some-times we get in-valid sub-devices(with the in-valid port-id's),
> due to which the IPC communication does not get response and causes the
> communication failures b/w primary/secondary process.
> So, need to validate the sub-device(tap) while secondary process probe in
> the Fail-safe PMD, to avoid such issues.
> 
> Bugzilla Id: 1116
> 
> Signed-off-by: Madhuker Mythri <madhuker.mythri@oracle.com>
> ---
>  drivers/net/failsafe/failsafe.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> index 32811403b4..51d4440ac7 100644
> --- a/drivers/net/failsafe/failsafe.c
> +++ b/drivers/net/failsafe/failsafe.c
> @@ -361,6 +361,9 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
>  			if (sdev->devargs.name[0] == '\0')
>  				continue;
>  
> +			if (!rte_eth_dev_is_valid_port(PORT_ID(sdev)))
> +				continue;
> +


This is in the 'FOREACH_SUBDEV()' block, why an invalid subdevice
provided by the macro?

Instead of invalid port check, should we fix the macro?

Overall I am not clear why this defect occurs, bugzilla report also
don't have much detail.
Can you please provide more details why this defect happens?
  
Stephen Hemminger Oct. 17, 2023, 4:49 p.m. UTC | #2
On Wed, 7 Dec 2022 17:21:42 +0000
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> This is in the 'FOREACH_SUBDEV()' block, why an invalid subdevice
> provided by the macro?
> 
> Instead of invalid port check, should we fix the macro?
> 
> Overall I am not clear why this defect occurs, bugzilla report also
> don't have much detail.
> Can you please provide more details why this defect happens?

This looks like a duplicate of same problem Oracle was having when there was
a race during setup and secondary process failed.

https://patchwork.dpdk.org/project/dpdk/patch/20211021214215.1633-1-vipul.ashri@oracle.com/
  

Patch

diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 32811403b4..51d4440ac7 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -361,6 +361,9 @@  rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
 			if (sdev->devargs.name[0] == '\0')
 				continue;
 
+			if (!rte_eth_dev_is_valid_port(PORT_ID(sdev)))
+				continue;
+
 			/* rebuild devargs to be able to get the bus name. */
 			ret = rte_devargs_parse(&devargs,
 						sdev->devargs.name);