Re: [PATCH net-next v2 1/8] ethtool: Add support for configuring frame preemption
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Date: 2021-01-19 23:05:37
Also in:
intel-wired-lan
Hi Vladimir, Vladimir Oltean [off-list ref] writes:
On Mon, Jan 18, 2021 at 04:40:21PM -0800, Vinicius Costa Gomes wrote:quoted
+int ethnl_set_preempt(struct sk_buff *skb, struct genl_info *info) +{ + struct ethnl_req_info req_info = {}; + struct nlattr **tb = info->attrs; + struct ethtool_fp preempt = {}; + struct net_device *dev; + bool mod = false; + int ret; + + ret = ethnl_parse_header_dev_get(&req_info, + tb[ETHTOOL_A_PREEMPT_HEADER], + genl_info_net(info), info->extack, + true); + if (ret < 0) + return ret; + dev = req_info.dev; + ret = -EOPNOTSUPP; + if (!dev->ethtool_ops->get_preempt || + !dev->ethtool_ops->set_preempt) + goto out_dev; + + rtnl_lock();I'm a bit of a noob when it comes to ethtool (netlink or otherwise). Why do you take the rtnl_mutex when updating some purely hardware values, what netdev state is there to protect? Can this get->modify->set sequence be serialized using other locking mechanism than rtnl_mutex?
From what I understand, configuration changes to netdevice should be protected by rtnl_mutex, for example, to avoid the device disappearing while the configuration is in progress. I don't think there's any other finer grained lock that can be used here. Cheers, -- Vinicius