RE: [lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc()

[Bill Auerbach]

>>>  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