diff options
author | Mike Christie <michaelc@cs.wisc.edu> | 2007-08-13 12:32:09 -0500 |
---|---|---|
committer | Mike Christie <michaelc@cs.wisc.edu> | 2007-08-13 12:32:09 -0500 |
commit | 95485deba5f177943fdefc3fa0e3a16a43806e33 (patch) | |
tree | e0da2281c0606f2d336bb784c208305b4af57d18 /etc | |
parent | 52b5eb9cdb5a6d20970b99ebd180110c55042702 (diff) | |
download | open-iscsi-95485deba5f177943fdefc3fa0e3a16a43806e33.tar.gz |
fix hash/crypto sg list computation
from erezz@voltaire.com
>> >> I have a problem with the RH4 backport, and I hope that you can help me
>> >> with that. I'm using the crypto_hash functions from
>> >> 2.6.16-18_compat.patch, and something goes wrong:
>> >>
>> >>
>> >> Here's the call stack:
>> >>
>> >>
>> >> crypto_digest_update (called from 2.6.16-18_compat.patch)
>> >>
>> >> crypto_hash_update
>> >>
>> >> partial_sg_digest_update
>> >>
>> >> iscsi_send_data
>> >>
>> >> iscsi_tcp_ctask_xmit
>> >>
>> >>
>> >> crypto_digest_update is called with nbytes = 8192. I can also see that
>> >> the scatterlist array has a single entry, and sg[0].length = 8192. Now,
>> >> the problem is that crypto_digest_update is called in the following way:
> >
> > Yeah, it is wrong. I think I sent mail to Boaz (Boaz did the
> > conversion patch) about this on the list.
> >
>> >>
>> >>
>> >> crypto_digest_update(desc->tfm, sg, (nbytes+(PAGE_SIZE-1)) / PAGE_SIZE);
>> >>
>> >>
>> >> (nbytes+(PAGE_SIZE-1)) / PAGE_SIZE = 2 (while the sg array length is
>> >> actually 1). Then, I get a NULL ptr error...
> >
> > Yeah, that calculation does not take into account offsets and other
> > cases.
> >
>> >>
>> >>
>> >> I took a look at older code (r702), and I see that crypto_digest_update
>> >> is called with len = 1. Can you explain the length calculation in the
>> >> backport (i.e. (nbytes+(PAGE_SIZE-1)) / PAGE_SIZE)?
>> >>
>> >>
>> >> Should it be something like the code below?
>> >>
>> >>
>> >> static inline int crypto_hash_update(struct hash_desc *desc,
>> >> struct scatterlist *sg,
>> >> unsigned int nbytes)
>> >> {
>> >>
>> >> crypto_digest_update(desc->tfm, sg, 1);
>> >>
>> >> return nbytes;
>> >> }
>> >>
>> >>
>> >> When I use this code, it works ok. However, I'm not sure if it's the
>> >> right way to do that.
>> >>
> >
> > Yeah, this looks about right. For open-iscsi we do not support
> > scsi_host_template->clustering. This means that all scatterlist
> > elements should only have one element (scsi/block layer will send
> > scatterlists like that and open-iscsi itself will do the same for
> > headers), and this "(nbytes+(PAGE_SIZE-1)) / PAGE_SIZE" calculation is
> > not really needed and is wrong anyways.
> >
> > What I just did for RHEL5 was to drop the digest to hash conversion
> > code. I had to cut out the hash changes and replace them with native
> > digest calls. The problem with that is that it is difficult to
> > maintain such an invasive change since upstream changes so much in
> > those paths.
Thanks. I've attached a patch that replaces the "(nbytes+(PAGE_SIZE-1))
/ PAGE_SIZE" with "1". The same fix was done in open-iscsi for OFED.
Diffstat (limited to 'etc')
0 files changed, 0 insertions, 0 deletions