diff mbox series

[v6,2/8] net/rnp: add ethdev probe and remove

Message ID 20230901023050.40893-3-caowenbo@mucse.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series drivers/net Add Support mucse N10 Pmd Driver |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

11 Sept. 1, 2023, 2:30 a.m. UTC 
  Add basic PCIe ethdev probe and remove.

Signed-off-by: Wenbo Cao <caowenbo@mucse.com>
---
 drivers/net/rnp/rnp.h        | 13 ++++++
 drivers/net/rnp/rnp_ethdev.c | 83 ++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)
 create mode 100644 drivers/net/rnp/rnp.h

Comments

Ferruh Yigit Sept. 5, 2023, 3:36 p.m. UTC | #1
Unaddressed 
On 9/1/2023 3:30 AM, Wenbo Cao wrote:
> Add basic PCIe ethdev probe and remove.
> 
> Signed-off-by: Wenbo Cao <caowenbo@mucse.com>
> ---
>  drivers/net/rnp/rnp.h        | 13 ++++++
>  drivers/net/rnp/rnp_ethdev.c | 83 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 96 insertions(+)
>  create mode 100644 drivers/net/rnp/rnp.h
> 
> diff --git a/drivers/net/rnp/rnp.h b/drivers/net/rnp/rnp.h
> new file mode 100644
> index 0000000000..76d281cc0a
> --- /dev/null
> +++ b/drivers/net/rnp/rnp.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2023 Mucse IC Design Ltd.
> + */
> +#ifndef __RNP_H__
> +#define __RNP_H__
> +
> +#define PCI_VENDOR_ID_MUCSE	(0x8848)
> +#define RNP_DEV_ID_N10G		(0x1000)
> +
> +struct rnp_eth_port {
> +} __rte_cache_aligned;
> +

Is the struct needs to be cache aligned?

> +#endif /* __RNP_H__ */
> diff --git a/drivers/net/rnp/rnp_ethdev.c b/drivers/net/rnp/rnp_ethdev.c
> index 9ce3c0b497..390f2e7743 100644
> --- a/drivers/net/rnp/rnp_ethdev.c
> +++ b/drivers/net/rnp/rnp_ethdev.c
> @@ -1,3 +1,86 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright(C) 2023 Mucse IC Design Ltd.
>   */
> +
> +#include <ethdev_pci.h>
> +#include <rte_io.h>
> +#include <rte_malloc.h>
> +
> +#include "rnp.h"
> +
> +static int
> +rnp_eth_dev_init(struct rte_eth_dev *eth_dev)
> +{
> +	RTE_SET_USED(eth_dev);
> +
> +	return -ENODEV;
> +}
> +
> +static int
> +rnp_eth_dev_uninit(struct rte_eth_dev *eth_dev)
> +{
> +	RTE_SET_USED(eth_dev);
> +
> +	return -ENODEV;
> +}
> +
> +static int
> +rnp_pci_remove(struct rte_pci_device *pci_dev)
> +{
> +	struct rte_eth_dev *eth_dev;
> +	int rc;
> +
> +	eth_dev = rte_eth_dev_allocated(pci_dev->device.name);
> +
> +	if (eth_dev) {
> +		/* Cleanup eth dev */
> +		rc = rte_eth_dev_pci_generic_remove(pci_dev,
> +				rnp_eth_dev_uninit);
> +		if (rc)
> +			return rc;
> +	}
> +	/* Nothing to be done for secondary processes */
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return 0;
> +

Primary also don't do anything after this stage, I suggest adding this
code when primary & secondary diverges.

> +	return 0;
> +}
> +
> +static int
> +rnp_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
> +{
> +	int rc;
> +
> +	RTE_SET_USED(pci_drv);
> +
> +	rc = rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct rnp_eth_port),
> +					   rnp_eth_dev_init);
> +
> +	/* On error on secondary, recheck if port exists in primary or
> +	 * in mid of detach state.
> +	 */
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY && rc)
> +		if (!rte_eth_dev_allocated(pci_dev->device.name))
> +			return 0;

Why this additional secondary check is required?
'rte_eth_dev_pci_generic_probe()' should be dealing with
primary/secondary process case.
11 Sept. 6, 2023, 10:42 a.m. UTC | #2
Unaddressed 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: 2023年9月5日 23:36
> To: Wenbo Cao <caowenbo@mucse.com>; Anatoly Burakov
> <anatoly.burakov@intel.com>
> Cc: dev@dpdk.org; thomas@monjalon.net; andrew.rybchenko@oktetlabs.ru;
> yaojun@mucse.com
> Subject: Re: [PATCH v6 2/8] net/rnp: add ethdev probe and remove
> 
> On 9/1/2023 3:30 AM, Wenbo Cao wrote:
> > Add basic PCIe ethdev probe and remove.
> >
> > Signed-off-by: Wenbo Cao <caowenbo@mucse.com>
> > ---
> >  drivers/net/rnp/rnp.h        | 13 ++++++
> >  drivers/net/rnp/rnp_ethdev.c | 83
> > ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 96 insertions(+)
> >  create mode 100644 drivers/net/rnp/rnp.h
> >
> > diff --git a/drivers/net/rnp/rnp.h b/drivers/net/rnp/rnp.h new file
> > mode 100644 index 0000000000..76d281cc0a
> > --- /dev/null
> > +++ b/drivers/net/rnp/rnp.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(C) 2023 Mucse IC Design Ltd.
> > + */
> > +#ifndef __RNP_H__
> > +#define __RNP_H__
> > +
> > +#define PCI_VENDOR_ID_MUCSE	(0x8848)
> > +#define RNP_DEV_ID_N10G		(0x1000)
> > +
> > +struct rnp_eth_port {
> > +} __rte_cache_aligned;
> > +
> 
> Is the struct needs to be cache aligned?
> 
This struct may use frequently so I want  it to fast to load.
> > +#endif /* __RNP_H__ */
> > diff --git a/drivers/net/rnp/rnp_ethdev.c
> > b/drivers/net/rnp/rnp_ethdev.c index 9ce3c0b497..390f2e7743 100644
> > --- a/drivers/net/rnp/rnp_ethdev.c
> > +++ b/drivers/net/rnp/rnp_ethdev.c
> > @@ -1,3 +1,86 @@
> >  /* SPDX-License-Identifier: BSD-3-Clause
> >   * Copyright(C) 2023 Mucse IC Design Ltd.
> >   */
> > +
> > +#include <ethdev_pci.h>
> > +#include <rte_io.h>
> > +#include <rte_malloc.h>
> > +
> > +#include "rnp.h"
> > +
> > +static int
> > +rnp_eth_dev_init(struct rte_eth_dev *eth_dev) {
> > +	RTE_SET_USED(eth_dev);
> > +
> > +	return -ENODEV;
> > +}
> > +
> > +static int
> > +rnp_eth_dev_uninit(struct rte_eth_dev *eth_dev) {
> > +	RTE_SET_USED(eth_dev);
> > +
> > +	return -ENODEV;
> > +}
> > +
> > +static int
> > +rnp_pci_remove(struct rte_pci_device *pci_dev) {
> > +	struct rte_eth_dev *eth_dev;
> > +	int rc;
> > +
> > +	eth_dev = rte_eth_dev_allocated(pci_dev->device.name);
> > +
> > +	if (eth_dev) {
> > +		/* Cleanup eth dev */
> > +		rc = rte_eth_dev_pci_generic_remove(pci_dev,
> > +				rnp_eth_dev_uninit);
> > +		if (rc)
> > +			return rc;
> > +	}
> > +	/* Nothing to be done for secondary processes */
> > +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > +		return 0;
> > +
> 
> Primary also don't do anything after this stage, I suggest adding this code when
> primary & secondary diverges.
> 
Ok, I get it.
> > +	return 0;
> > +}
> > +
> > +static int
> > +rnp_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device
> > +*pci_dev) {
> > +	int rc;
> > +
> > +	RTE_SET_USED(pci_drv);
> > +
> > +	rc = rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct
> rnp_eth_port),
> > +					   rnp_eth_dev_init);
> > +
> > +	/* On error on secondary, recheck if port exists in primary or
> > +	 * in mid of detach state.
> > +	 */
> > +	if (rte_eal_process_type() != RTE_PROC_PRIMARY && rc)
> > +		if (!rte_eth_dev_allocated(pci_dev->device.name))
> > +			return 0;
> 
> Why this additional secondary check is required?
> 'rte_eth_dev_pci_generic_probe()' should be dealing with primary/secondary
> process case.
> 
Because of multiple port mode single pcie-bdf the other port
Alloc use xx:xx.0_0, xx:xx.0_1, xx:xx.0_2, xx:xx.0_0,  use rnp_alloc_eth_port api.
diff mbox series

Patch

diff --git a/drivers/net/rnp/rnp.h b/drivers/net/rnp/rnp.h
new file mode 100644
index 0000000000..76d281cc0a
--- /dev/null
+++ b/drivers/net/rnp/rnp.h
@@ -0,0 +1,13 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2023 Mucse IC Design Ltd.
+ */
+#ifndef __RNP_H__
+#define __RNP_H__
+
+#define PCI_VENDOR_ID_MUCSE	(0x8848)
+#define RNP_DEV_ID_N10G		(0x1000)
+
+struct rnp_eth_port {
+} __rte_cache_aligned;
+
+#endif /* __RNP_H__ */
diff --git a/drivers/net/rnp/rnp_ethdev.c b/drivers/net/rnp/rnp_ethdev.c
index 9ce3c0b497..390f2e7743 100644
--- a/drivers/net/rnp/rnp_ethdev.c
+++ b/drivers/net/rnp/rnp_ethdev.c
@@ -1,3 +1,86 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(C) 2023 Mucse IC Design Ltd.
  */
+
+#include <ethdev_pci.h>
+#include <rte_io.h>
+#include <rte_malloc.h>
+
+#include "rnp.h"
+
+static int
+rnp_eth_dev_init(struct rte_eth_dev *eth_dev)
+{
+	RTE_SET_USED(eth_dev);
+
+	return -ENODEV;
+}
+
+static int
+rnp_eth_dev_uninit(struct rte_eth_dev *eth_dev)
+{
+	RTE_SET_USED(eth_dev);
+
+	return -ENODEV;
+}
+
+static int
+rnp_pci_remove(struct rte_pci_device *pci_dev)
+{
+	struct rte_eth_dev *eth_dev;
+	int rc;
+
+	eth_dev = rte_eth_dev_allocated(pci_dev->device.name);
+
+	if (eth_dev) {
+		/* Cleanup eth dev */
+		rc = rte_eth_dev_pci_generic_remove(pci_dev,
+				rnp_eth_dev_uninit);
+		if (rc)
+			return rc;
+	}
+	/* Nothing to be done for secondary processes */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	return 0;
+}
+
+static int
+rnp_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
+{
+	int rc;
+
+	RTE_SET_USED(pci_drv);
+
+	rc = rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct rnp_eth_port),
+					   rnp_eth_dev_init);
+
+	/* On error on secondary, recheck if port exists in primary or
+	 * in mid of detach state.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY && rc)
+		if (!rte_eth_dev_allocated(pci_dev->device.name))
+			return 0;
+	return rc;
+}
+
+static const struct rte_pci_id pci_id_rnp_map[] = {
+	{
+		RTE_PCI_DEVICE(PCI_VENDOR_ID_MUCSE, RNP_DEV_ID_N10G)
+	},
+	{
+		.vendor_id = 0,
+	},
+};
+
+static struct rte_pci_driver rte_rnp_pmd = {
+	.id_table = pci_id_rnp_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+	.probe = rnp_pci_probe,
+	.remove = rnp_pci_remove,
+};
+
+RTE_PMD_REGISTER_PCI(net_rnp, rte_rnp_pmd);
+RTE_PMD_REGISTER_PCI_TABLE(net_rnp, pci_id_rnp_map);
+RTE_PMD_REGISTER_KMOD_DEP(net_rnp, "igb_uio | uio_pci_generic | vfio-pci");