Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: David Miller @ 2009-10-30 19:41 UTC (permalink / raw)
  To: ebiederm; +Cc: netdev, fubar
In-Reply-To: <m1tyxhwxah.fsf@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 29 Oct 2009 17:16:54 -0700

> I recently had it pointed out to me that the bonding driver does not
> work in a network namespace.  So I have simplified the bonding driver
> a bit, added support for ip link add and ip link del, and finally made
> the bonding driver work in multiple network namespaces.
> 
> The most note worthy change in the patchset is the addition of support
> in the networking core for registering a sysfs group for a device.
> 
> Using this in the bonding driver simplifies the code and removes a
> userspace race between actions triggered by the netlink event and the
> bonding sysfs attributes appearing.

I've tossed patches 1-7 into net-next-2.6, thanks Eric.

^ permalink raw reply

* Re: Connection tracking and vlan
From: Caitlin Bestler @ 2009-10-30 19:51 UTC (permalink / raw)
  To: Adayadil Thomas; +Cc: Eric Dumazet, Herbert Xu, netdev, Patrick McHardy
In-Reply-To: <fb7befa20910301220y53e43608u3b37de17661feb4e@mail.gmail.com>

Yes, it is legitimate for a Bridge to see two different 10.*.*.*
networks on different VLANs.
A Bridge can even see that same MAC address being used by two
different end stations
on different VLANs (especially if the global bit is not set).

What is not legitimate is presenting both of those 10.*.*.* networks
for local delivery.

If you are only bridging the frames then there are no connections to
track, only frames.



On Fri, Oct 30, 2009 at 12:20 PM, Adayadil Thomas
<adayadil.thomas@gmail.com> wrote:
> On Fri, Oct 30, 2009 at 11:31 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Very strange, this question about vlan looks like discussion we had
>> yesterday (or the day before...) about interfaces (versus packet defragmentation)
>>
>> "IP conntracking" is about IP, and [V]LAN doesnt matter at all at this protocol level.
>>
>> Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
>> include interface name/index
>
> I am concerned about the following situation -
>
> The linux device is configured as a bridge and is deployed between the
> trunk ports
> of 2 switches. In this situation the linux device will be seeing 802.1q packets
> with vlan headers specifying the vlanids.
>
> It is valid to have an environment where private IP addresses are duplicated
> on different virtual LANs. i.e. it is valid to have machine A (
> 10.10.10.1) talking to
> machine B (10.10.10.2) on vlan 1,
> and
> at the same time machine C ( 10.10.10.1) talking to machine D
> (10.10.10.2) on vlan 2.
>
> Since they are on different LANs (VLANs), there should not be any issues.
>
> Now when VLANs are shared across switches, the trunk port will sent
> 802.1q tagged
> packets between the switches. Imagine these packets when going through
> the linux bridge.
> The 802.1q header should identify the separate vlans by the vlan id.
>
> If ip_conntrack does not consider vlans, it is possible that all 5
> tuple are the same
> and thus affect the connection tracking.
>
> I hope I have described the scenario well. If not I can explain in a
> more detailed fashion.
>
> Thanks
> --
> 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
>

^ permalink raw reply

* Re: Fw: [Bug 14470] New: freez in TCP stack
From: Herbert Xu @ 2009-10-30 20:18 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: eric.dumazet, davem, akpm, shemminger, netdev, kolo,
	bugzilla-daemon
In-Reply-To: <alpine.DEB.2.00.0910291316340.19761@wel-95.cs.helsinki.fi>

Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> wrote:
> 
> One more alternative along those lines could perhaps be:
> 
> We enter with empty write_queue there and with the hint being null, so we 
> take the else branch... and skb_peek then gives us the NULL ptr. However, 
> I cannot see how this could happen as all branches trap with return 
> before the reach tcp_xmit_retransmit_queue.

Why don't we add a WARN_ON in there and see if it triggers?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: Connection tracking and vlan
From: Adayadil Thomas @ 2009-10-30 20:40 UTC (permalink / raw)
  To: Caitlin Bestler; +Cc: Eric Dumazet, Herbert Xu, netdev, Patrick McHardy
In-Reply-To: <469958e00910301251he051d69p178ce53e84130765@mail.gmail.com>

On Fri, Oct 30, 2009 at 3:51 PM, Caitlin Bestler
<caitlin.bestler@gmail.com> wrote:
> Yes, it is legitimate for a Bridge to see two different 10.*.*.*
> networks on different VLANs.
> A Bridge can even see that same MAC address being used by two
> different end stations
> on different VLANs (especially if the global bit is not set).
>
> What is not legitimate is presenting both of those 10.*.*.* networks
> for local delivery.
>

I did not mean this case.

> If you are only bridging the frames then there are no connections to
> track, only frames.
>

This is more like what I was trying to do with the device, but with
stateful firewall functionality
for which I was using iptables/netfilter.

Thanks

^ permalink raw reply

* Re: pull request: wireless-next-2.6 2009-10-28
From: Jarek Poplawski @ 2009-10-30 20:44 UTC (permalink / raw)
  To: John W. Linville
  Cc: Johannes Berg, Bartlomiej Zolnierkiewicz, Pekka Enberg,
	David Miller, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20091030150223.GA2586-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

On Fri, Oct 30, 2009 at 11:02:24AM -0400, John W. Linville wrote:
> On Fri, Oct 30, 2009 at 11:06:16AM +0000, Jarek Poplawski wrote:
> 
> > There are various ways to disagree, and ignoring by John questions
> > from a merited developer both in this referenced lkml and current
> > threads looks at least strange (if not offensive) as well.
> 
> Did you read the thread for which Bartlomiej provided a link earlier?
> There were ten responses (only three of them from him) in that thread.
> His comments were not ignored, they were rejected.
> 
> Ever since Bartlomiej decided to tear himself away from
> drivers/staging, he has been nothing but negative -- petty, whining,
> indignat, whatever.  Just what has he done to merit any special
> consideration here?  Why should he have any sort of veto over rt2x00?
> 
> And of all things on which to take a stand -- how dare the rt2x00 guys
> use two header files instead of three?  The nerve of those people!!!
> 
> Ridiculous...

Maybe. Different tastes for sure, so simply no good solution.

Anyway, my point was somewhere else: Davem could be a good example;-)
I simply can't imagine a thread with such concerns left completely
uncommented before merging a patch. It's not about convincing anybody.
Usually you know at least if the concern is dismissed, or valid, but
fixable. (Sometimes you can even read between lines, or explicit, what
he really thinks...)

This discussion is mainly around trying to omit a maintainer with
"higher authority". Of course it's a mortal sin! ...Except, when the
maintainer doesn't seem to respond at all.

BTW, it seems Bartlomiej's main argument in the current thread has
changed to the non-working driver, so ridiculous only if you have a
clue.

Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Jay Vosburgh @ 2009-10-30 21:12 UTC (permalink / raw)
  To: David Miller; +Cc: ebiederm, netdev
In-Reply-To: <20091030.124153.00934819.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote:

>From: ebiederm@xmission.com (Eric W. Biederman)
>Date: Thu, 29 Oct 2009 17:16:54 -0700
>
>> I recently had it pointed out to me that the bonding driver does not
>> work in a network namespace.  So I have simplified the bonding driver
>> a bit, added support for ip link add and ip link del, and finally made
>> the bonding driver work in multiple network namespaces.
>> 
>> The most note worthy change in the patchset is the addition of support
>> in the networking core for registering a sysfs group for a device.
>> 
>> Using this in the bonding driver simplifies the code and removes a
>> userspace race between actions triggered by the netlink event and the
>> bonding sysfs attributes appearing.
>
>I've tossed patches 1-7 into net-next-2.6, thanks Eric.

	I put patches 1-7 on a recent net-next-2.6, and from a simple
"insmod bonding.ko; rmmod bonding" I'm seeing the following:

------------[ cut here ]------------
WARNING: at fs/proc/generic.c:847 remove_proc_entry+0x1a8/0x1c7()
Hardware name: IBM eserver xSeries 220 -[8645]-
remove_proc_entry: removing non-empty directory 'net/bonding', leaking at least 
'bond0'
Modules linked in: bonding(-) ipv6 microcode loop ppdev sworks_agp parport_pc tg
3 e100 agpgart parport mii libphy e1000 edd pata_serverworks [last unloaded: spe
edstep_lib]
Pid: 6216, comm: rmmod Not tainted 2.6.32-rc3-devel #19
Call Trace:
 [<c012ec9d>] warn_slowpath_common+0x60/0x90
 [<c012ed01>] warn_slowpath_fmt+0x24/0x27
 [<c01e3e55>] remove_proc_entry+0x1a8/0x1c7
 [<e0906435>] ? bond_net_exit+0x0/0xa3 [bonding]
 [<e09064c3>] bond_net_exit+0x8e/0xa3 [bonding]
 [<c02e6ee8>] unregister_pernet_gen_subsys+0x23/0x3d
 [<e0910baa>] bonding_exit+0x3a/0x66 [bonding]
 [<c015ccf3>] sys_delete_module+0x191/0x1f1
 [<c0147a30>] ? up_read+0x16/0x2a
 [<c0102a18>] ? restore_all_notrace+0x0/0x18
 [<c0353357>] ? do_page_fault+0x0/0x393
 [<c0102904>] sysenter_do_call+0x12/0x32
---[ end trace 8f3eaeee682a572c ]---

	Any thoughts?  I have not as yet investigated further.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [PATCH net-next 0/5] RDS: minor updates for net-next
From: Andrew Grover @ 2009-10-30 21:23 UTC (permalink / raw)
  To: David Miller; +Cc: andy.grover, netdev, rds-devel
In-Reply-To: <20091030.122326.73781593.davem@davemloft.net>

On Fri, Oct 30, 2009 at 12:23 PM, David Miller <davem@davemloft.net> wrote:
>> These patches fix some small issues with RDS, please review and
>> apply if ok.
> Adding new socket options is not appropriate for -rc* series
> submissions.
>
> The changes that fix races and panics are fine, the rest is not.
>
> Please respin your changes properly for net-2.6, and we can add the
> socket option and do the other stuff in net-next-2.6

I'm fine with it all going into net-next, actually. Even the fixes are
not for showstopper bugs, and RDS is not a widely-used protocol.

Thanks -- Regards -- Andy

^ permalink raw reply

* Re: [PATCH net-next 0/5] RDS: minor updates for net-next
From: David Miller @ 2009-10-30 22:06 UTC (permalink / raw)
  To: andy.grover; +Cc: andy.grover, netdev, rds-devel
In-Reply-To: <c0a09e5c0910301423k7726909frde1ae63e6ac9f009@mail.gmail.com>

From: Andrew Grover <andy.grover@gmail.com>
Date: Fri, 30 Oct 2009 14:23:13 -0700

> On Fri, Oct 30, 2009 at 12:23 PM, David Miller <davem@davemloft.net> wrote:
>>> These patches fix some small issues with RDS, please review and
>>> apply if ok.
>> Adding new socket options is not appropriate for -rc* series
>> submissions.
>>
>> The changes that fix races and panics are fine, the rest is not.
>>
>> Please respin your changes properly for net-2.6, and we can add the
>> socket option and do the other stuff in net-next-2.6
> 
> I'm fine with it all going into net-next, actually. Even the fixes are
> not for showstopper bugs, and RDS is not a widely-used protocol.

Okie dokie, I've tossed all 5 patches into net-next-2.6

Thanks!

^ permalink raw reply

* [net-next PATCH 0/4] qlge: Add ethtool self-test.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer

Changes for QLGE.

1) Add ethtool selftest.
2) Reduce debug print output.
3) Generic cleanup.




^ permalink raw reply

* [net-next PATCH 1/4] qlge: Add ethtool self-test.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>


Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h         |    6 ++
 drivers/net/qlge/qlge_ethtool.c |  112 +++++++++++++++++++++++++++++++++++++++
 drivers/net/qlge/qlge_main.c    |   20 +++++++-
 3 files changed, 137 insertions(+), 1 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 4b954e1..b9f65e0 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -1580,6 +1580,8 @@ enum {
 	QL_ALLMULTI = 6,
 	QL_PORT_CFG = 7,
 	QL_CAM_RT_SET = 8,
+	QL_SELFTEST = 9,
+	QL_LB_LINK_UP = 10,
 };
 
 /* link_status bit definitions */
@@ -1716,6 +1718,7 @@ struct ql_adapter {
 	struct completion ide_completion;
 	struct nic_operations *nic_ops;
 	u16 device_id;
+	atomic_t lb_count;
 };
 
 /*
@@ -1807,6 +1810,9 @@ int ql_mb_set_port_cfg(struct ql_adapter *qdev);
 int ql_wait_fifo_empty(struct ql_adapter *qdev);
 void ql_gen_reg_dump(struct ql_adapter *qdev,
 			struct ql_reg_dump *mpi_coredump);
+netdev_tx_t ql_lb_send(struct sk_buff *skb, struct net_device *ndev);
+void ql_check_lb_frame(struct ql_adapter *, struct sk_buff *);
+int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget);
 
 #if 1
 #define QL_ALL_DUMP
diff --git a/drivers/net/qlge/qlge_ethtool.c b/drivers/net/qlge/qlge_ethtool.c
index 62c4af0..058fa0a 100644
--- a/drivers/net/qlge/qlge_ethtool.c
+++ b/drivers/net/qlge/qlge_ethtool.c
@@ -36,6 +36,11 @@
 
 #include "qlge.h"
 
+static const char ql_gstrings_test[][ETH_GSTRING_LEN] = {
+	"Loopback test  (offline)"
+};
+#define QLGE_TEST_LEN (sizeof(ql_gstrings_test) / ETH_GSTRING_LEN)
+
 static int ql_update_ring_coalescing(struct ql_adapter *qdev)
 {
 	int i, status = 0;
@@ -251,6 +256,8 @@ static void ql_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 static int ql_get_sset_count(struct net_device *dev, int sset)
 {
 	switch (sset) {
+	case ETH_SS_TEST:
+		return QLGE_TEST_LEN;
 	case ETH_SS_STATS:
 		return ARRAY_SIZE(ql_stats_str_arr);
 	default:
@@ -429,6 +436,110 @@ static int ql_phys_id(struct net_device *ndev, u32 data)
 	return 0;
 }
 
+static int ql_start_loopback(struct ql_adapter *qdev)
+{
+	if (netif_carrier_ok(qdev->ndev)) {
+		set_bit(QL_LB_LINK_UP, &qdev->flags);
+		netif_carrier_off(qdev->ndev);
+	} else
+		clear_bit(QL_LB_LINK_UP, &qdev->flags);
+	qdev->link_config |= CFG_LOOPBACK_PCS;
+	return ql_mb_set_port_cfg(qdev);
+}
+
+static void ql_stop_loopback(struct ql_adapter *qdev)
+{
+	qdev->link_config &= ~CFG_LOOPBACK_PCS;
+	ql_mb_set_port_cfg(qdev);
+	if (test_bit(QL_LB_LINK_UP, &qdev->flags)) {
+		netif_carrier_on(qdev->ndev);
+		clear_bit(QL_LB_LINK_UP, &qdev->flags);
+	}
+}
+
+static void ql_create_lb_frame(struct sk_buff *skb,
+					unsigned int frame_size)
+{
+	memset(skb->data, 0xFF, frame_size);
+	frame_size &= ~1;
+	memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
+	memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
+	memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
+}
+
+void ql_check_lb_frame(struct ql_adapter *qdev,
+					struct sk_buff *skb)
+{
+	unsigned int frame_size = skb->len;
+
+	if ((*(skb->data + 3) == 0xFF) &&
+		(*(skb->data + frame_size / 2 + 10) == 0xBE) &&
+		(*(skb->data + frame_size / 2 + 12) == 0xAF)) {
+			atomic_dec(&qdev->lb_count);
+			return;
+	}
+}
+
+static int ql_run_loopback_test(struct ql_adapter *qdev)
+{
+	int i;
+	netdev_tx_t rc;
+	struct sk_buff *skb;
+	unsigned int size = SMALL_BUF_MAP_SIZE;
+
+	for (i = 0; i < 64; i++) {
+		skb = netdev_alloc_skb(qdev->ndev, size);
+		if (!skb)
+			return -ENOMEM;
+
+		skb->queue_mapping = 0;
+		skb_put(skb, size);
+		ql_create_lb_frame(skb, size);
+		rc = ql_lb_send(skb, qdev->ndev);
+		if (rc != NETDEV_TX_OK)
+			return -EPIPE;
+		atomic_inc(&qdev->lb_count);
+	}
+
+	ql_clean_lb_rx_ring(&qdev->rx_ring[0], 128);
+	return atomic_read(&qdev->lb_count) ? -EIO : 0;
+}
+
+static int ql_loopback_test(struct ql_adapter *qdev, u64 *data)
+{
+	*data = ql_start_loopback(qdev);
+	if (*data)
+		goto out;
+	*data = ql_run_loopback_test(qdev);
+out:
+	ql_stop_loopback(qdev);
+	return *data;
+}
+
+static void ql_self_test(struct net_device *ndev,
+				struct ethtool_test *eth_test, u64 *data)
+{
+	struct ql_adapter *qdev = netdev_priv(ndev);
+
+	if (netif_running(ndev)) {
+		set_bit(QL_SELFTEST, &qdev->flags);
+		if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
+			/* Offline tests */
+			if (ql_loopback_test(qdev, &data[0]))
+				eth_test->flags |= ETH_TEST_FL_FAILED;
+
+		} else {
+			/* Online tests */
+			data[0] = 0;
+		}
+		clear_bit(QL_SELFTEST, &qdev->flags);
+	} else {
+		QPRINTK(qdev, DRV, ERR,
+			"%s: is down, Loopback test will fail.\n", ndev->name);
+		eth_test->flags |= ETH_TEST_FL_FAILED;
+	}
+}
+
 static int ql_get_regs_len(struct net_device *ndev)
 {
 	return sizeof(struct ql_reg_dump);
@@ -575,6 +686,7 @@ const struct ethtool_ops qlge_ethtool_ops = {
 	.set_msglevel = ql_set_msglevel,
 	.get_link = ethtool_op_get_link,
 	.phys_id		 = ql_phys_id,
+	.self_test		 = ql_self_test,
 	.get_pauseparam		 = ql_get_pauseparam,
 	.set_pauseparam		 = ql_set_pauseparam,
 	.get_rx_csum = ql_get_rx_csum,
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index dd0ea02..9e4d3dc 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1680,6 +1680,13 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 		return;
 	}
 
+	/* loopback self test for ethtool */
+	if (test_bit(QL_SELFTEST, &qdev->flags)) {
+		ql_check_lb_frame(qdev, skb);
+		dev_kfree_skb_any(skb);
+		return;
+	}
+
 	prefetch(skb->data);
 	skb->dev = ndev;
 	if (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) {
@@ -2248,6 +2255,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 	return NETDEV_TX_OK;
 }
 
+
 static void ql_free_shadow_space(struct ql_adapter *qdev)
 {
 	if (qdev->rx_ring_shadow_reg_area) {
@@ -4173,7 +4181,6 @@ err_out:
 	return err;
 }
 
-
 static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_open		= qlge_open,
 	.ndo_stop		= qlge_close,
@@ -4242,10 +4249,21 @@ static int __devinit qlge_probe(struct pci_dev *pdev,
 	}
 	ql_link_off(qdev);
 	ql_display_dev_info(ndev);
+	atomic_set(&qdev->lb_count, 0);
 	cards_found++;
 	return 0;
 }
 
+netdev_tx_t ql_lb_send(struct sk_buff *skb, struct net_device *ndev)
+{
+	return qlge_send(skb, ndev);
+}
+
+int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget)
+{
+	return ql_clean_inbound_rx_ring(rx_ring, budget);
+}
+
 static void __devexit qlge_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 2/4] qlge: Change naming on vlan API.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>

Change name on vlan_rx_add, kill, register to match other driver API.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 9e4d3dc..01f78a9 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1985,7 +1985,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
-static void ql_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
+static void qlge_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 
@@ -2001,7 +2001,7 @@ static void ql_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
 	}
 }
 
-static void ql_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
+static void qlge_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	u32 enable_bit = MAC_ADDR_E;
@@ -2017,7 +2017,7 @@ static void ql_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
 	ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 }
 
-static void ql_vlan_rx_kill_vid(struct net_device *ndev, u16 vid)
+static void qlge_vlan_rx_kill_vid(struct net_device *ndev, u16 vid)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	u32 enable_bit = 0;
@@ -4191,9 +4191,9 @@ static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_set_mac_address	= qlge_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_tx_timeout		= qlge_tx_timeout,
-	.ndo_vlan_rx_register	= ql_vlan_rx_register,
-	.ndo_vlan_rx_add_vid	= ql_vlan_rx_add_vid,
-	.ndo_vlan_rx_kill_vid	= ql_vlan_rx_kill_vid,
+	.ndo_vlan_rx_register	= qlge_vlan_rx_register,
+	.ndo_vlan_rx_add_vid	= qlge_vlan_rx_add_vid,
+	.ndo_vlan_rx_kill_vid	= qlge_vlan_rx_kill_vid,
 };
 
 static int __devinit qlge_probe(struct pci_dev *pdev,
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 3/4] qlge: Reduce debug print output.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>


Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |   12 +++++
 drivers/net/qlge/qlge_main.c |  101 ++++++++++++++++++++++-------------------
 drivers/net/qlge/qlge_mpi.c  |   14 +++---
 3 files changed, 73 insertions(+), 54 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index b9f65e0..502c3af 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -27,6 +27,18 @@
 		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
 			   "%s: " fmt, __func__, ##args);  \
        } while (0)
+#if 0
+#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	\
+	do {	\
+		if (!((qdev)->msg_enable & NETIF_MSG_##nlevel))	\
+			;					\
+		else						\
+			dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
+					"%s: " fmt, __func__, ##args);  \
+	} while (0)
+#else
+#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
+#endif
 
 #define WQ_ADDR_ALIGN	0x3	/* 4 byte alignment */
 
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 01f78a9..57bbb90 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -359,7 +359,7 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
 			    (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) |
 			    (addr[5]);
 
-			QPRINTK(qdev, IFUP, DEBUG,
+			QPRINTK_DBG(qdev, IFUP, DEBUG,
 				"Adding %s address %pM"
 				" at index %d in the CAM.\n",
 				((type ==
@@ -414,7 +414,8 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
 			 * addressing. It's either MAC_ADDR_E on or off.
 			 * That's bit-27 we're talking about.
 			 */
-			QPRINTK(qdev, IFUP, INFO, "%s VLAN ID %d %s the CAM.\n",
+			QPRINTK_DBG(qdev, IFUP, INFO,
+				"%s VLAN ID %d %s the CAM.\n",
 				(enable_bit ? "Adding" : "Removing"),
 				index, (enable_bit ? "to" : "from"));
 
@@ -522,9 +523,9 @@ static int ql_set_routing_reg(struct ql_adapter *qdev, u32 index, u32 mask,
 	int status = -EINVAL; /* Return error if no mask match. */
 	u32 value = 0;
 
-	QPRINTK(qdev, IFUP, DEBUG,
-		"%s %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s mask %s the routing reg.\n",
-		(enable ? "Adding" : "Removing"),
+	QPRINTK_DBG(qdev, IFUP, DEBUG,
+		"%s %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s mask %s the routing "
+		"reg.\n", (enable ? "Adding" : "Removing"),
 		((index == RT_IDX_ALL_ERR_SLOT) ? "MAC ERROR/ALL ERROR" : ""),
 		((index == RT_IDX_IP_CSUM_ERR_SLOT) ? "IP CSUM ERROR" : ""),
 		((index ==
@@ -959,8 +960,9 @@ static int ql_8012_port_initialize(struct ql_adapter *qdev)
 		/* Another function has the semaphore, so
 		 * wait for the port init bit to come ready.
 		 */
-		QPRINTK(qdev, LINK, INFO,
-			"Another function has the semaphore, so wait for the port init bit to come ready.\n");
+		QPRINTK_DBG(qdev, LINK, INFO,
+			"Another function has the semaphore, so wait for the "
+			"port init bit to come ready.\n");
 		status = ql_wait_reg_rdy(qdev, STS, qdev->port_init, 0);
 		if (status) {
 			QPRINTK(qdev, LINK, CRIT,
@@ -969,7 +971,7 @@ static int ql_8012_port_initialize(struct ql_adapter *qdev)
 		return status;
 	}
 
-	QPRINTK(qdev, LINK, INFO, "Got xgmac semaphore!.\n");
+	QPRINTK_DBG(qdev, LINK, INFO, "Got xgmac semaphore!.\n");
 	/* Set the core reset. */
 	status = ql_read_xgmac_reg(qdev, GLOBAL_CFG, &data);
 	if (status)
@@ -1148,7 +1150,7 @@ static void ql_update_lbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 
 	while (rx_ring->lbq_free_cnt > 32) {
 		for (i = 0; i < 16; i++) {
-			QPRINTK(qdev, RX_STATUS, DEBUG,
+			QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 				"lbq: try cleaning clean_idx = %d.\n",
 				clean_idx);
 			lbq_desc = &rx_ring->lbq[clean_idx];
@@ -1181,7 +1183,7 @@ static void ql_update_lbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 	}
 
 	if (start_idx != clean_idx) {
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"lbq: updating prod idx = %d.\n",
 			rx_ring->lbq_prod_idx);
 		ql_write_db_reg(rx_ring->lbq_prod_idx,
@@ -1201,7 +1203,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 	while (rx_ring->sbq_free_cnt > 16) {
 		for (i = 0; i < 16; i++) {
 			sbq_desc = &rx_ring->sbq[clean_idx];
-			QPRINTK(qdev, RX_STATUS, DEBUG,
+			QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 				"sbq: try cleaning clean_idx = %d.\n",
 				clean_idx);
 			if (sbq_desc->p.skb == NULL) {
@@ -1247,7 +1249,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 	}
 
 	if (start_idx != clean_idx) {
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"sbq: updating prod idx = %d.\n",
 			rx_ring->sbq_prod_idx);
 		ql_write_db_reg(rx_ring->sbq_prod_idx,
@@ -1281,7 +1283,7 @@ static void ql_unmap_send(struct ql_adapter *qdev,
 			 * then its an OAL.
 			 */
 			if (i == 7) {
-				QPRINTK(qdev, TX_DONE, DEBUG,
+				QPRINTK_DBG(qdev, TX_DONE, DEBUG,
 					"unmapping OAL area.\n");
 			}
 			pci_unmap_single(qdev->pdev,
@@ -1291,8 +1293,8 @@ static void ql_unmap_send(struct ql_adapter *qdev,
 						       maplen),
 					 PCI_DMA_TODEVICE);
 		} else {
-			QPRINTK(qdev, TX_DONE, DEBUG, "unmapping frag %d.\n",
-				i);
+			QPRINTK_DBG(qdev, TX_DONE, DEBUG,
+					"unmapping frag %d.\n",	i);
 			pci_unmap_page(qdev->pdev,
 				       pci_unmap_addr(&tx_ring_desc->map[i],
 						      mapaddr),
@@ -1316,9 +1318,9 @@ static int ql_map_send(struct ql_adapter *qdev,
 	struct tx_buf_desc *tbd = mac_iocb_ptr->tbd;
 	int frag_cnt = skb_shinfo(skb)->nr_frags;
 
-	if (frag_cnt) {
-		QPRINTK(qdev, TX_QUEUED, DEBUG, "frag_cnt = %d.\n", frag_cnt);
-	}
+	if (frag_cnt)
+		QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "frag_cnt = %d.\n",
+				frag_cnt);
 	/*
 	 * Map the skb buffer first.
 	 */
@@ -1857,7 +1859,7 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 	/* While there are entries in the completion queue. */
 	while (prod != rx_ring->cnsmr_idx) {
 
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"cq_id = %d, prod = %d, cnsmr = %d.\n.", rx_ring->cq_id,
 			prod, rx_ring->cnsmr_idx);
 
@@ -1871,7 +1873,8 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 			break;
 		default:
 			QPRINTK(qdev, RX_STATUS, DEBUG,
-				"Hit default case, not handled! dropping the packet, opcode = %x.\n",
+				"Hit default case, not handled!	"
+				"dropping the packet, opcode = %x.\n",
 				net_rsp->opcode);
 		}
 		count++;
@@ -1904,7 +1907,7 @@ static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
 	/* While there are entries in the completion queue. */
 	while (prod != rx_ring->cnsmr_idx) {
 
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"cq_id = %d, prod = %d, cnsmr = %d.\n.", rx_ring->cq_id,
 			prod, rx_ring->cnsmr_idx);
 
@@ -1922,11 +1925,10 @@ static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
 						net_rsp);
 			break;
 		default:
-			{
-				QPRINTK(qdev, RX_STATUS, DEBUG,
-					"Hit default case, not handled! dropping the packet, opcode = %x.\n",
+			QPRINTK(qdev, RX_STATUS, DEBUG,
+				"Hit default case, not handled!	"
+				"dropping the packet, opcode = %x.\n",
 					net_rsp->opcode);
-			}
 		}
 		count++;
 		ql_update_cq(rx_ring);
@@ -1947,7 +1949,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 	int i, work_done = 0;
 	struct intr_context *ctx = &qdev->intr_context[rx_ring->cq_id];
 
-	QPRINTK(qdev, RX_STATUS, DEBUG, "Enter, NAPI POLL cq_id = %d.\n",
+	QPRINTK_DBG(qdev, RX_STATUS, DEBUG, "Enter, NAPI POLL cq_id = %d.\n",
 		rx_ring->cq_id);
 
 	/* Service the TX rings first.  They start
@@ -1960,7 +1962,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 		if ((ctx->irq_mask & (1 << trx_ring->cq_id)) &&
 			(ql_read_sh_reg(trx_ring->prod_idx_sh_reg) !=
 					trx_ring->cnsmr_idx)) {
-			QPRINTK(qdev, INTR, DEBUG,
+			QPRINTK_DBG(qdev, INTR, DEBUG,
 				"%s: Servicing TX completion ring %d.\n",
 				__func__, trx_ring->cq_id);
 			ql_clean_outbound_rx_ring(trx_ring);
@@ -1972,7 +1974,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 	 */
 	if (ql_read_sh_reg(rx_ring->prod_idx_sh_reg) !=
 					rx_ring->cnsmr_idx) {
-		QPRINTK(qdev, INTR, DEBUG,
+		QPRINTK_DBG(qdev, INTR, DEBUG,
 			"%s: Servicing RX completion ring %d.\n",
 			__func__, rx_ring->cq_id);
 		work_done = ql_clean_inbound_rx_ring(rx_ring, budget);
@@ -1991,11 +1993,12 @@ static void qlge_vlan_rx_register(struct net_device *ndev, struct vlan_group *gr
 
 	qdev->vlgrp = grp;
 	if (grp) {
-		QPRINTK(qdev, IFUP, DEBUG, "Turning on VLAN in NIC_RCV_CFG.\n");
+		QPRINTK_DBG(qdev, IFUP, DEBUG,
+					"Turning on VLAN in NIC_RCV_CFG.\n");
 		ql_write32(qdev, NIC_RCV_CFG, NIC_RCV_CFG_VLAN_MASK |
 			   NIC_RCV_CFG_VLAN_MATCH_AND_NON);
 	} else {
-		QPRINTK(qdev, IFUP, DEBUG,
+		QPRINTK_DBG(qdev, IFUP, DEBUG,
 			"Turning off VLAN in NIC_RCV_CFG.\n");
 		ql_write32(qdev, NIC_RCV_CFG, NIC_RCV_CFG_VLAN_MASK);
 	}
@@ -2058,7 +2061,7 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
 
 	spin_lock(&qdev->hw_lock);
 	if (atomic_read(&qdev->intr_context[0].irq_cnt)) {
-		QPRINTK(qdev, INTR, DEBUG, "Shared Interrupt, Not ours!\n");
+		QPRINTK_DBG(qdev, INTR, DEBUG, "Shared Interrupt, Not ours!\n");
 		spin_unlock(&qdev->hw_lock);
 		return IRQ_NONE;
 	}
@@ -2102,8 +2105,7 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
 	 */
 	var = ql_read32(qdev, ISR1);
 	if (var & intr_context->irq_mask) {
-				QPRINTK(qdev, INTR, INFO,
-			"Waking handler for rx_ring[0].\n");
+		QPRINTK_DBG(qdev, INTR, INFO, "Waking handler for rx_ring[0].\n");
 		ql_disable_completion_interrupt(qdev, intr_context->intr);
 					napi_schedule(&rx_ring->napi);
 				work_done++;
@@ -2200,9 +2202,9 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 		return NETDEV_TX_OK;
 
 	if (unlikely(atomic_read(&tx_ring->tx_count) < 2)) {
-		QPRINTK(qdev, TX_QUEUED, INFO,
-			"%s: shutting down tx queue %d du to lack of resources.\n",
-			__func__, tx_ring_idx);
+		QPRINTK_DBG(qdev, TX_QUEUED, INFO,
+			"shutting down tx queue %d du to lack of resources.\n",
+			tx_ring_idx);
 		netif_stop_subqueue(ndev, tx_ring->wq_id);
 		atomic_inc(&tx_ring->queue_stopped);
 		return NETDEV_TX_BUSY;
@@ -2222,7 +2224,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 	mac_iocb_ptr->frame_len = cpu_to_le16((u16) skb->len);
 
 	if (qdev->vlgrp && vlan_tx_tag_present(skb)) {
-		QPRINTK(qdev, TX_QUEUED, DEBUG, "Adding a vlan tag %d.\n",
+		QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "Adding a vlan tag %d.\n",
 			vlan_tx_tag_get(skb));
 		mac_iocb_ptr->flags3 |= OB_MAC_IOCB_V;
 		mac_iocb_ptr->vlan_tci = cpu_to_le16(vlan_tx_tag_get(skb));
@@ -2248,7 +2250,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 	wmb();
 
 	ql_write_db_reg(tx_ring->prod_idx, tx_ring->prod_idx_db_reg);
-	QPRINTK(qdev, TX_QUEUED, DEBUG, "tx queued, slot %d, len %d\n",
+	QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "tx queued, slot %d, len %d\n",
 		tx_ring->prod_idx, skb->len);
 
 	atomic_dec(&tx_ring->tx_count);
@@ -2783,10 +2785,10 @@ static int ql_start_rx_ring(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 		cqicb->pkt_delay = cpu_to_le16(qdev->rx_max_coalesced_frames);
 		break;
 	default:
-		QPRINTK(qdev, IFUP, DEBUG, "Invalid rx_ring->type = %d.\n",
+		QPRINTK_DBG(qdev, IFUP, DEBUG, "Invalid rx_ring->type = %d.\n",
 			rx_ring->type);
 	}
-	QPRINTK(qdev, IFUP, DEBUG, "Initializing rx work queue.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Initializing rx work queue.\n");
 	err = ql_write_cfg(qdev, cqicb, sizeof(struct cqicb),
 			   CFG_LCQ, rx_ring->cq_id);
 	if (err) {
@@ -2839,7 +2841,7 @@ static int ql_start_tx_ring(struct ql_adapter *qdev, struct tx_ring *tx_ring)
 		QPRINTK(qdev, IFUP, ERR, "Failed to load tx_ring.\n");
 		return err;
 	}
-	QPRINTK(qdev, IFUP, DEBUG, "Successfully loaded WQICB.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Successfully loaded WQICB.\n");
 	return err;
 }
 
@@ -3088,11 +3090,11 @@ static void ql_free_irq(struct ql_adapter *qdev)
 			if (test_bit(QL_MSIX_ENABLED, &qdev->flags)) {
 				free_irq(qdev->msi_x_entry[i].vector,
 					 &qdev->rx_ring[i]);
-				QPRINTK(qdev, IFDOWN, DEBUG,
+				QPRINTK_DBG(qdev, IFDOWN, DEBUG,
 					"freeing msix interrupt %d.\n", i);
 			} else {
 				free_irq(qdev->pdev->irq, &qdev->rx_ring[0]);
-				QPRINTK(qdev, IFDOWN, DEBUG,
+				QPRINTK_DBG(qdev, IFDOWN, DEBUG,
 					"freeing msi interrupt %d.\n", i);
 			}
 		}
@@ -3200,14 +3202,14 @@ static int ql_start_rss(struct ql_adapter *qdev)
 	memcpy((void *)&ricb->ipv6_hash_key[0], init_hash_seed, 40);
 	memcpy((void *)&ricb->ipv4_hash_key[0], init_hash_seed, 16);
 
-	QPRINTK(qdev, IFUP, DEBUG, "Initializing RSS.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Initializing RSS.\n");
 
 	status = ql_write_cfg(qdev, ricb, sizeof(*ricb), CFG_LR, 0);
 	if (status) {
 		QPRINTK(qdev, IFUP, ERR, "Failed to load RICB.\n");
 		return status;
 	}
-	QPRINTK(qdev, IFUP, DEBUG, "Successfully loaded RICB.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Successfully loaded RICB.\n");
 	return status;
 }
 
@@ -3679,7 +3681,7 @@ static int ql_configure_rings(struct ql_adapter *qdev)
 			rx_ring->lbq_size =
 			    rx_ring->lbq_len * sizeof(__le64);
 			rx_ring->lbq_buf_size = (u16)lbq_buf_len;
-			QPRINTK(qdev, IFUP, DEBUG,
+			QPRINTK_DBG(qdev, IFUP, DEBUG,
 				"lbq_buf_size %d, order = %d\n",
 				rx_ring->lbq_buf_size, qdev->lbq_buf_order);
 			rx_ring->sbq_len = NUM_SMALL_BUFFERS;
@@ -3917,8 +3919,13 @@ static int qlge_set_mac_address(struct net_device *ndev, void *p)
 	if (netif_running(ndev))
 		return -EBUSY;
 
-	if (!is_valid_ether_addr(addr->sa_data))
+	if (!is_valid_ether_addr(addr->sa_data)) {
+		QPRINTK_DBG(qdev, DRV, ERR,
+			"Invalid Mac addr %02x:%02x:%02x:%02x:%02x:%02x\n",
+			addr->sa_data[0], addr->sa_data[1], addr->sa_data[2],
+			addr->sa_data[3], addr->sa_data[4], addr->sa_data[5]);
 		return -EADDRNOTAVAIL;
+	}
 	memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
 
 	status = ql_sem_spinlock(qdev, SEM_MAC_ADDR_MASK);
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 80b6853..0a26055 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -207,7 +207,7 @@ static void ql_link_up(struct ql_adapter *qdev, struct mbox_params *mbcp)
 	 * to our liking.
 	 */
 	if (!test_bit(QL_PORT_CFG, &qdev->flags)) {
-		QPRINTK(qdev, DRV, ERR, "Queue Port Config Worker!\n");
+		QPRINTK_DBG(qdev, DRV, DEBUG, "Queue Port Config Worker!\n");
 		set_bit(QL_PORT_CFG, &qdev->flags);
 		/* Begin polled mode early so
 		 * we don't get another interrupt
@@ -277,7 +277,7 @@ static int ql_aen_lost(struct ql_adapter *qdev, struct mbox_params *mbcp)
 		int i;
 		QPRINTK(qdev, DRV, ERR, "Lost AEN detected.\n");
 		for (i = 0; i < mbcp->out_count; i++)
-			QPRINTK(qdev, DRV, ERR, "mbox_out[%d] = 0x%.08x.\n",
+			QPRINTK_DBG(qdev, DRV, ERR, "mbox_out[%d] = 0x%.08x.\n",
 					i, mbcp->mbox_out[i]);
 
 	}
@@ -658,7 +658,7 @@ int ql_mb_set_port_cfg(struct ql_adapter *qdev)
 		return status;
 
 	if (mbcp->mbox_out[0] == MB_CMD_STS_INTRMDT) {
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Port Config sent, wait for IDC.\n");
 	} else	if (mbcp->mbox_out[0] != MB_CMD_STS_GOOD) {
 		QPRINTK(qdev, DRV, ERR,
@@ -694,7 +694,7 @@ int ql_mb_get_port_cfg(struct ql_adapter *qdev)
 			"Failed Get Port Configuration.\n");
 		status = -EIO;
 	} else	{
-		QPRINTK(qdev, DRV, DEBUG,
+		QPRINTK_DBG(qdev, DRV, DEBUG,
 			"Passed Get Port Configuration.\n");
 		qdev->link_config = mbcp->mbox_out[1];
 		qdev->max_frame_size = mbcp->mbox_out[2];
@@ -898,7 +898,7 @@ int ql_mb_set_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 control)
 		return status;
 
 	if (mbcp->mbox_out[0] == MB_CMD_STS_INVLD_CMD) {
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Command not supported by firmware.\n");
 		status = -EINVAL;
 	} else if (mbcp->mbox_out[0] == MB_CMD_STS_ERR) {
@@ -906,7 +906,7 @@ int ql_mb_set_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 control)
 		 * already in the state we are trying to
 		 * change it to.
 		 */
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Command parameters make no change.\n");
 	}
 	return status;
@@ -937,7 +937,7 @@ static int ql_mb_get_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 *control)
 	}
 
 	if (mbcp->mbox_out[0] == MB_CMD_STS_INVLD_CMD) {
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Command not supported by firmware.\n");
 		status = -EINVAL;
 	} else if (mbcp->mbox_out[0] == MB_CMD_STS_ERR) {
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 4/4] qlge: Fix indentations.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>


Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 57bbb90..224fb7c 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -2107,9 +2107,9 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
 	if (var & intr_context->irq_mask) {
 		QPRINTK_DBG(qdev, INTR, INFO, "Waking handler for rx_ring[0].\n");
 		ql_disable_completion_interrupt(qdev, intr_context->intr);
-					napi_schedule(&rx_ring->napi);
-				work_done++;
-			}
+		napi_schedule(&rx_ring->napi);
+		work_done++;
+	}
 	ql_enable_completion_interrupt(qdev, intr_context->intr);
 	return work_done ? IRQ_HANDLED : IRQ_NONE;
 }
-- 
1.6.0.2


^ permalink raw reply related

* Re: [net-next PATCH 3/4] qlge: Reduce debug print output.
From: Joe Perches @ 2009-10-30 22:44 UTC (permalink / raw)
  To: Ron Mercer; +Cc: davem, netdev
In-Reply-To: <1256940816-27540-4-git-send-email-ron.mercer@qlogic.com>

On Fri, 2009-10-30 at 15:13 -0700, Ron Mercer wrote:
> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
[]
> diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
> index b9f65e0..502c3af 100644
> --- a/drivers/net/qlge/qlge.h
> +++ b/drivers/net/qlge/qlge.h
> @@ -27,6 +27,18 @@
>  		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
>  			   "%s: " fmt, __func__, ##args);  \
>         } while (0)
> +#if 0
> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	\
> +	do {	\
> +		if (!((qdev)->msg_enable & NETIF_MSG_##nlevel))	\
> +			;					\
> +		else						\
> +			dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
> +					"%s: " fmt, __func__, ##args);  \
> +	} while (0)
> +#else
> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
> +#endif

This uses an inverted test and it doesn't verify the args to
dev_printk when not #defined.

How about:

#ifdef DEBUG
#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	do {	\
	if ((qdev)->msg_enable & NETIF_MSG_##nlevel)		\
		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
			   "%s: " fmt, __func__, ##args);	\
} while (0)
#else
#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	do {	\
	if (0 && (qdev)->msg_enable & NETIF_MSG_##nlevel)	\
		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
			   "%s: " fmt, __func__, ##args);	\
} while (0)
#endif



^ permalink raw reply

* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric W. Biederman @ 2009-10-30 22:57 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: David Miller, netdev
In-Reply-To: <21200.1256937150@death.nxdomain.ibm.com>

Jay Vosburgh <fubar@us.ibm.com> writes:

> David Miller <davem@davemloft.net> wrote:
>
>>From: ebiederm@xmission.com (Eric W. Biederman)
>>Date: Thu, 29 Oct 2009 17:16:54 -0700
>>
>>> I recently had it pointed out to me that the bonding driver does not
>>> work in a network namespace.  So I have simplified the bonding driver
>>> a bit, added support for ip link add and ip link del, and finally made
>>> the bonding driver work in multiple network namespaces.
>>> 
>>> The most note worthy change in the patchset is the addition of support
>>> in the networking core for registering a sysfs group for a device.
>>> 
>>> Using this in the bonding driver simplifies the code and removes a
>>> userspace race between actions triggered by the netlink event and the
>>> bonding sysfs attributes appearing.
>>
>>I've tossed patches 1-7 into net-next-2.6, thanks Eric.
>
> 	I put patches 1-7 on a recent net-next-2.6, and from a simple
> "insmod bonding.ko; rmmod bonding" I'm seeing the following:
>
> ------------[ cut here ]------------
> WARNING: at fs/proc/generic.c:847 remove_proc_entry+0x1a8/0x1c7()
> Hardware name: IBM eserver xSeries 220 -[8645]-
> remove_proc_entry: removing non-empty directory 'net/bonding', leaking at least 
> 'bond0'
> Modules linked in: bonding(-) ipv6 microcode loop ppdev sworks_agp parport_pc tg
> 3 e100 agpgart parport mii libphy e1000 edd pata_serverworks [last unloaded: spe
> edstep_lib]
> Pid: 6216, comm: rmmod Not tainted 2.6.32-rc3-devel #19
> Call Trace:
>  [<c012ec9d>] warn_slowpath_common+0x60/0x90
>  [<c012ed01>] warn_slowpath_fmt+0x24/0x27
>  [<c01e3e55>] remove_proc_entry+0x1a8/0x1c7
>  [<e0906435>] ? bond_net_exit+0x0/0xa3 [bonding]
>  [<e09064c3>] bond_net_exit+0x8e/0xa3 [bonding]
>  [<c02e6ee8>] unregister_pernet_gen_subsys+0x23/0x3d
>  [<e0910baa>] bonding_exit+0x3a/0x66 [bonding]
>  [<c015ccf3>] sys_delete_module+0x191/0x1f1
>  [<c0147a30>] ? up_read+0x16/0x2a
>  [<c0102a18>] ? restore_all_notrace+0x0/0x18
>  [<c0353357>] ? do_page_fault+0x0/0x393
>  [<c0102904>] sysenter_do_call+0x12/0x32
> ---[ end trace 8f3eaeee682a572c ]---
>
> 	Any thoughts?  I have not as yet investigated further.

Weird.

We have already run:
rtnl_link_unregister. 
  rtnl_kill_links
    dellink(bond0)
      unregister_netdevice(bond0)
          bond_uninit
             bond_remove_proc_entry


So the proc entry should no longer be there.  I'm a little nervous about
the new unregister_netdevice_many but I don't see any obvious problems with
that code.

Were there by any chance any earlier errors that could have prevented the uninit?
You weren't inserting multiple copies of the bonding driver?

Eric

^ permalink raw reply

* Re: [PATCH net-next-2.6] veth: Fix veth_dellink method
From: Eric W. Biederman @ 2009-10-30 23:07 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
In-Reply-To: <4AEA8F0F.4030100@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> In commit 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b
> (net: add a list_head parameter to dellink() method),
> I forgot to actually use this parameter in veth_dellink.
>
> I remember feeling a bit uncomfortable about veth_close(),
> because it does :
>
> netif_carrier_off(dev);
> netif_carrier_off(priv->peer);
>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index ffb502d..9bed694 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -450,8 +450,8 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
>  	priv = netdev_priv(dev);
>  	peer = priv->peer;
>  
> -	unregister_netdevice(dev);
> -	unregister_netdevice(peer);
> +	unregister_netdevice_queue(dev, head);
> +	unregister_netdevice_queue(peer, head);

Unless I am mistaken you need to change the list_add_tail to
list_move_tail in unregister_netdevice_queue because we will be
adding each veth device twice.

Eric

^ permalink raw reply

* Re: Connection tracking and vlan
From: Eric W. Biederman @ 2009-10-30 23:15 UTC (permalink / raw)
  To: Adayadil Thomas; +Cc: Eric Dumazet, Herbert Xu, netdev, Patrick McHardy
In-Reply-To: <fb7befa20910301220y53e43608u3b37de17661feb4e@mail.gmail.com>

Adayadil Thomas <adayadil.thomas@gmail.com> writes:

> On Fri, Oct 30, 2009 at 11:31 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Very strange, this question about vlan looks like discussion we had
>> yesterday (or the day before...) about interfaces (versus packet defragmentation)
>>
>> "IP conntracking" is about IP, and [V]LAN doesnt matter at all at this protocol level.
>>
>> Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
>> include interface name/index
>
> I am concerned about the following situation -
>
> The linux device is configured as a bridge and is deployed between the
> trunk ports
> of 2 switches. In this situation the linux device will be seeing 802.1q packets
> with vlan headers specifying the vlanids.
>
> It is valid to have an environment where private IP addresses are duplicated
> on different virtual LANs. i.e. it is valid to have machine A (
> 10.10.10.1) talking to
> machine B (10.10.10.2) on vlan 1,
> and
> at the same time machine C ( 10.10.10.1) talking to machine D
> (10.10.10.2) on vlan 2.
>
> Since they are on different LANs (VLANs), there should not be any issues.
>
> Now when VLANs are shared across switches, the trunk port will sent
> 802.1q tagged
> packets between the switches. Imagine these packets when going through
> the linux bridge.
> The 802.1q header should identify the separate vlans by the vlan id.
>
> If ip_conntrack does not consider vlans, it is possible that all 5
> tuple are the same
> and thus affect the connection tracking.
>
> I hope I have described the scenario well. If not I can explain in a
> more detailed fashion.

Unless you have multiple network namespaces linux assumes all packets are
in the same ip space.  And 10.10.10.1 is the same machine no matter
which interface you talk to it on.

Eric

^ permalink raw reply

* Re: Connection tracking and vlan
From: Ben Greear @ 2009-10-30 23:25 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Adayadil Thomas, Eric Dumazet, Herbert Xu, netdev,
	Patrick McHardy
In-Reply-To: <m1639wmq1f.fsf@fess.ebiederm.org>

On 10/30/2009 04:15 PM, Eric W. Biederman wrote:

>> If ip_conntrack does not consider vlans, it is possible that all 5
>> tuple are the same
>> and thus affect the connection tracking.
>>
>> I hope I have described the scenario well. If not I can explain in a
>> more detailed fashion.
>
> Unless you have multiple network namespaces linux assumes all packets are
> in the same ip space.  And 10.10.10.1 is the same machine no matter
> which interface you talk to it on.

It only takes a relatively small patch that lets conn-track hash on a
skb->foo_mark, and allow that mark to be set on incoming packets
based on netdevice or whatever, (before the conn-track lookup is
done).

This is logically somewhat similar to using multiple routing
tables and has been working well for me for several years....

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Eric W. Biederman @ 2009-10-30 23:25 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <20091030143527.GA3141@kvack.org>

Benjamin LaHaise <bcrl@lhnet.ca> writes:

> On Thu, Oct 29, 2009 at 06:45:32PM -0700, Eric W. Biederman wrote:
>> The reason for the existence of sysfs_dirent is as things grow larger
>> we want to keep the amount of RAM consumed down.  So we don't pin
>> everything in the dcache.  So we try and keep the amount of memory
>> consumed down.
>
> I'm aware of that, but for users running into this sort of scaling issue, 
> the amount of RAM required is a non-issue (30,000 interfaces require about 
> 1GB of RAM at present), making the question more one of how to avoid the 
> overhead for users who don't require it.  I'd prefer a config option.  The 
> only way I can really see saving memory usage is to somehow tie sysfs dirent 
> lookups into the network stack's own tables for looking up device entries.  
> The network stack already has to cope with this kind of scaling, and that 
> would save the RAM.

There is that.  I'm trying to figure out how to add the improvements
without making sysfs_dirent larger.  Which I think that is doable.

>> So I would like to see how much we can par down.
>
>> For dealing with seeks in the middle of readdir I expect the best way
>> to do that is to be inspired by htrees in extNfs and return a hash of
>> the filename as our position, and keep the filename list sorted by
>> that hash.  Since we are optimizing for size we don't need to store
>> that hash.  Then we can turn that list into a some flavor of sorted
>> binary tree.
>
> readdir() generally isn't an issue at present.

Supporting seekdir into the middle of a directory is the entire reason
I keep the entries sorted by inode.  If we sort by a hash of the name.
We can use the hash to support directory position in readdir and seekdir.
And we can completely remove the linear list when the rb_tree is introduced.

>> I'm surprised sysfs_count_nlink shows up, as it is not directly on the
>> add or remove path.  I think the answer there is to change s_flags
>> into a set of bitfields and make link_count one of them, perhaps
>> 16bits long.  If we ever overflow our bitfield we can just set link
>> count to 0, and userspace (aka find) will know it can't optimized
>> based on link count.
>
> It shows up because of the bits of userspace (udev) touching the directory 
> from things like the hotplug code path.

I realized after sending the message that s_mode in sysfs_dirent is a
real size offense.  It is a 16bit field packed in between two longs.
So in practice it is possible to move the s_mode  up next to s_flags
and add a s_nlink after it both unsigned short and get a cheap sysfs_nlink.

>> I was expecting someone to run into problems with the linear directory
>> of sysfs someday.
>
> Alas, sysfs isn't the only offender.

Agreed. Sysfs is probably the easiest to untangle.

Since I'm not quite ready to post my patches.  I will briefly
mention what I have in my queue and hopefully get things posted.

I have changes to make it so that sysfs never has to go from
the sysfs_dirent to the sysfs inode.  

I have changes to sys_sysctl() so that it becomes a filesystem lookup
under /proc/sys.  Which ultimately makes the code easier to maintain
and debug.

Now back to getting things forward ported and ready to post.

Eric

^ permalink raw reply

* Re: [RFC, PATCH] net: suspicious test in dev_change_name()
From: Jarek Poplawski @ 2009-10-30 23:50 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AEAAFC4.9050309@gmail.com>

Eric Dumazet wrote, On 10/30/2009 10:20 AM:

> While preparing a patch for net-next-2.6, I noticed following code in dev_change_name()
> 
> int err = 0;
> ...
>         ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
>         ret = notifier_to_errno(ret);
> 
>         if (ret) {
> << HERE >>      if (err) {
>                         printk(KERN_ERR
>                                "%s: name change rollback failed: %d.\n",
>                                dev->name, ret);
>                 } else {
>                         err = ret;
>                         memcpy(dev->name, oldname, IFNAMSIZ);
>                         goto rollback;
>                 }
>         }
> 
> 
> It seems intent was to test if notifier_to_errno() was null ?


I don't think so: err stores the previous ret meaning rollback and
is checked for this later. But somebody forgot err can store previous
(positive) value here, so IMHO you're right: there is a bug in this
place ;-)

Jarek P.

> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b8f74cf..029cd41 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -939,9 +939,9 @@ rollback:
>  	write_unlock_bh(&dev_base_lock);
>  
>  	ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
> -	ret = notifier_to_errno(ret);
>  
>  	if (ret) {
> +		err = notifier_to_errno(ret);
>  		if (err) {
>  			printk(KERN_ERR
>  			       "%s: name change rollback failed: %d.\n",
> --
> 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
> 



^ permalink raw reply

* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Benjamin LaHaise @ 2009-10-30 23:53 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <m1my38lb0f.fsf@fess.ebiederm.org>

On Fri, Oct 30, 2009 at 04:25:52PM -0700, Eric W. Biederman wrote:
> I realized after sending the message that s_mode in sysfs_dirent is a
> real size offense.  It is a 16bit field packed in between two longs.
> So in practice it is possible to move the s_mode  up next to s_flags
> and add a s_nlink after it both unsigned short and get a cheap sysfs_nlink.

That doesn't work -- the number of directory entries can easily exceed 65535.  
Current mid range hardware is good enough to terminate 100,000 network 
interfaces on a single host.

> Since I'm not quite ready to post my patches.  I will briefly
> mention what I have in my queue and hopefully get things posted.
> 
> I have changes to make it so that sysfs never has to go from
> the sysfs_dirent to the sysfs inode.  

Ah, interesting.

> I have changes to sys_sysctl() so that it becomes a filesystem lookup
> under /proc/sys.  Which ultimately makes the code easier to maintain
> and debug.

That sounds like a much saner approach, but has the wrinkle that procfs can 
be configured out.

> Now back to getting things forward ported and ready to post.

I'm looking forward to those changes.  I've been ignoring procfs for the 
time being by disabling the per-interface entries in the network stack, 
but there is some desire to be able to enable rp_filter on a per-interface 
radius config at runtime.  rp_filter has to be disabled across the board 
on my access routers, as there are several places where assymetric routing 
is used for performance reasons.

		-ben

^ permalink raw reply

* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Jay Vosburgh @ 2009-10-31  0:10 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, netdev
In-Reply-To: <m1639wo5gb.fsf@fess.ebiederm.org>

Eric W. Biederman <ebiederm@xmission.com> wrote:

>Jay Vosburgh <fubar@us.ibm.com> writes:
>
>> David Miller <davem@davemloft.net> wrote:
>>
>>>From: ebiederm@xmission.com (Eric W. Biederman)
>>>Date: Thu, 29 Oct 2009 17:16:54 -0700
>>>
>>>> I recently had it pointed out to me that the bonding driver does not
>>>> work in a network namespace.  So I have simplified the bonding driver
>>>> a bit, added support for ip link add and ip link del, and finally made
>>>> the bonding driver work in multiple network namespaces.
>>>> 
>>>> The most note worthy change in the patchset is the addition of support
>>>> in the networking core for registering a sysfs group for a device.
>>>> 
>>>> Using this in the bonding driver simplifies the code and removes a
>>>> userspace race between actions triggered by the netlink event and the
>>>> bonding sysfs attributes appearing.
>>>
>>>I've tossed patches 1-7 into net-next-2.6, thanks Eric.
>>
>> 	I put patches 1-7 on a recent net-next-2.6, and from a simple
>> "insmod bonding.ko; rmmod bonding" I'm seeing the following:
>>
>> ------------[ cut here ]------------
>> WARNING: at fs/proc/generic.c:847 remove_proc_entry+0x1a8/0x1c7()
>> Hardware name: IBM eserver xSeries 220 -[8645]-
>> remove_proc_entry: removing non-empty directory 'net/bonding', leaking at least 
>> 'bond0'
>> Modules linked in: bonding(-) ipv6 microcode loop ppdev sworks_agp parport_pc tg
>> 3 e100 agpgart parport mii libphy e1000 edd pata_serverworks [last unloaded: spe
>> edstep_lib]
>> Pid: 6216, comm: rmmod Not tainted 2.6.32-rc3-devel #19
>> Call Trace:
>>  [<c012ec9d>] warn_slowpath_common+0x60/0x90
>>  [<c012ed01>] warn_slowpath_fmt+0x24/0x27
>>  [<c01e3e55>] remove_proc_entry+0x1a8/0x1c7
>>  [<e0906435>] ? bond_net_exit+0x0/0xa3 [bonding]
>>  [<e09064c3>] bond_net_exit+0x8e/0xa3 [bonding]
>>  [<c02e6ee8>] unregister_pernet_gen_subsys+0x23/0x3d
>>  [<e0910baa>] bonding_exit+0x3a/0x66 [bonding]
>>  [<c015ccf3>] sys_delete_module+0x191/0x1f1
>>  [<c0147a30>] ? up_read+0x16/0x2a
>>  [<c0102a18>] ? restore_all_notrace+0x0/0x18
>>  [<c0353357>] ? do_page_fault+0x0/0x393
>>  [<c0102904>] sysenter_do_call+0x12/0x32
>> ---[ end trace 8f3eaeee682a572c ]---
>>
>> 	Any thoughts?  I have not as yet investigated further.
>
>Weird.
>
>We have already run:
>rtnl_link_unregister. 
>  rtnl_kill_links
>    dellink(bond0)
>      unregister_netdevice(bond0)
>          bond_uninit
>             bond_remove_proc_entry
>
>
>So the proc entry should no longer be there.  I'm a little nervous about
>the new unregister_netdevice_many but I don't see any obvious problems with
>that code.
>
>Were there by any chance any earlier errors that could have prevented the uninit?
>You weren't inserting multiple copies of the bonding driver?

	No, to both questions.  Also, if I back out the 7 bonding
patches, the same insmod / rmmod does not panic.  

	I just set it up and did it again.  Fresh boot of the system
(which doesn't load bonding); "insmod drivers/net/bonding/bonding.ko;
rmmod bonding" and blammo.

	A little bisect action reveals that the problem first appears
after applying the fifth patch (below).  Does a basic insmod / rmmod
cycle work ok for you?  I'm specifying no options to bonding.

	-J

Subject: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
[...]
This implements a basic set of rtnl link ops and takes advantage of
the fact that rtnl_link_unregister kills all of the surviving
devices to all us to kill bond_free_all.  A module alias
is added so ip link add can pull in the bonding module.

Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
---
 drivers/net/bonding/bond_main.c |   55 +++++++++++++++++++++-----------------
 1 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7a37ecf..6da2a82 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4612,22 +4612,6 @@ static void bond_uninit(struct net_device *bond_dev)
 	netif_addr_unlock_bh(bond_dev);
 }

-/* Unregister and free all bond devices.
- * Caller must hold rtnl_lock.
- */
-static void bond_free_all(void)
-{
-	struct bonding *bond, *nxt;
-
-	list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) {
-		struct net_device *bond_dev = bond->dev;
-
-		unregister_netdevice(bond_dev);
-	}
-
-	bond_destroy_proc_dir();
-}
-
 /*------------------------- Module initialization ---------------------------*/

 /*
@@ -5065,6 +5049,23 @@ static int bond_init(struct net_device *bond_dev)
 	return 0;
 }

+static int bond_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+	if (tb[IFLA_ADDRESS]) {
+		if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+			return -EINVAL;
+		if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+			return -EADDRNOTAVAIL;
+	}
+	return 0;
+}
+
+static struct rtnl_link_ops bond_link_ops __read_mostly = {
+	.kind		= "bond",
+	.setup		= bond_setup,
+	.validate	= bond_validate,
+};
+
 /* Create a new bond based on the specified name and bonding parameters.
  * If name is NULL, obtain a suitable "bond%d" name for us.
  * Caller must NOT hold rtnl_lock; we need to release it here before we
@@ -5086,6 +5087,8 @@ int bond_create(const char *name)
 		goto out;
 	}

+	bond_dev->rtnl_link_ops = &bond_link_ops;
+
 	if (!name) {
 		res = dev_alloc_name(bond_dev, "bond%d");
 		if (res < 0)
@@ -5115,6 +5118,10 @@ static int __init bonding_init(void)

 	bond_create_proc_dir();

+	res = rtnl_link_register(&bond_link_ops);
+	if (res)
+		goto err;
+
 	for (i = 0; i < max_bonds; i++) {
 		res = bond_create(NULL);
 		if (res)
@@ -5128,14 +5135,12 @@ static int __init bonding_init(void)
 	register_netdevice_notifier(&bond_netdev_notifier);
 	register_inetaddr_notifier(&bond_inetaddr_notifier);
 	bond_register_ipv6_notifier();
-
-	goto out;
-err:
-	rtnl_lock();
-	bond_free_all();
-	rtnl_unlock();
 out:
 	return res;
+err:
+	rtnl_link_unregister(&bond_link_ops);
+	bond_destroy_proc_dir();
+	goto out;

 }

@@ -5147,9 +5152,8 @@ static void __exit bonding_exit(void)

 	bond_destroy_sysfs();

-	rtnl_lock();
-	bond_free_all();
-	rtnl_unlock();
+	rtnl_link_unregister(&bond_link_ops);
+	bond_destroy_proc_dir();
 }

 module_init(bonding_init);
@@ -5158,3 +5162,4 @@ MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 MODULE_DESCRIPTION(DRV_DESCRIPTION ", v" DRV_VERSION);
 MODULE_AUTHOR("Thomas Davis, tadavis@lbl.gov and many others");
+MODULE_ALIAS_RTNL_LINK("bond");
-- 
1.6.3.1.54.g99dd.dirty


---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply related

* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric W. Biederman @ 2009-10-31  0:27 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: David Miller, netdev
In-Reply-To: <21200.1256937150@death.nxdomain.ibm.com>

Jay Vosburgh <fubar@us.ibm.com> writes:

> 	I put patches 1-7 on a recent net-next-2.6, and from a simple
> "insmod bonding.ko; rmmod bonding" I'm seeing the following:
>

> 	Any thoughts?  I have not as yet investigated further.

I just tried to reproduce this, without luck.  My best guess is that there is
something odd particular to your setup. 

A simple while :; modprobe bonding ; rmmod bonding ; done runs without problems
for me.

Eric

^ permalink raw reply

* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Eric W. Biederman @ 2009-10-31  0:37 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <20091030235314.GH3141@kvack.org>

Benjamin LaHaise <bcrl@lhnet.ca> writes:

> On Fri, Oct 30, 2009 at 04:25:52PM -0700, Eric W. Biederman wrote:
>> I realized after sending the message that s_mode in sysfs_dirent is a
>> real size offense.  It is a 16bit field packed in between two longs.
>> So in practice it is possible to move the s_mode  up next to s_flags
>> and add a s_nlink after it both unsigned short and get a cheap sysfs_nlink.
>
> That doesn't work -- the number of directory entries can easily exceed 65535.  
> Current mid range hardware is good enough to terminate 100,000 network 
> interfaces on a single host.

On overflow you nlink becomes zero and you leave it there.  That is how
ondisk filesystems handle that case on directories, and find etc
knows how to deal.

>> Since I'm not quite ready to post my patches.  I will briefly
>> mention what I have in my queue and hopefully get things posted.
>> 
>> I have changes to make it so that sysfs never has to go from
>> the sysfs_dirent to the sysfs inode.  
>
> Ah, interesting.

I have to cleanup sysfs before I merge changes for supporting
multiple network namespaces.

>> I have changes to sys_sysctl() so that it becomes a filesystem lookup
>> under /proc/sys.  Which ultimately makes the code easier to maintain
>> and debug.
>
> That sounds like a much saner approach, but has the wrinkle that procfs can 
> be configured out.

So I will add the dependency.  There are very few serious users of sys_sysctl,
and all of them have been getting a deprecated interface warning every
time they use it for the last several years.

>> Now back to getting things forward ported and ready to post.
>
> I'm looking forward to those changes.  I've been ignoring procfs for the 
> time being by disabling the per-interface entries in the network stack, 
> but there is some desire to be able to enable rp_filter on a per-interface 
> radius config at runtime.  rp_filter has to be disabled across the board 
> on my access routers, as there are several places where assymetric routing 
> is used for performance reasons.

Just out of curiosity does the loose rp_filter mode work for you?

Eric

^ permalink raw reply

* [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth
From: Eric W. Biederman @ 2009-10-31  0:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
In-Reply-To: <m1hbtgmqew.fsf@fess.ebiederm.org>


I tested the recent unregister many changes and got a weird,
nasty and seemingly unrelasted kernel oops. Changing
unregister_netdevice_queue to use list_move_tail fixes
the problem for me.

ip link add type veth
rmmod veth

ls /sys/class/net/
showed one of the veth devices still present.

A subsequent ip link oopsed the box.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

---
diff --git a/net/core/dev.c b/net/core/dev.c
index 94f42a1..9c0b202 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5227,6 +5227,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	netdev_init_queues(dev);
 
 	INIT_LIST_HEAD(&dev->napi_list);
+	INIT_LIST_HEAD(&dev->unreg_list);
 	dev->priv_flags = IFF_XMIT_DST_RELEASE;
 	setup(dev);
 	strcpy(dev->name, name);
@@ -5308,7 +5309,7 @@ void unregister_netdevice_queue(struct net_device *dev, struct list_head *head)
 	ASSERT_RTNL();
 
 	if (head) {
-		list_add_tail(&dev->unreg_list, head);
+		list_move_tail(&dev->unreg_list, head);
 	} else {
 		rollback_registered(dev);
 		/* Finish processing unregister after unlock */

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox