Skip to content

Commit

Permalink
service/lbmap: Consider active backends while cleaning up service map…
Browse files Browse the repository at this point in the history
… entries

Service entry currently keeps a count of previous count of backend
entries so that when service backends are updated, the BPF service
map entries for old backends can be deleted.

However, we skip adding terminating backend entries to the services
BPF map so that it's not selected for new connections. As a result,
we need to keep track of only the active backends (aka non-terminating)
while cleaning up obsolete backend entries.

See PR #17716 for more details about how terminating backends are
processed for graceful removal.

Signed-off-by: Aditi Ghag <aditi@cilium.io>
  • Loading branch information
aditighag authored and borkmann committed Nov 12, 2021
1 parent b79a4a5 commit 94f67ec
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 23 deletions.
9 changes: 5 additions & 4 deletions pkg/maps/lbmap/lbmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type UpsertServiceParams struct {
IP net.IP
Port uint16
Backends map[string]loadbalancer.BackendID
PrevBackendCount int
PrevActiveBackendCount int
IPv6 bool
Type loadbalancer.SVCType
Local bool
Expand All @@ -72,8 +72,9 @@ type UpsertServiceParams struct {
// The corresponding backend entries (identified with the given backendIDs)
// have to exist before calling the function.
//
// The given prevBackendCount denotes a previous service backend entries count,
// so that the function can remove obsolete ones.
// The service's prevActiveBackendCount denotes the count of previously active
// backend entries that were added to the BPF map so that the function can remove
// obsolete ones.
func (lbmap *LBBPFMap) UpsertService(p *UpsertServiceParams) error {
var svcKey ServiceKey

Expand Down Expand Up @@ -136,7 +137,7 @@ func (lbmap *LBBPFMap) UpsertService(p *UpsertServiceParams) error {
return fmt.Errorf("Unable to update service %+v: %s", svcKey, err)
}

for i := slot; i <= p.PrevBackendCount; i++ {
for i := slot; i <= p.PrevActiveBackendCount; i++ {
svcKey.SetBackendSlot(i)
if err := deleteServiceLocked(svcKey); err != nil {
log.WithFields(logrus.Fields{
Expand Down
38 changes: 21 additions & 17 deletions pkg/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ type monitorNotify interface {
}

type svcInfo struct {
hash string
frontend lb.L3n4AddrID
backends []lb.Backend
backendByHash map[string]*lb.Backend
hash string
frontend lb.L3n4AddrID
backends []lb.Backend
activeBackendsCount int // Non-terminating backends count
backendByHash map[string]*lb.Backend

svcType lb.SVCType
svcTrafficPolicy lb.SVCTrafficPolicy
Expand Down Expand Up @@ -355,7 +356,7 @@ func (s *Service) UpsertService(params *lb.SVC) (bool, lb.ID, error) {
scopedLog.Debug("Acquired service ID")

onlyLocalBackends, filterBackends := svc.requireNodeLocalBackends(params.Frontend)
prevBackendCount := len(svc.backends)
prevActiveBackendCount := svc.activeBackendsCount

backendsCopy := []lb.Backend{}
for _, b := range params.Backends {
Expand All @@ -380,10 +381,9 @@ func (s *Service) UpsertService(params *lb.SVC) (bool, lb.ID, error) {
}

// Update lbmaps (BPF service maps)
if err = s.upsertServiceIntoLBMaps(svc, onlyLocalBackends, prevBackendCount, newBackends,
obsoleteBackendIDs, prevSessionAffinity,
prevLoadBalancerSourceRanges, obsoleteSVCBackendIDs,
scopedLog); err != nil {
if err = s.upsertServiceIntoLBMaps(svc, onlyLocalBackends, prevActiveBackendCount,
newBackends, obsoleteBackendIDs, prevSessionAffinity, prevLoadBalancerSourceRanges,
obsoleteSVCBackendIDs, scopedLog); err != nil {

return false, lb.ID(0), err
}
Expand Down Expand Up @@ -718,7 +718,7 @@ func (s *Service) addBackendsToAffinityMatchMap(svcID lb.ID, backendIDs []lb.Bac
}

func (s *Service) upsertServiceIntoLBMaps(svc *svcInfo, onlyLocalBackends bool,
prevBackendCount int, newBackends []lb.Backend, obsoleteBackendIDs []lb.BackendID,
prevActiveBackendCount int, newBackends []lb.Backend, obsoleteBackendIDs []lb.BackendID,
prevSessionAffinity bool, prevLoadBalancerSourceRanges []*cidr.CIDR,
obsoleteSVCBackendIDs []lb.BackendID, scopedLog *logrus.Entry) error {

Expand Down Expand Up @@ -786,6 +786,7 @@ func (s *Service) upsertServiceIntoLBMaps(svc *svcInfo, onlyLocalBackends bool,

// Upsert service entries into BPF maps
backends := make(map[string]lb.BackendID, len(svc.backends))
activeBackendsCount := 0
for _, b := range svc.backends {
// Skip adding the terminating backend to the service map so that it
// won't be selected to serve new requests. However, the backend is still
Expand All @@ -796,15 +797,17 @@ func (s *Service) upsertServiceIntoLBMaps(svc *svcInfo, onlyLocalBackends bool,
// list passed to this function.
if !b.Terminating {
backends[b.String()] = b.ID
activeBackendsCount++
}
}
svc.activeBackendsCount = activeBackendsCount

p := &lbmap.UpsertServiceParams{
ID: uint16(svc.frontend.ID),
IP: svc.frontend.L3n4Addr.IP,
Port: svc.frontend.L3n4Addr.L4Addr.Port,
Backends: backends,
PrevBackendCount: prevBackendCount,
PrevActiveBackendCount: prevActiveBackendCount,
IPv6: ipv6,
Type: svc.svcType,
Local: onlyLocalBackends,
Expand Down Expand Up @@ -897,12 +900,13 @@ func (s *Service) restoreServicesLocked() error {
}

newSVC := &svcInfo{
hash: svc.Frontend.Hash(),
frontend: svc.Frontend,
backends: svc.Backends,
backendByHash: map[string]*lb.Backend{},
svcType: svc.Type,
svcTrafficPolicy: svc.TrafficPolicy,
hash: svc.Frontend.Hash(),
frontend: svc.Frontend,
backends: svc.Backends,
activeBackendsCount: len(svc.Backends),
backendByHash: map[string]*lb.Backend{},
svcType: svc.Type,
svcTrafficPolicy: svc.TrafficPolicy,

sessionAffinity: svc.SessionAffinity,
sessionAffinityTimeoutSec: svc.SessionAffinityTimeoutSec,
Expand Down
4 changes: 2 additions & 2 deletions pkg/testutils/mockmaps/lbmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func (m *LBMockMap) UpsertService(p *lbmap.UpsertServiceParams) error {
frontend := lb.NewL3n4AddrID(lb.NONE, p.IP, p.Port, p.Scope, lb.ID(p.ID))
svc = &lb.SVC{Frontend: *frontend}
} else {
if p.PrevBackendCount != len(svc.Backends) {
return fmt.Errorf("Invalid backends count: %d vs %d", p.PrevBackendCount, len(svc.Backends))
if p.PrevActiveBackendCount != len(svc.Backends) {
return fmt.Errorf("Invalid backends count: %d vs %d", p.PrevActiveBackendCount, len(svc.Backends))
}
}
svc.Backends = backendsList
Expand Down

0 comments on commit 94f67ec

Please sign in to comment.