On 2/24/2019 9:40 PM, Andrew Lunn wrote: >>> This is getting less friendly all the time, and it was never very >>> friendly to start with. We have the strings which represent these link >>> modes in the table used for dumping caps. How about allowing the user >>> to list a comma separate list of modes. >>> >>> ethtool -s lan42 advertise >>> 100000baseKR2/Full,100000baseSR2/Full,100000baseCR2/Full >> >> In my preliminary netlink patchset, I'm adding support for e.g. >> >> ethtool -s eth0 advertise 100baseT/Half off 1000baseT/Full on >> >> I'm not sure what would be more useful, if switching individual modes or >> listing the whole set. After all, we can also support both. But I fully >> agree that the numeric bitmaps are already too inconvenient. > > Hi Michal > > So are you doing a read/modify/write? In that case, off/on makes > sense. For a pure write, i don't see the need for off/on. > > I've not had to use this much, so i don't know how it is typically > used. When i have used it, it is because i've got an SFP which can do > 1G and 2.5G, but the peer can only do 1G. I've needed to remove the > 2.5G in order to get link. So in that case, read/modify/write with an > off would make sense. > > Andrew > >> And there is another problem: >> >> + else if (speed_wanted == SPEED_20000 && >> + duplex_wanted == DUPLEX_FULL) >> + adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT & >> + ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT & >> + >> ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT & >> + ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT & >> + ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT; >> >> The test is for SPEED_20000 but then 200G modes are added. > > Oh, yes. Easy to miss. Maybe we should consider adding aliases, > #define SPEED_200G SPEED_200000, and
Totally agree. This will help. > > #define ETHTOOL_LINK_MODE_200GbaseCR4_Full_BIT > ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT In case you do it please consider adding an underscore after the speed, so it becomes ETHTOOL_LINK_MODE_200G_baseCR4_Full_BIT. I think it's more convenient for the human eye.
