[dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Viacheslav Ovsiienko]

The DPDK datapath in the transmit direction is very flexible.
The applications can build multisegment packets and manages
almost all data aspects - the memory pools where segments
are allocated from, the segment lengths, the memory attributes
like external, registered, etc.

In the receiving direction, the datapath is much less flexible,
the applications can only specify the memory pool to configure
the receiving queue and nothing more. In order to extend the
receiving datapath capabilities it is proposed to add the new
fields into rte_eth_rxconf structure:

struct rte_eth_rxconf {
    ...
    uint16_t rx_split_num; /* number of segments to split */
    uint16_t *rx_split_len; /* array of segment lengthes */
    struct rte_mempool **mp; /* array of segment memory pools */
    ...
};

The non-zero value of rx_split_num field configures the receiving
queue to split ingress packets into multiple segments to the mbufs
allocated from various memory pools according to the specified
lengths. The zero value of rx_split_num field provides the
backward compatibility and queue should be configured in a regular
way (with single/multiple mbufs of the same data buffer length
allocated from the single memory pool).

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 preserving compatibility with existing API.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index ea4cfa7..cd700ae 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -99,6 +99,11 @@ Deprecation Notices
   In 19.11 PMDs will still update the field even when the offload is not
   enabled.
 
+* ethdev: add new fields to ``rte_eth_rxconf`` to configure the receiving
+  queues to split ingress packets into multiple segments according to the
+  specified lengths into the buffers allocated from the specified
+  memory pools. The backward compatibility to existing API is preserved.
+
 * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
   will be deprecated in 20.11 and will be removed in 21.11.
   Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Jerin Jacob]

On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
<viacheslavo@mellanox.com> wrote:
>
> The DPDK datapath in the transmit direction is very flexible.
> The applications can build multisegment packets and manages
> almost all data aspects - the memory pools where segments
> are allocated from, the segment lengths, the memory attributes
> like external, registered, etc.
>
> In the receiving direction, the datapath is much less flexible,
> the applications can only specify the memory pool to configure
> the receiving queue and nothing more. In order to extend the
> receiving datapath capabilities it is proposed to add the new
> fields into rte_eth_rxconf structure:
>
> struct rte_eth_rxconf {
>     ...
>     uint16_t rx_split_num; /* number of segments to split */
>     uint16_t *rx_split_len; /* array of segment lengthes */
>     struct rte_mempool **mp; /* array of segment memory pools */

The pool has the packet length it's been configured for.
So I think, rx_split_len can be removed.

This feature also available in Marvell HW. So it not specific to one vendor.
Maybe we could just the use case mention the use case in the depreciation notice
and the tentative change in rte_eth_rxconf and exact details can be worked
out at the time of implementation.

With the above change:
Acked-by: Jerin Jacob <jerinj@marvell.com>


>     ...
> };
>
> The non-zero value of rx_split_num field configures the receiving
> queue to split ingress packets into multiple segments to the mbufs
> allocated from various memory pools according to the specified
> lengths. The zero value of rx_split_num field provides the
> backward compatibility and queue should be configured in a regular
> way (with single/multiple mbufs of the same data buffer length
> allocated from the single memory pool).
>
> 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 preserving compatibility with existing API.
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index ea4cfa7..cd700ae 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -99,6 +99,11 @@ Deprecation Notices
>    In 19.11 PMDs will still update the field even when the offload is not
>    enabled.
>
> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the receiving
> +  queues to split ingress packets into multiple segments according to the
> +  specified lengths into the buffers allocated from the specified
> +  memory pools. The backward compatibility to existing API is preserved.
> +
>  * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
>    will be deprecated in 20.11 and will be removed in 21.11.
>    Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``
> --
> 1.8.3.1
>

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

Hi, Jerin,

Thanks for the comment,  please, see below.

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, August 3, 2020 14:57
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Stephen
> Hemminger <stephen@networkplumber.org>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> David Marchand <david.marchand@redhat.com>
> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> 
> On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> <viacheslavo@mellanox.com> wrote:
> >
> > The DPDK datapath in the transmit direction is very flexible.
> > The applications can build multisegment packets and manages almost all
> > data aspects - the memory pools where segments are allocated from, the
> > segment lengths, the memory attributes like external, registered, etc.
> >
> > In the receiving direction, the datapath is much less flexible, the
> > applications can only specify the memory pool to configure the
> > receiving queue and nothing more. In order to extend the receiving
> > datapath capabilities it is proposed to add the new fields into
> > rte_eth_rxconf structure:
> >
> > struct rte_eth_rxconf {
> >     ...
> >     uint16_t rx_split_num; /* number of segments to split */
> >     uint16_t *rx_split_len; /* array of segment lengthes */
> >     struct rte_mempool **mp; /* array of segment memory pools */
> 
> The pool has the packet length it's been configured for.
> So I think, rx_split_len can be removed.

Yes, it is one of the supposed options - if pointer to array of segment lengths
is NULL , the queue_setup() could use the lengths from the pool's properties.
But we are talking about packet split, in general, it should not depend
on pool properties. What if application provides the single pool
and just wants to have the tunnel header in the first dedicated mbuf?

> 
> This feature also available in Marvell HW. So it not specific to one vendor.
> Maybe we could just the use case mention the use case in the depreciation
> notice and the tentative change in rte_eth_rxconf and exact details can be
> worked out at the time of implementation.
> 
So, if I understand correctly, the struct changes in the commit message
should be marked as just possible implementation?

With best regards,
Slava

> With the above change:
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> 
> 
> >     ...
> > };
> >
> > The non-zero value of rx_split_num field configures the receiving
> > queue to split ingress packets into multiple segments to the mbufs
> > allocated from various memory pools according to the specified
> > lengths. The zero value of rx_split_num field provides the backward
> > compatibility and queue should be configured in a regular way (with
> > single/multiple mbufs of the same data buffer length allocated from
> > the single memory pool).
> >
> > 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 preserving
> > compatibility with existing API.
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index ea4cfa7..cd700ae 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -99,6 +99,11 @@ Deprecation Notices
> >    In 19.11 PMDs will still update the field even when the offload is not
> >    enabled.
> >
> > +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
> > +receiving
> > +  queues to split ingress packets into multiple segments according to
> > +the
> > +  specified lengths into the buffers allocated from the specified
> > +  memory pools. The backward compatibility to existing API is preserved.
> > +
> >  * ethdev: ``rx_descriptor_done`` dev_ops and
> ``rte_eth_rx_descriptor_done``
> >    will be deprecated in 20.11 and will be removed in 21.11.
> >    Existing ``rte_eth_rx_descriptor_status`` and
> > ``rte_eth_tx_descriptor_status``
> > --
> > 1.8.3.1
> >

Re: [dpdk-dev] ***Spam*** [PATCH] doc: announce changes to ethdev rxconf structure

[Andrew Rybchenko]

On 8/3/20 1:58 PM, Viacheslav Ovsiienko wrote:
> The DPDK datapath in the transmit direction is very flexible.
> The applications can build multisegment packets and manages
> almost all data aspects - the memory pools where segments
> are allocated from, the segment lengths, the memory attributes
> like external, registered, etc.
>
> In the receiving direction, the datapath is much less flexible,
> the applications can only specify the memory pool to configure
> the receiving queue and nothing more. In order to extend the
> receiving datapath capabilities it is proposed to add the new
> fields into rte_eth_rxconf structure:
>
> struct rte_eth_rxconf {
>     ...
>     uint16_t rx_split_num; /* number of segments to split */
>     uint16_t *rx_split_len; /* array of segment lengthes */
>     struct rte_mempool **mp; /* array of segment memory pools */
>     ...
> };
>
> The non-zero value of rx_split_num field configures the receiving
> queue to split ingress packets into multiple segments to the mbufs
> allocated from various memory pools according to the specified
> lengths. The zero value of rx_split_num field provides the
> backward compatibility and queue should be configured in a regular
> way (with single/multiple mbufs of the same data buffer length
> allocated from the single memory pool).

From the above description it is not 100% clear how it will
coexist with:
 - existing mb_pool argument of the rte_eth_rx_queue_setup()
 - DEV_RX_OFFLOAD_SCATTER
 - DEV_RX_OFFLOAD_HEADER_SPLIT
How will application know that the feature is supported? Limitations?
Is it always split by specified/fixed length?
What happens if header length is actually different?

> 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 preserving compatibility with existing API.
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index ea4cfa7..cd700ae 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -99,6 +99,11 @@ Deprecation Notices
>    In 19.11 PMDs will still update the field even when the offload is not
>    enabled.
>  
> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the receiving
> +  queues to split ingress packets into multiple segments according to the
> +  specified lengths into the buffers allocated from the specified
> +  memory pools. The backward compatibility to existing API is preserved.
> +
>  * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
>    will be deprecated in 20.11 and will be removed in 21.11.
>    Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Jerin Jacob]

On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
>
> Hi, Jerin,
>
> Thanks for the comment,  please, see below.
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Monday, August 3, 2020 14:57
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> > Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Stephen
> > Hemminger <stephen@networkplumber.org>; Andrew Rybchenko
> > <arybchenko@solarflare.com>; Ajit Khaparde
> > <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> > David Marchand <david.marchand@redhat.com>
> > Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> >
> > On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> > <viacheslavo@mellanox.com> wrote:
> > >
> > > The DPDK datapath in the transmit direction is very flexible.
> > > The applications can build multisegment packets and manages almost all
> > > data aspects - the memory pools where segments are allocated from, the
> > > segment lengths, the memory attributes like external, registered, etc.
> > >
> > > In the receiving direction, the datapath is much less flexible, the
> > > applications can only specify the memory pool to configure the
> > > receiving queue and nothing more. In order to extend the receiving
> > > datapath capabilities it is proposed to add the new fields into
> > > rte_eth_rxconf structure:
> > >
> > > struct rte_eth_rxconf {
> > >     ...
> > >     uint16_t rx_split_num; /* number of segments to split */
> > >     uint16_t *rx_split_len; /* array of segment lengthes */
> > >     struct rte_mempool **mp; /* array of segment memory pools */
> >
> > The pool has the packet length it's been configured for.
> > So I think, rx_split_len can be removed.
>
> Yes, it is one of the supposed options - if pointer to array of segment lengths
> is NULL , the queue_setup() could use the lengths from the pool's properties.
> But we are talking about packet split, in general, it should not depend
> on pool properties. What if application provides the single pool
> and just wants to have the tunnel header in the first dedicated mbuf?
>
> >
> > This feature also available in Marvell HW. So it not specific to one vendor.
> > Maybe we could just the use case mention the use case in the depreciation
> > notice and the tentative change in rte_eth_rxconf and exact details can be
> > worked out at the time of implementation.
> >
> So, if I understand correctly, the struct changes in the commit message
> should be marked as just possible implementation?

Yes.

We may need to have a detailed discussion on the correct abstraction for various
HW is available with this feature.

On Marvell HW, We can configure TWO pools for given eth Rx queue.
One pool can be configured as a small packet pool and other one as
large packet pool.
And there is a threshold value to decide the pool between small and large.
For example:
- The small pool is configured 2k
- The large pool is configured with 10k
- And if the threshold value is configured as 2k.
Any packet size <=2K will land in small pool and others in a large pool.
The use case, we are targeting is to save the memory space for jumbo frames.

If you can share the MLX HW working model, Then we can find the
correct abstraction.

>
> With best regards,
> Slava
>
> > With the above change:
> > Acked-by: Jerin Jacob <jerinj@marvell.com>
> >
> >
> > >     ...
> > > };
> > >
> > > The non-zero value of rx_split_num field configures the receiving
> > > queue to split ingress packets into multiple segments to the mbufs
> > > allocated from various memory pools according to the specified
> > > lengths. The zero value of rx_split_num field provides the backward
> > > compatibility and queue should be configured in a regular way (with
> > > single/multiple mbufs of the same data buffer length allocated from
> > > the single memory pool).
> > >
> > > 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 preserving
> > > compatibility with existing API.
> > >
> > > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > > ---
> > >  doc/guides/rel_notes/deprecation.rst | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > b/doc/guides/rel_notes/deprecation.rst
> > > index ea4cfa7..cd700ae 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -99,6 +99,11 @@ Deprecation Notices
> > >    In 19.11 PMDs will still update the field even when the offload is not
> > >    enabled.
> > >
> > > +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
> > > +receiving
> > > +  queues to split ingress packets into multiple segments according to
> > > +the
> > > +  specified lengths into the buffers allocated from the specified
> > > +  memory pools. The backward compatibility to existing API is preserved.
> > > +
> > >  * ethdev: ``rx_descriptor_done`` dev_ops and
> > ``rte_eth_rx_descriptor_done``
> > >    will be deprecated in 20.11 and will be removed in 21.11.
> > >    Existing ``rte_eth_rx_descriptor_status`` and
> > > ``rte_eth_tx_descriptor_status``
> > > --
> > > 1.8.3.1
> > >

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, August 4, 2020 16:33
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Stephen
> Hemminger <stephen@networkplumber.org>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> David Marchand <david.marchand@redhat.com>
> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> 
> On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko
> <viacheslavo@mellanox.com> wrote:
> >
> > Hi, Jerin,
> >
> > Thanks for the comment,  please, see below.
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Monday, August 3, 2020 14:57
> > > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > > Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> > > Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> > > Stephen Hemminger <stephen@networkplumber.org>; Andrew
> Rybchenko
> > > <arybchenko@solarflare.com>; Ajit Khaparde
> > > <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > > <maxime.coquelin@redhat.com>; Olivier Matz
> <olivier.matz@6wind.com>;
> > > David Marchand <david.marchand@redhat.com>
> > > Subject: Re: [PATCH] doc: announce changes to ethdev rxconf
> > > structure
> > >
> > > On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> > > <viacheslavo@mellanox.com> wrote:
> > > >
> > > > The DPDK datapath in the transmit direction is very flexible.
> > > > The applications can build multisegment packets and manages almost
> > > > all data aspects - the memory pools where segments are allocated
> > > > from, the segment lengths, the memory attributes like external,
> registered, etc.
> > > >
> > > > In the receiving direction, the datapath is much less flexible,
> > > > the applications can only specify the memory pool to configure the
> > > > receiving queue and nothing more. In order to extend the receiving
> > > > datapath capabilities it is proposed to add the new fields into
> > > > rte_eth_rxconf structure:
> > > >
> > > > struct rte_eth_rxconf {
> > > >     ...
> > > >     uint16_t rx_split_num; /* number of segments to split */
> > > >     uint16_t *rx_split_len; /* array of segment lengthes */
> > > >     struct rte_mempool **mp; /* array of segment memory pools */
> > >
> > > The pool has the packet length it's been configured for.
> > > So I think, rx_split_len can be removed.
> >
> > Yes, it is one of the supposed options - if pointer to array of
> > segment lengths is NULL , the queue_setup() could use the lengths from the
> pool's properties.
> > But we are talking about packet split, in general, it should not
> > depend on pool properties. What if application provides the single
> > pool and just wants to have the tunnel header in the first dedicated mbuf?
> >
> > >
> > > This feature also available in Marvell HW. So it not specific to one
> vendor.
> > > Maybe we could just the use case mention the use case in the
> > > depreciation notice and the tentative change in rte_eth_rxconf and
> > > exact details can be worked out at the time of implementation.
> > >
> > So, if I understand correctly, the struct changes in the commit
> > message should be marked as just possible implementation?
> 
> Yes.
> 
> We may need to have a detailed discussion on the correct abstraction for
> various HW is available with this feature.
> 
> On Marvell HW, We can configure TWO pools for given eth Rx queue.
> One pool can be configured as a small packet pool and other one as large
> packet pool.
> And there is a threshold value to decide the pool between small and large.
> For example:
> - The small pool is configured 2k
> - The large pool is configured with 10k
> - And if the threshold value is configured as 2k.
> Any packet size <=2K will land in small pool and others in a large pool.
> The use case, we are targeting is to save the memory space for jumbo
> frames.

It is a little bit different than split feature, it is about somehow packet smart sorting.
"Buffer split" is just about more flexible description of rx buffers. Currently
the rx buffers can only be the chain of the buffers of the same size and
allocated from the same memory pool. It is simple and not versatile way,
we could extend this.

Of course, there is no any objection against sharing this split Rx buffer description
with other features, but, for the example above (2k/10k) it is only the part, it
would require some other parameters (threshold) not used by split. Yes, let's
discuss.

> 
> If you can share the MLX HW working model, Then we can find the correct
> abstraction.
From MLNX HW point of view buffer split feature does require nothing special.
The HW rx buffer descriptors support flexible buffer formats, there is
no problem to specify the chain of mbufs with different sizes and dedicated
pointers to receive and split packet into by hardware.


With best regards,
Slava
> >
> > > With the above change:
> > > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > >
> > >
> > > >     ...
> > > > };
> > > >
> > > > The non-zero value of rx_split_num field configures the receiving
> > > > queue to split ingress packets into multiple segments to the mbufs
> > > > allocated from various memory pools according to the specified
> > > > lengths. The zero value of rx_split_num field provides the
> > > > backward compatibility and queue should be configured in a regular
> > > > way (with single/multiple mbufs of the same data buffer length
> > > > allocated from the single memory pool).
> > > >
> > > > 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
> > > > preserving compatibility with existing API.
> > > >
> > > > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > > > ---
> > > >  doc/guides/rel_notes/deprecation.rst | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > b/doc/guides/rel_notes/deprecation.rst
> > > > index ea4cfa7..cd700ae 100644
> > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > @@ -99,6 +99,11 @@ Deprecation Notices
> > > >    In 19.11 PMDs will still update the field even when the offload is not
> > > >    enabled.
> > > >
> > > > +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
> > > > +receiving
> > > > +  queues to split ingress packets into multiple segments
> > > > +according to the
> > > > +  specified lengths into the buffers allocated from the specified
> > > > +  memory pools. The backward compatibility to existing API is
> preserved.
> > > > +
> > > >  * ethdev: ``rx_descriptor_done`` dev_ops and
> > > ``rte_eth_rx_descriptor_done``
> > > >    will be deprecated in 20.11 and will be removed in 21.11.
> > > >    Existing ``rte_eth_rx_descriptor_status`` and
> > > > ``rte_eth_tx_descriptor_status``
> > > > --
> > > > 1.8.3.1
> > > >

[dpdk-dev] [PATCH v2] doc: announce changes to ethdev rxconf structure

[Viacheslav Ovsiienko]

The DPDK datapath in the transmit direction is very flexible.
The applications can build multi-segment packets and manages
almost all data aspects - the memory pools where segments
are allocated from, the segment lengths, the memory attributes
like external, registered, etc.

In the receiving direction, the datapath is much less flexible,
the applications can only specify the memory pool to configure
the receiving queue and nothing more. The packet being received
can only be pushed to the chain of the mbufs of the same data
buffer size and allocated from the same pool. In order to extend
the receiving datapath buffer description it is proposed to add
the new fields into rte_eth_rxconf structure:

struct rte_eth_rxconf {
    ...
    uint16_t rx_split_num; /* number of segments to split */
    uint16_t *rx_split_len; /* array of segment lengths */
    struct rte_mempool **mp; /* array of segment memory pools */
    ...
};

The non-zero value of rx_split_num field configures the receiving
queue to split ingress packets into multiple segments to the mbufs
allocated from various memory pools according to the specified
lengths. The zero value of rx_split_num field provides the
backward compatibility and queue should be configured in a regular
way (with single/multiple mbufs of the same data buffer length
allocated from the single memory pool).

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 preserving compatibility with existing API.

The proposed extended description of receiving buffers might be
considered by other vendors to be involved into similar features
support, it is the subject for the further discussion.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Acked-by: Jerin Jacob <jerinjacobk@gmail.com>

---
v1->v2: commit message updated, proposed to consider the new
        fields for supporting similar features by multiple
	vendors
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index acf87d3..b6bdb83 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -99,6 +99,11 @@ Deprecation Notices
   In 19.11 PMDs will still update the field even when the offload is not
   enabled.
 
+* ethdev: add new fields to ``rte_eth_rxconf`` to configure the receiving
+  queues to split ingress packets into multiple segments according to the
+  specified lengths into the buffers allocated from the specified
+  memory pools. The backward compatibility to existing API is preserved.
+
 * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
   will be deprecated in 20.11 and will be removed in 21.11.
   Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2] doc: announce changes to ethdev rxconf structure

[Andrew Rybchenko]

On 8/5/20 11:49 AM, Viacheslav Ovsiienko wrote:
> The DPDK datapath in the transmit direction is very flexible.
> The applications can build multi-segment packets and manages
> almost all data aspects - the memory pools where segments
> are allocated from, the segment lengths, the memory attributes
> like external, registered, etc.
> 
> In the receiving direction, the datapath is much less flexible,
> the applications can only specify the memory pool to configure
> the receiving queue and nothing more. The packet being received
> can only be pushed to the chain of the mbufs of the same data
> buffer size and allocated from the same pool. In order to extend
> the receiving datapath buffer description it is proposed to add
> the new fields into rte_eth_rxconf structure:
> 
> struct rte_eth_rxconf {
>     ...
>     uint16_t rx_split_num; /* number of segments to split */
>     uint16_t *rx_split_len; /* array of segment lengths */
>     struct rte_mempool **mp; /* array of segment memory pools */
>     ...
> };
> 
> The non-zero value of rx_split_num field configures the receiving
> queue to split ingress packets into multiple segments to the mbufs
> allocated from various memory pools according to the specified
> lengths. The zero value of rx_split_num field provides the
> backward compatibility and queue should be configured in a regular
> way (with single/multiple mbufs of the same data buffer length
> allocated from the single memory pool).
> 
> 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 preserving compatibility with existing API.
> 
> The proposed extended description of receiving buffers might be
> considered by other vendors to be involved into similar features
> support, it is the subject for the further discussion.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Acked-by: Jerin Jacob <jerinjacobk@gmail.com>

I"m OK with the idea in general and we'll work on details
in the next release cycle.

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Re: [dpdk-dev] [PATCH v2] doc: announce changes to ethdev rxconf structure

[Thomas Monjalon]

05/08/2020 13:14, Andrew Rybchenko:
> On 8/5/20 11:49 AM, Viacheslav Ovsiienko wrote:
> > The DPDK datapath in the transmit direction is very flexible.
> > The applications can build multi-segment packets and manages
> > almost all data aspects - the memory pools where segments
> > are allocated from, the segment lengths, the memory attributes
> > like external, registered, etc.
> > 
> > In the receiving direction, the datapath is much less flexible,
> > the applications can only specify the memory pool to configure
> > the receiving queue and nothing more. The packet being received
> > can only be pushed to the chain of the mbufs of the same data
> > buffer size and allocated from the same pool. In order to extend
> > the receiving datapath buffer description it is proposed to add
> > the new fields into rte_eth_rxconf structure:
> > 
> > struct rte_eth_rxconf {
> >     ...
> >     uint16_t rx_split_num; /* number of segments to split */
> >     uint16_t *rx_split_len; /* array of segment lengths */
> >     struct rte_mempool **mp; /* array of segment memory pools */
> >     ...
> > };
> > 
> > The non-zero value of rx_split_num field configures the receiving
> > queue to split ingress packets into multiple segments to the mbufs
> > allocated from various memory pools according to the specified
> > lengths. The zero value of rx_split_num field provides the
> > backward compatibility and queue should be configured in a regular
> > way (with single/multiple mbufs of the same data buffer length
> > allocated from the single memory pool).
> > 
> > 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 preserving compatibility with existing API.
> > 
> > The proposed extended description of receiving buffers might be
> > considered by other vendors to be involved into similar features
> > support, it is the subject for the further discussion.
> > 
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > Acked-by: Jerin Jacob <jerinjacobk@gmail.com>
> 
> I"m OK with the idea in general and we'll work on details
> in the next release cycle.
> 
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

I agree we need to be more flexible with the mempools in Rx.

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Ferruh Yigit]

On 8/4/2020 2:32 PM, Jerin Jacob wrote:
> On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
>>
>> Hi, Jerin,
>>
>> Thanks for the comment,  please, see below.
>>
>>> -----Original Message-----
>>> From: Jerin Jacob <jerinjacobk@gmail.com>
>>> Sent: Monday, August 3, 2020 14:57
>>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
>>> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Stephen
>>> Hemminger <stephen@networkplumber.org>; Andrew Rybchenko
>>> <arybchenko@solarflare.com>; Ajit Khaparde
>>> <ajit.khaparde@broadcom.com>; Maxime Coquelin
>>> <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
>>> David Marchand <david.marchand@redhat.com>
>>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
>>>
>>> On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
>>> <viacheslavo@mellanox.com> wrote:
>>>>
>>>> The DPDK datapath in the transmit direction is very flexible.
>>>> The applications can build multisegment packets and manages almost all
>>>> data aspects - the memory pools where segments are allocated from, the
>>>> segment lengths, the memory attributes like external, registered, etc.
>>>>
>>>> In the receiving direction, the datapath is much less flexible, the
>>>> applications can only specify the memory pool to configure the
>>>> receiving queue and nothing more. In order to extend the receiving
>>>> datapath capabilities it is proposed to add the new fields into
>>>> rte_eth_rxconf structure:
>>>>
>>>> struct rte_eth_rxconf {
>>>>     ...
>>>>     uint16_t rx_split_num; /* number of segments to split */
>>>>     uint16_t *rx_split_len; /* array of segment lengthes */
>>>>     struct rte_mempool **mp; /* array of segment memory pools */
>>>
>>> The pool has the packet length it's been configured for.
>>> So I think, rx_split_len can be removed.
>>
>> Yes, it is one of the supposed options - if pointer to array of segment lengths
>> is NULL , the queue_setup() could use the lengths from the pool's properties.
>> But we are talking about packet split, in general, it should not depend
>> on pool properties. What if application provides the single pool
>> and just wants to have the tunnel header in the first dedicated mbuf?
>>
>>>
>>> This feature also available in Marvell HW. So it not specific to one vendor.
>>> Maybe we could just the use case mention the use case in the depreciation
>>> notice and the tentative change in rte_eth_rxconf and exact details can be
>>> worked out at the time of implementation.
>>>
>> So, if I understand correctly, the struct changes in the commit message
>> should be marked as just possible implementation?
> 
> Yes.
> 
> We may need to have a detailed discussion on the correct abstraction for various
> HW is available with this feature.
> 
> On Marvell HW, We can configure TWO pools for given eth Rx queue.
> One pool can be configured as a small packet pool and other one as
> large packet pool.
> And there is a threshold value to decide the pool between small and large.
> For example:
> - The small pool is configured 2k
> - The large pool is configured with 10k
> - And if the threshold value is configured as 2k.
> Any packet size <=2K will land in small pool and others in a large pool.
> The use case, we are targeting is to save the memory space for jumbo frames.

Out of curiosity, do you provide two different buffer address in the descriptor
and HW automatically uses one based on the size,
or driver uses one of the pools based on the configuration and possible largest
packet size?

> 
> If you can share the MLX HW working model, Then we can find the
> correct abstraction.
> 
>>
>> With best regards,
>> Slava
>>
>>> With the above change:
>>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>>>
>>>
>>>>     ...
>>>> };
>>>>
>>>> The non-zero value of rx_split_num field configures the receiving
>>>> queue to split ingress packets into multiple segments to the mbufs
>>>> allocated from various memory pools according to the specified
>>>> lengths. The zero value of rx_split_num field provides the backward
>>>> compatibility and queue should be configured in a regular way (with
>>>> single/multiple mbufs of the same data buffer length allocated from
>>>> the single memory pool).
>>>>
>>>> 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 preserving
>>>> compatibility with existing API.
>>>>
>>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>>>> ---
>>>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>> b/doc/guides/rel_notes/deprecation.rst
>>>> index ea4cfa7..cd700ae 100644
>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>> @@ -99,6 +99,11 @@ Deprecation Notices
>>>>    In 19.11 PMDs will still update the field even when the offload is not
>>>>    enabled.
>>>>
>>>> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
>>>> +receiving
>>>> +  queues to split ingress packets into multiple segments according to
>>>> +the
>>>> +  specified lengths into the buffers allocated from the specified
>>>> +  memory pools. The backward compatibility to existing API is preserved.
>>>> +
>>>>  * ethdev: ``rx_descriptor_done`` dev_ops and
>>> ``rte_eth_rx_descriptor_done``
>>>>    will be deprecated in 20.11 and will be removed in 21.11.
>>>>    Existing ``rte_eth_rx_descriptor_status`` and
>>>> ``rte_eth_tx_descriptor_status``
>>>> --
>>>> 1.8.3.1
>>>>

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Ferruh Yigit]

On 8/3/2020 3:31 PM, Andrew Rybchenko wrote:
> On 8/3/20 1:58 PM, Viacheslav Ovsiienko wrote:
>> The DPDK datapath in the transmit direction is very flexible.
>> The applications can build multisegment packets and manages
>> almost all data aspects - the memory pools where segments
>> are allocated from, the segment lengths, the memory attributes
>> like external, registered, etc.
>>
>> In the receiving direction, the datapath is much less flexible,
>> the applications can only specify the memory pool to configure
>> the receiving queue and nothing more. In order to extend the
>> receiving datapath capabilities it is proposed to add the new
>> fields into rte_eth_rxconf structure:
>>
>> struct rte_eth_rxconf {
>>     ...
>>     uint16_t rx_split_num; /* number of segments to split */
>>     uint16_t *rx_split_len; /* array of segment lengthes */
>>     struct rte_mempool **mp; /* array of segment memory pools */
>>     ...
>> };
>>
>> The non-zero value of rx_split_num field configures the receiving
>> queue to split ingress packets into multiple segments to the mbufs
>> allocated from various memory pools according to the specified
>> lengths. The zero value of rx_split_num field provides the
>> backward compatibility and queue should be configured in a regular
>> way (with single/multiple mbufs of the same data buffer length
>> allocated from the single memory pool).
> 
> From the above description it is not 100% clear how it will
> coexist with:
>  - existing mb_pool argument of the rte_eth_rx_queue_setup()

+1

>  - DEV_RX_OFFLOAD_SCATTER
>  - DEV_RX_OFFLOAD_HEADER_SPLIT
> How will application know that the feature is supported? Limitations?

+1

> Is it always split by specified/fixed length?
> What happens if header length is actually different?

As far as I understand intention is to filter specific packets to a queue first
and later do the split, so the header length will be fixed...

> 
>> 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 preserving compatibility with existing API.
>>
>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>> ---
>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>> index ea4cfa7..cd700ae 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -99,6 +99,11 @@ Deprecation Notices
>>    In 19.11 PMDs will still update the field even when the offload is not
>>    enabled.
>>  
>> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the receiving
>> +  queues to split ingress packets into multiple segments according to the
>> +  specified lengths into the buffers allocated from the specified
>> +  memory pools. The backward compatibility to existing API is preserved.
>> +
>>  * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
>>    will be deprecated in 20.11 and will be removed in 21.11.
>>    Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``
> 

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Stephen Hemminger]

On Thu, 6 Aug 2020 16:58:22 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 8/4/2020 2:32 PM, Jerin Jacob wrote:
> > On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko <viacheslavo@mellanox.com> wrote:  
> >>
> >> Hi, Jerin,
> >>
> >> Thanks for the comment,  please, see below.
> >>  
> >>> -----Original Message-----
> >>> From: Jerin Jacob <jerinjacobk@gmail.com>
> >>> Sent: Monday, August 3, 2020 14:57
> >>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> >>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> >>> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> >>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Stephen
> >>> Hemminger <stephen@networkplumber.org>; Andrew Rybchenko
> >>> <arybchenko@solarflare.com>; Ajit Khaparde
> >>> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> >>> <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> >>> David Marchand <david.marchand@redhat.com>
> >>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> >>>
> >>> On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> >>> <viacheslavo@mellanox.com> wrote:  
> >>>>
> >>>> The DPDK datapath in the transmit direction is very flexible.
> >>>> The applications can build multisegment packets and manages almost all
> >>>> data aspects - the memory pools where segments are allocated from, the
> >>>> segment lengths, the memory attributes like external, registered, etc.
> >>>>
> >>>> In the receiving direction, the datapath is much less flexible, the
> >>>> applications can only specify the memory pool to configure the
> >>>> receiving queue and nothing more. In order to extend the receiving
> >>>> datapath capabilities it is proposed to add the new fields into
> >>>> rte_eth_rxconf structure:
> >>>>
> >>>> struct rte_eth_rxconf {
> >>>>     ...
> >>>>     uint16_t rx_split_num; /* number of segments to split */
> >>>>     uint16_t *rx_split_len; /* array of segment lengthes */
> >>>>     struct rte_mempool **mp; /* array of segment memory pools */  
> >>>
> >>> The pool has the packet length it's been configured for.
> >>> So I think, rx_split_len can be removed.  
> >>
> >> Yes, it is one of the supposed options - if pointer to array of segment lengths
> >> is NULL , the queue_setup() could use the lengths from the pool's properties.
> >> But we are talking about packet split, in general, it should not depend
> >> on pool properties. What if application provides the single pool
> >> and just wants to have the tunnel header in the first dedicated mbuf?
> >>  
> >>>
> >>> This feature also available in Marvell HW. So it not specific to one vendor.
> >>> Maybe we could just the use case mention the use case in the depreciation
> >>> notice and the tentative change in rte_eth_rxconf and exact details can be
> >>> worked out at the time of implementation.
> >>>  
> >> So, if I understand correctly, the struct changes in the commit message
> >> should be marked as just possible implementation?  
> > 
> > Yes.
> > 
> > We may need to have a detailed discussion on the correct abstraction for various
> > HW is available with this feature.
> > 
> > On Marvell HW, We can configure TWO pools for given eth Rx queue.
> > One pool can be configured as a small packet pool and other one as
> > large packet pool.
> > And there is a threshold value to decide the pool between small and large.
> > For example:
> > - The small pool is configured 2k
> > - The large pool is configured with 10k
> > - And if the threshold value is configured as 2k.
> > Any packet size <=2K will land in small pool and others in a large pool.
> > The use case, we are targeting is to save the memory space for jumbo frames.  
> 
> Out of curiosity, do you provide two different buffer address in the descriptor
> and HW automatically uses one based on the size,
> or driver uses one of the pools based on the configuration and possible largest
> packet size?

I am all for allowing more configuration of buffer pool.
But don't want that to be exposed as a hardware specific requirement in the
API for applications. The worst case would be if your API changes required:

  if (strcmp(dev->driver_name, "marvell") == 0) {
     // make another mempool for this driver

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, August 6, 2020 19:16
> To: Andrew Rybchenko <arybchenko@solarflare.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> jerinjacobk@gmail.com; stephen@networkplumber.org;
> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> olivier.matz@6wind.com; david.marchand@redhat.com
> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> 
> On 8/3/2020 3:31 PM, Andrew Rybchenko wrote:
> > On 8/3/20 1:58 PM, Viacheslav Ovsiienko wrote:
> >> The DPDK datapath in the transmit direction is very flexible.
> >> The applications can build multisegment packets and manages almost
> >> all data aspects - the memory pools where segments are allocated
> >> from, the segment lengths, the memory attributes like external,
> >> registered, etc.
> >>
> >> In the receiving direction, the datapath is much less flexible, the
> >> applications can only specify the memory pool to configure the
> >> receiving queue and nothing more. In order to extend the receiving
> >> datapath capabilities it is proposed to add the new fields into
> >> rte_eth_rxconf structure:
> >>
> >> struct rte_eth_rxconf {
> >>     ...
> >>     uint16_t rx_split_num; /* number of segments to split */
> >>     uint16_t *rx_split_len; /* array of segment lengthes */
> >>     struct rte_mempool **mp; /* array of segment memory pools */
> >>     ...
> >> };
> >>
> >> The non-zero value of rx_split_num field configures the receiving
> >> queue to split ingress packets into multiple segments to the mbufs
> >> allocated from various memory pools according to the specified
> >> lengths. The zero value of rx_split_num field provides the backward
> >> compatibility and queue should be configured in a regular way (with
> >> single/multiple mbufs of the same data buffer length allocated from
> >> the single memory pool).
> >
> > From the above description it is not 100% clear how it will coexist
> > with:
> >  - existing mb_pool argument of the rte_eth_rx_queue_setup()
> 
> +1
- supposed to be NULL if the array of lengths/pools is used

> 
> >  - DEV_RX_OFFLOAD_SCATTER
> >  - DEV_RX_OFFLOAD_HEADER_SPLIT
> > How will application know that the feature is supported? Limitations?
> 
> +1
New flag  DEV_RX_OFFLOAD_BUFFER_SPLIT is supposed to be introduced.
The feature requires the DEV_RX_OFFLOAD_SCATTER is set.
If DEV_RX_OFFLOAD_HEADER_SPLIT is set the error is returned.

> 
> > Is it always split by specified/fixed length?
> > What happens if header length is actually different?
> 
> As far as I understand intention is to filter specific packets to a queue first
> and later do the split, so the header length will be fixed...

Not exactly. The filtering should be handled by rte_flow engine.
The intention is to provide the more flexible way to describe
rx buffers. Currently it is the single pool with fixed size segments. No way to
split the packet into multiple segments with specified lengths and in
the specified pools. What if packet payload should be stored in the physical
memory on other device (GPU/Storage)? What if caching is not desired for
the payload (just forwarding application)? We could provide the special NC pool.
What if packet should be split into the chunks with specific gaps?
For Tx direction we have this opportunity to gather packet from various
pools and any desired combinations , but Rx is much less flexible.
 
> >
> >> 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 preserving
> >> compatibility with existing API.
> >>
> >> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> >> ---
> >>  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/doc/guides/rel_notes/deprecation.rst
> >> b/doc/guides/rel_notes/deprecation.rst
> >> index ea4cfa7..cd700ae 100644
> >> --- a/doc/guides/rel_notes/deprecation.rst
> >> +++ b/doc/guides/rel_notes/deprecation.rst
> >> @@ -99,6 +99,11 @@ Deprecation Notices
> >>    In 19.11 PMDs will still update the field even when the offload is not
> >>    enabled.
> >>
> >> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
> >> +receiving
> >> +  queues to split ingress packets into multiple segments according
> >> +to the
> >> +  specified lengths into the buffers allocated from the specified
> >> +  memory pools. The backward compatibility to existing API is preserved.
> >> +
> >>  * ethdev: ``rx_descriptor_done`` dev_ops and
> ``rte_eth_rx_descriptor_done``
> >>    will be deprecated in 20.11 and will be removed in 21.11.
> >>    Existing ``rte_eth_rx_descriptor_status`` and
> >> ``rte_eth_tx_descriptor_status``
> >

Re: [dpdk-dev] [PATCH v2] doc: announce changes to ethdev rxconf structure

[Ferruh Yigit]

On 8/5/2020 9:49 AM, Viacheslav Ovsiienko wrote:
> The DPDK datapath in the transmit direction is very flexible.
> The applications can build multi-segment packets and manages
> almost all data aspects - the memory pools where segments
> are allocated from, the segment lengths, the memory attributes
> like external, registered, etc.
> 
> In the receiving direction, the datapath is much less flexible,
> the applications can only specify the memory pool to configure
> the receiving queue and nothing more. The packet being received
> can only be pushed to the chain of the mbufs of the same data
> buffer size and allocated from the same pool. In order to extend
> the receiving datapath buffer description it is proposed to add
> the new fields into rte_eth_rxconf structure:
> 
> struct rte_eth_rxconf {
>     ...
>     uint16_t rx_split_num; /* number of segments to split */
>     uint16_t *rx_split_len; /* array of segment lengths */
>     struct rte_mempool **mp; /* array of segment memory pools */
>     ...
> };

What is the way to say first 14 bytes will go first mempool and rest will go
second one?
Or do you have to define fixed sizes for all segments?
What if that 'rest' part larger than given buffer size for that mempool?

Intel NICs also has header split support, similar to what Jerin described,
header and data goes to different buffers, which doesn't require fixed sizes and
need only two mempools, not sure if it should be integrated to this feature but
we can discuss later.

Also there are some valid concerns Andrew highlighted, like how application will
know if PMD supports this feature etc.. and more.
But since these are design/implementation related concerns, not a blocker for
deprecation notice I think, overall no objection to config structure change, hence:

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

> 
> The non-zero value of rx_split_num field configures the receiving
> queue to split ingress packets into multiple segments to the mbufs
> allocated from various memory pools according to the specified
> lengths. The zero value of rx_split_num field provides the
> backward compatibility and queue should be configured in a regular
> way (with single/multiple mbufs of the same data buffer length
> allocated from the single memory pool).
> 
> 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 preserving compatibility with existing API.
> 
> The proposed extended description of receiving buffers might be
> considered by other vendors to be involved into similar features
> support, it is the subject for the further discussion.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Acked-by: Jerin Jacob <jerinjacobk@gmail.com>
> 
> ---
> v1->v2: commit message updated, proposed to consider the new
>         fields for supporting similar features by multiple
> 	vendors
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index acf87d3..b6bdb83 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -99,6 +99,11 @@ Deprecation Notices
>    In 19.11 PMDs will still update the field even when the offload is not
>    enabled.
>  
> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the receiving
> +  queues to split ingress packets into multiple segments according to the
> +  specified lengths into the buffers allocated from the specified
> +  memory pools. The backward compatibility to existing API is preserved.
> +
>  * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
>    will be deprecated in 20.11 and will be removed in 21.11.
>    Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``
> 

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Ferruh Yigit]

On 8/6/2020 5:29 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, August 6, 2020 19:16
>> To: Andrew Rybchenko <arybchenko@solarflare.com>; Slava Ovsiienko
>> <viacheslavo@mellanox.com>; dev@dpdk.org
>> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
>> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>> jerinjacobk@gmail.com; stephen@networkplumber.org;
>> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
>> olivier.matz@6wind.com; david.marchand@redhat.com
>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
>>
>> On 8/3/2020 3:31 PM, Andrew Rybchenko wrote:
>>> On 8/3/20 1:58 PM, Viacheslav Ovsiienko wrote:
>>>> The DPDK datapath in the transmit direction is very flexible.
>>>> The applications can build multisegment packets and manages almost
>>>> all data aspects - the memory pools where segments are allocated
>>>> from, the segment lengths, the memory attributes like external,
>>>> registered, etc.
>>>>
>>>> In the receiving direction, the datapath is much less flexible, the
>>>> applications can only specify the memory pool to configure the
>>>> receiving queue and nothing more. In order to extend the receiving
>>>> datapath capabilities it is proposed to add the new fields into
>>>> rte_eth_rxconf structure:
>>>>
>>>> struct rte_eth_rxconf {
>>>>     ...
>>>>     uint16_t rx_split_num; /* number of segments to split */
>>>>     uint16_t *rx_split_len; /* array of segment lengthes */
>>>>     struct rte_mempool **mp; /* array of segment memory pools */
>>>>     ...
>>>> };
>>>>
>>>> The non-zero value of rx_split_num field configures the receiving
>>>> queue to split ingress packets into multiple segments to the mbufs
>>>> allocated from various memory pools according to the specified
>>>> lengths. The zero value of rx_split_num field provides the backward
>>>> compatibility and queue should be configured in a regular way (with
>>>> single/multiple mbufs of the same data buffer length allocated from
>>>> the single memory pool).
>>>
>>> From the above description it is not 100% clear how it will coexist
>>> with:
>>>  - existing mb_pool argument of the rte_eth_rx_queue_setup()
>>
>> +1
> - supposed to be NULL if the array of lengths/pools is used
> 
>>
>>>  - DEV_RX_OFFLOAD_SCATTER
>>>  - DEV_RX_OFFLOAD_HEADER_SPLIT
>>> How will application know that the feature is supported? Limitations?
>>
>> +1
> New flag  DEV_RX_OFFLOAD_BUFFER_SPLIT is supposed to be introduced.
> The feature requires the DEV_RX_OFFLOAD_SCATTER is set.
> If DEV_RX_OFFLOAD_HEADER_SPLIT is set the error is returned.
> 
>>
>>> Is it always split by specified/fixed length?
>>> What happens if header length is actually different?
>>
>> As far as I understand intention is to filter specific packets to a queue first
>> and later do the split, so the header length will be fixed...
> 
> Not exactly. The filtering should be handled by rte_flow engine.
> The intention is to provide the more flexible way to describe
> rx buffers. Currently it is the single pool with fixed size segments. No way to
> split the packet into multiple segments with specified lengths and in
> the specified pools. What if packet payload should be stored in the physical
> memory on other device (GPU/Storage)? What if caching is not desired for
> the payload (just forwarding application)? We could provide the special NC pool.
> What if packet should be split into the chunks with specific gaps?
> For Tx direction we have this opportunity to gather packet from various
> pools and any desired combinations , but Rx is much less flexible.
>  
>>>
>>>> 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 preserving
>>>> compatibility with existing API.

If you don't know the packet types in advance, how can you use fixed sizes to
split a packet? Won't it be like having random parts of packet in each mempool..

>>>>
>>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>>>> ---
>>>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>> b/doc/guides/rel_notes/deprecation.rst
>>>> index ea4cfa7..cd700ae 100644
>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>> @@ -99,6 +99,11 @@ Deprecation Notices
>>>>    In 19.11 PMDs will still update the field even when the offload is not
>>>>    enabled.
>>>>
>>>> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
>>>> +receiving
>>>> +  queues to split ingress packets into multiple segments according
>>>> +to the
>>>> +  specified lengths into the buffers allocated from the specified
>>>> +  memory pools. The backward compatibility to existing API is preserved.
>>>> +
>>>>  * ethdev: ``rx_descriptor_done`` dev_ops and
>> ``rte_eth_rx_descriptor_done``
>>>>    will be deprecated in 20.11 and will be removed in 21.11.
>>>>    Existing ``rte_eth_rx_descriptor_status`` and
>>>> ``rte_eth_tx_descriptor_status``
>>>
> 

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, August 6, 2020 19:37
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> jerinjacobk@gmail.com; stephen@networkplumber.org;
> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> olivier.matz@6wind.com; david.marchand@redhat.com
> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> 
> On 8/6/2020 5:29 PM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Thursday, August 6, 2020 19:16
> >> To: Andrew Rybchenko <arybchenko@solarflare.com>; Slava Ovsiienko
> >> <viacheslavo@mellanox.com>; dev@dpdk.org
> >> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> >> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> >> jerinjacobk@gmail.com; stephen@networkplumber.org;
> >> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> >> olivier.matz@6wind.com; david.marchand@redhat.com
> >> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> >>
> >> On 8/3/2020 3:31 PM, Andrew Rybchenko wrote:
> >>> On 8/3/20 1:58 PM, Viacheslav Ovsiienko wrote:
> >>>> The DPDK datapath in the transmit direction is very flexible.
> >>>> The applications can build multisegment packets and manages almost
> >>>> all data aspects - the memory pools where segments are allocated
> >>>> from, the segment lengths, the memory attributes like external,
> >>>> registered, etc.
> >>>>
> >>>> In the receiving direction, the datapath is much less flexible, the
> >>>> applications can only specify the memory pool to configure the
> >>>> receiving queue and nothing more. In order to extend the receiving
> >>>> datapath capabilities it is proposed to add the new fields into
> >>>> rte_eth_rxconf structure:
> >>>>
> >>>> struct rte_eth_rxconf {
> >>>>     ...
> >>>>     uint16_t rx_split_num; /* number of segments to split */
> >>>>     uint16_t *rx_split_len; /* array of segment lengthes */
> >>>>     struct rte_mempool **mp; /* array of segment memory pools */
> >>>>     ...
> >>>> };
> >>>>
> >>>> The non-zero value of rx_split_num field configures the receiving
> >>>> queue to split ingress packets into multiple segments to the mbufs
> >>>> allocated from various memory pools according to the specified
> >>>> lengths. The zero value of rx_split_num field provides the backward
> >>>> compatibility and queue should be configured in a regular way (with
> >>>> single/multiple mbufs of the same data buffer length allocated from
> >>>> the single memory pool).
> >>>
> >>> From the above description it is not 100% clear how it will coexist
> >>> with:
> >>>  - existing mb_pool argument of the rte_eth_rx_queue_setup()
> >>
> >> +1
> > - supposed to be NULL if the array of lengths/pools is used
> >
> >>
> >>>  - DEV_RX_OFFLOAD_SCATTER
> >>>  - DEV_RX_OFFLOAD_HEADER_SPLIT
> >>> How will application know that the feature is supported? Limitations?
> >>
> >> +1
> > New flag  DEV_RX_OFFLOAD_BUFFER_SPLIT is supposed to be introduced.
> > The feature requires the DEV_RX_OFFLOAD_SCATTER is set.
> > If DEV_RX_OFFLOAD_HEADER_SPLIT is set the error is returned.
> >
> >>
> >>> Is it always split by specified/fixed length?
> >>> What happens if header length is actually different?
> >>
> >> As far as I understand intention is to filter specific packets to a
> >> queue first and later do the split, so the header length will be fixed...
> >
> > Not exactly. The filtering should be handled by rte_flow engine.
> > The intention is to provide the more flexible way to describe rx
> > buffers. Currently it is the single pool with fixed size segments. No
> > way to split the packet into multiple segments with specified lengths
> > and in the specified pools. What if packet payload should be stored in
> > the physical memory on other device (GPU/Storage)? What if caching is
> > not desired for the payload (just forwarding application)? We could provide
> the special NC pool.
> > What if packet should be split into the chunks with specific gaps?
> > For Tx direction we have this opportunity to gather packet from
> > various pools and any desired combinations , but Rx is much less flexible.
> >
> >>>
> >>>> 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 preserving
> >>>> compatibility with existing API.
> 
> If you don't know the packet types in advance, how can you use fixed sizes to
> split a packet? Won't it be like having random parts of packet in each
> mempool..
It is per queue configuration. We have the rte_flow engine and can filter out
the desired packets to the desired queue.

> 
> >>>>
> >>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> >>>> ---
> >>>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>  1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>> b/doc/guides/rel_notes/deprecation.rst
> >>>> index ea4cfa7..cd700ae 100644
> >>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>> @@ -99,6 +99,11 @@ Deprecation Notices
> >>>>    In 19.11 PMDs will still update the field even when the offload is not
> >>>>    enabled.
> >>>>
> >>>> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
> >>>> +receiving
> >>>> +  queues to split ingress packets into multiple segments according
> >>>> +to the
> >>>> +  specified lengths into the buffers allocated from the specified
> >>>> +  memory pools. The backward compatibility to existing API is
> preserved.
> >>>> +
> >>>>  * ethdev: ``rx_descriptor_done`` dev_ops and
> >> ``rte_eth_rx_descriptor_done``
> >>>>    will be deprecated in 20.11 and will be removed in 21.11.
> >>>>    Existing ``rte_eth_rx_descriptor_status`` and
> >>>> ``rte_eth_tx_descriptor_status``
> >>>
> >

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Jerin Jacob]

On Thu, Aug 6, 2020 at 9:56 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 6 Aug 2020 16:58:22 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> > On 8/4/2020 2:32 PM, Jerin Jacob wrote:
> > > On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
> > >>
> > >> Hi, Jerin,
> > >>
> > >> Thanks for the comment,  please, see below.
> > >>
> > >>> -----Original Message-----
> > >>> From: Jerin Jacob <jerinjacobk@gmail.com>
> > >>> Sent: Monday, August 3, 2020 14:57
> > >>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > >>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> > >>> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> > >>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Stephen
> > >>> Hemminger <stephen@networkplumber.org>; Andrew Rybchenko
> > >>> <arybchenko@solarflare.com>; Ajit Khaparde
> > >>> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > >>> <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> > >>> David Marchand <david.marchand@redhat.com>
> > >>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> > >>>
> > >>> On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> > >>> <viacheslavo@mellanox.com> wrote:
> > >>>>
> > >>>> The DPDK datapath in the transmit direction is very flexible.
> > >>>> The applications can build multisegment packets and manages almost all
> > >>>> data aspects - the memory pools where segments are allocated from, the
> > >>>> segment lengths, the memory attributes like external, registered, etc.
> > >>>>
> > >>>> In the receiving direction, the datapath is much less flexible, the
> > >>>> applications can only specify the memory pool to configure the
> > >>>> receiving queue and nothing more. In order to extend the receiving
> > >>>> datapath capabilities it is proposed to add the new fields into
> > >>>> rte_eth_rxconf structure:
> > >>>>
> > >>>> struct rte_eth_rxconf {
> > >>>>     ...
> > >>>>     uint16_t rx_split_num; /* number of segments to split */
> > >>>>     uint16_t *rx_split_len; /* array of segment lengthes */
> > >>>>     struct rte_mempool **mp; /* array of segment memory pools */
> > >>>
> > >>> The pool has the packet length it's been configured for.
> > >>> So I think, rx_split_len can be removed.
> > >>
> > >> Yes, it is one of the supposed options - if pointer to array of segment lengths
> > >> is NULL , the queue_setup() could use the lengths from the pool's properties.
> > >> But we are talking about packet split, in general, it should not depend
> > >> on pool properties. What if application provides the single pool
> > >> and just wants to have the tunnel header in the first dedicated mbuf?
> > >>
> > >>>
> > >>> This feature also available in Marvell HW. So it not specific to one vendor.
> > >>> Maybe we could just the use case mention the use case in the depreciation
> > >>> notice and the tentative change in rte_eth_rxconf and exact details can be
> > >>> worked out at the time of implementation.
> > >>>
> > >> So, if I understand correctly, the struct changes in the commit message
> > >> should be marked as just possible implementation?
> > >
> > > Yes.
> > >
> > > We may need to have a detailed discussion on the correct abstraction for various
> > > HW is available with this feature.
> > >
> > > On Marvell HW, We can configure TWO pools for given eth Rx queue.
> > > One pool can be configured as a small packet pool and other one as
> > > large packet pool.
> > > And there is a threshold value to decide the pool between small and large.
> > > For example:
> > > - The small pool is configured 2k
> > > - The large pool is configured with 10k
> > > - And if the threshold value is configured as 2k.
> > > Any packet size <=2K will land in small pool and others in a large pool.
> > > The use case, we are targeting is to save the memory space for jumbo frames.
> >
> > Out of curiosity, do you provide two different buffer address in the descriptor
> > and HW automatically uses one based on the size,
> > or driver uses one of the pools based on the configuration and possible largest
> > packet size?

The later one.

>
> I am all for allowing more configuration of buffer pool.
> But don't want that to be exposed as a hardware specific requirement in the
> API for applications. The worst case would be if your API changes required:
>
>   if (strcmp(dev->driver_name, "marvell") == 0) {
>      // make another mempool for this driver

There is no HW specific requirements here. If one pool specified(like
the existing situation),
HW will create scatter-gather frame.

It is mostly useful for the application use case where it needs single
contiguous of data
for processing(like crypto) and/or improving Rx/TX performance by
running in single seg mode
without losing too much of memory.


>
>

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Ferruh Yigit]

On 8/6/2020 5:39 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, August 6, 2020 19:37
>> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Andrew Rybchenko
>> <arybchenko@solarflare.com>; dev@dpdk.org
>> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
>> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>> jerinjacobk@gmail.com; stephen@networkplumber.org;
>> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
>> olivier.matz@6wind.com; david.marchand@redhat.com
>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
>>
>> On 8/6/2020 5:29 PM, Slava Ovsiienko wrote:
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Thursday, August 6, 2020 19:16
>>>> To: Andrew Rybchenko <arybchenko@solarflare.com>; Slava Ovsiienko
>>>> <viacheslavo@mellanox.com>; dev@dpdk.org
>>>> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
>>>> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>>>> jerinjacobk@gmail.com; stephen@networkplumber.org;
>>>> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
>>>> olivier.matz@6wind.com; david.marchand@redhat.com
>>>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
>>>>
>>>> On 8/3/2020 3:31 PM, Andrew Rybchenko wrote:
>>>>> On 8/3/20 1:58 PM, Viacheslav Ovsiienko wrote:
>>>>>> The DPDK datapath in the transmit direction is very flexible.
>>>>>> The applications can build multisegment packets and manages almost
>>>>>> all data aspects - the memory pools where segments are allocated
>>>>>> from, the segment lengths, the memory attributes like external,
>>>>>> registered, etc.
>>>>>>
>>>>>> In the receiving direction, the datapath is much less flexible, the
>>>>>> applications can only specify the memory pool to configure the
>>>>>> receiving queue and nothing more. In order to extend the receiving
>>>>>> datapath capabilities it is proposed to add the new fields into
>>>>>> rte_eth_rxconf structure:
>>>>>>
>>>>>> struct rte_eth_rxconf {
>>>>>>     ...
>>>>>>     uint16_t rx_split_num; /* number of segments to split */
>>>>>>     uint16_t *rx_split_len; /* array of segment lengthes */
>>>>>>     struct rte_mempool **mp; /* array of segment memory pools */
>>>>>>     ...
>>>>>> };
>>>>>>
>>>>>> The non-zero value of rx_split_num field configures the receiving
>>>>>> queue to split ingress packets into multiple segments to the mbufs
>>>>>> allocated from various memory pools according to the specified
>>>>>> lengths. The zero value of rx_split_num field provides the backward
>>>>>> compatibility and queue should be configured in a regular way (with
>>>>>> single/multiple mbufs of the same data buffer length allocated from
>>>>>> the single memory pool).
>>>>>
>>>>> From the above description it is not 100% clear how it will coexist
>>>>> with:
>>>>>  - existing mb_pool argument of the rte_eth_rx_queue_setup()
>>>>
>>>> +1
>>> - supposed to be NULL if the array of lengths/pools is used
>>>
>>>>
>>>>>  - DEV_RX_OFFLOAD_SCATTER
>>>>>  - DEV_RX_OFFLOAD_HEADER_SPLIT
>>>>> How will application know that the feature is supported? Limitations?
>>>>
>>>> +1
>>> New flag  DEV_RX_OFFLOAD_BUFFER_SPLIT is supposed to be introduced.
>>> The feature requires the DEV_RX_OFFLOAD_SCATTER is set.
>>> If DEV_RX_OFFLOAD_HEADER_SPLIT is set the error is returned.
>>>
>>>>
>>>>> Is it always split by specified/fixed length?
>>>>> What happens if header length is actually different?
>>>>
>>>> As far as I understand intention is to filter specific packets to a
>>>> queue first and later do the split, so the header length will be fixed...
>>>
>>> Not exactly. The filtering should be handled by rte_flow engine.
>>> The intention is to provide the more flexible way to describe rx
>>> buffers. Currently it is the single pool with fixed size segments. No
>>> way to split the packet into multiple segments with specified lengths
>>> and in the specified pools. What if packet payload should be stored in
>>> the physical memory on other device (GPU/Storage)? What if caching is
>>> not desired for the payload (just forwarding application)? We could provide
>> the special NC pool.
>>> What if packet should be split into the chunks with specific gaps?
>>> For Tx direction we have this opportunity to gather packet from
>>> various pools and any desired combinations , but Rx is much less flexible.
>>>
>>>>>
>>>>>> 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 preserving
>>>>>> compatibility with existing API.
>>
>> If you don't know the packet types in advance, how can you use fixed sizes to
>> split a packet? Won't it be like having random parts of packet in each
>> mempool..
> It is per queue configuration. We have the rte_flow engine and can filter out
> the desired packets to the desired queue.

That is what I was trying to say above, intentions is first filter the packets
to a specific queue, later split them into multiple mempools, you said "not
exactly", what is the difference I am missing?

> 
>>
>>>>>>
>>>>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>>>>>> ---
>>>>>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>  1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>> index ea4cfa7..cd700ae 100644
>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>> @@ -99,6 +99,11 @@ Deprecation Notices
>>>>>>    In 19.11 PMDs will still update the field even when the offload is not
>>>>>>    enabled.
>>>>>>
>>>>>> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
>>>>>> +receiving
>>>>>> +  queues to split ingress packets into multiple segments according
>>>>>> +to the
>>>>>> +  specified lengths into the buffers allocated from the specified
>>>>>> +  memory pools. The backward compatibility to existing API is
>> preserved.
>>>>>> +
>>>>>>  * ethdev: ``rx_descriptor_done`` dev_ops and
>>>> ``rte_eth_rx_descriptor_done``
>>>>>>    will be deprecated in 20.11 and will be removed in 21.11.
>>>>>>    Existing ``rte_eth_rx_descriptor_status`` and
>>>>>> ``rte_eth_tx_descriptor_status``
>>>>>
>>>
> 

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, August 6, 2020 19:43
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> jerinjacobk@gmail.com; stephen@networkplumber.org;
> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> olivier.matz@6wind.com; david.marchand@redhat.com
> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> 
> On 8/6/2020 5:39 PM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Thursday, August 6, 2020 19:37
> >> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Andrew Rybchenko
> >> <arybchenko@solarflare.com>; dev@dpdk.org
> >> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> >> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> >> jerinjacobk@gmail.com; stephen@networkplumber.org;
> >> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> >> olivier.matz@6wind.com; david.marchand@redhat.com
> >> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> >>
> >> On 8/6/2020 5:29 PM, Slava Ovsiienko wrote:
> >>>> -----Original Message-----
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>> Sent: Thursday, August 6, 2020 19:16
> >>>> To: Andrew Rybchenko <arybchenko@solarflare.com>; Slava Ovsiienko
> >>>> <viacheslavo@mellanox.com>; dev@dpdk.org
> >>>> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> >>>> <rasland@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>;
> >>>> jerinjacobk@gmail.com; stephen@networkplumber.org;
> >>>> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> >>>> olivier.matz@6wind.com; david.marchand@redhat.com
> >>>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf
> >>>> structure
> >>>>
> >>>> On 8/3/2020 3:31 PM, Andrew Rybchenko wrote:
> >>>>> On 8/3/20 1:58 PM, Viacheslav Ovsiienko wrote:
> >>>>>> The DPDK datapath in the transmit direction is very flexible.
> >>>>>> The applications can build multisegment packets and manages
> >>>>>> almost all data aspects - the memory pools where segments are
> >>>>>> allocated from, the segment lengths, the memory attributes like
> >>>>>> external, registered, etc.
> >>>>>>
> >>>>>> In the receiving direction, the datapath is much less flexible,
> >>>>>> the applications can only specify the memory pool to configure
> >>>>>> the receiving queue and nothing more. In order to extend the
> >>>>>> receiving datapath capabilities it is proposed to add the new
> >>>>>> fields into rte_eth_rxconf structure:
> >>>>>>
> >>>>>> struct rte_eth_rxconf {
> >>>>>>     ...
> >>>>>>     uint16_t rx_split_num; /* number of segments to split */
> >>>>>>     uint16_t *rx_split_len; /* array of segment lengthes */
> >>>>>>     struct rte_mempool **mp; /* array of segment memory pools */
> >>>>>>     ...
> >>>>>> };
> >>>>>>
> >>>>>> The non-zero value of rx_split_num field configures the receiving
> >>>>>> queue to split ingress packets into multiple segments to the
> >>>>>> mbufs allocated from various memory pools according to the
> >>>>>> specified lengths. The zero value of rx_split_num field provides
> >>>>>> the backward compatibility and queue should be configured in a
> >>>>>> regular way (with single/multiple mbufs of the same data buffer
> >>>>>> length allocated from the single memory pool).
> >>>>>
> >>>>> From the above description it is not 100% clear how it will
> >>>>> coexist
> >>>>> with:
> >>>>>  - existing mb_pool argument of the rte_eth_rx_queue_setup()
> >>>>
> >>>> +1
> >>> - supposed to be NULL if the array of lengths/pools is used
> >>>
> >>>>
> >>>>>  - DEV_RX_OFFLOAD_SCATTER
> >>>>>  - DEV_RX_OFFLOAD_HEADER_SPLIT
> >>>>> How will application know that the feature is supported? Limitations?
> >>>>
> >>>> +1
> >>> New flag  DEV_RX_OFFLOAD_BUFFER_SPLIT is supposed to be
> introduced.
> >>> The feature requires the DEV_RX_OFFLOAD_SCATTER is set.
> >>> If DEV_RX_OFFLOAD_HEADER_SPLIT is set the error is returned.
> >>>
> >>>>
> >>>>> Is it always split by specified/fixed length?
> >>>>> What happens if header length is actually different?
> >>>>
> >>>> As far as I understand intention is to filter specific packets to a
> >>>> queue first and later do the split, so the header length will be fixed...
> >>>
> >>> Not exactly. The filtering should be handled by rte_flow engine.
> >>> The intention is to provide the more flexible way to describe rx
> >>> buffers. Currently it is the single pool with fixed size segments.
> >>> No way to split the packet into multiple segments with specified
> >>> lengths and in the specified pools. What if packet payload should be
> >>> stored in the physical memory on other device (GPU/Storage)? What if
> >>> caching is not desired for the payload (just forwarding
> >>> application)? We could provide
> >> the special NC pool.
> >>> What if packet should be split into the chunks with specific gaps?
> >>> For Tx direction we have this opportunity to gather packet from
> >>> various pools and any desired combinations , but Rx is much less flexible.
> >>>
> >>>>>
> >>>>>> 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
> >>>>>> preserving compatibility with existing API.
> >>
> >> If you don't know the packet types in advance, how can you use fixed
> >> sizes to split a packet? Won't it be like having random parts of
> >> packet in each mempool..
> > It is per queue configuration. We have the rte_flow engine and can
> > filter out the desired packets to the desired queue.
> 
> That is what I was trying to say above, intentions is first filter the packets to
> a specific queue, later split them into multiple mempools, you said "not
> exactly", what is the difference I am missing?

Sorry, it is my bad - I mixed up with Marvell's queue capability to sort packets into two
pools depending on packet size. Yes, you are completely correct, first filter out the specific packets
to the dedicated queue and then split ones into the chunks of specified fixed sizes.
> 
> >
> >>
> >>>>>>
> >>>>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> >>>>>> ---
> >>>>>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>>  1 file changed, 5 insertions(+)
> >>>>>>
> >>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>> index ea4cfa7..cd700ae 100644
> >>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>> @@ -99,6 +99,11 @@ Deprecation Notices
> >>>>>>    In 19.11 PMDs will still update the field even when the offload is
> not
> >>>>>>    enabled.
> >>>>>>
> >>>>>> +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
> >>>>>> +receiving
> >>>>>> +  queues to split ingress packets into multiple segments
> >>>>>> +according to the
> >>>>>> +  specified lengths into the buffers allocated from the
> >>>>>> +specified
> >>>>>> +  memory pools. The backward compatibility to existing API is
> >> preserved.
> >>>>>> +
> >>>>>>  * ethdev: ``rx_descriptor_done`` dev_ops and
> >>>> ``rte_eth_rx_descriptor_done``
> >>>>>>    will be deprecated in 20.11 and will be removed in 21.11.
> >>>>>>    Existing ``rte_eth_rx_descriptor_status`` and
> >>>>>> ``rte_eth_tx_descriptor_status``
> >>>>>
> >>>
> >

[dpdk-dev] [PATCH v2] doc: fix the release notes for Mellanox PMD

[Viacheslav Ovsiienko]

The mlx5 PMD does not work on Power 9 platform with OFED
version 5.1-0.6.6.0 (latest GA release at the moment).
Release notes know issues chapter is updated accordingly,
workaround is proposed.

Fixes: 544f35c334e5 ("doc: add tested platforms with Mellanox NICs")

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

---
v1->v2: indent fix
        "might not/does not" fix
---
 doc/guides/rel_notes/release_20_08.rst | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_notes/release_20_08.rst
index 9761f5a..3b29aef 100644
--- a/doc/guides/rel_notes/release_20_08.rst
+++ b/doc/guides/rel_notes/release_20_08.rst
@@ -331,6 +331,11 @@ Known Issues
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* **mlx5 PMD does not work on Power 9 with OFED 5.1-0.6.6.0.**
+
+  Consider using the newer OFED releases, the previous
+  OFED 5.0-2.1.8.0, or upstream rdma-core library v29 and above.
+
 
 Tested Platforms
 ----------------
@@ -489,4 +494,4 @@ Tested Platforms
 
   * OFED:
 
-    * MLNX_OFED 5.1-0.6.6.0
+    * MLNX_OFED 5.0-2.1.8.0
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, August 6, 2020 19:32
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> jerinjacobk@gmail.com; stephen@networkplumber.org;
> arybchenko@solarflare.com; ajit.khaparde@broadcom.com;
> maxime.coquelin@redhat.com; olivier.matz@6wind.com;
> david.marchand@redhat.com
> Subject: Re: [PATCH v2] doc: announce changes to ethdev rxconf structure
> 
> On 8/5/2020 9:49 AM, Viacheslav Ovsiienko wrote:
> > The DPDK datapath in the transmit direction is very flexible.
> > The applications can build multi-segment packets and manages almost
> > all data aspects - the memory pools where segments are allocated from,
> > the segment lengths, the memory attributes like external, registered,
> > etc.
> >
> > In the receiving direction, the datapath is much less flexible, the
> > applications can only specify the memory pool to configure the
> > receiving queue and nothing more. The packet being received can only
> > be pushed to the chain of the mbufs of the same data buffer size and
> > allocated from the same pool. In order to extend the receiving
> > datapath buffer description it is proposed to add the new fields into
> > rte_eth_rxconf structure:
> >
> > struct rte_eth_rxconf {
> >     ...
> >     uint16_t rx_split_num; /* number of segments to split */
> >     uint16_t *rx_split_len; /* array of segment lengths */
> >     struct rte_mempool **mp; /* array of segment memory pools */
> >     ...
> > };
> 
> What is the way to say first 14 bytes will go first mempool and rest will go
> second one?
> Or do you have to define fixed sizes for all segments?
Yes - "rx_split_len" array defines the sizes of segments.

> What if that 'rest' part larger than given buffer size for that mempool?
Error. The supposed size for appropriate segment must conform the pool requirements.
So - the segment size must be less or equal to the mbuf buffer size from this pool.
And, must be supported by HW and PMD, if there are some other limitations - these
ones should be checked in rx_queue_setup and error returned if needed.

> 
> Intel NICs also has header split support, similar to what Jerin described,
> header and data goes to different buffers, which doesn't require fixed sizes
> and need only two mempools, not sure if it should be integrated to this
> feature but we can discuss later.
HEADER_SPLIT is port-wide feature, not per queue.
BTW, where the second pool (for the payload) is specified? Do I miss the second pool?

> 
> Also there are some valid concerns Andrew highlighted, like how application
> will know if PMD supports this feature etc.. and more.
> But since these are design/implementation related concerns, not a blocker
> for deprecation notice I think, overall no objection to config structure
> change, hence:
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Thanks a lot for the review, discussion and Deprecation Note ack.

> 
> >
> > The non-zero value of rx_split_num field configures the receiving
> > queue to split ingress packets into multiple segments to the mbufs
> > allocated from various memory pools according to the specified
> > lengths. The zero value of rx_split_num field provides the backward
> > compatibility and queue should be configured in a regular way (with
> > single/multiple mbufs of the same data buffer length allocated from
> > the single memory pool).
> >
> > 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 preserving
> > compatibility with existing API.
> >
> > The proposed extended description of receiving buffers might be
> > considered by other vendors to be involved into similar features
> > support, it is the subject for the further discussion.
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > Acked-by: Jerin Jacob <jerinjacobk@gmail.com>
> >
> > ---
> > v1->v2: commit message updated, proposed to consider the new
> >         fields for supporting similar features by multiple
> > 	vendors
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index acf87d3..b6bdb83 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -99,6 +99,11 @@ Deprecation Notices
> >    In 19.11 PMDs will still update the field even when the offload is not
> >    enabled.
> >
> > +* ethdev: add new fields to ``rte_eth_rxconf`` to configure the
> > +receiving
> > +  queues to split ingress packets into multiple segments according to
> > +the
> > +  specified lengths into the buffers allocated from the specified
> > +  memory pools. The backward compatibility to existing API is preserved.
> > +
> >  * ethdev: ``rx_descriptor_done`` dev_ops and
> ``rte_eth_rx_descriptor_done``
> >    will be deprecated in 20.11 and will be removed in 21.11.
> >    Existing ``rte_eth_rx_descriptor_status`` and
> > ``rte_eth_tx_descriptor_status``
> >

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, August 6, 2020 19:26
> To: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: Jerin Jacob <jerinjacobk@gmail.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; dpdk-dev <dev@dpdk.org>; Matan Azrad
> <matan@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>;
> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> David Marchand <david.marchand@redhat.com>
> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> 
> On Thu, 6 Aug 2020 16:58:22 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> > On 8/4/2020 2:32 PM, Jerin Jacob wrote:
> > > On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko
> <viacheslavo@mellanox.com> wrote:
> > >>
> > >> Hi, Jerin,
> > >>
> > >> Thanks for the comment,  please, see below.
> > >>
> > >>> -----Original Message-----
> > >>> From: Jerin Jacob <jerinjacobk@gmail.com>
> > >>> Sent: Monday, August 3, 2020 14:57
> > >>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > >>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> > >>> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> > >>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> > >>> Stephen Hemminger <stephen@networkplumber.org>; Andrew
> Rybchenko
> > >>> <arybchenko@solarflare.com>; Ajit Khaparde
> > >>> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > >>> <maxime.coquelin@redhat.com>; Olivier Matz
> > >>> <olivier.matz@6wind.com>; David Marchand
> > >>> <david.marchand@redhat.com>
> > >>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf
> > >>> structure
> > >>>
> > >>> On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> > >>> <viacheslavo@mellanox.com> wrote:
> > >>>>
> > >>>> The DPDK datapath in the transmit direction is very flexible.
> > >>>> The applications can build multisegment packets and manages
> > >>>> almost all data aspects - the memory pools where segments are
> > >>>> allocated from, the segment lengths, the memory attributes like
> external, registered, etc.
> > >>>>
> > >>>> In the receiving direction, the datapath is much less flexible,
> > >>>> the applications can only specify the memory pool to configure
> > >>>> the receiving queue and nothing more. In order to extend the
> > >>>> receiving datapath capabilities it is proposed to add the new
> > >>>> fields into rte_eth_rxconf structure:
> > >>>>
> > >>>> struct rte_eth_rxconf {
> > >>>>     ...
> > >>>>     uint16_t rx_split_num; /* number of segments to split */
> > >>>>     uint16_t *rx_split_len; /* array of segment lengthes */
> > >>>>     struct rte_mempool **mp; /* array of segment memory pools */
> > >>>
> > >>> The pool has the packet length it's been configured for.
> > >>> So I think, rx_split_len can be removed.
> > >>
> > >> Yes, it is one of the supposed options - if pointer to array of
> > >> segment lengths is NULL , the queue_setup() could use the lengths from
> the pool's properties.
> > >> But we are talking about packet split, in general, it should not
> > >> depend on pool properties. What if application provides the single
> > >> pool and just wants to have the tunnel header in the first dedicated
> mbuf?
> > >>
> > >>>
> > >>> This feature also available in Marvell HW. So it not specific to one
> vendor.
> > >>> Maybe we could just the use case mention the use case in the
> > >>> depreciation notice and the tentative change in rte_eth_rxconf and
> > >>> exact details can be worked out at the time of implementation.
> > >>>
> > >> So, if I understand correctly, the struct changes in the commit
> > >> message should be marked as just possible implementation?
> > >
> > > Yes.
> > >
> > > We may need to have a detailed discussion on the correct abstraction
> > > for various HW is available with this feature.
> > >
> > > On Marvell HW, We can configure TWO pools for given eth Rx queue.
> > > One pool can be configured as a small packet pool and other one as
> > > large packet pool.
> > > And there is a threshold value to decide the pool between small and
> large.
> > > For example:
> > > - The small pool is configured 2k
> > > - The large pool is configured with 10k
> > > - And if the threshold value is configured as 2k.
> > > Any packet size <=2K will land in small pool and others in a large pool.
> > > The use case, we are targeting is to save the memory space for jumbo
> frames.
> >
> > Out of curiosity, do you provide two different buffer address in the
> > descriptor and HW automatically uses one based on the size, or driver
> > uses one of the pools based on the configuration and possible largest
> > packet size?
> 
> I am all for allowing more configuration of buffer pool.
> But don't want that to be exposed as a hardware specific requirement in the
> API for applications. The worst case would be if your API changes required:
> 
>   if (strcmp(dev->driver_name, "marvell") == 0) {
>      // make another mempool for this driver
> 
I thought about adding some other segment attributes, vendor specific.
We could describe the segments with some descriptor structure (size, pool)
and add flags field to one. The proposals from other vendors are welcome.

Re: [dpdk-dev] [PATCH v2] doc: fix the release notes for Mellanox PMD

[Asaf Penso]

Regards,
Asaf Penso

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Viacheslav Ovsiienko
> Sent: Thursday, August 6, 2020 7:55 PM
> To: dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Subject: [dpdk-dev] [PATCH v2] doc: fix the release notes for Mellanox PMD
> 
> The mlx5 PMD does not work on Power 9 platform with OFED version 5.1-
> 0.6.6.0 (latest GA release at the moment).
> Release notes know issues chapter is updated accordingly, workaround is
> proposed.
> 
> Fixes: 544f35c334e5 ("doc: add tested platforms with Mellanox NICs")
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> 
> ---
> v1->v2: indent fix
>         "might not/does not" fix
> ---
>  doc/guides/rel_notes/release_20_08.rst | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/rel_notes/release_20_08.rst
> b/doc/guides/rel_notes/release_20_08.rst
> index 9761f5a..3b29aef 100644
> --- a/doc/guides/rel_notes/release_20_08.rst
> +++ b/doc/guides/rel_notes/release_20_08.rst
> @@ -331,6 +331,11 @@ Known Issues
>     Also, make sure to start the actual text at the margin.
>     =========================================================
> 
> +* **mlx5 PMD does not work on Power 9 with OFED 5.1-0.6.6.0.**
> +
> +  Consider using the newer OFED releases, the previous  OFED
> + 5.0-2.1.8.0, or upstream rdma-core library v29 and above.
> +
> 
>  Tested Platforms
>  ----------------
> @@ -489,4 +494,4 @@ Tested Platforms
> 
>    * OFED:
> 
> -    * MLNX_OFED 5.1-0.6.6.0
> +    * MLNX_OFED 5.0-2.1.8.0
> --
> 1.8.3.1
Acked-by: Asaf Penso <asafp@mellanox.com>

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Stephen Hemminger]

On Thu, 6 Aug 2020 17:03:31 +0000
Slava Ovsiienko <viacheslavo@mellanox.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Thursday, August 6, 2020 19:26
> > To: Ferruh Yigit <ferruh.yigit@intel.com>
> > Cc: Jerin Jacob <jerinjacobk@gmail.com>; Slava Ovsiienko
> > <viacheslavo@mellanox.com>; dpdk-dev <dev@dpdk.org>; Matan Azrad
> > <matan@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>;
> > Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> > <arybchenko@solarflare.com>; Ajit Khaparde
> > <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> > David Marchand <david.marchand@redhat.com>
> > Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> > 
> > On Thu, 6 Aug 2020 16:58:22 +0100
> > Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >   
> > > On 8/4/2020 2:32 PM, Jerin Jacob wrote:  
> > > > On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko  
> > <viacheslavo@mellanox.com> wrote:  
> > > >>
> > > >> Hi, Jerin,
> > > >>
> > > >> Thanks for the comment,  please, see below.
> > > >>  
> > > >>> -----Original Message-----
> > > >>> From: Jerin Jacob <jerinjacobk@gmail.com>
> > > >>> Sent: Monday, August 3, 2020 14:57
> > > >>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > > >>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@mellanox.com>;
> > > >>> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> > > >>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> > > >>> Stephen Hemminger <stephen@networkplumber.org>; Andrew  
> > Rybchenko  
> > > >>> <arybchenko@solarflare.com>; Ajit Khaparde
> > > >>> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > > >>> <maxime.coquelin@redhat.com>; Olivier Matz
> > > >>> <olivier.matz@6wind.com>; David Marchand
> > > >>> <david.marchand@redhat.com>
> > > >>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf
> > > >>> structure
> > > >>>
> > > >>> On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> > > >>> <viacheslavo@mellanox.com> wrote:  
> > > >>>>
> > > >>>> The DPDK datapath in the transmit direction is very flexible.
> > > >>>> The applications can build multisegment packets and manages
> > > >>>> almost all data aspects - the memory pools where segments are
> > > >>>> allocated from, the segment lengths, the memory attributes like  
> > external, registered, etc.  
> > > >>>>
> > > >>>> In the receiving direction, the datapath is much less flexible,
> > > >>>> the applications can only specify the memory pool to configure
> > > >>>> the receiving queue and nothing more. In order to extend the
> > > >>>> receiving datapath capabilities it is proposed to add the new
> > > >>>> fields into rte_eth_rxconf structure:
> > > >>>>
> > > >>>> struct rte_eth_rxconf {
> > > >>>>     ...
> > > >>>>     uint16_t rx_split_num; /* number of segments to split */
> > > >>>>     uint16_t *rx_split_len; /* array of segment lengthes */
> > > >>>>     struct rte_mempool **mp; /* array of segment memory pools */  
> > > >>>
> > > >>> The pool has the packet length it's been configured for.
> > > >>> So I think, rx_split_len can be removed.  
> > > >>
> > > >> Yes, it is one of the supposed options - if pointer to array of
> > > >> segment lengths is NULL , the queue_setup() could use the lengths from  
> > the pool's properties.  
> > > >> But we are talking about packet split, in general, it should not
> > > >> depend on pool properties. What if application provides the single
> > > >> pool and just wants to have the tunnel header in the first dedicated  
> > mbuf?  
> > > >>  
> > > >>>
> > > >>> This feature also available in Marvell HW. So it not specific to one  
> > vendor.  
> > > >>> Maybe we could just the use case mention the use case in the
> > > >>> depreciation notice and the tentative change in rte_eth_rxconf and
> > > >>> exact details can be worked out at the time of implementation.
> > > >>>  
> > > >> So, if I understand correctly, the struct changes in the commit
> > > >> message should be marked as just possible implementation?  
> > > >
> > > > Yes.
> > > >
> > > > We may need to have a detailed discussion on the correct abstraction
> > > > for various HW is available with this feature.
> > > >
> > > > On Marvell HW, We can configure TWO pools for given eth Rx queue.
> > > > One pool can be configured as a small packet pool and other one as
> > > > large packet pool.
> > > > And there is a threshold value to decide the pool between small and  
> > large.  
> > > > For example:
> > > > - The small pool is configured 2k
> > > > - The large pool is configured with 10k
> > > > - And if the threshold value is configured as 2k.
> > > > Any packet size <=2K will land in small pool and others in a large pool.
> > > > The use case, we are targeting is to save the memory space for jumbo  
> > frames.  
> > >
> > > Out of curiosity, do you provide two different buffer address in the
> > > descriptor and HW automatically uses one based on the size, or driver
> > > uses one of the pools based on the configuration and possible largest
> > > packet size?  
> > 
> > I am all for allowing more configuration of buffer pool.
> > But don't want that to be exposed as a hardware specific requirement in the
> > API for applications. The worst case would be if your API changes required:
> > 
> >   if (strcmp(dev->driver_name, "marvell") == 0) {
> >      // make another mempool for this driver
> >   
> I thought about adding some other segment attributes, vendor specific.
> We could describe the segments with some descriptor structure (size, pool)
> and add flags field to one. The proposals from other vendors are welcome.
> 

Please no snowflake API's "are driver is special"...

Think of how it can fit into a general model.
Also, just because your hardware has a special feature does not mean
the DPDK has to support it!

Re: [dpdk-dev] [PATCH v2] doc: announce changes to ethdev rxconf structure

[Thomas Monjalon]

06/08/2020 14:39, Thomas Monjalon:
> 05/08/2020 13:14, Andrew Rybchenko:
> > On 8/5/20 11:49 AM, Viacheslav Ovsiienko wrote:
> > > The DPDK datapath in the transmit direction is very flexible.
> > > The applications can build multi-segment packets and manages
> > > almost all data aspects - the memory pools where segments
> > > are allocated from, the segment lengths, the memory attributes
> > > like external, registered, etc.
> > > 
> > > In the receiving direction, the datapath is much less flexible,
> > > the applications can only specify the memory pool to configure
> > > the receiving queue and nothing more. The packet being received
> > > can only be pushed to the chain of the mbufs of the same data
> > > buffer size and allocated from the same pool. In order to extend
> > > the receiving datapath buffer description it is proposed to add
> > > the new fields into rte_eth_rxconf structure:
> > > 
> > > struct rte_eth_rxconf {
> > >     ...
> > >     uint16_t rx_split_num; /* number of segments to split */
> > >     uint16_t *rx_split_len; /* array of segment lengths */
> > >     struct rte_mempool **mp; /* array of segment memory pools */
> > >     ...
> > > };
> > > 
> > > The non-zero value of rx_split_num field configures the receiving
> > > queue to split ingress packets into multiple segments to the mbufs
> > > allocated from various memory pools according to the specified
> > > lengths. The zero value of rx_split_num field provides the
> > > backward compatibility and queue should be configured in a regular
> > > way (with single/multiple mbufs of the same data buffer length
> > > allocated from the single memory pool).
> > > 
> > > 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 preserving compatibility with existing API.
> > > 
> > > The proposed extended description of receiving buffers might be
> > > considered by other vendors to be involved into similar features
> > > support, it is the subject for the further discussion.
> > > 
> > > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > > Acked-by: Jerin Jacob <jerinjacobk@gmail.com>
> > 
> > I"m OK with the idea in general and we'll work on details
> > in the next release cycle.
> > 
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> I agree we need to be more flexible with the mempools in Rx.
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied

Implementation will require more design discussions.

Re: [dpdk-dev] [PATCH v2] doc: fix the release notes for Mellanox PMD

[Thomas Monjalon]

> > The mlx5 PMD does not work on Power 9 platform with OFED version 5.1-
> > 0.6.6.0 (latest GA release at the moment).
> > Release notes know issues chapter is updated accordingly, workaround is
> > proposed.
> > 
> > Fixes: 544f35c334e5 ("doc: add tested platforms with Mellanox NICs")
> > 
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > 
> Acked-by: Asaf Penso <asafp@mellanox.com>

Applied, thanks

Re: [dpdk-dev] [PATCH] doc: announce changes to ethdev rxconf structure

[Slava Ovsiienko]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, August 6, 2020 21:10
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Jerin Jacob
> <jerinjacobk@gmail.com>; dpdk-dev <dev@dpdk.org>; Matan Azrad
> <matan@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>;
> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Olivier Matz <olivier.matz@6wind.com>;
> David Marchand <david.marchand@redhat.com>
> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf structure
> 
> On Thu, 6 Aug 2020 17:03:31 +0000
> Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > Sent: Thursday, August 6, 2020 19:26
> > > To: Ferruh Yigit <ferruh.yigit@intel.com>
> > > Cc: Jerin Jacob <jerinjacobk@gmail.com>; Slava Ovsiienko
> > > <viacheslavo@mellanox.com>; dpdk-dev <dev@dpdk.org>; Matan Azrad
> > > <matan@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>;
> > > Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> > > <arybchenko@solarflare.com>; Ajit Khaparde
> > > <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > > <maxime.coquelin@redhat.com>; Olivier Matz
> <olivier.matz@6wind.com>;
> > > David Marchand <david.marchand@redhat.com>
> > > Subject: Re: [PATCH] doc: announce changes to ethdev rxconf
> > > structure
> > >
> > > On Thu, 6 Aug 2020 16:58:22 +0100
> > > Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > >
> > > > On 8/4/2020 2:32 PM, Jerin Jacob wrote:
> > > > > On Mon, Aug 3, 2020 at 6:36 PM Slava Ovsiienko
> > > <viacheslavo@mellanox.com> wrote:
> > > > >>
> > > > >> Hi, Jerin,
> > > > >>
> > > > >> Thanks for the comment,  please, see below.
> > > > >>
> > > > >>> -----Original Message-----
> > > > >>> From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > >>> Sent: Monday, August 3, 2020 14:57
> > > > >>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > > > >>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad
> <matan@mellanox.com>;
> > > > >>> Raslan Darawsheh <rasland@mellanox.com>; Thomas Monjalon
> > > > >>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> > > > >>> Stephen Hemminger <stephen@networkplumber.org>; Andrew
> > > Rybchenko
> > > > >>> <arybchenko@solarflare.com>; Ajit Khaparde
> > > > >>> <ajit.khaparde@broadcom.com>; Maxime Coquelin
> > > > >>> <maxime.coquelin@redhat.com>; Olivier Matz
> > > > >>> <olivier.matz@6wind.com>; David Marchand
> > > > >>> <david.marchand@redhat.com>
> > > > >>> Subject: Re: [PATCH] doc: announce changes to ethdev rxconf
> > > > >>> structure
> > > > >>>
> > > > >>> On Mon, Aug 3, 2020 at 4:28 PM Viacheslav Ovsiienko
> > > > >>> <viacheslavo@mellanox.com> wrote:
> > > > >>>>
> > > > >>>> The DPDK datapath in the transmit direction is very flexible.
> > > > >>>> The applications can build multisegment packets and manages
> > > > >>>> almost all data aspects - the memory pools where segments are
> > > > >>>> allocated from, the segment lengths, the memory attributes
> > > > >>>> like
> > > external, registered, etc.
> > > > >>>>
> > > > >>>> In the receiving direction, the datapath is much less
> > > > >>>> flexible, the applications can only specify the memory pool
> > > > >>>> to configure the receiving queue and nothing more. In order
> > > > >>>> to extend the receiving datapath capabilities it is proposed
> > > > >>>> to add the new fields into rte_eth_rxconf structure:
> > > > >>>>
> > > > >>>> struct rte_eth_rxconf {
> > > > >>>>     ...
> > > > >>>>     uint16_t rx_split_num; /* number of segments to split */
> > > > >>>>     uint16_t *rx_split_len; /* array of segment lengthes */
> > > > >>>>     struct rte_mempool **mp; /* array of segment memory pools
> > > > >>>> */
> > > > >>>
> > > > >>> The pool has the packet length it's been configured for.
> > > > >>> So I think, rx_split_len can be removed.
> > > > >>
> > > > >> Yes, it is one of the supposed options - if pointer to array of
> > > > >> segment lengths is NULL , the queue_setup() could use the
> > > > >> lengths from
> > > the pool's properties.
> > > > >> But we are talking about packet split, in general, it should
> > > > >> not depend on pool properties. What if application provides the
> > > > >> single pool and just wants to have the tunnel header in the
> > > > >> first dedicated
> > > mbuf?
> > > > >>
> > > > >>>
> > > > >>> This feature also available in Marvell HW. So it not specific
> > > > >>> to one
> > > vendor.
> > > > >>> Maybe we could just the use case mention the use case in the
> > > > >>> depreciation notice and the tentative change in rte_eth_rxconf
> > > > >>> and exact details can be worked out at the time of implementation.
> > > > >>>
> > > > >> So, if I understand correctly, the struct changes in the commit
> > > > >> message should be marked as just possible implementation?
> > > > >
> > > > > Yes.
> > > > >
> > > > > We may need to have a detailed discussion on the correct
> > > > > abstraction for various HW is available with this feature.
> > > > >
> > > > > On Marvell HW, We can configure TWO pools for given eth Rx queue.
> > > > > One pool can be configured as a small packet pool and other one
> > > > > as large packet pool.
> > > > > And there is a threshold value to decide the pool between small
> > > > > and
> > > large.
> > > > > For example:
> > > > > - The small pool is configured 2k
> > > > > - The large pool is configured with 10k
> > > > > - And if the threshold value is configured as 2k.
> > > > > Any packet size <=2K will land in small pool and others in a large pool.
> > > > > The use case, we are targeting is to save the memory space for
> > > > > jumbo
> > > frames.
> > > >
> > > > Out of curiosity, do you provide two different buffer address in
> > > > the descriptor and HW automatically uses one based on the size, or
> > > > driver uses one of the pools based on the configuration and
> > > > possible largest packet size?
> > >
> > > I am all for allowing more configuration of buffer pool.
> > > But don't want that to be exposed as a hardware specific requirement
> > > in the API for applications. The worst case would be if your API changes
> required:
> > >
> > >   if (strcmp(dev->driver_name, "marvell") == 0) {
> > >      // make another mempool for this driver
> > >
> > I thought about adding some other segment attributes, vendor specific.
> > We could describe the segments with some descriptor structure (size,
> > pool) and add flags field to one. The proposals from other vendors are
> welcome.
> >
> 
> Please no snowflake API's "are driver is special"...
> 
> Think of how it can fit into a general model.
> Also, just because your hardware has a special feature does not mean the
> DPDK has to support it!

Sure. The initial proposal is just about how to extend the Rx buffers description.
Now it is the single pool and the single fixed segment size only. Not very flexible so far.

With best regards, Slava