diff mbox series

[dpdk-dev,v2,5/5] net/virtio: improve offload check performance

Message ID 20180606123128.7868-6-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers show
Series net/virtio: Tx path selection and offload improvements | expand

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin June 6, 2018, 12:31 p.m. UTC
Instead of checking the multiple Virtio features bits for
every packet, let's do the check once at configure time and
store it in virtio_hw struct.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 19 +++++++++++++++++++
 drivers/net/virtio/virtio_pci.h    |  3 +++
 drivers/net/virtio/virtio_rxtx.c   | 31 +++++--------------------------
 3 files changed, 27 insertions(+), 26 deletions(-)

Comments

Tiwei Bie June 7, 2018, 4:51 a.m. UTC | #1
On Wed, Jun 06, 2018 at 02:31:28PM +0200, Maxime Coquelin wrote:
> Instead of checking the multiple Virtio features bits for
> every packet, let's do the check once at configure time and
> store it in virtio_hw struct.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
[...]
> @@ -270,8 +261,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
>  		 * which is wrong. Below subtract restores correct pkt size.
>  		 */
>  		cookie->pkt_len -= head_size;
> -		/* if offload disabled, it is not zeroed below, do it now */

I think there is no need to remove this comment.

Apart from that,

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

> -		if (offload == 0) {
> +		if (!vq->hw->has_tx_offload) {
>  			ASSIGN_UNLESS_EQUAL(hdr->csum_start, 0);
>  			ASSIGN_UNLESS_EQUAL(hdr->csum_offset, 0);
>  			ASSIGN_UNLESS_EQUAL(hdr->flags, 0);
[...]
Maxime Coquelin June 7, 2018, 7:22 a.m. UTC | #2
On 06/07/2018 06:51 AM, Tiwei Bie wrote:
> On Wed, Jun 06, 2018 at 02:31:28PM +0200, Maxime Coquelin wrote:
>> Instead of checking the multiple Virtio features bits for
>> every packet, let's do the check once at configure time and
>> store it in virtio_hw struct.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
> [...]
>> @@ -270,8 +261,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
>>   		 * which is wrong. Below subtract restores correct pkt size.
>>   		 */
>>   		cookie->pkt_len -= head_size;
>> -		/* if offload disabled, it is not zeroed below, do it now */
> 
> I think there is no need to remove this comment.

Oh right, that was not intentional.
Will add it again.

> Apart from that,
> 
> Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>


Thanks,
Maxime
>> -		if (offload == 0) {
>> +		if (!vq->hw->has_tx_offload) {
>>   			ASSIGN_UNLESS_EQUAL(hdr->csum_start, 0);
>>   			ASSIGN_UNLESS_EQUAL(hdr->csum_offset, 0);
>>   			ASSIGN_UNLESS_EQUAL(hdr->flags, 0);
> [...]
>
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 357968fdd..3d4b28be8 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1851,6 +1851,22 @@  virtio_dev_args(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static bool
+rx_offload_enabled(struct virtio_hw *hw)
+{
+	return vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO4) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO6);
+}
+
+static bool
+tx_offload_enabled(struct virtio_hw *hw)
+{
+	return vtpci_with_feature(hw, VIRTIO_NET_F_CSUM) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO4) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO6);
+}
+
 /*
  * Configure virtio device
  * It returns 0 on success.
@@ -1934,6 +1950,9 @@  virtio_dev_configure(struct rte_eth_dev *dev)
 		return -ENOTSUP;
 	}
 
+	hw->has_tx_offload = tx_offload_enabled(hw);
+	hw->has_rx_offload = rx_offload_enabled(hw);
+
 	if (dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
 		/* Enable vector (0) for Link State Intrerrupt */
 		if (VTPCI_OPS(hw)->set_config_irq(hw, 0) ==
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 7318bb318..337dc6180 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -6,6 +6,7 @@ 
 #define _VIRTIO_PCI_H_
 
 #include <stdint.h>
+#include <stdbool.h>
 
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
@@ -234,6 +235,8 @@  struct virtio_hw {
 	uint8_t	    support_simple_tx;
 	uint8_t     use_simple_rx;
 	uint8_t     use_simple_tx;
+	bool        has_tx_offload;
+	bool        has_rx_offload;
 	uint16_t    port_id;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
 	uint32_t    notify_off_multiplier;
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 92fab2174..8579a7567 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -225,13 +225,6 @@  virtio_tso_fix_cksum(struct rte_mbuf *m)
 	}
 }
 
-static inline int
-tx_offload_enabled(struct virtio_hw *hw)
-{
-	return vtpci_with_feature(hw, VIRTIO_NET_F_CSUM) ||
-		vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO4) ||
-		vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO6);
-}
 
 /* avoid write operation when necessary, to lessen cache issues */
 #define ASSIGN_UNLESS_EQUAL(var, val) do {	\
@@ -251,9 +244,7 @@  virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 	uint16_t head_idx, idx;
 	uint16_t head_size = vq->hw->vtnet_hdr_size;
 	struct virtio_net_hdr *hdr;
-	int offload;
 
-	offload = tx_offload_enabled(vq->hw);
 	head_idx = vq->vq_desc_head_idx;
 	idx = head_idx;
 	dxp = &vq->vq_descx[idx];
@@ -270,8 +261,7 @@  virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 		 * which is wrong. Below subtract restores correct pkt size.
 		 */
 		cookie->pkt_len -= head_size;
-		/* if offload disabled, it is not zeroed below, do it now */
-		if (offload == 0) {
+		if (!vq->hw->has_tx_offload) {
 			ASSIGN_UNLESS_EQUAL(hdr->csum_start, 0);
 			ASSIGN_UNLESS_EQUAL(hdr->csum_offset, 0);
 			ASSIGN_UNLESS_EQUAL(hdr->flags, 0);
@@ -309,7 +299,7 @@  virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 	}
 
 	/* Checksum Offload / TSO */
-	if (offload) {
+	if (vq->hw->has_tx_offload) {
 		if (cookie->ol_flags & PKT_TX_TCP_SEG)
 			cookie->ol_flags |= PKT_TX_TCP_CKSUM;
 
@@ -686,14 +676,6 @@  virtio_rx_offload(struct rte_mbuf *m, struct virtio_net_hdr *hdr)
 	return 0;
 }
 
-static inline int
-rx_offload_enabled(struct virtio_hw *hw)
-{
-	return vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM) ||
-		vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO4) ||
-		vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO6);
-}
-
 #define VIRTIO_MBUF_BURST_SZ 64
 #define DESC_PER_CACHELINE (RTE_CACHE_LINE_SIZE / sizeof(struct vring_desc))
 uint16_t
@@ -709,7 +691,6 @@  virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	int error;
 	uint32_t i, nb_enqueued;
 	uint32_t hdr_size;
-	int offload;
 	struct virtio_net_hdr *hdr;
 
 	nb_rx = 0;
@@ -731,7 +712,6 @@  virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 
 	nb_enqueued = 0;
 	hdr_size = hw->vtnet_hdr_size;
-	offload = rx_offload_enabled(hw);
 
 	for (i = 0; i < num ; i++) {
 		rxm = rcv_pkts[i];
@@ -760,7 +740,7 @@  virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		if (hw->vlan_strip)
 			rte_vlan_strip(rxm);
 
-		if (offload && virtio_rx_offload(rxm, hdr) < 0) {
+		if (hw->has_rx_offload && virtio_rx_offload(rxm, hdr) < 0) {
 			virtio_discard_rxbuf(vq, rxm);
 			rxvq->stats.errors++;
 			continue;
@@ -825,7 +805,6 @@  virtio_recv_mergeable_pkts(void *rx_queue,
 	uint16_t extra_idx;
 	uint32_t seg_res;
 	uint32_t hdr_size;
-	int offload;
 
 	nb_rx = 0;
 	if (unlikely(hw->started == 0))
@@ -843,7 +822,6 @@  virtio_recv_mergeable_pkts(void *rx_queue,
 	extra_idx = 0;
 	seg_res = 0;
 	hdr_size = hw->vtnet_hdr_size;
-	offload = rx_offload_enabled(hw);
 
 	while (i < nb_used) {
 		struct virtio_net_hdr_mrg_rxbuf *header;
@@ -888,7 +866,8 @@  virtio_recv_mergeable_pkts(void *rx_queue,
 		rx_pkts[nb_rx] = rxm;
 		prev = rxm;
 
-		if (offload && virtio_rx_offload(rxm, &header->hdr) < 0) {
+		if (hw->has_rx_offload &&
+				virtio_rx_offload(rxm, &header->hdr) < 0) {
 			virtio_discard_rxbuf(vq, rxm);
 			rxvq->stats.errors++;
 			continue;