<div dir="ltr">As an aside.  Is there a particular reason we have 5 different &quot;set metric&quot; command handlers in the code base?<div><br></div><div><div>sharpd@monster-01:~/quagga-2$ find . -name &#39;*.c&#39; -print | xargs grep &quot;DEFUN (set_metric&quot;</div><div>./bgpd/bgp_routemap.c:DEFUN (set_metric,</div><div>./ospf6d/ospf6_asbr.c:DEFUN (set_metric,</div><div>./ospfd/ospf_routemap.c:DEFUN (set_metric,</div><div>./ripd/rip_routemap.c:DEFUN (set_metric,</div><div>./ripngd/ripng_routemap.c:DEFUN (set_metric,</div></div><div><br></div><div>This causes issues with what is actually called due to the way the cli is created in vtysh/<a href="http://extract.pl">extract.pl</a> since it doesn&#39;t have anything to differentiate what actual function to call.  I added debugs to my version and noticed that the ospfd version is the one that is called(instead of all of them).  While it&#39;s ok or the normal &#39;set metric&#39; command, bgp depends on the set metric +/- command and it does not get parsed. </div><div><br></div><div>thanks!</div><div><br></div><div>donald</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 29, 2015 at 2:43 AM, Timo Teräs <span dir="ltr">&lt;<a href="mailto:timo.teras@iki.fi" target="_blank">timo.teras@iki.fi</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Use common code to parse, validate and adjust the route-map<br>
objects that contain a simple integer value. This also allows<br>
compiling the add/sub format metric object.<br>
<br>
Signed-off-by: Timo Teräs &lt;<a href="mailto:timo.teras@iki.fi">timo.teras@iki.fi</a>&gt;<br>
---<br>
 bgpd/bgp_routemap.c | 303 +++++++++++++++++++---------------------------------<br>
 1 file changed, 111 insertions(+), 192 deletions(-)<br>
<br>
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c<br>
index 416a3e5..4a0dac7 100644<br>
--- a/bgpd/bgp_routemap.c<br>
+++ b/bgpd/bgp_routemap.c<br>
@@ -101,6 +101,86 @@ o Local extensions<br>
<br>
 */<br>
<br>
+ /* generic value manipulation to be shared in multiple rules */<br>
+<br>
+#define RMAP_VALUE_SET 0<br>
+#define RMAP_VALUE_ADD 1<br>
+#define RMAP_VALUE_SUB 2<br>
+<br>
+struct rmap_value<br>
+{<br>
+  u_int8_t action;<br>
+  u_int32_t value;<br>
+};<br>
+<br>
+static int<br>
+route_value_match (struct rmap_value *rv, u_int32_t value)<br>
+{<br>
+  if (value == rv-&gt;value)<br>
+    return RMAP_MATCH;<br>
+<br>
+  return RMAP_NOMATCH;<br>
+}<br>
+<br>
+static u_int32_t<br>
+route_value_adjust (struct rmap_value *rv, u_int32_t current)<br>
+{<br>
+  u_int32_t value = rv-&gt;value;<br>
+<br>
+  switch (rv-&gt;action)<br>
+    {<br>
+    case RMAP_VALUE_ADD:<br>
+      if (current &gt; UINT32_MAX-value)<br>
+        return UINT32_MAX;<br>
+      return current + value;<br>
+    case RMAP_VALUE_SUB:<br>
+      if (current &lt;= value)<br>
+        return 0;<br>
+      return current - value;<br>
+    default:<br>
+      return value;<br>
+    }<br>
+}<br>
+<br>
+static void *<br>
+route_value_compile (const char *arg)<br>
+{<br>
+  u_int8_t action = RMAP_VALUE_SET;<br>
+  unsigned long larg;<br>
+  char *endptr = NULL;<br>
+  struct rmap_value *rv;<br>
+<br>
+  if (arg[0] == &#39;+&#39;)<br>
+    {<br>
+      action = RMAP_VALUE_ADD;<br>
+      arg++;<br>
+    }<br>
+  else if (arg[0] == &#39;-&#39;)<br>
+    {<br>
+      action = RMAP_VALUE_SUB;<br>
+      arg++;<br>
+    }<br>
+<br>
+  errno = 0;<br>
+  larg = strtoul (arg, &amp;endptr, 10);<br>
+  if (*arg == 0 || *endptr != 0 || errno || larg &gt; UINT32_MAX)<br>
+    return NULL;<br>
+<br>
+  rv = XMALLOC (MTYPE_ROUTE_MAP_COMPILED, sizeof(struct rmap_value));<br>
+  if (!rv)<br>
+    return NULL;<br>
+<br>
+  rv-&gt;action = action;<br>
+  rv-&gt;value = larg;<br>
+  return rv;<br>
+}<br>
+<br>
+static void<br>
+route_value_free (void *rule)<br>
+{<br>
+  XFREE (MTYPE_ROUTE_MAP_COMPILED, rule);<br>
+}<br>
+<br>
  /* generic as path object to be shared in multiple rules */<br>
<br>
 static void *<br>
@@ -518,62 +598,25 @@ static route_map_result_t<br>
 route_match_metric (void *rule, struct prefix *prefix,<br>
                    route_map_object_t type, void *object)<br>
 {<br>
-  u_int32_t *med;<br>
+  struct rmap_value *rv;<br>
   struct bgp_info *bgp_info;<br>
<br>
   if (type == RMAP_BGP)<br>
     {<br>
-      med = rule;<br>
+      rv = rule;<br>
       bgp_info = object;<br>
-<br>
-      if (bgp_info-&gt;attr-&gt;med == *med)<br>
-       return RMAP_MATCH;<br>
-      else<br>
-       return RMAP_NOMATCH;<br>
+      return route_value_match(rv, bgp_info-&gt;attr-&gt;med);<br>
     }<br>
   return RMAP_NOMATCH;<br>
 }<br>
<br>
-/* Route map `match metric&#39; match statement. `arg&#39; is MED value */<br>
-static void *<br>
-route_match_metric_compile (const char *arg)<br>
-{<br>
-  u_int32_t *med;<br>
-  char *endptr = NULL;<br>
-  unsigned long tmpval;<br>
-<br>
-  /* Metric value shoud be integer. */<br>
-  if (! all_digit (arg))<br>
-    return NULL;<br>
-<br>
-  errno = 0;<br>
-  tmpval = strtoul (arg, &amp;endptr, 10);<br>
-  if (*endptr != &#39;\0&#39; || errno || tmpval &gt; UINT32_MAX)<br>
-    return NULL;<br>
-<br>
-  med = XMALLOC (MTYPE_ROUTE_MAP_COMPILED, sizeof (u_int32_t));<br>
-<br>
-  if (!med)<br>
-    return med;<br>
-<br>
-  *med = tmpval;<br>
-  return med;<br>
-}<br>
-<br>
-/* Free route map&#39;s compiled `match metric&#39; value. */<br>
-static void<br>
-route_match_metric_free (void *rule)<br>
-{<br>
-  XFREE (MTYPE_ROUTE_MAP_COMPILED, rule);<br>
-}<br>
-<br>
 /* Route map commands for metric matching. */<br>
 struct route_map_rule_cmd route_match_metric_cmd =<br>
 {<br>
   &quot;metric&quot;,<br>
   route_match_metric,<br>
-  route_match_metric_compile,<br>
-  route_match_metric_free<br>
+  route_value_compile,<br>
+  route_value_free,<br>
 };<br>
<br>
 /* `match as-path ASPATH&#39; */<br>
@@ -992,64 +1035,34 @@ static route_map_result_t<br>
 route_set_local_pref (void *rule, struct prefix *prefix,<br>
                      route_map_object_t type, void *object)<br>
 {<br>
-  u_int32_t *local_pref;<br>
+  struct rmap_value *rv;<br>
   struct bgp_info *bgp_info;<br>
+  u_int32_t locpref = 0;<br>
<br>
   if (type == RMAP_BGP)<br>
     {<br>
       /* Fetch routemap&#39;s rule information. */<br>
-      local_pref = rule;<br>
+      rv = rule;<br>
       bgp_info = object;<br>
<br>
       /* Set local preference value. */<br>
+      if (bgp_info-&gt;attr-&gt;flag &amp; ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF))<br>
+       locpref = bgp_info-&gt;attr-&gt;local_pref;<br>
+<br>
       bgp_info-&gt;attr-&gt;flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF);<br>
-      bgp_info-&gt;attr-&gt;local_pref = *local_pref;<br>
+      bgp_info-&gt;attr-&gt;local_pref = route_value_adjust(rv, locpref);<br>
     }<br>
<br>
   return RMAP_OKAY;<br>
 }<br>
<br>
-/* set local preference compilation. */<br>
-static void *<br>
-route_set_local_pref_compile (const char *arg)<br>
-{<br>
-  unsigned long tmp;<br>
-  u_int32_t *local_pref;<br>
-  char *endptr = NULL;<br>
-<br>
-  /* Local preference value shoud be integer. */<br>
-  if (! all_digit (arg))<br>
-    return NULL;<br>
-<br>
-  errno = 0;<br>
-  tmp = strtoul (arg, &amp;endptr, 10);<br>
-  if (*endptr != &#39;\0&#39; || errno || tmp &gt; UINT32_MAX)<br>
-    return NULL;<br>
-<br>
-  local_pref = XMALLOC (MTYPE_ROUTE_MAP_COMPILED, sizeof (u_int32_t));<br>
-<br>
-  if (!local_pref)<br>
-    return local_pref;<br>
-<br>
-  *local_pref = tmp;<br>
-<br>
-  return local_pref;<br>
-}<br>
-<br>
-/* Free route map&#39;s local preference value. */<br>
-static void<br>
-route_set_local_pref_free (void *rule)<br>
-{<br>
-  XFREE (MTYPE_ROUTE_MAP_COMPILED, rule);<br>
-}<br>
-<br>
 /* Set local preference rule structure. */<br>
 struct route_map_rule_cmd route_set_local_pref_cmd =<br>
 {<br>
   &quot;local-preference&quot;,<br>
   route_set_local_pref,<br>
-  route_set_local_pref_compile,<br>
-  route_set_local_pref_free,<br>
+  route_value_compile,<br>
+  route_value_free,<br>
 };<br>
<br>
 /* `set weight WEIGHT&#39; */<br>
@@ -1059,18 +1072,20 @@ static route_map_result_t<br>
 route_set_weight (void *rule, struct prefix *prefix, route_map_object_t type,<br>
                  void *object)<br>
 {<br>
-  u_int32_t *weight;<br>
+  struct rmap_value *rv;<br>
   struct bgp_info *bgp_info;<br>
+  u_int32_t weight;<br>
<br>
   if (type == RMAP_BGP)<br>
     {<br>
       /* Fetch routemap&#39;s rule information. */<br>
-      weight = rule;<br>
+      rv = rule;<br>
       bgp_info = object;<br>
<br>
       /* Set weight value. */<br>
-      if (*weight)<br>
-        (bgp_attr_extra_get (bgp_info-&gt;attr))-&gt;weight = *weight;<br>
+      weight = route_value_adjust(rv, 0);<br>
+      if (weight)<br>
+        (bgp_attr_extra_get (bgp_info-&gt;attr))-&gt;weight = weight;<br>
       else if (bgp_info-&gt;attr-&gt;extra)<br>
         bgp_info-&gt;attr-&gt;extra-&gt;weight = 0;<br>
     }<br>
@@ -1078,47 +1093,13 @@ route_set_weight (void *rule, struct prefix *prefix, route_map_object_t type,<br>
   return RMAP_OKAY;<br>
 }<br>
<br>
-/* set local preference compilation. */<br>
-static void *<br>
-route_set_weight_compile (const char *arg)<br>
-{<br>
-  unsigned long tmp;<br>
-  u_int32_t *weight;<br>
-  char *endptr = NULL;<br>
-<br>
-  /* Local preference value shoud be integer. */<br>
-  if (! all_digit (arg))<br>
-    return NULL;<br>
-<br>
-  errno = 0;<br>
-  tmp = strtoul (arg, &amp;endptr, 10);<br>
-  if (*endptr != &#39;\0&#39; || errno || tmp &gt; UINT32_MAX)<br>
-    return NULL;<br>
-<br>
-  weight = XMALLOC (MTYPE_ROUTE_MAP_COMPILED, sizeof (u_int32_t));<br>
-<br>
-  if (weight == NULL)<br>
-    return weight;<br>
-<br>
-  *weight = tmp;<br>
-<br>
-  return weight;<br>
-}<br>
-<br>
-/* Free route map&#39;s local preference value. */<br>
-static void<br>
-route_set_weight_free (void *rule)<br>
-{<br>
-  XFREE (MTYPE_ROUTE_MAP_COMPILED, rule);<br>
-}<br>
-<br>
 /* Set local preference rule structure. */<br>
 struct route_map_rule_cmd route_set_weight_cmd =<br>
 {<br>
   &quot;weight&quot;,<br>
   route_set_weight,<br>
-  route_set_weight_compile,<br>
-  route_set_weight_free,<br>
+  route_value_compile,<br>
+  route_value_free,<br>
 };<br>
<br>
 /* `set metric METRIC&#39; */<br>
@@ -1128,94 +1109,32 @@ static route_map_result_t<br>
 route_set_metric (void *rule, struct prefix *prefix,<br>
                  route_map_object_t type, void *object)<br>
 {<br>
-  char *metric;<br>
-  u_int32_t metric_val;<br>
+  struct rmap_value *rv;<br>
   struct bgp_info *bgp_info;<br>
+  u_int32_t med = 0;<br>
<br>
   if (type == RMAP_BGP)<br>
     {<br>
       /* Fetch routemap&#39;s rule information. */<br>
-      metric = rule;<br>
+      rv = rule;<br>
       bgp_info = object;<br>
<br>
-      if (! (bgp_info-&gt;attr-&gt;flag &amp; ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC)))<br>
-       bgp_info-&gt;attr-&gt;med = 0;<br>
-      bgp_info-&gt;attr-&gt;flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC);<br>
+      if (bgp_info-&gt;attr-&gt;flag &amp; ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC))<br>
+       med = bgp_info-&gt;attr-&gt;med;<br>
<br>
-      if (all_digit (metric))<br>
-       {<br>
-         metric_val = strtoul (metric, (char **)NULL, 10);<br>
-         bgp_info-&gt;attr-&gt;med = metric_val;<br>
-       }<br>
-      else<br>
-       {<br>
-         metric_val = strtoul (metric+1, (char **)NULL, 10);<br>
-<br>
-         if (strncmp (metric, &quot;+&quot;, 1) == 0)<br>
-           {<br>
-             if (bgp_info-&gt;attr-&gt;med/2 + metric_val/2 &gt; BGP_MED_MAX/2)<br>
-               bgp_info-&gt;attr-&gt;med = BGP_MED_MAX - 1;<br>
-             else<br>
-               bgp_info-&gt;attr-&gt;med += metric_val;<br>
-           }<br>
-         else if (strncmp (metric, &quot;-&quot;, 1) == 0)<br>
-           {<br>
-             if (bgp_info-&gt;attr-&gt;med &lt;= metric_val)<br>
-               bgp_info-&gt;attr-&gt;med = 0;<br>
-             else<br>
-               bgp_info-&gt;attr-&gt;med -= metric_val;<br>
-           }<br>
-       }<br>
+      bgp_info-&gt;attr-&gt;med = route_value_adjust(rv, med);<br>
+      bgp_info-&gt;attr-&gt;flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC);<br>
     }<br>
   return RMAP_OKAY;<br>
 }<br>
<br>
-/* set metric compilation. */<br>
-static void *<br>
-route_set_metric_compile (const char *arg)<br>
-{<br>
-  unsigned long larg;<br>
-  char *endptr = NULL;<br>
-<br>
-  if (all_digit (arg))<br>
-    {<br>
-      /* set metric value check*/<br>
-      errno = 0;<br>
-      larg = strtoul (arg, &amp;endptr, 10);<br>
-      if (*endptr != &#39;\0&#39; || errno || larg &gt; UINT32_MAX)<br>
-        return NULL;<br>
-    }<br>
-  else<br>
-    {<br>
-      /* set metric +/-value check */<br>
-      if ((strncmp (arg, &quot;+&quot;, 1) != 0<br>
-          &amp;&amp; strncmp (arg, &quot;-&quot;, 1) != 0)<br>
-          || (! all_digit (arg+1)))<br>
-       return NULL;<br>
-<br>
-      errno = 0;<br>
-      larg = strtoul (arg+1, &amp;endptr, 10);<br>
-      if (*endptr != &#39;\0&#39; || errno || larg &gt; UINT32_MAX)<br>
-       return NULL;<br>
-    }<br>
-<br>
-  return XSTRDUP (MTYPE_ROUTE_MAP_COMPILED, arg);<br>
-}<br>
-<br>
-/* Free route map&#39;s compiled `set metric&#39; value. */<br>
-static void<br>
-route_set_metric_free (void *rule)<br>
-{<br>
-  XFREE (MTYPE_ROUTE_MAP_COMPILED, rule);<br>
-}<br>
-<br>
 /* Set metric rule structure. */<br>
 struct route_map_rule_cmd route_set_metric_cmd =<br>
 {<br>
   &quot;metric&quot;,<br>
   route_set_metric,<br>
-  route_set_metric_compile,<br>
-  route_set_metric_free,<br>
+  route_value_compile,<br>
+  route_value_free,<br>
 };<br>
<br>
 /* `set as-path prepend ASPATH&#39; */<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.3.6<br>
<br>
<br>
_______________________________________________<br>
Quagga-dev mailing list<br>
<a href="mailto:Quagga-dev@lists.quagga.net">Quagga-dev@lists.quagga.net</a><br>
<a href="https://lists.quagga.net/mailman/listinfo/quagga-dev" target="_blank">https://lists.quagga.net/mailman/listinfo/quagga-dev</a><br>
</font></span></blockquote></div><br></div>