Skip to content

Commit d66c288

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request #426 from anfredette/check-inodes
Check for Duplicate Network Namespace Inodes
2 parents 2f9d586 + eb2a3fd commit d66c288

File tree

14 files changed

+293
-78
lines changed

14 files changed

+293
-78
lines changed

‎Makefile‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,6 @@ build-agent-image: ## Build bpfman-agent image.
362362
push-images: ## Push bpfman-agent and bpfman-operator images.
363363
$(OCI_BIN) push ${BPFMAN_OPERATOR_IMG}
364364
$(OCI_BIN) push ${BPFMAN_AGENT_IMG}
365-
$(OCI_BIN) push ${BPFMAN_IMG}
366365

367366
.PHONY: load-images-kind
368367
load-images-kind: ## Load bpfman-agent, and bpfman-operator images into the running local kind devel cluster.

‎cmd/bpfman-agent/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,11 +267,11 @@ func interfaceListener(ctx context.Context, ifaceEvents <-chan ifaces.Event, int
267267
iface := event.Interface
268268
switch event.Type {
269269
case ifaces.EventAdded:
270-
setupLog.Info("interface created", "Name", iface.Name, "netns", iface.NetNS)
270+
setupLog.Info("interface created", "Name", iface.Name, "netns", iface.NSName, "NsHandle", iface.NetNS)
271271
interfaces.Store(iface, true)
272272
case ifaces.EventDeleted:
273-
setupLog.Info("interface deleted", "Name", iface.Name, "netns", iface.NetNS)
274-
interfaces.Store(iface, false)
273+
setupLog.Info("interface deleted", "Name", iface.Name, "netns", iface.NSName, "NsHandle", iface.NetNS)
274+
interfaces.Delete(iface)
275275
default:
276276
}
277277
}

‎config/bpfman-deployment/daemonset.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ spec:
147147
name: host-dockerd
148148
- mountPath: /etc/crictl.yaml
149149
name: host-crictl-config
150+
- mountPath: /host/proc
151+
name: host-proc
152+
mountPropagation: HostToContainer
150153
- mountPath: /var/run/netns
151154
name: var-run-netns
152155
mountPropagation: HostToContainer

‎controllers/bpfman-agent/cl_application_program.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,9 @@ func (r *ClBpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
125125
r.Logger = ctrl.Log.WithName("cluster-app")
126126
r.finalizer = internal.ClBpfApplicationControllerFinalizer
127127
r.recType = internal.ApplicationString
128+
r.NetnsCache = make(map[string]uint64)
128129

129-
r.Logger.Info("Enter BpfApplication Reconcile", "Name", req.Name)
130+
r.Logger.Info("Enter ClusterBpfApplication Reconcile", "Name", req.Name)
130131

131132
// Lookup K8s node object for this bpfman-agent This should always succeed
132133
if err := r.Get(ctx, types.NamespacedName{Namespace: v1.NamespaceAll, Name: r.NodeName}, r.ourNode); err != nil {

‎controllers/bpfman-agent/cl_tc_program.go‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -168,12 +168,17 @@ func (r *ClTcProgramReconciler) updateLinks(ctx context.Context, isBeingDeleted
168168

169169
if r.currentProgram.TC != nil && r.currentProgram.TC.Links != nil {
170170
for _, attachInfo := range r.currentProgram.TC.Links {
171-
expectedLinks, error := r.getExpectedLinks(ctx, attachInfo)
172-
if error != nil {
173-
return fmt.Errorf("failed to get node links: %v", error)
171+
expectedLinks, err := r.getExpectedLinks(ctx, attachInfo)
172+
if err != nil {
173+
r.Logger.V(1).Info("updateLinks() failed", "error", err)
174+
return fmt.Errorf("failed to get node links: %v", err)
174175
}
175176
for _, link := range expectedLinks {
176-
index := r.findLink(link)
177+
index, err := r.findLink(link)
178+
if err != nil {
179+
r.Logger.Info("Error", "Invalid link", r.printAttachInfo(link), "Error", err)
180+
continue
181+
}
177182
if index != nil {
178183
// Link already exists, so set ShouldAttach to true.
179184
r.currentProgramState.TC.Links[*index].AttachInfoStateCommon.ShouldAttach = true
@@ -194,18 +199,36 @@ func (r *ClTcProgramReconciler) updateLinks(ctx context.Context, isBeingDeleted
194199
return nil
195200
}
196201

197-
func (r *ClTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcAttachInfoState) *int {
202+
func (r *ClTcProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha1.ClTcAttachInfoState) string {
203+
var netnsPath string
204+
if attachInfoState.NetnsPath == "" {
205+
netnsPath = "host"
206+
} else {
207+
netnsPath = attachInfoState.NetnsPath
208+
}
209+
210+
return fmt.Sprintf("interfaceName: %s, netnsPath: %s, direction: %s, priority: %d",
211+
attachInfoState.InterfaceName, netnsPath, attachInfoState.Direction, attachInfoState.Priority)
212+
}
213+
214+
func (r *ClTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcAttachInfoState) (*int, error) {
215+
newNetnsId := r.getNetnsId(attachInfoState.NetnsPath)
216+
if newNetnsId == nil {
217+
return nil, fmt.Errorf("failed to get netnsId for path %s", attachInfoState.NetnsPath)
218+
}
219+
r.Logger.V(1).Info("findlink", "New Path", attachInfoState.NetnsPath, "NetnsId", newNetnsId)
198220
for i, a := range r.currentProgramState.TC.Links {
199221
// attachInfoState is the same as a if the the following fields are the
200-
// same: InterfaceName, Direction, Priority, NetnsPath, and ProceedOn.
201-
if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction &&
222+
// same: InterfaceName, Direction, Priority, ProceedOn, and network namespace.
223+
if a.InterfaceName == attachInfoState.InterfaceName &&
224+
a.Direction == attachInfoState.Direction &&
202225
a.Priority == attachInfoState.Priority &&
203-
reflect.DeepEqual(a.NetnsPath, attachInfoState.NetnsPath) &&
204-
reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) {
205-
return &i
226+
reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) &&
227+
reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) {
228+
return &i, nil
206229
}
207230
}
208-
return nil
231+
return nil, nil
209232
}
210233

211234
// processLinks calls reconcileBpfLink() for each link. It
@@ -291,22 +314,24 @@ func (r *ClTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo
291314

292315
// Handle interface discovery
293316
if isInterfacesDiscoveryEnabled(&attachInfo.InterfaceSelector) {
294-
discoveredInterfaces, err := getDiscoveredInterfaces(&attachInfo.InterfaceSelector, r.Interfaces)
295-
if err != nil {
296-
return nil, fmt.Errorf("failed to discover interfaces: %w", err)
297-
}
317+
discoveredInterfaces := getDiscoveredInterfaces(&attachInfo.InterfaceSelector, r.Interfaces)
318+
r.Logger.Info("getExpectedLinks", "num discoveredInterfaces", len(discoveredInterfaces))
298319
for _, intf := range discoveredInterfaces {
299320
nodeLinks = append(nodeLinks, createLinkEntry(intf.interfaceName, intf.netNSPath))
300321
}
322+
r.Logger.V(1).Info("getExpectedLinks-discovery", "Links created", len(nodeLinks))
301323
return nodeLinks, nil
302324
}
303325

304326
// Fetch interfaces if discovery is disabled
305327
interfaces, err := getInterfaces(&attachInfo.InterfaceSelector, r.ourNode)
306328
if err != nil {
329+
r.Logger.V(1).Info("getExpectedLinks failed to get interfaces", "error", err)
307330
return nil, fmt.Errorf("failed to get interfaces for XdpProgram: %w", err)
308331
}
309332

333+
r.Logger.Info("getExpectedLinks", "Number of interfaces", len(interfaces))
334+
310335
// Handle network namespaces if provided
311336
if attachInfo.NetworkNamespaces != nil {
312337
containerInfo, err := r.Containers.GetContainers(
@@ -317,6 +342,7 @@ func (r *ClTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo
317342
r.Logger,
318343
)
319344
if err != nil {
345+
r.Logger.V(1).Info("getExpectedLinks failed to get container pids", "error", err)
320346
return nil, fmt.Errorf("failed to get container pids: %w", err)
321347
}
322348

@@ -332,6 +358,7 @@ func (r *ClTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo
332358
nodeLinks = append(nodeLinks, createLinkEntry(iface, netnsPath))
333359
}
334360
}
361+
r.Logger.V(1).Info("getExpectedLinks", "Links created", len(nodeLinks))
335362
return nodeLinks, nil
336363
}
337364

@@ -340,6 +367,7 @@ func (r *ClTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo
340367
nodeLinks = append(nodeLinks, createLinkEntry(iface, ""))
341368
}
342369

370+
r.Logger.V(1).Info("getExpectedLinks", "Links created", len(nodeLinks))
343371
return nodeLinks, nil
344372
}
345373

‎controllers/bpfman-agent/cl_tcx_program.go‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,17 @@ func (r *ClTcxProgramReconciler) updateLinks(ctx context.Context, isBeingDeleted
133133

134134
if r.currentProgram.TCX != nil && r.currentProgram.TCX.Links != nil {
135135
for _, attachInfo := range r.currentProgram.TCX.Links {
136-
expectedLinks, error := r.getExpectedLinks(ctx, attachInfo)
137-
if error != nil {
138-
return fmt.Errorf("failed to get node links: %v", error)
136+
expectedLinks, err := r.getExpectedLinks(ctx, attachInfo)
137+
if err != nil {
138+
r.Logger.V(1).Info("updateLinks() failed", "error", err)
139+
return fmt.Errorf("failed to get node links: %v", err)
139140
}
140141
for _, link := range expectedLinks {
141-
index := r.findLink(link)
142+
index, err := r.findLink(link)
143+
if err != nil {
144+
r.Logger.Info("Error", "Invalid link", r.printAttachInfo(link), "Error", err)
145+
continue
146+
}
142147
if index != nil {
143148
// Link already exists, so set ShouldAttach to true.
144149
r.currentProgramState.TCX.Links[*index].AttachInfoStateCommon.ShouldAttach = true
@@ -159,17 +164,35 @@ func (r *ClTcxProgramReconciler) updateLinks(ctx context.Context, isBeingDeleted
159164
return nil
160165
}
161166

162-
func (r *ClTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcxAttachInfoState) *int {
167+
func (r *ClTcxProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha1.ClTcxAttachInfoState) string {
168+
var netnsPath string
169+
if attachInfoState.NetnsPath == "" {
170+
netnsPath = "host"
171+
} else {
172+
netnsPath = attachInfoState.NetnsPath
173+
}
174+
175+
return fmt.Sprintf("interfaceName: %s, netnsPath: %s, direction: %s, priority: %d",
176+
attachInfoState.InterfaceName, netnsPath, attachInfoState.Direction, attachInfoState.Priority)
177+
}
178+
179+
func (r *ClTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcxAttachInfoState) (*int, error) {
180+
newNetnsId := r.getNetnsId(attachInfoState.NetnsPath)
181+
if newNetnsId == nil {
182+
return nil, fmt.Errorf("failed to get netnsId for path %s", attachInfoState.NetnsPath)
183+
}
184+
r.Logger.V(1).Info("findlink", "New Path", attachInfoState.NetnsPath, "NetnsId", newNetnsId)
163185
for i, a := range r.currentProgramState.TCX.Links {
164186
// attachInfoState is the same as a if the the following fields are the
165-
// same: InterfaceName, NetnsPath, Priority, and Direction.
166-
if a.InterfaceName == attachInfoState.InterfaceName && a.Priority == attachInfoState.Priority &&
187+
// same: InterfaceName, Direction, Priority, and network namespace.
188+
if a.InterfaceName == attachInfoState.InterfaceName &&
167189
a.Direction == attachInfoState.Direction &&
168-
reflect.DeepEqual(a.NetnsPath, attachInfoState.NetnsPath) {
169-
return &i
190+
a.Priority == attachInfoState.Priority &&
191+
reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) {
192+
return &i, nil
170193
}
171194
}
172-
return nil
195+
return nil, nil
173196
}
174197

175198
// processLinks calls reconcileBpfLink() for each link. It
@@ -254,22 +277,25 @@ func (r *ClTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf
254277

255278
// Handle interface discovery
256279
if isInterfacesDiscoveryEnabled(&attachInfo.InterfaceSelector) {
257-
discoveredInterfaces, err := getDiscoveredInterfaces(&attachInfo.InterfaceSelector, r.Interfaces)
258-
if err != nil {
259-
return nil, fmt.Errorf("failed to discover interfaces: %w", err)
260-
}
280+
discoveredInterfaces := getDiscoveredInterfaces(&attachInfo.InterfaceSelector, r.Interfaces)
281+
282+
r.Logger.Info("getExpectedLinks", "num discoveredInterfaces", len(discoveredInterfaces))
261283
for _, intf := range discoveredInterfaces {
262284
nodeLinks = append(nodeLinks, createLinkEntry(intf.interfaceName, intf.netNSPath))
263285
}
286+
r.Logger.V(1).Info("getExpectedLinks-discovery", "Links created", len(nodeLinks))
264287
return nodeLinks, nil
265288
}
266289

267290
// Fetch interfaces if discovery is disabled
268291
interfaces, err := getInterfaces(&attachInfo.InterfaceSelector, r.ourNode)
269292
if err != nil {
293+
r.Logger.V(1).Info("getExpectedLinks failed to get interfaces", "error", err)
270294
return nil, fmt.Errorf("failed to get interfaces for XdpProgram: %w", err)
271295
}
272296

297+
r.Logger.Info("getExpectedLinks", "Number of interfaces", len(interfaces))
298+
273299
// Handle network namespaces if provided
274300
if attachInfo.NetworkNamespaces != nil {
275301
containerInfo, err := r.Containers.GetContainers(
@@ -280,6 +306,7 @@ func (r *ClTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf
280306
r.Logger,
281307
)
282308
if err != nil {
309+
r.Logger.V(1).Info("getExpectedLinks failed to get container pids", "error", err)
283310
return nil, fmt.Errorf("failed to get container pids: %w", err)
284311
}
285312

@@ -295,6 +322,7 @@ func (r *ClTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf
295322
nodeLinks = append(nodeLinks, createLinkEntry(iface, netnsPath))
296323
}
297324
}
325+
r.Logger.V(1).Info("getExpectedLinks", "Links created", len(nodeLinks))
298326
return nodeLinks, nil
299327
}
300328

@@ -303,6 +331,7 @@ func (r *ClTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf
303331
nodeLinks = append(nodeLinks, createLinkEntry(iface, ""))
304332
}
305333

334+
r.Logger.V(1).Info("getExpectedLinks", "Links created", len(nodeLinks))
306335
return nodeLinks, nil
307336
}
308337

0 commit comments

Comments
 (0)