Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 8 May 2014 20:57:13 +0000 (UTC)
From:      "Alexander V. Chernikov" <melifaro@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org
Subject:   svn commit: r265715 - in stable/9/sys: net netinet
Message-ID:  <201405082057.s48KvDVE057980@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: melifaro
Date: Thu May  8 20:57:13 2014
New Revision: 265715
URL: http://svnweb.freebsd.org/changeset/base/265715

Log:
  Merge 260488, r260508.
  
  r260488:
    Split rt_newaddrmsg_fib() into two different functions.
    Adding/deleting interface addresses involves access to 3 different subsystems,
    int different parts of code. Each call can fail, so reporting successful
    operation by rtsock in the middle of the process error-prone.
  
    Further split routing notification API and actual rtsock calls via creating
    public-available rt_addrmsg() / rt_routemsg() functions with "private"
    rtsock_* backend.
  
  r260508:
    Simplify inet alias handling code: if we're adding/removing alias which
    has the same prefix as some other alias on the same interface, use
    newly-added rt_addrmsg() instead of hand-rolled in_addralias_rtmsg().
  
    This eliminates the following rtsock messages:
  
    Pinned RTM_ADD for prefix (for alias addition).
    Pinned RTM_DELETE for prefix (for alias withdrawal).
  
    Example (got 10.0.0.1/24 on vlan4, playing with 10.0.0.2/24):
  
    before commit, addition:
  
      got message of size 116 on Fri Jan 10 14:13:15 2014
      RTM_NEWADDR: address being added to iface: len 116, metric 0, flags:
      sockaddrs: <NETMASK,IFP,IFA,BRD>
       255.255.255.0 vlan4:8.0.27.c5.29.d4 10.0.0.2 10.0.0.255
  
      got message of size 192 on Fri Jan 10 14:13:15 2014
      RTM_ADD: Add Route: len 192, pid: 0, seq 0, errno 0, flags:<UP,PINNED>
      locks:  inits:
      sockaddrs: <DST,GATEWAY,NETMASK>
       10.0.0.0 10.0.0.2 (255) ffff ffff ff
  
    after commit, addition:
  
      got message of size 116 on Fri Jan 10 13:56:26 2014
      RTM_NEWADDR: address being added to iface: len 116, metric 0, flags:
      sockaddrs: <NETMASK,IFP,IFA,BRD>
       255.255.255.0 vlan4:8.0.27.c5.29.d4 14.0.0.2 14.0.0.255
  
    before commit, wihdrawal:
  
      got message of size 192 on Fri Jan 10 13:58:59 2014
      RTM_DELETE: Delete Route: len 192, pid: 0, seq 0, errno 0, flags:<UP,PINNED>
      locks:  inits:
      sockaddrs: <DST,GATEWAY,NETMASK>
       10.0.0.0 10.0.0.2 (255) ffff ffff ff
  
      got message of size 116 on Fri Jan 10 13:58:59 2014
      RTM_DELADDR: address being removed from iface: len 116, metric 0, flags:
      sockaddrs: <NETMASK,IFP,IFA,BRD>
       255.255.255.0 vlan4:8.0.27.c5.29.d4 10.0.0.2 10.0.0.255
  
    adter commit, withdrawal:
  
      got message of size 116 on Fri Jan 10 14:14:11 2014
      RTM_DELADDR: address being removed from iface: len 116, metric 0, flags:
      sockaddrs: <NETMASK,IFP,IFA,BRD>
       255.255.255.0 vlan4:8.0.27.c5.29.d4 10.0.0.2 10.0.0.255
  
    Sending both RTM_ADD/RTM_DELETE messages to rtsock is completely wrong
    (and requires some hacks to keep prefix in route table on RTM_DELETE).
  
    I've tested this change with quagga (no change) and bird (*).
  
    bird alias handling is already broken in *BSD sysdep code, so nothing
    changes here, too.
  
    I'm going to MFC this change if there will be no complains about behavior
    change.
  
    While here, fix some style(9) bugs introduced by r260488
    (pointed by glebius and bde).

Modified:
  stable/9/sys/net/route.c
  stable/9/sys/net/route.h
  stable/9/sys/net/rtsock.c
  stable/9/sys/netinet/in.c
Directory Properties:
  stable/9/sys/   (props changed)
  stable/9/sys/net/   (props changed)

Modified: stable/9/sys/net/route.c
==============================================================================
--- stable/9/sys/net/route.c	Thu May  8 20:52:25 2014	(r265714)
+++ stable/9/sys/net/route.c	Thu May  8 20:57:13 2014	(r265715)
@@ -37,6 +37,7 @@
 #include "opt_inet.h"
 #include "opt_inet6.h"
 #include "opt_route.h"
+#include "opt_sctp.h"
 #include "opt_mrouting.h"
 #include "opt_mpath.h"
 
@@ -86,6 +87,14 @@
 #define	RT_NUMFIBS	1
 #endif
 
+#if defined(INET) || defined(INET6)
+#ifdef SCTP
+extern void sctp_addr_change(struct ifaddr *ifa, int cmd);
+#endif /* SCTP */
+#endif
+
+
+/* This is read-only.. */
 u_int rt_numfibs = RT_NUMFIBS;
 SYSCTL_UINT(_net, OID_AUTO, fibs, CTLFLAG_RD, &rt_numfibs, 0, "");
 /*
@@ -125,7 +134,8 @@ VNET_DEFINE(int, rttrash);		/* routes no
 
 
 /* compare two sockaddr structures */
-#define	sa_equal(a1, a2) (bcmp((a1), (a2), (a1)->sa_len) == 0)
+#define	sa_equal(a1, a2) (((a1)->sa_len == (a2)->sa_len) && \
+    (bcmp((a1), (a2), (a1)->sa_len) == 0))
 
 /*
  * Convert a 'struct radix_node *' to a 'struct rtentry *'.
@@ -1748,3 +1758,89 @@ rtinit(struct ifaddr *ifa, int cmd, int 
 	}
 	return (rtinit1(ifa, cmd, flags, fib));
 }
+
+/*
+ * Announce interface address arrival/withdraw
+ * Returns 0 on success.
+ */
+int
+rt_addrmsg(int cmd, struct ifaddr *ifa, int fibnum)
+{
+
+	KASSERT(cmd == RTM_ADD || cmd == RTM_DELETE,
+	    ("unexpected cmd %d", cmd));
+	
+	KASSERT(fibnum == RT_ALL_FIBS || (fibnum >= 0 && fibnum < rt_numfibs),
+	    ("%s: fib out of range 0 <=%d<%d", __func__, fibnum, rt_numfibs));
+
+	return (rtsock_addrmsg(cmd, ifa, fibnum));
+}
+
+/*
+ * Announce route addition/removal.
+ * Users of this function MUST validate input data BEFORE calling.
+ * However we have to be able to handle invalid data:
+ * if some userland app sends us "invalid" route message (invalid mask,
+ * no dst, wrong address families, etc...) we need to pass it back
+ * to app (and any other rtsock consumers) with rtm_errno field set to
+ * non-zero value.
+ * Returns 0 on success.
+ */
+int
+rt_routemsg(int cmd, struct ifnet *ifp, int error, struct rtentry *rt,
+    int fibnum)
+{
+
+	KASSERT(cmd == RTM_ADD || cmd == RTM_DELETE,
+	    ("unexpected cmd %d", cmd));
+	
+	KASSERT(fibnum == RT_ALL_FIBS || (fibnum >= 0 && fibnum < rt_numfibs),
+	    ("%s: fib out of range 0 <=%d<%d", __func__, fibnum, rt_numfibs));
+
+	KASSERT(rt_key(rt) != NULL, (":%s: rt_key must be supplied", __func__));
+
+	return (rtsock_routemsg(cmd, ifp, error, rt, fibnum));
+}
+
+void
+rt_newaddrmsg(int cmd, struct ifaddr *ifa, int error, struct rtentry *rt)
+{
+
+	rt_newaddrmsg_fib(cmd, ifa, error, rt, RT_ALL_FIBS);
+}
+
+/*
+ * This is called to generate messages from the routing socket
+ * indicating a network interface has had addresses associated with it.
+ */
+void
+rt_newaddrmsg_fib(int cmd, struct ifaddr *ifa, int error, struct rtentry *rt,
+    int fibnum)
+{
+
+	KASSERT(cmd == RTM_ADD || cmd == RTM_DELETE,
+		("unexpected cmd %u", cmd));
+	KASSERT(fibnum == RT_ALL_FIBS || (fibnum >= 0 && fibnum < rt_numfibs),
+	    ("%s: fib out of range 0 <=%d<%d", __func__, fibnum, rt_numfibs));
+
+#if defined(INET) || defined(INET6)
+#ifdef SCTP
+	/*
+	 * notify the SCTP stack
+	 * this will only get called when an address is added/deleted
+	 * XXX pass the ifaddr struct instead if ifa->ifa_addr...
+	 */
+	sctp_addr_change(ifa, cmd);
+#endif /* SCTP */
+#endif
+	if (cmd == RTM_ADD) {
+		rt_addrmsg(cmd, ifa, fibnum);
+		if (rt != NULL)
+			rt_routemsg(cmd, ifa->ifa_ifp, error, rt, fibnum);
+	} else {
+		if (rt != NULL)
+			rt_routemsg(cmd, ifa->ifa_ifp, error, rt, fibnum);
+		rt_addrmsg(cmd, ifa, fibnum);
+	}
+}
+

Modified: stable/9/sys/net/route.h
==============================================================================
--- stable/9/sys/net/route.h	Thu May  8 20:52:25 2014	(r265714)
+++ stable/9/sys/net/route.h	Thu May  8 20:57:13 2014	(r265715)
@@ -94,6 +94,7 @@ struct rt_metrics {
 #define	RT_DEFAULT_FIB	0	/* Explicitly mark fib=0 restricted cases */
 #define	RT_ALL_FIBS	-1	/* Announce event for every fib */
 extern u_int rt_numfibs;	/* number of usable routing tables */
+extern u_int rt_add_addr_allfibs;	/* Announce interfaces to all fibs */
 /*
  * XXX kernel function pointer `rt_output' is visible to applications.
  */
@@ -366,10 +367,15 @@ void	 rt_missmsg(int, struct rt_addrinfo
 void	 rt_missmsg_fib(int, struct rt_addrinfo *, int, int, int);
 void	 rt_newaddrmsg(int, struct ifaddr *, int, struct rtentry *);
 void	 rt_newaddrmsg_fib(int, struct ifaddr *, int, struct rtentry *, int);
+int	 rt_addrmsg(int, struct ifaddr *, int);
+int	 rt_routemsg(int, struct ifnet *ifp, int, struct rtentry *, int);
 void	 rt_newmaddrmsg(int, struct ifmultiaddr *);
 int	 rt_setgate(struct rtentry *, struct sockaddr *, struct sockaddr *);
 void 	 rt_maskedcopy(struct sockaddr *, struct sockaddr *, struct sockaddr *);
 
+int	rtsock_addrmsg(int, struct ifaddr *, int);
+int	rtsock_routemsg(int, struct ifnet *ifp, int, struct rtentry *, int);
+
 /*
  * Note the following locking behavior:
  *

Modified: stable/9/sys/net/rtsock.c
==============================================================================
--- stable/9/sys/net/rtsock.c	Thu May  8 20:52:25 2014	(r265714)
+++ stable/9/sys/net/rtsock.c	Thu May  8 20:57:13 2014	(r265715)
@@ -30,7 +30,6 @@
  * $FreeBSD$
  */
 #include "opt_compat.h"
-#include "opt_sctp.h"
 #include "opt_mpath.h"
 #include "opt_inet.h"
 #include "opt_inet6.h"
@@ -67,12 +66,6 @@
 #include <netinet6/scope6_var.h>
 #endif
 
-#if defined(INET) || defined(INET6)
-#ifdef SCTP
-extern void sctp_addr_change(struct ifaddr *ifa, int cmd);
-#endif /* SCTP */
-#endif
-
 #ifdef COMPAT_FREEBSD32
 #include <sys/mount.h>
 #include <compat/freebsd32/freebsd32.h>
@@ -1264,89 +1257,92 @@ rt_ifmsg(struct ifnet *ifp)
 }
 
 /*
- * This is called to generate messages from the routing socket
- * indicating a network interface has had addresses associated with it.
- * if we ever reverse the logic and replace messages TO the routing
- * socket indicate a request to configure interfaces, then it will
- * be unnecessary as the routing socket will automatically generate
- * copies of it.
+ * Announce interface address arrival/withdraw.
+ * Please do not call directly, use rt_addrmsg().
+ * Assume input data to be valid.
+ * Returns 0 on success.
  */
-void
-rt_newaddrmsg_fib(int cmd, struct ifaddr *ifa, int error, struct rtentry *rt,
-    int fibnum)
+int
+rtsock_addrmsg(int cmd, struct ifaddr *ifa, int fibnum)
 {
 	struct rt_addrinfo info;
-	struct sockaddr *sa = NULL;
-	int pass;
-	struct mbuf *m = NULL;
+	struct sockaddr *sa;
+	int ncmd;
+	struct mbuf *m;
+	struct ifa_msghdr *ifam;
 	struct ifnet *ifp = ifa->ifa_ifp;
 
-	KASSERT(cmd == RTM_ADD || cmd == RTM_DELETE,
-		("unexpected cmd %u", cmd));
-#if defined(INET) || defined(INET6)
-#ifdef SCTP
-	/*
-	 * notify the SCTP stack
-	 * this will only get called when an address is added/deleted
-	 * XXX pass the ifaddr struct instead if ifa->ifa_addr...
-	 */
-	sctp_addr_change(ifa, cmd);
-#endif /* SCTP */
-#endif
 	if (route_cb.any_count == 0)
-		return;
-	for (pass = 1; pass < 3; pass++) {
-		bzero((caddr_t)&info, sizeof(info));
-		if ((cmd == RTM_ADD && pass == 1) ||
-		    (cmd == RTM_DELETE && pass == 2)) {
-			struct ifa_msghdr *ifam;
-			int ncmd = cmd == RTM_ADD ? RTM_NEWADDR : RTM_DELADDR;
+		return (0);
 
-			info.rti_info[RTAX_IFA] = sa = ifa->ifa_addr;
-			info.rti_info[RTAX_IFP] = ifp->if_addr->ifa_addr;
-			info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
-			info.rti_info[RTAX_BRD] = ifa->ifa_dstaddr;
-			if ((m = rt_msg1(ncmd, &info)) == NULL)
-				continue;
-			ifam = mtod(m, struct ifa_msghdr *);
-			ifam->ifam_index = ifp->if_index;
-			ifam->ifam_metric = ifa->ifa_metric;
-			ifam->ifam_flags = ifa->ifa_flags;
-			ifam->ifam_addrs = info.rti_addrs;
-		}
-		if ((cmd == RTM_ADD && pass == 2) ||
-		    (cmd == RTM_DELETE && pass == 1)) {
-			struct rt_msghdr *rtm;
+	ncmd = cmd == RTM_ADD ? RTM_NEWADDR : RTM_DELADDR;
 
-			if (rt == NULL)
-				continue;
-			info.rti_info[RTAX_NETMASK] = rt_mask(rt);
-			info.rti_info[RTAX_DST] = sa = rt_key(rt);
-			info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
-			if ((m = rt_msg1(cmd, &info)) == NULL)
-				continue;
-			rtm = mtod(m, struct rt_msghdr *);
-			rtm->rtm_index = ifp->if_index;
-			rtm->rtm_flags |= rt->rt_flags;
-			rtm->rtm_errno = error;
-			rtm->rtm_addrs = info.rti_addrs;
-		}
-		if (fibnum != RT_ALL_FIBS) {
-			KASSERT(fibnum >= 0 && fibnum < rt_numfibs, ("%s: "
-			    "fibnum out of range 0 <= %d < %d", __func__,
-			     fibnum, rt_numfibs));
-			M_SETFIB(m, fibnum);
-			m->m_flags |= RTS_FILTER_FIB;
-		}
-		rt_dispatch(m, sa ? sa->sa_family : AF_UNSPEC);
+	bzero((caddr_t)&info, sizeof(info));
+	info.rti_info[RTAX_IFA] = sa = ifa->ifa_addr;
+	info.rti_info[RTAX_IFP] = ifp->if_addr->ifa_addr;
+	info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
+	info.rti_info[RTAX_BRD] = ifa->ifa_dstaddr;
+	if ((m = rt_msg1(ncmd, &info)) == NULL)
+		return (ENOBUFS);
+	ifam = mtod(m, struct ifa_msghdr *);
+	ifam->ifam_index = ifp->if_index;
+	ifam->ifam_metric = ifa->ifa_metric;
+	ifam->ifam_flags = ifa->ifa_flags;
+	ifam->ifam_addrs = info.rti_addrs;
+
+	if (fibnum != RT_ALL_FIBS) {
+		M_SETFIB(m, fibnum);
+		m->m_flags |= RTS_FILTER_FIB;
 	}
+
+	rt_dispatch(m, sa ? sa->sa_family : AF_UNSPEC);
+
+	return (0);
 }
 
-void
-rt_newaddrmsg(int cmd, struct ifaddr *ifa, int error, struct rtentry *rt)
+/*
+ * Announce route addition/removal.
+ * Please do not call directly, use rt_routemsg().
+ * Note that @rt data MAY be inconsistent/invalid:
+ * if some userland app sends us "invalid" route message (invalid mask,
+ * no dst, wrong address families, etc...) we need to pass it back
+ * to app (and any other rtsock consumers) with rtm_errno field set to
+ * non-zero value.
+ *
+ * Returns 0 on success.
+ */
+int
+rtsock_routemsg(int cmd, struct ifnet *ifp, int error, struct rtentry *rt,
+    int fibnum)
 {
+	struct rt_addrinfo info;
+	struct sockaddr *sa;
+	struct mbuf *m;
+	struct rt_msghdr *rtm;
+
+	if (route_cb.any_count == 0)
+		return (0);
 
-	rt_newaddrmsg_fib(cmd, ifa, error, rt, RT_ALL_FIBS);
+	bzero((caddr_t)&info, sizeof(info));
+	info.rti_info[RTAX_NETMASK] = rt_mask(rt);
+	info.rti_info[RTAX_DST] = sa = rt_key(rt);
+	info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
+	if ((m = rt_msg1(cmd, &info)) == NULL)
+		return (ENOBUFS);
+	rtm = mtod(m, struct rt_msghdr *);
+	rtm->rtm_index = ifp->if_index;
+	rtm->rtm_flags |= rt->rt_flags;
+	rtm->rtm_errno = error;
+	rtm->rtm_addrs = info.rti_addrs;
+
+	if (fibnum != RT_ALL_FIBS) {
+		M_SETFIB(m, fibnum);
+		m->m_flags |= RTS_FILTER_FIB;
+	}
+
+	rt_dispatch(m, sa ? sa->sa_family : AF_UNSPEC);
+
+	return (0);
 }
 
 /*

Modified: stable/9/sys/netinet/in.c
==============================================================================
--- stable/9/sys/netinet/in.c	Thu May  8 20:52:25 2014	(r265714)
+++ stable/9/sys/netinet/in.c	Thu May  8 20:57:13 2014	(r265715)
@@ -961,45 +961,6 @@ in_ifinit(struct ifnet *ifp, struct in_i
 	    ? RTF_HOST : 0)
 
 /*
- * Generate a routing message when inserting or deleting
- * an interface address alias.
- */
-static void in_addralias_rtmsg(int cmd, struct in_addr *prefix,
-    struct in_ifaddr *target)
-{
-	struct route pfx_ro;
-	struct sockaddr_in *pfx_addr;
-	struct rtentry msg_rt;
-
-	/* QL: XXX
-	 * This is a bit questionable because there is no
-	 * additional route entry added/deleted for an address
-	 * alias. Therefore this route report is inaccurate.
-	 */
-	bzero(&pfx_ro, sizeof(pfx_ro));
-	pfx_addr = (struct sockaddr_in *)(&pfx_ro.ro_dst);
-	pfx_addr->sin_len = sizeof(*pfx_addr);
-	pfx_addr->sin_family = AF_INET;
-	pfx_addr->sin_addr = *prefix;
-	rtalloc_ign_fib(&pfx_ro, 0, 0);
-	if (pfx_ro.ro_rt != NULL) {
-		msg_rt = *pfx_ro.ro_rt;
-
-		/* QL: XXX
-		 * Point the gateway to the new interface
-		 * address as if a new prefix route entry has
-		 * been added through the new address alias.
-		 * All other parts of the rtentry is accurate,
-		 * e.g., rt_key, rt_mask, rt_ifp etc.
-		 */
-		msg_rt.rt_gateway = (struct sockaddr *)&target->ia_addr;
-		rt_newaddrmsg(cmd, (struct ifaddr *)target, 0, &msg_rt);
-		RTFREE(pfx_ro.ro_rt);
-	}
-	return;
-}
-
-/*
  * Check if we have a route for the given prefix already or add one accordingly.
  */
 static int
@@ -1007,7 +968,7 @@ in_addprefix(struct in_ifaddr *target, i
 {
 	struct in_ifaddr *ia;
 	struct in_addr prefix, mask, p, m;
-	int error;
+	int error, fibnum;
 
 	if ((flags & RTF_HOST) != 0) {
 		prefix = target->ia_dstaddr.sin_addr;
@@ -1018,6 +979,8 @@ in_addprefix(struct in_ifaddr *target, i
 		prefix.s_addr &= mask.s_addr;
 	}
 
+	fibnum = rt_add_addr_allfibs ? RT_ALL_FIBS : target->ia_ifp->if_fib;
+
 	IN_IFADDR_RLOCK();
 	TAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) {
 		if (rtinitflags(ia)) {
@@ -1054,7 +1017,7 @@ in_addprefix(struct in_ifaddr *target, i
 				IN_IFADDR_RUNLOCK();
 				return (EEXIST);
 			} else {
-				in_addralias_rtmsg(RTM_ADD, &prefix, target);
+				rt_addrmsg(RTM_ADD, &target->ia_ifa, fibnum);
 				IN_IFADDR_RUNLOCK();
 				return (0);
 			}
@@ -1083,9 +1046,11 @@ in_scrubprefix(struct in_ifaddr *target,
 {
 	struct in_ifaddr *ia;
 	struct in_addr prefix, mask, p;
-	int error = 0;
+	int error = 0, fibnum;
 	struct sockaddr_in prefix0, mask0;
 
+	fibnum = rt_add_addr_allfibs ? RT_ALL_FIBS : target->ia_ifp->if_fib;
+
 	/*
 	 * Remove the loopback route to the interface address.
 	 * The "useloopback" setting is not consulted because if the
@@ -1137,7 +1102,7 @@ in_scrubprefix(struct in_ifaddr *target,
 	}
 
 	if ((target->ia_flags & IFA_ROUTE) == 0) {
-		in_addralias_rtmsg(RTM_DELETE, &prefix, target);
+		rt_addrmsg(RTM_DELETE, &target->ia_ifa, fibnum);
 		return (0);
 	}
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201405082057.s48KvDVE057980>