On Mon, Oct 26, 2015 at 07:08:19PM +0100, Martin Pieuchot wrote:
> This rewrites the code to send an ARP reply to no use ``myaddr''. The
> goal is to get rid of the per-ifp address list iterations.
>
> Instead do two route lookups.
>
> ok?
Should the "reply:" label stay before the "if (op != ARPOP_REQUEST)"?
Otherwise you could send a reply in response to a reply.
bluhm
>
> Index: netinet/if_ether.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.177
> diff -u -p -r1.177 if_ether.c
> --- netinet/if_ether.c 25 Oct 2015 11:58:11 -0000 1.177
> +++ netinet/if_ether.c 26 Oct 2015 18:01:12 -0000
> @@ -519,8 +519,9 @@ in_arpinput(struct mbuf *m)
> #endif
> char addr[INET_ADDRSTRLEN];
> int op, changed = 0;
> - unsigned int len;
> + unsigned int len, rdomain;
>
> + rdomain = rtable_l2(m->m_pkthdr.ph_rtableid);
> ifp = if_get(m->m_pkthdr.ph_ifidx);
> if (ifp == NULL) {
> m_freem(m);
> @@ -606,7 +607,7 @@ in_arpinput(struct mbuf *m)
> goto reply;
> }
> rt = arplookup(isaddr.s_addr, itaddr.s_addr == myaddr.s_addr, 0,
> - rtable_l2(m->m_pkthdr.ph_rtableid));
> + rdomain);
> if (rt != NULL && (sdl = satosdl(rt->rt_gateway)) != NULL) {
> la = (struct llinfo_arp *)rt->rt_llinfo;
> if (sdl->sdl_alen) {
> @@ -693,32 +694,29 @@ in_arpinput(struct mbuf *m)
> }
> }
> }
> -reply:
> - if (op != ARPOP_REQUEST) {
> -out:
> - rtfree(rt);
> - if_put(ifp);
> - m_freem(m);
> - return;
> - }
>
> + if (op != ARPOP_REQUEST)
> + goto out;
> rtfree(rt);
> - if (itaddr.s_addr == myaddr.s_addr) {
> - /* I am the target */
> - memcpy(ea->arp_tha, ea->arp_sha, sizeof(ea->arp_sha));
> - memcpy(ea->arp_sha, enaddr, sizeof(ea->arp_sha));
> - } else {
> - rt = arplookup(itaddr.s_addr, 0, SIN_PROXY,
> - rtable_l2(m->m_pkthdr.ph_rtableid));
> - if (rt == NULL)
> - goto out;
> - if (rt->rt_ifp->if_type == IFT_CARP && ifp->if_type != IFT_CARP)
> +
> +
> +reply:
> + /*
> + * Reply if we have a local or proxy entry.
> + */
> + rt = arplookup(itaddr.s_addr, 0, SIN_PROXY, rdomain);
> + if (!rtisvalid(rt)) {
> + rt = arplookup(itaddr.s_addr, 0, 0, rdomain);
> + if (!rtisvalid(rt) || !ISSET(rt->rt_flags, RTF_LOCAL))
> goto out;
> - memcpy(ea->arp_tha, ea->arp_sha, sizeof(ea->arp_sha));
> - sdl = satosdl(rt->rt_gateway);
> - memcpy(ea->arp_sha, LLADDR(sdl), sizeof(ea->arp_sha));
> - rtfree(rt);
> }
> + if (rt->rt_ifp->if_type == IFT_CARP && ifp->if_type != IFT_CARP)
> + goto out;
> + memcpy(ea->arp_tha, ea->arp_sha, sizeof(ea->arp_sha));
> + sdl = satosdl(rt->rt_gateway);
> + memcpy(ea->arp_sha, LLADDR(sdl), sizeof(ea->arp_sha));
> + rtfree(rt);
> +
>
> memcpy(ea->arp_tpa, ea->arp_spa, sizeof(ea->arp_spa));
> memcpy(ea->arp_spa, &itaddr, sizeof(ea->arp_spa));
> @@ -738,6 +736,11 @@ out:
> ifp->if_output(ifp, m, &sa, NULL);
> if_put(ifp);
> return;
> +
> +out:
> + rtfree(rt);
> + if_put(ifp);
> + m_freem(m);
> }
>
> /*
