lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Jan 2014 22:18:24 +0100 From: Nikolay Aleksandrov <nikolay@...hat.com> To: Zoltan Kiss <zoltan.kiss@...rix.com>, Jay Vosburgh <fubar@...ibm.com>, Veaceslav Falico <vfalico@...hat.com>, Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next] bonding: Use do_div to divide 64 bit numbers On 01/23/2014 09:44 PM, Nikolay Aleksandrov wrote: > On 01/23/2014 07:47 PM, Zoltan Kiss wrote: >> Nikolay Aleksandrov's recent bonding option API changes (25a9b54a and e4994612) >> introduced u64 as the type of downdelay and updelay. On 32 bit the division and >> modulo operations cause compile errors: >> >> ERROR: "__udivdi3" [drivers/net/bonding/bonding.ko] undefined! >> ERROR: "__umoddi3" [drivers/net/bonding/bonding.ko] undefined! >> >> This patch use the do_div macro, which guaranteed to do the right thing. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@...rix.com> >> --- >> drivers/net/bonding/bond_options.c | 19 +++++++++++-------- >> 1 file changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c >> index 4cee04a..4f94907 100644 >> --- a/drivers/net/bonding/bond_options.c >> +++ b/drivers/net/bonding/bond_options.c >> @@ -18,6 +18,7 @@ >> #include <linux/rcupdate.h> >> #include <linux/ctype.h> >> #include <linux/inet.h> >> +#include <asm/div64.h> >> #include "bonding.h" >> >> static struct bond_opt_value bond_mode_tbl[] = { >> @@ -727,19 +728,20 @@ int bond_option_miimon_set(struct bonding *bond, struct bond_opt_value *newval) >> >> int bond_option_updelay_set(struct bonding *bond, struct bond_opt_value *newval) >> { >> + u64 quotient = newval->value; >> + u64 remainder = do_div(quotient, bond->params.miimon); > Hi Zoltan, > Thanks for fixing this, a few comments though: > bond->params.miimon can be 0 here that's why there's a check afterwards, > also please separate the local variable definitions from the body with a > new line. > The same applies for downdelay. > > Nik In fact since we don't need the u64 and newval->value is limited to INT_MAX, can't we simply cast it to (int) and avoid the do_div entirely ? >> if (!bond->params.miimon) { >> pr_err("%s: Unable to set up delay as MII monitoring is disabled\n", >> bond->dev->name); >> return -EPERM; >> } >> - if ((newval->value % bond->params.miimon) != 0) { >> + if (remainder != 0) { >> pr_warn("%s: Warning: up delay (%llu) is not a multiple of miimon (%d), updelay rounded to %llu ms\n", >> bond->dev->name, newval->value, >> bond->params.miimon, >> - (newval->value / bond->params.miimon) * >> - bond->params.miimon); >> + quotient * bond->params.miimon); >> } >> - bond->params.updelay = newval->value / bond->params.miimon; >> + bond->params.updelay = quotient; >> pr_info("%s: Setting up delay to %d.\n", >> bond->dev->name, >> bond->params.updelay * bond->params.miimon); >> @@ -750,19 +752,20 @@ int bond_option_updelay_set(struct bonding *bond, struct bond_opt_value *newval) >> int bond_option_downdelay_set(struct bonding *bond, >> struct bond_opt_value *newval) >> { >> + u64 quotient = newval->value; >> + u64 remainder = do_div(quotient, bond->params.miimon); >> if (!bond->params.miimon) { >> pr_err("%s: Unable to set down delay as MII monitoring is disabled\n", >> bond->dev->name); >> return -EPERM; >> } >> - if ((newval->value % bond->params.miimon) != 0) { >> + if (remainder != 0) { >> pr_warn("%s: Warning: down delay (%llu) is not a multiple of miimon (%d), delay rounded to %llu ms\n", >> bond->dev->name, newval->value, >> bond->params.miimon, >> - (newval->value / bond->params.miimon) * >> - bond->params.miimon); >> + quotient * bond->params.miimon); >> } >> - bond->params.downdelay = newval->value / bond->params.miimon; >> + bond->params.downdelay = quotient; >> pr_info("%s: Setting down delay to %d.\n", >> bond->dev->name, >> bond->params.downdelay * bond->params.miimon); >> > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists