-
-
Notifications
You must be signed in to change notification settings - Fork 522
Ignore virtual interfaces for network IO accounting #1746
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
base: main
Are you sure you want to change the base?
Conversation
return (strncmp(name, "docker", 6) == 0 || strncmp(name, "veth", 4) == 0 || | ||
strncmp(name, "virbr", 5) == 0 || strncmp(name, "tun", 3) == 0 || | ||
strncmp(name, "tap", 3) == 0 || strncmp(name, "vboxnet", 7) == 0); | ||
} |
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 list is quite incomplete, and IMO hard-coding it like this is not ideal. I imagine the BSDs and other platforms may have a similar requirement.
The solution we use in pcp.io is to have an interfaces.conf configuration file (optionally, somewhere below /etc) that provides a regular expression - if interfaces match, they are culled from the calculation. This is platform agnostic - different platforms could have different config files - and indeed different sites may have their own naming conventions, custom drivers, etc.
This is the current regex we're using:
# interfaces.conf
#
# Regular expression describing network interfaces (from the
# network.interfaces.* metrics instance domain) that will be
# *excluded* from the network.all.* metrics calculation when
# aggregating (sum) stats from physical interfaces.
#
# Comments are the hash-to-end-of-line variety and any / all
# whitespace characters are removed before pmdalinux(1) uses
# regcomp(3) to compile the regular expression.
#
^(lo |
bond[0-9]+ |
team[0-9]+ |
tun[0-9]+ |
virbr[0-9]+ |
virbr[0-9]+-nic |
cni[0-9]+ |
cni-podman[0-9]+ |
docker[0-9]+ |
veth[0-9]+ |
face)$
Using a similar approach to this could simplify and improve this PR. Can do away with all the configure.ac/ifdef changes as well.
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.
Also, instead of relying on the name of the interface, using the interface type can help simplify things even more. Otherwise you risk breaking things depending on some settings (e.g. systemd's unusable names vs. the good ones like eth0) …
I did a quick test run to check for certain criteria of the interfaces, basically, what type they are, and if they have a parent interface assigned. A rough scetch from Copilot looks like this, to demonstrate the idea: #define _GNU_SOURCE
#include <netdb.h>
#include <stdio.h>
#include <string.h>
#include <net/if.h>
#include <linux/if_arp.h>
#include <netlink/netlink.h>
#include <netlink/socket.h>
#include <netlink/cache.h>
#include <netlink/route/link.h>
/* set up a NETLINK_ROUTE socket */
static struct nl_sock *setup_nl(void)
{
struct nl_sock *sock = nl_socket_alloc();
if (!sock)
return NULL;
if (nl_connect(sock, NETLINK_ROUTE) < 0) {
nl_socket_free(sock);
return NULL;
}
return sock;
}
/* map ARPHRD_* to a human link/… string */
static const char *link_type_str(int arptype)
{
switch (arptype) {
case ARPHRD_ETHER: return "link/ether";
case ARPHRD_LOOPBACK: return "link/loopback";
case ARPHRD_PPP: return "link/ppp";
default: return "link/other";
}
}
int query_interfaces(struct nl_sock *sock)
{
struct nl_cache *cache;
struct nl_object *obj;
int err;
err = rtnl_link_alloc_cache(sock, AF_UNSPEC, &cache);
if (err < 0)
return err;
printf("%-10s %-13s %-8s %-12s %-13s %-10s\n",
"IFACE", "LINK TYPE", "KIND", "VLAN PARENT",
"BRIDGE MASTER", "TOP-LEVEL");
for (obj = nl_cache_get_first(cache); obj; obj = nl_cache_get_next(obj)) {
struct rtnl_link *link = (struct rtnl_link *)obj;
const char *name = rtnl_link_get_name(link);
int arptype = rtnl_link_get_arptype(link);
const char *kind = rtnl_link_get_type(link);
char kindbuf[16];
/* if no IFLA_INFO_KIND, fall back to generic names */
if (!kind) {
if (arptype == ARPHRD_LOOPBACK)
strncpy(kindbuf, "lo", sizeof(kindbuf));
else if (arptype == ARPHRD_ETHER)
strncpy(kindbuf, "eth", sizeof(kindbuf));
else
strncpy(kindbuf, "other", sizeof(kindbuf));
kindbuf[sizeof(kindbuf)-1] = 0;
kind = kindbuf;
}
/* detect VLAN parent, if any */
int vlan_parent_idx = rtnl_link_get_link(link);
char vlan_parent[IF_NAMESIZE] = "-";
if (vlan_parent_idx > 0) {
if (!if_indextoname(vlan_parent_idx, vlan_parent))
strncpy(vlan_parent, "unknown", sizeof(vlan_parent));
}
/* detect bridge master, if any */
int br_master_idx = rtnl_link_get_master(link);
char br_master[IF_NAMESIZE] = "-";
if (br_master_idx > 0) {
if (!if_indextoname(br_master_idx, br_master))
strncpy(br_master, "unknown", sizeof(br_master));
}
/* top-level if it’s neither a VLAN nor enslaved to a bridge */
int is_sub = (vlan_parent_idx > 0) || (br_master_idx > 0);
const char *top_lvl = is_sub ? "no" : "yes";
printf("%-10s %-13s %-8s %-12s %-13s %-10s\n",
name,
link_type_str(arptype),
kind,
vlan_parent,
br_master,
top_lvl);
}
nl_cache_free(cache);
return 0;
}
int main(void)
{
struct nl_sock *sock = setup_nl();
if (!sock)
return 1;
if (query_interfaces(sock) < 0) {
nl_socket_free(sock);
return 1;
}
nl_socket_free(sock);
return 0;
} On my local machine this gives output like the following:
If counting only the "top-level" interfaces we should get rid of most of the duplicated accounting of traffic forwarded between interfaces. For the other An (untested) version for *BSD/Darwin/Solaris looks like: /*
* lanfilter_portable.c
*
* Portable interface lister with VLAN & bridge discovery
* Supports: FreeBSD, OpenBSD, NetBSD, DragonFlyBSD, macOS, Solaris
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <ifaddrs.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <net/if.h>
#include <net/if_types.h>
#include <net/if_dl.h>
#if defined(__FreeBSD__) || defined(__DragonFly__)
#include <sys/ioctl.h>
#include <net/if_vlan_var.h> /* struct ifvlanreq, SIOCGIFVLAN */
#include <net/if_bridge.h> /* struct ifbaconf, SIOCBRDGRTABLE */
#elif defined(__OpenBSD__) || defined(__NetBSD__)
#include <sys/ioctl.h>
#include <net/if_vlan.h> /* struct vlanreq, SIOCGIFVLAN / SIOCGLIFVLAN */
#include <net/if_bridge.h> /* struct ifbaconf / ifbrlconf */
#elif defined(__APPLE__)
#include <sys/ioctl.h>
#include <net/if_media.h> /* struct ifmediareq, SIOCGIFMEDIA */
#elif defined(__sun)
#include <dlfcn.h>
#include <libdladm.h>
#include <libdllink.h>
#endif
#define MAX_IFACES 128
struct iface {
char name[IFNAMSIZ];
int sdl_type;
char vlan_parent[IFNAMSIZ];
int vlan_tag;
char br_master[IFNAMSIZ];
};
static const char *link_type_str(int t) {
switch (t) {
case IFT_ETHER: return "link/ether";
case IFT_LOOP: return "link/loopback";
case IFT_PPP: return "link/ppp";
case IFT_BRIDGE: return "link/bridge";
#ifdef IFT_IEEE8021Q
case IFT_IEEE8021Q: return "link/vlan";
#endif
default: return "link/other";
}
}
static const char *kind_str(int t) {
switch (t) {
case IFT_ETHER: return "eth";
case IFT_LOOP: return "lo";
case IFT_PPP: return "ppp";
case IFT_BRIDGE: return "br";
#ifdef IFT_IEEE8021Q
case IFT_IEEE8021Q: return "vlan";
#endif
default: return "oth";
}
}
#ifdef __sun
/* Solaris VLAN callback */
static int
solaris_vlan_cb(dladm_handle_t dh, datalink_id_t linkid,
dladm_vlan_attr_t *vattr, void *arg)
{
struct iface *iflist = arg;
char ifname[DLADM_STRSIZE];
if (dladm_datalink_id2info(dh, linkid, NULL, NULL, ifname, 0)
!= DLADM_STATUS_OK) return DLADM_WALK_CONTINUE;
for (int i = 0; i < MAX_IFACES; i++) {
if (strcmp(iflist[i].name, ifname) == 0) {
strlcpy(iflist[i].vlan_parent,
vattr->la_parent_name, IFNAMSIZ);
iflist[i].vlan_tag = vattr->la_vid;
break;
}
}
return DLADM_WALK_CONTINUE;
}
/* Solaris Bridge callback */
static int
solaris_bridge_cb(dladm_handle_t dh, datalink_id_t bridge_id,
dladm_bridge_attr_t *battr, void *arg)
{
struct iface *iflist = arg;
char brname[DLADM_STRSIZE], memname[DLADM_STRSIZE];
if (dladm_datalink_id2info(dh, bridge_id, NULL, NULL,
brname, 0) != DLADM_STATUS_OK)
return DLADM_WALK_CONTINUE;
for (uint_t i = 0; i < battr->ba_nmembers; i++) {
datalink_id_t mid = battr->ba_member[i];
if (dladm_datalink_id2info(dh, mid, NULL, NULL,
memname, 0) != DLADM_STATUS_OK)
continue;
for (int j = 0; j < MAX_IFACES; j++) {
if (strcmp(iflist[j].name, memname) == 0) {
strlcpy(iflist[j].br_master, brname, IFNAMSIZ);
break;
}
}
}
return DLADM_WALK_CONTINUE;
}
#endif /* __sun */
int main(void) {
struct ifaddrs *ifa0, *ifa;
struct iface iflist[MAX_IFACES];
int count = 0;
if (getifaddrs(&ifa0) < 0) {
perror("getifaddrs");
return 1;
}
/* 1) Gather AF_LINK interfaces */
for (ifa = ifa0; ifa; ifa = ifa->ifa_next) {
if (!ifa->ifa_addr ||
ifa->ifa_addr->sa_family != AF_LINK)
continue;
if (count == MAX_IFACES) break;
struct sockaddr_dl *sdl = (struct sockaddr_dl*)ifa->ifa_addr;
strlcpy(iflist[count].name, ifa->ifa_name, IFNAMSIZ);
iflist[count].sdl_type = sdl->sdl_type;
strcpy(iflist[count].vlan_parent, "-");
iflist[count].vlan_tag = -1;
strcpy(iflist[count].br_master, "-");
count++;
}
freeifaddrs(ifa0);
#if defined(__FreeBSD__) || defined(__DragonFly__) \
|| defined(__OpenBSD__) || defined(__NetBSD__) \
|| defined(__APPLE__)
{
int sock = socket(AF_INET, SOCK_DGRAM, 0);
if (sock < 0) {
perror("socket");
return 1;
}
/* VLAN lookup */
for (int i = 0; i < count; i++) {
#if defined(__FreeBSD__) || defined(__DragonFly__)
struct ifvlanreq vr;
struct ifreq ifr;
memset(&vr, 0, sizeof(vr));
memset(&ifr, 0, sizeof(ifr));
strlcpy(ifr.ifr_name, iflist[i].name, IFNAMSIZ);
ifr.ifr_data = (caddr_t)&vr;
if (ioctl(sock, SIOCGIFVLAN, &ifr) == 0) {
strlcpy(iflist[i].vlan_parent,
vr.ifvr_parent, IFNAMSIZ);
iflist[i].vlan_tag = vr.ifvr_tag;
}
#elif defined(__OpenBSD__)
struct vlanreq vr;
struct ifreq ifr;
memset(&vr, 0, sizeof(vr));
memset(&ifr, 0, sizeof(ifr));
strlcpy(ifr.ifr_name, iflist[i].name, IFNAMSIZ);
ifr.ifr_data = (caddr_t)&vr;
if (ioctl(sock, SIOCGIFVLAN, &ifr) == 0) {
strlcpy(iflist[i].vlan_parent,
vr.vlr_parent, IFNAMSIZ);
iflist[i].vlan_tag = vr.vlr_tag;
}
#elif defined(__NetBSD__)
struct vlanreq vr;
struct ifreq ifr;
memset(&vr, 0, sizeof(vr));
memset(&ifr, 0, sizeof(ifr));
strlcpy(ifr.ifr_name, iflist[i].name, IFNAMSIZ);
ifr.ifr_data = (caddr_t)&vr;
if (ioctl(sock, SIOCGLIFVLAN, &ifr) == 0) {
strlcpy(iflist[i].vlan_parent,
vr.vlr_parent, IFNAMSIZ);
iflist[i].vlan_tag = vr.vlr_tag;
}
#endif
}
/* Bridge lookup */
#if defined(__FreeBSD__) || defined(__DragonFly__) || defined(__OpenBSD__)
for (int i = 0; i < count; i++) {
if (iflist[i].sdl_type != IFT_BRIDGE) continue;
size_t blen = 8192;
char *buf = malloc(blen);
struct ifbaconf *bac = (void*)buf;
bac->ifbac_len = blen;
strlcpy(bac->ifbac_name,
iflist[i].name, IFNAMSIZ);
if (ioctl(sock, SIOCBRDGRTABLE, bac) == 0) {
int n = bac->ifbac_len / sizeof(struct ifbareq);
for (int j = 0; j < n; j++) {
char *member = bac->ifbac_req[j].ifbr_member;
for (int k = 0; k < count; k++) {
if (strcmp(iflist[k].name, member)==0)
strlcpy(iflist[k].br_master,
iflist[i].name, IFNAMSIZ);
}
}
}
free(buf);
}
#elif defined(__NetBSD__)
for (int i = 0; i < count; i++) {
if (iflist[i].sdl_type != IFT_BRIDGE) continue;
size_t blen = 8192;
char *buf = malloc(blen);
struct ifbrlconf *lcf = (void*)buf;
lcf->ifbrl_len = blen;
strlcpy(lcf->ifbrl_name,
iflist[i].name, IFNAMSIZ);
if (ioctl(sock, SIOCGIFBRIDGE, lcf) == 0) {
int n = lcf->ifbrl_len / sizeof(struct ifbreq);
for (int j = 0; j < n; j++) {
char *member = lcf->ifbrl_buf[j].ifbr_ifsname;
for (int k = 0; k < count; k++) {
if (strcmp(iflist[k].name, member)==0)
strlcpy(iflist[k].br_master,
iflist[i].name, IFNAMSIZ);
}
}
}
free(buf);
}
#endif
close(sock);
}
#elif defined(__sun)
{
dladm_handle_t dh;
if (dladm_open(&dh, 0) != DLADM_STATUS_OK) {
fprintf(stderr, "dladm_open failed\n");
return 1;
}
dladm_walk_vlan(dh,
DLADM_OPT_ACTIVE | DLADM_OPT_PERSIST,
solaris_vlan_cb,
iflist);
dladm_walk_bridge(dh,
DLADM_OPT_ACTIVE | DLADM_OPT_PERSIST,
solaris_bridge_cb,
iflist);
dladm_close(dh);
}
#endif
/* Print table */
printf("%-10s %-15s %-6s %-8s %-12s %-5s\n",
"IFACE", "LINK TYPE", "KIND",
"VLAN", "BRIDGE", "TOP?");
for (int i = 0; i < count; i++) {
int is_sub = (iflist[i].vlan_tag >= 0) ||
(iflist[i].br_master[0] != '-');
printf("%-10s %-15s %-6s %-8s %-12s %-5s\n",
iflist[i].name,
link_type_str(iflist[i].sdl_type),
kind_str(iflist[i].sdl_type),
iflist[i].vlan_parent,
iflist[i].br_master,
is_sub ? "no" : "yes");
}
return 0;
} Based on this, everything based on *BSD basically uses the same strategy with only minor details different on each platform. The only larger outlier is Solaris (but I expected as much). Nonetheless, with some tweaking we should get a quite good baseline for filtering duplicated traffic. |
@@ -45,7 +45,11 @@ static void NetworkIOMeter_updateValues(Meter* this) { | |||
|
|||
/* update only every 500ms to have a sane span for rate calculation */ | |||
if (passedTimeInMs > 500) { | |||
#ifdef IGNORE_VIRTUAL_INTF | |||
hasNewData = Platform_getNetworkIO(&data, host->settings->ignoreVirtualNetworkInterfaces); |
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 don't like two kinds of function prototypes that depend on a compile time option. Even though the code can compile, the braces are unbalanced and would very likely confuse IDEs.
Another point is, you might not need to pass the boolean setting as a function argument. I suggest making it a member of the NetworkIOData
(bool ignoreVirtualInterfaces;
), and let NetworkIOMeter_updateValues
set it before calling Platform_getNetworkIO
.
Hello,
In Issue #1739 was asked to create an option to avoid virtual interfaces to count Rx/Tx.
[root cause]
Traffic that goes into or leave the virtual interface also passes through the physical interface. This duplicates the Rx and Tx values.
[solution]
Create a new Global Options to ignore the virtual interface to count traffic. The option is disabled by default.
At Platform_getNetworkIO() check for virtual interface before summing up Rx/Tx bytes and packets.
The code is protected by the flag IGNORE_VIRTUAL_INTF and its compilation flag --enable-ignore-virtual-intf.
[unit test]
I used this container in order to help me with this: https://hub.docker.com/r/openspeedtest/latest
This is my first pull request on public project, sorry for any mistake.