net/vhost-user.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
Fore-commit(c6beefd674) only saves features of queue0,
this makes wrong features of other queues in multiqueues
situation.
For examples:
qemu-system-aarch64 ... \
-chardev socket,id=charnet0,path=/var/run/vhost_sock \
-netdev vhost-user,chardev=charnet0,queues=2,id=hostnet0 \
...
There are two queues in nic assocated with one chardev.
When chardev is reconnected, it is necessary to save and
restore features of all queues.
Signed-of-by: Haibin Zhang <haibinzhang@tencent.com>
---
net/vhost-user.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 17532daaf3..ffbd94d944 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -226,7 +226,7 @@ static void chr_closed_bh(void *opaque)
NetClientState *ncs[MAX_QUEUE_NUM];
NetVhostUserState *s;
Error *err = NULL;
- int queues;
+ int queues, i;
queues = qemu_find_net_clients_except(name, ncs,
NET_CLIENT_DRIVER_NIC,
@@ -235,8 +235,12 @@ static void chr_closed_bh(void *opaque)
s = DO_UPCAST(NetVhostUserState, nc, ncs[0]);
- if (s->vhost_net) {
- s->acked_features = vhost_net_get_acked_features(s->vhost_net);
+ for (i = queues -1; i >= 0; i--) {
+ s = DO_UPCAST(NetVhostUserState, nc, ncs[i]);
+
+ if (s->vhost_net) {
+ s->acked_features = vhost_net_get_acked_features(s->vhost_net);
+ }
}
qmp_set_link(name, false, &err);
--
2.23.0
Thanks Haibin, On 9/22/20 5:27 AM, haibinzhang(张海斌) wrote: > Fore-commit(c6beefd674) only saves features of queue0, > this makes wrong features of other queues in multiqueues > situation. > For examples: > qemu-system-aarch64 ... \ > -chardev socket,id=charnet0,path=/var/run/vhost_sock \ > -netdev vhost-user,chardev=charnet0,queues=2,id=hostnet0 \ > ... > There are two queues in nic assocated with one chardev. > When chardev is reconnected, it is necessary to save and > restore features of all queues. > > Signed-of-by: Haibin Zhang <haibinzhang@tencent.com> > Indeed, this fixes the issue of vhost-user reconnection on multiqueue that I've also reproduced, thanks for posting it. However, I'd like to ask some questions to the experts: - Why do we need a feature negotiation per queue-pair on vhost-user? - Maybe I'm missing something but, isn't the problem that vhost-user device is reusing vhost_net layer (which forces nqueues = 2)? - Won't we have the same issue with vdpa? --- > net/vhost-user.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/net/vhost-user.c b/net/vhost-user.c > index 17532daaf3..ffbd94d944 100644 > --- a/net/vhost-user.c > +++ b/net/vhost-user.c > @@ -226,7 +226,7 @@ static void chr_closed_bh(void *opaque) > NetClientState *ncs[MAX_QUEUE_NUM]; > NetVhostUserState *s; > Error *err = NULL; > - int queues; > + int queues, i; > > queues = qemu_find_net_clients_except(name, ncs, > NET_CLIENT_DRIVER_NIC, > @@ -235,8 +235,12 @@ static void chr_closed_bh(void *opaque) > > s = DO_UPCAST(NetVhostUserState, nc, ncs[0]); > > - if (s->vhost_net) { > - s->acked_features = vhost_net_get_acked_features(s->vhost_net); > + for (i = queues -1; i >= 0; i--) { > + s = DO_UPCAST(NetVhostUserState, nc, ncs[i]); > + > + if (s->vhost_net) { > + s->acked_features = vhost_net_get_acked_features(s->vhost_net); > + } > } > > qmp_set_link(name, false, &err); > Thanks -- Adrián Moreno
On Wed, Sep 23, 2020 at 11:44:41AM +0200, Adrian Moreno wrote: > Thanks Haibin, > > > On 9/22/20 5:27 AM, haibinzhang(张海斌) wrote: > > Fore-commit(c6beefd674) only saves features of queue0, > > this makes wrong features of other queues in multiqueues > > situation. > > For examples: > > qemu-system-aarch64 ... \ > > -chardev socket,id=charnet0,path=/var/run/vhost_sock \ > > -netdev vhost-user,chardev=charnet0,queues=2,id=hostnet0 \ > > ... > > There are two queues in nic assocated with one chardev. > > When chardev is reconnected, it is necessary to save and > > restore features of all queues. > > > > Signed-of-by: Haibin Zhang <haibinzhang@tencent.com> > > > > > Indeed, this fixes the issue of vhost-user reconnection on multiqueue that I've > also reproduced, thanks for posting it. > > However, I'd like to ask some questions to the experts: > - Why do we need a feature negotiation per queue-pair on vhost-user? > - Maybe I'm missing something but, isn't the problem that vhost-user device is > reusing vhost_net layer (which forces nqueues = 2)? > - Won't we have the same issue with vdpa? queued this for now. Jason, what are your thoughts on the questions? > --- > > net/vhost-user.c | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/net/vhost-user.c b/net/vhost-user.c > > index 17532daaf3..ffbd94d944 100644 > > --- a/net/vhost-user.c > > +++ b/net/vhost-user.c > > @@ -226,7 +226,7 @@ static void chr_closed_bh(void *opaque) > > NetClientState *ncs[MAX_QUEUE_NUM]; > > NetVhostUserState *s; > > Error *err = NULL; > > - int queues; > > + int queues, i; > > > > queues = qemu_find_net_clients_except(name, ncs, > > NET_CLIENT_DRIVER_NIC, > > @@ -235,8 +235,12 @@ static void chr_closed_bh(void *opaque) > > > > s = DO_UPCAST(NetVhostUserState, nc, ncs[0]); > > > > - if (s->vhost_net) { > > - s->acked_features = vhost_net_get_acked_features(s->vhost_net); > > + for (i = queues -1; i >= 0; i--) { > > + s = DO_UPCAST(NetVhostUserState, nc, ncs[i]); > > + > > + if (s->vhost_net) { > > + s->acked_features = vhost_net_get_acked_features(s->vhost_net); > > + } > > } > > > > qmp_set_link(name, false, &err); > > > > Thanks > -- > Adrián Moreno
© 2016 - 2024 Red Hat, Inc.