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
[Linux] Connections in separate network namespaces are invisible #1691
base: master
Are you sure you want to change the base?
Conversation
@giampaolo Is there a problem with AppVeyor builds? The errors there seem to be unrelated to my modifications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started looking into this PR. Here's some considerations.
This patch assumes that /proc/{PID}/ns/net
(aka network namespaces) always exists. I suppose there may be circumstances (e.g. old Linux kernels) where this file may not exist, so I think the code should take that into account and fallback on using the old method (read /proc/net/tcp|udp).
Also, I suggest a refactoring: leave process_inet()
and process_unix()
methods alone. Instead of getting the namespace files in there, do that in retrieve()
method and call process_inet()
/ process_unix()
for each file.
Finally, but I don't think this is a problem for this PR, because we may implement #1581 sooner or later, the Python implementation will be altered. I cannot foresee how exactly that'll happen though, so ignore this part for now.
psutil/_pslinux.py
Outdated
net_namespaces = defaultdict(list) | ||
for pid in pids(): | ||
try: | ||
ns = readlink("%s/%s/ns/net" % (get_procfs_path(), pid)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use self._procfs_path
psutil/_pslinux.py
Outdated
net_namespaces[ns].append(pid) | ||
return net_namespaces | ||
|
||
def process_inet(self, file_name, family, type_, inodes, filter_pid=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a staticmethod
psutil/_pslinux.py
Outdated
@@ -72,7 +72,7 @@ | |||
# --- globals | |||
# ===================================================================== | |||
|
|||
|
|||
# TODO remove this comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
psutil/_pslinux.py
Outdated
|
||
def process_inet(self, file_name, family, type_, inodes, filter_pid=None): | ||
"""Parse /proc/*/net/tcp* and /proc/*/net/udp* files.""" | ||
net_ns = Connections.network_namespaces() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can avoid calling this if filter_pid is None
Added test that opens two processes. One behaves as if it's in the same network namespace as the testing process, the other - as if it's another network namespace. Psutil should detect a connection from the first but not from the second process before the fix. After it should detect from both.
If run with root privileges, will fetch all connections from all network namespaces in the system. If run without root privileges, will fetch connections only from processes accessible by this user, that is will fetch connections from processes for which the path /proc/<pid>/ns is accessible by this user.
Added a test checking the correct handling of missing /proc/PID/ns/net file
144f520
to
ec3760b
Compare
Thanks, I've addressed the review in ec3760b |
This is a continuation of PR #1631 which was accidentally closed.
Please see #1611 for explanation of the problem and related discussion.