Re: [dpdk-dev] [PATCH v6] net/nfb: new netcope driver

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/4/2019 10:05 AM, Rastislav Cernay wrote:
> From: Rastislav Cernay <cernay@netcope.com>
> 
> Added new net driver for Netcope nfb cards
> 
> Signed-off-by: Rastislav Cernay <cernay@netcope.com>

<...>

> @@ -0,0 +1,18 @@
> +;
> +; Supported features of the 'nfb' network poll mode driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Speed capabilities   = P
> +Link status          = Y
> +Queue start/stop     = Y
> +Promiscuous mode     = Y
> +Allmulticast mode    = Y
> +Basic stats          = Y
> +Extended stats       = Y
> +Stats per queue      = Y
> +Other kdrv           = Y
> +x86-64               = Y
> +Usage doc            = Y
> +Scattered Tx         = Y

We don't have 'Scattered Tx' feature defined, please remove.

<...>

> @@ -0,0 +1,143 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +    Copyright 2015 - 2016 CESNET

You may want update the copyright year to 2019.

<...>

> +Although each slot may be connected to a different CPU and therefore to a different
> +NUMA node, the card is represented as a single port in DPDK. To work with data
> +from the individual queues on the right NUMA node, connection of NUMA nodes on
> +first and last queue (each NUMA node has half of the queues) need to be checked.
> +
> +.. figure:: img/szedata2_nfb200g_architecture.svg

Getting following warning from ./devtools/checkpatches.sh:
Warning in /doc/guides/nics/nfb.rst:
Using explicit .svg extension instead of .*

the comment on the script says:
        # svg figures must be included with wildcard extension
        # because of png conversion for pdf docs
so it seems this breaks the pdf creation.

updating as following should solve it:
.. figure:: img/szedata2_nfb200g_architecture.*

<...>

> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += $(shell command -v pkg-config > /dev/null 2>&1 && pkg-config --cflags netcope-common)
> +LDLIBS += -lnfb
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
> +LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs

Are kvargs & ring libraries really needed? Can you please clean all unnecessary
libraries.

> +LDLIBS += -lrte_bus_pci
> +LDLIBS += $(shell command -v pkg-config > /dev/null 2>&1 && pkg-config --libs netcope-common)
> +
> +EXPORT_MAP := rte_nfb_pmd_version.map

The .map file name seems corrected, but this time EXPORT_MAP value is wrong J
It was correct and changed in this version and it is wrong now, shared library
build is still failing because of this.

<...>

> @@ -0,0 +1,596 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2019 Cesnet
> + * Copyright(c) 2019 Netcope Technologies, a.s. <info@netcope.com>
> + * All rights reserved.
> + */
> +
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <stdbool.h>
> +#include <err.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <sys/mman.h>
> +
> +#include <nfb/nfb.h>
> +#include <nfb/ndp.h>
> +#include <netcope/rxmac.h>
> +#include <netcope/txmac.h>
> +
> +#include <rte_mbuf.h>
> +#include <rte_ethdev.h>
> +#include <rte_ethdev_pci.h>
> +#include <rte_malloc.h>
> +#include <rte_memcpy.h>
> +#include <rte_kvargs.h>

Is kvargs header nedded, I don't see kvargs used.
Can you please clean all unnecessary headers, if you don't push initial version
clean, it is less likely they will be cleaned in the future.

This comment is valid for all .c files.