From: Martin Habets <habetsm.xilinx@gmail.com>
To: edward.cree@amd.com
Cc: linux-net-drivers@amd.com, davem@davemloft.net, kuba@kernel.org,
edumazet@google.com, pabeni@redhat.com,
Edward Cree <ecree.xilinx@gmail.com>,
netdev@vger.kernel.org, sudheer.mogilappagari@intel.com,
jdamato@fastly.com, andrew@lunn.ch, mw@semihalf.com,
linux@armlinux.org.uk, sgoutham@marvell.com, gakula@marvell.com,
sbhatta@marvell.com, hkelam@marvell.com, saeedm@nvidia.com,
leon@kernel.org, hkallweit1@gmail.com, nic_swsd@realtek.com,
jiawenwu@trustnetic.com, mengyuanlou@net-swift.com
Subject: Re: [PATCH v4 net-next 1/7] net: move ethtool-related netdev state into its own struct
Date: Mon, 2 Oct 2023 10:59:46 +0100 [thread overview]
Message-ID: <20231002095946.GA21694@gmail.com> (raw)
In-Reply-To: <31cdf199dad8e26bd5f732fd04b0d640c41f5616.1695838185.git.ecree.xilinx@gmail.com>
On Wed, Sep 27, 2023 at 07:13:32PM +0100, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> net_dev->ethtool is a pointer to new struct ethtool_netdev_state, which
> currently contains only the wol_enabled field.
>
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
One small nit below.
Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com>
> ---
> drivers/net/ethernet/realtek/r8169_main.c | 4 ++--
> drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c | 4 ++--
> drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 2 +-
> drivers/net/phy/phy.c | 2 +-
> drivers/net/phy/phy_device.c | 5 +++--
> drivers/net/phy/phylink.c | 2 +-
> include/linux/ethtool.h | 8 ++++++++
> include/linux/netdevice.h | 7 ++++---
> net/core/dev.c | 4 ++++
> net/ethtool/ioctl.c | 2 +-
> net/ethtool/wol.c | 2 +-
> 11 files changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 6351a2dc13bc..fe69416f2a93 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -1455,7 +1455,7 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
>
> if (tp->dash_type == RTL_DASH_NONE) {
> rtl_set_d3_pll_down(tp, !wolopts);
> - tp->dev->wol_enabled = wolopts ? 1 : 0;
> + tp->dev->ethtool->wol_enabled = wolopts ? 1 : 0;
> }
> }
>
> @@ -5321,7 +5321,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> rtl_set_d3_pll_down(tp, true);
> } else {
> rtl_set_d3_pll_down(tp, false);
> - dev->wol_enabled = 1;
> + dev->ethtool->wol_enabled = 1;
> }
>
> jumbo_max = rtl_jumbo_max(tp);
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c
> index ec0e869e9aac..091ee3d3e74d 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c
> @@ -34,9 +34,9 @@ static int ngbe_set_wol(struct net_device *netdev,
> wx->wol = 0;
> if (wol->wolopts & WAKE_MAGIC)
> wx->wol = WX_PSR_WKUP_CTL_MAG;
> - netdev->wol_enabled = !!(wx->wol);
> + netdev->ethtool->wol_enabled = !!(wx->wol);
> wr32(wx, WX_PSR_WKUP_CTL, wx->wol);
> - device_set_wakeup_enable(&pdev->dev, netdev->wol_enabled);
> + device_set_wakeup_enable(&pdev->dev, netdev->ethtool->wol_enabled);
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> index 2b431db6085a..6752f8d04d9c 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> @@ -636,7 +636,7 @@ static int ngbe_probe(struct pci_dev *pdev,
> if (wx->wol_hw_supported)
> wx->wol = NGBE_PSR_WKUP_CTL_MAG;
>
> - netdev->wol_enabled = !!(wx->wol);
> + netdev->ethtool->wol_enabled = !!(wx->wol);
> wr32(wx, NGBE_PSR_WKUP_CTL, wx->wol);
> device_set_wakeup_enable(&pdev->dev, wx->wol);
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index a5fa077650e8..32bfea9b5c6c 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1282,7 +1282,7 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
> if (netdev) {
> struct device *parent = netdev->dev.parent;
>
> - if (netdev->wol_enabled)
> + if (netdev->ethtool->wol_enabled)
> pm_system_wakeup();
> else if (device_may_wakeup(&netdev->dev))
> pm_wakeup_dev_event(&netdev->dev, 0, true);
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 2ce74593d6e4..62afc0424fbd 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -285,7 +285,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
> if (!netdev)
> goto out;
>
> - if (netdev->wol_enabled)
> + if (netdev->ethtool->wol_enabled)
> return false;
>
> /* As long as not all affected network drivers support the
> @@ -1859,7 +1859,8 @@ int phy_suspend(struct phy_device *phydev)
> return 0;
>
> phy_ethtool_get_wol(phydev, &wol);
> - phydev->wol_enabled = wol.wolopts || (netdev && netdev->wol_enabled);
> + phydev->wol_enabled = wol.wolopts ||
> + (netdev && netdev->ethtool->wol_enabled);
> /* If the device has WOL enabled, we cannot suspend the PHY */
> if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
> return -EBUSY;
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 0d7354955d62..b808ba1197c3 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -2172,7 +2172,7 @@ void phylink_suspend(struct phylink *pl, bool mac_wol)
> {
> ASSERT_RTNL();
>
> - if (mac_wol && (!pl->netdev || pl->netdev->wol_enabled)) {
> + if (mac_wol && (!pl->netdev || pl->netdev->ethtool->wol_enabled)) {
> /* Wake-on-Lan enabled, MAC handling */
> mutex_lock(&pl->state_mutex);
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 62b61527bcc4..8aeefc0b4e10 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -935,6 +935,14 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
> const struct ethtool_link_ksettings *cmd,
> u32 *dev_speed, u8 *dev_duplex);
>
> +/**
> + * struct ethtool_netdev_state - per-netdevice state for ethtool features
> + * @wol_enabled: Wake-on-LAN is enabled
> + */
> +struct ethtool_netdev_state {
> + unsigned wol_enabled:1;
The use of bool seems to be quite well established in the kernel these
days. I suggest you use that.
Martin
> +};
> +
> struct phy_device;
> struct phy_tdr_config;
> struct phy_plca_cfg;
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 7e520c14eb8c..05ea6cb56800 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -79,6 +79,7 @@ struct xdp_buff;
> struct xdp_frame;
> struct xdp_metadata_ops;
> struct xdp_md;
> +struct ethtool_netdev_state;
> /* DPLL specific */
> struct dpll_pin;
>
> @@ -2014,8 +2015,6 @@ enum netdev_ml_priv_type {
> * switch driver and used to set the phys state of the
> * switch port.
> *
> - * @wol_enabled: Wake-on-LAN is enabled
> - *
> * @threaded: napi threaded mode is enabled
> *
> * @net_notifier_list: List of per-net netdev notifier block
> @@ -2027,6 +2026,7 @@ enum netdev_ml_priv_type {
> * @udp_tunnel_nic_info: static structure describing the UDP tunnel
> * offload capabilities of the device
> * @udp_tunnel_nic: UDP tunnel offload state
> + * @ethtool: ethtool related state
> * @xdp_state: stores info on attached XDP BPF programs
> *
> * @nested_level: Used as a parameter of spin_lock_nested() of
> @@ -2389,7 +2389,6 @@ struct net_device {
> struct sfp_bus *sfp_bus;
> struct lock_class_key *qdisc_tx_busylock;
> bool proto_down;
> - unsigned wol_enabled:1;
> unsigned threaded:1;
>
> struct list_head net_notifier_list;
> @@ -2401,6 +2400,8 @@ struct net_device {
> const struct udp_tunnel_nic_info *udp_tunnel_nic_info;
> struct udp_tunnel_nic *udp_tunnel_nic;
>
> + struct ethtool_netdev_state *ethtool;
> +
> /* protected by rtnl_lock */
> struct bpf_xdp_entity xdp_state[__MAX_XDP_MODE];
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 606a366cc209..9e85a71e33ed 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10769,6 +10769,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> dev->real_num_rx_queues = rxqs;
> if (netif_alloc_rx_queues(dev))
> goto free_all;
> + dev->ethtool = kzalloc(sizeof(*dev->ethtool), GFP_KERNEL_ACCOUNT);
> + if (!dev->ethtool)
> + goto free_all;
>
> strcpy(dev->name, name);
> dev->name_assign_type = name_assign_type;
> @@ -10819,6 +10822,7 @@ void free_netdev(struct net_device *dev)
> return;
> }
>
> + kfree(dev->ethtool);
> netif_free_tx_queues(dev);
> netif_free_rx_queues(dev);
>
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 0b0ce4f81c01..de78b24fffc9 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -1461,7 +1461,7 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr)
> if (ret)
> return ret;
>
> - dev->wol_enabled = !!wol.wolopts;
> + dev->ethtool->wol_enabled = !!wol.wolopts;
> ethtool_notify(dev, ETHTOOL_MSG_WOL_NTF, NULL);
>
> return 0;
> diff --git a/net/ethtool/wol.c b/net/ethtool/wol.c
> index 0ed56c9ac1bc..a39d8000d808 100644
> --- a/net/ethtool/wol.c
> +++ b/net/ethtool/wol.c
> @@ -137,7 +137,7 @@ ethnl_set_wol(struct ethnl_req_info *req_info, struct genl_info *info)
> ret = dev->ethtool_ops->set_wol(dev, &wol);
> if (ret)
> return ret;
> - dev->wol_enabled = !!wol.wolopts;
> + dev->ethtool->wol_enabled = !!wol.wolopts;
> return 1;
> }
>
next prev parent reply other threads:[~2023-10-02 9:59 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-27 18:13 [PATCH v4 net-next 0/7] ethtool: track custom RSS contexts in the core edward.cree
2023-09-27 18:13 ` [PATCH v4 net-next 1/7] net: move ethtool-related netdev state into its own struct edward.cree
2023-09-29 18:15 ` Jacob Keller
2023-10-02 9:59 ` Martin Habets [this message]
2023-09-27 18:13 ` [PATCH v4 net-next 2/7] net: ethtool: attach an XArray of custom RSS contexts to a netdevice edward.cree
2023-09-29 18:17 ` Jacob Keller
2023-10-04 22:56 ` Jakub Kicinski
2023-09-29 20:59 ` Mogilappagari, Sudheer
2023-10-02 10:23 ` Martin Habets
2023-10-04 23:00 ` Jakub Kicinski
2023-10-05 18:32 ` Edward Cree
2023-10-04 23:10 ` Jakub Kicinski
2023-10-05 18:43 ` Edward Cree
2023-10-05 23:53 ` Jakub Kicinski
2023-09-27 18:13 ` [PATCH v4 net-next 3/7] net: ethtool: record custom RSS contexts in the XArray edward.cree
2023-09-29 18:20 ` Jacob Keller
2023-10-02 10:41 ` Martin Habets
2023-09-27 18:13 ` [PATCH v4 net-next 4/7] net: ethtool: let the core choose RSS context IDs edward.cree
2023-09-29 18:23 ` Jacob Keller
2023-10-02 10:54 ` Martin Habets
2023-09-27 18:13 ` [PATCH v4 net-next 5/7] net: ethtool: add an extack parameter to new rxfh_context APIs edward.cree
2023-09-29 18:24 ` Jacob Keller
2023-10-02 12:13 ` Martin Habets
2023-09-27 18:13 ` [PATCH v4 net-next 6/7] net: ethtool: add a mutex protecting RSS contexts edward.cree
2023-09-29 18:27 ` Jacob Keller
2023-10-02 12:16 ` Martin Habets
2023-10-04 23:16 ` Jakub Kicinski
2023-10-05 20:56 ` Edward Cree
2023-10-06 0:07 ` Jakub Kicinski
2023-12-07 14:15 ` Edward Cree
2023-12-07 16:45 ` Jakub Kicinski
2023-09-27 18:13 ` [PATCH v4 net-next 7/7] sfc: use new rxfh_context API edward.cree
2023-09-29 18:27 ` Jacob Keller
2023-10-02 13:01 ` Martin Habets
2023-10-05 20:54 ` Edward Cree
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231002095946.GA21694@gmail.com \
--to=habetsm.xilinx@gmail.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=ecree.xilinx@gmail.com \
--cc=edumazet@google.com \
--cc=edward.cree@amd.com \
--cc=gakula@marvell.com \
--cc=hkallweit1@gmail.com \
--cc=hkelam@marvell.com \
--cc=jdamato@fastly.com \
--cc=jiawenwu@trustnetic.com \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=linux-net-drivers@amd.com \
--cc=linux@armlinux.org.uk \
--cc=mengyuanlou@net-swift.com \
--cc=mw@semihalf.com \
--cc=netdev@vger.kernel.org \
--cc=nic_swsd@realtek.com \
--cc=pabeni@redhat.com \
--cc=saeedm@nvidia.com \
--cc=sbhatta@marvell.com \
--cc=sgoutham@marvell.com \
--cc=sudheer.mogilappagari@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.