Small patch to multicast code...

[gnn at freebsd.org]

At Thu, 21 Aug 2008 22:35:19 +0200,
Luigi Rizzo wrote:
> 
> On Thu, Aug 21, 2008 at 03:11:56PM -0400, gnn at freebsd.org wrote:
> > Hi,
> > 
> > Turns out there is a bug in the code that loops back multicast
> > packets.  If the underlying device driver supports checksum offloading
> > then the packet that is looped back, when it is transmitted on the
> > wire, is incorrect, due to the fact that the packet is not fully
> > copied.
> > 
> > Here is a patch.  Comments welcome.
> > 
> > Best,
> > George
> > 
> > Index: ip_output.c
> > ===================================================================
> > --- ip_output.c	(revision 181731)
> > +++ ip_output.c	(working copy)
> > @@ -1135,7 +1135,7 @@
> >  	register struct ip *ip;
> >  	struct mbuf *copym;
> >  
> > -	copym = m_copy(m, 0, M_COPYALL);
> > +	copym = m_dup(m, M_DONTWAIT);
> >  	if (copym != NULL && (copym->m_flags & M_EXT || copym->m_len < hlen))
> >  		copym = m_pullup(copym, hlen);
> >  	if (copym != NULL) {
> 
> I am slightly puzzled -- what is exactly the problem, i.e. what part
> of the packet on the wire is incorrect ? The IP header is within hlen so
> the m_pullup() should be enough to leave the original content intact.
> 
> The only thing i can think of is that it's the UDP checksum,
> residing beyond hlen, which is overwritten somewhere in the
> call to if_simloop -- in which case perhaps a better fix is
> to m_pullup() the udp header as well ?

It is the checksum that gets trashed, yes.

> (in any case, it is worthwhile to add a comment to explain
> what should be done -- the code paths using m_*() have become
> quite fragile with these hw support enhancements that now
> require selective modifications on previously shared, readonly buffers).

The m_*() routines actually have reasonable comments, it just seems
the wrong one was used here.

Best,
Gerge