[ofw] RE: [IPoIB CM][Patch] receive Chksum flags

Alex Estrin alex.estrin at qlogic.com
Fri Feb 13 09:54:59 PST 2009


Here is an updated patch with additional change - 
we still have to disable send Chksum offload if in connected mode. ( last time I removed it by mistake ).

Thanks,
Alex.

Index: kernel/ipoib_endpoint.c
===================================================================
--- kernel/ipoib_endpoint.c	(revision 1938)
+++ kernel/ipoib_endpoint.c	(working copy)
@@ -820,16 +820,10 @@
 		default:
 			CL_ASSERT( FALSE );
 		case CSUM_DISABLED:
+		case CSUM_ENABLED:
 			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet, TcpIpChecksumPacketInfo ) =
 				(void*)(uintn_t)chksum.Value;
 			break;
-		case CSUM_ENABLED:
-			/* Get the checksums directly from packet information. */
-			/* In this case, no one of cheksum's cat get false value */
-			/* If hardware checksum failed or wasn't calculated, NDIS will recalculate it again */
-			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet, TcpIpChecksumPacketInfo ) = 
-				(void*)(uintn_t)(p_desc->ndis_csum.Value);
-			break;
 		case CSUM_BYPASS:
 			/* Flag the checksums as having been calculated. */
 			chksum.Receive.NdisPacketTcpChecksumSucceeded = TRUE;
Index: kernel/ipoib_port.c
===================================================================
--- kernel/ipoib_port.c	(revision 1954)
+++ kernel/ipoib_port.c	(working copy)
@@ -1082,6 +1082,11 @@
 			NdisWriteErrorLogEntry( p_port->p_adapter->h_adapter,
 				EVENT_IPOIB_CONNECTED_MODE_ERR, 1, 0xbadc0de2 );
 		}
+		else 
+		{
+			if ( p_port->p_adapter->params.send_chksum_offload )
+			p_port->p_adapter->params.send_chksum_offload = CSUM_DISABLED;
+		}
 	}
 	IPOIB_EXIT( IPOIB_DBG_INIT );
 	return IB_SUCCESS;


> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex
> Estrin
> Sent: Thursday, February 12, 2009 12:41 PM
> To: Alex Naslednikov; ofw at lists.openfabrics.org
> Subject: [ofw] RE: [IPoIB CM][Patch] receive Chksum flags
> 
> Hi XaleX,
> 
> I agree with your point, although I believe we don't have to check
> 'cm_enabled' flag if we already in RC QP receive completion routine.
> 
> I think this change should work correctly.
> Please review.
> 
> Thanks,
> Alex.
> 
> 
> Index: kernel/ipoib_endpoint.c
> ===================================================================
> --- kernel/ipoib_endpoint.c	(revision 1938)
> +++ kernel/ipoib_endpoint.c	(working copy)
> @@ -820,16 +820,10 @@
>  		default:
>  			CL_ASSERT( FALSE );
>  		case CSUM_DISABLED:
> +		case CSUM_ENABLED:
>  			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet, TcpIpChecksumPacketInfo ) =
>  				(void*)(uintn_t)chksum.Value;
>  			break;
> -		case CSUM_ENABLED:
> -			/* Get the checksums directly from packet information. */
> -			/* In this case, no one of cheksum's cat get false value */
> -			/* If hardware checksum failed or wasn't calculated, NDIS will recalculate it again
> */
> -			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet, TcpIpChecksumPacketInfo ) =
> -				(void*)(uintn_t)(p_desc->ndis_csum.Value);
> -			break;
>  		case CSUM_BYPASS:
>  			/* Flag the checksums as having been calculated. */
>  			chksum.Receive.NdisPacketTcpChecksumSucceeded = TRUE;
> 
> 
> 
> > -----Original Message-----
> > From: Alex Naslednikov [mailto:xalex at mellanox.co.il]
> > Sent: Thursday, February 12, 2009 11:45 AM
> > To: Alex Estrin; ofw at lists.openfabrics.org
> > Cc: Tzachi Dar
> > Subject: RE: [IPoIB CM][Patch] receive Chksum flags
> >
> >  Hello, Alex,
> > I would like to point your attention on the following issue.
> > When CSUM is enabled, you take the value of CSUM from the WQE, thus
> > supposing that HW will put 0 (zero) there:
> > > +			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet,
> > TcpIpChecksumPacketInfo ) =
> > > +
> > (void*)(uintn_t)(p_desc->ndis_csum.Value);
> >
> > This is right for the new versions of our HW, but we can't assume this
> > generally. There are 2 possible solutions.
> > The best one (IMHO) is simply verify that CM mode and CSUM_ENABLED are
> > not set on simultaneously.
> > That is, simply unite 2 cases: CSUM_DISABLED and CSUM_ENABLED.
> >
> > The other is to check (in qp.c) which kind of QP we are working and each
> > time set 0 in appropriate place.
> > The problem with this solution is that it will have influence on
> > DataPath flow, which is not desirable.
> >
> > Thanks,
> > XaleX
> >
> > -----Original Message-----
> > From: Alex Estrin [mailto:alex.estrin at qlogic.com]
> > Sent: Tuesday, February 10, 2009 2:45 PM
> > To: ofw at lists.openfabrics.org; Alex Naslednikov
> > Subject: RE: [IPoIB CM][Patch] receive Chksum flags
> >
> > Applied in revision 1937.
> >
> > Thanks,
> > Alex.
> >
> > > -----Original Message-----
> > > From: ofw-bounces at lists.openfabrics.org
> > > [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Estrin
> > > Sent: Monday, February 09, 2009 1:06 PM
> > > To: ofw at lists.openfabrics.org; Alex Naslednikov
> > > Subject: [ofw] [IPoIB CM][Patch] receive Chksum flags
> > >
> > > Hello,
> > >
> > > Here is a patch to replace hardcoded TCP/IP Chksum flags indication
> > > with the same algorithm as used for datagram mode.
> > >
> > > Please review.
> > >
> > > Thanks,
> > > Alex.
> > >
> > > Index: kernel/ipoib_endpoint.c
> > > ===================================================================
> > > --- kernel/ipoib_endpoint.c	(revision 1933)
> > > +++ kernel/ipoib_endpoint.c	(working copy)
> > > @@ -791,6 +791,7 @@
> > >  	ipoib_cm_desc_t		*p_desc;
> > >  	uint32_t				i = 0;
> > >  	NDIS_PACKET				*p_packet;
> > > +	NDIS_TCP_IP_CHECKSUM_PACKET_INFO	chksum;
> > >
> > >  	IPOIB_ENTER( IPOIB_DBG_RECV );
> > >  	UNUSED_PARAM( p_endpt );
> > > @@ -813,12 +814,31 @@
> > >  			p_port->cm_recv_mgr.depth--;
> > >  			continue;
> > >  		}
> > > -		p_desc->ndis_csum.Value = 0;
> > > -		p_desc->ndis_csum.Receive.NdisPacketTcpChecksumSucceeded
> > = TRUE;
> > > -		p_desc->ndis_csum.Receive.NdisPacketUdpChecksumSucceeded
> > = TRUE;
> > > -		p_desc->ndis_csum.Receive.NdisPacketIpChecksumSucceeded
> > = TRUE;
> > > -		NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet,
> > TcpIpChecksumPacketInfo ) =
> > > -
> > (void*)(uintn_t)p_desc-
> > > >ndis_csum.Value;
> > > +		chksum.Value = 0;
> > > +		switch( p_port->p_adapter->params.recv_chksum_offload )
> > > +		{
> > > +		default:
> > > +			CL_ASSERT( FALSE );
> > > +		case CSUM_DISABLED:
> > > +			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet,
> > TcpIpChecksumPacketInfo ) =
> > > +				(void*)(uintn_t)chksum.Value;
> > > +			break;
> > > +		case CSUM_ENABLED:
> > > +			/* Get the checksums directly from packet
> > information. */
> > > +			/* In this case, no one of cheksum's cat get
> > false value */
> > > +			/* If hardware checksum failed or wasn't
> > calculated, NDIS will
> > > +recalculate it again
> > > */
> > > +			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet,
> > TcpIpChecksumPacketInfo ) =
> > > +
> > (void*)(uintn_t)(p_desc->ndis_csum.Value);
> > > +			break;
> > > +		case CSUM_BYPASS:
> > > +			/* Flag the checksums as having been calculated.
> > */
> > > +			chksum.Receive.NdisPacketTcpChecksumSucceeded =
> > TRUE;
> > > +			chksum.Receive.NdisPacketUdpChecksumSucceeded =
> > TRUE;
> > > +			chksum.Receive.NdisPacketIpChecksumSucceeded =
> > TRUE;
> > > +			NDIS_PER_PACKET_INFO_FROM_PACKET( p_packet,
> > TcpIpChecksumPacketInfo ) =
> > > +				(void*)(uintn_t)chksum.Value;
> > > +			break;
> > > +		}
> > >
> > >  		NDIS_SET_PACKET_STATUS(	p_packet, NDIS_STATUS_SUCCESS );
> > >  		p_port->cm_recv_mgr.recv_pkt_array[i] = p_packet;
> > > Index: kernel/ipoib_port.c
> > > ===================================================================
> > > --- kernel/ipoib_port.c	(revision 1933)
> > > +++ kernel/ipoib_port.c	(working copy)
> > > @@ -1082,16 +1082,6 @@
> > >  			NdisWriteErrorLogEntry(
> > p_port->p_adapter->h_adapter,
> > >  				EVENT_IPOIB_CONNECTED_MODE_ERR, 1,
> > 0xbadc0de2 );
> > >  		}
> > > -		else
> > > -		{
> > > -			/* now we can adjust csum capabilities */
> > > -		if (p_port->p_adapter->params.send_chksum_offload )
> > > -			p_port->p_adapter->params.send_chksum_offload =
> > CSUM_DISABLED;
> > > -
> > > -		if (p_port->p_adapter->params.recv_chksum_offload ==
> > CSUM_ENABLED)
> > > -			p_port->p_adapter->params.recv_chksum_offload =
> > CSUM_BYPASS;
> > > -		}
> > > -
> > >  	}
> > >  	IPOIB_EXIT( IPOIB_DBG_INIT );
> > >  	return IB_SUCCESS;
> > > _______________________________________________
> > > ofw mailing list
> > > ofw at lists.openfabrics.org
> > > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
-------------- next part --------------
A non-text attachment was scrubbed...
Name: recv_chksum_update.patch
Type: application/octet-stream
Size: 1473 bytes
Desc: recv_chksum_update.patch
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20090213/842809df/attachment.obj>


More information about the ofw mailing list