DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, Olivier Matz <olivier.matz@6wind.com>,
	Flavio Leitner <fbl@sysclose.org>,
	Ilya Maximets <i.maximets@ovn.org>,
	Chenbo Xia <chenbo.xia@intel.com>,
	"Stokes, Ian" <ian.stokes@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 4/4] vhost: fix offload flags in Rx path
Date: Fri, 30 Apr 2021 10:38:45 +0200
Message-ID: <26a61979-bc80-704c-2899-84cd1c50a391@redhat.com> (raw)
In-Reply-To: <CAJFAV8zLft93Btnt2gigXwOJ0N3kX=CuLKvSCtaX9aDWqrjAWA@mail.gmail.com>

Hi David,

On 4/29/21 10:21 PM, David Marchand wrote:
> On Thu, Apr 29, 2021 at 3:31 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>> On 4/29/21 3:30 PM, Maxime Coquelin wrote:
>>>> The vhost library current configures Tx offloading (PKT_TX_*) on any
>>>> packet received from a guest virtio device which asks for some offloading.
>>>>
>>>> This is problematic, as Tx offloading is something that the application
>>>> must ask for: the application needs to configure devices
>>>> to support every used offloads (ip, tcp checksumming, tso..), and the
>>>> various l2/l3/l4 lengths must be set following any processing that
>>>> happened in the application itself.
>>>>
>>>> On the other hand, the received packets are not marked wrt current
>>>> packet l3/l4 checksumming info.
>>>>
>>>> Copy virtio rx processing to fix those offload flags but accepting
>>>> VIRTIO_NET_HDR_GSO_ECN and VIRTIO_NET_HDR_GSO_UDP too.
>>>>
>>>> The vhost example has been updated accordingly: TSO is applied to any
>>>> packet marked LRO.
>>>>
>>>> Fixes: 859b480d5afd ("vhost: add guest offload setting")
>>
>> As I understand it, this change kind of break the ABI, but it is
>> actually fixing a misuse of the mbuf API, so I think we should
>> take this patch.
> 
> Indeed, this breaks the v21 ABI.
> 
> But the only usecase I can think of is an application using TSO /
> checksum offloads *only* for traffic coming from vhost.
> I say *only* for traffic coming from vhost, because to have this
> application do TSO / checksum offloaing for traffic coming from a
> physical port, it would comply with the mbuf API and set the PKT_TX_*
> flags.
> 
> Apart from the example/vhost, I am not sure there is such an
> application that only does v2v or v2p but _not_ p2v TSO / checksum
> offloading.
> (Note: I am unable to use this example... it seems unhappy with the
> mlx5 port I use => FPE because this driver does not support vmdq o_O)
> 
> 
> I see three options:
> - fix the vhost library and break the ABI that only works in an
> example (this current patch),
> - maintain the v21 ABI
>   * using symbol versioning, this adds no branch, recompiled
> application use the new ABI, this can't be backported to 20.11,
>   * keeping the current behavior by default, but introducing a new
> flag that an application would pass to rte_vhost_driver_register().
> This new flag triggers this current patch behavior but it would add an
> additional branch per packets bulk in vhost dequeue path. This *could*
> be backported to 20.11.

The flag option seems to be the best option, as it will not break ABI so
applications we don't know about using Vhost offloads won't be impacted
and can add support for the behaviour in a smooth way.

The hardest part with this solution is to find a proper name for that
flag...

Thanks,
Maxime


  reply	other threads:[~2021-04-30  8:39 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01  9:52 [dpdk-dev] [PATCH 0/5] Offload flags fixes David Marchand
2021-04-01  9:52 ` [dpdk-dev] [PATCH 1/5] mbuf: mark old offload flag as deprecated David Marchand
2021-04-07 20:14   ` Flavio Leitner
2021-04-08  7:23   ` Olivier Matz
2021-04-08  8:41     ` David Marchand
2021-04-01  9:52 ` [dpdk-dev] [PATCH 2/5] net/tap: do not touch Tx offload flags David Marchand
2021-04-07 20:15   ` Flavio Leitner
2021-04-08  7:41     ` Olivier Matz
2021-04-08 11:21       ` Flavio Leitner
2021-04-08 12:05         ` Olivier Matz
2021-04-08 12:58           ` Flavio Leitner
2021-04-09 13:30             ` Olivier Matz
2021-04-09 16:55               ` Flavio Leitner
2021-04-28 12:17               ` David Marchand
2021-04-08 12:16         ` Ananyev, Konstantin
2021-04-08  7:53   ` Olivier Matz
2021-04-28 12:12     ` David Marchand
2021-04-01  9:52 ` [dpdk-dev] [PATCH 3/5] net/virtio: " David Marchand
2021-04-13 14:17   ` Maxime Coquelin
2021-04-01  9:52 ` [dpdk-dev] [PATCH 4/5] net/virtio: refactor Tx offload helper David Marchand
2021-04-08 13:05   ` Flavio Leitner
2021-04-09  2:31   ` Ruifeng Wang
2021-04-01  9:52 ` [dpdk-dev] [PATCH 5/5] vhost: fix offload flags in Rx path David Marchand
2021-04-08  8:28   ` Olivier Matz
2021-04-08 18:38   ` Flavio Leitner
2021-04-13 15:27     ` Maxime Coquelin
2021-04-27 17:09       ` David Marchand
2021-04-27 17:19         ` David Marchand
2021-04-29  8:04 ` [dpdk-dev] [PATCH v2 0/4] Offload flags fixes David Marchand
2021-04-29  8:04   ` [dpdk-dev] [PATCH v2 1/4] mbuf: mark old offload flag as deprecated David Marchand
2021-04-29 12:14     ` Lance Richardson
2021-04-29 16:45     ` Ajit Khaparde
2021-04-29  8:04   ` [dpdk-dev] [PATCH v2 2/4] net/virtio: do not touch Tx offload flags David Marchand
2021-04-29 13:51     ` Flavio Leitner
2021-04-29  8:04   ` [dpdk-dev] [PATCH v2 3/4] net/virtio: refactor Tx offload helper David Marchand
2021-04-29 12:59     ` Maxime Coquelin
2021-04-29  8:04   ` [dpdk-dev] [PATCH v2 4/4] vhost: fix offload flags in Rx path David Marchand
2021-04-29 13:30     ` Maxime Coquelin
2021-04-29 13:31       ` Maxime Coquelin
2021-04-29 20:21         ` David Marchand
2021-04-30  8:38           ` Maxime Coquelin [this message]
2021-04-29 20:09       ` David Marchand
2021-04-29 18:39     ` Flavio Leitner
2021-04-29 19:18       ` David Marchand
2021-05-03 13:26 ` [dpdk-dev] [PATCH v3 0/4] Offload flags fixes David Marchand
2021-05-03 13:26   ` [dpdk-dev] [PATCH v3 1/4] mbuf: mark old offload flag as deprecated David Marchand
2021-05-03 14:02     ` Maxime Coquelin
2021-05-03 14:12     ` David Marchand
2021-05-03 13:26   ` [dpdk-dev] [PATCH v3 2/4] net/virtio: do not touch Tx offload flags David Marchand
2021-05-03 13:26   ` [dpdk-dev] [PATCH v3 3/4] net/virtio: refactor Tx offload helper David Marchand
2021-05-03 13:26   ` [dpdk-dev] [PATCH v3 4/4] vhost: fix offload flags in Rx path David Marchand
2021-05-03 15:24   ` [dpdk-dev] [PATCH v3 0/4] Offload flags fixes Maxime Coquelin
2021-05-03 16:21     ` David Marchand
2021-05-03 16:43 ` [dpdk-dev] [PATCH v4 0/3] " David Marchand
2021-05-03 16:43   ` [dpdk-dev] [PATCH v4 1/3] net/virtio: do not touch Tx offload flags David Marchand
2021-05-03 16:43   ` [dpdk-dev] [PATCH v4 2/3] net/virtio: refactor Tx offload helper David Marchand
2021-05-03 16:43   ` [dpdk-dev] [PATCH v4 3/3] vhost: fix offload flags in Rx path David Marchand
2021-05-04 11:07     ` Flavio Leitner
2021-05-08  6:24     ` Wang, Yinan
2021-05-12  3:29       ` Wang, Yinan
2021-05-12 15:20         ` David Marchand
2021-05-13  6:34           ` Wang, Yinan
2021-05-04  8:29   ` [dpdk-dev] [PATCH v4 0/3] Offload flags fixes 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=26a61979-bc80-704c-2899-84cd1c50a391@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=chenbo.xia@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=fbl@sysclose.org \
    --cc=i.maximets@ovn.org \
    --cc=ian.stokes@intel.com \
    --cc=olivier.matz@6wind.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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git