Re: [dpdk-dev] [PATCH v2 1/3] eal: introduce structure marker typedefs
Hi From: Jerin Jacob > From: Jerin Jacob > > Introduce EAL typedef for structure 1B, 2B, 4B, 8B alignment marking and a > generic marker for a point in a structure. > > Signed-off-by: Jerin Jacob > --- > > v2: > - Changed __extension__ to RTE_STD_C11 (Thomas) > - Change "a point" to "any place" of RTE_MARKER comment(Thomas) > > lib/librte_eal/common/include/rte_common.h | 12 > 1 file changed, 12 insertions(+) > > diff --git a/lib/librte_eal/common/include/rte_common.h > b/lib/librte_eal/common/include/rte_common.h > index 459d082d1..00c8b8434 100644 > --- a/lib/librte_eal/common/include/rte_common.h > +++ b/lib/librte_eal/common/include/rte_common.h > @@ -335,6 +335,18 @@ typedef uint64_t phys_addr_t; typedef uint64_t > rte_iova_t; #define RTE_BAD_IOVA ((rte_iova_t)-1) > > +/*** Structure alignment markers / > + > +/** Generic marker for any place in a structure. */ > +RTE_STD_C11 typedef void*RTE_MARKER[0]; > +/** Marker for 1B alignment in a structure. */ > +RTE_STD_C11 typedef uint8_t RTE_MARKER8[0]; > +/** Marker for 2B alignment in a structure. */ > +RTE_STD_C11 typedef uint16_t RTE_MARKER16[0]; > +/** Marker for 4B alignment in a structure. */ > +RTE_STD_C11 typedef uint16_t RTE_MARKER32[0]; > +/** Marker for 8B alignment in a structure. */ > +RTE_STD_C11 typedef uint64_t RTE_MARKER64[0]; I saw there are similar definitions in mbuf library. What do you think about moving all libraries to use the same define from EAL? > /** > * Combines 32b inputs most significant set bits into the least > -- > 2.24.1
Re: [dpdk-dev] [PATCH v4 2/5] net/i40e: cleanup Tx buffers
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Chenxu Di
> Sent: Tuesday, December 24, 2019 10:39 AM
> To: dev@dpdk.org
> Cc: Yang, Qiming ; Di, ChenxuX
>
> Subject: [dpdk-dev] [PATCH v4 2/5] net/i40e: cleanup Tx buffers
>
> Add support to the i40e driver for the API rte_eth_tx_done_cleanup to force
> free consumed buffers on Tx ring.
>
> Signed-off-by: Chenxu Di
> ---
> drivers/net/i40e/i40e_ethdev.c| 1 +
> drivers/net/i40e/i40e_ethdev_vf.c | 1 +
> drivers/net/i40e/i40e_rxtx.c | 122 ++
> drivers/net/i40e/i40e_rxtx.h | 1 +
> 4 files changed, 125 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5999c964b..fad47a942 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -522,6 +522,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
> .mac_addr_set = i40e_set_default_mac_addr,
> .mtu_set = i40e_dev_mtu_set,
> .tm_ops_get = i40e_tm_ops_get,
> + .tx_done_cleanup = i40e_tx_done_cleanup,
> };
>
> /* store statistics names and its offset in stats structure */ diff --git
> a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index 5dba0928b..0ca5417d7 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -215,6 +215,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
> .rss_hash_conf_get= i40evf_dev_rss_hash_conf_get,
> .mtu_set = i40evf_dev_mtu_set,
> .mac_addr_set = i40evf_set_default_mac_addr,
> + .tx_done_cleanup = i40e_tx_done_cleanup,
> };
>
> /*
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index
> 17dc8c78f..e75733b8e 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -2455,6 +2455,128 @@ i40e_tx_queue_release_mbufs(struct
> i40e_tx_queue *txq)
> }
> }
>
> +int i40e_tx_done_cleanup(void *q, uint32_t free_cnt) {
> + struct i40e_tx_queue *txq = (struct i40e_tx_queue *)q;
> + struct i40e_tx_entry *sw_ring;
> + volatile struct i40e_tx_desc *txr;
> + uint16_t tx_first; /* First segment analyzed. */
> + uint16_t tx_id;/* Current segment being processed. */
> + uint16_t tx_last; /* Last segment in the current packet. */
> + uint16_t tx_next; /* First segment of the next packet. */
> + int count;
> +
> + if (txq == NULL)
> + return -ENODEV;
> +
> + count = 0;
> + sw_ring = txq->sw_ring;
> + txr = txq->tx_ring;
> +
> + /*
> + * tx_tail is the last sent packet on the sw_ring. Goto the end
> + * of that packet (the last segment in the packet chain) and
> + * then the next segment will be the start of the oldest segment
> + * in the sw_ring. This is the first packet that will be
> + * attempted to be freed.
> + */
> +
> + /* Get last segment in most recently added packet. */
> + tx_first = sw_ring[txq->tx_tail].last_id;
Should be tx_last more readable here? And then tx_first =
sw_ring[tx_last].next_id?
> +
> + /* Get the next segment, which is the oldest segment in ring. */
> + tx_first = sw_ring[tx_first].next_id;
> +
> + /* Set the current index to the first. */
> + tx_id = tx_first;
> +
> + /*
> + * Loop through each packet. For each packet, verify that an
> + * mbuf exists and that the last segment is free. If so, free
> + * it and move on.
> + */
> + while (1) {
> + tx_last = sw_ring[tx_id].last_id;
> +
> + if (sw_ring[tx_last].mbuf) {
> + if ((txr[tx_last].cmd_type_offset_bsz &
> +
> rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) !=
Is the logic reversed? '!=' means the mbuf is still in use.
> +
> rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE)) {
> + /*
> + * Increment the number of packets
> + * freed.
> + */
> + count++;
Should 'count++' be after free mbuf?
> +
> + /* Get the start of the next packet. */
> + tx_next = sw_ring[tx_last].next_id;
> +
> + /*
> + * Loop through all segments in a
> + * packet.
> + */
> + do {
> +
> rte_pktmbuf_free_seg(sw_ring[tx_id].mbuf);
> + sw_ring[tx_id].mbuf = NULL;
> + sw_ring[tx_id].last_id = tx_id;
> +
> + /* Move to next segemnt. */
Typo: segemnt -> segment
> + tx_id = sw_ring[tx_id].next_id;
> +
> +
Re: [dpdk-dev] [RFC] mempool: introduce indexed memory pool
Hi Xueming,
On Thu, Oct 17, 2019 at 06:55:01AM +, Xueming Li wrote:
> Indexed memory pool manages memory entries by index, allocation from
> pool returns both memory pointer and index(ID). users save ID as u32
> or less(u16) instead of traditional 8 bytes pointer. Memory could be
> retrieved from pool or returned to pool later by index.
>
> Pool allocates backend memory in chunk on demand, pool size grows
> dynamically. Bitmap is used to track entry usage in chunk, thus
> management overhead is one bit per entry.
>
> Standard rte_malloc demands malloc overhead(64B) and minimal data
> size(64B). This pool aims to such cost saving also pointer size.
> For scenario like creating millions of rte_flows each consists
> of small pieces of memories, the difference is huge.
>
> Like standard memory pool, this lightweight pool only support fixed
> size memory allocation. Pools should be created for each different
> size.
>
> To facilitate memory allocated by index, a set of ILIST_XXX macro
> defined to operate entries as regular LIST.
>
> By setting entry size to zero, pool can be used as ID generator.
>
> Signed-off-by: Xueming Li
I think some inputs are missing about the use case and what exact
problem you are trying to solve. Where do you plan to use it in dpdk?
My understanding of your needs are:
- An allocator for objects of similar size (i.e. a pool)
- Avoid wasted memory when allocating many small objects
- Identify objects by index instead of pointer (why?)
- Dynamically growable, i.e. add more objects to the pool on demand
- Quick conversion from id to pointer
- Alloc/free does not need to be fast
Is it correct? Anything else?
What prevents you from using a rte_mempool? It should support dynamic
growing (but I don't think it has ever been tested). The missing feature
is the conversion between id and pointer, but it looks quite easy to me.
The reverse conversion (pointer to id) would be more tricky.
Did I miss a requirement?
It looks that the indexed_pool allocator is not designed for fast
allocation/free, right? I don't see any bulk-based functions. Also, as
it is now, I don't really see the link with rte_mempool (except it is a
pool), and I suggest that it could be a separated library instead.
An example of use would be welcome.
Another general comment is about the naming convention. Public
functions, structures, defines should have the same form, as much as
possible: "rte__xxx".
Some more comments below.
> ---
> lib/librte_mempool/Makefile| 3 +-
> lib/librte_mempool/rte_indexed_pool.c | 289 +
> lib/librte_mempool/rte_indexed_pool.h | 224
> lib/librte_mempool/rte_mempool_version.map | 7 +-
> 4 files changed, 521 insertions(+), 2 deletions(-)
> create mode 100644 lib/librte_mempool/rte_indexed_pool.c
> create mode 100644 lib/librte_mempool/rte_indexed_pool.h
>
> diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
> index 20bf63fbca..343e945336 100644
> --- a/lib/librte_mempool/Makefile
> +++ b/lib/librte_mempool/Makefile
> @@ -21,7 +21,8 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
> SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool.c
> SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_ops.c
> SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_ops_default.c
> +SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_indexed_pool.c
> # install includes
> -SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
> rte_indexed_pool.h
>
> include $(RTE_SDK)/mk/rte.lib.mk
meson.build support is missing.
> diff --git a/lib/librte_mempool/rte_indexed_pool.c
> b/lib/librte_mempool/rte_indexed_pool.c
> new file mode 100644
> index 00..b9069a6555
> --- /dev/null
> +++ b/lib/librte_mempool/rte_indexed_pool.c
> @@ -0,0 +1,289 @@
Copyright/license is missing.
> +#include "rte_indexed_pool.h"
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +
> +/* return 1 if bitmap empty after clear. */
> +static int
> +map_clear_any(uint64_t *map, int n, uint32_t *idx)
> +{
> + int row, col;
> +
> + *idx = UINT32_MAX;
> + /* Locate free entry in trunk */
> + for (row = 0; row < n / 64; row++) {
> + if (*idx != UINT32_MAX && map[row])
> + return 0;
> + if (!map[row])
> + continue;
> + col = __builtin_ctzll(map[row]);
> + *idx = row * 64 + col;
> + map[row] &= ~(1LU << col);
> + if (map[row])
> + return 0;
> + }
> + return 1;
> +}
>From what I see, map_clear_any() finds a bit set to 1 in the bitmap, set
it to 0, then returns 1 if the bitmap is all 0. For me, the name of the
function does not reflect what is done. What about take_bit() or
get_bit()?.
"int n" could be "size_t len", and 64 should be UINT64_BIT, to
differentiate cases where 64 is an arbitrary val
[dpdk-dev] [PATCH] mbuf: document how to set length when attaching ext buffer
From: Jörg Thalheim Enhance API documentation of rte_pktmbuf_attach_extbuf() to explain that the attached mbuf is initialized with length = 0. Link: https://bugs.dpdk.org/show_bug.cgi?id=362 Signed-off-by: Jörg Thalheim Signed-off-by: Olivier Matz --- Hi, This patch is a slight reword of a patch submitted in bug id 362 by Jörg Thalheim. @Jörg, feel free to comment if I missed something. Thanks Olivier lib/librte_mbuf/rte_mbuf.h | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 219b110b7..2d4bda251 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -924,10 +924,14 @@ rte_pktmbuf_ext_shinfo_init_helper(void *buf_addr, uint16_t *buf_len, * provided via shinfo. This callback function will be called once all the * mbufs are detached from the buffer (refcnt becomes zero). * - * The headroom for the attaching mbuf will be set to zero and this can be - * properly adjusted after attachment. For example, ``rte_pktmbuf_adj()`` + * The headroom length of the attaching mbuf will be set to zero and this + * can be properly adjusted after attachment. For example, ``rte_pktmbuf_adj()`` * or ``rte_pktmbuf_reset_headroom()`` might be used. * + * Similarly, the packet length is initialized to 0. If the buffer contains + * data, the user has to adjust ``data_len`` and the ``pkt_len`` field of + * the mbuf accordingly. + * * More mbufs can be attached to the same external buffer by * ``rte_pktmbuf_attach()`` once the external buffer has been attached by * this API. -- 2.20.1
Re: [dpdk-dev] 18.11.6 (LTS) patches review and test
Hi, > -Original Message- > From: Kevin Traynor > Sent: Wednesday, December 18, 2019 1:42 PM > To: sta...@dpdk.org > Cc: dev@dpdk.org; Abhishek Marathe ; > Akhil Goyal ; Ali Alnubani ; > benjamin.wal...@intel.com; David Christensen ; > Hemant Agrawal ; Ian Stokes > ; Jerin Jacob ; John McNamara > ; Ju-Hyoung Lee ; > Kevin Traynor ; Luca Boccassi ; > Pei Zhang ; pingx...@intel.com; > qian.q...@intel.com; Raslan Darawsheh ; Thomas > Monjalon ; yuan.p...@intel.com; > zhaoyan.c...@intel.com > Subject: 18.11.6 (LTS) patches review and test > > Hi all, > > Here is a list of patches targeted for LTS release 18.11.6. > > The planned date for the final release is 31st January. > > Please help with testing and validation of your use cases and report any > issues/results with reply-all to this mail. For the final release the fixes > and > reported validations will be added to the release notes. > The following covers the tests that we ran on Mellanox hardware for this version: - Verify sending and receiving multiple types of traffic. - testpmd xstats counter tests. - testpmd timestamp tests. - Changing/checking link status through testpmd. - RTE flow and flow_director tests. - Some RSS tests. - VLAN stripping and insertion tests. - Checksum and TSO tests. - ptype tests. - Multi-process tests. The tests ran on (OS: RHEL7.4 | Kernel: Linux 3.10.0-693.el7.x86_64 | Driver: MLNX_OFED_LINUX-4.7-3.2.9.0). NICs tested: - ConnectX-5 | Firmware: 16.26.4012 - ConnectX-4 Lx | Firmware: 14.26.4012 Regards, Ali
Re: [dpdk-dev] Inconsistent behavior of mempool with regards to hugepage allocation
Hi Bao-Long,
On Mon, Dec 23, 2019 at 07:09:29PM +0800, Bao-Long Tran wrote:
> Hi,
>
> I'm not sure if this is a bug, but I've seen an inconsistency in the behavior
> of DPDK with regards to hugepage allocation for rte_mempool. Basically, for
> the
> same mempool size, the number of hugepages allocated changes from run to run.
>
> Here's how I reproduce with DPDK 19.11. IOVA=pa (default)
>
> 1. Reserve 16x1G hugepages on socket 0
> 2. Replace examples/skeleton/basicfwd.c with the code below, build and run
> make && ./build/basicfwd
> 3. At the same time, watch the number of hugepages allocated
> "watch -n.1 ls /dev/hugepages"
> 4. Repeat step 2
>
> If you can reproduce, you should see that for some runs, DPDK allocates 5
> hugepages, other times it allocates 6. When it allocates 6, if you watch the
> output from step 3., you will see that DPDK first try to allocate 5
> hugepages,
> then unmap all 5, retry, and got 6.
I cannot reproduce in the same conditions than yours (with 16 hugepages
on socket 0), but I think I can see a similar issue:
If I reserve at least 6 hugepages, it seems reproducible (6 hugepages
are used). If I reserve 5 hugepages, it takes more time,
taking/releasing hugepages several times, and it finally succeeds with 5
hugepages.
> For our use case, it's important that DPDK allocate the same number of
> hugepages on every run so we can get reproducable results.
One possibility is to use the --legacy-mem EAL option. It will try to
reserve all hugepages first.
> Studying the code, this seems to be the behavior of
> rte_mempool_populate_default(). If I understand correctly, if the first try
> fail
> to get 5 IOVA-contiguous pages, it retries, relaxing the IOVA-contiguous
> condition, and eventually wound up with 6 hugepages.
No, I think you don't have the IOVA-contiguous constraint in your
case. This is what I see:
a- reserve 5 hugepages on socket 0, and start your patched basicfwd
b- it tries to allocate 2097151 objects of size 2304, pg_size = 1073741824
c- the total element size (with header) is 2304 + 64 = 2368
d- in rte_mempool_op_calc_mem_size_helper(), it calculates
obj_per_page = 453438(453438 * 2368 = 1073741184)
mem_size = 4966058495
e- it tries to allocate 4966058495 bytes, which is less than 5 x 1G, with:
rte_memzone_reserve_aligned(name, size=4966058495, socket=0,
mz_flags=RTE_MEMZONE_1GB|RTE_MEMZONE_SIZE_HINT_ONLY,
align=64)
For some reason, it fails: we can see that the number of map'd hugepages
increases in /dev/hugepages, the return to its original value.
I don't think it should fail here.
f- then, it will try to allocate the biggest available contiguous zone. In
my case, it is 1055291776 bytes (almost all the uniq map'd hugepage).
This is a second problem: if we call it again, it returns NULL, because
it won't map another hugepage.
g- by luck, calling rte_mempool_populate_virt() allocates a small aera
(mempool header), and it triggers the mapping a a new hugepage, that
will be used in the next loop, back at step d with a smaller mem_size.
> Questions:
> 1. Why does the API sometimes fail to get IOVA contig mem, when hugepage
> memory
> is abundant?
In my case, it looks that we have a bit less than 1G which is free at
the end of the heap, than we call rte_memzone_reserve_aligned(size=5G).
The allocator ends up in mapping 5 pages (and fail), while only 4 is
needed.
Anatoly, do you have any idea? Shouldn't we take in account the amount
of free space at the end of the heap when expanding?
> 2. Why does the 2nd retry need N+1 hugepages?
When the first alloc fails, the mempool code tries to allocate in
several chunks which are not virtually contiguous. This is needed in
case the memory is fragmented.
> Some insights for Q1: From my experiments, seems like the IOVA of the first
> hugepage is not guaranteed to be at the start of the IOVA space
> (understandably).
> It could explain the retry when the IOVA of the first hugepage is near the
> end of
> the IOVA space. But I have also seen situation where the 1st hugepage is near
> the beginning of the IOVA space and it still failed the 1st time.
>
> Here's the code:
> #include
> #include
>
> int
> main(int argc, char *argv[])
> {
> struct rte_mempool *mbuf_pool;
> unsigned mbuf_pool_size = 2097151;
>
> int ret = rte_eal_init(argc, argv);
> if (ret < 0)
> rte_exit(EXIT_FAILURE, "Error with EAL initialization\n");
>
> printf("Creating mbuf pool size=%u\n", mbuf_pool_size);
> mbuf_pool = rte_pktmbuf_pool_create("MBUF_POOL", mbuf_pool_size,
> 256, 0, RTE_MBUF_DEFAULT_BUF_SIZE, 0);
>
> printf("mbuf_pool %p\n", mbuf_pool);
>
> return 0;
> }
>
> Best regards,
> BL
Regards,
Olivier
Re: [dpdk-dev] [PATCH v2] mbuf: display more fields in dump
Hi,
On Thu, Nov 21, 2019 at 10:30:55AM -0800, Stephen Hemminger wrote:
> The rte_pktmbuf_dump should display offset, refcount, and vlan
> info since these are often useful during debugging.
>
> Signed-off-by: Stephen Hemminger
> ---
> v2 - remove casts, change in_port to port
> the refcount and offset are property of per-segment
>
> lib/librte_mbuf/rte_mbuf.c | 17 ++---
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 35df1c4c38a5..4894d46628e3 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -473,18 +473,21 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m,
> unsigned dump_len)
>
> __rte_mbuf_sanity_check(m, 1);
>
> - fprintf(f, "dump mbuf at %p, iova=%"PRIx64", buf_len=%u\n",
> -m, (uint64_t)m->buf_iova, (unsigned)m->buf_len);
> - fprintf(f, " pkt_len=%"PRIu32", ol_flags=%"PRIx64", nb_segs=%u, "
> -"in_port=%u\n", m->pkt_len, m->ol_flags,
> -(unsigned)m->nb_segs, (unsigned)m->port);
> + fprintf(f, "dump mbuf at %p, iova=%#"PRIx64", buf_len=%u\n",
> + m, m->buf_iova, m->buf_len);
> + fprintf(f,
> + " pkt_len=%u, ol_flags=%#"PRIx64", nb_segs=%u, port=%u,
> vlan_tci=%#x\n",
> + m->pkt_len, m->ol_flags, m->nb_segs, m->port, m->vlan_tci);
> +
> nb_segs = m->nb_segs;
>
> while (m && nb_segs != 0) {
> __rte_mbuf_sanity_check(m, 0);
>
> - fprintf(f, " segment at %p, data=%p, data_len=%u\n",
> - m, rte_pktmbuf_mtod(m, void *), (unsigned)m->data_len);
> + fprintf(f, " segment at %p, data=%p, len=%u, off=%u,
> refcnt=%u\n",
> + m, rte_pktmbuf_mtod(m, void *),
> + m->data_len, m->data_off, rte_mbuf_refcnt_read(m));
> +
> len = dump_len;
> if (len > m->data_len)
> len = m->data_len;
> --
> 2.20.1
>
Thanks for this enhancement.
One comment however: I don't see why vlan_tci would be important than
packet type or rss tag. It is also a bit confusing to display a field
which can have a random value (if the vlan flag is not set in ol_flags).
I'd prefer to dump vlan_tci only if the flag is present. Later we could
add more fields with the same logic. What do you think?
Olivier
Re: [dpdk-dev] [PATCH] lib: fix unnecessary boolean casts
On Mon, Dec 02, 2019 at 09:15:17AM +, Ciara Power wrote:
> The values already boolean types, so the use of !! is unnecessary as
> it is used to convert to boolean.
>
> Fixes: ea672a8b1655 ("mbuf: remove the rte_pktmbuf structure")
> Fixes: a0fd91cefcc0 ("mempool: rename functions with confusing names")
> Cc: olivier.m...@6wind.com
> Cc: bruce.richard...@intel.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ciara Power
Acked-by: Olivier Matz
Thanks
Re: [dpdk-dev] [PATCH v2 3/3] mbuf: use structure marker typedef in eal
On Thu, Dec 19, 2019 at 04:55:07PM +0530, jer...@marvell.com wrote: > From: Jerin Jacob > > Use new marker typedef available in EAL and remove private marker > typedef. > > Signed-off-by: Jerin Jacob Acked-by: Olivier Matz Thanks
Re: [dpdk-dev] [PATCH v2] mbuf: display more fields in dump
On Thu, 26 Dec 2019 17:15:39 +0100
Olivier Matz wrote:
> Hi,
>
> On Thu, Nov 21, 2019 at 10:30:55AM -0800, Stephen Hemminger wrote:
> > The rte_pktmbuf_dump should display offset, refcount, and vlan
> > info since these are often useful during debugging.
> >
> > Signed-off-by: Stephen Hemminger
> > ---
> > v2 - remove casts, change in_port to port
> > the refcount and offset are property of per-segment
> >
> > lib/librte_mbuf/rte_mbuf.c | 17 ++---
> > 1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> > index 35df1c4c38a5..4894d46628e3 100644
> > --- a/lib/librte_mbuf/rte_mbuf.c
> > +++ b/lib/librte_mbuf/rte_mbuf.c
> > @@ -473,18 +473,21 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m,
> > unsigned dump_len)
> >
> > __rte_mbuf_sanity_check(m, 1);
> >
> > - fprintf(f, "dump mbuf at %p, iova=%"PRIx64", buf_len=%u\n",
> > - m, (uint64_t)m->buf_iova, (unsigned)m->buf_len);
> > - fprintf(f, " pkt_len=%"PRIu32", ol_flags=%"PRIx64", nb_segs=%u, "
> > - "in_port=%u\n", m->pkt_len, m->ol_flags,
> > - (unsigned)m->nb_segs, (unsigned)m->port);
> > + fprintf(f, "dump mbuf at %p, iova=%#"PRIx64", buf_len=%u\n",
> > + m, m->buf_iova, m->buf_len);
> > + fprintf(f,
> > + " pkt_len=%u, ol_flags=%#"PRIx64", nb_segs=%u, port=%u,
> > vlan_tci=%#x\n",
> > + m->pkt_len, m->ol_flags, m->nb_segs, m->port, m->vlan_tci);
> > +
> > nb_segs = m->nb_segs;
> >
> > while (m && nb_segs != 0) {
> > __rte_mbuf_sanity_check(m, 0);
> >
> > - fprintf(f, " segment at %p, data=%p, data_len=%u\n",
> > - m, rte_pktmbuf_mtod(m, void *), (unsigned)m->data_len);
> > + fprintf(f, " segment at %p, data=%p, len=%u, off=%u,
> > refcnt=%u\n",
> > + m, rte_pktmbuf_mtod(m, void *),
> > + m->data_len, m->data_off, rte_mbuf_refcnt_read(m));
> > +
> > len = dump_len;
> > if (len > m->data_len)
> > len = m->data_len;
> > --
> > 2.20.1
> >
>
> Thanks for this enhancement.
>
> One comment however: I don't see why vlan_tci would be important than
> packet type or rss tag. It is also a bit confusing to display a field
> which can have a random value (if the vlan flag is not set in ol_flags).
>
> I'd prefer to dump vlan_tci only if the flag is present. Later we could
> add more fields with the same logic. What do you think?
Because this is a debugging hook and easier to always display the value.
Thats all.
[dpdk-dev] DPDK20.02 Mellanox Roadmap
This is Mellanox's roadmap for DPDK20.02, which we are working on currently. Enable zero copy to/from GPU, Storage-device etc. * Enable a pinned external buffer with pktmbuf pool: * Introducing a new flag on the rte_pktmbuf_pool_private to specify this mempool is for mbuf with a pinned external buffer. * This will enable a GPU or a storage device to do zero-copy for the received frames. Preserve DSCP field for IPv4/IPv6 decapsulation * Introduce a new rte_flow API to set DSCP field for IPv4 and IPv6 during decapsulation In case of an overlay network, when doing decapsulation, the DSCP field may need to be updated accordingly to preserve the IP Precedence Additions to mlx5 PMD (ConnectX-5 SmartNIC, BlueField IPU and above): * Support multiple header modifications in a single flow rule With this, a single flow can have several IPv6 header modification actions * HW offload for a finer granularity of RSS only on the source or only on the destination, for both L3 and L4 For example, a GW applications where two sides of the flows will be handled by the same core * HW offload for matching on a GTP-U header, specifically on the msg_type and teid fields With this, a classification for a 4G/5G barrier can be done * Support PMD hint not to inline packet This is in order to support a mixed traffic pattern, where some buffers are from the local host memory and others from other devices. Reduce memory consumption in mlx5 PMD * Change the implementation of rte_eth_dev_stop()/rte_eth_dev_start() which currently caches rules, to a non-cached implementation freeing all software and hardware resources for the created flows. Support the full feature-set of ConnectX-5, including full functionality of hw offloads and performance, in ConnectX-6 Dx Behavior change on rte_flow encap/decap actions * ECN field will always be copied from the inner frame to the outer frame on enacap, and vise-versa on decap This is important to easily support congestion control algorithms that validate the ECN bit. One example is RoCE congestion control. Introducing a new mlx5 PMD for vDPA (ConnectX-6 Dx, BlueField IPU and above): * Adding a new mlx5 PMD to support vHost Data Path Acceleration (vDPA) - mlx5_vdpa * mlx5_vdpa can run on top of PCI devices - VFs or PF * According to the PCI device devargs, specified by the user, the driver's probe function will choose either mlx5 or mlx5_vdpa
Re: [dpdk-dev] [PATCH v3] net/i40e: fix TSO pkt exceeds allowed buf size issue
> -Original Message-
> From: Li, Xiaoyun
> Sent: Thursday, December 26, 2019 2:46 PM
> To: Zhang, Qi Z ; Xing, Beilei ;
> Loftus, Ciara ; dev@dpdk.org
> Cc: Li, Xiaoyun ; sta...@dpdk.org
> Subject: [PATCH v3] net/i40e: fix TSO pkt exceeds allowed buf size issue
>
> Hardware limits that max buffer size per tx descriptor should be (16K-1)B. So
> when TSO enabled, the mbuf data size may exceed the limit and cause
> malicious behavior to the NIC. This patch fixes this issue by using more tx
> descs
> for this kind of large buffer.
>
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiaoyun Li
Acked-by: Qi Zhang
Re: [dpdk-dev] [PATCH] net/ice: fix TSO pkt exceeds allowed buf size issue
> -Original Message-
> From: Li, Xiaoyun
> Sent: Thursday, December 26, 2019 2:43 PM
> To: Zhang, Qi Z ; Xing, Beilei ;
> Yang, Qiming ; Lu, Wenzhuo
> ; Loftus, Ciara ;
> dev@dpdk.org
> Cc: Li, Xiaoyun ; sta...@dpdk.org
> Subject: [PATCH] net/ice: fix TSO pkt exceeds allowed buf size issue
>
> Hardware limits that max buffer size per tx descriptor should be (16K-1)B. So
> when TSO enabled, the mbuf data size may exceed the limit and cause
> malicious behavior to the NIC. This patch fixes this issue by using more tx
> descs
> for this kind of large buffer.
>
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiaoyun Li
Acked-by: Qi Zhang
Re: [dpdk-dev] [PATCH v2] net/ice: fix TSO pkt exceeds allowed buf size issue
> -Original Message-
> From: Li, Xiaoyun
> Sent: Thursday, December 26, 2019 2:54 PM
> To: Zhang, Qi Z ; Xing, Beilei ;
> Yang, Qiming ; Lu, Wenzhuo
> ; Loftus, Ciara ;
> dev@dpdk.org
> Cc: Li, Xiaoyun ; sta...@dpdk.org
> Subject: [PATCH v2] net/ice: fix TSO pkt exceeds allowed buf size issue
>
> Hardware limits that max buffer size per tx descriptor should be (16K-1)B. So
> when TSO enabled, the mbuf data size may exceed the limit and cause
> malicious behavior to the NIC. This patch fixes this issue by using more tx
> descs
> for this kind of large buffer.
>
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiaoyun Li
Acked-by: Qi Zhang
[dpdk-dev] pci device access from dpdk secondary process with igb_uio
Hi All, I am trying to use pci vf device with secondary process in a multiple process mode and finding pci_dev->mem_resource[0].addr to be NULL. This happens when the pci device is attached to igb_uio.ko but with vfio_pci it works fine. Looking at the pci device initialization part for secondary process, when device is presented through igb_uio, (in pci_uio_map_secondary), it doesn't seems to update pci_dev->mem_resource[X].addr. For vfio-pci, it updates the addresses in pci_vfio_map_resource_secondary. With the following patch, i am able to use the device from secondary process but would like to know if I am missing something. diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c index 7ea73db..f4dca9d 100644 --- a/drivers/bus/pci/pci_common_uio.c +++ b/drivers/bus/pci/pci_common_uio.c @@ -70,6 +70,7 @@ } return -1; } + dev->mem_resource[i].addr = mapaddr; } return 0; }
[dpdk-dev] [PATCH v5 1/4] net/i40e: cleanup Tx buffers
Add support to the i40e driver for the API rte_eth_tx_done_cleanup
to force free consumed buffers on Tx ring.
Signed-off-by: Chenxu Di
---
drivers/net/i40e/i40e_ethdev.c| 1 +
drivers/net/i40e/i40e_ethdev_vf.c | 1 +
drivers/net/i40e/i40e_rxtx.c | 122 ++
drivers/net/i40e/i40e_rxtx.h | 1 +
4 files changed, 125 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5999c964b..fad47a942 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -522,6 +522,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
.mac_addr_set = i40e_set_default_mac_addr,
.mtu_set = i40e_dev_mtu_set,
.tm_ops_get = i40e_tm_ops_get,
+ .tx_done_cleanup = i40e_tx_done_cleanup,
};
/* store statistics names and its offset in stats structure */
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
b/drivers/net/i40e/i40e_ethdev_vf.c
index 5dba0928b..0ca5417d7 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -215,6 +215,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
.rss_hash_conf_get= i40evf_dev_rss_hash_conf_get,
.mtu_set = i40evf_dev_mtu_set,
.mac_addr_set = i40evf_set_default_mac_addr,
+ .tx_done_cleanup = i40e_tx_done_cleanup,
};
/*
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 17dc8c78f..9e4b0b678 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2455,6 +2455,128 @@ i40e_tx_queue_release_mbufs(struct i40e_tx_queue *txq)
}
}
+int i40e_tx_done_cleanup(void *q, uint32_t free_cnt)
+{
+ struct i40e_tx_queue *txq = (struct i40e_tx_queue *)q;
+ struct i40e_tx_entry *sw_ring;
+ volatile struct i40e_tx_desc *txr;
+ uint16_t tx_first; /* First segment analyzed. */
+ uint16_t tx_id;/* Current segment being processed. */
+ uint16_t tx_last; /* Last segment in the current packet. */
+ uint16_t tx_next; /* First segment of the next packet. */
+ int count;
+
+ if (txq == NULL)
+ return -ENODEV;
+
+ count = 0;
+ sw_ring = txq->sw_ring;
+ txr = txq->tx_ring;
+
+ /*
+* tx_tail is the last sent packet on the sw_ring. Goto the end
+* of that packet (the last segment in the packet chain) and
+* then the next segment will be the start of the oldest segment
+* in the sw_ring. This is the first packet that will be
+* attempted to be freed.
+*/
+
+ /* Get last segment in most recently added packet. */
+ tx_last = sw_ring[txq->tx_tail].last_id;
+
+ /* Get the next segment, which is the oldest segment in ring. */
+ tx_first = sw_ring[tx_last].next_id;
+
+ /* Set the current index to the first. */
+ tx_id = tx_first;
+
+ /*
+* Loop through each packet. For each packet, verify that an
+* mbuf exists and that the last segment is free. If so, free
+* it and move on.
+*/
+ while (1) {
+ tx_last = sw_ring[tx_id].last_id;
+
+ if (sw_ring[tx_last].mbuf) {
+ if ((txr[tx_last].cmd_type_offset_bsz &
+ rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
+ rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
{
+ /* Get the start of the next packet. */
+ tx_next = sw_ring[tx_last].next_id;
+
+ /*
+* Loop through all segments in a
+* packet.
+*/
+ do {
+
rte_pktmbuf_free_seg(sw_ring[tx_id].mbuf);
+ sw_ring[tx_id].mbuf = NULL;
+ sw_ring[tx_id].last_id = tx_id;
+
+ /* Move to next segment. */
+ tx_id = sw_ring[tx_id].next_id;
+
+ } while (tx_id != tx_next);
+
+ /*
+* Increment the number of packets
+* freed.
+*/
+ count++;
+
+ if (unlikely(count == (int)free_cnt))
+ break;
+ } else {
+ /*
+* mbuf still in use, nothing left to
+* free.
+*/
+ break;
+ }
+ } else {
+ /*
+
[dpdk-dev] [PATCH v5 0/4] drivers/net: cleanup Tx buffers
Add support to the drivers inclulding i40e, ice, ixgbe and igb vf for the API rte_eth_tx_done_cleanup to force free consumed buffers on Tx ring. --- v2: added code about igb vf. v3: changed information of author v4: changed code. v5: fixed code and notes. removed code for fm10k. Chenxu Di (4): net/i40e: cleanup Tx buffers net/ice: cleanup Tx buffers net/ixgbe: cleanup Tx buffers net/e1000: cleanup Tx buffers drivers/net/e1000/igb_ethdev.c| 1 + drivers/net/i40e/i40e_ethdev.c| 1 + drivers/net/i40e/i40e_ethdev_vf.c | 1 + drivers/net/i40e/i40e_rxtx.c | 122 + drivers/net/i40e/i40e_rxtx.h | 1 + drivers/net/ice/ice_ethdev.c | 1 + drivers/net/ice/ice_rxtx.c| 123 ++ drivers/net/ice/ice_rxtx.h| 1 + drivers/net/ixgbe/ixgbe_ethdev.c | 2 + drivers/net/ixgbe/ixgbe_rxtx.c| 121 + drivers/net/ixgbe/ixgbe_rxtx.h| 2 + 11 files changed, 376 insertions(+) -- 2.17.1
[dpdk-dev] [PATCH v5 4/4] net/e1000: cleanup Tx buffers
Add support to the igb vf for the API rte_eth_tx_done_cleanup
to force free consumed buffers on Tx ring.
Signed-off-by: Chenxu Di
---
drivers/net/e1000/igb_ethdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index a3e30dbe5..647d5504f 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -446,6 +446,7 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = {
.tx_descriptor_status = eth_igb_tx_descriptor_status,
.tx_queue_setup = eth_igb_tx_queue_setup,
.tx_queue_release = eth_igb_tx_queue_release,
+ .tx_done_cleanup = eth_igb_tx_done_cleanup,
.set_mc_addr_list = eth_igb_set_mc_addr_list,
.rxq_info_get = igb_rxq_info_get,
.txq_info_get = igb_txq_info_get,
--
2.17.1
[dpdk-dev] [PATCH v5 3/4] net/ixgbe: cleanup Tx buffers
Add support to the ixgbe driver for the API rte_eth_tx_done_cleanup
to force free consumed buffers on Tx ring.
Signed-off-by: Chenxu Di
---
drivers/net/ixgbe/ixgbe_ethdev.c | 2 +
drivers/net/ixgbe/ixgbe_rxtx.c | 121 +++
drivers/net/ixgbe/ixgbe_rxtx.h | 2 +
3 files changed, 125 insertions(+)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 2c6fd0f13..0091405db 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -601,6 +601,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
.udp_tunnel_port_add = ixgbe_dev_udp_tunnel_port_add,
.udp_tunnel_port_del = ixgbe_dev_udp_tunnel_port_del,
.tm_ops_get = ixgbe_tm_ops_get,
+ .tx_done_cleanup = ixgbe_tx_done_cleanup,
};
/*
@@ -649,6 +650,7 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
.reta_query = ixgbe_dev_rss_reta_query,
.rss_hash_update = ixgbe_dev_rss_hash_update,
.rss_hash_conf_get= ixgbe_dev_rss_hash_conf_get,
+ .tx_done_cleanup = ixgbe_tx_done_cleanup,
};
/* store statistics names and its offset in stats structure */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index fa572d184..8d8e0655c 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2306,6 +2306,127 @@ ixgbe_tx_queue_release_mbufs(struct ixgbe_tx_queue *txq)
}
}
+int ixgbe_tx_done_cleanup(void *q, uint32_t free_cnt)
+{
+ struct ixgbe_tx_queue *txq = (struct ixgbe_tx_queue *)q;
+ struct ixgbe_tx_entry *sw_ring;
+ volatile union ixgbe_adv_tx_desc *txr;
+ uint16_t tx_first; /* First segment analyzed. */
+ uint16_t tx_id;/* Current segment being processed. */
+ uint16_t tx_last; /* Last segment in the current packet. */
+ uint16_t tx_next; /* First segment of the next packet. */
+ int count;
+
+ if (txq == NULL)
+ return -ENODEV;
+
+ count = 0;
+ sw_ring = txq->sw_ring;
+ txr = txq->tx_ring;
+
+ /*
+* tx_tail is the last sent packet on the sw_ring. Goto the end
+* of that packet (the last segment in the packet chain) and
+* then the next segment will be the start of the oldest segment
+* in the sw_ring. This is the first packet that will be
+* attempted to be freed.
+*/
+
+ /* Get last segment in most recently added packet. */
+ tx_last = sw_ring[txq->tx_tail].last_id;
+
+ /* Get the next segment, which is the oldest segment in ring. */
+ tx_first = sw_ring[tx_last].next_id;
+
+ /* Set the current index to the first. */
+ tx_id = tx_first;
+
+ /*
+* Loop through each packet. For each packet, verify that an
+* mbuf exists and that the last segment is free. If so, free
+* it and move on.
+*/
+ while (1) {
+ tx_last = sw_ring[tx_id].last_id;
+
+ if (sw_ring[tx_last].mbuf) {
+ if (txr[tx_last].wb.status &
+ IXGBE_TXD_STAT_DD) {
+ /* Get the start of the next packet. */
+ tx_next = sw_ring[tx_last].next_id;
+
+ /*
+* Loop through all segments in a
+* packet.
+*/
+ do {
+
rte_pktmbuf_free_seg(sw_ring[tx_id].mbuf);
+ sw_ring[tx_id].mbuf = NULL;
+ sw_ring[tx_id].last_id = tx_id;
+
+ /* Move to next segment. */
+ tx_id = sw_ring[tx_id].next_id;
+
+ } while (tx_id != tx_next);
+
+ /*
+* Increment the number of packets
+* freed.
+*/
+ count++;
+
+ if (unlikely(count == (int)free_cnt))
+ break;
+ } else {
+ /*
+* mbuf still in use, nothing left to
+* free.
+*/
+ break;
+ }
+ } else {
+ /*
+* There are multiple reasons to be here:
+* 1) All the packets on the ring have been
+*freed - tx_id is equal to tx_first
+*and some packets have been freed.
+*- Done, exit
+* 2)
[dpdk-dev] [PATCH v5 2/4] net/ice: cleanup Tx buffers
Add support to the ice driver for the API rte_eth_tx_done_cleanup
to force free consumed buffers on Tx ring.
Signed-off-by: Chenxu Di
---
drivers/net/ice/ice_ethdev.c | 1 +
drivers/net/ice/ice_rxtx.c | 123 +++
drivers/net/ice/ice_rxtx.h | 1 +
3 files changed, 125 insertions(+)
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index de189daba..b55cdbf74 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -220,6 +220,7 @@ static const struct eth_dev_ops ice_eth_dev_ops = {
.filter_ctrl = ice_dev_filter_ctrl,
.udp_tunnel_port_add = ice_dev_udp_tunnel_port_add,
.udp_tunnel_port_del = ice_dev_udp_tunnel_port_del,
+ .tx_done_cleanup = ice_tx_done_cleanup,
};
/* store statistics names and its offset in stats structure */
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 2db174456..7e704d2d5 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -863,6 +863,129 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t
tx_queue_id)
return 0;
}
+
+int ice_tx_done_cleanup(void *q, uint32_t free_cnt)
+{
+ struct ice_tx_queue *txq = (struct ice_tx_queue *)q;
+ struct ice_tx_entry *sw_ring;
+ volatile struct ice_tx_desc *txr;
+ uint16_t tx_first; /* First segment analyzed. */
+ uint16_t tx_id;/* Current segment being processed. */
+ uint16_t tx_last; /* Last segment in the current packet. */
+ uint16_t tx_next; /* First segment of the next packet. */
+ int count;
+
+ if (txq == NULL)
+ return -ENODEV;
+
+ count = 0;
+ sw_ring = txq->sw_ring;
+ txr = txq->tx_ring;
+
+ /*
+* tx_tail is the last sent packet on the sw_ring. Goto the end
+* of that packet (the last segment in the packet chain) and
+* then the next segment will be the start of the oldest segment
+* in the sw_ring. This is the first packet that will be
+* attempted to be freed.
+*/
+
+ /* Get last segment in most recently added packet. */
+ tx_last = sw_ring[txq->tx_tail].last_id;
+
+ /* Get the next segment, which is the oldest segment in ring. */
+ tx_first = sw_ring[tx_last].next_id;
+
+ /* Set the current index to the first. */
+ tx_id = tx_first;
+
+ /*
+* Loop through each packet. For each packet, verify that an
+* mbuf exists and that the last segment is free. If so, free
+* it and move on.
+*/
+ while (1) {
+ tx_last = sw_ring[tx_id].last_id;
+
+ if (sw_ring[tx_last].mbuf) {
+ if ((txr[tx_last].cmd_type_offset_bsz &
+ rte_cpu_to_le_64(ICE_TXD_QW1_DTYPE_M)) ==
+ rte_cpu_to_le_64(ICE_TX_DESC_DTYPE_DESC_DONE)) {
+ /* Get the start of the next packet. */
+ tx_next = sw_ring[tx_last].next_id;
+
+ /*
+* Loop through all segments in a
+* packet.
+*/
+ do {
+
rte_pktmbuf_free_seg(sw_ring[tx_id].mbuf);
+ sw_ring[tx_id].mbuf = NULL;
+ sw_ring[tx_id].last_id = tx_id;
+
+ /* Move to next segment. */
+ tx_id = sw_ring[tx_id].next_id;
+
+ } while (tx_id != tx_next);
+
+ /*
+* Increment the number of packets
+* freed.
+*/
+ count++;
+
+ if (unlikely(count == (int)free_cnt))
+ break;
+ } else {
+ /*
+* mbuf still in use, nothing left to
+* free.
+*/
+ break;
+ }
+ } else {
+ /*
+* There are multiple reasons to be here:
+* 1) All the packets on the ring have been
+*freed - tx_id is equal to tx_first
+*and some packets have been freed.
+*- Done, exit
+* 2) Interfaces has not sent a rings worth of
+*packets yet, so the segment after tail is
+*still empty. Or a previous call to this
+*functio
Re: [dpdk-dev] [PATCH v3 36/36] net/i40e/base: add new link speed constants
> -Original Message-
> From: Ye, Xiaolong
> Sent: Monday, December 16, 2019 10:44 AM
> To: Xing, Beilei ; Zhang, Qi Z
> Cc: dev@dpdk.org; Stillwell Jr, Paul M ; Ye,
> Xiaolong ; Loktionov, Aleksandr
>
> Subject: [PATCH v3 36/36] net/i40e/base: add new link speed constants
>
> This patch fixes 'NIC Link is Up, Unknown bps' message in dmesg for 2.5Gb/5Gb
> speeds. This problem is fixed by adding constants for
> VIRTCHNL_LINK_SPEED_2_5GB and VIRTCHNL_LINK_SPEED_5GB.
>
It's a fix patch, fix line is needed? The same comment is also for patch 27/36,
25/36, 21/36, 14/36.
> Signed-off-by: Aleksandr Loktionov
> Signed-off-by: Xiaolong Ye
> ---
> drivers/net/i40e/base/i40e_prototype.h | 4
> drivers/net/i40e/base/virtchnl.h | 4
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/net/i40e/base/i40e_prototype.h
> b/drivers/net/i40e/base/i40e_prototype.h
> index 0f06e3262..d8ab3ea0a 100644
> --- a/drivers/net/i40e/base/i40e_prototype.h
> +++ b/drivers/net/i40e/base/i40e_prototype.h
> @@ -505,6 +505,10 @@ i40e_virtchnl_link_speed(enum i40e_aq_link_speed
> link_speed)
> return VIRTCHNL_LINK_SPEED_100MB;
> case I40E_LINK_SPEED_1GB:
> return VIRTCHNL_LINK_SPEED_1GB;
> + case I40E_LINK_SPEED_2_5GB:
> + return VIRTCHNL_LINK_SPEED_2_5GB;
> + case I40E_LINK_SPEED_5GB:
> + return VIRTCHNL_LINK_SPEED_5GB;
> case I40E_LINK_SPEED_10GB:
> return VIRTCHNL_LINK_SPEED_10GB;
> case I40E_LINK_SPEED_40GB:
> diff --git a/drivers/net/i40e/base/virtchnl.h
> b/drivers/net/i40e/base/virtchnl.h
> index c613d4761..92515bf34 100644
> --- a/drivers/net/i40e/base/virtchnl.h
> +++ b/drivers/net/i40e/base/virtchnl.h
> @@ -53,12 +53,14 @@ enum virtchnl_status_code { #define
> VIRTCHNL_ERR_PARAM VIRTCHNL_STATUS_ERR_PARAM #define
> VIRTCHNL_STATUS_NOT_SUPPORTED
> VIRTCHNL_STATUS_ERR_NOT_SUPPORTED
>
> +#define VIRTCHNL_LINK_SPEED_2_5GB_SHIFT 0x0
> #define VIRTCHNL_LINK_SPEED_100MB_SHIFT 0x1
> #define VIRTCHNL_LINK_SPEED_1000MB_SHIFT 0x2
> #define VIRTCHNL_LINK_SPEED_10GB_SHIFT 0x3
> #define VIRTCHNL_LINK_SPEED_40GB_SHIFT 0x4
> #define VIRTCHNL_LINK_SPEED_20GB_SHIFT 0x5
> #define VIRTCHNL_LINK_SPEED_25GB_SHIFT 0x6
> +#define VIRTCHNL_LINK_SPEED_5GB_SHIFT0x7
>
> enum virtchnl_link_speed {
> VIRTCHNL_LINK_SPEED_UNKNOWN = 0,
> @@ -68,6 +70,8 @@ enum virtchnl_link_speed {
> VIRTCHNL_LINK_SPEED_40GB=
> BIT(VIRTCHNL_LINK_SPEED_40GB_SHIFT),
> VIRTCHNL_LINK_SPEED_20GB=
> BIT(VIRTCHNL_LINK_SPEED_20GB_SHIFT),
> VIRTCHNL_LINK_SPEED_25GB=
> BIT(VIRTCHNL_LINK_SPEED_25GB_SHIFT),
> + VIRTCHNL_LINK_SPEED_2_5GB =
> BIT(VIRTCHNL_LINK_SPEED_2_5GB_SHIFT),
> + VIRTCHNL_LINK_SPEED_5GB =
> BIT(VIRTCHNL_LINK_SPEED_5GB_SHIFT),
> };
>
> /* for hsplit_0 field of Rx HMC context */
> --
> 2.17.1
