On 16 Apr 2024, at 9:57, Jun Gu wrote:
> Hi Chaudron, thanks for your reply. Considerthe following scenario: > > - set a net device alias name (such as *enpxx*) into ovs. > > - |OVS_VPORT_CMD_NEW -> ||dev_get_by_name can query the net device using > *enpxx* name, but the dev->name that return is *ensxx* name. the > |net_device|struct including: ``` |struct net_device { > char name[IFNAMSIZ]; > RH_KABI_REPLACE(struct hlist_node name_hlist, > struct netdev_name_node *name_node) > ... > |``` name_hlist include enpxx and ensxx name and both of them point to the > same net_device, the net_device's name is ensxx. So when using *enpxx* name > to query net device, the ||net device's name that return is *ensxx* name.| > Then, openvswitch.ko will save the net device that name is*ensxx* to a hash > table. So this will cause the below process: > - ovs can'tobtain the enpxx net device by |OVS_VPORT_CMD_GET.| > -|dpif_netlink_port_poll will get a |notification after |OVS_VPORT_CMD_NEW|, > but the vport's name is ensxx. ovs will query the ensxx net device from > interface table, but nothing. So ovs will run the port_del operation. > - this will cause|OVS_VPORT_CMD_NEW| and OVS_VPORT_CMD_DEL operation to run > repeatedly. > So the above issue can be avoided by the patch. Thanks for the clarification, I forgot about the alias case. And you are right, OVS userspace does not support using interface aliases. Maybe you can update the commit message, and add a code comment to clarify this in your next revision? Cheers, Eelco > 在 4/15/24 18:04, Eelco Chaudron 写道: >> >> On 13 Apr 2024, at 10:48, jun.gu wrote: >> >>> Check vport name from dev_get_by_name, this can avoid to add and remove >>> NIC repeatedly when NIC rename failed at system startup. >>> >>> Signed-off-by: Jun Gu<jun...@easystack.cn> >>> --- >>> net/openvswitch/vport-netdev.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c >>> index 903537a5da22..de8977d7f329 100644 >>> --- a/net/openvswitch/vport-netdev.c >>> +++ b/net/openvswitch/vport-netdev.c >>> @@ -78,7 +78,7 @@ struct vport *ovs_netdev_link(struct vport *vport, const >>> char *name) >>> int err; >>> >>> vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name); >>> - if (!vport->dev) { >>> + if (!vport->dev) || strcmp(name, ovs_vport_name(vport)) { >> Hi Jun, not sure if I get the point here, as ovs_vport_name() translates >> into vport->dev->name. >> >> So are we trying to catch the interface rename between the >> dev_get_by_name(), and the code below? This rename could happen at any other >> place, so this check does not guarantee anything. Or am I missing something? >> >>> err = -ENODEV; >>> goto error_free_vport; >>> } >>> -- >>> 2.25.1 >>> >>> _______________________________________________ >>> dev mailing list >>> d...@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev