[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