From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 280915F5D for ; Mon, 4 Jun 2018 14:25:30 +0200 (CEST) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jun 2018 05:25:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,476,1520924400"; d="scan'208";a="56367641" Received: from debian.sh.intel.com (HELO debian) ([10.67.104.203]) by orsmga003.jf.intel.com with ESMTP; 04 Jun 2018 05:25:29 -0700 Date: Mon, 4 Jun 2018 20:25:42 +0800 From: Tiwei Bie To: Maxime Coquelin Cc: zhihong.wang@intel.com, dev@dpdk.org Message-ID: <20180604122542.GD21406@debian> References: <20180601124758.22652-1-maxime.coquelin@redhat.com> <20180601124758.22652-3-maxime.coquelin@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180601124758.22652-3-maxime.coquelin@redhat.com> User-Agent: Mutt/1.9.5 (2018-04-13) Subject: Re: [dpdk-dev] [PATCH 2/4] net/vhost: improve Tx path 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, 04 Jun 2018 12:25:31 -0000 On Fri, Jun 01, 2018 at 02:47:56PM +0200, Maxime Coquelin wrote: > This patch improves the Tx path selection depending on > whether the application request for offloads, and on whether > offload features have been negotiated. > > When the application doesn't request for Tx offload features, > the corresponding features bits aren't negotiated. > > When Tx offload virtio features have been negotiated, ensure > the simple Tx path isn't selected. > > Signed-off-by: Maxime Coquelin > --- > drivers/net/virtio/virtio_ethdev.c | 20 ++++++++++++++++++-- > drivers/net/virtio/virtio_ethdev.h | 3 --- > 2 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index e68e9d067..5730620ed 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -1798,8 +1798,10 @@ static int > virtio_dev_configure(struct rte_eth_dev *dev) > { > const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; > + const struct rte_eth_txmode *txmode = &dev->data->dev_conf.txmode; > struct virtio_hw *hw = dev->data->dev_private; > uint64_t rx_offloads = rxmode->offloads; > + uint64_t tx_offloads = txmode->offloads; > uint64_t req_features; > int ret; > > @@ -1821,6 +1823,15 @@ virtio_dev_configure(struct rte_eth_dev *dev) > (1ULL << VIRTIO_NET_F_GUEST_TSO4) | > (1ULL << VIRTIO_NET_F_GUEST_TSO6); > > + if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM | > + DEV_TX_OFFLOAD_UDP_CKSUM)) I think it's better to keep DEV_TX_OFFLOAD_TCP/UDP_CKSUM aligned, something like: if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM | DEV_TX_OFFLOAD_UDP_CKSUM)) > + req_features |= (1ULL << VIRTIO_NET_F_CSUM); > + > + if (tx_offloads & DEV_TX_OFFLOAD_TCP_TSO) > + req_features |= > + (1ULL << VIRTIO_NET_F_HOST_TSO4) | > + (1ULL << VIRTIO_NET_F_HOST_TSO6); > + > /* if request features changed, reinit the device */ > if (req_features != hw->req_guest_features) { > ret = virtio_init_device(dev, req_features); > @@ -1885,6 +1896,11 @@ virtio_dev_configure(struct rte_eth_dev *dev) > DEV_RX_OFFLOAD_TCP_CKSUM)) > hw->use_simple_rx = 0; > > + if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM | > + DEV_TX_OFFLOAD_UDP_CKSUM | > + DEV_TX_OFFLOAD_TCP_TSO)) Ditto. I think it's better to keep them aligned, something like: if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM | DEV_TX_OFFLOAD_UDP_CKSUM | DEV_TX_OFFLOAD_TCP_TSO)) Besides, we also need to consider not using simple Tx when DEV_TX_OFFLOAD_VLAN_INSERT is requested. Best regards, Tiwei Bie > + hw->use_simple_tx = 0; > + > return 0; > } > > @@ -2138,14 +2154,14 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > > dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS | > DEV_TX_OFFLOAD_VLAN_INSERT; > - if (hw->guest_features & (1ULL << VIRTIO_NET_F_CSUM)) { > + if (host_features & (1ULL << VIRTIO_NET_F_CSUM)) { > dev_info->tx_offload_capa |= > DEV_TX_OFFLOAD_UDP_CKSUM | > DEV_TX_OFFLOAD_TCP_CKSUM; > } > tso_mask = (1ULL << VIRTIO_NET_F_HOST_TSO4) | > (1ULL << VIRTIO_NET_F_HOST_TSO6); > - if ((hw->guest_features & tso_mask) == tso_mask) > + if ((host_features & tso_mask) == tso_mask) > dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO; > } > > diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h > index bb40064ea..b603665c7 100644 > --- a/drivers/net/virtio/virtio_ethdev.h > +++ b/drivers/net/virtio/virtio_ethdev.h > @@ -28,9 +28,6 @@ > 1u << VIRTIO_NET_F_CTRL_VQ | \ > 1u << VIRTIO_NET_F_CTRL_RX | \ > 1u << VIRTIO_NET_F_CTRL_VLAN | \ > - 1u << VIRTIO_NET_F_CSUM | \ > - 1u << VIRTIO_NET_F_HOST_TSO4 | \ > - 1u << VIRTIO_NET_F_HOST_TSO6 | \ > 1u << VIRTIO_NET_F_MRG_RXBUF | \ > 1u << VIRTIO_NET_F_MTU | \ > 1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE | \ > -- > 2.14.3 >