On Tue, 28 Aug 2007 15:07:04 +0200 Karsten Keil <[EMAIL PROTECTED]> wrote:
> Hello Steven, > > On Mon, Aug 27, 2007 at 09:16:55AM -0700, Stephen Hemminger wrote: > > On Mon, 27 Aug 2007 13:02:26 +0200 > > Karsten Keil <[EMAIL PROTECTED]> wrote: > > > > > On Fri, Aug 24, 2007 at 11:08:11AM -0700, Stephen Hemminger wrote: > > > > The following driver API is broken on any architecture with 64 bit > > > > addresses. > > > > because of cast that loses high bits. > > > > > > > > Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]> > > > > > > > > > > > > --- a/drivers/isdn/capi/capidrv.c 2007-06-25 09:03:12.000000000 > > > > -0700 > > > > +++ b/drivers/isdn/capi/capidrv.c 2007-08-24 11:06:46.000000000 > > > > -0700 > > > > @@ -1855,6 +1855,9 @@ static int if_sendbuf(int id, int channe > > > > return 0; > > > > } > > > > datahandle = nccip->datahandle; > > > > + > > > > + /* This won't work on 64 bit! */ > > > > + BUILD_BUG_ON(sizeof(skb->data) > sizeof(u32)); > > > > capi_fill_DATA_B3_REQ(&sendcmsg, global.ap.applid, > > > > card->msgid++, > > > > nccip->ncci, /* adr */ > > > > (u32) skb->data, /* Data */ > > > > > > > > > NACK. > > > > > > It is not a BUG. > > > > > > This is OK, since this field must have a value and on 32 it has the > > > correct > > > one) On 64 bit this field is ignored (but also need a value, random data > > > is > > > bad as well). > > > > If you are using it as a transaction ID, then you should generate one. > > There is no guarantee that two skb's won't have the same 32 bit data value > > on 64 bit. > > No, it's the address of the data buffer, but nobody use it in linux kernel. > A CAPI DATA_B3 message looks like this in Linux: > > <8 Byte CAPI HEADER> > <32 bit NCCI> > <32 bit Pointer to DataBuffer> > <16 bit length> > <16 bit handle> > <16 bit Flags> > <Start of DataBuffer> > > > So the payload data follows directely the Message header and inside > Linux we use this to address the data. But we must fill the pointer > with some data, so we use the correct value for 32 bit, for 64 bit > it is wrong yes, but since the value is not used ..., I want to > avoid using a special case for 64 bit (e.g. setiing it to zero). > Other OS use seperate buffers for the payload data, thats the reason why > this field exist. > For 64 bit a (optional) extention exist, which place a 64 bit Pointer > beween <16 bit Flags> and <Start DataBuffer> so you can adress seperate > data buffers under 64 as well, but we do no use that inside kernel CAPI > implementation. > Okay, maybe you want to take that nice explanation and put it as a comment as to what is going on there. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
