Re: [net-next-2.6 PATCH 1/8] e1000e: cleanup ethtool loopback setup code
From: David Miller <davem@davemloft.net>
Date: 2010-06-30 23:06:04
From: "Allan, Bruce W" <redacted> Date: Wed, 30 Jun 2010 15:41:19 -0700
I've been looking into your request number 2 above (as a reminder, it had to do with a patch I submitted that added a module parameter to e1000e in order to enable/disable Energy Efficient Ethernet for a particular type of adapter). For this new ethtool feature bit/flag for EEE, would you prefer it be set via: 1) the generic parameter setting option (e.g. -s ethX [eee on|off]), 2) yet another new show/change option pair, or 3) a new option that can set this new feature and be expandable to future features that are likewise not related to existing ethtool options (e.g. -F [eee on|off] [whizbang on|off])? For #2 or #3, it makes sense to use ethtool_op_[g|s]et_flags with new ETH_FLAG_<feature> and NETIF_F_<feature> defines, but #1 can be implemented that way or by using remaining reserved elements of struct ethtool_cmd - if your preference is for #1, would you prefer it be implemented with the former or latter?
I only have strong feelings about the kernel side, and an ETH_FLAG_* seems best for this since other devices will have this feature too. I don't think overloading parts of ethtool_cmd is wise.