[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070828130704.GA633@pingi.kke.suse.de>
Date: Tue, 28 Aug 2007 15:07:04 +0200
From: Karsten Keil <kkeil@...e.de>
To: Stephen Hemminger <shemminger@...ux-foundation.org>
Cc: isdn4linux@...tserv.isdn4linux.de, netdev@...r.kernel.org
Subject: Re: [PATCH] isdn capi driver broken on 64 bit.
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 <kkeil@...e.de> 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 <shemminger@...ux-foundation.org>
> > >
> > >
> > > --- 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.
-
--
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 16746 (AG Nuernberg)
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists