Re: [dpdk-dev] [PATCH] mbuf: remove redundant line in rte_pktmbuf_attach
> > > >> mi->next will be assigned to NULL few lines later, trivial patch
> > > >>
> > > >> Signed-off-by: Ilya V. Matveychikov
>
> Fixes: ea672a8b1655 ("mbuf: remove the rte_pktmbuf structure")
>
> Acked-by: Olivier Matz
Applied, thanks
Re: [dpdk-dev] [PATCH v5] ethdev: fix MAC address replay
2017-01-27 09:57, Steve Shin:
> This patch fixes a bug in replaying MAC address to the hardware
> in rte_eth_dev_config_restore() routine. Added default MAC replay as well.
>
> Fixes: 4bdefaade6d1 ("ethdev: VMDQ enhancements")
>
> ---
> v2: Added default MAC replay & Code optimization.
> v3: Covered a case (ex, SR-IOV) where multiple pools
> exist in the mac_pool_sel array.
> v4: removed a coding style warning.
> v5: Added default MAC replay with dev_ops->mac_addr_add.
The changelog should be after the SoB,
because everything after --- is removed when applying.
> Signed-off-by: Steve Shin
I've added
Reviewed-by: Igor Ryzhov
Applied, thanks
Re: [dpdk-dev] [PATCH v2] app/testpmd: fix memory leak
Hi Pablo,
On Fri, Jan 27, 2017 at 02:54:58PM +, Pablo de Lara wrote:
> Free memory when port flow entry creation fails.
>
> Coverity issue: 139600
> Fixes: 938a184a1870 ("app/testpmd: implement basic support for flow API")
>
> Signed-off-by: Pablo de Lara
> ---
> Changes in v2:
>
> - Removed unnecessary conditional
>
> app/test-pmd/config.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 5834498..467932f 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -954,6 +954,7 @@ port_flow_new(const struct rte_flow_attr *attr,
> goto store;
> }
> notsup:
> + free(pf);
> rte_errno = err;
> return NULL;
> }
> --
> 2.7.4
>
I think this is a false positive, which is why I did not address it during
the last round of Coverity issues.
As a two-pass function, errors are checked during the first pass, when pf is
not allocated yet. During the second pass, intermediate functions are not
supposed to return a different result and "notsup" cannot occur.
I think assert()/rte_assert() would make more sense and should fool Coverity
into understanding the expected behavior.
The commit log should reflect that we are addressing a false positive since
there is no problem with the code logic (of course unless I missed anything
obvious).
Thanks.
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [PATCH] ethdev: remove useless pointer initialization
2017-01-24 21:28, Emmanuel Roullit:
> Found with clang static analysis:
> lib/librte_ether/rte_ethdev.c:2467:22:
> warning: Value stored to 'dev' during its initialization is never read
> struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> ^~~ ~
>
> Fixes: 61207d014fc9 ("ethdev: fix data reset when allocating port")
Wrong commit reference.
Fixes: 88ac4396ad29 ("ethdev: add VMDq support")
> Signed-off-by: Emmanuel Roullit
Applied, thanks
Re: [dpdk-dev] [PATCH] mk: parallelize make config
On 1/23/2017 7:03 PM, Michał Mirosław wrote: > 2017-01-22 2:50 GMT+01:00 Ferruh Yigit : >> make config dependency resolving was always running serial, >> parallelize it for better performance. >> >> $ time make T=x86_64-native-linuxapp-gcc config >> real0m12.633s >> >> $ time make -j8 T=x86_64-native-linuxapp-gcc config >> real0m1.826s > [...] >> +$(RTE_OUTPUT)/.depdirs: $(DEPDIRS) >> + @rm -f $@ >> + @for f in $(DEPDIRS); do cat $$f >> $@; done >> + @sort -u -o $@ $@ > > This could be just one 'sort -u -o $@ $(DEPDIRS)' Yes, I will update in next version, thanks. > > Best Regards, > Michał Mirosław >
Re: [dpdk-dev] [PATCH] mk: parallelize make config
On 1/29/2017 3:29 PM, Thomas Monjalon wrote: > 2017-01-22 01:50, Ferruh Yigit: >> make config dependency resolving was always running serial, >> parallelize it for better performance. > > It could be interesting to explain why it was not parallelized, > and how you made it possible. I will try to explain in next version. > > The test script should be updated as below: > > --- a/devtools/test-build.sh > +++ b/devtools/test-build.sh > - make T=$2 O=$1 config > + make -j$J T=$2 O=$1 config I will update. > > >> --- a/mk/internal/rte.depdirs-post.mk >> +++ b/mk/internal/rte.depdirs-post.mk >> @@ -29,11 +29,12 @@ >> # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE >> # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> >> -.PHONY: depdirs >> -depdirs: >> -@for d in $(DEPDIRS-y); do \ >> -$(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $$d ; \ >> -done >> +.PHONY: depdirs $(DEPDIRS-y) >> +depdirs: $(DEPDIRS-y) >> +@echo "" > > Why this echo "" ? If there is no dependency (DEPDIRS-y) set and there is nothing to do here, make prints a line like "nothing to do", since we redirect the output to the files, that msg goes into .depdir files. To prevent that msg, added a dummy action here. > >> + >> +$(DEPDIRS-y): >> +@$(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $@ > > >> --- a/mk/rte.sdkdepdirs.mk >> +++ b/mk/rte.sdkdepdirs.mk >> @@ -36,19 +36,22 @@ ifeq (,$(wildcard $(RTE_OUTPUT)/Makefile)) >>$(error "need a make config first") >> endif >> >> +DEPDIRS = $(addsuffix /.depdirs, $(addprefix $(BUILDDIR)/,$(ROOTDIRS-y))) > > These DEPDIRS are files, although DEPDIRS in other contexts are directories. > I think it should be renamed. DEPDIR_FILES? Make sense, I will update. > >> # use a "for" in a shell to process dependencies: we don't want this >> # task to be run in parallel. > > You forgot to remove this obsolete comment. Will update on next version. > >> .PHONY: depdirs >> depdirs: $(RTE_OUTPUT)/.depdirs >> -$(RTE_OUTPUT)/.depdirs: $(RTE_OUTPUT)/.config >> -@rm -f $(RTE_OUTPUT)/.depdirs ; \ >> -for d in $(ROOTDIRS-y); do \ >> -if [ -f $(RTE_SRCDIR)/$$d/Makefile ]; then \ >> -[ -d $(BUILDDIR)/$$d ] || mkdir -p $(BUILDDIR)/$$d ; \ >> -$(MAKE) S=$$d -f $(RTE_SRCDIR)/$$d/Makefile depdirs \ >> ->> $(RTE_OUTPUT)/.depdirs ; \ >> -fi ; \ >> -done >> +$(RTE_OUTPUT)/.depdirs: $(DEPDIRS) >> +@rm -f $@ >> +@for f in $(DEPDIRS); do cat $$f >> $@; done >> +@sort -u -o $@ $@ >> + >> +$(DEPDIRS): $(RTE_OUTPUT)/.config >> +@f=$(lastword $(subst /, ,$(dir $@))); \ > > Could you use $(notdir $(@D)) ? Can be, I will try. > >> +[ -d $(BUILDDIR)/$$f ] || mkdir -p $(BUILDDIR)/$$f; \ >> +rm -f $@; \ > > Why this removal? To be sure we are not using old files, but this can be done below line with using ">" I guess. > >> +$(MAKE) S=$$f -f $(RTE_SRCDIR)/$$f/Makefile depdirs >> $@ > > This part is a bit complicated. > Could it be simplified by better naming $f? I will try. > > >> --- a/mk/rte.subdir.mk >> +++ b/mk/rte.subdir.mk >> @@ -76,7 +76,7 @@ clean: _postclean >> # include .depdirs and define rules to order priorities between build >> # of directories. >> # >> -include $(RTE_OUTPUT)/.depdirs >> +-include $(RTE_OUTPUT)/.depdirs >> >> define depdirs_rule >> $(1): $(sort $(patsubst $(S)/%,%,$(LOCAL_DEPDIRS-$(S)/$(1 >> @@ -84,16 +84,15 @@ endef >> >> $(foreach d,$(DIRS-y),$(eval $(call depdirs_rule,$(d >> >> +DEPDIRS = $(wildcard $(addprefix $(S)/,$(DIRS-y))) >> >> # use a "for" in a shell to process dependencies: we don't want this >> # task to be run in parallel. > > You forgot to remove this obsolete comment. Right, will update. > >> -.PHONY: depdirs >> -depdirs: >> -@for d in $(DIRS-y); do \ >> -if [ -f $(SRCDIR)/$$d/Makefile ]; then \ >> -$(MAKE) S=$S/$$d -f $(SRCDIR)/$$d/Makefile depdirs ; \ >> -fi ; \ >> -done >> +.PHONY: depdirs $(DEPDIRS) >> +depdirs: $(DEPDIRS) >> + >> +$(DEPDIRS): >> +@$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile depdirs >
Re: [dpdk-dev] [PATCH 03/13] rte_ether: set PKT_RX_VLAN_STRIPPED in rte_vlan_strip()
It is fixing the introduction of the new flag PKT_RX_VLAN_STRIPPED.
Fixes: b37b528d957c ("mbuf: add new Rx flags for stripped VLAN")
This patch is applying the flag to the software emulation case
(currently only for virtio).
So the comment of this flag should be changed:
/**
* A vlan has been stripped by the hardware and its tci is saved in
* mbuf->vlan_tci. This can only happen if vlan stripping is enabled
* in the RX configuration of the PMD.
*/
#define PKT_RX_VLAN_STRIPPED (1ULL << 6)
> Signed-off-by: Michał Mirosław
[...]
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -357,7 +357,7 @@ static inline int rte_vlan_strip(struct rte_mbuf *m)
> return -1;
>
> struct vlan_hdr *vh = (struct vlan_hdr *)(eh + 1);
> - m->ol_flags |= PKT_RX_VLAN_PKT;
> + m->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
> m->vlan_tci = rte_be_to_cpu_16(vh->vlan_tci);
>
> /* Copy ether header over rather than moving whole packet */
I think this flag should also be removed in the function rte_vlan_insert().
Re: [dpdk-dev] [PATCH] efd: fix compilation by removing dep to libmath
2017-01-27 14:45, De Lara Guarch, Pablo:
> From: Olivier Matz [mailto:olivier.m...@6wind.com]
> >
> > When we compile the dpdk with:
> > CONFIG_RTE_LIBRTE_EFD=y
> > CONFIG_RTE_LIBRTE_NFP_PMD=n
> > CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD=n
> > CONFIG_RTE_LIBRTE_SCHED=n
> > CONFIG_RTE_LIBRTE_METER=n
> >
> > The linker gives the following error:
> > lib/librte_efd.a(rte_efd.o): In function `rte_efd_create':
> > lib/librte_efd/rte_efd.c:560: undefined reference to `log2'
> > collect2: error: ld returned 1 exit status
> >
> > This is because the '-lm' is missing in mk/rte.app.mk.
> >
> > An alternative, which is proposed by this patch, is to use the compiler
> > builtin rte_bsf32() to process log2 instead of the libmath log2() that
> > requires to include math.h and link with -lm.
> >
> > Signed-off-by: Olivier Matz
>
> Acked-by: Pablo de Lara
>
> Nice catch, thanks!
Fixes: 56b6ef874f80 ("efd: new Elastic Flow Distributor library")
Applied, thanks
Re: [dpdk-dev] [PATCH 0/2] Enable zero verdicts in ACLs
> > This set enables one to have ACL matches return 0 where the distinction > > from no-match case is not needed. > > > > This is a resubmission of the patches as a series, rebased on net-next tree, > > no other changes vs v2. > > > > v2: fixes to prog_guide and ACL tests > > > > Michał Mirosław (2): > > acl: remove invalid test > > acl: allow zero verdict > > Acked-by: Konstantin Ananyev Applied, thanks
Re: [dpdk-dev] [PATCH 05/13] acl: fix acl_flow_data comments
> > Signed-off-by: Michał Mirosław > > Acked-by: Konstantin Ananyev Applied, thanks
[dpdk-dev] [PATCH v2] mk: parallelize make config
make config dependency resolving was always running serial, parallelize it for better performance. $ time make T=x86_64-native-linuxapp-gcc config real0m12.633s $ time make -j8 T=x86_64-native-linuxapp-gcc config real0m1.826s When config creation done under a single make target, using a for loop, make has no control on the action, and it needs to run as implemented in the rule. But if for loop converted into multiple targets, make can detect independent targets and run them parallel based on -j parameter. Signed-off-by: Ferruh Yigit --- devtools/test-build.sh | 2 +- mk/internal/rte.depdirs-post.mk | 11 ++- mk/rte.sdkdepdirs.mk| 21 ++--- mk/rte.subdir.mk| 17 +++-- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/devtools/test-build.sh b/devtools/test-build.sh index 680d79b..0f131fc 100755 --- a/devtools/test-build.sh +++ b/devtools/test-build.sh @@ -146,7 +146,7 @@ config () # fi if [ ! -e $1/.config ] || $reconfig ; then echo "== Configure $1" - make T=$2 O=$1 config + make -j$J T=$2 O=$1 config echo 'Customize configuration' # Built-in options (lowercase) diff --git a/mk/internal/rte.depdirs-post.mk b/mk/internal/rte.depdirs-post.mk index 102a369..eb73ad3 100644 --- a/mk/internal/rte.depdirs-post.mk +++ b/mk/internal/rte.depdirs-post.mk @@ -29,11 +29,12 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -.PHONY: depdirs -depdirs: - @for d in $(DEPDIRS-y); do \ - $(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $$d ; \ - done +.PHONY: depdirs $(DEPDIRS-y) +depdirs: $(DEPDIRS-y) + @echo "" + +$(DEPDIRS-y): + @$(RTE_SDK)/buildtools/depdirs-rule.sh $(S) $@ .PHONY: depgraph depgraph: diff --git a/mk/rte.sdkdepdirs.mk b/mk/rte.sdkdepdirs.mk index bebaf2a..38fd863 100644 --- a/mk/rte.sdkdepdirs.mk +++ b/mk/rte.sdkdepdirs.mk @@ -36,19 +36,18 @@ ifeq (,$(wildcard $(RTE_OUTPUT)/Makefile)) $(error "need a make config first") endif -# use a "for" in a shell to process dependencies: we don't want this -# task to be run in parallel. +DEPDIR_FILES = $(addsuffix /.depdirs, $(addprefix $(BUILDDIR)/,$(ROOTDIRS-y))) + .PHONY: depdirs depdirs: $(RTE_OUTPUT)/.depdirs -$(RTE_OUTPUT)/.depdirs: $(RTE_OUTPUT)/.config - @rm -f $(RTE_OUTPUT)/.depdirs ; \ - for d in $(ROOTDIRS-y); do \ - if [ -f $(RTE_SRCDIR)/$$d/Makefile ]; then \ - [ -d $(BUILDDIR)/$$d ] || mkdir -p $(BUILDDIR)/$$d ; \ - $(MAKE) S=$$d -f $(RTE_SRCDIR)/$$d/Makefile depdirs \ - >> $(RTE_OUTPUT)/.depdirs ; \ - fi ; \ - done +$(RTE_OUTPUT)/.depdirs: $(DEPDIR_FILES) + @rm -f $@ + @sort -u -o $@ $(DEPDIR_FILES) + +$(DEPDIR_FILES): $(RTE_OUTPUT)/.config + @dir=$(notdir $(@D)); \ + [ -d $(BUILDDIR)/$$dir ] || mkdir -p $(BUILDDIR)/$$dir; \ + $(MAKE) S=$$dir -f $(RTE_SRCDIR)/$$dir/Makefile depdirs > $@ .PHONY: depgraph depgraph: diff --git a/mk/rte.subdir.mk b/mk/rte.subdir.mk index 256e64e..5341f1f 100644 --- a/mk/rte.subdir.mk +++ b/mk/rte.subdir.mk @@ -76,7 +76,7 @@ clean: _postclean # include .depdirs and define rules to order priorities between build # of directories. # -include $(RTE_OUTPUT)/.depdirs +-include $(RTE_OUTPUT)/.depdirs define depdirs_rule $(1): $(sort $(patsubst $(S)/%,%,$(LOCAL_DEPDIRS-$(S)/$(1 @@ -84,16 +84,13 @@ endef $(foreach d,$(DIRS-y),$(eval $(call depdirs_rule,$(d +DEPDIRS = $(wildcard $(addprefix $(S)/,$(DIRS-y))) -# use a "for" in a shell to process dependencies: we don't want this -# task to be run in parallel. -.PHONY: depdirs -depdirs: - @for d in $(DIRS-y); do \ - if [ -f $(SRCDIR)/$$d/Makefile ]; then \ - $(MAKE) S=$S/$$d -f $(SRCDIR)/$$d/Makefile depdirs ; \ - fi ; \ - done +.PHONY: depdirs $(DEPDIRS) +depdirs: $(DEPDIRS) + +$(DEPDIRS): + @$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile depdirs .PHONY: depgraph depgraph: -- 2.9.3
Re: [dpdk-dev] rte_eth_from_rings
On Fri, Jan 27, 2017 at 10:31:52PM +, Sridhar Pitchai wrote: > Thanks Bruce. > > > I have created eth_dev from the rings as below. > rt = rte_eth_from_rings(port_p->name, > (struct rte_ring * const*)port_p->rx_ring_p, 2, > (struct rte_ring * const*)port_p->tx_ring_p, 2, > rte_socket_id()); > > Lets say I have a call back something like > > pkt_rx(void * pkt_p, struct pkt_metadata_t *meta_p) > > which is called when there is a pkt at the chip. > inside this function(pkt_rx) i will find the port and the corresponding > ring_p and > enqueue the pkt into the queue. I am assuming this should work. Kindly > correct me if i misunderstood you. > Yes, it should work. > > question 2: > Can use this eth_dev to create KNI interface, like below. > rte_eth_dev_info_get(fp_p->port_list[port].key.port_id, &dev_info); > ; > ; > ; > ops.port_id = fp_p->port_list[port].key.port_id; // rt from > rte_eth_from_rings(...) > ops.change_mtu = _kni_ifconfig_mtu; // static function > ops.config_network_if = _kni_ifconfig; // static functions > PORT(fp_p, port).kni_p = rte_kni_alloc(PORT(fp_p, port).mbuf_p, > &conf, &ops); > > I am assuming this should work as well. I find the netdevs created but when i > try to configure > them i am facing the following error. > > root@ VM snaproute/softpath (master) >ifconfig PORT_8 > PORT_8Link encap:Ethernet HWaddr be:35:c3:0f:f8:2f > BROADCAST MULTICAST MTU:1500 Metric:1 > RX packets:0 errors:0 dropped:0 overruns:0 frame:0 > TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:0 (0.0 B) TX bytes:0 (0.0 B) > > root@ VM snaproute/softpath (master) >ifconfig PORT_8 20.1.1.1/24 up > SIOCSIFFLAGS: Timer expired > SIOCSIFFLAGS: Timer expired > root@ VM snaproute/softpath (master) > > +Ferruh as KNI maintainer, to see if he can help out with this question. /Bruce > Thanks for the help. > > Thanks, > Sridhar Pitchai > > > > From: Bruce Richardson > Sent: Friday, January 27, 2017 1:36 PM > To: Sridhar Pitchai > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] rte_eth_from_rings > > On Fri, Jan 27, 2017 at 07:16:25PM +, Sridhar Pitchai wrote: > > Hi, > > > > I am trying to write a data path for packets punted to CPU(slowpath) from > > vender silicon like broadcom. I am planing to use "rte_eth_from_rings" like > > model where I will be able to read and write to the ring for the packets > > punted from vendor chip. > > > > > > the eth_dev abstraction provided by "rte_eth_from_rings" helps to build the > > dpdk data path. Can someone help me on how to read and write to the rings > > that is emulating the eth_dev. > > > > > To use the rings like an ethdev just use rte_eth_rx_burst and > rte_eth_tx_burst, passing in the port id of your newly created rings > ethdev. To access them directly as rings, just use the ring > enqueue/dequeue functions passing in the ring pointer as normal. > > Regards, > /Bruce
Re: [dpdk-dev] [PATCH 1/9] net/ixgbe: remove redundant EOL character from logs
On 1/27/2017 5:49 PM, Stephen Hemminger wrote: > On Fri, 27 Jan 2017 17:46:11 + > Ferruh Yigit wrote: > >> Signed-off-by: Ferruh Yigit > > Thanks for fixing this. > > Acked-by: Stephen Hemminger > Applied to dpdk-next-net/master, thanks. (all squashed into single commit: "drivers/net: remove redundant EOL char from logs")
[dpdk-dev] [PATCH] config: add default MPIPE PMD config to common base
Signed-off-by: Ferruh Yigit --- config/common_base | 6 ++ 1 file changed, 6 insertions(+) diff --git a/config/common_base b/config/common_base index 00a91b9..f44d4e4 100644 --- a/config/common_base +++ b/config/common_base @@ -219,6 +219,12 @@ CONFIG_RTE_LIBRTE_MLX5_DEBUG=n CONFIG_RTE_LIBRTE_MLX5_TX_MP_CACHE=8 # +# MPIPE PMD +# +CONFIG_RTE_LIBRTE_MPIPE_PMD=n +CONFIG_RTE_LIBRTE_MPIPE_PMD_DEBUG=n + +# # Compile burst-oriented Broadcom PMD driver # CONFIG_RTE_LIBRTE_BNX2X_PMD=n -- 2.9.3
Re: [dpdk-dev] [PATCH v4 1/6] eventdev: introduce event driven programming model
On Thu, Jan 26, 2017 at 08:39:57PM +, Eads, Gage wrote:
>
>
> > -Original Message-
> > From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com]
> > Sent: Thursday, January 26, 2017 3:39 AM
> > To: Eads, Gage
> > Cc: Richardson, Bruce ; 'dev@dpdk.org'
> > ; 'thomas.monja...@6wind.com'
> > ; 'hemant.agra...@nxp.com'
> > ; Van Haaren, Harry
> > ; McDaniel, Timothy
> >
> > Subject: Re: [dpdk-dev] [PATCH v4 1/6] eventdev: introduce event driven
> > programming model
> >
> > Considering different implementation has different behaviors, How about
> > enumerating the overflow policy at the port configuration time? and let
> > implementation act accordingly to avoid fast-patch change in
> > application(effects in all implementation irrespective of the capability)
> >
> > possible enumerating value at the port configuration time,
> > - PANIC or similar scheme to denote it cannot proceed
> > - TAIL DROP
> > or any expected application behavior you want to add
>
> I wonder if that's necessary? Hardware behavior a) means the function will
> always return nb_events. If hardware drops the event(s), I assume
> enqueue_burst would still return nb_events and the app behaves as if all
> events were sent. If the enqueue fails (ret < nb_events), app software could
> check rte_errno and take the action it deems necessary. So all fast-path
> enqueue code could look like:
>
> ret = rte_event_enqueue_burst(..., nb_events);
> if (ret < nb_events) {
I was concerned about this section of the application code get bloated with
drivers specific actions. But, If we want the actions based on per event then
I think, it makes sense to update the specification with new rte_errno values
for enqueue.
>
> }
Re: [dpdk-dev] buf->hash.rss always empty with i40e
On Sat, Jan 28, 2017 at 11:48:36AM +0100, tom.barbe...@ulg.ac.be wrote:
> Hi all,
>
> No matter the number of queues or the ETH_RSS_X parameter the mbuf->hash.rss
> field is empty using XL710 NICs. The exact same configuration gives me good
> hash value values with ixgbe/82599 cards.
>
> Any idea? I checked it is the same problem on 2.2 and 16.11. FlowDirector is
> not used.
It's hard to diagnose the exact problem without the full settings you
have tried, but one thing to watch out for that has caught me in the
past is that turning on RSS for IP packets, does not give an RSS value
if there is also TCP or UDP in the packets too. It only applies to IP
packets without TCP or UDP.
Here are settings that work for me with i40e driver:
static const struct rte_eth_conf port_conf_default = {
.rxmode = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN
},
.rx_adv_conf = {
.rss_conf = {
.rss_hf = ETH_RSS_IP | ETH_RSS_TCP |
ETH_RSS_UDP,
}
}
};
Regards,
/Bruce
Re: [dpdk-dev] [PATCH] config: add default MPIPE PMD config to common base
2017-01-30 10:40, Ferruh Yigit: > --- a/config/common_base > +++ b/config/common_base > @@ -219,6 +219,12 @@ CONFIG_RTE_LIBRTE_MLX5_DEBUG=n > CONFIG_RTE_LIBRTE_MLX5_TX_MP_CACHE=8 > > # > +# MPIPE PMD > +# > +CONFIG_RTE_LIBRTE_MPIPE_PMD=n > +CONFIG_RTE_LIBRTE_MPIPE_PMD_DEBUG=n NACK I think the whole tile arch must be removed from DPDK. It is not maintained.
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
On 1/29/2017 2:12 AM, Keith Wiles wrote: > The tap driver setup both rx and tx file descriptors when the > rte_eth_rx_queue_setup() causing the tx to be closed when tx setup > was called. Can you please describe the problem more. Without this patch rx->fd == tx->fd, with this patch rx and tx has different file descriptors. What was the wrong with rx and tx having same fd? As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that function will do nothing if rx or tx has valid fd. > > Signed-off-by: Keith Wiles <...>
Re: [dpdk-dev] [PATCH] ethdev: fix wrong memset
On 28/01/2017 13:14, Yuanhan Liu wrote: [..] I'll apply the patch from Remy which fixes a case where process creates I would ask you not to apply that. IMO, his patch may have "fixed" a crash he saw, but it doesn't looks like to me it really fixes anything: the driver may reference rte_eth_data belongs to another driver -- things can't be right afterwards. I've self-NAK'd it as the code path it was aimed at no longer occurrs. I'll restart this discussion with a bigger picture of what multiproc is, and what are the issues. Great! And I'm keen to know how the multiproc is actually used in real life (and why they don't use multiple thread). Most importantly, does people really care about it? (I fixed few virtio multiproc issues that I know there are some people/companies using it, and I want to know if there are more). The use-cases I'm coming across are to allow the attaching/detaching of external agents mainly for information-reporting purposes, without having to leave hooks everywhere. In this case main advantage is the relative ease that processes can be hot-plugged in a way threads cannot. I suppose something more heavyweight might be a primary process as a host physical interconnect, with secondary processes being virtual machines - in this case these might be large semi-independent code-bases one does not want in the same process image. To me the major down-side with DPDK's multiprocess model is how address-space layout randomisation can trip things up. I know parts of the code seems ASLR-safe, but I very much doubt all of it is. ..Remy
[dpdk-dev] [PATCH] crypto/scheduler: fix name parameter parsing
This patch fixes the name parsing issue. Originally, the unique
scheduler name created by system is not passed to vdev
initializer.
Fixes: 8b483eae ("crypto/scheduler: register scheduler vdev driver")
Signed-off-by: Fan Zhang
---
drivers/crypto/scheduler/scheduler_pmd.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/scheduler/scheduler_pmd.c
b/drivers/crypto/scheduler/scheduler_pmd.c
index 62418d0..eeafbe6 100644
--- a/drivers/crypto/scheduler/scheduler_pmd.c
+++ b/drivers/crypto/scheduler/scheduler_pmd.c
@@ -116,19 +116,22 @@ static int
cryptodev_scheduler_create(const char *name,
struct scheduler_init_params *init_params)
{
- char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
+ char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN] = {0};
struct rte_cryptodev *dev;
struct scheduler_ctx *sched_ctx;
if (init_params->def_p.name[0] == '\0') {
int ret = rte_cryptodev_pmd_create_dev_name(
- init_params->def_p.name,
+ crypto_dev_name,
RTE_STR(CRYPTODEV_NAME_SCHEDULER_PMD));
if (ret < 0) {
CS_LOG_ERR("failed to create unique name");
return ret;
}
+ } else {
+ strncpy(crypto_dev_name, init_params->def_p.name,
+ RTE_CRYPTODEV_NAME_MAX_LEN - 1);
}
dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
--
2.7.4
Re: [dpdk-dev] [PATCH] net/ixgbe: clean up rte_eth_dev_info_get
On 1/25/2017 5:24 AM, Tiwei Bie wrote: > On Wed, Jan 25, 2017 at 01:13:32PM +0800, Lu, Wenzhuo wrote: >> Hi Tiwei, >> >>> -Original Message- >>> From: Bie, Tiwei >>> Sent: Wednesday, January 25, 2017 11:17 AM >>> To: Lu, Wenzhuo >>> Cc: dev@dpdk.org >>> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: clean up rte_eth_dev_info_get >>> >>> On Wed, Jan 25, 2017 at 10:39:22AM +0800, Wenzhuo Lu wrote: It'not appropriate to call rte_eth_dev_info_get in PMD, as rte_eth_dev_info_get need to get info from PMD. Remove rte_eth_dev_info_get from PMD code and get the info directly. Signed-off-by: Wenzhuo Lu --- drivers/net/ixgbe/ixgbe_ethdev.c | 144 ++- 1 file changed, 68 insertions(+), 76 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 64ce55a..f14a68b 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -4401,17 +4401,17 @@ static int >>> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, int rar_entry; uint8_t *new_mac = (uint8_t *)(mac_addr); struct rte_eth_dev *dev; - struct rte_eth_dev_info dev_info; + struct rte_pci_device *pci_dev; RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV); dev = &rte_eth_devices[port]; - rte_eth_dev_info_get(port, &dev_info); + pci_dev = IXGBE_DEV_TO_PCI(dev); - if (is_ixgbe_pmd(dev_info.driver_name) != 0) + if (is_ixgbe_pmd(dev->data->drv_name)) return -ENOTSUP; >>> >>> The return value of is_ixgbe_pmd() is not boolean (actually I think it >>> should be >>> based on its name). If we omit the comparison with 0, the code looks weird. >>> It >>> looks like it'll return -ENOTSUP if the port's driver is ixgbe PMD. >> >> Yes, it’s weird. But what makes it weird is not the missing comparison but >> the function name. >> Better changing it to ixgbe_pmd_check. How about it? >> > > Yeah, I also prefer to change the helper function itself. But I'm not > good at the naming. I'd like to hear others' opinion. :-) Agree that it looks wrong without 0 comparison. Helper function is checking if the given port is an ixgbe port or not, unfortunately you need to this for PMD specific APIs. So What about is_device_supported(), I agree it is better if it returns bool, and I also think it is better if it gets the rte_eth_dev as input parameter, validating port based on name is API internal knowledge. Also instead of name comparison against fixed string, it can be eth_dev->driver->pci_drv.name against driver->name. This makes function more generic, and perhaps this helper function can be moved into ethdev layer, later. For this function needs to get both eth_dev and rte_driver as argument. > > Best regards, > Tiwei Bie >
Re: [dpdk-dev] [PATCH v2] app/testpmd: fix memory leak
Hi Adrien,
> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com]
> Sent: Monday, January 30, 2017 9:33 AM
> To: De Lara Guarch, Pablo
> Cc: Wu, Jingjing; dev@dpdk.org
> Subject: Re: [PATCH v2] app/testpmd: fix memory leak
>
> Hi Pablo,
>
> On Fri, Jan 27, 2017 at 02:54:58PM +, Pablo de Lara wrote:
> > Free memory when port flow entry creation fails.
> >
> > Coverity issue: 139600
> > Fixes: 938a184a1870 ("app/testpmd: implement basic support for flow
> API")
> >
> > Signed-off-by: Pablo de Lara
> > ---
> > Changes in v2:
> >
> > - Removed unnecessary conditional
> >
> > app/test-pmd/config.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 5834498..467932f 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -954,6 +954,7 @@ port_flow_new(const struct rte_flow_attr *attr,
> > goto store;
> > }
> > notsup:
> > + free(pf);
> > rte_errno = err;
> > return NULL;
> > }
> > --
> > 2.7.4
> >
>
> I think this is a false positive, which is why I did not address it during
> the last round of Coverity issues.
>
> As a two-pass function, errors are checked during the first pass, when pf is
> not allocated yet. During the second pass, intermediate functions are not
> supposed to return a different result and "notsup" cannot occur.
>
> I think assert()/rte_assert() would make more sense and should fool
> Coverity
> into understanding the expected behavior.
>
> The commit log should reflect that we are addressing a false positive since
> there is no problem with the code logic (of course unless I missed anything
> obvious).
Then, I would just ignore the coverity issue, changing the status to
intentional.
Thanks,
Pablo
>
> Thanks.
>
> --
> Adrien Mazarguil
> 6WIND
Re: [dpdk-dev] [PATCH] net/virtio: do not gso when no header is present
On Tue, Jan 24, 2017 at 09:36:03PM +0100, Emmanuel Roullit wrote:
> Found with clang static analysis:
> lib/librte_vhost/virtio_net.c:723:17: warning:
> Access to field 'data_off' results in a dereference of a null pointer
> (loaded from variable 'tcp_hdr')
> m->l4_len = (tcp_hdr->data_off & 0xf0) >> 2;
> ^
This is a good fix, thanks. But there are few minor nits. Firstly,
prefix is wrong: it should be "vhost" but not "net/virtio".
> Fixes: 2a51b1091cb5 ("vhost: support indirect descriptor in non-mergeable Rx")
That's not the original commit introduced such issue, d0cf91303d73
("vhost: add Tx offload capabilities") is.
I actually saw you have made this kind of mistakes (referencing the
wrong culprit commit) few times. I'm wondering how did you get that.
Besides those, I think it's a good candidate for a stable release:
thinking that a malicious guest might forge some invalid virtio net
headers, which would make this potential NULL dereference become real.
So, Cc: sta...@dpdk.org,
And Applied to dpdk-next-virtio.
--yliu
Re: [dpdk-dev] [PATCH] vhost: remove unneeded variable assignment
On Tue, Jan 24, 2017 at 09:31:29PM +0100, Emmanuel Roullit wrote:
> Found with clang static analysis:
> lib/librte_vhost/vhost_user.c:996:3: warning:
> Value stored to 'ret' is never read
> ret = vhost_user_get_vring_base(dev, &msg.payload.state);
> ^ ~~
>
> Fixes: 73c8f9f69c6c ("vhost: introduce reply ack feature")
Again, you were referencing the bad commit. For this case, I'd like to
remove such fixline, as this patch doesn't really "fix" anything. But
since you made it, I could apply it.
So applied to dpdk-next-virtio.
--yliu
[dpdk-dev] Unable to run test_pmd_perf
Hi All, We have VMs on a physical server with both VMs having 1 core 7 GB RAM with DPDK 16.04. The VMs are communicating using SR-IOV (per VM 1 VF is assigned). We thought of running the test_pmd_perf to test the number of cycles taken for RX/TX. But we are unable to get it working. Even if we assign 4 VCPUs to the VM the app says "*no avail lcore to test*." The command we are using are: cd dpdk16.04/app/test ./build/app/test -c f -- -p 1 RTE>> pmd_perf_autotest On searching we found that it might be an issue with our VMs using SRIOV. Please guide us on this. Thanks, Priyanka
Re: [dpdk-dev] buf->hash.rss always empty with i40e
That solved it ! Thanks.
However I think this should be documented somewhere, especially as the
behaviour is different between i40e and ixgbe.
Tom
- Mail original -
De: "Bruce Richardson"
À: "tom barbette"
Cc: dev@dpdk.org
Envoyé: Lundi 30 Janvier 2017 11:46:30
Objet: Re: [dpdk-dev] buf->hash.rss always empty with i40e
On Sat, Jan 28, 2017 at 11:48:36AM +0100, tom.barbe...@ulg.ac.be wrote:
> Hi all,
>
> No matter the number of queues or the ETH_RSS_X parameter the mbuf->hash.rss
> field is empty using XL710 NICs. The exact same configuration gives me good
> hash value values with ixgbe/82599 cards.
>
> Any idea? I checked it is the same problem on 2.2 and 16.11. FlowDirector is
> not used.
It's hard to diagnose the exact problem without the full settings you
have tried, but one thing to watch out for that has caught me in the
past is that turning on RSS for IP packets, does not give an RSS value
if there is also TCP or UDP in the packets too. It only applies to IP
packets without TCP or UDP.
Here are settings that work for me with i40e driver:
static const struct rte_eth_conf port_conf_default = {
.rxmode = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN
},
.rx_adv_conf = {
.rss_conf = {
.rss_hf = ETH_RSS_IP | ETH_RSS_TCP |
ETH_RSS_UDP,
}
}
};
Regards,
/Bruce
Re: [dpdk-dev] [PATCH] net/vhost: fix unix socket not removed as closing
On Tue, Jan 24, 2017 at 08:37:38AM +, Jianfeng Tan wrote:
> The commit aed0b12930b ("net/vhost: fix socket file deleted on stop")
> moves rte_vhost_driver_register and rte_vhost_driver_unregister from
> dev_start() and dev_stop() into driver's probe() and remove().
>
> Apps, like testpmd, using vhost pmd in server mode, usually calls
> dev_stop() and dev_close() as quitting, instead of driver-specific
> remove(). Then those unix socket files have no chance to get removed.
>
> Semantically, device-specific things should be put into device-specific
> APIs. Fix this issue by moving rte_vhost_driver_unregister, plus other
> structure free into dev_close().
>
> Fixes: aed0b12930b3 ("net/vhost: fix socket file deleted on stop")
The original commit will be cherry-picked to v16.11 stable release,
as a fix commit of it, this commit should also be picked. So,
Cc: sta...@dpdk.org
>
> Reported-by: Lei Yao
> Signed-off-by: Jianfeng Tan
> ---
> drivers/net/vhost/rte_eth_vhost.c | 48
> +++
> 1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/vhost/rte_eth_vhost.c
> b/drivers/net/vhost/rte_eth_vhost.c
> index 848a3da..93b8a52 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -801,6 +801,32 @@ eth_dev_stop(struct rte_eth_dev *dev)
> update_queuing_status(dev);
> }
>
> +static void
> +eth_dev_close(struct rte_eth_dev *dev)
> +{
> + struct pmd_internal *internal;
> + struct internal_list *list;
> +
> + internal = dev->data->dev_private;
> + if (!internal)
> + return;
> +
> + list = find_internal_resource(internal->iface_name);
> + if (!list)
> + return;
> +
> + pthread_mutex_lock(&internal_list_lock);
> + TAILQ_REMOVE(&internal_list, list, next);
> + pthread_mutex_unlock(&internal_list_lock);
> + rte_free(list);
> +
> + rte_vhost_driver_unregister(internal->iface_name);
Note that you should invoke rte_vhost_driver_unregister() before
removing "internal" from the list, otherwise you might get an error
like following from destory_device() callback (which will try to
find the "internal" again).
PMD: Invalid interface name: /tmp/vhost-net
Fixed and applied to dpdk-next-virtio.
--yliu
Re: [dpdk-dev] [dpdk-stable] [PATCH 1/2] net/virtio: fix performance regression due to TSO enabling
On Mon, Jan 16, 2017 at 12:26:41PM +0100, Michal Orsák wrote: > >>For such workload, I don't think it would behaviour worse on ARM. > >No reply yet; I will treat it as no objections, and please shout out if > >any. > > > >Both applied to dpdk-next-virtio. > > > > --yliu > Hello, > > > currently I am running short of time. If you have any test prepared which > i > can just ran, please send me a link. > >>>No link, but you could try: > >>> > >>>- a typical PVP test > >>> > >>>- a txonly test: running txonly fwd mode in guest PMD while running > >>> rxonly in fwd mode. > >>> > >>>The second is a micro test, thus I saw way bigger boost. > >>> > >>>When are you available for the testing, btw? > >>25.1.2017+ > >Okay, I will hold on a while to apply them. > Ok, I will send you results when I have them. Again, no reply yet. I'm appying them to dpdk-next-virtio. I really don't think it could perform worse in ARM, as it removes a costly cache invalidation operation (which should be more expensive than the instruction cycles). OTOH, again, testing is welcome, if it's later proved to be worse on ARM (which I highly doubt), I could revert them. --yliu
Re: [dpdk-dev] [dpdk-stable] [PATCH 1/2] net/virtio: fix performance regression due to TSO enabling
On 01/30/2017 02:30 PM, Yuanhan Liu wrote: On Mon, Jan 16, 2017 at 12:26:41PM +0100, Michal Orsák wrote: For such workload, I don't think it would behaviour worse on ARM. No reply yet; I will treat it as no objections, and please shout out if any. Both applied to dpdk-next-virtio. --yliu Hello, currently I am running short of time. If you have any test prepared which i can just ran, please send me a link. No link, but you could try: - a typical PVP test - a txonly test: running txonly fwd mode in guest PMD while running rxonly in fwd mode. The second is a micro test, thus I saw way bigger boost. When are you available for the testing, btw? 25.1.2017+ Okay, I will hold on a while to apply them. Ok, I will send you results when I have them. Again, no reply yet. I'm appying them to dpdk-next-virtio. I really don't think it could perform worse in ARM, as it removes a costly cache invalidation operation (which should be more expensive than the instruction cycles). OTOH, again, testing is welcome, if it's later proved to be worse on ARM (which I highly doubt), I could revert them. Or have a per-architecture definition of ASSIGN_UNLESS_EQUAL if it proved to regress on ARM? Maxime --yliu
Re: [dpdk-dev] [dpdk-stable] [PATCH 1/2] net/virtio: fix performance regression due to TSO enabling
On Mon, Jan 30, 2017 at 02:54:16PM +0100, Maxime Coquelin wrote: > >OTOH, again, testing is welcome, if it's later proved to be worse on > >ARM (which I highly doubt), I could revert them. > Or have a per-architecture definition of ASSIGN_UNLESS_EQUAL if it > proved to regress on ARM? Yes, we could try that. --yliu
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
> On Jan 30, 2017, at 5:00 AM, Yigit, Ferruh wrote:
>
> On 1/29/2017 2:12 AM, Keith Wiles wrote:
>> The tap driver setup both rx and tx file descriptors when the
>> rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
>> was called.
>
> Can you please describe the problem more.
> Without this patch rx->fd == tx->fd, with this patch rx and tx has
> different file descriptors.
Let me look at this more, I am getting the same FD for both. Must be something
else going on.
>
> What was the wrong with rx and tx having same fd?
>
> As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that
> function will do nothing if rx or tx has valid fd.
The rte_eth_rx_queue_setup() look at line 1146 if rxq has a value then release
it, which happens on both Rx/Tx code
rxq = dev->data->rx_queues;
if (rxq[rx_queue_id]) {
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release,
-ENOTSUP);
(*dev->dev_ops->rx_queue_release)(rxq[rx_queue_id]);
rxq[rx_queue_id] = NULL;
}
if (rx_conf == NULL)
rx_conf = &dev_info.default_rxconf;
ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
socket_id, rx_conf, mp);
>
>>
>> Signed-off-by: Keith Wiles
>
> <...>
Regards,
Keith
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
On 01/30/2017 12:00 PM, Ferruh Yigit wrote:> On 1/29/2017 2:12 AM, Keith
Wiles wrote:
The tap driver setup both rx and tx file descriptors when the
rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
was called.
Can you please describe the problem more.
Without this patch rx->fd == tx->fd, with this patch rx and tx has
different file descriptors.
What was the wrong with rx and tx having same fd?
As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that
function will do nothing if rx or tx has valid fd.
Signed-off-by: Keith Wiles
<...>
Hi,
The tap PMD recently broke for me because of this patch [1].
During init (eth_dev_tap_create()), the tap PMD allocates a shared RX/TX
queue through tun_alloc().
The recent patch now releases existing queues in rx_queue_setup(),
before adding new ones.
When rx_queue_setup() is called, it uses close() calls on all shared
queues, effectively deleting the netdevice.
That's the main issue here.
I tested Keith's patch [2], and it fixes that issue, using separate queues.
There is however a couple of other queues-related issues in the tap PMD,
but I'm not sure how to address them properly:
1. internals->fds[] gets filled only with RX queues (appart from index 0
that is common to both RX and TX).
It means that RX queues only will be deleted when calling
rte_pmd_tap_remove() or tap_tx_queue_release().
2. tap_dev_stop() is not symmetrical with tap_dev_start(): queues won't
get re-created after a stop.
It may be best to keep the very first fd (created with tun_alloc() in
eth_dev_tap_create() during probe) apart.
And then add separate TX/RX queues in internals->txq[] and
internals->rxq[] respectively.
What do you think?
[1] d00d7cc88335 ("ethdev: release queue before setting up")
[2] http://dpdk.org/ml/archives/dev/2017-January/056470.html
--
Pascal Mazon
www.6wind.com
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
> On Jan 30, 2017, at 8:38 AM, Pascal Mazon wrote:
>
> On 01/30/2017 12:00 PM, Ferruh Yigit wrote:> On 1/29/2017 2:12 AM, Keith
> Wiles wrote:
>>> The tap driver setup both rx and tx file descriptors when the
>>> rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
>>> was called.
>>
>> Can you please describe the problem more.
>> Without this patch rx->fd == tx->fd, with this patch rx and tx has
>> different file descriptors.
>>
>> What was the wrong with rx and tx having same fd?
>>
>> As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that
>> function will do nothing if rx or tx has valid fd.
>>
>>>
>>> Signed-off-by: Keith Wiles
>>
>> <...>
>>
>
> Hi,
>
> The tap PMD recently broke for me because of this patch [1].
>
> During init (eth_dev_tap_create()), the tap PMD allocates a shared RX/TX
> queue through tun_alloc().
> The recent patch now releases existing queues in rx_queue_setup(), before
> adding new ones.
>
> When rx_queue_setup() is called, it uses close() calls on all shared queues,
> effectively deleting the netdevice.
> That's the main issue here.
>
> I tested Keith's patch [2], and it fixes that issue, using separate queues.
>
> There is however a couple of other queues-related issues in the tap PMD, but
> I'm not sure how to address them properly:
>
> 1. internals->fds[] gets filled only with RX queues (appart from index 0 that
> is common to both RX and TX).
> It means that RX queues only will be deleted when calling
> rte_pmd_tap_remove() or tap_tx_queue_release().
>
> 2. tap_dev_stop() is not symmetrical with tap_dev_start(): queues won't get
> re-created after a stop.
>
> It may be best to keep the very first fd (created with tun_alloc() in
> eth_dev_tap_create() during probe) apart.
> And then add separate TX/RX queues in internals->txq[] and internals->rxq[]
> respectively.
> What do you think?
>
> [1] d00d7cc88335 ("ethdev: release queue before setting up")
> [2] http://dpdk.org/ml/archives/dev/2017-January/056470.html
Lets keep the current patch just to get over the current problem if everyone
agrees. I will address the comments Pascal brings up as a later updated to the
TAP PMD or I can try to get the other issues cleaned up.
>
>
> --
> Pascal Mazon
> www.6wind.com
Regards,
Keith
Re: [dpdk-dev] [PATCH 1/2] net/mlx5: fix link status query
On Wed, Jan 25, 2017 at 02:42:58PM +0200, Shahaf Shuler wrote:
> Trying to query the link status through new kernel ioctl API
> ETHTOOL_GLINKSETTINGS was always failing due to kernel bug.
> The bug was fixed on version 4.9
> this patch uses the legacy ioctl API for lower kernels.
>
> Fixes: 188408719888 ("net/mlx5: fix support for newer link speeds")
> CC: sta...@dpdk.org
>
> Signed-off-by: Shahaf Shuler
> ---
> while the ETHTOOL_GLINKSETTINGS API was introduced on kernel v4.6 [1]
>
> the bug was fixed only on kernel v4.9 [2]
>
>
>
> [1]
> https://github.com/torvalds/linux/commit/3f1ac7a700d039c61d8d8b99f28d605d489a60cf
>
> [2]
> https://github.com/torvalds/linux/commit/8006f6bf5e39f11c697f48df20382b81d2f2f8b8
>
> ---
> drivers/net/mlx5/mlx5_ethdev.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 8efdff7..e77238f 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -46,6 +46,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /* DPDK headers don't like -pedantic. */
> @@ -697,7 +698,7 @@ struct priv *
>
> /**
> * Retrieve physical link information (unlocked version using new ioctl from
> - * Linux 4.5).
> + * Linux 4.9).
> *
> * @param dev
> * Pointer to Ethernet device structure.
> @@ -707,7 +708,7 @@ struct priv *
> static int
> mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
> {
> -#ifdef ETHTOOL_GLINKSETTINGS
> +#if KERNEL_VERSION(4, 9, 0) <= LINUX_VERSION_CODE
> struct priv *priv = mlx5_get_priv(dev);
> struct ethtool_link_settings edata = {
> .cmd = ETHTOOL_GLINKSETTINGS,
> --
> 1.8.3.1
Hi Shahaf,
This function embeds some HAVE_ETHTOOL_LINK_MODE_* to handle the
different version of Linux Kernel where those link speeds were added.
With this patch, they becomes useless.
It could be great for configuration and compilation time to remove them
from this file and the Makefile.
Regards,
--
Nélio Laranjeiro
6WIND
Re: [dpdk-dev] [PATCH v9 1/7] lib: add information metrics library
Hi Remy,
> This patch adds a new information metric library that allows other
> modules to register named metrics and update their values. It is
> intended to be independent of ethdev, rather than mixing ethdev
> and non-ethdev information in xstats.
I'm still not convinced by this library, and this introduction does
not help a lot.
I would like to thanks Harry for the review of this series.
If we had more opinions or enthousiasm about this patch, it would
be easier to accept this new library and assert it is the way to go.
It could be a matter of technical board discussion if we had a clear
explanation of the needs, the pros and cons of this design.
The overview for using this library should be given in the prog guide.
2017-01-18 15:05, Remy Horton:
> --- a/config/common_base
> +++ b/config/common_base
> @@ -593,3 +593,8 @@ CONFIG_RTE_APP_TEST_RESOURCE_TAR=n
> CONFIG_RTE_TEST_PMD=y
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES=n
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS=n
> +
> +#
> +# Compile the device metrics library
> +#
> +CONFIG_RTE_LIBRTE_METRICS=y
I know the config file is not so well sorted.
However it would be a bit more logical below CONFIG_RTE_LIBRTE_JOBSTATS.
> diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> index 72d59b2..94f0f69 100644
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -150,4 +150,5 @@ There are many libraries, so their headers may be grouped
> by topics:
>[common] (@ref rte_common.h),
>[ABI compat] (@ref rte_compat.h),
>[keepalive] (@ref rte_keepalive.h),
> + [Device Metrics] (@ref rte_metrics.h),
No first letter uppercase in this list.
> --- a/doc/guides/rel_notes/release_17_02.rst
> +++ b/doc/guides/rel_notes/release_17_02.rst
> @@ -34,6 +34,12 @@ New Features
>
> Refer to the previous release notes for examples.
>
> + * **Added information metric library.**
> +
> + A library that allows information metrics to be added and update. It is
update -> updated
added and updated by who?
> + intended to provide a reporting mechanism that is independent of the
> + ethdev library.
and independent of the cryptodev library?
Does it apply to other types of devices (cryptodev/eventdev)?
> +
> This section is a comment. do not overwrite or remove it.
> Also, make sure to start the actual text at the margin.
> =
Your text should start below this line, and indented at the margin.
> @@ -205,6 +211,7 @@ The libraries prepended with a plus sign were incremented
> in this version.
> .. code-block:: diff
>
> librte_acl.so.2
> + + librte_bitratestats.so.1
not part of this patch
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -58,6 +58,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_TABLE) += librte_table
> DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += librte_pipeline
> DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += librte_reorder
> DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += librte_pdump
> +DIRS-$(CONFIG_RTE_LIBRTE_METRICS) += librte_metrics
insert it below librte_jobstats is a better choice
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.c
> +/**
> + * Internal stats metadata and value entry.
> + *
> + * @internal
> + * @param name
> + * Name of metric
> + * @param value
> + * Current value for metric
> + * @param idx_next_set
> + * Index of next root element (zero for none)
> + * @param idx_next_metric
> + * Index of next metric in set (zero for none)
> + *
> + * Only the root of each set needs idx_next_set but since it has to be
> + * assumed that number of sets could equal total number of metrics,
> + * having a separate set metadata table doesn't save any memory.
> + */
> +struct rte_metrics_meta_s {
> + char name[RTE_METRICS_MAX_NAME_LEN];
> + uint64_t value[RTE_MAX_ETHPORTS];
> + uint64_t nonport_value;
> + uint16_t idx_next_set;
> + uint16_t idx_next_stat;
> +};
It would be a lot easier to read with comments near each field.
It would avoid to forget some fields like nonport_value in this struct.
You do not need to use a doxygen syntax in a .c file.
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.h
> +/**
> + * @file
> + *
> + * RTE Metrics module
RTE is not meaningful here.
Please prefer DPDK.
> + *
> + * Metric information is populated using a push model, where the
> + * information provider calls an update function on the relevant
> + * metrics. Currently only bulk querying of metrics is supported.
> + */
This description should explain who is a provider (drivers?) and who
is the reader (registered thread?).
What do you mean by "push model"? A callback is used?
> +/**
> + * Global (rather than port-specific) metric.
It does not say what kind of constant it is. A special metric id?
> + *
> + * When used instead of port number by rte_metrics_update_metric()
> + * or rte_metrics_update_metric(), the global metrics, which are
> + * not associated with any specific p
Re: [dpdk-dev] [PATCH v7] app/testpmd: supported offload capabilities query
> > Add two new commands "show port cap " and "show > > port cap all"to diaplay what offload capabilities supported > > in ports. It will not only display all the capabilities of > > the port, but also the enabling condition for each capability > > in the running time. > > > > Signed-off-by: Qiming Yang > > Acked-by: Jingjing Wu > > Acked-by: Beilei Xing > > Acked-by: Pablo de Lara Applied, thanks
Re: [dpdk-dev] [PATCH] pdump: fix nb_rx_q may be used uninitialized warning
2017-01-18 07:26, Andrew Rybchenko:
> Fix GCC 4.8.2 20140120 (Red Hat 4.8.2-16) (RHEL 7.0) false warning
> when build with EXTRA_CFLAGS='--coverage'.
>
> Fixes: 278f945402c5 ("pdump: add new library for packet capture")
>
> Signed-off-by: Andrew Rybchenko
Applied, thanks
Re: [dpdk-dev] [PATCH] app/test: make PCI device id struct const
2017-01-27 09:53, Ferruh Yigit: > Signed-off-by: Ferruh Yigit Applied, thanks
Re: [dpdk-dev] [PATCH] app/test: fix overflow in EFD test
2017-01-25 11:29, Pablo de Lara:
> When RTE_EFD_VALUE_NUM_BITS is 32, there was a compilation issue
> because of an overflow:
>
> app/test/test_efd.c:157:55: error: overflow in expression;
> result is 2147483647 with type 'int' [-Werror,-Winteger-overflow]
> data[0] = mrand48() & ((1 << RTE_EFD_VALUE_NUM_BITS) - 1);
>
> This commit fixes the issue by using a setting a different
> macro VALUE_BITMASK with a conditional
>
> Fixes: 0e925aef2779 ("app/test: add EFD functional and perf tests")
>
> Reported-by: Yong Liu
> Signed-off-by: Pablo de Lara
Applied, thanks
Re: [dpdk-dev] [PATCH] examples/flow-distributor: rename to server_node_efd
2017-01-24 09:06, Pablo de Lara: > To avoid confusion with distributor app, this commit > renames the flow-distributor sample app to server_node_efd, > since it shows how to use the EFD library and it is based > on a server/nodes model. > > Signed-off-by: Pablo de Lara Are you sure of the name? :) Applied with a fix: --- a/examples/Makefile +++ b/examples/Makefile -DIRS-$(CONFIG_RTE_LIBRTE_EFD) += flow_distributor +DIRS-$(CONFIG_RTE_LIBRTE_EFD) += server_node_efd
Re: [dpdk-dev] [PATCH v2] tools: fix active interface detection in dpdk-devbind.py
2017-01-10 17:14, Yoni Gilad:
> When adding crypto devices, the "Active" and "Ssh_if" attributes of
> existing network devices were reset. This causes the following issues:
>
> - Network interfaces aren't marked as "*Active*" in the --status output.
> - Active network interfaces can be unbound without the --force option,
> causing loss of network connection.
>
> The reset was caused by the call to devices[d].update in
> get_crypto_details.
>
> This patch prevents the update on non-crypto devices.
>
> Fixes: cb4a1d14bf3e ("tools: bind crypto devices")
>
> CC: sta...@dpdk.org
>
> Signed-off-by: Yoni Gilad
> Acked-by: Pablo de Lara
Applied, thanks
Re: [dpdk-dev] [PATCH] crypto/scheduler: fix name parameter parsing
> -Original Message-
> From: Zhang, Roy Fan
> Sent: Monday, January 30, 2017 11:19 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo
> Subject: [PATCH] crypto/scheduler: fix name parameter parsing
>
> This patch fixes the name parsing issue. Originally, the unique
> scheduler name created by system is not passed to vdev
> initializer.
>
> Fixes: 8b483eae ("crypto/scheduler: register scheduler vdev driver")
>
> Signed-off-by: Fan Zhang
> ---
Applied to dpdk-next-crypto and merged to "crypto/scheduler: register scheduler
vdev driver".
Thanks,
Pablo
Re: [dpdk-dev] [PATCH 07/25] eal: Signal error when CPU isn't supported
Stephen Hemminger writes:
> On Fri, 27 Jan 2017 09:56:45 -0500
> Aaron Conole wrote:
>
>> It's now possible to gracefully exit the application, or for
>> applications which support non-dpdk datapaths working in concert with
>> DPDK datapaths, there no longer is the possibility of exiting for
>> unsupported CPUs.
>>
>> Signed-off-by: Aaron Conole
>> ---
>> lib/librte_eal/linuxapp/eal/eal.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
>> b/lib/librte_eal/linuxapp/eal/eal.c
>> index 413be16..cd976f5 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -752,7 +752,10 @@ rte_eal_init(int argc, char **argv)
>> char thread_name[RTE_MAX_THREAD_NAME_LEN];
>>
>> /* checks if the machine is adequate */
>> -rte_cpu_check_supported();
>> +if (!rte_cpu_is_supported()) {
>> +rte_errno = ENOTSUP;
>> +return -1;
>> +}
>>
>
> I like not having DPDK applications panic.
> My concern is that naive user will not know to check rte_errno. Why not put
> a high severity error out as well. If logging is not up just use stderr.
I'll work in a quick blurt using stderr.
Thanks for the review, Stephen!
-Aaron
Re: [dpdk-dev] [PATCH 14/25] eal: do not panic on tailq init
Stephen Hemminger writes: > On Fri, 27 Jan 2017 09:56:52 -0500 > Aaron Conole wrote: > >> + /* no need to TAILQ_REMOVE, we are going to disallow re-attemtps >> + * for rte_eal_init(). */ > > Please put multi-line comments in form: > /* > * this is a long comment > * because there really is lots to say > */ Okay, will do. > In many cases, having shorter comment is the best way to handle these. > Often developer writes long comment for themselves because what ever problem > they saw was urgent. Then comment becomes irrelevant later. > > /* TAILQ_REMOVE not needed, error is already fatal */ I'll fold this in. Thanks for the review!
Re: [dpdk-dev] [PATCH 15/25] eal: do not panic on alarm init
Bruce Richardson writes:
> On Fri, Jan 27, 2017 at 08:31:55AM -0800, Stephen Hemminger wrote:
>> On Fri, 27 Jan 2017 09:56:53 -0500
>> Aaron Conole wrote:
>>
>> > + if (rte_eal_alarm_init() < 0) {
>> > + RTE_LOG (ERR, EAL, "Cannot init interrupt-handling thread\n");
>> > + /* rte_eal_alarm_init sets rte_errno on failure. */
>> > + errno = rte_errno;
>>
>> Hmm. DPDK in general does not reset errno but instead uses error code
>> directly on return (best) or in some cases rte_errno
>
> I think we'll disagree on what way of returning error codes is best :-), but
> yes, DPDK does not generally modify errno.
Okay, I'll drop the errno set. I think it's a mistake from the first
version of the series (as RFC).
Thanks for the reviews, Bruce and Stephen!
[dpdk-dev] [PATCH v5] net/kni: add KNI PMD
Add KNI PMD which wraps librte_kni for ease of use. KNI PMD can be used as any regular PMD to send / receive packets to the Linux networking stack. Signed-off-by: Ferruh Yigit --- v5: * add kvargs "no_request_thread" to disable a specific pthread creation to handle control requests. * add documentation v4: * allow only single queue * use driver.name as name v3: * rebase on top of latest master v2: * updated driver name eth_kni -> net_kni --- MAINTAINERS | 5 + config/common_base | 1 + config/common_linuxapp | 1 + doc/guides/nics/features/kni.ini| 7 + doc/guides/nics/index.rst | 1 + doc/guides/nics/kni.rst | 197 drivers/net/Makefile| 1 + drivers/net/kni/Makefile| 64 drivers/net/kni/rte_eth_kni.c | 515 drivers/net/kni/rte_pmd_kni_version.map | 4 + mk/rte.app.mk | 10 +- 11 files changed, 801 insertions(+), 5 deletions(-) create mode 100644 doc/guides/nics/features/kni.ini create mode 100644 doc/guides/nics/kni.rst create mode 100644 drivers/net/kni/Makefile create mode 100644 drivers/net/kni/rte_eth_kni.c create mode 100644 drivers/net/kni/rte_pmd_kni_version.map diff --git a/MAINTAINERS b/MAINTAINERS index f071138..8eb83f5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -404,6 +404,11 @@ M: Keith Wiles F: drivers/net/tap/ F: doc/guides/nics/tap.rst +KNI PMD +M: Ferruh Yigit +F: drivers/net/kni/ +F: doc/guides/nics/kni.rst + Ring PMD M: Bruce Richardson F: drivers/net/ring/ diff --git a/config/common_base b/config/common_base index 61efb87..2e1bbd5 100644 --- a/config/common_base +++ b/config/common_base @@ -576,6 +576,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n # Compile librte_kni # CONFIG_RTE_LIBRTE_KNI=n +CONFIG_RTE_LIBRTE_PMD_KNI=n CONFIG_RTE_KNI_KMOD=n CONFIG_RTE_KNI_KMOD_ETHTOOL=n CONFIG_RTE_KNI_PREEMPT_DEFAULT=y diff --git a/config/common_linuxapp b/config/common_linuxapp index 00ebaac..d03a60a 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y CONFIG_RTE_EAL_VFIO=y CONFIG_RTE_KNI_KMOD=y CONFIG_RTE_LIBRTE_KNI=y +CONFIG_RTE_LIBRTE_PMD_KNI=y CONFIG_RTE_LIBRTE_VHOST=y CONFIG_RTE_LIBRTE_PMD_VHOST=y CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y diff --git a/doc/guides/nics/features/kni.ini b/doc/guides/nics/features/kni.ini new file mode 100644 index 000..6deb66a --- /dev/null +++ b/doc/guides/nics/features/kni.ini @@ -0,0 +1,7 @@ +; +; Supported features of the 'kni' network poll mode driver. +; +; Refer to default.ini for the full list of available PMD features. +; +[Features] +Usage doc= Y diff --git a/doc/guides/nics/index.rst b/doc/guides/nics/index.rst index 87f9334..5248625 100644 --- a/doc/guides/nics/index.rst +++ b/doc/guides/nics/index.rst @@ -46,6 +46,7 @@ Network Interface Controller Drivers i40e ixgbe intel_vf +kni mlx4 mlx5 nfp diff --git a/doc/guides/nics/kni.rst b/doc/guides/nics/kni.rst new file mode 100644 index 000..2e2032b --- /dev/null +++ b/doc/guides/nics/kni.rst @@ -0,0 +1,197 @@ +.. BSD LICENSE +Copyright(c) 2017 Intel Corporation. All rights reserved. +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +* Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. +* Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in +the documentation and/or other materials provided with the +distribution. +* Neither the name of Intel Corporation nor the names of its +contributors may be used to endorse or promote products derived +from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +KNI Poll Mode Driver +== + +KNI PMD is wrapper to the :ref:`librte_kni ` library. + +This PMD
Re: [dpdk-dev] [PATCH] config: add default MPIPE PMD config to common base
On 1/30/2017 10:46 AM, Thomas Monjalon wrote: > 2017-01-30 10:40, Ferruh Yigit: >> --- a/config/common_base >> +++ b/config/common_base >> @@ -219,6 +219,12 @@ CONFIG_RTE_LIBRTE_MLX5_DEBUG=n >> CONFIG_RTE_LIBRTE_MLX5_TX_MP_CACHE=8 >> >> # >> +# MPIPE PMD >> +# >> +CONFIG_RTE_LIBRTE_MPIPE_PMD=n >> +CONFIG_RTE_LIBRTE_MPIPE_PMD_DEBUG=n > > NACK > > I think the whole tile arch must be removed from DPDK. > It is not maintained. I also have question about if newly added (and enabled by default) PMDs should be disabled for this arch or not. It would be nice to get an update from maintainers. Thanks, ferruh
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
On 1/30/2017 2:38 PM, Pascal Mazon wrote:
> On 01/30/2017 12:00 PM, Ferruh Yigit wrote:> On 1/29/2017 2:12 AM, Keith
> Wiles wrote:
>>> The tap driver setup both rx and tx file descriptors when the
>>> rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
>>> was called.
>>
>> Can you please describe the problem more.
>> Without this patch rx->fd == tx->fd, with this patch rx and tx has
>> different file descriptors.
>>
>> What was the wrong with rx and tx having same fd?
>>
>> As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that
>> function will do nothing if rx or tx has valid fd.
>>
>>>
>>> Signed-off-by: Keith Wiles
>>
>> <...>
>>
>
> Hi,
>
> The tap PMD recently broke for me because of this patch [1].
>
> During init (eth_dev_tap_create()), the tap PMD allocates a shared RX/TX
> queue through tun_alloc().
> The recent patch now releases existing queues in rx_queue_setup(),
> before adding new ones.
>
> When rx_queue_setup() is called, it uses close() calls on all shared
> queues, effectively deleting the netdevice.
> That's the main issue here.
>
> I tested Keith's patch [2], and it fixes that issue, using separate queues.
Thanks for the clarification, and I am adding following patch to patch [2]:
Tested-by: Pascal Mazon
<...>
>
> [1] d00d7cc88335 ("ethdev: release queue before setting up")
> [2] http://dpdk.org/ml/archives/dev/2017-January/056470.html
>
>
Re: [dpdk-dev] rte_eth_from_rings
Thanks Bruce, Hi Ferruh, I experimented the following to get the KNI interface up, but not able to get it up yet. rte_kni_alloc(..) // with rte_eth_dev_info = NULL ; ; rte_kni_register_handlers // with rte_eth_dev_info set with right call_back for port_up_down and if_mtu ; ; ; I also added rte_kni_handle_request(kni_p) with these i am not able to bring the port UP. I also tried to do ioctl in interface, ifr.ifr_flags |= (IFF_UP | IFF_RUNNING); ioctl(fp_p->port_ioctl, SIOCSIFFLAGS, &ifr); this call fails. What am i missing ? can you kindly point out my mistake here. the eth_dev the KNI bound to is the eth_dev created from ring buffer. (my dev environment is ubuntu 16.04 VM but eventual the code will be running on ONL. do i need to have any kernel flags. i am running rte_kni.ko without any args) Thanks, Sridhar **put some syslog into ret_kni and find rte_kni_handle_request is failling at ret = kni_fifo_get(kni->req_q, (void **)&req, 1); From: Bruce Richardson Sent: Monday, January 30, 2017 2:33 AM To: Sridhar Pitchai Cc: dev@dpdk.org; ferruh.yi...@intel.com Subject: Re: [dpdk-dev] rte_eth_from_rings On Fri, Jan 27, 2017 at 10:31:52PM +, Sridhar Pitchai wrote: > Thanks Bruce. > > > I have created eth_dev from the rings as below. > rt = rte_eth_from_rings(port_p->name, > (struct rte_ring * const*)port_p->rx_ring_p, 2, > (struct rte_ring * const*)port_p->tx_ring_p, 2, > rte_socket_id()); > > Lets say I have a call back something like > > pkt_rx(void * pkt_p, struct pkt_metadata_t *meta_p) > > which is called when there is a pkt at the chip. > inside this function(pkt_rx) i will find the port and the corresponding > ring_p and > enqueue the pkt into the queue. I am assuming this should work. Kindly > correct me if i misunderstood you. > Yes, it should work. > > question 2: > Can use this eth_dev to create KNI interface, like below. > rte_eth_dev_info_get(fp_p->port_list[port].key.port_id, &dev_info); > ; > ; > ; > ops.port_id = fp_p->port_list[port].key.port_id; // rt from > rte_eth_from_rings(...) > ops.change_mtu = _kni_ifconfig_mtu; // static function > ops.config_network_if = _kni_ifconfig; // static functions > PORT(fp_p, port).kni_p = rte_kni_alloc(PORT(fp_p, port).mbuf_p, > &conf, &ops); > > I am assuming this should work as well. I find the netdevs created but when i > try to configure > them i am facing the following error. > > root@ VM snaproute/softpath (master) >ifconfig PORT_8 > PORT_8Link encap:Ethernet HWaddr be:35:c3:0f:f8:2f > BROADCAST MULTICAST MTU:1500 Metric:1 > RX packets:0 errors:0 dropped:0 overruns:0 frame:0 > TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:0 (0.0 B) TX bytes:0 (0.0 B) > > root@ VM snaproute/softpath (master) >ifconfig PORT_8 20.1.1.1/24 up > SIOCSIFFLAGS: Timer expired > SIOCSIFFLAGS: Timer expired > root@ VM snaproute/softpath (master) > > +Ferruh as KNI maintainer, to see if he can help out with this question. /Bruce > Thanks for the help. > > Thanks, > Sridhar Pitchai > > > > From: Bruce Richardson > Sent: Friday, January 27, 2017 1:36 PM > To: Sridhar Pitchai > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] rte_eth_from_rings > > On Fri, Jan 27, 2017 at 07:16:25PM +, Sridhar Pitchai wrote: > > Hi, > > > > I am trying to write a data path for packets punted to CPU(slowpath) from > > vender silicon like broadcom. I am planing to use "rte_eth_from_rings" like > > model where I will be able to read and write to the ring for the packets > > punted from vendor chip. > > > > > > the eth_dev abstraction provided by "rte_eth_from_rings" helps to build the > > dpdk data path. Can someone help me on how to read and write to the rings > > that is emulating the eth_dev. > > > > > To use the rings like an ethdev just use rte_eth_rx_burst and > rte_eth_tx_burst, passing in the port id of your newly created rings > ethdev. To access them directly as rings, just use the ring > enqueue/dequeue functions passing in the ring pointer as normal. > > Regards, > /Bruce
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
On 1/30/2017 2:34 PM, Wiles, Keith wrote:
>
>> On Jan 30, 2017, at 5:00 AM, Yigit, Ferruh wrote:
>>
>> On 1/29/2017 2:12 AM, Keith Wiles wrote:
>>> The tap driver setup both rx and tx file descriptors when the
>>> rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
>>> was called.
>>
>> Can you please describe the problem more.
>> Without this patch rx->fd == tx->fd, with this patch rx and tx has
>> different file descriptors.
>
> Let me look at this more, I am getting the same FD for both. Must be
> something else going on.
After patch, tun_alloc() called twice, one for Rx_q and other for Tx_q.
And tun_alloc does open() to "/dev/net/tun", I expect they get different
file descriptors.
And if they have same FD, won't this cause same problem,
rx_queue_setup() will close the FD, if Tx_q has same FD it will have
invalid descriptor.
>
>>
>> What was the wrong with rx and tx having same fd?
>>
>> As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that
>> function will do nothing if rx or tx has valid fd.
>
> The rte_eth_rx_queue_setup() look at line 1146 if rxq has a value then
> release it, which happens on both Rx/Tx code
>
> rxq = dev->data->rx_queues;
> if (rxq[rx_queue_id]) {
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release,
> -ENOTSUP);
> (*dev->dev_ops->rx_queue_release)(rxq[rx_queue_id]);
> rxq[rx_queue_id] = NULL;
> }
Got it thanks, I missed (relatively new) above code piece.
>
> if (rx_conf == NULL)
> rx_conf = &dev_info.default_rxconf;
>
> ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
> socket_id, rx_conf, mp);
>
>>
>>>
>>> Signed-off-by: Keith Wiles
>>
>> <...>
>
> Regards,
> Keith
>
Re: [dpdk-dev] [PATCH 3/3] doc: remove ABI changes in igb_uio
2017-01-24 13:35, Ferruh Yigit: > On 1/24/2017 7:34 AM, Jianfeng Tan wrote: > > We announced ABI changes to remove iomem and ioport mapping in > > igb_uio. But it has potential backward compatibility issue: cannot > > run old version DPDK on modified igb_uio. > > > > The purpose of this changes was to fix a bug: when DPDK app crashes, > > those devices by igb_uio are not stopped either DPDK PMD driver or > > igb_uio driver. We need to figure out new way to fix this bug. > > Hi Jianfeng, > > I believe it would be good to fix this potential defect. > > Is "remove iomem and ioport" a must for that fix? If so, I suggest > re-think about it. > > If I see correctly, dpdk1.8 and older uses igb_uio iomem files. So > backward compatibility is the possible issue for dpdk1.8 and older. > Since v1.8 two years old, I would prefer fixing defect instead of > keeping that backward compatibility. > > Jianfeng, Thomas, > > What do you think postponing this deprecation notice to next release, > instead of removing it, and discuss more? > > > And overall, if "remove iomem and ioport" is not a must for this fix, no > problem to remove deprecation notice. I have no strong opinion here. Jianfeng, do you agree with Ferruh?
Re: [dpdk-dev] [PATCH] doc: add PMD specific API
2017-01-27 12:27, Ferruh Yigit: > --- a/doc/api/doxy-api-index.md > +++ b/doc/api/doxy-api-index.md > @@ -153,3 +153,7 @@ There are many libraries, so their headers may be grouped > by topics: >[ABI compat] (@ref rte_compat.h), >[keepalive] (@ref rte_keepalive.h), >[version](@ref rte_version.h) > + > +- **PMD specific**: > + [ixgbe] (@ref rte_pmd_ixgbe.h), > + [i40e] (@ref rte_pmd_i40e.h) They could be grouped with bonding, vhost and KNI in a section device-specific, below "device" section.
Re: [dpdk-dev] [PATCH] doc: update release notes for I/O device memory access API
> > Signed-off-by: Jerin Jacob > > Acked-by: John McNamara Applied, thanks
Re: [dpdk-dev] [PATCH v2] mk: parallelize make config
2017-01-30 10:21, Ferruh Yigit: > make config dependency resolving was always running serial, > parallelize it for better performance. > > $ time make T=x86_64-native-linuxapp-gcc config > real0m12.633s > > $ time make -j8 T=x86_64-native-linuxapp-gcc config > real0m1.826s > > When config creation done under a single make target, using a for loop, > make has no control on the action, and it needs to run as implemented in > the rule. But if for loop converted into multiple targets, make can > detect independent targets and run them parallel based on -j parameter. > > Signed-off-by: Ferruh Yigit Looks a good improvement, thanks Ferruh. Acked-by: Thomas Monjalon Applied
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
> On Jan 30, 2017, at 11:42 AM, Yigit, Ferruh wrote:
>
> On 1/30/2017 2:34 PM, Wiles, Keith wrote:
>>
>>> On Jan 30, 2017, at 5:00 AM, Yigit, Ferruh wrote:
>>>
>>> On 1/29/2017 2:12 AM, Keith Wiles wrote:
The tap driver setup both rx and tx file descriptors when the
rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
was called.
>>>
>>> Can you please describe the problem more.
>>> Without this patch rx->fd == tx->fd, with this patch rx and tx has
>>> different file descriptors.
>>
>> Let me look at this more, I am getting the same FD for both. Must be
>> something else going on.
>
> After patch, tun_alloc() called twice, one for Rx_q and other for Tx_q.
> And tun_alloc does open() to "/dev/net/tun", I expect they get different
> file descriptors.
It is not called twice, it is only called once in the eth_dev_tap_create()
routine and the fd is placed in the rxq/txq using the same fd. Then look in the
rx/tx_setup_queue routines only update the fd and call tun_alloc if the fd is
-1. Now looking at this code it seems a bit silly, but it was trying to deal
with the setting up the new queue. It seems to be this logic not going to work
with multiple queues in the same device and needs to be reworked.
I need to rework the code and do some cleanup. The current patch should work
for a single queue per device.
Thanks
>
> And if they have same FD, won't this cause same problem,
> rx_queue_setup() will close the FD, if Tx_q has same FD it will have
> invalid descriptor.
>
>>
>>>
>>> What was the wrong with rx and tx having same fd?
>>>
>>> As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that
>>> function will do nothing if rx or tx has valid fd.
>>
>> The rte_eth_rx_queue_setup() look at line 1146 if rxq has a value then
>> release it, which happens on both Rx/Tx code
>>
>> rxq = dev->data->rx_queues;
>> if (rxq[rx_queue_id]) {
>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release,
>> -ENOTSUP);
>> (*dev->dev_ops->rx_queue_release)(rxq[rx_queue_id]);
>> rxq[rx_queue_id] = NULL;
>> }
>
> Got it thanks, I missed (relatively new) above code piece.
>
>>
>> if (rx_conf == NULL)
>> rx_conf = &dev_info.default_rxconf;
>>
>> ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
>>socket_id, rx_conf, mp);
>>
>>>
Signed-off-by: Keith Wiles
>>>
>>> <...>
>>
>> Regards,
>> Keith
Regards,
Keith
[dpdk-dev] [PATCH v2] doc: add device specific API
Signed-off-by: Ferruh Yigit --- doc/api/doxy-api-index.md | 6 +- doc/api/doxy-api.conf | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index f9958c4..ff57784 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -43,10 +43,14 @@ There are many libraries, so their headers may be grouped by topics: [rte_flow_driver](@ref rte_flow_driver.h), [cryptodev] (@ref rte_cryptodev.h), [devargs](@ref rte_devargs.h), + [PCI](@ref rte_pci.h), + +- **device specific**: [bond] (@ref rte_eth_bond.h), [vhost] (@ref rte_virtio_net.h), [KNI](@ref rte_kni.h), - [PCI](@ref rte_pci.h), + [ixgbe] (@ref rte_pmd_ixgbe.h), + [i40e] (@ref rte_pmd_i40e.h) - **memory**: [memseg] (@ref rte_memory.h), diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf index 6892315..b8a5fd8 100644 --- a/doc/api/doxy-api.conf +++ b/doc/api/doxy-api.conf @@ -32,6 +32,8 @@ PROJECT_NAME= DPDK INPUT = doc/api/doxy-api-index.md \ doc/api/examples.dox \ drivers/net/bonding \ + drivers/net/i40e \ + drivers/net/ixgbe \ lib/librte_eal/common/include \ lib/librte_eal/common/include/generic \ lib/librte_acl \ -- 2.9.3
[dpdk-dev] [PATCH v3] doc: add device specific API
Signed-off-by: Ferruh Yigit --- v3: * remove "," after [PCI] --- doc/api/doxy-api-index.md | 6 +- doc/api/doxy-api.conf | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index f9958c4..eb39f69 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -43,10 +43,14 @@ There are many libraries, so their headers may be grouped by topics: [rte_flow_driver](@ref rte_flow_driver.h), [cryptodev] (@ref rte_cryptodev.h), [devargs](@ref rte_devargs.h), + [PCI](@ref rte_pci.h) + +- **device specific**: [bond] (@ref rte_eth_bond.h), [vhost] (@ref rte_virtio_net.h), [KNI](@ref rte_kni.h), - [PCI](@ref rte_pci.h), + [ixgbe] (@ref rte_pmd_ixgbe.h), + [i40e] (@ref rte_pmd_i40e.h) - **memory**: [memseg] (@ref rte_memory.h), diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf index 6892315..b8a5fd8 100644 --- a/doc/api/doxy-api.conf +++ b/doc/api/doxy-api.conf @@ -32,6 +32,8 @@ PROJECT_NAME= DPDK INPUT = doc/api/doxy-api-index.md \ doc/api/examples.dox \ drivers/net/bonding \ + drivers/net/i40e \ + drivers/net/ixgbe \ lib/librte_eal/common/include \ lib/librte_eal/common/include/generic \ lib/librte_acl \ -- 2.9.3
Re: [dpdk-dev] [PATCH 25/25] rte_eal_init: add info about rte_errno codes
Stephen Hemminger writes: > On Fri, 27 Jan 2017 16:47:40 + > Bruce Richardson wrote: > >> On Fri, Jan 27, 2017 at 08:33:46AM -0800, Stephen Hemminger wrote: >> > On Fri, 27 Jan 2017 09:57:03 -0500 >> > Aaron Conole wrote: >> > >> > > diff --git a/lib/librte_eal/common/include/rte_eal.h >> > > b/lib/librte_eal/common/include/rte_eal.h >> > > index 03fee50..46e427f 100644 >> > > --- a/lib/librte_eal/common/include/rte_eal.h >> > > +++ b/lib/librte_eal/common/include/rte_eal.h >> > > @@ -159,7 +159,29 @@ int rte_eal_iopl_init(void); >> > > * function call and should not be further interpreted by the >> > > * application. The EAL does not take any ownership of the memory >> > > used >> > > * for either the argv array, or its members. >> > > - * - On failure, a negative error value. >> > > + * - On failure, -1 and rte_errno is set to a value indicating the >> > > cause >> > > + * for failure. >> > > + * >> > > + * The error codes returned via rte_errno: >> > > + * EACCES indicates a permissions issue. >> > > + * >> > > + * EAGAIN indicates either a bus or system resource was not >> > > available, >> > > + *try again. >> > > + * >> > > + * EALREADY indicates that the rte_eal_init function has already >> > > been >> > > + * called, and cannot be called again. >> > > + * >> > > + * EINVAL indicates invalid parameters were passed as argv/argc. >> > > + * >> > > + * EIO indicates failure to setup the logging handlers. This is >> > > usually >> > > + * caused by an out-of-memory condition. >> > > + * >> > > + * ENODEV indicates memory setup issues. >> > > + * >> > > + * ENOTSUP indicates that the EAL cannot initialize on this system. >> > > + * >> > > + * EUNATCH indicates that the PCI bus is either not present, or is >> > > not >> > > + * readable by the eal. >> > > */ >> > > int rte_eal_init(int argc, char **argv); >> > >> > Why use rte_errno? >> > Most DPDK calls just return negative value on error which >> > corresponds to error number. >> > Are you trying to keep ABI compatibility? Doesn't make sense >> > because before all these >> > errors were panic's no working application is going to care. >> >> Either will work, but I actually prefer this way. I view using rte_errno >> to be better as it can work in just about all cases, including with >> functions which return pointers. This allows you to have a standard >> method across all functions for returning error codes, and it only >> requires a single sentinal value to indicate error, rather than using a >> whole range of values. > > The problem is DPDK is getting more inconsistent on how this is done. > As long as error returns are always same as kernel/glibc errno's it really > doesn't > matter much which way the value is returned from a technical point of view > but the inconsistency is sure to be a usability problem and source of errors. I am using rte_errno here because I assumed it was the preferred method. In fact, looking at some recently contributed modules (for instance pdump), it seems that folks are using it. I'm not really sure the purpose of having rte_errno if it isn't used, so it'd be helpful to know if there's some consensus on reflecting errors via this variable, or on returning error codes. Whichever is the more consistent with the way the DPDK project does things, I'm game :). Thanks for the thoughts, and review.
Re: [dpdk-dev] [PATCH v5] net/kni: add KNI PMD
> -Original Message- > From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] > Sent: Monday, January 30, 2017 8:58 AM > To: Thomas Monjalon ; Yong Wang > > Cc: dev@dpdk.org; Ferruh Yigit > Subject: [PATCH v5] net/kni: add KNI PMD > > Add KNI PMD which wraps librte_kni for ease of use. > > KNI PMD can be used as any regular PMD to send / receive packets to the > Linux networking stack. > > Signed-off-by: Ferruh Yigit > --- Looks good except a few typos in the documentation added. Reviewed-by: Yong Wang > > v5: > * add kvargs "no_request_thread" to disable a specific pthread creation > to handle control requests. > * add documentation > > v4: > * allow only single queue > * use driver.name as name > > v3: > * rebase on top of latest master > > v2: > * updated driver name eth_kni -> net_kni > --- > MAINTAINERS | 5 + > config/common_base | 1 + > config/common_linuxapp | 1 + > doc/guides/nics/features/kni.ini| 7 + > doc/guides/nics/index.rst | 1 + > doc/guides/nics/kni.rst | 197 > drivers/net/Makefile| 1 + > drivers/net/kni/Makefile| 64 > drivers/net/kni/rte_eth_kni.c | 515 > > drivers/net/kni/rte_pmd_kni_version.map | 4 + > mk/rte.app.mk | 10 +- > 11 files changed, 801 insertions(+), 5 deletions(-) > create mode 100644 doc/guides/nics/features/kni.ini > create mode 100644 doc/guides/nics/kni.rst > create mode 100644 drivers/net/kni/Makefile > create mode 100644 drivers/net/kni/rte_eth_kni.c > create mode 100644 drivers/net/kni/rte_pmd_kni_version.map > > diff --git a/MAINTAINERS b/MAINTAINERS > index f071138..8eb83f5 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -404,6 +404,11 @@ M: Keith Wiles > F: drivers/net/tap/ > F: doc/guides/nics/tap.rst > > +KNI PMD > +M: Ferruh Yigit > +F: drivers/net/kni/ > +F: doc/guides/nics/kni.rst > + > Ring PMD > M: Bruce Richardson > F: drivers/net/ring/ > diff --git a/config/common_base b/config/common_base > index 61efb87..2e1bbd5 100644 > --- a/config/common_base > +++ b/config/common_base > @@ -576,6 +576,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n > # Compile librte_kni > # > CONFIG_RTE_LIBRTE_KNI=n > +CONFIG_RTE_LIBRTE_PMD_KNI=n > CONFIG_RTE_KNI_KMOD=n > CONFIG_RTE_KNI_KMOD_ETHTOOL=n > CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 00ebaac..d03a60a 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y > CONFIG_RTE_EAL_VFIO=y > CONFIG_RTE_KNI_KMOD=y > CONFIG_RTE_LIBRTE_KNI=y > +CONFIG_RTE_LIBRTE_PMD_KNI=y > CONFIG_RTE_LIBRTE_VHOST=y > CONFIG_RTE_LIBRTE_PMD_VHOST=y > CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y > diff --git a/doc/guides/nics/features/kni.ini > b/doc/guides/nics/features/kni.ini > new file mode 100644 > index 000..6deb66a > --- /dev/null > +++ b/doc/guides/nics/features/kni.ini > @@ -0,0 +1,7 @@ > +; > +; Supported features of the 'kni' network poll mode driver. > +; > +; Refer to default.ini for the full list of available PMD features. > +; > +[Features] > +Usage doc= Y > diff --git a/doc/guides/nics/index.rst b/doc/guides/nics/index.rst > index 87f9334..5248625 100644 > --- a/doc/guides/nics/index.rst > +++ b/doc/guides/nics/index.rst > @@ -46,6 +46,7 @@ Network Interface Controller Drivers > i40e > ixgbe > intel_vf > +kni > mlx4 > mlx5 > nfp > diff --git a/doc/guides/nics/kni.rst b/doc/guides/nics/kni.rst > new file mode 100644 > index 000..2e2032b > --- /dev/null > +++ b/doc/guides/nics/kni.rst > @@ -0,0 +1,197 @@ > +.. BSD LICENSE > +Copyright(c) 2017 Intel Corporation. All rights reserved. > +All rights reserved. > + > +Redistribution and use in source and binary forms, with or without > +modification, are permitted provided that the following conditions > +are met: > + > +* Redistributions of source code must retain the above copyright > +notice, this list of conditions and the following disclaimer. > +* Redistributions in binary form must reproduce the above copyright > +notice, this list of conditions and the following disclaimer in > +the documentation and/or other materials provided with the > +distribution. > +* Neither the name of Intel Corporation nor the names of its > +contributors may be used to endorse or promote products derived > +from this software without specific prior written permission. > + > +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > CONTRIBUTORS > +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT > NOT > +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND > FITNESS FOR > +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > COPYRIGHT > +OWNER OR CONTRIBUTORS
Re: [dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
On 1/30/2017 6:20 PM, Wiles, Keith wrote:
>
>> On Jan 30, 2017, at 11:42 AM, Yigit, Ferruh wrote:
>>
>> On 1/30/2017 2:34 PM, Wiles, Keith wrote:
>>>
On Jan 30, 2017, at 5:00 AM, Yigit, Ferruh wrote:
On 1/29/2017 2:12 AM, Keith Wiles wrote:
> The tap driver setup both rx and tx file descriptors when the
> rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
> was called.
Can you please describe the problem more.
Without this patch rx->fd == tx->fd, with this patch rx and tx has
different file descriptors.
>>>
>>> Let me look at this more, I am getting the same FD for both. Must be
>>> something else going on.
>>
>> After patch, tun_alloc() called twice, one for Rx_q and other for Tx_q.
>> And tun_alloc does open() to "/dev/net/tun", I expect they get different
>> file descriptors.
>
> It is not called twice, it is only called once in the eth_dev_tap_create()
> routine and the fd is placed in the rxq/txq using the same fd. Then look in
> the rx/tx_setup_queue routines only update the fd and call tun_alloc if the
> fd is -1. Now looking at this code it seems a bit silly, but it was trying to
> deal with the setting up the new queue. It seems to be this logic not going
> to work with multiple queues in the same device and needs to be reworked.
I see, right, it is not called twice for first queue of device.
But will be called twice for multiple queues.
Also if application does multiple rx/tx_queue_setup() calls, again,
tun_alloc() will be called twice.
This means two tap interface will be created, one for rx and one for tx,
which is wrong I guess.
The tun_alloc() logic is correct in current code, just setting both
rx_queues and tx_queues in rx_queue_setup() breaks logic, so I propose
to update just that part.
What do you think about following update [1], it will fix current code,
also will keep correct tun_alloc() call logic. But this will be still
broken for application does multiple rx/tx_queue_setup() calls case.
[1]
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -428,8 +428,6 @@ tap_setup_queue(struct rte_eth_dev *dev,
}
}
}
- dev->data->rx_queues[qid] = rx;
- dev->data->tx_queues[qid] = tx;
rx->fd = fd;
tx->fd = fd;
@@ -438,6 +436,26 @@ tap_setup_queue(struct rte_eth_dev *dev,
}
static int
+rx_setup_queue(struct rte_eth_dev *dev,
+ struct pmd_internals *internals,
+ uint16_t qid)
+{
+ dev->data->rx_queues[qid] = &internals->rxq[qid];
+
+ return tap_setup_queue(dev, internals, qid);
+}
+
+static int
+tx_setup_queue(struct rte_eth_dev *dev,
+ struct pmd_internals *internals,
+ uint16_t qid)
+{
+ dev->data->tx_queues[qid] = &internals->txq[qid];
+
+ return tap_setup_queue(dev, internals, qid);
+}
+
+static int
tap_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t rx_queue_id,
uint16_t nb_rx_desc __rte_unused,
@@ -469,7 +487,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
return -ENOMEM;
}
- fd = tap_setup_queue(dev, internals, rx_queue_id);
+ fd = rx_setup_queue(dev, internals, rx_queue_id);
if (fd == -1)
return -1;
@@ -493,7 +511,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
if (tx_queue_id >= internals->nb_queues)
return -1;
- ret = tap_setup_queue(dev, internals, tx_queue_id);
+ ret = tx_setup_queue(dev, internals, tx_queue_id);
if (ret == -1)
return -1;
>
> I need to rework the code and do some cleanup. The current patch should work
> for a single queue per device.
>
> Thanks
>
>>
>> And if they have same FD, won't this cause same problem,
>> rx_queue_setup() will close the FD, if Tx_q has same FD it will have
>> invalid descriptor.
>>
>>>
What was the wrong with rx and tx having same fd?
As far as I can see, rte_eth_rx_queue_setup() won't close tx->fd, that
function will do nothing if rx or tx has valid fd.
>>>
>>> The rte_eth_rx_queue_setup() look at line 1146 if rxq has a value then
>>> release it, which happens on both Rx/Tx code
>>>
>>> rxq = dev->data->rx_queues;
>>> if (rxq[rx_queue_id]) {
>>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release,
>>> -ENOTSUP);
>>> (*dev->dev_ops->rx_queue_release)(rxq[rx_queue_id]);
>>> rxq[rx_queue_id] = NULL;
>>> }
>>
>> Got it thanks, I missed (relatively new) above code piece.
>>
>>>
>>> if (rx_conf == NULL)
>>> rx_conf = &dev_info.default_rxconf;
>>>
>>> ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
>>> socket_id, rx_conf, mp);
>>>
>
> Signed-off-by: Keith Wiles
<...>
>>>
>>> Regards,
>>> Keith
>
> Regards,
>
Re: [dpdk-dev] [PATCH v5] net/kni: add KNI PMD
On 1/30/2017 7:05 PM, Yong Wang wrote: >> -Original Message- >> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] >> Sent: Monday, January 30, 2017 8:58 AM >> To: Thomas Monjalon ; Yong Wang >> >> Cc: dev@dpdk.org; Ferruh Yigit >> Subject: [PATCH v5] net/kni: add KNI PMD >> >> Add KNI PMD which wraps librte_kni for ease of use. >> >> KNI PMD can be used as any regular PMD to send / receive packets to the >> Linux networking stack. >> >> Signed-off-by: Ferruh Yigit >> --- > > Looks good except a few typos in the documentation added. Thank you for the review, I will send a new version with them fixed. > > Reviewed-by: Yong Wang > >> <...>
[dpdk-dev] [PATCH v6] net/kni: add KNI PMD
Add KNI PMD which wraps librte_kni for ease of use. KNI PMD can be used as any regular PMD to send / receive packets to the Linux networking stack. Signed-off-by: Ferruh Yigit Reviewed-by: Yong Wang --- v6: * documentation typos fixed v5: * add kvargs "no_request_thread" to disable a specific pthread creation to handle control requests. * add documentation v4: * allow only single queue * use driver.name as name v3: * rebase on top of latest master v2: * updated driver name eth_kni -> net_kni --- MAINTAINERS | 5 + config/common_base | 1 + config/common_linuxapp | 1 + doc/guides/nics/features/kni.ini| 7 + doc/guides/nics/index.rst | 1 + doc/guides/nics/kni.rst | 197 drivers/net/Makefile| 1 + drivers/net/kni/Makefile| 64 drivers/net/kni/rte_eth_kni.c | 515 drivers/net/kni/rte_pmd_kni_version.map | 4 + mk/rte.app.mk | 10 +- 11 files changed, 801 insertions(+), 5 deletions(-) create mode 100644 doc/guides/nics/features/kni.ini create mode 100644 doc/guides/nics/kni.rst create mode 100644 drivers/net/kni/Makefile create mode 100644 drivers/net/kni/rte_eth_kni.c create mode 100644 drivers/net/kni/rte_pmd_kni_version.map diff --git a/MAINTAINERS b/MAINTAINERS index f071138..8eb83f5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -404,6 +404,11 @@ M: Keith Wiles F: drivers/net/tap/ F: doc/guides/nics/tap.rst +KNI PMD +M: Ferruh Yigit +F: drivers/net/kni/ +F: doc/guides/nics/kni.rst + Ring PMD M: Bruce Richardson F: drivers/net/ring/ diff --git a/config/common_base b/config/common_base index 61efb87..2e1bbd5 100644 --- a/config/common_base +++ b/config/common_base @@ -576,6 +576,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n # Compile librte_kni # CONFIG_RTE_LIBRTE_KNI=n +CONFIG_RTE_LIBRTE_PMD_KNI=n CONFIG_RTE_KNI_KMOD=n CONFIG_RTE_KNI_KMOD_ETHTOOL=n CONFIG_RTE_KNI_PREEMPT_DEFAULT=y diff --git a/config/common_linuxapp b/config/common_linuxapp index 00ebaac..d03a60a 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y CONFIG_RTE_EAL_VFIO=y CONFIG_RTE_KNI_KMOD=y CONFIG_RTE_LIBRTE_KNI=y +CONFIG_RTE_LIBRTE_PMD_KNI=y CONFIG_RTE_LIBRTE_VHOST=y CONFIG_RTE_LIBRTE_PMD_VHOST=y CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y diff --git a/doc/guides/nics/features/kni.ini b/doc/guides/nics/features/kni.ini new file mode 100644 index 000..6deb66a --- /dev/null +++ b/doc/guides/nics/features/kni.ini @@ -0,0 +1,7 @@ +; +; Supported features of the 'kni' network poll mode driver. +; +; Refer to default.ini for the full list of available PMD features. +; +[Features] +Usage doc= Y diff --git a/doc/guides/nics/index.rst b/doc/guides/nics/index.rst index 87f9334..5248625 100644 --- a/doc/guides/nics/index.rst +++ b/doc/guides/nics/index.rst @@ -46,6 +46,7 @@ Network Interface Controller Drivers i40e ixgbe intel_vf +kni mlx4 mlx5 nfp diff --git a/doc/guides/nics/kni.rst b/doc/guides/nics/kni.rst new file mode 100644 index 000..77542b5 --- /dev/null +++ b/doc/guides/nics/kni.rst @@ -0,0 +1,197 @@ +.. BSD LICENSE +Copyright(c) 2017 Intel Corporation. All rights reserved. +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +* Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. +* Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in +the documentation and/or other materials provided with the +distribution. +* Neither the name of Intel Corporation nor the names of its +contributors may be used to endorse or promote products derived +from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +KNI Poll Mode Driver +== + +KNI PMD
Re: [dpdk-dev] [PATCH 25/25] rte_eal_init: add info about rte_errno codes
2017-01-30 13:38, Aaron Conole: > Stephen Hemminger writes: > > Bruce Richardson wrote: > >> On Fri, Jan 27, 2017 at 08:33:46AM -0800, Stephen Hemminger wrote: > >> > Why use rte_errno? > >> > Most DPDK calls just return negative value on error which > >> > corresponds to error number. > >> > Are you trying to keep ABI compatibility? Doesn't make sense > >> > because before all these > >> > errors were panic's no working application is going to care. > >> > >> Either will work, but I actually prefer this way. I view using rte_errno > >> to be better as it can work in just about all cases, including with > >> functions which return pointers. This allows you to have a standard > >> method across all functions for returning error codes, and it only > >> requires a single sentinal value to indicate error, rather than using a > >> whole range of values. > > > > The problem is DPDK is getting more inconsistent on how this is done. > > As long as error returns are always same as kernel/glibc errno's it really > > doesn't > > matter much which way the value is returned from a technical point of view > > but the inconsistency is sure to be a usability problem and source of > > errors. > > I am using rte_errno here because I assumed it was the preferred > method. In fact, looking at some recently contributed modules (for > instance pdump), it seems that folks are using it. > > I'm not really sure the purpose of having rte_errno if it isn't used, so > it'd be helpful to know if there's some consensus on reflecting errors > via this variable, or on returning error codes. Whichever is the more > consistent with the way the DPDK project does things, I'm game :). I think we can use both return value and rte_errno. We could try to enforce rte_errno as mandatory everywhere. Adrien did the recent rte_flow API. Please Adrien, could you give your thought?
Re: [dpdk-dev] [PATCH v3] doc: add device specific API
2017-01-30 18:27, Ferruh Yigit: > Signed-off-by: Ferruh Yigit Applied, thanks
Re: [dpdk-dev] [PATCH v3] doc: add device specific API
2017-01-30 21:21, Thomas Monjalon: > 2017-01-30 18:27, Ferruh Yigit: > > Signed-off-by: Ferruh Yigit > > Applied, thanks Actually no, there are a lot of doxygen errors in ixgbe and i40e API. I cannot be merged as is.
Re: [dpdk-dev] [PATCH v3] doc: add device specific API
On 1/30/2017 8:25 PM, Thomas Monjalon wrote: > 2017-01-30 21:21, Thomas Monjalon: >> 2017-01-30 18:27, Ferruh Yigit: >>> Signed-off-by: Ferruh Yigit >> >> Applied, thanks > > Actually no, there are a lot of doxygen errors in ixgbe and i40e API. > I cannot be merged as is. I already fixed and merged ixgbe and i40e doxygen errors in next-net.
Re: [dpdk-dev] [PATCH v3] doc: add device specific API
2017-01-30 20:27, Ferruh Yigit: > On 1/30/2017 8:25 PM, Thomas Monjalon wrote: > > 2017-01-30 21:21, Thomas Monjalon: > >> 2017-01-30 18:27, Ferruh Yigit: > >>> Signed-off-by: Ferruh Yigit > >> > >> Applied, thanks > > > > Actually no, there are a lot of doxygen errors in ixgbe and i40e API. > > I cannot be merged as is. > > I already fixed and merged ixgbe and i40e doxygen errors in next-net. OK, good to know.
[dpdk-dev] [PATCH v2] net/tap: fix invalid queue file descriptor
From: Keith Wiles
Rx and Tx queues share the common tap file descriptor, but save this
value separately.
Setting up Rx/Tx queue sets up both queues, release_queue close the
tap file but update file descriptor only for that queue.
This makes other queue's file descriptor invalid.
As a workaround, prevent release_queue callback to be called by default.
This is done by separating Rx/Tx setup functions, so that each only
setup its own queue, this prevents rte_eth_rx/tx_queue_setup() calling
release_queue before setup_queue.
Signed-off-by: Keith Wiles
Signed-off-by: Ferruh Yigit
---
drivers/net/tap/rte_eth_tap.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c0afc2d..91f63f5 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -428,8 +428,6 @@ tap_setup_queue(struct rte_eth_dev *dev,
}
}
}
- dev->data->rx_queues[qid] = rx;
- dev->data->tx_queues[qid] = tx;
rx->fd = fd;
tx->fd = fd;
@@ -438,6 +436,26 @@ tap_setup_queue(struct rte_eth_dev *dev,
}
static int
+rx_setup_queue(struct rte_eth_dev *dev,
+ struct pmd_internals *internals,
+ uint16_t qid)
+{
+ dev->data->rx_queues[qid] = &internals->rxq[qid];
+
+ return tap_setup_queue(dev, internals, qid);
+}
+
+static int
+tx_setup_queue(struct rte_eth_dev *dev,
+ struct pmd_internals *internals,
+ uint16_t qid)
+{
+ dev->data->tx_queues[qid] = &internals->txq[qid];
+
+ return tap_setup_queue(dev, internals, qid);
+}
+
+static int
tap_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t rx_queue_id,
uint16_t nb_rx_desc __rte_unused,
@@ -469,7 +487,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
return -ENOMEM;
}
- fd = tap_setup_queue(dev, internals, rx_queue_id);
+ fd = rx_setup_queue(dev, internals, rx_queue_id);
if (fd == -1)
return -1;
@@ -493,7 +511,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
if (tx_queue_id >= internals->nb_queues)
return -1;
- ret = tap_setup_queue(dev, internals, tx_queue_id);
+ ret = tx_setup_queue(dev, internals, tx_queue_id);
if (ret == -1)
return -1;
--
2.9.3
Re: [dpdk-dev] [PATCH v2] net/tap: fix invalid queue file descriptor
> On Jan 30, 2017, at 2:54 PM, Yigit, Ferruh wrote:
>
> From: Keith Wiles
>
> Rx and Tx queues share the common tap file descriptor, but save this
> value separately.
>
> Setting up Rx/Tx queue sets up both queues, release_queue close the
> tap file but update file descriptor only for that queue.
>
> This makes other queue's file descriptor invalid.
>
> As a workaround, prevent release_queue callback to be called by default.
>
> This is done by separating Rx/Tx setup functions, so that each only
> setup its own queue, this prevents rte_eth_rx/tx_queue_setup() calling
> release_queue before setup_queue.
>
> Signed-off-by: Keith Wiles
> Signed-off-by: Ferruh Yigit
Acked-by: Keith Wiles
> ---
> drivers/net/tap/rte_eth_tap.c | 26 ++
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index c0afc2d..91f63f5 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -428,8 +428,6 @@ tap_setup_queue(struct rte_eth_dev *dev,
> }
> }
> }
> - dev->data->rx_queues[qid] = rx;
> - dev->data->tx_queues[qid] = tx;
>
> rx->fd = fd;
> tx->fd = fd;
> @@ -438,6 +436,26 @@ tap_setup_queue(struct rte_eth_dev *dev,
> }
>
> static int
> +rx_setup_queue(struct rte_eth_dev *dev,
> + struct pmd_internals *internals,
> + uint16_t qid)
> +{
> + dev->data->rx_queues[qid] = &internals->rxq[qid];
> +
> + return tap_setup_queue(dev, internals, qid);
> +}
> +
> +static int
> +tx_setup_queue(struct rte_eth_dev *dev,
> + struct pmd_internals *internals,
> + uint16_t qid)
> +{
> + dev->data->tx_queues[qid] = &internals->txq[qid];
> +
> + return tap_setup_queue(dev, internals, qid);
> +}
> +
> +static int
> tap_rx_queue_setup(struct rte_eth_dev *dev,
> uint16_t rx_queue_id,
> uint16_t nb_rx_desc __rte_unused,
> @@ -469,7 +487,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
> return -ENOMEM;
> }
>
> - fd = tap_setup_queue(dev, internals, rx_queue_id);
> + fd = rx_setup_queue(dev, internals, rx_queue_id);
> if (fd == -1)
> return -1;
>
> @@ -493,7 +511,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
> if (tx_queue_id >= internals->nb_queues)
> return -1;
>
> - ret = tap_setup_queue(dev, internals, tx_queue_id);
> + ret = tx_setup_queue(dev, internals, tx_queue_id);
> if (ret == -1)
> return -1;
>
> --
> 2.9.3
>
Regards,
Keith
[dpdk-dev] [PATCH v7] net/kni: add KNI PMD
Add KNI PMD which wraps librte_kni for ease of use. KNI PMD can be used as any regular PMD to send / receive packets to the Linux networking stack. Signed-off-by: Ferruh Yigit Reviewed-by: Yong Wang --- v7: * Add dependency to CONFIG_RTE_LIBRTE_KNI config v6: * documentation typos fixed v5: * add kvargs "no_request_thread" to disable a specific pthread creation to handle control requests. * add documentation v4: * allow only single queue * use driver.name as name v3: * rebase on top of latest master v2: * updated driver name eth_kni -> net_kni --- MAINTAINERS | 5 + config/common_base | 1 + config/common_linuxapp | 1 + doc/guides/nics/features/kni.ini| 7 + doc/guides/nics/index.rst | 1 + doc/guides/nics/kni.rst | 197 drivers/net/Makefile| 4 + drivers/net/kni/Makefile| 64 drivers/net/kni/rte_eth_kni.c | 515 drivers/net/kni/rte_pmd_kni_version.map | 4 + mk/rte.app.mk | 10 +- 11 files changed, 804 insertions(+), 5 deletions(-) create mode 100644 doc/guides/nics/features/kni.ini create mode 100644 doc/guides/nics/kni.rst create mode 100644 drivers/net/kni/Makefile create mode 100644 drivers/net/kni/rte_eth_kni.c create mode 100644 drivers/net/kni/rte_pmd_kni_version.map diff --git a/MAINTAINERS b/MAINTAINERS index f071138..8eb83f5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -404,6 +404,11 @@ M: Keith Wiles F: drivers/net/tap/ F: doc/guides/nics/tap.rst +KNI PMD +M: Ferruh Yigit +F: drivers/net/kni/ +F: doc/guides/nics/kni.rst + Ring PMD M: Bruce Richardson F: drivers/net/ring/ diff --git a/config/common_base b/config/common_base index 61efb87..2e1bbd5 100644 --- a/config/common_base +++ b/config/common_base @@ -576,6 +576,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n # Compile librte_kni # CONFIG_RTE_LIBRTE_KNI=n +CONFIG_RTE_LIBRTE_PMD_KNI=n CONFIG_RTE_KNI_KMOD=n CONFIG_RTE_KNI_KMOD_ETHTOOL=n CONFIG_RTE_KNI_PREEMPT_DEFAULT=y diff --git a/config/common_linuxapp b/config/common_linuxapp index 00ebaac..d03a60a 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y CONFIG_RTE_EAL_VFIO=y CONFIG_RTE_KNI_KMOD=y CONFIG_RTE_LIBRTE_KNI=y +CONFIG_RTE_LIBRTE_PMD_KNI=y CONFIG_RTE_LIBRTE_VHOST=y CONFIG_RTE_LIBRTE_PMD_VHOST=y CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y diff --git a/doc/guides/nics/features/kni.ini b/doc/guides/nics/features/kni.ini new file mode 100644 index 000..6deb66a --- /dev/null +++ b/doc/guides/nics/features/kni.ini @@ -0,0 +1,7 @@ +; +; Supported features of the 'kni' network poll mode driver. +; +; Refer to default.ini for the full list of available PMD features. +; +[Features] +Usage doc= Y diff --git a/doc/guides/nics/index.rst b/doc/guides/nics/index.rst index 87f9334..5248625 100644 --- a/doc/guides/nics/index.rst +++ b/doc/guides/nics/index.rst @@ -46,6 +46,7 @@ Network Interface Controller Drivers i40e ixgbe intel_vf +kni mlx4 mlx5 nfp diff --git a/doc/guides/nics/kni.rst b/doc/guides/nics/kni.rst new file mode 100644 index 000..77542b5 --- /dev/null +++ b/doc/guides/nics/kni.rst @@ -0,0 +1,197 @@ +.. BSD LICENSE +Copyright(c) 2017 Intel Corporation. All rights reserved. +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +* Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. +* Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in +the documentation and/or other materials provided with the +distribution. +* Neither the name of Intel Corporation nor the names of its +contributors may be used to endorse or promote products derived +from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +KN
Re: [dpdk-dev] [PATCH v2] net/tap: fix invalid queue file descriptor
On 1/30/2017 8:57 PM, Wiles, Keith wrote: > >> On Jan 30, 2017, at 2:54 PM, Yigit, Ferruh wrote: >> >> From: Keith Wiles >> >> Rx and Tx queues share the common tap file descriptor, but save this >> value separately. >> >> Setting up Rx/Tx queue sets up both queues, release_queue close the >> tap file but update file descriptor only for that queue. >> >> This makes other queue's file descriptor invalid. >> >> As a workaround, prevent release_queue callback to be called by default. >> >> This is done by separating Rx/Tx setup functions, so that each only >> setup its own queue, this prevents rte_eth_rx/tx_queue_setup() calling >> release_queue before setup_queue. >> >> Signed-off-by: Keith Wiles >> Signed-off-by: Ferruh Yigit Applied to dpdk-next-net/master, thanks. (Just a reminder, defect still exists when app call rte_eth_rx/tx_queue_setup() multiple times)
Re: [dpdk-dev] [PATCH v9 1/7] lib: add information metrics library
Some points addressed. Will cover other ones later.
On 30/01/2017 15:50, Thomas Monjalon wrote:
[..]
+CONFIG_RTE_LIBRTE_METRICS=y
I know the config file is not so well sorted.
However it would be a bit more logical below CONFIG_RTE_LIBRTE_JOBSTATS.
Done. Rebase merging doesn't help with sorting here.
+ [Device Metrics] (@ref rte_metrics.h),
No first letter uppercase in this list.
Fixed.
+ A library that allows information metrics to be added and update. It is
update -> updated
Fixed.
added and updated by who?
Elaborated.
+ intended to provide a reporting mechanism that is independent of the
+ ethdev library.
and independent of the cryptodev library?
Yes. The aim is to have no sub-dependencies.
My original plan was to introduce some form of parameter registration
scheme into xstats to replace the current hard-coded tables, since I
suspected libbitrate/liblatency would not be the last additions. I
decided to spin it out into a library in its own right, as it seemed
cleaner than shoving a load of non-driver stuff into xstats.
Does it apply to other types of devices (cryptodev/eventdev)?
I've not been following cryptodev/eventdev, but short answer yes.
This section is a comment. do not overwrite or remove it.
Also, make sure to start the actual text at the margin.
=
Your text should start below this line, and indented at the margin.
Fixed.
+ + librte_bitratestats.so.1
not part of this patch
Fixed. Artefact of sorting out a merge mess-up.
+DIRS-$(CONFIG_RTE_LIBRTE_METRICS) += librte_metrics
insert it below librte_jobstats is a better choice
Done.
+struct rte_metrics_meta_s {
+ char name[RTE_METRICS_MAX_NAME_LEN];
+ uint64_t value[RTE_MAX_ETHPORTS];
+ uint64_t nonport_value;
+ uint16_t idx_next_set;
+ uint16_t idx_next_stat;
+};
It would be a lot easier to read with comments near each field.
It would avoid to forget some fields like nonport_value in this struct.
You do not need to use a doxygen syntax in a .c file.
Noted. Even though Doxygen isn't required, I think it preferable to use
a consistent style in both .c and .h files.
+ * RTE Metrics module
RTE is not meaningful here.
Please prefer DPDK.
Fixed.
+ * Metric information is populated using a push model, where the
+ * information provider calls an update function on the relevant
+ * metrics. Currently only bulk querying of metrics is supported.
+ */
This description should explain who is a provider (drivers?) and who
is the reader (registered thread?).
Noted. Will elaborate.
What do you mean by "push model"? A callback is used?
Updating is done in response to producers having new information. In
contrast in a pull model an update would happen in response to a polling
by a consumer.
Originally (back in August I think) I used a pull model where producers
would register callbacks that were called in response to
rte_metrics_get() by a consumer, but that assumed that producers and
consumers were within the same process. Using shared memory and making
it ASLR-safe means not using pointers.
Aside: In this former pull model, port_id was passed verbatim to the
producers' callbacks to interpret in whatever way they saw fit, so there
was no inherant need to stop "magic" values outside the usual 0-255 used
for port ids. Hence where the next thing originally came from...
+/**
+ * Global (rather than port-specific) metric.
It does not say what kind of constant it is. A special metric id?
Yes. Elaborated.
+ *
+ * When used instead of port number by rte_metrics_update_metric()
+ * or rte_metrics_update_metric(), the global metrics, which are
+ * not associated with any specific port, are updated.
+ */
+#define RTE_METRICS_GLOBAL -1
I thought you agreed that "port" is not really a good wording.
Certainly within the constant name. Don't see what's wrong with
referring to it in description though.
+ * An array of this structure is returned by rte_metrics_get_names().
+ * The struct rte_eth_stats references these names via their array index.
rte_eth_stats?
Good question - was going to put it down to cut'n'paste while baseing
the descriptions on Olivier Matz's rewording, but that was for xstats..
+ * Initializes metric module. This function must be called from
+ * a primary process before metrics are used.
Why not integrating it in the global init?
Is there some performance drawbacks?
There shouldn't be any significant performance penalities, but I am not
particularly fond in principle of initalising component libraries
regardless of whether they are used. (Actually it was previously
initalised on first use, but that had a race condition.)
+ * Registering a metric is the way third-parties declare a parameter
third-party? You mean the provider?
Yes.
+ * - \b -EINVAL: Error, invalid parameters
+ * - \b -ENOMEM: Error, maxim
[dpdk-dev] [dpdk-pdump] Problem with dpdk-pdump
Hi all, I just started to use dpdk-pdump and got some problem with it. Appreciate your help! By following the document, I got testpmd running with pdump framework initialized and tried to start dpdk-pdump as a secondary process. However, I failed to start dpdk-pdump. My testpmd was running like: testpmd -w :00:05.0 -w :00:06.0 -n 2 -c 0xf --socket-mem 1024 0 --proc-type primary --syslog local4 Then I started dpdk-pdump like dpdk-pdump -w :00:05.0 -w :00:06.0 -- --pdump 'port=0,queue=*,rx-dev=/tmp/rx.pcap’ but got error messages as shown below. I double check the PCI device :00:05.0 and it was bounded to dpdk driver (I specified whitelist because I have more than two ports in my system and only two of them were given to dpdk) EAL: Detected 8 lcore(s) Mon Jan 30 17:23:35 2017EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable clock cycles ! Mon Jan 30 17:23:35 2017PMD: bnxt_rte_pmd_init() called for (null) Mon Jan 30 17:23:35 2017EAL: PCI device :00:05.0 on NUMA socket -1 Mon Jan 30 17:23:35 2017EAL: probe driver: 1137:43 rte_enic_pmd Mon Jan 30 17:23:35 2017PMD: rte_enic_pmd: vNIC BAR0 res hdr not mem-mapped Mon Jan 30 17:23:35 2017PMD: rte_enic_pmd: vNIC registration failed, aborting Mon Jan 30 17:23:35 2017EAL: Error - exiting with code: 1 Cause: Mon Jan 30 17:23:35 2017Requested device :00:05.0 cannot be used
[dpdk-dev] [dpdk-announce] release candidate 17.02-rc2
A new DPDK release candidate is ready for testing: http://dpdk.org/browse/dpdk/tag/?id=v17.02-rc2 What's new in RC2? - a crypto scheduler driver - a crypto performance test application - a lot of fixes The release notes have been updated: http://dpdk.org/doc/guides/rel_notes/release_17_02.html If your work is not in this release notes, it means either your patches have not been accepted in 17.02, or you forgot to update the release notes as part of your patches. The next release candidate is expected to include only some bug fixes, packaging or documentation update. It would be great to be able to release before the middle of February. In the meantime, let's wish a happy new year to people in China :)
Re: [dpdk-dev] [PATCH v3 01/10] doc: add NXP dpaa2_sec in cryptodev
On 1/24/2017 9:03 PM, De Lara Guarch, Pablo wrote: Hi, -Original Message- From: akhil.go...@nxp.com [mailto:akhil.go...@nxp.com] Sent: Friday, January 20, 2017 2:05 PM To: dev@dpdk.org Cc: thomas.monja...@6wind.com; Doherty, Declan; De Lara Guarch, Pablo; Mcnamara, John; nhor...@tuxdriver.com; Akhil Goyal Subject: [PATCH v3 01/10] doc: add NXP dpaa2_sec in cryptodev From: Akhil Goyal Signed-off-by: Akhil Goyal Reviewed-by: Hemant Agrawal diff --git a/doc/guides/cryptodevs/index.rst b/doc/guides/cryptodevs/index.rst index 06c3f6e..941b865 100644 --- a/doc/guides/cryptodevs/index.rst +++ b/doc/guides/cryptodevs/index.rst @@ -38,6 +38,7 @@ Crypto Device Drivers overview aesni_mb aesni_gcm +dpaa2_sec armv8 kasumi openssl -- 2.9.3 This list is in alphabetical order. Also, I would add this patch at the end of the patchset, and not the first. Ok I would correct this.
