[dpdk-dev] [RFC] doc: announce max Rx packet len field deprecation

[dpdk-dev] [RFC] doc: announce max Rx packet len field deprecation

[Ferruh Yigit]

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
Cc: Matan Azrad <matan@nvidia.com>
Cc: Olivier Matz <olivier.matz@6wind.com>
Cc: Jerin Jacob <jerinj@marvell.com>
---
 doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 8ceb385141..d4a31392d3 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -149,6 +149,31 @@ Deprecation Notices
   will be limited to maximum 256 queues.
   Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
 
+* ethdev: In ``struct rte_eth_rxmode``, ``uint32_t max_rx_pkt_len`` will be
+  replaced by a new ``uint32_t mtu`` in v21.11.
+  The new ``mtu`` field will be used to configure the initial device MTU via
+  ``rte_eth_dev_configure()`` API.
+  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
+  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be used to store
+  the configured ``mtu`` value,
+  and this new ``(struct rte_eth_dev)->data->dev_conf.rxmode.mtu`` variable will
+  be used to store the user configuration request.
+  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME`` enabled,
+  ``mtu`` field will be always valid.
+  When ``mtu`` config is not provided by the application, default ``RTE_ETHER_MTU``
+  value will be used.
+  Driver is responsible from updating ``(struct rte_eth_dev)->data->mtu`` after MTU
+  set successfully, either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
+
+  Application may need to configure device for a specific Rx packet size, like for
+  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and device received packet size
+  can't be bigger than Rx buffer size.
+  To cover these cases application needs to know the device packet overhead to be
+  able to calculate the ``mtu`` corresponding to a Rx buffer size, for this
+  ``(struct rte_eth_dev_info).max_rx_pktlen`` will be kept,
+  the device packet overhead can be calculated as:
+  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct rte_eth_dev_info).max_mtu``
+
 * cryptodev: support for using IV with all sizes is added, J0 still can
   be used but only when IV length in following structs ``rte_crypto_auth_xform``,
   ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or equal
-- 
2.26.2

Re: [dpdk-dev] [RFC] doc: announce max Rx packet len field deprecation

[Ananyev, Konstantin]

> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Matan Azrad <matan@nvidia.com>
> Cc: Olivier Matz <olivier.matz@6wind.com>
> Cc: Jerin Jacob <jerinj@marvell.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 8ceb385141..d4a31392d3 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -149,6 +149,31 @@ Deprecation Notices
>    will be limited to maximum 256 queues.
>    Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
> 
> +* ethdev: In ``struct rte_eth_rxmode``, ``uint32_t max_rx_pkt_len`` will be
> +  replaced by a new ``uint32_t mtu`` in v21.11.

Probably no point to keep mtu value in rte_eth_rxmode.
Better to move it to rte_eth_conf.
Apart from that: +1 for this change.
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> +  The new ``mtu`` field will be used to configure the initial device MTU via
> +  ``rte_eth_dev_configure()`` API.
> +  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
> +  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be used to store
> +  the configured ``mtu`` value,
> +  and this new ``(struct rte_eth_dev)->data->dev_conf.rxmode.mtu`` variable will
> +  be used to store the user configuration request.
> +  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME`` enabled,
> +  ``mtu`` field will be always valid.
> +  When ``mtu`` config is not provided by the application, default ``RTE_ETHER_MTU``
> +  value will be used.
> +  Driver is responsible from updating ``(struct rte_eth_dev)->data->mtu`` after MTU
> +  set successfully, either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
> +
> +  Application may need to configure device for a specific Rx packet size, like for
> +  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and device received packet size
> +  can't be bigger than Rx buffer size.
> +  To cover these cases application needs to know the device packet overhead to be
> +  able to calculate the ``mtu`` corresponding to a Rx buffer size, for this
> +  ``(struct rte_eth_dev_info).max_rx_pktlen`` will be kept,
> +  the device packet overhead can be calculated as:
> +  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct rte_eth_dev_info).max_mtu``
> +
>  * cryptodev: support for using IV with all sizes is added, J0 still can
>    be used but only when IV length in following structs ``rte_crypto_auth_xform``,
>    ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or equal
> --
> 2.26.2

Re: [dpdk-dev] [RFC] doc: announce max Rx packet len field deprecation

[Kinsella, Ray]

On 20/10/2020 13:03, Ferruh Yigit wrote:
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Matan Azrad <matan@nvidia.com>
> Cc: Olivier Matz <olivier.matz@6wind.com>
> Cc: Jerin Jacob <jerinj@marvell.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 8ceb385141..d4a31392d3 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -149,6 +149,31 @@ Deprecation Notices
>    will be limited to maximum 256 queues.
>    Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
>  
> +* ethdev: In ``struct rte_eth_rxmode``, ``uint32_t max_rx_pkt_len`` will be
> +  replaced by a new ``uint32_t mtu`` in v21.11.
> +  The new ``mtu`` field will be used to configure the initial device MTU via
> +  ``rte_eth_dev_configure()`` API.
> +  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
> +  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be used to store
> +  the configured ``mtu`` value,
> +  and this new ``(struct rte_eth_dev)->data->dev_conf.rxmode.mtu`` variable will
> +  be used to store the user configuration request.
> +  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME`` enabled,
> +  ``mtu`` field will be always valid.
> +  When ``mtu`` config is not provided by the application, default ``RTE_ETHER_MTU``
> +  value will be used.
> +  Driver is responsible from updating ``(struct rte_eth_dev)->data->mtu`` after MTU
> +  set successfully, either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
> +
> +  Application may need to configure device for a specific Rx packet size, like for

An application ... 

> +  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and device received packet size
> +  can't be bigger than Rx buffer size.
> +  To cover these cases application needs to know the device packet overhead to be

An application ... 

> +  able to calculate the ``mtu`` corresponding to a Rx buffer size, for this
> +  ``(struct rte_eth_dev_info).max_rx_pktlen`` will be kept,
> +  the device packet overhead can be calculated as:
> +  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct rte_eth_dev_info).max_mtu``
> +
>  * cryptodev: support for using IV with all sizes is added, J0 still can
>    be used but only when IV length in following structs ``rte_crypto_auth_xform``,
>    ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or equal
> 

Re: [dpdk-dev] [RFC] doc: announce max Rx packet len field deprecation

[Andrew Rybchenko]

On 10/21/20 1:18 PM, Ananyev, Konstantin wrote:
>
>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Thomas Monjalon <thomas@monjalon.net>
>> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
>> Cc: Matan Azrad <matan@nvidia.com>
>> Cc: Olivier Matz <olivier.matz@6wind.com>
>> Cc: Jerin Jacob <jerinj@marvell.com>
>> ---
>>   doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>> index 8ceb385141..d4a31392d3 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -149,6 +149,31 @@ Deprecation Notices
>>     will be limited to maximum 256 queues.
>>     Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
>>
>> +* ethdev: In ``struct rte_eth_rxmode``, ``uint32_t max_rx_pkt_len`` will be
>> +  replaced by a new ``uint32_t mtu`` in v21.11.
> Probably no point to keep mtu value in rte_eth_rxmode.
> Better to move it to rte_eth_conf.
> Apart from that: +1 for this change.
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Do we really need the field in either rte_eth_rxmode or rte_eth_conf?
What's the point of duplication? We have dedicated API to get/set
which could be called in stopped state, set value saved in
data->mtu and used by the driver at start up and Rx queues setup
(to check scatter vs data room in mbuf consistency).

>> +  The new ``mtu`` field will be used to configure the initial device MTU via
>> +  ``rte_eth_dev_configure()`` API.
>> +  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
>> +  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be used to store
>> +  the configured ``mtu`` value,
>> +  and this new ``(struct rte_eth_dev)->data->dev_conf.rxmode.mtu`` variable will
>> +  be used to store the user configuration request.
>> +  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME`` enabled,
>> +  ``mtu`` field will be always valid.
>> +  When ``mtu`` config is not provided by the application, default ``RTE_ETHER_MTU``
>> +  value will be used.
>> +  Driver is responsible from updating ``(struct rte_eth_dev)->data->mtu`` after MTU
>> +  set successfully, either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
>> +
>> +  Application may need to configure device for a specific Rx packet size, like for
>> +  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and device received packet size
>> +  can't be bigger than Rx buffer size.
>> +  To cover these cases application needs to know the device packet overhead to be
>> +  able to calculate the ``mtu`` corresponding to a Rx buffer size, for this
>> +  ``(struct rte_eth_dev_info).max_rx_pktlen`` will be kept,
>> +  the device packet overhead can be calculated as:
>> +  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct rte_eth_dev_info).max_mtu``
>> +
>>   * cryptodev: support for using IV with all sizes is added, J0 still can
>>     be used but only when IV length in following structs ``rte_crypto_auth_xform``,
>>     ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or equal
>> --
>> 2.26.2

Re: [dpdk-dev] [RFC] doc: announce max Rx packet len field deprecation

[Ferruh Yigit]

On 10/21/2020 4:10 PM, Andrew Rybchenko wrote:
> On 10/21/20 1:18 PM, Ananyev, Konstantin wrote:
>>> Signed-off-by: Ferruh Yigit<ferruh.yigit@intel.com>
>>> ---
>>> Cc: Thomas Monjalon<thomas@monjalon.net>
>>> Cc: Andrew Rybchenko<arybchenko@solarflare.com>
>>> Cc: Konstantin Ananyev<konstantin.ananyev@intel.com>
>>> Cc: Matan Azrad<matan@nvidia.com>
>>> Cc: Olivier Matz<olivier.matz@6wind.com>
>>> Cc: Jerin Jacob<jerinj@marvell.com>
>>> ---
>>>   doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
>>>   1 file changed, 25 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>>> index 8ceb385141..d4a31392d3 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -149,6 +149,31 @@ Deprecation Notices
>>>     will be limited to maximum 256 queues.
>>>     Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
>>>
>>> +* ethdev: In ``struct rte_eth_rxmode``, ``uint32_t max_rx_pkt_len`` will be
>>> +  replaced by a new ``uint32_t mtu`` in v21.11.
>> Probably no point to keep mtu value in rte_eth_rxmode.
>> Better to move it to rte_eth_conf.
>> Apart from that: +1 for this change.
>> Acked-by: Konstantin Ananyev<konstantin.ananyev@intel.com>
> 
> Do we really need the field in either rte_eth_rxmode or rte_eth_conf?
> What's the point of duplication? We have dedicated API to get/set
> which could be called in stopped state, set value saved in
> data->mtu and used by the driver at start up and Rx queues setup
> (to check scatter vs data room in mbuf consistency).
> 

Not sure if we really need it, I had same thought.

The benefit of having it is, user can configure the Rx packet size within the 
``rte_eth_dev_configure()`` API, without it user will need to call the 
``rte_eth_dev_set_mtu()`` API explicitly, which will add another mandatory call 
to the device initialization, so I think having this fields simplifies the 
initialization.

Having it has the duplication problem, same thing configured by two different APIs.

>>> +  The new ``mtu`` field will be used to configure the initial device MTU via
>>> +  ``rte_eth_dev_configure()`` API.
>>> +  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
>>> +  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be used to store
>>> +  the configured ``mtu`` value,
>>> +  and this new ``(struct rte_eth_dev)->data->dev_conf.rxmode.mtu`` variable will
>>> +  be used to store the user configuration request.
>>> +  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME`` enabled,
>>> +  ``mtu`` field will be always valid.
>>> +  When ``mtu`` config is not provided by the application, default ``RTE_ETHER_MTU``
>>> +  value will be used.
>>> +  Driver is responsible from updating ``(struct rte_eth_dev)->data->mtu`` after MTU
>>> +  set successfully, either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
>>> +
>>> +  Application may need to configure device for a specific Rx packet size, like for
>>> +  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and device received packet size
>>> +  can't be bigger than Rx buffer size.
>>> +  To cover these cases application needs to know the device packet overhead to be
>>> +  able to calculate the ``mtu`` corresponding to a Rx buffer size, for this
>>> +  ``(struct rte_eth_dev_info).max_rx_pktlen`` will be kept,
>>> +  the device packet overhead can be calculated as:
>>> +  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct rte_eth_dev_info).max_mtu``
>>> +
>>>   * cryptodev: support for using IV with all sizes is added, J0 still can
>>>     be used but only when IV length in following structs ``rte_crypto_auth_xform``,
>>>     ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or equal
>>> --
>>> 2.26.2
> 

[dpdk-dev] [RFC v2] doc: announce max Rx packet len field deprecation

[Ferruh Yigit]

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
Cc: Matan Azrad <matan@nvidia.com>
Cc: Olivier Matz <olivier.matz@6wind.com>
Cc: Jerin Jacob <jerinj@marvell.com>

v2:
* ``uint32_t mtu`` moved to ``struct rte_eth_conf``
* The "Driver is responsible from updating ``(struct
  rte_eth_dev)->data->mtu``" updated because ethdev layer also can do
  this. The intention there was both APIs should update the variable.

Another open question is from Andrew, if we can remove the ``uint32_t
max_rx_pkt_len`` completely from the ``rte_eth_dev_configure()``.
This may force applications to have one more additional
``rte_eth_dev_set_mtu()`` call for device initialization, but if
applications are OK with the default values most of times, agree that
removing is easier solution, please comment.
---
 doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 96986fabd598..73e43581d615 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -88,6 +88,31 @@ Deprecation Notices
   will be limited to maximum 256 queues.
   Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
 
+* ethdev: ``uint32_t max_rx_pkt_len`` field of ``struct rte_eth_rxmode``, will be
+  replaced by a new ``uint32_t mtu`` field of ``struct rte_eth_conf`` in v21.11.
+  The new ``mtu`` field will be used to configure the initial device MTU via
+  ``rte_eth_dev_configure()`` API.
+  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
+  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be used to store
+  the configured ``mtu`` value,
+  and this new ``(struct rte_eth_dev)->data->dev_conf.mtu`` variable will
+  be used to store the user configuration request.
+  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME`` enabled,
+  ``mtu`` field will be always valid.
+  When ``mtu`` config is not provided by the application, default ``RTE_ETHER_MTU``
+  value will be used.
+  ``(struct rte_eth_dev)->data->mtu`` should be updated after MTU set successfully,
+  either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
+
+  An application may need to configure device for a specific Rx packet size, like for
+  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and device received packet size
+  can't be bigger than Rx buffer size.
+  To cover these cases an application needs to know the device packet overhead to be
+  able to calculate the ``mtu`` corresponding to a Rx buffer size, for this
+  ``(struct rte_eth_dev_info).max_rx_pktlen`` will be kept,
+  the device packet overhead can be calculated as:
+  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct rte_eth_dev_info).max_mtu``
+
 * sched: To allow more traffic classes, flexible mapping of pipe queues to
   traffic classes, and subport level configuration of pipes and queues
   changes will be made to macros, data structures and API functions defined
-- 
2.26.2

Re: [dpdk-dev] [RFC v2] doc: announce max Rx packet len field deprecation

[Ajit Khaparde]

On Tue, Nov 24, 2020 at 9:36 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Matan Azrad <matan@nvidia.com>
> Cc: Olivier Matz <olivier.matz@6wind.com>
> Cc: Jerin Jacob <jerinj@marvell.com>
>
> v2:
> * ``uint32_t mtu`` moved to ``struct rte_eth_conf``
> * The "Driver is responsible from updating ``(struct
>   rte_eth_dev)->data->mtu``" updated because ethdev layer also can do
>   this. The intention there was both APIs should update the variable.
>
> Another open question is from Andrew, if we can remove the ``uint32_t
> max_rx_pkt_len`` completely from the ``rte_eth_dev_configure()``.
> This may force applications to have one more additional
> ``rte_eth_dev_set_mtu()`` call for device initialization, but if
> applications are OK with the default values most of times, agree that
> removing is easier solution, please comment.
> ---
>  doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 96986fabd598..73e43581d615 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -88,6 +88,31 @@ Deprecation Notices
>    will be limited to maximum 256 queues.
>    Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
>
> +* ethdev: ``uint32_t max_rx_pkt_len`` field of ``struct rte_eth_rxmode``, will be
> +  replaced by a new ``uint32_t mtu`` field of ``struct rte_eth_conf`` in v21.11.
> +  The new ``mtu`` field will be used to configure the initial device MTU via
> +  ``rte_eth_dev_configure()`` API.
> +  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
> +  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be used to store
> +  the configured ``mtu`` value,
> +  and this new ``(struct rte_eth_dev)->data->dev_conf.mtu`` variable will
> +  be used to store the user configuration request.
> +  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME`` enabled,
> +  ``mtu`` field will be always valid.
> +  When ``mtu`` config is not provided by the application, default ``RTE_ETHER_MTU``
> +  value will be used.
> +  ``(struct rte_eth_dev)->data->mtu`` should be updated after MTU set successfully,
> +  either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
> +
> +  An application may need to configure device for a specific Rx packet size, like for
> +  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and device received packet size
> +  can't be bigger than Rx buffer size.
> +  To cover these cases an application needs to know the device packet overhead to be
> +  able to calculate the ``mtu`` corresponding to a Rx buffer size, for this
> +  ``(struct rte_eth_dev_info).max_rx_pktlen`` will be kept,
> +  the device packet overhead can be calculated as:
> +  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct rte_eth_dev_info).max_mtu``
> +
>  * sched: To allow more traffic classes, flexible mapping of pipe queues to
>    traffic classes, and subport level configuration of pipes and queues
>    changes will be made to macros, data structures and API functions defined
> --
> 2.26.2
>

Re: [dpdk-dev] [RFC v2] doc: announce max Rx packet len field deprecation

[Andrew Rybchenko]

On 11/24/20 8:36 PM, Ferruh Yigit wrote:
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

A couple of questions below, but anyway:

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

> ---
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Matan Azrad <matan@nvidia.com>
> Cc: Olivier Matz <olivier.matz@6wind.com>
> Cc: Jerin Jacob <jerinj@marvell.com>
> 
> v2:
> * ``uint32_t mtu`` moved to ``struct rte_eth_conf``
> * The "Driver is responsible from updating ``(struct
>   rte_eth_dev)->data->mtu``" updated because ethdev layer also can do
>   this. The intention there was both APIs should update the variable.
> 
> Another open question is from Andrew, if we can remove the ``uint32_t
> max_rx_pkt_len`` completely from the ``rte_eth_dev_configure()``.
> This may force applications to have one more additional
> ``rte_eth_dev_set_mtu()`` call for device initialization, but if
> applications are OK with the default values most of times, agree that
> removing is easier solution, please comment.

Still valid plus I'd remove JUMBO_FRAME offload since
it is redundant. We have max_mtu and max_rx_pktlen in dev_info.

Re: [dpdk-dev] [RFC v2] doc: announce max Rx packet len field deprecation

[Ferruh Yigit]

On 11/26/2020 11:28 AM, Andrew Rybchenko wrote:
> On 11/24/20 8:36 PM, Ferruh Yigit wrote:
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> A couple of questions below, but anyway:
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
>> ---
>> Cc: Thomas Monjalon <thomas@monjalon.net>
>> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
>> Cc: Matan Azrad <matan@nvidia.com>
>> Cc: Olivier Matz <olivier.matz@6wind.com>
>> Cc: Jerin Jacob <jerinj@marvell.com>
>>
>> v2:
>> * ``uint32_t mtu`` moved to ``struct rte_eth_conf``
>> * The "Driver is responsible from updating ``(struct
>>    rte_eth_dev)->data->mtu``" updated because ethdev layer also can do
>>    this. The intention there was both APIs should update the variable.
>>
>> Another open question is from Andrew, if we can remove the ``uint32_t
>> max_rx_pkt_len`` completely from the ``rte_eth_dev_configure()``.
>> This may force applications to have one more additional
>> ``rte_eth_dev_set_mtu()`` call for device initialization, but if
>> applications are OK with the default values most of times, agree that
>> removing is easier solution, please comment.
> 
> Still valid 

Yep, waiting for more comments for it.

> plus I'd remove JUMBO_FRAME offload since
> it is redundant. We have max_mtu and max_rx_pktlen in dev_info.
> 

Right, I missed that 'max_mtu' & 'max_rx_pktlen' can be used to detect jumbo 
frame capability. +1 to remove JUMBO_FRAME offload.

I don't know if should it be part of this deprecation notice, or a separate one.
It is related, but logically not exactly part of this deprecation notice.

Re: [dpdk-dev] [RFC v2] doc: announce max Rx packet len field deprecation

[Matan Azrad]

Hi

Looks very good direction - it will do some order here.

Will be also good to define what is MTU exactly:
Maximum Rx packet size \ Maximum Tx packet size \ both?
Is it include L2\L3 headers? Vlan?
How is it going together with ``uint32_t max_lro_pkt_size``?

Also when should the user configure DEV_RX_OFFLOAD_SCATTER?

Matan

From: Ferruh Yigit
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Matan Azrad <matan@nvidia.com>
> Cc: Olivier Matz <olivier.matz@6wind.com>
> Cc: Jerin Jacob <jerinj@marvell.com>
> 
> v2:
> * ``uint32_t mtu`` moved to ``struct rte_eth_conf``
> * The "Driver is responsible from updating ``(struct
>   rte_eth_dev)->data->mtu``" updated because ethdev layer also can do
>   this. The intention there was both APIs should update the variable.
> 
> Another open question is from Andrew, if we can remove the ``uint32_t
> max_rx_pkt_len`` completely from the ``rte_eth_dev_configure()``.
> This may force applications to have one more additional
> ``rte_eth_dev_set_mtu()`` call for device initialization, but if applications are
> OK with the default values most of times, agree that removing is easier
> solution, please comment.
> ---
>  doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 96986fabd598..73e43581d615 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -88,6 +88,31 @@ Deprecation Notices
>    will be limited to maximum 256 queues.
>    Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be
> removed.
> 
> +* ethdev: ``uint32_t max_rx_pkt_len`` field of ``struct
> +rte_eth_rxmode``, will be
> +  replaced by a new ``uint32_t mtu`` field of ``struct rte_eth_conf`` in v21.11.
> +  The new ``mtu`` field will be used to configure the initial device
> +MTU via
> +  ``rte_eth_dev_configure()`` API.
> +  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done now.
> +  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be
> +used to store
> +  the configured ``mtu`` value,
> +  and this new ``(struct rte_eth_dev)->data->dev_conf.mtu`` variable
> +will
> +  be used to store the user configuration request.
> +  Unlike ``max_rx_pkt_len``, which was valid only when ``JUMBO_FRAME``
> +enabled,
> +  ``mtu`` field will be always valid.
> +  When ``mtu`` config is not provided by the application, default
> +``RTE_ETHER_MTU``
> +  value will be used.
> +  ``(struct rte_eth_dev)->data->mtu`` should be updated after MTU set
> +successfully,
> +  either by ``rte_eth_dev_configure()`` or ``rte_eth_dev_set_mtu()``.
> +
> +  An application may need to configure device for a specific Rx packet
> + size, like for  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported and
> + device received packet size  can't be bigger than Rx buffer size.
> +  To cover these cases an application needs to know the device packet
> + overhead to be  able to calculate the ``mtu`` corresponding to a Rx
> + buffer size, for this  ``(struct rte_eth_dev_info).max_rx_pktlen``
> + will be kept,  the device packet overhead can be calculated as:
> +  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct
> + rte_eth_dev_info).max_mtu``
> +
>  * sched: To allow more traffic classes, flexible mapping of pipe queues to
>    traffic classes, and subport level configuration of pipes and queues
>    changes will be made to macros, data structures and API functions defined
> --
> 2.26.2

Re: [dpdk-dev] [RFC v2] doc: announce max Rx packet len fielddeprecation

[Morten Brørup]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> Sent: Thursday, November 26, 2020 7:31 PM
> 
> Hi
> 
> Looks very good direction - it will do some order here.
> 
> Will be also good to define what is MTU exactly:
> Maximum Rx packet size \ Maximum Tx packet size \ both?

If we cannot find a use case requiring different RX and TX packet size limits, I prefer combining them into one. And in this case, "mtu" (Max TRANSMISSION Unit) is not the optimal name.

Furthermore, the conventional use of "MTU" does not count the L2 headers.

> Is it include L2\L3 headers? Vlan?

I once proposed that it should count what goes on the wire, i.e. including L2 and any VLAN headers. But instead, it was decided to mimic what Linux does: Do not count single VLAN headers, but do count double VLAN headers. I don't know if double VLAN headers only count one of the VLAN headers or both. And I don't know if single VLAN headers are counted if VLAN stripping/appending is disabled.

In other words: Matan has a good point here... It should be well defined (and well documented)!

> How is it going together with ``uint32_t max_lro_pkt_size``?
> 
> Also when should the user configure DEV_RX_OFFLOAD_SCATTER?
> 
> Matan
> 
> From: Ferruh Yigit
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > Cc: Thomas Monjalon <thomas@monjalon.net>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> > Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > Cc: Matan Azrad <matan@nvidia.com>
> > Cc: Olivier Matz <olivier.matz@6wind.com>
> > Cc: Jerin Jacob <jerinj@marvell.com>
> >
> > v2:
> > * ``uint32_t mtu`` moved to ``struct rte_eth_conf``
> > * The "Driver is responsible from updating ``(struct
> >   rte_eth_dev)->data->mtu``" updated because ethdev layer also can do
> >   this. The intention there was both APIs should update the variable.
> >
> > Another open question is from Andrew, if we can remove the ``uint32_t
> > max_rx_pkt_len`` completely from the ``rte_eth_dev_configure()``.
> > This may force applications to have one more additional
> > ``rte_eth_dev_set_mtu()`` call for device initialization, but if
> applications are
> > OK with the default values most of times, agree that removing is
> easier
> > solution, please comment.
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index 96986fabd598..73e43581d615 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -88,6 +88,31 @@ Deprecation Notices
> >    will be limited to maximum 256 queues.
> >    Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be
> > removed.
> >
> > +* ethdev: ``uint32_t max_rx_pkt_len`` field of ``struct
> > +rte_eth_rxmode``, will be
> > +  replaced by a new ``uint32_t mtu`` field of ``struct
> rte_eth_conf`` in v21.11.
> > +  The new ``mtu`` field will be used to configure the initial device
> > +MTU via
> > +  ``rte_eth_dev_configure()`` API.
> > +  Later MTU can be changed by ``rte_eth_dev_set_mtu()`` API as done
> now.
> > +  The existing ``(struct rte_eth_dev)->data->mtu`` variable will be
> > +used to store
> > +  the configured ``mtu`` value,
> > +  and this new ``(struct rte_eth_dev)->data->dev_conf.mtu`` variable
> > +will
> > +  be used to store the user configuration request.
> > +  Unlike ``max_rx_pkt_len``, which was valid only when
> ``JUMBO_FRAME``
> > +enabled,
> > +  ``mtu`` field will be always valid.
> > +  When ``mtu`` config is not provided by the application, default
> > +``RTE_ETHER_MTU``
> > +  value will be used.
> > +  ``(struct rte_eth_dev)->data->mtu`` should be updated after MTU
> set
> > +successfully,
> > +  either by ``rte_eth_dev_configure()`` or
> ``rte_eth_dev_set_mtu()``.
> > +
> > +  An application may need to configure device for a specific Rx
> packet
> > + size, like for  cases ``DEV_RX_OFFLOAD_SCATTER`` is not supported
> and
> > + device received packet size  can't be bigger than Rx buffer size.
> > +  To cover these cases an application needs to know the device
> packet
> > + overhead to be  able to calculate the ``mtu`` corresponding to a Rx
> > + buffer size, for this  ``(struct rte_eth_dev_info).max_rx_pktlen``
> > + will be kept,  the device packet overhead can be calculated as:
> > +  ``(struct rte_eth_dev_info).max_rx_pktlen - (struct
> > + rte_eth_dev_info).max_mtu``
> > +

Please note that the vmxnet3 driver behaves different that other NIC drivers regarding when it can be configured in Linux. The interface needs to be running before it accepts many of the configuration parameters. I don't know if it behaves similarly in DPDK.

I have CC'ed the vmxnet3 maintainer, Yong Wang, in case VMware has an opinion regarding this RFC.

> >  * sched: To allow more traffic classes, flexible mapping of pipe
> queues to
> >    traffic classes, and subport level configuration of pipes and
> queues
> >    changes will be made to macros, data structures and API functions
> defined
> > --
> > 2.26.2
> 

Re: [dpdk-dev] [RFC v2] doc: announce max Rx packet len field deprecation

[Thomas Monjalon]

26/11/2020 13:34, Ferruh Yigit:
> On 11/26/2020 11:28 AM, Andrew Rybchenko wrote:
> > On 11/24/20 8:36 PM, Ferruh Yigit wrote:
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > 
> > A couple of questions below, but anyway:
> > 
> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > 
> >> Another open question is from Andrew, if we can remove the ``uint32_t
> >> max_rx_pkt_len`` completely from the ``rte_eth_dev_configure()``.
> >> This may force applications to have one more additional
> >> ``rte_eth_dev_set_mtu()`` call for device initialization, but if
> >> applications are OK with the default values most of times, agree that
> >> removing is easier solution, please comment.
> > 
> > Still valid 
> 
> Yep, waiting for more comments for it.

In general, I am in favor of
	- avoiding redundancy in API
	- have more specific functions in API
So yes, no need to keep a field for rte_eth_dev_configure()
if the same can be done with rte_eth_dev_set_mtu().

> > plus I'd remove JUMBO_FRAME offload since
> > it is redundant. We have max_mtu and max_rx_pktlen in dev_info.
> 
> Right, I missed that 'max_mtu' & 'max_rx_pktlen' can be used to detect jumbo 
> frame capability. +1 to remove JUMBO_FRAME offload.

If we can manage without this (strange) offload flag,
I am for dropping it.

> I don't know if should it be part of this deprecation notice, or a separate one.

Let's keep this first notice in 20.11 to show the direction.

> It is related, but logically not exactly part of this deprecation notice.

We can update or add more notices during next year.

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

Applied, thanks