Re: [RFC PATCH net-next v3 15/21] ethtool: provide link settings and link modes in GET_SETTINGS request
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2019-02-21 03:15:00
Also in:
lkml
On 2/18/2019 10:22 AM, Michal Kubecek wrote:
Implement GET_SETTINGS netlink request to get link settings and link mode information provided by ETHTOOL_GLINKSETTINGS ioctl command. The information is divided into two parts: supported, advertised and peer advertised link modes when ETH_SETTINGS_IM_LINKMODES flag is set in the request and other settings when ETH_SETTINGS_IM_LINKINFO is set. Signed-off-by: Michal Kubecek <redacted> ---
[snip]
+#define ETH_SETTINGS_IM_LINKINFO 0x01 +#define ETH_SETTINGS_IM_LINKMODES 0x02 + +#define ETH_SETTINGS_IM_ALL 0x03
You could define ETH_SETTINGS_IM_ALL as: #define ETH_SETTING_IM_ALL \ (ETH_SETTINGS_IM_LINKINFO | ETH_SETTINGS_IM_LINMODES) that would scale better IMHO, especially given that you have to keep bumping that mask with new bits in subsequent patches. [snip]
+ if (tb[ETHA_SETTINGS_INFOMASK]) + req_info->req_mask = nla_get_u32(tb[ETHA_SETTINGS_INFOMASK]); + if (tb[ETHA_SETTINGS_COMPACT]) + req_info->compact = true; + if (req_info->req_mask == 0) + req_info->req_mask = ETH_SETTINGS_IM_ALL;
What if userland is newer than the kernel and specifies a req_mask with bits set that you don't support? Should not you always do an & ETH_SETTINGS_IM_ALL here? [snip] -- Florian