[dpdk-dev] [PATCH] app/proc-info: fix security context info
We need to differentiate between crypto and ethernet security
context as they belong to different devices.
Fixes: d82d6ac64338 ("app/procinfo: add crypto security context info")
Cc: sta...@dpdk.org
Signed-off-by: Hemant Agrawal
---
app/proc-info/main.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index d743209f0d..6486a2419e 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -648,11 +648,16 @@ metrics_display(int port_id)
}
static void
-show_security_context(uint16_t portid)
+show_security_context(uint16_t portid, uint8_t inline_offload)
{
- void *p_ctx = rte_eth_dev_get_sec_ctx(portid);
+ void *p_ctx;
const struct rte_security_capability *s_cap;
+ if (inline_offload)
+ p_ctx = rte_eth_dev_get_sec_ctx(portid);
+ else
+ p_ctx = rte_cryptodev_get_sec_ctx(portid);
+
if (p_ctx == NULL)
return;
@@ -859,7 +864,7 @@ show_port(void)
}
#ifdef RTE_LIB_SECURITY
- show_security_context(i);
+ show_security_context(i, 1);
#endif
}
}
@@ -1224,7 +1229,7 @@ show_crypto(void)
}
#ifdef RTE_LIB_SECURITY
- show_security_context(i);
+ show_security_context(i, 0);
#endif
}
}
--
2.17.1
[dpdk-dev] [PATCH v3 0/2] examples/vhost: sample code refactor
Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replase rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path. --- v3: * added some variable initiation * cleaned some codes v2: * optimized patch structure * optimized git log * replased rte_atomicNN_xxx to atomic_XXX Cheng Jiang (2): examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path examples/vhost/ioat.c | 15 ++-- examples/vhost/main.c | 168 ++ examples/vhost/main.h | 7 +- 3 files changed, 130 insertions(+), 60 deletions(-) -- 2.29.2
[dpdk-dev] [PATCH v3 1/2] examples/vhost: add ioat ring space count and check
Add ioat ring space count and check, if ioat ring space is not enough
for the next async vhost packet enqueue, then just return to prevent
enqueue failure.
Signed-off-by: Cheng Jiang
---
examples/vhost/ioat.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index 71d8a1f1f..b0b04aa45 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -17,6 +17,7 @@ struct packet_tracker {
unsigned short next_read;
unsigned short next_write;
unsigned short last_remain;
+ unsigned short ioat_space;
};
struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
goto out;
}
rte_rawdev_start(dev_id);
-
+ cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
dma_info->nr++;
i++;
}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
src = descs[i_desc].src;
dst = descs[i_desc].dst;
i_seg = 0;
+ if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+ break;
while (i_seg < src->nr_segs) {
- /*
-* TODO: Assuming that the ring space of the
-* IOAT device is large enough, so there is no
-* error here, and the actual error handling
-* will be added later.
-*/
rte_ioat_enqueue_copy(dev_id,
(uintptr_t)(src->iov[i_seg].iov_base)
+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
i_seg++;
}
write &= mask;
- cb_tracker[dev_id].size_track[write] = i_seg;
+ cb_tracker[dev_id].size_track[write] = src->nr_segs;
+ cb_tracker[dev_id].ioat_space -= src->nr_segs;
write++;
}
} else {
@@ -186,6 +184,7 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
int dev_id = dma_bind[vid].dmas[queue_id * 2
+ VIRTIO_RXQ].dev_id;
n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
+ cb_tracker[dev_id].ioat_space += n_seg;
n_seg += cb_tracker[dev_id].last_remain;
if (!n_seg)
return 0;
--
2.29.2
[dpdk-dev] [PATCH v3 2/2] examples/vhost: refactor vhost data path
Change the vm2vm data path to batch enqueue for better performance.
Support latest async vhost API, refactor vhost async data path,
replase rte_atomicNN_xxx to atomic_XXX and clean some codes.
Signed-off-by: Cheng Jiang
---
examples/vhost/main.c | 168 ++
examples/vhost/main.h | 7 +-
2 files changed, 123 insertions(+), 52 deletions(-)
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 8d8c3038b..efc044b28 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -179,9 +179,18 @@ struct mbuf_table {
struct rte_mbuf *m_table[MAX_PKT_BURST];
};
+struct vhost_bufftable {
+ uint32_t len;
+ uint64_t pre_tsc;
+ struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
/* TX queue for each data core. */
struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
+/* TX queue for each vhost device. */
+struct vhost_bufftable vhost_bufftable[RTE_MAX_LCORE * MAX_VHOST_DEVICE];
+
#define MBUF_TABLE_DRAIN_TSC ((rte_get_tsc_hz() + US_PER_S - 1) \
/ US_PER_S * BURST_TX_DRAIN_US)
#define VLAN_HLEN 4
@@ -804,39 +813,84 @@ unlink_vmdq(struct vhost_dev *vdev)
}
}
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+ while (n--)
+ rte_pktmbuf_free(pkts[n]);
+}
+
static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+ struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+ uint16_t complete_count;
+
+ complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+ VIRTIO_RXQ, p_cpl, MAX_PKT_BURST);
+ if (complete_count) {
+ atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+ free_pkts(p_cpl, complete_count);
+ }
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
struct rte_mbuf *m)
{
uint16_t ret;
- struct rte_mbuf *m_cpl[1];
if (builtin_net_driver) {
ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
- } else if (async_vhost_driver) {
- ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
- &m, 1);
-
- if (likely(ret))
- dst_vdev->nr_async_pkts++;
-
- while (likely(dst_vdev->nr_async_pkts)) {
- if (rte_vhost_poll_enqueue_completed(dst_vdev->vid,
- VIRTIO_RXQ, m_cpl, 1))
- dst_vdev->nr_async_pkts--;
- }
} else {
ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
}
if (enable_stats) {
- rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
- rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+ atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+ atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
src_vdev->stats.tx_total++;
src_vdev->stats.tx += ret;
}
}
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+ uint16_t ret;
+ uint64_t queue_id = rte_lcore_id() * MAX_VHOST_DEVICE + vdev->vid;
+ uint16_t nr_xmit = vhost_bufftable[queue_id].len;
+ struct rte_mbuf **m = vhost_bufftable[queue_id].m_table;
+
+ if (builtin_net_driver) {
+ ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+ } else if (async_vhost_driver) {
+ uint32_t cpu_cpl_nr = 0;
+ uint16_t enqueue_fail = 0;
+ struct rte_mbuf *m_cpu_cpl[nr_xmit];
+ complete_async_pkts(vdev);
+ ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+ m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+ atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+ if (cpu_cpl_nr)
+ free_pkts(m_cpu_cpl, cpu_cpl_nr);
+ enqueue_fail = nr_xmit - ret;
+ if (enqueue_fail)
+ free_pkts(&m[ret], nr_xmit - ret);
+ } else {
+ ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+ m, nr_xmit);
+ }
+
+ if (enable_stats) {
+ atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+ atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+ }
+
+ if (!async_vhost_driver)
+ free_pkts(m, nr_xmit);
+}
+
/*
* Check if the packet destination MAC address is for a local device. If so
then put
* the packet on that devices RX queue. If not then return.
@@ -846,7 +900,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
{
struct rte_ether_hdr *pkt_hdr;
Re: [dpdk-dev] [PATCH] net/iavf: fix queue pairs configuration
Tested-by: Huang, ZhiminX
Regards,
HuangZhiMin
> -Original Message-
> From: dev On Behalf Of Zhang,Alvin
> Sent: Wednesday, December 23, 2020 1:30 PM
> To: Xing, Beilei ; Xu, Ting
> Cc: dev@dpdk.org; Zhang, AlvinX ; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/iavf: fix queue pairs configuration
>
> From: Alvin Zhang
>
> Check if there are enough queue pairs currently allocated, and if not, request
> PF to allocate them.
>
> Fixes: e436cd43835b ("net/iavf: negotiate large VF and request more queues")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Alvin Zhang
> ---
> drivers/net/iavf/iavf_ethdev.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index
> 7e3c26a..f015121 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -372,8 +372,10 @@ struct rte_iavf_xstats_name_off {
> } else {
> /* Check if large VF is already enabled. If so, disable and
>* release redundant queue resource.
> + * Or check if enough queue pairs. If not, request them from PF.
>*/
> - if (vf->lv_enabled) {
> + if (vf->lv_enabled ||
> + num_queue_pairs > vf->vsi_res->num_queue_pairs) {
> ret = iavf_queues_req_reset(dev, num_queue_pairs);
> if (ret)
> return ret;
> --
> 1.8.3.1
[dpdk-dev] [PATCH 0/3] bnxt patches
Some fixes and enchancements in the core bnxt PMD Somnath Kotur (3): net/bnxt: fix to init/destroy locks only once net/bnxt: fix error path handling of dev start op net/bnxt: check for chip reset in dev stop/close ops drivers/net/bnxt/bnxt.h| 5 + drivers/net/bnxt/bnxt_cpr.c| 2 + drivers/net/bnxt/bnxt_ethdev.c | 225 +++-- 3 files changed, 135 insertions(+), 97 deletions(-) -- 2.28.0.497.g54e85e7
[dpdk-dev] [PATCH 1/3] net/bnxt: fix to init/destroy locks only once
Invoking init/uninit locks in init_resources and uninit_resources
would end up initializing and destroying locks on every port start
stop which is not desired.
Move the 2 routines to dev_init and dev_close respectively as
locks need to be initialized and destroyed only once during the
lifetime of the driver.
Fixes: 1cb3d39a48f7 ("net/bnxt: synchronize between flow related functions")
Cc: sta...@dpdk.org
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_ethdev.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 0788d263d8..40e15c69d3 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1419,6 +1419,18 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+static void
+bnxt_uninit_locks(struct bnxt *bp)
+{
+ pthread_mutex_destroy(&bp->flow_lock);
+ pthread_mutex_destroy(&bp->def_cp_lock);
+ pthread_mutex_destroy(&bp->health_check_lock);
+ if (bp->rep_info) {
+ pthread_mutex_destroy(&bp->rep_info->vfr_lock);
+ pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
+ }
+}
+
static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1444,6 +1456,7 @@ static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
bnxt_free_link_info(bp);
bnxt_free_pf_info(bp);
bnxt_free_parent_info(bp);
+ bnxt_uninit_locks(bp);
rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
bp->tx_mem_zone = NULL;
@@ -4790,10 +4803,6 @@ static int bnxt_init_resources(struct bnxt *bp, bool
reconfig_dev)
return rc;
}
- rc = bnxt_init_locks(bp);
- if (rc)
- return rc;
-
return 0;
}
@@ -5280,6 +5289,10 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params
__rte_unused)
if (rc)
goto error_free;
+ rc = bnxt_init_locks(bp);
+ if (rc)
+ return rc;
+
rc = bnxt_init_resources(bp, false);
if (rc)
goto error_free;
@@ -5371,18 +5384,6 @@ bnxt_free_error_recovery_info(struct bnxt *bp)
bp->fw_cap &= ~BNXT_FW_CAP_ERROR_RECOVERY;
}
-static void
-bnxt_uninit_locks(struct bnxt *bp)
-{
- pthread_mutex_destroy(&bp->flow_lock);
- pthread_mutex_destroy(&bp->def_cp_lock);
- pthread_mutex_destroy(&bp->health_check_lock);
- if (bp->rep_info) {
- pthread_mutex_destroy(&bp->rep_info->vfr_lock);
- pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
- }
-}
-
static int
bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
{
@@ -5404,7 +5405,6 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
bnxt_uninit_ctx_mem(bp);
- bnxt_uninit_locks(bp);
bnxt_free_flow_stats_info(bp);
bnxt_free_rep_info(bp);
rte_free(bp->ptp_cfg);
--
2.28.0.497.g54e85e7
[dpdk-dev] [PATCH 3/3] net/bnxt: check for chip reset in dev stop/close ops
While the error recovery thread is running, an application
can invoke dev_stop or dev_close_op thus triggering a race
and unwanted consequences if dev_close is invoked while the
recovery is not yet completed.
Fix by having another lock to synchronize between the 2 threads and
return EGAIN if adapter is in the middle of recovery when dev_stop
or dev_close ops are invoked
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt.h| 5
drivers/net/bnxt/bnxt_cpr.c| 2 ++
drivers/net/bnxt/bnxt_ethdev.c | 49 +-
3 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 8374e9fadc..7c135370f0 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -719,6 +719,11 @@ struct bnxt {
* health_check_lock
*/
pthread_mutex_t health_check_lock;
+ /* synchronize between dev_stop/dev_close_op and
+* error recovery thread triggered as part of
+* HWRM_ASYNC_EVENT_CMPL_EVENT_ID_RESET_NOTIFY
+*/
+ pthread_mutex_t err_recovery_lock;
uint16_tmax_req_len;
uint16_tmax_resp_len;
uint16_thwrm_max_ext_req_len;
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index ee96ae81bf..6e172a9eea 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -133,6 +133,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
return;
}
+ pthread_mutex_lock(&bp->err_recovery_lock);
event_data = rte_le_to_cpu_32(async_cmp->event_data1);
/* timestamp_lo/hi values are in units of 100ms */
bp->fw_reset_max_msecs = async_cmp->timestamp_hi ?
@@ -152,6 +153,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
}
bp->flags |= BNXT_FLAG_FW_RESET;
+ pthread_mutex_unlock(&bp->err_recovery_lock);
rte_eal_alarm_set(US_PER_MS, bnxt_dev_reset_and_resume,
(void *)bp);
break;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 17dc49628d..0bd0762859 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1276,8 +1276,7 @@ static void bnxt_free_switch_domain(struct bnxt *bp)
}
}
-/* Unload the driver, release resources */
-static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
+static int bnxt_dev_stop(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
@@ -1345,6 +1344,22 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+/* Unload the driver, release resources */
+static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
+{
+ struct bnxt *bp = eth_dev->data->dev_private;
+
+ pthread_mutex_lock(&bp->err_recovery_lock);
+ if (bp->flags & BNXT_FLAG_FW_RESET) {
+ PMD_DRV_LOG(ERR,
+ "Adapter recovering from error..Please retry\n");
+ return -EAGAIN;
+ }
+ pthread_mutex_unlock(&bp->err_recovery_lock);
+
+ return bnxt_dev_stop(eth_dev);
+}
+
static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1411,7 +1426,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
return 0;
error:
- bnxt_dev_stop_op(eth_dev);
+ bnxt_dev_stop(eth_dev);
return rc;
}
@@ -1421,6 +1436,7 @@ bnxt_uninit_locks(struct bnxt *bp)
pthread_mutex_destroy(&bp->flow_lock);
pthread_mutex_destroy(&bp->def_cp_lock);
pthread_mutex_destroy(&bp->health_check_lock);
+ pthread_mutex_destroy(&bp->err_recovery_lock);
if (bp->rep_info) {
pthread_mutex_destroy(&bp->rep_info->vfr_lock);
pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
@@ -1435,13 +1451,21 @@ static int bnxt_dev_close_op(struct rte_eth_dev
*eth_dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
+ pthread_mutex_lock(&bp->err_recovery_lock);
+ if (bp->flags & BNXT_FLAG_FW_RESET) {
+ PMD_DRV_LOG(ERR,
+ "Adapter recovering from error...Please retry\n");
+ return -EAGAIN;
+ }
+ pthread_mutex_unlock(&bp->err_recovery_lock);
+
/* cancel the recovery handler before remove dev */
rte_eal_alarm_cancel(bnxt_dev_reset_and_resume, (void *)bp);
rte_eal_alarm_cancel(bnxt_dev_recover, (void *)bp);
bnxt_cancel_fc_thread(bp);
if (eth_dev->data->dev_started)
- ret = bnxt_dev_stop_op(eth_dev);
+ ret = bnxt_dev_stop(eth_dev);
bnxt_free_switch_domain(bp
[dpdk-dev] [PATCH 2/3] net/bnxt: fix error path handling of dev start op
Call bnxt_dev_stop in error path of bnxt_dev_start_op() to keep
it simple and consistent
Fixes: c09f57b49c13 ("net/bnxt: add start/stop/link update operations")
Cc: sta...@dpdk.org
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_ethdev.c | 144 -
1 file changed, 70 insertions(+), 74 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 40e15c69d3..17dc49628d 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1239,80 +1239,6 @@ static int bnxt_handle_if_change_status(struct bnxt *bp)
return rc;
}
-static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
-{
- struct bnxt *bp = eth_dev->data->dev_private;
- uint64_t rx_offloads = eth_dev->data->dev_conf.rxmode.offloads;
- int vlan_mask = 0;
- int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
-
- if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
- PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
- return -EINVAL;
- }
-
- if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
- PMD_DRV_LOG(ERR,
- "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
- bp->rx_cp_nr_rings, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-
- do {
- rc = bnxt_hwrm_if_change(bp, true);
- if (rc == 0 || rc != -EAGAIN)
- break;
-
- rte_delay_ms(BNXT_IF_CHANGE_RETRY_INTERVAL);
- } while (retry_cnt--);
-
- if (rc)
- return rc;
-
- if (bp->flags & BNXT_FLAG_IF_CHANGE_HOT_FW_RESET_DONE) {
- rc = bnxt_handle_if_change_status(bp);
- if (rc)
- return rc;
- }
-
- bnxt_enable_int(bp);
-
- rc = bnxt_init_chip(bp);
- if (rc)
- goto error;
-
- eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
- eth_dev->data->dev_started = 1;
-
- bnxt_link_update_op(eth_dev, 1);
-
- if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
- vlan_mask |= ETH_VLAN_FILTER_MASK;
- if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
- vlan_mask |= ETH_VLAN_STRIP_MASK;
- rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
- if (rc)
- goto error;
-
- /* Initialize bnxt ULP port details */
- rc = bnxt_ulp_port_init(bp);
- if (rc)
- goto error;
-
- eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev);
- eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev);
-
- bnxt_schedule_fw_health_check(bp);
-
- return 0;
-
-error:
- bnxt_shutdown_nic(bp);
- bnxt_free_tx_mbufs(bp);
- bnxt_free_rx_mbufs(bp);
- bnxt_hwrm_if_change(bp, false);
- eth_dev->data->dev_started = 0;
- return rc;
-}
-
static int bnxt_dev_set_link_up_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1419,6 +1345,76 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
+{
+ struct bnxt *bp = eth_dev->data->dev_private;
+ uint64_t rx_offloads = eth_dev->data->dev_conf.rxmode.offloads;
+ int vlan_mask = 0;
+ int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
+
+ if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
+ PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
+ return -EINVAL;
+ }
+
+ if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
+ PMD_DRV_LOG(ERR,
+ "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
+ bp->rx_cp_nr_rings, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+
+ do {
+ rc = bnxt_hwrm_if_change(bp, true);
+ if (rc == 0 || rc != -EAGAIN)
+ break;
+
+ rte_delay_ms(BNXT_IF_CHANGE_RETRY_INTERVAL);
+ } while (retry_cnt--);
+
+ if (rc)
+ return rc;
+
+ if (bp->flags & BNXT_FLAG_IF_CHANGE_HOT_FW_RESET_DONE) {
+ rc = bnxt_handle_if_change_status(bp);
+ if (rc)
+ return rc;
+ }
+
+ bnxt_enable_int(bp);
+
+ rc = bnxt_init_chip(bp);
+ if (rc)
+ goto error;
+
+ eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
+ eth_dev->data->dev_started = 1;
+
+ bnxt_link_update_op(eth_dev, 1);
+
+ if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
+ vlan_mask |= ETH_VLAN_FILTER_MASK;
+ if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+ vlan_mask |= ETH_VLAN_STRIP_MASK;
+ rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
+ if (rc)
+ goto error;
+
+ /* Initialize bnxt ULP port details */
+ rc = bnxt_ulp_port_init(bp);
[dpdk-dev] [PATCH v3] app/testpmd: increase array for fetching supported FEC caps
From: Karra Satwik
Request the driver for number of entries in the FEC caps
array and then dynamically allocate the array.
Signed-off-by: Karra Satwik
Signed-off-by: Rahul Lakkireddy
Acked-by: Xiaoyun Li
---
v3:
- Use unsigned int num, instead of int num
v2:
- Replace if (!speed_fec_capa) with if (speed_fec_capa == NULL)
app/test-pmd/cmdline.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2ccbaa039..1dfad5df4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -16263,11 +16263,9 @@ cmd_show_fec_capability_parsed(void *parsed_result,
__rte_unused struct cmdline *cl,
__rte_unused void *data)
{
-#define FEC_CAP_NUM 2
struct cmd_show_fec_capability_result *res = parsed_result;
- struct rte_eth_fec_capa speed_fec_capa[FEC_CAP_NUM];
- unsigned int num = FEC_CAP_NUM;
- unsigned int ret_num;
+ struct rte_eth_fec_capa *speed_fec_capa;
+ unsigned int num;
int ret;
if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
@@ -16275,17 +16273,31 @@ cmd_show_fec_capability_parsed(void *parsed_result,
return;
}
- ret = rte_eth_fec_get_capability(res->cmd_pid, speed_fec_capa, num);
+ ret = rte_eth_fec_get_capability(res->cmd_pid, NULL, 0);
if (ret == -ENOTSUP) {
printf("Function not implemented\n");
return;
} else if (ret < 0) {
- printf("Get FEC capability failed\n");
+ printf("Get FEC capability failed: %d\n", ret);
+ return;
+ }
+
+ num = (unsigned int)ret;
+ speed_fec_capa = calloc(num, sizeof(*speed_fec_capa));
+ if (speed_fec_capa == NULL) {
+ printf("Failed to alloc FEC capability buffer\n");
return;
}
- ret_num = (unsigned int)ret;
- show_fec_capability(ret_num, speed_fec_capa);
+ ret = rte_eth_fec_get_capability(res->cmd_pid, speed_fec_capa, num);
+ if (ret < 0) {
+ printf("Error getting FEC capability: %d\n", ret);
+ goto out;
+ }
+
+ show_fec_capability(num, speed_fec_capa);
+out:
+ free(speed_fec_capa);
}
cmdline_parse_token_string_t cmd_show_fec_capability_show =
--
2.24.0
Re: [dpdk-dev] [PATCH 0/5] add apistats function
snipped > > Thanks for your comments. > I know you kindly provided many valuable comments though I reply the > following first because I think it is important that my idea/proposal is > acceptable or not first. > > > Sharing an alternate approach, if RX-TX callbacks are enabled in DPDK (which > is by default). One can register a callback handler to update counters with > the > following information as `port-queue pair, lcoreid, total rx burst request, > total > empty rx burst, 1-8 pks, 9-16 pkts, 16-32 pkts`. Callback handlers can be > selectively enabled or disabled too. > > > > Can you please help me understand how `rte_apistats` would be different or > pros of having it as library (that needs to be added to linking and running in > case of using DPDK applications)? > You are correct. > Application can do that by using callback of rx_burst/tx_burst. > But needs to modify/add the code for RX/TX process logic. No the RX or TX application logic is not modified as the call back registration is done once right after port_init. > > Point of my patchset is couting is done by library so that APP only needs to > "retrieve/read" those data if needed (especially for existing applications). As mentioned in the other patchset the library function is enabled through and not conditionally built. Any application which is built with this patch set will be forced to invoke the calls. > > I think it makes some developers happy becaseu it is relatively easy to > measure > "how cpu core is bysy" easily. Not sure what do you mean, as there 2 factors which conflicts 1. there are existing uses cases and examples like power, metric, telemetry all uses RX/TX callbacks which does the same. 2. The current logic only helps in RX/TX cores and not other cores. So in case of pipeline model there are only a couple of RX or TX threads. > (I admit relatively rough measurement though. I think it is trade-off) > > What do you think? If I consider RX calls there are 3 main metrics 1. How many times RX is invoked. 2. How many times RX returns with `0` packets 3. How many times RX returns with `> 0` packets. With these values in the current logic you are trying to deduct actual CPU RX usage by `useful work = number of times `> 0` / total RX calls` As a end user I will always be happy to see telemetry data as `from time t1 to t2, 1. how many times per second on average RX calls were made. 2. how many times per second on average The calls returned packets in range of 1-4, 5-8, 9-16, 17 and more ` Current logic is not trying to address this problem. With my current understanding I am not convinced that one needs yet another library when the same can be done either with `RX/TX callback` > snipped
Re: [dpdk-dev] [dpdk-dev v2 0/2] add new UDP tunnel port for ecpri
> -Original Message- > From: Guo, Jia > Sent: Thursday, December 24, 2020 3:00 PM > To: Zhang, Qi Z ; Wu, Jingjing ; > Yang, Qiming ; Wang, Haiyue > > Cc: dev@dpdk.org; Guo, Jia > Subject: [dpdk-dev v2 0/2] add new UDP tunnel port for ecpri > > Add new UDP tunnel type and port params for ecpri configuration. > > v2: > separate patch set > > Jeff Guo (2): > ethdev: add new tunnel type for ecpri > app/testpmd: add new UDP tunnel port for ecpri > > app/test-pmd/cmdline.c | 7 +-- > lib/librte_ethdev/rte_ethdev.h | 1 + > 2 files changed, 6 insertions(+), 2 deletions(-) > > -- > 2.20.1 Reviewed-by: Qi Zhang
[dpdk-dev] [PATCH v2 0/3] bnxt patches
A couple of bnxt PMD fixes and an enhancement Somnath Kotur (3): net/bnxt: fix to init/destroy locks only once net/bnxt: fix error path handling of dev start op net/bnxt: check for chip reset in dev stop/close ops drivers/net/bnxt/bnxt.h| 5 + drivers/net/bnxt/bnxt_cpr.c| 2 + drivers/net/bnxt/bnxt_ethdev.c | 225 +++-- 3 files changed, 135 insertions(+), 97 deletions(-) -- v1->v2: Modified patch 1/3 to return error 2.28.0.497.g54e85e7
[dpdk-dev] [PATCH 3/3] net/bnxt: check for chip reset in dev stop/close ops
While the error recovery thread is running, an application
can invoke dev_stop or dev_close_op thus triggering a race
and unwanted consequences if dev_close is invoked while the
recovery is not yet completed.
Fix by having another lock to synchronize between the 2 threads and
return EGAIN if adapter is in the middle of recovery when dev_stop
or dev_close ops are invoked
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt.h| 5
drivers/net/bnxt/bnxt_cpr.c| 2 ++
drivers/net/bnxt/bnxt_ethdev.c | 49 +-
3 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 8374e9fadc..7c135370f0 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -719,6 +719,11 @@ struct bnxt {
* health_check_lock
*/
pthread_mutex_t health_check_lock;
+ /* synchronize between dev_stop/dev_close_op and
+* error recovery thread triggered as part of
+* HWRM_ASYNC_EVENT_CMPL_EVENT_ID_RESET_NOTIFY
+*/
+ pthread_mutex_t err_recovery_lock;
uint16_tmax_req_len;
uint16_tmax_resp_len;
uint16_thwrm_max_ext_req_len;
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index ee96ae81bf..6e172a9eea 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -133,6 +133,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
return;
}
+ pthread_mutex_lock(&bp->err_recovery_lock);
event_data = rte_le_to_cpu_32(async_cmp->event_data1);
/* timestamp_lo/hi values are in units of 100ms */
bp->fw_reset_max_msecs = async_cmp->timestamp_hi ?
@@ -152,6 +153,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
}
bp->flags |= BNXT_FLAG_FW_RESET;
+ pthread_mutex_unlock(&bp->err_recovery_lock);
rte_eal_alarm_set(US_PER_MS, bnxt_dev_reset_and_resume,
(void *)bp);
break;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index b6a9db1b66..a6794a417d 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1276,8 +1276,7 @@ static void bnxt_free_switch_domain(struct bnxt *bp)
}
}
-/* Unload the driver, release resources */
-static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
+static int bnxt_dev_stop(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
@@ -1345,6 +1344,22 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+/* Unload the driver, release resources */
+static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
+{
+ struct bnxt *bp = eth_dev->data->dev_private;
+
+ pthread_mutex_lock(&bp->err_recovery_lock);
+ if (bp->flags & BNXT_FLAG_FW_RESET) {
+ PMD_DRV_LOG(ERR,
+ "Adapter recovering from error..Please retry\n");
+ return -EAGAIN;
+ }
+ pthread_mutex_unlock(&bp->err_recovery_lock);
+
+ return bnxt_dev_stop(eth_dev);
+}
+
static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1411,7 +1426,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
return 0;
error:
- bnxt_dev_stop_op(eth_dev);
+ bnxt_dev_stop(eth_dev);
return rc;
}
@@ -1421,6 +1436,7 @@ bnxt_uninit_locks(struct bnxt *bp)
pthread_mutex_destroy(&bp->flow_lock);
pthread_mutex_destroy(&bp->def_cp_lock);
pthread_mutex_destroy(&bp->health_check_lock);
+ pthread_mutex_destroy(&bp->err_recovery_lock);
if (bp->rep_info) {
pthread_mutex_destroy(&bp->rep_info->vfr_lock);
pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
@@ -1435,13 +1451,21 @@ static int bnxt_dev_close_op(struct rte_eth_dev
*eth_dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
+ pthread_mutex_lock(&bp->err_recovery_lock);
+ if (bp->flags & BNXT_FLAG_FW_RESET) {
+ PMD_DRV_LOG(ERR,
+ "Adapter recovering from error...Please retry\n");
+ return -EAGAIN;
+ }
+ pthread_mutex_unlock(&bp->err_recovery_lock);
+
/* cancel the recovery handler before remove dev */
rte_eal_alarm_cancel(bnxt_dev_reset_and_resume, (void *)bp);
rte_eal_alarm_cancel(bnxt_dev_recover, (void *)bp);
bnxt_cancel_fc_thread(bp);
if (eth_dev->data->dev_started)
- ret = bnxt_dev_stop_op(eth_dev);
+ ret = bnxt_dev_stop(eth_dev);
bnxt_free_switch_domain(bp
[dpdk-dev] [PATCH 1/3] net/bnxt: fix to init/destroy locks only once
Invoking init/uninit locks in init_resources and uninit_resources
would end up initializing and destroying locks on every port start
stop which is not desired.
Move the 2 routines to dev_init and dev_close respectively as
locks need to be initialized and destroyed only once during the
lifetime of the driver.
Fixes: 1cb3d39a48f7 ("net/bnxt: synchronize between flow related functions")
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_ethdev.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 0788d263d8..6c38155da6 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1419,6 +1419,18 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+static void
+bnxt_uninit_locks(struct bnxt *bp)
+{
+ pthread_mutex_destroy(&bp->flow_lock);
+ pthread_mutex_destroy(&bp->def_cp_lock);
+ pthread_mutex_destroy(&bp->health_check_lock);
+ if (bp->rep_info) {
+ pthread_mutex_destroy(&bp->rep_info->vfr_lock);
+ pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
+ }
+}
+
static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1444,6 +1456,7 @@ static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
bnxt_free_link_info(bp);
bnxt_free_pf_info(bp);
bnxt_free_parent_info(bp);
+ bnxt_uninit_locks(bp);
rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
bp->tx_mem_zone = NULL;
@@ -4790,10 +4803,6 @@ static int bnxt_init_resources(struct bnxt *bp, bool
reconfig_dev)
return rc;
}
- rc = bnxt_init_locks(bp);
- if (rc)
- return rc;
-
return 0;
}
@@ -5280,6 +5289,10 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params
__rte_unused)
if (rc)
goto error_free;
+ rc = bnxt_init_locks(bp);
+ if (rc)
+ goto error_free;
+
rc = bnxt_init_resources(bp, false);
if (rc)
goto error_free;
@@ -5371,18 +5384,6 @@ bnxt_free_error_recovery_info(struct bnxt *bp)
bp->fw_cap &= ~BNXT_FW_CAP_ERROR_RECOVERY;
}
-static void
-bnxt_uninit_locks(struct bnxt *bp)
-{
- pthread_mutex_destroy(&bp->flow_lock);
- pthread_mutex_destroy(&bp->def_cp_lock);
- pthread_mutex_destroy(&bp->health_check_lock);
- if (bp->rep_info) {
- pthread_mutex_destroy(&bp->rep_info->vfr_lock);
- pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
- }
-}
-
static int
bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
{
@@ -5404,7 +5405,6 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
bnxt_uninit_ctx_mem(bp);
- bnxt_uninit_locks(bp);
bnxt_free_flow_stats_info(bp);
bnxt_free_rep_info(bp);
rte_free(bp->ptp_cfg);
--
2.28.0.497.g54e85e7
[dpdk-dev] [PATCH 2/3] net/bnxt: fix error path handling of dev start op
Call bnxt_dev_stop in error path of bnxt_dev_start_op() to keep
it simple and consistent
Fixes: c09f57b49c13 ("net/bnxt: add start/stop/link update operations")
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_ethdev.c | 144 -
1 file changed, 70 insertions(+), 74 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6c38155da6..b6a9db1b66 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1239,80 +1239,6 @@ static int bnxt_handle_if_change_status(struct bnxt *bp)
return rc;
}
-static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
-{
- struct bnxt *bp = eth_dev->data->dev_private;
- uint64_t rx_offloads = eth_dev->data->dev_conf.rxmode.offloads;
- int vlan_mask = 0;
- int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
-
- if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
- PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
- return -EINVAL;
- }
-
- if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
- PMD_DRV_LOG(ERR,
- "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
- bp->rx_cp_nr_rings, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-
- do {
- rc = bnxt_hwrm_if_change(bp, true);
- if (rc == 0 || rc != -EAGAIN)
- break;
-
- rte_delay_ms(BNXT_IF_CHANGE_RETRY_INTERVAL);
- } while (retry_cnt--);
-
- if (rc)
- return rc;
-
- if (bp->flags & BNXT_FLAG_IF_CHANGE_HOT_FW_RESET_DONE) {
- rc = bnxt_handle_if_change_status(bp);
- if (rc)
- return rc;
- }
-
- bnxt_enable_int(bp);
-
- rc = bnxt_init_chip(bp);
- if (rc)
- goto error;
-
- eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
- eth_dev->data->dev_started = 1;
-
- bnxt_link_update_op(eth_dev, 1);
-
- if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
- vlan_mask |= ETH_VLAN_FILTER_MASK;
- if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
- vlan_mask |= ETH_VLAN_STRIP_MASK;
- rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
- if (rc)
- goto error;
-
- /* Initialize bnxt ULP port details */
- rc = bnxt_ulp_port_init(bp);
- if (rc)
- goto error;
-
- eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev);
- eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev);
-
- bnxt_schedule_fw_health_check(bp);
-
- return 0;
-
-error:
- bnxt_shutdown_nic(bp);
- bnxt_free_tx_mbufs(bp);
- bnxt_free_rx_mbufs(bp);
- bnxt_hwrm_if_change(bp, false);
- eth_dev->data->dev_started = 0;
- return rc;
-}
-
static int bnxt_dev_set_link_up_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1419,6 +1345,76 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
+{
+ struct bnxt *bp = eth_dev->data->dev_private;
+ uint64_t rx_offloads = eth_dev->data->dev_conf.rxmode.offloads;
+ int vlan_mask = 0;
+ int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
+
+ if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
+ PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
+ return -EINVAL;
+ }
+
+ if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
+ PMD_DRV_LOG(ERR,
+ "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
+ bp->rx_cp_nr_rings, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+
+ do {
+ rc = bnxt_hwrm_if_change(bp, true);
+ if (rc == 0 || rc != -EAGAIN)
+ break;
+
+ rte_delay_ms(BNXT_IF_CHANGE_RETRY_INTERVAL);
+ } while (retry_cnt--);
+
+ if (rc)
+ return rc;
+
+ if (bp->flags & BNXT_FLAG_IF_CHANGE_HOT_FW_RESET_DONE) {
+ rc = bnxt_handle_if_change_status(bp);
+ if (rc)
+ return rc;
+ }
+
+ bnxt_enable_int(bp);
+
+ rc = bnxt_init_chip(bp);
+ if (rc)
+ goto error;
+
+ eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
+ eth_dev->data->dev_started = 1;
+
+ bnxt_link_update_op(eth_dev, 1);
+
+ if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
+ vlan_mask |= ETH_VLAN_FILTER_MASK;
+ if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+ vlan_mask |= ETH_VLAN_STRIP_MASK;
+ rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
+ if (rc)
+ goto error;
+
+ /* Initialize bnxt ULP port details */
+ rc = bnxt_ulp_port_init(bp);
+ if (rc)
+
[dpdk-dev] [PATCH v2 0/3] bnxt patches
A couple of bnxt PMD fixes and an enhancement Somnath Kotur (3): net/bnxt: fix to init/destroy locks only once net/bnxt: fix error path handling of dev start op net/bnxt: check for chip reset in dev stop/close ops drivers/net/bnxt/bnxt.h| 5 + drivers/net/bnxt/bnxt_cpr.c| 2 + drivers/net/bnxt/bnxt_ethdev.c | 225 +++-- 3 files changed, 135 insertions(+), 97 deletions(-) -- v1->v2: Modified patch 1/3 to return error 2.28.0.497.g54e85e7
[dpdk-dev] [PATCH 3/3] net/bnxt: check for chip reset in dev stop/close ops
While the error recovery thread is running, an application
can invoke dev_stop or dev_close_op thus triggering a race
and unwanted consequences if dev_close is invoked while the
recovery is not yet completed.
Fix by having another lock to synchronize between the 2 threads and
return EGAIN if adapter is in the middle of recovery when dev_stop
or dev_close ops are invoked
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt.h| 5
drivers/net/bnxt/bnxt_cpr.c| 2 ++
drivers/net/bnxt/bnxt_ethdev.c | 49 +-
3 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 8374e9fadc..7c135370f0 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -719,6 +719,11 @@ struct bnxt {
* health_check_lock
*/
pthread_mutex_t health_check_lock;
+ /* synchronize between dev_stop/dev_close_op and
+* error recovery thread triggered as part of
+* HWRM_ASYNC_EVENT_CMPL_EVENT_ID_RESET_NOTIFY
+*/
+ pthread_mutex_t err_recovery_lock;
uint16_tmax_req_len;
uint16_tmax_resp_len;
uint16_thwrm_max_ext_req_len;
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index ee96ae81bf..6e172a9eea 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -133,6 +133,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
return;
}
+ pthread_mutex_lock(&bp->err_recovery_lock);
event_data = rte_le_to_cpu_32(async_cmp->event_data1);
/* timestamp_lo/hi values are in units of 100ms */
bp->fw_reset_max_msecs = async_cmp->timestamp_hi ?
@@ -152,6 +153,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
}
bp->flags |= BNXT_FLAG_FW_RESET;
+ pthread_mutex_unlock(&bp->err_recovery_lock);
rte_eal_alarm_set(US_PER_MS, bnxt_dev_reset_and_resume,
(void *)bp);
break;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index b6a9db1b66..a6794a417d 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1276,8 +1276,7 @@ static void bnxt_free_switch_domain(struct bnxt *bp)
}
}
-/* Unload the driver, release resources */
-static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
+static int bnxt_dev_stop(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
@@ -1345,6 +1344,22 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+/* Unload the driver, release resources */
+static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
+{
+ struct bnxt *bp = eth_dev->data->dev_private;
+
+ pthread_mutex_lock(&bp->err_recovery_lock);
+ if (bp->flags & BNXT_FLAG_FW_RESET) {
+ PMD_DRV_LOG(ERR,
+ "Adapter recovering from error..Please retry\n");
+ return -EAGAIN;
+ }
+ pthread_mutex_unlock(&bp->err_recovery_lock);
+
+ return bnxt_dev_stop(eth_dev);
+}
+
static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1411,7 +1426,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
return 0;
error:
- bnxt_dev_stop_op(eth_dev);
+ bnxt_dev_stop(eth_dev);
return rc;
}
@@ -1421,6 +1436,7 @@ bnxt_uninit_locks(struct bnxt *bp)
pthread_mutex_destroy(&bp->flow_lock);
pthread_mutex_destroy(&bp->def_cp_lock);
pthread_mutex_destroy(&bp->health_check_lock);
+ pthread_mutex_destroy(&bp->err_recovery_lock);
if (bp->rep_info) {
pthread_mutex_destroy(&bp->rep_info->vfr_lock);
pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
@@ -1435,13 +1451,21 @@ static int bnxt_dev_close_op(struct rte_eth_dev
*eth_dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
+ pthread_mutex_lock(&bp->err_recovery_lock);
+ if (bp->flags & BNXT_FLAG_FW_RESET) {
+ PMD_DRV_LOG(ERR,
+ "Adapter recovering from error...Please retry\n");
+ return -EAGAIN;
+ }
+ pthread_mutex_unlock(&bp->err_recovery_lock);
+
/* cancel the recovery handler before remove dev */
rte_eal_alarm_cancel(bnxt_dev_reset_and_resume, (void *)bp);
rte_eal_alarm_cancel(bnxt_dev_recover, (void *)bp);
bnxt_cancel_fc_thread(bp);
if (eth_dev->data->dev_started)
- ret = bnxt_dev_stop_op(eth_dev);
+ ret = bnxt_dev_stop(eth_dev);
bnxt_free_switch_domain(bp
[dpdk-dev] [PATCH 1/3] net/bnxt: fix to init/destroy locks only once
Invoking init/uninit locks in init_resources and uninit_resources
would end up initializing and destroying locks on every port start
stop which is not desired.
Move the 2 routines to dev_init and dev_close respectively as
locks need to be initialized and destroyed only once during the
lifetime of the driver.
Fixes: 1cb3d39a48f7 ("net/bnxt: synchronize between flow related functions")
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_ethdev.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 0788d263d8..6c38155da6 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1419,6 +1419,18 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+static void
+bnxt_uninit_locks(struct bnxt *bp)
+{
+ pthread_mutex_destroy(&bp->flow_lock);
+ pthread_mutex_destroy(&bp->def_cp_lock);
+ pthread_mutex_destroy(&bp->health_check_lock);
+ if (bp->rep_info) {
+ pthread_mutex_destroy(&bp->rep_info->vfr_lock);
+ pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
+ }
+}
+
static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1444,6 +1456,7 @@ static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
bnxt_free_link_info(bp);
bnxt_free_pf_info(bp);
bnxt_free_parent_info(bp);
+ bnxt_uninit_locks(bp);
rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
bp->tx_mem_zone = NULL;
@@ -4790,10 +4803,6 @@ static int bnxt_init_resources(struct bnxt *bp, bool
reconfig_dev)
return rc;
}
- rc = bnxt_init_locks(bp);
- if (rc)
- return rc;
-
return 0;
}
@@ -5280,6 +5289,10 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params
__rte_unused)
if (rc)
goto error_free;
+ rc = bnxt_init_locks(bp);
+ if (rc)
+ goto error_free;
+
rc = bnxt_init_resources(bp, false);
if (rc)
goto error_free;
@@ -5371,18 +5384,6 @@ bnxt_free_error_recovery_info(struct bnxt *bp)
bp->fw_cap &= ~BNXT_FW_CAP_ERROR_RECOVERY;
}
-static void
-bnxt_uninit_locks(struct bnxt *bp)
-{
- pthread_mutex_destroy(&bp->flow_lock);
- pthread_mutex_destroy(&bp->def_cp_lock);
- pthread_mutex_destroy(&bp->health_check_lock);
- if (bp->rep_info) {
- pthread_mutex_destroy(&bp->rep_info->vfr_lock);
- pthread_mutex_destroy(&bp->rep_info->vfr_start_lock);
- }
-}
-
static int
bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
{
@@ -5404,7 +5405,6 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
bnxt_uninit_ctx_mem(bp);
- bnxt_uninit_locks(bp);
bnxt_free_flow_stats_info(bp);
bnxt_free_rep_info(bp);
rte_free(bp->ptp_cfg);
--
2.28.0.497.g54e85e7
[dpdk-dev] [PATCH 2/3] net/bnxt: fix error path handling of dev start op
Call bnxt_dev_stop in error path of bnxt_dev_start_op() to keep
it simple and consistent
Fixes: c09f57b49c13 ("net/bnxt: add start/stop/link update operations")
Signed-off-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_ethdev.c | 144 -
1 file changed, 70 insertions(+), 74 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6c38155da6..b6a9db1b66 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1239,80 +1239,6 @@ static int bnxt_handle_if_change_status(struct bnxt *bp)
return rc;
}
-static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
-{
- struct bnxt *bp = eth_dev->data->dev_private;
- uint64_t rx_offloads = eth_dev->data->dev_conf.rxmode.offloads;
- int vlan_mask = 0;
- int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
-
- if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
- PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
- return -EINVAL;
- }
-
- if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
- PMD_DRV_LOG(ERR,
- "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
- bp->rx_cp_nr_rings, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-
- do {
- rc = bnxt_hwrm_if_change(bp, true);
- if (rc == 0 || rc != -EAGAIN)
- break;
-
- rte_delay_ms(BNXT_IF_CHANGE_RETRY_INTERVAL);
- } while (retry_cnt--);
-
- if (rc)
- return rc;
-
- if (bp->flags & BNXT_FLAG_IF_CHANGE_HOT_FW_RESET_DONE) {
- rc = bnxt_handle_if_change_status(bp);
- if (rc)
- return rc;
- }
-
- bnxt_enable_int(bp);
-
- rc = bnxt_init_chip(bp);
- if (rc)
- goto error;
-
- eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
- eth_dev->data->dev_started = 1;
-
- bnxt_link_update_op(eth_dev, 1);
-
- if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
- vlan_mask |= ETH_VLAN_FILTER_MASK;
- if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
- vlan_mask |= ETH_VLAN_STRIP_MASK;
- rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
- if (rc)
- goto error;
-
- /* Initialize bnxt ULP port details */
- rc = bnxt_ulp_port_init(bp);
- if (rc)
- goto error;
-
- eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev);
- eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev);
-
- bnxt_schedule_fw_health_check(bp);
-
- return 0;
-
-error:
- bnxt_shutdown_nic(bp);
- bnxt_free_tx_mbufs(bp);
- bnxt_free_rx_mbufs(bp);
- bnxt_hwrm_if_change(bp, false);
- eth_dev->data->dev_started = 0;
- return rc;
-}
-
static int bnxt_dev_set_link_up_op(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -1419,6 +1345,76 @@ static int bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
return 0;
}
+static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
+{
+ struct bnxt *bp = eth_dev->data->dev_private;
+ uint64_t rx_offloads = eth_dev->data->dev_conf.rxmode.offloads;
+ int vlan_mask = 0;
+ int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
+
+ if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
+ PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
+ return -EINVAL;
+ }
+
+ if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
+ PMD_DRV_LOG(ERR,
+ "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
+ bp->rx_cp_nr_rings, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+
+ do {
+ rc = bnxt_hwrm_if_change(bp, true);
+ if (rc == 0 || rc != -EAGAIN)
+ break;
+
+ rte_delay_ms(BNXT_IF_CHANGE_RETRY_INTERVAL);
+ } while (retry_cnt--);
+
+ if (rc)
+ return rc;
+
+ if (bp->flags & BNXT_FLAG_IF_CHANGE_HOT_FW_RESET_DONE) {
+ rc = bnxt_handle_if_change_status(bp);
+ if (rc)
+ return rc;
+ }
+
+ bnxt_enable_int(bp);
+
+ rc = bnxt_init_chip(bp);
+ if (rc)
+ goto error;
+
+ eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
+ eth_dev->data->dev_started = 1;
+
+ bnxt_link_update_op(eth_dev, 1);
+
+ if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
+ vlan_mask |= ETH_VLAN_FILTER_MASK;
+ if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+ vlan_mask |= ETH_VLAN_STRIP_MASK;
+ rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
+ if (rc)
+ goto error;
+
+ /* Initialize bnxt ULP port details */
+ rc = bnxt_ulp_port_init(bp);
+ if (rc)
+
Re: [dpdk-dev] [PATCH v3] app/testpmd: increase array for fetching supported FEC caps
Acked-by: Min Hu (Connor)
在 2020/12/24 19:18, Rahul Lakkireddy 写道:
From: Karra Satwik
Request the driver for number of entries in the FEC caps
array and then dynamically allocate the array.
Signed-off-by: Karra Satwik
Signed-off-by: Rahul Lakkireddy
Acked-by: Xiaoyun Li
---
v3:
- Use unsigned int num, instead of int num
v2:
- Replace if (!speed_fec_capa) with if (speed_fec_capa == NULL)
app/test-pmd/cmdline.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2ccbaa039..1dfad5df4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -16263,11 +16263,9 @@ cmd_show_fec_capability_parsed(void *parsed_result,
__rte_unused struct cmdline *cl,
__rte_unused void *data)
{
-#define FEC_CAP_NUM 2
struct cmd_show_fec_capability_result *res = parsed_result;
- struct rte_eth_fec_capa speed_fec_capa[FEC_CAP_NUM];
- unsigned int num = FEC_CAP_NUM;
- unsigned int ret_num;
+ struct rte_eth_fec_capa *speed_fec_capa;
+ unsigned int num;
int ret;
if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
@@ -16275,17 +16273,31 @@ cmd_show_fec_capability_parsed(void *parsed_result,
return;
}
- ret = rte_eth_fec_get_capability(res->cmd_pid, speed_fec_capa, num);
+ ret = rte_eth_fec_get_capability(res->cmd_pid, NULL, 0);
if (ret == -ENOTSUP) {
printf("Function not implemented\n");
return;
} else if (ret < 0) {
- printf("Get FEC capability failed\n");
+ printf("Get FEC capability failed: %d\n", ret);
+ return;
+ }
+
+ num = (unsigned int)ret;
+ speed_fec_capa = calloc(num, sizeof(*speed_fec_capa));
+ if (speed_fec_capa == NULL) {
+ printf("Failed to alloc FEC capability buffer\n");
return;
}
- ret_num = (unsigned int)ret;
- show_fec_capability(ret_num, speed_fec_capa);
+ ret = rte_eth_fec_get_capability(res->cmd_pid, speed_fec_capa, num);
+ if (ret < 0) {
+ printf("Error getting FEC capability: %d\n", ret);
+ goto out;
+ }
+
+ show_fec_capability(num, speed_fec_capa);
+out:
+ free(speed_fec_capa);
}
cmdline_parse_token_string_t cmd_show_fec_capability_show =
[dpdk-dev] [PATCH v3 0/2] Enhance Async Enqueue for Small Packets
Async enqueue offloads large copies to DMA devices, and small copies are still performed by the CPU. However, it requires users to get enqueue completed packets by rte_vhost_poll_enqueue_completed(), even if they are completed by the CPU when rte_vhost_submit_enqueue_burst() returns. This design incurs extra overheads of tracking completed pktmbufs and function calls, thus degrading performance on small packets. The first patch cleans up async enqueue code, and the second patch enables rte_vhost_submit_enqueue_burst() to return completed packets. Change log == v3: - fix incorrect ret value when DMA ring is full - enhance description of API declaration and programmer guide v2: - fix typo - rename API variables - update programmer guide Jiayu Hu (2): vhost: cleanup async enqueue vhost: enhance async enqueue for small packets doc/guides/prog_guide/vhost_lib.rst | 8 +- lib/librte_vhost/rte_vhost_async.h | 32 +++-- lib/librte_vhost/vhost.c| 14 +- lib/librte_vhost/vhost.h| 7 +- lib/librte_vhost/vhost_user.c | 7 +- lib/librte_vhost/virtio_net.c | 258 6 files changed, 185 insertions(+), 141 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH v3 1/2] vhost: cleanup async enqueue
This patch removes unnecessary check and function calls, and it changes
appropriate types for internal variables and fixes typos.
Signed-off-by: Jiayu Hu
---
lib/librte_vhost/rte_vhost_async.h | 6 +++---
lib/librte_vhost/virtio_net.c | 16
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/lib/librte_vhost/rte_vhost_async.h
b/lib/librte_vhost/rte_vhost_async.h
index c73bd7c..3be4ee4 100644
--- a/lib/librte_vhost/rte_vhost_async.h
+++ b/lib/librte_vhost/rte_vhost_async.h
@@ -147,8 +147,8 @@ __rte_experimental
int rte_vhost_async_channel_unregister(int vid, uint16_t queue_id);
/**
- * This function submit enqueue data to async engine. This function has
- * no guranttee to the transfer completion upon return. Applications
+ * This function submits enqueue data to async engine. This function has
+ * no guarantee to the transfer completion upon return. Applications
* should poll transfer status by rte_vhost_poll_enqueue_completed()
*
* @param vid
@@ -167,7 +167,7 @@ uint16_t rte_vhost_submit_enqueue_burst(int vid, uint16_t
queue_id,
struct rte_mbuf **pkts, uint16_t count);
/**
- * This function check async completion status for a specific vhost
+ * This function checks async completion status for a specific vhost
* device queue. Packets which finish copying (enqueue) operation
* will be returned in an array.
*
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 6c51286..fc654be 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -1128,8 +1128,11 @@ async_mbuf_to_desc(struct virtio_net *dev, struct
vhost_virtqueue *vq,
}
out:
- async_fill_iter(src_it, tlen, src_iovec, tvec_idx);
- async_fill_iter(dst_it, tlen, dst_iovec, tvec_idx);
+ if (tlen) {
+ async_fill_iter(src_it, tlen, src_iovec, tvec_idx);
+ async_fill_iter(dst_it, tlen, dst_iovec, tvec_idx);
+ } else
+ src_it->count = 0;
return error;
}
@@ -1492,10 +1495,9 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
struct rte_vhost_iov_iter *src_it = it_pool;
struct rte_vhost_iov_iter *dst_it = it_pool + 1;
uint16_t n_free_slot, slot_idx = 0;
- uint16_t pkt_err = 0;
uint16_t segs_await = 0;
struct async_inflight_info *pkts_info = vq->async_pkts_info;
- int n_pkts = 0;
+ uint32_t n_pkts = 0, pkt_err = 0;
avail_head = __atomic_load_n(&vq->avail->idx, __ATOMIC_ACQUIRE);
@@ -1553,11 +1555,9 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
/*
* conditions to trigger async device transfer:
* - buffered packet number reaches transfer threshold
-* - this is the last packet in the burst enqueue
* - unused async iov number is less than max vhost vector
*/
if (pkt_burst_idx >= VHOST_ASYNC_BATCH_THRESHOLD ||
- (pkt_idx == count - 1 && pkt_burst_idx) ||
(VHOST_MAX_ASYNC_VEC / 2 - segs_await <
BUF_VECTOR_MAX)) {
n_pkts = vq->async_ops.transfer_data(dev->vid,
@@ -1569,7 +1569,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
segs_await = 0;
vq->async_pkts_inflight_n += pkt_burst_idx;
- if (unlikely(n_pkts < (int)pkt_burst_idx)) {
+ if (unlikely(n_pkts < pkt_burst_idx)) {
/*
* log error packets number here and do actual
* error processing when applications poll
@@ -1589,7 +1589,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
queue_id, tdes, 0, pkt_burst_idx);
vq->async_pkts_inflight_n += pkt_burst_idx;
- if (unlikely(n_pkts < (int)pkt_burst_idx))
+ if (unlikely(n_pkts < pkt_burst_idx))
pkt_err = pkt_burst_idx - n_pkts;
}
--
2.7.4
[dpdk-dev] [PATCH v3 2/2] vhost: enhance async enqueue for small packets
Async enqueue offloads large copies to DMA devices, and small copies
are still performed by the CPU. However, it requires users to get
enqueue completed packets by rte_vhost_poll_enqueue_completed(), even
if they are completed by the CPU when rte_vhost_submit_enqueue_burst()
returns. This design incurs extra overheads of tracking completed
pktmbufs and function calls, thus degrading performance on small packets.
This patch enhances async enqueue for small packets by enabling
rte_vhost_submit_enqueue_burst() to return completed packets.
Signed-off-by: Jiayu Hu
---
doc/guides/prog_guide/vhost_lib.rst | 8 +-
lib/librte_vhost/rte_vhost_async.h | 30 +++--
lib/librte_vhost/vhost.c| 14 +--
lib/librte_vhost/vhost.h| 7 +-
lib/librte_vhost/vhost_user.c | 7 +-
lib/librte_vhost/virtio_net.c | 242
6 files changed, 176 insertions(+), 132 deletions(-)
diff --git a/doc/guides/prog_guide/vhost_lib.rst
b/doc/guides/prog_guide/vhost_lib.rst
index ba4c62a..dc29229 100644
--- a/doc/guides/prog_guide/vhost_lib.rst
+++ b/doc/guides/prog_guide/vhost_lib.rst
@@ -245,11 +245,13 @@ The following is an overview of some key Vhost API
functions:
Unregister the async copy device channel from a vhost queue.
-* ``rte_vhost_submit_enqueue_burst(vid, queue_id, pkts, count)``
+* ``rte_vhost_submit_enqueue_burst(vid, queue_id, pkts, count, comp_pkts,
comp_count)``
Submit an enqueue request to transmit ``count`` packets from host to guest
- by async data path. Enqueue is not guaranteed to finish upon the return of
- this API call.
+ by async data path. Successfully enqueued packets can be transfer completed
+ or being occupied by DMA engines; transfer completed packets are returned in
+ ``comp_pkts``, but others are not guaranteed to finish, when this API
+ call returns.
Applications must not free the packets submitted for enqueue until the
packets are completed.
diff --git a/lib/librte_vhost/rte_vhost_async.h
b/lib/librte_vhost/rte_vhost_async.h
index 3be4ee4..59fe6f9 100644
--- a/lib/librte_vhost/rte_vhost_async.h
+++ b/lib/librte_vhost/rte_vhost_async.h
@@ -87,13 +87,8 @@ struct rte_vhost_async_channel_ops {
* inflight async packet information
*/
struct async_inflight_info {
- union {
- uint32_t info;
- struct {
- uint16_t descs; /* num of descs inflight */
- uint16_t segs; /* iov segs inflight */
- };
- };
+ struct rte_mbuf *mbuf;
+ uint16_t descs; /* num of descs inflight */
};
/**
@@ -147,9 +142,13 @@ __rte_experimental
int rte_vhost_async_channel_unregister(int vid, uint16_t queue_id);
/**
- * This function submits enqueue data to async engine. This function has
- * no guarantee to the transfer completion upon return. Applications
- * should poll transfer status by rte_vhost_poll_enqueue_completed()
+ * This function submits enqueue data to async engine. Successfully
+ * enqueued packets can be transfer completed or being occupied by DMA
+ * engines, when this API returns. Transfer completed packets are returned
+ * in comp_pkts, so users need to guarantee its size is greater than or
+ * equal to the size of pkts; for packets that are successfully enqueued
+ * but not transfer completed, users should poll transfer status by
+ * rte_vhost_poll_enqueue_completed().
*
* @param vid
* id of vhost device to enqueue data
@@ -159,12 +158,19 @@ int rte_vhost_async_channel_unregister(int vid, uint16_t
queue_id);
* array of packets to be enqueued
* @param count
* packets num to be enqueued
+ * @param comp_pkts
+ * empty array to get transfer completed packets. Users need to
+ * guarantee its size is greater than or equal to that of pkts
+ * @param comp_count
+ * num of packets that are transfer completed, when this API returns.
+ * If no packets are transfer completed, its value is set to 0.
* @return
- * num of packets enqueued
+ * num of packets enqueued, including in-flight and transfer completed
*/
__rte_experimental
uint16_t rte_vhost_submit_enqueue_burst(int vid, uint16_t queue_id,
- struct rte_mbuf **pkts, uint16_t count);
+ struct rte_mbuf **pkts, uint16_t count,
+ struct rte_mbuf **comp_pkts, uint32_t *comp_count);
/**
* This function checks async completion status for a specific vhost
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index b83cf63..47e378b 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -327,17 +327,17 @@ cleanup_device(struct virtio_net *dev, int destroy)
static void
vhost_free_async_mem(struct vhost_virtqueue *vq)
{
- if (vq->async_pkts_pending)
- rte_free(vq->async_pkts_pending);
if (vq->async_pkts_info)
rte_free(vq->async_pkts_info);
+ if (vq->async_descs_split)
+ rte_free(vq->async_descs_split);
Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
On Thu, 24 Dec 2020 11:57:48 +0800
dapengx...@intel.com wrote:
> From: YU DAPENG
>
> In interactive mode, if testpmd exit by calling rte_exit without cmdline
> resource release, terminal will not echo keyboard input. So add code to
> just show error message, but not exit testpmd when unexpected happens
> on starting packet forwarding in interactive mode. User can type "quit"
> to exit testpmd later.
>
> Fixes: 5a8fb55c48ab ("app/testpmd: support unidirectional configuration")
> Cc: sta...@dpdk.org
>
> Signed-off-by: YU DAPENG
Sounds like a more generic problem with rte_exit and librte_cmdline.
Would it better to fix it in librte_cmdline by adding an atexit() handler.
Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Hi Stephen,
Do you mean this solution?
1. support atexit() in librte_eal, other component can use it to register a
function to be called when rte_exit() is called.
2. in librte_cmdline, use atexit() to register a function to release resource
I am looking forward to more comments from other maintainers, since this
solution will modify librte_eal and librte_cmdline, but not just testpmd app.
-Original Message-
From: Stephen Hemminger [mailto:step...@networkplumber.org]
Sent: Friday, December 25, 2020 11:03 AM
To: Yu, DapengX
Cc: Lu, Wenzhuo ; Xing, Beilei ;
Iremonger, Bernard ; dev@dpdk.org; sta...@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
On Thu, 24 Dec 2020 11:57:48 +0800
dapengx...@intel.com wrote:
> From: YU DAPENG
>
> In interactive mode, if testpmd exit by calling rte_exit without
> cmdline resource release, terminal will not echo keyboard input. So
> add code to just show error message, but not exit testpmd when
> unexpected happens on starting packet forwarding in interactive mode. User
> can type "quit"
> to exit testpmd later.
>
> Fixes: 5a8fb55c48ab ("app/testpmd: support unidirectional
> configuration")
> Cc: sta...@dpdk.org
>
> Signed-off-by: YU DAPENG
Sounds like a more generic problem with rte_exit and librte_cmdline.
Would it better to fix it in librte_cmdline by adding an atexit() handler.
Re: [dpdk-dev] [PATCH v1 2/5] net/ice: refactor flow pattern parser
> -Original Message-
> From: Yan, Zhirun
> Sent: Monday, December 21, 2020 2:52 PM
> To: dev@dpdk.org; Zhang, Qi Z ; Cao, Yahui
> ; Wang, Xiao W ;
> Guo, Junfeng
> Cc: Su, Simei ; Xu, Ting ; Zhang,
> Yuying ; Yan, Zhirun
>
> Subject: [PATCH v1 2/5] net/ice: refactor flow pattern parser
>
> Distinguish inner/outer input set. And avoid too many nested
> conditionals in each type's parser. input_set_f is used for
> outer fields, input_set_l is used for inner or non-tunnel
> fields.
>
> Signed-off-by: Zhirun Yan
> ---
> drivers/net/ice/ice_fdir_filter.c | 467 +++---
> 1 file changed, 229 insertions(+), 238 deletions(-)
>
> diff --git a/drivers/net/ice/ice_fdir_filter.c
> b/drivers/net/ice/ice_fdir_filter.c
> index 92b8a7e6ad..1f2576a444 100644
> --- a/drivers/net/ice/ice_fdir_filter.c
> +++ b/drivers/net/ice/ice_fdir_filter.c
> @@ -1646,7 +1646,9 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter
> *ad,
> const struct rte_flow_item_vxlan *vxlan_spec, *vxlan_mask;
> const struct rte_flow_item_gtp *gtp_spec, *gtp_mask;
> const struct rte_flow_item_gtp_psc *gtp_psc_spec, *gtp_psc_mask;
> - uint64_t input_set = ICE_INSET_NONE;
> + uint64_t input_set_l = ICE_INSET_NONE;
> + uint64_t input_set_f = ICE_INSET_NONE;
> + uint64_t *input_set;
> uint8_t flow_type = ICE_FLTR_PTYPE_NONF_NONE;
> uint8_t ipv6_addr_mask[16] = {
> 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> @@ -1655,289 +1657,276 @@ ice_fdir_parse_pattern(__rte_unused struct
> ice_adapter *ad,
> uint32_t vtc_flow_cpu;
> uint16_t ether_type;
> enum rte_flow_item_type next_type;
> + bool is_outer = true;
> + struct ice_fdir_extra *p_ext_data;
> + struct ice_fdir_v4 *p_v4;
> + struct ice_fdir_v6 *p_v6;
>
> + for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> + if (item->type == RTE_FLOW_ITEM_TYPE_VXLAN)
> + tunnel_type = ICE_FDIR_TUNNEL_TYPE_VXLAN;
> + if (item->type == RTE_FLOW_ITEM_TYPE_GTPU)
> + tunnel_type = ICE_FDIR_TUNNEL_TYPE_GTPU;
> + if (item->type == RTE_FLOW_ITEM_TYPE_GTP_PSC)
> + tunnel_type = ICE_FDIR_TUNNEL_TYPE_GTPU_EH;
> + }
> +
> + /* This loop parse flow pattern and distinguish Non-tunnel and tunnel
> + * flow. input_set_l is used for non-tunnel and tunnel inner part.
> + */
...
>
> + input_set = (tunnel_type && is_outer) ?
> + &input_set_f : &input_set_l;
[Cao, Yahui] input_set_l used for inner filed or non-tunnel filed looks
confusing, can we just use input_set_l as non-tunnel filed or tunnel outer
field ?
> +
> + if (l3 == RTE_FLOW_ITEM_TYPE_IPV4)
> + p_v4 = (tunnel_type && is_outer) ?
> +&filter->input.ip_outer.v4 :
> +&filter->input.ip.v4;
> + if (l3 == RTE_FLOW_ITEM_TYPE_IPV6)
> + p_v6 = (tunnel_type && is_outer) ?
> +&filter->input.ip_outer.v6 :
> +&filter->input.ip.v6;
> +
[Cao, Yahui] p_v4 and p_v6 pointer assignment looks redundant since it will be
assigned in IPV4 and IPV6 switch case statement.
> switch (item_type) {
...
> +
> + p_v4 = (tunnel_type && is_outer) ?
> +&filter->input.ip_outer.v4 :
> +&filter->input.ip.v4;
[Cao, Yahui] it is assigned here again
> + p_v4->dst_ip = ipv4_spec->hdr.dst_addr;
> + p_v4->src_ip = ipv4_spec->hdr.src_addr;
> + p_v4->tos = ipv4_spec->hdr.type_of_service;
> break;
..
> +
> + p_v6 = (tunnel_type && is_outer) ?
> +&filter->input.ip_outer.v6 :
> +&filter->input.ip.v6;
[Cao, Yahui] it is assigned here again
> + rte_memcpy(&p_v6->dst_ip, ipv6_spec->hdr.dst_addr, 16);
> + rte_memcpy(&p_v6->src_ip, ipv6_spec->hdr.src_addr, 16);
> + vtc_flow_cpu =
> rte_be_to_cpu_32(ipv6_spec->hdr.vtc_flow);
> + p_v6->tc = (uint8_t)(vtc_flow_cpu >>
> ICE_FDIR_IPV6_TC_OFFSET);
> + p_v6->proto = ipv6_spec->hdr.proto;
> + p_v6->hlim = ipv6_spec->hdr.hop_limits;
> break;
..
> filter->tunnel_type = tunnel_type;
> filter->input.flow_type = flow_type;
> - filter->input_set = input_set;
> + filter->input_set = input_set_l;
> + filter->outer_input_set = input_set_f;
[Cao, Yahui] Correspondingly here, input_set and outer_input_set naming is also
confusing.
>
> return 0;
> }
> --
> 2.25.1
Re: [dpdk-dev] [PATCH v1 3/5] net/ice: add outer input set mask to distinguish outer fields
> -Original Message-
> From: Yan, Zhirun
> Sent: Monday, December 21, 2020 2:52 PM
> To: dev@dpdk.org; Zhang, Qi Z ; Cao, Yahui
> ; Wang, Xiao W ;
> Guo, Junfeng
> Cc: Su, Simei ; Xu, Ting ; Zhang,
> Yuying ; Yan, Zhirun
>
> Subject: [PATCH v1 3/5] net/ice: add outer input set mask to distinguish
> outer fields
>
> Add 64-bit input_set_mask_f for outer input set. input_set_mask_f is
> used for inner fields or non-tunnel fields. Adjust indentation of
> ice_pattern_match_item list in switch, RSS and FDIR for easy
> review. For fields in tunnel layer, like GTPU TEID, put them in
> outer part.
>
> Signed-off-by: Zhirun Yan
> ---
> drivers/net/ice/ice_fdir_filter.c | 110 -
> drivers/net/ice/ice_generic_flow.h | 1 +
> drivers/net/ice/ice_hash.c | 192 ++-
> drivers/net/ice/ice_switch_filter.c | 348 ++--
> 4 files changed, 231 insertions(+), 420 deletions(-)
>
> diff --git a/drivers/net/ice/ice_fdir_filter.c
> b/drivers/net/ice/ice_fdir_filter.c
> index 1f2576a444..76e0a8df38 100644
> --- a/drivers/net/ice/ice_fdir_filter.c
> +++ b/drivers/net/ice/ice_fdir_filter.c
> @@ -55,92 +55,74 @@
>
...
> + {pattern_eth_ipv4, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV4,ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV4_UDP,ICE_INSET_NONE},
> + {pattern_eth_ipv4_tcp, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV4_TCP,ICE_INSET_NONE},
> + {pattern_eth_ipv4_sctp, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV4_SCTP, ICE_INSET_NONE},
> + {pattern_eth_ipv6, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV6,ICE_INSET_NONE},
> + {pattern_eth_ipv6_udp, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV6_UDP,ICE_INSET_NONE},
> + {pattern_eth_ipv6_tcp, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV6_TCP,ICE_INSET_NONE},
> + {pattern_eth_ipv6_sctp, ICE_INSET_NONE,
> ICE_FDIR_INSET_ETH_IPV6_SCTP, ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_ipv4, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_L,ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_ipv4_udp, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_UDP_L,ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_ipv4_tcp, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_TCP_L,ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_ipv4_sctp, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_SCTP_L, ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_eth_ipv4, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_L,ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_eth_ipv4_udp, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_UDP_L,ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_eth_ipv4_tcp, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_TCP_L,ICE_INSET_NONE},
> + {pattern_eth_ipv4_udp_vxlan_eth_ipv4_sctp, ICE_INSET_NONE,
> ICE_FDIR_INSET_VXLAN_IPV4_SCTP_L, ICE_INSET_NONE},
> };
>
[Cao, Yahui] Why is the mask put in the 3rd column instead of 2nd column ? I
prefer the way that 2nd column is for outer field and 3rd column is for inner
field.
>
> static int
> --
> 2.25.1
Re: [dpdk-dev] [PATCH v1 4/5] net/ice: add outer input set mask check
I suggest that you can merge this commit into the Patch 3/5, since they are
all about input set mask changes.
> -Original Message-
> From: Yan, Zhirun
> Sent: Monday, December 21, 2020 2:52 PM
> To: dev@dpdk.org; Zhang, Qi Z ; Cao, Yahui
> ; Wang, Xiao W ;
> Guo, Junfeng
> Cc: Su, Simei ; Xu, Ting ; Zhang,
> Yuying ; Yan, Zhirun
>
> Subject: [PATCH v1 4/5] net/ice: add outer input set mask check
>
> Distinguish input set mask for inner/outer. Add outer input set
> mask check.
>
> Signed-off-by: Zhirun Yan
> ---
> drivers/net/ice/ice_fdir_filter.c | 3 ++-
> drivers/net/ice/ice_generic_flow.c | 2 ++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ice/ice_fdir_filter.c
> b/drivers/net/ice/ice_fdir_filter.c
> index 76e0a8df38..2d2b261368 100644
> --- a/drivers/net/ice/ice_fdir_filter.c
> +++ b/drivers/net/ice/ice_fdir_filter.c
> @@ -2020,7 +2020,8 @@ ice_fdir_parse(struct ice_adapter *ad,
> if (ret)
> goto error;
> input_set = filter->input_set | filter->outer_input_set;
> - if (!input_set || input_set & ~item->input_set_mask) {
> + if (!input_set || filter->input_set & ~item->input_set_mask ||
> + filter->outer_input_set & ~item->input_set_mask_f) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
> pattern,
> diff --git a/drivers/net/ice/ice_generic_flow.c
> b/drivers/net/ice/ice_generic_flow.c
> index 1429cbc3b6..6c20b070c7 100644
> --- a/drivers/net/ice/ice_generic_flow.c
> +++ b/drivers/net/ice/ice_generic_flow.c
> @@ -2088,6 +2088,8 @@ ice_search_pattern_match_item(const struct
> rte_flow_item pattern[],
> items)) {
> pattern_match_item->input_set_mask =
> array[i].input_set_mask;
> + pattern_match_item->input_set_mask_f =
> + array[i].input_set_mask_f;
> pattern_match_item->pattern_list =
> array[i].pattern_list;
> pattern_match_item->meta = array[i].meta;
> --
> 2.25.1
Re: [dpdk-dev] [RFC] mem_debug add more log
The performance of our simple scheme is better than asan. We are trying the asan solution. Regards, Peng,Zhihong -Original Message- From: Stephen Hemminger Sent: Tuesday, December 22, 2020 2:44 AM To: Peng, ZhihongX Cc: Wang, Haiyue ; Zhang, Qi Z ; Xing, Beilei ; dev@dpdk.org; Lin, Xueqin ; Yu, PingX Subject: Re: [dpdk-dev] [RFC] mem_debug add more log On Mon, 21 Dec 2020 07:35:10 + "Peng, ZhihongX" wrote: > 1. I think this implement doesn't add significant overhead. Overhead only > will be occurred in rte_malloc and rte_free. > > 2. Current existing address sanitizer infrastructure only support libc malloc. > > Regards, > Peng,Zhihong > > -Original Message- > From: Stephen Hemminger > Sent: Saturday, December 19, 2020 2:54 AM > To: Peng, ZhihongX > Cc: Wang, Haiyue ; Zhang, Qi Z > ; Xing, Beilei ; > dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC] mem_debug add more log > > On Fri, 18 Dec 2020 14:21:09 -0500 > Peng Zhihong wrote: > > > 1. The debugging log in current DPDK RTE_MALLOC_DEBUG mode is insufficient, > >which makes it difficult to locate the issues, such as: > >a) When a memeory overlflow occur in rte_free, there is a little log > > information. Even if abort here, we can find which API is core > > dumped but we still need to read the source code to find out where > > the requested memory is overflowed. > >b) Current DPDK can NOT find that the overflow if the memory has been > > used and not released. > >c) If there are two pieces of continuous memory, when the first block > > is not released and an overflow is occured and also the second block > > of memory is covered, a memory overflow will be detected once the > > second > > block of memory is released. However, current DPDK can not find the > > correct point of memory overflow. It only detect the memory overflow > > of the second block but should dedect the one of first block. > > > > -- > > | header cookie | data1 | trailer cookie | header cookie | > > data2 |trailer cookie | > > > > > > -- > > 2. To fix above issues, we can store the requested > > information When DPDK > >request memory. Including the requested address and requested momory's > >file, function and numbers of rows and then put it into a list. > > -- > > -- > >| struct list_head |>| struct malloc_info |>| struct malloc_info > > | > > -- > > -- > >The above 3 problems can be solved through this implementation: > >a) If there is a memory overflow in rte_free, you can traverse the > > list to find the information of overflow memory and print the > > overflow memory information. like this: > > code: > > 37 char *p = rte_zmalloc(NULL, 64, 0); > > 38 memset(p, 0, 65); > > 39 rte_free(p); > > 40 //rte_malloc_validate_all_memory(); > > memory error: > > EAL: Error: Invalid memory > > malloc memory address 0x17ff2c340 overflow in \ > > file:../examples/helloworld/main.c function:main line:37 > >b)c) Provide a interface to check all memory overflow in function > > rte_malloc_validate_all_memory, this function will check all > > memory on the list. Call this funcation manually at the exit > > point of business logic, we can find all overflow points in time. > > > > Signed-off-by: Peng Zhihong > > Good concept, but doesn't this add significant overhead? > > Maybe we could make rte_malloc work with existing address sanitizer > infrastructure in gcc/clang? That would provide faster and more immediate > better diagnostic info. Everybody builds there own custom debug hooks, and some of these are worth sharing. But lots of time debug code becomes a technical debt, creates API/ABI issues and causes more trouble than it is worth. Therefore my desire is for DPDK to be better supported by standard tools such as valgrind and address sanitizer. The standard tools catch more errors faster and do not create project maintenance workload. See: https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm
