On Wed, 22 Jan 2025 09:37:20 +0800 "Huang, Ying" <ying.huang@xxxxxxxxxxxxxxxxx> wrote: Hi Gregory and Ying, thank you both for your insights! [...snip...] > >> I still prefer to use 2 iw_table, one is for default, the other is for > >> manual. The default one will be used if the manual one is NULL. Both > >> are protected by RCU. The default one can be updated upon hotplug > >> blindly. This makes the whole model easier to be understood IMHO. > > `cat auto node0 node1` -> `true 5 1` > > and you do > > echo 0 > auto I think that when initially developing this patch, this was the intent that I had as well (in the v1 of this RFC patch, there was an iw_table and a separate default_iw_table). However, I think that the ideas of having a "default" and "manual" table made less sense over time, given that they behaved more like a "default" and "visible" table instead. That is, the visible layer is directly manipulable by the user, but does not necessarily only contain manually-set values; rather, most of the time, it probably still has a lot of auto-generated weights. I think that this analysis runs the risk of being a bit too semantically nit-picky, but as I'll explain below, I think both the 1-layer approach that I implemented in this RFC and the expected 2-layer behavior that you outline below are essentially the same, functionally. In other words, I think we agree on what the expected behavior should be : -) We just have to agree on what presentation would make the most sense for the user. > > what should a subsequent `cat auto node0 node1` output? > > > > `false 5 1` > > or > > `false 1 1` > > IMO, it should be > > `false 5 1` > > That is, we copy auto-generated weights to manual weights atomically and > use it. > > > Then lets say we do > > echo 7 > node0 > > Now, `cat auto node0 node1` outputs, > > `false 7 1` > > That is, we delete manual weights and use the auto-generated ones. > > > what should > > echo true > auto > > result in? > > > > `true 5 1` > > or > > `true 7 1` > > It should be > > `true 5 1` I see, so I think we actually agree on what the behavior for this is. Then there is no real "hidden state", it's either just using the default weights, or turning that off and being able to edit the states. > > The current code makes sure that when you switch modes from auto > > to manual, it inherits the current state - instead of there being > > some hidden state that suddenly takes precedence. > > I think that we can do that with two weight arrays. > > > So I prefer to just have one IW array and no hidden state. > > Then, when we switch from manual to auto mode, where to find > auto-generated weights? Re-calculate them? Even in manual mode, incoming bandwidth data is continuously stored. This way, when a user does decide to switch back to auto mode later, the system doesn't have to retrieve the bandwidth data all over again. As for the auto-generated weights, they are re-calculated based solely on the bandwidth data available. (I will note that re-calculating the weights are very quick, see reduce_interleave_weights) Based on your description of the expected behavior, everything you listed out is actually what currently happens in the one-layer system. Switching from auto --> manual inherits the auto-generated weights, and switching from manual --> auto wipes all previous user-stored data. At this point, I think that I am happy with either option. I wrote and re-wrote this a bunch of times, and came to the conclusion that now that we agree on the behavior of the interface, I have no strong opinion on whether we have a "hidden" default layer or a phantom default layer that is just generated every time a user needs it : -) Please let me know if I missed anything as well! Thank you all for your continued feedback and interest! Have a great day, Joshua
