[dpdk-dev] [PATCH] net/ena: enable multi segment in Tx offload flags

[dpdk-dev] [PATCH] net/ena: enable multi segment in Tx offload flags

[Ghalem Boudour]

The DPDK ENA driver does not provide multi-segment tx offload capability.
Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by default.

This flag in not listed in doc/guides/nics/features/default.ini, so
ena.ini does not need to be updated.

Signed-off-by: Ghalem Boudour <ghalem.boudour@6wind.com>
Reviewed-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/ena/ena_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index dfe68279fa7b..a8422ee425ce 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -2055,6 +2055,7 @@ static int ena_infos_get(struct rte_eth_dev *dev,
 			DEV_RX_OFFLOAD_TCP_CKSUM;
 
 	rx_feat |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	tx_feat |= DEV_TX_OFFLOAD_MULTI_SEGS;
 
 	/* Inform framework about available features */
 	dev_info->rx_offload_capa = rx_feat;
-- 
2.28.0

Re: [dpdk-dev] [PATCH] net/ena: enable multi segment in Tx offload flags

[Michał Krawczyk]

pon., 12 lip 2021 o 19:03 Ghalem Boudour <ghalem.boudour@6wind.com> napisał(a):
>
> The DPDK ENA driver does not provide multi-segment tx offload capability.
> Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by default.
>

Hi Ghalem,

This patch enables announcement of the DEV_TX_OFFLOAD_MULTI_SEGS
capability, but still the application may not request this offload.

As ENA PMD currently assumes all the mbufs may have multiple segments
(and we don't have fast-path for the other cases), I suggest
overwriting this flag in the ena_dev_configure(), similar to what
we're doing with the DEV_RX_OFFLOAD_RSS_HASH flag.

Best regards,
Michal

> This flag in not listed in doc/guides/nics/features/default.ini, so
> ena.ini does not need to be updated.
>
> Signed-off-by: Ghalem Boudour <ghalem.boudour@6wind.com>
> Reviewed-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index dfe68279fa7b..a8422ee425ce 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -2055,6 +2055,7 @@ static int ena_infos_get(struct rte_eth_dev *dev,
>                         DEV_RX_OFFLOAD_TCP_CKSUM;
>
>         rx_feat |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> +       tx_feat |= DEV_TX_OFFLOAD_MULTI_SEGS;
>
>         /* Inform framework about available features */
>         dev_info->rx_offload_capa = rx_feat;
> --
> 2.28.0
>

Re: [dpdk-dev] [PATCH] net/ena: enable multi segment in Tx offload flags

[Olivier Matz]

Hi Michał,

On Wed, Jul 14, 2021 at 12:02:32PM +0200, Michał Krawczyk wrote:
> pon., 12 lip 2021 o 19:03 Ghalem Boudour <ghalem.boudour@6wind.com> napisał(a):
> >
> > The DPDK ENA driver does not provide multi-segment tx offload capability.
> > Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by default.
> >
> 
> Hi Ghalem,
> 
> This patch enables announcement of the DEV_TX_OFFLOAD_MULTI_SEGS
> capability, but still the application may not request this offload.
> 
> As ENA PMD currently assumes all the mbufs may have multiple segments
> (and we don't have fast-path for the other cases), I suggest
> overwriting this flag in the ena_dev_configure(), similar to what
> we're doing with the DEV_RX_OFFLOAD_RSS_HASH flag.

To give some more context, our application currently checks if the
driver supports multi-segments by checking its capabilities, and asks
for the feature if it is advertised.

When dealing with drivers that do not advertise this capability, our app
linearizes the segmented mbufs before sending them to the driver.

I think this is the proper way to use the API: if the driver supports to
handle multisegmented mbufs, it should advertise the capability.

Regards,
Olivier


> Best regards,
> Michal
> 
> > This flag in not listed in doc/guides/nics/features/default.ini, so
> > ena.ini does not need to be updated.
> >
> > Signed-off-by: Ghalem Boudour <ghalem.boudour@6wind.com>
> > Reviewed-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >  drivers/net/ena/ena_ethdev.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> > index dfe68279fa7b..a8422ee425ce 100644
> > --- a/drivers/net/ena/ena_ethdev.c
> > +++ b/drivers/net/ena/ena_ethdev.c
> > @@ -2055,6 +2055,7 @@ static int ena_infos_get(struct rte_eth_dev *dev,
> >                         DEV_RX_OFFLOAD_TCP_CKSUM;
> >
> >         rx_feat |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> > +       tx_feat |= DEV_TX_OFFLOAD_MULTI_SEGS;
> >
> >         /* Inform framework about available features */
> >         dev_info->rx_offload_capa = rx_feat;
> > --
> > 2.28.0
> >

Re: [dpdk-dev] [PATCH] net/ena: enable multi segment in Tx offload flags

[Michał Krawczyk]

śr., 28 lip 2021 o 16:27 Olivier Matz <olivier.matz@6wind.com> napisał(a):
>
> Hi Michał,
>
> On Wed, Jul 14, 2021 at 12:02:32PM +0200, Michał Krawczyk wrote:
> > pon., 12 lip 2021 o 19:03 Ghalem Boudour <ghalem.boudour@6wind.com> napisał(a):
> > >
> > > The DPDK ENA driver does not provide multi-segment tx offload capability.
> > > Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by default.
> > >
> >
> > Hi Ghalem,
> >
> > This patch enables announcement of the DEV_TX_OFFLOAD_MULTI_SEGS
> > capability, but still the application may not request this offload.
> >
> > As ENA PMD currently assumes all the mbufs may have multiple segments
> > (and we don't have fast-path for the other cases), I suggest
> > overwriting this flag in the ena_dev_configure(), similar to what
> > we're doing with the DEV_RX_OFFLOAD_RSS_HASH flag.
>
> To give some more context, our application currently checks if the
> driver supports multi-segments by checking its capabilities, and asks
> for the feature if it is advertised.
>
> When dealing with drivers that do not advertise this capability, our app
> linearizes the segmented mbufs before sending them to the driver.
>
> I think this is the proper way to use the API: if the driver supports to
> handle multisegmented mbufs, it should advertise the capability.
>
> Regards,
> Olivier
>

Hi Olivier,

I agree we should advertise it. However, after advertising this
option, the application should ask PMD to use this feature if I
understand the offload API correctly.
I was thinking about the PMD behavior if this option won't be
requested by the application - it would be the same as for the
multisegment setup, that's why I suggested to also override this
feature at the configuration step.

With the RSS hash feature we're doing exactly the same - first we're
advertising that it's available, and then (if RSS was enabled), we're
setting this offload as enabled, even though the application didn't
request it explicitly.

Anyway, I won't be holding push of this patch as we're close to rc3.
@Ghalem, please add the fixline and Cc the dpdk@stable.org in the
commit log, so we will have this commit backported.

Thanks,
Michal

Re: [dpdk-dev] [PATCH] net/ena: enable multi segment in Tx offload flags

[Olivier Matz]

Hi Michał,

On Thu, Jul 29, 2021 at 08:40:00AM +0200, Michał Krawczyk wrote:
> śr., 28 lip 2021 o 16:27 Olivier Matz <olivier.matz@6wind.com> napisał(a):
> >
> > Hi Michał,
> >
> > On Wed, Jul 14, 2021 at 12:02:32PM +0200, Michał Krawczyk wrote:
> > > pon., 12 lip 2021 o 19:03 Ghalem Boudour <ghalem.boudour@6wind.com> napisał(a):
> > > >
> > > > The DPDK ENA driver does not provide multi-segment tx offload capability.
> > > > Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by default.
> > > >
> > >
> > > Hi Ghalem,
> > >
> > > This patch enables announcement of the DEV_TX_OFFLOAD_MULTI_SEGS
> > > capability, but still the application may not request this offload.
> > >
> > > As ENA PMD currently assumes all the mbufs may have multiple segments
> > > (and we don't have fast-path for the other cases), I suggest
> > > overwriting this flag in the ena_dev_configure(), similar to what
> > > we're doing with the DEV_RX_OFFLOAD_RSS_HASH flag.
> >
> > To give some more context, our application currently checks if the
> > driver supports multi-segments by checking its capabilities, and asks
> > for the feature if it is advertised.
> >
> > When dealing with drivers that do not advertise this capability, our app
> > linearizes the segmented mbufs before sending them to the driver.
> >
> > I think this is the proper way to use the API: if the driver supports to
> > handle multisegmented mbufs, it should advertise the capability.
> >
> > Regards,
> > Olivier
> >
> 
> Hi Olivier,
> 
> I agree we should advertise it. However, after advertising this
> option, the application should ask PMD to use this feature if I
> understand the offload API correctly.

Yes

> I was thinking about the PMD behavior if this option won't be
> requested by the application - it would be the same as for the
> multisegment setup, that's why I suggested to also override this
> feature at the configuration step.
> 
> With the RSS hash feature we're doing exactly the same - first we're
> advertising that it's available, and then (if RSS was enabled), we're
> setting this offload as enabled, even though the application didn't
> request it explicitly.

I have no strong opinion here. If there is no behavior change when the
option is set or unset by the application, I think both options are
acceptable: either return what the application asked for, or return the
option always set (as you do for hash).

> Anyway, I won't be holding push of this patch as we're close to rc3.
> @Ghalem, please add the fixline and Cc the dpdk@stable.org in the
> commit log, so we will have this commit backported.

Ghalem is currently off, I'll take care of it.

The multiseg Tx feature is there since the beginning, so the Fixes
line will be:
Fixes: 1173fca25af9 ("ena: add polling-mode driver")

I can also add the flag in ena_dev_configure(), if you feel it is more
consistent. Please let me know.

Thanks,
Olivier

Re: [dpdk-dev] [PATCH] net/ena: enable multi segment in Tx offload flags

[Michał Krawczyk]

czw., 29 lip 2021 o 09:26 Olivier Matz <olivier.matz@6wind.com> napisał(a):
>
> Hi Michał,
>
> On Thu, Jul 29, 2021 at 08:40:00AM +0200, Michał Krawczyk wrote:
> > śr., 28 lip 2021 o 16:27 Olivier Matz <olivier.matz@6wind.com> napisał(a):
> > >
> > > Hi Michał,
> > >
> > > On Wed, Jul 14, 2021 at 12:02:32PM +0200, Michał Krawczyk wrote:
> > > > pon., 12 lip 2021 o 19:03 Ghalem Boudour <ghalem.boudour@6wind.com> napisał(a):
> > > > >
> > > > > The DPDK ENA driver does not provide multi-segment tx offload capability.
> > > > > Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by default.
> > > > >
> > > >
> > > > Hi Ghalem,
> > > >
> > > > This patch enables announcement of the DEV_TX_OFFLOAD_MULTI_SEGS
> > > > capability, but still the application may not request this offload.
> > > >
> > > > As ENA PMD currently assumes all the mbufs may have multiple segments
> > > > (and we don't have fast-path for the other cases), I suggest
> > > > overwriting this flag in the ena_dev_configure(), similar to what
> > > > we're doing with the DEV_RX_OFFLOAD_RSS_HASH flag.
> > >
> > > To give some more context, our application currently checks if the
> > > driver supports multi-segments by checking its capabilities, and asks
> > > for the feature if it is advertised.
> > >
> > > When dealing with drivers that do not advertise this capability, our app
> > > linearizes the segmented mbufs before sending them to the driver.
> > >
> > > I think this is the proper way to use the API: if the driver supports to
> > > handle multisegmented mbufs, it should advertise the capability.
> > >
> > > Regards,
> > > Olivier
> > >
> >
> > Hi Olivier,
> >
> > I agree we should advertise it. However, after advertising this
> > option, the application should ask PMD to use this feature if I
> > understand the offload API correctly.
>
> Yes
>
> > I was thinking about the PMD behavior if this option won't be
> > requested by the application - it would be the same as for the
> > multisegment setup, that's why I suggested to also override this
> > feature at the configuration step.
> >
> > With the RSS hash feature we're doing exactly the same - first we're
> > advertising that it's available, and then (if RSS was enabled), we're
> > setting this offload as enabled, even though the application didn't
> > request it explicitly.
>
> I have no strong opinion here. If there is no behavior change when the
> option is set or unset by the application, I think both options are
> acceptable: either return what the application asked for, or return the
> option always set (as you do for hash).
>
> > Anyway, I won't be holding push of this patch as we're close to rc3.
> > @Ghalem, please add the fixline and Cc the dpdk@stable.org in the
> > commit log, so we will have this commit backported.
>
> Ghalem is currently off, I'll take care of it.
>
> The multiseg Tx feature is there since the beginning, so the Fixes
> line will be:
> Fixes: 1173fca25af9 ("ena: add polling-mode driver")
>

Ack

> I can also add the flag in ena_dev_configure(), if you feel it is more
> consistent. Please let me know.
>

Please add it, so it will be more consistent and verbose.

Thanks,
Michal

[dpdk-dev] [PATCH v2] net/ena: enable multi segment in Tx offload flags

[Olivier Matz]

From: Ghalem Boudour <ghalem.boudour@6wind.com>

The DPDK ENA driver does not provide multi-segment tx offload capability.
Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by
default, and always set it in dev->data->dev_conf.txmode.offload.

This flag in not listed in doc/guides/nics/features/default.ini, so
ena.ini does not need to be updated.

Fixes: 1173fca25af9 ("ena: add polling-mode driver")
Cc: stable@dpdk.org

Signed-off-by: Ghalem Boudour <ghalem.boudour@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

v2
* set DEV_TX_OFFLOAD_MULTI_SEGS in dev->data->dev_conf.txmode.offload
* add Fixes and Cc stable

 drivers/net/ena/ena_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index dfe68279fa..b59451034c 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1981,6 +1981,7 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
 
 	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
 		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	dev->data->dev_conf.txmode.offloads |= DEV_TX_OFFLOAD_MULTI_SEGS;
 
 	adapter->tx_selected_offloads = dev->data->dev_conf.txmode.offloads;
 	adapter->rx_selected_offloads = dev->data->dev_conf.rxmode.offloads;
@@ -2055,6 +2056,7 @@ static int ena_infos_get(struct rte_eth_dev *dev,
 			DEV_RX_OFFLOAD_TCP_CKSUM;
 
 	rx_feat |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	tx_feat |= DEV_TX_OFFLOAD_MULTI_SEGS;
 
 	/* Inform framework about available features */
 	dev_info->rx_offload_capa = rx_feat;
-- 
2.29.2

Re: [dpdk-dev] [PATCH v2] net/ena: enable multi segment in Tx offload flags

[Michał Krawczyk]

pt., 30 lip 2021 o 10:35 Olivier Matz <olivier.matz@6wind.com> napisał(a):
>
> From: Ghalem Boudour <ghalem.boudour@6wind.com>
>
> The DPDK ENA driver does not provide multi-segment tx offload capability.
> Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by
> default, and always set it in dev->data->dev_conf.txmode.offload.
>
> This flag in not listed in doc/guides/nics/features/default.ini, so
> ena.ini does not need to be updated.
>
> Fixes: 1173fca25af9 ("ena: add polling-mode driver")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ghalem Boudour <ghalem.boudour@6wind.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
> ---
>
> v2
> * set DEV_TX_OFFLOAD_MULTI_SEGS in dev->data->dev_conf.txmode.offload
> * add Fixes and Cc stable
>
>  drivers/net/ena/ena_ethdev.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index dfe68279fa..b59451034c 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1981,6 +1981,7 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
>
>         if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
>                 dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
> +       dev->data->dev_conf.txmode.offloads |= DEV_TX_OFFLOAD_MULTI_SEGS;
>
>         adapter->tx_selected_offloads = dev->data->dev_conf.txmode.offloads;
>         adapter->rx_selected_offloads = dev->data->dev_conf.rxmode.offloads;
> @@ -2055,6 +2056,7 @@ static int ena_infos_get(struct rte_eth_dev *dev,
>                         DEV_RX_OFFLOAD_TCP_CKSUM;
>
>         rx_feat |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> +       tx_feat |= DEV_TX_OFFLOAD_MULTI_SEGS;
>
>         /* Inform framework about available features */
>         dev_info->rx_offload_capa = rx_feat;
> --
> 2.29.2
>

Re: [dpdk-dev] [PATCH v2] net/ena: enable multi segment in Tx offload flags

[Thomas Monjalon]

30/07/2021 11:37, Michał Krawczyk:
> pt., 30 lip 2021 o 10:35 Olivier Matz <olivier.matz@6wind.com> napisał(a):
> >
> > From: Ghalem Boudour <ghalem.boudour@6wind.com>
> >
> > The DPDK ENA driver does not provide multi-segment tx offload capability.
> > Let's add DEV_TX_OFFLOAD_MULTI_SEGS to ports offload capability by
> > default, and always set it in dev->data->dev_conf.txmode.offload.
> >
> > This flag in not listed in doc/guides/nics/features/default.ini, so
> > ena.ini does not need to be updated.
> >
> > Fixes: 1173fca25af9 ("ena: add polling-mode driver")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ghalem Boudour <ghalem.boudour@6wind.com>
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Michal Krawczyk <mk@semihalf.com>

Applied, thanks.