[dpdk-dev,v2,5/5] net/virtio: improve offload check performance
Checks
Commit Message
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
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);
[...]
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);
> [...]
>
@@ -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) ==
@@ -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;
@@ -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;