Skip to content

Commit 679a89c

Browse files
sudarshan-reddysudarshan-reddy
committed
TUN-6617: Dont fallback to http2 if QUIC conn was successful.
cloudflared falls back aggressively to HTTP/2 protocol if a connection attempt with QUIC failed. This was done to ensure that machines with UDP egress disabled did not stop clients from connecting to the cloudlfare edge. This PR improves on that experience by having cloudflared remember if a QUIC connection was successful which implies UDP egress works. In this case, cloudflared does not fallback to HTTP/2 and keeps trying to connect to the edge with QUIC.
1 parent a768132 commit 679a89c

File tree

11 files changed

+133
-87
lines changed

11 files changed

+133
-87
lines changed

‎connection/control.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package connection
22

33
import (
44
"context"
5+
"fmt"
56
"io"
67
"net"
78
"time"
@@ -21,6 +22,7 @@ type controlStream struct {
2122
namedTunnelProperties *NamedTunnelProperties
2223
connIndex uint8
2324
edgeAddress net.IP
25+
protocol Protocol
2426

2527
newRPCClientFunc RPCClientFunc
2628

@@ -51,6 +53,7 @@ func NewControlStream(
5153
newRPCClientFunc RPCClientFunc,
5254
gracefulShutdownC <-chan struct{},
5355
gracePeriod time.Duration,
56+
protocol Protocol,
5457
) ControlStreamHandler {
5558
if newRPCClientFunc == nil {
5659
newRPCClientFunc = newRegistrationRPCClient
@@ -64,6 +67,7 @@ func NewControlStream(
6467
edgeAddress: edgeAddress,
6568
gracefulShutdownC: gracefulShutdownC,
6669
gracePeriod: gracePeriod,
70+
protocol: protocol,
6771
}
6872
}
6973

@@ -80,6 +84,9 @@ func (c *controlStream) ServeControlStream(
8084
rpcClient.Close()
8185
return err
8286
}
87+
88+
c.observer.logServerInfo(c.connIndex, registrationDetails.Location, c.edgeAddress, fmt.Sprintf("Connection %s registered", registrationDetails.UUID))
89+
c.observer.sendConnectedEvent(c.connIndex, c.protocol, registrationDetails.Location)
8390
c.connectedFuse.Connected()
8491

8592
// if conn index is 0 and tunnel is not remotely managed, then send local ingress rules configuration

‎connection/event.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ type Event struct {
55
Index uint8
66
EventType Status
77
Location string
8+
Protocol Protocol
89
URL string
910
}
1011

‎connection/http2_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ func newTestHTTP2Connection() (*HTTP2Connection, net.Conn) {
4343
nil,
4444
nil,
4545
1*time.Second,
46+
HTTP2,
4647
)
4748
return NewHTTP2Connection(
4849
cfdConn,
@@ -366,6 +367,7 @@ func TestServeControlStream(t *testing.T) {
366367
rpcClientFactory.newMockRPCClient,
367368
nil,
368369
1*time.Second,
370+
HTTP2,
369371
)
370372
http2Conn.controlStreamHandler = controlStream
371373

@@ -417,6 +419,7 @@ func TestFailRegistration(t *testing.T) {
417419
rpcClientFactory.newMockRPCClient,
418420
nil,
419421
1*time.Second,
422+
HTTP2,
420423
)
421424
http2Conn.controlStreamHandler = controlStream
422425

@@ -464,6 +467,7 @@ func TestGracefulShutdownHTTP2(t *testing.T) {
464467
rpcClientFactory.newMockRPCClient,
465468
shutdownC,
466469
1*time.Second,
470+
HTTP2,
467471
)
468472

469473
http2Conn.controlStreamHandler = controlStream

‎connection/observer.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ func (o *Observer) sendRegisteringEvent(connIndex uint8) {
5555
o.sendEvent(Event{Index: connIndex, EventType: RegisteringTunnel})
5656
}
5757

58-
func (o *Observer) sendConnectedEvent(connIndex uint8, location string) {
59-
o.sendEvent(Event{Index: connIndex, EventType: Connected, Location: location})
58+
func (o *Observer) sendConnectedEvent(connIndex uint8, protocol Protocol, location string) {
59+
o.sendEvent(Event{Index: connIndex, EventType: Connected, Protocol: protocol, Location: location})
6060
}
6161

6262
func (o *Observer) SendURL(url string) {

‎connection/quic_test.go‎

Lines changed: 57 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -81,63 +81,63 @@ func TestQUICServer(t *testing.T) {
8181
},
8282
expectedResponse: []byte("OK"),
8383
},
84-
//{
85-
// desc: "test http body request streaming",
86-
// dest: "/slow_echo_body",
87-
// connectionType: quicpogs.ConnectionTypeHTTP,
88-
// metadata: []quicpogs.Metadata{
89-
// {
90-
// Key: "HttpHeader:Cf-Ray",
91-
// Val: "123123123",
92-
// },
93-
// {
94-
// Key: "HttpHost",
95-
// Val: "cf.host",
96-
// },
97-
// {
98-
// Key: "HttpMethod",
99-
// Val: "POST",
100-
// },
101-
// {
102-
// Key: "HttpHeader:Content-Length",
103-
// Val: "24",
104-
// },
105-
// },
106-
// message: []byte("This is the message body"),
107-
// expectedResponse: []byte("This is the message body"),
108-
//},
109-
//{
110-
// desc: "test ws proxy",
111-
// dest: "/ws/echo",
112-
// connectionType: quicpogs.ConnectionTypeWebsocket,
113-
// metadata: []quicpogs.Metadata{
114-
// {
115-
// Key: "HttpHeader:Cf-Cloudflared-Proxy-Connection-Upgrade",
116-
// Val: "Websocket",
117-
// },
118-
// {
119-
// Key: "HttpHeader:Another-Header",
120-
// Val: "Misc",
121-
// },
122-
// {
123-
// Key: "HttpHost",
124-
// Val: "cf.host",
125-
// },
126-
// {
127-
// Key: "HttpMethod",
128-
// Val: "get",
129-
// },
130-
// },
131-
// message: wsBuf.Bytes(),
132-
// expectedResponse: []byte{0x82, 0x5, 0x48, 0x65, 0x6c, 0x6c, 0x6f},
133-
//},
134-
//{
135-
// desc: "test tcp proxy",
136-
// connectionType: quicpogs.ConnectionTypeTCP,
137-
// metadata: []quicpogs.Metadata{},
138-
// message: []byte("Here is some tcp data"),
139-
// expectedResponse: []byte("Here is some tcp data"),
140-
//},
84+
{
85+
desc: "test http body request streaming",
86+
dest: "/slow_echo_body",
87+
connectionType: quicpogs.ConnectionTypeHTTP,
88+
metadata: []quicpogs.Metadata{
89+
{
90+
Key: "HttpHeader:Cf-Ray",
91+
Val: "123123123",
92+
},
93+
{
94+
Key: "HttpHost",
95+
Val: "cf.host",
96+
},
97+
{
98+
Key: "HttpMethod",
99+
Val: "POST",
100+
},
101+
{
102+
Key: "HttpHeader:Content-Length",
103+
Val: "24",
104+
},
105+
},
106+
message: []byte("This is the message body"),
107+
expectedResponse: []byte("This is the message body"),
108+
},
109+
{
110+
desc: "test ws proxy",
111+
dest: "/ws/echo",
112+
connectionType: quicpogs.ConnectionTypeWebsocket,
113+
metadata: []quicpogs.Metadata{
114+
{
115+
Key: "HttpHeader:Cf-Cloudflared-Proxy-Connection-Upgrade",
116+
Val: "Websocket",
117+
},
118+
{
119+
Key: "HttpHeader:Another-Header",
120+
Val: "Misc",
121+
},
122+
{
123+
Key: "HttpHost",
124+
Val: "cf.host",
125+
},
126+
{
127+
Key: "HttpMethod",
128+
Val: "get",
129+
},
130+
},
131+
message: wsBuf.Bytes(),
132+
expectedResponse: []byte{0x82, 0x5, 0x48, 0x65, 0x6c, 0x6c, 0x6f},
133+
},
134+
{
135+
desc: "test tcp proxy",
136+
connectionType: quicpogs.ConnectionTypeTCP,
137+
metadata: []quicpogs.Metadata{},
138+
message: []byte("Here is some tcp data"),
139+
expectedResponse: []byte("Here is some tcp data"),
140+
},
141141
}
142142

143143
for _, test := range tests {

‎connection/rpc.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package connection
22

33
import (
44
"context"
5-
"fmt"
65
"io"
76
"net"
87
"time"
@@ -117,9 +116,6 @@ func (rsc *registrationServerClient) RegisterConnection(
117116

118117
observer.metrics.regSuccess.WithLabelValues("registerConnection").Inc()
119118

120-
observer.logServerInfo(connIndex, conn.Location, edgeAddress, fmt.Sprintf("Connection %s registered", conn.UUID))
121-
observer.sendConnectedEvent(connIndex, conn.Location)
122-
123119
return conn, nil
124120
}
125121

‎metrics/readiness_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414

1515
func TestReadyServer_makeResponse(t *testing.T) {
1616
type fields struct {
17-
isConnected map[int]bool
17+
isConnected map[uint8]tunnelstate.ConnectionInfo
1818
}
1919
tests := []struct {
2020
name string
@@ -25,11 +25,11 @@ func TestReadyServer_makeResponse(t *testing.T) {
2525
{
2626
name: "One connection online => HTTP 200",
2727
fields: fields{
28-
isConnected: map[int]bool{
29-
0: false,
30-
1: false,
31-
2: true,
32-
3: false,
28+
isConnected: map[uint8]tunnelstate.ConnectionInfo{
29+
0: {IsConnected: false},
30+
1: {IsConnected: false},
31+
2: {IsConnected: true},
32+
3: {IsConnected: false},
3333
},
3434
},
3535
wantOK: true,
@@ -38,11 +38,11 @@ func TestReadyServer_makeResponse(t *testing.T) {
3838
{
3939
name: "No connections online => no HTTP 200",
4040
fields: fields{
41-
isConnected: map[int]bool{
42-
0: false,
43-
1: false,
44-
2: false,
45-
3: false,
41+
isConnected: map[uint8]tunnelstate.ConnectionInfo{
42+
0: {IsConnected: false},
43+
1: {IsConnected: false},
44+
2: {IsConnected: false},
45+
3: {IsConnected: false},
4646
},
4747
},
4848
wantReadyConnections: 0,

‎supervisor/conn_aware_logger.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ type ConnAwareLogger struct {
1212
logger *zerolog.Logger
1313
}
1414

15-
func NewConnAwareLogger(logger *zerolog.Logger, observer *connection.Observer) *ConnAwareLogger {
15+
func NewConnAwareLogger(logger *zerolog.Logger, tracker *tunnelstate.ConnTracker, observer *connection.Observer) *ConnAwareLogger {
1616
connAwareLogger := &ConnAwareLogger{
17-
tracker: tunnelstate.NewConnTracker(logger),
17+
tracker: tracker,
1818
logger: logger,
1919
}
2020

‎supervisor/supervisor.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/cloudflare/cloudflared/retry"
2020
"github.com/cloudflare/cloudflared/signal"
2121
tunnelpogs "github.com/cloudflare/cloudflared/tunnelrpc/pogs"
22+
"github.com/cloudflare/cloudflared/tunnelstate"
2223
)
2324

2425
const (
@@ -88,7 +89,9 @@ func NewSupervisor(config *TunnelConfig, orchestrator *orchestration.Orchestrato
8889
}
8990

9091
reconnectCredentialManager := newReconnectCredentialManager(connection.MetricsNamespace, connection.TunnelSubsystem, config.HAConnections)
91-
log := NewConnAwareLogger(config.Log, config.Observer)
92+
93+
tracker := tunnelstate.NewConnTracker(config.Log)
94+
log := NewConnAwareLogger(config.Log, tracker, config.Observer)
9295

9396
var edgeAddrHandler EdgeAddrHandler
9497
if isStaticEdge { // static edge addresses
@@ -106,6 +109,7 @@ func NewSupervisor(config *TunnelConfig, orchestrator *orchestration.Orchestrato
106109
credentialManager: reconnectCredentialManager,
107110
edgeAddrs: edgeIPs,
108111
edgeAddrHandler: edgeAddrHandler,
112+
tracker: tracker,
109113
reconnectCh: reconnectCh,
110114
gracefulShutdownC: gracefulShutdownC,
111115
connAwareLogger: log,

‎supervisor/tunnel.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/cloudflare/cloudflared/signal"
2727
"github.com/cloudflare/cloudflared/tunnelrpc"
2828
tunnelpogs "github.com/cloudflare/cloudflared/tunnelrpc/pogs"
29+
"github.com/cloudflare/cloudflared/tunnelstate"
2930
)
3031

3132
const (
@@ -190,6 +191,7 @@ type EdgeTunnelServer struct {
190191
edgeAddrs *edgediscovery.Edge
191192
reconnectCh chan ReconnectSignal
192193
gracefulShutdownC <-chan struct{}
194+
tracker *tunnelstate.ConnTracker
193195

194196
connAwareLogger *ConnAwareLogger
195197
}
@@ -272,6 +274,12 @@ func (e EdgeTunnelServer) Serve(ctx context.Context, connIndex uint8, protocolFa
272274
return err
273275
}
274276

277+
// If a single connection has connected with the current protocol, we know we know we don't have to fallback
278+
// to a different protocol.
279+
if e.tracker.HasConnectedWith(e.config.ProtocolSelector.Current()) {
280+
return err
281+
}
282+
275283
if !selectNextProtocol(
276284
connLog.Logger(),
277285
protocolFallback,
@@ -461,6 +469,7 @@ func serveTunnel(
461469
nil,
462470
gracefulShutdownC,
463471
config.GracePeriod,
472+
protocol,
464473
)
465474

466475
switch protocol {

0 commit comments

Comments
 (0)