diff mbox

add netpoll support for 802.1q vlans

Message ID 20111207010424.GA16496@kvack.org
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Benjamin LaHaise Dec. 7, 2011, 1:04 a.m. UTC
Add netpoll support to 802.1q vlan devices.  Based on the netpoll support 
in the bridging code.  Tested on a forced_eth device with netconsole.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller Dec. 8, 2011, 5:07 a.m. UTC | #1
From: Benjamin LaHaise <bcrl@kvack.org>
Date: Tue, 6 Dec 2011 20:04:24 -0500

> Add netpoll support to 802.1q vlan devices.  Based on the netpoll support 
> in the bridging code.  Tested on a forced_eth device with netconsole.
> 
> Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>

Bridging doesn't try to invoke a downstream netpoll operation at all,
and neither should you.

Also, please format your subject line correctly so I don't have to edit
it when applying your patch.  You need to add an appropriate, lowercase,
subsystem prefix after the [PATCH] tag otherwise people scanning the
shortlog in GIT can't figure out what area your patch is in.

In this situation "vlan: " might be appropriate.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cong Wang Dec. 8, 2011, 9:14 a.m. UTC | #2
On Tue, 06 Dec 2011 20:04:24 -0500, Benjamin LaHaise wrote:

> Add netpoll support to 802.1q vlan devices.  Based on the netpoll
> support in the bridging code.  Tested on a forced_eth device with
> netconsole.
> 
> Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>

You also need to handle DEL_VLAN_CMD case.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin LaHaise Dec. 8, 2011, 3:35 p.m. UTC | #3
On Thu, Dec 08, 2011 at 12:07:30AM -0500, David Miller wrote:
> From: Benjamin LaHaise <bcrl@kvack.org>
> Date: Tue, 6 Dec 2011 20:04:24 -0500
> 
> > Add netpoll support to 802.1q vlan devices.  Based on the netpoll support 
> > in the bridging code.  Tested on a forced_eth device with netconsole.
> > 
> > Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
> 
> Bridging doesn't try to invoke a downstream netpoll operation at all,
> and neither should you.

Please have a look at net/bridge/br_device.c:br_netpoll_setup() which is 
where the approach came from.

Getting netconsole over vlans working is a requirement for those of us 
with machines plugged into ethernet switches that can't mix tagged and 
untagged packets on their ethernet ports.  I bumped into this while 
trying to capture a kernel crash on such a machine which doesn't have any 
external serial ports to use either.

> Also, please format your subject line correctly so I don't have to edit
> it when applying your patch.  You need to add an appropriate, lowercase,
> subsystem prefix after the [PATCH] tag otherwise people scanning the
> shortlog in GIT can't figure out what area your patch is in.
> 
> In this situation "vlan: " might be appropriate.

Sorry about that oversight, I'll try to remember for the future.

		-ben
David Miller Dec. 8, 2011, 4:50 p.m. UTC | #4
From: Benjamin LaHaise <bcrl@kvack.org>
Date: Thu, 8 Dec 2011 10:35:58 -0500

> On Thu, Dec 08, 2011 at 12:07:30AM -0500, David Miller wrote:
>> From: Benjamin LaHaise <bcrl@kvack.org>
>> Date: Tue, 6 Dec 2011 20:04:24 -0500
>> 
>> > Add netpoll support to 802.1q vlan devices.  Based on the netpoll support 
>> > in the bridging code.  Tested on a forced_eth device with netconsole.
>> > 
>> > Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
>> 
>> Bridging doesn't try to invoke a downstream netpoll operation at all,
>> and neither should you.
> 
> Please have a look at net/bridge/br_device.c:br_netpoll_setup() which is 
> where the approach came from.

I did and I'm not talking about that part, I'm talking about the
part where you invoke the downstream device's netpoll operation.

Did you even read my entire sentence, or did you just let the
knee jerk reaction set in before you got to the end?

> Getting netconsole over vlans working is a requirement for those of us 

What does this have to do with anything?  I never made any even remote
argument that this change wasn't useful.  I said only that you
implemented it incorrectly.

Take a deep breath and just implement the patch correctly.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 9fd45f3..df562b5 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -40,6 +40,8 @@  struct vlan_pcpu_stats {
 	u32			tx_dropped;
 };
 
+struct netpoll;
+
 /**
  *	struct vlan_dev_info - VLAN private device data
  *	@nr_ingress_mappings: number of ingress priority mappings
@@ -67,6 +69,9 @@  struct vlan_dev_info {
 
 	struct proc_dir_entry			*dent;
 	struct vlan_pcpu_stats __percpu		*vlan_pcpu_stats;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	struct netpoll				*netpoll;
+#endif
 };
 
 static inline struct vlan_dev_info *vlan_dev_info(const struct net_device *dev)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 2b5fcde..98417ff 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -33,6 +33,7 @@ 
 #include "vlan.h"
 #include "vlanproc.h"
 #include <linux/if_vlan.h>
+#include <linux/netpoll.h>
 
 /*
  *	Rebuild the Ethernet MAC header. This is called after an ARP
@@ -158,6 +159,8 @@  static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 
 	skb_set_dev(skb, vlan_dev_info(dev)->real_dev);
 	len = skb->len;
+	if (netpoll_tx_running(dev))
+		return skb->dev->netdev_ops->ndo_start_xmit(skb, skb->dev);
 	ret = dev_queue_xmit(skb);
 
 	if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
@@ -660,6 +663,60 @@  static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, st
 	return stats;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+void vlan_dev_poll_controller(struct net_device *dev)
+{
+	struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
+
+	if (real_dev->netdev_ops->ndo_poll_controller)
+		real_dev->netdev_ops->ndo_poll_controller(real_dev);
+}
+
+int vlan_dev_netpoll_setup(struct net_device *dev, struct netpoll_info *npinfo)
+{
+	struct vlan_dev_info *info = vlan_dev_info(dev);
+	struct net_device *real_dev = info->real_dev;
+	struct netpoll *netpoll;
+	int err = 0;
+
+	netpoll = kzalloc(sizeof(*netpoll), GFP_KERNEL);
+	err = -ENOMEM;
+	if (!netpoll)
+		goto out;
+
+	netpoll->dev = real_dev;
+	strlcpy(netpoll->dev_name, real_dev->name, IFNAMSIZ);
+
+	err = __netpoll_setup(netpoll);
+	if (err) {
+		kfree(netpoll);
+		goto out;
+	}
+
+	info->netpoll = netpoll;
+
+out:
+	return err;
+}
+
+void vlan_dev_netpoll_cleanup(struct net_device *dev)
+{
+	struct vlan_dev_info *info = vlan_dev_info(dev);
+	struct netpoll *netpoll = info->netpoll;
+
+	if (!netpoll)
+		return;
+
+	info->netpoll = NULL;
+
+        /* Wait for transmitting packets to finish before freeing. */
+        synchronize_rcu_bh();
+
+        __netpoll_cleanup(netpoll);
+        kfree(netpoll);
+}
+#endif /* CONFIG_NET_POLL_CONTROLLER */
+
 static const struct ethtool_ops vlan_ethtool_ops = {
 	.get_settings	        = vlan_ethtool_get_settings,
 	.get_drvinfo	        = vlan_ethtool_get_drvinfo,
@@ -688,6 +745,11 @@  static const struct net_device_ops vlan_netdev_ops = {
 	.ndo_fcoe_get_wwn	= vlan_dev_fcoe_get_wwn,
 	.ndo_fcoe_ddp_target	= vlan_dev_fcoe_ddp_target,
 #endif
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_poll_controller	= vlan_dev_poll_controller,
+	.ndo_netpoll_setup	= vlan_dev_netpoll_setup,
+	.ndo_netpoll_cleanup	= vlan_dev_netpoll_cleanup,
+#endif
 	.ndo_fix_features	= vlan_dev_fix_features,
 };