[ovs-dev] [RFC 4/5] dpctl: uses open_type when calling netdev_open

[Daniele Di Proietto]

I would prefer if dpctl kept using the datapath types.  The translation
from database types to datapath type should happen in ofproto, dpctl is
supposed to be used to interact with the datapath directly.

What do you guys think?

The rest of the series looks good to me as well.

Thanks,

Daniele

2016-07-18 10:00 GMT-07:00 Thadeu Lima de Souza Cascardo <
cascardo at redhat.com>:

> dpctl uses a user or database defined type when calling netdev_open.
> Instead, it
> should use the type from dpif_port_open_type. Otherwise, when using the
> internal
> type, it could open it with that type instead of the correct one, which
> would be
> tap or dummy.
> ---
>  lib/dpctl.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index 003602a..f896161 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -274,7 +274,8 @@ dpctl_add_if(int argc OVS_UNUSED, const char *argv[],
>              }
>          }
>
> -        error = netdev_open(name, type, &netdev);
> +        error = netdev_open(name, dpif_port_open_type(dpif_type(dpif),
> type),
> +                            &netdev);
>          if (error) {
>              dpctl_error(dpctl_p, error, "%s: failed to open network
> device",
>                          name);
> @@ -356,7 +357,8 @@ dpctl_set_if(int argc, const char *argv[], struct
> dpctl_params *dpctl_p)
>          dpif_port_destroy(&dpif_port);
>
>          /* Retrieve its existing configuration. */
> -        error = netdev_open(name, type, &netdev);
> +        error = netdev_open(name, dpif_port_open_type(dpif_type(dpif),
> type),
> +                            &netdev);
>          if (error) {
>              dpctl_error(dpctl_p, error, "%s: failed to open network
> device",
>                          name);
> @@ -558,10 +560,13 @@ show_dpif(struct dpif *dpif, struct dpctl_params
> *dpctl_p)
>      qsort(port_nos, n_port_nos, sizeof *port_nos, compare_port_nos);
>
>      for (int i = 0; i < n_port_nos; i++) {
> +        const char *type;
>          if (dpif_port_query_by_number(dpif, port_nos[i], &dpif_port)) {
>              continue;
>          }
>
> +        type = dpif_port_open_type(dpif_type(dpif), dpif_port.type);
> +
>          dpctl_print(dpctl_p, "\tport %u: %s",
>                      dpif_port.port_no, dpif_port.name);
>
> @@ -570,7 +575,7 @@ show_dpif(struct dpif *dpif, struct dpctl_params
> *dpctl_p)
>
>              dpctl_print(dpctl_p, " (%s", dpif_port.type);
>
> -            error = netdev_open(dpif_port.name, dpif_port.type, &netdev);
> +            error = netdev_open(dpif_port.name, type, &netdev);
>              if (!error) {
>                  struct smap config;
>
> @@ -603,7 +608,7 @@ show_dpif(struct dpif *dpif, struct dpctl_params
> *dpctl_p)
>              struct netdev_stats s;
>              int error;
>
> -            error = netdev_open(dpif_port.name, dpif_port.type, &netdev);
> +            error = netdev_open(dpif_port.name, type, &netdev);
>              if (error) {
>                  dpctl_print(dpctl_p, ", open failed (%s)",
>                              ovs_strerror(error));
> @@ -891,6 +896,7 @@ get_in_port_netdev_from_key(struct dpif *dpif, const
> struct ofpbuf *key)
>          struct dpif_port dpif_port;
>          odp_port_t port_no;
>          int error;
> +        const char *type;
>
>          port_no = ODP_PORT_C(nl_attr_get_u32(in_port_nla));
>          error = dpif_port_query_by_number(dpif, port_no, &dpif_port);
> @@ -898,7 +904,8 @@ get_in_port_netdev_from_key(struct dpif *dpif, const
> struct ofpbuf *key)
>              goto out;
>          }
>
> -        netdev_open(dpif_port.name, dpif_port.type, &dev);
> +        type = dpif_port_open_type(dpif_type(dpif), dpif_port.type);
> +        netdev_open(dpif_port.name, type, &dev);
>          dpif_port_destroy(&dpif_port);
>      }
>
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>