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

Alex Estrin alex.estrin at qlogic.com
Thu Feb 12 09:40:34 PST 2009


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



More information about the ofw mailing list