Timo,
On 17 Feb 2016, at 23:01, Timo Teras wrote:
On Wed, 17 Feb 2016 20:11:07 -0800
"Martin Winter" <[email protected]> wrote:
This is based on Quagga proposed 6 branch of Feb 10 (git f48d5b9957 -
which does no longer exist, [no] thanks to rebase on a public
git) on FreeBSD 10.2.
Zebra seems to fail delete any routes in FreeBSD RIB. It fails with
updates (better routes to different interface) and
it fails to remove them when quagga is killed.
Thanks for the testing. Sounds like this is breakage caused by my
atomic FIB patch which was pretty untested on *BSD.
Looks like the code talking to kernel does not handle RTM_CHANGE
correctly. As first test, perhaps just removing RTM_CHANGE usage might
fix the issues and revert to how it used to work.
Using RTM_CHANGE on kernels where it works is better, but it's left an
exercise for developer who has access and will to fix it on *BSD.
Thanks for the patch. Seems like you never tested it (failed to
compile),
but was close enough to guess what you probably meant. See inline on
patch
diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c
index 4d0a7db..9012280 100644
--- a/zebra/rt_socket.c
+++ b/zebra/rt_socket.c
@@ -68,7 +68,7 @@ sin_masklen (struct in_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib, int
family)
+kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in *mask = NULL;
@@ -245,7 +245,7 @@ sin6_masklen (struct in6_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib, int
family)
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in6 *mask;
struct sockaddr_in6 sin_dest, sin_mask, sin_gate;
@@ -363,33 +363,32 @@ kernel_rtm_ipv6 (int cmd, struct prefix *p,
struct rib *rib, int family)
#endif
+static int
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
I assume this should be
kernel_rtm - not kernel_rtm_ipv6
(otherwise you have a duplicate for kernel_rtm_ipv6() and a loop
in case of AF_INET6)
+{
+ switch (PREFIX_FAMILY(p))
+ {
+ case AF_INET:
+ return kernel_rtm_ipv4 (cmd, p, rib);
+ case AF_INET6:
+ return kernel_rtm_ipv6 (cmd, p, rib);
+ }
+ return 0;
+}
+
int
kernel_route_rib (struct prefix *p, struct rib *old, struct rib *new)
{
- struct rib *rib;
- int route = 0, cmd;
-
- if (!old && new)
- cmd = RTM_ADD;
- else if (old && !new)
- cmd = RTM_DELETE;
- else
- cmd = RTM_CHANGE;
-
- rib = new ? new : old;
+ int route = 0;
if (zserv_privs.change(ZPRIVS_RAISE))
zlog (NULL, LOG_ERR, "Can't raise privileges");
- switch (PREFIX_FAMILY(p))
- {
- case AF_INET:
- route = kernel_rtm_ipv4 (cmd, p, rib, AF_INET);
- break;
- case AF_INET6:
- route = kernel_rtm_ipv6 (cmd, p, rib, AF_INET6);
- break;
- }
+ if (old)
+ route |= kernel_rtm (RTM_DELETE, p, rib);
Changed to
route |= kernel_rtm (RTM_DELETE, p, old);
+
+ if (new)
+ route |= kernel_rtm (RTM_ADD, p, rib);
and changed to
route |= kernel_rtm (RTM_ADD, p, new);
(You removed the declaration of “rib” above - so rib is undefined)
if (zserv_privs.change(ZPRIVS_LOWER))
zlog (NULL, LOG_ERR, "Can't lower privileges");
Attached is a updated patch
With the changes it fixes the specific issue I’ve mentioned. I have
not verified the
patch on Linux yet. Will do a full run with this patch to see how many
of my approx
20..30 failing FreeBSD testcases it fixes (I assume many to all…)
- Martin
diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c
index 4d0a7db..9012280 100644
--- a/zebra/rt_socket.c
+++ b/zebra/rt_socket.c
@@ -68,7 +68,7 @@ sin_masklen (struct in_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib, int family)
+kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in *mask = NULL;
@@ -245,7 +245,7 @@ sin6_masklen (struct in6_addr mask)
/* Interface between zebra message and rtm message. */
static int
-kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib, int family)
+kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
{
struct sockaddr_in6 *mask;
struct sockaddr_in6 sin_dest, sin_mask, sin_gate;
@@ -363,33 +363,32 @@ kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib
*rib, int family)
#endif
+static int
+kernel_rtm (int cmd, struct prefix *p, struct rib *rib)
+{
+ switch (PREFIX_FAMILY(p))
+ {
+ case AF_INET:
+ return kernel_rtm_ipv4 (cmd, p, rib);
+ case AF_INET6:
+ return kernel_rtm_ipv6 (cmd, p, rib);
+ }
+ return 0;
+}
+
int
kernel_route_rib (struct prefix *p, struct rib *old, struct rib *new)
{
- struct rib *rib;
- int route = 0, cmd;
-
- if (!old && new)
- cmd = RTM_ADD;
- else if (old && !new)
- cmd = RTM_DELETE;
- else
- cmd = RTM_CHANGE;
-
- rib = new ? new : old;
+ int route = 0;
if (zserv_privs.change(ZPRIVS_RAISE))
zlog (NULL, LOG_ERR, "Can't raise privileges");
- switch (PREFIX_FAMILY(p))
- {
- case AF_INET:
- route = kernel_rtm_ipv4 (cmd, p, rib, AF_INET);
- break;
- case AF_INET6:
- route = kernel_rtm_ipv6 (cmd, p, rib, AF_INET6);
- break;
- }
+ if (old)
+ route |= kernel_rtm (RTM_DELETE, p, old);
+
+ if (new)
+ route |= kernel_rtm (RTM_ADD, p, new);
if (zserv_privs.change(ZPRIVS_LOWER))
zlog (NULL, LOG_ERR, "Can't lower privileges");
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
