From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Shahaf Shuler <shahafs@mellanox.com>, jingjing.wu@intel.com
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/5] app/testpmd: convert to new Ethdev offloads API
Date: Mon, 4 Dec 2017 14:31:00 -0800 [thread overview]
Message-ID: <a5919a2c-1576-55f9-f9d7-d3361bec6fdc@intel.com> (raw)
In-Reply-To: <20171123120804.143897-2-shahafs@mellanox.com>
On 11/23/2017 4:08 AM, Shahaf Shuler wrote:
> Ethdev Rx/Tx offloads API has changed since:
>
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
> commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
>
> Convert the application to use the new API.
Hi Shahaf,
As far as I can see patch does a few things:
1- Convert rxmode bitfields usage to rxmode offloads usage.
2- Apply some config options to port config and add some port config checks.
3- Adding device advertised capability checks for some offloads.
Would you mind separate 2 and 3 to their own patches, with that 1 should be
straightforward and 2 & 3 will be easy to review.
And is this update tested with PMDs both support new offload method and old
offload method?
Thanks,
ferruh
>
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
<...>
> } else if (!strcmp(res->name, "hw-vlan-extend")) {
> if (!strcmp(res->value, "on"))
> - rx_mode.hw_vlan_extend = 1;
> + rx_offloads |= DEV_RX_OFFLOAD_VLAN_EXTEND;
Not related to this patch, but since you are touching these, what is the
difference between DEV_RX_OFFLOAD_VLAN_EXTEND and DEV_RX_OFFLOAD_QINQ_STRIP ?
> @@ -3434,7 +3439,14 @@ cmd_tx_vlan_set_parsed(void *parsed_result,
> {
> struct cmd_tx_vlan_set_result *res = parsed_result;
>
> + if (!all_ports_stopped()) {
> + printf("Please stop all ports first\n");
> + return;
> + }
rte_eth_rxmode bitfields to "offloads" conversion is mostly straightforward, but
is above kind of modifications part of this conversion or are you adding missing
checks?
I would prefer making only conversion related changes in this patch, and extra
improvements in other patch.
> +
> tx_vlan_set(res->port_id, res->vlan_id);
> +
> + cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
Is this required for converting bitfield to offloads usage?
> }
>
> cmdline_parse_token_string_t cmd_tx_vlan_set_tx_vlan =
> @@ -3481,7 +3493,14 @@ cmd_tx_vlan_set_qinq_parsed(void *parsed_result,
> {
> struct cmd_tx_vlan_set_qinq_result *res = parsed_result;
>
> + if (!all_ports_stopped()) {
> + printf("Please stop all ports first\n");
> + return;
> + }
Same for all occurrence of these updates.
<...>
> @@ -3693,22 +3724,34 @@ cmd_csum_parsed(void *parsed_result,
>
> if (!strcmp(res->proto, "ip")) {
> mask = TESTPMD_TX_OFFLOAD_IP_CKSUM;
> + csum_offloads |= DEV_TX_OFFLOAD_IPV4_CKSUM;
> } else if (!strcmp(res->proto, "udp")) {
> mask = TESTPMD_TX_OFFLOAD_UDP_CKSUM;
> + csum_offloads |= DEV_TX_OFFLOAD_UDP_CKSUM;
> } else if (!strcmp(res->proto, "tcp")) {
> mask = TESTPMD_TX_OFFLOAD_TCP_CKSUM;
> + csum_offloads |= DEV_TX_OFFLOAD_TCP_CKSUM;
> } else if (!strcmp(res->proto, "sctp")) {
> mask = TESTPMD_TX_OFFLOAD_SCTP_CKSUM;
> + csum_offloads |= DEV_TX_OFFLOAD_SCTP_CKSUM;
> } else if (!strcmp(res->proto, "outer-ip")) {
> mask = TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM;
> + csum_offloads |= DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
> }
>
> - if (hw)
> + if (hw) {
> ports[res->port_id].tx_ol_flags |= mask;
> - else
> + ports[res->port_id].dev_conf.txmode.offloads |=
> + csum_offloads;
So you are updating port config as well as testpmd internal configuration, again
I guess this is not related to conversion to offloads usage.
<...>
> @@ -13017,6 +13093,13 @@ cmd_set_macsec_offload_on_parsed(
>
> switch (ret) {
> case 0:
> + rte_eth_dev_info_get(port_id, &dev_info);
> + if ((dev_info.tx_offload_capa &
> + DEV_TX_OFFLOAD_MACSEC_INSERT) == 0) {
> + printf("Warning: macsec insert enabled but not "
> + "supported by port %d\n", port_id);
> + }
This also adding another layer of check if device advertise requested
capability, this is an improvement independent from conversion, can you please
separate into its own patch?
<...>
> @@ -606,8 +616,8 @@ port_offload_cap_display(portid_t port_id)
>
> if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_VLAN_INSERT) {
> printf("VLAN insert: ");
> - if (ports[port_id].tx_ol_flags &
> - TESTPMD_TX_OFFLOAD_INSERT_VLAN)
This is removing testpmd local config check, just to double check if all places
that updates this local config covered to update device config variable?
And do we still need testpmd tx_ol_flags after these changes?
<...>
> @@ -1658,7 +1679,8 @@ rxtx_config_display(void)
> printf(" %s packet forwarding%s - CRC stripping %s - "
> "packets/burst=%d\n", cur_fwd_eng->fwd_mode_name,
> retry_enabled == 0 ? "" : " with retry",
> - rx_mode.hw_strip_crc ? "enabled" : "disabled",
> + (ports[0].dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP) ?
> + "enabled" : "disabled",
There is a global config option in testpmd, for all ports. Previous log was
print based on that config option, but now you are printing the value of first port.
I believe it is wrong to display only first port values, either log can be
updated to say testpmd default configs, or remove completely, or print for all
ports, what do you think?
<...>
> @@ -897,34 +900,31 @@ launch_args_parse(int argc, char** argv)
> }
> #endif
> if (!strcmp(lgopts[opt_idx].name, "disable-crc-strip"))
> - rx_mode.hw_strip_crc = 0;
> + rx_offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
> if (!strcmp(lgopts[opt_idx].name, "enable-lro"))
> - rx_mode.enable_lro = 1;
> - if (!strcmp(lgopts[opt_idx].name, "enable-scatter"))
> - rx_mode.enable_scatter = 1;
> + rx_offloads |= DEV_RX_OFFLOAD_TCP_LRO;
> + if (!strcmp(lgopts[opt_idx].name, "enable-scatter")) {
> + rx_offloads |= DEV_RX_OFFLOAD_SCATTER;
> + }
Can drop "{}"
<...>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index c3ab44849..e3a7c26b8 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -338,15 +338,10 @@ lcoreid_t latencystats_lcore_id = -1;
> */
> struct rte_eth_rxmode rx_mode = {
> .max_rx_pkt_len = ETHER_MAX_LEN, /**< Default maximum frame length. */
> - .split_hdr_size = 0,
> - .header_split = 0, /**< Header Split disabled. */
> - .hw_ip_checksum = 0, /**< IP checksum offload disabled. */
> - .hw_vlan_filter = 1, /**< VLAN filtering enabled. */
> - .hw_vlan_strip = 1, /**< VLAN strip enabled. */
> - .hw_vlan_extend = 0, /**< Extended VLAN disabled. */
> - .jumbo_frame = 0, /**< Jumbo Frame Support disabled. */
> - .hw_strip_crc = 1, /**< CRC stripping by hardware enabled. */
> - .hw_timestamp = 0, /**< HW timestamp enabled. */
> + .offloads = (DEV_RX_OFFLOAD_VLAN_FILTER |
> + DEV_RX_OFFLOAD_VLAN_STRIP |
> + DEV_RX_OFFLOAD_CRC_STRIP),
> + .ignore_offload_bitfield = 1, /**< Use rte_eth_rxq_conf offloads API */
Is comment correct?
Flag has two meaning I guess,
1) Ignore bitfield values for port based offload configuration.
2) For rxq, use rx_conf.offloads field.
testpmd is still using port based offload (rte_eth_rxmode) but "offloads"
variable instead of bitfields, right? queue specific ones are copy of port configs.
<...>
next prev parent reply other threads:[~2017-12-04 22:31 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-23 12:07 [dpdk-dev] [PATCH 0/5] convert testpmd to new ethdev " Shahaf Shuler
2017-11-23 12:08 ` [dpdk-dev] [PATCH 1/5] app/testpmd: convert to new Ethdev " Shahaf Shuler
2017-12-04 22:31 ` Ferruh Yigit [this message]
2017-12-05 6:39 ` Shahaf Shuler
2017-12-06 22:57 ` Ferruh Yigit
2017-12-07 7:55 ` Shahaf Shuler
2017-11-23 12:08 ` [dpdk-dev] [PATCH 2/5] app/testpmd: remove txqflags Shahaf Shuler
2017-12-04 22:31 ` Ferruh Yigit
2017-12-05 6:48 ` Shahaf Shuler
2017-12-06 23:04 ` Ferruh Yigit
2017-12-07 7:56 ` Shahaf Shuler
2017-12-12 12:45 ` Maciej Czekaj
2017-12-13 7:25 ` Shahaf Shuler
2017-11-23 12:08 ` [dpdk-dev] [PATCH 3/5] app/testpmd: add command line option for multiseg Shahaf Shuler
2017-11-23 12:08 ` [dpdk-dev] [PATCH 4/5] app/testpmd: add command line option for mbuf fast free Shahaf Shuler
2017-11-23 12:08 ` [dpdk-dev] [PATCH 5/5] app/testpmd: enforce offloads caps Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 00/10] convert testpmd to new ethdev offloads API Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 01/10] app/testpmd: fix port configuration print Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 02/10] app/testpmd: convert to new Ethdev Rx offloads API Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 03/10] app/testpmd: support check of single port stop Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 04/10] app/testpmd: convert to new Ethdev Tx offloads API Shahaf Shuler
2018-01-05 18:11 ` Maciej Czekaj
2018-01-07 15:24 ` Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 05/10] app/testpmd: fix flowgen forwarding ol flags Shahaf Shuler
2018-01-09 5:32 ` Lu, Wenzhuo
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 06/10] app/testpmd: cleanup internal Tx offloads flags field Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 07/10] app/testpmd: add command line option for Tx offloads Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 08/10] app/testpmd: remove txqflags Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 09/10] app/testpmd: enforce offloads caps Shahaf Shuler
2017-12-12 12:52 ` [dpdk-dev] [PATCH v2 10/10] app/testpmd: fix on the flight VLAN configuration Shahaf Shuler
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 00/10] convert testpmd to new ethdev offloads API Shahaf Shuler
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 01/10] app/testpmd: fix port configuration print Shahaf Shuler
2018-01-05 3:33 ` Lu, Wenzhuo
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 02/10] app/testpmd: convert to new Ethdev Rx offloads API Shahaf Shuler
2018-01-09 3:05 ` Lu, Wenzhuo
2018-01-19 19:30 ` Patil, Harish
2018-01-20 19:29 ` Shahaf Shuler
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 03/10] app/testpmd: support check of single port stop Shahaf Shuler
2018-01-09 3:20 ` Lu, Wenzhuo
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 04/10] app/testpmd: convert to new Ethdev Tx offloads API Shahaf Shuler
2018-01-09 5:27 ` Lu, Wenzhuo
2018-01-09 6:47 ` Shahaf Shuler
2018-01-09 7:13 ` Lu, Wenzhuo
2018-01-09 10:02 ` Shahaf Shuler
2018-01-09 11:37 ` Lu, Wenzhuo
2018-01-09 12:14 ` Ananyev, Konstantin
2018-01-10 0:37 ` Lu, Wenzhuo
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 05/10] app/testpmd: fix flowgen forwarding ol flags Shahaf Shuler
2018-01-09 5:35 ` Lu, Wenzhuo
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 06/10] app/testpmd: cleanup internal Tx offloads flags field Shahaf Shuler
2018-01-09 6:30 ` Lu, Wenzhuo
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 07/10] app/testpmd: add command line option for Tx offloads Shahaf Shuler
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 08/10] app/testpmd: remove txqflags Shahaf Shuler
2018-01-09 7:17 ` Lu, Wenzhuo
2018-01-09 10:07 ` Shahaf Shuler
2018-01-09 11:51 ` Lu, Wenzhuo
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 09/10] app/testpmd: enforce offloads caps Shahaf Shuler
2018-01-09 7:48 ` Lu, Wenzhuo
2017-12-26 9:44 ` [dpdk-dev] [PATCH v3 10/10] app/testpmd: fix on the flight VLAN configuration Shahaf Shuler
2018-01-09 8:05 ` Lu, Wenzhuo
2018-01-09 10:03 ` Shahaf Shuler
2018-01-09 11:44 ` Lu, Wenzhuo
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 00/11] convert testpmd to new ethdev offloads API Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 01/11] app/testpmd: fix port configuration print Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 02/11] app/testpmd: convert to new Ethdev Rx offloads API Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 03/11] app/testpmd: support check of single port stop Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 04/11] app/testpmd: convert to new Ethdev Tx offloads API Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 05/11] app/testpmd: fix flowgen forwarding ol flags Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 06/11] app/testpmd: cleanup internal Tx offloads flags field Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 07/11] app/testpmd: add command line option for Tx offloads Shahaf Shuler
2018-01-15 3:06 ` Lu, Wenzhuo
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 08/11] app/testpmd: remove txqflags Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 09/11] app/testpmd: enforce offloads caps Shahaf Shuler
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 10/11] app/testpmd: adjust on the flight VLAN configuration Shahaf Shuler
2018-01-15 3:30 ` Lu, Wenzhuo
2018-01-10 9:09 ` [dpdk-dev] [PATCH v4 11/11] app/testpmd: enable fast free Tx offload by default Shahaf Shuler
2018-01-15 3:33 ` Lu, Wenzhuo
2018-01-15 10:00 ` [dpdk-dev] [PATCH v4 00/11] convert testpmd to new ethdev offloads API Thomas Monjalon
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=a5919a2c-1576-55f9-f9d7-d3361bec6fdc@intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=jingjing.wu@intel.com \
--cc=shahafs@mellanox.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).