[ovs-dev] [PATCH] netdev-dpdk: Fix port init when lacking Tx offloads for TSO.

Flavio Leitner fbl at sysclose.org
Thu Feb 6 11:49:47 UTC 2020


On Thu, Feb 06, 2020 at 11:48:58AM +0100, Ilya Maximets wrote:
> On 2/5/20 7:01 PM, David Marchand wrote:
> > On Wed, Feb 5, 2020 at 12:42 PM Flavio Leitner <fbl at sysclose.org> wrote:
> >>
> >> On Tue, Feb 04, 2020 at 10:28:26PM +0100, David Marchand wrote:
> >>> The check on TSO capability did not ensure ip checksum, tcp checksum and
> >>> TSO tx offloads were available which resulted in a port init failure
> >>> (example below with a ena device):
> >>>
> >>> *2020-02-04T17:42:52.976Z|00084|dpdk|ERR|Ethdev port_id=0 requested Tx
> >>> offloads 0x2a doesn't match Tx offloads capabilities 0xe in
> >>> rte_eth_dev_configure()*
> >>>
> >>> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
> >>>
> >>> Reported-by: Ravi Kerur <rkerur at gmail.com>
> >>> Signed-off-by: David Marchand <david.marchand at redhat.com>
> >>> ---
> >>
> >> Works for me.
> >> Acked-by: Flavio Leitner <fbl at sysclose.org>
> >>
> >> This needs to go to master and branch-2.13.
> > 
> > Thinking again about this, don't we have an issue in that we always
> > ask drivers for offloads even if TSO is disabled?
> > 
> > I would expect a performance impact, since dpdk pmds (eyeing ixgbe
> > driver which checks txq->offloads == 0) love to select optimised tx
> > function based on offloads configuration.
> > 
> > 
> 
> 
> 
> Yes. There will be significant performance impact. Last time I compared
> 
> different i40e tx functions there was performance difference ~20-25% in
> 
> phy-phy test.
>   It was a long time ago, however we should not change the
> default behavior anyway.
> 
> 
> Suggesting following incremental:
> 
> 
> 
> ---
> 
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index eb1a7af94..6187129c0 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1132,12 +1132,15 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>          dev->hw_ol_features &= ~NETDEV_RX_HW_SCATTER;
>      }
> 
> -    if ((info.tx_offload_capa & tx_tso_offload_capa) == tx_tso_offload_capa) {
> -        dev->hw_ol_features |= NETDEV_TX_TSO_OFFLOAD;
> -    } else {
> -        dev->hw_ol_features &= ~NETDEV_TX_TSO_OFFLOAD;
> -        VLOG_WARN("Tx TSO offload is not supported on %s port "
> -                  DPDK_PORT_ID_FMT, netdev_get_name(&dev->up), dev->port_id);
> +    dev->hw_ol_features &= ~NETDEV_TX_TSO_OFFLOAD;
> +    if (userspace_tso_enabled()) {
> +        if ((info.tx_offload_capa & tx_tso_offload_capa)
> +            == tx_tso_offload_capa) {
> +            dev->hw_ol_features |= NETDEV_TX_TSO_OFFLOAD;
> +        } else {
> +            VLOG_WARN("%s: Tx TSO offload is not supported.",
> +                      netdev_get_name(&dev->up));
> +        }
>      }
> 
>      n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
> ---
> 
> Could anyone, please, test this?

I had the same patch under testing during the night:
today: 13252499.10 +-71996.09
patch: 13732079.60 +-97931.85 pps

We should do it.

Thanks,
-- 
fbl


More information about the dev mailing list