DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function
@ 2020-05-06 12:43 David Marchand
  2020-05-06 12:43 ` [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols David Marchand
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: David Marchand @ 2020-05-06 12:43 UTC (permalink / raw)
  To: dev
  Cc: grive, stable, Marko Kovacevic, Ori Kam, Bruce Richardson,
	Radu Nicolau, Akhil Goyal, Tomasz Kantecki, Sunil Kumar Kori,
	Pavan Nikhilesh, John McNamara, Anatoly Burakov, Maxime Coquelin,
	Zhihong Wang, Xiaolong Ye, Gaetan Rivet

rte_pci_probe() is private to the PCI bus.
Clean the remaining references in the documentation and comments.

Fixes: c752998b5e2e ("pci: introduce library and driver")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 doc/guides/sample_app_ug/l2_forward_event.rst        |  8 --------
 doc/guides/sample_app_ug/l2_forward_real_virtual.rst |  9 ---------
 doc/guides/sample_app_ug/link_status_intr.rst        |  7 -------
 doc/guides/sample_app_ug/multi_process.rst           |  2 +-
 drivers/bus/pci/pci_common.c                         |  6 +++---
 drivers/bus/pci/private.h                            | 10 ----------
 drivers/net/virtio/virtio_user_ethdev.c              |  2 +-
 7 files changed, 5 insertions(+), 39 deletions(-)

diff --git a/doc/guides/sample_app_ug/l2_forward_event.rst b/doc/guides/sample_app_ug/l2_forward_event.rst
index 8bdf352c4e..d536eee819 100644
--- a/doc/guides/sample_app_ug/l2_forward_event.rst
+++ b/doc/guides/sample_app_ug/l2_forward_event.rst
@@ -204,9 +204,6 @@ chapters that related to the Poll Mode and Event mode Driver in the
 
 .. code-block:: c
 
-    if (rte_pci_probe() < 0)
-        rte_panic("Cannot probe PCI\n");
-
     /* reset l2fwd_dst_ports */
 
     for (portid = 0; portid < RTE_MAX_ETHPORTS; portid++)
@@ -236,11 +233,6 @@ chapters that related to the Poll Mode and Event mode Driver in the
         rte_eth_dev_info_get((uint8_t) portid, &dev_info);
     }
 
-Observe that:
-
-*   rte_pci_probe() parses the devices on the PCI bus and initializes recognized
-    devices.
-
 The next step is to configure the RX and TX queues. For each port, there is only
 one RX queue (only one lcore is able to poll a given port). The number of TX
 queues depends on the number of available lcores. The rte_eth_dev_configure()
diff --git a/doc/guides/sample_app_ug/l2_forward_real_virtual.rst b/doc/guides/sample_app_ug/l2_forward_real_virtual.rst
index 39d6b0067a..671d0c7c19 100644
--- a/doc/guides/sample_app_ug/l2_forward_real_virtual.rst
+++ b/doc/guides/sample_app_ug/l2_forward_real_virtual.rst
@@ -194,9 +194,6 @@ in the *DPDK Programmer's Guide* - Rel 1.4 EAR and the *DPDK API Reference*.
 
 .. code-block:: c
 
-    if (rte_pci_probe() < 0)
-        rte_exit(EXIT_FAILURE, "Cannot probe PCI\n");
-
     /* reset l2fwd_dst_ports */
 
     for (portid = 0; portid < RTE_MAX_ETHPORTS; portid++)
@@ -226,12 +223,6 @@ in the *DPDK Programmer's Guide* - Rel 1.4 EAR and the *DPDK API Reference*.
         rte_eth_dev_info_get((uint8_t) portid, &dev_info);
     }
 
-Observe that:
-
-*   rte_igb_pmd_init_all() simultaneously registers the driver as a PCI driver and as an Ethernet* Poll Mode Driver.
-
-*   rte_pci_probe() parses the devices on the PCI bus and initializes recognized devices.
-
 The next step is to configure the RX and TX queues.
 For each port, there is only one RX queue (only one lcore is able to poll a given port).
 The number of TX queues depends on the number of available lcores.
diff --git a/doc/guides/sample_app_ug/link_status_intr.rst b/doc/guides/sample_app_ug/link_status_intr.rst
index 5283be8b7c..04c40f2854 100644
--- a/doc/guides/sample_app_ug/link_status_intr.rst
+++ b/doc/guides/sample_app_ug/link_status_intr.rst
@@ -88,9 +88,6 @@ To fully understand this code, it is recommended to study the chapters that rela
 
 .. code-block:: c
 
-    if (rte_pci_probe() < 0)
-        rte_exit(EXIT_FAILURE, "Cannot probe PCI\n");
-
     /*
      * Each logical core is assigned a dedicated TX queue on each port.
      */
@@ -115,10 +112,6 @@ To fully understand this code, it is recommended to study the chapters that rela
         rte_eth_dev_info_get((uint8_t) portid, &dev_info);
     }
 
-Observe that:
-
-*   rte_pci_probe()  parses the devices on the PCI bus and initializes recognized devices.
-
 The next step is to configure the RX and TX queues.
 For each port, there is only one RX queue (only one lcore is able to poll a given port).
 The number of TX queues depends on the number of available lcores.
diff --git a/doc/guides/sample_app_ug/multi_process.rst b/doc/guides/sample_app_ug/multi_process.rst
index 9c374da6f7..f2a79a6397 100644
--- a/doc/guides/sample_app_ug/multi_process.rst
+++ b/doc/guides/sample_app_ug/multi_process.rst
@@ -209,7 +209,7 @@ How the Application Works
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 The initialization calls in both the primary and secondary instances are the same for the most part,
-calling the rte_eal_init(), 1 G and 10 G driver initialization and then rte_pci_probe() functions.
+calling the rte_eal_init(), 1 G and 10 G driver initialization and then probing devices.
 Thereafter, the initialization done depends on whether the process is configured as a primary or secondary instance.
 
 In the primary instance, a memory pool is created for the packet mbufs and the network ports to be used are initialized -
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 3f55420769..ab73c009ac 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -288,8 +288,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
  * all registered drivers that have a matching entry in its id_table
  * for discovered devices.
  */
-int
-rte_pci_probe(void)
+static int
+pci_probe(void)
 {
 	struct rte_pci_device *dev = NULL;
 	size_t probed = 0, failed = 0;
@@ -675,7 +675,7 @@ rte_pci_get_iommu_class(void)
 struct rte_pci_bus rte_pci_bus = {
 	.bus = {
 		.scan = rte_pci_scan,
-		.probe = rte_pci_probe,
+		.probe = pci_probe,
 		.find_device = pci_find_device,
 		.plug = pci_plug,
 		.unplug = pci_unplug,
diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
index a205d4d9f0..af1c7ae5fe 100644
--- a/drivers/bus/pci/private.h
+++ b/drivers/bus/pci/private.h
@@ -17,16 +17,6 @@ struct rte_pci_device;
 
 extern struct rte_pci_bus rte_pci_bus;
 
-/**
- * Probe the PCI bus
- *
- * @return
- *   - 0 on success.
- *   - !0 on error.
- */
-int
-rte_pci_probe(void);
-
 /**
  * Scan the content of the PCI bus, and the devices in the devices
  * list
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 953f00d72d..4743c11fd2 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -720,7 +720,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 		goto end;
 	}
 
-	/* previously called by rte_pci_probe() for physical dev */
+	/* previously called by pci probing for physical dev */
 	if (eth_virtio_dev_init(eth_dev) < 0) {
 		PMD_INIT_LOG(ERR, "eth_virtio_dev_init fails");
 		virtio_user_eth_dev_free(eth_dev);
-- 
2.23.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols
  2020-05-06 12:43 [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function David Marchand
@ 2020-05-06 12:43 ` David Marchand
  2020-05-06 17:21   ` Gaëtan Rivet
  2020-05-06 14:05 ` [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function Gaëtan Rivet
  2020-05-11 14:56 ` David Marchand
  2 siblings, 1 reply; 8+ messages in thread
From: David Marchand @ 2020-05-06 12:43 UTC (permalink / raw)
  To: dev; +Cc: grive, Gaetan Rivet

Internal symbols do not need the rte_ prefix.
Some symbols do not need to be exposed in the private header and have
been made static.

Fixes: c752998b5e2e ("pci: introduce library and driver")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/pci/bsd/pci.c    |  8 ++++----
 drivers/bus/pci/linux/pci.c  |  8 ++++----
 drivers/bus/pci/pci_common.c | 26 ++++++++++++------------
 drivers/bus/pci/pci_params.c |  5 ++---
 drivers/bus/pci/private.h    | 38 ++++++------------------------------
 5 files changed, 29 insertions(+), 56 deletions(-)

diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index ebbfeb13a8..ef6b953cd1 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -288,7 +288,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 
 	/* device is valid, add in list (sorted) */
 	if (TAILQ_EMPTY(&rte_pci_bus.device_list)) {
-		rte_pci_add_device(dev);
+		pci_add_device(dev);
 	}
 	else {
 		struct rte_pci_device *dev2 = NULL;
@@ -299,7 +299,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 			if (ret > 0)
 				continue;
 			else if (ret < 0) {
-				rte_pci_insert_device(dev2, dev);
+				pci_insert_device(dev2, dev);
 			} else { /* already registered */
 				dev2->kdrv = dev->kdrv;
 				dev2->max_vfs = dev->max_vfs;
@@ -311,7 +311,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 			}
 			return 0;
 		}
-		rte_pci_add_device(dev);
+		pci_add_device(dev);
 	}
 
 	return 0;
@@ -326,7 +326,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
  * list. Call pci_scan_one() for each pci entry found.
  */
 int
-rte_pci_scan(void)
+pci_scan(void)
 {
 	int fd;
 	unsigned dev_count = 0;
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index ca783b1575..4dc37c8cad 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -335,7 +335,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 
 	/* device is valid, add in list (sorted) */
 	if (TAILQ_EMPTY(&rte_pci_bus.device_list)) {
-		rte_pci_add_device(dev);
+		pci_add_device(dev);
 	} else {
 		struct rte_pci_device *dev2;
 		int ret;
@@ -346,7 +346,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 				continue;
 
 			if (ret < 0) {
-				rte_pci_insert_device(dev2, dev);
+				pci_insert_device(dev2, dev);
 			} else { /* already registered */
 				if (!rte_dev_is_probed(&dev2->device)) {
 					dev2->kdrv = dev->kdrv;
@@ -388,7 +388,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 			return 0;
 		}
 
-		rte_pci_add_device(dev);
+		pci_add_device(dev);
 	}
 
 	return 0;
@@ -457,7 +457,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
  * list
  */
 int
-rte_pci_scan(void)
+pci_scan(void)
 {
 	struct dirent *e;
 	DIR *dir;
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index ab73c009ac..e3729bdb9a 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -81,8 +81,8 @@ pci_name_set(struct rte_pci_device *dev)
 /*
  * Match the PCI Driver and Device using the ID Table
  */
-int
-rte_pci_match(const struct rte_pci_driver *pci_drv,
+static int
+pci_match(const struct rte_pci_driver *pci_drv,
 	      const struct rte_pci_device *pci_dev)
 {
 	const struct rte_pci_id *id_table;
@@ -132,7 +132,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 	loc = &dev->addr;
 
 	/* The device is not blacklisted; Check if driver supports it */
-	if (!rte_pci_match(dr, dev))
+	if (!pci_match(dr, dev))
 		/* Match of device and driver failed */
 		return 1;
 
@@ -388,14 +388,14 @@ rte_pci_unregister(struct rte_pci_driver *driver)
 
 /* Add a device to PCI bus */
 void
-rte_pci_add_device(struct rte_pci_device *pci_dev)
+pci_add_device(struct rte_pci_device *pci_dev)
 {
 	TAILQ_INSERT_TAIL(&rte_pci_bus.device_list, pci_dev, next);
 }
 
 /* Insert a device into a predefined position in PCI bus */
 void
-rte_pci_insert_device(struct rte_pci_device *exist_pci_dev,
+pci_insert_device(struct rte_pci_device *exist_pci_dev,
 		      struct rte_pci_device *new_pci_dev)
 {
 	TAILQ_INSERT_BEFORE(exist_pci_dev, new_pci_dev, next);
@@ -403,7 +403,7 @@ rte_pci_insert_device(struct rte_pci_device *exist_pci_dev,
 
 /* Remove a device from PCI bus */
 static void
-rte_pci_remove_device(struct rte_pci_device *pci_dev)
+pci_remove_device(struct rte_pci_device *pci_dev)
 {
 	TAILQ_REMOVE(&rte_pci_bus.device_list, pci_dev, next);
 }
@@ -536,7 +536,7 @@ pci_unplug(struct rte_device *dev)
 	pdev = RTE_DEV_TO_PCI(dev);
 	ret = rte_pci_detach_dev(pdev);
 	if (ret == 0) {
-		rte_pci_remove_device(pdev);
+		pci_remove_device(pdev);
 		rte_devargs_remove(dev->devargs);
 		free(pdev);
 	}
@@ -609,8 +609,8 @@ pci_ignore_device(const struct rte_pci_device *dev)
 	return true;
 }
 
-enum rte_iova_mode
-rte_pci_get_iommu_class(void)
+static enum rte_iova_mode
+pci_get_iommu_class(void)
 {
 	enum rte_iova_mode iova_mode = RTE_IOVA_DC;
 	const struct rte_pci_device *dev;
@@ -635,7 +635,7 @@ rte_pci_get_iommu_class(void)
 		FOREACH_DRIVER_ON_PCIBUS(drv) {
 			enum rte_iova_mode dev_iova_mode;
 
-			if (!rte_pci_match(drv, dev))
+			if (!pci_match(drv, dev))
 				continue;
 
 			dev_iova_mode = pci_device_iova_mode(drv, dev);
@@ -674,7 +674,7 @@ rte_pci_get_iommu_class(void)
 
 struct rte_pci_bus rte_pci_bus = {
 	.bus = {
-		.scan = rte_pci_scan,
+		.scan = pci_scan,
 		.probe = pci_probe,
 		.find_device = pci_find_device,
 		.plug = pci_plug,
@@ -682,8 +682,8 @@ struct rte_pci_bus rte_pci_bus = {
 		.parse = pci_parse,
 		.dma_map = pci_dma_map,
 		.dma_unmap = pci_dma_unmap,
-		.get_iommu_class = rte_pci_get_iommu_class,
-		.dev_iterate = rte_pci_dev_iterate,
+		.get_iommu_class = pci_get_iommu_class,
+		.dev_iterate = pci_dev_iterate,
 		.hot_unplug_handler = pci_hot_unplug_handler,
 		.sigbus_handler = pci_sigbus_handler,
 	},
diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c
index 3192e9c967..16c64c9e9f 100644
--- a/drivers/bus/pci/pci_params.c
+++ b/drivers/bus/pci/pci_params.c
@@ -55,9 +55,8 @@ pci_dev_match(const struct rte_device *dev,
 }
 
 void *
-rte_pci_dev_iterate(const void *start,
-		    const char *str,
-		    const struct rte_dev_iterator *it __rte_unused)
+pci_dev_iterate(const void *start, const char *str,
+		const struct rte_dev_iterator *it __rte_unused)
 {
 	rte_bus_find_device_t find_device;
 	struct rte_kvargs *kvargs = NULL;
diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
index af1c7ae5fe..6992443bf9 100644
--- a/drivers/bus/pci/private.h
+++ b/drivers/bus/pci/private.h
@@ -24,7 +24,7 @@ extern struct rte_pci_bus rte_pci_bus;
  * @return
  *  0 on success, negative on error
  */
-int rte_pci_scan(void);
+int pci_scan(void);
 
 /**
  * Find the name of a PCI device.
@@ -41,7 +41,7 @@ pci_name_set(struct rte_pci_device *dev);
  *	PCI device to add
  * @return void
  */
-void rte_pci_add_device(struct rte_pci_device *pci_dev);
+void pci_add_device(struct rte_pci_device *pci_dev);
 
 /**
  * Insert a PCI device in the PCI Bus at a particular location in the device
@@ -54,7 +54,7 @@ void rte_pci_add_device(struct rte_pci_device *pci_dev);
  *	PCI device to be added before exist_pci_dev
  * @return void
  */
-void rte_pci_insert_device(struct rte_pci_device *exist_pci_dev,
+void pci_insert_device(struct rte_pci_device *exist_pci_dev,
 		struct rte_pci_device *new_pci_dev);
 
 /**
@@ -147,23 +147,8 @@ pci_uio_remap_resource(struct rte_pci_device *dev);
 int pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 		struct mapped_pci_resource *uio_res, int map_idx);
 
-/*
- * Match the PCI Driver and Device using the ID Table
- *
- * @param pci_drv
- *      PCI driver from which ID table would be extracted
- * @param pci_dev
- *      PCI device to match against the driver
- * @return
- *      1 for successful match
- *      0 for unsuccessful match
- */
-int
-rte_pci_match(const struct rte_pci_driver *pci_drv,
-	      const struct rte_pci_device *pci_dev);
-
 /**
- * OS specific callbacks for rte_pci_get_iommu_class
+ * OS specific callbacks for pci_get_iommu_class
  *
  */
 bool
@@ -173,16 +158,6 @@ enum rte_iova_mode
 pci_device_iova_mode(const struct rte_pci_driver *pci_drv,
 		     const struct rte_pci_device *pci_dev);
 
-/**
- * Get iommu class of PCI devices on the bus.
- * And return their preferred iova mapping mode.
- *
- * @return
- *   - enum rte_iova_mode.
- */
-enum rte_iova_mode
-rte_pci_get_iommu_class(void);
-
 /*
  * Iterate over internal devices,
  * matching any device against the provided
@@ -202,8 +177,7 @@ rte_pci_get_iommu_class(void);
  *   NULL otherwise.
  */
 void *
-rte_pci_dev_iterate(const void *start,
-		    const char *str,
-		    const struct rte_dev_iterator *it);
+pci_dev_iterate(const void *start, const char *str,
+		const struct rte_dev_iterator *it);
 
 #endif /* _PCI_PRIVATE_H_ */
-- 
2.23.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function
  2020-05-06 12:43 [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function David Marchand
  2020-05-06 12:43 ` [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols David Marchand
@ 2020-05-06 14:05 ` Gaëtan Rivet
  2020-05-11 14:56 ` David Marchand
  2 siblings, 0 replies; 8+ messages in thread
From: Gaëtan Rivet @ 2020-05-06 14:05 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, stable, Marko Kovacevic, Ori Kam, Bruce Richardson,
	Radu Nicolau, Akhil Goyal, Tomasz Kantecki, Sunil Kumar Kori,
	Pavan Nikhilesh, John McNamara, Anatoly Burakov, Maxime Coquelin,
	Zhihong Wang, Xiaolong Ye, Gaetan Rivet

Hello David,

On 06/05/20 14:43 +0200, David Marchand wrote:
> rte_pci_probe() is private to the PCI bus.
> Clean the remaining references in the documentation and comments.
> 
> Fixes: c752998b5e2e ("pci: introduce library and driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  doc/guides/sample_app_ug/l2_forward_event.rst        |  8 --------
>  doc/guides/sample_app_ug/l2_forward_real_virtual.rst |  9 ---------
>  doc/guides/sample_app_ug/link_status_intr.rst        |  7 -------
>  doc/guides/sample_app_ug/multi_process.rst           |  2 +-
>  drivers/bus/pci/pci_common.c                         |  6 +++---
>  drivers/bus/pci/private.h                            | 10 ----------
>  drivers/net/virtio/virtio_user_ethdev.c              |  2 +-
>  7 files changed, 5 insertions(+), 39 deletions(-)
> 
> diff --git a/doc/guides/sample_app_ug/l2_forward_event.rst b/doc/guides/sample_app_ug/l2_forward_event.rst
> index 8bdf352c4e..d536eee819 100644
> --- a/doc/guides/sample_app_ug/l2_forward_event.rst
> +++ b/doc/guides/sample_app_ug/l2_forward_event.rst
> @@ -204,9 +204,6 @@ chapters that related to the Poll Mode and Event mode Driver in the
>  
>  .. code-block:: c
>  
> -    if (rte_pci_probe() < 0)
> -        rte_panic("Cannot probe PCI\n");
> -
>      /* reset l2fwd_dst_ports */
>  
>      for (portid = 0; portid < RTE_MAX_ETHPORTS; portid++)
> @@ -236,11 +233,6 @@ chapters that related to the Poll Mode and Event mode Driver in the
>          rte_eth_dev_info_get((uint8_t) portid, &dev_info);
>      }
>  
> -Observe that:
> -
> -*   rte_pci_probe() parses the devices on the PCI bus and initializes recognized
> -    devices.
> -

Reading the docs overall, what needed to be covered by referencing
rte_pci_probe() here should be covered by the reference to
rte_eal_init() earlier.

So simply removing rte_pci_probe() line in these docs seems ok.

LGTM,

Reviewed-by: Gaetan Rivet <grive@u256.net>


-- 
Gaëtan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols
  2020-05-06 12:43 ` [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols David Marchand
@ 2020-05-06 17:21   ` Gaëtan Rivet
  2020-05-06 20:25     ` Stephen Hemminger
  2020-05-07 12:41     ` David Marchand
  0 siblings, 2 replies; 8+ messages in thread
From: Gaëtan Rivet @ 2020-05-06 17:21 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On 06/05/20 14:43 +0200, David Marchand wrote:
> Internal symbols do not need the rte_ prefix.
> Some symbols do not need to be exposed in the private header and have
> been made static.
> 
> Fixes: c752998b5e2e ("pci: introduce library and driver")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

For this patch, I would like to understand why we are having this
policy. Symbols that are emitted for later linking will be present in
archives generated by the framework. Am I wrong to think they can
conflict with user app symbols?

If that is correct, we should use pci_* prefix for static symbols,
rte_* for everything else, even "internal" symbols -- in the sense
that they are meant to be opaque to the user, but will still be linked
in static build.

If I'm wrong in thinking this, then ok with this policy and let's go
forward to align naming in PCI bus.

-- 
Gaëtan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols
  2020-05-06 17:21   ` Gaëtan Rivet
@ 2020-05-06 20:25     ` Stephen Hemminger
  2020-05-07 12:43       ` David Marchand
  2020-05-07 12:41     ` David Marchand
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2020-05-06 20:25 UTC (permalink / raw)
  To: Gaëtan Rivet; +Cc: David Marchand, dev

On Wed, 6 May 2020 19:21:23 +0200
Gaëtan Rivet <grive@u256.net> wrote:

> On 06/05/20 14:43 +0200, David Marchand wrote:
> > Internal symbols do not need the rte_ prefix.
> > Some symbols do not need to be exposed in the private header and have
> > been made static.
> > 
> > Fixes: c752998b5e2e ("pci: introduce library and driver")
> > 
> > Signed-off-by: David Marchand <david.marchand@redhat.com>  
> 
> For this patch, I would like to understand why we are having this
> policy. Symbols that are emitted for later linking will be present in
> archives generated by the framework. Am I wrong to think they can
> conflict with user app symbols?
> 
> If that is correct, we should use pci_* prefix for static symbols,
> rte_* for everything else, even "internal" symbols -- in the sense
> that they are meant to be opaque to the user, but will still be linked
> in static build.
> 
> If I'm wrong in thinking this, then ok with this policy and let's go
> forward to align naming in PCI bus.
> 

Agree that all symbols need a prefix.
Any symbol that is not static is visible to the application if static linking.
There is some pre-linking magic can be done but DPDK isn't doing it.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols
  2020-05-06 17:21   ` Gaëtan Rivet
  2020-05-06 20:25     ` Stephen Hemminger
@ 2020-05-07 12:41     ` David Marchand
  1 sibling, 0 replies; 8+ messages in thread
From: David Marchand @ 2020-05-07 12:41 UTC (permalink / raw)
  To: Gaëtan Rivet; +Cc: dev

On Wed, May 6, 2020 at 7:21 PM Gaëtan Rivet <grive@u256.net> wrote:
>
> On 06/05/20 14:43 +0200, David Marchand wrote:
> > Internal symbols do not need the rte_ prefix.
> > Some symbols do not need to be exposed in the private header and have
> > been made static.
> >
> > Fixes: c752998b5e2e ("pci: introduce library and driver")
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> For this patch, I would like to understand why we are having this
> policy. Symbols that are emitted for later linking will be present in
> archives generated by the framework. Am I wrong to think they can
> conflict with user app symbols?
>
> If that is correct, we should use pci_* prefix for static symbols,
> rte_* for everything else, even "internal" symbols -- in the sense
> that they are meant to be opaque to the user, but will still be linked
> in static build.
>
> If I'm wrong in thinking this, then ok with this policy and let's go
> forward to align naming in PCI bus.

I see your point.
This is a pain to read code which mixes internal and public functions
with the rte_ prefix.
I have no answer atm, I am ok with dropping this patch.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols
  2020-05-06 20:25     ` Stephen Hemminger
@ 2020-05-07 12:43       ` David Marchand
  0 siblings, 0 replies; 8+ messages in thread
From: David Marchand @ 2020-05-07 12:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Gaëtan Rivet, dev

On Wed, May 6, 2020 at 10:25 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
> > If that is correct, we should use pci_* prefix for static symbols,
> > rte_* for everything else, even "internal" symbols -- in the sense
> > that they are meant to be opaque to the user, but will still be linked
> > in static build.
> >
> > If I'm wrong in thinking this, then ok with this policy and let's go
> > forward to align naming in PCI bus.
> >
>
> Agree that all symbols need a prefix.
> Any symbol that is not static is visible to the application if static linking.
> There is some pre-linking magic can be done but DPDK isn't doing it.

Like automatically prefixing symbols?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function
  2020-05-06 12:43 [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function David Marchand
  2020-05-06 12:43 ` [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols David Marchand
  2020-05-06 14:05 ` [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function Gaëtan Rivet
@ 2020-05-11 14:56 ` David Marchand
  2 siblings, 0 replies; 8+ messages in thread
From: David Marchand @ 2020-05-11 14:56 UTC (permalink / raw)
  To: dev
  Cc: Gaetan Rivet, dpdk stable, Marko Kovacevic, Ori Kam,
	Bruce Richardson, Radu Nicolau, Akhil Goyal, Tomasz Kantecki,
	Sunil Kumar Kori, Pavan Nikhilesh, John McNamara,
	Anatoly Burakov, Maxime Coquelin, Zhihong Wang, Xiaolong Ye,
	Gaetan Rivet

On Wed, May 6, 2020 at 2:43 PM David Marchand <david.marchand@redhat.com> wrote:
>
> rte_pci_probe() is private to the PCI bus.
> Clean the remaining references in the documentation and comments.
>
> Fixes: c752998b5e2e ("pci: introduce library and driver")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Reviewed-by: Gaetan Rivet <grive@u256.net>

Applied.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-05-11 14:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 12:43 [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function David Marchand
2020-05-06 12:43 ` [dpdk-dev] [PATCH 2/2] bus/pci: cleanup private symbols David Marchand
2020-05-06 17:21   ` Gaëtan Rivet
2020-05-06 20:25     ` Stephen Hemminger
2020-05-07 12:43       ` David Marchand
2020-05-07 12:41     ` David Marchand
2020-05-06 14:05 ` [dpdk-dev] [PATCH 1/2] remove references to private PCI probe function Gaëtan Rivet
2020-05-11 14:56 ` David Marchand

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).