[dpdk-dev] [PATCH 00/16] e1000 base code update
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Monday, November 28, 2016 7:34 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Yigit, Ferruh > Subject: Re: [dpdk-dev] [PATCH 00/16] e1000 base code update > > 2016-11-25 12:58, Ferruh Yigit: > > Can you also please send another patch to: > > 1- add I219 to supported nics list > > 2- announce new supported nic in release notes. > > Please update also the web site: > http://dpdk.org/doc/nics Didn't know that. How to update it? Thanks.
[dpdk-dev] [PATCH 00/16] e1000 base code update
Hi Ferruh, > -Original Message- > From: Yigit, Ferruh > Sent: Friday, November 25, 2016 8:58 PM > To: Lu, Wenzhuo; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 00/16] e1000 base code update > > Hi Wenzhuo, > > On 11/23/2016 5:22 PM, Wenzhuo Lu wrote: > > Updated e1000 base code to fix several bugs and support > > i219 NICs. > > > > Wenzhuo Lu (16): > > e1000/base: increased ULP timer > > e1000/base: increase PHY PLL clock gate timing > > e1000/base: try more times to get HW mailbox lock > > e1000/base: add getting HW version support for i354 > > e1000/base: expose e1000_write_vfta_i350 > > e1000/base: add max RX jumbo frame define > > e1000/base: restore link speed after ULP exit > > e1000/base: clear ULP configuration register on ULP exit > > e1000/base: increase LANPHYPC low duration > > e1000/base: workaround for ULP entry flow > > e1000/base: enable new i219 devices > > e1000/base: always request clock during K1 at 1G link speed > > e1000/base: ability to force K1-off disabled > > e1000/base: support more i219 devices > > e1000/base: update readme > > e1000: add new i219 devices > > > > drivers/net/e1000/base/README | 4 +- > > drivers/net/e1000/base/e1000_82575.c | 1 - > > drivers/net/e1000/base/e1000_82575.h | 1 + > > drivers/net/e1000/base/e1000_api.c | 19 + > > drivers/net/e1000/base/e1000_defines.h | 9 + > > drivers/net/e1000/base/e1000_hw.h | 21 +- > > drivers/net/e1000/base/e1000_ich8lan.c | 865 > > +++-- > drivers/net/e1000/base/e1000_ich8lan.h | 21 +- > > drivers/net/e1000/base/e1000_mbx.c | 36 +- > > drivers/net/e1000/base/e1000_nvm.c | 1 + > > drivers/net/e1000/base/e1000_regs.h| 7 + > > drivers/net/e1000/em_ethdev.c | 34 +- > > 12 files changed, 949 insertions(+), 70 deletions(-) > > > > Based on this pathset. > > Can you also please send another patch to: > 1- add I219 to supported nics list > 2- announce new supported nic in release notes. > > Also as far as I can see there is no igb/e1000 documentation under > doc/guides/nics/*, it can good to provide one, not with above requested > patches perhaps, but in some suitable time. I'll handle them. Thanks for the reminder :) > > Thanks, > Ferruh
[dpdk-dev] [PATCH 00/16] e1000 base code update
Hi Ferruh, > -Original Message- > From: Yigit, Ferruh > Sent: Friday, November 25, 2016 8:54 PM > To: Lu, Wenzhuo; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 00/16] e1000 base code update > > On 11/23/2016 5:22 PM, Wenzhuo Lu wrote: > > Updated e1000 base code to fix several bugs and support > > i219 NICs. > > > > Wenzhuo Lu (16): > > e1000/base: increased ULP timer > > e1000/base: increase PHY PLL clock gate timing > > e1000/base: try more times to get HW mailbox lock > > e1000/base: add getting HW version support for i354 > > e1000/base: expose e1000_write_vfta_i350 > > e1000/base: add max RX jumbo frame define > > e1000/base: restore link speed after ULP exit > > e1000/base: clear ULP configuration register on ULP exit > > e1000/base: increase LANPHYPC low duration > > e1000/base: workaround for ULP entry flow > > e1000/base: enable new i219 devices > > e1000/base: always request clock during K1 at 1G link speed > > e1000/base: ability to force K1-off disabled > > e1000/base: support more i219 devices > > e1000/base: update readme > > e1000: add new i219 devices > > > > drivers/net/e1000/base/README | 4 +- > > drivers/net/e1000/base/e1000_82575.c | 1 - > > drivers/net/e1000/base/e1000_82575.h | 1 + > > drivers/net/e1000/base/e1000_api.c | 19 + > > drivers/net/e1000/base/e1000_defines.h | 9 + > > drivers/net/e1000/base/e1000_hw.h | 21 +- > > drivers/net/e1000/base/e1000_ich8lan.c | 865 > > +++-- > drivers/net/e1000/base/e1000_ich8lan.h | 21 +- > > drivers/net/e1000/base/e1000_mbx.c | 36 +- > > drivers/net/e1000/base/e1000_nvm.c | 1 + > > drivers/net/e1000/base/e1000_regs.h| 7 + > > drivers/net/e1000/em_ethdev.c | 34 +- > > 12 files changed, 949 insertions(+), 70 deletions(-) > > > > Series applied to dpdk-next-net/master, thanks. > > Some modifications done in both commit subject and logs, can you please > double check the updates. Thanks for applying these patches! I've checked them. Everything is fine:)
[dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()
Hi Wei,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Dai
> Sent: Wednesday, November 16, 2016 10:41 AM
> To: dev at dpdk.org; Burakov, Anatoly; david.marchand at 6wind.com; Dai, Wei
> Subject: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()
>
> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c, The
> return
> value of mknod() is ret, not f got by fopen().
> So the value of ret should be checked for mknod().
>
> Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")
>
> Signed-off-by: Wei Dai
> ---
> fix my local git setting and send same patch again to make merging easier
>
> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index 1786b75..3e4ffb5 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> @@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path,
> unsigned uio_num)
> snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
> dev = makedev(major, minor);
> ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
> - if (f == NULL) {
> + if (ret != 0) {
I think checkpatch will suggest to just use if (ret)
[dpdk-dev] [PATCH] net/ixgbe: fix link never come up problem with x552
Hi,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Zhao
> Sent: Wednesday, November 9, 2016 3:00 PM
> To: dev at dpdk.org
> Cc: Zhao1, Wei
> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix link never come up problem with
> x552
>
> From: zhao wei
>
> The links never coming up with DPDK16.11 when bring up x552 NIC, device id is
> 15ac.This is caused by delete some code which casing removes X550em SFP iXFI
> setup for the drivers in function ixgbe_setup_mac_link_sfp_x550em().Fix
> methord is recover the deleted code.
>
> Fixes: 1726b9cd9c40 ("net/ixgbe/base: remove X550em SFP iXFI setup")
>
> Signed-off-by: zhao wei
Acked-by: Wenzhuo Lu
Thanks for the patch. It's a critical issue. We need to fix it. I'll report it
to our kernel driver developers.
[dpdk-dev] rte_ixgbevf_pmd not reporting dropped packets
Hi Francesco, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Montorsi, Francesco > Sent: Tuesday, November 8, 2016 1:17 AM > To: dev at dpdk.org > Subject: [dpdk-dev] rte_ixgbevf_pmd not reporting dropped packets > > Hi all, > > I'm using DPDK inside the OS of a VM that is SR-IOV-accelerated. > I noticed however that the "rte_ixgbevf_pmd" PMD does not report drops... I > am sending packets at TX side at 14Mpps; at the RX side I'm using "testpmd" > and > it's reporting 'just' 12Mpps, but zero drops (also through xstats). > > Is this a known bug? > I found a message at > http://dpdk.org/ml/archives/dev/2016-March/035374.html > reporting a similar issue back in March this year... It's a known limitation. The stats are got from HW. But there're very limited counters for VF. There's no similar counter as imiss for VF :( > > > Thanks, > > Francesco Montorsi
[dpdk-dev] [PATCH] lib/ip_frag: fix IP reassembly not working issue
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, November 8, 2016 4:28 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; adrien.mazarguil at 6wind.com; Ananyev, Konstantin > Subject: Re: [dpdk-dev] [PATCH] lib/ip_frag: fix IP reassembly not working > issue > > 2016-11-06 12:16, Wenzhuo Lu: > > After changing pkt[0] to pkt[], the example IP reassembly is not > > working. > > It's weird because this change is fine. There should be no difference > > between them. > > As a workaround, revert this change. > > > > Fixes: 347a1e037fd3 (lib: use C99 syntax for zero-size arrays) > > > > Reported-by: Huilong Xu > > Signed-off-by: Wenzhuo Lu > > Applied, thanks > > Please keep us informed if you understand the issue. Just FYI, It's so weird, I have to work around it first. 1, I don't think there's any wrong with the code. 2, I cannot reproduce this issue on my machine, but Huilong can reproduce it on different machines. As Adrien said, it may be a compile issue. I guess it's a compiler issue. Maybe gcc 4.8.5 has its own problem. (I'm using gcc 4.8.3 which is fine.)
[dpdk-dev] dpdk16.11 RC2 package ipv4 reassembly example can't work
Hi Adrien,
> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Wednesday, November 2, 2016 11:21 PM
> To: Lu, Wenzhuo
> Cc: Ananyev, Konstantin; Liu, Yu Y; Chen, WeichunX; Xu, HuilongX;
> dev at dpdk.org
> Subject: Re: dpdk16.11 RC2 package ipv4 reassembly example can't work
>
> Hi all,
>
> On Wed, Nov 02, 2016 at 08:39:31AM +, Lu, Wenzhuo wrote:
> > Correct the typo of receiver.
> >
> > Hi Adrien,
> > The change from struct ip_frag_pkt pkt[0] to struct ip_frag_pkt pkt[] will
> make IP reassembly not working. I think this is not the root cause. Maybe
> Konstantin can give us some idea.
> > But I notice one thing, you change some from [0] to [], but others just add
> '__extension__'. I believe if you add '__extension__' for struct ip_frag_pkt
> pkt[0],
> we'll not hit this issue. Just curious why you use 2 ways to resolve the same
> problem.
>
> I've used the __extension__ method whenever the C99 syntax could not work
> due to invalid usage in the code, e.g. a flexible array cannot be the only
> member
> of a struct, you cannot make arrays out of structures that contain such
> fields,
> while there is no such constraint with the GNU syntax.
>
> For example see __extension__ uint8_t action_data[0] in struct
> rte_pipeline_table_entry. The C99 could not be used because of
> test_table_acl.c:
>
> struct rte_pipeline_table_entry entries[5];
>
> If replacing ip_frag_pkt[] with __extension__ ip_frag_pkt pkt[0] in
> rte_ip_frag.h
> solves the issue, either some code is breaking some constraint somewhere or
> this change broke the ABI (unlikely considering a simple recompilation should
> have taken care of the issue). I did not notice any change in sizeof(struct
> rte_ip_frag_tbl) nor offsetof(struct rte_ip_frag_tbl, pkt) on my setup,
> perhaps
> the compilation flags used in your test affect them somehow.
Thanks for your explanation. I also checked sizeof(struct rte_ip_frag_tbl). I
don't see any change either.
>
> Can you confirm whether only reverting this particular field solves the issue?
Yes. ip_frag_pkt pkt[0] or even ip_frag_pkt pkt[1] can work but ip_frag_pkt
pkt[] cannot :(
Do you like the idea of changing the ip_frag_pkt[] to __extension__ ip_frag_pkt
pkt[0]?
>
> > From: Xu, HuilongX
> > Sent: Wednesday, November 2, 2016 4:29 PM
> > To: drien.mazarguil at 6wind.com
> > Cc: Ananyev, Konstantin; Liu, Yu Y; Chen, WeichunX; Lu, Wenzhuo; Xu,
> > HuilongX
> > Subject: dpdk16.11 RC2 package ipv4 reassembly example can't work
> >
> > Hi mazarguil,
> > I find ip reassembly example can't work with dpdk16.11 rc2 package.
> > But when I reset dpdk code before
> 347a1e037fd323e6c2af55d17f7f0dc4bfe1d479, it works ok.
> > Could you have time to check this issue, thanks a lot.
> > Unzip password: intel123
> >
> > Test detail info:
> >
> > os:4.2.3-300.fc23.x86_64
> > gcc version:5.3.1 20160406 (Red Hat 5.3.1-6) (GCC)
> > NIC:03:00.0 Ethernet controller [0200]: Intel Corporation Ethernet
> > Connection X552/X557-AT 10GBASE-T [8086:15ad] and
> > 84:00.0 Ethernet controller [0200]: Intel Corporation 82599ES
> > 10-Gigabit SFI/SFP+ Network Connection [8086:10fb] (rev 01)
> > package: dpdk16.11.rc2.tar.gz
> > test steps:
> > 1. build and install dpdk
> > 2. build ip_reassembly example
> > 3. run ip_reassembly
> > ./examples/ip_reassembly/build/ip_reassembly -c 0x2 -n 4 - -p 0x1
> > --maxflows=1024 --flowttl=10s 4. set tester port mtu ip link set mtu
> > 9000 dev ens160f1 5. setup scapy on tester and send packet scapy pcap
> > = rdpcap("file.pcap") sendp(pcap, iface="ens160f1") 6. sniff packet on
> > tester and check packet test result:
> > dpdk16.04 reassembly packet successful but dpdk16.11 reassembly pack failed.
> >
> > comments:
> > file.pcap: send packets pcap file
> > tcpdump_16.04_reassembly_successful.pcap: sniff packets by tcpdump on
> 16.04.
> > tcpdump_reset_code_reassembly_failed.pcap: sniff packets by tcpdump on
> > 16.11
> > reset_code_reassembly_successful_.jpg: reassembly a packets successful
> > detail info
> > dpdk16.11_reassembly_failed.jpg: reassembly a packets failed detail
> > info
> >
>
> --
> Adrien Mazarguil
> 6WIND
[dpdk-dev] HOW to forward vlan pkt using DPDK loadbalancer and L2FD, with intel 82599ES
Hi Anning, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Caianning > Sent: Friday, November 4, 2016 10:51 AM > To: users at dpdk.org; dev at dpdk.org > Subject: [dpdk-dev] HOW to forward vlan pkt using DPDK loadbalancer and L2FD, > with intel 82599ES > > HI, > > I am using DPDK 16.07 example loadbalancer to as LB to forward vlan > pkt > to applications. LB and applications are using VFs bound with DPDK igb_uio > driver. > But I found the packet can not be send from LB to application. Pkt input to LB > with vlan, after LB processing it is sent out to application using another VF. > > As 82599 need vlan filters if we use VF to process vlan pkts, in the LB we > called > rte api to add a vlan filter and enable vlan flilter. And set port_conf with > vlan > strip off. > And for the application(L2FWD) we use the cmd: ip link set PFxxx VF yy vlan > VLANID. > > In the LB ,we can receive pkts with vlan, after processing(cut the head and > add > another L2 head with same vlan tag, set destination MAC to application VF), > it is > send out to the application VF. > the result is: LB can still processing ,but all the pkt post. > > We also did an experiment using testPMD to forward vlan pkt to application on > the same VF used by LB, and it is OK. > We tried kinds of different set of vlan filter, vlan strip, ip link set, but > found no > solution. > > Does anyone meet the similar problem? And what factor casued the difference? > > Thx. I saw you mentioned testpmd is fine. Testpmd is a very simple example, normally it forwards packets blindly. It means there's no route rule. It just forwards packets from one port to another. I guess the difference is the APP you use has some rules for routing/swithing.
[dpdk-dev] [PATCH v2] E1000: fix for forced speed/duplex config
Hi, > -Original Message- > From: Ananda Sathyanarayana [mailto:ananda at versa-networks.com] > Sent: Thursday, November 3, 2016 7:37 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Ananda Sathyanarayana > Subject: [PATCH v2] E1000: fix for forced speed/duplex config > > Fixed the formating/syntax issues reported > > From the code, it looks like, hw->mac.autoneg, variable is used to switch > between calling either autoneg function or forcing speed/duplex function. But > this variable is not modified in eth_em_start/eth_igb_start routines (it is > always > set to 1) even while forcing the link speed. > > Following discussion thread has some more information on this > > http://dpdk.org/ml/archives/dev/2016-October/049272.html > > Signed-off-by: Ananda Sathyanarayana Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH] E1000: fix for forced speed/duplex config
Hi Ananda, > -Original Message- > From: Ananda Sathyanarayana [mailto:ananda at versa-networks.com] > Sent: Wednesday, November 2, 2016 6:47 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Ananda Sathyanarayana > Subject: [PATCH] E1000: fix for forced speed/duplex config > > From the code, it looks like, hw->mac.autoneg, variable is used to switch > between calling either autoneg function or forcing speed/duplex function. But > this variable is not modified in eth_em_start/eth_igb_start routines (it is > always > set to 1) even while forcing the link speed. > > Following discussion thread has some more information on this > > http://dpdk.org/ml/archives/dev/2016-October/049272.html > > Signed-off-by: Ananda Sathyanarayana Thanks for the patch. It looks fine to me. But as it's a fix, would you like to add a Fixes tag for it?
[dpdk-dev] Unable to change source MAC address of packet
Hi Padam, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Padam Jeet Singh > Sent: Thursday, October 27, 2016 10:45 PM > To: Wiles, Keith > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] Unable to change source MAC address of packet > > > > On 27-Oct-2016, at 7:37 pm, Wiles, Keith wrote: > > > > > >> On Oct 27, 2016, at 6:33 AM, Padam Jeet Singh > wrote: > >> > >> Hi, > >> > >> I am crafting a packet in which the source MAC address as set in the > >> Ethernet > header is different than the transmit port?s default MAC address. A packet > capture of the packets coming out of this port however comes with source MAC > address of the port?s default MAC address. > >> > >> Altering the destination MAC address works fine and shows up correctly in > packet capture. > >> > >> The underlying network interface is an i210 and some logs added to the > eth_igb_xmit_pkts function show that the packets I have crafted indeed are > reaching the driver with the source MAC address set in the packet code of the > application. > >> > >> How can I disable this automatic source MAC address setting? > > > > The packets sent with rte_eth_tx_burst() are not forced to a give MAC > > address. > If you are using something on top of DPDK like Pktgen or OVS or something, > then it may try to force a source MAC address. > > No? not using pktgen or OVS. Plain simple code to take a packets from a KNI, > change source mac address on all received packets, and then tx_burst them to > a > port. > > > Maybe the hardware does it, but we need to know the NIC being used and > then someone maybe able to answer. I do not know of any Intel NICs do that. > > Intel i210 NIC (gigabit Ethernet) is being used. I have gone through the i210 > documentation and can?t see anything specific to setting of MAC address in > hardware for TX side. For RX side there are validations like MAC filtering, > but > nothing over TX. > > > > > Is this what you are doing. > > I agree that rte_eth_tx_burst does not overwrite the source MAC as I was able > to trace all the way to the IGB driver that source mac makes it intact. There > is no > offload flags enabled in the mbuf. Yet the packets to the other side comes out > as with source mac address of the port. > > Is there any standard DPDK app which crafts packets with different source MAC > than the port?s physical mac? (I checked the l2fwd example loads the port mac > before transmitting and then uses the same in TX function). I don?t have a i210 on hand, so I checked the datasheet of i210. I don't find any description about HW will change the MAC address in the frame. I believe HW should send the frame provided by SW except doing some offload like checksum... May I suggest to use testpmd forwarding a packet which has a different src MAC than the port's? > > > > >> > >> Thanks, > >> Padam > > > > Regards, > > Keith > >
[dpdk-dev] [PATCH v5 0/2] net/ixgbe: VMDq DCB with SRIOV
Hi, > -Original Message- > From: Iremonger, Bernard > Sent: Wednesday, October 19, 2016 6:21 PM > To: dev at dpdk.org; Shah, Rahul R; Lu, Wenzhuo > Cc: Iremonger, Bernard > Subject: [PATCH v5 0/2] net/ixgbe: VMDq DCB with SRIOV > > Changes in v5: > fix enable/disable of the QDE bit in the PFQDE register. > > Changes in v4: > changes to ixgbe patch following comments. > > Changes in v3: > rebase to latest master. > update commit message for ixgbe patch > add testpmd patch. > > Changes in v2: > rebase to latest master. > > Bernard Iremonger (2): > net/ixgbe: support multiqueue mode VMDq DCB with SRIOV > app/test_pmd: fix DCB configuration > > app/test-pmd/testpmd.c | 4 ++-- > drivers/net/ixgbe/ixgbe_ethdev.c | 11 ++- > drivers/net/ixgbe/ixgbe_rxtx.c | 35 ++- > 3 files changed, 30 insertions(+), 20 deletions(-) > > -- > 2.10.1 Series-Acked-by: Wenzhuo Lu
[dpdk-dev] Manual link speed/duplex configuration not working with DPDK
Hi Ananda,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ananda
> Sathyanarayana
> Sent: Saturday, October 22, 2016 3:27 AM
> To: dev at dpdk.org
> Cc: ananda at versa-networks.com; Vignesh Chinnakkannu
> Subject: [dpdk-dev] Manual link speed/duplex configuration not working with
> DPDK
>
> Hi All,
>
>
>
> While testing manual link speed/duplex configuration with DPDK 1.7.1, I
> observed the same issues mentioned by the below post
>
> http://dpdk.org/ml/archives/dev/2015-January/010834.html. I see the same
> issue with 16.04 as well.
Don?t know the history. Seems it?s discussion but not a patch. Guess that's why
it?s not accepted.
It looks OK to me. Maybe we can create a patch for it?
>
>
>
> Looks like the above patch is not accepted by the DPDK community yet. Any
> specific reason ?
>
>
>
> From the code, it looks like, hw->mac.autoneg, variable is used to switch
> between calling either autoneg function or forcing speed/duplex function.
> But this variable is not modified in eth_em_start/eth_igb_start routines (it
> is
> always set to 1) while forcing the link.
>
>
>
> s32 e1000_setup_copper_link_generic(struct e1000_hw *hw)
>
> {
>
> s32 ret_val;
>
> bool link;
>
>
>
> DEBUGFUNC("e1000_setup_copper_link_generic");
>
>
>
> if (hw->mac.autoneg) { always set, is not modified
> in eth_em_start/eth_igb_start
>
>
>
> /* Setup autoneg and flow control advertisement and perform
>
> * autonegotiation.
>
> */
>
> ret_val = e1000_copper_link_autoneg(hw);
>
> if (ret_val)
>
> return ret_val;
>
> } else {
>
> /* PHY will be set to 10H, 10F, 100H or 100F
>
> * depending on user settings.
>
> */
>
> DEBUGOUT("Forcing Speed and Duplex\n");
>
> ret_val = hw->phy.ops.force_speed_duplex(hw);
> Not called at all
>
> if (ret_val) {
>
> DEBUGOUT("Error Forcing Speed and Duplex\n");
>
> return ret_val;
>
> }
>
> }
>
>
>
> }
>
>
>
>
>
> Thanks,
>
> Ananda
[dpdk-dev] [PATCH] net/ixgbe: prevent duplicate callback on list
Hi Scott, > -Original Message- > From: Scott Daniels [mailto:daniels at research.att.com] > Sent: Thursday, October 20, 2016 10:11 AM > To: Lu, Wenzhuo > Cc: Zhang, Helin; Iremonger, Bernard; dev at dpdk.org; ZELEZNIAK, ALEX > Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: prevent duplicate callback on list > > > > On Wed, 19 Oct 2016, Lu, Wenzhuo wrote: > > > Hi Scott, > > > >> -Original Message- > >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of E. Scott Daniels > >> Sent: Thursday, October 20, 2016 6:23 AM > >> To: Zhang, Helin; Iremonger, Bernard > >> Cc: dev at dpdk.org; az5157 at att.com; E. Scott Daniels > >> Subject: [dpdk-dev] [PATCH] net/ixgbe: prevent duplicate callback on > >> list > >> > >> This change prevents the attempt to add a structure which is already > >> on the callback list. If a struct with matching parameters is found > >> on the list, then no action is taken. If a struct with matching > >> parameters is found on the list, then no action is taken. > >> > >> Signed-off-by: E. Scott Daniels > > I think the fix itself is good. But 2 things, 1, normally we don't > > create a cover letter for a patch set which only has one patch. Just > > sending the > patch itself is enough. > > 2, ' net/ixgbe: ' in the title is used to describe the component. So the > > title > should be ' lib/ether: prevent duplicate callback on list'. > > Thanks. > > Thanks for the advice. My mistake on the component. Is there an easy way to > fix, or does it make sense just to nack this and I'll submit one with the > correct > component. No need to NACK it. You can send a V2 with the correct component. And I think you can add my ack in the V2. Acked-by: Wenzhuo Lu BTW, I forgot to mention that as it's a fix. We always add a Fixes tag in the commit log. You can find the example from other one's mails :) > > Scott > > > > >
[dpdk-dev] [PATCH] net/ixgbe: prevent duplicate callback on list
Hi Scott, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of E. Scott Daniels > Sent: Thursday, October 20, 2016 6:23 AM > To: Zhang, Helin; Iremonger, Bernard > Cc: dev at dpdk.org; az5157 at att.com; E. Scott Daniels > Subject: [dpdk-dev] [PATCH] net/ixgbe: prevent duplicate callback on list > > This change prevents the attempt to add a structure which is already on the > callback list. If a struct with matching parameters is found on the list, > then no > action is taken. If a struct with matching parameters is found on the list, > then no > action is taken. > > Signed-off-by: E. Scott Daniels I think the fix itself is good. But 2 things, 1, normally we don't create a cover letter for a patch set which only has one patch. Just sending the patch itself is enough. 2, ' net/ixgbe: ' in the title is used to describe the component. So the title should be ' lib/ether: prevent duplicate callback on list'. Thanks.
[dpdk-dev] [PATCH v5 1/2] net/ixgbe: support multiqueue mode VMDq DCB with SRIOV
Hi, > -Original Message- > From: Iremonger, Bernard > Sent: Wednesday, October 19, 2016 6:21 PM > To: dev at dpdk.org; Shah, Rahul R; Lu, Wenzhuo > Cc: Iremonger, Bernard > Subject: [PATCH v5 1/2] net/ixgbe: support multiqueue mode VMDq DCB with > SRIOV > > Modify ixgbe_check_mq_mode function, > when SRIOV is enabled, enable mq_mode > ETH_MQ_RX_VMDQ_DCB and ETH_MQ_TX_VMDQ_DCB. > > Modify ixgbe_dcb_tx_hw_config function, > replace the struct ixgbe_hw parameter with a struct rte_eth_dev parameter and > handle SRIOV enabled. > > Modify ixgbe_dev_mq_rx_configure function, when SRIOV is enabled, enable > mq_mode ETH_MQ_RX_VMDQ_DCB. > > Modify ixgbe_configure_dcb function, > drop check on dev->data->nb_rx_queues. > > Signed-off-by: Rahul R Shah > Signed-off-by: Bernard Iremonger Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH v2 0/2] net/ixgbe: fix VF VLAN insert
Hi, > -Original Message- > From: Iremonger, Bernard > Sent: Wednesday, October 19, 2016 10:48 PM > To: dev at dpdk.org; daniels at research.att.com; Lu, Wenzhuo; az5157 at > att.com > Cc: Iremonger, Bernard > Subject: [PATCH v2 0/2] net/ixgbe: fix VF VLAN insert > > Changes in v2: > Add testpmd patch. > Update testpmd for change to rte_pmd_ixgbe_set_vf_vlan_insert function. > > Bernard Iremonger (1): > app/test_pmd: change to the VF VLAN insert command > > E. Scott Daniels (1): > net/ixgbe: fix VLAN insert parameter type and its use > > app/test-pmd/cmdline.c | 19 +-- > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +- > drivers/net/ixgbe/ixgbe_ethdev.c| 8 > drivers/net/ixgbe/rte_pmd_ixgbe.h | 9 + > 4 files changed, 19 insertions(+), 19 deletions(-) > > -- > 2.10.1 Series-Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH] net/ixgbe: fix vlan insert parameter type and its use
Hi Bernard,
> -Original Message-
> From: Iremonger, Bernard
> Sent: Wednesday, October 19, 2016 6:56 PM
> To: Lu, Wenzhuo; E. Scott Daniels; Zhang, Helin
> Cc: dev at dpdk.org; az5157 at att.com
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: fix vlan insert parameter type and
> its
> use
>
> Hi Wenzhuo, Scott,
>
>
>
> > > Subject: [dpdk-dev] [PATCH] net/ixgbe: fix vlan insert parameter
> > > type and its use
> > >
> > > The final parameter to rte_pmd_ixgbe_set_vf_vlan_insert is uint8_t
> > > and treated as a binary flag when it needs to be a uint16_t and
> > > treated as a VLAN id. The data sheet (sect 8.2.3.27.13) describes
> > > the right most
> > > 16 bits as the VLAN id that is to be inserted; the
> > > 16.11 code is accepting only a 1 or 0 thus effectively only
> > > allowing the VLAN id 1 to be inserted (0 disables the insertion setting).
> > >
> > > This patch changes the final parm name to represent the data that is
> > > being accepted (vlan_id), changes the type to permit all valid VLAN
> > > ids, and validates the parameter based on the range of 0 to 4095.
> > > Corresponding changes to prototype and documentation in the .h file.
> > >
> > > Fixes: 49e248223e9f71 ("net/ixgbe: add API for VF management")
> > >
> > > Signed-off-by: E. Scott Daniels
> > > ---
> > > drivers/net/ixgbe/ixgbe_ethdev.c | 8
> > > drivers/net/ixgbe/rte_pmd_ixgbe.h | 9 +
> > > 2 files changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > index 4ca5747..316af73 100644
> > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > @@ -4727,7 +4727,7 @@ rte_pmd_ixgbe_set_vf_mac_anti_spoof(uint8_t
> > > port, uint16_t vf, uint8_t on) }
> > >
> > > int
> > > -rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint8_t
> > > on)
> > > +rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf,
> > > +uint16_t
> > > +vlan_id)
> > > {
> > > struct ixgbe_hw *hw;
> > > uint32_t ctrl;
> > > @@ -4742,13 +4742,13 @@ rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t
> > port,
> > > uint16_t vf, uint8_t on)
> > > if (vf >= dev_info.max_vfs)
> > > return -EINVAL;
> > >
> > > - if (on > 1)
> > > + if (vlan_id > 4095)
> > > return -EINVAL;
> > >
> > > hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > > ctrl = IXGBE_READ_REG(hw, IXGBE_VMVIR(vf));
> > > - if (on) {
> > > - ctrl = on;
> > > + if (vlan_id) {
> > > + ctrl = vlan_id;
> > I believe this patch is a good idea of an enhancement. But you cannot
> > leverage the existing code like this.
> > The register IXGBE_VMVIR is only for enable/disable. We cannot write
> > the VLAN ID into it.
> > If you want to merge the 2 things, enabling/disabling and setting VLAN
> > ID together. I think we need a totally new implementation. So NACK.
>
> Reading the 82599 data sheet (sect 8.2.3.27.13), the change from Scott is
> correct.
> The NACK is not correct.
You're right. I made a mistake about the words 'default vlan'. It's not fixed
but the value of 'Port VLAN ID'.
So the previous patch is not good, we need to fix it. Thanks for correcting me.
>
> However changing the API means that where it is called needs to be changed
> too.
> It is called at present from app/testpmd/cmdline.c line 4731.
>
>
>
> > > ctrl |= IXGBE_VMVIR_VLANA_DEFAULT;
> > > } else {
> > > ctrl = 0;
> > > diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h
> > > b/drivers/net/ixgbe/rte_pmd_ixgbe.h
> > > index 2fdf530..c2fb826 100644
> > > --- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
> > > +++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
> > > @@ -99,16 +99,17 @@ int rte_pmd_ixgbe_set_vf_mac_anti_spoof(uint8_t
> > > port, uint16_t vf, uint8_t on);
> > > *The port identifier of the Ethernet device.
> > > * @param vf
> > > *ID specifying VF.
> > > - * @param on
> > > - *1 - Enable VF's vlan insert.
> > > - *0 - Disable VF's vlan insert
> > > + * @param vlan_id
> > > + *0 - Disable VF's vlan insert.
> > > + *n - Enable; n is inserted as the vlan id.
> > > *
> > > * @return
> > > * - (0) if successful.
> > > * - (-ENODEV) if *port* invalid.
> > > * - (-EINVAL) if bad parameter.
> > > */
> > > -int rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf,
> > > uint8_t on);
> > > +int rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf,
> > > + uint16_t vlan_id);
> > >
> > > /**
> > > * Enable/Disable tx loopback
> > > --
> > > 1.9.1
>
> Regards,
>
> Bernard.
[dpdk-dev] [PATCH] net/ixgbe: fix vlan insert parameter type and its use
Hi Daniels,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of E. Scott Daniels
> Sent: Wednesday, October 19, 2016 3:13 AM
> To: Zhang, Helin; Iremonger, Bernard
> Cc: dev at dpdk.org; az5157 at att.com; E. Scott Daniels
> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix vlan insert parameter type and its
> use
>
> The final parameter to rte_pmd_ixgbe_set_vf_vlan_insert is uint8_t and treated
> as a binary flag when it needs to be a uint16_t and treated as a VLAN id. The
> data sheet (sect 8.2.3.27.13) describes the right most 16 bits as the VLAN id
> that
> is to be inserted; the
> 16.11 code is accepting only a 1 or 0 thus effectively only allowing the
> VLAN id 1
> to be inserted (0 disables the insertion setting).
>
> This patch changes the final parm name to represent the data that is being
> accepted (vlan_id), changes the type to permit all valid VLAN ids, and
> validates
> the parameter based on the range of 0 to 4095. Corresponding changes to
> prototype and documentation in the .h file.
>
> Fixes: 49e248223e9f71 ("net/ixgbe: add API for VF management")
>
> Signed-off-by: E. Scott Daniels
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 8
> drivers/net/ixgbe/rte_pmd_ixgbe.h | 9 +
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 4ca5747..316af73 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -4727,7 +4727,7 @@ rte_pmd_ixgbe_set_vf_mac_anti_spoof(uint8_t port,
> uint16_t vf, uint8_t on) }
>
> int
> -rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint8_t on)
> +rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint16_t
> +vlan_id)
> {
> struct ixgbe_hw *hw;
> uint32_t ctrl;
> @@ -4742,13 +4742,13 @@ rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port,
> uint16_t vf, uint8_t on)
> if (vf >= dev_info.max_vfs)
> return -EINVAL;
>
> - if (on > 1)
> + if (vlan_id > 4095)
> return -EINVAL;
>
> hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> ctrl = IXGBE_READ_REG(hw, IXGBE_VMVIR(vf));
> - if (on) {
> - ctrl = on;
> + if (vlan_id) {
> + ctrl = vlan_id;
I believe this patch is a good idea of an enhancement. But you cannot leverage
the existing code like this.
The register IXGBE_VMVIR is only for enable/disable. We cannot write the VLAN
ID into it.
If you want to merge the 2 things, enabling/disabling and setting VLAN ID
together. I think we need a totally new implementation. So NACK.
> ctrl |= IXGBE_VMVIR_VLANA_DEFAULT;
> } else {
> ctrl = 0;
> diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h
> b/drivers/net/ixgbe/rte_pmd_ixgbe.h
> index 2fdf530..c2fb826 100644
> --- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
> +++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
> @@ -99,16 +99,17 @@ int rte_pmd_ixgbe_set_vf_mac_anti_spoof(uint8_t port,
> uint16_t vf, uint8_t on);
> *The port identifier of the Ethernet device.
> * @param vf
> *ID specifying VF.
> - * @param on
> - *1 - Enable VF's vlan insert.
> - *0 - Disable VF's vlan insert
> + * @param vlan_id
> + *0 - Disable VF's vlan insert.
> + *n - Enable; n is inserted as the vlan id.
> *
> * @return
> * - (0) if successful.
> * - (-ENODEV) if *port* invalid.
> * - (-EINVAL) if bad parameter.
> */
> -int rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint8_t on);
> +int rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf,
> + uint16_t vlan_id);
>
> /**
> * Enable/Disable tx loopback
> --
> 1.9.1
[dpdk-dev] [PATCH v2 3/3] app/testpmd: fix flow director endian issue
Hi Pablo,
> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Tuesday, October 18, 2016 4:19 AM
> To: Lu, Wenzhuo; dev at dpdk.org
> Cc: Lu, Wenzhuo
> Subject: RE: [dpdk-dev] [PATCH v2 3/3] app/testpmd: fix flow director endian
> issue
>
>
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wenzhuo Lu
> > Sent: Tuesday, October 11, 2016 11:24 PM
> > To: dev at dpdk.org
> > Cc: Lu, Wenzhuo
> > Subject: [dpdk-dev] [PATCH v2 3/3] app/testpmd: fix flow director
> > endian issue
> >
> > The vlan mask and tunnel id mask of flow director are defined as big
> > endian. So they should be converted.
> > When the mask is printed, the parameters are not converted either.
> > This patch converts the mask parameters.
> > Some lines of the mask print are too long, split them to more lines.
> >
> > Fixes: 7c554b4f0484 ("app/testpmd: update display of flow director
> > information")
> > Fixes: 53b2bb9b7ea7 ("app/testpmd: new flow director commands")
> > Signed-off-by: Wenzhuo Lu
>
> Commit message should have a space between the Fixes and the Signed-off line.
> Apart from that:
>
> Acked-by: Pablo de Lara
Thanks for reviewing. I'll correct the commit log for all these patches.
[dpdk-dev] [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with SRIOV
Hi Bernard,
> -Original Message-
> From: Iremonger, Bernard
> Sent: Tuesday, October 18, 2016 4:58 PM
> To: Lu, Wenzhuo; dev at dpdk.org; Shah, Rahul R
> Subject: RE: [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with
> SRIOV
>
> Hi Wenzhuo,
>
>
>
> > > Subject: RE: [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB
> > > with SRIOV
> > >
> > > Hi Wenzhuo,
> > >
> > > > > if (hw->mac.type != ixgbe_mac_82598EB) { @@ -3339,11 +3340,17
> > > > @@
> > > > > ixgbe_dcb_tx_hw_config(struct ixgbe_hw *hw,
> > > > > if (dcb_config->vt_mode)
> > > > > reg |= IXGBE_MTQC_VT_ENA;
> > > > > IXGBE_WRITE_REG(hw, IXGBE_MTQC, reg);
> > > > > -
> > > > > - /* Disable drop for all queues */
> > > > > - for (q = 0; q < 128; q++)
> > > > > - IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > > > - (IXGBE_QDE_WRITE | (q <<
> > > > > IXGBE_QDE_IDX_SHIFT)));
> > > > > + if (RTE_ETH_DEV_SRIOV(dev).active == 0) {
> > > > > + /* Disable drop for all queues in VMDQ mode*/
> > > > > + for (q = 0; q < 128; q++)
> > > > > + IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > > > + (IXGBE_QDE_WRITE |
> (q <<
> > > > > IXGBE_QDE_IDX_SHIFT) | IXGBE_QDE_ENABLE));
This code is used to enable drop, but the comments say 'disable drop'.
> > > > > + } else {
> > > > > + /* Enable drop for all queues in SRIOV mode */
> > > > > + for (q = 0; q < 128; q++)
> > > > > + IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > > > + (IXGBE_QDE_WRITE |
> (q <<
> > > > > IXGBE_QDE_IDX_SHIFT)));
This code is used to disable drop, but the comments say 'enable drop'.
I've checked the V4, the same problem too. I think the problem is the comments,
right?
> > > > > + }
> > > > I think it has nothing to do with mq mode. Do I miss something?
> > >
> > > Behavior is different when SRIOV is enabled.
> > I don't understand why the behavior is different. To my opinion, the
> > drop has nothing to do with the mode. We can enable or disable it.
> > The old behavior is disabling it by default. Now you change it to
> > disabling it by default in NO-SRIOV mode, but enabling it in SRIOV mode.
> > What I don't get is the reason.
>
> Please refer to section 4.6.11.3.1 page 180 of the 82599-10-gbe-controller-
> datasheet.pdf
> The last paragraph on page 180 states:
>
> "Queue Drop Enable (PFQDE) - In SR-IO the QDE bit should be set to 1b in the
> PFQDE register for all queues.
> In VMDq mode, the QDE bit should be set to 0b for all queues."
Got it, thanks a lot :)
>
>
> > > > > /* Enable the Tx desc arbiter */
> > > > > reg = IXGBE_READ_REG(hw, IXGBE_RTTDCS); @@ -3378,7
> > > > > +3385,7 @@ ixgbe_vmdq_dcb_hw_tx_config(struct rte_eth_dev *dev,
> > > > > vmdq_tx_conf->nb_queue_pools == ETH_16_POOLS
> > > > ?
> > > > > 0x : 0x);
> > > > >
> > > > > /*Configure general DCB TX parameters*/
> > > > > - ixgbe_dcb_tx_hw_config(hw, dcb_config);
> > > > > + ixgbe_dcb_tx_hw_config(dev, dcb_config);
> > > > > }
> > > > >
> > > > > static void
> > > > > @@ -3661,7 +3668,7 @@ ixgbe_dcb_hw_configure(struct rte_eth_dev
> > > > *dev,
> > > > > /*get DCB TX configuration parameters from
> > > > > rte_eth_conf*/
> > > > > ixgbe_dcb_tx_config(dev, dcb_config);
> > > > > /*Configure general DCB TX parameters*/
> > > > > - ixgbe_dcb_tx_hw_config(hw, dcb_config);
> > > > > + ixgbe_dcb_tx_hw_config(dev, dcb_config);
> > > > > break;
> > > > > default:
> > > > > PMD_INIT_LOG(ERR, "Incorrect DCB TX mode
> > > > configuration"); @@
> > > > > -3810,9 +3817,6 @@ void ixgbe_configure_dcb(struct rte_eth_dev
> > *dev)
> > > > > (dev_conf->rxmode.mq_mode != ETH_MQ_RX_DCB_RSS))
> > > > > return;
> > > > >
> > > > > - if (dev->data->nb_rx_queues != ETH_DCB_NUM_QUEUES)
> > > > > - return;
> > > > I remember it's a limitation of implementation. The reason is the
> > > > resource allocation. Why could we remove it now?
> > >
> > > ETH_DCB_NUM_QUEUES is 128, nb_rx_queues may not be 128.
> > I think it's a limitation to force the queue number to be
> > ETH_DCB_NUM_QUEUES.
> > Just to confirm it, have you try to set rx queue number to something
> > different from 128, like 64, 32...
>
> In my test scenario the nb_rx_queues is 1.
Glad to know that, thanks :)
>
> Regards,
>
> Bernard
[dpdk-dev] [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with SRIOV
Hi Bernard,
> -Original Message-
> From: Iremonger, Bernard
> Sent: Tuesday, October 18, 2016 12:27 AM
> To: Lu, Wenzhuo; dev at dpdk.org; Shah, Rahul R
> Subject: RE: [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with
> SRIOV
>
> Hi Wenzhuo,
>
> > > if (hw->mac.type != ixgbe_mac_82598EB) { @@ -3339,11 +3340,17
> > @@
> > > ixgbe_dcb_tx_hw_config(struct ixgbe_hw *hw,
> > > if (dcb_config->vt_mode)
> > > reg |= IXGBE_MTQC_VT_ENA;
> > > IXGBE_WRITE_REG(hw, IXGBE_MTQC, reg);
> > > -
> > > - /* Disable drop for all queues */
> > > - for (q = 0; q < 128; q++)
> > > - IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > - (IXGBE_QDE_WRITE | (q <<
> > > IXGBE_QDE_IDX_SHIFT)));
> > > + if (RTE_ETH_DEV_SRIOV(dev).active == 0) {
> > > + /* Disable drop for all queues in VMDQ mode*/
> > > + for (q = 0; q < 128; q++)
> > > + IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > + (IXGBE_QDE_WRITE | (q <<
> > > IXGBE_QDE_IDX_SHIFT) | IXGBE_QDE_ENABLE));
> > > + } else {
> > > + /* Enable drop for all queues in SRIOV mode */
> > > + for (q = 0; q < 128; q++)
> > > + IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > + (IXGBE_QDE_WRITE | (q <<
> > > IXGBE_QDE_IDX_SHIFT)));
> > > + }
> > I think it has nothing to do with mq mode. Do I miss something?
>
> Behavior is different when SRIOV is enabled.
I don't understand why the behavior is different. To my opinion, the drop has
nothing to do with the mode. We can enable or disable it.
The old behavior is disabling it by default. Now you change it to disabling it
by default in NO-SRIOV mode, but enabling it in SRIOV mode.
What I don't get is the reason.
>
> >
> > >
> > > /* Enable the Tx desc arbiter */
> > > reg = IXGBE_READ_REG(hw, IXGBE_RTTDCS); @@ -3378,7
> > > +3385,7 @@ ixgbe_vmdq_dcb_hw_tx_config(struct rte_eth_dev *dev,
> > > vmdq_tx_conf->nb_queue_pools == ETH_16_POOLS
> > ?
> > > 0x : 0x);
> > >
> > > /*Configure general DCB TX parameters*/
> > > - ixgbe_dcb_tx_hw_config(hw, dcb_config);
> > > + ixgbe_dcb_tx_hw_config(dev, dcb_config);
> > > }
> > >
> > > static void
> > > @@ -3661,7 +3668,7 @@ ixgbe_dcb_hw_configure(struct rte_eth_dev
> > *dev,
> > > /*get DCB TX configuration parameters from rte_eth_conf*/
> > > ixgbe_dcb_tx_config(dev, dcb_config);
> > > /*Configure general DCB TX parameters*/
> > > - ixgbe_dcb_tx_hw_config(hw, dcb_config);
> > > + ixgbe_dcb_tx_hw_config(dev, dcb_config);
> > > break;
> > > default:
> > > PMD_INIT_LOG(ERR, "Incorrect DCB TX mode
> > configuration"); @@
> > > -3810,9 +3817,6 @@ void ixgbe_configure_dcb(struct rte_eth_dev *dev)
> > > (dev_conf->rxmode.mq_mode != ETH_MQ_RX_DCB_RSS))
> > > return;
> > >
> > > - if (dev->data->nb_rx_queues != ETH_DCB_NUM_QUEUES)
> > > - return;
> > I remember it's a limitation of implementation. The reason is the
> > resource allocation. Why could we remove it now?
>
> ETH_DCB_NUM_QUEUES is 128, nb_rx_queues may not be 128.
I think it's a limitation to force the queue number to be ETH_DCB_NUM_QUEUES.
Just to confirm it, have you try to set rx queue number to something different
from 128, like 64, 32...
[dpdk-dev] [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with SRIOV
Hi Bernard, Rahul,
> -Original Message-
> From: Iremonger, Bernard
> Sent: Friday, October 14, 2016 9:29 PM
> To: dev at dpdk.org; Shah, Rahul R; Lu, Wenzhuo
> Cc: Iremonger, Bernard
> Subject: [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with SRIOV
>
> modify ixgbe_dcb_tx_hw_config function.
> modify ixgbe_dev_mq_rx_configure function.
> modify ixgbe_configure_dcb function.
Would you like to add more details about why we need to modify these functions
and what has been done? Thanks.
>
> Changes in v2:
> Rebased to DPDK v16.11-rc1
>
> Signed-off-by: Rahul R Shah
> Signed-off-by: Bernard Iremonger
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 9 -
> drivers/net/ixgbe/ixgbe_rxtx.c | 37 +
> 2 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 4ca5747..114698d 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1977,6 +1977,8 @@ ixgbe_check_mq_mode(struct rte_eth_dev *dev)
> /* check multi-queue mode */
> switch (dev_conf->rxmode.mq_mode) {
> case ETH_MQ_RX_VMDQ_DCB:
Should we store the mq mode here?
> + PMD_INIT_LOG(INFO, "ETH_MQ_RX_VMDQ_DCB
> mode supported in SRIOV");
> + break;
> case ETH_MQ_RX_VMDQ_DCB_RSS:
> /* DCB/RSS VMDQ in SRIOV mode, not implement yet
> */
> PMD_INIT_LOG(ERR, "SRIOV active,"
> @@ -2012,11 +2014,8 @@ ixgbe_check_mq_mode(struct rte_eth_dev *dev)
>
> switch (dev_conf->txmode.mq_mode) {
> case ETH_MQ_TX_VMDQ_DCB:
> - /* DCB VMDQ in SRIOV mode, not implement yet */
> - PMD_INIT_LOG(ERR, "SRIOV is active,"
> - " unsupported VMDQ mq_mode
> tx %d.",
> - dev_conf->txmode.mq_mode);
> - return -EINVAL;
> + PMD_INIT_LOG(INFO, "ETH_MQ_TX_VMDQ_DCB mode
> supported in SRIOV");
> + break;
> default: /* ETH_MQ_TX_VMDQ_ONLY or ETH_MQ_TX_NONE
> */
> dev->data->dev_conf.txmode.mq_mode =
> ETH_MQ_TX_VMDQ_ONLY;
> break;
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index
> 2ce8234..bb13889 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1,7 +1,7 @@
> /*-
> * BSD LICENSE
> *
> - * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> + * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
> * Copyright 2014 6WIND S.A.
> * All rights reserved.
> *
> @@ -3313,15 +3313,16 @@ ixgbe_vmdq_dcb_configure(struct rte_eth_dev
> *dev)
>
> /**
> * ixgbe_dcb_config_tx_hw_config - Configure general DCB TX parameters
> - * @hw: pointer to hardware structure
> + * @dev: pointer to eth_dev structure
> * @dcb_config: pointer to ixgbe_dcb_config structure
> */
> static void
> -ixgbe_dcb_tx_hw_config(struct ixgbe_hw *hw,
> +ixgbe_dcb_tx_hw_config(struct rte_eth_dev *dev,
> struct ixgbe_dcb_config *dcb_config) {
> uint32_t reg;
> uint32_t q;
> + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>
> PMD_INIT_FUNC_TRACE();
> if (hw->mac.type != ixgbe_mac_82598EB) { @@ -3339,11 +3340,17 @@
> ixgbe_dcb_tx_hw_config(struct ixgbe_hw *hw,
> if (dcb_config->vt_mode)
> reg |= IXGBE_MTQC_VT_ENA;
> IXGBE_WRITE_REG(hw, IXGBE_MTQC, reg);
> -
> - /* Disable drop for all queues */
> - for (q = 0; q < 128; q++)
> - IXGBE_WRITE_REG(hw, IXGBE_QDE,
> - (IXGBE_QDE_WRITE | (q <<
> IXGBE_QDE_IDX_SHIFT)));
> + if (RTE_ETH_DEV_SRIOV(dev).active == 0) {
> + /* Disable drop for all queues in VMDQ mode*/
> + for (q = 0; q < 128; q++)
> + IXGBE_WRITE_REG(hw, IXGBE_QDE,
> + (IXGBE_QDE_WRITE | (q <<
> IXGBE_QDE_IDX_SHIFT) | IXGBE_QDE_ENABLE));
> + } else {
> + /* Enable drop for all queues in SRIOV mode */
> + for (q = 0; q < 128; q++)
> + IXGBE_WRITE_REG(hw, IXGBE_QDE,
> +
[dpdk-dev] [RFC] net/ixgbe: start rxtx after all nic config done.
Hi Wei, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of wang wei > Sent: Tuesday, September 6, 2016 8:05 PM > To: thomas.monjalon at 6wind.com > Cc: dev at dpdk.org > Subject: [dpdk-dev] [RFC] net/ixgbe: start rxtx after all nic config done. > > if start rxtx before flow director config, will cause driver can't receive > packets > from nic. > > Signed-off-by: wang wei I think you're right. We should start rx/tx after the config is done.
[dpdk-dev] [PATCH v2] net/ixgbe: use queues assigned to PF instad of VF
Hi, > -Original Message- > From: Alex Zelezniak [mailto:alexz at att.com] > Sent: Tuesday, August 30, 2016 9:23 AM > To: dev at dpdk.org > Cc: Lu, Wenzhuo; Yigit, Ferruh; stephen at networkplumber.org; azelezniak > Subject: [PATCH v2] net/ixgbe: use queues assigned to PF instad of VF > > From: azelezniak > > v2: > * shorten Subject line > * added more thorough description > > v1: > this patch uses queues which belong to PF instead of queus 0 - nb_rx_queues > which belong to VF0 in SR-IOV configuration > > Signed-off-by: Alex Zelezniak Acked-by: Wenzhuo Lu Thanks for the patch:)
[dpdk-dev] [PATCH 1/2] net/ixgbe: fix mbufs leakage during Rx queue release
Hi,
> -Original Message-
> From: Kylulin, Yury
> Sent: Tuesday, August 30, 2016 12:51 AM
> To: Zhang, Helin; Ananyev, Konstantin; Wu, Jingjing
> Cc: Lu, Wenzhuo; dev at dpdk.org; Kylulin, Yury
> Subject: [PATCH 1/2] net/ixgbe: fix mbufs leakage during Rx queue release
>
> For the vector PMD release all mbufs from the Rx queue if no packets received
> after device start.
>
> Fixes: 11b220c6498d ("ixgbe: fix release queue mbufs")
>
> Signed-off-by: Yury Kylulin
Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
Hi Bernard,
> -Original Message-
> From: Iremonger, Bernard
> Sent: Friday, August 26, 2016 6:04 PM
> To: Lu, Wenzhuo; dev at dpdk.org
> Cc: De Lara Guarch, Pablo; Wu, Jingjing
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
>
> Hi Wenzhuo,
>
>
>
> > > > > If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues
> > > > > respectively, there is a failure when the port is started in
> > > > > ixgbe_check_mq_mode() at line
> > > > > 1990 in ixgbe_ethdev.c.
> > > > > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be
> > > > > less than or equal to 1.
> > > > I haven't hit this issue. Would you like to give more details
> > > > about how to hit it? I'll check if I miss something.
> > >
> > > There is a Niantic PF and VF bound to igb_uio. Port 0 is the PF and
> > > Port 1 is the VF.
> > > ./testpmd -c 3f -l 1-5 -n 4 -- -i
> > > testpmd> set corelist 2,3,4,5
> > > testpmd> port stop 0 /* PF is 0 */
> > > testpmd> port config 0 dcb vt on 4 pfc on port start 0 /* PF is 0 */
> > > line 1990 ixgbe_ethdev.c
> > > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less
> > > than or equal to 1.
> > > /* Works if nb_rx_q and nb_tx_q set to 1 */
> > To my opinion, it's a by-design limitation. After using the DCB
> > configuration CLI, the queue number is set to a fix number which is
> > the max number. But as you pointed, when SRIOV is active there's
> > another requirement for the queue number.
> > We need to investigate deeper and find a solution for it. But I think
> > it's another story. We need another patch for it.
>
> Line 1997 in testpmd.c
> 1997 if (dcb_mode == DCB_VT_ENABLED) {
> nb_rxq = rte_port->dev_info.max_rx_queues;
> nb_txq = rte_port->dev_info.max_tx_queues;
> } else {
> 2001 /*if vt is disabled, use all pf queues */
> if (rte_port->dev_info.vmdq_pool_base == 0) {
> nb_rxq = rte_port->dev_info.max_rx_queues;
> nb_txq = rte_port->dev_info.max_tx_queues;
> } else {
> nb_rxq = (queueid_t)num_tcs;
> nb_txq = (queueid_t)num_tcs;
> }
> }
>
> The comment at line 2001 implies that when dcb_mode is DCB_VT_ENABLED all
> pf queues should not be used.
> When dcb_mode is DCB_VT_ENABLED, setting nb_rxq and nb_txq equal to 1
> works when the PF (port 0) is started.
> When dcb_mode is DCB_VT_ENABLED, setting nb_rxq to max_rx_queues and
> nb_txq to max_tx_queues results in the following failure in ixgbe when the PF
> (port 0) is started.
> SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or
> equal to 1.
PF and its VF port cannot be used in the same APP. As VF depends on its PF. We
must wait until PF is completed, then run VF. Normally, like use DPDK PF on
host, then use DPDK VF on guest. If you use PF and VF on parallel, the behavior
is unpredicted. I think that's why you see this prompt " SRIOV is active,
nb_rx_q=128 nb_tx_q=128 queue number must be less than or equal to 1.".
I've tried PF only testpmd. The result is like " PMD: ixgbe_check_mq_mode():
SRIOV active, unsupported mq_mode rx 6.". It means DCB is not supported by DPDK
PF.
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
Hi Luca, > -Original Message- > From: Luca Boccassi [mailto:lboccass at Brocade.com] > Sent: Friday, August 26, 2016 8:58 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > On Mon, 2016-07-11 at 15:43 +, Luca Boccassi wrote: > > On Mon, 2016-07-11 at 13:02 +0100, Luca Boccassi wrote: > > > On Mon, 2016-07-11 at 01:32 +, Lu, Wenzhuo wrote: > > > > > > > > > > Unfortunately I found one issue: if PF is down, and then the VF > > > > > on the guest is down as well (ip link down) and then goes back > > > > > up before the PF, then calling rte_eth_dev_reset will return 0 > > > > > (success), even though the PF is still down and it should fail. This > > > > > is with > ixgbe. Any idea what could be the problem? > > > > I've found this interesting thing. I believe it?s the HW difference > > > > between > igb and ixgbe. When the link is down, ixgbe VF can be reset successfully but > igb > VF cannot. The expression is the registers of the ixgbe VF can be accessed > when > the PF link is down but igb VF cannot. > > > > It means, on ixgbe, when PF link is down, we reset the VF link. Then PF > > > > link > is up, we receive the message again and reset the VF link again. > > > > > > What message do you refer to here? I am seeing the RESET callback > > > only when the PF goes down, not when it goes up. > > > > > > At the moment, with ixgbe, this happens: > > > > > > PF down -> reset notification, rte_eth_dev_reset keeps failing -> VF > > > down -> VF up -> rte_eth_dev_reset in a loop/timer succeeds -> PF up > > > -> VF link has no-carrier, and traffic does NOT go through > > > > > > The problem is that there is just no way of being notified that PF > > > is up, and if rte_eth_dev_reset succeeds I have no way of knowing > > > that I need to run it again. > > > > I was now able to solve this use case, by having the rte_eth_dev_reset > > implementations return -EAGAIN if the dev is not up. This way I know, > > in the application, that I have to try again. What do you think? > > > > IMHO it makes sense, as the reset does not actually succeeds, and the > > caller should try again. The diff is very trivial, and attached for > > reference. > > Hi, > > Is there any update on resubmitting this patchset for 16.11? Thanks! Sorry, we're short of hands, so this feature is planned to R17.02. > > -- > Kind regards, > Luca Boccassi
[dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
Hi Bernard,
> > > >
> > > > The return value of rte_eth_dev_configure() should be checked.
> > > > Calling rte_eth_dev_configure() with parameters nb_rx_q and
> > > > nb_tx_q equal to 0 returns -EINVAL, and does nothing.
> > > > Should the values of nb_rx_q and nb_tx_q be non zero?
> > The 0 is used on purpose. Because I don't want to configure the
> > queues. The only purpose is to make all the configuration to write
> > into the device. And that's why the return value is ignored by (void).
>
> It might be useful to add a comment to explain why rte_eth_dev_configure() is
> being called in this way.
Thanks for the suggestion. I'll add it:)
> > >
> > > If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues
> > > respectively, there is a failure when the port is started in
> > > ixgbe_check_mq_mode() at line
> > > 1990 in ixgbe_ethdev.c.
> > > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less
> > > than or equal to 1.
> > I haven't hit this issue. Would you like to give more details about
> > how to hit it? I'll check if I miss something.
>
> There is a Niantic PF and VF bound to igb_uio. Port 0 is the PF and Port 1 is
> the
> VF.
> ./testpmd -c 3f -l 1-5 -n 4 -- -i
> testpmd> set corelist 2,3,4,5
> testpmd> port stop 0 /* PF is 0 */
> testpmd> port config 0 dcb vt on 4 pfc on port start 0 /* PF is 0 */
> line 1990 ixgbe_ethdev.c
> SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or
> equal to 1.
> /* Works if nb_rx_q and nb_tx_q set to 1 */
To my opinion, it's a by-design limitation. After using the DCB configuration
CLI, the queue number is set to a fix number which is the max number. But as
you pointed, when SRIOV is active there's another requirement for the queue
number.
We need to investigate deeper and find a solution for it. But I think it's
another story. We need another patch for it.
>
> testpmd> show port dcb_tc 0
> DCB infos for port 0
> TC NUMBER: 4 (showing 8 a lot of the time ??) /* port start should be
> called
> first */
> TC : 0 1 2 3
> Priority : 0 1 2 3
> BW percent :25% 25% 25% 25%
> RXQ base : 0 1 2 3
> RXQ number : 1 1 1 1
> TXQ base : 0 1 2 3
> TXQ number : 1 1 1 1
> testpmd> show port dcb_tc 1
> Failed to get dcb infos on port 1
>
> > >
> > > nb_rxq and nb_txq are equal to 1 at this point, if they are not
> > > changed, port start completes successfully.
> > >
> > >
> > > > > } else {
> > > > > /*if vt is disabled, use all pf queues */
> > > > > - if (dev_info.vmdq_pool_base == 0) {
> > > > > - nb_rxq = dev_info.max_rx_queues;
> > > > > - nb_txq = dev_info.max_tx_queues;
> > > > > + if (rte_port->dev_info.vmdq_pool_base == 0) {
> > > > > + nb_rxq = rte_port->dev_info.max_rx_queues;
> > > > > + nb_txq = rte_port->dev_info.max_tx_queues;
> > > > > } else {
> > > > > nb_rxq = (queueid_t)num_tcs;
> > > > > nb_txq = (queueid_t)num_tcs; @@ -1997,16 +2010,6
> @@
> > > > > init_port_dcb_config(portid_t pid,
> > > > > }
> > > > > rx_free_thresh = 64;
> > > > >
> > > > > - memset(_conf, 0, sizeof(struct rte_eth_conf));
> > > > > - /* Enter DCB configuration status */
> > > > > - dcb_config = 1;
> > > > > -
> > > > > - /*set configuration of DCB in vt mode and DCB in non-vt mode*/
> > > > > - retval = get_eth_dcb_conf(_conf, dcb_mode, num_tcs,
> > > > > pfc_en);
> > > > > - if (retval < 0)
> > > > > - return retval;
> > > > > -
> > > > > - rte_port = [pid];
> > > > > memcpy(_port->dev_conf, _conf, sizeof(struct
> > > > > rte_eth_conf));
> > > > >
> > > > > rxtx_port_config(rte_port);
> > > > > --
> > > > > 1.9.3
> Regards,
>
> Bernard
[dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
Hi Bernard,
> -Original Message-
> From: Iremonger, Bernard
> Sent: Wednesday, August 24, 2016 11:22 PM
> To: Iremonger, Bernard; Lu, Wenzhuo; dev at dpdk.org
> Cc: De Lara Guarch, Pablo; Lu, Wenzhuo
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
>
> Hi Wenzhuo,
>
>
>
> > > Subject: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on
> > > ixgbe
> > >
> > > An issue is found that DCB cannot be configured on ixgbe NICs. It's
> > > said the TX queue number is not right.
> > > On ixgbe the max TX queue number is not fixed, it depends on the
> > > multi- queue mode.
> > >
> > > This patch adds the device configuration before getting info in the
> > > DCB configuration process. So the right info can be got depending on
> > > the configuration.
> > >
> > > Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx
> > > queues)
>
> As the fix in this patch is to testpmd, I don't think the fixes line is
> correct.
The bug is introduced by this patch. Before this patch, APP need not care about
the multi-queue mode when it want to get the max queue number.
>
> > > Signed-off-by: Wenzhuo Lu
> > > ---
> > > app/test-pmd/testpmd.c | 39 +--
> > > 1 file changed, 21 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > 1428974..ba41bea 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -1962,17 +1962,30 @@ init_port_dcb_config(portid_t pid,
> > >uint8_t pfc_en)
> > > {
> > > struct rte_eth_conf port_conf;
> > > - struct rte_eth_dev_info dev_info;
> > > struct rte_port *rte_port;
> > > int retval;
> > > uint16_t i;
> > >
> > > - rte_eth_dev_info_get(pid, _info);
> > > + rte_port = [pid];
> > > +
> > > + memset(_conf, 0, sizeof(struct rte_eth_conf));
> > > + /* Enter DCB configuration status */
> > > + dcb_config = 1;
> > > +
> > > + /*set configuration of DCB in vt mode and DCB in non-vt mode*/
> > > + retval = get_eth_dcb_conf(_conf, dcb_mode, num_tcs,
> > > pfc_en);
> > > + if (retval < 0)
> > > + return retval;
> > > + port_conf.rxmode.hw_vlan_filter = 1;
> > > +
> > > + (void)rte_eth_dev_configure(pid, 0, 0, _conf);
> >
> > The return value of rte_eth_dev_configure() should be checked.
> > Calling rte_eth_dev_configure() with parameters nb_rx_q and nb_tx_q
> > equal to 0 returns -EINVAL, and does nothing.
> > Should the values of nb_rx_q and nb_tx_q be non zero?
The 0 is used on purpose. Because I don't want to configure the queues. The
only purpose is to make all the configuration to write into the device. And
that's why the return value is ignored by (void).
>
> The call to rte_eth_dev_configure() may not be necessary, as it is also called
> when the port is started.
In rte_ethdev.h, we can see it's said that rte_eth_dev_configure should be
called before any other function of ethernet API. As the DCB configuration is
changed here. We have to call rte_eth_dev_configure to make the configuration
is right before we can use rte_eth_dev_info_get.
>
> > > + rte_eth_dev_info_get(pid, _port->dev_info);
> > >
> > > /* If dev_info.vmdq_pool_base is greater than 0,
> > >* the queue id of vmdq pools is started after pf queues.
> > >*/
> > > - if (dcb_mode == DCB_VT_ENABLED && dev_info.vmdq_pool_base >
> > > 0) {
> > > + if (dcb_mode == DCB_VT_ENABLED &&
> > > + rte_port->dev_info.vmdq_pool_base > 0) {
> > > printf("VMDQ_DCB multi-queue mode is nonsensical"
> > > " for port %d.", pid);
> > > return -1;
> > > @@ -1982,13 +1995,13 @@ init_port_dcb_config(portid_t pid,
> > >* and has the same number of rxq and txq in dcb mode
> > >*/
> > > if (dcb_mode == DCB_VT_ENABLED) {
> > > - nb_rxq = dev_info.max_rx_queues;
> > > - nb_txq = dev_info.max_tx_queues;
> > > + nb_rxq = rte_port->dev_info.max_rx_queues;
> > > + nb_txq = rte_port->dev_info.max_tx_queues;
>
> If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues respectively,
> there is a failure when the port is started in ixgbe_check_mq_mode() at line
> 1990 in ixgbe_ethdev.c.
> SRI
[dpdk-dev] SR-IOV: dpdk16.07 on guest and ixgbe 3.19.1 on host - will that be a problem ? (requested invalid api version)
HI Gopakumar, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Gopakumar > Choorakkot Edakkunni > Sent: Saturday, August 20, 2016 10:57 PM > To: dev at dpdk.org > Subject: [dpdk-dev] SR-IOV: dpdk16.07 on guest and ixgbe 3.19.1 on host - will > that be a problem ? (requested invalid api version) > > I see the below message "requested invalid api version" on host ? Will that > be an > issue or is it benign ? Should I upgrade the host ixgbe to a later version ? > > On the host: > > *[69576.199458] ixgbe :88:00.1 p4p2: VF 2 requested invalid api version > 3* It means ixgbe doesn't support ixgbe_mbox_api_12. I suggest using something newer.
[dpdk-dev] [RFC v2] ethdev: introduce generic flow API
Hi Adrien, Thanks for the V2. May I ask a question that may a little out of the scope here. As currently we don't store all the flow rules in the driver of Intel NICs, we're trying to fill this gap. Considering we need to order the flow rules by the priority, I think it's better to introduce avl tree or RB tree or something like that. We can transplant the avl tree code from FreeBSD. But it doesn't make sense to put it in the PMD. As you mentioned you'll provide some common code in the lib, will you provide avl tree or something similar in the common code? If you have already done it, we need not waste time to do the same thing again :)
[dpdk-dev] VLAN with ixgbevf pmd
Hi Souvik, Take testpmd for example, normally we should disable HW vlan filter by adding parameter " --disable-hw-vlan-filter " or CLI " port config all hw-vlan-filter off". Hope it can help :) Best regards Wenzhuo Lu > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Dey, Souvik > Sent: Thursday, August 18, 2016 10:47 PM > To: dev at dpdk.org > Subject: [dpdk-dev] VLAN with ixgbevf pmd > > Hi, > > > > I am trying to get tagged packets to work in SRIOV mode. I am using dpdk 2.1 > version with an application on KVM. > > The setup is as below: The same configuration works for untagged packets. > > Guest VM (Virtual Function/Created tagged kni interfaces)--- > KVM host > (PF/no > tag on the VF ) -->Client server > > When the packet is tagged (vlan tag/id) the packet is sent from kni > interface to > the application is it received with the tag and is also sent out to the pmd. > But the > packets does not go out of the host. Neither any tagged packets are coming in > from out to the application. The ol_flags is set to 0 in my application. > > Can someone let me know what I am missing? Do we need to do some specific > configuration on the rx or tx ports for this ? Does the vlan id configured on > the > kni gets peculated down to the vf of the host too which is causing the issue ? > > > > Any help would be highly appreciated! > > > > -- > > Regards, > > Souvik
[dpdk-dev] Mbuf leak issue with IXGBE in vector mod
Hi Ori,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ori Zakin
> Sent: Thursday, August 4, 2016 11:38 PM
> To: Zhang, Helin; Ananyev, Konstantin; dev at dpdk.org
> Subject: [dpdk-dev] Mbuf leak issue with IXGBE in vector mod
>
> Hi,
>
>
> 1. When calling rte_eth_dev_stop mbuf pool is depleted.
> There appears to be a race condition that occurs when RTE_IXGBE_INC_VECTOR
> is defined:
> ixgbe_reset_rx_queue(struct ixgbe_adapter *adapter, struct ixgbe_rx_queue
> *rxq) { ??.
>
> #ifdef RTE_IXGBE_INC_VECTOR
> rxq->rxrearm_start = 0;
> sleep here appears to solve
> rxq->rxrearm_nb = 0;
> #endif
>
>
> Behaviour also described here:
> http://dpdk.org/ml/archives/users/2016-April/000488.html
>
>
> 2. Steps to recreate issue:
> * rte_mempool_free_count
> * rte_eth_dev_stop
> * rte_mempool_free_count - should see a spike in allocated mbufs from
> mempool.
Have you stopped the rx/tx before do this stop/start? I think we'll hit this
memory leak problem if the traffic is not stopped. There's an assumption that
it?s APP's responsibility to stop rx/tx before operating the ports as we know
dpdk is lockless.
>
> 3. 2 workarounds that appear to work:
> * Set CONFIG_RTE_IXGBE_INC_VECTOR=n.
> * Add sleep in ixgbe_reset_rx_queue
>
>
> Regards.
> Ori Zakin
[dpdk-dev] VF default MAC change
Hi Garik, From: Garik E [mailto:kira...@gmail.com] Sent: Monday, August 1, 2016 12:40 AM To: dev at dpdk.org Cc: Lu, Wenzhuo Subject: VF default MAC change Hello, I'm looking for API to update default VF MAC address during DPDK port initialization. The API should replace shell command `ip link set vf mac ' Target controllers are Intel 10G, Intel 40G [Wenzhuo] I think you?re looking for this one, rte_eth_dev_default_mac_addr_set. But it?s not supported by all the NICs. Talking about VF, only igb and ixgbe support it. Thank you.
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Adrien, > -Original Message- > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com] > Sent: Thursday, July 21, 2016 8:48 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Thomas Monjalon; Zhang, Helin; Wu, Jingjing; Rasesh Mody; > Ajit Khaparde; Rahul Lakkireddy; Jan Medala; John Daley; Chen, Jing D; > Ananyev, > Konstantin; Matej Vido; Alejandro Lucero; Sony Chacko; Jerin Jacob; De Lara > Guarch, Pablo; Olga Shern > Subject: Re: [RFC] Generic flow director/filtering/classification API > > Hi Wenzhuo, > > It seems that we agree on about everything now, just a few more comments > below after snipping the now irrelevant parts. Yes, I think we?re agree with each other now. And thanks for the additional explanation :)
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Adrien,
> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Wednesday, July 20, 2016 6:41 PM
> To: Lu, Wenzhuo
> Cc: dev at dpdk.org; Thomas Monjalon; Zhang, Helin; Wu, Jingjing; Rasesh Mody;
> Ajit Khaparde; Rahul Lakkireddy; Jan Medala; John Daley; Chen, Jing D;
> Ananyev,
> Konstantin; Matej Vido; Alejandro Lucero; Sony Chacko; Jerin Jacob; De Lara
> Guarch, Pablo; Olga Shern
> Subject: Re: [RFC] Generic flow director/filtering/classification API
>
> Hi Wenzhuo,
>
> On Wed, Jul 20, 2016 at 02:16:51AM +, Lu, Wenzhuo wrote:
> [...]
> > > So, today an application cannot combine N-tuple and FDIR flow rules
> > > and get a reliable outcome, unless it is designed for specific
> > > devices with a known behavior.
> > >
> > > > What's the right behavior of PMD if APP want to create a flow
> > > > director rule
> > > which has a higher or even equal priority than an existing n-tuple
> > > rule? Should PMD return fail?
> > >
> > > First remember applications only deal with the generic API, PMDs are
> > > responsible for choosing the most appropriate HW implementation to
> > > use according to the requested flow rules (FDIR, N-tuple or anything
> > > else).
> > >
> > > For the specific case of FDIR vs N-tuple, if the underlying HW
> > > supports both I do not see why the PMD would create a N-tuple rule.
> > > Doesn't FDIR support everything N-tuple can do and much more?
> > Talking about the filters, fdir can cover n-tuple. I think that's why i40e
> > only
> supports fdir but not n-tuple. But n-tuple has its own highlight. As we know,
> at
> least on intel NICs, fdir only supports per device mask. But n-tuple can
> support
> per rule mask.
> > As every pattern has spec and mask both, we cannot guarantee the masks are
> same. I think ixgbe will try to use n-tuple first if can. Because even the
> masks are
> different, we can support them all.
>
> OK, makes sense. In that case existing rules may indeed prevent subsequent
> ones from getting created if their priority is wrong. I do not think there is
> a way
> around that if the application needs this exact ordering.
Agree. I don?t see any workaround either. PMD has to return fail sometimes.
>
> > > Assuming such a thing happened anyway, that the PMD had to create a
> > > rule using a high priority filter type and that the application
> > > requests the creation of a rule that can only be done using a lower
> > > priority filter type, but also requested a higher priority for that rule,
> > > then yes,
> it should obviously fail.
> > >
> > > That is, unless the PMD can perform some kind of workaround to have both.
> > >
> > > > If so, do we need more fail reasons? According to this RFC, I
> > > > think we need
> > > return " EEXIST: collision with an existing rule. ", but it's not
> > > very clear, APP doesn't know the problem is priority, maybe more detailed
> reason is helpful.
> > >
> > > Possibly, I've defined a basic set of errors, there are quite a
> > > number of errno values to choose from. However I think we should not
> define too many values.
> > > In my opinion the basic set covers every possible failure:
> > >
> > > - EINVAL: invalid format, rule is broken or cannot be understood by the
> > > PMD
> > > anyhow.
> > >
> > > - ENOTSUP: pattern/actions look fine but something in the requested rule
> > > is
> > > not supported and thus cannot be applied.
> > >
> > > - EEXIST: pattern/actions are fine and could have been applied if only
> > > some
> > > other rule did not prevent the PMD to do it (I see it as the closest
> > > thing
> > > to "ETOOBAD" which unfortunately does not exist).
> > >
> > > - ENOMEM: like EEXIST, except it is due to the lack of resources not
> > > because
> > > of another rule. I wasn't sure which of ENOMEM or ENOSPC was better but
> > > settled on ENOMEM as it is well known. Still open to debate.
> > >
> > > Errno values are only useful to get a rough idea of the reason, and
> > > another mechanism is needed to pinpoint the exact problem for
> > > debugging/reporting purposes, something like:
> > >
> > > enum rte_flow_error_type {
> > > RTE_FLOW_ERROR_TYPE_NONE,
> > > RTE_FLOW_ERROR_TYPE_UNKNOWN,
> > > RTE_FLOW_ERROR_TYPE_PRIORITY,
> > >
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Adrien, > -Original Message- > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com] > Sent: Tuesday, July 19, 2016 9:12 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Thomas Monjalon; Zhang, Helin; Wu, Jingjing; Rasesh Mody; > Ajit Khaparde; Rahul Lakkireddy; Jan Medala; John Daley; Chen, Jing D; > Ananyev, > Konstantin; Matej Vido; Alejandro Lucero; Sony Chacko; Jerin Jacob; De Lara > Guarch, Pablo; Olga Shern > Subject: Re: [RFC] Generic flow director/filtering/classification API > > On Tue, Jul 19, 2016 at 08:11:48AM +, Lu, Wenzhuo wrote: > > Hi Adrien, > > Thanks for your clarification. Most of my questions are clear, but still > something may need to be discussed, comment below. > > Hi Wenzhuo, > > Please see below. > > [...] > > > > > Requirements for a new API: > > > > > > > > > > - Flexible and extensible without causing API/ABI problems for > > > > > existing > > > > > applications. > > > > > - Should be unambiguous and easy to use. > > > > > - Support existing filtering features and actions listed in `Filter > > > > > types`_. > > > > > - Support packet alteration. > > > > > - In case of overlapping filters, their priority should be well > > > > > documented. > > > > Does that mean we don't guarantee the consistent of priority? The > > > > priority can > > > be different on different NICs. So the behavior of the actions can be > different. > > > Right? > > > > > > No, the intent is precisely to define what happens in order to get a > > > consistent result across different devices, and document cases with > undefined behavior. > > > There must be no room left for interpretation. > > > > > > For example, the API must describe what happens when two overlapping > > > filters (e.g. one matching an Ethernet header, another one matching > > > an IP header) match a given packet at a given priority level. > > > > > > It is documented in section 4.1.1 (priorities) as "undefined behavior". > > > Applications remain free to do it and deal with consequences, at > > > least they know they cannot expect a consistent outcome, unless they > > > use different priority levels for both rules, see also 4.4.5 (flow rules > > > priority). > > > > > > > Seems the users still need to aware the some details of the HW? Do > > > > we need > > > to add the negotiation for the priority? > > > > > > Priorities as defined in this document may not be directly mappable > > > to HW capabilities (e.g. HW does not support enough priorities, or > > > that some corner case make them not work as described), in which > > > case the PMD may choose to simulate priorities (again 4.4.5), as > > > long as the end result follows the specification. > > > > > > So users must not be aware of some HW details, the PMD does and must > > > perform the needed workarounds to suit their expectations. Users may > > > only be impacted by errors while attempting to create rules that are > > > either unsupported or would cause them (or existing rules) to diverge from > the spec. > > The problem is sometime the priority of the filters is fixed according > > to > > > HW's implementation. For example, on ixgbe, n-tuple has a higher > > > priority than flow director. > > As a side note I did not know that N-tuple had a higher priority than flow > director on ixgbe, priorities among filter types do not seem to be documented > at > all in DPDK. This is one of the reasons I think we need a generic API to > handle > flow configuration. Totally agree with you. We haven't documented the info well enough. And even we do that, users have to study the details of every NIC, it can still make the filters very hard to use. I believe a generic API is very helpful here :) > > > So, today an application cannot combine N-tuple and FDIR flow rules and get a > reliable outcome, unless it is designed for specific devices with a known > behavior. > > > What's the right behavior of PMD if APP want to create a flow director rule > which has a higher or even equal priority than an existing n-tuple rule? > Should > PMD return fail? > > First remember applications only deal with the generic API, PMDs are > responsible for choosing the most appropriate HW implementation to use > according to the requested flow rules (FDIR, N-tuple or anything else). > > For the specific case of FDIR
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Adrien, Thanks for your clarification. Most of my questions are clear, but still something may need to be discussed, comment below. > -Original Message- > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com] > Sent: Thursday, July 7, 2016 6:27 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Thomas Monjalon; Zhang, Helin; Wu, Jingjing; Rasesh Mody; > Ajit Khaparde; Rahul Lakkireddy; Jan Medala; John Daley; Chen, Jing D; > Ananyev, > Konstantin; Matej Vido; Alejandro Lucero; Sony Chacko; Jerin Jacob; De Lara > Guarch, Pablo; Olga Shern > Subject: Re: [RFC] Generic flow director/filtering/classification API > > Hi Lu Wenzhuo, > > Thanks for your feedback, I'm replying below as well. > > On Thu, Jul 07, 2016 at 07:14:18AM +, Lu, Wenzhuo wrote: > > Hi Adrien, > > I have some questions, please see inline, thanks. > > > > > -Original Message- > > > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com] > > > Sent: Wednesday, July 6, 2016 2:17 AM > > > To: dev at dpdk.org > > > Cc: Thomas Monjalon; Zhang, Helin; Wu, Jingjing; Rasesh Mody; Ajit > > > Khaparde; Rahul Lakkireddy; Lu, Wenzhuo; Jan Medala; John Daley; > > > Chen, Jing D; Ananyev, Konstantin; Matej Vido; Alejandro Lucero; > > > Sony Chacko; Jerin Jacob; De Lara Guarch, Pablo; Olga Shern > > > Subject: [RFC] Generic flow director/filtering/classification API > > > > > > > > > Requirements for a new API: > > > > > > - Flexible and extensible without causing API/ABI problems for existing > > > applications. > > > - Should be unambiguous and easy to use. > > > - Support existing filtering features and actions listed in `Filter > > > types`_. > > > - Support packet alteration. > > > - In case of overlapping filters, their priority should be well > > > documented. > > Does that mean we don't guarantee the consistent of priority? The priority > > can > be different on different NICs. So the behavior of the actions can be > different. > Right? > > No, the intent is precisely to define what happens in order to get a > consistent > result across different devices, and document cases with undefined behavior. > There must be no room left for interpretation. > > For example, the API must describe what happens when two overlapping filters > (e.g. one matching an Ethernet header, another one matching an IP header) > match a given packet at a given priority level. > > It is documented in section 4.1.1 (priorities) as "undefined behavior". > Applications remain free to do it and deal with consequences, at least they > know > they cannot expect a consistent outcome, unless they use different priority > levels for both rules, see also 4.4.5 (flow rules priority). > > > Seems the users still need to aware the some details of the HW? Do we need > to add the negotiation for the priority? > > Priorities as defined in this document may not be directly mappable to HW > capabilities (e.g. HW does not support enough priorities, or that some corner > case make them not work as described), in which case the PMD may choose to > simulate priorities (again 4.4.5), as long as the end result follows the > specification. > > So users must not be aware of some HW details, the PMD does and must > perform the needed workarounds to suit their expectations. Users may only be > impacted by errors while attempting to create rules that are either > unsupported > or would cause them (or existing rules) to diverge from the spec. The problem is sometime the priority of the filters is fixed according to HW's implementation. For example, on ixgbe, n-tuple has a higher priority than flow director. What's the right behavior of PMD if APP want to create a flow director rule which has a higher or even equal priority than an existing n-tuple rule? Should PMD return fail? If so, do we need more fail reasons? According to this RFC, I think we need return " EEXIST: collision with an existing rule. ", but it's not very clear, APP doesn't know the problem is priority, maybe more detailed reason is helpful. > > > Behavior > > > > > > > > > - API operations are synchronous and blocking (``EAGAIN`` cannot be > > > returned). > > > > > > - There is no provision for reentrancy/multi-thread safety, although > > > nothing > > > should prevent different devices from being configured at the same > > > time. PMDs may protect their control path functions accordingly. > > > > > > - Stopping the data path (TX/RX) should not be necessary when managing &
[dpdk-dev] No RX frames on Intel 82599 VF
From: Garik E [mailto:kira...@gmail.com] Sent: Tuesday, July 12, 2016 12:48 PM To: Lu, Wenzhuo Cc: dev at dpdk.org Subject: Re: [dpdk-dev] No RX frames on Intel 82599 VF Hi, On the S2600WT2 server, when DPDK is bound to VF, there is no incoming traffic. But when the same VF is bound to ixgbevf driver and configured as Linux interface, it works normally. I was able to run ping and ssh through that VF. So my guess is that the RX issue is not due to malfunction hardware. The same binary works correctly on S2600WTTR server with 82599 VF I also tested the application with Mellanox ConnectX-4 on both servers There were no issues with CX-4 PF and VF For some reason DPDK VF RX functionality does not work on S2600WT2. [Wenzhuo] No clue now. But I think you can compare what?s the difference between S2600WT2 and S2600WTTR. On Tue, Jul 12, 2016 at 4:24 AM, Lu, Wenzhuo mailto:wenzhuo.lu at intel.com>> wrote: Hi Garik, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org<mailto:dev-bounces at dpdk.org>] On > Behalf Of Garik E > Sent: Tuesday, July 12, 2016 2:22 AM > To: dev at dpdk.org<mailto:dev at dpdk.org> > Subject: [dpdk-dev] No RX frames on Intel 82599 VF > > Hello, > > > I have two Intel servers S2600WTTR and S2600WT2 both with 82599 10G > Ethernet Controllers > > I run the same DPDK application on both servers. > > The application works with one interface bound to physical or virtual PCI > function depending on configuration > > The S2600WTTR server receives incoming traffic on physical and virtual > functions S2600WTTR is working right? > > The S2600WT2 server receives traffic only on physical function > > When I bind S2600WT2 VF to ixgbevf driver and configure it as Linux ETH > interface, it works normally. Don't understand what you're doing here. And you say *works*? Is S2600WT2 the one not working? > > > Network sniffer shows that Ethernet frames arrive to S2600WT2 port and > frames are valid, > > however DPDK does not receive them. > > > Where can I start to debug this issue ? > > > OS: RHEL 6.6 x86-64 > > DPDK: 16-07-rc1
[dpdk-dev] No RX frames on Intel 82599 VF
Hi Garik, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Garik E > Sent: Tuesday, July 12, 2016 2:22 AM > To: dev at dpdk.org > Subject: [dpdk-dev] No RX frames on Intel 82599 VF > > Hello, > > > I have two Intel servers S2600WTTR and S2600WT2 both with 82599 10G > Ethernet Controllers > > I run the same DPDK application on both servers. > > The application works with one interface bound to physical or virtual PCI > function depending on configuration > > The S2600WTTR server receives incoming traffic on physical and virtual > functions S2600WTTR is working right? > > The S2600WT2 server receives traffic only on physical function > > When I bind S2600WT2 VF to ixgbevf driver and configure it as Linux ETH > interface, it works normally. Don't understand what you're doing here. And you say *works*? Is S2600WT2 the one not working? > > > Network sniffer shows that Ethernet frames arrive to S2600WT2 port and > frames are valid, > > however DPDK does not receive them. > > > Where can I start to debug this issue ? > > > OS: RHEL 6.6 x86-64 > > DPDK: 16-07-rc1
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
> -Original Message- > From: Luca Boccassi [mailto:lboccass at Brocade.com] > Sent: Monday, July 11, 2016 11:43 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > On Mon, 2016-07-11 at 13:02 +0100, Luca Boccassi wrote: > > On Mon, 2016-07-11 at 01:32 +, Lu, Wenzhuo wrote: > > > > > > > > Unfortunately I found one issue: if PF is down, and then the VF on > > > > the guest is down as well (ip link down) and then goes back up > > > > before the PF, then calling rte_eth_dev_reset will return 0 > > > > (success), even though the PF is still down and it should fail. This is > > > > with > ixgbe. Any idea what could be the problem? > > > I've found this interesting thing. I believe it?s the HW difference > > > between igb > and ixgbe. When the link is down, ixgbe VF can be reset successfully but igb > VF > cannot. The expression is the registers of the ixgbe VF can be accessed when > the PF link is down but igb VF cannot. > > > It means, on ixgbe, when PF link is down, we reset the VF link. Then PF > > > link is > up, we receive the message again and reset the VF link again. > > > > What message do you refer to here? I am seeing the RESET callback only > > when the PF goes down, not when it goes up. > > > > At the moment, with ixgbe, this happens: > > > > PF down -> reset notification, rte_eth_dev_reset keeps failing -> VF > > down -> VF up -> rte_eth_dev_reset in a loop/timer succeeds -> PF up > > -> VF link has no-carrier, and traffic does NOT go through > > > > The problem is that there is just no way of being notified that PF is > > up, and if rte_eth_dev_reset succeeds I have no way of knowing that I > > need to run it again. > > I was now able to solve this use case, by having the rte_eth_dev_reset > implementations return -EAGAIN if the dev is not up. This way I know, in the > application, that I have to try again. What do you think? > > IMHO it makes sense, as the reset does not actually succeeds, and the caller > should try again. The diff is very trivial, and attached for reference. Yes, I think the change is reasonable. Sorry, I didn?t realize you're talking about the code you have changed. Maybe we're not on the same page when discussing before :) > > -- > Kind regards, > Luca Boccassi > > > Make rte_eth_dev_reset return EAGAIN if VF down > > If VF is down the reset will not happen, so the driver should return > EAGAIN to signal the application that it needs to call again > rte_eth_dev_reset. > > Signed-off-by: Luca Boccassi --- > drivers/net/e1000/igb_ethdev.c|2 +- > drivers/net/i40e/i40e_ethdev_vf.c |2 +- > drivers/net/ixgbe/ixgbe_ethdev.c |2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -6235,7 +6235,7 @@ ixgbevf_dev_reset(struct rte_eth_dev *de > > /* Nothing needs to be done if the device is not started. */ > if (!dev->data->dev_started) > - return 0; > + return -EAGAIN; > > PMD_DRV_LOG(DEBUG, "Link up/down event detected."); > > --- a/drivers/net/i40e/i40e_ethdev_vf.c > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > @@ -1504,7 +1504,7 @@ i40evf_handle_vf_reset(struct rte_eth_de >I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private); > > if (!dev->data->dev_started) > - return 0; > + return -EAGAIN; > > adapter->reset_number = 1; > i40e_vf_reset_dev(dev); > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -2609,7 +2609,7 @@ igbvf_dev_reset(struct rte_eth_dev *dev, > > /* Nothing needs to be done if the device is not started. */ > if (!dev->data->dev_started) > - return 0; > + return -EAGAIN; > > PMD_DRV_LOG(DEBUG, "Link up/down event detected."); >
[dpdk-dev] [PATCH] lib/librte_ether: bypass code cleanup
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Monday, July 11, 2016 3:11 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [PATCH] lib/librte_ether: bypass code cleanup > > 2016-07-11 14:21, Wenzhuo Lu: > > In testpmd code, device id is used directly to check if bypass is > > supported. But APP should not know the details of HW, the NIC specific > > info should not be exposed here. > > Right > > > This patch adds a new rte API to check if bypass is supported. > > Hmmm. It's true it is cleaner. But I am not sure having a generic API for > bypass is > a good idea at all. > I was thinking to totally remove it. > Maybe we can try to have a specific API by including ixgbe_bypass.h in the > application. According to Jingjing's comments, I think we need not check if bypass is supported. Every API will return fail if it's not supported. I'll send a V2 to remove the bypass_is_supported.
[dpdk-dev] [PATCH] lib/librte_ether: bypass code cleanup
> -Original Message- > From: Wu, Jingjing > Sent: Monday, July 11, 2016 3:51 PM > To: Lu, Wenzhuo; dev at dpdk.org > Cc: thomas.monjalon at 6wind.com; Lu, Wenzhuo > Subject: RE: [dpdk-dev] [PATCH] lib/librte_ether: bypass code cleanup > > Hi, Wenzhuo > > I don't think you need to create a new API: rte_eth_dev_bypass_supported. > If bypass in not supported, the Ops for bypass feature will not be > implemented in > driver, or driver need to return NOT supported. > > So I think what you need do is just remove the NIC specific things from > testpmd, > including "bypass_is_supported". Agree, thanks for the comments!
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
> > Unfortunately I found one issue: if PF is down, and then the VF on the guest > is > down as well (ip link down) and then goes back up before the PF, then calling > rte_eth_dev_reset will return 0 (success), even though the PF is still down > and it > should fail. This is with ixgbe. Any idea what could be the problem? I've found this interesting thing. I believe it?s the HW difference between igb and ixgbe. When the link is down, ixgbe VF can be reset successfully but igb VF cannot. The expression is the registers of the ixgbe VF can be accessed when the PF link is down but igb VF cannot. It means, on ixgbe, when PF link is down, we reset the VF link. Then PF link is up, we receive the message again and reset the VF link again. But on igb, when PF link is down, we cannot reset VF link successfully, so when the PF link is up, we cannot receive the message. No trigger for us to reset the VF link again. That's why on igb we have to try again and again until it succeed, means until PF link is up. So the return 0 by rte_eth_dev_reset means the resetting succeeded, not mean the rx/tx is ready. Rx/tx has to depend on the PF link is up. > > -- > Kind regards, > Luca Boccassi
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
> > > > > > > > > Hello Wenzhuo, > > > > > > > > > > > > > > > > > > I'm testing this patchset, but I am sporadically running > > > > > > > > > into an issue where the VFs reset fails after the PF flaps. > > > > > > > > > > > > > > > > > > I have a VM running on a KVM box with a X540-AT2, passing 2 > > > > > > > > > VFs > in. > > > > > > > > > > > > > > > > > > I am using calling rte_eth_dev_reset in response to a > > > > > > > > > RTE_ETH_EVENT_INTR_RESET callback, and the following > > > > > > > > > errors appear in the > > > > > > > > > log: > > > > > > > > > > > > > > > > > > PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to update > > > > > > > > > link. > > > > > > > > > PMD: ixgbe_alloc_rx_queue_mbufs(): RX mbuf alloc failed > > > > > > > > > queue_id=0 > > > > > > > > > PMD: ixgbevf_dev_start(): Unable to initialize RX > > > > > > > > > hardware > > > > > > > > > (-12) > > > > > > > > > PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to start > > > > > > > > > device. > > > > > > > > > > > > > > > > > > Jumping in with GDB, it seems that the rte_rxmbuf_alloc > > > > > > > > > call in ixgbe_alloc_rx_queue_mbufs returns NULL at > > > > > > > > > iteration 64 out of > > > 2048. > > > > > > > > > The application has ~500 2MB hugepages, and there's 2GB > > > > > > > > > of free memory available on top of that. > > > > > > > > > > > > > > > > > > Have you seen this before? Any pointer or suggestion for > debugging? > > > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Kind regards, > > > > > > > > > Luca Boccassi > > > > > > > > I think the problem is the mbuf occupied by the packets is > > > > > > > > not released. This > > > > > > > memory has to be released by the APP, so my patches haven?t > > > > > > > covered > > > this. > > > > > > > Actually an example is needed to show how to use the reset API. > > > > > > > I plan to modify the testpmd. > > > > > > > > You may notice this feature is postponed to 16.11. Would > > > > > > > > you like to wait for > > > > > > > the new version that will include an example? > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > Unfortunately we need the VF reset working sooner than that, > > > > > > > so one way or the other I'll need to sort it out. Given I've > > > > > > > got a use case where this is happening, if it can be helpful > > > > > > > for you I'm more than happy to help as a guinea pig. If you > > > > > > > could please give some guidance/guidelines with regards to > > > > > > > which API to use to sort the mbuf > > > > > problem, I can try it out and give back some feedback. > > > > > > > > > > > > > > Thanks! > > > > > > I made a stupid mistake and deleted all my code. So, I have to > > > > > > take some time to rewrite it :( Attached the example I used to > > > > > > test the reset API. It's > > > > > modified from the l2fwd example. So you can compare it with > > > > > l2fwd to see what need to be added. > > > > > > Hopefully it can help :) > > > > > > > > > > Thanks! That made me understand a couple of things more, and > > > > > I've got past the problem. > > > > > > > > > > Unfortunately now there's a bigger issue - rte_eth_dev_reset is > > > > > a blocking > > > call. > > > > > the _RESET event callback is fired when the PF goes down, but > > > > > when I call rte_eth_dev_reset it will block until the PF goes back up. > > > > > There is no way, as far as I can see, to know if the PF is back > > > > > up before > > > calling rte_eth_dev_reset. > > > > > > > > > > This is a problem because, as far as I understand, I have to > > > > > call all the rte_eth_dev_ APIs from the same thread, in my case > > > > > the master thread, and I can't have that block potentially > > > > > indefinitely. > > > > > > > > > > Would it be possible to have 2 events instead of 1, one when the > > > > > PF goes down and one when it goes up? This way an application > > > > > would be able to soft-stop the port (drain queues, etc) when the > > > > > PF is down, and then call the reset API when it goes back up. > > > > > > > > > > Thanks! > > > > Sorry we cannot have 2 events now. There're 2 problems to have 2 events. > > > > 1, Normally we use kernel driver for PF. Now the kernel driver > > > > only have one > > > kind of message for link down and up. So we cannot tell if it's down or > > > up. > > > > 2, When the PF is down, if we don't reset the VF, VF is not working. > > > > It cannot receive any message from PF. So we cannot know that when > > > > PF is up. It means normally we have to reset VF twice when PF down > > > > and up. (Surely we can wait a while when we receive the message > > > > from PF until PF is up. But we cannot tell how long the time is > > > > appropriate. > > > > So this *wait a while* may work for flash.) > > > > > > Thanks for the clarification, I understand. > > > > > > The problem with a blocking call is that we basically need to spawn > > > one thread per rte_eth_dev_reset call,
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
> -Original Message- > From: Luca Boccassi [mailto:lboccass at Brocade.com] > Sent: Thursday, July 7, 2016 6:21 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > On Thu, 2016-07-07 at 01:09 +, Lu, Wenzhuo wrote: > > Hi Luca, > > > > > > > -Original Message- > > > From: Luca Boccassi [mailto:lboccass at Brocade.com] > > > Sent: Thursday, July 7, 2016 12:23 AM > > > To: Lu, Wenzhuo > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > > > > > On Wed, 2016-07-06 at 00:45 +, Lu, Wenzhuo wrote: > > > > Hi Luca, > > > > > > > > > -Original Message- > > > > > From: Luca Boccassi [mailto:lboccass at Brocade.com] > > > > > Sent: Tuesday, July 5, 2016 5:53 PM > > > > > To: Lu, Wenzhuo > > > > > Cc: dev at dpdk.org > > > > > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > > > > > > > > > On Tue, 2016-07-05 at 00:52 +, Lu, Wenzhuo wrote: > > > > > > Hi Luca, > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > From: Luca Boccassi [mailto:lboccass at Brocade.com] > > > > > > > Sent: Monday, July 4, 2016 11:48 PM > > > > > > > To: Lu, Wenzhuo > > > > > > > Cc: dev at dpdk.org > > > > > > > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF > > > > > > > link > > > > > > > > > > > > > > On Mon, 2016-06-20 at 14:24 +0800, Wenzhuo Lu wrote: > > > > > > > > If the PF link is down and up, VF link will not work > > > > > > > > accordingly. > > > > > > > > This patch set addes the support of VF link reset. So, > > > > > > > > when VF receices the messges of physical link down/up. APP > > > > > > > > can reset the VF link and let it recover. > > > > > > > > > > > > > > > > PS: This patch set is splitted from a previous patch set, > > > > > > > > *automatic link recovery on ixgbe/igb VF*, and it's base > > > > > > > > on the patch set *support mailbox interruption on ixgbe/igb VF*. > > > > > > > > > > > > > > > > Wenzhuo Lu (3): > > > > > > > > lib/librte_ether: support device reset > > > > > > > > ixgbe: implement device reset on VF > > > > > > > > igb: implement device reset on VF > > > > > > > > > > > > > > > > Zhe Tao (1): > > > > > > > > i40e: implement device reset on VF > > > > > > > > > > > > > > > > v1: > > > > > > > > - Added the implementation for the VF reset functionality. > > > > > > > > v2: > > > > > > > > - Changed the i40e related operations during VF reset. > > > > > > > > v3: > > > > > > > > - Resent the patches because of the mail sent issue. > > > > > > > > v4: > > > > > > > > - Removed some VF reset emulation code. > > > > > > > > v5: > > > > > > > > - Removed all the code related with lock. > > > > > > > > v6: > > > > > > > > - Updated the NIC feature overview matrix. > > > > > > > > - Added more explanation in the doxygen comment of reset API. > > > > > > > > > > > > > > > > doc/guides/nics/overview.rst | 1 + > > > > > > > > doc/guides/rel_notes/release_16_07.rst | 13 ++ > > > > > > > > drivers/net/e1000/igb_ethdev.c | 59 > > > > > > > > > drivers/net/i40e/i40e_ethdev.h | 4 ++ > > > > > > > > drivers/net/i40e/i40e_ethdev_vf.c | 83 > > > > > > > ++ > > > > > > > > drivers/net/i40e/i40e_rxtx.c | 10 > > > > > > > > drivers/net/i40e/i40e_rxtx.h | 4 ++ > > > > > > > > drivers/net/ixgbe/ixgbe_ethdev.c | 64 > &
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Adrien, I have some questions, please see inline, thanks. > -Original Message- > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com] > Sent: Wednesday, July 6, 2016 2:17 AM > To: dev at dpdk.org > Cc: Thomas Monjalon; Zhang, Helin; Wu, Jingjing; Rasesh Mody; Ajit Khaparde; > Rahul Lakkireddy; Lu, Wenzhuo; Jan Medala; John Daley; Chen, Jing D; Ananyev, > Konstantin; Matej Vido; Alejandro Lucero; Sony Chacko; Jerin Jacob; De Lara > Guarch, Pablo; Olga Shern > Subject: [RFC] Generic flow director/filtering/classification API > > > Requirements for a new API: > > - Flexible and extensible without causing API/ABI problems for existing > applications. > - Should be unambiguous and easy to use. > - Support existing filtering features and actions listed in `Filter types`_. > - Support packet alteration. > - In case of overlapping filters, their priority should be well documented. Does that mean we don't guarantee the consistent of priority? The priority can be different on different NICs. So the behavior of the actions can be different. Right? Seems the users still need to aware the some details of the HW? Do we need to add the negotiation for the priority? > > Flow rules can have several distinct actions (such as counting, > encapsulating, decapsulating before redirecting packets to a particular > queue, etc.), instead of relying on several rules to achieve this and having > applications deal with hardware implementation details regarding their > order. I think normally HW doesn't support several actions in one rule. If a rule has several actions, seems HW has to split it to several rules. The order can still be a problem. > > ``ETH`` > ^^^ > > Matches an Ethernet header. > > - ``dst``: destination MAC. > - ``src``: source MAC. > - ``type``: EtherType. > - ``tags``: number of 802.1Q/ad tags defined. > - ``tag[]``: 802.1Q/ad tag definitions, innermost first. For each one: > > - ``tpid``: Tag protocol identifier. > - ``tci``: Tag control information. "ETH" means all the parameters, dst, src, type... need to be matched? The same question for IPv4, IPv6 ... > > ``UDP`` > ^^^ > > Matches a UDP header. > > - ``sport``: source port. > - ``dport``: destination port. > - ``length``: UDP length. > - ``checksum``: UDP checksum. Why checksum? Do we need to filter the packets by checksum value? > > ``VOID`` (action) > ^ > > Used as a placeholder for convenience. It is ignored and simply discarded by > PMDs. Don't understand why we need VOID. If it?s about the format. Why not guarantee it in rte layer? > > Behavior > > > - API operations are synchronous and blocking (``EAGAIN`` cannot be > returned). > > - There is no provision for reentrancy/multi-thread safety, although nothing > should prevent different devices from being configured at the same > time. PMDs may protect their control path functions accordingly. > > - Stopping the data path (TX/RX) should not be necessary when managing flow > rules. If this cannot be achieved naturally or with workarounds (such as > temporarily replacing the burst function pointers), an appropriate error > code must be returned (``EBUSY``). PMD cannot stop the data path without adding lock. So I think if some rules cannot be applied without stopping rx/tx, PMD has to return fail. Or let the APP to stop the data path. > > - PMDs, not applications, are responsible for maintaining flow rules > configuration when stopping and restarting a port or performing other > actions which may affect them. They can only be destroyed explicitly. Don?t understand " They can only be destroyed explicitly." If a new rule has conflict with an old one, what should we do? Return fail? > > ``ANY`` pattern item > > > This pattern item stands for anything, which can be difficult to translate > to something hardware would understand, particularly if followed by more > specific types. > > Consider the following pattern: > > +---++ > | 0 | ETHER | > +---++ > | 1 | ANY (``min`` = 1, ``max`` = 1) | > +---++ > | 2 | TCP| > +---++ > > Knowing that TCP does not make sense with something other than IPv4 and IPv6 > as L3, such a pattern may be translated to two flow rules instead: > > +---++ > | 0 | ETHER | > +---++ > | 1 | IPV4 (zeroed mask) | > +---++ > | 2 | TCP| > +---++ > > +-
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
Hi Luca, > -Original Message- > From: Luca Boccassi [mailto:lboccass at Brocade.com] > Sent: Thursday, July 7, 2016 12:23 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > On Wed, 2016-07-06 at 00:45 +, Lu, Wenzhuo wrote: > > Hi Luca, > > > > > -Original Message- > > > From: Luca Boccassi [mailto:lboccass at Brocade.com] > > > Sent: Tuesday, July 5, 2016 5:53 PM > > > To: Lu, Wenzhuo > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > > > > > On Tue, 2016-07-05 at 00:52 +, Lu, Wenzhuo wrote: > > > > Hi Luca, > > > > > > > > > > > > > -----Original Message- > > > > > From: Luca Boccassi [mailto:lboccass at Brocade.com] > > > > > Sent: Monday, July 4, 2016 11:48 PM > > > > > To: Lu, Wenzhuo > > > > > Cc: dev at dpdk.org > > > > > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > > > > > > > > > On Mon, 2016-06-20 at 14:24 +0800, Wenzhuo Lu wrote: > > > > > > If the PF link is down and up, VF link will not work accordingly. > > > > > > This patch set addes the support of VF link reset. So, when VF > > > > > > receices the messges of physical link down/up. APP can reset > > > > > > the VF link and let it recover. > > > > > > > > > > > > PS: This patch set is splitted from a previous patch set, > > > > > > *automatic link recovery on ixgbe/igb VF*, and it's base on > > > > > > the patch set *support mailbox interruption on ixgbe/igb VF*. > > > > > > > > > > > > Wenzhuo Lu (3): > > > > > > lib/librte_ether: support device reset > > > > > > ixgbe: implement device reset on VF > > > > > > igb: implement device reset on VF > > > > > > > > > > > > Zhe Tao (1): > > > > > > i40e: implement device reset on VF > > > > > > > > > > > > v1: > > > > > > - Added the implementation for the VF reset functionality. > > > > > > v2: > > > > > > - Changed the i40e related operations during VF reset. > > > > > > v3: > > > > > > - Resent the patches because of the mail sent issue. > > > > > > v4: > > > > > > - Removed some VF reset emulation code. > > > > > > v5: > > > > > > - Removed all the code related with lock. > > > > > > v6: > > > > > > - Updated the NIC feature overview matrix. > > > > > > - Added more explanation in the doxygen comment of reset API. > > > > > > > > > > > > doc/guides/nics/overview.rst | 1 + > > > > > > doc/guides/rel_notes/release_16_07.rst | 13 ++ > > > > > > drivers/net/e1000/igb_ethdev.c | 59 > > > > > > > > > > > > drivers/net/i40e/i40e_ethdev.h | 4 ++ > > > > > > drivers/net/i40e/i40e_ethdev_vf.c | 83 > > > > > ++ > > > > > > drivers/net/i40e/i40e_rxtx.c | 10 > > > > > > drivers/net/i40e/i40e_rxtx.h | 4 ++ > > > > > > drivers/net/ixgbe/ixgbe_ethdev.c | 64 > +- > > > > > > drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- > > > > > > drivers/net/ixgbe/ixgbe_rxtx.c | 12 +++-- > > > > > > lib/librte_ether/rte_ethdev.c | 17 +++ > > > > > > lib/librte_ether/rte_ethdev.h | 24 ++ > > > > > > lib/librte_ether/rte_ether_version.map | 7 +++ > > > > > > 13 files changed, 295 insertions(+), 5 deletions(-) > > > > > > > > > > Hello Wenzhuo, > > > > > > > > > > I'm testing this patchset, but I am sporadically running into an > > > > > issue where the VFs reset fails after the PF flaps. > > > > > > > > > > I have a VM running on a KVM box with a X540-AT2, passing 2 VFs in. > > > > > > > > > > I am using calling rte_eth_dev_reset in response to a > > > > > RTE_ETH_EVENT_INTR_RESET callback, and the following
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
Hi Luca, > -Original Message- > From: Luca Boccassi [mailto:lboccass at Brocade.com] > Sent: Tuesday, July 5, 2016 5:53 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > On Tue, 2016-07-05 at 00:52 +, Lu, Wenzhuo wrote: > > Hi Luca, > > > > > > > -Original Message- > > > From: Luca Boccassi [mailto:lboccass at Brocade.com] > > > Sent: Monday, July 4, 2016 11:48 PM > > > To: Lu, Wenzhuo > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v6 0/4] support reset of VF link > > > > > > On Mon, 2016-06-20 at 14:24 +0800, Wenzhuo Lu wrote: > > > > If the PF link is down and up, VF link will not work accordingly. > > > > This patch set addes the support of VF link reset. So, when VF > > > > receices the messges of physical link down/up. APP can reset the > > > > VF link and let it recover. > > > > > > > > PS: This patch set is splitted from a previous patch set, > > > > *automatic link recovery on ixgbe/igb VF*, and it's base on the > > > > patch set *support mailbox interruption on ixgbe/igb VF*. > > > > > > > > Wenzhuo Lu (3): > > > > lib/librte_ether: support device reset > > > > ixgbe: implement device reset on VF > > > > igb: implement device reset on VF > > > > > > > > Zhe Tao (1): > > > > i40e: implement device reset on VF > > > > > > > > v1: > > > > - Added the implementation for the VF reset functionality. > > > > v2: > > > > - Changed the i40e related operations during VF reset. > > > > v3: > > > > - Resent the patches because of the mail sent issue. > > > > v4: > > > > - Removed some VF reset emulation code. > > > > v5: > > > > - Removed all the code related with lock. > > > > v6: > > > > - Updated the NIC feature overview matrix. > > > > - Added more explanation in the doxygen comment of reset API. > > > > > > > > doc/guides/nics/overview.rst | 1 + > > > > doc/guides/rel_notes/release_16_07.rst | 13 ++ > > > > drivers/net/e1000/igb_ethdev.c | 59 > > > > drivers/net/i40e/i40e_ethdev.h | 4 ++ > > > > drivers/net/i40e/i40e_ethdev_vf.c | 83 > > > ++ > > > > drivers/net/i40e/i40e_rxtx.c | 10 > > > > drivers/net/i40e/i40e_rxtx.h | 4 ++ > > > > drivers/net/ixgbe/ixgbe_ethdev.c | 64 +- > > > > drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- > > > > drivers/net/ixgbe/ixgbe_rxtx.c | 12 +++-- > > > > lib/librte_ether/rte_ethdev.c | 17 +++ > > > > lib/librte_ether/rte_ethdev.h | 24 ++ > > > > lib/librte_ether/rte_ether_version.map | 7 +++ > > > > 13 files changed, 295 insertions(+), 5 deletions(-) > > > > > > Hello Wenzhuo, > > > > > > I'm testing this patchset, but I am sporadically running into an > > > issue where the VFs reset fails after the PF flaps. > > > > > > I have a VM running on a KVM box with a X540-AT2, passing 2 VFs in. > > > > > > I am using calling rte_eth_dev_reset in response to a > > > RTE_ETH_EVENT_INTR_RESET callback, and the following errors appear > > > in the > > > log: > > > > > > PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to update link. > > > PMD: ixgbe_alloc_rx_queue_mbufs(): RX mbuf alloc failed queue_id=0 > > > PMD: ixgbevf_dev_start(): Unable to initialize RX hardware (-12) > > > PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to start device. > > > > > > Jumping in with GDB, it seems that the rte_rxmbuf_alloc call in > > > ixgbe_alloc_rx_queue_mbufs returns NULL at iteration 64 out of 2048. > > > The application has ~500 2MB hugepages, and there's 2GB of free > > > memory available on top of that. > > > > > > Have you seen this before? Any pointer or suggestion for debugging? > > > > > > Thanks! > > > > > > -- > > > Kind regards, > > > Luca Boccassi > > I think the problem is the mbuf occupied by the packets is not released. > > This > memory has to be released by the APP, so my patches haven?t covered this. > Actually an example is needed
[dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
Hi Thomas, > -Original Message- > From: Lu, Wenzhuo > Sent: Thursday, June 30, 2016 4:23 PM > To: 'Thomas Monjalon' > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe > > Hi Thomas, > > > -Original Message- > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > Sent: Thursday, June 30, 2016 3:42 PM > > To: Lu, Wenzhuo > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on > > ixgbe > > > > 2016-06-30 01:40, Lu, Wenzhuo: > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Lu, Wenzhuo > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > > 2016-06-23 01:04, Lu, Wenzhuo: > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > > > > 2016-05-06 05:33, Wenzhuo Lu: > > > > > > > > +int > > > > > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id, > > > > > > > > + enum rte_eth_rx_mq_mode rx_mq_mode, > > > > > > > > + enum rte_eth_tx_mq_mode tx_mq_mode); > > > > > > > > > > > > > > I've really tried to think about it and I think it is more or > > > > > > > less a hack. > > > > > > > First, it is not explained in the doc when we should use > > > > > > > rte_eth_dev_mq_mode_set() instead of a simple call to > > > > > rte_eth_dev_configure(). > > > > > > > Second, I don't understand why having a function which > > > > > > > configures the "multiqueue modes" without configuring > > > > > > > properly > > RSS/VMDq/DCB. > > > > > > > Last, it is said that rte_eth_dev_configure() "must be > > > > > > > invoked first before any other function in the Ethernet API". > > > After checking the code, Honestly I'm confused. I don't find this > > > description. > > > > It's in the description of rte_eth_dev_configure(): > > http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n1904 > The bad news is this rule is not followed. I think the reason is something > has to > be done before configure. > > > > > > And on the contrary, I find rte_eth_dev_info_get is always called > > > before rte_eth_dev_configure. I believe it's the problem. > > > As rte_eth_dev_configure is not run, rte_eth_dev_info_get cannot get > > > the > > right info. > > > That why I have to add a API to set the mq_mode before > > rte_eth_dev_info_get. > > > Does that mean all the related examples are wrong? Any opinion? Thanks. > > > > My opinion is that this area needs a good cleanup and easy API :) > OK. My solution is > 1, Remove the description " rte_eth_dev_configure() must be invoked first > before any other function in the Ethernet API ". I think it's reasonable to > execute > the rte_eth_dev_info_get before rte_eth_dev_configure, because we need to > get some NIC specific limitation to check if the configuration is right. > 2, Add one more argument, " const struct rte_eth_conf *eth_conf ", for > rte_eth_dev_info_get, like this, void rte_eth_dev_info_get(uint8_t port_id, > struct rte_eth_dev_info *dev_info, const struct rte_eth_conf *eth_conf); > because getting the right info depends on the configuration. And I'm a little lost, my patch has nothing to do with rte_eth_dev_configure. I just try to fix the issue that rte_eth_dev_info_get has dependency with configuration. If the configure is not right, we cannot get the right info.
[dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, June 30, 2016 3:42 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe > > 2016-06-30 01:40, Lu, Wenzhuo: > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Lu, Wenzhuo > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > 2016-06-23 01:04, Lu, Wenzhuo: > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > > > 2016-05-06 05:33, Wenzhuo Lu: > > > > > > > +int > > > > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id, > > > > > > > + enum rte_eth_rx_mq_mode rx_mq_mode, > > > > > > > + enum rte_eth_tx_mq_mode tx_mq_mode); > > > > > > > > > > > > I've really tried to think about it and I think it is more or less > > > > > > a hack. > > > > > > First, it is not explained in the doc when we should use > > > > > > rte_eth_dev_mq_mode_set() instead of a simple call to > > > > rte_eth_dev_configure(). > > > > > > Second, I don't understand why having a function which > > > > > > configures the "multiqueue modes" without configuring properly > RSS/VMDq/DCB. > > > > > > Last, it is said that rte_eth_dev_configure() "must be invoked > > > > > > first before any other function in the Ethernet API". > > After checking the code, Honestly I'm confused. I don't find this > > description. > > It's in the description of rte_eth_dev_configure(): > http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n1904 The bad news is this rule is not followed. I think the reason is something has to be done before configure. > > > And on the contrary, I find rte_eth_dev_info_get is always called > > before rte_eth_dev_configure. I believe it's the problem. > > As rte_eth_dev_configure is not run, rte_eth_dev_info_get cannot get the > right info. > > That why I have to add a API to set the mq_mode before > rte_eth_dev_info_get. > > Does that mean all the related examples are wrong? Any opinion? Thanks. > > My opinion is that this area needs a good cleanup and easy API :) OK. My solution is 1, Remove the description " rte_eth_dev_configure() must be invoked first before any other function in the Ethernet API ". I think it's reasonable to execute the rte_eth_dev_info_get before rte_eth_dev_configure, because we need to get some NIC specific limitation to check if the configuration is right. 2, Add one more argument, " const struct rte_eth_conf *eth_conf ", for rte_eth_dev_info_get, like this, void rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info, const struct rte_eth_conf *eth_conf); because getting the right info depends on the configuration.
[dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
Hi Thomas, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Lu, Wenzhuo > Sent: Friday, June 24, 2016 8:46 AM > To: Thomas Monjalon > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe > > Hi Thomas, > > > > -Original Message- > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > Sent: Thursday, June 23, 2016 8:22 PM > > To: Lu, Wenzhuo > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on > > ixgbe > > > > 2016-06-23 01:04, Lu, Wenzhuo: > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > 2016-05-06 05:33, Wenzhuo Lu: > > > > > +int > > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id, > > > > > + enum rte_eth_rx_mq_mode rx_mq_mode, > > > > > + enum rte_eth_tx_mq_mode tx_mq_mode); > > > > > > > > I've really tried to think about it and I think it is more or less a > > > > hack. > > > > First, it is not explained in the doc when we should use > > > > rte_eth_dev_mq_mode_set() instead of a simple call to > > rte_eth_dev_configure(). > > > > Second, I don't understand why having a function which configures > > > > the "multiqueue modes" without configuring properly RSS/VMDq/DCB. > > > > Last, it is said that rte_eth_dev_configure() "must be invoked > > > > first before any other function in the Ethernet API". After checking the code, Honestly I'm confused. I don't find this description. And on the contrary, I find rte_eth_dev_info_get is always called before rte_eth_dev_configure. I believe it's the problem. As rte_eth_dev_configure is not run, rte_eth_dev_info_get cannot get the right info. That why I have to add a API to set the mq_mode before rte_eth_dev_info_get. Does that mean all the related examples are wrong? Any opinion? Thanks.
[dpdk-dev] Fwd: dpdk ixgbe PMD lro limits
Hi Asim, > -Original Message- > From: Asim Jamshed [mailto:asim.jamshed at gmail.com] > Sent: Tuesday, June 28, 2016 5:23 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] Fwd: dpdk ixgbe PMD lro limits > > On Tue, Jun 28, 2016 at 5:42 PM, Lu, Wenzhuo wrote: > > Hi Asim, > > > > > >> -Original Message- > >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Asim Jamshed > >> Sent: Tuesday, June 28, 2016 2:41 PM > >> To: dev at dpdk.org > >> Subject: [dpdk-dev] Fwd: dpdk ixgbe PMD lro limits > >> > >> Hi, > >> > >> Apologies in advance if this question has been asked in the past. > >> > >> I have been performing mTCP-related tests on dpdk-16.04 with ixgbe > >> PMD. I am using 82599ES 10-Gigabit adapters for my experiments. I > >> have a few queries regarding LRO. > >> > >> 1) What is the theoretical maximum size of the Ethernet frame I can > >> get from the driver once LRO is enabled? In my experiments, I was > >> seeing packet size as high as 16KB. Can it be as high as ~2^16 bytes (iph- > >tot_len)? > > I'm not sure about if I understand your question correctly. Assume you're > talking about the TCP segment after LRO. So, it's said there are no > limitations on > the maximum packet length. > > The issue that I am facing is fixing the size of mbuf. > The default size of mbuf is (2048 + sizeof(struct rte_mbuf) + > RTE_PKTMBUF_HEADROOM) which needs to be augmented once lro is enabled > (am I right?). I tried stretching the limits of the mempool by increasing > mbuf size > to (65536 + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM), and I got the > following error: I don't think you need to change the size of mbuf. Please check this field of mbuf, "struct rte_mbuf *next;/**< Next segment of scattered packet. */" > > Initializing port 0... EAL: Error - exiting with code: 1 > Cause: rte_eth_rx_queue_setup:err=-22, port=0, queueid: 0 > > My exact calls for rte_mempool_create() and > rte_eth_rx_queue_setup() were: > > pktmbuf_pool = rte_mempool_create(name, 8192, > MBUF_SIZE, 256, > sizeof(struct rte_pktmbuf_pool_private), > rte_pktmbuf_pool_init, NULL, > rte_pktmbuf_init, NULL, > rte_socket_id(), 0); > > ret = rte_eth_rx_queue_setup(portid, rxlcore_id, 128, > rte_eth_dev_socket_id(portid), _conf, > pktmbuf_pool); > > > > >> > >> 2) Since the NIC is reassembling payloads (of one flow) into a single > >> packet, what does the Ethernet controller do with the tcp checksum > >> field in the TCP header? I am observing that each LRO packet has > >> checksum value as zero? Is that normal? I could not find any relevant > documentation on the Web. > > That's expected. > > You can search 82599 datasheet. I think that's what you're looking for. > > Thanks! > > > Regards, > --Asim > > > > >> > >> Thanks in advance, > >> --Asim
[dpdk-dev] Fwd: dpdk ixgbe PMD lro limits
Hi Asim, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Asim Jamshed > Sent: Tuesday, June 28, 2016 2:41 PM > To: dev at dpdk.org > Subject: [dpdk-dev] Fwd: dpdk ixgbe PMD lro limits > > Hi, > > Apologies in advance if this question has been asked in the past. > > I have been performing mTCP-related tests on dpdk-16.04 with ixgbe PMD. I am > using 82599ES 10-Gigabit adapters for my experiments. I have a few queries > regarding LRO. > > 1) What is the theoretical maximum size of the Ethernet frame I can get from > the driver once LRO is enabled? In my experiments, I was seeing packet size as > high as 16KB. Can it be as high as ~2^16 bytes (iph->tot_len)? I'm not sure about if I understand your question correctly. Assume you're talking about the TCP segment after LRO. So, it's said there are no limitations on the maximum packet length. > > 2) Since the NIC is reassembling payloads (of one flow) into a single packet, > what does the Ethernet controller do with the tcp checksum field in the TCP > header? I am observing that each LRO packet has checksum value as zero? Is > that normal? I could not find any relevant documentation on the Web. That's expected. You can search 82599 datasheet. I think that's what you're looking for. > > Thanks in advance, > --Asim
[dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements
Hi Thomas, Markos, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Friday, June 24, 2016 3:13 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Markos Chandras > Subject: Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' > statements > > 2016-06-24 00:43, Lu, Wenzhuo: > > Thanks for this patch. But normally the code in the base directory is synced > from the kernel driver. So we don't change it if there's no critical issue. > It's easy > for us to maintain it. Thanks. > > I think a build error is critical enough. OK. The change itself looks fine to me.
[dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, June 23, 2016 8:22 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe > > 2016-06-23 01:04, Lu, Wenzhuo: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2016-05-06 05:33, Wenzhuo Lu: > > > > +int > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id, > > > > + enum rte_eth_rx_mq_mode rx_mq_mode, > > > > + enum rte_eth_tx_mq_mode tx_mq_mode); > > > > > > I've really tried to think about it and I think it is more or less a hack. > > > First, it is not explained in the doc when we should use > > > rte_eth_dev_mq_mode_set() instead of a simple call to > rte_eth_dev_configure(). > > > Second, I don't understand why having a function which configures > > > the "multiqueue modes" without configuring properly RSS/VMDq/DCB. > > > Last, it is said that rte_eth_dev_configure() "must be invoked first > > > before any other function in the Ethernet API". > > Sorry, didn't notice this announcement. > > > > > My opinion is that the primary goal of rte_eth_dev_configure() was > > > "Embedding all configuration information in a single data structure" > > > but it is currently configuring only speed and some flow steering > > > (only RSS, VMDq, DCB and flow director). > > > This bug and the state of the ethdev API clearly shows that we must > > > have one function per feature (or group of features) and drop > rte_eth_dev_configure(). > > > > > > You can argue it is a just a personal feeling and this comment comes > > > late, but I promise it is not easy to give a negative opinion because of > > > design > perspective. > > > I strongly feel we must stop workarounding the ethdev API issues and > > > start really fixing it. > > > > > > Hope you understand and agree to work on a new API. > > I have the same feeling with you. There's some problem with > rte_eth_dev_configure. So this patch is a workaround more than a real fix. > > But the problem is this API has already been used. What I think is could we > > take > this workaround as a first step. It need not ask the APP to change too much. > > Then we can discuss how could we rework on a new API or APIs. We all > > know the change in rte layer is not easy and need to be very careful > > :) > > We probably need more opinions. > I think it is not a good idea to introduce a new API only to workaround > another > one and keep confusion in place. > A similar approach which looks better is to introduce a new API which will > partly > replace the old one and will remain a good one when the old API will be > completely removed. > In other words, we should introduce a good API for flow steering as soon as > possible and deprecate rte_eth_dev_configure(). I think you're right. The workaround can make things confusing. Better to introduce a new API to replace rte_eth_dev_configure.
[dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements
Hi Markos, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Markos Chandras > Sent: Thursday, June 23, 2016 5:26 PM > To: dev at dpdk.org > Cc: Markos Chandras > Subject: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' > statements > > Add the missing braces to the 'if' statements to fix the misleading > identation. > This also fixes the following build errors when building with gcc >= 6: > > drivers/net/e1000/base/e1000_phy.c:4156:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] if > (locked) ^~ > > drivers/net/e1000/base/e1000_phy.c:4158:3: > note: ...this statement, but the latter is misleadingly indented as if it is > guarded > by the 'if' > if (!ready) > ^~ > > drivers/net/e1000/base/e1000_phy.c: In function 'e1000_write_phy_reg_mphy': > drivers/net/e1000/base/e1000_phy.c:4221:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] if > (locked) ^~ > > drivers/net/e1000/base/e1000_phy.c:4223:3: > note: ...this statement, but the latter is misleadingly indented as if it is > guarded > by the 'if' > if (!ready) > ^~ > > Signed-off-by: Markos Chandras Thanks for this patch. But normally the code in the base directory is synced from the kernel driver. So we don't change it if there's no critical issue. It's easy for us to maintain it. Thanks.
[dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, June 23, 2016 1:02 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe > > 2016-05-06 05:33, Wenzhuo Lu: > > An issue is found that DCB cannot be configured on ixgbe NICs. It's > > said the TX queue number is not right. > > On ixgbe the max TX queue number is not fixed, it depends on the > > multi-queue mode. The API rte_eth_dev_configure should be used to > > configure this mode. But the input of this API includes TX queue > > number. The problem is before the mode is configured, we cannot decide > > the TX queue number. > > > > This patch adds an API to configure RX & TX multi-queue mode > > separately. After the mode is configured, the max RX & TX queue number > > is decided. Then we can set the appropriate RX & TX queue number. > [...] > > +/** > > + * Set RX & TX multi_queue mode. > > + * > > + * @param port_id > > + * The port identifier of the Ethernet device. > > + * @param rx_mq_mode > > + * RX multi_queue mode. > > + * @param tx_mq_mode > > + * TX multi_queue mode. > > + * > > + * @return > > + * - (0) if successful. > > + * - (-ENODEV) if port identifier is invalid. > > + */ > > +int > > +rte_eth_dev_mq_mode_set(uint8_t port_id, > > + enum rte_eth_rx_mq_mode rx_mq_mode, > > + enum rte_eth_tx_mq_mode tx_mq_mode); > > I've really tried to think about it and I think it is more or less a hack. > First, it is not explained in the doc when we should use > rte_eth_dev_mq_mode_set() instead of a simple call to rte_eth_dev_configure(). > Second, I don't understand why having a function which configures the > "multiqueue modes" without configuring properly RSS/VMDq/DCB. > Last, it is said that rte_eth_dev_configure() "must be invoked first before > any > other function in the Ethernet API". Sorry, didn't notice this announcement. > > My opinion is that the primary goal of rte_eth_dev_configure() was "Embedding > all configuration information in a single data structure" > but it is currently configuring only speed and some flow steering (only RSS, > VMDq, DCB and flow director). > This bug and the state of the ethdev API clearly shows that we must have one > function per feature (or group of features) and drop rte_eth_dev_configure(). > > You can argue it is a just a personal feeling and this comment comes late, > but I > promise it is not easy to give a negative opinion because of design > perspective. > I strongly feel we must stop workarounding the ethdev API issues and start > really > fixing it. > > Hope you understand and agree to work on a new API. I have the same feeling with you. There's some problem with rte_eth_dev_configure. So this patch is a workaround more than a real fix. But the problem is this API has already been used. What I think is could we take this workaround as a first step. It need not ask the APP to change too much. Then we can discuss how could we rework on a new API or APIs. We all know the change in rte layer is not easy and need to be very careful :)
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Jerin,
> -Original Message-
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Wednesday, June 22, 2016 7:07 PM
> To: Thomas Monjalon
> Cc: Lu, Wenzhuo; dev at dpdk.org; Ananyev, Konstantin; Stephen Hemminger;
> Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin
> Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
>
> On Wed, Jun 22, 2016 at 11:18:21AM +0200, Thomas Monjalon wrote:
> > 2016-06-22 08:25, Lu, Wenzhuo:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2016-06-22 13:29, Jerin Jacob:
> > > > > Thomas,
> > > > > As a librte_ether maintainer any comments on this?
> > > >
> > > > +1 for adding details and make sure naming is good.
> > > > I don't really need to comment here because I have already done
> > > > this comment
> > > > earlier:
> > > > http://dpdk.org/ml/archives/dev/2016-June/041845.html
> > > > Thank you for insisting.
> > > I've add some details in this patch set. If it's not enough, please let
> > > me know.
> > > And I think this discussion is about what the API name should be like.
> > > Actually
> I think all the existing name is describing what is done by the API not when
> and
> where it should be used, like dev_start/stop.
> >
> > You're right, I overlooked it:
> >
> > + * The API will stop the port, clear the rx/tx queues, re-setup the
> > + rx/tx
> > + * queues, restart the port.
> >
> > Jerin, which detail do you think is needed?
>
> When to use what ? In what scenarios application need to use generic
> stop/start
> vs this new API?
I'll add more explanation. Actually I've written an example. But after
discussion we agree it's not a good idea to add a totally new example just for
one function. I'm thinking about now to fuse this example into testpmd.
>
> How about calling it as rte_eth_dev_restart() ?
Sounds good :)
>
> If existing stop and then start is same the new API in functional
> perspective, How
> about having generic implementation of rte_eth_dev_restart() if PMD specific
> restart handlers are NOT found.
Good suggestion, thanks.
>
> That why application need to call only rte_eth_dev_restart() for port
> restart. It
> can internally decide optimized stop/start or generic restart
>
> Jerin
>
> >
> > Wenzhuo, why this function is needed?
> > All these actions are already possible independently.
> > When looking at ixgbe implementation, I see:
> > ixgbevf_dev_stats_reset() which is not documented in the API
> > rte_delay_ms(1000);
> > do {} while
> > It looks to be some hacks.
> > If you really need some workarounds to handle some tricky situations,
> > maybe that the API is not detailed enough.
> >
> > > But anyway I'm open for changing the name. Is the name process_reset_intr
> you prefer? Thanks.
> >
> > Not sure.
> > If you really intend to add a generic reset, maybe rte_eth_dev_reset()
> > is a good name. We just need more justification.
> > After reading the doc, the user can understand it is just a wrapper of
> > existing functions. But it appears in the code that it does more and
> > can help in some situations.
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Thomas,
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, June 22, 2016 5:18 PM
> To: Lu, Wenzhuo; Jerin Jacob
> Cc: dev at dpdk.org; Ananyev, Konstantin; Stephen Hemminger; Richardson,
> Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin
> Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
>
> 2016-06-22 08:25, Lu, Wenzhuo:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-06-22 13:29, Jerin Jacob:
> > > > Thomas,
> > > > As a librte_ether maintainer any comments on this?
> > >
> > > +1 for adding details and make sure naming is good.
> > > I don't really need to comment here because I have already done this
> > > comment
> > > earlier:
> > > http://dpdk.org/ml/archives/dev/2016-June/041845.html
> > > Thank you for insisting.
> > I've add some details in this patch set. If it's not enough, please let me
> > know.
> > And I think this discussion is about what the API name should be like.
> > Actually I
> think all the existing name is describing what is done by the API not when and
> where it should be used, like dev_start/stop.
>
> You're right, I overlooked it:
>
> + * The API will stop the port, clear the rx/tx queues, re-setup the
> + rx/tx
> + * queues, restart the port.
>
> Jerin, which detail do you think is needed?
>
> Wenzhuo, why this function is needed?
As you said below and discussed before, it's a wrapper of the existing
functions. The benefit is helping the users avoid the complex implementation
when they want to stop and re-start the device.
> All these actions are already possible independently.
> When looking at ixgbe implementation, I see:
> ixgbevf_dev_stats_reset() which is not documented in the API
> rte_delay_ms(1000);
> do {} while
> It looks to be some hacks.
> If you really need some workarounds to handle some tricky situations, maybe
> that the API is not detailed enough.
Yes, you're right. Still something left. I'll add more detail.
>
> > But anyway I'm open for changing the name. Is the name process_reset_intr
> you prefer? Thanks.
>
> Not sure.
> If you really intend to add a generic reset, maybe rte_eth_dev_reset() is a
> good
> name. We just need more justification.
> After reading the doc, the user can understand it is just a wrapper of
> existing
> functions. But it appears in the code that it does more and can help in some
> situations.
I'll add more info. Thanks.
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, June 22, 2016 4:17 PM > To: Jerin Jacob > Cc: Lu, Wenzhuo; Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org; > Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset > > 2016-06-22 13:29, Jerin Jacob: > > Thomas, > > As a librte_ether maintainer any comments on this? > > +1 for adding details and make sure naming is good. > I don't really need to comment here because I have already done this comment > earlier: > http://dpdk.org/ml/archives/dev/2016-June/041845.html > Thank you for insisting. I've add some details in this patch set. If it's not enough, please let me know. And I think this discussion is about what the API name should be like. Actually I think all the existing name is describing what is done by the API not when and where it should be used, like dev_start/stop. But anyway I'm open for changing the name. Is the name process_reset_intr you prefer? Thanks.
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Jerin, > -Original Message- > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > Sent: Wednesday, June 22, 2016 2:10 PM > To: Lu, Wenzhuo > Cc: Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org; Richardson, > Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > thomas.monjalon at 6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset > > On Wed, Jun 22, 2016 at 05:05:14AM +, Lu, Wenzhuo wrote: > > > > > > > -Original Message- > > > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > > > Sent: Wednesday, June 22, 2016 12:15 PM > > > To: Lu, Wenzhuo > > > Cc: Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org; > > > Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; > > > Zhang, Helin; thomas.monjalon at 6wind.com > > > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support > > > device reset > > > > > > On Wed, Jun 22, 2016 at 03:32:16AM +, Lu, Wenzhuo wrote: > > > > Lost here. I think these RTE_ETH_EVENTs are used to connect the > > > > APP call > > > back functions with the events. > > > > Actually I want the APP to register a callback function > > > > reset_event_callback for > > > the reset event. Like this, > > > > /* register reset interrupt callback */ > > > > rte_eth_dev_callback_register(portid, > > > > RTE_ETH_EVENT_INTR_RESET, reset_event_callback, > > > NULL); And when the > > > > VF driver finds PF link down/up, it should use > > > _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET) to run > > > into the callback which is provided by APP. Means reset_event_callback > > > here. > > > > > > me too. Their is existing RTE_ETH_EVENT_INTR_RESET event to notify > > > the PF reset.I guess it is not for the PF link change or it isfor > > > generic VF reset request initiated by PF for everything. > > I think this event is for device reset not only for PF but also can for VF. > > I think > we can use this event when the driver want the APP to reset the device. The PF > link down/up caused VF reset is one of the cases. > > Then please correct description for the RTE_ETH_EVENT_INTR_RESET in > lib/librte_ether/rte_ethdev.h "/**< reset interrupt event, sent to VF on PF > reset > */" > > > > > > > > > file: lib/librte_ether/rte_ethdev.h > > > RTE_ETH_EVENT_INTR_RESET, > > > /**< reset interrupt event, sent to VF on PF reset */ > > > > > > ^^^ > > > > > > if application need to call rte_ethdev_reset() on > > > RTE_ETH_EVENT_INTR_RESET event then please mention it commit log or > API description. > > Good suggestion. I'll try to find where's the good place to add more > explanation. > > I guess then reset API can be changed to rte_ethdev_process_reset_intr() or > similar to reflect the use case(API called by application on reset event from > PF) > > The PMDs were PF does not generate the RTE_ETH_EVENT_INTR_RESET to VF > then VF's reset PMD callback shall be a 'nop' > > Jerin But I don't think it's appropriate to bind the RTE_ETH_EVENTs with the APIs. This patch set provide a reset API to reset the device. Don't mean this reset API only can be used when the APP hit the event RTE_ETH_EVENT_INTR_RESET. I can add some comments to suggest the user to call the reset API at that time. But I think APP can call the reset API anytime when it thinks it's necessary. So I don't like the name *process_reset_intr*, it hints that this API is only for the INTR_RESET event. > > > > > >
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
> -Original Message- > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > Sent: Wednesday, June 22, 2016 12:15 PM > To: Lu, Wenzhuo > Cc: Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org; Richardson, > Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > thomas.monjalon at 6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset > > On Wed, Jun 22, 2016 at 03:32:16AM +, Lu, Wenzhuo wrote: > > Hi Jerin, > > > > > -Original Message- > > > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > > > Sent: Wednesday, June 22, 2016 10:38 AM > > > To: Lu, Wenzhuo > > > Cc: Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org; > > > Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; > > > Zhang, Helin; thomas.monjalon at 6wind.com > > > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support > > > device reset > > > > > > On Wed, Jun 22, 2016 at 01:35:37AM +, Lu, Wenzhuo wrote: > > > > Hi Jerin, > > > > > > > > > -Original Message- > > > > > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > > > > > Sent: Tuesday, June 21, 2016 10:29 PM > > > > > To: Ananyev, Konstantin > > > > > Cc: Lu, Wenzhuo; Stephen Hemminger; dev at dpdk.org; Richardson, > > > > > Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > > > > > thomas.monjalon at 6wind.com > > > > > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support > > > > > device reset > > > > > > > > > > On Tue, Jun 21, 2016 at 02:03:15PM +, Ananyev, Konstantin wrote: > > > > > > > > > > > > > > > > > > > > > > > Hi Wenzhuo, > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Jun 20, 2016 at 02:24:27PM > > > > > > > > > > > > > > > > > +0800, Wenzhuo Lu > > > > > wrote: > > > > > > > > > > > > > > > > > > Add an API to reset the device. > > > > > > > > > > > > > > > > > > It's for VF device in this scenario, kernel > > > > > > > > > > > > > > > > > > PF + DPDK > VF. > > > > > > > > > > > > > > > > > > When the PF port down->up, APP should > > > > > > > > > > > > > > > > > > call this API to reset VF port. Most > > > > > > > > > > > > > > > > > > likely, APP should call it in its > > > > > > > > > > > > > > > > > > management thread and guarantee the > > > > > > > > > > > > > > > > > > thread safe. It means APP should stop > > > > > > > > > > > > > > > > > > the rx/tx and the device, then reset > > > > > > > > > > > > > > > > > > the device, then > > > > > recover the device and rx/tx. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Following is _a_ use-case for Device reset. > > > > > > > > > > > > > > > > > But may be not be _the_ use case. IMO, > > > > > > > > > > > > > > > > > We need to first say expected behavior > > > > > > > > > > > > > > > > > of this API and add a use-case > > > > > later. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Other use-case would be, PCIe VF with > > > > > > > > > > > > > > > > > functional level reset for SRIOV migration. > > > > > > > > > > > > > > > > > Are we on same page? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In my exp
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Jerin, > -Original Message- > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > Sent: Wednesday, June 22, 2016 10:38 AM > To: Lu, Wenzhuo > Cc: Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org; Richardson, > Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > thomas.monjalon at 6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset > > On Wed, Jun 22, 2016 at 01:35:37AM +, Lu, Wenzhuo wrote: > > Hi Jerin, > > > > > -Original Message- > > > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > > > Sent: Tuesday, June 21, 2016 10:29 PM > > > To: Ananyev, Konstantin > > > Cc: Lu, Wenzhuo; Stephen Hemminger; dev at dpdk.org; Richardson, Bruce; > > > Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > > > thomas.monjalon at 6wind.com > > > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support > > > device reset > > > > > > On Tue, Jun 21, 2016 at 02:03:15PM +, Ananyev, Konstantin wrote: > > > > > > > > > > > > > > > > > Hi Wenzhuo, > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Jun 20, 2016 at 02:24:27PM +0800, > > > > > > > > > > > > > > > Wenzhuo Lu > > > wrote: > > > > > > > > > > > > > > > > Add an API to reset the device. > > > > > > > > > > > > > > > > It's for VF device in this scenario, kernel PF > > > > > > > > > > > > > > > > + DPDK VF. > > > > > > > > > > > > > > > > When the PF port down->up, APP should call > > > > > > > > > > > > > > > > this API to reset VF port. Most likely, > > > > > > > > > > > > > > > > APP should call it in its management > > > > > > > > > > > > > > > > thread and guarantee the thread safe. It > > > > > > > > > > > > > > > > means APP should stop the rx/tx and the > > > > > > > > > > > > > > > > device, then reset the device, then > > > recover the device and rx/tx. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Following is _a_ use-case for Device reset. > > > > > > > > > > > > > > > But may be not be _the_ use case. IMO, We > > > > > > > > > > > > > > > need to first say expected behavior of this > > > > > > > > > > > > > > > API and add a use-case > > > later. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Other use-case would be, PCIe VF with > > > > > > > > > > > > > > > functional level reset for SRIOV migration. > > > > > > > > > > > > > > > Are we on same page? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In my experience with Linux devices, this is > > > > > > > > > > > > > > normally handled by the device driver in the > > > > > > > > > > > > > > start routine. Since any use case which needs > > > > > > > > > > > > > > this is going to do a stop/reset/start > > > > > > > > > > > > > > sequence, why not just have > > > the VF device driver do this in the start routine?. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Adding yet another API and state transistion > > > > > > > > > > > > > > if not necessary increases the complexity and > > > > > > > > > > > > > > required test > > > cases for all devices. > > > > > > > > > > > > > > > > > > > > > > > > > > I agree with Stephen here.I think if application > > > > > > > > > &
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Jerin, > -Original Message- > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > Sent: Tuesday, June 21, 2016 10:29 PM > To: Ananyev, Konstantin > Cc: Lu, Wenzhuo; Stephen Hemminger; dev at dpdk.org; Richardson, Bruce; Chen, > Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > thomas.monjalon at 6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset > > On Tue, Jun 21, 2016 at 02:03:15PM +, Ananyev, Konstantin wrote: > > > > > > > > > > > Hi Wenzhuo, > > > > > > > > > > > > > > > > > > > > On Mon, Jun 20, 2016 at 02:24:27PM +0800, Wenzhuo Lu > wrote: > > > > > > > > > > > > > > Add an API to reset the device. > > > > > > > > > > > > > > It's for VF device in this scenario, kernel PF + > > > > > > > > > > > > > > DPDK VF. > > > > > > > > > > > > > > When the PF port down->up, APP should call > > > > > > > > > > > > > > this API to reset VF port. Most likely, APP > > > > > > > > > > > > > > should call it in its management thread and > > > > > > > > > > > > > > guarantee the thread safe. It means APP should > > > > > > > > > > > > > > stop the rx/tx and the device, then reset the > > > > > > > > > > > > > > device, then > recover the device and rx/tx. > > > > > > > > > > > > > > > > > > > > > > > > > > Following is _a_ use-case for Device reset. But > > > > > > > > > > > > > may be not be _the_ use case. IMO, We need to > > > > > > > > > > > > > first say expected behavior of this API and add a > > > > > > > > > > > > > use-case > later. > > > > > > > > > > > > > > > > > > > > > > > > > > Other use-case would be, PCIe VF with functional > > > > > > > > > > > > > level reset for SRIOV migration. > > > > > > > > > > > > > Are we on same page? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In my experience with Linux devices, this is > > > > > > > > > > > > normally handled by the device driver in the start > > > > > > > > > > > > routine. Since any use case which needs this is > > > > > > > > > > > > going to do a stop/reset/start sequence, why not just > > > > > > > > > > > > have > the VF device driver do this in the start routine?. > > > > > > > > > > > > > > > > > > > > > > > > Adding yet another API and state transistion if > > > > > > > > > > > > not necessary increases the complexity and required test > cases for all devices. > > > > > > > > > > > > > > > > > > > > > > I agree with Stephen here.I think if application > > > > > > > > > > > needs to call start after the device reset then we > > > > > > > > > > > could add this logic in start itself rather exposing > > > > > > > > > > > a yet another API > > > > > > > > > > Do you mean changing the device_start to include all > > > > > > > > > > these actions, stop > > > > > > > > > device -> stop queue -> re-setup queue -> start queue -> start > device ? > > > > > > > > > > > > > > > > > > What was the expected API call sequence when you were > introduced this API? > > > > > > > > > > > > > > > > > > Point was to have implicit device reset in the API call > > > > > > > > > sequence(Wherever make sense for specific PMD) > > > > > > > > I think the API call sequence depends on the > > > > > > > > implementation of the APP. Let's say if there's not this > > > > > > > > r
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Jerin, > -Original Message- > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > Sent: Tuesday, June 21, 2016 3:37 PM > To: Lu, Wenzhuo > Cc: Stephen Hemminger; dev at dpdk.org; Ananyev, Konstantin; Richardson, > Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > thomas.monjalon at 6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset > > On Tue, Jun 21, 2016 at 06:14:29AM +, Lu, Wenzhuo wrote: > > Hi Jerin, Stephen, > > > > > > > -Original Message- > > > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > > > Sent: Tuesday, June 21, 2016 11:51 AM > > > To: Stephen Hemminger > > > Cc: Lu, Wenzhuo; dev at dpdk.org; Ananyev, Konstantin; Richardson, > > > Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > > > thomas.monjalon at 6wind.com > > > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support > > > device reset > > > > > > On Mon, Jun 20, 2016 at 09:17:14AM -0700, Stephen Hemminger wrote: > > > > On Mon, 20 Jun 2016 14:44:11 +0530 Jerin Jacob > > > > wrote: > > > > > > > > > On Mon, Jun 20, 2016 at 02:24:27PM +0800, Wenzhuo Lu wrote: > > > > > > Add an API to reset the device. > > > > > > It's for VF device in this scenario, kernel PF + DPDK VF. > > > > > > When the PF port down->up, APP should call this API to reset > > > > > > VF port. Most likely, APP should call it in its management > > > > > > thread and guarantee the thread safe. It means APP should stop > > > > > > the rx/tx and the device, then reset the device, then recover > > > > > > the device and rx/tx. > > > > > > > > > > Following is _a_ use-case for Device reset. But may be not be > > > > > _the_ use case. IMO, We need to first say expected behavior of > > > > > this API and add a use-case later. > > > > > > > > > > Other use-case would be, PCIe VF with functional level reset for > > > > > SRIOV migration. > > > > > Are we on same page? > > > > > > > > > > > > In my experience with Linux devices, this is normally handled by > > > > the device driver in the start routine. Since any use case which > > > > needs this is going to do a stop/reset/start sequence, why not > > > > just have the VF device driver do this in the start routine?. > > > > > > > > Adding yet another API and state transistion if not necessary > > > > increases the complexity and required test cases for all devices. > > > > > > I agree with Stephen here.I think if application needs to call start > > > after the device reset then we could add this logic in start itself > > > rather exposing a yet another API > > Do you mean changing the device_start to include all these actions, stop > device -> stop queue -> re-setup queue -> start queue -> start device ? > > What was the expected API call sequence when you were introduced this API? > > Point was to have implicit device reset in the API call sequence(Wherever make > sense for specific PMD) I think the API call sequence depends on the implementation of the APP. Let's say if there's not this reset API, APP can use this API call sequence to handle the PF link down/up event, rte_eth_dev_close -> rte_eth_rx_queue_setup -> rte_eth_tx_queue_setup -> rte_eth_dev_start. Actually our purpose is to use this reset API instead of the API call sequence. You can see the reset API is not necessary. The benefit is to save the code for APP. > > Jerin > > > > > > > > > >
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Jerin, Stephen, > -Original Message- > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > Sent: Tuesday, June 21, 2016 11:51 AM > To: Stephen Hemminger > Cc: Lu, Wenzhuo; dev at dpdk.org; Ananyev, Konstantin; Richardson, Bruce; > Chen, > Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin; > thomas.monjalon at 6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset > > On Mon, Jun 20, 2016 at 09:17:14AM -0700, Stephen Hemminger wrote: > > On Mon, 20 Jun 2016 14:44:11 +0530 > > Jerin Jacob wrote: > > > > > On Mon, Jun 20, 2016 at 02:24:27PM +0800, Wenzhuo Lu wrote: > > > > Add an API to reset the device. > > > > It's for VF device in this scenario, kernel PF + DPDK VF. > > > > When the PF port down->up, APP should call this API to reset VF > > > > port. Most likely, APP should call it in its management thread and > > > > guarantee the thread safe. It means APP should stop the rx/tx and > > > > the device, then reset the device, then recover the device and > > > > rx/tx. > > > > > > Following is _a_ use-case for Device reset. But may be not be _the_ > > > use case. IMO, We need to first say expected behavior of this API > > > and add a use-case later. > > > > > > Other use-case would be, PCIe VF with functional level reset for > > > SRIOV migration. > > > Are we on same page? > > > > > > In my experience with Linux devices, this is normally handled by the > > device driver in the start routine. Since any use case which needs > > this is going to do a stop/reset/start sequence, why not just have the > > VF device driver do this in the start routine?. > > > > Adding yet another API and state transistion if not necessary > > increases the complexity and required test cases for all devices. > > I agree with Stephen here.I think if application needs to call start after the > device reset then we could add this logic in start itself rather exposing a > yet > another API Do you mean changing the device_start to include all these actions, stop device -> stop queue -> re-setup queue -> start queue -> start device ? > > >
[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
Hi Jerin,
> -Original Message-
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Monday, June 20, 2016 5:14 PM
> To: Lu, Wenzhuo
> Cc: dev at dpdk.org; Ananyev, Konstantin; Richardson, Bruce; Chen, Jing D;
> Liang,
> Cunming; Wu, Jingjing; Zhang, Helin; thomas.monjalon at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
>
> On Mon, Jun 20, 2016 at 02:24:27PM +0800, Wenzhuo Lu wrote:
> > Add an API to reset the device.
> > It's for VF device in this scenario, kernel PF + DPDK VF.
> > When the PF port down->up, APP should call this API to reset VF port.
> > Most likely, APP should call it in its management thread and guarantee
> > the thread safe. It means APP should stop the rx/tx and the device,
> > then reset the device, then recover the device and rx/tx.
>
> Following is _a_ use-case for Device reset. But may be not be _the_ use case.
> IMO, We need to first say expected behavior of this API and add a use-case
> later.
Thanks for the suggestion, I'll reword it.
>
> Other use-case would be, PCIe VF with functional level reset for SRIOV
> migration.
> Are we on same page?
I'm not sure:) Does this SRIOV migration mean the migration of a Logical domain
that has a VF assigned to it?
>
> >
> > Signed-off-by: Wenzhuo Lu
> > ---
> > doc/guides/nics/overview.rst | 1 +
> > lib/librte_ether/rte_ethdev.c | 17 +
> > lib/librte_ether/rte_ethdev.h | 24
> > lib/librte_ether/rte_ether_version.map | 7 +++
> > 4 files changed, 49 insertions(+)
> >
> > diff --git a/doc/guides/nics/overview.rst
> > b/doc/guides/nics/overview.rst index 0bd8fae..c8a4985 100644
> > --- a/doc/guides/nics/overview.rst
> > +++ b/doc/guides/nics/overview.rst
> > @@ -89,6 +89,7 @@ Most of these differences are summarized below.
> > Speed capabilities
> > Link statusY Y Y Y Y Y Y Y Y Y Y Y Y Y
> > Y Y Y Y Y Y
> > Link status event Y Y Y Y Y Y Y Y Y Y
> > Y Y Y
> > + Link reset Y Y Y Y Y
>
> More appropriate would be "Device reset" ? Right?
Yes, sounds better :)
>
> > Queue status event
> > Y
> > Rx interrupt Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y
> > Queue start/stop Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y
> >Y Y Y
> > diff --git a/lib/librte_ether/rte_ethdev.c
> > b/lib/librte_ether/rte_ethdev.c index e148028..6c0449b 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -3346,3 +3346,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t
> port_id,
> > -ENOTSUP);
> > return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask,
> > en); }
> > +
> > +int
> > +rte_eth_dev_reset(uint8_t port_id)
> > +{
> > + struct rte_eth_dev *dev;
> > + int diag;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +
> > + dev = _eth_devices[port_id];
> > +
> > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
> > +
> > + diag = (*dev->dev_ops->dev_reset)(dev);
> > +
> > + return diag;
> > +}
> > diff --git a/lib/librte_ether/rte_ethdev.h
> > b/lib/librte_ether/rte_ethdev.h index 2757510..5b3ba12 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -1318,6 +1318,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
> > uint8_t en);
> > /**< @internal enable/disable the l2 tunnel offload functions */
> >
> > +typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev); /**<
> > + at internal Function used to reset a configured Ethernet device. */
> > +
> > #ifdef RTE_NIC_BYPASS
> >
> > enum {
> > @@ -1508,6 +1511,8 @@ struct eth_dev_ops {
> > eth_l2_tunnel_eth_type_conf_t l2_tunnel_eth_type_conf;
> > /** Enable/disable l2 tunnel offload functions */
> > eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
> > + /** Reset device. */
> > + eth_dev_reset_t dev_reset;
> > };
> >
> > /**
> > @@ -4253,6 +4258,25 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t
> port_id,
> > uint32_t mask,
> > uint8_t en);
> >
> > +/**
> > + * Re
[dpdk-dev] [PATCH v3] e1000: configure VLAN TPID
Hi, > -Original Message- > From: Xing, Beilei > Sent: Thursday, June 16, 2016 11:16 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Xing, Beilei > Subject: [PATCH v3] e1000: configure VLAN TPID > > This patch enables configuring the outer TPID for double VLAN. > Note that outer TPID of single VLAN and inner TPID of double VLAN are read > only. > > Signed-off-by: Beilei Xing Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH] ixgbe: fix missed packet types.
Hi,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Konstantin Ananyev
> Sent: Wednesday, June 15, 2016 8:59 PM
> To: dev at dpdk.org
> Cc: Ananyev, Konstantin
> Subject: [dpdk-dev] [PATCH] ixgbe: fix missed packet types.
>
> ixgbe PMD RX function(s) miss pacjet types that are:
> - correctly recognised by the underlying HW.
> - marked as supported by ixgbe_dev_supported_ptypes_get().
>
> Fixes: 9586ebd358d5 ("ixgbe: replace some offload flags with packet type")
>
> Signed-off-by: Konstantin Ananyev
Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH v3] ixgbe: configure VLAN TPID
Hi, > -Original Message- > From: Xing, Beilei > Sent: Tuesday, June 14, 2016 3:21 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Xing, Beilei > Subject: [PATCH v3] ixgbe: configure VLAN TPID > > The patch enables configuring the ether types of both inner and outer > VLANs. > > Signed-off-by: Beilei Xing Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
Hi Konstantin,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Monday, June 13, 2016 6:48 PM
> To: Lu, Wenzhuo; Tao, Zhe; dev at dpdk.org
> Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang,
> Helin
> Subject: RE: [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
>
> Hi Wenzhuo,
>
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > 3. I thought the plan was to introduce a locking in all
> > > > > > > appropriate control path functions (dev_start/dev_stop etc.)
> > > > > > > Without that locking version of RX/TX seems a bit useless.
> > > > > > > Yes, I understand that you do use locking inside dev_reset,
> > > > > > > but I suppose the plan was to have a generic solution, no?
> > > > > > > Again, interrupt fire when user invokes dev_start/stop or
> > > > > > > so, so we still need some synchronisation between them.
> > > > > > >
> > > > > > > To be more specific, I thought about something like that:
> > > > > > >
> > > > > > > static inline uint16_t
> > > > > > > rte_eth_rx_burst_lock(uint8_t port_id, uint16_t queue_id,
> > > > > > > struct rte_mbuf **rx_pkts, const uint16_t
> > > > > > > nb_pkts) {
> > > > > > > struct rte_eth_dev *dev = _eth_devices[port_id];
> > > > > > >
> > > > > > > #ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > > > > > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> > > > > > > RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_pkt_burst, 0);
> > > > > > >
> > > > > > > if (queue_id >= dev->data->nb_rx_queues) {
> > > > > > > RTE_PMD_DEBUG_TRACE("Invalid RX
> > > > > > > queue_id=%d\n",
> > > queue_id);
> > > > > > > return 0;
> > > > > > > }
> > > > > > > #endif
> > > > > > >
> > > > > > > + if
> > > > > > > + (rte_spinlock_trylock(>data-
> >rx_queue_state[rx_queue_id].
> > > > > > > + lock)
> > > > > == 0)
> > > > > > > + return 0;
> > > > > > > + else if (dev->data->rx_queue_state[rx_queue_id] ==
> > > > > > > RTE_ETH_QUEUE_STATE_STOPPED)) {
> > > > > > > + rte_spinlock_unlock(>data-
> > > >rx_queue_state[rx_queue_id].unlock);
> > > > > > > + return 0;
> > > > > > > +
> > > > > > >
> > > > > > > nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
> > > > > > > rx_pkts, nb_pkts);
> > > > > > >
> > > > > > > + rte_spinlock_unlock(>data-
> > > >rx_queue_state[rx_queue_id].un
> > > > > > > + lock
> > > > > > > + );
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > return nb_rx;
> > > > > > > }
> > > > > > >
> > > > > > > And inside queue_start:
> > > > > > >
> > > > > > > int
> > > > > > > rte_eth_dev_rx_queue_start(uint8_t port_id, uint16_t
> > > > > > > rx_queue_id)
> > > {
> > > > > > > struct rte_eth_dev *dev;
> > > > > > >
> > > > > > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
> > > > > > >
> > > > > > > dev = _eth_devices[port_id];
> > > > > > > if (rx_queue_id >= dev->data->nb_rx_queues) {
> > > > > > > RTE_PMD_DEBUG_TRACE("Invalid RX
> > > > > > > queue_id=%d\n",
> > > > > rx_queue_id);
> > > > > > > return -EINVAL;
> > > > > > > }
> > > > > > >
> > > > > > >
> > > > > > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_start,
> > > > > > > -ENOTSUP);
> > > > > > >
> > > &g
[dpdk-dev] [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
Hi Konstantin,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Monday, June 13, 2016 7:17 AM
> To: Lu, Wenzhuo; Tao, Zhe; dev at dpdk.org
> Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang,
> Helin
> Subject: RE: [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
>
> Hi Wenzhuo,
>
> >
> > Hi Konstantin,
> >
> >
> > > -Original Message-
> > > From: Ananyev, Konstantin
> > > Sent: Wednesday, June 8, 2016 5:20 PM
> > > To: Lu, Wenzhuo; Tao, Zhe; dev at dpdk.org
> > > Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing;
> > > Zhang, Helin
> > > Subject: RE: [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
> > >
> > >
> > >
> > > >
> > > > Hi Konstantin,
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Ananyev, Konstantin
> > > > > Sent: Tuesday, June 7, 2016 5:59 PM
> > > > > To: Tao, Zhe; dev at dpdk.org
> > > > > Cc: Lu, Wenzhuo; Richardson, Bruce; Chen, Jing D; Liang,
> > > > > Cunming; Wu, Jingjing; Zhang, Helin
> > > > > Subject: RE: [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock
> > > > > mode
> > > > >
> > > > >
> > > > > Hi Zhe & Wenzhuo,
> > > > >
> > > > > Please find my comments below.
> > > > > BTW, for clarification - is that patch for 16.11?
> > > > > I believe it's too late to introduce such significant change in 16.07.
> > > > > Thanks
> > > > > Konstantin
> > > > Thanks for the comments.
> > > > Honestly, our purpose is 16.07. Realizing the big impact, we use
> > > > NEXT_ABI to comment our change. So, I think although we want to
> > > > merge it in
> > > 16.07 this change will become effective after we remove NEXT_ABI in
> 16.11.
> > >
> > > I don't think it is achievable.
> > > First I think your code is not in proper shape yet, right now.
> > > Second, as you said, it is a significant change and I would like to
> > > hear opinions from the rest of the community.
> > Agree it should have risk. I mean our target is 16.07. But surely if it can
> > be
> achieved depends on the feedback from the community.
> >
> > >
> > > >
> > > > >
> > > > > > Define lock mode for RX/TX queue. Because when resetting the
> > > > > > device we want the resetting thread to get the lock of the
> > > > > > RX/TX queue to make sure the RX/TX is stopped.
> > > > > >
> > > > > > Using next ABI macro for this ABI change as it has too much
> > > > > > impact. 7 APIs and 1 global variable are impacted.
> > > > > >
> > > > > > Signed-off-by: Wenzhuo Lu
> > > > > > Signed-off-by: Zhe Tao
> > > > > > ---
> > > > > > lib/librte_ether/rte_ethdev.h | 62
> > > > > > +++
> > > > > > 1 file changed, 62 insertions(+)
> > > > > >
> > > > > > diff --git a/lib/librte_ether/rte_ethdev.h
> > > > > > b/lib/librte_ether/rte_ethdev.h index 74e895f..4efb5e9 100644
> > > > > > --- a/lib/librte_ether/rte_ethdev.h
> > > > > > +++ b/lib/librte_ether/rte_ethdev.h
> > > > > > @@ -354,7 +354,12 @@ struct rte_eth_rxmode {
> > > > > > jumbo_frame : 1, /**< Jumbo Frame Receipt
> enable. */
> > > > > > hw_strip_crc : 1, /**< Enable CRC stripping by
> hardware. */
> > > > > > enable_scatter : 1, /**< Enable scatter packets rx
> handler */
> > > > > > +#ifndef RTE_NEXT_ABI
> > > > > > enable_lro : 1; /**< Enable LRO */
> > > > > > +#else
> > > > > > + enable_lro : 1, /**< Enable LRO */
> > > > > > + lock_mode: 1; /**< Using lock path */
> > > > > > +#endif
> > > > > > };
> > > > > >
> > > > > > /**
> > > > > > @@ -634,11 +639,68 @@ struct rte_eth_txmode {
> > > > > > /**< If set, reject sending out tagged pkts */
> > > > > > hw_vlan
[dpdk-dev] [PATCH 2/8] lib/librte_ether: defind RX/TX lock mode
Hi Stephen, > -Original Message- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Saturday, June 11, 2016 2:12 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Tao, Zhe > Subject: Re: [dpdk-dev] [PATCH 2/8] lib/librte_ether: defind RX/TX lock mode > > On Wed, 8 Jun 2016 07:34:43 + > "Lu, Wenzhuo" wrote: > > > > > > > The fact that it requires lots more locking inside each device > > > driver implies to me this is not correct way to architect this. > > It's a good question. This patch set doesn't follow the regular assumption > > of > DPDK. > > But it's a requirement we've got from some customers. The users want the > driver does as much as it can. The best is the link state change is > transparent to > the users. > > The patch set tries to provide another choice if the users don't want to > > stop > their rx/tx to handle the reset event. > > Then bring those uses to the development world (on users mailing list) and > lets > start the discussion there. The requirements creeping in through the backdoor > also worries me. Got it. Then how about we only provide a reset API and let the APP to stop/start the rx/tx and call the API to reset the port? Thanks.
[dpdk-dev] [PATCH 2/8] lib/librte_ether: defind RX/TX lock mode
Hi Olivier, > -Original Message- > From: Olivier Matz [mailto:olivier.matz at 6wind.com] > Sent: Thursday, June 9, 2016 3:51 PM > To: Lu, Wenzhuo; Stephen Hemminger > Cc: dev at dpdk.org; Tao, Zhe > Subject: Re: [dpdk-dev] [PATCH 2/8] lib/librte_ether: defind RX/TX lock mode > > Hi, > > On 06/08/2016 09:34 AM, Lu, Wenzhuo wrote: > > Hi Stephen, > > > > > >> -Original Message- > >> From: Stephen Hemminger [mailto:stephen at networkplumber.org] > >> Sent: Wednesday, June 8, 2016 10:16 AM > >> To: Lu, Wenzhuo > >> Cc: dev at dpdk.org; Tao, Zhe > >> Subject: Re: [dpdk-dev] [PATCH 2/8] lib/librte_ether: defind RX/TX > >> lock mode > >> > >> On Mon, 6 Jun 2016 13:40:47 +0800 > >> Wenzhuo Lu wrote: > >> > >>> Define lock mode for RX/TX queue. Because when resetting the device > >>> we want the resetting thread to get the lock of the RX/TX queue to > >>> make sure the RX/TX is stopped. > >>> > >>> Using next ABI macro for this ABI change as it has too much impact. > >>> 7 APIs and 1 global variable are impacted. > >>> > >>> Signed-off-by: Wenzhuo Lu > >>> Signed-off-by: Zhe Tao > >> > >> Why does this patch set make a different assumption the rest of the DPDK? > >> > >> The rest of the DPDK operates on the principle that the application > >> is smart enough to stop the device before making changes. There is no > >> equivalent to the Linux kernel RTNL mutex. The API assumes > >> application threads are well behaved and will not try and sabotage each > other. > >> > >> If you restrict the reset operation to only being available when > >> RX/TX is stopped, then no lock is needed. > >> > >> The fact that it requires lots more locking inside each device driver > >> implies to me this is not correct way to architect this. > > +1 > > I'm not sure adding locks is the proper way to do. > This is the application responsibility to ensure that: > - control functions are not called concurrently on the same port > - rx/tx functions are not called when the device is stopped/reset/... > > However, I do think the usage paradigms of the ethdev api should be better > documented in rte_ethdev.h (ex: which functions can be called concurrently). > This would be a first step. > > If we really want a helper API to do that in DPDK, the _next_ step could be to > add them in the ethdev api to achieve this. Maybe something like (the function > names could be better): > > - to be called on one control thread: > > rte_eth_stop_rxtx(port) > rte_eth_start_rxtx(port) > > rte_eth_get_rxtx_state(port) > -> return "running" if at least one core is inside the rx/tx code > -> return "stopped" if all cores are outside the rx/tx code > > - to be called on dataplane cores: > > /* same than rte_eth_rx_burst(), but checks if rx/tx is allowed >* first, else do nothing */ > rte_eth_rx_burst_interruptible() > rte_eth_tx_burst_interruptible() > > > The code of control thread could be: > > rte_eth_stop_rxtx(port); > /* wait that all dataplane cores finished their processing */ > while (rte_eth_get_rxtx_state(port) != stopped) > ; > rte_eth_some_control_operation(port); > rte_eth_start_rxtx(port); > > > I think this could be done without any lock, just with the proper memory > barriers > and a per-core status. > > But this API may impose a paradigm to the application, and I'm not sure the > DPDK should do that. I don't quite catch your point. Seems your solution still need the APP to change the code. I think it's more complex than just letting the APP to stop the rx/tx and reset the port. Our purpose of this patch set is to let APP do less as possible. It's not a good choice if we make it more complex. And seems it's hard to stop and start rx/tx in rte layer. Normally APP should do that. To my opinion, we have to introduce lock in rte to achieve that. > > Regards, > Olivier
[dpdk-dev] [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
Hi Konstantin,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Wednesday, June 8, 2016 5:20 PM
> To: Lu, Wenzhuo; Tao, Zhe; dev at dpdk.org
> Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang,
> Helin
> Subject: RE: [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
>
>
>
> >
> > Hi Konstantin,
> >
> >
> > > -Original Message-
> > > From: Ananyev, Konstantin
> > > Sent: Tuesday, June 7, 2016 5:59 PM
> > > To: Tao, Zhe; dev at dpdk.org
> > > Cc: Lu, Wenzhuo; Richardson, Bruce; Chen, Jing D; Liang, Cunming;
> > > Wu, Jingjing; Zhang, Helin
> > > Subject: RE: [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
> > >
> > >
> > > Hi Zhe & Wenzhuo,
> > >
> > > Please find my comments below.
> > > BTW, for clarification - is that patch for 16.11?
> > > I believe it's too late to introduce such significant change in 16.07.
> > > Thanks
> > > Konstantin
> > Thanks for the comments.
> > Honestly, our purpose is 16.07. Realizing the big impact, we use
> > NEXT_ABI to comment our change. So, I think although we want to merge it in
> 16.07 this change will become effective after we remove NEXT_ABI in 16.11.
>
> I don't think it is achievable.
> First I think your code is not in proper shape yet, right now.
> Second, as you said, it is a significant change and I would like to hear
> opinions
> from the rest of the community.
Agree it should have risk. I mean our target is 16.07. But surely if it can be
achieved depends on the feedback from the community.
>
> >
> > >
> > > > Define lock mode for RX/TX queue. Because when resetting the
> > > > device we want the resetting thread to get the lock of the RX/TX
> > > > queue to make sure the RX/TX is stopped.
> > > >
> > > > Using next ABI macro for this ABI change as it has too much
> > > > impact. 7 APIs and 1 global variable are impacted.
> > > >
> > > > Signed-off-by: Wenzhuo Lu
> > > > Signed-off-by: Zhe Tao
> > > > ---
> > > > lib/librte_ether/rte_ethdev.h | 62
> > > > +++
> > > > 1 file changed, 62 insertions(+)
> > > >
> > > > diff --git a/lib/librte_ether/rte_ethdev.h
> > > > b/lib/librte_ether/rte_ethdev.h index 74e895f..4efb5e9 100644
> > > > --- a/lib/librte_ether/rte_ethdev.h
> > > > +++ b/lib/librte_ether/rte_ethdev.h
> > > > @@ -354,7 +354,12 @@ struct rte_eth_rxmode {
> > > > jumbo_frame : 1, /**< Jumbo Frame Receipt enable.
> > > > */
> > > > hw_strip_crc : 1, /**< Enable CRC stripping by
> > > > hardware. */
> > > > enable_scatter : 1, /**< Enable scatter packets rx
> > > > handler */
> > > > +#ifndef RTE_NEXT_ABI
> > > > enable_lro : 1; /**< Enable LRO */
> > > > +#else
> > > > + enable_lro : 1, /**< Enable LRO */
> > > > + lock_mode: 1; /**< Using lock path */
> > > > +#endif
> > > > };
> > > >
> > > > /**
> > > > @@ -634,11 +639,68 @@ struct rte_eth_txmode {
> > > > /**< If set, reject sending out tagged pkts */
> > > > hw_vlan_reject_untagged : 1,
> > > > /**< If set, reject sending out untagged pkts */
> > > > +#ifndef RTE_NEXT_ABI
> > > > hw_vlan_insert_pvid : 1;
> > > > /**< If set, enable port based VLAN insertion */
> > > > +#else
> > > > + hw_vlan_insert_pvid : 1,
> > > > + /**< If set, enable port based VLAN insertion */
> > > > + lock_mode : 1;
> > > > + /**< If set, using lock path */ #endif
> > > > };
> > > >
> > > > /**
> > > > + * The macros for the RX/TX lock mode functions */ #ifdef
> > > > +RTE_NEXT_ABI #define RX_LOCK_FUNCTION(dev, func) \
> > > > + (dev->data->dev_conf.rxmode.lock_mode ? \
> > > > + func ## _lock : func)
> > > > +
> > > > +#define TX_LOCK_FUNCTION(dev, func) \
> > > > + (dev->data->dev_conf.txmode.lock_mode ? \
> > > > + func ##
[dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset on VF
Hi Konstantin,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Wednesday, June 8, 2016 4:42 PM
> To: Lu, Wenzhuo; Tao, Zhe; dev at dpdk.org
> Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang,
> Helin
> Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF
>
>
>
> > -----Original Message-
> > From: Lu, Wenzhuo
> > Sent: Wednesday, June 08, 2016 8:24 AM
> > To: Ananyev, Konstantin; Tao, Zhe; dev at dpdk.org
> > Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing;
> > Zhang, Helin
> > Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF
> >
> > Hi Konstantin,
> >
> > > -Original Message-
> > > From: Ananyev, Konstantin
> > > Sent: Tuesday, June 7, 2016 6:03 PM
> > > To: Tao, Zhe; dev at dpdk.org
> > > Cc: Lu, Wenzhuo; Richardson, Bruce; Chen, Jing D; Liang, Cunming;
> > > Wu, Jingjing; Zhang, Helin
> > > Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Tao, Zhe
> > > > Sent: Tuesday, June 07, 2016 7:53 AM
> > > > To: dev at dpdk.org
> > > > Cc: Lu, Wenzhuo; Tao, Zhe; Ananyev, Konstantin; Richardson, Bruce;
> > > > Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin
> > > > Subject: [PATCH v4 4/8] ixgbe: implement device reset on VF
> > > >
> > > > Implement the device reset function.
> > > > 1, Add the fake RX/TX functions.
> > > > 2, The reset function tries to stop RX/TX by replacing
> > > >the RX/TX functions with the fake ones and getting the
> > > >locks to make sure the regular RX/TX finished.
> > > > 3, After the RX/TX stopped, reset the VF port, and then
> > > >release the locks and restore the RX/TX functions.
> > > >
> > > > Signed-off-by: Wenzhuo Lu
> > > >
> > > > static int
> > > > +ixgbevf_dev_reset(struct rte_eth_dev *dev) {
> > > > + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > > >dev_private);
> > > > + struct ixgbe_adapter *adapter =
> > > > + (struct ixgbe_adapter *)dev->data->dev_private;
> > > > + int diag = 0;
> > > > + uint32_t vteiam;
> > > > + uint16_t i;
> > > > + struct ixgbe_rx_queue *rxq;
> > > > + struct ixgbe_tx_queue *txq;
> > > > +
> > > > + /* Nothing needs to be done if the device is not started. */
> > > > + if (!dev->data->dev_started)
> > > > + return 0;
> > > > +
> > > > + PMD_DRV_LOG(DEBUG, "Link up/down event detected.");
> > > > +
> > > > + /**
> > > > +* Stop RX/TX by fake functions and locks.
> > > > +* Fake functions are used to make RX/TX lock easier.
> > > > +*/
> > > > + adapter->rx_backup = dev->rx_pkt_burst;
> > > > + adapter->tx_backup = dev->tx_pkt_burst;
> > > > + dev->rx_pkt_burst = ixgbevf_recv_pkts_fake;
> > > > + dev->tx_pkt_burst = ixgbevf_xmit_pkts_fake;
> > >
> > > If you have locking over each queue underneath, why do you still
> > > need fake functions?
> > The fake functions are used to help saving the time of waiting for the
> > locks.
> > As you see, we want to lock every queue. If we don't use fake functions we
> have to wait for every queue.
> > But if the real functions are replaced by fake functions, ideally when
> > we're waiting for the release of the first queue's lock, the other queues
> > will run
> into the fake functions. So we need not wait for them and get the locks
> directly.
>
> Well, data-path invokes only try_lock(), so it shouldn't be affected
> significantly,
> right?
> Control path still have to spin on lock and grab it before it can proceed, if
> it'll
> spin a bit longer I wouldn't see a big deal here.
> What I am trying to say - if we'll go that way - introduce sync
> control/datapath
> API anyway, we don't need any additional tricks here with rx/tx function
> replacement, correct?
> So let's keep it clean and simple, after all it is a control path and not
> need to be
> lightning fast.
> Konstantin
Agree, it's not necessary to add the fake functions. I'll remove them to make
it simple.
>
> >
> > &
[dpdk-dev] [PATCH] examples: add a new example for link reset
Hi Konstantin, Thomas, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, June 8, 2016 5:00 PM > To: Ananyev, Konstantin; Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] examples: add a new example for link reset > > 2016-06-08 08:37, Ananyev, Konstantin: > > > From: Ananyev, Konstantin > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wenzhuo Lu > > > > > Add a new example to show when the PF is down and up, VF port > > > > > can be reset and recover. > > > > > > > > Do we really need a totally new example for it? > > > > Can't we put it in one of already existing ones? > > > > Let say we have l3fwd-vf... wouldn't that suit your needs? > > > > Konstantin > > > I thought about just modifying an existing example. But I choose to > > > add a new one at last. The benefit of a totally new example is we can > > > make it > simple enough and focus on the reset function. > > > So it's easier for the users to find what we want to show. And it's > > > also easier for us as we don't need to care about if our > > > modification will break some function of the original example :) > > > > I still think that adding a new example for esch new feature/function in > rte_ethdev API iw way too expensive. > > If your change is not good enough and will break original example, > > then you probably re-work your feature patch to make it stable enough. > > After all people will use it in their existing apps, not write the new ones > > right? > > BTW, why not make it work with testpmd? > > After all it is a new PMD api, an that's for we have our testpmd here? > > +1 for testpmd I may not make myself clear. I said "function" but actually I mainly mean the performance impact but not the functionality. As we know l2fwd and l3fwd can be used to show the performance of DPDK, adding lock will break this function, showing the performance data. That's why I don't want to touch l2fwd and l3fwd. Agree that testpmd can be a choice. I'll try to modify testpmd, maybe add a parameter, like "testpmd --lock". So by default we will not use lock mode.
[dpdk-dev] [PATCH 2/8] lib/librte_ether: defind RX/TX lock mode
Hi Stephen, > -Original Message- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Wednesday, June 8, 2016 10:16 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Tao, Zhe > Subject: Re: [dpdk-dev] [PATCH 2/8] lib/librte_ether: defind RX/TX lock mode > > On Mon, 6 Jun 2016 13:40:47 +0800 > Wenzhuo Lu wrote: > > > Define lock mode for RX/TX queue. Because when resetting the device we > > want the resetting thread to get the lock of the RX/TX queue to make > > sure the RX/TX is stopped. > > > > Using next ABI macro for this ABI change as it has too much impact. 7 > > APIs and 1 global variable are impacted. > > > > Signed-off-by: Wenzhuo Lu > > Signed-off-by: Zhe Tao > > Why does this patch set make a different assumption the rest of the DPDK? > > The rest of the DPDK operates on the principle that the application is smart > enough to stop the device before making changes. There is no equivalent to the > Linux kernel RTNL mutex. The API assumes application threads are well behaved > and will not try and sabotage each other. > > If you restrict the reset operation to only being available when RX/TX is > stopped, > then no lock is needed. > > The fact that it requires lots more locking inside each device driver implies > to me > this is not correct way to architect this. It's a good question. This patch set doesn't follow the regular assumption of DPDK. But it's a requirement we've got from some customers. The users want the driver does as much as it can. The best is the link state change is transparent to the users. The patch set tries to provide another choice if the users don't want to stop their rx/tx to handle the reset event. And as discussed in the other thread, most probably we will move the lock from the PMD layer to rte lay. It'll avoid the change in every device.
[dpdk-dev] [PATCH] examples: add a new example for link reset
Hi Konstantin, > -Original Message- > From: Ananyev, Konstantin > Sent: Wednesday, June 8, 2016 8:25 AM > To: Lu, Wenzhuo; dev at dpdk.org > Cc: Lu, Wenzhuo > Subject: RE: [dpdk-dev] [PATCH] examples: add a new example for link reset > > > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wenzhuo Lu > > Sent: Monday, June 06, 2016 6:48 AM > > To: dev at dpdk.org > > Cc: Lu, Wenzhuo > > Subject: [dpdk-dev] [PATCH] examples: add a new example for link reset > > > > Add a new example to show when the PF is down and up, VF port can be > > reset and recover. > > Do we really need a totally new example for it? > Can't we put it in one of already existing ones? > Let say we have l3fwd-vf... wouldn't that suit your needs? > Konstantin I thought about just modifying an existing example. But I choose to add a new one at last. The benefit of a totally new example is we can make it simple enough and focus on the reset function. So it's easier for the users to find what we want to show. And it's also easier for us as we don't need to care about if our modification will break some function of the original example :)
[dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset on VF
Hi Konstantin,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Tuesday, June 7, 2016 6:03 PM
> To: Tao, Zhe; dev at dpdk.org
> Cc: Lu, Wenzhuo; Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu,
> Jingjing;
> Zhang, Helin
> Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF
>
>
>
> > -Original Message-
> > From: Tao, Zhe
> > Sent: Tuesday, June 07, 2016 7:53 AM
> > To: dev at dpdk.org
> > Cc: Lu, Wenzhuo; Tao, Zhe; Ananyev, Konstantin; Richardson, Bruce;
> > Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin
> > Subject: [PATCH v4 4/8] ixgbe: implement device reset on VF
> >
> > Implement the device reset function.
> > 1, Add the fake RX/TX functions.
> > 2, The reset function tries to stop RX/TX by replacing
> >the RX/TX functions with the fake ones and getting the
> >locks to make sure the regular RX/TX finished.
> > 3, After the RX/TX stopped, reset the VF port, and then
> >release the locks and restore the RX/TX functions.
> >
> > Signed-off-by: Wenzhuo Lu
> >
> > static int
> > +ixgbevf_dev_reset(struct rte_eth_dev *dev) {
> > + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + struct ixgbe_adapter *adapter =
> > + (struct ixgbe_adapter *)dev->data->dev_private;
> > + int diag = 0;
> > + uint32_t vteiam;
> > + uint16_t i;
> > + struct ixgbe_rx_queue *rxq;
> > + struct ixgbe_tx_queue *txq;
> > +
> > + /* Nothing needs to be done if the device is not started. */
> > + if (!dev->data->dev_started)
> > + return 0;
> > +
> > + PMD_DRV_LOG(DEBUG, "Link up/down event detected.");
> > +
> > + /**
> > +* Stop RX/TX by fake functions and locks.
> > +* Fake functions are used to make RX/TX lock easier.
> > +*/
> > + adapter->rx_backup = dev->rx_pkt_burst;
> > + adapter->tx_backup = dev->tx_pkt_burst;
> > + dev->rx_pkt_burst = ixgbevf_recv_pkts_fake;
> > + dev->tx_pkt_burst = ixgbevf_xmit_pkts_fake;
>
> If you have locking over each queue underneath, why do you still need fake
> functions?
The fake functions are used to help saving the time of waiting for the locks.
As you see, we want to lock every queue. If we don't use fake functions we have
to wait for every queue.
But if the real functions are replaced by fake functions, ideally when we're
waiting for the release of the first queue's lock,
the other queues will run into the fake functions. So we need not wait for them
and get the locks directly.
>
> > +
> > + if (dev->data->rx_queues)
> > + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > + rxq = dev->data->rx_queues[i];
> > + rte_spinlock_lock(>rx_lock);
> > + }
> > +
> > + if (dev->data->tx_queues)
> > + for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > + txq = dev->data->tx_queues[i];
> > + rte_spinlock_lock(>tx_lock);
> > + }
>
> Probably worth to create a separate function for the lines above:
> lock_all_queues(), unlock_all_queues.
> But as I sadi in previous mail - I think that code better be in rte_ethdev.
We're discussing it in the previous thread :)
> >
> > @@ -5235,11 +5243,21 @@ ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev)
> > rxdctl = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(i));
> > } while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE));
> > if (!poll_ms)
> > +#ifndef RTE_NEXT_ABI
> > + PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d",
> i); #else
> > + {
> > PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d",
> i);
> > + if (dev->data->dev_conf.rxmode.lock_mode)
> > + return -1;
> > + }
> > +#endif
>
>
> Why the code has to be different here?
As you see this rxtx_start may have chance to fail. I want to expose this
failure, so the reset function can try again.
> Thanks
> Konstantin
[dpdk-dev] [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
Hi Konstantin,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Tuesday, June 7, 2016 5:59 PM
> To: Tao, Zhe; dev at dpdk.org
> Cc: Lu, Wenzhuo; Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu,
> Jingjing;
> Zhang, Helin
> Subject: RE: [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode
>
>
> Hi Zhe & Wenzhuo,
>
> Please find my comments below.
> BTW, for clarification - is that patch for 16.11?
> I believe it's too late to introduce such significant change in 16.07.
> Thanks
> Konstantin
Thanks for the comments.
Honestly, our purpose is 16.07. Realizing the big impact, we use NEXT_ABI to
comment our change. So, I think although we want to merge it in 16.07 this
change will become effective after we remove NEXT_ABI in 16.11.
>
> > Define lock mode for RX/TX queue. Because when resetting the device we
> > want the resetting thread to get the lock of the RX/TX queue to make
> > sure the RX/TX is stopped.
> >
> > Using next ABI macro for this ABI change as it has too much impact. 7
> > APIs and 1 global variable are impacted.
> >
> > Signed-off-by: Wenzhuo Lu
> > Signed-off-by: Zhe Tao
> > ---
> > lib/librte_ether/rte_ethdev.h | 62
> > +++
> > 1 file changed, 62 insertions(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.h
> > b/lib/librte_ether/rte_ethdev.h index 74e895f..4efb5e9 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -354,7 +354,12 @@ struct rte_eth_rxmode {
> > jumbo_frame : 1, /**< Jumbo Frame Receipt enable. */
> > hw_strip_crc : 1, /**< Enable CRC stripping by hardware. */
> > enable_scatter : 1, /**< Enable scatter packets rx handler */
> > +#ifndef RTE_NEXT_ABI
> > enable_lro : 1; /**< Enable LRO */
> > +#else
> > + enable_lro : 1, /**< Enable LRO */
> > + lock_mode: 1; /**< Using lock path */
> > +#endif
> > };
> >
> > /**
> > @@ -634,11 +639,68 @@ struct rte_eth_txmode {
> > /**< If set, reject sending out tagged pkts */
> > hw_vlan_reject_untagged : 1,
> > /**< If set, reject sending out untagged pkts */
> > +#ifndef RTE_NEXT_ABI
> > hw_vlan_insert_pvid : 1;
> > /**< If set, enable port based VLAN insertion */
> > +#else
> > + hw_vlan_insert_pvid : 1,
> > + /**< If set, enable port based VLAN insertion */
> > + lock_mode : 1;
> > + /**< If set, using lock path */
> > +#endif
> > };
> >
> > /**
> > + * The macros for the RX/TX lock mode functions */ #ifdef
> > +RTE_NEXT_ABI #define RX_LOCK_FUNCTION(dev, func) \
> > + (dev->data->dev_conf.rxmode.lock_mode ? \
> > + func ## _lock : func)
> > +
> > +#define TX_LOCK_FUNCTION(dev, func) \
> > + (dev->data->dev_conf.txmode.lock_mode ? \
> > + func ## _lock : func)
> > +#else
> > +#define RX_LOCK_FUNCTION(dev, func) func
> > +
> > +#define TX_LOCK_FUNCTION(dev, func) func #endif
> > +
> > +/* Add the lock RX/TX function for VF reset */ #define
> > +GENERATE_RX_LOCK(func, nic) \ uint16_t func ## _lock(void *rx_queue,
> > +\
> > + struct rte_mbuf **rx_pkts, \
> > + uint16_t nb_pkts) \
> > +{ \
> > + struct nic ## _rx_queue *rxq = rx_queue; \
> > + uint16_t nb_rx = 0; \
> > + \
> > + if (rte_spinlock_trylock(>rx_lock)) { \
> > + nb_rx = func(rx_queue, rx_pkts, nb_pkts); \
> > + rte_spinlock_unlock(>rx_lock); \
> > + } \
> > + \
> > + return nb_rx; \
> > +}
> > +
> > +#define GENERATE_TX_LOCK(func, nic) \ uint16_t func ## _lock(void
> > +*tx_queue, \
> > + struct rte_mbuf **tx_pkts, \
> > + uint16_t nb_pkts) \
> > +{ \
> > + struct nic ## _tx_queue *txq = tx_queue; \
> > + uint16_t nb_tx = 0; \
> > + \
> > + if (rte_spinlock_trylock(>tx_lock)) { \
> > + nb_tx = func(tx_queue, tx_pkts, nb_pkts); \
> > + rte_spinlock_unlock(>tx_lock); \
> > + } \
> > + \
> > + return nb_tx; \
> > +}
>
> 1. As I said in off-line dicussiion, I think this locking could (and I think
> better be)
> i
[dpdk-dev] [PATCH v1] change log level for check when add port in blacklist
Hi, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of xu,huilong > Sent: Tuesday, May 31, 2016 10:40 AM > To: dev at dpdk.org > Cc: thomas.monjalon at 6wind.com; Xu, HuilongX > Subject: [dpdk-dev] [PATCH v1] change log level for check when add port in > blacklist > > maybe we should change log level, when add port in blacklist,for check it > easy. > and it not influence performance and function > > Signed-off-by: xu,huilong Acked-by: Wenzhuo Lu Considering the usability, prefer to show the log to users.
[dpdk-dev] [PATCH v2 00/15] i40e base driver update
Hi, Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH v2] ixgbe: configure VLAN TPID
Hi, > -Original Message- > From: Xing, Beilei > Sent: Friday, June 3, 2016 10:58 AM > To: Lu, Wenzhuo > Cc: dev at dpdk.org; Xing, Beilei > Subject: [PATCH v2] ixgbe: configure VLAN TPID > > The patch enables configuring the ether types of both inner and outer VLANs. > > Signed-off-by: Beilei Xing Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH] ethdev: change comments of VLAN type
Hi Beilei, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Beilei Xing > Sent: Thursday, May 26, 2016 3:28 PM > To: Wu, Jingjing > Cc: dev at dpdk.org; Xing, Beilei > Subject: [dpdk-dev] [PATCH] ethdev: change comments of VLAN type > > If the packet carries a single VLAN header, it is treated as the outer header. > So change the comments of inner VLAN and outer VLAN. > > Signed-off-by: Beilei Xing > --- > doc/guides/rel_notes/release_16_07.rst | 3 +++ > lib/librte_ether/rte_ethdev.h | 4 ++-- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/doc/guides/rel_notes/release_16_07.rst > b/doc/guides/rel_notes/release_16_07.rst > index 30e78d4..29db86c 100644 > --- a/doc/guides/rel_notes/release_16_07.rst > +++ b/doc/guides/rel_notes/release_16_07.rst > @@ -116,6 +116,9 @@ API Changes >ibadcrc, ibadlen, imcasts, fdirmatch, fdirmiss, >tx_pause_xon, rx_pause_xon, tx_pause_xoff, rx_pause_xoff. > > +* The comments of ``ETH_VLAN_TYPE_INNER`` and > ``ETH_VLAN_TYPE_OUTER`` > +in > + ``rte_vlan_type`` are changed. I think we need more explanation here. At least the info in the commit log.
[dpdk-dev] [PATCH 1/2] ixgbe: VF supports mailbox interruption for PF link up/down
Hi Jingjing,
Thanks for the comments. Please see inline.
> -Original Message-
> From: Wu, Jingjing
> Sent: Friday, May 27, 2016 4:31 PM
> To: Lu, Wenzhuo; dev at dpdk.org
> Cc: Lu, Wenzhuo
> Subject: RE: [dpdk-dev] [PATCH 1/2] ixgbe: VF supports mailbox interruption
> for
> PF link up/down
>
> > +static void ixgbevf_mbx_process(struct rte_eth_dev *dev) {
> > + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > + u32 in_msg = 0;
> > +
> > + if (ixgbe_read_mbx(hw, _msg, 1, 0))
> > + return;
> > +
> > + /* PF reset VF event */
> > + if (in_msg == IXGBE_PF_CONTROL_MSG)
> > + _rte_eth_dev_callback_process(dev,
> > RTE_ETH_EVENT_INTR_RESET); }
> > +
>
> RTE_ETH_EVENT_INTR_RESET is used for PF reset event reporting, and this
> patch is to support PF link change. Maybe RTE_ETH_EVENT_INTR_LSC should be
> used here instead.
> Or you need to distinguish which control message is coming from PF.
Ixgbe PF driver use IXGBE_PF_CONTROL_MSG to cover LSC, configuration, link
up/down.
After these events, VF cannot work. I want the user to reset the VF port to
make it work again.
Let me explain more in commit log to make it clearer.
>
> > +static int
> > +ixgbevf_dev_interrupt_get_status(struct rte_eth_dev *dev) {
> > + uint32_t eicr;
> > + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > + struct ixgbe_interrupt *intr =
> > + IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> > + ixgbevf_intr_disable(hw);
> > +
> > + /* read-on-clear nic registers here */
> > + eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
> > + intr->flags = 0;
> > +
> > + /* only one misc vector supported - mailbox */
> > + eicr &= IXGBE_VTEICR_MASK;
> > + if (eicr == IXGBE_MISC_VEC_ID)
> > + intr->flags |= IXGBE_FLAG_MAILBOX;
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev) {
> > + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > + struct ixgbe_interrupt *intr =
> > + IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> > +
> > + if (intr->flags & IXGBE_FLAG_MAILBOX) {
> > + ixgbevf_mbx_process(dev);
> > + intr->flags &= ~IXGBE_FLAG_MAILBOX;
> > + }
> > +
> > + ixgbevf_intr_enable(hw);
> > +
> > + return 0;
> > +}
>
>
> For the readability, it's better to put ixgbevf_intr_disable and
> ixgbevf_intr_enable in the same function, for example, at the beginning and
> ending of ixgbevf_dev_interrupt_handler.
O, I follow the style of ixgbe. It's a good suggestion, I may create a new
patch to enhance it later.
>
> Thanks
> Jingjing
[dpdk-dev] flow director on X550
Hi Nishant, From: Nishant Verma [mailto:vnis...@gmail.com] Sent: Thursday, May 26, 2016 11:40 AM To: Lu, Wenzhuo Cc: dev at dpdk.org Subject: Re: [dpdk-dev] flow director on X550 Hi Wenzhuo, Thanks for the reply. ?As par datasheet, if flow director filter matches then RSS won't impact as RSS is the last filter to be applied on received packet. But just to confirm, i tried and still issue persist. Any other thing that you think i can try, please let me know. [Wenzhuo] After checking the code, according to our implementation you cannot mask all the L4 ports if you want to direct the L4 packets. With all the ports masked, flow director only handles the raw IP packets. It means UDP/TCP/SCTP packets will not be handled. There?s a workaround. You can change the dest port mask to 0x01 (or src port if you like). And add 2 filters, one for dest IP 66.66.66.66 + dest port 0, the other for dest IP 66.66.66.66 + dest port 1. Thanks. On Wed, May 25, 2016 at 8:52 PM, Lu, Wenzhuo mailto:wenzhuo.lu at intel.com>> wrote: Hi Nishant, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org<mailto:dev-bounces at dpdk.org>] On > Behalf Of Nishant Verma > Sent: Thursday, May 26, 2016 4:08 AM > To: dev at dpdk.org<mailto:dev at dpdk.org> > Subject: [dpdk-dev] flow director on X550 > > Hi All, > > My system configuration is > ==>#. SuperMicro 1U >- BIOS: 1.0a >- Processor: Intel(R) Xeon(R) CPU D-1540 @ 2.00GHz >- Onboard NIC: Intel(R) X552/X557-AT (2x10G) > - Firmware-version: 0x81cf > - Device ID (PF/VF): 8086:15ad /8086:15a8 >- kernel driver version: 4.2.5 (ixgbe) > > I am working on DPDK 16.04 & pktgen 3.0.0 version. > > > My intention is to test flow director based on just destination IP. It means, > i will > use test-pmd and configure flow director and fdir mask, from pktgen i will > send > packet and check if packets are going to right queue or not. > > Here is the procedure, that i follow. > > 1. I run testpmd > * ./testpmd -c 0x -n 4 -- -i --portmask=0x3 --nb-cores=5 > --disable-link-check > --rxq=5 --txq=5 --pkt-filter-mode=perfect* We need add the parameter *--disable-rss* to avoid the impact of RSS. -- Rgds, Nishant
[dpdk-dev] flow director on X550
Hi Nishant, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Nishant Verma > Sent: Thursday, May 26, 2016 4:08 AM > To: dev at dpdk.org > Subject: [dpdk-dev] flow director on X550 > > Hi All, > > My system configuration is > ==>#. SuperMicro 1U >- BIOS: 1.0a >- Processor: Intel(R) Xeon(R) CPU D-1540 @ 2.00GHz >- Onboard NIC: Intel(R) X552/X557-AT (2x10G) > - Firmware-version: 0x81cf > - Device ID (PF/VF): 8086:15ad /8086:15a8 >- kernel driver version: 4.2.5 (ixgbe) > > I am working on DPDK 16.04 & pktgen 3.0.0 version. > > > My intention is to test flow director based on just destination IP. It means, > i will > use test-pmd and configure flow director and fdir mask, from pktgen i will > send > packet and check if packets are going to right queue or not. > > Here is the procedure, that i follow. > > 1. I run testpmd > * ./testpmd -c 0x -n 4 -- -i --portmask=0x3 --nb-cores=5 > --disable-link-check > --rxq=5 --txq=5 --pkt-filter-mode=perfect* We need add the parameter *--disable-rss* to avoid the impact of RSS.
[dpdk-dev] [PATCH] e1000: fix build with clang
Hi,
> -Original Message-
> From: Hiroyuki Mikita [mailto:h.mikita89 at gmail.com]
> Sent: Tuesday, May 24, 2016 10:48 PM
> To: Lu, Wenzhuo
> Cc: dev at dpdk.org
> Subject: [PATCH] e1000: fix build with clang
>
> GCC_VERSION is empty in case of clang:
> /bin/sh: line 0: test: -ge: unary operator expected
>
> It is the same issue as http://dpdk.org/dev/patchwork/patch/5994/
>
> Fixes: 366113dbfb69 ("e1000: suppress misleading indentation warning")
>
> Signed-off-by: Hiroyuki Mikita
Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH 0/4] automatic link recovery on ixgbe/igb VF
Hi, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wenzhuo Lu > Sent: Thursday, May 5, 2016 5:11 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH 0/4] automatic link recovery on ixgbe/igb VF > > Now if the PF link is down and up, VF doesn't handle this event, user need to > reset the VF port to let it recover. > This patch set addes the support of the mailbox interruption on VF. So, VF can > receice the messges for physical link down/up. > And VF will handle this event and let the VF link recover automatically. > > Wenzhuo Lu (4): > ixgbe: VF supports mailbox interruption for PF link up/down > igb: VF supports mailbox interruption for PF link up/down > ixgbe: automatic link recovery on VF > igb: automatic link recovery on VF > > doc/guides/rel_notes/release_16_07.rst | 11 ++ > drivers/net/e1000/e1000_ethdev.h | 14 ++ > drivers/net/e1000/igb_ethdev.c | 244 > + > drivers/net/e1000/igb_rxtx.c | 38 + > drivers/net/ixgbe/ixgbe_ethdev.c | 169 ++- > drivers/net/ixgbe/ixgbe_ethdev.h | 14 ++ > drivers/net/ixgbe/ixgbe_rxtx.c | 34 + > drivers/net/ixgbe/ixgbe_rxtx.h | 2 + > 8 files changed, 523 insertions(+), 3 deletions(-) > > -- > 1.9.3 Self Nack. Will split this patch set to 2 ones. One provides support of the mailbox interruption for PF link up/down. The other is for the mechanism of VF link recovery. For there's discussion about if we need to handle the link up/down event in driver layer. No matter the driver should handle the event or not, we need to mailbox interruption first.
[dpdk-dev] Query on RSS Rule
Hi Nishant, From: Nishant Verma [mailto:vnis...@gmail.com] Sent: Thursday, May 19, 2016 11:21 AM To: Lu, Wenzhuo Cc: dev at dpdk.org Subject: Re: [dpdk-dev] Query on RSS Rule Hi Wenzhuo, Tried UDP as well as TCP. Also use dump function to check if packet it correct or not. I found packet perfectly fine. but anyhow problem still remain the same. Wenzhuo: Glad to know the problem is not related to the protocol. I don?t find anything wrong in your code, except I?m not sure if the mask for dst_ip is right. Suppose it?s right ? Seems having to check if the register is right. I mean the registers in the function ixgbe_add_5tuple_filter. Not sure if it?s easy to check them in your APP. If not, maybe you can try testpmd first. On Wed, May 18, 2016 at 11:09 PM, Lu, Wenzhuo mailto:wenzhuo.lu at intel.com>> wrote: Hi Nishant, From: Nishant Verma [mailto:vnish11 at gmail.com<mailto:vnis...@gmail.com>] Sent: Thursday, May 19, 2016 10:29 AM To: Lu, Wenzhuo Cc: dev at dpdk.org<mailto:dev at dpdk.org> Subject: Re: [dpdk-dev] Query on RSS Rule Hi Wenzhuo, Thanks for the reply. Yes, i am using ixgbe. On software front, this is what i am doing. I am using DPDK 16.04 and pktgen 3.0.00 On my DPDK machine, i have configured RSS rule just for Destination IP (172.10.10.2). [rss]<https://cloud.githubusercontent.com/assets/7613402/15160737/2aa17d4e-16c9-11e6-9161-4178056176ca.png>
[dpdk-dev] Query on RSS Rule
Hi Nishant, From: Nishant Verma [mailto:vnis...@gmail.com] Sent: Thursday, May 19, 2016 10:29 AM To: Lu, Wenzhuo Cc: dev at dpdk.org Subject: Re: [dpdk-dev] Query on RSS Rule Hi Wenzhuo, Thanks for the reply. Yes, i am using ixgbe. On software front, this is what i am doing. I am using DPDK 16.04 and pktgen 3.0.00 On my DPDK machine, i have configured RSS rule just for Destination IP (172.10.10.2). [rss]<https://cloud.githubusercontent.com/assets/7613402/15160737/2aa17d4e-16c9-11e6-9161-4178056176ca.png>
[dpdk-dev] Query on RSS Rule
Hi Nishant, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Nishant Verma > Sent: Thursday, May 19, 2016 7:06 AM > To: dev at dpdk.org > Subject: [dpdk-dev] Query on RSS Rule > > ?Hi All, > > It's very basic question, but somehow i am blocked due to this issue. > Please help me out. > > I have configured NTUPLE filter in my application with just Destination IP > every > thing else(SRC IP, S_PORT, D_PORT & proto) is disabled. Suppose you're using 5-tuple, right? Suppose you're using a igb or ixgbe NIC as 5-tuple is only supported by igb/ixgbe, right? Would you like to let us know what you've done? I mean how you disable the other things. I think you might set the mask to do that. And please aware if the mask is FF, means the field is used. On the contrary, the mask should be 0. > But whenever i send packet from any machine, it means Different Source IP, > hash value at DPDK app side changed and hence result in, Arrival of packet at > different queue. > > Any hint would be appreciated. > > Thanks > > -- > Rgds, > ?NV
[dpdk-dev] [PATCH 3/4] ixgbe: automatic link recovery on VF
Hi Olivier, > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Tuesday, May 17, 2016 3:51 PM > To: Lu, Wenzhuo; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 3/4] ixgbe: automatic link recovery on VF > > Hi Wenzhuo, > > On 05/17/2016 03:11 AM, Lu, Wenzhuo wrote: > >> -Original Message- > >> From: Olivier Matz [mailto:olivier.matz at 6wind.com] If I understand > >> well, ixgbevf_dev_link_up_down_handler() is called by > >> ixgbevf_recv_pkts_fake() on a dataplane core. It means that the core > >> that acquired the lock will loop during 100us + 1sec at least. > >> If this core was also in charge of polling other queues of other > >> ports, or timers, many packets will be dropped (even with a 100us > >> loop). I don't think it is acceptable to actively wait inside a rx > >> function. > >> > >> I think it would avoid many issues to delegate this work to the > >> application, maybe by notifying it that the port is in a bad state > >> and must be restarted. The application could then properly stop > >> polling the queues, and stop and restart the port in a separate thread, > without bothering the dataplane cores. > > Thanks for the comments. > > Yes, you're right. I had a wrong assumption that every queue is handled by > > one > core. > > But surely it's not right, we cannot tell how the users will deploy their > > system. > > > > I plan to update this patch set. The solution now is, first let the > > users choose if they want this auto-reset feature. If so, we will > > apply another series rx/tx functions which have lock. So we can stop the > > rx/tx > of the bad ports. > > And we also apply a reset API for users. The APPs should call this API in > > their > management thread or so. > > It means APPs should guarantee the thread safe for the API. > > You see, there're 2 things, > > 1, Lock the rx/tx to stop them for users. > > 2, Apply a resetting API for users, and every NIC can do their own > > job. APPs need not to worry about the difference between different NICs. > > > > Surely, it's not *automatic* now. The reason is DPDK doesn't guarantee > > the thread safe. So the operations have to be left to the APPs and let them > > to > guarantee the thread safe. > > > > And if the users choose not using auto-reset feature, we will leave > > this work to the APP :) > > Yes, I think having 2 modes is a good approach: > > - the first mode would let the application know a reset has to >be performed, without active loop or lock in the rx/tx funcs. > - the second mode would transparently manage the reset in the driver, >but may lock the core during some time. For the second mode, at first we want to let the driver manage the reset transparently. But the bad news is in driver layer the operations is not thread safe. If we want the reset to be transparent, we need a whole new mechanism to guarantee the thread safe for the operations in driver layer. Obviously, it need to be discussed and cannot be finished in this release. So now we write a reset API for APP, and let APP call this API and guarantee the thread safe for all the operations. It's not transparent. But seems it's what we can do at this stage. > > By the way, you talk about a reset API, why not just using the usual > stop/start > functions? I think it would work the same. For ixgbe/igb, stop/start is enough. But for i40e, some other work should be done. (For example, the resource of the queues should be re-init.) So we think about introducing a new API, then different NICs can do what they have to do. > > Regards, > Olivier
[dpdk-dev] [PATCH 3/4] ixgbe: automatic link recovery on VF
Hi Olivier,
> -Original Message-
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Monday, May 16, 2016 8:01 PM
> To: Lu, Wenzhuo; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 3/4] ixgbe: automatic link recovery on VF
>
> Hi Wenzhuo,
>
> On 05/04/2016 11:10 PM, Wenzhuo Lu wrote:
> > When the physical link is down and recover later, the VF link cannot
> > recover until the user stop and start it manually.
> > This patch implements the automatic recovery of VF port.
> > The automatic recovery bases on the link up/down message received from
> > PF. When VF receives the link up/down message, it will replace the
> > RX/TX and operation functions with fake ones to stop RX/TX and any
> > future operation. Then reset the VF port.
> > After successfully resetting the port, recover the RX/TX and operation
> > functions.
> >
> > Signed-off-by: Wenzhuo Lu
> >
> > [...]
> >
> > +void
> > +ixgbevf_dev_link_up_down_handler(struct rte_eth_dev *dev) {
> > + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + struct ixgbe_adapter *adapter =
> > + (struct ixgbe_adapter *)dev->data->dev_private;
> > + int diag;
> > + uint32_t vteiam;
> > +
> > + /* Only one working core need to performance VF reset */
> > + if (rte_spinlock_trylock(>vf_reset_lock)) {
> > + /**
> > +* When fake rec/xmit is replaced, working thread may is
> running
> > +* into real RX/TX func, so wait long enough to assume all
> > +* working thread exit. The assumption is it will spend less
> > +* than 100us for each execution of RX and TX func.
> > +*/
> > + rte_delay_us(100);
> > +
> > + do {
> > + dev->data->dev_started = 0;
> > + ixgbevf_dev_stop(dev);
> > + rte_delay_us(100);
>
> If I understand well, ixgbevf_dev_link_up_down_handler() is called by
> ixgbevf_recv_pkts_fake() on a dataplane core. It means that the core that
> acquired the lock will loop during 100us + 1sec at least.
> If this core was also in charge of polling other queues of other ports, or
> timers,
> many packets will be dropped (even with a 100us loop). I don't think it is
> acceptable to actively wait inside a rx function.
>
> I think it would avoid many issues to delegate this work to the application,
> maybe by notifying it that the port is in a bad state and must be restarted.
> The
> application could then properly stop polling the queues, and stop and restart
> the
> port in a separate thread, without bothering the dataplane cores.
Thanks for the comments.
Yes, you're right. I had a wrong assumption that every queue is handled by one
core.
But surely it's not right, we cannot tell how the users will deploy their
system.
I plan to update this patch set. The solution now is, first let the users
choose if they want this
auto-reset feature. If so, we will apply another series rx/tx functions which
have lock. So we
can stop the rx/tx of the bad ports.
And we also apply a reset API for users. The APPs should call this API in their
management thread or so.
It means APPs should guarantee the thread safe for the API.
You see, there're 2 things,
1, Lock the rx/tx to stop them for users.
2, Apply a resetting API for users, and every NIC can do their own job. APPs
need not to worry about the difference
between different NICs.
Surely, it's not *automatic* now. The reason is DPDK doesn't guarantee the
thread safe. So the operations have to be
left to the APPs and let them to guarantee the thread safe.
And if the users choose not using auto-reset feature, we will leave this work
to the APP :)
>
>
> Regards,
> Olivier
[dpdk-dev] [PATCH] ixgbe: fix bad shift operation in ixgbe_set_pool_tx
Hi,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tomasz Kulasek
> Sent: Friday, April 15, 2016 10:33 PM
> To: dev at dpdk.org
> Cc: Zhang, Helin; Ananyev, Konstantin
> Subject: [dpdk-dev] [PATCH] ixgbe: fix bad shift operation in
> ixgbe_set_pool_tx
>
> CID 13190 (#1 of 1): Bad bit shift operation (BAD_SHIFT)
> large_shift: In expression 1 << pool, left shifting by more than 31 bits has
> undefined behavior. The shift amount, pool, is at least 32.
>
> This patch limits mask shift to be in range of 32 bit PFVFTE[1] register, for
> pool >
> 31.
>
> Fixes: fe3a45fd4104 ("ixgbe: add VMDq support")
>
> Signed-off-by: Tomasz Kulasek
Acked-by: Wenzhuo Lu
[dpdk-dev] [PATCH] ixgbe: fix bad shift operation in ixgbe_set_pool_rx
Hi Tomasz,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tomasz Kulasek
> Sent: Friday, April 15, 2016 9:39 PM
> To: dev at dpdk.org
> Cc: Zhang, Helin; Ananyev, Konstantin
> Subject: [dpdk-dev] [PATCH] ixgbe: fix bad shift operation in
> ixgbe_set_pool_rx
>
> CID 13193 (#1 of 1): Bad bit shift operation (BAD_SHIFT)
> large_shift: In expression 1 << pool, left shifting by more than 31 bits has
> undefined behavior. The shift amount, pool, is at least 32.
>
> This patch limits mask shift to be in range of 32 bit PFVFRE[1] register, for
> pool >
> 31.
>
> Fixes: fe3a45fd4104 ("ixgbe: add VMDq support")
>
> Signed-off-by: Tomasz Kulasek
Acked-by: Wenzhuo Lu
