Re: [lwip-users] Zero window and refused data problem

[Oleg Gladyshev]

Hi Simon,

first of all, I think you are "misusing" TCP as a queue here (and at least our implementation is not really meant for this). It might work (or not, as you see), but in my opinion, the various timeouts implemented by various stacks impose the risk that your setup won't work if you change the client's system (e.g. update windows).

I agree that my using of LwIP stack is a little strange, but I don't see any violations with TCP standard in my idea. Why not? :)

 

If I read correctly, by now you reported 2 possible issues:

1) segment is accepted but old window size is sent

 

I'm not sure what's best here. Of course, we must prevent silly window updates during normal operation. In your case, it would probably have been OK to send the actual/real window size, but we would haveto find a way to decide when it's OK and when not...

No, that's not issue. I understand why LwIP doesn't update window and I'm sure it is correct.

 

2) after storing a segment in "refused_data", no more ACKs are sent

 

The whole purpose of "refused_data" was to let the stack behave like a buffer overflowed: if your device cannot handle incoming data in the speed it is sent by the remote host, the remote host should throttle its sender. This is achieved by not handling/not answering a packet at all, just like it was dropped due to congestion. This should bring the remote host's TCP to send less. ACKing an old seqno instead might work for you, but I don't know what will be the result for all remote stacks, so I'm very reluctant to change this...

 

As you can see from this, TCP is meant to achieve the highest possible throughput possible for the combination of remote host, network and local host. What you want instead to make it a queue that keeps up a connection as long as possible without data being exchanged. I'm not fully convinced one can coexist with the other, but please come up with suggestions of how to fix this ;-)

 

Let's look at the problem more carefully. Suppose my device received full buffer, after that it read few bytes from the buffer and completely stopped reading.
Now sender (host) sees zero announced window, but in fact device can accept some bytes now.
With current LwIP realization from senders point of view we can observe such behavior: after peer's window become zero it answered to some window probe packets incrementing ACK fieild (while inner message box was not full) and after that it stopped answering at all. This will be threated as broken channel and sender will disconnect. And it will be right! But undesired for me.

But if LwIP could answer with old ACK value if refused_data still not null, from sender's point of view this will look like window is really full and device can't accept any new data. Nothing strange. And I think most TCP realizations covers this case.

I made small patch to force LwIP answering if refused_data value != 0. And this works for me. I made a test using Windows and Linux as senders and both of them did not break connection because they noticed that peer's TCP window is really zero.
But I wonder if my patch is correct.
Simon, could you look at the patch and say your opinion?

---
 core/tcp_in.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/core/tcp_in.c b/core/tcp_in.c
index 4ec971a..249f370 100644
--- a/core/tcp_in.c
+++ b/core/tcp_in.c
@@ -291,14 +291,22 @@ tcp_input(struct pbuf *p, struct netif *inp)
 
     /* If there is data which was previously "refused" by upper layer */
     if (pcb->refused_data != NULL) {
-      if ((tcp_process_refused_data(pcb) == ERR_ABRT) ||
-        ((pcb->refused_data != NULL) && (tcplen > 0))) {
+      if (tcp_process_refused_data(pcb) == ERR_ABRT) {
         /* pcb has been aborted or refused data is still refused and the new
            segment contains data */
         TCP_STATS_INC(tcp.drop);
         snmp_inc_tcpinerrs();
         goto aborted;
       }
+      if ((pcb->refused_data != NULL) && (tcplen > 0)) {
+          tcp_input_pcb = NULL;
+          /* Try to send something out. */
+          tcp_ack_now(pcb);
+          tcp_output(pcb);
+          TCP_STATS_INC(tcp.drop);
+          snmp_inc_tcpinerrs();
+          goto aborted;
+      }
     }
     tcp_input_pcb = pcb;
     err = tcp_process(pcb);
--

-- 
Best regards, Oleg