Skip to content

Commit ae9dbd8

Browse files
ignoramousignoramous
committed
netstack: refactor seamless
Merge snooper and magiclink, swap both fd and mtu, avoid clearing closed endpoint embed in magiclink, make pcap sink a required arg to init magiclink, rename vars.
1 parent f206435 commit ae9dbd8

5 files changed

Lines changed: 90 additions & 162 deletions

File tree

‎intra/netstack/fdbased.go‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,6 @@ var _ stack.LinkEndpoint = (*endpoint)(nil)
5252
// placeholder FD for whenever existing FD wrapped in struct fds is closed.
5353
const invalidfd int = -1
5454

55-
// Should WritePackets return NoSuchFile error if the fd is invalid?
56-
const errorOnInvalidFD = false
57-
5855
// wrapttl is the time to wait for the dispatcher to wrap up (close a previous FD).
5956
const waitttl = wrapttl
6057

@@ -273,8 +270,9 @@ func (e *endpoint) Dispose() (err error) {
273270
return nil
274271
}
275272

276-
// Implements Swapper.
277-
func (e *endpoint) Swap(fd int) (err error) {
273+
// Implements FdSwapper.
274+
func (e *endpoint) Swap(fd, mtu int) (err error) {
275+
e.SetMTU(uint32(mtu))
278276
return e.swap(fd, false)
279277
}
280278

@@ -470,10 +468,7 @@ func (e *endpoint) WritePackets(pkts stack.PacketBufferList) (int, tcpip.Error)
470468

471469
if fd == invalidfd {
472470
log.E("ns: tun(-1): WritePackets (to tun): fd invalid (pkts: %d)", pkts.Len())
473-
if errorOnInvalidFD {
474-
return 0, &tcpip.ErrNoSuchFile{}
475-
}
476-
return 0, nil
471+
return 0, &tcpip.ErrNoSuchFile{}
477472
}
478473

479474
batch := make([]unix.Iovec, 0, batchSz)
@@ -602,10 +597,7 @@ func (e *endpoint) InjectOutbound(dest tcpip.Address, packet *buffer.View) tcpip
602597

603598
if !f.ok() {
604599
log.E("ns: tun(%d): inject-outbound (to tun) to dst(%v): endpoint not attached", fd, dest)
605-
if errorOnInvalidFD {
606-
return &tcpip.ErrUnknownDevice{}
607-
}
608-
return nil // nothing to do
600+
return &tcpip.ErrUnknownDevice{}
609601
}
610602

611603
b := packet.AsSlice()

‎intra/netstack/seamless.go‎

Lines changed: 59 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,13 @@ import (
1515
"gvisor.dev/gvisor/pkg/tcpip/stack"
1616
)
1717

18-
// SnapLen is the maximum bytes of a packet to be saved. Packets with a length
19-
// less than or equal to snapLen will be saved in their entirety. Longer
20-
// packets will be truncated to snapLen.
21-
const SnapLen uint32 = 2048 // in bytes; some sufficient value
18+
// Clearing endpoint on Dispose will result in all stack.Linkpoint APIs
19+
// returning zero values (which may trip netstack?)
20+
const clearEndpointOnDispose = false
2221

2322
type FdSwapper interface {
2423
// Swap closes existing FDs; uses new fd.
25-
Swap(fd int) error
24+
Swap(fd, mtu int) error
2625
// Dispose closes all existing FDs.
2726
Dispose() error
2827
// Stat returns EpStat (fd, age, read, written, lastRead, lastWrite).
@@ -76,6 +75,8 @@ var _ stack.NetworkDispatcher = (*magiclink)(nil)
7675
var _ stack.GSOEndpoint = (*magiclink)(nil)
7776
var _ SeamlessEndpoint = (*magiclink)(nil)
7877

78+
var errMissingSink = errors.New("magic: pcap sink is nil")
79+
7980
// ref: github.com/google/gvisor/blob/91f58d2cc/pkg/tcpip/sample/tun_tcp_echo/main.go#L102
8081
func NewEndpoint(dev, mtu int, sink io.WriteCloser) (ep SeamlessEndpoint, err error) {
8182
defer func() {
@@ -85,6 +86,10 @@ func NewEndpoint(dev, mtu int, sink io.WriteCloser) (ep SeamlessEndpoint, err er
8586
log.I("netstack: new endpoint(fd:%d / mtu:%d); err? %v", dev, mtu, err)
8687
}()
8788

89+
if sink == nil {
90+
return nil, errMissingSink
91+
}
92+
8893
umtu := uint32(mtu)
8994
opt := Options{
9095
FDs: []int{dev},
@@ -95,39 +100,27 @@ func NewEndpoint(dev, mtu int, sink io.WriteCloser) (ep SeamlessEndpoint, err er
95100
return nil, err
96101
}
97102
// ref: github.com/google/gvisor/blob/aeabb785278/pkg/tcpip/link/sniffer/sniffer.go#L111-L131
98-
return snoop(ep, sink)
99-
}
100-
101-
func snoop(ep SeamlessEndpoint, sink io.WriteCloser) (SeamlessEndpoint, error) {
102-
if sink == nil {
103-
return ep, nil
104-
}
105-
// TODO: MTU instead of SnapLen? Must match pcapsink.begin()
106-
link, err := newSnoopyEndpoint(ep, sink, true /*write header*/)
107-
if err != nil {
108-
return nil, err
109-
}
110-
111-
v := core.NewVolatile[SeamlessEndpoint](link)
103+
v := core.NewVolatile[SeamlessEndpoint](ep)
112104
d := core.NewZeroVolatile[stack.NetworkDispatcher]()
113-
return &magiclink{v, d, sink}, nil
105+
106+
return &magiclink{v, d /*nil*/, sink}, nil
114107
}
115108

116109
func Pcap2Stdout(y bool) (ok bool) {
117110
if y {
118-
ok = LogPackets.CompareAndSwap(0, 1)
111+
ok = logPackets.CompareAndSwap(0, 1)
119112
} else {
120-
ok = LogPackets.CompareAndSwap(1, 0)
113+
ok = logPackets.CompareAndSwap(1, 0)
121114
}
122115
log.I("netstack: pcap stdout(%t): done?(%t)", y, ok)
123116
return
124117
}
125118

126119
func Pcap2File(y bool) (ok bool) {
127120
if y {
128-
ok = WritePCAP.CompareAndSwap(0, 1)
121+
ok = writePCAP.CompareAndSwap(0, 1)
129122
} else {
130-
ok = WritePCAP.CompareAndSwap(1, 0)
123+
ok = writePCAP.CompareAndSwap(1, 0)
131124
}
132125
log.I("netstack: pcap file(%t): done?(%t)", y, ok)
133126
return
@@ -139,10 +132,10 @@ func Pcap2File(y bool) (ok bool) {
139132
// - none: no packets are logged
140133
func PcapModes() string {
141134
var modes []string
142-
if LogPackets.Load() == 1 {
135+
if logPackets.Load() == 1 {
143136
modes = append(modes, "stdout")
144137
}
145-
if WritePCAP.Load() == 1 {
138+
if writePCAP.Load() == 1 {
146139
modes = append(modes, "file")
147140
}
148141
if len(modes) == 0 {
@@ -152,12 +145,12 @@ func PcapModes() string {
152145
}
153146

154147
// Swap implements SeamlessEndpoint.
155-
func (l *magiclink) Swap(fd int) (err error) {
148+
func (l *magiclink) Swap(fd, mtu int) (err error) {
156149
e := l.e.Load()
157150
hasSwappedFd := false
158151
needsNewEndpoint := e == nil
159152
if e != nil {
160-
err = e.Swap(fd)
153+
err = e.Swap(fd, mtu)
161154
hasSwappedFd = err == nil
162155
needsNewEndpoint = errors.Is(err, errNeedsNewEndpoint)
163156
}
@@ -168,46 +161,45 @@ func (l *magiclink) Swap(fd int) (err error) {
168161
return err
169162
}
170163

171-
if needsNewEndpoint {
172-
umtu := uint32(l.MTU())
173-
opt := Options{
174-
FDs: []int{fd},
175-
MTU: umtu,
176-
}
177-
178-
ep, err := newFdbasedInjectableEndpoint(&opt)
179-
if err != nil {
180-
log.E("netstack: magic(%d); swap: err %v", fd, err)
181-
return err
182-
}
164+
umtu := uint32(mtu)
165+
opt := Options{
166+
FDs: []int{fd},
167+
MTU: umtu,
168+
}
183169

184-
link, err := newSnoopyEndpoint(ep, l.s, false /*write header*/)
185-
if err != nil {
186-
log.E("netstack: magic(%d); swap: err %v", fd, err)
187-
return err
188-
}
170+
ep, err := newFdbasedInjectableEndpoint(&opt)
171+
if err != nil {
172+
log.E("netstack: magic(%d); swap: err %v", fd, err)
173+
return err
174+
}
189175

190-
if old := l.e.Swap(link); old != nil {
191-
core.Go("magic."+strconv.Itoa(fd), old.Close)
192-
}
176+
if old := l.e.Swap(ep); old != nil {
177+
core.Go("magic."+strconv.Itoa(fd), old.Close)
178+
}
193179

194-
d := l.d.Load()
195-
if d == nil {
196-
ep.Attach(nil) // attach the new endpoint to the dispatcher
197-
} else {
198-
ep.Attach(l) // attach the new endpoint to the existing dispatcher
199-
}
200-
logei(d == nil)("netstack: magic(%d) mtu: %d; swap: new ep... dispatch? %t",
201-
fd, umtu, d != nil)
180+
d := l.d.Load()
181+
if d == nil {
182+
ep.Attach(nil) // attach the new endpoint to the dispatcher
183+
} else {
184+
ep.Attach(l) // attach the new endpoint to the existing dispatcher
202185
}
186+
logei(d == nil)("netstack: magic(%d) mtu: %d; swap: new ep... dispatch? %t",
187+
fd, umtu, d != nil)
203188

204189
return nil
205190
}
206191

207192
// Dispose implements SeamlessEndpoint.
208193
func (l *magiclink) Dispose() error {
209194
if e := l.e.Load(); e != nil {
210-
l.e.Store(nil) // clear the endpoint
195+
if clearEndpointOnDispose {
196+
// will result in stack.LinkEndpoint impls return zero values
197+
// unsure if this will trip netstack in to thinking if this
198+
// endpoint is kaput, when in reality, this endpoint can swap
199+
// in a healthy endpoint at a later point in time, which then
200+
// we'd expect netstack to use as expected.
201+
l.e.Store(nil)
202+
}
211203
return e.Dispose()
212204
}
213205
log.W("netstack: magic: dispose; no endpoint")
@@ -271,6 +263,7 @@ func (l *magiclink) Attach(dispatcher stack.NetworkDispatcher) {
271263
e.Attach(l)
272264
}
273265
}
266+
log.I("netstack: magic: attach dispatcher? %t", dispatcher == nil)
274267
}
275268

276269
func (l *magiclink) IsAttached() bool {
@@ -281,6 +274,7 @@ func (l *magiclink) IsAttached() bool {
281274
}
282275

283276
func (l *magiclink) DeliverNetworkPacket(protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) {
277+
l.DumpPacket(DirectionRecv, protocol, pkt)
284278
if d := l.d.Load(); d != nil {
285279
d.DeliverNetworkPacket(protocol, pkt)
286280
}
@@ -293,9 +287,17 @@ func (l *magiclink) DeliverLinkPacket(protocol tcpip.NetworkProtocolNumber, pkt
293287
}
294288

295289
func (l *magiclink) WritePackets(pkts stack.PacketBufferList) (int, tcpip.Error) {
290+
if l.doPCAP() {
291+
for _, pkt := range pkts.AsSlice() {
292+
if pkt != nil { // nilaway
293+
l.DumpPacket(DirectionSend, pkt.NetworkProtocolNumber, pkt)
294+
}
295+
}
296+
}
296297
if e := l.e.Load(); e != nil {
297298
return e.WritePackets(pkts)
298299
}
300+
log.E("netstack: magic: write packets; no endpoint")
299301
return 0, &tcpip.ErrInvalidEndpointState{}
300302
}
301303

0 commit comments

Comments
 (0)