Re: [PATCH 7/9] net/virtio: rationalize setting of Rx/Tx handlers
From: Yuanhan Liu <hidden>
Date: 2017-09-01 09:19:23
On Thu, Aug 31, 2017 at 03:40:13PM +0200, Olivier Matz wrote:
The selection of Rx/Tx handlers is done at several places, group them in one function set_rxtx_funcs(). The update of hw->use_simple_rxtx is also rationalized: - initialized to 1 (prefer simple path) - in dev configure or rx/tx queue setup, if something prevents from using the simple path, change it to 0. - in dev start, set the handlers according to hw->use_simple_rxtx. Cc: stable@dpdk.org
This patch looks like a refactoring to me, that I don't think it's really a good idea to back port it to a stable release. ...
quoted hunk ↗ jump to hunk
@@ -1534,7 +1554,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf)); eth_dev->dev_ops = &virtio_eth_dev_ops; - eth_dev->tx_pkt_burst = &virtio_xmit_pkts; if (rte_eal_process_type() == RTE_PROC_SECONDARY) { if (!hw->virtio_user_dev) {@@ -1544,12 +1563,8 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) } virtio_set_vtpci_ops(hw); - if (hw->use_simple_rxtx) { - eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple; - eth_dev->rx_pkt_burst = virtio_recv_pkts_vec; - } else { - rx_func_get(eth_dev); - } + set_rxtx_funcs(eth_dev);
No need to invoke it here?
quoted hunk ↗ jump to hunk
+ return 0; }@@ -1726,6 +1741,18 @@ virtio_dev_configure(struct rte_eth_dev *dev) return -EBUSY; } + hw->use_simple_rxtx = 1; + +#if defined RTE_ARCH_X86 + if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE3))
DPDK now requires SSE4.2. You might want to add another patch to remove this testing.
quoted hunk ↗ jump to hunk
+ hw->use_simple_rxtx = 0; +#elif defined RTE_ARCH_ARM64 || defined CONFIG_RTE_ARCH_ARM + if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON)) + hw->use_simple_rxtx = 0; +#endif + if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) + hw->use_simple_rxtx = 0; + return 0; }@@ -1802,6 +1829,7 @@ virtio_dev_start(struct rte_eth_dev *dev) VIRTQUEUE_DUMP(txvq->vq); } + set_rxtx_funcs(dev); hw->started = 1; /* Initialize Link state */diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c index a32e3229f..c9d97b643 100644 --- a/drivers/net/virtio/virtio_rxtx.c +++ b/drivers/net/virtio/virtio_rxtx.c@@ -505,25 +505,14 @@ static void virtio_update_rxtx_handler(struct rte_eth_dev *dev, const struct rte_eth_txconf *tx_conf)
This function name doesn't quite make sense now after your refactoring.
{
- uint8_t use_simple_rxtx = 0;
struct virtio_hw *hw = dev->data->dev_private;
+ uint8_t use_simple_rxtx = hw->use_simple_rxtx;
-#if defined RTE_ARCH_X86
- if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE3))
- use_simple_rxtx = 1;
-#elif defined RTE_ARCH_ARM64 || defined CONFIG_RTE_ARCH_ARM
- if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))
- use_simple_rxtx = 1;
-#endif
- /* Use simple rx/tx func if single segment and no offloads */
- if (use_simple_rxtx &&
- (tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
- !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
- PMD_INIT_LOG(INFO, "Using simple rx/tx path");
- dev->tx_pkt_burst = virtio_xmit_pkts_simple;
- dev->rx_pkt_burst = virtio_recv_pkts_vec;
- hw->use_simple_rxtx = use_simple_rxtx;
- }
+ /* cannot use simple rxtx funcs with multisegs or offloads */
+ if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) != VIRTIO_SIMPLE_FLAGS)
+ use_simple_rxtx = 0;And that's what left in this function. How about just un-folding it? --yliu