[PATCH net-2.6] sfc: Set ip_summed correctly for page buffers passed to GRO

[PATCH net-2.6] sfc: Set ip_summed correctly for page buffers passed to GRO

[Ben Hutchings]

Page buffers containing packets with an incorrect checksum or using a
protocol not handled by hardware checksum offload were previously not
passed to LRO.  The conversion to GRO changed this, but did not set
the ip_summed value accordingly.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Cc: stable@kernel.org
---
This affects 2.6.31 and seems like a candidate for a stable update.

Ben.

 drivers/net/sfc/rx.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/rx.c b/drivers/net/sfc/rx.c
index ea59ed2..4b65c62 100644
--- a/drivers/net/sfc/rx.c
+++ b/drivers/net/sfc/rx.c
@@ -441,7 +441,8 @@ static void efx_rx_packet__check_len(struct efx_rx_queue *rx_queue,
  * the appropriate LRO method
  */
 static void efx_rx_packet_lro(struct efx_channel *channel,
-			      struct efx_rx_buffer *rx_buf)
+			      struct efx_rx_buffer *rx_buf,
+			      bool checksummed)
 {
 	struct napi_struct *napi = &channel->napi_str;
 
@@ -463,7 +464,8 @@ static void efx_rx_packet_lro(struct efx_channel *channel,
 		skb->len = rx_buf->len;
 		skb->data_len = rx_buf->len;
 		skb->truesize += rx_buf->len;
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		skb->ip_summed =
+			checksummed ? CHECKSUM_UNNECESSARY : CHECKSUM_NONE;
 
 		napi_gro_frags(napi);
 
@@ -472,6 +474,7 @@ out:
 		rx_buf->page = NULL;
 	} else {
 		EFX_BUG_ON_PARANOID(!rx_buf->skb);
+		EFX_BUG_ON_PARANOID(!checksummed);
 
 		napi_gro_receive(napi, rx_buf->skb);
 		rx_buf->skb = NULL;
@@ -567,7 +570,7 @@ void __efx_rx_packet(struct efx_channel *channel,
 	}
 
 	if (likely(checksummed || rx_buf->page)) {
-		efx_rx_packet_lro(channel, rx_buf);
+		efx_rx_packet_lro(channel, rx_buf, checksummed);
 		goto done;
 	}
 

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH net-2.6] sfc: Really allow RX checksum offload to be disabled

[Ben Hutchings]

We have never checked the efx_nic::rx_checksum_enabled flag everywhere
we should, and since the switch to GRO we don't check it anywhere.
It's simplest to check it in the one place where we initialise the
per-packet checksummed flag.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Cc: stable@kernel.org
---
I'm not sure whether this is serious enough to merit a stable update.
It's not a recent regression.

Ben.

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

diff --git a/drivers/net/sfc/falcon.c b/drivers/net/sfc/falcon.c
index 8776432..865638b 100644
--- a/drivers/net/sfc/falcon.c
+++ b/drivers/net/sfc/falcon.c
@@ -869,8 +869,9 @@ static void falcon_handle_rx_event(struct efx_channel *channel,
 		 * UDP/IPv4, then we can rely on the hardware checksum.
 		 */
 		checksummed =
-			rx_ev_hdr_type == FSE_AB_RX_EV_HDR_TYPE_IPV4_TCP ||
-			rx_ev_hdr_type == FSE_AB_RX_EV_HDR_TYPE_IPV4_UDP;
+			efx->rx_checksum_enabled &&
+			(rx_ev_hdr_type == FSE_AB_RX_EV_HDR_TYPE_IPV4_TCP ||
+			 rx_ev_hdr_type == FSE_AB_RX_EV_HDR_TYPE_IPV4_UDP);
 	} else {
 		falcon_handle_rx_not_ok(rx_queue, event, &rx_ev_pkt_ok,
 					&discard);

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net-2.6] sfc: Really allow RX checksum offload to be disabled

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 27 Oct 2009 19:44:33 +0000

> We have never checked the efx_nic::rx_checksum_enabled flag everywhere
> we should, and since the switch to GRO we don't check it anywhere.
> It's simplest to check it in the one place where we initialise the
> per-packet checksummed flag.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> Cc: stable@kernel.org
> ---
> I'm not sure whether this is serious enough to merit a stable update.
> It's not a recent regression.

This patch only applies to net-next-2.6, so I can't see how it could
be a -stable candidate :-)

So I've applied it there.

Re: [PATCH net-2.6] sfc: Really allow RX checksum offload to be disabled

[Ben Hutchings]

On Wed, 2009-10-28 at 02:49 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Tue, 27 Oct 2009 19:44:33 +0000
> 
> > We have never checked the efx_nic::rx_checksum_enabled flag everywhere
> > we should, and since the switch to GRO we don't check it anywhere.
> > It's simplest to check it in the one place where we initialise the
> > per-packet checksummed flag.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > Cc: stable@kernel.org
> > ---
> > I'm not sure whether this is serious enough to merit a stable update.
> > It's not a recent regression.
> 
> This patch only applies to net-next-2.6, so I can't see how it could
> be a -stable candidate :-)
> 
> So I've applied it there.

The register name update in net-next-2.6 changed the context for this
patch.  I'll send a new patch that will apply to the earlier versions.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net-2.6] sfc: Set ip_summed correctly for page buffers passed to GRO

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 27 Oct 2009 14:50:57 +0000

> Page buffers containing packets with an incorrect checksum or using a
> protocol not handled by hardware checksum offload were previously not
> passed to LRO.  The conversion to GRO changed this, but did not set
> the ip_summed value accordingly.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

Applied.

> This affects 2.6.31 and seems like a candidate for a stable update.

Queued up for -stable, thanks.