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 4D131A0032; Wed, 14 Sep 2022 11:25:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E8FF640156; Wed, 14 Sep 2022 11:25:49 +0200 (CEST) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id 4944240151 for ; Wed, 14 Sep 2022 11:25:49 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id ED62A5C013F; Wed, 14 Sep 2022 05:25:48 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Wed, 14 Sep 2022 05:25:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1663147548; x= 1663233948; bh=UJLqTBAOWbduzReQQ6J3CZx6GPVX3cMC0qqKkTAbZwM=; b=m l86xGkAnCycIcDVmsZ89W8vENTjxqG2XG/wGaYyIyP1qW83Iy/0VJTAoXzn10cSe 1Q/rhEYrQ1dgOS9sNFqPO5XTUQ5SuRcB5GRhnGQmRU3n6+1gg7YnJ6rHynw33k5Z 835Mh5yTXpQRPCE5ylx5okt8Jq82yy7Z5n2hruTFoeMTHjxW+fzhzXl6SCTWL9o7 l3WDf8gdzQxdp4NLOQdlhe1nmdhCXfJBGstcVfJEreBaCwwAptsm8p8fCPqlGH6S 926PQMO0KOX2hqZ3jJ5fWkk4hLSueNVaW5DTwV0hEZMs+RMnksB9ixprTlpX3eKA LfJBHsC8f+zyrTm/4Bsyg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1663147548; x= 1663233948; bh=UJLqTBAOWbduzReQQ6J3CZx6GPVX3cMC0qqKkTAbZwM=; b=M g6bvwVhOKjGTqZLjS1HPniLUfRk9BbSbyfjgDcwwpLR8ZkWvMMan4mCDvblgwIF5 baWp3jyyCmUyo8kZmMH9i4pfd0oEK2MhO1kcTQkxGqS5UYQHpWPOKi0EbMFmM1yS 3IRq+zme5UZ+6XbL0RFDsyL4jXobBwKFucRJqBw/EmdiC/8cRSaNP7DIDJ68dFN1 496f4dDeCtCS235UyElCpO4C134bWoHhLmDdjKyuUDHtnYRZE3JYnyKivGIPudUc 4kX3/hK4DvfKDkBQiGMoiXBPM3fpMUW3oWV0vWMdzdqV/gyU0+8WJ+4WHDb0Stkp 80Ea1zqRFrw1r9aQkAizQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeduiedgudeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefufffkjghfggfgtgesthhqredttddtudenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpeefhfejleeuvdevtddutdeutdevhfeijeethfffueejhfetuddu vedtkedtieekffenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 14 Sep 2022 05:25:47 -0400 (EDT) From: Thomas Monjalon To: Chaoyong He Cc: Ferruh Yigit , Jerin Jacob Kollanukkaran , Andrew Rybchenko , dev@dpdk.org, oss-drivers , Niklas Soderlund , "dev@dpdk.org" Subject: Re: [PATCH v8 05/12] net/nfp: add flower PF setup logic Date: Wed, 14 Sep 2022 11:25:46 +0200 Message-ID: <2997927.CbtlEUcBR6@thomas> In-Reply-To: References: <1662626702-17254-1-git-send-email-chaoyong.he@corigine.com> <9aed41cf-c1f0-fa2e-cc2c-f4dc1e17f390@xilinx.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" 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 14/09/2022 03:23, Chaoyong He: > > On 9/13/2022 7:51 AM, Chaoyong He wrote: > > > > > >> On 9/9/2022 3:36 AM, Chaoyong He wrote: > > >>>> On 9/8/2022 9:44 AM, Chaoyong He wrote: > > >>>>> Adds the vNIC initialization logic for the flower PF vNIC. The > > >>>>> flower firmware exposes this vNIC for the purposes of fallback > > >>>>> traffic in the switchdev use-case. > > >>>>> > > >>>>> Adds minimal dev_ops for this PF device. Because the device is > > >>>>> being exposed externally to DPDK it should also be configured > > >>>>> using DPDK helpers like rte_eth_configure(). For these helpers to > > >>>>> work the flower logic needs to implements a minimal set of dev_op= s. > > >>>>> > > >>>>> Signed-off-by: Chaoyong He > > >>>>> Reviewed-by: Niklas S=F6derlund > > >> > > >> <...> > > >> > > >>>>> +static int > > >>>>> +nfp_flower_init_pf_vnic(struct nfp_net_hw *hw) { int ret; > > >>>>> +uint16_t i; uint16_t n_txq; uint16_t n_rxq; uint16_t port_id; > > >>>>> +unsigned int numa_node; struct rte_mempool *mp; struct > > >>>>> +nfp_pf_dev *pf_dev; struct rte_eth_dev *eth_dev; struct > > >>>>> +nfp_app_fw_flower *app_fw_flower; > > >>>>> + > > >>>>> + static const struct rte_eth_conf port_conf =3D { > > >>>>> + .rxmode =3D { > > >>>>> + .mq_mode =3D RTE_ETH_MQ_RX_RSS, > > >>>>> + .offloads =3D RTE_ETH_RX_OFFLOAD_CHECKSUM, > > >>>>> + }, > > >>>>> + .txmode =3D { > > >>>>> + .mq_mode =3D RTE_ETH_MQ_TX_NONE, > > >>>>> + }, > > >>>>> + }; > > >>>>> + > > >>>>> + /* Set up some pointers here for ease of use */ pf_dev =3D > > >>>>> + hw->pf_dev; app_fw_flower =3D > > NFP_PRIV_TO_APP_FW_FLOWER(pf_dev- > > >>>>> app_fw_priv); > > >>>>> + > > >>>>> + /* > > >>>>> + * Perform the "common" part of setting up a flower vNIC. > > >>>>> + * Mostly reading configuration from hardware. > > >>>>> + */ > > >>>>> + ret =3D nfp_flower_init_vnic_common(hw, "pf_vnic"); if (ret != =3D 0) > > >>>>> + goto done; > > >>>>> + > > >>>>> + hw->eth_dev =3D rte_eth_dev_allocate("nfp_pf_vnic"); > > >>>>> + if (hw->eth_dev =3D=3D NULL) { > > >>>>> + ret =3D -ENOMEM; > > >>>>> + goto done; > > >>>>> + } > > >>>>> + > > >>>>> + /* Grab the pointer to the newly created rte_eth_dev here */ > > >>>>> + eth_dev =3D hw->eth_dev; > > >>>>> + > > >>>>> + numa_node =3D rte_socket_id(); > > >>>>> + > > >>>>> + /* Fill in some of the eth_dev fields */ eth_dev->device =3D > > >>>>> + &pf_dev->pci_dev->device; eth_dev->data->dev_private =3D hw; > > >>>>> + > > >>>>> + /* Create a mbuf pool for the PF */ > > >>>>> + app_fw_flower->pf_pktmbuf_pool =3D nfp_flower_pf_mp_create(); > > if > > >>>>> + (app_fw_flower->pf_pktmbuf_pool =3D=3D NULL) { > > >>>>> + ret =3D -ENOMEM; > > >>>>> + goto port_release; > > >>>>> + } > > >>>>> + > > >>>>> + mp =3D app_fw_flower->pf_pktmbuf_pool; > > >>>>> + > > >>>>> + /* Add Rx/Tx functions */ > > >>>>> + eth_dev->dev_ops =3D &nfp_flower_pf_vnic_ops; > > >>>>> + > > >>>>> + /* PF vNIC gets a random MAC */ > > >>>>> + eth_dev->data->mac_addrs =3D rte_zmalloc("mac_addr", > > >>>> RTE_ETHER_ADDR_LEN, 0); > > >>>>> + if (eth_dev->data->mac_addrs =3D=3D NULL) { > > >>>>> + ret =3D -ENOMEM; > > >>>>> + goto mempool_cleanup; > > >>>>> + } > > >>>>> + > > >>>>> + rte_eth_random_addr(eth_dev->data->mac_addrs->addr_bytes); > > >>>>> + rte_eth_dev_probing_finish(eth_dev); > > >>>>> + > > >>>>> + /* Configure the PF device now */ n_rxq =3D hw->max_rx_queues; > > >>>>> + n_txq =3D hw->max_tx_queues; port_id =3D hw->eth_dev->data- > > >port_id; > > >>>>> + > > >>>>> + ret =3D rte_eth_dev_configure(port_id, n_rxq, n_txq, &port_conf= ); > > >>>> > > >>>> Still not sure about PMD calling 'rte_eth_dev_configure()', can you > > >>>> please give more details on what specific configuration is expected > > >>>> with > > >> that call? > > >>> > > >>> The main configuration we need is the number of rx/tx queue. > > >>> So we should use the internal api `eth_dev_rx/tx_queue_config` to > > instead? > > >>> > > >> > > >> nb_rx_q/nb_tx_q are parameters provided by user (via > > >> rte_eth_dev_configure()), won't is wrong for PMD to set a value on i= ts > > own? > > >> > > >> Why nb_rx_q/nb_tx_q are required in the probe() stage? Probe stage is > > >> not to configure the device. > > > > > > Our nfp card use `control message` to exchange message between PMD > > and firmware when we use flower firmware. > > > The control message is in the form of pkt and we use a `ctrl vNIC` eh= tdev as > > the agent to send and receive these pkts. > > > e.g., if we want to create representor port, the PMD must send the > > corresponding control message to firmware. > > > > > > This `ctrl vNIC` is totally user app Invisible, to make it able to s= end and > > receive pkt, we must do some configure steps to this ethdev ourselves f= irstly. > > > We can don't use 'rte_eth_dev_configure()', but we still should do the > > needed configure steps to make sure the device can work. > > > > >=20 > > How ethdev instance becomes app invisible, unless owner set properly I > > think apps can able to see it. > > And if needs to be internal, do you really need to create an ethdev? Wh= y not > > communicate with HW via internal functions? > >=20 > > Cc'ed Jerin, their driver also communicate with FW before probing. > > @Jerin, can you please review this patch and help on the design? > >=20 > > @Thomas, @Andrew, can you please check the design too? I agree with Ferruh. ethdev ports are for upper levels. > There still main problem for now: > 1. Driver shouldn't need to update the bus related data struct. I don't see how it's related. Just do your communication as device-specific object. The EAL device is to manage HW resource, this is what you should extend. The ethdev port is to expose Rx/Tx to the application. > 2. Probe stage should not configure the device. When probing, you need to initialize communication with HW. > And the first problem won't exist if we solve the second problem.=20 >=20 > So, how about we move all the logics relates to configuration into servic= e? > According to the logic in rte_eal_init(): > ``` > ... > if (rte_bus_probe()) { > ... > ret =3D rte_service_start_with_defaults(); > if (ret < 0 && ret !=3D -ENOTSUP) { > ... > ``` > Then this can guarantee that these logics run after the probe stage. No, you can manage all in your driver with touching the rest of DPDK. > I'll send a new version patch series based on this, and we can continue d= iscussing about the best choice. Please try what is described above.