[dpdk-dev] Clang reporting a problem when adding another member initialization.
2014-10-03 21:05, Wiles, Roger Keith:
> I run into a problem with Clang report problem when I tried to add
> another member to the static initializer of the following in file
> ixgbe_rxtx_vec.c
[...]
> Then I removed the {} and it now builds. Is this a result of the
> changes to the mbuf structure and Clang being picky?
>
> Should I submit a patch to remove the ?{ }? values?
You should check this thread:
http://dpdk.org/ml/archives/dev/2014-September/005504.html
You may be hitting a compiler bug.
--
Thomas
[dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Keith Wiles
> Sent: Sunday, October 05, 2014 12:10 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk()
> and rte_pktmbuf_free_bulk()
>
> Minor helper routines to mirror the mempool routines and remove the code
> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
>
I believe such a change would cause a performance regression, as the extra init
code in the alloc_bulk() function would take additional cycles and is not
needed. The vector routines use the mempool function directly, so that there is
no overhead of mbuf initialization, as the vector routines use their additional
"knowledge" of what the mbufs will be used for to init them in a faster manner
than can be done inside the mbuf library.
/Bruce
> Signed-off-by: Keith Wiles
> ---
> lib/librte_mbuf/rte_mbuf.h | 77
> ++
> 1 file changed, 77 insertions(+)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 1c6e115..f298621 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -546,6 +546,41 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf
> *m)
> }
>
> /**
> + * @internal Allocate a list of mbufs from mempool *mp*.
> + * The use of that function is reserved for RTE internal needs.
> + * Please use rte_pktmbuf_alloc_bulk().
> + *
> + * @param mp
> + * The mempool from which mbuf is allocated.
> + * @param m_list
> + * The array to place the allocated rte_mbufs pointers.
> + * @param cnt
> + * The number of mbufs to allocate
> + * @return
> + * - 0 if the number of mbufs allocated was ok
> + * - <0 is an ERROR.
> + */
> +static inline int __rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct
> rte_mbuf *m_list[], int cnt)
> +{
> + struct rte_mbuf *m;
> + int ret;
> +
> + ret = rte_mempool_get_bulk(mp, (void **)m_list, cnt);
> + if ( ret == 0 ) {
> + int i;
> + for(i = 0; i < cnt; i++) {
> + m = *m_list++;
> +#ifdef RTE_MBUF_REFCNT
> + rte_mbuf_refcnt_set(m, 1);
> +#endif /* RTE_MBUF_REFCNT */
> + rte_pktmbuf_reset(m);
> + }
> + ret = cnt;
> + }
> + return ret;
> +}
> +
> +/**
> * Allocate a new mbuf from a mempool.
> *
> * This new mbuf contains one segment, which has a length of 0. The pointer
> @@ -671,6 +706,32 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
> }
>
> /**
> + * Allocate a list of mbufs from a mempool into a mbufs array.
> + *
> + * This mbuf list contains one segment per mbuf, which has a length of 0. The
> pointer
> + * to data is initialized to have some bytes of headroom in the buffer
> + * (if buffer size allows).
> + *
> + * The routine is just a simple wrapper routine to reduce code in the
> application
> and
> + * provide a cleaner API for multiple mbuf requests.
> + *
> + * @param mp
> + * The mempool from which the mbuf is allocated.
> + * @param m_list
> + * An array of mbuf pointers, cnt must be less then or equal to the size
> of the
> list.
> + * @param cnt
> + * Number of slots in the m_list array to fill.
> + * @return
> + * - The number of valid mbufs pointers in the m_list array.
> + * - Zero if the request cnt could not be allocated.
> + */
> +static inline int __attribute__((always_inline))
> +rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
> int16_t cnt)
> +{
> + return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
> +}
> +
> +/**
> * Free a segment of a packet mbuf into its original mempool.
> *
> * Free an mbuf, without parsing other segments in case of chained
> @@ -708,6 +769,22 @@ static inline void rte_pktmbuf_free(struct rte_mbuf
> *m)
> }
> }
>
> +/**
> + * Free a list of packet mbufs back into its original mempool.
> + *
> + * Free a list of mbufs by calling rte_pktmbuf_free() in a loop as a wrapper
> function.
> + *
> + * @param m_list
> + * An array of rte_mbuf pointers to be freed.
> + * @param npkts
> + * Number of packets to free in list.
> + */
> +static inline void rte_pktmbuf_free_bulk(struct rte_mbuf *m_list[], int16_t
> npkts)
> +{
> + while(npkts--)
> + rte_pktmbuf_free(*m_list++);
> +}
> +
> #ifdef RTE_MBUF_REFCNT
>
> /**
> --
> 2.1.0
[dpdk-dev] Possible bug in eal_pci pci_scan_one
Hi Guys,
I'm doing my development on kind of a cheap machine with no NUMA support...
but several years ago I used DPDK to build a NUMA box that could do 40 gbits
bidirectional L4-L7 stateful traffic replay.
So given the past experiences I had before, I wanted to clean the code up so
it'd work well if some crazy guy tried my code on one of these huge boxes,
too, but then I ran into some weird issues.
1) When I call rte_eth_dev_socket_id() I get back -1. But the call can return
-1 if the port_id is bogus or if pci_scan_one didn't get a numa_node (because
you're on a non-NUMA box for example).
int rte_eth_dev_socket_id(uint8_t port_id)
{
if (port_id >= nb_ports)
return -1;
return rte_eth_devices[port_id].pci_dev->numa_node;
}
So you couldn't tell the different between non-NUMA or a bad port value, etc.
2) The code's behavior and comments disagree with one another. In the
pci_scan_one function, there's this code:
/* get numa node */
snprintf(filename, sizeof(filename), "%s/numa_node",
dirname);
if (access(filename, R_OK) != 0) {
/* if no NUMA support just set node to 0 */
dev->numa_node = -1;
} else {
if (eal_parse_sysfs_value(filename, &tmp) < 0) {
free(dev);
return -1;
}
dev->numa_node = tmp;
}
It says, just use NUMA node 0 if there is no NUMA support. But then proceeds
to set the value to -1 in disagreement with the comment, and also stomping on
the other meaning for -1 in the higher function rte_eth_dev_socket_id.
3) In conclusion, it seems like some stuff is missing... first there needs to
be a function that will tell you the number of NUMA nodes present on the box
so you can create the right number of mbuf_pools, but I couldn't find that
function.
Then if you have the function, you can do some magic and shuffle the NICs
around to get them hooked to a core on the same NUMA, and the mbuf_pool on the
same NUMA.
When NUMA is not present, can we return 0 instead of -1, or return a specific
error code that the client can use to know he should just use Socket 0? Right
now I can't tell apart any potential errors or weird values from correct
values.
4) I'm willing to help make and test some patches... but first I want to
understand what is happening with these funny functions before doing things
blindly.
Thanks,
Matthew.
[dpdk-dev] [PATCH v2 1/4] Link combined shared library using CC
Signed-off-by: Sergio Gonzalez Monroy --- mk/rte.sharelib.mk | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk index c0a811a..73ce709 100644 --- a/mk/rte.sharelib.mk +++ b/mk/rte.sharelib.mk @@ -29,6 +29,8 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +include $(RTE_SDK)/mk/internal/rte.build-pre.mk + # VPATH contains at least SRCDIR VPATH += $(SRCDIR) @@ -40,12 +42,20 @@ LIB_ONE := lib$(RTE_LIBNAME).a endif endif +ifeq ($(LINK_USING_CC),1) +# Override the definition of LD here, since we're linking with CC +LD := $(CC) +LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs) +CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS)) +endif + .PHONY:sharelib sharelib: $(LIB_ONE) FORCE OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o) -O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE) +O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS)\ + -o $(RTE_OUTPUT)/lib/$(LIB_ONE) O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)"," LD $(@)") O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)" -- 1.9.3
[dpdk-dev] [PATCH v2 4/4] Link apps/DSOs against EXECENV_LDLIBS with --as-needed
Signed-off-by: Sergio Gonzalez Monroy --- mk/rte.app.mk | 4 ++-- mk/rte.lib.mk | 8 +++- mk/rte.sharelib.mk | 7 ++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 5e00e67..e775ad7 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -62,9 +62,9 @@ ifeq ($(NO_AUTOLIBS),) LDLIBS += --whole-archive LDLIBS += -l$(RTE_LIBNAME) LDLIBS += --no-whole-archive -LDLIBS += --start-group +LDLIBS += --as-needed LDLIBS += $(EXECENV_LDLIBS) -LDLIBS += --end-group +LDLIBS += --no-as-needed endif # ifeq ($(NO_AUTOLIBS),) diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk index e7420bf..947e17d 100644 --- a/mk/rte.lib.mk +++ b/mk/rte.lib.mk @@ -59,14 +59,20 @@ build: _postbuild exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1 +O_TO_S_LDLIBS := --as-needed +O_TO_S_LDLIBS += $(EXECENV_LDLIBS) +O_TO_S_LDLIBS += --no-as-needed + ifeq ($(LINK_USING_CC),1) # Override the definition of LD here, since we're linking with CC LD := $(CC) LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs) CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS)) +O_TO_S_LDLIBS := $(call linkerprefix,$(O_TO_S_LDLIBS)) endif -O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB) +O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS-y) \ + $(O_TO_S_LDLIBS) -o $(LIB) O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #') # fix syntax highlight O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)"," LD $(@)") O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)" diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk index 8fc6548..380aa7d 100644 --- a/mk/rte.sharelib.mk +++ b/mk/rte.sharelib.mk @@ -40,11 +40,16 @@ else LIB_ONE := lib$(RTE_LIBNAME).a endif +O_TO_L_LDLIBS := --as-needed +O_TO_L_LDLIBS += $(EXECENV_LDLIBS) +O_TO_L_LDLIBS += --no-as-needed + ifeq ($(LINK_USING_CC),1) # Override the definition of LD here, since we're linking with CC LD := $(CC) LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs) CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS)) +O_TO_L_LDLIBS := $(call linkerprefix,$(O_TO_L_LDLIBS)) endif .PHONY:sharelib @@ -54,7 +59,7 @@ OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o) ifeq ($(RTE_BUILD_SHARED_LIB),y) O_TO_L = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS)\ - -o $(RTE_OUTPUT)/lib/$(LIB_ONE) + $(O_TO_L_LDLIBS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE) L_DISP=LD else O_TO_L = $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS) -- 1.9.3
[dpdk-dev] [PATCH v2 2/4] Link apps only against single/combined library
Signed-off-by: Sergio Gonzalez Monroy --- mk/rte.app.mk | 160 +- 1 file changed, 2 insertions(+), 158 deletions(-) diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 34dff2a..5e00e67 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -60,164 +60,12 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib ifeq ($(NO_AUTOLIBS),) LDLIBS += --whole-archive - -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) -LDLIBS += -lrte_distributor -endif - -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) -LDLIBS += -lrte_kni -endif -endif - -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) -LDLIBS += -lrte_ivshmem -endif -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) -LDLIBS += -lrte_pipeline -endif - -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) -LDLIBS += -lrte_table -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) -LDLIBS += -lrte_port -endif - -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y) -LDLIBS += -lrte_timer -endif - -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y) -LDLIBS += -lrte_hash -endif - -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) -LDLIBS += -lrte_lpm -endif - -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y) -LDLIBS += -lrte_power -endif - -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y) -LDLIBS += -lrte_acl -endif - -ifeq ($(CONFIG_RTE_LIBRTE_METER),y) -LDLIBS += -lrte_meter -endif - -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) -LDLIBS += -lrte_sched -LDLIBS += -lm -LDLIBS += -lrt -endif - +LDLIBS += -l$(RTE_LIBNAME) +LDLIBS += --no-whole-archive LDLIBS += --start-group - -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y) -LDLIBS += -lrte_kvargs -endif - -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y) -LDLIBS += -lrte_mbuf -endif - -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y) -LDLIBS += -lrte_ip_frag -endif - -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y) -LDLIBS += -lethdev -endif - -ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) -LDLIBS += -lrte_malloc -endif - -ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y) -LDLIBS += -lrte_mempool -endif - -ifeq ($(CONFIG_RTE_LIBRTE_RING),y) -LDLIBS += -lrte_ring -endif - -ifeq ($(CONFIG_RTE_LIBC),y) -LDLIBS += -lc -LDLIBS += -lm -endif - -ifeq ($(CONFIG_RTE_LIBGLOSS),y) -LDLIBS += -lgloss -endif - -ifeq ($(CONFIG_RTE_LIBRTE_EAL),y) -LDLIBS += -lrte_eal -endif - -ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y) -LDLIBS += -lrte_cmdline -endif - -ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y) -LDLIBS += -lrte_cfgfile -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y) -LDLIBS += -lrte_pmd_bond -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y) -LDLIBS += -lrte_pmd_xenvirt -LDLIBS += -lxenstore -endif - -ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) -# plugins (link only if static libraries) - -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) -LDLIBS += -lrte_pmd_vmxnet3_uio -endif - -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) -LDLIBS += -lrte_pmd_virtio_uio -endif - -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) -LDLIBS += -lrte_pmd_i40e -endif - -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) -LDLIBS += -lrte_pmd_ixgbe -endif - -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y) -LDLIBS += -lrte_pmd_e1000 -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) -LDLIBS += -lrte_pmd_ring -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) -LDLIBS += -lrte_pmd_pcap -lpcap -endif - -endif # plugins - LDLIBS += $(EXECENV_LDLIBS) - LDLIBS += --end-group -LDLIBS += --no-whole-archive - endif # ifeq ($(NO_AUTOLIBS),) LDLIBS += $(CPU_LDLIBS) @@ -235,10 +83,6 @@ build: _postbuild exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1 -ifeq ($(RTE_BUILD_COMBINE_LIBS),y) -LDLIBS += -l$(RTE_LIBNAME) -endif - ifeq ($(LINK_USING_CC),1) LDLIBS := $(call linkerprefix,$(LDLIBS)) LDFLAGS := $(call linkerprefix,$(LDFLAGS)) -- 1.9.3
[dpdk-dev] [PATCH v2 0/4] Update build process
As per the proposal, this patch set does: - Remove CONFIG_RTE_BUILD_COMBINE_LIBS as a configuration option. - For static library, build a single/combined library. - For shared libraries, build both individual/separated and single/combined libraries. - Link apps only against single/combined libs. Sergio Gonzalez Monroy (4): Link combined shared library using CC Link apps only against single/combined library Update library build process Link apps/DSOs against EXECENV_LDLIBS with --as-needed config/common_bsdapp | 3 +- config/common_linuxapp | 3 +- mk/rte.app.mk | 164 ++--- mk/rte.lib.mk | 81 ++-- mk/rte.sdkbuild.mk | 2 +- mk/rte.sharelib.mk | 54 mk/rte.vars.mk | 4 -- 7 files changed, 54 insertions(+), 257 deletions(-) -- 1.9.3
[dpdk-dev] [PATCH v2 3/4] Update library build process
Remove COMBINE_LIBS option and by default build: - CONFIG_RTE_BUILD_SHARED_LIB=y : both individual and combined libraries - CONFIG_RTE_BUILD_SHARED_LIB=n : single combined library Signed-off-by: Sergio Gonzalez Monroy --- config/common_bsdapp | 3 +-- config/common_linuxapp | 3 +-- mk/rte.lib.mk | 73 -- mk/rte.sdkbuild.mk | 2 +- mk/rte.sharelib.mk | 41 +++- mk/rte.vars.mk | 4 --- 6 files changed, 29 insertions(+), 97 deletions(-) diff --git a/config/common_bsdapp b/config/common_bsdapp index eebd05b..65d2ecc 100644 --- a/config/common_bsdapp +++ b/config/common_bsdapp @@ -79,9 +79,8 @@ CONFIG_RTE_FORCE_INTRINSICS=n CONFIG_RTE_BUILD_SHARED_LIB=n # -# Combine to one single library +# Library name # -CONFIG_RTE_BUILD_COMBINE_LIBS=n CONFIG_RTE_LIBNAME=intel_dpdk # diff --git a/config/common_linuxapp b/config/common_linuxapp index 4713eb4..5bdcdf3 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -79,9 +79,8 @@ CONFIG_RTE_FORCE_INTRINSICS=n CONFIG_RTE_BUILD_SHARED_LIB=n # -# Combine to one single library +# Library name # -CONFIG_RTE_BUILD_COMBINE_LIBS=n CONFIG_RTE_LIBNAME="intel_dpdk" # diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk index f458258..e7420bf 100644 --- a/mk/rte.lib.mk +++ b/mk/rte.lib.mk @@ -66,48 +66,23 @@ LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs) CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS)) endif -O_TO_A = $(AR) crus $(LIB) $(OBJS-y) -O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight -O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)"," AR $(@)") -O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)" -O_TO_A_DO = @set -e; \ - echo $(O_TO_A_DISP); \ - $(O_TO_A) && \ - echo $(O_TO_A_CMD) > $(call exe2cmd,$(@)) - O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB) -O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight +O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #') # fix syntax highlight O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)"," LD $(@)") +O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)" O_TO_S_DO = @set -e; \ + cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib; \ echo $(O_TO_S_DISP); \ $(O_TO_S) && \ echo $(O_TO_S_CMD) > $(call exe2cmd,$(@)) -ifeq ($(RTE_BUILD_SHARED_LIB),n) -O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y) -O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight -O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)"," AR_C $(@)") -O_TO_C_DO = @set -e; \ - $(lib_dir) \ - $(copy_obj) -else -O_TO_C = $(LD) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB_ONE) -O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight -O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)"," LD_C $(@)") -O_TO_C_DO = @set -e; \ - $(lib_dir) \ - $(copy_obj) -endif - -copy_obj = cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib; -lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib; -include .$(LIB).cmd # # Archive objects in .a file if needed # -ifeq ($(RTE_BUILD_SHARED_LIB),y) $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE +ifeq ($(RTE_BUILD_SHARED_LIB),y) @[ -d $(dir $@) ] || mkdir -p $(dir $@) $(if $(D),\ @echo -n "$< -> $@ " ; \ @@ -116,51 +91,25 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \ echo "depfile_newer=$(call boolean,$(depfile_newer)) ") $(if $(or \ - $(file_missing),\ - $(call cmdline_changed,$(O_TO_S_STR)),\ - $(depfile_missing),\ - $(depfile_newer)),\ - $(O_TO_S_DO)) -ifeq ($(RTE_BUILD_COMBINE_LIBS),y) - $(if $(or \ -$(file_missing),\ -$(call cmdline_changed,$(O_TO_C_STR)),\ -$(depfile_missing),\ -$(depfile_newer)),\ -$(O_TO_C_DO)) -endif -else -$(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE - @[ -d $(dir $@) ] || mkdir -p $(dir $@) - $(if $(D),\ - @echo -n "$< -> $@ " ; \ - echo -n "file_missing=$(call boolean,$(file_missing)) " ; \ - echo -n "cmdline_changed=$(call boolean,$(call cmdline_changed,$(O_TO_A_STR))) " ; \ - echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \ - echo "depfile_newer=$(call boolean,$(depfile_newer)) ") - $(if $(or \ $(file_missing),\ - $(call cmdline_changed,$(O_TO_A_STR)),\ + $(call cmdline_changed,$(O_TO_S_STR)),\ $(depfile_missing),\ $(depfile_newer)),\ - $(O_TO_A_DO)) -ifeq ($(RTE_BUILD_COMBINE_LIBS),y) - $(if $(or \ -$(file_missing),\ -$(call cmdline_changed,$(O_TO_C_STR)),\ -$(depfile_missing),\ -$(depfile_newer)),\ -$(O_TO_C_DO)) -endif + $(O_TO_S_DO)) +else + @set -e; \ + cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib; endif # # install lib in $(RTE_OUTPUT)/lib # $(RTE_OUTPUT)/lib/$(LIB): $(LIB) +ifeq ($(RT
[dpdk-dev] [PATCH v3] distributor_app: new sample app
> -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Wednesday, October 1, 2014 5:08 PM > To: Richardson, Bruce > Cc: Pattan, Reshma; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > > > > > > > > 1)I had sent v5 patch which handles graceful shutdown of rx and tx > > > > threads upon SIGINT > > > I see it and will take a look shortly, thanks. > > > > > > > 2)Worker thread graceful shutdown was not handled as of now as it needs > some change in lcore_worker logic , which will be done in future enhancements. > > > Not sure I understand what you mean here. Can you elaborate? > > > rte_distributor_process which runs as part of rx thread will process incoming packets and checks for any requests for the packets from worker threads . If request is seen, it adds the packet/work to particular workers back log and proceed with processing of next packet. If no request seen the packet index will not be incremented and the while loop which is conditionally based on packet indexing runs in a continuous loop without breaking and rx thread will not proceed with next statement execution until unless rte_distributor_process comes out of while loop. This issue happens only when we enable graceful shutdown logic for both rx/worker threads, as workers threads gets killed and no request seen by rx thread and it stucks. Hence as of now graceful shutdown logic is provided only for rx thread. For worker threads will check what can be done in next enhancements. Thanks, Reshma > > > > 3)Freeing of mempool is also not handled , as the framework support is > > > > not > available. > > > Ew, I hadn't noticed that, freeing of mempools seems like something > > > we should implement. > > > > > > > 4)Cleaning of rx/tx queues not done, as it needs some extensive logic > which we haven't planned as of now. Will check the possibility of doing it in > future enhancementsi.e in next version of sample application. > > > We can't just flush the queues after we shutdown the workers? I > > > presume a queue flush operation exists, yes? > > > Neil > > > > Other than code hygiene, which does have some value in itself, I can't > > really see what the practical point of such cleanup would be. > > > This is really the only assertion I'm trying to make. I understand this > application > won't suffer from exiting uncleanly, and that makes the need for preforming > cleanup little more than overhead. > > But that said, hygine is exactly the point I'm driving at here. These are > example > applications, that presumably people look at when writing their own apps. If > you don't do things properly, people looking at your code are less likely to > do > them as well. Even if it doesn't hurt for you to exit uncleanly, it will hurt > someone, and if they look to these examples as a source of best practices, it > seems to me that it would be in everyones interest, if best practices were > demonstrated. > > Neil -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH] Fix librte_pmd_pcap driver double stop error
On Sat, Oct 04, 2014 at 07:14:21PM +0100, Nicol?s Pernas Maradei wrote: > Hi, > > You are correct, the parameters received in the driver are allocated in > devargs_list (char *params variable). However, they already get strdup'd in > rte_kvargs_parse(). This newly allocated string is part of kvlist and never > freed up. The params variable is never used again so it can be freed by > someone else using free_devargs_list(). I'd say it's safe enough to set up > pointers in the way it's currently done. > > Nico. > ok, that seems reasonable Acked-by: Neil Horman > On 2014-09-29 15:24, Neil Horman wrote: > >On Wed, Sep 10, 2014 at 05:17:05PM -0300, Nicol?s Pernas Maradei wrote: > >>From: Nicol?s Pernas Maradei > >> > >>librte_pmd_pcap driver was opening the pcap/interfaces only at init time > >>and > >>closing them only when the port was being stopped. This behaviour would > >>cause > >>problems (leading to segfault) if the user closed the port 2 times. The > >>first > >>time the pcap/interfaces would be normally closed but libpcap would > >>throw an > >>error causing a segfault if the closed pcaps/interfaces were closed > >>again. > >>This behaviour is solved by re-openning pcaps/interfaces when the port > >>is > >>started (only if these weren't open already for example at init time). > >> > >>Signed-off-by: Nicol?s Pernas Maradei > > > >This patch assigns pointers to strings that are allocated in the > >devargs_list. > >Given that there exists an api interface free_devargs_list(), I'm not sure > >that > >whats being done here is consistently safe. It seems like you should dup > >the > >strings to make sure you always have the storage allocated, or find some > >other > >method to store the needed information. > > > >Neil >
[dpdk-dev] [PATCH v3] distributor_app: new sample app
On Mon, Oct 06, 2014 at 02:16:22PM +, Pattan, Reshma wrote: > > > > -Original Message- > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > Sent: Wednesday, October 1, 2014 5:08 PM > > To: Richardson, Bruce > > Cc: Pattan, Reshma; dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > > > > > > > > > > > 1)I had sent v5 patch which handles graceful shutdown of rx and tx > > > > > threads upon SIGINT > > > > I see it and will take a look shortly, thanks. > > > > > > > > > 2)Worker thread graceful shutdown was not handled as of now as it > > > > > needs > > some change in lcore_worker logic , which will be done in future > > enhancements. > > > > Not sure I understand what you mean here. Can you elaborate? > > > > > rte_distributor_process which runs as part of rx thread will process incoming > packets and checks for any requests for the packets from worker threads . > If request is seen, it adds the packet/work to particular workers back log > and proceed with processing of next packet. > If no request seen the packet index will not be incremented and the while > loop which is conditionally based on packet indexing runs in a continuous > loop without breaking and rx thread will not proceed with next statement > execution until unless rte_distributor_process comes out of while loop. > This issue happens only when we enable graceful shutdown logic for both > rx/worker threads, as workers threads gets killed and no request seen by rx > thread and it stucks. > Hence as of now graceful shutdown logic is provided only for rx thread. For > worker threads will check what can be done in next enhancements. > > Thanks, > Reshma > I see what you're saying, Once you make a call to rte_distributor_get_pkt, you have no way to gracefully shut down use of the rte_distributor_library. Not just this application, but any application. Thats just not sane, and suggests that we integrated the rte_distributor library too soon. I would suggest that you prefix this patch with an update to the rte distributor library to allow rte_distributor_get_pkt and friends to return NULL if the queue is emtpy. Applications should be checking the return value for NULL anyway, and can preform the rte_pause operation. Then update this patch to do a clean exit. To say that "we will check what can be done in the next enhancements" is to say that this won't be addressed again until a paying custmoer gripes about it. Neil > > > > > 3)Freeing of mempool is also not handled , as the framework support > > > > > is not > > available. > > > > Ew, I hadn't noticed that, freeing of mempools seems like something > > > > we should implement. > > > > > > > > > 4)Cleaning of rx/tx queues not done, as it needs some extensive logic > > which we haven't planned as of now. Will check the possibility of doing it > > in > > future enhancementsi.e in next version of sample application. > > > > We can't just flush the queues after we shutdown the workers? I > > > > presume a queue flush operation exists, yes? > > > > Neil > > > > > > Other than code hygiene, which does have some value in itself, I can't > > > really see what the practical point of such cleanup would be. > > > > > This is really the only assertion I'm trying to make. I understand this > > application > > won't suffer from exiting uncleanly, and that makes the need for preforming > > cleanup little more than overhead. > > > > But that said, hygine is exactly the point I'm driving at here. These are > > example > > applications, that presumably people look at when writing their own apps. > > If > > you don't do things properly, people looking at your code are less likely > > to do > > them as well. Even if it doesn't hurt for you to exit uncleanly, it will > > hurt > > someone, and if they look to these examples as a source of best practices, > > it > > seems to me that it would be in everyones interest, if best practices were > > demonstrated. > > > > Neil > > -- > Intel Shannon Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare > Registered Number: 308263 > Business address: Dromore House, East Park, Shannon, Co. Clare > > This e-mail and any attachments may contain confidential material for the > sole use of the intended recipient(s). Any review or distribution by others > is strictly prohibited. If you are not the intended recipient, please contact > the sender and delete all copies. > > >
[dpdk-dev] [PATCH 0/4] Fix build issues with CONFIG_RTE_BUILD_COMBINE_LIBS=y
On Fri, Oct 03, 2014 at 02:21:50PM -0700, Matthew Hall wrote: > On Fri, Oct 03, 2014 at 03:15:46PM -0400, Neil Horman wrote: > > With a single archive, you get everything you build even if you don't need > > it. > > Right, I was trying to avoid that for people who specifically didn't want it, > if there are any... I'm not one of them. > > > But presumably if you're building a static binary, you're > > likely building the dpdk as well and can configure optional libraries out > > of the > > build. Separate libraries are more a need for downstream > > distributors/packagers, who use dynamic shared objects anyway. > > Yeah, I was thinking it'd be nice if the downstream packagers could get a > global '.a' and per-sublib '.a' as well. So that one dpdk package could be > used by a client app which wanted everything, or only wanted portions. > > > Backward compatibilty? the DPDK doesn't yet provide run time compatibility > > between releases (something I've been trying to change). Nobody provides > > compile time compatibility. To do so would require fixing API's > > permenently. > > Agreed. I was just advocating to avoid worsening the already existent issues. > ;) > > Matthew. > Fair enough Neil
[dpdk-dev] [PATCH v2 0/4] Update build process
On Mon, Oct 06, 2014 at 11:52:31AM +0100, Sergio Gonzalez Monroy wrote: > As per the proposal, this patch set does: > - Remove CONFIG_RTE_BUILD_COMBINE_LIBS as a configuration option. > - For static library, build a single/combined library. > - For shared libraries, build both individual/separated and single/combined >libraries. > - Link apps only against single/combined libs. > > > Sergio Gonzalez Monroy (4): > Link combined shared library using CC > Link apps only against single/combined library > Update library build process > Link apps/DSOs against EXECENV_LDLIBS with --as-needed > > config/common_bsdapp | 3 +- > config/common_linuxapp | 3 +- > mk/rte.app.mk | 164 > ++--- > mk/rte.lib.mk | 81 ++-- > mk/rte.sdkbuild.mk | 2 +- > mk/rte.sharelib.mk | 54 > mk/rte.vars.mk | 4 -- > 7 files changed, 54 insertions(+), 257 deletions(-) > > -- > 1.9.3 > > I see you removed the --whole-archive option when building the single library here. Have you checked to make sure that all the constructors haven't been stripped out? Neil
[dpdk-dev] [PATCH v2] Fix librte_pmd_pcap driver double stop error
On Sun, Oct 05, 2014 at 07:37:11PM +0100, Nicol?s Pernas Maradei wrote: > Hi, > > New in Patch v2: > > - Fixes an issue in eth_dev_start/stop where a single interface was always > opened/closed even though pcap files had been selected for rx/tx streams. > - The link_status was not being properly updated in case of using a single > interface for rx/tx streams. > > Nico. > > On 2014-10-04 23:24, Nicol?s Pernas Maradei wrote: > >From: Nicol?s Pernas Maradei > > > >librte_pmd_pcap driver was opening the pcap/interfaces only at init time > >and > >closing them only when the port was being stopped. This behaviour would > >cause > >problems (leading to segfault) if the user closed the port 2 times. The > >first > >time the pcap/interfaces would be normally closed but libpcap would throw > >an > >error causing a segfault if the closed pcaps/interfaces were closed again. > >This behaviour is solved by re-openning pcaps/interfaces when the port is > >started (only if these weren't open already for example at init time). > > > >Signed-off-by: Nicol?s Pernas Maradei Acked-by: Neil Horman
[dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
Hi Bruce,
Do I need to reject the for the new routines or just make sure the vector
driver does not get updated to use those routines?
Thanks
++Keith
On Oct 6, 2014, at 3:56 AM, Richardson, Bruce
wrote:
>
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Keith Wiles
>> Sent: Sunday, October 05, 2014 12:10 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk()
>> and rte_pktmbuf_free_bulk()
>>
>> Minor helper routines to mirror the mempool routines and remove the code
>> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
>> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
>>
>
> I believe such a change would cause a performance regression, as the extra
> init code in the alloc_bulk() function would take additional cycles and is
> not needed. The vector routines use the mempool function directly, so that
> there is no overhead of mbuf initialization, as the vector routines use their
> additional "knowledge" of what the mbufs will be used for to init them in a
> faster manner than can be done inside the mbuf library.
>
> /Bruce
>
>> Signed-off-by: Keith Wiles
>> ---
>> lib/librte_mbuf/rte_mbuf.h | 77
>> ++
>> 1 file changed, 77 insertions(+)
>>
>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
>> index 1c6e115..f298621 100644
>> --- a/lib/librte_mbuf/rte_mbuf.h
>> +++ b/lib/librte_mbuf/rte_mbuf.h
>> @@ -546,6 +546,41 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf
>> *m)
>> }
>>
>> /**
>> + * @internal Allocate a list of mbufs from mempool *mp*.
>> + * The use of that function is reserved for RTE internal needs.
>> + * Please use rte_pktmbuf_alloc_bulk().
>> + *
>> + * @param mp
>> + * The mempool from which mbuf is allocated.
>> + * @param m_list
>> + * The array to place the allocated rte_mbufs pointers.
>> + * @param cnt
>> + * The number of mbufs to allocate
>> + * @return
>> + * - 0 if the number of mbufs allocated was ok
>> + * - <0 is an ERROR.
>> + */
>> +static inline int __rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct
>> rte_mbuf *m_list[], int cnt)
>> +{
>> + struct rte_mbuf *m;
>> + int ret;
>> +
>> + ret = rte_mempool_get_bulk(mp, (void **)m_list, cnt);
>> + if ( ret == 0 ) {
>> + int i;
>> + for(i = 0; i < cnt; i++) {
>> + m = *m_list++;
>> +#ifdef RTE_MBUF_REFCNT
>> + rte_mbuf_refcnt_set(m, 1);
>> +#endif /* RTE_MBUF_REFCNT */
>> + rte_pktmbuf_reset(m);
>> + }
>> + ret = cnt;
>> + }
>> + return ret;
>> +}
>> +
>> +/**
>> * Allocate a new mbuf from a mempool.
>> *
>> * This new mbuf contains one segment, which has a length of 0. The pointer
>> @@ -671,6 +706,32 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
>> }
>>
>> /**
>> + * Allocate a list of mbufs from a mempool into a mbufs array.
>> + *
>> + * This mbuf list contains one segment per mbuf, which has a length of 0.
>> The
>> pointer
>> + * to data is initialized to have some bytes of headroom in the buffer
>> + * (if buffer size allows).
>> + *
>> + * The routine is just a simple wrapper routine to reduce code in the
>> application
>> and
>> + * provide a cleaner API for multiple mbuf requests.
>> + *
>> + * @param mp
>> + * The mempool from which the mbuf is allocated.
>> + * @param m_list
>> + * An array of mbuf pointers, cnt must be less then or equal to the size
>> of the
>> list.
>> + * @param cnt
>> + * Number of slots in the m_list array to fill.
>> + * @return
>> + * - The number of valid mbufs pointers in the m_list array.
>> + * - Zero if the request cnt could not be allocated.
>> + */
>> +static inline int __attribute__((always_inline))
>> +rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
>> int16_t cnt)
>> +{
>> + return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
>> +}
>> +
>> +/**
>> * Free a segment of a packet mbuf into its original mempool.
>> *
>> * Free an mbuf, without parsing other segments in case of chained
>> @@ -708,6 +769,22 @@ static inline void rte_pktmbuf_free(struct rte_mbuf
>> *m)
>> }
>> }
>>
>> +/**
>> + * Free a list of packet mbufs back into its original mempool.
>> + *
>> + * Free a list of mbufs by calling rte_pktmbuf_free() in a loop as a wrapper
>> function.
>> + *
>> + * @param m_list
>> + * An array of rte_mbuf pointers to be freed.
>> + * @param npkts
>> + * Number of packets to free in list.
>> + */
>> +static inline void rte_pktmbuf_free_bulk(struct rte_mbuf *m_list[], int16_t
>> npkts)
>> +{
>> + while(npkts--)
>> + rte_pktmbuf_free(*m_list++);
>> +}
>> +
>> #ifdef RTE_MBUF_REFCNT
>>
>> /**
>> --
>> 2.1.0
>
Keith Wiles, Principal Technologist with CTO office, Wind River mobile
972-213-5533
[dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
On Mon, Oct 06, 2014 at 03:50:38PM +0100, Wiles, Roger Keith wrote:
> Hi Bruce,
>
> Do I need to reject the for the new routines or just make sure the vector
> driver does not get updated to use those routines?
>
The new routines are probably useful in the general case. I see no issue
with having them in the code, so long as the vector driver is not modified
to use them.
/Bruce
> Thanks
> ++Keith
>
> On Oct 6, 2014, at 3:56 AM, Richardson, Bruce
> wrote:
>
> >
> >
> >> -Original Message-
> >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Keith Wiles
> >> Sent: Sunday, October 05, 2014 12:10 AM
> >> To: dev at dpdk.org
> >> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines
> >> rte_pktmbuf_alloc_bulk()
> >> and rte_pktmbuf_free_bulk()
> >>
> >> Minor helper routines to mirror the mempool routines and remove the code
> >> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
> >> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
> >>
> >
> > I believe such a change would cause a performance regression, as the extra
> > init code in the alloc_bulk() function would take additional cycles and is
> > not needed. The vector routines use the mempool function directly, so that
> > there is no overhead of mbuf initialization, as the vector routines use
> > their additional "knowledge" of what the mbufs will be used for to init
> > them in a faster manner than can be done inside the mbuf library.
> >
> > /Bruce
> >
> >> Signed-off-by: Keith Wiles
> >> ---
> >> lib/librte_mbuf/rte_mbuf.h | 77
> >> ++
> >> 1 file changed, 77 insertions(+)
> >>
> >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> >> index 1c6e115..f298621 100644
> >> --- a/lib/librte_mbuf/rte_mbuf.h
> >> +++ b/lib/librte_mbuf/rte_mbuf.h
> >> @@ -546,6 +546,41 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf
> >> *m)
> >> }
> >>
> >> /**
> >> + * @internal Allocate a list of mbufs from mempool *mp*.
> >> + * The use of that function is reserved for RTE internal needs.
> >> + * Please use rte_pktmbuf_alloc_bulk().
> >> + *
> >> + * @param mp
> >> + * The mempool from which mbuf is allocated.
> >> + * @param m_list
> >> + * The array to place the allocated rte_mbufs pointers.
> >> + * @param cnt
> >> + * The number of mbufs to allocate
> >> + * @return
> >> + * - 0 if the number of mbufs allocated was ok
> >> + * - <0 is an ERROR.
> >> + */
> >> +static inline int __rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct
> >> rte_mbuf *m_list[], int cnt)
> >> +{
> >> + struct rte_mbuf *m;
> >> + int ret;
> >> +
> >> + ret = rte_mempool_get_bulk(mp, (void **)m_list, cnt);
> >> + if ( ret == 0 ) {
> >> + int i;
> >> + for(i = 0; i < cnt; i++) {
> >> + m = *m_list++;
> >> +#ifdef RTE_MBUF_REFCNT
> >> + rte_mbuf_refcnt_set(m, 1);
> >> +#endif /* RTE_MBUF_REFCNT */
> >> + rte_pktmbuf_reset(m);
> >> + }
> >> + ret = cnt;
> >> + }
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> * Allocate a new mbuf from a mempool.
> >> *
> >> * This new mbuf contains one segment, which has a length of 0. The pointer
> >> @@ -671,6 +706,32 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
> >> }
> >>
> >> /**
> >> + * Allocate a list of mbufs from a mempool into a mbufs array.
> >> + *
> >> + * This mbuf list contains one segment per mbuf, which has a length of 0.
> >> The
> >> pointer
> >> + * to data is initialized to have some bytes of headroom in the buffer
> >> + * (if buffer size allows).
> >> + *
> >> + * The routine is just a simple wrapper routine to reduce code in the
> >> application
> >> and
> >> + * provide a cleaner API for multiple mbuf requests.
> >> + *
> >> + * @param mp
> >> + * The mempool from which the mbuf is allocated.
> >> + * @param m_list
> >> + * An array of mbuf pointers, cnt must be less then or equal to the
> >> size of the
> >> list.
> >> + * @param cnt
> >> + * Number of slots in the m_list array to fill.
> >> + * @return
> >> + * - The number of valid mbufs pointers in the m_list array.
> >> + * - Zero if the request cnt could not be allocated.
> >> + */
> >> +static inline int __attribute__((always_inline))
> >> +rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
> >> int16_t cnt)
> >> +{
> >> + return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
> >> +}
> >> +
> >> +/**
> >> * Free a segment of a packet mbuf into its original mempool.
> >> *
> >> * Free an mbuf, without parsing other segments in case of chained
> >> @@ -708,6 +769,22 @@ static inline void rte_pktmbuf_free(struct rte_mbuf
> >> *m)
> >> }
> >> }
> >>
> >> +/**
> >> + * Free a list of packet mbufs back into its original mempool.
> >> + *
> >> + * Free a list of mbufs by calling rte_pktmbuf_free() in a loop as a
> >> wr
[dpdk-dev] Clang reporting a problem when adding another member initialization.
On Oct 6, 2014, at 3:54 AM, Thomas Monjalon
wrote:
> 2014-10-03 21:05, Wiles, Roger Keith:
>> I run into a problem with Clang report problem when I tried to add
>> another member to the static initializer of the following in file
>> ixgbe_rxtx_vec.c
> [...]
>> Then I removed the {} and it now builds. Is this a result of the
>> changes to the mbuf structure and Clang being picky?
>>
>> Should I submit a patch to remove the ?{ }? values?
>
> You should check this thread:
> http://dpdk.org/ml/archives/dev/2014-September/005504.html
>
> You may be hitting a compiler bug.
>
Is the placement of the comma in the code the right location, it just does not
look right to me?
We can leave it as is, but we should have added some comment to the code to
explain this one bug.
> --
> Thomas
Keith Wiles, Principal Technologist with CTO office, Wind River mobile
972-213-5533
[dpdk-dev] [PATCH v2 0/4] Update build process
On Mon, Oct 06, 2014 at 10:49:46AM -0400, Neil Horman wrote: > On Mon, Oct 06, 2014 at 11:52:31AM +0100, Sergio Gonzalez Monroy wrote: > > As per the proposal, this patch set does: > > - Remove CONFIG_RTE_BUILD_COMBINE_LIBS as a configuration option. > > - For static library, build a single/combined library. > > - For shared libraries, build both individual/separated and single/combined > >libraries. > > - Link apps only against single/combined libs. > > > > > > Sergio Gonzalez Monroy (4): > > Link combined shared library using CC > > Link apps only against single/combined library > > Update library build process > > Link apps/DSOs against EXECENV_LDLIBS with --as-needed > > > > config/common_bsdapp | 3 +- > > config/common_linuxapp | 3 +- > > mk/rte.app.mk | 164 > > ++--- > > mk/rte.lib.mk | 81 ++-- > > mk/rte.sdkbuild.mk | 2 +- > > mk/rte.sharelib.mk | 54 > > mk/rte.vars.mk | 4 -- > > 7 files changed, 54 insertions(+), 257 deletions(-) > > > > -- > > 1.9.3 > > > > > > I see you removed the --whole-archive option when building the single library > here. Have you checked to make sure that all the constructors haven't been > stripped out? I am not entirely sure I follow. There is no --whole-archive when building libraries, at least not in my sources. The flag is used when linking apps and I have not removed it as you can see on patch 2/4. Sergio > Neil >
[dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
> Sent: Monday, October 06, 2014 3:54 PM
> To: Wiles, Roger Keith (Wind River)
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] Adding the routines
> rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
>
> On Mon, Oct 06, 2014 at 03:50:38PM +0100, Wiles, Roger Keith wrote:
> > Hi Bruce,
> >
> > Do I need to reject the for the new routines or just make sure the vector
> > driver does not get updated to use those routines?
> >
>
> The new routines are probably useful in the general case. I see no issue
> with having them in the code, so long as the vector driver is not modified
> to use them.
I 'd say the same thing for non-vector RX/TX PMD code-paths too.
BTW, are the new functions comments valid?
+ * @return
+ * - 0 if the number of mbufs allocated was ok
+ * - <0 is an ERROR.
+ */
+static inline int __rte_mbuf_raw_alloc_bulk(
Though, as I can see __rte_mbuf_raw_alloc_bulk() returns either:
- number of allocated mbuf (cnt)
- negative error code
And:
+ * @return
+ * - The number of valid mbufs pointers in the m_list array.
+ * - Zero if the request cnt could not be allocated.
+ */
+static inline int __attribute__((always_inline))
+rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
int16_t cnt)
+{
+ return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
+}
Shouldn't be "less than zero if the request cnt could not be allocated."?
BTW, is there any point to have __rte_mbuf_raw_alloc_bulk() at all?
After all, as you are calling rte_pktmbuf_reset() inside it, it doesn't look
__raw__ any more.
Might be just put its content into rte_pktmbuf_alloc_bulk() and get rid of it.
Also wonder, what is the advantage of having multiple counters inside the same
loop?
i.e:
+ for(i = 0; i < cnt; i++) {
+ m = *m_list++;
Why not just:
for(i = 0; i < cnt; i++) {
m = &m_list[i];
Same for free:
+ while(npkts--)
+ rte_pktmbuf_free(*m_list++);
While not just:
for (i = 0; i < npkts; i++)
rte_pktmbuf_free(&m_list[i]);
Konstantin
>
> /Bruce
>
> > Thanks
> > ++Keith
> >
> > On Oct 6, 2014, at 3:56 AM, Richardson, Bruce > intel.com> wrote:
> >
> > >
> > >
> > >> -Original Message-
> > >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Keith Wiles
> > >> Sent: Sunday, October 05, 2014 12:10 AM
> > >> To: dev at dpdk.org
> > >> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines
> > >> rte_pktmbuf_alloc_bulk()
> > >> and rte_pktmbuf_free_bulk()
> > >>
> > >> Minor helper routines to mirror the mempool routines and remove the code
> > >> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
> > >> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
> > >>
> > >
> > > I believe such a change would cause a performance regression, as the
> > > extra init code in the alloc_bulk() function would take
> additional cycles and is not needed. The vector routines use the mempool
> function directly, so that there is no overhead of mbuf
> initialization, as the vector routines use their additional "knowledge" of
> what the mbufs will be used for to init them in a faster manner
> than can be done inside the mbuf library.
> > >
> > > /Bruce
> > >
> > >> Signed-off-by: Keith Wiles
> > >> ---
> > >> lib/librte_mbuf/rte_mbuf.h | 77
> > >> ++
> > >> 1 file changed, 77 insertions(+)
> > >>
> > >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > >> index 1c6e115..f298621 100644
> > >> --- a/lib/librte_mbuf/rte_mbuf.h
> > >> +++ b/lib/librte_mbuf/rte_mbuf.h
> > >> @@ -546,6 +546,41 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf
> > >> *m)
> > >> }
> > >>
> > >> /**
> > >> + * @internal Allocate a list of mbufs from mempool *mp*.
> > >> + * The use of that function is reserved for RTE internal needs.
> > >> + * Please use rte_pktmbuf_alloc_bulk().
> > >> + *
> > >> + * @param mp
> > >> + * The mempool from which mbuf is allocated.
> > >> + * @param m_list
> > >> + * The array to place the allocated rte_mbufs pointers.
> > >> + * @param cnt
> > >> + * The number of mbufs to allocate
> > >> + * @return
> > >> + * - 0 if the number of mbufs allocated was ok
> > >> + * - <0 is an ERROR.
> > >> + */
> > >> +static inline int __rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp,
> > >> struct
> > >> rte_mbuf *m_list[], int cnt)
> > >> +{
> > >> + struct rte_mbuf *m;
> > >> + int ret;
> > >> +
> > >> + ret = rte_mempool_get_bulk(mp, (void **)m_list, cnt);
> > >> + if ( ret == 0 ) {
> > >> + int i;
> > >> + for(i = 0; i < cnt; i++) {
> > >> + m = *m_list++;
> > >> +#ifdef RTE_MBUF_REFCNT
> > >> + rte_mbuf_refcnt_set(m, 1);
> > >> +#endif /* RTE_MBUF_REFCNT */
> > >> + rte_pktmbuf_reset(m);
> > >> +
[dpdk-dev] [PATCH] Pass CC option when building kernel modules
At least on kernels 3.15 or newer, DPDK build is broken for CLANG target. The issue is that the kernel build system sets the flags before including DPDK makefile and therefore assumes the incorrect compiler. Signed-off-by: Sergio Gonzalez Monroy --- mk/rte.module.mk | 2 +- mk/target/generic/rte.vars.mk | 2 ++ mk/toolchain/clang/rte.vars.mk | 5 + mk/toolchain/gcc/rte.vars.mk | 1 + mk/toolchain/icc/rte.vars.mk | 5 + 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/mk/rte.module.mk b/mk/rte.module.mk index c4ca3fd..41c0d0f 100644 --- a/mk/rte.module.mk +++ b/mk/rte.module.mk @@ -78,7 +78,7 @@ build: _postbuild $(MODULE).ko: $(SRCS_LINKS) @if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi @$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \ - CROSS_COMPILE=$(CROSS) + CC=$(KERNELCC) CROSS_COMPILE=$(CROSS) # install module in $(RTE_OUTPUT)/kmod $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.mk index 6020f20..74ff771 100644 --- a/mk/target/generic/rte.vars.mk +++ b/mk/target/generic/rte.vars.mk @@ -149,4 +149,6 @@ endif export CFLAGS export LDFLAGS +else # ! ifeq ($(KERNELRELEASE),) +CC = $(KERNELCC) endif diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk index ee4f451..40cb389 100644 --- a/mk/toolchain/clang/rte.vars.mk +++ b/mk/toolchain/clang/rte.vars.mk @@ -38,11 +38,8 @@ # - define TOOLCHAIN_ASFLAGS variable (overriden by cmdline value) # -ifeq ($(KERNELRELEASE),) CC= $(CROSS)clang -else -CC= $(CROSS)gcc -endif +KERNELCC = $(CROSS)gcc CPP = $(CROSS)cpp # for now, we don't use as but nasm. # AS = $(CROSS)as diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk index 262ebdf..993eb26 100644 --- a/mk/toolchain/gcc/rte.vars.mk +++ b/mk/toolchain/gcc/rte.vars.mk @@ -39,6 +39,7 @@ # CC= $(CROSS)gcc +KERNELCC = $(CROSS)gcc CPP = $(CROSS)cpp # for now, we don't use as but nasm. # AS = $(CROSS)as diff --git a/mk/toolchain/icc/rte.vars.mk b/mk/toolchain/icc/rte.vars.mk index 612370d..f03a2a2 100644 --- a/mk/toolchain/icc/rte.vars.mk +++ b/mk/toolchain/icc/rte.vars.mk @@ -41,11 +41,8 @@ # Warning: we do not use CROSS environment variable as icc is mainly a # x86->x86 compiler -ifeq ($(KERNELRELEASE),) CC= icc -else -CC= gcc -endif +KERNELCC = gcc CPP = cpp AS= nasm AR= ar -- 1.9.3
[dpdk-dev] [PATCH v2 0/4] Update build process
On Mon, Oct 06, 2014 at 04:01:33PM +0100, Sergio Gonzalez Monroy wrote: > On Mon, Oct 06, 2014 at 10:49:46AM -0400, Neil Horman wrote: > > On Mon, Oct 06, 2014 at 11:52:31AM +0100, Sergio Gonzalez Monroy wrote: > > > As per the proposal, this patch set does: > > > - Remove CONFIG_RTE_BUILD_COMBINE_LIBS as a configuration option. > > > - For static library, build a single/combined library. > > > - For shared libraries, build both individual/separated and > > > single/combined > > >libraries. > > > - Link apps only against single/combined libs. > > > > > > > > > Sergio Gonzalez Monroy (4): > > > Link combined shared library using CC > > > Link apps only against single/combined library > > > Update library build process > > > Link apps/DSOs against EXECENV_LDLIBS with --as-needed > > > > > > config/common_bsdapp | 3 +- > > > config/common_linuxapp | 3 +- > > > mk/rte.app.mk | 164 > > > ++--- > > > mk/rte.lib.mk | 81 ++-- > > > mk/rte.sdkbuild.mk | 2 +- > > > mk/rte.sharelib.mk | 54 > > > mk/rte.vars.mk | 4 -- > > > 7 files changed, 54 insertions(+), 257 deletions(-) > > > > > > -- > > > 1.9.3 > > > > > > > > > > I see you removed the --whole-archive option when building the single > > library > > here. Have you checked to make sure that all the constructors haven't been > > stripped out? > > I am not entirely sure I follow. There is no --whole-archive when building > libraries, > at least not in my sources. > The flag is used when linking apps and I have not removed it as you can see > on patch 2/4. > > Sergio You're right, I saw in one of your patches --whole-archive, but it was context, not actual change, sorry Neil > > > Neil > > >
[dpdk-dev] [PATCH] Pass verbose flag to kernel module
--- mk/rte.module.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mk/rte.module.mk b/mk/rte.module.mk index c4ca3fd..bd3c596 100644 --- a/mk/rte.module.mk +++ b/mk/rte.module.mk @@ -78,7 +78,7 @@ build: _postbuild $(MODULE).ko: $(SRCS_LINKS) @if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi @$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \ - CROSS_COMPILE=$(CROSS) + V=$(if $(V),1,0) CROSS_COMPILE=$(CROSS) # install module in $(RTE_OUTPUT)/kmod $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko -- 1.9.3
[dpdk-dev] [PATCH] Pass CC option when building kernel modules
On Mon, Oct 06, 2014 at 04:57:02PM +0100, Sergio Gonzalez Monroy wrote: > At least on kernels 3.15 or newer, DPDK build is broken for CLANG target. > The issue is that the kernel build system sets the flags before including > DPDK makefile and therefore assumes the incorrect compiler. > > Signed-off-by: Sergio Gonzalez Monroy I can confirm that this patch fixes the clang compile for me on Fedora 20 with kernel 3.16.3-200.fc20.x86_64. Acked-by: Bruce Richardson > --- > mk/rte.module.mk | 2 +- > mk/target/generic/rte.vars.mk | 2 ++ > mk/toolchain/clang/rte.vars.mk | 5 + > mk/toolchain/gcc/rte.vars.mk | 1 + > mk/toolchain/icc/rte.vars.mk | 5 + > 5 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/mk/rte.module.mk b/mk/rte.module.mk > index c4ca3fd..41c0d0f 100644 > --- a/mk/rte.module.mk > +++ b/mk/rte.module.mk > @@ -78,7 +78,7 @@ build: _postbuild > $(MODULE).ko: $(SRCS_LINKS) > @if [ ! -f $(notdir Makefile) ]; then ln -nfs $(SRCDIR)/Makefile . ; fi > @$(MAKE) -C $(RTE_KERNELDIR) M=$(CURDIR) O=$(RTE_KERNELDIR) \ > - CROSS_COMPILE=$(CROSS) > + CC=$(KERNELCC) CROSS_COMPILE=$(CROSS) > > # install module in $(RTE_OUTPUT)/kmod > $(RTE_OUTPUT)/kmod/$(MODULE).ko: $(MODULE).ko > diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.mk > index 6020f20..74ff771 100644 > --- a/mk/target/generic/rte.vars.mk > +++ b/mk/target/generic/rte.vars.mk > @@ -149,4 +149,6 @@ endif > export CFLAGS > export LDFLAGS > > +else # ! ifeq ($(KERNELRELEASE),) > +CC = $(KERNELCC) > endif > diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk > index ee4f451..40cb389 100644 > --- a/mk/toolchain/clang/rte.vars.mk > +++ b/mk/toolchain/clang/rte.vars.mk > @@ -38,11 +38,8 @@ > # - define TOOLCHAIN_ASFLAGS variable (overriden by cmdline value) > # > > -ifeq ($(KERNELRELEASE),) > CC= $(CROSS)clang > -else > -CC= $(CROSS)gcc > -endif > +KERNELCC = $(CROSS)gcc > CPP = $(CROSS)cpp > # for now, we don't use as but nasm. > # AS = $(CROSS)as > diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk > index 262ebdf..993eb26 100644 > --- a/mk/toolchain/gcc/rte.vars.mk > +++ b/mk/toolchain/gcc/rte.vars.mk > @@ -39,6 +39,7 @@ > # > > CC= $(CROSS)gcc > +KERNELCC = $(CROSS)gcc > CPP = $(CROSS)cpp > # for now, we don't use as but nasm. > # AS = $(CROSS)as > diff --git a/mk/toolchain/icc/rte.vars.mk b/mk/toolchain/icc/rte.vars.mk > index 612370d..f03a2a2 100644 > --- a/mk/toolchain/icc/rte.vars.mk > +++ b/mk/toolchain/icc/rte.vars.mk > @@ -41,11 +41,8 @@ > # Warning: we do not use CROSS environment variable as icc is mainly a > # x86->x86 compiler > > -ifeq ($(KERNELRELEASE),) > CC= icc > -else > -CC= gcc > -endif > +KERNELCC = gcc > CPP = cpp > AS= nasm > AR= ar > -- > 1.9.3 >
[dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
On Oct 6, 2014, at 10:54 AM, Ananyev, Konstantin wrote:
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
>> Sent: Monday, October 06, 2014 3:54 PM
>> To: Wiles, Roger Keith (Wind River)
>> Cc: dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 2/2] Adding the routines
>> rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
>>
>> On Mon, Oct 06, 2014 at 03:50:38PM +0100, Wiles, Roger Keith wrote:
>>> Hi Bruce,
>>>
>>> Do I need to reject the for the new routines or just make sure the vector
>>> driver does not get updated to use those routines?
>>>
>>
>> The new routines are probably useful in the general case. I see no issue
>> with having them in the code, so long as the vector driver is not modified
>> to use them.
>
> I 'd say the same thing for non-vector RX/TX PMD code-paths too.
>
> BTW, are the new functions comments valid?
>
> + * @return
> + * - 0 if the number of mbufs allocated was ok
> + * - <0 is an ERROR.
> + */
> +static inline int __rte_mbuf_raw_alloc_bulk(
>
> Though, as I can see __rte_mbuf_raw_alloc_bulk() returns either:
> - number of allocated mbuf (cnt)
> - negative error code
Let me fix up the comments.
>
> And:
> + * @return
> + * - The number of valid mbufs pointers in the m_list array.
> + * - Zero if the request cnt could not be allocated.
> + */
> +static inline int __attribute__((always_inline))
> +rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
> int16_t cnt)
> +{
> + return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
> +}
>
> Shouldn't be "less than zero if the request cnt could not be allocated."?
>
> BTW, is there any point to have __rte_mbuf_raw_alloc_bulk() at all?
> After all, as you are calling rte_pktmbuf_reset() inside it, it doesn't look
> __raw__ any more.
> Might be just put its content into rte_pktmbuf_alloc_bulk() and get rid of it.
>
I was just following the non-bulk routine style __rte_mbuf_raw_alloc(), but I
can pull that into a single routine.
> Also wonder, what is the advantage of having multiple counters inside the
> same loop?
> i.e:
> + for(i = 0; i < cnt; i++) {
> + m = *m_list++;
>
> Why not just:
>
> for(i = 0; i < cnt; i++) {
>m = &m_list[i];
>
> Same for free:
> + while(npkts--)
> + rte_pktmbuf_free(*m_list++);
>
> While not just:
> for (i = 0; i < npkts; i++)
> rte_pktmbuf_free(&m_list[i]);
Maybe I have it wrong or the compilers are doing the right thing now, but at
one point the &m_list[i] would cause the compiler to generate a shift or
multiple of ?i? and then add it to the base of m_list. If that is not the case
anymore then I can update the code as you suggested. Using the *m_list++ just
adds the size of a pointer to a register and continues.
>
> Konstantin
>
>>
>> /Bruce
>>
>>> Thanks
>>> ++Keith
>>>
>>> On Oct 6, 2014, at 3:56 AM, Richardson, Bruce >> intel.com> wrote:
>>>
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Keith Wiles
> Sent: Sunday, October 05, 2014 12:10 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines
> rte_pktmbuf_alloc_bulk()
> and rte_pktmbuf_free_bulk()
>
> Minor helper routines to mirror the mempool routines and remove the code
> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
>
I believe such a change would cause a performance regression, as the extra
init code in the alloc_bulk() function would take
>> additional cycles and is not needed. The vector routines use the mempool
>> function directly, so that there is no overhead of mbuf
>> initialization, as the vector routines use their additional "knowledge" of
>> what the mbufs will be used for to init them in a faster manner
>> than can be done inside the mbuf library.
/Bruce
> Signed-off-by: Keith Wiles
> ---
> lib/librte_mbuf/rte_mbuf.h | 77
> ++
> 1 file changed, 77 insertions(+)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 1c6e115..f298621 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -546,6 +546,41 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf
> *m)
> }
>
> /**
> + * @internal Allocate a list of mbufs from mempool *mp*.
> + * The use of that function is reserved for RTE internal needs.
> + * Please use rte_pktmbuf_alloc_bulk().
> + *
> + * @param mp
> + * The mempool from which mbuf is allocated.
> + * @param m_list
> + * The array to place the allocated rte_mbufs pointers.
> + * @param cnt
> + * The number of mbufs to allocate
> + * @return
> + * - 0 if the number of mbufs allocated was ok
> + * - <0
[dpdk-dev] [PATCH v3] distributor_app: new sample app
> -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Monday, October 6, 2014 3:45 PM > To: Pattan, Reshma > Cc: dev at dpdk.org; Richardson, Bruce > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > On Mon, Oct 06, 2014 at 02:16:22PM +, Pattan, Reshma wrote: > > > > > > > -Original Message- > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > Sent: Wednesday, October 1, 2014 5:08 PM > > > To: Richardson, Bruce > > > Cc: Pattan, Reshma; dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > > > > > > > > > > > > > > 1)I had sent v5 patch which handles graceful shutdown of rx > > > > > > and tx threads upon SIGINT > > > > > I see it and will take a look shortly, thanks. > > > > > > > > > > > 2)Worker thread graceful shutdown was not handled as of now as > > > > > > it needs > > > some change in lcore_worker logic , which will be done in future > enhancements. > > > > > Not sure I understand what you mean here. Can you elaborate? > > > > > > > rte_distributor_process which runs as part of rx thread will process > > incoming > packets and checks for any requests for the packets from worker threads . > > If request is seen, it adds the packet/work to particular workers back log > > and > proceed with processing of next packet. > > If no request seen the packet index will not be incremented and the while > > loop > which is conditionally based on packet indexing runs in a continuous loop > without breaking and rx thread will not proceed with next statement execution > until unless rte_distributor_process comes out of while loop. > > This issue happens only when we enable graceful shutdown logic for both > rx/worker threads, as workers threads gets killed and no request seen by rx > thread and it stucks. > > Hence as of now graceful shutdown logic is provided only for rx thread. For > worker threads will check what can be done in next enhancements. > > > > Thanks, > > Reshma > > > > I see what you're saying, Once you make a call to rte_distributor_get_pkt, you > have no way to gracefully shut down use of the rte_distributor_library. Not > just > this application, but any application. Thats just not sane, and suggests > that we > integrated the rte_distributor library too soon. I would suggest that you > prefix > this patch with an update to the rte distributor library to allow > rte_distributor_get_pkt and friends to return NULL if the queue is emtpy. > Applications should be checking the return value for NULL anyway, and can > preform the rte_pause operation. Then update this patch to do a clean exit. > To > say that "we will check what can be done in the next enhancements" is to say > that this won't be addressed again until a paying custmoer gripes about it. > Neil > Hi Neil, We have rte_distributor_request_pkt and rte_distributor_poll_pkt() in dpdk.org, which can be used together(in place of rte_distributor_get_pkt) to check empty queue condition I believe. So no separate changes needed. Though we replace to rte_distributor_get_pkt with above two, the issue will not be solved as the thread that gets blocked is rx thread but not worker thread. Rx thread gets blocked in rte_distributore_process due to non-availability of requests from worker. How about sending dummy request_pkts upon SIGINT and allow rte_distributore_process to get to completion? Thanks, Reshma > > > > > > 3)Freeing of mempool is also not handled , as the framework > > > > > > support is not > > > available. > > > > > Ew, I hadn't noticed that, freeing of mempools seems like > > > > > something we should implement. > > > > > > > > > > > 4)Cleaning of rx/tx queues not done, as it needs some > > > > > > extensive logic > > > which we haven't planned as of now. Will check the possibility of doing > > > it in > > > future enhancementsi.e in next version of sample application. > > > > > We can't just flush the queues after we shutdown the workers? I > > > > > presume a queue flush operation exists, yes? > > > > > Neil > > > > > > > > Other than code hygiene, which does have some value in itself, I > > > > can't really see what the practical point of such cleanup would be. > > > > > > > This is really the only assertion I'm trying to make. I understand > > > this application won't suffer from exiting uncleanly, and that makes > > > the need for preforming cleanup little more than overhead. > > > > > > But that said, hygine is exactly the point I'm driving at here. > > > These are example applications, that presumably people look at when > > > writing their own apps. If you don't do things properly, people > > > looking at your code are less likely to do them as well. Even if it > > > doesn't hurt for you to exit uncleanly, it will hurt someone, and if > > > they look to these examples as a source of best practices, it seems > > > to me that it would be in everyones interest, if best prac
[dpdk-dev] section mismatch warnings
Hi Everyone, When we enable kernel config CONFIG_DEBUG_SECTION_MISMATCH=y and compile DPDK 1.7 with 3.14.17 kernel + gcc 4.8.2, we got lots of warnings like these. Do we have plan to fix them? If so, please advise on which version. Thanks. == Build lib/librte_eal/linuxapp/kni cut: /proc/version_signature: No such file or directory LD /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/built-in.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.o Building modules, stage 2. MODPOST 1 modules WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.o(.data+0x20): Section mismatch in reference from the variable igbuio_pci_driver to the function .text:igbuio_pci_probe() WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.o(.data+0x28): Section mismatch in reference from the variable igbuio_pci_driver to the function .text:igbuio_pci_remove() WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.o(.data+0x130): Section mismatch in reference from the variable dev_attr_max_vfs to the function .text:show_max_vfs() WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.o(.data+0x138): Section mismatch in reference from the variable dev_attr_max_vfs to the function .text:store_max_vfs() CC /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.mod.o LD [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko INSTALL-MODULE igb_uio.ko LD /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/built-in.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_main.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_api.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_common.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_ethtool.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_82599.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_82598.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_x540.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/ixgbe_phy.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/kcompat.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_82575.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_i210.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_api.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_mac.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_manage.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_mbx.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_nvm.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/e1000_phy.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_ethtool.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_hwmon.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_main.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_debugfs.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_param.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_procfs.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_vmdq.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/kni_misc.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/kni_net.o CC [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/kni_ethtool.o LD [M] /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/rte_kni.o WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/rte_kni.o(.data+0x20): Section mismatch in reference from the variable ixgbe_ethtool_ops to the function .text:ixgbe_get_settings() WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/rte_kni.o(.data+0x28): Section mismatch in reference from the variable ixgbe_ethtool_ops to the function .text:ixgbe_set_settings() WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/rte_kni.o(.data+0x30): Section mismatch in reference from the variable ixgbe_ethtool_ops to the function .text:ixgbe_get_drvinfo() WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/rte_kni.o(.data+0x38): Section mismatch in reference from the variable ixgbe_ethtool_ops to the function .text:ixgbe_get_regs_len() WARNING: /git/Pktgen-DPDK/dpdk/build/build/lib/librte_eal/linuxapp/kni/rte_kni.o(.data+0x40)
[dpdk-dev] [PATCH v3] distributor_app: new sample app
On Mon, Oct 06, 2014 at 05:34:13PM +, Pattan, Reshma wrote: > > > > -Original Message- > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > Sent: Monday, October 6, 2014 3:45 PM > > To: Pattan, Reshma > > Cc: dev at dpdk.org; Richardson, Bruce > > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > > > On Mon, Oct 06, 2014 at 02:16:22PM +, Pattan, Reshma wrote: > > > > > > > > > > -Original Message- > > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > > Sent: Wednesday, October 1, 2014 5:08 PM > > > > To: Richardson, Bruce > > > > Cc: Pattan, Reshma; dev at dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > > > > > > > > > > > > > > > > > 1)I had sent v5 patch which handles graceful shutdown of rx > > > > > > > and tx threads upon SIGINT > > > > > > I see it and will take a look shortly, thanks. > > > > > > > > > > > > > 2)Worker thread graceful shutdown was not handled as of now as > > > > > > > it needs > > > > some change in lcore_worker logic , which will be done in future > > enhancements. > > > > > > Not sure I understand what you mean here. Can you elaborate? > > > > > > > > > rte_distributor_process which runs as part of rx thread will process > > > incoming > > packets and checks for any requests for the packets from worker threads . > > > If request is seen, it adds the packet/work to particular workers back > > > log and > > proceed with processing of next packet. > > > If no request seen the packet index will not be incremented and the > > > while loop > > which is conditionally based on packet indexing runs in a continuous loop > > without breaking and rx thread will not proceed with next statement > > execution > > until unless rte_distributor_process comes out of while loop. > > > This issue happens only when we enable graceful shutdown logic for both > > rx/worker threads, as workers threads gets killed and no request seen by rx > > thread and it stucks. > > > Hence as of now graceful shutdown logic is provided only for rx thread. > > > For > > worker threads will check what can be done in next enhancements. > > > > > > Thanks, > > > Reshma > > > > > > > I see what you're saying, Once you make a call to rte_distributor_get_pkt, > > you > > have no way to gracefully shut down use of the rte_distributor_library. Not > > just > > this application, but any application. Thats just not sane, and suggests > > that we > > integrated the rte_distributor library too soon. I would suggest that you > > prefix > > this patch with an update to the rte distributor library to allow > > rte_distributor_get_pkt and friends to return NULL if the queue is emtpy. > > Applications should be checking the return value for NULL anyway, and can > > preform the rte_pause operation. Then update this patch to do a clean > > exit. To > > say that "we will check what can be done in the next enhancements" is to say > > that this won't be addressed again until a paying custmoer gripes about it. > > Neil > > > > Hi Neil, > > We have rte_distributor_request_pkt and rte_distributor_poll_pkt() in > dpdk.org, which can be used together(in place of rte_distributor_get_pkt) > to check empty queue condition I believe. So no separate changes needed. > Though we replace to rte_distributor_get_pkt with above two, the issue will > not be solved as the thread that gets blocked is rx thread but not worker > thread. > Rx thread gets blocked in rte_distributore_process due to non-availability of > requests from worker. > How about sending dummy request_pkts upon SIGINT and allow > rte_distributore_process to get to completion? > I suppose creating a flagged dummy packet to ensure that the worker threads can exit their spin loops works fine, yes. Neil > Thanks, > Reshma > > > > > > > > 3)Freeing of mempool is also not handled , as the framework > > > > > > > support is not > > > > available. > > > > > > Ew, I hadn't noticed that, freeing of mempools seems like > > > > > > something we should implement. > > > > > > > > > > > > > 4)Cleaning of rx/tx queues not done, as it needs some > > > > > > > extensive logic > > > > which we haven't planned as of now. Will check the possibility of doing > > > > it in > > > > future enhancementsi.e in next version of sample application. > > > > > > We can't just flush the queues after we shutdown the workers? I > > > > > > presume a queue flush operation exists, yes? > > > > > > Neil > > > > > > > > > > Other than code hygiene, which does have some value in itself, I > > > > > can't really see what the practical point of such cleanup would be. > > > > > > > > > This is really the only assertion I'm trying to make. I understand > > > > this application won't suffer from exiting uncleanly, and that makes > > > > the need for preforming cleanup little more than overhead. > > > > > > > > But that said, hygine is exactly the point I'm driving at here. > > > > The
[dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
On Oct 6, 2014, at 11:13 AM, Wiles, Roger Keith
wrote:
>
> On Oct 6, 2014, at 10:54 AM, Ananyev, Konstantin intel.com> wrote:
>
>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
>>> Sent: Monday, October 06, 2014 3:54 PM
>>> To: Wiles, Roger Keith (Wind River)
>>> Cc: dev at dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH 2/2] Adding the routines
>>> rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
>>>
>>> On Mon, Oct 06, 2014 at 03:50:38PM +0100, Wiles, Roger Keith wrote:
Hi Bruce,
Do I need to reject the for the new routines or just make sure the vector
driver does not get updated to use those routines?
>>>
>>> The new routines are probably useful in the general case. I see no issue
>>> with having them in the code, so long as the vector driver is not modified
>>> to use them.
>>
>> I 'd say the same thing for non-vector RX/TX PMD code-paths too.
>>
>> BTW, are the new functions comments valid?
>>
>> + * @return
>> + * - 0 if the number of mbufs allocated was ok
>> + * - <0 is an ERROR.
>> + */
>> +static inline int __rte_mbuf_raw_alloc_bulk(
>>
>> Though, as I can see __rte_mbuf_raw_alloc_bulk() returns either:
>> - number of allocated mbuf (cnt)
>> - negative error code
>
> Let me fix up the comments.
>>
>> And:
>> + * @return
>> + * - The number of valid mbufs pointers in the m_list array.
>> + * - Zero if the request cnt could not be allocated.
>> + */
>> +static inline int __attribute__((always_inline))
>> +rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
>> int16_t cnt)
>> +{
>> + return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
>> +}
>>
>> Shouldn't be "less than zero if the request cnt could not be allocated."?
>>
>> BTW, is there any point to have __rte_mbuf_raw_alloc_bulk() at all?
>> After all, as you are calling rte_pktmbuf_reset() inside it, it doesn't look
>> __raw__ any more.
>> Might be just put its content into rte_pktmbuf_alloc_bulk() and get rid of
>> it.
>>
> I was just following the non-bulk routine style __rte_mbuf_raw_alloc(), but I
> can pull that into a single routine.
>
>> Also wonder, what is the advantage of having multiple counters inside the
>> same loop?
>> i.e:
>> + for(i = 0; i < cnt; i++) {
>> + m = *m_list++;
>>
>> Why not just:
>>
>> for(i = 0; i < cnt; i++) {
>> m = &m_list[i];
>>
>> Same for free:
>> + while(npkts--)
>> + rte_pktmbuf_free(*m_list++);
>>
>> While not just:
>> for (i = 0; i < npkts; i++)
>> rte_pktmbuf_free(&m_list[i]);
>
> Maybe I have it wrong or the compilers are doing the right thing now, but at
> one point the &m_list[i] would cause the compiler to generate a shift or
> multiple of ?i? and then add it to the base of m_list. If that is not the
> case anymore then I can update the code as you suggested. Using the *m_list++
> just adds the size of a pointer to a register and continues.
I compared the clang assembler (.s file) output from an example test code I
wrote to see if we have any differences in the code using the two styles and I
found no difference and the code looked the same. I am not a Intel assembler
expert and I would suggest someone else determine if it generates different
code. I tried to compare the GCC outputs and it did look the same to me.
I have attached the code and output, please let me know if I did something
wrong, but as it stands using the original style is what I want to go with.
>>
>> Konstantin
>>
>>>
>>> /Bruce
>>>
Thanks
++Keith
On Oct 6, 2014, at 3:56 AM, Richardson, Bruce >>> intel.com> wrote:
>
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Keith Wiles
>> Sent: Sunday, October 05, 2014 12:10 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines
>> rte_pktmbuf_alloc_bulk()
>> and rte_pktmbuf_free_bulk()
>>
>> Minor helper routines to mirror the mempool routines and remove the code
>> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
>> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
>>
>
> I believe such a change would cause a performance regression, as the
> extra init code in the alloc_bulk() function would take
>>> additional cycles and is not needed. The vector routines use the mempool
>>> function directly, so that there is no overhead of mbuf
>>> initialization, as the vector routines use their additional "knowledge" of
>>> what the mbufs will be used for to init them in a faster manner
>>> than can be done inside the mbuf library.
>
> /Bruce
>
>> Signed-off-by: Keith Wiles
>> ---
>> lib/librte_mbuf/rte_mbuf.h | 77
>> ++
>> 1 file changed, 77 insertions(+)
>>
>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
>> index 1c6e115.
[dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
Attaching to the list does not work. If you want the code let me know it is
only about 5K in size.
On Oct 6, 2014, at 2:45 PM, Wiles, Roger Keith
wrote:
>
> On Oct 6, 2014, at 11:13 AM, Wiles, Roger Keith windriver.com> wrote:
>
>>
>> On Oct 6, 2014, at 10:54 AM, Ananyev, Konstantin > intel.com> wrote:
>>
From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
Sent: Monday, October 06, 2014 3:54 PM
To: Wiles, Roger Keith (Wind River)
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH 2/2] Adding the routines
rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
On Mon, Oct 06, 2014 at 03:50:38PM +0100, Wiles, Roger Keith wrote:
> Hi Bruce,
>
> Do I need to reject the for the new routines or just make sure the vector
> driver does not get updated to use those routines?
>
The new routines are probably useful in the general case. I see no issue
with having them in the code, so long as the vector driver is not modified
to use them.
>>>
>>> I 'd say the same thing for non-vector RX/TX PMD code-paths too.
>>>
>>> BTW, are the new functions comments valid?
>>>
>>> + * @return
>>> + * - 0 if the number of mbufs allocated was ok
>>> + * - <0 is an ERROR.
>>> + */
>>> +static inline int __rte_mbuf_raw_alloc_bulk(
>>>
>>> Though, as I can see __rte_mbuf_raw_alloc_bulk() returns either:
>>> - number of allocated mbuf (cnt)
>>> - negative error code
>>
>> Let me fix up the comments.
>>>
>>> And:
>>> + * @return
>>> + * - The number of valid mbufs pointers in the m_list array.
>>> + * - Zero if the request cnt could not be allocated.
>>> + */
>>> +static inline int __attribute__((always_inline))
>>> +rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
>>> int16_t cnt)
>>> +{
>>> + return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
>>> +}
>>>
>>> Shouldn't be "less than zero if the request cnt could not be allocated."?
>>>
>>> BTW, is there any point to have __rte_mbuf_raw_alloc_bulk() at all?
>>> After all, as you are calling rte_pktmbuf_reset() inside it, it doesn't
>>> look __raw__ any more.
>>> Might be just put its content into rte_pktmbuf_alloc_bulk() and get rid of
>>> it.
>>>
>> I was just following the non-bulk routine style __rte_mbuf_raw_alloc(), but
>> I can pull that into a single routine.
>>
>>> Also wonder, what is the advantage of having multiple counters inside the
>>> same loop?
>>> i.e:
>>> + for(i = 0; i < cnt; i++) {
>>> + m = *m_list++;
>>>
>>> Why not just:
>>>
>>> for(i = 0; i < cnt; i++) {
>>> m = &m_list[i];
>>>
>>> Same for free:
>>> + while(npkts--)
>>> + rte_pktmbuf_free(*m_list++);
>>>
>>> While not just:
>>> for (i = 0; i < npkts; i++)
>>>rte_pktmbuf_free(&m_list[i]);
>>
>> Maybe I have it wrong or the compilers are doing the right thing now, but at
>> one point the &m_list[i] would cause the compiler to generate a shift or
>> multiple of ?i? and then add it to the base of m_list. If that is not the
>> case anymore then I can update the code as you suggested. Using the
>> *m_list++ just adds the size of a pointer to a register and continues.
>
> I compared the clang assembler (.s file) output from an example test code I
> wrote to see if we have any differences in the code using the two styles and
> I found no difference and the code looked the same. I am not a Intel
> assembler expert and I would suggest someone else determine if it generates
> different code. I tried to compare the GCC outputs and it did look the same
> to me.
>
> I have attached the code and output, please let me know if I did something
> wrong, but as it stands using the original style is what I want to go with.
>
>>>
>>> Konstantin
>>>
/Bruce
> Thanks
> ++Keith
>
> On Oct 6, 2014, at 3:56 AM, Richardson, Bruce intel.com> wrote:
>
>>
>>
>>> -Original Message-
>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Keith Wiles
>>> Sent: Sunday, October 05, 2014 12:10 AM
>>> To: dev at dpdk.org
>>> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines
>>> rte_pktmbuf_alloc_bulk()
>>> and rte_pktmbuf_free_bulk()
>>>
>>> Minor helper routines to mirror the mempool routines and remove the code
>>> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
>>> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
>>>
>>
>> I believe such a change would cause a performance regression, as the
>> extra init code in the alloc_bulk() function would take
additional cycles and is not needed. The vector routines use the mempool
function directly, so that there is no overhead of mbuf
initialization, as the vector routines use their additional "knowledge" of
what the mbufs will be used for to init them in a faster manner
[dpdk-dev] [PATCH v2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()
Minor helper routines to mirror the mempool routines and remove the code
from applications. The ixgbe_rxtx_vec.c routine could be changed to use
the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
Combined __rte_mbuf_raw_alloc_bulk into the rte_pktmbuf_alloc_bulk function.
Fixed up the comments to state the correct return values.
Signed-off-by: Keith Wiles
---
lib/librte_mbuf/rte_mbuf.h | 55 ++
1 file changed, 55 insertions(+)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 1c6e115..337611b 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -671,6 +671,44 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
}
/**
+ * Allocate a list of mbufs from a mempool into a mbuf array.
+ *
+ * This mbuf list contains one segment per mbuf, which has a length of 0. The
pointer
+ * to data is initialized to have some bytes of headroom in the buffer
+ * (if buffer size allows).
+ *
+ * The routine is just a simple wrapper routine to reduce code in the
application and
+ * provide a cleaner API for multiple mbuf requests.
+ *
+ * @param mp
+ * The mempool from which the mbuf is allocated.
+ * @param m_list
+ * An array of mbuf pointers, cnt must be less then or equal to the size of
the array.
+ * @param cnt
+ * Number of slots in the m_list array to fill.
+ * @return
+ * - cnt is returned if a successful allocation.
+ * - <0 negative number is an error code.
+ */
+static inline int __attribute__((always_inline))
+rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
int16_t cnt)
+{
+ int ret;
+
+ ret = rte_mempool_get_bulk(mp, (void **)m_list, cnt);
+ if ( ret == 0 ) {
+ ret = cnt;
+ while(cnt--) {
+#ifdef RTE_MBUF_REFCNT
+ rte_mbuf_refcnt_set(*m_list, 1);
+#endif /* RTE_MBUF_REFCNT */
+ rte_pktmbuf_reset(*m_list++);
+ }
+ }
+ return ret;
+}
+
+/**
* Free a segment of a packet mbuf into its original mempool.
*
* Free an mbuf, without parsing other segments in case of chained
@@ -708,6 +746,23 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
}
}
+/**
+ * Free a list of packet mbufs back into its original mempool.
+ *
+ * Free a list of mbufs by calling rte_pktmbuf_free() in a loop as a wrapper
function.
+ *
+ * @param m_list
+ * An array of rte_mbuf pointers to be freed.
+ * @param npkts
+ * Number of packets to free in m_list.
+ */
+static inline void __attribute__((always_inline))
+rte_pktmbuf_free_bulk(struct rte_mbuf *m_list[], int16_t npkts)
+{
+ while(npkts--)
+ rte_pktmbuf_free(*m_list++);
+}
+
#ifdef RTE_MBUF_REFCNT
/**
--
2.1.0
[dpdk-dev] [PATCH v2 3/4] Update library build process
On Mon, Oct 06, 2014 at 11:52:34AM +0100, Sergio Gonzalez Monroy wrote: > Remove COMBINE_LIBS option and by default build: > - CONFIG_RTE_BUILD_SHARED_LIB=y : both individual and combined libraries > - CONFIG_RTE_BUILD_SHARED_LIB=n : single combined library As previously discussed.,It would be better for backward-compatibility of people linking against DPDK, if the static lib could come out as both a combined library and separate individual libraries by default. Otherwise everybody linking against DPDK has to change their code, and it won't be easy for them to move forward and backward against different DPDKs before and after this change. Thanks, Matthew.
[dpdk-dev] [PATCH] mk: link combined shared library with compiler to enable elf ctors
From: Michal Bella Signed-off-by: michal --- mk/rte.sharelib.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk index c0a811a..a315d98 100644 --- a/mk/rte.sharelib.mk +++ b/mk/rte.sharelib.mk @@ -45,7 +45,7 @@ sharelib: $(LIB_ONE) FORCE OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o) -O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE) +O_TO_S = $(CC) $(CPU_LDFLAGS) -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE) O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)"," LD $(@)") O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)" -- 1.9.1
[dpdk-dev] [PATCH 0/7] Patches to split architecture specific operations from DPDK
On 9/26/2014 2:33 AM, Chao Zhu wrote: > The set of patches split x86 architecture specific operations from DPDK and > put them to the > arch directories of i686 and x86_64 architecture. This will make the adpotion > of DPDK much easier > on other computer architecture. For a new architecture, just add an > architecture specific > directory and necessary building configuration files, then DPDK can support > it. Wouldn't the SSE specifics in rte_common.h and rte_common_vect.h need to be similarly split out into architecture specifics? Thanks -- Cyril.
[dpdk-dev] [PATCH 09/12] Remove iopl operation for IBM Power architecture
On 9/26/2014 2:36 AM, Chao Zhu wrote:
> iopl() call is mostly for the i386 architecture. In Power architecture.
> It doesn't exist. This patch modified rte_eal_iopl_init() and make it
> return -1 on Power. This means rte_config.flags will not contain
> EAL_FLG_HIGH_IOPL flag on IBM Power architecture.
Since iopl() is an x86-only thing, shouldn't the code be conditional on
defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) instead of below?
Better still, should we maybe break out an architecture specific init
function? This function could set iopl on x86, and possibly do other
lowlevel init things on other architectures...
> Signed-off-by: Chao Zhu
> ---
> lib/librte_eal/linuxapp/eal/eal.c | 11 +++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 4869e7c..8cc1f21 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -50,7 +50,10 @@
> #include
> #include
> #include
> +/* Power architecture doesn't have this header file */
> +#ifndef RTE_ARCH_PPC_64
> #include
> +#endif
>
> #include
> #include
> @@ -1019,11 +1022,19 @@ rte_eal_mcfg_complete(void)
>
> /*
>* Request iopl privilege for all RPL, returns 0 on success
> + *
> + * Power architecture doesn't have iopl function, so this function
> + * return -1 on Power architecture, because this function is only used
> + * in rte_eal_init to add EAL_FLG_HIGH_IOPL to rte_config.flags.
>*/
> static int
> rte_eal_iopl_init(void)
> {
> +#ifndef RTE_ARCH_PPC_64
> return iopl(HIGHEST_RPL);
> +#else
> + return -1;
> +#endif
> }
>
> /* Launch threads, called at application init(). */
