diff --git a/context/db_test.go b/context/db_test.go index 8e1537aa..a5794e61 100644 --- a/context/db_test.go +++ b/context/db_test.go @@ -273,3 +273,56 @@ func TestSMContextDBRoundTrip_PreservesFieldsAcrossSerializerMigration(t *testin t.Errorf("expected CommunicationClient to be dropped across the DB round trip, got %+v", recovered.CommunicationClient) } } + +// Releasing a session restored from MongoDB used to panic: the allocator it carries has no pool. +// The address cannot be returned to the running configuration's pool, but the release itself has +// to finish, and the session's own address is cleared either way. +func TestReleaseUeIpAddr_OnRestoredContext(t *testing.T) { + allocator, err := NewIPAllocator("192.168.100.0/24") + if err != nil { + t.Fatalf("NewIPAllocator: %v", err) + } + ip, err := allocator.Allocate(testSupi) + if err != nil { + t.Fatalf("Allocate: %v", err) + } + + original := &SMContext{ + Ref: "restored-release-ref", + Supi: testSupi, + Dnn: "restored-release-dnn", + PDUAddress: &UeIpAddr{Ip: ip}, + DNNInfo: &SnssaiSmfDnnInfo{UeIPAllocator: allocator, MTU: 1400}, + AnType: models.ACCESSTYPE__3_GPP_ACCESS, + // Real values for the same reason as the round-trip test above: AccessType, NfStatus and + // SscModes.DefaultSscMode reject "" in their own UnmarshalJSON. + AMFProfile: models.NFProfileDiscovery{ + NfInstanceId: "amf-instance-1", + NfType: models.NFTYPE_AMF, + NfStatus: models.NFSTATUS_REGISTERED, + }, + SelectedPCFProfile: models.NFProfileDiscovery{ + NfInstanceId: "pcf-instance-1", + NfType: models.NFTYPE_PCF, + NfStatus: models.NFSTATUS_REGISTERED, + }, + DnnConfiguration: models.DnnConfiguration{ + SscModes: models.SscModes{DefaultSscMode: models.SSCMODE_SSC_MODE_1}, + }, + } + + recovered := &SMContext{} + if err := recovered.UnmarshalJSON(mapToByte(ToBsonM(original))); err != nil { + t.Fatalf("round-trip unmarshal failed: %v", err) + } + if recovered.DNNInfo == nil || recovered.DNNInfo.UeIPAllocator == nil { + t.Fatalf("DNNInfo did not survive the round trip: %+v", recovered.DNNInfo) + } + + if err := recovered.ReleaseUeIpAddr(); err != nil { + t.Errorf("ReleaseUeIpAddr: %v", err) + } + if got := recovered.PDUAddress.Ip; !got.Equal(net.IPv4zero) { + t.Errorf("PDUAddress.Ip = %v, want the 0.0.0.0 sentinel", got) + } +} diff --git a/context/ip_allocator.go b/context/ip_allocator.go index 1cb22f4d..5e5e75e6 100644 --- a/context/ip_allocator.go +++ b/context/ip_allocator.go @@ -14,6 +14,12 @@ import ( "github.com/omec-project/smf/logger" ) +// IPAllocator hands out the UE addresses of one DNN's pool. +// +// A method can be reached on an allocator that has no pool: buildSnssaiSmfInfo stores a DNN whose +// ipDomain carries no usable ueSubnet with UeIPAllocator nil, and an SMContext read back from +// MongoDB carries an allocator that marshalled as {}, the type having only unexported fields. +// Each method answers such a receiver instead of dereferencing it. type IPAllocator struct { ipNetwork *net.IPNet g *_IDPool @@ -89,6 +95,11 @@ func (a *IPAllocator) StaticIpForImsi(imsi string) net.IP { // Allocate will allocate the IP address and returns it func (a *IPAllocator) Allocate(imsi string) (net.IP, error) { + // No pool, no addresses; the caller answers this error with IpAllocError. + if a == nil || a.g == nil || a.ipNetwork == nil { + return nil, errors.New("ip allocation failed: this DNN has no usable UE subnet") + } + // check if static IP already reserved for this IMSI if ip := a.StaticIpForImsi(imsi); ip != nil { return ip, nil @@ -110,6 +121,10 @@ func (a *IPAllocator) Allocate(imsi string) (net.IP, error) { } func (a *IPAllocator) ReserveStaticIps(ips *map[string]string) { + if a == nil || a.g == nil || a.ipNetwork == nil { + return + } + a.g.staticIps = ips for _, ipStr := range *ips { if ip := net.ParseIP(ipStr).To4(); ip != nil { @@ -120,11 +135,24 @@ func (a *IPAllocator) ReserveStaticIps(ips *map[string]string) { } func (a *IPAllocator) BlockIp(ip net.IP) { + if a == nil || a.g == nil || a.ipNetwork == nil { + return + } + offset := IPAddrOffset(ip, a.ipNetwork.IP) a.g.block(int64(offset)) } func (a *IPAllocator) Release(imsi string, ip net.IP) { + // There is no pool to return the address to, so the release is skipped and logged rather + // than passed over. Reconciling the running pool with restored sessions is out of scope. + if a == nil || a.g == nil || a.ipNetwork == nil { + logger.CtxLog.Warnf("cannot release UE address %v for %s: this allocator has no pool, "+ + "so the release is skipped", ip, imsi) + + return + } + // Don't release static IPs if a.StaticIpForImsi(imsi) != nil { return diff --git a/context/ip_allocator_test.go b/context/ip_allocator_test.go index d415c026..ce820fa0 100644 --- a/context/ip_allocator_test.go +++ b/context/ip_allocator_test.go @@ -169,3 +169,29 @@ func TestAReleasedAddressIsAllocatedAgainAfterRestarts(t *testing.T) { t.Errorf("allocation after %v was released: %v", first, err) } } + +// Every method of an allocator with no pool panicked before; see IPAllocator for the two ways a +// caller comes to hold one. +func TestIPAllocatorWithNoPool(t *testing.T) { + const imsi = "imsi-208930100007487" + + for name, allocator := range map[string]*smf_context.IPAllocator{ + "nil allocator": nil, + "restored allocator": {}, + } { + t.Run(name, func(t *testing.T) { + ip, err := allocator.Allocate(imsi) + if err == nil { + t.Errorf("Allocate returned %v and no error", ip) + } + if ip != nil { + t.Errorf("Allocate returned address %v alongside its error", ip) + } + + allocator.Release(imsi, net.ParseIP("192.168.1.5")) + allocator.BlockIp(net.ParseIP("192.168.1.6")) + static := map[string]string{imsi: "192.168.1.7"} + allocator.ReserveStaticIps(&static) + }) + } +}