Netdev List
 help / color / mirror / Atom feed
* [PATCH] sctp: Fix error count increments that were results of HEARTBEATS
From: Vlad Yasevich @ 2009-08-26 14:46 UTC (permalink / raw)
  To: chunbo.luo; +Cc: linux-sctp, netdev, yjwei, Vlad Yasevich
In-Reply-To: <1251191279-1764-1-git-send-email-chunbo.luo@windriver.com>

Here is what I am applying.  After walking through all the code
paths that flow through the sctp_do_8_2_transport_strike() function,
it became obviouse that were not doing the right thing all the time.
The post-increment trick just obfuscated the real behavior and actually
had problems when HEARTBEAT and DATA mixed.  I've tried to make the
new code much clearer in its intentens and behavior and it solves the
problem in all of my simulations.

Feel free to comment.
-vlad

---

SCTP RFC 4960 states that unacknowledged HEARTBEATS count as
errors agains a given transport or endpoint.  As such, we
should increment the error counts for only for unacknowledged
HB, otherwise we detect failure too soon.  This goes for both
the overall error count and the path error count.

Now, there is a difference in how the detection is done
between the two.  The path error detection is done after
the increment, so to detect it properly, we actually need
to exceed the path threshold.  The overall error detection
is done _BEFORE_ the increment.  Thus to detect the failure,
it's enough for the error count to match the threshold.
This is why all the state functions use '>=' to detect failure,
while path detection uses '>'.

Thanks goes to Chunbo Luo <chunbo.luo@windriver.com> who first
proposed patches to fix this issue and made me re-read the spec
and the code to figure out how this cruft really works.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/sm_sideeffect.c |   20 ++++++++++++++++----
 net/sctp/sm_statefuns.c  |    2 +-
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 238adf7..41cb73b 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -440,14 +440,26 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
 	/* The check for association's overall error counter exceeding the
 	 * threshold is done in the state function.
 	 */
-	/* When probing UNCONFIRMED addresses, the association overall
-	 * error count is NOT incremented
+	/* We are here due to a timer expiration.  If the timer was
+	 * not a HEARTBEAT, then normal error tracking is done.
+	 * If the timer was a heartbeat, we only increment error counts
+	 * when we already have an outstanding HEARTBEAT that has not
+	 * been acknowledged.
+	 * Additionaly, some tranport states inhibit error increments.
 	 */
-	if (transport->state != SCTP_UNCONFIRMED)
+	if (!is_hb) {
 		asoc->overall_error_count++;
+		if (transport->state != SCTP_INACTIVE)
+			transport->error_count++;
+	 } else if (transport->hb_sent) {
+		if (transport->state != SCTP_UNCONFIRMED)
+			asoc->overall_error_count++;
+	 	if (transport->state != SCTP_INACTIVE)
+			transport->error_count++;
+	}
 
 	if (transport->state != SCTP_INACTIVE &&
-	    (transport->error_count++ >= transport->pathmaxrxt)) {
+	    (transport->error_count > transport->pathmaxrxt)) {
 		SCTP_DEBUG_PRINTK_IPADDR("transport_strike:association %p",
 					 " transport IP: port:%d failed.\n",
 					 asoc,
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 7fb08a6..45b8bca 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -971,7 +971,7 @@ sctp_disposition_t sctp_sf_sendbeat_8_3(const struct sctp_endpoint *ep,
 {
 	struct sctp_transport *transport = (struct sctp_transport *) arg;
 
-	if (asoc->overall_error_count > asoc->max_retrans) {
+	if (asoc->overall_error_count >= asoc->max_retrans) {
 		sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR,
 				SCTP_ERROR(ETIMEDOUT));
 		/* CMD_ASSOC_FAILED calls CMD_DELETE_TCB. */
-- 
1.5.4.3


^ permalink raw reply related

* Re: [Bug #14016] mm/ipw2200 regression
From: Andrew Morton @ 2009-08-26 14:44 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Johannes Weiner, Pekka Enberg, Rafael J. Wysocki,
	Linux Kernel Mailing List, Kernel Testers List,
	Bartlomiej Zolnierkiewicz, Mel Gorman, netdev, linux-mm, Zhu Yi,
	James Ketrenos, Reinette Chatre, linux-wireless, ipw2100-devel
In-Reply-To: <20090826093747.GA10955@csn.ul.ie>

(cc IPW maintainers and mailing lists)

On Wed, 26 Aug 2009 10:37:49 +0100 Mel Gorman <mel@csn.ul.ie> wrote:

> On Wed, Aug 26, 2009 at 10:27:41AM +0200, Johannes Weiner wrote:
> > [Cc netdev]
> > 
> > On Wed, Aug 26, 2009 at 09:09:44AM +0300, Pekka Enberg wrote:
> > > On Tue, Aug 25, 2009 at 11:34 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote:
> > > > This message has been generated automatically as a part of a report
> > > > of recent regressions.
> > > >
> > > > The following bug entry is on the current list of known regressions
> > > > from 2.6.30. __Please verify if it still should be listed and let me know
> > > > (either way).
> > > >
> > > > Bug-Entry __ __ __ : http://bugzilla.kernel.org/show_bug.cgi?id=14016
> > > > Subject __ __ __ __ : mm/ipw2200 regression
> > > > Submitter __ __ __ : Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > Date __ __ __ __ __ __: 2009-08-15 16:56 (11 days old)
> > > > References __ __ __: http://marc.info/?l=linux-kernel&m=125036437221408&w=4
> > > 
> > > If am reading the page allocator dump correctly, there's plenty of
> > > pages left but we're unable to satisfy an order 6 allocation. There's
> > > no slab allocator involved so the page allocator changes that went
> > > into 2.6.31 seem likely. Mel, ideas?
> > 
> > It's an atomic order-6 allocation, the chances for this to succeed
> > after some uptime become infinitesimal.  The chunks > order-2 are
> > pretty much exhausted on this dump.
> > 
> > 64 pages, presumably 256k, for fw->boot_size while current ipw
> > firmware images have ~188k.  I don't know jack squat about this
> > driver, but given the field name and the struct:
> > 
> > 	struct ipw_fw {
> > 		__le32 ver;
> > 		__le32 boot_size;
> > 		__le32 ucode_size;
> > 		__le32 fw_size;
> > 		u8 data[0];
> > 	};
> > 
> > fw->boot_size alone being that big sounds a bit fishy to me.
> > 
> 
> Agreed. While there are a low number of order-6 pages free in the page
> allocation failure dump, there are not enough for watermarks to be
> satisified. As it's atomic, there is little that can be done from a VM
> perspective and it's the responsibility of the driver. I'm no driver expert
> but I'll have a go at fixing it anyway.
> 
> My reading of this is that the firmware is being loaded from a workqueue and
> I am failing to see any restriction on sleeping in the path. It would appear
> that the driver just used the most convenient *_alloc_coherent function
> available forgetting that it assumes GFP_ATOMIC. Can someone who does know
> which way is up with a driver tell me why the patch below might not
> work?
> 
> Bartlomiej, any chance you could give this a spin? Preferably, you'd
> have preempt enabled and CONFIG_DEBUG_SPINLOCK_SLEEP on as well because
> that combination will complain loudly if we really can't sleep in this
> path.
> 
> =====
> ipw2200: Avoid large GFP_ATOMIC allocation during firmware loading
> 
> ipw2200 uses pci_alloc_consistent() to allocate a large coherent buffer for
> the loading of firmware which is an order-6 allocation of GFP_ATOMIC. At
> system start-up time, this is not a problem. However, the firmware on the
> card can get confused and the corrective action taken is to reload the
> firmware and reinit the card. High-order GFP_ATOMIC allocations of this
> type can and will fail when the system is already up and running.
> 
> As the firmware is loaded from a workqueue, it should be possible for
> the driver to go to sleep. This patch converts the call of
> pci_alloc_consistent() which assumes GFP_ATOMIC to dma_alloc_coherent()
> which can specify its own flags.
> 
> The big downside with this patch is that it uses GFP_REPEAT to avoid the
> driver unloading. There is potential that this will cause a reclaim
> storm as the machine tries to find a free order-6 buffer. A suggested
> alternative for the driver owner is in the comments.
> 
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> --- 
>  drivers/net/wireless/ipw2x00/ipw2200.c |   14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c
> index 44c29b3..f2e251e 100644
> --- a/drivers/net/wireless/ipw2x00/ipw2200.c
> +++ b/drivers/net/wireless/ipw2x00/ipw2200.c
> @@ -3167,7 +3167,19 @@ static int ipw_load_firmware(struct ipw_priv *priv, u8 * data, size_t len)
>  	u8 *shared_virt;
>  
>  	IPW_DEBUG_TRACE("<< : \n");
> -	shared_virt = pci_alloc_consistent(priv->pci_dev, len, &shared_phys);
> +
> +	/*
> +	 * This is a whopping large allocation, in or around order-6 so
> +	 * dma_alloc_coherent is used to specify the GFP_KERNEL|__GFP_REPEAT
> +	 * flags. Note that this action means the system could go into a
> +	 * reclaim loop until it cannot reclaim any more trying to satisfy
> +	 * the allocation. It would be preferable if one buffer is allocated
> +	 * at driver initialisation and reused when the firmware needs to
> +	 * be reloaded, overwriting the existing firmware each time
> +	 */
> +	shared_virt = dma_alloc_coherent(
> +			priv->pci_dev == NULL ? NULL : &priv->pci_dev->dev, 
> +			len, &shared_phys, GFP_KERNEL|__GFP_REPEAT);
>  
>  	if (!shared_virt)
>  		return -ENOMEM;

Of course, the risk of making a change like this is that we'll then go
and leave it there.

To fix this code properly we should, as you say, stop doing an order-6
allocation altogether.

And right now I think it's doing _two_ order-6 allocations:

	shared_virt = pci_alloc_consistent(priv->pci_dev, len, &shared_phys);

	if (!shared_virt)
		return -ENOMEM;

	memmove(shared_virt, data, len);

whoever allocated `data' is being obnoxious as well.

It is perhaps pretty simple to make the second (GFP_ATOMIC) allocation
go away.  The code is already conveniently structured to do this:

	do {
		chunk = (struct fw_chunk *)(data + offset);
		offset += sizeof(struct fw_chunk);
		/* build DMA packet and queue up for sending */
		/* dma to chunk->address, the chunk->length bytes from data +
		 * offeset*/
		/* Dma loading */
		rc = ipw_fw_dma_add_buffer(priv, shared_phys + offset,
					   le32_to_cpu(chunk->address),
					   le32_to_cpu(chunk->length));
		if (rc) {
			IPW_DEBUG_INFO("dmaAddBuffer Failed\n");
			goto out;
		}

		offset += le32_to_cpu(chunk->length);
	} while (offset < len);

what is the typical/expected value of chunk->length here?  If it's
significantly less than 4096*(2^6), could we convert this function to
use a separate DMAable allocation per fw_chunk?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Eric Dumazet @ 2009-08-26 14:22 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Sridhar Samudrala, David Stevens, David S. Miller, netdev, niv,
	sri
In-Reply-To: <alpine.DEB.1.10.0908261006520.9924@gentwo.org>

Christoph Lameter a écrit :
> I also see no UDP stats for packet drops. If Erics fix gets in then
> ip_push_pending_frames() will give us an error code on drop.
> Incrementing SNDBUFERRORS would require this simple patch.
> 

I think it's already done in udp_sendmsg()

Code starting at line 765 in net/ipv4/udp.c

        /*
         * ENOBUFS = no kernel mem, SOCK_NOSPACE = no sndbuf space.  Reporting
         * ENOBUFS might not be good (it's not tunable per se), but otherwise
         * we don't have a good statistic (IpOutDiscards but it can be too many
         * things).  We could add another new stat but at least for now that
         * seems like overkill.
         */
        if (err == -ENOBUFS || test_bit(SOCK_NOSPACE, &sk->sk_socket->flags)) {
                UDP_INC_STATS_USER(sock_net(sk),
                                UDP_MIB_SNDBUFERRORS, is_udplite);
        }


> 
> UDP: Account for TX drops
> 
> UDP layer is currently not incrementing error counters when packets are
> dropped. Use the SNDBUFERRORS to indicate packet drops on send.
> 
> Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
> 
> ---
>  net/ipv4/udp.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> Index: linux-2.6/net/ipv4/udp.c
> ===================================================================
> --- linux-2.6.orig/net/ipv4/udp.c	2009-08-26 13:21:30.000000000 +0000
> +++ linux-2.6/net/ipv4/udp.c	2009-08-26 13:46:35.000000000 +0000
> @@ -559,6 +559,13 @@ static int udp_push_pending_frames(struc
> 
>  send:
>  	err = ip_push_pending_frames(sk);
> +
> +	if (err)
> +		/*
> +		 * Packet was dropped.
> +		 */
> +		UDP_INC_STATS_USER(sock_net(sk),
> +			UDP_MIB_SNDBUFERRORS, is_udplite);
>  out:
>  	up->len = 0;
>  	up->pending = 0;


^ permalink raw reply

* Re: [net-next 6/10] bnx2x: Update vlan_features
From: Or Gerlitz @ 2009-08-26 14:18 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Patrick McHardy, David Miller, netdev, Eric Dumazet
In-Reply-To: <3426.1248464836@death.nxdomain.ibm.com>

Jay Vosburgh wrote:
> 	Patrick, can you clarify one bit about your above statment?  You
> say the bonding features should be an "intersection"; is that a strict
> intersection (i.e., slave1->vlan_features | slave2->vlan_features), or
> does the NETIF_F_ONE_FOR_ALL logic apply for vlan_features as it does
> for regular dev->features (using netdev_incrmenet_features() to combine
> the feature sets)?
>
> 	In other words, if a bond has two slaves, one with, e.g.,
> NETIF_F_SG in its vlan_features, and the other slave has 0 in
> vlan_features, should the bond's vlan_features be NETIF_F_SG, or 0?
>   
Hi Jay,

So we're @ rc7 again and the "bonding: propogate vlan_features to 
bonding master" patch isn't queued yet... can you look into that so we 
have this for 2.6.32?

thanks,

Or.


^ permalink raw reply

* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Christoph Lameter @ 2009-08-26 14:08 UTC (permalink / raw)
  To: Sridhar Samudrala
  Cc: David Stevens, David S. Miller, Eric Dumazet, netdev,
	netdev-owner, niv, sri
In-Reply-To: <1251239734.3169.65.camel@w-sridhar.beaverton.ibm.com>

I also see no UDP stats for packet drops. If Erics fix gets in then
ip_push_pending_frames() will give us an error code on drop.
Incrementing SNDBUFERRORS would require this simple patch.


UDP: Account for TX drops

UDP layer is currently not incrementing error counters when packets are
dropped. Use the SNDBUFERRORS to indicate packet drops on send.

Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 net/ipv4/udp.c |    7 +++++++
 1 file changed, 7 insertions(+)

Index: linux-2.6/net/ipv4/udp.c
===================================================================
--- linux-2.6.orig/net/ipv4/udp.c	2009-08-26 13:21:30.000000000 +0000
+++ linux-2.6/net/ipv4/udp.c	2009-08-26 13:46:35.000000000 +0000
@@ -559,6 +559,13 @@ static int udp_push_pending_frames(struc

 send:
 	err = ip_push_pending_frames(sk);
+
+	if (err)
+		/*
+		 * Packet was dropped.
+		 */
+		UDP_INC_STATS_USER(sock_net(sk),
+			UDP_MIB_SNDBUFERRORS, is_udplite);
 out:
 	up->len = 0;
 	up->pending = 0;

^ permalink raw reply

* Re: [PATCH 1/2] trace_events: fix napi's tracepoint
From: Steven Rostedt @ 2009-08-26 14:01 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Ingo Molnar, Frederic Weisbecker, Neil Horman, Wei Yongjun,
	David Miller, Netdev, LKML
In-Reply-To: <4A94CACD.6020309@cn.fujitsu.com>


On Wed, 26 Aug 2009, Xiao Guangrong wrote:

> 
> 
> Xiao Guangrong wrote:
> > 
> > Xiao Guangrong wrote:
> >> Currently, the napi's tracepoint works will is depend on
> >> "DECLARE_TRACE" definiens in include/trace/define_trace.h,
> >> like below:
> >>
> >> #include <trace/events/skb.h>    // include define_trace.h
> >> #include <trace/events/napi.h>
> >>
> >> there have error, if we remove "#include <trace/events/skb.h>"
> >> or include napi.h in the front of include skb.h, It should
> >> depend on the definiens in include/linux/tracepoint.h and we
> >> can remove the "DECLARE_TRACE" definiens in
> >> include/trace/define_trace.h, because "TRACE_EVENT" not use it
> >>
> >> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> > 
> > Hi Steven,
> > 
> > I'm sorry, please pull this patch too, because 
> > "[PATCH 7/8] tracing/events: fix the include file dependencies" 
> > is based on this patch, else will occur building error.
> > 
> 
> Sorry again, I say the wrong words, it not has building error, just
> not complete fix the bug which I mention it in the changelog of
> "[PATCH 7/8] tracing/events: fix the include file dependencies", that
> is we can't include more TRACE_EVENT head file in .c file all the same,
> like below:
> 
> Both define TRACE_EVENT in trace_a.h and trace_b.h, if we include
> those in .c file, like this:
> 
> #define CREATE_TRACE_POINTS
> include <trace/events/trace_a.h>  // re-define DECLARE_TRACE
> 
> include <trace/events/trace_b.h>  // use the DECLARE_TRACE definition
> 				  // that re-define by trace_a.h

Hi Xiao,

I pushed out the "fix the include file dependencies" patch since that was 
a bug fix in its own right. I held off the NAPI patch because it looked 
like it needed rework and only depended on the include dependencies fix. 

Could you update this code to do as Neil and Ingo has mentioned.

Thanks,

-- Steve

^ permalink raw reply

* Re: [PATCH] r8169: Reduce looping in the interrupt handler.
From: David Dillow @ 2009-08-26 13:59 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Francois Romieu, Michael Riepe, Michael Buesch, Rui Santos,
	Michael B??ker, linux-kernel, netdev
In-Reply-To: <1251294974.14241.9.camel@obelisk.thedillows.org>

On Wed, 2009-08-26 at 09:56 -0400, David Dillow wrote:
> On Wed, 2009-08-26 at 00:58 -0700, Eric W. Biederman wrote:
> > diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> > index 3b19e0c..2214945 100644
> > --- a/drivers/net/r8169.c
> > +++ b/drivers/net/r8169.c
> 
> > +
> > +		/* Ignore the parts of status that reflect more than
> > +		 * the enabled interrupts.
> > +		 */
> > +		smp_rmb();
> > +		if (!(status & tp->intr_mask & tp->intr_event))
> > +			break;
> >  	}
> 
> This looks like an odd construct, since we're just about to go back the
> while condition up top -- why not just mask it here and let the loop
> handle it naturally?

Never mind, I see what you are doing -- avoiding a false loop if we get
status == 0xffff. I still don't like the aesthetics of it, but it makes
sense, and I'll blame it on the card. :)

I should really get some caffeine before posting...  


^ permalink raw reply

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-26 13:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: virtualization, Rusty Russell, kvm, netdev, linux-kernel,
	linux-mm, hpa, mingo, akpm
In-Reply-To: <200908261540.59900.arnd@arndb.de>

On Wed, Aug 26, 2009 at 03:40:59PM +0200, Arnd Bergmann wrote:
> On Tuesday 25 August 2009, Michael S. Tsirkin wrote:
> > >  I'd like to avoid that here,
> > > though it's kind of ugly.  We'd need VHOST_GET_FEATURES (and ACK) to take a
> > > struct like:
> > > 
> > >       u32 feature_size;
> > >       u32 features[];
> 
> Hmm, variable length ioctl arguments, I'd rather not go there.
> The ioctl infrastructure already has a length argument encoded
> in the ioctl number. We can use that if we need more, e.g.
> 
> /* now */
> #define VHOST_GET_FEATURES     _IOR(VHOST_VIRTIO, 0x00, __u64)
> /*
>  * uncomment if we run out of feature bits:
> 
> struct vhost_get_features2 {
> 	__u64 bits[2];
> };
> #define VHOST_GET_FEATURES2     _IOR(VHOST_VIRTIO, 0x00, \
> 			struct  vhost_get_features2)
>  */
> 
> > Thinking about this proposal some more, how will the guest
> > determine the size to supply the GET_FEATURES ioctl?
> 
> Wait, the *guest*?

Sorry. the userspace hypervisor.

> Maybe I misunderstood something in a major way here, but
> I expected the features to be negotiated between host
> user space (qemu) and host kernel, as well as between
> guest and qemu (as they are already), but never between
> guest and kernel.

Yes.

> I would certainly expect the bits to be distinct from
> the virtio-net feature bits. E.g. stuff like TAP frame
> format opposed to TCP socket frame format (length+date)
> is something we need to negotiate here but that the
> guest does not care about.

My idea is to use virtio format for things I share with virtio (e.g.
mergeable buffers).  Since we are a kind of transport, I thought that I
will use the transport bits, that is bits 28 and up for vhost things.

> > Since we are a bit tight in 32 bit space already,
> > let's just use a 64 bit integer and be done with it?
> 
> Can't hurt, but don't use a struct unless you think
> we are going to need more than 64 bits.
> 
> 	Arnd <><

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH] r8169: Reduce looping in the interrupt handler.
From: David Dillow @ 2009-08-26 13:56 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Francois Romieu, Michael Riepe, Michael Buesch, Rui Santos,
	Michael B??ker, linux-kernel, netdev
In-Reply-To: <m1iqgbknun.fsf_-_@fess.ebiederm.org>

On Wed, 2009-08-26 at 00:58 -0700, Eric W. Biederman wrote:
> As of 2.6.30 I have been observing soft lockups and netdev watchdog
> timeouts caused by looping in the r8169 interrupt handler.
> 
> - Introduce a hard limit to the maximum number of times we will loop in
>   the interrupt handler, and print a message when we hit it.
> 
> - Break out of the loop if after looking none of the events in status
>   are events we expect to be delivered by an interrupt.
> 
> With just the hard limit and message bits of my patch in my test case
> I get hit my limit of 10 loops 12 times.  After filtering by intr_mask
> and intr_event I don't get any warnings.
> 
> Any complaints from those who know the driver better than I?

Have you tried this under a sustained heavy transmit load? I think you
may have reintroduced the problem the original patch was trying to
prevent -- wedging the MSI interrupt by not ACKing all outstanding
interrupt sources, masked or otherwise. I'll try to test this out on the
machine where I've seen the problem consistently, but it may be this
weekend.

> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 3b19e0c..2214945 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c

> +
> +		/* Ignore the parts of status that reflect more than
> +		 * the enabled interrupts.
> +		 */
> +		smp_rmb();
> +		if (!(status & tp->intr_mask & tp->intr_event))
> +			break;
>  	}

This looks like an odd construct, since we're just about to go back the
while condition up top -- why not just mask it here and let the loop
handle it naturally?

^ permalink raw reply

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Arnd Bergmann @ 2009-08-26 13:40 UTC (permalink / raw)
  To: virtualization
  Cc: Michael S. Tsirkin, Rusty Russell, kvm, netdev, linux-kernel,
	linux-mm, hpa, mingo, akpm
In-Reply-To: <20090825175016.GA15790@redhat.com>

On Tuesday 25 August 2009, Michael S. Tsirkin wrote:
> >  I'd like to avoid that here,
> > though it's kind of ugly.  We'd need VHOST_GET_FEATURES (and ACK) to take a
> > struct like:
> > 
> >       u32 feature_size;
> >       u32 features[];

Hmm, variable length ioctl arguments, I'd rather not go there.
The ioctl infrastructure already has a length argument encoded
in the ioctl number. We can use that if we need more, e.g.

/* now */
#define VHOST_GET_FEATURES     _IOR(VHOST_VIRTIO, 0x00, __u64)
/*
 * uncomment if we run out of feature bits:

struct vhost_get_features2 {
	__u64 bits[2];
};
#define VHOST_GET_FEATURES2     _IOR(VHOST_VIRTIO, 0x00, \
			struct  vhost_get_features2)
 */

> Thinking about this proposal some more, how will the guest
> determine the size to supply the GET_FEATURES ioctl?

Wait, the *guest*?

Maybe I misunderstood something in a major way here, but
I expected the features to be negotiated between host
user space (qemu) and host kernel, as well as between
guest and qemu (as they are already), but never between
guest and kernel.

I would certainly expect the bits to be distinct from
the virtio-net feature bits. E.g. stuff like TAP frame
format opposed to TCP socket frame format (length+date)
is something we need to negotiate here but that the
guest does not care about.

> Since we are a bit tight in 32 bit space already,
> let's just use a 64 bit integer and be done with it?

Can't hurt, but don't use a struct unless you think
we are going to need more than 64 bits.

	Arnd <><

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [BUG bisected] 2.6.30.4: NET: active FTP doesn't work
From: Patrick McHardy @ 2009-08-26 12:51 UTC (permalink / raw)
  To: Vitezslav Samel; +Cc: Krzysztof Piotr Oledzki, Greg Kroah-Hartman, netdev
In-Reply-To: <20090826083358.GB6010@pc11.op.pod.cz>

Vitezslav Samel wrote:
> 	Hi!
> 
>   When testing 2.6.30.4+ kernel, I discovered active ftp does not work
> anymore (2.6.30.3 is OK).
> 
>                LAN              internet
>    machine1  -------  router1   -------   machine2
> 
> 
>  Active ftp from machine1 to machine2 connects, transfers some files and
> then hangs. Passive ftp works OK. The faulty box (router1)
> is router/firewall and is doing NAT.
> 
>   I bisected it down to commit 7500f93f415a2fc07e0031d99fa3964bf8981cfc
> in linux-2.6.30.y tree ("netfilter: tcp conntrack: fix unacknowledged
> data detection with NAT").
> 
>   Could you, please, take look at it?

Please send me a tcpdump (binary dump, with -o <file> and -s0) of an
FTP session on both the internal and external interfaces and your NAT
rules. Thanks.

^ permalink raw reply

* Re: [PATCH] 8 bytes kernel memory disclosure in AppleTalk getsockname.
From: Clement LECIGNE @ 2009-08-26 12:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-kernel, netdev
In-Reply-To: <4A952C2D.2010807@gmail.com>

Wed, Aug 26, 2009 at 02:35:57PM +0200, Eric Dumazet wrote:
> Clement LECIGNE a écrit :
> > Hi,
> > 
> > In function atalk_getname(), sockaddr_at is returned in userland without
> > zero'ing the "char sat_zero[8]" field. This bug allows user to display 8
> > bytes leaked from the kernel stack.
> > 
> > Here is a patch that zero the whole sockaddr_at structure before
> > processing it. It should fix this bug.
> > 
> > Signed-off-by: Clément Lecigne <clement.lecigne@netasq.com>
> > --- linux/net/appletalk/ddp.c	2009-08-26 11:35:59.000000000 +0200
> > +++ linux/net/appletalk/ddp.c	2009-08-26 11:36:30.000000000 +0200
> > @@ -1241,6 +1241,8 @@ static int atalk_getname(struct socket *
> >  		if (atalk_autobind(sk) < 0)
> >  			return -ENOBUFS;
> >  
> > +	memset(&sat, 0, sizeof(struct sockaddr_at));
> > +
> >  	*uaddr_len = sizeof(struct sockaddr_at);
> >  
> >  	if (peer) {
> > 
> Hi Clement
> 
> Well, I submitted same patch some weeks ago and I just checked that
> it was already in Linus tree.
> 
> author	Eric Dumazet <eric.dumazet@gmail.com>
> 	Thu, 6 Aug 2009 02:27:43 +0000 (02:27 +0000)
> committer	David S. Miller <davem@davemloft.net>
> 	Thu, 6 Aug 2009 20:08:45 +0000 (13:08 -0700)
> commit	3d392475c873c10c10d6d96b94d092a34ebd4791
> 
> appletalk: fix atalk_getname() leak
> 
> atalk_getname() can leak 8 bytes of kernel memory to user
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> 
> Dont worry, it'll be included in upcoming 2.6.31 kernel,
> and backported to previous ones as well.

Hi Eric,

Oups, shame on me, I have not checked Linus tree before submitting the
patch.

Sorry,

-- 
Clément LECIGNE,
-Only one remote hole in the default install, in more than 10 years!<br>
+Only two remote holes in the default install, in more than 10 years!<br>


^ permalink raw reply

* Re: [PATCH] 8 bytes kernel memory disclosure in AppleTalk getsockname.
From: Eric Dumazet @ 2009-08-26 12:38 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: clement.lecigne, linux-kernel, netdev
In-Reply-To: <200908262101.DBJ57868.OHOFFVLQOtMFSJ@I-love.SAKURA.ne.jp>

Tetsuo Handa a écrit :
> Clement LECIGNE wrote:
>> Here is a patch that zero the whole sockaddr_at structure before
>> processing it. It should fix this bug.
> 
> Same kind of bug at http://lkml.org/lkml/2006/7/25/51 .
> I wonder why not zero'ing sockaddr at the caller rather than
> zero'ing at individual ->getname(). Doing so will save some lines.
> --

It would not help zeroing buffer before calling ->getname(), since
getname() will overwrite it with a memcpy(buffer, &somestruct, sizeof(somestruct));

^ permalink raw reply

* Re: [PATCH] 8 bytes kernel memory disclosure in AppleTalk getsockname.
From: Eric Dumazet @ 2009-08-26 12:35 UTC (permalink / raw)
  To: Clement LECIGNE; +Cc: linux-kernel, netdev
In-Reply-To: <20090826111247.GA79673@clem1.netasq.com>

Clement LECIGNE a écrit :
> Hi,
> 
> In function atalk_getname(), sockaddr_at is returned in userland without
> zero'ing the "char sat_zero[8]" field. This bug allows user to display 8
> bytes leaked from the kernel stack.
> 
> Here is a patch that zero the whole sockaddr_at structure before
> processing it. It should fix this bug.
> 
> Signed-off-by: Clément Lecigne <clement.lecigne@netasq.com>
> --- linux/net/appletalk/ddp.c	2009-08-26 11:35:59.000000000 +0200
> +++ linux/net/appletalk/ddp.c	2009-08-26 11:36:30.000000000 +0200
> @@ -1241,6 +1241,8 @@ static int atalk_getname(struct socket *
>  		if (atalk_autobind(sk) < 0)
>  			return -ENOBUFS;
>  
> +	memset(&sat, 0, sizeof(struct sockaddr_at));
> +
>  	*uaddr_len = sizeof(struct sockaddr_at);
>  
>  	if (peer) {
> 
Hi Clement

Well, I submitted same patch some weeks ago and I just checked that
it was already in Linus tree.

author	Eric Dumazet <eric.dumazet@gmail.com>
	Thu, 6 Aug 2009 02:27:43 +0000 (02:27 +0000)
committer	David S. Miller <davem@davemloft.net>
	Thu, 6 Aug 2009 20:08:45 +0000 (13:08 -0700)
commit	3d392475c873c10c10d6d96b94d092a34ebd4791

appletalk: fix atalk_getname() leak

atalk_getname() can leak 8 bytes of kernel memory to user

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>


Dont worry, it'll be included in upcoming 2.6.31 kernel,
and backported to previous ones as well.

^ permalink raw reply

* Re: Page allocation failures in guest
From: Rusty Russell @ 2009-08-26 12:18 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Avi Kivity, Minchan Kim, kvm, LKML, linux-mm, Wu Fengguang,
	KOSAKI Motohiro, Rik van Riel, netdev
In-Reply-To: <20090826065501.7ab677b9@mjolnir.ossman.eu>

On Wed, 26 Aug 2009 02:25:01 pm Pierre Ossman wrote:
> On Wed, 26 Aug 2009 11:47:17 +0930
> Rusty Russell <rusty@rustcorp.com.au> wrote:
> 
> > On Fri, 14 Aug 2009 05:55:48 am Pierre Ossman wrote:
> > > On Wed, 12 Aug 2009 15:01:52 +0930
> > > Rusty Russell <rusty@rustcorp.com.au> wrote:
> > > > Subject: virtio: net refill on out-of-memory
> > ... 
> > > Patch applied. Now we wait. :)
> > 
> > Any results?
> > 
> 
> It's been up for 12 days, so I'd say it works. But there is nothing in
> dmesg, which suggests I haven't triggered the condition yet.

No, that's totally expected.  I wouldn't expect a GFP_ATOMIC order 0 alloc
failure to be noted, and the patch doesn't add any printks.

Dave, can you push this to Linus ASAP?

Thanks,
Rusty.

Subject: virtio: net refill on out-of-memory

If we run out of memory, use keventd to fill the buffer.  There's a
report of this happening: "Page allocation failures in guest",
Message-ID: <20090713115158.0a4892b0@mjolnir.ossman.eu>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 drivers/net/virtio_net.c |   61 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 46 insertions(+), 15 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -71,6 +71,9 @@ struct virtnet_info
 	struct sk_buff_head recv;
 	struct sk_buff_head send;
 
+	/* Work struct for refilling if we run low on memory. */
+	struct delayed_work refill;
+
 	/* Chain pages by the private ptr. */
 	struct page *pages;
 };
@@ -274,19 +277,22 @@ drop:
 	dev_kfree_skb(skb);
 }
 
-static void try_fill_recv_maxbufs(struct virtnet_info *vi)
+static bool try_fill_recv_maxbufs(struct virtnet_info *vi, gfp_t gfp)
 {
 	struct sk_buff *skb;
 	struct scatterlist sg[2+MAX_SKB_FRAGS];
 	int num, err, i;
+	bool oom = false;
 
 	sg_init_table(sg, 2+MAX_SKB_FRAGS);
 	for (;;) {
 		struct virtio_net_hdr *hdr;
 
 		skb = netdev_alloc_skb(vi->dev, MAX_PACKET_LEN + NET_IP_ALIGN);
-		if (unlikely(!skb))
+		if (unlikely(!skb)) {
+			oom = true;
 			break;
+		}
 
 		skb_reserve(skb, NET_IP_ALIGN);
 		skb_put(skb, MAX_PACKET_LEN);
@@ -297,7 +303,7 @@ static void try_fill_recv_maxbufs(struct
 		if (vi->big_packets) {
 			for (i = 0; i < MAX_SKB_FRAGS; i++) {
 				skb_frag_t *f = &skb_shinfo(skb)->frags[i];
-				f->page = get_a_page(vi, GFP_ATOMIC);
+				f->page = get_a_page(vi, gfp);
 				if (!f->page)
 					break;
 
@@ -326,31 +332,35 @@ static void try_fill_recv_maxbufs(struct
 	if (unlikely(vi->num > vi->max))
 		vi->max = vi->num;
 	vi->rvq->vq_ops->kick(vi->rvq);
+	return !oom;
 }
 
-static void try_fill_recv(struct virtnet_info *vi)
+/* Returns false if we couldn't fill entirely (OOM). */
+static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 {
 	struct sk_buff *skb;
 	struct scatterlist sg[1];
 	int err;
+	bool oom = false;
 
-	if (!vi->mergeable_rx_bufs) {
-		try_fill_recv_maxbufs(vi);
-		return;
-	}
+	if (!vi->mergeable_rx_bufs)
+		return try_fill_recv_maxbufs(vi, gfp);
 
 	for (;;) {
 		skb_frag_t *f;
 
 		skb = netdev_alloc_skb(vi->dev, GOOD_COPY_LEN + NET_IP_ALIGN);
-		if (unlikely(!skb))
+		if (unlikely(!skb)) {
+			oom = true;
 			break;
+		}
 
 		skb_reserve(skb, NET_IP_ALIGN);
 
 		f = &skb_shinfo(skb)->frags[0];
-		f->page = get_a_page(vi, GFP_ATOMIC);
+		f->page = get_a_page(vi, gfp);
 		if (!f->page) {
+			oom = true;
 			kfree_skb(skb);
 			break;
 		}
@@ -374,6 +384,7 @@ static void try_fill_recv(struct virtnet
 	if (unlikely(vi->num > vi->max))
 		vi->max = vi->num;
 	vi->rvq->vq_ops->kick(vi->rvq);
+	return !oom;
 }
 
 static void skb_recv_done(struct virtqueue *rvq)
@@ -386,6 +397,23 @@ static void skb_recv_done(struct virtque
 	}
 }
 
+static void refill_work(struct work_struct *work)
+{
+	struct virtnet_info *vi;
+	bool still_empty;
+
+	vi = container_of(work, struct virtnet_info, refill.work);
+	napi_disable(&vi->napi);
+	try_fill_recv(vi, GFP_KERNEL);
+	still_empty = (vi->num == 0);
+	napi_enable(&vi->napi);
+
+	/* In theory, this can happen: if we don't get any buffers in
+	 * we will *never* try to fill again. */
+	if (still_empty)
+		schedule_delayed_work(&vi->refill, HZ/2);
+}
+
 static int virtnet_poll(struct napi_struct *napi, int budget)
 {
 	struct virtnet_info *vi = container_of(napi, struct virtnet_info, napi);
@@ -401,10 +429,10 @@ again:
 		received++;
 	}
 
-	/* FIXME: If we oom and completely run out of inbufs, we need
-	 * to start a timer trying to fill more. */
-	if (vi->num < vi->max / 2)
-		try_fill_recv(vi);
+	if (vi->num < vi->max / 2) {
+		if (!try_fill_recv(vi, GFP_ATOMIC))
+			schedule_delayed_work(&vi->refill, 0);
+	}
 
 	/* Out of packets? */
 	if (received < budget) {
@@ -894,6 +922,7 @@ static int virtnet_probe(struct virtio_d
 	vi->vdev = vdev;
 	vdev->priv = vi;
 	vi->pages = NULL;
+	INIT_DELAYED_WORK(&vi->refill, refill_work);
 
 	/* If they give us a callback when all buffers are done, we don't need
 	 * the timer. */
@@ -942,7 +971,7 @@ static int virtnet_probe(struct virtio_d
 	}
 
 	/* Last of all, set up some receive buffers. */
-	try_fill_recv(vi);
+	try_fill_recv(vi, GFP_KERNEL);
 
 	/* If we didn't even get one input buffer, we're useless. */
 	if (vi->num == 0) {
@@ -959,6 +988,7 @@ static int virtnet_probe(struct virtio_d
 
 unregister:
 	unregister_netdev(dev);
+	cancel_delayed_work_sync(&vi->refill);
 free_vqs:
 	vdev->config->del_vqs(vdev);
 free:
@@ -987,6 +1017,7 @@ static void virtnet_remove(struct virtio
 	BUG_ON(vi->num != 0);
 
 	unregister_netdev(vi->dev);
+	cancel_delayed_work_sync(&vi->refill);
 
 	vdev->config->del_vqs(vi->vdev);
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH] 8 bytes kernel memory disclosure in AppleTalk getsockname.
From: Tetsuo Handa @ 2009-08-26 12:01 UTC (permalink / raw)
  To: clement.lecigne; +Cc: linux-kernel, netdev
In-Reply-To: <20090826111247.GA79673@clem1.netasq.com>

Clement LECIGNE wrote:
> Here is a patch that zero the whole sockaddr_at structure before
> processing it. It should fix this bug.

Same kind of bug at http://lkml.org/lkml/2006/7/25/51 .
I wonder why not zero'ing sockaddr at the caller rather than
zero'ing at individual ->getname(). Doing so will save some lines.

^ permalink raw reply

* [patch 3/5] [PATCH] qeth: Cleanup for cast-type determination.
From: frank.blaschka @ 2009-08-26 12:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, Klaus-Dieter Wacker
In-Reply-To: <20090826120105.727838000@de.ibm.com>

[-- Attachment #1: 609-qeth-cast-type.diff --]
[-- Type: text/plain, Size: 7488 bytes --]

From: Klaus-Dieter Wacker <kdwacker@de.ibm.com>

Clear separation of cast-type determination (send path) for layer-2
resp. layer-3. Allowing to have inline functions for qeth layer-
discipline.

Signed-off-by: Klaus-Dieter Wacker <kdwacker@de.ibm.com> 
---

 drivers/s390/net/qeth_core.h      |    1 
 drivers/s390/net/qeth_core_main.c |   49 --------------------------------------
 drivers/s390/net/qeth_l2_main.c   |   42 ++++++++------------------------
 drivers/s390/net/qeth_l3_main.c   |   47 +++++++++++++++++++++++++++++++++++-
 4 files changed, 57 insertions(+), 82 deletions(-)

Index: git_net-next/drivers/s390/net/qeth_core.h
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core.h
+++ git_net-next/drivers/s390/net/qeth_core.h
@@ -843,7 +843,6 @@ int qeth_default_setadapterparms_cb(stru
 int qeth_send_control_data(struct qeth_card *, int, struct qeth_cmd_buffer *,
 	int (*reply_cb)(struct qeth_card *, struct qeth_reply*, unsigned long),
 	void *reply_param);
-int qeth_get_cast_type(struct qeth_card *, struct sk_buff *);
 int qeth_get_priority_queue(struct qeth_card *, struct sk_buff *, int, int);
 int qeth_get_elements_no(struct qeth_card *, void *, struct sk_buff *, int);
 int qeth_do_send_packet_fast(struct qeth_card *, struct qeth_qdio_out_q *,
Index: git_net-next/drivers/s390/net/qeth_core_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core_main.c
+++ git_net-next/drivers/s390/net/qeth_core_main.c
@@ -2952,55 +2952,6 @@ void qeth_qdio_output_handler(struct ccw
 }
 EXPORT_SYMBOL_GPL(qeth_qdio_output_handler);
 
-int qeth_get_cast_type(struct qeth_card *card, struct sk_buff *skb)
-{
-	int cast_type = RTN_UNSPEC;
-
-	if (card->info.type == QETH_CARD_TYPE_OSN)
-		return cast_type;
-
-	if (skb_dst(skb) && skb_dst(skb)->neighbour) {
-		cast_type = skb_dst(skb)->neighbour->type;
-		if ((cast_type == RTN_BROADCAST) ||
-		    (cast_type == RTN_MULTICAST) ||
-		    (cast_type == RTN_ANYCAST))
-			return cast_type;
-		else
-			return RTN_UNSPEC;
-	}
-	/* try something else */
-	if (skb->protocol == ETH_P_IPV6)
-		return (skb_network_header(skb)[24] == 0xff) ?
-				RTN_MULTICAST : 0;
-	else if (skb->protocol == ETH_P_IP)
-		return ((skb_network_header(skb)[16] & 0xf0) == 0xe0) ?
-				RTN_MULTICAST : 0;
-	/* ... */
-	if (!memcmp(skb->data, skb->dev->broadcast, 6))
-		return RTN_BROADCAST;
-	else {
-		u16 hdr_mac;
-
-		hdr_mac = *((u16 *)skb->data);
-		/* tr multicast? */
-		switch (card->info.link_type) {
-		case QETH_LINK_TYPE_HSTR:
-		case QETH_LINK_TYPE_LANE_TR:
-			if ((hdr_mac == QETH_TR_MAC_NC) ||
-			    (hdr_mac == QETH_TR_MAC_C))
-				return RTN_MULTICAST;
-			break;
-		/* eth or so multicast? */
-		default:
-		if ((hdr_mac == QETH_ETH_MAC_V4) ||
-			    (hdr_mac == QETH_ETH_MAC_V6))
-				return RTN_MULTICAST;
-		}
-	}
-	return cast_type;
-}
-EXPORT_SYMBOL_GPL(qeth_get_cast_type);
-
 int qeth_get_priority_queue(struct qeth_card *card, struct sk_buff *skb,
 			int ipv, int cast_type)
 {
Index: git_net-next/drivers/s390/net/qeth_l2_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_l2_main.c
+++ git_net-next/drivers/s390/net/qeth_l2_main.c
@@ -216,36 +216,16 @@ static void qeth_l2_del_all_mc(struct qe
 	spin_unlock_bh(&card->mclock);
 }
 
-static void qeth_l2_get_packet_type(struct qeth_card *card,
-			struct qeth_hdr *hdr, struct sk_buff *skb)
+static inline int qeth_l2_get_cast_type(struct qeth_card *card,
+			struct sk_buff *skb)
 {
-	__u16 hdr_mac;
-
-	if (!memcmp(skb->data + QETH_HEADER_SIZE,
-		    skb->dev->broadcast, 6)) {
-		/* broadcast? */
-		hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_BROADCAST;
-		return;
-	}
-	hdr_mac = *((__u16 *)skb->data);
-	/* tr multicast? */
-	switch (card->info.link_type) {
-	case QETH_LINK_TYPE_HSTR:
-	case QETH_LINK_TYPE_LANE_TR:
-		if ((hdr_mac == QETH_TR_MAC_NC) ||
-		    (hdr_mac == QETH_TR_MAC_C))
-			hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_MULTICAST;
-		else
-			hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_UNICAST;
-		break;
-		/* eth or so multicast? */
-	default:
-		if ((hdr_mac == QETH_ETH_MAC_V4) ||
-		     (hdr_mac == QETH_ETH_MAC_V6))
-			hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_MULTICAST;
-		else
-			hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_UNICAST;
-	}
+	if (card->info.type == QETH_CARD_TYPE_OSN)
+		return RTN_UNSPEC;
+	if (is_broadcast_ether_addr(skb->data))
+		return RTN_BROADCAST;
+	if (is_multicast_ether_addr(skb->data))
+		return RTN_MULTICAST;
+	return RTN_UNSPEC;
 }
 
 static void qeth_l2_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
@@ -262,7 +242,7 @@ static void qeth_l2_fill_header(struct q
 	else if (cast_type == RTN_BROADCAST)
 		hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_BROADCAST;
 	else
-		qeth_l2_get_packet_type(card, hdr, skb);
+		hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_UNICAST;
 
 	hdr->hdr.l2.pkt_length = skb->len-QETH_HEADER_SIZE;
 	/* VSWITCH relies on the VLAN
@@ -672,7 +652,7 @@ static int qeth_l2_hard_start_xmit(struc
 	struct qeth_card *card = dev->ml_priv;
 	struct sk_buff *new_skb = skb;
 	int ipv = qeth_get_ip_version(skb);
-	int cast_type = qeth_get_cast_type(card, skb);
+	int cast_type = qeth_l2_get_cast_type(card, skb);
 	struct qeth_qdio_out_q *queue = card->qdio.out_qs
 		[qeth_get_priority_queue(card, skb, ipv, cast_type)];
 	int tx_bytes = skb->len;
Index: git_net-next/drivers/s390/net/qeth_l3_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_l3_main.c
+++ git_net-next/drivers/s390/net/qeth_l3_main.c
@@ -2525,6 +2525,51 @@ static int qeth_l3_do_ioctl(struct net_d
 	return rc;
 }
 
+int inline qeth_l3_get_cast_type(struct qeth_card *card, struct sk_buff *skb)
+{
+	int cast_type = RTN_UNSPEC;
+
+	if (skb_dst(skb) && skb_dst(skb)->neighbour) {
+		cast_type = skb_dst(skb)->neighbour->type;
+		if ((cast_type == RTN_BROADCAST) ||
+		    (cast_type == RTN_MULTICAST) ||
+		    (cast_type == RTN_ANYCAST))
+			return cast_type;
+		else
+			return RTN_UNSPEC;
+	}
+	/* try something else */
+	if (skb->protocol == ETH_P_IPV6)
+		return (skb_network_header(skb)[24] == 0xff) ?
+				RTN_MULTICAST : 0;
+	else if (skb->protocol == ETH_P_IP)
+		return ((skb_network_header(skb)[16] & 0xf0) == 0xe0) ?
+				RTN_MULTICAST : 0;
+	/* ... */
+	if (!memcmp(skb->data, skb->dev->broadcast, 6))
+		return RTN_BROADCAST;
+	else {
+		u16 hdr_mac;
+
+		hdr_mac = *((u16 *)skb->data);
+		/* tr multicast? */
+		switch (card->info.link_type) {
+		case QETH_LINK_TYPE_HSTR:
+		case QETH_LINK_TYPE_LANE_TR:
+			if ((hdr_mac == QETH_TR_MAC_NC) ||
+			    (hdr_mac == QETH_TR_MAC_C))
+				return RTN_MULTICAST;
+			break;
+		/* eth or so multicast? */
+		default:
+		if ((hdr_mac == QETH_ETH_MAC_V4) ||
+			    (hdr_mac == QETH_ETH_MAC_V6))
+				return RTN_MULTICAST;
+		}
+	}
+	return cast_type;
+}
+
 static void qeth_l3_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
 		struct sk_buff *skb, int ipv, int cast_type)
 {
@@ -2650,7 +2695,7 @@ static int qeth_l3_hard_start_xmit(struc
 	struct qeth_card *card = dev->ml_priv;
 	struct sk_buff *new_skb = NULL;
 	int ipv = qeth_get_ip_version(skb);
-	int cast_type = qeth_get_cast_type(card, skb);
+	int cast_type = qeth_l3_get_cast_type(card, skb);
 	struct qeth_qdio_out_q *queue = card->qdio.out_qs
 		[qeth_get_priority_queue(card, skb, ipv, cast_type)];
 	int tx_bytes = skb->len;


^ permalink raw reply

* [patch 4/5] [PATCH] qeth: display service_level info only when valid.
From: frank.blaschka @ 2009-08-26 12:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, Klaus-Dieter Wacker
In-Reply-To: <20090826120105.727838000@de.ibm.com>

[-- Attachment #1: 610-qeth-service-level.diff --]
[-- Type: text/plain, Size: 1076 bytes --]

From: Klaus-Dieter Wacker <kdwacker@de.ibm.com>

qeth displayed an entry in /proc/service_level even when no valid
MCL-string was available (the MCL info is blank). The change is to
create an entry in /proc/service_level only when MCL-string is
non-zero.

Signed-off-by: Klaus-Dieter Wacker <kdwacker@de.ibm.com> 
---

 drivers/s390/net/qeth_core_main.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: git_net-next/drivers/s390/net/qeth_core_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core_main.c
+++ git_net-next/drivers/s390/net/qeth_core_main.c
@@ -1160,8 +1160,9 @@ static void qeth_core_sl_print(struct se
 {
 	struct qeth_card *card = container_of(slr, struct qeth_card,
 					qeth_service_level);
-	seq_printf(m, "qeth: %s firmware level %s\n", CARD_BUS_ID(card),
-			card->info.mcl_level);
+	if (card->info.mcl_level[0])
+		seq_printf(m, "qeth: %s firmware level %s\n",
+			CARD_BUS_ID(card), card->info.mcl_level);
 }
 
 static struct qeth_card *qeth_alloc_card(void)


^ permalink raw reply

* [patch 1/5] [PATCH] qeth: HiperSockets SIGA retry support on CC=2.
From: frank.blaschka @ 2009-08-26 12:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, Klaus-Dieter Wacker
In-Reply-To: <20090826120105.727838000@de.ibm.com>

[-- Attachment #1: 605-hipersockets-SIGA-retry.diff --]
[-- Type: text/plain, Size: 5778 bytes --]

From: Klaus-Dieter Wacker <kdwacker@de.ibm.com>

Qeth HiperSockets support now retries sending of packets when the
IBM System z signals a temporary resource shortage (e.g. target
buffer full). The packet is enqueued into the device queue.
After 3 times of unsuccessful send the packet is dropped.

Signed-off-by: Klaus-Dieter Wacker <kdwacker@de.ibm.com>
---

 drivers/s390/net/qeth_core.h      |    9 +++++
 drivers/s390/net/qeth_core_main.c |   60 ++++++++++++++++++++++++++++++++------
 2 files changed, 61 insertions(+), 8 deletions(-)

Index: git_net-next/drivers/s390/net/qeth_core.h
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core.h
+++ git_net-next/drivers/s390/net/qeth_core.h
@@ -435,6 +435,7 @@ struct qeth_qdio_out_q {
 	 * index of buffer to be filled by driver; state EMPTY or PACKING
 	 */
 	int next_buf_to_fill;
+	int sync_iqdio_error;
 	/*
 	 * number of buffers that are currently filled (PRIMED)
 	 * -> these buffers are hardware-owned
@@ -685,6 +686,14 @@ struct qeth_mc_mac {
 	int is_vmac;
 };
 
+struct qeth_skb_data {
+	__u32 magic;
+	int count;
+};
+
+#define QETH_SKB_MAGIC 0x71657468
+#define QETH_SIGA_CC2_RETRIES 3
+
 struct qeth_card {
 	struct list_head list;
 	enum qeth_card_states state;
Index: git_net-next/drivers/s390/net/qeth_core_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core_main.c
+++ git_net-next/drivers/s390/net/qeth_core_main.c
@@ -927,8 +927,8 @@ out:
 	return;
 }
 
-static void qeth_clear_output_buffer(struct qeth_qdio_out_q *queue,
-		 struct qeth_qdio_out_buffer *buf)
+static void __qeth_clear_output_buffer(struct qeth_qdio_out_q *queue,
+		 struct qeth_qdio_out_buffer *buf, unsigned int qeth_skip_skb)
 {
 	int i;
 	struct sk_buff *skb;
@@ -937,11 +937,13 @@ static void qeth_clear_output_buffer(str
 	if (buf->buffer->element[0].flags & 0x40)
 		atomic_dec(&queue->set_pci_flags_count);
 
-	skb = skb_dequeue(&buf->skb_list);
-	while (skb) {
-		atomic_dec(&skb->users);
-		dev_kfree_skb_any(skb);
+	if (!qeth_skip_skb) {
 		skb = skb_dequeue(&buf->skb_list);
+		while (skb) {
+			atomic_dec(&skb->users);
+			dev_kfree_skb_any(skb);
+			skb = skb_dequeue(&buf->skb_list);
+		}
 	}
 	for (i = 0; i < QETH_MAX_BUFFER_ELEMENTS(queue->card); ++i) {
 		if (buf->buffer->element[i].addr && buf->is_header[i])
@@ -957,6 +959,12 @@ static void qeth_clear_output_buffer(str
 	atomic_set(&buf->state, QETH_QDIO_BUF_EMPTY);
 }
 
+static void qeth_clear_output_buffer(struct qeth_qdio_out_q *queue,
+		struct qeth_qdio_out_buffer *buf)
+{
+	__qeth_clear_output_buffer(queue, buf, 0);
+}
+
 void qeth_clear_qdio_buffers(struct qeth_card *card)
 {
 	int i, j;
@@ -2685,6 +2693,13 @@ static int qeth_handle_send_error(struct
 	int sbalf15 = buffer->buffer->element[15].flags & 0xff;
 
 	QETH_DBF_TEXT(TRACE, 6, "hdsnderr");
+	if (card->info.type == QETH_CARD_TYPE_IQD) {
+		if (sbalf15 == 0) {
+			qdio_err = 0;
+		} else {
+			qdio_err = 1;
+		}
+	}
 	qeth_check_qdio_errors(buffer->buffer, qdio_err, "qouterr");
 
 	if (!qdio_err)
@@ -2817,6 +2832,7 @@ static void qeth_flush_buffers(struct qe
 		}
 	}
 
+	queue->sync_iqdio_error = 0;
 	queue->card->dev->trans_start = jiffies;
 	if (queue->card->options.performance_stats) {
 		queue->card->perf_stats.outbound_do_qdio_cnt++;
@@ -2832,6 +2848,10 @@ static void qeth_flush_buffers(struct qe
 		queue->card->perf_stats.outbound_do_qdio_time +=
 			qeth_get_micros() -
 			queue->card->perf_stats.outbound_do_qdio_start_time;
+	if (rc > 0) {
+		if (!(rc & QDIO_ERROR_SIGA_BUSY))
+			queue->sync_iqdio_error = rc & 3;
+	}
 	if (rc) {
 		queue->card->stats.tx_errors += count;
 		/* ignore temporary SIGA errors without busy condition */
@@ -2899,6 +2919,7 @@ void qeth_qdio_output_handler(struct ccw
 	struct qeth_qdio_out_q *queue = card->qdio.out_qs[__queue];
 	struct qeth_qdio_out_buffer *buffer;
 	int i;
+	unsigned qeth_send_err;
 
 	QETH_DBF_TEXT(TRACE, 6, "qdouhdl");
 	if (qdio_error & QDIO_ERROR_ACTIVATE_CHECK_CONDITION) {
@@ -2915,8 +2936,9 @@ void qeth_qdio_output_handler(struct ccw
 	}
 	for (i = first_element; i < (first_element + count); ++i) {
 		buffer = &queue->bufs[i % QDIO_MAX_BUFFERS_PER_Q];
-		qeth_handle_send_error(card, buffer, qdio_error);
-		qeth_clear_output_buffer(queue, buffer);
+		qeth_send_err = qeth_handle_send_error(card, buffer, qdio_error);
+		__qeth_clear_output_buffer(queue, buffer,
+			(qeth_send_err == QETH_SEND_ERROR_RETRY) ? 1 : 0);
 	}
 	atomic_sub(count, &queue->used_buffers);
 	/* check if we need to do something on this outbound queue */
@@ -3159,7 +3181,10 @@ int qeth_do_send_packet_fast(struct qeth
 		int offset, int hd_len)
 {
 	struct qeth_qdio_out_buffer *buffer;
+	struct sk_buff *skb1;
+	struct qeth_skb_data *retry_ctrl;
 	int index;
+	int rc;
 
 	/* spin until we get the queue ... */
 	while (atomic_cmpxchg(&queue->state, QETH_OUT_Q_UNLOCKED,
@@ -3178,6 +3203,25 @@ int qeth_do_send_packet_fast(struct qeth
 	atomic_set(&queue->state, QETH_OUT_Q_UNLOCKED);
 	qeth_fill_buffer(queue, buffer, skb, hdr, offset, hd_len);
 	qeth_flush_buffers(queue, index, 1);
+	if (queue->sync_iqdio_error == 2) {
+		skb1 = skb_dequeue(&buffer->skb_list);
+		while (skb1) {
+			atomic_dec(&skb1->users);
+			skb1 = skb_dequeue(&buffer->skb_list);
+		}
+		retry_ctrl = (struct qeth_skb_data *) &skb->cb[16];
+		if (retry_ctrl->magic != QETH_SKB_MAGIC) {
+			retry_ctrl->magic = QETH_SKB_MAGIC;
+			retry_ctrl->count = 0;
+		}
+		if (retry_ctrl->count < QETH_SIGA_CC2_RETRIES) {
+			retry_ctrl->count++;
+			rc = dev_queue_xmit(skb);
+		} else {
+			dev_kfree_skb_any(skb);
+			QETH_DBF_TEXT(QERR, 2, "qrdrop");
+		}
+	}
 	return 0;
 out:
 	atomic_set(&queue->state, QETH_OUT_Q_UNLOCKED);


^ permalink raw reply

* [patch 0/5] s390: qeth patches for 2.6.32
From: frank.blaschka @ 2009-08-26 12:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390

Hi Dave,

here are a couple of qeth patches for 2.6.32 (net-next).

shortlog:
Klaus-Dieter Wacker (3)
qeth: HiperSockets SIGA retry support on CC=2.
qeth: Cleanup for cast-type determination.
qeth: display service_level info only when valid.

Ursula Braun (1)
qeth: display "undefined" value of sysfs-attribute "layer2"

Frank Blaschka (1)
qeth: scheduling while atomic during ifconfig online sequence

Thanks,
        Frank

^ permalink raw reply

* [patch 2/5] [PATCH] qeth: scheduling while atomic during ifconfig online sequence
From: frank.blaschka @ 2009-08-26 12:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390
In-Reply-To: <20090826120105.727838000@de.ibm.com>

[-- Attachment #1: 606-qeth-online-sequence.diff --]
[-- Type: text/plain, Size: 1036 bytes --]

From: Frank Blaschka <frank.blaschka@de.ibm.com>

In case the IP address list contains entries (not removed when the device was set
offline) this entries should be registered next time the device is brought online.
In the past this was done implicitly with the device open call but since we wait
in the set IPv4 IPA and the device open common code holds various locks this
does not work any longer.

Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---

 drivers/s390/net/qeth_l3_main.c |    1 +
 1 file changed, 1 insertion(+)

Index: git_net-next/drivers/s390/net/qeth_l3_main.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_l3_main.c
+++ git_net-next/drivers/s390/net/qeth_l3_main.c
@@ -3180,6 +3180,7 @@ static int __qeth_l3_set_online(struct c
 	netif_carrier_on(card->dev);
 
 	qeth_set_allowed_threads(card, 0xffffffff, 0);
+	qeth_l3_set_ip_addr_list(card);
 	if (recover_flag == CARD_STATE_RECOVER) {
 		if (recovery_mode)
 			qeth_l3_open(card->dev);


^ permalink raw reply

* [patch 5/5] [PATCH] qeth: display "undefined" value of sysfs-attribute "layer2"
From: frank.blaschka @ 2009-08-26 12:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, Ursula Braun
In-Reply-To: <20090826120105.727838000@de.ibm.com>

[-- Attachment #1: 612-qeth-layer2-attribute.diff --]
[-- Type: text/plain, Size: 985 bytes --]

From: Ursula Braun <ursula.braun@de.ibm.com>

If nothing has been written into the qeth sysfs-attribute layer2,
its value is "-1" meaning "not yet defined". But the value is
displayed as "1" meaning "layer2 selected". The patch changes the
reading of this "-1"-value to "-1" to make clear the layer2-attribute
has not yet been defined.

Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 drivers/s390/net/qeth_core_sys.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: git_net-next/drivers/s390/net/qeth_core_sys.c
===================================================================
--- git_net-next.orig/drivers/s390/net/qeth_core_sys.c
+++ git_net-next/drivers/s390/net/qeth_core_sys.c
@@ -364,7 +364,7 @@ static ssize_t qeth_dev_layer2_show(stru
 	if (!card)
 		return -EINVAL;
 
-	return sprintf(buf, "%i\n", card->options.layer2 ? 1:0);
+	return sprintf(buf, "%i\n", card->options.layer2);
 }
 
 static ssize_t qeth_dev_layer2_store(struct device *dev,


^ permalink raw reply

* [PATCH] 8 bytes kernel memory disclosure in AppleTalk getsockname.
From: Clement LECIGNE @ 2009-08-26 11:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev

Hi,

In function atalk_getname(), sockaddr_at is returned in userland without
zero'ing the "char sat_zero[8]" field. This bug allows user to display 8
bytes leaked from the kernel stack.

Here is a patch that zero the whole sockaddr_at structure before
processing it. It should fix this bug.

Signed-off-by: Clément Lecigne <clement.lecigne@netasq.com>
--- linux/net/appletalk/ddp.c	2009-08-26 11:35:59.000000000 +0200
+++ linux/net/appletalk/ddp.c	2009-08-26 11:36:30.000000000 +0200
@@ -1241,6 +1241,8 @@ static int atalk_getname(struct socket *
 		if (atalk_autobind(sk) < 0)
 			return -ENOBUFS;
 
+	memset(&sat, 0, sizeof(struct sockaddr_at));
+
 	*uaddr_len = sizeof(struct sockaddr_at);
 
 	if (peer) {

-- 
Clément LECIGNE,
-Only one remote hole in the default install, in more than 10 years!<br>
+Only two remote holes in the default install, in more than 10 years!<br>

^ permalink raw reply

* Re: [PATCH 1/2] Add an alternative cs89x0 driver
From: Kurt Van Dijck @ 2009-08-26 10:46 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: netdev
In-Reply-To: <1240387172-21818-2-git-send-email-s.hauer@pengutronix.de>

Hi Sacha,

I'm using a 2.6.25.
Converting to your platform_device based driver,
I needed to configure the irq (see patch, irq flags).
Looking in the old cs89x0.c, it's done in the driver. Should I have
configured the irq level elsewhere? Or is this patch valid to do?

Kurt

Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
---
Index: drivers/net/cirrus-cs89x0.c
===================================================================
--- drivers/net/cirrus-cs89x0.c	(revision 7107)
+++ drivers/net/cirrus-cs89x0.c	(working copy)
@@ -487,7 +487,8 @@
        }
 
        /* install interrupt handler */
-       result = request_irq(ndev->irq, &cirrus_interrupt, 0, ndev->name, ndev);
+       result = request_irq(ndev->irq, &cirrus_interrupt,
+			IRQF_TRIGGER_HIGH, ndev->name, ndev);
        if (result < 0) {
                printk(KERN_ERR "%s: could not register interrupt %d\n",
                       ndev->name, ndev->irq);

^ permalink raw reply

* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-26 11:00 UTC (permalink / raw)
  To: Bill Fink; +Cc: Linux Network Developers, brice, gallatin
In-Reply-To: <20090826031057.375303c9.billfink@mindspring.com>

On Wed, Aug 26, 2009 at 03:10:57AM -0400, Bill Fink wrote:
> On Fri, 21 Aug 2009, Neil Horman wrote:
> 
> > On Fri, Aug 21, 2009 at 12:14:21AM -0400, Bill Fink wrote:
> > > On Thu, 20 Aug 2009, Neil Horman wrote:
> > > 
> > > > On Thu, Aug 20, 2009 at 03:50:44AM -0400, Bill Fink wrote:
> > > > 
> > > > > When I tried an actual nuttcp performance test, even when rate limiting
> > > > > to just 1 Mbps, I immediately got a kernel oops.  I tried to get a
> > > > > crashdump via kexec/kdump, but the kexec kernel, instead of just
> > > > > generating a crashdump, fully booted the new kernel, which was
> > > > > extremely sluggish until I rebooted it through a BIOS re-init,
> > > > > and never produced a crashdump.  I tried this several times and
> > > > > an immediate kernel oops was always the result (with either a TCP
> > > > > or UDP test).  A ping test of 1000 9000-byte packets with an interval
> > > > > of 0.001 seconds (which is 72 Mbps for 1 second) on the other hand
> > > > > worked just fine.
> > > > 
> > > > The sluggishness is expected, since the kdump kernel operates out of such
> > > > limited memory.  don't know why you booted to a full system rather than did a
> > > > crash recovery.  Don't suppose you got a backtrace did you?
> > > 
> > > There was a backtrace on the screen but I didn't have a chance to
> > > record it.  BTW did anyone ever think to print the backtrace in
> > > reverse (first to some reserved memory and then output to the display)
> > > so the more interesting parts wouldn't have scrolled off the top of
> > > the screen?
> > > 
> > The real solution is to use a console to which the output doesn't scroll off the
> > screen.  Normally people use a serial console they can log, or a RAC card that
> > they can record. Even on a regular vga monitor in text mode, you can set up the
> > vt iirc to allow for scrolling.
> 
> None of our Asus P6T6 systems have serial consoles.  I don't know of
> any RAC cards for them either, nor are there spare PCI slots available
> in many cases.  I wouldn't think the Shift-PageUp trick would work
> with a crashed kernel, but I admit I didn't try it.  I haven't checked
> out netconsole yet either, but I'm not sure it would help either in a
> case like this that was a network related kernel crash.
> 
Any USB ports that you can attach a serial dongle to?  That would work as well,
or, as previously mentioned, netconsole also does the trick.

> In any case, a simple kernel command line that would provide a reversed
> backtrace would be a simple thing to facilitate Linux users providing
> useful info to Linux kernel developers in helping to debug kernel
> problems.  The most useful info would still be on the screen, so it
> could be transcribed or a photo image of the screen could be taken.
> 
I understand what your saying, I'm just saying there are currently several
options for you that have already solved this problem in differnt ways.

> Fortunately, in this specific case, the SuperMicro X8DAH+-F system
> does have a serial console, and after a fair amount of effort I was
> able to get it to work as desired, and was able to finally capture
> a backtrace of the kernel oops.  BTW I believe the reason the
> kexec/kdump didn't work was probably because it couldn't find
> a /proc/vmcore file, although I don't know why that would be,
> and the Fedora 10 /etc/init.d/kdump script will then just boot
> up normally if it fails to find the /proc/vmcore file (or it's
> zero size).
> 
I take care of kdump for fedora and RHEL.  If you file a bug on this, I'd be
happy to look into it further.

> The following shows a simple ping test usage of the skb_sources
> tracing feature:
> 
> [root@xeontest1 tracing]# numactl --membind=1 taskset -c 4 ping -c 5 -s 1472 192.168.1.10
> PING 192.168.1.10 (192.168.1.10) 1472(1500) bytes of data.
> 1480 bytes from 192.168.1.10: icmp_seq=1 ttl=64 time=0.139 ms
> 1480 bytes from 192.168.1.10: icmp_seq=2 ttl=64 time=0.182 ms
> 1480 bytes from 192.168.1.10: icmp_seq=3 ttl=64 time=0.178 ms
> 1480 bytes from 192.168.1.10: icmp_seq=4 ttl=64 time=0.188 ms
> 1480 bytes from 192.168.1.10: icmp_seq=5 ttl=64 time=0.178 ms
> 
> --- 192.168.1.10 ping statistics ---
> 5 packets transmitted, 5 received, 0% packet loss, time 3999ms
> rtt min/avg/max/mdev = 0.139/0.173/0.188/0.017 ms
> 
> [root@xeontest1 tracing]# cat trace
> # tracer: skb_sources
> #
> #       PID     ANID    CNID    IFC     RXQ     CCPU    LEN
> #        |       |       |       |       |       |       |
>         4217    1       1       eth2    0       4       1500
>         4217    1       1       eth2    0       4       1500
>         4217    1       1       eth2    0       4       1500
>         4217    1       1       eth2    0       4       1500
>         4217    1       1       eth2    0       4       1500
> 
> All is as was expected.
> 
> But if I try an actual nuttcp performance test (even rate limited
> to 1 Mbps), I get the following kernel oops:
> 
thank you, I think I see the problem, I'll have a patch for you in just a bit

Thanks
Neil

> [root@xeontest1 tracing]# numactl --membind=1 nuttcp -In2 -Ri1m -xc4/0 192.168.1.10
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000038
> IP: [<ffffffff810b01ab>] probe_skb_dequeue+0xf7/0x152
> PGD 337d12067 PUD 337d11067 PMD 0
> Oops: 0000 [#1] SMP
> last sysfs file: /sys/devices/pci0000:80/0000:80:07.0/0000:8b:00.0/0000:8c:04.0e
> CPU 4
> Modules linked in: w83627ehf hwmon_vid coretemp hwmon ipv6 dm_multipath uinput ]
> Pid: 4222, comm: nuttcp Not tainted 2.6.31-rc6-bf #3 X8DAH
> RIP: 0010:[<ffffffff810b01ab>]  [<ffffffff810b01ab>] probe_skb_dequeue+0xf7/0x12
> RSP: 0018:ffff8801a5811a88  EFLAGS: 00010213
> RAX: 0000000000000000 RBX: ffff88033906d154 RCX: 000000000000000d
> RDX: 000000000000f88c RSI: 000000000000000b RDI: ffff8803383d3044
> RBP: ffff8801a5811ab8 R08: 0000000000000001 R09: ffff8801ab311a00
> R10: 0000000000000005 R11: ffffc9000080e2b0 R12: ffff880337c45400
> R13: ffff88033906d150 R14: 0000000000000014 R15: ffffffff818bb890
> FS:  00007fa976d326f0(0000) GS:ffffc90000800000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000038 CR3: 000000033801e000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process nuttcp (pid: 4222, threadinfo ffff8801a5810000, task ffff8801ab2e5d00)
> Stack:
>  ffff8801a5811ab8 ffff8801b35d4ab0 0000000000000014 0000000000000000
> <0> 0000000000000014 0000000000000014 ffff8801a5811b18 ffffffff81366ae8
> <0> ffff8801a5811ed8 0000001439084000 ffff880337c45400 00000001001416ef
> Call Trace:
>  [<ffffffff81366ae8>] skb_copy_datagram_iovec+0x50/0x1f5
>  [<ffffffff813ac875>] tcp_rcv_established+0x278/0x6db
>  [<ffffffff813b3ef5>] tcp_v4_do_rcv+0x1b8/0x366
>  [<ffffffff8135f99e>] ? release_sock+0xab/0xb4
>  [<ffffffff8136004d>] ? sk_wait_data+0xc8/0xd6
>  [<ffffffff813a32d6>] tcp_prequeue_process+0x79/0x8f
>  [<ffffffff813a455d>] tcp_recvmsg+0x4e8/0xaa0
>  [<ffffffff8135ec90>] sock_common_recvmsg+0x37/0x4c
>  [<ffffffff8135cb06>] __sock_recvmsg+0x72/0x7f
>  [<ffffffff8135cbdd>] sock_aio_read+0xca/0xda
>  [<ffffffff810d9536>] ? vma_merge+0x2a0/0x318
>  [<ffffffff810f6d4f>] do_sync_read+0xec/0x132
>  [<ffffffff81067ddc>] ? autoremove_wake_function+0x0/0x3d
>  [<ffffffff811b646c>] ? security_file_permission+0x16/0x18
>  [<ffffffff810f785c>] vfs_read+0xc0/0x107
>  [<ffffffff810f7971>] sys_read+0x4c/0x75
>  [<ffffffff81011c82>] system_call_fastpath+0x16/0x1b
> Code: 44 89 73 30 89 43 14 41 0f b7 84 24 ac 00 00 00 89 43 28 65 8b 04 25 98 e
> RIP  [<ffffffff810b01ab>] probe_skb_dequeue+0xf7/0x152
>  RSP <ffff8801a5811a88>
> CR2: 0000000000000038
> 
> 						-Thanks
> 
> 						-Bill
> 

^ 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