| Message ID | 1526591733-4450-30-git-send-email-gvrose8192@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add ERSPAN support | expand |
On Thu, May 17, 2018 at 02:15:21PM -0700, Greg Rose wrote: > In netdev_to_ovs_vport_type() it checks for netdev types matching > "gre" with a strstr(). This makes it match ip6gre as well and return > OVS_VPORT_TYPE_GRE, which is clearly wrong. > > Move the usage of strstr() *after* all the exact matches with strcmp() > to avoid the problem permanently because when I added the ip6gre > type I ran into a very difficult to detect bug. > > Signed-off-by: Greg Rose <gvrose8192@gmail.com> I think it would be better to avoid strstr here entirely. As far as I can tell this is here because we used to have gre, ipsec_gre, gre64, and ipsec_gre64 vport types, but now only gre is left.
On 5/17/2018 4:12 PM, Ben Pfaff wrote: > On Thu, May 17, 2018 at 02:15:21PM -0700, Greg Rose wrote: >> In netdev_to_ovs_vport_type() it checks for netdev types matching >> "gre" with a strstr(). This makes it match ip6gre as well and return >> OVS_VPORT_TYPE_GRE, which is clearly wrong. >> >> Move the usage of strstr() *after* all the exact matches with strcmp() >> to avoid the problem permanently because when I added the ip6gre >> type I ran into a very difficult to detect bug. >> >> Signed-off-by: Greg Rose <gvrose8192@gmail.com> > I think it would be better to avoid strstr here entirely. As far as I > can tell this is here because we used to have gre, ipsec_gre, gre64, and > ipsec_gre64 vport types, but now only gre is left. So that's the history of that! Thanks Ben, it'll be fixed up in V2. - Greg
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 607b497..a3c5381 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -817,8 +817,6 @@ netdev_to_ovs_vport_type(const char *type) return OVS_VPORT_TYPE_STT; } else if (!strcmp(type, "geneve")) { return OVS_VPORT_TYPE_GENEVE; - } else if (strstr(type, "gre")) { - return OVS_VPORT_TYPE_GRE; } else if (!strcmp(type, "vxlan")) { return OVS_VPORT_TYPE_VXLAN; } else if (!strcmp(type, "lisp")) { @@ -829,6 +827,8 @@ netdev_to_ovs_vport_type(const char *type) return OVS_VPORT_TYPE_IP6ERSPAN; } else if (!strcmp(type, "ip6gre")) { return OVS_VPORT_TYPE_IP6GRE; + } else if (strstr(type, "gre")) { + return OVS_VPORT_TYPE_GRE; } else { return OVS_VPORT_TYPE_UNSPEC; }
In netdev_to_ovs_vport_type() it checks for netdev types matching "gre" with a strstr(). This makes it match ip6gre as well and return OVS_VPORT_TYPE_GRE, which is clearly wrong. Move the usage of strstr() *after* all the exact matches with strcmp() to avoid the problem permanently because when I added the ip6gre type I ran into a very difficult to detect bug. Signed-off-by: Greg Rose <gvrose8192@gmail.com> --- lib/dpif-netlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)