Skip to contentSkip to conversation

invisible network devices

RFC PATCH In review Jiri Benc 8 participants 22 messages Jan 29, 2007 – Feb 21, 2007

Subsystems (2)
lore
Share & tools
lore

Jiri Benc

author
#0 lore raw

RFC PATCH 1/6 invisible network devices

The d80211 stack needs a network interface (called 'wmaster') used for communication with the hardware (it has 802.11 qdisc attached which perform MAC level QoS). This interface is not intended for users and it confuses them.

As a short time solution, this patch allows net_device to be registered as "invisible". This means it is not in the dev name hash list, its ifindex is -1 and protocols are not notified about its registration/unregistration.

---
 include/linux/netdevice.h |    2 
 net/core/dev.c            |  223 ++++++++++++++++++++++++++++++----------------
 2 files changed, 151 insertions(+), 74 deletions(-)
include/linux/netdevice.h +2 -0
@@ -975,6 +975,8 @@ extern struct net_device *alloc_netdev(i
 				       void (*setup)(struct net_device *));
 extern int		register_netdev(struct net_device *dev);
 extern void		unregister_netdev(struct net_device *dev);
+extern int		register_invisible_netdevice(struct net_device *dev);
+extern void		unregister_invisible_netdevice(struct net_device *dev);
 /* Functions used for multicast support */
 extern void		dev_mc_upload(struct net_device *dev);
 extern int 		dev_mc_delete(struct net_device *dev, void *addr, int alen, int all);
net/core/dev.c +149 -74

294 lines not on this page · raw message

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

PATCH 2/6 d80211: use invisible network device for wmaster

Use register_invisible_netdevice for master interface.

---
 net/d80211/ieee80211.c       |   22 ++--------------------
 net/d80211/ieee80211_iface.c |    8 ++++++--
 2 files changed, 8 insertions(+), 22 deletions(-)
net/d80211/ieee80211.c +2 -20
@@ -4540,23 +4540,11 @@ int ieee80211_register_hw(struct ieee802
 	memcpy(local->mdev->dev_addr, local->hw.perm_addr, ETH_ALEN);
 	SET_NETDEV_DEV(local->mdev, local->hw.dev);
 
-	result = register_netdevice(local->mdev);
+	result = register_invisible_netdevice(local->mdev);
 	if (result < 0) {
 		rtnl_unlock();
 		goto fail_dev;
 	}
-	result = sysfs_create_link(&local->class_dev.kobj,
-				   &local->mdev->class_dev.kobj,
-				   "master");
-	if (result < 0) {
-		rtnl_unlock();
-		goto fail_masterlink;
-	}
-	result = ieee80211_sysfs_add_netdevice(local->mdev);
-	if (result < 0) {
-		rtnl_unlock();
-		goto fail_if_sysfs;
-	}
 
 	result = ieee80211_init_rate_ctrl_alg(local, NULL);
 	rtnl_unlock();
@@ -4593,11 +4581,7 @@ int ieee80211_register_hw(struct ieee802
 fail_wep:
 	rate_control_deinitialize(local);
 fail_rate:
-	ieee80211_sysfs_remove_netdevice(local->mdev);
-fail_if_sysfs:
-	sysfs_remove_link(&local->class_dev.kobj, "master");
-fail_masterlink:
-	unregister_netdev(local->mdev);
+	unregister_invisible_netdevice(local->mdev);
 fail_dev:
 	sta_info_stop(local);
 fail_sta_info:
@@ -4655,8 +4639,6 @@ void ieee80211_unregister_hw(struct ieee
 	if (local->apdev)
 		ieee80211_if_del_mgmt(local);
 
-	sysfs_remove_link(&local->class_dev.kobj, "master");
-
 	list_for_each_entry_safe(sdata, tmp, &local->sub_if_list, list)
 		__ieee80211_if_del(local, sdata);
 
net/d80211/ieee80211_iface.c +6 -2
@@ -309,8 +309,12 @@ void __ieee80211_if_del(struct ieee80211
 	struct net_device *dev = sdata->dev;
 
 	list_del(&sdata->list);
-	ieee80211_sysfs_remove_netdevice(dev);
-	unregister_netdevice(dev);
+	if (dev == local->mdev)
+		unregister_invisible_netdevice(dev);
+	else {
+		ieee80211_sysfs_remove_netdevice(dev);
+		unregister_netdevice(dev);
+	}
 	/* Except master interface, the net_device will be freed by
 	 * net_device->destructor (i. e. ieee80211_if_free). */
 }
author
#2 lore raw

PATCH 3/6 d80211: drop packets from nonexisting interfaces in PS mode

In reply to Jiri Benc

In a power saving mode, packets queued by devices that meanwhile disappeared has to be discarded.

---
 net/d80211/ieee80211.c |   46 +++++++++++++++++++++++++---------------------
 1 files changed, 25 insertions(+), 21 deletions(-)
net/d80211/ieee80211.c +25 -21
@@ -1104,10 +1104,10 @@ static int inline is_ieee80211_device(st
 
 /* Device in tx->dev has a reference added; use dev_put(tx->dev) when
  * finished with it. */
-static void inline ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
-					struct sk_buff *skb,
-					struct net_device *mdev,
-					struct ieee80211_tx_control *control)
+static int inline ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
+				       struct sk_buff *skb,
+				       struct net_device *mdev,
+				       struct ieee80211_tx_control *control)
 {
 	struct ieee80211_tx_packet_data *pkt_data;
 	struct net_device *dev;
@@ -1118,13 +1118,10 @@ static void inline ieee80211_tx_prepare(
 		dev_put(dev);
 		dev = NULL;
 	}
-	if (unlikely(!dev)) {
-		printk(KERN_WARNING "%s: NULL ifindex in pkt_data\n",
-		       mdev->name);
-		dev = mdev;
-		dev_hold(dev);
-	}
+	if (unlikely(!dev))
+		return -ENODEV;
 	__ieee80211_tx_prepare(tx, skb, dev, control);
+	return 0;
 }
 
 static inline int __ieee80211_queue_stopped(struct ieee80211_local *local,
@@ -1815,20 +1812,27 @@ ieee80211_get_buffered_bc(struct ieee802
 
 	if (bss->dtim_count != 0)
 		return NULL; /* send buffered bc/mc only after DTIM beacon */
-	skb = skb_dequeue(&bss->ps_bc_buf);
 	memset(control, 0, sizeof(*control));
-	if (!skb)
-		return NULL;
-	local->total_ps_buffered--;
+	while (1) {
+		skb = skb_dequeue(&bss->ps_bc_buf);
+		if (!skb)
+			return NULL;
+		local->total_ps_buffered--;
 
-	if (!skb_queue_empty(&bss->ps_bc_buf) && skb->len >= 2) {
-		struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
-		/* more buffered multicast/broadcast frames ==> set MoreData
-		 * flag in IEEE 802.11 header to inform PS STAs */
-		hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_MOREDATA);
-	}
+		if (!skb_queue_empty(&bss->ps_bc_buf) && skb->len >= 2) {
+			struct ieee80211_hdr *hdr =
+				(struct ieee80211_hdr *) skb->data;
+			/* more buffered multicast/broadcast frames ==> set
+			 * MoreData flag in IEEE 802.11 header to inform PS
+			 * STAs */
+			hdr->frame_control |=
+				cpu_to_le16(IEEE80211_FCTL_MOREDATA);
+		}
 
-	ieee80211_tx_prepare(&tx, skb, local->mdev, control);
+		if (ieee80211_tx_prepare(&tx, skb, local->mdev, control) == 0)
+			break;
+		dev_kfree_skb_any(skb);
+	}
 	sta = tx.sta;
 	tx.u.tx.ps_buffered = 1;
 
author
#3 lore raw

PATCH 4/6 d80211: don't display name of invisible network device

In reply to Jiri Benc

Invisible master interface does not have meaningful name. Display the wiphy identifier in kernel messages instead.

Also, remove the allocation of master interface name as it is purposeless now.

---
 net/d80211/ieee80211.c      |   98 +++++++++++++++++++++-----------------------
 net/d80211/ieee80211_scan.c |   28 ++++++------
 net/d80211/sta_info.c       |   12 ++---
 net/d80211/wme.c            |    8 ++-
 4 files changed, 74 insertions(+), 72 deletions(-)
net/d80211/ieee80211.c +48 -50
@@ -284,14 +284,14 @@ int ieee80211_get_hdrlen_from_skb(struct
 EXPORT_SYMBOL(ieee80211_get_hdrlen_from_skb);
 
 #ifdef CONFIG_D80211_LOWTX_FRAME_DUMP
-static void ieee80211_dump_frame(const char *ifname, const char *title,
+static void ieee80211_dump_frame(int hwindex, const char *title,
 				 struct sk_buff *skb)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	u16 fc;
 	int hdrlen;
 
-	printk(KERN_DEBUG "%s: %s (len=%d)", ifname, title, skb->len);
+	printk(KERN_DEBUG "wiphy%d: %s (len=%d)", hwindex, title, skb->len);
 	if (skb->len < 4) {
 		printk("\n");
 		return;
@@ -315,7 +315,7 @@ static void ieee80211_dump_frame(const c
 	printk("\n");
 }
 #else /* CONFIG_D80211_LOWTX_FRAME_DUMP */
-static inline void ieee80211_dump_frame(const char *ifname, const char *title,
+static inline void ieee80211_dump_frame(int hwindex, const char *title,
 					struct sk_buff *skb)
 {
 }
@@ -945,8 +945,8 @@ static void purge_old_ps_buffers(struct 
 	spin_unlock_bh(&local->sta_lock);
 
 	local->total_ps_buffered = total;
-	printk(KERN_DEBUG "%s: PS buffers full - purged %d frames\n",
-	       local->mdev->name, purged);
+	printk(KERN_DEBUG "wiphy%d: PS buffers full - purged %d frames\n",
+	       local->hw.index, purged);
 }
 
 
@@ -1147,7 +1147,7 @@ static int __ieee80211_tx(struct ieee802
 	int ret, i;
 
 	if (skb) {
-		ieee80211_dump_frame(local->mdev->name, "TX to low-level driver", skb);
+		ieee80211_dump_frame(local->hw.index, "TX to low-level driver", skb);
 		ret = local->ops->tx(local_to_hw(local), skb, control);
 		if (ret)
 			return IEEE80211_TX_AGAIN;
@@ -1174,7 +1174,7 @@ static int __ieee80211_tx(struct ieee802
 						~IEEE80211_TXCTL_RATE_CTRL_PROBE;
 			}
 
-			ieee80211_dump_frame(local->mdev->name,
+			ieee80211_dump_frame(local->hw.index,
 					     "TX to low-level driver", skb);
 			ret = local->ops->tx(local_to_hw(local),
 					    tx->u.tx.extra_frag[i],
@@ -1375,8 +1375,10 @@ static int ieee80211_master_start_xmit(s
 	}
 	if (unlikely(!odev)) {
 #ifdef CONFIG_D80211_VERBOSE_DEBUG
-		printk(KERN_DEBUG "%s: Discarded packet with nonexistent "
-		       "originating device\n", dev->name);
+		struct ieee80211_local *local = dev->ieee80211_ptr;
+
+		printk(KERN_DEBUG "wiphy%d: Discarded packet with nonexistent "
+		       "originating device\n", local->hw.index);
 #endif
 		dev_kfree_skb(skb);
 		return 0;
@@ -1764,8 +1766,8 @@ struct sk_buff * ieee80211_beacon_get(st
 		rate = rate_control_get_rate(local, local->mdev, skb, &extra);
 		if (!rate) {
 			if (net_ratelimit()) {
-				printk(KERN_DEBUG "%s: ieee80211_beacon_get: no rate "
-				       "found\n", local->mdev->name);
+				printk(KERN_DEBUG "wiphy%d: ieee80211_beacon_get: no rate "
+				       "found\n", local->hw.index);
 			}
 			dev_kfree_skb(skb);
 			return NULL;
@@ -1977,10 +1979,11 @@ static void ieee80211_tx_timeout(struct 
 {
 	struct ieee80211_local *local = dev->ieee80211_ptr;
 
-	printk(KERN_WARNING "%s: resetting interface.\n", dev->name);
+	printk(KERN_WARNING "wiphy%d: resetting interface.\n", local->hw.index);
 
 	if (local->ops->reset(local_to_hw(local)))
-		printk(KERN_ERR "%s: failed to reset interface.\n", dev->name);
+		printk(KERN_ERR "wiphy%d: failed to reset interface.\n",
+		       local->hw.index);
 	else
 		netif_wake_queue(dev);
 }
@@ -3493,24 +3496,25 @@ static void ieee80211_rx_michael_mic_rep
 
 	/* TODO: verify that this is not triggered by fragmented
 	 * frames (hw does not verify MIC for them). */
-	printk(KERN_DEBUG "%s: TKIP hwaccel reported Michael MIC "
+	printk(KERN_DEBUG "wiphy%d: TKIP hwaccel reported Michael MIC "
 	       "failure from " MAC_FMT " to " MAC_FMT " keyidx=%d\n",
-	       dev->name, MAC_ARG(hdr->addr2), MAC_ARG(hdr->addr1), keyidx);
+	       rx->local->hw.index, MAC_ARG(hdr->addr2), MAC_ARG(hdr->addr1),
+	       keyidx);
 
 	if (!sta) {
 		/* Some hardware versions seem to generate incorrect
 		 * Michael MIC reports; ignore them to avoid triggering
 		 * countermeasures. */
-		printk(KERN_DEBUG "%s: ignored spurious Michael MIC "
+		printk(KERN_DEBUG "wiphy%d: ignored spurious Michael MIC "
 		       "error for unknown address " MAC_FMT "\n",
-		       dev->name, MAC_ARG(hdr->addr2));
+		       rx->local->hw.index, MAC_ARG(hdr->addr2));
 		goto ignore;
 	}
 
 	if (!(rx->fc & IEEE80211_FCTL_PROTECTED)) {
-		printk(KERN_DEBUG "%s: ignored spurious Michael MIC "
+		printk(KERN_DEBUG "wiphy%d: ignored spurious Michael MIC "
 		       "error for a frame with no ISWEP flag (src "
-		       MAC_FMT ")\n", dev->name, MAC_ARG(hdr->addr2));
+		       MAC_FMT ")\n", rx->local->hw.index, MAC_ARG(hdr->addr2));
 		goto ignore;
 	}
 
@@ -3522,9 +3526,10 @@ static void ieee80211_rx_michael_mic_rep
 		 * for group keys and only the AP is sending real multicast
 		 * frames in BSS. */
 		if (keyidx) {
-			printk(KERN_DEBUG "%s: ignored Michael MIC error for "
+			printk(KERN_DEBUG "wiphy%d: ignored Michael MIC error for "
 			       "a frame with non-zero keyidx (%d) (src " MAC_FMT
-			       ")\n", dev->name, keyidx, MAC_ARG(hdr->addr2));
+			       ")\n", rx->local->hw.index, keyidx,
+			       MAC_ARG(hdr->addr2));
 			goto ignore;
 		}
 	}
@@ -3532,10 +3537,10 @@ static void ieee80211_rx_michael_mic_rep
 	if ((rx->fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA &&
 	    ((rx->fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_MGMT ||
 	     (rx->fc & IEEE80211_FCTL_STYPE) != IEEE80211_STYPE_AUTH)) {
-		printk(KERN_DEBUG "%s: ignored spurious Michael MIC "
+		printk(KERN_DEBUG "wiphy%d: ignored spurious Michael MIC "
 		       "error for a frame that cannot be encrypted "
 		       "(fc=0x%04x) (src " MAC_FMT ")\n",
-		       dev->name, rx->fc, MAC_ARG(hdr->addr2));
+		       rx->local->hw.index, rx->fc, MAC_ARG(hdr->addr2));
 		goto ignore;
 	}
 
@@ -3740,9 +3745,9 @@ void __ieee80211_rx(struct ieee80211_hw 
 				skb_new = skb_copy(skb, GFP_ATOMIC);
 				if (!skb_new) {
 					if (net_ratelimit())
-						printk(KERN_DEBUG "%s: failed to copy "
+						printk(KERN_DEBUG "wiphy%d: failed to copy "
 						       "multicast frame for %s",
-						       local->mdev->name, prev->dev->name);
+						       local->hw.index, prev->dev->name);
 					continue;
 				}
 				rx.skb = skb_new;
@@ -4019,8 +4024,8 @@ static void ieee80211_tasklet_handler(un
 			kfree(tx_status);
 			break;
 		default: /* should never get here! */
-			printk(KERN_ERR "%s: Unknown message type (%d)\n",
-			       local->mdev->name, skb->pkt_type);
+			printk(KERN_ERR "wiphy%d: Unknown message type (%d)\n",
+			       local->hw.index, skb->pkt_type);
 			dev_kfree_skb(skb);
 			break;
                 }
@@ -4102,8 +4107,8 @@ void ieee80211_tx_status(struct ieee8021
 
 	if (!status) {
                 printk(KERN_ERR
-		       "%s: ieee80211_tx_status called with NULL status\n",
-		       local->mdev->name);
+		       "wiphy%d: ieee80211_tx_status called with NULL status\n",
+		       local->hw.index);
 		dev_kfree_skb(skb);
 		return;
 	}
@@ -4157,10 +4162,10 @@ void ieee80211_tx_status(struct ieee8021
 				dev_queue_xmit(skb);
 			} else {
 				if (net_ratelimit()) {
-					printk(KERN_DEBUG "%s: dropped TX "
+					printk(KERN_DEBUG "wiphy%d: dropped TX "
 					       "filtered frame queue_len=%d "
 					       "PS=%d @%lu\n",
-					       local->mdev->name,
+					       local->hw.index,
 					       skb_queue_len(
 						       &sta->tx_filtered),
 					       !!(sta->flags & WLAN_STA_PS),
@@ -4360,14 +4365,14 @@ int ieee80211_init_rate_ctrl_alg(struct 
 
 	ref = rate_control_alloc(name, local);
 	if (!ref) {
-		printk(KERN_WARNING "%s: Failed to select rate control "
-		       "algorithm\n", local->mdev->name);
+		printk(KERN_WARNING "wiphy%d: Failed to select rate control "
+		       "algorithm\n", local->hw.index);
 		return -ENOENT;
 	}
 	res = rate_control_add_attrs(ref, &local->class_dev.kobj);
 	if (res < 0) {
-		printk(KERN_DEBUG "%s: Failed to register sysfs attributes "
-		       "for rate control\n", local->mdev->name);
+		printk(KERN_DEBUG "wiphy%d: Failed to register sysfs attributes "
+		       "for rate control\n", local->hw.index);
 		rate_control_put(ref);
 		return res;
 	}
@@ -4380,10 +4385,8 @@ int ieee80211_init_rate_ctrl_alg(struct 
 		sta_info_flush(local, NULL);
 	}
 
-	printk(KERN_DEBUG "%s: Selected rate control "
-	       "algorithm '%s'\n", local->mdev->name,
-	       ref->ops->name);
-
+	printk(KERN_DEBUG "wiphy%d: Selected rate control "
+	       "algorithm '%s'\n", local->hw.index, ref->ops->name);
 
 	return 0;
 }
@@ -4535,11 +4538,6 @@ int ieee80211_register_hw(struct ieee802
 	if (hw->flags & IEEE80211_HW_FRAGLIST)
 		local->mdev->features |= NETIF_F_FRAGLIST;
 	rtnl_lock();
-	result = dev_alloc_name(local->mdev, local->mdev->name);
-	if (result < 0) {
-		rtnl_unlock();
-		goto fail_dev;
-	}
 
 	memcpy(local->mdev->dev_addr, local->hw.perm_addr, ETH_ALEN);
 	SET_NETDEV_DEV(local->mdev, local->hw.dev);
@@ -4553,16 +4551,16 @@ int ieee80211_register_hw(struct ieee802
 	result = ieee80211_init_rate_ctrl_alg(local, NULL);
 	rtnl_unlock();
 	if (result < 0) {
-		printk(KERN_DEBUG "%s: Failed to initialize rate control "
-		       "algorithm\n", local->mdev->name);
+		printk(KERN_DEBUG "wiphy%d: Failed to initialize rate control "
+		       "algorithm\n", local->hw.index);
 		goto fail_rate;
 	}
 
 	result = ieee80211_wep_init(local);
 
 	if (result < 0) {
-		printk(KERN_DEBUG "%s: Failed to initialize wep\n",
-		       local->mdev->name);
+		printk(KERN_DEBUG "wiphy%d: Failed to initialize wep\n",
+		       local->hw.index);
 		goto fail_wep;
 	}
 
@@ -4673,8 +4671,8 @@ void ieee80211_unregister_hw(struct ieee
 
 	if (skb_queue_len(&local->skb_queue)
 			|| skb_queue_len(&local->skb_queue_unreliable))
-		printk(KERN_WARNING "%s: skb_queue not empty\n",
-		       local->mdev->name);
+		printk(KERN_WARNING "wiphy%d: skb_queue not empty\n",
+		       local->hw.index);
 	skb_queue_purge(&local->skb_queue);
 	skb_queue_purge(&local->skb_queue_unreliable);
 
net/d80211/ieee80211_scan.c +14 -14
@@ -116,9 +116,9 @@ static void ieee80211_scan_start(struct 
 	int ret;
 
 	if (!local->ops->passive_scan) {
-		printk(KERN_DEBUG "%s: Scan handler called, yet the hardware "
+		printk(KERN_DEBUG "wiphy%d: Scan handler called, yet the hardware "
 		       "does not support passive scanning. Disabled.\n",
-		       local->mdev->name);
+		       local->hw.index);
 		return;
 	}
 
@@ -135,8 +135,8 @@ static void ieee80211_scan_start(struct 
 	}
 
 	if (!local->scan.skb) {
-		printk(KERN_DEBUG "%s: Scan start called even though scan.skb "
-		       "is not set\n", local->mdev->name);
+		printk(KERN_DEBUG "wiphy%d: Scan start called even though scan.skb "
+		       "is not set\n", local->hw.index);
 	}
 
 	if (local->scan.our_mode_only) {
@@ -161,9 +161,9 @@ static void ieee80211_scan_start(struct 
 		skb_clone(local->scan.skb, GFP_ATOMIC) : NULL;
 	conf->tx_control = &local->scan.tx_control;
 #if 0
-	printk(KERN_DEBUG "%s: Doing scan on mode: %d freq: %d chan: %d "
+	printk(KERN_DEBUG "wiphy%d: Doing scan on mode: %d freq: %d chan: %d "
 	       "for %d ms\n",
-	       local->mdev->name, conf->scan_phymode, conf->scan_freq,
+	       local->hw.index, conf->scan_phymode, conf->scan_freq,
 	       conf->scan_channel, conf->scan_time);
 #endif
 	local->scan.rx_packets = 0;
@@ -193,8 +193,8 @@ static void ieee80211_scan_start(struct 
 			local->scan.mode = old_mode;
 			local->scan.chan_idx = old_chan_idx;
 		} else {
-			printk(KERN_DEBUG "%s: Got unknown error from "
-			       "passive_scan %d\n", local->mdev->name, ret);
+			printk(KERN_DEBUG "wiphy%d: Got unknown error from "
+			       "passive_scan %d\n", local->hw.index, ret);
 			local->scan.timer.expires = jiffies +
 				(local->scan.interval * HZ);
 		}
@@ -226,9 +226,9 @@ static void ieee80211_scan_stop(struct i
 				conf);
 
 #ifdef CONFIG_D80211_VERBOSE_DEBUG
-	printk(KERN_DEBUG "%s: Did scan on mode: %d freq: %d chan: %d "
+	printk(KERN_DEBUG "wiphy%d: Did scan on mode: %d freq: %d chan: %d "
 	       "GOT: %d Beacon: %d (%d)\n",
-	       local->mdev->name,
+	       local->hw.index,
 	       mode->mode, chan->freq, chan->chan,
 	       local->scan.rx_packets, local->scan.rx_beacon,
 	       local->scan.tries);
@@ -288,8 +288,8 @@ void ieee80211_init_scan(struct ieee8021
 		local->scan.skb = NULL;
 		memset(&local->scan.tx_control, 0,
 		       sizeof(local->scan.tx_control));
-		printk(KERN_DEBUG "%s: Does not support passive scan, "
-		       "disabled\n", local->mdev->name);
+		printk(KERN_DEBUG "wiphy%d: Does not support passive scan, "
+		       "disabled\n", local->hw.index);
 		return;
 	}
 
@@ -305,8 +305,8 @@ void ieee80211_init_scan(struct ieee8021
 	 * the low level changes channels */
 	local->scan.skb = alloc_skb(len, GFP_KERNEL);
 	if (!local->scan.skb) {
-		printk(KERN_WARNING "%s: Failed to allocate CTS packet for "
-		       "passive scan\n", local->mdev->name);
+		printk(KERN_WARNING "wiphy%d: Failed to allocate CTS packet for "
+		       "passive scan\n", local->hw.index);
 		return;
 	}
 
net/d80211/sta_info.c +6 -6
@@ -49,8 +49,8 @@ static void sta_info_hash_del(struct iee
 	if (s->hnext)
 		s->hnext = s->hnext->hnext;
 	else
-		printk(KERN_ERR "%s: could not remove STA " MAC_FMT " from "
-		       "hash table\n", local->mdev->name, MAC_ARG(sta->addr));
+		printk(KERN_ERR "wiphy%d: could not remove STA " MAC_FMT " from "
+		       "hash table\n", local->hw.index, MAC_ARG(sta->addr));
 }
 
 static inline struct sta_info *__sta_info_get(struct sta_info *sta)
@@ -172,8 +172,8 @@ struct sta_info * sta_info_add(struct ie
 	sta->key_idx_compression = HW_KEY_IDX_INVALID;
 
 #ifdef CONFIG_D80211_VERBOSE_DEBUG
-	printk(KERN_DEBUG "%s: Added STA " MAC_FMT "\n",
-	       local->mdev->name, MAC_ARG(addr));
+	printk(KERN_DEBUG "wiphy%d: Added STA " MAC_FMT "\n",
+	       local->hw.index, MAC_ARG(addr));
 #endif /* CONFIG_D80211_VERBOSE_DEBUG */
 
 	if (!in_interrupt()) {
@@ -194,8 +194,8 @@ static void finish_sta_info_free(struct 
 				 struct sta_info *sta)
 {
 #ifdef CONFIG_D80211_VERBOSE_DEBUG
-	printk(KERN_DEBUG "%s: Removed STA " MAC_FMT "\n",
-	       local->mdev->name, MAC_ARG(sta->addr));
+	printk(KERN_DEBUG "wiphy%d: Removed STA " MAC_FMT "\n",
+	       local->hw.index, MAC_ARG(sta->addr));
 #endif /* CONFIG_D80211_VERBOSE_DEBUG */
 
 	if (sta->key) {
net/d80211/wme.c +6 -2
@@ -421,7 +421,8 @@ static int wme_qdiscop_init(struct Qdisc
 						 qd->handle);
 		if (q->queues[i] == 0) {
 			q->queues[i] = &noop_qdisc;
-			printk(KERN_ERR "%s child qdisc %i creation failed", dev->name, i);
+			printk(KERN_ERR "wiphy%d: child qdisc %i creation failed",
+			       local->hw.index, i);
 		}
 	}
 
@@ -646,7 +647,10 @@ void ieee80211_install_qdisc(struct net_
 
 	qdisc = qdisc_create_dflt(dev, &wme_qdisc_ops, TC_H_ROOT);
 	if (!qdisc) {
-		printk(KERN_ERR "%s: qdisc installation failed\n", dev->name);
+		struct ieee80211_local *local = dev->ieee80211_ptr;
+
+		printk(KERN_ERR "wiphy%d: qdisc installation failed\n",
+		       local->hw.index);
 		return;
 	}
 
#4 lore raw

Re: [PATCH 4/6] d80211: don't display name of invisible network device

Quoted 5 lines · Jiri Benc

Invisible master interface does not have meaningful name. Display the wiphy identifier in kernel messages instead.

Also, remove the allocation of master interface name as it is purposeless now.

If the master netdev no longer has a name, how can you still use `tc' on it?

johannes

#5 lore raw

Re: [PATCH 4/6] d80211: don't display name of invisible network device

2007/1/30, Johannes Berg <[email protected]>:

Quoted 9 lines · Jiri Benc
Quoted 5 lines

Invisible master interface does not have meaningful name. Display the wiphy identifier in kernel messages instead.

Also, remove the allocation of master interface name as it is purposeless now.

If the master netdev no longer has a name, how can you still use `tc' on it?

I hope you can't, because that was recently proven to be able to subtly break the stack:

http://www.mail-archive.com/[email protected]/msg29219.html

Jan

#6 lore raw

Re: [PATCH 4/6] d80211: don't display name of invisible network device

Quoted 4 lines · Jan Kiszka
Quoted 2 lines

If the master netdev no longer has a name, how can you still use `tc' on it?

I hope you can't,

You know, being able to is the only (user interface) reason for the master dev's existence.

Quoted 4 lines

because that was recently proven to be able to
subtly break the stack:

http://www.mail-archive.com/[email protected]/msg29219.html

Well, that's a bug indeed, but the cure isn't "don't allow any qos operations" but rather "don't allow removal of the 802.11 qdisc".

johannes

author
#7 lore raw

Re: [PATCH 4/6] d80211: don't display name of invisible network device

Quoted 2 lines · Johannes Berg

Well, that's a bug indeed, but the cure isn't "don't allow any qos operations" but rather "don't allow removal of the 802.11 qdisc".

This solution can be described as "don't allow 802.11 qdisc customization for now, introduce that as a new feature later when qdiscs and netdev are not tied together".

It solves several problems by stripping down just one feature (which hardly anybody uses now and which is not working properly anyway) which will be reintroduced later. Quite feasible I would say given the fact that userspace-visible master network interface is perceived as a merge blocker by some people.

Jiri

Signature · 2 lines
Jiri Benc
SUSE Labs
#8 lore raw

Re: [PATCH 4/6] d80211: don't display name of invisible network device

Quoted 7 lines · Jiri Benc
Quoted 2 lines · Johannes Berg

Well, that's a bug indeed, but the cure isn't "don't allow any qos operations" but rather "don't allow removal of the 802.11 qdisc".

This solution can be described as "don't allow 802.11 qdisc customization for now, introduce that as a new feature later when qdiscs and netdev are not tied together".

Ok, that makes sense.

Quoted 5 lines

It solves several problems by stripping down just one feature (which hardly anybody uses now and which is not working properly anyway) which will be reintroduced later. Quite feasible I would say given the fact that userspace-visible master network interface is perceived as a merge blocker by some people.

Alright. Fine with me :)

johannes

author
#9 lore raw

PATCH 5/6 d80211: remove useless callbacks from wmaster

In reply to Jiri Benc

Invisible master interface doesn't need a lot of callbacks (actually, even the previous visible one didn't need them but that's another story).

---
 net/d80211/ieee80211.c       |    4 ----
 net/d80211/ieee80211_i.h     |    1 -
 net/d80211/ieee80211_ioctl.c |   22 ----------------------
 3 files changed, 27 deletions(-)
net/d80211/ieee80211.c +0 -4
@@ -4479,10 +4479,6 @@ struct ieee80211_hw *ieee80211_alloc_hw(
         sta_info_init(local);
 
 	mdev->hard_start_xmit = ieee80211_master_start_xmit;
-	mdev->wireless_handlers =
-		(struct iw_handler_def *) &ieee80211_iw_master_handler_def;
-        mdev->do_ioctl = ieee80211_ioctl;
-	mdev->change_mtu = ieee80211_change_mtu;
         mdev->tx_timeout = ieee80211_tx_timeout;
         mdev->get_stats = ieee80211_get_stats;
 	mdev->open = ieee80211_master_open;
net/d80211/ieee80211_i.h +0 -1
@@ -622,7 +622,6 @@ struct net_device_stats *ieee80211_dev_s
 /* ieee80211_ioctl.c */
 int ieee80211_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 extern const struct iw_handler_def ieee80211_iw_handler_def;
-extern const struct iw_handler_def ieee80211_iw_master_handler_def;
 
 /* Set hw encryption from ieee80211 */
 int ieee80211_set_hw_encryption(struct net_device *dev,
net/d80211/ieee80211_ioctl.c +0 -22
@@ -3243,25 +3243,3 @@ const struct iw_handler_def ieee80211_iw
 	.private_args	= (struct iw_priv_args *) ieee80211_ioctl_priv,
 	.get_wireless_stats = ieee80211_get_wireless_stats,
 };
-
-/* Wireless handlers for master interface */
-
-static const iw_handler ieee80211_master_handler[] =
-{
-	[SIOCGIWNAME  - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_giwname,
-	[SIOCSIWFREQ  - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_siwfreq,
-	[SIOCGIWFREQ  - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_giwfreq,
-	[SIOCGIWRANGE - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_giwrange,
-	[SIOCSIWRTS   - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_siwrts,
-	[SIOCGIWRTS   - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_giwrts,
-	[SIOCSIWFRAG  - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_siwfrag,
-	[SIOCGIWFRAG  - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_giwfrag,
-	[SIOCSIWRETRY - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_siwretry,
-	[SIOCGIWRETRY - SIOCIWFIRST] = (iw_handler) ieee80211_ioctl_giwretry,
-};
-
-const struct iw_handler_def ieee80211_iw_master_handler_def =
-{
-	.num_standard	= sizeof(ieee80211_master_handler) / sizeof(iw_handler),
-	.standard	= ieee80211_master_handler,
-};
author
#10 lore raw

PATCH 6/6 d80211: fix rtnl locking in ieee80211_register_hw

In reply to Jiri Benc

rtnl locking in ieee80211_register_hw was racy. This patch fixes that.

Unfortunatelly, the creation of master interface has to be still protected by rtnl lock as we need to protect against addition of a new virtual interface.

---
 net/d80211/ieee80211.c |    4 +---
 1 files changed, 1 insertion(+), 3 deletions(-)
net/d80211/ieee80211.c +1 -3
@@ -4540,12 +4540,10 @@ int ieee80211_register_hw(struct ieee802
 
 	result = register_invisible_netdevice(local->mdev);
 	if (result < 0) {
-		rtnl_unlock();
 		goto fail_dev;
 	}
 
 	result = ieee80211_init_rate_ctrl_alg(local, NULL);
-	rtnl_unlock();
 	if (result < 0) {
 		printk(KERN_DEBUG "wiphy%d: Failed to initialize rate control "
 		       "algorithm\n", local->hw.index);
@@ -4564,7 +4562,6 @@ int ieee80211_register_hw(struct ieee802
 	ieee80211_install_qdisc(local->mdev);
 
 	/* add one default STA interface */
-	rtnl_lock();
 	result = ieee80211_if_add(local->mdev, "wlan%d", 1, &sta_dev);
 	if (result == 0)
 		ieee80211_if_set_type(sta_dev, IEEE80211_IF_TYPE_STA);
@@ -4581,6 +4578,7 @@ fail_wep:
 fail_rate:
 	unregister_invisible_netdevice(local->mdev);
 fail_dev:
+	rtnl_unlock();
 	sta_info_stop(local);
 fail_sta_info:
 	ieee80211_dev_sysfs_del(local);
author
#11 lore raw

d80211: a patch for standalone d80211 tarball

In reply to Jiri Benc

This patch should be enough to allow creation of standalone d80211 tarball (i.e. version of d80211 that could be compiled without patching the vanilla kernel) after "invisible master interface" patches.

It is completely untested.

---
 net/d80211/ieee80211_i.h |    4 ++++
 1 files changed, 4 insertions(+)
net/d80211/ieee80211_i.h +4 -0
@@ -26,6 +26,10 @@
 /* ieee80211.o internal definitions, etc. These are not included into
  * low-level drivers. */
 
+#define ieee80211_ptr	ax25_ptr
+#define register_invisible_netdevice	register_netdevice
+#define unregister_invisible_netdevice	unregister_netdevice
+
 #ifndef ETH_P_PAE
 #define ETH_P_PAE 0x888E /* Port Access Entity (IEEE 802.1X) */
 #endif /* ETH_P_PAE */
#12 lore raw

Re: d80211: a patch for standalone d80211 tarball

Hi,

Quoted 3 lines

This patch should be enough to allow creation of standalone d80211 tarball (i.e. version of d80211 that could be compiled without patching the vanilla kernel) after "invisible master interface" patches.

That looks very nice indeed, that saves me quite a big patch I usually apply to the rt2x00-cvs version of d80211. :)

Quoted 1 line

It is completely untested.

I will put this patch into the rt2x00-cvs version of d80211 to see if it works. :)

Thanks.

Ivo

P.S.

I have some d80211 patches in testing that would add support for:
- software rts
- software sequence
- tx_header_room fixes

As soon as they are tested I'll send them to the list. :)

#13 lore raw

Re: d80211: a patch for standalone d80211 tarball

In reply to Jiri Benc

Hi, Jiri!

net/d80211/ieee80211_i.h line 31

Quoted 2 lines · Jiri Benc
net/d80211/ieee80211_i.h +2 -0
+#define register_invisible_netdevice	register_netdevice
+#define unregister_invisible_netdevice	unregister_netdevice

Please use macros with arguments whenever possible. This way, incorrect macro invocations would be detected even if the original prototype is missing. Also, the macro name could be used without arguments for other purposes (variable, gcc attribute etc).

Both cases are highly unlikely for the above macros, but please keep in mind that some aspiring programmers view the kernel as an example of good programming style.

Signature · 2 lines
Regards,
Pavel Roskin
author
#14 lore raw

Re: d80211: a patch for standalone d80211 tarball

Quoted 3 lines · Pavel Roskin

Both cases are highly unlikely for the above macros, but please keep in mind that some aspiring programmers view the kernel as an example of good programming style.

This wasn't meant to be included in the kernel (that would be indeed bad, although for another reasons) but to ease Ivo's and James' work. And it no longer matters anyway.

Jiri

Signature · 2 lines
Jiri Benc
SUSE Labs
#15 lore raw
In reply to Jiri Benc
Quoted 11 lines · On Mon, 29 Jan 2007 18:48:06 +0100 (CET) Jiri Benc <[email protected]> wrote:

The d80211 stack needs a network interface (called 'wmaster') used for communication with the hardware (it has 802.11 qdisc attached which perform MAC level QoS). This interface is not intended for users and it confuses them.

As a short time solution, this patch allows net_device to be registered as "invisible". This means it is not in the dev name hash list, its ifindex is -1 and protocols are not notified about its registration/unregistration.

Maybe code would be cleaner if you just could do:

	register_netdevice(mydev);

	cloak_netdevice(mydev);

And cloak_netdevice() just removed the network device from the name table. I would rather keep it with a real ifindex and in the device list, so that if the interface is miss used or the device is referenced by other devices, we don't see unexpected surprises like oops.

Signature · 1 line
Stephen Hemminger <[email protected]>
#16 lore raw

RFC 1/1 Alternative hidden netwirk device interface

Change to allow register_netdevice() to be called with a blank name. If name is blank, it is not put in name hash list, and doesn't show up in /sys or /proc

Compile tested only...

---
 net/core/dev.c |   56 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 33 insertions(+), 23 deletions(-)
net/core/dev.c +33 -23
index e660cb5..91f64e7 100644
@@ -737,7 +737,7 @@ int dev_change_name(struct net_device *d
 	if (dev->flags & IFF_UP)
 		return -EBUSY;
 
-	if (!dev_valid_name(newname))
+	if (hlist_unhashed(&dev->name_hlist) || !dev_valid_name(newname))
 		return -EINVAL;
 
 	if (strchr(newname, '%')) {
@@ -2108,6 +2108,10 @@ void dev_seq_stop(struct seq_file *seq, 
 
 static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev)
 {
+	/* hidden device */
+	if (hlist_unhashed(&dev->name_hlist))
+		return;
+
 	if (dev->get_stats) {
 		struct net_device_stats *stats = dev->get_stats(dev);
 
@@ -2868,10 +2872,6 @@ static inline void net_set_todo(struct n
  *
  *	Callers must hold the rtnl semaphore. You may want
  *	register_netdev() instead of this.
- *
- *	BUGS:
- *	The locking appears insufficient to guarantee two parallel registers
- *	will not get the same name.
  */
 
 int register_netdevice(struct net_device *dev)
@@ -2907,7 +2907,7 @@ #endif
 		}
 	}
  
-	if (!dev_valid_name(dev->name)) {
+	if (dev->name[0] && !dev_valid_name(dev->name)) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2917,15 +2917,18 @@ #endif
 		dev->iflink = dev->ifindex;
 
 	/* Check for existence of name */
-	head = dev_name_hash(dev->name);
-	hlist_for_each(p, head) {
-		struct net_device *d
-			= hlist_entry(p, struct net_device, name_hlist);
-		if (!strncmp(d->name, dev->name, IFNAMSIZ)) {
-			ret = -EEXIST;
- 			goto out;
+	if (dev->name[0]) {
+		head = dev_name_hash(dev->name);
+		hlist_for_each(p, head) {
+			struct net_device *d
+				= hlist_entry(p, struct net_device, name_hlist);
+			if (!strncmp(d->name, dev->name, IFNAMSIZ)) {
+				ret = -EEXIST;
+				goto out;
+			}
 		}
- 	}
+	} else
+		head = NULL;
 
 	/* Fix illegal SG+CSUM combinations. */
 	if ((dev->features & NETIF_F_SG) &&
@@ -2945,14 +2948,14 @@ #endif
 	if (dev->features & NETIF_F_UFO) {
 		if (!(dev->features & NETIF_F_HW_CSUM)) {
 			printk(KERN_ERR "%s: Dropping NETIF_F_UFO since no "
-					"NETIF_F_HW_CSUM feature.\n",
-							dev->name);
+			       "NETIF_F_HW_CSUM feature.\n",
+			       dev->name);
 			dev->features &= ~NETIF_F_UFO;
 		}
 		if (!(dev->features & NETIF_F_SG)) {
 			printk(KERN_ERR "%s: Dropping NETIF_F_UFO since no "
-					"NETIF_F_SG feature.\n",
-					dev->name);
+			       "NETIF_F_SG feature.\n",
+			       dev->name);
 			dev->features &= ~NETIF_F_UFO;
 		}
 	}
@@ -2965,8 +2968,7 @@ #endif
 	if (!dev->rebuild_header)
 		dev->rebuild_header = default_rebuild_header;
 
-	ret = netdev_register_sysfs(dev);
-	if (ret)
+	if (head && (ret = netdev_register_sysfs(dev)))
 		goto out;
 	dev->reg_state = NETREG_REGISTERED;
 
@@ -2982,7 +2984,12 @@ #endif
 	write_lock_bh(&dev_base_lock);
 	*dev_tail = dev;
 	dev_tail = &dev->next;
-	hlist_add_head(&dev->name_hlist, head);
+
+	if (head) 
+		hlist_add_head(&dev->name_hlist, head);
+	else
+		INIT_HLIST_NODE(&dev->name_hlist);
+		
 	hlist_add_head(&dev->index_hlist, dev_index_hash(dev->ifindex));
 	dev_hold(dev);
 	write_unlock_bh(&dev_base_lock);
@@ -3013,8 +3020,10 @@ int register_netdev(struct net_device *d
 {
 	int err;
 
-	rtnl_lock();
+	if (!dev->name[0])
+		return -EINVAL;
 
+	rtnl_lock();
 	/*
 	 * If the name is a format string the caller wants us to do a
 	 * name allocation.
@@ -3271,7 +3280,8 @@ int unregister_netdevice(struct net_devi
 	for (dp = &dev_base; (d = *dp) != NULL; dp = &d->next) {
 		if (d == dev) {
 			write_lock_bh(&dev_base_lock);
-			hlist_del(&dev->name_hlist);
+			if (!hlist_unhashed(&dev->name_hlist))
+				hlist_del(&dev->name_hlist);
 			hlist_del(&dev->index_hlist);
 			if (dev_tail == &dev->next)
 				dev_tail = dp;
Signature · 1 line
1.4.1
#17 lore raw

Re: [RFC] Alternative hidden netwirk device interface

Quoted 3 lines · Stephen Hemminger

Change to allow register_netdevice() to be called with a blank name. If name is blank, it is not put in name hash list, and doesn't show up in /sys or /proc

That sounds even better indeed!

Of course register_netdevice should grow a detailed explanation of this behaviour in the kdoc comment.

author
#18 lore raw

Re: [RFC] Alternative hidden netwirk device interface

In reply to Stephen Hemminger
Quoted 3 lines · Stephen Hemminger

Change to allow register_netdevice() to be called with a blank name. If name is blank, it is not put in name hash list, and doesn't show up in /sys or /proc

What about things like neigh_sysctl_register which expects nonempty dev name or rtnetlink_event which sends a RTM_NEWLINK event (I doubt that listeners are prepared for nonempty dev name)?

Also, if the interface has ifindex assigned it is accessible from user space (although it's unlikely anybody would call anything on it) - what should be the correct result of e.g. SIOCGIFNAME?

Thanks,

Jiri

Signature · 2 lines
Jiri Benc
SUSE Labs
#19 lore raw

Re: [RFC] Alternative hidden netwirk device interface

Quoted 8 lines · On Wed, 31 Jan 2007 19:26:47 +0100 Jiri Benc <[email protected]> wrote:
Quoted 3 lines · Stephen Hemminger

Change to allow register_netdevice() to be called with a blank name. If name is blank, it is not put in name hash list, and doesn't show up in /sys or /proc

What about things like neigh_sysctl_register which expects nonempty dev name or rtnetlink_event which sends a RTM_NEWLINK event (I doubt that listeners are prepared for nonempty dev name)?

that is fixable.

Quoted 3 lines

Also, if the interface has ifindex assigned it is accessible from user space (although it's unlikely anybody would call anything on it) - what should be the correct result of e.g. SIOCGIFNAME?

blank?

Quoted 5 lines

Thanks,

Jiri

Signature · 1 line
Stephen Hemminger <[email protected]>
maintainer
#20 lore raw

Re: [RFC] Alternative hidden netwirk device interface

In reply to Stephen Hemminger

From: Stephen Hemminger <[email protected]>
Date: Mon, 29 Jan 2007 14:09:58 -0800

Quoted 5 lines

Change to allow register_netdevice() to be called with a blank name. If name is blank, it is not put in name hash list, and doesn't show up in /sys or /proc

Compile tested only...

I have no objections to this.

It would be nice to wrap the hlist_unhashed() test around a well named inline function, such as "netdev_hidden()" or similar.

With that minor enhancement, and a nice changelog (including a description of what this would be used for), I'll happily merge this during the next merge window.

Thanks Stephen.

#21 lore raw
In reply to Stephen Hemminger
Quoted 11 lines · Stephen Hemminger

Maybe code would be cleaner if you just could do:

	register_netdevice(mydev);

	cloak_netdevice(mydev);

And cloak_netdevice() just removed the network device from the name table. I would rather keep it with a real ifindex and in the device list, so that if the interface is miss used or the device is referenced by other devices, we don't see unexpected surprises like oops.

I don't think first publishing it and then hiding it is a good idea, but I agreed that it should at least have a valid ifindex.