Ishai Rabinovitz <[EMAIL PROTECTED]> writes: > On Tue, May 23, 2006 at 10:10:05AM +0300, Arne Redlich wrote: >> Roland Dreier <[EMAIL PROTECTED]> writes: >> >> > > Roland, maybe this means we need scsi/srp.h in svn for now? >> > > svn is supposed to work on 2.6.16 ... >> > >> > As far as I can tell the bug in the header has no effect on how the IB >> > SRP initiator works. >> >> Roland, >> >> I'm afraid it *does* have an effect, unfortunately. There's the following >> code in ib_srp.c::srp_map_data(), around the lines 540 - 550: >> >> struct srp_indirect_buf *buf = (void *) cmd->add_data; >> >> /* snip */ >> >> buf->table_desc.va = cpu_to_be64(req->cmd->dma + >> sizeof *cmd + >> sizeof *buf); >> >> So if a target actually RDMA Reads the indirect descriptor table, it will >> use a wrong address. >> > > It looks to me that there is no effect after all. > This buf->table_desc.va should point to the desc_list array in the > srp_indirect_buf. > When the code enters the values to this array (buf->desc_list[i]) it uses the > address that is corresponding to sizeof *buf. > > To sum it up, there will be a change in the address the target sees but the > data will still be in the address the target sees.
No, unfortunately this is wrong. The code posted below resembles the offending part in ib_srp.c. It results in this output on an x86_64: sizeof p: 24 offset of table_desc: 0 offset of len: 16 offset of desc_list: 20 addr. of p: ffff81003c389f28 p->table_desc.va: ffff81003c389f40 p->desc_list[0]: ffff81003c389f3c Arne -- Arne Redlich Xiranet Communications GmbH /* --------------- cut here ------------------------------------------------ */ #include <linux/module.h> #include <scsi/srp.h> /* my <scsi/srp.h> is already fixed, here's the broken version again */ struct srp_indirect_buf_old { struct srp_direct_buf table_desc; __be32 len; struct srp_direct_buf desc_list[0] __attribute__((packed)); }; MODULE_LICENSE("GPL"); static void __exit test_fini(void) { return; } static int __init test_init(void) { struct srp_indirect_buf_old buf, *p; p = &buf; p->table_desc.va = (u64)(unsigned long)p; p->table_desc.va += sizeof(struct srp_indirect_buf_old); printk("sizeof p: %lu\n", sizeof(*p)); printk("offset of table_desc: %lu\n", offsetof(struct srp_indirect_buf_old, table_desc)); printk("offset of len: %lu\n", offsetof(struct srp_indirect_buf_old, len)); printk("offset of desc_list: %lu\n", offsetof(struct srp_indirect_buf_old, desc_list)); printk("addr. of p: %p\n", p); printk("p->table_desc.va: %llx\n", p->table_desc.va); printk("p->desc_list[0]: %p\n", &p->desc_list[0]); return 0; } module_init(test_init); module_exit(test_fini); _______________________________________________ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general