Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash

[Jason Wang <jasowang@redhat.com>]

On Fri, Apr 14, 2023 at 5:46âAM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
> > > >   For example, when the packets of certain
> > > > +tunnels are spread across multiple receive queues, these receive
> > > > queues may have an unbalanced
> > > > +amount of packets. This can cause a specific receive queue to
> > > > become full, resulting in packet loss.
> > >
> > >
> > > We have many places that can lead to packet dropping. For example, the
> > > automatic steering is best effort. I tend to avoid mentioning things
> > > like this.
> >
> > Ok. And Michael what do you think about this?
>
>
> I think this text did not do a great job explaining the
> security aspect. Here's a better, shorter explanation:
>
>         It is often an expectation of users that a tunnel isolates the external
>         network from the internal one. By completely ignoring entropy in the
>         external header and replacing it with entropy from the internal header,
>         for hash calculations, this expectation might be violated to a certain
>         extent, depending on how the hash is used. When the hash use is limited
>         to RSS queue selection, the effect will likely be limited to ability of
>         users inside the tunnel to cause packet drops in multiple queues (as
>         opposed to a single queue without the feature).

And this is only for GRE-in-UDP? This makes me think if we should add
GRE support for the outer header like:

https://docs.napatech.com/r/Feature-Set-N-ANL9/Hash-Key-Type-10-3-Tuple-GREv0

Thanks


>
> > >
> > >
> > > > +
> > > > +Possible mitigations:
> > > > +\begin{itemize}
> > > > +\item Use a tool with good forwarding performance to keep the
> > > > receive queue from filling up.
> > > > +\item If the QoS is unavailable, the driver can set
> > > > \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> > > > +      to disable inner header hash for encapsulated packets.
> > > > +\item Choose a hash key that can avoid queue collisions.
> > > > +\item Perform appropriate QoS before packets consume the receive
> > > > buffers of the receive queues.
> > > > +\end{itemize}
> > > > +
> > > > +The limitations mentioned above exist with/without the inner header
> > > > hash.
> > >
> > >
> > > This conflicts with the tile "Tunnel QoS limitation" which readers may
> > > think it happens only for tunnel.
> >
> > Perhaps a "QoS Advices" is better?
>
> Plural of "advice" is "advice" not "advices".
>
> This advice is somewhat bogus though.
>
> The point I keep trying to make is that this:
>
>         Choose a hash key that can avoid queue collisions.
>
> is impossible with the feature and possible without.
> This was the whole reason I asked for a security
> considerations sections.
>
> --
> MST
>