On Fri, Apr 10, 2020 at 10:47:53AM +0200, Martin Pieuchot wrote:
> On 09/04/20(Thu) 20:22, Laurent Salle wrote:
> > On 08/04/2020 06.52, Martin Pieuchot wrote:
> >
> > > It's the same bug as reported by sthen@. Two interfaces in the same
> > > subnet
> > > have two identical cloning routes:
> >
> > I've been able to reproduce systematically the problem with an OpenBSD
> > virtual machine running the latest snapshot and two vio interface with
> > different priority connected to the same lan with dhcp.
>
> Thanks for the report! Diff below seems to fix the issue here, could
> you try it?
I'm not convinced that this is the right solution. In your diff you insert
the MAC received on one interface into the arp node of another interface.
This feels wrong, arp entries should never cross over interfaces.
For example if for some reasons the two interfaces have the same gateway
IP but use different MACs for that IP then this breaks.
Wouldn't a proper fix start with finding the right interface route after
the rtalloc() call (line 534)? I guess an additoinal changes are needed in
arplookup(). The system needs to clone from the proper interface cloning
route as well.
> Index: netinet/if_ether.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.242
> diff -u -p -r1.242 if_ether.c
> --- netinet/if_ether.c 7 Nov 2019 11:23:23 -0000 1.242
> +++ netinet/if_ether.c 10 Apr 2020 08:45:42 -0000
> @@ -559,6 +559,23 @@ in_arpinput(struct ifnet *ifp, struct mb
>
> KERNEL_LOCK();
> error = arpcache(ifp, ea, rt);
> + if (error == 0 && ISSET(rt->rt_flags, RTF_CACHED)) {
> + /*
> + * RTF_CACHED entry are not deleted as long as
> + * their parent gateway route is alive, so make
> + * sure to update its sibling which might be on
> + * a different interface to not leave them as
> + * unresolved.
> + */
> + while ((rt = rtable_iterate(rt)) != NULL) {
> + struct ifnet *ifp0;
> +
> + ifp0 = if_get(rt->rt_ifidx);
> + if (ifp0 != NULL)
> + error = arpcache(ifp0, ea, rt);
> + if_put(ifp0);
> + }
> + }
> KERNEL_UNLOCK();
> if (error)
> goto out;
>
--
:wq Claudio
