Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
On 2019/4/25 23:51, Varghese, Vipin wrote:
Hi,
snipped
@@ -847,6 +847,10 @@ struct parse_val {
pdump_rxtx(pt->rx_ring, pt->rx_vdev_id, &pt->stats);
if (pt->dir & RTE_PDUMP_FLAG_TX)
pdump_rxtx(pt->tx_ring, pt->tx_vdev_id, &pt->stats);
+
+ /* Once primary exits, so will I. */
+ if (!rte_eal_primary_proc_alive(NULL))
+ quit_signal = 1;
}
As per the current suggested code flow check is added to while loop in function
`dump_packets'.
Thanks for the reply. Since want to make it clean, the code was here.
However, it seems need to take care of the performance impact first.
Questions:
1. What is impact in performance with and without patch?
A1. Do a little trick as the patch below to tested the impact in the single
core mode on Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz with no pkts.
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 3d208548fa13..804011b187c4 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -141,7 +141,7 @@ struct parse_val {
static int num_tuples;
static struct rte_eth_conf port_conf_default;
-static volatile uint8_t quit_signal;
+static volatile uint32_t quit_signal;
static uint8_t multiple_core_capture;
/**< display usage */
@@ -868,6 +868,7 @@ struct parse_val {
dump_packets(void)
{
int i;
+ uint64_t start, end;
uint32_t lcore_id = 0;
if (!multiple_core_capture) {
@@ -880,10 +881,20 @@ struct parse_val {
pdump_t[i].device_id,
pdump_t[i].queue);
- while (!quit_signal) {
+ /* make it hot */
+ rte_eal_primary_proc_alive(NULL);
+ rte_eal_primary_proc_alive(NULL)
+
+ start = rte_rdtsc();
+ while (quit_signal < 5) {
+ /* Just testing with and w/o the 'if' line below */
+ if (rte_eal_primary_proc_alive(NULL))
+ quit_signal++;
for (i = 0; i < num_tuples; i++)
pdump_packets(&pdump_t[i]);
}
+ end = rte_rdtsc();
+ printf("Totally count:%u, cost tsc:%lu\n", quit_signal, end -
start);
return;
}
The total tsc cost is about 338809671 with rte_eal_primary_proc_alive().
And the tsc cost is just about 513573 without rte_eal_primary_proc_alive().
The dpdk-pdump had also used taskset to bind to specify isolate core.
So it seems the patch do a great performance impact.
Maybe another async method should be introduced to monitor the primary status.
2. For various packet sizes and port speed what are delta in drops for packet
capture?
A2. Refer to A1, it's not needed anymore.
Note: If pdump application is still alive when primary is not running, primary
cannot be started. Is this a cue that pdump is still alive and has to be
terminated?
Yes, some guys complained that the residual dpdk-pdump impact the restart of
the primary app and refuse to add other mechanisms e.g. to kill the dpdk-pdump
in the app to avoid that case.
So the patch was created.
Is there any other ways to avoid that.
static int
--
1.7.12.4
Re: [dpdk-dev] [EXT] Re: [PATCH 5/6] crypto/dpaa_sec: fix session qp attach/detach
Hi Kevin,
Thanks we will look into it.
Regards,
Hemant
> -Original Message-
> From: Kevin Traynor
> Sent: Thursday, April 25, 2019 11:13 PM
> To: Akhil Goyal ; dev@dpdk.org
> Cc: Hemant Agrawal ; sta...@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH 5/6] crypto/dpaa_sec: fix session qp
> attach/detach
> Importance: High
>
> Caution: EXT Email
>
> On 25/04/2019 17:24, Kevin Traynor wrote:
> > On 27/03/2019 11:53, Akhil Goyal wrote:
> >> session inq and qp are assigned for each core from which the packets
> >> arrive. This was not correctly handled while supporting multiple
> >> sessions per queue pair.
> >> This patch fixes the attach and detach of queues for each core.
> >>
> >> Fixes: e79416d10fa3 ("crypto/dpaa_sec: support multiple sessions per
> >> queue pair")
> >> Cc: sta...@dpdk.org
> >
> > Hi, this will not backport to 18.11 branch as:
> > 4e694fe51171dcdbe94019189a0240833b45c943
> > Author: Akhil Goyal
> > Date: Wed Jan 9 15:14:17 2019 +
> >
> > crypto/dpaa_sec: support same session flows on multi-cores
> >
> > was added after 18.11 and dpaa_sec_sym_session_clear() is quite
> > different. Please send a backport of the bugfix for 18.11 branch, or
> > let me know if it's not needed.
> >
> > thanks,
> > Kevin.
> >
> >> Signed-off-by: Akhil Goyal
> >> ---
> >> drivers/crypto/dpaa_sec/dpaa_sec.c | 17 ++---
> >> 1 file changed, 10 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
> >> b/drivers/crypto/dpaa_sec/dpaa_sec.c
> >> index cb99be4e1..8305f19a3 100644
> >> --- a/drivers/crypto/dpaa_sec/dpaa_sec.c
> >> +++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
> >> @@ -1,7 +1,7 @@
> >> /* SPDX-License-Identifier: BSD-3-Clause
> >> *
> >> * Copyright (c) 2016 Freescale Semiconductor, Inc. All rights reserved.
> >> - * Copyright 2017-2018 NXP
> >> + * Copyright 2017-2019 NXP
> >> *
> >> */
> >>
> >> @@ -1940,13 +1940,13 @@ dpaa_sec_attach_rxq(struct
> >> dpaa_sec_dev_private *qi) {
> >> unsigned int i;
> >>
> >> -for (i = 0; i < qi->max_nb_sessions; i++) {
> >> +for (i = 0; i < qi->max_nb_sessions * MAX_DPAA_CORES; i++) {
> >> if (qi->inq_attach[i] == 0) {
>
> Btw, without knowing the crypto code, the use of max_nb_sessions in for
> loops with fixed size arrays being indexed looks suspect to me. In the snippet
> above
>
> unsigned char inq_attach[RTE_DPAA_MAX_RX_QUEUE];
> which means inq_attach[4096]
>
> At least in one place I can see
> .max_nb_sessions = MRVL_PMD_DEFAULT_MAX_NB_SESSIONS which
> makes max_nb_sessions=2048, while MAX_DPAA_CORES=4
>
> >> qi->inq_attach[i] = 1;
> >> return &qi->inq[i];
> >> }
> >> }
> >> -DPAA_SEC_WARN("All ses session in use %x", qi->max_nb_sessions);
> >> +DPAA_SEC_WARN("All session in use %u", qi->max_nb_sessions);
> >>
> >> return NULL;
> >> }
> >> @@ -2115,7 +2115,7 @@ dpaa_sec_sym_session_clear(struct
> rte_cryptodev *dev,
> >> struct rte_cryptodev_sym_session *sess) {
> >> struct dpaa_sec_dev_private *qi = dev->data->dev_private;
> >> -uint8_t index = dev->driver_id;
> >> +uint8_t index = dev->driver_id, i;
> >> void *sess_priv = get_sym_session_private_data(sess, index);
> >>
> >> PMD_INIT_FUNC_TRACE();
> >> @@ -2125,9 +2125,12 @@ dpaa_sec_sym_session_clear(struct
> rte_cryptodev *dev,
> >> if (sess_priv) {
> >> struct rte_mempool *sess_mp =
> >> rte_mempool_from_obj(sess_priv);
> >>
> >> -if (s->inq[rte_lcore_id() % MAX_DPAA_CORES])
> >> -dpaa_sec_detach_rxq(qi,
> >> -s->inq[rte_lcore_id() % MAX_DPAA_CORES]);
> >> +for (i = 0; i < MAX_DPAA_CORES; i++) {
> >> +if (s->inq[i])
> >> +dpaa_sec_detach_rxq(qi, s->inq[i]);
> >> +s->inq[i] = NULL;
> >> +s->qp[i] = NULL;
> >> +}
> >> rte_free(s->cipher_key.data);
> >> rte_free(s->auth_key.data);
> >> memset(s, 0, sizeof(dpaa_sec_session));
> >>
> >
Re: [dpdk-dev] [PATCH] eventdev: fix control flow issues
> -Original Message-
> From: dev On Behalf Of Abhinandan Gujjar
> Sent: Wednesday, April 24, 2019 10:05 PM
> To: jerin.ja...@caviumnetworks.com; dev@dpdk.org
> Cc: narender.vang...@intel.com; abhinandan.guj...@intel.com;
> john.mcnam...@intel.com
> Subject: [dpdk-dev] [PATCH] eventdev: fix control flow issues
>
> This patch fixes null pointer dereference and control flow issues reported in
> event crypto adapter by coverity
>
> Coverity issue: 279439 Dereference null return value Coverity issue: 279446
> Logically dead code Coverity issue: 279450 Logically dead code Coverity issue:
> 279462 Logically dead code
> Fixes: 7901eac3409 ("eventdev: add crypto adapter implementation")
Cc: sta...@dpdk.org
>
> Signed-off-by: Abhinandan Gujjar
Acked-by: Jerin Jacob
Re: [dpdk-dev] [PATCH v3 1/3] rte_ethdev: Add API function to read dev clock
@Andrew I applied your comments. Thanks. On 2019-04-25 20:28, Wiles, Keith wrote: What is a raw clock value? It took me a bit to find that it is in nano-seconds need to document that point. It is not in nanosecond, it has no units. Finding the relation between the device clock and the real time is the whole point of this API. Using timestamp variable is not very descriptive and some other name needs to be used. Also in the driver it is called *clock. The problem is that the "timestamp offload" feature filling the timestamp field of the pktmbuf is already badly named as it is given without time unit. Both of them are not timestamp but raw "clock" values, a number of ticks at an unknown frequency, with an unknow time base (ie is it the number of ticks since boot? Device is up? 1/1/1970?). One solution would be to have an union in the pktmbuf, changing the timestamp field into a "clock" or "timestamp" field. But it's a bit overkill. I propose to change the read_clock API to reference "clock" instead of timestamp everywhere. Another question is why does this routine not perform the looping/delaying and calling read_clock and then return frequency instead. If you want a timestamp reading function then this one is not being described that way and I would think it should be done someplace else or should be. The frequency is one thing, and would allow to give a relative time in second between packets. In practice though, we change the frequency (as Linux does regarding the TSC ticks) to catch up corrections applied by NTP on the source clock. The second thing is the time reference, to convert the clock time (of the packets) to the current wall time. One may want to use the monotonic clock, the real clock (we do follow both). Or no system clock at all and directly follow an NTP source. The point is, having a function to give the frequency is not enough. One can derive the frequency and the time reference with this API. I could write a helper in a second step to get the frequency out of read_clock. But the time reference implies timers, choosing a source, and stuff we don't want here, I think... /** Valid if PKT_RX_TIMESTAMP is set. The unit and time reference * are not normalized but are always the same for a given port. +* Some devices allow to query rte_eth_read_clock that will return the +* current device timestamp. The message here is not a good place for this detail, I would put it in the function call doc above. I can remove these lines, but with read_clock referencing only clock I feel like it's even more needed. Someone reading the doc will see "oh, I can use timestamp offloading to get precise timing of when my packets were received, great ! But it has no unit and no reference... What do I do with that?". It miss the last step "how I see... I can look at rte_eth_read_clock documentation to read more about conversion." Thanks ! Tom
Re: [dpdk-dev] [PATCH v8 1/4] rcu: add RCU library supporting QSBR mechanism
> -Original Message- > From: dev On Behalf Of Honnappa Nagarahalli > Sent: Friday, April 26, 2019 10:10 AM > To: konstantin.anan...@intel.com; step...@networkplumber.org; > paul...@linux.ibm.com; marko.kovace...@intel.com; dev@dpdk.org > Cc: honnappa.nagaraha...@arm.com; gavin...@arm.com; > dharmik.thak...@arm.com; malvika.gu...@arm.com > Subject: [dpdk-dev] [PATCH v8 1/4] rcu: add RCU library supporting QSBR > mechanism > > Add RCU library supporting quiescent state based memory reclamation method. > This library helps identify the quiescent state of the reader threads so that > the > writers can free the memory associated with the lock less data structures. > > Signed-off-by: Honnappa Nagarahalli > Reviewed-by: Steve Capper > Reviewed-by: Gavin Hu > Reviewed-by: Ola Liljedahl > Acked-by: Konstantin Ananyev > Acked-by: Paul E. McKenney Tested using UT on a armv8.2 24 cores machine(octeontx2) Tested-by: Jerin Jacob
Re: [dpdk-dev] Editor Config
On 25-Apr-19 6:19 PM, Stephen Hemminger wrote: Systemd uses this and it looks like a useful addition to the DPDK source base. Especially since we now have Windows developers. https://editorconfig.org/ I have suggested using clang-format in the past, but this would be a good step too. -- Thanks, Anatoly
Re: [dpdk-dev] Editor Config
On 4/25/19 8:19 PM, Stephen Hemminger wrote: Systemd uses this and it looks like a useful addition to the DPDK source base. Especially since we now have Windows developers. https://editorconfig.org/ +1
[dpdk-dev] [PATCH v3] examples/vm_power_manager: fix buffer overrun
The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
yet the code can attempt to look at the index at RTE_MAX_LCORE,
which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
RTE_MAX_LCORE_FREQS.
Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
Coverity issue: 337660
CC: sta...@dpdk.org
Signed-off-by: David Hunt
Acked-by: Reshma Pattan
---
v2 - Rebase to 19.05 RC2.
v3 - add CC stable
---
examples/vm_power_manager/power_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/power_manager.c
b/examples/vm_power_manager/power_manager.c
index 9553455be..9d4e587b0 100644
--- a/examples/vm_power_manager/power_manager.c
+++ b/examples/vm_power_manager/power_manager.c
@@ -143,7 +143,7 @@ power_manager_get_current_frequency(unsigned core_num)
rte_spinlock_lock(&global_core_freq_info[core_num].power_sl);
index = rte_power_get_freq(core_num);
rte_spinlock_unlock(&global_core_freq_info[core_num].power_sl);
- if (index >= RTE_MAX_LCORE)
+ if (index >= RTE_MAX_LCORE_FREQS)
freq = 0;
else
freq = global_core_freq_info[core_num].freqs[index];
--
2.17.1
[dpdk-dev] [PATCH v1] examples/vm_power_manager: fix string null termination
coverity complains about a null-termination after a read,
so we terminate conditionally on whether idx is within
the buffer or at the end of the buffer.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/channel_monitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..711722fef 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -808,6 +808,7 @@ read_json_packet(struct channel_info *chan_info)
int indent = 0;
do {
n_bytes = read(chan_info->fd, &json_data[idx], 1);
+ json_data[idx + (idx < MAX_JSON_STRING_LEN - 1)] = '\0';
if (n_bytes == 0)
break;
if (json_data[idx] == '{')
--
2.17.1
[dpdk-dev] [PATCH v1] examples/vm_power_manager: fix overflowed return value
Coverity complains about the return of a value that may
possibly overflow because of a multiply. Limit the value
so it cannot overflow.
Coverity issue: 337677
Fixes: 4b1a631b8a ("examples/vm_power: add oob monitoring functions")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/oob_monitor_x86.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/oob_monitor_x86.c
b/examples/vm_power_manager/oob_monitor_x86.c
index ebd96b205..2074eec1e 100644
--- a/examples/vm_power_manager/oob_monitor_x86.c
+++ b/examples/vm_power_manager/oob_monitor_x86.c
@@ -99,7 +99,10 @@ apply_policy(int core)
return -1.0;
}
- ratio = (float)miss_diff * (float)100 / (float)hits_diff;
+ ratio = (float)miss_diff / (float)hits_diff;
+ if (ratio > 1.0)
+ ratio = 1.0;
+ ratio *= 100.0f;
if (ratio < ci->branch_ratio_threshold)
power_manager_scale_core_min(core);
--
2.17.1
Re: [dpdk-dev] [PATCH v5] kni: add IOVA va support for kni
On 22-Apr-19 7:15 AM, kirankum...@marvell.com wrote:
From: Kiran Kumar K
With current KNI implementation kernel module will work only in
IOVA=PA mode. This patch will add support for kernel module to work
with IOVA=VA mode.
The idea is to get the physical address from iova address using
api iommu_iova_to_phys. Using this API, we will get the physical
address from iova address and later use phys_to_virt API to
convert the physical address to kernel virtual address.
With this approach we have compared the performance with IOVA=PA
and there is no difference observed. Seems like kernel is the
overhead.
This approach will not work with the kernel versions less than 4.4.0
because of API compatibility issues.
Signed-off-by: Kiran Kumar K
---
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index be9e6b0b9..e77a28066 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -35,6 +35,22 @@ static void kni_net_rx_normal(struct kni_dev *kni);
/* kni rx function pointer, with default to normal rx */
static kni_net_rx_t kni_net_rx_func = kni_net_rx_normal;
+/* iova to kernel virtual address */
+static void *
+iova2kva(struct kni_dev *kni, void *pa)
+{
+ return phys_to_virt(iommu_iova_to_phys(kni->domain,
+ (uintptr_t)pa));
+}
+
+static void *
+iova2data_kva(struct kni_dev *kni, struct rte_kni_mbuf *m)
+{
+ return phys_to_virt((iommu_iova_to_phys(kni->domain,
+ (uintptr_t)m->buf_physaddr) +
+m->data_off));
+}
+
Apologies, i've accidentally responded to the previous version with this
comment.
I don't see how this could possibly work, because for any
IOVA-contiguous chunk of memory, mbufs are allowed to cross page
boundaries. In this function, you're getting the start address of a
buffer, but there are no guarantees that the end of the buffer is on the
same physical page.
--
Thanks,
Anatoly
[dpdk-dev] [PATCH v3] vhost: support inflight share memory protocol feature
From: lilin24
This patch introduces two new messages VHOST_USER_GET_INFLIGHT_FD
and VHOST_USER_SET_INFLIGHT_FD to support transferring a shared
buffer between qemu and backend.
Firstly, qemu uses VHOST_USER_GET_INFLIGHT_FD to get the
shared buffer from backend. Then qemu should send it back
through VHOST_USER_SET_INFLIGHT_FD each time we start vhost-user.
This shared buffer is used to process inflight I/O when backend
reconnect.
Signed-off-by: lilin24
Signed-off-by: Ni Xun
Signed-off-by: Zhang Yu
v2:
- add set&clr inflight entry function
v3:
- simplified some function implementations
---
lib/librte_vhost/rte_vhost.h | 53 ++
lib/librte_vhost/vhost.c | 42
lib/librte_vhost/vhost.h | 11 ++
lib/librte_vhost/vhost_user.c | 231 ++
lib/librte_vhost/vhost_user.h | 16 ++-
5 files changed, 351 insertions(+), 2 deletions(-)
diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index d2c0c93f4..bc25591a8 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -71,6 +71,10 @@ extern "C" {
#define VHOST_USER_PROTOCOL_F_HOST_NOTIFIER 11
#endif
+#ifndef VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD
+#define VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD 12
+#endif
+
/** Indicate whether protocol features negotiation is supported. */
#ifndef VHOST_USER_F_PROTOCOL_FEATURES
#define VHOST_USER_F_PROTOCOL_FEATURES 30
@@ -98,12 +102,26 @@ struct rte_vhost_memory {
struct rte_vhost_mem_region regions[];
};
+typedef struct VhostUserInflightEntry {
+ uint8_t inflight;
+} VhostUserInflightEntry;
+
+typedef struct VhostInflightInfo {
+ uint16_t version;
+ uint16_t last_inflight_io;
+ uint16_t used_idx;
+ VhostUserInflightEntry desc[0];
+} VhostInflightInfo;
+
struct rte_vhost_vring {
struct vring_desc *desc;
struct vring_avail *avail;
struct vring_used *used;
uint64_tlog_guest_addr;
+ VhostInflightInfo *inflight;
+ intinflight_flag;
+
/** Deprecated, use rte_vhost_vring_call() instead. */
int callfd;
@@ -632,6 +650,41 @@ int rte_vhost_get_vhost_vring(int vid, uint16_t vring_idx,
int rte_vhost_vring_call(int vid, uint16_t vring_idx);
/**
+ * set inflight flag for a entry.
+ *
+ * @param vring
+ * the structure to hold the requested vring info
+ * @param idx
+ * inflight entry index
+ */
+void rte_vhost_set_inflight(struct rte_vhost_vring *vring,
+ uint16_t idx);
+
+/**
+ * clear inflight flag for a entry.
+ *
+ * @param vring
+ * the structure to hold the requested vring info
+ * @param last_used_idx
+ * next free used_idx
+ * @param idx
+ * inflight entry index
+ */
+void rte_vhost_clr_inflight(struct rte_vhost_vring *vring,
+ uint16_t last_used_idx, uint16_t idx);
+
+/**
+ * set last inflight io index.
+ *
+ * @param vring
+ * the structure to hold the requested vring info
+ * @param idx
+ * inflight entry index
+ */
+void rte_vhost_set_last_inflight_io(struct rte_vhost_vring *vring,
+ uint16_t idx);
+
+/**
* Get vhost RX queue avail count.
*
* @param vid
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index 163f4595e..9ba692935 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -76,6 +76,8 @@ cleanup_vq(struct vhost_virtqueue *vq, int destroy)
close(vq->callfd);
if (vq->kickfd >= 0)
close(vq->kickfd);
+ if (vq->inflight)
+ vq->inflight = NULL;
}
/*
@@ -589,6 +591,11 @@ rte_vhost_get_vhost_vring(int vid, uint16_t vring_idx,
vring->kickfd = vq->kickfd;
vring->size= vq->size;
+ vring->inflight = vq->inflight;
+
+ vring->inflight_flag = vq->inflight_flag;
+ vq->inflight_flag = 0;
+
return 0;
}
@@ -617,6 +624,41 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx)
return 0;
}
+void
+rte_vhost_set_inflight(struct rte_vhost_vring *vring, uint16_t idx)
+{
+ VhostInflightInfo *inflight = vring->inflight;
+ if (unlikely(!inflight))
+ return;
+ inflight->desc[idx].inflight = 1;
+}
+
+void
+rte_vhost_clr_inflight(struct rte_vhost_vring *vring,
+ uint16_t last_used_idx, uint16_t idx)
+{
+ VhostInflightInfo *inflight = vring->inflight;
+
+ if (unlikely(!inflight))
+ return;
+
+ rte_compiler_barrier();
+ inflight->desc[idx].inflight = 0;
+ rte_compiler_barrier();
+ inflight->used_idx = last_used_idx;
+}
+
+void
+rte_vhost_set_last_inflight_io(struct rte_vhost_vring *vring, uint16_t idx)
+{
+ VhostInflightInfo *inflight = vring->inflight;
+
+ if (unlikely(!inflight))
+ return;
+
+ inflight->last_inflight_io = idx;
+}
+
uint16_t
rte_vhost_avail_entries(int vid, uint16_t queue_id)
{
diff --git a/lib/librte_vhost/v
[dpdk-dev] [PATCH] ipc: fix send error handling
According to manpage, ENOBUFS error indicates that either the
input or the output queue is full. This should be considered
an error, but it is treated as an "ignore" condition. Fix the
code to report an error instead.
Fixes: bacaa2754017 ("eal: add channel for multi-process communication")
Cc: sta...@dpdk.org
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/common/eal_common_proc.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_proc.c
b/lib/librte_eal/common/eal_common_proc.c
index b46d644b3..f26a60595 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -678,11 +678,6 @@ send_msg(const char *dst_path, struct rte_mp_msg *msg, int
type)
unlink(dst_path);
return 0;
}
- if (errno == ENOBUFS) {
- RTE_LOG(ERR, EAL, "Peer cannot receive message %s\n",
- dst_path);
- return 0;
- }
RTE_LOG(ERR, EAL, "failed to send to (%s) due to %s\n",
dst_path, strerror(errno));
return -1;
--
2.17.1
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix overflowed return value
On 26-Apr-19 9:44 AM, David Hunt wrote:
Coverity complains about the return of a value that may
possibly overflow because of a multiply. Limit the value
so it cannot overflow.
Coverity issue: 337677
Fixes: 4b1a631b8a ("examples/vm_power: add oob monitoring functions")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/oob_monitor_x86.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/oob_monitor_x86.c
b/examples/vm_power_manager/oob_monitor_x86.c
index ebd96b205..2074eec1e 100644
--- a/examples/vm_power_manager/oob_monitor_x86.c
+++ b/examples/vm_power_manager/oob_monitor_x86.c
@@ -99,7 +99,10 @@ apply_policy(int core)
return -1.0;
}
- ratio = (float)miss_diff * (float)100 / (float)hits_diff;
+ ratio = (float)miss_diff / (float)hits_diff;
+ if (ratio > 1.0)
+ ratio = 1.0;
+ ratio *= 100.0f;
It should probably be the other way around - multiply first, then clamp.
Also, please use RTE_MIN.
if (ratio < ci->branch_ratio_threshold)
power_manager_scale_core_min(core);
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix string null termination
On 26-Apr-19 9:43 AM, David Hunt wrote:
coverity complains about a null-termination after a read,
so we terminate conditionally on whether idx is within
the buffer or at the end of the buffer.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/channel_monitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..711722fef 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -808,6 +808,7 @@ read_json_packet(struct channel_info *chan_info)
int indent = 0;
do {
n_bytes = read(chan_info->fd, &json_data[idx], 1);
+ json_data[idx + (idx < MAX_JSON_STRING_LEN - 1)] = '\0';
Why do it inside the loop and not after?
if (n_bytes == 0)
break;
if (json_data[idx] == '{')
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
Hi,
Looks like something in email format setting is affecting the style. Please
find my replies below
snipped
As per the current suggested code flow check is added to while loop in function
`dump_packets'.
Thanks for the reply. Since want to make it clean, the code was here.
However, it seems need to take care of the performance impact first.
Response> thanks for acknowledging the same.
Questions:
1. What is impact in performance with and without patch?
A1. Do a little trick as the patch below to tested the impact in the single
core mode on Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz with no pkts.
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 3d208548fa13..804011b187c4 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -141,7 +141,7 @@ struct parse_val {
static int num_tuples;
static struct rte_eth_conf port_conf_default;
-static volatile uint8_t quit_signal;
+static volatile uint32_t quit_signal;
static uint8_t multiple_core_capture;
/**< display usage */
@@ -868,6 +868,7 @@ struct parse_val {
dump_packets(void)
{
int i;
+ uint64_t start, end;
uint32_t lcore_id = 0;
if (!multiple_core_capture) {
@@ -880,10 +881,20 @@ struct parse_val {
pdump_t[i].device_id,
pdump_t[i].queue);
- while (!quit_signal) {
+ /* make it hot */
+ rte_eal_primary_proc_alive(NULL);
+ rte_eal_primary_proc_alive(NULL)
+
+ start = rte_rdtsc();
+ while (quit_signal < 5) {
+ /* Just testing with and w/o the 'if' line below */
+ if (rte_eal_primary_proc_alive(NULL))
+ quit_signal++;
for (i = 0; i < num_tuples; i++)
pdump_packets(&pdump_t[i]);
}
+ end = rte_rdtsc();
+ printf("Totally count:%u, cost tsc:%lu\n", quit_signal, end -
start);
return;
}
The total tsc cost is about 338809671 with rte_eal_primary_proc_alive().
And the tsc cost is just about 513573 without rte_eal_primary_proc_alive().
The dpdk-pdump had also used taskset to bind to specify isolate core.
So it seems the patch do a great performance impact.
Response> thanks for confirming the suspicion.
Maybe another async method should be introduced to monitor the primary status.
Response> yes, without affecting the capture thread.
2. For various packet sizes and port speed what are delta in drops for packet
capture?
A2. Refer to A1, it's not needed anymore.
Response> A1 there is performance impact.
Note: If pdump application is still alive when primary is not running, primary
cannot be started. Is this a cue that pdump is still alive and has to be
terminated?
Yes, some guys complained that the residual dpdk-pdump impact the restart of
the primary app and refuse to add other mechanisms e.g. to kill the dpdk-pdump
in the app to avoid that case.
So the patch was created.
Is there any other ways to avoid that.
Response> in my humble opinion, best way around is add user option like
‘--exit’; which then will add periodic rte_timer for user desired seconds ‘0.1,
0.5, 1.0, 5.0’. The timer callback can run on master core which sets
‘quit_signal’ once primary is no longer alive. In case of ‘multi thread’
capture master thread is not involved in dump_packets thus avoiding any packet
drops or performance issue..
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
I will leave this suggestion open for comments from the maintainer.
snipped
Hi,
Looks like something in email format setting is affecting the style. Please
find my replies below
snipped
As per the current suggested code flow check is added to while loop in function
`dump_packets'.
Thanks for the reply. Since want to make it clean, the code was here.
However, it seems need to take care of the performance impact first.
Response> thanks for acknowledging the same.
Questions:
1. What is impact in performance with and without patch?
A1. Do a little trick as the patch below to tested the impact in the single
core mode on Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz with no pkts.
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 3d208548fa13..804011b187c4 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -141,7 +141,7 @@ struct parse_val {
static int num_tuples;
static struct rte_eth_conf port_conf_default;
-static volatile uint8_t quit_signal;
+static volatile uint32_t quit_signal;
static uint8_t multiple_core_capture;
/**< display usage */
@@ -868,6 +868,7 @@ struct parse_val {
dump_packets(void)
{
int i;
+ uint64_t start, end;
uint32_t lcore_id = 0;
if (!multiple_core_capture) {
@@ -880,10 +881,20 @@ struct parse_val {
pdump_t[i].device_id,
pdump_t[i].queue);
- while (!quit_signal) {
+ /* make it hot */
+ rte_eal_primary_proc_alive(NULL);
+ rte_eal_primary_proc_alive(NULL)
+
+ start = rte_rdtsc();
+ while (quit_signal < 5) {
+ /* Just testing with and w/o the 'if' line below */
+ if (rte_eal_primary_proc_alive(NULL))
+ quit_signal++;
for (i = 0; i < num_tuples; i++)
pdump_packets(&pdump_t[i]);
}
+ end = rte_rdtsc();
+ printf("Totally count:%u, cost tsc:%lu\n", quit_signal, end -
start);
return;
}
The total tsc cost is about 338809671 with rte_eal_primary_proc_alive().
And the tsc cost is just about 513573 without rte_eal_primary_proc_alive().
The dpdk-pdump had also used taskset to bind to specify isolate core.
So it seems the patch do a great performance impact.
Response> thanks for confirming the suspicion.
Maybe another async method should be introduced to monitor the primary status.
Response> yes, without affecting the capture thread.
2. For various packet sizes and port speed what are delta in drops for packet
capture?
A2. Refer to A1, it's not needed anymore.
Response> A1 there is performance impact.
Note: If pdump application is still alive when primary is not running, primary
cannot be started. Is this a cue that pdump is still alive and has to be
terminated?
Yes, some guys complained that the residual dpdk-pdump impact the restart of
the primary app and refuse to add other mechanisms e.g. to kill the dpdk-pdump
in the app to avoid that case.
So the patch was created.
Is there any other ways to avoid that.
Response> in my humble opinion, best way around is add user option like
‘--exit’; which then will add periodic rte_timer for user desired seconds ‘0.1,
0.5, 1.0, 5.0’. The timer callback can run on master core which sets
‘quit_signal’ once primary is no longer alive. In case of ‘multi thread’
capture master thread is not involved in dump_packets thus avoiding any packet
drops or performance issue..
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix overflowed return value
Hi Anatoly,
On 26/4/2019 11:29 AM, Burakov, Anatoly wrote:
On 26-Apr-19 9:44 AM, David Hunt wrote:
Coverity complains about the return of a value that may
possibly overflow because of a multiply. Limit the value
so it cannot overflow.
Coverity issue: 337677
Fixes: 4b1a631b8a ("examples/vm_power: add oob monitoring functions")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/oob_monitor_x86.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/oob_monitor_x86.c
b/examples/vm_power_manager/oob_monitor_x86.c
index ebd96b205..2074eec1e 100644
--- a/examples/vm_power_manager/oob_monitor_x86.c
+++ b/examples/vm_power_manager/oob_monitor_x86.c
@@ -99,7 +99,10 @@ apply_policy(int core)
return -1.0;
}
- ratio = (float)miss_diff * (float)100 / (float)hits_diff;
+ ratio = (float)miss_diff / (float)hits_diff;
+ if (ratio > 1.0)
+ ratio = 1.0;
+ ratio *= 100.0f;
It should probably be the other way around - multiply first, then
clamp. Also, please use RTE_MIN.
I tried that, but coverity still sees an overflow condition. I need to
clamp first, then multiply. Then coverity is happy.
Also, do you really want me to change to use RTE_MIN? I honestly prefer
the code as it is.
if (ratio < ci->branch_ratio_threshold)
power_manager_scale_core_min(core);
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix string null termination
Hi Anatoly,
On 26/4/2019 11:33 AM, Burakov, Anatoly wrote:
On 26-Apr-19 9:43 AM, David Hunt wrote:
coverity complains about a null-termination after a read,
so we terminate conditionally on whether idx is within
the buffer or at the end of the buffer.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/channel_monitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..711722fef 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -808,6 +808,7 @@ read_json_packet(struct channel_info *chan_info)
int indent = 0;
do {
n_bytes = read(chan_info->fd, &json_data[idx], 1);
+ json_data[idx + (idx < MAX_JSON_STRING_LEN - 1)] = '\0';
Why do it inside the loop and not after?
No reason, really. I'll move it outside and re-spin.
if (n_bytes == 0)
break;
if (json_data[idx] == '{')
[dpdk-dev] [PATCH v2] examples/vm_power_manager: fix string null termination
coverity complains about a null-termination after a read,
so we terminate after exiting the do-while loop. The position
is conditional on whether idx is within the buffer or at the
end of the buffer.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
v2:
* Move null termination outside of do-while.
---
examples/vm_power_manager/channel_monitor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..03fdcd15a 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -822,6 +822,8 @@ read_json_packet(struct channel_info *chan_info)
break;
} while (indent > 0);
+ json_data[idx + (idx < MAX_JSON_STRING_LEN - 1)] = '\0';
+
if (indent > 0)
/*
* We've broken out of the read loop without getting
--
2.17.1
Re: [dpdk-dev] [PATCH v2] examples/vm_power_manager: fix string null termination
On 26-Apr-19 12:24 PM, David Hunt wrote:
coverity complains about a null-termination after a read,
so we terminate after exiting the do-while loop. The position
is conditional on whether idx is within the buffer or at the
end of the buffer.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
v2:
* Move null termination outside of do-while.
---
examples/vm_power_manager/channel_monitor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..03fdcd15a 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -822,6 +822,8 @@ read_json_packet(struct channel_info *chan_info)
break;
} while (indent > 0);
+ json_data[idx + (idx < MAX_JSON_STRING_LEN - 1)] = '\0';
+
I don't think you need this complicated logic here. You start at idx =
0, so even if you receive 0 bytes, you'll terminate buffer at index 0.
You also break when idx reaches (MAX_JSON_STRING_LEN - 1), so it's also
safe to do json_data[idx] after the loop. In all other cases, you still
increment idx before breaking out (e.g. when reaching indent == 0), so
it's also safe to do json_data[idx] in those cases.
if (indent > 0)
/*
* We've broken out of the read loop without getting
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix overflowed return value
On 26-Apr-19 12:14 PM, Hunt, David wrote:
Hi Anatoly,
On 26/4/2019 11:29 AM, Burakov, Anatoly wrote:
On 26-Apr-19 9:44 AM, David Hunt wrote:
Coverity complains about the return of a value that may
possibly overflow because of a multiply. Limit the value
so it cannot overflow.
Coverity issue: 337677
Fixes: 4b1a631b8a ("examples/vm_power: add oob monitoring functions")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/oob_monitor_x86.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/oob_monitor_x86.c
b/examples/vm_power_manager/oob_monitor_x86.c
index ebd96b205..2074eec1e 100644
--- a/examples/vm_power_manager/oob_monitor_x86.c
+++ b/examples/vm_power_manager/oob_monitor_x86.c
@@ -99,7 +99,10 @@ apply_policy(int core)
return -1.0;
}
- ratio = (float)miss_diff * (float)100 / (float)hits_diff;
+ ratio = (float)miss_diff / (float)hits_diff;
+ if (ratio > 1.0)
+ ratio = 1.0;
+ ratio *= 100.0f;
It should probably be the other way around - multiply first, then
clamp. Also, please use RTE_MIN.
I tried that, but coverity still sees an overflow condition. I need to
clamp first, then multiply. Then coverity is happy.
That's weird. This may be a bug in Coverity then. Please correct me if
i'm wrong, but floating point formats aren't precise, so by doing
multiplication on a value that doesn't exceed 1.0, you may very well end
up with a value that does exceed 100 by a tiny bit on account of
floating point approximations, rounding errors etc.
The question is, do we want correct code, or do we want to keep Coverity
happy? :) I'll have a look at the coverity issue itself, maybe i'm
missing something here...
Also, do you really want me to change to use RTE_MIN? I honestly prefer
the code as it is.
No strong opinion here.
if (ratio < ci->branch_ratio_threshold)
power_manager_scale_core_min(core);
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH v8 0/4] lib/rcu: add RCU library supporting QSBR mechanism
> -Original Message- > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com] > Sent: Friday, April 26, 2019 5:40 AM > To: Ananyev, Konstantin ; > step...@networkplumber.org; paul...@linux.ibm.com; Kovacevic, Marko > ; dev@dpdk.org > Cc: honnappa.nagaraha...@arm.com; gavin...@arm.com; dharmik.thak...@arm.com; > malvika.gu...@arm.com > Subject: [PATCH v8 0/4] lib/rcu: add RCU library supporting QSBR mechanism > > Lock-less data structures provide scalability and determinism. > They enable use cases where locking may not be allowed > (for ex: real-time applications). > > In the following paras, the term 'memory' refers to memory allocated > by typical APIs like malloc or anything that is representative of > memory, for ex: an index of a free element array. > > Since these data structures are lock less, the writers and readers > are accessing the data structures concurrently. Hence, while removing > an element from a data structure, the writers cannot return the memory > to the allocator, without knowing that the readers are not > referencing that element/memory anymore. Hence, it is required to > separate the operation of removing an element into 2 steps: > > Delete: in this step, the writer removes the reference to the element from > the data structure but does not return the associated memory to the > allocator. This will ensure that new readers will not get a reference to > the removed element. Removing the reference is an atomic operation. > > Free(Reclaim): in this step, the writer returns the memory to the > memory allocator, only after knowing that all the readers have stopped > referencing the deleted element. > > This library helps the writer determine when it is safe to free the > memory. > > This library makes use of thread Quiescent State (QS). QS can be > defined as 'any point in the thread execution where the thread does > not hold a reference to shared memory'. It is upto the application to > determine its quiescent state. Let us consider the following diagram: > > Time --> > > | | > RT1 $D1+++***D2*|**+++|+++**D3*$ > | | > RT2 $D1++|+**D2|***++**D3*$ > | | > RT3 $D1+++***|D2***|++**D2*$ > | | > |<--->| >Del | Free >| > Cannot free memory > during this period > (Grace Period) > > RTx - Reader thread > < and > - Start and end of while(1) loop > ***Dx*** - Reader thread is accessing the shared data structure Dx. >i.e. critical section. > +++ - Reader thread is not accessing any shared data structure. > i.e. non critical section or quiescent state. > Del - Point in time when the reference to the entry is removed using > atomic operation. > Free - Point in time when the writer can free the entry. > Grace Period - Time duration between Del and Free, during which memory cannot >be freed. > > As shown, thread RT1 accesses data structures D1, D2 and D3. When it is > accessing D2, if the writer has to remove an element from D2, the > writer cannot free the memory associated with that element immediately. > The writer can return the memory to the allocator only after the reader > stops referencing D2. In other words, reader thread RT1 has to enter > a quiescent state. > > Similarly, since thread RT3 is also accessing D2, writer has to wait till > RT3 enters quiescent state as well. > > However, the writer does not need to wait for RT2 to enter quiescent state. > Thread RT2 was not accessing D2 when the delete operation happened. > So, RT2 will not get a reference to the deleted entry. > > It can be noted that, the critical sections for D2 and D3 are quiescent states > for D1. i.e. for a given data structure Dx, any point in the thread execution > that does not reference Dx is a quiescent state. > > Since memory is not freed immediately, there might be a need for > provisioning of additional memory, depending on the application requirements. > > It is important to make sure that this library keeps the overhead of > identifying the end of grace period and subsequent freeing of memory, > to a minimum. The following paras explain how grace period and critical > section affect this overhead. > > The writer has to poll the readers to identify the end of grace period. > Polling introduces memory accesses and wastes CPU cycles. The memory > is not available for reuse during grace period. Longer grace periods > exasperate these conditions. > > The length of the critical section and the number of reader threads > is proportional to the duration of the grace period. K
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
On 2019/4/26 18:56, Varghese, Vipin wrote:
I will leave this suggestion open for comments from the maintainer.
Hi,
Thanks for your suggestion. I have also tried to add an slave core to
monitor the primary status this afternoon. It works.
I doubt if it can be add an new option as you suggested, but which will
also require people who complain the exiting to add an extra slave core
for that.
Please waiting for the new patch in one or two days.
snipped
Hi,
Looks like something in email format setting is affecting the style.
Please find my replies below
snipped
As per the current suggested code flow check is added to while loop in
function `dump_packets'.
Thanks for the reply. Since want to make it clean, the code was here.
However, it seems need to take care of the performance impact first.
Response> thanks for acknowledging the same.
Questions:
1. What is impact in performance with and without patch?
A1. Do a little trick as the patch below to tested the impact in the single
core mode on Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz with no pkts.
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 3d208548fa13..804011b187c4 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -141,7 +141,7 @@ struct parse_val {
static int num_tuples;
static struct rte_eth_conf port_conf_default;
-static volatile uint8_t quit_signal;
+static volatile uint32_t quit_signal;
static uint8_t multiple_core_capture;
/**< display usage */
@@ -868,6 +868,7 @@ struct parse_val {
dump_packets(void)
{
int i;
+ uint64_t start, end;
uint32_t lcore_id = 0;
if (!multiple_core_capture) {
@@ -880,10 +881,20 @@ struct parse_val {
pdump_t[i].device_id,
pdump_t[i].queue);
- while (!quit_signal) {
+ /* make it hot */
+ rte_eal_primary_proc_alive(NULL);
+ rte_eal_primary_proc_alive(NULL)
+
+ start = rte_rdtsc();
+ while (quit_signal < 5) {
+ /* Just testing with and w/o the 'if' line below */
+ if (rte_eal_primary_proc_alive(NULL))
+ quit_signal++;
for (i = 0; i < num_tuples; i++)
pdump_packets(&pdump_t[i]);
}
+ end = rte_rdtsc();
+ printf("Totally count:%u, cost tsc:%lu\n", quit_signal, end -
start);
return;
}
The total tsc cost is about 338809671 with rte_eal_primary_proc_alive().
And the tsc cost is just about 513573 without rte_eal_primary_proc_alive().
The dpdk-pdump had also used taskset to bind to specify isolate core.
So it seems the patch do a great performance impact.
Response> thanks for confirming the suspicion.
Maybe another async method should be introduced to monitor the primary status.
Response> yes, without affecting the capture thread.
2. For various packet sizes and port speed what are delta in drops for
packet capture?
A2. Refer to A1, it's not needed anymore.
Response> A1 there is performance impact.
Note: If pdump application is still alive when primary is not running, primary
cannot be started. Is this a cue that pdump is still alive and has to be
terminated?
Yes, some guys complained that the residual dpdk-pdump impact the restart of
the primary app and refuse to add other mechanisms e.g. to kill the dpdk-pdump
in the app to avoid that case.
So the patch was created.
Is there any other ways to avoid that.
Response> in my humble opinion, best way around is add user option
like ‘--exit’; which then will add periodic rte_timer for user desired
seconds ‘0.1, 0.5, 1.0, 5.0’. The timer callback can run on master
core which sets ‘quit_signal’ once primary is no longer alive. In case
of ‘multi thread’ capture master thread is not involved in
dump_packets thus avoiding any packet drops or performance issue..
Re: [dpdk-dev] [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> > > From: Ido Goshen
> > >
> > > 10BASE-T SFP+ copper transceivers become cheaper and popular So far
> > > those were blocked by ixgbe as “unsupported”.
> > > e.g.
> > > eth_ixgbe_dev_init(): Unsupported SFP+ Module
> > > eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
> > > EAL: Requested device :0a:00.0 cannot be used
> > >
> > > This patch expands the usage of allow_unsupported_sfp to be more
> > > general and makes ixgbe more tolerant to unknown SFPs
> >
> >
> > I don't think it is a good idea to change the base code to blindly allow
> > unknown SFPs.
> > Again in eth_ixgbe_dev_init() we do set
> > hw->allow_unsupported_sfp = 1;
> > so the function below will return success anyway,
>
> what's the reason to not allow unknown SFPs?
> as is they are explicitly blocked and not working anyway, why not give them a
> chance?
From my perspective the question should be opposite: why to allow it?
ixgbe base code is developed and maintained by Intel ND team for several
platforms.
It should be some good reason to change it inside DPDK project only.
As I said, in eth_ixgbe_dev_init() we already set hw->allow_unsupported_sfp =
1,
so unknown spf should be allowed by DPDK ixgbe PMD.
So what exact problem you are trying to solve here?
Konstantin
>
> More inputs
> 1. i40e already does support it (I didn't go deep into it but it just seems
> less strict on hw_init)
> 2. even with ixgbe it can work, because unsupported is only checked by
> ixgbe_init_hw
> so if the SFP is inserted after the app has started it does work
> kind of inconsistent
>
> >
> > >
> > > Signed-off-by: Ido Goshen
> > > ---
> > > drivers/net/ixgbe/base/ixgbe_phy.c | 22 +++---
> > > drivers/net/ixgbe/base/ixgbe_x550.c | 3 +++
> > > 2 files changed, 14 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > index dd118f9..ff96afc 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > @@ -1527,18 +1527,9 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > ixgbe_hw *hw)
> > > if (hw->phy.type == ixgbe_phy_sfp_intel) {
> > > status = IXGBE_SUCCESS;
> > > } else {
> > > - if (hw->allow_unsupported_sfp == true) {
> > > - EWARN(hw,
> > > - "WARNING: Intel (R)
> > Network Connections are quality tested using Intel (R) Ethernet
> > > Optics. "
> > > - "Using untested modules is
> > not supported and may cause unstable operation or damage
> > > to the module or the adapter. "
> > > - "Intel Corporation is not
> > responsible for any harm caused by using untested modules.\n");
> > > - status = IXGBE_SUCCESS;
> > > - } else {
> > > - DEBUGOUT("SFP+ module not
> > supported\n");
> > > - hw->phy.type =
> > > + hw->phy.type =
> > > ixgbe_phy_sfp_unsupported;
> > > - status =
> > IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > - }
> > > + status = IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > }
> > > } else {
> > > status = IXGBE_SUCCESS;
> > > @@ -1546,6 +1537,15 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > ixgbe_hw *hw)
> > > }
> > >
> > > out:
> > > + if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
> > > + hw->allow_unsupported_sfp) {
> > > + PMD_INIT_LOG(WARNING,
> > > + "WARNING: Intel (R) Network Connections
> > are quality tested using Intel (R) Ethernet Optics. "
> > > + "Using untested modules is not supported
> > and may cause unstable
> > > +operation or damage to the module or
> > > the adapter. "
> > > + "Intel Corporation is not responsible for any
> > harm caused by using untested modules.\n");
> > > + hw->phy.type = ixgbe_phy_unknown;
> > > + status = IXGBE_SUCCESS;
> > > + }
> > > return status;
> > >
> > > err_read_i2c_eeprom:
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > index a920a14..212d9a0 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > @@ -1539,6 +1539,9 @@ STATIC s32
> > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
> > > *linear = false;
> > > break;
> > > case ixgbe_sfp_type_unknown:
> > > + if (hw->allow_unsupported_sfp)
> > > + return IXGBE_SUCCESS;
> > > + /* fall through */
> > > case ixgbe_sfp_type_1g_cu_
Re: [dpdk-dev] [PATCH v2] examples/vm_power_manager: fix string null termination
On Fri, Apr 26, 2019 at 12:56:08PM +0100, Burakov, Anatoly wrote:
> On 26-Apr-19 12:24 PM, David Hunt wrote:
> > coverity complains about a null-termination after a read,
> > so we terminate after exiting the do-while loop. The position
> > is conditional on whether idx is within the buffer or at the
> > end of the buffer.
> >
> > Coverity issue: 337680
> > Fixes: a63504a90f ("examples/power: add JSON string handling")
> > CC: sta...@dpdk.org
> >
> > Signed-off-by: David Hunt
> >
> > ---
> > v2:
> > * Move null termination outside of do-while.
> > ---
> > examples/vm_power_manager/channel_monitor.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/examples/vm_power_manager/channel_monitor.c
> > b/examples/vm_power_manager/channel_monitor.c
> > index 971e4f2bc..03fdcd15a 100644
> > --- a/examples/vm_power_manager/channel_monitor.c
> > +++ b/examples/vm_power_manager/channel_monitor.c
> > @@ -822,6 +822,8 @@ read_json_packet(struct channel_info *chan_info)
> > break;
> > } while (indent > 0);
> > + json_data[idx + (idx < MAX_JSON_STRING_LEN - 1)] = '\0';
> > +
>
> I don't think you need this complicated logic here. You start at idx = 0, so
> even if you receive 0 bytes, you'll terminate buffer at index 0. You also
> break when idx reaches (MAX_JSON_STRING_LEN - 1), so it's also safe to do
> json_data[idx] after the loop. In all other cases, you still increment idx
> before breaking out (e.g. when reaching indent == 0), so it's also safe to
> do json_data[idx] in those cases.
>
+1 to that.
An alternative and simpler option might be to memset the who array to zero
before you start anyway.
/Bruce
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix overflowed return value
On 26-Apr-19 1:03 PM, Burakov, Anatoly wrote:
On 26-Apr-19 12:14 PM, Hunt, David wrote:
Hi Anatoly,
On 26/4/2019 11:29 AM, Burakov, Anatoly wrote:
On 26-Apr-19 9:44 AM, David Hunt wrote:
Coverity complains about the return of a value that may
possibly overflow because of a multiply. Limit the value
so it cannot overflow.
Coverity issue: 337677
Fixes: 4b1a631b8a ("examples/vm_power: add oob monitoring functions")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/oob_monitor_x86.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/oob_monitor_x86.c
b/examples/vm_power_manager/oob_monitor_x86.c
index ebd96b205..2074eec1e 100644
--- a/examples/vm_power_manager/oob_monitor_x86.c
+++ b/examples/vm_power_manager/oob_monitor_x86.c
@@ -99,7 +99,10 @@ apply_policy(int core)
return -1.0;
}
- ratio = (float)miss_diff * (float)100 / (float)hits_diff;
+ ratio = (float)miss_diff / (float)hits_diff;
+ if (ratio > 1.0)
+ ratio = 1.0;
+ ratio *= 100.0f;
It should probably be the other way around - multiply first, then
clamp. Also, please use RTE_MIN.
I tried that, but coverity still sees an overflow condition. I need to
clamp first, then multiply. Then coverity is happy.
That's weird. This may be a bug in Coverity then. Please correct me if
i'm wrong, but floating point formats aren't precise, so by doing
multiplication on a value that doesn't exceed 1.0, you may very well end
up with a value that does exceed 100 by a tiny bit on account of
floating point approximations, rounding errors etc.
The question is, do we want correct code, or do we want to keep Coverity
happy? :) I'll have a look at the coverity issue itself, maybe i'm
missing something here...
I think the real source of the problem is not that, and i believe
there's something wrong with Coverity's analysis here.
For some reason Coverity thinks that multiplying two floating point
values (100f and miss_diff converted to float) will result in /integer/
overflow (lolwut?), *and* it assumes that miss_diff is negative at that
point when it *can't* be, because if miss_diff was negative, we would've
done an early exit on line 77.
My guess is, this is the culprit:
"overflow: Multiply operation overflows on operands (float)miss_diff and
100f. Example values for operands: *100f = 268435456*, (float)miss_diff
= -2147483648."
The "100f = 268435456" part makes me suspect that Coverity somehow
thinks that "100f" is a variable name?
Also, do you really want me to change to use RTE_MIN? I honestly
prefer the code as it is.
No strong opinion here.
if (ratio < ci->branch_ratio_threshold)
power_manager_scale_core_min(core);
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH v2] examples/vm_power_manager: fix string null termination
On 26-Apr-19 1:31 PM, Bruce Richardson wrote:
On Fri, Apr 26, 2019 at 12:56:08PM +0100, Burakov, Anatoly wrote:
On 26-Apr-19 12:24 PM, David Hunt wrote:
coverity complains about a null-termination after a read,
so we terminate after exiting the do-while loop. The position
is conditional on whether idx is within the buffer or at the
end of the buffer.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
v2:
* Move null termination outside of do-while.
---
examples/vm_power_manager/channel_monitor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..03fdcd15a 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -822,6 +822,8 @@ read_json_packet(struct channel_info *chan_info)
break;
} while (indent > 0);
+ json_data[idx + (idx < MAX_JSON_STRING_LEN - 1)] = '\0';
+
I don't think you need this complicated logic here. You start at idx = 0, so
even if you receive 0 bytes, you'll terminate buffer at index 0. You also
break when idx reaches (MAX_JSON_STRING_LEN - 1), so it's also safe to do
json_data[idx] after the loop. In all other cases, you still increment idx
before breaking out (e.g. when reaching indent == 0), so it's also safe to
do json_data[idx] in those cases.
+1 to that.
An alternative and simpler option might be to memset the who array to zero
before you start anyway.
That'll cost us few extra cycles on a non-performance critical path full
of syscalls, surely we can't have that! :)
/Bruce
--
Thanks,
Anatoly
Re: [dpdk-dev] Editor Config
> On Apr 26, 2019, at 3:13 AM, Burakov, Anatoly > wrote: > > On 25-Apr-19 6:19 PM, Stephen Hemminger wrote: >> Systemd uses this and it looks like a useful addition to the DPDK source >> base. Especially since we now have Windows developers. >> https://editorconfig.org/ > > I have suggested using clang-format in the past, but this would be a good > step too. +1 for editorconfig and we need something like clang-format as well to complete the picture. > > -- > Thanks, > Anatoly Regards, Keith
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
On 26-Apr-19 1:08 PM, Suanming.Mou wrote: On 2019/4/26 18:56, Varghese, Vipin wrote: I will leave this suggestion open for comments from the maintainer. Hi, Thanks for your suggestion. I have also tried to add an slave core to monitor the primary status this afternoon. It works. I doubt if it can be add an new option as you suggested, but which will also require people who complain the exiting to add an extra slave core for that. Please waiting for the new patch in one or two days. You can use alarm API to check for this regularly. It's not like the interrupt thread is doing much anyway. Just set alarm to fire every N seconds, and that's it. -- Thanks, Anatoly
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix overflowed return value
On 26/4/2019 11:29 AM, Burakov, Anatoly wrote:
On 26-Apr-19 9:44 AM, David Hunt wrote:
Coverity complains about the return of a value that may
possibly overflow because of a multiply. Limit the value
so it cannot overflow.
Coverity issue: 337677
Fixes: 4b1a631b8a ("examples/vm_power: add oob monitoring functions")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
examples/vm_power_manager/oob_monitor_x86.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/oob_monitor_x86.c
b/examples/vm_power_manager/oob_monitor_x86.c
index ebd96b205..2074eec1e 100644
--- a/examples/vm_power_manager/oob_monitor_x86.c
+++ b/examples/vm_power_manager/oob_monitor_x86.c
@@ -99,7 +99,10 @@ apply_policy(int core)
return -1.0;
}
- ratio = (float)miss_diff * (float)100 / (float)hits_diff;
+ ratio = (float)miss_diff / (float)hits_diff;
+ if (ratio > 1.0)
+ ratio = 1.0;
+ ratio *= 100.0f;
It should probably be the other way around - multiply first, then
clamp. Also, please use RTE_MIN.
if (ratio < ci->branch_ratio_threshold)
power_manager_scale_core_min(core);
Anatoly and myself have spendt some time analysing the coverity issue
just now, and we have come to the conclusion that it's a false positive.
We also think it may be an issue with coverity, so for the moment I'll
mark the coverity issue as a false positive.
[dpdk-dev] [PATCH v3] examples/vm_power_manager: fix string null termination
coverity complains about a null-termination after a read,
so we terminate once we exit the do-while read loop.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
v2: Move null termination outside of do-while.
v3: Simplify null termimation
---
examples/vm_power_manager/channel_monitor.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..4a287109b 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -822,12 +822,7 @@ read_json_packet(struct channel_info *chan_info)
break;
} while (indent > 0);
- if (indent > 0)
- /*
-* We've broken out of the read loop without getting
-* a closing brace, so throw away the data
-*/
- json_data[idx] = 0;
+ json_data[idx] = '\0';
if (strlen(json_data) == 0)
continue;
--
2.17.1
Re: [dpdk-dev] [PATCH v3] examples/vm_power_manager: fix string null termination
On 26-Apr-19 3:04 PM, David Hunt wrote:
coverity complains about a null-termination after a read,
so we terminate once we exit the do-while read loop.
Coverity issue: 337680
Fixes: a63504a90f ("examples/power: add JSON string handling")
CC: sta...@dpdk.org
Signed-off-by: David Hunt
---
v2: Move null termination outside of do-while.
v3: Simplify null termimation
---
examples/vm_power_manager/channel_monitor.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/examples/vm_power_manager/channel_monitor.c
b/examples/vm_power_manager/channel_monitor.c
index 971e4f2bc..4a287109b 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -822,12 +822,7 @@ read_json_packet(struct channel_info *chan_info)
break;
} while (indent > 0);
- if (indent > 0)
- /*
-* We've broken out of the read loop without getting
-* a closing brace, so throw away the data
-*/
- json_data[idx] = 0;
+ json_data[idx] = '\0';
if (strlen(json_data) == 0)
continue;
Reviewed-by: Anatoly Burakov
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
On 2019/4/26 21:46, Burakov, Anatoly wrote:
On 26-Apr-19 1:08 PM, Suanming.Mou wrote:
On 2019/4/26 18:56, Varghese, Vipin wrote:
I will leave this suggestion open for comments from the maintainer.
Hi,
Thanks for your suggestion. I have also tried to add an slave core to
monitor the primary status this afternoon. It works.
I doubt if it can be add an new option as you suggested, but which
will also require people who complain the exiting to add an extra
slave core for that.
Please waiting for the new patch in one or two days.
You can use alarm API to check for this regularly. It's not like the
interrupt thread is doing much anyway. Just set alarm to fire every N
seconds, and that's it.
Hi,
Thank you very much for the suggestion. Yes, that seems the best
solution. Just tested it roughly as the code below:
+static void monitor_primary(void *arg __rte_unused)
+{
+ if (quit_signal)
+ return;
+
+ if (rte_eal_primary_proc_alive(NULL))
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+ else
+ quit_signal = 1;
+
+ return;
+}
+
static inline void
dump_packets(void)
{
int i;
uint32_t lcore_id = 0;
+ if (exit_with_primary)
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+
I will prepare the patch with option exit_with_primary.
Br,
Mou
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
On 26-Apr-19 3:32 PM, Suanming.Mou wrote:
On 2019/4/26 21:46, Burakov, Anatoly wrote:
On 26-Apr-19 1:08 PM, Suanming.Mou wrote:
On 2019/4/26 18:56, Varghese, Vipin wrote:
I will leave this suggestion open for comments from the maintainer.
Hi,
Thanks for your suggestion. I have also tried to add an slave core to
monitor the primary status this afternoon. It works.
I doubt if it can be add an new option as you suggested, but which
will also require people who complain the exiting to add an extra
slave core for that.
Please waiting for the new patch in one or two days.
You can use alarm API to check for this regularly. It's not like the
interrupt thread is doing much anyway. Just set alarm to fire every N
seconds, and that's it.
Hi,
Thank you very much for the suggestion. Yes, that seems the best
solution. Just tested it roughly as the code below:
+static void monitor_primary(void *arg __rte_unused)
+{
+ if (quit_signal)
+ return;
+
+ if (rte_eal_primary_proc_alive(NULL))
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+ else
+ quit_signal = 1;
+
+ return;
+}
+
static inline void
dump_packets(void)
{
int i;
uint32_t lcore_id = 0;
+ if (exit_with_primary)
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+
I will prepare the patch with option exit_with_primary.
Actually, i'm curious if this really does work. Unless my knowledge is
out of date, interrupt thread doesn't work in secondary processes, and
by extension neither should the alarm API...
--
Thanks,
Anatoly
[dpdk-dev] [PATCH] timer: fix reset/stop in callback for new API
The rte_timer_alt_manage function should track which is the running
timer and whether or not it was updated by a callback in the priv_timer
structure that corresponds to the running lcore, so that restarting
or stopping the timer from the callback works correctly.
Fixes: c0749f7096c7 ("timer: allow management in shared memory")
Signed-off-by: Erik Gabriel Carrillo
---
lib/librte_timer/rte_timer.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index d443b8c..9f2e921 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -830,7 +830,6 @@ rte_timer_alt_manage(uint32_t timer_data_id,
union rte_timer_status status;
struct rte_timer *tim, *next_tim, **pprev;
struct rte_timer *run_first_tims[RTE_MAX_LCORE];
- unsigned int runlist_lcore_ids[RTE_MAX_LCORE];
unsigned int this_lcore = rte_lcore_id();
struct rte_timer *prev[MAX_SKIPLIST_DEPTH + 1];
uint64_t cur_time;
@@ -899,7 +898,6 @@ rte_timer_alt_manage(uint32_t timer_data_id,
/* transition run-list from PENDING to RUNNING */
run_first_tims[nb_runlists] = tim;
- runlist_lcore_ids[nb_runlists] = poll_lcore;
pprev = &run_first_tims[nb_runlists];
nb_runlists++;
@@ -946,25 +944,24 @@ rte_timer_alt_manage(uint32_t timer_data_id,
break;
tim = run_first_tims[min_idx];
- privp = &data->priv_timer[runlist_lcore_ids[min_idx]];
/* Move down the runlist from which we picked a timer to
* execute
*/
run_first_tims[min_idx] = run_first_tims[min_idx]->sl_next[0];
- privp->updated = 0;
- privp->running_tim = tim;
+ data->priv_timer[this_lcore].updated = 0;
+ data->priv_timer[this_lcore].running_tim = tim;
/* Call the provided callback function */
f(tim);
- __TIMER_STAT_ADD(privp, pending, -1);
+ __TIMER_STAT_ADD(data->priv_timer, pending, -1);
/* the timer was stopped or reloaded by the callback
* function, we have nothing to do here
*/
- if (privp->updated == 1)
+ if (data->priv_timer[this_lcore].updated == 1)
continue;
if (tim->period == 0) {
@@ -989,7 +986,7 @@ rte_timer_alt_manage(uint32_t timer_data_id,
&data->priv_timer[this_lcore].list_lock);
}
- privp->running_tim = NULL;
+ data->priv_timer[this_lcore].running_tim = NULL;
}
return 0;
--
2.6.4
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
On 2019/4/26 22:39, Burakov, Anatoly wrote:
On 26-Apr-19 3:32 PM, Suanming.Mou wrote:
On 2019/4/26 21:46, Burakov, Anatoly wrote:
On 26-Apr-19 1:08 PM, Suanming.Mou wrote:
On 2019/4/26 18:56, Varghese, Vipin wrote:
I will leave this suggestion open for comments from the maintainer.
Hi,
Thanks for your suggestion. I have also tried to add an slave core
to monitor the primary status this afternoon. It works.
I doubt if it can be add an new option as you suggested, but which
will also require people who complain the exiting to add an extra
slave core for that.
Please waiting for the new patch in one or two days.
You can use alarm API to check for this regularly. It's not like the
interrupt thread is doing much anyway. Just set alarm to fire every
N seconds, and that's it.
Hi,
Thank you very much for the suggestion. Yes, that seems the best
solution. Just tested it roughly as the code below:
+static void monitor_primary(void *arg __rte_unused)
+{
+ if (quit_signal)
+ return;
+
+ if (rte_eal_primary_proc_alive(NULL))
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+ else
+ quit_signal = 1;
+
+ return;
+}
+
static inline void
dump_packets(void)
{
int i;
uint32_t lcore_id = 0;
+ if (exit_with_primary)
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+
I will prepare the patch with option exit_with_primary.
Actually, i'm curious if this really does work. Unless my knowledge is
out of date, interrupt thread doesn't work in secondary processes, and
by extension neither should the alarm API...
Uh... If I understand correctly, the alarm API has used in the secondary
before.
Refer to handle_primary_request()
Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
On 26-Apr-19 3:49 PM, Suanming.Mou wrote:
On 2019/4/26 22:39, Burakov, Anatoly wrote:
On 26-Apr-19 3:32 PM, Suanming.Mou wrote:
On 2019/4/26 21:46, Burakov, Anatoly wrote:
On 26-Apr-19 1:08 PM, Suanming.Mou wrote:
On 2019/4/26 18:56, Varghese, Vipin wrote:
I will leave this suggestion open for comments from the maintainer.
Hi,
Thanks for your suggestion. I have also tried to add an slave core
to monitor the primary status this afternoon. It works.
I doubt if it can be add an new option as you suggested, but which
will also require people who complain the exiting to add an extra
slave core for that.
Please waiting for the new patch in one or two days.
You can use alarm API to check for this regularly. It's not like the
interrupt thread is doing much anyway. Just set alarm to fire every
N seconds, and that's it.
Hi,
Thank you very much for the suggestion. Yes, that seems the best
solution. Just tested it roughly as the code below:
+static void monitor_primary(void *arg __rte_unused)
+{
+ if (quit_signal)
+ return;
+
+ if (rte_eal_primary_proc_alive(NULL))
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+ else
+ quit_signal = 1;
+
+ return;
+}
+
static inline void
dump_packets(void)
{
int i;
uint32_t lcore_id = 0;
+ if (exit_with_primary)
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+
I will prepare the patch with option exit_with_primary.
Actually, i'm curious if this really does work. Unless my knowledge is
out of date, interrupt thread doesn't work in secondary processes, and
by extension neither should the alarm API...
Uh... If I understand correctly, the alarm API has used in the secondary
before.
Refer to handle_primary_request()
Then my knowledge really is out of date :)
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH] app/test: enhance meson test run for FreeBSD and Linux
> -Original Message- > From: Babu Radhakrishnan, AgalyaX > Sent: Wednesday, March 6, 2019 9:20 AM > To: dev@dpdk.org > Cc: Pattan, Reshma ; Richardson, Bruce > ; Babu Radhakrishnan, AgalyaX > > Subject: [PATCH] app/test: enhance meson test run for FreeBSD and Linux > > From: Agalya Babu RadhaKrishnan > > 1)For linux, running all tests on same cores can increase failure rate. > So instead run each test case on separate lcore using -l EAL option. > Lcore for each test case will be selected in the incremental order of 0,1,2 > ... till it > reaches the maximum available cores in the system which can be known from > 'nproc' command. > On the other hand, by default meson runs all tests in parallel in different > threads > which can be controlled by MESON_TESTTHREAD option. > eg. MESON_TESTTHREAD=4 meson test --suite DPDK:fast-tests > > 2)FreeBSD dont support file-prefix, so changes are made to run the tests > without 'file-prefix'option. > eg. meson test --suite DPDK:fast-tests > > Signed-off-by: Agalya Babu RadhaKrishnan > Nacking this patch , as the similar patch http://patches.dpdk.org/patch/52730/ has been accepted. Nacked-by: Reshma Pattan
Re: [dpdk-dev] [PATCH 3/4] devtools/test-meson-builds: add testing of pkg-config file
On Wed, Apr 24, 2019 at 02:37:58PM +0100, Luca Boccassi wrote: > On Wed, 2019-04-24 at 13:31 +0100, Bruce Richardson wrote: > > On Wed, Apr 24, 2019 at 12:02:24PM +0100, Luca Boccassi wrote: > > > On Wed, 2019-04-24 at 11:41 +0100, Bruce Richardson wrote: > > > > On Wed, Apr 24, 2019 at 10:22:04AM +0100, Luca Boccassi wrote: > > > > > On Tue, 2019-04-23 at 23:06 +0100, Bruce Richardson wrote: > > > > > > The pkg-config file generated as part of the build of DPDK > > > > > > should > > > > > > allow > > > > > > applications to be built with an installed DPDK. We can test > > > > > > this > > > > > > as > > > > > > part of the build by doing an install of DPDK to a temporary > > > > > > directory > > > > > > within the build folder, and by then compiling up a few > > > > > > sample > > > > > > apps > > > > > > using make working off that directory. > > > > > > > > > > > > Signed-off-by: Bruce Richardson < > > > > > > bruce.richard...@intel.com > > > > > > > > > > > > > > > > > > > > > > > > --- devtools/test-meson-builds.sh | 17 + 1 > > > > > > file > > > > > > changed, 17 insertions(+) > > > > > > > > > > > > diff --git a/devtools/test-meson-builds.sh b/devtools/test- > > > > > > meson- > > > > > > builds.sh index 630a1a6fe..dfba2a782 100755 --- > > > > > > a/devtools/test-meson-builds.sh +++ b/devtools/test-meson- > > > > > > builds.sh @@ > > > > > > -90,3 +90,20 @@ if command -v $c >/dev/null 2>&1 ; then > > > > > > $use_shared > > > > > > --cross-file $f done fi + +## +# Test > > > > > > installation of > > > > > > the > > > > > > x86-default target, to be used for checking +# the sample > > > > > > apps > > > > > > build > > > > > > using the pkg-config file for cflags and libs > > > > > > +### > > > > > > +build_path=build-x86-default > > > > > > +DESTDIR=`pwd`/$build_path/install- > > > > > > root ; > > > > > > export DESTDIR > > > > > > +PKG_CONFIG_PATH=$DESTDIR/usr/local/lib64/pkgconfig ; > > > > > > export PKG_CONFIG_PATH +$ninja_cmd -C $build_path install + > > > > > > +# > > > > > > rather > > > > > > than hacking our environment, just edit the .pc file prefix > > > > > > value > > > > > > +sed > > > > > > -i "s|prefix=|prefix=$DESTDIR|" $PKG_CONFIG_PATH/libdpdk.pc > > > > > > > > > > What about just using meson's prefix option instead? Which is > > > > > how > > > > > it > > > > > would be used in a real use case > > > > > > > > > > > > > I don't think that would fully work, as my understanding is that > > > > the > > > > prefix > > > > option would apply only to the /usr/local parts, but not to the > > > > kernel > > > > modules which would still try and install in /lib/. > > > > > > > > /Bruce > > > > > > What about doing a meson configure -Denable_kmods=false before the > > > ninja install? The modules are not needed for that test anyway, > > > right? > > > Alternatively, the kernel src dir could be symlinked in the build > > > path, > > > and the kernel_dir option could be used > > > > > > I'm just worried that the test should be as "realistic" as > > > possible, to > > > avoid missing something > > > > > > > Yes, I did think of that too, but it does mean doing another > > configuration > > run in meson, and possibly a rebuild too if the rte_build_config.h > > file > > changes. Therefore I decided to use DESTDIR for the sake of speed > > here. I > > assumed there would be a pkg-config variable to adjust the output > > paths > > based on a sysroot, but couldn't find a suitable one. > > > > In any case, I'll see about changing things as you suggest in V2 - > > correctness is more important that speed here. > > > > /Bruce > > There actually is a pkg-config binary option, I just tried and it works > (it seems to be disabled by default on Debian and derivatives): -- > define-prefix > Any cmdline options to pkg-config don't solve the problem here as it means that the makefiles for any app need to be modified to use all those. Also, I've been looking at the option you suggest of disabling the kernel modules for the install step - the problem that this brings is that it either: * disables them permanently for the default build, meaning subsequent runs may fail to catch errors * causes us to constantly reconfigure the build directory with/without the kmod setting, causing unnecessary work and slowdown in the script. A third solution is to use a separate build folder for the pkg-config test builds, but I think we have enough builds already in the setup without adding another one. All-in-all, I feel at this point that the original solution of making a small change to the pkg-config file manually is the best solution for now. I don't see it as being terribly fragile, and it should catch 95% of problems with the pkg-config files. I suggest that any rework be looked at in a later set to improve things. Regards, /Bruce
[dpdk-dev] [PATCH v2 1/2] doc: fix spelling errors reported by aspell
Fix spelling errors in the guide docs. Signed-off-by: John McNamara --- doc/guides/compressdevs/overview.rst | 2 +- doc/guides/contributing/patches.rst| 2 +- doc/guides/cryptodevs/aesni_mb.rst | 2 +- doc/guides/cryptodevs/overview.rst | 2 +- doc/guides/cryptodevs/scheduler.rst| 2 +- doc/guides/eventdevs/opdl.rst | 2 +- doc/guides/eventdevs/sw.rst| 4 ++-- doc/guides/howto/lm_bond_virtio_sriov.rst | 2 +- doc/guides/howto/lm_virtio_vhost_user.rst | 4 ++-- doc/guides/howto/rte_flow.rst | 6 ++--- doc/guides/howto/telemetry.rst | 2 +- .../howto/virtio_user_as_exceptional_path.rst | 8 +++ doc/guides/nics/af_packet.rst | 4 ++-- doc/guides/nics/atlantic.rst | 2 +- doc/guides/nics/cxgbe.rst | 4 ++-- doc/guides/nics/dpaa.rst | 2 +- doc/guides/nics/dpaa2.rst | 2 +- doc/guides/nics/ena.rst| 2 +- doc/guides/nics/enetc.rst | 2 +- doc/guides/nics/enic.rst | 2 +- doc/guides/nics/features.rst | 2 +- doc/guides/nics/i40e.rst | 2 +- doc/guides/nics/ixgbe.rst | 4 ++-- doc/guides/nics/kni.rst| 2 +- doc/guides/nics/mlx4.rst | 2 +- doc/guides/nics/mlx5.rst | 4 ++-- doc/guides/nics/mvpp2.rst | 2 +- doc/guides/nics/netvsc.rst | 2 +- doc/guides/nics/nfb.rst| 2 +- doc/guides/nics/nfp.rst| 4 ++-- doc/guides/nics/sfc_efx.rst| 14 +-- doc/guides/nics/szedata2.rst | 2 +- doc/guides/nics/tap.rst| 2 +- doc/guides/platform/dpaa.rst | 4 ++-- doc/guides/platform/dpaa2.rst | 4 ++-- doc/guides/prog_guide/bbdev.rst| 4 ++-- doc/guides/prog_guide/compressdev.rst | 6 ++--- doc/guides/prog_guide/cryptodev_lib.rst| 12 +- doc/guides/prog_guide/dev_kit_build_system.rst | 2 +- doc/guides/prog_guide/efd_lib.rst | 2 +- doc/guides/prog_guide/env_abstraction_layer.rst| 2 +- .../prog_guide/event_ethernet_rx_adapter.rst | 6 ++--- doc/guides/prog_guide/eventdev.rst | 6 ++--- doc/guides/prog_guide/ipsec_lib.rst| 16 ++--- doc/guides/prog_guide/kernel_nic_interface.rst | 2 +- doc/guides/prog_guide/metrics_lib.rst | 2 +- doc/guides/prog_guide/multi_proc_support.rst | 2 +- doc/guides/prog_guide/profile_app.rst | 4 ++-- doc/guides/prog_guide/rte_flow.rst | 8 +++ doc/guides/prog_guide/rte_security.rst | 20 doc/guides/prog_guide/traffic_management.rst | 2 +- doc/guides/prog_guide/vhost_lib.rst| 2 +- doc/guides/rawdevs/ifpga_rawdev.rst| 2 +- doc/guides/rel_notes/known_issues.rst | 8 +++ doc/guides/rel_notes/release_17_11.rst | 10 doc/guides/sample_app_ug/bbdev_app.rst | 4 ++-- doc/guides/sample_app_ug/eventdev_pipeline.rst | 2 +- doc/guides/sample_app_ug/intro.rst | 2 +- doc/guides/sample_app_ug/ip_pipeline.rst | 4 ++-- doc/guides/sample_app_ug/ipsec_secgw.rst | 8 +++ doc/guides/sample_app_ug/performance_thread.rst| 4 ++-- doc/guides/sample_app_ug/qos_metering.rst | 2 +- doc/guides/sample_app_ug/test_pipeline.rst | 2 +- doc/guides/sample_app_ug/vhost.rst | 4 ++-- doc/guides/sample_app_ug/vhost_scsi.rst| 2 +- doc/guides/sample_app_ug/vm_power_management.rst | 10 doc/guides/testpmd_app_ug/run_app.rst | 10 doc/guides/testpmd_app_ug/testpmd_funcs.rst| 28 +++--- doc/guides/tools/cryptoperf.rst| 22 - doc/guides/tools/proc_info.rst | 6 ++--- doc/guides/tools/testbbdev.rst | 2 +- 71 files changed, 170 insertions(+), 170 deletions(-) diff --git a/doc/guides/compressdevs/overview.rst b/doc/guides/compressdevs/overview.rst index 70bbe82..809e4e6 100644 --- a/doc/guides/compressdevs/overview.rst +++ b/doc/guides/compressdevs/overview.rst @@ -18,7 +18,7 @@ Supported Feature Flags without making any modifications to it (no compression done). - "OOP SGL In SGL Out" feature flag stands for - "Out-of-place Scatter-gather list Input, Scatter-gater list Output", +
[dpdk-dev] [PATCH v2 2/2] doc: fix spelling errors reported by aspell
Fix spelling errors in the doxygen docs. Signed-off-by: John McNamara --- drivers/common/qat/qat_adf/icp_qat_fw_mmp_ids.h| 2 +- drivers/common/qat/qat_adf/icp_qat_fw_pke.h| 2 +- drivers/event/opdl/opdl_ring.h | 8 drivers/net/i40e/rte_pmd_i40e.h| 4 ++-- drivers/net/nfp/nfp_net.c | 18 +- drivers/net/nfp/nfp_net_pmd.h | 4 ++-- drivers/raw/dpaa2_qdma/rte_pmd_dpaa2_qdma.h| 6 +++--- examples/performance-thread/common/lthread_api.h | 10 +- lib/librte_acl/acl_vect.h | 4 ++-- lib/librte_bbdev/rte_bbdev.h | 4 ++-- lib/librte_bpf/rte_bpf.h | 6 +++--- lib/librte_bpf/rte_bpf_ethdev.h| 4 ++-- lib/librte_cryptodev/rte_crypto_asym.h | 12 ++-- lib/librte_cryptodev/rte_cryptodev.h | 2 +- lib/librte_distributor/rte_distributor_private.h | 2 +- lib/librte_eal/common/include/generic/rte_cycles.h | 2 +- lib/librte_eal/common/include/rte_class.h | 2 +- lib/librte_eal/common/include/rte_common.h | 4 ++-- lib/librte_eal/common/include/rte_eal.h| 4 ++-- lib/librte_eal/common/include/rte_log.h| 2 +- lib/librte_eal/common/include/rte_service.h| 2 +- lib/librte_eal/common/include/rte_tailq.h | 2 +- lib/librte_eal/common/include/rte_uuid.h | 4 ++-- lib/librte_eal/common/include/rte_vfio.h | 2 +- lib/librte_eal/linux/eal/eal_memory.c | 2 +- lib/librte_efd/rte_efd.h | 2 +- lib/librte_ethdev/rte_eth_ctrl.h | 2 +- lib/librte_ethdev/rte_ethdev.h | 8 lib/librte_ethdev/rte_ethdev_core.h| 4 ++-- lib/librte_ethdev/rte_ethdev_driver.h | 2 +- lib/librte_ethdev/rte_tm.h | 8 lib/librte_eventdev/rte_event_crypto_adapter.h | 2 +- lib/librte_eventdev/rte_event_eth_rx_adapter.h | 4 ++-- lib/librte_eventdev/rte_eventdev.h | 4 ++-- lib/librte_eventdev/rte_eventdev_pmd.h | 2 +- lib/librte_flow_classify/rte_flow_classify.h | 4 ++-- lib/librte_hash/rte_hash.h | 2 +- lib/librte_ip_frag/rte_ip_frag.h | 2 +- lib/librte_ipsec/crypto.h | 2 +- lib/librte_ipsec/misc.h| 2 +- lib/librte_ipsec/rte_ipsec_group.h | 2 +- lib/librte_ipsec/rte_ipsec_sa.h| 2 +- lib/librte_latencystats/rte_latencystats.h | 2 +- lib/librte_lpm/rte_lpm.h | 2 +- lib/librte_mbuf/rte_mbuf.h | 8 lib/librte_mbuf/rte_mbuf_ptype.h | 2 +- lib/librte_mempool/rte_mempool.h | 4 ++-- lib/librte_pipeline/rte_table_action.h | 2 +- lib/librte_power/rte_power_empty_poll.h| 2 +- lib/librte_rawdev/rte_rawdev.h | 6 +++--- lib/librte_rawdev/rte_rawdev_pmd.h | 8 lib/librte_reorder/rte_reorder.h | 2 +- lib/librte_ring/rte_ring.h | 2 +- lib/librte_sched/rte_sched.h | 2 +- lib/librte_security/rte_security.h | 4 ++-- lib/librte_table/rte_table_hash.h | 6 +++--- lib/librte_vhost/rte_vhost.h | 4 ++-- 57 files changed, 112 insertions(+), 112 deletions(-) diff --git a/drivers/common/qat/qat_adf/icp_qat_fw_mmp_ids.h b/drivers/common/qat/qat_adf/icp_qat_fw_mmp_ids.h index d9a42dd..00813cf 100644 --- a/drivers/common/qat/qat_adf/icp_qat_fw_mmp_ids.h +++ b/drivers/common/qat/qat_adf/icp_qat_fw_mmp_ids.h @@ -8,7 +8,7 @@ * @brief * This file documents the external interfaces that the QAT FW running * on the QAT Acceleration Engine provides to clients wanting to - * accelerate crypto assymetric applications + * accelerate crypto asymmetric applications */ #ifndef _ICP_QAT_FW_MMP_IDS_ diff --git a/drivers/common/qat/qat_adf/icp_qat_fw_pke.h b/drivers/common/qat/qat_adf/icp_qat_fw_pke.h index 1c1560a..b2cdf0a 100644 --- a/drivers/common/qat/qat_adf/icp_qat_fw_pke.h +++ b/drivers/common/qat/qat_adf/icp_qat_fw_pke.h @@ -10,7 +10,7 @@ * @brief * This file documents the external interfaces that the QAT FW running * on the QAT Acceleration Engine provides to clients wanting to - * accelerate crypto assymetric applications + * accelerate crypto asymmetric applications */ #ifndef _ICP_QAT_FW_PKE_H_ diff --git a/drivers/event/opdl/opdl_ring.h b/drivers/event/opdl/opdl_ring.h index 751a59d..14ababe 100644 --- a/drivers/event/opdl/opdl_ring.h +++ b/drivers/event/opdl/opdl_ring.h @@ -24,7 +24,7 @@ * packets. * * A opd
[dpdk-dev] [PATCH v6 0/1] New software event timer adapter
This patch introduces a new version of the event timer adapter software PMD [1]. In the original design, timer event producer lcores in the primary and secondary processes enqueued event timers into a ring, and a service core in the primary process dequeued them and processed them further. To improve performance, this version does away with the ring and lets lcores in both primary and secondary processes insert timers directly into timer skiplist data structures; the service core directly accesses the lists as well, when looking for timers that have expired. [1] https://doc.dpdk.org/guides/prog_guide/event_timer_adapter.html Changes in v6: - Fix implicit type conversion bug that caused full event buffer to sometimes not be correctly detected, resulting in lost events - Check return value of alt_timer_reset when resetting timer in event buffer full condition - Add timer list corresponding to service core to set of lists to scan when timers are reset by service core in event buffer full condition Changes in v5: - Rebase patch to apply with latest timer library - Fix event buffering bug where full buffer was treated as empty - Return rte_timer objects back to mempool after service function has returned from timer_manage() call instead of in callback Changes in v4: - Addressed the following comments from Mattias Ronnblom: - remove unnecessary header include - add missing read barrier in timer cancel function Changes in v3: - Addressed comments from Mattias Ronnblom: - remove unnecessary header include - remove unnecessary cast in mempool_put() call - update alignment of elements of array to avoid false sharing issue Changes in v2: - split this change out into its own patch series Erik Gabriel Carrillo (1): eventdev: add new software event timer adapter lib/librte_eventdev/rte_event_timer_adapter.c | 733 +++--- 1 file changed, 312 insertions(+), 421 deletions(-) -- 2.6.4
[dpdk-dev] [PATCH v6 1/1] eventdev: add new software event timer adapter
This patch introduces a new version of the event timer adapter software
PMD. In the original design, timer event producer lcores in the primary
and secondary processes enqueued event timers into a ring, and a
service core in the primary process dequeued them and processed them
further. To improve performance, this version does away with the ring
and lets lcores in both primary and secondary processes insert timers
directly into timer skiplist data structures; the service core directly
accesses the lists as well, when looking for timers that have expired.
Signed-off-by: Erik Gabriel Carrillo
---
lib/librte_eventdev/rte_event_timer_adapter.c | 733 +++---
1 file changed, 312 insertions(+), 421 deletions(-)
diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c
b/lib/librte_eventdev/rte_event_timer_adapter.c
index 2f7a760..e9104f8 100644
--- a/lib/librte_eventdev/rte_event_timer_adapter.c
+++ b/lib/librte_eventdev/rte_event_timer_adapter.c
@@ -34,7 +34,7 @@ static int evtim_buffer_logtype;
static struct rte_event_timer_adapter
adapters[RTE_EVENT_TIMER_ADAPTER_NUM_MAX];
-static const struct rte_event_timer_adapter_ops sw_event_adapter_timer_ops;
+static const struct rte_event_timer_adapter_ops swtim_ops;
#define EVTIM_LOG(level, logtype, ...) \
rte_log(RTE_LOG_ ## level, logtype, \
@@ -211,7 +211,7 @@ rte_event_timer_adapter_create_ext(
* implementation.
*/
if (adapter->ops == NULL)
- adapter->ops = &sw_event_adapter_timer_ops;
+ adapter->ops = &swtim_ops;
/* Allow driver to do some setup */
FUNC_PTR_OR_NULL_RET_WITH_ERRNO(adapter->ops->init, -ENOTSUP);
@@ -340,7 +340,7 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
* implementation.
*/
if (adapter->ops == NULL)
- adapter->ops = &sw_event_adapter_timer_ops;
+ adapter->ops = &swtim_ops;
/* Set fast-path function pointers */
adapter->arm_burst = adapter->ops->arm_burst;
@@ -428,8 +428,8 @@ rte_event_timer_adapter_stats_reset(struct
rte_event_timer_adapter *adapter)
#define EVENT_BUFFER_MASK (EVENT_BUFFER_SZ - 1)
struct event_buffer {
- uint16_t head;
- uint16_t tail;
+ size_t head;
+ size_t tail;
struct rte_event events[EVENT_BUFFER_SZ];
} __rte_cache_aligned;
@@ -455,7 +455,7 @@ event_buffer_init(struct event_buffer *bufp)
static int
event_buffer_add(struct event_buffer *bufp, struct rte_event *eventp)
{
- uint16_t head_idx;
+ size_t head_idx;
struct rte_event *buf_eventp;
if (event_buffer_full(bufp))
@@ -477,13 +477,16 @@ event_buffer_flush(struct event_buffer *bufp, uint8_t
dev_id, uint8_t port_id,
uint16_t *nb_events_flushed,
uint16_t *nb_events_inv)
{
- uint16_t head_idx, tail_idx, n = 0;
struct rte_event *events = bufp->events;
+ size_t head_idx, tail_idx;
+ uint16_t n = 0;
/* Instead of modulus, bitwise AND with mask to get index. */
head_idx = bufp->head & EVENT_BUFFER_MASK;
tail_idx = bufp->tail & EVENT_BUFFER_MASK;
+ RTE_ASSERT(head_idx < EVENT_BUFFER_SZ && tail_idx < EVENT_BUFFER_SZ);
+
/* Determine the largest contigous run we can attempt to enqueue to the
* event device.
*/
@@ -491,150 +494,155 @@ event_buffer_flush(struct event_buffer *bufp, uint8_t
dev_id, uint8_t port_id,
n = head_idx - tail_idx;
else if (head_idx < tail_idx)
n = EVENT_BUFFER_SZ - tail_idx;
+ else if (event_buffer_full(bufp))
+ n = EVENT_BUFFER_SZ - tail_idx;
else {
*nb_events_flushed = 0;
return;
}
+ n = RTE_MIN(EVENT_BUFFER_BATCHSZ, n);
*nb_events_inv = 0;
+
*nb_events_flushed = rte_event_enqueue_burst(dev_id, port_id,
&events[tail_idx], n);
- if (*nb_events_flushed != n && rte_errno == -EINVAL) {
- EVTIM_LOG_ERR("failed to enqueue invalid event - dropping it");
- (*nb_events_inv)++;
+ if (*nb_events_flushed != n) {
+ if (rte_errno == -EINVAL) {
+ EVTIM_LOG_ERR("failed to enqueue invalid event - "
+ "dropping it");
+ (*nb_events_inv)++;
+ } else if (rte_errno == -ENOSPC)
+ rte_pause();
}
+ if (*nb_events_flushed > 0)
+ EVTIM_BUF_LOG_DBG("enqueued %"PRIu16" timer events to event "
+ "device", *nb_events_flushed);
+
bufp->tail = bufp->tail + *nb_events_flushed + *nb_events_inv;
}
/*
* Software event timer adapter implementation
*/
-
-struct rte_event_timer_adapter_sw_data {
- /* List of messages for outstanding timers */
- TAILQ_HEAD(, msg) msgs_tailq_
Re: [dpdk-dev] [PATCH 3/4] devtools/test-meson-builds: add testing of pkg-config file
On Fri, 2019-04-26 at 15:56 +0100, Bruce Richardson wrote: > On Wed, Apr 24, 2019 at 02:37:58PM +0100, Luca Boccassi wrote: > > On Wed, 2019-04-24 at 13:31 +0100, Bruce Richardson wrote: > > > On Wed, Apr 24, 2019 at 12:02:24PM +0100, Luca Boccassi wrote: > > > > On Wed, 2019-04-24 at 11:41 +0100, Bruce Richardson wrote: > > > > > On Wed, Apr 24, 2019 at 10:22:04AM +0100, Luca Boccassi > > > > > wrote: > > > > > > On Tue, 2019-04-23 at 23:06 +0100, Bruce Richardson wrote: > > > > > > > The pkg-config file generated as part of the build of > > > > > > > DPDK > > > > > > > should > > > > > > > allow > > > > > > > applications to be built with an installed DPDK. We can > > > > > > > test > > > > > > > this > > > > > > > as > > > > > > > part of the build by doing an install of DPDK to a > > > > > > > temporary > > > > > > > directory > > > > > > > within the build folder, and by then compiling up a few > > > > > > > sample > > > > > > > apps > > > > > > > using make working off that directory. > > > > > > > > > > > > > > Signed-off-by: Bruce Richardson < > > > > > > > bruce.richard...@intel.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- devtools/test-meson-builds.sh | 17 + > > > > > > > 1 > > > > > > > file > > > > > > > changed, 17 insertions(+) > > > > > > > > > > > > > > diff --git a/devtools/test-meson-builds.sh > > > > > > > b/devtools/test- > > > > > > > meson- > > > > > > > builds.sh index 630a1a6fe..dfba2a782 100755 --- > > > > > > > a/devtools/test-meson-builds.sh +++ b/devtools/test- > > > > > > > meson- > > > > > > > builds.sh @@ > > > > > > > -90,3 +90,20 @@ if command -v $c >/dev/null 2>&1 ; then > > > > > > > $use_shared > > > > > > > --cross-file $f done fi + +## +# Test > > > > > > > installation of > > > > > > > the > > > > > > > x86-default target, to be used for checking +# the sample > > > > > > > apps > > > > > > > build > > > > > > > using the pkg-config file for cflags and libs > > > > > > > +### > > > > > > > +build_path=build-x86-default > > > > > > > +DESTDIR=`pwd`/$build_path/install- > > > > > > > root ; > > > > > > > export DESTDIR > > > > > > > +PKG_CONFIG_PATH=$DESTDIR/usr/local/lib64/pkgconfig ; > > > > > > > export PKG_CONFIG_PATH +$ninja_cmd -C $build_path install > > > > > > > + > > > > > > > +# > > > > > > > rather > > > > > > > than hacking our environment, just edit the .pc file > > > > > > > prefix > > > > > > > value > > > > > > > +sed > > > > > > > -i "s|prefix=|prefix=$DESTDIR|" > > > > > > > $PKG_CONFIG_PATH/libdpdk.pc > > > > > > > > > > > > What about just using meson's prefix option instead? Which > > > > > > is > > > > > > how > > > > > > it > > > > > > would be used in a real use case > > > > > > > > > > > > > > > > I don't think that would fully work, as my understanding is > > > > > that > > > > > the > > > > > prefix > > > > > option would apply only to the /usr/local parts, but not to > > > > > the > > > > > kernel > > > > > modules which would still try and install in /lib/. > > > > > > > > > > /Bruce > > > > > > > > What about doing a meson configure -Denable_kmods=false before > > > > the > > > > ninja install? The modules are not needed for that test anyway, > > > > right? > > > > Alternatively, the kernel src dir could be symlinked in the > > > > build > > > > path, > > > > and the kernel_dir option could be used > > > > > > > > I'm just worried that the test should be as "realistic" as > > > > possible, to > > > > avoid missing something > > > > > > > > > > Yes, I did think of that too, but it does mean doing another > > > configuration > > > run in meson, and possibly a rebuild too if the > > > rte_build_config.h > > > file > > > changes. Therefore I decided to use DESTDIR for the sake of speed > > > here. I > > > assumed there would be a pkg-config variable to adjust the output > > > paths > > > based on a sysroot, but couldn't find a suitable one. > > > > > > In any case, I'll see about changing things as you suggest in V2 > > > - > > > correctness is more important that speed here. > > > > > > /Bruce > > > > There actually is a pkg-config binary option, I just tried and it > > works > > (it seems to be disabled by default on Debian and derivatives): -- > > define-prefix > > > > Any cmdline options to pkg-config don't solve the problem here as it > means > that the makefiles for any app need to be modified to use all those. > > Also, I've been looking at the option you suggest of disabling the > kernel > modules for the install step - the problem that this brings is that > it either: > * disables them permanently for the default build, meaning subsequent > runs > may fail to catch errors > * causes us to constantly reconfigure the build directory > with/without > the kmod setting, causing unnecessary work and slowdown in the > script. > > A third solution is to use a separate build folder for the pkg-config > test > builds, but I think we have enough bu
Re: [dpdk-dev] [PATCH 0/4] add testing of libdpdk pkg-config file
On Tue, 2019-04-23 at 23:06 +0100, Bruce Richardson wrote: > As part of the meson build, a pkg-config file for libdpdk is created, > which > allows apps to be compiled and linked against DPDK by taking the > cflags and > lib parameter from pkgconfig. The example app makefiles have been > reworked > to take account of this support, but the build of them against the > .pc file > was not regularly tested. > > To rectify this, and give us greater confidence in the correctness of > the > .pc file, this set adds in the sample apps to the installation set > for > "ninja install" and then builds a subset of those apps against the > pkg-config file to test it. In the process a small error when > compiling > the cmdline sample app using the .pc file was fixed. > > Bruce Richardson (4): > examples: install examples as part of ninja install > examples: simplify getting list of all examples > devtools/test-meson-builds: add testing of pkg-config file > build: add libbsd to pkg-config file if enabled > > config/meson.build| 10 -- > devtools/test-meson-builds.sh | 17 + > examples/meson.build | 17 + > meson.build | 2 ++ > 4 files changed, 36 insertions(+), 10 deletions(-) Series-acked-by: Luca Boccassi -- Kind regards, Luca Boccassi
Re: [dpdk-dev] [PATCH 0/4] add testing of libdpdk pkg-config file
On Fri, Apr 26, 2019 at 05:11:18PM +0100, Luca Boccassi wrote: > On Tue, 2019-04-23 at 23:06 +0100, Bruce Richardson wrote: > > As part of the meson build, a pkg-config file for libdpdk is created, > > which > > allows apps to be compiled and linked against DPDK by taking the > > cflags and > > lib parameter from pkgconfig. The example app makefiles have been > > reworked > > to take account of this support, but the build of them against the > > .pc file > > was not regularly tested. > > > > To rectify this, and give us greater confidence in the correctness of > > the > > .pc file, this set adds in the sample apps to the installation set > > for > > "ninja install" and then builds a subset of those apps against the > > pkg-config file to test it. In the process a small error when > > compiling > > the cmdline sample app using the .pc file was fixed. > > > > Bruce Richardson (4): > > examples: install examples as part of ninja install > > examples: simplify getting list of all examples > > devtools/test-meson-builds: add testing of pkg-config file > > build: add libbsd to pkg-config file if enabled > > > > config/meson.build| 10 -- > > devtools/test-meson-builds.sh | 17 + > > examples/meson.build | 17 + > > meson.build | 2 ++ > > 4 files changed, 36 insertions(+), 10 deletions(-) > > Series-acked-by: Luca Boccassi > Thanks. I've actually got a V2 coming soon with some more rework, since I found some more issues (especially on BSD) as I worked through the changes. /Bruce
[dpdk-dev] [PATCH] vfio: expand non-viable group error message
"VFIO group is not viable" error message is correct
but not very user friendly for something which can
usually be easily rectified.
Add some additional text to give more of a hint.
Signed-off-by: Kevin Traynor
---
lib/librte_eal/linux/eal/eal_vfio.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/linux/eal/eal_vfio.c
b/lib/librte_eal/linux/eal/eal_vfio.c
index 19e70bb66..6892a2c14 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.c
+++ b/lib/librte_eal/linux/eal/eal_vfio.c
@@ -686,5 +686,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char
*dev_addr,
return -1;
} else if (!(group_status.flags & VFIO_GROUP_FLAGS_VIABLE)) {
- RTE_LOG(ERR, EAL, " %s VFIO group is not viable!\n", dev_addr);
+ RTE_LOG(ERR, EAL, " %s VFIO group is not viable! "
+ "Not all devices in IOMMU group bound to VFIO
or unbound\n",
+ dev_addr);
close(vfio_group_fd);
rte_vfio_clear_group(vfio_group_fd);
--
2.20.1
Re: [dpdk-dev] [PATCH] vfio: expand non-viable group error message
On 26-Apr-19 5:22 PM, Kevin Traynor wrote:
"VFIO group is not viable" error message is correct
but not very user friendly for something which can
usually be easily rectified.
Add some additional text to give more of a hint.
Signed-off-by: Kevin Traynor
---
lib/librte_eal/linux/eal/eal_vfio.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/linux/eal/eal_vfio.c
b/lib/librte_eal/linux/eal/eal_vfio.c
index 19e70bb66..6892a2c14 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.c
+++ b/lib/librte_eal/linux/eal/eal_vfio.c
@@ -686,5 +686,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char
*dev_addr,
return -1;
} else if (!(group_status.flags & VFIO_GROUP_FLAGS_VIABLE)) {
- RTE_LOG(ERR, EAL, " %s VFIO group is not viable!\n", dev_addr);
+ RTE_LOG(ERR, EAL, " %s VFIO group is not viable! "
+ "Not all devices in IOMMU group bound to VFIO or
unbound\n",
+ dev_addr);
close(vfio_group_fd);
rte_vfio_clear_group(vfio_group_fd);
Come on, everyone knows /that/!
Acked-by: Anatoly Burakov
--
Thanks,
Anatoly
[dpdk-dev] [PATCH v4] eventdev: add experimental tag back
Add the experimental tag back to the Rx event adapter callback,
the Rx event callback register and the Rx event adapter statistics
retrieval functions due to an API change to be proposed in a
future patch.
This patch also adds the experimental tag to these
function definitions and adds the functions to the EXPERIMENTAL
section of the map file, these were missing previously.
Fixes: 80bdf91dc8ee ("eventdev: promote adapter functions as stable")
Cc: jer...@marvell.com
Signed-off-by: Nikhil Rao
---
v4:
* move rte_event_eth_rx_adapter_cb_register to the experimental section of the
map file
* add EXPERIMENTAL to the MAINTAINERS file
v3:
* add experimental tag to the Rx event adapter stats retrieval function
v2:
* add patch explanation to commit message
lib/librte_eventdev/rte_event_eth_rx_adapter.h | 17 +++--
lib/librte_eventdev/rte_event_eth_rx_adapter.c | 11 ++-
lib/librte_eventdev/rte_eventdev_version.map | 9 +++--
MAINTAINERS| 2 +-
4 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
index cf23cde..176f8ca 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
@@ -173,6 +173,9 @@ struct rte_event_eth_rx_adapter_queue_conf {
};
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
* A structure used to retrieve statistics for an eth rx adapter instance.
*/
struct rte_event_eth_rx_adapter_stats {
@@ -201,6 +204,9 @@ struct rte_event_eth_rx_adapter_stats {
};
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
* Callback function invoked by the SW adapter before it continues
* to process packets. The callback is passed the size of the enqueue
* buffer in the SW adapter and the occupancy of the buffer. The
@@ -392,6 +398,9 @@ int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint16_t
eth_dev_id,
int rte_event_eth_rx_adapter_stop(uint8_t id);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
* Retrieve statistics for an adapter
*
* @param id
@@ -404,7 +413,8 @@ int rte_event_eth_rx_adapter_queue_del(uint8_t id, uint16_t
eth_dev_id,
* - 0: Success, retrieved successfully.
* - <0: Error code on failure.
*/
-int rte_event_eth_rx_adapter_stats_get(uint8_t id,
+int __rte_experimental
+rte_event_eth_rx_adapter_stats_get(uint8_t id,
struct rte_event_eth_rx_adapter_stats *stats);
/**
@@ -437,6 +447,9 @@ int rte_event_eth_rx_adapter_stats_get(uint8_t id,
int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
* Register callback to process Rx packets, this is supported for
* SW based packet transfers.
* @see rte_event_eth_rx_cb_fn
@@ -453,7 +466,7 @@ int rte_event_eth_rx_adapter_stats_get(uint8_t id,
* - 0: Success
* - <0: Error code on failure.
*/
-int
+int __rte_experimental
rte_event_eth_rx_adapter_cb_register(uint8_t id,
uint16_t eth_dev_id,
rte_event_eth_rx_adapter_cb_fn cb_fn,
diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
index 8d178be..27e0fe7 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -2296,7 +2296,7 @@ static int rxa_sw_add(struct rte_event_eth_rx_adapter
*rx_adapter,
return rxa_ctrl(id, 0);
}
-int
+int __rte_experimental
rte_event_eth_rx_adapter_stats_get(uint8_t id,
struct rte_event_eth_rx_adapter_stats *stats)
{
@@ -2383,10 +2383,11 @@ static int rxa_sw_add(struct rte_event_eth_rx_adapter
*rx_adapter,
return rx_adapter->service_inited ? 0 : -ESRCH;
}
-int rte_event_eth_rx_adapter_cb_register(uint8_t id,
- uint16_t eth_dev_id,
- rte_event_eth_rx_adapter_cb_fn cb_fn,
- void *cb_arg)
+int __rte_experimental
+rte_event_eth_rx_adapter_cb_register(uint8_t id,
+ uint16_t eth_dev_id,
+ rte_event_eth_rx_adapter_cb_fn cb_fn,
+ void *cb_arg)
{
struct rte_event_eth_rx_adapter *rx_adapter;
struct eth_device_info *dev_info;
diff --git a/lib/librte_eventdev/rte_eventdev_version.map
b/lib/librte_eventdev/rte_eventdev_version.map
index 88c3ce5..95fd089 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -63,7 +63,6 @@ DPDK_17.11 {
rte_event_eth_rx_adapter_queue_del;
rte_event_eth_rx_adapter_service_id_get;
rte_event_eth_rx_adapter_start;
-
[dpdk-dev] [PATCH v2 1/6] examples/l3fwd: fix compile on FreeBSD
On freebsd we need to include sys/socket.h to get the definition of
AF_INET in order to compile.
Fixes: d5ceea4ab160 ("examples/l3fwd: format IP addresses for printing")
Signed-off-by: Bruce Richardson
---
examples/l3fwd/l3fwd_lpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index 172a036b2..60a00639e 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
--
2.21.0
[dpdk-dev] [PATCH v2 2/6] examples: install examples as part of ninja install
When we install dpdk onto a system, we want to put the examples into
the /usr/share/dpdk (or /usr/local/share) directory for reference.
Signed-off-by: Bruce Richardson
Acked-by: Luca Boccassi
---
examples/meson.build | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/examples/meson.build b/examples/meson.build
index e4babf6bf..1a6134f12 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -8,12 +8,20 @@ endif
execinfo = cc.find_library('execinfo', required: false)
-allow_skips = true # don't flag an error if we can't build an app
+all_examples = run_command('sh', '-c',
+ 'cd $MESON_SOURCE_ROOT/$MESON_SUBDIR && for d in * ; do if [ -d $d ] ;
then echo $d ; fi ; done'
+ ).stdout().split()
+# install all example code on install - irrespective of whether the example in
+# question is to be built as part of this build or not.
+foreach ex:all_examples
+ install_subdir(ex,
+ install_dir: get_option('datadir') + '/dpdk/examples',
+ exclude_files: 'meson.build')
+endforeach
if get_option('examples').to_lower() == 'all'
- dirs = run_command('sh', '-c',
- 'cd $MESON_SOURCE_ROOT/$MESON_SUBDIR && for d in * ; do if [ -d
$d ] ; then echo $d ; fi ; done')
- examples = dirs.stdout().split()
+ examples = all_examples
+ allow_skips = true # don't flag an error if we can't build an app
else
examples = get_option('examples').split(',')
allow_skips = false # error out if we can't build a requested app
--
2.21.0
[dpdk-dev] [PATCH v2 0/6] add testing of libdpdk pkg-config file
As part of the meson build, a pkg-config file for libdpdk is created, which allows apps to be compiled and linked against DPDK by taking the cflags and lib parameter from pkgconfig. The example app makefiles have been reworked to take account of this support, but the build of them against the .pc file was not regularly tested. To rectify this, and give us greater confidence in the correctness of the .pc file, this set adds in the sample apps to the installation set for "ninja install" and then builds a subset of those apps against the pkg-config file to test it. In the process a small error when compiling the cmdline sample app using the .pc file was fixed. V2: Fixed a number of other problems encountered on FreeBSD. Replaced patch 2 of the original set, which broke on BSD with patch 6 of this set, which is the more correct solution. Bruce Richardson (6): examples/l3fwd: fix compile on freebsd examples: install examples as part of ninja install build: fix ninja install on FreeBSD devtools/test-meson-builds: add testing of pkg-config file build: add libbsd to pkg-config file if enabled examples: remove auto-generation of examples list buildtools/symlink-drivers-solibs.sh | 7 +++-- config/meson.build | 17 devtools/test-meson-builds.sh| 27 ++ examples/l3fwd/l3fwd_lpm.c | 1 + examples/meson.build | 41 +--- meson.build | 9 ++ 6 files changed, 82 insertions(+), 20 deletions(-) -- 2.21.0
[dpdk-dev] [PATCH v2 4/6] devtools/test-meson-builds: add testing of pkg-config file
The pkg-config file generated as part of the build of DPDK should allow
applications to be built with an installed DPDK. We can test this as
part of the build by doing an install of DPDK to a temporary directory
within the build folder, and by then compiling up a few sample apps
using make working off that directory.
Signed-off-by: Bruce Richardson
Acked-by: Luca Boccassi
---
devtools/test-meson-builds.sh | 27 +++
1 file changed, 27 insertions(+)
diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 630a1a6fe..73f993b98 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -15,6 +15,11 @@ srcdir=$(dirname $(readlink -f $0))/..
MESON=${MESON:-meson}
use_shared="--default-library=shared"
+if command -v gmake >/dev/null 2>&1 ; then
+ MAKE=gmake
+else
+ MAKE=make
+fi
if command -v ninja >/dev/null 2>&1 ; then
ninja_cmd=ninja
elif command -v ninja-build >/dev/null 2>&1 ; then
@@ -90,3 +95,25 @@ if command -v $c >/dev/null 2>&1 ; then
$use_shared --cross-file $f
done
fi
+
+##
+# Test installation of the x86-default target, to be used for checking
+# the sample apps build using the pkg-config file for cflags and libs
+###
+build_path=build-x86-default
+DESTDIR=`pwd`/$build_path/install-root ; export DESTDIR
+$ninja_cmd -C $build_path install
+
+pc_file=$(find $DESTDIR -name libdpdk.pc)
+PKG_CONFIG_PATH=$(dirname $pc_file) ; export PKG_CONFIG_PATH
+
+# rather than hacking our environment, just edit the .pc file prefix value
+sed -i -e "s|prefix=|prefix=$DESTDIR|" $pc_file
+
+for example in helloworld l2fwd l3fwd skeleton timer; do
+ echo "## Building $example"
+ $MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example
+done
+
+echo ""
+echo "## $0: Completed OK"
--
2.21.0
[dpdk-dev] [PATCH v2 3/6] build: fix ninja install on FreeBSD
The post-install script to symlink the PMDs from their own PMD directory to
the regular lib directory (so they would be found by ld at runtime) was
using the "-r" flag to ln to create relative symlinks. This flag is
unsupported by ln on FreeBSD causing the ninja install step to fail.
Reworking the script to take the relative driver path as parameter removes
the need for ln to calculate the relative path ensuring compatibility with
FreeBSD.
As part of the fix, we move the registration of the install script to the
config/meson.build file, from the top level one. This improves readability
as the script takes as parameters the variables set in that file.
Fixes: ed4d43d73e2b ("build: symlink drivers to library directory")
Signed-off-by: Bruce Richardson
---
buildtools/symlink-drivers-solibs.sh | 7 ---
config/meson.build | 7 +++
meson.build | 7 ---
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/buildtools/symlink-drivers-solibs.sh
b/buildtools/symlink-drivers-solibs.sh
index 9826c6ae3..42985e855 100644
--- a/buildtools/symlink-drivers-solibs.sh
+++ b/buildtools/symlink-drivers-solibs.sh
@@ -7,6 +7,7 @@
# others, e.g. PCI device PMDs depending on the PCI bus driver.
# parameters to script are paths relative to install prefix:
-# 1. directory containing driver files e.g. lib64/dpdk/drivers
-# 2. directory for installed regular libs e.g. lib64
-ln -rsf ${DESTDIR}/${MESON_INSTALL_PREFIX}/$1/*
${DESTDIR}/${MESON_INSTALL_PREFIX}/$2
+# 1. directory for installed regular libs e.g. lib64
+# 2. subdirectory of libdir where the pmds are
+
+cd ${MESON_INSTALL_DESTDIR_PREFIX}/$1 && ln -sfv $2/librte_*.so* .
diff --git a/config/meson.build b/config/meson.build
index f8aded6ed..3678348de 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -42,6 +42,13 @@ endif
driver_install_path = join_paths(get_option('libdir'), pmd_subdir_opt)
eal_pmd_path = join_paths(get_option('prefix'), driver_install_path)
+# driver .so files often depend upon the bus drivers for their connect bus,
+# e.g. ixgbe depends on librte_bus_pci. This means that the bus drivers need
+# to be in the library path, so symlink the drivers from the main lib
directory.
+meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
+ get_option('libdir'),
+ pmd_subdir_opt)
+
# set the machine type and cflags for it
if meson.is_cross_build()
machine = host_machine.cpu()
diff --git a/meson.build b/meson.build
index a96486597..d1e8e5239 100644
--- a/meson.build
+++ b/meson.build
@@ -63,13 +63,6 @@ configure_file(output: build_cfg,
# them.
dpdk_drivers = ['-Wl,--whole-archive'] + dpdk_drivers +
['-Wl,--no-whole-archive']
-# driver .so files often depend upon the bus drivers for their connect bus,
-# e.g. ixgbe depends on librte_bus_pci. This means that the bus drivers need
-# to be in the library path, so symlink the drivers from the main lib
directory.
-meson.add_install_script('buildtools/symlink-drivers-solibs.sh',
- driver_install_path,
- get_option('libdir'))
-
pkg = import('pkgconfig')
pkg.generate(name: meson.project_name(),
filebase: 'lib' + meson.project_name().to_lower(),
--
2.21.0
[dpdk-dev] [PATCH v2 5/6] build: add libbsd to pkg-config file if enabled
If libbsd is enabled in DPDK, the strlcpy and strlcat functions in
rte_string_fns.h redirect to the varients in libbsd, only using the
fallbacks if it is not enabled. Therefore, if libbsd is enabled, it needs
to be called out as a DPDK dependency in the pkgconfig file.
To ensure that we don't have undefined variables on non-Linux platforms, we
can remove the linux condition around the libbsd check - no harm comes in
looking for it on other OS, since it's an optional dependency.
To verify this builds correctly, the cmdline sample app is added to the
list of examples compiled by test-meson-builds. Without this change
compilation fails if libbsd is present on the system
Signed-off-by: Bruce Richardson
Acked-by: Luca Boccassi
---
config/meson.build| 10 --
devtools/test-meson-builds.sh | 2 +-
meson.build | 2 ++
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/config/meson.build b/config/meson.build
index 3678348de..0d25646f5 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -132,12 +132,10 @@ if numa_dep.found() and cc.has_header('numaif.h')
dpdk_extra_ldflags += '-lnuma'
endif
-# check for strlcpy
-if is_linux
- libbsd = dependency('libbsd', required: false)
- if libbsd.found()
- dpdk_conf.set('RTE_USE_LIBBSD', 1)
- endif
+# check for libbsd
+libbsd = dependency('libbsd', required: false)
+if libbsd.found()
+ dpdk_conf.set('RTE_USE_LIBBSD', 1)
endif
# add -include rte_config to cflags
diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 73f993b98..a9fdce81f 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -110,7 +110,7 @@ PKG_CONFIG_PATH=$(dirname $pc_file) ; export PKG_CONFIG_PATH
# rather than hacking our environment, just edit the .pc file prefix value
sed -i -e "s|prefix=|prefix=$DESTDIR|" $pc_file
-for example in helloworld l2fwd l3fwd skeleton timer; do
+for example in cmdline helloworld l2fwd l3fwd skeleton timer; do
echo "## Building $example"
$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example
done
diff --git a/meson.build b/meson.build
index d1e8e5239..46f9c5683 100644
--- a/meson.build
+++ b/meson.build
@@ -70,6 +70,8 @@ pkg.generate(name: meson.project_name(),
libraries: dpdk_libraries,
libraries_private: dpdk_drivers + dpdk_static_libraries +
['-Wl,-Bdynamic'] + dpdk_extra_ldflags,
+ requires: libbsd, # apps using rte_string_fns.h may need this if enabled
+ # if libbsd is not enabled, then this is blank
description: '''The Data Plane Development Kit (DPDK).
Note that CFLAGS might contain an -march flag higher than typical baseline.
This is required for a number of static inline functions in the public
headers.''',
--
2.21.0
[dpdk-dev] [PATCH v2 6/6] examples: remove auto-generation of examples list
The examples/meson.build file scanned the filesystem to find all examples
to build (for examples=all option) and install. However, using run_command
and scanning the filesystem prevented ninja from properly detecting the
addition or removal of any examples - one had to recreate the build
directory from scratch to guarantee correct detection of all examples. This
patch replaces this generated list with a static list of examples, thereby
allowing proper tracking by ninja/meson, but at the cost of having to
update this file when a new example is added or removed.
Signed-off-by: Bruce Richardson
---
examples/meson.build | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/examples/meson.build b/examples/meson.build
index 1a6134f12..8b6577cf7 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -8,9 +8,34 @@ endif
execinfo = cc.find_library('execinfo', required: false)
-all_examples = run_command('sh', '-c',
- 'cd $MESON_SOURCE_ROOT/$MESON_SUBDIR && for d in * ; do if [ -d $d ] ;
then echo $d ; fi ; done'
- ).stdout().split()
+# list of all example apps. Keep 1-3 per line, in alphabetical order.
+all_examples = [
+ 'bbdev_app', 'bond',
+ 'bpf', 'cmdline',
+ 'distributor', 'ethtool',
+ 'eventdev_pipeline', 'exception_path',
+ 'fips_validation', 'flow_classify',
+ 'flow_filtering', 'helloworld',
+ 'ip_fragmentation', 'ip_pipeline',
+ 'ip_reassembly', 'ipsec-secgw',
+ 'ipv4_multicast', 'kni',
+ 'l2fwd', 'l2fwd-cat',
+ 'l2fwd-crypto', 'l2fwd-jobstats',
+ 'l2fwd-keepalive', 'l3fwd',
+ 'l3fwd-acl', 'l3fwd-power',
+ 'l3fwd-vf', 'link_status_interrupt',
+ 'load_balancer', 'multi_process',
+ 'netmap_compat', 'packet_ordering',
+ 'performance-thread', 'ptpclient',
+ 'qos_meter', 'qos_sched',
+ 'quota_watermark', 'rxtx_callbacks',
+ 'server_node_efd', 'service_cores',
+ 'skeleton', 'tep_termination',
+ 'timer', 'vdpa',
+ 'vhost', 'vhost_crypto',
+ 'vhost_scsi', 'vm_power_manager',
+ 'vmdq', 'vmdq_dcb',
+]
# install all example code on install - irrespective of whether the example in
# question is to be built as part of this build or not.
foreach ex:all_examples
--
2.21.0
[dpdk-dev] [PATCH] doc: update references to removed function
Remove references to the (deleted) rte_event_port_enqueue_depth()
function in the Doxygen comments for rte_event_enqueue_burst() and
friends, and replace with references to rte_event_port_attr_get().
Fixes: 78ffab961155 ("eventdev: add port attribute function")
Signed-off-by: Erik Gabriel Carrillo
---
lib/librte_eventdev/rte_eventdev.h | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/lib/librte_eventdev/rte_eventdev.h
b/lib/librte_eventdev/rte_eventdev.h
index 62edc09..0f0da7f 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1363,7 +1363,8 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
* which contain the event object enqueue operations to be processed.
* @param nb_events
* The number of event objects to enqueue, typically number of
- * rte_event_port_enqueue_depth() available for this port.
+ * rte_event_port_attr_get(...RTE_EVENT_PORT_ATTR_ENQ_DEPTH...)
+ * available for this port.
*
* @return
* The number of event objects actually enqueued on the event device. The
@@ -1378,7 +1379,7 @@ __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
* - -ENOSPC The event port was backpressured and unable to enqueue
* one or more events. This error code is only applicable to
* closed systems.
- * @see rte_event_port_enqueue_depth()
+ * @see rte_event_port_attr_get(), RTE_EVENT_PORT_ATTR_ENQ_DEPTH
*/
static inline uint16_t
rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
@@ -1412,7 +1413,8 @@ rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
* which contain the event object enqueue operations to be processed.
* @param nb_events
* The number of event objects to enqueue, typically number of
- * rte_event_port_enqueue_depth() available for this port.
+ * rte_event_port_attr_get(...RTE_EVENT_PORT_ATTR_ENQ_DEPTH...)
+ * available for this port.
*
* @return
* The number of event objects actually enqueued on the event device. The
@@ -1427,7 +1429,8 @@ rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
* - -ENOSPC The event port was backpressured and unable to enqueue
* one or more events. This error code is only applicable to
* closed systems.
- * @see rte_event_port_enqueue_depth() rte_event_enqueue_burst()
+ * @see rte_event_port_attr_get(), RTE_EVENT_PORT_ATTR_ENQ_DEPTH
+ * @see rte_event_enqueue_burst()
*/
static inline uint16_t
rte_event_enqueue_new_burst(uint8_t dev_id, uint8_t port_id,
@@ -1461,7 +1464,8 @@ rte_event_enqueue_new_burst(uint8_t dev_id, uint8_t
port_id,
* which contain the event object enqueue operations to be processed.
* @param nb_events
* The number of event objects to enqueue, typically number of
- * rte_event_port_enqueue_depth() available for this port.
+ * rte_event_port_attr_get(...RTE_EVENT_PORT_ATTR_ENQ_DEPTH...)
+ * available for this port.
*
* @return
* The number of event objects actually enqueued on the event device. The
@@ -1476,7 +1480,8 @@ rte_event_enqueue_new_burst(uint8_t dev_id, uint8_t
port_id,
* - -ENOSPC The event port was backpressured and unable to enqueue
* one or more events. This error code is only applicable to
* closed systems.
- * @see rte_event_port_enqueue_depth() rte_event_enqueue_burst()
+ * @see rte_event_port_attr_get(), RTE_EVENT_PORT_ATTR_ENQ_DEPTH
+ * @see rte_event_enqueue_burst()
*/
static inline uint16_t
rte_event_enqueue_forward_burst(uint8_t dev_id, uint8_t port_id,
--
2.6.4
Re: [dpdk-dev] [PATCH v4] eventdev: add experimental tag back
On 26/04/2019 17:33, Nikhil Rao wrote:
> Add the experimental tag back to the Rx event adapter callback,
> the Rx event callback register and the Rx event adapter statistics
> retrieval functions due to an API change to be proposed in a
> future patch.
>
> This patch also adds the experimental tag to these
> function definitions and adds the functions to the EXPERIMENTAL
> section of the map file, these were missing previously.
>
> Fixes: 80bdf91dc8ee ("eventdev: promote adapter functions as stable")
> Cc: jer...@marvell.com
>
> Signed-off-by: Nikhil Rao
> ---
Acked-by: Kevin Traynor
Re: [dpdk-dev] [PATCH v6 1/1] eventdev: add new software event timer adapter
Hi Erik, A quick question. > -Original Message- > From: dev On Behalf Of Erik Gabriel Carrillo > Sent: Friday, April 26, 2019 10:14 AM > To: jerin.ja...@caviumnetworks.com > Cc: mattias.ronnb...@ericsson.com; pbhagavat...@caviumnetworks.com; > dev@dpdk.org > Subject: [dpdk-dev] [PATCH v6 1/1] eventdev: add new software event timer > adapter > > This patch introduces a new version of the event timer adapter software PMD. > In the original design, timer event producer lcores in the primary and > secondary processes enqueued event timers into a ring, and a service core in > the primary process dequeued them and processed them further. To improve > performance, this version does away with the ring and lets lcores in both > primary and secondary processes insert timers directly into timer skiplist > data > structures; the service core directly accesses the lists as well, when > looking for > timers that have expired. How do you ensure concurrent access to the timer skiplist? Are you using any locks or is it a lock-free data structure?
Re: [dpdk-dev] [PATCH v6 1/1] eventdev: add new software event timer adapter
> -Original Message- > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com] > Sent: Friday, April 26, 2019 1:51 PM > To: Carrillo, Erik G ; > jerin.ja...@caviumnetworks.com > Cc: mattias.ronnb...@ericsson.com; pbhagavat...@caviumnetworks.com; > dev@dpdk.org; Honnappa Nagarahalli ; > nd ; nd > Subject: RE: [dpdk-dev] [PATCH v6 1/1] eventdev: add new software event > timer adapter > > Hi Erik, > A quick question. > > > -Original Message- > > From: dev On Behalf Of Erik Gabriel Carrillo > > Sent: Friday, April 26, 2019 10:14 AM > > To: jerin.ja...@caviumnetworks.com > > Cc: mattias.ronnb...@ericsson.com; > pbhagavat...@caviumnetworks.com; > > dev@dpdk.org > > Subject: [dpdk-dev] [PATCH v6 1/1] eventdev: add new software event > > timer adapter > > > > This patch introduces a new version of the event timer adapter software > PMD. > > In the original design, timer event producer lcores in the primary and > > secondary processes enqueued event timers into a ring, and a service > > core in the primary process dequeued them and processed them further. > > To improve performance, this version does away with the ring and lets > > lcores in both primary and secondary processes insert timers directly > > into timer skiplist data structures; the service core directly > > accesses the lists as well, when looking for timers that have expired. > How do you ensure concurrent access to the timer skiplist? Are you using any > locks or is it a lock-free data structure? > > There are multiple timer skiplists, one for each lcore, and each has its lock that is acquired as necessary when adding or removing timers from the skiplists. This locking occurs in the underlying timer library, in the timer reset and stop functions. Regards, Erik
[dpdk-dev] [PATCH] crypto/qat: fix NULL cipher algo for non 8-byte multiple
NULL cipher algo of 4-byte multiple and other sizes caused firmware hang
due to use of wrong mode. Changed from ECB mode to CTR mode to fix.
Fixes: 98f060891615 ("crypto/qat: add symmetric session file")
cc: sta...@dpdk.org
Signed-off-by: Fiona Trahe
---
drivers/crypto/qat/qat_sym_session.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/qat/qat_sym_session.c
b/drivers/crypto/qat/qat_sym_session.c
index 5cc86f554..f66175d03 100644
--- a/drivers/crypto/qat/qat_sym_session.c
+++ b/drivers/crypto/qat/qat_sym_session.c
@@ -242,7 +242,8 @@ qat_sym_session_configure_cipher(struct rte_cryptodev *dev,
session->qat_mode = ICP_QAT_HW_CIPHER_ECB_MODE;
break;
case RTE_CRYPTO_CIPHER_NULL:
- session->qat_mode = ICP_QAT_HW_CIPHER_ECB_MODE;
+ session->qat_cipher_alg = ICP_QAT_HW_CIPHER_ALGO_NULL;
+ session->qat_mode = ICP_QAT_HW_CIPHER_CTR_MODE;
break;
case RTE_CRYPTO_CIPHER_KASUMI_F8:
if (qat_sym_validate_kasumi_key(cipher_xform->key.length,
--
2.13.6
[dpdk-dev] [PATCH] test/crypto: added NULL algo tests to loop test mechanism
Added NULL algo tests into loop test mechanism used
by block cipher tests as easier to extend there.
Included chain, cipher-only and auth-only use-cases.
Extended to cover out-of-place use-cases and use-cases
where data length is not an 8-byte multiple.
Signed-off-by: Fiona Trahe
---
app/test/test_cryptodev.c | 82 ++--
app/test/test_cryptodev_aes_test_vectors.h | 277 +++-
app/test/test_cryptodev_blockcipher.c | 9 +-
app/test/test_cryptodev_blockcipher.h | 1 +
app/test/test_cryptodev_hash_test_vectors.h | 56 +-
5 files changed, 406 insertions(+), 19 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9f31aaa7e..2b76f3ffc 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -1673,6 +1673,64 @@ test_authonly_qat_all(void)
return TEST_SUCCESS;
}
+
+static int
+test_AES_chain_null_all(void)
+{
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ int status;
+
+ status = test_blockcipher_all_tests(ts_params->mbuf_pool,
+ ts_params->op_mpool,
+ ts_params->session_mpool, ts_params->session_priv_mpool,
+ ts_params->valid_devs[0],
+ rte_cryptodev_driver_id_get(
+ RTE_STR(CRYPTODEV_NAME_NULL_PMD)),
+ BLKCIPHER_AES_CHAIN_TYPE);
+
+ TEST_ASSERT_EQUAL(status, 0, "Test failed");
+
+ return TEST_SUCCESS;
+}
+
+static int
+test_AES_cipheronly_null_all(void)
+{
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ int status;
+
+ status = test_blockcipher_all_tests(ts_params->mbuf_pool,
+ ts_params->op_mpool,
+ ts_params->session_mpool, ts_params->session_priv_mpool,
+ ts_params->valid_devs[0],
+ rte_cryptodev_driver_id_get(
+ RTE_STR(CRYPTODEV_NAME_NULL_PMD)),
+ BLKCIPHER_AES_CIPHERONLY_TYPE);
+
+ TEST_ASSERT_EQUAL(status, 0, "Test failed");
+
+ return TEST_SUCCESS;
+}
+
+static int
+test_authonly_null_all(void)
+{
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ int status;
+
+ status = test_blockcipher_all_tests(ts_params->mbuf_pool,
+ ts_params->op_mpool,
+ ts_params->session_mpool, ts_params->session_priv_mpool,
+ ts_params->valid_devs[0],
+ rte_cryptodev_driver_id_get(
+ RTE_STR(CRYPTODEV_NAME_NULL_PMD)),
+ BLKCIPHER_AUTHONLY_TYPE);
+
+ TEST_ASSERT_EQUAL(status, 0, "Test failed");
+
+ return TEST_SUCCESS;
+}
+
static int
test_AES_chain_mb_all(void)
{
@@ -9257,15 +9315,9 @@ static struct unit_test_suite cryptodev_qat_testsuite =
{
TEST_CASE_ST(ut_setup, ut_teardown,
test_MD5_HMAC_verify_case_2),
- /** NULL tests */
- TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_auth_only_operation),
- TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_cipher_only_operation),
- TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_cipher_auth_operation),
- TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_auth_cipher_operation),
+ /** NULL algo tests done in chain_all,
+* cipheronly and authonly suites
+*/
/** KASUMI tests */
TEST_CASE_ST(ut_setup, ut_teardown,
@@ -10304,17 +10356,15 @@ static struct unit_test_suite
cryptodev_null_testsuite = {
.teardown = testsuite_teardown,
.unit_test_cases = {
TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_auth_only_operation),
- TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_cipher_only_operation),
+ test_null_invalid_operation),
TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_cipher_auth_operation),
+ test_null_burst_operation),
TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_auth_cipher_operation),
+ test_AES_chain_null_all),
TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_invalid_operation),
+ test_AES_cipheronly_null_all),
TEST_CASE_ST(ut_setup, ut_teardown,
- test_null_burst_operation),
+ test_authonly_null_all),
TEST_CASES_END() /**< NULL terminate unit test array */
}
diff --git a/app/test/test_cryptodev_aes_test_vectors.h
b/app/test/test_cryptodev_aes_test_vectors.h
index e5b9da4fa..ee4fdc9a7 100644
--- a/app/test/test_cryptodev_aes_test_vectors.h
+++ b/app/test/test_cryptodev_aes_test_vectors.h
@@ -221,6 +221,141 @
[dpdk-dev] [PATCH] net/bonding: fix test bonding MAC assignment
From: Krzysztof Kanas
Fix test_set_bonded_port_initialization_mac_assignment so that it works
after 're run' test_link_bonding.
Fixes: f2ef6f21ee2e ("bond: fix mac assignment to slaves")
Cc: declan.dohe...@intel.com
Signed-off-by: Krzysztof Kanas
---
app/test/test_link_bonding.c | 53 +---
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 0fe1d78eb0f5..c00ec6c445bd 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -201,6 +201,7 @@ configure_ethdev(uint16_t port_id, uint8_t start, uint8_t
en_isr)
}
static int slaves_initialized;
+static int mac_slaves_initialized;
static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t cvar = PTHREAD_COND_INITIALIZER;
@@ -873,10 +874,11 @@ test_set_explicit_bonded_mac(void)
static int
test_set_bonded_port_initialization_mac_assignment(void)
{
- int i, slave_count, bonded_port_id;
+ int i, slave_count;
uint16_t slaves[RTE_MAX_ETHPORTS];
- int slave_port_ids[BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT];
+ static int bonded_port_id = -1;
+ static int slave_port_ids[BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT];
struct ether_addr slave_mac_addr, bonded_mac_addr, read_mac_addr;
@@ -887,42 +889,49 @@ test_set_bonded_port_initialization_mac_assignment(void)
/*
* 1. a - Create / configure bonded / slave ethdevs
*/
- bonded_port_id = rte_eth_bond_create("net_bonding_mac_ass_test",
- BONDING_MODE_ACTIVE_BACKUP, rte_socket_id());
- TEST_ASSERT(bonded_port_id > 0, "failed to create bonded device");
+ if (bonded_port_id == -1) {
+ bonded_port_id = rte_eth_bond_create("net_bonding_mac_ass_test",
+ BONDING_MODE_ACTIVE_BACKUP, rte_socket_id());
+ TEST_ASSERT(bonded_port_id > 0, "failed to create bonded
device");
- TEST_ASSERT_SUCCESS(configure_ethdev(bonded_port_id, 0, 0),
- "Failed to configure bonded ethdev");
+ TEST_ASSERT_SUCCESS(configure_ethdev(bonded_port_id, 0, 0),
+ "Failed to configure bonded ethdev");
+ }
- for (i = 0; i < BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT; i++) {
- char pmd_name[RTE_ETH_NAME_MAX_LEN];
+ if (!mac_slaves_initialized) {
- slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = i + 100;
+ for (i = 0; i < BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT; i++) {
+ char pmd_name[RTE_ETH_NAME_MAX_LEN];
- snprintf(pmd_name, RTE_ETH_NAME_MAX_LEN, "eth_slave_%d", i);
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = i + 100;
- slave_port_ids[i] = virtual_ethdev_create(pmd_name,
- &slave_mac_addr, rte_socket_id(), 1);
+ snprintf(pmd_name, RTE_ETH_NAME_MAX_LEN,
+ "eth_slave_%d", i);
- TEST_ASSERT(slave_port_ids[i] >= 0,
- "Failed to create slave ethdev %s", pmd_name);
+ slave_port_ids[i] = virtual_ethdev_create(pmd_name,
+ &slave_mac_addr, rte_socket_id(), 1);
- TEST_ASSERT_SUCCESS(configure_ethdev(slave_port_ids[i], 1, 0),
- "Failed to configure virtual ethdev %s",
- pmd_name);
- }
+ TEST_ASSERT(slave_port_ids[i] >= 0,
+ "Failed to create slave ethdev %s",
+ pmd_name);
+ TEST_ASSERT_SUCCESS(configure_ethdev(
+ slave_port_ids[i], 1, 0),
+ "Failed to configure virtual ethdev %s",
+ pmd_name);
+ }
+ mac_slaves_initialized = 1;
+ }
/*
-* 2. Add slave ethdevs to bonded device
-*/
+ * 2. Add slave ethdevs to bonded device
+ */
for (i = 0; i < BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT; i++) {
TEST_ASSERT_SUCCESS(rte_eth_bond_slave_add(bonded_port_id,
slave_port_ids[i]),
"Failed to add slave (%d) to bonded port (%d).",
slave_port_ids[i], bonded_port_id);
}
-
slave_count = rte_eth_bond_slaves_get(bonded_port_id, slaves,
RTE_MAX_ETHPORTS);
TEST_ASSERT_EQUAL(BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT, slave_count,
--
2.20.1
[dpdk-dev] [PATCH] net/mlx5: inherit master link settings for representors
There are some physical link settings can be queried from
Ethernet devices: link status, link speed, speed capabilities,
duplex mode, etc. These setting do not make a lot of sense for
representors due to missing physical link. The new kernel drivers
dropped query for link settings for representors causing the
ioctl call to fail. This patch adds some kind of emulation
of link settings to PMD - representors inherit the link parameters
from the master device. The actual link status (up/down)
is retrieved from the representor device.
Signed-off-by: Viacheslav Ovsiienko
---
drivers/net/mlx5/mlx5_ethdev.c | 101 -
1 file changed, 89 insertions(+), 12 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 57a6449..d1a70fc 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -628,6 +628,36 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev, char
*fw_ver, size_t fw_size)
}
/**
+ * Retrieve the master device for representor in the same switch domain.
+ *
+ * @param dev
+ * Pointer to representor Ethernet device structure.
+ *
+ * @return
+ * Master device structure on success, NULL otherwise.
+ */
+
+static struct rte_eth_dev *
+mlx5_find_master_dev(struct rte_eth_dev *dev)
+{
+ struct mlx5_priv *priv;
+ uint16_t port_id;
+ uint16_t domain_id;
+
+ priv = dev->data->dev_private;
+ domain_id = priv->domain_id;
+ assert(priv->representor);
+ RTE_ETH_FOREACH_DEV_OF(port_id, dev->device) {
+ priv = rte_eth_devices[port_id].data->dev_private;
+ if (priv &&
+ priv->master &&
+ priv->domain_id == domain_id)
+ return &rte_eth_devices[port_id];
+ }
+ return NULL;
+}
+
+/**
* DPDK callback to retrieve physical link information.
*
* @param dev
@@ -666,10 +696,34 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev, char
*fw_ver, size_t fw_size)
};
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr);
if (ret) {
- DRV_LOG(WARNING,
- "port %u ioctl(SIOCETHTOOL, ETHTOOL_GSET) failed: %s",
- dev->data->port_id, strerror(rte_errno));
- return ret;
+ if (ret == -ENOTSUP && priv->representor) {
+ struct rte_eth_dev *master;
+
+ /*
+* For representors we can try to inherit link
+* settings from the master device. Actually
+* link settings do not make a lot of sense
+* for representors due to missing physical
+* link. The old kernel drivers supported
+* emulated settings query for representors,
+* the new ones do not, so we have to add
+* this code for compatibility issues.
+*/
+ master = mlx5_find_master_dev(dev);
+ if (master) {
+ ifr = (struct ifreq) {
+ .ifr_data = (void *)&edata,
+ };
+ ret = mlx5_ifreq(master, SIOCETHTOOL, &ifr);
+ }
+ }
+ if (ret) {
+ DRV_LOG(WARNING,
+ "port %u ioctl(SIOCETHTOOL,"
+ " ETHTOOL_GSET) failed: %s",
+ dev->data->port_id, strerror(rte_errno));
+ return ret;
+ }
}
link_speed = ethtool_cmd_speed(&edata);
if (link_speed == -1)
@@ -722,6 +776,7 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev, char
*fw_ver, size_t fw_size)
struct ethtool_link_settings gcmd = { .cmd = ETHTOOL_GLINKSETTINGS };
struct ifreq ifr;
struct rte_eth_link dev_link;
+ struct rte_eth_dev *master = NULL;
uint64_t sc;
int ret;
@@ -740,11 +795,33 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev, char
*fw_ver, size_t fw_size)
};
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr);
if (ret) {
- DRV_LOG(DEBUG,
- "port %u ioctl(SIOCETHTOOL, ETHTOOL_GLINKSETTINGS)"
- " failed: %s",
- dev->data->port_id, strerror(rte_errno));
- return ret;
+ if (ret == -ENOTSUP && priv->representor) {
+ /*
+* For representors we can try to inherit link
+* settings from the master device. Actually
+* link settings do not make a lot of sense
+* for representors due to missing physical
+* link. The old kernel drivers supported
+
[dpdk-dev] [PATCH v3 0/2] net/mlx5: share Memory Regions for multiport devices
The patches [1] and [2] are both related to Memory Regions sharing and their applying order matters, this series just combines ones. The multiport Infiniband device support was introduced [3]. All active ports, belonging to the same Infiniband device use the single shared Infiniband context of that device and share the resources: - QPs are created within shared context - Verbs flows are also created with specifying port index - DV/DR resources - Protection Domain - Event Handlers This patchset adds support for Memory Regions sharing between ports, created on the base of multiport Infiniband device. The datapath of mlx5 uses the layered cache subsystem for allocating/releasing Memory Regions, only the lowest layer L3 is subject to share due to performance issues. [1] http://patches.dpdk.org/patch/53040/ [2] http://patches.dpdk.org/patch/53041/ [3] http://patches.dpdk.org/cover/51800/ Signed-off-by: Viacheslav Ovsiienko --- v3: - combine two patches in pathset - commit messages spellcheck v2: - intendation issues - comments cleanup v1: http://patches.dpdk.org/patch/52723/ Viacheslav Ovsiienko (2): net/mlx5: share Memory Regions for multiport device net/mlx5: update memory event callback for shared context drivers/net/mlx5/mlx5.c | 42 +--- drivers/net/mlx5/mlx5.h | 21 ++-- drivers/net/mlx5/mlx5_mr.c | 245 ++-- drivers/net/mlx5/mlx5_mr.h | 5 +- drivers/net/mlx5/mlx5_txq.c | 2 +- 5 files changed, 162 insertions(+), 153 deletions(-) -- 1.8.3.1
[dpdk-dev] [PATCH v3 2/2] net/mlx5: update memory event callback for shared context
Mellanox mlx5 PMD implements the list of devices to process the memory
free events to reflect the actual memory state to Memory Regions.
Because this list contains the devices and devices may share the
same context the callback routine may be called multiple times
with the same parameter, that is not optimal. This patch modifies
the list to contain the device contexts instead of device objects
and shared context is included in the list only once.
Signed-off-by: Viacheslav Ovsiienko
---
drivers/net/mlx5/mlx5.c| 6 +--
drivers/net/mlx5/mlx5.h| 6 +--
drivers/net/mlx5/mlx5_mr.c | 91 +-
3 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index b563e0f..de85e85 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -673,9 +673,9 @@ struct mlx5_dev_spawn_data {
mlx5_mprq_free_mp(dev);
/* Remove from memory callback device list. */
rte_rwlock_write_lock(&mlx5_shared_data->mem_event_rwlock);
- LIST_REMOVE(priv, mem_event_cb);
- rte_rwlock_write_unlock(&mlx5_shared_data->mem_event_rwlock);
assert(priv->sh);
+ LIST_REMOVE(priv->sh, mem_event_cb);
+ rte_rwlock_write_unlock(&mlx5_shared_data->mem_event_rwlock);
mlx5_free_shared_dr(priv);
if (priv->rss_conf.rss_key != NULL)
rte_free(priv->rss_conf.rss_key);
@@ -1574,7 +1574,7 @@ struct mlx5_dev_spawn_data {
/* Add device to memory callback list. */
rte_rwlock_write_lock(&mlx5_shared_data->mem_event_rwlock);
LIST_INSERT_HEAD(&mlx5_shared_data->mem_event_cb_list,
-priv, mem_event_cb);
+sh, mem_event_cb);
rte_rwlock_write_unlock(&mlx5_shared_data->mem_event_rwlock);
return eth_dev;
error:
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 2575732..82fcb29 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -99,7 +99,7 @@ struct mlx5_switch_info {
uint64_t switch_id; /**< Switch identifier. */
};
-LIST_HEAD(mlx5_dev_list, mlx5_priv);
+LIST_HEAD(mlx5_dev_list, mlx5_ibv_shared);
/* Shared data between primary and secondary processes. */
struct mlx5_shared_data {
@@ -276,6 +276,8 @@ struct mlx5_ibv_shared {
char ibdev_path[IBV_SYSFS_PATH_MAX]; /* IB device path for secondary */
struct ibv_device_attr_ex device_attr; /* Device properties. */
struct rte_pci_device *pci_dev; /* Backend PCI device. */
+ LIST_ENTRY(mlx5_ibv_shared) mem_event_cb;
+ /**< Called by memory event callback. */
struct {
uint32_t dev_gen; /* Generation number to flush local caches. */
rte_rwlock_t rwlock; /* MR Lock. */
@@ -322,8 +324,6 @@ struct mlx5_proc_priv {
((struct mlx5_proc_priv *)rte_eth_devices[port_id].process_private)
struct mlx5_priv {
- LIST_ENTRY(mlx5_priv) mem_event_cb;
- /**< Called by memory event callback. */
struct rte_eth_dev_data *dev_data; /* Pointer to device data. */
struct mlx5_ibv_shared *sh; /* Shared IB device context. */
uint32_t ibv_port; /* IB device port number. */
diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
index a7a63b1..66e8e87 100644
--- a/drivers/net/mlx5/mlx5_mr.c
+++ b/drivers/net/mlx5/mlx5_mr.c
@@ -327,7 +327,7 @@ struct mr_update_mp_data {
* mlx5_mr_create() on miss.
*
* @param dev
- * Pointer to Ethernet device.
+ * Pointer to Ethernet device shared context.
* @param mr
* Pointer to MR to insert.
*
@@ -335,13 +335,12 @@ struct mr_update_mp_data {
* 0 on success, -1 on failure.
*/
static int
-mr_insert_dev_cache(struct rte_eth_dev *dev, struct mlx5_mr *mr)
+mr_insert_dev_cache(struct mlx5_ibv_shared *sh, struct mlx5_mr *mr)
{
- struct mlx5_priv *priv = dev->data->dev_private;
unsigned int n;
- DRV_LOG(DEBUG, "port %u inserting MR(%p) to global cache",
- dev->data->port_id, (void *)mr);
+ DRV_LOG(DEBUG, "device %s inserting MR(%p) to global cache",
+ sh->ibdev_name, (void *)mr);
for (n = 0; n < mr->ms_bmp_n; ) {
struct mlx5_mr_cache entry;
@@ -350,7 +349,7 @@ struct mr_update_mp_data {
n = mr_find_next_chunk(mr, &entry, n);
if (!entry.end)
break;
- if (mr_btree_insert(&priv->sh->mr.cache, &entry) < 0) {
+ if (mr_btree_insert(&sh->mr.cache, &entry) < 0) {
/*
* Overflowed, but the global table cannot be expanded
* because of deadlock.
@@ -364,8 +363,8 @@ struct mr_update_mp_data {
/**
* Look up address in the original global MR list.
*
- * @param dev
- * Pointer to Ethernet device.
+ * @param sh
+ * Pointer to Ethernet device shared context.
* @param[out] entry
* Pointer to returning MR c
[dpdk-dev] [PATCH v3 1/2] net/mlx5: share Memory Regions for multiport device
The multiport Infiniband device support was introduced [1].
All active ports, belonging to the same Infiniband device use the single
shared Infiniband context of that device and share the resources:
- QPs are created within shared context
- Verbs flows are also created with specifying port index
- DV/DR resources
- Protection Domain
- Event Handlers
This patchset adds support for Memory Regions sharing between
ports, created on the base of multiport Infiniband device.
The datapath of mlx5 uses the layered cache subsystem for
allocating/releasing Memory Regions, only the lowest layer L3
is subject to share due to performance issues.
[1] http://patches.dpdk.org/cover/51800/
Signed-off-by: Viacheslav Ovsiienko
---
drivers/net/mlx5/mlx5.c | 40 +++
drivers/net/mlx5/mlx5.h | 15 ++--
drivers/net/mlx5/mlx5_mr.c | 164 ++--
drivers/net/mlx5/mlx5_mr.h | 5 +-
drivers/net/mlx5/mlx5_txq.c | 2 +-
5 files changed, 121 insertions(+), 105 deletions(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 1bb58b1..b563e0f 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -147,6 +147,7 @@ struct mlx5_dev_spawn_data {
struct mlx5_switch_info info; /**< Switch information. */
struct ibv_device *ibv_dev; /**< Associated IB device. */
struct rte_eth_dev *eth_dev; /**< Associated Ethernet device. */
+ struct rte_pci_device *pci_dev; /**< Backend PCI device. */
};
static LIST_HEAD(, mlx5_ibv_shared) mlx5_ibv_list = LIST_HEAD_INITIALIZER();
@@ -225,6 +226,7 @@ struct mlx5_dev_spawn_data {
sizeof(sh->ibdev_name));
strncpy(sh->ibdev_path, sh->ctx->device->ibdev_path,
sizeof(sh->ibdev_path));
+ sh->pci_dev = spawn->pci_dev;
pthread_mutex_init(&sh->intr_mutex, NULL);
/*
* Setting port_id to max unallowed value means
@@ -239,6 +241,22 @@ struct mlx5_dev_spawn_data {
err = ENOMEM;
goto error;
}
+ /*
+* Once the device is added to the list of memory event
+* callback, its global MR cache table cannot be expanded
+* on the fly because of deadlock. If it overflows, lookup
+* should be done by searching MR list linearly, which is slow.
+*
+* At this point the device is not added to the memory
+* event list yet, context is just being created.
+*/
+ err = mlx5_mr_btree_init(&sh->mr.cache,
+MLX5_MR_BTREE_CACHE_N * 2,
+sh->pci_dev->device.numa_node);
+ if (err) {
+ err = rte_errno;
+ goto error;
+ }
LIST_INSERT_HEAD(&mlx5_ibv_list, sh, next);
exit:
pthread_mutex_unlock(&mlx5_ibv_list_mutex);
@@ -286,6 +304,8 @@ struct mlx5_dev_spawn_data {
assert(rte_eal_process_type() == RTE_PROC_PRIMARY);
if (--sh->refcnt)
goto exit;
+ /* Release created Memory Regions. */
+ mlx5_mr_release(sh);
LIST_REMOVE(sh, next);
/*
* Ensure there is no async event handler installed.
@@ -651,7 +671,10 @@ struct mlx5_dev_spawn_data {
}
mlx5_proc_priv_uninit(dev);
mlx5_mprq_free_mp(dev);
- mlx5_mr_release(dev);
+ /* Remove from memory callback device list. */
+ rte_rwlock_write_lock(&mlx5_shared_data->mem_event_rwlock);
+ LIST_REMOVE(priv, mem_event_cb);
+ rte_rwlock_write_unlock(&mlx5_shared_data->mem_event_rwlock);
assert(priv->sh);
mlx5_free_shared_dr(priv);
if (priv->rss_conf.rss_key != NULL)
@@ -1548,19 +1571,6 @@ struct mlx5_dev_spawn_data {
goto error;
}
priv->config.flow_prio = err;
- /*
-* Once the device is added to the list of memory event
-* callback, its global MR cache table cannot be expanded
-* on the fly because of deadlock. If it overflows, lookup
-* should be done by searching MR list linearly, which is slow.
-*/
- err = mlx5_mr_btree_init(&priv->mr.cache,
-MLX5_MR_BTREE_CACHE_N * 2,
-eth_dev->device->numa_node);
- if (err) {
- err = rte_errno;
- goto error;
- }
/* Add device to memory callback list. */
rte_rwlock_write_lock(&mlx5_shared_data->mem_event_rwlock);
LIST_INSERT_HEAD(&mlx5_shared_data->mem_event_cb_list,
@@ -1757,6 +1767,7 @@ struct mlx5_dev_spawn_data {
list[ns].ibv_port = i;
list[ns].ibv_dev = ibv_match[0];
list[ns].eth_dev = NULL;
+ list[ns].pci_dev = pci_dev;
list[ns].ifindex = mlx5_nl_ifindex
(nl_rdma, list[ns].ibv_dev->name, i);
if (!list[ns].ifindex) {
Re: [dpdk-dev] [PATCH] vfio: expand non-viable group error message
> On 26-Apr-19 5:22 PM, Kevin Traynor wrote: > > "VFIO group is not viable" error message is correct > > but not very user friendly for something which can > > usually be easily rectified. > > > > Add some additional text to give more of a hint. > > > > Signed-off-by: Kevin Traynor > > --- Acked-by: Anatoly Burakov > Reviewd-by: Rami Rosen
