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 A80AA5B2E for ; Sat, 9 Jun 2018 10:04:46 +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-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 071639C0069; Sat, 9 Jun 2018 08:04:43 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Sat, 9 Jun 2018 09:04:29 +0100 To: Ferruh Yigit , Jerin Jacob , Wenzhuo Lu , "Jingjing Wu" , Bernard Iremonger , John McNamara , Marko Kovacevic , Neil Horman , Remy Horton , Ori Kam , Bruce Richardson , Pablo de Lara , Radu Nicolau , Akhil Goyal , Tomasz Kantecki , Harry van Haaren , Jijiang Liu , Ravi Kumar , Konstantin Ananyev , Maxime Coquelin , Tiwei Bie , Yong Wang , Amr Mokhtar , Declan Doherty , "Chas Williams" , David Hunt , "Cristian Dumitrescu" , Anatoly Burakov , Reshma Pattan , "Byron Marohn" , Thomas Monjalon CC: , Shahaf Shuler References: <20180608224141.42730-1-ferruh.yigit@intel.com> From: Andrew Rybchenko Message-ID: <553fb3e1-3ae4-d682-17cc-f7b894b0c285@solarflare.com> Date: Sat, 9 Jun 2018 11:04:24 +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: <20180608224141.42730-1-ferruh.yigit@intel.com> Content-Language: en-GB X-Originating-IP: [84.52.114.114] 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-23896.003 X-TM-AS-Result: No--15.624700-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1528531486-r_bUHf2vAsxd 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] [RFC] ethdev: remove all 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: Sat, 09 Jun 2018 08:04:47 -0000 On 06/09/2018 01:41 AM, Ferruh Yigit wrote: > Cc: Shahaf Shuler > > Signed-off-by: Ferruh Yigit > --- <...> > diff --git a/app/test-eventdev/test_perf_common.c b/app/test-eventdev/test_perf_common.c > index d00f91802..9fe042ffe 100644 > --- a/app/test-eventdev/test_perf_common.c > +++ b/app/test-eventdev/test_perf_common.c > @@ -680,13 +680,7 @@ perf_ethdev_setup(struct evt_test *test, struct evt_options *opt) > .mq_mode = ETH_MQ_RX_RSS, > .max_rx_pkt_len = ETHER_MAX_LEN, > .split_hdr_size = 0, > - .header_split = 0, > - .hw_ip_checksum = 0, > - .hw_vlan_filter = 0, > - .hw_vlan_strip = 0, > - .hw_vlan_extend = 0, > .jumbo_frame = 0, > - .hw_strip_crc = 1, I have 2 questions here:  1. Why is jumbo_frame kept? There is DEV_RX_OFFLOAD_JUMBO_FRAME.  2. Why is hw_strip_crc=1 discarded? Yes, I remember plans to make it      default behaviour and introduce flag to keep CRC, but right now the      patch looks like mixture of two changes which is not good. There are more cases in the patch where hw_strip_crc=1 is simply discarded. <...> > diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c > index 1b6499f85..ee8ae5b9f 100644 > --- a/drivers/net/sfc/sfc_ethdev.c > +++ b/drivers/net/sfc/sfc_ethdev.c > @@ -1089,7 +1089,6 @@ sfc_tx_queue_info_get(struct rte_eth_dev *dev, uint16_t tx_queue_id, > > memset(qinfo, 0, sizeof(*qinfo)); > > - qinfo->conf.txq_flags = txq_info->txq->flags; > qinfo->conf.offloads = txq_info->txq->offloads; > qinfo->conf.tx_free_thresh = txq_info->txq->free_thresh; > qinfo->conf.tx_deferred_start = txq_info->deferred_start; > diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c > index cc76a5b15..58a0df583 100644 > --- a/drivers/net/sfc/sfc_rx.c > +++ b/drivers/net/sfc/sfc_rx.c > @@ -1446,7 +1446,6 @@ sfc_rx_check_mode(struct sfc_adapter *sa, struct rte_eth_rxmode *rxmode) > if (~rxmode->offloads & DEV_RX_OFFLOAD_CRC_STRIP) { > sfc_warn(sa, "FCS stripping cannot be disabled - always on"); > rxmode->offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > - rxmode->hw_strip_crc = 1; > } > > return rc; > diff --git a/drivers/net/sfc/sfc_tx.c b/drivers/net/sfc/sfc_tx.c > index 1bcc2c697..6d42a1a65 100644 > --- a/drivers/net/sfc/sfc_tx.c > +++ b/drivers/net/sfc/sfc_tx.c > @@ -171,7 +171,6 @@ sfc_tx_qinit(struct sfc_adapter *sa, unsigned int sw_index, > txq->free_thresh = > (tx_conf->tx_free_thresh) ? tx_conf->tx_free_thresh : > SFC_TX_DEFAULT_FREE_THRESH; > - txq->flags = tx_conf->txq_flags; > txq->offloads = offloads; > > rc = sfc_dma_alloc(sa, "txq", sw_index, EFX_TXQ_SIZE(txq_info->entries), > @@ -182,7 +181,6 @@ sfc_tx_qinit(struct sfc_adapter *sa, unsigned int sw_index, > memset(&info, 0, sizeof(info)); > info.max_fill_level = txq_max_fill_level; > info.free_thresh = txq->free_thresh; > - info.flags = tx_conf->txq_flags; > info.offloads = offloads; > info.txq_entries = txq_info->entries; > info.dma_desc_size_max = encp->enc_tx_dma_desc_size_max; > @@ -431,18 +429,10 @@ sfc_tx_qstart(struct sfc_adapter *sa, unsigned int sw_index) > if (rc != 0) > goto fail_ev_qstart; > > - /* > - * The absence of ETH_TXQ_FLAGS_IGNORE is associated with a legacy > - * application which expects that IPv4 checksum offload is enabled > - * all the time as there is no legacy flag to turn off the offload. > - */ > - if ((txq->offloads & DEV_TX_OFFLOAD_IPV4_CKSUM) || > - (~txq->flags & ETH_TXQ_FLAGS_IGNORE)) > + if (txq->offloads & DEV_TX_OFFLOAD_IPV4_CKSUM) > flags |= EFX_TXQ_CKSUM_IPV4; > > - if ((txq->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) || > - ((~txq->flags & ETH_TXQ_FLAGS_IGNORE) && > - (offloads_supported & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM))) > + if (txq->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) > flags |= EFX_TXQ_CKSUM_INNER_IPV4; > > if ((txq->offloads & DEV_TX_OFFLOAD_TCP_CKSUM) || > @@ -453,16 +443,7 @@ sfc_tx_qstart(struct sfc_adapter *sa, unsigned int sw_index) > flags |= EFX_TXQ_CKSUM_INNER_TCPUDP; > } > > - /* > - * The absence of ETH_TXQ_FLAGS_IGNORE is associated with a legacy > - * application. In turn, the absence of ETH_TXQ_FLAGS_NOXSUMTCP is > - * associated specifically with a legacy application which expects > - * both TCP checksum offload and TSO to be enabled because the legacy > - * API does not provide a dedicated mechanism to control TSO. > - */ > - if ((txq->offloads & DEV_TX_OFFLOAD_TCP_TSO) || > - ((~txq->flags & ETH_TXQ_FLAGS_IGNORE) && > - (~txq->flags & ETH_TXQ_FLAGS_NOXSUMTCP))) > + if (txq->offloads & DEV_TX_OFFLOAD_TCP_TSO) > flags |= EFX_TXQ_FATSOV2; > > rc = efx_tx_qcreate(sa->nic, sw_index, 0, &txq->mem, net/sfc changes looks good. Plus 'struct sfc_txq -> flags' (drivers/net/sfc/sfc_tx.h) and 'struct sfc_dp_tx_qcreate_info -> flags' (drivers/net/sfc/sfc_dp_tx.h) should be removed since there are not used now. If finally rxmode.jumbo_frame is removed, it should removed from net/sfc as well (but compiler will help to find it in any case). After applying the patch: $ git grep ETH_TXQ_FLAGS drivers/net/fm10k/fm10k.h:#define FM10K_SIMPLE_TX_FLAG ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \ drivers/net/fm10k/fm10k.h: ETH_TXQ_FLAGS_NOOFFLOADS) In general I think that we should do it ASAP. Also it will guarantee that new PMDs do not use corresponding structure members etc.