Discussion of the VIRTIO specification
 help / color / mirror / Atom feed
From: Heng Qi <hengqi@linux.alibaba.com>
To: Parav Pandit <parav@nvidia.com>,
	"virtio-comment@lists.oasis-open.org"
	<virtio-comment@lists.oasis-open.org>
Cc: Shahaf Shuler <shahafs@nvidia.com>,
	"virtio@lists.oasis-open.org" <virtio@lists.oasis-open.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Subject: [virtio-comment] Re: [virtio] [PATCH requirements 5/7] net-features: Add n-tuple receive flow steering requirements
Date: Wed, 14 Jun 2023 11:43:02 +0800	[thread overview]
Message-ID: <7536eeda-ff4e-633a-89bf-aad94aca317a@linux.alibaba.com> (raw)
In-Reply-To: <PH0PR12MB5481978B30D222EE4E3254FBDC55A@PH0PR12MB5481.namprd12.prod.outlook.com>



在 2023/6/13 下午8:24, Parav Pandit 写道:
>
>> From: Heng Qi <hengqi@linux.alibaba.com>
>> Sent: Tuesday, June 13, 2023 1:04 AM
>>
>>
>> 在 2023/6/13 下午12:16, Parav Pandit 写道:
>>>> From: Heng Qi <hengqi@linux.alibaba.com>
>>>> Sent: Monday, June 12, 2023 10:57 PM
>>> [..]
>>>>> +4. The match criteria may optionally also include specific packet
>>>>> +data offset,
>>>> Is this the offset of the packet payload (not including the packet
>>>> header)? If yes, what might be the usage scenarios?
>>>>
>>> Some packet header fields may not be defined initially by us.
>>> For example some protocol FOO type which has some header after UDP
>> header.
>>> And wants to filter and steer to a specific RQ.
>> Ok, I got it.
>>
>>> A virtio may not have defined such protocol at the beginning. So a generic
>> extension allows to steer packet.
>>> At that point line between header and data is blurry. :)
>> Then I think it's just an *offset* from the start of the packet so we can use this
>> capability for the packet header as well.
>>
> Yes, will change it. "packet data" was misleading. It is just the packet offset.
>   
>>>>> +   length, and matching pattern, which may not be defined in the
>>>>> + standard
>>>> RFC.
>>>>> +5. Action includes (a) dropping or (b) forwarding the packet.
>>>>> +6. Destination location is a receive virtqueue index or rss context.
>>>>> +7. The device should process RFS rules before RSS rules, i.e., when there is
>> a
>>>>> +   miss on the RFS rule RSS rule applies.
>>>>> +8. The device should be able to add/remove these rules at a rate of 100K
>>>>> +   rules/sec.
>>>> Why do we need to add this limit for the device please? I think that
>>>> different devices may handle ctrq at different speeds.
>>>>
>>> ARFS rules insertion is fast and tcp streams should be able to converge quicker
>> on the desired cpu.
>>> So rule insertion should be fast enough.
>>> Many of the device can/may be able to do this as some semi-fast path
>> operation unlike CVQ which is typically a device level control operation, often
>> done as slow process.
>>
>> We need to consider ARFS for receive flow filters, but receive flow filters are
>> more infrastructure, do we need to offer a sub-feature to distinguish whether a
>> device supports this dedicated queue or not.
>> This becomes easier for users who only use receive flow filters without ARFS
>> enabled.
>> At this point, ARFS and receive flow filters become orthogonal.
>>
> If it’s a "flowvq" dedicated queue device both the requirements are addressed.
> Device can choose to implement in its preferred way.

I think this is a migration issue:
Devices A and B each offser the VIRTIO_NET_F_RECEIVE_FLOW_FILTER (this 
is a temporary name) feature.
And device A implements flowvq, device B does not implement flowvq, they 
are both using receive flow filters but ARFS is disabled.
The migration should fail at this time. Does this meet user expectations?

Or do we warn in the spec that using flow vq might cause the migration 
issue?

Thanks!

>
>>> So driver-device interface to be able to scale up to low latency and high
>> throughput flow insert/delete ops.
>>
>> Ok.
>>
>>> It is ok one device may choose to not support such high rate, but interface
>> definition wise I imagine to be on its own dedicated queue.
>>>>> +9. If multiple rules are programmed which has overlapping attributes for a
>>>>> +   received packet, the driver to define the location/priority of the rule.
>>>>> +10. The driver should be able to query flow steering table entries
>>>> programmed in
>>>>> +    the device by the flow id.
>>>> Combining points 9 and 10, can I deduce that the attributes of a flow
>>>> rule include identifier (id), position (index of the rule entry) and priority?
>>> Yes, I also think that way.
>>>
>>>> According to point 11, the driver needs to provide a unique id, if
>>>> the location and priority are not provided, they should have predefined
>> default values.
>>> Yes.
>>> I am not fully sure of it as it brings some uncertainty in behavior.
>>> So having second thought as driver to always provide it.
>> Yes, and this point is specific, then we can continue to confirm it in the future
>> spec version.
>>
> Ok. I will revise it in v1.
>
>>>>> +11. The driver should be able to add the entry for attributes (a) match
>>>>> +    criteria, (b) action, (c) destination and (d) assign a unique id of 32 bits.
>>>>> +12. The driver should be able to delete the steering rule entry via
>>>>> +a unique
>>>> id.
>>>>> +13. The driver should be able to add and delete the steering rules in out of
>>>>> +    order manner without depending on previous commands.
>>>>> +14. A group member device should be able to query the attributes of
>>>>> +the
>>>> flow
>>>>> +    steering that device supports.
>>>> Does a group member driver support insertion rules?  The group owner
>>>> driver and a group member driver should have the ability to
>>>> add/remove rules for themselves.
>>>>
>>> Yes, Group owner adds rule for the self, not on behalf.
>>> Same as group member.
>>> No difference between them as they operate on self device.
>> Ok.
>>
>>> In future, a group owner may want to steer the packet towards a group
>> member, like eswitch.
>>
>> Yes.
>> And what I want to say is that our cloud architecture is similar to two-layer
>> virtualization, but we only have one layer of eswitch, so we need VF id to let
>> eswitch provide simple flow steering for the second layer of virtualization, and
>> do not need complicated eswitch.
>> This is the purpose of carrying VF id, but now we can not consider this (VF id) in
>> the feature of receive flow filters, so as not to hinder the progress of this
>> feature.
>>
> Ok. Lets keep this in notes and mind to have the abstraction and don’t jump to bake vf id in the spec in the initial version.


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/


  reply	other threads:[~2023-06-14  3:43 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 22:02 [virtio-comment] [PATCH requirements 0/7] virtio net new features requirements Parav Pandit
2023-06-01 22:02 ` [virtio-comment] [PATCH requirements 1/7] net-features: Add requirements document for release 1.4 Parav Pandit
2023-06-06 22:15   ` Michael S. Tsirkin
2023-06-06 22:28     ` Parav Pandit
2023-06-06 22:56       ` Michael S. Tsirkin
2023-06-06 23:08         ` Parav Pandit
2023-06-06 23:18           ` Michael S. Tsirkin
2023-06-07  9:03             ` [virtio-comment] Re: [virtio] " Xuan Zhuo
2023-06-07 20:35           ` Michael S. Tsirkin
2023-06-07 20:39             ` Parav Pandit
2023-06-07 20:50               ` Michael S. Tsirkin
2023-06-07 20:53                 ` Parav Pandit
2023-06-07  9:31   ` Xuan Zhuo
2023-06-01 22:03 ` [virtio-comment] [PATCH requirements 2/7] net-features: Add low latency transmit queue requirements Parav Pandit
2023-06-06 22:25   ` Michael S. Tsirkin
2023-06-06 22:35     ` Parav Pandit
2023-06-01 22:03 ` [virtio-comment] [PATCH requirements 3/7] net-features: Add low latency receive " Parav Pandit
2023-06-06 22:33   ` Michael S. Tsirkin
2023-06-06 22:44     ` Parav Pandit
2023-06-06 23:03       ` Michael S. Tsirkin
2023-06-01 22:03 ` [virtio-comment] [PATCH requirements 4/7] net-features: Add notification coalescing requirements Parav Pandit
2023-06-06 22:36   ` Michael S. Tsirkin
2023-06-06 22:46     ` Parav Pandit
2023-06-06 23:06       ` Michael S. Tsirkin
2023-06-01 22:03 ` [virtio-comment] [PATCH requirements 5/7] net-features: Add n-tuple receive flow steering requirements Parav Pandit
2023-06-02  3:35   ` Heng Qi
2023-06-02  3:51     ` Parav Pandit
2023-06-02  4:39       ` [virtio-comment] Re: [virtio] " Heng Qi
2023-06-06 12:08         ` Heng Qi
2023-06-06 21:49           ` [virtio-comment] " Parav Pandit
2023-06-12 14:35             ` [virtio-comment] " Heng Qi
2023-06-12 17:26               ` [virtio-comment] " Parav Pandit
2023-06-13  2:28                 ` Heng Qi
2023-06-13  8:57                 ` [virtio-comment] " Michael S. Tsirkin
2023-06-13  9:16                   ` Cornelia Huck
2023-06-13 11:33                   ` [virtio-comment] " Parav Pandit
2023-06-07  2:47   ` Jason Wang
2023-06-07  3:22     ` Parav Pandit
2023-06-13  2:57   ` [virtio-comment] Re: [virtio] " Heng Qi
2023-06-13  4:16     ` [virtio-comment] " Parav Pandit
2023-06-13  5:04       ` [virtio-comment] " Heng Qi
2023-06-13 12:24         ` [virtio-comment] " Parav Pandit
2023-06-14  3:43           ` Heng Qi [this message]
2023-06-14  3:48             ` Parav Pandit
2023-06-14  3:53               ` Heng Qi
2023-06-01 22:03 ` [virtio-comment] [PATCH requirements 6/7] net-features: Add packet timestamp requirements Parav Pandit
2023-06-06 22:40   ` Michael S. Tsirkin
2023-06-06 22:51     ` Parav Pandit
2023-06-06 23:08       ` Michael S. Tsirkin
2023-06-01 22:03 ` [virtio-comment] [PATCH requirements 7/7] net-features: Add header data split requirements Parav Pandit
2023-06-06 22:41   ` Michael S. Tsirkin
2023-06-08 14:57     ` Parav Pandit
2023-06-02  3:06 ` [virtio-comment] Re: [virtio] [PATCH requirements 0/7] virtio net new features requirements Heng Qi
2023-06-06 22:49 ` [virtio-comment] " Michael S. Tsirkin
2023-06-06 22:56   ` Parav Pandit
2023-06-06 23:10     ` Michael S. Tsirkin
2023-06-07  2:49 ` Jason Wang
2023-06-07  3:33   ` Parav Pandit

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=7536eeda-ff4e-633a-89bf-aad94aca317a@linux.alibaba.com \
    --to=hengqi@linux.alibaba.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=parav@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio@lists.oasis-open.org \
    --cc=xuanzhuo@linux.alibaba.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