* [PATCHv2 net 1/4] bonding: fix send_peer_notif overflow
2023-04-27 3:39 [PATCHv2 net 0/4] bonding: fix send_peer_notif overflow Hangbin Liu
@ 2023-04-27 3:39 ` Hangbin Liu
2023-04-28 20:04 ` Simon Horman
2023-04-27 3:39 ` [PATCHv2 net 2/4] Documentation: bonding: fix the doc of peer_notif_delay Hangbin Liu
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Hangbin Liu @ 2023-04-27 3:39 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, Vincent Bernat, Hangbin Liu
Bonding send_peer_notif was defined as u8. Since commit 07a4ddec3ce9
("bonding: add an option to specify a delay between peer notifications").
the bond->send_peer_notif will be num_peer_notif multiplied by
peer_notif_delay, which is u8 * u32. This would cause the send_peer_notif
overflow easily. e.g.
ip link add bond0 type bond mode 1 miimon 100 num_grat_arp 30 peer_notify_delay 1000
To fix the overflow, let's set the send_peer_notif to u32 and limit
peer_notif_delay to 300s.
Fixes: 07a4ddec3ce9 ("bonding: add an option to specify a delay between peer notifications")
Reported-by: Liang Li <liali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
v2: define send_peer_notif as u32 and limit the peer_notif_delay to 300s
---
drivers/net/bonding/bond_netlink.c | 6 ++++++
drivers/net/bonding/bond_options.c | 8 +++++++-
include/net/bonding.h | 2 +-
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index c2d080fc4fc4..09a501cdea0c 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -244,6 +244,12 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
if (data[IFLA_BOND_PEER_NOTIF_DELAY]) {
int delay = nla_get_u32(data[IFLA_BOND_PEER_NOTIF_DELAY]);
+ if (delay > 300000) {
+ NL_SET_ERR_MSG_ATTR(extack, data[IFLA_BOND_PEER_NOTIF_DELAY],
+ "peer_notif_delay should be less than 300s");
+ return -EINVAL;
+ }
+
bond_opt_initval(&newval, delay);
err = __bond_opt_set(bond, BOND_OPT_PEER_NOTIF_DELAY, &newval,
data[IFLA_BOND_PEER_NOTIF_DELAY], extack);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index f71d5517f829..5310cb488f11 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -169,6 +169,12 @@ static const struct bond_opt_value bond_num_peer_notif_tbl[] = {
{ NULL, -1, 0}
};
+static const struct bond_opt_value bond_peer_notif_delay_tbl[] = {
+ { "off", 0, 0},
+ { "maxval", 300000, BOND_VALFLAG_MAX},
+ { NULL, -1, 0}
+};
+
static const struct bond_opt_value bond_primary_reselect_tbl[] = {
{ "always", BOND_PRI_RESELECT_ALWAYS, BOND_VALFLAG_DEFAULT},
{ "better", BOND_PRI_RESELECT_BETTER, 0},
@@ -488,7 +494,7 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
.id = BOND_OPT_PEER_NOTIF_DELAY,
.name = "peer_notif_delay",
.desc = "Delay between each peer notification on failover event, in milliseconds",
- .values = bond_intmax_tbl,
+ .values = bond_peer_notif_delay_tbl,
.set = bond_option_peer_notif_delay_set
}
};
diff --git a/include/net/bonding.h b/include/net/bonding.h
index c3843239517d..2d034e07b796 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -233,7 +233,7 @@ struct bonding {
*/
spinlock_t mode_lock;
spinlock_t stats_lock;
- u8 send_peer_notif;
+ u32 send_peer_notif;
u8 igmp_retrans;
#ifdef CONFIG_PROC_FS
struct proc_dir_entry *proc_entry;
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv2 net 2/4] Documentation: bonding: fix the doc of peer_notif_delay
2023-04-27 3:39 [PATCHv2 net 0/4] bonding: fix send_peer_notif overflow Hangbin Liu
2023-04-27 3:39 ` [PATCHv2 net 1/4] " Hangbin Liu
@ 2023-04-27 3:39 ` Hangbin Liu
2023-04-27 3:39 ` [PATCHv2 net 3/4] selftests: forwarding: lib: add netns support for tc rule handle stats get Hangbin Liu
2023-04-27 3:39 ` [PATCHv2 net 4/4] kselftest: bonding: add num_grat_arp test Hangbin Liu
3 siblings, 0 replies; 8+ messages in thread
From: Hangbin Liu @ 2023-04-27 3:39 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, Vincent Bernat, Hangbin Liu
Bonding only supports setting peer_notif_delay with miimon set.
Fixes: 0307d589c4d6 ("bonding: add documentation for peer_notif_delay")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
Documentation/networking/bonding.rst | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
index adc4bf4f3c50..28925e19622d 100644
--- a/Documentation/networking/bonding.rst
+++ b/Documentation/networking/bonding.rst
@@ -776,10 +776,11 @@ peer_notif_delay
Specify the delay, in milliseconds, between each peer
notification (gratuitous ARP and unsolicited IPv6 Neighbor
Advertisement) when they are issued after a failover event.
- This delay should be a multiple of the link monitor interval
- (arp_interval or miimon, whichever is active). The default
- value is 0 which means to match the value of the link monitor
- interval.
+ This delay should be a multiple of the MII link monitor interval
+ (miimon).
+
+ The valid range is 0 - 300000. The default value is 0, which means
+ to match the value of the MII link monitor interval.
prio
Slave priority. A higher number means higher priority.
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv2 net 3/4] selftests: forwarding: lib: add netns support for tc rule handle stats get
2023-04-27 3:39 [PATCHv2 net 0/4] bonding: fix send_peer_notif overflow Hangbin Liu
2023-04-27 3:39 ` [PATCHv2 net 1/4] " Hangbin Liu
2023-04-27 3:39 ` [PATCHv2 net 2/4] Documentation: bonding: fix the doc of peer_notif_delay Hangbin Liu
@ 2023-04-27 3:39 ` Hangbin Liu
2023-04-28 20:08 ` Simon Horman
2023-04-27 3:39 ` [PATCHv2 net 4/4] kselftest: bonding: add num_grat_arp test Hangbin Liu
3 siblings, 1 reply; 8+ messages in thread
From: Hangbin Liu @ 2023-04-27 3:39 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, Vincent Bernat, Hangbin Liu
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index d47499ba81c7..426bab05fe0a 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -773,8 +773,9 @@ tc_rule_handle_stats_get()
local id=$1; shift
local handle=$1; shift
local selector=${1:-.packets}; shift
+ local netns=${1:-""}; shift
- tc -j -s filter show $id \
+ tc $netns -j -s filter show $id \
| jq ".[] | select(.options.handle == $handle) | \
.options.actions[0].stats$selector"
}
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv2 net 4/4] kselftest: bonding: add num_grat_arp test
2023-04-27 3:39 [PATCHv2 net 0/4] bonding: fix send_peer_notif overflow Hangbin Liu
` (2 preceding siblings ...)
2023-04-27 3:39 ` [PATCHv2 net 3/4] selftests: forwarding: lib: add netns support for tc rule handle stats get Hangbin Liu
@ 2023-04-27 3:39 ` Hangbin Liu
3 siblings, 0 replies; 8+ messages in thread
From: Hangbin Liu @ 2023-04-27 3:39 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, Vincent Bernat, Hangbin Liu
TEST: num_grat_arp (active-backup miimon num_grat_arp 10) [ OK ]
TEST: num_grat_arp (active-backup miimon num_grat_arp 20) [ OK ]
TEST: num_grat_arp (active-backup miimon num_grat_arp 30) [ OK ]
TEST: num_grat_arp (active-backup miimon num_grat_arp 50) [ OK ]
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
.../drivers/net/bonding/bond_options.sh | 50 +++++++++++++++++++
.../drivers/net/bonding/bond_topo_3d1c.sh | 2 +
2 files changed, 52 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/bonding/bond_options.sh b/tools/testing/selftests/drivers/net/bonding/bond_options.sh
index db29a3146a86..607ba5c38977 100755
--- a/tools/testing/selftests/drivers/net/bonding/bond_options.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond_options.sh
@@ -6,6 +6,7 @@
ALL_TESTS="
prio
arp_validate
+ num_grat_arp
"
REQUIRE_MZ=no
@@ -255,6 +256,55 @@ arp_validate()
arp_validate_ns "active-backup"
}
+garp_test()
+{
+ local param="$1"
+ local active_slave exp_num real_num i
+ RET=0
+
+ # create bond
+ bond_reset "${param}"
+
+ bond_check_connection
+ [ $RET -ne 0 ] && log_test "num_grat_arp" "$retmsg"
+
+
+ # Add tc rules to count GARP number
+ for i in $(seq 0 2); do
+ tc -n ${g_ns} filter add dev s$i ingress protocol arp pref 1 handle 101 \
+ flower skip_hw arp_op request arp_sip ${s_ip4} arp_tip ${s_ip4} action pass
+ done
+
+ # Do failover
+ active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
+ ip -n ${s_ns} link set ${active_slave} down
+
+ exp_num=$(echo "${param}" | cut -f6 -d ' ')
+ sleep $((exp_num + 2))
+
+ active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
+
+ # check result
+ real_num=$(tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}")
+ if [ "${real_num}" -ne "${exp_num}" ]; then
+ echo "$real_num garp packets sent on active slave ${active_slave}"
+ RET=1
+ fi
+
+ for i in $(seq 0 2); do
+ tc -n ${g_ns} filter del dev s$i ingress
+ done
+}
+
+num_grat_arp()
+{
+ local val
+ for val in 10 20 30 50; do
+ garp_test "mode active-backup miimon 100 num_grat_arp $val peer_notify_delay 1000"
+ log_test "num_grat_arp" "active-backup miimon num_grat_arp $val"
+ done
+}
+
trap cleanup EXIT
setup_prepare
diff --git a/tools/testing/selftests/drivers/net/bonding/bond_topo_3d1c.sh b/tools/testing/selftests/drivers/net/bonding/bond_topo_3d1c.sh
index 4045ca97fb22..69ab99a56043 100644
--- a/tools/testing/selftests/drivers/net/bonding/bond_topo_3d1c.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond_topo_3d1c.sh
@@ -61,6 +61,8 @@ server_create()
ip -n ${g_ns} link set s${i} up
ip -n ${g_ns} link set s${i} master br0
ip -n ${s_ns} link set eth${i} master bond0
+
+ tc -n ${g_ns} qdisc add dev s${i} clsact
done
ip -n ${s_ns} link set bond0 up
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread