From 484981de347695ae3e267d0dff15c2332075b56c Mon Sep 17 00:00:00 2001 From: XANTRONIX Development Date: Wed, 19 Aug 2020 22:32:05 -0400 Subject: [PATCH] Implement better frame ack strategy Implement AX.25 v2.2, Section 6.4.6 "Receiving Acknowledgement" semantics more consistently Changes: * Add frame_ack() method in src/server.c to unify logic for acknowledging frames and handling I frame transmission flow control * Ensure that the Timer T1 retry counter is reset as appropriate when Timer T1 is restarted * Add excerpts from the AX.25 v2.2 documentation to accompanying code to better document the purpose of important aspects of the state machine and protocol * When receiving an S frame with N(R) acknowledgement of a frame sent earlier than indicated by V(S), set V(S) to that N(R) value * Implement patty_ax25_sock_resend_pending() as a means to resend one frame previously sent which remains unacknowledged; this uses V(S) to determine the frame to resend, and increments V(S) if there was indeed an unacknowledged frame * Call patty_ax25_sock_resend_pending() in src/server.c, method handle_sock() to send a single frame previously sent but pending acknowledgement --- include/patty/ax25/sock.h | 2 + src/server.c | 220 ++++++++++++++++++++------------------ src/sock.c | 54 +++++++--- 3 files changed, 158 insertions(+), 118 deletions(-) diff --git a/include/patty/ax25/sock.h b/include/patty/ax25/sock.h index 6022781..6d1306f 100644 --- a/include/patty/ax25/sock.h +++ b/include/patty/ax25/sock.h @@ -234,6 +234,8 @@ ssize_t patty_ax25_sock_recv_pending(patty_ax25_sock *sock); ssize_t patty_ax25_sock_resend(patty_ax25_sock *sock, int seq); +ssize_t patty_ax25_sock_resend_pending(patty_ax25_sock *sock); + int patty_ax25_sock_ack(patty_ax25_sock *sock, int nr); int patty_ax25_sock_ack_pending(patty_ax25_sock *sock); diff --git a/src/server.c b/src/server.c index b8b847c..c797f3c 100644 --- a/src/server.c +++ b/src/server.c @@ -303,10 +303,11 @@ static int sock_shutdown(patty_ax25_server *server, return 0; } - sock->state = PATTY_AX25_SOCK_PENDING_DISCONNECT; - ret = patty_ax25_sock_send_disc(sock, PATTY_AX25_FRAME_POLL); + sock->state = PATTY_AX25_SOCK_PENDING_DISCONNECT; + sock->retries = sock->n_retry; + patty_timer_start(&sock->timer_t1); return ret; @@ -423,6 +424,18 @@ error_sock_delete_local: return -1; } +static inline void sock_flow_stop(patty_ax25_server *server, + patty_ax25_sock *sock) { + fd_clear(server, sock->fd); + + sock->flow = PATTY_AX25_SOCK_WAIT; +} + +static inline void sock_flow_start(patty_ax25_server *server, + patty_ax25_sock *sock) { + fd_watch(server, sock->fd); +} + int patty_ax25_server_add_if(patty_ax25_server *server, patty_ax25_if *iface) { int fd; @@ -901,6 +914,8 @@ static int server_connect(patty_ax25_server *server, * will help us determine what version of AX.25 to apply for this * socket, or whether the peer is not accepting connections. */ + sock->retries = sock->n_retry; + patty_timer_start(&sock->timer_t1); default: @@ -1222,6 +1237,53 @@ static int reply_test(patty_ax25_if *iface, return reply_to(iface, frame, &reply); } +static int frame_ack(patty_ax25_server *server, + patty_ax25_sock *sock, + patty_ax25_frame *frame) { + sock->rx_pending = 0; + + if (patty_ax25_sock_ack(sock, frame->nr) > 0) { + /* + * AX.25 v2.2, Section 6.4.6 "Receiving Acknowledgement" + * + * Whenever an I or S frame is correctly received, even in a busy + * condition, the N(R) of the received frame is checked to see if it + * includes an acknowledgement of outstanding sent I frames. Timer + * T1 is canceled if the received frame actually acknowledges + * previously unacknowledged frames. If Timer T1 is canceled and + * there are still some frames that have been sent that are not + * acknowledged, Timer T1 is started again. + */ + patty_timer_stop(&sock->timer_t1); + } + + if (PATTY_AX25_FRAME_CONTROL_S(frame->control) && frame->pf) { + /* + * AX.25 v2.2, Section 6.4.11 "Waiting Acknowledgement" + * + * If the TNC correctly receives a supervisory response frame with the + * F bit set and an N(R) within the range from the last N(R) received + * to the last N(S) sent plus one, the TNC restarts Timer T1 and sets + * its send state variable V(S) to the received N(R). It may then + * resume with I frame transmission or retransmission, as appropriate. + */ + int min = sock->va, + max = sock->vs; + + if (max < min) { + max += sock->mode == PATTY_AX25_SOCK_SABME? 128: 8; + } + + if (frame->nr >= min && frame->nr <= max) { + sock->vs = frame->nr; + + patty_timer_start(&sock->timer_t1); + } + } + + return 0; +} + static int handle_frmr(patty_ax25_server *server, patty_ax25_if *iface, patty_ax25_sock *sock, @@ -1233,7 +1295,8 @@ static int handle_frmr(patty_ax25_server *server, if (sock->state == PATTY_AX25_SOCK_PENDING_CONNECT) { int ret = patty_ax25_sock_send_sabm(sock, PATTY_AX25_FRAME_POLL); - patty_timer_start(&sock->timer_t1, sock->n_ack); + sock->retries = sock->n_retry; + patty_timer_start(&sock->timer_t1); return ret; @@ -1502,18 +1565,7 @@ static int handle_i(patty_ax25_server *server, return frame->pf? reply_dm(iface, frame, PATTY_AX25_FRAME_FINAL): 0; } - patty_ax25_sock_ack(sock, frame->nr); - - if (patty_ax25_sock_ack_pending(sock) == 0) { - patty_timer_stop(&sock->timer_t1); - } else { - sock->retries = sock->n_retry; - sock->flow = PATTY_AX25_SOCK_WAIT; - - fd_clear(server, sock->fd); - - patty_timer_start(&sock->timer_t1); - } + frame_ack(server, sock, frame); if (frame->ns == sock->vr) { patty_ax25_sock_vr_incr(sock); @@ -1548,9 +1600,7 @@ static int handle_i(patty_ax25_server *server, * AX.25 v2.2 Section 6.2 "Poll/Final (P/F) Bit Procedures" */ if (frame->pf == 1) { - return FD_ISSET(sock->fd, &server->fds_watch)? - patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_RESPONSE, 1): - patty_ax25_sock_send_rnr(sock, PATTY_AX25_FRAME_RESPONSE, 1); + return patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_RESPONSE, 1); } return 0; @@ -1600,22 +1650,6 @@ reply_dm: return reply_dm(iface, frame, PATTY_AX25_FRAME_FINAL); } -static int frame_ack(patty_ax25_server *server, - patty_ax25_sock *sock, - patty_ax25_frame *frame) { - if (sock == NULL) { - return 0; - } - - if (patty_ax25_sock_ack(sock, frame->nr - 1) > 0) { - patty_timer_stop(&sock->timer_t1); - } - - if (patty_ax25_sock_ack_pending(sock) > 0) { - patty_timer_start(&sock->timer_t1); - } -} - static int handle_rr(patty_ax25_server *server, patty_ax25_sock *sock, patty_ax25_frame *frame) { @@ -1623,39 +1657,16 @@ static int handle_rr(patty_ax25_server *server, return 0; } - if (patty_ax25_sock_ack(sock, frame->nr) > 0) { - - } - - if (patty_ax25_sock_ack_pending(sock) == 0) { - patty_timer_stop(&sock->timer_t1); - } + frame_ack(server, sock, frame); switch (frame->cr) { case PATTY_AX25_FRAME_COMMAND: - if (frame->pf == 0) { - break; + if (frame->pf) { + return patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_RESPONSE, 1); } - return FD_ISSET(sock->fd, &server->fds_watch)? - patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_RESPONSE, 1): - patty_ax25_sock_send_rnr(sock, PATTY_AX25_FRAME_RESPONSE, 1); - case PATTY_AX25_FRAME_RESPONSE: - if (patty_ax25_sock_ack_pending(sock) == 0) { - sock->retries = sock->n_retry; - - fd_watch(server, sock->fd); - - patty_timer_start(&sock->timer_t3); - } else { - sock->retries = sock->n_retry; - sock->flow = PATTY_AX25_SOCK_WAIT; - - fd_clear(server, sock->fd); - - patty_timer_start(&sock->timer_t1); - } + sock_flow_start(server, sock); default: break; @@ -1671,33 +1682,16 @@ static int handle_rnr(patty_ax25_server *server, return 0; } - patty_ax25_sock_ack(sock, frame->nr); - - if (patty_ax25_sock_ack_pending(sock) == 0) { - patty_timer_stop(&sock->timer_t1); - } + frame_ack(server, sock, frame); switch (frame->cr) { case PATTY_AX25_FRAME_COMMAND: - if (frame->pf == 0) { - break; + if (frame->pf) { + return patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_RESPONSE, 1); } - return sock->flow == PATTY_AX25_SOCK_READY? - patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_RESPONSE, 1): - patty_ax25_sock_send_rnr(sock, PATTY_AX25_FRAME_RESPONSE, 1); - case PATTY_AX25_FRAME_RESPONSE: - sock->flow = PATTY_AX25_SOCK_WAIT; - sock->retries = sock->n_retry; - - fd_clear(server, sock->fd); - - if (patty_ax25_sock_ack_pending(sock) == 0) { - patty_timer_start(&sock->timer_t3); - } else { - patty_timer_start(&sock->timer_t1); - } + sock_flow_stop(server, sock); default: break; @@ -1709,31 +1703,26 @@ static int handle_rnr(patty_ax25_server *server, static int handle_rej(patty_ax25_server *server, patty_ax25_sock *sock, patty_ax25_frame *frame) { - unsigned int i, - end; - if (sock == NULL) { return 0; } - patty_ax25_sock_ack(sock, frame->nr); + frame_ack(server, sock, frame); - end = sock->vs; + switch (frame->cr) { + case PATTY_AX25_FRAME_COMMAND: + if (frame->pf) { + return patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_RESPONSE, 1); + } - if (frame->nr > end) { - end += (sock->mode == PATTY_AX25_SOCK_SABME)? 128: 8; - } + case PATTY_AX25_FRAME_RESPONSE: + sock_flow_start(server, sock); - for (i=frame->nr; inr); + frame_ack(server, sock, frame); /* * TODO: Read the fine print of section 4.3.2.4 @@ -1817,6 +1806,8 @@ static int handle_xid(patty_ax25_server *server, patty_timer_start(&remote->timer_t1); + remote->retries = remote->n_retry; + return ret; } @@ -1864,6 +1855,8 @@ static int handle_xid(patty_ax25_server *server, patty_timer_start(&remote->timer_t1); + remote->retries = remote->n_retry; + return ret; } @@ -2151,12 +2144,14 @@ static int handle_sock(uint32_t key, if (patty_timer_expired(&sock->timer_t3)) { /* - * AX.25 v.2.2 Section 6.7.1.3, "Inactive Link Timer T3" + * AX.25 v.2.2 Section 6.7.1.3 "Inactive Link Timer T3" */ int ret = FD_ISSET(sock->fd, &server->fds_watch)? patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_COMMAND, 1): patty_ax25_sock_send_rnr(sock, PATTY_AX25_FRAME_COMMAND, 1); + sock->retries = sock->n_retry; + patty_timer_stop(&sock->timer_t3); patty_timer_start(&sock->timer_t1); @@ -2172,16 +2167,14 @@ static int handle_sock(uint32_t key, } /* - * AX.25 v2.2, Section 6.4.1, "Sending I Frames" + * AX.25 v2.2, Section 6.4.1 "Sending I Frames" */ if (sock->flow == PATTY_AX25_SOCK_WAIT) { sock->flow = PATTY_AX25_SOCK_READY; } else if (patty_ax25_sock_flow_full(sock)) { int ret; - sock->flow = PATTY_AX25_SOCK_WAIT; - - fd_clear(server, sock->fd); + sock_flow_stop(server, sock); ret = patty_ax25_sock_send_rr(sock, PATTY_AX25_FRAME_COMMAND, 1); @@ -2191,6 +2184,21 @@ static int handle_sock(uint32_t key, return ret; } + /* + * Check to see if any frames are pending resend. If so, return, allowing + * service to another socket rather than sending another frame to the + * current peer. + */ + if ((len = patty_ax25_sock_resend_pending(sock)) < 0) { + goto error_sock_resend_pending; + } else if (len > 0) { + sock->retries = sock->n_retry; + + patty_timer_start(&sock->timer_t1); + + return 0; + } + if ((len = read(sock->fd, sock->rx_buf, sock->n_maxlen_rx)) < 0) { if (errno == EIO) { (void)sock_shutdown(server, sock); @@ -2204,9 +2212,8 @@ static int handle_sock(uint32_t key, goto error_sock_write; } - /* - * AX.25 v2.2, Section 6.4.1, "Sending I Frames" - */ + sock->retries = sock->n_retry; + patty_timer_start(&sock->timer_t1); } @@ -2214,6 +2221,7 @@ static int handle_sock(uint32_t key, error_client_by_sock: error_sock_write: +error_sock_resend_pending: error_unknown: return -1; } diff --git a/src/sock.c b/src/sock.c index b46a0cf..830b7db 100644 --- a/src/sock.c +++ b/src/sock.c @@ -61,17 +61,17 @@ static inline size_t tx_slot_size(patty_ax25_sock *sock) { } static inline size_t tx_slots_size(patty_ax25_sock *sock) { - return sock->n_window_tx * tx_slot_size(sock); + size_t slots = sock->mode == PATTY_AX25_SOCK_SABME? 128: 8; + + return slots * tx_slot_size(sock); } static inline int tx_seq(patty_ax25_sock *sock, int seq) { - return sock->mode == PATTY_AX25_SOCK_SABME? - (seq + 128) % 128: - (seq + 8) % 8; + return sock->mode == PATTY_AX25_SOCK_SABME? seq % 128: seq % 8; } static inline struct slot *tx_slot(patty_ax25_sock *sock, int seq) { - int i = tx_seq(sock, seq) % sock->n_window_tx; + int i = tx_seq(sock, seq); return (struct slot *) ((uint8_t *)sock->tx_slots + (i * tx_slot_size(sock))); @@ -82,7 +82,8 @@ static inline void *tx_slot_buf(patty_ax25_sock *sock, int seq) { } static int init_bufs(patty_ax25_sock *sock) { - size_t i; + size_t i, + slots = sock->mode == PATTY_AX25_SOCK_SABME? 128: 8; if ((sock->tx_buf = realloc(sock->tx_buf, tx_bufsz(sock))) == NULL) { goto error_realloc_tx_buf; @@ -96,7 +97,7 @@ static int init_bufs(patty_ax25_sock *sock) { goto error_realloc_tx_slots; } - for (i=0; in_window_tx; i++) { + for (i=0; ilen = 0; @@ -517,6 +518,14 @@ void *patty_ax25_sock_assembler_read(patty_ax25_sock *sock, } int patty_ax25_sock_flow_full(patty_ax25_sock *sock) { + /* + * 6.4.1 Sending I Frames + * + * The TNC does not transmit any more I frames if its send state variable + * V(S) equals the last received N(R) from the other side of the link plus + * k. If the TNC sent more I frames, the flow control window would be + * exceeded and errors could result. + */ return sock->vs == tx_seq(sock, sock->va + sock->n_window_tx)? 1: 0; } @@ -684,9 +693,29 @@ error_noif: ssize_t patty_ax25_sock_resend(patty_ax25_sock *sock, int seq) { struct slot *slot = tx_slot(sock, seq); - void *buf = tx_slot_buf(sock, seq); - return slot->len > 0? patty_ax25_if_send(sock->iface, buf, slot->len): 0; + return slot->len > 0? patty_ax25_if_send(sock->iface, slot + 1, slot->len): 0; +} + +ssize_t patty_ax25_sock_resend_pending(patty_ax25_sock *sock) { + struct slot *slot = tx_slot(sock, sock->vs); + + if (slot->len > 0 && !slot->ack) { + ssize_t len; + + if ((len = patty_ax25_sock_resend(sock, sock->vs)) < 0) { + goto error_resend; + } + + sock->vs = tx_seq(sock, sock->vs + 1); + + return len; + } + + return 0; + +error_resend: + return -1; } int patty_ax25_sock_ack(patty_ax25_sock *sock, int nr) { @@ -705,7 +734,7 @@ int patty_ax25_sock_ack(patty_ax25_sock *sock, int nr) { if (slot->len > 0 && !slot->ack) { slot->ack = 1; - sock->va = tx_seq(sock, i); + sock->va = tx_seq(sock, i + 1); ret++; } @@ -715,10 +744,11 @@ int patty_ax25_sock_ack(patty_ax25_sock *sock, int nr) { } int patty_ax25_sock_ack_pending(patty_ax25_sock *sock) { - int ret = 0, + int ret = 0, + slots = sock->mode == PATTY_AX25_SOCK_SABME? 128: 8, i; - for (i=0; in_window_tx; i++) { + for (i=0; ilen > 0 && !slot->ack) {