Fix #1073: Add support MSI/MSI-X interrupts during NIC queues configuration #1097
Conversation
| @@ -43,7 +43,8 @@ tdb_mod=tempesta_db | |||
| tfw_mod=tempesta_fw | |||
| declare -r LONG_OPTS="help,load,unload,start,stop,restart,reload" | |||
|
|
|||
| declare devs=$(ip addr show up | awk '/^[0-9]+/ { sub(/:/, "", $2); print $2}') | |||
| declare devs=$(ip addr show up | grep -P '^[0-9]+' | grep -Pv '\bLOOPBACK\b' \ | |||
| | awk '{ sub(/:/, "", $2); print $2}') | |||
There was a problem hiding this comment.
A comment #1073 (comment) and/or later one, explaining why do we exclude loopback from RPS setup, would be useful here.
| dev_irqs_path="/sys/class/net/$dev/device/msi_irqs" | ||
| irqs=($(grep $dev /proc/interrupts | sed -e 's/\s*\|:.*//g')) | ||
| if [ -z "$irqs" -a -d $dev_irqs_path ]; then | ||
| irqs=($(ls $dev_irqs_path)) |
There was a problem hiding this comment.
The things are going complex, so please write a short (as above) comment why do we need the 2 sources to learn interrupts.
There was a problem hiding this comment.
Comment added.
scripts/tfw_lib.sh
Outdated
| echo "...set rx channels to $RXQ_MAX, please wait..." | ||
| # Set maximum number of available channels for better | ||
| # packets hashing. | ||
| ethtool -L $dev rx $RXQ_MAX >/dev/null 2>&1 |
There was a problem hiding this comment.
I tried to do this with hands
root@9011:~# ethtool -L enp7s0 rx 128
rx unmodified, ignoring
no channel parameters changed, aborting
current values: tx 128 rx 8 other 0 combined 0
So we have the issue #1073 - we can not set the required number of channels and we shall not just silently ignore the error. Instead, please print a warning message so we'll be able to catch issues like #1073 quicker in future.
There was a problem hiding this comment.
By the way, why does the command fails?
There was a problem hiding this comment.
Yes - agree, error message should be added.
Command fails because the count of queues had already been set to 128 before the command invocation - so, repeated attempt for the same value is rejected.
There was a problem hiding this comment.
Error message added.
scripts/tempesta.sh
Outdated
| @@ -208,6 +208,15 @@ stop() | |||
| echo "...unload Tempesta modules" | |||
| unload_modules | |||
|
|
|||
| systemctl status irqbalance.service >/dev/null | |||
There was a problem hiding this comment.
Other irqbalance related functions are defined in tfw_lib.sh, please move this code there.
|
|
||
| calc() | ||
| { | ||
| echo "$1" | bc -iq | tail -1 | ||
| } | ||
|
|
||
| # Assigned IRQs can be reassigned by irqbalance. To avoid this, assigned vectors | ||
| # should be added to irqbalance config with '--banirq' option. | ||
| irqbalance_ban_irqs() |
There was a problem hiding this comment.
The third-party configuration file is modified here, so we need to add the good comment to document that the BAN_IRQ variable is automatically generated by the Tempesta. Also there is no need to modify the systemd service file and call systemd daemon-reload. We should only modify /etc/default/irqbalance in the following way:
# Generated by Tempesta to restrict rebalancing for NIC irqs....
TFW_BAN_IRQS="--banirq ..."
IRQBALANCE_ARGS="$TFW_BAN_IRQS"
And don't forget the quotes around BAN_IRQS value.
There was a problem hiding this comment.
It doesn't seem possible to pass TFW_BAN_IRQ via IRQBALANCE_ARGS since systemd does not expand nested variables in files mentioned in EnvironmentFile directive: it interprets them in key -> value manner.
Comment has been added.
No description provided.