DPDK patches and discussions
 help / color / mirror / Atom feed
From: Kevin Liu <jin.liu@corigine.com>
To: Ferruh Yigit <ferruh.yigit@xilinx.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: Niklas Soderlund <niklas.soderlund@corigine.com>,
	Diana Wang <na.wang@corigine.com>,
	Nole Zhang <peng.zhang@corigine.com>,
	Chaoyong He <chaoyong.he@corigine.com>
Subject: RE: [PATCH 07/14] net/nfp: support NFDK firmware
Date: Tue, 14 Jun 2022 09:30:50 +0000	[thread overview]
Message-ID: <DM6PR13MB3004C04D6082ED0CE5F51EF194AA9@DM6PR13MB3004.namprd13.prod.outlook.com> (raw)
In-Reply-To: <95cf1563-b99f-968b-6bc4-7dd08c6a3ebe@xilinx.com>

Now, I modify logic, just keep 'nfp_net_tx_queue_release()', delete 'nfp_net_nfdk_tx_queue_release()' & 'nfp_net_nfd3_tx_queue_release()'


>>>   My comment was to extract the logic into its own function as it is done is PF, so to have something like 'nfp_netvf_ethdev_ops_mount()'.

the logic I just use once in nfp_ethdev_vf.c, so not create a function. I will create 'nfp_netvf_ethdev_ops_mount()' in nfp_ethdev_vf.c  later

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@xilinx.com> 
Sent: Tuesday, June 14, 2022 17:22
To: Kevin Liu <jin.liu@corigine.com>; dev@dpdk.org
Cc: Niklas Soderlund <niklas.soderlund@corigine.com>; Diana Wang <na.wang@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Chaoyong He <chaoyong.he@corigine.com>
Subject: Re: [PATCH 07/14] net/nfp: support NFDK firmware

On 6/14/2022 9:49 AM, Kevin Liu wrote:
> We also want to just use one function 'nfp_net_tx_queue_release()' to service both NFD3 and NFDk, But we can not get the version of NFD in function 'nfp_net_tx_queue_release()',  now get NFD version through 'hw->ver'
> 

Again, it is up to you, but it should be possible to add 'dev' or 'hw' 
reference to the queue struct, to be able to access the version information.
And it can be possible to have something like 'struct fw_ops', set it during initialization and use in rest of the dev_ops.

> For the function 'nfp_net_ethdev_ops_mount()', the logic below is in two different C files, nfp_ethdev.c and nfp_ethdev_vf.c And the variable of struct eth_dev_ops is defined as static, if we want to use function both in nfp_ethdev.c and nfp_ethdev_vf.c We need to change the eth_dev_ops variable as non-static, this is not we want.
> 
> 	> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
> 	> +	case NFP_NET_CFG_VERSION_DP_NFD3:
> 	> +		break;
> 	> +	case NFP_NET_CFG_VERSION_DP_NFDK:
> 	> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
> 	> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
> 	> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
> 	> +			return -EINVAL;
> 	> +		}
> 	> +		break;
> 	> +	default:
> 	> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
> 	> +		return -EINVAL;
> 

My comment was to extract the logic into its own function as it is done is PF, so to have something like 'nfp_netvf_ethdev_ops_mount()'.


> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Friday, June 3, 2022 06:54
> To: Kevin Liu <jin.liu@corigine.com>; dev@dpdk.org
> Cc: Niklas Soderlund <niklas.soderlund@corigine.com>; Diana Wang <na.wang@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Chaoyong He <chaoyong.he@corigine.com>
> Subject: Re: [PATCH 07/14] net/nfp: support NFDK firmware
> 
> On 6/2/2022 2:52 AM, Jin Liu wrote:
>> Modify nfp driver logic, add firmware version (NFD3 or NFDK) judgment,
>> will according to the firmware version, mount different driver functions.
>>
> 
> Creating a new set of dev_ops for new FW is a way and it works, but it looks like it creates some duplication of the code, and maintaining multiple dev_ops can be difficult (driver already has different ones for PF & VF).
> 
> Another option can be keeping ethdev interface same, but manage different FWs closer to FW, where directly interacted with FW.
> Like keeping dev_ops as 'nfp_net_tx_queue_release()' and managing different FW within this function, instead of having two dev_ops, 'nfp_net_nfdk_tx_queue_release()' & 'nfp_net_nfd3_tx_queue_release()'.
> If difference is small, this can be better to reduce duplication.
> 
> What is the difference between two FWs, as far as I can see Tx descriptor is different and queue setup is affected, is it only diff?
> 
>> Signed-off-by: Jin Liu <jin.liu@corigine.com>
>> Signed-off-by: Diana Wang <na.wang@corigine.com>
>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 
> <...>
> 
>> @@ -296,6 +296,32 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
>>    	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
>>    	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
>>    
>> +	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
>> +	if (hw->ctrl_bar == NULL) {
>> +		PMD_DRV_LOG(ERR,
>> +			"hw->ctrl_bar is NULL. BAR0 not configured");
>> +		return -ENODEV;
>> +	}
>> +
>> +	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
>> +
>> +	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
>> +
>> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
>> +	case NFP_NET_CFG_VERSION_DP_NFD3:
>> +		break;
>> +	case NFP_NET_CFG_VERSION_DP_NFDK:
>> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
>> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
>> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
>> +			return -EINVAL;
>> +		}
>> +		break;
>> +	default:
>> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
>> +		return -EINVAL;
>> +	}
>> +
> 
> This part seems extracted to its own function for PF ('nfp_net_ethdev_ops_mount()'), why not do the same for VF, to have same logic between them.
> 
> 


  reply	other threads:[~2022-06-14  9:30 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-02  1:52 [PATCH 00/14] Add support of NFP3800 chip and firmware with NFDk Jin Liu
2022-06-02  1:52 ` [PATCH 01/14] net/nfp: change the coding style Jin Liu
2022-06-02 22:52   ` Ferruh Yigit
2022-06-14  8:50     ` Kevin Liu
2022-06-02  1:52 ` [PATCH 02/14] net/nfp: remove unnecessary forward function declaration Jin Liu
2022-06-02  1:52 ` [PATCH 03/14] net/nfp: remove pessimistic limit Jin Liu
2022-06-02  1:52 ` [PATCH 04/14] net/nfp: rename set MAC function Jin Liu
2022-06-02  1:52 ` [PATCH 05/14] net/nfp: rename function and struct Jin Liu
2022-06-02  1:52 ` [PATCH 06/14] net/nfp: support NFP3800 card Jin Liu
2022-06-02 22:52   ` Ferruh Yigit
2022-06-14  8:50     ` Kevin Liu
2022-06-02  1:52 ` [PATCH 07/14] net/nfp: support NFDK firmware Jin Liu
2022-06-02 22:53   ` Ferruh Yigit
2022-06-14  8:49     ` Kevin Liu
2022-06-14  9:21       ` Ferruh Yigit
2022-06-14  9:30         ` Kevin Liu [this message]
2022-06-02  1:52 ` [PATCH 08/14] net/nfp: structure adjustment Jin Liu
2022-06-02 22:54   ` Ferruh Yigit
2022-06-14  8:49     ` Kevin Liu
2022-06-02  1:52 ` [PATCH 09/14] net/nfp: nfdk netdev option and queue function Jin Liu
2022-06-02  1:53 ` [PATCH 10/14] net/nfp: add queue stop and close helper function Jin Liu
2022-06-02  1:53 ` [PATCH 11/14] net/nfp: nfdk stop and close function Jin Liu
2022-06-02  1:53 ` [PATCH 12/14] net/nfp: move macro from C file to head file Jin Liu
2022-06-02  1:53 ` [PATCH 13/14] net/nfp: nfdk packet xmit function Jin Liu
2022-06-02  1:53 ` [PATCH 14/14] net/nfp: modify RSS logic Jin Liu
2022-06-02 22:56   ` Ferruh Yigit
2022-06-14  8:50     ` Kevin Liu
2022-06-02 22:51 ` [PATCH 00/14] Add support of NFP3800 chip and firmware with NFDk Ferruh Yigit
2022-06-14  8:48   ` Kevin Liu
2022-06-16  2:39 ` [PATCH v2 00/15] " Jin Liu
2022-06-16  2:39   ` [PATCH v2 01/15] doc: update release note Jin Liu
2022-06-16 15:04     ` Ferruh Yigit
2022-06-16  2:39   ` [PATCH v2 02/15] doc: update nfp documentation Jin Liu
2022-06-16 15:04     ` Ferruh Yigit
2022-06-16  2:39   ` [PATCH v2 03/15] net/nfp: change the coding style Jin Liu
2022-06-16  2:39   ` [PATCH v2 04/15] net/nfp: remove unnecessary forward function declaration Jin Liu
2022-06-16  2:39   ` [PATCH v2 05/15] net/nfp: remove pessimistic limit Jin Liu
2022-06-16  2:39   ` [PATCH v2 06/15] net/nfp: rename set MAC function Jin Liu
2022-06-16  2:39   ` [PATCH v2 07/15] net/nfp: rename function and struct Jin Liu
2022-06-16  2:39   ` [PATCH v2 08/15] net/nfp: support NFP3800 card Jin Liu
2022-06-16 15:04     ` Ferruh Yigit
2022-06-16  2:39   ` [PATCH v2 09/15] net/nfp: support firmware with NFDk Jin Liu
2022-06-16  2:39   ` [PATCH v2 10/15] net/nfp: structure adjustment Jin Liu
2022-06-16  2:39   ` [PATCH v2 11/15] net/nfp: nfdk netdev option and queue function Jin Liu
2022-06-16  2:39   ` [PATCH v2 12/15] net/nfp: add queue stop and close helper function Jin Liu
2022-06-16  2:39   ` [PATCH v2 13/15] net/nfp: move macro from C file to head file Jin Liu
2022-06-16  2:39   ` [PATCH v2 14/15] net/nfp: nfdk packet xmit function Jin Liu
2022-06-16  2:39   ` [PATCH v2 15/15] net/nfp: modify RSS logic Jin Liu
2022-06-16 15:06   ` [PATCH v2 00/15] Add support of NFP3800 chip and firmware with NFDk Ferruh Yigit
2022-06-17  9:34   ` [PATCH v3 00/13] " Jin Liu
2022-06-17  9:34     ` [PATCH v3 01/13] net/nfp: change the coding style Jin Liu
2022-06-17  9:34     ` [PATCH v3 02/13] net/nfp: remove unnecessary forward function declaration Jin Liu
2022-06-17  9:34     ` [PATCH v3 03/13] net/nfp: remove pessimistic limit Jin Liu
2022-06-17  9:34     ` [PATCH v3 04/13] net/nfp: rename set MAC function Jin Liu
2022-06-17  9:34     ` [PATCH v3 05/13] net/nfp: rename function and struct Jin Liu
2022-06-17  9:34     ` [PATCH v3 06/13] net/nfp: support NFP3800 card Jin Liu
2022-06-17  9:34     ` [PATCH v3 07/13] net/nfp: support firmware with NFDk Jin Liu
2022-06-17  9:34     ` [PATCH v3 08/13] net/nfp: structure adjustment Jin Liu
2022-06-17  9:34     ` [PATCH v3 09/13] net/nfp: nfdk netdev option and queue function Jin Liu
2022-06-17  9:34     ` [PATCH v3 10/13] net/nfp: add queue stop and close helper function Jin Liu
2022-06-17  9:34     ` [PATCH v3 11/13] net/nfp: move macro from C file to head file Jin Liu
2022-06-17  9:34     ` [PATCH v3 12/13] net/nfp: nfdk packet xmit function Jin Liu
2022-06-22 16:03       ` Thomas Monjalon
2022-06-22 16:29         ` Ferruh Yigit
2022-06-17  9:34     ` [PATCH v3 13/13] net/nfp: modify RSS logic Jin Liu
2022-06-17 13:33     ` [PATCH v3 00/13] Add support of NFP3800 chip and firmware with NFDk Ferruh Yigit
2022-06-23  2:26     ` [PATCH v4 " Jin Liu
2022-06-23  2:26       ` [PATCH v4 01/13] net/nfp: change the coding style Jin Liu
2022-06-23  2:26       ` [PATCH v4 02/13] net/nfp: remove unnecessary forward function declaration Jin Liu
2022-06-23  2:26       ` [PATCH v4 03/13] net/nfp: remove pessimistic limit Jin Liu
2022-06-23  2:26       ` [PATCH v4 04/13] net/nfp: rename set MAC function Jin Liu
2022-06-23  2:26       ` [PATCH v4 05/13] net/nfp: rename function and struct Jin Liu
2022-06-23  2:26       ` [PATCH v4 06/13] net/nfp: support NFP3800 card Jin Liu
2022-06-23  2:26       ` [PATCH v4 07/13] net/nfp: support firmware with NFDk Jin Liu
2022-06-23  2:26       ` [PATCH v4 08/13] net/nfp: structure adjustment Jin Liu
2022-06-23  2:26       ` [PATCH v4 09/13] net/nfp: nfdk netdev option and queue function Jin Liu
2022-06-23  2:26       ` [PATCH v4 10/13] net/nfp: add queue stop and close helper function Jin Liu
2022-06-23  2:26       ` [PATCH v4 11/13] net/nfp: move macro from C file to head file Jin Liu
2022-06-23  2:26       ` [PATCH v4 12/13] net/nfp: nfdk packet xmit function Jin Liu
2022-06-23  2:26       ` [PATCH v4 13/13] net/nfp: modify RSS logic Jin Liu
2022-06-23  9:11       ` [PATCH v4 00/13] Add support of NFP3800 chip and firmware with NFDk Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR13MB3004C04D6082ED0CE5F51EF194AA9@DM6PR13MB3004.namprd13.prod.outlook.com \
    --to=jin.liu@corigine.com \
    --cc=chaoyong.he@corigine.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@xilinx.com \
    --cc=na.wang@corigine.com \
    --cc=niklas.soderlund@corigine.com \
    --cc=peng.zhang@corigine.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).