For clone creating or renaming operation, the interface name get back can be different.
PR: 270618
MFC after: 3 days
Differential D39396
ifconfig: Use the interface name we get back when configuring if_bridge with additional operating parameters zlei on Apr 3 2023, 9:20 AM. Authored by Tags None Referenced Files
Subscribers
Details
For clone creating or renaming operation, the interface name get back can be different. PR: 270618 # ifconfig epair create epair0a # ifconfig bridge create addm epair0a bridge0 # ifconfig bridge0 name br0 deletem epair0a br0
Diff Detail
Event TimelineComment Actions Tangential question: Should we add some atf-sh tests with the test plan, even if simplistic, for this and some of your previous ifconfig(8) improvements? If you think it's worth the trouble, I'll try to submit them. Comment Actions I do not have strong options for that.
It may not worth a test if you meant D39282. For D39282 the kernel still check VID passed into, and it is mainly to provide a good human-readable error as @melifaro commented.
@kp What do you think? Comment Actions I tend to think it's always worth having a test. It's surprising just how often even trivial tests find real bugs. Having the tests also makes future refactoring that much easier. The tests currently don't take all that much time, nor will a test case for this add a lot of runtime. (And if we get to the point where we have so many tests their run time becomes an issue I shall be glad to have that problem.) Comment Actions I'm not quite sure why ifr.ifr_name doesn't work here though. I'd expect ifclonecreate() to create the bridge, and the clone call there should have updated the name in ifr.ifr_name. After all, we copy ifr.ifr_name to name there. What am I missing? Comment Actions I couldn't agree more. Recently I found&fixed the old bug triggered by the netlink code which manifested from a simple if_stf test. Comment Actions lldb shows that ifr.ifr_name (this is the global one) is not updated on ifclonecreate().
That ifr is a local var. static void ifclonecreate(int s, void *arg) { struct ifreq ifr; struct clone_defcb *dcp; memset(&ifr, 0, sizeof(ifr)); (void) strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name)); ... /* * If we get a different name back than we put in, update record and * indicate it should be printed later. */ if (strncmp(name, ifr.ifr_name, sizeof(name)) != 0) { strlcpy(name, ifr.ifr_name, sizeof(name)); printifname = 1; } } Comment Actions Ahh, yes, that'd do it. One of these days someone very, very brave (braver than me, is what I'm hinting at here) is going to have to clean up ifconfig. |