Skip to content

Commit fcc708a

Browse files
adilGhaffarDevadilGhaffarDev
committed
Use temp network namespace for rename
Signed-off-by: Muhammad Adil Ghaffar <[email protected]>
1 parent 21c8cba commit fcc708a

File tree

3 files changed

+352
-136
lines changed

3 files changed

+352
-136
lines changed

‎pkg/sriov/sriov.go‎

Lines changed: 183 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package sriov
22

33
import (
44
"fmt"
5+
"net"
56

67
"github.com/containernetworking/plugins/pkg/ns"
78
"github.com/vishvananda/netlink"
@@ -67,81 +68,96 @@ func NewSriovManager() Manager {
6768
func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error {
6869
linkName := conf.OrigVfState.HostIFName
6970

70-
linkObj, err := s.nLink.LinkByName(linkName)
71+
// Save the original NS in case we need to restore it
72+
// after an error occurs
73+
initns, _ := ns.GetCurrentNS()
74+
75+
tempNS, err := ns.TempNetNS()
7176
if err != nil {
72-
return fmt.Errorf("error getting VF netdevice with name %s", linkName)
77+
return fmt.Errorf("failed to create tempNS: %v", err)
7378
}
79+
defer tempNS.Close()
7480

81+
linkObj, err := s.nLink.LinkByName(linkName)
82+
if err != nil {
83+
return fmt.Errorf("error: %v. Failed to get VF netdevice with name %s", err, linkName)
84+
}
85+
if linkObj.Attrs().Flags&net.FlagUp == net.FlagUp {
86+
defer func() {
87+
if err != nil {
88+
logging.Debug("Setting link up in defer because of ", "error", err)
89+
if linkObj, err := s.nLink.LinkByName(linkName); err == nil {
90+
_ = s.nLink.LinkSetUp(linkObj)
91+
}
92+
}
93+
}()
94+
}
7595
// Save the original effective MAC address before overriding it
7696
conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String()
7797

78-
// tempName used as intermediary name to avoid name conflicts
79-
tempName := fmt.Sprintf("%s%d", "temp_", linkObj.Attrs().Index)
80-
81-
// 1. Set link down
82-
logging.Debug("1. Set link down",
98+
// 1.Move interface to tempNS
99+
logging.Debug("1. Move the interface to tempNS",
83100
"func", "SetupVF",
84101
"linkObj", linkObj)
85-
if err := s.nLink.LinkSetDown(linkObj); err != nil {
86-
return fmt.Errorf("failed to down vf device %q: %v", linkName, err)
87-
}
88-
89-
// 2. Set temp name
90-
logging.Debug("2. Set temp name",
91-
"func", "SetupVF",
92-
"linkObj", linkObj,
93-
"tempName", tempName)
94-
if err := s.nLink.LinkSetName(linkObj, tempName); err != nil {
95-
return fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName)
102+
if err = s.nLink.LinkSetNsFd(linkObj, int(tempNS.Fd())); err != nil {
103+
return fmt.Errorf("failed to move %q to tempNS: %v", linkName, err)
96104
}
105+
err = tempNS.Do(func(linkNS ns.NetNS) error {
106+
// lookup the device in tempNS (index might have changed)
107+
tempNSLinkObj, err := s.nLink.LinkByName(linkName)
108+
if err != nil {
109+
return fmt.Errorf("failed to find %q in tempNS: %v", linkName, err)
110+
}
111+
// Rename the interface to pod interface name
112+
if err = s.nLink.LinkSetName(tempNSLinkObj, podifName); err != nil {
113+
return fmt.Errorf("failed to rename host device %q to %q: %v", linkName, podifName, err)
114+
}
97115

98-
// 3. Remove alt name from the nic
99-
logging.Debug("3. Remove interface original name from alt names",
100-
"func", "SetupVF",
101-
"linkObj", linkObj,
102-
"OriginalLinkName", linkName,
103-
"tempName", tempName)
104-
linkObj, err = s.nLink.LinkByName(tempName)
105-
if err != nil {
106-
return fmt.Errorf("error getting VF netdevice with name %s: %v", tempName, err)
107-
}
108-
for _, altName := range linkObj.Attrs().AltNames {
109-
if altName == linkName {
110-
if err := s.nLink.LinkDelAltName(linkObj, linkName); err != nil {
111-
return fmt.Errorf("error removing VF altname %s: %v", linkName, err)
116+
// 3. Remove alt name from the nic
117+
logging.Debug("3. Remove interface original name from alt names",
118+
"func", "SetupVF",
119+
"tempNSObj", tempNSLinkObj,
120+
"OriginalLinkName", linkName)
121+
for _, altName := range tempNSLinkObj.Attrs().AltNames {
122+
if altName == linkName {
123+
if err = s.nLink.LinkDelAltName(tempNSLinkObj, linkName); err != nil {
124+
return fmt.Errorf("error removing VF altname %s: %v", linkName, err)
125+
}
112126
}
113127
}
114-
}
115128

116-
// 4. Change netns
117-
logging.Debug("4. Change netns",
118-
"func", "SetupVF",
119-
"linkObj", linkObj,
120-
"netns.Fd()", int(netns.Fd()))
121-
if err := s.nLink.LinkSetNsFd(linkObj, int(netns.Fd())); err != nil {
122-
return fmt.Errorf("failed to move IF %s to netns: %q", tempName, err)
129+
// 4. Change netns
130+
logging.Debug("4. Change netns",
131+
"func", "SetupVF",
132+
"tempNSObj", tempNSLinkObj,
133+
"netns.Fd()", int(netns.Fd()))
134+
if err = s.nLink.LinkSetNsFd(tempNSLinkObj, int(netns.Fd())); err != nil {
135+
return fmt.Errorf("failed to move IF %s to netns: %q", podifName, err)
136+
}
137+
return nil
138+
})
139+
if err != nil {
140+
logging.Debug("Move the interface back to initNS because of ", "error", err)
141+
s.cleanupLinkInTempNS(tempNS, podifName, linkName, initns)
142+
return err
123143
}
124144

125-
if err := netns.Do(func(_ ns.NetNS) error {
126-
// 5. Set Pod IF name
127-
logging.Debug("5. Set Pod IF name",
128-
"func", "SetupVF",
129-
"linkObj", linkObj,
130-
"podifName", podifName)
131-
if err := s.nLink.LinkSetName(linkObj, podifName); err != nil {
132-
return fmt.Errorf("error setting container interface name %s for %s", linkName, tempName)
145+
err = netns.Do(func(_ ns.NetNS) error {
146+
netNSLinkObj, err := s.nLink.LinkByName(podifName)
147+
if err != nil {
148+
return fmt.Errorf("error: %v. Failed to get VF netdevice with name %s", err, podifName)
133149
}
134150

135-
// 6. Enable IPv4 ARP notify and IPv6 Network Discovery notify
151+
// 5. Enable IPv4 ARP notify and IPv6 Network Discovery notify
136152
// Error is ignored here because enabling this feature is only a performance enhancement.
137-
logging.Debug("6. Enable IPv4 ARP notify and IPv6 Network Discovery notify",
153+
logging.Debug("5. Enable IPv4 ARP notify and IPv6 Network Discovery notify",
138154
"func", "SetupVF",
139155
"podifName", podifName)
140156
_ = s.utils.EnableArpAndNdiscNotify(podifName)
141157

142-
// 7. Set MAC address
158+
// 6. Set MAC address
143159
if conf.MAC != "" {
144-
logging.Debug("7. Set MAC address",
160+
logging.Debug("6. Set MAC address",
145161
"func", "SetupVF",
146162
"s.nLink", s.nLink,
147163
"podifName", podifName,
@@ -152,37 +168,77 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns
152168
}
153169
}
154170

155-
logging.Debug("8. Enable Optimistic DAD for IPv6 addresses", "func", "SetupVF",
156-
"linkObj", linkObj)
171+
logging.Debug("7. Enable Optimistic DAD for IPv6 addresses", "func", "SetupVF",
172+
"linkObj", netNSLinkObj)
157173
_ = s.utils.EnableOptimisticDad(podifName)
158174

159-
// 9. Bring IF up in Pod netns
160-
logging.Debug("9. Bring IF up in Pod netns",
175+
// 8. Bring IF up in Pod netns
176+
logging.Debug("8. Bring IF up in Pod netns",
161177
"func", "SetupVF",
162-
"linkObj", linkObj)
163-
if err := s.nLink.LinkSetUp(linkObj); err != nil {
178+
"linkObj", netNSLinkObj)
179+
if err = s.nLink.LinkSetUp(netNSLinkObj); err != nil {
164180
return fmt.Errorf("error bringing interface up in container ns: %q", err)
165181
}
166182

167183
return nil
168-
}); err != nil {
169-
return fmt.Errorf("error setting up interface in container namespace: %q", err)
184+
})
185+
if err != nil {
186+
// Cleanup: try to move interface back to tempNS
187+
logging.Debug("Move the interface back to tempNS because of ", "error", err)
188+
_ = netns.Do(func(_ ns.NetNS) error {
189+
if netNSLinkObj, e := s.nLink.LinkByName(podifName); e == nil {
190+
linkSetNsFdError := s.nLink.LinkSetNsFd(netNSLinkObj, int(tempNS.Fd()))
191+
if linkSetNsFdError != nil {
192+
logging.Debug("LinkSetNsFd failed when trying to move back to tempNS", "error", linkSetNsFdError)
193+
}
194+
}
195+
return nil
196+
})
197+
// Restore the original link name in case of error in renaming
198+
s.cleanupLinkInTempNS(tempNS, podifName, linkName, initns)
199+
return err
170200
}
171201

172202
// Copy the MTU value to a new variable
173203
// and use it as a pointer
174204
vfMTU := linkObj.Attrs().MTU
175205
conf.MTU = &vfMTU
176-
177206
return nil
178207
}
179208

209+
// cleanupLinkInTempNS restores the original link name and moves it back to initns
210+
func (s *sriovManager) cleanupLinkInTempNS(tempNS ns.NetNS, podifName, linkName string, initns ns.NetNS) {
211+
_ = tempNS.Do(func(_ ns.NetNS) error {
212+
// Restore the original link name in case of error in renaming
213+
if tempNSLinkObj, err := s.nLink.LinkByName(podifName); err == nil {
214+
linkSetNameError := s.nLink.LinkSetName(tempNSLinkObj, linkName)
215+
if linkSetNameError != nil {
216+
logging.Debug("LinkSetName failed when trying to restore original name", "error", linkSetNameError)
217+
}
218+
}
219+
220+
// Try to move interface back to initns
221+
if tempNSLinkObj, e := s.nLink.LinkByName(linkName); e == nil {
222+
linkSetNsFdError := s.nLink.LinkSetNsFd(tempNSLinkObj, int(initns.Fd()))
223+
if linkSetNsFdError != nil {
224+
logging.Debug("LinkSetNsFd failed when trying to move back to initns in case of an error", "error", linkSetNsFdError)
225+
}
226+
}
227+
return nil
228+
})
229+
}
230+
180231
// ReleaseVF reset a VF from Pod netns and return it to init netns
181232
func (s *sriovManager) ReleaseVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error {
182233
initns, err := ns.GetCurrentNS()
183234
if err != nil {
184235
return fmt.Errorf("failed to get init netns: %v", err)
185236
}
237+
tempNS, err := ns.TempNetNS()
238+
if err != nil {
239+
return fmt.Errorf("failed to create tempNS: %v", err)
240+
}
241+
defer tempNS.Close()
186242

187243
return netns.Do(func(_ ns.NetNS) error {
188244
// get VF device
@@ -202,51 +258,82 @@ func (s *sriovManager) ReleaseVF(conf *sriovtypes.NetConf, podifName string, net
202258
return fmt.Errorf("failed to set link %s down: %q", podifName, err)
203259
}
204260

205-
// rename VF device
206-
logging.Debug("Rename VF device",
261+
logging.Debug("Move VF device to temp netns",
207262
"func", "ReleaseVF",
208263
"linkObj", linkObj,
209-
"conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName)
210-
err = s.nLink.LinkSetName(linkObj, conf.OrigVfState.HostIFName)
211-
if err != nil {
212-
return fmt.Errorf("failed to rename link %s to host name %s: %q", podifName, conf.OrigVfState.HostIFName, err)
264+
"tempNS.Fd()", int(tempNS.Fd()))
265+
if err = s.nLink.LinkSetNsFd(linkObj, int(tempNS.Fd())); err != nil {
266+
return fmt.Errorf("failed to move interface %s to temp netns: %v", podifName, err)
213267
}
214268

215-
if conf.MAC != "" {
216-
// reset effective MAC address
217-
logging.Debug("Reset effective MAC address",
218-
"func", "ReleaseVF",
219-
"s.nLink", s.nLink,
220-
"conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName,
221-
"conf.OrigVfState.EffectiveMAC", conf.OrigVfState.EffectiveMAC)
222-
err = utils.SetVFEffectiveMAC(s.nLink, conf.OrigVfState.HostIFName, conf.OrigVfState.EffectiveMAC)
269+
err = tempNS.Do(func(hostNS ns.NetNS) error {
270+
tempNSLinkObj, err := s.nLink.LinkByName(podifName)
223271
if err != nil {
224-
return fmt.Errorf("failed to restore original effective netlink MAC address %s: %v", conf.OrigVfState.EffectiveMAC, err)
272+
return fmt.Errorf("failed to find %q in tempNS: %v", podifName, err)
225273
}
226-
}
227274

228-
// reset MTU for VF device until if the MTU was captured in the cache
229-
if conf.OrigVfState.MTU != 0 {
230-
logging.Debug("Reset VF device MTU",
231-
"func", "ReleaseVF",
232-
"linkObj", linkObj,
233-
"conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName,
234-
"conf.OrigVfState.MTU", conf.OrigVfState.MTU)
235-
err = s.nLink.LinkSetMTU(linkObj, conf.OrigVfState.MTU)
236-
if err != nil {
237-
return fmt.Errorf("failed to reset MTU for link link %s: %q", conf.OrigVfState.HostIFName, err)
275+
// Move the interface back to netns on error
276+
defer func() {
277+
if err != nil {
278+
logging.Debug("Moving the interface back to netns because of ", "error", err)
279+
linkSetNsFdError := s.nLink.LinkSetNsFd(tempNSLinkObj, int(netns.Fd()))
280+
if linkSetNsFdError != nil {
281+
logging.Debug("LinkSetNsFd failed in defer", "error", linkSetNsFdError)
282+
}
283+
}
284+
}()
285+
286+
// Rename interface to linkName
287+
if err = s.nLink.LinkSetName(tempNSLinkObj, conf.OrigVfState.HostIFName); err != nil {
288+
return fmt.Errorf("failed to rename device %q to %q: %v", podifName, conf.OrigVfState.HostIFName, err)
238289
}
239-
}
240290

241-
// move VF device to init netns
242-
logging.Debug("Move VF device to init netns",
243-
"func", "ReleaseVF",
244-
"linkObj", linkObj,
245-
"initns.Fd()", int(initns.Fd()))
246-
if err = s.nLink.LinkSetNsFd(linkObj, int(initns.Fd())); err != nil {
247-
return fmt.Errorf("failed to move interface %s to init netns: %v", conf.OrigVfState.HostIFName, err)
248-
}
291+
// Rename the interface back to podIfName on error
292+
defer func() {
293+
if err != nil {
294+
logging.Debug("Renaming the interface back to podIfName because of ", "error", err)
295+
LinkSetNameError := s.nLink.LinkSetName(tempNSLinkObj, podifName)
296+
if LinkSetNameError != nil {
297+
logging.Debug("LinkSetName failed in defer", "error", LinkSetNameError)
298+
}
299+
}
300+
}()
301+
302+
if conf.MAC != "" {
303+
// reset effective MAC address
304+
logging.Debug("Reset effective MAC address",
305+
"func", "ReleaseVF",
306+
"s.nLink", s.nLink,
307+
"conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName,
308+
"conf.OrigVfState.EffectiveMAC", conf.OrigVfState.EffectiveMAC)
309+
err = utils.SetVFEffectiveMAC(s.nLink, conf.OrigVfState.HostIFName, conf.OrigVfState.EffectiveMAC)
310+
if err != nil {
311+
return fmt.Errorf("failed to restore original effective netlink MAC address %s: %v", conf.OrigVfState.EffectiveMAC, err)
312+
}
313+
}
314+
315+
// reset MTU for VF device until if the MTU was captured in the cache
316+
if conf.OrigVfState.MTU != 0 {
317+
logging.Debug("Reset VF device MTU",
318+
"func", "ReleaseVF",
319+
"linkObj", linkObj,
320+
"conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName,
321+
"conf.OrigVfState.MTU", conf.OrigVfState.MTU)
322+
err = s.nLink.LinkSetMTU(tempNSLinkObj, conf.OrigVfState.MTU)
323+
if err != nil {
324+
return fmt.Errorf("failed to reset MTU for link link %s: %q", conf.OrigVfState.HostIFName, err)
325+
}
326+
}
249327

328+
// Finally move the interface to the initns
329+
if err = s.nLink.LinkSetNsFd(tempNSLinkObj, int(initns.Fd())); err != nil {
330+
return fmt.Errorf("failed to move %q to initns: %v", conf.OrigVfState.HostIFName, err)
331+
}
332+
return nil
333+
})
334+
if err != nil {
335+
return fmt.Errorf("error setting up interface in temp namespace: %q", err)
336+
}
250337
return nil
251338
})
252339
}
@@ -273,7 +360,6 @@ func (s *sriovManager) ApplyVFConfig(conf *sriovtypes.NetConf) error {
273360
return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %s: %v", conf.VFID, *conf.Vlan, *conf.VlanQoS, *conf.VlanProto, err)
274361
}
275362
}
276-
277363
// 2. Set mac address
278364
if conf.MAC != "" {
279365
// when we restore the original hardware mac address we may get a device or resource busy. so we introduce retry

0 commit comments

Comments
 (0)