From 5afe2f93e512eeb2cac22292c466bcb1f86768c4 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Tue, 6 Oct 2026 14:52:50 -0700 Subject: [PATCH 1/2] Count a rule shape by the filter the UPF installs Signed-off-by: Arrobo, Gabriel --- backend/nfconfig/config.go | 36 +-------- configapi/slice_helpers.go | 114 ++++++++++++++++++++--------- configapi/slice_operations_test.go | 50 ++++++++++++- 3 files changed, 130 insertions(+), 70 deletions(-) diff --git a/backend/nfconfig/config.go b/backend/nfconfig/config.go index 7eaf8a7b..485307df 100644 --- a/backend/nfconfig/config.go +++ b/backend/nfconfig/config.go @@ -6,24 +6,16 @@ package nfconfig import ( - "fmt" "slices" "sort" "strconv" - "strings" "github.com/omec-project/openapi/v2/nfConfigApi" - "github.com/omec-project/webconsole/backend/factory" "github.com/omec-project/webconsole/backend/logger" "github.com/omec-project/webconsole/configapi" "github.com/omec-project/webconsole/configmodels" ) -const ( - tcp int32 = 6 - udp int32 = 17 -) - type accessAndMobilityKey struct { plmn configmodels.SliceSiteInfoPlmn sliceId configmodels.SliceSliceId @@ -427,7 +419,7 @@ func buildSlicePccRules(slice configmodels.Slice) []nfConfigApi.PccRule { func buildPccFlows(ruleConfig configmodels.SliceApplicationFilteringRules) []nfConfigApi.PccFlow { pccFlows := []nfConfigApi.PccFlow{} - description := buildFlowDescription(ruleConfig) + description := configapi.BuildSDFFilter(ruleConfig) var status nfConfigApi.Status if ruleConfig.Action == "deny" { status = nfConfigApi.STATUS_DISABLED @@ -445,32 +437,6 @@ func buildPccFlows(ruleConfig configmodels.SliceApplicationFilteringRules) []nfC return pccFlows } -func buildFlowDescription(ruleConfig configmodels.SliceApplicationFilteringRules) string { - endp := ruleConfig.Endpoint - if strings.HasPrefix(endp, "0.0.0.0") { - endp = "any" - } - - switch ruleConfig.Protocol { - case tcp: - return buildDescription("tcp", endp, ruleConfig.StartPort, ruleConfig.EndPort) - case udp: - return buildDescription("udp", endp, ruleConfig.StartPort, ruleConfig.EndPort) - default: - return fmt.Sprintf("permit out ip from %s to assigned", endp) - } -} - -func buildDescription(protocol, endpoint string, startPort, endPort int32) string { - if startPort == 0 && endPort == 0 { - return fmt.Sprintf("permit out %s from %s to assigned", protocol, endpoint) - } else if factory.WebUIConfig.Configuration.SdfComp { - return fmt.Sprintf("permit out %s from %s %s-%s to assigned", protocol, endpoint, strconv.FormatInt(int64(startPort), 10), strconv.FormatInt(int64(endPort), 10)) - } else { - return fmt.Sprintf("permit out %s from %s to assigned %s-%s", protocol, endpoint, strconv.FormatInt(int64(startPort), 10), strconv.FormatInt(int64(endPort), 10)) - } -} - func getSupportedDnns(slice configmodels.Slice, deviceGroups map[string]configmodels.DeviceGroups) []string { dnns := []string{} diff --git a/configapi/slice_helpers.go b/configapi/slice_helpers.go index fc6d467e..05f48413 100644 --- a/configapi/slice_helpers.go +++ b/configapi/slice_helpers.go @@ -231,6 +231,10 @@ func validateRulePorts(rule configmodels.SliceApplicationFilteringRules, sliceNa // UPF refuses every session that needs a shape it does not hold. const maxUpfRuleShapes = 8 +// anyEndpoint is the SDF keyword for any address: the endpoint an operator may give, and the one +// BuildSDFFilter writes for an endpoint beginning 0.0.0.0. +const anyEndpoint = "any" + // A rule's endpoint is written into its SDF filter as it is, and the UPF splits the filter into // fields, so anything but a single address, prefix or "any" changes what it parses: extra fields // can add a port, or turn the filter into one it cannot parse and installs as matching any traffic. @@ -238,7 +242,7 @@ const maxUpfRuleShapes = 8 // installs the rule matching any traffic. func validateRuleEndpoint(rule configmodels.SliceApplicationFilteringRules, sliceName string) error { endpoint := rule.Endpoint - if endpoint == "" || endpoint == "any" || net.ParseIP(endpoint) != nil { + if endpoint == "" || endpoint == anyEndpoint || net.ParseIP(endpoint) != nil { return nil } if _, _, err := net.ParseCIDR(endpoint); err == nil { @@ -251,32 +255,35 @@ type ruleShape struct { prefixLen int protocol bool port bool - // Set only for a stored endpoint of several fields, whose mask cannot be known here: every - // input of its SDF filter, so that only rules with the same filter share a shape. + // Set only for a stored endpoint of several fields, whose mask cannot be known here: the SDF + // filter BuildSDFFilter writes for it, as the UPF reads it, so that only rules writing the same + // filter share a shape -- including a modelled rule that happens to write the same one (see + // validateUpfRuleShapes). unknown string } -// The shape of the PDR the UPF installs for this rule, as buildFlowDescription writes its SDF -// filter. The UPF installs a PDR whose filter it cannot parse without the filter, matching any -// traffic, so an endpoint or a port range it cannot parse has the shape of a rule permitting any -// traffic. A range it parses but cannot install, it refuses before writing anything, so that rule -// takes no mask and reports false. validateRulePorts refuses both kinds of range, but a slice stored +// The shape of the PDR the UPF installs for this rule, as BuildSDFFilter writes its SDF filter. +// +// modelable is false for an endpoint of several fields, whose mask cannot be known here: the UPF may +// parse its fields, together with the protocol and ports written around them, into any shape. Such a +// rule is keyed by the filter it writes instead (see validateUpfRuleShapes). +// +// counts is false for a modelable rule the UPF refuses before installing anything -- a port range +// too wide -- so that rule takes no mask. An endpoint or port range the UPF cannot parse it drops +// with the rest of the filter, installing the rule as matching any traffic, so that reports the +// any-traffic shape and counts. validateRulePorts refuses a range of either kind, but a slice stored // before it may hold one. -func ruleShapeOf(rule configmodels.SliceApplicationFilteringRules) (ruleShape, bool) { - var shape ruleShape +func ruleShapeOf(rule configmodels.SliceApplicationFilteringRules) (shape ruleShape, modelable, counts bool) { // The UPF splits the filter with strings.Fields, so it reads the endpoint as these fields. fields := strings.Fields(rule.Endpoint) switch { case strings.HasPrefix(rule.Endpoint, "0.0.0.0"): - // buildFlowDescription writes such an endpoint as "any", which the UPF parses as any + // BuildSDFFilter writes such an endpoint as "any", which the UPF parses as any // address. case len(fields) > 1: - // validateRuleEndpoint refuses such an endpoint, but a slice stored before it may hold - // one. The UPF may parse its fields, together with the protocol and ports written around - // them, into any shape, so it is not modelled: a rule counts as a shape of its own unless - // another writes exactly the same filter. - return ruleShape{unknown: filterInputs(fields, rule)}, true - case len(fields) == 1 && fields[0] == "any": + // validateRuleEndpoint refuses such an endpoint, but a slice stored before it may hold one. + return ruleShape{}, false, false + case len(fields) == 1 && fields[0] == anyEndpoint: default: endpoint := strings.Join(fields, "") if !strings.Contains(endpoint, "/") { @@ -296,7 +303,7 @@ func ruleShapeOf(rule configmodels.SliceApplicationFilteringRules) (ruleShape, b _, ipNet, err := net.ParseCIDR(endpoint) if err != nil { // A filter the UPF cannot parse, including one with no endpoint, it drops. - return ruleShape{}, true + return ruleShape{}, true, true } ones, bits := ipNet.Mask.Size() // The UPF keeps only the low 32 bits of an IPv6 mask, so an IPv6 prefix reaches the @@ -308,30 +315,51 @@ func ruleShapeOf(rule configmodels.SliceApplicationFilteringRules) (ruleShape, b } if rule.Protocol == protocolTCP || rule.Protocol == protocolUDP { if rule.StartPort < 0 || rule.EndPort > math.MaxUint16 || rule.StartPort > rule.EndPort { - return ruleShape{}, true + return ruleShape{}, true, true } shape.protocol = true anyPort := rule.StartPort == 0 && (rule.EndPort == 0 || rule.EndPort == math.MaxUint16) if !anyPort && rule.EndPort-rule.StartPort+1 > maxRulePortRangeWidth { - return ruleShape{}, false + return ruleShape{}, true, false } shape.port = !anyPort } - return shape, true + return shape, true, true } -// What buildFlowDescription writes into the filter around an endpoint of several fields: the -// fields as the UPF reads them, the protocol, and the ports, which only TCP and UDP rules carry and -// which are left out when both are zero. -func filterInputs(fields []string, rule configmodels.SliceApplicationFilteringRules) string { - protocol, ports := "ip", "" - if rule.Protocol == protocolTCP || rule.Protocol == protocolUDP { - protocol = strconv.Itoa(int(rule.Protocol)) - if rule.StartPort != 0 || rule.EndPort != 0 { - ports = fmt.Sprintf("%d-%d", rule.StartPort, rule.EndPort) - } +// BuildSDFFilter renders the SDF filter installed for a rule: the flow description the UPF receives +// and splits into the fields it matches on. backend/nfconfig sends this to the datapath, and +// validateUpfRuleShapes keys a rule's PDR mask on it (through canonicalFlow). Rendering it in one +// place keeps the shape count from drifting away from the filter actually installed. +func BuildSDFFilter(rule configmodels.SliceApplicationFilteringRules) string { + endpoint := rule.Endpoint + if strings.HasPrefix(endpoint, "0.0.0.0") { + endpoint = anyEndpoint + } + var protocol string + switch rule.Protocol { + case protocolTCP: + protocol = "tcp" + case protocolUDP: + protocol = "udp" + default: + // Only TCP and UDP carry ports into the filter. + return fmt.Sprintf("permit out ip from %s to assigned", endpoint) } - return strings.Join(fields, " ") + "|" + protocol + "|" + ports + switch { + case rule.StartPort == 0 && rule.EndPort == 0: + return fmt.Sprintf("permit out %s from %s to assigned", protocol, endpoint) + case factory.WebUIConfig.Configuration.SdfComp: + return fmt.Sprintf("permit out %s from %s %d-%d to assigned", protocol, endpoint, rule.StartPort, rule.EndPort) + default: + return fmt.Sprintf("permit out %s from %s to assigned %d-%d", protocol, endpoint, rule.StartPort, rule.EndPort) + } +} + +// canonicalFlow is a rule's SDF filter tokenised the way the UPF reads it with strings.Fields, so +// two rules the UPF installs as the same filter compare equal despite incidental whitespace. +func canonicalFlow(rule configmodels.SliceApplicationFilteringRules) string { + return strings.Join(strings.Fields(BuildSDFFilter(rule)), " ") } func upfNameOf(slice configmodels.Slice) string { @@ -353,15 +381,28 @@ func validateUpfRuleShapes(request configmodels.Slice, sliceName string) (int, e return http.StatusInternalServerError, fmt.Errorf("failed to look up network slices: %w", err) } shapes := map[ruleShape]struct{}{} + // The filters written by rules whose mask is known. An endpoint of several fields is deferred + // until these are all collected, so it can share a modelled rule's shape when it writes the + // same filter -- the UPF installs one mask for the two -- rather than always counting as one of + // its own. + modeledFlows := map[string]struct{}{} + var deferredFlows []string add := func(slice configmodels.Slice) { if len(slice.ApplicationFilteringRules) == 0 { shapes[ruleShape{}] = struct{}{} return } for _, rule := range slice.ApplicationFilteringRules { - if shape, ok := ruleShapeOf(rule); ok { - shapes[shape] = struct{}{} + shape, modelable, counts := ruleShapeOf(rule) + if !modelable { + deferredFlows = append(deferredFlows, canonicalFlow(rule)) + continue } + if !counts { + continue + } + shapes[shape] = struct{}{} + modeledFlows[canonicalFlow(rule)] = struct{}{} } } add(request) @@ -370,6 +411,11 @@ func validateUpfRuleShapes(request configmodels.Slice, sliceName string) (int, e add(*slice) } } + for _, flow := range deferredFlows { + if _, ok := modeledFlows[flow]; !ok { + shapes[ruleShape{unknown: flow}] = struct{}{} + } + } if len(shapes) > maxUpfRuleShapes { return http.StatusBadRequest, fmt.Errorf("network slice %s would bring the filtering rules on UPF %s to %d distinct shapes, more than the %d the UPF can hold; "+ "a shape is the endpoint's prefix length, whether a protocol is given and whether a port is given", sliceName, upf, len(shapes), maxUpfRuleShapes) diff --git a/configapi/slice_operations_test.go b/configapi/slice_operations_test.go index 6c01231c..b0d927e6 100644 --- a/configapi/slice_operations_test.go +++ b/configapi/slice_operations_test.go @@ -1232,6 +1232,12 @@ func sevenPrefixRules() []configmodels.SliceApplicationFilteringRules { // The UPF's table is shared by every slice it serves, so the shapes are counted across them. func TestValidateUpfRuleShapes(t *testing.T) { + // canonicalFlow reads the spec-compliant-SDF flag to place a rule's ports as the UPF receives + // them; pin it so the shapes do not depend on whatever a prior test left in the global config. + origConfig := factory.WebUIConfig + defer func() { factory.WebUIConfig = origConfig }() + factory.WebUIConfig = &factory.Config{Configuration: &factory.Configuration{}} + allowAll := ruleTo("0.0.0.0/0", 0, 0, 0) testCases := []struct { name string @@ -1497,7 +1503,7 @@ func TestValidateUpfRuleShapes(t *testing.T) { expectedCode: http.StatusBadRequest, }, { - // buildFlowDescription writes only an endpoint beginning 0.0.0.0 as "any"; with a + // BuildSDFFilter writes only an endpoint beginning 0.0.0.0 as "any"; with a // space before it, the UPF reads the prefix as written. Nine shapes. name: "a stored 0.0.0.0 prefix after whitespace counts by its prefix", stored: []configmodels.Slice{sliceOnUpf("b", "upf1", ruleTo(" 0.0.0.0/8", 0, 0, 0))}, @@ -1539,6 +1545,48 @@ func TestValidateUpfRuleShapes(t *testing.T) { } } +// A stored endpoint of several fields shares a modelled rule's shape when it writes the same SDF +// filter: the UPF installs one mask for the two, so counting them apart would reject a slice that +// fits. With spec-compliant-SDF the ports sit right after the endpoint, so a legacy +// "10.0.0.0/8 80-80" carrying no ports of its own writes exactly what a clean "10.0.0.0/8" with +// ports 80-80 does. Without it the ports sit after "to assigned", so the two filters differ and the +// rules do count apart. +func TestValidateUpfRuleShapesFoldsLegacyEndpointWritingTheSameFilter(t *testing.T) { + origConfig := factory.WebUIConfig + defer func() { factory.WebUIConfig = origConfig }() + + modeled := ruleTo("10.0.0.0/8", protocolTCP, 80, 80) + legacy := ruleTo("10.0.0.0/8 80-80", protocolTCP, 0, 0) + // Seven prefix shapes, then an eighth from the modelled rule's port; the legacy rule is the one + // that either folds into the eighth or becomes a ninth. + posted := sliceOnUpf("a", "upf1", append(sevenPrefixRules(), modeled, legacy)...) + + testCases := []struct { + name string + specCompliant bool + expectedCode int + }{ + {"spec-compliant SDF writes the same filter for both", true, http.StatusOK}, + {"legacy SDF writes a different filter for each", false, http.StatusBadRequest}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + factory.WebUIConfig = &factory.Config{Configuration: &factory.Configuration{SdfComp: tc.specCompliant}} + originalDBClient := dbadapter.CommonDBClient + defer func() { dbadapter.CommonDBClient = originalDBClient }() + dbadapter.CommonDBClient = &NetworkSliceMockDBClient{} + + code, err := validateUpfRuleShapes(posted, posted.SliceName) + if code != tc.expectedCode { + t.Errorf("expected `%v`, got `%v` (%v)", tc.expectedCode, code, err) + } + if (err != nil) != (tc.expectedCode != http.StatusOK) { + t.Errorf("unexpected error %v for status %v", err, code) + } + }) + } +} + // The count needs every slice on the UPF; without them it cannot say the slice fits. func TestValidateUpfRuleShapesReportsAFailedLookup(t *testing.T) { originalDBClient := dbadapter.CommonDBClient From 576c029ee4d7eed186981233d47c49415f4bbc90 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Tue, 6 Oct 2026 15:03:45 -0700 Subject: [PATCH 2/2] Address review comments Signed-off-by: Arrobo, Gabriel --- configapi/slice_helpers.go | 10 +++---- configapi/slice_operations_test.go | 44 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/configapi/slice_helpers.go b/configapi/slice_helpers.go index 05f48413..ebad6e84 100644 --- a/configapi/slice_helpers.go +++ b/configapi/slice_helpers.go @@ -381,10 +381,10 @@ func validateUpfRuleShapes(request configmodels.Slice, sliceName string) (int, e return http.StatusInternalServerError, fmt.Errorf("failed to look up network slices: %w", err) } shapes := map[ruleShape]struct{}{} - // The filters written by rules whose mask is known. An endpoint of several fields is deferred - // until these are all collected, so it can share a modelled rule's shape when it writes the - // same filter -- the UPF installs one mask for the two -- rather than always counting as one of - // its own. + // The filters written by the modelable rules. An endpoint of several fields is deferred until + // these are all collected, so it takes no shape of its own when it writes the same filter as one + // of them: the UPF reads one filter for the two and either installs a single mask for both or + // refuses both alike, rather than the deferred copy counting as a shape it never installs. modeledFlows := map[string]struct{}{} var deferredFlows []string add := func(slice configmodels.Slice) { @@ -398,11 +398,11 @@ func validateUpfRuleShapes(request configmodels.Slice, sliceName string) (int, e deferredFlows = append(deferredFlows, canonicalFlow(rule)) continue } + modeledFlows[canonicalFlow(rule)] = struct{}{} if !counts { continue } shapes[shape] = struct{}{} - modeledFlows[canonicalFlow(rule)] = struct{}{} } } add(request) diff --git a/configapi/slice_operations_test.go b/configapi/slice_operations_test.go index b0d927e6..521c0fcb 100644 --- a/configapi/slice_operations_test.go +++ b/configapi/slice_operations_test.go @@ -1587,6 +1587,50 @@ func TestValidateUpfRuleShapesFoldsLegacyEndpointWritingTheSameFilter(t *testing } } +// A modelable rule the UPF refuses before installing anything -- a port range too wide -- takes no +// mask, and a legacy endpoint writing the same filter is refused the same way, so it must take no +// shape either. With spec-compliant-SDF the ports sit right after the endpoint, so a legacy +// "10.0.0.0/8 1000-1100" carrying no ports of its own writes exactly what a clean "10.0.0.0/8" with +// the oversized range 1000-1100 does; the legacy copy folds into the refused rule and adds nothing. +// Without it the filters differ, so the legacy copy still counts as a shape of its own. +func TestValidateUpfRuleShapesFoldsLegacyEndpointOntoARefusedFilter(t *testing.T) { + origConfig := factory.WebUIConfig + defer func() { factory.WebUIConfig = origConfig }() + + // The range is wider than maxRulePortRangeWidth, so the UPF refuses the rule before installing a + // mask and it takes no shape of its own. + refused := ruleTo("10.0.0.0/8", protocolTCP, 1000, 1100) + legacy := ruleTo("10.0.0.0/8 1000-1100", protocolTCP, 0, 0) + // Seven prefix shapes and any traffic make eight; the legacy rule either folds onto the refused + // filter and adds nothing or becomes a ninth. + posted := sliceOnUpf("a", "upf1", append(sevenPrefixRules(), ruleTo("0.0.0.0/0", 0, 0, 0), refused, legacy)...) + + testCases := []struct { + name string + specCompliant bool + expectedCode int + }{ + {"spec-compliant SDF writes the refused filter for both", true, http.StatusOK}, + {"legacy SDF writes a different filter for each", false, http.StatusBadRequest}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + factory.WebUIConfig = &factory.Config{Configuration: &factory.Configuration{SdfComp: tc.specCompliant}} + originalDBClient := dbadapter.CommonDBClient + defer func() { dbadapter.CommonDBClient = originalDBClient }() + dbadapter.CommonDBClient = &NetworkSliceMockDBClient{} + + code, err := validateUpfRuleShapes(posted, posted.SliceName) + if code != tc.expectedCode { + t.Errorf("expected `%v`, got `%v` (%v)", tc.expectedCode, code, err) + } + if (err != nil) != (tc.expectedCode != http.StatusOK) { + t.Errorf("unexpected error %v for status %v", err, code) + } + }) + } +} + // The count needs every slice on the UPF; without them it cannot say the slice fits. func TestValidateUpfRuleShapesReportsAFailedLookup(t *testing.T) { originalDBClient := dbadapter.CommonDBClient