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

2023-05-14 Thread Heng Qi




在 2023/5/12 下午7:27, Michael S. Tsirkin 写道:

On Fri, May 12, 2023 at 03:23:46PM +0800, Heng Qi wrote:

On Fri, May 12, 2023 at 02:54:34AM -0400, Michael S. Tsirkin wrote:

On Fri, May 12, 2023 at 02:00:19PM +0800, Heng Qi wrote:

On Thu, May 11, 2023 at 02:22:12AM -0400, Michael S. Tsirkin wrote:

On Wed, May 10, 2023 at 05:15:37PM +0800, Heng Qi wrote:


在 2023/5/9 下午11:15, Michael S. Tsirkin 写道:

On Tue, May 09, 2023 at 10:22:19PM +0800, Heng Qi wrote:

在 2023/5/5 下午10:56, Michael S. Tsirkin 写道:

On Fri, May 05, 2023 at 09:51:15PM +0800, Heng Qi wrote:

On Thu, Apr 27, 2023 at 01:13:29PM -0400, Michael S. Tsirkin wrote:

On Thu, Apr 27, 2023 at 10:28:29AM +0800, Heng Qi wrote:

在 2023/4/26 下午10:48, Michael S. Tsirkin 写道:

On Wed, Apr 26, 2023 at 10:14:30PM +0800, Heng Qi wrote:

This does not mean that every device needs to implement and support all of
these, they can choose to support some protocols they want.

I add these because we have scale application scenarios for modern protocols
VXLAN-GPE/GENEVE:

+\item In scenarios where the same flow passing through different tunnels is 
expected to be received in the same queue,
+  warm caches, lessing locking, etc. are optimized to obtain receiving 
performance.


Maybe the legacy GRE, VXLAN-GPE and GENEVE? But it has a little crossover.

Thanks.

But VXLAN-GPE/GENEVE can use source port for entropy.

It is recommended that the UDP source port number
 be calculated using a hash of fields from the inner packet

That is best because
it allows end to end control and is protocol agnostic.

Yes. I agree with this, I don't think we have an argument on this point
right now.:)

For VXLAN-GPE/GENEVE or other modern tunneling protocols, we have to deal
with
scenarios where the same flow passes through different tunnels.

Having them hashed to the same rx queue, is hard to do via outer headers.

All that is missing is symmetric Toepliz and all is well?

The scenarios above or in the commit log also require inner headers.

Hmm I am not sure I get it 100%.
Could you show an example with inner header hash in the port #,
hash is symmetric, and you still have trouble?


It kinds of sounds like not enough entropy is not the problem
at this point.

Sorry for the late reply. :)

For modern tunneling protocols, yes.


You now want to drop everything from the header
except the UDP source port. Is that a fair summary?


For example, for the same flow passing through different VXLAN tunnels,
packets in this flow have the same inner header and different outer
headers. Sometimes these packets of the flow need to be hashed to the
same rxq, then we can use the inner header as the hash input.

Thanks!

So, they will have the same source port yes?

Yes. The outer source port can be calculated using the 5-tuple of the
original packet,
and the outer ports are the same but the outer IPs are different after
different directions of the same flow pass through different tunnels.

Any way to use that

We use it in monitoring, firewall and other scenarios.


so we don't depend on a specific protocol?

Yes, selected tunneling protocols can be used in this scenario like this.

Thanks.


No, the question was - can we generalize this somehow then?
For example, a flag to ignore source IP when hashing?
Or maybe just for UDP packets?

1. I think the common solution is based on the inner header, so that
GRE/IPIP tunnels can also enjoy inner symmetric hashing.

2. The VXLAN spec does not show that the outer source port in both
directions of the same flow must be the same [1]
(although the outer source port is calculated based on the consistent hash
in the kernel. The consistent hash will sort the five-tuple before
calculating hashing),
but it is best not to assume that consistent hashing is used in all VXLAN
implementations.

I agree, best not to assume if it's not in the spec.
The requirement to hash two sides to same queue might
not be necessary for everyone though, right?

The outer source port is also not reliable when it needs to be hashed to
the same queue, but the inner header identifies a flow reliably and
universally.


The GENEVE spec uses "SHOUlD"[2].

What about other tunnels? Could you summarize please?

Sure.

The VXLAN spec[1] does not show that the outer source port in both
directions of the same flow must be the same.

VXLAN-GPE[2]("SHOULD")/GENEVE[3]("SHOULD")/GRE-in-UDP[4.1]/STT[5]
recommend that the outer source port of the same flow be calculated
based on the inner header hash and set to the same.

But the udp source port of GRE-in-UDP may be used in a scenario similar
to NAPT [4.2], where the udp source port is no longer used for entropy,
but for identifying different internal hosts. So using udp source port
does not identify the same stream. This is why using the inner header is
more general, since information about the original stream can reliably
identify a flow.

[1] "Source Port: It is recommended that the UDP source port number be
calculated using a has

[virtio-dev] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ

2023-05-14 Thread Jason Wang



在 2023/5/11 21:04, Michael S. Tsirkin 写道:

On Thu, May 11, 2023 at 03:06:48PM +0800, Jason Wang wrote:

With that I don't see legacy 3 commands anyway conflict with [1].

It doesn't, but it's not elegant since:

1) Modern transport, admin virtqueue with well defined transport commands
2) Legacy transport, works moe like a PCI transport on top of the
admin virtqueue

Transport virtqueue tend to be transport independent, but 2) tie it
somehow to the PCI.

This "somehow" matters though. The connection is really just the
common config layout.


That's why I suggest to

1) introduce legacy transport commands

or

2) tweak your proposal to become a PCI over admin virtqueue

So you are saying it's really not generally "legacy over
admin queue" it is more like "legacy pci over admin queue"?



Yes.



Good point.

And I feel it's a good point that this chooses legacy pci but it could
really be mmio or ccw or the upcoming vq transport just as well.  I suspect
mmio or ccw won't work well for a physical pci card though.



Right, but as discussed, a possible user is the SIOV transitional device 
via transport virtqueues.




transport vq might, but I am worried we'll have to spend extra
work clarifying it's a legacy interface. For example we will
have to explain that only 32 bits of features must be used.
Focusing more effort on transport vq is attractive though ...
It is worth examining this option in more depth.
Did you?



My understanding is, for any case, we need such clarifications. What 
Parav proposed here is not the access to the actual registers but stick 
to the semantic of those registers. I think we must say high 32bits of 
the features must be assumed to be 0.




Want to share how is transport vq closer than
legacy pci to what we might want?



I'm not sure I get the question, but in another thread I show 10+ 
commands to access the legacy facilities which is really very close to 
the transport virtqueue.


Thanks






Some commands functionality is common between [1] and this proposal.
But that's how the legacy is. It is confined to legacy emulation.
So [1] can be done as follow_on to AQ and these series.

A small note about [2].
virtio_transportq_ctrl_dev_attribute should be detached from CREATE call and 
split to two commands.
So that VF and SF/SIOV can both utilize it.
SF/SIOV_R1 can use the creation and config part.
VFs will use only the device features + config space.

Good point. This could be fixed.

Thanks




-
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 v2 0/2] transport-pci: Introduce legacy registers access using AQ

2023-05-14 Thread Jason Wang



在 2023/5/11 22:31, Parav Pandit 写道:

From: Jason Wang 
Sent: Thursday, May 11, 2023 3:17 AM

a configq/cmdq discussion is for new features for PF, VF, and SF/SIOV
devices to work in _non_ backward compatible way, because for new
features there is no such thing as backward compatibility between
guest driver and the device.

Ok, so what command did configq/cmdq have?
I thought it was similar to transport virtqueue, but it looks more like a 
control virtqueue here.

Yes, it is more like a cvq to do device attributes discovery and configuration.
Just that its generic enough.
For example
creating VQ that spans multiple physical addresses.
Discovery and enabling new device attributes which are rarely accessed or 
accessed during device init/cleanup time.

The fundamental difference between cfgvq to tvq is:
Cfgvq is between the member virtio device and its driver.
Tvq=aq is on group owner of the virtio device.


Compatibility is coming at a cost which is not scaling.
If you attempt to scale, it breaks the future path forward to go without

mediation.

So eve growing these fields and optionally placement on configq
doesn't really help and device builder to build it efficiently
(without much predictability).

Config queue is not the only choice, we have a lot of other
choices (for example PASID may help to reduce the on-chip resources).


PASID is for process isolation security construct, it is not for transportation.

I meant with PASID you don't even need a BAR.


Not sure I fully understand, but my guess is, this is coming from
mediation thought process.

Not actually. I meant, for example without PASID, for modern devices, the
common cfg structure must be placed on a BAR through MMIO.

But with the help of PASID, it could stay in the memory and the device can fetch
it via DMA. The help to reduce the resources for registers.


Yes, it can stay in memory and device can fetch via DMA using a VQ using its 
own q from guest driver.
Why does it need PASID?



Let me clarify my understanding.

If we want a per VF virtqueue that is used by the hypervisor. We need 
PASID. Otherwise not.


Thanks


-
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 v2 0/2] transport-pci: Introduce legacy registers access using AQ

2023-05-14 Thread Jason Wang



在 2023/5/11 19:35, Michael S. Tsirkin 写道:

On Thu, May 11, 2023 at 03:20:52PM +0800, Jason Wang wrote:

On Thu, May 11, 2023 at 12:08 AM Parav Pandit  wrote:



On 5/10/2023 12:22 AM, Jason Wang wrote:

On Wed, May 10, 2023 at 11:51 AM Jason Wang  wrote:
Just to make sure we are at the same page.

1) if the hardware has configq and we need to make it work for current
virtio-pci driver, hypervisor needs to trap guest PCI access and
translate it to configq command. This means the hypervisor needs to
hide the configq from guests. In this case the configq needs a
dedicated DMA address which is what PASID can help.
2) if the hardware can report the device states, unless we want to
migrate L2 guest, hypervisor should hide it from L1, so PASID is
required to isolate the DMA for guest traffic and device state.

A configq negotiates + discovers new fields for the PCI PF, VF, SF/SIOV
devices over PCIe or other transports.
So no need to hide/mediate for hw based devices, like cvq and like data vqs.

As discussed, I may misunderstand the function of the configq. If it
works like cvq, then yes. We don't need PASID.

But if we want to report device state via let's say migration q, it
requires PASID since it is used by the hypervisor.

Thanks

I don't exactly get this part. With SRIOV migration q normally would be
part of the PF, accessing hypervisor memory.



Yes, I meant if we have a per VF adminq.

Thanks









For vdpa kind of use case it can work like cvq mediation, which I
believe is happening without PASID today.

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: [RFC PATCH v3] can: virtio: Initial virtio CAN driver.

2023-05-14 Thread Marc Kleine-Budde
On 12.05.2023 19:39:40, Harald Mommer wrote:
> > > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> > > index ff8f76295d13..19314adaff59 100644
> > > --- a/drivers/net/can/Makefile
> > > +++ b/drivers/net/can/Makefile
> > > @@ -17,8 +17,8 @@ obj-$(CONFIG_CAN_AT91)+= at91_can.o
> > >   obj-$(CONFIG_CAN_BXCAN)+= bxcan.o
> > >   obj-$(CONFIG_CAN_CAN327)   += can327.o
> > >   obj-$(CONFIG_CAN_CC770)+= cc770/
> > > -obj-$(CONFIG_CAN_C_CAN)+= c_can/
> > >   obj-$(CONFIG_CAN_CTUCANFD) += ctucanfd/
> > > +obj-$(CONFIG_CAN_C_CAN)+= c_can/
> > This reordering is unrelated to this patch goal. Please send it as a
> > separate patch.
> 
> @Marc Kleine-Budde: We got this reordering change from you.

That reordering was not intended.

> How to proceed?

Remove that change and sorry for the confusion.

> We can split this in 2 commits, reordering and on top adding virtio CAN. No
> issue, a question of minutes and done. Fine. But here the word "patch" was
> used, not the word "commit".

I think in first approximation patches and commits can be/are used
interchangeably :) (One thought process might be: If you make it a
separate commit and send it here, it's no longer in git and thus just a
patch, no longer a commit.)

> Sending a separate patch to somewhere? Maybe
> Mikhail does this fight to get this in (unlikely), I personally would prefer
> to run away. Or we don't reorder at all, wrong ordering remains and we will
> not make only you unhappy.

regards,
Marc

-- 
Pengutronix e.K. | Marc Kleine-Budde  |
Embedded Linux   | https://www.pengutronix.de |
Vertretung Nürnberg  | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |


signature.asc
Description: PGP signature