On Wed, Nov 11, 2015 at 09:24:36PM +0800, Yuanhan Liu wrote: > > Patch 1 rename RESET_DEVICE back to RESET_OWNER > > Patch 2 introduced a new function: vhost_net_reset(), which is invoked > when reset happens, say, driver is unloaded (unbinded). > > Michael suggested to do that only when MQ is not negotiated. > However, reset happens no matter MQ is enabled or negotiated > or not, and we should give a sign to backend to reset some > device to a proper state after it happens.
I don't think it's needed: we set all state at start anyway. > Note that the message sent is still RESET_OWNER. It might not > be a good idea, but we could not simply rename it to RESET_DEVICE, > and maybe adding another RESET_DEVICE might be better. So modern clients don't need this at all. Old clients need something to stop device, but singling out reset is not a good idea: even if driver is unloaded, you need to do that. snabbswitch (ab)uses RESET_OWNER for this, so maybe RESET_OWNER should be renamed DISABLE_ALL, to just make it stop all queues. Does any dpdk version that was released respond to RESET_OWNER in some way? How exactly? > Patch 3 and 4 send SET_PROTOCOL_FEATURES at start, just like we send > SET_FEATURES. > > Patch 5 send SET_VRING_ENABLE at start/stop > > Michael, I intended to send it when MQ is negotiated as you suggested, > however, I found that VHOST_USER_PROTOCOL_F_MQ is defined in > vhost-user.c, > which is not accessible to vhost.c. > > Exporting it to vhost.h will resolve that, however, it's not a good > idea to move vhost user specific stuff to there. We could also > introduce > another vhost callback to test whether MQ is negotiated: I just don't > think it's worthy. > > Hence, here I just used a simple test: invoke set_vring_enable() just > when it is defined. Judging the callback self has another MQ check, > I guess it's okay. > > And sorry that it took so long to send this version. Hmm, you are saying everyone needs SET_VRING_ENABLE? Maybe we should make SET_VRING_ENABLE depend on protocol features then, and not MQ? I applied patches 1 and 5 for now. Thanks! > --- > Yuanhan Liu (5): > vhost: rename RESET_DEVICE backto RESET_OWNER > vhost: reset vhost net when virtio_net_reset happens > vhost: introduce vhost_set/get_protocol_features callbacks > vhost: send SET_PROTOCOL_FEATURES at start > vhost: send SET_VRING_ENABLE at start/stop > > docs/specs/vhost-user.txt | 4 ++-- > hw/net/vhost_net.c | 20 ++++++++++++++------ > hw/net/virtio-net.c | 14 ++++++++++++++ > hw/virtio/vhost-backend.c | 2 +- > hw/virtio/vhost-user.c | 13 ++++++++++--- > hw/virtio/vhost.c | 17 +++++++++++++++++ > include/hw/virtio/vhost-backend.h | 6 ++++++ > include/net/vhost_net.h | 1 + > linux-headers/linux/vhost.h | 2 +- > tests/vhost-user-bridge.c | 6 +++--- > tests/vhost-user-test.c | 4 ++-- > 11 files changed, 71 insertions(+), 18 deletions(-) > > -- > 1.9.0
