On Wed, Feb 15, 2017 at 10:46:20AM +0200, Roi Dayan wrote:
> 
> 
> On 14/02/2017 17:52, Simon Horman wrote:
> >On Wed, Feb 08, 2017 at 05:29:19PM +0200, Roi Dayan wrote:
> >>From: Paul Blakey <pa...@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 <pa...@mellanox.com>
> >>Reviewed-by: Roi Dayan <r...@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.
> >
> 
> I'm not sure I'm following you here. how would this be done?
> we want to same dpif_port and netdev resolution so we can use it later.
> right that we use it in dpif_class it self but other classes would want
> maybe to use this in the future so it's being handles in dpif_port_add.

I was just suggesting a minor cleanup as it seems a bit untidy to have
struct dpif_port data marshalled inline. My assumption was that
dpif->dpif_class->port_add(), which is called just above the code in
question, was doing the same marshalling and it could be leveraged.  I no
longer think that is the case.

...
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to