From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id E5FF01BDD5 for ; Wed, 4 Jul 2018 13:16:32 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us4.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 51E22140073; Wed, 4 Jul 2018 11:16:31 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Wed, 4 Jul 2018 12:16:25 +0100 To: Thomas Monjalon , , CC: , , , References: <1571938.317irMz1sZ@xps> <20180702212750.16758-1-thomas@monjalon.net> <20180702212750.16758-4-thomas@monjalon.net> From: Andrew Rybchenko Message-ID: <1d842244-297a-1e0d-c86c-989a99b6c3c8@solarflare.com> Date: Wed, 4 Jul 2018 14:16:21 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180702212750.16758-4-thomas@monjalon.net> Content-Language: en-US X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23946.003 X-TM-AS-Result: No--18.466000-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1530702992-4TGpGGHuyXV1 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2 3/5] ethdev: convert remaining apps to new offload API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Jul 2018 11:16:33 -0000 On 07/03/2018 12:27 AM, Thomas Monjalon wrote: > Some test applications and examples were not converted > to the new offload API introduced in 17.11. > > For reference, see "Hardware Offload" in > doc/guides/prog_guide/poll_mode_drv.rst > > Signed-off-by: Thomas Monjalon > --- <...> > diff --git a/doc/guides/sample_app_ug/link_status_intr.rst b/doc/guides/sample_app_ug/link_status_intr.rst > index 8c11ba4ae..c7665fe5c 100644 > --- a/doc/guides/sample_app_ug/link_status_intr.rst > +++ b/doc/guides/sample_app_ug/link_status_intr.rst > @@ -137,10 +137,7 @@ The global configuration is stored in a static structure: > static const struct rte_eth_conf port_conf = { > .rxmode = { > .split_hdr_size = 0, > - .header_split = 0, /**< Header Split disabled */ > - .hw_ip_checksum = 0, /**< IP checksum offload disabled */ > - .hw_vlan_filter = 0, /**< VLAN filtering disabled */ > - .hw_strip_crc= 0, /**< CRC stripped by hardware */ > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP, Is it intended that CRC strip was disabled before and now it is becoming enabled? > }, > .txmode = {}, > .intr_conf = { <...> > diff --git a/examples/bbdev_app/main.c b/examples/bbdev_app/main.c > index 254cc0676..045a190b9 100644 > --- a/examples/bbdev_app/main.c > +++ b/examples/bbdev_app/main.c > @@ -64,11 +64,7 @@ static const struct rte_eth_conf port_conf = { > .mq_mode = ETH_MQ_RX_NONE, > .max_rx_pkt_len = ETHER_MAX_LEN, > .split_hdr_size = 0, > - .header_split = 0, /**< Header Split disabled */ > - .hw_ip_checksum = 0, /**< IP checksum offload disabled */ > - .hw_vlan_filter = 0, /**< VLAN filtering disabled */ > - .jumbo_frame = 0, /**< Jumbo Frame Support disabled */ > - .hw_strip_crc = 0, /**< CRC stripped by hardware */ > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP, Is it intended that CRC strip was disabled before and now it is becoming enabled? > }, > .txmode = { > .mq_mode = ETH_MQ_TX_NONE, <...> > diff --git a/test/test/test_pmd_perf.c b/test/test/test_pmd_perf.c > index 54bc4f6b0..4549965fc 100644 > --- a/test/test/test_pmd_perf.c > +++ b/test/test/test_pmd_perf.c > @@ -65,14 +65,7 @@ static struct rte_eth_conf port_conf = { > .mq_mode = ETH_MQ_RX_NONE, > .max_rx_pkt_len = ETHER_MAX_LEN, > .split_hdr_size = 0, > - .header_split = 0, /**< Header Split disabled */ > - .hw_ip_checksum = 0, /**< IP checksum offload enabled */ > - .hw_vlan_filter = 0, /**< VLAN filtering disabled */ > - .hw_vlan_strip = 0, /**< VLAN strip enabled. */ > - .hw_vlan_extend = 0, /**< Extended VLAN disabled. */ > - .jumbo_frame = 0, /**< Jumbo Frame Support disabled */ > - .hw_strip_crc = 1, /**< CRC stripped by hardware */ > - .enable_scatter = 0, /**< scatter rx disabled */ > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP, > }, > .txmode = { > .mq_mode = ETH_MQ_TX_NONE, > @@ -97,11 +90,6 @@ static struct rte_eth_txconf tx_conf = { > }, > .tx_free_thresh = 32, /* Use PMD default values */ > .tx_rs_thresh = 32, /* Use PMD default values */ > - .txq_flags = (ETH_TXQ_FLAGS_NOMULTSEGS | > - ETH_TXQ_FLAGS_NOVLANOFFL | > - ETH_TXQ_FLAGS_NOXSUMSCTP | > - ETH_TXQ_FLAGS_NOXSUMUDP | > - ETH_TXQ_FLAGS_NOXSUMTCP) > }; > > enum { > @@ -808,38 +796,29 @@ test_set_rxtx_conf(cmdline_fixed_string_t mode) > > if (!strcmp(mode, "vector")) { > /* vector rx, tx */ > - tx_conf.txq_flags = 0xf01; I'd say that 100% correct equivalent would be: tx_conf.offloads &= ~(DEV_TX_OFFLOAD_VLAN_INSERT |           DEV_TX_OFFLOAD_IPV4_CKSUM | DEV_TX_OFFLOAD_UDP_CKSUM |           DEV_TX_OFFLOAD_TCP_CKSUM | DEV_TX_OFFLOAD_SCTP_CKSUM |           DEV_TX_OFFLOAD_MULTI_SEGS); I guess the function may be called few times with different mode set. If so, similar fixes should be applied below as well. > tx_conf.tx_rs_thresh = 32; > tx_conf.tx_free_thresh = 32; > - port_conf.rxmode.hw_ip_checksum = 0; > - port_conf.rxmode.enable_scatter = 0; port_conf.rxmode.offloads &= ~(DEV_RX_OFFLOAD_CHECKSUM | DEV_RX_OFFLOAD_SCATTER); > return 0; > } else if (!strcmp(mode, "scalar")) { > /* bulk alloc rx, full-featured tx */ > - tx_conf.txq_flags = 0; I think here we should enable offloads listed above to have full-featured Tx: tx_conf.offloads |=  ... > tx_conf.tx_rs_thresh = 32; > tx_conf.tx_free_thresh = 32; > - port_conf.rxmode.hw_ip_checksum = 1; > - port_conf.rxmode.enable_scatter = 0; > + port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_CHECKSUM; port_conf.rxmode.offloads &= ~DEV_RX_OFFLOAD_SCATTER; > return 0; > } else if (!strcmp(mode, "hybrid")) { > /* bulk alloc rx, vector tx > * when vec macro not define, > * using the same rx/tx as scalar > */ > - tx_conf.txq_flags = 0xf01; As in similar case above. > tx_conf.tx_rs_thresh = 32; > tx_conf.tx_free_thresh = 32; > - port_conf.rxmode.hw_ip_checksum = 1; > - port_conf.rxmode.enable_scatter = 0; > + port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_CHECKSUM; As in similar case above > return 0; > } else if (!strcmp(mode, "full")) { > /* full feature rx,tx pair */ > - tx_conf.txq_flags = 0x0; /* must condition */ As in similar case above. > tx_conf.tx_rs_thresh = 32; > tx_conf.tx_free_thresh = 32; > - port_conf.rxmode.hw_ip_checksum = 0; > - port_conf.rxmode.enable_scatter = 1; /* must condition */ > + port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; port_conf.rxmode.offloads &= ~DEV_RX_OFFLOAD_CHECKSUM; > return 0; > } > In general I think that it would be really good to avoid changes in behaviour when technical changes are done.