| Message ID | 20190322125839.28137-1-i.maximets@samsung.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] bridge: Propagate patch port pairing errors to db. | expand |
On 22 Mar 2019, at 13:58, Ilya Maximets wrote: > 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." > > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Thanks for taking care of this Ilya! Acked-by: Eelco Chaudron <echaudro@redhat.com>
On Mon, Mar 25, 2019 at 12:48:34PM +0100, Eelco Chaudron wrote: > > > On 22 Mar 2019, at 13:58, Ilya Maximets wrote: > > > 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." > > > > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > > Thanks for taking care of this Ilya! > > Acked-by: Eelco Chaudron <echaudro@redhat.com> Thanks, Ilya (and Eelco). I applied this to master.
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 21dd54bab..f0d387ccd 100644 --- 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 d1a87a59e..eb62a8fb3 100644 --- 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 40780e276..2b33ee382 100644 --- 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 50208d481..510ace103 100644 --- 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 a427b0122..48d8c4de1 100644 --- 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) {
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." Signed-off-by: Ilya Maximets <i.maximets@samsung.com> --- ofproto/ofproto-dpif.c | 21 +++++++++++++++++++++ ofproto/ofproto-provider.h | 11 +++++++++++ ofproto/ofproto.c | 11 +++++++++++ ofproto/ofproto.h | 3 +++ vswitchd/bridge.c | 13 +++++++++++++ 5 files changed, 59 insertions(+)