<div dir="auto"><div>Hi Ferruh<div dir="auto"><br></div><div dir="auto">You have to rely on user to call stop before calling close/remove.</div><div dir="auto">This is mandated in ethdev library, as implemented in:</div><div dir="auto">febc855b358e ("ethdev: forbid closing started device")<br></div><div dir="auto"><br></div>Yours, Junxiao<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 24, 2021, 06:02 Ferruh Yigit <<a href="mailto:ferruh.yigit@intel.com">ferruh.yigit@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 11/18/2021 5:33 PM, Junxiao Shi wrote:<br>
> Bugzilla ID: 888<br>
> Fixes: febc855b358e ("ethdev: forbid closing started device")<br>
> <br>
> Signed-off-by: Junxiao Shi <<a href="mailto:git@mail1.yoursunny.com" target="_blank" rel="noreferrer">git@mail1.yoursunny.com</a>><br>
<br>
Thanks Junxiao, +1 to this fix, cc'ed memif maintainer Jakub.<br>
<br>
> ---<br>
> drivers/net/memif/rte_eth_memif.c | 11 ++++++++---<br>
> 1 file changed, 8 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c<br>
> index 43d7378329..e3d523af57 100644<br>
> --- a/drivers/net/memif/rte_eth_memif.c<br>
> +++ b/drivers/net/memif/rte_eth_memif.c<br>
> @@ -1260,6 +1260,13 @@ memif_dev_start(struct rte_eth_dev *dev)<br>
> return ret;<br>
> }<br>
> <br>
> +static int<br>
> +memif_dev_stop(struct rte_eth_dev *dev)<br>
> +{<br>
> + memif_disconnect(dev);<br>
<br>
Is the 'memif_dev_stop()' safe to be called multiple times?<br>
If 'memif_dev_close()' calls 'memif_dev_stop()' (see below), it is possible<br>
to call 'memif_dev_stop()' multiple times, so it should be protected.<br>
<br>
> + return 0;<br>
> +}<br>
> +<br>
> static int<br>
> memif_dev_close(struct rte_eth_dev *dev)<br>
> {<br>
> @@ -1268,7 +1275,6 @@ memif_dev_close(struct rte_eth_dev *dev)<br>
> <br>
> if (rte_eal_process_type() == RTE_PROC_PRIMARY) {<br>
> memif_msg_enq_disconnect(pmd->cc, "Device closed", 0);<br>
> - memif_disconnect(dev);<br>
> <br>
> for (i = 0; i < dev->data->nb_rx_queues; i++)<br>
> (*dev->dev_ops->rx_queue_release)(dev, i);<br>
> @@ -1276,8 +1282,6 @@ memif_dev_close(struct rte_eth_dev *dev)<br>
> (*dev->dev_ops->tx_queue_release)(dev, i);<br>
> <br>
> memif_socket_remove_device(dev);<br>
> - } else {<br>
> - memif_disconnect(dev);<br>
<br>
Should we add 'memif_dev_stop()' within the close function?<br>
Otherwise we are relying on user to stop, but at least in remove path<br>
('rte_pmd_memif_remove()') that may not be the case.<br>
<br>
> }<br>
> <br>
> rte_free(dev->process_private);<br>
> @@ -1515,6 +1519,7 @@ memif_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t qid __rte_unused)<br>
> <br>
> static const struct eth_dev_ops ops = {<br>
> .dev_start = memif_dev_start,<br>
> + .dev_stop = memif_dev_stop,<br>
> .dev_close = memif_dev_close,<br>
> .dev_infos_get = memif_dev_info,<br>
> .dev_configure = memif_dev_configure,<br>
> <br>
<br>
</blockquote></div></div></div>