Messages in this thread | | | Subject | Re: [RFC PATCH net-next v3 15/21] ethtool: provide link settings and link modes in GET_SETTINGS request | From | Florian Fainelli <> | Date | Wed, 20 Feb 2019 19:14:50 -0800 |
| |
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 <mkubecek@suse.cz> > ---
[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
| |