DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 01/39] examples/l2fwd: convert to new ethdev offloads API
Date: Mon, 27 Nov 2017 07:03:54 +0000	[thread overview]
Message-ID: <VI1PR05MB3149BCFDA16806DE7D4E1421C3250@VI1PR05MB3149.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <7229daf4-fffd-764c-ec0f-b30e8be39af3@solarflare.com>

Monday, November 27, 2017 8:35 AM, Andrew Rybchenko:

Yes this is right. Not exposing the CRC offload flag means the device don’t support CRC strip toggling, however it does not explicitly say if device always strip/not.
I guess device that has such limitation should specify it on the “Limitation” section of the PMD guide.

If it is interpreted in such way it sounds like loss of functionality.
Don't think it is a good way to rely on documentation here. It should
be more reliable way. PMD still can check if offload is not enabled and
complain, but there is no way to say that it is strictly required.
As I understand similar things are covered with so-called fixed offloads
in Linux.

Can you elaborate which functionality is being lost here?
If your suggestion is for the PMD to force the CRC STRIP offload in case it is not supporting *not* to strip CRC then I am OK with that.

Yes it is.
With the new Tx offloads API the application can choose the Tx offloads it wants to use according to its needs.
For l2fwd case – it doesn’t use any of them. Any default txq flag the PMD set there is irrelevant.
What I tried to do is not to preserve the entire old behavior rather to evolve the examples/applications while keeping the same functionality (i.e. the offloads which the application use are set, the rest are not).

That's true for checksum and VLAN offloads, but false for fast-free.
As I understand l2fwd and many other examples meet fast-free
requirements and if PMD supports it, it should be used since it will
show better performance results.

I agree about the Fast free offload. However IMO such optimization can be introduced on other series which further more optimize the performance of such applications, what do you think?

--Shahaf

From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Monday, November 27, 2017 8:35 AM
To: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 01/39] examples/l2fwd: convert to new ethdev offloads API

On 11/26/2017 10:41 AM, Shahaf Shuler wrote:

>>+            .ignore_offload_bitfield = 1,

>>+            .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
>
>It is not directly related to the patch.
>May be I miss something, but it looks like there is no way to say that
>"I always strip CRC and cannot preserve it".

Yes this is right. Not exposing the CRC offload flag means the device don’t support CRC strip toggling, however it does not explicitly say if device always strip/not.
I guess device that has such limitation should specify it on the “Limitation” section of the PMD guide.

If it is interpreted in such way it sounds like loss of functionality.
Don't think it is a good way to rely on documentation here. It should
be more reliable way. PMD still can check if offload is not enabled and
complain, but there is no way to say that it is strictly required.
As I understand similar things are covered with so-called fixed offloads
in Linux.



>>+            txq_conf = dev_info.default_txconf;

>>+            txq_conf.txq_flags = ETH_TXQ_FLAGS_IGNORE;

>>+            txq_conf.offloads = port_conf.txmode.offloads;
>
>It looks like it is not 100% equivalent. As far as I can see dev_info get does
>not convert txq_flags to offloads in default_txconf and in any case txq_conf.offloads
>are overwritten here. So, if PMD provides default txq_flags, it is lost.
>If it is intentionally, it should be highlighted and explained.

Yes it is.
With the new Tx offloads API the application can choose the Tx offloads it wants to use according to its needs.
For l2fwd case – it doesn’t use any of them. Any default txq flag the PMD set there is irrelevant.
What I tried to do is not to preserve the entire old behavior rather to evolve the examples/applications while keeping the same functionality (i.e. the offloads which the application use are set, the rest are not).

That's true for checksum and VLAN offloads, but false for fast-free.
As I understand l2fwd and many other examples meet fast-free
requirements and if PMD supports it, it should be used since it will
show better performance results.


Moreover – it is a wrong approach, IMO, that the PMD set default offloads flags to the application. It has no knowledge to do so. I think this mechanism was initially created since the Tx offloads were all set by default, so it provided a mean to have good OOB configuration. Now when all offloads are set, I am not sure such API is needed anymore.
Will be happy to hear more opinion on that.

I agree that blindly using PMD default offloads is a wrong approach.

  reply	other threads:[~2017-11-27  7:03 UTC|newest]

Thread overview: 134+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-23 12:14 [dpdk-dev] [PATCH 00/39] convert examples " Shahaf Shuler
2017-11-23 12:14 ` [dpdk-dev] [PATCH 01/39] examples/l2fwd: convert " Shahaf Shuler
2017-11-24 15:00   ` Andrew Rybchenko
2017-11-26  7:41     ` Shahaf Shuler
2017-11-27  6:34       ` Andrew Rybchenko
2017-11-27  7:03         ` Shahaf Shuler [this message]
2017-11-27  7:33           ` Jerin Jacob
2017-11-27 19:41             ` Shahaf Shuler
2017-11-27  7:40           ` Andrew Rybchenko
2017-11-23 12:14 ` [dpdk-dev] [PATCH 02/39] examples/l2fwd-crypto: " Shahaf Shuler
2017-11-23 12:14 ` [dpdk-dev] [PATCH 03/39] examples/l2fwd-jobstats: " Shahaf Shuler
2017-11-23 12:14 ` [dpdk-dev] [PATCH 04/39] examples/l2fwd-keepalive: " Shahaf Shuler
2017-11-23 12:14 ` [dpdk-dev] [PATCH 05/39] examples/l3fwd: move to " Shahaf Shuler
2017-11-23 12:14 ` [dpdk-dev] [PATCH 06/39] examples/l3fwd-acl: convert to new " Shahaf Shuler
2017-12-11 14:48   ` Ananyev, Konstantin
2017-11-23 12:14 ` [dpdk-dev] [PATCH 07/39] examples/l3fwd-power: " Shahaf Shuler
2017-12-11 14:00   ` Hunt, David
2017-11-23 12:14 ` [dpdk-dev] [PATCH 08/39] examples/l3fwd-vf: " Shahaf Shuler
2017-11-23 12:14 ` [dpdk-dev] [PATCH 09/39] examples/bond: " Shahaf Shuler
2017-12-11 11:08   ` Radu Nicolau
2017-12-04 12:12 ` [dpdk-dev] [PATCH 00/39] convert examples " Shahaf Shuler
2017-12-12 12:26 ` [dpdk-dev] [PATCH v2 " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 01/39] examples/l2fwd: convert " Shahaf Shuler
2017-12-19 12:16     ` De Lara Guarch, Pablo
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 02/39] examples/l2fwd-crypto: " Shahaf Shuler
2017-12-19 12:18     ` De Lara Guarch, Pablo
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 03/39] examples/l2fwd-jobstats: " Shahaf Shuler
2017-12-19 12:33     ` De Lara Guarch, Pablo
2017-12-21 13:47       ` Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 04/39] examples/l2fwd-keepalive: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to " Shahaf Shuler
2017-12-12 17:12     ` Ananyev, Konstantin
2017-12-13  7:21       ` Shahaf Shuler
2017-12-13  7:55         ` Jerin Jacob
2017-12-13 12:10           ` Ananyev, Konstantin
2017-12-13 17:32             ` Jerin Jacob
2017-12-21 14:26               ` Shahaf Shuler
2017-12-18 16:00     ` Maciej Czekaj
2017-12-21 14:08       ` Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 06/39] examples/l3fwd-acl: convert to new " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 07/39] examples/l3fwd-power: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 08/39] examples/l3fwd-vf: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 09/39] examples/bond: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 10/39] examples/exception_path: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 11/39] examples/kni: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 12/39] examples/ip_fragmentation: convert to new " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 13/39] examples/ip_pipeline: convert to new ethdev " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 14/39] examples/ip_reassembly: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 15/39] examples/ipsec-secgw: " Shahaf Shuler
2017-12-19 12:38     ` De Lara Guarch, Pablo
2017-12-21 13:45       ` Shahaf Shuler
2018-01-08 16:27         ` De Lara Guarch, Pablo
2018-01-09  7:07           ` Akhil Goyal
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 16/39] examples/ipv4_multicast: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 17/39] examples/link_status_interrupt: convert to new " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 18/39] examples/load_balancer: convert to new ethdev " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 19/39] examples/multi_process: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 20/39] examples/netmap_compat: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 21/39] examples/performance-thread: convert to new " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 22/39] examples/qos_meter: convert to new ethdev " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 23/39] examples/qos_sched: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 24/39] examples/quota_watermark: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 25/39] examples/tep_termination: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 26/39] examples/vhost: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 27/39] examples/vmdq: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 28/39] examples/vmdq_dcb: " Shahaf Shuler
2017-12-12 12:26   ` [dpdk-dev] [PATCH v2 29/39] examples/vm_power_manager: convert to new " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 30/39] examples/distributor: convert to new ethdev " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 31/39] examples/ethtool: " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 32/39] examples/eventdev_pipeline: convert to new " Shahaf Shuler
2017-12-15 15:20     ` Van Haaren, Harry
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 33/39] examples/flow_classify: convert to new ethdev " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 34/39] examples/flow_filtering: " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 35/39] examples/packet_ordering: " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 36/39] examples/ptpclient: " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 37/39] examples/rxtx_callbacks: " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 38/39] examples/server_node_efd: " Shahaf Shuler
2017-12-12 12:35   ` [dpdk-dev] [PATCH v2 39/39] examples/skeleton: " Shahaf Shuler
2017-12-26  9:22   ` [dpdk-dev] [PATCH v3 00/39] convert examples " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 01/39] examples/l2fwd: convert " Shahaf Shuler
2018-01-10 12:12       ` De Lara Guarch, Pablo
2018-01-12 13:30       ` Ferruh Yigit
2018-01-14 10:37         ` Shahaf Shuler
2018-01-15 10:20           ` Ferruh Yigit
2018-01-15 11:02             ` Shahaf Shuler
2018-01-15 11:34               ` Ferruh Yigit
2018-01-15 11:41                 ` Shahaf Shuler
2018-01-15 11:56                   ` Ferruh Yigit
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 02/39] examples/l2fwd-crypto: " Shahaf Shuler
2018-01-10 12:12       ` De Lara Guarch, Pablo
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 03/39] examples/l2fwd-jobstats: " Shahaf Shuler
2018-01-10 12:13       ` De Lara Guarch, Pablo
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 04/39] examples/l2fwd-keepalive: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 05/39] examples/l3fwd: convert to " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 06/39] examples/l3fwd-acl: convert to new " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 07/39] examples/l3fwd-power: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 08/39] examples/l3fwd-vf: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 09/39] examples/bond: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 10/39] examples/exception_path: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 11/39] examples/kni: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 12/39] examples/ip_fragmentation: convert to new " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 13/39] examples/ip_pipeline: convert to new ethdev " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 14/39] examples/ip_reassembly: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 15/39] examples/ipsec-secgw: " Shahaf Shuler
2018-01-16 11:10       ` Nicolau, Radu
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 16/39] examples/ipv4_multicast: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 17/39] examples/link_status_interrupt: convert to new " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 18/39] examples/load_balancer: convert to new ethdev " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 19/39] examples/multi_process: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 20/39] examples/netmap_compat: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 21/39] examples/performance-thread: convert to new " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 22/39] examples/qos_meter: convert to new ethdev " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 23/39] examples/qos_sched: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 24/39] examples/quota_watermark: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 25/39] examples/tep_termination: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 26/39] examples/vhost: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 27/39] examples/vmdq: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 28/39] examples/vmdq_dcb: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 29/39] examples/vm_power_manager: convert to new " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 30/39] examples/distributor: convert to new ethdev " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 31/39] examples/ethtool: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 32/39] examples/eventdev_pipeline: convert to new " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 33/39] examples/flow_classify: convert to new ethdev " Shahaf Shuler
2018-01-03 15:09       ` Iremonger, Bernard
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 34/39] examples/flow_filtering: " Shahaf Shuler
2017-12-26 13:57       ` Ori Kam
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 35/39] examples/packet_ordering: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 36/39] examples/ptpclient: " Shahaf Shuler
2018-01-10 12:14       ` De Lara Guarch, Pablo
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 37/39] examples/rxtx_callbacks: " Shahaf Shuler
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 38/39] examples/server_node_efd: " Shahaf Shuler
2018-01-10 12:15       ` De Lara Guarch, Pablo
2017-12-26  9:23     ` [dpdk-dev] [PATCH v3 39/39] examples/skeleton: " Shahaf Shuler
2018-01-16 16:25     ` [dpdk-dev] [PATCH v3 00/39] convert examples " Ferruh Yigit

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=VI1PR05MB3149BCFDA16806DE7D4E1421C3250@VI1PR05MB3149.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    /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).