Re: [lwip-users] snd_queuelen does not count the number of segments...

[Kieran Mansley]

On Tue, 2006-11-28 at 13:48 -0500, Tom Hennen wrote:
> I think there's been some miscommunication.  But it's helped me to get
> a better handle on what is going on.
>  
> When I'm queuing packets I'm actually queuing *outgoing* packets (and
> I'm not using any threading).  The problem that I've described pops up
> during tcp retransmissions.
>  
> Imagine this scenario:
>  
> 1. TCP sends segments A B C, incrementing pcb->snd_queuelen by 3
> 2. The link layer queues A B and C, so they all form one long pbuf
> chain
> 3. The link layer sends A B and C, removing them from the queue,
> so A B and C are no longer chained
> 4. No ACK has arrived for A B and C yet and so tcp_rexmit_rto
> moves A B and C from the unacked list to the unsent list
> 5. tcp_output then moves A B and C from the unsent list back to the
> unacked list and then sends the packets via ip_output
> 6. The link layer queues A B and C, forming one long pbuf chain (NOTE:
> the packets have not yet been transmitted)
> 7. An ACK arrives for segment A
> 8. tcp_receive decrements pcb->snd_queuelen by pbuf_clen(A)
> 8a. since A B and C are still queued in the link layer the chain
> length is 3 instead of 1, thus pcb->snd_queuelen is decremented by 3
> instead of the correct value of 1.
> 9. An ACK arrives for segment B
> 10. tcp_receive decrements pcb->snd_queuelen by pbuf_clen(B)
> 10a. since A B and C are still queued in the link layer the chain
> length is 2 instead of 1, thus pcb->snd_queuelen is decremented by 2
> instead of the correct value of 1.  At this point pcb->snd_queuelen is
> -1(or 255) instead of 1.
>  
> So the packet queuing by the link layer combines with a subtle TCP
> timing issue to cause this problem.  If the link layer didn't queue
> packets, or if it could transmit the packets faster, this wouldn't
> occur.
>  
> So, how should this problem be resolved?  Should I be able to use pbuf
> queues for outgoing packets in the link layer?  If so, then I would
> think tcp should use something other than pbuf_clen to determine how
> many pbufs to decrement snd_queuelen by.

Ahh, apologies for misunderstanding you.  I think by using pbuf_queue()
you are essentially corrupting the internal state in lwIP.  pbuf_queue()
modifies the next pointer in the pbuf, but this is already being used
(for the unsent/unacked lists) at the time you are modifying it.  Our
pbuf queues/lists/chains only support the pbuf being in one
queue/list/chain at once, and by adding it to another you're breaking
that assumption.

To resolve it, one solution would be to create yourself a little link-
layer container structure for a pbuf:

struct link_pbuf {
  struct link_pbuf *next;
  struct pbuf *p;
}

Then you can use this next pointer to queue them up however you like
internally to the link layer without risk of corrupting the higher
layers.

Kieran