[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc()
From: |
Bill Auerbach |
Subject: |
RE: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc() |
Date: |
Wed, 15 Jul 2009 17:15:24 -0400 |
>>> pbuf_alloc(PBUF_RAW, 0, PBUF_POOL);
>>
>> The above is used in PPP,
>
>To me, that's not a reason to allow zero-length pbufs
I was only pointing out if you assert for it this will come up and cause more
work fixing.
>the PPP code is
>unfortunately not very actively maintained and thus may (beside this
>one) contain other bugs or invalid code: lines like
>
> *((u_char*)nb->payload + nb->len++) = PPP_ESCAPE;
Ppp may not be the best example code, but if it works at least it's usable to
some. Now that you said this, how will you explain that invalid or buggy code
(or illegal use of internal data) is used in production code in CVS? :-)
>I'd rather fix that than allowing
>zero-length pbufs throughout the stack (which is in most cases a bug).
Or expand the API to access these items. Payload is one that I presume should
be off limits too, but isn't accessing it essential to zero-copy support and it
must be used in low-level drivers. There are problems reported from accessing
len instead of tot_len. An API for them, even just hidden in macros, would
keep it efficient and lower misuse. E.g.
#define pbuf_get_payload(p) (p->payload)
>So you're fiddling around with pbuf->len, too? Code like that is not
>supported!
It won't require support and I'll fix it if updates break it. Being able to
fiddle with this stuff allowed me to achieve what I wanted without (in this
case) changing any base lwIP code.
>In general, application code playing around with lwIP struct
>members without using API functions provided for that use is (except for
>some rare cases) not allowed/supported.
My case is rare then. :-) TCP is inefficient and not real-time on most
sub-200MHz embedded processors. UDP is fast but not reliable. So I wrote a
protocol for UDP that is reliable but I wanted to build it on pbufs since that
is the low-level interface to sending packets. My program, of which the
high-speed required part was TCP now uses this protocol with only 2 or 3 code
line changes.
I've learned a lot about lwIP and where it spends a lot of time. If you want
to create a task to make lwIP 3-4 times faster, I can help you do this.
Efficiency isn't prioritized very high and I had to have it, so I resorted to
doing what I had to. Sorry I fiddled with the internals - it saved this
project.
>Unfortunately, with C, it can't
>really be avoided by our code/files in a decent way.
You can make all the members you want untouched const. Yes, the internal code
has to be changed to a cast to change them, but that's one time at the benefit
of forcing people to do things properly. And people like me who have a need
otherwise can cast in their code to make the point "I broke the rules". Please
don't try to limit what we *can* do because it's not what we're *supposed* to
do.
>[There are some
>rare cases like netif->flags= where most available code uses the
>internal struct members, but I'd vote for introducing API
>functions/defines for these places, too.]
I agree. I added some functions for hostname support. API functions can be
macros (which lwIP already utilized a good bit) and efficiency doesn't have to
be sacrificed. You could use inline functions but not all compilers handle
that the same, if at all.
>You should know the length before calling pbuf_alloc() and pass it
>instead of 0.
I'll explain why I did this: I have a static pbuf that has the EHTHDR, IPHDR,
and UDPHDR filled in and those static parts checksummed. I chain this to a
payload pbuf. When I need to send UDP data, I update the 3 or 4 items in the
IPHDR and UDPHDR, and add the changes to the static checksum and fill it in. I
update the payload pointer, len and tot_len of each and call the
netif->linkoutput. It's simple and I can stream at 970MbS (my device can't
support this but it's what I get if I just point to memory and send data). One
of the reasons for the speed is the repeated pbuf_alloc/pbuf_free/udp_sendto
calls took 2/3rds of the overall time. I'm also able to maintain a TCP
connection for control packets and connection processing. It's worked great
for me.
Bill
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Iordan Neshev, 2009/07/15
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Kieran Mansley, 2009/07/15
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Kieran Mansley, 2009/07/15
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Iordan Neshev, 2009/07/15
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Kieran Mansley, 2009/07/15
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Simon Goldschmidt, 2009/07/15
- RE: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Bill Auerbach, 2009/07/15
- Re: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), address@hidden, 2009/07/15
- RE: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(),
Bill Auerbach <=
- Re: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Alain Mouette, 2009/07/15
- Re: RE: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Simon Goldschmidt, 2009/07/16
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Simon Goldschmidt, 2009/07/16
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Iordan Neshev, 2009/07/16
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Kieran Mansley, 2009/07/16
- [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Simon Goldschmidt, 2009/07/27
- Re: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), Alain Mouette, 2009/07/16
- Re: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc(), address@hidden, 2009/07/16