* [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define
@ 2020-11-13 23:16 Antonio Cardace
2020-11-13 23:16 ` [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings Antonio Cardace
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Antonio Cardace @ 2020-11-13 23:16 UTC (permalink / raw)
To: netdev, David S . Miller, Jakub Kicinski, Michal Kubecek
This bitmask represents all existing coalesce parameters.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
---
include/linux/ethtool.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 6408b446051f..e3da25b51ae4 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -215,6 +215,7 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
#define ETHTOOL_COALESCE_TX_USECS_HIGH BIT(19)
#define ETHTOOL_COALESCE_TX_MAX_FRAMES_HIGH BIT(20)
#define ETHTOOL_COALESCE_RATE_SAMPLE_INTERVAL BIT(21)
+#define ETHTOOL_COALESCE_ALL_PARAMS GENMASK(21, 0)
#define ETHTOOL_COALESCE_USECS \
(ETHTOOL_COALESCE_RX_USECS | ETHTOOL_COALESCE_TX_USECS)
--
2.28.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings
2020-11-13 23:16 [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Antonio Cardace
@ 2020-11-13 23:16 ` Antonio Cardace
2020-11-16 16:03 ` Michal Kubecek
2020-11-17 0:47 ` Jakub Kicinski
2020-11-13 23:16 ` [PATCH net-next v3 3/4] selftests: extract common functions in ethtool-common.sh Antonio Cardace
` (3 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Antonio Cardace @ 2020-11-13 23:16 UTC (permalink / raw)
To: netdev, David S . Miller, Jakub Kicinski, Michal Kubecek
Add ethtool ring and coalesce settings support for testing.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
---
drivers/net/netdevsim/ethtool.c | 82 ++++++++++++++++++++++++++-----
drivers/net/netdevsim/netdevsim.h | 9 +++-
2 files changed, 79 insertions(+), 12 deletions(-)
diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index f1884d90a876..169154dba0cc 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -13,9 +13,9 @@ nsim_get_pause_stats(struct net_device *dev,
{
struct netdevsim *ns = netdev_priv(dev);
- if (ns->ethtool.report_stats_rx)
+ if (ns->ethtool.pauseparam.report_stats_rx)
pause_stats->rx_pause_frames = 1;
- if (ns->ethtool.report_stats_tx)
+ if (ns->ethtool.pauseparam.report_stats_tx)
pause_stats->tx_pause_frames = 2;
}
@@ -25,8 +25,8 @@ nsim_get_pauseparam(struct net_device *dev, struct ethtool_pauseparam *pause)
struct netdevsim *ns = netdev_priv(dev);
pause->autoneg = 0; /* We don't support ksettings, so can't pretend */
- pause->rx_pause = ns->ethtool.rx;
- pause->tx_pause = ns->ethtool.tx;
+ pause->rx_pause = ns->ethtool.pauseparam.rx;
+ pause->tx_pause = ns->ethtool.pauseparam.tx;
}
static int
@@ -37,28 +37,88 @@ nsim_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam *pause)
if (pause->autoneg)
return -EINVAL;
- ns->ethtool.rx = pause->rx_pause;
- ns->ethtool.tx = pause->tx_pause;
+ ns->ethtool.pauseparam.rx = pause->rx_pause;
+ ns->ethtool.pauseparam.tx = pause->tx_pause;
+ return 0;
+}
+
+static int nsim_get_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *coal)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ memcpy(coal, &ns->ethtool.coalesce, sizeof(ns->ethtool.coalesce));
+ return 0;
+}
+
+static int nsim_set_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *coal)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ memcpy(&ns->ethtool.coalesce, coal, sizeof(ns->ethtool.coalesce));
+ return 0;
+}
+
+static void nsim_get_ringparam(struct net_device *dev,
+ struct ethtool_ringparam *ring)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ memcpy(ring, &ns->ethtool.ring, sizeof(ns->ethtool.ring));
+}
+
+static int nsim_set_ringparam(struct net_device *dev,
+ struct ethtool_ringparam *ring)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ memcpy(&ns->ethtool.ring, ring, sizeof(ns->ethtool.ring));
return 0;
}
static const struct ethtool_ops nsim_ethtool_ops = {
- .get_pause_stats = nsim_get_pause_stats,
- .get_pauseparam = nsim_get_pauseparam,
- .set_pauseparam = nsim_set_pauseparam,
+ .get_pause_stats = nsim_get_pause_stats,
+ .get_pauseparam = nsim_get_pauseparam,
+ .set_pauseparam = nsim_set_pauseparam,
+ .supported_coalesce_params = ETHTOOL_COALESCE_ALL_PARAMS,
+ .set_coalesce = nsim_set_coalesce,
+ .get_coalesce = nsim_get_coalesce,
+ .get_ringparam = nsim_get_ringparam,
+ .set_ringparam = nsim_set_ringparam,
};
+static void nsim_ethtool_ring_init(struct netdevsim *ns)
+{
+ ns->ethtool.ring.rx_max_pending = 4096;
+ ns->ethtool.ring.rx_jumbo_max_pending = 4096;
+ ns->ethtool.ring.rx_mini_max_pending = 4096;
+ ns->ethtool.ring.tx_max_pending = 4096;
+}
+
void nsim_ethtool_init(struct netdevsim *ns)
{
struct dentry *ethtool, *dir;
ns->netdev->ethtool_ops = &nsim_ethtool_ops;
+ nsim_ethtool_ring_init(ns);
+
ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
dir = debugfs_create_dir("pause", ethtool);
debugfs_create_bool("report_stats_rx", 0600, dir,
- &ns->ethtool.report_stats_rx);
+ &ns->ethtool.pauseparam.report_stats_rx);
debugfs_create_bool("report_stats_tx", 0600, dir,
- &ns->ethtool.report_stats_tx);
+ &ns->ethtool.pauseparam.report_stats_tx);
+
+ dir = debugfs_create_dir("ring", ethtool);
+ debugfs_create_u32("rx_max_pending", 0600, dir,
+ &ns->ethtool.ring.rx_max_pending);
+ debugfs_create_u32("rx_jumbo_max_pending", 0600, dir,
+ &ns->ethtool.ring.rx_jumbo_max_pending);
+ debugfs_create_u32("rx_mini_max_pending", 0600, dir,
+ &ns->ethtool.ring.rx_mini_max_pending);
+ debugfs_create_u32("tx_max_pending", 0600, dir,
+ &ns->ethtool.ring.tx_max_pending);
}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 827fc80f50a0..b023dc0a4259 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -15,6 +15,7 @@
#include <linux/debugfs.h>
#include <linux/device.h>
+#include <linux/ethtool.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/netdevice.h>
@@ -51,13 +52,19 @@ struct nsim_ipsec {
u32 ok;
};
-struct nsim_ethtool {
+struct nsim_ethtool_pauseparam {
bool rx;
bool tx;
bool report_stats_rx;
bool report_stats_tx;
};
+struct nsim_ethtool {
+ struct nsim_ethtool_pauseparam pauseparam;
+ struct ethtool_coalesce coalesce;
+ struct ethtool_ringparam ring;
+};
+
struct netdevsim {
struct net_device *netdev;
struct nsim_dev *nsim_dev;
--
2.28.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next v3 3/4] selftests: extract common functions in ethtool-common.sh
2020-11-13 23:16 [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Antonio Cardace
2020-11-13 23:16 ` [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings Antonio Cardace
@ 2020-11-13 23:16 ` Antonio Cardace
2020-11-16 16:17 ` Michal Kubecek
2020-11-13 23:16 ` [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests Antonio Cardace
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Antonio Cardace @ 2020-11-13 23:16 UTC (permalink / raw)
To: netdev, David S . Miller, Jakub Kicinski, Michal Kubecek
Factor out some useful functions so that they can be reused
by other ethtool-netdevsim scripts.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
---
.../drivers/net/netdevsim/ethtool-common.sh | 69 +++++++++++++++++++
.../drivers/net/netdevsim/ethtool-pause.sh | 63 +----------------
2 files changed, 71 insertions(+), 61 deletions(-)
create mode 100644 tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
new file mode 100644
index 000000000000..fa44cf6e732c
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
@@ -0,0 +1,69 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+NSIM_ID=$((RANDOM % 1024))
+NSIM_DEV_SYS=/sys/bus/netdevsim/devices/netdevsim$NSIM_ID
+NSIM_DEV_DFS=/sys/kernel/debug/netdevsim/netdevsim$NSIM_ID/ports/0
+NSIM_NETDEV=
+num_passes=0
+num_errors=0
+
+function cleanup_nsim {
+ if [ -e $NSIM_DEV_SYS ]; then
+ echo $NSIM_ID > /sys/bus/netdevsim/del_device
+ fi
+}
+
+function cleanup {
+ cleanup_nsim
+}
+
+trap cleanup EXIT
+
+function get_netdev_name {
+ local -n old=$1
+
+ new=$(ls /sys/class/net)
+
+ for netdev in $new; do
+ for check in $old; do
+ [ $netdev == $check ] && break
+ done
+
+ if [ $netdev != $check ]; then
+ echo $netdev
+ break
+ fi
+ done
+}
+
+function check {
+ local code=$1
+ local str=$2
+ local exp_str=$3
+
+ if [ $code -ne 0 ]; then
+ ((num_errors++))
+ return
+ fi
+
+ if [ "$str" != "$exp_str" ]; then
+ echo -e "Expected: '$exp_str', got '$str'"
+ ((num_errors++))
+ return
+ fi
+
+ ((num_passes++))
+}
+
+function make_netdev {
+ # Make a netdevsim
+ old_netdevs=$(ls /sys/class/net)
+
+ if ! $(lsmod | grep -q netdevsim); then
+ modprobe netdevsim
+ fi
+
+ echo $NSIM_ID > /sys/bus/netdevsim/new_device
+ echo `get_netdev_name old_netdevs`
+}
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-pause.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-pause.sh
index 25c896b9e2eb..b4a7abfe5454 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/ethtool-pause.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-pause.sh
@@ -1,60 +1,7 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0-only
-NSIM_ID=$((RANDOM % 1024))
-NSIM_DEV_SYS=/sys/bus/netdevsim/devices/netdevsim$NSIM_ID
-NSIM_DEV_DFS=/sys/kernel/debug/netdevsim/netdevsim$NSIM_ID/ports/0
-NSIM_NETDEV=
-num_passes=0
-num_errors=0
-
-function cleanup_nsim {
- if [ -e $NSIM_DEV_SYS ]; then
- echo $NSIM_ID > /sys/bus/netdevsim/del_device
- fi
-}
-
-function cleanup {
- cleanup_nsim
-}
-
-trap cleanup EXIT
-
-function get_netdev_name {
- local -n old=$1
-
- new=$(ls /sys/class/net)
-
- for netdev in $new; do
- for check in $old; do
- [ $netdev == $check ] && break
- done
-
- if [ $netdev != $check ]; then
- echo $netdev
- break
- fi
- done
-}
-
-function check {
- local code=$1
- local str=$2
- local exp_str=$3
-
- if [ $code -ne 0 ]; then
- ((num_errors++))
- return
- fi
-
- if [ "$str" != "$exp_str" ]; then
- echo -e "Expected: '$exp_str', got '$str'"
- ((num_errors++))
- return
- fi
-
- ((num_passes++))
-}
+source ethtool-common.sh
# Bail if ethtool is too old
if ! ethtool -h | grep include-stat 2>&1 >/dev/null; then
@@ -62,13 +9,7 @@ if ! ethtool -h | grep include-stat 2>&1 >/dev/null; then
exit 4
fi
-# Make a netdevsim
-old_netdevs=$(ls /sys/class/net)
-
-modprobe netdevsim
-echo $NSIM_ID > /sys/bus/netdevsim/new_device
-
-NSIM_NETDEV=`get_netdev_name old_netdevs`
+NSIM_NETDEV=$(make_netdev)
set -o pipefail
--
2.28.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests
2020-11-13 23:16 [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Antonio Cardace
2020-11-13 23:16 ` [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings Antonio Cardace
2020-11-13 23:16 ` [PATCH net-next v3 3/4] selftests: extract common functions in ethtool-common.sh Antonio Cardace
@ 2020-11-13 23:16 ` Antonio Cardace
2020-11-17 0:45 ` Jakub Kicinski
2020-11-16 16:02 ` [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Michal Kubecek
2020-11-17 0:48 ` Jakub Kicinski
4 siblings, 1 reply; 15+ messages in thread
From: Antonio Cardace @ 2020-11-13 23:16 UTC (permalink / raw)
To: netdev, David S . Miller, Jakub Kicinski, Michal Kubecek
Add scripts to test ring and coalesce settings
of netdevsim.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
---
.../drivers/net/netdevsim/ethtool-coalesce.sh | 68 +++++++++++++++++++
.../drivers/net/netdevsim/ethtool-ring.sh | 53 +++++++++++++++
2 files changed, 121 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh
create mode 100755 tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh
new file mode 100755
index 000000000000..3b322c99be69
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh
@@ -0,0 +1,68 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+source ethtool-common.sh
+
+function get_value {
+ local key=$1
+
+ echo $(ethtool -c $NSIM_NETDEV | \
+ awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
+}
+
+if ! ethtool -h | grep -q coalesce; then
+ echo "SKIP: No --coalesce support in ethtool"
+ exit 4
+fi
+
+NSIM_NETDEV=$(make_netdev)
+
+set -o pipefail
+
+declare -A SETTINGS_MAP=(
+ ["rx-frames-low"]="rx-frame-low"
+ ["tx-frames-low"]="tx-frame-low"
+ ["rx-frames-high"]="rx-frame-high"
+ ["tx-frames-high"]="tx-frame-high"
+ ["rx-usecs"]="rx-usecs"
+ ["rx-frames"]="rx-frames"
+ ["rx-usecs-irq"]="rx-usecs-irq"
+ ["rx-frames-irq"]="rx-frames-irq"
+ ["tx-usecs"]="tx-usecs"
+ ["tx-frames"]="tx-frames"
+ ["tx-usecs-irq"]="tx-usecs-irq"
+ ["tx-frames-irq"]="tx-frames-irq"
+ ["stats-block-usecs"]="stats-block-usecs"
+ ["pkt-rate-low"]="pkt-rate-low"
+ ["rx-usecs-low"]="rx-usecs-low"
+ ["tx-usecs-low"]="tx-usecs-low"
+ ["pkt-rate-high"]="pkt-rate-high"
+ ["rx-usecs-high"]="rx-usecs-high"
+ ["tx-usecs-high"]="tx-usecs-high"
+ ["sample-interval"]="sample-interval"
+)
+
+for key in ${!SETTINGS_MAP[@]}; do
+ query_key=${SETTINGS_MAP[$key]}
+ value=$((RANDOM % $((2**32-1))))
+ ethtool -C $NSIM_NETDEV "$key" "$value"
+ s=$(get_value "$query_key")
+ check $? "$s" "$value"
+done
+
+# bool settings which ethtool displays on the same line
+ethtool -C $NSIM_NETDEV adaptive-rx on
+s=$(ethtool -c $NSIM_NETDEV | grep -q "Adaptive RX: on TX: off")
+check $? "$s" ""
+
+ethtool -C $NSIM_NETDEV adaptive-tx on
+s=$(ethtool -c $NSIM_NETDEV | grep -q "Adaptive RX: on TX: on")
+check $? "$s" ""
+
+if [ $num_errors -eq 0 ]; then
+ echo "PASSED all $((num_passes)) checks"
+ exit 0
+else
+ echo "FAILED $num_errors/$((num_errors+num_passes)) checks"
+ exit 1
+fi
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh
new file mode 100755
index 000000000000..513b9875c637
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh
@@ -0,0 +1,53 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+source ethtool-common.sh
+
+function get_value {
+ local key=$1
+
+ echo $(ethtool -g $NSIM_NETDEV | \
+ tail -n +$CURR_SETT_LINE | \
+ awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[\t ]/, "", $2); print $2}')
+}
+
+if ! ethtool -h | grep -q set-ring >/dev/null; then
+ echo "SKIP: No --set-ring support in ethtool"
+ exit 4
+fi
+
+NSIM_NETDEV=$(make_netdev)
+
+set -o pipefail
+
+declare -A SETTINGS_MAP=(
+ ["rx"]="RX"
+ ["rx-mini"]="RX Mini"
+ ["rx-jumbo"]="RX Jumbo"
+ ["tx"]="TX"
+)
+
+MAX_VALUE=$((RANDOM % $((2**32-1))))
+RING_MAX_LIST=$(ls $NSIM_DEV_DFS/ethtool/ring/)
+
+for ring_max_entry in $RING_MAX_LIST; do
+ echo $MAX_VALUE > $NSIM_DEV_DFS/ethtool/ring/$ring_max_entry
+done
+
+CURR_SETT_LINE=$(ethtool -g $NSIM_NETDEV | grep -i -m1 -n 'Current hardware settings' | cut -f1 -d:)
+
+for key in ${!SETTINGS_MAP[@]}; do
+ query_key=${SETTINGS_MAP[$key]}
+ value=$((RANDOM % $MAX_VALUE))
+ ethtool -G $NSIM_NETDEV "$key" "$value"
+ s=$(get_value "$query_key")
+ check $? "$s" "$value"
+done
+
+if [ $num_errors -eq 0 ]; then
+ echo "PASSED all $((num_passes)) checks"
+ exit 0
+else
+ echo "FAILED $num_errors/$((num_errors+num_passes)) checks"
+ exit 1
+fi
--
2.28.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define
2020-11-13 23:16 [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Antonio Cardace
` (2 preceding siblings ...)
2020-11-13 23:16 ` [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests Antonio Cardace
@ 2020-11-16 16:02 ` Michal Kubecek
2020-11-17 0:48 ` Jakub Kicinski
4 siblings, 0 replies; 15+ messages in thread
From: Michal Kubecek @ 2020-11-16 16:02 UTC (permalink / raw)
To: Antonio Cardace; +Cc: netdev, David S . Miller, Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 930 bytes --]
On Sat, Nov 14, 2020 at 12:16:52AM +0100, Antonio Cardace wrote:
> This bitmask represents all existing coalesce parameters.
>
> Signed-off-by: Antonio Cardace <acardace@redhat.com>
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
> ---
> include/linux/ethtool.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 6408b446051f..e3da25b51ae4 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -215,6 +215,7 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
> #define ETHTOOL_COALESCE_TX_USECS_HIGH BIT(19)
> #define ETHTOOL_COALESCE_TX_MAX_FRAMES_HIGH BIT(20)
> #define ETHTOOL_COALESCE_RATE_SAMPLE_INTERVAL BIT(21)
> +#define ETHTOOL_COALESCE_ALL_PARAMS GENMASK(21, 0)
>
> #define ETHTOOL_COALESCE_USECS \
> (ETHTOOL_COALESCE_RX_USECS | ETHTOOL_COALESCE_TX_USECS)
> --
> 2.28.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings
2020-11-13 23:16 ` [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings Antonio Cardace
@ 2020-11-16 16:03 ` Michal Kubecek
2020-11-17 0:47 ` Jakub Kicinski
1 sibling, 0 replies; 15+ messages in thread
From: Michal Kubecek @ 2020-11-16 16:03 UTC (permalink / raw)
To: Antonio Cardace; +Cc: netdev, David S . Miller, Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 239 bytes --]
On Sat, Nov 14, 2020 at 12:16:53AM +0100, Antonio Cardace wrote:
> Add ethtool ring and coalesce settings support for testing.
>
> Signed-off-by: Antonio Cardace <acardace@redhat.com>
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 3/4] selftests: extract common functions in ethtool-common.sh
2020-11-13 23:16 ` [PATCH net-next v3 3/4] selftests: extract common functions in ethtool-common.sh Antonio Cardace
@ 2020-11-16 16:17 ` Michal Kubecek
2020-11-16 16:38 ` Antonio Cardace
0 siblings, 1 reply; 15+ messages in thread
From: Michal Kubecek @ 2020-11-16 16:17 UTC (permalink / raw)
To: Antonio Cardace; +Cc: netdev, David S . Miller, Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]
On Sat, Nov 14, 2020 at 12:16:54AM +0100, Antonio Cardace wrote:
> Factor out some useful functions so that they can be reused
> by other ethtool-netdevsim scripts.
>
> Signed-off-by: Antonio Cardace <acardace@redhat.com>
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
Just one comment:
[...]
> +function get_netdev_name {
> + local -n old=$1
> +
> + new=$(ls /sys/class/net)
> +
> + for netdev in $new; do
> + for check in $old; do
> + [ $netdev == $check ] && break
> + done
> +
> + if [ $netdev != $check ]; then
> + echo $netdev
> + break
> + fi
> + done
> +}
[...]
> +function make_netdev {
> + # Make a netdevsim
> + old_netdevs=$(ls /sys/class/net)
> +
> + if ! $(lsmod | grep -q netdevsim); then
> + modprobe netdevsim
> + fi
> +
> + echo $NSIM_ID > /sys/bus/netdevsim/new_device
> + echo `get_netdev_name old_netdevs`
> +}
This would be rather unpredictable if someone ran another selftest (or
anything else that would create a network device) in parallel. IMHO it
would be safer (and easier) to get the name of the new device from
/sys/bus/netdevsim/devices/netdevsim${NSIM_ID}/net/
But as this is not new code and you are just moving existing code, it
can be done in a separate patch.
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 3/4] selftests: extract common functions in ethtool-common.sh
2020-11-16 16:17 ` Michal Kubecek
@ 2020-11-16 16:38 ` Antonio Cardace
0 siblings, 0 replies; 15+ messages in thread
From: Antonio Cardace @ 2020-11-16 16:38 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, David S . Miller, Jakub Kicinski
On Mon, Nov 16, 2020 at 05:17:02PM +0100, Michal Kubecek wrote:
> On Sat, Nov 14, 2020 at 12:16:54AM +0100, Antonio Cardace wrote:
> > Factor out some useful functions so that they can be reused
> > by other ethtool-netdevsim scripts.
> >
> > Signed-off-by: Antonio Cardace <acardace@redhat.com>
>
> Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
>
> Just one comment:
>
> [...]
> > +function get_netdev_name {
> > + local -n old=$1
> > +
> > + new=$(ls /sys/class/net)
> > +
> > + for netdev in $new; do
> > + for check in $old; do
> > + [ $netdev == $check ] && break
> > + done
> > +
> > + if [ $netdev != $check ]; then
> > + echo $netdev
> > + break
> > + fi
> > + done
> > +}
> [...]
> > +function make_netdev {
> > + # Make a netdevsim
> > + old_netdevs=$(ls /sys/class/net)
> > +
> > + if ! $(lsmod | grep -q netdevsim); then
> > + modprobe netdevsim
> > + fi
> > +
> > + echo $NSIM_ID > /sys/bus/netdevsim/new_device
> > + echo `get_netdev_name old_netdevs`
> > +}
>
> This would be rather unpredictable if someone ran another selftest (or
> anything else that would create a network device) in parallel. IMHO it
> would be safer (and easier) to get the name of the new device from
>
> /sys/bus/netdevsim/devices/netdevsim${NSIM_ID}/net/
>
> But as this is not new code and you are just moving existing code, it
> can be done in a separate patch.
Yes it does make sense, I can send a patch for this once this is merged.
Thanks for the review.
Antonio
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests
2020-11-13 23:16 ` [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests Antonio Cardace
@ 2020-11-17 0:45 ` Jakub Kicinski
2020-11-17 11:32 ` Antonio Cardace
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2020-11-17 0:45 UTC (permalink / raw)
To: Antonio Cardace; +Cc: netdev, David S . Miller, Michal Kubecek
On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:
> Add scripts to test ring and coalesce settings
> of netdevsim.
>
> Signed-off-by: Antonio Cardace <acardace@redhat.com>
> @@ -0,0 +1,68 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +source ethtool-common.sh
> +
> +function get_value {
> + local key=$1
> +
> + echo $(ethtool -c $NSIM_NETDEV | \
> + awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> +}
> +
> +if ! ethtool -h | grep -q coalesce; then
> + echo "SKIP: No --coalesce support in ethtool"
> + exit 4
I think the skip exit code for selftests is 2
> +fi
> +
> +NSIM_NETDEV=$(make_netdev)
> +
> +set -o pipefail
> +
> +declare -A SETTINGS_MAP=(
> + ["rx-frames-low"]="rx-frame-low"
> + ["tx-frames-low"]="tx-frame-low"
> + ["rx-frames-high"]="rx-frame-high"
> + ["tx-frames-high"]="tx-frame-high"
> + ["rx-usecs"]="rx-usecs"
> + ["rx-frames"]="rx-frames"
> + ["rx-usecs-irq"]="rx-usecs-irq"
> + ["rx-frames-irq"]="rx-frames-irq"
> + ["tx-usecs"]="tx-usecs"
> + ["tx-frames"]="tx-frames"
> + ["tx-usecs-irq"]="tx-usecs-irq"
> + ["tx-frames-irq"]="tx-frames-irq"
> + ["stats-block-usecs"]="stats-block-usecs"
> + ["pkt-rate-low"]="pkt-rate-low"
> + ["rx-usecs-low"]="rx-usecs-low"
> + ["tx-usecs-low"]="tx-usecs-low"
> + ["pkt-rate-high"]="pkt-rate-high"
> + ["rx-usecs-high"]="rx-usecs-high"
> + ["tx-usecs-high"]="tx-usecs-high"
> + ["sample-interval"]="sample-interval"
> +)
> +
> +for key in ${!SETTINGS_MAP[@]}; do
> + query_key=${SETTINGS_MAP[$key]}
> + value=$((RANDOM % $((2**32-1))))
> + ethtool -C $NSIM_NETDEV "$key" "$value"
> + s=$(get_value "$query_key")
It would be better to validate the entire config, not just the most
recently set key. This way we would catch the cases where setting
attr breaks the value of another.
> + check $? "$s" "$value"
> +done
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings
2020-11-13 23:16 ` [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings Antonio Cardace
2020-11-16 16:03 ` Michal Kubecek
@ 2020-11-17 0:47 ` Jakub Kicinski
1 sibling, 0 replies; 15+ messages in thread
From: Jakub Kicinski @ 2020-11-17 0:47 UTC (permalink / raw)
To: Antonio Cardace; +Cc: netdev, David S . Miller, Michal Kubecek
On Sat, 14 Nov 2020 00:16:53 +0100 Antonio Cardace wrote:
> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
> index f1884d90a876..169154dba0cc 100644
> --- a/drivers/net/netdevsim/ethtool.c
> +++ b/drivers/net/netdevsim/ethtool.c
> @@ -13,9 +13,9 @@ nsim_get_pause_stats(struct net_device *dev,
> {
> struct netdevsim *ns = netdev_priv(dev);
>
> - if (ns->ethtool.report_stats_rx)
> + if (ns->ethtool.pauseparam.report_stats_rx)
> pause_stats->rx_pause_frames = 1;
> - if (ns->ethtool.report_stats_tx)
> + if (ns->ethtool.pauseparam.report_stats_tx)
> pause_stats->tx_pause_frames = 2;
> }
>
> @@ -25,8 +25,8 @@ nsim_get_pauseparam(struct net_device *dev, struct ethtool_pauseparam *pause)
> struct netdevsim *ns = netdev_priv(dev);
>
> pause->autoneg = 0; /* We don't support ksettings, so can't pretend */
> - pause->rx_pause = ns->ethtool.rx;
> - pause->tx_pause = ns->ethtool.tx;
> + pause->rx_pause = ns->ethtool.pauseparam.rx;
> + pause->tx_pause = ns->ethtool.pauseparam.tx;
> }
>
> static int
> @@ -37,28 +37,88 @@ nsim_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam *pause)
> if (pause->autoneg)
> return -EINVAL;
>
> - ns->ethtool.rx = pause->rx_pause;
> - ns->ethtool.tx = pause->tx_pause;
> + ns->ethtool.pauseparam.rx = pause->rx_pause;
> + ns->ethtool.pauseparam.tx = pause->tx_pause;
> + return 0;
> +}
Please separate the refactoring / moving pauseparam into another struct
out to its own patch. This makes review easier.
> +static int nsim_get_coalesce(struct net_device *dev,
> + struct ethtool_coalesce *coal)
> +{
> + struct netdevsim *ns = netdev_priv(dev);
> +
> + memcpy(coal, &ns->ethtool.coalesce, sizeof(ns->ethtool.coalesce));
> + return 0;
> +}
> +
> +static int nsim_set_coalesce(struct net_device *dev,
> + struct ethtool_coalesce *coal)
> +{
> + struct netdevsim *ns = netdev_priv(dev);
> +
> + memcpy(&ns->ethtool.coalesce, coal, sizeof(ns->ethtool.coalesce));
> + return 0;
> +}
> +
> +static void nsim_get_ringparam(struct net_device *dev,
> + struct ethtool_ringparam *ring)
> +{
> + struct netdevsim *ns = netdev_priv(dev);
> +
> + memcpy(ring, &ns->ethtool.ring, sizeof(ns->ethtool.ring));
> +}
> +
> +static int nsim_set_ringparam(struct net_device *dev,
> + struct ethtool_ringparam *ring)
> +{
> + struct netdevsim *ns = netdev_priv(dev);
> +
> + memcpy(&ns->ethtool.ring, ring, sizeof(ns->ethtool.ring));
> return 0;
> }
>
> static const struct ethtool_ops nsim_ethtool_ops = {
> - .get_pause_stats = nsim_get_pause_stats,
> - .get_pauseparam = nsim_get_pauseparam,
> - .set_pauseparam = nsim_set_pauseparam,
> + .get_pause_stats = nsim_get_pause_stats,
> + .get_pauseparam = nsim_get_pauseparam,
> + .set_pauseparam = nsim_set_pauseparam,
> + .supported_coalesce_params = ETHTOOL_COALESCE_ALL_PARAMS,
Please make this member first. I think that's what all drivers do.
> + .set_coalesce = nsim_set_coalesce,
> + .get_coalesce = nsim_get_coalesce,
> + .get_ringparam = nsim_get_ringparam,
> + .set_ringparam = nsim_set_ringparam,
> };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define
2020-11-13 23:16 [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Antonio Cardace
` (3 preceding siblings ...)
2020-11-16 16:02 ` [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Michal Kubecek
@ 2020-11-17 0:48 ` Jakub Kicinski
4 siblings, 0 replies; 15+ messages in thread
From: Jakub Kicinski @ 2020-11-17 0:48 UTC (permalink / raw)
To: Antonio Cardace; +Cc: netdev, David S . Miller, Michal Kubecek
Please add a cover letter to this series.
Preferably with the output of the test passing :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests
2020-11-17 0:45 ` Jakub Kicinski
@ 2020-11-17 11:32 ` Antonio Cardace
2020-11-17 17:15 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Antonio Cardace @ 2020-11-17 11:32 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, David S . Miller, Michal Kubecek
On Mon, Nov 16, 2020 at 04:45:03PM -0800, Jakub Kicinski wrote:
> On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:
> > Add scripts to test ring and coalesce settings
> > of netdevsim.
> >
> > Signed-off-by: Antonio Cardace <acardace@redhat.com>
>
> > @@ -0,0 +1,68 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +source ethtool-common.sh
> > +
> > +function get_value {
> > + local key=$1
> > +
> > + echo $(ethtool -c $NSIM_NETDEV | \
> > + awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> > +}
> > +
> > +if ! ethtool -h | grep -q coalesce; then
> > + echo "SKIP: No --coalesce support in ethtool"
> > + exit 4
>
> I think the skip exit code for selftests is 2
In the ethtool-pause.sh selftest the exit code is 4 (I copied it from
there), should I change that too?
>
> > +fi
> > +
> > +NSIM_NETDEV=$(make_netdev)
> > +
> > +set -o pipefail
> > +
> > +declare -A SETTINGS_MAP=(
> > + ["rx-frames-low"]="rx-frame-low"
> > + ["tx-frames-low"]="tx-frame-low"
> > + ["rx-frames-high"]="rx-frame-high"
> > + ["tx-frames-high"]="tx-frame-high"
> > + ["rx-usecs"]="rx-usecs"
> > + ["rx-frames"]="rx-frames"
> > + ["rx-usecs-irq"]="rx-usecs-irq"
> > + ["rx-frames-irq"]="rx-frames-irq"
> > + ["tx-usecs"]="tx-usecs"
> > + ["tx-frames"]="tx-frames"
> > + ["tx-usecs-irq"]="tx-usecs-irq"
> > + ["tx-frames-irq"]="tx-frames-irq"
> > + ["stats-block-usecs"]="stats-block-usecs"
> > + ["pkt-rate-low"]="pkt-rate-low"
> > + ["rx-usecs-low"]="rx-usecs-low"
> > + ["tx-usecs-low"]="tx-usecs-low"
> > + ["pkt-rate-high"]="pkt-rate-high"
> > + ["rx-usecs-high"]="rx-usecs-high"
> > + ["tx-usecs-high"]="tx-usecs-high"
> > + ["sample-interval"]="sample-interval"
> > +)
> > +
> > +for key in ${!SETTINGS_MAP[@]}; do
> > + query_key=${SETTINGS_MAP[$key]}
> > + value=$((RANDOM % $((2**32-1))))
> > + ethtool -C $NSIM_NETDEV "$key" "$value"
> > + s=$(get_value "$query_key")
>
> It would be better to validate the entire config, not just the most
> recently set key. This way we would catch the cases where setting
> attr breaks the value of another.
>
Good idea, will do.
Thanks,
Antonio
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests
2020-11-17 11:32 ` Antonio Cardace
@ 2020-11-17 17:15 ` Jakub Kicinski
2020-11-18 21:43 ` Willem de Bruijn
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2020-11-17 17:15 UTC (permalink / raw)
To: Antonio Cardace; +Cc: netdev, David S . Miller, Michal Kubecek
On Tue, 17 Nov 2020 12:32:36 +0100 Antonio Cardace wrote:
> On Mon, Nov 16, 2020 at 04:45:03PM -0800, Jakub Kicinski wrote:
> > On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:
> > > Add scripts to test ring and coalesce settings
> > > of netdevsim.
> > >
> > > Signed-off-by: Antonio Cardace <acardace@redhat.com>
> >
> > > @@ -0,0 +1,68 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +source ethtool-common.sh
> > > +
> > > +function get_value {
> > > + local key=$1
> > > +
> > > + echo $(ethtool -c $NSIM_NETDEV | \
> > > + awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> > > +}
> > > +
> > > +if ! ethtool -h | grep -q coalesce; then
> > > + echo "SKIP: No --coalesce support in ethtool"
> > > + exit 4
> >
> > I think the skip exit code for selftests is 2
> In the ethtool-pause.sh selftest the exit code is 4 (I copied it from
> there), should I change that too?
Sorry I misremembered it's 4. We can leave that as is.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests
2020-11-17 17:15 ` Jakub Kicinski
@ 2020-11-18 21:43 ` Willem de Bruijn
2020-11-18 23:19 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Willem de Bruijn @ 2020-11-18 21:43 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Antonio Cardace, Network Development, David S . Miller, Michal Kubecek
On Tue, Nov 17, 2020 at 12:19 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Nov 2020 12:32:36 +0100 Antonio Cardace wrote:
> > On Mon, Nov 16, 2020 at 04:45:03PM -0800, Jakub Kicinski wrote:
> > > On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:
> > > > Add scripts to test ring and coalesce settings
> > > > of netdevsim.
> > > >
> > > > Signed-off-by: Antonio Cardace <acardace@redhat.com>
> > >
> > > > @@ -0,0 +1,68 @@
> > > > +#!/bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > +
> > > > +source ethtool-common.sh
> > > > +
> > > > +function get_value {
> > > > + local key=$1
> > > > +
> > > > + echo $(ethtool -c $NSIM_NETDEV | \
> > > > + awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> > > > +}
> > > > +
> > > > +if ! ethtool -h | grep -q coalesce; then
> > > > + echo "SKIP: No --coalesce support in ethtool"
> > > > + exit 4
> > >
> > > I think the skip exit code for selftests is 2
> > In the ethtool-pause.sh selftest the exit code is 4 (I copied it from
> > there), should I change that too?
>
> Sorry I misremembered it's 4. We can leave that as is.
Instead of having to remember, maybe we should have a file in
tools/testing/selftest to define constants?
I defined them one-off in tools/testing/selftests/net/udpgso_bench.sh
readonly KSFT_PASS=0
readonly KSFT_FAIL=1
readonly KSFT_SKIP=4
along with some other kselftest shell support infra. But having each
test figure this out independently is duplicative and error prone.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests
2020-11-18 21:43 ` Willem de Bruijn
@ 2020-11-18 23:19 ` Jakub Kicinski
0 siblings, 0 replies; 15+ messages in thread
From: Jakub Kicinski @ 2020-11-18 23:19 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Antonio Cardace, Network Development, David S . Miller,
Michal Kubecek, linux-kselftest
On Wed, 18 Nov 2020 16:43:33 -0500 Willem de Bruijn wrote:
> On Tue, Nov 17, 2020 at 12:19 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > Sorry I misremembered it's 4. We can leave that as is.
>
> Instead of having to remember, maybe we should have a file in
> tools/testing/selftest to define constants?
>
> I defined them one-off in tools/testing/selftests/net/udpgso_bench.sh
>
> readonly KSFT_PASS=0
> readonly KSFT_FAIL=1
> readonly KSFT_SKIP=4
>
> along with some other kselftest shell support infra. But having each
> test figure this out independently is duplicative and error prone.
Sounds like a good idea, I was surprised it wasn't already defined in
any lib.
CCing the selftest ML.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-11-18 23:20 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 23:16 [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Antonio Cardace
2020-11-13 23:16 ` [PATCH net-next v3 2/4] netdevsim: support ethtool ring and coalesce settings Antonio Cardace
2020-11-16 16:03 ` Michal Kubecek
2020-11-17 0:47 ` Jakub Kicinski
2020-11-13 23:16 ` [PATCH net-next v3 3/4] selftests: extract common functions in ethtool-common.sh Antonio Cardace
2020-11-16 16:17 ` Michal Kubecek
2020-11-16 16:38 ` Antonio Cardace
2020-11-13 23:16 ` [PATCH net-next v3 4/4] selftests: add ring and coalesce selftests Antonio Cardace
2020-11-17 0:45 ` Jakub Kicinski
2020-11-17 11:32 ` Antonio Cardace
2020-11-17 17:15 ` Jakub Kicinski
2020-11-18 21:43 ` Willem de Bruijn
2020-11-18 23:19 ` Jakub Kicinski
2020-11-16 16:02 ` [PATCH net-next v3 1/4] ethtool: add ETHTOOL_COALESCE_ALL_PARAMS define Michal Kubecek
2020-11-17 0:48 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.