Re: [dpdk-dev] [PATCH 8/8] ipc: fix net/mlx5 memleak
On 22-Apr-19 6:51 PM, Yongseok Koh wrote:
On Apr 17, 2019, at 7:44 AM, Herakliusz Lipiec
wrote:
When sending multiple requests, rte_mp_request_sync
can succeed sending a few of those requests, but then
fail on a later one and in the end return with rc=-1.
The upper layers - e.g. device hotplug - currently
handles this case as if no messages were sent and no
memory for response buffers was allocated, which is
not true. Fixed by always freeing reply message buffers.
Fixes: 9a8ab29b84d3 ("net/mlx5: replace IPC socket with EAL API")
Fixes: c18cf501a7af ("net/mlx5: enable secondary process to register DMA
memory")
Cc: ys...@mellanox.com
Signed-off-by: Herakliusz Lipiec
---
drivers/net/mlx5/mlx5_mp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_mp.c b/drivers/net/mlx5/mlx5_mp.c
index cea74adb6..c9915b1d5 100644
--- a/drivers/net/mlx5/mlx5_mp.c
+++ b/drivers/net/mlx5/mlx5_mp.c
@@ -258,6 +258,7 @@ mlx5_mp_req_mr_create(struct rte_eth_dev *dev, uintptr_t
addr)
if (ret) {
DRV_LOG(ERR, "port %u request to primary process failed",
dev->data->port_id);
+ free(mp_rep.msgs);
return -rte_errno;
}
assert(mp_rep.nb_received == 1);
@@ -295,7 +296,8 @@ mlx5_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev)
if (ret) {
DRV_LOG(ERR, "port %u request to primary process failed",
dev->data->port_id);
- return -rte_errno;
+ ret = -rte_errno;
+ goto exit;
These two requests will be made by a secondary process targeting to the primary.
Then, there's only one request in this case and we don't need to take care of
that.
Right?
Same comment for mlx4.
Hi Yongseok,
mp_rep.msgs is potentially allocated regardless of whether you're in
primary or secondary, and whether the call to mp_request_sync succeeded
or failed. Hence, need to free in all cases.
See this patch: http://patches.dpdk.org/patch/52868/
and this bug: https://bugs.dpdk.org/show_bug.cgi?id=228
Thanks,
Yongseok
}
assert(mp_rep.nb_received == 1);
mp_res = &mp_rep.msgs[0];
--
2.17.2
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH 1/8] ipc: fix rte_mp_request_sync memleak
On 17-Apr-19 3:38 PM, Herakliusz Lipiec wrote:
When sending multiple requests, rte_mp_request_sync
can succeed sending a few of those requests, but then
fail on a later one and in the end return with rc=-1.
The upper layers - e.g. device hotplug - currently
handles this case as if no messages were sent and no
memory for response buffers was allocated, which is
not true. Fixed by always initializing message buffer
to NULL.
Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
Cc: jianfeng@intel.com
Cc: jia@intel.com
Cc: gi.z.zh...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
---
lib/librte_eal/common/eal_common_proc.c | 6 +++---
lib/librte_eal/common/include/rte_eal.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_proc.c
b/lib/librte_eal/common/eal_common_proc.c
index b46d644b3..abaff5164 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -927,13 +927,13 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
rte_mp_reply *reply,
RTE_LOG(DEBUG, EAL, "request: %s\n", req->name);
- if (check_input(req) == false)
- return -1;
-
reply->nb_sent = 0;
reply->nb_received = 0;
reply->msgs = NULL;
+ if (check_input(req) == false)
+ return -1;
+
if (internal_config.no_shconf) {
RTE_LOG(DEBUG, EAL, "No shared files mode enabled, IPC is
disabled\n");
return 0;
diff --git a/lib/librte_eal/common/include/rte_eal.h
b/lib/librte_eal/common/include/rte_eal.h
index 833433229..575f8119e 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -309,7 +309,8 @@ rte_mp_sendmsg(struct rte_mp_msg *msg);
* This function sends a request message to the peer process, and will
* block until receiving reply message from the peer process.
*
- * @note The caller is responsible to free reply->replies.
+ * @note The caller is responsible to free reply->replies (even if the function
+ *returned failure).
*
* @param req
* The req argument contains the customized request message.
These patches should've been submitted as one patchset - otherwise it is
very difficult to review it. Please resubmit as a proper patchset.
As far as the code itself goes,
Acked-by: Anatoly Burakov
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH 1/8] ipc: fix rte_mp_request_sync memleak
On 17-Apr-19 3:38 PM, Herakliusz Lipiec wrote:
When sending multiple requests, rte_mp_request_sync
can succeed sending a few of those requests, but then
fail on a later one and in the end return with rc=-1.
The upper layers - e.g. device hotplug - currently
handles this case as if no messages were sent and no
memory for response buffers was allocated, which is
not true. Fixed by always initializing message buffer
to NULL.
Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
Cc: jianfeng@intel.com
Cc: jia@intel.com
Cc: gi.z.zh...@intel.com
Cc: sta...@dpdk.org
Also, Qi Zhang's email is wrong. Please don't write anything out
manually, copy-paste is your friend :)
--
Thanks,
Anatoly
[dpdk-dev] [PATCH] net/sfc: fix MTU change to check Rx scatter consistency
From: Igor Romanov
Rx queue setup function checks configured MTU to make sure that
no oversized packets can be received. But a following call to
set MTU function might make this check irrelevant.
Add a function to check MTU size against Rx buffer size and
additional Rx queue info, including Rx scatter offload.
Fixes: e961cf425e02 ("net/sfc: support MTU change")
Cc: sta...@dpdk.org
Signed-off-by: Igor Romanov
Signed-off-by: Andrew Rybchenko
---
drivers/net/sfc/sfc_ethdev.c | 34 ++
drivers/net/sfc/sfc_rx.c | 22 ++
drivers/net/sfc/sfc_rx.h | 4
3 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index ff192314d..a007d4564 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -865,6 +865,34 @@ sfc_flow_ctrl_set(struct rte_eth_dev *dev, struct
rte_eth_fc_conf *fc_conf)
return -rc;
}
+static int
+sfc_check_scatter_on_all_rx_queues(struct sfc_adapter *sa, size_t pdu)
+{
+ struct sfc_adapter_shared * const sas = sfc_sa2shared(sa);
+ const efx_nic_cfg_t *encp = efx_nic_cfg_get(sa->nic);
+ boolean_t scatter_enabled;
+ const char *error;
+ unsigned int i;
+
+ for (i = 0; i < sas->rxq_count; i++) {
+ if ((sas->rxq_info[i].state & SFC_RXQ_INITIALIZED) == 0)
+ continue;
+
+ scatter_enabled = (sas->rxq_info[i].type_flags &
+ EFX_RXQ_FLAG_SCATTER);
+
+ if (!sfc_rx_check_scatter(pdu, sa->rxq_ctrl[i].buf_size,
+ encp->enc_rx_prefix_size,
+ scatter_enabled, &error)) {
+ sfc_err(sa, "MTU check for RxQ %u failed: %s", i,
+ error);
+ return EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int
sfc_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
{
@@ -891,6 +919,10 @@ sfc_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
sfc_adapter_lock(sa);
+ rc = sfc_check_scatter_on_all_rx_queues(sa, pdu);
+ if (rc != 0)
+ goto fail_check_scatter;
+
if (pdu != sa->port.pdu) {
if (sa->state == SFC_ADAPTER_STARTED) {
sfc_stop(sa);
@@ -927,6 +959,8 @@ sfc_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
sfc_err(sa, "cannot start with neither new (%u) nor old (%u) "
"PDU max size - port is stopped",
(unsigned int)pdu, (unsigned int)old_pdu);
+
+fail_check_scatter:
sfc_adapter_unlock(sa);
fail_inval:
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index 5d8c2765c..f2221119c 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -360,6 +360,18 @@ sfc_efx_rx_qdesc_status(struct sfc_dp_rxq *dp_rxq,
uint16_t offset)
return RTE_ETH_RX_DESC_UNAVAIL;
}
+boolean_t
+sfc_rx_check_scatter(size_t pdu, size_t rx_buf_size, uint32_t rx_prefix_size,
+boolean_t rx_scatter_enabled, const char **error)
+{
+ if ((rx_buf_size < pdu + rx_prefix_size) && !rx_scatter_enabled) {
+ *error = "Rx scatter is disabled and RxQ mbuf pool object size
is too small";
+ return B_FALSE;
+ }
+
+ return B_TRUE;
+}
+
/** Get Rx datapath ops by the datapath RxQ handle */
const struct sfc_dp_rx *
sfc_dp_rx_by_dp_rxq(const struct sfc_dp_rxq *dp_rxq)
@@ -975,6 +987,7 @@ sfc_rx_qinit(struct sfc_adapter *sa, unsigned int sw_index,
struct sfc_dp_rx_qcreate_info info;
struct sfc_dp_rx_hw_limits hw_limits;
uint16_t rx_free_thresh;
+ const char *error;
memset(&hw_limits, 0, sizeof(hw_limits));
hw_limits.rxq_max_entries = sa->rxq_max_entries;
@@ -1005,10 +1018,11 @@ sfc_rx_qinit(struct sfc_adapter *sa, unsigned int
sw_index,
goto fail_bad_conf;
}
- if ((buf_size < sa->port.pdu + encp->enc_rx_prefix_size) &&
- (~offloads & DEV_RX_OFFLOAD_SCATTER)) {
- sfc_err(sa, "Rx scatter is disabled and RxQ %u mbuf pool "
- "object size is too small", sw_index);
+ if (!sfc_rx_check_scatter(sa->port.pdu, buf_size,
+ encp->enc_rx_prefix_size,
+ (offloads & DEV_RX_OFFLOAD_SCATTER),
+ &error)) {
+ sfc_err(sa, "RxQ %u MTU check failed: %s", sw_index, error);
sfc_err(sa, "RxQ %u calculated Rx buffer size is %u vs "
"PDU size %u plus Rx prefix %u bytes",
sw_index, buf_size, (unsigned int)sa->port.pdu,
diff --git a/drivers/net/sfc/sfc_rx.h b/drivers/net/sfc/sfc_rx.h
index ee1402022..aca0925b5 100644
--- a/drivers/net/sfc/sfc_rx.h
+++ b/drivers/net/sfc/sfc
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun
Hi Thomas,
On 22/4/2019 10:54 PM, Thomas Monjalon wrote:
10/04/2019 14:49, David Hunt:
The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
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
Signed-off-by: David Hunt
It seems to have been fixed in another patch, isn't it?
It was not fixed in another patch, although I can see the confusion.
A previous patch made the #defines more consistent, and
POWER_MGR_MAX_CPUS was changed to RTE_MAX_LCORE on the affected line.
However, this was later revealed as a coverity issue, and was fixed in
this patch to be RTE_LCORE_MAX_FREQS, which is the size of the array
it's trying to index into.
So looking at RC2, this patch is still needed.
Rgds,
Dave.
Re: [dpdk-dev] [PATCH 4/8] ipc: fix vfio memleak
On 17-Apr-19 3:41 PM, Herakliusz Lipiec wrote:
When sending multiple requests, rte_mp_request_sync
can succeed sending a few of those requests, but then
fail on a later one and in the end return with rc=-1.
The upper layers - e.g. device hotplug - currently
handles this case as if no messages were sent and no
memory for response buffers was allocated, which is
not true. Fixed by always freeing reply message buffers.
This commit message is duplicated in multiple commits, and does not
really describe the problem you're fixing. Since you've already updated
the IPC API docs to ask caller to free even in case of failure, i think
here (and in other similar patches) it is sufficient to just say
something like:
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Fixes: 83a73c5fef66 ("vfio: use generic multi-process channel")
Cc: jianfeng@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
---
For actual code,
Acked-by: Anatoly Burakov
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun
23/04/2019 10:21, Hunt, David:
> Hi Thomas,
>
> On 22/4/2019 10:54 PM, Thomas Monjalon wrote:
> > 10/04/2019 14:49, David Hunt:
> >> The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
> >> yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
> >> 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
> >>
> >> Signed-off-by: David Hunt
> > It seems to have been fixed in another patch, isn't it?
> >
>
> It was not fixed in another patch, although I can see the confusion.
>
> A previous patch made the #defines more consistent, and
> POWER_MGR_MAX_CPUS was changed to RTE_MAX_LCORE on the affected line.
> However, this was later revealed as a coverity issue, and was fixed in
> this patch to be RTE_LCORE_MAX_FREQS, which is the size of the array
> it's trying to index into.
>
> So looking at RC2, this patch is still needed.
I think it needs to be rebased in a v2 then.
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun
On 23/4/2019 9:33 AM, Thomas Monjalon wrote:
23/04/2019 10:21, Hunt, David:
Hi Thomas,
On 22/4/2019 10:54 PM, Thomas Monjalon wrote:
10/04/2019 14:49, David Hunt:
The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
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
Signed-off-by: David Hunt
It seems to have been fixed in another patch, isn't it?
It was not fixed in another patch, although I can see the confusion.
A previous patch made the #defines more consistent, and
POWER_MGR_MAX_CPUS was changed to RTE_MAX_LCORE on the affected line.
However, this was later revealed as a coverity issue, and was fixed in
this patch to be RTE_LCORE_MAX_FREQS, which is the size of the array
it's trying to index into.
So looking at RC2, this patch is still needed.
I think it needs to be rebased in a v2 then.
Sure, will do.
Re: [dpdk-dev] [PATCH] examples/l3fwd: fix em mode datapath selection
23/04/2019 04:47, Pavan Nikhilesh Bhagavatula:
>From: Thomas Monjalon
> >10/04/2019 09:29, Pavan Nikhilesh Bhagavatula:
> >> From: Pavan Nikhilesh
> >>
> >> Currently, l3wfd em mode has two datapath modes em_sequential and
> >> em_hlm. We can select either of them by defining
> >NO_HASH_MULTI_LOOKUP
> >> to one or zero.
> >> The code checks if NO_HASH_MULTI_LOOKUP is defined or not instead of
> >> checking for the value.
> >>
> >> Fixes: 52c97adc1f0f ("examples/l3fwd: fix exact match performance")
> >> Cc: sta...@dpdk.org
> >>
> >> Signed-off-by: Pavan Nikhilesh
> >> ---
> >> --- a/examples/l3fwd/l3fwd_em.c
> >> +++ b/examples/l3fwd/l3fwd_em.c
> >> #if defined RTE_ARCH_X86 || defined RTE_MACHINE_CPUFLAG_NEON -#if
> >> defined(NO_HASH_MULTI_LOOKUP)
> >> +#if NO_HASH_MULTI_LOOKUP
> >
> >A quick grep shows that it used in another place with #ifdef:
> >
> >examples/l3fwd/l3fwd.h:#if !defined(NO_HASH_MULTI_LOOKUP) &&
> >defined(RTE_MACHINE_CPUFLAG_NEON)
> >
> >
>
> #if !defined(NO_HASH_MULTI_LOOKUP) && defined(RTE_MACHINE_CPUFLAG_NEON)
> #define NO_HASH_MULTI_LOOKUP 1
> #endif
>
> This macro is used to set l3fwd_em_sequential as the default EM datapath on
> AARCH64
> as its performance is better. (http://patches.dpdk.org/patch/49372/)
>
> make -C examples/l3fwd #Selects l3fwd_em_sequential by default on
> AARCH 64
>
> Currently, we cannot select em_hlm without manually editing the macro as
> using the below command still
> sets em_sequential as the default datapath because the macro modified in the
> patch that selects the datapath
> checks if NO_HASH_MULTI_LOOKUP is defined or not rather than its value.
>
> EXTRA_CFLAGS='-DNO_HASH_MULTI_LOOKUP=0' make -C examples/l3fwd
>
> I hope I cleared up things a bit.
In my understanding, we should check the value in the other case too,
instead of #if defined.
Re: [dpdk-dev] [dpdk-stable] [PATCH v2] raw/ifpga_rawdev: fix fd leak in rte fpga do pr
When you need to re-send a patch, please apply it with "git am"
so the original author is kept.
Another nit: the title is full of abbreviation, it is hard to read.
The title prefix should be "raw/ifpga", even if checkpatch complains.
Please re-send, thanks.
23/04/2019 03:36, Xu, Rosen:
> Hi Thomas,
>
> Qian has some problem with his email. So some patch can't be found in patch
> work.
> For it's a urgent bug. After we have aligned with Qian, Andy send this patch.
> Thanks.
>
> > -Original Message-
> > From: Pei, Andy
> > Sent: Tuesday, April 23, 2019 9:19
> > To: Thomas Monjalon ; Xu, Rosen
> >
> > Cc: sta...@dpdk.org; dev@dpdk.org; Kovacevic, Marko
> > ; liq...@163.com
> > Subject: RE: [dpdk-stable] [PATCH v2] raw/ifpga_rawdev: fix fd leak in rte
> > fpga do pr
> >
> > Hi Thomas,
> >
> > My patch is the same as LI Qiang's patch.
> > I was told that Qiang's patch cannot get onto the patchwork, so I just help
> > him do this.
> >
> > -Original Message-
> > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > Sent: Tuesday, April 23, 2019 1:52 AM
> > To: Xu, Rosen ; Pei, Andy
> > Cc: sta...@dpdk.org; dev@dpdk.org; Kovacevic, Marko
> > ; liq...@163.com
> > Subject: Re: [dpdk-stable] [PATCH v2] raw/ifpga_rawdev: fix fd leak in rte
> > fpga do pr
> >
> > > > In rte_fpga_do_pr() function, if 'stat' returns error,
> > > > rte_fpga_do_pr() returns -EINVAL without closing the 'file_fd' that
> > > > has been opened.
> > > > After this patch, 'file_fd' is closed before rte_fpga_do_pr()
> > > > returns -EINVAL when 'stat' returns error
> > > >
> > > > Coverity issue: 279441
> > > > Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
> > > > Cc: rosen...@intel.com
> > > > Cc: marko.kovace...@intel.com
> > > > Cc: liq...@163.com
> > > > Cc: andy@intel.com
> > > > Cc: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Andy Pei
> > >
> > > Acked-by: Rosen Xu
> >
> > Li Qiang proposed the same patch.
> > Why he is not referenced here?
> > Which patch should I merge?
Re: [dpdk-dev] [PATCH] eventdev: add experimental tag back
> -Original Message-
> From: Nikhil Rao
> Sent: Tuesday, April 23, 2019 11:20 AM
> To: tho...@monjalon.net
> Cc: dev@dpdk.org; Nikhil Rao ; Jerin Jacob Kollanukkaran
>
> Subject: [EXT] [PATCH] eventdev: add experimental tag back
>
> --
> Add the experimental tag back to the Rx event adapter callback and the Rx
> event
> callback register functions. This patch also adds the experimental tag in the
> callback register function definition and adds the function 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
> ---
diff --git a/lib/librte_eventdev/rte_eventdev_version.map
> b/lib/librte_eventdev/rte_eventdev_version.map
> index 88c3ce5..9bfc666 100644
> --- a/lib/librte_eventdev/rte_eventdev_version.map
> +++ b/lib/librte_eventdev/rte_eventdev_version.map
> @@ -124,3 +124,9 @@ DPDK_19.05 {
> rte_event_timer_arm_tmo_tick_burst;
> rte_event_timer_cancel_burst;
> } DPDK_18.05;
> +
> +EXPERIMENTAL {
> + global:
> +
> + rte_event_eth_rx_adapter_cb_register;
This symbol is present in DPDK_19.05 section above.
Please remove the duplicate entry from DPDK_19.05.
With above fix:
Acked-by: Jerin Jacob
> +};
> --
> 1.8.3.1
[dpdk-dev] [PATCH v2] 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
Signed-off-by: David Hunt
Acked-by: Reshma Pattan
---
v2 - Rebase to 19.05 RC2.
---
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
Re: [dpdk-dev] [PATCH v4] kni: add IOVA va support for kni
On 22-Apr-19 5:39 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
---
+/* iova to kernel virtual address */
+static void *
+iova2kva(struct kni_dev *kni, void *pa)
+{
+ return phys_to_virt(iommu_iova_to_phys(kni->domain,
+ (dma_addr_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,
+ (dma_addr_t)m->buf_physaddr) +
+m->data_off));
Does this account for mbufs crossing page boundary? In IOVA as VA mode,
the mempool is likely allocated in one go, so the mempool allocator will
not care for preventing mbufs from crossing page boundary. The data may
very well start at the very end of a page, and continue through the
beginning of next page, which will have a different physical address.
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH] examples/l3fwd: fix em mode datapath selection
>-Original Message-
>From: dev On Behalf Of Thomas Monjalon
>Sent: Tuesday, April 23, 2019 2:05 PM
>To: Pavan Nikhilesh Bhagavatula
>Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran ; Marko
>Kovacevic ; Ori Kam ;
>Bruce Richardson ; Pablo de Lara
>; Radu Nicolau ;
>Akhil Goyal ; Tomasz Kantecki
>; sta...@dpdk.org
>Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: fix em mode datapath
>selection
>
>23/04/2019 04:47, Pavan Nikhilesh Bhagavatula:
>>From: Thomas Monjalon
>> >10/04/2019 09:29, Pavan Nikhilesh Bhagavatula:
>> >> From: Pavan Nikhilesh
>> >>
>> >> Currently, l3wfd em mode has two datapath modes em_sequential and
>> >> em_hlm. We can select either of them by defining
>> >NO_HASH_MULTI_LOOKUP
>> >> to one or zero.
>> >> The code checks if NO_HASH_MULTI_LOOKUP is defined or not instead
>> >> of checking for the value.
>> >>
>> >> Fixes: 52c97adc1f0f ("examples/l3fwd: fix exact match performance")
>> >> Cc: sta...@dpdk.org
>> >>
>> >> Signed-off-by: Pavan Nikhilesh
>> >> ---
>> >> --- a/examples/l3fwd/l3fwd_em.c
>> >> +++ b/examples/l3fwd/l3fwd_em.c
>> >> #if defined RTE_ARCH_X86 || defined RTE_MACHINE_CPUFLAG_NEON
>-#if
>> >> defined(NO_HASH_MULTI_LOOKUP)
>> >> +#if NO_HASH_MULTI_LOOKUP
>> >
>> >A quick grep shows that it used in another place with #ifdef:
>> >
>> >examples/l3fwd/l3fwd.h:#if !defined(NO_HASH_MULTI_LOOKUP) &&
>> >defined(RTE_MACHINE_CPUFLAG_NEON)
>> >
>> >
>>
>> #if !defined(NO_HASH_MULTI_LOOKUP) &&
>> defined(RTE_MACHINE_CPUFLAG_NEON) #define
>NO_HASH_MULTI_LOOKUP 1
>> #endif
>>
>> This macro is used to set l3fwd_em_sequential as the default EM
>> datapath on AARCH64 as its performance is better.
>> (http://patches.dpdk.org/patch/49372/)
>>
>> make -C examples/l3fwd #Selects l3fwd_em_sequential by default on
>AARCH 64
>>
>> Currently, we cannot select em_hlm without manually editing the macro
>> as using the below command still sets em_sequential as the default
>> datapath because the macro modified in the patch that selects the datapath
>checks if NO_HASH_MULTI_LOOKUP is defined or not rather than its value.
>>
>> EXTRA_CFLAGS='-DNO_HASH_MULTI_LOOKUP=0' make -C examples/l3fwd
>>
>> I hope I cleared up things a bit.
>
>In my understanding, we should check the value in the other case too, instead
>of #if defined.
That will lead to undefined and redefined error:
[dpdk] # make -C examples/l3fwd
make: Entering directory '/root/pavan/dpdk-int/examples/l3fwd'
CC main.o
CC l3fwd_lpm.o
In file included from /root/dpdk/examples/l3fwd/l3fwd_lpm.c:28:0:
/root/dpdk/examples/l3fwd/l3fwd.h:14:6: error: "NO_HASH_MULTI_LOOKUP" is not
defined, evaluates to 0 [-Werror=undef]
#if !NO_HASH_MULTI_LOOKUP && defined(RTE_MACHINE_CPUFLAG_NEON)
[dpdk] # EXTRA_CFLAGS='-DNO_HASH_MULTI_LOOKUP=0' make -C examples/l3fwd
make: Entering directory '/root/dpdk/examples/l3fwd'
CC main.o
]
#define NO_HASH_MULTI_LOOKUP 1
n file included from /root/dpdk/examples/l3fwd/l3fwd_lpm.c:28:0:
:0:0: note:
]
#define NO_HASH_MULTI_LOOKUP 1is the location of the previous definition
>
>
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun
On 18/04/2019 16:14, Pattan, Reshma wrote:
>> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>> Coverity issue: 337660
> Candidate for sta...@dpdk.org?
There is no reply to this comment - re-asking as I will probably have
the same question in a few weeks :-)
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun
Hi Kevin,
On 23/4/2019 11:26 AM, Kevin Traynor wrote:
On 18/04/2019 16:14, Pattan, Reshma wrote:
Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
Coverity issue: 337660
Candidate for sta...@dpdk.org?
There is no reply to this comment - re-asking as I will probably have
the same question in a few weeks :-)
There was a v2 this morning and it had a --cc stable on it. :)
Rgds,
Dave.
Re: [dpdk-dev] [PATCH] net/ice: fix coverity issues
On 4/18/2019 2:31 AM, Wenzhuo Lu wrote:
> Fix the issues reported by Coverity check, "Null-checking
> vsi suggests that it may be null, but it has already been
> dereferenced on all paths leading to the check."
Hi Wenzhuo,
Can you please list the coverity issues addressed, the format we have is:
Coverity issue:
Also the patch title doesn't say much, can you please put what exactly fixed,
"fix possible null pointer dereference" ?
>
> Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops")
> Fixes: ff963bfa7cb1 ("net/ice: support RSS")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Wenzhuo Lu
<...>
Re: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun
On 23/04/2019 11:31, Hunt, David wrote:
> Hi Kevin,
>
> On 23/4/2019 11:26 AM, Kevin Traynor wrote:
>> On 18/04/2019 16:14, Pattan, Reshma wrote:
Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
Coverity issue: 337660
>>> Candidate for sta...@dpdk.org?
>> There is no reply to this comment - re-asking as I will probably have
>> the same question in a few weeks :-)
>
>
> There was a v2 this morning and it had a --cc stable on it. :)
>
Yes, you're right, but that just puts it in the email and it gets lost
from the commit message. The 'Cc: sta...@dpdk.org' tag is needed in the
commit message so it can be picked up by scripts later when finding
which patches should be backported.
https://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported
As it's discussed, maybe Thomas can add it for this patch when applying.
thanks,
Kevin.
> Rgds,
> Dave.
>
[dpdk-dev] [PATCH v2] doc: fix interactive commands in testpmd guide
Added some missing documentation for interactive mode commands
Fixes: 01b2092a5e ("testpmd: add dump commands for debug")
Fixes: caf05a1b86 ("app/testpmd: new command to dump log types")
Fixes: 0f62d63593ed ("app/testpmd: support tunneled TSO in checksum engine")
Fixes: 8fff667578a7
("app/testpmd: new command to add/remove multicast MAC addresses")
Fixes: ae03d0d18adf ("app/testpmd: command to configure MTU")
Fixes: f79959ea1504 ("app/testpmd: allow to configure RSS hash key")
Fixes: caf05a1b8608 ("app/testpmd: new command to dump log types")
Cc: sta...@dpdk.org
Signed-off-by: Agalya Babu RadhaKrishnan
Reviewed-by: Rami Rosen
Acked-by: Bernard Iremonger
---
v2: Added fixes lines and addressed comments.
---
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 100
1 file changed, 100 insertions(+)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5d4dc6f0c..cdcc51957 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -467,6 +467,56 @@ Show Tx metadata value set for a specific port::
testpmd> show port (port_id) tx_metadata
+dump physmem
+
+
+Dumps all physical memory segment layouts::
+
+ testpmd> dump_physmem
+
+dump memzone
+
+
+Dumps the layout of all memory zones::
+
+ testpmd> dump_memzone
+
+
+dump struct size
+
+
+Dumps the size of all memory structures::
+
+ testpmd> dump_struct_sizes
+
+dump ring
+~
+
+Dumps the status of all or specific element in DPDK rings::
+
+ testpmd> dump_ring [ring_name]
+
+dump mempool
+
+
+Dumps the statistics of all or specific memory pool::
+
+ testpmd> dump_mempool [mempool_name]
+
+dump devargs
+
+
+Dumps the user device list::
+
+ testpmd> dump_devargs
+
+dump log types
+~~
+
+Dumps the log level for all the dpdk modules::
+
+ testpmd> dump_log_types
+
Configuration Functions
---
@@ -1041,6 +1091,20 @@ Display the status of TCP Segmentation Offload::
testpmd> tso show (port_id)
+tunnel tso set
+~~
+
+Set tso segment size of tunneled packets for a port in csum engine::
+
+ testpmd> tunnel_tso set (tso_segsz) (port_id)
+
+tunnel tso show
+~~~
+
+Display the status of tunneled TCP Segmentation Offload for a port::
+
+ testpmd> tunnel_tso show (port_id)
+
set port - gro
~~
@@ -1162,6 +1226,22 @@ Remove a MAC address from a port::
testpmd> mac_addr remove (port_id) (XX:XX:XX:XX:XX:XX)
+mcast_addr add
+~~
+
+To add the multicast MAC address to/from the set of multicast addresses
+filtered by port::
+
+ testpmd> mcast_addr add (port_id) (mcast_addr)
+
+mcast_addr remove
+~
+
+To remove the multicast MAC address to/from the set of multicast addresses
+filtered by port::
+
+ testpmd> mcast_addr remove (port_id) (mcast_addr)
+
mac_addr add (for VF)
~
@@ -2183,6 +2263,26 @@ testpmd will add this value to any Tx packet sent from
this port::
testpmd> port config (port_id) tx_metadata (value)
+port config mtu
+~~~
+
+To configure MTU(Maximum Transmission Unit) on devices using testpmd::
+
+ testpmd> port config mtu (port_id) (value)
+
+port config rss hash key
+
+
+To configure the RSS hash key used to compute the RSS
+hash of input [IP] packets received on port::
+
+ testpmd> port config rss-hash-key (ipv4|ipv4-frag|\
+ ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|\
+ ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|\
+ ipv6-other|l2-payload|ipv6-ex|ipv6-tcp-ex|\
+ ipv6-udp-ex )
+
Link Bonding Functions
--
--
2.17.2
Re: [dpdk-dev] [PATCH] net/iavf: fix stats reset
On 4/22/2019 3:18 AM, Qiming Yang wrote:
> stats_reset has been missed when support stats in iavf driver.
> This patch add statistics reset function.
>
> Fixes: f4a41a6953af ("net/avf: support stats")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Qiming Yang
<...>
> @@ -977,16 +979,71 @@ iavf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
> return 0;
> }
>
> +static void
> +iavf_stat_update_48(uint64_t *offset,
> +uint64_t *stat)
> +{
> + if (*stat >= *offset)
> + *stat = *stat - *offset;
> + else
> + *stat = (uint64_t)((*stat +
> + ((uint64_t)1 << IAVF_48_BIT_WIDTH)) - *offset);
> +
> + *stat &= IAVF_48_BIT_MASK;
> +}
> +
> +static void
> +iavf_stat_update_32(uint64_t *offset,
> +uint64_t *stat)
> +{
> + if (*stat >= *offset)
> + *stat = (uint64_t)(*stat - *offset);
> + else
> + *stat = (uint64_t)((*stat +
> + ((uint64_t)1 << IAVF_32_BIT_WIDTH)) - *offset);
> +}
> +
> +static void
> +iavf_update_stats(struct iavf_vsi *vsi,
> + struct virtchnl_eth_stats *nes)
This syntax looks odd, will fix while merging, similar to above functions, no
need to break parameter lines into multiple line, will update them too.
Re: [dpdk-dev] [PATCH] eventdev: add experimental tag back
On 23/04/2019 06:50, Nikhil Rao wrote:
> Add the experimental tag back to the Rx event adapter callback
> and the Rx event callback register functions. This patch also
You should add an explanation why in the commit message. It is an
unusual thing to do and the other mail thread would be too hard to find.
> adds the experimental tag in the callback register
> function definition and adds the function 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
> ---
> lib/librte_eventdev/rte_event_eth_rx_adapter.h | 8 +++-
> lib/librte_eventdev/rte_event_eth_rx_adapter.c | 9 +
> lib/librte_eventdev/rte_eventdev_version.map | 6 ++
> 3 files changed, 18 insertions(+), 5 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..32b52a7 100644
> --- a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> @@ -201,6 +201,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
> @@ -437,6 +440,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 +459,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..e6001e2 100644
> --- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> @@ -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..9bfc666 100644
> --- a/lib/librte_eventdev/rte_eventdev_version.map
> +++ b/lib/librte_eventdev/rte_eventdev_version.map
> @@ -124,3 +124,9 @@ DPDK_19.05 {
> rte_event_timer_arm_tmo_tick_burst;
> rte_event_timer_cancel_burst;
> } DPDK_18.05;
> +
> +EXPERIMENTAL {
> + global:
> +
> + rte_event_eth_rx_adapter_cb_register;
> +};
>
Re: [dpdk-dev] [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped for inline crypto
Hi Bernard,
> -Original Message-
> From: Akhil Goyal
> Sent: Thursday, April 18, 2019 7:21 PM
> To: Bernard Iremonger ; dev@dpdk.org;
> konstantin.anan...@intel.com
> Cc: sta...@dpdk.org
> Subject: RE: [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped for
> inline crypto
>
> Hi Bernard,
>
> > - RTE_LOG_DP(DEBUG, IPSEC, "Create session for SA spi %u on cryptodev
> > "
> > - "%u qp %u\n", sa->spi,
> > - ipsec_ctx->tbl[cdev_id_qp].id,
> > - ipsec_ctx->tbl[cdev_id_qp].qp);
> > + if ((sa == NULL) || (pool == NULL))
> > + return -EINVAL;
> >
> > - if (sa->type != RTE_SECURITY_ACTION_TYPE_NONE) {
> > - struct rte_security_session_conf sess_conf = {
> > + struct rte_security_session_conf sess_conf = {
> > .action_type = sa->type,
> > .protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> > {.ipsec = {
> > @@ -90,247 +65,340 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct
> > ipsec_sa *sa)
> > } },
> > .crypto_xform = sa->xforms,
> > .userdata = NULL,
> > -
> > };
> >
> > - if (sa->type ==
> RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL)
> > {
> > - struct rte_security_ctx *ctx = (struct
> > rte_security_ctx *)
> > -
> > rte_cryptodev_get_sec_ctx(
> > -
> > ipsec_ctx->tbl[cdev_id_qp].id);
> > -
> > - /* Set IPsec parameters in conf */
> > - set_ipsec_conf(sa, &(sess_conf.ipsec));
> > -
> > - sa->sec_session = rte_security_session_create(ctx,
> > - &sess_conf,
> > ipsec_ctx->session_pool);
> > - if (sa->sec_session == NULL) {
> > - RTE_LOG(ERR, IPSEC,
> > - "SEC Session init failed: err: %d\n", ret);
> > - return -1;
> > - }
> > - } else if (sa->type ==
> > RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> {
> > - struct rte_flow_error err;
> > - struct rte_security_ctx *ctx = (struct
> > rte_security_ctx *)
> > -
> > rte_eth_dev_get_sec_ctx(
> > - sa->portid);
> > - const struct rte_security_capability *sec_cap;
> > - int ret = 0;
> > -
> > - sa->sec_session = rte_security_session_create(ctx,
> > - &sess_conf,
> > ipsec_ctx->session_pool);
> > - if (sa->sec_session == NULL) {
> > - RTE_LOG(ERR, IPSEC,
> > - "SEC Session init failed: err: %d\n", ret);
> > - return -1;
> > - }
> > + if (sa->type == RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
> > + ctx = (struct rte_security_ctx *)
> > + rte_eth_dev_get_sec_ctx(sa->portid);
>
> This is breaking the lookaside mode. Ctx was retrieved using the
> ipsec_ctx->tbl
> struct rte_security_ctx *ctx = (struct rte_security_ctx *)
> rte_cryptodev_get_sec_ctx(
> ipsec_ctx->tbl[cdev_id_qp].id);
>
> I am looking into it, but I don't have time left to get it integrated in RC2.
> So this
> has to be pushed to RC3
It looks like there are multiple issues in this patch wrt lookaside and none
cases. Only the inline cases seem to be working.
1. the patch removes the cdev_mapping concept completely. Cdev_id_qp is not
getting used.
The port_id cannot be used in case of crypto, the mapping of cdev/qp/core
is done differently for inbound and outbound ports which is missed in this
patch.
2. crypto sessions are created using the session mempool and the private data
is allocated using the session priv_mempool which is removed in this patch.
This will break cases where the priv data is more than the size of sess_mp
element size.
Also the security sessions need to be allocated using the session_priv_mp
instead of the session_mp.
Please check this one.
http://patches.dpdk.org/patch/52981/
Ideally this issue should be resolved by adding another parameter in
rte_security_session_create which can take another mempool pointer for private
data allocation. But this cannot be done in this release as it would need a
deprecation notice.
With the above issues I don't see your patch going in 19.05 release cycle.
Regards,
Akhil
>
>
>
> >
> > - sec_cap = rte_security_capabilities_get(ctx);
[dpdk-dev] [PATCH] net/mlx5: fix E-Switch flow without port item
When creating a flow rule without the port_id pattern item, always the
PF was selected.
This commit fixes this issue, if no port_id pattern item is available
then we use the port that the flow was created on as source port.
Fixes: 822fb3195348 ("net/mlx5: add port id item to Direct Verbs")
Signed-off-by: Ori Kam
---
drivers/net/mlx5/mlx5_flow_dv.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index c2a2fc6..d17adbe 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3623,6 +3623,8 @@ struct field_modify_info modify_tcp[] = {
union flow_dv_attr flow_attr = { .attr = 0 };
struct mlx5_flow_dv_tag_resource tag_resource;
uint32_t modify_action_position = UINT32_MAX;
+ void *match_mask = matcher.mask.buf;
+ void *match_value = dev_flow->dv.value.buf;
flow->group = attr->group;
if (attr->transfer)
@@ -3895,23 +3897,8 @@ struct field_modify_info modify_tcp[] = {
}
dev_flow->dv.actions_n = actions_n;
flow->actions = action_flags;
- if (attr->ingress && !attr->transfer &&
- (priv->representor || priv->master)) {
- /* It was validated - we support unidirection flows only. */
- assert(!attr->egress);
- /*
-* Add matching on source vport index only
-* for ingress rules in E-Switch configurations.
-*/
- flow_dv_translate_item_source_vport(matcher.mask.buf,
- dev_flow->dv.value.buf,
- priv->vport_id,
- 0x);
- }
for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
- void *match_mask = matcher.mask.buf;
- void *match_value = dev_flow->dv.value.buf;
switch (items->type) {
case RTE_FLOW_ITEM_TYPE_PORT_ID:
@@ -4018,6 +4005,19 @@ struct field_modify_info modify_tcp[] = {
}
item_flags |= last_item;
}
+ if (((attr->ingress && !attr->transfer) ||
+(attr->transfer && !(item_flags & MLX5_FLOW_ITEM_PORT_ID))) &&
+ (priv->representor || priv->master)) {
+ /* It was validated - we support unidirection flows only. */
+ assert(!attr->egress);
+ /*
+* Add matching on source vport index only
+* for ingress rules in E-Switch configurations.
+*/
+ if (flow_dv_translate_item_port_id(dev, match_mask,
+ match_value, NULL))
+ return -rte_errno;
+ }
assert(!flow_dv_check_valid_spec(matcher.mask.buf,
dev_flow->dv.value.buf));
dev_flow->layers = item_flags;
--
1.8.3.1
Re: [dpdk-dev] [RFC v2 1/2] eal: replace libc-based random number generation with LFSR
On Mon, Apr 22, 2019 at 07:44:39PM +0200, Mattias Rönnblom wrote: > On 2019-04-22 17:52, Mattias Rönnblom wrote: > > On 2019-04-22 13:34, Neil Horman wrote: > > > > > > +uint64_t __rte_experimental > > > > +rte_rand(void) > > > Do you really want to mark this as experimental? I know it will > > > trigger the > > > symbol checker with a warning if you don't, but this function > > > already existed > > > previously and was accepted as part of the ABI. Given that the > > > prototype hasn't > > > changed, I think you just need to accept it as a non-experimental > > > function > > > > > > > I'll remove the experimental tag and move it into the 19_05 section > > (without suggesting it should go into 19.05). That maneuver seems not to > > trigger any build warnings/errors. > > > > OK, so that wasn't true. It does trigger a build error, courtesy of > buildtools/check-experimental-syms.sh. > > I can't see any obvious way around it. Ideas, anyone? > No, we would have to waive it. But its pretty clear that This function has been around forever, so I think it would be worse to demote it to an experimental symbol. The only thing you're doing here is moving it from an inline function (which is arguably part of the ABI, even if it never appeared as a symbol in the ELF file), to a fully fleged symbol with a new implementation. Neil
Re: [dpdk-dev] [PATCH] net/sfc: fix MTU change to check Rx scatter consistency
On 4/23/2019 9:14 AM, Andrew Rybchenko wrote:
> From: Igor Romanov
>
> Rx queue setup function checks configured MTU to make sure that
> no oversized packets can be received. But a following call to
> set MTU function might make this check irrelevant.
>
> Add a function to check MTU size against Rx buffer size and
> additional Rx queue info, including Rx scatter offload.
>
> Fixes: e961cf425e02 ("net/sfc: support MTU change")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Igor Romanov
> Signed-off-by: Andrew Rybchenko
Applied to dpdk-next-net/master, thanks.
Re: [dpdk-dev] [PATCH v2] doc: fix interactive commands in testpmd guide
On 4/23/2019 11:44 AM, Agalya Babu RadhaKrishnan wrote:
> Added some missing documentation for interactive mode commands
>
> Fixes: 01b2092a5e ("testpmd: add dump commands for debug")
> Fixes: caf05a1b86 ("app/testpmd: new command to dump log types")
> Fixes: 0f62d63593ed ("app/testpmd: support tunneled TSO in checksum engine")
> Fixes: 8fff667578a7
> ("app/testpmd: new command to add/remove multicast MAC addresses")
> Fixes: ae03d0d18adf ("app/testpmd: command to configure MTU")
> Fixes: f79959ea1504 ("app/testpmd: allow to configure RSS hash key")
> Fixes: caf05a1b8608 ("app/testpmd: new command to dump log types")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Agalya Babu RadhaKrishnan
>
> Reviewed-by: Rami Rosen
> Acked-by: Bernard Iremonger
Applied to dpdk-next-net/master, thanks.
Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix pool usage for security session
Hi Akhil,
> Currently, two separate mempools are being used for creating crypto
> sessions and its private data.
> crypto sessions are created and initialized separately, so a separate
> mempool is passed to each API, but in case of security sessions, where
> only one API create and initialize the private data as well.
> So if session mempool is passed to create a security session, the
> mempool element size is not sufficient enough to hold the private
> data as well.
> As a perfect solution, the security session create API should take 2
> mempools for header and private data and initiatlize accordingly,
> but that would mean an API breakage, which will be done in the next
> release cycle. So introducing this patch as a workaround to resolve this
> issue.
>
> Fixes: 261bbff75e34 ("examples: use separate crypto session mempools")
> Cc: roy.fan.zh...@intel.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Akhil Goyal
> ---
> examples/ipsec-secgw/ipsec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
> index 4352cb842..7b8533077 100644
> --- a/examples/ipsec-secgw/ipsec.c
> +++ b/examples/ipsec-secgw/ipsec.c
> @@ -102,7 +102,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct
> ipsec_sa *sa)
> set_ipsec_conf(sa, &(sess_conf.ipsec));
>
> sa->sec_session = rte_security_session_create(ctx,
> - &sess_conf, ipsec_ctx->session_pool);
> + &sess_conf,
> ipsec_ctx->session_priv_pool);
> if (sa->sec_session == NULL) {
> RTE_LOG(ERR, IPSEC,
> "SEC Session init failed: err: %d\n", ret);
> @@ -117,7 +117,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct
> ipsec_sa *sa)
> int ret = 0;
>
> sa->sec_session = rte_security_session_create(ctx,
> - &sess_conf, ipsec_ctx->session_pool);
> + &sess_conf,
> ipsec_ctx->session_priv_pool);
> if (sa->sec_session == NULL) {
> RTE_LOG(ERR, IPSEC,
> "SEC Session init failed: err: %d\n", ret);
> --
Looks good to me , but seems incomplete.
I think we also need to:
static int32_t
cryptodevs_init(void)
{
...
/* create session pools for eth devices that implement security */
RTE_ETH_FOREACH_DEV(port_id) {
if ((enabled_port_mask & (1 << port_id)) &&
rte_eth_dev_get_sec_ctx(port_id)) {
int socket_id = rte_eth_dev_socket_id(port_id);
- if (!socket_ctx[socket_id].session_pool) {
+ if (!socket_ctx[socket_id].session_priv_pool) {
char mp_name[RTE_MEMPOOL_NAMESIZE];
struct rte_mempool *sess_mp;
snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
"sess_mp_%u", socket_id);
sess_mp = rte_mempool_create(mp_name,
(CDEV_MP_NB_OBJS * 2),
max_sess_sz,
CDEV_MP_CACHE_SZ,
0, NULL, NULL, NULL,
NULL, socket_id,
0);
if (sess_mp == NULL)
rte_exit(EXIT_FAILURE,
"Cannot create session pool "
"on socket %d\n", socket_id);
else
printf("Allocated session pool "
"on socket %d\n", socket_id);
- socket_ctx[socket_id].session_pool = sess_mp;
+ socket_ctx[socket_id].session_priv_pool =
sess_mp;
}
}
}
Konstantin
> 2.17.1
Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix pool usage for security session
>
>
> Looks good to me , but seems incomplete.
> I think we also need to:
> static int32_t
> cryptodevs_init(void)
> {
>...
>
> /* create session pools for eth devices that implement security */
> RTE_ETH_FOREACH_DEV(port_id) {
> if ((enabled_port_mask & (1 << port_id)) &&
> rte_eth_dev_get_sec_ctx(port_id)) {
> int socket_id = rte_eth_dev_socket_id(port_id);
>
> - if (!socket_ctx[socket_id].session_pool) {
> + if (!socket_ctx[socket_id].session_priv_pool) {
> char mp_name[RTE_MEMPOOL_NAMESIZE];
> struct rte_mempool *sess_mp;
>
> snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
> "sess_mp_%u", socket_id);
> sess_mp = rte_mempool_create(mp_name,
> (CDEV_MP_NB_OBJS * 2),
> max_sess_sz,
> CDEV_MP_CACHE_SZ,
> 0, NULL, NULL, NULL,
> NULL, socket_id,
> 0);
> if (sess_mp == NULL)
> rte_exit(EXIT_FAILURE,
> "Cannot create session pool "
> "on socket %d\n", socket_id);
> else
> printf("Allocated session pool "
> "on socket %d\n", socket_id);
> - socket_ctx[socket_id].session_pool = sess_mp;
> + socket_ctx[socket_id].session_priv_pool =
> sess_mp;
>
> }
> }
> }
>
> Konstantin
Thanks,
Will be sending a v2 soon.
[dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix pool usage for security session
Currently, two separate mempools are being used for creating crypto
sessions and its private data.
crypto sessions are created and initialized separately, so a separate
mempool is passed to each API, but in case of security sessions, where
only one API create and initialize the private data as well.
So if session mempool is passed to create a security session, the
mempool element size is not sufficient enough to hold the private
data as well.
As a perfect solution, the security session create API should take 2
mempools for header and private data and initiatlize accordingly,
but that would mean an API breakage, which will be done in the next
release cycle. So introducing this patch as a workaround to resolve this
issue.
Fixes: 261bbff75e34 ("examples: use separate crypto session mempools")
Cc: roy.fan.zh...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Akhil Goyal
---
v2: incorporated Konstantin's comments.
examples/ipsec-secgw/ipsec-secgw.c | 5 +++--
examples/ipsec-secgw/ipsec.c | 4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/examples/ipsec-secgw/ipsec-secgw.c
b/examples/ipsec-secgw/ipsec-secgw.c
index 2e203393d..478dd80c2 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1791,7 +1791,7 @@ cryptodevs_init(void)
rte_eth_dev_get_sec_ctx(port_id)) {
int socket_id = rte_eth_dev_socket_id(port_id);
- if (!socket_ctx[socket_id].session_pool) {
+ if (!socket_ctx[socket_id].session_priv_pool) {
char mp_name[RTE_MEMPOOL_NAMESIZE];
struct rte_mempool *sess_mp;
@@ -1811,7 +1811,8 @@ cryptodevs_init(void)
else
printf("Allocated session pool "
"on socket %d\n", socket_id);
- socket_ctx[socket_id].session_pool = sess_mp;
+ socket_ctx[socket_id].session_priv_pool =
+ sess_mp;
}
}
}
diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index 4352cb842..7b8533077 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -102,7 +102,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa
*sa)
set_ipsec_conf(sa, &(sess_conf.ipsec));
sa->sec_session = rte_security_session_create(ctx,
- &sess_conf, ipsec_ctx->session_pool);
+ &sess_conf,
ipsec_ctx->session_priv_pool);
if (sa->sec_session == NULL) {
RTE_LOG(ERR, IPSEC,
"SEC Session init failed: err: %d\n", ret);
@@ -117,7 +117,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa
*sa)
int ret = 0;
sa->sec_session = rte_security_session_create(ctx,
- &sess_conf, ipsec_ctx->session_pool);
+ &sess_conf,
ipsec_ctx->session_priv_pool);
if (sa->sec_session == NULL) {
RTE_LOG(ERR, IPSEC,
"SEC Session init failed: err: %d\n", ret);
--
2.17.1
Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix pool usage for security session
> -Original Message-
> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> Sent: Tuesday, April 23, 2019 1:20 PM
> To: dev@dpdk.org
> Cc: Nicolau, Radu ; Ananyev, Konstantin
> ; Akhil Goyal ;
> Zhang, Roy Fan ; sta...@dpdk.org
> Subject: [PATCH v2] examples/ipsec-secgw: fix pool usage for security session
>
> Currently, two separate mempools are being used for creating crypto
> sessions and its private data.
> crypto sessions are created and initialized separately, so a separate
> mempool is passed to each API, but in case of security sessions, where
> only one API create and initialize the private data as well.
> So if session mempool is passed to create a security session, the
> mempool element size is not sufficient enough to hold the private
> data as well.
> As a perfect solution, the security session create API should take 2
> mempools for header and private data and initiatlize accordingly,
> but that would mean an API breakage, which will be done in the next
> release cycle. So introducing this patch as a workaround to resolve this
> issue.
>
> Fixes: 261bbff75e34 ("examples: use separate crypto session mempools")
> Cc: roy.fan.zh...@intel.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Akhil Goyal
> ---
> v2: incorporated Konstantin's comments.
>
>
> examples/ipsec-secgw/ipsec-secgw.c | 5 +++--
> examples/ipsec-secgw/ipsec.c | 4 ++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> b/examples/ipsec-secgw/ipsec-secgw.c
> index 2e203393d..478dd80c2 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -1791,7 +1791,7 @@ cryptodevs_init(void)
> rte_eth_dev_get_sec_ctx(port_id)) {
> int socket_id = rte_eth_dev_socket_id(port_id);
>
> - if (!socket_ctx[socket_id].session_pool) {
> + if (!socket_ctx[socket_id].session_priv_pool) {
> char mp_name[RTE_MEMPOOL_NAMESIZE];
> struct rte_mempool *sess_mp;
>
> @@ -1811,7 +1811,8 @@ cryptodevs_init(void)
> else
> printf("Allocated session pool "
> "on socket %d\n", socket_id);
> - socket_ctx[socket_id].session_pool = sess_mp;
> + socket_ctx[socket_id].session_priv_pool =
> + sess_mp;
> }
> }
> }
> diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
> index 4352cb842..7b8533077 100644
> --- a/examples/ipsec-secgw/ipsec.c
> +++ b/examples/ipsec-secgw/ipsec.c
> @@ -102,7 +102,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct
> ipsec_sa *sa)
> set_ipsec_conf(sa, &(sess_conf.ipsec));
>
> sa->sec_session = rte_security_session_create(ctx,
> - &sess_conf, ipsec_ctx->session_pool);
> + &sess_conf,
> ipsec_ctx->session_priv_pool);
> if (sa->sec_session == NULL) {
> RTE_LOG(ERR, IPSEC,
> "SEC Session init failed: err: %d\n", ret);
> @@ -117,7 +117,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct
> ipsec_sa *sa)
> int ret = 0;
>
> sa->sec_session = rte_security_session_create(ctx,
> - &sess_conf, ipsec_ctx->session_pool);
> + &sess_conf,
> ipsec_ctx->session_priv_pool);
> if (sa->sec_session == NULL) {
> RTE_LOG(ERR, IPSEC,
> "SEC Session init failed: err: %d\n", ret);
Acked-by: Konstantin Ananyev
> --
> 2.17.1
Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix SPD no-match is misinterpreted
> -Original Message- > From: Zhang, Roy Fan > Sent: Friday, April 5, 2019 12:10 AM > To: Ananyev, Konstantin ; dev@dpdk.org > Cc: Akhil Goyal ; Ananyev, Konstantin > ; sta...@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix SPD no-match is > misinterpreted > > > > > -Original Message- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Konstantin > > Ananyev > > Sent: Thursday, April 4, 2019 1:13 PM > > To: dev@dpdk.org > > Cc: akhil.go...@nxp.com; Ananyev, Konstantin > > ; sta...@dpdk.org > > Subject: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix SPD no-match is > > misinterpreted > > Acked-by: Fan Zhang Acked-by: Akhil Goyal Applied to dpdk-next-crypto Thanks.
Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix pool usage for security session
> > Acked-by: Konstantin Ananyev > Applied to dpdk-next-crypto
Re: [dpdk-dev] [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped for inline crypto
Hi Akhil,
>
> > -Original Message-
> > From: Akhil Goyal
> > Sent: Thursday, April 18, 2019 7:21 PM
> > To: Bernard Iremonger ; dev@dpdk.org;
> > konstantin.anan...@intel.com
> > Cc: sta...@dpdk.org
> > Subject: RE: [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped for
> > inline crypto
> >
> > Hi Bernard,
> >
> > > - RTE_LOG_DP(DEBUG, IPSEC, "Create session for SA spi %u on
> > > cryptodev "
> > > - "%u qp %u\n", sa->spi,
> > > - ipsec_ctx->tbl[cdev_id_qp].id,
> > > - ipsec_ctx->tbl[cdev_id_qp].qp);
> > > + if ((sa == NULL) || (pool == NULL))
> > > + return -EINVAL;
> > >
> > > - if (sa->type != RTE_SECURITY_ACTION_TYPE_NONE) {
> > > - struct rte_security_session_conf sess_conf = {
> > > + struct rte_security_session_conf sess_conf = {
> > > .action_type = sa->type,
> > > .protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> > > {.ipsec = {
> > > @@ -90,247 +65,340 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct
> > > ipsec_sa *sa)
> > > } },
> > > .crypto_xform = sa->xforms,
> > > .userdata = NULL,
> > > -
> > > };
> > >
> > > - if (sa->type ==
> > RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL)
> > > {
> > > - struct rte_security_ctx *ctx = (struct
> > > rte_security_ctx *)
> > > -
> > > rte_cryptodev_get_sec_ctx(
> > > -
> > > ipsec_ctx->tbl[cdev_id_qp].id);
> > > -
> > > - /* Set IPsec parameters in conf */
> > > - set_ipsec_conf(sa, &(sess_conf.ipsec));
> > > -
> > > - sa->sec_session = rte_security_session_create(ctx,
> > > - &sess_conf,
> > > ipsec_ctx->session_pool);
> > > - if (sa->sec_session == NULL) {
> > > - RTE_LOG(ERR, IPSEC,
> > > - "SEC Session init failed: err: %d\n",
> > > ret);
> > > - return -1;
> > > - }
> > > - } else if (sa->type ==
> > > RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> > {
> > > - struct rte_flow_error err;
> > > - struct rte_security_ctx *ctx = (struct
> > > rte_security_ctx *)
> > > -
> > > rte_eth_dev_get_sec_ctx(
> > > - sa->portid);
> > > - const struct rte_security_capability *sec_cap;
> > > - int ret = 0;
> > > -
> > > - sa->sec_session = rte_security_session_create(ctx,
> > > - &sess_conf,
> > > ipsec_ctx->session_pool);
> > > - if (sa->sec_session == NULL) {
> > > - RTE_LOG(ERR, IPSEC,
> > > - "SEC Session init failed: err: %d\n",
> > > ret);
> > > - return -1;
> > > - }
> > > + if (sa->type == RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
> > > + ctx = (struct rte_security_ctx *)
> > > + rte_eth_dev_get_sec_ctx(sa->portid);
> >
> > This is breaking the lookaside mode. Ctx was retrieved using the
> > ipsec_ctx->tbl
> > struct rte_security_ctx *ctx = (struct rte_security_ctx *)
> > rte_cryptodev_get_sec_ctx(
> > ipsec_ctx->tbl[cdev_id_qp].id);
> >
> > I am looking into it, but I don't have time left to get it integrated in
> > RC2. So this
> > has to be pushed to RC3
>
> It looks like there are multiple issues in this patch wrt lookaside and none
> cases. Only the inline cases seem to be working.
>
> 1. the patch removes the cdev_mapping concept completely. Cdev_id_qp is not
> getting used.
Not exactly.
cdev_id_qp is still setup, and is still used to decide to which crypto-dev to
enqueuer the crypto-op:
ipsec_enqueue(...)
{
...
enqueue_cop(&ipsec_ctx->tbl[sa->cdev_id_qp], &priv->cop);
Same in ipsec_process().
For initialization, yes cdev_id_qp is not used anymore.
As discussed here:
https://mails.dpdk.org/archives/dev/2019-March/127725.html
I think the problem you are hitting with lookaside-proto is that for it
we use 2 different values here:
a) In create_sec_session we use portid (it also should be
rte_cryptodev_get_sec_ctx() here)
if (sa->type == RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
ctx = (struct rte_security_ctx *)
rte_eth_dev_get_sec_ctx(sa->portid);
b) in enqueue() we use cdev_id_qp
Right now these values
Re: [dpdk-dev] [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped for inline crypto
Hi Konstantin,
> Hi Akhil,
>
> >
> > > -Original Message-
> > > From: Akhil Goyal
> > > Sent: Thursday, April 18, 2019 7:21 PM
> > > To: Bernard Iremonger ; dev@dpdk.org;
> > > konstantin.anan...@intel.com
> > > Cc: sta...@dpdk.org
> > > Subject: RE: [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped
> for
> > > inline crypto
> > >
> > > Hi Bernard,
> > >
> > > > - RTE_LOG_DP(DEBUG, IPSEC, "Create session for SA spi %u on
> cryptodev "
> > > > - "%u qp %u\n", sa->spi,
> > > > - ipsec_ctx->tbl[cdev_id_qp].id,
> > > > - ipsec_ctx->tbl[cdev_id_qp].qp);
> > > > + if ((sa == NULL) || (pool == NULL))
> > > > + return -EINVAL;
> > > >
> > > > - if (sa->type != RTE_SECURITY_ACTION_TYPE_NONE) {
> > > > - struct rte_security_session_conf sess_conf = {
> > > > + struct rte_security_session_conf sess_conf = {
> > > > .action_type = sa->type,
> > > > .protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> > > > {.ipsec = {
> > > > @@ -90,247 +65,340 @@ create_session(struct ipsec_ctx *ipsec_ctx,
> struct
> > > > ipsec_sa *sa)
> > > > } },
> > > > .crypto_xform = sa->xforms,
> > > > .userdata = NULL,
> > > > -
> > > > };
> > > >
> > > > - if (sa->type ==
> > > RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL)
> > > > {
> > > > - struct rte_security_ctx *ctx = (struct
> > > > rte_security_ctx *)
> > > > -
> > > > rte_cryptodev_get_sec_ctx(
> > > > -
> > > > ipsec_ctx->tbl[cdev_id_qp].id);
> > > > -
> > > > - /* Set IPsec parameters in conf */
> > > > - set_ipsec_conf(sa, &(sess_conf.ipsec));
> > > > -
> > > > - sa->sec_session =
> > > > rte_security_session_create(ctx,
> > > > - &sess_conf,
> > > > ipsec_ctx->session_pool);
> > > > - if (sa->sec_session == NULL) {
> > > > - RTE_LOG(ERR, IPSEC,
> > > > - "SEC Session init failed: err: %d\n",
> > > > ret);
> > > > - return -1;
> > > > - }
> > > > - } else if (sa->type ==
> RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> > > {
> > > > - struct rte_flow_error err;
> > > > - struct rte_security_ctx *ctx = (struct
> > > > rte_security_ctx *)
> > > > -
> > > > rte_eth_dev_get_sec_ctx(
> > > > - sa->portid);
> > > > - const struct rte_security_capability *sec_cap;
> > > > - int ret = 0;
> > > > -
> > > > - sa->sec_session =
> > > > rte_security_session_create(ctx,
> > > > - &sess_conf,
> > > > ipsec_ctx->session_pool);
> > > > - if (sa->sec_session == NULL) {
> > > > - RTE_LOG(ERR, IPSEC,
> > > > - "SEC Session init failed: err: %d\n",
> > > > ret);
> > > > - return -1;
> > > > - }
> > > > + if (sa->type ==
> RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
> > > > + ctx = (struct rte_security_ctx *)
> > > > + rte_eth_dev_get_sec_ctx(sa->portid);
> > >
> > > This is breaking the lookaside mode. Ctx was retrieved using the
> > > ipsec_ctx-
> >tbl
> > > struct rte_security_ctx *ctx = (struct rte_security_ctx *)
> > > rte_cryptodev_get_sec_ctx(
> > > ipsec_ctx->tbl[cdev_id_qp].id);
> > >
> > > I am looking into it, but I don't have time left to get it integrated in
> > > RC2. So
> this
> > > has to be pushed to RC3
> >
> > It looks like there are multiple issues in this patch wrt lookaside and
> > none cases.
> Only the inline cases seem to be working.
> >
> > 1. the patch removes the cdev_mapping concept completely. Cdev_id_qp is
> not getting used.
>
> Not exactly.
> cdev_id_qp is still setup, and is still used to decide to which crypto-dev to
> enqueuer the crypto-op:
> ipsec_enqueue(...)
> {
>...
>enqueue_cop(&ipsec_ctx->tbl[sa->cdev_id_qp], &priv->cop);
I don't see anybody filling "sa->cdev_id_qp". Please let me know if I have
missed it somewhere.
It is memset to 0 I guess.
>
>
> Same in ipsec_process().
>
> For initialization, yes cdev_id_qp is not used anymore.
> As discussed here:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dp
> dk.org%2Farchives%2Fdev%2F2019-
> March%2F127725.ht
[dpdk-dev] [PATCH v2] app/testpmd: fix help info for interactive commands
Added some missing help info for interactive mode commands
Fixes: 6673fe0ce2 ("app/testpmd: add TM commands to mark packets")
Fixes: c73a907187 ("app/testpmd: add commands to test new offload API")
Fixes: e977e4199a ("app/testpmd: add commands to load/unload BPF filters")
Fixes: c18feafa19 ("app/testpmd: support metadata as flow rule item")
Signed-off-by: Agalya Babu RadhaKrishnan
Reviewed-by: Rami Rosen
---
v2: added TM to separate group and addressed comments.
---
app/test-pmd/cmdline.c | 262 -
1 file changed, 178 insertions(+), 84 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5a10c5f38..147a617d5 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -93,14 +93,15 @@ static void cmd_help_brief_parsed(__attribute__((unused))
void *parsed_result,
cl,
"\n"
"Help is available for the following sections:\n\n"
- "help control: Start and stop forwarding.\n"
- "help display: Displaying port, stats and config "
+ "help control: Start and stop
forwarding.\n"
+ "help display: Displaying port, stats
and config "
"information.\n"
- "help config : Configuration information.\n"
- "help ports : Configuring ports.\n"
- "help registers : Reading and setting port registers.\n"
- "help filters: Filters configuration help.\n"
- "help all: All of the above sections.\n\n"
+ "help config : Configuration
information.\n"
+ "help ports : Configuring ports.\n"
+ "help registers : Reading and setting port
registers.\n"
+ "help filters: Filters configuration
help.\n"
+ "help traffic_management : Traffic Management
commmands.\n"
+ "help all: All of the above
sections.\n\n"
);
}
@@ -209,26 +210,32 @@ static void cmd_help_long_parsed(void *parsed_result,
"show port meter stats (port_id) (meter_id) (clear)\n"
"Get meter stats on a port\n\n"
-"show port tm cap (port_id)\n"
-" Display the port TM capability.\n\n"
-
-"show port tm level cap (port_id) (level_id)\n"
-" Display the port TM hierarchical level
capability.\n\n"
-
-"show port tm node cap (port_id) (node_id)\n"
-" Display the port TM node capability.\n\n"
-
-"show port tm node type (port_id) (node_id)\n"
-" Display the port TM node type.\n\n"
-
-"show port tm node stats (port_id) (node_id) (clear)\n"
-" Display the port TM node stats.\n\n"
"show fwd stats all\n"
"Display statistics for all fwd engines.\n\n"
"clear fwd stats all\n"
"Clear statistics for all fwd engines.\n\n"
+
+ "show port (port_id) rx_offload capabilities\n"
+ "List all per queue and per port Rx offloading"
+ " capabilities of a port\n\n"
+
+ "show port (port_id) rx_offload configuration\n"
+ "List port level and all queue level"
+ " Rx offloading configuration\n\n"
+
+ "show port (port_id) tx_offload capabilities\n"
+ "List all per queue and per port"
+ " Tx offloading capabilities of a port\n\n"
+
+ "show port (port_id) tx_offload configuration\n"
+ "List port level and all queue level"
+ " Tx offloading configuration\n\n"
+
+ "show port (port_id) tx_metadata\n"
+ "Show Tx metadata value set"
+ " for a specific port\n\n"
);
}
@@ -646,11 +653,6 @@ static void cmd_help_long_parsed(void *parsed_result,
"E-tag set filter del e-tag-id (value) port (port_id)\n"
"Delete an E-tag forwarding filter on a port\n\n"
-#if defined RTE_LIBRTE_PMD_SOFTNIC && defined RTE_LIBRTE_SCHED
- "set port tm hierarchy default (port_id)\n"
- " Set default traffic Management hierarchy on a
port\n\n"
-
-#endif
"ddp add (port_id)
(profile_path[,backup_profile_path])\n"
"Load a pr
Re: [dpdk-dev] [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped for inline crypto
> >
> > >
> > > > -Original Message-
> > > > From: Akhil Goyal
> > > > Sent: Thursday, April 18, 2019 7:21 PM
> > > > To: Bernard Iremonger ; dev@dpdk.org;
> > > > konstantin.anan...@intel.com
> > > > Cc: sta...@dpdk.org
> > > > Subject: RE: [PATCH v4 1/2] examples/ipsec-secgw: fix 1st packet dropped
> > for
> > > > inline crypto
> > > >
> > > > Hi Bernard,
> > > >
> > > > > - RTE_LOG_DP(DEBUG, IPSEC, "Create session for SA spi %u on
> > cryptodev "
> > > > > - "%u qp %u\n", sa->spi,
> > > > > - ipsec_ctx->tbl[cdev_id_qp].id,
> > > > > - ipsec_ctx->tbl[cdev_id_qp].qp);
> > > > > + if ((sa == NULL) || (pool == NULL))
> > > > > + return -EINVAL;
> > > > >
> > > > > - if (sa->type != RTE_SECURITY_ACTION_TYPE_NONE) {
> > > > > - struct rte_security_session_conf sess_conf = {
> > > > > + struct rte_security_session_conf sess_conf = {
> > > > > .action_type = sa->type,
> > > > > .protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> > > > > {.ipsec = {
> > > > > @@ -90,247 +65,340 @@ create_session(struct ipsec_ctx *ipsec_ctx,
> > struct
> > > > > ipsec_sa *sa)
> > > > > } },
> > > > > .crypto_xform = sa->xforms,
> > > > > .userdata = NULL,
> > > > > -
> > > > > };
> > > > >
> > > > > - if (sa->type ==
> > > > RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL)
> > > > > {
> > > > > - struct rte_security_ctx *ctx = (struct
> > > > > rte_security_ctx *)
> > > > > -
> > > > > rte_cryptodev_get_sec_ctx(
> > > > > -
> > > > > ipsec_ctx->tbl[cdev_id_qp].id);
> > > > > -
> > > > > - /* Set IPsec parameters in conf */
> > > > > - set_ipsec_conf(sa, &(sess_conf.ipsec));
> > > > > -
> > > > > - sa->sec_session =
> > > > > rte_security_session_create(ctx,
> > > > > - &sess_conf,
> > > > > ipsec_ctx->session_pool);
> > > > > - if (sa->sec_session == NULL) {
> > > > > - RTE_LOG(ERR, IPSEC,
> > > > > - "SEC Session init failed: err: %d\n",
> > > > > ret);
> > > > > - return -1;
> > > > > - }
> > > > > - } else if (sa->type ==
> > RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> > > > {
> > > > > - struct rte_flow_error err;
> > > > > - struct rte_security_ctx *ctx = (struct
> > > > > rte_security_ctx *)
> > > > > -
> > > > > rte_eth_dev_get_sec_ctx(
> > > > > - sa->portid);
> > > > > - const struct rte_security_capability *sec_cap;
> > > > > - int ret = 0;
> > > > > -
> > > > > - sa->sec_session =
> > > > > rte_security_session_create(ctx,
> > > > > - &sess_conf,
> > > > > ipsec_ctx->session_pool);
> > > > > - if (sa->sec_session == NULL) {
> > > > > - RTE_LOG(ERR, IPSEC,
> > > > > - "SEC Session init failed: err: %d\n",
> > > > > ret);
> > > > > - return -1;
> > > > > - }
> > > > > + if (sa->type ==
> > RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
> > > > > + ctx = (struct rte_security_ctx *)
> > > > > + rte_eth_dev_get_sec_ctx(sa->portid);
> > > >
> > > > This is breaking the lookaside mode. Ctx was retrieved using the
> > > > ipsec_ctx-
> > >tbl
> > > > struct rte_security_ctx *ctx = (struct rte_security_ctx *)
> > > > rte_cryptodev_get_sec_ctx(
> > > > ipsec_ctx->tbl[cdev_id_qp].id);
> > > >
> > > > I am looking into it, but I don't have time left to get it integrated
> > > > in RC2. So
> > this
> > > > has to be pushed to RC3
> > >
> > > It looks like there are multiple issues in this patch wrt lookaside and
> > > none cases.
> > Only the inline cases seem to be working.
> > >
> > > 1. the patch removes the cdev_mapping concept completely. Cdev_id_qp is
> > not getting used.
> >
> > Not exactly.
> > cdev_id_qp is still setup, and is still used to decide to which crypto-dev
> > to
> > enqueuer the crypto-op:
> > ipsec_enqueue(...)
> > {
> >...
> >enqueue_cop(&ipsec_ctx->tbl[sa->cdev_id_qp], &priv->cop);
>
> I don't see anybody filling "sa->cdev_id_qp". Please let me know if I have
> missed it somewhere.
> It is memset to 0 I guess.
Yep, true we lost it
Re: [dpdk-dev] ABI and inline functions
On 18/04/2019 11:28, Bruce Richardson wrote: > On Thu, Apr 18, 2019 at 04:34:53AM +, Honnappa Nagarahalli wrote: >>> >>> On Wed, Apr 17, 2019 at 05:12:43AM +, Honnappa Nagarahalli wrote: Hello, There was a conversation [1] in the context of RCU library. I thought it needs participation from broader audience. Summary for the context (please look at [1] for full details) >>> >>> Thanks for kicking off this discussion >>> 1) How do we provide ABI compatibility when the code base contains >>> inline functions? Unless we freeze development in these inline functions and >>> corresponding structures, it might not be possible to claim ABI >>> compatibility. >>> Application has to be recompiled. >>> >>> I agree that in some cases the application "might" need to be recompiled, >>> but on the other hand I also think that there are many cases where ABI >>> function versioning can still be used to mitigate things. For example, if we >>> think of a couple of various scenarios: >>> >>> 1. If everything is inline and variables are allocated by app, e.g. >>> spinlock on stack, then there is no issue as everything is application >>> contained. >> If there is a bug fix which requires the structure to change, the >> application would need to recompile. I guess you are talking about a >> scenario when nothing changed in the inline function/variables. >> > > If the application wants the bugfix, then yes. However, if the app is > unaffected by the bug, then it should have the option of updating DPDK > without a recompile. I would also imagine that should be an extremely rare case, that a bugfix would require a structure change ... perhaps for an alignment issues? > >>> >>> 2. If the situation is as in #1, but the structures in question are passed >>> to >>> non-inline DPDK functions. In this case, any changes to the structures >>> require >>> those functions taking the structures to be versioned for old and new >>> structures >> I think this can get complicated on the inline function side. The >> application and the DPDK library will end up having different inline >> functions. The changed inline function needs to be aware of 2 structure >> formats or the inline function needs to be duplicated (one for each version >> of the structure). I guess these are the workarounds we have to do. >> > No, there is never any need for two versions of the inline functions, only > the newest version is needed. This is because in the case of a newly compiled > application only the newest version of the non-inline functions is ever > used. The other older versions are only used at runtime for compatilibity > with pre-compiled apps with the older inlines. > >>> >>> 3. If instead we have the case, like in rte_ring, where the data >>> structures are allocated using functions, but they are used via inlines >>> in the app. In this case the creation functions (and any other function >>> using the structures) need to be versioned so that the application gets >>> the expected structure back from the creation. >> The new structure members have to be added such that the previous layout >> is not affected. Either add at the end or use the gaps that are left >> because of cache line alignment. >> > Not necessarily. There is nothing that requires the older and newer > versions of a function to use the same structure. We can rename the > original structure when versioning the old function, and then create a new > structure with the original name for later recompiled code using the newest > ABIs. > >>> >>> It might be useful to think of what other scenarios we have and what ones >>> are likely to be problematic, especially those that can't be solved by >>> having >>> multiple function versions. >>> 2) Every function that is not in the direct datapath (fastpath?) should not be inline. Exceptions or things like rx/tx burst, ring enqueue/dequeue, and packet alloc/free - Stephen >>> >>> Agree with this. Anything not data path should not be inline. The next >> Yes, very clear on this. >> >>> question then is for data path items how to determine whether they need to >>> be inline or not. In general my rule-of-thumb: >>> * anything dealing with bursts of packets should not be inline >>> * anything what works with single packet at a time should be inline >>> >>> The one exception to this rule is cases where we have to consider "empty >>> polling" as a likely use-case. For example, rte_ring_dequeue_burst works >>> with bursts of packets, but there is a valid application use case where a >>> thread could be polling a set of rings where only a small number are >>> actually busy. Right now, polling an empty ring only costs a few cycles, >>> meaning that it's neglible to have a few polls of empty rings before you get >>> to a busy one. Having that function not-inline will cause that cost to jump >>> significantly, so could cause problems. (This leads to the interesting >>> scenario >>> where ring en
Re: [dpdk-dev] [PATCH v2] app/testpmd: fix help info for interactive commands
On 4/23/2019 2:51 PM, Agalya Babu RadhaKrishnan wrote:
> Added some missing help info for interactive mode commands
>
> Fixes: 6673fe0ce2 ("app/testpmd: add TM commands to mark packets")
> Fixes: c73a907187 ("app/testpmd: add commands to test new offload API")
> Fixes: e977e4199a ("app/testpmd: add commands to load/unload BPF filters")
> Fixes: c18feafa19 ("app/testpmd: support metadata as flow rule item")
>
> Signed-off-by: Agalya Babu RadhaKrishnan
>
> Reviewed-by: Rami Rosen
Reviewed-by: Ferruh Yigit
Applied to dpdk-next-net/master, thanks.
Re: [dpdk-dev] ABI and inline functions
On 17/04/2019 19:51, Stephen Hemminger wrote: > On Wed, 17 Apr 2019 17:46:34 + > Jerin Jacob Kollanukkaran wrote: > >>> -Original Message- >>> From: dev On Behalf Of Bruce Richardson >>> Sent: Wednesday, April 17, 2019 2:07 PM >>> To: Honnappa Nagarahalli >>> Cc: dev@dpdk.org; Stephen Hemminger ; >>> Ananyev, Konstantin ; tho...@monjalon.net; >>> Ray Kinsella ; nd >>> Subject: Re: [dpdk-dev] ABI and inline functions >>> 2) Every function that is not in the direct datapath (fastpath?) should not be inline. Exceptions or things like rx/tx burst, ring enqueue/dequeue, and packet alloc/free - Stephen >>> >>> Agree with this. Anything not data path should not be inline. The next >>> question >>> then is for data path items how to determine whether they need to be inline >>> or >>> not. In general my rule-of-thumb: >>> * anything dealing with bursts of packets should not be inline >> >> # I think, we need consider the how fat is the function too, >> If it is light weight, probably we need to make it inline Well if it light weight however it is not dealing directly with packets, probably does not matter if is not-inline? >> # Except the forward case, In real world use case, it is not trivial make >> large >> burst of packet to process it even though function prototype burst. >> We may need to consider that >> # For the given function if some argument is used with inline other function, >> probably no point in making that function alone inline as the structure >> is exposed in some other function. >> >>> * anything what works with single packet at a time should be inline >>> In this context, does it make sense to say that we will maintain API compatibility rather than saying ABI compatibility? This will also send the right message to the end users. >>> I would value ABI compatibility much higher than API compatibility. If >>> someone >>> is recompiling the application anyway, making a couple of small changes >>> (large >>> rework is obviously a different issue) to the code should not be a massive >>> issue, I >>> hope. On the other hand, ABI compatibility is needed to allow seamless >>> update >>> from one version to another, and it's that ABI compatiblity that allows >>> distro's to >>> pick up our latest and greatest versions. >> >> IMO, We have two primary use case for DPDK >> >> 1) NFV kind of use case where the application needs to run on multiple >> platform >> without recompiling it. >> 2) Fixed appliance use case where embed SoC like Intel Denverton or ARM64 >> integrated >> Controller used. For fixed appliance use case, end user care more of >> performance >> than ABI compatibility as it easy to recompile the end user application vs >> the cost of >> hitting performance impact. > > Nobody cares about compatiablity until they have to the first security update. > +1
Re: [dpdk-dev] [EXT] Re: ABI and inline functions
On 18/04/2019 06:56, Jerin Jacob Kollanukkaran wrote: > >> -Original Message- >> From: Stephen Hemminger >> Sent: Thursday, April 18, 2019 12:21 AM >> To: Jerin Jacob Kollanukkaran >> Cc: Bruce Richardson ; Honnappa Nagarahalli >> ; dev@dpdk.org; Ananyev, Konstantin >> ; tho...@monjalon.net; Ray Kinsella >> ; nd >> Subject: [EXT] Re: [dpdk-dev] ABI and inline functions >>> I would value ABI compatibility much higher than API compatibility. If someone is recompiling the application anyway, making a couple of small changes (large rework is obviously a different issue) to the code should not be a massive issue, I hope. On the other hand, ABI compatibility is needed to allow seamless update from one version to another, and it's that ABI compatiblity that allows distro's to pick up our >> latest and greatest versions. >>> >>> IMO, We have two primary use case for DPDK >>> >>> 1) NFV kind of use case where the application needs to run on multiple >> platform >>> without recompiling it. >>> 2) Fixed appliance use case where embed SoC like Intel Denverton or >>> ARM64 integrated Controller used. For fixed appliance use case, end >>> user care more of performance than ABI compatibility as it easy to >>> recompile the end user application vs the cost of hitting performance >>> impact. >> >> Nobody cares about compatiablity until they have to the first security >> update. > > For fixed appliance case, The update(FW update) will be a single blob which > Include all the components. So they can back port the security fix and > recompile > the sw as needed. > > The very similar category is DPDK running in smart NICs(Runs as FW in PCIe > EP device). So is there a real versus a perceived compromise happen here - that we are compromising optimal performance in order to make API stability happen? Do we have specific an examples that this is actually the case? Thanks, Ray K
Re: [dpdk-dev] [PATCH 2/2] crypto/aesni_mb: cleanup of version check code
> -Original Message- > From: Richardson, Bruce > Sent: Friday, April 19, 2019 11:01 AM > To: dev@dpdk.org > Cc: De Lara Guarch, Pablo ; Doherty, > Declan ; Richardson, Bruce > > Subject: [PATCH 2/2] crypto/aesni_mb: cleanup of version check code > > The version check for the IPSec_MB library present in the aesni_gcm library's > meson.build file is a little cleaner than that given here, so update this one > so > that both work identically. > > While one could use the checks done in the other right now, potentially in > future they may have different version dependencies, or may be compiled in > different orders, so keep the code duplicated for safety, since it's only a > few > lines. > > Signed-off-by: Bruce Richardson Acked-by: Pablo de Lara
Re: [dpdk-dev] [PATCH 1/2] crypto/aesni_gcm: add dependency version check
> -Original Message- > From: Richardson, Bruce > Sent: Friday, April 19, 2019 11:01 AM > To: dev@dpdk.org > Cc: De Lara Guarch, Pablo ; Doherty, > Declan ; Richardson, Bruce > > Subject: [PATCH 1/2] crypto/aesni_gcm: add dependency version check > > The aesni_mb driver and the aesni_gcm driver both require the same version > of the IPSec_MB library, but only the former has a check of the library found > to see if it's the correct version. Add a similar check to the aesni_gcm > library's meson.build file, so that the auto-detection of dependencies works > correctly. > > Signed-off-by: Bruce Richardson > --- > drivers/crypto/aesni_gcm/meson.build | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/crypto/aesni_gcm/meson.build > b/drivers/crypto/aesni_gcm/meson.build > index 70f57ad73..7183cfcba 100644 > --- a/drivers/crypto/aesni_gcm/meson.build > +++ b/drivers/crypto/aesni_gcm/meson.build The patch looks good, but we should broaden its scope and also modify the Makefile, to check for the library version, like in the aesni_mb PMD. Could you add that check too? Thanks! Pablo
Re: [dpdk-dev] [PATCH 1/2] crypto/aesni_gcm: add dependency version check
On Tue, Apr 23, 2019 at 03:38:58PM +0100, De Lara Guarch, Pablo wrote: > > > > -Original Message- > > From: Richardson, Bruce > > Sent: Friday, April 19, 2019 11:01 AM > > To: dev@dpdk.org > > Cc: De Lara Guarch, Pablo ; Doherty, > > Declan ; Richardson, Bruce > > > > Subject: [PATCH 1/2] crypto/aesni_gcm: add dependency version check > > > > The aesni_mb driver and the aesni_gcm driver both require the same version > > of the IPSec_MB library, but only the former has a check of the library > > found > > to see if it's the correct version. Add a similar check to the aesni_gcm > > library's meson.build file, so that the auto-detection of dependencies works > > correctly. > > > > Signed-off-by: Bruce Richardson > > --- > > drivers/crypto/aesni_gcm/meson.build | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/crypto/aesni_gcm/meson.build > > b/drivers/crypto/aesni_gcm/meson.build > > index 70f57ad73..7183cfcba 100644 > > --- a/drivers/crypto/aesni_gcm/meson.build > > +++ b/drivers/crypto/aesni_gcm/meson.build > > The patch looks good, but we should broaden its scope and also modify the > Makefile, > to check for the library version, like in the aesni_mb PMD. > > Could you add that check too? > I thought about doing so, but decided not to do so because the driver isn't enabled by default. Therefore the default out-of-the-box build is not broken when using an old version, as it is with meson. That being said, I'll look to see if the aesni_mb checks can be easiest ported over for a V2. Regards, /Bruce
Re: [dpdk-dev] [PATCH] app/testpmd: set fixed flag when exact link speed is chosen
On 4/12/2019 2:12 PM, Andrew Rybchenko wrote:
> Setting exact link speed makes sense if auto-negotiation is
> disabled. Fixed flag is required to disable auto-negotiation.
Hi Andrew,
Fixed link speed is not supported by all PMDs and this change is breaking them
to set the speed in testpmd.
As long as device speed set correct, I believe the command doesn't really care
about if link mode is auto-negotiation or not.
I am for reverting this commit, is there any objection to revert it?
>
> Fixes: 88fbedcd5e5a ("app/testpmd: move speed and duplex parsing in a
> function")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Andrew Rybchenko
> ---
> app/test-pmd/cmdline.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 2ab03c111..691d818a6 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1440,17 +1440,17 @@ parse_and_check_speed_duplex(char *speedstr, char
> *duplexstr, uint32_t *speed)
> return -1;
> }
> if (!strcmp(speedstr, "1000")) {
> - *speed = ETH_LINK_SPEED_1G;
> + *speed = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_FIXED;
> } else if (!strcmp(speedstr, "1")) {
> - *speed = ETH_LINK_SPEED_10G;
> + *speed = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_FIXED;
> } else if (!strcmp(speedstr, "25000")) {
> - *speed = ETH_LINK_SPEED_25G;
> + *speed = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_FIXED;
> } else if (!strcmp(speedstr, "4")) {
> - *speed = ETH_LINK_SPEED_40G;
> + *speed = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_FIXED;
> } else if (!strcmp(speedstr, "5")) {
> - *speed = ETH_LINK_SPEED_50G;
> + *speed = ETH_LINK_SPEED_50G | ETH_LINK_SPEED_FIXED;
> } else if (!strcmp(speedstr, "10")) {
> - *speed = ETH_LINK_SPEED_100G;
> + *speed = ETH_LINK_SPEED_100G | ETH_LINK_SPEED_FIXED;
> } else if (!strcmp(speedstr, "auto")) {
> *speed = ETH_LINK_SPEED_AUTONEG;
> } else {
>
Re: [dpdk-dev] [PATCH] app/testpmd: set fixed flag when exact link speed is chosen
On 4/23/2019 4:02 PM, Ferruh Yigit wrote:
> On 4/12/2019 2:12 PM, Andrew Rybchenko wrote:
>> Setting exact link speed makes sense if auto-negotiation is
>> disabled. Fixed flag is required to disable auto-negotiation.
>
> Hi Andrew,
>
> Fixed link speed is not supported by all PMDs and this change is breaking them
> to set the speed in testpmd.
>
> As long as device speed set correct, I believe the command doesn't really care
> about if link mode is auto-negotiation or not.
>
> I am for reverting this commit, is there any objection to revert it?
cc'ing Wenjie who reported the issue.
>
>>
>> Fixes: 88fbedcd5e5a ("app/testpmd: move speed and duplex parsing in a
>> function")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Andrew Rybchenko
>> ---
>> app/test-pmd/cmdline.c | 12 ++--
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 2ab03c111..691d818a6 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -1440,17 +1440,17 @@ parse_and_check_speed_duplex(char *speedstr, char
>> *duplexstr, uint32_t *speed)
>> return -1;
>> }
>> if (!strcmp(speedstr, "1000")) {
>> -*speed = ETH_LINK_SPEED_1G;
>> +*speed = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_FIXED;
>> } else if (!strcmp(speedstr, "1")) {
>> -*speed = ETH_LINK_SPEED_10G;
>> +*speed = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_FIXED;
>> } else if (!strcmp(speedstr, "25000")) {
>> -*speed = ETH_LINK_SPEED_25G;
>> +*speed = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_FIXED;
>> } else if (!strcmp(speedstr, "4")) {
>> -*speed = ETH_LINK_SPEED_40G;
>> +*speed = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_FIXED;
>> } else if (!strcmp(speedstr, "5")) {
>> -*speed = ETH_LINK_SPEED_50G;
>> +*speed = ETH_LINK_SPEED_50G | ETH_LINK_SPEED_FIXED;
>> } else if (!strcmp(speedstr, "10")) {
>> -*speed = ETH_LINK_SPEED_100G;
>> +*speed = ETH_LINK_SPEED_100G | ETH_LINK_SPEED_FIXED;
>> } else if (!strcmp(speedstr, "auto")) {
>> *speed = ETH_LINK_SPEED_AUTONEG;
>> } else {
>>
>
[dpdk-dev] Hugepages not being deleted
Hello, I am currently working on a patch to fix the eal_flags_autotest test as it currently fails on many platforms. I have made some progress, however I stumbled upon a possible issue with EAL and hugepages. Looking at the code and some documentation it appears to me that hupepages are supposed to be automatically deleted on dynamic memory mode as the dpdk process exits. The test however reports that this is not happening. This can be shown by: bash# export DPDK_TEST=eal_flags_autotest bash# ./build/app/test/dpdk-test ... Error - hugepage files for memtest1 were not deleted! Error in test_file_prefix() Test Failed bash# ls /dev/hugepages/ #hugetlbfs is mounted on /dev/hugepages memtest1map_0 memtest1map_1 memtest1map_2 memtest1map_3 memtest1map_4 memtest1map_5 memtest1map_6 memtest1map_7 memtest1map_8 rtemap_0 To me it appears that the hugepages are in fact not being deleted correctly. Is this an anomaly or is anyone else seeing this issue as well? Michael Santana Note, if you are running on a system with less than 8 cores please see patch https://github.com/Maickii/dpdk-2/commit/7cfad856611e3ded4050f670ec11d1b2e17851d8.patch
[dpdk-dev] [PATCH] net/ring: fix return value check
'rte_eth_dev_get_port_by_name()' return value is not checked caught by
coverity, adding return value check.
Coverity issue: 305853
Fixes: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API")
Cc: sta...@dpdk.org
Signed-off-by: Ferruh Yigit
---
drivers/net/ring/rte_eth_ring.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 115a882b5..5ec646594 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -385,7 +385,11 @@ rte_eth_from_rings(const char *name, struct rte_ring
*const rx_queues[],
return -1;
}
- rte_eth_dev_get_port_by_name(ring_name, &port_id);
+ ret = rte_eth_dev_get_port_by_name(ring_name, &port_id);
+ if (ret) {
+ rte_errno = ENODEV;
+ return -1;
+ }
return port_id;
}
--
2.20.1
Re: [dpdk-dev] [RFC v2 1/2] eal: replace libc-based random number generation with LFSR
On Mon, 22 Apr 2019 19:44:39 +0200 Mattias Rönnblom wrote: > On 2019-04-22 17:52, Mattias Rönnblom wrote: > > On 2019-04-22 13:34, Neil Horman wrote: > > > >>> +uint64_t __rte_experimental > >>> +rte_rand(void) > >> Do you really want to mark this as experimental? I know it will > >> trigger the > >> symbol checker with a warning if you don't, but this function already > >> existed > >> previously and was accepted as part of the ABI. Given that the > >> prototype hasn't > >> changed, I think you just need to accept it as a non-experimental > >> function > >> > > > > I'll remove the experimental tag and move it into the 19_05 section > > (without suggesting it should go into 19.05). That maneuver seems not to > > trigger any build warnings/errors. > > > > OK, so that wasn't true. It does trigger a build error, courtesy of > buildtools/check-experimental-syms.sh. > > I can't see any obvious way around it. Ideas, anyone? Ignore the error, the build tool is not smart enough for this kind of change.
[dpdk-dev] [PATCH v2] lib/crypto: include dependency in asym header
include rte_crypto_sym.h in asym header file. Changes in v2: - Change commit summary from “fix alphabetical ordering of headers” to better reflect intent of change. Previous patch set version http://mails.dpdk.org/archives/dev/2019-April/130010.html . - revert change in rte_crypto.h Ayuj Verma (1): lib/crypto: include dependency in asym header lib/librte_cryptodev/rte_crypto_asym.h | 2 ++ 1 file changed, 2 insertions(+) -- 1.8.3.1
[dpdk-dev] [PATCH v2] lib/crypto: include dependency in asym header
include rte_crypto_sym.h in asym header file.
Signed-off-by: Ayuj Verma
Signed-off-by: Shally Verma
---
lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
b/lib/librte_cryptodev/rte_crypto_asym.h
index 5e43620..a55923a 100644
--- a/lib/librte_cryptodev/rte_crypto_asym.h
+++ b/lib/librte_cryptodev/rte_crypto_asym.h
@@ -25,6 +25,8 @@
#include
#include
+#include "rte_crypto_sym.h"
+
typedef struct rte_crypto_param_t {
uint8_t *data;
/**< pointer to buffer holding data */
--
1.8.3.1
[dpdk-dev] [PATCH] net/kni: fix return value check
'rte_kni_release()' return value is not checked, adding it.
Coverity issue: 336837
Fixes: 75e2bc54c018 ("net/kni: add KNI PMD")
Cc: sta...@dpdk.org
Signed-off-by: Ferruh Yigit
---
drivers/net/kni/rte_eth_kni.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 89f44737c..1f232e4da 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -459,6 +459,7 @@ eth_kni_remove(struct rte_vdev_device *vdev)
struct rte_eth_dev *eth_dev;
struct pmd_internals *internals;
const char *name;
+ int ret;
name = rte_vdev_device_name(vdev);
PMD_LOG(INFO, "Un-Initializing eth_kni for %s", name);
@@ -477,7 +478,9 @@ eth_kni_remove(struct rte_vdev_device *vdev)
eth_kni_dev_stop(eth_dev);
internals = eth_dev->data->dev_private;
- rte_kni_release(internals->kni);
+ ret = rte_kni_release(internals->kni);
+ if (ret)
+ PMD_LOG(WARNING, "Not able to release kni for %s", name);
rte_eth_dev_release_port(eth_dev);
--
2.20.1
[dpdk-dev] [PATCH v2 0/3] improve IPsec_MB dependency checks
For both meson and make builds, the version checks for IPsec_MB library were incomplete. This fixes both and makes the checks consistent. Bruce Richardson (3): crypto/aesni_gcm: add dependency version check crypto/aesni_mb: cleanup of version check code crypto/aesni_gcm: add check for build dependency drivers/crypto/aesni_gcm/Makefile| 19 --- drivers/crypto/aesni_gcm/meson.build | 11 +++ drivers/crypto/aesni_mb/meson.build | 18 +- 3 files changed, 36 insertions(+), 12 deletions(-) -- 2.20.1
[dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: add dependency version check
The aesni_mb driver and the aesni_gcm driver both require the same version
of the IPSec_MB library, but only the former has a check of the library
found by meson to see if it's the correct version. Add a similar check to
the aesni_gcm library's meson.build file, so that the auto-detection of
dependencies works correctly.
Signed-off-by: Bruce Richardson
---
drivers/crypto/aesni_gcm/meson.build | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/crypto/aesni_gcm/meson.build
b/drivers/crypto/aesni_gcm/meson.build
index 70f57ad73..7183cfcba 100644
--- a/drivers/crypto/aesni_gcm/meson.build
+++ b/drivers/crypto/aesni_gcm/meson.build
@@ -1,11 +1,22 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2018 Intel Corporation
+IMB_required_ver = '0.52.0'
lib = cc.find_library('IPSec_MB', required: false)
if not lib.found()
build = false
else
ext_deps += lib
+
+ # version comes with quotes, so we split based on " and take the middle
+ imb_ver = cc.get_define('IMB_VERSION_STR',
+ prefix : '#include').split('"')[1]
+
+ if (imb_ver == '') or (imb_ver.version_compare('<' + IMB_required_ver))
+ message('IPSec_MB version >= @0@ is required, found version
@1@'.format(
+ IMB_required_ver, imb_ver))
+ build = false
+ endif
endif
allow_experimental_apis = true
--
2.20.1
[dpdk-dev] [PATCH v2 2/3] crypto/aesni_mb: cleanup of version check code
The version check for the IPSec_MB library present in the aesni_gcm
library's meson.build file is a little cleaner than that given here,
so update this one so that both work identically.
While one could use the checks done in the other right now, potentially in
future they may have different version dependencies, or may be compiled in
different orders, so keep the code duplicated for safety, since it's only a
few lines.
Signed-off-by: Bruce Richardson
Acked-by: Pablo de Lara
---
drivers/crypto/aesni_mb/meson.build | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/crypto/aesni_mb/meson.build
b/drivers/crypto/aesni_mb/meson.build
index fbc4878af..7c1eb3f86 100644
--- a/drivers/crypto/aesni_mb/meson.build
+++ b/drivers/crypto/aesni_mb/meson.build
@@ -1,25 +1,25 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2018 Intel Corporation
-IPSec_MB_ver_0_52 = '0.52.0'
+
+IMB_required_ver = '0.52.0'
lib = cc.find_library('IPSec_MB', required: false)
if not lib.found()
build = false
else
ext_deps += lib
- imb_arr = cc.get_define('IMB_VERSION_STR',
- prefix : '#include').split('"')
-
- imb_ver = ''.join(imb_arr)
+ # version comes with quotes, so we split based on " and take the middle
+ imb_ver = cc.get_define('IMB_VERSION_STR',
+ prefix : '#include').split('"')[1]
- if (imb_ver == '') or (imb_ver.version_compare('<' + IPSec_MB_ver_0_52))
- message('IPSec_MB version >= 0.52 is required')
+ if (imb_ver == '') or (imb_ver.version_compare('<' + IMB_required_ver))
+ message('IPSec_MB version >= @0@ is required, found version
@1@'.format(
+ IMB_required_ver, imb_ver))
build = false
- else
- sources = files('rte_aesni_mb_pmd.c', 'rte_aesni_mb_pmd_ops.c')
endif
endif
+sources = files('rte_aesni_mb_pmd.c', 'rte_aesni_mb_pmd_ops.c')
allow_experimental_apis = true
deps += ['bus_vdev']
--
2.20.1
[dpdk-dev] [PATCH v2 3/3] crypto/aesni_gcm: add check for build dependency
The aesni_mb driver has a check in its Makefile for the correct version of the IPsec_MB library, but this check was missed for the aesni_gcm driver. Add this check to the makefile, removing an unnecessary assignment in the process. Suggested-by: Pablo de Lara Signed-off-by: Bruce Richardson --- drivers/crypto/aesni_gcm/Makefile | 19 --- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/crypto/aesni_gcm/Makefile b/drivers/crypto/aesni_gcm/Makefile index 9241ad762..39eff3db0 100644 --- a/drivers/crypto/aesni_gcm/Makefile +++ b/drivers/crypto/aesni_gcm/Makefile @@ -23,11 +23,24 @@ LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring LDLIBS += -lrte_cryptodev LDLIBS += -lrte_bus_vdev +IMB_HDR = $(shell echo '\#include ' | \ + $(CC) -E $(EXTRA_CFLAGS) - | grep 'intel-ipsec-mb.h' | \ + head -n1 | cut -d'"' -f2) + +# Detect library version +IMB_VERSION = $(shell grep -e "IMB_VERSION_STR" $(IMB_HDR) | cut -d'"' -f2) +IMB_VERSION_NUM = $(shell grep -e "IMB_VERSION_NUM" $(IMB_HDR) | cut -d' ' -f3) + +ifeq ($(IMB_VERSION),) +$(error "IPSec_MB version >= 0.52 is required") +endif + +ifeq ($(shell expr $(IMB_VERSION_NUM) \< 0x3400), 1) +$(error "IPSec_MB version >= 0.52 is required") +endif + # library source files SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += aesni_gcm_pmd.c SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += aesni_gcm_pmd_ops.c -# export include files -SYMLINK-y-include += - include $(RTE_SDK)/mk/rte.lib.mk -- 2.20.1
Re: [dpdk-dev] [PATCH] app/testpmd: set fixed flag when exact link speed is chosen
On 4/23/19 6:04 PM, Ferruh Yigit wrote:
On 4/23/2019 4:02 PM, Ferruh Yigit wrote:
On 4/12/2019 2:12 PM, Andrew Rybchenko wrote:
Setting exact link speed makes sense if auto-negotiation is
disabled. Fixed flag is required to disable auto-negotiation.
Hi Andrew,
Fixed link speed is not supported by all PMDs and this change is breaking them
to set the speed in testpmd.
Not all. sfc definitely supports it.
It looks like bnxt, e1000, igb support it as well.
As long as device speed set correct, I believe the command doesn't really care
about if link mode is auto-negotiation or not.
Sometimes it is really important to disable auto-negotiation.
I am for reverting this commit, is there any objection to revert it?
cc'ing Wenjie who reported the issue.
May be I misunderstood the purpose of the command.
Typically if someone wants to set link speed, it is assumed that
auto-negotiation should be disabled since user specifies what he really
wants.
Of course, it is still valid request to keep auto-negotiation enabled, but
limit set of advertised speeds (may be keep only one).
So, I'm not happy to revert it completely. There is an option to revert
to old
behaviour and add optional argument to disable auto-negotiation, but I
can't say that I like it, since it would make sense if the command
allows more
than one speed to be advertised (to be auto-negotiated). If it is only
one speed,
the default should be autoneg disabled.
Anyway documentation of the command should be improved.
Andrew.
Fixes: 88fbedcd5e5a ("app/testpmd: move speed and duplex parsing in a function")
Cc: sta...@dpdk.org
Signed-off-by: Andrew Rybchenko
---
app/test-pmd/cmdline.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2ab03c111..691d818a6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1440,17 +1440,17 @@ parse_and_check_speed_duplex(char *speedstr, char
*duplexstr, uint32_t *speed)
return -1;
}
if (!strcmp(speedstr, "1000")) {
- *speed = ETH_LINK_SPEED_1G;
+ *speed = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "1")) {
- *speed = ETH_LINK_SPEED_10G;
+ *speed = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "25000")) {
- *speed = ETH_LINK_SPEED_25G;
+ *speed = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "4")) {
- *speed = ETH_LINK_SPEED_40G;
+ *speed = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "5")) {
- *speed = ETH_LINK_SPEED_50G;
+ *speed = ETH_LINK_SPEED_50G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "10")) {
- *speed = ETH_LINK_SPEED_100G;
+ *speed = ETH_LINK_SPEED_100G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "auto")) {
*speed = ETH_LINK_SPEED_AUTONEG;
} else {
Re: [dpdk-dev] [PATCH] net/ring: fix return value check
On Tue, Apr 23, 2019 at 04:31:00PM +0100, Ferruh Yigit wrote:
> 'rte_eth_dev_get_port_by_name()' return value is not checked caught by
> coverity, adding return value check.
>
> Coverity issue: 305853
> Fixes: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ferruh Yigit
> ---
> drivers/net/ring/rte_eth_ring.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
> index 115a882b5..5ec646594 100644
> --- a/drivers/net/ring/rte_eth_ring.c
> +++ b/drivers/net/ring/rte_eth_ring.c
> @@ -385,7 +385,11 @@ rte_eth_from_rings(const char *name, struct rte_ring
> *const rx_queues[],
> return -1;
> }
>
> - rte_eth_dev_get_port_by_name(ring_name, &port_id);
> + ret = rte_eth_dev_get_port_by_name(ring_name, &port_id);
> + if (ret) {
> + rte_errno = ENODEV;
> + return -1;
> + }
>
> return port_id;
> }
Looks reasonable to me.
Acked-by: Bruce Richardson
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: set fixed flag when exact link speed is chosen
On 4/23/2019 4:44 PM, Andrew Rybchenko wrote:
> On 4/23/19 6:04 PM, Ferruh Yigit wrote:
>> On 4/23/2019 4:02 PM, Ferruh Yigit wrote:
>>> On 4/12/2019 2:12 PM, Andrew Rybchenko wrote:
Setting exact link speed makes sense if auto-negotiation is
disabled. Fixed flag is required to disable auto-negotiation.
>>> Hi Andrew,
>>>
>>> Fixed link speed is not supported by all PMDs and this change is breaking
>>> them
>>> to set the speed in testpmd.
>
> Not all. sfc definitely supports it.
> It looks like bnxt, e1000, igb support it as well.
The ones we know for now as not supported are ixgbe and i40e.
>
>>> As long as device speed set correct, I believe the command doesn't really
>>> care
>>> about if link mode is auto-negotiation or not.
>
> Sometimes it is really important to disable auto-negotiation.
If this is the case, we should cover this option in testpmd, but perhaps as an
extension to current command, like new parameter?
>
>>> I am for reverting this commit, is there any objection to revert it?
>> cc'ing Wenjie who reported the issue.
>
> May be I misunderstood the purpose of the command.
> Typically if someone wants to set link speed, it is assumed that
> auto-negotiation should be disabled since user specifies what he really
> wants.
> Of course, it is still valid request to keep auto-negotiation enabled, but
> limit set of advertised speeds (may be keep only one).
I think the purpose of the command is not clear, and what you said makes sense,
only it is breaking the some PMDs is problem I think.
>
> So, I'm not happy to revert it completely. There is an option to revert
> to old
> behaviour and add optional argument to disable auto-negotiation, but I
> can't say that I like it, since it would make sense if the command
> allows more
> than one speed to be advertised (to be auto-negotiated). If it is only
> one speed,
> the default should be autoneg disabled.
Indeed I was suggesting same thing above, I didn't get the problem with this
approach, if the 'fixed' arg added it will work as you suggested, single fixed
speed value.
>
> Anyway documentation of the command should be improved.
>
> Andrew.
>
Fixes: 88fbedcd5e5a ("app/testpmd: move speed and duplex parsing in a
function")
Cc: sta...@dpdk.org
Signed-off-by: Andrew Rybchenko
---
app/test-pmd/cmdline.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2ab03c111..691d818a6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1440,17 +1440,17 @@ parse_and_check_speed_duplex(char *speedstr, char
*duplexstr, uint32_t *speed)
return -1;
}
if (!strcmp(speedstr, "1000")) {
- *speed = ETH_LINK_SPEED_1G;
+ *speed = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "1")) {
- *speed = ETH_LINK_SPEED_10G;
+ *speed = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "25000")) {
- *speed = ETH_LINK_SPEED_25G;
+ *speed = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "4")) {
- *speed = ETH_LINK_SPEED_40G;
+ *speed = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "5")) {
- *speed = ETH_LINK_SPEED_50G;
+ *speed = ETH_LINK_SPEED_50G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "10")) {
- *speed = ETH_LINK_SPEED_100G;
+ *speed = ETH_LINK_SPEED_100G | ETH_LINK_SPEED_FIXED;
} else if (!strcmp(speedstr, "auto")) {
*speed = ETH_LINK_SPEED_AUTONEG;
} else {
>
Re: [dpdk-dev] [BUG] net/af_xdp: Current code can only create one af_xdp device
Hi Xiaolong,
I tested your commit "net/af_xdp: fix creating multiple instance" on the
current master branch. It does not work for me in the following minimal
test setting:
1) allocate 2x 1GB huge pages for DPDK
2) ip link add p1 type veth peer name p2
3) ./dpdk-testpmd --vdev=net_af_xdp0,iface=p1
--vdev=net_af_xdp1,iface=p2 (I also tested this with two igb devices,
with the same errors)
I'm using Linux 5.1-rc6 and an up to date libbpf. The setup works for
the first device and fails for the second device when creating bpf maps
in libbpf ("qidconf_map" or "xsks_map"). It seems, that these maps also
need unique names and cannot exist twice under the same name.
Furthermore if running step 3 again after it failed for the first time,
xdp vdev allocation already fails for the first xdp vdev and does not
reach the second one. Please let me know if you need some program output
or more information from me.
Best regards,
Markus
On 4/18/19 3:05 AM, Ye Xiaolong wrote:
> Hi, Markus
>
> On 04/17, Markus Theil wrote:
>> I tested the new af_xdp based device on the current master branch and
>> noticed, that the usage of static mempool names allows only for the
>> creation of a single af_xdp vdev. If a second vdev of the same type gets
>> created, the mempool allocation fails.
> Thanks for reporting, could you paste the cmdline you used and the error log?
> Are you referring to ring creation or mempool creation?
>
>
> Thanks,
> Xiaolong
>> Best regards,
>> Markus Theil
Re: [dpdk-dev] [PATCH] doc: update release notes for windows support
On 4/22/2019 3:42 PM, Thomas Monjalon wrote: 10/04/2019 23:00, Pallavi Kadam: Added documentation for Windows support on 19.05 release. Signed-off-by: Pallavi Kadam Reviewed-by: Anand Rawat --- +* **Added Windows Support.** + + * Added Windows support to compile and build Hello World Sample Application. + * Added Windows-specific EAL changes and meson changes. + * Added windows support for kvargs. You forgot the uppercase for Windows. + * Updated meson for building DLL on windows using DEF files. + * Added rte_os.h for essential os specific macros and typedefs + * Updated make build system to include path for rte_os.h. I think this is too much details for a release notes. The useful information here is the first item explaining the support level is for compiling helloworld. Ok, will reduce it in v2.
Re: [dpdk-dev] [dpdk-announce] release candidate 19.05-rc2
Tested by Ju-Hyoung Lee. All Rx packets are missing. [LISAv2 Test Results Summary] Test Run On : 04/23/2019 10:56:54 ARM Image Under Test : Canonical : UbuntuServer : 18.04-LTS : latest Total Test Cases : 8 (0 Passed, 6 Failed, 2 Aborted, 0 Skipped) Total Time (dd:hh:mm) : 0:4:57 ID TestArea TestCaseName TestResult TestDuration(in minutes) --- 1 DPDK VERIFY-SRIOV-FAILSAFE-FOR-DPDK FAIL65.42 DPDK-TESTPMD : Initial SRIOV : DPDK 19.05.0-rc2 : TxPPS : 10493699 : RxPPS : 0 DPDK-TESTPMD : Synthetic : DPDK 19.05.0-rc2 : TxPPS : 528935 : RxPPS : 0 DPDK-TESTPMD : Re-Enable SRIOV : DPDK 19.05.0-rc2 : TxPps : 10720592 : RxPps : 0 2 DPDK PERF-DPDK-SINGLE-CORE-PPS-DS4 FAIL15.89 Performance report : dpdk_version test_mode core max_rx_pps tx_pps_avg rx_pps_avg fwdtx_pps_avg tx_b ytes - -- -- -- - 19.05.0-rc2 rxonly11 11439777 0 0 4... 19.05.0-rc2 io12 11435071 0 0 4... 3 DPDK VERIFY-DPDK-OVS ABORTED17.68 4 DPDK PERF-DPDK-SINGLE-CORE-PPS-DS15 FAIL15.76 Performance report : dpdk_version test_mode core max_rx_pps tx_pps_avg rx_pps_avg fwdtx_pps_avg tx_b ytes - -- -- -- - 19.05.0-rc2 rxonly11 12431395 0 0 4... 19.05.0-rc2 io11 12538518 0 0 4... 5 DPDK PERF-DPDK-MULTICORE-PPS-DS15 FAIL24.51 Performance report : dpdk_version test_mode core max_rx_pps tx_pps_avg rx_pps_avg fwdtx_pps_avg tx_b ytes - -- -- -- - 19.05.0-rc2 rxonly21 22945732 0 0 9... 19.05.0-rc2 io21 23589806 0 0 9... 19.05.0-rc2 rxonly42 21959870 0 0 8... 19.05.0-rc2 io43 23135657 0 0 9... 19.05.0-rc2 rxonly82 22621430 0 0 8... 19.05.0-rc2 io82 22779723 0 0 8... 6 DPDK VERIFY-DPDK-FAILSAFE-DURING-TRAFFIC FAIL74.89 7 DPDK PERF-DPDK-FWD-PPS-DS15 FAIL21.39 Performance report : dpdk_version core tx_pps_avg fwdrx_pps_avg fwdtx_pps_avg rx_pps_avg -- - - -- 19.05.0-rc2 222430482 0 0 0 19.05.0-rc2 421855880 0 0 0 19.05.0-rc2 821746093 0 0 0 8 DPDK PERF-DPDK-MULTICORE-PPS-F32 Aborted0 Logs can be found at C:\LISAv2\YJ34\TestResults\2019-23-04-10-56-48-8929 04/23/2019 15:54:14 : [INFO ] Creating 'C:\LISAv2\YJ34\Azure-YJ34-TestLogs.zip' from 'C:\LISAv2\YJ34\TestResults\2019-23-04-10-56-48-8929' 04/23/2019 15:54:14 : [INFO ] C:\LISAv2\YJ34\Azure-YJ34-TestLogs.zip created successfully. 04/23/2019 15:54:14 : [INFO ] Analyzing results.. 04/23/2019 15:54:14 : [INFO ] Copying all files back to original working directory: C:\Jenkins-Slaves\azure-slave-2-executor-8\workspace\job-azuretest\pipeline-dpdk. 04/23/2019 15:54:23 : [INFO ] Setting workspace back to original location: C:\Jenkins-Slaves\azure-slave-2-executor-8\workspace\job-azuretest\pipeline-dpdk 04/23/2019 15:54:23 : [INFO ] Forcefully exiting with exit code 0 as ExitWithZero flag was set to True 04/23/2019 15:54:23 : [INFO ] LISAv2 exit code: 0 -Original Message- From: announce On Behalf Of Thomas Monjalon Sent: Monday, April 22, 2019 4:00 PM To: annou...@dpdk.org Subject: [dpdk-announce] release candidate 19.05-rc2 A new DPDK release candidate is ready
[dpdk-dev] [PATCH 0/4] net/ring: driver fixes
Minor fixes for the net/ring driver. Stephen Hemminger (4): net/ring: fix coding style net/ring: use sizeof() with snprintf net/ring: use rte_calloc_socket net/ring: check length of ring name drivers/net/ring/rte_eth_ring.c | 90 - 1 file changed, 55 insertions(+), 35 deletions(-) -- 2.20.1
[dpdk-dev] [PATCH 1/4] net/ring: fix coding style
Whitespace fixes to bring inline with current DPDK style.
Signed-off-by: Stephen Hemminger
---
drivers/net/ring/rte_eth_ring.c | 58 ++---
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 115a882b507c..91e5f5f8f262 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -45,8 +45,8 @@ struct ring_queue {
};
struct pmd_internals {
- unsigned max_rx_queues;
- unsigned max_tx_queues;
+ unsigned int max_rx_queues;
+ unsigned int max_tx_queues;
struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
@@ -55,12 +55,11 @@ struct pmd_internals {
enum dev_action action;
};
-
static struct rte_eth_link pmd_link = {
- .link_speed = ETH_SPEED_NUM_10G,
- .link_duplex = ETH_LINK_FULL_DUPLEX,
- .link_status = ETH_LINK_DOWN,
- .link_autoneg = ETH_LINK_FIXED,
+ .link_speed = ETH_SPEED_NUM_10G,
+ .link_duplex = ETH_LINK_FULL_DUPLEX,
+ .link_status = ETH_LINK_DOWN,
+ .link_autoneg = ETH_LINK_FIXED,
};
static int eth_ring_logtype;
@@ -138,6 +137,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t
rx_queue_id,
struct rte_mempool *mb_pool __rte_unused)
{
struct pmd_internals *internals = dev->data->dev_private;
+
dev->data->rx_queues[rx_queue_id] =
&internals->rx_ring_queues[rx_queue_id];
return 0;
}
@@ -149,6 +149,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
tx_queue_id,
const struct rte_eth_txconf *tx_conf
__rte_unused)
{
struct pmd_internals *internals = dev->data->dev_private;
+
dev->data->tx_queues[tx_queue_id] =
&internals->tx_ring_queues[tx_queue_id];
return 0;
}
@@ -156,9 +157,10 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
tx_queue_id,
static void
eth_dev_info(struct rte_eth_dev *dev,
- struct rte_eth_dev_info *dev_info)
+struct rte_eth_dev_info *dev_info)
{
struct pmd_internals *internals = dev->data->dev_private;
+
dev_info->max_mac_addrs = 1;
dev_info->max_rx_pktlen = (uint32_t)-1;
dev_info->max_rx_queues = (uint16_t)internals->max_rx_queues;
@@ -169,7 +171,7 @@ eth_dev_info(struct rte_eth_dev *dev,
static int
eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
{
- unsigned i;
+ unsigned int i;
unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
const struct pmd_internals *internal = dev->data->dev_private;
@@ -197,8 +199,9 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats
*stats)
static void
eth_stats_reset(struct rte_eth_dev *dev)
{
- unsigned i;
+ unsigned int i;
struct pmd_internals *internal = dev->data->dev_private;
+
for (i = 0; i < dev->data->nb_rx_queues; i++)
internal->rx_ring_queues[i].rx_pkts.cnt = 0;
for (i = 0; i < dev->data->nb_tx_queues; i++) {
@@ -250,8 +253,10 @@ static struct rte_vdev_driver pmd_ring_drv;
static int
do_eth_dev_ring_create(const char *name,
- struct rte_ring * const rx_queues[], const unsigned
nb_rx_queues,
- struct rte_ring *const tx_queues[], const unsigned nb_tx_queues,
+ struct rte_ring * const rx_queues[],
+ const unsigned int nb_rx_queues,
+ struct rte_ring *const tx_queues[],
+ const unsigned int nb_tx_queues,
const unsigned int numa_node, enum dev_action action,
struct rte_eth_dev **eth_dev_p)
{
@@ -260,7 +265,7 @@ do_eth_dev_ring_create(const char *name,
struct rte_eth_dev *eth_dev = NULL;
void **rx_queues_local = NULL;
void **tx_queues_local = NULL;
- unsigned i;
+ unsigned int i;
PMD_LOG(INFO, "Creating rings-backed ethdev on numa socket %u",
numa_node);
@@ -344,10 +349,10 @@ do_eth_dev_ring_create(const char *name,
int
rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
- const unsigned nb_rx_queues,
+ const unsigned int nb_rx_queues,
struct rte_ring *const tx_queues[],
- const unsigned nb_tx_queues,
- const unsigned numa_node)
+ const unsigned int nb_tx_queues,
+ const unsigned int numa_node)
{
struct ring_internal_args args = {
.rx_queues = rx_queues,
@@ -398,16 +403,16 @@ rte_eth_from_ring(struct rte_ring *r)
}
static int
-eth_dev_ring_create(const char *name, const unsigned numa_node,
+eth_dev_ring_create(const char *name, const unsigned int numa_node,
enum dev_action action, struct rte_eth_dev **eth_dev)
{
/* r
[dpdk-dev] [PATCH 2/4] net/ring: use sizeof() with snprintf
Don't hard code string length in two place; use sizeof() instead.
Ring name shoudl use RTE_RING_NAMESIZE.
Signed-off-by: Stephen Hemminger
---
drivers/net/ring/rte_eth_ring.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 91e5f5f8f262..2e4ca3b16a1c 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -362,8 +362,8 @@ rte_eth_from_rings(const char *name, struct rte_ring *const
rx_queues[],
.numa_node = numa_node,
.addr = &args,
};
- char args_str[32] = { 0 };
- char ring_name[32] = { 0 };
+ char args_str[32];
+ char ring_name[RTE_RING_NAMESIZE];
uint16_t port_id = RTE_MAX_ETHPORTS;
int ret;
@@ -381,8 +381,9 @@ rte_eth_from_rings(const char *name, struct rte_ring *const
rx_queues[],
return -1;
}
- snprintf(args_str, 32, "%s=%p", ETH_RING_INTERNAL_ARG, &args);
- snprintf(ring_name, 32, "net_ring_%s", name);
+ snprintf(args_str, sizeof(args_str), "%s=%p",
+ETH_RING_INTERNAL_ARG, &args);
+ snprintf(ring_name, sizeof(ring_name), "net_ring_%s", name);
ret = rte_vdev_init(ring_name, args_str);
if (ret) {
--
2.20.1
[dpdk-dev] [PATCH 4/4] net/ring: check length of ring name
The ring name is passed in as part of this drivers API, but it
doesn't check that the name is not truncated.
Signed-off-by: Stephen Hemminger
---
drivers/net/ring/rte_eth_ring.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 0355f2b7c4d8..9125692f2c9b 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -383,7 +383,12 @@ rte_eth_from_rings(const char *name, struct rte_ring
*const rx_queues[],
snprintf(args_str, sizeof(args_str), "%s=%p",
ETH_RING_INTERNAL_ARG, &args);
- snprintf(ring_name, sizeof(ring_name), "net_ring_%s", name);
+
+ ret = snprintf(ring_name, sizeof(ring_name), "net_ring_%s", name);
+ if (ret >= (int)sizeof(ring_name)) {
+ rte_errno = ENAMETOOLONG;
+ return -1;
+ }
ret = rte_vdev_init(ring_name, args_str);
if (ret) {
@@ -417,7 +422,15 @@ eth_dev_ring_create(const char *name, const unsigned int
numa_node,
RTE_PMD_RING_MAX_TX_RINGS);
for (i = 0; i < num_rings; i++) {
- snprintf(rng_name, sizeof(rng_name), "ETH_RXTX%u_%s", i, name);
+ int cc;
+
+ cc = snprintf(rng_name, sizeof(rng_name),
+ "ETH_RXTX%u_%s", i, name);
+ if (cc >= (int)sizeof(rng_name)) {
+ rte_errno = ENAMETOOLONG;
+ return -1;
+ }
+
rxtx[i] = (action == DEV_CREATE) ?
rte_ring_create(rng_name, 1024, numa_node,
RING_F_SP_ENQ|RING_F_SC_DEQ) :
--
2.20.1
[dpdk-dev] [PATCH 3/4] net/ring: use rte_calloc_socket
Use rte_calloc_socket instead of computing size.
Signed-off-by: Stephen Hemminger
---
drivers/net/ring/rte_eth_ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 2e4ca3b16a1c..0355f2b7c4d8 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -270,15 +270,15 @@ do_eth_dev_ring_create(const char *name,
PMD_LOG(INFO, "Creating rings-backed ethdev on numa socket %u",
numa_node);
- rx_queues_local = rte_zmalloc_socket(name,
- sizeof(void *) * nb_rx_queues, 0, numa_node);
+ rx_queues_local = rte_calloc_socket(name, nb_rx_queues,
+ sizeof(void *), 0, numa_node);
if (rx_queues_local == NULL) {
rte_errno = ENOMEM;
goto error;
}
- tx_queues_local = rte_zmalloc_socket(name,
- sizeof(void *) * nb_tx_queues, 0, numa_node);
+ tx_queues_local = rte_calloc_socket(name, nb_tx_queues,
+ sizeof(void *), 0, numa_node);
if (tx_queues_local == NULL) {
rte_errno = ENOMEM;
goto error;
--
2.20.1
[dpdk-dev] [PATCH v2 1/4] net/ring: fix coding style
Whitespace fixes to bring inline with current DPDK style.
Signed-off-by: Stephen Hemminger
---
v2 - fix signoff line
drivers/net/ring/rte_eth_ring.c | 58 ++---
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 115a882b507c..91e5f5f8f262 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -45,8 +45,8 @@ struct ring_queue {
};
struct pmd_internals {
- unsigned max_rx_queues;
- unsigned max_tx_queues;
+ unsigned int max_rx_queues;
+ unsigned int max_tx_queues;
struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
@@ -55,12 +55,11 @@ struct pmd_internals {
enum dev_action action;
};
-
static struct rte_eth_link pmd_link = {
- .link_speed = ETH_SPEED_NUM_10G,
- .link_duplex = ETH_LINK_FULL_DUPLEX,
- .link_status = ETH_LINK_DOWN,
- .link_autoneg = ETH_LINK_FIXED,
+ .link_speed = ETH_SPEED_NUM_10G,
+ .link_duplex = ETH_LINK_FULL_DUPLEX,
+ .link_status = ETH_LINK_DOWN,
+ .link_autoneg = ETH_LINK_FIXED,
};
static int eth_ring_logtype;
@@ -138,6 +137,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t
rx_queue_id,
struct rte_mempool *mb_pool __rte_unused)
{
struct pmd_internals *internals = dev->data->dev_private;
+
dev->data->rx_queues[rx_queue_id] =
&internals->rx_ring_queues[rx_queue_id];
return 0;
}
@@ -149,6 +149,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
tx_queue_id,
const struct rte_eth_txconf *tx_conf
__rte_unused)
{
struct pmd_internals *internals = dev->data->dev_private;
+
dev->data->tx_queues[tx_queue_id] =
&internals->tx_ring_queues[tx_queue_id];
return 0;
}
@@ -156,9 +157,10 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
tx_queue_id,
static void
eth_dev_info(struct rte_eth_dev *dev,
- struct rte_eth_dev_info *dev_info)
+struct rte_eth_dev_info *dev_info)
{
struct pmd_internals *internals = dev->data->dev_private;
+
dev_info->max_mac_addrs = 1;
dev_info->max_rx_pktlen = (uint32_t)-1;
dev_info->max_rx_queues = (uint16_t)internals->max_rx_queues;
@@ -169,7 +171,7 @@ eth_dev_info(struct rte_eth_dev *dev,
static int
eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
{
- unsigned i;
+ unsigned int i;
unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
const struct pmd_internals *internal = dev->data->dev_private;
@@ -197,8 +199,9 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats
*stats)
static void
eth_stats_reset(struct rte_eth_dev *dev)
{
- unsigned i;
+ unsigned int i;
struct pmd_internals *internal = dev->data->dev_private;
+
for (i = 0; i < dev->data->nb_rx_queues; i++)
internal->rx_ring_queues[i].rx_pkts.cnt = 0;
for (i = 0; i < dev->data->nb_tx_queues; i++) {
@@ -250,8 +253,10 @@ static struct rte_vdev_driver pmd_ring_drv;
static int
do_eth_dev_ring_create(const char *name,
- struct rte_ring * const rx_queues[], const unsigned
nb_rx_queues,
- struct rte_ring *const tx_queues[], const unsigned nb_tx_queues,
+ struct rte_ring * const rx_queues[],
+ const unsigned int nb_rx_queues,
+ struct rte_ring *const tx_queues[],
+ const unsigned int nb_tx_queues,
const unsigned int numa_node, enum dev_action action,
struct rte_eth_dev **eth_dev_p)
{
@@ -260,7 +265,7 @@ do_eth_dev_ring_create(const char *name,
struct rte_eth_dev *eth_dev = NULL;
void **rx_queues_local = NULL;
void **tx_queues_local = NULL;
- unsigned i;
+ unsigned int i;
PMD_LOG(INFO, "Creating rings-backed ethdev on numa socket %u",
numa_node);
@@ -344,10 +349,10 @@ do_eth_dev_ring_create(const char *name,
int
rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
- const unsigned nb_rx_queues,
+ const unsigned int nb_rx_queues,
struct rte_ring *const tx_queues[],
- const unsigned nb_tx_queues,
- const unsigned numa_node)
+ const unsigned int nb_tx_queues,
+ const unsigned int numa_node)
{
struct ring_internal_args args = {
.rx_queues = rx_queues,
@@ -398,16 +403,16 @@ rte_eth_from_ring(struct rte_ring *r)
}
static int
-eth_dev_ring_create(const char *name, const unsigned numa_node,
+eth_dev_ring_create(const char *name, const unsigned int numa_node,
enum dev_action action, struct rte_eth_dev **e
[dpdk-dev] [PATCH v2 0/4] net/ring: driver fixes
Minor fixes and cleanups for the net/ring driver. Stephen Hemminger (4): net/ring: fix coding style net/ring: use sizeof() with snprintf net/ring: use rte_calloc_socket net/ring: check length of ring name drivers/net/ring/rte_eth_ring.c | 90 - 1 file changed, 55 insertions(+), 35 deletions(-) -- 2.20.1
[dpdk-dev] [PATCH v2 2/4] net/ring: use sizeof() with snprintf
Don't hard code string length in two place; use sizeof() instead.
Ring name shoudl use RTE_RING_NAMESIZE.
Signed-off-by: Stephen Hemminger
---
drivers/net/ring/rte_eth_ring.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 91e5f5f8f262..2e4ca3b16a1c 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -362,8 +362,8 @@ rte_eth_from_rings(const char *name, struct rte_ring *const
rx_queues[],
.numa_node = numa_node,
.addr = &args,
};
- char args_str[32] = { 0 };
- char ring_name[32] = { 0 };
+ char args_str[32];
+ char ring_name[RTE_RING_NAMESIZE];
uint16_t port_id = RTE_MAX_ETHPORTS;
int ret;
@@ -381,8 +381,9 @@ rte_eth_from_rings(const char *name, struct rte_ring *const
rx_queues[],
return -1;
}
- snprintf(args_str, 32, "%s=%p", ETH_RING_INTERNAL_ARG, &args);
- snprintf(ring_name, 32, "net_ring_%s", name);
+ snprintf(args_str, sizeof(args_str), "%s=%p",
+ETH_RING_INTERNAL_ARG, &args);
+ snprintf(ring_name, sizeof(ring_name), "net_ring_%s", name);
ret = rte_vdev_init(ring_name, args_str);
if (ret) {
--
2.20.1
[dpdk-dev] [PATCH v2 3/4] net/ring: use rte_calloc_socket
Use rte_calloc_socket instead of computing size.
Signed-off-by: Stephen Hemminger
---
drivers/net/ring/rte_eth_ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 2e4ca3b16a1c..0355f2b7c4d8 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -270,15 +270,15 @@ do_eth_dev_ring_create(const char *name,
PMD_LOG(INFO, "Creating rings-backed ethdev on numa socket %u",
numa_node);
- rx_queues_local = rte_zmalloc_socket(name,
- sizeof(void *) * nb_rx_queues, 0, numa_node);
+ rx_queues_local = rte_calloc_socket(name, nb_rx_queues,
+ sizeof(void *), 0, numa_node);
if (rx_queues_local == NULL) {
rte_errno = ENOMEM;
goto error;
}
- tx_queues_local = rte_zmalloc_socket(name,
- sizeof(void *) * nb_tx_queues, 0, numa_node);
+ tx_queues_local = rte_calloc_socket(name, nb_tx_queues,
+ sizeof(void *), 0, numa_node);
if (tx_queues_local == NULL) {
rte_errno = ENOMEM;
goto error;
--
2.20.1
[dpdk-dev] [PATCH v2 4/4] net/ring: check length of ring name
The ring name is passed in as part of this drivers API, but it
doesn't check that the name is not truncated.
Signed-off-by: Stephen Hemminger
---
v2 - fix whitespace creep
drivers/net/ring/rte_eth_ring.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 0355f2b7c4d8..979993fd3e11 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -383,7 +383,12 @@ rte_eth_from_rings(const char *name, struct rte_ring
*const rx_queues[],
snprintf(args_str, sizeof(args_str), "%s=%p",
ETH_RING_INTERNAL_ARG, &args);
- snprintf(ring_name, sizeof(ring_name), "net_ring_%s", name);
+
+ ret = snprintf(ring_name, sizeof(ring_name), "net_ring_%s", name);
+ if (ret >= (int)sizeof(ring_name)) {
+ rte_errno = ENAMETOOLONG;
+ return -1;
+ }
ret = rte_vdev_init(ring_name, args_str);
if (ret) {
@@ -417,7 +422,15 @@ eth_dev_ring_create(const char *name, const unsigned int
numa_node,
RTE_PMD_RING_MAX_TX_RINGS);
for (i = 0; i < num_rings; i++) {
- snprintf(rng_name, sizeof(rng_name), "ETH_RXTX%u_%s", i, name);
+ int cc;
+
+ cc = snprintf(rng_name, sizeof(rng_name),
+ "ETH_RXTX%u_%s", i, name);
+ if (cc >= (int)sizeof(rng_name)) {
+ rte_errno = ENAMETOOLONG;
+ return -1;
+ }
+
rxtx[i] = (action == DEV_CREATE) ?
rte_ring_create(rng_name, 1024, numa_node,
RING_F_SP_ENQ|RING_F_SC_DEQ) :
--
2.20.1
Re: [dpdk-dev] [RFC v2 1/2] eal: replace libc-based random number generation with LFSR
On 2019-04-23 13:33, Neil Horman wrote: On Mon, Apr 22, 2019 at 07:44:39PM +0200, Mattias Rönnblom wrote: On 2019-04-22 17:52, Mattias Rönnblom wrote: On 2019-04-22 13:34, Neil Horman wrote: +uint64_t __rte_experimental +rte_rand(void) Do you really want to mark this as experimental? I know it will trigger the symbol checker with a warning if you don't, but this function already existed previously and was accepted as part of the ABI. Given that the prototype hasn't changed, I think you just need to accept it as a non-experimental function I'll remove the experimental tag and move it into the 19_05 section (without suggesting it should go into 19.05). That maneuver seems not to trigger any build warnings/errors. OK, so that wasn't true. It does trigger a build error, courtesy of buildtools/check-experimental-syms.sh. I can't see any obvious way around it. Ideas, anyone? No, we would have to waive it. I don't understand. What do you mean? But its pretty clear that This function has been around forever, so I think it would be worse to demote it to an experimental symbol. The only thing you're doing here is moving it from an inline function (which is arguably part of the ABI, even if it never appeared as a symbol in the ELF file), to a fully fleged symbol with a new implementation. I agree it shouldn't be marked experimental. The reason for doing so was to avoid triggering a build error.
Re: [dpdk-dev] [RFC v2 1/2] eal: replace libc-based random number generation with LFSR
On 2019-04-23 17:31, Stephen Hemminger wrote: On Mon, 22 Apr 2019 19:44:39 +0200 Mattias Rönnblom wrote: On 2019-04-22 17:52, Mattias Rönnblom wrote: On 2019-04-22 13:34, Neil Horman wrote: +uint64_t __rte_experimental +rte_rand(void) Do you really want to mark this as experimental? I know it will trigger the symbol checker with a warning if you don't, but this function already existed previously and was accepted as part of the ABI. Given that the prototype hasn't changed, I think you just need to accept it as a non-experimental function I'll remove the experimental tag and move it into the 19_05 section (without suggesting it should go into 19.05). That maneuver seems not to trigger any build warnings/errors. OK, so that wasn't true. It does trigger a build error, courtesy of buildtools/check-experimental-syms.sh. I can't see any obvious way around it. Ideas, anyone? Ignore the error, the build tool is not smart enough for this kind of change. It stops the build.
[dpdk-dev] [PATCH v2 1/8] ipc: fix rte_mp_request_sync memleak
When sending multiple requests, rte_mp_request_sync
can succeed sending a few of those requests, but then
fail on a later one and in the end return with rc=-1.
The upper layers - e.g. device hotplug - currently
handles this case as if no messages were sent and no
memory for response buffers was allocated, which is
not true. Fixed by always initializing message buffer
to NULL.
Bugzilla ID: 228
Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
Cc: jianfeng@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
Acked-by: Anatoly Burakov
---
lib/librte_eal/common/eal_common_proc.c | 6 +++---
lib/librte_eal/common/include/rte_eal.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_proc.c
b/lib/librte_eal/common/eal_common_proc.c
index b46d644b3..abaff5164 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -927,13 +927,13 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
rte_mp_reply *reply,
RTE_LOG(DEBUG, EAL, "request: %s\n", req->name);
- if (check_input(req) == false)
- return -1;
-
reply->nb_sent = 0;
reply->nb_received = 0;
reply->msgs = NULL;
+ if (check_input(req) == false)
+ return -1;
+
if (internal_config.no_shconf) {
RTE_LOG(DEBUG, EAL, "No shared files mode enabled, IPC is
disabled\n");
return 0;
diff --git a/lib/librte_eal/common/include/rte_eal.h
b/lib/librte_eal/common/include/rte_eal.h
index 833433229..575f8119e 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -309,7 +309,8 @@ rte_mp_sendmsg(struct rte_mp_msg *msg);
* This function sends a request message to the peer process, and will
* block until receiving reply message from the peer process.
*
- * @note The caller is responsible to free reply->replies.
+ * @note The caller is responsible to free reply->replies (even if the function
+ *returned failure).
*
* @param req
* The req argument contains the customized request message.
--
2.17.2
[dpdk-dev] [PATCH v2 0/8] ipc: fix possible memleaks
When sending multiple requests, rte_mp_request_sync can succeed sending a few of those requests, but then fail on a later one and in the end return with rc=-1. The upper layers - e.g. device hotplug - currently handles this case as if no messages were sent and no memory for response buffers was allocated, which is not true. Fixed by always initializing message buffer to NULL and calling free everytime rte_mp_request_sync is used. v2: - resending as patchset to make it easier to review it. - changed commit message as requested. - added bugzilla id. Bugzilla ID: 228 Herakliusz Lipiec (8): ipc: fix rte_mp_request_sync memleak ipc: fix hotplug memleak ipc: fix vdev memleak ipc: fix vfio memleak ipc: fix pdump memleak ipc: fix tap pmd memleak ipc: fix net/mlx4 memleak ipc: fix net/mlx5 memleak drivers/bus/vdev/vdev.c | 3 +-- drivers/net/mlx4/mlx4_mp.c | 4 +++- drivers/net/mlx5/mlx5_mp.c | 4 +++- drivers/net/tap/rte_eth_tap.c | 2 ++ lib/librte_eal/common/eal_common_proc.c | 6 +++--- lib/librte_eal/common/hotplug_mp.c | 2 ++ lib/librte_eal/common/include/rte_eal.h | 3 ++- lib/librte_eal/linux/eal/eal_vfio.c | 8 lib/librte_pdump/rte_pdump.c| 2 +- 9 files changed, 21 insertions(+), 13 deletions(-) -- 2.17.2
[dpdk-dev] [PATCH v2 2/8] ipc: fix hotplug memleak
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Bugzilla ID: 228
Fixes: ac9e4a17370f ("eal: support attach/detach shared device from secondary")
Cc: qi.z.zh...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
---
lib/librte_eal/common/hotplug_mp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_eal/common/hotplug_mp.c
b/lib/librte_eal/common/hotplug_mp.c
index 4052a5c7f..2c8366afa 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -377,6 +377,7 @@ int eal_dev_hotplug_request_to_primary(struct
eal_dev_mp_req *req)
ret = rte_mp_request_sync(&mp_req, &mp_reply, &ts);
if (ret || mp_reply.nb_received != 1) {
RTE_LOG(ERR, EAL, "cannot send request to primary");
+ free(mp_reply.msgs);
if (!ret)
return -1;
return ret;
@@ -405,6 +406,7 @@ int eal_dev_hotplug_request_to_secondary(struct
eal_dev_mp_req *req)
ret = rte_mp_request_sync(&mp_req, &mp_reply, &ts);
if (ret != 0) {
RTE_LOG(ERR, EAL, "rte_mp_request_sync failed\n");
+ free(mp_reply.msgs);
return ret;
}
--
2.17.2
[dpdk-dev] [PATCH v2 6/8] ipc: fix tap pmd memleak
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Bugzilla ID: 228
Fixes: c9aa56edec8e ("net/tap: access primary process queues from secondary")
Cc: rasl...@mellanox.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
---
drivers/net/tap/rte_eth_tap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index e9fda8cf6..d70412d62 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -2104,6 +2104,7 @@ tap_mp_attach_queues(const char *port_name, struct
rte_eth_dev *dev)
if (ret < 0) {
TAP_LOG(ERR, "Failed to request queues from primary: %d",
rte_errno);
+ free(replies.msgs);
return -1;
}
reply = &replies.msgs[0];
@@ -2119,6 +2120,7 @@ tap_mp_attach_queues(const char *port_name, struct
rte_eth_dev *dev)
for (queue = 0; queue < reply_param->txq_count; queue++)
process_private->txq_fds[queue] = reply->fds[fd_iterator++];
+ free(replies.msgs);
return 0;
}
--
2.17.2
[dpdk-dev] [PATCH v2 5/8] ipc: fix pdump memleak
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Bugzilla ID: 228
Fixes: 660098d61f57 ("pdump: use generic multi-process channel")
Cc: jianfeng@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
Acked-By: Reshma Pattan
---
lib/librte_pdump/rte_pdump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 14744b9ff..3787c3e32 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -525,8 +525,8 @@ pdump_prepare_client_request(char *device, uint16_t queue,
rte_errno = resp->err_value;
if (!resp->err_value)
ret = 0;
- free(mp_reply.msgs);
}
+ free(mp_reply.msgs);
if (ret < 0)
RTE_LOG(ERR, PDUMP,
--
2.17.2
[dpdk-dev] [PATCH v2 3/8] ipc: fix vdev memleak
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Bugzilla ID: 228
Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
Cc: jianfeng@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
---
drivers/bus/vdev/vdev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 04f76a63f..7c43f2ddd 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -429,10 +429,9 @@ vdev_scan(void)
mp_rep = &mp_reply.msgs[0];
resp = (struct vdev_param *)mp_rep->param;
VDEV_LOG(INFO, "Received %d vdevs", resp->num);
- free(mp_reply.msgs);
} else
VDEV_LOG(ERR, "Failed to request vdev from primary");
-
+ free(mp_reply.msgs);
/* Fall through to allow private vdevs in secondary process */
}
--
2.17.2
[dpdk-dev] [PATCH v2 4/8] ipc: fix vfio memleak
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Bugzilla ID: 228
Fixes: 83a73c5fef66 ("vfio: use generic multi-process channel")
Cc: jianfeng@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Herakliusz Lipiec
Acked-by: Anatoly Burakov
---
lib/librte_eal/linux/eal/eal_vfio.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/librte_eal/linux/eal/eal_vfio.c
b/lib/librte_eal/linux/eal/eal_vfio.c
index 19e70bb66..d293df062 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.c
+++ b/lib/librte_eal/linux/eal/eal_vfio.c
@@ -319,8 +319,8 @@ vfio_open_group_fd(int iommu_group_num)
RTE_LOG(ERR, EAL, " bad VFIO group fd\n");
vfio_group_fd = 0;
}
- free(mp_reply.msgs);
}
+ free(mp_reply.msgs);
if (vfio_group_fd < 0)
RTE_LOG(ERR, EAL, " cannot request group fd\n");
@@ -583,8 +583,8 @@ vfio_sync_default_container(void)
p = (struct vfio_mp_param *)mp_rep->param;
if (p->result == SOCKET_OK)
iommu_type_id = p->iommu_type_id;
- free(mp_reply.msgs);
}
+ free(mp_reply.msgs);
if (iommu_type_id < 0) {
RTE_LOG(ERR, EAL, "Could not get IOMMU type for default
container\n");
return -1;
@@ -1050,8 +1050,8 @@ vfio_get_default_container_fd(void)
free(mp_reply.msgs);
return mp_rep->fds[0];
}
- free(mp_reply.msgs);
}
+ free(mp_reply.msgs);
RTE_LOG(ERR, EAL, " cannot request default container fd\n");
return -1;
@@ -1182,8 +1182,8 @@ rte_vfio_get_container_fd(void)
free(mp_reply.msgs);
return vfio_container_fd;
}
- free(mp_reply.msgs);
}
+ free(mp_reply.msgs);
RTE_LOG(ERR, EAL, " cannot request container fd\n");
return -1;
--
2.17.2
[dpdk-dev] [PATCH v2 8/8] ipc: fix net/mlx5 memleak
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Bugzilla ID:228
Fixes: 9a8ab29b84d3 ("net/mlx5: replace IPC socket with EAL API")
Fixes: c18cf501a7af ("net/mlx5: enable secondary process to register DMA
memory")
Cc: ys...@mellanox.com
Signed-off-by: Herakliusz Lipiec
---
drivers/net/mlx5/mlx5_mp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_mp.c b/drivers/net/mlx5/mlx5_mp.c
index cea74adb6..c9915b1d5 100644
--- a/drivers/net/mlx5/mlx5_mp.c
+++ b/drivers/net/mlx5/mlx5_mp.c
@@ -258,6 +258,7 @@ mlx5_mp_req_mr_create(struct rte_eth_dev *dev, uintptr_t
addr)
if (ret) {
DRV_LOG(ERR, "port %u request to primary process failed",
dev->data->port_id);
+ free(mp_rep.msgs);
return -rte_errno;
}
assert(mp_rep.nb_received == 1);
@@ -295,7 +296,8 @@ mlx5_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev)
if (ret) {
DRV_LOG(ERR, "port %u request to primary process failed",
dev->data->port_id);
- return -rte_errno;
+ ret = -rte_errno;
+ goto exit;
}
assert(mp_rep.nb_received == 1);
mp_res = &mp_rep.msgs[0];
--
2.17.2
[dpdk-dev] [PATCH v2 7/8] ipc: fix net/mlx4 memleak
When sending synchronous IPC requests, the caller must free the response
buffer even if the request returned failure. Fix the code to correctly
use the IPC API.
Bugzilla ID:228
Fixes: 0b259b8e9655 ("net/mlx4: enable secondary process to register DMA
memory")
Cc: ys...@mellanox.com
Signed-off-by: Herakliusz Lipiec
---
drivers/net/mlx4/mlx4_mp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx4/mlx4_mp.c b/drivers/net/mlx4/mlx4_mp.c
index 183622453..f4cff7486 100644
--- a/drivers/net/mlx4/mlx4_mp.c
+++ b/drivers/net/mlx4/mlx4_mp.c
@@ -255,6 +255,7 @@ mlx4_mp_req_mr_create(struct rte_eth_dev *dev, uintptr_t
addr)
if (ret) {
ERROR("port %u request to primary process failed",
dev->data->port_id);
+ free(mp_rep.msgs);
return -rte_errno;
}
assert(mp_rep.nb_received == 1);
@@ -292,7 +293,8 @@ mlx4_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev)
if (ret) {
ERROR("port %u request to primary process failed",
dev->data->port_id);
- return -rte_errno;
+ ret = -rte_errno;
+ goto exit;
}
assert(mp_rep.nb_received == 1);
mp_res = &mp_rep.msgs[0];
--
2.17.2
[dpdk-dev] [PATCH v2] doc: update release notes for windows support
Added documentation for Windows support on 19.05 release. Signed-off-by: Pallavi Kadam Reviewed-by: Anand Rawat Reviewed-by: Ranjit Menon --- doc/guides/rel_notes/release_19_05.rst | 4 1 file changed, 4 insertions(+) diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst index d5ed564ab..7e91e7d88 100644 --- a/doc/guides/rel_notes/release_19_05.rst +++ b/doc/guides/rel_notes/release_19_05.rst @@ -187,6 +187,10 @@ New Features Improved testpmd application performance on ARM platform. For ``macswap`` forwarding mode, NEON intrinsics were used to do swap to save CPU cycles. +* **Added Windows Support.** + + Added Windows support to compile and build Hello World Sample Application. + Removed Items - -- 2.18.0.windows.1
Re: [dpdk-dev] [PATCH 8/8] ipc: fix net/mlx5 memleak
> On Apr 23, 2019, at 1:09 AM, Burakov, Anatoly
> wrote:
>
> On 22-Apr-19 6:51 PM, Yongseok Koh wrote:
>>> On Apr 17, 2019, at 7:44 AM, Herakliusz Lipiec
>>> wrote:
>>>
>>> When sending multiple requests, rte_mp_request_sync
>>> can succeed sending a few of those requests, but then
>>> fail on a later one and in the end return with rc=-1.
>>> The upper layers - e.g. device hotplug - currently
>>> handles this case as if no messages were sent and no
>>> memory for response buffers was allocated, which is
>>> not true. Fixed by always freeing reply message buffers.
>>>
>>> Fixes: 9a8ab29b84d3 ("net/mlx5: replace IPC socket with EAL API")
>>> Fixes: c18cf501a7af ("net/mlx5: enable secondary process to register DMA
>>> memory")
>>> Cc: ys...@mellanox.com
>>> Signed-off-by: Herakliusz Lipiec
>>> ---
>>> drivers/net/mlx5/mlx5_mp.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/mlx5/mlx5_mp.c b/drivers/net/mlx5/mlx5_mp.c
>>> index cea74adb6..c9915b1d5 100644
>>> --- a/drivers/net/mlx5/mlx5_mp.c
>>> +++ b/drivers/net/mlx5/mlx5_mp.c
>>> @@ -258,6 +258,7 @@ mlx5_mp_req_mr_create(struct rte_eth_dev *dev,
>>> uintptr_t addr)
>>> if (ret) {
>>> DRV_LOG(ERR, "port %u request to primary process failed",
>>> dev->data->port_id);
>>> + free(mp_rep.msgs);
>>> return -rte_errno;
>>> }
>>> assert(mp_rep.nb_received == 1);
>>> @@ -295,7 +296,8 @@ mlx5_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev)
>>> if (ret) {
>>> DRV_LOG(ERR, "port %u request to primary process failed",
>>> dev->data->port_id);
>>> - return -rte_errno;
>>> + ret = -rte_errno;
>>> + goto exit;
>> These two requests will be made by a secondary process targeting to the
>> primary.
>> Then, there's only one request in this case and we don't need to take care
>> of that.
>> Right?
>> Same comment for mlx4.
>
> Hi Yongseok,
>
> mp_rep.msgs is potentially allocated regardless of whether you're in primary
> or secondary, and whether the call to mp_request_sync succeeded or failed.
> Hence, need to free in all cases.
Then, it looks fine to me.
>
> See this patch:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatches.dpdk.org%2Fpatch%2F52868%2F&data=02%7C01%7Cyskoh%40mellanox.com%7C007b61ef9d964dc79e7108d6c7c2f9d8%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636916037564345993&sdata=O%2BoG%2F8P8cXwKS%2FDfZyMiG3CiIDpeXe3dPMJgVilzFWY%3D&reserved=0
> and this bug:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.dpdk.org%2Fshow_bug.cgi%3Fid%3D228&data=02%7C01%7Cyskoh%40mellanox.com%7C007b61ef9d964dc79e7108d6c7c2f9d8%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636916037564345993&sdata=jLA5wLqT%2BfW3p79rg2SVEZ16GS37dgqdF4NwmiRU%2B7A%3D&reserved=0
>
>> Thanks,
>> Yongseok
>>> }
>>> assert(mp_rep.nb_received == 1);
>>> mp_res = &mp_rep.msgs[0];
>>> --
>>> 2.17.2
>>>
>
>
> --
> Thanks,
> Anatoly
Re: [dpdk-dev] [PATCH v2 7/8] ipc: fix net/mlx4 memleak
> On Apr 23, 2019, at 10:43 AM, Herakliusz Lipiec
> wrote:
>
> When sending synchronous IPC requests, the caller must free the response
> buffer even if the request returned failure. Fix the code to correctly
> use the IPC API.
>
> Bugzilla ID:228
> Fixes: 0b259b8e9655 ("net/mlx4: enable secondary process to register DMA
> memory")
> Cc: ys...@mellanox.com
> Signed-off-by: Herakliusz Lipiec
> ---
Acked-by: Yongseok Koh
> drivers/net/mlx4/mlx4_mp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/mlx4/mlx4_mp.c b/drivers/net/mlx4/mlx4_mp.c
> index 183622453..f4cff7486 100644
> --- a/drivers/net/mlx4/mlx4_mp.c
> +++ b/drivers/net/mlx4/mlx4_mp.c
> @@ -255,6 +255,7 @@ mlx4_mp_req_mr_create(struct rte_eth_dev *dev, uintptr_t
> addr)
> if (ret) {
> ERROR("port %u request to primary process failed",
> dev->data->port_id);
> + free(mp_rep.msgs);
> return -rte_errno;
> }
> assert(mp_rep.nb_received == 1);
> @@ -292,7 +293,8 @@ mlx4_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev)
> if (ret) {
> ERROR("port %u request to primary process failed",
> dev->data->port_id);
> - return -rte_errno;
> + ret = -rte_errno;
> + goto exit;
> }
> assert(mp_rep.nb_received == 1);
> mp_res = &mp_rep.msgs[0];
> --
> 2.17.2
>
Re: [dpdk-dev] [PATCH v2 8/8] ipc: fix net/mlx5 memleak
> On Apr 23, 2019, at 10:43 AM, Herakliusz Lipiec
> wrote:
>
> When sending synchronous IPC requests, the caller must free the response
> buffer even if the request returned failure. Fix the code to correctly
> use the IPC API.
>
> Bugzilla ID:228
> Fixes: 9a8ab29b84d3 ("net/mlx5: replace IPC socket with EAL API")
> Fixes: c18cf501a7af ("net/mlx5: enable secondary process to register DMA
> memory")
> Cc: ys...@mellanox.com
> Signed-off-by: Herakliusz Lipiec
> ---
Acked-by: Yongseok Koh
> drivers/net/mlx5/mlx5_mp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/mlx5/mlx5_mp.c b/drivers/net/mlx5/mlx5_mp.c
> index cea74adb6..c9915b1d5 100644
> --- a/drivers/net/mlx5/mlx5_mp.c
> +++ b/drivers/net/mlx5/mlx5_mp.c
> @@ -258,6 +258,7 @@ mlx5_mp_req_mr_create(struct rte_eth_dev *dev, uintptr_t
> addr)
> if (ret) {
> DRV_LOG(ERR, "port %u request to primary process failed",
> dev->data->port_id);
> + free(mp_rep.msgs);
> return -rte_errno;
> }
> assert(mp_rep.nb_received == 1);
> @@ -295,7 +296,8 @@ mlx5_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev)
> if (ret) {
> DRV_LOG(ERR, "port %u request to primary process failed",
> dev->data->port_id);
> - return -rte_errno;
> + ret = -rte_errno;
> + goto exit;
> }
> assert(mp_rep.nb_received == 1);
> mp_res = &mp_rep.msgs[0];
> --
> 2.17.2
>
Re: [dpdk-dev] [PATCH] net/mlx5: fix E-Switch flow without port item
On Tue, Apr 23, 2019 at 11:19:16AM +, Ori Kam wrote:
> When creating a flow rule without the port_id pattern item, always the
> PF was selected.
>
> This commit fixes this issue, if no port_id pattern item is available
> then we use the port that the flow was created on as source port.
>
> Fixes: 822fb3195348 ("net/mlx5: add port id item to Direct Verbs")
>
> Signed-off-by: Ori Kam
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 30 +++---
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index c2a2fc6..d17adbe 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3623,6 +3623,8 @@ struct field_modify_info modify_tcp[] = {
> union flow_dv_attr flow_attr = { .attr = 0 };
> struct mlx5_flow_dv_tag_resource tag_resource;
> uint32_t modify_action_position = UINT32_MAX;
> + void *match_mask = matcher.mask.buf;
> + void *match_value = dev_flow->dv.value.buf;
>
> flow->group = attr->group;
> if (attr->transfer)
> @@ -3895,23 +3897,8 @@ struct field_modify_info modify_tcp[] = {
> }
> dev_flow->dv.actions_n = actions_n;
> flow->actions = action_flags;
> - if (attr->ingress && !attr->transfer &&
> - (priv->representor || priv->master)) {
> - /* It was validated - we support unidirection flows only. */
> - assert(!attr->egress);
> - /*
> - * Add matching on source vport index only
> - * for ingress rules in E-Switch configurations.
> - */
> - flow_dv_translate_item_source_vport(matcher.mask.buf,
> - dev_flow->dv.value.buf,
> - priv->vport_id,
> - 0x);
> - }
> for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
> int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
> - void *match_mask = matcher.mask.buf;
> - void *match_value = dev_flow->dv.value.buf;
>
> switch (items->type) {
> case RTE_FLOW_ITEM_TYPE_PORT_ID:
> @@ -4018,6 +4005,19 @@ struct field_modify_info modify_tcp[] = {
> }
> item_flags |= last_item;
> }
> + if (((attr->ingress && !attr->transfer) ||
> + (attr->transfer && !(item_flags & MLX5_FLOW_ITEM_PORT_ID))) &&
> + (priv->representor || priv->master)) {
>From the validations, I could figure out
- Either ingress (I) or egress (E) must be specified
- Transfer (T) can't be egress
- Port ID (P) is valid only if transfer (T) is specified.
(!T and I) or (T and !P)
= (I - T) + (T - P)
= I - P
So, this condition is equivalent to
if (attr->ingress && !!(item_flags & MLX5_FLOW_ITEM_PORT_ID) &&
(priv->representor || priv->master)) {
...
}
Right?
If agreed, please add comment properly.
> + /* It was validated - we support unidirection flows only. */
> + assert(!attr->egress);
This comment and assert are there to mention ingress and egress are exclusive.
Is it still relevant? Did you also test the patch with enabling DEBUG?
> + /*
> + * Add matching on source vport index only
> + * for ingress rules in E-Switch configurations.
> + */
Please make this comment appropriate as well.
Thanks,
Yongseok
> + if (flow_dv_translate_item_port_id(dev, match_mask,
> +match_value, NULL))
> + return -rte_errno;
> + }
> assert(!flow_dv_check_valid_spec(matcher.mask.buf,
>dev_flow->dv.value.buf));
> dev_flow->layers = item_flags;
> --
> 1.8.3.1
>
Re: [dpdk-dev] [PATCH v7 1/3] rcu: add RCU library supporting QSBR mechanism
> > On Mon, Apr 22, 2019 at 11:31:28PM -0500, Honnappa Nagarahalli wrote: > > 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 > > Much better! > > Acked-by: Paul E. McKenney > Thanks a lot, appreciate your feedback. Any views from maintainers on including this library into RC3? IMO, this library is independent and should not affect existing code.
[dpdk-dev] [PATCH 1/4] 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/dpdk) directory for reference.
Signed-off-by: Bruce Richardson
---
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.20.1
[dpdk-dev] [PATCH 0/4] 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. 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(-) -- 2.20.1
[dpdk-dev] [PATCH 2/4] examples: simplify getting list of all examples
Using find rather than shelling out to a one-liner script is easier to
read and understand, as well as allowing shorter lines in the code.
Signed-off-by: Bruce Richardson
---
examples/meson.build | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/examples/meson.build b/examples/meson.build
index 1a6134f12..19f2686b2 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -8,8 +8,9 @@ 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'
+all_examples = run_command('find', meson.current_source_dir(),
+ '-maxdepth', '1', '-mindepth', '1',
+ '-type', 'd', '-printf', '%f '
).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.
--
2.20.1
[dpdk-dev] [PATCH 3/4] 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 --- 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 + +for example in helloworld l2fwd l3fwd skeleton timer; do + echo "## Building $example" + make -C $DESTDIR/usr/local/share/dpdk/examples/$example +done -- 2.20.1
[dpdk-dev] [PATCH 4/4] 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
---
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 f8aded6ed..64e2315dd 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -125,12 +125,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 dfba2a782..41080353b 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -103,7 +103,7 @@ $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
-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 a96486597..3c173a587 100644
--- a/meson.build
+++ b/meson.build
@@ -77,6 +77,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.20.1
Re: [dpdk-dev] [PATCH 0/4] add testing of libdpdk pkg-config file
On Tue, 23 Apr 2019 23:06:40 +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(-) > My experiments with Ubuntu 18.04 showed the default version of meson was too old (broken) and generated bad cflags. Getting later one for Debian stable-backports worked.
[dpdk-dev] DPDK Windows Community call - 18 April 2019
Attendees: (Present) Thomas (tho...@monjalon.net) Harini (harini.ramakrish...@microsoft.com) Bruce (bruce.richard...@intel.com) Cathal (cathal.oh...@intel.com) Anand (anand.ra...@intel.com) Pallavi (pallavi.ka...@intel.com) Ranjit (ranjit.me...@intel.com) Eilon (eil...@mellanox.com) Raslan (rasl...@mellanox.com) (yoh...@mellanox.com) Jeffrey (jeffrey.tip...@microsoft.com) Omar (ocard...@microsoft.com) Khoa (k...@microsoft.com) New interested attendees added to meeting invite for next call: haseeb.g...@cavium.com shsha...@marvell.com nareshkumar@broadcom.com Agenda: -- Meeting for 1 hour as a place holder every 2 weeks. Expect call to last 30-40 minutes or longer as needed. Encourage other dpdk.org windows partners to join this call. Minutes via public mailing list. a. Current release b. Improvements/Shared objectives - sync up on the bus/pci and general status - Mellanox agrees bus/pci port should be our next target - Intel has latest bus/pci updates available in windows draft repo c. Any other business Minutes: --- - Removing rte_panic() calls in EAL (and other libraries) - Some functions that call rte_panic() are defined as 'void' return. - will need to modify these functions - which breaks ABI. - If we're doing this it would require deprecation notice in 19.05. - Group discussion on removal of rte panics void returns. - Might need another definition for fatal error codes - Why? The application layer will assess whether something is fatal - Better to return what the error is and let the application take the decision on whether or not it’s a fatal error. - Advice is to use current error codes that is close enough to the error seen. - Use a matching a core id error – we should pass back an int-return and allow the app to decide what action to take. - Thomas says Arnon Warshavsky (ar...@qwilt.com) was working on removing rte panics last year (eal: replace rte_panic instances to return an error value). - Check the list of what APIs need to change and get the list out now to the community in 19.05. - This woudl be a pre-announcement message to the mailing list – this is sent as a patch in 19.05, explaining what will be changed in which files. - Offline discussion with Arnon: see mailing list for detail. - What other changes do we need to do in EAL to make upstreaming easier? - Open source libraries to support common code (BSD licenses). - May need a better wrapper on Linux files to use for windows. - Some of them (eg: pthreads) not supported in windows - so we did macro substitutions to use Windows threads. - Wrappers could be used more widely. - Need to work out how to do this more widely. - Please send your ideas for a solution to the mailing list to seek comments (RFC). - We need to identify the blockers we’re aware of at the moment - Perhaps, TSC is one. - Send interesting topics for the community to work on. - Is the bus/pci code available? - Ask is to move the bus/pci code over to the latest branch (intel). - Intel/Microsoft currently working to tidy up the branches & names on windows draft repo. - Any new code for the tree? - Microsoft is trying to add new code to existing UIO driver - This will be available on windows draft repo (18.08 branch)
[dpdk-dev] [PATCH] net/mlx5: fix flow action destroy
ibv_destroy_flow_action() refers to QP. QP must not be freed until
corresponding action is destroyed.
Fixes: 3eb004431072 ("net/mlx5: fix release of jump to queue action")
Cc: or...@mellanox.com
Signed-off-by: Yongseok Koh
---
drivers/net/mlx5/mlx5_rxq.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 9f27f607b7..0a4c02e71e 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1946,11 +1946,11 @@ int
mlx5_hrxq_release(struct rte_eth_dev *dev, struct mlx5_hrxq *hrxq)
{
if (rte_atomic32_dec_and_test(&hrxq->refcnt)) {
- claim_zero(mlx5_glue->destroy_qp(hrxq->qp));
- mlx5_ind_table_ibv_release(dev, hrxq->ind_table);
#ifdef HAVE_IBV_FLOW_DV_SUPPORT
mlx5_glue->destroy_flow_action(hrxq->action);
#endif
+ claim_zero(mlx5_glue->destroy_qp(hrxq->qp));
+ mlx5_ind_table_ibv_release(dev, hrxq->ind_table);
LIST_REMOVE(hrxq, next);
rte_free(hrxq);
return 0;
@@ -2217,11 +2217,11 @@ mlx5_hrxq_drop_release(struct rte_eth_dev *dev)
struct mlx5_hrxq *hrxq = priv->drop_queue.hrxq;
if (rte_atomic32_dec_and_test(&hrxq->refcnt)) {
- claim_zero(mlx5_glue->destroy_qp(hrxq->qp));
- mlx5_ind_table_ibv_drop_release(dev);
#ifdef HAVE_IBV_FLOW_DV_SUPPORT
mlx5_glue->destroy_flow_action(hrxq->action);
#endif
+ claim_zero(mlx5_glue->destroy_qp(hrxq->qp));
+ mlx5_ind_table_ibv_drop_release(dev);
rte_free(hrxq);
priv->drop_queue.hrxq = NULL;
}
--
2.11.0
Re: [dpdk-dev] [dpdk-stable] [PATCH v2] raw/ifpga_rawdev: fix fd leak in rte fpga do pr
Hi,
I saw Qiang's patch on patchwork today,
[v3] drivers: ifpga_rawdev: fix fd leak in rte_fpga_do_pr
So I think it seems OK to ignore my patch.
-Original Message-
From: Thomas Monjalon [mailto:tho...@monjalon.net]
Sent: Tuesday, April 23, 2019 4:45 PM
To: Pei, Andy
Cc: Xu, Rosen ; sta...@dpdk.org; dev@dpdk.org; Kovacevic,
Marko ; liq...@163.com
Subject: Re: [dpdk-stable] [PATCH v2] raw/ifpga_rawdev: fix fd leak in rte fpga
do pr
When you need to re-send a patch, please apply it with "git am"
so the original author is kept.
Another nit: the title is full of abbreviation, it is hard to read.
The title prefix should be "raw/ifpga", even if checkpatch complains.
Please re-send, thanks.
23/04/2019 03:36, Xu, Rosen:
> Hi Thomas,
>
> Qian has some problem with his email. So some patch can't be found in patch
> work.
> For it's a urgent bug. After we have aligned with Qian, Andy send this patch.
> Thanks.
>
> > -Original Message-
> > From: Pei, Andy
> > Sent: Tuesday, April 23, 2019 9:19
> > To: Thomas Monjalon ; Xu, Rosen
> >
> > Cc: sta...@dpdk.org; dev@dpdk.org; Kovacevic, Marko
> > ; liq...@163.com
> > Subject: RE: [dpdk-stable] [PATCH v2] raw/ifpga_rawdev: fix fd leak
> > in rte fpga do pr
> >
> > Hi Thomas,
> >
> > My patch is the same as LI Qiang's patch.
> > I was told that Qiang's patch cannot get onto the patchwork, so I
> > just help him do this.
> >
> > -Original Message-
> > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > Sent: Tuesday, April 23, 2019 1:52 AM
> > To: Xu, Rosen ; Pei, Andy
> > Cc: sta...@dpdk.org; dev@dpdk.org; Kovacevic, Marko
> > ; liq...@163.com
> > Subject: Re: [dpdk-stable] [PATCH v2] raw/ifpga_rawdev: fix fd leak
> > in rte fpga do pr
> >
> > > > In rte_fpga_do_pr() function, if 'stat' returns error,
> > > > rte_fpga_do_pr() returns -EINVAL without closing the 'file_fd'
> > > > that has been opened.
> > > > After this patch, 'file_fd' is closed before rte_fpga_do_pr()
> > > > returns -EINVAL when 'stat' returns error
> > > >
> > > > Coverity issue: 279441
> > > > Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev
> > > > driver")
> > > > Cc: rosen...@intel.com
> > > > Cc: marko.kovace...@intel.com
> > > > Cc: liq...@163.com
> > > > Cc: andy@intel.com
> > > > Cc: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Andy Pei
> > >
> > > Acked-by: Rosen Xu
> >
> > Li Qiang proposed the same patch.
> > Why he is not referenced here?
> > Which patch should I merge?
[dpdk-dev] [DPDK] changed default-target from linux to linuxapp for back-compatibility of validate-abi.sh
Signed-off-by: Peng Huang
---
devtools/validate-abi.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/devtools/validate-abi.sh b/devtools/validate-abi.sh
index 54df2e4..138436d 100755
--- a/devtools/validate-abi.sh
+++ b/devtools/validate-abi.sh
@@ -9,7 +9,7 @@ set -e
abicheck=abi-compliance-checker
abidump=abi-dumper
default_dst=abi-check
-default_target=x86_64-native-linux-gcc
+default_target=x86_64-native-linuxapp-gcc
# trap on error
err_report() {
--
1.8.3.1
Re: [dpdk-dev] [PATCH] net/ice: fix coverity issues
Hi Ferruh, > -Original Message- > From: Yigit, Ferruh > Sent: Tuesday, April 23, 2019 6:40 PM > To: Lu, Wenzhuo ; dev@dpdk.org > Cc: sta...@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] net/ice: fix coverity issues > > On 4/18/2019 2:31 AM, Wenzhuo Lu wrote: > > Fix the issues reported by Coverity check, "Null-checking vsi suggests > > that it may be null, but it has already been dereferenced on all paths > > leading to the check." > > Hi Wenzhuo, > > Can you please list the coverity issues addressed, the format we have is: > Coverity issue: > > Also the patch title doesn't say much, can you please put what exactly fixed, > "fix possible null pointer dereference" ? Thanks for the comments. Will send V2.
[dpdk-dev] [PATCH] app/testpmd: fix unintentional integer overflow
Fix the potential overflow in expression 1 << begin by using 1ULL.
Coverity issue: 279437, 279445
Fixes: c73a9071877a ("app/testpmd: add commands to test new offload API")
Cc: sta...@dpdk.org
Signed-off-by: Tiwei Bie
---
app/test-pmd/cmdline.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5a10c5f38..03926f913 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -17739,7 +17739,7 @@ print_rx_offloads(uint64_t offloads)
begin = __builtin_ctzll(offloads);
end = sizeof(offloads) * CHAR_BIT - __builtin_clzll(offloads);
- single_offload = 1 << begin;
+ single_offload = 1ULL << begin;
for (bit = begin; bit < end; bit++) {
if (offloads & single_offload)
printf(" %s",
@@ -18133,7 +18133,7 @@ print_tx_offloads(uint64_t offloads)
begin = __builtin_ctzll(offloads);
end = sizeof(offloads) * CHAR_BIT - __builtin_clzll(offloads);
- single_offload = 1 << begin;
+ single_offload = 1ULL << begin;
for (bit = begin; bit < end; bit++) {
if (offloads & single_offload)
printf(" %s",
--
2.17.1
