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 <f...@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 <rke...@gmail.com> > >>> Signed-off-by: David Marchand <david.march...@redhat.com> > >>> --- > >> > >> Works for me. > >> Acked-by: Flavio Leitner <f...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev