[PATCH] babel: Log the reason when refusing to run on an interface
The babel protocol code checks various properties of an interface before starting it: whether the interface is up, whether it supports multicast, and whether it has a link-local address assigned. However, it doesn't give any feedback if any of those checks fail, it just silently ignores the interface. Fix this by explicitly logging errors when any of the checks fail; to avoid spamming the logs for all interfaces on the system, move the checks so they are only performed after it was determined that an interface is actually configured by an interface pattern. Reported-by: Stefan Haller <stefan.haller@stha.de> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- proto/babel/babel.c | 50 ++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/proto/babel/babel.c b/proto/babel/babel.c index 4b6b9d7f9f6f..6ac3bb6a7fec 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1639,6 +1639,29 @@ babel_remove_iface(struct babel_proto *p, struct babel_iface *ifa) rfree(ifa->pool); /* contains ifa itself, locks, socket, etc */ } +static int iface_is_valid(struct babel_proto *p, struct iface *iface) +{ + if (!(iface->flags & IF_UP)) { + log(L_ERR "%s: Interface '%s' is down", p->p.name, iface->name); + return 0; + } + + if (!(iface->flags & IF_MULTICAST)) { + log(L_ERR "%s: Interface '%s' does not support multicast", + p->p.name, iface->name); + return 0; + } + + /* We need a link-local address to communicate */ + if (!iface->llv6) { + log(L_ERR "%s: Interface '%s' has no link-local address assigned", + p->p.name, iface->name); + return 0; + } + + return 1; +} + static void babel_if_notify(struct proto *P, unsigned flags, struct iface *iface) { @@ -1655,19 +1678,8 @@ babel_if_notify(struct proto *P, unsigned flags, struct iface *iface) if (ifa) babel_remove_iface(p, ifa); - if (!(iface->flags & IF_UP)) - return; - - /* We only speak multicast */ - if (!(iface->flags & IF_MULTICAST)) - return; - - /* Ignore ifaces without link-local address */ - if (!iface->llv6) - return; - struct babel_iface_config *ic = (void *) iface_patt_find(&cf->iface_list, iface, NULL); - if (ic) + if (ic && iface_is_valid(p, iface)) babel_add_iface(p, iface, ic); return; @@ -1733,20 +1745,12 @@ babel_reconfigure_ifaces(struct babel_proto *p, struct babel_config *cf) WALK_LIST(iface, iface_list) { - if (!(iface->flags & IF_UP)) - continue; - - /* Ignore non-multicast ifaces */ - if (!(iface->flags & IF_MULTICAST)) - continue; - - /* Ignore ifaces without link-local address */ - if (!iface->llv6) - continue; - struct babel_iface *ifa = babel_find_iface(p, iface); struct babel_iface_config *ic = (void *) iface_patt_find(&cf->iface_list, iface, NULL); + if (ic && !iface_is_valid(p, iface)) + continue; + if (ifa && ic) { if (babel_reconfigure_iface(p, ifa, ic)) -- 2.31.1
On Tue, Apr 20, 2021 at 12:27:05AM +0200, Toke Høiland-Jørgensen wrote:
The babel protocol code checks various properties of an interface before starting it: whether the interface is up, whether it supports multicast, and whether it has a link-local address assigned. However, it doesn't give any feedback if any of those checks fail, it just silently ignores the interface.
Fix this by explicitly logging errors when any of the checks fail; to avoid spamming the logs for all interfaces on the system, move the checks so they are only performed after it was determined that an interface is actually configured by an interface pattern.
Hi Merged with some rework.
+static int iface_is_valid(struct babel_proto *p, struct iface *iface) +{ + if (!(iface->flags & IF_UP)) { + log(L_ERR "%s: Interface '%s' is down", p->p.name, iface->name); + return 0; + }
This should be silent. Interface down is valid administrative condition.
+ if (!(iface->flags & IF_MULTICAST)) { + log(L_ERR "%s: Interface '%s' does not support multicast", + p->p.name, iface->name); + return 0; + }
This is OK
+ /* We need a link-local address to communicate */ + if (!iface->llv6) { + log(L_ERR "%s: Interface '%s' has no link-local address assigned", + p->p.name, iface->name); + return 0; + }
This is complicated. At first it makes sense, but if i remember correctly there is a transient condition where immediately after iface goes up, OS does DAD on the address, during which it is unassigned, and after end of DAD it will appear. Therefore for each new iface, there would be this error message in log and then perfectly working iface. So it is probably better to keep it also silent. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (2)
-
Ondrej Zajicek -
Toke Høiland-Jørgensen