On 6/23/24 17:46, Andrew Lunn wrote: >>> But what about when the system is under memory pressure? You say it >>> allocates memory. What happens if those allocations fail. Does >>> changing the MTU take me from a working system to a dead system? It is >>> good practice to not kill a working system under situations like >>> memory pressure. You try to first allocate the memory you need to >>> handle the new MTU, and only if successful do you free existing memory >>> you no longer need. That means if you cannot allocate the needed >>> memory, you still have the old memory, you can keep the old MTU and >>> return -ENOMEM, and the system keeps running. >>> >> >> That's a good optimization for these kind of on-the-fly configurations but >> as you wrote before, changing an MTU value is not a hot path so out of >> cost-benefit considerations we didn't find it mandatory to optimize this >> flow. > > I would not call this an optimization. And it is not just about > changing the MTU. ethtool set_ringparam() is also likely to run into > this problem, and any other configuration which requires reallocating > the rings. > > This is something else which comes up every few months on the list, > and drivers writers who monitor the list will write their drivers that > why, not 'optimise' it later. > Actually I was wrong, we don't allocate memory in this port reset flow, we only reset the rings. But I get your point, it makes sense. >> I get your point but still it will be good if it would be documented >> somewhere IMHO. > > Kernel documentation is poor, agreed. But kernel policy is also > somewhat fluid, best practices change, and any developers can > influence that policy, different subsystems can and do have > contradictory policy, etc. The mailing list is the best place to learn > and to take part in this community. You need to be on the list for > other reasons as well. > Ok, got it. >> I'm familiar with this logic but I don't understand your point. The point >> you are making is that setting this Autoneg bit in lp_advertising is >> pointless? I see other vendors setting it too in case that autoneg was >> completed. >> Is that redundant also in their case? because it looks to me that in this >> case we followed the same logic and conventions other vendors followed. > > Please show us the output from ethtool. Does it look like the example > i showed? I must admit, i'm more from the embedded world and don't > have access to high speed interfaces. But the basic concept of > auto-neg should not change that much. > > Andrew Here is the output: $ ethtool eth0 Settings for eth0: Supported ports: [ FIBRE Backplane ] Supported link modes: 100000baseKR4/Full 100000baseSR4/Full 100000baseCR4/Full 100000baseLR4_ER4/Full Supported pause frame use: Symmetric Supports auto-negotiation: Yes Supported FEC modes: Not reported Advertised link modes: 100000baseKR4/Full 100000baseSR4/Full 100000baseCR4/Full 100000baseLR4_ER4/Full Advertised pause frame use: Symmetric Advertised auto-negotiation: Yes Advertised FEC modes: Not reported Link partner advertised link modes: Not reported Link partner advertised pause frame use: No Link partner advertised auto-negotiation: Yes Link partner advertised FEC modes: Not reported Speed: 100000Mb/s Duplex: Full Auto-negotiation: on There are few points to mention: 1. We don't allow to modify the advertised link modes so by definition the advertised ones are a copy of the supported ones. 2. Reading the peer advertised link modes is not supported so we don't report them (similarly to some other vendors). 3. Our speed is fixed and also cannot be changed so we don't mask lp_advertising with advertising to pick the highest speed. We aim for a specific speed and hence it's binary - or we'll have a link with that specific speed or we won't have a link at all. 4. If we support autoneg and it was completed, we can conclude that also our peer supports autoneg and hence we report that.
