Re: Split rtinit()
On Thu, Aug 29, 2013 at 11:20:56AM +0200, Martin Pieuchot wrote: On 27/08/13(Tue) 10:44, Kenneth R Westerback wrote: On Tue, Aug 27, 2013 at 03:38:49PM +0200, Martin Pieuchot wrote: So I started to play with the routine table and I'm slowly trying to unify the various code paths to add and delete route entries. The diff below is a first step, it splits rtinit() into rt_add() and rt_delete() there should be no functional change. ok? That makes s much more sense. :-). mikeb@ pointed out that the names I picked were maybe too generic. This kept me thinking and I'd like to try to separate the logic of adding a route to network vs route to host first. Why? There is no difference between a host route and a network route. The fact that the host route has no netmask should not result into a different set of functions. In other words, forget this diff for the moment (: -- :wq Claudio
Re: Split rtinit()
On 27/08/13(Tue) 10:44, Kenneth R Westerback wrote: On Tue, Aug 27, 2013 at 03:38:49PM +0200, Martin Pieuchot wrote: So I started to play with the routine table and I'm slowly trying to unify the various code paths to add and delete route entries. The diff below is a first step, it splits rtinit() into rt_add() and rt_delete() there should be no functional change. ok? That makes s much more sense. :-). mikeb@ pointed out that the names I picked were maybe too generic. This kept me thinking and I'd like to try to separate the logic of adding a route to network vs route to host first. In other words, forget this diff for the moment (:
Split rtinit()
So I started to play with the routine table and I'm slowly trying to
unify the various code paths to add and delete route entries. The
diff below is a first step, it splits rtinit() into rt_add() and
rt_delete() there should be no functional change.
ok?
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c20 Aug 2013 09:14:22 - 1.262
+++ net/if.c27 Aug 2013 13:33:03 -
@@ -353,7 +353,7 @@ if_free_sadl(struct ifnet *ifp)
return;
s = splnet();
- rtinit(ifa, RTM_DELETE, 0);
+ rt_del(ifa, 0);
#if 0
ifa_del(ifp, ifa);
ifp-if_lladdr = NULL;
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:33:03 -
@@ -1069,11 +1069,10 @@ rt_maskedcopy(struct sockaddr *src, stru
* for an interface.
*/
int
-rtinit(struct ifaddr *ifa, int cmd, int flags)
+rt_add(struct ifaddr *ifa, int flags)
{
struct rtentry *rt;
- struct sockaddr *dst, *deldst;
- struct mbuf *m = NULL;
+ struct sockaddr *dst;
struct rtentry *nrt = NULL;
int error;
struct rt_addrinfo info;
@@ -1081,35 +1080,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
u_short rtableid = ifa-ifa_ifp-if_rdomain;
dst = flags RTF_HOST ? ifa-ifa_dstaddr : ifa-ifa_addr;
- if (cmd == RTM_DELETE) {
- if ((flags RTF_HOST) == 0 ifa-ifa_netmask) {
- m = m_get(M_DONTWAIT, MT_SONAME);
- if (m == NULL)
- return (ENOBUFS);
- deldst = mtod(m, struct sockaddr *);
- rt_maskedcopy(dst, deldst, ifa-ifa_netmask);
- dst = deldst;
- }
- if ((rt = rtalloc1(dst, 0, rtableid)) != NULL) {
- rt-rt_refcnt--;
- /* try to find the right route */
- while (rt rt-rt_ifa != ifa)
- rt = (struct rtentry *)
- ((struct radix_node *)rt)-rn_dupedkey;
- if (!rt) {
- if (m != NULL)
- (void) m_free(m);
- return (flags RTF_HOST ? EHOSTUNREACH
- : ENETUNREACH);
- }
- }
- }
bzero(info, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags | ifa-ifa_flags;
info.rti_info[RTAX_DST] = dst;
- if (cmd == RTM_ADD)
- info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
+ info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
info.rti_info[RTAX_LABEL] =
rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
@@ -1120,22 +1095,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
* change it to meet bsdi4 behavior.
*/
info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
- error = rtrequest1(cmd, info, RTP_CONNECTED, nrt, rtableid);
- if (cmd == RTM_DELETE) {
- if (error == 0 (rt = nrt) != NULL) {
- rt_newaddrmsg(cmd, ifa, error, nrt);
- if (rt-rt_refcnt = 0) {
- rt-rt_refcnt++;
- rtfree(rt);
- }
- }
- if (m != NULL)
- (void) m_free(m);
- }
- if (cmd == RTM_ADD error == 0 (rt = nrt) != NULL) {
+ error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, nrt, rtableid);
+ if (error == 0 (rt = nrt) != NULL) {
rt-rt_refcnt--;
if (rt-rt_ifa != ifa) {
- printf(rtinit: wrong ifa (%p) was (%p)\n,
+ printf(%s: wrong ifa (%p) was (%p)\n, __func__,
ifa, rt-rt_ifa);
if (rt-rt_ifa-ifa_rtrequest)
rt-rt_ifa-ifa_rtrequest(RTM_DELETE, rt, NULL);
@@ -1146,8 +1110,68 @@ rtinit(struct ifaddr *ifa, int cmd, int
if (ifa-ifa_rtrequest)
ifa-ifa_rtrequest(RTM_ADD, rt, NULL);
}
- rt_newaddrmsg(cmd, ifa, error, nrt);
+ rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
+ }
+
+ return (error);
+}
+
+int
+rt_del(struct ifaddr *ifa, int flags)
+{
+ struct rtentry *rt;
+ struct sockaddr *dst, *deldst;
+ struct mbuf *m = NULL;
+ struct rtentry *nrt = NULL;
+
Re: Split rtinit()
On Tue, Aug 27, 2013 at 03:38:49PM +0200, Martin Pieuchot wrote:
So I started to play with the routine table and I'm slowly trying to
unify the various code paths to add and delete route entries. The
diff below is a first step, it splits rtinit() into rt_add() and
rt_delete() there should be no functional change.
ok?
That makes s much more sense. :-).
ok krw@ (note: not a routing table guru!)
Ken
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c 20 Aug 2013 09:14:22 - 1.262
+++ net/if.c 27 Aug 2013 13:33:03 -
@@ -353,7 +353,7 @@ if_free_sadl(struct ifnet *ifp)
return;
s = splnet();
- rtinit(ifa, RTM_DELETE, 0);
+ rt_del(ifa, 0);
#if 0
ifa_del(ifp, ifa);
ifp-if_lladdr = NULL;
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:33:03 -
@@ -1069,11 +1069,10 @@ rt_maskedcopy(struct sockaddr *src, stru
* for an interface.
*/
int
-rtinit(struct ifaddr *ifa, int cmd, int flags)
+rt_add(struct ifaddr *ifa, int flags)
{
struct rtentry *rt;
- struct sockaddr *dst, *deldst;
- struct mbuf *m = NULL;
+ struct sockaddr *dst;
struct rtentry *nrt = NULL;
int error;
struct rt_addrinfo info;
@@ -1081,35 +1080,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
u_short rtableid = ifa-ifa_ifp-if_rdomain;
dst = flags RTF_HOST ? ifa-ifa_dstaddr : ifa-ifa_addr;
- if (cmd == RTM_DELETE) {
- if ((flags RTF_HOST) == 0 ifa-ifa_netmask) {
- m = m_get(M_DONTWAIT, MT_SONAME);
- if (m == NULL)
- return (ENOBUFS);
- deldst = mtod(m, struct sockaddr *);
- rt_maskedcopy(dst, deldst, ifa-ifa_netmask);
- dst = deldst;
- }
- if ((rt = rtalloc1(dst, 0, rtableid)) != NULL) {
- rt-rt_refcnt--;
- /* try to find the right route */
- while (rt rt-rt_ifa != ifa)
- rt = (struct rtentry *)
- ((struct radix_node *)rt)-rn_dupedkey;
- if (!rt) {
- if (m != NULL)
- (void) m_free(m);
- return (flags RTF_HOST ? EHOSTUNREACH
- : ENETUNREACH);
- }
- }
- }
bzero(info, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags | ifa-ifa_flags;
info.rti_info[RTAX_DST] = dst;
- if (cmd == RTM_ADD)
- info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
+ info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
info.rti_info[RTAX_LABEL] =
rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
@@ -1120,22 +1095,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
* change it to meet bsdi4 behavior.
*/
info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
- error = rtrequest1(cmd, info, RTP_CONNECTED, nrt, rtableid);
- if (cmd == RTM_DELETE) {
- if (error == 0 (rt = nrt) != NULL) {
- rt_newaddrmsg(cmd, ifa, error, nrt);
- if (rt-rt_refcnt = 0) {
- rt-rt_refcnt++;
- rtfree(rt);
- }
- }
- if (m != NULL)
- (void) m_free(m);
- }
- if (cmd == RTM_ADD error == 0 (rt = nrt) != NULL) {
+ error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, nrt, rtableid);
+ if (error == 0 (rt = nrt) != NULL) {
rt-rt_refcnt--;
if (rt-rt_ifa != ifa) {
- printf(rtinit: wrong ifa (%p) was (%p)\n,
+ printf(%s: wrong ifa (%p) was (%p)\n, __func__,
ifa, rt-rt_ifa);
if (rt-rt_ifa-ifa_rtrequest)
rt-rt_ifa-ifa_rtrequest(RTM_DELETE, rt, NULL);
@@ -1146,8 +1110,68 @@ rtinit(struct ifaddr *ifa, int cmd, int
if (ifa-ifa_rtrequest)
ifa-ifa_rtrequest(RTM_ADD, rt, NULL);
}
- rt_newaddrmsg(cmd, ifa, error, nrt);
+ rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
+ }
+
+ return (error);
+}
+
+int
+rt_del(struct ifaddr *ifa, int flags)
+{
+ struct rtentry *rt;
+
