net/ethtool: add netlink interface for the PLCA RS

Hello Piergiorgio Beruto,

The patch 8580e16c28f3: "net/ethtool: add netlink interface for the PLCA RS" from Jan 9, 2023, leads to the following Smatch static checker warning:

	net/ethtool/plca.c:155 ethnl_set_plca_cfg()
	info: return a literal instead of 'ret'

net/ethtool/plca.c

    140 int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info)
    141 {
    142         struct ethnl_req_info req_info = {};
    143         struct nlattr **tb = info->attrs;
    144         const struct ethtool_phy_ops *ops;
    145         struct phy_plca_cfg plca_cfg;
    146         struct net_device *dev;
    147         bool mod = false;
    148         int ret;
    149 
    150         ret = ethnl_parse_header_dev_get(&req_info,
    151                                          tb[ETHTOOL_A_PLCA_HEADER],
    152                                          genl_info_net(info), info->extack,
    153                                          true);
    154         if (!ret)

--> 155 return ret;

This looks like the if statement is reversed. Otherwise if this is a short cut to success, please write it like so:

	if (!ret)
		return 0;

There are a bunch of if (!ret) if statements in this function but the rest look intentional in context. It's pretty common to reverse the last if statement. (Not a fan, myself though).

    156 
    157         dev = req_info.dev;
    158 
    159         rtnl_lock();
    160 
    161         // check that the PHY device is available and connected
    162         if (!dev->phydev) {
    163                 ret = -EOPNOTSUPP;
    164                 goto out_rtnl;
    165         }
    166 
    167         ops = ethtool_phy_ops;
    168         if (!ops || !ops->set_plca_cfg) {
    169                 ret = -EOPNOTSUPP;
    170                 goto out_rtnl;
    171         }
    172 
    173         ret = ethnl_ops_begin(dev);
    174         if (!ret)
    175                 goto out_rtnl;
    176 
    177         memset(&plca_cfg, 0xff, sizeof(plca_cfg));
    178         plca_update_sint(&plca_cfg.enabled, tb[ETHTOOL_A_PLCA_ENABLED], &mod);
    179         plca_update_sint(&plca_cfg.node_id, tb[ETHTOOL_A_PLCA_NODE_ID], &mod);
    180         plca_update_sint(&plca_cfg.node_cnt, tb[ETHTOOL_A_PLCA_NODE_CNT], &mod);
    181         plca_update_sint(&plca_cfg.to_tmr, tb[ETHTOOL_A_PLCA_TO_TMR], &mod);
    182         plca_update_sint(&plca_cfg.burst_cnt, tb[ETHTOOL_A_PLCA_BURST_CNT],
    183                          &mod);
    184         plca_update_sint(&plca_cfg.burst_tmr, tb[ETHTOOL_A_PLCA_BURST_TMR],
    185                          &mod);
    186 
    187         ret = 0;
    188         if (!mod)
    189                 goto out_ops;
    190 
    191         ret = ops->set_plca_cfg(dev->phydev, &plca_cfg, info->extack);
    192         if (!ret)
    193                 goto out_ops;
    194 
    195         ethtool_notify(dev, ETHTOOL_MSG_PLCA_NTF, NULL);
    196 
    197 out_ops:
    198         ethnl_ops_complete(dev);
    199 out_rtnl:
    200         rtnl_unlock();
    201         ethnl_parse_header_dev_put(&req_info);
    202 
    203         return ret;
    204 }

regards,
dan carpenter

Hello Dan,
thank you very much for your review.

I have already submitted a patch to fix that. PLease, see
https://patchwork.kernel.org/project/netdevbpf/patch/f2277af8951a51cfee2fb905af8d7a812b7beaf4.1673616357.git.piergiorgio.beruto@gmail.com/

Thanks,
Piergiorgio