modem: cmux: Handle C/R bit from address field

The C/R bit in the address field should be handled as explained
in 5.2.1.2 in the spec (3GPP TS 127.010).

To detect if the frame is a command or a response,
we need to know who was the initiator of the CMUX channel.

>    Initiator is the station that take the initiative to initialize
>    the multiplexer (i.e. sends the SABM command at DLCI 0 )

See the table from given section of the specification.

Also, on UIH frames 5.4.3.1 says
>    The frames sent by the initiating station have the C/R bit set to 1
>    and those sent by the responding station have the C/R bit set to 0.

NOTE: This is different than a C/R bit in the Type field.

Signed-off-by: Seppo Takalo <[email protected]>

Author: SeppoTakalo

include/zephyr/modem/cmux.h

@@ -142,10 +142,11 @@ struct modem_cmux {
 
 	/* State */
 	enum modem_cmux_state state;
-	bool flow_control_on;
+	bool flow_control_on : 1;
+	bool initiator : 1;
 
 	/* Work lock */
-	bool attached;
+	bool attached : 1;
 	struct k_spinlock work_lock;
 
 	/* Receive state*/

subsys/modem/modem_cmux.c

@@ -461,12 +461,13 @@ static void modem_cmux_on_cld_command(struct modem_cmux *cmux, struct modem_cmux
 static void modem_cmux_on_control_frame_ua(struct modem_cmux *cmux)
 {
 	if (cmux->state != MODEM_CMUX_STATE_CONNECTING) {
-		LOG_DBG("Unexpected UA frame");
+		LOG_DBG("Unexpected UA frame in state %d", cmux->state);
 		return;
 	}
 
 	LOG_DBG("CMUX connected");
 	cmux->state = MODEM_CMUX_STATE_CONNECTED;
+	cmux->initiator = true;
 	k_mutex_lock(&cmux->transmit_rb_lock, K_FOREVER);
 	cmux->flow_control_on = true;
 	k_mutex_unlock(&cmux->transmit_rb_lock);
@@ -593,16 +594,16 @@ static void modem_cmux_dm_response_transmit(struct modem_cmux *cmux)
 
 static void modem_cmux_on_control_frame_sabm(struct modem_cmux *cmux)
 {
-	modem_cmux_connect_response_transmit(cmux);
-
 	if ((cmux->state == MODEM_CMUX_STATE_CONNECTED) ||
 	    (cmux->state == MODEM_CMUX_STATE_DISCONNECTING)) {
 		LOG_DBG("Connect request not accepted");
 		return;
 	}
 
 	LOG_DBG("CMUX connection request received");
+	cmux->initiator = false;
 	cmux->state = MODEM_CMUX_STATE_CONNECTED;
+	modem_cmux_connect_response_transmit(cmux);
 	k_mutex_lock(&cmux->transmit_rb_lock, K_FOREVER);
 	cmux->flow_control_on = true;
 	k_mutex_unlock(&cmux->transmit_rb_lock);
@@ -615,6 +616,11 @@ static void modem_cmux_on_control_frame(struct modem_cmux *cmux)
 {
 	modem_cmux_log_received_frame(&cmux->frame);
 
+	if (cmux->state == MODEM_CMUX_STATE_CONNECTED && cmux->frame.cr == cmux->initiator) {
+		LOG_DBG("Received a response frame, dropping");
+		return;
+	}
+
 	switch (cmux->frame.type) {
 	case MODEM_CMUX_FRAME_TYPE_UA:
 		modem_cmux_on_control_frame_ua(cmux);
@@ -659,6 +665,12 @@ static void modem_cmux_on_dlci_frame_dm(struct modem_cmux_dlci *dlci)
 
 static void modem_cmux_on_dlci_frame_ua(struct modem_cmux_dlci *dlci)
 {
+	/* Drop invalid UA frames */
+	if (dlci->cmux->frame.cr != dlci->cmux->initiator) {
+		LOG_DBG("Received a response frame, dropping");
+		return;
+	}
+
 	switch (dlci->state) {
 	case MODEM_CMUX_DLCI_STATE_OPENING:
 		LOG_DBG("DLCI %u opened", dlci->dlci_address);
@@ -744,6 +756,11 @@ static void modem_cmux_on_dlci_frame(struct modem_cmux *cmux)
 
 	modem_cmux_log_received_frame(&cmux->frame);
 
+	if (cmux->state != MODEM_CMUX_STATE_CONNECTED) {
+		LOG_DBG("Unexpected DLCI frame in state %d", cmux->state);
+		return;
+	}
+
 	dlci = modem_cmux_find_dlci(cmux);
 	if (dlci == NULL) {
 		LOG_WRN("Frame intended for unconfigured DLCI %u.",
@@ -1085,6 +1102,7 @@ static void modem_cmux_connect_handler(struct k_work *item)
 	cmux = CONTAINER_OF(dwork, struct modem_cmux, connect_work);
 
 	cmux->state = MODEM_CMUX_STATE_CONNECTING;
+	cmux->initiator = true;
 
 	static const struct modem_cmux_frame frame = {
 		.dlci_address = 0,
@@ -1117,7 +1135,7 @@ static void modem_cmux_disconnect_handler(struct k_work *item)
 
 	struct modem_cmux_frame frame = {
 		.dlci_address = 0,
-		.cr = true,
+		.cr = cmux->initiator,
 		.pf = false,
 		.type = MODEM_CMUX_FRAME_TYPE_UIH,
 		.data = data,
@@ -1196,7 +1214,7 @@ static int modem_cmux_dlci_pipe_api_transmit(void *data, const uint8_t *buf, siz
 
 		struct modem_cmux_frame frame = {
 			.dlci_address = dlci->dlci_address,
-			.cr = true,
+			.cr = cmux->initiator,
 			.pf = false,
 			.type = MODEM_CMUX_FRAME_TYPE_UIH,
 			.data = buf,
@@ -1271,7 +1289,7 @@ static void modem_cmux_dlci_open_handler(struct k_work *item)
 
 	struct modem_cmux_frame frame = {
 		.dlci_address = dlci->dlci_address,
-		.cr = true,
+		.cr = dlci->cmux->initiator,
 		.pf = true,
 		.type = MODEM_CMUX_FRAME_TYPE_SABM,
 		.data = NULL,
@@ -1300,7 +1318,7 @@ static void modem_cmux_dlci_close_handler(struct k_work *item)
 
 	struct modem_cmux_frame frame = {
 		.dlci_address = dlci->dlci_address,
-		.cr = true,
+		.cr = dlci->cmux->initiator,
 		.pf = true,
 		.type = MODEM_CMUX_FRAME_TYPE_DISC,
 		.data = NULL,

tests/subsys/modem/modem_cmux/src/main.c

@@ -886,4 +886,23 @@ ZTEST(modem_cmux, test_modem_cmux_split_large_data)
 		     "Incorrect number of bytes transmitted %d", ret);
 }
 
+ZTEST(modem_cmux, test_modem_cmux_invalid_cr)
+{
+	uint32_t events;
+	int ret;
+
+	/* We are initiator, so any CMD with CR set, should be dropped */
+	modem_backend_mock_put(&bus_mock, cmux_frame_control_cld_cmd,
+			       sizeof(cmux_frame_control_cld_cmd));
+
+	modem_backend_mock_put(&bus_mock, cmux_frame_control_sabm_cmd,
+			       sizeof(cmux_frame_control_sabm_cmd));
+
+
+	events = k_event_wait_all(&cmux_event, (MODEM_CMUX_EVENT_CONNECTED | MODEM_CMUX_EVENT_DISCONNECTED),
+				  false, K_MSEC(100));
+
+	zassert_false(events, "Wrong CMD should have been ignored");
+}
+
 ZTEST_SUITE(modem_cmux, NULL, test_modem_cmux_setup, test_modem_cmux_before, NULL, NULL);