[Babel-users] the routing atomic update wet paint - because *I* care

[Matthieu Boutier]

> Is there anywhere a good reference to netlink?

iproute2, libnl, kernel sources ?


>> If I understand it correctly you just need to set the routes with
>> NLM_F_CREATE | NLM_F_REPLACE to get the atomic replacement.

> -DEFINES = $(PLATFORM_DEFINES) -DVERSION=\"$(VERSION)\"
> +DEFINES = $(PLATFORM_DEFINES) -DVERSION=\"$(VERSION)\" -DIPV6_SUBTREES

IPV6_SUBTREES macro is out-of-date now: we use dynamic configuration.

> -    if(operation == ROUTE_ADD) {
> -        buf.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL;
> +    switch(operation) {
> +    case ROUTE_ADD:
> +        buf.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_REPLACE;

Why did you remove EXCL? and why using REPLACE here?

> +    case ROUTE_MODIFY:
> +        buf.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_REPLACE;
> +        buf.nh.nlmsg_type = RTM_NEWROUTE;

Seems reasonable (but I would keep EXCL).

> +    if(operation == ROUTE_FLUSH) {
> +     rtm->rtm_scope = RT_SCOPE_NOWHERE;
> +    }

This should be a separate patch: why this change?

> +    if(operation == ROUTE_FLUSH) {
> +        *(int*)RTA_DATA(rta) = 0;
> +    } else {
>         *(int*)RTA_DATA(rta) = ifindex;
> -
> + }

Same, why this change?

The main flaw is that you never use the "new*" arguments: no chances to work.

You can try this untested patch (and note I have no clue how REPLACE should work):

$ git diff kernel_netlink.c
diff --git a/kernel_netlink.c b/kernel_netlink.c
index eb2e801..f115003 100644
--- a/kernel_netlink.c
+++ b/kernel_netlink.c
@@ -929,6 +929,9 @@ kernel_route(int operation, const unsigned char *dest, unsigned short plen,
     struct rtattr *rta;
     int len = sizeof(buf.raw);
     int rc, ipv4, table, use_src = 0;
+    const int has_atomic_replacement = has_ipv6_subtrees; /* Dave says that if a
+        kernel is new enough to do IPV6_SUBTREES, then it can do atomic
+        updates */
 
     if(!nl_setup) {
         fprintf(stderr,"kernel_route: netlink not initialized.\n");
@@ -948,6 +951,12 @@ kernel_route(int operation, const unsigned char *dest, unsigned short plen,
         }
     }
 
+    if(has_atomic_replacement && operation == ROUTE_MODIFY) {
+        gate = newgate;
+        ifindex = newifindex;
+        metric = newmetric;
+    }
+
     /* Check that the protocol family is consistent. */
     if(plen >= 96 && v4mapped(dest)) {
         if(!v4mapped(gate) ||
@@ -962,6 +971,7 @@ kernel_route(int operation, const unsigned char *dest, unsigned short plen,
         }
     }
 
+    if(has_atomic_replacement) {
     if(operation == ROUTE_MODIFY) {
         if(newmetric == metric && memcmp(newgate, gate, 16) == 0 &&
            newifindex == ifindex)
@@ -988,6 +998,7 @@ kernel_route(int operation, const unsigned char *dest, unsigned short plen,
         }
         return rc;
     }
+    }
 
 
     ipv4 = v4mapped(gate);
@@ -1019,9 +1030,16 @@ kernel_route(int operation, const unsigned char *dest, unsigned short plen,
     if(operation == ROUTE_ADD) {
         buf.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL;
         buf.nh.nlmsg_type = RTM_NEWROUTE;
-    } else {
+    } else if(operation == ROUTE_FLUSH) {
         buf.nh.nlmsg_flags = NLM_F_REQUEST;
         buf.nh.nlmsg_type = RTM_DELROUTE;
+    } else if(operation == ROUTE_MODIFY) {
+        buf.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL
+            | NLM_F_REPLACE;
+        buf.nh.nlmsg_type = RTM_NEWROUTE;
+    } else {
+        errno = EINVAL;
+        return -1;
     }
 
     rtm = NLMSG_DATA(&buf.nh);

Matthieu