On Fri, May 01, 2020 at 05:52:10PM +0200, Michal Kubecek wrote:
> On Fri, May 01, 2020 at 09:46:32AM +0200, Oleksij Rempel wrote:
> [...]
> > static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr
> > **tb,
> > struct ethtool_link_ksettings *ksettings,
> > bool *mod)
> > {
> > struct ethtool_link_settings *lsettings = &ksettings->base;
> > bool req_speed, req_duplex;
> > + const struct nlattr *attr;
> > int ret;
> >
> > + attr = tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG];
> > + if (attr) {
>
> Introducing the variable makes little sense if this is the only place
> where it is used. But if you decide to use it also in the two other
> places working with the attribute, it should probably have more
> descriptive name.
>
> Michal
>
> > + u8 cfg = nla_get_u8(tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]);
> > + if (!ethnl_validate_master_slave_cfg(cfg))
> > + return -EOPNOTSUPP;
> > + }
Also, please set extack error message and bad attribute when the check
fails.
Michal
> > +
> > *mod = false;
> > req_speed = tb[ETHTOOL_A_LINKMODES_SPEED];
> > req_duplex = tb[ETHTOOL_A_LINKMODES_DUPLEX];
> > @@ -311,6 +357,8 @@ static int ethnl_update_linkmodes(struct genl_info
> > *info, struct nlattr **tb,
> > mod);
> > ethnl_update_u8(&lsettings->duplex, tb[ETHTOOL_A_LINKMODES_DUPLEX],
> > mod);
> > + ethnl_update_u8(&lsettings->master_slave_cfg,
> > + tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG], mod);
> >
> > if (!tb[ETHTOOL_A_LINKMODES_OURS] && lsettings->autoneg &&
> > (req_speed || req_duplex) &&
> > --
> > 2.26.2
> >
