Netdev List
 help / color / mirror / Atom feed
* [patch 0/2] s390: claw and ctcm patches
From: Ursula Braun @ 2010-08-12 11:58 UTC (permalink / raw)
  To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens

Dave,

these patches repair warnings seen with the linux-2.6.git 2.6.35
caused by the used constants READ / WRITE.

The patches apply on linux-2.6.git and net-next-2.6.git.

shortlog:
Heiko Carstens (1)
claw: rename READ/WRITE defines to avoid redefinitions

Ursula Braun (1)
ctcm: rename READ/WRITE defines to avoid redefinitions

Thanks,
      Ursula


^ permalink raw reply

* [patch 1/2] [PATCH] claw: rename READ/WRITE defines to avoid redefinitions
From: Ursula Braun @ 2010-08-12 11:58 UTC (permalink / raw)
  To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens, Ursula Braun
In-Reply-To: <20100812115826.983015000@linux.vnet.ibm.com>

[-- Attachment #1: 601-claw-rename.diff --]
[-- Type: text/plain, Size: 15473 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

READ/WRITE seems to be a bit too generic for defines in a device driver.
Just rename them to READ_CHANNEL/WRITE_CHANNEL which should suffice.

Fixes this:

In file included from drivers/s390/net/claw.c:93:
drivers/s390/net/claw.h:78:1: warning: "WRITE" redefined
In file included from /home2/heicarst/linux-2.6/arch/s390/include/asm/debug.h:12,
                 from drivers/s390/net/claw.c:68:
include/linux/fs.h:156:1: warning: this is the location of the previous definition

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

 drivers/s390/net/claw.c |  118 ++++++++++++++++++++++++------------------------
 drivers/s390/net/claw.h |    4 -
 2 files changed, 61 insertions(+), 61 deletions(-)

--- a/drivers/s390/net/claw.c
+++ b/drivers/s390/net/claw.c
@@ -386,7 +386,7 @@ claw_tx(struct sk_buff *skb, struct net_
         struct chbk *p_ch;
 
 	CLAW_DBF_TEXT(4, trace, "claw_tx");
-        p_ch=&privptr->channel[WRITE];
+	p_ch = &privptr->channel[WRITE_CHANNEL];
         spin_lock_irqsave(get_ccwdev_lock(p_ch->cdev), saveflags);
         rc=claw_hw_tx( skb, dev, 1 );
         spin_unlock_irqrestore(get_ccwdev_lock(p_ch->cdev), saveflags);
@@ -407,7 +407,7 @@ static struct sk_buff *
 claw_pack_skb(struct claw_privbk *privptr)
 {
 	struct sk_buff *new_skb,*held_skb;
-	struct chbk *p_ch = &privptr->channel[WRITE];
+	struct chbk *p_ch = &privptr->channel[WRITE_CHANNEL];
 	struct claw_env  *p_env = privptr->p_env;
 	int	pkt_cnt,pk_ind,so_far;
 
@@ -515,15 +515,15 @@ claw_open(struct net_device *dev)
 		privptr->p_env->write_size=CLAW_FRAME_SIZE;
 	}
         claw_set_busy(dev);
-	tasklet_init(&privptr->channel[READ].tasklet, claw_irq_tasklet,
-        	(unsigned long) &privptr->channel[READ]);
+	tasklet_init(&privptr->channel[READ_CHANNEL].tasklet, claw_irq_tasklet,
+		(unsigned long) &privptr->channel[READ_CHANNEL]);
         for ( i = 0; i < 2;  i++) {
 		CLAW_DBF_TEXT_(2, trace, "opn_ch%d", i);
                 init_waitqueue_head(&privptr->channel[i].wait);
 		/* skb_queue_head_init(&p_ch->io_queue); */
-		if (i == WRITE)
+		if (i == WRITE_CHANNEL)
 			skb_queue_head_init(
-				&privptr->channel[WRITE].collect_queue);
+				&privptr->channel[WRITE_CHANNEL].collect_queue);
                 privptr->channel[i].flag_a = 0;
                 privptr->channel[i].IO_active = 0;
                 privptr->channel[i].flag  &= ~CLAW_TIMER;
@@ -551,12 +551,12 @@ claw_open(struct net_device *dev)
                 if((privptr->channel[i].flag & CLAW_TIMER) == 0x00)
                         del_timer(&timer);
         }
-        if ((((privptr->channel[READ].last_dstat |
-		privptr->channel[WRITE].last_dstat) &
+	if ((((privptr->channel[READ_CHANNEL].last_dstat |
+		privptr->channel[WRITE_CHANNEL].last_dstat) &
            ~(DEV_STAT_CHN_END | DEV_STAT_DEV_END)) != 0x00) ||
-           (((privptr->channel[READ].flag |
-	   	privptr->channel[WRITE].flag) & CLAW_TIMER) != 0x00)) {
-		dev_info(&privptr->channel[READ].cdev->dev,
+	   (((privptr->channel[READ_CHANNEL].flag |
+		privptr->channel[WRITE_CHANNEL].flag) & CLAW_TIMER) != 0x00)) {
+		dev_info(&privptr->channel[READ_CHANNEL].cdev->dev,
 			"%s: remote side is not ready\n", dev->name);
 		CLAW_DBF_TEXT(2, trace, "notrdy");
 
@@ -608,8 +608,8 @@ claw_open(struct net_device *dev)
                         }
                 }
 		privptr->buffs_alloc = 0;
-		privptr->channel[READ].flag= 0x00;
-		privptr->channel[WRITE].flag = 0x00;
+		privptr->channel[READ_CHANNEL].flag = 0x00;
+		privptr->channel[WRITE_CHANNEL].flag = 0x00;
                 privptr->p_buff_ccw=NULL;
                 privptr->p_buff_read=NULL;
                 privptr->p_buff_write=NULL;
@@ -652,10 +652,10 @@ claw_irq_handler(struct ccw_device *cdev
         }
 
 	/* Try to extract channel from driver data. */
-	if (privptr->channel[READ].cdev == cdev)
-		p_ch = &privptr->channel[READ];
-	else if (privptr->channel[WRITE].cdev == cdev)
-		p_ch = &privptr->channel[WRITE];
+	if (privptr->channel[READ_CHANNEL].cdev == cdev)
+		p_ch = &privptr->channel[READ_CHANNEL];
+	else if (privptr->channel[WRITE_CHANNEL].cdev == cdev)
+		p_ch = &privptr->channel[WRITE_CHANNEL];
 	else {
 		dev_warn(&cdev->dev, "The device is not a CLAW device\n");
 		CLAW_DBF_TEXT(2, trace, "badchan");
@@ -813,7 +813,7 @@ claw_irq_handler(struct ccw_device *cdev
 			claw_clearbit_busy(TB_TX, dev);
 			claw_clear_busy(dev);
 		}
-		p_ch_r = (struct chbk *)&privptr->channel[READ];
+		p_ch_r = (struct chbk *)&privptr->channel[READ_CHANNEL];
 		if (test_and_set_bit(CLAW_BH_ACTIVE,
 			(void *)&p_ch_r->flag_a) == 0)
 			tasklet_schedule(&p_ch_r->tasklet);
@@ -878,13 +878,13 @@ claw_release(struct net_device *dev)
         for ( i = 1; i >=0 ;  i--) {
                 spin_lock_irqsave(
 			get_ccwdev_lock(privptr->channel[i].cdev), saveflags);
-             /*   del_timer(&privptr->channel[READ].timer);  */
+	     /*   del_timer(&privptr->channel[READ_CHANNEL].timer);  */
  		privptr->channel[i].claw_state = CLAW_STOP;
                 privptr->channel[i].IO_active = 0;
                 parm = (unsigned long) &privptr->channel[i];
-		if (i == WRITE)
+		if (i == WRITE_CHANNEL)
 			claw_purge_skb_queue(
-				&privptr->channel[WRITE].collect_queue);
+				&privptr->channel[WRITE_CHANNEL].collect_queue);
                 rc = ccw_device_halt (privptr->channel[i].cdev, parm);
 	        if (privptr->system_validate_comp==0x00)  /* never opened? */
                    init_waitqueue_head(&privptr->channel[i].wait);
@@ -971,16 +971,16 @@ claw_release(struct net_device *dev)
         privptr->mtc_skipping = 1;
         privptr->mtc_offset=0;
 
-        if (((privptr->channel[READ].last_dstat |
-		privptr->channel[WRITE].last_dstat) &
+	if (((privptr->channel[READ_CHANNEL].last_dstat |
+		privptr->channel[WRITE_CHANNEL].last_dstat) &
 		~(DEV_STAT_CHN_END | DEV_STAT_DEV_END)) != 0x00) {
-		dev_warn(&privptr->channel[READ].cdev->dev,
+		dev_warn(&privptr->channel[READ_CHANNEL].cdev->dev,
 			"Deactivating %s completed with incorrect"
 			" subchannel status "
 			"(read %02x, write %02x)\n",
                 dev->name,
-		privptr->channel[READ].last_dstat,
-		privptr->channel[WRITE].last_dstat);
+		privptr->channel[READ_CHANNEL].last_dstat,
+		privptr->channel[WRITE_CHANNEL].last_dstat);
 		 CLAW_DBF_TEXT(2, trace, "badclose");
         }
 	CLAW_DBF_TEXT(4, trace, "rlsexit");
@@ -1324,7 +1324,7 @@ claw_hw_tx(struct sk_buff *skb, struct n
 
 	CLAW_DBF_TEXT(4, trace, "hw_tx");
 	privptr = (struct claw_privbk *)(dev->ml_priv);
-        p_ch=(struct chbk *)&privptr->channel[WRITE];
+	p_ch = (struct chbk *)&privptr->channel[WRITE_CHANNEL];
 	p_env =privptr->p_env;
 	claw_free_wrt_buf(dev);	/* Clean up free chain if posible */
         /*  scan the write queue to free any completed write packets   */
@@ -1357,7 +1357,7 @@ claw_hw_tx(struct sk_buff *skb, struct n
                                 claw_strt_out_IO(dev );
                                 claw_free_wrt_buf( dev );
                                 if (privptr->write_free_count==0) {
-					ch = &privptr->channel[WRITE];
+					ch = &privptr->channel[WRITE_CHANNEL];
 					atomic_inc(&skb->users);
 					skb_queue_tail(&ch->collect_queue, skb);
                                 	goto Done;
@@ -1369,7 +1369,7 @@ claw_hw_tx(struct sk_buff *skb, struct n
                 }
                 /*  tx lock  */
                 if (claw_test_and_setbit_busy(TB_TX,dev)) { /* set to busy */
-			ch = &privptr->channel[WRITE];
+			ch = &privptr->channel[WRITE_CHANNEL];
 			atomic_inc(&skb->users);
 			skb_queue_tail(&ch->collect_queue, skb);
                         claw_strt_out_IO(dev );
@@ -1385,7 +1385,7 @@ claw_hw_tx(struct sk_buff *skb, struct n
             privptr->p_write_free_chain == NULL ) {
 
                 claw_setbit_busy(TB_NOBUFFER,dev);
-		ch = &privptr->channel[WRITE];
+		ch = &privptr->channel[WRITE_CHANNEL];
 		atomic_inc(&skb->users);
 		skb_queue_tail(&ch->collect_queue, skb);
 		CLAW_DBF_TEXT(2, trace, "clawbusy");
@@ -1397,7 +1397,7 @@ claw_hw_tx(struct sk_buff *skb, struct n
         while (len_of_data > 0) {
                 p_this_ccw=privptr->p_write_free_chain;  /* get a block */
 		if (p_this_ccw == NULL) { /* lost the race */
-			ch = &privptr->channel[WRITE];
+			ch = &privptr->channel[WRITE_CHANNEL];
 			atomic_inc(&skb->users);
 			skb_queue_tail(&ch->collect_queue, skb);
 			goto Done2;
@@ -2067,7 +2067,7 @@ claw_process_control( struct net_device 
 			*catch up to each other */
 	privptr = dev->ml_priv;
         p_env=privptr->p_env;
-	tdev = &privptr->channel[READ].cdev->dev;
+	tdev = &privptr->channel[READ_CHANNEL].cdev->dev;
 	memcpy( &temp_host_name, p_env->host_name, 8);
         memcpy( &temp_ws_name, p_env->adapter_name , 8);
 	dev_info(tdev, "%s: CLAW device %.8s: "
@@ -2245,7 +2245,7 @@ claw_process_control( struct net_device 
 			dev->name, temp_ws_name,
 			p_ctlbk->linkid);
 			privptr->active_link_ID = p_ctlbk->linkid;
-			p_ch = &privptr->channel[WRITE];
+			p_ch = &privptr->channel[WRITE_CHANNEL];
 			wake_up(&p_ch->wait);  /* wake up claw_open ( WRITE) */
 		break;
 	case CONNECTION_RESPONSE:
@@ -2296,7 +2296,7 @@ claw_process_control( struct net_device 
 				"%s: Confirmed Now packing\n", dev->name);
 				p_env->packing = DO_PACKED;
 			}
-			p_ch = &privptr->channel[WRITE];
+			p_ch = &privptr->channel[WRITE_CHANNEL];
 			wake_up(&p_ch->wait);
 		} else {
 			dev_warn(tdev, "Activating %s failed because of"
@@ -2556,7 +2556,7 @@ unpack_read(struct net_device *dev )
 	p_packd=NULL;
 	privptr = dev->ml_priv;
 
-	p_dev = &privptr->channel[READ].cdev->dev;
+	p_dev = &privptr->channel[READ_CHANNEL].cdev->dev;
 	p_env = privptr->p_env;
         p_this_ccw=privptr->p_read_active_first;
 	while (p_this_ccw!=NULL && p_this_ccw->header.flag!=CLAW_PENDING) {
@@ -2728,7 +2728,7 @@ claw_strt_read (struct net_device *dev, 
         struct ccwbk*p_ccwbk;
         struct chbk *p_ch;
         struct clawh *p_clawh;
-        p_ch=&privptr->channel[READ];
+	p_ch = &privptr->channel[READ_CHANNEL];
 
 	CLAW_DBF_TEXT(4, trace, "StRdNter");
         p_clawh=(struct clawh *)privptr->p_claw_signal_blk;
@@ -2782,7 +2782,7 @@ claw_strt_out_IO( struct net_device *dev
 		return;
 	}
 	privptr = (struct claw_privbk *)dev->ml_priv;
-        p_ch=&privptr->channel[WRITE];
+	p_ch = &privptr->channel[WRITE_CHANNEL];
 
 	CLAW_DBF_TEXT(4, trace, "strt_io");
         p_first_ccw=privptr->p_write_active_first;
@@ -2875,7 +2875,7 @@ claw_free_netdevice(struct net_device * 
 	if (dev->flags & IFF_RUNNING)
 		claw_release(dev);
 	if (privptr) {
-		privptr->channel[READ].ndev = NULL;  /* say it's free */
+		privptr->channel[READ_CHANNEL].ndev = NULL;  /* say it's free */
 	}
 	dev->ml_priv = NULL;
 #ifdef MODULE
@@ -2960,18 +2960,18 @@ claw_new_device(struct ccwgroup_device *
 	struct ccw_dev_id dev_id;
 
 	dev_info(&cgdev->dev, "add for %s\n",
-		 dev_name(&cgdev->cdev[READ]->dev));
+		 dev_name(&cgdev->cdev[READ_CHANNEL]->dev));
 	CLAW_DBF_TEXT(2, setup, "new_dev");
 	privptr = dev_get_drvdata(&cgdev->dev);
-	dev_set_drvdata(&cgdev->cdev[READ]->dev, privptr);
-	dev_set_drvdata(&cgdev->cdev[WRITE]->dev, privptr);
+	dev_set_drvdata(&cgdev->cdev[READ_CHANNEL]->dev, privptr);
+	dev_set_drvdata(&cgdev->cdev[WRITE_CHANNEL]->dev, privptr);
 	if (!privptr)
 		return -ENODEV;
 	p_env = privptr->p_env;
-	ccw_device_get_id(cgdev->cdev[READ], &dev_id);
-	p_env->devno[READ] = dev_id.devno;
-	ccw_device_get_id(cgdev->cdev[WRITE], &dev_id);
-	p_env->devno[WRITE] = dev_id.devno;
+	ccw_device_get_id(cgdev->cdev[READ_CHANNEL], &dev_id);
+	p_env->devno[READ_CHANNEL] = dev_id.devno;
+	ccw_device_get_id(cgdev->cdev[WRITE_CHANNEL], &dev_id);
+	p_env->devno[WRITE_CHANNEL] = dev_id.devno;
 	ret = add_channel(cgdev->cdev[0],0,privptr);
 	if (ret == 0)
 		ret = add_channel(cgdev->cdev[1],1,privptr);
@@ -2980,14 +2980,14 @@ claw_new_device(struct ccwgroup_device *
 			" failed with error code %d\n", ret);
 		goto out;
 	}
-	ret = ccw_device_set_online(cgdev->cdev[READ]);
+	ret = ccw_device_set_online(cgdev->cdev[READ_CHANNEL]);
 	if (ret != 0) {
 		dev_warn(&cgdev->dev,
 			"Setting the read subchannel online"
 			" failed with error code %d\n", ret);
 		goto out;
 	}
-	ret = ccw_device_set_online(cgdev->cdev[WRITE]);
+	ret = ccw_device_set_online(cgdev->cdev[WRITE_CHANNEL]);
 	if (ret != 0) {
 		dev_warn(&cgdev->dev,
 			"Setting the write subchannel online "
@@ -3002,8 +3002,8 @@ claw_new_device(struct ccwgroup_device *
 	}
 	dev->ml_priv = privptr;
 	dev_set_drvdata(&cgdev->dev, privptr);
-	dev_set_drvdata(&cgdev->cdev[READ]->dev, privptr);
-	dev_set_drvdata(&cgdev->cdev[WRITE]->dev, privptr);
+	dev_set_drvdata(&cgdev->cdev[READ_CHANNEL]->dev, privptr);
+	dev_set_drvdata(&cgdev->cdev[WRITE_CHANNEL]->dev, privptr);
 	/* sysfs magic */
         SET_NETDEV_DEV(dev, &cgdev->dev);
 	if (register_netdev(dev) != 0) {
@@ -3021,16 +3021,16 @@ claw_new_device(struct ccwgroup_device *
 			goto out;
 		}
 	}
-	privptr->channel[READ].ndev = dev;
-	privptr->channel[WRITE].ndev = dev;
+	privptr->channel[READ_CHANNEL].ndev = dev;
+	privptr->channel[WRITE_CHANNEL].ndev = dev;
 	privptr->p_env->ndev = dev;
 
 	dev_info(&cgdev->dev, "%s:readsize=%d  writesize=%d "
 		"readbuffer=%d writebuffer=%d read=0x%04x write=0x%04x\n",
                 dev->name, p_env->read_size,
 		p_env->write_size, p_env->read_buffers,
-                p_env->write_buffers, p_env->devno[READ],
-		p_env->devno[WRITE]);
+		p_env->write_buffers, p_env->devno[READ_CHANNEL],
+		p_env->devno[WRITE_CHANNEL]);
 	dev_info(&cgdev->dev, "%s:host_name:%.8s, adapter_name "
 		":%.8s api_type: %.8s\n",
                 dev->name, p_env->host_name,
@@ -3072,10 +3072,10 @@ claw_shutdown_device(struct ccwgroup_dev
 	priv = dev_get_drvdata(&cgdev->dev);
 	if (!priv)
 		return -ENODEV;
-	ndev = priv->channel[READ].ndev;
+	ndev = priv->channel[READ_CHANNEL].ndev;
 	if (ndev) {
 		/* Close the device */
-		dev_info(&cgdev->dev, "%s: shutting down \n",
+		dev_info(&cgdev->dev, "%s: shutting down\n",
 			ndev->name);
 		if (ndev->flags & IFF_RUNNING)
 			ret = claw_release(ndev);
@@ -3083,8 +3083,8 @@ claw_shutdown_device(struct ccwgroup_dev
 		unregister_netdev(ndev);
 		ndev->ml_priv = NULL;  /* cgdev data, not ndev's to free */
 		claw_free_netdevice(ndev, 1);
-		priv->channel[READ].ndev = NULL;
-		priv->channel[WRITE].ndev = NULL;
+		priv->channel[READ_CHANNEL].ndev = NULL;
+		priv->channel[WRITE_CHANNEL].ndev = NULL;
 		priv->p_env->ndev = NULL;
 	}
 	ccw_device_set_offline(cgdev->cdev[1]);
@@ -3115,8 +3115,8 @@ claw_remove_device(struct ccwgroup_devic
 	priv->channel[1].irb=NULL;
 	kfree(priv);
 	dev_set_drvdata(&cgdev->dev, NULL);
-	dev_set_drvdata(&cgdev->cdev[READ]->dev, NULL);
-	dev_set_drvdata(&cgdev->cdev[WRITE]->dev, NULL);
+	dev_set_drvdata(&cgdev->cdev[READ_CHANNEL]->dev, NULL);
+	dev_set_drvdata(&cgdev->cdev[WRITE_CHANNEL]->dev, NULL);
 	put_device(&cgdev->dev);
 
 	return;
--- a/drivers/s390/net/claw.h
+++ b/drivers/s390/net/claw.h
@@ -74,8 +74,8 @@
 #define MAX_ENVELOPE_SIZE       65536
 #define CLAW_DEFAULT_MTU_SIZE   4096
 #define DEF_PACK_BUFSIZE	32768
-#define READ                    0
-#define WRITE                   1
+#define READ_CHANNEL		0
+#define WRITE_CHANNEL		1
 
 #define TB_TX                   0          /* sk buffer handling in process  */
 #define TB_STOP                 1          /* network device stop in process */


^ permalink raw reply

* [patch 2/2] [PATCH] ctcm: rename READ/WRITE defines to avoid redefinitions
From: Ursula Braun @ 2010-08-12 11:58 UTC (permalink / raw)
  To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens, Ursula Braun
In-Reply-To: <20100812115826.983015000@linux.vnet.ibm.com>

[-- Attachment #1: 602-ctcm-rename.diff --]
[-- Type: text/plain, Size: 25838 bytes --]

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

READ/WRITE seems to be a bit too generic for defines in a device
driver. Just rename them to CTCM_READ/CTCM_WRITE to avoid warnings.

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

 drivers/s390/net/ctcm_fsms.c  |   60 ++++++++++++++++---------------
 drivers/s390/net/ctcm_main.c  |   80 +++++++++++++++++++++---------------------
 drivers/s390/net/ctcm_main.h  |    4 +-
 drivers/s390/net/ctcm_mpc.c   |   64 +++++++++++++++++----------------
 drivers/s390/net/ctcm_sysfs.c |   20 +++++-----
 5 files changed, 118 insertions(+), 110 deletions(-)

diff -urpN linux-2.6/drivers/s390/net/ctcm_fsms.c linux-2.6-patched/drivers/s390/net/ctcm_fsms.c
--- linux-2.6/drivers/s390/net/ctcm_fsms.c	2010-08-02 00:11:14.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_fsms.c	2010-08-12 13:29:50.000000000 +0200
@@ -454,7 +454,7 @@ static void chx_firstio(fsm_instance *fi
 	if ((fsmstate == CTC_STATE_SETUPWAIT) &&
 	    (ch->protocol == CTCM_PROTO_OS390)) {
 		/* OS/390 resp. z/OS */
-		if (CHANNEL_DIRECTION(ch->flags) == READ) {
+		if (CHANNEL_DIRECTION(ch->flags) == CTCM_READ) {
 			*((__u16 *)ch->trans_skb->data) = CTCM_INITIAL_BLOCKLEN;
 			fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC,
 				     CTC_EVENT_TIMER, ch);
@@ -472,14 +472,14 @@ static void chx_firstio(fsm_instance *fi
 	 * if in compatibility mode, since VM TCP delays the initial
 	 * frame until it has some data to send.
 	 */
-	if ((CHANNEL_DIRECTION(ch->flags) == WRITE) ||
+	if ((CHANNEL_DIRECTION(ch->flags) == CTCM_WRITE) ||
 	    (ch->protocol != CTCM_PROTO_S390))
 		fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
 
 	*((__u16 *)ch->trans_skb->data) = CTCM_INITIAL_BLOCKLEN;
 	ch->ccw[1].count = 2;	/* Transfer only length */
 
-	fsm_newstate(fi, (CHANNEL_DIRECTION(ch->flags) == READ)
+	fsm_newstate(fi, (CHANNEL_DIRECTION(ch->flags) == CTCM_READ)
 		     ? CTC_STATE_RXINIT : CTC_STATE_TXINIT);
 	rc = ccw_device_start(ch->cdev, &ch->ccw[0],
 					(unsigned long)ch, 0xff, 0);
@@ -495,7 +495,7 @@ static void chx_firstio(fsm_instance *fi
 	 * reply from VM TCP which brings up the RX channel to it's
 	 * final state.
 	 */
-	if ((CHANNEL_DIRECTION(ch->flags) == READ) &&
+	if ((CHANNEL_DIRECTION(ch->flags) == CTCM_READ) &&
 	    (ch->protocol == CTCM_PROTO_S390)) {
 		struct net_device *dev = ch->netdev;
 		struct ctcm_priv *priv = dev->ml_priv;
@@ -600,15 +600,15 @@ static void ctcm_chx_start(fsm_instance 
 	int rc;
 
 	CTCM_DBF_TEXT_(SETUP, CTC_DBF_INFO, "%s(%s): %s",
-			CTCM_FUNTAIL, ch->id,
-			(CHANNEL_DIRECTION(ch->flags) == READ) ? "RX" : "TX");
+		CTCM_FUNTAIL, ch->id,
+		(CHANNEL_DIRECTION(ch->flags) == CTCM_READ) ? "RX" : "TX");
 
 	if (ch->trans_skb != NULL) {
 		clear_normalized_cda(&ch->ccw[1]);
 		dev_kfree_skb(ch->trans_skb);
 		ch->trans_skb = NULL;
 	}
-	if (CHANNEL_DIRECTION(ch->flags) == READ) {
+	if (CHANNEL_DIRECTION(ch->flags) == CTCM_READ) {
 		ch->ccw[1].cmd_code = CCW_CMD_READ;
 		ch->ccw[1].flags = CCW_FLAG_SLI;
 		ch->ccw[1].count = 0;
@@ -622,7 +622,8 @@ static void ctcm_chx_start(fsm_instance 
 			"%s(%s): %s trans_skb alloc delayed "
 			"until first transfer",
 			CTCM_FUNTAIL, ch->id,
-			(CHANNEL_DIRECTION(ch->flags) == READ) ? "RX" : "TX");
+			(CHANNEL_DIRECTION(ch->flags) == CTCM_READ) ?
+				"RX" : "TX");
 	}
 	ch->ccw[0].cmd_code = CCW_CMD_PREPARE;
 	ch->ccw[0].flags = CCW_FLAG_SLI | CCW_FLAG_CC;
@@ -720,7 +721,7 @@ static void ctcm_chx_cleanup(fsm_instanc
 
 	ch->th_seg = 0x00;
 	ch->th_seq_num = 0x00;
-	if (CHANNEL_DIRECTION(ch->flags) == READ) {
+	if (CHANNEL_DIRECTION(ch->flags) == CTCM_READ) {
 		skb_queue_purge(&ch->io_queue);
 		fsm_event(priv->fsm, DEV_EVENT_RXDOWN, dev);
 	} else {
@@ -799,7 +800,8 @@ static void ctcm_chx_setuperr(fsm_instan
 		fsm_newstate(fi, CTC_STATE_STARTRETRY);
 		fsm_deltimer(&ch->timer);
 		fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
-		if (!IS_MPC(ch) && (CHANNEL_DIRECTION(ch->flags) == READ)) {
+		if (!IS_MPC(ch) &&
+		    (CHANNEL_DIRECTION(ch->flags) == CTCM_READ)) {
 			int rc = ccw_device_halt(ch->cdev, (unsigned long)ch);
 			if (rc != 0)
 				ctcm_ccw_check_rc(ch, rc,
@@ -811,10 +813,10 @@ static void ctcm_chx_setuperr(fsm_instan
 	CTCM_DBF_TEXT_(ERROR, CTC_DBF_CRIT,
 		"%s(%s) : %s error during %s channel setup state=%s\n",
 		CTCM_FUNTAIL, dev->name, ctc_ch_event_names[event],
-		(CHANNEL_DIRECTION(ch->flags) == READ) ? "RX" : "TX",
+		(CHANNEL_DIRECTION(ch->flags) == CTCM_READ) ? "RX" : "TX",
 		fsm_getstate_str(fi));
 
-	if (CHANNEL_DIRECTION(ch->flags) == READ) {
+	if (CHANNEL_DIRECTION(ch->flags) == CTCM_READ) {
 		fsm_newstate(fi, CTC_STATE_RXERR);
 		fsm_event(priv->fsm, DEV_EVENT_RXDOWN, dev);
 	} else {
@@ -945,7 +947,7 @@ static void ctcm_chx_rxdisc(fsm_instance
 	fsm_event(priv->fsm, DEV_EVENT_TXDOWN, dev);
 
 	fsm_newstate(fi, CTC_STATE_DTERM);
-	ch2 = priv->channel[WRITE];
+	ch2 = priv->channel[CTCM_WRITE];
 	fsm_newstate(ch2->fsm, CTC_STATE_DTERM);
 
 	ccw_device_halt(ch->cdev, (unsigned long)ch);
@@ -1074,13 +1076,13 @@ static void ctcm_chx_iofatal(fsm_instanc
 	fsm_deltimer(&ch->timer);
 	CTCM_DBF_TEXT_(ERROR, CTC_DBF_ERROR,
 		"%s: %s: %s unrecoverable channel error",
-			CTCM_FUNTAIL, ch->id, rd == READ ? "RX" : "TX");
+			CTCM_FUNTAIL, ch->id, rd == CTCM_READ ? "RX" : "TX");
 
 	if (IS_MPC(ch)) {
 		priv->stats.tx_dropped++;
 		priv->stats.tx_errors++;
 	}
-	if (rd == READ) {
+	if (rd == CTCM_READ) {
 		fsm_newstate(fi, CTC_STATE_RXERR);
 		fsm_event(priv->fsm, DEV_EVENT_RXDOWN, dev);
 	} else {
@@ -1503,7 +1505,7 @@ static void ctcmpc_chx_firstio(fsm_insta
 	switch (fsm_getstate(fi)) {
 	case CTC_STATE_STARTRETRY:
 	case CTC_STATE_SETUPWAIT:
-		if (CHANNEL_DIRECTION(ch->flags) == READ) {
+		if (CHANNEL_DIRECTION(ch->flags) == CTCM_READ) {
 			ctcmpc_chx_rxidle(fi, event, arg);
 		} else {
 			fsm_newstate(fi, CTC_STATE_TXIDLE);
@@ -1514,7 +1516,7 @@ static void ctcmpc_chx_firstio(fsm_insta
 		break;
 	};
 
-	fsm_newstate(fi, (CHANNEL_DIRECTION(ch->flags) == READ)
+	fsm_newstate(fi, (CHANNEL_DIRECTION(ch->flags) == CTCM_READ)
 		     ? CTC_STATE_RXINIT : CTC_STATE_TXINIT);
 
 done:
@@ -1753,8 +1755,8 @@ static void ctcmpc_chx_send_sweep(fsm_in
 	struct net_device *dev = ach->netdev;
 	struct ctcm_priv *priv = dev->ml_priv;
 	struct mpc_group *grp = priv->mpcg;
-	struct channel *wch = priv->channel[WRITE];
-	struct channel *rch = priv->channel[READ];
+	struct channel *wch = priv->channel[CTCM_WRITE];
+	struct channel *rch = priv->channel[CTCM_READ];
 	struct sk_buff *skb;
 	struct th_sweep *header;
 	int rc = 0;
@@ -2070,7 +2072,7 @@ static void dev_action_start(fsm_instanc
 	fsm_newstate(fi, DEV_STATE_STARTWAIT_RXTX);
 	if (IS_MPC(priv))
 		priv->mpcg->channels_terminating = 0;
-	for (direction = READ; direction <= WRITE; direction++) {
+	for (direction = CTCM_READ; direction <= CTCM_WRITE; direction++) {
 		struct channel *ch = priv->channel[direction];
 		fsm_event(ch->fsm, CTC_EVENT_START, ch);
 	}
@@ -2092,7 +2094,7 @@ static void dev_action_stop(fsm_instance
 	CTCMY_DBF_DEV_NAME(SETUP, dev, "");
 
 	fsm_newstate(fi, DEV_STATE_STOPWAIT_RXTX);
-	for (direction = READ; direction <= WRITE; direction++) {
+	for (direction = CTCM_READ; direction <= CTCM_WRITE; direction++) {
 		struct channel *ch = priv->channel[direction];
 		fsm_event(ch->fsm, CTC_EVENT_STOP, ch);
 		ch->th_seq_num = 0x00;
@@ -2183,11 +2185,11 @@ static void dev_action_chup(fsm_instance
 
 	if (IS_MPC(priv)) {
 		if (event == DEV_EVENT_RXUP)
-			mpc_channel_action(priv->channel[READ],
-				READ, MPC_CHANNEL_ADD);
+			mpc_channel_action(priv->channel[CTCM_READ],
+				CTCM_READ, MPC_CHANNEL_ADD);
 		else
-			mpc_channel_action(priv->channel[WRITE],
-				WRITE, MPC_CHANNEL_ADD);
+			mpc_channel_action(priv->channel[CTCM_WRITE],
+				CTCM_WRITE, MPC_CHANNEL_ADD);
 	}
 }
 
@@ -2239,11 +2241,11 @@ static void dev_action_chdown(fsm_instan
 	}
 	if (IS_MPC(priv)) {
 		if (event == DEV_EVENT_RXDOWN)
-			mpc_channel_action(priv->channel[READ],
-				READ, MPC_CHANNEL_REMOVE);
+			mpc_channel_action(priv->channel[CTCM_READ],
+				CTCM_READ, MPC_CHANNEL_REMOVE);
 		else
-			mpc_channel_action(priv->channel[WRITE],
-				WRITE, MPC_CHANNEL_REMOVE);
+			mpc_channel_action(priv->channel[CTCM_WRITE],
+				CTCM_WRITE, MPC_CHANNEL_REMOVE);
 	}
 }
 
diff -urpN linux-2.6/drivers/s390/net/ctcm_main.c linux-2.6-patched/drivers/s390/net/ctcm_main.c
--- linux-2.6/drivers/s390/net/ctcm_main.c	2010-08-02 00:11:14.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.c	2010-08-12 13:29:50.000000000 +0200
@@ -267,7 +267,7 @@ static struct channel *channel_get(enum 
 		else {
 			ch->flags |= CHANNEL_FLAGS_INUSE;
 			ch->flags &= ~CHANNEL_FLAGS_RWMASK;
-			ch->flags |= (direction == WRITE)
+			ch->flags |= (direction == CTCM_WRITE)
 			    ? CHANNEL_FLAGS_WRITE : CHANNEL_FLAGS_READ;
 			fsm_newstate(ch->fsm, CTC_STATE_STOPPED);
 		}
@@ -388,7 +388,8 @@ int ctcm_ch_alloc_buffer(struct channel 
 		CTCM_DBF_TEXT_(ERROR, CTC_DBF_ERROR,
 			"%s(%s): %s trans_skb allocation error",
 			CTCM_FUNTAIL, ch->id,
-			(CHANNEL_DIRECTION(ch->flags) == READ) ? "RX" : "TX");
+			(CHANNEL_DIRECTION(ch->flags) == CTCM_READ) ?
+				"RX" : "TX");
 		return -ENOMEM;
 	}
 
@@ -399,7 +400,8 @@ int ctcm_ch_alloc_buffer(struct channel 
 		CTCM_DBF_TEXT_(ERROR, CTC_DBF_ERROR,
 			"%s(%s): %s set norm_cda failed",
 			CTCM_FUNTAIL, ch->id,
-			(CHANNEL_DIRECTION(ch->flags) == READ) ? "RX" : "TX");
+			(CHANNEL_DIRECTION(ch->flags) == CTCM_READ) ?
+				"RX" : "TX");
 		return -ENOMEM;
 	}
 
@@ -603,14 +605,14 @@ static void ctcmpc_send_sweep_req(struct
 
 	priv = dev->ml_priv;
 	grp = priv->mpcg;
-	ch = priv->channel[WRITE];
+	ch = priv->channel[CTCM_WRITE];
 
 	/* sweep processing is not complete until response and request */
 	/* has completed for all read channels in group		       */
 	if (grp->in_sweep == 0) {
 		grp->in_sweep = 1;
-		grp->sweep_rsp_pend_num = grp->active_channels[READ];
-		grp->sweep_req_pend_num = grp->active_channels[READ];
+		grp->sweep_rsp_pend_num = grp->active_channels[CTCM_READ];
+		grp->sweep_req_pend_num = grp->active_channels[CTCM_READ];
 	}
 
 	sweep_skb = __dev_alloc_skb(MPC_BUFSIZE_DEFAULT, GFP_ATOMIC|GFP_DMA);
@@ -911,7 +913,7 @@ static int ctcm_tx(struct sk_buff *skb, 
 		return NETDEV_TX_BUSY;
 
 	dev->trans_start = jiffies;
-	if (ctcm_transmit_skb(priv->channel[WRITE], skb) != 0)
+	if (ctcm_transmit_skb(priv->channel[CTCM_WRITE], skb) != 0)
 		return NETDEV_TX_BUSY;
 	return NETDEV_TX_OK;
 }
@@ -994,7 +996,7 @@ static int ctcmpc_tx(struct sk_buff *skb
 	}
 
 	dev->trans_start = jiffies;
-	if (ctcmpc_transmit_skb(priv->channel[WRITE], skb) != 0) {
+	if (ctcmpc_transmit_skb(priv->channel[CTCM_WRITE], skb) != 0) {
 		CTCM_DBF_TEXT_(MPC_ERROR, CTC_DBF_ERROR,
 			"%s(%s): device error - dropped",
 					CTCM_FUNTAIL, dev->name);
@@ -1035,7 +1037,7 @@ static int ctcm_change_mtu(struct net_de
 		return -EINVAL;
 
 	priv = dev->ml_priv;
-	max_bufsize = priv->channel[READ]->max_bufsize;
+	max_bufsize = priv->channel[CTCM_READ]->max_bufsize;
 
 	if (IS_MPC(priv)) {
 		if (new_mtu > max_bufsize - TH_HEADER_LENGTH)
@@ -1226,10 +1228,10 @@ static void ctcm_irq_handler(struct ccw_
 	priv = dev_get_drvdata(&cgdev->dev);
 
 	/* Try to extract channel from driver data. */
-	if (priv->channel[READ]->cdev == cdev)
-		ch = priv->channel[READ];
-	else if (priv->channel[WRITE]->cdev == cdev)
-		ch = priv->channel[WRITE];
+	if (priv->channel[CTCM_READ]->cdev == cdev)
+		ch = priv->channel[CTCM_READ];
+	else if (priv->channel[CTCM_WRITE]->cdev == cdev)
+		ch = priv->channel[CTCM_WRITE];
 	else {
 		dev_err(&cdev->dev,
 			"%s: Internal error: Can't determine channel for "
@@ -1587,13 +1589,13 @@ static int ctcm_new_device(struct ccwgro
 		goto out_ccw2;
 	}
 
-	for (direction = READ; direction <= WRITE; direction++) {
+	for (direction = CTCM_READ; direction <= CTCM_WRITE; direction++) {
 		priv->channel[direction] =
-		    channel_get(type, direction == READ ? read_id : write_id,
-				direction);
+			channel_get(type, direction == CTCM_READ ?
+				read_id : write_id, direction);
 		if (priv->channel[direction] == NULL) {
-			if (direction == WRITE)
-				channel_free(priv->channel[READ]);
+			if (direction == CTCM_WRITE)
+				channel_free(priv->channel[CTCM_READ]);
 			goto out_dev;
 		}
 		priv->channel[direction]->netdev = dev;
@@ -1617,13 +1619,13 @@ static int ctcm_new_device(struct ccwgro
 
 	dev_info(&dev->dev,
 		"setup OK : r/w = %s/%s, protocol : %d\n",
-			priv->channel[READ]->id,
-			priv->channel[WRITE]->id, priv->protocol);
+			priv->channel[CTCM_READ]->id,
+			priv->channel[CTCM_WRITE]->id, priv->protocol);
 
 	CTCM_DBF_TEXT_(SETUP, CTC_DBF_INFO,
 		"setup(%s) OK : r/w = %s/%s, protocol : %d", dev->name,
-			priv->channel[READ]->id,
-			priv->channel[WRITE]->id, priv->protocol);
+			priv->channel[CTCM_READ]->id,
+			priv->channel[CTCM_WRITE]->id, priv->protocol);
 
 	return 0;
 out_unregister:
@@ -1635,10 +1637,10 @@ out_ccw2:
 out_ccw1:
 	ccw_device_set_offline(cgdev->cdev[0]);
 out_remove_channel2:
-	readc = channel_get(type, read_id, READ);
+	readc = channel_get(type, read_id, CTCM_READ);
 	channel_remove(readc);
 out_remove_channel1:
-	writec = channel_get(type, write_id, WRITE);
+	writec = channel_get(type, write_id, CTCM_WRITE);
 	channel_remove(writec);
 out_err_result:
 	return result;
@@ -1660,19 +1662,19 @@ static int ctcm_shutdown_device(struct c
 	if (!priv)
 		return -ENODEV;
 
-	if (priv->channel[READ]) {
-		dev = priv->channel[READ]->netdev;
+	if (priv->channel[CTCM_READ]) {
+		dev = priv->channel[CTCM_READ]->netdev;
 		CTCM_DBF_DEV(SETUP, dev, "");
 		/* Close the device */
 		ctcm_close(dev);
 		dev->flags &= ~IFF_RUNNING;
 		ctcm_remove_attributes(&cgdev->dev);
-		channel_free(priv->channel[READ]);
+		channel_free(priv->channel[CTCM_READ]);
 	} else
 		dev = NULL;
 
-	if (priv->channel[WRITE])
-		channel_free(priv->channel[WRITE]);
+	if (priv->channel[CTCM_WRITE])
+		channel_free(priv->channel[CTCM_WRITE]);
 
 	if (dev) {
 		unregister_netdev(dev);
@@ -1685,11 +1687,11 @@ static int ctcm_shutdown_device(struct c
 	ccw_device_set_offline(cgdev->cdev[1]);
 	ccw_device_set_offline(cgdev->cdev[0]);
 
-	if (priv->channel[READ])
-		channel_remove(priv->channel[READ]);
-	if (priv->channel[WRITE])
-		channel_remove(priv->channel[WRITE]);
-	priv->channel[READ] = priv->channel[WRITE] = NULL;
+	if (priv->channel[CTCM_READ])
+		channel_remove(priv->channel[CTCM_READ]);
+	if (priv->channel[CTCM_WRITE])
+		channel_remove(priv->channel[CTCM_WRITE]);
+	priv->channel[CTCM_READ] = priv->channel[CTCM_WRITE] = NULL;
 
 	return 0;
 
@@ -1720,11 +1722,11 @@ static int ctcm_pm_suspend(struct ccwgro
 
 	if (gdev->state == CCWGROUP_OFFLINE)
 		return 0;
-	netif_device_detach(priv->channel[READ]->netdev);
-	ctcm_close(priv->channel[READ]->netdev);
+	netif_device_detach(priv->channel[CTCM_READ]->netdev);
+	ctcm_close(priv->channel[CTCM_READ]->netdev);
 	if (!wait_event_timeout(priv->fsm->wait_q,
 	    fsm_getstate(priv->fsm) == DEV_STATE_STOPPED, CTCM_TIME_5_SEC)) {
-		netif_device_attach(priv->channel[READ]->netdev);
+		netif_device_attach(priv->channel[CTCM_READ]->netdev);
 		return -EBUSY;
 	}
 	ccw_device_set_offline(gdev->cdev[1]);
@@ -1745,9 +1747,9 @@ static int ctcm_pm_resume(struct ccwgrou
 	rc = ccw_device_set_online(gdev->cdev[0]);
 	if (rc)
 		goto err_out;
-	ctcm_open(priv->channel[READ]->netdev);
+	ctcm_open(priv->channel[CTCM_READ]->netdev);
 err_out:
-	netif_device_attach(priv->channel[READ]->netdev);
+	netif_device_attach(priv->channel[CTCM_READ]->netdev);
 	return rc;
 }
 
diff -urpN linux-2.6/drivers/s390/net/ctcm_main.h linux-2.6-patched/drivers/s390/net/ctcm_main.h
--- linux-2.6/drivers/s390/net/ctcm_main.h	2010-08-02 00:11:14.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.h	2010-08-12 13:29:50.000000000 +0200
@@ -111,8 +111,8 @@ enum ctcm_channel_types {
 
 #define CTCM_INITIAL_BLOCKLEN	2
 
-#define READ			0
-#define WRITE			1
+#define CTCM_READ		0
+#define CTCM_WRITE		1
 
 #define CTCM_ID_SIZE		20+3
 
diff -urpN linux-2.6/drivers/s390/net/ctcm_mpc.c linux-2.6-patched/drivers/s390/net/ctcm_mpc.c
--- linux-2.6/drivers/s390/net/ctcm_mpc.c	2010-08-02 00:11:14.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_mpc.c	2010-08-12 13:29:50.000000000 +0200
@@ -419,8 +419,8 @@ void ctc_mpc_establish_connectivity(int 
 		return;
 	priv = dev->ml_priv;
 	grp = priv->mpcg;
-	rch = priv->channel[READ];
-	wch = priv->channel[WRITE];
+	rch = priv->channel[CTCM_READ];
+	wch = priv->channel[CTCM_WRITE];
 
 	CTCM_DBF_TEXT_(MPC_SETUP, CTC_DBF_INFO,
 			"%s(%s): state=%s",
@@ -578,7 +578,7 @@ void ctc_mpc_flow_control(int port_num, 
 			"%s: %s: flowc = %d",
 				CTCM_FUNTAIL, dev->name, flowc);
 
-	rch = priv->channel[READ];
+	rch = priv->channel[CTCM_READ];
 
 	mpcg_state = fsm_getstate(grp->fsm);
 	switch (flowc) {
@@ -622,7 +622,7 @@ static void mpc_rcvd_sweep_resp(struct m
 	struct net_device *dev = rch->netdev;
 	struct ctcm_priv   *priv = dev->ml_priv;
 	struct mpc_group  *grp = priv->mpcg;
-	struct channel	  *ch = priv->channel[WRITE];
+	struct channel	  *ch = priv->channel[CTCM_WRITE];
 
 	CTCM_PR_DEBUG("%s: ch=0x%p id=%s\n", __func__, ch, ch->id);
 	CTCM_D3_DUMP((char *)mpcginfo->sweep, TH_SWEEP_LENGTH);
@@ -656,7 +656,7 @@ static void ctcmpc_send_sweep_resp(struc
 	int rc = 0;
 	struct th_sweep *header;
 	struct sk_buff *sweep_skb;
-	struct channel *ch  = priv->channel[WRITE];
+	struct channel *ch  = priv->channel[CTCM_WRITE];
 
 	CTCM_PR_DEBUG("%s: ch=0x%p id=%s\n", __func__, rch, rch->id);
 
@@ -712,7 +712,7 @@ static void mpc_rcvd_sweep_req(struct mp
 	struct net_device *dev     = rch->netdev;
 	struct ctcm_priv  *priv = dev->ml_priv;
 	struct mpc_group  *grp  = priv->mpcg;
-	struct channel	  *ch	   = priv->channel[WRITE];
+	struct channel	  *ch	   = priv->channel[CTCM_WRITE];
 
 	if (do_debug)
 		CTCM_DBF_TEXT_(MPC_TRACE, CTC_DBF_DEBUG,
@@ -721,8 +721,8 @@ static void mpc_rcvd_sweep_req(struct mp
 	if (grp->in_sweep == 0) {
 		grp->in_sweep = 1;
 		ctcm_test_and_set_busy(dev);
-		grp->sweep_req_pend_num = grp->active_channels[READ];
-		grp->sweep_rsp_pend_num = grp->active_channels[READ];
+		grp->sweep_req_pend_num = grp->active_channels[CTCM_READ];
+		grp->sweep_rsp_pend_num = grp->active_channels[CTCM_READ];
 	}
 
 	CTCM_D3_DUMP((char *)mpcginfo->sweep, TH_SWEEP_LENGTH);
@@ -906,14 +906,14 @@ void mpc_group_ready(unsigned long adev)
 	fsm_newstate(grp->fsm, MPCG_STATE_READY);
 
 	/* Put up a read on the channel */
-	ch = priv->channel[READ];
+	ch = priv->channel[CTCM_READ];
 	ch->pdu_seq = 0;
 	CTCM_PR_DBGDATA("ctcmpc: %s() ToDCM_pdu_seq= %08x\n" ,
 			__func__, ch->pdu_seq);
 
 	ctcmpc_chx_rxidle(ch->fsm, CTC_EVENT_START, ch);
 	/* Put the write channel in idle state */
-	ch = priv->channel[WRITE];
+	ch = priv->channel[CTCM_WRITE];
 	if (ch->collect_len > 0) {
 		spin_lock(&ch->collect_lock);
 		ctcm_purge_skb_queue(&ch->collect_queue);
@@ -960,7 +960,8 @@ void mpc_channel_action(struct channel *
 		"%s: %i / Grp:%s total_channels=%i, active_channels: "
 		"read=%i, write=%i\n", __func__, action,
 		fsm_getstate_str(grp->fsm), grp->num_channel_paths,
-		grp->active_channels[READ], grp->active_channels[WRITE]);
+		grp->active_channels[CTCM_READ],
+		grp->active_channels[CTCM_WRITE]);
 
 	if ((action == MPC_CHANNEL_ADD) && (ch->in_mpcgroup == 0)) {
 		grp->num_channel_paths++;
@@ -994,10 +995,11 @@ void mpc_channel_action(struct channel *
 				grp->xid_skb->data,
 				grp->xid_skb->len);
 
-		ch->xid->xid2_dlc_type = ((CHANNEL_DIRECTION(ch->flags) == READ)
+		ch->xid->xid2_dlc_type =
+			((CHANNEL_DIRECTION(ch->flags) == CTCM_READ)
 				? XID2_READ_SIDE : XID2_WRITE_SIDE);
 
-		if (CHANNEL_DIRECTION(ch->flags) == WRITE)
+		if (CHANNEL_DIRECTION(ch->flags) == CTCM_WRITE)
 			ch->xid->xid2_buf_len = 0x00;
 
 		ch->xid_skb->data = ch->xid_skb_data;
@@ -1006,8 +1008,8 @@ void mpc_channel_action(struct channel *
 
 		fsm_newstate(ch->fsm, CH_XID0_PENDING);
 
-		if ((grp->active_channels[READ]  > 0) &&
-		    (grp->active_channels[WRITE] > 0) &&
+		if ((grp->active_channels[CTCM_READ] > 0) &&
+		    (grp->active_channels[CTCM_WRITE] > 0) &&
 			(fsm_getstate(grp->fsm) < MPCG_STATE_XID2INITW)) {
 			fsm_newstate(grp->fsm, MPCG_STATE_XID2INITW);
 			CTCM_DBF_TEXT_(MPC_SETUP, CTC_DBF_NOTICE,
@@ -1027,10 +1029,10 @@ void mpc_channel_action(struct channel *
 		if (grp->channels_terminating)
 					goto done;
 
-		if (((grp->active_channels[READ] == 0) &&
-					(grp->active_channels[WRITE] > 0))
-			|| ((grp->active_channels[WRITE] == 0) &&
-					(grp->active_channels[READ] > 0)))
+		if (((grp->active_channels[CTCM_READ] == 0) &&
+					(grp->active_channels[CTCM_WRITE] > 0))
+			|| ((grp->active_channels[CTCM_WRITE] == 0) &&
+					(grp->active_channels[CTCM_READ] > 0)))
 			fsm_event(grp->fsm, MPCG_EVENT_INOP, dev);
 	}
 done:
@@ -1038,7 +1040,8 @@ done:
 		"exit %s: %i / Grp:%s total_channels=%i, active_channels: "
 		"read=%i, write=%i\n", __func__, action,
 		fsm_getstate_str(grp->fsm), grp->num_channel_paths,
-		grp->active_channels[READ], grp->active_channels[WRITE]);
+		grp->active_channels[CTCM_READ],
+		grp->active_channels[CTCM_WRITE]);
 
 	CTCM_PR_DEBUG("exit %s: ch=0x%p id=%s\n", __func__, ch, ch->id);
 }
@@ -1392,8 +1395,8 @@ static void mpc_action_go_inop(fsm_insta
 		(grp->port_persist == 0))
 		fsm_deltimer(&priv->restart_timer);
 
-	wch = priv->channel[WRITE];
-	rch = priv->channel[READ];
+	wch = priv->channel[CTCM_WRITE];
+	rch = priv->channel[CTCM_READ];
 
 	switch (grp->saved_state) {
 	case MPCG_STATE_RESET:
@@ -1480,8 +1483,8 @@ static void mpc_action_timeout(fsm_insta
 
 	priv = dev->ml_priv;
 	grp = priv->mpcg;
-	wch = priv->channel[WRITE];
-	rch = priv->channel[READ];
+	wch = priv->channel[CTCM_WRITE];
+	rch = priv->channel[CTCM_READ];
 
 	switch (fsm_getstate(grp->fsm)) {
 	case MPCG_STATE_XID2INITW:
@@ -1586,7 +1589,7 @@ static int mpc_validate_xid(struct mpcg_
 	CTCM_D3_DUMP((char *)xid, XID2_LENGTH);
 
 	/*the received direction should be the opposite of ours  */
-	if (((CHANNEL_DIRECTION(ch->flags) == READ) ? XID2_WRITE_SIDE :
+	if (((CHANNEL_DIRECTION(ch->flags) == CTCM_READ) ? XID2_WRITE_SIDE :
 				XID2_READ_SIDE) != xid->xid2_dlc_type) {
 		rc = 2;
 		/* XID REJECTED: r/w channel pairing mismatch */
@@ -1912,7 +1915,7 @@ static void mpc_action_doxid7(fsm_instan
 	if (grp == NULL)
 		return;
 
-	for (direction = READ; direction <= WRITE; direction++)	{
+	for (direction = CTCM_READ; direction <= CTCM_WRITE; direction++) {
 		struct channel *ch = priv->channel[direction];
 		struct xid2 *thisxid = ch->xid;
 		ch->xid_skb->data = ch->xid_skb_data;
@@ -2152,14 +2155,15 @@ static int mpc_send_qllc_discontact(stru
 			return -ENOMEM;
 		}
 
-		*((__u32 *)skb_push(skb, 4)) = priv->channel[READ]->pdu_seq;
-		priv->channel[READ]->pdu_seq++;
+		*((__u32 *)skb_push(skb, 4)) =
+			priv->channel[CTCM_READ]->pdu_seq;
+		priv->channel[CTCM_READ]->pdu_seq++;
 		CTCM_PR_DBGDATA("ctcmpc: %s ToDCM_pdu_seq= %08x\n",
-				__func__, priv->channel[READ]->pdu_seq);
+				__func__, priv->channel[CTCM_READ]->pdu_seq);
 
 		/* receipt of CC03 resets anticipated sequence number on
 		      receiving side */
-		priv->channel[READ]->pdu_seq = 0x00;
+		priv->channel[CTCM_READ]->pdu_seq = 0x00;
 		skb_reset_mac_header(skb);
 		skb->dev = dev;
 		skb->protocol = htons(ETH_P_SNAP);
diff -urpN linux-2.6/drivers/s390/net/ctcm_sysfs.c linux-2.6-patched/drivers/s390/net/ctcm_sysfs.c
--- linux-2.6/drivers/s390/net/ctcm_sysfs.c	2010-08-02 00:11:14.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_sysfs.c	2010-08-12 13:29:50.000000000 +0200
@@ -38,8 +38,8 @@ static ssize_t ctcm_buffer_write(struct 
 	int bs1;
 	struct ctcm_priv *priv = dev_get_drvdata(dev);
 
-	if (!(priv && priv->channel[READ] &&
-			(ndev = priv->channel[READ]->netdev))) {
+	ndev = priv->channel[CTCM_READ]->netdev;
+	if (!(priv && priv->channel[CTCM_READ] && ndev)) {
 		CTCM_DBF_TEXT(SETUP, CTC_DBF_ERROR, "bfnondev");
 		return -ENODEV;
 	}
@@ -55,12 +55,12 @@ static ssize_t ctcm_buffer_write(struct 
 	    (bs1 < (ndev->mtu + LL_HEADER_LENGTH + 2)))
 					goto einval;
 
-	priv->channel[READ]->max_bufsize = bs1;
-	priv->channel[WRITE]->max_bufsize = bs1;
+	priv->channel[CTCM_READ]->max_bufsize = bs1;
+	priv->channel[CTCM_WRITE]->max_bufsize = bs1;
 	if (!(ndev->flags & IFF_RUNNING))
 		ndev->mtu = bs1 - LL_HEADER_LENGTH - 2;
-	priv->channel[READ]->flags |= CHANNEL_FLAGS_BUFSIZE_CHANGED;
-	priv->channel[WRITE]->flags |= CHANNEL_FLAGS_BUFSIZE_CHANGED;
+	priv->channel[CTCM_READ]->flags |= CHANNEL_FLAGS_BUFSIZE_CHANGED;
+	priv->channel[CTCM_WRITE]->flags |= CHANNEL_FLAGS_BUFSIZE_CHANGED;
 
 	CTCM_DBF_DEV(SETUP, ndev, buf);
 	return count;
@@ -85,9 +85,9 @@ static void ctcm_print_statistics(struct
 	p += sprintf(p, "  Device FSM state: %s\n",
 		     fsm_getstate_str(priv->fsm));
 	p += sprintf(p, "  RX channel FSM state: %s\n",
-		     fsm_getstate_str(priv->channel[READ]->fsm));
+		     fsm_getstate_str(priv->channel[CTCM_READ]->fsm));
 	p += sprintf(p, "  TX channel FSM state: %s\n",
-		     fsm_getstate_str(priv->channel[WRITE]->fsm));
+		     fsm_getstate_str(priv->channel[CTCM_WRITE]->fsm));
 	p += sprintf(p, "  Max. TX buffer used: %ld\n",
 		     priv->channel[WRITE]->prof.maxmulti);
 	p += sprintf(p, "  Max. chained SKBs: %ld\n",
@@ -102,7 +102,7 @@ static void ctcm_print_statistics(struct
 		     priv->channel[WRITE]->prof.tx_time);
 
 	printk(KERN_INFO "Statistics for %s:\n%s",
-				priv->channel[WRITE]->netdev->name, sbuf);
+				priv->channel[CTCM_WRITE]->netdev->name, sbuf);
 	kfree(sbuf);
 	return;
 }
@@ -125,7 +125,7 @@ static ssize_t stats_write(struct device
 		return -ENODEV;
 	/* Reset statistics */
 	memset(&priv->channel[WRITE]->prof, 0,
-				sizeof(priv->channel[WRITE]->prof));
+				sizeof(priv->channel[CTCM_WRITE]->prof));
 	return count;
 }
 


^ permalink raw reply

* Re: [PATCH net-next-2.6] bridge: 64bit rx/tx counters
From: Eric Dumazet @ 2010-08-12 12:16 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Miller, Stephen Hemminger, netdev, bhutchings, Nick Piggin
In-Reply-To: <20100809214740.c5d186d2.akpm@linux-foundation.org>

Le lundi 09 août 2010 à 21:47 -0700, Andrew Morton a écrit :

> Oh for fuck's sake.  Will you guys just stop adding generic kernel
> infrastructure behind everyone's backs?
> 
> Had I actually been aware that this stuff was going into the tree I'd
> have pointed out that the u64_stats_* api needs renaming. 
> s/stats/counter/ because it has no business assuming that the counter
> is being used for statistics.
> 
> 

Sure. Someone suggested to change the name, considering values could
also be signed (s64 instead of u64_...)

> And all this open-coded per-cpu counter stuff added all over the place.
> Were percpu_counters tested or reviewed and found inadequate and unfixable?
> If so, please do tell.
> 

percpu_counters tries hard to maintain a view of the current value of
the (global) counter. This adds a cost because of a shared cache line
and locking. (__percpu_counter_sum() is not very scalable on big hosts,
it locks the percpu_counter lock for a possibly long iteration)


For network stats we dont want to maintain this central value, we do the
folding only when necessary. And this folding has zero effect on
concurrent writers (counter updates)

For network stack, we also need to update two values, a packet counter
and a bytes counter. percpu_counter is not very good for the 'bytes
counter', since we would have to use a arbitrary big bias value.
Using several percpu_counter would also probably use more cache lines.

Also please note this stuff is only needed for 32bit arches. 

Using percpu_counter would slow down network stack on modern arches.


I am very well aware of the percpu_counter stuff, I believe I tried to
optimize it a bit in the past.

Thanks



^ permalink raw reply

* Re: [patch net-next-2.6] bridge: allow hub-like behaviour
From: Stephen Hemminger @ 2010-08-12 12:35 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <20100812111523.GC3115@psychotron.brq.redhat.com>

On Thu, 12 Aug 2010 13:15:24 +0200
Jiri Pirko <jpirko@redhat.com> wrote:

> Wed, Aug 11, 2010 at 06:07:41PM CEST, shemminger@vyatta.com wrote:
> >NO. unnecessary
> >
> >Already possible, without patch. Just set the hold time
> >on the bridge to 0!
> 
> Sorry, but could you please point me a sysfs node where should I do this
> setting? Unable to find this.
> 
> Thanks.
> 

/sys/class/net/ethX/brport/hold_timer

^ permalink raw reply

* Re: [patch net-next-2.6] bridge: allow hub-like behaviour
From: Jiri Pirko @ 2010-08-12 12:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <20100812083541.33d8759a@s6510>

Thu, Aug 12, 2010 at 02:35:41PM CEST, shemminger@vyatta.com wrote:
>On Thu, 12 Aug 2010 13:15:24 +0200
>Jiri Pirko <jpirko@redhat.com> wrote:
>
>> Wed, Aug 11, 2010 at 06:07:41PM CEST, shemminger@vyatta.com wrote:
>> >NO. unnecessary
>> >
>> >Already possible, without patch. Just set the hold time
>> >on the bridge to 0!
>> 
>> Sorry, but could you please point me a sysfs node where should I do this
>> setting? Unable to find this.
>> 
>> Thanks.
>> 
>
>/sys/class/net/ethX/brport/hold_timer

That's read-only:

static BRPORT_ATTR(hold_timer, S_IRUGO, show_hold_timer, NULL);


^ permalink raw reply

* [xfrm_user] BUG: sleeping function called from invalid context
From: Luca Tettamanti @ 2010-08-12 12:50 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel

Hello,
I just noticed this message in my log (I'm not sure what charon was
doing at the time...):

BUG: sleeping function called from invalid context at /home/kronos/src/linux-2.6.git/mm/slub.c:1701
in_atomic(): 1, irqs_disabled(): 0, pid: 10411, name: charon
Pid: 10411, comm: charon Not tainted 2.6.35 #271
Call Trace:
 [<ffffffff810275d7>] __might_sleep+0xf8/0xfa
 [<ffffffff810a99be>] kmem_cache_alloc+0x35/0x9d
 [<ffffffff81265c8e>] xfrm_policy_alloc+0x20/0xa1
 [<ffffffffa0372d73>] xfrm_compile_policy+0x112/0x16f [xfrm_user]
 [<ffffffff812686e4>] xfrm_user_policy+0xb8/0x131
 [<ffffffff81233b5b>] do_ip_setsockopt+0xa83/0xb2e
 [<ffffffff8102c0ce>] ? get_parent_ip+0x11/0x41
 [<ffffffff81080838>] ? unlock_page+0x22/0x27
 [<ffffffff810935c3>] ? __do_fault+0x4d4/0x50c
 [<ffffffff8109497d>] ? handle_mm_fault+0x45d/0x8a4
 [<ffffffff810afbef>] ? get_empty_filp+0x79/0x15a
 [<ffffffff8102c0e5>] ? get_parent_ip+0x28/0x41
 [<ffffffff8102c0e5>] ? get_parent_ip+0x28/0x41
 [<ffffffff81283385>] ? sub_preempt_count+0x92/0xa6
 [<ffffffff812832b6>] ? do_page_fault+0x3d5/0x412
 [<ffffffff8125522e>] ? inet_bind+0x1cc/0x1dc
 [<ffffffff81233cc2>] ip_setsockopt+0x23/0x82
 [<ffffffff810acfbf>] ? fd_install+0x54/0x5d
 [<ffffffff8124d175>] udp_setsockopt+0x24/0x26
 [<ffffffff811fd5c5>] sock_common_setsockopt+0xf/0x11
 [<ffffffff811fb0a8>] sys_setsockopt+0x81/0xa2
 [<ffffffff8100296b>] system_call_fastpath+0x16/0x1b


xfrm_user_policy takes read_lock(&xfrm_km_lock) before calling
xfrm_compile_policy (via km->compile_policy), which in turn calls
xfrm_policy_alloc with GFP_KERNEL.

Luca

^ permalink raw reply

* Re: [patch net-next-2.6] bridge: allow hub-like behaviour
From: Stephen Hemminger @ 2010-08-12 12:55 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <20100812124847.GD3115@psychotron.brq.redhat.com>

On Thu, 12 Aug 2010 14:48:48 +0200
Jiri Pirko <jpirko@redhat.com> wrote:

> Thu, Aug 12, 2010 at 02:35:41PM CEST, shemminger@vyatta.com wrote:
> >On Thu, 12 Aug 2010 13:15:24 +0200
> >Jiri Pirko <jpirko@redhat.com> wrote:
> >
> >> Wed, Aug 11, 2010 at 06:07:41PM CEST, shemminger@vyatta.com wrote:
> >> >NO. unnecessary
> >> >
> >> >Already possible, without patch. Just set the hold time
> >> >on the bridge to 0!
> >> 
> >> Sorry, but could you please point me a sysfs node where should I do this
> >> setting? Unable to find this.
> >> 
> >> Thanks.
> >> 
> >
> >/sys/class/net/ethX/brport/hold_timer
> 
> That's read-only:
> 
> static BRPORT_ATTR(hold_timer, S_IRUGO, show_hold_timer, NULL);
> 

Guess you have to use ioctl() interface to change it for now.
Looks like a missed opportunity when sysfs support was added.
There are some other values like forward_delay that have same problem.

^ permalink raw reply

* Re: [patch net-next-2.6] bridge: allow hub-like behaviour
From: Stephen Hemminger @ 2010-08-12 13:02 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <20100812124847.GD3115@psychotron.brq.redhat.com>

On Thu, 12 Aug 2010 14:48:48 +0200
Jiri Pirko <jpirko@redhat.com> wrote:

> Thu, Aug 12, 2010 at 02:35:41PM CEST, shemminger@vyatta.com wrote:
> >On Thu, 12 Aug 2010 13:15:24 +0200
> >Jiri Pirko <jpirko@redhat.com> wrote:
> >
> >> Wed, Aug 11, 2010 at 06:07:41PM CEST, shemminger@vyatta.com wrote:
> >> >NO. unnecessary
> >> >
> >> >Already possible, without patch. Just set the hold time
> >> >on the bridge to 0!
> >> 
> >> Sorry, but could you please point me a sysfs node where should I do this
> >> setting? Unable to find this.
> >> 
> >> Thanks.
> >> 
> >
> >/sys/class/net/ethX/brport/hold_timer

Too early...

hold_time is computed from ageing time, so that is the value
you need to set:

$ ls -l /sys/class/net/br0/bridge/ageing_time 
-rw-r--r-- 1 root root 4096 2010-08-12 08:34 /sys/class/net/br0/bridge/ageing_time

^ permalink raw reply

* Re: [xfrm_user] BUG: sleeping function called from invalid context
From: Alex Badea @ 2010-08-12 13:13 UTC (permalink / raw)
  To: Luca Tettamanti; +Cc: netdev, linux-kernel
In-Reply-To: <20100812125027.GA12982@nb-core2.darkstar.lan>

Hi,

On 08/12/2010 03:50 PM, Luca Tettamanti wrote:
>  [<ffffffff8124d175>] udp_setsockopt+0x24/0x26
>  [<ffffffff811fd5c5>] sock_common_setsockopt+0xf/0x11
>  [<ffffffff811fb0a8>] sys_setsockopt+0x81/0xa2
>  [<ffffffff8100296b>] system_call_fastpath+0x16/0x1b
> 
> 
> xfrm_user_policy takes read_lock(&xfrm_km_lock) before calling
> xfrm_compile_policy (via km->compile_policy), which in turn calls
> xfrm_policy_alloc with GFP_KERNEL.

Since it's a setsockopt(), I suspect it was setting up per-socket bypass
policies.

Regards,
Alex

^ permalink raw reply

* Re: [patch net-next-2.6] bridge: allow hub-like behaviour
From: Jiri Pirko @ 2010-08-12 13:20 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <20100812085520.4f56ebe6@s6510>

Thu, Aug 12, 2010 at 02:55:20PM CEST, shemminger@vyatta.com wrote:
>On Thu, 12 Aug 2010 14:48:48 +0200
>Jiri Pirko <jpirko@redhat.com> wrote:
>
>> Thu, Aug 12, 2010 at 02:35:41PM CEST, shemminger@vyatta.com wrote:
>> >On Thu, 12 Aug 2010 13:15:24 +0200
>> >Jiri Pirko <jpirko@redhat.com> wrote:
>> >
>> >> Wed, Aug 11, 2010 at 06:07:41PM CEST, shemminger@vyatta.com wrote:
>> >> >NO. unnecessary
>> >> >
>> >> >Already possible, without patch. Just set the hold time
>> >> >on the bridge to 0!
>> >> 
>> >> Sorry, but could you please point me a sysfs node where should I do this
>> >> setting? Unable to find this.
>> >> 
>> >> Thanks.
>> >> 
>> >
>> >/sys/class/net/ethX/brport/hold_timer
>> 
>> That's read-only:
>> 
>> static BRPORT_ATTR(hold_timer, S_IRUGO, show_hold_timer, NULL);
>> 
>
>Guess you have to use ioctl() interface to change it for now.
>Looks like a missed opportunity when sysfs support was added.
>There are some other values like forward_delay that have same problem.


Oh you mislead me. It's not hold_timer but hold_time function:

static inline unsigned long hold_time(const struct net_bridge *br)
{
        return br->topology_change ? br->forward_delay : br->ageing_time;
}

called from br_fdb_update().

^ permalink raw reply

* Re: [patch net-next-2.6] bridge: allow hub-like behaviour
From: Jiri Pirko @ 2010-08-12 13:40 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <20100812090251.0f47c5e3@s6510>

Thu, Aug 12, 2010 at 03:02:51PM CEST, shemminger@vyatta.com wrote:
>On Thu, 12 Aug 2010 14:48:48 +0200
>Jiri Pirko <jpirko@redhat.com> wrote:
>
>> Thu, Aug 12, 2010 at 02:35:41PM CEST, shemminger@vyatta.com wrote:
>> >On Thu, 12 Aug 2010 13:15:24 +0200
>> >Jiri Pirko <jpirko@redhat.com> wrote:
>> >
>> >> Wed, Aug 11, 2010 at 06:07:41PM CEST, shemminger@vyatta.com wrote:
>> >> >NO. unnecessary
>> >> >
>> >> >Already possible, without patch. Just set the hold time
>> >> >on the bridge to 0!
>> >> 
>> >> Sorry, but could you please point me a sysfs node where should I do this
>> >> setting? Unable to find this.
>> >> 
>> >> Thanks.
>> >> 
>> >
>> >/sys/class/net/ethX/brport/hold_timer
>
>Too early...
>
>hold_time is computed from ageing time, so that is the value
>you need to set:
>
>$ ls -l /sys/class/net/br0/bridge/ageing_time 
>-rw-r--r-- 1 root root 4096 2010-08-12 08:34 /sys/class/net/br0/bridge/ageing_time

Yep, figured that out. Works as expected. Thanks Stephen.

^ permalink raw reply

* Re: [Bugme-new] [Bug 16568] New: Regression and incompatibility with Windows SP2-SP3-Vista TCP stack causing lost connections
From: Andrew Morton @ 2010-08-12 14:40 UTC (permalink / raw)
  To: netdev; +Cc: bugzilla-daemon, bugme-daemon, yuriy
In-Reply-To: <bug-16568-10286@https.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).


On Thu, 12 Aug 2010 08:20:01 GMT bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=16568
> 
>            Summary: Regression and incompatibility with Windows
>                     SP2-SP3-Vista TCP stack causing lost connections
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 2.6.30+
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: high
>           Priority: P1
>          Component: IPV4
>         AssignedTo: shemminger@linux-foundation.org
>         ReportedBy: yuriy@ucoz.com
>         Regression: No
> 
> 
> Hi.
> I administer about 50 highly-loaded web servers (free CMS hosting) under linux.
> Having on most of them kernel versions between 2.6.24 and 2.6.29 at the
> beginnig of the year, I made TCP sysctls tunings for increasing DDOS and
> different flooding protection (our servers have attacks rather often).
> tcp_tw_recyle=1 was among of them, as many manuals in the net recommend to do
> this and linux documentation does not say anything bad. Having periodic kernel
> panics connected with bugs in ethernet card drivers and ext3 and after founding
> that 2.6.31+ kernels work faster with ext3, I upgraded almost all kernels to
> 2.6.32.8, which was already being tested on several servers for several months. 
> Somewhen after that we began to receive complaints from our users (site owners)
> that they (and their visitors) see very unstable work of their sites. It looked
> like HTTP-connections were just lost in a random way. Not everybody had the
> problem, just a small percent. We tried to find problem with internet providers
> or buggy firewalls, but finally came to conclusion that problem is connected
> with our servers. Analizing situations with lost connections using tcpdump i
> found that client host send packets, BUT LINUX JUST IGNORES THEM, there was
> SYN-packet repeated 3 times with interval of 3 secs, but NO SYN-ACK reply.
> Most problems had users with Windows SP3 (i.e. almost all users with SP3 had
> the problem). I booted one server with old 2.6.24 kernel and found that problem
> dissappeared. Then began look for exact kernel version, that introduced
> incompatibility. Using binary search I compiled several kernels between 2.6.24
> and 2.6.32.8 and found that 2.6.29.6 DO NO have the problem, but 2.6.30 DOES.
> Studing commits made to tcp_input.c and tcp_ipv4.c (which i supposed were
> involved) between that releases I found this one.
>   author    Eric Dumazet <dada1@cosmosbay.com>    
>     Wed, 11 Mar 2009 16:23:57 +0000 (09:23 -0700)
>   committer    David S. Miller <davem@davemloft.net>    
>     Wed, 11 Mar 2009 16:23:57 +0000 (09:23 -0700)
>   commit    fc1ad92dfc4e363a055053746552cdb445ba5c57
> 
>   tcp: allow timestamps even if SYN packet has tsval=0
> 
>   Some systems send SYN packets with apparently wrong RFC1323 timestamp
>   option values [timestamp tsval=0 tsecr=0].
>   It might be for security reasons (http://www.secuobs.com/plugs/25220.shtml )
>   Linux TCP stack ignores this option and sends back a SYN+ACK packet
>   without timestamp option, thus many TCP flows cannot use timestamps
>   and lose some benefit of RFC1323.
>   Other operating systems seem to not care about initial tsval value, and let
>   tcp flows to negotiate timestamp option.
> 
>   net/ipv4/tcp_ipv4.c         diff :
> 
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1226,15 +1226,6 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff
> *skb)
>         if (want_cookie && !tmp_opt.saw_tstamp)
>                 tcp_clear_options(&tmp_opt);
> 
> -       if (tmp_opt.saw_tstamp && !tmp_opt.rcv_tsval) {
> -               /* Some OSes (unknown ones, but I see them on web server, which
> -                * contains information interesting only for windows'
> -                * users) do not send their stamp in SYN. It is easy case.
> -                * We simply do not advertise TS support.
> -                */
> -               tmp_opt.saw_tstamp = 0;
> -               tmp_opt.tstamp_ok  = 0;
> -       }
>         tmp_opt.tstamp_ok = tmp_opt.saw_tstamp;
> 
>         tcp_openreq_init(req, &tmp_opt, skb);
> 
> Removing that was not very good. Having analized lost connections from SP3 I
> know that they have timestamps turned on and timestamp value is 0. Here is it:
> 13:39:10.430498 IP 192.168.99.130.3493 > 192.168.99.100.80: S
> 2507911465:2507911465(0) win 65535 <mss 1460,nop,wscale 3,nop,nop,timestamp 0
> 0,nop,nop,sackOK>
>         0x0000:  4500 0040 2bda 4000 8006 86a6 c0a8 6382  E..@+.@.......c.
>         0x0010:  c0a8 6364 0da5 0050 957b b129 0000 0000  ..cd...P.{.)....
>         0x0020:  b002 ffff 992c 0000 0204 05b4 0103 0303  .....,..........
>         0x0030:  0101 080a 0000 0000 0000 0000 0101 0402  ................
> 
> Having above code fragment removed we got tmp_opt.tstamp_ok=1, as i understand.
> But a little later in source code of tcp_ipv4.c read:
>         /* VJ's idea. We save last timestamp seen
>          * from the destination in peer table, when entering
>          * state TIME-WAIT, and check against it before
>          * accepting new connection request.
>          *
>          * If "isn" is not zero, this request hit alive
>          * timewait bucket, so that all the necessary checks
>          * are made in the function processing timewait state.
>          */
>         if (tmp_opt.saw_tstamp &&
>             tcp_death_row.sysctl_tw_recycle &&
>             (dst = inet_csk_route_req(sk, req)) != NULL &&
>             (peer = rt_get_peer((struct rtable *)dst)) != NULL &&
>             peer->v4daddr == saddr) {
>             if ((u32)get_seconds() - peer->tcp_ts_stamp < TCP_PAWS_MSL &&
>                 (s32)(peer->tcp_ts - req->ts_recent) >
>                             TCP_PAWS_WINDOW) {
>                 NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_PAWSPASSIVEREJECTED);
>                 goto drop_and_release;
>             }
>         }
> which in some way (tmp_opt.saw_tstamp && tcp_death_row.sysctl_tw_recycle are
> true), random way, having not closed time-wait sockets from the pear, leads to
> packet ignorence.
> 
> As for me, i understand, that i should not enable tw_recycle, BUT DOCUMENTATION
> DOES NOT STATE, that enabling it i'll got random and rather often lost of
> connections from some types of popular clients (like Windows).
> Concerning above stated commit, it should include something to prevent above
> condition to become true if tmp_opt.rcv_tsval==0. I'm not sure, but something
> like
>         if (tmp_opt.saw_tstamp &&
> +           tmp_opt.rcv_tsval &&
>             tcp_death_row.sysctl_tw_recycle &&
>             (dst = inet_csk_route_req(sk, req)) != NULL &&
>             (peer = rt_get_peer((struct rtable *)dst)) != NULL &&
> 
> just to not provide regression and strong TCP-stack incompatibility in case
> tw_recycle is enabled.
> Also documentation does not state, that tw_recyle should not be used at all for
> internet servers, because web-clients, which are behind NAT, will have problems
> connected with the same above condition because successive connections from
> different clients (which have common IP) could have incompatible timestamps.
> 
> Sorry if i detracted somebody busy from his work with my unimportant problem.
> 


^ permalink raw reply

* Re: [PATCH 3/6 v2] au1000-eth: fix all "line over 80 characters warnings"
From: Joe Perches @ 2010-08-12 14:48 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, David Miller
In-Reply-To: <201008121009.59133.florian@openwrt.org>

On Thu, 2010-08-12 at 10:09 +0200, Florian Fainelli wrote:
> --- a/drivers/net/au1000_eth.c
> +++ b/drivers/net/au1000_eth.c
> @@ -377,12 +377,15 @@ static int au1000_mii_probe(struct net_device *dev)
> -		for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++)
> +		/* find the first (lowest address) PHY
> +		 * on the current MAC's MII bus */
> +		for (phy_addr = 0;
> +			phy_addr < PHY_MAX_ADDR; phy_addr++)

Breaking up the for is not an improvement, it's already < 80

> @ -390,22 +393,26 @@ static int au1000_mii_probe(struct net_device *dev)
>  			if (!phydev && (aup->mac_id == 1)) {
>  				/* no PHY found, maybe we have a dual PHY? */
>  				dev_info(&dev->dev, ": no PHY found on MAC1, "
> -					"let's see if it's attached to MAC0...\n");
> +					"let's see if it's attached to MAC0..."
> +					"\n");

Definitely not an improvement.

>  
> -				/* find the first (lowest address) non-attached PHY on
> -				 * the MAC0 MII bus */
> -				for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++) {
> +				/* find the first (lowest address) non-attached
> +				 * PHY on the MAC0 MII bus */
> +				for (phy_addr = 0; phy_addr < PHY_MAX_ADDR;
> +								phy_addr++) {
>  					struct phy_device *const tmp_phydev =
> -							aup->mii_bus->phy_map[phy_addr];
> +						aup->mii_bus->phy_map[phy_addr];
>  
>  					if (aup->mac_id == 1)
>  						break;

Can you find a way to reduce the indentation level instead?

> @@ -709,7 +719,8 @@ static int au1000_rx(struct net_device *dev)
>  			frmlen -= 4; /* Remove FCS */
>  			skb = dev_alloc_skb(frmlen + 2);
>  			if (skb == NULL) {
> -				netdev_err(dev, "Memory squeeze, dropping packet.\n");
> +				netdev_err(dev, "Memory squeeze,"
> +							" dropping packet.\n");

Not an improvement. etc...

> +	netif_dbg(aup, drv, dev, "au1000_multicast_list: flags=%x\n",
> +							dev->flags);
[]
> +		dev_err(&pdev->dev, "failed to request memory region for"
> +						" base registers\n");
[]
> +		dev_err(&pdev->dev, "failed to request memory region for"
> +						" MAC enable register\n");
[]
>  		dev_err(&pdev->dev, "MAC0-associated PHY attached 2nd MACs MII"
> -				"bus not supported yet\n");
> +				" bus not supported yet\n");



^ permalink raw reply

* [PATCH 1/2] s2io: remove unused code
From: Jon Mason @ 2010-08-12 15:03 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Ramkrishna Vepa, Sivakumar Subramani, Sreenivasa Honnur

usr_addrs is never referenced outside the initial struct population
during open, thus making it unnecessary.  Remove it, the code that
references it, and the struct that it is the only user of.

Signed-off-by: Jon Mason <jon.mason@exar.com>
---
 drivers/net/s2io.c |    2 --
 drivers/net/s2io.h |    8 --------
 2 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 18bc5b7..0124f12 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -5124,8 +5124,6 @@ static void s2io_set_multicast(struct net_device *dev)
 		/* Create the new Rx filter list and update the same in H/W. */
 		i = 0;
 		netdev_for_each_mc_addr(ha, dev) {
-			memcpy(sp->usr_addrs[i].addr, ha->addr,
-			       ETH_ALEN);
 			mac_addr = 0;
 			for (j = 0; j < ETH_ALEN; j++) {
 				mac_addr |= ha->addr[j];
diff --git a/drivers/net/s2io.h b/drivers/net/s2io.h
index 0af0335..924f618 100644
--- a/drivers/net/s2io.h
+++ b/drivers/net/s2io.h
@@ -816,12 +816,6 @@ struct mac_info {
 	struct stat_block *stats_info;	/* Logical address of the stat block */
 };
 
-/* structure representing the user defined MAC addresses */
-struct usr_addr {
-	char addr[ETH_ALEN];
-	int usage_cnt;
-};
-
 /* Default Tunable parameters of the NIC. */
 #define DEFAULT_FIFO_0_LEN 4096
 #define DEFAULT_FIFO_1_7_LEN 512
@@ -894,9 +888,7 @@ struct s2io_nic {
 #define ALL_MULTI   2
 
 #define MAX_ADDRS_SUPPORTED 64
-	u16 usr_addr_count;
 	u16 mc_addr_count;
-	struct usr_addr usr_addrs[256];
 
 	u16 m_cast_flg;
 	u16 all_multi_pos;
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/2] s2io: Update driver version
From: Jon Mason @ 2010-08-12 15:03 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Ramkrishna Vepa, Sivakumar Subramani, Sreenivasa Honnur

Update s2io driver's version to reflect the series of patches.

Signed-off-by: Jon Mason <jon.mason@exar.com>
---
 drivers/net/s2io.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 0124f12..5f062dd 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -90,7 +90,7 @@
 #include "s2io.h"
 #include "s2io-regs.h"
 
-#define DRV_VERSION "2.0.26.26"
+#define DRV_VERSION "2.0.26.27"
 
 /* S2io Driver name & version. */
 static char s2io_driver_name[] = "Neterion";
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH net-next-2.6] bridge: 64bit rx/tx counters
From: Andrew Morton @ 2010-08-12 15:07 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Stephen Hemminger, netdev, bhutchings, Nick Piggin
In-Reply-To: <1281615375.2494.20.camel@edumazet-laptop>

On Thu, 12 Aug 2010 14:16:15 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote:

> > And all this open-coded per-cpu counter stuff added all over the place.
> > Were percpu_counters tested or reviewed and found inadequate and unfixable?
> > If so, please do tell.
> > 
> 
> percpu_counters tries hard to maintain a view of the current value of
> the (global) counter. This adds a cost because of a shared cache line
> and locking. (__percpu_counter_sum() is not very scalable on big hosts,
> it locks the percpu_counter lock for a possibly long iteration)

Could be.  Is percpu_counter_read_positive() unsuitable?

> 
> For network stats we dont want to maintain this central value, we do the
> folding only when necessary.

hm.  Well, why?  That big walk across all possible CPUs could be really
expensive for some applications.  Especially if num_possible_cpus is
much larger than num_online_cpus, which iirc can happen in
virtualisation setups; probably it can happen in non-virtualised
machines too.

> And this folding has zero effect on
> concurrent writers (counter updates)

The fastpath looks a little expensive in the code you've added.  The
write_seqlock() does an rmw and a wmb() and the stats inc is a 64-bit
rmw whereas percpu_counters do a simple 32-bit add.  So I'd expect that
at some suitable batch value, percpu-counters are faster on 32-bit. 

They'll usually be slower on 64-bit, until that num_possible_cpus walk
bites you.

percpu_counters might need some work to make them irq-friendly.  That
bare spin_lock().

btw, I worry a bit about seqlocks in the presence of interrupts:

static inline void write_seqcount_begin(seqcount_t *s)
{
	s->sequence++;
	smp_wmb();
}

are we assuming that the ++ there is atomic wrt interrupts?  I think
so.  Is that always true for all architectures, compiler versions, etc?

> For network stack, we also need to update two values, a packet counter
> and a bytes counter. percpu_counter is not very good for the 'bytes
> counter', since we would have to use a arbitrary big bias value.

OK, that's a nasty problem for percpu-counters.

> Using several percpu_counter would also probably use more cache lines.
> 
> Also please note this stuff is only needed for 32bit arches. 
> 
> Using percpu_counter would slow down network stack on modern arches.

Was this ever quantified?

> 
> I am very well aware of the percpu_counter stuff, I believe I tried to
> optimize it a bit in the past.

^ permalink raw reply

* Re: [Bugme-new] [Bug 16568] New: Regression and incompatibility with Windows SP2-SP3-Vista TCP stack causing lost connections
From: Eric Dumazet @ 2010-08-12 15:09 UTC (permalink / raw)
  To: Andrew Morton; +Cc: netdev, bugzilla-daemon, bugme-daemon, yuriy
In-Reply-To: <20100812074041.cf62b793.akpm@linux-foundation.org>

Le jeudi 12 août 2010 à 07:40 -0700, Andrew Morton a écrit :
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> 
> On Thu, 12 Aug 2010 08:20:01 GMT bugzilla-daemon@bugzilla.kernel.org wrote:
> 
> > https://bugzilla.kernel.org/show_bug.cgi?id=16568
> > 
> >            Summary: Regression and incompatibility with Windows
> >                     SP2-SP3-Vista TCP stack causing lost connections
> >            Product: Networking
> >            Version: 2.5
> >     Kernel Version: 2.6.30+
> >           Platform: All
> >         OS/Version: Linux
> >               Tree: Mainline
> >             Status: NEW
> >           Severity: high
> >           Priority: P1
> >          Component: IPV4
> >         AssignedTo: shemminger@linux-foundation.org
> >         ReportedBy: yuriy@ucoz.com
> >         Regression: No
> > 
> > 
> > Hi.
> > I administer about 50 highly-loaded web servers (free CMS hosting) under linux.
> > Having on most of them kernel versions between 2.6.24 and 2.6.29 at the
> > beginnig of the year, I made TCP sysctls tunings for increasing DDOS and
> > different flooding protection (our servers have attacks rather often).
> > tcp_tw_recyle=1 was among of them, as many manuals in the net recommend to do
> > this and linux documentation does not say anything bad. Having periodic kernel
> > panics connected with bugs in ethernet card drivers and ext3 and after founding
> > that 2.6.31+ kernels work faster with ext3, I upgraded almost all kernels to
> > 2.6.32.8, which was already being tested on several servers for several months. 
> > Somewhen after that we began to receive complaints from our users (site owners)
> > that they (and their visitors) see very unstable work of their sites. It looked
> > like HTTP-connections were just lost in a random way. Not everybody had the
> > problem, just a small percent. We tried to find problem with internet providers
> > or buggy firewalls, but finally came to conclusion that problem is connected
> > with our servers. Analizing situations with lost connections using tcpdump i
> > found that client host send packets, BUT LINUX JUST IGNORES THEM, there was
> > SYN-packet repeated 3 times with interval of 3 secs, but NO SYN-ACK reply.
> > Most problems had users with Windows SP3 (i.e. almost all users with SP3 had
> > the problem). I booted one server with old 2.6.24 kernel and found that problem
> > dissappeared. Then began look for exact kernel version, that introduced
> > incompatibility. Using binary search I compiled several kernels between 2.6.24
> > and 2.6.32.8 and found that 2.6.29.6 DO NO have the problem, but 2.6.30 DOES.
> > Studing commits made to tcp_input.c and tcp_ipv4.c (which i supposed were
> > involved) between that releases I found this one.
> >   author    Eric Dumazet <dada1@cosmosbay.com>    
> >     Wed, 11 Mar 2009 16:23:57 +0000 (09:23 -0700)
> >   committer    David S. Miller <davem@davemloft.net>    
> >     Wed, 11 Mar 2009 16:23:57 +0000 (09:23 -0700)
> >   commit    fc1ad92dfc4e363a055053746552cdb445ba5c57
> > 
> >   tcp: allow timestamps even if SYN packet has tsval=0
> > 
> >   Some systems send SYN packets with apparently wrong RFC1323 timestamp
> >   option values [timestamp tsval=0 tsecr=0].
> >   It might be for security reasons (http://www.secuobs.com/plugs/25220.shtml )
> >   Linux TCP stack ignores this option and sends back a SYN+ACK packet
> >   without timestamp option, thus many TCP flows cannot use timestamps
> >   and lose some benefit of RFC1323.
> >   Other operating systems seem to not care about initial tsval value, and let
> >   tcp flows to negotiate timestamp option.
> > 
> >   net/ipv4/tcp_ipv4.c         diff :
> > 
> > --- a/net/ipv4/tcp_ipv4.c
> > +++ b/net/ipv4/tcp_ipv4.c
> > @@ -1226,15 +1226,6 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff
> > *skb)
> >         if (want_cookie && !tmp_opt.saw_tstamp)
> >                 tcp_clear_options(&tmp_opt);
> > 
> > -       if (tmp_opt.saw_tstamp && !tmp_opt.rcv_tsval) {
> > -               /* Some OSes (unknown ones, but I see them on web server, which
> > -                * contains information interesting only for windows'
> > -                * users) do not send their stamp in SYN. It is easy case.
> > -                * We simply do not advertise TS support.
> > -                */
> > -               tmp_opt.saw_tstamp = 0;
> > -               tmp_opt.tstamp_ok  = 0;
> > -       }
> >         tmp_opt.tstamp_ok = tmp_opt.saw_tstamp;
> > 
> >         tcp_openreq_init(req, &tmp_opt, skb);
> > 
> > Removing that was not very good. Having analized lost connections from SP3 I
> > know that they have timestamps turned on and timestamp value is 0. Here is it:
> > 13:39:10.430498 IP 192.168.99.130.3493 > 192.168.99.100.80: S
> > 2507911465:2507911465(0) win 65535 <mss 1460,nop,wscale 3,nop,nop,timestamp 0
> > 0,nop,nop,sackOK>
> >         0x0000:  4500 0040 2bda 4000 8006 86a6 c0a8 6382  E..@+.@.......c.
> >         0x0010:  c0a8 6364 0da5 0050 957b b129 0000 0000  ..cd...P.{.)....
> >         0x0020:  b002 ffff 992c 0000 0204 05b4 0103 0303  .....,..........
> >         0x0030:  0101 080a 0000 0000 0000 0000 0101 0402  ................
> > 
> > Having above code fragment removed we got tmp_opt.tstamp_ok=1, as i understand.
> > But a little later in source code of tcp_ipv4.c read:
> >         /* VJ's idea. We save last timestamp seen
> >          * from the destination in peer table, when entering
> >          * state TIME-WAIT, and check against it before
> >          * accepting new connection request.
> >          *
> >          * If "isn" is not zero, this request hit alive
> >          * timewait bucket, so that all the necessary checks
> >          * are made in the function processing timewait state.
> >          */
> >         if (tmp_opt.saw_tstamp &&
> >             tcp_death_row.sysctl_tw_recycle &&
> >             (dst = inet_csk_route_req(sk, req)) != NULL &&
> >             (peer = rt_get_peer((struct rtable *)dst)) != NULL &&
> >             peer->v4daddr == saddr) {
> >             if ((u32)get_seconds() - peer->tcp_ts_stamp < TCP_PAWS_MSL &&
> >                 (s32)(peer->tcp_ts - req->ts_recent) >
> >                             TCP_PAWS_WINDOW) {
> >                 NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_PAWSPASSIVEREJECTED);
> >                 goto drop_and_release;
> >             }
> >         }
> > which in some way (tmp_opt.saw_tstamp && tcp_death_row.sysctl_tw_recycle are
> > true), random way, having not closed time-wait sockets from the pear, leads to
> > packet ignorence.
> > 
> > As for me, i understand, that i should not enable tw_recycle, BUT DOCUMENTATION
> > DOES NOT STATE, that enabling it i'll got random and rather often lost of
> > connections from some types of popular clients (like Windows).
> > Concerning above stated commit, it should include something to prevent above
> > condition to become true if tmp_opt.rcv_tsval==0. I'm not sure, but something
> > like
> >         if (tmp_opt.saw_tstamp &&
> > +           tmp_opt.rcv_tsval &&
> >             tcp_death_row.sysctl_tw_recycle &&
> >             (dst = inet_csk_route_req(sk, req)) != NULL &&
> >             (peer = rt_get_peer((struct rtable *)dst)) != NULL &&
> > 
> > just to not provide regression and strong TCP-stack incompatibility in case
> > tw_recycle is enabled.
> > Also documentation does not state, that tw_recyle should not be used at all for
> > internet servers, because web-clients, which are behind NAT, will have problems
> > connected with the same above condition because successive connections from
> > different clients (which have common IP) could have incompatible timestamps.
> > 
> > Sorry if i detracted somebody busy from his work with my unimportant problem.
> > 
> 
> --

Hi Yuriy

Interesting analysis but wrong conclusions :)

Clients using RFC1323 (timestamps) and behind a NAT device will barf on
your setup. No matter they use Windows SP3 or other operating system.

Only because RFC1323 is more often enabled at client level (a registry
change on Windows XP, Vista or Seven I dont know), you start noticing
your server drops more connections than before.

Point is  :

Dont mess with tcp_tw_recycle=1, tcp_timestamps=1 on public machines

Its a non working setup, for clients behind NAT devices (since their
TSVAL will probably lead to incorrect behavior on server, with infamous
LINUX_MIB_PAWSPASSIVEREJECTED status seen on netstat -s, as you
discovered.

And your patch solves nothing for this very common case, unless the NAT
device is able to overwrite TSVAL values with its own values (very
unlikely !!!)

A working setup is (and is the default) :

tcp_tw_recycle=0
tcp_timestamps=1


Documentation might be improved, but I feel whole "tcp_tw_recycle"
affair is really too tricky to be ever documented (not mentioning using
it ;) )




^ permalink raw reply

* Re: [PATCH] netpoll: use non-BH variant of RCU
From: Paul E. McKenney @ 2010-08-12 15:42 UTC (permalink / raw)
  To: David Miller; +Cc: herbert, linville, netdev
In-Reply-To: <20100811.230936.183035599.davem@davemloft.net>

On Wed, Aug 11, 2010 at 11:09:36PM -0700, David Miller wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Date: Wed, 11 Aug 2010 15:00:47 -0700
> 
> > @@ -113,6 +113,12 @@ int rcu_my_thread_group_empty(void)
> >  	return thread_group_empty(current);
> >  }
> >  EXPORT_SYMBOL_GPL(rcu_my_thread_group_empty);
> > +
> > +void rcu_read_unlock_bh_irqsoff_check(void)
> > +{
> > +	WARN_ON_ONCE(in_irq() || irqs_disabled());
> > +}
> > +EXPORT_SYMBOL_GPL(rcu_read_unlock_bh_irqsoff_check);
> >  #endif /* #ifdef CONFIG_PROVE_RCU */
> 
> Is this WARN_ON_ONCE() test inverted?  It seems to be called where we
> "should be" in an IRQ or have IRQs disabled.

You are quite correct.  (Beat head against wall.)  :-/

------------------------------------------------------------------------

#!/bin/bash

for ((i=0;i<100;i++))
do
	echo "De Morgan when converting rcu_lockdep_assert() to WARN_ON_ONCE()"
done

------------------------------------------------------------------------

This does sort of defeat the purpose of writing something 100 times, but 
I am after all a software developer!!!

Thank you very much for catching this, and please see below for a
replacement patch.

							Thanx, Paul


commit 2c9ace45088a25b474167d04b416d279f4ea3401
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Wed Aug 11 14:54:33 2010 -0700

    rcu: add rcu_read_lock_bh_irqsoff() and rcu_read_unlock_bh_irqsoff()
    
    The rcu_read_lock_bh() and rcu_read_unlock_bh() functions can no
    longer be used when interrupts are disabled due to new debug checks in
    the _local_bh_enable() function.  This commit therefore supplies new
    functions that may only be called with either interrupts disabled or
    from interrupt handlers, and this is checked for under CONFIG_PROVE_RCU.
    
    Requested-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 9fbc54a..08cdc58 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -270,10 +270,13 @@ extern int rcu_my_thread_group_empty(void);
 		(p); \
 	})
 
+void rcu_read_unlock_bh_irqsoff_check(void);
+
 #else /* #ifdef CONFIG_PROVE_RCU */
 
 #define rcu_dereference_check(p, c)	rcu_dereference_raw(p)
 #define rcu_dereference_protected(p, c) (p)
+#define rcu_read_unlock_bh_irqsoff_check() do { } while (0)
 
 #endif /* #else #ifdef CONFIG_PROVE_RCU */
 
@@ -361,13 +364,13 @@ static inline void rcu_read_unlock(void)
  */
 static inline void rcu_read_lock_bh(void)
 {
-	__rcu_read_lock_bh();
+	local_bh_disable();
 	__acquire(RCU_BH);
 	rcu_read_acquire_bh();
 }
 
 /*
- * rcu_read_unlock_bh - marks the end of a softirq-only RCU critical section
+ * rcu_read_unlock_bh() - marks the end of a softirq-only RCU critical section
  *
  * See rcu_read_lock_bh() for more information.
  */
@@ -375,7 +378,34 @@ static inline void rcu_read_unlock_bh(void)
 {
 	rcu_read_release_bh();
 	__release(RCU_BH);
-	__rcu_read_unlock_bh();
+	local_bh_enable();
+}
+
+/**
+ * rcu_read_lock_bh_irqsoff() - mark the beginning of an RCU-bh critical section
+ *
+ * This is equivalent of rcu_read_lock_bh(), but to be used where the
+ * caller either is in an irq handler or has irqs disabled.  Note that
+ * this function assumes that PREEMPT_RT kernels run irq handlers at
+ * higher priority than softirq handlers!
+ */
+static inline void rcu_read_lock_bh_irqsoff(void)
+{
+	rcu_read_unlock_bh_irqsoff_check();
+	__acquire(RCU_BH);
+	rcu_read_acquire_bh();
+}
+
+/*
+ * rcu_read_unlock_bh_irqsoff - marks the end of an RCU-bh critical section
+ *
+ * See rcu_read_lock_bh_irqsoff() for more information.
+ */
+static inline void rcu_read_unlock_bh_irqsoff(void)
+{
+	rcu_read_release_bh();
+	__release(RCU_BH);
+	rcu_read_unlock_bh_irqsoff_check();
 }
 
 /**
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index e2e8931..009c7f3 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -36,8 +36,6 @@ static inline void rcu_note_context_switch(int cpu)
 
 #define __rcu_read_lock()	preempt_disable()
 #define __rcu_read_unlock()	preempt_enable()
-#define __rcu_read_lock_bh()	local_bh_disable()
-#define __rcu_read_unlock_bh()	local_bh_enable()
 #define call_rcu_sched		call_rcu
 
 #define rcu_init_sched()	do { } while (0)
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index c0ed1c0..98b50d8 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -75,15 +75,6 @@ static inline int rcu_preempt_depth(void)
 
 #endif /* #else #ifdef CONFIG_TREE_PREEMPT_RCU */
 
-static inline void __rcu_read_lock_bh(void)
-{
-	local_bh_disable();
-}
-static inline void __rcu_read_unlock_bh(void)
-{
-	local_bh_enable();
-}
-
 extern void call_rcu_sched(struct rcu_head *head,
 			   void (*func)(struct rcu_head *rcu));
 extern void synchronize_rcu_bh(void);
diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c
index 4d16983..ae6ae40 100644
--- a/kernel/rcupdate.c
+++ b/kernel/rcupdate.c
@@ -113,6 +113,12 @@ int rcu_my_thread_group_empty(void)
 	return thread_group_empty(current);
 }
 EXPORT_SYMBOL_GPL(rcu_my_thread_group_empty);
+
+void rcu_read_unlock_bh_irqsoff_check(void)
+{
+	WARN_ON_ONCE(!in_irq() && !irqs_disabled());
+}
+EXPORT_SYMBOL_GPL(rcu_read_unlock_bh_irqsoff_check);
 #endif /* #ifdef CONFIG_PROVE_RCU */
 
 #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD

^ permalink raw reply related

* Re: sky2 driver fails to handle "rx length error: status 0x5d60100 length 2982" gracefully
From: Stephen Hemminger @ 2010-08-12 16:00 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Stephen Hemminger, Linux NetDev
In-Reply-To: <AANLkTikDOerA4g+-=v+wM8cmysnemCTqEx0ncCCERLY6@mail.gmail.com>

On Wed, 11 Aug 2010 22:36:57 -0700
Maciej Żenczykowski <zenczykowski@gmail.com> wrote:

> Here's lspci (it's an otherwise stock MacBook Pro 4,1 with a
> non-standard wireless atheros mini-pci nic, replacing the std
> broadcom.)
> 
> $ lspci
> 00:00.0 Host bridge: Intel Corporation Mobile PM965/GM965/GL960 Memory
> Controller Hub (rev 03)
> 00:01.0 PCI bridge: Intel Corporation Mobile PM965/GM965/GL960 PCI
> Express Root Port (rev 03)
> 00:1a.0 USB Controller: Intel Corporation 82801H (ICH8 Family) USB
> UHCI Controller #4 (rev 03)
> 00:1a.1 USB Controller: Intel Corporation 82801H (ICH8 Family) USB
> UHCI Controller #5 (rev 03)
> 00:1a.7 USB Controller: Intel Corporation 82801H (ICH8 Family) USB2
> EHCI Controller #2 (rev 03)
> 00:1b.0 Audio device: Intel Corporation 82801H (ICH8 Family) HD Audio
> Controller (rev 03)
> 00:1c.0 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express
> Port 1 (rev 03)
> 00:1c.2 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express
> Port 3 (rev 03)
> 00:1c.4 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express
> Port 5 (rev 03)
> 00:1c.5 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express
> Port 6 (rev 03)
> 00:1d.0 USB Controller: Intel Corporation 82801H (ICH8 Family) USB
> UHCI Controller #1 (rev 03)
> 00:1d.1 USB Controller: Intel Corporation 82801H (ICH8 Family) USB
> UHCI Controller #2 (rev 03)
> 00:1d.2 USB Controller: Intel Corporation 82801H (ICH8 Family) USB
> UHCI Controller #3 (rev 03)
> 00:1d.7 USB Controller: Intel Corporation 82801H (ICH8 Family) USB2
> EHCI Controller #1 (rev 03)
> 00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev f3)
> 00:1f.0 ISA bridge: Intel Corporation 82801HEM (ICH8M) LPC Interface
> Controller (rev 03)
> 00:1f.1 IDE interface: Intel Corporation 82801HBM/HEM (ICH8M/ICH8M-E)
> IDE Controller (rev 03)
> 00:1f.2 IDE interface: Intel Corporation 82801HBM/HEM (ICH8M/ICH8M-E)
> SATA IDE Controller (rev 03)
> 00:1f.3 SMBus: Intel Corporation 82801H (ICH8 Family) SMBus Controller (rev 03)
> 01:00.0 VGA compatible controller: nVidia Corporation G84 [GeForce
> 8600M GT] (rev a1)
> 0b:00.0 Network controller: Atheros Communications Inc. AR928X
> Wireless Network Adapter (PCI-Express) (rev 01)
> 0c:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8058
> PCI-E Gigabit Ethernet Controller (rev 13)
> 0d:03.0 FireWire (IEEE 1394): Texas Instruments TSB82AA2 IEEE-1394b
> Link Layer Controller (rev 02)
> 
> (more verbose lspci included in bugzilla entry)
> 
> At least one other person has seen this on a desktop non-mac machine
> (see bugzilla) entry.
> What would you like from dmesg?
> Is the following enough?
> 
> Aug  9 12:09:11 nike kernel: sky2: driver version 1.27
> Aug  9 12:09:11 nike kernel: sky2 0000:0c:00.0: PCI INT A -> GSI 17
> (level, low) -> IRQ 17
> Aug  9 12:09:11 nike kernel: sky2 0000:0c:00.0: Yukon-2 EC Ultra chip revision 3
> Aug  9 12:09:11 nike kernel: sky2 0000:0c:00.0: eth0: addr 00:1f:5b:xx:xx:xx
> ...
> Aug  9 12:09:22 nike kernel: sky2 0000:0c:00.0: eth0: enabling interface
> Aug  9 12:09:22 nike kernel: ADDRCONF(NETDEV_UP): eth0: link is not ready
> ...
> Aug  9 12:09:25 nike kernel: sky2 0000:0c:00.0: eth0: Link is up at
> 1000 Mbps, full duplex, flow control rx
> Aug  9 12:09:25 nike kernel: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> ...
> Aug 11 16:21:19 nike kernel: sky2 0000:0c:00.0: eth0: rx length error:
> status 0x5d60100 length 2982
> Aug 11 16:21:27 nike kernel: eth0: hw csum failure.
> ...
> 
> I'd just like to point out that this has happened something like 5
> times in the past 30 days on a machine which is on 24/7 with wired
> ethernet plugged in nearly 100% of the time.

Probably he only thing the driver can do in these cases is automatically
turn off checksumming if it suspects the chip is having problems.

Is there a known good older kernel version?


^ permalink raw reply

* RE: e1000e crashes with 2.6.34.x and ThinkPad T60
From: Allan, Bruce W @ 2010-08-12 16:12 UTC (permalink / raw)
  To: Marc Haber
  Cc: Tantilov, Emil S, Linux Kernel Developers,
	Linux Kernel Network Developers, e1000-devel@lists.sf.net
In-Reply-To: <20100812105029.GA6816@torres.zugschlus.de>

On Thursday, August 12, 2010 3:50 AM, Marc Haber wrote:
> Hi,
> 
> On Tue, Aug 10, 2010 at 09:34:52AM -0700, Allan, Bruce W wrote:
>> [adding e1000-devel, the Intel wired ethernet developers mailing
>> list] 
> 
> Thanks.
> 
>> We have had other recent reports of issues with this part that are
>> due to ASPM L1 being enabled.  Would you please try disabling L1
>> after the driver is loaded as follows (assuming your adapter is
>> still PCI bus/device/number 02:00.0 as indicated in the lspci output
>> you provided earlier): 1) First check the hexadecimal value of the
>> LnkCtl register - # setpci -s 2:0.0 0xf0
> 
> $ sudo setpci --version
> setpci version 3.1.7
> $ sudo setpci -s 2:0.0 0xf0
> setpci: Missing width.
> Try `setpci --help' for more information.
> $
> 
> Looking at --help doesn't help me, sorry.
> 
> Greetings
> Marc

Hmm, that's a newer version than I am familiar with.  Apparently in
more recent versions, the tool is requiring a width be specified for
unnamed registers and/or registers for which the width is unknown.
That being the case, append the width specifier .B (one byte) to the
register address.  For example:

# setpci -s 2:0.0 0xf0.B

HTH,
Bruce.

^ permalink raw reply

* Re: sky2 driver fails to handle "rx length error: status 0x5d60100 length 2982" gracefully
From: Stephen Hemminger @ 2010-08-12 16:16 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Stephen Hemminger, Linux NetDev
In-Reply-To: <20100812120044.5b1880ad@s6510>


> > Aug 11 16:21:19 nike kernel: sky2 0000:0c:00.0: eth0: rx length error:
> > status 0x5d60100 length 2982
> > Aug 11 16:21:27 nike kernel: eth0: hw csum failure.

Are you trying to run with Jumbo >1500 MTU?

^ permalink raw reply

* Re: Fw: [PATCH net-next] drivers/s390/net/qeth_l3_main.c: Use %pI6
From: Ursula Braun @ 2010-08-12 16:36 UTC (permalink / raw)
  To: Ursula Braun1; +Cc: blaschka, linux-s390, netdev, linux-kernel
In-Reply-To: <OF89C50871.5ED34E2A-ONC125777D.005A40DE-C125777D.005A4DDE@de.ibm.com>

ACK
Thanks Joe - we make sure your patch is submitted to Dave Miller within
our next submission of qeth patches.

Ursula

> From:     Joe Perches <joe@perches.com>
> To:       Frank Blaschka <blaschka@linux.vnet.ibm.com>
> Subject:  [PATCH net-next] drivers/s390/net/qeth_l3_main.c: Use %pI6

> Format an ipv6 address using vsprintf extensions.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/s390/net/qeth_l3_main.c |    7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/s390/net/qeth_l3_main.c
> b/drivers/s390/net/qeth_l3_main.c
> index e22ae24..561bdc8 100644
> --- a/drivers/s390/net/qeth_l3_main.c
> +++ b/drivers/s390/net/qeth_l3_main.c
> @@ -103,12 +103,7 @@ int qeth_l3_string_to_ipaddr4(const char *buf,
> __u8 *addr)
> 
> void qeth_l3_ipaddr6_to_string(const __u8 *addr, char *buf)
> {
> -                 sprintf(buf, "%02x%02x:%02x%02x:%02x%02x:%02x%02x"
> -                                       ":%02x%02x:%02x%02x:%02x%02x:%
> 02x%02x",
> -                                       addr[0], addr[1], addr[2],
> addr[3],
> -                                       addr[4], addr[5], addr[6],
> addr[7],
> -                                       addr[8], addr[9], addr[10],
> addr[11],
> -                                       addr[12], addr[13], addr[14],
> addr[15]);
> +                 sprintf(buf, "%pI6", addr);
> }
> 
> int qeth_l3_string_to_ipaddr6(const char *buf, __u8 *addr)
> 
> 

^ permalink raw reply

* mmotm 2010-08-11 - lockdep whinges at e1000e driver ifconfig up
From: Valdis.Kletnieks @ 2010-08-12 16:37 UTC (permalink / raw)
  To: Andrew Morton, davem, jeffrey.t.kirsher, jesse.brandeburg, kaber,
	jengelh, eric.dumazet
  Cc: linux-kernel, netdev, e1000-devel, netfilter
In-Reply-To: <201008112340.o7BNenDe021017@imap1.linux-foundation.org>

[-- Attachment #1: Type: text/plain, Size: 6017 bytes --]

On Wed, 11 Aug 2010 16:10:49 PDT, akpm@linux-foundation.org said:
> The mm-of-the-moment snapshot 2010-08-11-16-10 has been uploaded to
> 
>    http://userweb.kernel.org/~akpm/mmotm/

Not sure if it's an e1000e bug, or an iptables bug that happened to trip on
like the first few packets accepted after the interface came up, so I'll cc:
everybody and let ya'll fight over it. :)

[  431.011194] e1000e 0000:00:19.0: irq 46 for MSI/MSI-X
[  431.062183] e1000e 0000:00:19.0: irq 46 for MSI/MSI-X
[  431.064607] ADDRCONF(NETDEV_UP): eth0: link is not ready
[  432.691161] e1000e: eth0 NIC Link is Up 100 Mbps Full Duplex, Flow Control: None
[  432.691177] e1000e 0000:00:19.0: eth0: 10/100 speed: disabling TSO
[  432.695461] ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[  432.697278] 
[  432.697279] =================================
[  432.697477] [ INFO: inconsistent lock state ]
[  432.697581] 2.6.35-mmotm0811 #1
[  432.697682] ---------------------------------
[  432.697785] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
[  432.697890] kworker/0:0/0 [HC0[0]:SC1[2]:HE1:SE0] takes:
[  432.697994]  (&(&lock->lock)->rlock){+.?...}, at: [<ffffffff814d39c8>] ip6t_do_table+0xc1/0x646
[  432.698028] {SOFTIRQ-ON-W} state was registered at:
[  432.698028]   [<ffffffff8106762f>] __lock_acquire+0x3a3/0xd6a
[  432.698028]   [<ffffffff81068514>] lock_acquire+0x10a/0x130
[  432.698028]   [<ffffffff81557cd9>] _raw_spin_lock+0x36/0x45
[  432.698028]   [<ffffffff814d3375>] xt_info_wrlock+0x1c/0x1e
[  432.698028]   [<ffffffff814d48df>] get_counters+0x93/0x14a
[  432.698028]   [<ffffffff814d49c3>] alloc_counters.clone.3+0x2d/0x41
[  432.698028]   [<ffffffff814d4f98>] do_ip6t_get_ctl+0x110/0x367
[  432.698028]   [<ffffffff814402a7>] nf_sockopt+0x5c/0x88
[  432.698028]   [<ffffffff814402e6>] nf_getsockopt+0x13/0x15
[  432.698028]   [<ffffffff814ba05e>] ipv6_getsockopt+0x94/0xc3
[  432.698028]   [<ffffffff814c1175>] rawv6_getsockopt+0x48/0x54
[  432.698028]   [<ffffffff8141533a>] sock_common_getsockopt+0xf/0x11
[  432.698028]   [<ffffffff814147dd>] sys_getsockopt+0x75/0x93
[  432.698028]   [<ffffffff8100272b>] system_call_fastpath+0x16/0x1b
[  432.698028] irq event stamp: 3554312
[  432.698028] hardirqs last  enabled at (3554312): [<ffffffff8103fcb9>] _local_bh_enable_ip+0x139/0x178
[  432.698028] hardirqs last disabled at (3554311): [<ffffffff8103fc3a>] _local_bh_enable_ip+0xba/0x178
[  432.698028] softirqs last  enabled at (3554260): [<ffffffff81040282>] __do_softirq+0x273/0x289
[  432.698028] softirqs last disabled at (3554277): [<ffffffff8100364c>] call_softirq+0x1c/0x28
[  432.698028] 
[  432.698028] other info that might help us debug this:
[  432.698028] 3 locks held by kworker/0:0/0:
[  432.698028]  #0:  (&idev->mc_ifc_timer){+.-...}, at: [<ffffffff810468f9>] run_timer_softirq+0x1d2/0x442
[  432.698028]  #1:  (rcu_read_lock){.+.+..}, at: [<ffffffff814c3c2a>] rcu_read_lock+0x0/0x35
[  432.698028]  #2:  (rcu_read_lock){.+.+..}, at: [<ffffffff8143e840>] rcu_read_lock+0x0/0x35
[  432.698028] 
[  432.698028] stack backtrace:
[  432.698028] Pid: 0, comm: kworker/0:0 Not tainted 2.6.35-mmotm0811 #1
[  432.698028] Call Trace:
[  432.698028]  <IRQ>  [<ffffffff810670a2>] valid_state+0x17c/0x18e
[  432.698028]  [<ffffffff81066967>] ? check_usage_forwards+0x0/0x87
[  432.698028]  [<ffffffff81067193>] mark_lock+0xdf/0x1d8
[  432.698028]  [<ffffffff810675b1>] __lock_acquire+0x325/0xd6a
[  432.698028]  [<ffffffff8106768f>] ? __lock_acquire+0x403/0xd6a
[  432.698028]  [<ffffffff810687fc>] ? mark_held_locks+0x50/0x72
[  432.698028]  [<ffffffff814d39c8>] ? ip6t_do_table+0xc1/0x646
[  432.698028]  [<ffffffff81068514>] lock_acquire+0x10a/0x130
[  432.698028]  [<ffffffff814d39c8>] ? ip6t_do_table+0xc1/0x646
[  432.698028]  [<ffffffff8103fce6>] ? _local_bh_enable_ip+0x166/0x178
[  432.698028]  [<ffffffff81557cd9>] _raw_spin_lock+0x36/0x45
[  432.698028]  [<ffffffff814d39c8>] ? ip6t_do_table+0xc1/0x646
[  432.698028]  [<ffffffff814d39c8>] ip6t_do_table+0xc1/0x646
[  432.698028]  [<ffffffff8103fce6>] ? _local_bh_enable_ip+0x166/0x178
[  432.698028]  [<ffffffff8103fd10>] ? local_bh_enable+0xd/0xf
[  432.698028]  [<ffffffff8144234b>] ? nf_conntrack_in+0x4a9/0x5b9
[  432.698028]  [<ffffffff814d5ee7>] ip6table_filter_hook+0x17/0x1c
[  432.698028]  [<ffffffff8143ec43>] nf_iterate+0x41/0x84
[  432.698028]  [<ffffffff814c3ec5>] ? dst_output+0x0/0x70
[  432.698028]  [<ffffffff8143ecf9>] nf_hook_slow+0x73/0xde
[  432.698028]  [<ffffffff814c3ec5>] ? dst_output+0x0/0x70
[  432.698028]  [<ffffffff8104710a>] ? msleep_interruptible+0x5b/0x72
[  432.698028]  [<ffffffff814c508e>] NF_HOOK.clone.21+0x3e/0x52
[  432.698028]  [<ffffffff81499c6c>] ? xfrm_lookup+0x11/0x2e
[  432.698028]  [<ffffffff814c531f>] mld_sendpack+0x27d/0x3dd
[  432.698028]  [<ffffffff814c5ad6>] mld_ifc_timer_expire+0x1ca/0x207
[  432.698028]  [<ffffffff810469eb>] run_timer_softirq+0x2c4/0x442
[  432.698028]  [<ffffffff810468f9>] ? run_timer_softirq+0x1d2/0x442
[  432.698028]  [<ffffffff81059222>] ? __run_hrtimer+0x1ec/0x234
[  432.698028]  [<ffffffff814c590c>] ? mld_ifc_timer_expire+0x0/0x207
[  432.698028]  [<ffffffff81040080>] ? __do_softirq+0x71/0x289
[  432.698028]  [<ffffffff81040155>] __do_softirq+0x146/0x289
[  432.698028]  [<ffffffff810a29bc>] ? time_hardirqs_off+0x1b/0x2f
[  432.698028]  [<ffffffff8100364c>] call_softirq+0x1c/0x28
[  432.698028]  [<ffffffff81004bc3>] do_softirq+0x44/0xf1
[  432.698028]  [<ffffffff8104035a>] irq_exit+0x4a/0xb4
[  432.698028]  [<ffffffff8101a3dd>] smp_apic_timer_interrupt+0x79/0x87
[  432.698028]  [<ffffffff81003113>] apic_timer_interrupt+0x13/0x20
[  432.698028]  <EOI>  [<ffffffff81277630>] ? acpi_idle_enter_simple+0x122/0x15a
[  432.698028]  [<ffffffff8127762b>] ? acpi_idle_enter_simple+0x11d/0x15a
[  432.698028]  [<ffffffff813b9c3c>] cpuidle_idle_call+0x9b/0x15d
[  432.698028]  [<ffffffff81000c73>] cpu_idle+0x85/0x169
[  432.698028]  [<ffffffff81b5e906>] start_secondary+0x1b1/0x1b5
[  497.031095] ADDRCONF(NETDEV_UP): wlan0: link is not ready



[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply

* Re: sky2 driver fails to handle "rx length error: status 0x5d60100 length 2982" gracefully
From: Maciej Żenczykowski @ 2010-08-12 16:58 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Stephen Hemminger, Linux NetDev
In-Reply-To: <20100812121600.2e971e66@s6510>

I'm not sure if there is a known good kernel.  It seems to be getting
worse over time (as I upgrade kernels), but maybe the hardware is
aging and the situation is becoming more likely.  When it first
started happening it was like once every 2-3 months or even rarer.
Now it has happened again since the last time I posted to this
thread...

Aug 12 08:29:08 nike kernel: sky2 0000:0c:00.0: eth0: rx length error:
status 0x5e50100 length 3013

> Are you trying to run with Jumbo >1500 MTU?

No, normal 1500 MTU network, with ipv4 and ipv6 native traffic.  Not a
huge amount of traffic either.
And indeed the problem seems to happen just as easily (if not easier)
when the machine (and thus the network) is close(r) to idle (ie.
overnight, etc) - although that might just be a matter of more time
passing.

Are you sure there is nothing the driver could do on seeing such an error?
It seems like since "ip link set eth0 down && ip link set eth0 up"
fixes it, what it should do is some sort of partial reset...

I will try to verify if 'ethtool -K eth0 rx off && ethtool -K eth0 rx
on' is enough to fix the problem (when it happens once again).
Afterwards I'll turn of rx csum (ethtool -K eth0 rx off) and will see
if it happens again.

^ 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