Skip to contentSkip to conversation

iw: nl80211: add NLA_F_NESTED to nla_nest_start() with older libnl versions

PATCH In review Gokul Sivakumar 2 participants 9 messages Sep 10, 2021 – Sep 23, 2021

lore
Share & tools
lore
#0 lore raw

PATCH 1/4 iw: nl80211: add NLA_F_NESTED to nla_nest_start() with older libnl versions

It is noticed in Kernel version 5.14.0-rc4+, that when sending the NL cmd NL80211_CMD_SET_TID_CONFIG with nested attrs under NL80211_ATTR_TID_CONFIG, kernel returnes a response with the error "NLA_F_NESTED is missing".

 $ sudo ./iw dev wlan0 set tidconf tids 0x1 ampdu on
 kernel reports: NLA_F_NESTED is missing
 command failed: Invalid argument (-22))

Fix this by setting NLA_F_NESTED flag everytime when using nla_nest_start() library function. This is needed to make cfg80211 allow the nl80211 command NL80211_ATTR_TID_CONFIG in the new kernel versions that enforce netlink attribute policy validation.

---
 iw.h | 5 +++++
 1 file changed, 5 insertions(+)
iw.h +5 -0
index a118f5b..545fd0e 100644
@@ -11,6 +11,11 @@
 #include "nl80211.h"
 #include "ieee80211.h"
 
+#ifndef NL_CAPABILITY_VERSION_3_5_0
+#define nla_nest_start(msg, attrtype) \
+       nla_nest_start(msg, NLA_F_NESTED | (attrtype))
+#endif
+
 /* support for extack if compilation headers are too old */
 #ifndef NETLINK_EXT_ACK
 #define NETLINK_EXT_ACK 11
Signature · 1 line
2.25.1
#1 lore raw

PATCH 2/4 iw: link: mention the need for MAC addr arg in link get_sta cmd usage menu

The usage menu shown when running the hidden "link get_sta" command is not mentioning the need for the MAC address argument. Without this, the cmd is always failing with the output shown below.

 $ ./iw dev wlan0 link get_sta
 Usage:  ./iw [options] dev <devname> link get_sta
 ...

To avoid this, let the user know about the mandatory "MAC address" argument like below.

 $ ./iw dev wlan0 link get_sta
 Usage:  ./iw [options] dev <devname> link get_sta <mac-addr>
 ...
---
 link.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
link.c +1 -1
index 1ed7f63..2074488 100644
@@ -273,7 +273,7 @@ static int handle_link(struct nl80211_state *state,
 }
 TOPLEVEL(link, NULL, 0, 0, CIB_NETDEV, handle_link,
 	 "Print information about the current link, if any.");
-HIDDEN(link, get_sta, "", NL80211_CMD_GET_STATION, 0,
+HIDDEN(link, get_sta, "<mac-addr>", NL80211_CMD_GET_STATION, 0,
 	CIB_NETDEV, handle_link_sta);
 HIDDEN(link, get_bss, NULL, NL80211_CMD_GET_SCAN, NLM_F_DUMP,
 	CIB_NETDEV, handle_scan_for_link);
Signature · 1 line
2.25.1
#2 lore raw

PATCH 3/4 iw: event: add the missing time display format in the "iw event" help menu

In reply to Gokul Sivakumar

The option used to print the events with timestamp in Human readable format is not listed in the "$ iw event -h" output.

 $ ./iw event -h
 Usage:  ./iw [options] event [-t|-r] [-f] [-n]
 ...

So add "-T" option to the help menu.

 $ ./iw event -h
 Usage:  ./iw [options] event [-t|-T|-r] [-f] [-n]
 ...
---
 event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
event.c +1 -1
index 14f101b..e0908dd 100644
@@ -1457,7 +1457,7 @@ static int print_events(struct nl80211_state *state,
 
 	return __do_listen_events(state, 0, NULL, 0, NULL, &args);
 }
-TOPLEVEL(event, "[-t|-r] [-f]", 0, 0, CIB_NONE, print_events,
+TOPLEVEL(event, "[-t|-T|-r] [-f]", 0, 0, CIB_NONE, print_events,
 	"Monitor events from the kernel.\n"
 	"-t - print timestamp\n"
 	"-T - print absolute, human-readable timestamp\n"
Signature · 1 line
2.25.1
#3 lore raw

PATCH 4/4 iw: mesh: add comments in the mesh confguration parameter printing sections

In reply to Gokul Sivakumar
---
 mesh.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mesh.c +2 -1
index 943edf5..0fb98a3 100644
@@ -400,6 +400,7 @@ static int print_mesh_param_handler(struct nl_msg *msg, void *arg)
 	if (!mdescr) {
 		unsigned int i;
 
+		/* print out all the supported mesh parameters */
 		for (i = 0; i < ARRAY_SIZE(_mesh_param_descrs); i++) {
 			mdescr = &_mesh_param_descrs[i];
 			if (mesh_params[mdescr->mesh_param_num]) {
@@ -411,7 +412,7 @@ static int print_mesh_param_handler(struct nl_msg *msg, void *arg)
 		return NL_SKIP;
 	}
 
-	/* print out the mesh parameter */
+	/* print out the requested mesh parameter */
 	if (mesh_params[mdescr->mesh_param_num]) {
 		mdescr->nla_print_fn(mesh_params[mdescr->mesh_param_num]);
 		printf("\n");
Signature · 1 line
2.25.1
#5 lore raw
Quoted 9 lines · Johannes Berg

Applied 2-4, but

Quoted 2 lines

+#ifndef NL_CAPABILITY_VERSION_3_5_0

I can find no evidence of that symbol ever existing anywhere?

johannes

The symbol NL_CAPABILITY_VERSION_3_5_0 is part of the libnl library and this will be defined when using the libnl library version >= 3.5.0. From libnl 3.5.0, the library itself handles setting NLA_F_NESTED flag when using nla_nest_start() lib function. Please refer the commit 7de65a0 ("attr: mark nested attributes as NLA_F_NESTED") in libnl gitub tree (https://github.com/thom311/libnl/commit/7de65a0).

With this new code changes in iw.h, it will helpful if "iw" uses older (< 3.5.0) libnl versions. In such cases NL_CAPABILITY_VERSION_3_5_0 will not be defined in libnl and so iw itself will set the NLA_F_NESTED flag when invoking the lib function nla_nest_start().

And hostapd/supplicant already started following the same approach.

Gokul

#6 lore raw
Quoted 7 lines · Gokul Sivakumar

The symbol NL_CAPABILITY_VERSION_3_5_0 is part of the libnl library and this will be defined when using the libnl library version >= 3.5.0. From libnl 3.5.0, the library itself handles setting NLA_F_NESTED flag when using nla_nest_start() lib function. Please refer the commit 7de65a0 ("attr: mark nested attributes as NLA_F_NESTED") in libnl gitub tree (https://github.com/thom311/libnl/commit/7de65a0).

Huh ok, I guess I missed the memo on the (official?) tree moving ...

johannes

#7 lore raw
Quoted 9 lines · Johannes Berg
Quoted 7 lines · Gokul Sivakumar

The symbol NL_CAPABILITY_VERSION_3_5_0 is part of the libnl library and this will be defined when using the libnl library version >= 3.5.0. From libnl 3.5.0, the library itself handles setting NLA_F_NESTED flag when using nla_nest_start() lib function. Please refer the commit 7de65a0 ("attr: mark nested attributes as NLA_F_NESTED") in libnl gitub tree (https://github.com/thom311/libnl/commit/7de65a0).

Quoted 4 lines

Huh ok, I guess I missed the memo on the (official?) tree moving ...

johannes

I beleive https://github.com/thom311/libnl/ is the official tree for libnl and I see Pull Requests are getting accepted in Github. Others can confirm!

The git tree git://git.infradead.org/users/tgr/libnl.git mentioned in
https://www.infradead.org/~tgr/libnl/ doesn't seem to be accessible.

Gokul

#8 lore raw
Quoted 10 lines · Gokul Sivakumar
Quoted 9 lines · Johannes Berg
Quoted 7 lines · Gokul Sivakumar

The symbol NL_CAPABILITY_VERSION_3_5_0 is part of the libnl library and this will be defined when using the libnl library version >= 3.5.0. From libnl 3.5.0, the library itself handles setting NLA_F_NESTED flag when using nla_nest_start() lib function. Please refer the commit 7de65a0 ("attr: mark nested attributes as NLA_F_NESTED") in libnl gitub tree (https://github.com/thom311/libnl/commit/7de65a0).

Quoted 7 lines
Quoted 4 lines

Huh ok, I guess I missed the memo on the (official?) tree moving ...

johannes

I beleive https://github.com/thom311/libnl/ is the official tree for libnl and I see Pull Requests are getting accepted in Github. Others can confirm!

Yeah, debian seems to be shipping from that tree too.

johannes