Fix checking for netlink errors when receiving only a single message #50
Conversation
|
Thanks! Please rebase on master as Travis tests were fixed (there was a netfilter policy change on Travis, which broke our tests). |
|
Done. |
| os.strerror(errno), errno)) | ||
| err.errno = errno | ||
| raise err | ||
| # A non-error message |
| os.strerror(errno), errno)) | ||
| err.errno = errno | ||
| raise err | ||
| # an ack |
There was a problem hiding this comment.
This is only possible in case the errno contained in the message is zero. Currently, the handling in this case is inconsistent when multiple is True (error message is skipped and not included in the final list) and when multiple is False (error message is appended to the list of received messages). I think this should be unified.
There was a problem hiding this comment.
When multiple is False, the entire point of calling this message is to receive the ACK. If we skip it, all uses of multiple=False will break.
Maybe it makes more sense to have a dedicated function recv_ack, and rename this one into recv_until_done?
There was a problem hiding this comment.
I see, so an ACK is actually a message of type NLMSG_ERROR, but with an errno payload of zero. The problem with two methods is that they would most likely be very similar, so perhaps it would be better to just document this behavior in the method's documentation block.
This whole netlink code is really ugly as it was imported from somewhere else and we should eventually refactor it.
There was a problem hiding this comment.
an ACK is actually a message of type NLMSG_ERROR, but with an errno payload of zero.
Correct.
better to just document this behavior in the method's documentation block.
Will do.
|
I rebased and added documentation to |
|
Rebased. |
The current implementation fails to raise an exception when an error is returned.
This masks a problem where on newer kernels, tunneldigger fails to create tunnels because l2tpv3 session IDs have to be globally unique.