*: add file-based lock for old iptables versions #12
Conversation
|
|
||
| // Unlock closes the lock which implicitly unlocks it as well | ||
| func (l *fileLock) Unlock() error { | ||
| l.mu.Lock() |
There was a problem hiding this comment.
oops, this isn't right
iptables <1.4.20 does not support blocking (the `--wait` option) to prevent concurrent invocations from interrupting each other. To work around this, this patch adds a file-based lock (flock), using the same file that xtables now uses [1] (`/run/xtables.lock`). Note that this follows a similar behaviour to xtables in that it's best-effort only: if the lock can't be acquired for some reason, operations proceed anyway. h/t to @janeczku and @adfernandes for original fix suggestions [1]: http://git.netfilter.org/iptables/commit/?id=aa562a660d1555b13cffbac1e744033e91f82707
|
👍 Merci beaucoup monsieur! |
|
|
||
| type fileLock struct { | ||
| // mu is used to protect against concurrent invocations from within this process | ||
| mu sync.Mutex |
There was a problem hiding this comment.
It's not clear to me what exactly this is serializing access to. The only thing here is the fd, and the fd doesn't get written to at any point after instantiating the lock, so we shouldn't need to serialize access to it should we?
There was a problem hiding this comment.
As discussed OOB: the idea is that the file lock is used for process concurrency and the mutex lock is for thread (goroutine) concurrency - since the fileLock is file-descriptor based and hence repeated locks would just succeed anyway.
|
I don't like the copy-and-paste "--wait" args append stuff, but the defer in there interferes with using a function, golang... sigh. lgtm |
*: add file-based lock for old iptables versions
iptables <1.4.20 does not support blocking (the
--waitoption) toprevent concurrent invocations from interrupting each other. To work
around this, this patch adds a file-based lock (flock), using the same
file that xtables now uses 1.
Note that this follows a similar behaviour to xtables in that it's
best-effort only: if the lock can't be acquired for some reason,
operations proceed anyway.
h/t to @janeczku and @adfernandes for original fix suggestions