fixeria has submitted this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35639?usp=email )

Change subject: isdn/v110_ta: avoid redundant .status_update_cb() calls
......................................................................

isdn/v110_ta: avoid redundant .status_update_cb() calls

Let's be smarter and call the status update callback iff the V.24
flagmask was actually changed.

Change-Id: I9626d3e737d4e072fa163115c4cdf9ee6ee0968e
Related: OS#4396
---
M src/isdn/v110_ta.c
M tests/v110/ta_test.err
2 files changed, 44 insertions(+), 36 deletions(-)

Approvals:
  osmith: Looks good to me, approved
  Jenkins Builder: Verified
  pespin: Looks good to me, but someone else must approve




diff --git a/src/isdn/v110_ta.c b/src/isdn/v110_ta.c
index 7757dcc..6730b20 100644
--- a/src/isdn/v110_ta.c
+++ b/src/isdn/v110_ta.c
@@ -247,12 +247,15 @@
        }
 }

-static void v110_ta_flags_updated(const struct osmo_v110_ta *ta)
+static void v110_ta_flags_update(struct osmo_v110_ta *ta, unsigned int 
v24_flags)
 {
-       const struct osmo_v110_ta_cfg *cfg = ta->cfg;
+       struct osmo_v110_ta_cfg *cfg = ta->cfg;

+       if (ta->state.v24_flags == v24_flags)
+               return;
        if (cfg->status_update_cb != NULL)
-               cfg->status_update_cb(cfg->priv, ta->state.v24_flags);
+               cfg->status_update_cb(cfg->priv, v24_flags);
+       ta->state.v24_flags = v24_flags;
 }

 static const struct osmo_tdef_state_timeout v110_ta_fsm_timeouts[32] = {
@@ -272,6 +275,7 @@
 {
        struct osmo_v110_ta *ta = (struct osmo_v110_ta *)fi->priv;
        struct v110_ta_state *ts = &ta->state;
+       unsigned int v24_flags = ta->state.v24_flags;

        /* 7.1.1.2 During the idle (or ready) state the TA will transmit 
continuous binary 1s into the B-channel */
        ts->tx.d_bit_mode = V110_TA_DBIT_M_ALL_ONE; /* circuit 103: continuous 
binary '1' */
@@ -282,10 +286,10 @@
        /* - circuit 104: continuous binary '1' */
        ts->rx.d_bit_mode = V110_TA_DBIT_M_ALL_ONE;
        /* - circuits 107, 106, 109 = OFF */
-       V24_FLAGMASK_SET_OFF(ts->v24_flags, OSMO_V110_TA_C_106);
-       V24_FLAGMASK_SET_OFF(ts->v24_flags, OSMO_V110_TA_C_107);
-       V24_FLAGMASK_SET_OFF(ts->v24_flags, OSMO_V110_TA_C_109);
-       v110_ta_flags_updated(ta);
+       V24_FLAGMASK_SET_OFF(v24_flags, OSMO_V110_TA_C_106);
+       V24_FLAGMASK_SET_OFF(v24_flags, OSMO_V110_TA_C_107);
+       V24_FLAGMASK_SET_OFF(v24_flags, OSMO_V110_TA_C_109);
+       v110_ta_flags_update(ta, v24_flags);
 }

 /* ITU-T V.110 Section 7.1.1 */
@@ -354,24 +358,25 @@
        case V110_TA_EV_RX_FRAME_IND:
        {
                const struct osmo_v110_decoded_frame *df = data;
+               unsigned int v24_flags = ta->state.v24_flags;

                /* 7.1.2.4 When the receiver recognizes that the status of bits 
S and X are ON */
                if (v110_df_s_bits_are(df, V110_SX_BIT_ON) &&
                    v110_df_x_bits_are(df, V110_SX_BIT_ON)) {
                        /* ... it will perform the following functions: */
                        /*  a) Turn ON circuit 107 toward the DTE and stop 
timer T1 */
-                       V24_FLAGMASK_SET_ON(ts->v24_flags, OSMO_V110_TA_C_107);
+                       V24_FLAGMASK_SET_ON(v24_flags, OSMO_V110_TA_C_107);
                        osmo_timer_del(&fi->timer);
                        /*  b) Then, circuit 103 may be connected to the data 
bits in the frame; however, the
                         *  DTE must maintain a binary 1 condition on circuit 
103 until circuit 106 is turned
                         *  ON in the next portion of the sequence. */
                        /*  c) Turn ON circuit 109 and connect the data bits to 
circuit 104. */
-                       V24_FLAGMASK_SET_ON(ts->v24_flags, OSMO_V110_TA_C_109);
+                       V24_FLAGMASK_SET_ON(v24_flags, OSMO_V110_TA_C_109);
                        ts->rx.d_bit_mode = V110_TA_DBIT_M_FORWARD;
                        /*  d) After an interval of N bits (see 6.3), it will 
turn ON circuit 106. */
-                       V24_FLAGMASK_SET_ON(ts->v24_flags, OSMO_V110_TA_C_106);
+                       V24_FLAGMASK_SET_ON(v24_flags, OSMO_V110_TA_C_106);
                        ts->tx.d_bit_mode = V110_TA_DBIT_M_FORWARD;
-                       v110_ta_flags_updated(ta);
+                       v110_ta_flags_update(ta, v24_flags);
                        /*  Circuit 106 transitioning from OFF to ON will cause 
the transmitted data to
                         *  transition from binary 1 to the data mode. */
                        v110_ta_fsm_state_chg(V110_TA_ST_DATA_TRANSFER);
@@ -396,13 +401,14 @@
 {
        struct osmo_v110_ta *ta = (struct osmo_v110_ta *)fi->priv;
        struct v110_ta_state *ts = &ta->state;
+       unsigned int v24_flags = ta->state.v24_flags;

        /* 7.1.3.1 While in the data transfer state, the following circuit 
conditions exist:
         *  a): 105, 107, 108, and 109 are in the ON condition */
        /* XXX: OSMO_ASSERT(V24_FLAGMASK_IS_ON(ts->v24_flags, 
OSMO_V110_TA_C_105)); */
-       V24_FLAGMASK_SET_ON(ts->v24_flags, OSMO_V110_TA_C_107);
+       V24_FLAGMASK_SET_ON(v24_flags, OSMO_V110_TA_C_107);
        /* XXX: OSMO_ASSERT(V24_FLAGMASK_IS_ON(ts->v24_flags, 
OSMO_V110_TA_C_108)); */
-       V24_FLAGMASK_SET_ON(ts->v24_flags, OSMO_V110_TA_C_109);
+       V24_FLAGMASK_SET_ON(v24_flags, OSMO_V110_TA_C_109);
        /*  b) data is being transmitted on circuit 103 and received on circuit 
104 */
        ts->rx.d_bit_mode = V110_TA_DBIT_M_FORWARD;
        ts->tx.d_bit_mode = V110_TA_DBIT_M_FORWARD;
@@ -410,9 +416,9 @@
         *  flow control is being used, either or both circuits may be in the 
ON or the OFF condition. */
        if (!ta->cfg->flow_ctrl.end_to_end) {
                /* XXX: OSMO_ASSERT(V24_FLAGMASK_IS_ON(ts->v24_flags, 
OSMO_V110_TA_C_133)); */
-               V24_FLAGMASK_SET_ON(ts->v24_flags, OSMO_V110_TA_C_106);
+               V24_FLAGMASK_SET_ON(v24_flags, OSMO_V110_TA_C_106);
        }
-       v110_ta_flags_updated(ta);
+       v110_ta_flags_update(ta, v24_flags);

        /* 7.1.3.2 While in the data transfer state, the following status bit 
conditions exist: */
        /*  a) status bits S in both directions are in the ON condition; */
@@ -453,14 +459,15 @@
        case V110_TA_EV_RX_FRAME_IND:
        {
                const struct osmo_v110_decoded_frame *df = data;
+               unsigned int v24_flags = ta->state.v24_flags;

                /* 7.1.4.2 ... this TA will recognize the transition of the 
status bits S from
                 * ON to OFF and the data bits from data to binary 0 as a 
disconnect request */
                if (v110_df_s_bits_are(df, V110_SX_BIT_OFF) && 
v110_df_d_bits_are(df, 0)) {
                        /* ... and it will turn OFF circuits 107 and 109. */
-                       V24_FLAGMASK_SET_OFF(ts->v24_flags, OSMO_V110_TA_C_107);
-                       V24_FLAGMASK_SET_OFF(ts->v24_flags, OSMO_V110_TA_C_109);
-                       v110_ta_flags_updated(ta);
+                       V24_FLAGMASK_SET_OFF(v24_flags, OSMO_V110_TA_C_107);
+                       V24_FLAGMASK_SET_OFF(v24_flags, OSMO_V110_TA_C_109);
+                       v110_ta_flags_update(ta, v24_flags);
                        /* DTE should respond by turning OFF circuit 108 */
                        break; /* XXX: shall we forward D-bits to DTE anyway? */
                }
@@ -478,14 +485,15 @@
 {
        struct osmo_v110_ta *ta = (struct osmo_v110_ta *)fi->priv;
        struct v110_ta_state *ts = &ta->state;
+       unsigned int v24_flags = ta->state.v24_flags;

        /* 7.1.4.1 At the completion of the data transfer phase, the local DTE 
will indicate a
         * disconnect request by turning OFF circuit 108. This will cause the 
following to occur: */
        /*  a) the status bits S in the frame toward ISDN will turn OFF, status 
bits X are kept ON */
        ts->tx.s_bits = V110_SX_BIT_OFF;
        /*  b) circuit 106 will be turned OFF */
-       V24_FLAGMASK_SET_OFF(ts->v24_flags, OSMO_V110_TA_C_106);
-       v110_ta_flags_updated(ta);
+       V24_FLAGMASK_SET_OFF(v24_flags, OSMO_V110_TA_C_106);
+       v110_ta_flags_update(ta, v24_flags);
        /*  c) the data bits in the frame will be set to binary 0. */
        ts->tx.d_bit_mode = V110_TA_DBIT_M_ALL_ZERO;

@@ -554,6 +562,7 @@
 {
        struct osmo_v110_ta *ta = (struct osmo_v110_ta *)fi->priv;
        struct v110_ta_state *ts = &ta->state;
+       unsigned int v24_flags = ta->state.v24_flags;

        switch (event) {
        case V110_TA_EV_V24_STATUS_CHG:
@@ -575,8 +584,8 @@
                ts->tx.x_bits = V110_SX_BIT_OFF;
                ts->tx.d_bit_mode = V110_TA_DBIT_M_ALL_ZERO;
                /* TODO: actually Tx those frames (delay state transition) */
-               V24_FLAGMASK_SET_OFF(ts->v24_flags, OSMO_V110_TA_C_107);
-               v110_ta_flags_updated(ta);
+               V24_FLAGMASK_SET_OFF(v24_flags, OSMO_V110_TA_C_107);
+               v110_ta_flags_update(ta, v24_flags);
                v110_ta_fsm_state_chg(V110_TA_ST_DISCONNECTING);
                break;
        default:
diff --git a/tests/v110/ta_test.err b/tests/v110/ta_test.err
index 347f8f6..e8f80e6 100644
--- a/tests/v110/ta_test.err
+++ b/tests/v110/ta_test.err
@@ -2,7 +2,6 @@
 ==== Running test_idle_ready()
 DLGLOBAL DEBUG V110-TA(test_idle_ready){IDLE_READY}: Allocated
 DLGLOBAL DEBUG V110-TA(test_idle_ready){IDLE_READY}: State change to 
IDLE_READY (no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 Initial status: 0x00000000
 circuit 106/CTS (Clear to Send) is OFF (expected to be OFF)
 circuit 107/DSR (Data Set Ready) is OFF (expected to be OFF)
@@ -29,7 +28,6 @@
 setting circuit 108/DTR (Data Terminal Ready) OFF
 DLGLOBAL DEBUG V110-TA(test_idle_ready){CONNECT_TA_TO_LINE}: Received Event 
V24_STATUS_CHG
 DLGLOBAL DEBUG V110-TA(test_idle_ready){CONNECT_TA_TO_LINE}: State change to 
IDLE_READY (no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 osmo_v110_ta_set_circuit() returns 0
 setting circuit 108/DTR (Data Terminal Ready) ON
 DLGLOBAL DEBUG V110-TA(test_idle_ready){IDLE_READY}: Received Event 
V24_STATUS_CHG
@@ -40,7 +38,6 @@
 ==== Running test_conn_ta_line()
 DLGLOBAL DEBUG V110-TA(test_conn_ta_line){IDLE_READY}: Allocated
 DLGLOBAL DEBUG V110-TA(test_conn_ta_line){IDLE_READY}: State change to 
IDLE_READY (no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 setting circuit 108/DTR (Data Terminal Ready) ON
 DLGLOBAL DEBUG V110-TA(test_conn_ta_line){IDLE_READY}: Received Event 
V24_STATUS_CHG
 DLGLOBAL DEBUG V110-TA(test_conn_ta_line){IDLE_READY}: State change to 
CONNECT_TA_TO_LINE (T1, 10s)
@@ -79,7 +76,6 @@
 DLGLOBAL DEBUG V110-TA(test_conn_ta_line){CONNECT_TA_TO_LINE}: Received Event 
RX_FRAME_IND
 v110_ta_test_status_update_cb(status=0x0000001e)
 DLGLOBAL DEBUG V110-TA(test_conn_ta_line){CONNECT_TA_TO_LINE}: State change to 
DATA_TRANSFER (no timeout)
-v110_ta_test_status_update_cb(status=0x0000001e)
 v110_ta_test_rx_cb(buf_size=48): 
010101010101010101010101010101010101010101010101
 osmo_v110_ta_frame_in() returns 0
 DLGLOBAL DEBUG V110-TA(test_conn_ta_line){DATA_TRANSFER}: Deallocated
@@ -87,7 +83,6 @@
 ==== Running test_data_transfer()
 DLGLOBAL DEBUG V110-TA(test_data_transfer){IDLE_READY}: Allocated
 DLGLOBAL DEBUG V110-TA(test_data_transfer){IDLE_READY}: State change to 
IDLE_READY (no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 setting circuit 108/DTR (Data Terminal Ready) ON
 DLGLOBAL DEBUG V110-TA(test_data_transfer){IDLE_READY}: Received Event 
V24_STATUS_CHG
 DLGLOBAL DEBUG V110-TA(test_data_transfer){IDLE_READY}: State change to 
CONNECT_TA_TO_LINE (T1, 10s)
@@ -102,7 +97,6 @@
 DLGLOBAL DEBUG V110-TA(test_data_transfer){CONNECT_TA_TO_LINE}: Received Event 
RX_FRAME_IND
 v110_ta_test_status_update_cb(status=0x0000001e)
 DLGLOBAL DEBUG V110-TA(test_data_transfer){CONNECT_TA_TO_LINE}: State change 
to DATA_TRANSFER (no timeout)
-v110_ta_test_status_update_cb(status=0x0000001e)
 v110_ta_test_rx_cb(buf_size=48): 
010101010101010101010101010101010101010101010101
 osmo_v110_ta_frame_in() returns 0
 circuit 106/CTS (Clear to Send) is ON (expected to be ON)
@@ -127,7 +121,6 @@
 ==== Running test_data_transfer_disc_local()
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_local){IDLE_READY}: Allocated
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_local){IDLE_READY}: State 
change to IDLE_READY (no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 setting circuit 108/DTR (Data Terminal Ready) ON
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_local){IDLE_READY}: Received 
Event V24_STATUS_CHG
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_local){IDLE_READY}: State 
change to CONNECT_TA_TO_LINE (T1, 10s)
@@ -142,7 +135,6 @@
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_local){CONNECT_TA_TO_LINE}: 
Received Event RX_FRAME_IND
 v110_ta_test_status_update_cb(status=0x0000001e)
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_local){CONNECT_TA_TO_LINE}: 
State change to DATA_TRANSFER (no timeout)
-v110_ta_test_status_update_cb(status=0x0000001e)
 v110_ta_test_rx_cb(buf_size=48): 
010101010101010101010101010101010101010101010101
 osmo_v110_ta_frame_in() returns 0
 local TE initiates disconnection
@@ -189,7 +181,6 @@
 ==== Running test_data_transfer_disc_remote()
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){IDLE_READY}: Allocated
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){IDLE_READY}: State 
change to IDLE_READY (no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 setting circuit 108/DTR (Data Terminal Ready) ON
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){IDLE_READY}: Received 
Event V24_STATUS_CHG
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){IDLE_READY}: State 
change to CONNECT_TA_TO_LINE (T1, 10s)
@@ -204,7 +195,6 @@
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){CONNECT_TA_TO_LINE}: 
Received Event RX_FRAME_IND
 v110_ta_test_status_update_cb(status=0x0000001e)
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){CONNECT_TA_TO_LINE}: 
State change to DATA_TRANSFER (no timeout)
-v110_ta_test_status_update_cb(status=0x0000001e)
 v110_ta_test_rx_cb(buf_size=48): 
010101010101010101010101010101010101010101010101
 osmo_v110_ta_frame_in() returns 0
 remote TE initiates disconnection
@@ -227,7 +217,6 @@
 osmo_v110_ta_set_circuit() returns 0
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){DISCONNECTING}: 
Received Event DESYNC_IND
 DLGLOBAL DEBUG V110-TA(test_data_transfer_disc_remote){DISCONNECTING}: State 
change to IDLE_READY (no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 circuit 106/CTS (Clear to Send) is OFF (expected to be OFF)
 circuit 107/DSR (Data Set Ready) is OFF (expected to be OFF)
 circuit 109/DCD (Data Carrier Detect) is OFF (expected to be OFF)
@@ -236,7 +225,6 @@
 ==== Running test_syncing()
 DLGLOBAL DEBUG V110-TA(test_syncing){IDLE_READY}: Allocated
 DLGLOBAL DEBUG V110-TA(test_syncing){IDLE_READY}: State change to IDLE_READY 
(no timeout)
-v110_ta_test_status_update_cb(status=0x00000000)
 setting circuit 108/DTR (Data Terminal Ready) ON
 DLGLOBAL DEBUG V110-TA(test_syncing){IDLE_READY}: Received Event V24_STATUS_CHG
 DLGLOBAL DEBUG V110-TA(test_syncing){IDLE_READY}: State change to 
CONNECT_TA_TO_LINE (T1, 10s)
@@ -251,7 +239,6 @@
 DLGLOBAL DEBUG V110-TA(test_syncing){CONNECT_TA_TO_LINE}: Received Event 
RX_FRAME_IND
 v110_ta_test_status_update_cb(status=0x0000001e)
 DLGLOBAL DEBUG V110-TA(test_syncing){CONNECT_TA_TO_LINE}: State change to 
DATA_TRANSFER (no timeout)
-v110_ta_test_status_update_cb(status=0x0000001e)
 v110_ta_test_rx_cb(buf_size=48): 
010101010101010101010101010101010101010101010101
 osmo_v110_ta_frame_in() returns 0
 osmo_v110_ta_sync_ind(): the lower layer indicates out-of-sync event
@@ -270,7 +257,6 @@
 osmo_v110_ta_sync_ind(): the lower layer indicates sync event
 DLGLOBAL DEBUG V110-TA(test_syncing){RESYNCING}: Received Event SYNC_IND
 DLGLOBAL DEBUG V110-TA(test_syncing){RESYNCING}: State change to DATA_TRANSFER 
(no timeout)
-v110_ta_test_status_update_cb(status=0x0000001e)
 osmo_v110_ta_frame_out(): S-bits are expected to be 0 (ON)
 osmo_v110_ta_frame_out(): X-bits are expected to be 0 (ON)
 osmo_v110_ta_frame_out(): D-bits are to be set by .tx_cb()

--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35639?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I9626d3e737d4e072fa163115c4cdf9ee6ee0968e
Gerrit-Change-Number: 35639
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: osmith <osm...@sysmocom.de>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-MessageType: merged

Reply via email to