Re: [Xen-devel] [RFC PATCH] Data integrity extension support for xen-block

[Juergen Gross <jgross@xxxxxxxx>]

On 07/04/16 12:00, Bob Liu wrote:
> * What's data integrity extension and why?
> Modern filesystems feature checksumming of data and metadata to protect 
> against
> data corruption.  However, the detection of the corruption is done at read 
> time
> which could potentially be months after the data was written.  At that point 
> the
> original data that the application tried to write is most likely lost.
> 
> The solution in Linux is the data integrity framework which enables protection
> information to be pinned to I/Os and sent to/received from controllers that
> support it. struct bio has been extended with a pointer to a struct bip which
> in turn contains the integrity metadata. The bip is essentially a trimmed down
> bio with a bio_vec and some housekeeping.
> 
> * Issues when xen-block get involved.
> xen-blkfront only transmits the normal data of struct bio while the integrity
> metadata buffer(struct bio_integrity_payload in each bio) is ignored.
> 
> * Proposal of transmitting bio integrity payload.
> Adding an extra request following the normal data request, this extra request
> contains the integrity payload.
> The xen-blkback will reconstruct an new bio with both received normal data and
> integrity metadata.
> 
> Welcome any better ideas, thank you!
> 
> [1] http://lwn.net/Articles/280023/
> [2] https://www.kernel.org/doc/Documentation/block/data-integrity.txt
> 
> Signed-off-by: Bob Liu <bob.liu@xxxxxxxxxx>
> ---
>  xen/include/public/io/blkif.h |   50 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> index 99f0326..3d8d39f 100644
> --- a/xen/include/public/io/blkif.h
> +++ b/xen/include/public/io/blkif.h
> @@ -635,6 +635,28 @@
>  #define BLKIF_OP_INDIRECT          6
>  
>  /*
> + * Recognized only if "feature-extra-request" is present in backend xenbus 
> info.
> + * A request with BLKIF_OP_EXTRA_FLAG indicates an extra request is followed
> + * in the shared ring buffer.
> + *
> + * By this way, extra data like bio integrity payload can be transmitted from
> + * frontend to backend.
> + *
> + * The 'wire' format is like:
> + *  Request 1: xen_blkif_request
> + * [Request 2: xen_blkif_extra_request]    (only if request 1 has 
> BLKIF_OP_EXTRA_FLAG)
> + *  Request 3: xen_blkif_request
> + *  Request 4: xen_blkif_request
> + * [Request 5: xen_blkif_extra_request]    (only if request 4 has 
> BLKIF_OP_EXTRA_FLAG)
> + *  ...
> + *  Request N: xen_blkif_request
> + *
> + * If a backend does not recognize BLKIF_OP_EXTRA_FLAG, it should *not* 
> create the
> + * "feature-extra-request" node!
> + */
> +#define BLKIF_OP_EXTRA_FLAG (0x80)
> +
> +/*
>   * Maximum scatter/gather segments per request.
>   * This is carefully chosen so that sizeof(blkif_ring_t) <= PAGE_SIZE.
>   * NB. This could be 12 if the ring indexes weren't stored in the same page.
> @@ -703,6 +725,34 @@ struct blkif_request_indirect {
>  };
>  typedef struct blkif_request_indirect blkif_request_indirect_t;
>  
> +enum blkif_extra_request_type {
> +     BLKIF_EXTRA_TYPE_DIX = 1,               /* Data integrity extension 
> payload.  */
> +};
> +
> +struct bio_integrity_req {
> +     /*
> +      * Grant mapping for transmitting bio integrity payload to backend.
> +      */
> +     grant_ref_t *gref;
> +     unsigned int nr_grefs;
> +     unsigned int len;
> +};

How does the payload look like? It's structure should be defined here
or a reference to it's definition in case it is a standard should be
given.

> +
> +/*
> + * Extra request, must follow a normal-request and a normal-request can
> + * only be followed by one extra request.
> + */

Wouldn't it be possible to include this in the original request (e.g.
it could be the first or last indirect segment) ?


Juergen

> +struct blkif_request_extra {
> +     uint8_t type;           /* BLKIF_EXTRA_TYPE_* */
> +     uint16_t _pad1;
> +#ifndef CONFIG_X86_32
> +     uint32_t _pad2;         /* offsetof(blkif_...,u.extra.id) == 8 */
> +#endif
> +     uint64_t id;
> +     struct bio_integrity_req bi_req;
> +} __attribute__((__packed__));
> +typedef struct blkif_request_extra blkif_request_extra_t;
> +
>  struct blkif_response {
>      uint64_t        id;              /* copied from request */
>      uint8_t         operation;       /* copied from request */
> 


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