[virtio-dev] RE: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
> From: Michael S. Tsirkin > Sent: Wednesday, February 8, 2023 9:09 AM > > > header: it allow users inside the tunnel control queueing outside. > > > By observing packet loss some information leaks between tunnels. > > > > > I likely didn't understand. Can you please explain? > > > > Queuing is always done on the inner header with/without encapsulation. > > Hash is always reported for inner header. > > It is only adding the ability to hash even when outer header exists. > > > If hashing just on outer header (currently the only option) then a given > tunnel > all lands in a given queue. > Just keep that queue separate and users of this tunnel can not learn whether > other queues are overflowing, and can not overflow other queues. > > > If you hash inner header then user can flood device with packets of a given > connection and the same connection in a different tunnel hashes to the same > queue. Now one tunnel can > - cause DoS for another tunnel > - cause packet loss or latency triggering possible security bugs within guest > - detect that another tunnel is using the connection by > detecting its own packet loss or increased latency > Yes. It can lead to above issues. Steering on inner is on best effort based sw implementations running on top of net device. To avoid above issues, a hierarchical model is needed. I am not aware of any. To my knowledge, usually who care for above issues end up using a different net device for each VNI and achieve the desired hierarchy. > > > If queuing to be decided based on outer header (hash), then that is > > different. > > Hashing both inner and outer in a flat q structure unlikely works, right? > > Because both hashes can result in different q selection. > > > That's the point. > > Is there any precedent in OSes for configuring things like this that we can > look > at? > ethtool -N (not yet part of virtio) is the closest match that can steer based on inner and outer both, but it is not hierarchical, and it is orthogonal to this feature. > > > > > > > Ideas for solving this they all involve hashing both inner and outer > > > header: > > > 1- report two sets of hashes. overkill? > > > 2- hash both headers together > > > 2- add salt. can come from driver or device itself > > > > > > More ideas? > > > > > > -- > > > MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
On Wed, Feb 08, 2023 at 02:05:52PM +, Parav Pandit wrote: > > From: Michael S. Tsirkin > > Sent: Wednesday, February 8, 2023 8:52 AM > > > > On Wed, Feb 08, 2023 at 01:38:36PM +, Parav Pandit wrote: > > > > > > > From: Michael S. Tsirkin > > > > Sent: Wednesday, February 8, 2023 8:32 AM > > > > > > > > On Wed, Feb 08, 2023 at 05:18:32AM +, Parav Pandit wrote: > > > > > > From: Heng Qi > > > > > > Sent: Tuesday, February 7, 2023 10:25 PM > > > > > > > > > > [..] > > > > > > >> > > > > > > >> Do you think we need both hash_types and hash_tunnel_types? > > > > > > > In struct virtio_net_config we need two fields. > > > > > > > a. supported_hash_types (already exists) b. > > > > > > > supported_hash_tunnel_type > > > > > > > -> bitmap indicating for which outer headers, inner hash > > > > > > > -> calculation is > > > > > > supported. > > > > > > > > > > > > Thanks for the suggestion, we seem to have reached an agreement. > > > > > > > > > > > > > > > > > > > > In struct virtio_net_hdr we need two fields. > > > > > > > a. hash_report (already exists) b. hash_tunnel_type 8 bits -> > > > > > > > absolute value indicating which outer header > > > > > > exists when inner header hash calculated. > > > > > > > You already have it in your patch named as hash_report_tunnel. > > > > > > > May be better to name as hash_report_tunnel_type to make it > > > > > > > clearer that its > > > > > > type. > > > > > > > > > > > > Sure. > > > > > > > > > > > > Thanks for your reply. > > > > > > > > > > I had one last question. Why do we need to inform the > > > > hash_report_tunnel_type of the outer header in the virtio_net_hdr? > > > > > Is this for debug? Or is there a use case that will process this > > > > > value? > > > > > > > > Well we have hash_report which is kind of similar (and also kind of > > > > pointless but I think it's there because WHQL wants it). > > > Hash_report is useful. It tells hash_value is in which namespace > > > (ipv4-tcp/ipv4 > > udp etc). > > > OS can use this value to find tcp connection in a given namespace. > > > > > > > Maybe we can steal some bits > > > > from there instead of a new field? > > > > > > > I do not have problem adding extra bits. I just don't find that just > > > telling that > > its vxlan or nvgre to the OS is useful. > > > If OS needs to know about outer header details, it needs to know the VNI > > information than just telling vxlan. > > > > This does make sense. > > > > > > > > > > > > I have a follow up question though: are we only hashing the inner > > > > header or both inner and outer header? Somewhat confused on this. > > > > > > > I understood as inner header. But worth to describe it. May be there. > > > Need to > > read v8 patch. > > > > Hmm. I just realized that there's a security problem with hashing just the > > inner > > header: it allow users inside the tunnel control queueing outside. > > By observing packet loss some information leaks between tunnels. > > > Ah I know now. > We are leaking outer header information inside the virtio net hdr, and outer > header might be already stripped off by a different entity. > > I think the use case here is it's the same sw entity that owns the virtio net > device does the encap/decap too. No not exactly, we are leaking info between encap tunnels. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
On Wed, Feb 08, 2023 at 02:00:14PM +, Parav Pandit wrote: > > From: Michael S. Tsirkin > > Sent: Wednesday, February 8, 2023 8:52 AM > > > > On Wed, Feb 08, 2023 at 01:38:36PM +, Parav Pandit wrote: > > > > > > > From: Michael S. Tsirkin > > > > Sent: Wednesday, February 8, 2023 8:32 AM > > > > > > > > On Wed, Feb 08, 2023 at 05:18:32AM +, Parav Pandit wrote: > > > > > > From: Heng Qi > > > > > > Sent: Tuesday, February 7, 2023 10:25 PM > > > > > > > > > > [..] > > > > > > >> > > > > > > >> Do you think we need both hash_types and hash_tunnel_types? > > > > > > > In struct virtio_net_config we need two fields. > > > > > > > a. supported_hash_types (already exists) b. > > > > > > > supported_hash_tunnel_type > > > > > > > -> bitmap indicating for which outer headers, inner hash > > > > > > > -> calculation is > > > > > > supported. > > > > > > > > > > > > Thanks for the suggestion, we seem to have reached an agreement. > > > > > > > > > > > > > > > > > > > > In struct virtio_net_hdr we need two fields. > > > > > > > a. hash_report (already exists) b. hash_tunnel_type 8 bits -> > > > > > > > absolute value indicating which outer header > > > > > > exists when inner header hash calculated. > > > > > > > You already have it in your patch named as hash_report_tunnel. > > > > > > > May be better to name as hash_report_tunnel_type to make it > > > > > > > clearer that its > > > > > > type. > > > > > > > > > > > > Sure. > > > > > > > > > > > > Thanks for your reply. > > > > > > > > > > I had one last question. Why do we need to inform the > > > > hash_report_tunnel_type of the outer header in the virtio_net_hdr? > > > > > Is this for debug? Or is there a use case that will process this > > > > > value? > > > > > > > > Well we have hash_report which is kind of similar (and also kind of > > > > pointless but I think it's there because WHQL wants it). > > > Hash_report is useful. It tells hash_value is in which namespace > > > (ipv4-tcp/ipv4 > > udp etc). > > > OS can use this value to find tcp connection in a given namespace. > > > > > > > Maybe we can steal some bits > > > > from there instead of a new field? > > > > > > > I do not have problem adding extra bits. I just don't find that just > > > telling that > > its vxlan or nvgre to the OS is useful. > > > If OS needs to know about outer header details, it needs to know the VNI > > information than just telling vxlan. > > > > This does make sense. > > > > > > > > > > > > I have a follow up question though: are we only hashing the inner > > > > header or both inner and outer header? Somewhat confused on this. > > > > > > > I understood as inner header. But worth to describe it. May be there. > > > Need to > > read v8 patch. > > > > Hmm. I just realized that there's a security problem with hashing just the > > inner > > header: it allow users inside the tunnel control queueing outside. > > By observing packet loss some information leaks between tunnels. > > > I likely didn't understand. Can you please explain? > > Queuing is always done on the inner header with/without encapsulation. > Hash is always reported for inner header. > It is only adding the ability to hash even when outer header exists. If hashing just on outer header (currently the only option) then a given tunnel all lands in a given queue. Just keep that queue separate and users of this tunnel can not learn whether other queues are overflowing, and can not overflow other queues. If you hash inner header then user can flood device with packets of a given connection and the same connection in a different tunnel hashes to the same queue. Now one tunnel can - cause DoS for another tunnel - cause packet loss or latency triggering possible security bugs within guest - detect that another tunnel is using the connection by detecting its own packet loss or increased latency > If queuing to be decided based on outer header (hash), then that is different. > Hashing both inner and outer in a flat q structure unlikely works, right? > Because both hashes can result in different q selection. That's the point. Is there any precedent in OSes for configuring things like this that we can look at? > > > > Ideas for solving this they all involve hashing both inner and outer > > header: > > 1- report two sets of hashes. overkill? > > 2- hash both headers together > > 2- add salt. can come from driver or device itself > > > > More ideas? > > > > -- > > MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
> From: Michael S. Tsirkin > Sent: Wednesday, February 8, 2023 8:52 AM > > On Wed, Feb 08, 2023 at 01:38:36PM +, Parav Pandit wrote: > > > > > From: Michael S. Tsirkin > > > Sent: Wednesday, February 8, 2023 8:32 AM > > > > > > On Wed, Feb 08, 2023 at 05:18:32AM +, Parav Pandit wrote: > > > > > From: Heng Qi > > > > > Sent: Tuesday, February 7, 2023 10:25 PM > > > > > > > > [..] > > > > > >> > > > > > >> Do you think we need both hash_types and hash_tunnel_types? > > > > > > In struct virtio_net_config we need two fields. > > > > > > a. supported_hash_types (already exists) b. > > > > > > supported_hash_tunnel_type > > > > > > -> bitmap indicating for which outer headers, inner hash > > > > > > -> calculation is > > > > > supported. > > > > > > > > > > Thanks for the suggestion, we seem to have reached an agreement. > > > > > > > > > > > > > > > > > In struct virtio_net_hdr we need two fields. > > > > > > a. hash_report (already exists) b. hash_tunnel_type 8 bits -> > > > > > > absolute value indicating which outer header > > > > > exists when inner header hash calculated. > > > > > > You already have it in your patch named as hash_report_tunnel. > > > > > > May be better to name as hash_report_tunnel_type to make it > > > > > > clearer that its > > > > > type. > > > > > > > > > > Sure. > > > > > > > > > > Thanks for your reply. > > > > > > > > I had one last question. Why do we need to inform the > > > hash_report_tunnel_type of the outer header in the virtio_net_hdr? > > > > Is this for debug? Or is there a use case that will process this value? > > > > > > Well we have hash_report which is kind of similar (and also kind of > > > pointless but I think it's there because WHQL wants it). > > Hash_report is useful. It tells hash_value is in which namespace > > (ipv4-tcp/ipv4 > udp etc). > > OS can use this value to find tcp connection in a given namespace. > > > > > Maybe we can steal some bits > > > from there instead of a new field? > > > > > I do not have problem adding extra bits. I just don't find that just > > telling that > its vxlan or nvgre to the OS is useful. > > If OS needs to know about outer header details, it needs to know the VNI > information than just telling vxlan. > > This does make sense. > > > > > > > > I have a follow up question though: are we only hashing the inner > > > header or both inner and outer header? Somewhat confused on this. > > > > > I understood as inner header. But worth to describe it. May be there. Need > > to > read v8 patch. > > Hmm. I just realized that there's a security problem with hashing just the > inner > header: it allow users inside the tunnel control queueing outside. > By observing packet loss some information leaks between tunnels. > Ah I know now. We are leaking outer header information inside the virtio net hdr, and outer header might be already stripped off by a different entity. I think the use case here is it's the same sw entity that owns the virtio net device does the encap/decap too. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
> From: Michael S. Tsirkin > Sent: Wednesday, February 8, 2023 8:52 AM > > On Wed, Feb 08, 2023 at 01:38:36PM +, Parav Pandit wrote: > > > > > From: Michael S. Tsirkin > > > Sent: Wednesday, February 8, 2023 8:32 AM > > > > > > On Wed, Feb 08, 2023 at 05:18:32AM +, Parav Pandit wrote: > > > > > From: Heng Qi > > > > > Sent: Tuesday, February 7, 2023 10:25 PM > > > > > > > > [..] > > > > > >> > > > > > >> Do you think we need both hash_types and hash_tunnel_types? > > > > > > In struct virtio_net_config we need two fields. > > > > > > a. supported_hash_types (already exists) b. > > > > > > supported_hash_tunnel_type > > > > > > -> bitmap indicating for which outer headers, inner hash > > > > > > -> calculation is > > > > > supported. > > > > > > > > > > Thanks for the suggestion, we seem to have reached an agreement. > > > > > > > > > > > > > > > > > In struct virtio_net_hdr we need two fields. > > > > > > a. hash_report (already exists) b. hash_tunnel_type 8 bits -> > > > > > > absolute value indicating which outer header > > > > > exists when inner header hash calculated. > > > > > > You already have it in your patch named as hash_report_tunnel. > > > > > > May be better to name as hash_report_tunnel_type to make it > > > > > > clearer that its > > > > > type. > > > > > > > > > > Sure. > > > > > > > > > > Thanks for your reply. > > > > > > > > I had one last question. Why do we need to inform the > > > hash_report_tunnel_type of the outer header in the virtio_net_hdr? > > > > Is this for debug? Or is there a use case that will process this value? > > > > > > Well we have hash_report which is kind of similar (and also kind of > > > pointless but I think it's there because WHQL wants it). > > Hash_report is useful. It tells hash_value is in which namespace > > (ipv4-tcp/ipv4 > udp etc). > > OS can use this value to find tcp connection in a given namespace. > > > > > Maybe we can steal some bits > > > from there instead of a new field? > > > > > I do not have problem adding extra bits. I just don't find that just > > telling that > its vxlan or nvgre to the OS is useful. > > If OS needs to know about outer header details, it needs to know the VNI > information than just telling vxlan. > > This does make sense. > > > > > > > > I have a follow up question though: are we only hashing the inner > > > header or both inner and outer header? Somewhat confused on this. > > > > > I understood as inner header. But worth to describe it. May be there. Need > > to > read v8 patch. > > Hmm. I just realized that there's a security problem with hashing just the > inner > header: it allow users inside the tunnel control queueing outside. > By observing packet loss some information leaks between tunnels. > I likely didn't understand. Can you please explain? Queuing is always done on the inner header with/without encapsulation. Hash is always reported for inner header. It is only adding the ability to hash even when outer header exists. If queuing to be decided based on outer header (hash), then that is different. Hashing both inner and outer in a flat q structure unlikely works, right? Because both hashes can result in different q selection. > > Ideas for solving this they all involve hashing both inner and outer > header: > 1- report two sets of hashes. overkill? > 2- hash both headers together > 2- add salt. can come from driver or device itself > > More ideas? > > -- > MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
On Wed, Feb 08, 2023 at 01:38:36PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Wednesday, February 8, 2023 8:32 AM > > > > On Wed, Feb 08, 2023 at 05:18:32AM +, Parav Pandit wrote: > > > > From: Heng Qi > > > > Sent: Tuesday, February 7, 2023 10:25 PM > > > > > > [..] > > > > >> > > > > >> Do you think we need both hash_types and hash_tunnel_types? > > > > > In struct virtio_net_config we need two fields. > > > > > a. supported_hash_types (already exists) b. > > > > > supported_hash_tunnel_type > > > > > -> bitmap indicating for which outer headers, inner hash > > > > > -> calculation is > > > > supported. > > > > > > > > Thanks for the suggestion, we seem to have reached an agreement. > > > > > > > > > > > > > > In struct virtio_net_hdr we need two fields. > > > > > a. hash_report (already exists) > > > > > b. hash_tunnel_type 8 bits -> absolute value indicating which > > > > > outer header > > > > exists when inner header hash calculated. > > > > > You already have it in your patch named as hash_report_tunnel. > > > > > May be better to name as hash_report_tunnel_type to make it > > > > > clearer that its > > > > type. > > > > > > > > Sure. > > > > > > > > Thanks for your reply. > > > > > > I had one last question. Why do we need to inform the > > hash_report_tunnel_type of the outer header in the virtio_net_hdr? > > > Is this for debug? Or is there a use case that will process this value? > > > > Well we have hash_report which is kind of similar (and also kind of > > pointless > > but I think it's there because WHQL wants it). > Hash_report is useful. It tells hash_value is in which namespace > (ipv4-tcp/ipv4 udp etc). > OS can use this value to find tcp connection in a given namespace. > > > Maybe we can steal some bits > > from there instead of a new field? > > > I do not have problem adding extra bits. I just don't find that just telling > that its vxlan or nvgre to the OS is useful. > If OS needs to know about outer header details, it needs to know the VNI > information than just telling vxlan. This does make sense. > > > > I have a follow up question though: are we only hashing the inner header or > > both inner and outer header? Somewhat confused on this. > > > I understood as inner header. But worth to describe it. May be there. Need to > read v8 patch. Hmm. I just realized that there's a security problem with hashing just the inner header: it allow users inside the tunnel control queueing outside. By observing packet loss some information leaks between tunnels. Ideas for solving this they all involve hashing both inner and outer header: 1- report two sets of hashes. overkill? 2- hash both headers together 2- add salt. can come from driver or device itself More ideas? -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
> From: Michael S. Tsirkin > Sent: Wednesday, February 8, 2023 8:32 AM > > On Wed, Feb 08, 2023 at 05:18:32AM +, Parav Pandit wrote: > > > From: Heng Qi > > > Sent: Tuesday, February 7, 2023 10:25 PM > > > > [..] > > > >> > > > >> Do you think we need both hash_types and hash_tunnel_types? > > > > In struct virtio_net_config we need two fields. > > > > a. supported_hash_types (already exists) b. > > > > supported_hash_tunnel_type > > > > -> bitmap indicating for which outer headers, inner hash > > > > -> calculation is > > > supported. > > > > > > Thanks for the suggestion, we seem to have reached an agreement. > > > > > > > > > > > In struct virtio_net_hdr we need two fields. > > > > a. hash_report (already exists) > > > > b. hash_tunnel_type 8 bits -> absolute value indicating which > > > > outer header > > > exists when inner header hash calculated. > > > > You already have it in your patch named as hash_report_tunnel. > > > > May be better to name as hash_report_tunnel_type to make it > > > > clearer that its > > > type. > > > > > > Sure. > > > > > > Thanks for your reply. > > > > I had one last question. Why do we need to inform the > hash_report_tunnel_type of the outer header in the virtio_net_hdr? > > Is this for debug? Or is there a use case that will process this value? > > Well we have hash_report which is kind of similar (and also kind of pointless > but I think it's there because WHQL wants it). Hash_report is useful. It tells hash_value is in which namespace (ipv4-tcp/ipv4 udp etc). OS can use this value to find tcp connection in a given namespace. > Maybe we can steal some bits > from there instead of a new field? > I do not have problem adding extra bits. I just don't find that just telling that its vxlan or nvgre to the OS is useful. If OS needs to know about outer header details, it needs to know the VNI information than just telling vxlan. > > I have a follow up question though: are we only hashing the inner header or > both inner and outer header? Somewhat confused on this. > I understood as inner header. But worth to describe it. May be there. Need to read v8 patch. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
On Wed, Feb 08, 2023 at 05:18:32AM +, Parav Pandit wrote: > > From: Heng Qi > > Sent: Tuesday, February 7, 2023 10:25 PM > > [..] > > >> > > >> Do you think we need both hash_types and hash_tunnel_types? > > > In struct virtio_net_config we need two fields. > > > a. supported_hash_types (already exists) b. supported_hash_tunnel_type > > > -> bitmap indicating for which outer headers, inner hash calculation is > > supported. > > > > Thanks for the suggestion, we seem to have reached an agreement. > > > > > > > > In struct virtio_net_hdr we need two fields. > > > a. hash_report (already exists) > > > b. hash_tunnel_type 8 bits -> absolute value indicating which outer header > > exists when inner header hash calculated. > > > You already have it in your patch named as hash_report_tunnel. > > > May be better to name as hash_report_tunnel_type to make it clearer that > > > its > > type. > > > > Sure. > > > > Thanks for your reply. > > I had one last question. Why do we need to inform the hash_report_tunnel_type > of the outer header in the virtio_net_hdr? > Is this for debug? Or is there a use case that will process this value? Well we have hash_report which is kind of similar (and also kind of pointless but I think it's there because WHQL wants it). Maybe we can steal some bits from there instead of a new field? I have a follow up question though: are we only hashing the inner header or both inner and outer header? Somewhat confused on this. In fact, CC Yuri for thoughts and suggestions from windows side of things. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
On Wed, Jan 11, 2023 at 12:45:06PM +0800, Jason Wang wrote:
> On Wed, Jan 11, 2023 at 11:23 AM Heng Qi wrote:
> >
> >
> >
> > 在 2023/1/10 下午3:26, Heng Qi 写道:
> > > On Tue, Jan 10, 2023 at 12:57:38AM -0500, Michael S. Tsirkin wrote:
> > >> On Tue, Jan 10, 2023 at 12:25:02AM -0500, Michael S. Tsirkin wrote:
> > This will give extra pressure on the management stack, e.g it requires
> > the device to have an out of spec way for introspection.
> >
> > Thanks
> > >>> As I tried to explain this is already the case. Feature bits do not
> > >>> describe device capabilities fully, some of them are in config space.
>
> Yes.
>
> > >> To be precise, this does not necessarily require introspection, but
> > >> it does require management control over config space
> > >> such as supported hash types just like it has control over feature bits.
> > >> E.g. QEMU currently seems to hard-code these to
> > >> #define VIRTIO_NET_RSS_SUPPORTED_HASHES (VIRTIO_NET_RSS_HASH_TYPE_IPv4 |
> > >> \
> > >> VIRTIO_NET_RSS_HASH_TYPE_TCPv4
> > >> | \
> > >> VIRTIO_NET_RSS_HASH_TYPE_UDPv4
> > >> | \
> > >> VIRTIO_NET_RSS_HASH_TYPE_IPv6
> > >> | \
> > >> VIRTIO_NET_RSS_HASH_TYPE_TCPv6
> > >> | \
> > >> VIRTIO_NET_RSS_HASH_TYPE_UDPv6
> > >> | \
> > >> VIRTIO_NET_RSS_HASH_TYPE_IP_EX
> > >> | \
> > >>
> > >> VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | \
> > >>
> > >> VIRTIO_NET_RSS_HASH_TYPE_UDP_EX)
> > >>
> > >> but there's no reason not to give management control over these.
>
> Note that the management expects the migration compatibility to work
> with machine types. So it needs a way to disable some tunnel hash
> types to make it work for old machine types.
yes.
This means qemu will need to create properties for these things
and control through machine type compatibility machinery.
For those not hacking qemu - "machine type" is
a string roughly describing a version of guest/host interface used.
> > > Yes, QEMU has requirements for live migration: the PCI config space will
> > > be
> > > checked in get_pci_config_device(), and if src and dst are inconsistent,
> > > it
> > > will prompt that the live migration failed.
>
> It might be too late since it can't work for the second run (unlike
> subsection).
This is really a low level detail of qemu. I'm not sure how important
this is for the spec.
> >
> > To be clearer, I mean \filed{supported_hash_types} in structure
> > virtio_net_config.
>
> Yes.
>
> Thanks
>
> >
> > Thanks.
> >
> > > In fact, this is also done within our group. Live migration requires that
> > > the two VMs have the same rss configuration, otherwise the migration will
> > > fail.
> > >
> > > Therefore, it seems that we can regularize the description of
> > > VIRTIO_NET_F_HASH_TUNNEL into
> > > "[VIRTIO_NET_F_HASH_TUNNEL(52)] Device supports inner header hash for
> > > tunnel-encapsulated packets.",
> > > and use different hash_types to help the migration determine whether it
> > > can succeed.
> > >
> > > Thanks.
> > >
> > >> --
> > >> MST
> > > This publicly archived list offers a means to provide input to the
> > > OASIS Virtual I/O Device (VIRTIO) TC.
> > >
> > > In order to verify user consent to the Feedback License terms and
> > > to minimize spam in the list archive, subscription is required
> > > before posting.
> > >
> > > Subscribe: virtio-comment-subscr...@lists.oasis-open.org
> > > Unsubscribe: virtio-comment-unsubscr...@lists.oasis-open.org
> > > List help: virtio-comment-h...@lists.oasis-open.org
> > > List archive: https://lists.oasis-open.org/archives/virtio-comment/
> > > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> > > List Guidelines:
> > > https://www.oasis-open.org/policies-guidelines/mailing-lists
> > > Committee: https://www.oasis-open.org/committees/virtio/
> > > Join OASIS: https://www.oasis-open.org/join/
> >
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
On Wed, Jan 11, 2023 at 11:23 AM Heng Qi wrote:
>
>
>
> 在 2023/1/10 下午3:26, Heng Qi 写道:
> > On Tue, Jan 10, 2023 at 12:57:38AM -0500, Michael S. Tsirkin wrote:
> >> On Tue, Jan 10, 2023 at 12:25:02AM -0500, Michael S. Tsirkin wrote:
> This will give extra pressure on the management stack, e.g it requires
> the device to have an out of spec way for introspection.
>
> Thanks
> >>> As I tried to explain this is already the case. Feature bits do not
> >>> describe device capabilities fully, some of them are in config space.
Yes.
> >> To be precise, this does not necessarily require introspection, but
> >> it does require management control over config space
> >> such as supported hash types just like it has control over feature bits.
> >> E.g. QEMU currently seems to hard-code these to
> >> #define VIRTIO_NET_RSS_SUPPORTED_HASHES (VIRTIO_NET_RSS_HASH_TYPE_IPv4 | \
> >> VIRTIO_NET_RSS_HASH_TYPE_TCPv4 |
> >> \
> >> VIRTIO_NET_RSS_HASH_TYPE_UDPv4 |
> >> \
> >> VIRTIO_NET_RSS_HASH_TYPE_IPv6 | \
> >> VIRTIO_NET_RSS_HASH_TYPE_TCPv6 |
> >> \
> >> VIRTIO_NET_RSS_HASH_TYPE_UDPv6 |
> >> \
> >> VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> >> \
> >> VIRTIO_NET_RSS_HASH_TYPE_TCP_EX
> >> | \
> >> VIRTIO_NET_RSS_HASH_TYPE_UDP_EX)
> >>
> >> but there's no reason not to give management control over these.
Note that the management expects the migration compatibility to work
with machine types. So it needs a way to disable some tunnel hash
types to make it work for old machine types.
> > Yes, QEMU has requirements for live migration: the PCI config space will be
> > checked in get_pci_config_device(), and if src and dst are inconsistent, it
> > will prompt that the live migration failed.
It might be too late since it can't work for the second run (unlike subsection).
>
> To be clearer, I mean \filed{supported_hash_types} in structure
> virtio_net_config.
Yes.
Thanks
>
> Thanks.
>
> > In fact, this is also done within our group. Live migration requires that
> > the two VMs have the same rss configuration, otherwise the migration will
> > fail.
> >
> > Therefore, it seems that we can regularize the description of
> > VIRTIO_NET_F_HASH_TUNNEL into
> > "[VIRTIO_NET_F_HASH_TUNNEL(52)] Device supports inner header hash for
> > tunnel-encapsulated packets.",
> > and use different hash_types to help the migration determine whether it can
> > succeed.
> >
> > Thanks.
> >
> >> --
> >> MST
> > This publicly archived list offers a means to provide input to the
> > OASIS Virtual I/O Device (VIRTIO) TC.
> >
> > In order to verify user consent to the Feedback License terms and
> > to minimize spam in the list archive, subscription is required
> > before posting.
> >
> > Subscribe: virtio-comment-subscr...@lists.oasis-open.org
> > Unsubscribe: virtio-comment-unsubscr...@lists.oasis-open.org
> > List help: virtio-comment-h...@lists.oasis-open.org
> > List archive: https://lists.oasis-open.org/archives/virtio-comment/
> > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> > List Guidelines:
> > https://www.oasis-open.org/policies-guidelines/mailing-lists
> > Committee: https://www.oasis-open.org/committees/virtio/
> > Join OASIS: https://www.oasis-open.org/join/
>
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash
在 2023/1/10 下午3:26, Heng Qi 写道:
On Tue, Jan 10, 2023 at 12:57:38AM -0500, Michael S. Tsirkin wrote:
On Tue, Jan 10, 2023 at 12:25:02AM -0500, Michael S. Tsirkin wrote:
This will give extra pressure on the management stack, e.g it requires
the device to have an out of spec way for introspection.
Thanks
As I tried to explain this is already the case. Feature bits do not
describe device capabilities fully, some of them are in config space.
To be precise, this does not necessarily require introspection, but
it does require management control over config space
such as supported hash types just like it has control over feature bits.
E.g. QEMU currently seems to hard-code these to
#define VIRTIO_NET_RSS_SUPPORTED_HASHES (VIRTIO_NET_RSS_HASH_TYPE_IPv4 | \
VIRTIO_NET_RSS_HASH_TYPE_TCPv4 | \
VIRTIO_NET_RSS_HASH_TYPE_UDPv4 | \
VIRTIO_NET_RSS_HASH_TYPE_IPv6 | \
VIRTIO_NET_RSS_HASH_TYPE_TCPv6 | \
VIRTIO_NET_RSS_HASH_TYPE_UDPv6 | \
VIRTIO_NET_RSS_HASH_TYPE_IP_EX | \
VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | \
VIRTIO_NET_RSS_HASH_TYPE_UDP_EX)
but there's no reason not to give management control over these.
Yes, QEMU has requirements for live migration: the PCI config space will be
checked in get_pci_config_device(), and if src and dst are inconsistent, it
will prompt that the live migration failed.
To be clearer, I mean \filed{supported_hash_types} in structure
virtio_net_config.
Thanks.
In fact, this is also done within our group. Live migration requires that
the two VMs have the same rss configuration, otherwise the migration will fail.
Therefore, it seems that we can regularize the description of
VIRTIO_NET_F_HASH_TUNNEL into
"[VIRTIO_NET_F_HASH_TUNNEL(52)] Device supports inner header hash for
tunnel-encapsulated packets.",
and use different hash_types to help the migration determine whether it can
succeed.
Thanks.
--
MST
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscr...@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscr...@lists.oasis-open.org
List help: virtio-comment-h...@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
