On Mon, Aug 14 2017, Rob Pierce <r...@2keys.ca> wrote:
> ifstated currently tracks and maintains the index of each monitored interface
> and does not maintain interface names. This means we need to re-index on
> interface departure and arrival.
>
> The following diff moves away from indexes to names. Indexes are still
> required,
> but easily obtained dynamically as needed. This helps simplify the next diff
> that will provide support for interface departure and arrival.
>
> Suggested by deraadt.
>
> No intended functional change. Regress tests pass.
>
> Ok?
The idea looks sound to me, however I would keep the "interface" symbol
in parse.y (your diff doesn't remove all "interface" references btw).
The current code checks the existence of the interface at startup. If
the interface doesn't exists, you get a syntax error. This could happen
because of a missing interface (an interesting case), or because of
a typo. Whether or not we're erroring out, it is nice to print
a diagnostic message.
I'm not sure this change was intended, so here's a tentative diff that
that keeps the existing behavior. Regress tests pass.
Index: ifstated.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.59
diff -u -p -r1.59 ifstated.c
--- ifstated.c 14 Aug 2017 03:15:28 -0000 1.59
+++ ifstated.c 15 Aug 2017 03:04:47 -0000
@@ -61,8 +61,8 @@ void external_handler(int, short, void
void external_exec(struct ifsd_external *, int);
void check_external_status(struct ifsd_state *);
void external_evtimer_setup(struct ifsd_state *, int);
-void scan_ifstate(int, int, int);
-int scan_ifstate_single(int, int, struct ifsd_state *);
+void scan_ifstate(const char *, int, int);
+int scan_ifstate_single(const char *, int, struct ifsd_state *);
void fetch_ifstate(int);
__dead void usage(void);
void adjust_expressions(struct ifsd_expression_list *, int);
@@ -233,6 +233,8 @@ rt_msg_handler(int fd, short event, void
char msg[2048];
struct rt_msghdr *rtm = (struct rt_msghdr *)&msg;
struct if_msghdr ifm;
+ char ifnamebuf[IFNAMSIZ];
+ char *ifname;
ssize_t len;
if ((len = read(fd, msg, sizeof(msg))) == -1) {
@@ -250,7 +252,10 @@ rt_msg_handler(int fd, short event, void
switch (rtm->rtm_type) {
case RTM_IFINFO:
memcpy(&ifm, rtm, sizeof(ifm));
- scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1);
+ ifname = if_indextoname(ifm.ifm_index, ifnamebuf);
+ /* ifname is NULL on interface departure */
+ if (ifname != NULL)
+ scan_ifstate(ifname, ifm.ifm_data.ifi_link_state, 1);
break;
case RTM_DESYNC:
fetch_ifstate(1);
@@ -431,7 +436,7 @@ external_evtimer_setup(struct ifsd_state
#define LINK_STATE_IS_DOWN(_s) (!LINK_STATE_IS_UP((_s)))
int
-scan_ifstate_single(int ifindex, int s, struct ifsd_state *state)
+scan_ifstate_single(const char *ifname, int s, struct ifsd_state *state)
{
struct ifsd_ifstate *ifstate;
struct ifsd_expression_list expressions;
@@ -440,7 +445,7 @@ scan_ifstate_single(int ifindex, int s,
TAILQ_INIT(&expressions);
TAILQ_FOREACH(ifstate, &state->interface_states, entries) {
- if (ifstate->ifindex == ifindex) {
+ if (strcmp(ifstate->ifname, ifname) == 0) {
if (ifstate->prevstate != s &&
(ifstate->prevstate != -1 || !opt_inhibit)) {
struct ifsd_expression *expression;
@@ -472,15 +477,15 @@ scan_ifstate_single(int ifindex, int s,
}
void
-scan_ifstate(int ifindex, int s, int do_eval)
+scan_ifstate(const char *ifname, int s, int do_eval)
{
struct ifsd_state *state;
int cur_eval = 0;
- if (scan_ifstate_single(ifindex, s, &conf->initstate) && do_eval)
+ if (scan_ifstate_single(ifname, s, &conf->initstate) && do_eval)
eval_state(&conf->initstate);
TAILQ_FOREACH(state, &conf->states, entries) {
- if (scan_ifstate_single(ifindex, s, state) &&
+ if (scan_ifstate_single(ifname, s, state) &&
(do_eval && state == conf->curstate))
cur_eval = 1;
}
@@ -619,8 +624,8 @@ fetch_ifstate(int do_eval)
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
if (ifa->ifa_addr->sa_family == AF_LINK) {
struct if_data *ifdata = ifa->ifa_data;
- scan_ifstate(if_nametoindex(ifa->ifa_name),
- ifdata->ifi_link_state, do_eval);
+ scan_ifstate(ifa->ifa_name, ifdata->ifi_link_state,
+ do_eval);
}
}
Index: ifstated.h
===================================================================
RCS file: /d/cvs/src/usr.sbin/ifstated/ifstated.h,v
retrieving revision 1.18
diff -u -p -r1.18 ifstated.h
--- ifstated.h 14 Aug 2017 03:15:28 -0000 1.18
+++ ifstated.h 15 Aug 2017 03:04:47 -0000
@@ -41,7 +41,7 @@ struct ifsd_ifstate {
#define IFSD_LINKUP 2
int prevstate;
u_int32_t refcount;
- u_short ifindex;
+ char ifname[IFNAMSIZ];
};
struct ifsd_external {
Index: parse.y
===================================================================
RCS file: /d/cvs/src/usr.sbin/ifstated/parse.y,v
retrieving revision 1.45
diff -u -p -r1.45 parse.y
--- parse.y 23 Jul 2017 13:53:54 -0000 1.45
+++ parse.y 15 Aug 2017 03:23:00 -0000
@@ -85,7 +85,7 @@ struct ifsd_state *curstate;
void link_states(struct ifsd_action *);
void set_expression_depth(struct ifsd_expression *, int);
void init_state(struct ifsd_state *);
-struct ifsd_ifstate *new_ifstate(u_short, int);
+struct ifsd_ifstate *new_ifstate(char *, int);
struct ifsd_external *new_external(char *, u_int32_t);
typedef struct {
@@ -93,7 +93,6 @@ typedef struct {
int64_t number;
char *string;
struct in_addr addr;
- u_short interface;
struct ifsd_expression *expression;
struct ifsd_ifstate *ifstate;
@@ -114,7 +113,7 @@ typedef struct {
%token <v.string> STRING
%token <v.number> NUMBER
%type <v.string> string
-%type <v.interface> interface
+%type <v.string> interface
%type <v.ifstate> if_test
%type <v.external> ext_test
%type <v.expression> expr term
@@ -170,12 +169,12 @@ conf_main : INITSTATE STRING {
;
interface : STRING {
- if (($$ = if_nametoindex($1)) == 0) {
+ if (if_nametoindex($1) == 0) {
yyerror("unknown interface %s", $1);
free($1);
YYERROR;
}
- free($1);
+ $$ = $1;
}
;
@@ -933,7 +932,7 @@ init_state(struct ifsd_state *state)
}
struct ifsd_ifstate *
-new_ifstate(u_short ifindex, int s)
+new_ifstate(char *ifname, int s)
{
struct ifsd_ifstate *ifstate = NULL;
struct ifsd_state *state;
@@ -944,12 +943,16 @@ new_ifstate(u_short ifindex, int s)
state = &conf->initstate;
TAILQ_FOREACH(ifstate, &state->interface_states, entries)
- if (ifstate->ifindex == ifindex && ifstate->ifstate == s)
+ if (strcmp(ifstate->ifname, ifname) == 0 &&
+ ifstate->ifstate == s)
break;
if (ifstate == NULL) {
if ((ifstate = calloc(1, sizeof(*ifstate))) == NULL)
err(1, NULL);
- ifstate->ifindex = ifindex;
+ if (strlcpy(ifstate->ifname, ifname,
+ sizeof(ifstate->ifname)) == 0)
+ errx(1, "ifname strlcpy truncation");
+ free(ifname);
ifstate->ifstate = s;
TAILQ_INIT(&ifstate->expressions);
TAILQ_INSERT_TAIL(&state->interface_states, ifstate, entries);
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
