netdev - Re: [RFC PATCH] net: add a tx_queue attribute rate_queue

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]

Date:	Tue, 25 Jun 2013 20:00:37 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	John Fastabend <john.fastabend@...il.com>
CC:	<netdev@...r.kernel.org>, <therbert@...gle.com>,
	<ben@...adent.org.uk>, <jesse.brandeburg@...el.com>,
	<jeffrey.t.kirsher@...el.com>
Subject: Re: [RFC PATCH] net: add a tx_queue attribute rate_queue_limits in
 Mbps

On Sun, 2013-06-23 at 20:24 -0700, John Fastabend wrote:
[...]
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
[...]
> +int ixgbe_set_rate_limit(struct net_device *dev, int index, u32 *tx_rate)
> +{
> +	struct ixgbe_adapter *a = netdev_priv(dev);
> +	struct ixgbe_hw *hw = &a->hw;
> +	int linkspeed = ixgbe_link_mbps(a);
> +	u8 reg_idx = a->tx_ring[index]->reg_idx;
> +	u32 bcnrc = ixgbe_bcnrc_from_rate(a, *tx_rate, linkspeed);
> +
> +	/* rate limit cannot be less than 10Mbs or greater than link speed */
> +	if (*tx_rate && ((*tx_rate <= 10) || (*tx_rate > linkspeed)))
> +		return -EINVAL;

What if the link is not up?

What if the link speed is currently higher than tx_rate, but is later
reduced below tx_rate?

[...]
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -574,6 +574,7 @@ struct netdev_queue {
>  #ifdef CONFIG_BQL
>  	struct dql		dql;
>  #endif
> +	unsigned long		rate_limit;

Should be u32, consistent with variables in ndo_set_ratelimit and
set_tx_rate_limit.

>  } ____cacheline_aligned_in_smp;
>  
>  static inline int netdev_queue_numa_node_read(const struct netdev_queue *q)
> @@ -932,6 +933,14 @@ struct netdev_fcoe_hbainfo {
>   *	that determine carrier state from physical hardware properties (eg
>   *	network cables) or protocol-dependent mechanisms (eg
>   *	USB_CDC_NOTIFY_NETWORK_CONNECTION) should NOT implement this function.
> + *
> + * int (*ndo_set_ratelimit)(struct net_device *dev,
> + *			    int queue_index, u32 *maxrate)
> + *	Called to set the rate limit in Mpbs specified by maxrate of the
> + *	specified queue_index. It is expected that hardware way may quantize
> + *	the rate limits. In these cases the driver should guarentee the
> + *	specfied maxrate is not exceeded and return the set value in maxrate.
> + *	Zero should be returned on sucess otherwise use appropriate error code.
[...]

In general, how does maxrate interact with link speed?

There are also several spelling and grammar errors in this text.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html