Skip to content

Commit

Permalink
Do Get before Set of dataplane route.
Browse files Browse the repository at this point in the history
Get is less expensive so it's worth it if the route hasn't
changed (the mainline).
  • Loading branch information
fasaxc committed Aug 23, 2024
1 parent 5792403 commit fe7dde7
Showing 1 changed file with 25 additions and 20 deletions.
45 changes: 25 additions & 20 deletions felix/routetable/route_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -801,12 +801,16 @@ func (r *RouteTable) doFullResync(nl netlinkshim.Interface) error {
// once but avoid leaking the function parameter.
scratchRoute = route
r.onIfaceSeen(route.LinkIndex)
kernKey, kernRoute, ok := r.netlinkRouteToKernelRoute(&scratchRoute)
if !ok {

if !r.routeIsOurs(&scratchRoute) {
// Not a route that we're managing.
return true
}
r.kernelRoutes.Dataplane().Set(kernKey, kernRoute)

kernKey, kernRoute := r.netlinkRouteToKernelRoute(&scratchRoute)
if oldRoute, ok := r.kernelRoutes.Dataplane().Get(kernKey); !ok || oldRoute != kernRoute {
r.kernelRoutes.Dataplane().Set(kernKey, kernRoute)
}
seenKeys.Add(kernKey)
r.livenessCallback()
return true
Expand Down Expand Up @@ -900,13 +904,16 @@ func (r *RouteTable) resyncIface(nl netlinkshim.Interface, ifaceName string) err
// This copy avoids an alloc per iteration. We leak scratchRoute
// once but avoid leaking the function parameter.
scratchRoute = route
kernKey, kernRoute, ok := r.netlinkRouteToKernelRoute(&scratchRoute)
if !ok {
// Not a route that we're managing, so we don't want it to
// be a candidate for us to delete.

if !r.routeIsOurs(&scratchRoute) {
// Not a route that we're managing.
return true
}
r.kernelRoutes.Dataplane().Set(kernKey, kernRoute)

kernKey, kernRoute := r.netlinkRouteToKernelRoute(&scratchRoute)
if oldRoute, ok := r.kernelRoutes.Dataplane().Get(kernKey); !ok || oldRoute != kernRoute {
r.kernelRoutes.Dataplane().Set(kernKey, kernRoute)
}
seenRoutes.Add(kernKey)
return true
})
Expand Down Expand Up @@ -1069,23 +1076,19 @@ func (r *RouteTable) routeKeyForCIDR(cidr ip.CIDR) kernelRouteKey {
return kernelRouteKey{CIDR: cidr}
}

// netlinkRouteToKernelRoute converts (only) routes that we own back
// to our kernelRoute/Key structs (returning ok=true). Other routes
// are ignored and returned with ok = false.
func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (kernKey kernelRouteKey, kernRoute kernelRoute, ok bool) {
func (r *RouteTable) routeIsOurs(route *netlink.Route) bool {
// We're on the hot path, so it's worth avoiding the overheads of
// WithField(s) if debug is disabled.
debug := log.IsLevelEnabled(log.DebugLevel)
logCxt := r.logCxt
if debug {
if log.IsLevelEnabled(log.DebugLevel) {
logCxt = logCxt.WithField("route", route)
}
ifaceName := ""
if routeIsSpecialNoIfRoute(route) {
ifaceName = InterfaceNone
} else if routeIsIPv6Bootstrap(route) {
logCxt.Debug("Ignoring IPv6 bootstrap route, kernel manages these.")
return
return false
} else {
ifaceName = r.ifaceIndexToName[route.LinkIndex]
if ifaceName == "" {
Expand All @@ -1095,15 +1098,18 @@ func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (kernKey ke
// a route for a just-deleted interface, in which case
// we don't care.
logCxt.Debug("Ignoring route for unknown iface")
return
return false
}
}

if !r.ownershipPolicy.RouteIsOurs(ifaceName, route) {
logCxt.Debug("Ignoring route (it doesn't belong to us).")
return
return false
}
return true
}

func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (kernKey kernelRouteKey, kernRoute kernelRoute) {
kernKey = kernelRouteKey{
CIDR: ip.CIDRFromIPNet(route.Dst),
Priority: route.Priority,
Expand All @@ -1118,13 +1124,12 @@ func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (kernKey ke
OnLink: route.Flags&unix.RTNH_F_ONLINK != 0,
Protocol: route.Protocol,
}
if debug {
logCxt.WithFields(log.Fields{
if log.IsLevelEnabled(log.DebugLevel) {
r.logCxt.WithFields(log.Fields{
"kernRoute": kernRoute,
"kernKey": kernKey,
}).Debug("Loaded route from kernel.")
}
ok = true
return
}

Expand Down

0 comments on commit fe7dde7

Please sign in to comment.