Skip to content

Commit

Permalink
TUN-8861: Rename Session Limiter to Flow Limiter
Browse files Browse the repository at this point in the history
## Summary
Session is the concept used for UDP flows. Therefore, to make
the session limiter ambiguous for both TCP and UDP, this commit
renames it to flow limiter.

Closes TUN-8861
  • Loading branch information
jcsf committed Jan 20, 2025
1 parent 8c2eda1 commit 4eb0f8c
Show file tree
Hide file tree
Showing 23 changed files with 295 additions and 295 deletions.
4 changes: 2 additions & 2 deletions connection/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
pkgerrors "github.com/pkg/errors"
"github.com/rs/zerolog"

cfdsession "github.com/cloudflare/cloudflared/session"
cfdflow "github.com/cloudflare/cloudflared/flow"

"github.com/cloudflare/cloudflared/stream"
"github.com/cloudflare/cloudflared/tracing"
Expand Down Expand Up @@ -107,7 +107,7 @@ func (moc *mockOriginProxy) ProxyTCP(
r *TCPRequest,
) error {
if r.CfTraceID == "flow-rate-limited" {
return pkgerrors.Wrap(cfdsession.ErrTooManyActiveSessions, "tcp flow rate limited")
return pkgerrors.Wrap(cfdflow.ErrTooManyActiveFlows, "tcp flow rate limited")
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions connection/http2.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/rs/zerolog"
"golang.org/x/net/http2"

cfdsession "github.com/cloudflare/cloudflared/session"
cfdflow "github.com/cloudflare/cloudflared/flow"

"github.com/cloudflare/cloudflared/tracing"
tunnelpogs "github.com/cloudflare/cloudflared/tunnelrpc/pogs"
Expand Down Expand Up @@ -336,7 +336,7 @@ func (rp *http2RespWriter) WriteErrorResponse(err error) bool {
return false
}

if errors.Is(err, cfdsession.ErrTooManyActiveSessions) {
if errors.Is(err, cfdflow.ErrTooManyActiveFlows) {
rp.setResponseMetaHeader(responseMetaHeaderCfdFlowRateLimited)
} else {
rp.setResponseMetaHeader(responseMetaHeaderCfd)
Expand Down
4 changes: 2 additions & 2 deletions connection/quic_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/rs/zerolog"
"golang.org/x/sync/errgroup"

cfdsession "github.com/cloudflare/cloudflared/session"
cfdflow "github.com/cloudflare/cloudflared/flow"

cfdquic "github.com/cloudflare/cloudflared/quic"
"github.com/cloudflare/cloudflared/tracing"
Expand Down Expand Up @@ -185,7 +185,7 @@ func (q *quicConnection) handleDataStream(ctx context.Context, stream *rpcquic.R

var metadata []pogs.Metadata
// Check the type of error that was throw and add metadata that will help identify it on OTD.
if errors.Is(err, cfdsession.ErrTooManyActiveSessions) {
if errors.Is(err, cfdflow.ErrTooManyActiveFlows) {
metadata = append(metadata, pogs.ErrorFlowConnectRateLimitedKey)
}

Expand Down
6 changes: 3 additions & 3 deletions connection/quic_connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"github.com/stretchr/testify/require"
"golang.org/x/net/nettest"

cfdsession "github.com/cloudflare/cloudflared/session"
cfdflow "github.com/cloudflare/cloudflared/flow"

"github.com/cloudflare/cloudflared/datagramsession"
"github.com/cloudflare/cloudflared/ingress"
Expand Down Expand Up @@ -508,7 +508,7 @@ func TestBuildHTTPRequest(t *testing.T) {

func (moc *mockOriginProxyWithRequest) ProxyTCP(ctx context.Context, rwa ReadWriteAcker, tcpRequest *TCPRequest) error {
if tcpRequest.Dest == "rate-limit-me" {
return pkgerrors.Wrap(cfdsession.ErrTooManyActiveSessions, "failed tcp stream")
return pkgerrors.Wrap(cfdflow.ErrTooManyActiveFlows, "failed tcp stream")
}

_ = rwa.AckConnection("")
Expand Down Expand Up @@ -828,7 +828,7 @@ func testTunnelConnection(t *testing.T, serverAddr netip.AddrPort, index uint8)
conn,
index,
sessionManager,
cfdsession.NewLimiter(0),
cfdflow.NewLimiter(0),
datagramMuxer,
packetRouter,
15 * time.Second,
Expand Down
18 changes: 9 additions & 9 deletions connection/quic_datagram_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"go.opentelemetry.io/otel/trace"
"golang.org/x/sync/errgroup"

cfdsession "github.com/cloudflare/cloudflared/session"
cfdflow "github.com/cloudflare/cloudflared/flow"

"github.com/cloudflare/cloudflared/datagramsession"
"github.com/cloudflare/cloudflared/ingress"
Expand Down Expand Up @@ -46,8 +46,8 @@ type datagramV2Connection struct {

// sessionManager tracks active sessions. It receives datagrams from quic connection via datagramMuxer
sessionManager datagramsession.Manager
// sessionLimiter tracks active sessions across the tunnel and limits new sessions if they are above the limit.
sessionLimiter cfdsession.Limiter
// flowLimiter tracks active sessions across the tunnel and limits new sessions if they are above the limit.
flowLimiter cfdflow.Limiter

// datagramMuxer mux/demux datagrams from quic connection
datagramMuxer *cfdquic.DatagramMuxerV2
Expand All @@ -65,7 +65,7 @@ func NewDatagramV2Connection(ctx context.Context,
index uint8,
rpcTimeout time.Duration,
streamWriteTimeout time.Duration,
sessionLimiter cfdsession.Limiter,
flowLimiter cfdflow.Limiter,
logger *zerolog.Logger,
) DatagramSessionHandler {
sessionDemuxChan := make(chan *packet.Session, demuxChanCapacity)
Expand All @@ -77,7 +77,7 @@ func NewDatagramV2Connection(ctx context.Context,
conn: conn,
index: index,
sessionManager: sessionManager,
sessionLimiter: sessionLimiter,
flowLimiter: flowLimiter,
datagramMuxer: datagramMuxer,
packetRouter: packetRouter,
rpcTimeout: rpcTimeout,
Expand Down Expand Up @@ -121,7 +121,7 @@ func (q *datagramV2Connection) RegisterUdpSession(ctx context.Context, sessionID
log := q.logger.With().Int(management.EventTypeKey, int(management.UDP)).Logger()

// Try to start a new session
if err := q.sessionLimiter.Acquire(management.UDP.String()); err != nil {
if err := q.flowLimiter.Acquire(management.UDP.String()); err != nil {
log.Warn().Msgf("Too many concurrent sessions being handled, rejecting udp proxy to %s:%d", dstIP, dstPort)

err := pkgerrors.Wrap(err, "failed to start udp session due to rate limiting")
Expand All @@ -135,7 +135,7 @@ func (q *datagramV2Connection) RegisterUdpSession(ctx context.Context, sessionID
if err != nil {
log.Err(err).Msgf("Failed to create udp proxy to %s:%d", dstIP, dstPort)
tracing.EndWithErrorStatus(registerSpan, err)
q.sessionLimiter.Release()
q.flowLimiter.Release()
return nil, err
}
registerSpan.SetAttributes(
Expand All @@ -148,12 +148,12 @@ func (q *datagramV2Connection) RegisterUdpSession(ctx context.Context, sessionID
originProxy.Close()
log.Err(err).Str(datagramsession.LogFieldSessionID, datagramsession.FormatSessionID(sessionID)).Msgf("Failed to register udp session")
tracing.EndWithErrorStatus(registerSpan, err)
q.sessionLimiter.Release()
q.flowLimiter.Release()
return nil, err
}

go func() {
defer q.sessionLimiter.Release() // we do the release here, instead of inside the `serveUDPSession` just to keep all acquire/release calls in the same method.
defer q.flowLimiter.Release() // we do the release here, instead of inside the `serveUDPSession` just to keep all acquire/release calls in the same method.
q.serveUDPSession(session, closeAfterIdleHint)
}()

Expand Down
12 changes: 6 additions & 6 deletions connection/quic_datagram_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

cfdflow "github.com/cloudflare/cloudflared/flow"
"github.com/cloudflare/cloudflared/mocks"
cfdsession "github.com/cloudflare/cloudflared/session"
)

type mockQuicConnection struct {
Expand Down Expand Up @@ -75,7 +75,7 @@ func TestRateLimitOnNewDatagramV2UDPSession(t *testing.T) {
log := zerolog.Nop()
conn := &mockQuicConnection{}
ctrl := gomock.NewController(t)
sessionLimiterMock := mocks.NewMockLimiter(ctrl)
flowLimiterMock := mocks.NewMockLimiter(ctrl)

datagramConn := NewDatagramV2Connection(
context.Background(),
Expand All @@ -84,13 +84,13 @@ func TestRateLimitOnNewDatagramV2UDPSession(t *testing.T) {
0,
0*time.Second,
0*time.Second,
sessionLimiterMock,
flowLimiterMock,
&log,
)

sessionLimiterMock.EXPECT().Acquire("udp").Return(cfdsession.ErrTooManyActiveSessions)
sessionLimiterMock.EXPECT().Release().Times(0)
flowLimiterMock.EXPECT().Acquire("udp").Return(cfdflow.ErrTooManyActiveFlows)
flowLimiterMock.EXPECT().Release().Times(0)

_, err := datagramConn.RegisterUdpSession(context.Background(), uuid.New(), net.IPv4(0, 0, 0, 0), 1000, 1*time.Second, "")
require.ErrorIs(t, err, cfdsession.ErrTooManyActiveSessions)
require.ErrorIs(t, err, cfdflow.ErrTooManyActiveFlows)
}
77 changes: 77 additions & 0 deletions flow/limiter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package flow

import (
"errors"
"sync"
)

const (
unlimitedActiveFlows = 0
)

var (
ErrTooManyActiveFlows = errors.New("too many active flows")
)

type Limiter interface {
// Acquire tries to acquire a free slot for a flow, if the value of flows is already above
// the maximum it returns ErrTooManyActiveFlows.
Acquire(flowType string) error
// Release releases a slot for a flow.
Release()
// SetLimit allows to hot swap the limit value of the limiter.
SetLimit(uint64)
}

type flowLimiter struct {
limiterLock sync.Mutex
activeFlowsCounter uint64
maxActiveFlows uint64
unlimited bool
}

func NewLimiter(maxActiveFlows uint64) Limiter {
flowLimiter := &flowLimiter{
maxActiveFlows: maxActiveFlows,
unlimited: isUnlimited(maxActiveFlows),
}

return flowLimiter
}

func (s *flowLimiter) Acquire(flowType string) error {
s.limiterLock.Lock()
defer s.limiterLock.Unlock()

if !s.unlimited && s.activeFlowsCounter >= s.maxActiveFlows {
flowRegistrationsDropped.WithLabelValues(flowType).Inc()
return ErrTooManyActiveFlows
}

s.activeFlowsCounter++
return nil
}

func (s *flowLimiter) Release() {
s.limiterLock.Lock()
defer s.limiterLock.Unlock()

if s.activeFlowsCounter <= 0 {
return
}

s.activeFlowsCounter--
}

func (s *flowLimiter) SetLimit(newMaxActiveFlows uint64) {
s.limiterLock.Lock()
defer s.limiterLock.Unlock()

s.maxActiveFlows = newMaxActiveFlows
s.unlimited = isUnlimited(newMaxActiveFlows)
}

// isUnlimited checks if the value received matches the configuration for the unlimited flow limiter.
func isUnlimited(value uint64) bool {
return value == unlimitedActiveFlows
}
119 changes: 119 additions & 0 deletions flow/limiter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package flow_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/cloudflare/cloudflared/flow"
)

func TestFlowLimiter_Unlimited(t *testing.T) {
unlimitedLimiter := flow.NewLimiter(0)

for i := 0; i < 1000; i++ {
err := unlimitedLimiter.Acquire("test")
require.NoError(t, err)
}
}

func TestFlowLimiter_Limited(t *testing.T) {
maxFlows := uint64(5)
limiter := flow.NewLimiter(maxFlows)

for i := uint64(0); i < maxFlows; i++ {
err := limiter.Acquire("test")
require.NoError(t, err)
}

err := limiter.Acquire("should fail")
require.ErrorIs(t, err, flow.ErrTooManyActiveFlows)
}

func TestFlowLimiter_AcquireAndReleaseFlow(t *testing.T) {
maxFlows := uint64(5)
limiter := flow.NewLimiter(maxFlows)

// Acquire the maximum number of flows
for i := uint64(0); i < maxFlows; i++ {
err := limiter.Acquire("test")
require.NoError(t, err)
}

// Validate acquire 1 more flows fails
err := limiter.Acquire("should fail")
require.ErrorIs(t, err, flow.ErrTooManyActiveFlows)

// Release the maximum number of flows
for i := uint64(0); i < maxFlows; i++ {
limiter.Release()
}

// Validate acquire 1 more flows works
err = limiter.Acquire("shouldn't fail")
require.NoError(t, err)

// Release a 10x the number of max flows
for i := uint64(0); i < 10*maxFlows; i++ {
limiter.Release()
}

// Validate it still can only acquire a value = number max flows.
for i := uint64(0); i < maxFlows; i++ {
err := limiter.Acquire("test")
require.NoError(t, err)
}
err = limiter.Acquire("should fail")
require.ErrorIs(t, err, flow.ErrTooManyActiveFlows)
}

func TestFlowLimiter_SetLimit(t *testing.T) {
maxFlows := uint64(5)
limiter := flow.NewLimiter(maxFlows)

// Acquire the maximum number of flows
for i := uint64(0); i < maxFlows; i++ {
err := limiter.Acquire("test")
require.NoError(t, err)
}

// Validate acquire 1 more flows fails
err := limiter.Acquire("should fail")
require.ErrorIs(t, err, flow.ErrTooManyActiveFlows)

// Set the flow limiter to support one more request
limiter.SetLimit(maxFlows + 1)

// Validate acquire 1 more flows now works
err = limiter.Acquire("shouldn't fail")
require.NoError(t, err)

// Validate acquire 1 more flows doesn't work because we already reached the limit
err = limiter.Acquire("should fail")
require.ErrorIs(t, err, flow.ErrTooManyActiveFlows)

// Release all flows
for i := uint64(0); i < maxFlows+1; i++ {
limiter.Release()
}

// Validate 1 flow works again
err = limiter.Acquire("shouldn't fail")
require.NoError(t, err)

// Set the flow limit to 1
limiter.SetLimit(1)

// Validate acquire 1 more flows doesn't work
err = limiter.Acquire("should fail")
require.ErrorIs(t, err, flow.ErrTooManyActiveFlows)

// Set the flow limit to unlimited
limiter.SetLimit(0)

// Validate it can acquire a lot of flows because it is now unlimited.
for i := uint64(0); i < 10*maxFlows; i++ {
err := limiter.Acquire("shouldn't fail")
require.NoError(t, err)
}
}
Loading

0 comments on commit 4eb0f8c

Please sign in to comment.