Skip to content

Commit

Permalink
net: ethtool: fix ioctl confusing drivers about desired HDS user config
Browse files Browse the repository at this point in the history
The legacy ioctl path does not have support for extended attributes.
So we issue a GET to fetch the current settings from the driver,
in an attempt to keep them unchanged. HDS is a bit "special" as
the GET only returns on/off while the SET takes a "ternary" argument
(on/off/default). If the driver was in the "default" setting -
executing the ioctl path binds it to on or off, even tho the user
did not intend to change HDS config.

Factor the relevant logic out of the netlink code and reuse it.

Fixes: 87c8f84 ("bnxt_en: add support for tcp-data-split ethtool command")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Tested-by: Daniel Xu <dxu@dxuuu.xyz>
Tested-by: Taehee Yoo <ap420073@gmail.com>
Link: https://patch.msgid.link/20250221025141.1132944-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Feb 24, 2025
1 parent f15176b commit db10fde
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 7 deletions.
16 changes: 16 additions & 0 deletions net/ethtool/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <linux/rtnetlink.h>
#include <linux/ptp_clock_kernel.h>
#include <linux/phy_link_topology.h>
#include <net/netdev_queues.h>

#include "netlink.h"
#include "common.h"
Expand Down Expand Up @@ -771,6 +772,21 @@ int ethtool_check_ops(const struct ethtool_ops *ops)
return 0;
}

void ethtool_ringparam_get_cfg(struct net_device *dev,
struct ethtool_ringparam *param,
struct kernel_ethtool_ringparam *kparam,
struct netlink_ext_ack *extack)
{
memset(param, 0, sizeof(*param));
memset(kparam, 0, sizeof(*kparam));

param->cmd = ETHTOOL_GRINGPARAM;
dev->ethtool_ops->get_ringparam(dev, param, kparam, extack);

/* Driver gives us current state, we want to return current config */
kparam->tcp_data_split = dev->cfg->hds_config;
}

static void ethtool_init_tsinfo(struct kernel_ethtool_ts_info *info)
{
memset(info, 0, sizeof(*info));
Expand Down
6 changes: 6 additions & 0 deletions net/ethtool/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ int ethtool_check_max_channel(struct net_device *dev,
struct ethtool_channels channels,
struct genl_info *info);
int ethtool_check_rss_ctx_busy(struct net_device *dev, u32 rss_context);

void ethtool_ringparam_get_cfg(struct net_device *dev,
struct ethtool_ringparam *param,
struct kernel_ethtool_ringparam *kparam,
struct netlink_ext_ack *extack);

int __ethtool_get_ts_info(struct net_device *dev, struct kernel_ethtool_ts_info *info);
int ethtool_get_ts_info_by_phc(struct net_device *dev,
struct kernel_ethtool_ts_info *info,
Expand Down
4 changes: 2 additions & 2 deletions net/ethtool/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2059,8 +2059,8 @@ static int ethtool_get_ringparam(struct net_device *dev, void __user *useraddr)

static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr)
{
struct ethtool_ringparam ringparam, max = { .cmd = ETHTOOL_GRINGPARAM };
struct kernel_ethtool_ringparam kernel_ringparam;
struct ethtool_ringparam ringparam, max;
int ret;

if (!dev->ethtool_ops->set_ringparam || !dev->ethtool_ops->get_ringparam)
Expand All @@ -2069,7 +2069,7 @@ static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr)
if (copy_from_user(&ringparam, useraddr, sizeof(ringparam)))
return -EFAULT;

dev->ethtool_ops->get_ringparam(dev, &max, &kernel_ringparam, NULL);
ethtool_ringparam_get_cfg(dev, &max, &kernel_ringparam, NULL);

/* ensure new ring parameters are within the maximums */
if (ringparam.rx_pending > max.rx_max_pending ||
Expand Down
9 changes: 4 additions & 5 deletions net/ethtool/rings.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,16 @@ ethnl_set_rings_validate(struct ethnl_req_info *req_info,
static int
ethnl_set_rings(struct ethnl_req_info *req_info, struct genl_info *info)
{
struct kernel_ethtool_ringparam kernel_ringparam = {};
struct ethtool_ringparam ringparam = {};
struct kernel_ethtool_ringparam kernel_ringparam;
struct net_device *dev = req_info->dev;
struct ethtool_ringparam ringparam;
struct nlattr **tb = info->attrs;
const struct nlattr *err_attr;
bool mod = false;
int ret;

dev->ethtool_ops->get_ringparam(dev, &ringparam,
&kernel_ringparam, info->extack);
kernel_ringparam.tcp_data_split = dev->cfg->hds_config;
ethtool_ringparam_get_cfg(dev, &ringparam, &kernel_ringparam,
info->extack);

ethnl_update_u32(&ringparam.rx_pending, tb[ETHTOOL_A_RINGS_RX], &mod);
ethnl_update_u32(&ringparam.rx_mini_pending,
Expand Down

0 comments on commit db10fde

Please sign in to comment.