Skip to content

Commit 9cd3cab

Browse files
[nexus] stop echoing the underlay member list from group updates
Tag and source updates read the underlay group and had written the read member list straight back. `multicast_group_update_underlay` is a full-list replace whose only concurrency control is the per-group tag, so that echo-write could silently revert a member add or remove that landed between the read and the write. `update_groups` now reads the underlay group only to obtain the tag authorizing the external update, creating the group (memberless) when absent, and never writing the underlay member list. The member add and remove paths remain the list's only writers. This also removes an underlay write the planned member-writer transition would otherwise have to delete.
1 parent 232fe89 commit 9cd3cab

1 file changed

Lines changed: 24 additions & 42 deletions

File tree

nexus/src/app/multicast/dataplane.rs

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,9 @@ impl MulticastDataplaneClient {
532532
}
533533

534534
/// Update a multicast group's tag (name) and/or sources in the dataplane.
535+
///
536+
/// Membership is left untouched: the underlay member list is written only
537+
/// by the member add and remove paths.
535538
pub(crate) async fn update_groups(
536539
&self,
537540
params: GroupUpdateParams<'_>,
@@ -608,15 +611,20 @@ impl MulticastDataplaneClient {
608611
let sources = sources_dpd.clone();
609612
let underlay_ip_admin = underlay_ip_admin.clone();
610613
async move {
611-
// Ensure/get underlay members, create if missing
612-
let (members, existing_tag) = match client
614+
// Read the underlay group, creating it if absent.
615+
//
616+
// The member list is never written back from here. A tag
617+
// and source update does not change membership, and
618+
// `multicast_group_update_underlay` is a full-list
619+
// replace whose only concurrency control is the tag,
620+
// which is per-group rather than per-version, so a write
621+
// would silently revert a member add or remove that
622+
// landed since the read.
623+
let underlay = match client
613624
.multicast_group_get_underlay(&underlay_ip_admin)
614625
.await
615626
{
616-
Ok(r) => {
617-
let inner = r.into_inner();
618-
(inner.members, inner.tag)
619-
}
627+
Ok(r) => r.into_inner(),
620628
Err(DpdError::ErrorResponse(resp))
621629
if resp.status()
622630
== reqwest::StatusCode::NOT_FOUND =>
@@ -631,15 +639,13 @@ impl MulticastDataplaneClient {
631639
"underlay multicast group missing tag",
632640
)
633641
})?;
634-
let created = self
635-
.dpd_ensure_underlay_created(
636-
client,
637-
underlay_ip_admin.clone(),
638-
db_tag,
639-
switch_slot,
640-
)
641-
.await?;
642-
(created.members, created.tag)
642+
self.dpd_ensure_underlay_created(
643+
client,
644+
underlay_ip_admin.clone(),
645+
db_tag,
646+
switch_slot,
647+
)
648+
.await?
643649
}
644650
Err(e) => {
645651
error!(
@@ -655,33 +661,13 @@ impl MulticastDataplaneClient {
655661
}
656662
};
657663

658-
// Update underlay preserving members, using existing
659-
// tag for authorization
660-
let underlay_entry =
661-
MulticastGroupUpdateUnderlayEntry { members };
664+
// The existing tag authorizes the external update below.
662665
let tag: MulticastTag =
663-
existing_tag.try_into().map_err(|e| {
666+
underlay.tag.clone().try_into().map_err(|e| {
664667
Error::internal_error(&format!(
665668
"invalid multicast tag: {e}"
666669
))
667670
})?;
668-
let underlay_response = client
669-
.multicast_group_update_underlay(
670-
&underlay_ip_admin,
671-
&tag,
672-
&underlay_entry,
673-
)
674-
.await
675-
.map_err(|e| {
676-
error!(
677-
self.log,
678-
"failed to update underlay";
679-
"underlay_ip" => %underlay_ip_admin,
680-
"switch" => ?switch_slot,
681-
"error" => %e
682-
);
683-
Error::internal_error("failed to update underlay")
684-
})?;
685671

686672
// Prepare external update/create entries with pre-computed data.
687673
//
@@ -717,11 +703,7 @@ impl MulticastDataplaneClient {
717703
)
718704
.await?;
719705

720-
Ok::<_, Error>((
721-
switch_slot,
722-
underlay_response.into_inner(),
723-
external_response,
724-
))
706+
Ok::<_, Error>((switch_slot, underlay, external_response))
725707
}
726708
});
727709

0 commit comments

Comments
 (0)