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

David Marchand david.marchand at redhat.com
Thu Feb 6 12:28:36 UTC 2020


On Thu, Feb 6, 2020 at 12:50 PM Flavio Leitner <fbl at sysclose.org> wrote:
>
> 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

I am fine with taking my original patch as is.
Then either Ilya or you send a follow up patch.

Thanks.

-- 
David Marchand



More information about the dev mailing list