| Message ID | 20171102180616.24084-3-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Joao Martins [mailto:joao.m.martins@oracle.com] > Sent: 02 November 2017 18:06 > To: Xen Development List <xen-devel@lists.xen.org> > Cc: Joao Martins <joao.m.martins@oracle.com>; Konrad Rzeszutek Wilk > <konrad.wilk@oracle.com>; Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu > <wei.liu2@citrix.com> > Subject: [PATCH RFC 2/8] public/io/netif: add directory for backend > parameters > > The proposed directory provides a mechanism for tools to control the > maximum feature set of the device being provisioned by backend. > The parameters/features include offloading features, number of > queues etc. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > xen/include/public/io/netif.h | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h > index 2454448baa..a412e4771d 100644 > --- a/xen/include/public/io/netif.h > +++ b/xen/include/public/io/netif.h > @@ -161,6 +161,22 @@ > */ > > /* > + * The directory "require" maybe be created in backend path by tools > + * domain to override the maximum feature set that backend provides to > the > + * frontend. The children entries within this directory are features names > + * and the correspondent values that should be used backend as defaults > e.g.: > + * > + * /local/domain/X/backend/<domid>/<handle>/require > + * /local/domain/X/backend/<domid>/<handle>/require/multi-queue- > max-queues = "2" > + * /local/domain/X/backend/<domid>/<handle>/require/feature-no-csum- > offload = "1" > + * > + * In the example above, network backend will negotiate up to a maximum > of > + * two queues with frontend plus disabling IPv4 checksum offloading. > + * > + * This directory and its children entries shall only be visible to the backend. > + */ > + What should happen if the toolstack sets something in 'require' that the backend cannot provide? I don't see anything in your RFC patches to check that the backend has responded appropriately to the keys. Paul > +/* > * Control ring > * ============ > * > -- > 2.11.0
On Mon, Nov 06, 2017 at 10:33:59AM +0000, Paul Durrant wrote: > > -----Original Message----- > > From: Joao Martins [mailto:joao.m.martins@oracle.com] > > Sent: 02 November 2017 18:06 > > To: Xen Development List <xen-devel@lists.xen.org> > > Cc: Joao Martins <joao.m.martins@oracle.com>; Konrad Rzeszutek Wilk > > <konrad.wilk@oracle.com>; Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu > > <wei.liu2@citrix.com> > > Subject: [PATCH RFC 2/8] public/io/netif: add directory for backend > > parameters > > > > The proposed directory provides a mechanism for tools to control the > > maximum feature set of the device being provisioned by backend. > > The parameters/features include offloading features, number of > > queues etc. > > > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > > --- > > xen/include/public/io/netif.h | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h > > index 2454448baa..a412e4771d 100644 > > --- a/xen/include/public/io/netif.h > > +++ b/xen/include/public/io/netif.h > > @@ -161,6 +161,22 @@ > > */ > > > > /* > > + * The directory "require" maybe be created in backend path by tools > > + * domain to override the maximum feature set that backend provides to > > the > > + * frontend. The children entries within this directory are features names > > + * and the correspondent values that should be used backend as defaults > > e.g.: > > + * > > + * /local/domain/X/backend/<domid>/<handle>/require > > + * /local/domain/X/backend/<domid>/<handle>/require/multi-queue- > > max-queues = "2" > > + * /local/domain/X/backend/<domid>/<handle>/require/feature-no-csum- > > offload = "1" > > + * > > + * In the example above, network backend will negotiate up to a maximum > > of > > + * two queues with frontend plus disabling IPv4 checksum offloading. > > + * > > + * This directory and its children entries shall only be visible to the backend. > > + */ > > + > > What should happen if the toolstack sets something in 'require' that > the backend cannot provide? I don't see anything in your RFC patches > to check that the backend has responded appropriately to the keys. Hmm, you're right that this RFC doesn't handle that properly - but for the ones the backend provide I had suggested (albeit not implemented here) back in the other thread that we could compare the values of feature in "require" with the one announced to the frontend. But well this wouldn't cover the non-provided ones, and possibly would fall a bit as a hack. I could change the format of the entries within "require" directory to be e.g. "<id>-<feature-name> = <feature-value>" and the acknowledgement entry would come in the form "<id>-status = <error code>". Consequently the lack of a "<id>-status" entry would have a stronger semantic i.e. unsupported and ignored. The toolstack then would have means to check whether the feature was really succesfull set as desired or not. But then one question comes to mind: should the backend be prevented to init in the event that the features requested fail to be set? In which case uevent (on Linux) isn't triggered and xenbus state doesn't get changed and toolstack would fail with timeout later on. Also, a nice thing of this stuff is that we could also use this to set set backend implementation specific parameters that are not described or relevant in I/O specs. But then I start to wonder where would be the correct place for backends to specify its maximum feature set of changeable entries? Maybe: /local/domain/X/backend/vif/features/<feature-name> /local/domain/X/backend/vif/features/<feature-name>-desc = "Description of <feature-name>" Cheers, Joao
diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h index 2454448baa..a412e4771d 100644 --- a/xen/include/public/io/netif.h +++ b/xen/include/public/io/netif.h @@ -161,6 +161,22 @@ */ /* + * The directory "require" maybe be created in backend path by tools + * domain to override the maximum feature set that backend provides to the + * frontend. The children entries within this directory are features names + * and the correspondent values that should be used backend as defaults e.g.: + * + * /local/domain/X/backend/<domid>/<handle>/require + * /local/domain/X/backend/<domid>/<handle>/require/multi-queue-max-queues = "2" + * /local/domain/X/backend/<domid>/<handle>/require/feature-no-csum-offload = "1" + * + * In the example above, network backend will negotiate up to a maximum of + * two queues with frontend plus disabling IPv4 checksum offloading. + * + * This directory and its children entries shall only be visible to the backend. + */ + +/* * Control ring * ============ *
The proposed directory provides a mechanism for tools to control the maximum feature set of the device being provisioned by backend. The parameters/features include offloading features, number of queues etc. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- xen/include/public/io/netif.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)