| Message ID | 1452171912-29857-2-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2016-01-07 at 13:05 +0000, Paul Durrant wrote: > Currently there is no documented wire format for guest receive-side > packets but the location of the 'wire format' comment block suggests > it is the same as transmit-side. This is almost true but there is a > subtle difference in the use of the 'size' field for the first fragment. > > For clarity this patch creates separate comment blocks for receive > and transmit side packet wire formats, tries to be more clear about the > distinction between 'fragments' and 'extras', and documents the subtlety > concerning the size field of the first fragment. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Keir Fraser <keir@xen.org> > Cc: Tim Deegan <tim@xen.org> > --- > xen/include/public/io/netif.h | 39 ++++++++++++++++++++++++++----------- > -- > 1 file changed, 26 insertions(+), 13 deletions(-) > > diff --git a/xen/include/public/io/netif.h > b/xen/include/public/io/netif.h > index e103cf3..1790ea0 100644 > --- a/xen/include/public/io/netif.h > +++ b/xen/include/public/io/netif.h > @@ -151,22 +151,22 @@ > */ > > /* > - * This is the 'wire' format for packets: > - * Request 1: netif_tx_request_t -- NETTXF_* (any flags) > - * [Request 2: netif_extra_info_t] (only if request 1 has > - * NETTXF_extra_info) > - * [Request 3: netif_extra_info_t] (only if request 2 has > - * XEN_NETIF_EXTRA_MORE) > - * Request 4: netif_tx_request_t -- NETTXF_more_data > - * Request 5: netif_tx_request_t -- NETTXF_more_data > - * ... > - * Request N: netif_tx_request_t -- 0 > - */ > - > -/* > * Guest transmit > * ============== > * > + * This is the 'wire' format for packets: > + * Fragment 1: netif_tx_request_t - flags = NETTXF_* > + * size = total packet size > + * [Extra 1: netif_extra_info_t] - (only if fragment 1 flags include > + * NETTXF_extra_info) > + * [Extra N: netif_extra_info_t] - (only if extra N-1 flags include > + * XEN_NETIF_EXTRA_MORE) > + * ... > + * Fragment N: netif_tx_request_t - (only if fragment N-1 flags include > + * NETTXF_more_data) For Fragment 2 is it the Flags of Fragment N-1 = 1 which are relevant, or the flags in the optional Extra N which may be in the middle (i.e. the immediately preceding slot)? Am I correct in remembering that in the presence of NETTXF_more_data the only way to know the actual size of Fragment 1 is to take away the total of all the extras from Frag 1's size? > + * flags = 0 > + * size = fragment size > + * > * Ring slot size is 12 octets, however not all request/response > * structs use the full size. > * > @@ -202,6 +202,19 @@ > * Guest receive > * ============= > * > + * This is the 'wire' format for packets: > + * Fragment 1: netif_rx_request_t - flags = NETRXF_* > + * size = fragment size > + * [Extra 1: netif_extra_info_t] - (only if fragment 1 flags include > + * NETRXF_extra_info) > + * [Extra N: netif_extra_info_t] - (only if extra N-1 flags include > + * XEN_NETIF_EXTRA_MORE) > + * ... > + * Fragment N: netif_rx_request_t - (only if fragment N-1 flags include > + * NETRXF_more_data) > + * flags = 0 > + * size = fragment size Same Q re which NETRXF_more_data is relevant. In this path there is no indication of the total packet size other than adding everything up? Given that they differ in a subtle way would a quick but explicit "NOTE: RX and TX differ" be a useful addition do you think? > + * > * Ring slot size is 8 octets. > * > * rx request (netif_rx_request_t)
> -----Original Message----- [snip] > > * Guest transmit > > * ============== > > * > > + * This is the 'wire' format for packets: > > + * Fragment 1: netif_tx_request_t - flags = NETTXF_* > > + * size = total packet size > > + * [Extra 1: netif_extra_info_t] - (only if fragment 1 flags include > > + * NETTXF_extra_info) > > + * [Extra N: netif_extra_info_t] - (only if extra N-1 flags include > > + * XEN_NETIF_EXTRA_MORE) > > + * ... > > + * Fragment N: netif_tx_request_t - (only if fragment N-1 flags include > > + * NETTXF_more_data) > > For Fragment 2 is it the Flags of Fragment N-1 = 1 which are relevant, or > the flags in the optional Extra N which may be in the middle (i.e. the > immediately preceding slot)? It's Fragment N-1's flags. The flags on the Extras are not relevant. In fact the only Extra flag defined is the one that says there's another Extra :-) > > Am I correct in remembering that in the presence of NETTXF_more_data the > only way to know the actual size of Fragment 1 is to take away the total of > all the extras from Frag 1's size? That's right. > > > + * flags = 0 > > + * size = fragment size > > + * > > * Ring slot size is 12 octets, however not all request/response > > * structs use the full size. > > * > > @@ -202,6 +202,19 @@ > > * Guest receive > > * ============= > > * > > + * This is the 'wire' format for packets: > > + * Fragment 1: netif_rx_request_t - flags = NETRXF_* > > + * size = fragment size > > + * [Extra 1: netif_extra_info_t] - (only if fragment 1 flags include > > + * NETRXF_extra_info) > > + * [Extra N: netif_extra_info_t] - (only if extra N-1 flags include > > + * XEN_NETIF_EXTRA_MORE) > > + * ... > > + * Fragment N: netif_rx_request_t - (only if fragment N-1 flags include > > + * NETRXF_more_data) > > + * flags = 0 > > + * size = fragment size > > Same Q re which NETRXF_more_data is relevant. > Same answer :-) > In this path there is no indication of the total packet size other than > adding everything up? > Correct. > Given that they differ in a subtle way would a quick but explicit "NOTE: RX > and TX differ" be a useful addition do you think? > Yes, that's probably a good plan. I'll stick an extra comment in to that effect. Paul > > + * > > * Ring slot size is 8 octets. > > * > > * rx request (netif_rx_request_t)
On Fri, 2016-01-08 at 15:56 +0000, Paul Durrant wrote: > > -----Original Message----- > [snip] > > > * Guest transmit > > > * ============== > > > * > > > + * This is the 'wire' format for packets: > > > + * Fragment 1: netif_tx_request_t - flags = NETTXF_* > > > + * size = total packet size > > > + * [Extra 1: netif_extra_info_t] - (only if fragment 1 flags > > > include > > > + * NETTXF_extra_info) > > > + * [Extra N: netif_extra_info_t] - (only if extra N-1 flags > > > include > > > + * XEN_NETIF_EXTRA_MORE) > > > + * ... > > > + * Fragment N: netif_tx_request_t - (only if fragment N-1 flags > > > include > > > + * NETTXF_more_data) > > > > For Fragment 2 is it the Flags of Fragment N-1 = 1 which are relevant, > > or > > the flags in the optional Extra N which may be in the middle (i.e. the > > immediately preceding slot)? > > It's Fragment N-1's flags. The flags on the Extras are not relevant. I think it would be worth saying that explicitly, even though your text is strictly correct someone might not quite follow. > In fact the only Extra flag defined is the one that says there's another > Extra :-) As opposed to extra types of which there are several. > > > > > Am I correct in remembering that in the presence of NETTXF_more_data the > > only way to know the actual size of Fragment 1 is to take away the total of > > all the extras from Frag 1's size? > > That's right. Can you add a note please? > > Same Q re which NETRXF_more_data is relevant. > > > > Same answer :-) Same request ;-) > > In this path there is no indication of the total packet size other than > > adding everything up? > > > > Correct. Again perhaps worth a note. > > Given that they differ in a subtle way would a quick but explicit > > "NOTE: RX > > and TX differ" be a useful addition do you think? > > > > Yes, that's probably a good plan. I'll stick an extra comment in to that > effect. Ta.
diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h index e103cf3..1790ea0 100644 --- a/xen/include/public/io/netif.h +++ b/xen/include/public/io/netif.h @@ -151,22 +151,22 @@ */ /* - * This is the 'wire' format for packets: - * Request 1: netif_tx_request_t -- NETTXF_* (any flags) - * [Request 2: netif_extra_info_t] (only if request 1 has - * NETTXF_extra_info) - * [Request 3: netif_extra_info_t] (only if request 2 has - * XEN_NETIF_EXTRA_MORE) - * Request 4: netif_tx_request_t -- NETTXF_more_data - * Request 5: netif_tx_request_t -- NETTXF_more_data - * ... - * Request N: netif_tx_request_t -- 0 - */ - -/* * Guest transmit * ============== * + * This is the 'wire' format for packets: + * Fragment 1: netif_tx_request_t - flags = NETTXF_* + * size = total packet size + * [Extra 1: netif_extra_info_t] - (only if fragment 1 flags include + * NETTXF_extra_info) + * [Extra N: netif_extra_info_t] - (only if extra N-1 flags include + * XEN_NETIF_EXTRA_MORE) + * ... + * Fragment N: netif_tx_request_t - (only if fragment N-1 flags include + * NETTXF_more_data) + * flags = 0 + * size = fragment size + * * Ring slot size is 12 octets, however not all request/response * structs use the full size. * @@ -202,6 +202,19 @@ * Guest receive * ============= * + * This is the 'wire' format for packets: + * Fragment 1: netif_rx_request_t - flags = NETRXF_* + * size = fragment size + * [Extra 1: netif_extra_info_t] - (only if fragment 1 flags include + * NETRXF_extra_info) + * [Extra N: netif_extra_info_t] - (only if extra N-1 flags include + * XEN_NETIF_EXTRA_MORE) + * ... + * Fragment N: netif_rx_request_t - (only if fragment N-1 flags include + * NETRXF_more_data) + * flags = 0 + * size = fragment size + * * Ring slot size is 8 octets. * * rx request (netif_rx_request_t)
Currently there is no documented wire format for guest receive-side packets but the location of the 'wire format' comment block suggests it is the same as transmit-side. This is almost true but there is a subtle difference in the use of the 'size' field for the first fragment. For clarity this patch creates separate comment blocks for receive and transmit side packet wire formats, tries to be more clear about the distinction between 'fragments' and 'extras', and documents the subtlety concerning the size field of the first fragment. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Keir Fraser <keir@xen.org> Cc: Tim Deegan <tim@xen.org> --- xen/include/public/io/netif.h | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-)