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
