Netdev List
 help / color / mirror / Atom feed
* Re: Bad TCP checksum error
From: Rick Jones @ 2007-10-26 16:59 UTC (permalink / raw)
  To: Gaurav Aggarwal
  Cc: netfilter-devel, netdev, linux-net, linux-kernel, davidsen
In-Reply-To: <1a41e0840710260903w33516f62kc53e5eb56e424d69@mail.gmail.com>

Checksum Offload on the NIC(s) can complicate things.  First, if you are tracing 
on the sender, the tracepoint is before the NIC has computed the full checksum. 
  IIRC only a partial checksum is passed-down to the NIC when CKO is in use.

So, making certain your trace is from the "wire" or the receiver rather than the 
sender would be a good thing, and trying again with CKO disabled on the 
interface(s) (via ethtool) might be something worth looking at.  Ultimately, 
doing the partial checksum modificiations in a CKO-friendly manner might be a 
good thing.

rick jones

^ permalink raw reply

* [PATCH 2] isdn: fix 'and' typo's
From: Roel Kluin @ 2007-10-26 17:20 UTC (permalink / raw)
  To: netdev

This patch is similar to the former, but affects isdn drivers.
also note these changes in drivers/isdn/i4l/isdn_ttyfax.c:
-		if (!info->faxonline & 1)	/* not outgoing connection */
+		if (!info->faxonline)	/* not outgoing connection */
...
-			if ((f->phase != ISDN_FAX_PHASE_D) || (!info->faxonline & 1))
+			if ((f->phase != ISDN_FAX_PHASE_D) || (!info->faxonline))
rationale: not already makes it binary

The condition '!x & y,' does make little sense: the '!' has a higher
priority than '&'. It behaves therefore like '!x && y'. In the case 
bitanding flags, however, '!(x & y)' appears to be desired.

Warning: the change of '!x & y,' to '!(x & y)' may change behavior. if
not desired, I propose changing this to '!x && y', to make it explicitly
clear.
--
    Fix priority mistakes similar to '!x & y'
    
    Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index ee2b0b9..8325022 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -310,7 +310,7 @@ act2000_command(act2000_card * card, isdn_ctrl * c)
 			}
 			break;
 		case ISDN_CMD_DIAL:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			if (!(chan = find_channel(card, c->arg & 0x0f)))
 				break;
@@ -339,7 +339,7 @@ act2000_command(act2000_card * card, isdn_ctrl * c)
 			}
 			return ret;
 		case ISDN_CMD_ACCEPTD:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			if (!(chan = find_channel(card, c->arg & 0x0f)))
 				break;
@@ -347,11 +347,11 @@ act2000_command(act2000_card * card, isdn_ctrl * c)
 				actcapi_select_b2_protocol_req(card, chan);
 			return 0;
 		case ISDN_CMD_ACCEPTB:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			return 0;
 		case ISDN_CMD_HANGUP:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			if (!(chan = find_channel(card, c->arg & 0x0f)))
 				break;
@@ -366,7 +366,7 @@ act2000_command(act2000_card * card, isdn_ctrl * c)
 			}
 			return 0;
 		case ISDN_CMD_SETEAZ:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			if (!(chan = find_channel(card, c->arg & 0x0f)))
 				break;
@@ -386,7 +386,7 @@ act2000_command(act2000_card * card, isdn_ctrl * c)
 			actcapi_listen_req(card);
 			return 0;
 		case ISDN_CMD_CLREAZ:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			if (!(chan = find_channel(card, c->arg & 0x0f)))
 				break;
@@ -394,14 +394,14 @@ act2000_command(act2000_card * card, isdn_ctrl * c)
 			actcapi_listen_req(card);
 			return 0;
 		case ISDN_CMD_SETL2:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			if (!(chan = find_channel(card, c->arg & 0x0f)))
 				break;
 			chan->l2prot = (c->arg >> 8);
 			return 0;
 		case ISDN_CMD_SETL3:
-			if (!card->flags & ACT2000_FLAGS_RUNNING)
+			if (!(card->flags & ACT2000_FLAGS_RUNNING))
 				return -ENODEV;
 			if ((c->arg >> 8) != ISDN_PROTO_L3_TRANS) {
 				printk(KERN_WARNING "L3 protocol unknown\n");
@@ -524,7 +524,7 @@ if_writecmd(const u_char __user *buf, int len, int id, int channel)
         act2000_card *card = act2000_findcard(id);
 
         if (card) {
-                if (!card->flags & ACT2000_FLAGS_RUNNING)
+                if (!(card->flags & ACT2000_FLAGS_RUNNING))
                         return -ENODEV;
                 return (len);
         }
@@ -539,7 +539,7 @@ if_readstatus(u_char __user * buf, int len, int id, int channel)
         act2000_card *card = act2000_findcard(id);
 	
         if (card) {
-                if (!card->flags & ACT2000_FLAGS_RUNNING)
+                if (!(card->flags & ACT2000_FLAGS_RUNNING))
                         return -ENODEV;
                 return (act2000_readstatus(buf, len, card));
         }
@@ -554,7 +554,7 @@ if_sendbuf(int id, int channel, int ack, struct sk_buff *skb)
         act2000_card *card = act2000_findcard(id);
 	
         if (card) {
-                if (!card->flags & ACT2000_FLAGS_RUNNING)
+                if (!(card->flags & ACT2000_FLAGS_RUNNING))
                         return -ENODEV;
 		return (act2000_sendbuf(card, channel, ack, skb));
         }
diff --git a/drivers/isdn/i4l/isdn_ttyfax.c b/drivers/isdn/i4l/isdn_ttyfax.c
index a943d07..d854f22 100644
--- a/drivers/isdn/i4l/isdn_ttyfax.c
+++ b/drivers/isdn/i4l/isdn_ttyfax.c
@@ -834,7 +834,7 @@ isdn_tty_cmd_FCLASS2(char **p, modem_info * info)
 		char *rp = &f->resolution;
 
 		p[0] += 2;
-		if (!info->faxonline & 1)	/* not outgoing connection */
+		if (!info->faxonline)	/* not outgoing connection */
 			PARSE_ERROR1;
 
 		for (i = 0; (((*p[0] >= '0') && (*p[0] <= '9')) || (*p[0] == ',')) && (i < 4); i++) {
@@ -906,7 +906,7 @@ isdn_tty_cmd_FCLASS2(char **p, modem_info * info)
 			sprintf(rs, "\r\n0-2");
 			isdn_tty_at_cout(rs, info);
 		} else {
-			if ((f->phase != ISDN_FAX_PHASE_D) || (!info->faxonline & 1))
+			if ((f->phase != ISDN_FAX_PHASE_D) || (!info->faxonline))
 				PARSE_ERROR1;
 			par = isdn_getnum(p);
 			if ((par < 0) || (par > 2))
diff --git a/drivers/isdn/icn/icn.c b/drivers/isdn/icn/icn.c
index 82d957b..bf7997a 100644
--- a/drivers/isdn/icn/icn.c
+++ b/drivers/isdn/icn/icn.c
@@ -1302,7 +1302,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_DIAL:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			if (card->leased)
 				break;
@@ -1328,7 +1328,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_ACCEPTD:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			if (c->arg < ICN_BCH) {
 				a = c->arg + 1;
@@ -1348,7 +1348,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_ACCEPTB:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			if (c->arg < ICN_BCH) {
 				a = c->arg + 1;
@@ -1366,7 +1366,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_HANGUP:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			if (c->arg < ICN_BCH) {
 				a = c->arg + 1;
@@ -1375,7 +1375,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_SETEAZ:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			if (card->leased)
 				break;
@@ -1391,7 +1391,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_CLREAZ:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			if (card->leased)
 				break;
@@ -1405,7 +1405,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_SETL2:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			if ((c->arg & 255) < ICN_BCH) {
 				a = c->arg;
@@ -1424,7 +1424,7 @@ icn_command(isdn_ctrl * c, icn_card * card)
 			}
 			break;
 		case ISDN_CMD_SETL3:
-			if (!card->flags & ICN_FLAGS_RUNNING)
+			if (!(card->flags & ICN_FLAGS_RUNNING))
 				return -ENODEV;
 			return 0;
 		default:
@@ -1471,7 +1471,7 @@ if_writecmd(const u_char __user *buf, int len, int id, int channel)
 	icn_card *card = icn_findcard(id);
 
 	if (card) {
-		if (!card->flags & ICN_FLAGS_RUNNING)
+		if (!(card->flags & ICN_FLAGS_RUNNING))
 			return -ENODEV;
 		return (icn_writecmd(buf, len, 1, card));
 	}
@@ -1486,7 +1486,7 @@ if_readstatus(u_char __user *buf, int len, int id, int channel)
 	icn_card *card = icn_findcard(id);
 
 	if (card) {
-		if (!card->flags & ICN_FLAGS_RUNNING)
+		if (!(card->flags & ICN_FLAGS_RUNNING))
 			return -ENODEV;
 		return (icn_readstatus(buf, len, card));
 	}
@@ -1501,7 +1501,7 @@ if_sendbuf(int id, int channel, int ack, struct sk_buff *skb)
 	icn_card *card = icn_findcard(id);
 
 	if (card) {
-		if (!card->flags & ICN_FLAGS_RUNNING)
+		if (!(card->flags & ICN_FLAGS_RUNNING))
 			return -ENODEV;
 		return (icn_sendbuf(channel, ack, skb, card));
 	}
diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index bb92e3c..a335c85 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -1184,7 +1184,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 			}
 			break;
 		case ISDN_CMD_DIAL:
-			if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+			if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 				return -ENODEV;
 			if (card->leased)
 				break;
@@ -1210,7 +1210,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 			}
 			break;
 		case ISDN_CMD_ACCEPTD:
-			if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+			if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 				return -ENODEV;
 			if (c->arg < ISDNLOOP_BCH) {
 				a = c->arg + 1;
@@ -1238,7 +1238,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 			}
 			break;
 		case ISDN_CMD_ACCEPTB:
-			if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+			if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 				return -ENODEV;
 			if (c->arg < ISDNLOOP_BCH) {
 				a = c->arg + 1;
@@ -1264,7 +1264,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 				i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
 				break;
 		case ISDN_CMD_HANGUP:
-				if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+				if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 					return -ENODEV;
 				if (c->arg < ISDNLOOP_BCH) {
 					a = c->arg + 1;
@@ -1273,7 +1273,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 				}
 				break;
 		case ISDN_CMD_SETEAZ:
-				if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+				if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 					return -ENODEV;
 				if (card->leased)
 					break;
@@ -1289,7 +1289,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 				}
 				break;
 		case ISDN_CMD_CLREAZ:
-				if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+				if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 					return -ENODEV;
 				if (card->leased)
 					break;
@@ -1303,7 +1303,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 				}
 				break;
 		case ISDN_CMD_SETL2:
-				if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+				if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 					return -ENODEV;
 				if ((c->arg & 255) < ISDNLOOP_BCH) {
 					a = c->arg;
@@ -1333,7 +1333,7 @@ isdnloop_command(isdn_ctrl * c, isdnloop_card * card)
 				}
 				break;
 		case ISDN_CMD_SETL3:
-				if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+				if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 					return -ENODEV;
 				return 0;
 		default:
@@ -1380,7 +1380,7 @@ if_writecmd(const u_char __user *buf, int len, int id, int channel)
 	isdnloop_card *card = isdnloop_findcard(id);
 
 	if (card) {
-		if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+		if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 			return -ENODEV;
 		return (isdnloop_writecmd(buf, len, 1, card));
 	}
@@ -1395,7 +1395,7 @@ if_readstatus(u_char __user *buf, int len, int id, int channel)
 	isdnloop_card *card = isdnloop_findcard(id);
 
 	if (card) {
-		if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+		if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 			return -ENODEV;
 		return (isdnloop_readstatus(buf, len, card));
 	}
@@ -1410,7 +1410,7 @@ if_sendbuf(int id, int channel, int ack, struct sk_buff *skb)
 	isdnloop_card *card = isdnloop_findcard(id);
 
 	if (card) {
-		if (!card->flags & ISDNLOOP_FLAGS_RUNNING)
+		if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 			return -ENODEV;
 		/* ack request stored in skb scratch area */
 		*(skb->head) = ack;

^ permalink raw reply related

* [PATCH 3] isdn: incorrect assignments in if
From: Roel Kluin @ 2007-10-26 17:26 UTC (permalink / raw)
  To: netdev

    incorrect assignments in if
    
    Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/isdn/hisax/elsa.c b/drivers/isdn/hisax/elsa.c
index 948a9b2..ed610ed 100644
--- a/drivers/isdn/hisax/elsa.c
+++ b/drivers/isdn/hisax/elsa.c
@@ -883,7 +883,7 @@ setup_elsa_isa(struct IsdnCard *card)
 	val += 'A' - 3;
 	if (val == 'B' || val == 'C')
 		val ^= 1;
-	if ((cs->subtyp == ELSA_PCFPRO) && (val = 'G'))
+	if ((cs->subtyp == ELSA_PCFPRO) && (val == 'G'))
 		val = 'C';
 	printk(KERN_INFO
 	       "Elsa: %s found at %#lx Rev.:%c IRQ %d\n",

^ permalink raw reply related

* [PATCH 1] net: fix and typo's
From: Roel Kluin @ 2007-10-26 17:07 UTC (permalink / raw)
  To: netdev

A few patches with changes to net code. I have sent these to the lkml
previously, but they were not yet merged. I am fairly new to kernel 
programming, so it is possible that I make some mistakes. I'll explain my
rationale, please nack if incorrect, an additional bit of explanation is
appreciated even more.

The condition '!x & y,' does make little sense: the '!' has a higher
priority than '&'. It behaves therefore like '!x && y'. In the case 
bitanding flags, however, '!(x & y)' appears to be desired.

Warning: the change of '!x & y,' to '!(x & y)' may change behavior. if
not desired, I propose changing this to '!x && y', to make it explicitly
clear.

These '&' typo's can be spotted with:
a="A-Za-z0-9_"
git-grep "\![^$a()]*[$a]\+\([$a.]*\|->\)*\(\[[$a.]*\]\)\?[ \W]*&[^&]\+"

--
        Fix priority mistakes similar to '!x & y'
    
        Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
index cf70522..14141a5 100644
--- a/drivers/net/e1000e/82571.c
+++ b/drivers/net/e1000e/82571.c
@@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		/* quad ports only support WoL on port A */
 		if (adapter->flags & FLAG_IS_QUAD_PORT &&
-		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
+		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;
 
diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index 074055e..e3eca6d 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -6407,7 +6407,7 @@ static int airo_set_encode(struct net_device *dev,
 			set_wep_key(local, index, NULL, 0, perm, 1);
 		} else
 			/* Don't complain if only change the mode */
-			if(!dwrq->flags & IW_ENCODE_MODE) {
+			if(!(dwrq->flags & IW_ENCODE_MODE)) {
 				return -EINVAL;
 			}
 	}
diff --git a/drivers/net/wireless/atmel.c b/drivers/net/wireless/atmel.c
index 059ce3f..57cc7e5 100644
--- a/drivers/net/wireless/atmel.c
+++ b/drivers/net/wireless/atmel.c
@@ -1759,7 +1759,7 @@ static int atmel_set_encode(struct net_device *dev,
 			priv->default_key = index;
 		} else
 			/* Don't complain if only change the mode */
-			if (!dwrq->flags & IW_ENCODE_MODE) {
+			if (!(dwrq->flags & IW_ENCODE_MODE)) {
 				return -EINVAL;
 			}
 	}
diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
index c6f5aa3..d93438c 100644
--- a/drivers/net/wireless/libertas/wext.c
+++ b/drivers/net/wireless/libertas/wext.c
@@ -1380,7 +1380,7 @@ static int wlan_get_encodeext(struct net_device *dev,
 		index = adapter->wep_tx_keyidx;
 	}
 
-	if (!ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY &&
+	if (!(ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY) &&
 	    ext->alg != IW_ENCODE_ALG_WEP) {
 		if (index != 0 || adapter->mode != IW_MODE_INFRA)
 			goto out;
diff --git a/drivers/net/wireless/p54common.c b/drivers/net/wireless/p54common.c
index 1437db0..8ee1453 100644
--- a/drivers/net/wireless/p54common.c
+++ b/drivers/net/wireless/p54common.c
@@ -374,7 +374,7 @@ static void p54_rx_frame_sent(struct ieee80211_hw *dev, struct sk_buff *skb)
 			if ((entry_hdr->magic1 & cpu_to_le16(0x4000)) != 0)
 				pad = entry_data->align[0];
 
-			if (!status.control.flags & IEEE80211_TXCTL_NO_ACK) {
+			if (!(status.control.flags & IEEE80211_TXCTL_NO_ACK)) {
 				if (!(payload->status & 0x01))
 					status.flags |= IEEE80211_TX_STATUS_ACK;
 				else
diff --git a/drivers/net/wireless/prism54/isl_ioctl.c b/drivers/net/wireless/prism54/isl_ioctl.c
index 6d80ca4..b9d0073 100644
--- a/drivers/net/wireless/prism54/isl_ioctl.c
+++ b/drivers/net/wireless/prism54/isl_ioctl.c
@@ -1118,7 +1118,7 @@ prism54_set_encode(struct net_device *ndev, struct iw_request_info *info,
 			    mgt_set_request(priv, DOT11_OID_DEFKEYID, 0,
 					    &index);
 		} else {
-			if (!dwrq->flags & IW_ENCODE_MODE) {
+			if (!(dwrq->flags & IW_ENCODE_MODE)) {
 				/* we cannot do anything. Complain. */
 				return -EINVAL;
 			}
@@ -2610,7 +2610,7 @@ prism2_ioctl_set_encryption(struct net_device *dev,
 			    mgt_set_request(priv, DOT11_OID_DEFKEYID, 0,
 					    &index);
 		} else {
-			if (!param->u.crypt.flags & IW_ENCODE_MODE) {
+			if (!(param->u.crypt.flags & IW_ENCODE_MODE)) {
 				/* we cannot do anything. Complain. */
 				return -EINVAL;
 			}
diff --git a/drivers/net/wireless/zd1211rw/zd_rf_uw2453.c b/drivers/net/wireless/zd1211rw/zd_rf_uw2453.c
index 857dcf3..3c1cca4 100644
--- a/drivers/net/wireless/zd1211rw/zd_rf_uw2453.c
+++ b/drivers/net/wireless/zd1211rw/zd_rf_uw2453.c
@@ -403,7 +403,7 @@ static int uw2453_init_hw(struct zd_rf *rf)
 		if (r)
 			return r;
 
-		if (!intr_status & 0xf) {
+		if (!(intr_status & 0xf)) {
 			dev_dbg_f(zd_chip_dev(chip),
 				"PLL locked on configuration %d\n", i);
 			found_config = i;
diff --git a/net/ieee80211/ieee80211_wx.c b/net/ieee80211/ieee80211_wx.c
index d309e8f..623489a 100644
--- a/net/ieee80211/ieee80211_wx.c
+++ b/net/ieee80211/ieee80211_wx.c
@@ -709,7 +709,7 @@ int ieee80211_wx_get_encodeext(struct ieee80211_device *ieee,
 	} else
 		idx = ieee->tx_keyidx;
 
-	if (!ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY &&
+	if (!(ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY) &&
 	    ext->alg != IW_ENCODE_ALG_WEP)
 		if (idx != 0 || ieee->iw_mode != IW_MODE_INFRA)
 			return -EINVAL;

^ permalink raw reply related

* Re: stateless 1:1 NAT
From: Florin Andrei @ 2007-10-26 17:49 UTC (permalink / raw)
  To: netdev
In-Reply-To: <4716661B.3020301@trash.net>

Patrick McHardy wrote:
> Florin Andrei wrote:
>> OpenBSD 4.1 as a firewall fails even worse in this test case (it 
>> freezes instantly).
>> OpenBSD 4.2 works fine under the UDP flood, as if nothing happened.
> 
> And Linux 2.6.23? :)

Same as 2.6.18, actually maybe a little bit worse than .18: the current 
download does not even complete, and of course a new one doesn't start.

I may test 2.6.24 and stateless 1:1 NAT and we'll see what happens. I've 
been told that stateless 1:1 NAT is already in the .24_rc1 so I may test 
that.

-- 
Florin Andrei

http://florin.myip.org/

^ permalink raw reply

* Re: [PATCH] dev_change_name: ignore changes to same name
From: Rick Jones @ 2007-10-26 17:51 UTC (permalink / raw)
  To: David Miller; +Cc: shemminger, netdev
In-Reply-To: <20071026.035352.114927735.davem@davemloft.net>

David Miller wrote:
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Wed, 24 Oct 2007 06:44:45 -0700
> 
> 
>>Prevent error/backtrace from dev_rename() when changing
>>name of network device to the same name. This is a common
>>situation with udev and other scripts that bind addr to device.
>>
>>Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> 
> Fair enough, applied.

Very timely!  I'd just built/booted a 2.6.24-rc1 kernel yesterday and it spent 
quite a  long time with stack traces (I have 16 interfaces on the system and a 
9600 baud console...I can only imagine what it would have been like on a big 
system...).  With the patch applied to that tree, it is now down to only three 
or so stack traces related to renames that were not there on 2.6.23.mumble.  Not 
sure if that warrants an acked-by but feel free if it makes sense.  (I'm still 
really fuzzy on those, even after reading SubmittingPatches)

rick jones

^ permalink raw reply

* Re: [PATCH 1] net: fix and typo's
From: Kok, Auke @ 2007-10-26 17:51 UTC (permalink / raw)
  To: Roel Kluin; +Cc: netdev, Jeff Garzik
In-Reply-To: <47221EBE.3070109@tiscali.nl>

Roel Kluin wrote:
> A few patches with changes to net code. I have sent these to the lkml
> previously, but they were not yet merged. I am fairly new to kernel 
> programming, so it is possible that I make some mistakes. I'll explain my
> rationale, please nack if incorrect, an additional bit of explanation is
> appreciated even more.
> 
> The condition '!x & y,' does make little sense: the '!' has a higher
> priority than '&'. It behaves therefore like '!x && y'. In the case 
> bitanding flags, however, '!(x & y)' appears to be desired.
> 
> Warning: the change of '!x & y,' to '!(x & y)' may change behavior. if
> not desired, I propose changing this to '!x && y', to make it explicitly
> clear.
> 
> These '&' typo's can be spotted with:
> a="A-Za-z0-9_"
> git-grep "\![^$a()]*[$a]\+\([$a.]*\|->\)*\(\[[$a.]*\]\)\?[ \W]*&[^&]\+"
> 
> --
>         Fix priority mistakes similar to '!x & y'
>     
>         Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
> ---
> diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
> index cf70522..14141a5 100644
> --- a/drivers/net/e1000e/82571.c
> +++ b/drivers/net/e1000e/82571.c
> @@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
>  			adapter->flags &= ~FLAG_HAS_WOL;
>  		/* quad ports only support WoL on port A */
>  		if (adapter->flags & FLAG_IS_QUAD_PORT &&
> -		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
> +		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
>  			adapter->flags &= ~FLAG_HAS_WOL;
>  		break;
>  



Ack this e1000e change here!


Auke


(PS since there was only 1 netdriver patch here and the rest is wireless, I would
have suggested splitting this patch up in two and sending them to the wireless
maintainer and netdevice maintainer separately. But I'm sure this will get picked
up anyway.)

^ permalink raw reply

* Re: 2.6.24-rc1 fails with lockup - /sbin/ifconfig / inet_ioctl() / dev_close() / rtl8169_down()
From: Ingo Molnar @ 2007-10-26 17:56 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Francois Romieu, Romano Giannetti, Peter Zijlstra,
	Linux Kernel Mailing List, David S. Miller, netdev, Edward Hsu,
	Jeff Garzik, Andrew Morton
In-Reply-To: <20071026094833.539a69aa@freepuppy.rosehill>


* Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> > hm, from your log it appears that lockdep did not find anything, 
> > still the hang does trigger.
> > 
> > it's /sbin/ifconfig and inet_ioctl() / dev_close() / rtl8169_down() 
> > that seems to be hanging. I've extracted the relevant backtrace 
> > below. I've Cc:-ed people who might have a better idea about what's 
> > going on.
> 
> Are you building with NAPI enabled or not. Looks like the following 
> might help the non-napi case.

the config from:

  http://www.dea.icai.upcomillas.es/romano/linux/info/2624rc1_1/

suggests that NAPI was disabled:

  CONFIG_R8169=m
  # CONFIG_R8169_NAPI is not set
  CONFIG_R8169_VLAN=y

	Ingo

^ permalink raw reply

* [PATCH 4] Unlock before BUG, but preserve normal operation
From: Roel Kluin @ 2007-10-26 18:00 UTC (permalink / raw)
  To: netdev

I am a bit uncertain about this, so if you have comments on this, it is much
appreciated.

The rationale:
in the default case the BUG() kills the current process, but with the lock
still held this causes the system to hang; therefore the unlock before the
BUG(). 

When CONFIG_BUG is disabled - e.g. for embedded systems - the BUG(); will
not kill the process and there should not be a double unlock in that case.
--
    Unlock before BUG(), but preserve normal operation in the case that
    CONFIG_BUG is disabled.
    
    Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index e3c8284..f337578 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -8722,7 +8722,9 @@ static int ipw_wx_get_freq(struct net_device *dev,
 			break;
 
 		default:
+			mutex_unlock(&priv->mutex);
 			BUG();
+			return -EINVAL;
 		}
 	} else
 		wrqu->freq.m = 0;
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 657ee69..e551b0b 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -752,8 +752,11 @@ all_acked:
 		sp->call = call;
 		rxrpc_get_call(call);
 		spin_lock_bh(&call->lock);
-		if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0)
+		if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0) {
+			spin_unlock_bh(&call->lock);
 			BUG();
+			goto process_further;
+		}
 		spin_unlock_bh(&call->lock);
 		goto process_further;
 	}
diff --git a/net/rxrpc/ar-call.c b/net/rxrpc/ar-call.c
index 3c04b00..48804e1 100644
--- a/net/rxrpc/ar-call.c
+++ b/net/rxrpc/ar-call.c
@@ -426,9 +426,12 @@ void rxrpc_release_call(struct rxrpc_call *call)
 	       call->rx_first_oos);
 
 	spin_lock_bh(&call->lock);
-	if (test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags))
+	if (test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags)) {
+		spin_unlock_bh(&call->lock);
 		BUG();
-	spin_unlock_bh(&call->lock);
+	} else {
+		spin_unlock_bh(&call->lock);
+	}
 
 	/* dissociate from the socket
 	 * - the socket's ref on the call is passed to the death timer


^ permalink raw reply related

* Re: [PATCH 1] net: fix and typo's
From: Stephen Hemminger @ 2007-10-26 18:25 UTC (permalink / raw)
  To: Roel Kluin; +Cc: netdev
In-Reply-To: <47221EBE.3070109@tiscali.nl>

On Fri, 26 Oct 2007 19:07:10 +0200
Roel Kluin <12o3l@tiscali.nl> wrote:

> A few patches with changes to net code. I have sent these to the lkml
> previously, but they were not yet merged. I am fairly new to kernel 
> programming, so it is possible that I make some mistakes. I'll explain my
> rationale, please nack if incorrect, an additional bit of explanation is
> appreciated even more.
> 
> The condition '!x & y,' does make little sense: the '!' has a higher
> priority than '&'. It behaves therefore like '!x && y'. In the case 
> bitanding flags, however, '!(x & y)' appears to be desired.
> 
> Warning: the change of '!x & y,' to '!(x & y)' may change behavior. if
> not desired, I propose changing this to '!x && y', to make it explicitly
> clear.
> 
> These '&' typo's can be spotted with:
> a="A-Za-z0-9_"
> git-grep "\![^$a()]*[$a]\+\([$a.]*\|->\)*\(\[[$a.]*\]\)\?[ \W]*&[^&]\+"
> 
> --
>         Fix priority mistakes similar to '!x & y'
>     
>         Signed-off-by: Roel Kluin <12o3l@tiscali.nl>

I agree these look good, but maybe they should be broken into separate patches to
make bisection of any induced regressions easier.

Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* [PATCH 5, last for now]  net: unlock before return
From: Roel Kluin @ 2007-10-26 18:32 UTC (permalink / raw)
  To: netdev

To David Miller: this was the one you asked to send. I removed the incorrect one
(net/bridge/netfilter/ebtables.c). Note the drivers/net/cris/eth_v10.c, which
you may not yet have reviewed, But I checked, and there was not a similar
comment above the header of the function :)

Roel

--
     unlock before return fixes
     Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/net/cris/eth_v10.c b/drivers/net/cris/eth_v10.c
index edd6828..5478549 100644
--- a/drivers/net/cris/eth_v10.c
+++ b/drivers/net/cris/eth_v10.c
@@ -1476,6 +1476,7 @@ e100_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 			e100_set_duplex(dev, autoneg);
 			break;
 		default:
+			spin_unlock(&np->lock);
 			return -EINVAL;
 	}
 	spin_unlock(&np->lock);
diff --git a/net/9p/mux.c b/net/9p/mux.c
index f140147..c9f0805 100644
--- a/net/9p/mux.c
+++ b/net/9p/mux.c
@@ -222,8 +222,10 @@ static int p9_mux_poll_start(struct p9_conn *m)
 	}
 
 	if (i >= ARRAY_SIZE(p9_mux_poll_tasks)) {
-		if (vptlast == NULL)
+		if (vptlast == NULL) {
+			mutex_unlock(&p9_mux_task_lock);
 			return -ENOMEM;
+		}
 
 		P9_DPRINTK(P9_DEBUG_MUX, "put in proc %d\n", i);
 		list_add(&m->mux_list, &vptlast->mux_list);
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 9be1826..cf18097 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1079,7 +1079,7 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb,
 						    CTA_TUPLE_MASTER,
 						    u3);
 			if (err < 0)
-				return err;
+				goto out_unlock;
 
 			master_h = __nf_conntrack_find(&master, NULL);
 			if (master_h == NULL) {
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 509defe..859fdc0 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -750,8 +750,10 @@ static int rose_connect(struct socket *sock, struct sockaddr *uaddr, int addr_le
 
 	rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
 					 &diagnostic);
-	if (!rose->neighbour)
-		return -ENETUNREACH;
+	if (!rose->neighbour) {
+		err = -ENETUNREACH;
+		goto out_release;
+	}
 
 	rose->lci = rose_new_lci(rose->neighbour);
 	if (!rose->lci) {

^ permalink raw reply related

* [PATCH] r8169: don't call napi_disable if not doing NAPI
From: Stephen Hemminger @ 2007-10-26 18:33 UTC (permalink / raw)
  To: Ingo Molnar, Francois Romieu, Jeff Garzik
  Cc: Romano Giannetti, Peter Zijlstra, Linux Kernel Mailing List,
	David S. Miller, netdev, Edward Hsu, Andrew Morton
In-Reply-To: <20071026175613.GA18770@elte.hu>

Don't call napi_disable if not configured.
And make sure that any misuse of napi_xxx in future fails
with a compile error.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/drivers/net/r8169.c	2007-10-24 21:38:43.000000000 -0700
+++ b/drivers/net/r8169.c	2007-10-26 11:27:02.000000000 -0700
@@ -392,7 +392,9 @@ struct rtl8169_private {
 	void __iomem *mmio_addr;	/* memory map physical address */
 	struct pci_dev *pci_dev;	/* Index of PCI device */
 	struct net_device *dev;
+#ifdef CONFIG_R8169_NAPI
 	struct napi_struct napi;
+#endif
 	spinlock_t lock;		/* spin lock flag */
 	u32 msg_enable;
 	int chipset;
@@ -2989,13 +2991,16 @@ static void rtl8169_down(struct net_devi
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
 	void __iomem *ioaddr = tp->mmio_addr;
-	unsigned int poll_locked = 0;
 	unsigned int intrmask;
 
 	rtl8169_delete_timer(dev);
 
 	netif_stop_queue(dev);
 
+#ifdef CONFIG_R8169_NAPI
+	napi_disable(&tp->napi);
+#endif
+
 core_down:
 	spin_lock_irq(&tp->lock);
 
@@ -3009,11 +3014,6 @@ core_down:
 
 	synchronize_irq(dev->irq);
 
-	if (!poll_locked) {
-		napi_disable(&tp->napi);
-		poll_locked++;
-	}
-
 	/* Give a racing hard_start_xmit a few cycles to complete. */
 	synchronize_sched();  /* FIXME: should this be synchronize_irq()? */
 

^ permalink raw reply

* Re: [PATCH 1] net: fix and typo's
From: Roel Kluin @ 2007-10-26 19:54 UTC (permalink / raw)
  To: Kok, Auke; +Cc: netdev, Jeff Garzik
In-Reply-To: <47222932.80408@intel.com>

Kok, Auke wrote:

> Ack this e1000e change here!

> (PS since there was only 1 netdriver patch here and the rest is wireless, I would
> have suggested splitting this patch up in two and sending them to the wireless
> maintainer and netdevice maintainer separately. But I'm sure this will get picked
> up anyway.)

Ok, I've split the patch and sent the rest to wireless. Here's this single fix again
you acked.

--
        Fix priority mistakes similar to '!x & y'
    
        Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
index cf70522..14141a5 100644
--- a/drivers/net/e1000e/82571.c
+++ b/drivers/net/e1000e/82571.c
@@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		/* quad ports only support WoL on port A */
 		if (adapter->flags & FLAG_IS_QUAD_PORT &&
-		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
+		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;
 

^ permalink raw reply related

* Re: [PATCH 1] net: fix and typo's
From: Kok, Auke @ 2007-10-26 20:06 UTC (permalink / raw)
  To: Roel Kluin; +Cc: netdev, Jeff Garzik
In-Reply-To: <4722460E.4010201@tiscali.nl>

Roel Kluin wrote:
> Kok, Auke wrote:
> 
>> Ack this e1000e change here!
> 
>> (PS since there was only 1 netdriver patch here and the rest is wireless, I would
>> have suggested splitting this patch up in two and sending them to the wireless
>> maintainer and netdevice maintainer separately. But I'm sure this will get picked
>> up anyway.)
> 
> Ok, I've split the patch and sent the rest to wireless. Here's this single fix again
> you acked.

OK, bedankt Roel!

I'll forward this to Jeff for merging.

Cheers,

Auke



> --
>         Fix priority mistakes similar to '!x & y'
>     
>         Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
> ---
> diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
> index cf70522..14141a5 100644
> --- a/drivers/net/e1000e/82571.c
> +++ b/drivers/net/e1000e/82571.c
> @@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
>  			adapter->flags &= ~FLAG_HAS_WOL;
>  		/* quad ports only support WoL on port A */
>  		if (adapter->flags & FLAG_IS_QUAD_PORT &&
> -		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
> +		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
>  			adapter->flags &= ~FLAG_HAS_WOL;
>  		break;
>  

^ permalink raw reply

* Re: [PATCH] r8169: don't call napi_disable if not doing NAPI
From: Francois Romieu @ 2007-10-26 20:17 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Ingo Molnar, Jeff Garzik, Romano Giannetti, Peter Zijlstra,
	Linux Kernel Mailing List, David S. Miller, netdev, Edward Hsu,
	Andrew Morton
In-Reply-To: <20071026113305.57dea765@freepuppy.rosehill>

Stephen Hemminger <shemminger@linux-foundation.org> :
> Don't call napi_disable if not configured.
> And make sure that any misuse of napi_xxx in future fails
> with a compile error.

Disable napi polling early and remove the useless poll_locked logic.

> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

Acked-off-by: Francois Romieu <romieu@fr.zoreil.com>

-- 
Ueimor

^ permalink raw reply

* Re: [0/3] Distributed storage. Mirror algo extension for automatic recovery.
From: Andrew Morton @ 2007-10-26 20:55 UTC (permalink / raw)
  To: Evgeniy Polyakov; +Cc: netdev, linux-kernel, linux-fsdevel
In-Reply-To: <20071018191741.GA848@2ka.mipt.ru>

On Thu, 18 Oct 2007 23:17:41 +0400
Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:

> I'm pleased to announce sixth release of the distributed storage
> subsystem, which allows to form a storage on top of remote and local
> nodes, which in turn can be exported to another storage as a node to
> form tree-like storages.

I went back and re-read last month's discussion and I'm not seeing any
reason why we shouldn't start thinking about merging this.

How close is it to that stage?  A peek at your development blog indicates
that things are still changing at a moderate rate?


^ permalink raw reply

* Files, sockets, and closing
From: Stephen Hemminger @ 2007-10-26 21:03 UTC (permalink / raw)
  To: David S. Miller, Al Viro; +Cc: netdev

Looking at this bug:
http://bugzilla.kernel.org/show_bug.cgi?id=9149

Exposes some rather deep issues in the filesystem/socket/inet/tcp
layering. It seems that sys_close() zaps the file table entry, but
since each thread has a separate reference, the actual tcp_close()
doesn't happen until the last thread calls close/exits.

I am no VFS expert.
The semantically correct fix appears to be complex.
  * add a flush handle to the socket_file_ops
  * propagate the flush through socket to inet and tcp
  * split tcp_close into two parts.
     1) tcp_flush - flush buffers and wakeup all other threads on the socket 
     2) tcp_release - release last reference and cleanup.

Bogus patch for explanation purposes:

--- a/include/linux/net.h	2007-10-26 12:44:41.000000000 -0700
+++ b/include/linux/net.h	2007-10-26 13:56:39.000000000 -0700
@@ -128,6 +128,7 @@ struct proto_ops {
 	int		family;
 	struct module	*owner;
 	int		(*release)   (struct socket *sock);
+	void		(*flush)     (struct socket *sock);
 	int		(*bind)	     (struct socket *sock,
 				      struct sockaddr *myaddr,
 				      int sockaddr_len);
--- a/include/net/tcp.h	2007-10-26 12:44:42.000000000 -0700
+++ b/include/net/tcp.h	2007-10-26 13:06:53.000000000 -0700
@@ -367,6 +367,7 @@ extern void			tcp_enter_loss(struct sock
 extern void			tcp_clear_retrans(struct tcp_sock *tp);
 extern void			tcp_update_metrics(struct sock *sk);
 
+extern void			tcp_flush(struct sock *sk);
 extern void			tcp_close(struct sock *sk, 
 					  long timeout);
 extern unsigned int		tcp_poll(struct file * file, struct socket *sock, struct poll_table_struct *wait);
--- a/net/ipv4/af_inet.c	2007-10-26 12:44:46.000000000 -0700
+++ b/net/ipv4/af_inet.c	2007-10-26 13:56:00.000000000 -0700
@@ -386,6 +386,14 @@ out_rcu_unlock:
 	goto out;
 }
 
+/* The file handle is closed, but not all threads maybe gone */
+void inet_flush(struct socket *sock)
+{
+	struct sock *sk = sock->sk;
+
+	if (sk && sk->sk_prot->flush)
+		sk->sk_prot->flush(sk);
+}
 
 /*
  *	The peer socket should always be NULL (or else). When we call this
@@ -822,6 +830,7 @@ int inet_ioctl(struct socket *sock, unsi
 const struct proto_ops inet_stream_ops = {
 	.family		   = PF_INET,
 	.owner		   = THIS_MODULE,
+	.flush		   = inet_flush,
 	.release	   = inet_release,
 	.bind		   = inet_bind,
 	.connect	   = inet_stream_connect,
--- a/net/ipv4/tcp.c	2007-10-26 12:44:46.000000000 -0700
+++ b/net/ipv4/tcp.c	2007-10-26 14:00:42.000000000 -0700
@@ -1557,11 +1557,10 @@ void tcp_shutdown(struct sock *sk, int h
 	}
 }
 
-void tcp_close(struct sock *sk, long timeout)
+void tcp_flush(struct sock *sk)
 {
 	struct sk_buff *skb;
 	int data_was_unread = 0;
-	int state;
 
 	lock_sock(sk);
 	sk->sk_shutdown = SHUTDOWN_MASK;
@@ -1572,7 +1571,7 @@ void tcp_close(struct sock *sk, long tim
 		/* Special case. */
 		inet_csk_listen_stop(sk);
 
-		goto adjudge_to_death;
+		return;
 	}
 
 	/*  We need to flush the recv. buffs.  We do this only on the
@@ -1632,10 +1631,16 @@ void tcp_close(struct sock *sk, long tim
 		 */
 		tcp_send_fin(sk);
 	}
+	release_sock(sk);
+}
 
+void tcp_close(struct sock *sk, long timeout)
+{
+	int state;
+
+	lock_sock(sk);
 	sk_stream_wait_close(sk, timeout);
 
-adjudge_to_death:
 	state = sk->sk_state;
 	sock_hold(sk);
 	sock_orphan(sk);
@@ -2524,6 +2529,7 @@ void __init tcp_init(void)
 	tcp_register_congestion_control(&tcp_reno);
 }
 
+EXPORT_SYMBOL(tcp_flush);
 EXPORT_SYMBOL(tcp_close);
 EXPORT_SYMBOL(tcp_disconnect);
 EXPORT_SYMBOL(tcp_getsockopt);
--- a/net/ipv4/tcp_ipv4.c	2007-10-26 12:44:46.000000000 -0700
+++ b/net/ipv4/tcp_ipv4.c	2007-10-26 13:06:04.000000000 -0700
@@ -2420,6 +2420,7 @@ void tcp4_proc_exit(void)
 struct proto tcp_prot = {
 	.name			= "TCP",
 	.owner			= THIS_MODULE,
+	.flush			= tcp_flush,
 	.close			= tcp_close,
 	.connect		= tcp_v4_connect,
 	.disconnect		= tcp_disconnect,
--- a/net/ipv6/tcp_ipv6.c	2007-10-26 12:44:47.000000000 -0700
+++ b/net/ipv6/tcp_ipv6.c	2007-10-26 13:06:25.000000000 -0700
@@ -2109,6 +2109,7 @@ void tcp6_proc_exit(void)
 struct proto tcpv6_prot = {
 	.name			= "TCPv6",
 	.owner			= THIS_MODULE,
+	.flush			= tcp_flush,
 	.close			= tcp_close,
 	.connect		= tcp_v6_connect,
 	.disconnect		= tcp_disconnect,
--- a/net/socket.c	2007-10-26 12:44:48.000000000 -0700
+++ b/net/socket.c	2007-10-26 13:47:48.000000000 -0700
@@ -100,7 +100,7 @@ static ssize_t sock_aio_read(struct kioc
 static ssize_t sock_aio_write(struct kiocb *iocb, const struct iovec *iov,
 			  unsigned long nr_segs, loff_t pos);
 static int sock_mmap(struct file *file, struct vm_area_struct *vma);
-
+static int sock_flush(struct file *file, fl_owner_t id);
 static int sock_close(struct inode *inode, struct file *file);
 static unsigned int sock_poll(struct file *file,
 			      struct poll_table_struct *wait);
@@ -130,6 +130,7 @@ static const struct file_operations sock
 #endif
 	.mmap =		sock_mmap,
 	.open =		sock_no_open,	/* special open code to disallow open via /proc */
+	.flush =	sock_flush,
 	.release =	sock_close,
 	.fasync =	sock_fasync,
 	.sendpage =	sock_sendpage,
@@ -961,6 +962,15 @@ static int sock_mmap(struct file *file, 
 	return sock->ops->mmap(file, sock, vma);
 }
 
+static int sock_flush(struct file *file, fl_owner_t id)
+{
+	struct socket *sock = file->private_data;
+
+	if (sock->ops && sock->ops->flush)
+		sock->ops->flush(sock);
+	return 0;
+}
+
 static int sock_close(struct inode *inode, struct file *filp)
 {
 	/*
--- a/include/net/sock.h	2007-10-26 12:44:42.000000000 -0700
+++ b/include/net/sock.h	2007-10-26 13:50:03.000000000 -0700
@@ -515,6 +515,7 @@ struct timewait_sock_ops;
 struct proto {
 	void			(*close)(struct sock *sk, 
 					long timeout);
+	void			(*flush)(struct sock *sk);
 	int			(*connect)(struct sock *sk,
 				        struct sockaddr *uaddr, 
 					int addr_len);

^ permalink raw reply

* [PATCH] 2.6.24-rc1 remove architecture warning compiling fealnx on ia64
From: Rick Jones @ 2007-10-26 21:04 UTC (permalink / raw)
  To: netdev

The likelihood of one of these being used on an ia64 box is epsilon
but it would still be nice to get rid of the warning:

 #warning Processor architecture undefined! 

when compiling on same. So, pick some likely similar architectures
and follow those leads. Compile tested only.

Signed-off-by: Rick Jones <rick.jones2@hp.com>

---
diff -r 35e54d4beaad drivers/net/fealnx.c
--- a/drivers/net/fealnx.c	Wed Oct 24 05:06:40 2007 +0000
+++ b/drivers/net/fealnx.c	Fri Oct 26 06:12:00 2007 -0700
@@ -866,7 +866,7 @@ static int netdev_open(struct net_device
 //   np->bcrvalue=0x04 | 0x0x38;  /* big-endian, 256 burst length */
 	np->bcrvalue = 0x04 | 0x10;	/* big-endian, tx 8 burst length */
 	np->crvalue = 0xe00;	/* rx 128 burst length */
-#elif defined(__alpha__) || defined(__x86_64__)
+#elif defined(__alpha__) || defined(__x86_64__) || defined(__ia64__)
 // 89/9/1 modify,
 //   np->bcrvalue=0x38;           /* little-endian, 256 burst length */
 	np->bcrvalue = 0x10;	/* little-endian, 8 burst length */

^ permalink raw reply

* [PATCH v2] 2.6.24-rc1 remove architecture warning compiling fealnx on ia64
From: Jeff Garzik @ 2007-10-26 21:27 UTC (permalink / raw)
  To: Rick Jones; +Cc: netdev, Francois Romieu
In-Reply-To: <200710262104.OAA14012@tardy.cup.hp.com>

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

Rick Jones wrote:
> The likelihood of one of these being used on an ia64 box is epsilon
> but it would still be nice to get rid of the warning:
> 
>  #warning Processor architecture undefined! 
> 
> when compiling on same. So, pick some likely similar architectures
> and follow those leads. Compile tested only.
> 
> Signed-off-by: Rick Jones <rick.jones2@hp.com>
> 
> ---
> diff -r 35e54d4beaad drivers/net/fealnx.c
> --- a/drivers/net/fealnx.c	Wed Oct 24 05:06:40 2007 +0000
> +++ b/drivers/net/fealnx.c	Fri Oct 26 06:12:00 2007 -0700
> @@ -866,7 +866,7 @@ static int netdev_open(struct net_device
>  //   np->bcrvalue=0x04 | 0x0x38;  /* big-endian, 256 burst length */
>  	np->bcrvalue = 0x04 | 0x10;	/* big-endian, tx 8 burst length */
>  	np->crvalue = 0xe00;	/* rx 128 burst length */
> -#elif defined(__alpha__) || defined(__x86_64__)
> +#elif defined(__alpha__) || defined(__x86_64__) || defined(__ia64__)
>  // 89/9/1 modify,
>  //   np->bcrvalue=0x38;           /* little-endian, 256 burst length */
>  	np->bcrvalue = 0x10;	/* little-endian, 8 burst length */

Overall that poor driver is more than a little bit ugly :)

Looking at the large chain of highly-similar values, I would tend to 
prefer the attached patch...

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2354 bytes --]

diff --git a/drivers/net/fealnx.c b/drivers/net/fealnx.c
index 7bb9c72..3c1364d 100644
--- a/drivers/net/fealnx.c
+++ b/drivers/net/fealnx.c
@@ -90,6 +90,7 @@ static int full_duplex[MAX_UNITS] = { -1, -1, -1, -1, -1, -1, -1, -1 };
 #include <asm/processor.h>	/* Processor type for cache alignment. */
 #include <asm/io.h>
 #include <asm/uaccess.h>
+#include <asm/byteorder.h>
 
 /* These identify the driver base version and may not be removed. */
 static char version[] =
@@ -861,40 +862,20 @@ static int netdev_open(struct net_device *dev)
 	   Wait the specified 50 PCI cycles after a reset by initializing
 	   Tx and Rx queues and the address filter list.
 	   FIXME (Ueimor): optimistic for alpha + posted writes ? */
-#if defined(__powerpc__) || defined(__sparc__)
-// 89/9/1 modify,
-//   np->bcrvalue=0x04 | 0x0x38;  /* big-endian, 256 burst length */
-	np->bcrvalue = 0x04 | 0x10;	/* big-endian, tx 8 burst length */
-	np->crvalue = 0xe00;	/* rx 128 burst length */
-#elif defined(__alpha__) || defined(__x86_64__)
-// 89/9/1 modify,
-//   np->bcrvalue=0x38;           /* little-endian, 256 burst length */
-	np->bcrvalue = 0x10;	/* little-endian, 8 burst length */
-	np->crvalue = 0xe00;	/* rx 128 burst length */
-#elif defined(__i386__)
-#if defined(MODULE)
-// 89/9/1 modify,
-//   np->bcrvalue=0x38;           /* little-endian, 256 burst length */
+
 	np->bcrvalue = 0x10;	/* little-endian, 8 burst length */
-	np->crvalue = 0xe00;	/* rx 128 burst length */
-#else
-	/* When not a module we can work around broken '486 PCI boards. */
-#define x86 boot_cpu_data.x86
-// 89/9/1 modify,
-//   np->bcrvalue=(x86 <= 4 ? 0x10 : 0x38);
-	np->bcrvalue = 0x10;
-	np->crvalue = (x86 <= 4 ? 0xa00 : 0xe00);
-	if (x86 <= 4)
-		printk(KERN_INFO "%s: This is a 386/486 PCI system, setting burst "
-		       "length to %x.\n", dev->name, (x86 <= 4 ? 0x10 : 0x38));
+#ifdef __BIG_ENDIAN
+	np->bcrvalue |= 0x04;	/* big-endian */
 #endif
-#else
-// 89/9/1 modify,
-//   np->bcrvalue=0x38;
-	np->bcrvalue = 0x10;
-	np->crvalue = 0xe00;	/* rx 128 burst length */
-#warning Processor architecture undefined!
+
+#if defined(__i386__) && !defined(MODULE)
+	if (boot_cpu_data.x86 <= 4)
+		np->crvalue = 0xa00;
+	else
 #endif
+		np->crvalue = 0xe00;	/* rx 128 burst length */
+
+
 // 89/12/29 add,
 // 90/1/16 modify,
 //   np->imrvalue=FBE|TUNF|CNTOVF|RBU|TI|RI;

^ permalink raw reply related

* Re: [PATCH v2] 2.6.24-rc1 remove architecture warning compiling fealnx on ia64
From: Rick Jones @ 2007-10-26 21:38 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, Francois Romieu
In-Reply-To: <47225BAD.4080801@garzik.org>

> Overall that poor driver is more than a little bit ugly :)

Well, given some of the code I've put into netperf over the years I wasn't going 
to be the first to cast a stone :) but I won't disagree with the assesment :)

> Looking at the large chain of highly-similar values, I would tend to 
> prefer the attached patch...

That achieves my desired end result of no warning while compiling on ia64 so I'm 
happy with it.  I suspect that may also get rid of a warning on parisc so I 
should be doubly pleased :)

rick jones


^ permalink raw reply

* Re: Files, sockets, and closing
From: Al Viro @ 2007-10-26 21:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, netdev
In-Reply-To: <20071026140319.5bcc8ac0@freepuppy.rosehill>

On Fri, Oct 26, 2007 at 02:03:19PM -0700, Stephen Hemminger wrote:
> Looking at this bug:
> http://bugzilla.kernel.org/show_bug.cgi?id=9149
> 
> Exposes some rather deep issues in the filesystem/socket/inet/tcp
> layering. It seems that sys_close() zaps the file table entry, but
> since each thread has a separate reference, the actual tcp_close()
> doesn't happen until the last thread calls close/exits.

No.  It's not about that at all.  Threads in his case _share_ descriptor
table and the thing he's complaining about is that another thread has
removed the descriptor from their (shared) descriptor table and he's
not getting notified.  It's not about struct file (or socket) at all;
it's all on descriptor level.

The reference to struct file is held by accept() itself, _not_ by descriptor
table.  And he would have the same problem if the opened socket had been
inherited from parent (and still opened by it) - it's really not about
the damn thing getting shut down, etc.

What happens is that there is a mapping from descriptors to opened files,
a reference to opened file is obtained by it once per syscall and that file
remains open at least until the end of syscall.   Whether the descriptor
you've passed remains refering to the same file is up to userland code.
If you have another thread and that thread rips the descriptor out of your
shared descriptor table, it's your responsibility to keep them sane and
happy.

close() from another thread is not a way to abort blocked accept().  Never
promised to be that.  Just as close() from another thread is not a way to
abort blocked write() or read() or sendmsg() or...

^ permalink raw reply

* Re: [PATCH] Remove pointless casts from void pointers,
From: Lennert Buytenhek @ 2007-10-26 21:58 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: LKML, akpm, rmk, kernel, tony.luck, jwboyer, benh, paulus,
	dmitry.torokhov, netdev, linux-scsi, linux-serial, linux-wireless,
	bryan.wu, adaplas
In-Reply-To: <9799624f63e093aa915947aea8fb1b8a5df959a1.1193390973.git.jeff@garzik.org>

On Fri, Oct 26, 2007 at 05:40:22AM -0400, Jeff Garzik wrote:

>  arch/arm/mach-pxa/ssp.c                    |    2 +-
>  arch/arm/mach-s3c2410/usb-simtec.c         |    2 +-
>  arch/arm/plat-omap/mailbox.c               |    2 +-

FWIW

Acked-by: Lennert Buytenhek <buytenh@wantstofly.org>

^ permalink raw reply

* Re: Files, sockets, and closing
From: Stephen Hemminger @ 2007-10-26 22:09 UTC (permalink / raw)
  To: Al Viro; +Cc: David S. Miller, netdev
In-Reply-To: <20071026214513.GG8181@ftp.linux.org.uk>

On Fri, 26 Oct 2007 22:45:13 +0100
Al Viro <viro@ftp.linux.org.uk> wrote:

> On Fri, Oct 26, 2007 at 02:03:19PM -0700, Stephen Hemminger wrote:
> > Looking at this bug:
> > http://bugzilla.kernel.org/show_bug.cgi?id=9149
> > 
> > Exposes some rather deep issues in the filesystem/socket/inet/tcp
> > layering. It seems that sys_close() zaps the file table entry, but
> > since each thread has a separate reference, the actual tcp_close()
> > doesn't happen until the last thread calls close/exits.
> 
> No.  It's not about that at all.  Threads in his case _share_ descriptor
> table and the thing he's complaining about is that another thread has
> removed the descriptor from their (shared) descriptor table and he's
> not getting notified.  It's not about struct file (or socket) at all;
> it's all on descriptor level.
> 
> The reference to struct file is held by accept() itself, _not_ by descriptor
> table.  And he would have the same problem if the opened socket had been
> inherited from parent (and still opened by it) - it's really not about
> the damn thing getting shut down, etc.
> 
> What happens is that there is a mapping from descriptors to opened files,
> a reference to opened file is obtained by it once per syscall and that file
> remains open at least until the end of syscall.   Whether the descriptor
> you've passed remains refering to the same file is up to userland code.
> If you have another thread and that thread rips the descriptor out of your
> shared descriptor table, it's your responsibility to keep them sane and
> happy.
> 
> close() from another thread is not a way to abort blocked accept().  Never
> promised to be that.  Just as close() from another thread is not a way to
> abort blocked write() or read() or sendmsg() or...

The problem is the Linux interpretation conflicts with the expectation
of applications that run on other Unix systems.  Most likely, it is
one of those corner cases not covered by SUS or Posix specs otherwise
it would have come up earlier. The existing Linux behavior works fine
it just isn't expected (or well documented).

I'm fine with just closing the bug (which is what I did initially), but
where should this get documented?

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* [PATCH] skye/skge: sparse fix - data can't ever be bigger than LONG_MAX / HZ
From: Auke Kok @ 2007-10-26 22:10 UTC (permalink / raw)
  To: shemminger; +Cc: netdev

Trivial replacement - use INT_MAX instead here.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Cc: shemminger@linux-foundation.org
---

 drivers/net/sk98lin/skethtool.c |    4 ++--
 drivers/net/skge.c              |    8 ++++----
 drivers/net/sky2.c              |    8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/sk98lin/skethtool.c b/drivers/net/sk98lin/skethtool.c
index 5a6da89..4549b97 100644
--- a/drivers/net/sk98lin/skethtool.c
+++ b/drivers/net/sk98lin/skethtool.c
@@ -430,8 +430,8 @@ static int locateDevice(struct net_device *dev, u32 data)
 	DEV_NET *pNet = netdev_priv(dev);
 	SK_AC *pAC = pNet->pAC;
 
-	if(!data || data > (u32)(MAX_SCHEDULE_TIMEOUT / HZ))
-		data = (u32)(MAX_SCHEDULE_TIMEOUT / HZ);
+	if (!data)
+		data = INT_MAX;
 
 	/* start blinking */
 	pAC->LedsOn = 0;
diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index b9961dc..696a79e 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -783,10 +783,10 @@ static int skge_phys_id(struct net_device *dev, u32 data)
 	unsigned long ms;
 	enum led_mode mode = LED_MODE_TST;
 
-	if (!data || data > (u32)(MAX_SCHEDULE_TIMEOUT / HZ))
-		ms = jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT / HZ) * 1000;
-	else
-		ms = data * 1000;
+	if (!data)
+		data = INT_MAX;
+
+	ms = data * HZ;
 
 	while (ms > 0) {
 		skge_led(skge, mode);
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index c27c7d6..1381d04 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -3336,10 +3336,10 @@ static int sky2_phys_id(struct net_device *dev, u32 data)
 	int interrupted;
 	int onoff = 1;
 
-	if (!data || data > (u32) (MAX_SCHEDULE_TIMEOUT / HZ))
-		ms = jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT);
-	else
-		ms = data * 1000;
+	if (!data)
+		data = INT_MAX:
+
+	ms = data * HZ;
 
 	/* save initial values */
 	spin_lock_bh(&sky2->phy_lock);

^ permalink raw reply related

* [PATCH] pcnet: fix sparse triviality
From: Auke Kok @ 2007-10-26 22:11 UTC (permalink / raw)
  To: pcnet32; +Cc: netdev

Since data can never exceed u32, it can't even be larger than LONG_MAX/HZ.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Cc: pcnet32@verizon.net
---

 drivers/net/pcnet32.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index ff92aca..3573e77 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -1101,9 +1101,8 @@ static int pcnet32_phys_id(struct net_device *dev, u32 data)
 	mod_timer(&lp->blink_timer, jiffies);
 	set_current_state(TASK_INTERRUPTIBLE);
 
-	/* AV: the limit here makes no sense whatsoever */
-	if ((!data) || (data > (u32) (MAX_SCHEDULE_TIMEOUT / HZ)))
-		data = (u32) (MAX_SCHEDULE_TIMEOUT / HZ);
+	if (!data)
+		data = INT_MAX;
 
 	msleep_interruptible(data * 1000);
 	del_timer_sync(&lp->blink_timer);

^ permalink raw reply related


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