diff mbox series

[v3] netifd: add devtype to ubus call

Message ID 20211209151819.5835-1-fe@dev.tdt.de
State Superseded
Headers show
Series [v3] netifd: add devtype to ubus call | expand

Commit Message

Florian Eckert Dec. 9, 2021, 3:18 p.m. UTC 
Every network device has a type. There is no standard interface here.
The type can be determined either from the file
'/sys/class/net/<device>/uevent' or, if no information is found
there, from the file '/sys/class/net/<device>/type'.

This new function first checks whether there is a DEVTYPE=<type> sring in
the 'uevent' file and uses it. If it does not find this information,
the 'type' is used as a fallback and mapped the number to a character
sequence.

This new 'devtype' information can be found in the network.device ubus
call.

Command:
ubus call network.device status

Output:
{
	"eth0": {
		"devtype": "ethernet",

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
v2:
  - Remove debug log output
v3:
  - Use the information mainly from file 'uevnet'.
  - If 'uevent' does not provide the information use file 'type'
 ...-system-linux-add-interface-protocol.patch | 107 ++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch

Comments

Hans Dedecker Dec. 25, 2021, 7:44 p.m. UTC | #1 
Hi Florian,

Can you create the patch against the netifd repo ?
Further comments inline

Hans

On Thu, Dec 9, 2021 at 4:18 PM Florian Eckert <fe@dev.tdt.de> wrote:
>
> Every network device has a type. There is no standard interface here.
> The type can be determined either from the file
> '/sys/class/net/<device>/uevent' or, if no information is found
> there, from the file '/sys/class/net/<device>/type'.
>
> This new function first checks whether there is a DEVTYPE=<type> sring in
> the 'uevent' file and uses it. If it does not find this information,
> the 'type' is used as a fallback and mapped the number to a character
> sequence.
>
> This new 'devtype' information can be found in the network.device ubus
> call.
>
> Command:
> ubus call network.device status
>
> Output:
> {
>         "eth0": {
>                 "devtype": "ethernet",
>
> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
> v2:
>   - Remove debug log output
> v3:
>   - Use the information mainly from file 'uevnet'.
>   - If 'uevent' does not provide the information use file 'type'
>  ...-system-linux-add-interface-protocol.patch | 107 ++++++++++++++++++
>  1 file changed, 107 insertions(+)
>  create mode 100644 package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch
>
> diff --git a/package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch b/package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch
> new file mode 100644
> index 0000000000..62662b3df3
> --- /dev/null
> +++ b/package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch
> @@ -0,0 +1,107 @@
> +--- a/system-linux.c
> ++++ b/system-linux.c
> +@@ -2395,6 +2395,50 @@ system_if_force_external(const char *ifn
> +       return stat(dev_sysfs_path(ifname, "phy80211"), &s) == 0;
> + }
> +
> ++static inline unsigned short
> ++system_netdevtype_to_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;
> ++}
> ++
> ++static void
> ++system_add_devtype(struct blob_buf *b, const char *ifname)
> ++{
> ++      char buf[100];
> ++      bool found = false;
> ++
> ++      if (!system_get_dev_sysfs("uevent", ifname, buf, sizeof(buf))) {
> ++              const char *info = "DEVTYPE=";
> ++              char* context = NULL;
> ++              const char *line = strtok_r(buf, "\r\n", &context);
> ++              while( line != NULL ) {
nitpick: keep coding style consistent by using a white space after
while and before the bracket
> ++                      char *index = strstr(line, info);
> ++                      if(index != NULL) {
nitpick: use a white space after if
> ++                              blobmsg_add_string(b, "devtype", index + strlen(info));
> ++                              found = true;
> ++                              break;
> ++                      }
> ++                      line = strtok_r(NULL, "\r\n", &context);
> ++              }
> ++      }
> ++
> ++      if (!found) {
> ++              int i;
> ++              unsigned short number = 0;
> ++              if (!system_get_dev_sysfs("type", ifname, buf, sizeof(buf))) {
> ++                      number = strtoul(buf, NULL, 0);
> ++                      i = system_netdevtype_to_pos(number);
> ++                      blobmsg_add_string(b, "devtype", netdev_type_name[i]);
> ++              }
> ++      }
> ++}
> ++
> + int
> + system_if_dump_info(struct device *dev, struct blob_buf *b)
> + {
> +@@ -2430,6 +2474,8 @@ system_if_dump_info(struct device *dev,
> +               blobmsg_add_u8(b, "autoneg", !!ecmd.autoneg);
> +       }
> +
> ++      system_add_devtype(b, dev->ifname);
> ++
> +       return 0;
> + }
> +
> +--- 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", "ethernet", "eethernet", "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", "ie80211-prism",
> ++      "ieee80211-radiotap", "phonet", "phonet-pipe",
> ++      "ieee802154", "void", "none"
> ++};
Merge these two arrays into one array, each entry having the netdev
type number and the corresponding string.
Implement a function which uses the array and takes as argument the
netdev type number and returns the corresponding string
> ++
> + enum tunnel_param {
> +       TUNNEL_ATTR_TYPE,
> +       TUNNEL_ATTR_REMOTE,
> --
> 2.20.1
>
Florian Eckert Jan. 10, 2022, 3:55 p.m. UTC | #2 
Hello Hans,

thanks for you review.

>> +--- 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", "ethernet", "eethernet", "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", "ie80211-prism",
>> ++      "ieee80211-radiotap", "phonet", "phonet-pipe",
>> ++      "ieee802154", "void", "none"
>> ++};
> Merge these two arrays into one array, each entry having the netdev
> type number and the corresponding string.
> Implement a function which uses the array and takes as argument the
> netdev type number and returns the corresponding string

I am not quite sure if I have understood this correctly.
If you take a look at the include where the ARPHRD are defined.
https://elixir.bootlin.com/linux/v5.16-rc1/source/include/uapi/linux/if_arp.h
Then the array must have 65535 entries ARPHRD_VOID is at position 
0xFFFF.
That's quite a lot for the fact that there are 58 ids.

This is the code I would use?

static const char * const netdev_type[] = {
  [ARPHRD_NETROM] = "netrom",
  [ARPHRD_ETHER] = "ethernet",
...
...
};

Or should I define an array of netdev_type structs?

struct netdev_type {
   const char *name;
   unsigned short number;
};

#define DEV_TYPE(_name, _number) \
{                                \
   .name = _name,                 \
   .number = _number              \
}                                \


static const struct netdev_type netdev_types[] = {
{
   DEV_TYPE("netrom", ARPHRD_NETROM),
   DEV_TYPE("ethernet", ARPHRD_ETHER),
...
...
};

Best Regards
Florian
Hans Dedecker Jan. 10, 2022, 7:29 p.m. UTC | #3 
On Mon, Jan 10, 2022 at 4:55 PM Florian Eckert <fe@dev.tdt.de> wrote:
>
> Hello Hans,
>
> thanks for you review.
>
> >> +--- 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", "ethernet", "eethernet", "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", "ie80211-prism",
> >> ++      "ieee80211-radiotap", "phonet", "phonet-pipe",
> >> ++      "ieee802154", "void", "none"
> >> ++};
> > Merge these two arrays into one array, each entry having the netdev
> > type number and the corresponding string.
> > Implement a function which uses the array and takes as argument the
> > netdev type number and returns the corresponding string
>
> I am not quite sure if I have understood this correctly.
> If you take a look at the include where the ARPHRD are defined.
> https://elixir.bootlin.com/linux/v5.16-rc1/source/include/uapi/linux/if_arp.h
> Then the array must have 65535 entries ARPHRD_VOID is at position
> 0xFFFF.
> That's quite a lot for the fact that there are 58 ids.
>
> This is the code I would use?
>
> static const char * const netdev_type[] = {
>   [ARPHRD_NETROM] = "netrom",
>   [ARPHRD_ETHER] = "ethernet",
> ...
> ...
> };
>
> Or should I define an array of netdev_type structs?
This is indeed what I had in mind

Hans
>
> struct netdev_type {
>    const char *name;
>    unsigned short number;
> };
>
> #define DEV_TYPE(_name, _number) \
> {                                \
>    .name = _name,                 \
>    .number = _number              \
> }                                \
>
>
> static const struct netdev_type netdev_types[] = {
> {
>    DEV_TYPE("netrom", ARPHRD_NETROM),
>    DEV_TYPE("ethernet", ARPHRD_ETHER),
> ...
> ...
> };
>
> Best Regards
> Florian
diff mbox series

Patch

diff --git a/package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch b/package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch
new file mode 100644
index 0000000000..62662b3df3
--- /dev/null
+++ b/package/network/config/netifd/patches/001-system-linux-add-interface-protocol.patch
@@ -0,0 +1,107 @@ 
+--- a/system-linux.c
++++ b/system-linux.c
+@@ -2395,6 +2395,50 @@ system_if_force_external(const char *ifn
+ 	return stat(dev_sysfs_path(ifname, "phy80211"), &s) == 0;
+ }
+ 
++static inline unsigned short
++system_netdevtype_to_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;
++}
++
++static void
++system_add_devtype(struct blob_buf *b, const char *ifname)
++{
++	char buf[100];
++	bool found = false;
++
++	if (!system_get_dev_sysfs("uevent", ifname, buf, sizeof(buf))) {
++		const char *info = "DEVTYPE=";
++		char* context = NULL;
++		const char *line = strtok_r(buf, "\r\n", &context);
++		while( line != NULL ) {
++			char *index = strstr(line, info);
++			if(index != NULL) {
++				blobmsg_add_string(b, "devtype", index + strlen(info));
++				found = true;
++				break;
++			}
++			line = strtok_r(NULL, "\r\n", &context);
++		}
++	}
++
++	if (!found) {
++		int i;
++		unsigned short number = 0;
++		if (!system_get_dev_sysfs("type", ifname, buf, sizeof(buf))) {
++			number = strtoul(buf, NULL, 0);
++			i = system_netdevtype_to_pos(number);
++			blobmsg_add_string(b, "devtype", netdev_type_name[i]);
++		}
++	}
++}
++
+ int
+ system_if_dump_info(struct device *dev, struct blob_buf *b)
+ {
+@@ -2430,6 +2474,8 @@ system_if_dump_info(struct device *dev,
+ 		blobmsg_add_u8(b, "autoneg", !!ecmd.autoneg);
+ 	}
+ 
++	system_add_devtype(b, dev->ifname);
++
+ 	return 0;
+ }
+ 
+--- 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", "ethernet", "eethernet", "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", "ie80211-prism",
++	"ieee80211-radiotap", "phonet", "phonet-pipe",
++	"ieee802154", "void", "none"
++};
++
+ enum tunnel_param {
+ 	TUNNEL_ATTR_TYPE,
+ 	TUNNEL_ATTR_REMOTE,