From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f177.google.com (mail-pf0-f177.google.com [209.85.192.177]) by dpdk.org (Postfix) with ESMTP id C61E77CFB for ; Fri, 1 Sep 2017 11:19:23 +0200 (CEST) Received: by mail-pf0-f177.google.com with SMTP id r187so6492377pfr.3 for ; Fri, 01 Sep 2017 02:19:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fridaylinux-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=g8TlwE7RkaxhmbCX2xESH4RXLbsZSmY2UeODGRbVil4=; b=uxCbf7nCHoP4nMWa/m+Zbf76HjC3S5vJlbzD2v0N6tESwLCfo16eLxTkZaZtgOdFG6 5byjZ6D2p3G75WIOEs/BxREP3GoMfR0YGdslF0uLPyyektNKUH19rMkPjHEyqqMI1TfM 7+Tsf092FHkZT/Ny/a12ZMLPFzQu01z+16FM/CDATdObGkz/VO+FgCUbbLQ0/E8pt2Jg 0ql0dr3LRj7uMreGJREjBMAggK/qhaT4x19SRHCG5XN/E7FOiBAykcfo6LAmZKTmkBZ0 MYhaI+jUSv397IUyt2hue779KkUYZvCslnzUP+iXq1S6cTSYJJzI/HgzPMyq1lLgmod1 bglA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=g8TlwE7RkaxhmbCX2xESH4RXLbsZSmY2UeODGRbVil4=; b=iwjkeIKD3kAb5QiNjAtBEp5zo6zy6a8JdzDOmN14U3O/AA+6kjj9MidxQfow3bKnKp NsRQdKtQc+ivd3w/ZUwb/4KqKpKxVNSwI7ipRGRY1JwSo/5vUTqd+LtgOM9Pz59A/lf1 LnUKPrEwzxOwzMbvrv4GfgwRq8hgUtu26/b8WLq0tZzTOjfb/9KiqO0rJgIS7RNq5VBW 4r3dlONvVatFxCbx9Da4YjIy2Nmis1AlVIWOCRhrM3w+himQBLNEZlex9ArO0mF3ACnn v9WYfz7V8jCD9lv4Db3klxlLw08hClfW+DOQmLTiJm02Dv/XGl9H0TB1XhYZG+9T1Unp YBOQ== X-Gm-Message-State: AHPjjUhXYW/lqurmJjt0N/dT42wmgJBYy676f1DAgrsyfTxMDPSKpBIe V45FG67cUvyokobO X-Google-Smtp-Source: ADKCNb4ms2bE9LizW12eGLGB9XNWfIHUkT4wKLiTjVRZEdrCffYJkuL2xh/Od7c2jPZVgtHVye2tXA== X-Received: by 10.98.25.3 with SMTP id 3mr1578665pfz.55.1504257562889; Fri, 01 Sep 2017 02:19:22 -0700 (PDT) Received: from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id e4sm2857115pfc.38.2017.09.01.02.19.19 (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 01 Sep 2017 02:19:21 -0700 (PDT) Date: Fri, 1 Sep 2017 17:19:16 +0800 From: Yuanhan Liu To: Olivier Matz Cc: dev@dpdk.org, maxime.coquelin@redhat.com, stephen@networkplumber.org, stable@dpdk.org Message-ID: <20170901091916.GT9736@yliu-home> References: <20170831134015.1383-1-olivier.matz@6wind.com> <20170831134015.1383-8-olivier.matz@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170831134015.1383-8-olivier.matz@6wind.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [dpdk-dev] [PATCH 7/9] net/virtio: rationalize setting of Rx/Tx handlers 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: Fri, 01 Sep 2017 09:19:24 -0000 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. ... > @@ -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? > + > 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. > + 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