Re: [PATCH net-next 2/6] igb: Add double-check MTA_REGISTER for i210 and i211

[Nick Lowe <nick.lowe@gmail.com>]

Hi all,

> > Looks like a potential infinite loop on persistent failure.
> > Also you don't need "is_failed", you can use while (i >= 0), or
> > assign i = hw->mac.mta_reg_count, or consider using a goto.
>
> We will make a follow on patch to address these issues.
>
> Thanks,
> Tony

The patch for this that has been queued is as follows:

+     int failed_cnt = 3;
+     bool is_failed;
+     int i;
+
+     do {
+          is_failed = false;
+          for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) {
+               if (array_rd32(E1000_MTA, i) != hw->mac.mta_shadow[i]) {
+                    is_failed = true;
+                    array_wr32(E1000_MTA, i, hw->mac.mta_shadow[i]);
+                    wrfl();
+               }
+          }
+          if (is_failed && --failed_cnt <= 0) {
+               hw_dbg("Failed to update MTA_REGISTER, too many retries");
+               break;
+          }
+     } while (is_failed);

https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git/commit/?h=dev-queue&id=9db33b54fb98525e323d0d3f16b01778f17b9493

This will not reset the counter when checking each register and it
will not debug output which register failed, this does not seem
optimal.

Could it make more sense to instead do something like this? (Untested)

+     int i;
+     int attempt;
+     for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) {
+          for (attempt = 3; attempt >= 1; attempt--) {
+               if (array_rd32(E1000_MTA, i) != hw->mac.mta_shadow[i]) {
+                    array_wr32(E1000_MTA, i, hw->mac.mta_shadow[i]);
+                    wrfl();
+
+                    if (attempt == 1 && array_rd32(E1000_MTA, i) !=
hw->mac.mta_shadow[i]) {
+                         hw_dbg("Failed to update MTA_REGISTER %d,
too many retries\n", i);
+                    }
+               }
+          }
+     }

Best,

Nick