From 9cd305950f9c109d83b3d8659601fd356816fb72 Mon Sep 17 00:00:00 2001 From: wb2osz Date: Mon, 1 Jan 2018 16:38:43 -0500 Subject: [PATCH] More efficient and reliable connected mode lost frame recovery. --- ax25_link.c | 671 +++++++++++++++++++++++++++++++++++++++------------- ax25_pad2.c | 60 ++++- ax25_pad2.h | 6 +- xid.c | 207 ++++++++++++---- xid.h | 9 +- 5 files changed, 730 insertions(+), 223 deletions(-) diff --git a/ax25_link.c b/ax25_link.c index f088c8f..4d352b4 100644 --- a/ax25_link.c +++ b/ax25_link.c @@ -33,8 +33,8 @@ * get refused, and fall back to trying version 2.0. * * - * State Client App State Mach. Peer - * ----- ---------- ----------- ---- + * State Client App State Machine Peer + * ----- ---------- ------------- ---- * * 0 disc * Conn. Req ---> @@ -51,8 +51,8 @@ * Typical sequence when other end initiates connection. * * - * State Client App State Mach. Peer - * ----- ---------- ----------- ---- + * State Client App State Machine Peer + * ----- ---------- ------------- ---- * * 0 disc * <--- SABME or SABM @@ -63,20 +63,33 @@ * * *note: * - * By reading the v2.2 spec, I expected a 2.0 implementation to send - * FRMR in response to SABME. In testing, I found that the KPC-3+ sent DM. + * After carefully studying the v2.2 spec, I expected a 2.0 implementation to send + * FRMR in response to SABME. This is important. If a v2.2 implementation + * gets FRMR, in response to SABME, it switches to v2.0 and sends SABM instead. * - * After consulting the 2.0 spec, it says, that when in disconnected - * mode, it will respond to any command other than SABM or UI frame with a DM - * response with P/F set to 1. I can see where they might get that idea. + * The v2.0 protocol spec, section 2.3.4.3.3.1, states that FRMR should be sent when + * an invalid or not implemented command is received. That all fits together. * - * I think the rule about sending FRMR for any unrecognized command should take precedence. + * In testing, I found that the KPC-3+ sent DM. + * + * I can see where they might get that idea. + * The v2.0 spec says that when in disconnected mode, it should respond to any + * command other than SABM or UI frame with a DM response with P/F set to 1. + * I think it was implemented wrong. 2.3.4.3.3.1 should take precedence. + * + * The TM-D710 does absolutely nothing in response to SABME. + * Not responding at all is just plain wrong. To work around this, I put + * in a special hack to start sending SABM after a certain number of + * SABME go unanswered. There is more discussion in the User Guide. * - * * References: * * AX.25 Amateur Packet-Radio Link-Layer Protocol Version 2.0, October 1984 * - * https://www.tapr.org/pub_ax25.html + * https://www.tapr.org/pub_ax25.html + * http://lea.hamradio.si/~s53mv/nbp/nbp/AX25V20.pdf + * + * At first glance, they look pretty much the same, but the second one + * is more complete with 4 appendices, including a state table. * * * AX.25 Link Access Protocol for Amateur Packet Radio Version 2.2 Revision: July 1998 * @@ -92,17 +105,26 @@ * "This is a new version of the 1998 standard. It has had all figures * redone using Microsoft Visio. Errors in the SDL have been corrected." * - * The SDL diagrams are dated 2006. I wish I knew about this version earlier - * before doing most of the implementation. :-( + * The SDL diagrams are dated 2006. I wish I had known about this version, with + * several corrections, before doing most of the implementation. :-( * + * The title page still says July 1998 so it's not immediately obvious this + * is different than the one on the TAPR site. + * + * * AX.25 ... Latest revision, in progress. + * + * http://www.nj7p.org/ + * + * This is currently being revised in cooperation with software authors + * who have noticed some issues during implementation. * * The functions here are based on the SDL diagrams but turned inside out. * It seems more intuitive to have a function for each type of input and then decide * what to do depending on the state. This also reduces duplicate code because we * often see the same flow chart segments, for the same input, appearing in multiple states. * - * Erratum: The protocol spec has many places that appear to be errors or are ambiguous so I wasn't - * sure what to do. These should be annotated with Errata comments so we can easily go + * Errata: The protocol spec has many places that appear to be errors or are ambiguous so I wasn't + * sure what to do. These should be annotated with "erratum" comments so we can easily go * back and revisit them. * * X.25: The AX.25 protocol is based on, but does not necessarily adhere to, the X.25 protocol. @@ -110,11 +132,12 @@ * * http://www.itu.int/rec/T-REC-X.25-199610-I/en/ * - * Current Status: This is still under development. + * Version 1.4, released April 2017: * - * Features partially tested: + * Features tested reasonably well: * * Connect to/from a KPC-3+ and send I frames in both directions. + * Same with TM-D710A. * v2.2 connect between two instances of direwolf. (Can't find another v2.2 for testing.) * Modulo 8 & 128 sequence numbers. * Recovery from simulated transmission errors using either REJ or SREJ. @@ -128,6 +151,12 @@ * T3 timer. * Compatibility with additional types of TNC. * + * Version 1.5, December 2017: + * + * Implemented Multi Selective Reject. + * More efficient generation of SREJ frames. + * Reduced number of duplicate I frames sent for both REJ and SREJ cases. + * *------------------------------------------------------------------*/ #include "direwolf.h" @@ -157,6 +186,7 @@ #define MAX(a,b) ((a)>(b)?(a):(b)) // Debug switches for different types of information. +// Should have command line options instead of changing source and recompiling. static int s_debug_protocol_errors = 1; // Less serious Protocol errors. // Useful for debugging but unnecessarily alarming other times. @@ -166,7 +196,6 @@ static int s_debug_client_app = 0; // Interaction with client application. static int s_debug_radio = 0; // Received frames and channel busy status. // lm_data_indication, lm_channel_busy - static int s_debug_variables = 0; // Variables, state changes. static int s_debug_retry = 0; // Related to lost I frames, REJ, SREJ, timeout, resending. @@ -239,10 +268,10 @@ typedef struct ax25_dlsm_s { // Determines whether we have one or two control // octets. 128 allows a much larger window size. - int srej_enabled; // Is other end capable of processing SREJ? (Am I allowed to send it?) - // Starts out as false for v2.0 or true for v2.2. - // Can be changed when exchanging capabilities. - // Can be used only with modulo 128. + enum srej_e srej_enable; // Is other end capable of processing SREJ? (Am I allowed to send it?) + // Starts out as 'srej_none' for v2.0 or 'srej_single' for v2.2. + // Can be changed to 'srej_multi' with XID exchange. + // Should be used only with modulo 128. (Is this enforced?) int n1_paclen; // Maximum length of information field, in bytes. // Starts out as 256 but can be negotiated higher. @@ -288,11 +317,19 @@ typedef struct ax25_dlsm_s { int reject_exception; // A REJ frame has been sent to the remote station. (boolean) + // This is used only when receving an I frame, in states 3 & 4, SREJ not enabled. + // When an I frame has an unepected N(S), + // - if not already set, set it and send REJ. + // When an I frame with expected N(S) is received, clear it. + // This would prevent us from sending additional REJ while + // waiting for result from first one. + // What happens if the REJ gets lost? Is it resent somehow? + int own_receiver_busy; // Layer 3 is busy and can't receive I frames. // We have no API to convey this information so it should always be 0. int acknowledge_pending; // I frames have been successfully received but not yet - // acknowledged to the remote station. + // acknowledged TO the remote station. // Set when receiving the next expected I frame and P=0. // This gets cleared by sending any I, RR, RNR, REJ. // Cleared when sending SREJ with F=1. @@ -469,6 +506,7 @@ static reg_callsign_t *reg_callsign_list = NULL; text_color_set(DW_COLOR_DEBUG); \ dw_printf ("V(S) = %d at %s %d\n", S->vs, __func__, __LINE__); \ } \ + assert (S->vs >= 0 && S->vs < S->modulo); \ } // If other guy acks reception of an I frame, we should never get an REJ or SREJ @@ -480,6 +518,7 @@ static reg_callsign_t *reg_callsign_list = NULL; text_color_set(DW_COLOR_DEBUG); \ dw_printf ("V(A) = %d at %s %d\n", S->va, __func__, __LINE__); \ } \ + assert (S->va >= 0 && S->va < S->modulo); \ int x = AX25MODULO(n-1, S->modulo, __FILE__, __func__, __LINE__); \ while (S->txdata_by_ns[x] != NULL) { \ cdata_delete (S->txdata_by_ns[x]); \ @@ -493,12 +532,25 @@ static reg_callsign_t *reg_callsign_list = NULL; text_color_set(DW_COLOR_DEBUG); \ dw_printf ("V(R) = %d at %s %d\n", S->vr, __func__, __LINE__); \ } \ + assert (S->vr >= 0 && S->vr < S->modulo); \ + } + +#define SET_RC(n) { S->rc = (n); \ + if (s_debug_variables) { \ + text_color_set(DW_COLOR_DEBUG); \ + dw_printf ("rc = %d at %s %d, state = %d\n", S->rc, __func__, __LINE__, S->state); \ + } \ } //TODO: Make this a macro so we can simplify calls yet keep debug output if something goes wrong. +#if 0 +#define AX25MODULO(n) ax25modulo((n), S->modulo, __FILE__, __func__, __LINE__) +static int ax25modulo(int n, int m, const char *file, const char *func, int line) +#else static int AX25MODULO(int n, int m, const char *file, const char *func, int line) +#endif { if (m != 8 && m != 128) { text_color_set(DW_COLOR_ERROR); @@ -537,7 +589,7 @@ static int is_ns_in_window (ax25_dlsm_t *S, int ns); static void send_srej_frames (ax25_dlsm_t *S, int *resend, int count, int allow_f1); static void rr_rnr_frame (ax25_dlsm_t *S, int ready, cmdres_t cr, int pf, int nr); static void rej_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, int nr); -static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, int nr); +static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, int nr, unsigned char *info_ptr, int info_len); static void sabm_e_frame (ax25_dlsm_t *S, int extended, int p); static void disc_frame (ax25_dlsm_t *S, int f); @@ -1003,7 +1055,7 @@ void dl_disconnect_request (dlq_item_t *E) case state_1_awaiting_connection: case state_5_awaiting_v22_connection: -// TODO: FIXME requeue. Not sure what to do here. +// TODO: "requeue." Not sure what to do here. // If we put it back in the queue we will get it back again probably still in same state. // Need a way to defer it until the next state change. @@ -1044,7 +1096,7 @@ void dl_disconnect_request (dlq_item_t *E) case state_4_timer_recovery: discard_i_queue (S); - S->rc = 0; // I think this should be 1 but I'm not that worried about it. + SET_RC(0); // I think this should be 1 but I'm not that worried about it. cmdres_t cmd = cr_cmd; int p = 1; @@ -1332,7 +1384,28 @@ static void data_request_good_size (ax25_dlsm_t *S, cdata_t *txdata) break; } - i_frame_pop_off_queue (S); + // v1.5 change in strategy. + // New I frames, not sent yet, are delayed until after processing anything in the received transmission. + // Give the transmit process a kick unless other side is busy or we have reached our window size. + // Previously we had i_frame_pop_off_queue here which would start sending new stuff before we + // finished dealing with stuff already in progress. + + switch (S->state) { + + case state_3_connected: + case state_4_timer_recovery: + + if ( ( ! S->peer_receiver_busy ) && + S->vs != AX25MODULO(S->va + S->k_maxframe, S->modulo, __FILE__, __func__, __LINE__) ) { + + S->acknowledge_pending = 1; + lm_seize_request (S->chan); + } + break; + + default: + break; + } } /* end data_request_good_size */ @@ -1808,6 +1881,15 @@ void lm_channel_busy (dlq_item_t *E) * Description: C4.2. This primitive indicates to the Data-link State Machine that * the transmission opportunity has arrived. * + * Version 1.5: Originally this only invoked inquiry_response to provide an ack if not already + * taken care of by an earlier frame in this transmission. + * After noticing the unnecessary I frame duplication and differing N(R) in the same + * transmission, I came to the conclusion that we should delay sending of new + * (not resends as a result of rej or srej) frames until after after processing + * of everything in the incoming transmission. + * The protocol spec simply has "I frame pops off queue" without any indication about + * what might trigger this event. + * *------------------------------------------------------------------------------*/ static void lm_seize_confirm (ax25_dlsm_t *S) @@ -1825,12 +1907,19 @@ static void lm_seize_confirm (ax25_dlsm_t *S) case state_3_connected: case state_4_timer_recovery: + // v1.5 change in strategy. + // New I frames, not sent yet, are delayed until after processing anything in the received transmission. + // Previously we started sending new frames, from the client app, as soon as they arrived. + // Now, we first take care of those in progress before throwing more into the mix. + + i_frame_pop_off_queue(S); + if (S->acknowledge_pending) { S->acknowledge_pending = 0; enquiry_response (S, frame_not_AX25, 0); } -// Implemenation difference: The flow chart for state 3 has LM-RELEASE Request here. +// Implementation difference: The flow chart for state 3 has LM-RELEASE Request here. // I don't think I need it because the transmitter will turn off // automatically once the queue is empty. @@ -2047,8 +2136,14 @@ void lm_data_indication (dlq_item_t *E) rej_frame (S, cr, pf, nr); break; - case frame_type_S_SREJ: // Selective Reject - Ask for single frame repeat - srej_frame (S, cr, pf, nr); + case frame_type_S_SREJ: // Selective Reject - Ask for selective frame(s) repeat + { + unsigned char *info_ptr; + int info_len; + + info_len = ax25_get_info (E->pp, &info_ptr); + srej_frame (S, cr, pf, nr, info_ptr, info_len); + } break; case frame_type_U_SABME: // Set Async Balanced Mode, Extended @@ -2108,8 +2203,6 @@ void lm_data_indication (dlq_item_t *E) break; } - i_frame_pop_off_queue (S); - } /* end lm_data_indication */ @@ -2241,7 +2334,7 @@ static void i_frame (ax25_dlsm_t *S, cmdres_t cr, int p, int nr, int ns, int pid // Erratum: Discrepancy between original and 2006 version. // Pattern noticed: Anytime we have "is_good_nr" which tests for V(A) <= N(R) <= V(S), - // we should always call "check_i_frame_acked" or at least set V(A) from N(R). + // we should always call "check_i_frame_ackd" or at least set V(A) from N(R). #if 0 // Erratum: original - states 3 & 4 differ here. @@ -2260,6 +2353,22 @@ static void i_frame (ax25_dlsm_t *S, cmdres_t cr, int p, int nr, int ns, int pid check_i_frame_ackd (S,nr); #endif +// Erratum: v1.5 - My addition. +// I noticed that we sometimes got stuck in state 4 and rc crept up slowly even though +// we received 'I' frames with N(R) values indicating that the other side received everything +// that we sent. Eventually rc could reach the limit and we would get an error. +// If we are in state 4, and other guy ack'ed last I frame we sent, transition to state 3. +// We had a similar situation for RR/RNR for cases other than response, F=1. + + if (S->state == state_4_timer_recovery && S->va == S->vs) { + + STOP_T1; + select_t1_value (S); + START_T3; + SET_RC(0); + enter_new_state (S, state_3_connected, __func__, __LINE__); + } + if (S->own_receiver_busy) { // This should be unreachable because we currently don't have a way to set own_receiver_busy. // But we might the capability someday so implement this while we are here. @@ -2271,7 +2380,7 @@ static void i_frame (ax25_dlsm_t *S, cmdres_t cr, int p, int nr, int ns, int pid int nr = S->vr; packet_t pp; - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RNR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RNR, S->modulo, nr, f, NULL, 0); // I wonder if this difference is intentional or if only one place was // was modified after a cut-n-paste of the flow chart segment. @@ -2291,13 +2400,17 @@ static void i_frame (ax25_dlsm_t *S, cmdres_t cr, int p, int nr, int ns, int pid #endif S->acknowledge_pending = 0; } + } - else { // N(R) out of expected range. + else { // Own receiver not busy. i_frame_continued (S, p, ns, pid, info_ptr, info_len); } + + } - else { + else { // N(R) not in expected range. + nr_error_recovery (S); // my enhancement. See below. enter_new_state (S, S->modulo == 128 ? state_5_awaiting_v22_connection : state_1_awaiting_connection, __func__, __LINE__); @@ -2357,6 +2470,8 @@ static void i_frame (ax25_dlsm_t *S, cmdres_t cr, int p, int nr, int ns, int pid * * 4.3.2.4. Selective Reject (SREJ) Command and Response * + * (Erratum: SREJ is only response with F bit.) + * * The selective reject, SREJ, frame is used by the receiving TNC to request retransmission of the single I frame * numbered N(R). If the P/F bit in the SREJ frame is set to "1", then I frames numbered up to N(R)-1 inclusive are * considered as acknowledged. However, if the P/F bit in the SREJ frame is set to "0", then the N(R) of the SREJ @@ -2404,6 +2519,9 @@ static void i_frame (ax25_dlsm_t *S, cmdres_t cr, int p, int nr, int ns, int pid * * 6.4.4.3. Selective Reject-Reject (SREJ/REJ) * + * (Erratum: REJ/SREJ should not be mixed. Basic (mod 8) allows only REJ. + * Extended (mod 128) gives you a choice of one or the other for a link.) + * * When an I frame is received with a correct FCS but its send sequence number N(S) does not match the current * receiver's receive state variable, and if N(S) indicates 2 or more frames are missing, a REJ frame is transmitted. * All subsequently received frames are discarded until the lost frame is correctly received. If only one frame is @@ -2482,7 +2600,7 @@ static void i_frame_continued (ax25_dlsm_t *S, int p, int ns, int pid, char *inf cmdres_t cr = cr_res; // response with F set to 1. packet_t pp; - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); S->acknowledge_pending = 0; } @@ -2513,12 +2631,12 @@ static void i_frame_continued (ax25_dlsm_t *S, int p, int ns, int pid, char *inf cmdres_t cr = cr_res; // response with F set to 1. packet_t pp; - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); S->acknowledge_pending = 0; } } - else if ( ! S->srej_enabled) { + else if (S->srej_enable == srej_none) { // The received sequence number is not the expected one and we can't use SREJ. // The old v2.0 approach is to send and REJ with the number we are expecting. @@ -2540,7 +2658,7 @@ static void i_frame_continued (ax25_dlsm_t *S, int p, int ns, int pid, char *inf dw_printf ("sending REJ, at %s %d, SREJ not enabled case, V(R)=%d", __func__, __LINE__, S->vr); } - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_REJ, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_REJ, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); S->acknowledge_pending = 0; @@ -2590,31 +2708,60 @@ static void i_frame_continued (ax25_dlsm_t *S, int p, int ns, int pid, char *inf int nr = S->vr; packet_t pp; - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RNR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RNR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); } else if (S->rxdata_by_ns[ AX25MODULO(ns - 1, S->modulo, __FILE__, __func__, __LINE__)] == NULL) { // Ask for missing frames when we don't have N(S)-1 in the receive buffer. -// I would think that we would want to set F when sending N(R) = V(R) but it says use F=0 here. -// Don't understand why yet. +// In version 1.4: +// We end up sending more SREJ than necessary and and get back redundant information. Example: +// When we see 113 missing, we ask for a resend. +// When we see 115 & 116 missing, a cummulative SREJ asks for everything. +// The other end dutifully sends 113 twice. +// +// [0.4] DW1>DW0:(SREJ res, n(r)=113, f=0) +// [0.4] DW1>DW0:(SREJ res, n(r)=113, f=1)<0xe6><0xe8> +// +// [0L] DW0>DW1:(I cmd, n(s)=113, n(r)=11, p=0, pid=0xf0)0114 send data<0x0d> +// [0L] DW0>DW1:(I cmd, n(s)=113, n(r)=11, p=0, pid=0xf0)0114 send data<0x0d> +// [0L] DW0>DW1:(I cmd, n(s)=115, n(r)=11, p=0, pid=0xf0)0116 send data<0x0d> +// [0L] DW0>DW1:(I cmd, n(s)=116, n(r)=11, p=0, pid=0xf0)0117 send data<0x0d> -// I like my method better. It does not generate duplicate requests for gaps in the same transmission. -// This creates a cummulative list each time and would cause repeats to be sent more often than necessary. - int resend[128]; - int count = 0; +// Version 1.5: +// Don't generate duplicate requests for gaps in the same transmission. + +// Ideally, we might wait until carrier drops and then use one Multi-SREJ for entire transmission but +// we will keep that for another day. +// Probably need a flag similar to acknowledge_pending (or ask_resend_count, here) and the ask_for_resend array. +// It could then be processed first in lm_seize_confirm. + + int ask_for_resend[128]; + int ask_resend_count = 0; int x; - int allow_f1 = 0; // F=1 from X.25 2.4.6.4 b) 3) - for (x = S->vr; x != ns; x = AX25MODULO(x + 1, S->modulo, __FILE__, __func__, __LINE__)) { - if (S->rxdata_by_ns[x] == NULL) { - resend[count++] = AX25MODULO(x, S->modulo, __FILE__, __func__, __LINE__); - } +// Version 1.5 +// Erratum: AX.25 says use F=0 here. Doesn't make sense. +// We would want to set F when sending N(R) = V(R). +// int allow_f1 = 0; // F=1 from X.25 2.4.6.4 b) 3) + int allow_f1 = 1; // F=1 from X.25 2.4.6.4 b) 3) + +// send only for this gap, not cummulative from V(R). + + int last = AX25MODULO(ns - 1, S->modulo, __FILE__, __func__, __LINE__); + int first = last; + while (first != S->vr && S->rxdata_by_ns[AX25MODULO(first - 1, S->modulo, __FILE__, __func__, __LINE__)] == NULL) { + first = AX25MODULO(first - 1, S->modulo, __FILE__, __func__, __LINE__); } - - send_srej_frames (S, resend, count, allow_f1); + x = first; + do { + ask_for_resend[ask_resend_count++] = AX25MODULO(x, S->modulo, __FILE__, __func__, __LINE__); + x = AX25MODULO(x + 1, S->modulo, __FILE__, __func__, __LINE__); + } while (x != AX25MODULO(last + 1, S->modulo, __FILE__, __func__, __LINE__)); + + send_srej_frames (S, ask_for_resend, ask_resend_count, allow_f1); } } else { @@ -2666,8 +2813,8 @@ static void i_frame_continued (ax25_dlsm_t *S, int p, int ns, int pid, char *inf // In this case, we need to ask for a resend of 1 & 2. // More generally, the range of V(R) thru N(S)-1. - int resend[128]; - int count = 0; + int ask_for_resend[128]; + int ask_resend_count = 0; int i; int allow_f1 = 1; @@ -2675,10 +2822,10 @@ text_color_set(DW_COLOR_ERROR); dw_printf ("%s:%d, zero exceptions, V(R)=%d, N(S)=%d\n", __func__, __LINE__, S->vr, ns); for (i = S->vr; i != ns; i = AX25MODULO(i+1, S->modulo, __FILE__, __func__, __LINE__)) { - resend[count++] = i; + ask_for_resend[ask_resend_count++] = i; } - send_srej_frames (S, resend, count, allow_f1); + send_srej_frames (S, ask_for_resend, ask_resend_count, allow_f1); } else { @@ -2693,7 +2840,7 @@ dw_printf ("%s:%d, zero exceptions, V(R)=%d, N(S)=%d\n", __func__, __LINE__, S-> // This can be achieved by searching S->rxdata_by_ns, starting with N(S)-1, and counting // how many empty slots we have before finding a saved frame. - int count = 0; + int ask_resend_count = 0; int first; text_color_set(DW_COLOR_ERROR); @@ -2705,7 +2852,7 @@ dw_printf ("%s:%d, %d srej exceptions, V(R)=%d, N(S)=%d\n", __func__, __LINE__, // Oops! Went too far. This I frame was already processed. text_color_set(DW_COLOR_ERROR); dw_printf ("INTERNAL ERROR calulating what to put in SREJ, %s line %d\n", __func__, __LINE__); - dw_printf ("V(R)=%d, N(S)=%d, SREJ exception=%d, first=%d, count=%d\n", S->vr, ns, selective_reject_exception(S), first, count); + dw_printf ("V(R)=%d, N(S)=%d, SREJ exception=%d, first=%d, ask_resend_count=%d\n", S->vr, ns, selective_reject_exception(S), first, ask_resend_count); int k; for (k=0; k<128; k++) { if (S->rxdata_by_ns[k] != NULL) { @@ -2714,31 +2861,29 @@ dw_printf ("%s:%d, %d srej exceptions, V(R)=%d, N(S)=%d\n", __func__, __LINE__, } break; } - count++; + ask_resend_count++; first = AX25MODULO(first - 1, S->modulo, __FILE__, __func__, __LINE__); } // Go beyond the slot where we already have an I frame. first = AX25MODULO(first + 1, S->modulo, __FILE__, __func__, __LINE__); - // The count could be 0. e.g. We got 4 rather than 7 in this example. + // The ask_resend_count could be 0. e.g. We got 4 rather than 7 in this example. - if (count > 0) { - int resend[128]; + if (ask_resend_count > 0) { + int ask_for_resend[128]; int n; int allow_f1 = 1; - for (n = 0; n < count; n++) { - resend[n] = AX25MODULO(first + n, S->modulo, __FILE__, __func__, __LINE__);; + for (n = 0; n < ask_resend_count; n++) { + ask_for_resend[n] = AX25MODULO(first + n, S->modulo, __FILE__, __func__, __LINE__);; } - send_srej_frames (S, resend, count, allow_f1); + send_srej_frames (S, ask_for_resend, ask_resend_count, allow_f1); } } /* end SREJ exception */ - - #endif // my earlier attempt. @@ -2753,7 +2898,7 @@ dw_printf ("%s:%d, %d srej exceptions, V(R)=%d, N(S)=%d\n", __func__, __LINE__, S->acknowledge_pending = 0; #endif - } /* end srej_enabled */ + } /* end srej enabled */ } /* end i_frame_continued */ @@ -2821,16 +2966,22 @@ static int is_ns_in_window (ax25_dlsm_t *S, int ns) * Purpose: Ask for a resend of I frames with specified sequence numbers. * * Inputs: resend - Array of N(S) values for missing I frames. + * * count - Number of items in array. + * * allow_f1 - When true, set F=1 when asking for V(R). + * * X.25 section 2.4.6.4 b) 3) says F should be set to 0 * when receiving I frame out of sequence. + * * X.25 sections 2.4.6.11 & 2.3.5.2.2 say set F to 1 when * responding to command with P=1. (our enquiry_response function). * - * Future? The X.25 protocol spec allows additional sequence numbers in one frame + * Version 1.5: The X.25 protocol spec allows additional sequence numbers in one frame * by using the INFO part. - * It would be easy but we haven't done that here to remain compatible with AX.25. + * By default that feature is off but can be negotiated with XID. + * We should be able to use this between two direwolf stations while + * maintaining compatibility with the original AX.25 v2.2. * *------------------------------------------------------------------------------*/ @@ -2895,6 +3046,60 @@ static void send_srej_frames (ax25_dlsm_t *S, int *resend, int count, int allow_ dw_printf ("\n"); } +// Multi-SREJ - Use info part for additional sequence number(s) instead of sending separate SREJ for each. + + if (S->srej_enable == srej_multi && count > 1) { + + unsigned char info[128]; + int info_len = 0; + + for (i = 1; i < count; i++) { // skip first one + + if (resend[i] < 0 || resend[i] >= S->modulo) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("INTERNAL ERROR, additional nr=%d, modulo=%d, %s line %d\n", resend[i], S->modulo, __func__, __LINE__); + } + + // There is also a form to specify a range but I don't + // think it is worth the effort to generate it. Maybe later. + + if (S->modulo == 8) { + info[info_len++] = resend[i] << 5; + } + else { + info[info_len++] = resend[i] << 1; + } + } + + f = 0; + nr = resend[0]; + f = allow_f1 && (nr == S->vr); + // Possibly set if we are asking for the next after + // the last one received in contiguous order. + + // This could only apply to the first in + // the list so this would not go in the loop. + + if (f) { // In this case the other end is being + // informed of my V(R) so no additional + // RR etc. is needed. + // TODO: Need to think about this. + S->acknowledge_pending = 0; + } + + if (nr < 0 || nr >= S->modulo) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("INTERNAL ERROR, nr=%d, modulo=%d, %s line %d\n", nr, S->modulo, __func__, __LINE__); + nr = AX25MODULO(nr, S->modulo, __FILE__, __func__, __LINE__); + } + + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_SREJ, S->modulo, nr, f, info, info_len); + lm_data_request (S->chan, TQ_PRIO_1_LO, pp); + return; + } + +// Multi-SREJ not enabled. Send separate SREJ for each desired sequence number. + for (i = 0; i < count; i++) { nr = resend[i]; @@ -2915,7 +3120,7 @@ static void send_srej_frames (ax25_dlsm_t *S, int *resend, int count, int allow_ nr = AX25MODULO(nr, S->modulo, __FILE__, __func__, __LINE__); } - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_SREJ, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_SREJ, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); } @@ -3019,7 +3224,6 @@ static void rr_rnr_frame (ax25_dlsm_t *S, int ready, cmdres_t cr, int pf, int nr // dw_printf ("rr_rnr_frame (), line %d, state=%d, good nr=%d, calling check_i_frame_ackd\n", __LINE__, S->state, nr); check_i_frame_ackd (S, nr); - // keep current state. } else { if (s_debug_retry) { @@ -3040,9 +3244,11 @@ static void rr_rnr_frame (ax25_dlsm_t *S, int ready, cmdres_t cr, int pf, int nr if (cr == cr_res && pf == 1) { +// RR/RNR Response with F==1. + if (s_debug_retry) { text_color_set(DW_COLOR_DEBUG); - dw_printf ("rr_rnr_frame (), Response, pf=1, line %d, state=%d, good nr, calling check_i_frame_ackd\n", __LINE__, S->state); + dw_printf ("rr_rnr_frame (), Response, f=%d, line %d, state=%d, good nr, calling check_i_frame_ackd\n", pf, __LINE__, S->state); } STOP_T1; @@ -3053,7 +3259,7 @@ static void rr_rnr_frame (ax25_dlsm_t *S, int ready, cmdres_t cr, int pf, int nr SET_VA(nr); if (S->vs == S->va) { // all caught up with ack from other guy. START_T3; - S->rc =0; // My enhancement. See Erratum note in select_t1_value. + SET_RC(0); // My enhancement. See Erratum note in select_t1_value. enter_new_state (S, state_3_connected, __func__, __LINE__); } else { @@ -3081,21 +3287,44 @@ static void rr_rnr_frame (ax25_dlsm_t *S, int ready, cmdres_t cr, int pf, int nr } } else { + +// RR/RNR command, either P value. +// RR/RNR response, F==0 + if (cr == cr_cmd && pf == 1) { int f = 1; enquiry_response (S, ready ? frame_type_S_RR : frame_type_S_RNR, f); } if (is_good_nr(S,nr)) { + SET_VA(nr); - // REJ state 4 is identical but has conditional invoke_retransmission here. + +// Erratum: v1.5 - my addition. +// I noticed that we sometimes got stuck in state 4 and rc crept up slowly even though +// we received RR frames with N(R) values indicating that the other side received everything +// that we sent. Eventually rc could reach the limit and we would get an error. +// If we are in state 4, and other guy ack'ed last I frame we sent, transition to state 3. +// The same thing was done for receving I frames after check_i_frame_ackd. + +// Thought: Could we simply call check_i_frame_ackd, for consistency, rather than only setting V(A)? + + if (cr == cr_res && pf == 0) { + + if (S->vs == S->va) { // all caught up with ack from other guy. + STOP_T1; + select_t1_value (S); + START_T3; + SET_RC(0); + enter_new_state (S, state_3_connected, __func__, __LINE__); + } + } } else { nr_error_recovery (S); enter_new_state (S, S->modulo == 128 ? state_5_awaiting_v22_connection : state_1_awaiting_connection, __func__, __LINE__); } } - break; } @@ -3293,7 +3522,7 @@ static void rej_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, int nr) SET_VA(nr); if (S->vs == S->va) { START_T3; - S->rc =0; // My enhancement. See Erratum note in select_t1_value. + SET_RC(0); // My enhancement. See Erratum note in select_t1_value. enter_new_state (S, state_3_connected, __func__, __LINE__); } else { @@ -3356,6 +3585,8 @@ static void rej_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, int nr) * cr - Is this command or response? * f - Final bit. When set, it is ack-ing up thru N(R)-1 * nr - N(R) from the frame. Peer has asked for a resend of I frame with this N(S). + * info - Information field, used only for Multi-SREJ + * info_len - Information field length, bytes. * * Description: 4.3.2.4. Selective Reject (SREJ) Command and Response * @@ -3450,7 +3681,9 @@ static void rej_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, int nr) * *------------------------------------------------------------------------------*/ -static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) +static int resend_for_srej (ax25_dlsm_t *S, int nr, unsigned char *info, int info_len); + +static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr, unsigned char *info, int info_len) { switch (S->state) { @@ -3463,7 +3696,7 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) case state_5_awaiting_v22_connection: // Do nothing. - // Erratum: The original spec said stay in same state. + // Erratum: The original spec said stay in same state. (Seems correct.) // 2006 revision shows state 5 transitioning into 1. I think that is wrong. // probably a cut-n-paste from state 1 to 5 and that part not updated. break; @@ -3507,28 +3740,13 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) START_T3; select_t1_value(S); - // Resend I frame with N(S) equal to the N(R) in the SREJ. - // Note: X.25 says info part can contain additional sequence numbers. - // Here we stay with Ax.25 and don't consider the info part. - cdata_t *txdata = S->txdata_by_ns[nr]; - - if (txdata != NULL) { - - cmdres_t cr = cr_cmd; - int i_frame_ns = nr; - int i_frame_nr = S->vr; - int p = 0; - - packet_t pp = ax25_i_frame (S->addrs, S->num_addr, cr, S->modulo, i_frame_nr, i_frame_ns, p, txdata->pid, (unsigned char *)(txdata->data), txdata->len); - - // dw_printf ("calling lm_data_request for I frame, %s line %d\n", __func__, __LINE__); - - lm_data_request (S->chan, TQ_PRIO_1_LO, pp); + int num_resent = resend_for_srej (S, nr, info, info_len); + if (num_resent) { // my addition // Erratum: We sent I frame(s) and want to timeout if no ack comes back. -// We also sent N(R) so no need for extra RR at the end only for that. +// We also sent N(R), from V(R), so no need for extra RR at the end only for that. // We would sometimes end up in a situation where T1 was stopped on // both ends and everyone would wait for the other guy to timeout and do something. @@ -3538,10 +3756,6 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) START_T1; S->acknowledge_pending = 0; } - else { - text_color_set(DW_COLOR_ERROR); - dw_printf ("Stream %d: INTERNAL ERROR for SREJ. I frame for N(S)=%d is not available.\n", S->stream_id, nr); - } // keep same state. } else { @@ -3553,6 +3767,11 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) case state_4_timer_recovery: + if (s_debug_timers) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("state 4 timer recovery, %s %d nr=%d, f=%d\n", __func__, __LINE__, nr, f); + } + S->peer_receiver_busy = 0; // Erratum: Original Flow chart has "check need for response here." @@ -3576,14 +3795,29 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) if (f) { // f=1 means ack up thru previous sequence. // Erratum: 2006 version tests "P". Original has "F." SET_VA(nr); + + if (s_debug_timers) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("state 4 timer recovery, %s %d set v(a)= %d\n", __func__, __LINE__, S->va); + } } if (S->vs == S->va) { // ACKs all caught up. Back to state 3. + // Erratum: I think this is unreachable. + // If the other side is asking for I frame with sequence X, it must have + // received X+1 or later. That means my V(S) must be X+2 or greater. + // So, I don't think we can ever have V(S) == V(A) here. + // If we were to remove the 'if' test and true part, case 4 would then + // be exactly the same as state 4. We need to rely on RR to get us + // back to state 3. + START_T3; - S->rc =0; // My enhancement. See Erratum note in select_t1_value. + SET_RC(0); // My enhancement. See Erratum note in select_t1_value. enter_new_state (S, state_3_connected, __func__, __LINE__); + // text_color_set(DW_COLOR_ERROR); + // dw_printf ("state 4 timer recovery, go to state 3 \n"); } else { @@ -3592,21 +3826,11 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) #if 1 // This is from the original protocol spec. // Resend I frame with N(S) equal to the N(R) in the SREJ. - cdata_t *txdata = S->txdata_by_ns[nr]; - - if (txdata != NULL) { - - cmdres_t cr = cr_cmd; - int i_frame_ns = nr; - int i_frame_nr = S->vr; - int p = 0; - - packet_t pp = ax25_i_frame (S->addrs, S->num_addr, cr, S->modulo, i_frame_nr, i_frame_ns, p, txdata->pid, (unsigned char *)(txdata->data), txdata->len); - - // dw_printf ("calling lm_data_request for I frame, %s line %d\n", __func__, __LINE__); - - lm_data_request (S->chan, TQ_PRIO_1_LO, pp); + //text_color_set(DW_COLOR_ERROR); + //dw_printf ("state 4 timer recovery, send requested frame(s) \n"); + int num_resent = resend_for_srej (S, nr, info, info_len); + if (num_resent) { // my addition // Erratum: We sent I frame(s) and want to timeout if no ack comes back. // We also sent N(R), from V(R), so no need for extra RR at the end only for that. @@ -3619,11 +3843,6 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) START_T1; S->acknowledge_pending = 0; } - else { - text_color_set(DW_COLOR_ERROR); - dw_printf ("Stream %d: INTERNAL ERROR for SREJ. I frame for N(S)=%d is not available.\n", S->stream_id, nr); - } - #else // Erratum! This is from the 2006 revision. // We should resend only the single requested I frame. // I think there was a cut-n-paste from the REJ flow chart and this particular place did not get changed. @@ -3641,6 +3860,87 @@ static void srej_frame (ax25_dlsm_t *S, cmdres_t cr, int f, int nr) } /* end srej_frame */ +/*------------------------------------------------------------------------------ + * + * Name: resend_for_srej + * + * Purpose: Resend the I frame(s) specified in SREJ response. + * + * Inputs: S - Data Link State Machine. + * nr - N(R) from the frame. Peer has asked for a resend of I frame with this N(S). + * info - Information field, might contain additional sequence numbers for Multi-SREJ. + * info_len - Information field length, bytes. + * + * Returns: Number of frames sent. Should be at least one. + * + * Description: Simply resend requested frame(s). + * The calling context will worry about the F bit and other state stuff. + * + *------------------------------------------------------------------------------*/ + +static int resend_for_srej (ax25_dlsm_t *S, int nr, unsigned char *info, int info_len) +{ + cmdres_t cr = cr_cmd; + int i_frame_nr = S->vr; + int i_frame_ns = nr; + int p = 0; + int num_resent = 0; + + // Resend I frame with N(S) equal to the N(R) in the SREJ. + // Additional sequence numbers can be in optional information part. + + cdata_t *txdata = S->txdata_by_ns[i_frame_ns]; + + if (txdata != NULL) { + packet_t pp = ax25_i_frame (S->addrs, S->num_addr, cr, S->modulo, i_frame_nr, i_frame_ns, p, txdata->pid, (unsigned char *)(txdata->data), txdata->len); + // dw_printf ("calling lm_data_request for I frame, %s line %d\n", __func__, __LINE__); + lm_data_request (S->chan, TQ_PRIO_1_LO, pp); + num_resent++; + } + else { + text_color_set(DW_COLOR_ERROR); + dw_printf ("Stream %d: INTERNAL ERROR for SREJ. I frame for N(S)=%d is not available.\n", S->stream_id, i_frame_ns); + } + +// Multi-SREJ if there is an information part. + + int j; + for (j = 0; j < info_len; j++) { + + // We can have a single sequence number like this: + // xxx00000 (mod 8) + // xxxxxxx0 (mod 128) + // or we can have span (mod 128 only) like this, with the first and last: + // xxxxxxx1 + // xxxxxxx1 + // + // Note that the sequence number is shifted left by one + // and if the LSB is set, there should be two adjacent bytes + // with it set. + + if (S->modulo == 8) { + i_frame_ns = (info[j] >> 5) & 0x07; // no provision for span. + } + else { + i_frame_ns = (info[j] >> 1) & 0x7f; // TODO: test LSB and possible loop here. + } + + txdata = S->txdata_by_ns[i_frame_ns]; + if (txdata != NULL) { + packet_t pp = ax25_i_frame (S->addrs, S->num_addr, cr, S->modulo, i_frame_nr, i_frame_ns, p, txdata->pid, (unsigned char *)(txdata->data), txdata->len); + lm_data_request (S->chan, TQ_PRIO_1_LO, pp); + num_resent++; + } + else { + text_color_set(DW_COLOR_ERROR); + dw_printf ("Stream %d: INTERNAL ERROR for Multi-SREJ. I frame for N(S)=%d is not available.\n", S->stream_id, i_frame_ns); + } + } + return (num_resent); + +} /* end resend_for_srej */ + + /*------------------------------------------------------------------------------ @@ -3733,7 +4033,7 @@ static void sabm_e_frame (ax25_dlsm_t *S, int extended, int p) INIT_T1V_SRT; START_T3; - S->rc =0; // My enhancement. See Erratum note in select_t1_value. + SET_RC(0); // My enhancement. See Erratum note in select_t1_value. enter_new_state (S, state_3_connected, __func__, __LINE__); break; @@ -3841,7 +4141,7 @@ static void sabm_e_frame (ax25_dlsm_t *S, int extended, int p) SET_VS(0); SET_VA(0); SET_VR(0); - S->rc =0; // My enhancement. See Erratum note in select_t1_value. + SET_RC(0); // My enhancement. See Erratum note in select_t1_value. enter_new_state (S, state_3_connected, __func__, __LINE__); } break; @@ -4269,7 +4569,7 @@ static void ua_frame (ax25_dlsm_t *S, int f) mdl_negotiate_request (S); } - S->rc =0; // My enhancement. See Erratum note in select_t1_value. + SET_RC(0); // My enhancement. See Erratum note in select_t1_value. enter_new_state (S, state_3_connected, __func__, __LINE__); } else { @@ -4547,7 +4847,7 @@ static void ui_frame (ax25_dlsm_t *S, cmdres_t cr, int pf) static void xid_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, unsigned char *info_ptr, int info_len) { struct xid_param_s param; - char desc[120]; + char desc[150]; int ok; unsigned char xinfo[40]; int xlen; @@ -4574,7 +4874,7 @@ static void xid_frame (ax25_dlsm_t *S, cmdres_t cr, int pf, unsigned char *info_ if (ok) { negotiation_response (S, ¶m); - xlen = xid_encode (¶m, xinfo); + xlen = xid_encode (¶m, xinfo, res); pp = ax25_u_frame (S->addrs, S->num_addr, res, frame_type_U_XID, f, nopid, xinfo, xlen); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); @@ -4814,7 +5114,7 @@ static void t1_expiry (ax25_dlsm_t *S) packet_t pp; - S->rc++; + SET_RC(S->rc+1); if (S->rc > S->peak_rc_value) S->peak_rc_value = S->rc; // Keep statistics. pp = ax25_u_frame (S->addrs, S->num_addr, cmd, (S->state == state_5_awaiting_v22_connection) ? frame_type_U_SABME : frame_type_U_SABM, p, nopid, NULL, 0); @@ -4840,7 +5140,7 @@ static void t1_expiry (ax25_dlsm_t *S) packet_t pp; - S->rc++; + SET_RC(S->rc+1); if (S->rc > S->peak_rc_value) S->peak_rc_value = S->rc; pp = ax25_u_frame (S->addrs, S->num_addr, cmd, frame_type_U_DISC, p, nopid, NULL, 0); @@ -4853,7 +5153,7 @@ static void t1_expiry (ax25_dlsm_t *S) case state_3_connected: - S->rc = 1; + SET_RC(1); transmit_enquiry (S); enter_new_state (S, state_4_timer_recovery, __func__, __LINE__); break; @@ -4864,25 +5164,25 @@ static void t1_expiry (ax25_dlsm_t *S) // Erratum: 2006 version, page 103, is missing yes/no labels on decision blocks. - if (S->va != S->vr) { + if (S->va != S->vs) { if (s_debug_protocol_errors) { text_color_set(DW_COLOR_ERROR); - dw_printf ("Stream %d: AX.25 Protocol Error I: N2 timeouts: unacknowledged data.\n", S->stream_id); + dw_printf ("Stream %d: AX.25 Protocol Error I: %d timeouts: unacknowledged sent data.\n", S->stream_id, S->n2_retry); } } else if (S->peer_receiver_busy) { if (s_debug_protocol_errors) { text_color_set(DW_COLOR_ERROR); - dw_printf ("Stream %d: AX.25 Protocol Error U: N2 timeouts: extended peer busy condition.\n", S->stream_id); + dw_printf ("Stream %d: AX.25 Protocol Error U: %d timeouts: extended peer busy condition.\n", S->stream_id, S->n2_retry); } } else { if (s_debug_protocol_errors) { text_color_set(DW_COLOR_ERROR); - dw_printf ("Stream %d: AX.25 Protocol Error T: N2 timeouts: no response to enquiry.\n", S->stream_id); + dw_printf ("Stream %d: AX.25 Protocol Error T: %d timeouts: no response to enquiry.\n", S->stream_id, S->n2_retry); } } @@ -4907,7 +5207,7 @@ static void t1_expiry (ax25_dlsm_t *S) enter_new_state (S, state_0_disconnected, __func__, __LINE__); } else { - S->rc++; + SET_RC(S->rc+1); if (S->rc > S->peak_rc_value) S->peak_rc_value = S->rc; // gather statistics. transmit_enquiry (S); @@ -4971,7 +5271,7 @@ static void t3_expiry (ax25_dlsm_t *S) // Erratum: Original sets RC to 0, 2006 revision sets RC to 1 which makes more sense. - S->rc = 1; + SET_RC(1); transmit_enquiry (S); enter_new_state (S, state_4_timer_recovery, __func__, __LINE__); break; @@ -5034,7 +5334,7 @@ static void tm201_expiry (ax25_dlsm_t *S) initiate_negotiation (S, ¶m); - xlen = xid_encode (¶m, xinfo); + xlen = xid_encode (¶m, xinfo, cmd); pp = ax25_u_frame (S->addrs, S->num_addr, cmd, frame_type_U_XID, p, nopid, xinfo, xlen); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); @@ -5108,7 +5408,7 @@ static void establish_data_link (ax25_dlsm_t *S) // Flow chart shows setting RC to 0 and we end up sending SAMB(e) 11 times when N2 (RETRY) is 10. // It should be 1 rather than 0. - S->rc = 1; + SET_RC(1); pp = ax25_u_frame (S->addrs, S->num_addr, cmd, (S->modulo == 128) ? frame_type_U_SABME : frame_type_U_SABM, p, nopid, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); STOP_T3; @@ -5203,14 +5503,14 @@ static void transmit_enquiry (ax25_dlsm_t *S) if (s_debug_retry) { text_color_set(DW_COLOR_ERROR); - dw_printf ("\n****** TRANSMIT ENQUIRY ******\n\n"); + dw_printf ("\n****** TRANSMIT ENQUIRY RR/RNR cmd P=1 ****** state=%d, rc=%d\n\n", S->state, S->rc); } // This is the ONLY place that we send RR/RNR *command* with P=1. // Everywhere else should be response. // I don't think we ever use RR/RNR command P=0 but need to check on that. - pp = ax25_s_frame (S->addrs, S->num_addr, cmd, S->own_receiver_busy ? frame_type_S_RNR : frame_type_S_RR, S->modulo, nr, p); + pp = ax25_s_frame (S->addrs, S->num_addr, cmd, S->own_receiver_busy ? frame_type_S_RNR : frame_type_S_RR, S->modulo, nr, p, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); @@ -5254,7 +5554,7 @@ static void transmit_enquiry (ax25_dlsm_t *S) * The next response frame returned to a supervisory command frame with the P bit set to "1", received during * the information transfer state, is an RR, RNR or REJ response frame with the F bit set to "1". * - * Erattum! The flow chart says RR/RNR *command* but I'm confident should be response. + * Erattum! The flow chart says RR/RNR *command* but I'm confident it should be response. * * Erratum: Ax.25 spec has nothing here for SREJ. See X.25 2.4.6.11 for explanation. * @@ -5284,13 +5584,14 @@ static void enquiry_response (ax25_dlsm_t *S, ax25_frame_type_t frame_type, int // I'm busy. - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RNR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RNR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); - S->acknowledge_pending = 0; + S->acknowledge_pending = 0; // because we sent N(R) from V(R). } - else if (S->srej_enabled) { + else if (S->srej_enable == srej_single || S->srej_enable == srej_multi) { + // SREJ is enabled. This is based on X.25 2.4.6.11. @@ -5337,7 +5638,7 @@ static void enquiry_response (ax25_dlsm_t *S, ax25_frame_type_t frame_type, int // Not waiting for fill in of missing frames. X.25 2.4.6.11 c) - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); S->acknowledge_pending = 0; @@ -5351,7 +5652,12 @@ static void enquiry_response (ax25_dlsm_t *S, ax25_frame_type_t frame_type, int // And when we look at what happens when RR response, F=1 is received in state 4, it is // effectively REJ when N(R) is not the same as V(S). - pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f); + if (s_debug_retry) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("\n****** ENQUIRY RESPONSE srej not enbled, sending RR resp F=%d ******\n\n", f); + } + + pp = ax25_s_frame (S->addrs, S->num_addr, cr, frame_type_S_RR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); S->acknowledge_pending = 0; @@ -5363,7 +5669,7 @@ static void enquiry_response (ax25_dlsm_t *S, ax25_frame_type_t frame_type, int // For cases other than (RR, RNR, I) command, P=1. - pp = ax25_s_frame (S->addrs, S->num_addr, cr, S->own_receiver_busy ? frame_type_S_RNR : frame_type_S_RR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, S->own_receiver_busy ? frame_type_S_RNR : frame_type_S_RR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); S->acknowledge_pending = 0; @@ -5375,7 +5681,7 @@ static void enquiry_response (ax25_dlsm_t *S, ax25_frame_type_t frame_type, int // Erratum: This is woefully inadequate when SREJ is enabled. // Erratum: Flow chart says RR/RNR command but I'm confident it should be response. - pp = ax25_s_frame (S->addrs, S->num_addr, cr, S->own_receiver_busy ? frame_type_S_RNR : frame_type_S_RR, S->modulo, nr, f); + pp = ax25_s_frame (S->addrs, S->num_addr, cr, S->own_receiver_busy ? frame_type_S_RNR : frame_type_S_RR, S->modulo, nr, f, NULL, 0); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); S->acknowledge_pending = 0; @@ -5395,9 +5701,14 @@ static void enquiry_response (ax25_dlsm_t *S, ax25_frame_type_t frame_type, int * Continue will all up to and including current V(S) value. * * Description: Resend one or more frames that have already been sent. + * Should always send at least one. * * This is probably the result of getting REJ asking for a resend. * + * Context: I would expect the caller to clear 'acknowledge_pending' after calling this + * because we sent N(R), from V(R), to ack what was received from other guy. + * I would also expect Stop T3 & Start T1 at the same place. + * *------------------------------------------------------------------------------*/ static void invoke_retransmission (ax25_dlsm_t *S, int nr_input) @@ -5411,6 +5722,19 @@ static void invoke_retransmission (ax25_dlsm_t *S, int nr_input) int local_vs; int sent_count = 0; + + if (s_debug_misc) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("invoke_retransmission(): starting with %d, state=%d, rc=%d, \n", nr_input, S->state, S->rc); + } + +// I don't think we should be here if SREJ is enabled. +// TODO: Figure out why this happens occasionally. + +// if (S->srej_enable != srej_none) { +// text_color_set(DW_COLOR_ERROR); +// dw_printf ("Internal Error, Did not expect to be here when SREJ enabled. %s %s %d\n", __FILE__, __func__, __LINE__); +// } if (S->txdata_by_ns[nr_input] == NULL) { text_color_set(DW_COLOR_ERROR); @@ -5431,7 +5755,7 @@ static void invoke_retransmission (ax25_dlsm_t *S, int nr_input) if (s_debug_misc) { text_color_set(DW_COLOR_INFO); - dw_printf ("invoke_retransmission(): state=%d, Resending N(S) = %d, probably as result of REJ N(R) = %d\n", S->state, ns, nr_input); + dw_printf ("invoke_retransmission(): Resending N(S) = %d\n", ns); } packet_t pp = ax25_i_frame (S->addrs, S->num_addr, cr, S->modulo, nr, ns, p, @@ -5470,7 +5794,9 @@ static void invoke_retransmission (ax25_dlsm_t *S, int nr_input) * * Outputs: S->va - updated from nr. * - * Description: TBD... Document when this is used. + * Description: This is called for: + * - 'I' frame received and N(R) is in expected range, states 3 & 4. + * - RR/RNR command with p=1 received and N(R) is in expected range, state 3 only. * *------------------------------------------------------------------------------*/ @@ -5702,7 +6028,7 @@ static void select_t1_value (ax25_dlsm_t *S) static void set_version_2_0 (ax25_dlsm_t *S) { - S->srej_enabled = 0; + S->srej_enable = srej_none; S->modulo = 8; S->n1_paclen = g_misc_config_p->paclen; S->k_maxframe = g_misc_config_p->maxframe_basic; @@ -5719,8 +6045,8 @@ static void set_version_2_0 (ax25_dlsm_t *S) static void set_version_2_2 (ax25_dlsm_t *S) { - S->srej_enabled = 1; - //S->srej_enabled = 0; // temporarily disable for testing of REJ only with modulo 128 + S->srej_enable = srej_single; // Start with single. + // Can be increased to multi with XID exchange. S->modulo = 128; S->n1_paclen = g_misc_config_p->paclen; S->k_maxframe = g_misc_config_p->maxframe_extended; @@ -5757,7 +6083,7 @@ static void set_version_2_2 (ax25_dlsm_t *S) * * Pattern noticed: Anytime we have "is_good_nr" returning true, we should always * - set V(A) from N(R) or - * - call "check_i_frame_acked" which does the same and some timer stuff. + * - call "check_i_frame_ackd" which does the same and some timer stuff. * *------------------------------------------------------------------------------*/ @@ -5804,7 +6130,7 @@ static int is_good_nr (ax25_dlsm_t *S, int nr) * k * * Outputs: v(s) is incremented for each processed. - * ack_pending = 0 + * acknowledge_pending = 0 * *------------------------------------------------------------------------------*/ @@ -5975,7 +6301,7 @@ static void discard_i_queue (ax25_dlsm_t *S) * *------------------------------------------------------------------------------*/ -// TODO: requeuing... +// TODO: requeuing??? static void enter_new_state (ax25_dlsm_t *S, enum dlsm_state_e new_state, const char *from_func, int from_line) { @@ -6039,7 +6365,7 @@ static void mdl_negotiate_request (ax25_dlsm_t *S) initiate_negotiation (S, ¶m); - xlen = xid_encode (¶m, xinfo); + xlen = xid_encode (¶m, xinfo, cmd); pp = ax25_u_frame (S->addrs, S->num_addr, cmd, frame_type_U_XID, p, nopid, xinfo, xlen); lm_data_request (S->chan, TQ_PRIO_1_LO, pp); @@ -6072,7 +6398,17 @@ static void mdl_negotiate_request (ax25_dlsm_t *S) static void initiate_negotiation (ax25_dlsm_t *S, struct xid_param_s *param) { param->full_duplex = 0; - param->rej = S->srej_enabled ? selective_reject : implicit_reject; + switch (S->srej_enable) { + case srej_single: + case srej_multi: + param->srej = srej_multi; // see if other end reconizes it. + break; + case srej_none: + default: + param->srej = srej_none; + break; + } + param->modulo = S->modulo; param->i_field_length_rx = S->n1_paclen; // Hmmmm. Should we ask for what the user // specified for PACLEN or offer the maximum @@ -6106,9 +6442,7 @@ static void initiate_negotiation (ax25_dlsm_t *S, struct xid_param_s *param) static void negotiation_response (ax25_dlsm_t *S, struct xid_param_s *param) { -// Full duplex would not be that difficult. -// Just ignore the Carrier Detect when transmitting. -// But we haven't done that yet. +// TODO: Integrate with new full duplex capability in v1.5. param->full_duplex = 0; @@ -6126,11 +6460,8 @@ static void negotiation_response (ax25_dlsm_t *S, struct xid_param_s *param) // Erratum: 2006 version, section, 4.3.3.7 says default selective reject - reject. // We can't do that. - if (param->rej == unknown_reject) { - param->rej = (param->modulo == 128) ? selective_reject : implicit_reject; // not specified, set default - } - else { - param->rej = MIN(param->rej, selective_reject); + if (param->srej == srej_not_specified) { + param->srej = (param->modulo == 128) ? srej_single : srej_none; // not specified, set default } // We can currently do up to 2k. @@ -6198,8 +6529,8 @@ static void negotiation_response (ax25_dlsm_t *S, struct xid_param_s *param) static void complete_negotiation (ax25_dlsm_t *S, struct xid_param_s *param) { - if (param->rej != unknown_reject) { - S->srej_enabled = param->rej >= selective_reject; + if (param->srej != srej_not_specified) { + S->srej_enable = param->srej; } if (param->modulo != modulo_unknown) { diff --git a/ax25_pad2.c b/ax25_pad2.c index 7c37dbf..516fe9d 100644 --- a/ax25_pad2.c +++ b/ax25_pad2.c @@ -355,15 +355,19 @@ packet_t ax25_u_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_ad * * pf - Poll/Final flag. * + * pinfo - Pointer to data for Info field. Allowed only for SREJ. + * + * info_len - Length for Info field. + * * * Returns: Pointer to new packet object. * *------------------------------------------------------------------------------*/ #if AX25MEMDEBUG -packet_t ax25_s_frame_debug (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf, char *src_file, int src_line) +packet_t ax25_s_frame_debug (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf, unsigned char *pinfo, int info_len, char *src_file, int src_line) #else -packet_t ax25_s_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf) +packet_t ax25_s_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf, unsigned char *pinfo, int info_len) #endif { packet_t this_p; @@ -383,7 +387,7 @@ packet_t ax25_s_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_ad if ( ! set_addrs (this_p, addrs, num_addr, cr)) { text_color_set(DW_COLOR_ERROR); - dw_printf ("Internal error in %s: Could not set addresses for U frame.\n", __func__); + dw_printf ("Internal error in %s: Could not set addresses for S frame.\n", __func__); ax25_delete (this_p); return (NULL); } @@ -401,6 +405,14 @@ packet_t ax25_s_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_ad nr &= (modulo - 1); } + // Erratum: The AX.25 spec is not clear about whether SREJ should be command, response, or both. + // The underlying X.25 spec clearly says it is reponse only. Let's go with that. + + if (ftype == frame_type_S_SREJ && cr != cr_res) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("Internal error in %s: SREJ must be response.\n", __func__); + } + switch (ftype) { case frame_type_S_RR: ctrl = 0x01; break; @@ -434,6 +446,24 @@ packet_t ax25_s_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_ad this_p->frame_len++; } + if (ftype == frame_type_S_SREJ) { + if (pinfo != NULL && info_len > 0) { + if (info_len > AX25_MAX_INFO_LEN) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("Internal error in %s: SREJ frame, Invalid information field length %d.\n", __func__, info_len); + info_len = AX25_MAX_INFO_LEN; + } + memcpy (p, pinfo, info_len); + p += info_len; + this_p->frame_len += info_len; + } + } + else { + if (pinfo != NULL || info_len != 0) { + text_color_set(DW_COLOR_ERROR); + dw_printf ("Internal error in %s: Info part not allowed for RR, RNR, REJ frame.\n", __func__); + } + } *p = '\0'; assert (p == this_p->frame_data + this_p->frame_len); @@ -813,7 +843,7 @@ int main () text_color_set(DW_COLOR_INFO); dw_printf ("\nConstruct S frame, cmd=%d, ftype=%d, pid=0x%02x\n", cr, ftype, pid); - pp = ax25_s_frame (addrs, num_addr, cr, ftype, modulo, nr, pf); + pp = ax25_s_frame (addrs, num_addr, cr, ftype, modulo, nr, pf, NULL, 0); ax25_hex_dump (pp); ax25_delete (pp); @@ -827,7 +857,7 @@ int main () text_color_set(DW_COLOR_INFO); dw_printf ("\nConstruct S frame, cmd=%d, ftype=%d, pid=0x%02x\n", cr, ftype, pid); - pp = ax25_s_frame (addrs, num_addr, cr, ftype, modulo, nr, pf); + pp = ax25_s_frame (addrs, num_addr, cr, ftype, modulo, nr, pf, NULL, 0); ax25_hex_dump (pp); ax25_delete (pp); @@ -835,6 +865,26 @@ int main () } } +/* SREJ is only S frame which can have information part. */ + + static char srej_info[] = { 1<<1, 2<<1, 3<<1, 4<<1 }; + + ftype = frame_type_S_SREJ; + for (pf = 0; pf <= 1; pf++) { + + modulo = 128; + nr = 127; + cr = cr_res; + + text_color_set(DW_COLOR_INFO); + dw_printf ("\nConstruct Multi-SREJ S frame, cmd=%d, ftype=%d, pid=0x%02x\n", cr, ftype, pid); + + pp = ax25_s_frame (addrs, num_addr, cr, ftype, modulo, nr, pf, srej_info, (int)(sizeof(srej_info))); + + ax25_hex_dump (pp); + ax25_delete (pp); + } + dw_printf ("\n----------\n\n"); /* I frame */ diff --git a/ax25_pad2.h b/ax25_pad2.h index 4860941..c6dc17a 100644 --- a/ax25_pad2.h +++ b/ax25_pad2.h @@ -22,14 +22,14 @@ packet_t ax25_u_frame_debug (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int pf, int pid, unsigned char *pinfo, int info_len, char *src_file, int src_line); -packet_t ax25_s_frame_debug (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf, char *src_file, int src_line); +packet_t ax25_s_frame_debug (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf, unsigned char *pinfo, int info_len, char *src_file, int src_line); packet_t ax25_i_frame_debug (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, int modulo, int nr, int ns, int pf, int pid, unsigned char *pinfo, int info_len, char *src_file, int src_line); #define ax25_u_frame(a,n,c,f,p,q,i,l) ax25_u_frame_debug(a,n,c,f,p,q,i,l,__FILE__,__LINE__) -#define ax25_s_frame(a,n,c,f,m,r,p) ax25_s_frame_debug(a,n,c,f,m,r,p,__FILE__,__LINE__) +#define ax25_s_frame(a,n,c,f,m,r,p,i,l) ax25_s_frame_debug(a,n,c,f,m,r,p,i,l,__FILE__,__LINE__) #define ax25_i_frame(a,n,c,m,r,s,p,q,i,l) ax25_i_frame_debug(a,n,c,m,r,s,p,q,i,l,__FILE__,__LINE__) @@ -38,7 +38,7 @@ packet_t ax25_i_frame_debug (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int packet_t ax25_u_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int pf, int pid, unsigned char *pinfo, int info_len); -packet_t ax25_s_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf); +packet_t ax25_s_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, ax25_frame_type_t ftype, int modulo, int nr, int pf, unsigned char *pinfo, int info_len); packet_t ax25_i_frame (char addrs[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN], int num_addr, cmdres_t cr, int modulo, int nr, int ns, int pf, int pid, unsigned char *pinfo, int info_len); diff --git a/xid.c b/xid.c index 90ce437..a96b89a 100644 --- a/xid.c +++ b/xid.c @@ -2,7 +2,7 @@ // // This file is part of Dire Wolf, an amateur radio packet TNC. // -// Copyright (C) 2014, 2016 John Langner, WB2OSZ +// Copyright (C) 2014, 2016, 2017 John Langner, WB2OSZ // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -50,6 +50,7 @@ #include #include #include +#include #include "textcolor.h" #include "xid.h" @@ -66,10 +67,36 @@ #define PI_Ack_Timer 9 #define PI_Retries 10 +// Forget about the bit order at the physical layer (e.g. HDLC). +// It doesn't matter at all here. We are dealing with bytes. +// A different encoding could send the bits in the opposite order. + +// The bit numbers are confusing because this one table (Fig. 4.5) starts +// with 1 for the LSB when everywhere else refers to the LSB as bit 0. + +// The first byte must be of the form 0xx0 0001 +// The second byte must be of the form 0000 0000 +// If we process the two byte "Classes of Procedures" like +// the other multibyte numeric fields, with the more significant +// byte first, we end up with the bit masks below. +// The bit order would be 8 7 6 5 4 3 2 1 16 15 14 13 12 11 10 9 + +// (This has nothing to do with the HDLC serializing order. +// I'm talking about the way we would normally write binary numbers.) + #define PV_Classes_Procedures_Balanced_ABM 0x0100 #define PV_Classes_Procedures_Half_Duplex 0x2000 #define PV_Classes_Procedures_Full_Duplex 0x4000 + +// The first byte must be of the form 1000 0xx0 +// The second byte must be of the form 1010 xx00 +// The third byte must be of the form 0000 0010 +// If we process the three byte "HDLC Optional Parmeters" like +// the other multibyte numeric fields, with the most significant +// byte first, we end up with bit masks like this. +// The bit order would be 8 7 6 5 4 3 2 1 16 15 14 13 12 11 10 9 24 23 22 21 20 19 18 17 + #define PV_HDLC_Optional_Functions_REJ_cmd_resp 0x020000 #define PV_HDLC_Optional_Functions_SREJ_cmd_resp 0x040000 #define PV_HDLC_Optional_Functions_Extended_Address 0x800000 @@ -79,6 +106,9 @@ #define PV_HDLC_Optional_Functions_TEST_cmd_resp 0x002000 #define PV_HDLC_Optional_Functions_16_bit_FCS 0x008000 +#define PV_HDLC_Optional_Functions_Multi_SREJ_cmd_resp 0x000020 +#define PV_HDLC_Optional_Functions_Segmenter 0x000040 + #define PV_HDLC_Optional_Functions_Synchronous_Tx 0x000002 @@ -127,7 +157,7 @@ int xid_parse (unsigned char *info, int info_len, struct xid_param_s *result, ch // rej and modulo are enum so we can't use G_UNKNOWN there. result->full_duplex = G_UNKNOWN; - result->rej = unknown_reject; + result->srej = srej_not_specified; result->modulo = modulo_unknown; result->i_field_length_rx = G_UNKNOWN; result->window_size_rx = G_UNKNOWN; @@ -204,21 +234,31 @@ int xid_parse (unsigned char *info, int info_len, struct xid_param_s *result, ch case PI_HDLC_Optional_Functions: - if ((pval & PV_HDLC_Optional_Functions_REJ_cmd_resp) && (pval & PV_HDLC_Optional_Functions_SREJ_cmd_resp)) { - result->rej = selective_reject_reject; /* Both bits set */ - strlcat (desc, "SREJ-REJ ", desc_size); - } - else if ((pval & PV_HDLC_Optional_Functions_REJ_cmd_resp) && ! (pval & PV_HDLC_Optional_Functions_SREJ_cmd_resp)) { - result->rej = implicit_reject; /* Only REJ is set */ + // Pick highest of those offered. + + if (pval & PV_HDLC_Optional_Functions_REJ_cmd_resp) { strlcat (desc, "REJ ", desc_size); } - else if ( ! (pval & PV_HDLC_Optional_Functions_REJ_cmd_resp) && pval & PV_HDLC_Optional_Functions_SREJ_cmd_resp) { - result->rej = selective_reject; /* Only SREJ is set */ + if (pval & PV_HDLC_Optional_Functions_SREJ_cmd_resp) { strlcat (desc, "SREJ ", desc_size); } + if (pval & PV_HDLC_Optional_Functions_Multi_SREJ_cmd_resp) { + strlcat (desc, "Multi-SREJ ", desc_size); + } + + if (pval & PV_HDLC_Optional_Functions_Multi_SREJ_cmd_resp) { + result->srej = srej_multi; + } + else if (pval & PV_HDLC_Optional_Functions_SREJ_cmd_resp) { + result->srej = srej_single; + } + else if (pval & PV_HDLC_Optional_Functions_REJ_cmd_resp) { + result->srej = srej_none; + } else { text_color_set (DW_COLOR_ERROR); - dw_printf ("XID error: Expected one or both of REJ, SREJ to be set.\n"); + dw_printf ("XID error: Expected at least one of REJ, SREJ, Multi-SREJ to be set.\n"); + result->srej = srej_none; } if ((pval & PV_HDLC_Optional_Functions_Modulo_8) && ! (pval & PV_HDLC_Optional_Functions_Modulo_128)) { @@ -331,10 +371,10 @@ int xid_parse (unsigned char *info, int info_len, struct xid_param_s *result, ch * 0 = half duplex. * 1 = full duplex. * - * rej - One of: implicit_reject, selective_reject, selective_reject_reject. - * As command, what am I capable of processing? - * As response, take minimum of - * + * srej - Level of selective reject. + * srej_none (use REJ), srej_single, srej_multi + * As command, offer a menu of what I can handle. (i.e. perhaps multiple bits set) + * As response, take minimum of what is offered and what I can handle. (one bit set) * * modulo - 8 or 128. * @@ -355,6 +395,8 @@ int xid_parse (unsigned char *info, int info_len, struct xid_param_s *result, ch * Default is 10. * Use G_UNKNOWN to omit this. * + * cr - Is it a command or response? + * * Outputs: info - Information part of XID frame. * Does not include the control byte. * Use buffer of 40 bytes just to be safe. @@ -382,10 +424,17 @@ int xid_parse (unsigned char *info, int info_len, struct xid_param_s *result, ch * Comment: I have a problem with "... occurs at any time." What if we were in the middle * of transferring a large file with k=32 then along comes XID which says switch to modulo 8? * + * Insight: Or is it Erratum? + * After reading the base standards documents, it seems that the XID command should offer + * up a menu of all the acceptable choices. e.g. REJ, SREJ, Multi-SREJ. One or more bits + * can be set. The XID response, would set a single bit which is the desired choice from + * among those offered. + * Should go back and review half/full duplex and modulo. + * *--------------------------------------------------------------------*/ -int xid_encode (struct xid_param_s *param, unsigned char *info) +int xid_encode (struct xid_param_s *param, unsigned char *info, cmdres_t cr) { unsigned char *p; int len; @@ -438,11 +487,43 @@ int xid_encode (struct xid_param_s *param, unsigned char *info) PV_HDLC_Optional_Functions_16_bit_FCS | PV_HDLC_Optional_Functions_Synchronous_Tx; - if (param->rej == implicit_reject || param->rej == selective_reject_reject || param->rej == unknown_reject) - x |= PV_HDLC_Optional_Functions_REJ_cmd_resp; + //text_color_set (DW_COLOR_ERROR); + //dw_printf ("****** XID temp hack - test no SREJ ******\n"); + // param->srej = srej_none; - if (param->rej == selective_reject || param->rej == selective_reject_reject) - x |= PV_HDLC_Optional_Functions_SREJ_cmd_resp; + if (cr == cr_cmd) { + // offer a "menu" of acceptable choices. i.e. 1, 2 or 3 bits set. + switch (param->srej) { + case srej_none: + default: + x |= PV_HDLC_Optional_Functions_REJ_cmd_resp; + break; + case srej_single: + x |= PV_HDLC_Optional_Functions_REJ_cmd_resp | + PV_HDLC_Optional_Functions_SREJ_cmd_resp; + break; + case srej_multi: + x |= PV_HDLC_Optional_Functions_REJ_cmd_resp | + PV_HDLC_Optional_Functions_SREJ_cmd_resp | + PV_HDLC_Optional_Functions_Multi_SREJ_cmd_resp; + break; + } + } + else { + // for response, set only a single bit. + switch (param->srej) { + case srej_none: + default: + x |= PV_HDLC_Optional_Functions_REJ_cmd_resp; + break; + case srej_single: + x |= PV_HDLC_Optional_Functions_SREJ_cmd_resp; + break; + case srej_multi: + x |= PV_HDLC_Optional_Functions_Multi_SREJ_cmd_resp; + break; + } + } if (param->modulo == modulo_128) x |= PV_HDLC_Optional_Functions_Modulo_128; @@ -531,7 +612,7 @@ static unsigned char example[27] = { /* GL */ 0x17, /* (2 bytes) */ /* PI */ 0x02, /* Parameter Indicator - classes of procedures */ /* PL */ 0x02, /* Parameter Length */ -#if 0 // Example in the protocol spec looks wrong. +#if 0 // Erratum: Example in the protocol spec looks wrong. /* PV */ 0x00, /* Parameter Variable - Half Duplex, Async, Balanced Mode */ /* PV */ 0x20, /* */ #else // I think it should be like this instead. @@ -545,6 +626,11 @@ static unsigned char example[27] = { /* PV */ 0x02, /* synchronous transmit */ /* PI */ 0x06, /* Parameter Indicator - Rx I field length (bits) */ /* PL */ 0x02, /* Parameter Length */ + +// Erratum: The text does not say anything about the byte order for multibyte +// numeric values. In the example, we have two cases where 16 bit numbers are +// sent with the more significant byte first. + /* PV */ 0x04, /* Parameter Variable - 1024 bits (128 octets) */ /* PV */ 0x00, /* */ /* PI */ 0x08, /* Parameter Indicator - Rx window size */ @@ -565,7 +651,7 @@ int main (int argc, char *argv[]) { struct xid_param_s param2; int n; unsigned char info[40]; // Currently max of 27 but things can change. - char desc[100]; // 80 is not adequate. + char desc[150]; // I've seen 109. /* parse example. */ @@ -573,13 +659,15 @@ int main (int argc, char *argv[]) { n = xid_parse (example, sizeof(example), ¶m, desc, sizeof(desc)); text_color_set (DW_COLOR_DEBUG); - dw_printf ("%s\n", desc); + dw_printf ("%d: %s\n", __LINE__, desc); + fflush (stdout); + SLEEP_SEC (1); text_color_set (DW_COLOR_ERROR); assert (n==1); assert (param.full_duplex == 0); - assert (param.rej == selective_reject_reject); + assert (param.srej == srej_single); assert (param.modulo == modulo_128); assert (param.i_field_length_rx == 128); assert (param.window_size_rx == 2); @@ -588,7 +676,7 @@ int main (int argc, char *argv[]) { /* encode and verify it comes out the same. */ - n = xid_encode (¶m, info); + n = xid_encode (¶m, info, cr_cmd); assert (n == sizeof(example)); n = memcmp(info, example, 27); @@ -599,52 +687,56 @@ int main (int argc, char *argv[]) { assert (n == 0); -/* try a couple different values. */ +/* try a couple different values, no srej. */ param.full_duplex = 1; - param.rej = implicit_reject; + param.srej = srej_none; param.modulo = modulo_8; param.i_field_length_rx = 2048; param.window_size_rx = 3; param.ack_timer = 1234; param.retries = 12; - n = xid_encode (¶m, info); + n = xid_encode (¶m, info, cr_cmd); n = xid_parse (info, n, ¶m2, desc, sizeof(desc)); text_color_set (DW_COLOR_DEBUG); - dw_printf ("%s\n", desc); + dw_printf ("%d: %s\n", __LINE__, desc); + fflush (stdout); + SLEEP_SEC (1); text_color_set (DW_COLOR_ERROR); assert (param2.full_duplex == 1); - assert (param2.rej == implicit_reject); + assert (param2.srej == srej_none); assert (param2.modulo == modulo_8); assert (param2.i_field_length_rx == 2048); assert (param2.window_size_rx == 3); assert (param2.ack_timer == 1234); assert (param2.retries == 12); -/* The third possbility for rej. We don't use this. */ +/* Other values, single srej. */ param.full_duplex = 0; - param.rej = selective_reject; + param.srej = srej_single; param.modulo = modulo_8; param.i_field_length_rx = 61; param.window_size_rx = 4; param.ack_timer = 5555; param.retries = 9; - n = xid_encode (¶m, info); + n = xid_encode (¶m, info, cr_cmd); n = xid_parse (info, n, ¶m2, desc, sizeof(desc)); text_color_set (DW_COLOR_DEBUG); - dw_printf ("%s\n", desc); + dw_printf ("%d: %s\n", __LINE__, desc); + fflush (stdout); + SLEEP_SEC (1); text_color_set (DW_COLOR_ERROR); assert (param2.full_duplex == 0); - assert (param2.rej == selective_reject); + assert (param2.srej == srej_single); assert (param2.modulo == modulo_8); assert (param2.i_field_length_rx == 61); assert (param2.window_size_rx == 4); @@ -652,26 +744,57 @@ int main (int argc, char *argv[]) { assert (param2.retries == 9); +/* Other values, multi srej. */ + + param.full_duplex = 0; + param.srej = srej_multi; + param.modulo = modulo_128; + param.i_field_length_rx = 61; + param.window_size_rx = 4; + param.ack_timer = 5555; + param.retries = 9; + + n = xid_encode (¶m, info, cr_cmd); + n = xid_parse (info, n, ¶m2, desc, sizeof(desc)); + + text_color_set (DW_COLOR_DEBUG); + dw_printf ("%d: %s\n", __LINE__, desc); + fflush (stdout); + SLEEP_SEC (1); + + text_color_set (DW_COLOR_ERROR); + + assert (param2.full_duplex == 0); + assert (param2.srej == srej_multi); + assert (param2.modulo == modulo_128); + assert (param2.i_field_length_rx == 61); + assert (param2.window_size_rx == 4); + assert (param2.ack_timer == 5555); + assert (param2.retries == 9); + + /* Specify some and not others. */ param.full_duplex = 0; - param.rej = selective_reject; + param.srej = srej_single; param.modulo = modulo_8; param.i_field_length_rx = G_UNKNOWN; param.window_size_rx = G_UNKNOWN; param.ack_timer = 999; param.retries = G_UNKNOWN; - n = xid_encode (¶m, info); + n = xid_encode (¶m, info, cr_cmd); n = xid_parse (info, n, ¶m2, desc, sizeof(desc)); text_color_set (DW_COLOR_DEBUG); - dw_printf ("%s\n", desc); + dw_printf ("%d: %s\n", __LINE__, desc); + fflush (stdout); + SLEEP_SEC (1); text_color_set (DW_COLOR_ERROR); assert (param2.full_duplex == 0); - assert (param2.rej == selective_reject); + assert (param2.srej == srej_single); assert (param2.modulo == modulo_8); assert (param2.i_field_length_rx == G_UNKNOWN); assert (param2.window_size_rx == G_UNKNOWN); @@ -684,12 +807,14 @@ int main (int argc, char *argv[]) { n = xid_parse (info, n, ¶m2, desc, sizeof(desc)); text_color_set (DW_COLOR_DEBUG); - dw_printf ("%s\n", desc); + dw_printf ("%d: %s\n", __LINE__, desc); + fflush (stdout); + SLEEP_SEC (1); text_color_set (DW_COLOR_ERROR); assert (param2.full_duplex == G_UNKNOWN); - assert (param2.rej == unknown_reject); + assert (param2.srej == srej_not_specified); assert (param2.modulo == modulo_unknown); assert (param2.i_field_length_rx == G_UNKNOWN); assert (param2.window_size_rx == G_UNKNOWN); diff --git a/xid.h b/xid.h index c94003e..a221b73 100644 --- a/xid.h +++ b/xid.h @@ -2,6 +2,7 @@ /* xid.h */ + #include "ax25_pad.h" // for enum ax25_modulo_e @@ -9,10 +10,10 @@ struct xid_param_s { int full_duplex; - // Order is important because negotiation keeps the lower value. - // We will support only 1 & 2. + // Order is important because negotiation keeps the lower value of + // REJ (srej_none), SREJ (default without negotiation), Multi-SREJ (if both agree). - enum rej_e {unknown_reject=0, implicit_reject=1, selective_reject=2, selective_reject_reject=3 } rej; + enum srej_e { srej_none=0, srej_single=1, srej_multi=2, srej_not_specified=3 } srej; enum ax25_modulo_e modulo; @@ -28,4 +29,4 @@ struct xid_param_s { int xid_parse (unsigned char *info, int info_len, struct xid_param_s *result, char *desc, int desc_size); -int xid_encode (struct xid_param_s *param, unsigned char *info); \ No newline at end of file +int xid_encode (struct xid_param_s *param, unsigned char *info, cmdres_t cr); \ No newline at end of file