Guidelines for new PV protocol submission

Guidelines for new PV protocol submission

[Roger Pau Monné]

Hello,

I should probably have done this earlier because I've been aware of this
issue for a long time (since I've started dealing with the PV blk protocol).

The current way to describe PV protocols in Xen is very inefficient
IMHO. Using C structs as "the description" of a binary protocol seems
very wrong, specially taking into account that different ABIs can
generate different layouts for the same C struct. This is for example a
problem in the PV blk protocol, since the binary layout of the
structures change depending on the bitness.

In order to avoid this, I would like to request that any new PV protocol
that's added to Xen be described in binary terms, just like it's
normally done with other protocols. As a reference see for example the
following section from the TCP RFC:

http://tools.ietf.org/html/rfc793#page-15

I think this is both more easy to understand and removes the bitness
problem of using C structs.

Then each user of this protocol could define it's own set of structures
that would map to the binary layout, which should be almost trivial.
There would be no problem with using __packed or similar gcc'isms as
each implementation could choose the more convenient way to represent
this layout internally.

Roger.

Re: Guidelines for new PV protocol submission

[David Vrabel]

On 20/01/15 12:47, Roger Pau Monné wrote:
> Hello,
> 
> I should probably have done this earlier because I've been aware of this
> issue for a long time (since I've started dealing with the PV blk protocol).
> 
> The current way to describe PV protocols in Xen is very inefficient
> IMHO. Using C structs as "the description" of a binary protocol seems
> very wrong, specially taking into account that different ABIs can
> generate different layouts for the same C struct. This is for example a
> problem in the PV blk protocol, since the binary layout of the
> structures change depending on the bitness.
> 
> In order to avoid this, I would like to request that any new PV protocol
> that's added to Xen be described in binary terms, just like it's
> normally done with other protocols. As a reference see for example the
> following section from the TCP RFC:
> 
> http://tools.ietf.org/html/rfc793#page-15
> 
> I think this is both more easy to understand and removes the bitness
> problem of using C structs.
> 
> Then each user of this protocol could define it's own set of structures
> that would map to the binary layout, which should be almost trivial.
> There would be no problem with using __packed or similar gcc'isms as
> each implementation could choose the more convenient way to represent
> this layout internally.

+1

We did this for migration v2 protocol[1] and I agree that this is a much
better way of specifying binary protocols.

David

[1] http://xenbits.xen.org/people/andrewcoop/domain-save-format-F.pdf

Re: Guidelines for new PV protocol submission

[Andrew Cooper]

On 20/01/15 13:20, David Vrabel wrote:
> On 20/01/15 12:47, Roger Pau Monné wrote:
>> Hello,
>>
>> I should probably have done this earlier because I've been aware of this
>> issue for a long time (since I've started dealing with the PV blk protocol).
>>
>> The current way to describe PV protocols in Xen is very inefficient
>> IMHO. Using C structs as "the description" of a binary protocol seems
>> very wrong, specially taking into account that different ABIs can
>> generate different layouts for the same C struct. This is for example a
>> problem in the PV blk protocol, since the binary layout of the
>> structures change depending on the bitness.
>>
>> In order to avoid this, I would like to request that any new PV protocol
>> that's added to Xen be described in binary terms, just like it's
>> normally done with other protocols. As a reference see for example the
>> following section from the TCP RFC:
>>
>> http://tools.ietf.org/html/rfc793#page-15
>>
>> I think this is both more easy to understand and removes the bitness
>> problem of using C structs.
>>
>> Then each user of this protocol could define it's own set of structures
>> that would map to the binary layout, which should be almost trivial.
>> There would be no problem with using __packed or similar gcc'isms as
>> each implementation could choose the more convenient way to represent
>> this layout internally.
> +1
>
> We did this for migration v2 protocol[1] and I agree that this is a much
> better way of specifying binary protocols.

+1

It is far nicer to work from a document than to worry how another
compiler might change the structure.

~Andrew

Re: Guidelines for new PV protocol submission

[Jan Beulich]

>>> On 20.01.15 at 14:39, <andrew.cooper3@citrix.com> wrote:
> It is far nicer to work from a document than to worry how another
> compiler might change the structure.

Compilers have (almost, i.e. leaving aside bitfields) no freedom in laying
out structures - any platform's ABI defines how this needs to be done.
Or else code compiled with different compilers wouldn't inter-operate.

Jan

Re: Guidelines for new PV protocol submission

[Andrew Cooper]

On 20/01/15 13:48, Jan Beulich wrote:
>>>> On 20.01.15 at 14:39, <andrew.cooper3@citrix.com> wrote:
>> It is far nicer to work from a document than to worry how another
>> compiler might change the structure.
> Compilers have (almost, i.e. leaving aside bitfields) no freedom in laying
> out structures - any platform's ABI defines how this needs to be done.
> Or else code compiled with different compilers wouldn't inter-operate.

Widths of types are not consistent.

Attempting to use the Xen header files on windows requires sed because
unsigned longs are still 32 bits even on 64bit windows.  (i.e. an
unsigned long is not valid for a pointer of natural width, which occurs
even in the HVM ABI).

Assuming a posix view of the world is not good enough for the Xen
ecosystem, which is why a document is preferable to a C structure.

~Andrew

Re: Guidelines for new PV protocol submission

[Jan Beulich]

>>> On 20.01.15 at 15:00, <andrew.cooper3@citrix.com> wrote:
> On 20/01/15 13:48, Jan Beulich wrote:
>>>>> On 20.01.15 at 14:39, <andrew.cooper3@citrix.com> wrote:
>>> It is far nicer to work from a document than to worry how another
>>> compiler might change the structure.
>> Compilers have (almost, i.e. leaving aside bitfields) no freedom in laying
>> out structures - any platform's ABI defines how this needs to be done.
>> Or else code compiled with different compilers wouldn't inter-operate.
> 
> Widths of types are not consistent.

Hence the need to use fixed-width ones (and explicit padding).

> Attempting to use the Xen header files on windows requires sed because
> unsigned longs are still 32 bits even on 64bit windows.  (i.e. an
> unsigned long is not valid for a pointer of natural width, which occurs
> even in the HVM ABI).

I think we made quite a bit of progress eliminating the uses of "long"
in the public headers.

Jan

Re: Guidelines for new PV protocol submission

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] Guidelines for new PV protocol submission"):
> On 20.01.15 at 14:39, <andrew.cooper3@citrix.com> wrote:
> > It is far nicer to work from a document than to worry how another
> > compiler might change the structure.
> 
> Compilers have (almost, i.e. leaving aside bitfields) no freedom in laying
> out structures - any platform's ABI defines how this needs to be done.
> Or else code compiled with different compilers wouldn't inter-operate.

This is true within an ABI on a particular operating system, but there
is nothing stopping an operating system from supporting multiple ABIs.
There is nothing stopping an operating system from using an ABI whose
struct layout rules are different to those specified in the processor
manufacturer's reference manuals.  (Perhaps even only such an ABI.
Embedded OS developers often decide ABI details for themselves.)

Also, a processor manufacturer who invents a new sub-architecture (eg,
a 64-bit version of an existing 32-bit architecture) may decide to
apply new structure layout rules.  So trying to define things in terms
of C structs can lead to data layouts which are different on different
sub-architectures all of which can run on the same CPUs, which is
highly undesirable.  Using uintNN_t types helps with this but is not
sufficient.

Looking at things more formally: if we intend to publish a binary
protocol specification, doing so as a C struct means that we are
implicitly including not just the C specification but also the C
standard and all the processor manufacturers' struct layout rules into
our specification.  The result is a specification where definitively
determining the meaning is an awful lot of work.

And because we are making assumptions about struct layout rules beyond
what is specified in the C standard itself, we may encounter (ie, want
to port Xen to) an architecture where the same struct results in a
different layout - so the specification as a C struct is (in
principle, and sometimes in practice too) not stable across
architectures.

I am 100% in agreement with Roger's proposal.  I think we should
extend this principle to as much of the guest ABI as possible, not
just PV protocols - it would be good to do it for guest-visible
hypervisor and tools ABIs too.

Ian.

Re: Guidelines for new PV protocol submission

[Ian Campbell]

On Tue, 2015-01-20 at 14:21 +0000, Ian Jackson wrote:
> I am 100% in agreement with Roger's proposal.  I think we should
> extend this principle to as much of the guest ABI as possible, not
> just PV protocols - it would be good to do it for guest-visible
> hypervisor and tools ABIs too.

Agreed, but lets start with something simple and achievable, like
xen/include/public/io/*.h.

I don't think we should invent one up, but if there is an existing, well
know and decent IDL which suits our needs we should consider using it.

Ian.

Re: Guidelines for new PV protocol submission

[Jan Beulich]

>>> On 20.01.15 at 13:47, <roger.pau@citrix.com> wrote:
> I should probably have done this earlier because I've been aware of this
> issue for a long time (since I've started dealing with the PV blk protocol).
> 
> The current way to describe PV protocols in Xen is very inefficient
> IMHO. Using C structs as "the description" of a binary protocol seems
> very wrong, specially taking into account that different ABIs can
> generate different layouts for the same C struct. This is for example a
> problem in the PV blk protocol, since the binary layout of the
> structures change depending on the bitness.
> 
> In order to avoid this, I would like to request that any new PV protocol
> that's added to Xen be described in binary terms, just like it's
> normally done with other protocols. As a reference see for example the
> following section from the TCP RFC:
> 
> http://tools.ietf.org/html/rfc793#page-15 
> 
> I think this is both more easy to understand and removes the bitness
> problem of using C structs.

Since such a layout can be translated to a C struct, I'm having
trouble seeing why not providing C struct-s from the beginning
(and as canonical reference) would be better or more efficient.
The mistake with blkif wasn't that it was written down as a C
struct, but that bitness wasn't taken into account from the
start.

> Then each user of this protocol could define it's own set of structures
> that would map to the binary layout, which should be almost trivial.
> There would be no problem with using __packed or similar gcc'isms as
> each implementation could choose the more convenient way to represent
> this layout internally.

That option exists anyway, and upstream Linux has been doing
so (not to the better imo especially for blkif).

Jan

Re: Guidelines for new PV protocol submission

[Ian Campbell]

On Tue, 2015-01-20 at 13:47 +0100, Roger Pau Monné wrote:
> Hello,
> 
> I should probably have done this earlier because I've been aware of this
> issue for a long time (since I've started dealing with the PV blk protocol).
> 
> The current way to describe PV protocols in Xen is very inefficient
> IMHO. Using C structs as "the description" of a binary protocol seems
> very wrong, specially taking into account that different ABIs can
> generate different layouts for the same C struct. This is for example a
> problem in the PV blk protocol, since the binary layout of the
> structures change depending on the bitness.
> 
> In order to avoid this, I would like to request that any new PV protocol
> that's added to Xen be described in binary terms, just like it's
> normally done with other protocols. As a reference see for example the
> following section from the TCP RFC:
> 
> http://tools.ietf.org/html/rfc793#page-15

How about on the next doc day I'll convert netif.h and you do blkif.h?

Ian.


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

Re: Guidelines for new PV protocol submission

[Roger Pau Monné]

El 05/02/15 a les 11.42, Ian Campbell ha escrit:
> On Tue, 2015-01-20 at 13:47 +0100, Roger Pau Monné wrote:
>> Hello,
>>
>> I should probably have done this earlier because I've been aware of this
>> issue for a long time (since I've started dealing with the PV blk protocol).
>>
>> The current way to describe PV protocols in Xen is very inefficient
>> IMHO. Using C structs as "the description" of a binary protocol seems
>> very wrong, specially taking into account that different ABIs can
>> generate different layouts for the same C struct. This is for example a
>> problem in the PV blk protocol, since the binary layout of the
>> structures change depending on the bitness.
>>
>> In order to avoid this, I would like to request that any new PV protocol
>> that's added to Xen be described in binary terms, just like it's
>> normally done with other protocols. As a reference see for example the
>> following section from the TCP RFC:
>>
>> http://tools.ietf.org/html/rfc793#page-15
> 
> How about on the next doc day I'll convert netif.h and you do blkif.h?

Sounds like a plan, although I think I got the short straw ;).

Roger.


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

Re: Guidelines for new PV protocol submission

[Ian Campbell]

On Mon, 2015-02-09 at 12:01 +0100, Roger Pau Monné wrote:
> El 05/02/15 a les 11.42, Ian Campbell ha escrit:
> > On Tue, 2015-01-20 at 13:47 +0100, Roger Pau Monné wrote:
> >> Hello,
> >>
> >> I should probably have done this earlier because I've been aware of this
> >> issue for a long time (since I've started dealing with the PV blk protocol).
> >>
> >> The current way to describe PV protocols in Xen is very inefficient
> >> IMHO. Using C structs as "the description" of a binary protocol seems
> >> very wrong, specially taking into account that different ABIs can
> >> generate different layouts for the same C struct. This is for example a
> >> problem in the PV blk protocol, since the binary layout of the
> >> structures change depending on the bitness.
> >>
> >> In order to avoid this, I would like to request that any new PV protocol
> >> that's added to Xen be described in binary terms, just like it's
> >> normally done with other protocols. As a reference see for example the
> >> following section from the TCP RFC:
> >>
> >> http://tools.ietf.org/html/rfc793#page-15
> > 
> > How about on the next doc day I'll convert netif.h and you do blkif.h?
> 
> Sounds like a plan, although I think I got the short straw ;).

I know :-P

Ian.


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

Re: Guidelines for new PV protocol submission

[Vitaly Chernooky]

[-- Attachment #1.1: Type: text/plain, Size: 1946 bytes --]

Hi All!

On Tue, Jan 20, 2015 at 2:47 PM, Roger Pau Monné <roger.pau@citrix.com>
wrote:

> Hello,
>
> I should probably have done this earlier because I've been aware of this
> issue for a long time (since I've started dealing with the PV blk
> protocol).
>
> The current way to describe PV protocols in Xen is very inefficient
> IMHO. Using C structs as "the description" of a binary protocol seems
> very wrong, specially taking into account that different ABIs can
> generate different layouts for the same C struct. This is for example a
> problem in the PV blk protocol, since the binary layout of the
> structures change depending on the bitness.
>
> In order to avoid this, I would like to request that any new PV protocol
> that's added to Xen be described in binary terms, just like it's
> normally done with other protocols. As a reference see for example the
> following section from the TCP RFC:
>
> http://tools.ietf.org/html/rfc793#page-15


Guys, what do you think about using an Interface Description Language such
as Google Protocol Buffers or something like?

With best regards,


> I think this is both more easy to understand and removes the bitness
> problem of using C structs.
>
> Then each user of this protocol could define it's own set of structures
> that would map to the binary layout, which should be almost trivial.
> There would be no problem with using __packed or similar gcc'isms as
> each implementation could choose the more convenient way to represent
> this layout internally.
>
> Roger.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>



-- 
*Vitaly Chernooky | Senior Developer - Product Engineering and Development*
GlobalLogic
P *+380.44.4929695 ext.1136* M *+380.98.7920568* S cvv_2k
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

[-- Attachment #1.2: Type: text/html, Size: 3055 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: Guidelines for new PV protocol submission

[Ian Campbell]

On Mon, 2015-02-09 at 13:59 +0200, Vitaly Chernooky wrote:

> Guys, what do you think about using an Interface Description Language
> such as Google Protocol Buffers or something like?

After a bit of googling and reading it seems like Google Protocol
Buffers includes a specific wire encoding, a la ASN.1 or CORBA, whereas
we need to be able to describe existing binary formats.

I haven't evaluated the suitability for new PV protocols, but it seems
like Protocol Buffers are not really intended for shared memory
protocols (rather network transparent ones). Perhaps I'm wrong though, I
just had a quick look.

Ian.

Re: Guidelines for new PV protocol submission

[Vitaly Chernooky]

[-- Attachment #1.1: Type: text/plain, Size: 1184 bytes --]

On Mon, Feb 9, 2015 at 3:15 PM, Ian Campbell <ian.campbell@citrix.com>
wrote:

> On Mon, 2015-02-09 at 13:59 +0200, Vitaly Chernooky wrote:
>
> > Guys, what do you think about using an Interface Description Language
> > such as Google Protocol Buffers or something like?
>
> After a bit of googling and reading it seems like Google Protocol
> Buffers includes a specific wire encoding, a la ASN.1 or CORBA, whereas
> we need to be able to describe existing binary formats.
>
> I haven't evaluated the suitability for new PV protocols, but it seems
> like Protocol Buffers are not really intended for shared memory
> protocols (rather network transparent ones). Perhaps I'm wrong though, I
> just had a quick look.
>

I just want to mention than machine-readable protocol description is
reasonable and possibly has some advantages for us over rfc-like protocol
description style. So i'm interested to know another opinions about it.

With best regards,


> Ian.
>


-- 
*Vitaly Chernooky | Senior Developer - Product Engineering and Development*
GlobalLogic
P *+380.44.4929695 ext.1136* M *+380.98.7920568* S cvv_2k
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

[-- Attachment #1.2: Type: text/html, Size: 2186 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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