From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4167C41CBC; Fri, 17 Feb 2023 09:24:21 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 23BEE40F17; Fri, 17 Feb 2023 09:24:21 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 4A05B40EE1 for ; Fri, 17 Feb 2023 09:24:20 +0100 (CET) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 6E8017D; Fri, 17 Feb 2023 11:24:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 6E8017D DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1676622259; bh=bjv8ZSorFHTAn336N5XplrGAVdfiOBXijX4AV/jQXM8=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=NQyfIxJdHdaaxRtG3s1Tl57ZUcgJeUgBrcC+bE4JlMovz0YaJCPRKmxQtz7Ud3S7N 4yFS4N2UyEszhwLJKFjTYMagNjr5JPNqgPELBNyd9S9z+ebcUQ7dsrdcwiotmqEDRW yG2l7+kPCmn0d3yYx5qNjuQu4VWMEeRei8/r5VbU= Message-ID: Date: Fri, 17 Feb 2023 11:24:19 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH v5 1/2] ethdev: introduce the Tx map API for aggregated ports Content-Language: en-US To: Ferruh Yigit , Jiawei Wang , viacheslavo@nvidia.com, orika@nvidia.com, thomas@monjalon.net, Aman Singh , Yuying Zhang Cc: dev@dpdk.org, rasland@nvidia.com References: <20230203050717.46914-1-jiaweiw@nvidia.com> <20230214154836.9681-1-jiaweiw@nvidia.com> <20230214154836.9681-2-jiaweiw@nvidia.com> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2/16/23 20:58, Ferruh Yigit wrote: > On 2/14/2023 3:48 PM, Jiawei Wang wrote: >> When multiple ports are aggregated into a single DPDK port, >> (example: Linux bonding, DPDK bonding, failsafe, etc.), >> we want to know which port use for Tx via a queue. >> >> This patch introduces the new ethdev API >> rte_eth_dev_map_aggr_tx_affinity(), it's used to map a Tx queue >> with an aggregated port of the DPDK port (specified with port_id), >> The affinity is the number of the aggregated port. >> Value 0 means no affinity and traffic could be routed to any >> aggregated port, this is the default current behavior. >> >> The maximum number of affinity is given by rte_eth_dev_count_aggr_ports(). >> >> Add the trace point for ethdev rte_eth_dev_count_aggr_ports() >> and rte_eth_dev_map_aggr_tx_affinity() functions. >> >> Add the testpmd command line: >> testpmd> port config (port_id) txq (queue_id) affinity (value) >> >> For example, there're two physical ports connected to >> a single DPDK port (port id 0), and affinity 1 stood for >> the first physical port and affinity 2 stood for the second >> physical port. >> Use the below commands to config tx phy affinity for per Tx Queue: >> port config 0 txq 0 affinity 1 >> port config 0 txq 1 affinity 1 >> port config 0 txq 2 affinity 2 >> port config 0 txq 3 affinity 2 >> >> These commands config the Tx Queue index 0 and Tx Queue index 1 with >> phy affinity 1, uses Tx Queue 0 or Tx Queue 1 send packets, >> these packets will be sent from the first physical port, and similar >> with the second physical port if sending packets with Tx Queue 2 >> or Tx Queue 3. >> >> Signed-off-by: Jiawei Wang > > <...> > >> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >> index dc0a4eb12c..1d5b3a16b2 100644 >> --- a/lib/ethdev/rte_ethdev.c >> +++ b/lib/ethdev/rte_ethdev.c >> @@ -6915,6 +6915,55 @@ rte_eth_buffer_split_get_supported_hdr_ptypes(uint16_t port_id, uint32_t *ptypes >> return j; >> } >> >> +int rte_eth_dev_count_aggr_ports(uint16_t port_id) >> +{ >> + struct rte_eth_dev *dev; >> + int ret; >> + >> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> + dev = &rte_eth_devices[port_id]; >> + >> + if (*dev->dev_ops->count_aggr_ports == NULL) >> + return -ENOTSUP; > > What do you think to return a default value when dev_ops is not defined, > assuming device is not a bounded device. > Not sure which one is better for application, return a default value or > error. I think 0 is better here. It simply means that rte_eth_dev_map_aggr_tx_affinity() cannot be used as well as corresponding flow API item. It will be true even for bonding as long as corresponding API is not supported. >> + ret = eth_err(port_id, (*dev->dev_ops->count_aggr_ports)(port_id)); >> + >> + rte_eth_trace_count_aggr_ports(port_id, ret); >> + >> + return ret; >> +} >> + >> +int rte_eth_dev_map_aggr_tx_affinity(uint16_t port_id, uint16_t tx_queue_id, >> + uint8_t affinity) >> +{ >> + struct rte_eth_dev *dev; >> + int ret; >> + >> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> + dev = &rte_eth_devices[port_id]; >> + >> + if (tx_queue_id >= dev->data->nb_tx_queues) { >> + RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", tx_queue_id); >> + return -EINVAL; >> + } >> + > > Although documentation says this API should be called before configure, Documentation says "after". Anyway, it is better to check vs dev_configured. > if user misses it I guess above can crash, is there a way to add runtime > check, like checking 'dev->data->dev_configured'? > > >> + if (*dev->dev_ops->map_aggr_tx_affinity == NULL) >> + return -ENOTSUP; >> + >> + if (dev->data->dev_started) { >> + RTE_ETHDEV_LOG(ERR, >> + "Port %u must be stopped to allow configuration\n", >> + port_id); >> + return -EBUSY; >> + } >> + >> + ret = eth_err(port_id, (*dev->dev_ops->map_aggr_tx_affinity)(port_id, >> + tx_queue_id, affinity)); >> + > > Should API check if port_id is a bonding port before it continue with > mapping? Since it is a control path I think it is a good idea to call rte_eth_dev_count_aggr_ports() and chck affinity value.