more arp cleanup

2015-12-02 Thread Claudio Jeker
More rt_ifp killing. This time in in_arpinput().
This function is a bit special because of the way we propagte multicast
and broadcast packets it is possible that multiple interfaces recieve the
same request. In most cases this is because of -- you can guess --
carp(4). So simplify these checks and make them more generic at the same
time (in the SINPROXY case). In other words only the interface holding the
proxy arp route will answer to the requests.

OK?
-- 
:wq Claudio

Index: if_ether.c
===
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.195
diff -u -p -r1.195 if_ether.c
--- if_ether.c  2 Dec 2015 18:38:19 -   1.195
+++ if_ether.c  2 Dec 2015 18:42:36 -
@@ -560,7 +560,7 @@ in_arpinput(struct mbuf *m)
   ether_sprintf(ea->arp_sha),
   ifp->if_xname);
goto out;
-   } else if (rt->rt_ifp != ifp) {
+   } else if (rt->rt_ifidx != ifp->if_index) {
 #if NCARP > 0
if (ifp->if_type != IFT_CARP)
 #endif
@@ -639,10 +639,9 @@ out:
rt = arplookup(itaddr.s_addr, 0, SIN_PROXY, rdomain);
if (rt == NULL)
goto out;
-#if NCARP > 0
-   if (rt->rt_ifp->if_type == IFT_CARP && ifp->if_type != IFT_CARP)
+   /* protect from possible duplicates only owner should respond */
+   if (rt->rt_ifidx != ifp->if_index)
goto out;
-#endif
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));



Re: more arp cleanup

2015-12-02 Thread Claudio Jeker
Last but of rt_ifp cleanup. Since we want to print the interface names in
those log messages we need to do the if_get/if_put dance there. Since this
is only in 2 places which should not be super common that should be fine
and with this arp should be MP save :)

-- 
:wq Claudio

Index: if_ether.c
===
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.196
diff -u -p -r1.196 if_ether.c
--- if_ether.c  2 Dec 2015 21:09:06 -   1.196
+++ if_ether.c  2 Dec 2015 21:16:14 -
@@ -565,14 +565,19 @@ in_arpinput(struct mbuf *m)
if (ifp->if_type != IFT_CARP)
 #endif
{
+   struct ifnet *rifp = if_get(
+   rt->rt_ifidx);
+   if (rifp == NULL)
+   goto out;
inet_ntop(AF_INET, ,
addr, sizeof(addr));
log(LOG_WARNING, "arp: attempt"
   " to overwrite entry for"
   " %s on %s by %s on %s\n",
-  addr, rt->rt_ifp->if_xname,
+  addr, rifp->if_xname,
   ether_sprintf(ea->arp_sha),
   ifp->if_xname);
+   if_put(rifp);
}
goto out;
} else {
@@ -587,13 +592,17 @@ in_arpinput(struct mbuf *m)
changed = 1;
}
} else if (!if_isconnected(ifp, rt->rt_ifidx)) {
+   struct ifnet *rifp = if_get(rt->rt_ifidx);
+   if (rifp == NULL)
+   goto out;
inet_ntop(AF_INET, , addr, sizeof(addr));
log(LOG_WARNING,
"arp: attempt to add entry for %s "
"on %s by %s on %s\n", addr,
-   rt->rt_ifp->if_xname,
+   rifp->if_xname,
ether_sprintf(ea->arp_sha),
ifp->if_xname);
+   if_put(rifp);
goto out;
}
sdl->sdl_alen = sizeof(ea->arp_sha);




Re: more arp cleanup

2015-12-02 Thread Alexander Bluhm
On Wed, Dec 02, 2015 at 10:19:23PM +0100, Claudio Jeker wrote:
> Last but of rt_ifp cleanup. Since we want to print the interface names in
> those log messages we need to do the if_get/if_put dance there. Since this
> is only in 2 places which should not be super common that should be fine
> and with this arp should be MP save :)

OK bluhm@

> 
> -- 
> :wq Claudio
> 
> Index: if_ether.c
> ===
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.196
> diff -u -p -r1.196 if_ether.c
> --- if_ether.c2 Dec 2015 21:09:06 -   1.196
> +++ if_ether.c2 Dec 2015 21:16:14 -
> @@ -565,14 +565,19 @@ in_arpinput(struct mbuf *m)
>   if (ifp->if_type != IFT_CARP)
>  #endif
>   {
> + struct ifnet *rifp = if_get(
> + rt->rt_ifidx);
> + if (rifp == NULL)
> + goto out;
>   inet_ntop(AF_INET, ,
>   addr, sizeof(addr));
>   log(LOG_WARNING, "arp: attempt"
>  " to overwrite entry for"
>  " %s on %s by %s on %s\n",
> -addr, rt->rt_ifp->if_xname,
> +addr, rifp->if_xname,
>  ether_sprintf(ea->arp_sha),
>  ifp->if_xname);
> + if_put(rifp);
>   }
>   goto out;
>   } else {
> @@ -587,13 +592,17 @@ in_arpinput(struct mbuf *m)
>   changed = 1;
>   }
>   } else if (!if_isconnected(ifp, rt->rt_ifidx)) {
> + struct ifnet *rifp = if_get(rt->rt_ifidx);
> + if (rifp == NULL)
> + goto out;
>   inet_ntop(AF_INET, , addr, sizeof(addr));
>   log(LOG_WARNING,
>   "arp: attempt to add entry for %s "
>   "on %s by %s on %s\n", addr,
> - rt->rt_ifp->if_xname,
> + rifp->if_xname,
>   ether_sprintf(ea->arp_sha),
>   ifp->if_xname);
> + if_put(rifp);
>   goto out;
>   }
>   sdl->sdl_alen = sizeof(ea->arp_sha);



Re: more arp cleanup

2015-12-02 Thread Martin Pieuchot
On 02/12/15(Wed) 22:19, Claudio Jeker wrote:
> Last but of rt_ifp cleanup. Since we want to print the interface names in
> those log messages we need to do the if_get/if_put dance there. Since this
> is only in 2 places which should not be super common that should be fine
> and with this arp should be MP save :)

ok mpi@

> 
> -- 
> :wq Claudio
> 
> Index: if_ether.c
> ===
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.196
> diff -u -p -r1.196 if_ether.c
> --- if_ether.c2 Dec 2015 21:09:06 -   1.196
> +++ if_ether.c2 Dec 2015 21:16:14 -
> @@ -565,14 +565,19 @@ in_arpinput(struct mbuf *m)
>   if (ifp->if_type != IFT_CARP)
>  #endif
>   {
> + struct ifnet *rifp = if_get(
> + rt->rt_ifidx);
> + if (rifp == NULL)
> + goto out;
>   inet_ntop(AF_INET, ,
>   addr, sizeof(addr));
>   log(LOG_WARNING, "arp: attempt"
>  " to overwrite entry for"
>  " %s on %s by %s on %s\n",
> -addr, rt->rt_ifp->if_xname,
> +addr, rifp->if_xname,
>  ether_sprintf(ea->arp_sha),
>  ifp->if_xname);
> + if_put(rifp);
>   }
>   goto out;
>   } else {
> @@ -587,13 +592,17 @@ in_arpinput(struct mbuf *m)
>   changed = 1;
>   }
>   } else if (!if_isconnected(ifp, rt->rt_ifidx)) {
> + struct ifnet *rifp = if_get(rt->rt_ifidx);
> + if (rifp == NULL)
> + goto out;
>   inet_ntop(AF_INET, , addr, sizeof(addr));
>   log(LOG_WARNING,
>   "arp: attempt to add entry for %s "
>   "on %s by %s on %s\n", addr,
> - rt->rt_ifp->if_xname,
> + rifp->if_xname,
>   ether_sprintf(ea->arp_sha),
>   ifp->if_xname);
> + if_put(rifp);
>   goto out;
>   }
>   sdl->sdl_alen = sizeof(ea->arp_sha);
>