netifd: system-linux: add dev_type info for ubus network.device status

Every network device has a type. This is stored in the sysfs under
'/sys/class/net/<device>/type' as a number and can be queried.

This commit adds this information as a string to the ubus.
'ubus call network.device status'
{
...
	"eth0": {
		"dev_type": "ETHER",
...
}

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 system-linux.c | 21 +++++++++++++++++++++
 system.h       | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

Hi,

imho these types are not that useful in practice (e.g. tap devices etc. are
all reported as "ethernet". Maybe expose /sys/class/net/$devname/uevent
DEVTYP= instead.

~ Jo

Hello Jo

> imho these types are not that useful in practice (e.g. tap devices etc. 
> are
> all reported as "ethernet". Maybe expose /sys/class/net/$devname/uevent
> DEVTYP= instead.

I have now taken a look at your suggestion.
Unfortunately, I found that not all network interfaces have set the 
DEVTYPE
attribute set in their uevent file. I have not yet found any information
who sets this value. Does this do the driver or the subsystem?
I need to do some more research

The following devices does have this DEVTYPE attribute set on my targets

root@st-dev-07 /sys/class/net # cat docker0/uevent
DEVTYPE=bridge
INTERFACE=docker0
IFINDEX=16

root@st-dev-07 /sys/class/net # cat wlan0/uevent
DEVTYPE=wlan
INTERFACE=wlan0
IFINDEX=14

root@st-dev-07 /sys/class/net # cat wwan0/uevent
DEVTYPE=wwan
INTERFACE=wwan0
IFINDEX=5

root@VR2-120770 ~ # cat /sys/class/net/pppoe-xdsl/uevent
DEVTYPE=ppp
INTERFACE=pppoe-xdsl
IFINDEX=2285
root@VR2-120770 ~ #


The following devices does *not* have this DEVTYPE attribute set on my 
targets:

root@st-dev-07 /sys/class/net # cat eth0/uevent
INTERFACE=eth0
IFINDEX=2

root@st-dev-07 /sys/class/net # cat sit0/uevent
INTERFACE=sit0
IFINDEX=7

root@st-dev-07 /sys/class/net # cat ip6_vti0/uevent
INTERFACE=ip6_vti0
IFINDEX=9

The question now if this DEVTYPE is not set what should we do?
Set now `dev_type` value in the ubus output til we update this in the 
Kernel?

Regards

Florian

Hi,

> I have now taken a look at your suggestion.
> Unfortunately, I found that not all network interfaces have set the DEVTYPE
> attribute set in their uevent file. I have not yet found any information
> who sets this value. Does this do the driver or the subsystem?

afair it is set by the responsible driver.

> [...]
> The question now if this DEVTYPE is not set what should we do?

I think in case of an absent DEVTYPE variable you can assume "ethernet" as
fallback (cross check with /sys/class/net/*/type == 1).

Another approach would be falling back to your original mapping in case
DEVTYPE is not present (so 1->ethernet, 24->ppp, etc.)


~ Jo

This is probably after the fact, but...


> On Dec 6, 2021, at 3:15 AM, Florian Eckert <fe@dev.tdt.de> wrote:
> 
> Every network device has a type. This is stored in the sysfs under
> '/sys/class/net/<device>/type' as a number and can be queried.
> 
> This commit adds this information as a string to the ubus.
> 'ubus call network.device status'
> {
> ...
> 	"eth0": {
> 		"dev_type": "ETHER",
> ...
> }
> 
> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
> system-linux.c | 21 +++++++++++++++++++++
> system.h       | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
> 
> diff --git a/system-linux.c b/system-linux.c
> index e768853..4ac34cc 100644
> --- a/system-linux.c
> +++ b/system-linux.c
> @@ -2395,6 +2395,17 @@ system_if_force_external(const char *ifname)
> 	return stat(dev_sysfs_path(ifname, "phy80211"), &s) == 0;
> }
> 
> +static inline unsigned short netdev_type_pos(unsigned short dev_type)


Is there not a typedef'd definition for this?  If not, should we add one like "dev_type_t"?


> +{
> +	int i;


This can only be a positive (cardinal) number... therefore, please use an unsigned instead.


> +
> +	for (i = 0; i < ARRAY_SIZE(netdev_type_number); i++)
> +		if (netdev_type_number[i] == dev_type)
> +			return i;
> +	/* the last key is used by default */
> +	return ARRAY_SIZE(netdev_type_number) - 1;
> +}
> +
> int
> system_if_dump_info(struct device *dev, struct blob_buf *b)
> {
> @@ -2402,6 +2413,9 @@ system_if_dump_info(struct device *dev, struct blob_buf *b)
> 	struct ifreq ifr;
> 	char *s;
> 	void *c;
> +	unsigned short dev_type = 0;
> +	int i;


netdev_type_pos() returns an unsigned short.  Please make this concur.


> +	char buf[10];
> 
> 	memset(&ecmd, 0, sizeof(ecmd));
> 	memset(&ifr, 0, sizeof(ifr));
> @@ -2430,6 +2444,13 @@ system_if_dump_info(struct device *dev, struct blob_buf *b)
> 		blobmsg_add_u8(b, "autoneg", !!ecmd.autoneg);
> 	}
> 
> +	if (!system_get_dev_sysfs("type", dev->ifname, buf, sizeof(buf))) {
> +		dev_type = strtoul(buf, NULL, 0);
> +		i = netdev_type_pos(dev_type);
> +		blobmsg_add_string(b, "dev_type", netdev_type_name[i]);
> +		netifd_log_message(L_CRIT, "Interface type %s detected\n", netdev_type_name[i]);
> +	}
> +
> 	return 0;
> }
> 
> diff --git a/system.h b/system.h
> index 1f7037d..fdd8a64 100644
> --- a/system.h
> +++ b/system.h
> @@ -23,6 +23,42 @@
> #include "iprule.h"
> #include "utils.h"
> 
> +static const unsigned short netdev_type_number[] = {
> +	ARPHRD_NETROM, ARPHRD_ETHER, ARPHRD_EETHER, ARPHRD_AX25,
> +	ARPHRD_PRONET, ARPHRD_CHAOS, ARPHRD_IEEE802, ARPHRD_ARCNET,
> +	ARPHRD_APPLETLK, ARPHRD_DLCI, ARPHRD_ATM, ARPHRD_METRICOM,
> +	ARPHRD_IEEE1394, ARPHRD_EUI64, ARPHRD_INFINIBAND, ARPHRD_SLIP,
> +	ARPHRD_CSLIP, ARPHRD_SLIP6, ARPHRD_CSLIP6, ARPHRD_RSRVD,
> +	ARPHRD_ADAPT, ARPHRD_ROSE, ARPHRD_X25, ARPHRD_HWX25,
> +	ARPHRD_PPP, ARPHRD_CISCO, ARPHRD_LAPB, ARPHRD_DDCMP,
> +	ARPHRD_RAWHDLC, ARPHRD_TUNNEL, ARPHRD_TUNNEL6, ARPHRD_FRAD,
> +	ARPHRD_SKIP, ARPHRD_LOOPBACK, ARPHRD_LOCALTLK, ARPHRD_FDDI,
> +	ARPHRD_BIF, ARPHRD_SIT, ARPHRD_IPDDP, ARPHRD_IPGRE,
> +	ARPHRD_PIMREG, ARPHRD_HIPPI, ARPHRD_ASH, ARPHRD_ECONET,
> +	ARPHRD_IRDA, ARPHRD_FCPP, ARPHRD_FCAL, ARPHRD_FCPL,
> +	ARPHRD_FCFABRIC, ARPHRD_IEEE80211, ARPHRD_IEEE80211_PRISM,
> +	ARPHRD_IEEE80211_RADIOTAP, ARPHRD_PHONET, ARPHRD_PHONET_PIPE,
> +	ARPHRD_IEEE802154, ARPHRD_VOID, ARPHRD_NONE
> +};
> +
> +static const char *const netdev_type_name[] = {
> +	"NETROM", "ETHER", "EETHER", "AX25",
> +	"PRONET", "CHAOS", "IEEE802", "ARCNET",
> +	"APPLETLK", "DLCI", "ATM", "METRICOM",
> +	"IEEE1394", "EUI64", "INFINIBAND", "SLIP",
> +	"CSLIP", "SLIP6", "CSLIP6", "RSRVD",
> +	"ADAPT", "ROSE", "X25", "HWX25",
> +	"PPP", "CISCO", "LAPB", "DDCMP",
> +	"RAWHDLC", "TUNNEL", "TUNNEL6", "FRAD",
> +	"SKIP", "LOOPBACK", "LOCALTLK", "FDDI",
> +	"BIF", "SIT", "IPDDP", "IPGRE",
> +	"PIMREG", "HIPPI", "ASH", "ECONET",
> +	"IRDA", "FCPP", "FCAL", "FCPL",
> +	"FCFABRIC", "IEEE80211", "IEEE80211_PRISM",
> +	"IEEE80211_RADIOTAP", "PHONET", "PHONET_PIPE",
> +	"IEEE802154", "VOID", "NONE"
> +};


I'm worried about holes/discontinuities in ARPHRD_* space... would that break things?  I guess this could be a sparse array...

-Philip


> +
> enum tunnel_param {
> 	TUNNEL_ATTR_TYPE,
> 	TUNNEL_ATTR_REMOTE,
> -- 
> 2.20.1

> On Dec 7, 2021, at 5:26 AM, Jo-Philipp Wich <jo@mein.io> wrote:
> 
> Hi,
> 
>> I have now taken a look at your suggestion.
>> Unfortunately, I found that not all network interfaces have set the DEVTYPE
>> attribute set in their uevent file. I have not yet found any information
>> who sets this value. Does this do the driver or the subsystem?
> 
> afair it is set by the responsible driver.
> 
>> [...]
>> The question now if this DEVTYPE is not set what should we do?
> 
> I think in case of an absent DEVTYPE variable you can assume "ethernet" as
> fallback (cross check with /sys/class/net/*/type == 1).


What do IP-level devices (like GRE or IP-over-IP) tunnels do?

-Philip


> 
> Another approach would be falling back to your original mapping in case
> DEVTYPE is not present (so 1->ethernet, 24->ppp, etc.)
> 
> 
> ~ Jo