[dpdk-dev] [PATCH] i40e: fix shared code compile warning
Hi Thomas, -Original Message- From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] Sent: Tuesday, June 24, 2014 4:47 PM To: Chen, Jing D Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning Hi, 2014-06-24 13:22, Chen Jing D: > Fix a compile warning in shared code on 32-bits RHEL6.3/6.5. > > Signed-off-by: Chen Jing D(Mark) In such case you should show the error message in the log. lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c:917: error: integer constant is too large for ?long? type > --- a/lib/librte_pmd_i40e/Makefile > +++ b/lib/librte_pmd_i40e/Makefile > @@ -52,6 +52,7 @@ CFLAGS_SHARED_DRIVERS += > -Wno-missing-field-initializers CFLAGS_SHARED_DRIVERS += > -Wno-pointer-to-int-cast CFLAGS_SHARED_DRIVERS += > -Wno-format-nonliteral CFLAGS_SHARED_DRIVERS += -Wno-format-security > +CFLAGS_i40e_lan_hmc.o += -Wno-error I know we shouldn't modify base drivers. But this one seems to be an important error. In such case, we already modified base driver. Recently: http://dpdk.org/ml/archives/dev/2014-June/003498.html I think it's different. The logic is right after adding the patch. Below is my finding. In this case, it met the error when compile on 32-bits OS. The message is : /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c: In function ?i40e_write_qword?: /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c:917: error: integer constant is too large for ?long? type /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c: In function ?i40e_read_qword?: /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/librte_pmd_i40e/i40e/i40e_lan_hmc.c:1097: error: integer constant is too large for ?long? type I found the code that cause errors. 'mask' is 'uint64_t' type and is assigned to value 0X___. Compiler assumes the constant is 'int' type by default. If changed it to ox___ULL, the warning will be gone. if (ce_info->width < 64) mask = ((u64)1 << ce_info->width) - 1; else mask = 0x; besides that, I dis-assembler the code with the patch and get below segment. It seems right. if (ce_info->width < 64) 1946: 8b 45 0cmov0xc(%ebp),%eax 1949: 0f b7 40 04 movzwl 0x4(%eax),%eax 194d: 66 83 f8 3f cmp$0x3f,%ax 1951: 77 30 ja 1983 mask = ((u64)1 << ce_info->width) - 1; 1953: 8b 45 0cmov0xc(%ebp),%eax 1956: 0f b7 40 04 movzwl 0x4(%eax),%eax 195a: 0f b7 c8movzwl %ax,%ecx 195d: b8 01 00 00 00 mov$0x1,%eax 1962: ba 00 00 00 00 mov$0x0,%edx 1967: 0f a5 c2shld %cl,%eax,%edx 196a: d3 e0 shl%cl,%eax 196c: f6 c1 20test $0x20,%cl 196f: 74 04 je 1975 1971: 89 c2 mov%eax,%edx 1973: 31 c0 xor%eax,%eax 1975: 83 c0 ffadd$0x,%eax 1978: 83 d2 ffadc$0x,%edx 197b: 89 45 e0mov%eax,-0x20(%ebp) 197e: 89 55 e4mov%edx,-0x1c(%ebp) 1981: eb 0e jmp1991 else mask = 0x; 1983: c7 45 e0 ff ff ff ffmovl $0x,-0x20(%ebp) 198a: c7 45 e4 ff ff ff ffmovl $0x,-0x1c(%ebp) -- Thomas
[dpdk-dev] [PATCH] i40e: fix shared code compile warning
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, June 24, 2014 6:06 PM > To: Chen, Jing D > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] i40e: fix shared code compile warning > > 2014-06-24 09:47, Chen, Jing D: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-06-24 13:22, Chen Jing D: > > > > +CFLAGS_i40e_lan_hmc.o += -Wno-error > > > > > > I know we shouldn't modify base drivers. But this one seems to be an > > > important error. In such case, we already modified base driver. Recently: > > > http://dpdk.org/ml/archives/dev/2014-June/003498.html > > > > I think it's different. The logic is right after adding the patch. > > Below is my finding. > > > In this case, it met the error when compile on 32-bits OS. The message is : > > > > > /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/libr > te_pm > > d_i40e > > /i40e/i40e_lan_hmc.c: In function ?i40e_write_qword?: > > > /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/libr > te_pm > > d_i40 > > e/i40e/i40e_lan_hmc.c:917: error: integer constant is too large for ?long? > > type > > > /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/libr > te_pm > > d_i40 > > e/i40e/i40e_lan_hmc.c: In function ?i40e_read_qword?: > > > /jenkins/workspace/DPDK_AUTO_IDT_VM_RHEL65_32_BUILD/DPDK/lib/libr > te_pm > > d_i40 > > e/i40e/i40e_lan_hmc.c:1097: error: integer constant is too large for ?long? > > type > > I found the code that cause errors. 'mask' is 'uint64_t' type and is > > assigned to value 0X___. Compiler assumes the constant > > is 'int' type by default. If changed it to ox___ULL, > > the warning will be gone. > > > if (ce_info->width < 64) > > mask = ((u64)1 << ce_info->width) - 1; > > else > > mask = 0x; > > > > besides that, I dis-assembler the code with the patch and get below > segment. > > It seems right. > > > if (ce_info->width < 64) > > 1946: 8b 45 0cmov0xc(%ebp),%eax > > 1949: 0f b7 40 04 movzwl 0x4(%eax),%eax > > 194d: 66 83 f8 3f cmp$0x3f,%ax > > 1951: 77 30 ja 1983 > > mask = ((u64)1 << ce_info->width) - 1; > > 1953: 8b 45 0cmov0xc(%ebp),%eax > > 1956: 0f b7 40 04 movzwl 0x4(%eax),%eax > > 195a: 0f b7 c8movzwl %ax,%ecx > > 195d: b8 01 00 00 00 mov$0x1,%eax > > 1962: ba 00 00 00 00 mov$0x0,%edx > > 1967: 0f a5 c2shld %cl,%eax,%edx > > 196a: d3 e0 shl%cl,%eax > > 196c: f6 c1 20test $0x20,%cl > > 196f: 74 04 je 1975 > > 1971: 89 c2 mov%eax,%edx > > 1973: 31 c0 xor%eax,%eax > > 1975: 83 c0 ffadd$0x,%eax > > 1978: 83 d2 ffadc$0x,%edx > > 197b: 89 45 e0mov%eax,-0x20(%ebp) > > 197e: 89 55 e4mov%edx,-0x1c(%ebp) > > 1981: eb 0e jmp1991 > > else > > mask = 0x; > > 1983: c7 45 e0 ff ff ff ffmovl $0x,-0x20(%ebp) > > 198a: c7 45 e4 ff ff ff ffmovl $0x,-0x1c(%ebp) > > Maybe I don't understand. You are saying you can fix the compiler warning > by adding ULL to the constant. This is a simple patch and is a lot nicer than > CFLAGS_i40e_lan_hmc.o += -Wno-error > Even if the asm code seems right, it would be more secure to remove this > warning. > > PS: please try to configure your mailer to add citation marks. > > -- > Thomas [Chen, Jing D] Generally speaking, I can fix the compile error by 2 ways. 1. change shared code with "ULL" suffix, which means that we had to maintain it. In any time shared code updates, we needs to find out what change we've made in history and compare with new shared code. I don?t know who should be responsible for that and how we record the changes. If you have good idea, please share with me. 2. my committed patch to ignore the warning on that file and continue the compile . By doing so, the logic is right and didn't change code's behavior. We also needn't maintain it. Even in bad case we find bugs hidden behind the '--CFLAGS_i40e_lan_hmc.0' warning in future, we can fix it after bug occurred. That's simple.
[dpdk-dev] [PATCH 1/3] i40e: explicit shared code naming as base driver
Hi Thomas,
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, June 25, 2014 8:22 PM
> To: dev at dpdk.org
> Cc: Chen, Jing D; Zhang, Helin; Ananyev, Konstantin; De Lara Guarch, Pablo
> Subject: [PATCH 1/3] i40e: explicit shared code naming as base driver
>
> The PMD is built on top of the base driver which is provided by Intel and
> shouldn't be modified to allow easy batch upgrade from Intel.
>
> The base driver is a "shared code" between many projects. But in DPDK, the
> "base driver" naming makes more sense.
>
> Signed-off-by: Thomas Monjalon
> ---
> lib/librte_pmd_i40e/Makefile | 33 -
> lib/librte_pmd_i40e/i40e_ethdev.c | 12 ++--
> 2 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/lib/librte_pmd_i40e/Makefile b/lib/librte_pmd_i40e/Makefile
> index 09f2087..77d08fb 100644
> --- a/lib/librte_pmd_i40e/Makefile
> +++ b/lib/librte_pmd_i40e/Makefile
> @@ -39,26 +39,25 @@ LIB = librte_pmd_i40e.a CFLAGS += -O3 CFLAGS
> += $(WERROR_FLAGS)
>
> -ifeq ($(CC), icc)
> -CFLAGS_SHARED_DRIVERS = -wd593
> -else
> -CFLAGS_SHARED_DRIVERS = -Wno-unused-but-set-variable
> -CFLAGS_SHARED_DRIVERS += -Wno-sign-compare
> -CFLAGS_SHARED_DRIVERS += -Wno-unused-value
> -CFLAGS_SHARED_DRIVERS += -Wno-unused-parameter
> -CFLAGS_SHARED_DRIVERS += -Wno-strict-aliasing
> -CFLAGS_SHARED_DRIVERS += -Wno-format -CFLAGS_SHARED_DRIVERS +=
> -Wno-missing-field-initializers -CFLAGS_SHARED_DRIVERS +=
> -Wno-pointer-to-int-cast -CFLAGS_SHARED_DRIVERS +=
> -Wno-format-nonliteral -CFLAGS_SHARED_DRIVERS +=
> -Wno-format-security -endif
> -
> #
> # Add extra flags for ND source files to disable warnings #
> -SHARED_DRIVERS_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard
> $(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
> -$(foreach obj, $(SHARED_DRIVERS_OBJS), $(eval
> CFLAGS_$(obj)+=$(CFLAGS_SHARED_DRIVERS)))
> +ifeq ($(CC), icc)
> +CFLAGS_BASE_DRIVER = -wd593
> +else
> +CFLAGS_BASE_DRIVER = -Wno-unused-but-set-variable
> CFLAGS_BASE_DRIVER
> ++= -Wno-sign-compare CFLAGS_BASE_DRIVER += -Wno-unused-value
> +CFLAGS_BASE_DRIVER += -Wno-unused-parameter CFLAGS_BASE_DRIVER
> +=
> +-Wno-strict-aliasing CFLAGS_BASE_DRIVER += -Wno-format
> +CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
> +CFLAGS_BASE_DRIVER += -Wno-pointer-to-int-cast CFLAGS_BASE_DRIVER
> +=
> +-Wno-format-nonliteral CFLAGS_BASE_DRIVER += -Wno-format-security
> endif
> +OBJS_BASE_DRIVER=$(patsubst %.c,%.o,$(notdir $(wildcard
> +$(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
> +$(foreach obj, $(OBJS_BASE_DRIVER), $(eval
> +CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER)))
>
> VPATH += $(RTE_SDK)/lib/librte_pmd_i40e/i40e
>
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 3311d73..6bc3998 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -387,10 +387,10 @@ eth_i40e_dev_init(__rte_unused struct eth_driver
> *eth_drv,
> return ret;
> }
>
> - /* Initialize the shared code */
> + /* Initialize the shared code (base driver) */
> ret = i40e_init_shared_code(hw);
> if (ret) {
> - PMD_INIT_LOG(ERR, "Failed to init shared code: %d", ret);
> + PMD_INIT_LOG(ERR, "Failed to init shared code (base driver):
> %d",
> +ret);
> return ret;
> }
>
> @@ -1497,7 +1497,7 @@ i40e_dev_rss_reta_query(struct rte_eth_dev
> *dev, }
>
> /**
> - * i40e_allocate_dma_mem_d - specific memory alloc for shared code
> + * i40e_allocate_dma_mem_d - specific memory alloc for shared code
> + (base driver)
> * @hw: pointer to the HW structure
> * @mem: pointer to mem struct to fill out
> * @size: size of memory requested
> @@ -1531,7 +1531,7 @@
> i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw, }
>
> /**
> - * i40e_free_dma_mem_d - specific memory free for shared code
> + * i40e_free_dma_mem_d - specific memory free for shared code (base
> + driver)
> * @hw: pointer to the HW structure
> * @mem: ptr to mem struct to free
> **/
> @@ -1549,7 +1549,7 @@ i40e_free_dma_mem_d(__attribute__((unused))
> struct i40e_hw *hw, }
>
> /**
> - * i40e_allocate_virt_mem_d - specific memory alloc for shared code
> + * i40e_allocate_virt_mem_d - specific memory alloc for shared code
> + (base driver)
> * @hw: pointer to the HW structure
> * @mem: pointer to mem struct to fill out
> * @size: size of memory requested
> @@ -1572,7 +1572,7 @@
> i40e_allocate_virt_mem_d(__attribute__((unused)) struct i4
[dpdk-dev] [PATCH 1/3] i40e: explicit shared code naming as base driver
My bad. Please ignore " BTW, won't this patch overwrite previous one that fix
GCC 32bits warning?"
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Chen, Jing D
> Sent: Wednesday, June 25, 2014 10:44 PM
> To: Thomas Monjalon; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/3] i40e: explicit shared code naming as
> base driver
>
> Hi Thomas,
>
> > -Original Message-
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > Sent: Wednesday, June 25, 2014 8:22 PM
> > To: dev at dpdk.org
> > Cc: Chen, Jing D; Zhang, Helin; Ananyev, Konstantin; De Lara Guarch,
> > Pablo
> > Subject: [PATCH 1/3] i40e: explicit shared code naming as base driver
> >
> > The PMD is built on top of the base driver which is provided by Intel
> > and shouldn't be modified to allow easy batch upgrade from Intel.
> >
> > The base driver is a "shared code" between many projects. But in DPDK,
> > the "base driver" naming makes more sense.
> >
> > Signed-off-by: Thomas Monjalon
> > ---
> > lib/librte_pmd_i40e/Makefile | 33
> -
> > lib/librte_pmd_i40e/i40e_ethdev.c | 12 ++--
> > 2 files changed, 22 insertions(+), 23 deletions(-)
> >
> > diff --git a/lib/librte_pmd_i40e/Makefile
> > b/lib/librte_pmd_i40e/Makefile index 09f2087..77d08fb 100644
> > --- a/lib/librte_pmd_i40e/Makefile
> > +++ b/lib/librte_pmd_i40e/Makefile
> > @@ -39,26 +39,25 @@ LIB = librte_pmd_i40e.a CFLAGS += -O3
> CFLAGS
> > += $(WERROR_FLAGS)
> >
> > -ifeq ($(CC), icc)
> > -CFLAGS_SHARED_DRIVERS = -wd593
> > -else
> > -CFLAGS_SHARED_DRIVERS = -Wno-unused-but-set-variable
> > -CFLAGS_SHARED_DRIVERS += -Wno-sign-compare
> -CFLAGS_SHARED_DRIVERS +=
> > -Wno-unused-value -CFLAGS_SHARED_DRIVERS +=
> -Wno-unused-parameter
> > -CFLAGS_SHARED_DRIVERS += -Wno-strict-aliasing
> -CFLAGS_SHARED_DRIVERS
> > += -Wno-format -CFLAGS_SHARED_DRIVERS +=
> > -Wno-missing-field-initializers -CFLAGS_SHARED_DRIVERS +=
> > -Wno-pointer-to-int-cast -CFLAGS_SHARED_DRIVERS +=
> > -Wno-format-nonliteral -CFLAGS_SHARED_DRIVERS +=
> -Wno-format-security
> > -endif
> > -
> > #
> > # Add extra flags for ND source files to disable warnings #
> > -SHARED_DRIVERS_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard
> > $(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
> > -$(foreach obj, $(SHARED_DRIVERS_OBJS), $(eval
> > CFLAGS_$(obj)+=$(CFLAGS_SHARED_DRIVERS)))
> > +ifeq ($(CC), icc)
> > +CFLAGS_BASE_DRIVER = -wd593
> > +else
> > +CFLAGS_BASE_DRIVER = -Wno-unused-but-set-variable
> > CFLAGS_BASE_DRIVER
> > ++= -Wno-sign-compare CFLAGS_BASE_DRIVER += -Wno-unused-value
> > +CFLAGS_BASE_DRIVER += -Wno-unused-parameter
> CFLAGS_BASE_DRIVER =
> > +-Wno-strict-aliasing CFLAGS_BASE_DRIVER += -Wno-format
> > +CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
> > +CFLAGS_BASE_DRIVER += -Wno-pointer-to-int-cast
> CFLAGS_BASE_DRIVER =
> > +-Wno-format-nonliteral CFLAGS_BASE_DRIVER += -Wno-format-security
> > endif
> > +OBJS_BASE_DRIVER=$(patsubst %.c,%.o,$(notdir $(wildcard
> > +$(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
> > +$(foreach obj, $(OBJS_BASE_DRIVER), $(eval
> > +CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER)))
> >
> > VPATH += $(RTE_SDK)/lib/librte_pmd_i40e/i40e
> >
> > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> > b/lib/librte_pmd_i40e/i40e_ethdev.c
> > index 3311d73..6bc3998 100644
> > --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> > @@ -387,10 +387,10 @@ eth_i40e_dev_init(__rte_unused struct
> eth_driver
> > *eth_drv,
> > return ret;
> > }
> >
> > - /* Initialize the shared code */
> > + /* Initialize the shared code (base driver) */
> > ret = i40e_init_shared_code(hw);
> > if (ret) {
> > - PMD_INIT_LOG(ERR, "Failed to init shared code: %d", ret);
> > + PMD_INIT_LOG(ERR, "Failed to init shared code (base driver):
> > %d",
> > +ret);
> > return ret;
> > }
> >
> > @@ -1497,7 +1497,7 @@ i40e_dev_rss_reta_query(struct rte_eth_dev
> *dev,
> > }
> >
> > /**
> > - * i40e_allocate_dma_mem_d - specific memory alloc for shared code
> > + * i40e_allocate_dma_mem_d - specific memory alloc for shared code
> > + (base driver)
> > * @hw: pointer to the HW structure
> > * @mem: pointer to mem struct to fill out
> > * @size: s
[dpdk-dev] [PATCH v2 0/6] i40e VMDQ support
Hi, Any comments on this patch? > -Original Message- > From: Chen, Jing D > Sent: Thursday, October 16, 2014 6:07 PM > To: dev at dpdk.org > Cc: Ananyev, Konstantin; thomas.monjalon at 6wind.com; Chen, Jing D > Subject: [PATCH v2 0/6] i40e VMDQ support > > From: "Chen Jing D(Mark)" > > v2: > - Fix a few typos. > - Add comments for RX mq mode flags. > - Remove '\n' from some log messages. > - Remove 'Acked-by' in commit log. > > v1: > Define extra VMDQ arguments to expand VMDQ configuration. This also > includes change in igb and ixgbe PMD driver. In the meanwhile, fix 2 > defects in rte_ether library. > > Add full VMDQ support in i40e PMD driver. renamed some functions, setup > VMDQ VSI after it's enabled in application. It also make some improvement > on macaddr add/delete to support setting multiple macaddr for single or > multiple pools. > > Finally, change i40e rx/tx_queue_setup and dev_start/stop functions to > configure/switch queues belonging to VMDQ pools. > > > Chen Jing D(Mark) (6): > ether: enhancement for VMDQ support > igb: change for VMDQ arguments expansion > ixgbe: change for VMDQ arguments expansion > i40e: add VMDQ support > i40e: macaddr add/del enhancement > i40e: Add full VMDQ pools support > > config/common_linuxapp |1 + > lib/librte_ether/rte_ethdev.c | 12 +- > lib/librte_ether/rte_ethdev.h | 43 +++- > lib/librte_pmd_e1000/igb_ethdev.c |3 + > lib/librte_pmd_i40e/i40e_ethdev.c | 499 > ++- > lib/librte_pmd_i40e/i40e_ethdev.h | 21 ++- > lib/librte_pmd_i40e/i40e_rxtx.c | 125 +++-- > lib/librte_pmd_ixgbe/ixgbe_ethdev.c |1 + > 8 files changed, 536 insertions(+), 169 deletions(-) > > -- > 1.7.7.6
[dpdk-dev] [PATCH v2 4/6] i40e: add VMDQ support
Hi, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, November 04, 2014 2:34 AM > To: Chen, Jing D > Cc: dev at dpdk.org; Ananyev, Konstantin > Subject: Re: [PATCH v2 4/6] i40e: add VMDQ support > > Hi Jing, > > 2014-10-16 18:07, Chen Jing D: > > --- a/config/common_linuxapp > > +++ b/config/common_linuxapp > > @@ -208,6 +208,7 @@ > CONFIG_RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC=y > > CONFIG_RTE_LIBRTE_I40E_ALLOW_UNSUPPORTED_SFP=n > > CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC=n > > CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF=4 > > +CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=4 > > It seems you missed Pablo's comment. > Should you add this option in BSD configuration? Sorry, missed Pablo's email. Will add it for BSD. > > -- > Thomas
[dpdk-dev] [PATCH v2 2/6] igb: change for VMDQ arguments expansion
Hi, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, November 04, 2014 2:37 AM > To: Chen, Jing D > Cc: dev at dpdk.org; Ananyev, Konstantin > Subject: Re: [PATCH v2 2/6] igb: change for VMDQ arguments expansion > > 2014-10-16 18:07, Chen Jing D: > > --- a/lib/librte_pmd_e1000/igb_ethdev.c > > +++ b/lib/librte_pmd_e1000/igb_ethdev.c > > @@ -1286,18 +1286,21 @@ eth_igb_infos_get(struct rte_eth_dev *dev, > > dev_info->max_rx_queues = 16; > > dev_info->max_tx_queues = 16; > > dev_info->max_vmdq_pools = ETH_8_POOLS; > > + dev_info->vmdq_queue_num = 16; > > break; > > > > case e1000_82580: > > dev_info->max_rx_queues = 8; > > dev_info->max_tx_queues = 8; > > dev_info->max_vmdq_pools = ETH_8_POOLS; > > + dev_info->vmdq_queue_num = 8; > > break; > > > > case e1000_i350: > > dev_info->max_rx_queues = 8; > > dev_info->max_tx_queues = 8; > > dev_info->max_vmdq_pools = ETH_8_POOLS; > > + dev_info->vmdq_queue_num = 8; > > break; > > Why not simply set it only once? > dev_info->vmdq_queue_num = dev_info->max_rx_queues; There are some other NIC types in this 'switch'. Vmdq_queue_num is set in case max_vmdq_pools is not 0. > > -- > Thomas
[dpdk-dev] [PATCH v2 1/6] ether: enhancement for VMDQ support
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, November 04, 2014 6:17 AM > To: Chen, Jing D > Cc: dev at dpdk.org; Ananyev, Konstantin > Subject: Re: [PATCH v2 1/6] ether: enhancement for VMDQ support > > 2014-10-16 18:07, Chen Jing D: > > /** > > + * Simple flags to indicate RX mq mode, which can be used independently > or combined > > + * in enum rte_eth_rx_mq_mode definition. > > + */ > > +#define ETH_MQ_RX_RSS_FLAG 0x1 > > +#define ETH_MQ_RX_DCB_FLAG 0x2 > > +#define ETH_MQ_RX_VMDQ_FLAG 0x4 > > The comment would be more useful by explaining that these flags are used > for rte_eth_conf.rxmode.mq_mode. Yes, that's more straightforward. > > > + /**< None of DCB,RSS or VMDQ mode */ > > + ETH_MQ_RX_NONE = 0, > > + > > + /**< For RX side, only RSS is on */ > > + ETH_MQ_RX_RSS = ETH_MQ_RX_RSS_FLAG, > > + /**< For RX side,only DCB is on. */ > > + ETH_MQ_RX_DCB = ETH_MQ_RX_DCB_FLAG, > > + /**< Both DCB and RSS enable */ > > + ETH_MQ_RX_DCB_RSS = ETH_MQ_RX_RSS_FLAG | > ETH_MQ_RX_DCB_FLAG, > > + > > + /**< Only VMDQ, no RSS nor DCB */ > > + ETH_MQ_RX_VMDQ_ONLY = ETH_MQ_RX_VMDQ_FLAG, > > + /**< RSS mode with VMDQ */ > > + ETH_MQ_RX_VMDQ_RSS = ETH_MQ_RX_RSS_FLAG | > ETH_MQ_RX_VMDQ_FLAG, > > + /**< Use VMDQ+DCB to route traffic to queues */ > > + ETH_MQ_RX_VMDQ_DCB = ETH_MQ_RX_VMDQ_FLAG | > ETH_MQ_RX_DCB_FLAG, > > + /**< Enable both VMDQ and DCB in VMDq */ > > + ETH_MQ_RX_VMDQ_DCB_RSS = ETH_MQ_RX_RSS_FLAG | > ETH_MQ_RX_DCB_FLAG | > > +ETH_MQ_RX_VMDQ_FLAG, > > Doxygen comments placed before should start with /** not /**<. My mistake. Thanks for pointing it out. > > > + /** Specify the queue range belongs to VMDQ pools if VMDQ > applicable. */ > > + uint16_t vmdq_queue_base; > > + uint16_t vmdq_queue_num; > > Please explain what mean the values in vmdq_queue_base and > vmdq_queue_num. I thinks the name is self- explanatory, I also add some comments for them. As previous max_rx/tx_queues indicates how many queues available, these 2 variables defines the queue ranges for VM usage. What kind of explanations you needs me to add? > > Thanks > -- > Thomas
[dpdk-dev] [PATCH] i40e: fix of PF interrupt handling
> -Original Message- > From: Zhang, Helin > Sent: Tuesday, November 04, 2014 4:08 PM > To: dev at dpdk.org > Cc: Cao, Waterman; Cao, Min; Xu, HuilongX; Chen, Jing D; Zhang, Helin > Subject: [PATCH] i40e: fix of PF interrupt handling > > 'PFINT_ICR0_ENA' shouldn't be cleared in user space ISR, > otherwise adminq interrupts might be missed during > co-working with VF initialization. > > Signed-off-by: Helin Zhang > --- > lib/librte_pmd_i40e/i40e_ethdev.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c > b/lib/librte_pmd_i40e/i40e_ethdev.c > index 661d146..ea10c26 100644 > --- a/lib/librte_pmd_i40e/i40e_ethdev.c > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c > @@ -3574,7 +3574,6 @@ i40e_dev_interrupt_delayed_handler(void *param) > i40e_dev_link_update(dev, 0); > _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC); > > - I40E_WRITE_REG(hw, I40E_PFINT_ICR0_ENA, > I40E_PFINT_ICR0_ENA_MASK); > i40e_pf_enable_irq0(hw); > rte_intr_enable(&(dev->pci_dev->intr_handle)); > } > @@ -3601,7 +3600,6 @@ i40e_dev_interrupt_handler(__rte_unused struct > rte_intr_handle *handle, > > /* Disable interrupt */ > i40e_pf_disable_irq0(hw); > - I40E_WRITE_REG(hw, I40E_PFINT_ICR0_ENA, 0); > > /* read out interrupt causes */ > icr0 = I40E_READ_REG(hw, I40E_PFINT_ICR0); > @@ -3663,7 +3661,6 @@ i40e_dev_interrupt_handler(__rte_unused struct > rte_intr_handle *handle, > > done: > /* Enable interrupt */ > - I40E_WRITE_REG(hw, I40E_PFINT_ICR0_ENA, > I40E_PFINT_ICR0_ENA_MASK); > i40e_pf_enable_irq0(hw); > rte_intr_enable(&(dev->pci_dev->intr_handle)); > } > -- > 1.8.1.4 Acked-by : Jing Chen
[dpdk-dev] [PATCH v2 1/6] ether: enhancement for VMDQ support
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, November 04, 2014 4:54 PM > To: Chen, Jing D > Cc: dev at dpdk.org; Ananyev, Konstantin > Subject: Re: [PATCH v2 1/6] ether: enhancement for VMDQ support > > 2014-11-04 05:50, Chen, Jing D: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-10-16 18:07, Chen Jing D: > > > > + /** Specify the queue range belongs to VMDQ pools if VMDQ > > > applicable. */ > > > > + uint16_t vmdq_queue_base; > > > > + uint16_t vmdq_queue_num; > > > > > > Please explain what mean the values in vmdq_queue_base and > > > vmdq_queue_num. > > > > I thinks the name is self- explanatory, I also add some comments for them. > > As previous max_rx/tx_queues indicates how many queues available, > these > > 2 variables defines the queue ranges for VM usage. > > I understand clearly now. > > > What kind of explanations you needs me to add? > > You cannot put a doxygen comment which apply to 2 fields. > Try do describe precisely the meaning of each field. > Example: /**< first queue ID in the range for VMDQ pool */ > and /**< size of the queue range for VMDQ pool */ Thanks! Got you. > > -- > Thomas
[dpdk-dev] [PATCH v2 0/2] examples/vmdq: support new VMDQ API
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie > Sent: Monday, November 10, 2014 8:30 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v2 0/2] examples/vmdq: support new VMDQ > API > > This patch supports new VMDQ API in vmdq example. > > v2 changes: > * code rebase > * allow app to specify num_pools different with max_nb_pools > * fix serious cs issues > > > Huawei Xie (2): > support new VMDQ API in vmdq example > fix cs issues in vmdq example > > examples/vmdq/main.c | 233 ++ > - > 1 file changed, 139 insertions(+), 94 deletions(-) > > -- > 1.8.1.4 Acked-by : Jing Chen
[dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie > Sent: Monday, November 10, 2014 10:46 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix > > This patchset fixes "set vlan filter" issue. > > v2 changes: > * add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA array > operation. > > Huawei Xie (2): > vlan id set fix > add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation > > lib/librte_pmd_i40e/i40e_ethdev.c | 20 ++-- > lib/librte_pmd_i40e/i40e_ethdev.h | 9 + > 2 files changed, 19 insertions(+), 10 deletions(-) > > -- > 1.8.1.4 Acked-by : Jing Chen
[dpdk-dev] [PATCH] i40e: Fix a vlan bug
Yes, the same to his. As he is in vacation, I'd like to send out patch for him.
> -Original Message-
> From: Qiu, Michael
> Sent: Thursday, December 04, 2014 6:19 PM
> To: Chen, Jing D; dev at dpdk.org
> Cc: Xie, Huawei
> Subject: Re: [dpdk-dev] [PATCH] i40e: Fix a vlan bug
>
> Hi Mark,
>
> I think Huawei (huawei.xie at intel.com) has one patch set to fix this issue.
>
> If your patch is totally different with him:
>
> [dpdk-dev] [PATCH v4 0/2] lib/librte_pmd_i40e: set vlan filter fix
>
> please ignore my comments :)
>
> But you both calculation are different.
>
> Thanks,
> Michael
> On 12/4/2014 5:51 PM, Chen Jing D(Mark) wrote:
> > From: "Chen Jing D(Mark)"
> >
> > i40e uses an bitmap array to store those vlan tags that are set by
> > application. In function i40e_set_vlan_filter, it stores vlan tag
> > to wrong place. This change will fix it.
> >
> > Signed-off-by: Chen Jing D(Mark)
> > ---
> > lib/librte_pmd_i40e/i40e_ethdev.c | 11 ---
> > 1 files changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> > index 87e750a..cebc21d 100644
> > --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> > @@ -4163,8 +4163,8 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
> > {
> > uint32_t vid_idx, vid_bit;
> >
> > - vid_idx = (uint32_t) ((vlan_id >> 5) & 0x7F);
> > - vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
> > + vid_idx = (uint32_t)(vlan_id / I40E_UINT32_BIT_SIZE);
> > + vid_bit = (uint32_t)(1 << (vlan_id % I40E_UINT32_BIT_SIZE));
> >
> > if (vsi->vfta[vid_idx] & vid_bit)
> > return 1;
> > @@ -4178,14 +4178,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,
> > {
> > uint32_t vid_idx, vid_bit;
> >
> > -#define UINT32_BIT_MASK 0x1F
> > -#define VALID_VLAN_BIT_MASK 0xFFF
> > /* VFTA is 32-bits size array, each element contains 32 vlan bits, Find
> the
> > * element first, then find the bits it belongs to
> > */
> > - vid_idx = (uint32_t) ((vlan_id & VALID_VLAN_BIT_MASK) >>
> > - sizeof(uint32_t));
> > - vid_bit = (uint32_t) (1 << (vlan_id & UINT32_BIT_MASK));
> > + vid_idx = (uint32_t)(vlan_id / I40E_UINT32_BIT_SIZE);
> > + vid_bit = (uint32_t)(1 << (vlan_id % I40E_UINT32_BIT_SIZE));
> >
> > if (on)
> > vsi->vfta[vid_idx] |= vid_bit;
[dpdk-dev] [PATCH] i40e: Fix a vlan bug
As I don't know what commit he is based on, I'd like to generate a new patch with latest dpdk repo. > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, December 04, 2014 6:26 PM > To: Chen, Jing D > Cc: dev at dpdk.org; Qiu, Michael > Subject: Re: [dpdk-dev] [PATCH] i40e: Fix a vlan bug > > 2014-12-04 10:18, Qiu, Michael: > > Hi Mark, > > > > I think Huawei (huawei.xie at intel.com) has one patch set to fix this > > issue. > > > > If your patch is totally different with him: > > > > [dpdk-dev] [PATCH v4 0/2] lib/librte_pmd_i40e: set vlan filter fix > > > > please ignore my comments :) > > > > But you both calculation are different. > > Yes, please Jing (Mark), if you reworked the v4 patch, it would clearer > to have a changelog, to name it v5 and to insert it in the previous > thread with --in-reply-to. > At the moment, both patches block each other. > > -- > Thomas
[dpdk-dev] [PATCH] i40e: Fix a vlan bug
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, December 4, 2014 6:39 PM > To: Chen, Jing D > Cc: dev at dpdk.org; Qiu, Michael > Subject: Re: [dpdk-dev] [PATCH] i40e: Fix a vlan bug > > 2014-12-04 10:30, Chen, Jing D: > > As I don't know what commit he is based on, I'd like to generate a new > patch with latest dpdk repo. > > There's something wrong here. You rework a patch and you don't know what > was the current status but you expect that the reviewers can understand it > better than you? You don't understand me. Please read my above words again. As I said, he is in vacation, I came to fix problem. I know exactly what's the problem. So, I used simple way. > You are breaking all the elementary rules of patch management. Please kindly list all the elementary rules of patch management, please. If possible, can you post it somewhere so other new guys can find and follow? > We have currently 2 fixes pending for the same bug. > > PS: please don't top post. I apologized for top post. > > -- > Thomas > > > > -Original Message- > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > Sent: Thursday, December 04, 2014 6:26 PM > > > To: Chen, Jing D > > > Cc: dev at dpdk.org; Qiu, Michael > > > Subject: Re: [dpdk-dev] [PATCH] i40e: Fix a vlan bug > > > > > > 2014-12-04 10:18, Qiu, Michael: > > > > Hi Mark, > > > > > > > > I think Huawei (huawei.xie at intel.com) has one patch set to fix this > issue. > > > > > > > > If your patch is totally different with him: > > > > > > > > [dpdk-dev] [PATCH v4 0/2] lib/librte_pmd_i40e: set vlan filter fix > > > > > > > > please ignore my comments :) > > > > > > > > But you both calculation are different. > > > > > > Yes, please Jing (Mark), if you reworked the v4 patch, it would > > > clearer to have a changelog, to name it v5 and to insert it in the > > > previous thread with --in-reply-to. > > > At the moment, both patches block each other. > > > > > > -- > > > Thomas
[dpdk-dev] [PATCH] i40e: Fix a vlan bug
Hi, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, December 4, 2014 11:33 PM > To: Chen, Jing D > Cc: dev at dpdk.org; Qiu, Michael > Subject: Re: [dpdk-dev] [PATCH] i40e: Fix a vlan bug > > 2014-12-04 14:29, Chen, Jing D: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-12-04 10:30, Chen, Jing D: > > > > As I don't know what commit he is based on, I'd like to generate a > > > > new > > > patch with latest dpdk repo. > > > > > > There's something wrong here. You rework a patch and you don't know > > > what was the current status but you expect that the reviewers can > > > understand it better than you? > > > > You don't understand me. Please read my above words again. > > Yes there probably is a misunderstanding. > > > As I said, he is in vacation, I came to fix problem. I know exactly what's > > the > problem. So, I used simple way. > > So Huawei was trying to fix the bug and you suggest another way to fix it. > But you didn't explain why your fix is better than the previous one. > And we don't know if it's the continuation of his work or not. > If you are trying to fix exactly the same problem, incrementing the version > number of the patch makes clear that previous version doesn't need to be > reviewed, reworked or applied. In patchwork language, it supersedes the > previous patch which won't appear anymore. > OK, I prefer to follow Huawei's patch set and drop my commit. > > > You are breaking all the elementary rules of patch management. > > > > Please kindly list all the elementary rules of patch management, please. > > If possible, can you post it somewhere so other new guys can find and > follow? > > They are explained in http://dpdk.org/dev#send. > That's the ones I've enumerated in my first email: > - changelog > - increment version number (v5 here) > - use --in-reply-to > Thanks for explanation. > > > We have currently 2 fixes pending for the same bug. > > To sum it up, we need: > 1) a review > 2) an agreement that the Huawei's fix is superseded by this one > > Thank you > -- > Thomas > > > > PS: please don't top post. > > > > I apologized for top post. > > > > > > > > -- > > > Thomas > > > > > > > > -Original Message- > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > > Sent: Thursday, December 04, 2014 6:26 PM > > > > > To: Chen, Jing D > > > > > Cc: dev at dpdk.org; Qiu, Michael > > > > > Subject: Re: [dpdk-dev] [PATCH] i40e: Fix a vlan bug > > > > > > > > > > 2014-12-04 10:18, Qiu, Michael: > > > > > > Hi Mark, > > > > > > > > > > > > I think Huawei (huawei.xie at intel.com) has one patch set to fix > > > > > > this > > > issue. > > > > > > > > > > > > If your patch is totally different with him: > > > > > > > > > > > > [dpdk-dev] [PATCH v4 0/2] lib/librte_pmd_i40e: set vlan filter > > > > > > fix > > > > > > > > > > > > please ignore my comments :) > > > > > > > > > > > > But you both calculation are different. > > > > > > > > > > Yes, please Jing (Mark), if you reworked the v4 patch, it would > > > > > clearer to have a changelog, to name it v5 and to insert it in > > > > > the previous thread with --in-reply-to. > > > > > At the moment, both patches block each other. > > > > > > > > > > -- > > > > > Thomas > >
[dpdk-dev] [PATCH v4 0/2] lib/librte_pmd_i40e: set vlan filter fix
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie > Sent: Tuesday, December 2, 2014 5:02 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v4 0/2] lib/librte_pmd_i40e: set vlan filter fix > > This patchset fixes "set vlan filter" issue. > > v2 changes: > * add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA array > operation. > > v3 changes: > * code style fix > * rebase on latest commit > > v4 changes: > * add more descriptive commit message > > Huawei Xie (2): > vlan id set fix > add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation > > lib/librte_pmd_i40e/i40e_ethdev.c | 20 ++-- > lib/librte_pmd_i40e/i40e_ethdev.h | 9 + > 2 files changed, 19 insertions(+), 10 deletions(-) > Acked-by: Jing Chen > -- > 1.8.1.4
[dpdk-dev] [PATCH v2] i40e: workaround for X710 performance issues
Hi Helin,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Helin Zhang
> Sent: Monday, December 15, 2014 3:56 PM
> To: dev at dpdk.org
> Cc: Rowden, Aaron F
> Subject: [dpdk-dev] [PATCH v2] i40e: workaround for X710 performance
> issues
>
> As the fixes of below performance issues on X710 may not be integrated
> in latest version of firmware, a workaround in software PMD is needed.
> It is to re-configure 3 specific registers after being initialized.
> - Cannot achieve line rate on X710.
packet size?
> - Performance reduction when promiscuous mode is disabled.
You'd better add above descriptions in line with the code.
> Note that this workaround can be removed if the fixes are integrated
> in the firmware in future.
>
I saw below code applied register setting in case it's 40G device. Can you give
more description on what device this patch would boost performance?
Will 10G fiber interface benefit from the change?
> Signed-off-by: Helin Zhang
> ---
> lib/librte_pmd_i40e/i40e_ethdev.c | 87
> +++
> 1 file changed, 87 insertions(+)
>
> v2 changes:
> * Added a compile error fix.
>
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 008d62c..82c072b 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -198,6 +198,7 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
> enum rte_filter_type filter_type,
> enum rte_filter_op filter_op,
> void *arg);
> +static void i40e_configure_registers(struct i40e_hw *hw);
>
> /* Default hash key buffer for RSS */
> static uint32_t rss_key_default[I40E_PFQF_HKEY_MAX_INDEX + 1];
> @@ -443,6 +444,15 @@ eth_i40e_dev_init(__rte_unused struct eth_driver
> *eth_drv,
> /* Clear PXE mode */
> i40e_clear_pxe_mode(hw);
>
> + /*
> + * On X710, as old version of firmwares may have performance issues,
> + * 3 registers need to be re-configured with new values. And the
> latest
> + * version of firmware may not contain the fixes, workaround in SW
> + * driver is needed. This workaround can be removed when the fixes
> are
> + * integrated in firmware in future.
> + */
> + i40e_configure_registers(hw);
> +
> /* Get hw capabilities */
> ret = i40e_get_cap(hw);
> if (ret != I40E_SUCCESS) {
> @@ -5294,3 +5304,80 @@ i40e_pctype_to_flowtype(enum
> i40e_filter_pctype pctype)
>
> return flowtype_table[pctype];
> }
> +
> +static int
> +i40e_debug_read_register(struct i40e_hw *hw, uint32_t addr, uint64_t
> *val)
> +{
> + struct i40e_aq_desc desc;
> + struct i40e_aqc_debug_reg_read_write *cmd =
> + (struct i40e_aqc_debug_reg_read_write
> *)&desc.params.raw;
> + enum i40e_status_code status;
> +
> + i40e_fill_default_direct_cmd_desc(&desc,
> i40e_aqc_opc_debug_read_reg);
> + cmd->address = rte_cpu_to_le_32(addr);
> + status = i40e_asq_send_command(hw, &desc, NULL, 0, NULL);
> + if (status < 0)
> + return status;
> +
> + *val = ((uint64_t)(rte_le_to_cpu_32(cmd->value_high)) <<
> (CHAR_BIT *
> + sizeof(uint32_t))) + rte_le_to_cpu_32(cmd-
> >value_low);
> +
> + return status;
> +}
> +
> +/*
> + * On X710, as old version of firmwares may have performance issues,
> + * 3 registers need to be re-configured with new values. And the latest
> version
> + * of firmware may not contain the fixes, workaround in SW driver is
> needed.
> + * This workaround can be removed when the fixes are integrated in
> firmware in
> + * future.
> + */
> +static void
> +i40e_configure_registers(struct i40e_hw *hw)
> +{
> +#define I40E_GL_SWR_PRI_JOIN_MAP_0 0x26CE00
> +#define I40E_GL_SWR_PRI_JOIN_MAP_2 0x26CE08
> +#define I40E_GL_SWR_PM_UP_THR0x269FBC
> +#define I40E_GL_SWR_PRI_JOIN_MAP_0_VALUE 0x1200
> +#define I40E_GL_SWR_PRI_JOIN_MAP_2_VALUE 0x011f0200
> +#define I40E_GL_SWR_PM_UP_THR_VALUE 0x03030303
> +
> + static const struct {
> + uint32_t addr;
> + uint64_t val;
> + } reg_table[] = {
> + {I40E_GL_SWR_PRI_JOIN_MAP_0,
> I40E_GL_SWR_PRI_JOIN_MAP_0_VALUE},
> + {I40E_GL_SWR_PRI_JOIN_MAP_2,
> I40E_GL_SWR_PRI_JOIN_MAP_2_VALUE},
> + {I40E_GL_SWR_PM_UP_THR,
> I40E_GL_SWR_PM_UP_THR_VALUE},
> + };
> + uint64_t reg;
> + uint32_t i;
> + int ret;
> +
> + /* Below fix is for X710 only */
> + if (i40e_is_40G_device(hw->device_id))
> + return;
> +
> + for (i = 0; i < RTE_DIM(reg_table); i++) {
> + ret = i40e_debug_read_register(hw, reg_table[i].addr, ®);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Failed to read from 0x%x\n",
> + reg_table[i].addr);
> + break;
> +
[dpdk-dev] [PATCH v3] i40e: workaround for X710 performance issues
> -Original Message-
> From: Zhang, Helin
> Sent: Tuesday, December 16, 2014 4:23 PM
> To: dev at dpdk.org
> Cc: Chen, Jing D; Wu, Jingjing; Liu, Jijiang; Cao, Waterman; Lu, Patrick;
> Rowden, Aaron F; Zhang, Helin
> Subject: [PATCH v3] i40e: workaround for X710 performance issues
>
> On X710, performance number is far from the expectation on recent
> firmware versions. The fix for this issue may not be integrated in
> the following firmware version. So the workaround in software driver
> is needed. It needs to modify the initial values of 3 internal only
> registers. Note that the workaround can be removed when it is fixed
> in firmware in the future.
>
> Signed-off-by: Helin Zhang
> ---
> lib/librte_pmd_i40e/i40e_ethdev.c | 89
> +++
> 1 file changed, 89 insertions(+)
>
> v2 changes:
> * Added a compile error fix.
>
> v3 changes:
> * Used PRIx32 and PRIx64 instead for printing uint32_t and uint64_t
> variables.
> * Re-worded annotations, and commit logs.
>
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 008d62c..624f0ce 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -198,6 +198,7 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
> enum rte_filter_type filter_type,
> enum rte_filter_op filter_op,
> void *arg);
> +static void i40e_configure_registers(struct i40e_hw *hw);
>
> /* Default hash key buffer for RSS */
> static uint32_t rss_key_default[I40E_PFQF_HKEY_MAX_INDEX + 1];
> @@ -443,6 +444,16 @@ eth_i40e_dev_init(__rte_unused struct eth_driver
> *eth_drv,
> /* Clear PXE mode */
> i40e_clear_pxe_mode(hw);
>
> + /*
> + * On X710, performance number is far from the expectation on
> recent
> + * firmware versions. The fix for this issue may not be integrated in
> + * the following firmware version. So the workaround in software
> driver
> + * is needed. It needs to modify the initial values of 3 internal only
> + * registers. Note that the workaround can be removed when it is
> fixed
> + * in firmware in the future.
> + */
> + i40e_configure_registers(hw);
> +
> /* Get hw capabilities */
> ret = i40e_get_cap(hw);
> if (ret != I40E_SUCCESS) {
> @@ -5294,3 +5305,81 @@ i40e_pctype_to_flowtype(enum
> i40e_filter_pctype pctype)
>
> return flowtype_table[pctype];
> }
> +
> +static int
> +i40e_debug_read_register(struct i40e_hw *hw, uint32_t addr, uint64_t
> *val)
> +{
> + struct i40e_aq_desc desc;
> + struct i40e_aqc_debug_reg_read_write *cmd =
> + (struct i40e_aqc_debug_reg_read_write
> *)&desc.params.raw;
> + enum i40e_status_code status;
> +
> + i40e_fill_default_direct_cmd_desc(&desc,
> i40e_aqc_opc_debug_read_reg);
> + cmd->address = rte_cpu_to_le_32(addr);
> + status = i40e_asq_send_command(hw, &desc, NULL, 0, NULL);
> + if (status < 0)
> + return status;
> +
> + *val = ((uint64_t)(rte_le_to_cpu_32(cmd->value_high)) <<
> (CHAR_BIT *
> + sizeof(uint32_t))) + rte_le_to_cpu_32(cmd-
> >value_low);
> +
> + return status;
> +}
> +
> +/*
> + * On X710, performance number is far from the expectation on recent
> firmware
> + * versions. The fix for this issue may not be integrated in the following
> + * firmware version. So the workaround in software driver is needed. It
> needs
> + * to modify the initial values of 3 internal only registers. Note that the
> + * workaround can be removed when it is fixed in firmware in the future.
> + */
> +static void
> +i40e_configure_registers(struct i40e_hw *hw)
> +{
> +#define I40E_GL_SWR_PRI_JOIN_MAP_0 0x26CE00
> +#define I40E_GL_SWR_PRI_JOIN_MAP_2 0x26CE08
> +#define I40E_GL_SWR_PM_UP_THR0x269FBC
> +#define I40E_GL_SWR_PRI_JOIN_MAP_0_VALUE 0x1200
> +#define I40E_GL_SWR_PRI_JOIN_MAP_2_VALUE 0x011f0200
> +#define I40E_GL_SWR_PM_UP_THR_VALUE 0x03030303
> +
> + static const struct {
> + uint32_t addr;
> + uint64_t val;
> + } reg_table[] = {
> + {I40E_GL_SWR_PRI_JOIN_MAP_0,
> I40E_GL_SWR_PRI_JOIN_MAP_0_VALUE},
> + {I40E_GL_SWR_PRI_JOIN_MAP_2,
> I40E_GL_SWR_PRI_JOIN_MAP_2_VALUE},
> + {I40E_GL_SWR_PM_UP_THR,
> I40E_GL_SWR_PM_UP_THR_VALUE},
> + };
> + uint64_t reg;
> + uint32_t i;
> + int ret;
> +
> + /* Below fix is for X710 onl
[dpdk-dev] [PATCH 0/3] Rename field name for RX/TX queue start/stop
Hi Thomas, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > Sent: Tuesday, July 22, 2014 5:38 PM > To: Ouyang, Changchun > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 0/3] Rename field name for RX/TX queue > start/stop > > Hi, > > 2014-07-22 15:47, Ouyang Changchun: > > This patch series include 3 things: > > 1) Rename the field name from start_rx_per_q to rx_enable_queue in > > struct rte_eth_rxconf, and do same thing for TX. > > This patch also update description for field rx_enable_queue and > tx_enable_queue. > > 2) According to 1), update field name from start_rx_per_q to > rx_enable_queue in struct igb_rx_queue > > in ixgbe PMD, do same thing for TX. > > 3) Update its reference in sample vhost. > > In order to be atomic (and do not break git bisect), you should submit > it in one patch. > Title would be "ethdev: rename queue enabler field" or something like that. > But the most important in such change is to explain why you make it. > > Thanks > -- > Thomas The reason adding this patch is that "start_rx_per_q" and "start_tx_per_q" has requirement in NIC driver in some cases. The implication includes: 1. don't fill mbuf address in RX ring in later dev start function call. 2. don't try to switch this rx/tx queues on in later dev start function call. Instead, application will call rte_eth_dev_rx/tx_queue_start/stop to control this queue. If the NIC driver tried to support these 2 options, it will have to satisfy above 2 conditions. But the problem is that the 2 fields definition don't have a word to claim on their requirement. So, we needs this patch and add comments. As for renaming, it's not so important. Just for better understanding.
[dpdk-dev] [PATCH v2] ethdev: Rename RX/TX enable queue field for queue start and stop
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ouyang Changchun
> Sent: Wednesday, July 23, 2014 12:48 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2] ethdev: Rename RX/TX enable queue field
> for queue start and stop
>
> Update comments for the field start_rx_per_q for better readability.
> Rename the field name to rx_enable_queue for better readability too.
> Accordingly Update its reference in sample vhost.
>
> Signed-off-by: Changchun Ouyang
Acked-by Chen Jing (Mark)
> ---
> examples/vhost/main.c | 4 ++--
> lib/librte_ether/rte_ethdev.h | 16 ++--
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8
> lib/librte_pmd_ixgbe/ixgbe_rxtx.h | 4 ++--
> 4 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 193aa25..2eea431 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -2984,9 +2984,9 @@ MAIN(int argc, char *argv[])
> char pool_name[RTE_MEMPOOL_NAMESIZE];
> char ring_name[RTE_MEMPOOL_NAMESIZE];
>
> - rx_conf_default.start_rx_per_q = (uint8_t)zero_copy;
> + rx_conf_default.rx_enable_queue = (uint8_t)zero_copy;
> rx_conf_default.rx_drop_en = 0;
> - tx_conf_default.start_tx_per_q = (uint8_t)zero_copy;
> + tx_conf_default.tx_enable_queue = (uint8_t)zero_copy;
> nb_mbuf = num_rx_descriptor
> + num_switching_cores * MBUF_CACHE_SIZE_ZCP
> + num_switching_cores * MAX_PKT_BURST;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 50df654..ba439f6 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -604,7 +604,16 @@ struct rte_eth_rxconf {
> struct rte_eth_thresh rx_thresh; /**< RX ring threshold registers. */
> uint16_t rx_free_thresh; /**< Drives the freeing of RX descriptors.
> */
> uint8_t rx_drop_en; /**< Drop packets if no descriptors are available.
> */
> - uint8_t start_rx_per_q; /**< start rx per queue. */
> + /**< If rx_enable_queue is true, rte_eth_dev_rx_queue_start
> should be
> + invocated to start RX for one queue after rte_eth_dev_start
> is
> + invocated, and rte_eth_dev_rx_queue_start instead of
> + rte_eth_dev_start is responsible for allocating mbuf from
> + mempool and setup the DMA physical address. It is useful in
> + such scenario: buffer address is not available at the point of
> + rte_eth_dev_start's invocating but available later, e.g. in
> + VHOST zero copy case, the buffer address used to setup
> DMA
> + address is available only after one VM(guest) startup. */
> + uint8_t rx_enable_queue;
> };
>
> #define ETH_TXQ_FLAGS_NOMULTSEGS 0x0001 /**< nb_segs=1 for all
> mbufs */
> @@ -625,7 +634,10 @@ struct rte_eth_txconf {
> uint16_t tx_rs_thresh; /**< Drives the setting of RS bit on TXDs. */
> uint16_t tx_free_thresh; /**< Drives the freeing of TX buffers. */
> uint32_t txq_flags; /**< Set flags for the Tx queue */
> - uint8_t start_tx_per_q; /**< start tx per queue. */
> + /**< If tx_enable_queue is true, rte_eth_dev_tx_queue_start must
> be
> + invocated to start TX for one queue after rte_eth_dev_start
> is
> + invocated. Refer to start_rx_per_q for the use case. */
> + uint8_t tx_enable_queue;
> };
>
> /**
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index dfc2076..2872fad 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1846,7 +1846,7 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev
> *dev,
> txq->port_id = dev->data->port_id;
> txq->txq_flags = tx_conf->txq_flags;
> txq->ops = &def_txq_ops;
> - txq->start_tx_per_q = tx_conf->start_tx_per_q;
> + txq->tx_enable_queue = tx_conf->tx_enable_queue;
>
> /*
>* Modification to set VFTDT for virtual function if vf is detected
> @@ -2091,7 +2091,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev
> *dev,
> rxq->crc_len = (uint8_t) ((dev->data-
> >dev_conf.rxmode.hw_strip_crc) ?
> 0 : ETHER_CRC_LEN);
> rxq->drop_en = rx_conf->rx_drop_en;
> - rxq->start_rx_per_q = rx_conf->start_rx_per_q;
> + rxq->rx_enable_queue = rx_conf->rx_enable_queue;
>
> /*
>* Allocate RX ring hardware descriptors. A memzone large enough to
> @@ -3652,13 +3652,13 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>
> for (i = 0; i < dev->data->nb_tx_queues; i++) {
> txq = dev->data->tx_queues[i];
> - if (!txq->start_tx_per_q)
> + if (!txq->tx_enable_queue)
> ixgbe_dev_tx_queue_start(dev, i);
> }
>
> for (i = 0; i <
[dpdk-dev] [PATCH] i40e:fix an issue in i40e_dev_info_get
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jijiang Liu
> Sent: Tuesday, May 19, 2015 1:56 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] i40e:fix an issue in i40e_dev_info_get
>
> To get device VMDQ info when only i40e VMDQ feature is enabled.
>
> Signed-off-by: Jijiang Liu
>
> ---
> lib/librte_pmd_i40e/i40e_ethdev.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 96700e4..1faae83 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -1565,7 +1565,7 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
> ETH_TXQ_FLAGS_NOOFFLOADS,
> };
>
> - if (pf->flags | I40E_FLAG_VMDQ) {
> + if (pf->flags & I40E_FLAG_VMDQ) {
> dev_info->max_vmdq_pools = pf->max_nb_vmdq_vsi;
> dev_info->vmdq_queue_base = dev_info->max_rx_queues;
> dev_info->vmdq_queue_num = pf->vmdq_nb_qps *
> --
> 1.7.7.6
Acked-by: Jing Chen
[dpdk-dev] [PATCH] librte_pmd_fm10k: Fix max_vfs issue in fm10k PMD
Hi, > -Original Message- > From: Michael Qiu [mailto:qiudayu at cn.ibm.com] > Sent: Tuesday, April 14, 2015 5:25 PM > To: dev at dpdk.org > Cc: Chen, Jing D; Qiu, Michael > Subject: [PATCH] librte_pmd_fm10k: Fix max_vfs issue in fm10k PMD > > From: Michael Qiu > > In DPDK, max_vfs means vf numbers created, not the max number vfs > the device supported. > > Signed-off-by: Michael Qiu > --- > lib/librte_pmd_fm10k/fm10k_ethdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c > b/lib/librte_pmd_fm10k/fm10k_ethdev.c > index 0312fad..297ff88 100644 > --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c > +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c > @@ -770,7 +770,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev, > dev_info->max_tx_queues = hw->mac.max_queues; > dev_info->max_mac_addrs = 1; > dev_info->max_hash_mac_addrs = 0; > - dev_info->max_vfs= FM10K_MAX_VF_NUM; > + dev_info->max_vfs= dev->pci_dev->max_vfs; > dev_info->max_vmdq_pools = ETH_64_POOLS; > dev_info->rx_offload_capa = > DEV_RX_OFFLOAD_IPV4_CKSUM | > -- > 1.9.3 Acked-by Jing Chen
Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
Hi, Vincent, > -Original Message- > From: Vincent JARDIN [mailto:vincent.jar...@6wind.com] > Sent: Tuesday, January 10, 2017 9:30 PM > To: Scott Daniels ; dev@dpdk.org > Cc: kaust...@research.att.com; az5...@att.com; Chen, Jing D > > Subject: Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF > on > i40e > > Hi Scott, > > Le 04/01/2017 à 22:09, Scott Daniels a écrit : > > With holidays we are a bit late with our thoughts, but would like to > > toss them into the mix. > > Same, I hope I am not missing emails. I do appreciate your arguments, it > provides lot of light. See below, > > > The original NAK is understandable, however having the ability to > > configure the PF via DPDK is advantageous for several reasons: > > > > 1) While some functions may be duplicated and/or available from the kernel > > driver, it is often not possible to introduce new kernel drivers into > > production without a large amount of additional testing of the entire > > platform which can cause a significant delay when introducing a DPDK based > > product. If the PF control is a part of the DPDK environment, then only > > the application needs to pass the operational testing before deployment; a > > much more simple task. > > So we agree: you confirm that your foresee the benefits of using DPDK to > *bypass the role of the Kernel being the PF* of reference for the > hypervisor. > > > 2) If the driver changes are upstreamed into the kernel proper, the > > difficulty of operational readiness testing increases as a new kernel is > > introduced, and further undermines the ability to quickly and easily > > release a DPDK based application into production. While the application > > may eventually fall back on driver and/or kernel support, this could be > > years away. > > I do agree with the benefits of the agilities and the upsides it brings. > But they are other options to get the same agility without creating a > fragmentation of PFs. > > For example, you do not have to update the whole kernel, you can just > update the PF kernel module that can be upgraded with the latest needed > features. > > > 3) As DPDK is being used to configure the NIC, it just seems to make > > sense, for consistency, that the configuration capabilities should include > > the ability to configure the PF as is proposed. > > From this perspective, the kernel modules are fine for the PF: most > kernels of hypervisors support it without the need to upgrade their kernels. > > To summarize, I understand that you need a flexible way to upgrade PF > features without touching/changing the kernel. So let's check the kernel > module option? VFD brings some interesting capabilities, could it be a > way to push and stimulate the i40e features instead of using DPDK? > >https://sourceforge.net/projects/e1000/files/i40e%20stable/ > for instance could be better stimulated. May I ask what if DPDK VF need a new extension function from PF? Then, we'll have to build kernel community expertise and submit patch to Linux PF and wait for merge. Your proposal indicates DPDK community submitters will have to ask Linux community to authorize if we'll have any requirements in DPDK VF. Comparing fragmentation, the extra dependency worry me most. Can you imagine how long it will be for any VF features gets ready?
Re: [dpdk-dev] fm10k pmd limitations
> -Original Message- > From: Yigit, Ferruh > Sent: Thursday, January 12, 2017 7:57 PM > To: Shaham Fridenberg ; dev@dpdk.org > Cc: Chen, Jing D > Subject: Re: [dpdk-dev] fm10k pmd limitations > > On 12/13/2016 1:49 PM, Shaham Fridenberg wrote: > > Hey guys, > > > > I'm using dpdk 16.4 and fm10k card. According to the code, there's no > support for disabling vlan stripping and VLAN QinQ in pmd fm10k. > > Does anybody know why? If there's any way to work-around it, or when is a > behavior change expected? Yes, HW will always strip it and driver will copy it into (struct rte_mbuf *)mbuf->vlan_tci. > > I need my VF to receive the packets with the VLAN headers. VF can read from rte_mbuf. > > Even if it's possible to change this configurations through the linux kernel > fm10k driver? > > > > Thanks! > > > > CC fm10k maintainer: Jing Chen
Re: [dpdk-dev] [RFC 1/2] doc: introduction to prgdev
Hi, Jan, On 2/1/2017 7:41 PM, Jan Blunck wrote: On Fri, Jan 20, 2017 at 4:21 AM, Chen Jing D(Mark) wrote: This is the documentation to describe what prgdev is, how to use prgdev API and accomplish an image download. Signed-off-by: Chen Jing D(Mark) --- doc/guides/prog_guide/prgdev_lib.rst | 457 ++ 1 files changed, 457 insertions(+), 0 deletions(-) create mode 100644 doc/guides/prog_guide/prgdev_lib.rst diff --git a/doc/guides/prog_guide/prgdev_lib.rst b/doc/guides/prog_guide/prgdev_lib.rst new file mode 100644 index 000..3917c18 --- /dev/null +++ b/doc/guides/prog_guide/prgdev_lib.rst @@ -0,0 +1,457 @@ +Overview + + +More and more devices start to support on-die programming, which include +NIC, GPU, FPGA, etc. This capability has a requirement to drivers that +introduce a API for application to download/upload image to/from the HW. +So, it's necessary to provide a generic API to perform image download, upload, +validation, etc. + +This RFC patch intends to introduce a DPDK generic programming device layer, +called prgdev below, to provide an abstract, generic APIs for applications to +program hardware without knowing the details of programmable devices. From +driver's perspective, they'll try to adapt their functions to the abstract +APIs defined in prgdev. + +The major purpose of prgdev is to help DPDK users to load/upgrade RTL images +for FPGA devices, or upgrade firmware for programmble NICs. + +But those devices that have the capability to do on-die programming may +expose versatile talents to apps. Add/delete/update entries in flow table, +update/overwrite the firmware/microcode, add a new algorithm, update profiles +, etc, which is hard to be abstracted as generic behaviors. So, prgdev won't +include the APIs to perform device-unique actions in current stage until it +became popular. On the contratry, it only provides the simple capability to +download a segment of buffer(image) from host to on-die device, or vice versa. +The image could be an algorithm, a flow table in the on-die SRAM, a firmware +image running in the device, a function within a pipeline, etc. prgdev won't +try to inteprete the content and it's transparent to prgdev. Apps and devices +can communicate with a language they can understand each other, which is not +provided by prgdev to decide what needs to be programmed. + +When the set of APIs is introduced, a general question is why we need it in +DPDK community? Why we can't use offline tool to perform same thing? The answer +is the prgdev provide a generic, online API to applications, in the meanwile, +offers a capability to switch driver dynamically when downloaded image changed +personality and a new driver is required. Comparing offline tool, it have online +programmability (see below examples); Comparing online tool, it provides a +generic API for many HWs; Comparing generic online tool/API for many products, +it provides a capability to switch driver dynamically. + +There are various means to reach same goal, we'll try to find the best/better +way to approach. All above advantages will help prgdev to be a 'better choice'. + From my point of view this doesn't belong into the DPDK. On Linux this is traditionally handled by udev and you already have the freedom to use userspace applications to program a device requiring firmware in that case. I don't believe that modeling this in the DPDK explicitly is the right thing to do. Can you kindly educate me what udev can do? It's for NIC firmware upgrade? This prgdev is not only for NIC, the major use case is FPGA, but I found programmable NIC is also applicable to the API model and can be added into the scope. Especially if the device supports changing personality it is required to unplug the existing personality before reprogramming. You can do this already today. Also writing OOB firmware data that changes configuration should be possible today by handling interrupts. What if application is using DPDK, can we do in the same manner without leaving DPDK instance? Maybe we can come up with an example application that demonstrates how the different infrastructure components could get orchestrated. Do you have a device in mind that supports this? As Cunming suggested, the incoming FPGA is the desired device to use this API. OVS that is using DPDK can benefit from the API and download personalities in running time for blank FPGA, or upgrade for prior-programmed case. Regards, Jan
Re: [dpdk-dev] [RFC 2/2] prgdev: introduce generic prgdev API
Hi, Keith, Stephen,
> -Original Message-
> From: Wiles, Keith
> Sent: Thursday, February 9, 2017 7:07 AM
> To: Stephen Hemminger
> Cc: Chen, Jing D ; dev@dpdk.org; Rogers, Gerald
>
> Subject: Re: [dpdk-dev] [RFC 2/2] prgdev: introduce generic prgdev API
>
>
> > On Feb 8, 2017, at 4:49 PM, Stephen Hemminger
> wrote:
> >
> > On Fri, 20 Jan 2017 11:21:38 +0800
> > "Chen Jing D(Mark)" wrote:
> >
> >> +struct rte_prg_dev_info {
> >> + struct rte_pci_device *pci_dev; /**< Device PCI information. */
> >
> > NAK
> >
> > No new API should refer to PCI devices. We are trying to make all
> > references to devices generic. Crypto suceeded, ether is in process, but no
> new ones please.
>
> The design is not PCI centric and should not be PCI centric, it is just what
> they
> started with months ago. So please look past the PCI bits as they have not
> integrated into the latest code base yet.
>
I got you. Will remove the reference in formal patch.
Re: [dpdk-dev] [PATCH v2 29/32] net/i40e: parse more VF parameter and configure
Hi, Ferruh, > -Original Message- > From: Yigit, Ferruh > Sent: Wednesday, December 7, 2016 11:19 PM > To: Lu, Wenzhuo ; dev@dpdk.org > Cc: Chen, Jing D > Subject: Re: [dpdk-dev] [PATCH v2 29/32] net/i40e: parse more VF parameter > and configure > > On 12/7/2016 3:32 AM, Wenzhuo Lu wrote: > > When VF requested to configure TX queue, a few parameters are missed > > to be configured in PF host. This change have more fields parsed and > > configured for TX context. > > What is the effect of missing Tx paramters configured? > > If this cause a bug, this patch should be a fix. > This need to analyze case by case. If PF driver is serving a DPDK VF client, the missing part is OK. If serving a Linux VF client, the missing part will cause some unexpected TX queue behaviors. So, this is an enhancement to support Linux VF client. > > > > Signed-off-by: Chen Jing D(Mark) > > --- > > drivers/net/i40e/i40e_pf.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c > > index 8319c2c..1ad5ed1 100644 > > --- a/drivers/net/i40e/i40e_pf.c > > +++ b/drivers/net/i40e/i40e_pf.c > > @@ -422,10 +422,12 @@ > > > > /* clear the context structure first */ > > memset(&tx_ctx, 0, sizeof(tx_ctx)); > > - tx_ctx.new_context = 1; > > tx_ctx.base = txq->dma_ring_addr / I40E_QUEUE_BASE_ADDR_UNIT; > > tx_ctx.qlen = txq->ring_len; > > tx_ctx.rdylist = rte_le_to_cpu_16(vf->vsi->info.qs_handle[0]); > > + tx_ctx.head_wb_ena = txq->headwb_enabled; > > + tx_ctx.head_wb_addr = txq->dma_headwb_addr; > > + > > err = i40e_clear_lan_tx_queue_context(hw, abs_queue_id); > > if (err != I40E_SUCCESS) > > return err; > >
Re: [dpdk-dev] [PATCH v2 28/32] net/i40e: return correct vsi_id
Ferruh, > -Original Message- > From: Yigit, Ferruh > Sent: Wednesday, December 7, 2016 11:16 PM > To: Lu, Wenzhuo ; dev@dpdk.org > Cc: Chen, Jing D > Subject: Re: [dpdk-dev] [PATCH v2 28/32] net/i40e: return correct vsi_id > > On 12/7/2016 3:32 AM, Wenzhuo Lu wrote: > > PF host didn't return correct VSI id to VF. > > This change fix it. > > This looks like a fix for current code, > can you please update commit title and log to reflect the fix? > This is similar patch to support Linux VF client. DPDK VF client needn't Vsi ID. > > > > Signed-off-by: Chen Jing D(Mark) > > --- > > drivers/net/i40e/i40e_pf.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c > > index 0f582ed..8319c2c 100644 > > --- a/drivers/net/i40e/i40e_pf.c > > +++ b/drivers/net/i40e/i40e_pf.c > > @@ -351,8 +351,7 @@ > > > > /* Change below setting if PF host can support more VSIs for VF */ > > vf_res->vsi_res[0].vsi_type = I40E_VSI_SRIOV; > > - /* As assume Vf only has single VSI now, always return 0 */ > > - vf_res->vsi_res[0].vsi_id = 0; > > + vf_res->vsi_res[0].vsi_id = vf->vsi->vsi_id; > > vf_res->vsi_res[0].num_queue_pairs = vf->vsi->nb_qps; > > ether_addr_copy(&vf->mac_addr, > > (struct ether_addr *)vf_res->vsi_res[0].default_mac_addr); > >
Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
HI, Ferruh,
Best Regards,
Mark
> -Original Message-
> From: Yigit, Ferruh
> Sent: Wednesday, December 07, 2016 10:18 PM
> To: Lu, Wenzhuo ; dev@dpdk.org
> Cc: Chen, Jing D
> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
>
> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> > Add a function to configure vlan strip enable/disable for specific
> > SRIOV VF device.
> >
> > Signed-off-by: Chen Jing D(Mark)
> > ---
>
> <...>
>
> > +
> > +/* Set vlan strip on/off for specific VF from host */
> > +int
> > +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
> > +{
> > + struct rte_eth_dev *dev;
> > + struct i40e_pf *pf;
> > + struct i40e_vsi *vsi;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> > +
> > + dev = &rte_eth_devices[port];
> > + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > +
> > + if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > + PMD_DRV_LOG(ERR, "Invalid argument.");
> > + return -EINVAL;
> > + }
> > +
> > + vsi = pf->vfs[vf_id].vsi;
> > +
> > + if (vsi)
> > + return i40e_vsi_config_vlan_stripping(vsi, !!on);
> > + else
>
> if vd_if is valid, can vsi be NULL? If so this check may be required in
> some prev patches too.
It's a little impossible. This sanity check just make the code stronger.
>
> > + return -EINVAL;
> > +}
> > diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> > b/drivers/net/i40e/rte_pmd_i40e.h
> > index ca5e05a..043ae62 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.h
> > +++ b/drivers/net/i40e/rte_pmd_i40e.h
> > @@ -187,4 +187,24 @@ int rte_pmd_i40e_set_vf_multicast_promisc(uint8_t port,
> > int rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
> > struct ether_addr *mac_addr);
> >
> > +/**
> > + * Enable/Disable vf vlan strip for all queues in a pool
> > + *
> > + * @param port
> > + *The port identifier of the Ethernet device.
> > + * @param vf
> > + *ID specifying VF.
> > + * @param on
> > + *1 - Enable VF's vlan strip on RX queues.
> > + *0 - Disable VF's vlan strip on RX queues.
> > + *
> > + * @return
> > + * - (0) if successful.
> > + * - (-ENOTSUP) if hardware doesn't support this feature.
>
> Is this error type returned?
Good catch. Only -EINVAL and -ENODEV would be returned.
>
> > + * - (-ENODEV) if *port* invalid.
> > + * - (-EINVAL) if bad parameter.
> > + */
> <...>
Re: [dpdk-dev] [PATCH v2 27/32] net/i40e: change version number to support Linux VF
Hi, Ferruh,
> -Original Message-
> From: Yigit, Ferruh
> Sent: Wednesday, December 07, 2016 11:14 PM
> To: Lu, Wenzhuo ; dev@dpdk.org
> Cc: Chen, Jing D
> Subject: Re: [dpdk-dev] [PATCH v2 27/32] net/i40e: change version number to
> support Linux VF
>
> On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> > i40e PF host only support to work with DPDK VF driver, Linux
> > VF driver is not supported. This change will enhance in version
> > number returned.
> >
> > Current version info returned won't be able to be recognized
> > by Linux VF driver, change to values that both DPDK VF and Linux
> > driver can recognize.
> >
> > The expense is original DPDK host specific feature like
> > CFG_VLAN_PVID and CONFIG_VSI_QUEUES_EXT will not available.
> >
> > DPDK VF also can't identify host driver by version number returned.
> > It always assume talking with Linux PF.
>
> I guess you mention from following code [1], should it be also updated
> to prevent it giving wrong information:
I'd like to update it into some docs.
>
> [1] i40e_ethdev_vf.c
> if (vf->version_major == I40E_DPDK_VERSION_MAJOR)
> PMD_DRV_LOG(INFO, "Peer is DPDK PF host");
> else if ((vf->version_major == I40E_VIRTCHNL_VERSION_MAJOR) &&
> (vf->version_minor <= I40E_VIRTCHNL_VERSION_MINOR))
> PMD_DRV_LOG(INFO, "Peer is Linux PF host");
> else {
>
> >
> > Signed-off-by: Chen Jing D(Mark)
> > ---
>
> <...>
Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
Hi, Ferruh,
> -Original Message-
> From: Yigit, Ferruh
> Sent: Thursday, December 08, 2016 6:43 PM
> To: Chen, Jing D ; Lu, Wenzhuo ;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
>
> On 12/8/2016 9:10 AM, Chen, Jing D wrote:
> > HI, Ferruh,
> >
> > Best Regards,
> > Mark
> >
> >
> >> -Original Message-
> >> From: Yigit, Ferruh
> >> Sent: Wednesday, December 07, 2016 10:18 PM
> >> To: Lu, Wenzhuo ; dev@dpdk.org
> >> Cc: Chen, Jing D
> >> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
> >>
> >> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> >>> Add a function to configure vlan strip enable/disable for specific
> >>> SRIOV VF device.
> >>>
> >>> Signed-off-by: Chen Jing D(Mark)
> >>> ---
> >>
> >> <...>
> >>
> >>> +
> >>> +/* Set vlan strip on/off for specific VF from host */
> >>> +int
> >>> +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
> >>> +{
> >>> + struct rte_eth_dev *dev;
> >>> + struct i40e_pf *pf;
> >>> + struct i40e_vsi *vsi;
> >>> +
> >>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> >>> +
> >>> + dev = &rte_eth_devices[port];
> >>> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> >>> +
> >>> + if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> >>> + PMD_DRV_LOG(ERR, "Invalid argument.");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + vsi = pf->vfs[vf_id].vsi;
> >>> +
> >>> + if (vsi)
> >>> + return i40e_vsi_config_vlan_stripping(vsi, !!on);
> >>> + else
> >>
> >> if vd_if is valid, can vsi be NULL? If so this check may be required in
> >> some prev patches too.
> >
> > It's a little impossible. This sanity check just make the code stronger.
> >
>
> If it is impossible, do you agree to remove this? And if this can be
> possible we must update other patches, almost all other patches assume
> this can't be NULL.
I'll recommend other patches to add it, too.
The reason is we can't image if there is some code change have impact in
future, the necessary sanity check in slow patch make code stronger.
Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
Hi, Vincent, > -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Vincent JARDIN > Sent: Saturday, December 17, 2016 4:36 AM > To: Yigit, Ferruh ; dev@dpdk.org > Cc: Wu, Jingjing ; Zhang, Helin > Subject: Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF > on > i40e > > Le 16/12/2016 à 20:02, Ferruh Yigit a écrit : > > As we need to support the scenario kernel PF + DPDK VF, > > DPDK follows the interface between kernel PF + kernel VF. > > Please, it has to be proven that DPDK provides the same interface that > the kernel. It seems insane to duplicate kernel's PF into the DPDK > without a strong guarantee that it is a strict same behaviour. For instance, >- what will happen when the DPDK's PF will have a new feature which > is not into the kernel? >- what will happen when the Kernel's PF will have a feature with > different capabilities that the kernel? > > Please, make it clear before. Currently, for me it is a nack of this serie. > Let me try to explain. When we DPDK developed i40e drivers several years ago, we found the API and data structures defined in shared code for VF and PF device communication can satisfy DPDK's requirements, so we just followed that API. At that time, we'll try to satisfy 3 scenarios. 1. Linux PF + Linux VF : it's not our scope. 2. Linux PF + DPDK VF: there is no problems observed since we use same API. 3. DPDK PF + DPDK VF: that's fully under control. The only scenario was not considered is DPDK PF + Linux VF. Since then, both Linux and DPDK keep developing code. Then, we found it's necessary to extend VF capability (Like promiscuous mode). It will be hard to ask Linux PF to support that service considering upstream effort in Linux world. So, supporting it in scenario of DPDK PF + DPDK VF became best candidate. But how can VF identify who is serving it then decide if extended request can be dispatched? So, DPDK PF will send a special version number for this purpose. As time passed by, we realized there some use case for DPDK PF + Linux VF and it's not supported yet. Then, we found our implementation in DPDK PF is not complete since we only had considered how to serve DPDK VF at that time. So, we need some improvement on the PF host driver. Besides that, 'send a special version' to VF doesn't work now since Linux VF can't interpret the version info. So, we behave the same as Linux PF driver with sacrifice of extended function in DPDK VF. Let me summary the chang here. 1. We shared the same interface with Linux driver from beginning. 2. This change will support both Linux VF and DPDK VF. 3. We'll find a way for DPDK VF identifying who is serving it so it can use extended function in future release.
Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] > Sent: Monday, December 19, 2016 9:21 PM > To: Chen, Jing D > Cc: dev@dpdk.org; Vincent JARDIN ; Yigit, Ferruh > ; Wu, Jingjing ; Zhang, Helin > > Subject: Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel > VF on i40e > > 2016-12-19 09:01, Chen, Jing D: > > Since then, both Linux and DPDK keep developing code. Then, we found > > it's necessary to extend VF capability (Like promiscuous mode). It > > will be hard to ask Linux PF to support that service considering upstream > effort in Linux world. > > Please, could you clarify this? > Do you mean you cannot change the Linux driver? There are 2 things here. One is to add extended functionality into Linux driver, Another is to have it upstream into kernel world. The first one can be achieved easier, while later one may be difficult. They will have concern why VM can have such privilege (like promisc mode). But I need to check as I know there is some mechanism now to make a VM privileged.
Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
Hi, > -Original Message- > From: Vincent JARDIN [mailto:vincent.jar...@6wind.com] > Sent: Monday, December 19, 2016 9:46 PM > To: Chen, Jing D ; Thomas Monjalon > > Cc: dev@dpdk.org; Yigit, Ferruh ; Wu, Jingjing > ; Zhang, Helin > Subject: Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF > on > i40e > > Le 19/12/2016 à 14:39, Chen, Jing D a écrit : > > They will > > have concern why VM can have such privilege (like promisc mode). But I need > > to check as I know there is some mechanism now to make a VM privileged. > > From iproute2's man: > <-- > trust on|off - trust the specified VF user. This enables that VF user > can set a specific feature which may impact security and/or performance. > (e.g. VF multicast promiscuous mode) > --> > > So yes, it is possible to get PF features upstream'd into the kernel > without having a specific PF into DPDK. That's a collaboration with another team. we'll follow-up that but not guarantee it will happen. May I ask if my reply make it clear? Still NAC for this patch?
Re: [dpdk-dev] [PATCH] net/fm10k/base: add a break statement
Hi, Chenghu, > -Original Message- > From: Chenghu Yao [mailto:yao.chen...@zte.com.cn] > Sent: Wednesday, December 21, 2016 11:05 AM > To: Chen, Jing D > Cc: dev@dpdk.org; Chenghu Yao > Subject: [PATCH] net/fm10k/base: add a break statement > > In function fm10k_mbx_create_reply(), the last case branch > has no break statement. > > Signed-off-by: Chenghu Yao > --- > drivers/net/fm10k/base/fm10k_mbx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/fm10k/base/fm10k_mbx.c > b/drivers/net/fm10k/base/fm10k_mbx.c > index 2e70434..45d6ddb 100644 > --- a/drivers/net/fm10k/base/fm10k_mbx.c > +++ b/drivers/net/fm10k/base/fm10k_mbx.c > @@ -1084,6 +1084,7 @@ STATIC s32 fm10k_mbx_create_reply(struct fm10k_hw > *hw, > case FM10K_STATE_CLOSED: > /* generate new header based on data */ > fm10k_mbx_create_disconnect_hdr(mbx); > + break; > default: > break; > } Thanks for contributing code. But there are 2 problems here. 1. You are modifying base code under 'base' directory. It assumed READ ONLY because there is another Intel team are maintaining it. 2. Without your change, the code won't have any negative effect. Yes, I appreciate your change to make it stronger. So, I'd to say 'NAC' for this patch.
Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
Hi, Vincent,
> -Original Message-
> From: Vincent JARDIN [mailto:vincent.jar...@6wind.com]
> Sent: Tuesday, December 20, 2016 11:19 PM
> To: Chen, Jing D ; Thomas Monjalon
>
> Cc: dev@dpdk.org; Yigit, Ferruh ; Wu, Jingjing
> ; Zhang, Helin
> Subject: Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF
> on
> i40e
>
> Le 20/12/2016 à 05:48, Chen, Jing D a écrit :
> > That's a collaboration with another team. we'll follow-up that but not
> > guarantee
> > it will happen.
> > May I ask if my reply make it clear? Still NAC for this patch?
>
> Yes still nack, I am not confident with this PF approach since you are
> breaking Linux PF behavior. It does not provide guarantees with PF.
> Something is missing to guarantee the compatibilities.
Let me introduce the rationale of API between PF and VF.
There is a common head file visible to both PF and VF, which includes a set of
command
and data structures, which is managed by a version number. Below is an example.
Major_verion=1
Minor_verion=1
enum i40e_virtchnl_ops {
CMD_GET_VERSION,
CMD_RESET_VF,
..
}
struct i40e_virtchnl_version_info {
u32 major;
u32 minor;
};
..
So, both PF and VF strictly follow the spec. VF send request with expected
command and
data structures to PF host. PF do sanity check and configure, finally applied
to HW.
As developing the code, it's possible to have multiple version of API occurred
with 'major_verion'
or 'minor_version' changed. So, at the initialization stage, VF and PF will use
a command
'CMD_GET_VERSION' to negotiate what language (what version of API) they should
use
and setup conversation.
So, from my perspective, there is no issue here.
In the meanwhile, we have some test models ongoing to validate combination of
Linux and
DPDK drivers for VF and PF. We'll fully support below 4 cases going forward.
1. DPDK PF + DPDK VF
2. DPDK PF + Linux VF
3. Linux PF + DPDK VF
4. Linux PF + Linux VF (it's not our scope)
After applied this patch, i've done below test without observing compatibility
issue.
1. DPDK PF + DPDK VF (middle of 16.11 and 17.02 code base). PF to support API
1.0 while VF
to support API 1.1/1.0
2. DPDK PF + Linux VF 1.5.14. PF to support 1.0, while Linux to support 1.1/1.0
Linux PF + DPDK VF has been tested with 1.0 API long time ago. There is some
test activities
ongoing.
Finally, please give strong reasons to support your NAC.
Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
Vincent, Sorry, I missed this reply. > > Le 22/12/2016 à 09:10, Chen, Jing D a écrit : > > In the meanwhile, we have some test models ongoing to validate > > combination of Linux and DPDK drivers for VF and PF. We'll fully support > below 4 cases going forward. > > 1. DPDK PF + DPDK VF > > 2. DPDK PF + Linux VF > > + DPDK PF + FreeBSD VF > + DPDK PF + Windows VF > + DPDK PF + OS xyz VF > If all drivers follow same API spec, what's the problem here? What extra DPDK PF effort you observed? > > 3. Linux PF + DPDK VF > > 4. Linux PF + Linux VF (it's not our scope) > > So, you confirm the issue: having DPDK becoming a PF, even if SRIOV protocol > includes version-ing, it doubles the combinatory cases. If extended functions are needed, the answer is yes. That's not a big problem, right? I have several workarounds/approaches to support extended funcs while following original API spec. I can fix it in this release. In order to have a mature solution, I left it here for further implementation. > > > > > After applied this patch, i've done below test without observing > compatibility issue. > > 1. DPDK PF + DPDK VF (middle of 16.11 and 17.02 code base). PF to support > API 1.0 while VF > > to support API 1.1/1.0 > > 2. DPDK PF + Linux VF 1.5.14. PF to support 1.0, while Linux to > > support 1.1/1.0 > > > > Linux PF + DPDK VF has been tested with 1.0 API long time ago. There > > is some test activities ongoing. > > > > Finally, please give strong reasons to support your NAC. > > I feel bad because I do recognize the strong and hard work that you have done > on this PF development, but I feel we need first to assess if DPDK should > become a PF or not. I know ixgbe did open the path and that they are some > historical DPDK PF supports in Intel NICs, but before we generalize it, we > have > to make sure we are not turning this DataPlane development Kit into a > ControlPlane Driver Kit that we are scared to upstream into Linux kernel. Even > if "DPDK is not Linux", it does not mean that Linux should be ignored. In case > of DPDK on other OS, same, their PF could be extended too. > Thanks for the recognition of our work on PF driver. :) > So currently, yes, I do keep a nack't > > Since DPDK PF features can be into Linux PF features too and since Linux (and > other hypervisors) has already some tools to manage PF (see iproute2, etc.), > why should we have an other management path with DPDK? > DPDK is aimed to be a Dataplane Development kit, not a management/control > plane driver kit. Before we debated on Dataplane and ControPlane, can you answer me a question, why we have generic filter API? Is it a API for dataplane? I can't imagine that we'll have to say 'you need to use Linux PF' driver when users want to deploy PF + VF cases. Why we can't provide an alternative option. they are not exclusive and users can decide which combination is better for them. The reason why we developed DPDK PF host driver is we have requirements from users. Our motivation is simple, there are requirements, we satisfy them. Sorry, you NACK can't convince me. > > Assuming you want to use DPDK PF for dataplane feature, that could be OK > then, using: >- configure one VF on the hypervisor from Linux's PF, let's name if > VF_forPFtraffic, see http://dpdk.org/doc/guides/howto/flow_bifurcation.html >- have no (or few IO)s to the PF's queue >- assign the traffic to all VF_forPFtraffic's queues of the hypervisor, >- run DPDK into the hypervisor's VF_forPFtraffic > > Doing so, we get the same benefit of running DPDK over PF or running DPDK > over VF_forPFtraffic, don't we? It is a benefit of: >http://dpdk.org/doc/guides/howto/flow_bifurcation.html > > Thank you, >Vincent >
Re: [dpdk-dev] [PATCH 0/4] enhancement to i40e PF host driver
Hi, Vincent, Since original patch set are split into 2 different patches, can we discuss in this thread? Attach original discussion below. Sorry for top-post. > > Le 22/12/2016 à 09:10, Chen, Jing D a écrit : > > In the meanwhile, we have some test models ongoing to validate > > combination of Linux and DPDK drivers for VF and PF. We'll fully > > support > below 4 cases going forward. > > 1. DPDK PF + DPDK VF > > 2. DPDK PF + Linux VF > > + DPDK PF + FreeBSD VF > + DPDK PF + Windows VF > + DPDK PF + OS xyz VF > If all drivers follow same API spec, what's the problem here? What extra DPDK PF effort you observed? > > 3. Linux PF + DPDK VF > > 4. Linux PF + Linux VF (it's not our scope) > > So, you confirm the issue: having DPDK becoming a PF, even if SRIOV > protocol includes version-ing, it doubles the combinatory cases. If extended functions are needed, the answer is yes. That's not a big problem, right? I have several workarounds/approaches to support extended funcs while following original API spec. I can fix it in this release. In order to have a mature solution, I left it here for further implementation. > > > > > After applied this patch, i've done below test without observing > compatibility issue. > > 1. DPDK PF + DPDK VF (middle of 16.11 and 17.02 code base). PF to > > support > API 1.0 while VF > > to support API 1.1/1.0 > > 2. DPDK PF + Linux VF 1.5.14. PF to support 1.0, while Linux to > > support 1.1/1.0 > > > > Linux PF + DPDK VF has been tested with 1.0 API long time ago. There > > is some test activities ongoing. > > > > Finally, please give strong reasons to support your NAC. > > I feel bad because I do recognize the strong and hard work that you > have done on this PF development, but I feel we need first to assess > if DPDK should become a PF or not. I know ixgbe did open the path and > that they are some historical DPDK PF supports in Intel NICs, but > before we generalize it, we have to make sure we are not turning this > DataPlane development Kit into a ControlPlane Driver Kit that we are > scared to upstream into Linux kernel. Even if "DPDK is not Linux", it > does not mean that Linux should be ignored. In case of DPDK on other OS, > same, their PF could be extended too. > Thanks for the recognition of our work on PF driver. :) > So currently, yes, I do keep a nack't > > Since DPDK PF features can be into Linux PF features too and since > Linux (and other hypervisors) has already some tools to manage PF (see > iproute2, etc.), why should we have an other management path with DPDK? > DPDK is aimed to be a Dataplane Development kit, not a > management/control plane driver kit. Before we debated on Dataplane and ControPlane, can you answer me a question, why we have generic filter API? Is it a API for dataplane? I can't imagine that we'll have to say 'you need to use Linux PF' driver when users want to deploy PF + VF cases. Why we can't provide an alternative option. they are not exclusive and users can decide which combination is better for them. The reason why we developed DPDK PF host driver is we have requirements from users. Our motivation is simple, there are requirements, we satisfy them. Sorry, you NACK can't convince me. > > Assuming you want to use DPDK PF for dataplane feature, that could be > OK then, using: >- configure one VF on the hypervisor from Linux's PF, let's name if > VF_forPFtraffic, see http://dpdk.org/doc/guides/howto/flow_bifurcation.html >- have no (or few IO)s to the PF's queue >- assign the traffic to all VF_forPFtraffic's queues of the hypervisor, >- run DPDK into the hypervisor's VF_forPFtraffic > > Doing so, we get the same benefit of running DPDK over PF or running > DPDK over VF_forPFtraffic, don't we? It is a benefit of: >http://dpdk.org/doc/guides/howto/flow_bifurcation.html > > Thank you, >Vincent > > -Original Message- > From: Vincent JARDIN [mailto:vincent.jar...@6wind.com] > Sent: Friday, December 23, 2016 8:53 PM > To: Chen, Jing D ; dev@dpdk.org > Cc: Yigit, Ferruh ; Wu, Jingjing > > Subject: Re: [PATCH 0/4] enhancement to i40e PF host driver > > Thanks for this update. > > Still, I would like to get good arguments why DPDK should be a PF because it > creates fragmentations. Please, first, let's reply to: >http://dpdk.org/ml/archives/dev/2016-December/053107.html > > Having both Linux and DPDK being PF, it means that we double the > combinations so we double the issues. > > The following should be used instead: assuming you want
Re: [dpdk-dev] [PATCH v3] test: add delay time in test alarm
+ while (flag != 2 && count++ < RTE_TEST_MAX_REPEAT)
+ rte_delay_ms(10);
Why you don't replace "2" and "10" with macro?
-Original Message-
From: Yang, Qiming
Sent: Tuesday, June 20, 2017 11:24 AM
To: dev@dpdk.org
Cc: Chen, Jing D ; Wu, Jingjing ;
Yang, Qiming
Subject: [PATCH v3] test: add delay time in test alarm
Because accuracy of timing to the microsecond is not guaranteed in
rte_eal_alarm_set, this function will not be called before the requested time,
but may be called a period of time afterwards which can not be calculated. In
order to ensure test alarm running success, this patch added the delay time
before check the flag.
Signed-off-by: Qiming Yang
---
v2 changes:
* fixed coding style problems
v3 changes:
* replaced the numeric by macro
---
---
test/test/test_alarm.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/test/test/test_alarm.c b/test/test/test_alarm.c index
ecb2f6d..40f55b5 100644
--- a/test/test/test_alarm.c
+++ b/test/test/test_alarm.c
@@ -47,6 +47,7 @@
#define RTE_TEST_ALARM_TIMEOUT 10 /* ms */
#define RTE_TEST_CHECK_PERIOD 3 /* ms */
+#define RTE_TEST_MAX_REPEAT20
static volatile int flag;
@@ -96,6 +97,7 @@ static int
test_multi_alarms(void)
{
int rm_count = 0;
+ int count = 0;
cb_count.cnt = 0;
printf("Expect 6 callbacks in order...\n"); @@ -169,7 +171,10 @@
test_multi_alarms(void)
printf("Error, cancelling head-of-list leads to premature
callback\n");
return -1;
}
- rte_delay_ms(10);
+
+ while (flag != 2 && count++ < RTE_TEST_MAX_REPEAT)
+ rte_delay_ms(10);
+
if (flag != 2) {
printf("Error - expected callback not called\n");
rte_eal_alarm_cancel(test_remove_in_callback, (void *)-1); @@
-212,7 +217,7 @@ test_alarm(void)
printf("fail to set alarm callback\n");
return -1;
}
- while (flag == 0 && count ++ < 6)
+ while (flag == 0 && count++ < RTE_TEST_MAX_REPEAT)
rte_delay_ms(RTE_TEST_CHECK_PERIOD);
if (flag == 0){
--
2.7.4
Re: [dpdk-dev] [PATCH v3] test: add delay time in test alarm
> >
> > -Original Message-
> > From: Yang, Qiming
> > Sent: Tuesday, June 20, 2017 11:24 AM
> > To: dev@dpdk.org
> > Cc: Chen, Jing D ; Wu, Jingjing
> > ; Yang, Qiming
> > Subject: [PATCH v3] test: add delay time in test alarm
> >
> > Because accuracy of timing to the microsecond is not guaranteed in
> > rte_eal_alarm_set, this function will not be called before the
> > requested time, but may be called a period of time afterwards which
> > can not be calculated. In order to ensure test alarm running success,
> > this patch added the delay time before check the flag.
> >
> > Signed-off-by: Qiming Yang
> > ---
> > v2 changes:
> > * fixed coding style problems
> > v3 changes:
> > * replaced the numeric by macro
> > ---
> > ---
> > test/test/test_alarm.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/test/test/test_alarm.c b/test/test/test_alarm.c index
> > ecb2f6d..40f55b5 100644
> > --- a/test/test/test_alarm.c
> > +++ b/test/test/test_alarm.c
> > @@ -47,6 +47,7 @@
> >
> > #define RTE_TEST_ALARM_TIMEOUT 10 /* ms */
> > #define RTE_TEST_CHECK_PERIOD 3 /* ms */
> > +#define RTE_TEST_MAX_REPEAT20
> >
> > static volatile int flag;
> >
> > @@ -96,6 +97,7 @@ static int
> > test_multi_alarms(void)
> > {
> > int rm_count = 0;
> > + int count = 0;
> > cb_count.cnt = 0;
> >
> > printf("Expect 6 callbacks in order...\n"); @@ -169,7 +171,10 @@
> > test_multi_alarms(void)
> > printf("Error, cancelling head-of-list leads to premature
> > callback\n");
> > return -1;
> > }
> > - rte_delay_ms(10);
> > +
> > + while (flag != 2 && count++ < RTE_TEST_MAX_REPEAT)
> > + rte_delay_ms(10);
> > +
> > if (flag != 2) {
> > printf("Error - expected callback not called\n");
> > rte_eal_alarm_cancel(test_remove_in_callback, (void *)-1);
> @@
> > -212,7 +217,7 @@ test_alarm(void)
> > printf("fail to set alarm callback\n");
> > return -1;
> > }
> > - while (flag == 0 && count ++ < 6)
> > + while (flag == 0 && count++ < RTE_TEST_MAX_REPEAT)
> > rte_delay_ms(RTE_TEST_CHECK_PERIOD);
> >
> > if (flag == 0){
> > --
> > 2.7.4
Acked-by : Jing Chen
Re: [dpdk-dev] [PATCH] net/fm10k: fix secondary process crash
> -Original Message-
> From: Wang, Xiao W
> Sent: Tuesday, March 28, 2017 11:59 AM
> To: Chen, Jing D
> Cc: dev@dpdk.org; Wang, Xiao W ; sta...@dpdk.org
> Subject: [PATCH] net/fm10k: fix secondary process crash
>
> If the primary process has initialized all the queues to vector pmd mode, the
> secondary process should not use scalar code path, because the per queue data
> structures haven't been prepared for that, e.g. txq->ops is for vector Tx
> rather
> than scalar Tx.
>
> Fixes: a6ce64a97520 ("fm10k: introduce vector driver")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiao Wang
> ---
> drivers/net/fm10k/fm10k_ethdev.c | 28 ++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 388f929..680d617 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -2750,6 +2750,21 @@ static void __attribute__((cold))
> int use_sse = 1;
> uint16_t tx_ftag_en = 0;
>
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> + /* primary process has set the ftag flag and txq_flags */
> + txq = dev->data->tx_queues[0];
> + if (fm10k_tx_vec_condition_check(txq)) {
> + dev->tx_pkt_burst = fm10k_xmit_pkts;
> + dev->tx_pkt_prepare = fm10k_prep_pkts;
> + PMD_INIT_LOG(DEBUG, "Use regular Tx func");
> + } else {
> + PMD_INIT_LOG(DEBUG, "Use vector Tx func");
> + dev->tx_pkt_burst = fm10k_xmit_pkts_vec;
> + dev->tx_pkt_prepare = NULL;
> + }
> + return;
> + }
> +
Why we need to check process type? What would happen if no changes made here?
> if (fm10k_check_ftag(dev->device->devargs))
> tx_ftag_en = 1;
>
> @@ -2810,6 +2825,9 @@ static void __attribute__((cold))
> else
> PMD_INIT_LOG(DEBUG, "Use regular Rx func");
>
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + return;
> +
> for (i = 0; i < dev->data->nb_rx_queues; i++) {
> struct fm10k_rx_queue *rxq = dev->data->rx_queues[i];
>
> @@ -2856,9 +2874,15 @@ static void __attribute__((cold))
> dev->tx_pkt_burst = &fm10k_xmit_pkts;
> dev->tx_pkt_prepare = &fm10k_prep_pkts;
>
> - /* only initialize in the primary process */
> - if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + /*
> + * Primary process does the whole initialization, for secondary
> + * processes, we just select the same Rx and Tx function as primary.
> + */
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> + fm10k_set_rx_function(dev);
> + fm10k_set_tx_function(dev);
> return 0;
> + }
>
> rte_eth_copy_pci_info(dev, pdev);
> dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
> --
> 1.8.3.1
Re: [dpdk-dev] [PATCH] net/fm10k: fix secondary process crash
> -Original Message-
> From: Wang, Xiao W
> Sent: Tuesday, March 28, 2017 11:59 AM
> To: Chen, Jing D
> Cc: dev@dpdk.org; Wang, Xiao W ;
> sta...@dpdk.org
> Subject: [PATCH] net/fm10k: fix secondary process crash
>
> If the primary process has initialized all the queues to vector pmd mode, the
> secondary process should not use scalar code path, because the per queue
> data structures haven't been prepared for that, e.g. txq->ops is for vector Tx
> rather than scalar Tx.
>
> Fixes: a6ce64a97520 ("fm10k: introduce vector driver")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiao Wang
Acked-by : Jing Chen
Re: [dpdk-dev] [PATCH 6/6] doc: introduction to prgdev
Hi, Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] > Sent: Monday, March 6, 2017 11:27 PM > To: Chen, Jing D > Cc: dev@dpdk.org; Liang, Cunming ; Rogers, > Gerald ; Wiles, Keith ; > Richardson, Bruce ; Mcnamara, John > > Subject: Re: [dpdk-dev] [PATCH 6/6] doc: introduction to prgdev > > 2017-03-02 12:03, Chen Jing D: > > +Overview > > + > > I think the first review pass of this series must be focused on the overview > you give here (thanks for the detailed explanations). > > I'll try to sum up while commenting. > > [...] > The target is programmable devices. > > > +The major purpose of prgdev is to help DPDK users to load/upgrade RTL > > +images for FPGA devices, or upgrade firmware for programmble NICs. > > This is a control plane feature. > You want to have it in DPDK to allow dynamic programmation while running > the device, right? Exactly right. > > [...] > > +When the set of APIs is introduced, a general question is why we need > > +it in DPDK community? > > Good question :) > > [...] > > +Any devices, having the capability to store/load a piece of info > > +to/from the deivce then changed hardware behavior, and applicable to > > +prgdev programming model, could be registered as a prgdev device. > > + > > +The device can be programmed (dynamic) within DPDK or have been prior > > +programmed > > +(static) prior to a DPDK application being launched. > [...] > > +Besides the simple API to upload/download image, the prgdev also > > +introduces a mechanism internally to switch drivers and > > +register/unregister device on the fly. With this mechanism, apps can > > +use the programmable device, unbind other personalities on demand, > then program it and bind it back with new personalities. > > +Apps can follow below examples to simply complete the whole process. > > I disagree about the specific bind/unbind for prgdev. > We must improve binding inside DPDK for every devices. First of all, let us try to imagine what is the safe status for device before apps intending to download some image to it - apps wouldn't operate on the device, any behaviors like configuring registers, receive/transmit data may impair the device or make the download failed. Following first answer to prevent app accessing device during image downloading, how can we achieve that? Detach drivers with device is a smart idea, right? But the problem is how can apps use prgdev API to download image after all drivers detached with the device? So, the final question is how can we just detached others driver except prgdev one? I can't find answer in current DPDK framework, that's why I'd like to introduce bind/unbind functions to detach other drivers from the device. I'm open to this problem. If any suggestion or mechanism can help on it, I can remove these 2. BTW, not all devices or image download actions need the device to detach the device, depending on hardware implementation. > > > +Note that bind/unbind actions are different concept from that a whole > > +device attach/detach. For example, ``rte_eal_dev_detach()``, which > > +will try to detach the drivers with device and remove the whole > > +device from specific class of devices (ethdev, for example). Then, the > whole device is invisible until a new 'probe' > > +is activated. > > I do not understand. See above explanations. > > > +During the whole procedure of image upload/download, prgdev handler > > +is always valid and apps can use it operate on programmable device. > > +The reason why unbind is necessary is it may break the hardware when > > +programming is in progress while other functions are active. Using > > +programmble NIC as an example, it's a little impossible to > > +receive/forward packets for hardware while updating firmware. In this > > +case, apps need to detach ethdev function before firmware upgrading. > > +Again, prgdev handler is still valid, it's function-level detach, > > +different from device-level detach. After finishing image download, > > +original function needs to attach back, either use same or different > > +drivers, depends on personalities changed or not. For a programmble NIC, > the driver may be the same. For FPGA, it may not, see below examples to get > more details. > > If the personality of the device is changed, it must be seen as a new device > from e.g. the ethdev point of view, while keeping the same prgdev device. > In other words, a device can have several interfaces at the same time (ethdev, > cryptodev, eventdev, prgdev, whatever). > I think we can dynamicall
Re: [dpdk-dev] [PATCH 6/6] doc: introduction to prgdev
> > > > > > > +Another reason to provide bind/unbind action is programmble > > > > +devices, like FPGA, are not identified driver by 'vendor ID' and > > > > +'device ID', they might not be changed in all the ways, even FPGA > > > > +is fully programmed. So, it depends on internal mechanism of > > > > +FPGA, not 'vendor ID' and 'device ID' to identify proper drivers, > > > > +either a register value or signature, depending on FPGA hardware > > > > +design. In this case, EAL or other bus driver doesn't have the > > > > +capability to identify proper driver for FPGA device. With prgdev > > > > +introduced, while FPGA is always a prgdev, FPGA can use prgdev as > > > > +primary driver, to find proper > > > function driver. > > > > > > You mean prgdev should help the bus layer to map the right driver > interface? > > > It looks weird and dangerous. The standard way to identify a PCI > > > device is to look at its IDs. Other unknown methods must be strongly > discussed. > > > > For programmable Ethernet device, it's not truce. But for FPGA, it's. > > When FPGA is produced, the device ID indicate what model it is and > > won't be changed anyway, even being reprogrammed. It used some > > not-generic mechanism, like AFU id to distinguish the personalities. > > So, for FPGA, a prgdev driver can be used as primary driver to identify > personalities and then register to specific devices. > > Sounds like we would need an FPGA bus driver in that case. I think that would > be a better solution than having a specific device driver loading other > drivers. > I don't object to introduce a pseudo bus for FPGA, but it's a matter of work that FPGA driver needs to consider, not in scope of prgdev. Besides that, I can see DPDK EAL will do other bus probe first, then PCI bus probe, which is not friendly to introduce pseudo bus for an actual PCI device. > /Bruce
Re: [dpdk-dev] i40e SR-IOV with Linux PF
>
> +Cc dev@dpdk.org
>
> 2017-03-13 15:29, Thomas Monjalon:
> > Hi i40e developers,
> >
> > Referring to the VFD discussion, I thought basic behaviours were the
> > same regardless of the PF driver:
> > http://dpdk.org/ml/archives/dev/2016-December/053056.html
> > "
> > > In the meanwhile, we have some test models ongoing to validate
> > > combination of Linux and DPDK drivers for VF and PF.
> > > We'll fully support below 4 cases going forward.
> > > 1. DPDK PF + DPDK VF
> > > 2. DPDK PF + Linux VF
> > > 3. Linux PF + DPDK VF
> > > 4. Linux PF + Linux VF (it's not our scope)
> > [...]
> > > Linux PF + DPDK VF has been tested with 1.0 API long time ago.
> > > There is some test activities ongoing.
> > "
> >
> > I think the Linux PF case is important and deserves more consideration.
> > When looking at the code, specifically i40evf_vlan_offload_set() and
> > i40evf_vlan_pvid_set(), I read this:
> > "
> > /* Linux pf host doesn't support vlan offload yet */
> > if (vf->version_major == I40E_DPDK_VERSION_MAJOR) { "
> >
> > Is there some work in progress on Linux side to get the same behaviour
> > as with a DPDK PF?
> >
> > At least, it must be documented in
> > doc/guides/nics/features/i40e_vf.ini
> > and marked as partially supported (P instead of Y) in
> > doc/guides/nics/i40e.rst
Thanks for pointing it out. We'll sync our code with latest kernel driver and
document and comment properly soon.
[dpdk-dev] [PATCH 0/3] net: fix out of order rx read issue
Hi, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Qi Zhang > Sent: Monday, October 17, 2016 1:24 AM > To: Wu, Jingjing ; Zhang, Helin > > Cc: dev at dpdk.org; Zhang, Qi Z > Subject: [dpdk-dev] [PATCH 0/3] net: fix out of order rx read issue > > Volatile point has been cast to non-volatile point when call _mm_loadu_si128, > so add compile barrier to prevent compiler reorder. > > Qi Zhang (3): > net/i40e: fix out of order rx read issue > net/ixgbe: fix out of order rx read issue > net/fm10k: fix out of order rx read issue > > drivers/net/fm10k/fm10k_rxtx_vec.c | 3 +++ > drivers/net/i40e/i40e_rxtx_vec.c | 3 +++ > drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 3 +++ > 3 files changed, 9 insertions(+) I have an overall comment on the committed message. You'd better to describe why we have out of order issue here and the requirement on the execution order of the 4 loads.
[dpdk-dev] [PATCH v2 0/5] implement new Rx checksum flag
Hi, > -Original Message- > From: Wang, Xiao W > Sent: Tuesday, September 06, 2016 9:27 AM > To: dev at dpdk.org > Cc: Chen, Jing D ; olivier.matz at 6wind.com; Wang, > Xiao W > > Subject: [PATCH v2 0/5] implement new Rx checksum flag > > v2: > * Removed hw_ip_checksum check in fm10k_rx_vec_condition_check(). > > * Defined CKSUM_SHIFT for SSE bits shift. > > * Changed commit title from "add back Rx checksum offload" to > "fix Rx checksum flags". > > * Added new cksum flag support for ixgbe vector Rx, based on patch > (http://dpdk.org/dev/patchwork/patch/14630/) which came earlier. > > v1: > Following http://dpdk.org/dev/patchwork/patch/14941/, this patch set > implements newly defined Rx checksum flag for igb, ixgbe, i40e and fm10k. > > Currently, ixgbe and fm10k support Rx checksum offload in both scalar > and vector datapath, while the other two don't, this patch set keeps > this situation. > > Note: This patch set has dependency on the following patches: > > "mbuf: add new Rx checksum mbuf flags" > (http://dpdk.org/dev/patchwork/patch/14941/) > > "ixgbe: support checksum flags in sse vector Rx function" > (http://dpdk.org/dev/patchwork/patch/14630/) > > Xiao Wang (5): > net/fm10k: fix Rx checksum flags > net/fm10k: implement new Rx checksum flag > net/e1000: implement new Rx checksum flag > net/ixgbe: implement new Rx checksum flag > net/i40e: implement new Rx checksum flag > > drivers/net/e1000/igb_rxtx.c | 4 +++- > drivers/net/fm10k/fm10k_rxtx.c | 14 ++ > drivers/net/fm10k/fm10k_rxtx_vec.c | 24 +--- > drivers/net/i40e/i40e_rxtx.c | 6 ++ > drivers/net/ixgbe/ixgbe_rxtx.c | 4 +++- > drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 30 -- > 6 files changed, 67 insertions(+), 15 deletions(-) Acked-by : Jing Chen
[dpdk-dev] [PATCH v3] net: fix build error with clang
Hi,
> -Original Message-
> From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> Sent: Tuesday, September 27, 2016 5:20 AM
> To: dev at dpdk.org
> Cc: Jerin Jacob ; Chen, Jing D
> ; Liang, Cunming ;
> Richardson, Bruce ; Thomas Monjalon
>
> Subject: Re: [PATCH v3] net: fix build error with clang
>
> Can any PMD guys review it? It blocks a virtio patchset apply...
>
> Thanks.
>
> --yliu
>
> On Mon, Sep 26, 2016 at 12:29:13PM +0800, Yuanhan Liu wrote:
> > Interestingly, clang and gcc has different prototype for _mm_prefetch().
> > For gcc, we have
> >
> >_mm_prefetch (const void *__P, enum _mm_hint __I)
> >
> > While for clang, it's
> >
> >#define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0,
> > (sel)))
> >
> > That's how the following error comes with clang:
> >
> >error: cast from 'const void *' to 'void *' drops const qualifier
> >[-Werror,-Wcast-qual]
> >_mm_prefetch((const void *)rused, _MM_HINT_T0);
> >/usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
> >note: expanded from macro '_mm_prefetch'
> > #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
> > 0, (sel)))
> >
> > What's weird is that the build was actaully Okay before. I met it
> > while apply Jerin's vector support for ARM patch set: he just move
> > this peiece of code to another file, nothing else changed.
> >
> > This patch fix the issue when Jerin's patchset is applied. Thus, I
> > think it's still needed.
> >
> > Similarly, make the same change to other _mm_prefetch users, just in
> > case this weird issue shows up again somehow later.
> >
> > Fixes: fc3d66212fed ("virtio: add vector Rx")
> > Fixes: c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
> > Fixes: 9ed94e5bb04e ("i40e: add vector Rx")
> > Fixes: 7092be8437bd ("fm10k: add vector Rx")
> >
> > Cc: Jerin Jacob
> > Cc: Chen Jing D(Mark)
> > Cc: Cunming Liang
> > Cc: Bruce Richardson
> > CC: Thomas Monjalon
> > Signed-off-by: Yuanhan Liu
Acked-by: Jing Chen
Re: [dpdk-dev] [PATCH] net/fm10k: initialize link status in device start
Hi,
> -Original Message-
> From: Wang, Xiao W
> Sent: Wednesday, May 31, 2017 7:07 PM
> To: Chen, Jing D
> Cc: dev@dpdk.org; Wang, Xiao W ;
> sta...@dpdk.org
> Subject: [PATCH] net/fm10k: initialize link status in device start
>
> If port LSC interrupt is configured, application will read link status
> directly, so
> driver need to prepare that value in advance.
Fm10k host driver can't manage PHY directly and provide a fake link status,
so it always provide a constant value, whatever lsc is set or not.
I think you need to reorganize the message. :)
>
> Fixes: 9ae6068c86da ("fm10k: add dev start/stop")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiao Wang
> ---
> drivers/net/fm10k/fm10k_ethdev.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index a742eec..54bf10c 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -84,6 +84,8 @@ static void fm10k_MAC_filter_set(struct rte_eth_dev
> *dev, static void fm10k_set_rx_function(struct rte_eth_dev *dev); static
> void fm10k_set_tx_function(struct rte_eth_dev *dev); static int
> fm10k_check_ftag(struct rte_devargs *devargs);
> +static int fm10k_link_update(struct rte_eth_dev *dev,
> + __rte_unused int wait_to_complete);
>
> struct fm10k_xstats_name_off {
> char name[RTE_ETH_XSTATS_NAME_SIZE];
> @@ -1166,6 +1168,9 @@ static inline int fm10k_glort_valid(struct fm10k_hw
> *hw)
> if (!(dev->data->dev_conf.rxmode.mq_mode &
> ETH_MQ_RX_VMDQ_FLAG))
> fm10k_vlan_filter_set(dev, hw->mac.default_vid, true);
>
> + if (dev->data->dev_conf.intr_conf.lsc != 0)
> + fm10k_link_update(dev, 0);
> +
I'll recommend updating link status anyway when port starts, not considering
lsc set status.
> return 0;
> }
>
> --
> 1.8.3.1
Re: [dpdk-dev] [PATCH v2] net/fm10k: initialize link status in device start
> -Original Message-
> From: Wang, Xiao W
> Sent: Thursday, June 22, 2017 7:20 PM
> To: Chen, Jing D
> Cc: dev@dpdk.org; sta...@dpdk.org; Wang, Xiao W
> Subject: [PATCH v2] net/fm10k: initialize link status in device start
>
> Fm10k host driver can't manage PHY directly and provides a fake link status by
> always reporting LINK_UP. We should initialize link status in device start,
> otherwise application will get LINK_DOWN status when LSC configured.
>
> Fixes: 9ae6068c86da ("fm10k: add dev start/stop")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiao Wang
> ---
> v2:
> * Rewrite commit message, add information about fm10k PHY.
> * Always do link_update in dev_start.
> ---
> drivers/net/fm10k/fm10k_ethdev.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 7172a0f..c5c4712 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -84,6 +84,8 @@ static void fm10k_MAC_filter_set(struct rte_eth_dev
> *dev, static void fm10k_set_rx_function(struct rte_eth_dev *dev); static
> void
> fm10k_set_tx_function(struct rte_eth_dev *dev); static int
> fm10k_check_ftag(struct rte_devargs *devargs);
> +static int fm10k_link_update(struct rte_eth_dev *dev,
> + __rte_unused int wait_to_complete);
>
> struct fm10k_xstats_name_off {
> char name[RTE_ETH_XSTATS_NAME_SIZE];
> @@ -1166,6 +1168,8 @@ static inline int fm10k_glort_valid(struct fm10k_hw
> *hw)
> if (!(dev->data->dev_conf.rxmode.mq_mode &
> ETH_MQ_RX_VMDQ_FLAG))
> fm10k_vlan_filter_set(dev, hw->mac.default_vid, true);
>
> + fm10k_link_update(dev, 0);
> +
> return 0;
> }
>
Acked-by : Jing Chen
Re: [dpdk-dev] [PATCH v2] test: add delay time in test alarm
Hi,
> diff --git a/test/test/test_alarm.c b/test/test/test_alarm.c index
> ecb2f6d..cbae1a0 100644
> --- a/test/test/test_alarm.c
> +++ b/test/test/test_alarm.c
> @@ -96,6 +96,7 @@ static int
> test_multi_alarms(void)
> {
> int rm_count = 0;
> + int count = 0;
> cb_count.cnt = 0;
>
> printf("Expect 6 callbacks in order...\n"); @@ -169,7 +170,10 @@
> test_multi_alarms(void)
> printf("Error, cancelling head-of-list leads to premature
> callback\n");
> return -1;
> }
> - rte_delay_ms(10);
> +
> + while (flag != 2 && count++ < 6)
> + rte_delay_ms(10);
> +
> if (flag != 2) {
> printf("Error - expected callback not called\n");
> rte_eal_alarm_cancel(test_remove_in_callback, (void *)-1);
> @@ -212,7 +216,7 @@ test_alarm(void)
> printf("fail to set alarm callback\n");
> return -1;
> }
> - while (flag == 0 && count ++ < 6)
> + while (flag == 0 && count++ < 20)
> rte_delay_ms(RTE_TEST_CHECK_PERIOD);
>
What's the criteria to delay 20* RTE_TEST_CHECK_PERIOD ms? Add more comments?
> if (flag == 0){
> --
> 2.7.4
Overall comment is to replace numeric with macro.
Re: [dpdk-dev] SIMD Rx/Tx paths
> 15/05/2017 16:12, Richardson, Bruce: > > From: Yigit, Ferruh > > > On 5/15/2017 2:15 PM, Bruce Richardson wrote: > > > > On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote: > > > >> Hi, > > > >> > > > >> I would like to open a discussion about SIMD code in drivers. > > > >> > > > >> I think we should not have different behaviours or features > > > >> capabilities, in the different code paths of a same driver. > > > >> I suggest to consider such differences as exceptions. > > > >> So we should merge features files (i.e. matrix columns), and > > > >> remove these files: > > > >> > > > >> % ls doc/guides/nics/features/*_vec.ini > > > >> > > > >> doc/guides/nics/features/fm10k_vec.ini > > > >> doc/guides/nics/features/fm10k_vf_vec.ini > > > >> doc/guides/nics/features/i40e_vec.ini > > > >> doc/guides/nics/features/i40e_vf_vec.ini > > > >> doc/guides/nics/features/ixgbe_vec.ini > > > >> doc/guides/nics/features/ixgbe_vf_vec.ini > > > >> doc/guides/nics/features/virtio_vec.ini > > > >> > > > >> If a feature is not supported in all code paths of a driver, it > > > >> must be marked as partially (P) supported. > > > >> > > > >> Then the mid-term goal will be to try removing these inconsistencies. > > > >> > > > >> Opinions/comments? > > > > > > > > Yes, there are inconsistencies, but if they are hidden from the > > > > user, e.g. by having the driver select automatically the most > > > > appropriate path, I don't think we should need to mark the support as > "partial". > > > > Instead, it should be marked as being fully supported, but perhaps > > > > with a note indicating that a performance hit may be experienced > > > > due to the code taking a less-optimised driver path. After all, > > > > especially in the TX code path, a lot of the speed-up comes from > > > > not supporting different features, as well as from the > > > > vectorization. In those cases "closing the gap" may mean losing > > > > performance for those who don't want the features, which is not > > > > acceptable. Any feature support we can add, without affecting > performance, should of course be implemented. > > > > > > I mostly agree with Bruce, except for PMD selection the patch > > > automatically. > > > > > > There is a trade off between feature set and performance, scalar > > > driver favors features and vector driver favors performance, I think > > > good to have both. > > > > > > And I agree that feature support should be added to vector drivers > > > as long as it doesn't effect the performance. > > > > > > Related to the driver auto selecting the path, I concern this may > > > confuse the user, because he may end up a situation he doesn't clear > > > about supported features, I am for more explicit way to select the > > > scalar or vector driver. > > > > > > And for merging the features files, most of the items are already > > > same for scalar and vector drivers, so perhaps we can merge files > > > and use different syntax for features that is different for scalar and > > > vector: > > > Ys: Yes Scalar [no vector] > > > Yv: Yes Vector [no scalar] > > > Y: Yes Both > > > Ps: Partially Scalar [no vector] > > > Pv: Partially Vector [no scalar] > > > P: Partially Both > > > YsPv, YvPs > > Please remember that there are different vector code paths (SSE/AVX, NEON, > Altivec). > > > For the table, I don't really mind so much what scheme is agreed. For the > user doing the coding, while I can accept that it might be useful to support > explicitly request a vector or scalar driver, I'd definitely prefer the > default > state to remain auto-selection based on features requested. If a user want > TSO, then pick the best driver path that supports TSO, and don't force the > user to read up on what the different paths are! > > I agree. > If we can be sure what the application needs, we can select the best code > path and mark the feature supported. > But can we be sure of the expectations for every features? > How do we know that the application relies on certain packet types (which > are not recognized by some code paths)? I also agree auto-selection on tx/rx func. User needn't worry about how PMD to satisfy its' requirement, result is more important. Besides that, we should do more work in rx/tx configuration to help PMD better decide the best rx/tx. Pkt_type is a good example. A possible way is to expose all possible PMD offload features into structure rte_eth_rxmode and rte_eth_txmode or a new structure.
[dpdk-dev] [PATCH] doc: add fm10k driver
Hi, John, Best Regards, Mark > -Original Message- > From: Mcnamara, John > Sent: Wednesday, December 09, 2015 10:21 PM > To: Chen, Jing D; dev at dpdk.org > Subject: RE: [PATCH] doc: add fm10k driver > > > -Original Message----- > > From: Chen, Jing D > > Sent: Wednesday, December 9, 2015 8:25 AM > > To: dev at dpdk.org > > Cc: Mcnamara, John; Chen, Jing D > > Subject: [PATCH] doc: add fm10k driver > > > > From: "Chen Jing D(Mark)" > > > > This documentation covers introdutions and limitations on Intel > > FM1 series products. > > Hi Mark, > > Thanks for that. > > The docs build cleanly but there is one whitespace warning on merge. > > Minor comments below. > > > > > @@ -0,0 +1,54 @@ > > +.. BSD LICENSE > > +Copyright(c) 2010-2015 Intel Corporation. All rights reserved. > > The year should be 2015 only and this line shouldn't be indented in relation > to the next lines. > > > > > +FM10K Poll Mode Driver > > +== > > +The FM10K poll mode driver library provides support for Intel FM1 > > +family of 40GbE/100GbE adapters. > > The DPDK Documentation Guidelines say to leave a blank line after section > headers (here and with the other sections): > > > +FM10K Poll Mode Driver > > +== > > > > +The FM10K poll mode driver library provides support for Intel FM1 > > +family of 40GbE/100GbE adapters. > > See: http://dpdk.org/doc/guides/contributing/documentation.html#rst- > guidelines > > > > +The FM10K poll mode driver library provides support for Intel FM1 > > +family of 40GbE/100GbE adapters. > > Might be worth introducing the common FM10K name here as well: > > The FM10K poll mode driver library provides support for the Intel FM1 > (FM10K) family of 40GbE/100GbE adapters. > > > +Intel FM1 family of NICs integrate an hardware switch and multiple > > +host interfaces. FM10K PMD driver only manages host interfaces. For the > > The doc uses FM1 in some places and FM10K in others. It should use one > or the other consistently. > > > > > +switch component, another switch driver has to be loaded prior to FM10K > > PMD driver. > > +The switch driver either can be acquired by Intel support or from below > > link: > > +https://github.com/match-interface > > Better to add an actual link like: > > The switch driver can be acquired for Intel support or from the > `Match Interface <https://github.com/match-interface>`_ project. > > Also should that be to: https://github.com/match-interface/match > > > > > + > > +CRC strip > > +FM1 family always strip CRC for every packets coming into host > > interface. > > > Limitations > --- > > Switch manager > ~~ > > The Intel FM1 family of NICs integrate a hardware switch and multiple > host > > Etc. > > > +Max packet length > > +FM1 family support maximum of 15K jumbo frame. The value is fixed > > +and can't be changed. So, even (struct > > +rte_eth_conf).rxmode.max_rx_pkt_len is set to a value other than 15364, > > the frames with 15364 byte still can reach to host interface. > > This isn't clear (to me). Maybe something like: > > The FM1 family of NICS support a maximum of a 15K jumbo frame. The > value > is fixed and cannot be changed. So, even when the > ``rxmode.max_rx_pkt_len`` > member of ``struct rte_eth_conf`` is set to a value lower than 15364, frames > up to 15364 bytes can still reach the host interface. > > > Regards, > > John. > -- Many thanks for the comments, I'll change accordingly.
[dpdk-dev] [PATCH 00/18] lib/librte_pmd_fm10k : fm10k pmd driver
Hi, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > Sent: Saturday, January 31, 2015 6:19 AM > To: Shaw, Jeffrey B > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 00/18] lib/librte_pmd_fm10k : fm10k pmd > driver > > 2015-01-30 13:46, Jeff Shaw: > > On Fri, Jan 30, 2015 at 04:26:33PM -0500, Neil Horman wrote: > > > On Fri, Jan 30, 2015 at 01:07:16PM +0800, Chen Jing D(Mark) wrote: > > > > From: "Chen Jing D(Mark)" > > > > Jeff Shaw (18): > > > > fm10k: add base driver > [...] > > > > lib/librte_pmd_fm10k/SHARED/fm10k_api.c | 327 > [...] > > > > > > Why is there a SHARED directory in the driver? Are there other drivers > that use > > > the shared fm10k code? > > > > No, the other poll-mode drivers do not use the shared fm10k code. The > > directory is similar to the 'ixgbe' and 'i40e' directories in their > > respective PMDs, only that it is named 'SHARED' for the fm10k driver. > > So shared is a bad name in the context of DPDK. > Inside Intel, it can be understood that you share it between projects, > but in DPDK, it's only a base driver. > OK, I'll change "SHARED" to "fm10k". > -- > Thomas
[dpdk-dev] [PATCH 03/18] fm10k: Add empty fm10k files
Hi Neil, > -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Saturday, January 31, 2015 10:02 PM > To: Chen, Jing D > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 03/18] fm10k: Add empty fm10k files > > On Fri, Jan 30, 2015 at 01:07:19PM +0800, Chen Jing D(Mark) wrote: > > From: Jeff Shaw > > > > Define macros and basic data structure. > > Define rte_log wrapper functions. > > > > Signed-off-by: Jeff Shaw > > Signed-off-by: Chen Jing D(Mark) > > --- > > lib/librte_pmd_fm10k/Makefile | 96 > > lib/librte_pmd_fm10k/fm10k.h | 224 > + > > lib/librte_pmd_fm10k/fm10k_logs.h | 66 +++ > > 3 files changed, 386 insertions(+), 0 deletions(-) > > create mode 100644 lib/librte_pmd_fm10k/Makefile > > create mode 100644 lib/librte_pmd_fm10k/fm10k.h > > create mode 100644 lib/librte_pmd_fm10k/fm10k_ethdev.c > > create mode 100644 lib/librte_pmd_fm10k/fm10k_logs.h > > create mode 100644 lib/librte_pmd_fm10k/fm10k_rxtx.c > > > Why are you adding empty files? The 2 ".c" files are empty while the 2 ".h" files include code. "Makefile" includes rules to compile the ".c" files, I don't like to break the compile for every single patch, that's why the 2 ".c" files are added in this patch. > > Neil Thanks for your comments. Mark
[dpdk-dev] [PATCH 04/18] fm10k: add fm10k device id
Hi Neil, > -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Saturday, January 31, 2015 10:20 PM > To: Chen, Jing D > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 04/18] fm10k: add fm10k device id > > On Fri, Jan 30, 2015 at 01:07:20PM +0800, Chen Jing D(Mark) wrote: > > From: Jeff Shaw > > > > Add fm10k device ID list into rte_pci_dev_ids.h. > > > > Signed-off-by: Jeff Shaw > > Signed-off-by: Chen Jing D(Mark) > > --- > > lib/librte_eal/common/include/rte_pci_dev_ids.h | 22 > ++ > > 1 files changed, 22 insertions(+), 0 deletions(-) > > > > diff --git a/lib/librte_eal/common/include/rte_pci_dev_ids.h > b/lib/librte_eal/common/include/rte_pci_dev_ids.h > > index c922de9..f54800e 100644 > > --- a/lib/librte_eal/common/include/rte_pci_dev_ids.h > > +++ b/lib/librte_eal/common/include/rte_pci_dev_ids.h > > @@ -132,6 +132,14 @@ > > #define RTE_PCI_DEV_ID_DECL_VMXNET3(vend, dev) > > #endif > > > > +#ifndef RTE_PCI_DEV_ID_DECL_FM10K > > +#define RTE_PCI_DEV_ID_DECL_FM10K(vend, dev) > > +#endif > > + > > +#ifndef RTE_PCI_DEV_ID_DECL_FM10KVF > > +#define RTE_PCI_DEV_ID_DECL_FM10KVF(vend, dev) > > +#endif > > + > I know this isn't the job of this patch series, but I don't really understand > why we bother with this pattern for filling out pci id tables. A PMD supports > specific hardware, we might as well use the generic RTE_PCI_DEVICE macro > in the > driver rather than creating a FM10K specific wrapper, only to have to do > some > ifdef trickery in the rte_cpi_dev_ids file and some include magic to fill it > out. > > I'd suggest that you just use RTE_PCI_DEVICE macro here, and make your > own table > (keep the specific device id values in the common file. Then we can clean out > the macro maggic in a later update. I partially agree with you. Maybe a better solution is to use the mechanism that applied in kernel to register PCI driver. Driver maintains a device list that it can manage and provide a hook function to detect if new device can be managed by the driver. Then, DPDK core library needn't worry about the long device list (Maybe script that unbind/bind device needs such info, it's another story). But I'd like to keep current implementation unchanged since final decision is not made yet. If new mechanism is introduced, I would like to update to adapt to the new changes. > Neil Best Regards, Mark
[dpdk-dev] [PATCH 16/18] fm10k: add PF and VF interrupt handling function
Hi Neil,
> -Original Message-
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Sunday, February 01, 2015 8:43 AM
> To: Chen, Jing D
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 16/18] fm10k: add PF and VF interrupt
> handling function
>
> On Fri, Jan 30, 2015 at 01:07:32PM +0800, Chen Jing D(Mark) wrote:
> > From: Jeff Shaw
> >
> > 1. Add 2 interrupt handling functions, one for PF and one for VF.
> > 2. Enable interrupt after completing initialization of NIC.
> >
> This seems to do way more than enable interrupt handling. Can you be a bit
> more
> desriptive here?
OK, I'll try to add more description in the log.
> Neil
>
> > Signed-off-by: Jeff Shaw
> > Signed-off-by: Chen Jing D(Mark)
> > ---
> > lib/librte_pmd_fm10k/fm10k_ethdev.c | 268
> +++
> > 1 files changed, 268 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > index 40e3a2b..685fa8f 100644
> > --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > @@ -1325,6 +1325,256 @@ fm10k_rss_hash_conf_get(struct rte_eth_dev
> *dev,
> > return 0;
> > }
> >
> > +static void
> > +fm10k_dev_enable_intr_pf(struct rte_eth_dev *dev)
> > +{
> > + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + uint32_t int_map = FM10K_INT_MAP_IMMEDIATE;
> > +
> > + /* Bind all local non-queue interrupt to vector 0 */
> > + int_map |= 0;
> > +
> > + FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_Mailbox),
> int_map);
> > + FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_PCIeFault),
> int_map);
> > + FM10K_WRITE_REG(hw,
> FM10K_INT_MAP(fm10k_int_SwitchUpDown), int_map);
> > + FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SwitchEvent),
> int_map);
> > + FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_SRAM),
> int_map);
> > + FM10K_WRITE_REG(hw, FM10K_INT_MAP(fm10k_int_VFLR),
> int_map);
> > +
> > + /* Enable misc causes */
> > + FM10K_WRITE_REG(hw, FM10K_EIMR,
> FM10K_EIMR_ENABLE(PCA_FAULT) |
> > + FM10K_EIMR_ENABLE(THI_FAULT) |
> > + FM10K_EIMR_ENABLE(FUM_FAULT) |
> > + FM10K_EIMR_ENABLE(MAILBOX) |
> > + FM10K_EIMR_ENABLE(SWITCHREADY) |
> > + FM10K_EIMR_ENABLE(SWITCHNOTREADY) |
> > + FM10K_EIMR_ENABLE(SRAMERROR) |
> > + FM10K_EIMR_ENABLE(VFLR));
> > +
> > + /* Enable ITR 0 */
> > + FM10K_WRITE_REG(hw, FM10K_ITR(0), FM10K_ITR_AUTOMASK |
> > + FM10K_ITR_MASK_CLEAR);
> > + FM10K_WRITE_FLUSH(hw);
> > +}
> > +
> > +static void
> > +fm10k_dev_enable_intr_vf(struct rte_eth_dev *dev)
> > +{
> > + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + uint32_t int_map = FM10K_INT_MAP_IMMEDIATE;
> > +
> > + /* Bind all local non-queue interrupt to vector 0 */
> > + int_map |= 0;
> > +
> > + /* Only INT 0 availiable, other 15 are reserved. */
> > + FM10K_WRITE_REG(hw, FM10K_VFINT_MAP, int_map);
> > +
> > + /* Enable ITR 0 */
> > + FM10K_WRITE_REG(hw, FM10K_VFITR(0), FM10K_ITR_AUTOMASK |
> > + FM10K_ITR_MASK_CLEAR);
> > + FM10K_WRITE_FLUSH(hw);
> > +}
> > +
> > +static int
> > +fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
> > +{
> > + struct fm10k_fault fault;
> > + int err;
> > + const char *estr = "Unknown error";
> > +
> > + /* Process PCA fault */
> > + if (eicr & FM10K_EIMR_PCA_FAULT) {
> > + err = fm10k_get_fault(hw, FM10K_PCA_FAULT, &fault);
> > + if (err)
> > + goto error;
> > + switch (fault.type) {
> > + case PCA_NO_FAULT:
> > + estr = "PCA_NO_FAULT"; break;
> > + case PCA_UNMAPPED_ADDR:
> > + estr = "PCA_UNMAPPED_ADDR"; break;
> > + case PCA_BAD_QACCESS_PF:
> > + estr = "PCA_BAD_QACCESS_PF"; break;
> > + case PCA_BAD_QACCESS_VF:
> > + estr = "PCA_BAD_QACCESS_VF"; break;
> > + case PCA_MALICIOUS_REQ:
> > + estr = "PCA_MALICIOUS_REQ"; break;
&
[dpdk-dev] [PATCH 18/18] Change mk/rte.app.mk to add fm10k lib into link
Hi Neil, > -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Sunday, February 01, 2015 8:51 AM > To: Chen, Jing D > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 18/18] Change mk/rte.app.mk to add fm10k > lib into link > > On Fri, Jan 30, 2015 at 01:07:34PM +0800, Chen Jing D(Mark) wrote: > > From: Jeff Shaw > > > > Signed-off-by: Jeff Shaw > > Signed-off-by: Chen Jing D(Mark) > > --- > > mk/rte.app.mk |4 > > 1 files changed, 4 insertions(+), 0 deletions(-) > > > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > > index 4294d9a..87d8763 100644 > > --- a/mk/rte.app.mk > > +++ b/mk/rte.app.mk > > @@ -211,6 +211,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) > > LDLIBS += -lrte_pmd_i40e > > endif > > > > +ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y) > > +LDLIBS += -lrte_pmd_fm10k > > +endif > > + > > ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) > > LDLIBS += -lrte_pmd_ixgbe > > endif > > -- > > 1.7.7.6 > > > > > This patch should be merged with patch 17, and patch 2, and placed at the > end of > your series to avoid a FTBFS issue My rationale is to make every single patch not to break the compile. So, I'd like to add the binary library into compile and link in last 2 patches, after all the actual code are patched. For Patch 2, I think you are right, maybe a better way is to move it as patch "16". But I'm not sure whether I should merge these 3 together. You know, somebody may not happy to see the changes in different directory to appear in single patch. > Neil
Re: [dpdk-dev] eth_fm10k_dev_init failed as there is no Glort update
Hi,
> -Original Message-
> From: Krishna S [mailto:k.shar...@gmail.com]
> Sent: Thursday, August 17, 2017 5:02 PM
> To: Chen, Jing D
> Cc: dev@dpdk.org; Wang, Xiao W
> Subject: Re: eth_fm10k_dev_init failed as there is no Glort update
>
> Hi Everyone,
>
> Can anyone help me on this?
>
> Regards,
> Krishna Sharma
>
> On Wed, Aug 16, 2017 at 7:24 PM, Krishna S wrote:
> > Hi Cheng,
> >
> > I am working on a system which uses fm10k driver.
> >
> > In my system, I am hitting an error in eth_fm10k_dev_init().
> >
> > We are waiting for VF to get GLoRT update message once
> > update_lport_state(hw, hw->mac.dglort_map, 1, 1); is done.
> >
> > Check goes something like this.
> > if (hw->mac.type == fm10k_mac_vf) {
> > int glort_valid = 0;
> > int i;
> >
> > for (i = 0; i < MAX_QUERY_GLORT_STATE_TIMES; i++) {
> > glort_valid = fm10k_glort_valid(hw);
> > if (glort_valid) {
> > PMD_INIT_LOG(INFO, "GloRT update took ~%u
> > ms!",
> > (i * WAIT_GLORT_MSG_US/1000));
> > break;
> > }
> >
> > But we are at times failing to get an update and so initialising fails.
> >
> > Could you kindly give pointers on what could be possible reasons why
> > is there no update from PF ?
DPDK doesn't support DPDK PF + DPDK VF case, only kernel PF + DPDK/kernel VF
driver case.
> >
> >
> > --
> > Regards,
> > Krishna Sharma
>
>
>
> --
> Regards,
> Krishna Sharma
Re: [dpdk-dev] eth_fm10k_dev_init failed as there is no Glort update
Some corretness.
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Chen, Jing D
> Sent: Thursday, August 17, 2017 5:06 PM
> To: Krishna S
> Cc: dev@dpdk.org; Wang, Xiao W
> Subject: Re: [dpdk-dev] eth_fm10k_dev_init failed as there is no Glort update
>
> Hi,
>
> > -Original Message-
> > From: Krishna S [mailto:k.shar...@gmail.com]
> > Sent: Thursday, August 17, 2017 5:02 PM
> > To: Chen, Jing D
> > Cc: dev@dpdk.org; Wang, Xiao W
> > Subject: Re: eth_fm10k_dev_init failed as there is no Glort update
> >
> > Hi Everyone,
> >
> > Can anyone help me on this?
> >
> > Regards,
> > Krishna Sharma
> >
> > On Wed, Aug 16, 2017 at 7:24 PM, Krishna S
> wrote:
> > > Hi Cheng,
> > >
> > > I am working on a system which uses fm10k driver.
> > >
> > > In my system, I am hitting an error in eth_fm10k_dev_init().
> > >
> > > We are waiting for VF to get GLoRT update message once
> > > update_lport_state(hw, hw->mac.dglort_map, 1, 1); is done.
> > >
> > > Check goes something like this.
> > > if (hw->mac.type == fm10k_mac_vf) {
> > > int glort_valid = 0;
> > > int i;
> > >
> > > for (i = 0; i < MAX_QUERY_GLORT_STATE_TIMES; i++) {
> > > glort_valid = fm10k_glort_valid(hw);
> > > if (glort_valid) {
> > > PMD_INIT_LOG(INFO, "GloRT update took ~%u
> > > ms!",
> > > (i * WAIT_GLORT_MSG_US/1000));
> > > break;
> > > }
> > >
> > > But we are at times failing to get an update and so initialising fails.
> > >
> > > Could you kindly give pointers on what could be possible reasons why
> > > is there no update from PF ?
>
> DPDK doesn't support DPDK PF + DPDK VF case, only kernel PF + DPDK/kernel
> VF driver case.
fm10k doesn't support DPDK fm10k PF + DPDK fm10k VF case, only kernel fm10k PF
+ DPDK/kernel
fm10k VF driver case.
>
> > >
> > >
> > > --
> > > Regards,
> > > Krishna Sharma
> >
> >
> >
> > --
> > Regards,
> > Krishna Sharma
Re: [dpdk-dev] eth_fm10k_dev_init failed as there is no Glort update
> -Original Message-
> From: Krishna S [mailto:k.shar...@gmail.com]
> Sent: Thursday, August 17, 2017 6:33 PM
> To: Chen, Jing D
> Cc: dev@dpdk.org; Wang, Xiao W
> Subject: Re: eth_fm10k_dev_init failed as there is no Glort update
>
> We are having kernel fm10k PF + fm10k VF driver. We are getting the
> update(glort updated) in all the cases in most of the times.
> But randomly, we are seeing this issue in one of VF initialisation.
>
> The frequency of issue is, say 1 in 20 times.
This is DPDK community, kernel driver is not in the scope.
Please turn to kernel community or ask local technical support from Intel.
>
>
> On Thu, Aug 17, 2017 at 2:44 PM, Chen, Jing D
> wrote:
> > Some corretness.
> >
> >> -Original Message-
> >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Chen, Jing D
> >> Sent: Thursday, August 17, 2017 5:06 PM
> >> To: Krishna S
> >> Cc: dev@dpdk.org; Wang, Xiao W
> >> Subject: Re: [dpdk-dev] eth_fm10k_dev_init failed as there is no
> >> Glort update
> >>
> >> Hi,
> >>
> >> > -Original Message-
> >> > From: Krishna S [mailto:k.shar...@gmail.com]
> >> > Sent: Thursday, August 17, 2017 5:02 PM
> >> > To: Chen, Jing D
> >> > Cc: dev@dpdk.org; Wang, Xiao W
> >> > Subject: Re: eth_fm10k_dev_init failed as there is no Glort update
> >> >
> >> > Hi Everyone,
> >> >
> >> > Can anyone help me on this?
> >> >
> >> > Regards,
> >> > Krishna Sharma
> >> >
> >> > On Wed, Aug 16, 2017 at 7:24 PM, Krishna S
> >> wrote:
> >> > > Hi Cheng,
> >> > >
> >> > > I am working on a system which uses fm10k driver.
> >> > >
> >> > > In my system, I am hitting an error in eth_fm10k_dev_init().
> >> > >
> >> > > We are waiting for VF to get GLoRT update message once
> >> > > update_lport_state(hw, hw->mac.dglort_map, 1, 1); is done.
> >> > >
> >> > > Check goes something like this.
> >> > > if (hw->mac.type == fm10k_mac_vf) {
> >> > > int glort_valid = 0;
> >> > > int i;
> >> > >
> >> > > for (i = 0; i < MAX_QUERY_GLORT_STATE_TIMES; i++) {
> >> > > glort_valid = fm10k_glort_valid(hw);
> >> > > if (glort_valid) {
> >> > > PMD_INIT_LOG(INFO, "GloRT update took
> >> > > ~%u ms!",
> >> > > (i * WAIT_GLORT_MSG_US/1000));
> >> > > break;
> >> > > }
> >> > >
> >> > > But we are at times failing to get an update and so initialising fails.
> >> > >
> >> > > Could you kindly give pointers on what could be possible reasons
> >> > > why is there no update from PF ?
> >>
> >> DPDK doesn't support DPDK PF + DPDK VF case, only kernel PF +
> >> DPDK/kernel VF driver case.
> >
> > fm10k doesn't support DPDK fm10k PF + DPDK fm10k VF case, only kernel
> > fm10k PF + DPDK/kernel fm10k VF driver case.
> >
> >>
> >> > >
> >> > >
> >> > > --
> >> > > Regards,
> >> > > Krishna Sharma
> >> >
> >> >
> >> >
> >> > --
> >> > Regards,
> >> > Krishna Sharma
>
>
>
> --
> Regards,
> Krishna Sharma
[dpdk-dev] [PATCH v2] doc: add Vector FM10K introductions
Hi, John, Best Regards, Mark > -Original Message- > From: Mcnamara, John > Sent: Monday, February 22, 2016 9:47 PM > To: Chen, Jing D; dev at dpdk.org > Subject: RE: [PATCH v2] doc: add Vector FM10K introductions > > > -Original Message----- > > From: Chen, Jing D > > Sent: Saturday, February 6, 2016 6:48 AM > > To: dev at dpdk.org > > Cc: Mcnamara, John ; Chen, Jing D > > > > Subject: [PATCH v2] doc: add Vector FM10K introductions > > > > From: "Chen Jing D(Mark)" > > > > Add introductions on how to enable Vector FM10K Rx/Tx functions, the > > preconditions and assumptions on Rx/Tx configuration parameters. > > Hi Mark, > > Thanks for the update. A few minor comments below. > > > > > +Vector PMD (vPMD) uses Intel? SIMD instructions to optimize packet I/O. > > +It improves load/store bandwidth efficiency of L1 data cache by using a > > +wider SSE/AVX register 1 (1). > > This should probably be "register (1)" > > > > +The wider register gives space to hold multiple packet buffers so as to > > +save instruction number when processing bulk of packets. > > Maybe a little clearer as: > > The wider register gives space to hold multiple packet buffers so as to save > on the number of instructions when bulk processing packets. > > > > + > > +There is no change to PMD API. The RX/TX handler are the only two > > +entries for vPMD packet I/O. They are transparently registered at > > +runtime RX/TX execution if all condition checks pass. > > s/if all condition checks pass./if all conditions are met./ > > > > +As vPMD is focused on high throughput, it 4 packets at a time. So it > > s/it 4 packets at a time/it processes 4 packets at a time/ > > John Many thanks for the comments. I'll change and send a new version soon.
[dpdk-dev] [PATCH v3 1/3] fm10k: enable FTAG based forwarding
Hi, Bruce, Thomas, Best Regards, Mark > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > Sent: Thursday, February 25, 2016 12:38 AM > To: Richardson, Bruce; Wang, Xiao W > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 1/3] fm10k: enable FTAG based > forwarding > > 2016-02-24 15:42, Bruce Richardson: > > On Thu, Feb 04, 2016 at 11:38:47AM +0800, Wang Xiao W wrote: > > > This patch enables reading sglort info into mbuf for RX and inserting > > > an FTAG at the beginning of the packet for TX. The vlan_tci_outer field > > > selected from rte_mbuf structure for sglort is not used in fm10k now. > > > In FTAG based forwarding mode, the switch will forward packets > according > > > to glort info in FTAG rather than mac and vlan table. > > > > > > To activate this feature, user needs to turn > ``CONFIG_RTE_LIBRTE_FM10K_FTAG_FWD`` > > > to y in common_linuxapp or common_bsdapp. Currently this feature is > supported > > > only on PF, because FM10K_PFVTCTL register is read-only for VF. > > > > > > Signed-off-by: Wang Xiao W > > > > Any comments on this patch? > > > > My thoughts: is there a way in which this could be done without adding in a > new > > build time config option? > > Bruce, it's simpler to explain that build time options are forbidden to > enable such options. > Or the terrific kid's approach: one day, the Big Build-Option Eater will come > and will eat every undecided features! ;) This feature is trying to use FTAG (a unique tech in fm10k) instead of mac/vlan to forward packets. App need a way to tell PMD driver that which forwarding style it would like to use. So, the best option is to let packets carry a flag in mbuf to tell drive in fast path. You can see that this is unique to fm10k and we thought community won't like to see this flag introduced into mbuf. If you do agree, we can introduce a new flag. So, we step backwards and assume customer will use static configurations to enable this feature. After it is enabled, we'll assume app will use FTAG for all packets.
[dpdk-dev] [PATCH v3 1/3] fm10k: enable FTAG based forwarding
Hi, Bruce, > -Original Message- > From: Richardson, Bruce > Sent: Thursday, February 25, 2016 9:35 PM > To: Chen, Jing D > Cc: Thomas Monjalon ; Wang, Xiao W > ; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 1/3] fm10k: enable FTAG based > forwarding > > On Thu, Feb 25, 2016 at 10:04:02AM +, Chen, Jing D wrote: > > Hi, Bruce, Thomas, > > > > Best Regards, > > Mark > > > > > -Original Message- > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas > Monjalon > > > Sent: Thursday, February 25, 2016 12:38 AM > > > To: Richardson, Bruce; Wang, Xiao W > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v3 1/3] fm10k: enable FTAG based > > > forwarding > > > > > > 2016-02-24 15:42, Bruce Richardson: > > > > On Thu, Feb 04, 2016 at 11:38:47AM +0800, Wang Xiao W wrote: > > > > > This patch enables reading sglort info into mbuf for RX and > > > > > inserting an FTAG at the beginning of the packet for TX. The > > > > > vlan_tci_outer field selected from rte_mbuf structure for sglort is > > > > > not > used in fm10k now. > > > > > In FTAG based forwarding mode, the switch will forward packets > > > according > > > > > to glort info in FTAG rather than mac and vlan table. > > > > > > > > > > To activate this feature, user needs to turn > > > ``CONFIG_RTE_LIBRTE_FM10K_FTAG_FWD`` > > > > > to y in common_linuxapp or common_bsdapp. Currently this feature > > > > > is > > > supported > > > > > only on PF, because FM10K_PFVTCTL register is read-only for VF. > > > > > > > > > > Signed-off-by: Wang Xiao W > > > > > > > > Any comments on this patch? > > > > > > > > My thoughts: is there a way in which this could be done without > > > > adding in a > > > new > > > > build time config option? > > > > > > Bruce, it's simpler to explain that build time options are forbidden > > > to enable such options. > > > Or the terrific kid's approach: one day, the Big Build-Option Eater > > > will come and will eat every undecided features! ;) > > > > This feature is trying to use FTAG (a unique tech in fm10k) instead of > > mac/vlan to forward packets. App need a way to tell PMD driver that > > which forwarding style it would like to use. > > Why not just specify this in the port configuration at setup time? > Please educate me. I think the port configuration flags are also common to all PMD Drivers. Is it possible to add a flag like "RTE_USE_FTAG" and pass to PMD driver? > > So, the best option is to let packets carry a flag in mbuf to tell drive in > > fast > path. > > You can see that this is unique to fm10k and we thought community > > won't like to see this flag introduced into mbuf. If you do agree, we can > introduce a new flag. > > Why does it need to be specified per-mbuf? The existing config flag added is > global, so a per-mbuf flag shouldn't be needed to get equivalent behaviour. > > > So, we step backwards and assume customer will use static > > configurations to enable this feature. After it is enabled, we'll assume app > will use FTAG for all packets. > > Yes, but instead of compile time option, why not port config-time option > instead? > > /Bruce
[dpdk-dev] [PATCH] fm10k: fix VF cannot receive broadcast traffic
Hi,
> -Original Message-
> From: Wang, Xiao W
> Sent: Monday, June 06, 2016 5:01 PM
> To: Chen, Jing D
> Cc: dev at dpdk.org; Wang, Xiao W
> Subject: [PATCH] fm10k: fix VF cannot receive broadcast traffic
>
> When app tries promisc/allmulti setting, fm10k will check if a valid glort
> is acquired, if not then exit without doing anything. It's a long journey
> for VF to acquire glort info from VF to PF mailbox, PF to switch mailbox.
> It could be a long interval that's out of DPDK's control. Thus, app may
> fail on promisc/allmulti setting in VF. In fact, we don't need a valid
> glort value in VF, so this patch just skips the glort check for VF.
>
> Fixes: df02ba864695 ("fm10k: support promiscuous mode")
>
> Signed-off-by: Wang Xiao W
Acked-by: Jing Chen
[dpdk-dev] [PATCH] fm10k: fix VF cannot receive broadcast traffic
Hi, Bruce,
> -Original Message-
> From: Richardson, Bruce
> Sent: Friday, June 17, 2016 6:19 PM
> To: Wang, Xiao W
> Cc: Chen, Jing D ; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] fm10k: fix VF cannot receive broadcast
> traffic
>
> On Mon, Jun 06, 2016 at 05:00:47PM +0800, Wang Xiao W wrote:
> > When app tries promisc/allmulti setting, fm10k will check if a valid
> > glort is acquired, if not then exit without doing anything. It's a
> > long journey for VF to acquire glort info from VF to PF mailbox, PF to
> > switch
> mailbox.
> > It could be a long interval that's out of DPDK's control. Thus, app
> > may
>
> I think the use of "thus" here is wrong, as I suspect that the failure is not
> due
> to the "long interval that's out of DPDK's control", but instead due to not
> having a valid glort.
The logic in VF is glort ID is invalid at beginning. When VF port is enabled by
sending mailbox to PF, PF will send a message back to VF without carrying valid
info. Then, VF will fake a glort ID.
In this case, it's useless to do sanity check of VALID glort ID. Besides that,
VF didn't
use glort ID to do functional call at all.
>
> > fail on promisc/allmulti setting in VF. In fact, we don't need a valid
> > glort value in VF, so this patch just skips the glort check for VF.
> >
> > Fixes: df02ba864695 ("fm10k: support promiscuous mode")
> >
> > Signed-off-by: Wang Xiao W
>
> I rework this commit message for you on apply. Please check the updated
> version when done.
>
> /Bruce
[dpdk-dev] [PATCH v4 05/12] pmd/fm10k: add dev_ptype_info_get implementation
Hi,
-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jianfeng Tan
Sent: Thursday, February 25, 2016 6:09 PM
To: dev at dpdk.org
Subject: [dpdk-dev] [PATCH v4 05/12] pmd/fm10k: add dev_ptype_info_get
implementation
Signed-off-by: Jianfeng Tan
---
drivers/net/fm10k/fm10k_ethdev.c | 50 ++
drivers/net/fm10k/fm10k_rxtx.c | 3 +++
drivers/net/fm10k/fm10k_rxtx_vec.c | 3 +++
3 files changed, 56 insertions(+)
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 421266b..429cbdd 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1335,6 +1335,55 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
};
}
+#ifdef RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE
+static const uint32_t *
+fm10k_dev_ptype_info_get(struct rte_eth_dev *dev) {
+ if (dev->rx_pkt_burst == fm10k_recv_pkts ||
+ dev->rx_pkt_burst == fm10k_recv_scattered_pkts) {
+ static uint32_t ptypes[] = {
+ /* refers to rx_desc_to_ol_flags() */
+ RTE_PTYPE_L2_ETHER,
+ RTE_PTYPE_L3_IPV4,
+ RTE_PTYPE_L3_IPV4_EXT,
+ RTE_PTYPE_L3_IPV6,
+ RTE_PTYPE_L3_IPV6_EXT,
+ RTE_PTYPE_L4_TCP,
+ RTE_PTYPE_L4_UDP,
+ RTE_PTYPE_UNKNOWN
+ };
+
+ return ptypes;
+ } else if (dev->rx_pkt_burst == fm10k_recv_pkts_vec ||
+ dev->rx_pkt_burst == fm10k_recv_scattered_pkts_vec) {
+ static uint32_t ptypes_vec[] = {
+ /* refers to fm10k_desc_to_pktype_v() */
+ RTE_PTYPE_L3_IPV4,
+ RTE_PTYPE_L3_IPV4_EXT,
+ RTE_PTYPE_L3_IPV6,
+ RTE_PTYPE_L3_IPV6_EXT,
+ RTE_PTYPE_L4_TCP,
+ RTE_PTYPE_L4_UDP,
+ RTE_PTYPE_TUNNEL_GENEVE,
+ RTE_PTYPE_TUNNEL_NVGRE,
+ RTE_PTYPE_TUNNEL_VXLAN,
+ RTE_PTYPE_TUNNEL_GRE,
+ RTE_PTYPE_UNKNOWN
+ };
+
+ return ptypes_vec;
+ }
+
+ return NULL;
+}
May I know when " fm10k_dev_ptype_info_get " will be called? In fm10k, the
actual
Rx/tx func will be decided after port is started.
[dpdk-dev] [PATCH v3 00/18] fm10k: update shared code
Hi, Xiao > -Original Message- > From: Wang, Xiao W > Sent: Tuesday, March 8, 2016 8:15 AM > To: Richardson, Bruce ; Chen, Jing D > > Cc: Chen, Jing D ; dev at dpdk.org; He, Shaopeng > > Subject: RE: [PATCH v3 00/18] fm10k: update shared code > > > > > -Original Message- > > From: Richardson, Bruce > > Sent: Tuesday, March 8, 2016 9:24 PM > > To: Wang, Xiao W ; Chen, Jing D > > > > Cc: Chen, Jing D ; dev at dpdk.org; He, Shaopeng > > > > Subject: Re: [PATCH v3 00/18] fm10k: update shared code > > > > On Fri, Feb 19, 2016 at 07:06:47PM +0800, Wang Xiao W wrote: > > > v3: > > > * Fixed checkpatch.pl warning about long commit message. > > > * Fixed the issue of compile failure about part of patches applied. > > > * Split the misc-small-fixes patch into several patches. > > > > > > v2: > > > * Put the two extra fix patches ahead of the base code patches. > > > > > > This patch set has passed regression test. > > > > > > Wang Xiao W (18): > > > fm10k: use default mailbox message handler for PF > > > fm10k/base: correct typecast in fm10k_update_xc_addr_pf > > > fm10k/base: cleanup namespace pollution > > > fm10k/base: use bitshift for itr_scale > > > fm10k/base: reset max_queues on init_hw_vf failure > > > fm10k/base: document ITR scale workaround in VF TDLEN register > > > fm10k/base: cleanup lines over 80 characters > > > fm10k/base: cleanup useless else > > > fm10k/base: use BIT macro instead of open-coded bit-shifting of 1 > > > fm10k/base: do not use CamelCase > > > fm10k/base: use memcpy for mac addr copy > > > fm10k/base: allow removal of is_slot_appropriate function > > > fm10k/base: consistently use VLAN ID when referencing vid variables > > > fm10k/base: imporve comment per upstream review changes > > > fm10k/base: fix TLV structures alignment > > > fm10k/base: move constants to the right of binary operators > > > fm10k/base: minor cleanups > > > fm10k/base: remove unused struct element > > > > > > drivers/net/fm10k/base/fm10k_api.c | 2 + > > > drivers/net/fm10k/base/fm10k_api.h | 2 + > > > drivers/net/fm10k/base/fm10k_mbx.c | 63 +++- > > > drivers/net/fm10k/base/fm10k_mbx.h | 11 +-- > > > drivers/net/fm10k/base/fm10k_osdep.h | 32 ++ > > > drivers/net/fm10k/base/fm10k_pf.c| 88 + > > > drivers/net/fm10k/base/fm10k_pf.h| 18 ++-- > > > drivers/net/fm10k/base/fm10k_tlv.c | 40 > > > drivers/net/fm10k/base/fm10k_tlv.h | 9 +- > > > drivers/net/fm10k/base/fm10k_type.h | 182 +++- > --- > > > drivers/net/fm10k/base/fm10k_vf.c| 32 -- > > > drivers/net/fm10k/fm10k_ethdev.c | 41 +++- > > > 12 files changed, 222 insertions(+), 298 deletions(-) > > > > > > -- > > > 1.9.3 > > > > > Hi Mark, > > > > Can we get fm10k maintainer review and/or ack on this patchset please. > > > > Thanks, > > /Bruce > > Hi Bruce, > > Mark has reviewed and acked the patch set in v2, and I put the "Acked-by " > in the v3 01/18 patch. > It's the same for my FTAG patch. > It's better to add acked-by in both patch set and cover letter, this may be more helpful for maintainers. > Best Regards, > Xiao
[dpdk-dev] [PATCH 03/18] fm10k: Add empty fm10k files
Hi Neil, > -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Monday, February 02, 2015 9:39 PM > To: Chen, Jing D > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 03/18] fm10k: Add empty fm10k files > > On Mon, Feb 02, 2015 at 05:34:43AM +, Chen, Jing D wrote: > > Hi Neil, > > > > > -Original Message- > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > Sent: Saturday, January 31, 2015 10:02 PM > > > To: Chen, Jing D > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH 03/18] fm10k: Add empty fm10k files > > > > > > On Fri, Jan 30, 2015 at 01:07:19PM +0800, Chen Jing D(Mark) wrote: > > > > From: Jeff Shaw > > > > > > > > Define macros and basic data structure. > > > > Define rte_log wrapper functions. > > > > > > > > Signed-off-by: Jeff Shaw > > > > Signed-off-by: Chen Jing D(Mark) > > > > --- > > > > lib/librte_pmd_fm10k/Makefile | 96 > > > > lib/librte_pmd_fm10k/fm10k.h | 224 > > > + > > > > lib/librte_pmd_fm10k/fm10k_logs.h | 66 +++ > > > > 3 files changed, 386 insertions(+), 0 deletions(-) > > > > create mode 100644 lib/librte_pmd_fm10k/Makefile > > > > create mode 100644 lib/librte_pmd_fm10k/fm10k.h > > > > create mode 100644 lib/librte_pmd_fm10k/fm10k_ethdev.c > > > > create mode 100644 lib/librte_pmd_fm10k/fm10k_logs.h > > > > create mode 100644 lib/librte_pmd_fm10k/fm10k_rxtx.c > > > > > > > Why are you adding empty files? > > > > The 2 ".c" files are empty while the 2 ".h" files include code. "Makefile" > includes rules to > > compile the ".c" files, I don't like to break the compile for every single > > patch, > that's why > > the 2 ".c" files are added in this patch. > > > That doesn't really answer the question. Theres no need to add empty files > here. Just add the headers alone and add the empy files on the first commit > where you have code to put in them. Adjust the makefile so that you add > them > into the compilation in the same commit that you populate the file to avoid a > FTBFS error. > Neil Got you. I'll add the content with new files. Thanks! > > > > > > > Neil > > > > Thanks for your comments. > > Mark > >
[dpdk-dev] [PATCH 10/18] fm10k: add dev start/stop functions
Hi Michael,
> -Original Message-
> From: Qiu, Michael
> Sent: Wednesday, February 04, 2015 10:36 AM
> To: Chen, Jing D; dev at dpdk.org
> Cc: Zhang, Helin; Shaw, Jeffrey B
> Subject: Re: [PATCH 10/18] fm10k: add dev start/stop functions
>
> On 1/30/2015 1:08 PM, Chen, Jing D wrote:
> > From: Jeff Shaw
> >
> > 1. Add function to initialize single RX queue.
> > 2. Add function to initialize single TX queue.
> > 3. Add fm10k_dev_start, fm10k_dev_stop and fm10k_dev_close
> >functions.
> >
> > Signed-off-by: Jeff Shaw
> > Signed-off-by: Chen Jing D(Mark)
> > ---
> > lib/librte_pmd_fm10k/fm10k_ethdev.c | 220
> +++
> > 1 files changed, 220 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > index b4b49cd..3cf5e25 100644
> > --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > @@ -49,6 +49,8 @@
> > #define CHARS_PER_UINT32 (sizeof(uint32_t))
> > #define BIT_MASK_PER_UINT32 ((1 << CHARS_PER_UINT32) - 1)
> >
> > +static void fm10k_close_mbx_service(struct fm10k_hw *hw);
> > +
> > static void
> > fm10k_mbx_initlock(struct fm10k_hw *hw)
> > {
> > @@ -268,6 +270,98 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
> > }
> >
> > static int
> > +fm10k_dev_tx_init(struct rte_eth_dev *dev)
> > +{
> > + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + int i, ret;
> > + struct fm10k_tx_queue *txq;
> > + uint64_t base_addr;
> > + uint32_t size;
> > +
> > + /* Disable TXINT to avoid possible interrupt */
> > + for (i = 0; i < hw->mac.max_queues; i++)
> > + FM10K_WRITE_REG(hw, FM10K_TXINT(i),
> > + 3 << FM10K_TXINT_TIMER_SHIFT);
> > +
> > + /* Setup TX queue */
> > + for (i = 0; i < dev->data->nb_tx_queues; ++i) {
> > + txq = dev->data->tx_queues[i];
> > + base_addr = txq->hw_ring_phys_addr;
> > + size = txq->nb_desc * sizeof(struct fm10k_tx_desc);
> > +
> > + /* disable queue to avoid issues while updating state */
> > + ret = tx_queue_disable(hw, i);
> > + if (ret) {
> > + PMD_LOG(ERR, "failed to disable queue %d\n", i);
> > + return -1;
> > + }
> > +
> > + /* set location and size for descriptor ring */
> > + FM10K_WRITE_REG(hw, FM10K_TDBAL(i),
> > + base_addr & 0xULL);
>
> Here better to make a address mask here.
OK. Thanks for pointing it out.
>
> > + FM10K_WRITE_REG(hw, FM10K_TDBAH(i),
> > + base_addr >> (CHAR_BIT * sizeof(uint32_t)));
> > + FM10K_WRITE_REG(hw, FM10K_TDLEN(i), size);
> > + }
> > + return 0;
> > +}
> > +
> > +static int
> > +fm10k_dev_rx_init(struct rte_eth_dev *dev)
> > +{
> > + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + int i, ret;
> > + struct fm10k_rx_queue *rxq;
> > + uint64_t base_addr;
> > + uint32_t size;
> > + uint32_t rxdctl = FM10K_RXDCTL_WRITE_BACK_MIN_DELAY;
> > + uint16_t buf_size;
> > + struct rte_pktmbuf_pool_private *mbp_priv;
> > +
> > + /* Disable RXINT to avoid possible interrupt */
> > + for (i = 0; i < hw->mac.max_queues; i++)
> > + FM10K_WRITE_REG(hw, FM10K_RXINT(i),
> > + 3 << FM10K_RXINT_TIMER_SHIFT);
> > +
> > + /* Setup RX queues */
> > + for (i = 0; i < dev->data->nb_rx_queues; ++i) {
> > + rxq = dev->data->rx_queues[i];
> > + base_addr = rxq->hw_ring_phys_addr;
> > + size = rxq->nb_desc * sizeof(union fm10k_rx_desc);
> > +
> > + /* disable queue to avoid issues while updating state */
> > + ret = rx_queue_disable(hw, i);
> > + if (ret) {
> > + PMD_LOG(ERR, "failed to disable queue %d\n", i);
> > + return -1;
> > + }
> > +
> > + /* Setup the Base and Length of the Rx Descriptor Ring */
> > + FM10K_WRITE_REG(hw, FM10K_RDBAL(i),
> > + base_addr & 0xULL);
>
> Here better to make a add
[dpdk-dev] [PATCH v4 10/15] fm10k: add receive and tranmit function
> -Original Message-
> From: Shaw, Jeffrey B
> Sent: Thursday, February 12, 2015 1:29 AM
> To: Chen, Jing D
> Cc: dev at dpdk.org; Zhang, Helin; Qiu, Michael; nhorman at tuxdriver.com;
> thomas.monjalon at 6wind.com; david.marchand at 6wind.com
> Subject: Re: [PATCH v4 10/15] fm10k: add receive and tranmit function
>
> On Wed, Feb 11, 2015 at 09:31:33AM +0800, Chen Jing D(Mark) wrote:
>
> > +uint16_t
> > +fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> > + uint16_t nb_pkts)
> > +{
> > + struct rte_mbuf *mbuf;
> > + union fm10k_rx_desc desc;
> > + struct fm10k_rx_queue *q = rx_queue;
> > + uint16_t count = 0;
> > + int alloc = 0;
> > + uint16_t next_dd;
> > +
> > + next_dd = q->next_dd;
> > +
> > + nb_pkts = RTE_MIN(nb_pkts, q->alloc_thresh);
> > + for (count = 0; count < nb_pkts; ++count) {
> > + mbuf = q->sw_ring[next_dd];
> > + desc = q->hw_ring[next_dd];
> > + if (!(desc.d.staterr & FM10K_RXD_STATUS_DD))
> > + break;
> > +#ifdef RTE_LIBRTE_FM10K_DEBUG_RX
> > + dump_rxd(&desc);
> > +#endif
> > + rte_pktmbuf_pkt_len(mbuf) = desc.w.length;
> > + rte_pktmbuf_data_len(mbuf) = desc.w.length;
> > +
> > + mbuf->ol_flags = 0;
> > +#ifdef RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE
> > + rx_desc_to_ol_flags(mbuf, &desc);
> > +#endif
> > +
> > + mbuf->hash.rss = desc.d.rss;
> > +
> > + rx_pkts[count] = mbuf;
> > + if (++next_dd == q->nb_desc) {
> > + next_dd = 0;
> > + alloc = 1;
> > + }
> > +
> > + /* Prefetch next mbuf while processing current one. */
> > + rte_prefetch0(q->sw_ring[next_dd]);
> > +
> > + /*
> > +* When next RX descriptor is on a cache-line boundary,
> > +* prefetch the next 4 RX descriptors and the next 8 pointers
> > +* to mbufs.
> > +*/
> > + if ((next_dd & 0x3) == 0) {
> > + rte_prefetch0(&q->hw_ring[next_dd]);
> > + rte_prefetch0(&q->sw_ring[next_dd]);
> > + }
> > + }
> > +
> > + q->next_dd = next_dd;
> > +
> > + if ((q->next_dd > q->next_trigger) || (alloc == 1)) {
> > + rte_mempool_get_bulk(q->mp, (void **)&q->sw_ring[q-
> >next_alloc],
> > + q->alloc_thresh);
>
> The return value should be checked here in case the mempool runs out
> of buffers. Thanks Helin for spotting this. I'm not sure how I missed it
> originally.
Thanks! I'll change accordingly.
>
> > + for (; q->next_alloc <= q->next_trigger; ++q->next_alloc) {
> > + mbuf = q->sw_ring[q->next_alloc];
> > +
> > + /* setup static mbuf fields */
> > + fm10k_pktmbuf_reset(mbuf, q->port_id);
> > +
> > + /* write descriptor */
> > + desc.q.pkt_addr =
> MBUF_DMA_ADDR_DEFAULT(mbuf);
> > + desc.q.hdr_addr =
> MBUF_DMA_ADDR_DEFAULT(mbuf);
> > + q->hw_ring[q->next_alloc] = desc;
> > + }
> > + FM10K_PCI_REG_WRITE(q->tail_ptr, q->next_trigger);
> > + q->next_trigger += q->alloc_thresh;
> > + if (q->next_trigger >= q->nb_desc) {
> > + q->next_trigger = q->alloc_thresh - 1;
> > + q->next_alloc = 0;
> > + }
> > + }
> > +
> > + return count;
> > +}
> > +
>
> Thanks,
> Jeff
[dpdk-dev] [PATCH v4 12/15] fm10k: Add scatter receive function
> -Original Message-
> From: Shaw, Jeffrey B
> Sent: Thursday, February 12, 2015 1:33 AM
> To: Chen, Jing D
> Cc: dev at dpdk.org; Zhang, Helin; Qiu, Michael; nhorman at tuxdriver.com;
> thomas.monjalon at 6wind.com; david.marchand at 6wind.com
> Subject: Re: [PATCH v4 12/15] fm10k: Add scatter receive function
>
> On Wed, Feb 11, 2015 at 09:31:35AM +0800, Chen Jing D(Mark) wrote:
> >
> > +uint16_t
> > +fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> > + uint16_t nb_pkts)
> > +{
> > + struct rte_mbuf *mbuf;
> > + union fm10k_rx_desc desc;
> > + struct fm10k_rx_queue *q = rx_queue;
> > + uint16_t count = 0;
> > + uint16_t nb_rcv, nb_seg;
> > + int alloc = 0;
> > + uint16_t next_dd;
> > + struct rte_mbuf *first_seg = q->pkt_first_seg;
> > + struct rte_mbuf *last_seg = q->pkt_last_seg;
> > +
> > + next_dd = q->next_dd;
> > + nb_rcv = 0;
> > +
> > + nb_seg = RTE_MIN(nb_pkts, q->alloc_thresh);
> > + for (count = 0; count < nb_seg; count++) {
> > + mbuf = q->sw_ring[next_dd];
> > + desc = q->hw_ring[next_dd];
> > + if (!(desc.d.staterr & FM10K_RXD_STATUS_DD))
> > + break;
> > +#ifdef RTE_LIBRTE_FM10K_DEBUG_RX
> > + dump_rxd(&desc);
> > +#endif
> > +
> > + if (++next_dd == q->nb_desc) {
> > + next_dd = 0;
> > + alloc = 1;
> > + }
> > +
> > + /* Prefetch next mbuf while processing current one. */
> > + rte_prefetch0(q->sw_ring[next_dd]);
> > +
> > + /*
> > +* When next RX descriptor is on a cache-line boundary,
> > +* prefetch the next 4 RX descriptors and the next 8 pointers
> > +* to mbufs.
> > +*/
> > + if ((next_dd & 0x3) == 0) {
> > + rte_prefetch0(&q->hw_ring[next_dd]);
> > + rte_prefetch0(&q->sw_ring[next_dd]);
> > + }
> > +
> > + /* Fill data length */
> > + rte_pktmbuf_data_len(mbuf) = desc.w.length;
> > +
> > + /*
> > +* If this is the first buffer of the received packet,
> > +* set the pointer to the first mbuf of the packet and
> > +* initialize its context.
> > +* Otherwise, update the total length and the number of
> segments
> > +* of the current scattered packet, and update the pointer to
> > +* the last mbuf of the current packet.
> > +*/
> > + if (!first_seg) {
> > + first_seg = mbuf;
> > + first_seg->pkt_len = desc.w.length;
> > + } else {
> > + first_seg->pkt_len =
> > + (uint16_t)(first_seg->pkt_len +
> > + rte_pktmbuf_data_len(mbuf));
> > + first_seg->nb_segs++;
> > + last_seg->next = mbuf;
> > + }
> > +
> > + /*
> > +* If this is not the last buffer of the received packet,
> > +* update the pointer to the last mbuf of the current
> scattered
> > +* packet and continue to parse the RX ring.
> > +*/
> > + if (!(desc.d.staterr & FM10K_RXD_STATUS_EOP)) {
> > + last_seg = mbuf;
> > + continue;
> > + }
> > +
> > + first_seg->ol_flags = 0;
> > +#ifdef RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE
> > + rx_desc_to_ol_flags(first_seg, &desc);
> > +#endif
> > + first_seg->hash.rss = desc.d.rss;
> > +
> > + /* Prefetch data of first segment, if configured to do so. */
> > + rte_packet_prefetch((char *)first_seg->buf_addr +
> > + first_seg->data_off);
> > +
> > + /*
> > +* Store the mbuf address into the next entry of the array
> > +* of returned packets.
> > +*/
> > + rx_pkts[nb_rcv++] = first_seg;
> > +
> > + /*
> > +* Setup receipt context for a new packet.
> > +*/
> > + first_seg = NULL;
> > + }
> > +
> > + q->next_dd = next_dd;
> > + q->pkt_first_seg = first_s
[dpdk-dev] [PATCH v5 06/17] fm10k: add rx_queue_setup/release function
Hi,
From: David Marchand [mailto:david.march...@6wind.com]
Sent: Friday, February 13, 2015 7:08 PM
To: Chen, Jing D
Cc: dev at dpdk.org; Zhang, Helin; Qiu, Michael; Neil Horman; Thomas Monjalon;
Shaw, Jeffrey B
Subject: Re: [PATCH v5 06/17] fm10k: add rx_queue_setup/release function
Hello,?
On Fri, Feb 13, 2015 at 9:19 AM, Chen Jing D(Mark)
wrote:
[snip]?
+static int
+fm10k_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
+? ? ? ?uint16_t nb_desc, unsigned int socket_id,
+? ? ? ?const struct rte_eth_rxconf *conf, struct rte_mempool *mp)
+{
+? ? ? ?struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+? ? ? ?struct fm10k_rx_queue *q;
+? ? ? ?const struct rte_memzone *mz;
+
+? ? ? ?PMD_INIT_FUNC_TRACE();
+
+? ? ? ?/* make sure the mempool element size can account for alignment. Use
+? ? ? ? * RTE_LOG directly to make sure this error is seen. */
Comment is not valid anymore since you call PMD_INIT_LOG.
[Mark] Thanks! I'll change the comments.
?
+? ? ? ?if (!mempool_element_size_valid(mp)) {
+? ? ? ? ? ? ? ?PMD_INIT_LOG(ERR, "Error : Mempool element size is too small");
+? ? ? ? ? ? ? ?return (-EINVAL);
+? ? ? ?}
+
--?
David Marchand?
[dpdk-dev] [PATCH v5 10/17] fm10k: add receive and tranmit function
Hi, From: David Marchand [mailto:david.march...@6wind.com] Sent: Friday, February 13, 2015 7:43 PM To: Chen, Jing D Cc: dev at dpdk.org; Zhang, Helin; Qiu, Michael; Neil Horman; Thomas Monjalon; Shaw, Jeffrey B Subject: Re: [PATCH v5 10/17] fm10k: add receive and tranmit function Hello,? On Fri, Feb 13, 2015 at 9:19 AM, Chen Jing D(Mark) wrote: [snip]? + +? ? ? ?/* set checksum flags on first descriptor of packet. SCTP checksum +? ? ? ? * offload is not supported, but we do not explicitely check for this +? ? ? ? * case in favor of greatly simplified processing. */ Checkpatch is complaining : WARNING: 'explicitely' may be misspelled - perhaps 'explicitly'? [Mark] Thanks, I'll change it. #328: FILE: lib/librte_pmd_fm10k/fm10k_rxtx.c:261: + ? ? ? ?* offload is not supported, but we do not explicitely check for this --? David Marchand
[dpdk-dev] [PATCH v5 10/17] fm10k: add receive and tranmit function
Hi,
From: David Marchand [mailto:david.march...@6wind.com]
Sent: Friday, February 13, 2015 7:54 PM
To: Chen, Jing D
Cc: dev at dpdk.org; Zhang, Helin; Qiu, Michael; Neil Horman; Thomas Monjalon;
Shaw, Jeffrey B
Subject: Re: [PATCH v5 10/17] fm10k: add receive and tranmit function
On Fri, Feb 13, 2015 at 9:19 AM, Chen Jing D(Mark) mailto:jing.d.chen at intel.com>> wrote:
[snip]
+ if ((q->next_dd > q->next_trigger) || (alloc == 1)) {
+ ret = rte_mempool_get_bulk(q->mp,
+ (void **)&q->sw_ring[q->next_alloc],
+ q->alloc_thresh);
+
+ if (unlikely(ret != 0)) {
+ PMD_RX_LOG(ERR, "Failed to alloc mbuf");
rx_mbuf_alloc_failed++ ?
[Mark] Thanks, I?ll change that.
--
David Marchand
[dpdk-dev] [PATCH v5 12/17] fm10k: Add scatter receive function
Hi,
From: David Marchand [mailto:david.march...@6wind.com]
Sent: Friday, February 13, 2015 7:55 PM
To: Chen, Jing D
Cc: dev at dpdk.org; Zhang, Helin; Qiu, Michael; Neil Horman; Thomas Monjalon;
Shaw, Jeffrey B
Subject: Re: [PATCH v5 12/17] fm10k: Add scatter receive function
Hello,
On Fri, Feb 13, 2015 at 9:19 AM, Chen Jing D(Mark) mailto:jing.d.chen at intel.com>> wrote:
[snip]
+ if (unlikely(ret != 0)) {
+ PMD_RX_LOG(ERR, "Failed to alloc mbuf");
Idem mono segment.
rx_mbuf_alloc_failed++ ?
[Mark] Thanks, I?ll change that.
--
David Marchand
[dpdk-dev] [PATCH v5 15/17] fm10k: add PF and VF interrupt handling function
Hi, From: David Marchand [mailto:david.march...@6wind.com] Sent: Friday, February 13, 2015 7:42 PM To: Chen, Jing D Cc: dev at dpdk.org; Zhang, Helin; Qiu, Michael; Neil Horman; Thomas Monjalon; Shaw, Jeffrey B Subject: Re: [PATCH v5 15/17] fm10k: add PF and VF interrupt handling function Hello, On Fri, Feb 13, 2015 at 9:19 AM, Chen Jing D(Mark) mailto:jing.d.chen at intel.com>> wrote: [snip] + + /* Only INT 0 availiable, other 15 are reserved. */ available. [Mark] Thanks, I?ll change it. -- David Marchand
[dpdk-dev] [PATCH 2/2] doc: update release note for fm10k pmd driver
Hi Michael, > -Original Message- > From: Qiu, Michael > Sent: Friday, March 06, 2015 4:07 PM > To: Chen, Jing D; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 2/2] doc: update release note for fm10k > pmd driver > > On 3/6/2015 2:40 PM, Chen Jing D(Mark) wrote: > > From: "Chen Jing D(Mark)" > > > > Add feature list for fm10k driver. > > > > Signed-off-by: Chen Jing D(Mark) > > --- > > doc/guides/rel_notes/new_features.rst | 20 > > 1 files changed, 20 insertions(+), 0 deletions(-) > > > > diff --git a/doc/guides/rel_notes/new_features.rst > b/doc/guides/rel_notes/new_features.rst > > index 2993b1e..c08d5a8 100644 > > --- a/doc/guides/rel_notes/new_features.rst > > +++ b/doc/guides/rel_notes/new_features.rst > > @@ -58,4 +58,24 @@ New Features > > > > * Packet Distributor Sample Application > > > > +* Poll Mode Driver - PCIE host-interface of Intel Ethernet Switch > FM1 Series (librte_pmd_fm10k) > > + > > +* Basic Rx/Tx functions for PF/VF > > + > > +* Interrupt handling support for PF/VF > > + > > +* Per queue start/stop functions for PF/VF > > + > > +* Support Mailbox handling between PF/VF and PF/Switch Manager > > + > > +* Receive Side Scaling (RSS) for PF/VF > > + > > +* Scatter receive function for PF/VF > > + > > +* Reta update/query for PF/VF > > + > > +* VLAN filter set for PF > > + > > +* Link status query for PF/VF. > > Why only last has '.'? I think should be keep the same style. Thanks for your comments. > > Thanks, > Michael > > + > > For further features supported in this release, see Chapter 3 Supported > Features.
[dpdk-dev] [PATCH] librte_pmd_fm10k: Set pointer to NULL after free
> -Original Message-
> From: Qiu, Michael
> Sent: Friday, March 06, 2015 3:57 PM
> To: dev at dpdk.org
> Cc: Chen, Jing D; Qiu, Michael
> Subject: [PATCH] librte_pmd_fm10k: Set pointer to NULL after free
>
> It could be a potential not safe issue.
>
> Signed-off-by: Michael Qiu
> ---
> lib/librte_pmd_fm10k/fm10k_ethdev.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> index 07ea1e7..30962d3 100644
> --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> @@ -142,9 +142,12 @@ rx_queue_free(struct fm10k_rx_queue *q)
> if (q) {
> PMD_INIT_LOG(DEBUG, "Freeing rx queue %p", q);
> rx_queue_clean(q);
> - if (q->sw_ring)
> + if (q->sw_ring) {
> rte_free(q->sw_ring);
> + q->sw_ring = NULL;
> + }
> rte_free(q);
> + q = NULL;
> }
> }
>
> @@ -225,11 +228,16 @@ tx_queue_free(struct fm10k_tx_queue *q)
> if (q) {
> PMD_INIT_LOG(DEBUG, "Freeing tx queue %p", q);
> tx_queue_clean(q);
> - if (q->rs_tracker.list)
> + if (q->rs_tracker.list) {
> rte_free(q->rs_tracker.list);
> - if (q->sw_ring)
> + q->rs_tracker.list = NULL;
> + }
> + if (q->sw_ring) {
> rte_free(q->sw_ring);
> + q->sw_ring = NULL;
> + }
> rte_free(q);
> + q = NULL;
> }
> }
>
> --
> 1.9.3
Acked-by: Jing Chen
[dpdk-dev] [PATCH] fm10k: Fix queue start twice failed
Hi, Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, March 31, 2015 4:02 AM > To: Qiu, Michael > Cc: dev at dpdk.org; Chen, Jing D > Subject: Re: [dpdk-dev] [PATCH] fm10k: Fix queue start twice failed > > 2015-03-25 18:48, Michael Qiu: > > When use "port 0 rxq 0 start" in testpmd twice, the rx queue 0 on > > port 0 will failed to work. > > > > The root casue is the rxqctl enable bit need to reset if already > > enabled. > > Please try to reword this message to make it clear. > > > Signed-off-by: Michael Qiu > > --- > > lib/librte_pmd_fm10k/fm10k_ethdev.c | 56 +--- > - > > 1 file changed, 32 insertions(+), 24 deletions(-) > > Jing, should it enter in 2.0? Michael and I discussed and thought it's not the best solution to fix this issue. So, we'd like postpone this fix. Thanks! >
[dpdk-dev] [PATCH 1/2] examples/vhost: support new VMDQ API and new nic i40e
Hi,
> -Original Message-
> From: Xie, Huawei
> Sent: Thursday, November 13, 2014 6:34 AM
> To: dev at dpdk.org
> Cc: Chen, Jing D; Xie, Huawei
> Subject: [PATCH 1/2] examples/vhost: support new VMDQ API and new nic
> i40e
>
> In Niantic, if VMDQ mode is set, all queues are allocated to VMDQ in DPDK.
> In I40E, only configured part of continous queues are allocated to VMDQ.
> The rte_eth_dev_info structure is extened to provide VMDQ queue base,
> queue number, and VMDQ pool base information.
> This patch support the new VMDQ API in vhost example.
>
> FIXME in PMD:
> * added mac address will be flushed at rte_eth_dev_start.
> * we don't support selectively setting up queues well.
>
> Signed-off-by: Huawei Xie
> ---
> examples/vhost/main.c | 25 +++--
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index a93f7a0..2b1bf02 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -53,7 +53,7 @@
>
> #include "main.h"
>
> -#define MAX_QUEUES 128
> +#define MAX_QUEUES 256
>
> /* the maximum number of external ports supported */
> #define MAX_SUP_PORTS 1
> @@ -282,6 +282,9 @@ static struct rte_eth_conf vmdq_conf_default = {
> static unsigned lcore_ids[RTE_MAX_LCORE];
> static uint8_t ports[RTE_MAX_ETHPORTS];
> static unsigned num_ports = 0; /**< The number of ports specified in
> command line */
> +static uint16_t num_pf_queues, num_vmdq_queues;
> +static uint16_t vmdq_pool_base, vmdq_queue_base;
> +static uint16_t queues_per_pool;
>
> static const uint16_t external_pkt_default_vlan_tag = 2000;
> const uint16_t vlan_tags[] = {
> @@ -417,7 +420,6 @@ port_init(uint8_t port)
>
> /*configure the number of supported virtio devices based on VMDQ
> limits */
> num_devices = dev_info.max_vmdq_pools;
> - num_queues = dev_info.max_rx_queues;
>
> if (zero_copy) {
> rx_ring_size = num_rx_descriptor;
> @@ -437,10 +439,19 @@ port_init(uint8_t port)
> retval = get_eth_conf(&port_conf, num_devices);
> if (retval < 0)
> return retval;
> + /* NIC queues are divided into pf queues and vmdq queues. */
> + num_pf_queues = dev_info.max_rx_queues -
> dev_info.vmdq_queue_num;
> + queues_per_pool = dev_info.vmdq_queue_num /
> dev_info.max_vmdq_pools;
> + num_vmdq_queues = num_devices * queues_per_pool;
> + num_queues = num_pf_queues + num_vmdq_queues;
> + vmdq_queue_base = dev_info.vmdq_queue_base;
> + vmdq_pool_base = dev_info.vmdq_pool_base;
> + printf("pf queue num: %u, configured vmdq pool num: %u, each
> vmdq pool has %u queues\n",
> + num_pf_queues, num_devices, queues_per_pool);
>
> if (port >= rte_eth_dev_count()) return -1;
>
> - rx_rings = (uint16_t)num_queues,
> + rx_rings = (uint16_t)dev_info.max_rx_queues;
You removed line 'num_queues = dev_info.max_rx_queues' and calculate
'num_queues'
with another equation. I assume you thought it may not equals.
So, why you assign dev_info.max_rx_queues to rx_rings again? Won't it better to
use 'num_queues'
> /* Configure ethernet device. */
> retval = rte_eth_dev_configure(port, rx_rings, tx_rings, &port_conf);
> if (retval != 0)
> @@ -931,7 +942,8 @@ link_vmdq(struct vhost_dev *vdev, struct rte_mbuf
> *m)
> vdev->vlan_tag);
>
> /* Register the MAC address. */
> - ret = rte_eth_dev_mac_addr_add(ports[0], &vdev->mac_address,
> (uint32_t)dev->device_fh);
> + ret = rte_eth_dev_mac_addr_add(ports[0], &vdev->mac_address,
> + (uint32_t)dev->device_fh +
> vmdq_pool_base);
> if (ret)
> RTE_LOG(ERR, VHOST_DATA, "(%"PRIu64") Failed to add
> device MAC address to VMDQ\n",
> dev->device_fh);
> @@ -2602,7 +2614,7 @@ new_device (struct virtio_net *dev)
> ll_dev->vdev = vdev;
> add_data_ll_entry(&ll_root_used, ll_dev);
> vdev->vmdq_rx_q
> - = dev->device_fh * (num_queues / num_devices);
> + = dev->device_fh * queues_per_pool + vmdq_queue_base;
>
> if (zero_copy) {
> uint32_t index = vdev->vmdq_rx_q;
> @@ -2837,7 +2849,8 @@ MAIN(int argc, char *argv[])
> unsigned lcore_id, core_id = 0;
> unsigned nb_ports, valid_num_ports;
> int ret;
> - uint8_t portid, queue_id = 0;
> + uint8_t portid;
> + uint16_t queue_id;
> static pthread_t tid;
>
> /* init EAL */
> --
> 1.8.1.4
[dpdk-dev] [PATCH 2/2] examples/vhost: use factorized default Rx/Tx configuration
Hi,
> -Original Message-
> From: Xie, Huawei
> Sent: Thursday, November 13, 2014 6:34 AM
> To: dev at dpdk.org
> Cc: Chen, Jing D; Xie, Huawei
> Subject: [PATCH 2/2] examples/vhost: use factorized default Rx/Tx
> configuration
>
> Refer to Pablo's commit:
> "use factorized default Rx/Tx configuration
>
> For apps that were using default rte_eth_rxconf and rte_eth_txconf
> structures, these have been removed and now they are obtained by
> calling rte_eth_dev_info_get, just before setting up RX/TX queues."
>
> move zero copy's deferred start set up ahead.
>
> Signed-off-by: Huawei Xie
> ---
> examples/vhost/main.c | 78 +++--
> --
> 1 file changed, 22 insertions(+), 56 deletions(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 2b1bf02..fa36913 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -79,25 +79,6 @@
> + RTE_PKTMBUF_HEADROOM)
> #define MBUF_CACHE_SIZE_ZCP 0
>
> -/*
> - * RX and TX Prefetch, Host, and Write-back threshold values should be
> - * carefully set for optimal performance. Consult the network
> - * controller's datasheet and supporting DPDK documentation for guidance
> - * on how these parameters should be set.
> - */
> -#define RX_PTHRESH 8 /* Default values of RX prefetch threshold reg. */
> -#define RX_HTHRESH 8 /* Default values of RX host threshold reg. */
> -#define RX_WTHRESH 4 /* Default values of RX write-back threshold reg. */
> -
> -/*
> - * These default values are optimized for use with the Intel(R) 82599 10 GbE
> - * Controller and the DPDK ixgbe PMD. Consider using other values for other
> - * network controllers and/or network drivers.
> - */
> -#define TX_PTHRESH 36 /* Default values of TX prefetch threshold reg. */
> -#define TX_HTHRESH 0 /* Default values of TX host threshold reg. */
> -#define TX_WTHRESH 0 /* Default values of TX write-back threshold reg. */
> -
> #define MAX_PKT_BURST 32 /* Max burst size for RX/TX */
> #define BURST_TX_DRAIN_US 100/* TX drain every ~100us */
>
> @@ -217,32 +198,6 @@ static uint32_t burst_rx_retry_num =
> BURST_RX_RETRIES;
> /* Character device basename. Can be set by user. */
> static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
>
> -
> -/* Default configuration for rx and tx thresholds etc. */
> -static struct rte_eth_rxconf rx_conf_default = {
> - .rx_thresh = {
> - .pthresh = RX_PTHRESH,
> - .hthresh = RX_HTHRESH,
> - .wthresh = RX_WTHRESH,
> - },
> - .rx_drop_en = 1,
> -};
> -
> -/*
> - * These default values are optimized for use with the Intel(R) 82599 10 GbE
> - * Controller and the DPDK ixgbe/igb PMD. Consider using other values for
> other
> - * network controllers and/or network drivers.
> - */
> -static struct rte_eth_txconf tx_conf_default = {
> - .tx_thresh = {
> - .pthresh = TX_PTHRESH,
> - .hthresh = TX_HTHRESH,
> - .wthresh = TX_WTHRESH,
> - },
> - .tx_free_thresh = 0, /* Use PMD default values */
> - .tx_rs_thresh = 0, /* Use PMD default values */
> -};
> -
> /* empty vmdq configuration structure. Filled in programatically */
> static struct rte_eth_conf vmdq_conf_default = {
> .rxmode = {
> @@ -410,7 +365,9 @@ port_init(uint8_t port)
> {
> struct rte_eth_dev_info dev_info;
> struct rte_eth_conf port_conf;
> - uint16_t rx_rings, tx_rings;
> + struct rte_eth_rxconf *rxconf;
> + struct rte_eth_txconf *txconf;
> + int16_t rx_rings, tx_rings;
> uint16_t rx_ring_size, tx_ring_size;
> int retval;
> uint16_t q;
> @@ -418,6 +375,21 @@ port_init(uint8_t port)
> /* The max pool number from dev_info will be used to validate the
> pool number specified in cmd line */
> rte_eth_dev_info_get (port, &dev_info);
>
> + rxconf = &dev_info.default_rxconf;
> + txconf = &dev_info.default_txconf;
> + rxconf->rx_drop_en = 1;
> +
> + /*
> + * Zero copy defers queue RX/TX start to the time when guest
> + * finishes its startup and packet buffers from that guest are
> + * available.
> + */
> + if (zero_copy) {
> + rxconf->rx_deferred_start = 1;
> + rxconf->rx_drop_en = 0;
> + txconf->tx_deferred_start = 1;
> + }
> +
May I know why 'rx_drop_en' is cleared after 'zero_copy' set?
> /*configure the number of supported virtio devices based on VMDQ
> limits */
> num_devices = dev_
[dpdk-dev] [PATCH 1/2] examples/vhost: support new VMDQ API and new nic i40e
> -Original Message-
> From: Xie, Huawei
> Sent: Friday, November 14, 2014 2:31 PM
> To: Chen, Jing D; dev at dpdk.org
> Subject: RE: [PATCH 1/2] examples/vhost: support new VMDQ API and new
> nic i40e
>
>
>
> > -Original Message-
> > From: Chen, Jing D
> > Sent: Wednesday, November 12, 2014 10:58 PM
> > To: Xie, Huawei; dev at dpdk.org
> > Subject: RE: [PATCH 1/2] examples/vhost: support new VMDQ API and
> new nic
> > i40e
> >
> > Hi,
> >
> > > -Original Message-
> > > From: Xie, Huawei
> > > Sent: Thursday, November 13, 2014 6:34 AM
> > > To: dev at dpdk.org
> > > Cc: Chen, Jing D; Xie, Huawei
> > > Subject: [PATCH 1/2] examples/vhost: support new VMDQ API and new
> nic
> > > i40e
> > >
> > > In Niantic, if VMDQ mode is set, all queues are allocated to VMDQ in
> DPDK.
> > > In I40E, only configured part of continous queues are allocated to VMDQ.
> > > The rte_eth_dev_info structure is extened to provide VMDQ queue base,
> > > queue number, and VMDQ pool base information.
> > > This patch support the new VMDQ API in vhost example.
> > >
> > > FIXME in PMD:
> > > * added mac address will be flushed at rte_eth_dev_start.
> > > * we don't support selectively setting up queues well.
> > >
> > > Signed-off-by: Huawei Xie
> > > ---
> > > examples/vhost/main.c | 25 +++--
> > > 1 file changed, 19 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> > > index a93f7a0..2b1bf02 100644
> > > --- a/examples/vhost/main.c
> > > +++ b/examples/vhost/main.c
> > > @@ -53,7 +53,7 @@
> > >
> > > #include "main.h"
> > >
> > > -#define MAX_QUEUES 128
> > > +#define MAX_QUEUES 256
> > >
> > > /* the maximum number of external ports supported */
> > > #define MAX_SUP_PORTS 1
> > > @@ -282,6 +282,9 @@ static struct rte_eth_conf vmdq_conf_default = {
> > > static unsigned lcore_ids[RTE_MAX_LCORE];
> > > static uint8_t ports[RTE_MAX_ETHPORTS];
> > > static unsigned num_ports = 0; /**< The number of ports specified in
> > > command line */
> > > +static uint16_t num_pf_queues, num_vmdq_queues;
> > > +static uint16_t vmdq_pool_base, vmdq_queue_base;
> > > +static uint16_t queues_per_pool;
> > >
> > > static const uint16_t external_pkt_default_vlan_tag = 2000;
> > > const uint16_t vlan_tags[] = {
> > > @@ -417,7 +420,6 @@ port_init(uint8_t port)
> > >
> > > /*configure the number of supported virtio devices based on VMDQ
> > > limits */
> > > num_devices = dev_info.max_vmdq_pools;
> > > - num_queues = dev_info.max_rx_queues;
> > >
> > > if (zero_copy) {
> > > rx_ring_size = num_rx_descriptor;
> > > @@ -437,10 +439,19 @@ port_init(uint8_t port)
> > > retval = get_eth_conf(&port_conf, num_devices);
> > > if (retval < 0)
> > > return retval;
> > > + /* NIC queues are divided into pf queues and vmdq queues. */
> > > + num_pf_queues = dev_info.max_rx_queues -
> > > dev_info.vmdq_queue_num;
> > > + queues_per_pool = dev_info.vmdq_queue_num /
> > > dev_info.max_vmdq_pools;
> > > + num_vmdq_queues = num_devices * queues_per_pool;
> > > + num_queues = num_pf_queues + num_vmdq_queues;
> > > + vmdq_queue_base = dev_info.vmdq_queue_base;
> > > + vmdq_pool_base = dev_info.vmdq_pool_base;
> > > + printf("pf queue num: %u, configured vmdq pool num: %u, each
> > > vmdq pool has %u queues\n",
> > > + num_pf_queues, num_devices, queues_per_pool);
> > >
> > > if (port >= rte_eth_dev_count()) return -1;
> > >
> > > - rx_rings = (uint16_t)num_queues,
> > > + rx_rings = (uint16_t)dev_info.max_rx_queues;
> >
> > You removed line 'num_queues = dev_info.max_rx_queues' and calculate
> > 'num_queues'
> > with another equation. I assume you thought it may not equals.
> > So, why you assign dev_info.max_rx_queues to rx_rings again? Won't it
> better to
> > use 'num_queues'
>
> Actually they are the same here.
> We use max_rx_queues just to say that we initialize all queues rather than
> part of
> queues.
> If all PMDs(1G,10G,i40e) supports selectively initializing queues, then we
> could only initialize
&
[dpdk-dev] [PATCH 0/3] fix of lsc interrupt in i40e PF
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Helin Zhang > Sent: Wednesday, September 17, 2014 3:54 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH 0/3] fix of lsc interrupt in i40e PF > > The patches include the fix for link status change interrupt > in i40e PF, and code style fixes. > > Helin Zhang (3): > i40e: renaming some local variables > i40e: rework of PF interrupt cause enable flags processing > i40e: fix of interrupt based link status change > > lib/librte_pmd_i40e/i40e_ethdev.c | 174 ++- > --- > 1 file changed, 122 insertions(+), 52 deletions(-) > > -- > 1.8.1.4 Acked-by: Jing Chen
[dpdk-dev] [PATCH] lib/librte_pmd_i40e: i40e vlan filter set fix
> -Original Message-
> From: Xie, Huawei
> Sent: Sunday, September 28, 2014 1:49 PM
> To: dev at dpdk.org
> Cc: Xie, Huawei; Chen, Jing D; Zhang, Helin
> Subject: [PATCH] lib/librte_pmd_i40e: i40e vlan filter set fix
>
> the right shift bits should be 5 rather than 4.
> vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F)
>
> Signed-off-by: Huawei Xie
> CC: Jing Chen
> CC: Helin Zhang
>
> ---
> lib/librte_pmd_i40e/i40e_ethdev.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 9009bd4..9c9d831 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -3786,14 +3786,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,
> {
> uint32_t vid_idx, vid_bit;
>
> -#define UINT32_BIT_MASK 0x1F
> -#define VALID_VLAN_BIT_MASK 0xFFF
> /* VFTA is 32-bits size array, each element contains 32 vlan bits, Find
> the
>* element first, then find the bits it belongs to
>*/
> - vid_idx = (uint32_t) ((vlan_id & VALID_VLAN_BIT_MASK) >>
> - sizeof(uint32_t));
> - vid_bit = (uint32_t) (1 << (vlan_id & UINT32_BIT_MASK));
> + vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
> + vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
>
> if (on)
> vsi->vfta[vid_idx] |= vid_bit;
> --
> 1.8.1.4
Please try to use macro to replace numbers.
[dpdk-dev] [PATCH 0/4] support control packet filter on Fortville
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jingjing Wu > Sent: Thursday, September 25, 2014 2:59 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH 0/4] support control packet filter on Fortville > > The patch set enables control packet filter on Fortville. > Control packet filter can assign packet to specific destination by filtering > with > mac address and ethertype or only ethertype. > > It mainly includes: > - Use new filter mechanism discussed at > http://dpdk.org/ml/archives/dev/2014-September/005179.html. > - control packet filter implementation in i40e pmd driver > > jingjing.wu (4): > new filter APIs definition > define CTRL_PKT filter type and its structure > ctrl_pkt filter implementation in i40e pmd driver > add commands for control packet filter > > app/test-pmd/cmdline.c| 149 > +++ > lib/librte_ether/Makefile | 1 + > lib/librte_ether/rte_eth_ctrl.h | 102 > lib/librte_ether/rte_ethdev.c | 32 > lib/librte_ether/rte_ethdev.h | 44 +++ > lib/librte_pmd_i40e/i40e_ethdev.c | 161 > ++ > 6 files changed, 489 insertions(+) > create mode 100644 lib/librte_ether/rte_eth_ctrl.h > > -- > 1.8.1.4 Acked-by: Jing Chen
[dpdk-dev] [PATCH 0/6] i40e VMDQ support
Hi Thomas, Any comments with below patch? -Original Message- From: Chen, Jing D Sent: Tuesday, September 23, 2014 9:14 PM To: dev at dpdk.org Cc: Chen, Jing D Subject: [PATCH 0/6] i40e VMDQ support From: "Chen Jing D(Mark)" Define extra VMDQ arguments to expand VMDQ configuration. This also includes change in igb and ixgbe PMD driver. In the meanwhile, fix 2 defects in rte_ether library. Add full VMDQ support in i40e PMD driver. renamed some functions, setup VMDQ VSI after it's enabled in application. It also make some improvement on macaddr add/delete to support setting multiple macaddr for single or multiple pools. Finally, change i40e rx/tx_queue_setup and dev_start/stop functions to configure/switch queues belonging to VMDQ pools. Chen Jing D(Mark) (6): ether: enhancement for VMDQ support igb: change for VMDQ arguments expansion ixgbe: change for VMDQ arguments expansion i40e: add VMDQ support i40e: macaddr add/del enhancement i40e: Add full VMDQ pools support config/common_linuxapp |1 + lib/librte_ether/rte_ethdev.c | 12 +- lib/librte_ether/rte_ethdev.h | 39 ++- lib/librte_pmd_e1000/igb_ethdev.c |3 + lib/librte_pmd_i40e/i40e_ethdev.c | 509 ++- lib/librte_pmd_i40e/i40e_ethdev.h | 21 ++- lib/librte_pmd_i40e/i40e_rxtx.c | 125 +++-- lib/librte_pmd_ixgbe/ixgbe_ethdev.c |1 + 8 files changed, 537 insertions(+), 174 deletions(-) -- 1.7.7.6
[dpdk-dev] [PATCH 1/6] ether: enhancement for VMDQ support
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, October 14, 2014 10:10 PM
> To: Chen, Jing D
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/6] ether: enhancement for VMDQ support
>
> 2014-09-23 21:14, Chen Jing D:
> > The change includes several parts:
> > 1. Clear pool bitmap when trying to remove specific MAC.
> > 2. Define RSS, DCB and VMDQ flags to combine rx_mq_mode.
> > 3. Use 'struct' to replace 'union', which to expand the rx_adv_conf
> >arguments to better support RSS, DCB and VMDQ.
> > 4. Fix bug in rte_eth_dev_config_restore function, which will restore
> >all MAC address to default pool.
> > 5. Define additional 3 arguments for better VMDQ support.
> >
> > Signed-off-by: Chen Jing D(Mark)
> > Acked-by: Konstantin Ananyev
> > Acked-by: Jingjing Wu
> > Acked-by: Jijiang Liu
> > Acked-by: Huawei Xie
>
> Whaou, there were a lot of reviewers!
> The patch should be really clean. Let's see :)
First time I saw you are so humorous. :)
>
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > /* add address to the hardware */
> > - if (*dev->dev_ops->mac_addr_add)
> > + if (*dev->dev_ops->mac_addr_add &&
> > + dev->data->mac_pool_sel[i] & (1ULL << pool))
> > (*dev->dev_ops->mac_addr_add)(dev, &addr, i,
> pool);
>
> > + /* Update pool bitmap in NIC data structure */
> > + dev->data->mac_pool_sel[index] = 0;
>
> Reset is a better word than "Update" in this case.
> But do we really need a comment for that?
Accept.
>
> > +#define ETH_MQ_RX_RSS_FLAG 0x1
> > +#define ETH_MQ_RX_DCB_FLAG 0x2
> > +#define ETH_MQ_RX_VMDQ_FLAG 0x4
>
> Need a comment to know where these flags can be used.
Accept.
>
> > enum rte_eth_rx_mq_mode {
> > - ETH_MQ_RX_NONE = 0, /**< None of DCB,RSS or VMDQ mode */
> > -
> > - ETH_MQ_RX_RSS, /**< For RX side, only RSS is on */
> > - ETH_MQ_RX_DCB, /**< For RX side,only DCB is on. */
> > - ETH_MQ_RX_DCB_RSS, /**< Both DCB and RSS enable */
> > -
> > - ETH_MQ_RX_VMDQ_ONLY, /**< Only VMDQ, no RSS nor DCB */
> > - ETH_MQ_RX_VMDQ_RSS, /**< RSS mode with VMDQ */
> > - ETH_MQ_RX_VMDQ_DCB, /**< Use VMDQ+DCB to route traffic to
> queues */
> > - ETH_MQ_RX_VMDQ_DCB_RSS, /**< Enable both VMDQ and DCB in
> VMDq */
> > + /**< None of DCB,RSS or VMDQ mode */
> > + ETH_MQ_RX_NONE = 0,
> > +
> > + /**< For RX side, only RSS is on */
> > + ETH_MQ_RX_RSS = ETH_MQ_RX_RSS_FLAG,
> > + /**< For RX side,only DCB is on. */
> > + ETH_MQ_RX_DCB = ETH_MQ_RX_DCB_FLAG,
> > + /**< Both DCB and RSS enable */
> > + ETH_MQ_RX_DCB_RSS = ETH_MQ_RX_RSS_FLAG |
> ETH_MQ_RX_DCB_FLAG,
> > +
> > + /**< Only VMDQ, no RSS nor DCB */
> > + ETH_MQ_RX_VMDQ_ONLY = ETH_MQ_RX_VMDQ_FLAG,
> > + /**< RSS mode with VMDQ */
> > + ETH_MQ_RX_VMDQ_RSS = ETH_MQ_RX_RSS_FLAG |
> ETH_MQ_RX_VMDQ_FLAG,
> > + /**< Use VMDQ+DCB to route traffic to queues */
> > + ETH_MQ_RX_VMDQ_DCB = ETH_MQ_RX_VMDQ_FLAG |
> ETH_MQ_RX_DCB_FLAG,
> > + /**< Enable both VMDQ and DCB in VMDq */
> > + ETH_MQ_RX_VMDQ_DCB_RSS = ETH_MQ_RX_RSS_FLAG |
> ETH_MQ_RX_DCB_FLAG |
> > +ETH_MQ_RX_VMDQ_FLAG,
> > };
>
> Why not simply remove all these combinations and keep only flags?
> Please keep it simple.
One reason is back-compatibility.
Another reason is not all NIC driver support all the combined modes, only
limited sets
driver supported. Under this condition, it's better to use the combination
definition
(VMDQ_DCB, DCB_RSS, etc) to let driver check whether it supports.
>
> > + /**< Specify the queue range belongs to VMDQ pools if VMDQ
> applicable */
> > + uint16_t vmdq_queue_base;
> > + uint16_t vmdq_queue_num;
>
> If comment is before, it should be /** not /**<.
Accept.
>
> > + uint16_t vmdq_pool_base; /** < Specify the start pool ID of VMDQ
> pools */
>
> There is a typo with the space --^
> Please, when writing comments, ask yourself if each word is required
> and how it can be shorter.
> Example here: /**< first ID of VMDQ pools */
>
> Conclusion: NACK
> There are only few typos and minor things but it would help to have more
> careful reviews. Having a list of people at the beginning of the patch
> didn't help in this case.
I listed all the code reviewers out to reduce their workload to reply the email,
not mean to make it easier to be applied.
>
> Thanks for your attention
> --
> Thomas
[dpdk-dev] [PATCH 5/6] i40e: macaddr add/del enhancement
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, October 14, 2014 10:25 PM
> To: Chen, Jing D
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 5/6] i40e: macaddr add/del enhancement
>
> 2014-09-23 21:14, Chen Jing D:
> > + PMD_DRV_LOG(ERR, "VMDQ not %s, can't set mac to
> pool %u\n",
> > + pf->flags | I40E_FLAG_VMDQ ? "configured" :
> "enabled",
> > + pool);
> [...]
> > - if (ret != I40E_SUCCESS) {
> > - PMD_DRV_LOG(ERR, "Failed to write mac address");
> > + if (pool > pf->nb_cfg_vmdq_vsi) {
> > + PMD_DRV_LOG(ERR, "Pool number %u invalid. Max pool
> is %u\n",
> > + pool, pf->nb_cfg_vmdq_vsi);
> [...]
> > - PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter");
> > + PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter\n");
> [...]
> > + PMD_DRV_LOG(ERR, "Failed to remove
> MACVLAN filter\n");
>
> I'm pretty sure you rebased this patch and solved the conflicts without
> updating your patch accordingly. Indeed carriage returns have been removed
> from logs recently.
> Hint: rebase conflicts are really often meaningful ;)
>
Thanks for your suggestion.
> --
> Thomas
[dpdk-dev] [PATCH 1/6] ether: enhancement for VMDQ support
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, October 15, 2014 4:11 PM
> To: Chen, Jing D
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/6] ether: enhancement for VMDQ support
>
> 2014-10-15 06:59, Chen, Jing D:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > enum rte_eth_rx_mq_mode {
> > > > - ETH_MQ_RX_NONE = 0, /**< None of DCB,RSS or VMDQ mode */
> > > > -
> > > > - ETH_MQ_RX_RSS, /**< For RX side, only RSS is on */
> > > > - ETH_MQ_RX_DCB, /**< For RX side,only DCB is on. */
> > > > - ETH_MQ_RX_DCB_RSS, /**< Both DCB and RSS enable */
> > > > -
> > > > - ETH_MQ_RX_VMDQ_ONLY, /**< Only VMDQ, no RSS nor DCB */
> > > > - ETH_MQ_RX_VMDQ_RSS, /**< RSS mode with VMDQ */
> > > > - ETH_MQ_RX_VMDQ_DCB, /**< Use VMDQ+DCB to route traffic to
> > > queues */
> > > > - ETH_MQ_RX_VMDQ_DCB_RSS, /**< Enable both VMDQ and DCB in
> > > VMDq */
> > > > + /**< None of DCB,RSS or VMDQ mode */
> > > > + ETH_MQ_RX_NONE = 0,
> > > > +
> > > > + /**< For RX side, only RSS is on */
> > > > + ETH_MQ_RX_RSS = ETH_MQ_RX_RSS_FLAG,
> > > > + /**< For RX side,only DCB is on. */
> > > > + ETH_MQ_RX_DCB = ETH_MQ_RX_DCB_FLAG,
> > > > + /**< Both DCB and RSS enable */
> > > > + ETH_MQ_RX_DCB_RSS = ETH_MQ_RX_RSS_FLAG |
> > > ETH_MQ_RX_DCB_FLAG,
> > > > +
> > > > + /**< Only VMDQ, no RSS nor DCB */
> > > > + ETH_MQ_RX_VMDQ_ONLY = ETH_MQ_RX_VMDQ_FLAG,
> > > > + /**< RSS mode with VMDQ */
> > > > + ETH_MQ_RX_VMDQ_RSS = ETH_MQ_RX_RSS_FLAG |
> > > ETH_MQ_RX_VMDQ_FLAG,
> > > > + /**< Use VMDQ+DCB to route traffic to queues */
> > > > + ETH_MQ_RX_VMDQ_DCB = ETH_MQ_RX_VMDQ_FLAG |
> > > ETH_MQ_RX_DCB_FLAG,
> > > > + /**< Enable both VMDQ and DCB in VMDq */
> > > > + ETH_MQ_RX_VMDQ_DCB_RSS = ETH_MQ_RX_RSS_FLAG |
> > > ETH_MQ_RX_DCB_FLAG |
> > > > +ETH_MQ_RX_VMDQ_FLAG,
> > > > };
> > >
> > > Why not simply remove all these combinations and keep only flags?
> > > Please keep it simple.
> >
> > One reason is back-compatibility.
>
> I understand but I think we should prefer cleanup.
> As there is no way to advertise deprecation of flags, it should be
> simply removed.
>
> > Another reason is not all NIC driver support all the combined modes, only
> limited sets
> > driver supported. Under this condition, it's better to use the combination
> definition
> > (VMDQ_DCB, DCB_RSS, etc) to let driver check whether it supports.
>
> Driver can do the same checks with simple flags and it's probably simpler
> (e.g. a driver which doesn't support VMDQ had no need to check all VMDQ
> combinations).
Below is an example with the change in ixgbe_dcb_hw_configure(). DCB only
can be enabled in case DCB or VMDQ_DCB is selected.
Before the change:
switch(dev->data->dev_conf.rxmode.mq_mode){
case ETH_MQ_RX_VMDQ_DCB:
.
case ETH_MQ_RX_DCB:
.
default:
FAILED.
}
With the change, it will be:
switch(dev->data->dev_conf.rxmode.mq_mode){
case ETH_MQ_RX_VMDQ_FLAG | ETH_MQ_RX_DCB_FLAG:
.
case ETH_MQ_RX_DCB_FLAG:
.
Default:
FAILED
}
Won't it look weird for reading? In fact, it's more complex in
rte_eth_dev_check_mq_mode(),
With the change, the code will look weird.
In fact, I don't see benefit with the change to old code. New PMD driver can
use simple flag while
old driver (IXGBE/IGB) can use original definition.
>
> > > There are only few typos and minor things but it would help to have more
> > > careful reviews. Having a list of people at the beginning of the patch
> > > didn't help in this case.
> >
> > I listed all the code reviewers out to reduce their workload to reply the
> email,
> > not mean to make it easier to be applied.
>
> I have no problem with listing of reviewers when submitting patches.
> To say more, I prefer you list them by yourself and you add new reviewers
> when sending new versions of the patchset.
> But I would like reviewers to be more careful. They are especially useful to
> discuss design choices and check typos.
> Having reviewer give credits to the patch only if we are confident that the
> review task is generally seriously achieved.
>
> --
> Thomas
[dpdk-dev] [PATCH 0/2] fix of configuring inside NIC RX interrupt
> -Original Message- > From: Zhang, Helin > Sent: Wednesday, October 29, 2014 11:43 AM > To: dev at dpdk.org > Cc: Cao, Waterman; Chen, Jing D; Zhang, Helin > Subject: [PATCH 0/2] fix of configuring inside NIC RX interrupt > > Inside NIC RX interrupt is needed for single RX descriptor > write back. The fix is to correct the wrong configuration > of register 'I40E_QINT_RQCTL'. In addition, several code > style fixes are added. > Note that interrupt will be inside NIC only, that means it > will never be reported outside NIC hardware. > > Helin Zhang (2): > i40e: code style fix > i40e: fix of configuring inside NIC RX interrupt > > lib/librte_pmd_i40e/i40e_ethdev.c | 23 +-- > 1 file changed, 13 insertions(+), 10 deletions(-) > > -- > 1.8.1.4 Acked-by: Jing Chen
[dpdk-dev] [PATCH] fm10k: add Intel Boulder Rapid NIC support
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Michael Qiu > Sent: Friday, September 25, 2015 10:31 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] fm10k: add Intel Boulder Rapid NIC support > > Boulder Rapid is Intel new NIC within fm10k family. > This patch make DPDK driver support this new NIC. > > Signed-off-by: Michael Qiu Acked-by : Jing Chen
[dpdk-dev] [PATCH 2/5] fm10k: enable Rx queue interrupts for PF and VF
Hi,
Best Regards,
Mark
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shaopeng He
> Sent: Friday, September 25, 2015 1:37 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH 2/5] fm10k: enable Rx queue interrupts for PF
> and VF
>
> The patch does below things for fm10k PF and VF:
> - Setup NIC to generate MSI-X interrupts
> - Set the RXINT register to map interrupt causes to vectors
> - Implement interrupt enable/disable functions
The description is too simple, can you help to extend?
Besides that, there are complicated changes in this patch.
Can you help you split it to several smaller ones for better understanding?
>
> Signed-off-by: Shaopeng He
> ---
> drivers/net/fm10k/fm10k_ethdev.c | 147
> +--
> 1 file changed, 140 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index a82cd59..6648934 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>
> static int
> +fm10k_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t
> queue_id)
> +{
> + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> + /* Enable ITR */
> + if (hw->mac.type == fm10k_mac_pf)
> + FM10K_WRITE_REG(hw, FM10K_ITR(Q2V(dev, queue_id)),
> + FM10K_ITR_AUTOMASK |
> FM10K_ITR_MASK_CLEAR);
> + else
> + FM10K_WRITE_REG(hw, FM10K_VFITR(Q2V(dev, queue_id)),
> + FM10K_ITR_AUTOMASK |
> FM10K_ITR_MASK_CLEAR);
> + rte_intr_enable(&dev->pci_dev->intr_handle);
> + return 0;
> +}
> +
> +static int
> +fm10k_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t
> queue_id)
> +{
> + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> + /* Disable ITR */
> + if (hw->mac.type == fm10k_mac_pf)
> + FM10K_WRITE_REG(hw, FM10K_ITR(Q2V(dev, queue_id)),
> + FM10K_ITR_MASK_SET);
> + else
> + FM10K_WRITE_REG(hw, FM10K_VFITR(Q2V(dev, queue_id)),
> + FM10K_ITR_MASK_SET);
In previous enable function, you'll use rte_intr_enable() to enable interrupt,
but
You needn't disable it in this function?
> + return 0;
> +}
> +
> +static int
> +fm10k_dev_rxq_interrupt_setup(struct rte_eth_dev *dev)
> +{
> + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> + struct rte_intr_handle *intr_handle = &dev->pci_dev->intr_handle;
> + uint32_t intr_vector, vec;
> + uint16_t queue_id;
> + int result = 0;
> +
> + /* fm10k needs interrupt for mailbox
> + * so igb_uio is not supported for rx interrupt
> + */
I guess you'll support both igb_uio and VFIO, RX interrupt mode only enabled in
case VFIO is used.
I suggest you add more comments here for better understanding.
> + if (!rte_intr_cap_multiple(intr_handle) ||
> + dev->data->dev_conf.intr_conf.rxq == 0)
> + return result;
> +
> + intr_vector = dev->data->nb_rx_queues;
> +
> + /* disable interrupt first */
> + rte_intr_disable(&dev->pci_dev->intr_handle);
> + if (hw->mac.type == fm10k_mac_pf)
> + fm10k_dev_disable_intr_pf(dev);
> + else
> + fm10k_dev_disable_intr_vf(dev);
> +
> + if (rte_intr_efd_enable(intr_handle, intr_vector)) {
> + PMD_INIT_LOG(ERR, "Failed to init event fd");
> + result = -EIO;
> + }
> +
> + if (rte_intr_dp_is_en(intr_handle) && !result) {
> + intr_handle->intr_vec = rte_zmalloc("intr_vec",
> + dev->data->nb_rx_queues * sizeof(int), 0);
> + if (intr_handle->intr_vec) {
> + for (queue_id = 0, vec = RX_VEC_START;
> + queue_id < dev->data-
> >nb_rx_queues;
> + queue_id++) {
> + intr_handle->intr_vec[queue_id] = vec;
> + if (vec < intr_handle->nb_efd - 1 +
> RX_VEC_START)
> + vec++;
No "else" to handle exceptional case?
> + }
> + } else {
> + PMD_INIT_LOG(ERR, "Failed to allocate %d
> rx_queues"
> + " intr_vec", dev->data->nb_rx_queues);
> + result = -ENOMEM;
> + }
> + }
> +
> + if (hw->mac.type == fm10k_mac_pf)
> + fm10k_dev_enable_intr_pf(dev);
> + else
> + fm10k_dev_enable_intr_vf(dev);
> + rte_intr_enable(&dev->pci_dev->intr_handle);
> + hw->mac.ops.update_int_moderator(hw);
> + return result;
> +}
> +
> +static int
> fm10k_dev_handle_fault(struct fm10k_hw *hw, uint32_t eicr)
> {
> struct fm10k_fault fault;
> @@ -2050,6 +2181,8 @@ static const struct eth_dev_ops
> fm10k_eth_dev_ops = {
> .tx_queue_setup = fm
[dpdk-dev] [PATCH v2 03/16] fm10k: Add a new func to initialize all parameters
Hi, Stephen,
Best Regards,
Mark
> -Original Message-
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Thursday, October 22, 2015 11:58 PM
> To: Chen, Jing D
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 03/16] fm10k: Add a new func to initialize
> all parameters
>
> On Thu, 22 Oct 2015 17:44:51 +0800
> "Chen Jing D(Mark)" wrote:
>
> > +static void
> > +fm10k_params_init(struct rte_eth_dev *dev)
> > +{
> > + struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + struct fm10k_dev_info *info = FM10K_DEV_PRIVATE_TO_INFO(dev);
> > + /* Inialize bus info. Normally we would call fm10k_get_bus_info(),
> but
> > +* there is no way to get link status without reading BAR4. Until this
> > +* works, assume we have maximum bandwidth.
> > +* @todo - fix bus info
>
> Minor nit. I would prefer that DPDK follow current Linux kernel
> style which is to always have a blank line after declarations.
> This improves readability.
>
Thanks the comments! I'll change accordingly.
> I.e:
>
> static void
> fm10k_params_init(struct rte_eth_dev *dev)
> {
> struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> struct fm10k_dev_info *info = FM10K_DEV_PRIVATE_TO_INFO(dev);
>
> /* Inialize bus info. Normally we would call fm10k_get_bus_info(),
> but
>* there is no way to get link status without reading BAR4. Until this
>* works, assume we have maximum bandwidth.
>* @todo - fix bus info
[dpdk-dev] [PATCH v2 01/16] fm10k: add new vPMD file
Hi, Stephen, Best Regards, Mark > -Original Message- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Thursday, October 22, 2015 11:59 PM > To: Chen, Jing D > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 01/16] fm10k: add new vPMD file > > On Thu, 22 Oct 2015 17:44:49 +0800 > "Chen Jing D(Mark)" wrote: > > > +#ifndef __INTEL_COMPILER > > +#pragma GCC diagnostic ignored "-Wcast-qual" > > +#endif > > Since this is new code, can't you make it work correctly > with Gcc. Rather than turning off a useful diagnostic. This macro is necessary for later SSE functions or I'll have to add some Un-necessary cast to avoid compile failure. I can add it in later patch. But it will have to show up anyway, right?
[dpdk-dev] [PATCH v2 01/16] fm10k: add new vPMD file
Hi, Bruce, Best Regards, Mark > -Original Message- > From: Richardson, Bruce > Sent: Friday, October 23, 2015 6:01 PM > To: Chen, Jing D > Cc: Stephen Hemminger; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 01/16] fm10k: add new vPMD file > > On Fri, Oct 23, 2015 at 08:39:56AM +, Chen, Jing D wrote: > > Hi, Stephen, > > > > Best Regards, > > Mark > > > > > > > -Original Message- > > > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > > > Sent: Thursday, October 22, 2015 11:59 PM > > > To: Chen, Jing D > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v2 01/16] fm10k: add new vPMD file > > > > > > On Thu, 22 Oct 2015 17:44:49 +0800 > > > "Chen Jing D(Mark)" wrote: > > > > > > > +#ifndef __INTEL_COMPILER > > > > +#pragma GCC diagnostic ignored "-Wcast-qual" > > > > +#endif > > > > > > Since this is new code, can't you make it work correctly > > > with Gcc. Rather than turning off a useful diagnostic. > > > > This macro is necessary for later SSE functions or I'll have to add some > > Un-necessary cast to avoid compile failure. > > > > I can add it in later patch. But it will have to show up anyway, right? > > Actually, casting won't make the warnings go away either. You'll always get a > warning about removing the volatile from the structure members when you > pass > them to the intrinsic functions. Only option is to temporarily disable the > warning > [or else write your own versions of the intrinsics that do support volatiles > :-)] > > /Bruce Thanks for the clarifications. :)
[dpdk-dev] [PATCH v2 06/16] fm10k: add Vector RX function
Hi, Steve,
Best Regards,
Mark
> -Original Message-
> From: Liang, Cunming
> Sent: Tuesday, October 27, 2015 1:25 PM
> To: Chen, Jing D; dev at dpdk.org
> Cc: Tao, Zhe; He, Shaopeng; Ananyev, Konstantin; Richardson, Bruce
> Subject: Re: [PATCH v2 06/16] fm10k: add Vector RX function
>
> Hi,
>
> On 10/22/2015 5:44 PM, Chen Jing D(Mark) wrote:
> > From: "Chen Jing D(Mark)"
> >
> > Add func fm10k_recv_raw_pkts_vec to parse raw packets, in which
> > includes possible chained packets.
> > Add func fm10k_recv_pkts_vec to receive single mbuf packet.
> >
> > Signed-off-by: Chen Jing D(Mark)
> > ---
> > drivers/net/fm10k/fm10k.h |1 +
> > drivers/net/fm10k/fm10k_rxtx_vec.c | 196
>
> > 2 files changed, 197 insertions(+), 0 deletions(-)
> [...]
> > + /* mask to shuffle from desc. to mbuf */
> > + shuf_msk = _mm_set_epi8(
> > + 7, 6, 5, 4, /* octet 4~7, 32bits rss */
> > + 15, 14, /* octet 14~15, low 16 bits vlan_macip */
> > + 13, 12, /* octet 12~13, 16 bits data_len */
> > + 0xFF, 0xFF, /* skip high 16 bits pkt_len, zero out */
> > + 13, 12, /* octet 12~13, low 16 bits pkt_len */
> > + 0xFF, 0xFF, /* skip high 16 bits pkt_type */
> > + 0xFF, 0xFF /* Skip pkt_type field in shuffle operation */
> > + );
> > +
> > + /* Cache is empty -> need to scan the buffer rings, but first move
> > +* the next 'n' mbufs into the cache
> > +*/
> > + mbufp = &rxq->sw_ring[next_dd];
> > +
> > + /* A. load 4 packet in one loop
> > +* [A*. mask out 4 unused dirty field in desc]
> > +* B. copy 4 mbuf point from swring to rx_pkts
> > +* C. calc the number of DD bits among the 4 packets
> > +* [C*. extract the end-of-packet bit, if requested]
> > +* D. fill info. from desc to mbuf
> > +*/
> > + for (pos = 0, nb_pkts_recd = 0; pos < nb_pkts;
> It's necessary to floor align the nb_pkts into RTE_FM10K_DESCS_PER_LOOP,
> otherwise it may exceed the rx_pkts array.
> e.g. nb_pkts is 6, it executes twice in the loop which has chance to get
> 8 packets done, but rx_pkts only expect 6 packets.
You are right. I'll change accordingly.
> >
> > + pos += RTE_FM10K_DESCS_PER_LOOP,
> > + rxdp += RTE_FM10K_DESCS_PER_LOOP) {
> > + __m128i descs0[RTE_FM10K_DESCS_PER_LOOP];
> > + __m128i pkt_mb1, pkt_mb2, pkt_mb3, pkt_mb4;
> > + __m128i zero, staterr, sterr_tmp1, sterr_tmp2;
> > + __m128i mbp1, mbp2; /* two mbuf pointer in one XMM reg.
> */
> > +
> >
[dpdk-dev] [PATCH v2 08/16] fm10k: add Vector RX scatter function
Hi, Steve,
Best Regards,
Mark
> -Original Message-
> From: Liang, Cunming
> Sent: Tuesday, October 27, 2015 1:28 PM
> To: Chen, Jing D; dev at dpdk.org
> Cc: Tao, Zhe; He, Shaopeng; Ananyev, Konstantin; Richardson, Bruce
> Subject: Re: [PATCH v2 08/16] fm10k: add Vector RX scatter function
>
> Hi,
>
> On 10/22/2015 5:44 PM, Chen Jing D(Mark) wrote:
> > From: "Chen Jing D(Mark)"
> >
> > Add func fm10k_recv_scattered_pkts_vec to receive chained packets
> > with SSE instructions.
> >
> > Signed-off-by: Chen Jing D(Mark)
> > ---
> > drivers/net/fm10k/fm10k.h |2 +
> > drivers/net/fm10k/fm10k_rxtx_vec.c | 88
>
> > 2 files changed, 90 insertions(+), 0 deletions(-)
> >
> [...]
> > +
> > +/*
> > + * vPMD receive routine that reassembles scattered packets
> > + *
> > + * Notice:
> > + * - don't support ol_flags for rss and csum err
> > + * - nb_pkts < RTE_IXGBE_DESCS_PER_LOOP, just return no packet
> > + * - nb_pkts > RTE_IXGBE_MAX_RX_BURST, only scan
> RTE_IXGBE_MAX_RX_BURST
> > + * numbers of DD bit
> In order to make sure nb_pkts > RTE_IXGBE_MAX_RX_BURST, it's necessary
> to do RTE_MIN().
I'll remove the improper comments. In func fm10k_recv_raw_pkts_vec, it will use
nb_pkts as index to iterate properly.
After then, below func will use actual received packet size nb_bufs as index to
iterate.
So, I think RTE_MIN() is not necessary?
> > + * - floor align nb_pkts to a RTE_IXGBE_DESC_PER_LOOP power-of-two
> > + */
> > +uint16_t
> > +fm10k_recv_scattered_pkts_vec(void *rx_queue,
> > + struct rte_mbuf **rx_pkts,
> > + uint16_t nb_pkts)
> > +{
> > + struct fm10k_rx_queue *rxq = rx_queue;
> > + uint8_t split_flags[RTE_FM10K_MAX_RX_BURST] = {0};
> > + unsigned i = 0;
> > +
> > + /* get some new buffers */
> > + uint16_t nb_bufs = fm10k_recv_raw_pkts_vec(rxq, rx_pkts, nb_pkts,
> > + split_flags);
> > + if (nb_bufs == 0)
> > + return 0;
> > +
> > + /* happy day case, full burst + no packets to be joined */
> > + const uint64_t *split_fl64 = (uint64_t *)split_flags;
> > + if (rxq->pkt_first_seg == NULL &&
> > + split_fl64[0] == 0 && split_fl64[1] == 0 &&
> > + split_fl64[2] == 0 && split_fl64[3] == 0)
> > + return nb_bufs;
> > +
> > + /* reassemble any packets that need reassembly*/
> > + if (rxq->pkt_first_seg == NULL) {
> > + /* find the first split flag, and only reassemble then*/
> > + while (i < nb_bufs && !split_flags[i])
> > + i++;
> > + if (i == nb_bufs)
> > + return nb_bufs;
> > + }
> > + return i + fm10k_reassemble_packets(rxq, &rx_pkts[i], nb_bufs - i,
> > + &split_flags[i]);
> > +}
[dpdk-dev] [PATCH v2 08/16] fm10k: add Vector RX scatter function
Hi, Steve,
Best Regards,
Mark
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Chen, Jing D
> Sent: Tuesday, October 27, 2015 1:44 PM
> To: Liang, Cunming; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 08/16] fm10k: add Vector RX scatter
> function
>
> Hi, Steve,
>
> Best Regards,
> Mark
>
>
> > -Original Message-
> > From: Liang, Cunming
> > Sent: Tuesday, October 27, 2015 1:28 PM
> > To: Chen, Jing D; dev at dpdk.org
> > Cc: Tao, Zhe; He, Shaopeng; Ananyev, Konstantin; Richardson, Bruce
> > Subject: Re: [PATCH v2 08/16] fm10k: add Vector RX scatter function
> >
> > Hi,
> >
> > On 10/22/2015 5:44 PM, Chen Jing D(Mark) wrote:
> > > From: "Chen Jing D(Mark)"
> > >
> > > Add func fm10k_recv_scattered_pkts_vec to receive chained packets
> > > with SSE instructions.
> > >
> > > Signed-off-by: Chen Jing D(Mark)
> > > ---
> > > drivers/net/fm10k/fm10k.h |2 +
> > > drivers/net/fm10k/fm10k_rxtx_vec.c | 88
> >
> > > 2 files changed, 90 insertions(+), 0 deletions(-)
> > >
> > [...]
> > > +
> > > +/*
> > > + * vPMD receive routine that reassembles scattered packets
> > > + *
> > > + * Notice:
> > > + * - don't support ol_flags for rss and csum err
> > > + * - nb_pkts < RTE_IXGBE_DESCS_PER_LOOP, just return no packet
> > > + * - nb_pkts > RTE_IXGBE_MAX_RX_BURST, only scan
> > RTE_IXGBE_MAX_RX_BURST
> > > + * numbers of DD bit
> > In order to make sure nb_pkts > RTE_IXGBE_MAX_RX_BURST, it's
> necessary
> > to do RTE_MIN().
My bad. You indicates nb_pkts should be less or equal than
RTE_IXGBE_MAX_TX_BURST.
I'll change accordingly.
>
> I'll remove the improper comments. In func fm10k_recv_raw_pkts_vec, it
> will use
> nb_pkts as index to iterate properly.
> After then, below func will use actual received packet size nb_bufs as index
> to iterate.
> So, I think RTE_MIN() is not necessary?
>
> > > + * - floor align nb_pkts to a RTE_IXGBE_DESC_PER_LOOP power-of-two
> > > + */
> > > +uint16_t
> > > +fm10k_recv_scattered_pkts_vec(void *rx_queue,
> > > + struct rte_mbuf **rx_pkts,
> > > + uint16_t nb_pkts)
> > > +{
> > > + struct fm10k_rx_queue *rxq = rx_queue;
> > > + uint8_t split_flags[RTE_FM10K_MAX_RX_BURST] = {0};
> > > + unsigned i = 0;
> > > +
> > > + /* get some new buffers */
> > > + uint16_t nb_bufs = fm10k_recv_raw_pkts_vec(rxq, rx_pkts, nb_pkts,
> > > + split_flags);
> > > + if (nb_bufs == 0)
> > > + return 0;
> > > +
> > > + /* happy day case, full burst + no packets to be joined */
> > > + const uint64_t *split_fl64 = (uint64_t *)split_flags;
> > > + if (rxq->pkt_first_seg == NULL &&
> > > + split_fl64[0] == 0 && split_fl64[1] == 0 &&
> > > + split_fl64[2] == 0 && split_fl64[3] == 0)
> > > + return nb_bufs;
> > > +
> > > + /* reassemble any packets that need reassembly*/
> > > + if (rxq->pkt_first_seg == NULL) {
> > > + /* find the first split flag, and only reassemble then*/
> > > + while (i < nb_bufs && !split_flags[i])
> > > + i++;
> > > + if (i == nb_bufs)
> > > + return nb_bufs;
> > > + }
> > > + return i + fm10k_reassemble_packets(rxq, &rx_pkts[i], nb_bufs - i,
> > > + &split_flags[i]);
> > > +}
