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

[Tom Hennen]

Instead it counts the number of pbufs.  Why?

 

Looking at tcp_enqueue it looks as though snd_queuelen (via 'queuelen') is counting the number of pbufs in the queue rather than the number of segments.  Specifically on lines 231 and 244 of tcp_out.c '2' is added to queuelen even though only 1 segment is added.

 

This is further supported by the use of pbuf_clen on line 762 of tcp_in.c where the number of pbufs in the segment chain is used to decrement the queue length.

 

Is there any reason for this?

 

Why not just count the number of segments and only increment and decrement by '1' for each segment?  Wouldn't the actual number of segments make more sense?

 

Thanks,

 

Tom Hennen

[Kieran Mansley]

On Mon, 2006-11-27 at 15:57 -0500, Tom Hennen wrote:
> Instead it counts the number of pbufs. Why?
>

Because the configuration limit is also specified in the number of
pbufs. This allows people using it to have a much better estimate of
the amount of pbufs (and so memory) that can be used for the send queue,
which is important if you haven't got much memory (as per most lwIP
systems).

If we just counted segments, which can vary somewhat in size, you would
end up over-limiting the number of very small segments that can be
queued as your limit would have to be small enough to prevent lots of
large segments being queued as they would use up too much memory.

Perhaps counting the bytes would make most sense, but as our pbufs are
the resource it's trying to prevent being entirely consumed by the send
queue, I'm happy to leave it as it is.

Are you seeing a real problem, or just curious about why it's like that?

Kieran

_______________________________________________
lwip-users mailing list
lwip-...@nongnu.org
http://lists.nongnu.org/mailman/listinfo/lwip-users

[Tom Hennen]

I think I understand you're reasoning, after a fashion. 

 

It does seem to be a problem for me because I'm queuing the pbufs in my link layer driver (using pbuf_queue).  As a result the length of the pbuf chain reported by pbuf_clen is not what tcp_in expects it to be and so pcb->snd_queuelen ends up wrapping around from 0 to ~255 whenever pbuf_clen(next->p) > pcb->snd_queuelen.

 

This seems to indicate that I shouldn't be using pbuf_queue in the link layer driver and that I should be doing something else instead; pbuf_queue is just so convenient...
 

Might one possible solution be to store the number of pbufs in the segment structure?  That or instead of using pbuf_clen use something like pbuf_packetlen which uses the packet end indicator of len == tot_len instead of next == NULL?

 

Thanks,

 

Tom
 

[Kieran Mansley]

On Tue, 2006-11-28 at 08:53 -0500, Tom Hennen wrote:
> I think I understand you're reasoning, after a fashion.
>
> It does seem to be a problem for me because I'm queuing the pbufs in
> my link layer driver (using pbuf_queue). As a result the length of
> the pbuf chain reported by pbuf_clen is not what tcp_in expects it to
> be and so pcb->snd_queuelen ends up wrapping around from 0 to ~255
> whenever pbuf_clen(next->p) > pcb->snd_queuelen.

That packets that you pass in to lwIP from the link layer should have no
direct effect on snd_queuelen, so how you construct them should be of no
consequence.

When an ACK is passed in from your link layer snd_queuelen is
decremented by the number of pbufs in ***the packet that is
acknowledged***, not the number of pbufs in the packet that you've just
passed in that does the acknowledging.

snd_queuelen was earlier (in tcp_out.c:tcp_enqueue()) incremented by the
number of pbufs in the same packet (when it was enqueued on the
unacknowledged or unsent list) so there is something really wrong if
you've ended up with snd_queuelen < the number of pbufs in one of the
packets in those queues. We should probably be asserting this in
tcp_in.c:tcp_receive().

For this to happen, my guess is your packet queues are getting
corrupted, most likely due to insufficient locking and protection of the
stack resulting in two threads accessing it at the same time.

[Tom Hennen]

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.

 

Thanks for the help,

 

Tom
 

On 11/28/06, Kieran Mansley <kie...@recoil.org> wrote:

[Kieran Mansley]

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.