[ovs-dev] [PATCH ovs V3 06/25] dpif: Save added ports in a port map for netdev flow api use

Simon Horman simon.horman at netronome.com
Tue Feb 14 15:52:16 UTC 2017


On Wed, Feb 08, 2017 at 05:29:19PM +0200, Roi Dayan wrote:
> From: Paul Blakey <paulb at mellanox.com>
> 
> To use netdev flow offloading api, dpifs needs to iterate over
> added ports. This addition inserts the added dpif ports in a hash map,
> The map will also be used to translate dpif ports to netdevs.
> 
> Signed-off-by: Paul Blakey <paulb at mellanox.com>
> Reviewed-by: Roi Dayan <roid at mellanox.com>
> ---
>  lib/dpif.c   |  25 ++++++++++++
>  lib/dpif.h   |   2 +
>  lib/netdev.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/netdev.h |  15 ++++++++
>  4 files changed, 163 insertions(+)
> 
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 57aa3c6..d4e4c0a 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c

...

> @@ -545,6 +560,14 @@ dpif_port_add(struct dpif *dpif, struct netdev *netdev, odp_port_t *port_nop)
>      if (!error) {
>          VLOG_DBG_RL(&dpmsg_rl, "%s: added %s as port %"PRIu32,
>                      dpif_name(dpif), netdev_name, port_no);
> +
> +        /* temp dpif_port, will be cloned in netdev_hmap_port_add */
> +        struct dpif_port dpif_port;
> +
> +        dpif_port.type = CONST_CAST(char *, netdev_get_type(netdev));
> +        dpif_port.name = CONST_CAST(char *, netdev_name);
> +        dpif_port.port_no = port_no;
> +        netdev_hmap_port_add(netdev, DPIF_HMAP_KEY(dpif), &dpif_port);

I wonder if it would be cleaner to get the dpif_port from the dpif API,
for example by providing a optionally NULL struct dpif_port * parameter to
dpif_class->port_add.

>      } else {
>          VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port: %s",
>                       dpif_name(dpif), netdev_name, ovs_strerror(error));

...

> diff --git a/lib/netdev.h b/lib/netdev.h
> index d7d4199..96300c4 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -181,6 +181,21 @@ int netdev_init_flow_api(struct netdev *);
>  extern bool netdev_flow_api_enabled;
>  void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
>  
> +struct dpif_port;
> +int netdev_hmap_port_add(struct netdev *, const void *obj, struct dpif_port *);
> +struct netdev *netdev_hmap_port_get(odp_port_t port, const void *obj);
> +int netdev_hmap_port_del(odp_port_t port, const void *obj);
> +struct netdev_flow_dump **netdev_ports_flow_dumps_create(const void *obj,
> +                                                         int *ports);
> +void netdev_ports_flow_flush(const void *obj);

Two functions function above are defined in the following patch.
Probably their definitions should go there too.

> +int netdev_ports_flow_del(const void *obj, const ovs_u128 *ufid,
> +                          struct dpif_flow_stats *stats);

Likewise, the above patch seems to be defined in patch 17 of this series.

> +int netdev_ports_flow_get(const void *obj, struct match *match,
> +                          struct nlattr **actions,
> +                          struct dpif_flow_stats *stats,
> +                          const ovs_u128 *ufid, struct ofpbuf *buf);

The above function is declared in this patch, used in patch 12 and
defined in patch 17. This does not seem correct from a bisection point of
view.

> +odp_port_t netdev_hmap_port_get_byifidx(int ifindex);
> +
>  /* native tunnel APIs */
>  /* Structure to pass parameters required to build a tunnel header. */
>  struct netdev_tnl_build_header_params {
> -- 
> 2.7.4
> 


More information about the dev mailing list