Skip to content

Commit a0a0bba

Browse files
robmryrobmry
committed
Discard ErrDumpInterrupted and return data after maxRetries
Returning possibly inconsistent data avoids retrying indefinitely, and matches the behaviour of vishvananda/netlink prior to version 1.2.1, in which the NLM_F_DUMP_INTR flag was ignored. Signed-off-by: Rob Murray <[email protected]>
1 parent 2f02f45 commit a0a0bba

1 file changed

Lines changed: 35 additions & 21 deletions

File tree

‎internal/nlwrap/nlwrap_linux.go‎

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,26 @@
55
// wrapped. Functions that use the package handle need to be called as "nlwrap.X"
66
// instead of "netlink.X".
77
//
8-
// The wrapped functions currently return EINTR when NLM_F_DUMP_INTR flagged
9-
// in a netlink response, meaning something changed during the dump so results
10-
// may be incomplete or inconsistent.
8+
// When netlink.ErrDumpInterrupted is returned, the wrapped functions retry up to
9+
// maxAttempts times. This error means NLM_F_DUMP_INTR was flagged in a netlink
10+
// response, meaning something changed during the dump so results may be
11+
// incomplete or inconsistent.
1112
//
12-
// At present, the possibly incomplete/inconsistent results are not returned
13-
// by netlink functions along with the EINTR. So, it's not possible to do
14-
// anything but retry. After maxAttempts the EINTR will be returned to the
15-
// caller.
13+
// To avoid retrying indefinitely, if netlink.ErrDumpInterrupted is still
14+
// returned after maxAttempts, the wrapped functions will discard the error, log
15+
// a stack trace to make the issue visible and aid in debugging, and return the
16+
// possibly inconsistent results. Returning possibly inconsistent results matches
17+
// the behaviour of vishvananda/netlink versions prior to 1.2.1, in which the
18+
// NLM_F_DUMP_INTR flag was ignored.
1619
package nlwrap
1720

1821
import (
1922
"context"
20-
"errors"
2123

2224
"github.com/containerd/log"
25+
"github.com/pkg/errors"
2326
"github.com/vishvananda/netlink"
2427
"github.com/vishvananda/netns"
25-
"golang.org/x/sys/unix"
2628
)
2729

2830
// Arbitrary limit on max attempts at netlink calls if they are repeatedly interrupted.
@@ -56,20 +58,32 @@ func (h Handle) Close() {
5658

5759
func retryOnIntr(f func() error) {
5860
for attempt := 0; attempt < maxAttempts; attempt += 1 {
59-
if err := f(); !errors.Is(err, unix.EINTR) {
61+
if err := f(); !errors.Is(err, netlink.ErrDumpInterrupted) {
6062
return
6163
}
6264
}
6365
log.G(context.TODO()).Infof("netlink call interrupted after %d attempts", maxAttempts)
6466
}
6567

68+
func discardErrDumpInterrupted(err error) error {
69+
if errors.Is(err, netlink.ErrDumpInterrupted) {
70+
// The netlink function has returned possibly-inconsistent data along with the
71+
// error. Discard the error and return the data. This restores the behaviour of
72+
// the netlink package prior to v1.2.1, in which NLM_F_DUMP_INTR was ignored in
73+
// the netlink response.
74+
log.G(context.TODO()).Warnf("discarding ErrDumpInterrupted: %+v", errors.WithStack(err))
75+
return nil
76+
}
77+
return err
78+
}
79+
6680
// AddrList calls nlh.Handle.AddrList, retrying if necessary.
6781
func (nlh Handle) AddrList(link netlink.Link, family int) (addrs []netlink.Addr, err error) {
6882
retryOnIntr(func() error {
6983
addrs, err = nlh.Handle.AddrList(link, family) //nolint:forbidigo
7084
return err
7185
})
72-
return addrs, err
86+
return addrs, discardErrDumpInterrupted(err)
7387
}
7488

7589
// AddrList calls netlink.AddrList, retrying if necessary.
@@ -78,7 +92,7 @@ func AddrList(link netlink.Link, family int) (addrs []netlink.Addr, err error) {
7892
addrs, err = netlink.AddrList(link, family) //nolint:forbidigo
7993
return err
8094
})
81-
return addrs, err
95+
return addrs, discardErrDumpInterrupted(err)
8296
}
8397

8498
// ConntrackDeleteFilters calls nlh.Handle.ConntrackDeleteFilters, retrying if necessary.
@@ -91,7 +105,7 @@ func (nlh Handle) ConntrackDeleteFilters(
91105
matched, err = nlh.Handle.ConntrackDeleteFilters(table, family, filters...) //nolint:forbidigo
92106
return err
93107
})
94-
return matched, err
108+
return matched, discardErrDumpInterrupted(err)
95109
}
96110

97111
// ConntrackTableList calls netlink.ConntrackTableList, retrying if necessary.
@@ -103,7 +117,7 @@ func ConntrackTableList(
103117
flows, err = netlink.ConntrackTableList(table, family) //nolint:forbidigo
104118
return err
105119
})
106-
return flows, err
120+
return flows, discardErrDumpInterrupted(err)
107121
}
108122

109123
// LinkByName calls nlh.Handle.LinkByName, retrying if necessary. The netlink function
@@ -114,7 +128,7 @@ func (nlh Handle) LinkByName(name string) (link netlink.Link, err error) {
114128
link, err = nlh.Handle.LinkByName(name) //nolint:forbidigo
115129
return err
116130
})
117-
return link, err
131+
return link, discardErrDumpInterrupted(err)
118132
}
119133

120134
// LinkByName calls netlink.LinkByName, retrying if necessary. The netlink
@@ -125,7 +139,7 @@ func LinkByName(name string) (link netlink.Link, err error) {
125139
link, err = netlink.LinkByName(name) //nolint:forbidigo
126140
return err
127141
})
128-
return link, err
142+
return link, discardErrDumpInterrupted(err)
129143
}
130144

131145
// LinkList calls nlh.Handle.LinkList, retrying if necessary.
@@ -134,7 +148,7 @@ func (nlh Handle) LinkList() (links []netlink.Link, err error) {
134148
links, err = nlh.Handle.LinkList() //nolint:forbidigo
135149
return err
136150
})
137-
return links, err
151+
return links, discardErrDumpInterrupted(err)
138152
}
139153

140154
// LinkList calls netlink.Handle.LinkList, retrying if necessary.
@@ -143,7 +157,7 @@ func LinkList() (links []netlink.Link, err error) {
143157
links, err = netlink.LinkList() //nolint:forbidigo
144158
return err
145159
})
146-
return links, err
160+
return links, discardErrDumpInterrupted(err)
147161
}
148162

149163
// RouteList calls nlh.Handle.RouteList, retrying if necessary.
@@ -152,7 +166,7 @@ func (nlh Handle) RouteList(link netlink.Link, family int) (routes []netlink.Rou
152166
routes, err = nlh.Handle.RouteList(link, family) //nolint:forbidigo
153167
return err
154168
})
155-
return routes, err
169+
return routes, discardErrDumpInterrupted(err)
156170
}
157171

158172
// XfrmPolicyList calls nlh.Handle.XfrmPolicyList, retrying if necessary.
@@ -161,7 +175,7 @@ func (nlh Handle) XfrmPolicyList(family int) (policies []netlink.XfrmPolicy, err
161175
policies, err = nlh.Handle.XfrmPolicyList(family) //nolint:forbidigo
162176
return err
163177
})
164-
return policies, err
178+
return policies, discardErrDumpInterrupted(err)
165179
}
166180

167181
// XfrmStateList calls nlh.Handle.XfrmStateList, retrying if necessary.
@@ -170,5 +184,5 @@ func (nlh Handle) XfrmStateList(family int) (states []netlink.XfrmState, err err
170184
states, err = nlh.Handle.XfrmStateList(family) //nolint:forbidigo
171185
return err
172186
})
173-
return states, err
187+
return states, discardErrDumpInterrupted(err)
174188
}

0 commit comments

Comments
 (0)