[lwip-users] Re: [lwip] Memory leak and queued packets in etharp.c

[Martin Glunz]

Hi Leon

I agree. Your proposal will not require changes to higher
protocol levels. It does also not require function * passing
from the ethernet driver to the etharp module.
But we'll have to implement some new chaining method
in the pbuf.c module, also the dechaining and free
functions must be modified or at least audited.
In the meantime I've found another problem in memp.c:
pbuf_ref(). It increases the reference count of the
passed pbuf, but not of the chained pbufs (if any).
Under certain circumstances this causes memp_sanity()
to fail. I've fixed this for the moment with a new function:

/*------------------------------------------------------------------------------
/* pbuf_ref_chain():
 *
 * Increments the reference count of all pbufs in a chain.
 */
/*------------------------------------------------------------------------------
void
pbuf_ref_chain(struct pbuf *p)
{
  while (p != NULL) {
    p->ref++;
    p=p->next;
  }
}

inserted into pbuf.c. This has to be called from etharp.c instead
of pbuf_ref().
But with your idea, there might be some more modifying and testing
of the pbufs necessary.
As a conclusion I think, it will be possible and a neat solution,
but some work on the pbuf module is required. I hope I can check
this out next week or so.

> 
> Hello Martin,
> 
> > > My proposal was to use the ->next pointer in combination
> > > with the tot_len field.
> > >
> > > See this chained list of 2 packets, consisting of
> > > 4 resp. 2 pbufs.
> > >
> > >         #1 #2 #3 #4  #5  #6
> > > tot_len 80 60 40 20 100  50<- this number is changed to 50 instead of
> the buggy 100.
> > > len     20 20 20 20  50  50
> > >
> 
> > Interesting Idea. Should work without adding additional fields
> > to the pbufs. But as you said:
> 
> > > However, this would change the ways pbuf's are linked and
> > > thus introduce changes for the complete stack, or at least
> > > needs a code-audit to ensure that this use of the pbuf
> > > structure does not affect current code.
> >
> I have thought about this some more. Queued packets on etharp.c do not
> affect any layers above etharp.c or some netif.c, so these two layers
> are affected anyway. So, the code changes and audit is reduced to the
> etharp.c and any Ethernet drivers.
> 
> Now, if a driver would support sending multiple packets chained in the
> way pictured above, we are done. It requires the least number of changes
> and IMHO, is the neatest solution.
> 

Martin Glunz

WANTED: Schrödinger's Cat. Dead or Alive.

There are only 10 kinds of people. Those who
understand binary and those who don't.
[This message was sent through the lwip discussion list.]