Hey! > > I completely understand why there may be concerns at editing such critical > code for a use case that is not very common. > > Unfortunately my current situation requires renaming the interfaces. Multiple > modems may take turns being 'selected' (having the interface renamed) to pass > traffic. I hope to remove that need one day, but until then I will need this > patch. >
So you have a modem that is in connected state but isn't selected to pass traffic, and when it gets selected, the net interface is renamed to some other name? I think it is the first time I've seen something like this :) Is it because some other app uses a hardcoded net interface name somewhere or something like that? > Even if this is an uncommon problem, I still think this change (or at least > my approach) could be useful to pull into ModemManager mainline. Yes, people > don't rename interfaces often, but it is something that Linux/udev support, > these events effectively breaks the Modem object lifecycle in ModemManager > (which makes ModemManager look buggy), the log messages that are printed out > do not make it clear that renaming the interface caused it (it took my a long > time to figure it out), and I have not seen any piece of documentation > warning that renaming any interface managed by ModemManager is unsupported. > Technically it is completely possible, but my worry is that it may introduce an amount of complexity that is not worth the effort. Updating the documentation to say that MM currently does not support interface renaming once the device has been initially probed would be quicker... and maybe we should do that right away even before thinking about supporting your usecase. > You do bring up an interesting point I had missed: what if someone renamed a > control interface? My code currently would not catch that, but maybe it > should! > Renaming the control interface would be overkill. Renaming the net port may be somewhat easier. > ModemManager handles tons of events and states for modems and all the weird > things they can do. If a user could do something with the ModemManager API > that caused strange errors like I described, the issue would be fixed in the > driver/etc. > > Linux Interfaces can be renamed using the Linux API, and while they are > renamed, ModemManager is in a precarious state until the interface is changed > back to its original name. Removing or resetting the modem in that precarious > state will cause ModemManager to act strangely for that modem until it is > removed again with the original interface name (which could be a problem if > another modem was added while the interface was renamed). And linux+udev > gives us all the tools to handle these known and supported events. I > personally think these are enough reasons to address this behavior. > > I see this as an issue of stability, reliability, and resiliency. Even if > this behavior is rarely encountered, I believe there are substantial reasons > to fix it. That does not mean that my code is the best way to do it. If you > end up agreeing that this behavior should be fixed, I will make revisions to > address your concerns. > MM also has a lot of limitations, and one of the limitations that I always warn about is that the state of the modem should not be touched out of MM's own APIs. E.g. if you run QMI commands that modify the state of the modem using qmicli, MM may not pick them up. If the state needs to be updated, it should be done using MM's own APIs. I think we can say the same about this problem, you're trying to modify the state of how the modem is exposed in the kernel out of MM's eyes, and expect MM to pick that up. Technically both things could be possible, but it may really be too much complexity introduced for little gain, especially when the issues can be avoided in other ways :/ > To the complexity of my code, I tried to made it pretty straight forward and > self contained. When a rename event happens, find the Modem object associated > with the old device name, remove the old name from the modem, and add the new > name. I could make the replacement logic into a function to keep the code > more readable if you would prefer. > Aren't we fully re-probing the modem from scratch upon the net interface rename? We could try to do that as an initial step, in the same way as we do the forced reprobing when e.g. the proxy dies or when the SIM card is switched. The reprobing logic will make MM re-detect the modem object with the new state. > You mention opening the door to more complex interactions, and that is > interesting. If I don't correctly handle the udev events, then I am not > really fixing anything! > > I just looked up all of the udev event types: add, remove, change, move, > online, offline, bind, and unbind. > > ModemManager already has support for add, remove, change (kind of, more on > that later), and with my patch move (I would argue that the upstream version > of the move handler was incorrect). > > The online and offline events are supposedly for things like a network > interface coming up or going down, so I do not think these events will be > relevant to ModemManager at all. The bind and unbind events are for when > drivers are attached or detached from the device, which I also believe is > useless to ModemManager and could not affect anything with the Modem > lifecycle. > "bind" is required by the udev rules to ensure the tags are re-set on that event. > But the change event is interesting. I am getting contradicting descriptions > from different sources about change vs move. Perhaps you are right that this > could cause similar issues. I will probably need to rework this to properly > handle both in case change can be triggered in a rename, but either way, more > research is required. > > I may also be double handling move, which could be handled more gracefully. > > I will try to fix these up in the coming weeks or months as time allows. I > welcome feedback and ideas even before I finish the patch. > I don't want to discourage patching MM, but I'd suggest you first try to approach the issue looking at forcing a full re-probe of the device. Although if I'm thinking about it right, this would mean the modem would get completely disconnected during the net interface rename, which is something you may not want... -- Aleksander
