Netdev List
 help / color / mirror / Atom feed
* Fw: [Bugme-new] [Bug 4628] New: Test server hang while running rhr (network) test on RHEL4 with kernel 2.6.12-rc1-mm4
From: Andrew Morton @ 2005-05-16  9:59 UTC (permalink / raw)
  To: netdev; +Cc: hejianj, linuxppc64-dev, Anton Blanchard


Might be a bug in the e100 driver, might not be.

I assume this is the

	BUG_ON(skb->list != NULL);

in __kfree_skb(), although the line number is off-by-one, and the
.__kfree_skb+0x188/0x240 would tend to contradict that.  Anton, can you
help work out where we went splat please?

tx timeouts are fairly rare events, so this might not be a recently-added
bug.

Do we know if it is repeatable?



Begin forwarded message:

Date: Mon, 16 May 2005 02:44:04 -0700
From: bugme-daemon@osdl.org
To: bugme-new@lists.osdl.org
Subject: [Bugme-new] [Bug 4628] New: Test server hang while running rhr (network) test on RHEL4 with kernel 2.6.12-rc1-mm4


http://bugme.osdl.org/show_bug.cgi?id=4628

           Summary: Test server hang while running rhr (network) test on
                    RHEL4 with kernel 2.6.12-rc1-mm4
    Kernel Version: 2.6.12-rc1 with mm4 patch
            Status: NEW
          Severity: normal
             Owner: anton@samba.org
         Submitter: hejianj@cn.ibm.com
                CC: hanwenb@cn.ibm.com,mridge@us.ibm.com,rende@cn.ibm.com,wa
                    ngjs@cn.ibm.com


Distribution:
RHEL4 with kernel 2.6.12-rc1-mm4

Hardware Environment:
IBM OpenPower( CHRP IBM,9124-720 )

Software Environment:
RHEL4
RHR: rhr2-rhel4-1.0-14a.noarch.rpm

Problem Description:
The test server hang while running rhr (network) test on RHEL4 with kernel 
2.6.12-rc1-mm4.

Steps to reproduce:
1. Download kernel 2.6.12-rc1 and 2.6.12-rc1-mm4 patch from kernel.org, then 
build the kernel on OpenPower 720
2. Download rhr2-rhel4-1.0-14a.noarch.rpm from rhn.redhat.com and install it on 
the test machine.
3. Configure and run the rhr test via invoking redhat-ready.

Additional information:
Here is the backtrace from xmon.

3:mon> e
cpu 0x3: Vector: 700 (Program Check) at [c00000000ffe7920]
    pc: c00000000029632c: .__kfree_skb+0x188/0x240
    lr: c000000000296328: .__kfree_skb+0x184/0x240
    sp: c00000000ffe7ba0
   msr: 8000000000029032
  current = 0xc000000107f94040
  paca    = 0xc000000000431c00
    pid   = 0, comm = swapper
kernel BUG in __kfree_skb at net/core/skbuff.c:282!

3:mon> t
[c00000000ffe7c40] d0000000000ebac4 .e100_rx_clean_list+0xa0/0x144 [e100]
[c00000000ffe7ce0] d0000000000ed6dc .e100_tx_timeout+0x7c/0xb0 [e100]
[c00000000ffe7d70] c0000000002b87bc .dev_watchdog+0xc8/0x154
[c00000000ffe7e00] c00000000006d6b4 .run_timer_softirq+0x180/0x298
[c00000000ffe7ed0] c0000000000667d8 .__do_softirq+0xdc/0x1b8
[c00000000ffe7f90] c000000000014bf0 .call_do_softirq+0x14/0x24
[c000000086b43860] c0000000000102c4 .do_softirq+0x98/0xac
[c000000086b438f0] c0000000000669cc .irq_exit+0x70/0x8c
[c000000086b43970] c000000000011fb8 .timer_interrupt+0x398/0x47c
[c000000086b43a90] c00000000000a2b4 decrementer_common+0xb4/0x100
--- Exception: 901 (Decrementer) at c000000000010554 .dedicated_idle+0x114/0x280
[c000000086b43e80] c0000000000108c8 .cpu_idle+0x3c/0x54
[c000000086b43f00] c00000000003cc8c .start_secondary+0x108/0x148
[c000000086b43f90] c00000000000bd84 .enable_64b_mode+0x0/0x28

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

^ permalink raw reply

* Re: [patch 02/10] A new 10GB Ethernet Driver by Chelsio Communications
From: Christoph Hellwig @ 2005-05-16  7:12 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: akpm, davem, christoph, tinay, sbardone, christoph, Netdev
In-Reply-To: <4287D826.3020704@pobox.com>

On Sun, May 15, 2005 at 07:15:50PM -0400, Jeff Garzik wrote:
> applied

to what tree?  the driver is still a rather huge mess..

^ permalink raw reply

* Re: patch tulip-natsemi-dp83840a-phy-fix.patch added to -mm tree
From: Grant Grundler @ 2005-05-16  5:08 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: akpm, T-Bone, grundler, varenet, Linux Kernel, Netdev
In-Reply-To: <42881C58.40001@pobox.com>

On Mon, May 16, 2005 at 12:06:48AM -0400, Jeff Garzik wrote:
> Note that while the patch creates the correct behavior, the delays above 
> occur inside spin_lock_irqsave() and/or timer context.

Yes. Agreed.  So what?

If tulip phy init code is hit so often *and* seeing the worst case
2ms delay that it's a problem, the delay doesn't matter.
Something else is fundamentally wrong.

Fixing code that doesn't comply with published specs and has
been proven to not work on at least 3 archs seems a bit more
important than an occasional < 1ms (normal case) delay.

> I have been to get HP to fix this patch's delay problem for -years-.

I was "encouraged" to rewrite the tulip phy init sequence in 2002
to use schedule_timeout() and heard a claim it would be trivial.

I looked into it and concluded it was NOT trivial.
I approached Jeff at OLS2002 (or 2003) and explained why I thought it
was NOT trivial.  Didn't get a response that resolved any of the
concerns I raised. I'd be willing to take another look at fresh
ideas once all of the tulip patches I have ouststanding go in.

If the original suggestion really were trivial, we wouldn't be having
this conversation now.  Some high school student would have taken care
of it by now, right?

thanks,
grant

^ permalink raw reply

* Re: patch tulip-natsemi-dp83840a-phy-fix.patch added to -mm tree
From: Jeff Garzik @ 2005-05-16  4:06 UTC (permalink / raw)
  To: akpm; +Cc: T-Bone, grundler, varenet, Linux Kernel, Netdev
In-Reply-To: <200505101955.j4AJtX9x032464@shell0.pdx.osdl.net>

akpm@osdl.org wrote:
> +
> +					/* flush posted writes */
> +					ioread32(ioaddr + CSR15);
> +
> +					/* Sect 3.10.3 in DP83840A.pdf (p39) */
> +					udelay(500);
> +
> +					/* Section 4.2 in DP83840A.pdf (p43) */
> +					/* and IEEE 802.3 "22.2.4.1.1 Reset" */
> +					while (timeout-- &&
> +						(tulip_mdio_read (dev, phy_num, MII_BMCR) & BMCR_RESET))
> +						udelay(100);


Note that while the patch creates the correct behavior, the delays above 
occur inside spin_lock_irqsave() and/or timer context.

I have been to get HP to fix this patch's delay problem for -years-.

	Jeff

^ permalink raw reply

* Re: [PATCH] reduce stack usage for arlan
From: Jeff Garzik @ 2005-05-16  3:22 UTC (permalink / raw)
  To: Jörn Engel; +Cc: netdev
In-Reply-To: <20050308153457.GB8703@wohnheim.fh-wedel.de>

Jörn Engel wrote:
> Jean Tourrilhes indicated that this driver is currently unmaintained.
> Jeff, do you want this patch?

>  #define SARLSTR(var,nn) {\
> -	char tmpStr[400];\
> +	char tmpStr[50];\
>  	int  tmpLn = nn;\
> -	if (nn > 399 ) tmpLn = 399; \
> +	BUG_ON(tmpLn > 49);\


I'm uncomfortable with arbitrarily lowering the string size, even if it 
does include a BUG_ON()...

	Jeff

^ permalink raw reply

* Re: [PATCH][RESENT] Generic HDLC update
From: Jeff Garzik @ 2005-05-16  2:24 UTC (permalink / raw)
  To: Krzysztof Halasa; +Cc: netdev
In-Reply-To: <m3acns8c6y.fsf@defiant.localdomain>

applied

^ permalink raw reply

* Re: [PATCH 8/12] orinoco: manual roaming for Symbol and Intersilfirmware
From: David Gibson @ 2005-05-16  1:06 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: jgarzik, proski, netdev
In-Reply-To: <20050514153041.GI3643@lst.de>

On Sat, May 14, 2005 at 05:30:41PM +0200, Christoph Hellwig wrote:
> Patch from Pavel Roskin

Acked-by: David Gibson <hermes@gibson.dropbear.id.au>

> Index: linux-2.6/drivers/net/wireless/orinoco.h
> ===================================================================
> +++ linux-2.6/drivers/net/wireless/orinoco.h	2005-05-14 17:03:00.000000000 +0200
> @@ -22,6 +22,8 @@
>  
>  #define WIRELESS_SPY		// enable iwspy support
>  
> +#define MAX_SCAN_LEN		4096
> +
>  #define ORINOCO_MAX_KEY_SIZE	14
>  #define ORINOCO_MAX_KEYS	4
>  
> @@ -48,6 +50,7 @@
>  	/* driver state */
>  	int open;
>  	u16 last_linkstatus;
> +	struct work_struct join_work;
>  
>  	/* Net device stuff */
>  	struct net_device *ndev;
> @@ -84,6 +87,8 @@
>  	int bitratemode;
>   	char nick[IW_ESSID_MAX_SIZE+1];
>  	char desired_essid[IW_ESSID_MAX_SIZE+1];
> +	char desired_bssid[ETH_ALEN];
> +	int bssid_fixed;
>  	u16 frag_thresh, mwo_robust;
>  	u16 channel;
>  	u16 ap_density, rts_thresh;
> Index: linux-2.6/drivers/net/wireless/orinoco.c
> ===================================================================
> +++ linux-2.6/drivers/net/wireless/orinoco.c	2005-05-14 17:03:00.000000000 +0200
> @@ -1247,6 +1247,75 @@
>  	       dev->name, s, status);
>  }
>  
> +/* Search scan results for requested BSSID, join it if found */
> +static void orinoco_join_ap(struct net_device *dev)
> +{
> +	struct orinoco_private *priv = netdev_priv(dev);
> +	struct hermes *hw = &priv->hw;
> +	int err;
> +	unsigned long flags;
> +	struct join_req {
> +		u8 bssid[ETH_ALEN];
> +		u16 channel;
> +	} __attribute__ ((packed)) req;
> +	const int atom_len = offsetof(struct prism2_scan_apinfo, atim);
> +	struct prism2_scan_apinfo *atom;
> +	int offset = 4;
> +	u8 *buf;
> +	u16 len;
> +
> +	/* Allocate buffer for scan results */
> +	buf = kmalloc(MAX_SCAN_LEN, GFP_KERNEL);
> +	if (! buf)
> +		return;
> +
> +	if (orinoco_lock(priv, &flags) != 0)
> +		goto out;
> +
> +	/* Sanity checks in case user changed something in the meantime */
> +	if (! priv->bssid_fixed)
> +		goto out;
> +
> +	if (strlen(priv->desired_essid) == 0)
> +		goto out;
> +
> +	/* Read scan results from the firmware */
> +	err = hermes_read_ltv(hw, USER_BAP,
> +			      HERMES_RID_SCANRESULTSTABLE,
> +			      MAX_SCAN_LEN, &len, buf);
> +	if (err) {
> +		printk(KERN_ERR "%s: Cannot read scan results\n",
> +		       dev->name);
> +		goto out;
> +	}
> +
> +	len = HERMES_RECLEN_TO_BYTES(len);
> +
> +	/* Go through the scan results looking for the channel of the AP
> +	 * we were requested to join */
> +	for (; offset + atom_len <= len; offset += atom_len) {
> +		atom = (struct prism2_scan_apinfo *) (buf + offset);
> +		if (memcmp(&atom->bssid, priv->desired_bssid, ETH_ALEN) == 0)
> +			goto found;
> +	}
> +
> +	DEBUG(1, "%s: Requested AP not found in scan results\n",
> +	      dev->name);
> +	goto out;
> +
> + found:
> +	memcpy(req.bssid, priv->desired_bssid, ETH_ALEN);
> +	req.channel = atom->channel;	/* both are little-endian */
> +	err = HERMES_WRITE_RECORD(hw, USER_BAP, HERMES_RID_CNFJOINREQUEST,
> +				  &req);
> +	if (err)
> +		printk(KERN_ERR "%s: Error issuing join request\n", dev->name);
> +
> + out:
> +	kfree(buf);
> +	orinoco_unlock(priv, &flags);
> +}
> +
>  static void __orinoco_ev_info(struct net_device *dev, hermes_t *hw)
>  {
>  	struct orinoco_private *priv = netdev_priv(dev);
> @@ -1477,6 +1546,36 @@
>  	return err;
>  }
>  
> +/* Set fixed AP address */
> +static int __orinoco_hw_set_wap(struct orinoco_private *priv)
> +{
> +	int roaming_flag;
> +	int err = 0;
> +	hermes_t *hw = &priv->hw;
> +
> +	switch (priv->firmware_type) {
> +	case FIRMWARE_TYPE_AGERE:
> +		/* not supported */
> +		break;
> +	case FIRMWARE_TYPE_INTERSIL:
> +		if (priv->bssid_fixed)
> +			roaming_flag = 2;
> +		else
> +			roaming_flag = 1;
> +
> +		err = hermes_write_wordrec(hw, USER_BAP,
> +					   HERMES_RID_CNFROAMINGMODE,
> +					   roaming_flag);
> +		break;
> +	case FIRMWARE_TYPE_SYMBOL:
> +		err = HERMES_WRITE_RECORD(hw, USER_BAP,
> +					  HERMES_RID_CNFMANDATORYBSSID_SYMBOL,
> +					  &priv->desired_bssid);
> +		break;
> +	}
> +	return err;
> +}
> +
>  /* Change the WEP keys and/or the current keys.  Can be called
>   * either from __orinoco_hw_setup_wep() or directly from
>   * orinoco_ioctl_setiwencode().  In the later case the association
> @@ -1662,6 +1761,13 @@
>  		}
>  	}
>  
> +	/* Set the desired BSSID */
> +	err = __orinoco_hw_set_wap(priv);
> +	if (err) {
> +		printk(KERN_ERR "%s: Error %d setting AP address\n",
> +		       dev->name, err);
> +		return err;
> +	}
>  	/* Set the desired ESSID */
>  	idbuf.len = cpu_to_le16(strlen(priv->desired_essid));
>  	memcpy(&idbuf.val, priv->desired_essid, sizeof(idbuf.val));
> @@ -2432,6 +2538,7 @@
>  				   * before anything else touches the
>  				   * hardware */
>  	INIT_WORK(&priv->reset_work, (void (*)(void *))orinoco_reset, dev);
> +	INIT_WORK(&priv->join_work, (void (*)(void *))orinoco_join_ap, dev);
>  
>  	netif_carrier_off(dev);
>  	priv->last_linkstatus = 0xffff;
> @@ -2593,6 +2700,67 @@
>  	return 0;
>  }
>  
> +static int orinoco_ioctl_setwap(struct net_device *dev,
> +				struct iw_request_info *info,
> +				struct sockaddr *ap_addr,
> +				char *extra)
> +{
> +	struct orinoco_private *priv = netdev_priv(dev);
> +	int err = -EINPROGRESS;		/* Call commit handler */
> +	unsigned long flags;
> +	static const u8 off_addr[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
> +	static const u8 any_addr[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> +
> +	if (orinoco_lock(priv, &flags) != 0)
> +		return -EBUSY;
> +
> +	/* Enable automatic roaming - no sanity checks are needed */
> +	if (memcmp(&ap_addr->sa_data, off_addr, ETH_ALEN) == 0 ||
> +	    memcmp(&ap_addr->sa_data, any_addr, ETH_ALEN) == 0) {
> +		priv->bssid_fixed = 0;
> +		memset(priv->desired_bssid, 0, ETH_ALEN);
> +
> +		/* "off" means keep existing connection */
> +		if (ap_addr->sa_data[0] == 0) {
> +			__orinoco_hw_set_wap(priv);
> +			err = 0;
> +		}
> +		goto out;
> +	}
> +
> +	if (priv->firmware_type == FIRMWARE_TYPE_AGERE) {
> +		printk(KERN_WARNING "%s: Lucent/Agere firmware doesn't "
> +		       "support manual roaming\n",
> +		       dev->name);
> +		err = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	if (priv->iw_mode != IW_MODE_INFRA) {
> +		printk(KERN_WARNING "%s: Manual roaming supported only in "
> +		       "managed mode\n", dev->name);
> +		err = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	/* Intersil firmware hangs without Desired ESSID */
> +	if (priv->firmware_type == FIRMWARE_TYPE_INTERSIL &&
> +	    strlen(priv->desired_essid) == 0) {
> +		printk(KERN_WARNING "%s: Desired ESSID must be set for "
> +		       "manual roaming\n", dev->name);
> +		err = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	/* Finally, enable manual roaming */
> +	priv->bssid_fixed = 1;
> +	memcpy(priv->desired_bssid, &ap_addr->sa_data, ETH_ALEN);
> +
> + out:
> +	orinoco_unlock(priv, &flags);
> +	return err;
> +}
> +
>  static int orinoco_ioctl_getwap(struct net_device *dev,
>  				struct iw_request_info *info,
>  				struct sockaddr *ap_addr,
> @@ -3890,6 +4058,7 @@
>  	[SIOCGIWRANGE -SIOCIWFIRST] (iw_handler) orinoco_ioctl_getiwrange,
>  	[SIOCSIWSPY   -SIOCIWFIRST] (iw_handler) orinoco_ioctl_setspy,
>  	[SIOCGIWSPY   -SIOCIWFIRST] (iw_handler) orinoco_ioctl_getspy,
> +	[SIOCSIWAP    -SIOCIWFIRST] (iw_handler) orinoco_ioctl_setwap,
>  	[SIOCGIWAP    -SIOCIWFIRST] (iw_handler) orinoco_ioctl_getwap,
>  	[SIOCSIWESSID -SIOCIWFIRST] (iw_handler) orinoco_ioctl_setessid,
>  	[SIOCGIWESSID -SIOCIWFIRST] (iw_handler) orinoco_ioctl_getessid,
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/people/dgibson

^ permalink raw reply

* Re: [PATCH 3/12] orinoco: Symbol 3.0x firmware needs broken_disableport
From: David Gibson @ 2005-05-16  1:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: jgarzik, proski, netdev
In-Reply-To: <20050514153010.GD3643@lst.de>

On Sat, May 14, 2005 at 05:30:10PM +0200, Christoph Hellwig wrote:
> Patch from Pavel Roskin.

Acked-by: David Gibson <hermes@gibson.dropbear.id.au>

> Index: linux-2.6/drivers/net/wireless/orinoco.c
> ===================================================================
> +++ linux-2.6/drivers/net/wireless/orinoco.c	2005-05-14 16:57:39.000000000 +0200
> @@ -2223,6 +2223,8 @@
>  			       firmver >= 0x31000;
>  		priv->has_preamble = (firmver >= 0x20000);
>  		priv->ibss_port = 4;
> + 		priv->broken_disableport = (firmver == 0x25013) ||
> + 					   (firmver >= 0x30000 && firmver <= 0x31000);
>  		/* Tested with Intel firmware : 0x20015 => Jean II */
>  		/* Tested with 3Com firmware : 0x15012 & 0x22001 => Jean II */
>  		break;
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/people/dgibson

^ permalink raw reply

* Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
From: Herbert Xu @ 2005-05-16  1:00 UTC (permalink / raw)
  To: David S. Miller; +Cc: johnpol, netdev
In-Reply-To: <20050515.173944.122590923.davem@davemloft.net>

On Sun, May 15, 2005 at 05:39:44PM -0700, David S. Miller wrote:
> 
> This code should therefore just skb_orphan(), and nothing
> more.  Right?

Ideally we should move the ownership to the head skb here as well.
However, doing this would cause havoc when you have nasty users
sending fragments from two different sockets :)

So let's stick with just the skb_orphan.

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

^ permalink raw reply

* Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
From: David S. Miller @ 2005-05-16  0:39 UTC (permalink / raw)
  To: herbert; +Cc: johnpol, netdev
In-Reply-To: <20050515235913.GA20242@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 16 May 2005 09:59:13 +1000

> Actually that turns out to be OK since ip_ct_gather_frags zeros the
> skb->sk field before feeding it to ip_defrag.  Once the packet has
> been reassembled it tries to put the sk back which fails unless the
> head skb comes in last which is never the case on the output path.
> 
> Although the second part is buggy, it does not violate the invariant
> I stated before.

This code should therefore just skb_orphan(), and nothing
more.  Right?

^ permalink raw reply

* Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
From: David S. Miller @ 2005-05-16  0:32 UTC (permalink / raw)
  To: herbert; +Cc: johnpol, netdev
In-Reply-To: <20050515235913.GA20242@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 16 May 2005 09:59:13 +1000

> Please let me know if you still have any questions/problems with my
> patch.

No objections, I see what you are doing now.

It is basically the inverse of the case where
we hold the frag truesizes in the head SKB.

Once we rip apart the fraglist, we have to
repropagate the truesize and sock ownership
down from the head SKB to the frags.

I'll apply your patch, and Evgeniy's too.

Thanks.

^ permalink raw reply

* Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
From: Herbert Xu @ 2005-05-15 23:59 UTC (permalink / raw)
  To: David S. Miller; +Cc: johnpol, netdev
In-Reply-To: <20050515232020.GA19859@gondor.apana.org.au>

On Mon, May 16, 2005 at 09:20:20AM +1000, herbert wrote:
> 
> BTW, I missed the netfilter defrag code in my patch.  I'll send an
> updated version soon.

Actually that turns out to be OK since ip_ct_gather_frags zeros the
skb->sk field before feeding it to ip_defrag.  Once the packet has
been reassembled it tries to put the sk back which fails unless the
head skb comes in last which is never the case on the output path.

Although the second part is buggy, it does not violate the invariant
I stated before.

Please let me know if you still have any questions/problems with my
patch.

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

^ permalink raw reply

* Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
From: Herbert Xu @ 2005-05-15 23:20 UTC (permalink / raw)
  To: David S. Miller; +Cc: johnpol, netdev
In-Reply-To: <20050515.152441.106262257.davem@davemloft.net>

On Sun, May 15, 2005 at 03:24:41PM -0700, David S. Miller wrote:
>
> > So let's go the other way and make this an invariant:
> > 
> > 	For any skb on a frag_list, skb->sk must be NULL.
> > 
> > That is, the socket ownership always belongs to the head skb.
> > It turns out that the implementation is actually pretty simple.
> 
> Yes, I was just about to inform you that when SKBs are
> free'd up, only the head SKB's sk gets the destructor run
> on it.

Dave, I'm actually agreeing with you :) My patch does these two things:

1) Do what you did below for IPv4 to IPv6 as well.
2) Do what the comment (the one that you removed below) said and actually
   undo the truesize aggregation on the fast path in ip*_fragment.

I presume you won't have any problems with 1) since that's exactly
the same as your patch below except that it's for IPv6.

2) is needed because on the fast path the frag_list is unraveled so it
no longer makes sense to have the wmem all accounted to the head skb.
Please note that we only undo the attribution if the frag is not shared.
So the problem that your patch is meant fix won't occur here.

> @@ -1113,12 +1109,10 @@
>  		tail_skb = &(tmp_skb->next);
>  		skb->len += tmp_skb->len;
>  		skb->data_len += tmp_skb->len;
> -#if 0 /* Logically correct, but useless work, ip_fragment() will have to undo */
>  		skb->truesize += tmp_skb->truesize;
>  		__sock_put(tmp_skb->sk);
>  		tmp_skb->destructor = NULL;
>  		tmp_skb->sk = NULL;
> -#endif

All I'm saying is that if we remove the #if 0 then we should do as it
suggests and undo this accounting in ip_fragment.

BTW, I missed the netfilter defrag code in my patch.  I'll send an
updated version soon.

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

^ permalink raw reply

* Re: [patch 02/10] A new 10GB Ethernet Driver by Chelsio Communications
From: Jeff Garzik @ 2005-05-15 23:15 UTC (permalink / raw)
  To: akpm; +Cc: davem, christoph, tinay, sbardone, christoph, Netdev
In-Reply-To: <200503302134.j2ULYLVO028974@shell0.pdx.osdl.net>

applied

^ permalink raw reply

* Re: [PATCH] tlan: restore deleted module parameters.
From: Jeff Garzik @ 2005-05-15 22:25 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Stefaan De Roeck, netdev
In-Reply-To: <20050504153311.7d1a0fe9@dxpl.pdx.osdl.net>

applied

^ permalink raw reply

* Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
From: David S. Miller @ 2005-05-15 22:24 UTC (permalink / raw)
  To: herbert; +Cc: johnpol, netdev
In-Reply-To: <20050515114121.GA4830@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Subject: Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
Date: Sun, 15 May 2005 21:41:21 +1000

> So let's go the other way and make this an invariant:
> 
> 	For any skb on a frag_list, skb->sk must be NULL.
> 
> That is, the socket ownership always belongs to the head skb.
> It turns out that the implementation is actually pretty simple.

Yes, I was just about to inform you that when SKBs are
free'd up, only the head SKB's sk gets the destructor run
on it.

Please refer to this patch from July of 2004:

[IPV4]: Fix multicast socket hangs.

If a multicast packet gets looped back, the sending
socket can hang if a local read just sits and does
not empty its receive queue.

The problem is that when an SKB clone is freed up,
the destructor is only invoked for the head SKB when
there is a fraglist (which is created for fragmentation).

The solution is to account the fragment list SKB lengths
in the top-level head SKB, then it all works out.

Signed-off-by: David S. Miller <davem@redhat.com>

--- 1/net/ipv4/ip_output.c	2005-05-15 15:23:19 -07:00
+++ 2/net/ipv4/ip_output.c	2005-05-15 15:23:19 -07:00
@@ -498,10 +498,6 @@
 			    skb_headroom(frag) < hlen)
 			    goto slow_path;
 
-			/* Correct socket ownership. */
-			if (frag->sk == NULL && skb->sk)
-				goto slow_path;
-
 			/* Partially cloned skb? */
 			if (skb_shared(frag))
 				goto slow_path;
@@ -1113,12 +1109,10 @@
 		tail_skb = &(tmp_skb->next);
 		skb->len += tmp_skb->len;
 		skb->data_len += tmp_skb->len;
-#if 0 /* Logically correct, but useless work, ip_fragment() will have to undo */
 		skb->truesize += tmp_skb->truesize;
 		__sock_put(tmp_skb->sk);
 		tmp_skb->destructor = NULL;
 		tmp_skb->sk = NULL;
-#endif
 	}
 
 	/* Unless user demanded real pmtu discovery (IP_PMTUDISC_DO), we allow

^ permalink raw reply

* Re: [-mm patch] SIS900 must select MII
From: Jeff Garzik @ 2005-05-15 22:24 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Andrew Morton, venza, linux-kernel, netdev
In-Reply-To: <20050430115249.GA3654@stusta.de>

applied

^ permalink raw reply

* Re: [PATCH 2.6.12-rc3] pcnet32: fix resource leak with loopback test (with Signed-off-by)
From: Jeff Garzik @ 2005-05-15 22:22 UTC (permalink / raw)
  To: Don Fry; +Cc: tsbogend, netdev
In-Reply-To: <20050429215117.GA1082@us.ibm.com>

applied to 2.6.x

^ permalink raw reply

* Re: resend: [PATCH 2.6.11.8] drivers/net/wireless/airo.c: airo WEXT and quality corrections
From: Jeff Garzik @ 2005-05-15 22:21 UTC (permalink / raw)
  To: Dan Williams; +Cc: netdev, davem, breed
In-Reply-To: <1115732751.16008.8.camel@dcbw.boston.redhat.com>

applied

^ permalink raw reply

* Re: Fw: [Bugme-new] [Bug 4482] New: natsemi: incorrect initialization of IPv6 Neighbor-discovery multicast
From: Jeff Garzik @ 2005-05-15 22:19 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Andrew Morton, netdev, svallet
In-Reply-To: <20050416062416.GA29177@gondor.apana.org.au>

applied

^ permalink raw reply

* Re: [1/1] xfrm: skb_cow_data() does not set proper owner for new skbs.
From: David S. Miller @ 2005-05-15 22:18 UTC (permalink / raw)
  To: herbert; +Cc: johnpol, netdev
In-Reply-To: <E1DXE3h-0002jR-00@gondolin.me.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Subject: Re: [1/1] xfrm: skb_cow_data() does not set proper owner for new skbs.
Date: Sun, 15 May 2005 18:01:09 +1000

> IPv4's ip_push_pending_frames attributes all the data to the first
> skb which breaks when the packet is sent through ip_fragment since
> the latter does not undo the truesize attribution.

I'm %99 sure this one is on purpose.  I'll have to dig into
the archives and source repo history to get the details
though.

^ permalink raw reply

* Re: [patch linux-2.6.12-rc3-mm3 1/7] r8169: de-obfuscate supported PCI ID
From: Jeff Garzik @ 2005-05-15 22:14 UTC (permalink / raw)
  To: Francois Romieu; +Cc: Andrew Morton, netdev
In-Reply-To: <20050505225656.GA11200@electric-eye.fr.zoreil.com>

All seven look OK, but it failed on the first patch:

[jgarzik@pretzel netdev-2.6]$ dotest /g/tmp/mbox

Applying 'r8169: de-obfuscate supported PCI ID'

patching file drivers/net/r8169.c
Hunk #1 FAILED at 174.
1 out of 1 hunk FAILED -- saving rejects to file drivers/net/r8169.c.rej

^ permalink raw reply

* Re: [PATCH] forcedeth Update error handling
From: Jeff Garzik @ 2005-05-15 22:10 UTC (permalink / raw)
  To: Manfred Spraul; +Cc: AAbdulla, Netdev, Carl-Daniel Hailfinger, adq
In-Reply-To: <42655935.9050703@colorfullife.com>

applied

^ permalink raw reply

* Re: [PATCH 1/4] iseries_veth: Don't send packets to LPARs which aren't up
From: Jeff Garzik @ 2005-05-15 22:08 UTC (permalink / raw)
  To: michael; +Cc: Andrew Morton, netdev, linux-kernel
In-Reply-To: <200505121747.27752.michael@ellerman.id.au>

applied patches 1-4

^ permalink raw reply

* Re: [2.6 patch] document that 8139TOO supports 8129/8130
From: Jeff Garzik @ 2005-05-15 21:59 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: netdev, linux-kernel
In-Reply-To: <20050513040445.GD3603@stusta.de>

Adrian Bunk wrote:
> +	  the RTL 8129/81308139 chips. If you have one of those, say Y and

typo:  "81308139"

^ 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