-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
session.pre-down is run after the interface is removed #56
Comments
Or maybe this is a race condition? If I read that code in |
This is true and in this case we would need to wait for the hook to finish executing before proceeding with tunnel teardown. |
Right. I think, actually, that for our purpose a post-down hook is good enough, but I would have to check. I mean, deleting the interface will remove it from the bridge, so we don't have to do that in the hook. |
Yes, for us such behavior was ok as well and this is the reason why I didn't implement "blocking" hooks. Also, a hook could in theory loop forever and block the tunnel from being freed. |
Well, but then it shouldn't be called pre-down hook. Removing that hook would be the more honest thing to do. |
Well, sure. An admin can always misconfigure their server. Actually, we have hooks that rely on this blocking behavior. They modify iptables and could mess things up when running concurrently. (Yes, it's a bad hack, but it's needed to work around a bad firewall.) So this [undocumented] change from synchronous to asynchronous hooks could cause trouble for us. And even for the non-hacky part... if the tunnel is only added to the bridge asynchronously, doesn't that mean that the client could already be sending data into the tunnel before it is even plugged into the bridge? |
We have the following line in our pre-down hook:
With the old version of the broker (before the rewrite, see e.g. https://github.com/ffrl/tunneldigger/) on old kernels (3.16.0), that worked just fine. However, on kernel 4.9.30 and with the latest broker, we now have errors in the log:
I am not seeing these errors with the new broker on an old kernel. So it seems the kernel update is the reason here, not the broker update.
That is strange, isn't it? How would the kernel even know already that the tunnel is dead? Does L2TP have in-band signaling for tearing down the tunnel?
The text was updated successfully, but these errors were encountered: