[ovs-dev] [PATCH ovs V8 05/26] dpif: Save added ports in a port map for netdev flow api use

[Simon Horman]

On Tue, May 16, 2017 at 12:16:46PM +0300, Roi Dayan wrote:
> 
> 
> On 08/05/2017 15:49, Simon Horman wrote:
> >On Wed, May 03, 2017 at 06:07:56PM +0300, 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>
> >>Reviewed-by: Simon Horman <simon.horman at netronome.com>
> >
> >...
> >
> >>diff --git a/lib/netdev.h b/lib/netdev.h
> >>index 7435fdf..9aa7e5e 100644
> >>--- a/lib/netdev.h
> >>+++ b/lib/netdev.h
> >>@@ -181,6 +181,12 @@ 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_ports_insert(struct netdev *, const void *obj, struct dpif_port *);
> >>+struct netdev *netdev_ports_get(odp_port_t port, const void *obj);
> >>+int netdev_ports_remove(odp_port_t port, const void *obj);
> >>+odp_port_t netdev_ifindex_to_odp_port(int ifindex);
> >>+
> >> /* native tunnel APIs */
> >> /* Structure to pass parameters required to build a tunnel header. */
> >> struct netdev_tnl_build_header_params {
> >
> >This patch seems to only partially address the review provided
> >by Joe Stringer for v7. In particular:
> >
> >* netdev_ports_get() -> netdev_ports_lookup()
> >* Feedback regarding 'obj' being a not particularly clear abstraction.
> >
> 
> we did refactor all functions to have prefix netdev_ports_*
> there are both functions netdev_ports_get() and netdev_ports_lookup().
> did I miss something?

Thanks, I see that now. Looks good.

> about 'obj', I mentioned this in the changelog that it's left out for now
> and could be done in followup commit. is it ok?

Yes, that is fine by me.