Re: [lwip-devel] (no subject)

[lukem . lwip]

On Thu, 18 Mar 2004, Leon Woestenberg wrote:

> Hello Luke,
>
> On Thu, 2004-03-18 at 06:23, address@hidden wrote:
> > Could I get some feedback on the new pbuf implementation in the
> > "lukem-pbuf" CVS branch? It should be a drop-in replacement for the
> > original implementation at the moment.
> >
> In what sense did you improve on the design? Could you elaborate?

Allows the definition of new pbuf types by code external to lwIP. This is
needed to correctly handle external memory allocators which require
callbacks for things like realloc and free. This is achieved by keeping an
extra pointer in each pbuf, which refers to a struct containing a bunch of
function pointers to the functions which should be used to perform various
operations on the pbuf. This effectively replaces the one-byte type flag
with a pointer sized pointer, which means memory impact is minimal on
small systems.

> > The only unimplemented feature right now is that POOL pbufs are just an
> > alias for RAM pbufs, and REF pbufs are just an alias for ROM pbufs. I
> > don't think this will cause anything to break, but am happy to hear
> > otherwise.
> >
> Does copy-on-queue work correctly for non-ROM REF pbufs?

I was not aware of that funtionality. I will look into it.

> > I'd like to get this branch merged soon so that I can start with
> > a new pcb lookup implementation. To that end, please let me know of
> > any changes that would be needed before a merge.
> >
> As the pbuf stuff is core to lwIP, merging would need:
> - full compatibility, or good reasons why the API contract changed.

The API has changed, but in a backwards compatible way - there are a few
compatibility functions in place, such that changes are not required
anywhere else in lwIP other than in the pbuf code. This also goes for the
standard unix port, and should work for others too.

> - a good explanation of what you did in the new pbuf code, and why.
> - improvement of the existing code in efficiency or feature-wise.
> - review by some pairs of eyes which are familiar with lwIP.
> - some testing.

Well, I have been testing it here for a while now, and have fixed any bugs
I found. Of course someone else should test in a different environment.

I really think that the best way to understand the changes would be to
look at the code - it's not that hard...

pbuf.h:
http://savannah.nongnu.org/cgi-bin/viewcvs/lwip/lwip/src/include/lwip/pbuf.h?rev=1.16.2.1&only_with_tag=lukem-pbuf&content-type=text/vnd.viewcvs-markup

pbuf.c:
http://savannah.nongnu.org/cgi-bin/viewcvs/lwip/lwip/src/core/pbuf.c?rev=1.64.2.2&only_with_tag=lukem-pbuf&content-type=text/vnd.viewcvs-markup

pbuf-ram.h:
http://savannah.nongnu.org/cgi-bin/viewcvs/lwip/lwip/src/include/lwip/Attic/pbuf-ram.h?rev=1.1.2.1&only_with_tag=lukem-pbuf&content-type=text/vnd.viewcvs-markup

pbuf-ram.c:
http://savannah.nongnu.org/cgi-bin/viewcvs/lwip/lwip/src/core/Attic/pbuf-ram.c?rev=1.1.2.1&only_with_tag=lukem-pbuf&content-type=text/vnd.viewcvs-markup

pbuf-rom.h:
http://savannah.nongnu.org/cgi-bin/viewcvs/lwip/lwip/src/include/lwip/Attic/pbuf-rom.h?rev=1.1.2.1&only_with_tag=lukem-pbuf&content-type=text/vnd.viewcvs-markup

pbuf-rom.c:
http://savannah.nongnu.org/cgi-bin/viewcvs/lwip/lwip/src/core/Attic/pbuf-rom.c?rev=1.1.2.2&only_with_tag=lukem-pbuf&content-type=text/vnd.viewcvs-markup

Again, let me re-iterate that pbuf_ref and pbuf_pool are not implemented.
Given the apparent resistance to change, I will probably end up
implementing these anyway.

--
Luke