Skip to contentSkip to conversation

nl80211 + packet injection with it and d80211

RFC New Johannes Berg 3 participants 7 messages Aug 17, 2006 – Aug 21, 2006

Subsystems (3)
lore
Share & tools
lore

Johannes Berg

author maintainer
#0 lore raw

RFC 1/1 nl80211 + packet injection with it and d80211

Hey,

This is a patch laying the groundwork for nl80211. I've decided to implement it below net/wireless/ so that when we add further things like configfs support for some things that can just be added there.

Currently, it lays the groundwork for netlink userspace communication. None of the attribute stuff and configuration stuff I talked about is implemented yet, nor anything like having wext go over netlink obviously.

This patch now serves mainly to illustrate how I intend to multiplex everything. Also, as added bonus, it implements packet injection over netlink into the d80211 stack :) NB: missing is tx status notification via netlink. The patch was large enough already.

I'd appreciate any feedback on the implementation.

johannes

include/net/nl80211.h new file +55 -0
@@ -0,0 +1,55 @@
+#ifndef __NET_NL80211_H
+#define __NET_NL80211_H
+
+#include <linux/netlink.h>
+#include <net/genetlink.h>
+#include <linux/nl80211.h>
+#include <linux/skbuff.h>
+
+/*
+ * 802.11 netlink in-kernel interface
+ *
+ * Copyright 2006 Johannes Berg <[email protected]>
+ */
+
+/**
+ * struct nl80211_call - backend call for wireless configuration
+ *
+ * An array of pointers to &struct nl80211_call is registered
+ * by each backend (driver or wireless stack) within
+ * &struct nl80211_driver.
+ *
+ * @doit: see &struct genl_ops doit
+ * @dumpit: see &struct genl_ops dumpit
+ */
+struct nl80211_call {
+	int	(*doit)(struct sk_buff *skb,
+			struct genl_info *info);
+	int	(*dumpit)(struct sk_buff *skb,
+			  struct netlink_callback *cb);
+};
+
+/**
+ * struct nl80211_driver - backend description for wireless configuration
+ *
+ * This struct is registered by fullmac card drivers and/or wireless stacks
+ * in order to handle configuration requests on their interfaces.
+ *
+ * @get_calls_for_ifindex: return an array of size NL80211_CMD_MAX
+ *			   with an entry for each 
+ */
+struct nl80211_driver {
+	struct nl80211_call**	(*get_calls_for_ifindex)(int ifindex);
+
+	/* private: */
+	struct list_head driver_list;
+};
+
+extern void nl80211_register_driver(struct nl80211_driver *drv);
+extern void nl80211_unregister_driver(struct nl80211_driver *drv);
+extern void *nl80211hdr_put(struct sk_buff *skb, u32 pid,
+			    u32 seq, int flags, u8 cmd);
+extern void *nl80211msg_new(struct sk_buff **skb, u32 pid,
+			    u32 seq, int flags, u8 cmd);
+
+#endif /* __NET_NL80211_H */
net/Kconfig +3 -0
@@ -250,6 +250,9 @@
 config WIRELESS_EXT
 	bool
 
+config NETLINK_80211
+	tristate
+
 endif   # if NET
 endmenu # Networking
 
net/Makefile +1 -0
@@ -44,6 +44,7 @@
 obj-$(CONFIG_VLAN_8021Q)	+= 8021q/
 obj-$(CONFIG_IP_DCCP)		+= dccp/
 obj-$(CONFIG_IP_SCTP)		+= sctp/
+obj-$(CONFIG_NETLINK_80211)	+= wireless/
 obj-$(CONFIG_D80211)		+= d80211/
 obj-$(CONFIG_IEEE80211)		+= ieee80211/
 obj-$(CONFIG_TIPC)		+= tipc/
net/d80211/Kconfig +1 -0
@@ -3,6 +3,7 @@
 	select CRYPTO
 	select CRYPTO_ARC4
 	select CRYPTO_AES
+	select NETLINK_80211
 	---help---
 	This option enables the hardware independent IEEE 802.11
 	networking stack.
net/wireless/Makefile new file +4 -0
@@ -0,0 +1,4 @@
+obj-$(CONFIG_NETLINK_80211) += cfg80211.o
+
+cfg80211-objs := \
+	nl80211.o
net/wireless/nl80211.c new file +209 -0

209 lines not on this page · raw message

include/linux/Kbuild +1 -1
@@ -28,7 +28,7 @@
 	sound.h stddef.h synclink.h telephony.h termios.h ticable.h	\
 	times.h tiocl.h tipc.h toshiba.h ultrasound.h un.h utime.h	\
 	utsname.h video_decoder.h video_encoder.h videotext.h vt.h	\
-	wavefront.h wireless.h xattr.h x25.h zorro_ids.h
+	wavefront.h wireless.h xattr.h x25.h zorro_ids.h nl80211.h
 
 unifdef-y += acct.h adb.h adfs_fs.h agpgart.h apm_bios.h atalk.h	\
 	atmarp.h atmdev.h atm.h atm_tcp.h audit.h auto_fs.h binfmts.h	\
include/linux/nl80211.h new file +79 -0

79 lines not on this page · raw message

net/d80211/Makefile +2 -1
@@ -15,7 +15,8 @@
 	michael.o \
 	tkip.o \
 	aes_ccm.o \
-	wme.o
+	wme.o \
+	ieee80211_cfg.o
 
 ifeq ($(CONFIG_NET_SCHED),)
   80211-objs += fifo_qdisc.o
net/d80211/ieee80211.c +56 -18

173 lines not on this page · raw message

net/d80211/ieee80211_cfg.c new file +97 -0

97 lines not on this page · raw message

net/d80211/ieee80211_cfg.h new file +7 -0
@@ -0,0 +1,7 @@
+#ifndef __IEEE80211_CFG_H
+#define __IEEE80211_CFG_H
+
+extern void ieee80211_cfg_init(void);
+extern void ieee80211_cfg_exit(void);
+
+#endif /* __IEEE80211_CFG_H */
net/d80211/ieee80211_i.h +4 -6
@@ -151,12 +151,10 @@
 struct ieee80211_tx_packet_data {
 	int ifindex;
 	unsigned long jiffies;
-	unsigned int req_tx_status:1;
-	unsigned int do_not_encrypt:1;
-	unsigned int pkt_probe_resp:1;
-	unsigned int requeue:1;
-	unsigned int queue:4;
-	unsigned int mgmt_iface:1;
+/* we simply re-use NL80211_FLAG_* here */
+	unsigned int flags;
+	unsigned int queue;
+	unsigned int injected;
 };
 
 struct ieee80211_tx_stored_packet {
net/d80211/ieee80211_sta.c +5 -3
@@ -23,6 +23,7 @@
 #include <linux/if_arp.h>
 #include <linux/wireless.h>
 #include <linux/random.h>
+#include <linux/nl80211.h>
 #include <net/iw_handler.h>
 #include <asm/types.h>
 #include <asm/delay.h>
@@ -400,10 +401,11 @@
 	pkt_data = (struct ieee80211_tx_packet_data *) skb->cb;
 	memset(pkt_data, 0, sizeof(struct ieee80211_tx_packet_data));
 	pkt_data->ifindex = sdata->dev->ifindex;
-	pkt_data->mgmt_iface = (sdata->type == IEEE80211_IF_TYPE_MGMT);
-	pkt_data->do_not_encrypt = !encrypt;
+	pkt_data->injected = (sdata->type == IEEE80211_IF_TYPE_MGMT);
+	if (encrypt)
+		pkt_data->flags |= NL80211_FLAG_ENCRYPT;
 	if (probe_resp)
-		pkt_data->pkt_probe_resp = 1;
+		pkt_data->flags |= NL80211_FLAG_PROBE_RESPONSE;
 
 	dev_queue_xmit(skb);
 }
net/d80211/wme.c +4 -2
@@ -12,6 +12,7 @@
 #include <linux/skbuff.h>
 #include <linux/module.h>
 #include <linux/if_arp.h>
+#include <linux/nl80211.h>
 #include <net/ip.h>
 
 #include <net/d80211.h>
@@ -190,7 +191,8 @@
 		return IEEE80211_TX_QUEUE_DATA0;
 	}
 
-	if (unlikely(pkt_data->mgmt_iface)) {
+	/* FIXME: this needs to be revisited for more generic injection */
+	if (unlikely(pkt_data->injected)) {
 		/* Data frames from hostapd (mainly, EAPOL) use AC_VO
 		* and they will include QoS control fields if
 		* the target STA is using WME. */
@@ -236,7 +238,7 @@
 	struct Qdisc *qdisc;
 	int err, queue;
 
-	if (pkt_data->requeue) {
+	if (pkt_data->flags & NL80211_FLAG_REQUEUE) {
 		skb_queue_tail(&q->requeued[pkt_data->queue], skb);
 		return 0;
 	}
author maintainer
#1 lore raw

RFC 1/1 nl80211 + packet injection with it and d80211

Here's a new version with a cleaned up interface and methods for each command instead of multiplexing the commands right away. This cuts down code in nl80211 users because attributes are already checked and unpacked etc.

Packet injection also still works :P

johannes

include/net/nl80211.h new file +62 -0
@@ -0,0 +1,62 @@
+#ifndef __NET_NL80211_H
+#define __NET_NL80211_H
+
+#include <linux/netlink.h>
+#include <net/genetlink.h>
+#include <linux/nl80211.h>
+#include <linux/skbuff.h>
+
+/*
+ * 802.11 netlink in-kernel interface
+ *
+ * Copyright 2006 Johannes Berg <[email protected]>
+ */
+
+/**
+ * struct nl80211_driver - backend description for wireless configuration
+ *
+ * This struct is registered by fullmac card drivers and/or wireless stacks
+ * in order to handle configuration requests on their interfaces.
+ *
+ * @get_priv_for_ifindex: return the private pointer that is then passed to
+ *			  the other calls as the first argument.
+ *			  Note that if this returns %NULL, then it is assumed
+ *			  that the driver doesn't handle that interface,
+ *			  hence priv is unused for some reason it still needs
+ *			  to return a non-%NULL pointer!
+ *
+ * @release_priv: release the private pointer if necessary. Note that for
+ *		  example if you used the ifindex and dev_get_by_index(),
+ *		  you have to dev_put() here.
+ *
+ * @inject_packet: inject the given frame with the NL80211_FLAG_*
+ *		   flags onto the given queue.
+ */
+struct nl80211_driver {
+	void*	(*get_priv_for_ifindex)(int ifindex);
+	void	(*release_priv)(void *priv);
+
+	int	(*inject_packet)(void *priv, void *frame, int framelen,
+				 u32 flags, int queue);
+
+	/* more things to be added...
+	 *
+	 * for a (*configure)(...) call I'd probably guess that the
+	 * best bet would be to have one call that returns all
+	 * possible options, one that sets them based on the
+	 * struct genl_info *info, and one for that optimised
+	 * set-at-once thing.
+	 */
+
+	/* private: */
+	struct list_head driver_list;
+};
+
+extern void nl80211_register_driver(struct nl80211_driver *drv);
+extern void nl80211_unregister_driver(struct nl80211_driver *drv);
+extern void *nl80211hdr_put(struct sk_buff *skb, u32 pid,
+			    u32 seq, int flags, u8 cmd);
+extern void *nl80211msg_new(struct sk_buff **skb, u32 pid,
+			    u32 seq, int flags, u8 cmd);
+
+#endif /* __NET_NL80211_H */
net/Kconfig +3 -0
@@ -250,6 +250,9 @@
 config WIRELESS_EXT
 	bool
 
+config NETLINK_80211
+	tristate
+
 endif   # if NET
 endmenu # Networking
 
net/Makefile +1 -0
@@ -44,6 +44,7 @@
 obj-$(CONFIG_VLAN_8021Q)	+= 8021q/
 obj-$(CONFIG_IP_DCCP)		+= dccp/
 obj-$(CONFIG_IP_SCTP)		+= sctp/
+obj-$(CONFIG_NETLINK_80211)	+= wireless/
 obj-$(CONFIG_D80211)		+= d80211/
 obj-$(CONFIG_IEEE80211)		+= ieee80211/
 obj-$(CONFIG_TIPC)		+= tipc/
net/d80211/Kconfig +1 -0
@@ -3,6 +3,7 @@
 	select CRYPTO
 	select CRYPTO_ARC4
 	select CRYPTO_AES
+	select NETLINK_80211
 	---help---
 	This option enables the hardware independent IEEE 802.11
 	networking stack.
net/wireless/Makefile new file +4 -0
@@ -0,0 +1,4 @@
+obj-$(CONFIG_NETLINK_80211) += cfg80211.o
+
+cfg80211-objs := \
+	nl80211.o
net/wireless/nl80211.c new file +240 -0

240 lines not on this page · raw message

include/linux/Kbuild +1 -1
@@ -28,7 +28,7 @@
 	sound.h stddef.h synclink.h telephony.h termios.h ticable.h	\
 	times.h tiocl.h tipc.h toshiba.h ultrasound.h un.h utime.h	\
 	utsname.h video_decoder.h video_encoder.h videotext.h vt.h	\
-	wavefront.h wireless.h xattr.h x25.h zorro_ids.h
+	wavefront.h wireless.h xattr.h x25.h zorro_ids.h nl80211.h
 
 unifdef-y += acct.h adb.h adfs_fs.h agpgart.h apm_bios.h atalk.h	\
 	atmarp.h atmdev.h atm.h atm_tcp.h audit.h auto_fs.h binfmts.h	\
include/linux/nl80211.h new file +70 -0

70 lines not on this page · raw message

net/d80211/Makefile +2 -1
@@ -15,7 +15,8 @@
 	michael.o \
 	tkip.o \
 	aes_ccm.o \
-	wme.o
+	wme.o \
+	ieee80211_cfg.o
 
 ifeq ($(CONFIG_NET_SCHED),)
   80211-objs += fifo_qdisc.o
net/d80211/ieee80211.c +60 -18

177 lines not on this page · raw message

net/d80211/ieee80211_cfg.c new file +78 -0

78 lines not on this page · raw message

net/d80211/ieee80211_cfg.h new file +7 -0
@@ -0,0 +1,7 @@
+#ifndef __IEEE80211_CFG_H
+#define __IEEE80211_CFG_H
+
+extern void ieee80211_cfg_init(void);
+extern void ieee80211_cfg_exit(void);
+
+#endif /* __IEEE80211_CFG_H */
net/d80211/ieee80211_i.h +7 -6
@@ -151,12 +151,13 @@
 struct ieee80211_tx_packet_data {
 	int ifindex;
 	unsigned long jiffies;
-	unsigned int req_tx_status:1;
-	unsigned int do_not_encrypt:1;
-	unsigned int pkt_probe_resp:1;
-	unsigned int requeue:1;
-	unsigned int queue:4;
-	unsigned int mgmt_iface:1;
+/* we simply re-use NL80211_FLAG_* here */
+	unsigned int flags;
+	unsigned int queue;
+#define TX_FLAG_INJECTED	(1<<0)
+#define TX_FLAG_REQUEUE		(1<<1)
+#define TX_FLAG_PROBERESP	(1<<2)
+	unsigned int internal_flags;
 };
 
 struct ieee80211_tx_stored_packet {
net/d80211/ieee80211_sta.c +6 -3
@@ -23,6 +23,7 @@
 #include <linux/if_arp.h>
 #include <linux/wireless.h>
 #include <linux/random.h>
+#include <linux/nl80211.h>
 #include <net/iw_handler.h>
 #include <asm/types.h>
 #include <asm/delay.h>
@@ -400,10 +401,12 @@
 	pkt_data = (struct ieee80211_tx_packet_data *) skb->cb;
 	memset(pkt_data, 0, sizeof(struct ieee80211_tx_packet_data));
 	pkt_data->ifindex = sdata->dev->ifindex;
-	pkt_data->mgmt_iface = (sdata->type == IEEE80211_IF_TYPE_MGMT);
-	pkt_data->do_not_encrypt = !encrypt;
+	if (sdata->type == IEEE80211_IF_TYPE_MGMT)
+		pkt_data->internal_flags |= TX_FLAG_INJECTED;
+	if (encrypt)
+		pkt_data->flags |= NL80211_FLAG_ENCRYPT;
 	if (probe_resp)
-		pkt_data->pkt_probe_resp = 1;
+		pkt_data->internal_flags |= TX_FLAG_PROBERESP;
 
 	dev_queue_xmit(skb);
 }
net/d80211/wme.c +4 -2
@@ -12,6 +12,7 @@
 #include <linux/skbuff.h>
 #include <linux/module.h>
 #include <linux/if_arp.h>
+#include <linux/nl80211.h>
 #include <net/ip.h>
 
 #include <net/d80211.h>
@@ -190,7 +191,8 @@
 		return IEEE80211_TX_QUEUE_DATA0;
 	}
 
-	if (unlikely(pkt_data->mgmt_iface)) {
+	/* FIXME: this needs to be revisited for more generic injection */
+	if (unlikely(pkt_data->internal_flags & TX_FLAG_INJECTED)) {
 		/* Data frames from hostapd (mainly, EAPOL) use AC_VO
 		* and they will include QoS control fields if
 		* the target STA is using WME. */
@@ -236,7 +238,7 @@
 	struct Qdisc *qdisc;
 	int err, queue;
 
-	if (pkt_data->requeue) {
+	if (pkt_data->internal_flags & TX_FLAG_REQUEUE) {
 		skb_queue_tail(&q->requeued[pkt_data->queue], skb);
 		return 0;
 	}
#2 lore raw

Johannes,

thanks a lot for your work on nl80211! New netlink interface is one of the most important things regarding 802.11 stack now.

Quoted 13 lines · Johannes Berg

[...]
+/**
+ * struct nl80211_driver - backend description for wireless configuration
+ *
+ * This struct is registered by fullmac card drivers and/or wireless stacks
+ * in order to handle configuration requests on their interfaces.
+ *
+ * @get_priv_for_ifindex: return the private pointer that is then passed to

+ *			  the other calls as the first argument.
+ *			  Note that if this returns %NULL, then it is assumed
+ *			  that the driver doesn't handle that interface,
+ *			  hence priv is unused for some reason it still needs
+ *			  to return a non-%NULL pointer!
Quoted 4 lines

+ *
+ * @release_priv: release the private pointer if necessary. Note that for
+ * example if you used the ifindex and dev_get_by_index(),
+ * you have to dev_put() here.

Seems ineffective to me. Couldn't you require users of nl80211 to fill ieee80211_ptr field in all of their net_devices and use that to find out owner of the device? Then you don't need to call into every registered nl80211 user for each message.

include/net/nl80211.h line 37

Quoted 7 lines

+ *
+ * @inject_packet: inject the given frame with the NL80211_FLAG_*
+ * flags onto the given queue.
+ */

include/net/nl80211.h new file +3 -0
+struct nl80211_driver {
+	void*	(*get_priv_for_ifindex)(int ifindex);
+	void	(*release_priv)(void *priv);

Maybe put_priv would be better name for release_priv?

Quoted 2 lines

[...]
+static int nl80211_drvpriv_by_ifidx(int ifindex, void **priv, struct nl80211_driver **drv)

Cut long lines, please.

net/wireless/nl80211.c line 90

Quoted 6 lines

[...]

net/wireless/nl80211.c new file +4 -0
+static int nl80211_get_commands(struct sk_buff *skb, struct genl_info *info)
[...]
+
+	/* unconditionally allow NL80211_CMD_GET_COMMANDS */
+	skb_put(skb, 1);

Shouldn't this be skb_put(msg, 1)? And in several other cases below too?

net/wireless/nl80211.c line 97

Quoted 7 lines
net/wireless/nl80211.c new file +7 -0
+	*data++ = NL80211_CMD_GET_COMMANDS;
+
+#define CHECK_CMD(ptr, cmd)			\
+	if (drv->ptr) {				\
+		skb_put(skb, 1);		\
+		*data++ = NL80211_CMD_##cmd;	\
+	}

Could you move this define out of the function?

Quoted 9 lines
net/wireless/nl80211.c new file +9 -0
+
+	CHECK_CMD(inject_packet, INJECT);
+
+	nla_nest_end(msg, start);
+
+	err = genlmsg_end(msg, hdr);
+	if (err)
+		goto msg_free;
+

net/wireless/nl80211.c line 116

Quoted 10 lines
net/wireless/nl80211.c new file +10 -0
+	err = genlmsg_unicast(msg, info->snd_pid);
+	goto release;
+
+ nla_put_failure:
+	err = genlmsg_cancel(skb, hdr);
+ msg_free:
+	nlmsg_free(skb);
+ release:
+	if (drv->release_priv)
+		drv->release_priv(priv);

If get_priv_for_ifindex turns out to be really necessary, please rename nl80211_drvpriv_from_info to nl80211_get_drvpriv_from_info and introduce a new nl80211_put_drvpriv function (so it's immediately clear that returned pointers need some releasing):

void nl80211_put_drvpriv(struct nl80211_driver *drv, void *priv)

{
        if (drv->release_priv)
                drv->release_priv(priv);
}

And rename nl80211_drvpriv_by_ifidx to nl80211_get_drvpriv_by_ifidx, too.

net/wireless/nl80211.c line 175

Quoted 7 lines

[...]

net/wireless/nl80211.c new file +6 -0
+void nl80211_register_driver(struct nl80211_driver *drv)
+{
+	mutex_lock(&nl80211_drv_mutex);
+	list_add(&drv->driver_list, &nl80211_drv_list);
+	mutex_unlock(&nl80211_drv_mutex);
+}

Don't modify passed nl80211_driver. This way drivers cannot pass a static structure. Allocate you own structure and store a pointer to passed nl80211_driver there (or copy it into that your structure if you don't like dereferencing two pointers when calling callbacks).

net/wireless/nl80211.c line 192

Quoted 8 lines

[...]

net/wireless/nl80211.c new file +7 -0
+void *nl80211hdr_put(struct sk_buff *skb, u32 pid, u32 seq, int flags, u8 cmd)
+{
+	/* since there is no private header just add the generic one */
+	return genlmsg_put(skb, pid, seq, nl80211_fam.id, 0,
+			   flags, cmd, nl80211_fam.version);
+}
+EXPORT_SYMBOL_GPL(nl80211hdr_put);

Why is this exported? It should be used by nl80211msg_new only, shouldn't it?

include/linux/nl80211.h line 39

Quoted 8 lines

[...]

include/linux/nl80211.h new file +7 -0
+/* currently supported attributes.
+ * don't change the order or add anything inbetween, this is ABI! */
+enum {
+	NL80211_ATTR_UNSPEC,
+
+	/* network device (ifindex) to operate on */
+	NL80211_ATTR_IFINDEX,

Currently, master device is represented by a special net_device in d80211. It's not nice, "wiphy" device should be represented in some other way. Although I don't expect this would change (at least not in a foreseeable future) as it would require rewriting of great part of networking core, we should be prepared for it, so we would not be forced to change ABI or inventing workarounds if that eventually would happen.

Also, d80211 allows some operations even without having any network interface present (of course, master interface is still present, but it's not nice to use its ifindex, especially in the light of previous paragraph).

I propose this:
- add NL80211_ATTR_WIPHY_INDEX attributte
- add wiphy_index field to nl80211_driver and require non-d80211 users to set it to -1
- allow NL80211_ATTR_IFINDEX to be optional in some calls - the exact list of these calls is independent of any driver and can be hardcoded in nl80211
- in such calls, specifying either NL80211_ATTR_IFINDEX or NL80211_ATTR_WIPHY_INDEX is enough; of course, when corresponding driver does not support wiphys (i.e. nl80211_driver.wiphy_index is -1), only NL80211_ATTR_IFINDEX can be specified
- in other calls, NL80211_ATTR_WIPHY_INDEX should not be present

Alternatively, you could introduce a flag to denote if NL80211_ATTR_IFINDEX field contains ifindex or wiphy index.

Comments to the d80211 part later (probably on Tuesday).

Thanks,

Jiri

Signature · 2 lines
Jiri Benc
SUSE Labs
author maintainer
#3 lore raw

Jiri,

Quoted 2 lines

thanks a lot for your work on nl80211! New netlink interface is one of the most important things regarding 802.11 stack now.

Wait till you see my weekend patch series ;)

Quoted 4 lines

Seems ineffective to me. Couldn't you require users of nl80211 to fill ieee80211_ptr field in all of their net_devices and use that to find out owner of the device? Then you don't need to call into every registered nl80211 user for each message.

What about non-d80211 fullmac drivers?

Quoted 1 line

Maybe put_priv would be better name for release_priv?

Yeah, ok.

Quoted 4 lines
Quoted 2 lines

[...]
+static int nl80211_drvpriv_by_ifidx(int ifindex, void **priv, struct nl80211_driver **drv)

Cut long lines, please.

Oops.

Quoted 4 lines

net/wireless/nl80211.c line 90

Quoted 2 lines
net/wireless/nl80211.c new file +2 -0
+	/* unconditionally allow NL80211_CMD_GET_COMMANDS */
+	skb_put(skb, 1);

Shouldn't this be skb_put(msg, 1)? And in several other cases below too?

Woah, yes. I really should've tested that code as well.

Quoted 7 lines

net/wireless/nl80211.c line 97

Quoted 5 lines
net/wireless/nl80211.c new file +5 -0
+#define CHECK_CMD(ptr, cmd)			\
+	if (drv->ptr) {				\
+		skb_put(skb, 1);		\
+		*data++ = NL80211_CMD_##cmd;	\
+	}

Could you move this define out of the function?

Sure. I'll also undef it afterwards.

Quoted 7 lines

void nl80211_put_drvpriv(struct nl80211_driver *drv, void *priv)

{
        if (drv->release_priv)
                drv->release_priv(priv);
}

And rename nl80211_drvpriv_by_ifidx to nl80211_get_drvpriv_by_ifidx, too.

Sure.

Quoted 4 lines

Don't modify passed nl80211_driver. This way drivers cannot pass a static structure. Allocate you own structure and store a pointer to passed nl80211_driver there (or copy it into that your structure if you don't like dereferencing two pointers when calling callbacks).

Hmm, ok. I don't really like that too much either, but yeah, probably a good idea. OTOH, if we make a copy anyway we could just as well require drivers to make a copy, no? (And allocating just 12 bytes for three pointers for the linked list feels stupid)

Quoted 11 lines

net/wireless/nl80211.c line 192

Quoted 8 lines

[...]

net/wireless/nl80211.c new file +7 -0
+void *nl80211hdr_put(struct sk_buff *skb, u32 pid, u32 seq, int flags, u8 cmd)
+{
+	/* since there is no private header just add the generic one */
+	return genlmsg_put(skb, pid, seq, nl80211_fam.id, 0,
+			   flags, cmd, nl80211_fam.version);
+}
+EXPORT_SYMBOL_GPL(nl80211hdr_put);

Why is this exported? It should be used by nl80211msg_new only, shouldn't it?

No, for dumpit() callbacks you need this. Not sure if we'll ever have any of those, but...

Quoted 7 lines

Currently, master device is represented by a special net_device in d80211. It's not nice, "wiphy" device should be represented in some other way. Although I don't expect this would change (at least not in a foreseeable future) as it would require rewriting of great part of networking core, we should be prepared for it, so we would not be forced to change ABI or inventing workarounds if that eventually would happen.

Quoted 6 lines

Also, d80211 allows some operations even without having any network interface present (of course, master interface is still present, but it's not nice to use its ifindex, especially in the light of previous paragraph).

I propose this:
- add NL80211_ATTR_WIPHY_INDEX attributte

We can do that at any time we want.

Quoted 2 lines

- add wiphy_index field to nl80211_driver and require non-d80211 users to set it to -1

Woah, wait. Let's do it the other way around then, we give them a wiphy index. That way, we have a central registry for it here in nl80211.

Quoted 3 lines

- allow NL80211_ATTR_IFINDEX to be optional in some calls - the exact list of these calls is independent of any driver and can be hardcoded in nl80211

That's what I pretty much do already.

Quoted 8 lines

- in such calls, specifying either NL80211_ATTR_IFINDEX or NL80211_ATTR_WIPHY_INDEX is enough; of course, when corresponding driver does not support wiphys (i.e. nl80211_driver.wiphy_index is -1), only NL80211_ATTR_IFINDEX can be specified
- in other calls, NL80211_ATTR_WIPHY_INDEX should not be present

Alternatively, you could introduce a flag to denote if NL80211_ATTR_IFINDEX field contains ifindex or wiphy index.

Ick, no flag. Let's just have two separate attributes, and on some calls either one or both are valid.

johannes

author maintainer
#4 lore raw
In reply to Jiri Benc

Here's a respun patch hopefully addressing most of Jiri's comments, and allowing to add/remove virtual interfaces via netlink as well.

I introduced a new notion of 'wiphy' which is just a number handed out by nl80211 for each registered backend, and I also changed it so that each backend must register once for each physical device.

Also, drivers wishing to use this to full capacity are required to fill in ieee80211_ptr in struct net_device for all of their devices, otherwise the wiphy number is useless!

NB: The new notion of wiphy will be confusing because now nl80211 and d80211 can hand out different numbers. I expect that either d80211 will just use the wiphy number nl80211 handed out (will need new API to query it) or we rename things or whatever. To be shaken out :)

Oh, I also expect there to be a configfs/sysfs interface to nl80211 directly so that backends (d80211...) don't create their sysfs/configfs dirs but use debugfs for truly not too interesting things like debug counters, and all the other stuff is in nl80211. Or something like that anyway :)

With this patch, NL80211_ATTR_IFINDEX of any netdevice attached to a wiphy can be used in place of NL80211_ATTR_WIPHY, but some calls like adding a virtual device to a wiphy that has no netdev (something that cannot happen at the moment due to wmaster) require ATTR_WIPHY for obvious reasons. Oh and I figured that the list walking is maybe dumb, but we will never have thousands of wiphys in a single machine...

Anyway, with this patch we ought to be able to get rid of wmaster0 without userspace visible changes (except of course that doing
wconf dev add xxx0 dev wmaster0
will no longer work and you'll need to do
wconf dev add xxx0 wiphy 0
or something :)

Hrm, I keep forgetting things :) Another thing that I just remembered:
This relies on the patch by Thomas Graf he posted in
http://marc.theaimsgroup.com/?l=linux-netdev&m=115218779631578&w=2 for validation of the netdevice name length/null termination.

Patch below. NB: only compiled, not tried this time, want to go home :)

johannes

include/net/nl80211.h new file +74 -0
@@ -0,0 +1,74 @@
+#ifndef __NET_NL80211_H
+#define __NET_NL80211_H
+
+#include <linux/netlink.h>
+#include <linux/nl80211.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <net/genetlink.h>
+
+/*
+ * 802.11 netlink in-kernel interface
+ *
+ * Copyright 2006 Johannes Berg <[email protected]>
+ */
+
+/**
+ * struct nl80211_ops - backend description for wireless configuration
+ *
+ * This struct is registered by fullmac card drivers and/or wireless stacks
+ * in order to handle configuration requests on their interfaces.
+ *
+ * The priv pointer passed to each call is the pointer that was
+ * registered in nl80211_register_driver().
+ *
+ * All callbacks except where otherwise noted should return 0
+ * on success or a negative error code.
+ *
+ * @inject_packet: inject the given frame with the NL80211_FLAG_*
+ *		   flags onto the given queue.
+ *
+ * @add_virtual_intf: create a new virtual interface with the given name
+ *
+ * @del_virtual_intf: remove the given virtual interface (during the call,
+ *                    one reference to the interface is held)
+ */
+struct nl80211_ops {
+	int	(*inject_packet)(void *priv, void *frame, int framelen,
+				 u32 flags, int queue);
+
+	int	(*add_virtual_intf)(void *priv, char *name);
+	int	(*del_virtual_intf)(void *priv, struct net_device *dev);
+
+	/* more things to be added...
+	 *
+	 * for a (*configure)(...) call I'd probably guess that the
+	 * best bet would be to have one call that returns all
+	 * possible options, one that sets them based on the
+	 * struct genl_info *info, and one for that optimised
+	 * set-at-once thing.
+	 */
+};
+
+/*
+ * register a given method structure with the nl80211 system
+ * and associate the 'priv' pointer with it.
+ * NOTE: for proper operation, this priv pointer MUST also be
+ * assigned to each &struct net_device's @ieee80211_ptr member!
+ */
+extern int nl80211_register(struct nl80211_ops *ops, void *priv);
+/*
+ * unregister a device with the given priv pointer.
+ * After this call, no more requests can be made with this priv
+ * pointer, but the call may sleep to wait for an outstanding
+ * request that is being handled.
+ */
+extern void nl80211_unregister(void *priv);
+
+/* helper functions */
+extern void *nl80211hdr_put(struct sk_buff *skb, u32 pid,
+			    u32 seq, int flags, u8 cmd);
+extern void *nl80211msg_new(struct sk_buff **skb, u32 pid,
+			    u32 seq, int flags, u8 cmd);
+
+#endif /* __NET_NL80211_H */
net/Kconfig +3 -0
@@ -250,6 +250,9 @@
 config WIRELESS_EXT
 	bool
 
+config NETLINK_80211
+	tristate
+
 endif   # if NET
 endmenu # Networking
 
net/Makefile +1 -0
@@ -44,6 +44,7 @@
 obj-$(CONFIG_VLAN_8021Q)	+= 8021q/
 obj-$(CONFIG_IP_DCCP)		+= dccp/
 obj-$(CONFIG_IP_SCTP)		+= sctp/
+obj-$(CONFIG_NETLINK_80211)	+= wireless/
 obj-$(CONFIG_D80211)		+= d80211/
 obj-$(CONFIG_IEEE80211)		+= ieee80211/
 obj-$(CONFIG_TIPC)		+= tipc/
net/d80211/Kconfig +1 -0
@@ -3,6 +3,7 @@
 	select CRYPTO
 	select CRYPTO_ARC4
 	select CRYPTO_AES
+	select NETLINK_80211
 	---help---
 	This option enables the hardware independent IEEE 802.11
 	networking stack.
net/wireless/Makefile new file +4 -0
@@ -0,0 +1,4 @@
+obj-$(CONFIG_NETLINK_80211) += cfg80211.o
+
+cfg80211-objs := \
+	nl80211.o
net/wireless/nl80211.c new file +447 -0

447 lines not on this page · raw message

include/linux/Kbuild +1 -1
@@ -28,7 +28,7 @@
 	sound.h stddef.h synclink.h telephony.h termios.h ticable.h	\
 	times.h tiocl.h tipc.h toshiba.h ultrasound.h un.h utime.h	\
 	utsname.h video_decoder.h video_encoder.h videotext.h vt.h	\
-	wavefront.h wireless.h xattr.h x25.h zorro_ids.h
+	wavefront.h wireless.h xattr.h x25.h zorro_ids.h nl80211.h
 
 unifdef-y += acct.h adb.h adfs_fs.h agpgart.h apm_bios.h atalk.h	\
 	atmarp.h atmdev.h atm.h atm_tcp.h audit.h auto_fs.h binfmts.h	\
include/linux/nl80211.h new file +84 -0

84 lines not on this page · raw message

net/d80211/Makefile +2 -1
@@ -15,7 +15,8 @@
 	michael.o \
 	tkip.o \
 	aes_ccm.o \
-	wme.o
+	wme.o \
+	ieee80211_cfg.o
 
 ifeq ($(CONFIG_NET_SCHED),)
   80211-objs += fifo_qdisc.o
net/d80211/ieee80211.c +65 -18

194 lines not on this page · raw message

net/d80211/ieee80211_cfg.c new file +97 -0

97 lines not on this page · raw message

net/d80211/ieee80211_cfg.h new file +9 -0
@@ -0,0 +1,9 @@
+#ifndef __IEEE80211_CFG_H
+#define __IEEE80211_CFG_H
+
+#include "ieee80211_i.h"
+
+extern int ieee80211_cfg_init(struct ieee80211_local *local);
+extern void ieee80211_cfg_exit(struct ieee80211_local *local);
+
+#endif /* __IEEE80211_CFG_H */
net/d80211/ieee80211_i.h +7 -6
@@ -151,12 +151,13 @@
 struct ieee80211_tx_packet_data {
 	int ifindex;
 	unsigned long jiffies;
-	unsigned int req_tx_status:1;
-	unsigned int do_not_encrypt:1;
-	unsigned int pkt_probe_resp:1;
-	unsigned int requeue:1;
-	unsigned int queue:4;
-	unsigned int mgmt_iface:1;
+/* we simply re-use NL80211_FLAG_* here */
+	unsigned int flags;
+	unsigned int queue;
+#define TX_FLAG_INJECTED	(1<<0)
+#define TX_FLAG_REQUEUE		(1<<1)
+#define TX_FLAG_PROBERESP	(1<<2)
+	unsigned int internal_flags;
 };
 
 struct ieee80211_tx_stored_packet {
net/d80211/ieee80211_sta.c +6 -3
@@ -23,6 +23,7 @@
 #include <linux/if_arp.h>
 #include <linux/wireless.h>
 #include <linux/random.h>
+#include <linux/nl80211.h>
 #include <net/iw_handler.h>
 #include <asm/types.h>
 #include <asm/delay.h>
@@ -400,10 +401,12 @@
 	pkt_data = (struct ieee80211_tx_packet_data *) skb->cb;
 	memset(pkt_data, 0, sizeof(struct ieee80211_tx_packet_data));
 	pkt_data->ifindex = sdata->dev->ifindex;
-	pkt_data->mgmt_iface = (sdata->type == IEEE80211_IF_TYPE_MGMT);
-	pkt_data->do_not_encrypt = !encrypt;
+	if (sdata->type == IEEE80211_IF_TYPE_MGMT)
+		pkt_data->internal_flags |= TX_FLAG_INJECTED;
+	if (encrypt)
+		pkt_data->flags |= NL80211_FLAG_ENCRYPT;
 	if (probe_resp)
-		pkt_data->pkt_probe_resp = 1;
+		pkt_data->internal_flags |= TX_FLAG_PROBERESP;
 
 	dev_queue_xmit(skb);
 }
net/d80211/wme.c +4 -2
@@ -12,6 +12,7 @@
 #include <linux/skbuff.h>
 #include <linux/module.h>
 #include <linux/if_arp.h>
+#include <linux/nl80211.h>
 #include <net/ip.h>
 
 #include <net/d80211.h>
@@ -190,7 +191,8 @@
 		return IEEE80211_TX_QUEUE_DATA0;
 	}
 
-	if (unlikely(pkt_data->mgmt_iface)) {
+	/* FIXME: this needs to be revisited for more generic injection */
+	if (unlikely(pkt_data->internal_flags & TX_FLAG_INJECTED)) {
 		/* Data frames from hostapd (mainly, EAPOL) use AC_VO
 		* and they will include QoS control fields if
 		* the target STA is using WME. */
@@ -236,7 +238,7 @@
 	struct Qdisc *qdisc;
 	int err, queue;
 
-	if (pkt_data->requeue) {
+	if (pkt_data->internal_flags & TX_FLAG_REQUEUE) {
 		skb_queue_tail(&q->requeued[pkt_data->queue], skb);
 		return 0;
 	}
author maintainer
#5 lore raw
In reply to Johannes Berg

Another thing that crossed my mind: Should we have a netlink call for adding virtual interfaces as well? Because then, the ipw2x00 drivers can have their monitor interface(s) added the same way.

johannes

#6 lore raw
Quoted 3 lines · Johannes Berg

Another thing that crossed my mind: Should we have a netlink call for adding virtual interfaces as well? Because then, the ipw2x00 drivers can have their monitor interface(s) added the same way.

Yes.

Signature · 2 lines
John W. Linville
[email protected]