Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Ben Hutchings @ 2010-07-20 14:29 UTC (permalink / raw)
  To: Stefan Assmann
  Cc: netdev, Linux Kernel Mailing List, davem, Andy Gospodarek,
	Rose, Gregory V, Duyck, Alexander H, Casey Leedom, Harald Hoyer
In-Reply-To: <4C45996B.3000003@redhat.com>

On Tue, 2010-07-20 at 14:41 +0200, Stefan Assmann wrote:
> On 20.07.2010 14:07, Ben Hutchings wrote:
> > On Tue, 2010-07-20 at 13:47 +0200, Stefan Assmann wrote:
> >> On 20.07.2010 13:20, Ben Hutchings wrote:
> >>> On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
> >>>> From: Stefan Assmann <sassmann@redhat.com>
> 
> [snip]
> 
> >> Actually Dave Miller suggested to put it there. What other place is
> >> there to put it?
> > 
> > If Dave said that then I'm sure it's OK.
> > 
> > However, if you define this as an interface flag (net_device::flags;
> > <linux/if.h>) and add it to the set of changeable flags in
> > __dev_change_flags(), user-space will be able to clear the flag if it
> > later sets a stable address.
> 
> As I said I'm not that knowledgeable about this MAC address stealing
> thing and I'm assuming that's what you're aiming at. Would you really
> want/need it to be user-space writable? Currently all I can think of is
> the scenario where you set a "stable" address that outlasts a reboot so
> udev might be able to assign it a permanent name after it gets stable.
>
> So it might make sense to have it writable, but I'd like to avoid to add
> unnecessary complexity that may cause errors if it's not necessary.
> Read-only is simple, just read the flag and deal with it.

Once this flag has been added, it may be used by any tool and not just
udev, so it ought to indicate the status of the currently assigned
address.  That requires that it be writable.

> Btw, the driver itself could also alter the flag. Then we'd have a well
> defined way of setting a stable address.

The driver can't know whether an address assigned by the user is stable.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH] drop_monitor: Add error code to detect duplicate state changes
From: Neil Horman @ 2010-07-20 14:52 UTC (permalink / raw)
  To: netdev; +Cc: davem, nhorman

Hey-
	Patch to add -EAGAIN error to dropwatch netlink message handling code.
-EAGAIN will be returned anytime userspace attempts to transition the state of
the drop monitor service to a state that its already in.  That allows user space
to detect this condition, so it doesn't wait for a success ACK that will never
arrive.  Tested successfully by me

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 drop_monitor.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index ad41529..646ef3b 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -223,6 +223,11 @@ static int set_all_monitor_traces(int state)
 
 	spin_lock(&trace_state_lock);
 
+	if (state == trace_state) {
+		rc = -EAGAIN;
+		goto out_unlock;
+	}
+
 	switch (state) {
 	case TRACE_ON:
 		rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
@@ -251,11 +256,12 @@ static int set_all_monitor_traces(int state)
 
 	if (!rc)
 		trace_state = state;
+	else
+		rc = -EINPROGRESS;
 
+out_unlock:
 	spin_unlock(&trace_state_lock);
 
-	if (rc)
-		return -EINPROGRESS;
 	return rc;
 }
 

^ permalink raw reply related

* [PATCH] e1000e: Fix irq_synchronize in MSI-X case
From: Jean Delvare @ 2010-07-20 15:12 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Bruce Allan, Jesse Brandeburg

Synchronize all IRQs when in MSI-X IRQ mode.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
I sent this patch to the e1000-devel list on June 8th, 2010, but
didn't receive any answer:
http://sourceforge.net/mailarchive/forum.php?thread_name=201006081818.59098.jdelvare%40suse.de&forum_name=e1000-devel

I don't know how critical synchronize_irq() is, so I don't know if
this patch should go to stable branches or not.

 drivers/net/e1000e/netdev.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -1939,7 +1939,13 @@ static void e1000_irq_disable(struct e10
 	if (adapter->msix_entries)
 		ew32(EIAC_82574, 0);
 	e1e_flush();
-	synchronize_irq(adapter->pdev->irq);
+
+	if (adapter->msix_entries) {
+		synchronize_irq(adapter->msix_entries[0].vector);
+		synchronize_irq(adapter->msix_entries[1].vector);
+		synchronize_irq(adapter->msix_entries[2].vector);
+	} else
+		synchronize_irq(adapter->pdev->irq);
 }
 
 /**

-- 
Jean Delvare
Suse L3

^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Eric Dumazet @ 2010-07-20 15:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Johannes Berg
In-Reply-To: <1279631789.2498.71.camel@edumazet-laptop>

Le mardi 20 juillet 2010 à 15:16 +0200, Eric Dumazet a écrit :
> Please note following potential bug was discovered by code review, and
> my patch not even tested, please double check !
> 
> Thanks
> 
> [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
> 
> commit 1dacc76d0014 
> (net/compat/wext: send different messages to compat tasks)
> introduced a race condition on netlink, in case MSG_PEEK is used.
> 
> An skb given by skb_recv_datagram() might be shared, we must clone it
> before any modification, or risk fatal corruption.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---

Oh well, skb_copy() or skb_unshare() is needed.

[PATCH net-next-2.6 v2] netlink: netlink_recvmsg() fix

commit 1dacc76d0014 
(net/compat/wext: send different messages to compat tasks)
introduced a race condition on netlink, in case MSG_PEEK is used.

An skb given by skb_recv_datagram() might be shared, we must copy it
before any modification, or risk fatal corruption.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/netlink/af_netlink.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7aeaa83..1537fa5 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1405,7 +1405,7 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 	struct netlink_sock *nlk = nlk_sk(sk);
 	int noblock = flags&MSG_DONTWAIT;
 	size_t copied;
-	struct sk_buff *skb, *frag __maybe_unused = NULL;
+	struct sk_buff *skb;
 	int err;
 
 	if (flags&MSG_OOB)
@@ -1440,7 +1440,12 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 			kfree_skb(skb);
 			skb = compskb;
 		} else {
-			frag = skb_shinfo(skb)->frag_list;
+			skb = skb_unshare(skb, GFP_KERNEL);
+			if (!skb) {
+				err = -ENOMEM;
+				goto out;
+			}
+			kfree_skb(skb_shinfo(skb)->frag_list);
 			skb_shinfo(skb)->frag_list = NULL;
 		}
 	}
@@ -1477,10 +1482,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 	if (flags & MSG_TRUNC)
 		copied = skb->len;
 
-#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
-	skb_shinfo(skb)->frag_list = frag;
-#endif
-
 	skb_free_datagram(sk, skb);
 
 	if (nlk->cb && atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2)



^ permalink raw reply related

* Re: [PATCH] phy: add suspend/resume in the ic+
From: Randy Dunlap @ 2010-07-20 15:24 UTC (permalink / raw)
  To: Giuseppe CAVALLARO; +Cc: netdev
In-Reply-To: <1279609954-30274-1-git-send-email-peppe.cavallaro@st.com>

On Tue, 20 Jul 2010 09:12:34 +0200 Giuseppe CAVALLARO wrote:

> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/net/phy/icplus.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
> index 439adaf..3f2583f 100644
> --- a/drivers/net/phy/icplus.c
> +++ b/drivers/net/phy/icplus.c
> @@ -116,6 +116,8 @@ static struct phy_driver ip175c_driver = {
>  	.config_init	= &ip175c_config_init,
>  	.config_aneg	= &ip175c_config_aneg,
>  	.read_status	= &ip175c_read_status,
> +	.suspend	= genphy_suspend,
> +	.resume		= genphy_resume,
>  	.driver		= { .owner = THIS_MODULE,},
>  };
>  
> -- 

I was wondering how that works when CONFIG_PM is disabled, but I did a build
with it disabled and see that genphy_suspend and genphy_resume are always
built -- even when PM is disabled.  Interesting.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* [PATCH] e1000e: Drop a useless statement
From: Jean Delvare @ 2010-07-20 15:30 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Auke Kok, Bruce Allan, Jesse Brandeburg

err is set again a few lines below.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Auke Kok <auke-jan.h.kok@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
I sent this patch to the e1000-devel list on June 8th, 2010, but
didn't receive any answer:
http://sourceforge.net/mailarchive/forum.php?thread_name=201006081820.25381.jdelvare%40suse.de&forum_name=e1000-devel

 drivers/net/e1000e/netdev.c |    2 --
 1 file changed, 2 deletions(-)

--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -5557,8 +5557,6 @@ static int __devinit e1000_probe(struct
 	if (err)
 		goto err_sw_init;
 
-	err = -EIO;
-
 	memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
 	memcpy(&hw->nvm.ops, ei->nvm_ops, sizeof(hw->nvm.ops));
 	memcpy(&hw->phy.ops, ei->phy_ops, sizeof(hw->phy.ops));

-- 
Jean Delvare
Suse L3

^ permalink raw reply

* Re: [PATCH 11/19] drivers/net/irda: use for_each_pci_dev()
From: Jiri Kosina @ 2010-07-20 15:33 UTC (permalink / raw)
  To: Kulikov Vasiliy
  Cc: Kernel Janitors, Samuel Ortiz, David S. Miller, Stephen Hemminger,
	Joe Perches, Eric Dumazet, netdev, linux-kernel
In-Reply-To: <1278173056-11779-1-git-send-email-segooon@gmail.com>

On Sat, 3 Jul 2010, Kulikov Vasiliy wrote:

> Use for_each_pci_dev() to simplify the code.
> 
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
> ---
>  drivers/net/irda/smsc-ircc2.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/irda/smsc-ircc2.c b/drivers/net/irda/smsc-ircc2.c
> index d67e484..850ca1c 100644
> --- a/drivers/net/irda/smsc-ircc2.c
> +++ b/drivers/net/irda/smsc-ircc2.c
> @@ -2848,9 +2848,7 @@ static int __init smsc_ircc_preconfigure_subsystems(unsigned short ircc_cfg,
>  	unsigned short ss_device = 0x0000;
>  	int ret = 0;
>  
> -	dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev);
> -
> -	while (dev != NULL) {
> +	for_each_pci_dev(dev) {
>  		struct smsc_ircc_subsystem_configuration *conf;
>  
>  		/*
> @@ -2899,7 +2897,6 @@ static int __init smsc_ircc_preconfigure_subsystems(unsigned short ircc_cfg,
>  					ret = -ENODEV;
>  			}
>  		}
> -		dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev);
>  	}
>  
>  	return ret;

Doesn't seem to hit linux-next as of today. Dave, are you going to merge 
it?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

^ permalink raw reply

* Re: [PATCH net-next 0/3] cxgb4vf: fixes for several small issues discovered by QA
From: Casey Leedom @ 2010-07-20 15:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20100719.204002.219738044.davem@davemloft.net>

| From: David Miller <davem@davemloft.net>
| Date: Monday, July 19, 2010 08:40 pm
| 
| From: Casey Leedom <leedom@chelsio.com>
| Date: Mon, 19 Jul 2010 18:12:10 -0700
| 
| >   A couple of small (but important) fixes discovered by our QA people. 
| >   I've also
| > 
| > included a patch to add myself as the maintainer of cxgb4vf in the
| > MAINTAINERS file which I think is the protocol but please correct me if
| > changes to that file are usually performed by someone else.
| 
| Well, where are they?

  Oops!  Sorry!!  It was my first effort at using "git send-email" and in my manic 
attempt to make sure I wasn't screwing it up I had directed the patches to 
myself to verify that the process worked and then forgot to change the To: 
address to netdev@vger.kernel.org in my "real attempt."

  Patches coming forthwith.

Casey

^ permalink raw reply

* [PATCH net-next 1/3] cxgb4vf: Fix off-by-one error checking for the end of the mailbox delay array
From: Casey Leedom @ 2010-07-20 15:58 UTC (permalink / raw)
  To: netdev

>From 1d32860335fad8c67e23254aec7c30750276f2b4 Mon Sep 17 00:00:00 2001
From: Casey Leedom <leedom@chelsio.com>
Date: Mon, 19 Jul 2010 17:51:46 -0700
Subject: [PATCH net-next 1/3] cxgb4vf: Fix off-by-one error checking for the end of the mailbox delay array

Fix off-by-one error in checking for the end of the mailbox response delay
array.  We ended up walking off the end and, if we were unlucky, we'd end up
pulling in a 0 and never terminate the mailbox response delay loop ...

Signed-off-by: Casey Leedom <leedom@chelsio.com>
---
 drivers/net/cxgb4vf/t4vf_hw.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/cxgb4vf/t4vf_hw.c b/drivers/net/cxgb4vf/t4vf_hw.c
index 1ef2528..ea1c123 100644
--- a/drivers/net/cxgb4vf/t4vf_hw.c
+++ b/drivers/net/cxgb4vf/t4vf_hw.c
@@ -163,7 +163,7 @@ int t4vf_wr_mbox_core(struct adapter *adapter, const void *cmd, int size,
 	for (i = 0; i < 500; i += ms) {
 		if (sleep_ok) {
 			ms = delay[delay_idx];
-			if (delay_idx < ARRAY_SIZE(delay))
+			if (delay_idx < ARRAY_SIZE(delay) - 1)
 				delay_idx++;
 			msleep(ms);
 		} else
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH net-next 2/3] cxgb4vf: Fix bug where we were only allocating one queue in MSI mode
From: Casey Leedom @ 2010-07-20 15:59 UTC (permalink / raw)
  To: netdev

>From 7e141cafe989958267803791aa1bcacfffe5cfb2 Mon Sep 17 00:00:00 2001
From: Casey Leedom <leedom@chelsio.com>
Date: Mon, 19 Jul 2010 17:53:48 -0700
Subject: [PATCH net-next 2/3] cxgb4vf: Fix bug where we were only allocating one queue in MSI mode

Fix bug in setup_sge_queues() where we were incorrectly only allocating a
single "Queue Set" for MSI mode.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
---
 drivers/net/cxgb4vf/cxgb4vf_main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
index d065516..a165632 100644
--- a/drivers/net/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
@@ -533,10 +533,9 @@ static int setup_sge_queues(struct adapter *adapter)
 		struct port_info *pi = netdev_priv(dev);
 		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_qset];
 		struct sge_eth_txq *txq = &s->ethtxq[pi->first_qset];
-		int nqsets = (adapter->flags & USING_MSIX) ? pi->nqsets : 1;
 		int qs;
 
-		for (qs = 0; qs < nqsets; qs++, rxq++, txq++) {
+		for (qs = 0; qs < pi->nqsets; qs++, rxq++, txq++) {
 			err = t4vf_sge_alloc_rxq(adapter, &rxq->rspq, false,
 						 dev, msix++,
 						 &rxq->fl, t4vf_ethrx_handler);
@@ -565,10 +564,9 @@ static int setup_sge_queues(struct adapter *adapter)
 		struct port_info *pi = netdev_priv(dev);
 		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_qset];
 		struct sge_eth_txq *txq = &s->ethtxq[pi->first_qset];
-		int nqsets = (adapter->flags & USING_MSIX) ? pi->nqsets : 1;
 		int qs;
 
-		for (qs = 0; qs < nqsets; qs++, rxq++, txq++) {
+		for (qs = 0; qs < pi->nqsets; qs++, rxq++, txq++) {
 			IQ_MAP(s, rxq->rspq.abs_id) = &rxq->rspq;
 			EQ_MAP(s, txq->q.abs_id) = &txq->q;
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH net-next 3/3] cxgb4vf: add maintainer entry for cxgb4vf
From: Casey Leedom @ 2010-07-20 15:59 UTC (permalink / raw)
  To: netdev

>From 7710873beb494b46333fdf9841b4a117bdb66a5a Mon Sep 17 00:00:00 2001
From: Casey Leedom <leedom@chelsio.com>
Date: Mon, 19 Jul 2010 17:55:33 -0700
Subject: [PATCH net-next 3/3] cxgb4vf: add maintainer entry for cxgb4vf

Adding myself as the official maintainer of the Chelsio T4 Virtual function
Driver (cxgb4vf).

Signed-off-by: Casey Leedom <leedom@chelsio.com>
---
 MAINTAINERS |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c05b499..aa73c9a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1767,6 +1767,13 @@ W:	http://www.openfabrics.org
 S:	Supported
 F:	drivers/infiniband/hw/cxgb4/
 
+CXGB4VF ETHERNET DRIVER (CXGB4VF)
+M:	Casey Leedom <leedom@chelsio.com>
+L:	netdev@vger.kernel.org
+W:	http://www.chelsio.com
+S:	Supported
+F:	drivers/net/cxgb4vf/
+
 CYBERPRO FB DRIVER
 M:	Russell King <linux@arm.linux.org.uk>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH net-next] drivers/acpi/acpica/utmisc.c: Use printk extension %pV
From: Bjorn Helgaas @ 2010-07-20 16:08 UTC (permalink / raw)
  To: Joe Perches; +Cc: Len Brown, linux-acpi, linux-kernel, netdev, David Miller
In-Reply-To: <1279602367.19374.20.camel@Joe-Laptop.home>

On Monday, July 19, 2010 11:06:07 pm Joe Perches wrote:
> Consolidates the printk messages to a single
> call so the messages can not be interleaved.
> 
> Reduces text a bit.
> 
> $ size drivers/acpi/acpica/utmisc.o.*
>    text	   data	    bss	    dec	    hex	filename
>    7822	     56	   1832	   9710	   25ee	drivers/acpi/acpica/utmisc.o.old
>    7748	     56	   1736	   9540	   2544	drivers/acpi/acpica/utmisc.o.new
> 
> Depends on net-next commit 7db6f5fb65a82af03229eef104dc9899c5eecf33
> (vsprintf: Recursive vsnprintf: Add "%pV", struct va_format)

drivers/acpi/acpica/utmisc.c is part of the ACPI CA and is used in
several different OSes, but %pV sounds like a Linux-specific feature,
so I don't see how this patch can work.

Bjorn

^ permalink raw reply

* Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation
From: Alexander Duyck @ 2010-07-20 16:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com, bphilips@novell.com, Skidmore, Donald C
In-Reply-To: <1279607980.2458.82.camel@edumazet-laptop>

Eric Dumazet wrote:
> Le lundi 19 juillet 2010 à 16:59 -0700, Jeff Kirsher a écrit :
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> This change removes UDP from the supported protocols for RSS hashing.  The
>> reason for removing this protocol is because IP fragmentation was causing a
>> network flow to be broken into two streams, one for fragmented, and one for
>> non-fragmented and this in turn was causing out-of-order issues.
>>
> 
> Jeff, does it mean all UDP packets are going to be delivered to a single
> queue ?
> 
> This would be a serious regression.
> 
> Many UDP applications try hard to not use fragments. 
> 
> They are going to pay the price because some application :
> - Use big segments, fragmented.
> - Is subject to OOO artifacts.
> 
> We would like some clarifications please :)
> 
> 
> 

The packets will still be hashed on source and destination IPv4/IPv6 
addresses.  The change just drops reading the UDP source/destination 
ports since in the case of fragmented packets they are not available and 
as such were being parsed as IPv4/IPv6 packets.  By making this change 
the queue selection is consistent between all packets in the UDP stream.

The only regression I would expect to see would be in testing between 
two fixed systems since the IP addresses of the two systems would be 
fixed and so running multiple flows between the two would yield the same 
  RSS hash for multiple UDP streams.  As long as multiple ip addresses 
are used  you should see multiple RSS hashes generated and as such the 
load should still be distributed.

Thanks,

Alex

^ permalink raw reply

* Re: [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation
From: Eric Dumazet @ 2010-07-20 16:39 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com, bphilips@novell.com, Skidmore, Donald C
In-Reply-To: <4C45CAC6.1050006@intel.com>

Le mardi 20 juillet 2010 à 09:11 -0700, Alexander Duyck a écrit :

> The packets will still be hashed on source and destination IPv4/IPv6 
> addresses.  The change just drops reading the UDP source/destination 
> ports since in the case of fragmented packets they are not available and 
> as such were being parsed as IPv4/IPv6 packets.  By making this change 
> the queue selection is consistent between all packets in the UDP stream.
> 

Excellent, this is perfect IMHO.

> The only regression I would expect to see would be in testing between 
> two fixed systems since the IP addresses of the two systems would be 
> fixed and so running multiple flows between the two would yield the same 
>   RSS hash for multiple UDP streams.  As long as multiple ip addresses 
> are used  you should see multiple RSS hashes generated and as such the 
> load should still be distributed.
> 

Ack. Fortunately, one can still use RPS to spread load onto multiple
cpus in this case.

This until ixgpe fills skb->rxhash with a non null value.
If it happens one day, we shall remind _not_ filling it for UDP packets.

BTW, this reminds me a netdev discussion we had for bnx2x

http://www.kerneltrap.com/mailarchive/linux-netdev/2010/4/23/6275415/thread

And now, I understand why Toepliz hash doesnt use src port/dst port,
since this is not available on fragments, obviously...

Thanks !



^ permalink raw reply

* [PATCH] drop_monitor: convert some kfree_skb call sites to consume_skb
From: Neil Horman @ 2010-07-20 16:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, viro, eparis, nhorman

Convert a few calls from kfree_skb to consume_skb

Noticed while I was working on dropwatch that I was detecting lots of internal
skb drops in several places.  While some are legitimate, several were not,
freeing skbs that were at the end of their life, rather than being discarded due
to an error.  This patch converts those calls sites from using kfree_skb to
consume_skb, which quiets the in-kernel drop_monitor code from detecting them as
drops.  Tested successfully by myself

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Eric Paris <eparis@redhat.com>
CC: Al Viro <viro@zeniv.linux.org.uk>


 kernel/audit.c           |    2 +-
 net/netlink/af_netlink.c |    9 +++++----
 net/unix/af_unix.c       |    2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index c71bd26..8296aa5 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -407,7 +407,7 @@ static void kauditd_send_skb(struct sk_buff *skb)
 		audit_hold_skb(skb);
 	} else
 		/* drop the extra reference if sent ok */
-		kfree_skb(skb);
+		consume_skb(skb);
 }
 
 static int kauditd_thread(void *dummy)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7aeaa83..8648a99 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1076,14 +1076,15 @@ int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 pid,
 	sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list)
 		do_one_broadcast(sk, &info);
 
-	kfree_skb(skb);
+	consume_skb(skb);
 
 	netlink_unlock_table();
 
-	kfree_skb(info.skb2);
-
-	if (info.delivery_failure)
+	if (info.delivery_failure) {
+		kfree_skb(info.skb2);
 		return -ENOBUFS;
+	} else
+		consume_skb(info.skb2);
 
 	if (info.delivered) {
 		if (info.congested && (allocation & __GFP_WAIT))
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 75ba48b..4414a18 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1906,7 +1906,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
 				break;
 			}
 
-			kfree_skb(skb);
+			consume_skb(skb);
 
 			if (siocb->scm->fp)
 				break;

^ permalink raw reply related

* Re: [PATCH] net: Add batman-adv meshing protocol
From: David Miller @ 2010-07-20 16:59 UTC (permalink / raw)
  To: sven.eckelmann; +Cc: b.a.t.m.a.n, siwu, netdev
In-Reply-To: <201007201028.12470.sven.eckelmann@gmx.de>

From: Sven Eckelmann <sven.eckelmann@gmx.de>
Date: Tue, 20 Jul 2010 10:28:10 +0200

> I would leave that to the original author of those functions.

Whoever you leave it to, this needs to be fixed up before this
code can be integrated.  So if those people are not active or
don't have time, you very much should consider the reality that
you might have to do these fixups yourself.


^ permalink raw reply

* Re: [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring
From: Brandeburg, Jesse @ 2010-07-20 17:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com, bphilips@novell.com, Duyck, Alexander H
In-Reply-To: <1279600823.2458.59.camel@edumazet-laptop>



On Mon, 19 Jul 2010, Eric Dumazet wrote:

> Le lundi 19 juillet 2010 à 16:43 -0700, Jeff Kirsher a écrit :
> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > 
> > This change makes it possible to limit the number of descriptors down to 48
> > per ring.  The reason for this change is to address a variation on hardware
> > errata 10 for 82546GB in which descriptors will be lost if more than 32
> > descriptors are fetched and the PCI-X MRBC is 512.
> > -#define E1000_MIN_TXD                       80
> > +#define E1000_MIN_TXD                       48
> >  #define E1000_MAX_82544_TXD               4096
> >  
> >  #define E1000_DEFAULT_RXD                  256
> >  #define E1000_MAX_RXD                      256
> > -#define E1000_MIN_RXD                       80
> > +#define E1000_MIN_RXD                       48
> >  #define E1000_MAX_82544_RXD               4096

> So this limit is a pure software one ?

Yes, the hardware will work fine (with the exception of some limits when 
certain performance registers are configured like [TR]XDCTL) all the way 
down to 1 descriptor, that said, the practical limit is probably more like 
8 for RX descriptors, and is 2 * (MAX_SKB_FRAGS + 1) for transmit if TSO 
is enabled.

if all offloads are disabled you could run with only 8 tx descriptors, and 
I believe it would work fine.

> Why not let an admin chose a lower limit if he wants to ?

I think in this case just to prevent odd interaction bugs with low values 
(there would have to be several driver changes and the testing work would 
increase to support very low values)

> I am asking because big ring sizes can be a latency source in some
> workloads.

yes, if you are trying to pace traffic in software.  That said, at least 
on transmit, most frames exit immediately from the hardware unless some 
external event like flow control is slowing down transmit.

Jesse

^ permalink raw reply

* Re: [PATCH] net: Add batman-adv meshing protocol
From: Sven Eckelmann @ 2010-07-20 17:16 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
	siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX
In-Reply-To: <20100720.095928.113690788.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

[-- Attachment #1: Type: Text/Plain, Size: 1169 bytes --]

On Tuesday 20 July 2010 18:59:28 David Miller wrote:
> From: Sven Eckelmann <sven.eckelmann-Mmb7MZpHnFY@public.gmane.org>
> Date: Tue, 20 Jul 2010 10:28:10 +0200
> 
> > I would leave that to the original author of those functions.
> 
> Whoever you leave it to, this needs to be fixed up before this
> code can be integrated.  So if those people are not active or
> don't have time, you very much should consider the reality that
> you might have to do these fixups yourself.

They are active and informed about it. I never wanted that you integrate it 
right away, but asked for a review to get everything in shape for the real 
move from staging to net.

Maybe there is a misunderstanding who I am. When you look at the MAINTAINERS 
file than you will notice that my name doesn't appear. I just volunteered to 
keep the different branches in sync and submitted it for a review to you and 
netdev after a discussion with Marek Lindner. He is together with Simon 
Wunderlich the real maintainer of the codebase.

I will stop here if you prefer not to talk with an intermediary. So Marek or 
Simon have to work directly with you.

thanks,
	Sven

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

^ permalink raw reply

* Re: Question about way that NICs deliver packets to the kernel
From: Rick Jones @ 2010-07-20 17:16 UTC (permalink / raw)
  To: Junchang Wang; +Cc: Ben Hutchings, romieu, netdev
In-Reply-To: <20100720011522.GA4123@host-a-160.ustcsz.edu.cn>

Junchang Wang wrote:
> On Fri, Jul 16, 2010 at 10:58:46AM -0700, Rick Jones wrote:
> 
>>>Hi Ben,
>>>I added options -c -C to netperf's command line. Result is as follows:
>>>                   scheme 1    scheme 2    Imp.
>>>Throughput:     683M        718M       5%
>>>CPU usage:     47.8%       45.6%
>>>
>>>That really surprised me because "top" command showed the CPU usage
>>>was fluctuating between 0.5% and 1.5% rather that between 45% and 50%.
>>
> 
> Hi rick,
> very sorry for my late reply. Just recovered from the final exam.:)
> 
> 
>>Can you tell us a bit more about the system, and which version of
>>netperf you are using?  
> 
> 
> The target machine is a Pentium Dual-core E2200 desktop with a r8169 
> gigabit NIC. (I couldn't find a better server with old pci slot.)
> 
> Another machine is a Nehalem based system with Intel 82576 NIC.
> 
> The target machine executes netserver and Nehalem machine executes netperf.
> The version of netperf is 2.4.5
> 
> 
>>Any chance that the CPU utilization you were
>>looking at in top was just that being charged to netperf the process?
> 
> 
> What I see on target machine is as follows:
> 
> top - 21:37:12 up 21 min,  6 users,  load average: 0.43, 0.28, 0.19
> Tasks: 152 total,   2 running, 149 sleeping,   0 stopped,   1 zombie
> Cpu(s):  2.3%us,  1.5%sy,  0.1%ni, 89.5%id,  2.7%wa,  0.0%hi,  3.9%si,  0.0%
> Mem:   2074064k total,   690200k used,  1383864k free,    39372k buffers
> Swap:  2096476k total,        0k used,  2096476k free,   435044k cached
> 
> PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND    
> 3916 root      20   0  2228  584  296 R 84.6  0.0   0:07.12 netserver    

You said this was a dual-core system right?  So two cores, no threads?  If so, 
then that does look odd - if netserver is consuming 84% of a CPU (core) and 
there are only two CPUs (cores) in the system, how the system can be 89.5% idle 
is beyond me. The 48% reported by netperf below makes better sense. If you press 
"1" while top is running it should start to show per-CPU statistics

> It shows the CPU usage of taget machine is around 10%.
> 
> while Nehalem machine's report is as follows:
> 
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.2.1 (192.168.2.1) port 0 AF_INET
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
> 
> 87380  16384  16384    10.05       679.79   1.63     48.27    1.571   11.634 
> 
> It shows the CPU usage of target machine is 48.27%.

Clearly something is out of joint - let's go off-list (or on to 
netperf-talk@netperf.org) and hash that out to see what may be happening.  It 
will probably involve variations on grabbing the top-of-trunk, adding the debug 
option etc.

> 
> 
>>"Network processing" does not often get charged to the responsible
>>process, so netperf reports system-wide CPU utilization on the
>>assumption it is the only thing causing the CPUs to be utilized.
> 
> 
> My understand of your commends is:
> 1)except running in ksoftirqd, network processing cannot be correctly counted
>   because it runs in interrupt contexts that do not get charged to a correct
>   process. So "top" misses lots of CPU usage in high interrupt rate network
>   situation.

Top *shouldn't* miss it as far as reporting overall CPU utlization.  It just may 
not be charged to the process on who's behalf the work is done.

> 2)As you have mentioned in netperf's manual, netperf uses /proc/stat on Linux
>   to retrieve time spent in idle mode. In other words, it accumulates cpu time
>   spent in all other modes, including hardware interrupt, software interrupt,
>   etc., making the CPU usage more accurate in high interrupt situation.

That is the theory.  In practice however...  while the top output you've 
provided looks like there is an "issue" in top, netperf has been known to have a 
bug or three.

> 3)Since most processes in target machine are in sleeping mode, the CPU usage
>   of network processing is in actually very close to 48.27%. Right?

I do not expect there to be a huge discrepancy between the overall CPU 
utilization reported by top and the CPU utilization reported by netperf.  That 
there seems to be such a discrepancy has me wanting to make certain that netperf 
is operating correctly.

happy benchmarking,

rick jones

> 
> Correct me if any of them are incorrect. Thanks.
> 
> --Junchang
> --
> 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: Very low latency TCP for clusters
From: Rick Jones @ 2010-07-20 17:24 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tom Herbert, netdev
In-Reply-To: <1279603570.2458.66.camel@edumazet-laptop>

Eric Dumazet wrote:
> Le lundi 19 juillet 2010 à 16:37 -0700, Tom Herbert a écrit :
> 
>>That's pretty pokey ;-) I see numbers around 25 usecs between to
>>machines, this is with TCP_NBRR.  With TCP_RR it's more like 35 usecs,
>>so eliminating the scheduler is already a big reduction.  That leaves
>>18 usecs in device time, interrupt processing, network, and cache
>>misses; 7 usecs in TCP processing, user space.  While 5 usecs is an
>>aggressive goal, I am not ready to concede that there's an
>>architectural limit in either NICs, TCP, or sockets that can't be
>>overcome.
> 
> Last time I tried TCP_NBRR, it was not working (not even compiled in), I
> guess I should submit a bug report to Rick ;)

Indeed!-)  Actually, my first thought upon reading what Tom wrote was "Wow, I'm 
amazed it still works" :)  That code probably hasn't been visited much since the 
heydays of T/TCP (transactional).

Getting it compiled-in probably required a hand-editing of the config.h file 
after the ./configure.  Patches to add a --enable-nbrr would be graciously accepted.

happy benchmarking,

rick jones

^ permalink raw reply

* [PATCH net-next-2.6] ixgbe: fix ethtool stats
From: Eric Dumazet @ 2010-07-20 17:28 UTC (permalink / raw)
  To: Jeff Kirsher, David Miller; +Cc: Jesse Brandeburg, PJ Waskiewicz, netdev

Note : I am currently unable to test following patch, could you please
Intel guys test it and Ack (or Nack) it ?

Thanks !

[PATCH net-next-2.6] ixgbe: fix ethtool stats

In latest changes about 64bit stats on 32bit arches,
[commit 28172739f0a276eb8 (net: fix 64 bit counters on 32 bit arches)],
I missed ixgbe uses a bit of magic in its ixgbe_gstrings_stats
definition.

IXGBE_NETDEV_STAT() must now assume offsets relative to
rtnl_link_stats64, not relative do dev->stats.

As a bonus, we also get 64bit stats on ethtool -S

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/ixgbe/ixgbe_ethtool.c |   42 ++++++++++++++--------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c
index da54b38..dcebc82 100644
--- a/drivers/net/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ixgbe/ixgbe_ethtool.c
@@ -54,14 +54,14 @@ struct ixgbe_stats {
 				sizeof(((struct ixgbe_adapter *)0)->m), \
 				offsetof(struct ixgbe_adapter, m)
 #define IXGBE_NETDEV_STAT(m)	NETDEV_STATS, \
-				sizeof(((struct net_device *)0)->m), \
-				offsetof(struct net_device, m) - offsetof(struct net_device, stats)
+				sizeof(((struct rtnl_link_stats64 *)0)->m), \
+				offsetof(struct rtnl_link_stats64, m)
 
 static struct ixgbe_stats ixgbe_gstrings_stats[] = {
-	{"rx_packets", IXGBE_NETDEV_STAT(stats.rx_packets)},
-	{"tx_packets", IXGBE_NETDEV_STAT(stats.tx_packets)},
-	{"rx_bytes", IXGBE_NETDEV_STAT(stats.rx_bytes)},
-	{"tx_bytes", IXGBE_NETDEV_STAT(stats.tx_bytes)},
+	{"rx_packets", IXGBE_NETDEV_STAT(rx_packets)},
+	{"tx_packets", IXGBE_NETDEV_STAT(tx_packets)},
+	{"rx_bytes", IXGBE_NETDEV_STAT(rx_bytes)},
+	{"tx_bytes", IXGBE_NETDEV_STAT(tx_bytes)},
 	{"rx_pkts_nic", IXGBE_STAT(stats.gprc)},
 	{"tx_pkts_nic", IXGBE_STAT(stats.gptc)},
 	{"rx_bytes_nic", IXGBE_STAT(stats.gorc)},
@@ -69,27 +69,27 @@ static struct ixgbe_stats ixgbe_gstrings_stats[] = {
 	{"lsc_int", IXGBE_STAT(lsc_int)},
 	{"tx_busy", IXGBE_STAT(tx_busy)},
 	{"non_eop_descs", IXGBE_STAT(non_eop_descs)},
-	{"rx_errors", IXGBE_NETDEV_STAT(stats.rx_errors)},
-	{"tx_errors", IXGBE_NETDEV_STAT(stats.tx_errors)},
-	{"rx_dropped", IXGBE_NETDEV_STAT(stats.rx_dropped)},
-	{"tx_dropped", IXGBE_NETDEV_STAT(stats.tx_dropped)},
-	{"multicast", IXGBE_NETDEV_STAT(stats.multicast)},
+	{"rx_errors", IXGBE_NETDEV_STAT(rx_errors)},
+	{"tx_errors", IXGBE_NETDEV_STAT(tx_errors)},
+	{"rx_dropped", IXGBE_NETDEV_STAT(rx_dropped)},
+	{"tx_dropped", IXGBE_NETDEV_STAT(tx_dropped)},
+	{"multicast", IXGBE_NETDEV_STAT(multicast)},
 	{"broadcast", IXGBE_STAT(stats.bprc)},
 	{"rx_no_buffer_count", IXGBE_STAT(stats.rnbc[0]) },
-	{"collisions", IXGBE_NETDEV_STAT(stats.collisions)},
-	{"rx_over_errors", IXGBE_NETDEV_STAT(stats.rx_over_errors)},
-	{"rx_crc_errors", IXGBE_NETDEV_STAT(stats.rx_crc_errors)},
-	{"rx_frame_errors", IXGBE_NETDEV_STAT(stats.rx_frame_errors)},
+	{"collisions", IXGBE_NETDEV_STAT(collisions)},
+	{"rx_over_errors", IXGBE_NETDEV_STAT(rx_over_errors)},
+	{"rx_crc_errors", IXGBE_NETDEV_STAT(rx_crc_errors)},
+	{"rx_frame_errors", IXGBE_NETDEV_STAT(rx_frame_errors)},
 	{"hw_rsc_aggregated", IXGBE_STAT(rsc_total_count)},
 	{"hw_rsc_flushed", IXGBE_STAT(rsc_total_flush)},
 	{"fdir_match", IXGBE_STAT(stats.fdirmatch)},
 	{"fdir_miss", IXGBE_STAT(stats.fdirmiss)},
-	{"rx_fifo_errors", IXGBE_NETDEV_STAT(stats.rx_fifo_errors)},
-	{"rx_missed_errors", IXGBE_NETDEV_STAT(stats.rx_missed_errors)},
-	{"tx_aborted_errors", IXGBE_NETDEV_STAT(stats.tx_aborted_errors)},
-	{"tx_carrier_errors", IXGBE_NETDEV_STAT(stats.tx_carrier_errors)},
-	{"tx_fifo_errors", IXGBE_NETDEV_STAT(stats.tx_fifo_errors)},
-	{"tx_heartbeat_errors", IXGBE_NETDEV_STAT(stats.tx_heartbeat_errors)},
+	{"rx_fifo_errors", IXGBE_NETDEV_STAT(rx_fifo_errors)},
+	{"rx_missed_errors", IXGBE_NETDEV_STAT(rx_missed_errors)},
+	{"tx_aborted_errors", IXGBE_NETDEV_STAT(tx_aborted_errors)},
+	{"tx_carrier_errors", IXGBE_NETDEV_STAT(tx_carrier_errors)},
+	{"tx_fifo_errors", IXGBE_NETDEV_STAT(tx_fifo_errors)},
+	{"tx_heartbeat_errors", IXGBE_NETDEV_STAT(tx_heartbeat_errors)},
 	{"tx_timeout_count", IXGBE_STAT(tx_timeout_count)},
 	{"tx_restart_queue", IXGBE_STAT(restart_queue)},
 	{"rx_long_length_errors", IXGBE_STAT(stats.roc)},



^ permalink raw reply related

* Re: [PATCH net-next] drivers/acpi/acpica/utmisc.c: Use printk extension %pV
From: Joe Perches @ 2010-07-20 18:16 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Len Brown, linux-acpi, linux-kernel, netdev, David Miller
In-Reply-To: <201007201008.04496.bjorn.helgaas@hp.com>

On Tue, 2010-07-20 at 10:08 -0600, Bjorn Helgaas wrote:
> On Monday, July 19, 2010 11:06:07 pm Joe Perches wrote:
> > Consolidates the printk messages to a single
> > call so the messages can not be interleaved.
> > 
> > Reduces text a bit.
> > 
> > $ size drivers/acpi/acpica/utmisc.o.*
> >    text	   data	    bss	    dec	    hex	filename
> >    7822	     56	   1832	   9710	   25ee	drivers/acpi/acpica/utmisc.o.old
> >    7748	     56	   1736	   9540	   2544	drivers/acpi/acpica/utmisc.o.new
> > 
> > Depends on net-next commit 7db6f5fb65a82af03229eef104dc9899c5eecf33
> > (vsprintf: Recursive vsnprintf: Add "%pV", struct va_format)
> 
> drivers/acpi/acpica/utmisc.c is part of the ACPI CA and is used in
> several different OSes, but %pV sounds like a Linux-specific feature,
> so I don't see how this patch can work.

Well, maybe something like this then.
It still reduces overall size a little bit, but less.

$ size drivers/acpi/acpica/utmisc.o
   text	   data	    bss	    dec	    hex	filename
   7816	     56	   1784	   9656	   25b8	drivers/acpi/acpica/utmisc.o

I'm not sure that utmisc.c is the right place for an #ifdef though.
---
 drivers/acpi/acpica/utmisc.c |   76 +++++++++++++++++++++++++++---------------
 1 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/acpica/utmisc.c b/drivers/acpi/acpica/utmisc.c
index e8d0724..874e4d2 100644
--- a/drivers/acpi/acpica/utmisc.c
+++ b/drivers/acpi/acpica/utmisc.c
@@ -51,10 +51,45 @@
 ACPI_MODULE_NAME("utmisc")
 
 /*
- * Common suffix for messages
+ * Single line messages, adds a newline after message
  */
-#define ACPI_COMMON_MSG_SUFFIX \
-	acpi_os_printf(" (%8.8X/%s-%u)\n", ACPI_CA_VERSION, module_name, line_number)
+
+static void ACPI_PRINTF_LIKE(5)
+acpi_put_message(const char *module_name, u32 line_number,
+		 const char *fmt, va_list *args, const char *message, ...)
+{
+#ifdef __KERNEL__
+	va_list message_args;
+	struct va_format vaf1;
+	struct va_format vaf2;
+
+	va_start(message_args, message);
+	vaf1.fmt = message;
+	vaf1.va = &message_args;
+	vaf2.fmt = fmt;
+	vaf2.va = args;
+	if (module_name)
+		acpi_os_printf("%pV%pV (%8.8X/%s-%u)\n",
+			       &vaf1, &vaf2,
+			       ACPI_CA_VERSION, module_name, line_number);
+	else
+		acpi_os_printf("%pV%pV\n", &vaf1, &vaf2);
+	va_end(message_args);
+#else
+	va_list message_args;
+
+	va_start(message_args, message);
+	acpi_os_vprintf(message, message_args);
+	acpi_os_vprintf(fmt, *args);
+	if (module_name)
+		acpi_os_printf(" (%8.8X/%s-%u)\n",
+			       ACPI_CA_VERSION, module_name, line_number);
+	else
+		acpi_os_printf("\n");
+	va_end(message_args);
+#endif
+}
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ut_validate_exception
@@ -1064,11 +1099,9 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
 {
 	va_list args;
 
-	acpi_os_printf("ACPI Error: ");
-
 	va_start(args, format);
-	acpi_os_vprintf(format, args);
-	ACPI_COMMON_MSG_SUFFIX;
+	acpi_put_message(module_name, line_number, format, &args,
+			 "ACPI Error: ");
 	va_end(args);
 }
 
@@ -1078,11 +1111,9 @@ acpi_exception(const char *module_name,
 {
 	va_list args;
 
-	acpi_os_printf("ACPI Exception: %s, ", acpi_format_exception(status));
-
 	va_start(args, format);
-	acpi_os_vprintf(format, args);
-	ACPI_COMMON_MSG_SUFFIX;
+	acpi_put_message(module_name, line_number, format, &args,
+			 "ACPI Exception: %s, ", acpi_format_exception(status));
 	va_end(args);
 }
 
@@ -1091,11 +1122,9 @@ acpi_warning(const char *module_name, u32 line_number, const char *format, ...)
 {
 	va_list args;
 
-	acpi_os_printf("ACPI Warning: ");
-
 	va_start(args, format);
-	acpi_os_vprintf(format, args);
-	ACPI_COMMON_MSG_SUFFIX;
+	acpi_put_message(module_name, line_number, format, &args,
+			 "ACPI Warning: ");
 	va_end(args);
 }
 
@@ -1104,11 +1133,8 @@ acpi_info(const char *module_name, u32 line_number, const char *format, ...)
 {
 	va_list args;
 
-	acpi_os_printf("ACPI: ");
-
 	va_start(args, format);
-	acpi_os_vprintf(format, args);
-	acpi_os_printf("\n");
+	acpi_put_message(NULL, 0, format, &args, "ACPI: ");
 	va_end(args);
 }
 
@@ -1152,11 +1178,9 @@ acpi_ut_predefined_warning(const char *module_name,
 		return;
 	}
 
-	acpi_os_printf("ACPI Warning for %s: ", pathname);
-
 	va_start(args, format);
-	acpi_os_vprintf(format, args);
-	ACPI_COMMON_MSG_SUFFIX;
+	acpi_put_message(module_name, line_number, format, &args,
+			 "ACPI Warning for %s: ", pathname);
 	va_end(args);
 }
 
@@ -1194,10 +1218,8 @@ acpi_ut_predefined_info(const char *module_name,
 		return;
 	}
 
-	acpi_os_printf("ACPI Info for %s: ", pathname);
-
 	va_start(args, format);
-	acpi_os_vprintf(format, args);
-	ACPI_COMMON_MSG_SUFFIX;
+	acpi_put_message(module_name, line_number, format, &args,
+			 "ACPI Info for %s: ", pathname);
 	va_end(args);
 }



^ permalink raw reply related

* Re: [PATCH] net: Add batman-adv meshing protocol
From: Marek Lindner @ 2010-07-20 18:23 UTC (permalink / raw)
  To: b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David Miller,
	siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX
In-Reply-To: <20100720.095928.113690788.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On Tuesday, July 20, 2010 18:59:28 David Miller wrote:
> Whoever you leave it to, this needs to be fixed up before this
> code can be integrated.  So if those people are not active or
> don't have time, you very much should consider the reality that
> you might have to do these fixups yourself.

No worries - we are quite alive and follow the discussion closely. The raised 
issues will be addressed pretty soon.

@Sven: I think you do a great job integrating all those patches and trees. 
Your helping hand would be really missed.  :)


Regards,
Marek

^ permalink raw reply

* Re: [B.A.T.M.A.N.] [PATCH] net: Add batman-adv meshing protocol
From: Abraham Arce @ 2010-07-20 18:41 UTC (permalink / raw)
  To: Marek Lindner; +Cc: b.a.t.m.a.n, David Miller, sven.eckelmann, netdev, siwu
In-Reply-To: <201007202023.17883.lindner_marek@yahoo.de>

Hi,

On Tue, Jul 20, 2010 at 1:23 PM, Marek Lindner <lindner_marek@yahoo.de> wrote:
> On Tuesday, July 20, 2010 18:59:28 David Miller wrote:
>> Whoever you leave it to, this needs to be fixed up before this
>> code can be integrated.  So if those people are not active or
>> don't have time, you very much should consider the reality that
>> you might have to do these fixups yourself.
>
> No worries - we are quite alive and follow the discussion closely. The raised
> issues will be addressed pretty soon.
>
> @Sven: I think you do a great job integrating all those patches and trees.
> Your helping hand would be really missed.  :)

Minor comments...

1. Is it ok to have a README in drivers directory? why not moving it
to Documentation/networking?
2. Compilation error, one extra arguments passed

- stats = (struct net_device_stats *)dev_get_stats(skb->dev, &temp);
+ stats = (struct net_device_stats *)dev_get_stats(skb->dev, &temp);

  LD      kernel/built-in.o
net/batman-adv/hard-interface.c: In function 'batman_skb_recv':
net/batman-adv/hard-interface.c:482: error: too many arguments to
function 'dev_get_stats'
make[2]: *** [net/batman-adv/hard-interface.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [net/batman-adv] Error 2
make: *** [net] Error 2

Best Regards
Abraham

^ permalink raw reply

* Re: [PATCH] net: Add batman-adv meshing protocol
From: Sven Eckelmann @ 2010-07-20 18:49 UTC (permalink / raw)
  To: Abraham Arce
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Marek Lindner,
	David Miller, siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX
In-Reply-To: <AANLkTim4QSjgC2BbrEJEpefQahbkSgaRPPfJ-N0aPL-E-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: Text/Plain, Size: 1034 bytes --]

Abraham Arce wrote:
> Minor comments...
> 
> 1. Is it ok to have a README in drivers directory? why not moving it
> to Documentation/networking?

Thanks, I will check if Documentation/networking is a better place.

> 2. Compilation error, one extra arguments passed
> 
> - stats = (struct net_device_stats *)dev_get_stats(skb->dev, &temp);
> + stats = (struct net_device_stats *)dev_get_stats(skb->dev, &temp);
> 
>   LD      kernel/built-in.o
> net/batman-adv/hard-interface.c: In function 'batman_skb_recv':
> net/batman-adv/hard-interface.c:482: error: too many arguments to
> function 'dev_get_stats'
> make[2]: *** [net/batman-adv/hard-interface.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [net/batman-adv] Error 2
> make: *** [net] Error 2

Do you use linux-next or net-next-2.6? Because this is needed to get it 
compiled as dev_get_stats was changed some time ago (commit 
28172739f0a276eb8d6ca917b3974c2edb036da3), but this is not part of 2.6.35.

Best regards,
	Sven

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

^ permalink raw reply


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