This was a problem with open5gs and more than one PDU session.
The core network sets UE AMBR only once, leading to failure in the gNB
for the second PDU session.
(A proper handling of AMBR has to be develop at some point in the gNB.)
- remove hardcoded values
- adapt gNB RRC code
- adapt gNB MAC code
- adapt physical simulators, where we put the hardcoded values
that were everywhere else before; if there is a need for those
simulators to get configurable values, more work is needed
(but it's doubtful that it is needed)
To be used in a later commit to retrieve NR PDCP configuration
parameters.
To use, add the following to the configuration file (outside of other
configurations). Adapt the values to your setup. The names ("len18bits",
"ms100", etc.) come from 3GPP TS 38.331.
pdcp = {
drb = {
sn_size = "len18bits"
t_reordering = "ms100"
discard_timer = "infinity"
}
}
Default values are applied if the configuration is absent.
The default values are the ones given above.
To be used in a later commit to retrieve NR RLC configuration
parameters.
To use, add the following to the configuration file (outside of other
configurations). Adapt the values to your setup. The names ("ms45",
"size12", etc.) come from 3GPP TS 38.331.
rlc = {
srb = {
t_poll_retransmit = "ms45"
t_reassembly = "ms25"
t_status_prohibit = "ms0"
poll_pdu = "infinity"
poll_byte = "infinity"
max_retx_threshold = "t8"
sn_field_length = "size12"
}
drb_am = {
t_poll_retransmit = "ms45"
t_reassembly = "ms15"
t_status_prohibit = "ms15"
poll_pdu = "p64"
poll_byte = "kB500"
max_retx_threshold = "t32"
sn_field_length = "size18"
}
drb_um = {
t_reassembly = "ms15"
sn_field_length = "size12"
}
}
Default values are applied if the configuration is absent.
The default values are the ones given above.
When data is generated by "normal" T tracers, it should be fine.
But it might come from unknown sources and then content might be
"strange". So let's validate input data a bit more.
The way allocation is done (granularity of 64KiB) makes a crash more
difficult to achieve, but if a trace is exactly 64KiB, we miss 4 bytes
in the realloc and then anything may happen.
In NR with several layers, we may have MAC PDU of size bigger than 64KB,
which does not fit into a UDP buffer, destroying macpdu2wireshark.
Let's add an option to dump to a pcap file instead of using UDP packets.
The DLT 252 magic is used (check wireshark documentation to understand).
Run macpdu2wireshark with -h to see how to configure wireshark to read
the produced files.
The following error was appearing when running LTE UE:
inet_pton(): cannot convert -64.-74.0.2 to IPv4 network address
The allocated address by the core actually is 192.182.0.2.
On my host "char" is signed, leading to the above error (192 and 182
are seen as signed).
Let's force "ip_addr" unsigned and everyone is happy.
In Bearer Context Modification Request, the security information
field is needed only if we change the PDCP algorithms/keys. Since we
don't, no need to pass it.
Let's keep the old code, maybe the current understanding of the
specifications is wrong.
If the Bearer Context Modification Request does not contain security
information, the PDCP entity shall keep the currently configured security
settings.
Passing -1 for ciphering_algorithm and integrity_algorithm is the way
to implement this logic.
Check if the IE is memory allocated to deal with presence/absence of this information
element in Bearer Context Modification Request and add encoding/decoding
in the code.
Note that for decoding the integrity settings may be absent, in which
case we consider NIA0 to be used (no integrity protection). To be refined
if needed.
And for encoding, we always encode integrity settings, even if it's NIA0.
May also be refined if needed.
This was not a big problem, but we were advertising wrong
algorithm and deriving keys using this wrong algorithm in case
DRB ciphering and/or integrity was not active. (Say SRBs
are configured with NIA2 but DRBs are configured without
integrity, we would advertise NIA2 and send the integrity
key derived for NIA2, instead of NIA0 for both.)
It was not a problem because in the PDU Session Resource To Setup
item, there is "securityIndication" which we use to effectively
activate/deactivate the ciphering and/or integrity.
But it did not look clean to see a SecurityInformation with
incorrect data when inspecting the message in wireshark.
So let's use the correct values.
We could also not include the SecurityInformation if both ciphering
and integrity are not used, and only include ciphering if integrity
is not used (because integrity settings are optional).
But then if only integrity is used, we still need to include
ciphering, setting algorithm to NEA0 (no ciphering), because
the ciphering settings are always included.
This logic is too complex, let's use the simple one to always
include SecurityInformation with NEA0 and/or NIA0 if ciphering
and/or integrity is not activated.
Useful to debug realtime issues, for example when calling a periodical
routine, if it's not programmed carefully enough, it may be that the first
call takes way longer than the next ones (for example it uses some memory
that is not mapped in the process, leading to many page faults the first
time it's executed).
Having a periodic log for the timing of this routine with min/avg/max/count
will reveal that a call is way longer than average and will help in
finding where exactly in the code the time is spent, thanks to the ease
of use of the T tracer; you put T(T_XX, T_INT(1)) just before the code
you want to measure, you put T(T_XX, T_INT(0)) just after and time_meas
will show the delta time between those events. Then you cut the routine
in smaller pieces, measure with T(1)/T(0), and you finally reach the small
guilty part that takes too long.
In time_meas.c, when getting 0 for the event, we log delta time with
previous 1 for the event. "start_valid" was not cleared, so receiving
a second 0 without 1 in between may lead to wrong logging.
Should not happen in practice, but still, little bug.
(Note: we don't protect against streams of 1, but this case should also
not happen.)
MIB, SIB1, random access and regular scheduling are traced.
At the beginning of a connection (rrc setup request and rrc setup), the
RNTI is reported as 0. Might be fixed, somehow, if absolutely needed (not
sure).
./run_tests.sh was not functional anymore because of the introduction
of AssertFatal() in nr_rlc_entity.c
Including the .h and providing exit_function() solves the problem.
Some people needed that.
Let's stop to 19 for the time being. It's tricky enough.
We can increase later if needed. (A previous commit
made the tracers to accept up to 32 variables.)
T_MAX_ARGS was 16 and some people start to use the T machinery with
more than 16 values. So be it. Let tracers be friendly and accept 32.
(More commits to follow for full support.)
Also why is T_MAX_ARGS in T_defs.h when it's only used by event.h?
Hum?
It is very possible that the operating system returns a file
descriptor bigger than the size of bridge->buf (MAX_FD_RFSIMU, which
is 250 as of now). As a minimum, let's fail if the socket file
descriptor is out of bounds.
A proper solution may be implemented later, if needed.
Integration: `2024.w33`
See merge request oai/openairinterface5g!2911
- !2784 Reworking beam structures at NR MAC gNB
- !2889 Fix delta for PUSCH DMRS type 2
- !2908 NR MAC UE CSI improvements
- !2896 Fix for llr functions reading/writing outside buffers
- !2902 nr rlc: reduce "SDU rejected" logging
- !2884 NR UE: Cleanup global variables used for command line parameter storage
- !2910 NR gNB DMRS symbol race
- CI: Update phytest-timing test threshold
- make it work for SRB or DRB
- adapt callers of this function
- cleanup key derivation: the User Plane keys are generated at some places
but are not used, remove the generation
Compiling the nrUE with -fsanitize=undefined leads to a crash
because the function nas_stream_encrypt_eia1() accesses the data
as uint32_t but the data is not aligned.
Actually this function wants 64 bits (big-endian) of data, so let's
introduce a simple function to read a big-endian 64 bits value.
It may seem bad (unoptimized), but looking at the output of gcc
without -fsanitize=undefined this will be translated as just one
movbeq instruction (on the machine where I did the test), which is
actually less instructions than the previous version which was doing
too movbel plus orq.
Moreover, after the main loop we need to process the last remaining
bytes (so less than 8). The code was wrong, reading past the input
data (and also not caring about alignment). It may work, it may fail,
it depends on many things. But it's plain wrong. So this was replaced
for something better, simpler. No need for mask32bit() anymore, only
one call to U64() is needed.
And now -fsanitize=undefined is happy, so all is good.
We also restrict the input length to be multiple of 8 bits. I don't
think it's a problem. To be refined if I'm wrong. (I think RRC and
NAS messages are 8-bits aligned, so it shouldn't be a problem.)
It should not be > 0xffffff.
Normally the core network would prevent this case from happening.
Plus to reach this value means to send a lot of NAS messages, which
is very doubtful in practice.
Anyway, it's better to deal with the case.
We exit if that happens. To be refined if needed (very doubtful).
This function was very wrong, now it's just wrong.
(It does not sanitize the input properly. Some bad input may have
unexpected effects on the nrUE.)
It needs a serious rewrite.
I just wanted to "fix" it so that it processes correctly some
correct input data.
OAI core network is sending Deregistation Accept in clear, even
after security context activation.
The standards is not clear to me, so let's accept it.
To be removed if incorrect with respect to the standards.
(The core network will need to be fixed then.)
The version before this commit is wrong, taking wrong byte for msg_type
in some cases.
I don't have much time to implement a proper NAS PDU parser, so let's go
with hardcoded values.
Some later work will be done to have a cleaner solution. As of now, it's
too much work.
Get the security algorithms from SecurityModeCommand.
Restructure the code a bit: add the function handle_security_mode_command()
and adapt the code to use it.
Derive also knas_enc (not used yet).
Do the derivation for knas_enc and knas_int only after reception
of SecurityModeCommand. We need the algorithms configured by core
network to derive those keys.
The starting point was the function do_NR_DLInformationTransfer() which
may deal with more than 256 bytes.
Then why not cleanup all the functions in the same file.
(And do a bit of formattting fixing while we're here.)
Then cleanup the callers of those functions.
Then, because of do_SIB23_NR() let's also cleanup in some
structures.
Not all the codebase was cleaned up, there surely remain some uint8_t
(and char/short/uint16_t) which are wrong.
By using a struct we may have warnings at compilation time in some
cases if the type is not used properly.
The type definition becomes a bit weird, so we need to properly
comment.
If too strange, we can revert to the classical "void" definition.
- rename "lib_ctx" to "mac_implementation" which is what is stored because
of previous commit
- rename "mac" to "mac_context" to be clearer with what is stored in this
variable too
The main work of this commit is to rewrite cipher_aes_128_cbc_cmac()
which was too slow and created issues when running the 4G eNB on some
machines (LLL when using a b210).
Instead of creating the security context, use it, and delete it, all in
cipher_aes_128_cbc_cmac(), we introduce the function init_aes_128_cbc_cmac()
to create it, and we change cipher_aes_128_cbc_cmac() to just use it,
and we also adapt free_aes_128_cbc_cmac(). The function aes_128_cbc_cmac()
is removed, it does not follow the new init/use/free API.
Doing so, on the primary test machine used to do this work (a powerful
and fast machine where it is possible to set the CPUs' frequencies)
it was possible to limit the CPUs' frequencies to 500 MHz (it was not
possible to go lower) without any LLL (using a b210, quectel UE in
idle, no data traffic, only periodic measurement reports). Before this
commit, it was necessary to set a frequency of at least 1.8 GHz
otherwise some LLL would appear.
Then we need to adapt the rest of the code to take into account those
changes.
A notion of security container (stream_security_container_t) containing
ciphering and integrity contexts is introduced.
And the code is harmonized for, hopefully, better readability.
The (opaque) type stream_security_context_t is introduced, used by
both integrity and ciphering contexts. The idea is to use a
init()/use()/free() API for all the security procedures, identical for
all the ciphering and integrity procedures.
Introduce the structure nr_pdcp_integrity_data_t and adapt code to use it.
Note: in nr_pdcp_sdu_t we keep the 'count' variable (which is also present
in nr_pdcp_integrity_data_t). They represent the same value, but 'count'
in nr_pdcp_integrity_data_t is to be specifically used for integrity while
the other one has other uses, so it's better to keep it.
Cedric Roux