Masa Murayama wrote: > Garrett, > > Thank you very much for quick answer for me. I feel that > the review of checksum code is very dufficult without > hme datasheet. Many points I concerned, became very clear > by your detailed answers. Thank you very much. > > I still have a question on hme chipset specification. > > >>> line 4772-4274, hmestart >>> Doesn't it need to tell the protocol for the transmitting >>> packet, UDP or TCP, to hme hardware? >>> Nics that have the partial hardware checksum function for tx, >>> like intel pro/1000 and Marvell PCIe based chipset, for which >>> I've written drivers, requires to specify UDP or TCP in tx >>> descriptors, because they cannot detect it automatically. >>> How about hme chipset? The checksum calculations for UDP and >>> TCP, are not same. >>> In the current solaris hw checksum frame work, nics must >>> support both of UDP and TCP, otherwise we cannot use it, >>> I think. >>> >>> >> No. We are registered for "partial" checksum, which means that it is a >> simple ones-complement calculation over the specified region. The stack >> will have compensated by putting the appropriate value in the csum field >> so that the hardware checksum is "corrected." All the hardware has to >> do is put the ones-complement checksum from the indicated region at the >> specified offset. >> > > In my understanding, the nic with partial checksum does: > > tcp: > Add ones-complement calculation over specified region, and existing > checksum in tcp header, which is checksum for psuedo header. > Then, store ones-complement of the result into the checksum field. > > udp: > Add ones-complement calculation over specified region, and existing > checksum in udp header, which is checksum for psuedo header. > Then, stores ones-complement of the result into the checksum field. > > BUT, if the result above, is 0000, the nic stores FFFF to avoid > udp checksum option become disabled. (this is the only difference > between tcp and udp checksum calculations) > > Therefore, nic *must* know if the protocol is tcp or udp, I think. > For udp, the stack cannot compensate the checksum for psuedo header, > because it won't know if the final result is 0000 or not. > It is no problem of course, if the hme chipset can detect protocol on > tx packet. >
I need to research this and get back to you. I do know that the code as I wrote it works. -- Garrett > Best regards, > > -masa > > ----- Original Message ----- > >> Date: Wed, 11 Jul 2007 12:45:28 -0700 >> From: "Garrett D'Amore" <[EMAIL PROTECTED]> >> To: Masayuki Murayama <[EMAIL PROTECTED]> >> CC: [email protected] >> Subject: Re: [networking-discuss] looking for code reviewers for hme (GLDv3, >> ip checksum offload) >> >> >> Masayuki Murayama wrote: >> >>> Garret, >>> >>> This is my suggestion on the new hme: >>> >>> >>> >>>> Masayuki Murayama wrote: >>>> >>>> >>>>>> I got my second reviewer, so no need for anyone >>>>>> >>>>>> >>>> else >>>> >>>> >>>>>> to review it if >>>>>> they don't want to. >>>>>> >>>>>> >>>>>> >>>>> It is very good for you! >>>>> >>>>> >>>>> >>>>> >>>>>> Of course, review feedback is >>>>>> always appreciated... >>>>>> I can always follow up with a 2nd RTI if it is >>>>>> warranted. (Plus, I'll >>>>>> be in this code again shortly, to add support for >>>>>> >>>>>> >>>> QFE >>>> >>>> >>>>>> devices.) >>>>>> >>>>>> >>>>>> >>>>> I understand you hasten the code to be reviewed. >>>>> I'll focus and review the VLAN tagging, GLDv3 and >>>>> hw checksum part of the code in one or two days, >>>>> >>>>> >>>> and will >>>> >>>> >>>>> post the result here. >>>>> >>>>> >>>>> >>>> Okay, great! >>>> >>>> >>>> >>> line 3984-3985, hme_m_tx >>> Messages blocks should be deallocated without transmitting if >>> hme chipset stops transmitting packets when the link is down, >>> I think. >>> Actually it'll depend on hme chipset behavior. I recommend >>> to ensure it by its datasheet. >>> >>> >> I'm pretty sure the code is correct as it stands. The MAC controller is >> fairly separate from the PHY, and the chip will properly return packets >> with a carrier failure if the link is down. >> >> >>> line 4772-4274, hmestart >>> Doesn't it need to tell the protocol for the transmitting >>> packet, UDP or TCP, to hme hardware? >>> Nics that have the partial hardware checksum function for tx, >>> like intel pro/1000 and Marvell PCIe based chipset, for which >>> I've written drivers, requires to specify UDP or TCP in tx >>> descriptors, because they cannot detect it automatically. >>> How about hme chipset? The checksum calculations for UDP and >>> TCP, are not same. >>> In the current solaris hw checksum frame work, nics must >>> support both of UDP and TCP, otherwise we cannot use it, >>> I think. >>> >>> >> No. We are registered for "partial" checksum, which means that it is a >> simple ones-complement calculation over the specified region. The stack >> will have compensated by putting the appropriate value in the csum field >> so that the hardware checksum is "corrected." All the hardware has to >> do is put the ones-complement checksum from the indicated region at the >> specified offset. >> >> >>> line 6032-6039: hmeread >>> In my experience, partially calculated hw checksum for rx >>> packets, must be applied on only TCP or UDP packets, not >>> on other packets like ICMP. Actually it depends on hme >>> chipset specification. Please ensure it. >>> >>> >> HME doesn't care. You could run it over IPX packets. Of course, the >> result would be kinda nonsensical. >> >> Anyway, the stack won't supply checksum fields if the packet isn't >> appropriate for this kind of offload. >> >> >>> And I wonder all generations of hme chipset can detect >>> IPv6 packets? Rx hw checksum function should be aware of >>> protocols for rx packets. >>> >>> >> Again, you don't have to know this. All the hardware does is generate a >> checksum over the specified ethernet payload. The stack will "adjust" >> the computed results to account for differences in protocols, etc. >> >> >>> line 6037: hmeread >>> I think the start offset parameter which is passed >>> hcksum_assoc() should be offset to UDP or TCP header from >>> IP header, instead of 0. The offset to checksum field >>> should be passed to hcksum_assoc(), instead of 0. >>> >>> >> Nope. The offset is the offset from the IP header. In our receive >> function, the hardware generates the checksum from that IP header, and >> the stack makes the appropriate adjustments. >> >> >>> To debug this code is very difficult. Solaris ip layer >>> passes rx packets when software checksum is ok, even if >>> the hardware checksum isn't ok. >>> >>> >> Yes, but there is statistic reporting for that. Trust me, when I had >> the incorrect algorithm, I noticed. :-) A number of kinds of >> connectivity just started failing. >> >> >>> line 5858-5869: hmeread_dma >>> same with what I suggested on hmeread >>> >>> >> And same replies. :-) >> >> -- Garrett >> >>> Best Regards, >>> -masa >>> >>> >>> This message posted from opensolaris.org >>> _______________________________________________ >>> networking-discuss mailing list >>> [email protected] >>> >>> > > _______________________________________________ networking-discuss mailing list [email protected]
