On Tue, Feb 19, 2019 at 10:51:40AM +0900, Chris Mi wrote: > If vxlan dst_port is not default 4789, "ovs-dpctl dump-flows" > will fail. The error message is: > > netdev_linux|INFO|ioctl(SIOCGIFINDEX) on vxlan_sys_4789 device failed: No > such device > > That's because when calling netdev_vport_construct() for netdev > vxlan_sys_xxxx, the default dst_port is used. Actually, the dst_port > value is in the name of the netdev. Use it to avoid the error.
During ovs initialization, the interface is created and then the construct is called. That's where this patch is setting the dst_port, which looks okay because after that set_tunnel_config() is called when setting the iface, and dst_port gets updated with the db config. However, during dump-flows it opens the netdev but doesn't look at the db as far as I can tell, then you see the issue because it is hardcoded to the default port. I don't think it makes sense to check the db at this point since we care more about the real iface name. With that said, the patch makes sense to me and as you said, would be nice to apply the same fix to other tunnel types. fbl > > Signed-off-by: Chris Mi <[email protected]> > --- > > v1 > == > > Any comment about this patch? We are not sure if it is correct > to verify the port from the name. If it is correct, is it applicable > for other tunnels? Thanks! > > > lib/netdev-vport.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c > index 808a43f99..be68fad1a 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -189,17 +189,28 @@ netdev_vport_alloc(void) > int > netdev_vport_construct(struct netdev *netdev_) > { > + const struct netdev_class *class = netdev_get_class(netdev_); > + const char *dpif_port = netdev_vport_class_get_dpif_port(class); > struct netdev_vport *dev = netdev_vport_cast(netdev_); > const char *type = netdev_get_type(netdev_); > > ovs_mutex_init(&dev->mutex); > eth_addr_random(&dev->etheraddr); > > - /* Add a default destination port for tunnel ports if none specified. */ > + /* Add a default destination port for tunnel ports if none specified. > + * For vxlan, the dst_port is in the netdev name when creating it, so > + * use it instead of the default one */ > if (!strcmp(type, "geneve")) { > dev->tnl_cfg.dst_port = htons(GENEVE_DST_PORT); > } else if (!strcmp(type, "vxlan")) { > - dev->tnl_cfg.dst_port = htons(VXLAN_DST_PORT); > + const char *p, *name = netdev_->name; > + uint16_t port = VXLAN_DST_PORT; > + > + if (!strncmp(name, dpif_port, strlen(dpif_port))) { > + p = name + strlen(dpif_port) + 1; > + port = atoi(p); > + } > + dev->tnl_cfg.dst_port = htons(port); > update_vxlan_global_cfg(netdev_, NULL, &dev->tnl_cfg); > } else if (!strcmp(type, "lisp")) { > dev->tnl_cfg.dst_port = htons(LISP_DST_PORT); > -- > 2.14.4 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
