Message ID | 20180702135642.52577-9-yong.liu@intel.com (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Maxime Coquelin |
Headers | show |
Series | support in-order feature | expand |
Context | Check | Description |
---|---|---|
ci/checkpatch | success | coding style OK |
ci/Intel-compilation | success | Compilation OK |
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 <yong.liu@intel.com> > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > 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. 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; > } >
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 <yong.liu@intel.com> >> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> >> 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; >> } >>
On 07/02/2018 02:41 PM, Maxime Coquelin wrote: > > > 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 <yong.liu@intel.com> >>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> >>> >>> 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. Running some more tests, I noticed that it is still broken. For example the case where host does not support IN_ORDER. If mergeable buffer is negotiated, the simple path gets selected, which is wrong. I fixed that directly in the patch. Regards, Maxime > 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; >>> } >>>
> -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] > Sent: Monday, July 02, 2018 11:18 PM > To: Liu, Yong <yong.liu@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>; > Yigit, Ferruh <ferruh.yigit@intel.com> > Cc: Wang, Zhihong <zhihong.wang@intel.com>; dev@dpdk.org > Subject: Re: [PATCH v5 8/9] net/virtio: add in-order Rx/Tx into selection > > > > On 07/02/2018 02:41 PM, Maxime Coquelin wrote: > > > > > > 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 <yong.liu@intel.com> > >>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > >>> > >>> 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. > > Running some more tests, I noticed that it is still broken. > For example the case where host does not support IN_ORDER. > If mergeable buffer is negotiated, the simple path gets selected, which > is wrong. > > I fixed that directly in the patch. Thanks Maxime. Sorry, I focused on in-order part and ignored original path check. > > Regards, > Maxime > > 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; > >>> } > >>>
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; + } + } + #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; }