diff mbox series

[net-next,v2,5/5] enic: Obtain the Link speed only after the link comes up

Message ID 20241227031410.25607-6-johndale@cisco.com (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series enic: Use Page Pool API for receiving packets | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 2 this patch: 2
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 11 this patch: 11
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 30 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

John Daley Dec. 27, 2024, 3:14 a.m. UTC 
The link speed that is used to index the table of minimum RX adaptive
coalescing values is incorrect because the link speed was being checked
before the link was up. Change the adaptive RX coalescing setup function
to run after the Link comes up.

There could be a minor bandwidth impact when adaptive interrupts were
enabled. The low end of the adaptive interrupt range was being set to 0
for all packets instead of 3us for packets less the 1000 bytes and 6us
for larger packet for link speeds greater

Co-developed-by: Nelson Escobar <neescoba@cisco.com>
Signed-off-by: Nelson Escobar <neescoba@cisco.com>
Co-developed-by: Satish Kharat <satishkh@cisco.com>
Signed-off-by: Satish Kharat <satishkh@cisco.com>
Signed-off-by: John Daley <johndale@cisco.com>
---
 drivers/net/ethernet/cisco/enic/enic_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Andrew Lunn Dec. 27, 2024, 3:59 p.m. UTC | #1 
On Thu, Dec 26, 2024 at 07:14:10PM -0800, John Daley wrote:
> The link speed that is used to index the table of minimum RX adaptive
> coalescing values is incorrect because the link speed was being checked
> before the link was up. Change the adaptive RX coalescing setup function
> to run after the Link comes up.
> 
> There could be a minor bandwidth impact when adaptive interrupts were
> enabled. The low end of the adaptive interrupt range was being set to 0
> for all packets instead of 3us for packets less the 1000 bytes and 6us
> for larger packet for link speeds greater

There are two changes here, so please break it into two patches.

> +++ b/drivers/net/ethernet/cisco/enic/enic_main.c
> @@ -84,6 +84,8 @@ MODULE_AUTHOR("Scott Feldman <scofeldm@cisco.com>");
>  MODULE_LICENSE("GPL");
>  MODULE_DEVICE_TABLE(pci, enic_id_table);
>  
> +static void enic_set_rx_coal_setting(struct enic *enic);
> +

Please don't add forward declarations. Move the code around
instead. You can add a preparation patch which does the move, and in
the commit message say there is no functional change. That makes is
quick and easy to review.

      
    Andrew

---
pw-bot: cr
diff mbox series

Patch

diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 5bfd89749237..7c2bfe4b7997 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -84,6 +84,8 @@  MODULE_AUTHOR("Scott Feldman <scofeldm@cisco.com>");
 MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, enic_id_table);
 
+static void enic_set_rx_coal_setting(struct enic *enic);
+
 #define ENIC_MAX_COALESCE_TIMERS		10
 /*  Interrupt moderation table, which will be used to decide the
  *  coalescing timer values
@@ -109,7 +111,7 @@  static struct enic_intr_mod_table mod_table[ENIC_MAX_COALESCE_TIMERS + 1] = {
 static struct enic_intr_mod_range mod_range[ENIC_MAX_LINK_SPEEDS] = {
 	{0,  0}, /* 0  - 4  Gbps */
 	{0,  3}, /* 4  - 10 Gbps */
-	{3,  6}, /* 10 - 40 Gbps */
+	{3,  6}, /* 10+ Gbps */
 };
 
 static void enic_init_affinity_hint(struct enic *enic)
@@ -436,6 +438,7 @@  static void enic_link_check(struct enic *enic)
 	if (link_status && !carrier_ok) {
 		netdev_info(enic->netdev, "Link UP\n");
 		netif_carrier_on(enic->netdev);
+		enic_set_rx_coal_setting(enic);
 	} else if (!link_status && carrier_ok) {
 		netdev_info(enic->netdev, "Link DOWN\n");
 		netif_carrier_off(enic->netdev);
@@ -3016,7 +3019,6 @@  static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	timer_setup(&enic->notify_timer, enic_notify_timer, 0);
 
 	enic_rfs_flw_tbl_init(enic);
-	enic_set_rx_coal_setting(enic);
 	INIT_WORK(&enic->reset, enic_reset);
 	INIT_WORK(&enic->tx_hang_reset, enic_tx_hang_reset);
 	INIT_WORK(&enic->change_mtu_work, enic_change_mtu_work);