Re: [dpdk-dev] [PATCH V4 1/3] ethdev: introduce FEC API
On Thu, Sep 10, 2020 at 12:26 AM Min Hu (Connor) wrote:
> This patch adds Forward error correction(FEC) support for ethdev.
> Introduce APIs which support query and config FEC information in
> hardware.
>
> Signed-off-by: Min Hu (Connor)
> Reviewed-by: Wei Hu (Xavier)
> Reviewed-by: Chengwen Feng
> Reviewed-by: Chengchang Tang
>
Reviewed-by: Ajit Khaparde
>
> ---
> v2->v3:
> add function return value "-ENOTSUP" for API
> ---
> lib/librte_ethdev/rte_ethdev.c | 50 +
> lib/librte_ethdev/rte_ethdev.h | 75
> +++
> lib/librte_ethdev/rte_ethdev_core.h | 77
>
> lib/librte_ethdev/rte_ethdev_version.map | 5 +++
> 4 files changed, 207 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c
> b/lib/librte_ethdev/rte_ethdev.c
> index 7858ad5..b7ac791 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -3642,6 +3642,56 @@ rte_eth_led_off(uint16_t port_id)
> return eth_err(port_id, (*dev->dev_ops->dev_led_off)(dev));
> }
>
> +int
> +rte_eth_fec_get_capability(uint16_t port_id, uint8_t *fec_cap)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get_capability,
> -ENOTSUP);
> + return eth_err(port_id, (*dev->dev_ops->fec_get_capability)(dev,
> + fec_cap));
> +}
> +
> +int
> +rte_eth_fec_get(uint16_t port_id, enum rte_fec_mode *mode)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get, -ENOTSUP);
> + return eth_err(port_id, (*dev->dev_ops->fec_get)(dev, mode));
> +}
> +
> +int
> +rte_eth_fec_set(uint16_t port_id, enum rte_fec_mode mode)
> +{
> + struct rte_eth_dev *dev;
> + uint8_t fec_mode_mask;
> + int ret;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> + ret = rte_eth_fec_get_capability(port_id, &fec_mode_mask);
> + if (ret)
> + return ret;
> +
> + /*
> +* Check whether the configured mode is within the FEC capability.
> +* If not, the configured mode will not be supported.
> +*/
> + if (!(fec_mode_mask & (1U << (uint8_t)mode))) {
> + RTE_ETHDEV_LOG(ERR, "unsupported fec mode=%d\n", mode);
> + return -EINVAL;
> + }
> +
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_set, -ENOTSUP);
> + return eth_err(port_id, (*dev->dev_ops->fec_set)(dev, mode));
> +}
> +
> /*
> * Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to
> find
> * an empty spot.
> diff --git a/lib/librte_ethdev/rte_ethdev.h
> b/lib/librte_ethdev/rte_ethdev.h
> index 70295d7..c353480 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1511,6 +1511,17 @@ struct rte_eth_dcb_info {
> struct rte_eth_dcb_tc_queue_mapping tc_queue;
> };
>
> +/**
> + * This enum indicates the possible (forward error correction)FEC modes
> + * of an ethdev port.
> + */
> +enum rte_fec_mode {
> + ETH_FEC_NOFEC = 0, /**< FEC is off */
> + ETH_FEC_BASER, /**< FEC using common algorithm */
> + ETH_FEC_RS, /**< FEC using RS algorithm */
> + ETH_FEC_AUTO/**< FEC autonegotiation modes */
> +};
> +
> #define RTE_ETH_ALL RTE_MAX_ETHPORTS
>
> /* Macros to check for valid port */
> @@ -3328,6 +3339,70 @@ int rte_eth_led_on(uint16_t port_id);
> int rte_eth_led_off(uint16_t port_id);
>
> /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Get Forward Error Correction(FEC) capability.
> + *
> + * @param port_id
> + * The port identifier of the Ethernet device.
> + * @param fec_cap
> + * returns the FEC capability from the device, as follows:
> + * bit0: nofec
> + * bit1: baser
> + * bit2: rs
> + * bit3: auto
> + * @return
> + * - (0) if successful.
> + * - (-ENOTSUP) if underlying hardware OR driver doesn't support.
> + * that operation.
> + * - (-EIO) if device is removed.
> + * - (-ENODEV) if *port_id* invalid.
> + */
> +__rte_experimental
> +int rte_eth_fec_get_capability(uint16_t port_id, uint8_t *fec_cap);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Get current Forward Error Correction(FEC) mode.
> + *
> + * @param port_id
> + * The port identifier of the Ethernet device.
> + * @param mode
> + * returns the FEC mode from the device.
> + * @return
> + * - (0) if successful.
> + * - (-ENOTSUP) if underlying hardware OR driver doesn't support.
> + * that oper
Re: [dpdk-dev] [PATCH v2] kernel: remove igb_uio
11/09/2020 18:22, Ferruh Yigit: > On 9/11/2020 4:54 PM, Thomas Monjalon wrote: > > As decided in the Technical Board in November 2019, > > the kernel module igb_uio is moved to the dpdk-kmods repository > > in the /linux/igb_uio/ directory. > > > > Minutes of Technical Board meeting: > > https://mails.dpdk.org/archives/dev/2019-November/151763.html > > > > Signed-off-by: Thomas Monjalon > > --- > > v2: update few docs (including release notes) > > <...> > > > @@ -292,7 +292,6 @@ F: doc/guides/linux_gsg/ > > > > Linux UIO > > M: Ferruh Yigit > > -F: kernel/linux/igb_uio/ > > F: drivers/bus/pci/linux/*uio* > > > > We are loosing maintainer information for the kernel module, same concern for > other kernel modules in the 'dpdk-kmods' repo. There is not even contribution guidelines, mailing-list etc. > One option is 'dpdk-kmods' repo can have it's maintainers file, but I believe > better to have single maintainer file, and cover the maintainer and location > information in this file. I think the contribution infos (including maintainers contact) should be inside the repo dpdk-kmods. > <...> > > +which can be found in the repository ``dpdk-kmods``. > > There is no information where 'dpdk-kmods' repo is. It can be good to have it > as > link. The real URL would be the git ones: git://dpdk.org/dpdk-kmods http://dpdk.org/git/dpdk-kmods but the most convenient (for clicking) should be the cgit one: http://git.dpdk.org/dpdk-kmods/ or directly the sub-directory in cgit: http://git.dpdk.org/dpdk-kmods/tree/linux/igb_uio Note that this last one does not show the git URLs for cloning. Which one do you prefer?
Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port
03/08/2020 20:50, Thomas Monjalon: > 18/04/2019 12:59, Thomas Monjalon: > > Hi all, > > > > Since DPDK 18.11, the behaviour of the close operation is changed > > if RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver: > > port is released (i.e. totally freed and data erased) on close. > > This new behaviour is enabled per driver for a migration period. > > > > Looking at the code, you can see these comments: > > /* old behaviour: only free queue arrays */ > > RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n" > > "The driver %s should migrate to the new behaviour.\n", > > /* new behaviour: send event + reset state + free all data */ > > > > You can find an advice in the commit: > > http://git.dpdk.org/dpdk/commit/?id=23ea57a2a > > " > > When enabling RTE_ETH_DEV_CLOSE_REMOVE, > > the PMD must free all its private resources for the port, > > in its dev_close function. > > It is advised to call the dev_close function in the remove function > > in order to support removing a device without closing its ports. > > " > > > > It would be great to complete this migration for the next LTS > > version, which will be 19.11. > > For the record, it did not happen in 19.11. > > > Following drivers should be migrated: > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep > > -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > [...] > > The progress in April 2019 was 4 of 46 (9%). > > > Please let's progress smoothly on this topic, thanks. > > More than one year later, the progress is 26 of 53 (49%). > > > The concerned maintainers (Cc) can be found with the following command: > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 > > -type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | > > sort | uniq -u) > > We cannot wait forever. Temporary cannot be longer than 2 years. > I am going to send a deprecation notice to remove the "temporary" > flag RTE_ETH_DEV_CLOSE_REMOVE. The deprecation notice was merged in 20.08: http://mails.dpdk.org/archives/dev/2020-August/177314.html > It will break drivers which are not migrated. > It will probably help to find motivation in new priorities. > > More details on what to do can be found in this mail thread: > http://inbox.dpdk.org/dev/1748144.UFpUr2FPnr@xps/ Summary: * The freeing of private port resources must be moved in the PMD from the ".remove(device)" function to the ".dev_close(port)" function. * If a generic resource (.mac_addrs or .hash_mac_addrs) cannot be freed, it must be set to NULL in ".dev_close" PMD function to protect from subsequent rte_eth_dev_release_port() freeing. * Note 1: The generic resources are freed in rte_eth_dev_release_port(), after ".dev_close" is called in rte_eth_dev_close(), but not when calling ".dev_close" directly from the ".remove" PMD function. That's why rte_eth_dev_release_port() must still be called explicitly from ".remove(device)" after calling the ".dev_close" PMD function. * Note 2: If a device can have multiple ports, the common resources must be freed only in the ".remove(device)" function. * Note 3: The port is supposed to be in a stopped state when it is closed. If it is not the case, it is free to the PMD implementation how to react when trying to close a non-stopped port: either try to stop it automatically or just return an error.
Re: [dpdk-dev] [PATCH 1/2] config: add Graviton2(arm64) meson configuration
On 9/11/20 8:23 PM, Honnappa Nagarahalli wrote: > > +Jerin, Hemant, Dharmik > > > Hi Vimal, > Few comments inline. > >> >> Add meson build configuration for Graviton2 platform with 64-bit ARM >> Neoverse N1 cores. This patch makes the following changes to generic >> Neoverse N1 config: >> >> 1. increase lcore limit to 64 >> 2. increase memory support to 1TB > There will be multiple SoCs with N1 cores. All of them will have the same > implementor ID and part number. But, they will have different values for > these configurable parameters. > IMO, from usage perspective, we have 2 cases: > 1) Ability to build a portable binary that can run on multiple Arm SoCs (for > ex: BlueField, thunderx1, thunderx2, N1SDP, Graviton2 etc) > 2) Ability to build a binary which would run only on a SoC it was compiled > for and provide the most optimized binary for that SoC. But, this may not be > portable. > > For 1) we have default march. > > For 2) we do not have the capability today in meson build (at least, this is > my understanding, please correct me if I am wrong). In this case, the user > knows the target platform for compilation. IMO, we should add the capability > to take the target platform as an input from the user (similar to the make > build system) and Graviton2 can be one such target platform. My intention was to have parameters that work for both N1SDP and Graviton2 rather than 2). Does the change to RTE_MAX_LCORE and RTE_MAX_MEM_MB make them incompatible with N1SDP? I'm not sure if taking target platform from user is the best option here. Would this be specific to N1 since other platforms like thunderx differentiate the flags with part number? > >> 3. remove +crc from -march as that is default when setting armv8.2 >> >> For more information about Graviton2 platform, refer to: >> https://aws.amazon.com/ec2/graviton/ >> >> Signed-off-by: Vimal Chungath >> --- >> config/arm/arm64_graviton2_linux_gcc | 17 + >> config/arm/meson.build | 12 +++- >> 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 >> config/arm/arm64_graviton2_linux_gcc >> >> diff --git a/config/arm/arm64_graviton2_linux_gcc >> b/config/arm/arm64_graviton2_linux_gcc >> new file mode 100644 >> index 0..022e06303 >> --- /dev/null >> +++ b/config/arm/arm64_graviton2_linux_gcc >> @@ -0,0 +1,17 @@ >> +[binaries] >> +c = 'aarch64-linux-gnu-gcc' >> +cpp = 'aarch64-linux-gnu-cpp' >> +ar = 'aarch64-linux-gnu-gcc-ar' >> +strip = 'aarch64-linux-gnu-strip' >> +pkgconfig = 'aarch64-linux-gnu-pkg-config' >> +pcap-config = '' >> + >> +[host_machine] >> +system = 'linux' >> +cpu_family = 'aarch64' >> +cpu = 'armv8-a' >> +endian = 'little' >> + >> +[properties] >> +implementor_id = '0x41' >> +implementor_pn = '0xd0c' >> diff --git a/config/arm/meson.build b/config/arm/meson.build index >> 8728051d5..64e277ebc 100644 >> --- a/config/arm/meson.build >> +++ b/config/arm/meson.build >> @@ -86,6 +86,16 @@ flags_octeontx2_extra = [ >> ['RTE_ARM_FEATURE_ATOMICS', true], >> ['RTE_EAL_IGB_UIO', false], >> ['RTE_USE_C11_MEM_MODEL', true]] >> +flags_n1generic_extra = [ >> + ['RTE_MACHINE', '"neoverse-n1"'], >> + ['RTE_MAX_LCORE', 64], >> + ['RTE_CACHE_LINE_SIZE', 64], >> + ['RTE_ARM_FEATURE_ATOMICS', true], >> + ['RTE_USE_C11_MEM_MODEL', true], >> + ['RTE_MAX_MEM_MB', 1048576], >> + ['RTE_MAX_NUMA_NODES', 1], >> + ['RTE_EAL_NUMA_AWARE_HUGEPAGES', false], >> + ['RTE_LIBRTE_VHOST_NUMA', false]] >> >> machine_args_generic = [ >> ['default', ['-march=armv8-a+crc']], >> @@ -97,7 +107,7 @@ machine_args_generic = [ >> ['0xd09', ['-mcpu=cortex-a73']], >> ['0xd0a', ['-mcpu=cortex-a75']], >> ['0xd0b', ['-mcpu=cortex-a76']], >> - ['0xd0c', ['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'], >> flags_n1sdp_extra]] >> + ['0xd0c', ['-march=armv8.2-a+crypto', '-mcpu=neoverse-n1'], >> +flags_n1generic_extra]] >> >> machine_args_cavium = [ >> ['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']], >> -- >> 2.16.6 >
Re: [dpdk-dev] [PATCH 0/2] build: add Graviton2(arm64) config
On 9/9/20 1:03 AM, David Marchand wrote: > > Hello Vimal, > > On Wed, Sep 9, 2020 at 3:11 AM Vimal Chungath wrote: >> >> These two patches add meson and make build configuration >> for AWS Graviton2 platform. I plan to send platform guide >> patch separately. >> >> Vimal Chungath (2): >> config: add Graviton2(arm64) meson configuration >> config: add Graviton2(arm64) defconfig > > I did not look at the patches, just looked at the cover letter. > You can drop the makefile config files since make support has been dropped. > Thanks. Thanks. I will remove make config files in next version.
Re: [dpdk-dev] [PATCH] EAL: Called remove() of drivers for vdev and pci buses
On 08/07/20 17:03 +0500, Muhammad Bilal wrote:
> while using memif with app, the resources are not cleaned on exit,
> So an error occurred on running it second time. The cause of this problem
> is that remove() of memif driver is not called by rte_eal_cleanup() which
> is counterpart of probe() called from rte_eal_init(). This is a case for
> all other divers e.g pci, so to solve this problem I have added the
> functionality of calling remove() function of all the driver attached to
> devices on vdev and pci buses.
>
Hi Muhammad,
review inline.
> Bugzilla ID: 353
> Signed-off-by: Muhammad Bilal
> ---
> drivers/bus/pci/pci_common.c | 23 +++
> drivers/bus/vdev/vdev.c| 19 ++-
> lib/librte_eal/common/eal_common_bus.c | 18 ++
> lib/librte_eal/include/rte_bus.h | 23 +++
> lib/librte_eal/linux/eal.c | 2 ++
> 5 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 245d94f59..203b32a23 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -320,6 +320,28 @@ pci_probe(void)
> return (probed && probed == failed) ? -1 : 0;
> }
>
> +/*
> + * Scan the content of the PCI bus, and call the remove() function for
> + * all registered drivers of devices that have already been probed.
> + */
You are not scanning the content of the PCI bus here.
The PCI bus referred in this comment is the actual, physical one.
It is only scanned during rte_pci_scan.
You are iterating registered PCI devices and removing them.
> +static int
> +pci_remove(void)
> +{
> + struct rte_pci_device *dev = NULL;
> + int ret = 0;
> +
> + FOREACH_DEVICE_ON_PCIBUS(dev) {
> + if (rte_dev_is_probed(&dev->device))
> + if (rte_pci_detach_dev(dev) != 0) {
> + RTE_LOG(INFO, EAL,
> + "failed to detach driver form %s\n",
s/form/from/
> + dev->device.name);
> + ret = -1;
> + }
Memory is leaked from not freeing the rte_pci_device here.
The device iterator must be made safe (using TAILQ_FOREACH_SAFE)
and the dev pointer removed from the list and freed.
There might be other resources still open.
> + }
> + return ret;
> +}
> +
> /* dump one device */
> static int
> pci_dump_one_device(FILE *f, struct rte_pci_device *dev)
> @@ -669,6 +691,7 @@ struct rte_pci_bus rte_pci_bus = {
> .bus = {
> .scan = rte_pci_scan,
> .probe = pci_probe,
> + .remove = pci_remove,
> .find_device = pci_find_device,
> .plug = pci_plug,
> .unplug = pci_unplug,
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index a89ea2353..692c936b9 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -546,6 +546,22 @@ vdev_unplug(struct rte_device *dev)
> return rte_vdev_uninit(dev->name);
> }
>
> +static int
> +vdev_remove(void)
> +{
> + struct rte_vdev_device *dev;
> + int ret = 0;
> +
> + TAILQ_FOREACH(dev, &vdev_device_list, next) {
> + if (vdev_remove_driver(dev) != 0) {
> + VDEV_LOG(INFO, "driver of %s is not removed\n",
> + dev->device.name);
> + ret = -1;
> + }
> + }
Same as PCI bus, it should be made safe and the dev pointer removed
from the TAILQ and freed as well.
> + return ret;
> +}
> +
> static struct rte_bus rte_vdev_bus = {
> .scan = vdev_scan,
> .probe = vdev_probe,
> @@ -554,7 +570,8 @@ static struct rte_bus rte_vdev_bus = {
> .unplug = vdev_unplug,
> .parse = vdev_parse,
> .dev_iterate = rte_vdev_dev_iterate,
> -};
> + .remove = vdev_remove,
> + };
>
> RTE_REGISTER_BUS(vdev, rte_vdev_bus);
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c
> b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..bff95a0ae 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -85,6 +85,24 @@ rte_bus_probe(void)
> return 0;
> }
>
> +/* Remove all devices of all buses */
> +int
> +rte_bus_remove(void)
> +{
> + int ret;
> + struct rte_bus *bus;
> +
> + TAILQ_FOREACH(bus, &rte_bus_list, next) {
> + if (!strcmp(bus->name, "vdev") || !strcmp(bus->name, "pci")) {
You should only check that the bus sets the ops, not how the bus is named.
> + ret = bus->remove();
> + if (ret)
> + RTE_LOG(INFO, EAL, "Bus (%s) remove failed.\n",
> + bus->name);
> + }
> + }
> + return 0;
> +}
> +
> /* Dump information of a single bus */
> static int
> bus_dump_one(FILE *f, str
Re: [dpdk-dev] [PATCH v2 0/7] update i40e base code
> -Original Message-
> From: dev On Behalf Of Guinan Sun
> Sent: Saturday, September 12, 2020 11:01 AM
> To: dev@dpdk.org
> Cc: Guo, Jia ; Xing, Beilei ; Sun,
> GuinanX
> Subject: [dpdk-dev] [PATCH v2 0/7] update i40e base code
>
> update i40e base code.
>
> source code of i40e driver:
> cid-i40e.2020.08.27.tar.gz dropped by the team which develop basic drivers
> for any i40e NIC.
>
> changelog in ND share repo:
> From c0bfc1c07bb3 ("i40e-shared: Add VLAN field for input set") To
> 1a82d59f0797 ("i40e-shared: Fix PHY configuration parameters when enabling
> EEE")
>
> The following commits are ignored.
> 5b7d5a698092 ("i40e-shared: use linux packing style")
> f16fa495c503 ("i40e-shared: Fix compilation issue with __packed")
> c0bfc1c07bb3 ("i40e-shared: Add VLAN field for input set") dcc3e90e9acd
> ("i40e-shared: enable pipe monitor thresholds")
> Acked-by: Qi Zhang
> ---
> v2:
> * Remove two useless patches for dpdk.
> * Modify commit message.
>
> Guinan Sun (7):
> net/i40e/base: update FW API version
> net/i40e/base: fix missing function header arguments
> net/i40e/base: add support for minimum rollback revision
> net/i40e/base: fix Rx only for unicast promisc on VLAN
> net/i40e/base: add EEE LPI status check for X722 adapters
> net/i40e/base: fix PHY config param when enabling EEE
> net/i40e/base: update version
>
> drivers/net/i40e/base/README| 2 +-
> drivers/net/i40e/base/i40e_adminq_cmd.h | 24 +++-
> drivers/net/i40e/base/i40e_common.c | 82 +
> drivers/net/i40e/base/i40e_dcb.c| 3 +-
> drivers/net/i40e/base/i40e_prototype.h | 4 ++
> 5 files changed, 98 insertions(+), 17 deletions(-)
>
> --
> 2.17.1
V1 has been reverted from dpdk-next-net-intel.
V2 has been applied to dpdk-next-intel-intel.
Thanks
Qi
