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.

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.

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.
 And I wonder all generations of hme chipset can detect
IPv6 packets? Rx hw checksum function should be aware of
protocols for rx packets.

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.

 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.

line 5858-5869: hmeread_dma
 same with what I suggested on hmeread

Best Regards,
-masa
 
 
This message posted from opensolaris.org
_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to