On Thu, Jun 29, 2023 at 07:54:29AM -0400, Michael S. Tsirkin wrote: > On Thu, Jun 29, 2023 at 11:31:58AM +0800, Jason Wang wrote: > > > > 在 2023/6/28 18:10, Michael S. Tsirkin 写道: > > > On Wed, Jun 28, 2023 at 11:46:22AM +0800, Jason Wang wrote: > > > > On Wed, Jun 28, 2023 at 12:35 AM Heng Qi <[email protected]> > > > > wrote: > > > > > 1. Currently, a received encapsulated packet has an outer and an > > > > > inner header, but > > > > > the virtio device is unable to calculate the hash for the inner > > > > > header. The same > > > > > flow can traverse through different tunnels, resulting in the > > > > > encapsulated > > > > > packets being spread across multiple receive queues (refer to the > > > > > figure below). > > > > > However, in certain scenarios, we may need to direct these > > > > > encapsulated packets of > > > > > the same flow to a single receive queue. This facilitates the > > > > > processing > > > > > of the flow by the same CPU to improve performance (warm caches, less > > > > > locking, etc.). > > > > > > > > > > client1 client2 > > > > > | +-------+ | > > > > > +------->|tunnels|<--------+ > > > > > +-------+ > > > > > | | > > > > > v v > > > > > +-----------------+ > > > > > | monitoring host | > > > > > +-----------------+ > > > > > > > > > > To achieve this, the device can calculate a symmetric hash based on > > > > > the inner headers > > > > > of the same flow. > > > > > > > > > > 2. For legacy systems, they may lack entropy fields which modern > > > > > protocols have in > > > > > the outer header, resulting in multiple flows with the same outer > > > > > header but > > > > > different inner headers being directed to the same receive queue. > > > > > This results in > > > > > poor receive performance. > > > > > > > > > > To address this limitation, inner header hash can be used to enable > > > > > the device to advertise > > > > > the capability to calculate the hash for the inner packet, regaining > > > > > better receive performance. > > > > > > > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/173 > > > > > Signed-off-by: Heng Qi <[email protected]> > > > > > Reviewed-by: Xuan Zhuo <[email protected]> > > > > > Reviewed-by: Parav Pandit <[email protected]> > > > > > --- > > > > > v18->v19: > > > > > 1. Have a single structure instead of two. @Michael S . > > > > > Tsirkin > > > > > 2. Some small rewrites. @Michael S . Tsirkin > > > > > 3. Rebase to master. > > > > > > > > > > v17->v18: > > > > > 1. Some rewording suggestions from Michael (Thanks!). > > > > > 2. Use 0 to disable inner header hash and remove > > > > > VIRTIO_NET_HASH_TUNNEL_TYPE_NONE. > > > > > v16->v17: > > > > > 1. Some small rewrites. @Parav Pandit > > > > > 2. Add Parav's Reviewed-by tag (Thanks!). > > > > > > > > > > v15->v16: > > > > > 1. Remove the hash_option. In order to delimit the inner > > > > > header hash and RSS > > > > > configuration, the ability to configure the outer src udp > > > > > port hash is given > > > > > to RSS. This is orthogonal to inner header hash, which > > > > > will be done in the > > > > > RSS capability extension topic (considered as an RSS > > > > > extension together > > > > > with the symmetric toeplitz hash algorithm, etc.). @Parav > > > > > Pandit @Michael S . Tsirkin > > > > > 2. Fix a 'field' typo. @Parav Pandit > > > > > > > > > > v14->v15: > > > > > 1. Add tunnel hash option suggested by @Michael S . Tsirkin > > > > > 2. Adjust some descriptions. > > > > > > > > > > v13->v14: > > > > > 1. Move supported_hash_tunnel_types from config space into > > > > > cvq command. @Parav Pandit > > > > I may miss some discussions, but this complicates the provisioning a > > > > lot. > > > > > > > > Having it in the config space, then a type agnostic provisioning > > > > through config space + feature bits just works fine. > > > > > > > > If we move it only via cvq, we need device specific provisioning > > > > interface. > > > > > > > > Thanks > > > Yea that's what I said too. Debugging too. I think we should build a > > > consistent solution that allows accessing config space through DMA, > > > separately from this effort. Parav do you think you can live with this > > > approach so this specific proposal can move forward? > > > > > > We can probably go another way, invent a new device configuration space > > capability which fixed size like PCI configuration access capability? > > > > struct virtio_pci_cfg_cap { > > struct virtio_pci_cap cap; > > u8 dev_cfg_data[4]; /* Data for device configuration space access. > > */ > > }; > > > > So it won't grow as the size of device configuration space grows. > > > > Thanks > > It is true, it does not have to be DMA strictly speaking. > > The basic issue is with synchronous access. > > We can change the capability in some way to allow asynch then > that works. E.g. make device change length to 0 and driver > must poll before considering the operation done. > > Having said that, this will increase # of VM exits even more. > Not a fan, DMA seems cleaner.
And again, I am now guilty of this too - hijacking inner hash thread for config space change discussions :( Let's all stop please, start a new thread wen we are ready. -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
