netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 0/2] s390: qeth bug fixes for 2.6.38 next rc
@ 2011-01-13  6:42 frank.blaschka
  2011-01-13  6:42 ` [patch 1/2] [PATCH] qeth: postpone open till recovery is finished frank.blaschka
  2011-01-13  6:42 ` [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug frank.blaschka
  0 siblings, 2 replies; 6+ messages in thread
From: frank.blaschka @ 2011-01-13  6:42 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390

Hi Dave,

here are 2 qeth bug fixes for 2.6.38 next rc (net-2.6)

shortlog:

Ursula Braun (1)
qeth: postpone open till recovery is finished

Frank Blaschka (1)
qeth: l3 hw tx csum circumvent hw bug

Thanks,
        Frank

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 1/2] [PATCH] qeth: postpone open till recovery is finished
  2011-01-13  6:42 [patch 0/2] s390: qeth bug fixes for 2.6.38 next rc frank.blaschka
@ 2011-01-13  6:42 ` frank.blaschka
  2011-01-13  6:42 ` [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug frank.blaschka
  1 sibling, 0 replies; 6+ messages in thread
From: frank.blaschka @ 2011-01-13  6:42 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, Ursula Braun

[-- Attachment #1: 601-qeth-postpone-open.diff --]
[-- Type: text/plain, Size: 3138 bytes --]

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

The open function of qeth is not executed if the qeth device is in
state DOWN or HARDSETUP. A recovery switches from state SOFTSETUP to 
HARDSETUP to DOWN to HARDSETUP and back to SOFTSETUP. If open and
recover are running concurrently, open fails if it hits the states
HARDSETUP or DOWN. This patch inserts waiting for recovery finish
in the qeth open functions to enable successful qeth device opening
in spite of a running recovery.

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

 drivers/s390/net/qeth_l2_main.c |   18 ++++++++++++++++--
 drivers/s390/net/qeth_l3_main.c |   18 ++++++++++++++++--
 2 files changed, 32 insertions(+), 4 deletions(-)

--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -831,12 +831,14 @@ tx_drop:
 	return NETDEV_TX_OK;
 }
 
-static int qeth_l2_open(struct net_device *dev)
+static int __qeth_l2_open(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
 	int rc = 0;
 
 	QETH_CARD_TEXT(card, 4, "qethopen");
+	if (card->state == CARD_STATE_UP)
+		return rc;
 	if (card->state != CARD_STATE_SOFTSETUP)
 		return -ENODEV;
 
@@ -857,6 +859,18 @@ static int qeth_l2_open(struct net_devic
 	return rc;
 }
 
+static int qeth_l2_open(struct net_device *dev)
+{
+	struct qeth_card *card = dev->ml_priv;
+
+	QETH_CARD_TEXT(card, 5, "qethope_");
+	if (qeth_wait_for_threads(card, QETH_RECOVER_THREAD)) {
+		QETH_CARD_TEXT(card, 3, "openREC");
+		return -ERESTARTSYS;
+	}
+	return __qeth_l2_open(dev);
+}
+
 static int qeth_l2_stop(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
@@ -1046,7 +1060,7 @@ contin:
 	if (recover_flag == CARD_STATE_RECOVER) {
 		if (recovery_mode &&
 		    card->info.type != QETH_CARD_TYPE_OSN) {
-			qeth_l2_open(card->dev);
+			__qeth_l2_open(card->dev);
 		} else {
 			rtnl_lock();
 			dev_open(card->dev);
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -3240,12 +3240,14 @@ tx_drop:
 	return NETDEV_TX_OK;
 }
 
-static int qeth_l3_open(struct net_device *dev)
+static int __qeth_l3_open(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
 	int rc = 0;
 
 	QETH_CARD_TEXT(card, 4, "qethopen");
+	if (card->state == CARD_STATE_UP)
+		return rc;
 	if (card->state != CARD_STATE_SOFTSETUP)
 		return -ENODEV;
 	card->data.state = CH_STATE_UP;
@@ -3260,6 +3262,18 @@ static int qeth_l3_open(struct net_devic
 	return rc;
 }
 
+static int qeth_l3_open(struct net_device *dev)
+{
+	struct qeth_card *card = dev->ml_priv;
+
+	QETH_CARD_TEXT(card, 5, "qethope_");
+	if (qeth_wait_for_threads(card, QETH_RECOVER_THREAD)) {
+		QETH_CARD_TEXT(card, 3, "openREC");
+		return -ERESTARTSYS;
+	}
+	return __qeth_l3_open(dev);
+}
+
 static int qeth_l3_stop(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
@@ -3564,7 +3578,7 @@ contin:
 		netif_carrier_off(card->dev);
 	if (recover_flag == CARD_STATE_RECOVER) {
 		if (recovery_mode)
-			qeth_l3_open(card->dev);
+			__qeth_l3_open(card->dev);
 		else {
 			rtnl_lock();
 			dev_open(card->dev);


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug
  2011-01-13  6:42 [patch 0/2] s390: qeth bug fixes for 2.6.38 next rc frank.blaschka
  2011-01-13  6:42 ` [patch 1/2] [PATCH] qeth: postpone open till recovery is finished frank.blaschka
@ 2011-01-13  6:42 ` frank.blaschka
  2011-01-13  7:47   ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: frank.blaschka @ 2011-01-13  6:42 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390

[-- Attachment #1: 602-qeth-l3-tx-csum.diff --]
[-- Type: text/plain, Size: 781 bytes --]

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

Some OSA level have a bug in the hw tx csum logic. We can circumvent
this bug by turning on IP hw csum also.

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

 drivers/s390/net/qeth_l3_main.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru
 	 */
 	if (iph->protocol == IPPROTO_UDP)
 		hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP;
-	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ;
+	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ |
+		QETH_HDR_EXT_CSUM_HDR_REQ;
+	iph->check = 0;
 	if (card->options.performance_stats)
 		card->perf_stats.tx_csum++;
 }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug
  2011-01-13  6:42 ` [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug frank.blaschka
@ 2011-01-13  7:47   ` David Miller
  2011-01-13  8:23     ` Frank Blaschka
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2011-01-13  7:47 UTC (permalink / raw)
  To: frank.blaschka; +Cc: netdev, linux-s390

From: frank.blaschka@de.ibm.com
Date: Thu, 13 Jan 2011 07:42:25 +0100

> --- a/drivers/s390/net/qeth_l3_main.c
> +++ b/drivers/s390/net/qeth_l3_main.c
> @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru
>  	 */
>  	if (iph->protocol == IPPROTO_UDP)
>  		hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP;
> -	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ;
> +	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ |
> +		QETH_HDR_EXT_CSUM_HDR_REQ;
> +	iph->check = 0;
>  	if (card->options.performance_stats)
>  		card->perf_stats.tx_csum++;
>  }

You may not change the packet header contents blindly like this.
Otherwise unpredictable contents will be seen by tcpdump and any
other code path which has a clone of this packet.

Thus, you'll need to guard this change with something like:

		if (skb_header_cloned(skb) &&
		    pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
			dev_kfree_skb(skb);
			goto tx_fail;
		}

Please fix this and resubmit your two patches.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug
  2011-01-13  7:47   ` David Miller
@ 2011-01-13  8:23     ` Frank Blaschka
  2011-01-16  4:46       ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Frank Blaschka @ 2011-01-13  8:23 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-s390

On Wed, Jan 12, 2011 at 11:47:35PM -0800, David Miller wrote:
> From: frank.blaschka@de.ibm.com
> Date: Thu, 13 Jan 2011 07:42:25 +0100
> 
> > --- a/drivers/s390/net/qeth_l3_main.c
> > +++ b/drivers/s390/net/qeth_l3_main.c
> > @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru
> >  	 */
> >  	if (iph->protocol == IPPROTO_UDP)
> >  		hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP;
> > -	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ;
> > +	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ |
> > +		QETH_HDR_EXT_CSUM_HDR_REQ;
> > +	iph->check = 0;
> >  	if (card->options.performance_stats)
> >  		card->perf_stats.tx_csum++;
> >  }
> 
> You may not change the packet header contents blindly like this.
> Otherwise unpredictable contents will be seen by tcpdump and any
> other code path which has a clone of this packet.
> 
> Thus, you'll need to guard this change with something like:
> 
> 		if (skb_header_cloned(skb) &&
> 		    pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
> 			dev_kfree_skb(skb);
> 			goto tx_fail;
> 		}
Yes I know. Because of the suboptimal l3 driver design :-) we already have
a private copy of the skb at this place. Thx!
> 
> Please fix this and resubmit your two patches.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug
  2011-01-13  8:23     ` Frank Blaschka
@ 2011-01-16  4:46       ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2011-01-16  4:46 UTC (permalink / raw)
  To: frank.blaschka; +Cc: netdev, linux-s390

From: Frank Blaschka <frank.blaschka@de.ibm.com>
Date: Thu, 13 Jan 2011 09:23:59 +0100

> On Wed, Jan 12, 2011 at 11:47:35PM -0800, David Miller wrote:
>> From: frank.blaschka@de.ibm.com
>> Date: Thu, 13 Jan 2011 07:42:25 +0100
>> 
>> > --- a/drivers/s390/net/qeth_l3_main.c
>> > +++ b/drivers/s390/net/qeth_l3_main.c
>> > @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru
>> >  	 */
>> >  	if (iph->protocol == IPPROTO_UDP)
>> >  		hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP;
>> > -	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ;
>> > +	hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ |
>> > +		QETH_HDR_EXT_CSUM_HDR_REQ;
>> > +	iph->check = 0;
>> >  	if (card->options.performance_stats)
>> >  		card->perf_stats.tx_csum++;
>> >  }
>> 
>> You may not change the packet header contents blindly like this.
>> Otherwise unpredictable contents will be seen by tcpdump and any
>> other code path which has a clone of this packet.
>> 
>> Thus, you'll need to guard this change with something like:
>> 
>> 		if (skb_header_cloned(skb) &&
>> 		    pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
>> 			dev_kfree_skb(skb);
>> 			goto tx_fail;
>> 		}
> Yes I know. Because of the suboptimal l3 driver design :-) we already have
> a private copy of the skb at this place. Thx!

I see, thanks for explaining.

Both patches applied, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-01-16  4:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-13  6:42 [patch 0/2] s390: qeth bug fixes for 2.6.38 next rc frank.blaschka
2011-01-13  6:42 ` [patch 1/2] [PATCH] qeth: postpone open till recovery is finished frank.blaschka
2011-01-13  6:42 ` [patch 2/2] [PATCH] qeth: l3 hw tx csum circumvent hw bug frank.blaschka
2011-01-13  7:47   ` David Miller
2011-01-13  8:23     ` Frank Blaschka
2011-01-16  4:46       ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).