Skip to content

[Linux] Connections in separate network namespaces are invisible - #1691

Open
davereikher wants to merge 4 commits into
giampaolo:masterfrom
davereikher:linux_all_ns
Open

[Linux] Connections in separate network namespaces are invisible#1691
davereikher wants to merge 4 commits into
giampaolo:masterfrom
davereikher:linux_all_ns

Conversation

@davereikher

Copy link
Copy Markdown

This is a continuation of PR #1631 which was accidentally closed.
Please see #1611 for explanation of the problem and related discussion.

@davereikher

Copy link
Copy Markdown
Author

@giampaolo Is there a problem with AppVeyor builds? The errors there seem to be unrelated to my modifications.
If so, then this PR is ready for code review.

@giampaolo giampaolo left a comment

Copy link
Copy Markdown
Owner

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.

Comment thread psutil/_pslinux.py Outdated
net_namespaces = defaultdict(list)
for pid in pids():
try:
ns = readlink("%s/%s/ns/net" % (get_procfs_path(), pid))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use self._procfs_path

Comment thread psutil/_pslinux.py Outdated
net_namespaces[ns].append(pid)
return net_namespaces

def process_inet(self, file_name, family, type_, inodes, filter_pid=None):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a staticmethod

Comment thread psutil/_pslinux.py Outdated
# =====================================================================


# TODO remove this comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Comment thread 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()

Copy link
Copy Markdown
Owner

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

giampaolo added a commit that referenced this pull request Feb 13, 2020
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
@davereikher

Copy link
Copy Markdown
Author

Thanks, I've addressed the review in ec3760b

@giampaolo giampaolo added the linux platform : Linux specific label Nov 15, 2020
@giampaolo giampaolo added the bug type label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug type linux platform : Linux specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants