S2io: module loadable parameters
Subsystems (2)
PATCH 5/8 S2io: module loadable parameters
Hi,
This patch adds/modifies the following module loadable parameters.
1. Max Txds per List.
2. statistics refresh time
3. pause frame control parameters including gap between two successive frames, threshold
watermarks
4. RTI and TTI configuration parameters including ranges, packet counts and timeout periods.
For further information please read the section 3.5 of XFrame H/W spec.
5. PCI/PCI-X configuration variables latency_timer, MMRBC and OST.
6. OS offload features TSO (If support available) and checksum offload.
7. If NAPI is not in use, a variable indicate_max_pkts can be used to limit number of Rx
side packets processed for one call to Rx Intr handler.
Major objections to this patch.
Comments:
1) Should use new module_param() not older, less-type-strict MODULE_PARM() in 2.6.x kernels
2) don't create individual module options when arrays are far better suited to the task. The following technique simply bloats up the code and makes use of loops impossible:
Review
Quoted 8 lines
(quoted diff) +8
-0+static unsigned int tx_fifo_len_0 = DEFAULT_FIFO_LEN; +static unsigned int tx_fifo_len_1; +static unsigned int tx_fifo_len_2; +static unsigned int tx_fifo_len_3; +static unsigned int tx_fifo_len_4; +static unsigned int tx_fifo_len_5; +static unsigned int tx_fifo_len_6; +static unsigned int tx_fifo_len_7;
3) do not enable NETIF_F_SG without also enabling a checksum-offload/TSO mode of some sort
Review
Quoted 7 lines
(quoted diff) +3
-1- dev->features |= NETIF_F_SG | NETIF_F_IP_CSUM; + dev->features |= NETIF_F_SG; + if (cksum_offload_enable) + dev->features |= NETIF_F_IP_CSUM; if (sp->high_dma_flag == TRUE) dev->features |= NETIF_F_HIGHDMA;
4) I am unsure of the value of verify_load_parm(). Besides being huge, due to issue #2 (above), typically user module option limits should be documented, not necessarily tested. Unix philosophy "root allowed to shoot themselves in the foot"