On 7/7/17, 5:58 AM, "ovs-dev-boun...@openvswitch.org on behalf of Aaron Conole"
<ovs-dev-boun...@openvswitch.org on behalf of acon...@redhat.com> wrote:
Ben Pfaff <b...@ovn.org> writes:
> It took me a few minutes to figure out what the code for deciding on the
> vhost socket directory was doing, because it was needlessly complicated.
> This simplifies it.
>
> Build tested only.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> This is a repost of a patch first posted in April.
Somehow this dropped off my radar. Sorry.
Maybe worth adding a CC and Fixes
CC: Aaron Conole <acon...@redhat.com>
Fixes: d8a8f353c23ee (“netdev-dpdk: Restrict vhost_sock_dir”)
> lib/dpdk.c | 64
++++++++++++++++++++++----------------------------------------
> 1 file changed, 23 insertions(+), 41 deletions(-)
>
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index 8da6c324407e..1bd31ba345f5 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -42,29 +42,29 @@ static FILE *log_stream = NULL; /* Stream for
DPDK log redirection */
>
> static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets
*/
>
> -static int
> -process_vhost_flags(char *flag, const char *default_val, int size,
> - const struct smap *ovs_other_config,
> - char **new_val)
> +static char *
> +get_custom_vhost_sock_dir(const char *stem, const char *suffix)
> {
> - const char *val;
> - int changed = 0;
> + if (!suffix) {
> + return NULL;
> + }
>
> - val = smap_get(ovs_other_config, flag);
> + if (strstr(suffix, "..")) {
> + VLOG_ERR("ignoring vhost-sock-dir '%s' with invalid characters
'..'",
> + suffix);
> + return NULL;
> + }
>
> - /* Process the vhost-sock-dir flag if it is provided, otherwise
resort to
> - * default value.
> - */
> - if (val && (strlen(val) <= size)) {
> - changed = 1;
> - *new_val = xstrdup(val);
> - VLOG_INFO("User-provided %s in use: %s", flag, *new_val);
> - } else {
> - VLOG_INFO("No %s provided - defaulting to %s", flag,
default_val);
> - *new_val = xstrdup(default_val);
> + struct stat s;
> + char *dir = xasprintf("%s/%s", stem, suffix);
> + if (stat(dir, &s)) {
> + VLOG_ERR("%s: ignoring requested vhost socket directory (%s)",
> + dir, ovs_strerror(errno));
> + free(dir);
> + return NULL;
> }
>
> - return changed;
> + return dir;
> }
>
> static char **
> @@ -311,7 +311,6 @@ dpdk_init__(const struct smap *ovs_other_config)
> bool auto_determine = true;
> int err = 0;
> cpu_set_t cpuset;
> - char *sock_dir_subcomponent;
>
> log_stream = fopencookie(NULL, "w+", dpdk_log_func);
> if (log_stream == NULL) {
> @@ -321,28 +320,11 @@ dpdk_init__(const struct smap *ovs_other_config)
> rte_openlog_stream(log_stream);
> }
>
> - if (process_vhost_flags("vhost-sock-dir", ovs_rundir(),
> - NAME_MAX, ovs_other_config,
> - &sock_dir_subcomponent)) {
> - struct stat s;
> - if (!strstr(sock_dir_subcomponent, "..")) {
> - vhost_sock_dir = xasprintf("%s/%s", ovs_rundir(),
> - sock_dir_subcomponent);
> -
> - err = stat(vhost_sock_dir, &s);
> - if (err) {
> - VLOG_ERR("vhost-user sock directory '%s' does not
exist.",
> - vhost_sock_dir);
> - }
> - } else {
> - vhost_sock_dir = xstrdup(ovs_rundir());
> - VLOG_ERR("vhost-user sock directory request '%s/%s' has
invalid"
> - "characters '..' - using %s instead.",
> - ovs_rundir(), sock_dir_subcomponent, ovs_rundir());
> - }
> - free(sock_dir_subcomponent);
> - } else {
> - vhost_sock_dir = sock_dir_subcomponent;
> + vhost_sock_dir = get_custom_vhost_sock_dir(ovs_rundir(),
> + smap_get(ovs_other_config,
> +
"vhost-sock-dir"));
> + if (!vhost_sock_dir) {
> + vhost_sock_dir = xstrdup(ovs_rundir());
> }
Because of the way DPDK works, I think we need a VLOG_INFO here
indicating which path we chose for the vhost socket directory.
Otherwise, if the user changes the db value and forgets to restart, we
have no indicator to tell us where to look. Unless there's some other
way of knowing.
Otherwise, LGTM.
If it was tested, then I won’t bother.
>
> argv = grow_argv(&argv, 0, 1);
_______________________________________________
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=JJMih84adYRvWhtsWpsg8AJNXFDOFJMGJWX-nU3WQNk&s=ZLwfX00i0GIxl4Oe4hlMUZuWcAuFELTxTpZyIbhMF98&e=
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
