On Wed, Dec 02, 2015 at 09:26:35AM +0000, Stuart Henderson wrote:
> On 2015/12/02 09:44, Martin Pieuchot wrote:
> > If the kernel tries to create (cloned) an ARP entry and found an
> > existing conflicting route, do not try to be clever and just bail.
> > 
> > I'm fine with rtalloc(9) taking the KERNEL_LOCK when cloning an entry
> > but I'd prefer the ARP layer to not try to delete anything in the hot
> > path.
> > 
> > If you entered a conflicting entry in your routing table, that's your
> > problem, you deal with it.
> > 
> > Ok?
> 
> Just trying to think it through a bit:
> 
> Where are these conflicting routes going to come from, if you already have
> a RTF_GATEWAY route it's not going to need to ARP for that address is it?
> Is this something to do with ICMP redirects?
> 
> It would be nice to recreate the conditions that cause it to make sure that
> this doesn't result in an ARP storm if the new entry can't be added and we
> keep on trying to resolve it.
> 

Mpi and I came to the conclusion that it should be impossible to get into
this case because you would need to clone this route from a route with a
gateway which is not really possible and so this feels a bit like belts
and suspenders here.

If you can manage to create this condition (cloned route with a gateway or
no llinfo connected) then I would like to know how :)

> 
> > Index: netinet/if_ether.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/if_ether.c,v
> > retrieving revision 1.191
> > diff -u -p -r1.191 if_ether.c
> > --- netinet/if_ether.c      1 Dec 2015 12:22:18 -0000       1.191
> > +++ netinet/if_ether.c      2 Dec 2015 08:40:13 -0000
> > @@ -707,12 +707,9 @@ arplookup(u_int32_t addr, int create, in
> >     flags = (create) ? (RT_REPORT|RT_RESOLVE) : 0;
> >  
> >     rt = rtalloc((struct sockaddr *)&sin, flags, tableid);
> > -   if (rt == NULL)
> > -           return (NULL);
> > -   if ((rt->rt_flags & RTF_GATEWAY) || (rt->rt_flags & RTF_LLINFO) == 0 ||
> > +   if (!rtisvalid(rt) || ISSET(rt->rt_flags, RTF_GATEWAY) ||
> > +       !ISSET(rt->rt_flags, RTF_LLINFO) ||
> >         rt->rt_gateway->sa_family != AF_LINK) {
> > -           if (create && (rt->rt_flags & RTF_CLONED))
> > -                   rtdeletemsg(rt, tableid);
> >             rtfree(rt);
> >             return (NULL);
> >     }
> > 
> 

-- 
:wq Claudio

Reply via email to