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

@@ -139,10 +139,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

@@ -451,11 +451,12 @@ 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;
 	}
 
 	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);
@@ -526,15 +527,15 @@ static void modem_cmux_connect_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;
 	}
 
+	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);
@@ -547,6 +548,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);
@@ -584,6 +590,12 @@ static struct modem_cmux_dlci *modem_cmux_find_dlci(struct modem_cmux *cmux)
 
 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:
 		dlci->state = MODEM_CMUX_DLCI_STATE_OPEN;
@@ -665,6 +677,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("Ignoring frame intended for unconfigured DLCI %u.",
@@ -1007,6 +1024,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,
@@ -1039,7 +1057,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,
@@ -1118,7 +1136,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,
@@ -1193,7 +1211,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,
@@ -1222,7 +1240,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/prj.conf

@@ -8,3 +8,4 @@ CONFIG_MODEM_CMUX=y
 CONFIG_MODEM_CMUX_MTU=64
 
 CONFIG_ZTEST=y
+CONFIG_MODEM_CMUX_LOG_LEVEL_DBG=y

tests/subsys/modem/modem_cmux_pair/prj.conf

@@ -8,3 +8,4 @@ CONFIG_MODEM_CMUX=y
 CONFIG_MODEM_CMUX_MTU=64
 
 CONFIG_ZTEST=y
+CONFIG_MODEM_CMUX_LOG_LEVEL_DBG=y