Re: [dpdk-dev] [PATCH v2 15/15] eal: Enable Port Hotplug as default in Linux and BSD

[Tetsuya Mukawa <mukawa@igel.co.jp>]

On 2015/03/14 0:47, Iremonger, Bernard wrote:
>
>> -----Original Message-----
>> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
>> Sent: Thursday, March 12, 2015 10:18 AM
>> To: dev@dpdk.org
>> Cc: Iremonger, Bernard; Richardson, Bruce; Tetsuya Mukawa
>> Subject: [PATCH v2 15/15] eal: Enable Port Hotplug as default in Linux and BSD
>>
>> This patch removes CONFIG_RTE_LIBRTE_EAL_HOTPLUG option, and enables it as default in both
>> Linux and BSD.
> Hi Tetsuya,
>
> This patch should probably be split with the config changes as the first patch of this patch set.
> The #ifdef RTE_LIBRTE_EAL_HOTPLUG changes should be no longer necessary as they will have been taken care of in the earlier patches (v2-11, v2-12, v2-13).
> Maybe the  /lib/librte_eal/bsdapp/eal/rte_eal_version.map changes could be moved to one of the earlier bsd patches?

Hi Bernard,

Could I make it sure your suggestion?
Did you mean that hotplug support for linuxapp and bsdapp should be
enabled at first of consolidation patches?

It seems I can remove "#ifdef RTE_LIBRTE_EAL_HOTPLUG" together with
below patches.
 - v2-11
 - v2-12
 - v2-13
(Probably this will clean up the last patch a bit.)

But I cannot enable hotplug support at the first of consolidation
patches, because bsdapp doesn't have some hotplug implementation at the
first.
(Actually, functions related with detach() are only exist in linuxapp at
the first.)
All related functions are moved to common while patch set, then bsdapp
can support hotplug.

I will clean up the last patch as much as possible, then submit again.
Is it okay for this patch?

Regards,
Tetsuya

> Regards,
>
> Bernard.
>
>
>> Also, to support port hotplug, rte_eal_pci_scan() and below missing symbols should be exported to
>> ethdev library.
>>  - rte_eal_parse_devargs_str()
>>  - rte_eal_pci_close_one()
>>  - rte_eal_pci_probe_one()
>>  - rte_eal_pci_scan()
>>  - rte_eal_vdev_init()
>>  - rte_eal_vdev_uninit()
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>>  config/common_bsdapp                          |  6 ------
>>  config/common_linuxapp                        |  5 -----
>>  lib/librte_eal/bsdapp/eal/eal_pci.c           |  6 +++---
>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map |  6 ++++++
>>  lib/librte_eal/common/eal_common_dev.c        |  2 --
>>  lib/librte_eal/common/eal_common_pci.c        |  8 --------
>>  lib/librte_eal/common/eal_common_pci_uio.c    |  2 --
>>  lib/librte_eal/common/include/rte_pci.h       |  2 --
>>  lib/librte_ether/rte_ethdev.c                 | 21 ---------------------
>>  9 files changed, 9 insertions(+), 49 deletions(-)
>>
>> diff --git a/config/common_bsdapp b/config/common_bsdapp index 8ff4dc2..88b44e9 100644
>> --- a/config/common_bsdapp
>> +++ b/config/common_bsdapp
>> @@ -116,12 +116,6 @@ CONFIG_RTE_LIBRTE_EAL_BSDAPP=y
>> CONFIG_RTE_LIBRTE_EAL_LINUXAPP=n
>>
>>  #
>> -# Compile Environment Abstraction Layer to support hotplug -# So far, Hotplug functions only support
>> linux -# -CONFIG_RTE_LIBRTE_EAL_HOTPLUG=n
>> -
>> -#
>>  # Compile Environment Abstraction Layer to support Vmware TSC map  #
>> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
>> diff --git a/config/common_linuxapp b/config/common_linuxapp index 97f1c9e..f9c9780 100644
>> --- a/config/common_linuxapp
>> +++ b/config/common_linuxapp
>> @@ -114,11 +114,6 @@ CONFIG_RTE_PCI_MAX_READ_REQUEST_SIZE=0
>>  CONFIG_RTE_LIBRTE_EAL_LINUXAPP=y
>>
>>  #
>> -# Compile Environment Abstraction Layer to support hotplug -# -
>> CONFIG_RTE_LIBRTE_EAL_HOTPLUG=y
>> -
>> -#
>>  # Compile Environment Abstraction Layer to support Vmware TSC map  #
>> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> index b6785d4..50c9544 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> @@ -305,8 +305,8 @@ skipdev:
>>   * Scan the content of the PCI bus, and add the devices in the devices
>>   * list. Call pci_scan_one() for each pci entry found.
>>   */
>> -static int
>> -pci_scan(void)
>> +int
>> +rte_eal_pci_scan(void)
>>  {
>>  	int fd;
>>  	unsigned dev_count = 0;
>> @@ -362,7 +362,7 @@ rte_eal_pci_init(void)
>>  	if (internal_config.no_pci)
>>  		return 0;
>>
>> -	if (pci_scan() < 0) {
>> +	if (rte_eal_pci_scan() < 0) {
>>  		RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
>>  		return -1;
>>  	}
>> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> index 67b6a6c..7e850a9 100644
>> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> @@ -37,14 +37,20 @@ DPDK_2.0 {
>>  	rte_eal_lcore_role;
>>  	rte_eal_mp_remote_launch;
>>  	rte_eal_mp_wait_lcore;
>> +	rte_eal_parse_devargs_str;
>> +	rte_eal_pci_close_one;
>>  	rte_eal_pci_dump;
>>  	rte_eal_pci_probe;
>> +	rte_eal_pci_probe_one;
>>  	rte_eal_pci_register;
>> +	rte_eal_pci_scan;
>>  	rte_eal_pci_unregister;
>>  	rte_eal_process_type;
>>  	rte_eal_remote_launch;
>>  	rte_eal_tailq_lookup;
>>  	rte_eal_tailq_register;
>> +	rte_eal_vdev_init;
>> +	rte_eal_vdev_uninit;
>>  	rte_eal_wait_lcore;
>>  	rte_exit;
>>  	rte_get_hpet_cycles;
>> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
>> index 92a5a94..4089d66 100644
>> --- a/lib/librte_eal/common/eal_common_dev.c
>> +++ b/lib/librte_eal/common/eal_common_dev.c
>> @@ -125,7 +125,6 @@ rte_eal_dev_init(void)
>>  	return 0;
>>  }
>>
>> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>>  int
>>  rte_eal_vdev_uninit(const char *name)
>>  {
>> @@ -151,4 +150,3 @@ rte_eal_vdev_uninit(const char *name)
>>  	RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
>>  	return -EINVAL;
>>  }
>> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
>> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
>> index 6d98194..126dd01 100644
>> --- a/lib/librte_eal/common/eal_common_pci.c
>> +++ b/lib/librte_eal/common/eal_common_pci.c
>> @@ -122,7 +122,6 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
>>  	return mapaddr;
>>  }
>>
>> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>>  /* unmap a particular resource */
>>  void
>>  pci_unmap_resource(void *requested_addr, size_t size) @@ -139,7 +138,6 @@
>> pci_unmap_resource(void *requested_addr, size_t size)
>>  		RTE_LOG(DEBUG, EAL, "  PCI memory unmapped at %p\n",
>>  				requested_addr);
>>  }
>> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
>>
>>  /* Map pci device */
>>  static int
>> @@ -171,7 +169,6 @@ pci_map_device(struct rte_pci_device *dev)
>>  	return ret;
>>  }
>>
>> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>>  /* Unmap pci device */
>>  static void
>>  pci_unmap_device(struct rte_pci_device *dev) @@ -196,7 +193,6 @@ pci_unmap_device(struct
>> rte_pci_device *dev)
>>  		break;
>>  	}
>>  }
>> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
>>
>>  /*
>>   * If vendor/device ID match, call the devinit() function of the @@ -275,7 +271,6 @@
>> rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
>>  	return 1;
>>  }
>>
>> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>>  /*
>>   * If vendor/device ID match, call the devuninit() function of the
>>   * driver.
>> @@ -334,7 +329,6 @@ rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
>>  	/* return positive value if driver is not found */
>>  	return 1;
>>  }
>> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
>>
>>  /*
>>   * If vendor/device ID match, call the devinit() function of all @@ -363,7 +357,6 @@
>> pci_probe_all_drivers(struct rte_pci_device *dev)
>>  	return 1;
>>  }
>>
>> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>>  /*
>>   * If vendor/device ID match, call the devuninit() function of all
>>   * registered driver for the given device. Return -1 if initialization @@ -454,7 +447,6 @@ err_return:
>>  			dev->addr.devid, dev->addr.function);
>>  	return -1;
>>  }
>> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
>>
>>  /*
>>   * Scan the content of the PCI bus, and call the devinit() function for diff --git
>> a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
>> index b9d7726..fe5dd5f 100644
>> --- a/lib/librte_eal/common/eal_common_pci_uio.c
>> +++ b/lib/librte_eal/common/eal_common_pci_uio.c
>> @@ -154,7 +154,6 @@ fail:
>>  	return -1;
>>  }
>>
>> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>>  static void
>>  pci_uio_unmap(struct mapped_pci_resource *uio_res)  { @@ -223,4 +222,3 @@
>> pci_uio_unmap_resource(struct rte_pci_device *dev)
>>  	dev->intr_handle.fd = -1;
>>  	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;  } -#endif /*
>> RTE_LIBRTE_EAL_HOTPLUG */ diff --git a/lib/librte_eal/common/include/rte_pci.h
>> b/lib/librte_eal/common/include/rte_pci.h
>> index d3b883e..b3c6a0f 100644
>> --- a/lib/librte_eal/common/include/rte_pci.h
>> +++ b/lib/librte_eal/common/include/rte_pci.h
>> @@ -369,7 +369,6 @@ int rte_eal_pci_probe(void);  void *pci_map_resource(void *requested_addr,
>> int fd, off_t offset,
>>  		size_t size, int additional_flags);
>>
>> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>>  /**
>>   * Map pci resouce.
>>   */
>> @@ -404,7 +403,6 @@ int rte_eal_pci_probe_one(struct rte_pci_addr *addr);
>>   *   - Negative on error.
>>   */
>>  int rte_eal_pci_close_one(struct rte_pci_addr *addr); -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
>>
>>  /**
>>   * Dump the content of the PCI bus.
>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index e23d7da..818384a
>> 100644
>> --- a/lib/librte_ether/rte_ethdev.c
>> +++ b/lib/librte_ether/rte_ethdev.c
>> @@ -425,9 +425,6 @@ rte_eth_dev_count(void)
>>  	return (nb_ports);
>>  }
>>
>> -/* So far, DPDK hotplug function only supports linux */ -#ifdef RTE_LIBRTE_EAL_HOTPLUG
>> -
>>  static enum rte_eth_dev_type
>>  rte_eth_dev_get_device_type(uint8_t port_id)  { @@ -708,24 +705,6 @@
>> rte_eth_dev_detach(uint8_t port_id, char *name)
>>  	} else
>>  		return rte_eth_dev_detach_vdev(port_id, name);  } -#else /*
>> RTE_LIBRTE_EAL_HOTPLUG */ -int -rte_eth_dev_attach(const char *devargs __rte_unused,
>> -			uint8_t *port_id __rte_unused)
>> -{
>> -	RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n");
>> -	return -1;
>> -}
>> -
>> -/* detach the device, then store the name of the device */ -int -rte_eth_dev_detach(uint8_t port_id
>> __rte_unused,
>> -			char *name __rte_unused)
>> -{
>> -	RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n");
>> -	return -1;
>> -}
>> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
>>
>>  static int
>>  rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
>> --
>> 1.9.1