Good evening all,
Currently, ifstated does not detect the removal of an IFT_CARP pseudo device.
As such, you can delete a carp interface and have ifstated happily remain in
the current state without detecting any interface change.
The reasons are two fold:
1. The routing socket is only monitored for RTM_IFINFO messages and therefore
misses interface departure and arrival events. On an interface departure,
for example, there is only one RTM_IFINFO message, and that particular
message identifies the departed (carp) interface as up and master.
2. As I have recently discovered, LINK_STATE_UNKNOWN is equivalent to
LINK_STATE_UP, so even if a removal was detected, the state change would go
unnoticed (i.e. there would be no state change). I ran into this while
trying to address (1).
The following diff resolves this problem by adding RTM_IFANNOUNCE to the
routing socket filter, and the route message handler now handles interface
departures and arrivals. Upon departure, a rescan is performed with a (forced)
state of LINK_STATE_DOWN, and upon the arrival of a previously configured
pseudo device the interface is re-indexed and a rescan is performed with the
actual interface state.
I have also updated the regression test to cover carp interface departure and
arrival (diff included for testing convenience).
At this stage I am only looking for general comments on the approach and/or
some testing if you are an ifstated user.
For example, maybe it is unnecessary to save and check the interface type,
assuming that interface departure and arrival will always be on a (supported)
pseudo device (e.g. IFT_CARP), and actual physical interfaces will not behave
in this manner. Also, ifi_type is not available via an ioctl under pledge, so
obtaining this information dynamically through routing messages may be a bit
of a hack.
Thanks and regards,
Rob
Index: src/usr.sbin/ifstated/ifstated.c
===================================================================
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.56
diff -u -p -r1.56 ifstated.c
--- src/usr.sbin/ifstated/ifstated.c 24 Jul 2017 12:33:59 -0000 1.56
+++ src/usr.sbin/ifstated/ifstated.c 31 Jul 2017 20:48:42 -0000
@@ -28,6 +28,7 @@
#include <sys/wait.h>
#include <net/if.h>
+#include <net/if_types.h>
#include <net/route.h>
#include <netinet/in.h>
@@ -73,6 +74,8 @@ void do_action(struct ifsd_action *);
void remove_action(struct ifsd_action *, struct ifsd_state *);
void remove_expression(struct ifsd_expression *,
struct ifsd_state *);
+void set_iftype(int, u_char);
+void reindex_if(int, char *);
__dead void
usage(void)
@@ -149,7 +152,7 @@ main(int argc, char *argv[])
if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0)
err(1, "no routing socket");
- rtfilter = ROUTE_FILTER(RTM_IFINFO);
+ rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_IFANNOUNCE);
if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER,
&rtfilter, sizeof(rtfilter)) == -1) /* not fatal */
log_warn("%s: setsockopt msgfilter", __func__);
@@ -231,24 +234,79 @@ void
rt_msg_handler(int fd, short event, void *arg)
{
char msg[2048];
+ char ifname[IFNAMSIZ];
struct rt_msghdr *rtm = (struct rt_msghdr *)&msg;
struct if_msghdr ifm;
+ struct if_announcemsghdr ifan;
+ struct ifaddrs *ifap, *ifa;
ssize_t len;
len = read(fd, msg, sizeof(msg));
/* XXX ignore errors? */
- if (len < sizeof(struct rt_msghdr))
+ if ((size_t)len < sizeof(rtm->rtm_msglen) || len < rtm->rtm_msglen)
return;
if (rtm->rtm_version != RTM_VERSION)
return;
- if (rtm->rtm_type != RTM_IFINFO)
- return;
-
- memcpy(&ifm, rtm, sizeof(ifm));
- scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1);
+ switch (rtm->rtm_type) {
+ /*
+ * We can't access ifi_type via an ioctl under pledge, but we can get
+ * it for free in an RTM_IFINFO and update ifstate accordingly. This is
+ * (may be?) required for reindexing, and reindexing isn't necessary
+ * until we have received an RTM_IFINFO message.
+ */
+ case RTM_IFINFO:
+ memcpy(&ifm, rtm, sizeof(ifm));
+ set_iftype(ifm.ifm_index, ifm.ifm_data.ifi_type);
+ scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1);
+ break;
+ case RTM_IFANNOUNCE:
+ memcpy(&ifan, rtm, sizeof(ifan));
+ switch (ifan.ifan_what) {
+ case IFAN_ARRIVAL:
+ /*
+ * We only reindex IFT_CARP pseudo devices that have
+ * already been configured, and then rescan with the
+ * actual interface state.
+ */
+ log_warnx("interface #%d arrived", ifan.ifan_index);
+ if_indextoname(ifan.ifan_index, ifname);
+ reindex_if(ifan.ifan_index, ifname);
+ if (getifaddrs(&ifap) != 0)
+ err(1, "getifaddrs");
+ for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+ if (ifa->ifa_addr->sa_family == AF_LINK) {
+ struct if_data *ifdata = ifa->ifa_data;
+ if (strcmp(ifa->ifa_name, ifname) == 0)
+ {
+ scan_ifstate(ifan.ifan_index,
+ ifdata->ifi_link_state, 1);
+ break;
+ }
+ }
+ }
+ freeifaddrs(ifap);
+ break;
+ case IFAN_DEPARTURE:
+ /*
+ * Pseudo devices configured in ifstated that have
+ * departed need to be explicitly scanned as
+ * LINK_STATE_DOWN since LINK_STATE_UNKNOWN is
+ * equivalent to LINK_STATE_UP (which results in no
+ * detected interface stata change).
+ */
+ log_warnx("interface #%d departed", ifan.ifan_index);
+ scan_ifstate(ifan.ifan_index, LINK_STATE_DOWN, 1);
+ break;
+ default:
+ break;
+ }
+ default:
+ break;
+ }
+ return;
}
void
@@ -414,6 +472,42 @@ external_evtimer_setup(struct ifsd_state
evtimer_del(&external->ev);
}
break;
+ }
+ }
+}
+
+void
+set_iftype(int ifindex, u_char iftype)
+{
+ struct ifsd_state *state;
+ struct ifsd_ifstate *ifstate;
+
+ TAILQ_FOREACH(state, &conf->states, entries) {
+ TAILQ_FOREACH(ifstate, &state->interface_states, entries) {
+ if (ifstate->ifindex == ifindex)
+ if (ifstate->iftype == 0)
+ ifstate->iftype = iftype;
+ }
+ }
+}
+
+void
+reindex_if(int ifindex, char *ifname)
+{
+ struct ifsd_state *state;
+ struct ifsd_ifstate *ifstate;
+
+ TAILQ_FOREACH(state, &conf->states, entries) {
+ TAILQ_FOREACH(ifstate, &state->interface_states, entries) {
+ if (strcmp(ifstate->ifname, ifname) == 0) {
+ if (ifindex > ifstate->ifindex) {
+ if (ifstate->iftype == IFT_CARP)
+ log_warnx("%s: reindexing %s",
+ state->name,
+ ifstate->ifname);
+ ifstate->ifindex = ifindex;
+ }
+ }
}
}
}
Index: src/usr.sbin/ifstated/ifstated.h
===================================================================
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v
retrieving revision 1.17
diff -u -p -r1.17 ifstated.h
--- src/usr.sbin/ifstated/ifstated.h 23 Jul 2017 13:48:18 -0000 1.17
+++ src/usr.sbin/ifstated/ifstated.h 31 Jul 2017 20:48:42 -0000
@@ -42,6 +42,8 @@ struct ifsd_ifstate {
int prevstate;
u_int32_t refcount;
u_short ifindex;
+ char ifname[IFNAMSIZ];
+ u_char iftype;
};
struct ifsd_external {
Index: src/usr.sbin/ifstated/parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v
retrieving revision 1.45
diff -u -p -r1.45 parse.y
--- src/usr.sbin/ifstated/parse.y 23 Jul 2017 13:53:54 -0000 1.45
+++ src/usr.sbin/ifstated/parse.y 31 Jul 2017 20:48:43 -0000
@@ -950,6 +950,7 @@ new_ifstate(u_short ifindex, int s)
if ((ifstate = calloc(1, sizeof(*ifstate))) == NULL)
err(1, NULL);
ifstate->ifindex = ifindex;
+ if_indextoname(ifstate->ifindex, ifstate->ifname);
ifstate->ifstate = s;
TAILQ_INIT(&ifstate->expressions);
TAILQ_INSERT_TAIL(&state->interface_states, ifstate, entries);
Index: src/regress/usr.sbin/ifstated/ifstated
===================================================================
RCS file: /cvs/src/regress/usr.sbin/ifstated/ifstated,v
retrieving revision 1.3
diff -u -p -r1.3 ifstated
--- src/regress/usr.sbin/ifstated/ifstated 31 Jul 2017 18:41:21 -0000
1.3
+++ src/regress/usr.sbin/ifstated/ifstated 31 Jul 2017 20:48:43 -0000
@@ -123,12 +123,21 @@ changing state to demoted
changing state to primary
changing state to demoted
changing state to primary
+changing state to demoted
+changing state to primary
+changing state to primary
+changing state to demoted
changing state to primary
EOF
(cd working && nohup ifstated -dvf ./ifstated.conf > ifstated.log 2>&1) &
sleep ${SLEEP}
+ifconfig carp${VHIDA} destroy
+sleep ${SLEEP}
+ifconfig carp${VHIDA} inet ${PREFIX}.${VHIDA} netmask 255.255.255.0 broadcast \
+ ${PREFIX}.255 vhid ${VHIDA} carpdev ${NIC}
+sleep ${SLEEP}
ifconfig carp${VHIDA} down
sleep ${SLEEP}
ifconfig carp${VHIDA} up
@@ -142,12 +151,21 @@ ifconfig carp${VHIDA} down
sleep ${SLEEP}
ifconfig carp${VHIDB} destroy
sleep ${SLEEP}
+ifconfig carp${VHIDA} destroy
+sleep ${SLEEP}
+ifconfig carp${VHIDA} inet ${PREFIX}.${VHIDA} netmask 255.255.255.0 broadcast \
+ ${PREFIX}.255 vhid ${VHIDA} carpdev ${NIC}
ifconfig carp${VHIDA} up
sleep ${SLEEP}
ifconfig carp${VHIDB} inet ${PREFIX}.${VHIDB} netmask 255.255.255.0 broadcast \
${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC}
sleep ${SLEEP}
kill -HUP $(pgrep ifstated) >/dev/null 2>&1
+sleep ${SLEEP}
+ifconfig carp${VHIDA} destroy
+sleep ${SLEEP}
+ifconfig carp${VHIDA} inet ${PREFIX}.${VHIDA} netmask 255.255.255.0 broadcast \
+ ${PREFIX}.255 vhid ${VHIDA} carpdev ${NIC}
sleep ${SLEEP}
grep ^changing working/ifstated.log > working/output.new
