[virtio-dev] RE: [virtio-comment] Re: [PATCH v10] virtio-net: support inner header hash

2023-03-16 Thread Parav Pandit

> From: Heng Qi 
> Sent: Thursday, March 16, 2023 9:39 AM
> A rough description of this scene is as follows:

> Client1 sends packets to client2, and client2 sends packets to client1
> respectively.
> This is called two directions, and they are in the same flow.
> The packets in the two directions may reach the processing host through
> different tunnels. In this scenario, we need to hash these packets to the same
> queue for the same cpu to process, so inner symmetric hashing is required.
> 
> 
> client1   client2
> | |
> |  __ |
> +->| tunnels |<---+
>|-|
>   |  |
>   |  |
>   |  |
>   v  v
> +-+
> | processing host |
> +-+
> 
I understood now. The key piece is the processing host is different from 
clients 1 and 2, I was missing that.
The key is there are two tunnels and processing data on the inner header for 
the different tunnels is your primary requirement.
Hence inner hashing helps to serialize them.


[virtio-dev] RE: [virtio-comment] Re: [PATCH v10] virtio-net: support inner header hash

2023-03-16 Thread Parav Pandit


> From: Michael S. Tsirkin 
> Sent: Thursday, March 16, 2023 3:36 AM
> 
> On Wed, Mar 15, 2023 at 07:24:57PM -0400, Parav Pandit wrote:
> > > So what's left, GRE?  GRE is actually different, in that it's not IP
> > > at all.
> > Sorry, I wrongly wrote nvegre above.
> >
> > IPoIP, GRE and NVGRE are left.
> >
> > vxlan and geneve has the udp src entropy.
> >
> > >
> > Not sure I understand "its not IP at all".
> >
> 
> GRE header is distinct from IP header and has its own flow ID.

Yes. GRE is IP header + GRE header.
The latest one has the key in GRE header, the older one has only IP.

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



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

2023-03-16 Thread Heng Qi




在 2023/3/16 上午7:24, Parav Pandit 写道:


On 3/15/2023 8:10 AM, Michael S. Tsirkin wrote:

On Tue, Mar 14, 2023 at 11:23:55PM -0400, Parav Pandit wrote:
If not, for now it may be better to skip vxlan and nvegre as they 
inherently

have unique outer header UDP src port based on the inner header.


So what's left, GRE?  GRE is actually different, in that it's not IP at
all.

Sorry, I wrongly wrote nvegre above.

IPoIP, GRE and NVGRE are left.

vxlan and geneve has the udp src entropy.




Not sure I understand "its not IP at all".

GRE has outer IP header + GRE header with the key to identify the flow.
The key is effectively the hash for the flow.


So if we are talking about GRE, hash is indeed not calculated at all at
the moment, right? 
Hash of the outer IP header of the src and dst IP can be still 
calculated currently for GRE when the optional key is not present.



And I would say a natural first step for GRE is
actually adding a hash type that will support this protocol.

For GRE and NVGRE GRE_header.key as the flow/hash identifier should 
work without inner header hash.
Older version of the GRE doesn't have key, so inner header hash is 
useful.


Yes, indeed.
The old GRE does not have keys such as flow id. Even with the new GRE, 
we have no chance to use optional fields,
because we have connected with various operators, and the most basic 
fields can get the best compatibility.


Thanks.




How about doing that? It seems like this should be a small step
and completely uncontroversial.


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



-
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: [PATCH v10] virtio-net: support inner header hash

2023-03-16 Thread Heng Qi
On Wed, Mar 15, 2023 at 07:06:53PM -0400, Parav Pandit wrote:
> 
> 
> On 3/15/2023 9:19 AM, Heng Qi wrote:
> >
> >
> >在 2023/3/15 上午11:23, Parav Pandit 写道:
> >>
> >>
> >>On 3/6/2023 10:48 AM, Heng Qi wrote:
> >>
> [..]
> >>>  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> >>>+\item[VIRTIO_NET_F_HASH_TUNNEL] Requires VIRTIO_NET_F_CTRL_VQ.
> >>I think this should also say that HASH_TUNNEL requires either of
> >>the F_RSS or F_HASH_REPORT.
> >>Because without it HASH_TUNNEL is not useful.
> >
> >F_HASH_TUNNEL indicates that the hash should be calculated using
> >the inner packet header, even without F_RSS or F_HASH_REPORT,
> >we can continue to use the hash value in scenarios such as RPS or
> >ebpf programs.
> Yes.
> Even for rps or ebpf programs, F_HASH_TUNNEL is fine.
> When such feature arrives in future, it above line will have OR
> condition for RPS feature bit.
> 
> >
> >
> >I think it's fine to let F_HASH_TUNNEL rely on F_RSS or
> >_F_HASH_REPORT as those are probably important scenarios where
> >inner packet header hash is used.
> Yes.
> 
> >>If not, for now it may be better to skip vxlan and nvegre as
> >>they inherently have unique outer header UDP src port based on
> >>the inner header.
> >
> >Symmetric hashing ignores the order of the five-tuples when
> >calculating the hash, that is, using (a1,a2,p1,p2) and
> >(a2,a1,p2,p1) respectively can calculate the same hash.
> >There is a scenario that the two directions client1->client2 and
> >client2->client1 of the same flow may pass through different
> >tunnels.
> >In order to allow the data in two directions to be processed by
> >the same CPU, we need to calculate a symmetric hash based on the
> >inner packet header.
> I am lost in two directions and two clients above.
> When you say two directions, do you mean tx and rx?
> and do symmetric hashing between tx and rx between two end points
> within single tunnel?
>

A rough description of this scene is as follows:
Client1 sends packets to client2, and client2 sends packets to client1 
respectively.
This is called two directions, and they are in the same flow.
The packets in the two directions may reach the processing host through
different tunnels. In this scenario, we need to hash these packets to the
same queue for the same cpu to process, so inner symmetric hashing is required.


client1   client2
| |
|  __ |
+->| tunnels |<---+
   |-|
  |  |
  |  |
  |  |
  v  v
+-+
| processing host |
+-+


> >>>+\field{hash_tunnel_types} is set to
> >>>VIRTIO_NET_HASH_TUNNEL_TYPE_NONE by the device for the
> >>>+unencapsulated packets.
> >>>+
> I missed this before. unencapsulated is not a term.
> s/unencapsulated packets/Non encapsulated packets or non tunneled packets
> 

Yes, you are right!

Thanks.

> 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: [PATCH v10] virtio-net: support inner header hash

2023-03-16 Thread Michael S. Tsirkin
On Wed, Mar 15, 2023 at 07:24:57PM -0400, Parav Pandit wrote:
> > So what's left, GRE?  GRE is actually different, in that it's not IP at
> > all.
> Sorry, I wrongly wrote nvegre above.
> 
> IPoIP, GRE and NVGRE are left.
> 
> vxlan and geneve has the udp src entropy.
> 
> > 
> Not sure I understand "its not IP at all".
> 

GRE header is distinct from IP header and has its own flow ID.



-- 
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: [PATCH v10] virtio-net: support inner header hash

2023-03-15 Thread Parav Pandit



On 3/15/2023 8:10 AM, Michael S. Tsirkin wrote:

On Tue, Mar 14, 2023 at 11:23:55PM -0400, Parav Pandit wrote:

If not, for now it may be better to skip vxlan and nvegre as they inherently
have unique outer header UDP src port based on the inner header.


So what's left, GRE?  GRE is actually different, in that it's not IP at
all.

Sorry, I wrongly wrote nvegre above.

IPoIP, GRE and NVGRE are left.

vxlan and geneve has the udp src entropy.




Not sure I understand "its not IP at all".

GRE has outer IP header + GRE header with the key to identify the flow.
The key is effectively the hash for the flow.


So if we are talking about GRE, hash is indeed not calculated at all at
the moment, right?  
Hash of the outer IP header of the src and dst IP can be still 
calculated currently for GRE when the optional key is not present.



And I would say a natural first step for GRE is
actually adding a hash type that will support this protocol.

For GRE and NVGRE GRE_header.key as the flow/hash identifier should work 
without inner header hash.

Older version of the GRE doesn't have key, so inner header hash is useful.


How about doing that? It seems like this should be a small step
and completely uncontroversial.


-
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: [PATCH v10] virtio-net: support inner header hash

2023-03-15 Thread Heng Qi




在 2023/3/15 下午8:10, Michael S. Tsirkin 写道:

On Tue, Mar 14, 2023 at 11:23:55PM -0400, Parav Pandit wrote:

If not, for now it may be better to skip vxlan and nvegre as they inherently
have unique outer header UDP src port based on the inner header.

So what's left, GRE?  GRE is actually different, in that it's not IP at
all.


I do not think so, I mentioned that VXLAN and GENEVE need inner 
symmetric hashing, and we need this.


And we know inner hashing doesn't conflict with other ways of adding 
entropy.


Thanks.


So if we are talking about GRE, hash is indeed not calculated at all at
the moment, right?  And I would say a natural first step for GRE is
actually adding a hash type that will support this protocol.

How about doing that? It seems like this should be a small step
and completely uncontroversial.





-
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: [PATCH v10] virtio-net: support inner header hash

2023-03-15 Thread Michael S. Tsirkin
On Tue, Mar 14, 2023 at 11:23:55PM -0400, Parav Pandit wrote:
> If not, for now it may be better to skip vxlan and nvegre as they inherently
> have unique outer header UDP src port based on the inner header.

So what's left, GRE?  GRE is actually different, in that it's not IP at
all.

So if we are talking about GRE, hash is indeed not calculated at all at
the moment, right?  And I would say a natural first step for GRE is
actually adding a hash type that will support this protocol.

How about doing that? It seems like this should be a small step
and completely uncontroversial.


-- 
MST


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