On 24 July 2020 19:15:17 EEST, Stephen Hemminger <step...@networkplumber.org>
wrote:
>
>The bridge portion of ip command was not scaling so the
>values were off.
>
>The netlink API's for setting and reading timers all conform
>to the kernel standard of scaling the values by USER_HZ (100).
>
>Fixes: 28d84b429e4e ("add bridge master device support")
>Fixes: 7f3d55922645 ("iplink: bridge: add support for
>IFLA_BR_MCAST_MEMBERSHIP_INTVL")
>Fixes: 10082a253fb2 ("iplink: bridge: add support for
>IFLA_BR_MCAST_LAST_MEMBER_INTVL")
>Fixes: 1f2244b851dd ("iplink: bridge: add support for
>IFLA_BR_MCAST_QUERIER_INTVL")
>Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
>---
While I agree this should have been done from the start, it's too late to
change.
We'll break everyone using these commands.
We have been discussing to add _ms version of all these which do the proper
scaling. I'd prefer that, it's least disruptive
to users.
Every user of the old commands scales the values by now.
>
>Compile tested only.
>
>
> ip/iplink_bridge.c | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
>diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
>index 3e81aa059cb3..48495a08c484 100644
>--- a/ip/iplink_bridge.c
>+++ b/ip/iplink_bridge.c
>@@ -24,6 +24,7 @@
>
> static unsigned int xstats_print_attr;
> static int filter_index;
>+static unsigned int hz;
>
> static void print_explain(FILE *f)
> {
>@@ -85,19 +86,22 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> {
> __u32 val;
>
>+ if (!hz)
>+ hz = get_user_hz();
>+
> while (argc > 0) {
> if (matches(*argv, "forward_delay") == 0) {
> NEXT_ARG();
> if (get_u32(&val, *argv, 0))
> invarg("invalid forward_delay", *argv);
>
>- addattr32(n, 1024, IFLA_BR_FORWARD_DELAY, val);
>+ addattr32(n, 1024, IFLA_BR_FORWARD_DELAY, val * hz);
> } else if (matches(*argv, "hello_time") == 0) {
> NEXT_ARG();
> if (get_u32(&val, *argv, 0))
> invarg("invalid hello_time", *argv);
>
>- addattr32(n, 1024, IFLA_BR_HELLO_TIME, val);
>+ addattr32(n, 1024, IFLA_BR_HELLO_TIME, val * hz);
> } else if (matches(*argv, "max_age") == 0) {
> NEXT_ARG();
> if (get_u32(&val, *argv, 0))
>@@ -109,7 +113,7 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> if (get_u32(&val, *argv, 0))
> invarg("invalid ageing_time", *argv);
>
>- addattr32(n, 1024, IFLA_BR_AGEING_TIME, val);
>+ addattr32(n, 1024, IFLA_BR_AGEING_TIME, val * hz);
> } else if (matches(*argv, "stp_state") == 0) {
> NEXT_ARG();
> if (get_u32(&val, *argv, 0))
>@@ -266,7 +270,7 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> *argv);
>
> addattr64(n, 1024, IFLA_BR_MCAST_LAST_MEMBER_INTVL,
>- mcast_last_member_intvl);
>+ mcast_last_member_intvl * hz);
> } else if (matches(*argv, "mcast_membership_interval") == 0) {
> __u64 mcast_membership_intvl;
>
>@@ -276,7 +280,7 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> *argv);
>
> addattr64(n, 1024, IFLA_BR_MCAST_MEMBERSHIP_INTVL,
>- mcast_membership_intvl);
>+ mcast_membership_intvl * hz);
> } else if (matches(*argv, "mcast_querier_interval") == 0) {
> __u64 mcast_querier_intvl;
>
>@@ -286,7 +290,7 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> *argv);
>
> addattr64(n, 1024, IFLA_BR_MCAST_QUERIER_INTVL,
>- mcast_querier_intvl);
>+ mcast_querier_intvl * hz);
> } else if (matches(*argv, "mcast_query_interval") == 0) {
> __u64 mcast_query_intvl;
>
>@@ -296,7 +300,7 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> *argv);
>
> addattr64(n, 1024, IFLA_BR_MCAST_QUERY_INTVL,
>- mcast_query_intvl);
>+ mcast_query_intvl * hz);
> } else if (!matches(*argv, "mcast_query_response_interval")) {
> __u64 mcast_query_resp_intvl;
>
>@@ -306,7 +310,7 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> *argv);
>
> addattr64(n, 1024, IFLA_BR_MCAST_QUERY_RESPONSE_INTVL,
>- mcast_query_resp_intvl);
>+ mcast_query_resp_intvl * hz);
> } else if (!matches(*argv, "mcast_startup_query_interval")) {
> __u64 mcast_startup_query_intvl;
>
>@@ -316,7 +320,7 @@ static int bridge_parse_opt(struct link_util *lu,
>int argc, char **argv,
> *argv);
>
> addattr64(n, 1024, IFLA_BR_MCAST_STARTUP_QUERY_INTVL,
>- mcast_startup_query_intvl);
>+ mcast_startup_query_intvl * hz);
> } else if (matches(*argv, "mcast_stats_enabled") == 0) {
> __u8 mcast_stats_enabled;
>
>@@ -407,29 +411,32 @@ static void bridge_print_opt(struct link_util
>*lu, FILE *f, struct rtattr *tb[])
> if (!tb)
> return;
>
>+ if (!hz)
>+ hz = get_user_hz();
>+
> if (tb[IFLA_BR_FORWARD_DELAY])
> print_uint(PRINT_ANY,
> "forward_delay",
> "forward_delay %u ",
>- rta_getattr_u32(tb[IFLA_BR_FORWARD_DELAY]));
>+ rta_getattr_u32(tb[IFLA_BR_FORWARD_DELAY]) / hz);
>
> if (tb[IFLA_BR_HELLO_TIME])
> print_uint(PRINT_ANY,
> "hello_time",
> "hello_time %u ",
>- rta_getattr_u32(tb[IFLA_BR_HELLO_TIME]));
>+ rta_getattr_u32(tb[IFLA_BR_HELLO_TIME]) / hz);
>
> if (tb[IFLA_BR_MAX_AGE])
> print_uint(PRINT_ANY,
> "max_age",
> "max_age %u ",
>- rta_getattr_u32(tb[IFLA_BR_MAX_AGE]));
>+ rta_getattr_u32(tb[IFLA_BR_MAX_AGE]) / hz);
>
> if (tb[IFLA_BR_AGEING_TIME])
> print_uint(PRINT_ANY,
> "ageing_time",
> "ageing_time %u ",
>- rta_getattr_u32(tb[IFLA_BR_AGEING_TIME]));
>+ rta_getattr_u32(tb[IFLA_BR_AGEING_TIME]) / hz);
>
> if (tb[IFLA_BR_STP_STATE])
> print_uint(PRINT_ANY,
>@@ -605,37 +612,37 @@ static void bridge_print_opt(struct link_util
>*lu, FILE *f, struct rtattr *tb[])
> print_lluint(PRINT_ANY,
> "mcast_last_member_intvl",
> "mcast_last_member_interval %llu ",
>-
>rta_getattr_u64(tb[IFLA_BR_MCAST_LAST_MEMBER_INTVL]));
>+
>rta_getattr_u64(tb[IFLA_BR_MCAST_LAST_MEMBER_INTVL]) / hz);
>
> if (tb[IFLA_BR_MCAST_MEMBERSHIP_INTVL])
> print_lluint(PRINT_ANY,
> "mcast_membership_intvl",
> "mcast_membership_interval %llu ",
>-
>rta_getattr_u64(tb[IFLA_BR_MCAST_MEMBERSHIP_INTVL]));
>+
>rta_getattr_u64(tb[IFLA_BR_MCAST_MEMBERSHIP_INTVL]) / hz);
>
> if (tb[IFLA_BR_MCAST_QUERIER_INTVL])
> print_lluint(PRINT_ANY,
> "mcast_querier_intvl",
> "mcast_querier_interval %llu ",
>- rta_getattr_u64(tb[IFLA_BR_MCAST_QUERIER_INTVL]));
>+ rta_getattr_u64(tb[IFLA_BR_MCAST_QUERIER_INTVL]) /
>hz);
>
> if (tb[IFLA_BR_MCAST_QUERY_INTVL])
> print_lluint(PRINT_ANY,
> "mcast_query_intvl",
> "mcast_query_interval %llu ",
>- rta_getattr_u64(tb[IFLA_BR_MCAST_QUERY_INTVL]));
>+ rta_getattr_u64(tb[IFLA_BR_MCAST_QUERY_INTVL]) /
>hz);
>
> if (tb[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL])
> print_lluint(PRINT_ANY,
> "mcast_query_response_intvl",
> "mcast_query_response_interval %llu ",
>-
>rta_getattr_u64(tb[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]));
>+
>rta_getattr_u64(tb[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]) / hz);
>
> if (tb[IFLA_BR_MCAST_STARTUP_QUERY_INTVL])
> print_lluint(PRINT_ANY,
> "mcast_startup_query_intvl",
> "mcast_startup_query_interval %llu ",
>-
>rta_getattr_u64(tb[IFLA_BR_MCAST_STARTUP_QUERY_INTVL]));
>+
>rta_getattr_u64(tb[IFLA_BR_MCAST_STARTUP_QUERY_INTVL]) / hz);
>
> if (tb[IFLA_BR_MCAST_STATS_ENABLED])
> print_uint(PRINT_ANY,
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
