[RFC PATCH 1/6] invisible network devices

[RFC PATCH 1/6] invisible network devices

[Jiri Benc]

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.

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 include/linux/netdevice.h |    2 
 net/core/dev.c            |  223 ++++++++++++++++++++++++++++++----------------
 2 files changed, 151 insertions(+), 74 deletions(-)

--- dscape.orig/include/linux/netdevice.h
+++ dscape/include/linux/netdevice.h
@@ -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);
--- dscape.orig/net/core/dev.c
+++ dscape/net/core/dev.c
@@ -2891,37 +2891,10 @@ static inline void net_set_todo(struct n
 	spin_unlock(&net_todo_list_lock);
 }
 
-/**
- *	register_netdevice	- register a network device
- *	@dev: device to register
- *
- *	Take a completed network device structure and add it to the kernel
- *	interfaces. A %NETDEV_REGISTER message is sent to the netdev notifier
- *	chain. 0 is returned on success. A negative errno code is returned
- *	on a failure to set up the device, or if the name is a duplicate.
- *
- *	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)
+static int init_netdevice(struct net_device *dev)
 {
-	struct hlist_head *head;
-	struct hlist_node *p;
 	int ret;
 
-	BUG_ON(dev_boot_phase);
-	ASSERT_RTNL();
-
-	might_sleep();
-
-	/* When net_device's are persistent, this will be fatal. */
-	BUG_ON(dev->reg_state != NETREG_UNINITIALIZED);
-
 	spin_lock_init(&dev->queue_lock);
 	spin_lock_init(&dev->_xmit_lock);
 	dev->xmit_lock_owner = -1;
@@ -2929,7 +2902,7 @@ int register_netdevice(struct net_device
 	spin_lock_init(&dev->ingress_lock);
 #endif
 
-	dev->iflink = -1;
+	dev->iflink = dev->ifindex = -1;
 
 	/* Init, if this function is available */
 	if (dev->init) {
@@ -2940,27 +2913,13 @@ int register_netdevice(struct net_device
 			goto out;
 		}
 	}
- 
-	if (!dev_valid_name(dev->name)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	dev->ifindex = dev_new_index();
-	if (dev->iflink == -1)
-		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;
-		}
- 	}
+	ret = 0;
+out:
+	return ret;
+}
 
+static void setup_netdevice(struct net_device *dev)
+{
 	/* Fix illegal SG+CSUM combinations. */
 	if ((dev->features & NETIF_F_SG) &&
 	    !(dev->features & NETIF_F_ALL_CSUM)) {
@@ -2999,17 +2958,75 @@ int register_netdevice(struct net_device
 	if (!dev->rebuild_header)
 		dev->rebuild_header = default_rebuild_header;
 
-	ret = netdev_register_sysfs(dev);
-	if (ret)
-		goto out;
-	dev->reg_state = NETREG_REGISTERED;
-
 	/*
 	 *	Default initial state at registry is that the
 	 *	device is present.
 	 */
 
 	set_bit(__LINK_STATE_PRESENT, &dev->state);
+}
+
+/**
+ *	register_netdevice	- register a network device
+ *	@dev: device to register
+ *
+ *	Take a completed network device structure and add it to the kernel
+ *	interfaces. A %NETDEV_REGISTER message is sent to the netdev notifier
+ *	chain. 0 is returned on success. A negative errno code is returned
+ *	on a failure to set up the device, or if the name is a duplicate.
+ *
+ *	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)
+{
+	struct hlist_head *head;
+	struct hlist_node *p;
+	int ret;
+
+	BUG_ON(dev_boot_phase);
+	ASSERT_RTNL();
+
+	might_sleep();
+
+	/* When net_device's are persistent, this will be fatal. */
+	BUG_ON(dev->reg_state != NETREG_UNINITIALIZED);
+
+	ret = init_netdevice(dev);
+	if (ret)
+		goto out;
+
+	if (!dev_valid_name(dev->name)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	dev->ifindex = dev_new_index();
+	if (dev->iflink == -1)
+		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;
+		}
+ 	}
+
+	ret = netdev_register_sysfs(dev);
+	if (ret)
+		goto out;
+
+	setup_netdevice(dev);
+	dev->reg_state = NETREG_REGISTERED;
 
 	dev->next = NULL;
 	dev_init_scheduler(dev);
@@ -3030,6 +3047,31 @@ out:
 	return ret;
 }
 
+int register_invisible_netdevice(struct net_device *dev)
+{
+	int ret;
+
+	BUG_ON(dev_boot_phase);
+	might_sleep();
+	BUG_ON(dev->reg_state != NETREG_UNINITIALIZED);
+
+	ret = init_netdevice(dev);
+	if (ret)
+		goto out;
+
+	setup_netdevice(dev);
+	dev->reg_state = NETREG_REGISTERED;
+
+	dev->next = NULL;
+	dev_init_scheduler(dev);
+	dev_hold(dev);
+
+	ret = 0;
+out:
+	return ret;
+}
+EXPORT_SYMBOL(register_invisible_netdevice);
+
 /**
  *	register_netdev	- register a network device
  *	@dev: device to register
@@ -3118,6 +3160,23 @@ static void netdev_wait_allrefs(struct n
 	}
 }
 
+static void destruct_netdevice(struct net_device *dev)
+{
+	netdev_wait_allrefs(dev);
+
+	/* paranoia */
+	BUG_ON(atomic_read(&dev->refcnt));
+	BUG_TRAP(!dev->ip_ptr);
+	BUG_TRAP(!dev->ip6_ptr);
+	BUG_TRAP(!dev->dn_ptr);
+
+	/* It must be the very last action,
+	 * after this 'dev' may point to freed up memory.
+	 */
+	if (dev->destructor)
+		dev->destructor(dev);
+}
+
 /* The sequence is:
  *
  *	rtnl_lock();
@@ -3175,19 +3234,7 @@ void netdev_run_todo(void)
 		netdev_unregister_sysfs(dev);
 		dev->reg_state = NETREG_UNREGISTERED;
 
-		netdev_wait_allrefs(dev);
-
-		/* paranoia */
-		BUG_ON(atomic_read(&dev->refcnt));
-		BUG_TRAP(!dev->ip_ptr);
-		BUG_TRAP(!dev->ip6_ptr);
-		BUG_TRAP(!dev->dn_ptr);
-
-		/* It must be the very last action,
-		 * after this 'dev' may point to freed up memory.
-		 */
-		if (dev->destructor)
-			dev->destructor(dev);
+		destruct_netdevice(dev);
 	}
 
 out:
@@ -3269,6 +3316,17 @@ void synchronize_net(void) 
 	synchronize_rcu();
 }
 
+static void uninit_netdevice(struct net_device *dev)
+{
+	/*
+	 *	Flush the multicast chain
+	 */
+	dev_mc_discard(dev);
+
+	if (dev->uninit)
+		dev->uninit(dev);
+}
+
 /**
  *	unregister_netdevice - remove device from the kernel
  *	@dev: device
@@ -3332,14 +3390,8 @@ int unregister_netdevice(struct net_devi
 	   this device. They should clean all the things.
 	*/
 	raw_notifier_call_chain(&netdev_chain, NETDEV_UNREGISTER, dev);
-	
-	/*
-	 *	Flush the multicast chain
-	 */
-	dev_mc_discard(dev);
 
-	if (dev->uninit)
-		dev->uninit(dev);
+	uninit_netdevice(dev);
 
 	/* Notifier chain MUST detach us from master device. */
 	BUG_TRAP(!dev->master);
@@ -3353,6 +3405,29 @@ int unregister_netdevice(struct net_devi
 	return 0;
 }
 
+void unregister_invisible_netdevice(struct net_device *dev)
+{
+	BUG_ON(dev_boot_phase);
+	BUG_ON(dev->reg_state != NETREG_REGISTERED);
+	might_sleep();
+
+	dev_close(dev);
+	dev->reg_state = NETREG_UNREGISTERING;
+	synchronize_net();
+	dev_shutdown(dev);
+
+	uninit_netdevice(dev);
+
+	BUG_TRAP(!dev->master);
+
+	synchronize_net();
+	dev_put(dev);
+
+	dev->reg_state = NETREG_UNREGISTERED;
+	destruct_netdevice(dev);
+}
+EXPORT_SYMBOL(unregister_invisible_netdevice);
+
 /**
  *	unregister_netdev - remove device from the kernel
  *	@dev: device

-- 
Jiri Benc
SUSE Labs

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

[Jiri Benc]

Use register_invisible_netdevice for master interface.

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 net/d80211/ieee80211.c       |   22 ++--------------------
 net/d80211/ieee80211_iface.c |    8 ++++++--
 2 files changed, 8 insertions(+), 22 deletions(-)

--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -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);
 
--- dscape.orig/net/d80211/ieee80211_iface.c
+++ dscape/net/d80211/ieee80211_iface.c
@@ -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). */
 }

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

[Jiri Benc]

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

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 net/d80211/ieee80211.c |   46 +++++++++++++++++++++++++---------------------
 1 files changed, 25 insertions(+), 21 deletions(-)

--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -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;
 

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

[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.

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 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(-)

--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -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);
 
--- dscape.orig/net/d80211/ieee80211_scan.c
+++ dscape/net/d80211/ieee80211_scan.c
@@ -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;
 	}
 
--- dscape.orig/net/d80211/sta_info.c
+++ dscape/net/d80211/sta_info.c
@@ -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) {
--- dscape.orig/net/d80211/wme.c
+++ dscape/net/d80211/wme.c
@@ -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;
 	}
 

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

On Mon, 2007-01-29 at 18:48 +0100, Jiri Benc wrote:
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

[Jan Kiszka]

2007/1/30, Johannes Berg <johannes@sipsolutions.net>:
> On Mon, 2007-01-29 at 18:48 +0100, Jiri Benc wrote:
> > 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/netdev@vger.kernel.org/msg29219.html

Jan

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On Tue, 2007-01-30 at 15:00 +0100, Jan Kiszka wrote:

> > 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.

> because that was recently proven to be able to
> subtly break the stack:
> 
> http://www.mail-archive.com/netdev@vger.kernel.org/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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

[Jiri Benc]

On Wed, 31 Jan 2007 19:58:58 +0100, Johannes Berg wrote:
> 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

-- 
Jiri Benc
SUSE Labs

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On Thu, 2007-02-01 at 16:17 +0100, Jiri Benc wrote:
> On Wed, 31 Jan 2007 19:58:58 +0100, Johannes Berg wrote:
> > 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.

> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[PATCH 5/6] d80211: remove useless callbacks from wmaster

[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).

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 net/d80211/ieee80211.c       |    4 ----
 net/d80211/ieee80211_i.h     |    1 -
 net/d80211/ieee80211_ioctl.c |   22 ----------------------
 3 files changed, 27 deletions(-)

--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -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;
--- dscape.orig/net/d80211/ieee80211_i.h
+++ dscape/net/d80211/ieee80211_i.h
@@ -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,
--- dscape.orig/net/d80211/ieee80211_ioctl.c
+++ dscape/net/d80211/ieee80211_ioctl.c
@@ -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,
-};

[PATCH 6/6] d80211: fix rtnl locking in ieee80211_register_hw

[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.

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 net/d80211/ieee80211.c |    4 +---
 1 files changed, 1 insertion(+), 3 deletions(-)

--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -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);

d80211: a patch for standalone d80211 tarball

[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.

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 net/d80211/ieee80211_i.h |    4 ++++
 1 files changed, 4 insertions(+)

--- dscape.orig/net/d80211/ieee80211_i.h
+++ dscape/net/d80211/ieee80211_i.h
@@ -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 */

Re: d80211: a patch for standalone d80211 tarball

[Ivo Van Doorn]

Hi,

> 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. :)

> 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. :)

Re: d80211: a patch for standalone d80211 tarball

[Pavel Roskin]

Hi, Jiri!

On Mon, 2007-01-29 at 18:48 +0100, Jiri Benc wrote:

> +#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.

-- 
Regards,
Pavel Roskin

Re: d80211: a patch for standalone d80211 tarball

[Jiri Benc]

On Mon, 29 Jan 2007 15:23:07 -0500, Pavel Roskin wrote:
> 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

-- 
Jiri Benc
SUSE Labs

Re: [RFC PATCH 1/6] invisible network devices

[Stephen Hemminger]

On Mon, 29 Jan 2007 18:48:06 +0100 (CET)
Jiri Benc <jbenc@suse.cz> 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.
> 
> Signed-off-by: Jiri Benc <jbenc@suse.cz>
>

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.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[RFC] Alternative hidden netwirk device interface

[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

Compile tested only...

---
 net/core/dev.c |   56 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index e660cb5..91f64e7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -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;
-- 
1.4.1

Re: [RFC] Alternative hidden netwirk device interface

[Christoph Hellwig]

On Mon, Jan 29, 2007 at 02:09:58PM -0800, Stephen Hemminger wrote:
> 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.

Re: [RFC] Alternative hidden netwirk device interface

[Jiri Benc]

On Mon, 29 Jan 2007 14:09:58 -0800, Stephen Hemminger wrote:
> 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

-- 
Jiri Benc
SUSE Labs

Re: [RFC] Alternative hidden netwirk device interface

[Stephen Hemminger]

On Wed, 31 Jan 2007 19:26:47 +0100
Jiri Benc <jbenc@suse.cz> wrote:

> On Mon, 29 Jan 2007 14:09:58 -0800, Stephen Hemminger wrote:
> > 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.

> 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?

> 
> Thanks,
> 
>  Jiri
> 


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [RFC] Alternative hidden netwirk device interface

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Mon, 29 Jan 2007 14:09:58 -0800

> 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.

Re: [RFC PATCH 1/6] invisible network devices

[Christoph Hellwig]

On Mon, Jan 29, 2007 at 10:28:14AM -0800, Stephen Hemminger wrote:
> 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.