xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Feature control on PV devices
@ 2017-09-07 16:53 Joao Martins
  2017-09-08  8:49 ` Joao Martins
  2017-09-14 16:10 ` Wei Liu
  0 siblings, 2 replies; 11+ messages in thread
From: Joao Martins @ 2017-09-07 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: Boris Ostrovsky, Annie Li, Ankur Arora

Hey!

We wanted to brought up this small proposal regarding the lack of
parameterization on PV devices on Xen.

Currently users don't have a way for enforce and control what
features/queues/etc the backend provides. So far there's only global parameters
on backends, and specs do not mention anything in this regard.

The most obvious example is netback/blkback max_queues module parameter where it
sets the limit the maximum queues for all devices which is not that flexible.
Other examples include controlling offloads visible by the NIC (e.g. disabling
checksum offload, disabling scather-gather), others more about I/O path (e.g.
disable blkif indirect descriptors, limit number of pages for the ring), or less
grant usage by minimizing number of queues/descriptors.

Of course there could be more examples, as this seems to be ortoghonal to the
kinds of PV backends we have. And seems like all features appear to be published
on the same xenbus state?

The idea to address this would be very simple:

- Toolstack when initializing device paths, writes additional entries in the
form of 'request-<feature-name>' = <feature-value>. These entries are only
visible by the backend and toolstack;

- Backend reads this entries and uses <feature-value> as the value of
<feature-name>, which will then be visible on the frontend.

[ Removal of the 'request-*' xenstore entries could represent a feedback look
  that the backend indeed read and used the value. Or else it could simply be
  ignored. ]

And that's it.

In pratice user would do: E.g.

domain.cfg:
...
name = "guest"
kernel = "bzImage"
vif = ["bridge=br0,queues=2"]
disk = [
"format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
]
...

Toolstack writes:

/local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
/local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
/local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0

Backends reads and seeds with (and assuming it passes backend validation ofc):

/local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
/local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
/local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0

The XL configuration entry for controlling these tunable are just examples it's
not clear the general preference for this. An alternative could be:

vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]

Which lets us have more generic feature control, without sticking to particular
features names.

Naturally libvirt could be a consumer of this (as it already has the 'queues'
and host 'tso4', 'tso6', etc in their XML schemas)

Thoughts? Do folks think the correct way of handling this?

Cheers,
Joao

[0] https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L2102

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-07 16:53 Feature control on PV devices Joao Martins
@ 2017-09-08  8:49 ` Joao Martins
  2017-09-14 16:08   ` Joao Martins
  2017-09-14 16:10 ` Wei Liu
  1 sibling, 1 reply; 11+ messages in thread
From: Joao Martins @ 2017-09-08  8:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Boris Ostrovsky, Annie Li, Ankur Arora

[Forgot two important details regarding Xenbus states]

On 09/07/2017 05:53 PM, Joao Martins wrote:
> Hey!
> 
> We wanted to brought up this small proposal regarding the lack of
> parameterization on PV devices on Xen.
> 
> Currently users don't have a way for enforce and control what
> features/queues/etc the backend provides. So far there's only global parameters
> on backends, and specs do not mention anything in this regard.
> 
> The most obvious example is netback/blkback max_queues module parameter where it
> sets the limit the maximum queues for all devices which is not that flexible.
> Other examples include controlling offloads visible by the NIC (e.g. disabling
> checksum offload, disabling scather-gather), others more about I/O path (e.g.
> disable blkif indirect descriptors, limit number of pages for the ring), or less
> grant usage by minimizing number of queues/descriptors.
> 
> Of course there could be more examples, as this seems to be ortoghonal to the
> kinds of PV backends we have. And seems like all features appear to be published
> on the same xenbus state?
> 
> The idea to address this would be very simple:
> 
> - Toolstack when initializing device paths, writes additional entries in the
> form of 'request-<feature-name>' = <feature-value>. These entries are only
> visible by the backend and toolstack;
>
And after that we switch the device state to XenbusStateInitialising as usual.

> 
> - Backend reads this entries and uses <feature-value> as the value of
> <feature-name>, which will then be visible on the frontend.
> 
And after that we switch state to XenbusStateInitWait as usual. No changes are
involved in xenbus state changes other than reading what the toolstack had
written in "request-*" and seed accordingly. Backends without support would
simply ignore these new entries.

> [ Removal of the 'request-*' xenstore entries could represent a feedback look
>   that the backend indeed read and used the value. Or else it could simply be
>   ignored. ]
> 
> And that's it.
> 
> In pratice user would do: E.g.
> 
> domain.cfg:
> ...
> name = "guest"
> kernel = "bzImage"
> vif = ["bridge=br0,queues=2"]
> disk = [
> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
> ]
> ...
> 
> Toolstack writes:
> 
> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0

/local/domain/0/backend/vbd/8/51713/state = 1 (XenbusStateInitialising)

> 
> Backends reads and seeds with (and assuming it passes backend validation ofc):
> 
> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
> 
/local/domain/0/backend/vbd/8/51713/state = 2 (XenbusStateInitWait)

> The XL configuration entry for controlling these tunable are just examples it's
> not clear the general preference for this. An alternative could be:
> 
> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
> 
> Which lets us have more generic feature control, without sticking to particular
> features names.
> 
> Naturally libvirt could be a consumer of this (as it already has the 'queues'
> and host 'tso4', 'tso6', etc in their XML schemas)
> 
> Thoughts? Do folks think the correct way of handling this?
> 
> Cheers,
> Joao
> 
> [0] https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L2102
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-08  8:49 ` Joao Martins
@ 2017-09-14 16:08   ` Joao Martins
  2017-09-18 19:59     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 11+ messages in thread
From: Joao Martins @ 2017-09-14 16:08 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Stefano Stabellini, Ankur Arora, Annie Li,
	Boris Ostrovsky

[ Realized that I didn't CC the maintainers,
  so doing that now, +Linux folks +PV interfaces czar
  Sorry for the noise! ]

On 09/08/2017 09:49 AM, Joao Martins wrote:
> [Forgot two important details regarding Xenbus states]
> On 09/07/2017 05:53 PM, Joao Martins wrote:
>> Hey!
>>
>> We wanted to brought up this small proposal regarding the lack of
>> parameterization on PV devices on Xen.
>>
>> Currently users don't have a way for enforce and control what
>> features/queues/etc the backend provides. So far there's only global parameters
>> on backends, and specs do not mention anything in this regard.
>>
>> The most obvious example is netback/blkback max_queues module parameter where it
>> sets the limit the maximum queues for all devices which is not that flexible.
>> Other examples include controlling offloads visible by the NIC (e.g. disabling
>> checksum offload, disabling scather-gather), others more about I/O path (e.g.
>> disable blkif indirect descriptors, limit number of pages for the ring), or less
>> grant usage by minimizing number of queues/descriptors.
>>
>> Of course there could be more examples, as this seems to be ortoghonal to the
>> kinds of PV backends we have. And seems like all features appear to be published
>> on the same xenbus state?
>>
>> The idea to address this would be very simple:
>>
>> - Toolstack when initializing device paths, writes additional entries in the
>> form of 'request-<feature-name>' = <feature-value>. These entries are only
>> visible by the backend and toolstack;
>>
> And after that we switch the device state to XenbusStateInitialising as usual.
> 
>>
>> - Backend reads this entries and uses <feature-value> as the value of
>> <feature-name>, which will then be visible on the frontend.
>>
> And after that we switch state to XenbusStateInitWait as usual. No changes are
> involved in xenbus state changes other than reading what the toolstack had
> written in "request-*" and seed accordingly. Backends without support would
> simply ignore these new entries.
> 
>> [ Removal of the 'request-*' xenstore entries could represent a feedback look
>>   that the backend indeed read and used the value. Or else it could simply be
>>   ignored. ]
>>
>> And that's it.
>>
>> In pratice user would do: E.g.
>>
>> domain.cfg:
>> ...
>> name = "guest"
>> kernel = "bzImage"
>> vif = ["bridge=br0,queues=2"]
>> disk = [
>> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
>> ]
>> ...
>>
>> Toolstack writes:
>>
>> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0
> 
> /local/domain/0/backend/vbd/8/51713/state = 1 (XenbusStateInitialising)
> 
>>
>> Backends reads and seeds with (and assuming it passes backend validation ofc):
>>
>> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
>>
> /local/domain/0/backend/vbd/8/51713/state = 2 (XenbusStateInitWait)
> 
>> The XL configuration entry for controlling these tunable are just examples it's
>> not clear the general preference for this. An alternative could be:
>>
>> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
>>
>> Which lets us have more generic feature control, without sticking to particular
>> features names.
>>
>> Naturally libvirt could be a consumer of this (as it already has the 'queues'
>> and host 'tso4', 'tso6', etc in their XML schemas)
>>
>> Thoughts? Do folks think the correct way of handling this?
>>
>> Cheers,
>> Joao
>>
>> [0] https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L2102
>>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-07 16:53 Feature control on PV devices Joao Martins
  2017-09-08  8:49 ` Joao Martins
@ 2017-09-14 16:10 ` Wei Liu
  2017-09-14 16:18   ` Joao Martins
  1 sibling, 1 reply; 11+ messages in thread
From: Wei Liu @ 2017-09-14 16:10 UTC (permalink / raw)
  To: Joao Martins; +Cc: xen-devel, Boris Ostrovsky, Wei Liu, Annie Li, Ankur Arora

On Thu, Sep 07, 2017 at 05:53:54PM +0100, Joao Martins wrote:
> Hey!
> 
> We wanted to brought up this small proposal regarding the lack of
> parameterization on PV devices on Xen.
> 
> Currently users don't have a way for enforce and control what
> features/queues/etc the backend provides. So far there's only global parameters
> on backends, and specs do not mention anything in this regard.
> 
> The most obvious example is netback/blkback max_queues module parameter where it
> sets the limit the maximum queues for all devices which is not that flexible.
> Other examples include controlling offloads visible by the NIC (e.g. disabling
> checksum offload, disabling scather-gather), others more about I/O path (e.g.
> disable blkif indirect descriptors, limit number of pages for the ring), or less
> grant usage by minimizing number of queues/descriptors.
> 
> Of course there could be more examples, as this seems to be ortoghonal to the
> kinds of PV backends we have. And seems like all features appear to be published
> on the same xenbus state?
> 
> The idea to address this would be very simple:
> 
> - Toolstack when initializing device paths, writes additional entries in the
> form of 'request-<feature-name>' = <feature-value>. These entries are only
> visible by the backend and toolstack;
> 
> - Backend reads this entries and uses <feature-value> as the value of
> <feature-name>, which will then be visible on the frontend.
> 
> [ Removal of the 'request-*' xenstore entries could represent a feedback look
>   that the backend indeed read and used the value. Or else it could simply be
>   ignored. ]
> 
> And that's it.
> 
> In pratice user would do: E.g.
> 
> domain.cfg:
> ...
> name = "guest"
> kernel = "bzImage"
> vif = ["bridge=br0,queues=2"]
> disk = [
> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"

There needs to be a way to distinguish parameters consumed by toolstack
vs the ones passed on to backends. The parameters passed to backends
should start with a predefined prefix.

> ]
> ...
> 
> Toolstack writes:
> 
> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0
> 
> Backends reads and seeds with (and assuming it passes backend validation ofc):
> 
> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
> 
> The XL configuration entry for controlling these tunable are just examples it's
> not clear the general preference for this. An alternative could be:
> 
> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
> 
> Which lets us have more generic feature control, without sticking to particular
> features names.
> 
> Naturally libvirt could be a consumer of this (as it already has the 'queues'
> and host 'tso4', 'tso6', etc in their XML schemas)
> 
> Thoughts? Do folks think the correct way of handling this?
> 

I think having a way to control backend features in xl/libxl is a good
thing.

> Cheers,
> Joao
> 
> [0] https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L2102
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-14 16:10 ` Wei Liu
@ 2017-09-14 16:18   ` Joao Martins
  2017-09-15 11:19     ` Wei Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Joao Martins @ 2017-09-14 16:18 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, Boris Ostrovsky, Annie Li, Ankur Arora

On 09/14/2017 05:10 PM, Wei Liu wrote:
> On Thu, Sep 07, 2017 at 05:53:54PM +0100, Joao Martins wrote:
>> Hey!
>>
>> We wanted to brought up this small proposal regarding the lack of
>> parameterization on PV devices on Xen.
>>
>> Currently users don't have a way for enforce and control what
>> features/queues/etc the backend provides. So far there's only global parameters
>> on backends, and specs do not mention anything in this regard.
>>
>> The most obvious example is netback/blkback max_queues module parameter where it
>> sets the limit the maximum queues for all devices which is not that flexible.
>> Other examples include controlling offloads visible by the NIC (e.g. disabling
>> checksum offload, disabling scather-gather), others more about I/O path (e.g.
>> disable blkif indirect descriptors, limit number of pages for the ring), or less
>> grant usage by minimizing number of queues/descriptors.
>>
>> Of course there could be more examples, as this seems to be ortoghonal to the
>> kinds of PV backends we have. And seems like all features appear to be published
>> on the same xenbus state?
>>
>> The idea to address this would be very simple:
>>
>> - Toolstack when initializing device paths, writes additional entries in the
>> form of 'request-<feature-name>' = <feature-value>. These entries are only
>> visible by the backend and toolstack;
>>
>> - Backend reads this entries and uses <feature-value> as the value of
>> <feature-name>, which will then be visible on the frontend.
>>
>> [ Removal of the 'request-*' xenstore entries could represent a feedback look
>>   that the backend indeed read and used the value. Or else it could simply be
>>   ignored. ]
>>
>> And that's it.
>>
>> In pratice user would do: E.g.
>>
>> domain.cfg:
>> ...
>> name = "guest"
>> kernel = "bzImage"
>> vif = ["bridge=br0,queues=2"]
>> disk = [
>> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
> 
> There needs to be a way to distinguish parameters consumed by toolstack
> vs the ones passed on to backends. The parameters passed to backends
> should start with a predefined prefix.
> 
Hmm, which seems to be inline with the "request" prefix when controlling certain
features enabled/disabled? Oh wait, perhaps you mean wrt to the UI/config-format
rather than xenstore entries and such? If it's the latter, see below,

>> ]
>> ...
>>
>> Toolstack writes:
>>
>> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0
>>
>> Backends reads and seeds with (and assuming it passes backend validation ofc):
>>
>> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
>> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
>>
>> The XL configuration entry for controlling these tunable are just examples it's
>> not clear the general preference for this. An alternative could be:
>>
>> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
>>
>> Which lets us have more generic feature control, without sticking to particular
>> features names.
>>

In case the above was about config format, this one suggested above sounds more
general, and easy to reuse across backends. Maybe instead of "features", could
be "backend_features" since, most PV backends declare a "backend" and a
"backend_id" as per libxl IDL.

>> Naturally libvirt could be a consumer of this (as it already has the 'queues'
>> and host 'tso4', 'tso6', etc in their XML schemas)
>>
>> Thoughts? Do folks think the correct way of handling this?
>>
> 
> I think having a way to control backend features in xl/libxl is a good
> thing.

Thanks!

> 
>> Cheers,
>> Joao
>>
>> [0] https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L2102

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-14 16:18   ` Joao Martins
@ 2017-09-15 11:19     ` Wei Liu
  2017-09-15 11:34       ` Juergen Gross
  0 siblings, 1 reply; 11+ messages in thread
From: Wei Liu @ 2017-09-15 11:19 UTC (permalink / raw)
  To: Joao Martins; +Cc: xen-devel, Boris Ostrovsky, Wei Liu, Annie Li, Ankur Arora

On Thu, Sep 14, 2017 at 05:18:44PM +0100, Joao Martins wrote:
> On 09/14/2017 05:10 PM, Wei Liu wrote:
> > On Thu, Sep 07, 2017 at 05:53:54PM +0100, Joao Martins wrote:
> >> Hey!
> >>
> >> We wanted to brought up this small proposal regarding the lack of
> >> parameterization on PV devices on Xen.
> >>
> >> Currently users don't have a way for enforce and control what
> >> features/queues/etc the backend provides. So far there's only global parameters
> >> on backends, and specs do not mention anything in this regard.
> >>
> >> The most obvious example is netback/blkback max_queues module parameter where it
> >> sets the limit the maximum queues for all devices which is not that flexible.
> >> Other examples include controlling offloads visible by the NIC (e.g. disabling
> >> checksum offload, disabling scather-gather), others more about I/O path (e.g.
> >> disable blkif indirect descriptors, limit number of pages for the ring), or less
> >> grant usage by minimizing number of queues/descriptors.
> >>
> >> Of course there could be more examples, as this seems to be ortoghonal to the
> >> kinds of PV backends we have. And seems like all features appear to be published
> >> on the same xenbus state?
> >>
> >> The idea to address this would be very simple:
> >>
> >> - Toolstack when initializing device paths, writes additional entries in the
> >> form of 'request-<feature-name>' = <feature-value>. These entries are only
> >> visible by the backend and toolstack;
> >>
> >> - Backend reads this entries and uses <feature-value> as the value of
> >> <feature-name>, which will then be visible on the frontend.
> >>
> >> [ Removal of the 'request-*' xenstore entries could represent a feedback look
> >>   that the backend indeed read and used the value. Or else it could simply be
> >>   ignored. ]
> >>
> >> And that's it.
> >>
> >> In pratice user would do: E.g.
> >>
> >> domain.cfg:
> >> ...
> >> name = "guest"
> >> kernel = "bzImage"
> >> vif = ["bridge=br0,queues=2"]
> >> disk = [
> >> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
> > 
> > There needs to be a way to distinguish parameters consumed by toolstack
> > vs the ones passed on to backends. The parameters passed to backends
> > should start with a predefined prefix.
> > 
> Hmm, which seems to be inline with the "request" prefix when controlling certain
> features enabled/disabled? Oh wait, perhaps you mean wrt to the UI/config-format
> rather than xenstore entries and such? If it's the latter, see below,

I was thinking about xl config syntax.

> 
> >> ]
> >> ...
> >>
> >> Toolstack writes:
> >>
> >> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0
> >>
> >> Backends reads and seeds with (and assuming it passes backend validation ofc):
> >>
> >> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
> >>
> >> The XL configuration entry for controlling these tunable are just examples it's
> >> not clear the general preference for this. An alternative could be:
> >>
> >> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
> >>
> >> Which lets us have more generic feature control, without sticking to particular
> >> features names.
> >>
> 
> In case the above was about config format, this one suggested above sounds more
> general, and easy to reuse across backends. Maybe instead of "features", could
> be "backend_features" since, most PV backends declare a "backend" and a
> "backend_id" as per libxl IDL.
> 

The proposed syntax looks a bit difficult to parse.

What's wrong with request-XXX=YYY syntax? We can have many of those as
we like. Xl just picks those and concatenate them into backend_features.

Assuming we just dump things into backend_features, once the syntax is
determined, we can only extend it but not change it because we need to
maintain backward-compatibility in both xl and libxl.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-15 11:19     ` Wei Liu
@ 2017-09-15 11:34       ` Juergen Gross
  2017-09-15 12:46         ` Wei Liu
  2017-09-15 13:56         ` Joao Martins
  0 siblings, 2 replies; 11+ messages in thread
From: Juergen Gross @ 2017-09-15 11:34 UTC (permalink / raw)
  To: Wei Liu, Joao Martins; +Cc: xen-devel, Boris Ostrovsky, Annie Li, Ankur Arora

On 15/09/17 13:19, Wei Liu wrote:
> On Thu, Sep 14, 2017 at 05:18:44PM +0100, Joao Martins wrote:
>> On 09/14/2017 05:10 PM, Wei Liu wrote:
>>> On Thu, Sep 07, 2017 at 05:53:54PM +0100, Joao Martins wrote:
>>>> Hey!
>>>>
>>>> We wanted to brought up this small proposal regarding the lack of
>>>> parameterization on PV devices on Xen.
>>>>
>>>> Currently users don't have a way for enforce and control what
>>>> features/queues/etc the backend provides. So far there's only global parameters
>>>> on backends, and specs do not mention anything in this regard.
>>>>
>>>> The most obvious example is netback/blkback max_queues module parameter where it
>>>> sets the limit the maximum queues for all devices which is not that flexible.
>>>> Other examples include controlling offloads visible by the NIC (e.g. disabling
>>>> checksum offload, disabling scather-gather), others more about I/O path (e.g.
>>>> disable blkif indirect descriptors, limit number of pages for the ring), or less
>>>> grant usage by minimizing number of queues/descriptors.
>>>>
>>>> Of course there could be more examples, as this seems to be ortoghonal to the
>>>> kinds of PV backends we have. And seems like all features appear to be published
>>>> on the same xenbus state?
>>>>
>>>> The idea to address this would be very simple:
>>>>
>>>> - Toolstack when initializing device paths, writes additional entries in the
>>>> form of 'request-<feature-name>' = <feature-value>. These entries are only
>>>> visible by the backend and toolstack;
>>>>
>>>> - Backend reads this entries and uses <feature-value> as the value of
>>>> <feature-name>, which will then be visible on the frontend.
>>>>
>>>> [ Removal of the 'request-*' xenstore entries could represent a feedback look
>>>>   that the backend indeed read and used the value. Or else it could simply be
>>>>   ignored. ]
>>>>
>>>> And that's it.
>>>>
>>>> In pratice user would do: E.g.
>>>>
>>>> domain.cfg:
>>>> ...
>>>> name = "guest"
>>>> kernel = "bzImage"
>>>> vif = ["bridge=br0,queues=2"]
>>>> disk = [
>>>> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
>>>
>>> There needs to be a way to distinguish parameters consumed by toolstack
>>> vs the ones passed on to backends. The parameters passed to backends
>>> should start with a predefined prefix.
>>>
>> Hmm, which seems to be inline with the "request" prefix when controlling certain
>> features enabled/disabled? Oh wait, perhaps you mean wrt to the UI/config-format
>> rather than xenstore entries and such? If it's the latter, see below,
> 
> I was thinking about xl config syntax.
> 
>>
>>>> ]
>>>> ...
>>>>
>>>> Toolstack writes:
>>>>
>>>> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
>>>> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
>>>> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0

I'd rather use a specific directory, e.g.:

/local/domain/0/backend/vif/8/0/request/multi-queue-max-queues = 2
/local/domain/0/backend/vbd/8/51713/request/multi-queue-max-queues = 2
/local/domain/0/backend/vbd/8/51713/request/max-ring-page-order = 0

This will enable the backend to just look for all entries in
.../request/ instead of having to try all possible features.

>>>>
>>>> Backends reads and seeds with (and assuming it passes backend validation ofc):
>>>>
>>>> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
>>>> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
>>>> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
>>>>
>>>> The XL configuration entry for controlling these tunable are just examples it's
>>>> not clear the general preference for this. An alternative could be:
>>>>
>>>> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
>>>>
>>>> Which lets us have more generic feature control, without sticking to particular
>>>> features names.
>>>>
>>
>> In case the above was about config format, this one suggested above sounds more
>> general, and easy to reuse across backends. Maybe instead of "features", could
>> be "backend_features" since, most PV backends declare a "backend" and a
>> "backend_id" as per libxl IDL.
>>
> 
> The proposed syntax looks a bit difficult to parse.
> 
> What's wrong with request-XXX=YYY syntax? We can have many of those as
> we like. Xl just picks those and concatenate them into backend_features.

Is it possible to parse those without having to know about individual
XXX values? Otherwise we'd be able to support only features known by xl
instead of those known by the various backends.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-15 11:34       ` Juergen Gross
@ 2017-09-15 12:46         ` Wei Liu
  2017-09-15 13:56         ` Joao Martins
  1 sibling, 0 replies; 11+ messages in thread
From: Wei Liu @ 2017-09-15 12:46 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Wei Liu, Ankur Arora, Annie Li, xen-devel, Joao Martins,
	Boris Ostrovsky

On Fri, Sep 15, 2017 at 01:34:49PM +0200, Juergen Gross wrote:
> >>>>
> >>>> Backends reads and seeds with (and assuming it passes backend validation ofc):
> >>>>
> >>>> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
> >>>> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
> >>>> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
> >>>>
> >>>> The XL configuration entry for controlling these tunable are just examples it's
> >>>> not clear the general preference for this. An alternative could be:
> >>>>
> >>>> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
> >>>>
> >>>> Which lets us have more generic feature control, without sticking to particular
> >>>> features names.
> >>>>
> >>
> >> In case the above was about config format, this one suggested above sounds more
> >> general, and easy to reuse across backends. Maybe instead of "features", could
> >> be "backend_features" since, most PV backends declare a "backend" and a
> >> "backend_id" as per libxl IDL.
> >>
> > 
> > The proposed syntax looks a bit difficult to parse.
> > 
> > What's wrong with request-XXX=YYY syntax? We can have many of those as
> > we like. Xl just picks those and concatenate them into backend_features.
> 
> Is it possible to parse those without having to know about individual
> XXX values? Otherwise we'd be able to support only features known by xl
> instead of those known by the various backends.
> 

Xl sees a list of key-value pairs. If the key of a pair starts with the
predefined prefix, xl (eventually) passes the pair on to backend. I
don't think knowing individual XXX value is needed or desired.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-15 11:34       ` Juergen Gross
  2017-09-15 12:46         ` Wei Liu
@ 2017-09-15 13:56         ` Joao Martins
  1 sibling, 0 replies; 11+ messages in thread
From: Joao Martins @ 2017-09-15 13:56 UTC (permalink / raw)
  To: Juergen Gross, Wei Liu; +Cc: xen-devel, Boris Ostrovsky, Annie Li, Ankur Arora

On 09/15/2017 12:34 PM, Juergen Gross wrote:
> On 15/09/17 13:19, Wei Liu wrote:
>> On Thu, Sep 14, 2017 at 05:18:44PM +0100, Joao Martins wrote:
>>> On 09/14/2017 05:10 PM, Wei Liu wrote:
>>>> On Thu, Sep 07, 2017 at 05:53:54PM +0100, Joao Martins wrote:
>>>>> Hey!
>>>>>
>>>>> We wanted to brought up this small proposal regarding the lack of
>>>>> parameterization on PV devices on Xen.
>>>>>
>>>>> Currently users don't have a way for enforce and control what
>>>>> features/queues/etc the backend provides. So far there's only global parameters
>>>>> on backends, and specs do not mention anything in this regard.
>>>>>
>>>>> The most obvious example is netback/blkback max_queues module parameter where it
>>>>> sets the limit the maximum queues for all devices which is not that flexible.
>>>>> Other examples include controlling offloads visible by the NIC (e.g. disabling
>>>>> checksum offload, disabling scather-gather), others more about I/O path (e.g.
>>>>> disable blkif indirect descriptors, limit number of pages for the ring), or less
>>>>> grant usage by minimizing number of queues/descriptors.
>>>>>
>>>>> Of course there could be more examples, as this seems to be ortoghonal to the
>>>>> kinds of PV backends we have. And seems like all features appear to be published
>>>>> on the same xenbus state?
>>>>>
>>>>> The idea to address this would be very simple:
>>>>>
>>>>> - Toolstack when initializing device paths, writes additional entries in the
>>>>> form of 'request-<feature-name>' = <feature-value>. These entries are only
>>>>> visible by the backend and toolstack;
>>>>>
>>>>> - Backend reads this entries and uses <feature-value> as the value of
>>>>> <feature-name>, which will then be visible on the frontend.
>>>>>
>>>>> [ Removal of the 'request-*' xenstore entries could represent a feedback look
>>>>>   that the backend indeed read and used the value. Or else it could simply be
>>>>>   ignored. ]
>>>>>
>>>>> And that's it.
>>>>>
>>>>> In pratice user would do: E.g.
>>>>>
>>>>> domain.cfg:
>>>>> ...
>>>>> name = "guest"
>>>>> kernel = "bzImage"
>>>>> vif = ["bridge=br0,queues=2"]
>>>>> disk = [
>>>>> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
>>>>
>>>> There needs to be a way to distinguish parameters consumed by toolstack
>>>> vs the ones passed on to backends. The parameters passed to backends
>>>> should start with a predefined prefix.
>>>>
>>> Hmm, which seems to be inline with the "request" prefix when controlling certain
>>> features enabled/disabled? Oh wait, perhaps you mean wrt to the UI/config-format
>>> rather than xenstore entries and such? If it's the latter, see below,
>>
>> I was thinking about xl config syntax.
>>
>>>
>>>>> ]
>>>>> ...
>>>>>
>>>>> Toolstack writes:
>>>>>
>>>>> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
>>>>> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
>>>>> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0
> 
> I'd rather use a specific directory, e.g.:
> 
> /local/domain/0/backend/vif/8/0/request/multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/request/multi-queue-max-queues = 2
> /local/domain/0/backend/vbd/8/51713/request/max-ring-page-order = 0
> 
> This will enable the backend to just look for all entries in
> .../request/ instead of having to try all possible features.
> 
Yeap, sounds better and cleaner indeed.

And backend can simply remove the whole directory when it's done consuming
the parameters as a signal to the toolstack? Or maybe it might be enough to
simply detect that request/XXX and XXX xenstores entries have the same value.

>>>>> Backends reads and seeds with (and assuming it passes backend validation ofc):
>>>>>
>>>>> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
>>>>> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
>>>>> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
>>>>>
>>>>> The XL configuration entry for controlling these tunable are just examples it's
>>>>> not clear the general preference for this. An alternative could be:
>>>>>
>>>>> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
>>>>>
>>>>> Which lets us have more generic feature control, without sticking to particular
>>>>> features names.
>>>>>
>>>
>>> In case the above was about config format, this one suggested above sounds more
>>> general, and easy to reuse across backends. Maybe instead of "features", could
>>> be "backend_features" since, most PV backends declare a "backend" and a
>>> "backend_id" as per libxl IDL.
>>>
>>
>> The proposed syntax looks a bit difficult to parse.
>>
>> What's wrong with request-XXX=YYY syntax? We can have many of those as
>> we like. Xl just picks those and concatenate them into backend_features.
> 
No problem at all assuming the backend_features on IDL is a list of XXX=YYY - I
suggested the above syntax simply as a start given how 'target' is put together
for disks with  backendtype=qemu (e.g. rbd parameters). But yours it's much
better from user perspective.

Thinking if there was a problem with emulated NICs, but I suppose the same
request-XXX=YYY could be used to toggle the emulated device offloadings (e.g.
for virtio we would use require-guest_ecn=0). But it's probably a bit early to
worry about that. libxl "backend_features" is generic enough to accomodate this.

> Is it possible to parse those without having to know about individual
> XXX values? Otherwise we'd be able to support only features known by xl
> instead of those known by the various backends.

/nods

I too would keep this stateless from toolstack perspective.

Joao

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-14 16:08   ` Joao Martins
@ 2017-09-18 19:59     ` Konrad Rzeszutek Wilk
  2017-09-19 15:32       ` Joao Martins
  0 siblings, 1 reply; 11+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-09-18 19:59 UTC (permalink / raw)
  To: Joao Martins
  Cc: Juergen Gross, Stefano Stabellini, Ankur Arora, Annie Li,
	xen-devel, Boris Ostrovsky

On Thu, Sep 14, 2017 at 05:08:18PM +0100, Joao Martins wrote:
> [ Realized that I didn't CC the maintainers,
>   so doing that now, +Linux folks +PV interfaces czar
>   Sorry for the noise! ]
> 
> On 09/08/2017 09:49 AM, Joao Martins wrote:
> > [Forgot two important details regarding Xenbus states]
> > On 09/07/2017 05:53 PM, Joao Martins wrote:
> >> Hey!
> >>
> >> We wanted to brought up this small proposal regarding the lack of
> >> parameterization on PV devices on Xen.
> >>
> >> Currently users don't have a way for enforce and control what
> >> features/queues/etc the backend provides. So far there's only global parameters
> >> on backends, and specs do not mention anything in this regard.

How would this scale with say FreeBSD backends? And I am assuming you are
also thinking about device driver backends - where you can't easily
get access to the backend and change the SysFS parameters (if they have
it all)?

> >>
> >> The most obvious example is netback/blkback max_queues module parameter where it
> >> sets the limit the maximum queues for all devices which is not that flexible.
> >> Other examples include controlling offloads visible by the NIC (e.g. disabling
> >> checksum offload, disabling scather-gather), others more about I/O path (e.g.
> >> disable blkif indirect descriptors, limit number of pages for the ring), or less
> >> grant usage by minimizing number of queues/descriptors.
> >>
> >> Of course there could be more examples, as this seems to be ortoghonal to the
> >> kinds of PV backends we have. And seems like all features appear to be published
> >> on the same xenbus state?
> >>
> >> The idea to address this would be very simple:
> >>
> >> - Toolstack when initializing device paths, writes additional entries in the
> >> form of 'request-<feature-name>' = <feature-value>. These entries are only
> >> visible by the backend and toolstack;
> >>
> > And after that we switch the device state to XenbusStateInitialising as usual.
> > 
> >>
> >> - Backend reads this entries and uses <feature-value> as the value of
> >> <feature-name>, which will then be visible on the frontend.
> >>
> > And after that we switch state to XenbusStateInitWait as usual. No changes are
> > involved in xenbus state changes other than reading what the toolstack had
> > written in "request-*" and seed accordingly. Backends without support would
> > simply ignore these new entries.
> > 
> >> [ Removal of the 'request-*' xenstore entries could represent a feedback look
> >>   that the backend indeed read and used the value. Or else it could simply be
> >>   ignored. ]
> >>
> >> And that's it.
> >>
> >> In pratice user would do: E.g.
> >>
> >> domain.cfg:
> >> ...
> >> name = "guest"
> >> kernel = "bzImage"
> >> vif = ["bridge=br0,queues=2"]
> >> disk = [
> >> "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2,queues=1,max-ring-page-order=0"
> >> ]
> >> ...
> >>
> >> Toolstack writes:
> >>
> >> /local/domain/0/backend/vif/8/0/request-multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/request-multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/request-max-ring-page-order = 0
> > 
> > /local/domain/0/backend/vbd/8/51713/state = 1 (XenbusStateInitialising)
> > 
> >>
> >> Backends reads and seeds with (and assuming it passes backend validation ofc):
> >>
> >> /local/domain/0/backend/vif/8/0/multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/multi-queue-max-queues = 2
> >> /local/domain/0/backend/vbd/8/51713/max-ring-page-order = 0
> >>
> > /local/domain/0/backend/vbd/8/51713/state = 2 (XenbusStateInitWait)
> > 
> >> The XL configuration entry for controlling these tunable are just examples it's
> >> not clear the general preference for this. An alternative could be:
> >>
> >> vif = ["bridge=br0,features=queues:2\\;max-ring-page-order:0"]
> >>
> >> Which lets us have more generic feature control, without sticking to particular
> >> features names.
> >>
> >> Naturally libvirt could be a consumer of this (as it already has the 'queues'
> >> and host 'tso4', 'tso6', etc in their XML schemas)
> >>
> >> Thoughts? Do folks think the correct way of handling this?
> >>
> >> Cheers,
> >> Joao
> >>
> >> [0] https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L2102
> >>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Feature control on PV devices
  2017-09-18 19:59     ` Konrad Rzeszutek Wilk
@ 2017-09-19 15:32       ` Joao Martins
  0 siblings, 0 replies; 11+ messages in thread
From: Joao Martins @ 2017-09-19 15:32 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Juergen Gross, Stefano Stabellini, Ankur Arora, Annie Li,
	xen-devel, Boris Ostrovsky

On 09/18/2017 08:59 PM, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 14, 2017 at 05:08:18PM +0100, Joao Martins wrote:
>> [ Realized that I didn't CC the maintainers,
>>   so doing that now, +Linux folks +PV interfaces czar
>>   Sorry for the noise! ]
>>
>> On 09/08/2017 09:49 AM, Joao Martins wrote:
>>> [Forgot two important details regarding Xenbus states]
>>> On 09/07/2017 05:53 PM, Joao Martins wrote:
>>>> Hey!
>>>>
>>>> We wanted to brought up this small proposal regarding the lack of
>>>> parameterization on PV devices on Xen.
>>>>
>>>> Currently users don't have a way for enforce and control what
>>>> features/queues/etc the backend provides. So far there's only global parameters
>>>> on backends, and specs do not mention anything in this regard.
> 
> How would this scale with say FreeBSD backends?
>
This is per-device parameter configuration support, based on xenstore entries.
All backend needs to understand is that the request/XXX xenstore entries and
superseed whatever global defaults were defined by backend (after validation).
So what I am proposing here makes no OS assumptions and should work for FreeBSD
or any other.

> And I am assuming you are
> also thinking about device driver backends - where you can't easily
> get access to the backend and change the SysFS parameters (if they have
> it all)?
> 
Yeah - Provided that the xenstore entries will be created with permissions for
toolstack domain and the backend domain then backends other than Dom0 should
work too. Note that this is device setup (e.g. domain create time), i.e. the
configuration of what the frontend is allowed to see/use.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-09-19 15:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-07 16:53 Feature control on PV devices Joao Martins
2017-09-08  8:49 ` Joao Martins
2017-09-14 16:08   ` Joao Martins
2017-09-18 19:59     ` Konrad Rzeszutek Wilk
2017-09-19 15:32       ` Joao Martins
2017-09-14 16:10 ` Wei Liu
2017-09-14 16:18   ` Joao Martins
2017-09-15 11:19     ` Wei Liu
2017-09-15 11:34       ` Juergen Gross
2017-09-15 12:46         ` Wei Liu
2017-09-15 13:56         ` Joao Martins

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).