It may happen that the drb to configure is not 1, so we may need
this array in rrc_gNB_generate_dedicatedRRCReconfiguration().
Not tested, this commit is mostly to harmonize with other places in
openair2/RRC/NR/rrc_gNB.c
This commit fixes a bug caused by global variables in the gNB_MAC_INST
structure used to transfer MAC scheduler's results to the PHY layer.
When using several threads in the PHY layer, it may happen that the
MAC scheduler is called a second time by the second PHY thread for the
next slot before the content of the global variables in the gNB_MAC_INST
structure used to store the result of the MAC scheduler for the current
slot have been consumed by the first thread, leading to unpredictable
results (all of them certainly wrong).
The solution is to introduce a small memory management module for the
interface between the PHY threads and the MAC scheduler. The MAC
scheduler now fills the data structures passed to it, and not a
global variable anymore. The PHY layer manages the structures and
ensures that no data is overwritten before being fully consumed by
the various threads of the PHY layer.
Some care has to be taken because several PHY threads will access the
same structures at the same time for a given slot (in read mode, so
it's okay). A mechanism of reference counting has thus been introduced.
Only one CC is supported. This restriction should not be too hard
to remove if/when needed. (The code before this commit does not seem
to work for more than one CC anyway.)
For some (unclear) reason, nr-softmodem with aw2s device does not print
"Bye." when stopping it with control+c, which is problematic in the CI.
This little hack solves the problem.
(Maybe not the right solution, to be changed if needed.)
The d array has to be cleared at the first usage, which may not necessarily
be when new_rx is true. For example, in case of DTX detected for a first
transmission, the next transmission won't have new_rx true, so d won't be
cleared, containing data from a previous transmission. Or when a LDPC
decoding is cancelled for a segment when new_rx is true, a decoding for
a retransmission will not clear d because then new_rx will not be true.
(Yes, the logic of cancellation will be changed in the future, but the
changes will be orthogonal to the ones of this commit.)
new_rx is removed.
The LOG_x() and T() logic was wrong.
LOG_x() goes to the terminal when T_stdout == 1 or T_stdout == 2.
It goes to T() when T_stdout == 0 or T_stdout == 2.
The call T() does the check for T_stdout == 0 or T_stdout == 2.
So we have to call it out of the check "if (T_stdout)".
So, well, let's fix that.
Some spacing was fixed as well. clang-format may complain.
On a setup, length of cellGroupConfig was > 255.
Using uint8_t to store the length is thus wrong
and gives a bad encoding of the cellGroupConfig.
So let's use uint32_t to store length instead of uint8_t.
Some problems when we release the device:
- Calling the destructors is wrong, we should set the pointers to NULL.
These are shared pointers, this is C++.
- We have to stop rx by doing a STREAM_MODE_STOP_CONTINUOUS.
- The end-of-burst for tx has been simplified.
This problem when we acquire the device:
- Calling get_rx_stream() many times seems to be problematic.
We now should have a better behavior with UHD 4.2 and N310. (Hopefully with
others too.) No crash seen so far.
There was a problem testing accelleran cu + oai du + oai ue + rfsim
because accelleran cu configures sdap header for UL but not for DL
and our sdap implementation deals with either "no sdap header at all"
or "sdap headers for both UL and DL".
The code has been modified to deal with the other cases
("sdap header for UL but not DL" and "sdap header for DL but not UL").
has_sdapDLheader and has_sdapULheader have been removed, has_sdap
too, replaced by has_sdap_rx and has_sdap_tx, which makes the code
independant of ue or gnb and is, I think, clearer.
With this work, accelleran cu + oai du + oai ue + rfsim works. We
also have oai gnb + oai ue + rfsim functional for all cases of sdap
header configuration (this won't happen for users, the code of gnb has to
be modified to have sdap header for only ul or only dl). It also works
with a COTS UE for all cases of sdap configuration (again, this won't
happen for users, only "no sdap header" and "sdap headers for both
UL and DL" exist in oai gnb with the current code).
To have other cases, you can edit openair2/RRC/NR/rrc_gNB_radio_bearers.c
and search for NR_SDAP_Config__sdap_HeaderDL_present in there.
To enable sdap, run the gnb with the command line argument
--gNBs.[0].enable_sdap 1
There was a problem in rrc_gNB_process_NGAP_PDUSESSION_RELEASE_COMMAND()
where we PROTOCOL_CTXT_SET_BY_INSTANCE() that does not initialize
eNB_index leading to a crash later on in pdcp. So let's initialize
everything.
Plus, while we're here, use do { } while(0) in the macros, just to avoid
problems like:
if (condition)
macro;
Without do { } while(0) only the first statement of the macro is done
in the if. (Not sure if the code has the problem somewhere, but doesn't
hurt to fix.)
There was a crash when we exit the nr softmodem with AW2S device.
The array rxbase[] was surely freed() by some other thread, so
let's not access it after detecting oai_exit != 0.
We also need to unblock a possible reader thread. The chosen way to
do may be not the best. To be checked.
Not only the teid is received from the DU but also the IP address. We
have to use it too.
Also, don't connect the UDP socket. The parameter remote_s_address in the
configuration file must not be used, at least not for gtp-u because the
IP address is received from the DU (plus for later, for multiple DUs, it
won't work at all).
- AssertFatal() in nr_rlc_srb0_recv_sdu() if the srb0 is not found
- do_RRCSetup() may return -1, handle it properly
- do_RRCReject() may return -1, handle it properly
- remove one switch in rrc_gNB_generate_RRCReject()
Using Accelleran CU, the DRB ID is 4 (not 1), we need to set and access
the array 'drb_priority' correctly in the DU specific functions.
Before this commit, we had the following error:
[NR_MAC] ASN1 message CellGroupConfig encoding failed (rlc-BearerToAddModList, 18446744073709551615)!
the following:
if (cur_wait_list == entity->wait_end)
end_wait_list = prev_wait_list;
was done after cur_wait_list is updated to its new value, but the old value
has to be used in this test. So end_wait_list was not set properly.
This was leading to random crashes later on because then entity->wait_list
was NULL but entity->wait_end was not (because of the bug fixed by this
commit).
The new value for cur_wait_list is also now set using prev_wait_list->next
just before the goto, as done in other places in the code.
Also "cur" was removed, for it has no purpose and makes the code harder
to understand.
- don't call exit() if get_message() fails, simply terminate the thread and
disable the T tracer
- T_dont_fork disappears, monitor_and_kill() as well, so now it's only 2
processes: the main (enb/gnb/ue) and the local tracer
Introduce the function nr_rlc_activate_avg_time_to_tx() to
activate/deactivate collection of avg time to tx stats (which
may have an impact on realtime processing, so better not compute
them when not needed).
Fix a little bug in time_average_get_average() for when there is not stat.
We reuse existing code from eia1 for actual computation. Maybe not the most
efficient. To be revised if needed.
Some hacks in CMakeLists.txt were introduced to compile
phy simulators. To be removed at some point.
This will probably impact negatively the Accelleran interoperability
but the previous version is not correct and breaks the oai nr ue.
To be checked in a CU/DU scenario with Accelleran.
Conflicts:
openair2/F1AP/f1ap_du_ue_context_management.c
openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_dlsch.c
openair2/RRC/NR/MESSAGES/asn1_msg.h
openair2/RRC/NR/rrc_gNB.c
openair3/ocp-gtpu/gtp_itf.cpp
There were some important conflicts in openair3/ocp-gtpu/gtp_itf.cpp
I rewrote a bit the function Gtpv1uHandleGpdu(). It might have been
done wrongly.
To be tested and fixed if needed.
To redo the merge, if needed:
git checkout 9b2a163ebb
git merge 5ea828e149
Functions generate_missing() and missing_size() had different
computations of sn_count. Not sure if it is a problem or not,
but let's do the same in both functions.
Hopefully the computation is correct.
T tracer was not functional anymore. because init_tpools() was called
before T_Config_Init().
The problem was that init_tpools() creates some threads and T_Config_Init()
does call fork() and the child process (which is the one running the
softmodem, the parent process is used to monitor it, this is the design
of the T tracer, maybe to change at some point, it's too complicated) has
no thread after the fork(). The threads created by init_tpools() are gone.
Game over.
In conclusion: T_Config_Init() has to be called very early in the life of
the softmodem, at least before init_tpools().
T tracer was not functional anymore. because init_tpools() was called
before T_Config_Init().
The problem was that init_tpools() creates some threads and T_Config_Init()
does call fork() and the child process (which is the one running the
softmodem, the parent process is used to monitor it, this is the design
of the T tracer, maybe to change at some point, it's too complicated) has
no thread after the fork(). The threads created by init_tpools() are gone.
Game over.
In conclusion: T_Config_Init() has to be called very early in the life of
the softmodem, at least before init_tpools().
We have to eat all the bytes, not only one.
We have to get the bit we want from the right buffer position.
Note: only one bit is supported from this "EPS network feature support",
not sure how to properly deal with it, so let's keep the code as it is.
This commit was made because the MAGMA MME sends "EPS network feature
support" to the UE in the NAS message "Attach Accept". OAI MME does not
send this. So the bug about size was not seen.
Maybe this fix won't work for non-openair gNB. We use the same keys and
algorithms as for SRBs, which is what openair gNB does.
(I hope I'm right there.)
To be changed if needed.
Ticks have to come every millisecond. The code was calling tick
only for DL slots.
(This whole logic will change at some point, this tick mechanism is not
good. But in the meantime, let's fix anyway.)
The previous commit created two new warnings:
warning: 'avgs' may be used uninitialized in this function
warning: 'seq_index' may be used uninitialized in this function
This commit removes them.
Maybe it's not the correct solution. To be fixed if needed.
Before this commit, pdcp_data_ind is done in the realtime PHY/MAC
thread, and this may have an impact on realtime performances.
Let's introduce a new thread to do the job, freeing the realtime
PHY/MAC logic.
This is not a minor change. It may have positive and/or negative
effects. To be tested carefully.
TX and ReTX should be faster now (for RLC AM, UM and TM).
RLC AM Status (function nr_rlc_entity_am.c:status_size()) was not modified,
so its CPU usage is the same as before. (It's more complicated to change
than for TX and ReTX.)
Note: some modifications have been done to the UE too, but for syntactic
reasons. The UE may fail to work, not tested.
For the UE, we have the problem of handling properly SecurityModeCommand.
We don't verify its integrity. This is a bit complicated to handle properly
and may require some API changes in the PDCP layer. So, as of today, the
integrity of this message is not verified.
Also, NSA mode has been touched, but it should not be a problem. But not
tested, so not sure.
When there is a retransmission, we want to use the same number of RBs.
The RBs are allocated by groups (RBGs).
In some cases (like a cell with 25 PRBs), the last RBG contains less RBs.
So in case of a retransmission, if this last RBG was used before we need
to reuse it to have the same number of RBs. If not, we will use more RBs
for the retransmission than the previous transmission.
In an experiment with two UEs (quectel modules) it was seen that when a
retransmission happens with a different number of RBs then the UE is not
happy at all, leading to way more NACKs in the harq processes than should
happen (actually the first NACK was not a NACK at all, but the eNB did
interpret it as a NACK; so the next transmission should simply be discarded
by the UE that successfully received the first transmission; instead the UE
fails to decode the data and sends a NACK, a real one this time).
Maybe it's not the correct solution, but it improve things, there are
much less NACKs.
The problem is that when using those RBs with MCS 28 we may exceed the
code rate 0.93 and according to 36.213 7.1.7 the UE may skip decoding
PDSCH entirely in this case.
This is a hack. The real solution is to check that the code rate is below
0.93 for each scheduling decision and, I don't know, reduce the MCS if the
code rate is above, so that in the end it is below. That means that we need
a proper resource grid for the configuration of the eNB and this is not an
easy thing (at least from my point of view) given all the possible
configurations for the eNB, so I prefer not to do it rather than do something
incorrect, thus this hack.
The problem of this hack is that we won't use all the available RBs for
scheduling, potentially reducing the maximum throughput achievable.
To be fixed properly at some time, by someone who understands fully the
resource grid and all the possible combinations (fdd/tdd, number of
antennas, whatever else).
Just in case there is a problem, better to see it.
max retx reached means "radio link failure". We should do
something about it at some point. For the moment, let's
inform the user that there is a problem.
We can set back to LOG_D later in time when we deal with it
properly.
- use bearer-1 in compute_t of integrity NIA2
- compare integrity at the right place in nr_pdcp_entity_recv_pdu
- proper handling of security parameters settings (algos, keys)
This commit implements integrity and security settings for nr pdcp. It will
only work for SRBs.
This commit has not been tested. It may fail to work completely. To be checked.
Exit gNB if connection to eNB failed - to be modified if needed.
We may want to try to connect over and over again until we succeed
but the modifications to the code to get this behavior are complex.
Exit on error is a simple solution.
The bug was:
connect a phone, disconnect, repeat five times. The fifth time the phone
does not reconnect because there is no free RA process.
When an UE is removed, we have to free associated RA processes.
Before this commit, when the gNB crashes, the eNB keeps running.
Same, if the eNB crashes, the gNB keeps running.
In a far past when this happened, the other program (the one not
crashing) was forced to exit.
Then this behavior was changed, for some reason.
But the code was not finished, so now we have a system in an inconsistent
state.
So either we accept that the connection between eNB and gNB can break,
that is one of the programs crashes, and we clean the state of the
program that keeps running. But this is a complex work and it will
surely not survive very long, because someone will change something
in the code later that will break this complex behavior.
Or, simpler, we go back to initial behavior, which is: the program that
did not crash does actually exit when the other crashes.
This commit provides the second solution.
It can easily be reverted whenever someone wants to implement the complex
solution.
We used to set 'does_nr' of an UE if we detect ENDC supported in the
UE capabilities.
Then we activate NR measurements if 'does_nr' is true.
The problem is that if the eNB is not connected to a gNB but the UE
reports some NR measurements (because a gNB is running somewhere near)
then the eNB will crash when starting the switch to NR.
A (quick) solution is to set 'does_nr' only if there is a gNB connected to
the eNB.
Maybe not the best solution. To be changed if needed.
This code has not been tested with a COTS UE. We need SRB1/2 (so we need SA)
to test.
The activation of security and integrity for SRB1 will need more code
(new functions) because it happens after some time, not at creation time
of the PDCP entity and there is no way to do this with current code.
We do some sanity checks for incoming rlc control packets. One of them
is to check that so_start is not > so_end when present.
But when 'range' is present and bigger than 1 it means that so_start refers
to one PDU and so_end to another one. So we may well have so_start > so_end.
This commit fixes that and reorganizes a bit the code to do the check before
processing and rejecting the PDU if the values are not correct. (Before this
commit we were NACKing the whole PDUs if so_start > so_end.)
The discard timer is used by the PDCP layer to discard packets that were not
transmitted to RLC after some time.
We don't really need it for the moment, maybe never for the default
bearer which is a best-effort bearer passing all kind of IP traffic.
Maybe for voice over IP kind of traffic it's important, or realtime
applications.
I ran an iperf UDP uplink test and saw no error on the PHY layer but still
lots of packets lost in the iperf logs, which was very surprising. By
analyzing the logs in wireshark I saw that, with the current scheduler,
an IP packet sent by the UE took 40ms to be sent piece by piece to the
gNB, and then the next uplink IP packet was never sent at all because
the discard timer was expired, leading to the high count of packets lost
in the iperf logs.
So let's put infinity for the moment for the discard timer. From my
understanding of the moment it's not a problem. If for some reason we
should set it to some non-infinity value, then we can change later.
Implement data transfer (38.331 5.2):
- transmit operation (in nr_pdcp_entity_recv_sdu)
- receive operation (in nr_pdcp_entity_recv_pdu)
- t-Reordering expiration
To keep track of time the function nr_pdcp_wakeup_timer_thread has been
added and is called by the gNB's MAC scheduler via nr_pdcp_tick.
(The UE has not been modified.)
A thread has been introduced to set time of all the existing PDCP
entities in the system. This probably does not scale well with many
PDCP entities (many UEs connected to the gNB). To be changed at some
point when needed. Hopefully, with a few UEs this mechanism won't be
too problematic (ie. won't cause realtime problems).
The problem with time in PDCP is that SDUs are kept if some are
missing, to do reordering. A timer is started and when it expires
we have to transmit the SDUs even if some are still missing. We
want to do that as soon as possible. So we have to check timers
often (every millisecond).
In RLC the time is set when there is some activity (SDU or PDU
received). In PDCP we cannot wait for this kind of activity. We
would keep packets longer than necessary, which may disturb
upper layers (think: TCP). That is why we introduced the thread,
as a simple mechanism to keep packets no longer than necessary.
The locking done by this thread may create realtime issues.
The whole logic of time keeping can be changed later if it turns
out that it is not efficient.
The functions dealing with DRB AM have been removed. All the types
of PDCP entities (DRB AM, DRB UM, SRB) use the same mechanism for
transmit and receive operations. So the code has been made common
for all those types.
We don't deal with:
- entity re-establishment, entity release or entity suspend
- SDU discard (the timer is present but not used)
- status reporting
- data recovery
- data volume calculation
- header compression / decompression
There is no plan to implement those features in a near future.
Integrity protection is not done yet, and will be implemented soon.
Minimalist gnb tracer to trace (for the moment) rxdataF when some
decoding for PUCCH or PUSCH has been done.
Important: T_BUFFER_MAX and T_CACHE_SIZE have been changed. T_BUFFER_MAX
is twice as big as before. T_CACHE_SIZE has been devided by two. Globally,
the memory usage of the T tracer is the same, but since we divided by two
the number of buffers in the cache, we may hit some realtime problems if
all the buffers are busy at some point. The simple solution would be to
use 8192*2 for T_CACHE_SIZE as it was, but then we double the memory usage
of the T tracer. This, or another solution, to be defined.
- a new section in the configuration file to select security algorithms,
with new code to deal with it
- cleanup CG-ConfigInfo: specs seem to indicate that we must not
add mcg_RB_Config; the gNB has to deal with that
- as a consequence, modify fill_default_rbconfig() that is called in
every cases and needs security and bearer parameters
The new section in the configuration file looks like:
security = {
# preferred ciphering algorithms
# the first one of the list that an UE supports in chosen
# valid values: nea0, nea1, nea2, nea3
ciphering_algorithms = ( "nea0", "nea2" );
# preferred integrity algorithms
# the first one of the list that an UE supports in chosen
# valid values: nia0, nia1, nia2, nia3
integrity_algorithms = ( "nia0" );
};
Cedric Roux