Message ID | 1498192031-61177-1-git-send-email-dlu998@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Thu, Jun 22, 2017 at 09:27:11PM -0700, 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. There is no need to do this extra tracking of this port, > hence it is skipped by checking for the port name as the type does > not help here. 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> > --- > lib/dpif.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif.c b/lib/dpif.c > index 10bdd70..8624d34 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -350,7 +350,11 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp) > struct netdev *netdev; > int err; > > - if (!strcmp(dpif_port.type, "internal")) { > + /* ovs-netdev is a tap device that is used as an > + * internal port for the userspace datapath, hence > + * don't track it here. */ > + if (!strcmp(dpif_port.type, "internal") || > + (!strcmp(dpif_port.name, "ovs-netdev"))) { I don't understand this issue, so this is not a review, but please do note this style point from coding-style.rst: Do not parenthesize the operands of ``&&`` and ``||`` unless operator precedence makes it necessary, or unless the operands are themselves expressions that use ``&&`` and ``||``. Thus: :: if (!isdigit((unsigned char)s[0]) || !isdigit((unsigned char)s[1]) || !isdigit((unsigned char)s[2])) { printf("string %s does not start with 3-digit code\n", s); } but :: if (rule && (!best || rule->priority > best->priority)) { best = rule; }
On 6/23/17, 1:26 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ben Pfaff" <ovs-dev-bounces@openvswitch.org on behalf of blp@ovn.org> wrote: On Thu, Jun 22, 2017 at 09:27:11PM -0700, 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. There is no need to do this extra tracking of this port, > hence it is skipped by checking for the port name as the type does > not help here. 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> > --- > lib/dpif.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif.c b/lib/dpif.c > index 10bdd70..8624d34 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -350,7 +350,11 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp) > struct netdev *netdev; > int err; > > - if (!strcmp(dpif_port.type, "internal")) { > + /* ovs-netdev is a tap device that is used as an > + * internal port for the userspace datapath, hence > + * don't track it here. */ > + if (!strcmp(dpif_port.type, "internal") || > + (!strcmp(dpif_port.name, "ovs-netdev"))) { I don't understand this issue, so this is not a review, but please do note this style point from coding-style.rst: Do not parenthesize the operands of ``&&`` and ``||`` unless operator precedence makes it necessary, or unless the operands are themselves expressions that use ``&&`` and ``||``. Thus: :: if (!isdigit((unsigned char)s[0]) || !isdigit((unsigned char)s[1]) || !isdigit((unsigned char)s[2])) { printf("string %s does not start with 3-digit code\n", s); } but :: if (rule && (!best || rule->priority > best->priority)) { best = rule; } Thanks for the reminder _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=Cvf-v52TVTcXUOH8suwH429_Yw7GH70eQv8do78OqO4&s=WfZDuxBQ0mf0fynx4BWSxBWe1OeFFvg4G_aHRCkAlCs&e=
diff --git a/lib/dpif.c b/lib/dpif.c index 10bdd70..8624d34 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -350,7 +350,11 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp) struct netdev *netdev; int err; - if (!strcmp(dpif_port.type, "internal")) { + /* ovs-netdev is a tap device that is used as an + * internal port for the userspace datapath, hence + * don't track it here. */ + if (!strcmp(dpif_port.type, "internal") || + (!strcmp(dpif_port.name, "ovs-netdev"))) { continue; }
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. There is no need to do this extra tracking of this port, hence it is skipped by checking for the port name as the type does not help here. 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> --- lib/dpif.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)