From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id 5165B1B594 for ; Mon, 2 Jul 2018 14:41:12 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 941D2F68A7; Mon, 2 Jul 2018 12:41:11 +0000 (UTC) Received: from [10.36.112.41] (ovpn-112-41.ams2.redhat.com [10.36.112.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5A2E41C55C; Mon, 2 Jul 2018 12:41:10 +0000 (UTC) From: Maxime Coquelin To: Marvin Liu , tiwei.bie@intel.com, Ferruh Yigit Cc: zhihong.wang@intel.com, dev@dpdk.org References: <20180702135642.52577-1-yong.liu@intel.com> <20180702135642.52577-9-yong.liu@intel.com> <8bc6e2e3-055d-2cb4-320d-ba14184c7522@redhat.com> Message-ID: Date: Mon, 2 Jul 2018 14:41:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <8bc6e2e3-055d-2cb4-320d-ba14184c7522@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 02 Jul 2018 12:41:11 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 02 Jul 2018 12:41:11 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:'' Subject: Re: [dpdk-dev] [PATCH v5 8/9] net/virtio: add in-order Rx/Tx into selection X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Jul 2018 12:41:12 -0000 On 07/02/2018 01:24 PM, Maxime Coquelin wrote: > > > On 07/02/2018 03:56 PM, Marvin Liu wrote: >> After IN_ORDER Rx/Tx paths added, need to update Rx/Tx path selection >> logic. >> >> Rx path select logic: If IN_ORDER and merge-able are enabled will select >> IN_ORDER Rx path. If IN_ORDER is enabled, Rx offload and merge-able are >> disabled will select simple Rx path. Otherwise will select normal Rx >> path. >> >> Tx path select logic: If IN_ORDER is enabled will select IN_ORDER Tx >> path. Otherwise will select default Tx path. >> >> Signed-off-by: Marvin Liu >> Reviewed-by: Maxime Coquelin >> >> diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst >> index 46e292c4d..7c099fb7c 100644 >> --- a/doc/guides/nics/virtio.rst >> +++ b/doc/guides/nics/virtio.rst >> @@ -201,7 +201,7 @@ The packet transmission flow is: >>   Virtio PMD Rx/Tx Callbacks >>   -------------------------- >> -Virtio driver has 3 Rx callbacks and 2 Tx callbacks. >> +Virtio driver has 4 Rx callbacks and 3 Tx callbacks. >>   Rx callbacks: >> @@ -215,6 +215,9 @@ Rx callbacks: >>      Vector version without mergeable Rx buffer support, also fixes >> the available >>      ring indexes and uses vector instructions to optimize performance. >> +#. ``virtio_recv_mergeable_pkts_inorder``: >> +   In-order version with mergeable Rx buffer support. >> + >>   Tx callbacks: >>   #. ``virtio_xmit_pkts``: >> @@ -223,6 +226,8 @@ Tx callbacks: >>   #. ``virtio_xmit_pkts_simple``: >>      Vector version fixes the available ring indexes to optimize >> performance. >> +#. ``virtio_xmit_pkts_inorder``: >> +   In-order version. >>   By default, the non-vector callbacks are used: >> @@ -254,6 +259,12 @@ Example of using the vector version of the virtio >> poll mode driver in >>      testpmd -l 0-2 -n 4 -- -i --tx-offloads=0x0 --rxq=1 --txq=1 >> --nb-cores=1 >> +In-order callbacks only work on simulated virtio user vdev. >> + >> +*   For Rx: If mergeable Rx buffers is enabled and in-order is >> enabled then >> +    ``virtio_xmit_pkts_inorder`` is used. >> + >> +*   For Tx: If in-order is enabled then ``virtio_xmit_pkts_inorder`` >> is used. >>   Interrupt mode >>   -------------- >> diff --git a/drivers/net/virtio/virtio_ethdev.c >> b/drivers/net/virtio/virtio_ethdev.c >> index df50a571a..df7981ddb 100644 >> --- a/drivers/net/virtio/virtio_ethdev.c >> +++ b/drivers/net/virtio/virtio_ethdev.c >> @@ -1320,6 +1320,11 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev) >>           PMD_INIT_LOG(INFO, "virtio: using simple Rx path on port %u", >>               eth_dev->data->port_id); >>           eth_dev->rx_pkt_burst = virtio_recv_pkts_vec; >> +    } else if (hw->use_inorder_rx) { >> +        PMD_INIT_LOG(INFO, >> +            "virtio: using inorder mergeable buffer Rx path on port %u", >> +            eth_dev->data->port_id); >> +        eth_dev->rx_pkt_burst = &virtio_recv_mergeable_pkts_inorder; >>       } else if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { >>           PMD_INIT_LOG(INFO, >>               "virtio: using mergeable buffer Rx path on port %u", >> @@ -1335,6 +1340,10 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev) >>           PMD_INIT_LOG(INFO, "virtio: using simple Tx path on port %u", >>               eth_dev->data->port_id); >>           eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple; >> +    } else if (hw->use_inorder_tx) { >> +        PMD_INIT_LOG(INFO, "virtio: using inorder Tx path on port %u", >> +            eth_dev->data->port_id); >> +        eth_dev->tx_pkt_burst = virtio_xmit_pkts_inorder; >>       } else { >>           PMD_INIT_LOG(INFO, "virtio: using standard Tx path on port %u", >>               eth_dev->data->port_id); >> @@ -1874,20 +1883,27 @@ virtio_dev_configure(struct rte_eth_dev *dev) >>       hw->use_simple_rx = 1; >>       hw->use_simple_tx = 1; >> +    if (vtpci_with_feature(hw, VIRTIO_F_IN_ORDER)) { >> +        /* Simple Tx not compatible with in-order ring */ >> +        hw->use_inorder_tx = 1; >> +        hw->use_simple_tx = 0; >> +        if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { >> +            hw->use_inorder_rx = 1; >> +            hw->use_simple_rx = 0; >> +        } else { >> +            hw->use_inorder_rx = 0; >> +            if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM | >> +                       DEV_RX_OFFLOAD_TCP_CKSUM)) >> +                hw->use_simple_rx = 0; > It is applied, but I think this is still not good. > > Simple Rx is set to 1 by default, so if IN_ORDER isn't negotiated, > and UDP/TCP_CSUM is enabled, simple Rx keeps being selected. > > I'll fix it in my series that I'm doing on top. Actually, after discussion with Ferruh, I fixed it directly in the patch. Thanks, Maxime > Regards, > Maxime > >> +        } >> +    } >> + >>   #if defined RTE_ARCH_ARM64 || defined RTE_ARCH_ARM >>       if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON)) { >>           hw->use_simple_rx = 0; >>           hw->use_simple_tx = 0; >>       } >>   #endif >> -    if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) { >> -        hw->use_simple_rx = 0; >> -        hw->use_simple_tx = 0; >> -    } >> - >> -    if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM | >> -               DEV_RX_OFFLOAD_TCP_CKSUM)) >> -        hw->use_simple_rx = 0; >>       return 0; >>   } >>