Hi Flavio, > -----Original Message----- > From: Flavio Leitner <[email protected]> > Sent: Wednesday, February 20, 2019 11:01 PM > To: Chris Mi <[email protected]> > Cc: [email protected]; [email protected] > Subject: Re: [ovs-dev] [PATCH] netdev-vport: Use the vxlan dst_port in > netdev name > > 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.
Thanks for your detailed info and support for this change. Patch v2 was sent for review. Thanks, Chris > > 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma > il.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- > dev&data=02%7C01%7Cchrism%40mellanox.com%7C7ea17aefa05649b8 > 815f08d6974440ce%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C63 > 6862716738809934&sdata=vsI2UA1d96E2LxRiX9cDiNCb4UxjOVlMSsvZ2D > Xoy30%3D&reserved=0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
