[dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Matan Azrad]

It may be needed by the user to limit the LRO session packet size.
In order to allow the above limitation, add new Rx configuration for
the maximum LRO session size.

In addition, Add a new capability to expose the maximum LRO session
size supported by the port.

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 doc/guides/nics/features.rst   | 2 ++
 lib/librte_ethdev/rte_ethdev.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index c4e128d..6d1c8a5 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -193,10 +193,12 @@ LRO
 Supports Large Receive Offload.
 
 * **[uses]       rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_TCP_LRO``.
+  ``dev_conf.rxmode.max_lro_session_len``.
 * **[implements] datapath**: ``LRO functionality``.
 * **[implements] rte_eth_dev_data**: ``lro``.
 * **[provides]   mbuf**: ``mbuf.ol_flags:PKT_RX_LRO``, ``mbuf.tso_segsz``.
 * **[provides]   rte_eth_dev_info**: ``rx_offload_capa,rx_queue_offload_capa:DEV_RX_OFFLOAD_TCP_LRO``.
+* **[provides]   rte_eth_dev_info**: ``max_lro_session_len``.
 
 
 .. _nic_features_tso:
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index dc6596b..6dbb9d9 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -395,6 +395,7 @@ struct rte_eth_rxmode {
 	/** The multi-queue packet distribution mode to be used, e.g. RSS. */
 	enum rte_eth_rx_mq_mode mq_mode;
 	uint32_t max_rx_pkt_len;  /**< Only used if JUMBO_FRAME enabled. */
+	uint32_t max_lro_session_len;  /**< The maximum LRO session size. */
 	uint16_t split_hdr_size;  /**< hdr buf size (header_split enabled).*/
 	/**
 	 * Per-port Rx offloads to be set using DEV_RX_OFFLOAD_* flags.
@@ -1148,6 +1149,8 @@ struct rte_eth_dev_info {
 	const uint32_t *dev_flags; /**< Device flags */
 	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
 	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX pkt. */
+	uint32_t max_lro_session_len;
+	/**< Maximum configurable length of LRO session. */
 	uint16_t max_rx_queues; /**< Maximum number of RX queues. */
 	uint16_t max_tx_queues; /**< Maximum number of TX queues. */
 	uint32_t max_mac_addrs; /**< Maximum number of MAC addresses. */
-- 
1.8.3.1

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Ferruh Yigit]

On 8/29/2019 8:47 AM, Matan Azrad wrote:
> It may be needed by the user to limit the LRO session packet size.
> In order to allow the above limitation, add new Rx configuration for
> the maximum LRO session size.
> 
> In addition, Add a new capability to expose the maximum LRO session
> size supported by the port.
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>

Hi Matan,

Is there any existing user of this new field?

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Matan Azrad]

Hi Ferruh

From: Ferruh Yigit <ferruh.yigit@intel.com>
> On 8/29/2019 8:47 AM, Matan Azrad wrote:
> > It may be needed by the user to limit the LRO session packet size.
> > In order to allow the above limitation, add new Rx configuration for
> > the maximum LRO session size.
> >
> > In addition, Add a new capability to expose the maximum LRO session
> > size supported by the port.
> >
> > Signed-off-by: Matan Azrad <matan@mellanox.com>
> 
> Hi Matan,
> 
> Is there any existing user of this new field?

All the LRO users need it due to the next reasons:

1. If scatter is enabled - The dpdk user can limit the LRO session size created by the HW by this field, if no field like that - there is no way to limit it.
2. No scatter - the dpdk user may want to limit the LRO packet size in order to save enough tail-room in the mbuf for its own usage.
3. The limitation of max_rx_pkt_len is not enough - doesn't make sense to limit LRO traffic as single packet. 



Matan  

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Ferruh Yigit]

On 9/15/2019 8:48 AM, Matan Azrad wrote:
> Hi Ferruh
> 
> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> On 8/29/2019 8:47 AM, Matan Azrad wrote:
>>> It may be needed by the user to limit the LRO session packet size.
>>> In order to allow the above limitation, add new Rx configuration for
>>> the maximum LRO session size.
>>>
>>> In addition, Add a new capability to expose the maximum LRO session
>>> size supported by the port.
>>>
>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
>>
>> Hi Matan,
>>
>> Is there any existing user of this new field?
> 
> All the LRO users need it due to the next reasons:
> 
> 1. If scatter is enabled - The dpdk user can limit the LRO session size created by the HW by this field, if no field like that - there is no way to limit it.
> 2. No scatter - the dpdk user may want to limit the LRO packet size in order to save enough tail-room in the mbuf for its own usage.
> 3. The limitation of max_rx_pkt_len is not enough - doesn't make sense to limit LRO traffic as single packet. 
> 

So should there be more complement patches to this RFC? To update the users of
the field with the new field.

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Matan Azrad]

Hi

From: Ferruh Yigit
> On 9/15/2019 8:48 AM, Matan Azrad wrote:
> > Hi Ferruh
> >
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> On 8/29/2019 8:47 AM, Matan Azrad wrote:
> >>> It may be needed by the user to limit the LRO session packet size.
> >>> In order to allow the above limitation, add new Rx configuration for
> >>> the maximum LRO session size.
> >>>
> >>> In addition, Add a new capability to expose the maximum LRO session
> >>> size supported by the port.
> >>>
> >>> Signed-off-by: Matan Azrad <matan@mellanox.com>
> >>
> >> Hi Matan,
> >>
> >> Is there any existing user of this new field?
> >
> > All the LRO users need it due to the next reasons:
> >
> > 1. If scatter is enabled - The dpdk user can limit the LRO session size created
> by the HW by this field, if no field like that - there is no way to limit it.
> > 2. No scatter - the dpdk user may want to limit the LRO packet size in order
> to save enough tail-room in the mbuf for its own usage.
> > 3. The limitation of max_rx_pkt_len is not enough - doesn't make sense to
> limit LRO traffic as single packet.
> >
> 
> So should there be more complement patches to this RFC? To update the
> users of the field with the new field.


We already exposed it as ABI breakage in the last deprecation notice.
We probably cannot complete it for 19.11 version, hopefully for 20.02 it will be completed.

Matan

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Thomas Monjalon]

24/09/2019 14:03, Matan Azrad:
> From: Ferruh Yigit
> > On 9/15/2019 8:48 AM, Matan Azrad wrote:
> > > Hi Ferruh
> > >
> > > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > >> On 8/29/2019 8:47 AM, Matan Azrad wrote:
> > >>> It may be needed by the user to limit the LRO session packet size.
> > >>> In order to allow the above limitation, add new Rx configuration for
> > >>> the maximum LRO session size.
> > >>>
> > >>> In addition, Add a new capability to expose the maximum LRO session
> > >>> size supported by the port.
> > >>>
> > >>> Signed-off-by: Matan Azrad <matan@mellanox.com>
> > >>
> > >> Hi Matan,
> > >>
> > >> Is there any existing user of this new field?
> > >
> > > All the LRO users need it due to the next reasons:
> > >
> > > 1. If scatter is enabled - The dpdk user can limit the LRO session size created
> > by the HW by this field, if no field like that - there is no way to limit it.
> > > 2. No scatter - the dpdk user may want to limit the LRO packet size in order
> > to save enough tail-room in the mbuf for its own usage.
> > > 3. The limitation of max_rx_pkt_len is not enough - doesn't make sense to
> > limit LRO traffic as single packet.
> > >
> > 
> > So should there be more complement patches to this RFC? To update the
> > users of the field with the new field.
> 
> 
> We already exposed it as ABI breakage in the last deprecation notice.
> We probably cannot complete it for 19.11 version, hopefully for 20.02 it will be completed.

We won't break the ABI in 20.02.
What should be done in 19.11?

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Ferruh Yigit]

On 10/2/2019 2:58 PM, Thomas Monjalon wrote:
> 24/09/2019 14:03, Matan Azrad:
>> From: Ferruh Yigit
>>> On 9/15/2019 8:48 AM, Matan Azrad wrote:
>>>> Hi Ferruh
>>>>
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> On 8/29/2019 8:47 AM, Matan Azrad wrote:
>>>>>> It may be needed by the user to limit the LRO session packet size.
>>>>>> In order to allow the above limitation, add new Rx configuration for
>>>>>> the maximum LRO session size.
>>>>>>
>>>>>> In addition, Add a new capability to expose the maximum LRO session
>>>>>> size supported by the port.
>>>>>>
>>>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
>>>>>
>>>>> Hi Matan,
>>>>>
>>>>> Is there any existing user of this new field?
>>>>
>>>> All the LRO users need it due to the next reasons:
>>>>
>>>> 1. If scatter is enabled - The dpdk user can limit the LRO session size created
>>> by the HW by this field, if no field like that - there is no way to limit it.
>>>> 2. No scatter - the dpdk user may want to limit the LRO packet size in order
>>> to save enough tail-room in the mbuf for its own usage.
>>>> 3. The limitation of max_rx_pkt_len is not enough - doesn't make sense to
>>> limit LRO traffic as single packet.
>>>>
>>>
>>> So should there be more complement patches to this RFC? To update the
>>> users of the field with the new field.
>>
>>
>> We already exposed it as ABI breakage in the last deprecation notice.
>> We probably cannot complete it for 19.11 version, hopefully for 20.02 it will be completed.
> 
> We won't break the ABI in 20.02.
> What should be done in 19.11?
> 

The ask was to add code that uses new added fields, this patch only adds new
field to two public ethdev struct.

@Thomas, @Andrew, if this patch doesn't goes it on this release it will have to
wait a year. I would like to see the implementation but it is not there, what is
your comment?

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Ananyev, Konstantin]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, October 18, 2019 5:36 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Matan Azrad <matan@mellanox.com>
> Cc: dev@dpdk.org; Andrew Rybchenko <arybchenko@solarflare.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>
> Subject: Re: [RFC] ethdev: add new fields for max LRO session size
> 
> On 10/2/2019 2:58 PM, Thomas Monjalon wrote:
> > 24/09/2019 14:03, Matan Azrad:
> >> From: Ferruh Yigit
> >>> On 9/15/2019 8:48 AM, Matan Azrad wrote:
> >>>> Hi Ferruh
> >>>>
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>> On 8/29/2019 8:47 AM, Matan Azrad wrote:
> >>>>>> It may be needed by the user to limit the LRO session packet size.
> >>>>>> In order to allow the above limitation, add new Rx configuration for
> >>>>>> the maximum LRO session size.
> >>>>>>
> >>>>>> In addition, Add a new capability to expose the maximum LRO session
> >>>>>> size supported by the port.
> >>>>>>
> >>>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
> >>>>>
> >>>>> Hi Matan,
> >>>>>
> >>>>> Is there any existing user of this new field?
> >>>>
> >>>> All the LRO users need it due to the next reasons:
> >>>>
> >>>> 1. If scatter is enabled - The dpdk user can limit the LRO session size created
> >>> by the HW by this field, if no field like that - there is no way to limit it.
> >>>> 2. No scatter - the dpdk user may want to limit the LRO packet size in order
> >>> to save enough tail-room in the mbuf for its own usage.
> >>>> 3. The limitation of max_rx_pkt_len is not enough - doesn't make sense to
> >>> limit LRO traffic as single packet.
> >>>>
> >>>
> >>> So should there be more complement patches to this RFC? To update the
> >>> users of the field with the new field.
> >>
> >>
> >> We already exposed it as ABI breakage in the last deprecation notice.
> >> We probably cannot complete it for 19.11 version, hopefully for 20.02 it will be completed.
> >
> > We won't break the ABI in 20.02.
> > What should be done in 19.11?
> >
> 
> The ask was to add code that uses new added fields, this patch only adds new
> field to two public ethdev struct.
> 
> @Thomas, @Andrew, if this patch doesn't goes it on this release it will have to
> wait a year. I would like to see the implementation but it is not there, what is
> your comment?

Just a side note, if I am not mistaken, there is a 6B gap in eth_rxmode:

struct rte_eth_rxmode {
	/** The multi-queue packet distribution mode to be used, e.g. RSS. */
	enum rte_eth_rx_mq_mode mq_mode;
	uint32_t max_rx_pkt_len;  /**< Only used if JUMBO_FRAME enabled. */
	uint16_t split_hdr_size;  /**< hdr buf size (header_split enabled).*/   <---- offset 8
	/**
	 * Per-port Rx offloads to be set using DEV_RX_OFFLOAD_* flags.
	 * Only offloads set on rx_offload_capa field on rte_eth_dev_info
	 * structure are allowed to be set.
	 */
	uint64_t offloads;        <--- offset 16
};


So we can reserve these 6B, and then reuse for LRO, or whatever.
Might be it would help somehow.

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Andrew Rybchenko]

On 10/18/19 7:35 PM, Ferruh Yigit wrote:
> On 10/2/2019 2:58 PM, Thomas Monjalon wrote:
>> 24/09/2019 14:03, Matan Azrad:
>>> From: Ferruh Yigit
>>>> On 9/15/2019 8:48 AM, Matan Azrad wrote:
>>>>> Hi Ferruh
>>>>>
>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>> On 8/29/2019 8:47 AM, Matan Azrad wrote:
>>>>>>> It may be needed by the user to limit the LRO session packet size.
>>>>>>> In order to allow the above limitation, add new Rx configuration for
>>>>>>> the maximum LRO session size.
>>>>>>>
>>>>>>> In addition, Add a new capability to expose the maximum LRO session
>>>>>>> size supported by the port.
>>>>>>>
>>>>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
>>>>>> Hi Matan,
>>>>>>
>>>>>> Is there any existing user of this new field?
>>>>> All the LRO users need it due to the next reasons:
>>>>>
>>>>> 1. If scatter is enabled - The dpdk user can limit the LRO session size created
>>>> by the HW by this field, if no field like that - there is no way to limit it.
>>>>> 2. No scatter - the dpdk user may want to limit the LRO packet size in order
>>>> to save enough tail-room in the mbuf for its own usage.
>>>>> 3. The limitation of max_rx_pkt_len is not enough - doesn't make sense to
>>>> limit LRO traffic as single packet.
>>>> So should there be more complement patches to this RFC? To update the
>>>> users of the field with the new field.
>>>
>>> We already exposed it as ABI breakage in the last deprecation notice.
>>> We probably cannot complete it for 19.11 version, hopefully for 20.02 it will be completed.
>> We won't break the ABI in 20.02.
>> What should be done in 19.11?
>>
> The ask was to add code that uses new added fields, this patch only adds new
> field to two public ethdev struct.
>
> @Thomas, @Andrew, if this patch doesn't goes it on this release it will have to
> wait a year. I would like to see the implementation but it is not there, what is
> your comment?

I don't mind to accept it in 19.11 modulo better description of
what is LRO session length/size.

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Matan Azrad]

Hi All

From: Andrew Rybchenko
> On 10/18/19 7:35 PM, Ferruh Yigit wrote:
> > On 10/2/2019 2:58 PM, Thomas Monjalon wrote:
> >> 24/09/2019 14:03, Matan Azrad:
> >>> From: Ferruh Yigit
> >>>> On 9/15/2019 8:48 AM, Matan Azrad wrote:
> >>>>> Hi Ferruh
> >>>>>
> >>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>>> On 8/29/2019 8:47 AM, Matan Azrad wrote:
> >>>>>>> It may be needed by the user to limit the LRO session packet size.
> >>>>>>> In order to allow the above limitation, add new Rx configuration
> >>>>>>> for the maximum LRO session size.
> >>>>>>>
> >>>>>>> In addition, Add a new capability to expose the maximum LRO
> >>>>>>> session size supported by the port.
> >>>>>>>
> >>>>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
> >>>>>> Hi Matan,
> >>>>>>
> >>>>>> Is there any existing user of this new field?
> >>>>> All the LRO users need it due to the next reasons:
> >>>>>
> >>>>> 1. If scatter is enabled - The dpdk user can limit the LRO session
> >>>>> size created
> >>>> by the HW by this field, if no field like that - there is no way to limit it.
> >>>>> 2. No scatter - the dpdk user may want to limit the LRO packet
> >>>>> size in order
> >>>> to save enough tail-room in the mbuf for its own usage.
> >>>>> 3. The limitation of max_rx_pkt_len is not enough - doesn't make
> >>>>> sense to
> >>>> limit LRO traffic as single packet.
> >>>> So should there be more complement patches to this RFC? To update
> >>>> the users of the field with the new field.
> >>>
> >>> We already exposed it as ABI breakage in the last deprecation notice.
> >>> We probably cannot complete it for 19.11 version, hopefully for 20.02 it
> will be completed.
> >> We won't break the ABI in 20.02.
> >> What should be done in 19.11?
> >>
> > The ask was to add code that uses new added fields, this patch only
> > adds new field to two public ethdev struct.
> >
> > @Thomas, @Andrew, if this patch doesn't goes it on this release it
> > will have to wait a year. I would like to see the implementation but
> > it is not there, what is your comment?
> 
> I don't mind to accept it in 19.11 modulo better description of what is LRO
> session length/size.


Thanks,

We can create the patch for ethdev for 19.11-RC2.

Also PMD implementation to use it for RC2.

Is it OK? (We need to break ABI as described in the deprecation notice)

Matan 

Re: [dpdk-dev] [RFC] ethdev: add new fields for max LRO session size

[Ferruh Yigit]

On 10/27/2019 9:04 AM, Matan Azrad wrote:
> Hi All
> 
> From: Andrew Rybchenko
>> On 10/18/19 7:35 PM, Ferruh Yigit wrote:
>>> On 10/2/2019 2:58 PM, Thomas Monjalon wrote:
>>>> 24/09/2019 14:03, Matan Azrad:
>>>>> From: Ferruh Yigit
>>>>>> On 9/15/2019 8:48 AM, Matan Azrad wrote:
>>>>>>> Hi Ferruh
>>>>>>>
>>>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>>> On 8/29/2019 8:47 AM, Matan Azrad wrote:
>>>>>>>>> It may be needed by the user to limit the LRO session packet size.
>>>>>>>>> In order to allow the above limitation, add new Rx configuration
>>>>>>>>> for the maximum LRO session size.
>>>>>>>>>
>>>>>>>>> In addition, Add a new capability to expose the maximum LRO
>>>>>>>>> session size supported by the port.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
>>>>>>>> Hi Matan,
>>>>>>>>
>>>>>>>> Is there any existing user of this new field?
>>>>>>> All the LRO users need it due to the next reasons:
>>>>>>>
>>>>>>> 1. If scatter is enabled - The dpdk user can limit the LRO session
>>>>>>> size created
>>>>>> by the HW by this field, if no field like that - there is no way to limit it.
>>>>>>> 2. No scatter - the dpdk user may want to limit the LRO packet
>>>>>>> size in order
>>>>>> to save enough tail-room in the mbuf for its own usage.
>>>>>>> 3. The limitation of max_rx_pkt_len is not enough - doesn't make
>>>>>>> sense to
>>>>>> limit LRO traffic as single packet.
>>>>>> So should there be more complement patches to this RFC? To update
>>>>>> the users of the field with the new field.
>>>>>
>>>>> We already exposed it as ABI breakage in the last deprecation notice.
>>>>> We probably cannot complete it for 19.11 version, hopefully for 20.02 it
>> will be completed.
>>>> We won't break the ABI in 20.02.
>>>> What should be done in 19.11?
>>>>
>>> The ask was to add code that uses new added fields, this patch only
>>> adds new field to two public ethdev struct.
>>>
>>> @Thomas, @Andrew, if this patch doesn't goes it on this release it
>>> will have to wait a year. I would like to see the implementation but
>>> it is not there, what is your comment?
>>
>> I don't mind to accept it in 19.11 modulo better description of what is LRO
>> session length/size.
> 
> 
> Thanks,
> 
> We can create the patch for ethdev for 19.11-RC2.
> 
> Also PMD implementation to use it for RC2.
> 
> Is it OK? (We need to break ABI as described in the deprecation notice)
> 

OK from me to have ethdev update and PMD implementation for rc2.