bridge: Propagate patch port pairing errors to db.

Virtual ports like 'patch' ports that almost fully implemented on
'ofproto' layer could have internal to 'ofproto' statuses that
could not be retrieved from 'netdev' or other layers. For example,
in current implementation there is no way to get the patch port
pairing status (i.e. if it has usable peer?).

New 'ofproto-provider' API function 'vport_get_status' introduced to
cover this gap. It allowes 'bridge' layer to retrive current status
of ofproto virtual ports and propagate it to DB.
For now we're only interested in pairing errors of 'patch' ports.
That are propagated to the 'error' column of the 'Interface' table.

Ex.:

  $ ovs-vsctl show
    ...
    Bridge "br1"
      ...
      Port "patch1"
        Interface "patch1"
          type: patch
          options: {peer="patch0"}
          error: "No usable peer 'patch0' exists in 'system' datapath."

    Bridge "br0"
      datapath_type: netdev
      ...
      Port "patch0"
        Interface "patch0"
          type: patch
          options: {peer="patch1"}
          error: "No usable peer 'patch1' exists in 'netdev' datapath."

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 21dd54bab09b9de669ddac6d27a2d704b269df2f..f0d387ccd12b1bc4bc73dca001634f4e61a3cd64 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3767,6 +3767,26 @@ port_get_stats(const struct ofport *ofport_, struct netdev_stats *stats)
     return error;
 }
 
+static int
+vport_get_status(const struct ofport *ofport_, char **errp)
+{
+    struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
+    char *peer_name;
+
+    if (!netdev_vport_is_patch(ofport->up.netdev) || ofport->peer) {
+        return 0;
+    }
+
+    peer_name = netdev_vport_patch_peer(ofport->up.netdev);
+    if (!peer_name) {
+        return 0;
+    }
+    *errp = xasprintf("No usable peer '%s' exists in '%s' datapath.",
+                      peer_name, ofport->up.ofproto->type);
+    free(peer_name);
+    return EINVAL;
+}
+
 static int
 port_get_lacp_stats(const struct ofport *ofport_, struct lacp_slave_stats *stats)
 {
@@ -6045,6 +6065,7 @@ const struct ofproto_class ofproto_dpif_class = {
     port_del,
     port_set_config,
     port_get_stats,
+    vport_get_status,
     port_dump_start,
     port_dump_next,
     port_dump_done,

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index d1a87a59e47e3460fd3b9e2173ebf2acd46b8000..eb62a8fb3d0093de48d1d0b463c27eb00de32b51 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1076,6 +1076,17 @@ struct ofproto_class {
     int (*port_get_stats)(const struct ofport *port,
                           struct netdev_stats *stats);
 
+    /* Get status of the virtual port (ex. tunnel, patch).
+     *
+     * Returns '0' if 'port' is not a virtual port or has no errors.
+     * Otherwise, stores the error string in '*errp' and returns positive errno
+     * value. The caller is responsible for freeing '*errp' (with free()).
+     *
+     * This function may be a null pointer if the ofproto implementation does
+     * not support any virtual ports or their states.
+     */
+    int (*vport_get_status)(const struct ofport *port, char **errp);
+
     /* Port iteration functions.
      *
      * The client might not be entirely in control of the ports within an

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 453239df25ddb8878e4a5aba679f1f55735617aa..3579aac0d0f580f858d5cc6e158394880ffc6e4d 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2627,6 +2627,17 @@ ofproto_port_get_stats(const struct ofport *port, struct netdev_stats *stats)
     return error;
 }
 
+int
+ofproto_vport_get_status(const struct ofproto *ofproto, ofp_port_t ofp_port,
+                         char **errp)
+{
+    struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
+
+    return (ofport && ofproto->ofproto_class->vport_get_status)
+           ? ofproto->ofproto_class->vport_get_status(ofport, errp)
+           : EOPNOTSUPP;
+}
+
 static int
 update_port(struct ofproto *ofproto, const char *name)
 {

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 50208d481862a3a4f8f3ba51c46e2ffdbedc191b..510ace103a0d1f0a69f5f23d40cb6d5032bf6ece 100644 (file)
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -317,6 +317,9 @@ void ofproto_port_set_config(struct ofproto *, ofp_port_t ofp_port,
                              const struct smap *cfg);
 int ofproto_port_get_stats(const struct ofport *, struct netdev_stats *stats);
 
+int ofproto_vport_get_status(const struct ofproto *, ofp_port_t ofp_port,
+                             char **errp);
+
 int ofproto_port_query_by_name(const struct ofproto *, const char *devname,
                                struct ofproto_port *);
 

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index a427b0122b49a82ac015afdae7cfa5c9fa40e7d7..48d8c4de1c4d5506a0433f1843ffbabbc49fbb17 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2256,11 +2256,24 @@ static void
 iface_refresh_ofproto_status(struct iface *iface)
 {
     int current;
+    int error;
+    char *errp = NULL;
 
     if (iface_is_synthetic(iface)) {
         return;
     }
 
+    error = ofproto_vport_get_status(iface->port->bridge->ofproto,
+                                     iface->ofp_port, &errp);
+    if (error && error != EOPNOTSUPP) {
+        /* Need to verify to avoid race with transaction from
+         * 'bridge_reconfigure' that clears errors explicitly. */
+        ovsrec_interface_verify_error(iface->cfg);
+        ovsrec_interface_set_error(iface->cfg,
+                                   errp ? errp : ovs_strerror(error));
+        free(errp);
+    }
+
     current = ofproto_port_is_lacp_current(iface->port->bridge->ofproto,
                                            iface->ofp_port);
     if (current >= 0) {