On Wed, Feb 08, 2023 at 02:05:52PM +0000, Parav Pandit wrote: > > From: Michael S. Tsirkin <m...@redhat.com> > > Sent: Wednesday, February 8, 2023 8:52 AM > > > > On Wed, Feb 08, 2023 at 01:38:36PM +0000, Parav Pandit wrote: > > > > > > > From: Michael S. Tsirkin <m...@redhat.com> > > > > Sent: Wednesday, February 8, 2023 8:32 AM > > > > > > > > On Wed, Feb 08, 2023 at 05:18:32AM +0000, Parav Pandit wrote: > > > > > > From: Heng Qi <hen...@linux.alibaba.com> > > > > > > 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