I''d like reviewers for: 6639676 xnb should use hardware partial checksum offload if it is available 6649354 xnf sends bad checksum on small IPv6 packets (PV domU) Webrev is at http://dme.org/solaris/webrev/6649354/
On Tue, Jul 29, 2008 at 06:44:08PM +0100, David Edmondson wrote:> I''d like reviewers for: > > 6639676 xnb should use hardware partial checksum offload if it is > available > 6649354 xnf sends bad checksum on small IPv6 packets (PV domU) > > Webrev is at http://dme.org/solaris/webrev/6649354/I updated this slightly to avoid the need to declare a flag day.
> On Tue, Jul 29, 2008 at 06:44:08PM +0100, David Edmondson wrote: > > I''d like reviewers for: > > > > 6639676 xnb should use hardware partial checksum offload if it is available > > 6649354 xnf sends bad checksum on small IPv6 packets (PV domU) > > > > Webrev is at http://dme.org/solaris/webrev/6649354/ > > I updated this slightly to avoid the need to declare a flag day.In xnb.c, line 361..363: is that assert correct? It should be OK if we get a cksum > 0xFFFF after adding the high & low 16 bits at line 361. Before the assert, another |cksum = (cksum >> 16) + (cksum & 0xFFFF);| is needed. 357 cksum += (dst >> 16) + (dst & 0xFFFF); 358 cksum += (src >> 16) + (src & 0xFFFF); 359 cksum += length - IP_SIMPLE_HDR_LENGTH; 360 361 cksum = (cksum >> 16) + (cksum & 0xFFFF); 362 363 ASSERT(cksum <= 0xFFFF); The above code should trigger the assert with: dst ip: 255.255.255.255 src ip: 10.235.244.240 proto: 6 len: 32 ffff+ffff+aeb+f4f0+6+20=X 2ffff With such a packet it should compute checksum as 0x10001, after line 361. And a debug kernel would crash at line 363. This message posted from opensolaris.org
> > > Webrev is at http://dme.org/solaris/webrev/6649354/ > > > > I updated this slightly to avoid the need to declare a flag day. > > In xnb.c, line 361..363: is that assert correct?Same problem in xnf.c, lines 1176 ...1178. This message posted from opensolaris.org
On Fri, Aug 01, 2008 at 10:09:58AM -0700, J??rgen Keil wrote:> > > > Webrev is at http://dme.org/solaris/webrev/6649354/ > > > > > > I updated this slightly to avoid the need to declare a flag day. > > > > In xnb.c, line 361..363: is that assert correct? > > Same problem in xnf.c, lines 1176 ...1178.Juergen, you are correct - I lost my second fold somewhere. Thanks for the careful review.
On Fri, Aug 01, 2008 at 05:10:53PM +0100, David Edmondson wrote:> On Tue, Jul 29, 2008 at 06:44:08PM +0100, David Edmondson wrote: > > I''d like reviewers for: > > > > 6639676 xnb should use hardware partial checksum offload if it is > > available > > 6649354 xnf sends bad checksum on small IPv6 packets (PV domU) > > > > Webrev is at http://dme.org/solaris/webrev/6649354/ > > I updated this slightly to avoid the need to declare a flag day.After digging out my copy of the 5704 PRM, I''ve added a fix for: 6682441 can''t get any xnf on bge network action without setting ip:dohwcksum ...as well as fixing the pseudo-checksum calculation problems pointed out by Juergen.
David Edmondson wrote:> On Fri, Aug 01, 2008 at 05:10:53PM +0100, David Edmondson wrote: >> On Tue, Jul 29, 2008 at 06:44:08PM +0100, David Edmondson wrote: >>> I''d like reviewers for: >>> >>> 6639676 xnb should use hardware partial checksum offload if it is >>> available >>> 6649354 xnf sends bad checksum on small IPv6 packets (PV domU) >>> >>> Webrev is at http://dme.org/solaris/webrev/6649354/ >> I updated this slightly to avoid the need to declare a flag day. > > After digging out my copy of the 5704 PRM, I''ve added a fix for: > > 6682441 can''t get any xnf on bge network action without setting > ip:dohwcksum > > ...as well as fixing the pseudo-checksum calculation problems pointed > out by Juergen.that''s not reflected in the webrev yet. Will you update this? Michael -- Michael Schuster http://blogs.sun.com/recursion Recursion, n.: see ''Recursion''
On Mon, Aug 04, 2008 at 07:58:30AM -0700, Michael Schuster wrote:>> After digging out my copy of the 5704 PRM, I''ve added a fix for: >> >> 6682441 can''t get any xnf on bge network action without setting >> ip:dohwcksum >> >> ...as well as fixing the pseudo-checksum calculation problems pointed >> out by Juergen. > > that''s not reflected in the webrev yet. Will you update this?It''s reflected in the version I see. Maybe you''re getting a cached version?
David Edmondson wrote:> On Mon, Aug 04, 2008 at 07:58:30AM -0700, Michael Schuster wrote: >>> After digging out my copy of the 5704 PRM, I''ve added a fix for: >>> >>> 6682441 can''t get any xnf on bge network action without setting >>> ip:dohwcksum >>> >>> ...as well as fixing the pseudo-checksum calculation problems pointed >>> out by Juergen. >> that''s not reflected in the webrev yet. Will you update this? > > It''s reflected in the version I see. Maybe you''re getting a cached > version?no, I''m not, it''s all there - it''s obviously too early for me ;-) sorry about that. Michael -- Michael Schuster http://blogs.sun.com/recursion Recursion, n.: see ''Recursion''
Hi,
xnb.c line 322:
if (ipha->ipha_protocol == IPPROTO_TCP) {
Seems redundant, because we already know we are IPPROTO_UDP:
307 switch (ipha->ipha_protocol) {
.
.
309 case IPPROTO_UDP:
.
.
Regards.
Sorry I''m wrong, it still could IPPROTO_TCP. Carlo Magno Arellano wrote:> Hi, > > xnb.c line 322: > > if (ipha->ipha_protocol == IPPROTO_TCP) { > > > Seems redundant, because we already know we are IPPROTO_UDP: > > 307 switch (ipha->ipha_protocol) { > . > . > 309 case IPPROTO_UDP: > . > . > > > Regards. > > _______________________________________________ > xen-discuss mailing list > xen-discuss@opensolaris.org > >