From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E51B9A04B7; Wed, 14 Oct 2020 15:38:18 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E14221DEB3; Wed, 14 Oct 2020 15:31:43 +0200 (CEST) Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by dpdk.org (Postfix) with ESMTP id 0658E1DE70 for ; Wed, 14 Oct 2020 15:31:41 +0200 (CEST) Received: by mail-wr1-f67.google.com with SMTP id s9so3858387wro.8 for ; Wed, 14 Oct 2020 06:31:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=S7/hqWt6B8f+0PiKhhMz7d/xiqu65BX1oKb2rnECikg=; b=GYGYI/twxZNQWuOMYsN2xVPQIuW0uAgCMZD5CL1KzdXAm2ZJSFyAfLmPpLueuaHh15 8VYL4Mkllqm3r/2XD2JO8YtuO1Tt41y+MCFTztJNZPdIiGQIhIoCW0ueYGSkbtPn8Rvy W6E4h5emcRPGxUYOvQRZBzVqXd2E5OYLbOZEPx6yaGmMOL7rdaa6tj47e4h45YwW9O3N tkYH36R7PzDLlOAfzzcXBNrAnN/fG1LeH18RIz7j2MlvaQ8PPU4bBwnvb5gwDOvr72nb hs17y079EGUPIjBTK9ms32bl/ghIkSrKy/6web86c0hYdOpdCDiFeLFrOgbgd7/Xwkwq fbEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=S7/hqWt6B8f+0PiKhhMz7d/xiqu65BX1oKb2rnECikg=; b=k9KtBd24vUum5X0hC6LYjrhzEzmH9MGW1qNJhEvthiODs/vszyFylnz/zDhsyVOdDa 65lyIhZET1MA4HVsm2f5yQGRPJ51/6ChDHbbuyARwYhLkPG+KoUYSv1dh7U5qAt2fQfR sVAa0oWe22gLTT+QYVmXTVi3cVjsaEBm3jlmZLCBnWDGlb1D5Q3OoHIq1E2OjeLI48jx IDgxDlcV/sDwjd3dqcnoH3XH+ezXVqcMCjKwJ6xV0JyvdIJieW6/4O8inS+Ds5qwWzSC /OjiMA7fCmKcGrYCgfdQQgvcdD5Fxu+01UcoyVRdkH5f7trxUJ5berbtomWWRCxuf5di l7Lw== X-Gm-Message-State: AOAM530dmd2czYGrg6doS0wSNBm4JA/EK9FJstI4OYOnY11/UegCv1WA kRHSYE+aN7w3BKPInTtI8d71gw== X-Google-Smtp-Source: ABdhPJy303KySL0Cn0tY7E7jFE7d6yVqQw/GTCc2LJMkQVaTUUKdN2QDF9fnEA84fwP0vuFbZ5fQXw== X-Received: by 2002:adf:fd82:: with SMTP id d2mr5659755wrr.304.1602682300382; Wed, 14 Oct 2020 06:31:40 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id x15sm5536270wrr.36.2020.10.14.06.31.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Oct 2020 06:31:39 -0700 (PDT) Date: Wed, 14 Oct 2020 15:31:38 +0200 From: Olivier Matz To: Ferruh Yigit Cc: Viacheslav Ovsiienko , dev@dpdk.org, thomasm@monjalon.net, stephen@networkplumber.org, jerinjacobk@gmail.com, maxime.coquelin@redhat.com, david.marchand@redhat.com, arybchenko@solarflare.com Message-ID: <20201014133138.GR21395@platinum> References: <1602616917-22193-1-git-send-email-viacheslavo@nvidia.com> <1602616917-22193-2-git-send-email-viacheslavo@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v5 1/6] ethdev: introduce Rx buffer split X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Slava, After reviewing this version and comparing with v4, I think having the configuration in rxconf (like in this patch) is a better choice. Few comments below. On Tue, Oct 13, 2020 at 11:34:16PM +0100, Ferruh Yigit wrote: > On 10/13/2020 8:21 PM, Viacheslav Ovsiienko wrote: > > The DPDK datapath in the transmit direction is very flexible. > > An application can build the multi-segment packet and manages > > almost all data aspects - the memory pools where segments > > are allocated from, the segment lengths, the memory attributes > > like external buffers, registered for DMA, etc. > > > > In the receiving direction, the datapath is much less flexible, > > an application can only specify the memory pool to configure the > > receiving queue and nothing more. In order to extend receiving > > datapath capabilities it is proposed to add the way to provide > > extended information how to split the packets being received. > > > > The following structure is introduced to specify the Rx packet > > segment: > > > > struct rte_eth_rxseg { > > struct rte_mempool *mp; /* memory pools to allocate segment from */ > > uint16_t length; /* segment maximal data length, > > configures "split point" */ > > uint16_t offset; /* data offset from beginning > > of mbuf data buffer */ > > uint32_t reserved; /* reserved field */ > > }; > > > > The segment descriptions are added to the rte_eth_rxconf structure: > > rx_seg - pointer the array of segment descriptions, each element > > describes the memory pool, maximal data length, initial > > data offset from the beginning of data buffer in mbuf. > > This array allows to specify the different settings for > > each segment in individual fashion. > > n_seg - number of elements in the array > > > > If the extended segment descriptions is provided with these new > > fields the mp parameter of the rte_eth_rx_queue_setup must be > > specified as NULL to avoid ambiguity. > > > > The new offload flag RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT in device > > capabilities is introduced to present the way for PMD to report to > > application about supporting Rx packet split to configurable > > segments. Prior invoking the rte_eth_rx_queue_setup() routine > > application should check RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT flag. > > > > If the Rx queue is configured with new routine the packets being > > received will be split into multiple segments pushed to the mbufs > > with specified attributes. The PMD will split the received packets > > into multiple segments according to the specification in the > > description array: > > > > - the first network buffer will be allocated from the memory pool, > > specified in the first segment description element, the second > > network buffer - from the pool in the second segment description > > element and so on. If there is no enough elements to describe > > the buffer for entire packet of maximal length the pool from the > > last valid element will be used to allocate the buffers from for the > > rest of segments > > > > - the offsets from the segment description elements will provide > > the data offset from the buffer beginning except the first mbuf - > > for this one the offset is added to the RTE_PKTMBUF_HEADROOM to get > > actual offset from the buffer beginning. If there is no enough > > elements to describe the buffer for entire packet of maximal length > > the offsets for the rest of segment will be supposed to be zero. > > > > - the data length being received to each segment is limited by the > > length specified in the segment description element. The data > > receiving starts with filling up the first mbuf data buffer, if the > > specified maximal segment length is reached and there are data > > remaining (packet is longer than buffer in the first mbuf) the > > following data will be pushed to the next segment up to its own > > maximal length. If the first two segments is not enough to store > > all the packet remaining data the next (third) segment will > > be engaged and so on. If the length in the segment description > > element is zero the actual buffer size will be deduced from > > the appropriate memory pool properties. If there is no enough > > elements to describe the buffer for entire packet of maximal > > length the buffer size will be deduced from the pool of the last > > valid element for the remaining segments. > > > > For example, let's suppose we configured the Rx queue with the > > following segments: > > seg0 - pool0, len0=14B, off0=2 > > seg1 - pool1, len1=20B, off1=128B > > seg2 - pool2, len2=20B, off2=0B > > seg3 - pool3, len3=512B, off3=0B > > > > The packet 46 bytes long will look like the following: > > seg0 - 14B long @ RTE_PKTMBUF_HEADROOM + 2 in mbuf from pool0 > > seg1 - 20B long @ 128 in mbuf from pool1 > > seg2 - 12B long @ 0 in mbuf from pool2 > > > > The packet 1500 bytes long will look like the following: > > seg0 - 14B @ RTE_PKTMBUF_HEADROOM + 2 in mbuf from pool0 > > seg1 - 20B @ 128 in mbuf from pool1 > > seg2 - 20B @ 0 in mbuf from pool2 > > seg3 - 512B @ 0 in mbuf from pool3 > > seg4 - 512B @ 0 in mbuf from pool3 > > seg5 - 422B @ 0 in mbuf from pool3 > > > > The offload RTE_ETH_RX_OFFLOAD_SCATTER must be present and > > configured to support new buffer split feature (if n_seg > > is greater than one). > > > > The new approach would allow splitting the ingress packets into > > multiple parts pushed to the memory with different attributes. > > For example, the packet headers can be pushed to the embedded > > data buffers within mbufs and the application data into > > the external buffers attached to mbufs allocated from the > > different memory pools. The memory attributes for the split > > parts may differ either - for example the application data > > may be pushed into the external memory located on the dedicated > > physical device, say GPU or NVMe. This would improve the DPDK > > receiving datapath flexibility with preserving compatibility > > with existing API. > > > > Signed-off-by: Viacheslav Ovsiienko > > --- > > doc/guides/nics/features.rst | 15 +++++ > > doc/guides/rel_notes/release_20_11.rst | 9 +++ > > lib/librte_ethdev/rte_ethdev.c | 95 ++++++++++++++++++++++++-------- > > lib/librte_ethdev/rte_ethdev.h | 58 ++++++++++++++++++- > > lib/librte_ethdev/rte_ethdev_version.map | 1 + > > Can you please update deprecation notice too, to remove the notice? > > > 5 files changed, 155 insertions(+), 23 deletions(-) > > > > diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst > > index dd8c955..a45a9e8 100644 > > --- a/doc/guides/nics/features.rst > > +++ b/doc/guides/nics/features.rst > > @@ -185,6 +185,21 @@ Supports receiving segmented mbufs. > > * **[related] eth_dev_ops**: ``rx_pkt_burst``. > > +.. _nic_features_buffer_split: > > + > > +Buffer Split on Rx > > +------------------ > > + > > +Scatters the packets being received on specified boundaries to segmented mbufs. > > + > > +* **[uses] rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT``. > > uses 'rx_seg and rx_nseg' from 'rte_eth_rxconf', perhaps it can be another line. > > > +* **[implements] datapath**: ``Buffer Split functionality``. > > +* **[implements] rte_eth_dev_data**: ``buffer_split``. > > What is implemented here? > > > +* **[provides] rte_eth_dev_info**: ``rx_offload_capa:RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT``. > > +* **[provides] eth_dev_ops**: ``rxq_info_get:buffer_split``. > > Is this correct? > > > +* **[related] API**: ``rte_eth_rx_queue_setup()``. > > + > > + > > .. _nic_features_lro: > > LRO > > diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst > > index bcc0fc2..f67ec62 100644 > > --- a/doc/guides/rel_notes/release_20_11.rst > > +++ b/doc/guides/rel_notes/release_20_11.rst > > @@ -60,6 +60,12 @@ New Features > > Added the FEC API which provides functions for query FEC capabilities and > > current FEC mode from device. Also, API for configuring FEC mode is also provided. > > +* **Introduced extended buffer description for receiving.** > > + > > + Added the extended Rx queue setup routine providing the individual > > This looks wrong with last version. > > > + descriptions for each Rx segment with maximal size, buffer offset and memory > > + pool to allocate data buffers from. > > + > > * **Updated Broadcom bnxt driver.** > > Updated the Broadcom bnxt driver with new features and improvements, including: > > @@ -253,6 +259,9 @@ API Changes > > As the data of ``uint8_t`` will be truncated when queue number under > > a TC is greater than 256. > > +* ethdev: Added fields rx_seg and rx_nseg to rte_eth_rxconf structure > > + to provide extended description of the receiving buffer. > > + > > * vhost: Moved vDPA APIs from experimental to stable. > > * rawdev: Added a structure size parameter to the functions > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > > index 892c246..7b64a6e 100644 > > --- a/lib/librte_ethdev/rte_ethdev.c > > +++ b/lib/librte_ethdev/rte_ethdev.c > > @@ -105,6 +105,9 @@ struct rte_eth_xstats_name_off { > > #define RTE_RX_OFFLOAD_BIT2STR(_name) \ > > { DEV_RX_OFFLOAD_##_name, #_name } > > +#define RTE_ETH_RX_OFFLOAD_BIT2STR(_name) \ > > + { RTE_ETH_RX_OFFLOAD_##_name, #_name } > > + > > static const struct { > > uint64_t offload; > > const char *name; > > @@ -128,9 +131,11 @@ struct rte_eth_xstats_name_off { > > RTE_RX_OFFLOAD_BIT2STR(SCTP_CKSUM), > > RTE_RX_OFFLOAD_BIT2STR(OUTER_UDP_CKSUM), > > RTE_RX_OFFLOAD_BIT2STR(RSS_HASH), > > + RTE_ETH_RX_OFFLOAD_BIT2STR(BUFFER_SPLIT), > > }; > > #undef RTE_RX_OFFLOAD_BIT2STR > > +#undef RTE_ETH_RX_OFFLOAD_BIT2STR > > #define RTE_TX_OFFLOAD_BIT2STR(_name) \ > > { DEV_TX_OFFLOAD_##_name, #_name } > > @@ -1770,10 +1775,14 @@ struct rte_eth_dev * > > struct rte_mempool *mp) > > { > > int ret; > > + uint16_t seg_idx; > > uint32_t mbp_buf_size; > > struct rte_eth_dev *dev; > > struct rte_eth_dev_info dev_info; > > struct rte_eth_rxconf local_conf; > > + const struct rte_eth_rxseg *rx_seg; > > + struct rte_eth_rxseg seg_single = { .mp = mp}; missing space > > + uint16_t n_seg; > > void **rxq; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); > > @@ -1784,13 +1793,32 @@ struct rte_eth_dev * > > return -EINVAL; > > } > > - if (mp == NULL) { > > - RTE_ETHDEV_LOG(ERR, "Invalid null mempool pointer\n"); > > - return -EINVAL; > > - } > > - > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_setup, -ENOTSUP); > > + rx_seg = rx_conf->rx_seg; > > + n_seg = rx_conf->rx_nseg; > > + if (rx_seg == NULL) { > > + /* Exclude ambiguities about segment descrtiptions. */ > > + if (n_seg) { > > + RTE_ETHDEV_LOG(ERR, > > + "Non empty array with null pointer\n"); > > + return -EINVAL; > > + } > > + rx_seg = &seg_single; > > + n_seg = 1; > > Why setting 'rx_seg' & 'n_seg'? Why not leaving them NULL and 0 when not used? > This was PMD can do NULL/0 check and can know they are not used. I think they are just set locally to factorize the checks below, but I agree it is questionnable: it seems the only check which is really factorized is about private_data_size. > I think better to do a "if (mp == NULL)" check here, both 'rx_seg' & 'mp' > should not be NULL. Agree, something like this looks more simple: if (mp == NULL) { if (n_seg == 0 || rx_seg == NULL) RTE_ETHDEV_LOG(ERR, "..."); } else { if (n_seg != 0 || rx_seg != NULL) RTE_ETHDEV_LOG(ERR, "..."); rx_seg = &seg_single; n_seg = 1; } > > + } else { > > + if (n_seg == 0) { > > + RTE_ETHDEV_LOG(ERR, > > + "Invalid zero descriptions number\n"); > > + return -EINVAL; > > + } > > + if (mp) { > > + RTE_ETHDEV_LOG(ERR, > > + "Memory pool duplicated definition\n"); > > + return -EINVAL; > > + } > > + } > > + > > /* > > * Check the size of the mbuf data buffer. > > * This value must be provided in the private data of the memory pool. > > @@ -1800,23 +1828,48 @@ struct rte_eth_dev * > > if (ret != 0) > > return ret; > > - if (mp->private_data_size < sizeof(struct rte_pktmbuf_pool_private)) { > > - RTE_ETHDEV_LOG(ERR, "%s private_data_size %d < %d\n", > > - mp->name, (int)mp->private_data_size, > > - (int)sizeof(struct rte_pktmbuf_pool_private)); > > - return -ENOSPC; > > - } > > - mbp_buf_size = rte_pktmbuf_data_room_size(mp); > > + for (seg_idx = 0; seg_idx < n_seg; seg_idx++) { > > + struct rte_mempool *mpl = rx_seg[seg_idx].mp; > > + uint32_t length = rx_seg[seg_idx].length; > > + uint32_t offset = rx_seg[seg_idx].offset; > > + uint32_t head_room = seg_idx ? 0 : RTE_PKTMBUF_HEADROOM; > > - if (mbp_buf_size < dev_info.min_rx_bufsize + RTE_PKTMBUF_HEADROOM) { > > - RTE_ETHDEV_LOG(ERR, > > - "%s mbuf_data_room_size %d < %d (RTE_PKTMBUF_HEADROOM=%d + min_rx_bufsize(dev)=%d)\n", > > - mp->name, (int)mbp_buf_size, > > - (int)(RTE_PKTMBUF_HEADROOM + dev_info.min_rx_bufsize), > > - (int)RTE_PKTMBUF_HEADROOM, > > - (int)dev_info.min_rx_bufsize); > > - return -EINVAL; > > + if (mpl == NULL) { > > + RTE_ETHDEV_LOG(ERR, "Invalid null mempool pointer\n"); > > + return -EINVAL; > > + } > > + > > + if (mpl->private_data_size < > > + sizeof(struct rte_pktmbuf_pool_private)) { > > + RTE_ETHDEV_LOG(ERR, "%s private_data_size %d < %d\n", > > + mpl->name, (int)mpl->private_data_size, > > + (int)sizeof(struct rte_pktmbuf_pool_private)); > > + return -ENOSPC; > > + } > > + > > + mbp_buf_size = rte_pktmbuf_data_room_size(mpl); > > + length = length ? length : (mbp_buf_size - head_room); > > + if (mbp_buf_size < length + offset + head_room) { Is length == 0 allowed? Or is it to handle the case where mp != NULL? Is the test needed in that case? It seems it is equivalent to do "if (offset > 0)". Wouldn't it be better to check mp like this? if (mp == NULL) { if (mbp_buf_size < length + offset + head_room) ...error... } > > + RTE_ETHDEV_LOG(ERR, > > + "%s mbuf_data_room_size %u < %u" > > + " (segment length=%u + segment offset=%u)\n", > > + mpl->name, mbp_buf_size, > > + length + offset, length, offset); > > + return -EINVAL; > > + } > > } > > + /* Check the minimal buffer size for the single segment only. */ > > This is the main branch, what do you think moving the comment to the > beggining of above for loop and add a comment about testing the multiple > segment. > > Btw, I have a concern that this single/multi segment can cause a confusion > with multi segment packets. Can something else, like "split package" can be > used instead of segment? > > > + if (mp && (mbp_buf_size < dev_info.min_rx_bufsize + > > + RTE_PKTMBUF_HEADROOM)) { > > + RTE_ETHDEV_LOG(ERR, > > + "%s mbuf_data_room_size %u < %u " > > + "(RTE_PKTMBUF_HEADROOM=%u + " > > + "min_rx_bufsize(dev)=%u)\n", > > + mp->name, mbp_buf_size, > > + RTE_PKTMBUF_HEADROOM + dev_info.min_rx_bufsize, > > + RTE_PKTMBUF_HEADROOM, dev_info.min_rx_bufsize); > > + return -EINVAL; > > + } The "}" is not indented correctly > > /* Use default specified by driver, if nb_rx_desc is zero */ > > if (nb_rx_desc == 0) { > > @@ -1914,8 +1967,6 @@ struct rte_eth_dev * > > dev->data->min_rx_buf_size = mbp_buf_size; > > } > > - rte_ethdev_trace_rxq_setup(port_id, rx_queue_id, nb_rx_desc, mp, > > - rx_conf, ret); > > Is this removed intentionally? > > > return eth_err(port_id, ret); > > } > > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h > > index 5bcfbb8..9cf0a03 100644 > > --- a/lib/librte_ethdev/rte_ethdev.h > > +++ b/lib/librte_ethdev/rte_ethdev.h > > @@ -970,6 +970,16 @@ struct rte_eth_txmode { > > }; > > /** > > + * A structure used to configure an RX packet segment to split. > > + */ > > +struct rte_eth_rxseg { > > + struct rte_mempool *mp; /**< Memory pools to allocate segment from */ pools -> pool > > + uint16_t length; /**< Segment data length, configures split point. */ > > + uint16_t offset; /**< Data offset from beginning of mbuf data buffer */ > > + uint32_t reserved; /**< Reserved field */ To allow future additions to the structure, should the reserved field always be set to 0? If yes, maybe it should be specified here and checked in rte_eth_rx_queue_setup(). Some "." missing at the end of comments. > > +}; > > + > > +/** > > * A structure used to configure an RX ring of an Ethernet port. > > */ > > struct rte_eth_rxconf { > > @@ -977,13 +987,23 @@ struct rte_eth_rxconf { > > uint16_t rx_free_thresh; /**< Drives the freeing of RX descriptors. */ > > uint8_t rx_drop_en; /**< Drop packets if no descriptors are available. */ > > uint8_t rx_deferred_start; /**< Do not start queue with rte_eth_dev_start(). */ > > + uint16_t rx_nseg; /**< Number of descriptions in rx_seg array. */ > > + /** > > + * The pointer to the array of segment descriptions, each element > > + * describes the memory pool, maximal segment data length, initial > > + * data offset from the beginning of data buffer in mbuf. This allow allow -> allows or maybe "This allows to specify" -> "This specifies" > > + * to specify the dedicated properties for each segment in the receiving > > + * buffer - pool, buffer offset, maximal segment size. The number of > > + * segment descriptions in the array is specified by the rx_nseg > > + * field. > > + */ > > What do you think providing a short description here, and move above comment > to abice "struct rte_eth_rxseg" struct? +1 > > > + struct rte_eth_rxseg *rx_seg; > > /** > > * Per-queue Rx offloads to be set using DEV_RX_OFFLOAD_* flags. > > * Only offloads set on rx_queue_offload_capa or rx_offload_capa > > * fields on rte_eth_dev_info structure are allowed to be set. > > */ > > uint64_t offloads; > > - > > unrelated > > > uint64_t reserved_64s[2]; /**< Reserved for future fields */ > > void *reserved_ptrs[2]; /**< Reserved for future fields */ > > }; > > @@ -1260,6 +1280,7 @@ struct rte_eth_conf { > > #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 > > #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 > > #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 > > +#define RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT 0x00100000 > > #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \ > > DEV_RX_OFFLOAD_UDP_CKSUM | \ > > @@ -2027,6 +2048,41 @@ int rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_queue, > > * No need to repeat any bit in rx_conf->offloads which has already been > > * enabled in rte_eth_dev_configure() at port level. An offloading enabled > > * at port level can't be disabled at queue level. > > Can it be possible to put a kind of marker here, like "@rx_seg & @rx_nseg", > to clarify what are you talking about. > > > + * The configuration structure also contains the pointer to the array > > + * of the receiving buffer segment descriptions, each element describes > > + * the memory pool, maximal segment data length, initial data offset from > > + * the beginning of data buffer in mbuf. This allow to specify the dedicated > > + * properties for each segment in the receiving buffer - pool, buffer > > + * offset, maximal segment size. If RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT offload > > + * flag is configured the PMD will split the received packets into multiple > > + * segments according to the specification in the description array: > > + * - the first network buffer will be allocated from the memory pool, > > + * specified in the first segment description element, the second > > + * network buffer - from the pool in the second segment description > > + * element and so on. If there is no enough elements to describe > > + * the buffer for entire packet of maximal length the pool from the last > > + * valid element will be used to allocate the buffers from for the rest > > + * of segments. > > + * - the offsets from the segment description elements will provide the > > + * data offset from the buffer beginning except the first mbuf - for this > > + * one the offset is added to the RTE_PKTMBUF_HEADROOM to get actual > > + * offset from the buffer beginning. If there is no enough elements > > + * to describe the buffer for entire packet of maximal length the offsets > > + * for the rest of segment will be supposed to be zero. > > + * - the data length being received to each segment is limited by the > > + * length specified in the segment description element. The data receiving > > + * starts with filling up the first mbuf data buffer, if the specified > > + * maximal segment length is reached and there are data remaining > > + * (packet is longer than buffer in the first mbuf) the following data > > + * will be pushed to the next segment up to its own length. If the first > > + * two segments is not enough to store all the packet data the next > > + * (third) segment will be engaged and so on. If the length in the segment > > + * description element is zero the actual buffer size will be deduced > > + * from the appropriate memory pool properties. If there is no enough > > + * elements to describe the buffer for entire packet of maximal length > > + * the buffer size will be deduced from the pool of the last valid > > + * element for the all remaining segments. > > + * > > I think as a first thing the comment should clarify that if @rx_seg provided > 'mb_pool' should be NULL, and if split Rx feature is not used "@rx_seg & > @rx_nseg" should be NULL and 0. > > Also above is too wordy, it is hard to follow. Like "@rx_seg & @rx_nseg" are > only taken into account if application provides > 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' offload should be clearer to see, etc. > Can you try to simplify it, perhpas moving some of above comments to the > "struct rte_eth_rxseg" can work? > > > * @param mb_pool > > * The pointer to the memory pool from which to allocate *rte_mbuf* network > > * memory buffers to populate each descriptor of the receive ring. > > diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map > > index f8a0945..25f7cee 100644 > > --- a/lib/librte_ethdev/rte_ethdev_version.map > > +++ b/lib/librte_ethdev/rte_ethdev_version.map > > @@ -232,6 +232,7 @@ EXPERIMENTAL { > > rte_eth_fec_get_capability; > > rte_eth_fec_get; > > rte_eth_fec_set; > > + > > unrelated