[PATCH v3 2/5] bbremote: Fix RATP handshake, errata #7321 for RFC916

[Jules Maselbas <jmaselbas@kalray.eu>]

   Side A                                             Side B
1. CLOSED                                             LISTEN
2. [OPEN request]
      SYN_SENT -->        <SN=0><CTL=SYN>         --> SYN_RECEIVED
3. ESTABLISHED <--   <SN=0><AN=1><CTL=SYN,ACK>    <--
4.             -->      <SN=1><AN=0><CTL=ACK>     ...
5.             ...   <SN=0><AN=1><CTL=SYN,ACK>    <-- (retransmit)
6. (packet sent by A at 4. finally arrives to B)
               ...                                --> ESTABLISHED
7. (packet resent by B at 5. finally arrives to A)
   CLOSED (C2) <--                                ...
8.             -->      <SN=1><AN=1><CTL=RST>     --> (connection reset)

The Figure above illustrate the current issue RATP can face during the
three-way handshake, and how behavior C2 can cause a connection to be
closed immediately after being established.

In the Figure above, side A try to establish a connection with side B,
which is in the LISTEN state. Commented line:
 1. side A is in the CLOSED state and side B is in the LISTEN state;
 2. side A open a new connection and send a SYN packet and is received by
    side B which enters the SYN_RECEIVED state and send back a SYN-ACK;
 3. side A receive the SYN-ACK packet from B;
 4. side A respond with an ACK packet and move to the ESTABLISHED state.
    Meanwhile;
 5. side B hasn't received yet the ACK from side A and decide to
    retransmit the SYN-ACK packet;
 6. side B finally receive the ACK from side A and move to the
    ESTABLISHED state;
 7. side A finally receive the duplicated SYN-ACK packet from side B,
    execute behavior C2: the received packet doesn't have the expected
    SN and has the SYN flag set, thus respond by sending a legal reset.
  8. side B receive the reset and close the connection.

One solution could be to tweak the initial RTO value of side B in order
to prevent sending a duplicated SYN-ACK packet, however the initial RTT
value is likely inaccurate during the handshake. This solution seems a
bit brittle.

The second solution would be to consider that a host has crashed only if
the packet received has the SYN flag set but not the ACK flag. The
rational is that the first step during handshake is to send a packet
only containing the SYN flag, however a packet containing both ACK and
SYN flags must have come after the initial handshake exchange and can
be considered as a duplicated and be dropped.

I proposed the following errata to RFC916 [1]:
[Page 29]
- If SYN was set we assume that the other end crashed and has
- attempted to open a new connection. We respond by sending a
- legal reset:
+ If the SYN flag was set but the ACK was not set then we assume
+ that the other end crashed and has attempted to open a new connection.
+ We respond by sending a legal reset:

[Page 30]
- If neither RST, FIN, nor SYN flags were set it is assumed that
- this packet is a duplicate of one already received. Send an
- ACK back:
+ If neither RST nor FIN flags were set, or if SYN and ACK flags
+ were set, it is assumed that this packet is a duplicate of one
+ already received. Send an ACK back:

[1] https://www.rfc-editor.org/errata/eid7321

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 scripts/remote/ratp.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/remote/ratp.py b/scripts/remote/ratp.py
index 25ca442d15..956c788234 100644
--- a/scripts/remote/ratp.py
+++ b/scripts/remote/ratp.py
@@ -345,7 +345,7 @@ class RatpConnection(object):
         if r.c_rst or r.c_fin:
             return False
 
-        if r.c_syn:
+        if r.c_syn and not r.c_ack:
             s = RatpPacket(flags='RA')
             s.c_sn = r.c_an
             s.c_an = (r.c_sn + 1) % 2
-- 
2.17.1