webnet/tailscale
1
0
Fork 0
Code Issues 1 Pull Requests 1 Packages Projects Releases Wiki Activity

feature/conn25: drop returned error from NewFlow signature

Browse Source 
The returned error in the signature is left over from previous
implementations and was only returning nil.

If we know NewFlow will succeed we can fire a create hook (implemented
in a future commit) before NewFlow, which will prevent a remove hook for
a flow from firing before the create hook for the same flow.

Updates tailscale/corp#38630

Signed-off-by: Michael Ben-Ami <mzb@tailscale.com>
This commit is contained in:
Michael Ben-Ami
2026-06-12 10:44:42 -04:00
committed by mzbenami
parent da11aa54b7
commit 2a0eafc20f
3 changed files with 20 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
feature/conn25/datapath.go
+6 -10
View File
@@ -175,13 +175,11 @@ func (dh *datapathHandler) HandlePacketFromWireGuard(p *packet.Parsed, tun *tstu
Tuple: flowtrack.MakeTuple(p.IPProto, netip.AddrPortFrom(realIP, p.Dst.Port()), p.Src),
Action: dh.snatAction(transitIP),
}
if err := dh.connectorFlowTable.NewFlow(FlowData{
dh.connectorFlowTable.NewFlow(FlowData{
FromTun: incoming,
FromWG: outgoing,
}); err != nil {
dh.debugLogf("error installing flow, passing packet unmodified: %v", err)
return filter.Accept
}
})
outgoing.Action(p)
return filter.Accept
}
@@ -244,13 +242,11 @@ func (dh *datapathHandler) HandlePacketFromTunDevice(p *packet.Parsed) filter.Re
Tuple: flowtrack.MakeTuple(p.IPProto, netip.AddrPortFrom(transitIP, p.Dst.Port()), p.Src),
Action: dh.snatAction(magicIP),
}
if err := dh.clientFlowTable.NewFlow(FlowData{
dh.clientFlowTable.NewFlow(FlowData{
FromTun: outgoing,
FromWG: incoming,
}); err != nil {
dh.debugLogf("error installing flow from tun device, passing packet unmodified: %v", err)
return filter.Accept
}
})
outgoing.Action(p)
return filter.Accept
}
feature/conn25/flowtable.go
+1 -3
View File
@@ -196,7 +196,7 @@ func (t *FlowTable) lookup(k flowtrack.Tuple, dir Origin) (PacketAction, bool) {
// would cause the table to exceed its maximum size, the least recently used
// (looked-up or created) flow is evicted. data is not validated, the caller must
// supply non-nil packet actions.
func (t *FlowTable) NewFlow(data FlowData) error {
func (t *FlowTable) NewFlow(data FlowData) {
t.mu.Lock()
defer t.mu.Unlock()
@@ -217,8 +217,6 @@ func (t *FlowTable) NewFlow(data FlowData) error {
t.fromTunCache[data.FromTun.Tuple] = ele
t.fromWGCache[data.FromWG.Tuple] = ele
return nil
}
// StartExpiredSweeper starts a sweeper that removes idle flows that have
feature/conn25/flowtable_test.go
+13 -20
View File
@@ -59,13 +59,6 @@ func mkFlows(n int) []FlowData {
return flows
}
func mustInstallFlow(t *testing.T, ft *FlowTable, flow FlowData) {
t.Helper()
if err := ft.NewFlow(flow); err != nil {
t.Fatalf("error installing flow: %v", err)
}
}
func assertFlowHit(t *testing.T, ft *FlowTable, dir Origin, tuple flowtrack.Tuple) PacketAction {
t.Helper()
return assertFlowLookup(t, ft, dir, tuple, true)
@@ -113,7 +106,7 @@ func TestFlowTable_NewFlow_Lookup(t *testing.T) {
fromWGTuple := mkTuple("4.3.2.2:80", "1.2.3.4:1000")
flow1, tunCount1, wgCount1 := mkFlowWithActions(fromTunTuple, fromWGTuple)
mustInstallFlow(t, ft, flow1)
ft.NewFlow(flow1)
// Test basic lookups, and perform actions on packet.
assertFlowHit(t, ft, FromTun, fromTunTuple)(nilPacket)
@@ -136,13 +129,13 @@ func TestFlowTable_NewFlow_Lookup(t *testing.T) {
// Overwriting from-tun tuple removes the from-wg tuple as well.
newFromWGTuple := mkTuple("9.9.9.9:99", "8.8.8.8:88")
flow2 := mkFlow(fromTunTuple, newFromWGTuple)
mustInstallFlow(t, ft, flow2)
ft.NewFlow(flow2)
assertFlowMiss(t, ft, FromWireGuard, fromWGTuple)
// Overwriting the from-wg tuple removes the from-tun tuple as well.
newFromTunTuple := mkTuple("8.8.8.8:88", "9.9.9.9:99")
flow3 := mkFlow(newFromTunTuple, newFromWGTuple)
mustInstallFlow(t, ft, flow3)
ft.NewFlow(flow3)
assertFlowMiss(t, ft, FromTun, fromTunTuple)
}
@@ -162,8 +155,8 @@ func TestFlowTable_OneReplacesTwo(t *testing.T) {
flow2, tunCount2, wgCount2 := mkFlowWithActions(tunTuple2, wgTuple2)
// Install the first two flows.
mustInstallFlow(t, ft, flow1)
mustInstallFlow(t, ft, flow2)
ft.NewFlow(flow1)
ft.NewFlow(flow2)
// Confirm they are properly installed through lookups.
assertFlowHit(t, ft, FromTun, tunTuple1)
@@ -173,7 +166,7 @@ func TestFlowTable_OneReplacesTwo(t *testing.T) {
// flow3 tuples overlap with flow1 and flow2.
flow3, tunCount3, wgCount3 := mkFlowWithActions(tunTuple1, wgTuple2)
mustInstallFlow(t, ft, flow3)
ft.NewFlow(flow3)
// flow3 lookups hit on both of their tuples.
tunAction3 := assertFlowHit(t, ft, FromTun, tunTuple1)
@@ -213,14 +206,14 @@ func TestFlowTable_Eviction(t *testing.T) {
d := mkFlow(dTun, dWG)
// Install a and b.
mustInstallFlow(t, ft, a)
mustInstallFlow(t, ft, b)
ft.NewFlow(a)
ft.NewFlow(b)
// Move a to the front from tun side, b is ready for eviction.
assertFlowHit(t, ft, FromTun, aTun)
// Install c.
mustInstallFlow(t, ft, c)
ft.NewFlow(c)
// Check b is out.
assertFlowMiss(t, ft, FromTun, bTun)
@@ -234,7 +227,7 @@ func TestFlowTable_Eviction(t *testing.T) {
assertFlowHit(t, ft, FromWireGuard, aWG)
// Install d.
mustInstallFlow(t, ft, d)
ft.NewFlow(d)
// Check c is out.
assertFlowMiss(t, ft, FromTun, cTun)
@@ -334,7 +327,7 @@ func TestFlowTable_removeIdle(t *testing.T) {
flows := mkFlows(len(tt.flowSpecs))
for i, spec := range tt.flowSpecs {
time.Sleep(time.Until(start.Add(spec.installAt)))
mustInstallFlow(t, ft, flows[i])
ft.NewFlow(flows[i])
}
var wantRemovedCount int
@@ -365,9 +358,9 @@ func TestFlowTable_removeIdle(t *testing.T) {
ft := NewFlowTable(0, WithFlowIdleTimeout(time.Minute))
flows := mkFlows(2)
mustInstallFlow(t, ft, flows[0]) // t=0 (flow 0 install)
ft.NewFlow(flows[0]) // t=0 (flow 0 install)
time.Sleep(30 * time.Second) //
mustInstallFlow(t, ft, flows[1]) // t=30s (flow 1 install)
ft.NewFlow(flows[1]) // t=30s (flow 1 install)
time.Sleep(60 * time.Second) //
assertFlowHit(t, ft, FromTun, flows[0].FromTun.Tuple) // t=90s (flow 0 looked up, lastSeen bumped)
time.Sleep(15 * time.Second) //