Re: [PATCH] pcnet32.c: modify RX ring size through module parameter
I have several problems with this patch. First, it assumes you only have one device or you want all devices to operate with the same receive ring size. (use module_param_array like full_duplex or options). Second, the mininum number of descriptors should be 4 not 2, or the loopback test will look past the end of the receive ring looking for status to change, and then try and pick up an skb pointer past the end of the array and try to dereference it. Either fix the loopback test to work with the minimum number of tx and rx descriptors instead of the hard coded 4, or make the minimum rx ring size be 4. numbuffs = min(4 , min(lp-tx_ring_size, lp-rx_ring_size)); Another nit is the description says it is the RX Ring Buffer Size which might be misunderstood as the size of the receive buffer, not the size of the receive descriptor ring. (RX Ring Size would be better). Lastly, the patch also will not apply against pcnet32.c in mainline 2.6.17-rc4 due to whitespace changes. On Mon, May 15, 2006 at 11:32:14AM +0800, Wen Hsin Chang wrote: This patch is created from pcnet32.c v1.32. it will allow users to specify RX ring size upon module insertion via module parameter 'rx_log_size'. This is needed in some cases where too small the rx ring size will cause RX errors upon remote installation via pcnet32 NIC card. Signed-off-by: Wen Hsin Chang [EMAIL PROTECTED] -- Don Fry [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pcnet32.c: modify RX ring size through module parameter
Why is this necessary? There is already an ethtool function to set the rx ring size (pcnet32_set_ringparam). Since module parameters are being phased out in favor of the ethtool functions, why not use the existing ethtool infrastructure for this? Thanks, Jon On Mon, May 15, 2006 at 11:32:14AM +0800, Wen Hsin Chang wrote: This patch is created from pcnet32.c v1.32. it will allow users to specify RX ring size upon module insertion via module parameter 'rx_log_size'. This is needed in some cases where too small the rx ring size will cause RX errors upon remote installation via pcnet32 NIC card. Signed-off-by: Wen Hsin Chang [EMAIL PROTECTED] --- a/drivers/net/pcnet32.c2006-03-30 09:49:10.0 +0800 +++ b/drivers/net/pcnet32.c2006-05-15 11:14:45.0 +0800 @@ -93,6 +93,9 @@ static struct net_device *pcnet32_dev; static int max_interrupt_work = 2; static int rx_copybreak = 200; +/* Module parameter to specify RX ring size at module insertion */ +static int rx_log_size = 0; + #define PCNET32_PORT_AUI 0x00 #define PCNET32_PORT_10BT 0x01 #define PCNET32_PORT_GPSI 0x02 @@ -1264,7 +1267,10 @@ pcnet32_probe1(unsigned long ioaddr, int lp-name = chipname; lp-shared_irq = shared; lp-tx_ring_size = TX_RING_SIZE;/* default tx ring size */ -lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */ +if (!rx_log_size) +lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */ +else +lp-rx_ring_size = (1 (rx_log_size)); lp-tx_mod_mask = lp-tx_ring_size - 1; lp-rx_mod_mask = lp-rx_ring_size - 1; lp-tx_len_bits = (PCNET32_LOG_TX_BUFFERS 12); @@ -2707,6 +2713,11 @@ module_param(tx_start_pt, int, 0); MODULE_PARM_DESC(tx_start_pt, DRV_NAME transmit start point (0-3)); module_param(pcnet32vlb, int, 0); MODULE_PARM_DESC(pcnet32vlb, DRV_NAME Vesa local bus (VLB) support (0/1)); + +/* Module parameter to specify RX ring size at module insertion */ +module_param(rx_log_size, int, 0); +MODULE_PARM_DESC(rx_log_size, DRV_NAME RX Ring Buffer Size (log_2 #BUF) ); + module_param_array(options, int, NULL, 0); MODULE_PARM_DESC(options, DRV_NAME initial option setting(s) (0-15)); module_param_array(full_duplex, int, NULL, 0); @@ -2731,6 +2742,12 @@ static int __init pcnet32_init_module(vo if ((tx_start_pt = 0) (tx_start_pt = 3)) tx_start = tx_start_pt; + +/* validating rx_log_size */ +if ((rx_log_size = 0) || +(rx_log_size PCNET32_LOG_MAX_RX_BUFFERS)) +rx_log_size = 0; + /* find the PCI devices */ if (!pci_module_init(pcnet32_driver)) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pcnet32.c: modify RX ring size through module parameter
There are cases where rx errors were found due to rx ring size being too small during remote installation using pcnet32. It is found that ethtool functionality may not be available under such circumenstance. This is the purpose behind this patch. However, as Don pointed out, there are several points that need to be considered. I'll further revise this patch and see how to come up with a better solution. Thanks for your comment~ Wen Hsin Chang Jon Mason wrote: Why is this necessary? There is already an ethtool function to set the rx ring size (pcnet32_set_ringparam). Since module parameters are being phased out in favor of the ethtool functions, why not use the existing ethtool infrastructure for this? Thanks, Jon On Mon, May 15, 2006 at 11:32:14AM +0800, Wen Hsin Chang wrote: This patch is created from pcnet32.c v1.32. it will allow users to specify RX ring size upon module insertion via module parameter 'rx_log_size'. This is needed in some cases where too small the rx ring size will cause RX errors upon remote installation via pcnet32 NIC card. Signed-off-by: Wen Hsin Chang [EMAIL PROTECTED] --- a/drivers/net/pcnet32.c2006-03-30 09:49:10.0 +0800 +++ b/drivers/net/pcnet32.c2006-05-15 11:14:45.0 +0800 @@ -93,6 +93,9 @@ static struct net_device *pcnet32_dev; static int max_interrupt_work = 2; static int rx_copybreak = 200; +/* Module parameter to specify RX ring size at module insertion */ +static int rx_log_size = 0; + #define PCNET32_PORT_AUI 0x00 #define PCNET32_PORT_10BT 0x01 #define PCNET32_PORT_GPSI 0x02 @@ -1264,7 +1267,10 @@ pcnet32_probe1(unsigned long ioaddr, int lp-name = chipname; lp-shared_irq = shared; lp-tx_ring_size = TX_RING_SIZE;/* default tx ring size */ -lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */ +if (!rx_log_size) +lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */ +else +lp-rx_ring_size = (1 (rx_log_size)); lp-tx_mod_mask = lp-tx_ring_size - 1; lp-rx_mod_mask = lp-rx_ring_size - 1; lp-tx_len_bits = (PCNET32_LOG_TX_BUFFERS 12); @@ -2707,6 +2713,11 @@ module_param(tx_start_pt, int, 0); MODULE_PARM_DESC(tx_start_pt, DRV_NAME transmit start point (0-3)); module_param(pcnet32vlb, int, 0); MODULE_PARM_DESC(pcnet32vlb, DRV_NAME Vesa local bus (VLB) support (0/1)); + +/* Module parameter to specify RX ring size at module insertion */ +module_param(rx_log_size, int, 0); +MODULE_PARM_DESC(rx_log_size, DRV_NAME RX Ring Buffer Size (log_2 #BUF) ); + module_param_array(options, int, NULL, 0); MODULE_PARM_DESC(options, DRV_NAME initial option setting(s) (0-15)); module_param_array(full_duplex, int, NULL, 0); @@ -2731,6 +2742,12 @@ static int __init pcnet32_init_module(vo if ((tx_start_pt = 0) (tx_start_pt = 3)) tx_start = tx_start_pt; + +/* validating rx_log_size */ +if ((rx_log_size = 0) || +(rx_log_size PCNET32_LOG_MAX_RX_BUFFERS)) +rx_log_size = 0; + /* find the PCI devices */ if (!pci_module_init(pcnet32_driver)) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pcnet32.c: modify RX ring size through module parameter
This patch is created from pcnet32.c v1.32. it will allow users to specify RX ring size upon module insertion via module parameter 'rx_log_size'. This is needed in some cases where too small the rx ring size will cause RX errors upon remote installation via pcnet32 NIC card. Signed-off-by: Wen Hsin Chang [EMAIL PROTECTED] --- a/drivers/net/pcnet32.c2006-03-30 09:49:10.0 +0800 +++ b/drivers/net/pcnet32.c2006-05-15 11:14:45.0 +0800 @@ -93,6 +93,9 @@ static struct net_device *pcnet32_dev; static int max_interrupt_work = 2; static int rx_copybreak = 200; +/* Module parameter to specify RX ring size at module insertion */ +static int rx_log_size = 0; + #define PCNET32_PORT_AUI 0x00 #define PCNET32_PORT_10BT 0x01 #define PCNET32_PORT_GPSI 0x02 @@ -1264,7 +1267,10 @@ pcnet32_probe1(unsigned long ioaddr, int lp-name = chipname; lp-shared_irq = shared; lp-tx_ring_size = TX_RING_SIZE;/* default tx ring size */ -lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */ +if (!rx_log_size) +lp-rx_ring_size = RX_RING_SIZE;/* default rx ring size */ +else +lp-rx_ring_size = (1 (rx_log_size)); lp-tx_mod_mask = lp-tx_ring_size - 1; lp-rx_mod_mask = lp-rx_ring_size - 1; lp-tx_len_bits = (PCNET32_LOG_TX_BUFFERS 12); @@ -2707,6 +2713,11 @@ module_param(tx_start_pt, int, 0); MODULE_PARM_DESC(tx_start_pt, DRV_NAME transmit start point (0-3)); module_param(pcnet32vlb, int, 0); MODULE_PARM_DESC(pcnet32vlb, DRV_NAME Vesa local bus (VLB) support (0/1)); + +/* Module parameter to specify RX ring size at module insertion */ +module_param(rx_log_size, int, 0); +MODULE_PARM_DESC(rx_log_size, DRV_NAME RX Ring Buffer Size (log_2 #BUF) ); + module_param_array(options, int, NULL, 0); MODULE_PARM_DESC(options, DRV_NAME initial option setting(s) (0-15)); module_param_array(full_duplex, int, NULL, 0); @@ -2731,6 +2742,12 @@ static int __init pcnet32_init_module(vo if ((tx_start_pt = 0) (tx_start_pt = 3)) tx_start = tx_start_pt; + +/* validating rx_log_size */ +if ((rx_log_size = 0) || +(rx_log_size PCNET32_LOG_MAX_RX_BUFFERS)) +rx_log_size = 0; + /* find the PCI devices */ if (!pci_module_init(pcnet32_driver)) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
