netdev - Re: [PATCH RFC v2 10/11] iavf: Add net_shaper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zqziup5Ele_jBm3M@nanopsycho.orion>
Date: Fri, 2 Aug 2024 15:44:26 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
	Madhu Chittim <madhu.chittim@...el.com>,
	Sridhar Samudrala <sridhar.samudrala@...el.com>,
	Simon Horman <horms@...nel.org>,
	John Fastabend <john.fastabend@...il.com>,
	Sunil Kovvuri Goutham <sgoutham@...vell.com>,
	Jamal Hadi Salim <jhs@...atatu.com>
Subject: Re: [PATCH RFC v2 10/11] iavf: Add net_shaper_ops support

Wed, Jul 24, 2024 at 10:24:56PM CEST, pabeni@...hat.com wrote:
>From: Sudheer Mogilappagari <sudheer.mogilappagari@...el.com>

[...]


>+static int
>+iavf_shaper_set(struct net_device *dev,
>+		const struct net_shaper_info *shaper,
>+		struct netlink_ext_ack *extack)
>+{
>+	struct iavf_adapter *adapter = netdev_priv(dev);
>+	bool need_cfg_update = false;
>+	enum net_shaper_scope scope;
>+	int id, ret = 0;
>+
>+	ret = iavf_verify_shaper_info(dev, shaper, extack);
>+	if (ret)
>+		return ret;
>+
>+	scope = net_shaper_handle_scope(shaper->handle);
>+	id = net_shaper_handle_id(shaper->handle);
>+
>+	if (scope == NET_SHAPER_SCOPE_QUEUE) {
>+		struct iavf_ring *tx_ring = &adapter->tx_rings[id];
>+
>+		tx_ring->q_shaper.bw_min = div_u64(shaper->bw_min, 1000);
>+		tx_ring->q_shaper.bw_max = div_u64(shaper->bw_max, 1000);

Isn't this more or less what ndo_set_tx_maxrate() does? I mean, the only
difference I see is ability to set min/max. Do I read this correctly?

If yes, could we have the ndo_set_tx_maxrate() consolidation included in
this patchset (or at least to post follow-up RFC)? I mean, without that,
there are 2 uapis to set queue rate limit for different drivers
(net-shaper for iavf, sysfs tx_maxrate for the rest), which looks messy.


>+		tx_ring->q_shaper_update = true;
>+		need_cfg_update = true;
>+	}
>+
>+	if (need_cfg_update)
>+		adapter->aq_required |= IAVF_FLAG_AQ_CONFIGURE_QUEUES_BW;
>+
>+	return 0;
>+}

[...]