netdev - Re: [PATCH v2 net-next 1/2] enic: Add support for adaptive interrupt coalescing

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] [day] [month] [year] [list]

Date:	Mon, 26 May 2014 18:42:15 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Govindarajulu Varadarajan <_govind@....com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org, benve@...co.com,
	neepatel@...co.com, ssujith@...co.com
Subject: Re: [PATCH v2 net-next 1/2] enic: Add support for adaptive
 interrupt coalescing

On Tue, 2014-05-20 at 03:14 +0530, Govindarajulu Varadarajan wrote:
[...]
> @@ -191,17 +207,31 @@ static int enic_set_coalesce(struct net_device *netdev,
>  	struct enic *enic = netdev_priv(netdev);
>  	u32 tx_coalesce_usecs;
>  	u32 rx_coalesce_usecs;
> +	u32 rx_coalesce_usecs_low;
> +	u32 rx_coalesce_usecs_high;
> +	u32 coalesce_usecs_max;
>  	unsigned int i, intr;
> +	struct enic_rx_coal *rxcoal = &enic->rx_coalesce_setting;
>  
> +	coalesce_usecs_max = vnic_dev_get_intr_coal_timer_max(enic->vdev);
>  	tx_coalesce_usecs = min_t(u32, ecmd->tx_coalesce_usecs,
> -		vnic_dev_get_intr_coal_timer_max(enic->vdev));
> +				  coalesce_usecs_max);
>  	rx_coalesce_usecs = min_t(u32, ecmd->rx_coalesce_usecs,
> -		vnic_dev_get_intr_coal_timer_max(enic->vdev));
> +				  coalesce_usecs_max);
> +
> +	rx_coalesce_usecs_low = min_t(u32, ecmd->rx_coalesce_usecs_low,
> +				      coalesce_usecs_max);
> +	rx_coalesce_usecs_high = min_t(u32, ecmd->rx_coalesce_usecs_high,
> +				       coalesce_usecs_max);
>  
>  	switch (vnic_dev_get_intr_mode(enic->vdev)) {
>  	case VNIC_DEV_INTR_MODE_INTX:
>  		if (tx_coalesce_usecs != rx_coalesce_usecs)
>  			return -EINVAL;
> +		if (ecmd->use_adaptive_rx_coalesce	||
> +		    ecmd->rx_coalesce_usecs_low		||
> +		    ecmd->rx_coalesce_usecs_high)
> +			return -EOPNOTSUPP;

-EINVAL, please.  The operation (set coalescing parameters) is
supported, just not these particular arguments.

>  		intr = enic_legacy_io_intr();
>  		vnic_intr_coalescing_timer_set(&enic->intr[intr],
> @@ -210,6 +240,10 @@ static int enic_set_coalesce(struct net_device *netdev,
>  	case VNIC_DEV_INTR_MODE_MSI:
>  		if (tx_coalesce_usecs != rx_coalesce_usecs)
>  			return -EINVAL;
> +		if (ecmd->use_adaptive_rx_coalesce	||
> +		    ecmd->rx_coalesce_usecs_low		||
> +		    ecmd->rx_coalesce_usecs_high)
> +			return -EOPNOTSUPP;

-EINVAL again.
 
>  		vnic_intr_coalescing_timer_set(&enic->intr[0],
>  			tx_coalesce_usecs);
> @@ -221,12 +255,27 @@ static int enic_set_coalesce(struct net_device *netdev,
>  				tx_coalesce_usecs);
>  		}
>  
> -		for (i = 0; i < enic->rq_count; i++) {
> -			intr = enic_msix_rq_intr(enic, i);
> -			vnic_intr_coalescing_timer_set(&enic->intr[intr],
> -				rx_coalesce_usecs);
> +		if (rxcoal->use_adaptive_rx_coalesce) {
> +			if (!ecmd->use_adaptive_rx_coalesce) {
> +				rxcoal->use_adaptive_rx_coalesce = 0;
> +				enic_intr_coal_set_rx(enic, rx_coalesce_usecs);
> +			}
> +		} else {
> +			if (ecmd->use_adaptive_rx_coalesce)
> +				rxcoal->use_adaptive_rx_coalesce = 1;
> +			else
> +				enic_intr_coal_set_rx(enic, rx_coalesce_usecs);
>  		}

This seems to be an overly complicated way of writing:

		rxcoal->use_adaptive_rx_coalesce = !!ecmd->use_adaptive_rx_coalesce;
		if (!rxcoal->use_adaptive_rx_coalesce)
			enic_intr_coal_set_rx(enic, rx_coalesce_usecs);

> +		if (ecmd->rx_coalesce_usecs_high) {
> +			if (rx_coalesce_usecs_high <
> +			    (rx_coalesce_usecs_low + ENIC_AIC_LARGE_PKT_DIFF))
> +				return -EINVAL;
[...]

It's a bit late to detect invalid arguments when you've already started
applying changes.

Ben.

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)