Re: [dpdk-dev] [RFC PATCH 00/11] net/virtio: packed ring layout
On Mon, May 08, 2017 at 01:02:57PM +0800, Yuanhan Liu wrote: > On Fri, May 05, 2017 at 09:57:11AM -0400, Jens Freimann wrote: > > Hi Yuanhan, > > > > I rebased your patches on next-virtio/for-testing to current master, > > made sure every patch compiles and still works. > > Thanks for that. > > > I'm implementing the receive path now to eventually get some benchmark > > results for that as well (Patches not included yet) > > Good to know. Any progress? I'm asking because that was also my plan for > this week: make Rx work. I haven't started it though. Nothing worth showing yet, I'm still figuring things out and will most likely need a few more days before I have a first prototype. regards, Jens
[dpdk-dev] [PATCH] eal/bsd: don't zero the pages during mmap in contigmem
Don't zero the pages during mmap in contigmem. Instead, zero the
pages after mmap in primary process. Otherwise, the multi-process
support will be broken, as the pages will be zeroed when secondary
processes map the memory.
Fixes: 82f931805506 ("contigmem: zero all pages during mmap")
Cc: sta...@dpdk.org
Signed-off-by: Tiwei Bie
---
lib/librte_eal/bsdapp/contigmem/contigmem.c | 1 -
lib/librte_eal/bsdapp/eal/eal_memory.c | 3 +++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/bsdapp/contigmem/contigmem.c
b/lib/librte_eal/bsdapp/contigmem/contigmem.c
index da971debe..3826055f7 100644
--- a/lib/librte_eal/bsdapp/contigmem/contigmem.c
+++ b/lib/librte_eal/bsdapp/contigmem/contigmem.c
@@ -227,7 +227,6 @@ contigmem_mmap_single(struct cdev *cdev, vm_ooffset_t
*offset, vm_size_t size,
if (buffer_index >= contigmem_num_buffers)
return EINVAL;
- memset(contigmem_buffers[buffer_index], 0, contigmem_buffer_size);
*offset = (vm_ooffset_t)vtophys(contigmem_buffers[buffer_index]);
*obj = vm_pager_allocate(OBJT_DEVICE, cdev, size, nprot, *offset,
curthread->td_ucred);
diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c
b/lib/librte_eal/bsdapp/eal/eal_memory.c
index 3614da8db..a2809d66a 100644
--- a/lib/librte_eal/bsdapp/eal/eal_memory.c
+++ b/lib/librte_eal/bsdapp/eal/eal_memory.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
#include
@@ -121,6 +122,8 @@ rte_eal_hugepage_init(void)
seg->nrank = mcfg->nrank;
seg->socket_id = 0;
+ memset(seg->addr, 0, seg->len);
+
RTE_LOG(INFO, EAL, "Mapped memory segment %u @ %p:
physaddr:0x%"
PRIx64", len %zu\n",
seg_idx, addr, physaddr,
hpi->hugepage_sz);
--
2.12.1
Re: [dpdk-dev] [PATCH] doc: API change notice for librte_table
-Original Message- From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Cristian Dumitrescu Sent: Tuesday, May 2, 2017 7:23 PM To: dev@dpdk.org Subject: [dpdk-dev] [PATCH] doc: API change notice for librte_table Signed-off-by: Cristian Dumitrescu Acked-by: Jasvinder Singh
Re: [dpdk-dev] Fwd: Sharing tables among pipelines
Hi Nidhia, For my pipeline application, I want to share same table between two different pipeline. Is that possible? If yes, how can I do it? [Jasvinder] - In the current ip pipeline application, we don’t have such illustration of sharing a table between two pipelines. Therefore, you need to tweak the code to implement it for your application. Thanks, Jasvinder
Re: [dpdk-dev] [PATCH] eal/bsd: don't zero the pages during mmap in contigmem
On Mon, May 08, 2017 at 08:09:16AM +, Tiwei Bie wrote:
> Don't zero the pages during mmap in contigmem. Instead, zero the
> pages after mmap in primary process. Otherwise, the multi-process
> support will be broken, as the pages will be zeroed when secondary
> processes map the memory.
>
> Fixes: 82f931805506 ("contigmem: zero all pages during mmap")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Tiwei Bie
> ---
I agree there is a problem here, but I'm not sure about the solution to
it. I still think that the kernel should zero the pages before they get
given to userspace. Is there any way to keep that working e.g
* have them zeroed on mmap only when they are not already mmaped into
another process?
* have them zeroed on init, and again on unmap by the last process to
have them mapped?
/Bruce
Re: [dpdk-dev] [PATCH 1/2] eal/bsd: fix ioport write operation
On Sun, May 07, 2017 at 01:33:33PM +, Tiwei Bie wrote:
> The first param of out*() on FreeBSD is port, and the second one is
> data. But they are reversed in DPDK. This patch fixes it.
>
> Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API") Cc:
> sta...@dpdk.org
>
> Signed-off-by: Tiwei Bie ---
How was this bug discovered so that we can verify that it is fixed?. Is
this in use by virtio or was it just discovered via code inspection?
/Bruce
Re: [dpdk-dev] Occasional instability in RSS Hashes/Queues from X540 NIC
Hi, Matt Could you send me the pcap file? I'm dealing with this issue. Thank you~ Qiming > -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Matt Laswell > Sent: Friday, May 5, 2017 9:05 PM > To: dev@dpdk.org > Subject: Re: [dpdk-dev] Occasional instability in RSS Hashes/Queues from > X540 NIC > > On Thu, May 4, 2017 at 1:15 PM, Matt Laswell wrote: > > > Hey Keith, > > > > Here is a hexdump of a subset of one of my packet captures. In this > > capture, all of the packets are part of the same TCP connection, which > > happens to be NFSv3 traffic. All of them except packet number 6 get > > the correct RSS hash and go to the right queue. Packet number 6 (an NFS > rename > > reply with an NFS error) gets RSS hash 0 and goes to queue 0. Whenever I > > repeat this test, the reply to this particular rename attempt always > > goes to the wrong core, though it seemingly differs from the rest of > > the flow only in layers 4-7. > > > > I'll also attach a pcap to this email, in case that's a more > > convenient way to interact with the packets. > > > > -- > > Matt Laswell > > lasw...@infinite.io > > > > > > 16:08:37.093306 IP 10.151.3.81.disclose > 10.151.3.161.nfsd: Flags > > [P.], seq 3173509264:3173509380, ack 3244259549, win 580, options > > [nop,nop,TS val > > 23060466 ecr 490971270], length 116: NFS request xid 2690728524 112 > > access fh > > > Unknown/8B6BFEBB0400CFABD1030100DABC050 > 201 > > 00 > NFS_ACCESS_READ|NFS_ACCESS_LOOKUP|NFS_ACCESS_MODIFY|NFS_ > > ACCESS_EXTEND|NFS_ACCESS_DELETE > > 0x: 4500 00a8 6d0f 4000 4006 b121 0a97 0351 E...m.@.@..!...Q > > 0x0010: 0a97 03a1 029b 0801 bd27 e890 c15f 78dd .'..._x. > > 0x0020: 8018 0244 1cba 0101 080a 015f dff2 ...D._.. > > 0x0030: 1d43 a086 8000 0070 a061 424c .C.p.aBL > > 0x0040: 0002 0001 86a3 0003 0004 > > 0x0050: 0001 0020 0107 8d2f 0007 .../ > > 0x0060: 6573 7869 3275 3100 esxi2u1. > > 0x0070: 0001 > > 0x0080: 0020 8b6b febb 0400 cfab d103 .k.. > > 0x0090: 0100 dabc 0502 > > 0x00a0: 0100 001f > > 16:08:37.095837 IP 10.151.3.161.nfsd > 10.151.3.81.disclose: Flags > > [P.], seq 1:125, ack 116, win 28688, options [nop,nop,TS val 490971270 > > ecr 23060466], length 124: NFS reply xid 2690728524 reply ok 120 > > access c 001f > > 0x: 4500 00b0 1b80 4000 4006 02a9 0a97 03a1 E.@.@... > > 0x0010: 0a97 0351 0801 029b c15f 78dd bd27 e904 ...Q._x..'.. > > 0x0020: 8018 7010 a61a 0101 080a 1d43 a086 ..p..C.. > > 0x0030: 015f dff2 8000 0078 a061 424c 0001 ._.x.aBL > > 0x0040: > > 0x0050: 0001 0002 01ed > > 0x0060: 0003 > > 0x0070: 0029 0800 00ff ...) > > 0x0080: 00ff bbfe 6b8b 0001 ..k. > > 0x0090: 03d1 abcf 5908 f554 3272 e4e6 5908 f554 Y..T2r..Y..T > > 0x00a0: 3272 e4e6 5908 f554 3365 2612 001f 2r..Y..T3e&. > > 16:08:37.096235 IP 10.151.3.81.disclose > 10.151.3.161.nfsd: Flags > > [P.], seq 256:372, ack 285, win 589, options [nop,nop,TS val 23060467 > > ecr 490971270], length 116: NFS request xid 2724282956 112 access fh > > Unknown/ > > > 8B6BFEBB0400D0ABD1030100DABC05020100 > > NFS_ACCESS_READ|NFS_ACCESS_LOOKUP|NFS_ACCESS_MODIFY|NFS_ > > ACCESS_EXTEND|NFS_ACCESS_DELETE > > 0x: 4500 00a8 6d11 4000 4006 b11f 0a97 0351 E...m.@.@..Q > > 0x0010: 0a97 03a1 029b 0801 bd27 e990 c15f 79f9 .'..._y. > > 0x0020: 8018 024d 1cba 0101 080a 015f dff3 ...M._.. > > 0x0030: 1d43 a086 8000 0070 a261 424c .C.p.aBL > > 0x0040: 0002 0001 86a3 0003 0004 > > 0x0050: 0001 0020 0107 8d2f 0007 .../ > > 0x0060: 6573 7869 3275 3100 esxi2u1. > > 0x0070: 0001 > > 0x0080: 0020 8b6b febb 0400 d0ab d103 .k.. > > 0x0090: 0100 dabc 0502 > > 0x00a0: 0100 001f > > 16:08:37.098361 IP 10.151.3.161.nfsd > 10.151.3.81.disclose: Flags > > [P.], seq 285:409, ack 372, win 28688, options [nop,nop,TS val > > 490971270 ecr 23060467], length 124: NFS reply xid 2724282956 reply ok > > 120 access c 001f > > 0x: 4500 00b0 1b81 4000 4006 02a8 0a97 03a1 E.@.@... > > 0x0010: 0a97 0351 0801 029b c15f 79f9 bd27 ea04 ...Q._y..'.. > > 0x0020: 8018 7010 ec45 0101 080a 1d43 a086 ..p..E...C.. > > 0x0030:
Re: [dpdk-dev] [PATCH 1/2] eal/bsd: fix ioport write operation
On Mon, May 08, 2017 at 09:55:01AM +0100, Bruce Richardson wrote:
> On Sun, May 07, 2017 at 01:33:33PM +, Tiwei Bie wrote:
> > The first param of out*() on FreeBSD is port, and the second one is
> > data. But they are reversed in DPDK. This patch fixes it.
> >
> > Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API") Cc:
> > sta...@dpdk.org
> >
> > Signed-off-by: Tiwei Bie ---
> How was this bug discovered so that we can verify that it is fixed?. Is
> this in use by virtio or was it just discovered via code inspection?
>
The virtio PMD in legacy mode doesn't work FreeBSD, and I tried to
fix this issue. And then I found this bug.
Best regards,
Tiwei Bie
Re: [dpdk-dev] [PATCH] eal/bsd: don't zero the pages during mmap in contigmem
On Mon, May 08, 2017 at 09:53:57AM +0100, Bruce Richardson wrote:
> On Mon, May 08, 2017 at 08:09:16AM +, Tiwei Bie wrote:
> > Don't zero the pages during mmap in contigmem. Instead, zero the
> > pages after mmap in primary process. Otherwise, the multi-process
> > support will be broken, as the pages will be zeroed when secondary
> > processes map the memory.
> >
> > Fixes: 82f931805506 ("contigmem: zero all pages during mmap")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Tiwei Bie
> > ---
> I agree there is a problem here, but I'm not sure about the solution to
> it. I still think that the kernel should zero the pages before they get
> given to userspace. Is there any way to keep that working e.g
>
> * have them zeroed on mmap only when they are not already mmaped into
> another process?
> * have them zeroed on init, and again on unmap by the last process to
> have them mapped?
>
I think it's the simplest way to fix it in userspace, so I just did it.
I'd like to fix it in kernel if you also prefer this.
Best regards,
Tiwei Bie
Re: [dpdk-dev] [PATCH] doc: notice for changes in kni structures
On 5/4/2017 10:20 PM, Ferruh Yigit wrote: On 5/3/2017 12:31 PM, Hemant Agrawal wrote: Signed-off-by: Hemant Agrawal --- doc/guides/rel_notes/deprecation.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index a3e7c72..0c1ef2c 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -81,3 +81,10 @@ Deprecation Notices - ``rte_crpytodev_scheduler_mode_get``, replaced by ``rte_cryptodev_scheduler_mode_get`` - ``rte_crpytodev_scheduler_mode_set``, replaced by ``rte_cryptodev_scheduler_mode_set`` + +* kni: additional functionality is planned to be added in kni to support mtu, macaddr, + gso_size, promiscusity configuration. + some of the kni structure will be changed to support additional functionality + e.g ``rte_kni_request`` to support promiscusity`` and mac_addr, rte_kni_request is between KNI library and KNI kernel module, shouldn't be part of API. + ``rte_kni_mbu`` to support the configured gso_size, Again, rte_kni_mbuf should be only concern of KNI kernel module. + ``rte_kni_device_info`` and ``rte_kni_conf`` to also support mtu and macaddr. rte_kni_device_info also between KNI library and KNI kernel module. I think deprecation notice not required for above ones. But you KNI patchset updates rte_kni_conf and rte_kni_ops. These are part of KNI API and changing them cause ABI breakage, but if new fields appended in these structs, this will not cause an ABI breakage, and I think that is better to do instead of deprecation notice, what do you think? I agree. And apart from above ABI issues, adding new fields to "rte_kni_ops" means DPDK application that use KNI should implement them, right? Well, it depend, if the application is interested in this information or not? So this suggest everyone require to set promiscuity of KNI device should implement this. yes! Can't we find another way that all can benefit from a common implementation? how you want it differently? Any ideas? Thanks, ferruh
Re: [dpdk-dev] [PATCH 1/2] eal/bsd: fix ioport write operation
On Mon, May 08, 2017 at 05:07:36PM +0800, Tiwei Bie wrote:
> On Mon, May 08, 2017 at 09:55:01AM +0100, Bruce Richardson wrote:
> > On Sun, May 07, 2017 at 01:33:33PM +, Tiwei Bie wrote:
> > > The first param of out*() on FreeBSD is port, and the second one is
> > > data. But they are reversed in DPDK. This patch fixes it.
> > >
> > > Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API") Cc:
> > > sta...@dpdk.org
> > >
> > > Signed-off-by: Tiwei Bie ---
> > How was this bug discovered so that we can verify that it is fixed?. Is
> > this in use by virtio or was it just discovered via code inspection?
> >
>
> The virtio PMD in legacy mode doesn't work FreeBSD, and I tried to
> fix this issue. And then I found this bug.
>
> Best regards,
> Tiwei Bie
Acked-by: Bruce Richardson
Re: [dpdk-dev] [PATCH 2/2] eal/bsd: fix the read operation on PCI configuration space
On Sun, May 07, 2017 at 01:33:34PM +, Tiwei Bie wrote:
> Some drivers (such as virtio) may need to read more than 4 bytes
> data from PCI configuration space via rte_eal_pci_read_config().
> But it will return with an error on FreeBSD when the expected
> data length is bigger than the size of pi.pi_data whose type is
> u_int32_t. This patch removes this limitation.
>
> Fixes: 632b2d1deeed ("eal: provide functions to access PCI config")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Tiwei Bie
> ---
Acked-by: Bruce Richardson
Re: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration removal
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Slawomir > Mrozowicz > Sent: Friday, May 05, 2017 12:24 PM > To: Doherty, Declan > Cc: dev@dpdk.org; Mrozowicz, SlawomirX > Subject: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration > removal > > Refer to RFC patch - cryptodev: remove crypto device type enumeration > > It is planned to remove device type enumeration rte_cryptodev_type from > library to remove the coupling between crypto PMD and the > librte_cryptodev. > > In this case following stuctures will be changed: rte_cryptodev_session, > rte_cryptodev_sym_session, rte_cryptodev_info, rte_cryptodev. > > It is planned to change the function rte_cryptodev_count_devtype(). > The function prototype doesn’t clearly show the operation. > From next release 17.08 the dev_type will be changed to driver_id. > So the function name will change to > rte_cryptodev_device_count_by_driver(). > > Signed-off-by: Slawomir Mrozowicz Acked-by: Pablo de Lara
Re: [dpdk-dev] [PATCH] doc: announce API changes in crypto library
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Kulasek > Sent: Thursday, May 04, 2017 4:37 PM > To: dev@dpdk.org > Cc: Doherty, Declan > Subject: [dpdk-dev] [PATCH] doc: announce API changes in crypto library > > API changes are planned for 17.08 to made sessions agnostic to the > underlaying devices, removing coupling with crypto PMDs, so a single > session can be used on multiple devices. > > It requires to change "struct rte_cryptodev_sym_session" to store more > than one private data for devices, as well as remove redundant dev_id > and dev_type. > > Effected public functions: > > - rte_cryptodev_sym_session_pool_create > - rte_cryptodev_sym_session_create > - rte_cryptodev_sym_session_free > > While session will not be directly associated with device, followed API > will be changed adding uint8_t dev_id to the argument list: > > - rte_cryptodev_queue_pair_attach_sym_session > - rte_cryptodev_queue_pair_detach_sym_session > > Signed-off-by: Tomasz Kulasek Acked-by: Pablo de Lara
Re: [dpdk-dev] [RFC PATCH 5/5] kni: support multiple userspace process working with kni module
On 5/5/2017 6:38 PM, Ferruh Yigit wrote:
On 5/3/2017 12:21 PM, Hemant Agrawal wrote:
in case of multiple application using the same KNI module,
protect that one application will only clean it's own devices.
Idea looks OK, but there is already a check in the module that prevents
/dev/kni opened by more than one process [1], did you already removed
that limitation? Or is this something else?
[1]
kni_open(...) {
...
if (test_and_set_bit(KNI_DEV_IN_USE_BIT_NUM, &knet->device_in_use))
return -EBUSY;
...
}
Yes! I have removed that. I will send that in next version.
Signed-off-by: Hemant Agrawal
---
<...>
Re: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Slawomir > Mrozowicz > Sent: Thursday, May 04, 2017 10:39 AM > To: Doherty, Declan > Cc: dev@dpdk.org; Mrozowicz, SlawomirX > Subject: [dpdk-dev] [PATCH] doc: announce public crypto PMD names > removal > > The following PMD names definitions will be moved to the individual PMDs > to remove the coupling between crypto PMDs and the librte_cryptodev: > CRYPTODEV_NAME_NULL_PMD > CRYPTODEV_NAME_AESNI_MB_PMD > CRYPTODEV_NAME_AESNI_GCM_PMD > CRYPTODEV_NAME_OPENSSL_PMD > CRYPTODEV_NAME_QAT_SYM_PMD > CRYPTODEV_NAME_SNOW3G_PMD > CRYPTODEV_NAME_KASUMI_PMD > CRYPTODEV_NAME_ZUC_PMD > CRYPTODEV_NAME_ARMV8_PMD > CRYPTODEV_NAME_SCHEDULER_PMD > CRYPTODEV_NAME_DPAA2_SEC_PMD > > Signed-off-by: Slawomir Mrozowicz Acked-by: Pablo de Lara
Re: [dpdk-dev] [RFC PATCH 1/5] kni: change and configure mac address
On 5/5/2017 4:58 PM, Ferruh Yigit wrote:
On 5/3/2017 12:21 PM, Hemant Agrawal wrote:
This patch adds following:
1. option to configure the mac address during create
2. inform usespace, if mac address is being changed in linux
Signed-off-by: Hemant Agrawal
---
.../linuxapp/eal/include/exec-env/rte_kni_common.h| 3 +++
lib/librte_eal/linuxapp/kni/kni_misc.c| 6 +-
lib/librte_eal/linuxapp/kni/kni_net.c | 15 +--
lib/librte_kni/rte_kni.c | 12
lib/librte_kni/rte_kni.h | 8
5 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 2ac879f..e9fdc73 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -80,6 +80,7 @@ enum rte_kni_req_id {
RTE_KNI_REQ_UNKNOWN = 0,
RTE_KNI_REQ_CHANGE_MTU,
RTE_KNI_REQ_CFG_NETWORK_IF,
+ RTE_KNI_REQ_CHANGE_MAC_ADDR,
RTE_KNI_REQ_MAX,
};
@@ -92,6 +93,7 @@ struct rte_kni_request {
union {
uint32_t new_mtu;/**< New MTU */
uint8_t if_up; /**< 1: interface up, 0: interface down */
+ uint8_t mac_addr[6]; /**< MAC address for interface */
};
int32_t result; /**< Result for processing request */
} __attribute__((__packed__));
@@ -168,6 +170,7 @@ struct rte_kni_device_info {
/* mbuf size */
unsigned mbuf_size;
+ char macaddr[6]; /**< Mac Address */
I think it is good to use same variable name for same reason, above uses
"mac_addr" here "macaddr", please pick one.
And perhaps field comment can be removed, it is not adding any extra
information.
ok
};
#define KNI_DEVICE "kni"
diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 7590f1f..90879fa 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -458,12 +458,16 @@ struct kni_net {
if (kni->lad_dev)
ether_addr_copy(net_dev->dev_addr, kni->lad_dev->dev_addr);
- else
+ else {
/*
* Generate random mac address. eth_random_addr() is the newer
* version of generating mac address in linux kernel.
*/
random_ether_addr(net_dev->dev_addr);
+
+ /* todo - check if user supplied mac address is available*/
Can we implement todo J
I am just thinking about how to check, if user has provided the mac
address or he want the random address.
Can we make it mandatory for user to supply the mac address always?
+ memcpy(net_dev->dev_addr, dev_info.macaddr, ETH_ALEN);
+ }
ret = register_netdev(net_dev);
if (ret) {
diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
b/lib/librte_eal/linuxapp/kni/kni_net.c
index db9f489..866cbdd 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -668,12 +668,23 @@
static int
kni_net_set_mac(struct net_device *netdev, void *p)
{
+ int ret;
+ struct rte_kni_request req;
struct sockaddr *addr = p;
+ struct kni_dev *kni;
+
+ kni = netdev_priv(netdev);
+ memset(&req, 0, sizeof(req));
+ req.req_id = RTE_KNI_REQ_CHANGE_MAC_ADDR;
- if (!is_valid_ether_addr((unsigned char *)(addr->sa_data)))
+ if (!is_valid_ether_addr((unsigned char *)(addr->sa_data))) {
Not required to add {
ok
return -EADDRNOTAVAIL;
+ }
+ memcpy(req.mac_addr, addr->sa_data, netdev->addr_len);
memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
- return 0;
+ ret = kni_net_process_request(kni, &req);
+
+ return (ret == 0 ? req.result : ret);
If config_mac_address() is not defined, req.result will be undefined.
You should give initial value to req.
I am doing a memset to req. so req.result will be '0'.
}
#ifdef HAVE_CHANGE_CARRIER_CB
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 52fcd4b..d5a717b 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -52,6 +52,10 @@
#define MAX_MBUF_BURST_NUM32
+#ifndef ETH_ADDR_LEN
+#define ETH_ADDR_LEN 6
+#endif
This is redundant, rte_kni.h has something similar.
ok
+
/* Maximum number of ring entries */
#define KNI_FIFO_COUNT_MAX 1024
#define KNI_FIFO_SIZE (KNI_FIFO_COUNT_MAX * sizeof(void *) + \
@@ -368,6 +372,8 @@ struct rte_kni *
dev_info.group_id = conf->group_id;
dev_info.mbuf_size = conf->mbuf_size;
+ memcpy(dev_info.macaddr, conf->macaddr, ETH_ADDR_LEN);
+
snprintf(ctx->name, RTE_KNI_NA
Re: [dpdk-dev] [PATCH] doc: notice for changes in kni structures
On 5/3/2017 9:20 PM, Stephen Hemminger wrote: On Wed, 3 May 2017 17:01:31 +0530 Hemant Agrawal wrote: Signed-off-by: Hemant Agrawal --- doc/guides/rel_notes/deprecation.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index a3e7c72..0c1ef2c 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -81,3 +81,10 @@ Deprecation Notices - ``rte_crpytodev_scheduler_mode_get``, replaced by ``rte_cryptodev_scheduler_mode_get`` - ``rte_crpytodev_scheduler_mode_set``, replaced by ``rte_cryptodev_scheduler_mode_set`` + +* kni: additional functionality is planned to be added in kni to support mtu, macaddr, + gso_size, promiscusity configuration. + some of the kni structure will be changed to support additional functionality + e.g ``rte_kni_request`` to support promiscusity`` and mac_addr, + ``rte_kni_mbu`` to support the configured gso_size, + ``rte_kni_device_info`` and ``rte_kni_conf`` to also support mtu and macaddr. Why are these exposed as something applications should care about? The data structures in rte_kni_common.h are not an API Agreed. This is not needed.
Re: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration removal
On 05/05/2017 12:24 PM, Slawomir Mrozowicz wrote: Refer to RFC patch - cryptodev: remove crypto device type enumeration It is planned to remove device type enumeration rte_cryptodev_type from library to remove the coupling between crypto PMD and the librte_cryptodev. In this case following stuctures will be changed: rte_cryptodev_session, rte_cryptodev_sym_session, rte_cryptodev_info, rte_cryptodev. It is planned to change the function rte_cryptodev_count_devtype(). The function prototype doesn’t clearly show the operation. From next release 17.08 the dev_type will be changed to driver_id. So the function name will change to rte_cryptodev_device_count_by_driver(). Signed-off-by: Slawomir Mrozowicz --- ... Acked-by: Declan Doherty
Re: [dpdk-dev] [RFC] cryptodev: remove crypto device type enumeration
On 05/05/2017 4:37 PM, Declan Doherty wrote: On 05/05/2017 12:07 PM, Slawomir Mrozowicz wrote: This RFC changes device type identification to be based on a unique driver id replacing the current device type enumeration, which needed library changes every time a new crypto driver was added. The driver id is assigned dynamically during driver registration using the new macro RTE_PMD_REGISTER_CRYPTO_DRIVER which returns a unique uint8_t identifier for that driver. New APIs are also introduced to allow retrieval of the driver id using the driver name. Library code is included in the RFC (implementation in progress, API complete). Signed-off-by: Slawomir Mrozowicz --- Acked-by: Declan Doherty sorry please ignore this ack, it was meant for the doc announce patch regarding the same.
[dpdk-dev] cryptodev: proposed changes for 17.08
Hey folks, as maintainers of crypto PMDs I just want to draw your attention to the following doc announcements and RFCs patches for proposed changes to the cryptodev library in 17.08. - removal of PMD names from librte_cryptodev announce - http://dpdk.org/dev/patchwork/patch/24084/ - removal of crypto device type enumerations announce - http://dpdk.org/dev/patchwork/patch/24120/ rfc - http://dpdk.org/dev/patchwork/patch/24119/ - changes to crypto sessions to make PMD agnostic announce - http://dpdk.org/dev/patchwork/patch/24102/ rfc - http://dpdk.org/dev/patchwork/patch/24091/ - changes to crypto op layout announce - http://dpdk.org/dev/patchwork/patch/24012/ rfc - http://dpdk.org/dev/patchwork/patch/24011/ It would be great if you could spend some time to review these, and if possible ack the announce patches and provide feedback to the RFC threads. Thanks Declan
Re: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal
-Original Message- > Date: Thu, 4 May 2017 11:39:19 +0200 > From: Slawomir Mrozowicz > To: declan.dohe...@intel.com > CC: dev@dpdk.org, Slawomir Mrozowicz > Subject: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal > X-Mailer: git-send-email 2.9.3 > > The following PMD names definitions will be moved to the individual PMDs > to remove the coupling between crypto PMDs and the librte_cryptodev: > CRYPTODEV_NAME_NULL_PMD > CRYPTODEV_NAME_AESNI_MB_PMD > CRYPTODEV_NAME_AESNI_GCM_PMD > CRYPTODEV_NAME_OPENSSL_PMD > CRYPTODEV_NAME_QAT_SYM_PMD > CRYPTODEV_NAME_SNOW3G_PMD > CRYPTODEV_NAME_KASUMI_PMD > CRYPTODEV_NAME_ZUC_PMD > CRYPTODEV_NAME_ARMV8_PMD > CRYPTODEV_NAME_SCHEDULER_PMD > CRYPTODEV_NAME_DPAA2_SEC_PMD > > Signed-off-by: Slawomir Mrozowicz Acked-by: Jerin Jacob
[dpdk-dev] [PATCH] net/ixgbe: fix LSC interrupt issue
There is a bug in previous fix for lsc interrupt.
lsc interrupt is not disabled before delayed handler,
that cause the delayed handler be re-entered.
Fixes: 9b667210700e ("net/ixgbe: fix blocked interrupts")
Cc: sta...@dpdk.org
Signed-off-by: Qi Zhang
---
drivers/net/ixgbe/ixgbe_ethdev.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ec667d8..c680aab 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4107,14 +4107,15 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
timeout = IXGBE_LINK_DOWN_CHECK_TIMEOUT;
ixgbe_dev_link_status_print(dev);
- intr->mask_original = intr->mask;
- /* only disable lsc interrupt */
- intr->mask &= ~IXGBE_EIMS_LSC;
if (rte_eal_alarm_set(timeout * 1000,
ixgbe_dev_interrupt_delayed_handler,
(void *)dev) < 0)
PMD_DRV_LOG(ERR, "Error setting alarm");
- else
- intr->mask = intr->mask_original;
+ else {
+ /* remember orignal mask */
+ intr->mask_original = intr->mask;
+ /* only disable lsc interrupt */
+ intr->mask &= ~IXGBE_EIMS_LSC;
+ }
}
PMD_DRV_LOG(DEBUG, "enable intr immediately");
--
2.7.4
Re: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration removal
-Original Message- > Date: Fri, 5 May 2017 13:24:19 +0200 > From: Slawomir Mrozowicz > To: declan.dohe...@intel.com > CC: dev@dpdk.org, Slawomir Mrozowicz > Subject: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration > removal > X-Mailer: git-send-email 2.9.3 > > Refer to RFC patch - cryptodev: remove crypto device type enumeration > > It is planned to remove device type enumeration rte_cryptodev_type from > library to remove the coupling between crypto PMD and the librte_cryptodev. > > In this case following stuctures will be changed: rte_cryptodev_session, > rte_cryptodev_sym_session, rte_cryptodev_info, rte_cryptodev. > > It is planned to change the function rte_cryptodev_count_devtype(). > The function prototype doesn’t clearly show the operation. > From next release 17.08 the dev_type will be changed to driver_id. > So the function name will change to rte_cryptodev_device_count_by_driver(). > > Signed-off-by: Slawomir Mrozowicz Acked-by: Jerin Jacob
Re: [dpdk-dev] [PATCH] net/ixgbe: fix LSC interrupt issue
Hi Thomas,
This is fix for critical issues exposed by Gaetan Rivet's
commit(8ea656f8c300175ac84f3cbe1117f5ef11ffc4eb) to enable LSC.
This fix two issues. One is "VF testpmd can't set up successfully on Niantic
nic.", another is " kernel VF sometimes can't link up successfully when test
DPDK PF+kernel VF".
The fix is verified and code review show it is low risk.
It would be great if it can be released with DPDK 17.05.
Thanks & Regards,
Yu Liu
-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Qi Zhang
Sent: Monday, May 8, 2017 11:58 AM
To: Zhang, Helin ; Lu, Wenzhuo
Cc: dev@dpdk.org; Zhang, Qi Z ; sta...@dpdk.org
Subject: [dpdk-dev] [PATCH] net/ixgbe: fix LSC interrupt issue
There is a bug in previous fix for lsc interrupt.
lsc interrupt is not disabled before delayed handler, that cause the delayed
handler be re-entered.
Fixes: 9b667210700e ("net/ixgbe: fix blocked interrupts")
Cc: sta...@dpdk.org
Signed-off-by: Qi Zhang
---
drivers/net/ixgbe/ixgbe_ethdev.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ec667d8..c680aab 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4107,14 +4107,15 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
timeout = IXGBE_LINK_DOWN_CHECK_TIMEOUT;
ixgbe_dev_link_status_print(dev);
- intr->mask_original = intr->mask;
- /* only disable lsc interrupt */
- intr->mask &= ~IXGBE_EIMS_LSC;
if (rte_eal_alarm_set(timeout * 1000,
ixgbe_dev_interrupt_delayed_handler,
(void *)dev) < 0)
PMD_DRV_LOG(ERR, "Error setting alarm");
- else
- intr->mask = intr->mask_original;
+ else {
+ /* remember orignal mask */
+ intr->mask_original = intr->mask;
+ /* only disable lsc interrupt */
+ intr->mask &= ~IXGBE_EIMS_LSC;
+ }
}
PMD_DRV_LOG(DEBUG, "enable intr immediately");
--
2.7.4
Re: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jerin Jacob > Sent: Monday, May 8, 2017 12:02 PM > To: Mrozowicz, SlawomirX > Cc: Doherty, Declan ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] doc: announce public crypto PMD names > removal > > -Original Message- > > Date: Thu, 4 May 2017 11:39:19 +0200 > > From: Slawomir Mrozowicz > > To: declan.dohe...@intel.com > > CC: dev@dpdk.org, Slawomir Mrozowicz > > Subject: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal > > X-Mailer: git-send-email 2.9.3 > > > > The following PMD names definitions will be moved to the individual PMDs > > to remove the coupling between crypto PMDs and the librte_cryptodev: > > CRYPTODEV_NAME_NULL_PMD > > CRYPTODEV_NAME_AESNI_MB_PMD > > CRYPTODEV_NAME_AESNI_GCM_PMD > > CRYPTODEV_NAME_OPENSSL_PMD > > CRYPTODEV_NAME_QAT_SYM_PMD > > CRYPTODEV_NAME_SNOW3G_PMD > > CRYPTODEV_NAME_KASUMI_PMD > > CRYPTODEV_NAME_ZUC_PMD > > CRYPTODEV_NAME_ARMV8_PMD > > CRYPTODEV_NAME_SCHEDULER_PMD > > CRYPTODEV_NAME_DPAA2_SEC_PMD > > > > Signed-off-by: Slawomir Mrozowicz > > Acked-by: Jerin Jacob Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration removal
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jerin Jacob > Sent: Monday, May 8, 2017 12:05 PM > To: Mrozowicz, SlawomirX > Cc: Doherty, Declan ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration > removal > > -Original Message- > > Date: Fri, 5 May 2017 13:24:19 +0200 > > From: Slawomir Mrozowicz > > To: declan.dohe...@intel.com > > CC: dev@dpdk.org, Slawomir Mrozowicz > > Subject: [dpdk-dev] [PATCH] doc: announce crypto device type enumeration > > removal > > X-Mailer: git-send-email 2.9.3 > > > > Refer to RFC patch - cryptodev: remove crypto device type enumeration > > > > It is planned to remove device type enumeration rte_cryptodev_type from > > library to remove the coupling between crypto PMD and the librte_cryptodev. > > > > In this case following stuctures will be changed: rte_cryptodev_session, > > rte_cryptodev_sym_session, rte_cryptodev_info, rte_cryptodev. > > > > It is planned to change the function rte_cryptodev_count_devtype(). > > The function prototype doesn’t clearly show the operation. > > From next release 17.08 the dev_type will be changed to driver_id. > > So the function name will change to rte_cryptodev_device_count_by_driver(). > > > > Signed-off-by: Slawomir Mrozowicz > > Acked-by: Jerin Jacob Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH] doc: announce API changes in crypto library
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Kulasek > Sent: Thursday, May 4, 2017 4:37 PM > To: dev@dpdk.org > Cc: Doherty, Declan > Subject: [dpdk-dev] [PATCH] doc: announce API changes in crypto library > > API changes are planned for 17.08 to made sessions agnostic to the > underlaying devices, removing coupling with crypto PMDs, so a single > session can be used on multiple devices. > > It requires to change "struct rte_cryptodev_sym_session" to store more > than one private data for devices, as well as remove redundant dev_id > and dev_type. > > Effected public functions: > > - rte_cryptodev_sym_session_pool_create > - rte_cryptodev_sym_session_create > - rte_cryptodev_sym_session_free > > While session will not be directly associated with device, followed API > will be changed adding uint8_t dev_id to the argument list: > > - rte_cryptodev_queue_pair_attach_sym_session > - rte_cryptodev_queue_pair_detach_sym_session > > Signed-off-by: Tomasz Kulasek Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH] doc: announce crypto structures rework
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Declan Doherty > Sent: Friday, May 5, 2017 4:34 PM > To: De Lara Guarch, Pablo ; dev@dpdk.org > Cc: akhil.go...@nxp.com; hemant.agra...@nxp.com; > zbigniew.bo...@caviumnetworks.com; jerin.ja...@caviumnetworks.com > Subject: Re: [dpdk-dev] [PATCH] doc: announce crypto structures rework > > On 28/04/2017 7:06 PM, De Lara Guarch, Pablo wrote: > > The current crypto operation and symmetric crypto operation > > structures will be reworked for correctness and improvement, > > reducing also their sizes, to fit into less cache lines, > > as stated in the following RFC: > > > > http://dpdk.org/dev/patchwork/patch/24011/ > > > > Signed-off-by: Pablo de Lara > > --- > > doc/guides/rel_notes/deprecation.rst | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/doc/guides/rel_notes/deprecation.rst > b/doc/guides/rel_notes/deprecation.rst > > index a3e7c72..6045557 100644 > > --- a/doc/guides/rel_notes/deprecation.rst > > +++ b/doc/guides/rel_notes/deprecation.rst > > @@ -81,3 +81,6 @@ Deprecation Notices > > > >- ``rte_crpytodev_scheduler_mode_get``, replaced by > ``rte_cryptodev_scheduler_mode_get`` > >- ``rte_crpytodev_scheduler_mode_set``, replaced by > ``rte_cryptodev_scheduler_mode_set`` > > + > > +* cryptodev: the structures ``rte_crypto_op``, ``rte_crypto_sym_op`` and > ``rte_crypto_sym_xform`` > > + will be restructured in 17.08, for correctness and improvement. > > > > Acked-by: Declan Doherty Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH] doc: announce crypto structures rework
On 5/8/2017 7:26 PM, Trahe, Fiona wrote: -Original Message- From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Declan Doherty Sent: Friday, May 5, 2017 4:34 PM To: De Lara Guarch, Pablo ; dev@dpdk.org Cc: akhil.go...@nxp.com; hemant.agra...@nxp.com; zbigniew.bo...@caviumnetworks.com; jerin.ja...@caviumnetworks.com Subject: Re: [dpdk-dev] [PATCH] doc: announce crypto structures rework On 28/04/2017 7:06 PM, De Lara Guarch, Pablo wrote: The current crypto operation and symmetric crypto operation structures will be reworked for correctness and improvement, reducing also their sizes, to fit into less cache lines, as stated in the following RFC: http://dpdk.org/dev/patchwork/patch/24011/ Signed-off-by: Pablo de Lara --- doc/guides/rel_notes/deprecation.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index a3e7c72..6045557 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -81,3 +81,6 @@ Deprecation Notices - ``rte_crpytodev_scheduler_mode_get``, replaced by ``rte_cryptodev_scheduler_mode_get`` - ``rte_crpytodev_scheduler_mode_set``, replaced by ``rte_cryptodev_scheduler_mode_set`` + +* cryptodev: the structures ``rte_crypto_op``, ``rte_crypto_sym_op`` and ``rte_crypto_sym_xform`` + will be restructured in 17.08, for correctness and improvement. Acked-by: Declan Doherty Acked-by: Fiona Trahe Acked-by: Hemant Agrawal
Re: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal
On 5/8/2017 6:38 PM, Trahe, Fiona wrote: -Original Message- From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jerin Jacob Sent: Monday, May 8, 2017 12:02 PM To: Mrozowicz, SlawomirX Cc: Doherty, Declan ; dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal -Original Message- Date: Thu, 4 May 2017 11:39:19 +0200 From: Slawomir Mrozowicz To: declan.dohe...@intel.com CC: dev@dpdk.org, Slawomir Mrozowicz Subject: [dpdk-dev] [PATCH] doc: announce public crypto PMD names removal X-Mailer: git-send-email 2.9.3 The following PMD names definitions will be moved to the individual PMDs to remove the coupling between crypto PMDs and the librte_cryptodev: CRYPTODEV_NAME_NULL_PMD CRYPTODEV_NAME_AESNI_MB_PMD CRYPTODEV_NAME_AESNI_GCM_PMD CRYPTODEV_NAME_OPENSSL_PMD CRYPTODEV_NAME_QAT_SYM_PMD CRYPTODEV_NAME_SNOW3G_PMD CRYPTODEV_NAME_KASUMI_PMD CRYPTODEV_NAME_ZUC_PMD CRYPTODEV_NAME_ARMV8_PMD CRYPTODEV_NAME_SCHEDULER_PMD CRYPTODEV_NAME_DPAA2_SEC_PMD Signed-off-by: Slawomir Mrozowicz Acked-by: Jerin Jacob Acked-by: Fiona Trahe Acked-by: Hemant Agrawal
Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several devices per vfio group
-Original Message-
> Date: Sun, 30 Apr 2017 19:29:49 +0200
> From: Thomas Monjalon
> To: Alejandro Lucero
> Cc: dev@dpdk.org, "Burakov, Anatoly"
> Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
> devices per vfio group
>
> 28/04/2017 15:25, Burakov, Anatoly:
> > From: Alejandro Lucero [mailto:alejandro.luc...@netronome.com]
> > > VFIO allows a secure way of assigning devices to user space and those
> > > devices which can not be isolated from other ones are set in same VFIO
> > > group. Releasing or unplugging a device should be aware of remaining
> > > devices is the same group for avoiding to close such a group.
> > >
> > > Fixes: 94c0776b1bad ("vfio: support hotplug")
> > >
> > > Signed-off-by: Alejandro Lucero
> >
> > I have tested this on my setup on an old kernel with multiple
> > attach/detaches, and it works (whereas it fails without this patch).
> >
> > Acked-by: Anatoly Burakov
>
> Applied, thanks
This patch creates issue when large number of PCIe devices connected to system.
Found it through git bisect.
This issue is, vfio_group_fd goes beyond 64(VFIO_MAX_GROUPS) and writes
to wrong memory on following code execution and sub sequentially creates
issues in vfio mapping or such.
vfio_cfg.vfio_groups[vfio_group_fd].devices++;
I can increase VFIO_MAX_GROUPS, but I think, it is not correct fix as
vfio_group_fd generated from open system call.
I add some prints the code for debug. Please find below the output.
Any thoughts from VFIO experts?
➜ [master]83xx [dpdk-master] $ git diff
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index d3eae20..2d8ee4c 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -100,6 +100,7 @@ vfio_get_group_fd(int iommu_group_no)
snprintf(filename, sizeof(filename),
VFIO_GROUP_FMT, iommu_group_no);
vfio_group_fd = open(filename, O_RDWR);
+ printf("## name %s vfio_group_fd %d\n", filename,
vfio_group_fd);
if (vfio_group_fd < 0) {
/* if file not found, it's not an error */
if (errno != ENOENT) {
@@ -259,6 +260,8 @@ vfio_setup_device(const char *sysfs_base, const char
*dev_addr,
if (vfio_group_fd < 0)
return -1;
+ printf(" iommu_group_fd %d vfio_group_fd=%d\n", iommu_group_no,
vfio_group_fd);
+
/* if group_fd == 0, that means the device isn't managed by VFIO
* */
if (vfio_group_fd == 0) {
RTE_LOG(WARNING, EAL, " %s not managed by VFIO driver,
skipping\n",
@@ -266,6 +269,7 @@ vfio_setup_device(const char *sysfs_base, const char
*dev_addr,
return 1;
}
/*
* at this point, we know that this group is viable (meaning,
* all devices
* are either bound to VFIO or not bound to anything)
@@ -359,6 +363,7 @@ vfio_setup_device(const char *sysfs_base, const char
*dev_addr,
return -1;
}
vfio_cfg.vfio_groups[vfio_group_fd].devices++;
+ printf("vfio_group_fd %d device %d\n", vfio_group_fd,
vfio_cfg.vfio_groups[vfio_group_fd].devices++);
return 0;
}
output log
--
EAL: PCI device :07:00.1 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/114 vfio_group_fd 44
iommu_group_fd 114 vfio_group_fd=44
EAL: using IOMMU type 1 (Type 1)
vfio_group_fd 44 device 1
EAL: PCI device :07:00.2 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/115 vfio_group_fd 47
iommu_group_fd 115 vfio_group_fd=47
vfio_group_fd 47 device 1
EAL: PCI device :07:00.3 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/116 vfio_group_fd 50
iommu_group_fd 116 vfio_group_fd=50
vfio_group_fd 50 device 1
EAL: PCI device :07:00.4 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/117 vfio_group_fd 53
iommu_group_fd 117 vfio_group_fd=53
vfio_group_fd 53 device 1
EAL: PCI device :07:00.5 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/118 vfio_group_fd 56
iommu_group_fd 118 vfio_group_fd=56
vfio_group_fd 56 device 1
EAL: PCI device :07:00.6 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/119 vfio_group_fd 59
iommu_group_fd 119 vfio_group_fd=59
vfio_group_fd 59 device 1
EAL: PCI device :07:00.7 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/120 vfio_group_fd 62
iommu_group_fd 120 vfio_group_fd=62
vfio_group_fd 62 device 1
EAL: PCI device :07:01.0 on NUMA socket 0
EAL: probe driver: 177d:a04b octeontx_ssovf
## name /dev/vfio/121 vfio_group_fd 65
iommu_group_fd 121 vfio_group_fd=65
vfio_group_fd 65 device 1632632833
[dpdk-dev] Fw: New Defects reported by Coverity Scan for DPDK Data Plane Development Kit
Lots of new warnings. Most of them from the ARK driver.
Begin forwarded message:
Date: Mon, 08 May 2017 03:17:22 -0700
From: scan-ad...@coverity.com
To: step...@networkplumber.org
Subject: New Defects reported by Coverity Scan for DPDK Data Plane Development
Kit
Hi,
Please find the latest report on new defect(s) introduced to DPDK Data Plane
Development Kit found with Coverity Scan.
15 new defect(s) introduced to DPDK Data Plane Development Kit found with
Coverity Scan.
26 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 15 of 15 defect(s)
** CID 144526: Incorrect expression (USELESS_CALL)
*** CID 144526: Incorrect expression (USELESS_CALL)
/drivers/net/bonding/rte_eth_bond_pmd.c: 2486 in bond_remove()
2480
2481eth_dev->dev_ops = NULL;
2482eth_dev->rx_pkt_burst = NULL;
2483eth_dev->tx_pkt_burst = NULL;
2484
2485internals = eth_dev->data->dev_private;
>>> CID 144526: Incorrect expression (USELESS_CALL)
>>> Calling "rte_bitmap_free(internals->vlan_filter_bmp)" is only useful
>>> for its return value, which is ignored.
2486rte_bitmap_free(internals->vlan_filter_bmp);
2487rte_free(internals->vlan_filter_bmpmem);
2488rte_free(eth_dev->data->dev_private);
2489rte_free(eth_dev->data->mac_addrs);
2490
2491rte_eth_dev_release_port(eth_dev);
** CID 144525: Control flow issues (UNREACHABLE)
/usr/src/kernels/4.8.10-200.fc24.x86_64/arch/x86/include/asm/jump_label.h: 60
in arch_static_branch_jump()
*** CID 144525: Control flow issues (UNREACHABLE)
/usr/src/kernels/4.8.10-200.fc24.x86_64/arch/x86/include/asm/jump_label.h: 60
in arch_static_branch_jump()
54 _ASM_ALIGN "\n\t"
55 _ASM_PTR "1b, %l[l_yes], %c0 + %c1 \n\t"
56 ".popsection \n\t"
57 : : "i" (key), "i" (branch) : : l_yes);
58
59 return false;
>>> CID 144525: Control flow issues (UNREACHABLE)
>>> This code cannot be reached: "l_yes:
return true;".
60 l_yes:
61 return true;
62 }
63
64 #ifdef CONFIG_X86_64
65 typedef u64 jump_label_t;
** CID 144524: Insecure data handling (TAINTED_STRING)
/drivers/net/ark/ark_ethdev.c: 199 in check_for_ext()
*** CID 144524: Insecure data handling (TAINTED_STRING)
/drivers/net/ark/ark_ethdev.c: 199 in check_for_ext()
193 PMD_DEBUG_LOG(DEBUG, "ARK EXT NO dll path specified\n");
194 return 0;
195 }
196 PMD_DRV_LOG(INFO, "ARK EXT found dll path at %s\n", dllpath);
197
198 /* Open and load the .so */
>>> CID 144524: Insecure data handling (TAINTED_STRING)
>>> Passing tainted string "dllpath" to "dlopen", which cannot accept
>>> tainted data.
199 ark->d_handle = dlopen(dllpath, RTLD_LOCAL | RTLD_LAZY);
200 if (ark->d_handle == NULL) {
201 PMD_DRV_LOG(ERR, "Could not load user extension %s\n",
202 dllpath);
203 return -1;
204 }
** CID 144523: Code maintainability issues (SIZEOF_MISMATCH)
/app/proc_info/main.c: 489 in nic_xstats_display()
*** CID 144523: Code maintainability issues (SIZEOF_MISMATCH)
/app/proc_info/main.c: 489 in nic_xstats_display()
483
484 len = rte_eth_xstats_get_names_by_id(port_id, NULL, 0, NULL);
485 if (len < 0) {
486 printf("Cannot get xstats count\n");
487 return;
488 }
>>> CID 144523: Code maintainability issues (SIZEOF_MISMATCH)
>>> Passing argument "8UL /* sizeof (values) */ * len" to function "malloc"
>>> and then casting the return value to "uint64_t *" is suspicious. In this
>>> particular case "sizeof (uint64_t *)" happens to be equal to "sizeof
>>> (uint64_t)", but this is not a portable assumption.
489 values = malloc(sizeof(values) * len);
490 if (values == NULL) {
491 printf("Cannot allocate memory for xstats\n");
492 return;
493 }
494
** CID 144522: Code maintainability issues (SIZEOF_MISMATCH)
/lib/librte_ether/rte_ethdev.c: 1717 in rte_eth_xstats_get_by_id()
*** CID 144522: Code maintainability issues (SIZEOF_MISMATCH)
/lib/l
Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several devices per vfio group
Hi Jerin,
On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob
wrote:
> -Original Message-
> > Date: Sun, 30 Apr 2017 19:29:49 +0200
> > From: Thomas Monjalon
> > To: Alejandro Lucero
> > Cc: dev@dpdk.org, "Burakov, Anatoly"
> > Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
> > devices per vfio group
> >
> > 28/04/2017 15:25, Burakov, Anatoly:
> > > From: Alejandro Lucero [mailto:alejandro.luc...@netronome.com]
> > > > VFIO allows a secure way of assigning devices to user space and those
> > > > devices which can not be isolated from other ones are set in same
> VFIO
> > > > group. Releasing or unplugging a device should be aware of remaining
> > > > devices is the same group for avoiding to close such a group.
> > > >
> > > > Fixes: 94c0776b1bad ("vfio: support hotplug")
> > > >
> > > > Signed-off-by: Alejandro Lucero
> > >
> > > I have tested this on my setup on an old kernel with multiple
> attach/detaches, and it works (whereas it fails without this patch).
> > >
> > > Acked-by: Anatoly Burakov
> >
> > Applied, thanks
>
> This patch creates issue when large number of PCIe devices connected to
> system.
> Found it through git bisect.
>
> This issue is, vfio_group_fd goes beyond 64(VFIO_MAX_GROUPS) and writes
> to wrong memory on following code execution and sub sequentially creates
> issues in vfio mapping or such.
>
vfio_cfg.vfio_groups[vfio_group_fd].devices++;
>
> I can increase VFIO_MAX_GROUPS, but I think, it is not correct fix as
> vfio_group_fd generated from open system call.
>
> I add some prints the code for debug. Please find below the output.
> Any thoughts from VFIO experts?
>
>
That is a silly but serious bug. We are using the file descriptor as the
index for updating devices counter of a vfio group structure internal to
DPDK VFIO code. We should be using the vfio_group that file descriptor is
registered with.
I will send a fix where vfio_group_device_get/put/count functions are
implemented which take the file descriptor as a parameter and then go
through the vfio_group array for working with the right one.
Thomas, is this fix in time yet for 17.05? I will send the patch today but
I can just test it against a system with the "normal" case for VFIO device
groups. Maybe Jerin or/and Anatoly can test it against the other case.
> ➜ [master]83xx [dpdk-master] $ git diff
> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> index d3eae20..2d8ee4c 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> @@ -100,6 +100,7 @@ vfio_get_group_fd(int iommu_group_no)
> snprintf(filename, sizeof(filename),
> VFIO_GROUP_FMT, iommu_group_no);
> vfio_group_fd = open(filename, O_RDWR);
> + printf("## name %s vfio_group_fd %d\n", filename,
> vfio_group_fd);
> if (vfio_group_fd < 0) {
> /* if file not found, it's not an error */
> if (errno != ENOENT) {
> @@ -259,6 +260,8 @@ vfio_setup_device(const char *sysfs_base, const char
> *dev_addr,
> if (vfio_group_fd < 0)
> return -1;
>
> + printf(" iommu_group_fd %d vfio_group_fd=%d\n",
> iommu_group_no, vfio_group_fd);
> +
> /* if group_fd == 0, that means the device isn't managed by VFIO
> * */
> if (vfio_group_fd == 0) {
> RTE_LOG(WARNING, EAL, " %s not managed by VFIO driver,
> skipping\n",
> @@ -266,6 +269,7 @@ vfio_setup_device(const char *sysfs_base, const char
> *dev_addr,
> return 1;
> }
> /*
> * at this point, we know that this group is viable (meaning,
> * all devices
> * are either bound to VFIO or not bound to anything)
> @@ -359,6 +363,7 @@ vfio_setup_device(const char *sysfs_base, const char
> *dev_addr,
> return -1;
> }
> vfio_cfg.vfio_groups[vfio_group_fd].devices++;
> + printf("vfio_group_fd %d device %d\n", vfio_group_fd,
> vfio_cfg.vfio_groups[vfio_group_fd].devices++);
>
> return 0;
> }
>
>
> output log
> --
>
> EAL: PCI device :07:00.1 on NUMA socket 0
> EAL: probe driver: 177d:a04b octeontx_ssovf
> ## name /dev/vfio/114 vfio_group_fd 44
> iommu_group_fd 114 vfio_group_fd=44
> EAL: using IOMMU type 1 (Type 1)
> vfio_group_fd 44 device 1
> EAL: PCI device :07:00.2 on NUMA socket 0
> EAL: probe driver: 177d:a04b octeontx_ssovf
> ## name /dev/vfio/115 vfio_group_fd 47
> iommu_group_fd 115 vfio_group_fd=47
> vfio_group_fd 47 device 1
> EAL: PCI device :07:00.3 on NUMA socket 0
> EAL: probe driver: 177d:a04b octeontx_ssovf
> ## name /dev/vfio/116 vfio_group_fd 50
> iommu_group_fd 116 vfio_group_fd=50
> vfio_group_fd 50 device 1
> EAL: PCI device :07:00.4 on NUMA socket 0
> EAL: probe driver: 177d:a04b octeontx_ssovf
> ## name /dev/vfio/117 vfio_gro
Re: [dpdk-dev] [PATCH v2] app/testpmd: support non contiguous socket ids
--Shahaf
> -Original Message-
> From: Wu, Jingjing [mailto:jingjing...@intel.com]
> Sent: Monday, May 8, 2017 3:54 AM
> To: Shahaf Shuler
> Cc: dev@dpdk.org; Thomas Monjalon ;
> sta...@dpdk.org
> Subject: RE: [PATCH v2] app/testpmd: support non contiguous socket ids
>
>
>
> > -Original Message-
> > From: Shahaf Shuler [mailto:shah...@mellanox.com]
> > Sent: Sunday, May 7, 2017 2:06 PM
> > To: Wu, Jingjing
> > Cc: dev@dpdk.org; Thomas Monjalon ;
> > sta...@dpdk.org
> > Subject: RE: [PATCH v2] app/testpmd: support non contiguous socket ids
> >
> > Saturday, May 6, 2017 4:41 AM, Wu, Jingjing:
> > > >
> > > > The test assumes the socket ids are contiguous. This is not
> > > > necessarily the case on all servers and may cause mempool creation to
> fail.
> > > >
> > > > Fixing it by detecting the list of valid socket ids and use it for
> > > > the mempool creation.
> > > >
> > > > Fixes: 7acf894d07d1 ("app/testpmd: detect numa socket count")
> > > >
> > > > CC: sta...@dpdk.org
> > > > Signed-off-by: Shahaf Shuler
> > > > ---
> > > > on v2:
> > > > * fix minor typo on commit message : be->by.
> > > > ---
> > > > app/test-pmd/parameters.c | 38 -
>
> > > -
> > > > app/test-pmd/testpmd.c| 38 +-
> -
> > > ---
> > > > app/test-pmd/testpmd.h| 4 +++-
> > > > 3 files changed, 60 insertions(+), 20 deletions(-)
> > > >
> >
> > [..]
> >
> > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > > dfe64427d..a556a8aff 100644
> > > > --- a/app/test-pmd/testpmd.c
> > > > +++ b/app/test-pmd/testpmd.c
> >
> > [..]
> >
> > > > +/*
> > > > * Setup default configuration.
> > > > */
> > > > static void
> > > > @@ -388,12 +405,14 @@ set_default_fwd_lcores_config(void)
> > > >
> > > > nb_lc = 0;
> > > > for (i = 0; i < RTE_MAX_LCORE; i++) {
> > > > - sock_num = rte_lcore_to_socket_id(i) + 1;
> > > > - if (sock_num > max_socket) {
> > > > - if (sock_num > RTE_MAX_NUMA_NODES)
> > > > - rte_exit(EXIT_FAILURE, "Total sockets
> > > greater
> > > > than %u\n", RTE_MAX_NUMA_NODES);
> > > > - max_socket = sock_num;
> > > > + sock_num = rte_lcore_to_socket_id(i);
> > > +1 is missed?
> >
> > I don't think so.
> > On previous implementation this logic was meant to find the max_socket.
> > max_socket was the first invalid socket_id and was used, for example :
> >
> > if (socket_id >= max_socket) {
> >
> > or
> >
> > for (i = 0; i < max_socket; i++)
> >
> > now, on above logic, we list the valid socket id. Therefore
> > rte_lcore_to_socket_id return a valid id and not need to add 1.
> >
> >
> OK, but at list the following check "if (sock_num >
> RTE_MAX_NUMA_NODES)"
> Should be "if (sock_num +1 > RTE_MAX_NUMA_NODES)", right?
>
Right, i prefer the following though, what do you think?
for (i = 0; i < RTE_MAX_LCORE; i++) {
sock_num = rte_lcore_to_socket_id(i);
if (new_socket_id(sock_num)) {
if (num_sockets >= RTE_MAX_NUMA_NODES) {
rte_exit(EXIT_FAILURE,
"Total sockets greater than %u\n",
RTE_MAX_NUMA_NODES);
}
socket_ids[num_sockets++] = sock_num;
}
> One more thing, if this patch is fixing a bug, I think "fix" should be added
> in
> title.
OK, I guess I can change the commit title and message.
BTW note that there is a V3 with Thomas requested changes.
If we agree on above I can submit a v4 with the last updates.
>
> Thanks
> Jingjing
[dpdk-dev] [PATCH] vfio: use right index when tracking devices in a vfio group
Previous fix for properly handling devices from the same VFIO group
introduced another bug where the file descriptor of a kernel vfio
group is used as the index for tracking number of devices of a vfio
group struct handled by dpdk vfio code. Instead of the file
descriptor itself, the vfio group object that file descriptor is
registered with has to be used.
This patch introduces specific functions for incrementing or
decrementing the device counter for a specific vfio group using the
vfio file descriptor as a parameter. Note the code is not optimized
as the vfio group is found sequentially going through the vfio group
array but this should not be a problem as this is not related to
packet handling at all.
Fixes: a9c349e3a100 ("vfio: fix device unplug when several devices per group")
Signed-off-by: Alejandro Lucero
---
lib/librte_eal/linuxapp/eal/eal_vfio.c | 60 +++---
1 file changed, 49 insertions(+), 11 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index d3eae20..21d126f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -172,6 +172,44 @@
return -1;
}
+
+static int
+get_vfio_group_idx(int vfio_group_fd)
+{
+ int i;
+ for (i = 0; i < VFIO_MAX_GROUPS; i++)
+ if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd)
+ return i;
+ return -1;
+}
+
+static void
+vfio_group_device_get(int vfio_group_fd)
+{
+ int i;
+
+ i = get_vfio_group_idx(vfio_group_fd);
+ vfio_cfg.vfio_groups[i].devices++;
+}
+
+static void
+vfio_group_device_put(int vfio_group_fd)
+{
+ int i;
+
+ i = get_vfio_group_idx(vfio_group_fd);
+ vfio_cfg.vfio_groups[i].devices--;
+}
+
+static int
+vfio_group_device_count(int vfio_group_fd)
+{
+ int i;
+
+ i = get_vfio_group_idx(vfio_group_fd);
+ return vfio_cfg.vfio_groups[i].devices;
+}
+
int
clear_group(int vfio_group_fd)
{
@@ -180,15 +218,14 @@
if (internal_config.process_type == RTE_PROC_PRIMARY) {
- for (i = 0; i < VFIO_MAX_GROUPS; i++)
- if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd) {
- vfio_cfg.vfio_groups[i].group_no = -1;
- vfio_cfg.vfio_groups[i].fd = -1;
- vfio_cfg.vfio_groups[i].devices = 0;
- vfio_cfg.vfio_active_groups--;
- return 0;
- }
- return -1;
+ i = get_vfio_group_idx(vfio_group_fd);
+ if ( i < 0)
+ return -1;
+ vfio_cfg.vfio_groups[i].group_no = -1;
+ vfio_cfg.vfio_groups[i].fd = -1;
+ vfio_cfg.vfio_groups[i].devices = 0;
+ vfio_cfg.vfio_active_groups--;
+ return 0;
}
/* This is just for SECONDARY processes */
@@ -358,7 +395,7 @@
clear_group(vfio_group_fd);
return -1;
}
- vfio_cfg.vfio_groups[vfio_group_fd].devices++;
+ vfio_group_device_get(vfio_group_fd);
return 0;
}
@@ -406,7 +443,8 @@
/* An VFIO group can have several devices attached. Just when there is
* no devices remaining should the group be closed.
*/
- if (--vfio_cfg.vfio_groups[vfio_group_fd].devices == 0) {
+ vfio_group_device_put(vfio_group_fd);
+ if (!vfio_group_device_count(vfio_group_fd)) {
if (close(vfio_group_fd) < 0) {
RTE_LOG(INFO, EAL, "Error when closing vfio_group_fd
for %s\n",
--
1.9.1
Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several devices per vfio group
I have just sent a fix for this.
Because this was not detected by my testing and Anatoly's either, I suspect
we do not have any automated tests related to VFIO or UIO hotplug.
With other file descriptors already opened by the app, this problem could
have been detected with far less devices than those 64 set by default with
VFIO_MAX_GROUPS. My automated tests use just up to 8VFs but incrementing
that would not help as I can not test the one-vfio-group-per-device case.
It seems there is a way for testing this with older kernels where VFs share
same VFIO group but I would prefer to have something better. I'm working on
adding some sort of test mode to the VFIO kernel code just for this kind of
things, but I would like also to have some automated tests inside DPDK
which can catch this or other similar bugs in the future.
I have not sent any new test to DPDK but I will work on adding some for
this VFIO hotplug feature.
On Mon, May 8, 2017 at 5:44 PM, Alejandro Lucero <
alejandro.luc...@netronome.com> wrote:
> Hi Jerin,
>
> On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob m> wrote:
>
>> -Original Message-
>> > Date: Sun, 30 Apr 2017 19:29:49 +0200
>> > From: Thomas Monjalon
>> > To: Alejandro Lucero
>> > Cc: dev@dpdk.org, "Burakov, Anatoly"
>> > Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
>> > devices per vfio group
>> >
>> > 28/04/2017 15:25, Burakov, Anatoly:
>> > > From: Alejandro Lucero [mailto:alejandro.luc...@netronome.com]
>> > > > VFIO allows a secure way of assigning devices to user space and
>> those
>> > > > devices which can not be isolated from other ones are set in same
>> VFIO
>> > > > group. Releasing or unplugging a device should be aware of remaining
>> > > > devices is the same group for avoiding to close such a group.
>> > > >
>> > > > Fixes: 94c0776b1bad ("vfio: support hotplug")
>> > > >
>> > > > Signed-off-by: Alejandro Lucero
>> > >
>> > > I have tested this on my setup on an old kernel with multiple
>> attach/detaches, and it works (whereas it fails without this patch).
>> > >
>> > > Acked-by: Anatoly Burakov
>> >
>> > Applied, thanks
>>
>> This patch creates issue when large number of PCIe devices connected to
>> system.
>> Found it through git bisect.
>>
>> This issue is, vfio_group_fd goes beyond 64(VFIO_MAX_GROUPS) and writes
>> to wrong memory on following code execution and sub sequentially creates
>> issues in vfio mapping or such.
>>
> vfio_cfg.vfio_groups[vfio_group_fd].devices++;
>>
>> I can increase VFIO_MAX_GROUPS, but I think, it is not correct fix as
>> vfio_group_fd generated from open system call.
>>
>> I add some prints the code for debug. Please find below the output.
>> Any thoughts from VFIO experts?
>>
>>
> That is a silly but serious bug. We are using the file descriptor as the
> index for updating devices counter of a vfio group structure internal to
> DPDK VFIO code. We should be using the vfio_group that file descriptor is
> registered with.
>
> I will send a fix where vfio_group_device_get/put/count functions are
> implemented which take the file descriptor as a parameter and then go
> through the vfio_group array for working with the right one.
>
> Thomas, is this fix in time yet for 17.05? I will send the patch today but
> I can just test it against a system with the "normal" case for VFIO device
> groups. Maybe Jerin or/and Anatoly can test it against the other case.
>
>
>> ➜ [master]83xx [dpdk-master] $ git diff
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> index d3eae20..2d8ee4c 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> @@ -100,6 +100,7 @@ vfio_get_group_fd(int iommu_group_no)
>> snprintf(filename, sizeof(filename),
>> VFIO_GROUP_FMT, iommu_group_no);
>> vfio_group_fd = open(filename, O_RDWR);
>> + printf("## name %s vfio_group_fd %d\n", filename,
>> vfio_group_fd);
>> if (vfio_group_fd < 0) {
>> /* if file not found, it's not an error */
>> if (errno != ENOENT) {
>> @@ -259,6 +260,8 @@ vfio_setup_device(const char *sysfs_base, const char
>> *dev_addr,
>> if (vfio_group_fd < 0)
>> return -1;
>>
>> + printf(" iommu_group_fd %d vfio_group_fd=%d\n",
>> iommu_group_no, vfio_group_fd);
>> +
>> /* if group_fd == 0, that means the device isn't managed by VFIO
>> * */
>> if (vfio_group_fd == 0) {
>> RTE_LOG(WARNING, EAL, " %s not managed by VFIO driver,
>> skipping\n",
>> @@ -266,6 +269,7 @@ vfio_setup_device(const char *sysfs_base, const char
>> *dev_addr,
>> return 1;
>> }
>> /*
>> * at this point, we know that this group is viable (meaning,
>> * all devices
>> * are either bound to VFIO or not bound to anything)
>> @@ -359,6 +363,7 @@
Re: [dpdk-dev] [PATCH] net/qede: fix RSS table entries for 100G adapter
Hi Thomas,
> From: Rasesh Mody [mailto:rasesh.m...@cavium.com]
> Sent: Sunday, May 07, 2017 3:53 PM
>
> With the change in base APIs the logic for 100G handling needs to be
> adjusted to pass cid values instead for queue ids. The current API works
> assuming its queue id.
>
> Fixes: 69d7ba88f1a1 ("net/qede/base: use L2-handles for RSS configuration")
This is a *CRITICAL MUST FIX* patch for 100Gb adapters supported by qed/qede on
DPDK 17.05. There will be functional and performance issues/regression without
this fix on 100Gb adapters.
Without this change, RSS in 100Gb adapters do not function properly and leads
to performance issues. Rx Queue stop command on 100Gb adapter will results in
FW exception, where the Rx queue stop request gets stuck in FW and leads to
PMD/application error saying failed to stop RXQ 0.
The issue was introduced by one of the 17.05 base driver patches
(net/qede/base: use L2-handles for RSS configuration), which has changed the
logic to use queue handles instead of queue ids. However, we missed to make the
corresponding changes in 17.05 QEDE for the 100Gb adapter. This patch addresses
that by making changes in QEDE to work with queue handles and associate the
queues to respective HW functions based on queue handles.
Please include this fix in 17.05 release.
Thanks!
-Rasesh
>
> Signed-off-by: Rasesh Mody
> ---
> drivers/net/qede/qede_eth_if.c | 27 -
> drivers/net/qede/qede_eth_if.h |2 -
> drivers/net/qede/qede_ethdev.c | 85
> ++--
> drivers/net/qede/qede_ethdev.h |1 +
> 4 files changed, 73 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/qede/qede_eth_if.c
> b/drivers/net/qede/qede_eth_if.c index 86bb129..a3c0b13 100644
> --- a/drivers/net/qede/qede_eth_if.c
> +++ b/drivers/net/qede/qede_eth_if.c
> @@ -67,33 +67,6 @@ static int qed_stop_vport(struct ecore_dev *edev,
> uint8_t vport_id)
> return 0;
> }
>
> -bool qed_update_rss_parm_cmt(struct ecore_dev *edev, uint16_t *p_tbl)
> -{
> - uint16_t max = 0, k;
> - bool rss_mode = 0; /* disable */
> - int divisor;
> -
> - /* Find largest entry, since it's possible RSS needs to
> - * be disabled [in case only 1 queue per-hwfn]
> - */
> - for (k = 0; k < ECORE_RSS_IND_TABLE_SIZE; k++)
> - max = (max > p_tbl[k]) ? max : p_tbl[k];
> -
> - /* Either fix RSS values or disable RSS */
> - if (edev->num_hwfns < max + 1) {
> - divisor = (max + edev->num_hwfns - 1) / edev->num_hwfns;
> - DP_VERBOSE(edev, ECORE_MSG_SPQ,
> -"CMT - fixing RSS values (modulo %02x)\n",
> -divisor);
> - for (k = 0; k < ECORE_RSS_IND_TABLE_SIZE; k++)
> - p_tbl[k] = p_tbl[k] % divisor;
> -
> - rss_mode = 1;
> - }
> -
> - return rss_mode;
> -}
> -
> static int
> qed_update_vport(struct ecore_dev *edev, struct
> qed_update_vport_params *params) { diff --git
> a/drivers/net/qede/qede_eth_if.h b/drivers/net/qede/qede_eth_if.h
> index d845bac..097e025 100644
> --- a/drivers/net/qede/qede_eth_if.h
> +++ b/drivers/net/qede/qede_eth_if.h
> @@ -129,6 +129,4 @@ struct qed_eth_ops { int
> qed_configure_filter_rx_mode(struct rte_eth_dev *eth_dev,
>enum qed_filter_rx_mode_type type);
>
> -bool qed_update_rss_parm_cmt(struct ecore_dev *edev, uint16_t *p_tbl);
> -
> #endif /* _QEDE_ETH_IF_H */
> diff --git a/drivers/net/qede/qede_ethdev.c
> b/drivers/net/qede/qede_ethdev.c index 4ef5765..7501eb2 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -1576,6 +1576,61 @@ static int qede_rss_hash_conf_get(struct
> rte_eth_dev *eth_dev,
> return 0;
> }
>
> +static bool qede_update_rss_parm_cmt(struct ecore_dev *edev,
> + struct ecore_rss_params *rss)
> +{
> + int i, fn;
> + bool rss_mode = 1; /* enable */
> + struct ecore_queue_cid *cid;
> + struct ecore_rss_params *t_rss;
> +
> + /* In regular scenario, we'd simply need to take input handlers.
> + * But in CMT, we'd have to split the handlers according to the
> + * engine they were configured on. We'd then have to understand
> + * whether RSS is really required, since 2-queues on CMT doesn't
> + * require RSS.
> + */
> +
> + /* CMT should be round-robin */
> + for (i = 0; i < ECORE_RSS_IND_TABLE_SIZE; i++) {
> + cid = rss->rss_ind_table[i];
> +
> + if (cid->p_owner == ECORE_LEADING_HWFN(edev))
> + t_rss = &rss[0];
> + else
> + t_rss = &rss[1];
> +
> + t_rss->rss_ind_table[i / edev->num_hwfns] = cid;
> + }
> +
> + t_rss = &rss[1];
> + t_rss->update_rss_ind_table = 1;
> + t_rss->rss_table_size_log = 7;
> + t_rss->update_rss_config = 1;
> +
> + /* Make sure RSS is actually required */
> + for_
[dpdk-dev] [PATCH v2] doc: Merge l3fwd and l3fwd-acl documentation files
Merge l3fwd and l3fwd-acl documentation to reflect the code. Add examples of LPM and Exact Match rules in .svg files. Signed-off-by: Ravi Kerur --- v2: Remove binary PNG from .svg files. v1: Merge relevant contents of l3fwd and l3fwd-acl documentation. Modify l3fwd document with ACL specific information. Remove l3fwd-acl documentation file. --- doc/guides/sample_app_ug/img/ipv4_hash_rule.svg| 108 ++ doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg | 96 + doc/guides/sample_app_ug/index.rst | 1 - doc/guides/sample_app_ug/l3_forward.rst| 326 - .../sample_app_ug/l3_forward_access_ctrl.rst | 385 - 5 files changed, 521 insertions(+), 395 deletions(-) create mode 100644 doc/guides/sample_app_ug/img/ipv4_hash_rule.svg create mode 100644 doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg delete mode 100644 doc/guides/sample_app_ug/l3_forward_access_ctrl.rst diff --git a/doc/guides/sample_app_ug/img/ipv4_hash_rule.svg b/doc/guides/sample_app_ug/img/ipv4_hash_rule.svg new file mode 100644 index 000..76edfda --- /dev/null +++ b/doc/guides/sample_app_ug/img/ipv4_hash_rule.svg @@ -0,0 +1,108 @@ + + + +http://purl.org/dc/elements/1.1/"; + xmlns:cc="http://creativecommons.org/ns#"; + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; + xmlns:svg="http://www.w3.org/2000/svg"; + xmlns="http://www.w3.org/2000/svg"; + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"; + xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"; + width="744.09448819" + height="1052.3622047" + id="svg2" + version="1.1" + inkscape:version="0.48.4 r9939" + sodipodi:docname="New document 1"> + + + + + +image/svg+xml +http://purl.org/dc/dcmitype/StillImage"; /> + + + + + + E101.0.0.0 100.10.0.0 101 11 0x06 0 + + + + + + Src Addr Dest Addr Src port Dest port ProtocolFwd + diff --git a/doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg b/doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg new file mode 100644 index 000..1b7f02c --- /dev/null +++ b/doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg @@ -0,0 +1,96 @@ + + + +http://purl.org/dc/elements/1.1/"; + xmlns:cc="http://creativecommons.org/ns#"; + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; + xmlns:svg="http://www.w3.org/2000/svg"; + xmlns="http://www.w3.org/2000/svg"; + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"; + xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"; + width="744.09448819" + height="1052.3622047" + id="svg2" + version="1.1" + inkscape:version="0.48.4 r9939" + sodipodi:docname="New document 1"> + + + + + +image/svg+xml +http://purl.org/dc/dcmitype/StillImage"; /> + + + + + + L101.10.1.0/24 0 + + Dest Addr/Prefix Fwd + diff --git a/doc/guides/sample_app_ug/index.rst b/doc/guides/sample_app_ug/index.rst index 02611ef..62cafb0 100644 --- a/doc/guides/sample_app_ug/index.rst +++ b/doc/guides/sample_app_ug/index.rst @@ -53,7 +53,6 @@ Sample Applications User Guides l2_forward_cat l3_forward l3_forward_power_man -l3_forward_access_ctrl l3_forward_virtual link_status_intr load_balancer diff --git a/doc/guides/sample_app_ug/l3_forward.rst b/doc/guides/sample_app_ug/l3_forward.rst index 6a6b8fb..69063bd 100644 --- a/doc/guides/sample_app_ug/l3_forward.rst +++ b/doc/guides/sample_app_ug/l3_forward.rst @@ -1,5 +1,5 @@ .. BSD LICENSE -Copyright(c) 2010-2014 Intel Corporation. All rights reserved. +Copyright(c) 2010-2017 Intel Corporation. All rights reserved. All rights reserved. Redistribution and use in source and binary forms, with or without @@ -31,12 +31,18 @@ L3 Forwarding Sample Application -The L3 Forwarding application is a simple example of packet processing using the DPDK. -The application performs L3 forwarding. +The L3 Forwarding with Hash, LPM or Access Control application is a simple example of packet processing using the DPDK. +The application performs L3 forwarding when Hash or LPM is used. +The application performs a security check on received packets when Access Control is used. +Packets that are in the Access Control List (ACL), which is loaded during initialization, are dropped. +Others are forwarded to the correct port. Overview +Hash and LPM + + The application demonstrates the use of the hash and LPM libraries in the DPDK to implement packet forwarding. The initialization and run-time paths are very similar to those of the :doc:`l2_forward_real_virtual`. The main difference from the L2 Forwarding sample appl
Re: [dpdk-dev] [PATCH v1] doc: Merge l3fwd and l3fwd-acl documentation files
On Sun, May 7, 2017 at 1:50 PM, Thomas Monjalon wrote: > Hi, > > 25/04/2017 20:39, Ravi Kerur: > > Merge relevant contents of l3fwd and l3fwd-acl documentation. > > Modify l3fwd document with ACL specific information. > > Remove l3fwd-acl documentation file. > > > > Signed-off-by: Ravi Kerur > > --- > > doc/guides/sample_app_ug/img/ipv4_hash_rule.svg| 158 + > > doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg | 139 > > doc/guides/sample_app_ug/index.rst | 1 - > > doc/guides/sample_app_ug/l3_forward.rst| 326 > - > > .../sample_app_ug/l3_forward_access_ctrl.rst | 385 > - > > 5 files changed, 614 insertions(+), 395 deletions(-) > > create mode 100644 doc/guides/sample_app_ug/img/ipv4_hash_rule.svg > > create mode 100644 doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg > > delete mode 100644 doc/guides/sample_app_ug/l3_forward_access_ctrl.rst > > I've not looked at the content. > I just do not understand why a patch for merging content is adding > some new files. > Moreover, these SVG files contains some binary PNG. > Please send only some source files. > Thanks John and Thomas for review. We have created two new ".svg" files illustrating an example for LPM and Exact Match rules and hence they are reflected in the patch. I have created .svg files directly from inkscape and they now contain only html. I have sent 'v2', please review it and let me know if additional changes are required.
Re: [dpdk-dev] [PATCH v2] app/testpmd: support non contiguous socket ids
>
> Right, i prefer the following though, what do you think?
>
>for (i = 0; i < RTE_MAX_LCORE; i++) {
> sock_num = rte_lcore_to_socket_id(i);
> if (new_socket_id(sock_num)) {
> if (num_sockets >= RTE_MAX_NUMA_NODES) {
> rte_exit(EXIT_FAILURE,
> "Total sockets greater than %u\n",
> RTE_MAX_NUMA_NODES);
> }
> socket_ids[num_sockets++] = sock_num;
> }
>
That looks good :)
> > One more thing, if this patch is fixing a bug, I think "fix" should be
> > added in title.
>
> OK, I guess I can change the commit title and message.
>
> BTW note that there is a V3 with Thomas requested changes.
> If we agree on above I can submit a v4 with the last updates.
>
>
OK.
Thanks
Jingjing
Re: [dpdk-dev] [PATCH] net/ixgbe: fix LSC interrupt issue
Hi Qi,
> -Original Message-
> From: Zhang, Qi Z
> Sent: Monday, May 8, 2017 11:58 AM
> To: Zhang, Helin; Lu, Wenzhuo
> Cc: dev@dpdk.org; Zhang, Qi Z; sta...@dpdk.org
> Subject: [PATCH] net/ixgbe: fix LSC interrupt issue
>
> There is a bug in previous fix for lsc interrupt.
> lsc interrupt is not disabled before delayed handler, that cause the delayed
> handler be re-entered.
>
> Fixes: 9b667210700e ("net/ixgbe: fix blocked interrupts")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Qi Zhang
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index ec667d8..c680aab 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -4107,14 +4107,15 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev
> *dev,
> timeout = IXGBE_LINK_DOWN_CHECK_TIMEOUT;
>
> ixgbe_dev_link_status_print(dev);
> - intr->mask_original = intr->mask;
> - /* only disable lsc interrupt */
> - intr->mask &= ~IXGBE_EIMS_LSC;
> if (rte_eal_alarm_set(timeout * 1000,
> ixgbe_dev_interrupt_delayed_handler,
> (void *)dev) < 0)
> PMD_DRV_LOG(ERR, "Error setting alarm");
> - else
> - intr->mask = intr->mask_original;
> + else {
> + /* remember orignal mask */
Acked-by: Wenzhuo Lu
Except orignal -> original
Re: [dpdk-dev] [PATCH] vfio: use right index when tracking devices in a vfio group
-Original Message-
> Date: Mon, 8 May 2017 18:44:18 +0100
> From: Alejandro Lucero
> To: dev@dpdk.org
> CC: anatoly.bura...@intel.com, jerin.ja...@caviumnetworks.com,
> tho...@monjalon.net
> Subject: [PATCH] vfio: use right index when tracking devices in a vfio group
> X-Mailer: git-send-email 1.9.1
>
> Previous fix for properly handling devices from the same VFIO group
> introduced another bug where the file descriptor of a kernel vfio
> group is used as the index for tracking number of devices of a vfio
> group struct handled by dpdk vfio code. Instead of the file
> descriptor itself, the vfio group object that file descriptor is
> registered with has to be used.
>
> This patch introduces specific functions for incrementing or
> decrementing the device counter for a specific vfio group using the
> vfio file descriptor as a parameter. Note the code is not optimized
> as the vfio group is found sequentially going through the vfio group
> array but this should not be a problem as this is not related to
> packet handling at all.
>
> Fixes: a9c349e3a100 ("vfio: fix device unplug when several devices per group")
>
> Signed-off-by: Alejandro Lucero
Tested-by: Jerin Jacob
> ---
> lib/librte_eal/linuxapp/eal/eal_vfio.c | 60
> +++---
> 1 file changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> index d3eae20..21d126f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> @@ -172,6 +172,44 @@
> return -1;
> }
>
> +
> +static int
> +get_vfio_group_idx(int vfio_group_fd)
> +{
> + int i;
> + for (i = 0; i < VFIO_MAX_GROUPS; i++)
> + if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd)
> + return i;
> + return -1;
> +}
> +
> +static void
> +vfio_group_device_get(int vfio_group_fd)
> +{
> + int i;
> +
> + i = get_vfio_group_idx(vfio_group_fd);
> + vfio_cfg.vfio_groups[i].devices++;
> +}
> +
> +static void
> +vfio_group_device_put(int vfio_group_fd)
> +{
> + int i;
> +
> + i = get_vfio_group_idx(vfio_group_fd);
> + vfio_cfg.vfio_groups[i].devices--;
> +}
> +
> +static int
> +vfio_group_device_count(int vfio_group_fd)
> +{
> + int i;
> +
> + i = get_vfio_group_idx(vfio_group_fd);
> + return vfio_cfg.vfio_groups[i].devices;
> +}
> +
> int
> clear_group(int vfio_group_fd)
> {
> @@ -180,15 +218,14 @@
>
> if (internal_config.process_type == RTE_PROC_PRIMARY) {
>
> - for (i = 0; i < VFIO_MAX_GROUPS; i++)
> - if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd) {
> - vfio_cfg.vfio_groups[i].group_no = -1;
> - vfio_cfg.vfio_groups[i].fd = -1;
> - vfio_cfg.vfio_groups[i].devices = 0;
> - vfio_cfg.vfio_active_groups--;
> - return 0;
> - }
> - return -1;
> + i = get_vfio_group_idx(vfio_group_fd);
> + if ( i < 0)
> + return -1;
> + vfio_cfg.vfio_groups[i].group_no = -1;
> + vfio_cfg.vfio_groups[i].fd = -1;
> + vfio_cfg.vfio_groups[i].devices = 0;
> + vfio_cfg.vfio_active_groups--;
> + return 0;
> }
>
> /* This is just for SECONDARY processes */
> @@ -358,7 +395,7 @@
> clear_group(vfio_group_fd);
> return -1;
> }
> - vfio_cfg.vfio_groups[vfio_group_fd].devices++;
> + vfio_group_device_get(vfio_group_fd);
>
> return 0;
> }
> @@ -406,7 +443,8 @@
> /* An VFIO group can have several devices attached. Just when there is
>* no devices remaining should the group be closed.
>*/
> - if (--vfio_cfg.vfio_groups[vfio_group_fd].devices == 0) {
> + vfio_group_device_put(vfio_group_fd);
> + if (!vfio_group_device_count(vfio_group_fd)) {
>
> if (close(vfio_group_fd) < 0) {
> RTE_LOG(INFO, EAL, "Error when closing vfio_group_fd
> for %s\n",
> --
> 1.9.1
>
Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several devices per vfio group
-Original Message-
> Date: Mon, 8 May 2017 17:44:37 +0100
> From: Alejandro Lucero
> To: Jerin Jacob
> Cc: Thomas Monjalon , dev , "Burakov,
> Anatoly"
> Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
> devices per vfio group
>
> Hi Jerin,
>
> On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob
> wrote:
>
> > -Original Message-
> > > Date: Sun, 30 Apr 2017 19:29:49 +0200
> > > From: Thomas Monjalon
> > > To: Alejandro Lucero
> > > Cc: dev@dpdk.org, "Burakov, Anatoly"
> > > Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
> > > devices per vfio group
> > >
> > > 28/04/2017 15:25, Burakov, Anatoly:
> > > > From: Alejandro Lucero [mailto:alejandro.luc...@netronome.com]
> > > > > VFIO allows a secure way of assigning devices to user space and those
> > > > > devices which can not be isolated from other ones are set in same
> > VFIO
> > > > > group. Releasing or unplugging a device should be aware of remaining
> > > > > devices is the same group for avoiding to close such a group.
> > > > >
> > > > > Fixes: 94c0776b1bad ("vfio: support hotplug")
> > > > >
> > > > > Signed-off-by: Alejandro Lucero
> > > >
> > > > I have tested this on my setup on an old kernel with multiple
> > attach/detaches, and it works (whereas it fails without this patch).
> > > >
> > > > Acked-by: Anatoly Burakov
> > >
> > > Applied, thanks
> >
> > This patch creates issue when large number of PCIe devices connected to
> > system.
> > Found it through git bisect.
> >
> > This issue is, vfio_group_fd goes beyond 64(VFIO_MAX_GROUPS) and writes
> > to wrong memory on following code execution and sub sequentially creates
> > issues in vfio mapping or such.
> >
> vfio_cfg.vfio_groups[vfio_group_fd].devices++;
> >
> > I can increase VFIO_MAX_GROUPS, but I think, it is not correct fix as
> > vfio_group_fd generated from open system call.
> >
> > I add some prints the code for debug. Please find below the output.
> > Any thoughts from VFIO experts?
> >
> >
> That is a silly but serious bug. We are using the file descriptor as the
> index for updating devices counter of a vfio group structure internal to
> DPDK VFIO code. We should be using the vfio_group that file descriptor is
> registered with.
>
> I will send a fix where vfio_group_device_get/put/count functions are
> implemented which take the file descriptor as a parameter and then go
> through the vfio_group array for working with the right one.
>
> Thomas, is this fix in time yet for 17.05? I will send the patch today but
> I can just test it against a system with the "normal" case for VFIO device
> groups. Maybe Jerin or/and Anatoly can test it against the other case.
Thanks Alejandro for the patch.
Tested your patch on failure setup, it works fine after applying your
patch.
IMO, for v17.05, this fix must go in or we need to revert the original offending
patch(a9c349e3a100 ("vfio: fix device unplug when several devices per
group")) as it breaks DPDK running on the system with few PCIe devices
connected in.
[dpdk-dev] [PATCH] ethdev: fix wrong sizeof argument in malloc function
From: Michal Jastrzebski
Coverity reported that an argument for sizeof was used improperly.
We should allocate memory for value size that pointer points to,
instead of pointer size itself.
Coverity issue: 144522
Fixes: 79c913a42f0e ("ethdev: retrieve xstats by ID")
Signed-off-by: Michal Jastrzebski
---
lib/librte_ether/rte_ethdev.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 8cf8b65..83898a8 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1714,7 +1714,7 @@ struct rte_eth_dev *
size = rte_eth_xstats_get_by_id(port_id, NULL, NULL, 0);
- values_copy = malloc(sizeof(values_copy) * size);
+ values_copy = malloc(sizeof(*values_copy) * size);
if (!values_copy) {
RTE_PMD_DEBUG_TRACE(
"ERROR: can't allocate memory for values_copy\n");
--
1.7.9.5
[dpdk-dev] [PATCH] proc-info: wrong sizeof argument in malloc function
From: Michal Jastrzebski
Coverity reported that an argument for sizeof was used improperly.
We should allocate memory for value size that pointer points to,
instead of pointer size itself.
Coverity issue: 144523, 144521
Fixes: 7ac16a3660c0 ("app/proc-info: support xstats by ID and by name")
Signed-off-by: Michal Jastrzebski
---
app/proc_info/main.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 17a1c87..d4f6a82 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -434,7 +434,7 @@ static void collectd_resolve_cnt_type(char *cnt_type,
size_t cnt_type_len,
int ret, i;
static const char *nic_stats_border = "";
- values = malloc(sizeof(values) * len);
+ values = malloc(sizeof(*values) * len);
if (values == NULL) {
printf("Cannot allocate memory for xstats\n");
return;
@@ -486,7 +486,7 @@ static void collectd_resolve_cnt_type(char *cnt_type,
size_t cnt_type_len,
printf("Cannot get xstats count\n");
return;
}
- values = malloc(sizeof(values) * len);
+ values = malloc(sizeof(*values) * len);
if (values == NULL) {
printf("Cannot allocate memory for xstats\n");
return;
--
1.7.9.5
[dpdk-dev] [PATCH v3] eal: Set numa node value for system which not support it.
The NUMA node information for PCI devices provided through
sysfs is invalid for AMD Opteron(TM) Processor 62xx and 63xx
on Red Hat Enterprise Linux 6, and VMs on some hypervisors.
It is good to see more checking for valid values.
Signed-off-by: Tonghao Zhang
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 595622b..19ef8bd 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -310,19 +310,15 @@
dev->max_vfs = (uint16_t)tmp;
}
- /* get numa node */
+ /* get numa node, default to 0 if not present */
snprintf(filename, sizeof(filename), "%s/numa_node",
dirname);
- if (access(filename, R_OK) != 0) {
- /* if no NUMA support, set default to 0 */
- dev->device.numa_node = 0;
- } else {
- if (eal_parse_sysfs_value(filename, &tmp) < 0) {
- free(dev);
- return -1;
- }
+
+if (eal_parse_sysfs_value(filename, &tmp) == 0 &&
+tmp < RTE_MAX_NUMA_NODES)
dev->device.numa_node = tmp;
- }
+else
+dev->device.numa_node = 0;
rte_pci_device_name(addr, dev->name, sizeof(dev->name));
dev->device.name = dev->name;
--
1.8.3.1
[dpdk-dev] [PATCH v3] eal: Set numa node value for system which not support it.
The NUMA node information for PCI devices provided through
sysfs is invalid for AMD Opteron(TM) Processor 62xx and 63xx
on Red Hat Enterprise Linux 6, and VMs on some hypervisors.
It is good to see more checking for valid values.
Signed-off-by: Tonghao Zhang
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 595622b..95a051f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -310,19 +310,15 @@
dev->max_vfs = (uint16_t)tmp;
}
- /* get numa node */
+ /* get numa node, default to 0 if not present */
snprintf(filename, sizeof(filename), "%s/numa_node",
dirname);
- if (access(filename, R_OK) != 0) {
- /* if no NUMA support, set default to 0 */
- dev->device.numa_node = 0;
- } else {
- if (eal_parse_sysfs_value(filename, &tmp) < 0) {
- free(dev);
- return -1;
- }
+
+ if (eal_parse_sysfs_value(filename, &tmp) == 0 &&
+ tmp < RTE_MAX_NUMA_NODES)
dev->device.numa_node = tmp;
- }
+ else
+ dev->device.numa_node = 0;
rte_pci_device_name(addr, dev->name, sizeof(dev->name));
dev->device.name = dev->name;
--
1.8.3.1
