> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@amd.com>
> Sent: Thursday, March 2, 2023 7:51 PM
> To: Liu, Mingxia <mingxia....@intel.com>; dev@dpdk.org; Xing, Beilei
> <beilei.x...@intel.com>; Zhang, Yuying <yuying.zh...@intel.com>
> Subject: Re: [PATCH v8 01/21] net/cpfl: support device initialization
>
> On 3/2/2023 11:24 AM, Liu, Mingxia wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yi...@amd.com>
> >> Sent: Thursday, March 2, 2023 5:31 PM
> >> To: Liu, Mingxia <mingxia....@intel.com>; dev@dpdk.org; Xing, Beilei
> >> <beilei.x...@intel.com>; Zhang, Yuying <yuying.zh...@intel.com>
> >> Subject: Re: [PATCH v8 01/21] net/cpfl: support device initialization
> >>
> >> On 3/2/2023 10:35 AM, Mingxia Liu wrote:
> >>> Support device init and add the following dev ops:
> >>> - dev_configure
> >>> - dev_close
> >>> - dev_infos_get
> >>> - link_update
> >>> - dev_supported_ptypes_get
> >>>
> >>> Signed-off-by: Mingxia Liu <mingxia....@intel.com>
> >>
> >> <...>
> >>
> >>> --- /dev/null
> >>> +++ b/doc/guides/nics/cpfl.rst
> >>> @@ -0,0 +1,85 @@
> >>> +.. SPDX-License-Identifier: BSD-3-Clause
> >>> + Copyright(c) 2022 Intel Corporation.
> >>> +
> >>
> >> s/2022/2023
> >>
> >> <...>
> >>
> >>> +static int
> >>> +cpfl_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> >>> + struct rte_pci_device *pci_dev) {
> >>> + struct cpfl_vport_param vport_param;
> >>> + struct cpfl_adapter_ext *adapter;
> >>> + struct cpfl_devargs devargs;
> >>> + char name[RTE_ETH_NAME_MAX_LEN];
> >>> + int i, retval;
> >>> + bool first_probe = false;
> >>> +
> >>> + if (!cpfl_adapter_list_init) {
> >>> + rte_spinlock_init(&cpfl_adapter_lock);
> >>> + TAILQ_INIT(&cpfl_adapter_list);
> >>> + cpfl_adapter_list_init = true;
> >>> + }
> >>> +
> >>> + adapter = cpfl_find_adapter_ext(pci_dev);
> >>> + if (adapter == NULL) {
> >>> + first_probe = true;
> >>> + adapter = rte_zmalloc("cpfl_adapter_ext",
> >>> + sizeof(struct cpfl_adapter_ext), 0);
> >>> + if (adapter == NULL) {
> >>> + PMD_INIT_LOG(ERR, "Failed to allocate adapter.");
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + retval = cpfl_adapter_ext_init(pci_dev, adapter);
> >>> + if (retval != 0) {
> >>> + PMD_INIT_LOG(ERR, "Failed to init adapter.");
> >>> + return retval;
> >>> + }
> >>> +
> >>> + rte_spinlock_lock(&cpfl_adapter_lock);
> >>> + TAILQ_INSERT_TAIL(&cpfl_adapter_list, adapter, next);
> >>> + rte_spinlock_unlock(&cpfl_adapter_lock);
> >>> + }
> >>> +
> >>> + retval = cpfl_parse_devargs(pci_dev, adapter, &devargs);
> >>> + if (retval != 0) {
> >>> + PMD_INIT_LOG(ERR, "Failed to parse private devargs");
> >>> + goto err;
> >>> + }
> >>> +
> >>> + if (devargs.req_vport_nb == 0) {
> >>> + /* If no vport devarg, create vport 0 by default. */
> >>> + vport_param.adapter = adapter;
> >>> + vport_param.devarg_id = 0;
> >>> + vport_param.idx = cpfl_vport_idx_alloc(adapter);
> >>> + if (vport_param.idx == CPFL_INVALID_VPORT_IDX) {
> >>> + PMD_INIT_LOG(ERR, "No space for vport %u",
> >> vport_param.devarg_id);
> >>> + return 0;
> >>> + }
> >>> + snprintf(name, sizeof(name), "cpfl_%s_vport_0",
> >>> + pci_dev->device.name);
> >>> + retval = rte_eth_dev_create(&pci_dev->device, name,
> >>> + sizeof(struct idpf_vport),
> >>> + NULL, NULL, cpfl_dev_vport_init,
> >>> + &vport_param);
> >>> + if (retval != 0)
> >>> + PMD_DRV_LOG(ERR, "Failed to create default vport
> >> 0");
> >>> + } else {
> >>> + for (i = 0; i < devargs.req_vport_nb; i++) {
> >>> + vport_param.adapter = adapter;
> >>> + vport_param.devarg_id = devargs.req_vports[i];
> >>> + vport_param.idx = cpfl_vport_idx_alloc(adapter);
> >>> + if (vport_param.idx == CPFL_INVALID_VPORT_IDX) {
> >>> + PMD_INIT_LOG(ERR, "No space for
> >> vport %u", vport_param.devarg_id);
> >>> + break;
> >>> + }
> >>> + snprintf(name, sizeof(name), "cpfl_%s_vport_%d",
> >>> + pci_dev->device.name,
> >>> + devargs.req_vports[i]);
> >>> + retval = rte_eth_dev_create(&pci_dev->device,
> >> name,
> >>> + sizeof(struct idpf_vport),
> >>> + NULL, NULL,
> >> cpfl_dev_vport_init,
> >>> + &vport_param);
> >>> + if (retval != 0)
> >>> + PMD_DRV_LOG(ERR, "Failed to create
> >> vport %d",
> >>> + vport_param.devarg_id);
> >>> + }
> >>> + }
> >>> +
> >>> + return 0;
> >>> +
> >>> +err:
> >>> + if (first_probe) {
> >>> + rte_spinlock_lock(&cpfl_adapter_lock);
> >>> + TAILQ_REMOVE(&cpfl_adapter_list, adapter, next);
> >>> + rte_spinlock_unlock(&cpfl_adapter_lock);
> >>> + cpfl_adapter_ext_deinit(adapter);
> >>> + rte_free(adapter);
> >>> + }
> >>
> >> Is 'first_probe' left intentionally? If so, what is the reason to
> >> have this condition?
> > [beileix] It's related to create vports at runtime, this feature will be
> implemented in the future, but now it doesn't suppor.
>
> is it possible to remove it now and add back when needed? This is confusing as
> it is.
>
Make sense. Will remove it in this patch.
Beilei
