From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: Heng Qi <hengqi@linux.alibaba.com>,
virtio-dev@lists.oasis-open.org,
virtio-comment@lists.oasis-open.org,
Parav Pandit <parav@nvidia.com>,
Yuri Benditovich <yuri.benditovich@daynix.com>,
Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Subject: Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
Date: Fri, 14 Apr 2023 03:18:45 -0400 [thread overview]
Message-ID: <20230414031531-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CACGkMEttCB7J4bd8HoB4z7zQrhZArBHCECWcuuHy2eJVoV7tew@mail.gmail.com>
On Fri, Apr 14, 2023 at 11:10:36AM +0800, Jason Wang wrote:
> 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 is whenever we discard outer header entropy.
> 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
I think this feature is only useful when entropy from inner header
is not exported to outer header, e.g. for legacy GRE
https://datatracker.ietf.org/doc/rfc2784/
When there's entropy in outer header, it's best to just use that.
This will mean that we do not need to keep adding stuff to
spec as more tunneling protocols appear.
>
> >
> > > >
> > > >
> > > > > +
> > > > > +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
> >
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-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@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/
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: Heng Qi <hengqi@linux.alibaba.com>,
virtio-dev@lists.oasis-open.org,
virtio-comment@lists.oasis-open.org,
Parav Pandit <parav@nvidia.com>,
Yuri Benditovich <yuri.benditovich@daynix.com>,
Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Subject: [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
Date: Fri, 14 Apr 2023 03:18:45 -0400 [thread overview]
Message-ID: <20230414031531-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CACGkMEttCB7J4bd8HoB4z7zQrhZArBHCECWcuuHy2eJVoV7tew@mail.gmail.com>
On Fri, Apr 14, 2023 at 11:10:36AM +0800, Jason Wang wrote:
> 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 is whenever we discard outer header entropy.
> 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
I think this feature is only useful when entropy from inner header
is not exported to outer header, e.g. for legacy GRE
https://datatracker.ietf.org/doc/rfc2784/
When there's entropy in outer header, it's best to just use that.
This will mean that we do not need to keep adding stuff to
spec as more tunneling protocols appear.
>
> >
> > > >
> > > >
> > > > > +
> > > > > +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
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
next prev parent reply other threads:[~2023-04-14 7:20 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-03 4:58 [virtio-comment] [PATCH v12] virtio-net: support inner header hash Heng Qi
2023-04-03 4:58 ` [virtio-dev] " Heng Qi
2023-04-03 5:08 ` [virtio-comment] " Heng Qi
2023-04-03 5:08 ` Heng Qi
2023-04-11 12:08 ` [virtio-comment] " Heng Qi
2023-04-11 12:08 ` [virtio-dev] " Heng Qi
2023-04-11 21:03 ` [virtio-comment] " Parav Pandit
2023-04-11 21:03 ` [virtio-dev] " Parav Pandit
2023-04-12 3:05 ` [virtio-comment] " Heng Qi
2023-04-12 3:05 ` [virtio-dev] " Heng Qi
2023-04-12 3:30 ` [virtio-comment] " Parav Pandit
2023-04-12 3:30 ` [virtio-dev] " Parav Pandit
2023-04-13 3:40 ` [virtio-comment] " Jason Wang
2023-04-13 3:40 ` [virtio-dev] " Jason Wang
2023-04-13 11:03 ` [virtio-comment] " Heng Qi
2023-04-13 11:03 ` [virtio-dev] " Heng Qi
2023-04-13 21:43 ` Michael S. Tsirkin
2023-04-13 21:43 ` [virtio-dev] " Michael S. Tsirkin
2023-04-14 3:56 ` [virtio-comment] " Heng Qi
2023-04-14 3:56 ` Heng Qi
2023-04-14 7:06 ` [virtio-comment] " Michael S. Tsirkin
2023-04-14 7:06 ` Michael S. Tsirkin
2023-04-13 21:46 ` Michael S. Tsirkin
2023-04-13 21:46 ` [virtio-dev] " Michael S. Tsirkin
2023-04-14 3:10 ` Jason Wang
2023-04-14 3:10 ` [virtio-dev] " Jason Wang
2023-04-14 4:00 ` [virtio-comment] " Heng Qi
2023-04-14 4:00 ` Heng Qi
2023-04-14 4:08 ` [virtio-comment] " Parav Pandit
2023-04-14 4:08 ` Parav Pandit
2023-04-14 4:54 ` [virtio-comment] " Heng Qi
2023-04-14 4:54 ` Heng Qi
2023-04-14 7:18 ` Michael S. Tsirkin [this message]
2023-04-14 7:18 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230414031531-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=hengqi@linux.alibaba.com \
--cc=jasowang@redhat.com \
--cc=parav@nvidia.com \
--cc=virtio-comment@lists.oasis-open.org \
--cc=virtio-dev@lists.oasis-open.org \
--cc=xuanzhuo@linux.alibaba.com \
--cc=yuri.benditovich@daynix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.