From: "Michael S. Tsirkin" <mst@redhat.com>
To: Parav Pandit <parav@nvidia.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>,
Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
Cornelia Huck <cohuck@redhat.com>
Subject: Re: [virtio-comment] Re: [PATCH v18] virtio-net: support inner header hash
Date: Thu, 22 Jun 2023 12:28:17 -0400 [thread overview]
Message-ID: <20230622120756-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB54815D6D07A49CE775620088DC22A@PH0PR12MB5481.namprd12.prod.outlook.com>
On Thu, Jun 22, 2023 at 12:32:33PM +0000, Parav Pandit wrote:
>
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, June 22, 2023 2:23 AM
> >
> > On Wed, Jun 21, 2023 at 08:52:04PM +0000, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Wednesday, June 21, 2023 4:38 PM
> > >
> > > > > > And the field is RO so no memory cost to exposing it in all VFs.
> > > > > Two structures do not bring the asymmetry.
> > > > > Accessing current and enabled fields via two different mechanism
> > > > > is bringing
> > > > the asymmetry.
> > > >
> > > > I guess it's a matter of taste, but it is clearly more consistent
> > > > with other hash things, to which it's very similar.
> > > >
> > > This is consistent with new commands we define including notification
> > coalescing whose GET is not coming config space.
> >
> > But there GET just reports the current state. Not the read only capability. So
> > there would be cost per VF to keep it in config space.
> > This one is RO no cost per VF. Let's make it convenient?
> >
> And each VF can have different value hence requires per VF storage in the device.
>
> > > >
> > > > Nah, config space is too convenient when we can live with its
> > > > limitations. I don't thin kwe prefer not to keep growing it.
> > > > For some things such as this one it's perfect.
> > > >
> > > Fields are different between different devices.
> >
> > Not sure what's the implication?
> Implication is device needs to store this in always available on-chip memory which is not good.
Oh by devices you mean VFs. Now I get your motivation, at least. Thanks.
> >
> > > > For example, for migration driver might want to validate that two
> > > > devices have same capability. doing it without dma is nicer.
> > > >
> > > A migration driver for real world scenario, will almost have to use the dma for
> > amount of data it needs to exchange.
> >
> > Not migration itself, provisioning.
> >
> Provisioning driver usually do not attach to the member device directly.
> This requires device reset, followed by reaching _DRIVER stage, querying features etc and config area.
> And unbinding it and second reset by member driver. Ugh.
> Provisioning driver also needs to get the state or capabilities even when member driver is already attached.
> So config space is not much a gain either.
>
Absolutely, that's why we have admin commands. I was hoping for an
admin command that basically gets/sets RO fields of the config space.
> > > > Another example, future admin transport will have ability to
> > > > provision devices by supplying their config space.
> > > > This will include this capability automatically, if instead we hide
> > > > it in a command we need to do extra custom work.
> > > >
> > > > > So we do not prefer to keep growing the config space anymore,
> > > > > hence GET is the right approach to me.
> > > >
> > > > Heh I know you hate config space. Let it go, stop wasting time
> > > > arguing about the same thing on every turn and instead help define
> > > > admin transport to solve it
> > >
> > > This was discussed many times, a driver to have a direct (non-intercepted by
> > owner device) channel to device.
> > > If you mean this non-intercepted channel as admin transport, fine.
> >
> > we can do that, sure.
> >
> > > If you mean this is intercepted and it is going over admin cmd, then it is of no
> > use for all future interfaces.
> > >
> > > We discussed this in thread with you and Jason.
> > > I provided concrete example with size and device provisioning math too and
> > other example of multi-physical address VQ.
> > > So transporting register by register over some admin transport is sub-optimal.
> > >
> >
> > Not register by register, we can send all of config space as long as it's RO. This
> > field is.
> >
> It is RO in context of one member device, but every VF can have different value.
> The device will never know if one will use new cmdvq to access or some old driver will use without it.
> And hence, it always needs to provision it on onchip memory for backward compatibility.
>
> Instead of decision point being RO vs RW,
> any new fields via cmdvq and existing fields stays in cfg space, give predictable behavior to size the member devices in the system.
> Once the cmdvq is available, we can get rid of GET command used in this version for new future features.
> Till that arrives, GET command is the efficient way.
I understand. I just don't much like these patchwork solutions though.
And I don't like that we will pay by not having a single conherent
way to provision and query capabilities through config space,
instead just for this thing we will have a special thing.
Why don't we focus on a work on a full solution? Just don't implement
this thing in your devices meanwhile until we do.
> > --
> > 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: Parav Pandit <parav@nvidia.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>,
Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
Cornelia Huck <cohuck@redhat.com>
Subject: [virtio-dev] Re: [virtio-comment] Re: [PATCH v18] virtio-net: support inner header hash
Date: Thu, 22 Jun 2023 12:28:17 -0400 [thread overview]
Message-ID: <20230622120756-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB54815D6D07A49CE775620088DC22A@PH0PR12MB5481.namprd12.prod.outlook.com>
On Thu, Jun 22, 2023 at 12:32:33PM +0000, Parav Pandit wrote:
>
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, June 22, 2023 2:23 AM
> >
> > On Wed, Jun 21, 2023 at 08:52:04PM +0000, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Wednesday, June 21, 2023 4:38 PM
> > >
> > > > > > And the field is RO so no memory cost to exposing it in all VFs.
> > > > > Two structures do not bring the asymmetry.
> > > > > Accessing current and enabled fields via two different mechanism
> > > > > is bringing
> > > > the asymmetry.
> > > >
> > > > I guess it's a matter of taste, but it is clearly more consistent
> > > > with other hash things, to which it's very similar.
> > > >
> > > This is consistent with new commands we define including notification
> > coalescing whose GET is not coming config space.
> >
> > But there GET just reports the current state. Not the read only capability. So
> > there would be cost per VF to keep it in config space.
> > This one is RO no cost per VF. Let's make it convenient?
> >
> And each VF can have different value hence requires per VF storage in the device.
>
> > > >
> > > > Nah, config space is too convenient when we can live with its
> > > > limitations. I don't thin kwe prefer not to keep growing it.
> > > > For some things such as this one it's perfect.
> > > >
> > > Fields are different between different devices.
> >
> > Not sure what's the implication?
> Implication is device needs to store this in always available on-chip memory which is not good.
Oh by devices you mean VFs. Now I get your motivation, at least. Thanks.
> >
> > > > For example, for migration driver might want to validate that two
> > > > devices have same capability. doing it without dma is nicer.
> > > >
> > > A migration driver for real world scenario, will almost have to use the dma for
> > amount of data it needs to exchange.
> >
> > Not migration itself, provisioning.
> >
> Provisioning driver usually do not attach to the member device directly.
> This requires device reset, followed by reaching _DRIVER stage, querying features etc and config area.
> And unbinding it and second reset by member driver. Ugh.
> Provisioning driver also needs to get the state or capabilities even when member driver is already attached.
> So config space is not much a gain either.
>
Absolutely, that's why we have admin commands. I was hoping for an
admin command that basically gets/sets RO fields of the config space.
> > > > Another example, future admin transport will have ability to
> > > > provision devices by supplying their config space.
> > > > This will include this capability automatically, if instead we hide
> > > > it in a command we need to do extra custom work.
> > > >
> > > > > So we do not prefer to keep growing the config space anymore,
> > > > > hence GET is the right approach to me.
> > > >
> > > > Heh I know you hate config space. Let it go, stop wasting time
> > > > arguing about the same thing on every turn and instead help define
> > > > admin transport to solve it
> > >
> > > This was discussed many times, a driver to have a direct (non-intercepted by
> > owner device) channel to device.
> > > If you mean this non-intercepted channel as admin transport, fine.
> >
> > we can do that, sure.
> >
> > > If you mean this is intercepted and it is going over admin cmd, then it is of no
> > use for all future interfaces.
> > >
> > > We discussed this in thread with you and Jason.
> > > I provided concrete example with size and device provisioning math too and
> > other example of multi-physical address VQ.
> > > So transporting register by register over some admin transport is sub-optimal.
> > >
> >
> > Not register by register, we can send all of config space as long as it's RO. This
> > field is.
> >
> It is RO in context of one member device, but every VF can have different value.
> The device will never know if one will use new cmdvq to access or some old driver will use without it.
> And hence, it always needs to provision it on onchip memory for backward compatibility.
>
> Instead of decision point being RO vs RW,
> any new fields via cmdvq and existing fields stays in cfg space, give predictable behavior to size the member devices in the system.
> Once the cmdvq is available, we can get rid of GET command used in this version for new future features.
> Till that arrives, GET command is the efficient way.
I understand. I just don't much like these patchwork solutions though.
And I don't like that we will pay by not having a single conherent
way to provision and query capabilities through config space,
instead just for this thing we will have a special thing.
Why don't we focus on a work on a full solution? Just don't implement
this thing in your devices meanwhile until we do.
> > --
> > 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-06-22 16:28 UTC|newest]
Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-21 13:50 [virtio-comment] [PATCH v18] virtio-net: support inner header hash Heng Qi
2023-06-21 13:50 ` [virtio-dev] " Heng Qi
2023-06-21 15:38 ` [virtio-comment] " Michael S. Tsirkin
2023-06-21 15:38 ` [virtio-dev] " Michael S. Tsirkin
2023-06-21 16:46 ` [virtio-comment] " Heng Qi
2023-06-21 16:46 ` [virtio-dev] " Heng Qi
2023-06-21 17:52 ` Parav Pandit
2023-06-21 17:52 ` [virtio-dev] " Parav Pandit
2023-06-21 19:25 ` Michael S. Tsirkin
2023-06-21 19:25 ` [virtio-dev] " Michael S. Tsirkin
2023-06-21 19:28 ` Parav Pandit
2023-06-21 19:28 ` [virtio-dev] " Parav Pandit
2023-06-21 19:35 ` Michael S. Tsirkin
2023-06-21 19:35 ` [virtio-dev] " Michael S. Tsirkin
2023-06-21 19:39 ` Parav Pandit
2023-06-21 19:39 ` [virtio-dev] " Parav Pandit
2023-06-21 19:45 ` Michael S. Tsirkin
2023-06-21 19:45 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 0:46 ` [virtio-comment] " Heng Qi
2023-06-22 0:46 ` Heng Qi
2023-06-21 19:32 ` Michael S. Tsirkin
2023-06-21 19:32 ` [virtio-dev] " Michael S. Tsirkin
2023-06-21 19:37 ` Parav Pandit
2023-06-21 19:37 ` [virtio-dev] " Parav Pandit
2023-06-21 20:16 ` Michael S. Tsirkin
2023-06-21 20:16 ` [virtio-dev] " Michael S. Tsirkin
2023-06-21 20:24 ` Parav Pandit
2023-06-21 20:24 ` [virtio-dev] " Parav Pandit
2023-06-21 20:37 ` Michael S. Tsirkin
2023-06-21 20:37 ` [virtio-dev] " Michael S. Tsirkin
2023-06-21 20:52 ` Parav Pandit
2023-06-21 20:52 ` [virtio-dev] " Parav Pandit
2023-06-22 0:59 ` [virtio-comment] " Heng Qi
2023-06-22 0:59 ` Heng Qi
2023-06-22 1:04 ` [virtio-comment] " Parav Pandit
2023-06-22 1:04 ` Parav Pandit
2023-06-22 1:17 ` [virtio-comment] " Heng Qi
2023-06-22 1:17 ` Heng Qi
2023-06-22 6:23 ` Michael S. Tsirkin
2023-06-22 6:23 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 12:32 ` Parav Pandit
2023-06-22 12:32 ` [virtio-dev] " Parav Pandit
2023-06-22 13:42 ` Heng Qi
2023-06-22 13:42 ` [virtio-dev] " Heng Qi
2023-06-22 14:27 ` Parav Pandit
2023-06-22 14:27 ` [virtio-dev] " Parav Pandit
2023-06-22 16:46 ` Michael S. Tsirkin
2023-06-22 16:46 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 16:54 ` Parav Pandit
2023-06-22 16:54 ` [virtio-dev] " Parav Pandit
2023-06-22 17:03 ` Michael S. Tsirkin
2023-06-22 17:03 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 17:11 ` Parav Pandit
2023-06-22 17:11 ` [virtio-dev] " Parav Pandit
2023-06-22 17:28 ` Michael S. Tsirkin
2023-06-22 17:28 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 17:58 ` Parav Pandit
2023-06-22 17:58 ` [virtio-dev] " Parav Pandit
2023-06-28 10:41 ` Michael S. Tsirkin
2023-06-28 10:41 ` [virtio-dev] " Michael S. Tsirkin
2023-06-28 16:46 ` Parav Pandit
2023-06-28 16:46 ` [virtio-dev] " Parav Pandit
2023-06-28 17:08 ` Michael S. Tsirkin
2023-06-28 17:08 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 16:28 ` Michael S. Tsirkin [this message]
2023-06-22 16:28 ` Michael S. Tsirkin
2023-06-22 16:42 ` Parav Pandit
2023-06-22 16:42 ` [virtio-dev] " Parav Pandit
2023-06-22 16:54 ` Michael S. Tsirkin
2023-06-22 16:54 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 17:04 ` [virtio-comment] " Parav Pandit
2023-06-22 17:04 ` Parav Pandit
2023-06-22 17:14 ` [virtio-comment] " Michael S. Tsirkin
2023-06-22 17:14 ` Michael S. Tsirkin
2023-06-22 17:20 ` [virtio-comment] " Parav Pandit
2023-06-22 17:20 ` Parav Pandit
2023-06-22 17:43 ` [virtio-comment] " Michael S. Tsirkin
2023-06-22 17:43 ` Michael S. Tsirkin
2023-06-22 18:12 ` [virtio-comment] " Parav Pandit
2023-06-22 18:12 ` Parav Pandit
2023-06-22 18:36 ` [virtio-comment] " Michael S. Tsirkin
2023-06-22 18:36 ` Michael S. Tsirkin
2023-06-22 17:11 ` Michael S. Tsirkin
2023-06-22 17:11 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 17:15 ` Parav Pandit
2023-06-22 17:15 ` [virtio-dev] " Parav Pandit
2023-06-22 17:37 ` Michael S. Tsirkin
2023-06-22 17:37 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 17:51 ` [virtio-comment] " Parav Pandit
2023-06-22 17:51 ` Parav Pandit
2023-06-22 18:11 ` [virtio-comment] " Michael S. Tsirkin
2023-06-22 18:11 ` Michael S. Tsirkin
2023-06-22 18:17 ` [virtio-comment] " Parav Pandit
2023-06-22 18:17 ` Parav Pandit
2023-06-22 18:40 ` [virtio-comment] " Michael S. Tsirkin
2023-06-22 18:40 ` Michael S. Tsirkin
2023-06-22 18:50 ` [virtio-comment] " Parav Pandit
2023-06-22 18:50 ` [virtio-dev] " Parav Pandit
2023-06-22 19:02 ` Michael S. Tsirkin
2023-06-22 19:02 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 20:27 ` Parav Pandit
2023-06-22 20:27 ` [virtio-dev] " Parav Pandit
2023-06-28 10:47 ` Michael S. Tsirkin
2023-06-28 10:47 ` [virtio-dev] " Michael S. Tsirkin
2023-06-22 0:41 ` Heng Qi
2023-06-22 0:41 ` 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=20230622120756-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=cohuck@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.