net/failsafe: Fix crash due to in-valid sub-device port id
Checks
Commit Message
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
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?
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/
On Wed, 7 Dec 2022 17:21:42 +0000
Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> 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.
Fix the underlying communication issue. If there is no response then
the code is continuing on in an invalid state. Lots more will likely
break.
> > 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?
>
I am going to reject this patch.
@@ -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);