gro: fix bug where handshake packets would be stuck waiting in GRO

Also enables GRO in syscall tests. GRO should be totally transparent and should
not affect behavior. This ensures it's tested and fixes the bug it uncovered.

GRO would not immediately flush the final ACK in the SYN-SYN/ACK-ACK handshake.
This could lead to a situation where

- Client A calls connect(), which returns once the final ACK of the handshake
  is sent and A reaches state ESTABLISHED
- The ACK gets GRO'd, delaying it from reaching the server
- Client B calls non-blocking connect()
- Client B's ACK gets GRO'd as well
- Client B is marked as ESTABLISHED
- The server, with accept queue size 1, is only going to accept one connection,
  but two clients are ESTABLISHED.

It now immediately flushes packets with no payload, as they are important to
TCP connection state and management.

PiperOrigin-RevId: 516870932
This commit is contained in:
Kevin Krakauer
2023-03-15 10:54:43 -07:00
committed by gVisor bot
parent cc1f80913f
commit c122d8d6c8
4 changed files with 27 additions and 15 deletions
+3 -2
View File
@@ -226,6 +226,8 @@ func (gb *groBucket) found(gd *groDispatcher, groPkt *groPacket, flushGROPkt boo
// Flush groPkt or merge the packets.
pktSize := pkt.Data().Size()
flags := tcpHdr.Flags()
dataOff := tcpHdr.DataOffset()
tcpPayloadSize := pkt.Data().Size() - len(ipHdr) - int(dataOff)
if flushGROPkt {
// Flush the existing GRO packet. Don't hold bucket.mu while
// processing the packet.
@@ -239,12 +241,10 @@ func (gb *groBucket) found(gd *groDispatcher, groPkt *groPacket, flushGROPkt boo
} else if groPkt != nil {
// Merge pkt in to GRO packet.
buf := pkt.Data().ToBuffer()
dataOff := tcpHdr.DataOffset()
buf.TrimFront(int64(len(ipHdr)) + int64(dataOff))
groPkt.pkt.Data().MergeBuffer(&buf)
buf.Release()
// Update the IP total length.
tcpPayloadSize := pkt.Data().Size() - len(ipHdr) - int(dataOff)
updateIPHdr(groPkt.ipHdr, tcpPayloadSize)
// Add flags from the packet to the GRO packet.
groPkt.tcpHdr.SetFlags(uint8(groPkt.tcpHdr.Flags() | (flags & (header.TCPFlagFin | header.TCPFlagPsh))))
@@ -261,6 +261,7 @@ func (gb *groBucket) found(gd *groDispatcher, groPkt *groPacket, flushGROPkt boo
// malformed, a local GSO packet, or has already been handled by host
// GRO.
flush := header.TCPFlags(flags)&(header.TCPFlagUrg|header.TCPFlagPsh|header.TCPFlagRst|header.TCPFlagSyn|header.TCPFlagFin) != 0
flush = flush || tcpPayloadSize == 0
if groPkt != nil {
flush = flush || pktSize != groPkt.initialLength
}
+13 -7
View File
@@ -121,17 +121,19 @@ type XDPLink struct {
LinkAddress net.HardwareAddr
QDisc config.QueueingDiscipline
Neighbors []Neighbor
GvisorGROTimeout time.Duration
// NumChannels controls how many underlying FDs are to be used to
// create this endpoint.
NumChannels int
}
// LoopbackLink configures a loopback li nk.
// LoopbackLink configures a loopback link.
type LoopbackLink struct {
Name string
Addresses []IPWithPrefix
Routes []Route
Name string
Addresses []IPWithPrefix
Routes []Route
GvisorGROTimeout time.Duration
}
// CreateLinksAndRoutesArgs are arguments to CreateLinkAndRoutes.
@@ -216,7 +218,10 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct
linkEP := packetsocket.New(ethernet.New(loopback.New()))
log.Infof("Enabling loopback interface %q with id %d on addresses %+v", link.Name, nicID, link.Addresses)
opts := stack.NICOptions{Name: link.Name}
opts := stack.NICOptions{
Name: link.Name,
GROTimeout: link.GvisorGROTimeout,
}
if err := n.createNICWithAddrs(nicID, linkEP, opts, link.Addresses); err != nil {
return err
}
@@ -383,8 +388,9 @@ func (n *Network) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct
log.Infof("Enabling interface %q with id %d on addresses %+v (%v) w/ %d channels", link.Name, nicID, link.Addresses, mac, link.NumChannels)
opts := stack.NICOptions{
Name: link.Name,
QDisc: qDisc,
Name: link.Name,
QDisc: qDisc,
GROTimeout: link.GvisorGROTimeout,
}
if err := n.createNICWithAddrs(nicID, sniffEP, opts, link.Addresses); err != nil {
return err
+10 -6
View File
@@ -60,7 +60,7 @@ func setupNetwork(conn *urpc.Client, pid int, conf *config.Config) error {
switch conf.Network {
case config.NetworkNone:
log.Infof("Network is disabled, create loopback interface only")
if err := createDefaultLoopbackInterface(conn); err != nil {
if err := createDefaultLoopbackInterface(conf, conn); err != nil {
return fmt.Errorf("creating default loopback interface: %v", err)
}
case config.NetworkSandbox:
@@ -78,9 +78,11 @@ func setupNetwork(conn *urpc.Client, pid int, conf *config.Config) error {
return nil
}
func createDefaultLoopbackInterface(conn *urpc.Client) error {
func createDefaultLoopbackInterface(conf *config.Config, conn *urpc.Client) error {
link := boot.DefaultLoopbackLink
link.GvisorGROTimeout = conf.GvisorGROTimeout
if err := conn.Call(boot.NetworkCreateLinksAndRoutes, &boot.CreateLinksAndRoutesArgs{
LoopbackLinks: []boot.LoopbackLink{boot.DefaultLoopbackLink},
LoopbackLinks: []boot.LoopbackLink{link},
}, nil); err != nil {
return fmt.Errorf("creating loopback link and routes: %v", err)
}
@@ -157,7 +159,7 @@ func createInterfacesAndRoutesFromNS(conn *urpc.Client, nsPath string, conf *con
// We build our own loopback device.
if iface.Flags&net.FlagLoopback != 0 {
link, err := loopbackLink(iface, allAddrs)
link, err := loopbackLink(conf, iface, allAddrs)
if err != nil {
return fmt.Errorf("getting loopback link for iface %q: %w", iface.Name, err)
}
@@ -261,6 +263,7 @@ func createInterfacesAndRoutesFromNS(conn *urpc.Client, nsPath string, conf *con
Neighbors: neighbors,
LinkAddress: linkAddress,
Addresses: addresses,
GvisorGROTimeout: conf.GvisorGROTimeout,
})
} else {
link := boot.FDBasedLink{
@@ -492,9 +495,10 @@ func createSocketXDP(iface net.Interface) ([]*os.File, error) {
// loopbackLink returns the link with addresses and routes for a loopback
// interface.
func loopbackLink(iface net.Interface, addrs []net.Addr) (boot.LoopbackLink, error) {
func loopbackLink(conf *config.Config, iface net.Interface, addrs []net.Addr) (boot.LoopbackLink, error) {
link := boot.LoopbackLink{
Name: iface.Name,
Name: iface.Name,
GvisorGROTimeout: conf.GvisorGROTimeout,
}
for _, addr := range addrs {
ipNet, ok := addr.(*net.IPNet)
+1
View File
@@ -229,6 +229,7 @@ func runRunsc(tc *gtest.TestCase, spec *specs.Spec) error {
"-watchdog-action=panic",
"-platform", *platform,
"-file-access", *fileAccess,
"-gvisor-gro=200000ns",
}
if *network == "host" && !testutil.TestEnvSupportsRawSockets {