Skip to content

Commit

Permalink
network: Don't store the network info as pointers if slices used
Browse files Browse the repository at this point in the history
By storing the network information such as routes as pointers, we
end up having issues if we try to remove some elements from the list
as at the same time this list is parsed through by a loop.

For interfaces, we can use maps instead, because we have a way to
keep track of them based on their names. Maps are safer and allow
to keep pointers in this case.

In both cases, routes and interfaces list can be modified properly
while we loop over every items.

Fixes kata-containers#226

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
  • Loading branch information
Sebastien Boeuf committed Apr 30, 2018
1 parent 74e720e commit 1f5cf20
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 12 deletions.
1 change: 1 addition & 0 deletions grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,7 @@ func (a *agentGRPC) CreateSandbox(ctx context.Context, req *pb.CreateSandboxRequ

a.sandbox.id = req.Hostname
a.sandbox.containers = make(map[string]*container)
a.sandbox.network.ifaces = make(map[string]*pb.Interface)
a.sandbox.network.dns = req.Dns
a.sandbox.running = true

Expand Down
20 changes: 8 additions & 12 deletions network.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ import (
// related information.
type network struct {
ifacesLock sync.Mutex
ifaces []*pb.Interface
ifaces map[string]*pb.Interface

routesLock sync.Mutex
routes []*pb.Route
routes []pb.Route

dns []string
}
Expand Down Expand Up @@ -139,7 +139,7 @@ func (s *sandbox) addInterface(netHandle *netlink.Handle, iface *pb.Interface) (
}

// Update sandbox interface list.
s.network.ifaces = append(s.network.ifaces, iface)
s.network.ifaces[iface.Name] = iface

return iface, nil
}
Expand Down Expand Up @@ -172,12 +172,7 @@ func (s *sandbox) removeInterface(netHandle *netlink.Handle, iface *pb.Interface
}

// Update sandbox interface list.
for idx, sIface := range s.network.ifaces {
if sIface.Name == iface.Name {
s.network.ifaces = append(s.network.ifaces[:idx], s.network.ifaces[idx+1:]...)
break
}
}
delete(s.network.ifaces, iface.Name)

return nil, nil
}
Expand Down Expand Up @@ -465,7 +460,7 @@ func (s *sandbox) updateRoute(netHandle *netlink.Handle, route *pb.Route, add bo
}

// Add route to sandbox route list.
s.network.routes = append(s.network.routes, route)
s.network.routes = append(s.network.routes, *route)
} else {
if err := netHandle.RouteDel(netRoute); err != nil {
return grpcStatus.Errorf(codes.Internal, "Could not remove route dest(%s)/gw(%s)/dev(%s): %v",
Expand Down Expand Up @@ -508,8 +503,9 @@ func (s *sandbox) removeNetwork() error {
}
defer netHandle.Delete()

for _, route := range s.network.routes {
if err := s.removeRoute(netHandle, route); err != nil {
routeList := s.network.routes
for _, route := range routeList {
if err := s.removeRoute(netHandle, &route); err != nil {
return grpcStatus.Errorf(codes.Internal, "Could not remove network route %v: %v",
route, err)
}
Expand Down

0 comments on commit 1f5cf20

Please sign in to comment.