linux-kernel - Re: [PATCH] [RFC] net: fec: Allow turning off IRQ 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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 20 Feb 2023 21:07:36 +0100 (CET)
From:   Richard Weinberger <richard@....at>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>, pabeni@...hat.com,
        kuba@...nel.org, edumazet@...gle.com, davem <davem@...emloft.net>,
        linux-imx <linux-imx@....com>,
        xiaoning wang <xiaoning.wang@....com>,
        shenwei wang <shenwei.wang@....com>,
        wei fang <wei.fang@....com>
Subject: Re: [PATCH] [RFC] net: fec: Allow turning off IRQ coalescing

Andrew,

----- Ursprüngliche Mail -----
>> -	/* Must be greater than zero to avoid unpredictable behavior */
>> -	if (!fep->rx_time_itr || !fep->rx_pkts_itr ||
>> -	    !fep->tx_time_itr || !fep->tx_pkts_itr)
>> -		return;
>> +	if (!fep->rx_time_itr || !fep->rx_pkts_itr) {
>> +		if (fep->rx_time_itr || fep->rx_pkts_itr) {
>> +			dev_warn(dev, "Rx coalesced frames and usec have to be "
>> +				      "both positive or both zero to disable Rx "
>> +				      "coalescence completely\n");
>> +			return -EINVAL;
>> +		}
> 
> Hi Richard
> 
> Why do this validation here, and not in fec_enet_set_coalesce() where
> there are already checks? fec_enet_set_coalesce() also has extack, so
> you can return useful messages to user space, not just the kernel log.

Using extack is a good point, the driver does not use it at all so far.
So I'd do a second patch which cleans this up.

I did the check in fec_enet_itr_coal_set() because the check is used to
set both disable_rx_itr and disable_tx_itr.
Of course I can place the check into fec_enet_set_coalesce() and then
pass disable_rx_itr and disable_tx_itr to fec_enet_itr_coal_set().

Thanks,
//richard