[v5,2/6] app/testpmd: allow detaching a port not closed
Checks
Commit Message
The testpmd application aim is for testing;
so order of operations should not be enforced.
There was a test to forbid detaching before closing a port.
However, it may interesting to test what happens in such case.
It is possible for a PMD to automatically close the port when detaching.
in order to avoid a crash, it is checked that the port must be stopped
before detaching (as for closing).
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
app/test-pmd/testpmd.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Comments
On 10/18/18 4:23 AM, Thomas Monjalon wrote:
> The testpmd application aim is for testing;
> so order of operations should not be enforced.
>
> There was a test to forbid detaching before closing a port.
> However, it may interesting to test what happens in such case.
> It is possible for a PMD to automatically close the port when detaching.
Yes. In the case of net/sfc it requires a patch to call sfc_dev_close()
from uninit. I think network PMD maintainers should be notified
to double-check drivers.
> in order to avoid a crash, it is checked that the port must be stopped
> before detaching (as for closing).
I thought that it is sufficient to stop traffic and the port may be stopped
automatically by PMD. Not sure about it, just would like to clarify my
previous notes.
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
[...]
Hi Thomas,
> -----Original Message-----
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> Sent: Thursday, October 18, 2018 8:45 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; ophirmu@mellanox.com; Iremonger, Bernard
> <bernard.iremonger@intel.com>; rahul.lakkireddy@chelsio.com
> Subject: Re: [PATCH v5 2/6] app/testpmd: allow detaching a port not closed
>
> On 10/18/18 4:23 AM, Thomas Monjalon wrote:
> > The testpmd application aim is for testing; so order of operations
> > should not be enforced.
> >
> > There was a test to forbid detaching before closing a port.
> > However, it may interesting to test what happens in such case.
> > It is possible for a PMD to automatically close the port when detaching.
>
> Yes. In the case of net/sfc it requires a patch to call sfc_dev_close() from uninit.
> I think network PMD maintainers should be notified to double-check drivers.
>
> > in order to avoid a crash, it is checked that the port must be stopped
> > before detaching (as for closing).
>
> I thought that it is sufficient to stop traffic and the port may be stopped
> automatically by PMD. Not sure about it, just would like to clarify my previous
> notes.
>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>
> [...]
This patch seems too risky as it may impact some of the PMD's.
Regards,
Bernard.
18/10/2018 12:51, Iremonger, Bernard:
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> > On 10/18/18 4:23 AM, Thomas Monjalon wrote:
> > > The testpmd application aim is for testing; so order of operations
> > > should not be enforced.
> > >
> > > There was a test to forbid detaching before closing a port.
> > > However, it may interesting to test what happens in such case.
> > > It is possible for a PMD to automatically close the port when detaching.
> >
> > Yes. In the case of net/sfc it requires a patch to call sfc_dev_close() from uninit.
> > I think network PMD maintainers should be notified to double-check drivers.
> >
> > > in order to avoid a crash, it is checked that the port must be stopped
> > > before detaching (as for closing).
> >
> > I thought that it is sufficient to stop traffic and the port may be stopped
> > automatically by PMD. Not sure about it, just would like to clarify my previous
> > notes.
> >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> >
> > [...]
>
> This patch seems too risky as it may impact some of the PMD's.
Yes, it will not work for all PMDs.
If we want to allow this scenario, we'll need to improve some PMDs.
This patch is just allowing to test the scenario.
It will help PMD developers, so I think it is more helpful than risky.
@@ -2343,13 +2343,15 @@ detach_port(portid_t port_id)
printf("Detaching a port...\n");
if (!port_is_closed(port_id)) {
- printf("Please close port first\n");
- return;
+ if (ports[port_id].port_status != RTE_PORT_STOPPED) {
+ printf("Port not stopped\n");
+ return;
+ }
+ printf("Port was not closed\n");
+ if (ports[port_id].flow_list)
+ port_flow_flush(port_id);
}
- if (ports[port_id].flow_list)
- port_flow_flush(port_id);
-
if (rte_eth_dev_detach(port_id, name)) {
TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
return;