| Message ID | 1506438251-16537-1-git-send-email-antonio.fischetti@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,1/4] netdev-dpdk: fix mempool management with vhu client. | expand |
On 9/26/17, 8:04 AM, "ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com> wrote:
In a PVP test where vhostuser ports are configured as
clients, OvS crashes when QEMU is launched.
This patch avoids the repeated calls to netdev_change_seq_changed
after the requested mempool is already acquired.
CC: Ciara Loftus <ciara.loftus@intel.com>
CC: Kevin Traynor <ktraynor@redhat.com>
CC: Aaron Conole <aconole@redhat.com>
Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.")
Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>
---
To replicate the bug scenario:
[Darrell] Just curious, but reproducibility with below steps ?; what about using libvirt ?
Do we have the stacktrace ?
PVP test setup
--------------
CLIENT_SOCK_DIR=/tmp
SOCK0=dpdkvhostuser0
SOCK1=dpdkvhostuser1
1 PMD
Add 2 dpdk ports, n_rxq=1
Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-path
ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK0"
ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK1"
Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
add-flow br0 in_port=1,action=output:3
add-flow br0 in_port=3,action=output:1
add-flow br0 in_port=4,action=output:2
add-flow br0 in_port=2,action=output:4
Launch QEMU
-----------
As OvS vhu ports are acting as clients, we must specify 'server' in the next command.
VM_IMAGE=<path/to/your/vm/image>
sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name us-vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend-file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem -mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev type=vhost-user,id=mynet2,chardev=char1,vhostforce -device virtio-net-pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic
Expected behavior
-----------------
With this fix OvS shouldn't crash.
---
lib/netdev-dpdk.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 648d719..2f5ec71 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -549,7 +549,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
dmp->socket_id);
if (dmp->mp) {
VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
- dmp->mp_size);
+ dmp->mp_size);
} else if (rte_errno == EEXIST) {
/* A mempool with the same name already exists. We just
* retrieve its pointer to be returned to the caller. */
@@ -571,6 +571,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
* initializes some OVS specific fields of dp_packet.
*/
rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
+
return dmp;
}
} while (!mp_exists &&
@@ -3247,11 +3248,14 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
netdev_dpdk_remap_txqs(dev);
- err = netdev_dpdk_mempool_configure(dev);
- if (err) {
- return err;
- } else {
- netdev_change_seq_changed(&dev->up);
+ if (dev->requested_socket_id != dev->socket_id
+ || dev->requested_mtu != dev->mtu) {
[Darrell] Can this check be moved to common code in
netdev_dpdk_mempool_configure() and using a special return value
that skips netdev_change_seq_changed(&dev->up); in the callers, if
the call would be redundant ?
+ err = netdev_dpdk_mempool_configure(dev);
+ if (err) {
+ return err;
+ } else {
+ netdev_change_seq_changed(&dev->up);
+ }
}
if (netdev_dpdk_get_vid(dev) >= 0) {
--
2.4.11
_______________________________________________
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=eAz1yiSbXvMuAA292xJwliYRlu6n1Y0nqm0MuU2JTXY&s=4gLg6fgwmWYEM4s5v4S0NjY52DBbxKTmaQalT8194NE&e=
On 9/26/17, 12:58 PM, "Darrell Ball" <dball@vmware.com> wrote:
On 9/26/17, 8:04 AM, "ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com> wrote:
In a PVP test where vhostuser ports are configured as
clients, OvS crashes when QEMU is launched.
This patch avoids the repeated calls to netdev_change_seq_changed
after the requested mempool is already acquired.
CC: Ciara Loftus <ciara.loftus@intel.com>
CC: Kevin Traynor <ktraynor@redhat.com>
CC: Aaron Conole <aconole@redhat.com>
Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.")
Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>
---
To replicate the bug scenario:
[Darrell] Just curious, but reproducibility with below steps ?; what about using libvirt ?
Do we have the stacktrace ?
PVP test setup
--------------
CLIENT_SOCK_DIR=/tmp
SOCK0=dpdkvhostuser0
SOCK1=dpdkvhostuser1
1 PMD
Add 2 dpdk ports, n_rxq=1
Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-path
ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK0"
ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK1"
Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
add-flow br0 in_port=1,action=output:3
add-flow br0 in_port=3,action=output:1
add-flow br0 in_port=4,action=output:2
add-flow br0 in_port=2,action=output:4
Launch QEMU
-----------
As OvS vhu ports are acting as clients, we must specify 'server' in the next command.
VM_IMAGE=<path/to/your/vm/image>
sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name us-vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend-file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem -mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev type=vhost-user,id=mynet2,chardev=char1,vhostforce -device virtio-net-pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic
Expected behavior
-----------------
With this fix OvS shouldn't crash.
---
lib/netdev-dpdk.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 648d719..2f5ec71 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -549,7 +549,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
dmp->socket_id);
if (dmp->mp) {
VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
- dmp->mp_size);
+ dmp->mp_size);
[Darrell]
is this needed?
} else if (rte_errno == EEXIST) {
/* A mempool with the same name already exists. We just
* retrieve its pointer to be returned to the caller. */
@@ -571,6 +571,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
* initializes some OVS specific fields of dp_packet.
*/
rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
+
return dmp;
}
} while (!mp_exists &&
@@ -3247,11 +3248,14 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
netdev_dpdk_remap_txqs(dev);
- err = netdev_dpdk_mempool_configure(dev);
- if (err) {
- return err;
- } else {
- netdev_change_seq_changed(&dev->up);
+ if (dev->requested_socket_id != dev->socket_id
+ || dev->requested_mtu != dev->mtu) {
[Darrell] Can this check be moved to common code in
netdev_dpdk_mempool_configure() and using a special return value
that skips netdev_change_seq_changed(&dev->up); in the callers, if
the call would be redundant ?
+ err = netdev_dpdk_mempool_configure(dev);
+ if (err) {
+ return err;
+ } else {
+ netdev_change_seq_changed(&dev->up);
+ }
}
if (netdev_dpdk_get_vid(dev) >= 0) {
--
2.4.11
_______________________________________________
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=eAz1yiSbXvMuAA292xJwliYRlu6n1Y0nqm0MuU2JTXY&s=4gLg6fgwmWYEM4s5v4S0NjY52DBbxKTmaQalT8194NE&e=
Thanks Darrell, replies inline. -Antonio > -----Original Message----- > From: Darrell Ball [mailto:dball@vmware.com] > Sent: Tuesday, September 26, 2017 9:04 PM > To: Fischetti, Antonio <antonio.fischetti@intel.com>; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 1/4] netdev-dpdk: fix mempool management with vhu > client. > > > > On 9/26/17, 12:58 PM, "Darrell Ball" <dball@vmware.com> wrote: > > > > On 9/26/17, 8:04 AM, "ovs-dev-bounces@openvswitch.org on behalf of > antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of > antonio.fischetti@intel.com> wrote: > > In a PVP test where vhostuser ports are configured as > clients, OvS crashes when QEMU is launched. > This patch avoids the repeated calls to netdev_change_seq_changed > after the requested mempool is already acquired. > > CC: Ciara Loftus <ciara.loftus@intel.com> > CC: Kevin Traynor <ktraynor@redhat.com> > CC: Aaron Conole <aconole@redhat.com> > Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each > port.") > Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> > --- > To replicate the bug scenario: > > [Darrell] Just curious, but reproducibility with below steps ?; what about > using libvirt ? > Do we have the stacktrace ? [Antonio] Actually I didn't try with libvirt. I also saw that it didn't crash when the vhostuser ports were set with type=dpdkvhostuser. Below is the stacktrace on a crash, ie when setting type=dpdkvhostuserclient: Thread 26 "pmd124" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7f73ca7fc700 (LWP 20176)] 0x0000000000410fa9 in stack_dequeue () (gdb) bt #0 0x0000000000410fa9 in stack_dequeue () #1 0x00000000005cdc17 in rte_mempool_ops_dequeue_bulk (mp=0x7f72fb83c940, obj_table=0x7f73ca7fb258, n=1) at /home/user/dpdk/x86_64-native-linuxapp-gcc/include/rte_mempool.h:474 #2 0x00000000005cdff9 in __mempool_generic_get (cache=0x0, n=1, obj_table=0x7f73ca7fb258, mp=0x7f72fb83c940 ) at /home/user/dpdk/x86_64-native-linuxapp-gcc/include/rte_mempool.h:1218 #3 rte_mempool_generic_get (flags=0, cache=0x0, n=1, obj_table=0x7f73ca7fb258, mp=0x7f72fb83c940) at /home/ user/dpdk/x86_64-native-linuxapp-gcc/include/rte_mempool.h:1256 #4 rte_mempool_get_bulk (n=1, obj_table=0x7f73ca7fb258, mp=0x7f72fb83c940) at /home/user/dpdk/x86_64-nat ive-linuxapp-gcc/include/rte_mempool.h:1289 #5 rte_mempool_get (obj_p=0x7f73ca7fb258, mp=0x7f72fb83c940) at /home/user/dpdk/x86_64-native-linuxapp-g cc/include/rte_mempool.h:1315 #6 rte_mbuf_raw_alloc (mp=0x7f72fb83c940) at /home/user/dpdk/x86_64-native-linuxapp-gcc/include/rte_mbuf .h:822 #7 0x00000000005ce14b in rte_pktmbuf_alloc (mp=0x7f72fb83c940) at /home/user/dpdk/x86_64-native-linuxapp -gcc/include/rte_mbuf.h:1122 #8 0x00000000005d283a in rte_vhost_dequeue_burst (vid=0, queue_id=1, mbuf_pool=0x7f72fb83c940, pkts=0x7f73c a7fb830, count=1) at /home/user/dpdk/lib/librte_vhost/virtio_net.c:1116 #9 0x00000000007b4025 in netdev_dpdk_vhost_rxq_recv (rxq=0x7f72ffdab080, batch=0x7f73ca7fb820) at lib/netde v-dpdk.c:1650 #10 0x000000000070d331 in netdev_rxq_recv () #11 0x00000000006ea8ce in dp_netdev_process_rxq_port () #12 0x00000000006eaba0 in pmd_thread_main () #13 0x000000000075cb34 in ovsthread_wrapper () #14 0x00007f742a0e65ca in start_thread () from /lib64/libpthread.so.0 #15 0x00007f742990f0cd in clone () from /lib64/libc.so.6 > > > > PVP test setup > -------------- > CLIENT_SOCK_DIR=/tmp > SOCK0=dpdkvhostuser0 > SOCK1=dpdkvhostuser1 > > 1 PMD > Add 2 dpdk ports, n_rxq=1 > Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost- > server-path > ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server- > path="$CLIENT_SOCK_DIR/$SOCK0" > ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server- > path="$CLIENT_SOCK_DIR/$SOCK1" > > Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1 > add-flow br0 in_port=1,action=output:3 > add-flow br0 in_port=3,action=output:1 > add-flow br0 in_port=4,action=output:2 > add-flow br0 in_port=2,action=output:4 > > Launch QEMU > ----------- > As OvS vhu ports are acting as clients, we must specify 'server' in the > next command. > VM_IMAGE=<path/to/your/vm/image> > > sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 - > name us-vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend- > file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem - > mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev > socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost- > user,id=mynet1,chardev=char0,vhostforce -device virtio-net- > pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev > socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev type=vhost- > user,id=mynet2,chardev=char1,vhostforce -device virtio-net- > pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic > > Expected behavior > ----------------- > With this fix OvS shouldn't crash. > --- > lib/netdev-dpdk.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 648d719..2f5ec71 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -549,7 +549,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu) > dmp->socket_id); > if (dmp->mp) { > VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", > mp_name, > - dmp->mp_size); > + dmp->mp_size); > > > [Darrell] > is this needed? [Antonio] ok will remove. > > > > } else if (rte_errno == EEXIST) { > /* A mempool with the same name already exists. We just > * retrieve its pointer to be returned to the caller. */ > @@ -571,6 +571,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu) > * initializes some OVS specific fields of dp_packet. > */ > rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL); > + > return dmp; > } > } while (!mp_exists && > @@ -3247,11 +3248,14 @@ dpdk_vhost_reconfigure_helper(struct > netdev_dpdk *dev) > > netdev_dpdk_remap_txqs(dev); > > - err = netdev_dpdk_mempool_configure(dev); > - if (err) { > - return err; > - } else { > - netdev_change_seq_changed(&dev->up); > + if (dev->requested_socket_id != dev->socket_id > + || dev->requested_mtu != dev->mtu) { > > > [Darrell] Can this check be moved to common code in > netdev_dpdk_mempool_configure() and using a special return value > that skips netdev_change_seq_changed(&dev->up); in the callers, if > the call would be redundant ? [Antonio] sure, I can return EEXIST from netdev_dpdk_mempool_configure(). > > > + err = netdev_dpdk_mempool_configure(dev); > + if (err) { > + return err; > + } else { > + netdev_change_seq_changed(&dev->up); > + } > } > > if (netdev_dpdk_get_vid(dev) >= 0) { > -- > 2.4.11 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__mail.openvswitch.org_mailman_listinfo_ovs- > 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih- > uZnsw&m=eAz1yiSbXvMuAA292xJwliYRlu6n1Y0nqm0MuU2JTXY&s=4gLg6fgwmWYEM4s5v4S0NjY52 > DBbxKTmaQalT8194NE&e= > > >
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 648d719..2f5ec71 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -549,7 +549,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu) dmp->socket_id); if (dmp->mp) { VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name, - dmp->mp_size); + dmp->mp_size); } else if (rte_errno == EEXIST) { /* A mempool with the same name already exists. We just * retrieve its pointer to be returned to the caller. */ @@ -571,6 +571,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu) * initializes some OVS specific fields of dp_packet. */ rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL); + return dmp; } } while (!mp_exists && @@ -3247,11 +3248,14 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev) netdev_dpdk_remap_txqs(dev); - err = netdev_dpdk_mempool_configure(dev); - if (err) { - return err; - } else { - netdev_change_seq_changed(&dev->up); + if (dev->requested_socket_id != dev->socket_id + || dev->requested_mtu != dev->mtu) { + err = netdev_dpdk_mempool_configure(dev); + if (err) { + return err; + } else { + netdev_change_seq_changed(&dev->up); + } } if (netdev_dpdk_get_vid(dev) >= 0) {
In a PVP test where vhostuser ports are configured as clients, OvS crashes when QEMU is launched. This patch avoids the repeated calls to netdev_change_seq_changed after the requested mempool is already acquired. CC: Ciara Loftus <ciara.loftus@intel.com> CC: Kevin Traynor <ktraynor@redhat.com> CC: Aaron Conole <aconole@redhat.com> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.") Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> --- To replicate the bug scenario: PVP test setup -------------- CLIENT_SOCK_DIR=/tmp SOCK0=dpdkvhostuser0 SOCK1=dpdkvhostuser1 1 PMD Add 2 dpdk ports, n_rxq=1 Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-path ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK0" ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK1" Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1 add-flow br0 in_port=1,action=output:3 add-flow br0 in_port=3,action=output:1 add-flow br0 in_port=4,action=output:2 add-flow br0 in_port=2,action=output:4 Launch QEMU ----------- As OvS vhu ports are acting as clients, we must specify 'server' in the next command. VM_IMAGE=<path/to/your/vm/image> sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name us-vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend-file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem -mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev type=vhost-user,id=mynet2,chardev=char1,vhostforce -device virtio-net-pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic Expected behavior ----------------- With this fix OvS shouldn't crash. --- lib/netdev-dpdk.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)