Re: tcp hostcache and ip fastforward for review

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