diff options
author | Daniele Di Proietto <diproiettod@vmware.com> | 2016-12-20 17:58:14 -0800 |
---|---|---|
committer | Daniele Di Proietto <diproiettod@vmware.com> | 2017-01-11 18:29:39 -0800 |
commit | 9fff138ec3a6dbe75073d16cba7fbe86ac273c36 (patch) | |
tree | 641387779a4ad2a35a429d4a47c8c0f99e254f5d | |
parent | bd4e172b287c76bf9fea0a5afa751dc3fe37e2d4 (diff) | |
download | openvswitch-9fff138ec3a6dbe75073d16cba7fbe86ac273c36.tar.gz |
netdev: Add 'errp' to set_config().
Since 55e075e65ef9("netdev-dpdk: Arbitrary 'dpdk' port naming"),
set_config() is used to identify a DPDK device, so it's better to report
its detailed error message to the user. Tunnel devices and patch ports
rely a lot on set_config() as well.
This commit adds a param to set_config() that can be used to return
an error message and makes use of that in netdev-dpdk and netdev-vport.
Before this patch:
$ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
ovs-vsctl: Error detected while setting up 'dpdk0': dpdk0: could not set
configuration (Invalid argument). See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/var/log/openvswitch/".
$ ovs-vsctl add-port br0 p+ -- set Interface p+ type=patch
ovs-vsctl: Error detected while setting up 'p+': p+: could not set
configuration (Invalid argument). See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/var/log/openvswitch/".
$ ovs-vsctl add-port br0 gnv0 -- set Interface gnv0 type=geneve
ovs-vsctl: Error detected while setting up 'gnv0': gnv0: could not set
configuration (Invalid argument). See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/var/log/openvswitch/".
After this patch:
$ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
ovs-vsctl: Error detected while setting up 'dpdk0': 'dpdk0' is missing
'options:dpdk-devargs'. The old 'dpdk<port_id>' names are not
supported. See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/var/log/openvswitch/".
$ ovs-vsctl add-port br0 p+ -- set Interface p+ type=patch
ovs-vsctl: Error detected while setting up 'p+': p+: patch type requires
valid 'peer' argument. See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/var/log/openvswitch/".
$ ovs-vsctl add-port br0 gnv0 -- set Interface gnv0 type=geneve
ovs-vsctl: Error detected while setting up 'gnv0': gnv0: geneve type
requires valid 'remote_ip' argument. See ovs-vswitchd log for
details.
ovs-vsctl: The default log directory is "/var/log/openvswitch/".
CC: Ciara Loftus <ciara.loftus@intel.com>
CC: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Acked-by: Kevin Traynor <ktraynor@redhat.com>
Tested-by: Ciara Loftus <ciara.loftus@intel.com>
-rw-r--r-- | lib/netdev-dpdk.c | 27 | ||||
-rw-r--r-- | lib/netdev-dummy.c | 3 | ||||
-rw-r--r-- | lib/netdev-provider.h | 9 | ||||
-rw-r--r-- | lib/netdev-vport.c | 76 | ||||
-rw-r--r-- | lib/netdev.c | 10 |
5 files changed, 84 insertions, 41 deletions
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index b4869d838..409ee6405 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1132,7 +1132,7 @@ netdev_dpdk_lookup_by_port_id(int port_id) } static int -netdev_dpdk_process_devargs(const char *devargs) +netdev_dpdk_process_devargs(const char *devargs, char **errp) { uint8_t new_port_id = UINT8_MAX; @@ -1145,7 +1145,7 @@ netdev_dpdk_process_devargs(const char *devargs) VLOG_INFO("Device '%s' attached to DPDK", devargs); } else { /* Attach unsuccessful */ - VLOG_INFO("Error attaching device '%s' to DPDK", devargs); + VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", devargs); return -1; } } @@ -1184,7 +1184,8 @@ dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, } static int -netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args) +netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, + char **errp) { struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); bool rx_fc_en, tx_fc_en, autoneg; @@ -1225,7 +1226,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args) * is valid */ if (!(dev->devargs && !strcmp(dev->devargs, new_devargs) && rte_eth_dev_is_valid_port(dev->port_id))) { - int new_port_id = netdev_dpdk_process_devargs(new_devargs); + int new_port_id = netdev_dpdk_process_devargs(new_devargs, errp); if (!rte_eth_dev_is_valid_port(new_port_id)) { err = EINVAL; } else if (new_port_id == dev->port_id) { @@ -1235,10 +1236,10 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args) struct netdev_dpdk *dup_dev; dup_dev = netdev_dpdk_lookup_by_port_id(new_port_id); if (dup_dev) { - VLOG_WARN("'%s' is trying to use device '%s' which is " - "already in use by '%s'.", - netdev_get_name(netdev), new_devargs, - netdev_get_name(&dup_dev->up)); + VLOG_WARN_BUF(errp, "'%s' is trying to use device '%s' " + "which is already in use by '%s'", + netdev_get_name(netdev), new_devargs, + netdev_get_name(&dup_dev->up)); err = EADDRINUSE; } else { int sid = rte_eth_dev_socket_id(new_port_id); @@ -1251,7 +1252,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args) } } } else { - /* dpdk-devargs unspecified - can't configure device */ + VLOG_WARN_BUF(errp, "'%s' is missing 'options:dpdk-devargs'. " + "The old 'dpdk<port_id>' names are not supported", + netdev_get_name(netdev)); err = EINVAL; } @@ -1288,7 +1291,8 @@ out: } static int -netdev_dpdk_ring_set_config(struct netdev *netdev, const struct smap *args) +netdev_dpdk_ring_set_config(struct netdev *netdev, const struct smap *args, + char **errp OVS_UNUSED) { struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); @@ -1301,7 +1305,8 @@ netdev_dpdk_ring_set_config(struct netdev *netdev, const struct smap *args) static int netdev_dpdk_vhost_client_set_config(struct netdev *netdev, - const struct smap *args) + const struct smap *args, + char **errp OVS_UNUSED) { struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); const char *path; diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index 2f9365e15..e6e36cdb7 100644 --- a/lib/netdev-dummy.c +++ b/lib/netdev-dummy.c @@ -830,7 +830,8 @@ netdev_dummy_set_in6(struct netdev *netdev_, struct in6_addr *in6, #define DUMMY_MAX_QUEUES_PER_PORT 1024 static int -netdev_dummy_set_config(struct netdev *netdev_, const struct smap *args) +netdev_dummy_set_config(struct netdev *netdev_, const struct smap *args, + char **errp OVS_UNUSED) { struct netdev_dummy *netdev = netdev_dummy_cast(netdev_); const char *pcap; diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index c8507a56b..8346fc4bb 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -277,8 +277,13 @@ struct netdev_class { /* Changes the device 'netdev''s configuration to 'args'. * * If this netdev class does not support configuration, this may be a null - * pointer. */ - int (*set_config)(struct netdev *netdev, const struct smap *args); + * pointer. + * + * If the return value is not zero (meaning that an error occurred), + * the provider can allocate a string with an error message in '*errp'. + * The caller has to call free on it. */ + int (*set_config)(struct netdev *netdev, const struct smap *args, + char **errp); /* Returns the tunnel configuration of 'netdev'. If 'netdev' is * not a tunnel, returns null. diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 02a246aff..ad5ffcc81 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -35,6 +35,7 @@ #include "netdev-native-tnl.h" #include "netdev-provider.h" #include "netdev-vport-private.h" +#include "openvswitch/dynamic-string.h" #include "ovs-router.h" #include "packets.h" #include "poll-loop.h" @@ -397,15 +398,17 @@ parse_tunnel_ip(const char *value, bool accept_mcast, bool *flow, } static int -set_tunnel_config(struct netdev *dev_, const struct smap *args) +set_tunnel_config(struct netdev *dev_, const struct smap *args, char **errp) { struct netdev_vport *dev = netdev_vport_cast(dev_); const char *name = netdev_get_name(dev_); const char *type = netdev_get_type(dev_); + struct ds errors = DS_EMPTY_INITIALIZER; bool needs_dst_port, has_csum; uint16_t dst_proto = 0, src_proto = 0; struct netdev_tunnel_config tnl_cfg; struct smap_node *node; + int err; has_csum = strstr(type, "gre") || strstr(type, "geneve") || strstr(type, "stt") || strstr(type, "vxlan"); @@ -433,25 +436,24 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) SMAP_FOR_EACH (node, args) { if (!strcmp(node->key, "remote_ip")) { - int err; err = parse_tunnel_ip(node->value, false, &tnl_cfg.ip_dst_flow, &tnl_cfg.ipv6_dst, &dst_proto); switch (err) { case ENOENT: - VLOG_WARN("%s: bad %s 'remote_ip'", name, type); + ds_put_format(&errors, "%s: bad %s 'remote_ip'\n", name, type); break; case EINVAL: - VLOG_WARN("%s: multicast remote_ip=%s not allowed", - name, node->value); - return EINVAL; + ds_put_format(&errors, + "%s: multicast remote_ip=%s not allowed\n", + name, node->value); + goto out; } } else if (!strcmp(node->key, "local_ip")) { - int err; err = parse_tunnel_ip(node->value, true, &tnl_cfg.ip_src_flow, &tnl_cfg.ipv6_src, &src_proto); switch (err) { case ENOENT: - VLOG_WARN("%s: bad %s 'local_ip'", name, type); + ds_put_format(&errors, "%s: bad %s 'local_ip'\n", name, type); break; } } else if (!strcmp(node->key, "tos")) { @@ -464,7 +466,8 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) if (*endptr == '\0' && tos == (tos & IP_DSCP_MASK)) { tnl_cfg.tos = tos; } else { - VLOG_WARN("%s: invalid TOS %s", name, node->value); + ds_put_format(&errors, "%s: invalid TOS %s\n", name, + node->value); } } } else if (!strcmp(node->key, "ttl")) { @@ -498,7 +501,8 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) if (!strcmp(type, "vxlan") && !strcmp(ext, "gbp")) { tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GBP); } else { - VLOG_WARN("%s: unknown extension '%s'", name, ext); + ds_put_format(&errors, "%s: unknown extension '%s'\n", + name, ext); } ext = strtok_r(NULL, ",", &save_ptr); @@ -506,24 +510,33 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) free(str); } else { - VLOG_WARN("%s: unknown %s argument '%s'", name, type, node->key); + ds_put_format(&errors, "%s: unknown %s argument '%s'\n", + name, type, node->key); } } if (!ipv6_addr_is_set(&tnl_cfg.ipv6_dst) && !tnl_cfg.ip_dst_flow) { - VLOG_ERR("%s: %s type requires valid 'remote_ip' argument", - name, type); - return EINVAL; + ds_put_format(&errors, + "%s: %s type requires valid 'remote_ip' argument\n", + name, type); + err = EINVAL; + goto out; } if (tnl_cfg.ip_src_flow && !tnl_cfg.ip_dst_flow) { - VLOG_ERR("%s: %s type requires 'remote_ip=flow' with 'local_ip=flow'", - name, type); - return EINVAL; + ds_put_format(&errors, + "%s: %s type requires 'remote_ip=flow' " + "with 'local_ip=flow'\n", + name, type); + err = EINVAL; + goto out; } if (src_proto && dst_proto && src_proto != dst_proto) { - VLOG_ERR("%s: 'remote_ip' and 'local_ip' has to be of the same address family", - name); - return EINVAL; + ds_put_format(&errors, + "%s: 'remote_ip' and 'local_ip' " + "has to be of the same address family\n", + name); + err = EINVAL; + goto out; } if (!tnl_cfg.ttl) { tnl_cfg.ttl = DEFAULT_TTL; @@ -545,7 +558,18 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) } ovs_mutex_unlock(&dev->mutex); - return 0; + err = 0; + +out: + ds_chomp(&errors, '\n'); + VLOG_WARN("%s", ds_cstr(&errors)); + if (err) { + *errp = ds_steal_cstr(&errors); + } + + ds_destroy(&errors); + + return err; } static int @@ -693,7 +717,7 @@ get_patch_config(const struct netdev *dev_, struct smap *args) } static int -set_patch_config(struct netdev *dev_, const struct smap *args) +set_patch_config(struct netdev *dev_, const struct smap *args, char **errp) { struct netdev_vport *dev = netdev_vport_cast(dev_); const char *name = netdev_get_name(dev_); @@ -701,17 +725,19 @@ set_patch_config(struct netdev *dev_, const struct smap *args) peer = smap_get(args, "peer"); if (!peer) { - VLOG_ERR("%s: patch type requires valid 'peer' argument", name); + VLOG_ERR_BUF(errp, "%s: patch type requires valid 'peer' argument", + name); return EINVAL; } if (smap_count(args) > 1) { - VLOG_ERR("%s: patch type takes only a 'peer' argument", name); + VLOG_ERR_BUF(errp, "%s: patch type takes only a 'peer' argument", + name); return EINVAL; } if (!strcmp(name, peer)) { - VLOG_ERR("%s: patch peer must not be self", name); + VLOG_ERR_BUF(errp, "%s: patch peer must not be self", name); return EINVAL; } diff --git a/lib/netdev.c b/lib/netdev.c index f7a1001f2..afc3818d4 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -417,13 +417,19 @@ netdev_set_config(struct netdev *netdev, const struct smap *args, char **errp) { if (netdev->netdev_class->set_config) { const struct smap no_args = SMAP_INITIALIZER(&no_args); + char *verbose_error = NULL; int error; error = netdev->netdev_class->set_config(netdev, - args ? args : &no_args); + args ? args : &no_args, + &verbose_error); if (error) { - VLOG_WARN_BUF(errp, "%s: could not set configuration (%s)", + VLOG_WARN_BUF(verbose_error ? NULL : errp, + "%s: could not set configuration (%s)", netdev_get_name(netdev), ovs_strerror(error)); + if (verbose_error) { + *errp = verbose_error; + } } return error; } else if (args && !smap_is_empty(args)) { |