Skip to content
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.

Sign up for GitHub

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

Add in support for network interface flags. #2037

Merged
merged 4 commits into from
Sep 6, 2022
Merged

Add in support for network interface flags. #2037

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a5842b
Add in support for network interface flags.
clalancette Dec 20, 2021
e05f926
Fixes from review.
clalancette Aug 18, 2022
d83c959
Fixes from review.
clalancette Sep 6, 2022
3082526
More fixes from review.
clalancette Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CREDITS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,3 +789,7 @@ I: 2099

N: Torsten Blum
I: 2114

N: Chris Lalancette
W: https://github.com/clalancette
I: 2037
1 change: 1 addition & 0 deletions HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

- 1053_: drop Python 2.6 support. (patches by Matthieu Darbois and Hugo van
Kemenade)
- 2037_: Add additional flags to net_if_stats.
- 2050_, [Linux]: increase ``read(2)`` buffer size from 1k to 32k when reading
``/proc`` pseudo files line by line. This should help having more consistent
results.
Expand Down
4 changes: 2 additions & 2 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ Network
snicaddr(family=<AddressFamily.AF_LINK: 17>, address='c4:85:08:45:06:41', netmask=None, broadcast='ff:ff:ff:ff:ff:ff', ptp=None)]}
>>>
>>> psutil.net_if_stats()
{'lo': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_UNKNOWN: 0>, speed=0, mtu=65536),
'wlan0': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_FULL: 2>, speed=100, mtu=1500)}
{'lo': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_UNKNOWN: 0>, speed=0, mtu=65536, flags='up,loopback,running'),
'wlan0': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_FULL: 2>, speed=100, mtu=1500, flags='up,broadcast,running,multicast')}
>>>

Sensors
Expand Down
7 changes: 5 additions & 2 deletions docs/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,20 +735,23 @@ Network
- **speed**: the NIC speed expressed in mega bits (MB), if it can't be
determined (e.g. 'localhost') it will be set to ``0``.
- **mtu**: NIC's maximum transmission unit expressed in bytes.
- **flags**: a string of comma-separate flags on the interface (may be ``None``).
Copy link
Owner

Choose a reason for hiding this comment

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

  1. I think an empty string is better than None. This way anybody can do if "running" in flags without pre-emptively checking if flag is not None.
  2. please also list all the possible strings values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to an empty string everywhere below. Also listed all of the possible string values in e05f926.


Example:

>>> import psutil
>>> psutil.net_if_stats()
{'eth0': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_FULL: 2>, speed=100, mtu=1500),
'lo': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_UNKNOWN: 0>, speed=0, mtu=65536)}
{'eth0': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_FULL: 2>, speed=100, mtu=1500, flags='up,broadcast,running,multicast'),
'lo': snicstats(isup=True, duplex=<NicDuplex.NIC_DUPLEX_UNKNOWN: 0>, speed=0, mtu=65536, flags='up,loopback,running')}

Also see `nettop.py`_ and `ifconfig.py`_ for an example application.

.. versionadded:: 3.0.0

.. versionchanged:: 5.7.3 `isup` on UNIX also checks whether the NIC is running.

.. versionchanged:: 5.9.1 *flags* field was added.
Copy link
Owner

Choose a reason for hiding this comment

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

- .. versionchanged:: 5.9.1 *flags* field was added.
+ .. versionchanged:: 5.9.1 *flags* field was added on POSIX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e05f926.


Sensors
-------

Expand Down
2 changes: 1 addition & 1 deletion psutil/_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class BatteryTime(enum.IntEnum):
snicaddr = namedtuple('snicaddr',
['family', 'address', 'netmask', 'broadcast', 'ptp'])
# psutil.net_if_stats()
snicstats = namedtuple('snicstats', ['isup', 'duplex', 'speed', 'mtu'])
snicstats = namedtuple('snicstats', ['isup', 'duplex', 'speed', 'mtu', 'flags'])
# psutil.cpu_stats()
scpustats = namedtuple(
'scpustats', ['ctx_switches', 'interrupts', 'soft_interrupts', 'syscalls'])
Expand Down
2 changes: 1 addition & 1 deletion psutil/_psaix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ def net_if_stats():
duplex = re_result.group(2)

duplex = duplex_map.get(duplex, NIC_DUPLEX_UNKNOWN)
ret[name] = _common.snicstats(isup, duplex, speed, mtu)
ret[name] = _common.snicstats(isup, duplex, speed, mtu, None)
Copy link
Owner

Choose a reason for hiding this comment

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

Since psutil_net_if_is_running function was already working on AIX / SunOS, I guess you can enable this code on those platforms as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've enabled it for AIX, as it is basically the same code. That said, I don't have access to an AIX machine, so I can't verify it there.

See my comment below about SunOS.

return ret


Expand Down
11 changes: 9 additions & 2 deletions psutil/_psbsd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ def net_if_stats():
for name in names:
try:
mtu = cext_posix.net_if_mtu(name)
isup = cext_posix.net_if_is_running(name)
flags = cext_posix.net_if_flags(name)
duplex, speed = cext_posix.net_if_duplex_speed(name)
except OSError as err:
# https://github.com/giampaolo/psutil/issues/1279
Expand All @@ -390,7 +390,14 @@ def net_if_stats():
else:
if hasattr(_common, 'NicDuplex'):
duplex = _common.NicDuplex(duplex)
ret[name] = _common.snicstats(isup, duplex, speed, mtu)
flag_list = []
for flagname, bit in _psposix.POSIX_NET_FLAGS:
if flags & (1 << bit):
flag_list.append(flagname)
Copy link
Owner

Choose a reason for hiding this comment

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

dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, good call. Removed in e05f926.


output_flags = ','.join(flag_list)
Copy link
Owner

Choose a reason for hiding this comment

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

please rename this variable to just flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I kept this as output_flags currently, as we still need to use flags for the list of flags. I'm happy to change the name of both if you want, just let me know what you would prefer.

isup = 'running' in output_flags
ret[name] = _common.snicstats(isup, duplex, speed, mtu, output_flags)
return ret


Expand Down
6 changes: 4 additions & 2 deletions psutil/_pslinux.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ def net_if_stats():
for name in names:
try:
mtu = cext_posix.net_if_mtu(name)
isup = cext_posix.net_if_is_running(name)
flags = cext_posix.net_if_flags(name)
duplex, speed = cext.net_if_duplex_speed(name)
except OSError as err:
# https://github.com/giampaolo/psutil/issues/1279
Expand All @@ -1069,7 +1069,9 @@ def net_if_stats():
else:
debug(err)
else:
ret[name] = _common.snicstats(isup, duplex_map[duplex], speed, mtu)
output_flags = ','.join(flags)
Copy link
Owner

Choose a reason for hiding this comment

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

please rename this variable to just flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment here.

isup = 'running' in output_flags
ret[name] = _common.snicstats(isup, duplex_map[duplex], speed, mtu, output_flags)
return ret


Expand Down
6 changes: 4 additions & 2 deletions psutil/_psosx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def net_if_stats():
for name in names:
try:
mtu = cext_posix.net_if_mtu(name)
isup = cext_posix.net_if_is_running(name)
flags = cext_posix.net_if_flags(name)
duplex, speed = cext_posix.net_if_duplex_speed(name)
except OSError as err:
# https://github.com/giampaolo/psutil/issues/1279
Expand All @@ -272,7 +272,9 @@ def net_if_stats():
else:
if hasattr(_common, 'NicDuplex'):
duplex = _common.NicDuplex(duplex)
ret[name] = _common.snicstats(isup, duplex, speed, mtu)
output_flags = ','.join(flags)
Copy link
Owner

Choose a reason for hiding this comment

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

please rename this variable to just flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment here.

isup = 'running' in output_flags
ret[name] = _common.snicstats(isup, duplex, speed, mtu, output_flags)
return ret


Expand Down
2 changes: 1 addition & 1 deletion psutil/_pssunos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def net_if_stats():
isup, duplex, speed, mtu = items
if hasattr(_common, 'NicDuplex'):
duplex = _common.NicDuplex(duplex)
ret[name] = _common.snicstats(isup, duplex, speed, mtu)
ret[name] = _common.snicstats(isup, duplex, speed, mtu, None)
Copy link
Owner

Choose a reason for hiding this comment

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

let's enable this on sunos as well (see my previous comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So SunOS is a completely different beast than Linux, BSD, macOS, or Windows. Looking at the code in

psutil/psutil/_psutil_sunos.c

Line 1484 in 4446e3b

psutil_net_if_stats(PyObject* self, PyObject* args) {
, it looks like it doesn't even use the same ioctl (it is using something called SIOCGLIFFLAGS), and then it verifies that data with another structure (kstat?). So I don't think I can easily enable SunOS, and I don't have access to a SunOS machine.

return ret


Expand Down
101 changes: 101 additions & 0 deletions psutil/_psutil_posix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,106 @@ psutil_net_if_mtu(PyObject *self, PyObject *args) {
}


/*
* Get all of the NIC flags and return them.
*/
static PyObject *
psutil_net_if_flags(PyObject *self, PyObject *args) {
char *nic_name;
int sock = -1;
int ret;
struct ifreq ifr;
PyObject *py_retlist = PyList_New(0);
PyObject *py_flag = NULL;
Copy link
Owner

Choose a reason for hiding this comment

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

For consistency with the rest of the code (in other C modules) please call this "py_str".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e05f926.

short int flags;

if (py_retlist == NULL)
return NULL;

if (! PyArg_ParseTuple(args, "s", &nic_name))
return NULL;
Copy link
Owner

Choose a reason for hiding this comment

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

This should be goto error;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e05f926.


sock = socket(AF_INET, SOCK_DGRAM, 0);
if (sock == -1)
return NULL;
Copy link
Owner

Choose a reason for hiding this comment

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

You should add PyErr_SetFromErrno(PyExc_OSError);, then goto error;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e05f926.


PSUTIL_STRNCPY(ifr.ifr_name, nic_name, sizeof(ifr.ifr_name));
ret = ioctl(sock, SIOCGIFFLAGS, &ifr);
if (ret == -1)
goto error;
Copy link
Owner

Choose a reason for hiding this comment

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

You should add PyErr_SetFromErrno(PyExc_OSError);, then goto error;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e05f926.


close(sock);
sock = -1;

flags = ifr.ifr_flags & 0xFFFF;

if (flags & IFF_UP) {
Copy link
Owner

Choose a reason for hiding this comment

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

To be extra sure I think it's a good idea to check whether all these constants exist. I would do:

#ifdef IFF_UP
    // your code
#endif
#ifdef IFF_BROADCAST
    // your code
#endif
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a fantastic idea. It also allows us to specify additional flags that only occur on certain platforms. Changed in e05f926.

py_flag = PyUnicode_DecodeFSDefault("up");
Copy link
Owner

Choose a reason for hiding this comment

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

You should also check the return value for all these calls. This should be:

py_flag = PyUnicode_DecodeFSDefault("up");
if (! py_flag)
    goto error;

It may make sense to refactor all of these repetitions by introducing a utility function, but we can do that as a second refactoring step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good call. I added a helper function for this in e05f926, which I think cleans up the code nicely.

if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_BROADCAST) {
py_flag = PyUnicode_DecodeFSDefault("broadcast");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_DEBUG) {
py_flag = PyUnicode_DecodeFSDefault("debug");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_LOOPBACK) {
py_flag = PyUnicode_DecodeFSDefault("loopback");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_POINTOPOINT) {
py_flag = PyUnicode_DecodeFSDefault("pointopoint");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_NOTRAILERS) {
py_flag = PyUnicode_DecodeFSDefault("notrailers");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_RUNNING) {
py_flag = PyUnicode_DecodeFSDefault("running");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_NOARP) {
py_flag = PyUnicode_DecodeFSDefault("noarp");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_PROMISC) {
py_flag = PyUnicode_DecodeFSDefault("promisc");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_ALLMULTI) {
py_flag = PyUnicode_DecodeFSDefault("allmulti");
if (PyList_Append(py_retlist, py_flag))
goto error;
}
if (flags & IFF_MULTICAST) {
py_flag = PyUnicode_DecodeFSDefault("multicast");
if (PyList_Append(py_retlist, py_flag))
goto error;
}

return py_retlist;

error:
Py_XDECREF(py_flag);
Py_DECREF(py_retlist);
if (sock != -1)
close(sock);
return NULL;
}


/*
* Inspect NIC flags, returns a bool indicating whether the NIC is
* running. References:
Expand Down Expand Up @@ -667,6 +767,7 @@ static PyMethodDef mod_methods[] = {
{"getpagesize", psutil_getpagesize_pywrapper, METH_VARARGS},
{"getpriority", psutil_posix_getpriority, METH_VARARGS},
{"net_if_addrs", psutil_net_if_addrs, METH_VARARGS},
{"net_if_flags", psutil_net_if_flags, METH_VARARGS},
{"net_if_is_running", psutil_net_if_is_running, METH_VARARGS},
{"net_if_mtu", psutil_net_if_mtu, METH_VARARGS},
{"setpriority", psutil_posix_setpriority, METH_VARARGS},
Expand Down
2 changes: 1 addition & 1 deletion psutil/_pswindows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ def net_if_stats():
isup, duplex, speed, mtu = items
if hasattr(_common, 'NicDuplex'):
duplex = _common.NicDuplex(duplex)
ret[name] = _common.snicstats(isup, duplex, speed, mtu)
ret[name] = _common.snicstats(isup, duplex, speed, mtu, None)
Copy link
Owner

Choose a reason for hiding this comment

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

let's return "" instead of None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e05f926.

return ret


Expand Down
2 changes: 1 addition & 1 deletion psutil/tests/test_system.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ def test_net_if_stats(self):
psutil.NIC_DUPLEX_UNKNOWN)
for name, stats in nics.items():
self.assertIsInstance(name, str)
isup, duplex, speed, mtu = stats
isup, duplex, speed, mtu, flags = stats
Copy link
Owner

Choose a reason for hiding this comment

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

please add:

self.assertIsInstance(flags, str)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d83c959.

self.assertIsInstance(isup, bool)
self.assertIn(duplex, all_duplexes)
self.assertIn(duplex, all_duplexes)
Expand Down