Fix MBS Broadcast distribution session lifecycle and Service Announcement crashes - #49
Draft
jordijoangimenez wants to merge 15 commits into
Draft
Fix MBS Broadcast distribution session lifecycle and Service Announcement crashes#49jordijoangimenez wants to merge 15 commits into
jordijoangimenez wants to merge 15 commits into
Conversation
TS 29.581 defines PATCH /dist-sessions/{distSessionRef} as operating on
the flat DistSession resource directly, with no wrapper property. The
JSON-Patch built here addressed paths under a non-existent /distSession
prefix instead: '' for a full replace (RFC 6901: the whole document is
the empty JSON Pointer) and /distSessionState for the state-only case.
A conformant MBSTF peer would reject these patches; this only worked
because the paired MBSTF side independently tolerates the same
non-standard prefix.
These fields were previously only ever copied across when the Distribution Session is INACTIVE -- if PATCHed while ESTABLISHED/ACTIVE the request was accepted (200) but the change was silently dropped, so e.g. a PATCH narrowing objAcqIds would echo back the old array. Reject the request outright (ModelException, MODIFICATION_NOT_ALLOWED) instead of silently no-op'ing it.
TS 26.502 clause 4.5.6 lists external target service areas and the NR RedCap UE class among the parameters the MBS Application Provider may update at any time, alongside mbsServInfo/mbsFSAId/tgtServAreas -- not restricted to INACTIVE like the fields below them. A PATCH/PUT changing either while the session was ACTIVE/ESTABLISHED was previously silently dropped instead of applied.
mcc()/mnc() converted the spec's digit strings to plain uint16_t, discarding whether the MNC has 2 or 3 digits (and any leading zero). Downstream code then re-guessed the digit count from the numeric value (mnc<100?2:3), misclassifying any real 3-digit MNC under 100 (e.g. "001"-"099") as 2-digit. Added mncLen() (from the source string's actual length) and wired both call sites to the new length-aware mb_smf_sc_ncgi_set_plmn_id_len()/mb_smf_sc_tai_new_len() functions in rt-5gc-service-consumers instead of the guessing ones.
Two gaps in processUserDataIngSessionUpdate()'s PUT path (PATCH on this resource is intentionally not implemented yet, so unaffected): - actPeriods and actPeriodsRepRule are mutually exclusive per TS 29.580 clause 6, but a request setting both silently prioritised actPeriods instead of being rejected, unlike the equivalent POST-path check. - mbsSessionId and locationDependent must never be updated after initial provisioning per TS 29.580 clause 5.3.2.4.2, but only mbsDistSessionId was actually restored from the stored value before the change-detection comparison -- the other two passed through unprotected.
… Patch'
MBSTF's actual PATCH target for /dist-sessions/{id} is CreateReqData, whose
generated applyPatch() only recognises paths under its own "/distSession"
property -- it does not expose DistSession's fields at the top level. The
previous "/distSessionState" (no wrapper) path was rejected outright,
meaning every state-only PATCH (activate/deactivate) silently failed and
triggered a rollback -- including the one that activates the built-in
"USER SERVICE ANNOUNCEMENT CHANNEL" session, i.e. the real MBS-4-MC
broadcast Service Announcement carousel never got updated with
newly-provisioned services.
createMbsSession() always sent MBS_SERVICE_TYPE_MULTICAST to the SMF/MB-SMF regardless of the parent MBS User Service's own servType. SMF's Nmbsmf handler only triggers the Namf_MBSBroadcast context-create call -- the step that actually drives NGAP Broadcast Session Setup to the gNB -- if the service type is broadcast; for MULTICAST it correctly does nothing there (multicast UE-join uses a separate, currently-unimplemented Namf_MBSCommunication procedure instead). So every BROADCAST User Service ended up silently treated as MULTICAST at the MB-SMF boundary: PFCP/N4mb and MBSTF FLUTE transmission all completed normally, but NGAP never reached the gNB, no MRB was ever created for the new session, and content had no bearer to travel over -- dropped after leaving the UPF with no visible error anywhere. The actual servType has to be captured at construction time in the owning UserDataIngSession instance (which has access to mbsUserService()), not looked up later via a static locate(ingSessionId) call -- that lookup races against this object's own registration into the id->instance map and always loses (the ContextData is built and createMbsSession() invoked on it before the constructing UserDataIngSession finishes registering itself), silently falling back to MULTICAST regardless.
sendMbstfRequests() used to be called exactly once (guarded by requested_mbstf_dist_session, which was never reset), and the wait following it had no deadline of its own. If MBSTF's response to that single request was ever lost, confirmed live via gdb: this worker thread stuck forever in the wait_for() below, with a real, established TCP connection to MBSTF sitting idle (some transient SBI/SCP hiccup around the same moment, not a deadlock or a bug in the wait itself) -- the announcement channel's distribution session would never be created and no Service Announcement content would ever be pushed, for the lifetime of the MBSF process, with no way to recover short of restarting it. Retry after a bounded number of wait iterations (5 seconds) instead of waiting on the same request forever.
…es POST
The POST branch of the /nmbsf-mbs-us/v1/mbs-user-services dispatch matched on
resource0 == "mbs-user-services" alone, without checking that there was no
sub-resource after it -- so a request actually meant for a sub-resource path (e.g.
a client hitting /mbs-user-services/{id}/ingest-sessions instead of the real
/nmbsf-mbs-ud-ingest/v1/sessions endpoint) got parsed as if it were a brand-new
MBSUserService creation body instead. Confirmed live: that body is missing fields
MBSUserService's constructor requires (e.g. extServiceIds), and
checkAndSetUserServiceAnnouncementChannel() constructs a raw MBSUserService from it
with no try/catch of its own, so the resulting fiveg_mag_reftools::ModelException
was uncaught, called std::terminate(), and took the whole MBSF process down --
instead of the 400 Bad Request a malformed or misrouted client request should get.
Two independent fixes: only match the POST-creates-a-new-service case when there is
no resource1 (mbs-user-services is a collection endpoint, so a real create request
never has one), and wrap checkAndSetUserServiceAnnouncementChannel() in a try/catch
so a genuinely malformed but correctly-routed body (missing a required field) gets
a proper error response instead of crashing the process either way.
mbsf.yaml.in: cross-reference comment on userServiceAnnouncement pointing at
rt-mbs-client's new static announcement-channel bootstrap config, which must agree
with ssmDestinationAddress/ssmPort/the hardcoded announcement TSI here.
…onfigured one The announcement-channel-specific branch of UserDataIngSession's distribution session setup drew a fresh random port (32768-65535) for its SSM the same way the regular per-content-session branch does -- correct there (a new content session legitimately gets a new port every time), wrong here: the Service Announcement channel is meant to be a single, fixed, well-known channel, which is exactly why mbsf.yaml already has a userServiceAnnouncement.ssmPort config value and rt-mbs-client has a matching static mbsf_client.announcement_channel bootstrap config -- ssmPort was already correctly plumbed through Context (Context::userServiceAnnSsmPort()), it just wasn't being used for this. Confirmed live via tcpdump on the UE's own TUN device: with the random port, MBSTF genuinely transmitted the real FLUTE carousel content on some other, unpredictable port every run (e.g. 41873) while a client statically configured with ssmPort's value (3000) filtered every real packet out silently, since the destination port never matched -- despite the PDCP/RLC/GW chain now correctly delivering the content to the TUN device (confirmed by the prior PDCP SN-size fix in srsRAN_Project_mbs). This made the whole reproducible-bootstrap design pointless: the one value it depended on being fixed wasn't actually fixed. Fix: use App::self().context()->userServiceAnnSsmPort() for the announcement channel's own SSM port instead of the random generator. Builds clean.
…n session
createMbsSession() unconditionally built a brand new MBSMFMBSSession (and the
underlying mb_smf_sc_mbs_session_new_ipv4()/_ipv6() C session object) on every
call, only guarding the *assignment* to context_data->MBSSession
('if (!context_data->MBSSession)' further down) rather than the work itself.
isMBSSessionCreated() only flips true once the underlying MB-SMF session
genuinely reaches CREATED state -- if MBSTF ever rejects the distribution
session (confirmed live: a malformed request), that never happens, and
userServiceAnnChannelDistributionSessionInfo()'s periodic check
('if (!isMBSSessionCreated(key)) createMbsSession(...)') called this again,
immediately, every single loop iteration, forever: no backoff, no bound.
Confirmed live: MBSF spun at the announcement-channel workerLoop's tick rate
(tens of iterations/second) reconstructing the C session object and
re-notifying MB-SMF each time, until it crashed.
Fix: skip entirely once a session object for this context already exists. The
caller's own retry-driving state (MBSSessionStatus, receivedMBSTFResponse) is
what should progress it from here, not another blind rebuild.
Builds clean.
…issing wrapper
Two compounding bugs blocked every activate/deactivate on an
already-created distribution session:
1. UserDataIngSession's update-merge loop compared the full
MBSDistributionSessionInfo (including mbsDistSessState) to decide
needsUpdate vs the lightweight stateUpdate path. Since this API has
no separate state-only endpoint, every activate/deactivate PUT
changed state alongside resending the rest of the body, so it always
(mis)classified as needsUpdate -- triggering a full session rebuild
PATCH instead of the purpose-built stateUpdate PATCH.
2. That needsUpdate PATCH path itself was also broken: it sent the
DistSession's JSON directly as the patch value for an empty-path
add/replace, but MBSTF's actual patch target for /dist-sessions/{id}
is a CreateReqData, whose fromJSON() requires the value to be a full
CreateReqData document (a distSession key wrapping the fields) --
confirmed live via MBSTF's own error: 'Mandatory Information Element
Missing: distSession: Field "distSession" is required'.
Fixed both: state-only changes now correctly route through the
lightweight stateUpdate path (state normalised out before the equality
check), and the needsUpdate path now wraps its patch value correctly.
Verified live: PUT to ACTIVE on a real pushed-content distribution
session succeeded and MBSTF actually transmitted the object, which
previously failed outright.
processUserDataIngSessionUpdate()'s caller only caught std::out_of_range, not ModelException. updateMBSDistributionSessionInfo() correctly throws a ModelException when a client PATCHes objDistrInfo/pckDistrInfo while the Distribution Session isn't INACTIVE (a real, intentional validation, not a bug) -- but with no catch for that type, the exception propagated all the way out of the SBI request handler uncaught and crashed the entire MBSF process via std::terminate(), taking down every other active session with it over a single bad client request. Confirmed live: PATCHing objAcqIds on an ACTIVE session crashed MBSF outright. Fixed by catching ModelException here too and converting it to a proper error response, same pattern already used for the actPeriods/actPeriodsRepRule validation a few lines above.
Context::addMbsSessionId()/deleteMbsSessionId() track SSM addresses currently
in use, keyed by a UniqueMbsSessionId built from the SSM plus service area
info. Both of the two deletion-completion call sites that were supposed to
release this on delete passed the wrong key type:
- setMBSSessionDeleted() passed ids.second (the distSessionInfoKey, e.g.
"AP_MBS_SESSION_1") to removeFromRegistry() -- which is keyed by the real
MBSTF-assigned distribution session ID -- and ids.first (the ingSessionId,
a UUID) to removeDistributionSessionInfo() -- which is keyed by
distSessionInfoKey. Both erase()-by-wrong-key calls silently no-op.
- setMBSTFDistSessionDeletedFlag()'s whole cleanup block was commented out,
and its own draft had the identical ids->first/ids->second mismatch, so
re-enabling it as originally written would have no-op'd the same way.
Net effect, confirmed live: a deleted MBS User Service's SSM address stayed
registered in Context::m_mbsSessionIds forever, so recreating a session on
the same address (e.g. any repeated test run, or any deployment that reuses
a small SSM pool) logs a permanent "Attempt to insert duplicate
UniqueMBSSessionId" warning and leaks a UserDataIngSession object every time.
Fixed both call sites to use ContextData's own correctly-populated fields
(mbstfDistSessionId, distSessionInfoKey) instead of the mismatched ids pair.
Re-enabling setMBSTFDistSessionDeletedFlag()'s full cleanup (including the
final deleteUserDataIngSession() call) also had to be reverted after live
testing surfaced a double-deletion race with setMBSSessionDeleted() -- both
fire for the same logical session on a normal delete, and both would try to
tear down the same UserDataIngSession, destroying it before
setMBSSessionDeleted()'s own deferred DELETE HTTP response could be sent
("User Data Ingest Session deleted before N pending responses sent", then
the portal's request timing out). setMBSSessionDeleted() remains the sole
trigger for the final teardown; setMBSTFDistSessionDeletedFlag() now only
does its own partial-removal (markForDeletion) and registry-cleanup duties.
Verified live: 3 consecutive create+delete cycles on the same SSM address
produce no duplicate-registration warnings and no deletion errors, and the
full MBS Broadcast Service Announcement flow (verify-e2e.sh) still completes
cleanly end to end afterwards.
jordijoangimenez
marked this pull request as draft
August 14, 2026 10:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes to make MBSF's MBS Broadcast Distribution Session lifecycle (create → activate/deactivate → update → delete) and Service Announcement channel work reliably end-to-end, including several crashes found by live-testing the full pipeline against a real gNB/UE/5GC/MBSTF stack.
Note on PATCH: none of this touches the client-facing
PATCH(RFC 7396 JSON Merge Patch) onMBSUserService/MBSUserDataIngSession— that's intentionally on hold pending 5G-MAG/Standards#182 per #45, and stays that way here. Everything below is either the already-implemented PUT path on.../sessions/{id}(UpdateIndMBSUserDataIngSession, TS 29.580 §5.3.2.4.2), or the internal MBSF→MBSTFPATCH(RFC 6902 JSON Patch, Nmb2 reference point, TS 29.581 — a different method on a different resource, already in scope and unaffected by #45).What's included, in order
Bump rt-common-shared submodule: fix CJson copy-assignment SIGILL crash — see the linked rt-common-shared PR.
CJson::operator=(const CJson&)fell off the end of the function with no return statement; GCC lowered this to a realud2/SIGILL trap the first time a plain lvalue copy-assignment exercised it, crashing MBSF outright. Found via core dump + gdb.Fix MBSF process-wide crash on invalid objDistrInfo update —
updateMBSDistributionSessionInfo()correctly throws when a client tries to PATCHobjDistrInfo/pckDistrInfowhile the Distribution Session isn't INACTIVE (a real, intentional validation), but nothing in the PUT handler caught that exception type, so it propagated out of the SBI request handler uncaught and crashed the whole process — taking down every other active session with it over one bad request. Confirmed live; now caught and converted to a proper error response.Fix distribution session state-change PATCH: wrong classification + missing wrapper — two compounding bugs that together broke every activate/deactivate on an already-created session:
mbsDistSessState) to decide "rebuild the whole MBSTF session" vs. "just PATCH the state" — since every activate/deactivate PUT necessarily changes state alongside resending the rest of the body, it always took the "rebuild" path instead of the lightweight state-only one.DistSession's JSON directly as the PATCH value for an empty-path add/replace, but MBSTF's actual PATCH target for/dist-sessions/{id}is aCreateReqData, whosefromJSON()requires the value to be a full document with adistSessionwrapper key — confirmed via MBSTF's own error, "Mandatory Information Element Missing: distSession: Field 'distSession' is required."Verified live end-to-end: PUT to ACTIVE on a real pushed-content Distribution Session succeeded and MBSTF actually transmitted the object, which previously failed outright.
Fix MBSF crash: unbounded retry loop when MBSTF rejects a distribution session —
createMbsSession()wasn't idempotent: it rebuilt a brand-new session wrapper (re-triggering the underlying session-creation call) on every invocation, and the periodic status checker called it again every loop tick with no backoff since "is created" never became true after a rejection. This spun at tens of iterations/second until MBSF crashed. Reproduced with a deliberately malformed request; confirmed the fix stops the crash on repeat.Fix Service Announcement channel using a random port instead of the configured one — the announcement-channel-specific ingest branch drew a random ephemeral port instead of using the configured
ssmPort, making it impossible for any statically-configured client to actually receive the broadcast Service Announcement carousel. Confirmed via packet capture: real traffic arrived on a random port while a statically-configured client filtered everything out silently.Fix uncaught-exception crash on a malformed/misrouted
mbs-user-servicesPOST — the POST dispatch matched sub-resource paths it shouldn't have, and a malformed body threw uncaught throughcheckAndSetUserServiceAnnouncementChannel(); now returns a proper 400.Fix announcement channel worker hanging forever on a lost MBSTF response — the worker's request-response wait had no deadline; a lost/dropped response (real, transient SBI hiccup — not a bug in the wait itself) left the announcement channel's Distribution Session permanently uncreated, with no way to recover short of restarting MBSF. Now retries after a bounded number of iterations.
Propagate the real MBS User Service type instead of hardcoding MULTICAST —
createMbsSession()always sentMBS_SERVICE_TYPE_MULTICASTto the SMF regardless of the parent User Service's actualservType. SMF's handler only triggersNamf_MBSBroadcastcontext-create — the step that drives NGAP Broadcast Session Setup to the gNB — forservice_type == BROADCAST. So every BROADCAST User Service was silently treated as MULTICAST at the MB-SMF boundary: PFCP/N4mb and MBSTF FLUTE transmission completed normally, but NGAP never reached the gNB, no MRB was ever created, and content had no bearer to travel over — dropped after leaving the UPF with no visible error anywhere. This was the single biggest reason real content pushed through MBSF never reached the UE.Fix MBSTF rejecting distSessionState PATCH with 'Unknown path in JSON Patch' — the internal Nmb2 PATCH path used a bare
/distSessionStateJSON Pointer, but MBSTF's actual PATCH target (CreateReqData, patching a nestedDistSession) only recognises paths under its own/distSessionprefix. Every state-only PATCH (activate/deactivate) silently failed and rolled back — including the one that activates the built-in Service Announcement channel session, so the real Service Announcement carousel never got updated with newly-provisioned services.Harden MBS User Data Ingest Session PUT validation —
actPeriods/actPeriodsRepRulemutual exclusion wasn't enforced on the PUT path the way it already was on POST;mbsSessionId/locationDependent(immutable-after-create per TS 29.580 §5.3.2.4.2) weren't fully protected from a PUT silently changing them.Preserve MNC digit count when building NCGI/TAI PLMN Ids —
mcc()/mnc()discarded whether the MNC has 2 or 3 digits, so downstream code re-guessed the digit count from the numeric value and misclassified any real 3-digit MNC under 100 (e.g. "001"–"099") as 2-digit.Stop gating ExtTgtServAreas/NrRedCapUeInfo updates on INACTIVE state — TS 26.502 §4.5.6 lists these among the parameters an MBS Application Provider may update at any time; a PATCH/PUT changing either while ACTIVE/ESTABLISHED was previously silently dropped instead of applied.