On 25/02/16 07:22, Elo, Matias (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Zoltan Kiss [mailto:[email protected]]
Sent: Wednesday, February 24, 2016 9:10 PM
To: Elo, Matias (Nokia - FI/Espoo) <[email protected]>; lng-
[email protected]
Subject: Re: [lng-odp] [API-NEXT PATCH v4 13/13] linux-generic: dpdk: close
resources in odp_pktio_close()
On 24/02/16 07:36, Elo, Matias (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Zoltan Kiss [mailto:[email protected]]
Sent: Friday, February 19, 2016 6:15 PM
To: Elo, Matias (Nokia - FI/Espoo) <[email protected]>; lng-
[email protected]
Subject: Re: [lng-odp] [API-NEXT PATCH v4 13/13] linux-generic: dpdk: close
resources in odp_pktio_close()
pktio_entry already has a 'state' field, why not using that?
Sorry for the slow reply. The state field in pktio_entry is modified outside
dpdk
pktio code, so it cannot be used to track if rte_eth_dev_start() has been
actually
called. If rte_eth_dev_start() has not been called, rte_eth_dev_close() cannot
be used in dpdk_close().
"state" is set to STATE_START only if rte_eth_dev_start() succeeded (and
everything else), and the stop function will call rte_eth_dev_close()
only if "state" is STATE_START. I think it's good enough for tracking
It is not the stop() function, which is calling rte_eth_dev_close(). It is close(). And
because close() can be called only on a stopped interface the "state" variable
cannot be used to check if the interface has been started.
I see. It seems DPDK has a different startup process, but I think it's
definitely a bug for them that close() crashes when you call it on an
interface never been started. Either they should document that the
application should track that, or even better, check "struct
rte_eth_dev" for that (dev->data->dev_started == 1) before closing.
Anyhow, we can work that around by checking ourselves, we don't need an
extra variable for that.
-Matias
-Matias
On 15/02/16 10:50, Matias Elo wrote:
Free/close open resources in odp_pktio_close().
Reviewed-by: Petri Savolainen <[email protected]>
Signed-off-by: Matias Elo <[email protected]>
---
platform/linux-generic/include/odp_packet_dpdk.h | 1 +
platform/linux-generic/pktio/dpdk.c | 19 ++++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/platform/linux-generic/include/odp_packet_dpdk.h
b/platform/linux-generic/include/odp_packet_dpdk.h
index 9c8f7be..a085582 100644
--- a/platform/linux-generic/include/odp_packet_dpdk.h
+++ b/platform/linux-generic/include/odp_packet_dpdk.h
@@ -54,6 +54,7 @@ typedef struct {
uint16_t mtu; /**< maximum transmission unit */
/** DPDK packet pool name (pktpool_<ifname>) */
char pool_name[IF_NAMESIZE + 8];
+ odp_bool_t started; /**< DPDK device has been started */
uint8_t port_id; /**< DPDK port identifier */
unsigned min_rx_burst; /**< minimum RX burst size */
odp_pktin_hash_proto_t hash; /**< Packet input hash protocol
*/
diff --git a/platform/linux-generic/pktio/dpdk.c b/platform/linux-
generic/pktio/dpdk.c
index 1082c59..c81bb43 100644
--- a/platform/linux-generic/pktio/dpdk.c
+++ b/platform/linux-generic/pktio/dpdk.c
@@ -217,8 +217,23 @@ static int dpdk_setup_port(pktio_entry_t
*pktio_entry)
return 0;
}
-static int dpdk_close(pktio_entry_t *pktio_entry ODP_UNUSED)
+static int dpdk_close(pktio_entry_t *pktio_entry)
{
+ pkt_dpdk_t *pkt_dpdk = &pktio_entry->s.pkt_dpdk;
+ unsigned idx;
+ unsigned i, j;
+
+ /* Free cache packets */
+ for (i = 0; i < PKTIO_MAX_QUEUES; i++) {
+ idx = pkt_dpdk->rx_cache[i].s.idx;
+
+ for (j = 0; j < pkt_dpdk->rx_cache[i].s.count; j++)
+ rte_pktmbuf_free(pkt_dpdk->rx_cache[i].s.pkt[idx++]);
+ }
+
+ if (pkt_dpdk->started)
+ rte_eth_dev_close(pkt_dpdk->port_id);
+
return 0;
}
@@ -515,6 +530,8 @@ static int dpdk_start(pktio_entry_t *pktio_entry)
ret, port_id);
return -1;
}
+ pkt_dpdk->started = 1;
+
return 0;
}
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
