From e511fc90927639b8c74652738112969e6a626d16 Mon Sep 17 00:00:00 2001 From: Lucas Manning Date: Tue, 28 Dec 2021 13:42:25 -0800 Subject: [PATCH] Remove WritePacket from LinkEndpoint. PiperOrigin-RevId: 418672809 --- pkg/tcpip/link/channel/channel.go | 10 --- pkg/tcpip/link/ethernet/ethernet.go | 6 -- pkg/tcpip/link/fdbased/endpoint.go | 6 +- pkg/tcpip/link/fdbased/endpoint_test.go | 16 +++- pkg/tcpip/link/loopback/loopback.go | 6 -- pkg/tcpip/link/muxed/injectable.go | 10 --- pkg/tcpip/link/muxed/injectable_test.go | 13 ++- pkg/tcpip/link/nested/nested.go | 5 -- pkg/tcpip/link/pipe/pipe.go | 13 +-- pkg/tcpip/link/sharedmem/sharedmem.go | 12 --- pkg/tcpip/link/sharedmem/sharedmem_test.go | 85 ++++++++++++------- pkg/tcpip/link/sniffer/sniffer.go | 8 -- pkg/tcpip/link/waitable/waitable.go | 13 --- pkg/tcpip/link/waitable/waitable_test.go | 51 ++++++----- pkg/tcpip/network/arp/arp_test.go | 8 -- .../network/internal/testutil/testutil.go | 18 +--- pkg/tcpip/network/ipv6/icmp_test.go | 14 +-- pkg/tcpip/stack/forwarding_test.go | 27 +++--- pkg/tcpip/stack/nic.go | 2 +- pkg/tcpip/stack/registration.go | 12 --- 20 files changed, 141 insertions(+), 194 deletions(-) diff --git a/pkg/tcpip/link/channel/channel.go b/pkg/tcpip/link/channel/channel.go index 8f44a8a47..83531f6f8 100644 --- a/pkg/tcpip/link/channel/channel.go +++ b/pkg/tcpip/link/channel/channel.go @@ -239,16 +239,6 @@ func (e *Endpoint) LinkAddress() tcpip.LinkAddress { return e.linkAddr } -// WritePacket stores outbound packets into the channel. -func (e *Endpoint) WritePacket(_ stack.RouteInfo, _ tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - // Write returns false if the queue is full. A full queue is not an error - // from the perspective of a LinkEndpoint so we ignore Write's return - // value and always return nil from this method. - _ = e.q.Write(pkt) - - return nil -} - // WritePackets stores outbound packets into the channel. func (e *Endpoint) WritePackets(_ stack.RouteInfo, pkts stack.PacketBufferList, _ tcpip.NetworkProtocolNumber) (int, tcpip.Error) { n := 0 diff --git a/pkg/tcpip/link/ethernet/ethernet.go b/pkg/tcpip/link/ethernet/ethernet.go index 60c592c12..8913d6d49 100644 --- a/pkg/tcpip/link/ethernet/ethernet.go +++ b/pkg/tcpip/link/ethernet/ethernet.go @@ -80,12 +80,6 @@ func (e *Endpoint) Capabilities() stack.LinkEndpointCapabilities { return c } -// WritePacket implements stack.LinkEndpoint. -func (e *Endpoint) WritePacket(r stack.RouteInfo, proto tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - e.AddHeader(e.LinkAddress(), r.RemoteLinkAddress, proto, pkt) - return e.Endpoint.WritePacket(r, proto, pkt) -} - // WritePackets implements stack.LinkEndpoint. func (e *Endpoint) WritePackets(r stack.RouteInfo, pkts stack.PacketBufferList, proto tcpip.NetworkProtocolNumber) (int, tcpip.Error) { linkAddr := e.LinkAddress() diff --git a/pkg/tcpip/link/fdbased/endpoint.go b/pkg/tcpip/link/fdbased/endpoint.go index 058242f96..83e156885 100644 --- a/pkg/tcpip/link/fdbased/endpoint.go +++ b/pkg/tcpip/link/fdbased/endpoint.go @@ -508,9 +508,9 @@ func (e *endpoint) AddHeader(local, remote tcpip.LinkAddress, protocol tcpip.Net // WriteRawPacket implements stack.LinkEndpoint. func (*endpoint) WriteRawPacket(*stack.PacketBuffer) tcpip.Error { return &tcpip.ErrNotSupported{} } -// WritePacket writes outbound packets to the file descriptor. If it is not +// writePacket writes outbound packets to the file descriptor. If it is not // currently writable, the packet is dropped. -func (e *endpoint) WritePacket(r stack.RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { +func (e *endpoint) writePacket(r stack.RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { if e.hdrSize > 0 { e.AddHeader(r.LocalLinkAddress, r.RemoteLinkAddress, protocol, pkt) } @@ -641,7 +641,7 @@ func (e *endpoint) sendBatch(batchFD int, pkts []*stack.PacketBuffer) (int, tcpi // if necessary (by using e.writevMaxIovs instead of // rawfile.MaxIovs). pkt := batch[0] - if err := e.WritePacket(pkt.EgressRoute, pkt.NetworkProtocolNumber, pkt); err != nil { + if err := e.writePacket(pkt.EgressRoute, pkt.NetworkProtocolNumber, pkt); err != nil { return packets, err } packets++ diff --git a/pkg/tcpip/link/fdbased/endpoint_test.go b/pkg/tcpip/link/fdbased/endpoint_test.go index eccd21579..aa13ba096 100644 --- a/pkg/tcpip/link/fdbased/endpoint_test.go +++ b/pkg/tcpip/link/fdbased/endpoint_test.go @@ -199,6 +199,10 @@ func testWritePacket(t *testing.T, plen int, eth bool, gsoMaxSize uint32, hash u Data: payload.ToVectorisedView(), }) pkt.Hash = hash + // Every PacketBuffer must have these set: + // See nic.writePacket. + pkt.EgressRoute = r + pkt.NetworkProtocolNumber = proto // Build header. b := pkt.NetworkHeader().Push(netHdrLen) @@ -218,7 +222,9 @@ func testWritePacket(t *testing.T, plen int, eth bool, gsoMaxSize uint32, hash u L3HdrLen: l3HdrLen, } } - if err := c.ep.WritePacket(r, proto, pkt); err != nil { + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, proto); err != nil { t.Fatalf("WritePacket failed: %v", err) } @@ -333,7 +339,13 @@ func TestPreserveSrcAddress(t *testing.T) { ReserveHeaderBytes: header.EthernetMinimumSize, Data: buffer.VectorisedView{}, }) - if err := c.ep.WritePacket(r, proto, pkt); err != nil { + // Every PacketBuffer must have these set: + // See nic.writePacket. + pkt.NetworkProtocolNumber = proto + pkt.EgressRoute = r + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, proto); err != nil { t.Fatalf("WritePacket failed: %v", err) } diff --git a/pkg/tcpip/link/loopback/loopback.go b/pkg/tcpip/link/loopback/loopback.go index 68dc30a79..9b4d0ca1e 100644 --- a/pkg/tcpip/link/loopback/loopback.go +++ b/pkg/tcpip/link/loopback/loopback.go @@ -74,12 +74,6 @@ func (*endpoint) LinkAddress() tcpip.LinkAddress { // Wait implements stack.LinkEndpoint.Wait. func (*endpoint) Wait() {} -// WritePacket implements stack.LinkEndpoint.WritePacket. It delivers outbound -// packets to the network-layer dispatcher. -func (e *endpoint) WritePacket(_ stack.RouteInfo, _ tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - return e.WriteRawPacket(pkt) -} - // WritePackets implements stack.LinkEndpoint.WritePackets. func (e *endpoint) WritePackets(_ stack.RouteInfo, pkts stack.PacketBufferList, _ tcpip.NetworkProtocolNumber) (int, tcpip.Error) { n := 0 diff --git a/pkg/tcpip/link/muxed/injectable.go b/pkg/tcpip/link/muxed/injectable.go index 844f5959b..0dae3eca7 100644 --- a/pkg/tcpip/link/muxed/injectable.go +++ b/pkg/tcpip/link/muxed/injectable.go @@ -95,16 +95,6 @@ func (m *InjectableEndpoint) WritePackets(r stack.RouteInfo, pkts stack.PacketBu return endpoint.WritePackets(r, pkts, protocol) } -// WritePacket writes outbound packets to the appropriate LinkInjectableEndpoint -// based on the RemoteAddress. HandleLocal only works if r.RemoteAddress has a -// route registered in this endpoint. -func (m *InjectableEndpoint) WritePacket(r stack.RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - if endpoint, ok := m.routes[r.RemoteAddress]; ok { - return endpoint.WritePacket(r, protocol, pkt) - } - return &tcpip.ErrNoRoute{} -} - // InjectOutbound writes outbound packets to the appropriate // LinkInjectableEndpoint based on the dest address. func (m *InjectableEndpoint) InjectOutbound(dest tcpip.Address, packet []byte) tcpip.Error { diff --git a/pkg/tcpip/link/muxed/injectable_test.go b/pkg/tcpip/link/muxed/injectable_test.go index 040e3a35b..f129748d8 100644 --- a/pkg/tcpip/link/muxed/injectable_test.go +++ b/pkg/tcpip/link/muxed/injectable_test.go @@ -54,7 +54,11 @@ func TestInjectableEndpointDispatch(t *testing.T) { var packetRoute stack.RouteInfo packetRoute.RemoteAddress = dstIP - endpoint.WritePacket(packetRoute, ipv4.ProtocolNumber, pkt) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := endpoint.WritePackets(packetRoute, pkts, ipv4.ProtocolNumber); err != nil { + t.Fatalf("Unable to write packets: %s", err) + } buf := make([]byte, 6500) bytesRead, err := sock.Read(buf) @@ -76,7 +80,12 @@ func TestInjectableEndpointDispatchHdrOnly(t *testing.T) { pkt.TransportHeader().Push(1)[0] = 0xFA var packetRoute stack.RouteInfo packetRoute.RemoteAddress = dstIP - endpoint.WritePacket(packetRoute, ipv4.ProtocolNumber, pkt) + + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := endpoint.WritePackets(packetRoute, pkts, ipv4.ProtocolNumber); err != nil { + t.Fatalf("Unable to write packets: %s", err) + } buf := make([]byte, 6500) bytesRead, err := sock.Read(buf) if err != nil { diff --git a/pkg/tcpip/link/nested/nested.go b/pkg/tcpip/link/nested/nested.go index 83a6c1cc8..99c5b9bfc 100644 --- a/pkg/tcpip/link/nested/nested.go +++ b/pkg/tcpip/link/nested/nested.go @@ -102,11 +102,6 @@ func (e *Endpoint) LinkAddress() tcpip.LinkAddress { return e.child.LinkAddress() } -// WritePacket implements stack.LinkEndpoint. -func (e *Endpoint) WritePacket(r stack.RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - return e.child.WritePacket(r, protocol, pkt) -} - // WritePackets implements stack.LinkEndpoint. func (e *Endpoint) WritePackets(r stack.RouteInfo, pkts stack.PacketBufferList, protocol tcpip.NetworkProtocolNumber) (int, tcpip.Error) { return e.child.WritePackets(r, pkts, protocol) diff --git a/pkg/tcpip/link/pipe/pipe.go b/pkg/tcpip/link/pipe/pipe.go index 0be0a1efd..a7620e709 100644 --- a/pkg/tcpip/link/pipe/pipe.go +++ b/pkg/tcpip/link/pipe/pipe.go @@ -70,14 +70,6 @@ func (e *Endpoint) deliverPackets(r stack.RouteInfo, proto tcpip.NetworkProtocol } } -// WritePacket implements stack.LinkEndpoint. -func (e *Endpoint) WritePacket(r stack.RouteInfo, proto tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - var pkts stack.PacketBufferList - pkts.PushBack(pkt) - e.deliverPackets(r, proto, pkts) - return nil -} - // WritePackets implements stack.LinkEndpoint. func (e *Endpoint) WritePackets(r stack.RouteInfo, pkts stack.PacketBufferList, proto tcpip.NetworkProtocolNumber) (int, tcpip.Error) { n := pkts.Len() @@ -129,5 +121,8 @@ func (*Endpoint) AddHeader(_, _ tcpip.LinkAddress, _ tcpip.NetworkProtocolNumber // WriteRawPacket implements stack.LinkEndpoint. func (e *Endpoint) WriteRawPacket(pkt *stack.PacketBuffer) tcpip.Error { - return e.WritePacket(stack.RouteInfo{}, 0, pkt) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + _, err := e.WritePackets(stack.RouteInfo{}, pkts, 0) + return err } diff --git a/pkg/tcpip/link/sharedmem/sharedmem.go b/pkg/tcpip/link/sharedmem/sharedmem.go index c4f347cde..e304149b9 100644 --- a/pkg/tcpip/link/sharedmem/sharedmem.go +++ b/pkg/tcpip/link/sharedmem/sharedmem.go @@ -363,18 +363,6 @@ func (e *endpoint) writePacketLocked(r stack.RouteInfo, protocol tcpip.NetworkPr return nil } -// WritePacket writes outbound packets to the file descriptor. If it is not -// currently writable, the packet is dropped. -func (e *endpoint) WritePacket(_ stack.RouteInfo, _ tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - e.mu.Lock() - defer e.mu.Unlock() - if err := e.writePacketLocked(pkt.EgressRoute, pkt.NetworkProtocolNumber, pkt); err != nil { - return err - } - e.tx.notify() - return nil -} - // WritePackets implements stack.LinkEndpoint.WritePackets. func (e *endpoint) WritePackets(_ stack.RouteInfo, pkts stack.PacketBufferList, protocol tcpip.NetworkProtocolNumber) (int, tcpip.Error) { n := 0 diff --git a/pkg/tcpip/link/sharedmem/sharedmem_test.go b/pkg/tcpip/link/sharedmem/sharedmem_test.go index a49f5f87d..16759c5fa 100644 --- a/pkg/tcpip/link/sharedmem/sharedmem_test.go +++ b/pkg/tcpip/link/sharedmem/sharedmem_test.go @@ -233,8 +233,10 @@ func TestSimpleSend(t *testing.T) { // See nic.writePacket. pkt.EgressRoute = r pkt.NetworkProtocolNumber = proto - if err := c.ep.WritePacket(r, proto, pkt); err != nil { - t.Fatalf("WritePacket failed: %v", err) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, proto); err != nil { + t.Fatalf("WritePackets failed: %s", err) } // Receive packet. @@ -306,8 +308,11 @@ func TestPreserveSrcAddressInSend(t *testing.T) { // See nic.writePacket. pkt.EgressRoute = r pkt.NetworkProtocolNumber = proto - if err := c.ep.WritePacket(r, proto, pkt); err != nil { - t.Fatalf("WritePacket failed: %v", err) + + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, proto); err != nil { + t.Fatalf("WritePackets failed: %s", err) } // Receive packet. @@ -362,8 +367,10 @@ func TestFillTxQueue(t *testing.T) { Data: buf.ToVectorisedView(), }) - if err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt); err != nil { - t.Fatalf("WritePacket failed unexpectedly: %v", err) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber); err != nil { + t.Fatalf("WritePackets failed unexpectedly: %s", err) } // Check that they have different IDs. @@ -380,9 +387,11 @@ func TestFillTxQueue(t *testing.T) { ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buf.ToVectorisedView(), }) - err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber) if _, ok := err.(*tcpip.ErrWouldBlock); !ok { - t.Fatalf("got WritePacket(...) = %v, want %s", err, &tcpip.ErrWouldBlock{}) + t.Fatalf("got WritePackets(...) = %s, want %s", err, &tcpip.ErrWouldBlock{}) } } @@ -403,13 +412,17 @@ func TestFillTxQueueAfterBadCompletion(t *testing.T) { buf := buffer.NewView(100) // Send two packets so that the id slice has at least two slots. - for i := 2; i > 0; i-- { - pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{ - ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), - Data: buf.ToVectorisedView(), - }) - if err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt); err != nil { - t.Fatalf("WritePacket failed unexpectedly: %v", err) + { + var pkts stack.PacketBufferList + for i := 2; i > 0; i-- { + pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{ + ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), + Data: buf.ToVectorisedView(), + }) + pkts.PushBack(pkt) + } + if _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber); err != nil { + t.Fatalf("WritePackets failed unexpectedly: %s", err) } } @@ -431,8 +444,10 @@ func TestFillTxQueueAfterBadCompletion(t *testing.T) { ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buf.ToVectorisedView(), }) - if err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt); err != nil { - t.Fatalf("WritePacket failed unexpectedly: %v", err) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber); err != nil { + t.Fatalf("WritePackets failed unexpectedly: %s", err) } // Check that they have different IDs. @@ -449,9 +464,11 @@ func TestFillTxQueueAfterBadCompletion(t *testing.T) { ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buf.ToVectorisedView(), }) - err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber) if _, ok := err.(*tcpip.ErrWouldBlock); !ok { - t.Fatalf("got WritePacket(...) = %v, want %s", err, &tcpip.ErrWouldBlock{}) + t.Fatalf("got WritePackets(...) = %s, want %s", err, &tcpip.ErrWouldBlock{}) } } @@ -475,8 +492,10 @@ func TestFillTxMemory(t *testing.T) { ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buf.ToVectorisedView(), }) - if err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt); err != nil { - t.Fatalf("WritePacket failed unexpectedly: %v", err) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber); err != nil { + t.Fatalf("WritePackets failed unexpectedly: %s", err) } // Check that they have different IDs. @@ -494,9 +513,11 @@ func TestFillTxMemory(t *testing.T) { ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buf.ToVectorisedView(), }) - err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber) if _, ok := err.(*tcpip.ErrWouldBlock); !ok { - t.Fatalf("got WritePacket(...) = %v, want %s", err, &tcpip.ErrWouldBlock{}) + t.Fatalf("got WritePackets(...) = %s, want %s", err, &tcpip.ErrWouldBlock{}) } } @@ -521,8 +542,10 @@ func TestFillTxMemoryWithMultiBuffer(t *testing.T) { ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buf.ToVectorisedView(), }) - if err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt); err != nil { - t.Fatalf("WritePacket failed unexpectedly: %v", err) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber); err != nil { + t.Fatalf("WritePackets failed unexpectedly: %s", err) } // Pull the posted buffer. @@ -532,24 +555,28 @@ func TestFillTxMemoryWithMultiBuffer(t *testing.T) { // Attempt to write a two-buffer packet. It must fail. { + var pkts stack.PacketBufferList pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{ ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buffer.NewView(bufferSize).ToVectorisedView(), }) - err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt) + pkts.PushBack(pkt) + _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber) if _, ok := err.(*tcpip.ErrWouldBlock); !ok { - t.Fatalf("got WritePacket(...) = %v, want %s", err, &tcpip.ErrWouldBlock{}) + t.Fatalf("got WritePackets(...) = %s, want %s", err, &tcpip.ErrWouldBlock{}) } } // Attempt to write the one-buffer packet again. It must succeed. { + var pkts stack.PacketBufferList pkt := stack.NewPacketBuffer(stack.PacketBufferOptions{ ReserveHeaderBytes: int(c.ep.MaxHeaderLength()), Data: buf.ToVectorisedView(), }) - if err := c.ep.WritePacket(r, header.IPv4ProtocolNumber, pkt); err != nil { - t.Fatalf("WritePacket failed unexpectedly: %v", err) + pkts.PushBack(pkt) + if _, err := c.ep.WritePackets(r, pkts, header.IPv4ProtocolNumber); err != nil { + t.Fatalf("WritePackets failed unexpectedly: %s", err) } } } diff --git a/pkg/tcpip/link/sniffer/sniffer.go b/pkg/tcpip/link/sniffer/sniffer.go index 759ce8802..fabe6411c 100644 --- a/pkg/tcpip/link/sniffer/sniffer.go +++ b/pkg/tcpip/link/sniffer/sniffer.go @@ -161,14 +161,6 @@ func (e *endpoint) dumpPacket(dir direction, protocol tcpip.NetworkProtocolNumbe } } -// WritePacket implements the stack.LinkEndpoint interface. It is called by -// higher-level protocols to write packets; it just logs the packet and -// forwards the request to the lower endpoint. -func (e *endpoint) WritePacket(r stack.RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - e.dumpPacket(directionSend, protocol, pkt) - return e.Endpoint.WritePacket(r, protocol, pkt) -} - // WritePackets implements the stack.LinkEndpoint interface. It is called by // higher-level protocols to write packets; it just logs the packet and // forwards the request to the lower endpoint. diff --git a/pkg/tcpip/link/waitable/waitable.go b/pkg/tcpip/link/waitable/waitable.go index 116e4defb..8513054a0 100644 --- a/pkg/tcpip/link/waitable/waitable.go +++ b/pkg/tcpip/link/waitable/waitable.go @@ -96,19 +96,6 @@ func (e *Endpoint) LinkAddress() tcpip.LinkAddress { return e.lower.LinkAddress() } -// WritePacket implements stack.LinkEndpoint.WritePacket. It is called by -// higher-level protocols to write packets. It only forwards packets to the -// lower endpoint if Wait or WaitWrite haven't been called. -func (e *Endpoint) WritePacket(r stack.RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - if !e.writeGate.Enter() { - return nil - } - - err := e.lower.WritePacket(r, protocol, pkt) - e.writeGate.Leave() - return err -} - // WritePackets implements stack.LinkEndpoint.WritePackets. It is called by // higher-level protocols to write packets. It only forwards packets to the // lower endpoint if Wait or WaitWrite haven't been called. diff --git a/pkg/tcpip/link/waitable/waitable_test.go b/pkg/tcpip/link/waitable/waitable_test.go index b0e4237bd..28dbd0f23 100644 --- a/pkg/tcpip/link/waitable/waitable_test.go +++ b/pkg/tcpip/link/waitable/waitable_test.go @@ -22,6 +22,8 @@ import ( "gvisor.dev/gvisor/pkg/tcpip/stack" ) +var _ stack.LinkEndpoint = (*countedEndpoint)(nil) + type countedEndpoint struct { dispatchCount int writeCount int @@ -69,11 +71,6 @@ func (e *countedEndpoint) LinkAddress() tcpip.LinkAddress { return e.linkAddr } -func (e *countedEndpoint) WritePacket(stack.RouteInfo, tcpip.NetworkProtocolNumber, *stack.PacketBuffer) tcpip.Error { - e.writeCount++ - return nil -} - // WritePackets implements stack.LinkEndpoint.WritePackets. func (e *countedEndpoint) WritePackets(_ stack.RouteInfo, pkts stack.PacketBufferList, _ tcpip.NetworkProtocolNumber) (int, tcpip.Error) { e.writeCount += pkts.Len() @@ -101,25 +98,35 @@ func (e *countedEndpoint) AddHeader(local, remote tcpip.LinkAddress, protocol tc func TestWaitWrite(t *testing.T) { ep := &countedEndpoint{} wep := New(ep) - - // Write and check that it goes through. - wep.WritePacket(stack.RouteInfo{}, 0, stack.NewPacketBuffer(stack.PacketBufferOptions{})) - if want := 1; ep.writeCount != want { - t.Fatalf("Unexpected writeCount: got=%v, want=%v", ep.writeCount, want) + { + var pkts stack.PacketBufferList + pkts.PushBack(stack.NewPacketBuffer(stack.PacketBufferOptions{})) + // Write and check that it goes through. + wep.WritePackets(stack.RouteInfo{}, pkts, 0) + if want := 1; ep.writeCount != want { + t.Fatalf("Unexpected writeCount: got=%v, want=%v", ep.writeCount, want) + } + } + { + var pkts stack.PacketBufferList + pkts.PushBack(stack.NewPacketBuffer(stack.PacketBufferOptions{})) + // Wait on dispatches, then try to write. It must go through. + wep.WaitDispatch() + wep.WritePackets(stack.RouteInfo{}, pkts, 0) + if want := 2; ep.writeCount != want { + t.Fatalf("Unexpected writeCount: got=%v, want=%v", ep.writeCount, want) + } } - // Wait on dispatches, then try to write. It must go through. - wep.WaitDispatch() - wep.WritePacket(stack.RouteInfo{}, 0, stack.NewPacketBuffer(stack.PacketBufferOptions{})) - if want := 2; ep.writeCount != want { - t.Fatalf("Unexpected writeCount: got=%v, want=%v", ep.writeCount, want) - } - - // Wait on writes, then try to write. It must not go through. - wep.WaitWrite() - wep.WritePacket(stack.RouteInfo{}, 0, stack.NewPacketBuffer(stack.PacketBufferOptions{})) - if want := 2; ep.writeCount != want { - t.Fatalf("Unexpected writeCount: got=%v, want=%v", ep.writeCount, want) + { + var pkts stack.PacketBufferList + pkts.PushBack(stack.NewPacketBuffer(stack.PacketBufferOptions{})) + // Wait on writes, then try to write. It must not go through. + wep.WaitWrite() + wep.WritePackets(stack.RouteInfo{}, pkts, 0) + if want := 2; ep.writeCount != want { + t.Fatalf("Unexpected writeCount: got=%v, want=%v", ep.writeCount, want) + } } } diff --git a/pkg/tcpip/network/arp/arp_test.go b/pkg/tcpip/network/arp/arp_test.go index 8dc5c3c3c..4e4d385b8 100644 --- a/pkg/tcpip/network/arp/arp_test.go +++ b/pkg/tcpip/network/arp/arp_test.go @@ -428,14 +428,6 @@ func (t *testLinkEndpoint) WritePackets(r stack.RouteInfo, pkts stack.PacketBuff return t.LinkEndpoint.WritePackets(r, pkts, protocol) } -func (t *testLinkEndpoint) WritePacket(r stack.RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - if t.writeErr != nil { - return t.writeErr - } - - return t.LinkEndpoint.WritePacket(r, protocol, pkt) -} - func TestLinkAddressRequest(t *testing.T) { const nicID = 1 diff --git a/pkg/tcpip/network/internal/testutil/testutil.go b/pkg/tcpip/network/internal/testutil/testutil.go index 4d4d98caf..220aa766d 100644 --- a/pkg/tcpip/network/internal/testutil/testutil.go +++ b/pkg/tcpip/network/internal/testutil/testutil.go @@ -61,27 +61,17 @@ func (*MockLinkEndpoint) MaxHeaderLength() uint16 { return 0 } // LinkAddress implements LinkEndpoint.LinkAddress. func (*MockLinkEndpoint) LinkAddress() tcpip.LinkAddress { return "" } -// WritePacket implements LinkEndpoint.WritePacket. -func (ep *MockLinkEndpoint) WritePacket(_ stack.RouteInfo, _ tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - if ep.allowPackets == 0 { - return ep.err - } - ep.allowPackets-- - ep.WrittenPackets = append(ep.WrittenPackets, pkt) - return nil -} - // WritePackets implements LinkEndpoint.WritePackets. func (ep *MockLinkEndpoint) WritePackets(r stack.RouteInfo, pkts stack.PacketBufferList, protocol tcpip.NetworkProtocolNumber) (int, tcpip.Error) { var n int - for pkt := pkts.Front(); pkt != nil; pkt = pkt.Next() { - if err := ep.WritePacket(r, protocol, pkt); err != nil { - return n, err + if ep.allowPackets == 0 { + return n, ep.err } + ep.allowPackets-- + ep.WrittenPackets = append(ep.WrittenPackets, pkt) n++ } - return n, nil } diff --git a/pkg/tcpip/network/ipv6/icmp_test.go b/pkg/tcpip/network/ipv6/icmp_test.go index dc4907e21..99f1acfb1 100644 --- a/pkg/tcpip/network/ipv6/icmp_test.go +++ b/pkg/tcpip/network/ipv6/icmp_test.go @@ -77,10 +77,6 @@ func (*stubLinkEndpoint) LinkAddress() tcpip.LinkAddress { return "" } -func (*stubLinkEndpoint) WritePacket(stack.RouteInfo, tcpip.NetworkProtocolNumber, *stack.PacketBuffer) tcpip.Error { - return nil -} - func (*stubLinkEndpoint) WritePackets(stack.RouteInfo, stack.PacketBufferList, tcpip.NetworkProtocolNumber) (int, tcpip.Error) { return 0, nil } @@ -135,14 +131,20 @@ func (*testInterface) Spoofing() bool { } func (t *testInterface) WritePacket(r *stack.Route, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { - return t.LinkEndpoint.WritePacket(r.Fields(), protocol, pkt) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + _, err := t.LinkEndpoint.WritePackets(r.Fields(), pkts, protocol) + return err } func (t *testInterface) WritePacketToRemote(remoteLinkAddr tcpip.LinkAddress, protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) tcpip.Error { var r stack.RouteInfo r.NetProto = protocol r.RemoteLinkAddress = remoteLinkAddr - return t.LinkEndpoint.WritePacket(r, protocol, pkt) + var pkts stack.PacketBufferList + pkts.PushBack(pkt) + _, err := t.LinkEndpoint.WritePackets(r, pkts, protocol) + return err } func (t *testInterface) HandleNeighborProbe(tcpip.NetworkProtocolNumber, tcpip.Address, tcpip.LinkAddress) tcpip.Error { diff --git a/pkg/tcpip/stack/forwarding_test.go b/pkg/tcpip/stack/forwarding_test.go index c2f1f4798..f6c2d4bc2 100644 --- a/pkg/tcpip/stack/forwarding_test.go +++ b/pkg/tcpip/stack/forwarding_test.go @@ -312,26 +312,21 @@ func (e *fwdTestLinkEndpoint) LinkAddress() tcpip.LinkAddress { return e.linkAddr } -func (e fwdTestLinkEndpoint) WritePacket(r RouteInfo, _ tcpip.NetworkProtocolNumber, pkt *PacketBuffer) tcpip.Error { - p := fwdTestPacketInfo{ - RemoteLinkAddress: r.RemoteLinkAddress, - LocalLinkAddress: r.LocalLinkAddress, - Pkt: pkt, - } - - select { - case e.C <- p: - default: - } - - return nil -} - // WritePackets stores outbound packets into the channel. func (e *fwdTestLinkEndpoint) WritePackets(r RouteInfo, pkts PacketBufferList, protocol tcpip.NetworkProtocolNumber) (int, tcpip.Error) { n := 0 for pkt := pkts.Front(); pkt != nil; pkt = pkt.Next() { - e.WritePacket(r, protocol, pkt) + p := fwdTestPacketInfo{ + RemoteLinkAddress: r.RemoteLinkAddress, + LocalLinkAddress: r.LocalLinkAddress, + Pkt: pkt, + } + + select { + case e.C <- p: + default: + } + n++ } diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index f6b9b6179..93bc6541f 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -346,7 +346,7 @@ func (n *nic) WriteRawPacket(pkt *PacketBuffer) tcpip.Error { return n.rawLinkEP.WriteRawPacket(pkt) } -// WritePacket implements LinkWriter. +// WritePacket implements NetworkEndpoint. func (n *nic) WritePacket(r *Route, protocol tcpip.NetworkProtocolNumber, pkt *PacketBuffer) tcpip.Error { routeInfo, _, err := r.resolvedFields(nil) switch err.(type) { diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go index da3cd0749..19ee098f8 100644 --- a/pkg/tcpip/stack/registration.go +++ b/pkg/tcpip/stack/registration.go @@ -862,18 +862,6 @@ type LinkEndpoint interface { NetworkLinkEndpoint LinkWriter LinkRawWriter - - // TODO(b/211019749): Remove WritePacket, it's no longer used outside the context of - // tests and LinkEndpoint wrappers. - // WritePacket writes a packet with the given protocol and route. - // - // WritePacket may modify the packet buffer. The packet buffer's - // network and transport header must be set. - // - // To participate in transparent bridging, a LinkEndpoint implementation - // should call eth.Encode with header.EthernetFields.SrcAddr set to - // r.LocalLinkAddress if it is provided. - WritePacket(RouteInfo, tcpip.NetworkProtocolNumber, *PacketBuffer) tcpip.Error } // InjectableLinkEndpoint is a LinkEndpoint where inbound packets are