cvs commit: src/sys/dev/bge if_bge.c

[Oleg Bulyzhin]

On Tue, Feb 07, 2006 at 08:44:42PM +1100, Bruce Evans wrote:
> On Mon, 6 Feb 2006, Nate Lawson wrote:
> 
> >Nate Lawson wrote:
> >>Oleg Bulyzhin wrote:
> >>
> >>>On Mon, Feb 06, 2006 at 02:21:09PM -0800, Nate Lawson wrote:
> >>>
> >>>>Oleg Bulyzhin wrote:
> >>>>
> >>>>>        nq = q->m_nextpkt;
> >>>>>        q->m_nextpkt = NULL;
> >>>>>        m->m_pkthdr.csum_flags &= q->m_pkthdr.csum_flags;
> >>>>>-        m->m_pkthdr.csum_data += q->m_pkthdr.csum_data;
> >>>>>+        sum = m->m_pkthdr.csum_data + q->m_pkthdr.csum_data;
> >>>>>+        m->m_pkthdr.csum_data = (sum & 0xffff) + (sum >> 16);
> >>>>>        m_cat(m, q);
> >>>>>    }
> >>>>>#ifdef MAC
> >...
> >Sam Leffler mentioned this comment from NetBSD would be helpful.  Might 
> >you add it to clear up misunderstandings like I had?
> >
> >sys/mbuf.h has useful comments not found in the freebsd file:
> >...
> >* Note for in-bound TCP/UDP checksums, we expect the csum_data to NOT
> >* be bit-wise inverted (the final step in the calculation of an IP
> >* checksum) -- this is so we can accumulate the checksum for fragmented
> >* packets during reassembly.
> >*/
> 
> This almost makes the carry-folding (including the above change)
> unnecessary too.  csum_data can accumulate at least 64K terms each
> less than 0x10000 before it might have a carry out of its 32-bit int.
> I think there can't be 64K fragments, so the unpatched version would
> work if the final step did the carry-folding and no intermediate step
> assumes that csum_data < 0x10000.  I couldn't find any final or
> intermediate steps that would cause problems.
> 
> Bruce

You are right. If we are not going to reassemble more than 64k fragments we
can just add (into original ip_reass()):
m->m_pkthdr.csum_data =
    (m->m_pkthdr.csum_data & 0xffff) + (m->m_pkthdr.csum_data >> 16);
right after for {} loop.

-- 
Oleg.