[Bug 1452875] Re: irqbalance degrades network performance on HP M800 server cartridge

[Robie Basak]

We talked about the general approach on IRC and Manoj will update the
bug. We'll get the general approach agreed first. But while I'm looking
at the patch, here are some review comments:

+enum _bool {false = 0, true = 1};
+typedef enum _bool Bool;

Please don't do this unless it's convention in the existing source. Use
an int instead as this is standard C convention unless there's some
reason to do otherwise.

+	char *platform = "HP ProLiant m800 Server Cartridge";
+	char *sys_file = "/proc/device-tree/model";

These should be declared const.

+       while (fp != NULL && fread(tmp, 1, sizeof(tmp), fp) != 0) {

Won't this go forever if there is some kind of unexpected error? It
would be clearer to just return immediately on error - once for the
return value of fopen and separately for the fread. I also don't think
there's any need to run fread in a loop - if it fails the first time,
then just treat the file as unreadable (and presumably fail the test
negative). Or, if the /proc/device-tree/model file might be bigger than
the 256 bytes you've allocated in tmp, then the approach needs to be
different from just strstr().

+                          "Balancing is ineffective on %s.\n",
platform);

Please explain in the error message that this is the reason that
irqbalance is exiting - otherwise it could be confused to a sysadmin
wondering why irqbalance is exiting despite this particular message. Eg.
"Balancing is ineffective on %s: exiting.\n"

+       if (disable_on_platform())

As this code will never be generic, can you name this disable_on_hp_m800
or similar please, so it's clearer to someone reading the code?

Finally, please add "Forwarded: not-needed" to the dep3 header together
with an explanation of why forwarding is not needed in the description.

-- 
You received this bug notification because you are a member of Ubuntu
Server Team, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/1452875

Title:
  irqbalance degrades network performance on HP M800 server cartridge

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/irqbalance/+bug/1452875/+subscriptions