[1/3] ethdev: support API to set max LRO packet size
Checks
Commit Message
This patch implements [1], to support API for configuration and
validation of max size for LRO aggregated packet.
API change notice [2] is removed, and release notes for 19.11
are updated accordingly.
Various PMDs using LRO offload are updated, the new data members are
initialized to ensure they don't fail validation.
[1] http://patches.dpdk.org/patch/58217/
[2] http://patches.dpdk.org/patch/57492/
Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
doc/guides/nics/features.rst | 2 ++
doc/guides/rel_notes/deprecation.rst | 4 ---
doc/guides/rel_notes/release_19_11.rst | 8 ++++++
drivers/net/bnxt/bnxt_ethdev.c | 1 +
drivers/net/hinic/hinic_pmd_ethdev.c | 1 +
drivers/net/ixgbe/ixgbe_ethdev.c | 2 ++
drivers/net/ixgbe/ixgbe_vf_representor.c | 1 +
drivers/net/mlx5/mlx5.h | 3 +++
drivers/net/mlx5/mlx5_ethdev.c | 1 +
drivers/net/mlx5/mlx5_rxq.c | 1 -
drivers/net/qede/qede_ethdev.c | 1 +
drivers/net/virtio/virtio_ethdev.c | 1 +
drivers/net/vmxnet3/vmxnet3_ethdev.c | 1 +
lib/librte_ethdev/rte_ethdev.c | 44 ++++++++++++++++++++++++++++++++
lib/librte_ethdev/rte_ethdev.h | 4 +++
15 files changed, 70 insertions(+), 5 deletions(-)
Comments
On 11/5/19 11:40 AM, Dekel Peled wrote:
> This patch implements [1], to support API for configuration and
> validation of max size for LRO aggregated packet.
> API change notice [2] is removed, and release notes for 19.11
> are updated accordingly.
>
> Various PMDs using LRO offload are updated, the new data members are
> initialized to ensure they don't fail validation.
>
> [1] http://patches.dpdk.org/patch/58217/
> [2] http://patches.dpdk.org/patch/57492/
>
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
Few comments below, otherwise
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
[snip]
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 85ab5f0..2f52090 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1156,6 +1156,26 @@ struct rte_eth_dev *
> return name;
> }
>
> +static inline int
> +rte_eth_check_lro_pkt_size(uint16_t port_id, uint32_t config_size,
> + uint32_t dev_info_size)
As I understand Thomas prefers static functions without rte_eth_ prefix.
I think it is reasonable.
> +{
> + int ret = 0;
> +
> + if (config_size > dev_info_size) {
> + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d max_lro_pkt_size %u > "
> + "max allowed value %u\n",
> + port_id, config_size, dev_info_size);
> + ret = -EINVAL;
> + } else if (config_size < RTE_ETHER_MIN_LEN) {
Shouldn't config_size == 0 fallback to maximum?
(I don't know and I simply would like to get comments on it)
> + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d max_lro_pkt_size %u < "
> + "min allowed value %u\n", port_id, config_size,
> + (unsigned int)RTE_ETHER_MIN_LEN);
> + ret = -EINVAL;
> + }
> + return ret;
> +}
> +
> int
> rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
> const struct rte_eth_conf *dev_conf)
> @@ -1286,6 +1306,18 @@ struct rte_eth_dev *
> RTE_ETHER_MAX_LEN;
> }
>
> + /*
> + * If LRO is enabled, check that the maximum aggregated packet
> + * size is supported by the configured device.
> + */
> + if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
> + ret = rte_eth_check_lro_pkt_size(
> + port_id, dev_conf->rxmode.max_lro_pkt_size,
> + dev_info.max_lro_pkt_size);
> + if (ret)
if (ret != 0)
https://doc.dpdk.org/guides/contributing/coding_style.html#function-calls
and the style dominates in rte_ethdev.c
> + goto rollback;
> + }
> +
> /* Any requested offloading must be within its device capabilities */
> if ((dev_conf->rxmode.offloads & dev_info.rx_offload_capa) !=
> dev_conf->rxmode.offloads) {
> @@ -1790,6 +1822,18 @@ struct rte_eth_dev *
> return -EINVAL;
> }
>
> + /*
> + * If LRO is enabled, check that the maximum aggregated packet
> + * size is supported by the configured device.
> + */
> + if (local_conf.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
> + int ret = rte_eth_check_lro_pkt_size(port_id,
> + dev->data->dev_conf.rxmode.max_lro_pkt_size,
> + dev_info.max_lro_pkt_size);
> + if (ret)
if (ret != 0)
https://doc.dpdk.org/guides/contributing/coding_style.html#function-calls
and the style dominates in rte_ethdev.c
> + return ret;
> + }
> +
> ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
> socket_id, &local_conf, mp);
> if (!ret) {
>
[snip]
05/11/2019 13:39, Andrew Rybchenko:
> On 11/5/19 11:40 AM, Dekel Peled wrote:
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > +static inline int
> > +rte_eth_check_lro_pkt_size(uint16_t port_id, uint32_t config_size,
> > + uint32_t dev_info_size)
>
> As I understand Thomas prefers static functions without rte_eth_ prefix.
> I think it is reasonable.
Indeed, rte_ prefix should be reserved for public API.
Thanks, PSB.
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Tuesday, November 5, 2019 2:40 PM
> To: Dekel Peled <dekelp@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; nhorman@tuxdriver.com;
> ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com;
> anatoly.burakov@intel.com; xuanziyang2@huawei.com;
> cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
> wenzhuo.lu@intel.com; konstantin.ananyev@intel.com; Matan Azrad
> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Slava
> Ovsiienko <viacheslavo@mellanox.com>; rmody@marvell.com;
> shshaikh@marvell.com; maxime.coquelin@redhat.com;
> tiwei.bie@intel.com; zhihong.wang@intel.com; yongwang@vmware.com;
> Thomas Monjalon <thomas@monjalon.net>; ferruh.yigit@intel.com;
> jingjing.wu@intel.com; bernard.iremonger@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 1/3] ethdev: support API to set max LRO packet size
>
> On 11/5/19 11:40 AM, Dekel Peled wrote:
> > This patch implements [1], to support API for configuration and
> > validation of max size for LRO aggregated packet.
> > API change notice [2] is removed, and release notes for 19.11 are
> > updated accordingly.
> >
> > Various PMDs using LRO offload are updated, the new data members are
> > initialized to ensure they don't fail validation.
> >
> > [1]
> >
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> es.dpdk.org%2Fpatch%2F58217%2F&data=02%7C01%7Cdekelp%40mell
> anox.co
> >
> m%7C751aa0cb18b94b8a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149
> 256f461
> >
> b%7C0%7C1%7C637085543948425032&sdata=C2laHnaMCQZbDUneQD0
> 2Kpi5iAcr%
> > 2FYDAS%2BMuO8IcV9s%3D&reserved=0
> > [2]
> >
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> es.dpdk.org%2Fpatch%2F57492%2F&data=02%7C01%7Cdekelp%40mell
> anox.co
> >
> m%7C751aa0cb18b94b8a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149
> 256f461
> >
> b%7C0%7C1%7C637085543948435028&sdata=XnexdrRYNmFyLqT9IL6ZKa
> CLF2JKr
> > oKPDVML7gXKceE%3D&reserved=0
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
>
> Few comments below, otherwise
>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
> [snip]
>
> > diff --git a/lib/librte_ethdev/rte_ethdev.c
> > b/lib/librte_ethdev/rte_ethdev.c index 85ab5f0..2f52090 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -1156,6 +1156,26 @@ struct rte_eth_dev *
> > return name;
> > }
> >
> > +static inline int
> > +rte_eth_check_lro_pkt_size(uint16_t port_id, uint32_t config_size,
> > + uint32_t dev_info_size)
>
> As I understand Thomas prefers static functions without rte_eth_ prefix.
> I think it is reasonable.
Will remove prefix.
>
> > +{
> > + int ret = 0;
> > +
> > + if (config_size > dev_info_size) {
> > + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d
> max_lro_pkt_size %u > "
> > + "max allowed value %u\n",
> > + port_id, config_size, dev_info_size);
> > + ret = -EINVAL;
> > + } else if (config_size < RTE_ETHER_MIN_LEN) {
>
> Shouldn't config_size == 0 fallback to maximum?
> (I don't know and I simply would like to get comments on it)
>
This check is for value smaller than minimum, not just 0.
> > + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d
> max_lro_pkt_size %u < "
> > + "min allowed value %u\n", port_id, config_size,
> > + (unsigned int)RTE_ETHER_MIN_LEN);
> > + ret = -EINVAL;
> > + }
> > + return ret;
> > +}
> > +
> > int
> > rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t
> nb_tx_q,
> > const struct rte_eth_conf *dev_conf) @@ -1286,6
> +1306,18 @@
> > struct rte_eth_dev *
> >
> RTE_ETHER_MAX_LEN;
> > }
> >
> > + /*
> > + * If LRO is enabled, check that the maximum aggregated packet
> > + * size is supported by the configured device.
> > + */
> > + if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
> > + ret = rte_eth_check_lro_pkt_size(
> > + port_id, dev_conf-
> >rxmode.max_lro_pkt_size,
> > + dev_info.max_lro_pkt_size);
> > + if (ret)
>
> if (ret != 0)
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.d
> pdk.org%2Fguides%2Fcontributing%2Fcoding_style.html%23function-
> calls&data=02%7C01%7Cdekelp%40mellanox.com%7C751aa0cb18b94b8
> a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C
> 637085543948435028&sdata=wnlhZz70T2l%2B0UOe54XqRhJG53Pc6zMqI
> %2F%2FSu%2B5qDqc%3D&reserved=0
> and the style dominates in rte_ethdev.c
>
Will change.
> > + goto rollback;
> > + }
> > +
> > /* Any requested offloading must be within its device capabilities */
> > if ((dev_conf->rxmode.offloads & dev_info.rx_offload_capa) !=
> > dev_conf->rxmode.offloads) {
> > @@ -1790,6 +1822,18 @@ struct rte_eth_dev *
> > return -EINVAL;
> > }
> >
> > + /*
> > + * If LRO is enabled, check that the maximum aggregated packet
> > + * size is supported by the configured device.
> > + */
> > + if (local_conf.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
> > + int ret = rte_eth_check_lro_pkt_size(port_id,
> > + dev->data-
> >dev_conf.rxmode.max_lro_pkt_size,
> > + dev_info.max_lro_pkt_size);
> > + if (ret)
>
> if (ret != 0)
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.d
> pdk.org%2Fguides%2Fcontributing%2Fcoding_style.html%23function-
> calls&data=02%7C01%7Cdekelp%40mellanox.com%7C751aa0cb18b94b8
> a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C
> 637085543948435028&sdata=wnlhZz70T2l%2B0UOe54XqRhJG53Pc6zMqI
> %2F%2FSu%2B5qDqc%3D&reserved=0
> and the style dominates in rte_ethdev.c
>
Will change.
> > + return ret;
> > + }
> > +
> > ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id,
> nb_rx_desc,
> > socket_id, &local_conf, mp);
> > if (!ret) {
> >
>
> [snip]
On 11/5/19 5:18 PM, Dekel Peled wrote:
> Thanks, PSB.
>
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> Sent: Tuesday, November 5, 2019 2:40 PM
>> To: Dekel Peled <dekelp@mellanox.com>; john.mcnamara@intel.com;
>> marko.kovacevic@intel.com; nhorman@tuxdriver.com;
>> ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com;
>> anatoly.burakov@intel.com; xuanziyang2@huawei.com;
>> cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
>> wenzhuo.lu@intel.com; konstantin.ananyev@intel.com; Matan Azrad
>> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Slava
>> Ovsiienko <viacheslavo@mellanox.com>; rmody@marvell.com;
>> shshaikh@marvell.com; maxime.coquelin@redhat.com;
>> tiwei.bie@intel.com; zhihong.wang@intel.com; yongwang@vmware.com;
>> Thomas Monjalon <thomas@monjalon.net>; ferruh.yigit@intel.com;
>> jingjing.wu@intel.com; bernard.iremonger@intel.com
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH 1/3] ethdev: support API to set max LRO packet size
>>
>> On 11/5/19 11:40 AM, Dekel Peled wrote:
>>> This patch implements [1], to support API for configuration and
>>> validation of max size for LRO aggregated packet.
>>> API change notice [2] is removed, and release notes for 19.11 are
>>> updated accordingly.
>>>
>>> Various PMDs using LRO offload are updated, the new data members are
>>> initialized to ensure they don't fail validation.
>>>
>>> [1]
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
>>>
>> es.dpdk.org%2Fpatch%2F58217%2F&data=02%7C01%7Cdekelp%40mell
>> anox.co
>>>
>> m%7C751aa0cb18b94b8a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149
>> 256f461
>>>
>> b%7C0%7C1%7C637085543948425032&sdata=C2laHnaMCQZbDUneQD0
>> 2Kpi5iAcr%
>>> 2FYDAS%2BMuO8IcV9s%3D&reserved=0
>>> [2]
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
>>>
>> es.dpdk.org%2Fpatch%2F57492%2F&data=02%7C01%7Cdekelp%40mell
>> anox.co
>>>
>> m%7C751aa0cb18b94b8a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149
>> 256f461
>>>
>> b%7C0%7C1%7C637085543948435028&sdata=XnexdrRYNmFyLqT9IL6ZKa
>> CLF2JKr
>>> oKPDVML7gXKceE%3D&reserved=0
>>>
>>> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
>>
>> Few comments below, otherwise
>>
>> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
[snip]
>>> diff --git a/lib/librte_ethdev/rte_ethdev.c
>>> b/lib/librte_ethdev/rte_ethdev.c index 85ab5f0..2f52090 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> @@ -1156,6 +1156,26 @@ struct rte_eth_dev *
>>> return name;
>>> }
>>>
>>> +static inline int
>>> +rte_eth_check_lro_pkt_size(uint16_t port_id, uint32_t config_size,
>>> + uint32_t dev_info_size)
>>
>> As I understand Thomas prefers static functions without rte_eth_ prefix.
>> I think it is reasonable.
>
> Will remove prefix.
>
>>
>>> +{
>>> + int ret = 0;
>>> +
>>> + if (config_size > dev_info_size) {
>>> + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d
>> max_lro_pkt_size %u > "
>>> + "max allowed value %u\n",
>>> + port_id, config_size, dev_info_size);
>>> + ret = -EINVAL;
>>> + } else if (config_size < RTE_ETHER_MIN_LEN) {
>>
>> Shouldn't config_size == 0 fallback to maximum?
>> (I don't know and I simply would like to get comments on it)
>>
>
> This check is for value smaller than minimum, not just 0.
Yes, I know, but the question still remains.
>>> + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d
>> max_lro_pkt_size %u < "
>>> + "min allowed value %u\n", port_id, config_size,
>>> + (unsigned int)RTE_ETHER_MIN_LEN);
>>> + ret = -EINVAL;
>>> + }
>>> + return ret;
>>> +}
>>> +
>>> int
>>> rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t
>> nb_tx_q,
>>> const struct rte_eth_conf *dev_conf) @@ -1286,6
[snip]
Thanks, PSB.
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Tuesday, November 5, 2019 4:27 PM
> To: Dekel Peled <dekelp@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; nhorman@tuxdriver.com;
> ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com;
> anatoly.burakov@intel.com; xuanziyang2@huawei.com;
> cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
> wenzhuo.lu@intel.com; konstantin.ananyev@intel.com; Matan Azrad
> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Slava
> Ovsiienko <viacheslavo@mellanox.com>; rmody@marvell.com;
> shshaikh@marvell.com; maxime.coquelin@redhat.com;
> tiwei.bie@intel.com; zhihong.wang@intel.com; yongwang@vmware.com;
> Thomas Monjalon <thomas@monjalon.net>; ferruh.yigit@intel.com;
> jingjing.wu@intel.com; bernard.iremonger@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 1/3] ethdev: support API to set max LRO packet size
>
> On 11/5/19 5:18 PM, Dekel Peled wrote:
> > Thanks, PSB.
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <arybchenko@solarflare.com>
> >> Sent: Tuesday, November 5, 2019 2:40 PM
> >> To: Dekel Peled <dekelp@mellanox.com>; john.mcnamara@intel.com;
> >> marko.kovacevic@intel.com; nhorman@tuxdriver.com;
> >> ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com;
> >> anatoly.burakov@intel.com; xuanziyang2@huawei.com;
> >> cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
> >> wenzhuo.lu@intel.com; konstantin.ananyev@intel.com; Matan Azrad
> >> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Slava
> >> Ovsiienko <viacheslavo@mellanox.com>; rmody@marvell.com;
> >> shshaikh@marvell.com; maxime.coquelin@redhat.com;
> >> tiwei.bie@intel.com; zhihong.wang@intel.com;
> yongwang@vmware.com;
> >> Thomas Monjalon <thomas@monjalon.net>; ferruh.yigit@intel.com;
> >> jingjing.wu@intel.com; bernard.iremonger@intel.com
> >> Cc: dev@dpdk.org
> >> Subject: Re: [PATCH 1/3] ethdev: support API to set max LRO packet
> >> size
> >>
> >> On 11/5/19 11:40 AM, Dekel Peled wrote:
> >>> This patch implements [1], to support API for configuration and
> >>> validation of max size for LRO aggregated packet.
> >>> API change notice [2] is removed, and release notes for 19.11 are
> >>> updated accordingly.
> >>>
> >>> Various PMDs using LRO offload are updated, the new data members
> are
> >>> initialized to ensure they don't fail validation.
> >>>
> >>> [1]
> >>>
> >>
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatc
> >> h
> >>>
> >>
> es.dpdk.org%2Fpatch%2F58217%2F&data=02%7C01%7Cdekelp%40mell
> >> anox.co
> >>>
> >>
> m%7C751aa0cb18b94b8a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149
> >> 256f461
> >>>
> >>
> b%7C0%7C1%7C637085543948425032&sdata=C2laHnaMCQZbDUneQD0
> >> 2Kpi5iAcr%
> >>> 2FYDAS%2BMuO8IcV9s%3D&reserved=0
> >>> [2]
> >>>
> >>
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatc
> >> h
> >>>
> >>
> es.dpdk.org%2Fpatch%2F57492%2F&data=02%7C01%7Cdekelp%40mell
> >> anox.co
> >>>
> >>
> m%7C751aa0cb18b94b8a447c08d761ed4051%7Ca652971c7d2e4d9ba6a4d149
> >> 256f461
> >>>
> >>
> b%7C0%7C1%7C637085543948435028&sdata=XnexdrRYNmFyLqT9IL6ZKa
> >> CLF2JKr
> >>> oKPDVML7gXKceE%3D&reserved=0
> >>>
> >>> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> >>
> >> Few comments below, otherwise
> >>
> >> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
> [snip]
>
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c
> >>> b/lib/librte_ethdev/rte_ethdev.c index 85ab5f0..2f52090 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.c
> >>> +++ b/lib/librte_ethdev/rte_ethdev.c
> >>> @@ -1156,6 +1156,26 @@ struct rte_eth_dev *
> >>> return name;
> >>> }
> >>>
> >>> +static inline int
> >>> +rte_eth_check_lro_pkt_size(uint16_t port_id, uint32_t config_size,
> >>> + uint32_t dev_info_size)
> >>
> >> As I understand Thomas prefers static functions without rte_eth_ prefix.
> >> I think it is reasonable.
> >
> > Will remove prefix.
> >
> >>
> >>> +{
> >>> + int ret = 0;
> >>> +
> >>> + if (config_size > dev_info_size) {
> >>> + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d
> >> max_lro_pkt_size %u > "
> >>> + "max allowed value %u\n",
> >>> + port_id, config_size, dev_info_size);
> >>> + ret = -EINVAL;
> >>> + } else if (config_size < RTE_ETHER_MIN_LEN) {
> >>
> >> Shouldn't config_size == 0 fallback to maximum?
> >> (I don't know and I simply would like to get comments on it)
> >>
> >
> > This check is for value smaller than minimum, not just 0.
>
> Yes, I know, but the question still remains.
Application may set value 0 explicitly, don't think it should be modified.
>
> >>> + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d
> >> max_lro_pkt_size %u < "
> >>> + "min allowed value %u\n", port_id, config_size,
> >>> + (unsigned int)RTE_ETHER_MIN_LEN);
> >>> + ret = -EINVAL;
> >>> + }
> >>> + return ret;
> >>> +}
> >>> +
> >>> int
> >>> rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t
> >> nb_tx_q,
> >>> const struct rte_eth_conf *dev_conf) @@ -1286,6
>
> [snip]
@@ -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_pkt_size``.
* **[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_pkt_size``.
.. _nic_features_tso:
@@ -87,10 +87,6 @@ Deprecation Notices
This scheme will allow PMDs to avoid lookup to internal ptype table on Rx and
thereby improve Rx performance if application wishes do so.
-* ethdev: New 32-bit fields may be added for maximum LRO session size, in
- struct ``rte_eth_dev_info`` for the port capability and in struct
- ``rte_eth_rxmode`` for the port configuration.
-
* 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
@@ -380,6 +380,14 @@ ABI Changes
align the Ethernet header on receive and all known encapsulations
preserve the alignment of the header.
+* ethdev: Added 32-bit fields for maximum LRO aggregated packet size, in
+ struct ``rte_eth_dev_info`` for the port capability and in struct
+ ``rte_eth_rxmode`` for the port configuration.
+ Application should use the new field in struct ``rte_eth_rxmode`` to configure
+ the requested size.
+ PMD should use the new field in struct ``rte_eth_dev_info`` to report the
+ supported port capability.
+
Shared Library Versions
-----------------------
@@ -535,6 +535,7 @@ static int bnxt_dev_info_get_op(struct rte_eth_dev *eth_dev,
/* Fast path specifics */
dev_info->min_rx_bufsize = 1;
dev_info->max_rx_pktlen = BNXT_MAX_PKT_LEN;
+ dev_info->max_lro_pkt_size = BNXT_MAX_PKT_LEN;
dev_info->rx_offload_capa = BNXT_DEV_RX_OFFLOAD_SUPPORT;
if (bp->flags & BNXT_FLAG_PTP_SUPPORTED)
@@ -727,6 +727,7 @@ static void hinic_get_speed_capa(struct rte_eth_dev *dev, uint32_t *speed_capa)
info->max_tx_queues = nic_dev->nic_cap.max_sqs;
info->min_rx_bufsize = HINIC_MIN_RX_BUF_SIZE;
info->max_rx_pktlen = HINIC_MAX_JUMBO_FRAME_SIZE;
+ info->max_lro_pkt_size = HINIC_MAX_JUMBO_FRAME_SIZE;
info->max_mac_addrs = HINIC_MAX_UC_MAC_ADDRS;
info->min_mtu = HINIC_MIN_MTU_SIZE;
info->max_mtu = HINIC_MAX_MTU_SIZE;
@@ -3804,6 +3804,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
}
dev_info->min_rx_bufsize = 1024; /* cf BSIZEPACKET in SRRCTL register */
dev_info->max_rx_pktlen = 15872; /* includes CRC, cf MAXFRS register */
+ dev_info->max_lro_pkt_size = 15872;
dev_info->max_mac_addrs = hw->mac.num_rar_entries;
dev_info->max_hash_mac_addrs = IXGBE_VMDQ_NUM_UC_MAC;
dev_info->max_vfs = pci_dev->max_vfs;
@@ -3927,6 +3928,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
dev_info->max_tx_queues = (uint16_t)hw->mac.max_tx_queues;
dev_info->min_rx_bufsize = 1024; /* cf BSIZEPACKET in SRRCTL reg */
dev_info->max_rx_pktlen = 9728; /* includes CRC, cf MAXFRS reg */
+ dev_info->max_lro_pkt_size = 9728;
dev_info->max_mtu = dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
dev_info->max_mac_addrs = hw->mac.num_rar_entries;
dev_info->max_hash_mac_addrs = IXGBE_VMDQ_NUM_UC_MAC;
@@ -48,6 +48,7 @@
dev_info->min_rx_bufsize = 1024;
/**< Minimum size of RX buffer. */
dev_info->max_rx_pktlen = 9728;
+ dev_info->max_lro_pkt_size = 9728;
/**< Maximum configurable length of RX pkt. */
dev_info->max_rx_queues = IXGBE_VF_MAX_RX_QUEUES;
/**< Maximum number of RX queues. */
@@ -203,6 +203,9 @@ struct mlx5_hca_attr {
#define MLX5_LRO_SUPPORTED(dev) \
(((struct mlx5_priv *)((dev)->data->dev_private))->config.lro.supported)
+/* Maximal size of aggregated LRO packet. */
+#define MLX5_MAX_LRO_SIZE (UINT8_MAX * 256u)
+
/* LRO configurations structure. */
struct mlx5_lro_config {
uint32_t supported:1; /* Whether LRO is supported. */
@@ -606,6 +606,7 @@ struct ethtool_link_settings {
/* FIXME: we should ask the device for these values. */
info->min_rx_bufsize = 32;
info->max_rx_pktlen = 65536;
+ info->max_lro_pkt_size = MLX5_MAX_LRO_SIZE;
/*
* Since we need one CQ per QP, the limit is the minimum number
* between the two values.
@@ -1701,7 +1701,6 @@ struct mlx5_rxq_obj *
return 0;
}
-#define MLX5_MAX_LRO_SIZE (UINT8_MAX * 256u)
#define MLX5_MAX_TCP_HDR_OFFSET ((unsigned int)(sizeof(struct rte_ether_hdr) + \
sizeof(struct rte_vlan_hdr) * 2 + \
sizeof(struct rte_ipv6_hdr)))
@@ -1277,6 +1277,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
dev_info->min_rx_bufsize = (uint32_t)QEDE_MIN_RX_BUFF_SIZE;
dev_info->max_rx_pktlen = (uint32_t)ETH_TX_MAX_NON_LSO_PKT_LEN;
+ dev_info->max_lro_pkt_size = (uint32_t)ETH_TX_MAX_NON_LSO_PKT_LEN;
dev_info->rx_desc_lim = qede_rx_desc_lim;
dev_info->tx_desc_lim = qede_tx_desc_lim;
@@ -2435,6 +2435,7 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_TX_QUEUES);
dev_info->min_rx_bufsize = VIRTIO_MIN_RX_BUFSIZE;
dev_info->max_rx_pktlen = VIRTIO_MAX_RX_PKTLEN;
+ dev_info->max_lro_pkt_size = VIRTIO_MAX_RX_PKTLEN;
dev_info->max_mac_addrs = VIRTIO_MAX_MAC_ADDRS;
host_features = VTPCI_OPS(hw)->get_features(hw);
@@ -1161,6 +1161,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev)
dev_info->max_tx_queues = VMXNET3_MAX_TX_QUEUES;
dev_info->min_rx_bufsize = 1518 + RTE_PKTMBUF_HEADROOM;
dev_info->max_rx_pktlen = 16384; /* includes CRC, cf MAXFRS register */
+ dev_info->max_lro_pkt_size = 16384;
dev_info->speed_capa = ETH_LINK_SPEED_10G;
dev_info->max_mac_addrs = VMXNET3_MAX_MAC_ADDRS;
@@ -1156,6 +1156,26 @@ struct rte_eth_dev *
return name;
}
+static inline int
+rte_eth_check_lro_pkt_size(uint16_t port_id, uint32_t config_size,
+ uint32_t dev_info_size)
+{
+ int ret = 0;
+
+ if (config_size > dev_info_size) {
+ RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d max_lro_pkt_size %u > "
+ "max allowed value %u\n",
+ port_id, config_size, dev_info_size);
+ ret = -EINVAL;
+ } else if (config_size < RTE_ETHER_MIN_LEN) {
+ RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%d max_lro_pkt_size %u < "
+ "min allowed value %u\n", port_id, config_size,
+ (unsigned int)RTE_ETHER_MIN_LEN);
+ ret = -EINVAL;
+ }
+ return ret;
+}
+
int
rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
const struct rte_eth_conf *dev_conf)
@@ -1286,6 +1306,18 @@ struct rte_eth_dev *
RTE_ETHER_MAX_LEN;
}
+ /*
+ * If LRO is enabled, check that the maximum aggregated packet
+ * size is supported by the configured device.
+ */
+ if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
+ ret = rte_eth_check_lro_pkt_size(
+ port_id, dev_conf->rxmode.max_lro_pkt_size,
+ dev_info.max_lro_pkt_size);
+ if (ret)
+ goto rollback;
+ }
+
/* Any requested offloading must be within its device capabilities */
if ((dev_conf->rxmode.offloads & dev_info.rx_offload_capa) !=
dev_conf->rxmode.offloads) {
@@ -1790,6 +1822,18 @@ struct rte_eth_dev *
return -EINVAL;
}
+ /*
+ * If LRO is enabled, check that the maximum aggregated packet
+ * size is supported by the configured device.
+ */
+ if (local_conf.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
+ int ret = rte_eth_check_lro_pkt_size(port_id,
+ dev->data->dev_conf.rxmode.max_lro_pkt_size,
+ dev_info.max_lro_pkt_size);
+ if (ret)
+ return ret;
+ }
+
ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
socket_id, &local_conf, mp);
if (!ret) {
@@ -395,6 +395,8 @@ 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. */
+ /** Maximal allowed size of LRO aggregated packet. */
+ uint32_t max_lro_pkt_size;
uint16_t split_hdr_size; /**< hdr buf size (header_split enabled).*/
/**
* Per-port Rx offloads to be set using DEV_RX_OFFLOAD_* flags.
@@ -1223,6 +1225,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. */
+ /** Maximum configurable size of LRO aggregated packet. */
+ uint32_t max_lro_pkt_size;
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. */