Re: [dpdk-dev] [PATCH] devtools: update headline prefix for drivers
2017-01-26 14:07, Ferruh Yigit: > For the patches that touch multiple drivers in same driver group, > script forces headline prefix start with drv group. > > Like for net/a net/b net/c, patch title should be "net: x y z" > > Update rule to let "driver" prefix in headline, > for above sample patch title becomes: "drivers/net: x y z" > > This prevents patch confused with library with same name. > > Signed-off-by: Ferruh Yigit Applied, thanks
Re: [dpdk-dev] [PATCH] devtools: add git log checks for nvm, tso, Vlan
> Signed-off-by: Ferruh Yigit Applied, thanks
Re: [dpdk-dev] [PATCH] mk: remove default toolchain prefix for ThunderX
2017-01-23 15:09, Jerin Jacob: > On Mon, Jan 23, 2017 at 10:15:57AM +0100, Thomas Monjalon wrote: > > The environment variable CROSS must be set when using a cross-toolchain. > > However it is counter intuitive to set a default value, considering > > the toolchain required to build this architecture is well known. > > It is especially weird when using a native toolchain and requiring to > > unset this variable on the command line. > > > > Signed-off-by: Thomas Monjalon > > Reviewed-by: Jerin Jacob Applied
Re: [dpdk-dev] [PATCH] mk: parallelize make config
2017-01-22 01:50, Ferruh Yigit: > make config dependency resolving was always running serial, > parallelize it for better performance. It could be interesting to explain why it was not parallelized, and how you made it possible. The test script should be updated as below: --- a/devtools/test-build.sh +++ b/devtools/test-build.sh - make T=$2 O=$1 config + make -j$J T=$2 O=$1 config > --- a/mk/internal/rte.depdirs-post.mk > +++ b/mk/internal/rte.depdirs-post.mk > @@ -29,11 +29,12 @@ > # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > -.PHONY: depdirs > -depdirs: > - @for d in $(DEPDIRS-y); do \ > - $(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $$d ; \ > - done > +.PHONY: depdirs $(DEPDIRS-y) > +depdirs: $(DEPDIRS-y) > + @echo "" Why this echo "" ? > + > +$(DEPDIRS-y): > + @$(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $@ > --- a/mk/rte.sdkdepdirs.mk > +++ b/mk/rte.sdkdepdirs.mk > @@ -36,19 +36,22 @@ ifeq (,$(wildcard $(RTE_OUTPUT)/Makefile)) >$(error "need a make config first") > endif > > +DEPDIRS = $(addsuffix /.depdirs, $(addprefix $(BUILDDIR)/,$(ROOTDIRS-y))) These DEPDIRS are files, although DEPDIRS in other contexts are directories. I think it should be renamed. DEPDIR_FILES? > # use a "for" in a shell to process dependencies: we don't want this > # task to be run in parallel. You forgot to remove this obsolete comment. > .PHONY: depdirs > depdirs: $(RTE_OUTPUT)/.depdirs > -$(RTE_OUTPUT)/.depdirs: $(RTE_OUTPUT)/.config > - @rm -f $(RTE_OUTPUT)/.depdirs ; \ > - for d in $(ROOTDIRS-y); do \ > - if [ -f $(RTE_SRCDIR)/$$d/Makefile ]; then \ > - [ -d $(BUILDDIR)/$$d ] || mkdir -p $(BUILDDIR)/$$d ; \ > - $(MAKE) S=$$d -f $(RTE_SRCDIR)/$$d/Makefile depdirs \ > - >> $(RTE_OUTPUT)/.depdirs ; \ > - fi ; \ > - done > +$(RTE_OUTPUT)/.depdirs: $(DEPDIRS) > + @rm -f $@ > + @for f in $(DEPDIRS); do cat $$f >> $@; done > + @sort -u -o $@ $@ > + > +$(DEPDIRS): $(RTE_OUTPUT)/.config > + @f=$(lastword $(subst /, ,$(dir $@))); \ Could you use $(notdir $(@D)) ? > + [ -d $(BUILDDIR)/$$f ] || mkdir -p $(BUILDDIR)/$$f; \ > + rm -f $@; \ Why this removal? > + $(MAKE) S=$$f -f $(RTE_SRCDIR)/$$f/Makefile depdirs >> $@ This part is a bit complicated. Could it be simplified by better naming $f? > --- a/mk/rte.subdir.mk > +++ b/mk/rte.subdir.mk > @@ -76,7 +76,7 @@ clean: _postclean > # include .depdirs and define rules to order priorities between build > # of directories. > # > -include $(RTE_OUTPUT)/.depdirs > +-include $(RTE_OUTPUT)/.depdirs > > define depdirs_rule > $(1): $(sort $(patsubst $(S)/%,%,$(LOCAL_DEPDIRS-$(S)/$(1 > @@ -84,16 +84,15 @@ endef > > $(foreach d,$(DIRS-y),$(eval $(call depdirs_rule,$(d > > +DEPDIRS = $(wildcard $(addprefix $(S)/,$(DIRS-y))) > > # use a "for" in a shell to process dependencies: we don't want this > # task to be run in parallel. You forgot to remove this obsolete comment. > -.PHONY: depdirs > -depdirs: > - @for d in $(DIRS-y); do \ > - if [ -f $(SRCDIR)/$$d/Makefile ]; then \ > - $(MAKE) S=$S/$$d -f $(SRCDIR)/$$d/Makefile depdirs ; \ > - fi ; \ > - done > +.PHONY: depdirs $(DEPDIRS) > +depdirs: $(DEPDIRS) > + > +$(DEPDIRS): > + @$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile depdirs
Re: [dpdk-dev] [PATCH] buildtools: allow symlinks within a source directory
2017-01-23 12:11, Bruce Richardson: > When creating the symlinks for header files to the include folder, the > relpath script dereferenced all symlinks. This made it impossible to > have file A.h renamed to B.h and then symlinked back to its original > name. This is useful to be able to do when refactoring or reworking > a library. Change this so that we just use the dirname of the path from > readlink, we can use the basename as it was originally, even if it was a > symlink. > > Signed-off-by: Bruce Richardson Applied, thanks
Re: [dpdk-dev] [PATCH 2/2] config: disable KNI ethtool by default
2017-01-17 19:35, Thomas Monjalon: > 2017-01-17 18:01, Ferruh Yigit: > > KNI ethtool support (KNI control path) is not commonly used, > > and it tends to break the build with new version of the Linux kernel. > > > > KNI ethtool feature is disabled by default. KNI datapath is not effected > > from this update. > > > > It is possible to enable feature explicitly with config option: > > "CONFIG_RTE_KNI_KMOD_ETHTOOL=y" > > > > Signed-off-by: Ferruh Yigit > > Acked-by: Thomas Monjalon Applied, thanks
Re: [dpdk-dev] [PATCH 10/13] KNI: provided netif name's source is user-space
2016-12-14 17:35, Ferruh Yigit: > On 12/14/2016 5:35 PM, Ferruh Yigit wrote: > > On 12/14/2016 5:19 PM, Michał Mirosław wrote: > >> On Wed, Dec 14, 2016 at 05:06:23PM +, Ferruh Yigit wrote: > >>> Hi Michal, > >>> > >>> On 12/13/2016 1:08 AM, Michał Mirosław wrote: > Signed-off-by: Michał Mirosław > --- > > Acked-by: Ferruh Yigit Applied, thanks
Re: [dpdk-dev] [PATCH 11/13] KNI: guard against unterminated dev_info.name leading to BUG in alloc_netdev()
2016-12-14 17:33, Ferruh Yigit: > On 12/13/2016 1:08 AM, Michał Mirosław wrote: > > Signed-off-by: Michał Mirosław > > --- > > Acked-by: Ferruh Yigit Applied, thanks
Re: [dpdk-dev] [PATCH] eal: fix wrong log at startup
2017-01-24 17:21, Olivier Matz:
> The log "Debug logs available - lower performance" should
> now only be displayed when dataplane debug logs are enabled.
>
> The issue occurs only if the default log level (CONFIG_RTE_LOG_LEVEL) is
> set to DEBUG in the configuration, which is not the case by default.
>
> Fixes: 5d8f0baf69ea ("log: do not drop debug logs at compile time")
>
> Signed-off-by: Olivier Matz
Applied, thanks
Re: [dpdk-dev] [PATCH] eal: fail eal when no hugepages were found
2017-01-24 21:22, Emmanuel Roullit:
> Found with clang static analysis:
> lib/librte_eal/linuxapp/eal/eal_memory.c:1004:11:
> warning: Call to 'malloc' has an allocation size of 0 bytes
> tmp_hp = malloc(nr_hugepages * sizeof(struct hugepage_file));
> ^~~
>
> The --no-huge case, where nr_hugepages would be 0
> as well, is handled earlier.
>
> Fixes: 5e823a451261 ("ethdev: remove some VF functions")
This commit reference seems wrong. Please check
Re: [dpdk-dev] [PATCH] eal: reset driver name pointer on failure
2017-01-24 21:26, Emmanuel Roullit:
> The pointer set by strdup() needs to be cleared on failure to avoid a
> potential double-free from the caller.
>
> Found with clang static analysis:
> lib/librte_eal/common/eal_common_devargs.c:123:2:
> warning: Attempt to free released memory
> free(buf);
> ^
>
> Fixes: 3fe2e5fec82b ("eal: fix argument parsing check")
The real bug origin is:
Fixes: 0fe11ec592b2 ("eal: add vdev init and uninit")
> Signed-off-by: Emmanuel Roullit
Applied, thanks
Re: [dpdk-dev] [PATCH] mempool: fix stack handler dequeue
2017-01-23 18:11, Olivier Matz:
> The return value of the stack handler is wrong: it should be 0 on
> success, not the number of objects dequeued.
>
> This could lead to memory leaks depending on how the caller checks the
> return value (ret < 0 or ret != 0). This was also breaking autotests
> with debug enabled, because the debug cookies are only updated when the
> function returns 0, so the cookies were not updated, leading to
> an abort().
>
> Fixes: 295a530b0844 ("mempool: add stack mempool handler")
>
> CC: sta...@dpdk.org
> Signed-off-by: Olivier Matz
Applied, thanks
[dpdk-dev] rte_port_ring and SP/MP, SC/MC flags
Hello, I'd like to use rte_port_ring abstract in my application and I'm a little confused about how it treats underlying ring flags. According to DPDK API reference, when creating a ring (via rte_ring_create()/rte_ring_init()), RING_F_SP_ENQ/RING_F_SC_DEQ may be specified. These flags affect the choice of MP/SP, MC/SC operation when using 'default' ring enq/deq API, i.e. rte_ring_enqueue()/rte_ring_dequeue(), rte_ring_enqueue_bulk()/rte_ring_dequeue_bulk(), rte_ring_enqueue_burst()/rte_ring_dequeue_burst(). These API then choose which version of enq/deq to use considering the flags. If you use designated API straightforward, those API (*_mp_*, *_sc_* etc.) don't care about these flags and perform required operations right away. When I use rte_port_ring abstraction, '.f_create()' functions check for flags which were used when creating an underlying ring (see lib/librte_port/rte_port_ring.c:75). But then different call tables use designated ring API which makes checking flags pointless. I find it confusing to be forced to choose between SP/MP, SC/MC twice, when creating ring at first and creating abstraction afterwards. And I see no point in checking for ring flags when creating abstraction because it really does not affect the operation of this abstraction anyway. Is this behaviour anyhow justified? -- Yerden Zhumabekov
