[dpdk-dev] MinGW guide
The Windows user guide is referencing a general link to download MinGW: http://mingw-w64.org/doku.php/download For those who want to install MinGW on Windows, I thing it is simpler to give this more direct link: http://mingw-w64.org/doku.php/download/mingw-builds It avoids hesitating between different bundles (Cygwin, Msys2, Win-Builds, etc). Then when installing mingw-w64-install.exe, a choice must be done between threads POSIX or Win32. If I understand well, POSIX must be chosen? If it is important, it should be documented in my opinion.
Re: [dpdk-dev] [PATCH 01/27] eventdev: dlb upstream prerequisites
On 2020-06-13 05:59, Jerin Jacob wrote:
> On Sat, Jun 13, 2020 at 2:56 AM McDaniel, Timothy
> wrote:
>> The DLB hardware does not conform exactly to the eventdev interface.
>> 1) It has a limit on the number of queues that may be linked to a port.
>> 2) Some ports a further restricted to a maximum of 1 linked queue.
>> 3) It does not (currently) have the ability to carry the flow_id as part
>> of the event (QE) payload.
>>
>> Due to the above, we would like to propose the following enhancements.
>
> Thanks, McDaniel, Good to see new HW PMD for eventdev.
>
> + Ray and Neil.
>
> Hello McDaniel,
> I assume this patchset is for v20.08. It is adding new elements in
> pubic structures. Have you checked the ABI breakage?
>
> I will review the rest of the series if there is NO ABI breakage as we
> can not have the ABI breakage 20.08 version.
>
>
> ABI validator
> ~~
> 1. meson build
> 2. Compile and install known stable abi libs i.e ToT.
> DESTDIR=$PWD/install-meson-stable ninja -C build install
> Compile and install with patches to be verified.
> DESTDIR=$PWD/install-meson-new ninja -C build install
> 3. Gen ABI for both
> devtools/gen-abi.sh install-meson-stable
> devtools/gen-abi.sh install-meson-new
> 4. Run abi checker
> devtools/check-abi.sh install-meson-stable install-meson-new
>
>
> DPDK_ABI_REF_DIR=/build/dpdk/reference/ DPDK_ABI_REF_VERSION=v20.02
> ./devtools/test-meson-builds.sh
> DPDK_ABI_REF_DIR - needs an absolute path, for reasons that are still
> unclear to me.
> DPDK_ABI_REF_VERSION - you need to use the last DPDK release.
>
>> 1) Add new fields to the rte_event_dev_info struct. These fields allow
>> the device to advertize its capabilities so that applications can take
>> the appropriate actions based on those capabilities.
>>
>> struct rte_event_dev_info {
>> uint32_t max_event_port_links;
>> /**< Maximum number of queues that can be linked to a single event
>> * port by this device.
>> */
>>
>> uint8_t max_single_link_event_port_queue_pairs;
>> /**< Maximum number of event ports and queues that are optimized for
>> * (and only capable of) single-link configurations supported by
>> this
>> * device. These ports and queues are not accounted for in
>> * max_event_ports or max_event_queues.
>> */
>> }
>>
>> 2) Add a new field to the rte_event_dev_config struct. This field allows the
>> application to specify how many of its ports are limited to a single link,
>> or will be used in single link mode.
>>
>> /** Event device configuration structure */
>> struct rte_event_dev_config {
>> uint8_t nb_single_link_event_port_queues;
>> /**< Number of event ports and queues that will be singly-linked to
>> * each other. These are a subset of the overall event ports and
>> * queues; this value cannot exceed *nb_event_ports* or
>> * *nb_event_queues*. If the device has ports and queues that are
>> * optimized for single-link usage, this field is a hint for how
>> many
>> * to allocate; otherwise, regular event ports and queues can be
>> used.
>> */
>> }
>>
>> 3) Replace the dedicated implicit_release_disabled field with a bit field
>> of explicit port capabilities. The implicit_release_disable functionality
>> is assiged to one bit, and a port-is-single-link-only attribute is
>> assigned to other, with the remaining bits available for future assignment.
>>
>> * Event port configuration bitmap flags */
>> #define RTE_EVENT_PORT_CFG_DISABLE_IMPL_REL(1ULL << 0)
>> /**< Configure the port not to release outstanding events in
>> * rte_event_dev_dequeue_burst(). If set, all events received
>> through
>> * the port must be explicitly released with RTE_EVENT_OP_RELEASE or
>> * RTE_EVENT_OP_FORWARD. Must be unset if the device is not
>> * RTE_EVENT_DEV_CAP_IMPLICIT_RELEASE_DISABLE capable.
>> */
>> #define RTE_EVENT_PORT_CFG_SINGLE_LINK (1ULL << 1)
>>
>> /**< This event port links only to a single event queue.
>> *
>> * @see rte_event_port_setup(), rte_event_port_link()
>> */
>>
>> #define RTE_EVENT_PORT_ATTR_IMPLICIT_RELEASE_DISABLE 3
>> /**
>> * The implicit release disable attribute of the port
>> */
>>
>> struct rte_event_port_conf {
>> uint32_t event_port_cfg; /**< Port cfg
>> flags(EVENT_PORT_CFG_) */
>> }
>>
>> 4) Add UMWAIT/UMONITOR bit to rte_cpuflags
>>
>> 5) Added a new API that is useful for probing PCI devices.
>>
>> /**
>> * @internal
>> * Wrapper for use by pci drivers as a .probe function to attach to
>> a event
>> * interface. Same as rte_event_pmd_pci_probe, except caller can
>> speci
[dpdk-dev] [PATCH 1/5] mbuf: fix out-of-bounds access at dyn field register
We should make sure off + size < sizeof(struct rte_mbuf) to avoid
possible out-of-bounds access of free_space array, there is no issue
currently due to the low bits of free_flags (which is adjacent to
free_space) are always set to 0. But we shouldn't rely on it since it's
fragile and layout of struct mbuf_dyn_shm may be changed in the future.
This patch adds boundary check explicitly to avoid potential risk of
out-of-bounds access.
Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
Cc: sta...@dpdk.org
Signed-off-by: Xiaolong Ye
Acked-by: Olivier Matz
---
lib/librte_mbuf/rte_mbuf_dyn.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
index 953e3ec31c..13d6da6d16 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.c
+++ b/lib/librte_mbuf/rte_mbuf_dyn.c
@@ -69,7 +69,8 @@ process_score(void)
for (off = 0; off < sizeof(struct rte_mbuf); off++) {
/* get the size of the free zone */
- for (size = 0; shm->free_space[off + size]; size++)
+ for (size = 0; (off + size) < sizeof(struct rte_mbuf) &&
+shm->free_space[off + size]; size++)
;
if (size == 0)
continue;
--
2.17.1
[dpdk-dev] [PATCH 0/5] small fixes for mbuf's dynamic field/flag feature
This series contains some small fixes and enhancement for mbuf's dynamic field/flag feature. Xiaolong Ye (5): mbuf: fix out-of-bounds access at dyn field register mbuf: fix missing errno for dyn field/flag registration mbuf: fix free_space setting for dynamic field mbuf: fix a dynamic field dump log mbuf: support to dump free_flags for dynamic flag lib/librte_mbuf/rte_mbuf_dyn.c | 34 +++--- 1 file changed, 27 insertions(+), 7 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH 5/5] mbuf: support to dump free_flags for dynamic flag
Add support to dump free_flags as below format:
Free bit in mbuf->ol_flags (0 = occupied, 1 = free):
: 0 0 0 0 0 0 0 0
0008: 0 0 0 0 0 0 0 0
0010: 0 0 0 0 0 0 0 1
0018: 1 1 1 1 1 1 1 1
0020: 1 1 1 1 1 1 1 1
0028: 1 0 0 0 0 0 0 0
0030: 0 0 0 0 0 0 0 0
0038: 0 0 0 0 0 0 0 0
Signed-off-by: Xiaolong Ye
---
lib/librte_mbuf/rte_mbuf_dyn.c | 8
1 file changed, 8 insertions(+)
diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
index f071651acf..538a43f695 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.c
+++ b/lib/librte_mbuf/rte_mbuf_dyn.c
@@ -559,5 +559,13 @@ void rte_mbuf_dyn_dump(FILE *out)
fprintf(out, "%2.2x%s", shm->free_space[i],
(i % 8 != 7) ? " " : "\n");
}
+ fprintf(out, "Free bit in mbuf->ol_flags (0 = occupied, 1 = free):\n");
+ for (i = 0; i < sizeof(uint64_t) * CHAR_BIT; i++) {
+ if ((i % 8) == 0)
+ fprintf(out, " %4.4zx: ", i);
+ fprintf(out, "%1.1x%s", (shm->free_flags & (1ULL << i)) ? 1 : 0,
+ (i % 8 != 7) ? " " : "\n");
+ }
+
rte_mcfg_tailq_write_unlock();
}
--
2.17.1
[dpdk-dev] [PATCH 3/5] mbuf: fix free_space setting for dynamic field
The value free_space[i] is used to save the size of biggest aligned
element that can fit in the zone, current implementation has one flaw,
for example, if user registers dynfield1 (size = 4, align = 4, req = 124)
first, the free_space would be as below after registration:
0070: 08 08 08 08 08 08 08 08
0078: 08 08 08 08 00 00 00 00
Then if user continues to register dynfield2 (size = 4, align = 4),
free_space would become:
0070: 00 00 00 00 04 04 04 04
0078: 04 04 04 04 00 00 00 00
Further request dynfield3 (size = 8, align = 8) would fail to register
due to alignment requirement can't be satisfied, though there is enough
space remained in mbuf.
This patch fixes above issue by saving alignment only in aligned zone,
after the fix, above registrations order can be satisfied, free_space
would be like:
After dynfield1 registration:
0070: 08 08 08 08 08 08 08 08
0078: 04 04 04 04 00 00 00 00
After dynfield2 registration:
0070: 08 08 08 08 08 08 08 08
0078: 00 00 00 00 00 00 00 00
After dynfield3 registration:
0070: 00 00 00 00 00 00 00 00
0078: 00 00 00 00 00 00 00 00
This patch also reduces iterations in process_score() by jumping align
steps in each loop.
Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
Cc: sta...@dpdk.org
Signed-off-by: Xiaolong Ye
---
lib/librte_mbuf/rte_mbuf_dyn.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
index de7d2eb9a5..fd51e1b68e 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.c
+++ b/lib/librte_mbuf/rte_mbuf_dyn.c
@@ -67,13 +67,16 @@ process_score(void)
shm->free_space[i] = 1;
}
- for (off = 0; off < sizeof(struct rte_mbuf); off++) {
+ off = 0;
+ while (off < sizeof(struct rte_mbuf)) {
/* get the size of the free zone */
for (size = 0; (off + size) < sizeof(struct rte_mbuf) &&
shm->free_space[off + size]; size++)
;
- if (size == 0)
+ if (size == 0) {
+ off++;
continue;
+ }
/* get the alignment of biggest object that can fit in
* the zone at this offset.
@@ -84,8 +87,10 @@ process_score(void)
;
/* save it in free_space[] */
- for (i = off; i < off + size; i++)
+ for (i = off; i < off + align; i++)
shm->free_space[i] = RTE_MAX(align, shm->free_space[i]);
+
+ off += align;
}
}
--
2.17.1
[dpdk-dev] [PATCH 2/5] mbuf: fix missing errno for dyn field/flag registration
Set rte_errno as ENOMEM when allocation failure.
Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
Cc: sta...@dpdk.org
Signed-off-by: Xiaolong Ye
---
lib/librte_mbuf/rte_mbuf_dyn.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
index 13d6da6d16..de7d2eb9a5 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.c
+++ b/lib/librte_mbuf/rte_mbuf_dyn.c
@@ -278,12 +278,15 @@ __rte_mbuf_dynfield_register_offset(const struct
rte_mbuf_dynfield *params,
mbuf_dynfield_tailq.head, mbuf_dynfield_list);
te = rte_zmalloc("MBUF_DYNFIELD_TAILQ_ENTRY", sizeof(*te), 0);
- if (te == NULL)
+ if (te == NULL) {
+ rte_errno = ENOMEM;
return -1;
+ }
mbuf_dynfield = rte_zmalloc("mbuf_dynfield", sizeof(*mbuf_dynfield), 0);
if (mbuf_dynfield == NULL) {
rte_free(te);
+ rte_errno = ENOMEM;
return -1;
}
@@ -456,12 +459,15 @@ __rte_mbuf_dynflag_register_bitnum(const struct
rte_mbuf_dynflag *params,
mbuf_dynflag_tailq.head, mbuf_dynflag_list);
te = rte_zmalloc("MBUF_DYNFLAG_TAILQ_ENTRY", sizeof(*te), 0);
- if (te == NULL)
+ if (te == NULL) {
+ rte_errno = ENOMEM;
return -1;
+ }
mbuf_dynflag = rte_zmalloc("mbuf_dynflag", sizeof(*mbuf_dynflag), 0);
if (mbuf_dynflag == NULL) {
rte_free(te);
+ rte_errno = ENOMEM;
return -1;
}
--
2.17.1
[dpdk-dev] [PATCH 4/5] mbuf: fix a dynamic field dump log
For each mbuf byte, free_space[i] == 0 means the space is occupied,
free_space[i] != 0 means space is free.
Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
Cc: sta...@dpdk.org
Signed-off-by: Xiaolong Ye
---
lib/librte_mbuf/rte_mbuf_dyn.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
index fd51e1b68e..f071651acf 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.c
+++ b/lib/librte_mbuf/rte_mbuf_dyn.c
@@ -552,7 +552,7 @@ void rte_mbuf_dyn_dump(FILE *out)
dynflag->params.name, dynflag->bitnum,
dynflag->params.flags);
}
- fprintf(out, "Free space in mbuf (0 = free, value = zone
alignment):\n");
+ fprintf(out, "Free space in mbuf (0 = occupied, value = free zone
alignment):\n");
for (i = 0; i < sizeof(struct rte_mbuf); i++) {
if ((i % 8) == 0)
fprintf(out, " %4.4zx: ", i);
--
2.17.1
Re: [dpdk-dev] [PATCH 9/9] vhost: only use vDPA config workaround if needed
Hi Maxime
From: Maxime Coquelin:
> On 6/9/20 1:09 PM, Matan Azrad wrote:
> > Hi Maxime
> >
> > From: Maxime Coquelin
> >> Hi Matan,
> >>
> >> On 6/8/20 11:19 AM, Matan Azrad wrote:
> >>> Hi Maxime
> >>>
> >>> From: Maxime Coquelin:
> Hi Matan,
>
> On 6/7/20 12:38 PM, Matan Azrad wrote:
> > Hi Maxime
> >
> > Thanks for the huge work.
> > Please see a suggestion inline.
> >
> > From: Maxime Coquelin:
> >> Sent: Thursday, May 14, 2020 11:02 AM
> >> To: xiaolong...@intel.com; Shahaf Shuler
> ;
> >> Matan Azrad ; amore...@redhat.com;
> >> xiao.w.w...@intel.com; Slava Ovsiienko
> >> ;
> >> dev@dpdk.org
> >> Cc: jasow...@redhat.com; l...@redhat.com; Maxime Coquelin
> >>
> >> Subject: [PATCH 9/9] vhost: only use vDPA config workaround if
> >> needed
> >>
> >> Now that we have Virtio device status support, let's only use the
> >> vDPA workaround if it is not supported.
> >>
> >> This patch also document why Virtio device status protocol
> >> feature support is strongly advised.
> >>
> >> Signed-off-by: Maxime Coquelin
> >> ---
> >> lib/librte_vhost/vhost_user.c | 16 ++--
> >> 1 file changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/librte_vhost/vhost_user.c
> >> b/lib/librte_vhost/vhost_user.c index e5a44be58d..67e96a872a
> >> 100644
> >> --- a/lib/librte_vhost/vhost_user.c
> >> +++ b/lib/librte_vhost/vhost_user.c
> >> @@ -2847,8 +2847,20 @@ vhost_user_msg_handler(int vid, int fd)
> >>if (!vdpa_dev)
> >>goto out;
> >>
> >> - if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED) &&
> >> - request == VHOST_USER_SET_VRING_CALL)
> {
> >> + if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
> >> + /*
> >> + * Workaround when Virtio device status protocol
> >> + * feature is not supported, wait for
> SET_VRING_CALL
> >> + * request. This is not ideal as some frontends like
> >> + * Virtio-user may not send this request, so vDPA
> device
> >> + * may never be configured. Virtio device status
> support
> >> + * on frontend side is strongly advised.
> >> + */
> >> + if (!(dev->protocol_features &
> >> + (1ULL <<
> >> VHOST_USER_PROTOCOL_F_STATUS)) &&
> >> + (request !=
> >> VHOST_USER_SET_VRING_CALL))
> >> + goto out;
> >> +
> >
> > When status protocol feature is not supported, in the current
> > code, the
> vDPA configuration triggering depends in:
> > 1. Device is ready - all the queues are configured (datapath
> > addresses,
> callfd and kickfd) .
> > 2. last command is callfd.
> >
> >
> > The code doesn't take into account that some queues may stay
> disabled.
> > Maybe the correct timing is:
> > 1. Device is ready - all the enabled queues are configured and MEM
> > table is
> configured.
>
> I think current virtio_is_ready() already assumes the mem table is
> configured, otherwise we would not have vq->desc, vq->used and
> vq->avail being set as it needs to be translated using the mem table.
>
> >>> Yes, but if you don't expect to check them for disabled queues you
> >>> need to
> >> check mem table to be sure it was set.
> >>
> >> Even disabled queues should be allocated/configured by the guest driver.
> > Is it by spec?
>
> Sorry, that was a misunderstanding from my side.
> The number of queues set by the driver using MQ_VQ_PAIRS_SET control
> message have to be allocated and configured by the driver:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdocs.o
> asis-open.org%2Fvirtio%2Fvirtio%2Fv1.0%2Fcs04%2Fvirtio-v1.0-
> cs04.html%23x1-
> 1940001&data=02%7C01%7Cmatan%40mellanox.com%7Cbed5d361fbff
> 47ab766008d80c99cc53%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%
> 7C637273201984684513&sdata=zbBLclza39Fi5QenFtRx%2F1T29Dgj4w%2
> FudJ6obp5RxYo%3D&reserved=0
>
Do you mean to the sentence:
"The driver MUST configure the virtqueues before enabling them with the
VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command." ?
Maybe I miss English understanding here but it looks like this sentence doesn't
say if the driver should do configuration for queues that will not be enabled
by the virtio driver (stay disabled forever).
> > We saw that windows virtio guest driver doesn't configure disabled
> queues.
> > Is it bug in windows guest?
> > You probably can take a look here:
> > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Fvirtio-win%2Fkvm-guest-drivers-
> windows&data=02%7C01%7Cmat
> >
> an%40mellanox.com%7Cbed5d361fbff47ab766008d80c99cc53%7Ca652971c7d
> 2e4d9
> >
> ba6a4d149256f461b%7
