From b5114cf8a1b87219330fba4d3189788d8fc1a8d1 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:08 +0200 Subject: ratp: add missing transition to SYN-RECEIVED in behavior B The reference says: If the SYN flag was set but the ACK was not set then the other end of the connection has executed an active open also. Acknowledge the SYN, choose your MDL, and send: Go to the SYN-RECEIVED state without any further processing. Add this missing step. This fix isn't related to any barebox<->bbremote interaction issue, because the case where both ends start an active connection doesn't apply in this case. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index 22e83636fd..0d384aa4e9 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -619,21 +619,25 @@ static void ratp_behaviour_b(struct ratp_internal *ri, void *pkt) if (hdr->control & RATP_CONTROL_SYN) { uint8_t control; + ri->sn_received = ratp_sn(hdr); + if (hdr->control & RATP_CONTROL_ACK) { control = ratp_set_sn(ratp_an(hdr)) | ratp_set_an(!ratp_sn(hdr)) | RATP_CONTROL_ACK; + ratp_send_hdr(ri, control); + ratp_state_change(ri, RATP_STATE_ESTABLISHED); } else { + struct ratp_header synack = {}; + control = ratp_set_an(!ratp_sn(hdr)) | RATP_CONTROL_SYN | RATP_CONTROL_ACK; + ratp_create_packet(ri, &synack, control, 255); + ratp_send_pkt(ri, &synack, sizeof(synack)); + ratp_state_change(ri, RATP_STATE_SYN_RECEIVED); } - - ri->sn_received = ratp_sn(hdr); - - ratp_send_hdr(ri, control); - ratp_state_change(ri, RATP_STATE_ESTABLISHED); } } -- cgit v1.2.3 From 95ec61ecb0bd1aad69b7f387768369c09ff532b4 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:09 +0200 Subject: ratp: avoid unnecessary variable initializations This is just a cleanup; the variables are completely initialized later on so the initial values are totally discarded anyway. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index 0d384aa4e9..e9536499e2 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -382,7 +382,7 @@ static int ratp_send_ack(struct ratp_internal *ri, struct ratp_header *hdr) static int ratp_send_next_data(struct ratp_internal *ri) { uint16_t crc; - uint8_t control = RATP_CONTROL_ACK; + uint8_t control; struct ratp_header *hdr; int pktlen; struct ratp_message *msg; @@ -594,7 +594,7 @@ static void ratp_behaviour_b(struct ratp_internal *ri, void *pkt) if ((hdr->control & RATP_CONTROL_ACK) && !ratp_an_expected(ri, hdr)) { if (!(hdr->control & RATP_CONTROL_RST)) { - uint8_t control = RATP_CONTROL_RST; + uint8_t control; control = RATP_CONTROL_RST | ratp_set_sn(ratp_an(hdr)); -- cgit v1.2.3 From bdafe8150dddcde945170dfdb268542a128ea038 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:10 +0200 Subject: ratp: send missing RST in behavior C2 The reference says: 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: Add this missing step. This issue affects the barebox<->bbremote interaction if bbremote crashes without closing the channel and restarts opening a complete new connection. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/ratp.c b/lib/ratp.c index e9536499e2..a8ac52c75f 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -731,8 +731,15 @@ static int ratp_behaviour_c2(struct ratp_internal *ri, void *pkt) return 1; if (hdr->control & RATP_CONTROL_SYN) { + uint8_t control; + ri->status = -ECONNRESET; pr_debug("Error: Connection reset\n"); + + control = RATP_CONTROL_RST | RATP_CONTROL_ACK | + ratp_set_sn(ratp_an(hdr)) | ratp_set_an(!ratp_sn(hdr)); + ratp_send_hdr(ri, control); + ratp_state_change(ri, RATP_STATE_CLOSED); return 1; } -- cgit v1.2.3 From f86ab0b1317ae1373f89edef945a63f564d06538 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:11 +0200 Subject: ratp: add missing RST flag in behavior G The reference says: If the ACK flag was set then send: If the ACK flag was not set then send: (i.e. in both cases RST is needed) The RST flag is set initially in the 'control' variable; so instead of completely overwriting it, make sure the additional flags are OR-ed to the current value. This fix isn't related to any failed barebox<->bbremote interaction (it really is a very rare corner case). Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ratp.c b/lib/ratp.c index a8ac52c75f..43b8b04dc7 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -1038,7 +1038,7 @@ static int ratp_behaviour_g(struct ratp_internal *ri, void *pkt) if (hdr->control & RATP_CONTROL_ACK) control |= ratp_set_sn(ratp_an(hdr)); else - control = ratp_set_an(ratp_sn(hdr) + 1) | RATP_CONTROL_ACK; + control |= ratp_set_an(ratp_sn(hdr) + 1) | RATP_CONTROL_ACK; ratp_send_hdr(ri, control); -- cgit v1.2.3 From 072505f4ede34bc533396dd48275c117529bd5b9 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:12 +0200 Subject: ratp: completely ignore RST flagged packets in behavior G The reference says: This procedure represents the behavior of the CLOSED state of a connection. All incoming packets are discarded. If the packet had the RST flag set take no action. Otherwise it is necessary to build a RST packet. So, skip building the RST packet if the incoming one had RST set. This commit fixes an infinite loop of messages sent and received between both ends during the connection close procedure, found when testing barebox against a third party ratp implementation. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/ratp.c b/lib/ratp.c index 43b8b04dc7..d3c252047a 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -1033,6 +1033,9 @@ static int ratp_behaviour_g(struct ratp_internal *ri, void *pkt) pr_debug("%s\n", __func__); + if (hdr->control & RATP_CONTROL_RST) + return 0; + control = RATP_CONTROL_RST; if (hdr->control & RATP_CONTROL_ACK) -- cgit v1.2.3 From ed2f11bbe3ead86611afbef011d9c6ac72c376ff Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:13 +0200 Subject: ratp: fix data presence check Looking at the "data length" and SO flag isn't enough to declare a packet with or without data, because SYN flagged packets will also use the "data length" field to define MDL. So, improve the check to match against SYN|RST|FIN flagged packets, which can never have data. This commit fixed a segfault in barebox when an unexpected SYN packet was sent in the middle of a connection; barebox thought the packet had data because the "data length" in the SYN packet was different than 0. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 4 ++-- scripts/remote/ratp.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index d3c252047a..c946bea1a5 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -165,7 +165,7 @@ static bool ratp_has_data(struct ratp_header *hdr) { if (hdr->control & RATP_CONTROL_SO) return 1; - if (hdr->data_length) + if (!(hdr->control & (RATP_CONTROL_SYN | RATP_CONTROL_RST | RATP_CONTROL_FIN)) && hdr->data_length) return 1; return 0; } @@ -1338,7 +1338,7 @@ static int ratp_behaviour_i1(struct ratp_internal *ri, void *pkt) struct ratp_header *hdr = pkt; uint8_t control = 0; - if (!hdr->data_length && !(hdr->control & RATP_CONTROL_SO)) + if (!ratp_has_data (hdr)) return 1; pr_vdebug("%s **received** %d\n", __func__, hdr->data_length); diff --git a/scripts/remote/ratp.py b/scripts/remote/ratp.py index 079fb871a3..a41d2e8a3f 100644 --- a/scripts/remote/ratp.py +++ b/scripts/remote/ratp.py @@ -525,7 +525,7 @@ class RatpConnection(object): # Our fin was lost, rely on retransmission return False - if r.length or r.c_so: + if (r.length and not r.c_syn and not r.c_rst and not r.c_fin) or r.c_so: self._retrans = None s = RatpPacket(flags='RA') s.c_sn = r.c_an @@ -596,7 +596,7 @@ class RatpConnection(object): if r.c_so: self._r_sn = r.c_sn self._rx_buf.append(chr(r.length)) - elif r.length: + elif r.length and not r.c_syn and not r.c_rst and not r.c_fin: self._r_sn = r.c_sn self._rx_buf.append(r.payload) else: -- cgit v1.2.3 From b2fb42fb63a838383873f9de1a4037d8403a5cec Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:14 +0200 Subject: ratp: fix single byte sending flagged with SO When a single data byte is sent, it is flagged as SO and the actual data goes in the length byte within the header. Worth noting that this issue wasn't found in any barebox<->bbremote interaction because all data packets sent between them always have more than one byte (due to the barebox command specific header). Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index c946bea1a5..846f8130cf 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -384,6 +384,7 @@ static int ratp_send_next_data(struct ratp_internal *ri) uint16_t crc; uint8_t control; struct ratp_header *hdr; + uint8_t *data; int pktlen; struct ratp_message *msg; int len; @@ -409,19 +410,19 @@ static int ratp_send_next_data(struct ratp_internal *ri) RATP_CONTROL_ACK; hdr = msg->buf; + data = (uint8_t *)(hdr + 1); if (msg->eor) control |= RATP_CONTROL_EOR; + pktlen = sizeof(struct ratp_header); if (len > 1) { - void *data = hdr + 1; - pktlen = sizeof(*hdr) + len + 2; + pktlen += len + 2; crc = cyg_crc16(data, len); put_unaligned_be16(crc, data + len); - } else { - pktlen = sizeof(struct ratp_header); + } else if (len == 1) { control |= RATP_CONTROL_SO; - len = 0; + len = *data; } ratp_create_packet(ri, hdr, control, len); -- cgit v1.2.3 From e90817a514083345e80b248955e5d3d130561621 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:15 +0200 Subject: ratp: remove bogus data checks in behavior C2 The SN validation was being completely ignored if the packet had no data (e.g. for RST, FIN or SYN or plain ACKs). This condition is now removed so that the SN check is done. The second check removed was actually never being used, as it was already being tested for not having data in the first one. These two fixes are a cleanup to follow the protocol correctly. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 8 +------- scripts/remote/ratp.py | 16 +++++----------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index 846f8130cf..321721ab71 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -721,9 +721,6 @@ static int ratp_behaviour_c2(struct ratp_internal *ri, void *pkt) pr_debug("%s\n", __func__); - if (!ratp_has_data(hdr)) - return 0; - if (ratp_sn_expected(ri, hdr)) return 0; @@ -745,9 +742,6 @@ static int ratp_behaviour_c2(struct ratp_internal *ri, void *pkt) return 1; } - if (!ratp_has_data(hdr)) - return 1; - pr_debug("Sending ack for duplicate message\n"); ret = ratp_send_ack(ri, hdr); if (ret) @@ -1846,4 +1840,4 @@ eor: } return 0; -} \ No newline at end of file +} diff --git a/scripts/remote/ratp.py b/scripts/remote/ratp.py index a41d2e8a3f..0dfc8420c7 100644 --- a/scripts/remote/ratp.py +++ b/scripts/remote/ratp.py @@ -339,9 +339,6 @@ class RatpConnection(object): def _c2(self, r): logging.info("C2") - if r.length == 0 and r.c_so == 0: - return True - if r.c_sn != self._r_sn: return True @@ -358,14 +355,11 @@ class RatpConnection(object): self._state = RatpState.closed return False - # FIXME: only ack duplicate data packages? - # This is not documented in RFC 916 - if r.length or r.c_so: - logging.info("C2: duplicate data packet, dropping") - s = RatpPacket(flags='A') - s.c_sn = r.c_an - s.c_an = (r.c_sn + 1) % 2 - self._write(s) + logging.info("C2: duplicate packet") + s = RatpPacket(flags='A') + s.c_sn = r.c_an + s.c_an = (r.c_sn + 1) % 2 + self._write(s) return False -- cgit v1.2.3 From 45a5fdf884b533dd6fb11342b089306a0b066e34 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:16 +0200 Subject: ratp: remove FIXME comment: FIN always requires ACK Section 3.4 in the RFC916 shows a packet flow for the connection close where the initial packet sent by the endpoint starting the close has just the FIN flag set, without an ACK: --> <-- --> This may lead to think that it is actually allowed to send the initial packet with just FIN set, without ACK-ing any other packet from the peer. But, this is actually not possible, the packet MUST be ACK-ing a previous packet from the peer, even if this is just a duplicated ACK, because otherwise the packet with the FIN wouldn't get processed in the H2 behavior (FIN processing) of the peer, as the F2 behavior (ACK processing) would filter it out. This is actually the same reasoning why data packets always have ACK set, even if the same ACK has already been sent previously (e.g. with a simple ACK packet without data); if they didn't have it, they would be filtered out in the F2 behavior, never arriving the I1 behavior, which is where the received data is processed. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- 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 0dfc8420c7..e6b3e19b69 100644 --- a/scripts/remote/ratp.py +++ b/scripts/remote/ratp.py @@ -724,7 +724,7 @@ class RatpConnection(object): deadline = monotonic() + timeout logging.info("CLOSE") if self._state == RatpState.established: - fin = RatpPacket(flags='FA') # FIXME: only F? + fin = RatpPacket(flags='FA') fin.c_sn = (self._s_sn + 1) % 2 fin.c_an = (self._r_sn + 1) % 2 self._write(fin) -- cgit v1.2.3 From e03f80b26ce527d2fc262ca09bd16f55f3d3df20 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:17 +0200 Subject: ratp: fix sending ACKs without data All ACKs without data must be built in the same way from the input message: The code was actually doing this instead: This change fixes how the retransmissions are ACK-ed by the peer, and would have affected the barebox<->bbremote interactions. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index 321721ab71..5c52d3b5f6 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -364,13 +364,12 @@ static bool ratp_sn_expected(struct ratp_internal *ri, struct ratp_header *hdr) static int ratp_send_ack(struct ratp_internal *ri, struct ratp_header *hdr) { - uint8_t control = RATP_CONTROL_ACK; + uint8_t control; int ret; - if (hdr->control & RATP_CONTROL_SN) - control |= RATP_CONTROL_AN; - else - control |= 0; + control = ratp_set_sn(ratp_an(hdr)) | + ratp_set_an(ratp_sn(hdr) + 1) | + RATP_CONTROL_ACK; ret = ratp_send_hdr(ri, control); if (ret) -- cgit v1.2.3 From 9adcfb5c5318a4735511840883f6f4cd52e62699 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:18 +0200 Subject: ratp: consolidate ratp_sn_expected() and ratp_an_expected() This is no code change in ratp_sn_expected(), it's just rewritten in the same way as ratp_an_expected(), which follows the RFC916 approach. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ratp.c b/lib/ratp.c index 5c52d3b5f6..46a2b645c9 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -359,7 +359,7 @@ static bool ratp_an_expected(struct ratp_internal *ri, struct ratp_header *hdr) static bool ratp_sn_expected(struct ratp_internal *ri, struct ratp_header *hdr) { - return ratp_sn(hdr) != ri->sn_received; + return ratp_sn(hdr) == (ri->sn_received + 1) % 2; } static int ratp_send_ack(struct ratp_internal *ri, struct ratp_header *hdr) -- cgit v1.2.3 From 9e5063aeebeddfcd31164ec0b952c7f48b9361bb Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:19 +0200 Subject: ratp: prefer using ratp_send_ack() in behaviour I1 Instead of manually constructing an ACK, use the ratp_send_ack() method, which already does that properly. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index 46a2b645c9..c7f3f41713 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -1330,7 +1330,6 @@ static int msg_recv(struct ratp_internal *ri, void *pkt) static int ratp_behaviour_i1(struct ratp_internal *ri, void *pkt) { struct ratp_header *hdr = pkt; - uint8_t control = 0; if (!ratp_has_data (hdr)) return 1; @@ -1341,15 +1340,10 @@ static int ratp_behaviour_i1(struct ratp_internal *ri, void *pkt) msg_recv(ri, pkt); - if (list_empty(&ri->sendmsg) || ri->sendmsg_current) { - control = ratp_set_sn(!ri->sn_sent) | - ratp_set_an(ri->sn_received + 1) | - RATP_CONTROL_ACK; - - ratp_send_hdr(ri, control); - } else { + if (list_empty(&ri->sendmsg) || ri->sendmsg_current) + ratp_send_ack(ri, hdr); + else ratp_send_next_data(ri); - } return 0; } -- cgit v1.2.3 From 6b3a3e56faa63397b3c160e74767c620ebca91a7 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:20 +0200 Subject: ratp: send initial data in behaviour B if any pending And also, use ratp_send_ack() instead of manually constructing an ACK if no data is pending to be sent. The current barebox implementation doesn't allow any queueing of data until the connection is established, so this is probably not a case that would get run anyway. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index c7f3f41713..e810a9e541 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -622,11 +622,11 @@ static void ratp_behaviour_b(struct ratp_internal *ri, void *pkt) ri->sn_received = ratp_sn(hdr); if (hdr->control & RATP_CONTROL_ACK) { - control = ratp_set_sn(ratp_an(hdr)) | - ratp_set_an(!ratp_sn(hdr)) | - RATP_CONTROL_ACK; - ratp_send_hdr(ri, control); ratp_state_change(ri, RATP_STATE_ESTABLISHED); + if (list_empty(&ri->sendmsg) || ri->sendmsg_current) + ratp_send_ack(ri, hdr); + else + ratp_send_next_data(ri); } else { struct ratp_header synack = {}; -- cgit v1.2.3 From 5113a3dee9f6798dd015500dc33ee29277f37e49 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:21 +0200 Subject: ratp: don't ignore data that may arrive in behaviour H1 If an input packet arrives H1 that has data in it, we need to: * track sn_received * if we have data pending, send it * if we don't have data pending, send a plain ACK This process, as noted in RFC916, is the same as the I1 procedure, so go and run it: Go to the ESTABLISHED state and execute procedure I1 to process any data which might be in this packet. This fix allows the peer to queue data in the last packet doing the connection establishment. It doesn't apply to the barebox<->bbremote interaction because bbremote won't queue data until the connection is completely established, but it allows third party ratp implementations to do that. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 8 +++++++- scripts/remote/ratp.py | 14 ++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index e810a9e541..2dee41009e 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -1042,6 +1042,8 @@ static int ratp_behaviour_g(struct ratp_internal *ri, void *pkt) return 0; } +static int ratp_behaviour_i1(struct ratp_internal *ri, void *pkt); + /* * Our SYN has been acknowledged. At this point we are * technically in the ESTABLISHED state. Send any initial data @@ -1062,7 +1064,11 @@ static int ratp_behaviour_h1(struct ratp_internal *ri, void *pkt) ratp_state_change(ri, RATP_STATE_ESTABLISHED); - return 0; + /* If the input message has data (i.e. it is not just an ACK + * without data) then we need to send back an ACK ourselves, + * or even data if we have it pending. This is the same + * procedure done in i1, so just run it. */ + return ratp_behaviour_i1 (ri, pkt); } /* diff --git a/scripts/remote/ratp.py b/scripts/remote/ratp.py index e6b3e19b69..7972d31f2f 100644 --- a/scripts/remote/ratp.py +++ b/scripts/remote/ratp.py @@ -489,12 +489,8 @@ class RatpConnection(object): def _h1(self, r): logging.info("H1") - - # FIXME: initial data? self._state = RatpState.established - self._r_sn = r.c_sn - - return False + return self._common_i1(r) def _h2(self, r): logging.info("H2") @@ -584,9 +580,7 @@ class RatpConnection(object): self._time_wait_deadline = monotonic() + self._get_rto() return False - def _i1(self, r): - logging.info("I1") - + def _common_i1(self, r): if r.c_so: self._r_sn = r.c_sn self._rx_buf.append(chr(r.length)) @@ -608,6 +602,10 @@ class RatpConnection(object): self._write(s) return False + def _i1(self, r): + logging.info("I1") + return self._common_i1(r) + def _machine(self, pkt): logging.info("State: %r", self._state) if self._state == RatpState.listen: -- cgit v1.2.3 From 22da8b1c90297256b6003d178066d51bdada37a8 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:22 +0200 Subject: ratp: consolidate setting the next AN or SN flags Setting the next AN or SN flag was being done in two different ways throughout the code; e.g. here for AN: ratp_set_an(ratp_sn(hdr) + 1); or: ratp_set_an(!ratp_sn(hdr)); We define a pair of new ratp_set_next_sn() and ratp_set_next_an() macros that make it clear that the next value is desired, and also hide the computation of the actual flag value within the macro, so the previous example would now look like: ratp_set_next_an(ratp_sn(hdr)); Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index 2dee41009e..46a82c69ae 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -139,8 +139,10 @@ static bool ratp_an(struct ratp_header *hdr) return hdr->control & RATP_CONTROL_AN ? 1 : 0; } -#define ratp_set_sn(sn) (((sn) % 2) ? RATP_CONTROL_SN : 0) -#define ratp_set_an(an) (((an) % 2) ? RATP_CONTROL_AN : 0) +#define ratp_set_sn(sn) (sn ? RATP_CONTROL_SN : 0) +#define ratp_set_an(an) (an ? RATP_CONTROL_AN : 0) +#define ratp_set_next_sn(sn) (((sn + 1) % 2) ? RATP_CONTROL_SN : 0) +#define ratp_set_next_an(an) (((an + 1) % 2) ? RATP_CONTROL_AN : 0) static inline int ratp_header_ok(struct ratp_internal *ri, struct ratp_header *h) { @@ -368,7 +370,7 @@ static int ratp_send_ack(struct ratp_internal *ri, struct ratp_header *hdr) int ret; control = ratp_set_sn(ratp_an(hdr)) | - ratp_set_an(ratp_sn(hdr) + 1) | + ratp_set_next_an(ratp_sn(hdr)) | RATP_CONTROL_ACK; ret = ratp_send_hdr(ri, control); @@ -404,8 +406,8 @@ static int ratp_send_next_data(struct ratp_internal *ri) len = msg->len; - control = ratp_set_sn(ri->sn_sent + 1) | - ratp_set_an(ri->sn_received + 1) | + control = ratp_set_next_sn(ri->sn_sent) | + ratp_set_next_an(ri->sn_received) | RATP_CONTROL_ACK; hdr = msg->buf; @@ -630,7 +632,7 @@ static void ratp_behaviour_b(struct ratp_internal *ri, void *pkt) } else { struct ratp_header synack = {}; - control = ratp_set_an(!ratp_sn(hdr)) | + control = ratp_set_next_an(ratp_sn(hdr)) | RATP_CONTROL_SYN | RATP_CONTROL_ACK; @@ -734,7 +736,7 @@ static int ratp_behaviour_c2(struct ratp_internal *ri, void *pkt) pr_debug("Error: Connection reset\n"); control = RATP_CONTROL_RST | RATP_CONTROL_ACK | - ratp_set_sn(ratp_an(hdr)) | ratp_set_an(!ratp_sn(hdr)); + ratp_set_sn(ratp_an(hdr)) | ratp_set_next_an(ratp_sn(hdr)); ratp_send_hdr(ri, control); ratp_state_change(ri, RATP_STATE_CLOSED); @@ -1035,7 +1037,7 @@ static int ratp_behaviour_g(struct ratp_internal *ri, void *pkt) if (hdr->control & RATP_CONTROL_ACK) control |= ratp_set_sn(ratp_an(hdr)); else - control |= ratp_set_an(ratp_sn(hdr) + 1) | RATP_CONTROL_ACK; + control |= ratp_set_next_an(ratp_sn(hdr)) | RATP_CONTROL_ACK; ratp_send_hdr(ri, control); @@ -1099,7 +1101,7 @@ static int ratp_behaviour_h2(struct ratp_internal *ri, void *pkt) ri->status = -ENETDOWN; control = ratp_set_sn(ratp_an(hdr)) | - ratp_set_an(ratp_sn(hdr) + 1) | + ratp_set_next_an(ratp_sn(hdr)) | RATP_CONTROL_FIN | RATP_CONTROL_ACK; @@ -1165,7 +1167,7 @@ static int ratp_behaviour_h3(struct ratp_internal *ri, void *pkt) if (ratp_has_data(hdr)) { control = ratp_set_sn(ratp_an(hdr)) | - ratp_set_an(ratp_sn(hdr) + 1) | + ratp_set_next_an(ratp_sn(hdr)) | RATP_CONTROL_RST | RATP_CONTROL_ACK; ratp_send_hdr(ri, control); @@ -1176,7 +1178,7 @@ static int ratp_behaviour_h3(struct ratp_internal *ri, void *pkt) } control = ratp_set_sn(ratp_an(hdr)) | - ratp_set_an(ratp_sn(hdr) + 1) | + ratp_set_next_an(ratp_sn(hdr)) | RATP_CONTROL_ACK; expected = ratp_an_expected(ri, hdr); @@ -1278,7 +1280,7 @@ static int ratp_behaviour_h6(struct ratp_internal *ri, void *pkt) if (!(hdr->control & RATP_CONTROL_FIN)) return 1; - control = ratp_set_sn(ratp_an(hdr) + 1) | RATP_CONTROL_ACK; + control = ratp_set_next_sn(ratp_an(hdr)) | RATP_CONTROL_ACK; ratp_send_hdr(ri, control); @@ -1695,8 +1697,8 @@ void ratp_close(struct ratp *ratp) ratp_state_change(ri, RATP_STATE_FIN_WAIT); - control = ratp_set_sn(!ri->sn_sent) | - ratp_set_an(ri->sn_received + 1) | + control = ratp_set_next_sn(ri->sn_sent) | + ratp_set_next_an(ri->sn_received) | RATP_CONTROL_FIN | RATP_CONTROL_ACK; ratp_create_packet(ri, &fin, control, 0); -- cgit v1.2.3 From 5bde92c92b0efb675969d09386667f110f63cd6c Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 21 Jun 2017 21:13:23 +0200 Subject: ratp: user close may happen in SYN-RECEIVED state The reference says: 5.2.3. SYN-RECEIVED ... Departures - A CLOSE request is made by the user. Create a packet with FIN set. Send it and go to the FIN-WAIT state. Add this missing step. Probably not a real usecase for barebox anyway as there is no user triggered close. Signed-off-by: Aleksander Morgado Signed-off-by: Sascha Hauer --- lib/ratp.c | 2 +- scripts/remote/ratp.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ratp.c b/lib/ratp.c index 46a82c69ae..e7fbf640a7 100644 --- a/lib/ratp.c +++ b/lib/ratp.c @@ -1689,7 +1689,7 @@ void ratp_close(struct ratp *ratp) if (!ri) return; - if (ri->state == RATP_STATE_ESTABLISHED) { + if (ri->state == RATP_STATE_ESTABLISHED || ri->state == RATP_STATE_SYN_RECEIVED) { uint64_t start; u8 control; diff --git a/scripts/remote/ratp.py b/scripts/remote/ratp.py index 7972d31f2f..44f3e2f40a 100644 --- a/scripts/remote/ratp.py +++ b/scripts/remote/ratp.py @@ -721,7 +721,7 @@ class RatpConnection(object): def close(self, timeout=1.0): deadline = monotonic() + timeout logging.info("CLOSE") - if self._state == RatpState.established: + if self._state == RatpState.established or self._state == RatpState.syn_received: fin = RatpPacket(flags='FA') fin.c_sn = (self._s_sn + 1) % 2 fin.c_an = (self._r_sn + 1) % 2 -- cgit v1.2.3