-
Notifications
You must be signed in to change notification settings - Fork 8k
modem: cmux: Handle C/R bit from address field #96617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
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"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Received a response frame? Shouldn't this rather say that the received C/R bit is wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not wrong. See 3GPP TS 27.010: 5.2.1.2 Address Field
The value of the C/R bit changes its meaning depending on which side send the packet.
Most implementations don't care, but for certain commands where the response is exact copy of the command, we should be able to distinct if we just received a response to our command or was this a new command.
In this PR, I'm not doing other than dropping unexpected frames, but on my follow up PR that I'm working, I use this information to know that our Power Saving Command is actually replied and we can shut down the UART.
Same with Close Down command.
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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this rather be
if (dlci->cmux->frame.cr != dlci->cmux->initiator) { | |
if (dlci->cmux->frame.cr == dlci->cmux->initiator) { |
as we are receiving the frame?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the image on my previous comment.
The value depends on which side of the CMUX pipe we are. So are we initiator or not.
105952e
to
e09525a
Compare
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]>
e09525a
to
a4bb1eb
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test case for this?
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.
See the table from given section of the specification.
Also, on UIH frames 5.4.3.1 says
NOTE: This is different than a C/R bit in the Type field.