On 07/01/15(Wed) 19:00, Florian Riehm wrote:
> Hi Martin,
>
> Thanks for your diff! Regardless of my problem it makes our code
> more clear. The loop in rt_newaddrmsg() was ugly.
>
> >
> > Here's a diff that should generate a RTM_ADD message for every CLONING
> > route added while keeping the existing RTM_NEWADDR/RTM_DELADDR logic.
> >
> > dhclient(8) is happy with this change, does it fix your use case too?
>
> There is a small bug in your diff of route.c, because rt_ifa_del() is
> never called with flag RTF_CLONING, so ospfd doesn't notice if an address
> gets deleted.
>
> I was thinking about this three options to fix the problem:
> 1.) Check with rt->rt_flags instead of flags for RTF_CLONING flag:
> -if (flags & (RTF_LOCAL|RTF_CLONING))
> +if (rt->rt_flags & (RTF_LOCAL|RTF_CLONING))
>
> 2) Add RTF_CLONING to flags argument at certain calls of rt_ifa_add() /
> rt_ifa_delete()
>
> 3.) Remove the check completely and call always rt_sendmsg()
>
> At the moment I would prefer the 3. solution. After rtrequest1() has been
> called, we check for error and check if rt ist not NULL. If this conditions
> are
> true we have added or deleted a route. Why should we not send a route message
> then?
> Below an updated diff for route.c with my fix.
I agree with you, we should always send RTM_{ADD,DELETE} messages.
Complete diff below, anybody wants to ok it?
Index: net/route.c
===================================================================
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.198
diff -u -p -r1.198 route.c
--- net/route.c 8 Jan 2015 15:05:44 -0000 1.198
+++ net/route.c 12 Jan 2015 13:53:02 -0000
@@ -382,11 +382,13 @@ void
rt_sendmsg(struct rtentry *rt, int cmd, u_int rtableid)
{
struct rt_addrinfo info;
+ struct sockaddr_rtlabel sa_rl;
- bzero(&info, sizeof(info));
+ memset(&info, 0, sizeof(info));
info.rti_info[RTAX_DST] = rt_key(rt);
info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
info.rti_info[RTAX_NETMASK] = rt_mask(rt);
+ info.rti_info[RTAX_LABEL] = rtlabel_id2sa(rt->rt_labelid, &sa_rl);
if (rt->rt_ifp != NULL) {
info.rti_info[RTAX_IFP] =(struct sockaddr *)rt->rt_ifp->if_sadl;
info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
@@ -1138,7 +1140,8 @@ rt_ifa_add(struct ifaddr *ifa, int flags
* userland that a new address has been added.
*/
if (flags & RTF_LOCAL)
- rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
+ rt_sendaddrmsg(nrt, RTM_NEWADDR);
+ rt_sendmsg(nrt, RTM_ADD, rtableid);
}
return (error);
}
@@ -1193,7 +1196,8 @@ rt_ifa_del(struct ifaddr *ifa, int flags
error = rtrequest1(RTM_DELETE, &info, prio, &nrt, rtableid);
if (error == 0 && (rt = nrt) != NULL) {
if (flags & RTF_LOCAL)
- rt_newaddrmsg(RTM_DELETE, ifa, error, nrt);
+ rt_sendaddrmsg(nrt, RTM_DELADDR);
+ rt_sendmsg(nrt, RTM_DELETE, rtableid);
if (rt->rt_refcnt <= 0) {
rt->rt_refcnt++;
rtfree(rt);
Index: net/route.h
===================================================================
RCS file: /home/ncvs/src/sys/net/route.h,v
retrieving revision 1.103
diff -u -p -r1.103 route.h
--- net/route.h 8 Jan 2015 15:05:44 -0000 1.103
+++ net/route.h 12 Jan 2015 13:53:02 -0000
@@ -357,9 +357,9 @@ void rt_ifannouncemsg(struct ifnet *, i
void rt_maskedcopy(struct sockaddr *,
struct sockaddr *, struct sockaddr *);
void rt_sendmsg(struct rtentry *, int, u_int);
+void rt_sendaddrmsg(struct rtentry *, int);
void rt_missmsg(int, struct rt_addrinfo *, int, struct ifnet *, int,
u_int);
-void rt_newaddrmsg(int, struct ifaddr *, int, struct rtentry *);
int rt_setgate(struct rtentry *, struct sockaddr *,
struct sockaddr *, u_int);
int rt_checkgate(struct ifnet *, struct rtentry *, struct sockaddr *,
Index: net/rtsock.c
===================================================================
RCS file: /home/ncvs/src/sys/net/rtsock.c,v
retrieving revision 1.155
diff -u -p -r1.155 rtsock.c
--- net/rtsock.c 19 Dec 2014 18:57:17 -0000 1.155
+++ net/rtsock.c 12 Jan 2015 13:53:02 -0000
@@ -1137,70 +1137,36 @@ rt_ifmsg(struct ifnet *ifp)
* copies of it.
*/
void
-rt_newaddrmsg(int cmd, struct ifaddr *ifa, int error, struct rtentry *rt)
+rt_sendaddrmsg(struct rtentry *rt, int cmd)
{
- struct rt_addrinfo info;
- struct sockaddr *sa = NULL;
- int pass;
- struct mbuf *m = NULL;
+ struct ifaddr *ifa = rt->rt_ifa;
struct ifnet *ifp = ifa->ifa_ifp;
+ struct mbuf *m = NULL;
+ struct rt_addrinfo info;
+ struct ifa_msghdr *ifam;
if (route_cb.any_count == 0)
return;
- for (pass = 1; pass < 3; pass++) {
- bzero(&info, sizeof(info));
- if ((cmd == RTM_ADD && pass == 1) ||
- (cmd == RTM_DELETE && pass == 2)) {
- struct ifa_msghdr *ifam;
- int ncmd;
- if (cmd == RTM_ADD)
- ncmd = RTM_NEWADDR;
- else
- ncmd = RTM_DELADDR;
+ memset(&info, 0, sizeof(info));
+ info.rti_info[RTAX_IFA] = ifa->ifa_addr;
+ info.rti_info[RTAX_IFP] = (struct sockaddr *)ifp->if_sadl;
+ info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
+ info.rti_info[RTAX_BRD] = ifa->ifa_dstaddr;
+ if ((m = rt_msg1(cmd, &info)) == NULL)
+ return;
+ 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;
+ ifam->ifam_tableid = ifp->if_rdomain;
- info.rti_info[RTAX_IFA] = sa = ifa->ifa_addr;
- info.rti_info[RTAX_IFP] =
- (struct sockaddr *)ifp->if_sadl;
- 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;
- ifam->ifam_tableid = ifp->if_rdomain;
- }
- if ((cmd == RTM_ADD && pass == 2) ||
- (cmd == RTM_DELETE && pass == 1)) {
- struct rt_msghdr *rtm;
- struct sockaddr_rtlabel sa_rl;
-
- if (rt == 0)
- 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;
- info.rti_info[RTAX_LABEL] =
- rtlabel_id2sa(rt->rt_labelid, &sa_rl);
- 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_priority = rt->rt_priority & RTP_MASK;
- rtm->rtm_errno = error;
- rtm->rtm_addrs = info.rti_addrs;
- rtm->rtm_tableid = ifp->if_rdomain;
- }
- if (sa == NULL)
- route_proto.sp_protocol = 0;
- else
- route_proto.sp_protocol = sa->sa_family;
- route_input(m, &route_proto, &route_src, &route_dst);
- }
+ if (ifa->ifa_addr == NULL)
+ route_proto.sp_protocol = 0;
+ else
+ route_proto.sp_protocol = ifa->ifa_addr->sa_family;
+ route_input(m, &route_proto, &route_src, &route_dst);
}
/*
