Date: Thu, 21 Aug 2008 17:59:38 -0400 From: gnn@freebsd.org To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: net@freebsd.org Subject: Re: Small patch to multicast code... Message-ID: <m23aky6ncl.wl%gnn@neville-neil.com> In-Reply-To: <20080821203519.GA51534@onelab2.iet.unipi.it> References: <m27iaa6v43.wl%gnn@neville-neil.com> <20080821203519.GA51534@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
At Thu, 21 Aug 2008 22:35:19 +0200,
Luigi Rizzo wrote:
>
> On Thu, Aug 21, 2008 at 03:11:56PM -0400, gnn@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?m23aky6ncl.wl%gnn>