ofproto/bond: Improve admissibility debug readability.

The admissibility check currently log a message like (line wrapped in
this commitlog):
bond(revalidator11)|DBG|member (dpdk0): Admissibility
verdict is to drop pkt as different port is learned.active member: false,
may_enable: true enable: true LACP status:2

Fix spaces around the period character and separate debug infos with
commas.
Prefix all log messages in this check with bond and member names.
Display a human readable string for LACP status.

New logs look like:
bond(revalidator11)|DBG|bond dpdkbond0: member dpdk0: admissibility
verdict is to drop pkt as different port is learned, active member: false,
may_enable: true, enabled: true, LACP status: off

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

3 files changed, 28 insertions, 26 deletions

diff --git a/lib/lacp.c b/lib/lacp.c
index 540b2aa8c..89d711225 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -429,6 +429,20 @@ lacp_status(const struct lacp *lacp) OVS_EXCLUDED(mutex)
     }
 }
 
+const char *lacp_status_description(enum lacp_status lacp_status)
+{
+    switch (lacp_status) {
+    case LACP_NEGOTIATED:
+        return "negotiated";
+    case LACP_CONFIGURED:
+        return "configured";
+    case LACP_DISABLED:
+        return "off";
+    default:
+        return "<unknown>";
+    }
+}
+
 /* Registers 'member_' as subordinate to 'lacp'.  This should be called at
  * least once per member in a LACP managed bond.  Should also be called
  * whenever a member's settings change. */
diff --git a/lib/lacp.h b/lib/lacp.h
index 908ec201c..1ca06f762 100644
--- a/lib/lacp.h
+++ b/lib/lacp.h
@@ -49,6 +49,7 @@ bool lacp_is_active(const struct lacp *);
 bool lacp_process_packet(struct lacp *, const void *member,
                          const struct dp_packet *packet);
 enum lacp_status lacp_status(const struct lacp *);
+const char *lacp_status_description(enum lacp_status);
 
 struct lacp_member_settings {
     char *name;                       /* Name (for debugging). */
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 2dcfeda71..cdfdf0b9d 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -876,7 +876,7 @@ bond_check_admissibility(struct bond *bond, const void *member_,
         if (!member->enabled && member->may_enable) {
             VLOG_DBG_RL(&rl, "bond %s: member %s: "
                         "main thread has not yet enabled member",
-                         bond->name, bond->active_member->name);
+                        bond->name, bond->active_member->name);
         }
         goto out;
     case LACP_CONFIGURED:
@@ -913,9 +913,9 @@ bond_check_admissibility(struct bond *bond, const void *member_,
         /* Drop all packets which arrive on backup members.  This is similar to
          * how Linux bonding handles active-backup bonds. */
         if (bond->active_member != member) {
-            VLOG_DBG_RL(&rl, "active-backup bond received packet on backup"
-                        " member (%s) destined for " ETH_ADDR_FMT,
-                        member->name, ETH_ADDR_ARGS(eth_dst));
+            VLOG_DBG_RL(&rl, "bond %s: member %s: active-backup bond received "
+                        "packet on backup member destined for " ETH_ADDR_FMT,
+                        bond->name, member->name, ETH_ADDR_ARGS(eth_dst));
             goto out;
         }
         verdict = BV_ACCEPT;
@@ -935,17 +935,17 @@ bond_check_admissibility(struct bond *bond, const void *member_,
     OVS_NOT_REACHED();
 out:
     if (member && (verdict != BV_ACCEPT)) {
-        VLOG_DBG_RL(&rl, "member (%s): "
-                    "Admissibility verdict is to drop pkt %s."
-                    "active member: %s, may_enable: %s enable: %s "
-                    "LACP status:%d",
-                    member->name,
+        VLOG_DBG_RL(&rl, "bond %s: member %s: "
+                    "admissibility verdict is to drop pkt%s, "
+                    "active member: %s, may_enable: %s, enabled: %s, "
+                    "LACP status: %s",
+                    bond->name, member->name,
                     (verdict == BV_DROP_IF_MOVED) ?
-                        "as different port is learned" : "",
+                        " as different port is learned" : "",
                     (bond->active_member == member) ? "true" : "false",
                     member->may_enable ? "true" : "false",
                     member->enabled ? "true" : "false",
-                    bond->lacp_status);
+                    lacp_status_description(bond->lacp_status));
     }
 
     ovs_rwlock_unlock(&rwlock);
@@ -1503,21 +1503,8 @@ bond_print_details(struct ds *ds, const struct bond *bond)
                       bond->next_rebalance - time_msec());
     }
 
-    ds_put_cstr(ds, "lacp_status: ");
-    switch (bond->lacp_status) {
-    case LACP_NEGOTIATED:
-        ds_put_cstr(ds, "negotiated\n");
-        break;
-    case LACP_CONFIGURED:
-        ds_put_cstr(ds, "configured\n");
-        break;
-    case LACP_DISABLED:
-        ds_put_cstr(ds, "off\n");
-        break;
-    default:
-        ds_put_cstr(ds, "<unknown>\n");
-        break;
-    }
+    ds_put_format(ds, "lacp_status: %s\n",
+                  lacp_status_description(bond->lacp_status));
 
     ds_put_format(ds, "lacp_fallback_ab: %s\n",
                   bond->lacp_fallback_ab ? "true" : "false");