virtio-dev.lists.oasis-open.org archive mirror
 help / color / mirror / Atom feed
From: Parav Pandit <parav@nvidia.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Heng Qi <hengqi@linux.alibaba.com>,
	"virtio-comment@lists.oasis-open.org"
	<virtio-comment@lists.oasis-open.org>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	Jason Wang <jasowang@redhat.com>,
	Yuri Benditovich <yuri.benditovich@daynix.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
	"ailan@redhat.com" <ailan@redhat.com>
Subject: RE: [PATCH v9] virtio-net: support inner header hash
Date: Mon, 27 Feb 2023 00:29:33 +0000	[thread overview]
Message-ID: <PH0PR12MB54819632C955C5D087117416DCAF9@PH0PR12MB5481.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20230224030725-mutt-send-email-mst@kernel.org>


> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Friday, February 24, 2023 3:13 AM

[..]
> > The inner hash is only needed for GRE, IPIP etc.
> > For VXLAN and NVGRE Linux kernel transmit side uses the entropy of the
> source port of the outer header.
> > It does that based on the inner header.
> > Refer to [1] as one example.
> >
> > [1]
> > https://elixir.bootlin.com/linux/latest/source/drivers/net/geneve.c#L9
> > 22
> 
> But I think hash was requested for RSS with dpdk, no?
> 
Yes, but if src inserts the entropy (dpdk or kernel), UDP based tunnel can live with outer header hash.
Ip over ip, gre tunnels needs benefit if queues do not overflow or the processing is fast enough as Heng explained.


> > > The lookup will work like this then:
> > >
> > > calculate outer hash
> > > if (rss[outer hash] & tunnel bit)
> > Tunnel bit, you mean tunneled packet, right?
> 
> this idea stores a bit in the indirection table which signals which of the hashes
> to use for rss
> 
> > > then
> > > 	calculate inner hash
> > > 	return rss[inner hash] & ~tunnel bit
> > Why to end with a tunnel bit?
> 
> 
> this just clears the bit so we end up with a vq number.
> 
> > > else
> > > 	return rss[outer hash]
> > >
> > >

Above scheme partitions the rss indirection table into two parts.
1. one for tunnel processing
2. second without it. (this one uses outer hash as today)

When #1 is done in your example, it is without hierarchy.
So inner hash can still result in collision, as before in same VQ. 
Say VQ 0,1,2,3.
Indirection is setup so that 0,1 has tunnel bit set.
2,3 has tunnel bit cleared.
Rss of our hash finds it true and inner hash for two different tunnel is still maps to single VQ.

> > > this fixes the security issue returning us back to status quo :
> > > specific tunnels can be directed to separate queues.
> > >
> > The number of tunnels is far higher than the number of queues with para virt
> driver doing decap.
> 
> True. This seeks to get us back to where we are before the feature:
> driver can send specific outer hashes to specific queues.
> outer hash collisions remain a problem.
> 
So far mlx5 device has done hash on inner header for non udp.

For steering packets to specific queues is done by flow programming to the specific RQs which supports for outer, and inner both.
Ethtool -config-nfc has it for long time too, such flow steering is due for virtio net too.
It is orthogonal to RSS.

> 
> > >
> > > This is for RSS.
> > >
> > >
> > > For hash reporting indirection table is not used.
> > > Maybe it is enough to signal to driver that inner hash was used.
> > > We do need that signalling though.
> > >
> > > My question would be whether it's practical to implement in hardware.
> >
> > In above example, hw calculating double hash is difficult without much gain.
> > Either calculating on one inner or outer makes sense.
> >
> > Signaling whether calculated on inner or outer is fine because hw exactly tells
> what it did.
> 
> This, in a sense, is what reporting hash tunnel type did.
> Do you now think we need it?

I don't see a consumer sw of it.


WARNING: multiple messages have this Message-ID (diff)
From: Parav Pandit <parav@nvidia.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Heng Qi <hengqi@linux.alibaba.com>,
	"virtio-comment@lists.oasis-open.org"
	<virtio-comment@lists.oasis-open.org>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	Jason Wang <jasowang@redhat.com>,
	Yuri Benditovich <yuri.benditovich@daynix.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
	"ailan@redhat.com" <ailan@redhat.com>
Subject: [virtio-dev] RE: [PATCH v9] virtio-net: support inner header hash
Date: Mon, 27 Feb 2023 00:29:33 +0000	[thread overview]
Message-ID: <PH0PR12MB54819632C955C5D087117416DCAF9@PH0PR12MB5481.namprd12.prod.outlook.com> (raw)
Message-ID: <20230227002933.gaxSlKiffcugT3Jhyzb0zqyW9x_JUG_xbrzRA8D54Ho@z> (raw)
In-Reply-To: <20230224030725-mutt-send-email-mst@kernel.org>


> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Friday, February 24, 2023 3:13 AM

[..]
> > The inner hash is only needed for GRE, IPIP etc.
> > For VXLAN and NVGRE Linux kernel transmit side uses the entropy of the
> source port of the outer header.
> > It does that based on the inner header.
> > Refer to [1] as one example.
> >
> > [1]
> > https://elixir.bootlin.com/linux/latest/source/drivers/net/geneve.c#L9
> > 22
> 
> But I think hash was requested for RSS with dpdk, no?
> 
Yes, but if src inserts the entropy (dpdk or kernel), UDP based tunnel can live with outer header hash.
Ip over ip, gre tunnels needs benefit if queues do not overflow or the processing is fast enough as Heng explained.


> > > The lookup will work like this then:
> > >
> > > calculate outer hash
> > > if (rss[outer hash] & tunnel bit)
> > Tunnel bit, you mean tunneled packet, right?
> 
> this idea stores a bit in the indirection table which signals which of the hashes
> to use for rss
> 
> > > then
> > > 	calculate inner hash
> > > 	return rss[inner hash] & ~tunnel bit
> > Why to end with a tunnel bit?
> 
> 
> this just clears the bit so we end up with a vq number.
> 
> > > else
> > > 	return rss[outer hash]
> > >
> > >

Above scheme partitions the rss indirection table into two parts.
1. one for tunnel processing
2. second without it. (this one uses outer hash as today)

When #1 is done in your example, it is without hierarchy.
So inner hash can still result in collision, as before in same VQ. 
Say VQ 0,1,2,3.
Indirection is setup so that 0,1 has tunnel bit set.
2,3 has tunnel bit cleared.
Rss of our hash finds it true and inner hash for two different tunnel is still maps to single VQ.

> > > this fixes the security issue returning us back to status quo :
> > > specific tunnels can be directed to separate queues.
> > >
> > The number of tunnels is far higher than the number of queues with para virt
> driver doing decap.
> 
> True. This seeks to get us back to where we are before the feature:
> driver can send specific outer hashes to specific queues.
> outer hash collisions remain a problem.
> 
So far mlx5 device has done hash on inner header for non udp.

For steering packets to specific queues is done by flow programming to the specific RQs which supports for outer, and inner both.
Ethtool -config-nfc has it for long time too, such flow steering is due for virtio net too.
It is orthogonal to RSS.

> 
> > >
> > > This is for RSS.
> > >
> > >
> > > For hash reporting indirection table is not used.
> > > Maybe it is enough to signal to driver that inner hash was used.
> > > We do need that signalling though.
> > >
> > > My question would be whether it's practical to implement in hardware.
> >
> > In above example, hw calculating double hash is difficult without much gain.
> > Either calculating on one inner or outer makes sense.
> >
> > Signaling whether calculated on inner or outer is fine because hw exactly tells
> what it did.
> 
> This, in a sense, is what reporting hash tunnel type did.
> Do you now think we need it?

I don't see a consumer sw of it.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


  parent reply	other threads:[~2023-02-27  0:29 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-18 14:37 [PATCH v9] virtio-net: support inner header hash Heng Qi
2023-02-20 15:53 ` [virtio-comment] Re: [virtio-dev] " Heng Qi
2023-02-20 16:12   ` Michael S. Tsirkin
2023-02-21  4:20 ` Parav Pandit
2023-02-21  6:14   ` [virtio-comment] " Heng Qi
2023-02-21 12:47     ` Parav Pandit
2023-02-21 13:34       ` Heng Qi
2023-02-21 15:32         ` Parav Pandit
2023-02-21 16:44           ` [virtio-comment] Re: [virtio-dev] " Heng Qi
2023-02-21 16:50             ` Parav Pandit
2023-02-21 17:13               ` Michael S. Tsirkin
2023-02-21 17:40                 ` [virtio-comment] " Parav Pandit
2023-02-21 17:44                   ` Michael S. Tsirkin
2023-02-21 17:54                     ` Parav Pandit
2023-02-21 17:17               ` [virtio-comment] " Heng Qi
2023-02-21 17:39                 ` Parav Pandit
2023-02-21 13:37       ` Heng Qi
2023-02-21 17:05   ` Michael S. Tsirkin
2023-02-21 19:29     ` Parav Pandit
2023-02-21 21:23       ` Michael S. Tsirkin
2023-02-21 21:36         ` Parav Pandit
2023-02-21 21:46           ` Michael S. Tsirkin
2023-02-21 22:32             ` Parav Pandit
2023-02-21 23:18               ` Michael S. Tsirkin
2023-02-22  1:41                 ` Parav Pandit
2023-02-22  2:51                 ` [virtio-dev] " Heng Qi
2023-02-22  2:34       ` [virtio-dev] " Heng Qi
2023-02-22  6:21         ` Michael S. Tsirkin
2023-02-22  7:03           ` Heng Qi
2023-02-22 11:29             ` Michael S. Tsirkin
2023-03-01 14:32   ` [virtio-dev] " Heng Qi
2023-02-21 17:50 ` Michael S. Tsirkin
2023-02-22  3:22   ` Jason Wang
2023-02-22  6:46     ` Heng Qi
2023-02-22 11:30       ` Michael S. Tsirkin
2023-02-23  2:50       ` Jason Wang
2023-02-23  4:41         ` [virtio-dev] " Heng Qi
2023-02-24  2:45           ` Jason Wang
2023-02-24  4:47             ` [virtio-comment] " Heng Qi
2023-02-24  8:07             ` Michael S. Tsirkin
2023-02-23 13:03         ` Michael S. Tsirkin
2023-02-24  2:26           ` Jason Wang
2023-02-24  8:06             ` [virtio-dev] " Michael S. Tsirkin
2023-02-27  4:07               ` Jason Wang
2023-02-27  4:07                 ` [virtio-dev] " Jason Wang
2023-02-27  7:39                 ` Michael S. Tsirkin
2023-02-27  7:39                   ` [virtio-dev] " Michael S. Tsirkin
2023-02-27  8:35                   ` Jason Wang
2023-02-27  8:35                     ` [virtio-dev] " Jason Wang
2023-02-27 12:38                     ` Heng Qi
2023-02-27 12:38                       ` [virtio-dev] " Heng Qi
2023-02-27 17:49                     ` Michael S. Tsirkin
2023-02-27 17:49                       ` [virtio-dev] " Michael S. Tsirkin
2023-02-28  3:04                       ` Jason Wang
2023-02-28  3:04                         ` [virtio-dev] " Jason Wang
2023-02-28  8:52                         ` Michael S. Tsirkin
2023-02-28  8:52                           ` [virtio-dev] " Michael S. Tsirkin
2023-02-28  9:56                           ` Heng Qi
2023-02-28  9:56                             ` Heng Qi
2023-02-28 11:04                         ` Michael S. Tsirkin
2023-02-28 11:04                           ` [virtio-dev] " Michael S. Tsirkin
2023-03-01  2:36                           ` Jason Wang
2023-03-01  2:36                             ` [virtio-dev] " Jason Wang
2023-03-01 10:36                             ` Michael S. Tsirkin
2023-03-02  2:57                               ` Jason Wang
2023-03-02  7:42                                 ` Michael S. Tsirkin
2023-03-02  7:57                                   ` Jason Wang
2023-03-02  8:09                                     ` Michael S. Tsirkin
2023-03-02  8:15                                       ` Jason Wang
2023-03-02  8:41                                         ` Michael S. Tsirkin
2023-03-02  8:59                                           ` Jason Wang
2023-03-02  9:46                                             ` Michael S. Tsirkin
2023-02-23 13:13 ` Michael S. Tsirkin
2023-02-23 14:40   ` [virtio-comment] " Parav Pandit
2023-02-24  8:13     ` Michael S. Tsirkin
2023-02-24 14:38       ` [virtio-dev] " Heng Qi
2023-02-24 17:10         ` Michael S. Tsirkin
2023-02-24 17:10           ` Michael S. Tsirkin
2023-02-27  0:29       ` Parav Pandit [this message]
2023-02-27  0:29         ` [virtio-dev] " Parav Pandit
2023-02-24  4:42   ` Heng Qi
2023-02-24  8:04     ` Michael S. Tsirkin
2023-02-28 11:16 ` Michael S. Tsirkin
2023-02-28 11:16   ` [virtio-dev] " Michael S. Tsirkin
2023-03-01  2:56   ` Heng Qi
2023-03-01  2:56     ` Heng Qi
2023-03-08 14:39     ` [virtio-dev] Re: [virtio-comment] " Michael S. Tsirkin
2023-03-09  4:55       ` Heng Qi
2023-03-09 19:36         ` Michael S. Tsirkin
2023-03-11  3:23           ` Heng Qi
2023-03-15 11:58             ` [virtio-dev] Re: [virtio-comment] " Michael S. Tsirkin
2023-03-15 12:55               ` Heng Qi
2023-03-15 14:57                 ` Michael S. Tsirkin
2023-03-16 13:17                   ` Heng Qi
2023-03-20 19:45                     ` Michael S. Tsirkin
2023-03-30 12:10                       ` Heng Qi
2023-03-20 19:48                 ` Michael S. Tsirkin
2023-03-30 12:37                   ` Heng Qi
2023-04-08 10:29                     ` Michael S. Tsirkin
2023-04-10 13:26                       ` Heng Qi
2023-03-01  3:30   ` [virtio-comment] " Heng Qi
2023-03-01  3:30     ` [virtio-dev] " Heng Qi
2023-03-01 11:07     ` Michael S. Tsirkin
2023-03-01 15:10       ` Heng Qi
2023-03-09 12:28   ` [virtio-dev] " Heng Qi

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=PH0PR12MB54819632C955C5D087117416DCAF9@PH0PR12MB5481.namprd12.prod.outlook.com \
    --to=parav@nvidia.com \
    --cc=ailan@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=hengqi@linux.alibaba.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).