Date: Thu, 13 Nov 2003 00:43:31 +0100 From: Andre Oppermann <oppermann@pipeline.ch> To: Jesper Skriver <jesper@skriver.dk> Cc: sam@errno.com Subject: Re: tcp hostcache and ip fastforward for review Message-ID: <3FB2C5A3.E6F33B07@pipeline.ch> References: <3FAE68FB.64D262FF@pipeline.ch> <20031112225326.GI41949@FreeBSD.org> <3FB2BE8A.6C880085@pipeline.ch> <20031112232341.GJ41949@skriver.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
Jesper Skriver wrote:
>
> On Thu, Nov 13, 2003 at 12:13:14AM +0100, Andre Oppermann wrote:
> > Jesper Skriver wrote:
> > >
> > > On Sun, Nov 09, 2003 at 05:19:07PM +0100, Andre Oppermann wrote:
> > > > Hello all,
> > > >
> > > > this patch contains three things (to be separated for committing):
> > ...
> > > > ip_fastforward
> > > >
> > > > - removes ip_flow forwarding code
> > > > - adds full direct process-to-completion IPv4 forwarding code
> > > > - handles ip fragmentation incl. hw support (ip_flow did not)
> > > > - supports ipfw and ipfilter (ip_flow did not)
> > > > - supports divert and ipfw fwd (ip_flow did not)
> > > > - drops anything it can't handle back to normal ip_input
> > >
> > > I have a few comments to this code, see inline, look for #jesper
> >
> > Answers also inline. [All whitespace bugs are fixed and omitted here]
>
> One comment at the bottom.
>
> > > Apart from that it looks good.
> >
> > Thanks for reviewing!
> >
> > > /Jesper
> > >
> > > > +int
> > > > +ip_fastforward(struct mbuf *m)
> > > > +{
> > ...
> > > > +
> > > > + /*
> > > > + * Only unicast IP, not from loopback, no L2 or IP broadcast,
> > > > + * no multicast, no INADDR_ANY
> > > > + */
> > > > + if ((m->m_pkthdr.rcvif->if_flags & IFF_LOOPBACK) ||
> > > > + (ntohl(ip->ip_src.s_addr) == (u_long)INADDR_BROADCAST) ||
> > >
> > > #jesper
> > > You will never see packets with a multicast source address.
> >
> > I hope so but we can never be sure. Here we look at what we've got
> > straight from the wire. Everything is possible there. I only need
> > to craft an apropriate packet...
>
> True, but do we really care if forwarded such a packet ?
Yes, never forward anything that should not be. Stop the problem
right here instead of letting it become a problem somewhere else.
> And if we want to check, we should just drop it directly instead of
> giving the packet to ip_input.
Makes sense.
Can we ever have a packet that has a source address with INADDR_BROADCAST
or IN_MULTICAST? I can't think of such a case.
Can we ever have a packet with destination address INADDR_ANY? Maybe
for BOOTP? But then the source address would be 0.0.0.0 too?
With fallback to ip_input() in all those cases I wanted to make sure that
I don't break any obscure hack or protocol I didn't think of.
--
Andre
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3FB2C5A3.E6F33B07>