Patchwork [v2] e1000: Fix multi-descriptor packet checksum offload

login
register
mail settings
Submitter Stefan Hajnoczi
Date March 7, 2011, 8:04 p.m.
Message ID <1299528247-22545-1-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/85795/
State New
Headers show

Comments

Stefan Hajnoczi - March 7, 2011, 8:04 p.m.
The PCI/PCI-X Family of Gigabit Ethernet Controllers Software
Developer’s Manual states the following about the POPTS field:

  Provides a number of options which control the handling of this
  packet.  This field is ignored except on the first data descriptor of
  a packet.

The current implementation always loads the field and its checksum
offload flags.  This patch uses only the first descriptor's POPTS field
in order to comply with the specification.

When Solaris sends multi-descriptor packets it fills in POPTS for the
first descriptor only.  Therefore this patch is necessary in order to
perform checksum offload correctly for multi-descriptor packets.

Reported-by: Daniel Pecka <dpecka@techniservit.cz>
Reported-by: Gabriele A. Trombetti <gabriele.trombetti@itb.cnr.it>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
v2:
 * Fix Reported-by: details

 hw/e1000.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Aurelien Jarno - March 21, 2011, 8:43 p.m.
On Mon, Mar 07, 2011 at 08:04:07PM +0000, Stefan Hajnoczi wrote:
> The PCI/PCI-X Family of Gigabit Ethernet Controllers Software
> Developer’s Manual states the following about the POPTS field:
> 
>   Provides a number of options which control the handling of this
>   packet.  This field is ignored except on the first data descriptor of
>   a packet.
> 
> The current implementation always loads the field and its checksum
> offload flags.  This patch uses only the first descriptor's POPTS field
> in order to comply with the specification.
> 
> When Solaris sends multi-descriptor packets it fills in POPTS for the
> first descriptor only.  Therefore this patch is necessary in order to
> perform checksum offload correctly for multi-descriptor packets.
> 
> Reported-by: Daniel Pecka <dpecka@techniservit.cz>
> Reported-by: Gabriele A. Trombetti <gabriele.trombetti@itb.cnr.it>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
> v2:
>  * Fix Reported-by: details
> 
>  hw/e1000.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/hw/e1000.c b/hw/e1000.c
> index 0a4574c..2a4d5c7 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -446,7 +446,9 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>          return;
>      } else if (dtype == (E1000_TXD_CMD_DEXT | E1000_TXD_DTYP_D)) {
>          // data descriptor
> -        tp->sum_needed = le32_to_cpu(dp->upper.data) >> 8;
> +        if (tp->size == 0) {
> +            tp->sum_needed = le32_to_cpu(dp->upper.data) >> 8;
> +        }
>          tp->cptse = ( txd_lower & E1000_TXD_CMD_TSE ) ? 1 : 0;
>      } else {
>          // legacy descriptor
> -- 
> 1.7.2.3
> 
> 
>

Patch

diff --git a/hw/e1000.c b/hw/e1000.c
index 0a4574c..2a4d5c7 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -446,7 +446,9 @@  process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
         return;
     } else if (dtype == (E1000_TXD_CMD_DEXT | E1000_TXD_DTYP_D)) {
         // data descriptor
-        tp->sum_needed = le32_to_cpu(dp->upper.data) >> 8;
+        if (tp->size == 0) {
+            tp->sum_needed = le32_to_cpu(dp->upper.data) >> 8;
+        }
         tp->cptse = ( txd_lower & E1000_TXD_CMD_TSE ) ? 1 : 0;
     } else {
         // legacy descriptor