Message ID | 20211206101546.830-1-fe@dev.tdt.de |
---|---|
State | Superseded |
Headers | show |
Series | netifd: system-linux: add dev_type info for ubus network.device status | expand |
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
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) +{ + int i; + + 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; + 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" +}; + enum tunnel_param { TUNNEL_ATTR_TYPE, TUNNEL_ATTR_REMOTE,
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(+)