| Message ID | 1498236753-124382-1-git-send-email-dlu998@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 23/06/2017 19:52, Darrell Ball wrote: > Hardware offload introduced extra tracking of netdev ports. This > included ovs-netdev, which is really for internal infra usage for > the userpace datapath. This breaks cleanup of the userspace > datapath. One effect is that all userspace datapath system tests > fail except for the first one run. There is no need to do this > extra tracking of this port, hence it is skipped by also checking > for the port name as the type does not help in this case. Adding > an extra type or field is another way to fix this, but this would > probably be overkill as this port is a constant and the misuse > potential very limited. > > CC: Paul Blakey <paulb@mellanox.com> > Fixes: 32b77c316d9982("dpif: Save added ports in a port map.") > Signed-off-by: Darrell Ball <dlu998@gmail.com> > --- > > v1->v2: Add a utility function to check for "internal" ports. > Add an extra check for add port case. > > lib/dpif.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif.c b/lib/dpif.c > index 10bdd70..754b83b 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -100,6 +100,16 @@ static bool should_log_flow_message(const struct vlog_module *module, > /* Incremented whenever tnl route, arp, etc changes. */ > struct seq *tnl_conf_seq; > > +static bool > +dpif_is_internal_port(const char *type, const char *name) > +{ > + /* ovs-netdev is a tap device that is used as an > + * internal port for the userspace datapath, hence > + * treat it as such. */ > + return !strcmp(type, "internal") || > + !strcmp(name, "ovs-netdev"); > +} > + > static void > dp_initialize(void) > { > @@ -350,7 +360,7 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp) > struct netdev *netdev; > int err; > > - if (!strcmp(dpif_port.type, "internal")) { > + if (dpif_is_internal_port(dpif_port.type, dpif_port.name)) { > continue; > } > > @@ -556,7 +566,9 @@ dpif_port_add(struct dpif *dpif, struct netdev *netdev, odp_port_t *port_nop) > VLOG_DBG_RL(&dpmsg_rl, "%s: added %s as port %"PRIu32, > dpif_name(dpif), netdev_name, port_no); > > - if (strcmp(netdev_get_type(netdev), "internal")) { > + if (!dpif_is_internal_port(netdev_get_type(netdev), > + netdev_get_name(netdev))) { > + > struct dpif_port dpif_port; > > dpif_port.type = CONST_CAST(char *, netdev_get_type(netdev)); > Reviewed-by: Roi Dayan <roid@mellanox.com>
diff --git a/lib/dpif.c b/lib/dpif.c index 10bdd70..754b83b 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -100,6 +100,16 @@ static bool should_log_flow_message(const struct vlog_module *module, /* Incremented whenever tnl route, arp, etc changes. */ struct seq *tnl_conf_seq; +static bool +dpif_is_internal_port(const char *type, const char *name) +{ + /* ovs-netdev is a tap device that is used as an + * internal port for the userspace datapath, hence + * treat it as such. */ + return !strcmp(type, "internal") || + !strcmp(name, "ovs-netdev"); +} + static void dp_initialize(void) { @@ -350,7 +360,7 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp) struct netdev *netdev; int err; - if (!strcmp(dpif_port.type, "internal")) { + if (dpif_is_internal_port(dpif_port.type, dpif_port.name)) { continue; } @@ -556,7 +566,9 @@ dpif_port_add(struct dpif *dpif, struct netdev *netdev, odp_port_t *port_nop) VLOG_DBG_RL(&dpmsg_rl, "%s: added %s as port %"PRIu32, dpif_name(dpif), netdev_name, port_no); - if (strcmp(netdev_get_type(netdev), "internal")) { + if (!dpif_is_internal_port(netdev_get_type(netdev), + netdev_get_name(netdev))) { + struct dpif_port dpif_port; dpif_port.type = CONST_CAST(char *, netdev_get_type(netdev));
Hardware offload introduced extra tracking of netdev ports. This included ovs-netdev, which is really for internal infra usage for the userpace datapath. This breaks cleanup of the userspace datapath. One effect is that all userspace datapath system tests fail except for the first one run. There is no need to do this extra tracking of this port, hence it is skipped by also checking for the port name as the type does not help in this case. Adding an extra type or field is another way to fix this, but this would probably be overkill as this port is a constant and the misuse potential very limited. CC: Paul Blakey <paulb@mellanox.com> Fixes: 32b77c316d9982("dpif: Save added ports in a port map.") Signed-off-by: Darrell Ball <dlu998@gmail.com> --- v1->v2: Add a utility function to check for "internal" ports. Add an extra check for add port case. lib/dpif.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)