| 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
| ||
|
Date: Sun, 27 Aug 2017 11:34:10 +0300
From: "Neftin, Sasha" <sasha.neftin@...el.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
jeffrey.t.kirsher@...el.com, alexander.h.duyck@...el.com,
raanan.avargil@...el.com, dima.ruinskiy@...el.com
Cc: netdev@...r.kernel.org, Willem de Bruijn <willemb@...gle.com>,
intel-wired-lan@...ts.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH] e1000e: apply burst mode settings only
on default
On 8/27/2017 11:32, Neftin, Sasha wrote:
> On 8/27/2017 11:30, Neftin, Sasha wrote:
>> On 8/25/2017 18:06, Willem de Bruijn wrote:
>>> From: Willem de Bruijn <willemb@...gle.com>
>>>
>>> Devices that support FLAG2_DMA_BURST have different default values
>>> for RDTR and RADV. Apply burst mode default settings only when no
>>> explicit value was passed at module load.
>>>
>>> The RDTR default is zero. If the module is loaded for low latency
>>> operation with RxIntDelay=0, do not override this value with a burst
>>> default of 32.
>>>
>>> Move the decision to apply burst values earlier, where explicitly
>>> initialized module variables can be distinguished from defaults.
>>>
>>> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
>>> ---
>>> drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
>>> drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
>>> drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
>>> 3 files changed, 15 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h
>>> b/drivers/net/ethernet/intel/e1000e/e1000.h
>>> index 98e68888abb1..2311b31bdcac 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/e1000.h
>>> +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
>>> @@ -94,10 +94,6 @@ struct e1000_info;
>>> */
>>> #define E1000_CHECK_RESET_COUNT 25
>>> -#define DEFAULT_RDTR 0
>>> -#define DEFAULT_RADV 8
>>> -#define BURST_RDTR 0x20
>>> -#define BURST_RADV 0x20
>>> #define PCICFG_DESC_RING_STATUS 0xe4
>>> #define FLUSH_DESC_REQUIRED 0x100
>>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
>>> b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> index 327dfe5bedc0..47b89aac7969 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> @@ -3223,14 +3223,6 @@ static void e1000_configure_rx(struct
>>> e1000_adapter *adapter)
>>> */
>>> ew32(RXDCTL(0), E1000_RXDCTL_DMA_BURST_ENABLE);
>>> ew32(RXDCTL(1), E1000_RXDCTL_DMA_BURST_ENABLE);
>>> -
>>> - /* override the delay timers for enabling bursting, only if
>>> - * the value was not set by the user via module options
>>> - */
>>> - if (adapter->rx_int_delay == DEFAULT_RDTR)
>>> - adapter->rx_int_delay = BURST_RDTR;
>>> - if (adapter->rx_abs_int_delay == DEFAULT_RADV)
>>> - adapter->rx_abs_int_delay = BURST_RADV;
>>> }
>>> /* set the Receive Delay Timer Register */
>>> diff --git a/drivers/net/ethernet/intel/e1000e/param.c
>>> b/drivers/net/ethernet/intel/e1000e/param.c
>>> index 6d8c39abee16..bb696c98f9b0 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/param.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/param.c
>>> @@ -73,17 +73,25 @@ E1000_PARAM(TxAbsIntDelay, "Transmit Absolute
>>> Interrupt Delay");
>>> /* Receive Interrupt Delay in units of 1.024 microseconds
>>> * hardware will likely hang if you set this to anything but zero.
>>> *
>>> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
>>> + *
>>> * Valid Range: 0-65535
>>> */
>>> E1000_PARAM(RxIntDelay, "Receive Interrupt Delay");
>>> +#define DEFAULT_RDTR 0
>>> +#define BURST_RDTR 0x20
>>> #define MAX_RXDELAY 0xFFFF
>>> #define MIN_RXDELAY 0
>>> /* Receive Absolute Interrupt Delay in units of 1.024 microseconds
>>> + *
>>> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
>>> *
>>> * Valid Range: 0-65535
>>> */
>>> E1000_PARAM(RxAbsIntDelay, "Receive Absolute Interrupt Delay");
>>> +#define DEFAULT_RADV 8
>>> +#define BURST_RADV 0x20
>>> #define MAX_RXABSDELAY 0xFFFF
>>> #define MIN_RXABSDELAY 0
>>> @@ -297,6 +305,9 @@ void e1000e_check_options(struct e1000_adapter
>>> *adapter)
>>> .max = MAX_RXDELAY } }
>>> };
>>> + if (adapter->flags2 & FLAG2_DMA_BURST)
>>> + opt.def = BURST_RDTR;
>>> +
>>> if (num_RxIntDelay > bd) {
>>> adapter->rx_int_delay = RxIntDelay[bd];
>>> e1000_validate_option(&adapter->rx_int_delay, &opt,
>>> @@ -307,7 +318,7 @@ void e1000e_check_options(struct e1000_adapter
>>> *adapter)
>>> }
>>> /* Receive Absolute Interrupt Delay */
>>> {
>>> - static const struct e1000_option opt = {
>>> + static struct e1000_option opt = {
>>> .type = range_option,
>>> .name = "Receive Absolute Interrupt Delay",
>>> .err = "using default of "
>>> @@ -317,6 +328,9 @@ void e1000e_check_options(struct e1000_adapter
>>> *adapter)
>>> .max = MAX_RXABSDELAY } }
>>> };
>>> + if (adapter->flags2 & FLAG2_DMA_BURST)
>>> + opt.def = BURST_RADV;
>>> +
>>> if (num_RxAbsIntDelay > bd) {
>>> adapter->rx_abs_int_delay = RxAbsIntDelay[bd];
>>> e1000_validate_option(&adapter->rx_abs_int_delay, &opt,
>>
>> This patch looks good for me, but I would like hear second opinion.
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@...osl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@...osl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Powered by blists - more mailing lists