From 30ffafcda0390a5781ca0087a32d463bbe91f695 Mon Sep 17 00:00:00 2001 From: Lucas Manning Date: Wed, 10 Jul 2024 11:33:01 -0700 Subject: [PATCH] Automated rollback of changelist 650779522 PiperOrigin-RevId: 651095633 --- pkg/tcpip/stack/nic.go | 12 +++++------- pkg/tcpip/stack/stack.go | 37 ++++++++++--------------------------- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index a4fcc8fba..a2520d663 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -311,10 +311,7 @@ func (n *nic) enable() tcpip.Error { // remove detaches NIC from the link endpoint and releases network endpoint // resources. This guarantees no packets between this NIC and the network // stack. -// -// It returns an action that has to be excuted after releasing the Stack lock -// and any error encountered. -func (n *nic) remove(closeLinkEndpoint bool) (func(), tcpip.Error) { +func (n *nic) remove(closeLinkEndpoint bool) tcpip.Error { n.enableDisableMu.Lock() n.disableLocked() @@ -329,7 +326,6 @@ func (n *nic) remove(closeLinkEndpoint bool) (func(), tcpip.Error) { // We must not hold n.enableDisableMu here. n.linkResQueue.cancel() - var deferAct func() // Prevent packets from going down to the link before shutting the link down. n.qDisc.Close() n.NetworkLinkEndpoint.Attach(nil) @@ -339,10 +335,12 @@ func (n *nic) remove(closeLinkEndpoint bool) (func(), tcpip.Error) { // The link endpoint has to be closed without holding a // netstack lock, because it can trigger other netstack // operations. - deferAct = ep.Close + go func() { + ep.Close() + }() } - return deferAct, nil + return nil } // setPromiscuousMode enables or disables promiscuous mode. diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 7dc7cd357..23baeefd1 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -1001,28 +1001,25 @@ func (s *Stack) CheckNIC(id tcpip.NICID) bool { // RemoveNIC removes NIC and all related routes from the network stack. func (s *Stack) RemoveNIC(id tcpip.NICID) tcpip.Error { s.mu.Lock() - deferAct, err := s.removeNICLocked(id) - s.mu.Unlock() - if deferAct != nil { - deferAct() - } - return err + defer s.mu.Unlock() + + return s.removeNICLocked(id) } // removeNICLocked removes NIC and all related routes from the network stack. // // +checklocks:s.mu -func (s *Stack) removeNICLocked(id tcpip.NICID) (func(), tcpip.Error) { +func (s *Stack) removeNICLocked(id tcpip.NICID) tcpip.Error { nic, ok := s.nics[id] if !ok { - return nil, &tcpip.ErrUnknownNICID{} + return &tcpip.ErrUnknownNICID{} } delete(s.nics, id) if nic.Primary != nil { b := nic.Primary.NetworkLinkEndpoint.(CoordinatorNIC) if err := b.DelNIC(nic); err != nil { - return nil, err + return err } } @@ -1927,22 +1924,14 @@ func (s *Stack) Wait() { p.Wait() } - deferActs := make([]func(), 0) - s.mu.Lock() + defer s.mu.Unlock() + for id, n := range s.nics { // Remove NIC to ensure that qDisc goroutines are correctly // terminated on stack teardown. - act, _ := s.removeNICLocked(id) + s.removeNICLocked(id) n.NetworkLinkEndpoint.Wait() - if act != nil { - deferActs = append(deferActs, act) - } - } - s.mu.Unlock() - - for _, act := range deferActs { - act() } } @@ -2387,14 +2376,8 @@ func (s *Stack) SetNICStack(id tcpip.NICID, peer *Stack) (tcpip.NICID, tcpip.Err // Remove routes in-place. n tracks the number of routes written. s.RemoveRoutes(func(r tcpip.Route) bool { return r.NIC == id }) ne := nic.NetworkLinkEndpoint.(LinkEndpoint) - deferAct, err := nic.remove(false /* closeLinkEndpoint */) + nic.remove(false /* closeLinkEndpoint */) s.mu.Unlock() - if deferAct != nil { - deferAct() - } - if err != nil { - return 0, err - } id = tcpip.NICID(peer.NextNICID()) return id, peer.CreateNICWithOptions(id, ne, NICOptions{Name: nic.Name()})