From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 712A12BEA for ; Fri, 7 Oct 2016 09:25:27 +0200 (CEST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6CA017EA8F; Fri, 7 Oct 2016 07:25:26 +0000 (UTC) Received: from [10.36.6.222] (vpn1-6-222.ams2.redhat.com [10.36.6.222]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u977PLeV010347 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 7 Oct 2016 03:25:22 -0400 To: Olivier Matz , dev@dpdk.org, yuanhan.liu@linux.intel.com References: <1469088510-7552-1-git-send-email-olivier.matz@6wind.com> <1475485223-30566-1-git-send-email-olivier.matz@6wind.com> <1475485223-30566-11-git-send-email-olivier.matz@6wind.com> Cc: konstantin.ananyev@intel.com, sugesh.chandran@intel.com, bruce.richardson@intel.com, jianfeng.tan@intel.com, helin.zhang@intel.com, adrien.mazarguil@6wind.com, stephen@networkplumber.org, dprovan@bivio.net, xiao.w.wang@intel.com From: Maxime Coquelin Message-ID: Date: Fri, 7 Oct 2016 09:25:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1475485223-30566-11-git-send-email-olivier.matz@6wind.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 07 Oct 2016 07:25:26 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 10/12] virtio: add Tx checksum offload support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Oct 2016 07:25:27 -0000 Hi Olivier, On 10/03/2016 11:00 AM, Olivier Matz wrote: > Signed-off-by: Olivier Matz > --- > drivers/net/virtio/virtio_ethdev.c | 7 +++++ > drivers/net/virtio/virtio_ethdev.h | 1 + > drivers/net/virtio/virtio_rxtx.c | 57 +++++++++++++++++++++++++------------- > 3 files changed, 45 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index 43cb096..55024cd 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -1578,6 +1578,13 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > dev_info->rx_offload_capa = > DEV_RX_OFFLOAD_TCP_CKSUM | > DEV_RX_OFFLOAD_UDP_CKSUM; > + dev_info->tx_offload_capa = 0; > + > + if (hw->guest_features & (1ULL << VIRTIO_NET_F_CSUM)) { > + dev_info->tx_offload_capa |= > + DEV_TX_OFFLOAD_UDP_CKSUM | > + DEV_TX_OFFLOAD_TCP_CKSUM; > + } > } > > /* > diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h > index 2fc9218..202aa2e 100644 > --- a/drivers/net/virtio/virtio_ethdev.h > +++ b/drivers/net/virtio/virtio_ethdev.h > @@ -62,6 +62,7 @@ > 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_MRG_RXBUF | \ > 1ULL << VIRTIO_F_VERSION_1) > > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c > index eda678a..4ae11e7 100644 > --- a/drivers/net/virtio/virtio_rxtx.c > +++ b/drivers/net/virtio/virtio_rxtx.c > @@ -213,13 +213,14 @@ static inline void > virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie, > uint16_t needed, int use_indirect, int can_push) > { > + struct virtio_tx_region *txr = txvq->virtio_net_hdr_mz->addr; > struct vq_desc_extra *dxp; > struct virtqueue *vq = txvq->vq; > struct vring_desc *start_dp; > uint16_t seg_num = cookie->nb_segs; > uint16_t head_idx, idx; > uint16_t head_size = vq->hw->vtnet_hdr_size; > - unsigned long offs; > + struct virtio_net_hdr *hdr; > > head_idx = vq->vq_desc_head_idx; > idx = head_idx; > @@ -230,10 +231,9 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie, > start_dp = vq->vq_ring.desc; > > if (can_push) { > - /* put on zero'd transmit header (no offloads) */ > - void *hdr = rte_pktmbuf_prepend(cookie, head_size); > - > - memset(hdr, 0, head_size); > + /* prepend cannot fail, checked by caller */ > + hdr = (struct virtio_net_hdr *) > + rte_pktmbuf_prepend(cookie, head_size); > } else if (use_indirect) { > /* setup tx ring slot to point to indirect > * descriptor list stored in reserved region. > @@ -241,14 +241,11 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie, > * the first slot in indirect ring is already preset > * to point to the header in reserved region > */ > - struct virtio_tx_region *txr = txvq->virtio_net_hdr_mz->addr; > - > - offs = idx * sizeof(struct virtio_tx_region) > - + offsetof(struct virtio_tx_region, tx_indir); > - > - start_dp[idx].addr = txvq->virtio_net_hdr_mem + offs; > + start_dp[idx].addr = txvq->virtio_net_hdr_mem + > + RTE_PTR_DIFF(&txr[idx].tx_indir, txr); > start_dp[idx].len = (seg_num + 1) * sizeof(struct vring_desc); > start_dp[idx].flags = VRING_DESC_F_INDIRECT; > + hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr; > > /* loop below will fill in rest of the indirect elements */ > start_dp = txr[idx].tx_indir; > @@ -257,15 +254,40 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie, > /* setup first tx ring slot to point to header > * stored in reserved region. > */ > - offs = idx * sizeof(struct virtio_tx_region) > - + offsetof(struct virtio_tx_region, tx_hdr); > - > - start_dp[idx].addr = txvq->virtio_net_hdr_mem + offs; > + start_dp[idx].addr = txvq->virtio_net_hdr_mem + > + RTE_PTR_DIFF(&txr[idx].tx_hdr, txr); > start_dp[idx].len = vq->hw->vtnet_hdr_size; > start_dp[idx].flags = VRING_DESC_F_NEXT; > + hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr; > + > idx = start_dp[idx].next; > } > > + /* Checksum Offload */ > + switch (cookie->ol_flags & PKT_TX_L4_MASK) { > + case PKT_TX_UDP_CKSUM: > + hdr->csum_start = cookie->l2_len + cookie->l3_len; > + hdr->csum_offset = 6; > + hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM; > + break; > + > + case PKT_TX_TCP_CKSUM: > + hdr->csum_start = cookie->l2_len + cookie->l3_len; > + hdr->csum_offset = 16; > + hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM; > + break; > + > + default: > + hdr->csum_start = 0; > + hdr->csum_offset = 0; > + hdr->flags = 0; > + break; > + } > + > + hdr->gso_type = 0; > + hdr->gso_size = 0; > + hdr->hdr_len = 0; In he case we don't use any offload, have you measured the performance regression with current code when using a dedicated descriptor for the header? I haven't tested you series, but I would think it is more than 15% for 64 bytes packets based on my trials to use a single descriptor. Indeed, without your series, when using a dedicated desc for the header, the header is not accessed in the virtio transmit path. It is zeroed at init time. Could we keep the same behaviour when offloading features aren't negotiated? Thanks, Maxime