Hi, Andrew Thank you for the comments, please see my replies below.
> -----Original Message----- > From: Andrew Rybchenko <arybche...@solarflare.com> > Sent: Thursday, September 17, 2020 19:55 > To: Slava Ovsiienko <viachesl...@nvidia.com>; dev@dpdk.org > Cc: Thomas Monjalon <thom...@mellanox.com>; > step...@networkplumber.org; ferruh.yi...@intel.com; Shahaf Shuler > <shah...@nvidia.com>; olivier.m...@6wind.com; jerinjac...@gmail.com; > maxime.coque...@redhat.com; david.march...@redhat.com; Asaf Penso > <as...@nvidia.com> > Subject: Re: [dpdk-dev] [RFC] ethdev: introduce Rx buffer split > [snip] > > > > For example, let's suppose we configured the Rx queue with the > > following segments: > > seg0 - pool0, len0=14B, off0=RTE_PKTMBUF_HEADROOM > > seg1 - pool1, len1=20B, off1=0B > > 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 in mbuf from pool0 > > seg1 - 20B long @ 0 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 in mbuf from pool0 > > seg1 - 20B @ 0 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 behaviour is logical, but what to do if HW can't do it, i.e. use the last > segment many times. Should it reject configuration if provided segments are > insufficient to fit MTU packet? How to report the limitation? > (I'm still trying to convince that SCATTER and BUFFER_SPLIT should be > independent). BUFFER_SPLIT is rather the way to tune SCATTER. Currently scattering happens on unconditional mbuf data buffer boundaries (we have reserved HEAD space in the first mbuf and fill this one to the buffer end, the next mbuf buffers might be filled completely). BUFFER_SPLIT provides the way to specify the desired points to split packet, not just blindly follow buffer boundaries. There is the check inplemented in common part if each split segment fits the mbuf allocated from appropriate pool. PMD should do extra check internally whether it supports the requested split settings, if not - call will be rejected. [snip] > > I dislike the idea to introduce new device operation. > rte_eth_rxconf has reserved space and BUFFER_SPLIT offload will mean that > PMD looks at the split configuration location there. > We considered the approach of pushing split setting to the rxconf structure. [http://patches.dpdk.org/patch/75205/] But it seems there are some issues: - the split configuration description requires the variable length array (due to variations in number of segments), so rte_eth_rxconf structure would have the variable length (not nice, IMO). We could push pointers to the array of rte_eth_rxseg, but we would lost the single structure (and contiguous memory) simplicity, this approach has no advantages over the specifying the split configuration as parameters of setup_ex(). - it would introduces the ambiguity, rte_eth_rx_queue_setup() specifies the single mbuf pool as parameter. What should we do with it? Set to NULL? Treat as the first pool? I would prefer to specify all split segments in uniform fashion, i.e. as array or rte_eth_rxseg structures (and it can be easily updated with some extra segment attributes if needed). So, in my opinion, we should remove/replace the pool parameter in rx_queue_setup (by introducing new func). - specifying the new extended setup roiutine has an advantage that we should not update any PMDs code in part of existing implementations of rte_eth_rx_queue_setup(). If PMD supports BUFFER_SPLIT (or other related feature) it just should provide rte_eth_rx_queue_setup_ex() and check the DEV_RX_OFFLOAD_BUFFER_SPLIT (or HEADER_SPLIT, or ever feature) it supports. The common code does not check the feature flags - it is on PMDs' own. In order to configure PMD to perfrom arbitrary desired Rx spliting the application should check DEV_RX_OFFLOAD_BUFFER_SPLIT in port capabilites, if found - set DEV_RX_OFFLOAD_BUFFER_SPLIT in configuration and call rte_eth_rx_queue_setup_ex(). And this approach can be followed for any other split related feature. With best regards, Slava