Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport] fix: use only 46 bits for priorities of Kong routes (#5024) #5124

Merged
merged 1 commit into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/dataplane/parser/translators/atc_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import (
)

const (
// FromResourceKindPriorityShiftBits is the highest 2 bits 51-50 used in priority field of Kong route
// FromResourceKindPriorityShiftBits is the highest 2 bits 45-44 used in priority field of Kong route
// to note the kind of the resource from which the route is translated.
// 11 - routes from Ingress.
// 10 - routes from HTTPRoute.
// 01 - routes from GRPCRoute.
FromResourceKindPriorityShiftBits = 50
FromResourceKindPriorityShiftBits = 44
// ResourceKindBitsIngress is the value of highest 2 bits for routes from ingresses.
ResourceKindBitsIngress = 3
// ResourceKindBitsHTTPRoute is the value of highest 2 bits for routes from HTTPRoutes.
Expand Down
46 changes: 23 additions & 23 deletions internal/dataplane/parser/translators/grpcroute_atc.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,11 @@ func CalculateGRCPRouteMatchPriorityTraits(match SplitGRPCRouteMatch) GRPCRouteP

// EncodeToPriority turns GRPCRoute priority traits into the integer expressed priority.
//
// 4 3 2 1
// 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
// +-+---------------+---------------------+---------------------+---------+---------------------------+
// |P| host len | GRPC service length | GRPC method length |Header No| relative order |
// +-+---------------+---------------------+---------------------+---------+---------------------------+
// 4 3 2 1
// 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
// +-+---------------+---------------------+---------------------+---------+----------------+
// |P| host len | GRPC service length | GRPC method length |Header No| relative order |
// +-+---------------+---------------------+---------------------+---------+----------------+
//
// Where:
// P: set to 1 if the hostname is non-wildcard.
Expand All @@ -304,17 +304,17 @@ func CalculateGRCPRouteMatchPriorityTraits(match SplitGRPCRouteMatch) GRPCRouteP
// to assign higher priority for method match with `Exact` match?
func (t GRPCRoutePriorityTraits) EncodeToPriority() RoutePriorityType {
const (
// PreciseHostnameShiftBits assigns bit 49 for marking if the hostname is non-wildcard.
PreciseHostnameShiftBits = 49
// HostnameLengthShiftBits assigns bits 41-48 for the length of hostname.
HostnameLengthShiftBits = 41
// ServiceLengthShiftBits assigns bits 30-40 for the length of `Service` in method match.
ServiceLengthShiftBits = 30
// MethodLengthShiftBits assigns bits 19-29 for the length of `Method` in method match.
MethodLengthShiftBits = 19
// HeaderCountShiftBits assigns bits 14-18 for the number of header matches.
HeaderCountShiftBits = 14
// bits 0-13 are used for relative order of creation timestamp, namespace/name, and internal order of rules and matches.
// PreciseHostnameShiftBits assigns bit 43 for marking if the hostname is non-wildcard.
PreciseHostnameShiftBits = 43
// HostnameLengthShiftBits assigns bits 35-42 for the length of hostname.
HostnameLengthShiftBits = 35
// ServiceLengthShiftBits assigns bits 24-34 for the length of `Service` in method match.
ServiceLengthShiftBits = 24
// MethodLengthShiftBits assigns bits 13-23 for the length of `Method` in method match.
MethodLengthShiftBits = 13
// HeaderCountShiftBits assigns bits 8-12 for the number of header matches.
HeaderCountShiftBits = 8
// bits 0-7 are used for relative order of creation timestamp, namespace/name, and internal order of rules and matches.
// the bits are calculated by sorting GRPCRoutes with the same priority calculated from the fields above
// and start from all 1s, then decrease by one for each GRPCRoute.
)
Expand Down Expand Up @@ -366,11 +366,11 @@ func AssignRoutePriorityToSplitGRPCRouteMatches(

splitGRPCRoutesToPriority := make([]SplitGRPCRouteMatchToPriority, 0, len(splitGRPCouteMatches))

// Bits 0-13 (14 bits) are assigned for relative order of GRPCRoutes.
// Bits 0-7 (8 bits) are assigned for relative order of GRPCRoutes.
// If multiple GRPCRoutes are assigned to the same priority in the previous step,
// sort them then starts with 2^14 -1 and decrease by one for each GRPCRoute;
// sort them then starts with 2^8 -1 and decrease by one for each GRPCRoute;
// If only one GRPCRoute occupies the priority, fill the relative order bits with all 1s.
const relativeOrderAssignedBits = 14
const relativeOrderAssignedBits = 8
const defaultRelativeOrderPriorityBits RoutePriorityType = (1 << relativeOrderAssignedBits) - 1

for priority, matches := range priorityToSplitGRPCRouteMatches {
Expand All @@ -389,7 +389,7 @@ func AssignRoutePriorityToSplitGRPCRouteMatches(

for i, match := range matches {
relativeOrderBits := defaultRelativeOrderPriorityBits - RoutePriorityType(i)
// Although it is very unlikely that there are 2^14 = 16384 GRPCRoutes
// Although it is very unlikely that there are 2^8 = 256 GRPCRoutes
// should be given priority by their relative order, here we limit the
// relativeOrderBits to be at least 0.
if relativeOrderBits <= 0 {
Expand All @@ -401,10 +401,10 @@ func AssignRoutePriorityToSplitGRPCRouteMatches(
})
}

// Just in case, log a very unlikely scenario where we have more than 2^14 routes with the same base
// Just in case, log a very unlikely scenario where we have more than 2^8 routes with the same base
// priority and we have no bit space for them to be deterministically ordered.
if len(matches) > (1 << 14) {
logger.V(util.WarnLevel).Info("Too many GRPCRoute matches to be deterministically ordered", "grpcroute_number", len(matches))
if len(matches) > (1 << 8) {
logger.Error(nil, "Too many GRPCRoute matches to be deterministically ordered", "grpcroute_number", len(matches))
}
}

Expand Down
18 changes: 9 additions & 9 deletions internal/dataplane/parser/translators/grpcroute_atc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ func TestGRPCRouteTraitsEncodeToPriority(t *testing.T) {
HostnameLength: 15,
ServiceLength: 7,
},
exprectedPriority: (1 << 50) | (1 << 49) | (15 << 41) | (7 << 30),
exprectedPriority: (1 << 44) | (1 << 43) | (15 << 35) | (7 << 24),
},
{
name: "non precise hostname",
Expand All @@ -625,7 +625,7 @@ func TestGRPCRouteTraitsEncodeToPriority(t *testing.T) {
MethodLength: 7,
HeaderCount: 3,
},
exprectedPriority: (1 << 50) | (15 << 41) | (7 << 30) | (7 << 19) | (3 << 14),
exprectedPriority: (1 << 44) | (15 << 35) | (7 << 24) | (7 << 13) | (3 << 8),
},
}

Expand All @@ -648,7 +648,7 @@ func TestAssignRoutePriorityToSplitGRPCRouteMatches(t *testing.T) {
matchIndex int
}
now := time.Now()
const maxRelativeOrderPriorityBits = (1 << 14) - 1
const maxRelativeOrderPriorityBits = (1 << 8) - 1

testCases := []struct {
name string
Expand Down Expand Up @@ -1124,14 +1124,14 @@ func TestKongExpressionRouteFromSplitGRPCRouteWithPriority(t *testing.T) {
RuleIndex: 0,
MatchIndex: 0,
},
Priority: 1024,
Priority: 1 << 14,
},
expectedRoute: kongstate.Route{
Route: kong.Route{
Name: kong.String("grpcroute.default.no-hostname-exact-method._.0.0"),
PreserveHost: kong.Bool(true),
Expression: kong.String(`http.path == "/pets/list"`),
Priority: kong.Uint64(1024),
Priority: kong.Uint64(1 << 14),
},
ExpressionRoutes: true,
},
Expand Down Expand Up @@ -1176,14 +1176,14 @@ func TestKongExpressionRouteFromSplitGRPCRouteWithPriority(t *testing.T) {
RuleIndex: 0,
MatchIndex: 0,
},
Priority: 1024,
Priority: (1 << 31) + 1,
},
expectedRoute: kongstate.Route{
Route: kong.Route{
Name: kong.String("grpcroute.default.precise-hostname-regex-method.foo.com.0.0"),
Expression: kong.String(`(http.path ~ "^/name/[a-z0-9]+") && (http.host == "foo.com")`),
PreserveHost: kong.Bool(true),
Priority: kong.Uint64(1024),
Priority: kong.Uint64((1 << 31) + 1),
},
ExpressionRoutes: true,
},
Expand Down Expand Up @@ -1245,14 +1245,14 @@ func TestKongExpressionRouteFromSplitGRPCRouteWithPriority(t *testing.T) {
RuleIndex: 0,
MatchIndex: 1,
},
Priority: 1024,
Priority: (1 << 42) + 1,
},
expectedRoute: kongstate.Route{
Route: kong.Route{
Name: kong.String("grpcroute.default.wildcard-hostname-header-match._.foo.com.0.1"),
Expression: kong.String(`(http.path ^= "/name/") && (http.headers.foo == "bar") && (http.host =^ ".foo.com")`),
PreserveHost: kong.Bool(true),
Priority: kong.Uint64(1024),
Priority: kong.Uint64((1 << 42) + 1),
},
ExpressionRoutes: true,
},
Expand Down
58 changes: 29 additions & 29 deletions internal/dataplane/parser/translators/httproute_atc.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,11 @@ func CalculateHTTPRouteMatchPriorityTraits(match SplitHTTPRouteMatch) HTTPRouteP

// EncodeToPriority turns HTTPRoute priority traits into the integer expressed priority.
//
// 4 3 2 1
// 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
// +-+---------------+-+-+-------------------+-+---------+---------+-----------------------------------+
// |P| host len |E|R| Path length |M|Header No|Query No.| relative order |
// +-+---------------+-+-+-------------------+-+---------+-------- +-----------------------------------+
// 4 3 2 1
// 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
// +-+---------------+-+-+-------------------+-+---------+---------+-----------------------+
// |P| host len |E|R| Path length |M|Header No|Query No.| relative order |
// +-+---------------+-+-+-------------------+-+---------+-------- +-----------------------+
//
// Where:
// P: set to 1 if the hostname is non-wildcard.
Expand All @@ -365,23 +365,23 @@ func CalculateHTTPRouteMatchPriorityTraits(match SplitHTTPRouteMatch) HTTPRouteP
// relative order: relative order of creation timestamp, namespace and name and internal rule/match order between different (split) HTTPRoutes.
func (t HTTPRoutePriorityTraits) EncodeToPriority() RoutePriorityType {
const (
// PreciseHostnameShiftBits assigns bit 49 for marking if the hostname is non-wildcard.
PreciseHostnameShiftBits = 49
// HostnameLengthShiftBits assigns bits 41-48 for the length of hostname.
HostnameLengthShiftBits = 41
// ExactPathShiftBits assigns bit 40 to mark if the match is exact path match.
ExactPathShiftBits = 40
// RegularExpressionPathShiftBits assigns bit 39 to mark if the match is regex path match.
RegularExpressionPathShiftBits = 39
// PathLengthShiftBits assigns bits 29-38 to path length. (max length = 1024, but must start with /)
PathLengthShiftBits = 29
// MethodMatchShiftBits assigns bit 28 to mark if method is specified.
MethodMatchShiftBits = 28
// HeaderNumberShiftBits assign bits 23-27 to number of headers. (max number of headers = 16)
HeaderNumberShiftBits = 23
// QueryParamNumberShiftBits makes bits 18-22 used for number of query params (max number of query params = 16)
QueryParamNumberShiftBits = 18
// bits 0-17 are used for relative order of creation timestamp, namespace/name, and internal order of rules and matches.
// PreciseHostnameShiftBits assigns bit 43 for marking if the hostname is non-wildcard.
PreciseHostnameShiftBits = 43
// HostnameLengthShiftBits assigns bits 35-42 for the length of hostname.
HostnameLengthShiftBits = 35
// ExactPathShiftBits assigns bit 34 to mark if the match is exact path match.
ExactPathShiftBits = 34
// RegularExpressionPathShiftBits assigns bit 33 to mark if the match is regex path match.
RegularExpressionPathShiftBits = 33
// PathLengthShiftBits assigns bits 23-32 to path length. (max length = 1024, but must start with /)
PathLengthShiftBits = 23
// MethodMatchShiftBits assigns bit 22 to mark if method is specified.
MethodMatchShiftBits = 22
// HeaderNumberShiftBits assign bits 17-21 to number of headers. (max number of headers = 16)
HeaderNumberShiftBits = 17
// QueryParamNumberShiftBits makes bits 12-16 used for number of query params (max number of query params = 16)
QueryParamNumberShiftBits = 12
// bits 0-11 are used for relative order of creation timestamp, namespace/name, and internal order of rules and matches.
// the bits are calculated by sorting HTTPRoutes with the same priority calculated from the fields above
// and start from all 1s, then decrease by one for each HTTPRoute.
)
Expand Down Expand Up @@ -434,11 +434,11 @@ func AssignRoutePriorityToSplitHTTPRouteMatches(

httpRouteMatchesToPriorities := make([]SplitHTTPRouteMatchToKongRoutePriority, 0, len(splitHTTPRouteMatches))

// Bits 0-17 (18 bits) are assigned for relative order of matches.
// Bits 0-11 (12 bits) are assigned for relative order of matches.
// If multiple matches are assigned to the same priority in the previous step,
// sort them then starts with 2^18 -1 and decrease by one for each HTTPRoute;
// sort them then starts with 2^12 -1 and decrease by one for each HTTPRoute;
// If only one match occupies the priority, fill the relative order bits with all 1s.
const RelativeOrderAssignedBits = 18
const RelativeOrderAssignedBits = 12
const defaultRelativeOrderPriorityBits = (uint64(1) << RelativeOrderAssignedBits) - 1
for priority, matches := range priorityToSplitHTTPRouteMatches {
if len(matches) == 1 {
Expand All @@ -455,7 +455,7 @@ func AssignRoutePriorityToSplitHTTPRouteMatches(

for i, match := range matches {
relativeOrderBits := defaultRelativeOrderPriorityBits - RoutePriorityType(i)
// Although it is very unlikely that there are 2^18 = 262144 HTTPRoutes
// Although it is very unlikely that there are 2^12 = 4096 HTTPRoutes
// should be given priority by their relative order, here we limit the
// relativeOrderBits to be at least 0.
if relativeOrderBits <= 0 {
Expand All @@ -466,10 +466,10 @@ func AssignRoutePriorityToSplitHTTPRouteMatches(
Priority: priority + relativeOrderBits,
})
}
// Just in case, log a very unlikely scenario where we have more than 2^18 matches with the same base
// Just in case, log a very unlikely scenario where we have more than 2^12 matches with the same base
// priority and we have no bit space for them to be deterministically ordered.
if len(matches) > (1 << 18) {
logger.V(util.WarnLevel).Info("Too many HTTPRoute matches to be deterministically ordered", "match_number", len(matches))
if len(matches) > (1 << 12) {
logger.Error(nil, "Too many HTTPRoute matches to be deterministically ordered", "match_number", len(matches))
}
}

Expand Down
Loading