From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Tiwei Bie <tiwei.bie@intel.com>
Cc: zhihong.wang@intel.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 4/4] net/virtio: improve offload check performance
Date: Tue, 5 Jun 2018 13:58:00 +0200 [thread overview]
Message-ID: <b8e91f1c-1d9c-f07c-6aa6-4944b88ea93f@redhat.com> (raw)
In-Reply-To: <20180605112029.GA27423@debian>
On 06/05/2018 01:20 PM, Tiwei Bie wrote:
> On Tue, Jun 05, 2018 at 11:43:11AM +0200, Maxime Coquelin wrote:
>> On 06/05/2018 05:10 AM, Tiwei Bie wrote:
>>> On Mon, Jun 04, 2018 at 04:29:56PM +0200, Maxime Coquelin wrote:
>>>> On 06/04/2018 01:55 PM, Tiwei Bie wrote:
>>>>> On Fri, Jun 01, 2018 at 02:47:58PM +0200, Maxime Coquelin wrote:
>>> [...]
>>>>>> @@ -253,13 +246,15 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
>>>>>> 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];
>>>>>> dxp->cookie = (void *)cookie;
>>>>>> dxp->ndescs = needed;
>>>>>> + offload = vq->hw->has_tx_offload &&
>>>>>> + (cookie->ol_flags & PKT_TX_OFFLOAD_MASK);
>>>>>
>>>>> If features aren't negotiated, I think there is no need to
>>>>> check ol_flags and update the net header.
>>>>
>>>> Isn't what the code is doing?
>>>> has_tx_offload will be false if none of the Tx offload features have
>>>> been negotiated, so ol_flags won't be checked in that case.
>>>
>>> Hmm.. Somehow I treated 'and' as 'or'..
>>>
>>> I have another question. When 'can_push' is false
>>> and 'vq->hw->has_tx_offload' is true, there will
>>> be a chance that virtio net hdr won't be zeroed
>>> when ol_flags doesn't specify any Tx offloads.
>>
>> Right, good catch.
>> It may be better to remove this small optimization.
>> Indeed, with the series, if the application does not enable offloads,
>> then the Virtio features are re-negotiated with the offload features.
>
> Yeah. It's a good idea to disable the features when
> the corresponding Tx offloads aren't requested by
> the applications! I like it!
>
> This issue happens for the mbufs whose ol_flags
> doesn't specify Tx offloads when applications
> enable Tx offloads and can_push is false. I think
> when applications enable Tx offloads, although
> most packets to be sent will have Tx offloads
> specified in their ol_flags, it's still possible
> that some packets don't have Tx offloads specified
> in their ol_flags.
Reading again my reply, I think it wasn't clear enough,
let me rephrase it.
My proposal is to keep disabling the features if the corresponding
Tx offloads aren't negotiated by the application, but just to remove the
check on mbuf's ol_flags, so:
offload = vq->hw->has_tx_offload;
Doing that, we retrieve the old behaviour, i.e. if Virtio features are
negotiated but no ol_flags set, virtio-net header will be cleared.
Maxime
> Best regards,
> Tiwei Bie
>
>>
>> Thanks,
>> Maxime
>>
>>> Best regards,
>>> Tiwei Bie
>>>
next prev parent reply other threads:[~2018-06-05 11:58 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-01 12:47 [dpdk-dev] [PATCH 0/4] net/virtio: Tx path selection and offload improvements Maxime Coquelin
2018-06-01 12:47 ` [dpdk-dev] [PATCH 1/4] net/virtio: use simple path for Tx even if Rx mergeable Maxime Coquelin
2018-06-01 12:47 ` [dpdk-dev] [PATCH 2/4] net/vhost: improve Tx path selection Maxime Coquelin
2018-06-04 12:25 ` Tiwei Bie
2018-06-01 12:47 ` [dpdk-dev] [PATCH 3/4] net/virtio: don't use simple Rx path if TCP LRO requested Maxime Coquelin
2018-06-04 11:59 ` Tiwei Bie
2018-06-04 14:32 ` Maxime Coquelin
2018-06-05 13:39 ` Maxime Coquelin
2018-06-01 12:47 ` [dpdk-dev] [PATCH 4/4] net/virtio: improve offload check performance Maxime Coquelin
2018-06-04 11:55 ` Tiwei Bie
2018-06-04 14:29 ` Maxime Coquelin
2018-06-05 3:10 ` Tiwei Bie
2018-06-05 9:43 ` Maxime Coquelin
2018-06-05 11:20 ` Tiwei Bie
2018-06-05 11:58 ` Maxime Coquelin [this message]
2018-06-05 12:21 ` Tiwei Bie
2018-06-04 7:42 ` [dpdk-dev] [PATCH 0/4] net/virtio: Tx path selection and offload improvements Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b8e91f1c-1d9c-f07c-6aa6-4944b88ea93f@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=dev@dpdk.org \
--cc=tiwei.bie@intel.com \
--cc=zhihong.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).