From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id EB4E4A057C; Thu, 26 Mar 2020 07:50:24 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B9AC5378E; Thu, 26 Mar 2020 07:50:23 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 441BC2AB for ; Thu, 26 Mar 2020 07:50:22 +0100 (CET) IronPort-SDR: 8aq6+w3T7K16bb4xaLeLKcUQ4tpVFXHgELI2h73jzFdAcNAo0tYjYsLrhUwxJ2CR08BZPcE8+6 /Zj/0PUxZq9Q== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2020 23:50:21 -0700 IronPort-SDR: noHvcdpvpF3scrI68Z/TMZBYFPSdC7OZpCfRDFoPW0ETByqqBOpBGk4758SCgogJx5RPs5QrDk H2998kiiDLvQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,307,1580803200"; d="scan'208";a="326469937" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga001.jf.intel.com with ESMTP; 25 Mar 2020 23:50:20 -0700 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 25 Mar 2020 23:50:20 -0700 Received: from shsmsx603.ccr.corp.intel.com (10.109.6.143) by SHSMSX601.ccr.corp.intel.com (10.109.6.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 26 Mar 2020 14:50:18 +0800 Received: from shsmsx603.ccr.corp.intel.com ([10.109.6.143]) by SHSMSX603.ccr.corp.intel.com ([10.109.6.143]) with mapi id 15.01.1713.004; Thu, 26 Mar 2020 14:50:18 +0800 From: "Wu, Jingjing" To: "Rong, Leyi" , "Zhang, Qi Z" , "Ye, Xiaolong" CC: "dev@dpdk.org" , "Rong, Leyi" Thread-Topic: [dpdk-dev] [PATCH 04/12] net/iavf: flexible Rx descriptor support in normal path Thread-Index: AQHV+2i1L3r/jpMxsEm7aEt00HRIl6haeMLg Date: Thu, 26 Mar 2020 06:50:18 +0000 Message-ID: <486c8c3962414a8d97e08f2e9a53e54d@intel.com> References: <20200316074603.10998-1-leyi.rong@intel.com> <20200316074603.10998-5-leyi.rong@intel.com> In-Reply-To: <20200316074603.10998-5-leyi.rong@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 04/12] net/iavf: flexible Rx descriptor support in normal path 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" One general comment: Looks like there are exact same code comparing with legacy rx, such as logi= c to update tail, multi-segments loop. It will be good if all the common code can be wrapped and use for multi rec= v functions [...] +/* Get the number of used descriptors of a rx queue for flexible RXD */=20 +uint32_t iavf_dev_rxq_count_flex_rxd(struct rte_eth_dev *dev, uint16_t=20 +queue_id) { #define IAVF_RXQ_SCAN_INTERVAL 4 + volatile union iavf_rx_flex_desc *rxdp; + struct iavf_rx_queue *rxq; + uint16_t desc =3D 0; + + rxq =3D dev->data->rx_queues[queue_id]; + rxdp =3D (volatile union iavf_rx_flex_desc *)&rxq->rx_ring[rxq->rx_tail]; + while ((desc < rxq->nb_rx_desc) && + rte_le_to_cpu_16(rxdp->wb.status_error0) & + (1 << IAVF_RX_FLEX_DESC_STATUS0_DD_S)) { + /* Check the DD bit of a rx descriptor of each 4 in a group, + * to avoid checking too frequently and downgrading performance + * too much. + */ + desc +=3D IAVF_RXQ_SCAN_INTERVAL; + rxdp +=3D IAVF_RXQ_SCAN_INTERVAL; + if (rxq->rx_tail + desc >=3D rxq->nb_rx_desc) + rxdp =3D (volatile union iavf_rx_flex_desc *) + &(rxq->rx_ring[rxq->rx_tail + + desc - rxq->nb_rx_desc]); + } + + return desc; +} No much difference between iavf_dev_rxq_count. Why do we need a new one? DD= bit is located in the same place, right? Can we merge those two functions to one? =20 + /* Get the number of used descriptors of a rx queue */ uint32_t iavf_dev= _rxq_count(struct rte_eth_dev *dev, uint16_t queue_id) @@ -1795,6 +2264,10 = @@ iavf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id) =20 rxq =3D dev->data->rx_queues[queue_id]; rxdp =3D &rxq->rx_ring[rxq->rx_tail]; + + if (rxq->rxdid =3D=3D IAVF_RXDID_COMMS_OVS_1) + return iavf_dev_rxq_count_flex_rxd(dev, queue_id); + while ((desc < rxq->nb_rx_desc) && ((rte_le_to_cpu_64(rxdp->wb.qword1.status_error_len) & IAVF_RXD_QW1_STATUS_MASK) >> IAVF_RXD_QW1_STATUS_SHIFT) & @@ -1813,6 +2= 286,31 @@ iavf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id) return desc; } =20 +int +iavf_dev_rx_desc_status_flex_rxd(void *rx_queue, uint16_t offset) { + volatile union iavf_rx_flex_desc *rxdp; + struct iavf_rx_queue *rxq =3D rx_queue; + uint32_t desc; + + if (unlikely(offset >=3D rxq->nb_rx_desc)) + return -EINVAL; + + if (offset >=3D rxq->nb_rx_desc - rxq->nb_rx_hold) + return RTE_ETH_RX_DESC_UNAVAIL; + + desc =3D rxq->rx_tail + offset; + if (desc >=3D rxq->nb_rx_desc) + desc -=3D rxq->nb_rx_desc; + + rxdp =3D (volatile union iavf_rx_flex_desc *)&rxq->rx_ring[desc]; + if (rte_le_to_cpu_16(rxdp->wb.status_error0) & + (1 << IAVF_RX_FLEX_DESC_STATUS0_DD_S)) + return RTE_ETH_RX_DESC_DONE; + + return RTE_ETH_RX_DESC_AVAIL; +} + Similar comments as above. [......]=20 { @@ -569,6 +596,20 @@ iavf_configure_queues(struct iavf_adapter *adapter= ) vc_qp->rxq.ring_len =3D rxq[i]->nb_rx_desc; vc_qp->rxq.dma_ring_addr =3D rxq[i]->rx_ring_phys_addr; vc_qp->rxq.databuffer_size =3D rxq[i]->rx_buf_len; + + if (vf->vf_res->vf_cap_flags & + VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC && + vf->supported_rxdid & BIT(IAVF_RXDID_COMMS_OVS_1)) { + vc_qp->rxq.rxdid =3D IAVF_RXDID_COMMS_OVS_1; + rxq[i]->rxdid =3D IAVF_RXDID_COMMS_OVS_1; Because this function is used to construct virtchnl message for configure q= ueues. The rxdid in rxq[i] should be set before this function is called. How about= to move this line assignment to iavf_dev_rx_queue_setup? + PMD_DRV_LOG(NOTICE, "request RXDID =3D=3D %d in " + "Queue[%d]", vc_qp->rxq.rxdid, i); + } else { + vc_qp->rxq.rxdid =3D IAVF_RXDID_LEGACY_1; + rxq[i]->rxdid =3D IAVF_RXDID_LEGACY_1; Same as above. + PMD_DRV_LOG(NOTICE, "request RXDID =3D=3D %d in " + "Queue[%d]", vc_qp->rxq.rxdid, i); + } } } =20 -- 2.17.1