Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
Hi Gustavo,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Gustavo-A-R-Silva/tcp_lp-use-64-bit-arithmetic-instead-of-32-bit/20180202-135349
config: i386-randconfig-i0-201804 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
net/ipv4/tcp_lp.o: In function `tcp_lp_remote_hz_estimator':
>> net/ipv4/tcp_lp.c:150: undefined reference to `__divdi3'
vim +150 net/ipv4/tcp_lp.c
125
126 /**
127 * tcp_lp_remote_hz_estimator
128 *
129 * Estimate remote HZ.
130 * We keep on updating the estimated value, where original TCP-LP
131 * implementation only guest it for once and use forever.
132 */
133 static u32 tcp_lp_remote_hz_estimator(struct sock *sk)
134 {
135 struct tcp_sock *tp = tcp_sk(sk);
136 struct lp *lp = inet_csk_ca(sk);
137 s64 rhz = (s64)lp->remote_hz << 6; /* remote HZ << 6 */
138 s64 m = 0;
139
140 /* not yet record reference time
141 * go away!! record it before come back!! */
142 if (lp->remote_ref_time == 0 || lp->local_ref_time == 0)
143 goto out;
144
145 /* we can't calc remote HZ with no different!! */
146 if (tp->rx_opt.rcv_tsval == lp->remote_ref_time ||
147 tp->rx_opt.rcv_tsecr == lp->local_ref_time)
148 goto out;
149
> 150 m = (s64)TCP_TS_HZ *
151 (tp->rx_opt.rcv_tsval - lp->remote_ref_time) /
152 (tp->rx_opt.rcv_tsecr - lp->local_ref_time);
153 if (m < 0)
154 m = -m;
155
156 if (rhz > 0) {
157 m -= rhz >> 6; /* m is now error in remote HZ est */
158 rhz += m; /* 63/64 old + 1/64 new */
159 } else
160 rhz = m << 6;
161
162 out:
163 /* record time for successful remote HZ calc */
164 if ((rhz >> 6) > 0)
165 lp->flag |= LP_VALID_RHZ;
166 else
167 lp->flag &= ~LP_VALID_RHZ;
168
169 /* record reference time stamp */
170 lp->remote_ref_time = tp->rx_opt.rcv_tsval;
171 lp->local_ref_time = tp->rx_opt.rcv_tsecr;
172
173 return rhz >> 6;
174 }
175
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
Hi Gustavo, Thank you for the patch! Yet something to improve: [auto build test ERROR on net/master] [also build test ERROR on v4.15 next-20180202] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Gustavo-A-R-Silva/tcp_lp-use-64-bit-arithmetic-instead-of-32-bit/20180202-135349 config: i386-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): WARNING: modpost: missing MODULE_LICENSE() in drivers/auxdisplay/img-ascii-lcd.o see include/linux/module.h for more information WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/accel/kxsd9-i2c.o see include/linux/module.h for more information WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/adc/qcom-vadc-common.o see include/linux/module.h for more information WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/mtk-vcodec/mtk-vcodec-common.o see include/linux/module.h for more information WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/soc_camera/soc_scale_crop.o see include/linux/module.h for more information WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/tegra-cec/tegra_cec.o see include/linux/module.h for more information WARNING: modpost: missing MODULE_LICENSE() in drivers/pinctrl/pxa/pinctrl-pxa2xx.o see include/linux/module.h for more information >> ERROR: "__divdi3" [net/ipv4/tcp_lp.ko] undefined! >> ERROR: "__udivdi3" [net/ipv4/tcp_lp.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
Hi Andrew, Quoting Andrew Lunn : On Wed, Jan 31, 2018 at 07:07:49PM -0600, Gustavo A. R. Silva wrote: Hi Alan, Quoting Alan Cox : >On Wed, 31 Jan 2018 18:24:07 -0600 >"Gustavo A. R. Silva" wrote: > >>Cast to s64 some variables and a macro in order to give the >>compiler complete information about the proper arithmetic to >>use. Notice that these elements are used in contexts that >>expect expressions of type s64 (64 bits, signed). >> >>Currently such expression are being evaluated using 32-bit >>arithmetic. > >The question you need to ask is 'can it overflow 32bit maths', otherwise >you are potentially making the system do extra work for no reason. > Yeah, I get your point and it seems that in this particular case there is no risk of a 32bit overflow, but in general and IMHO as the code evolves, the use of incorrect arithmetic may have security implications in the future, so I advocate for code correctness in this case. Hi Gustavo Is this on the hotpath? How much overhead does it add to 32 bit architectures which don't have 64 bit arithmetic in hardware? There are a lot of embedded systems which are 32 bit. I'm sorry, I don't have access to 32-bit hardware at the moment. Thanks -- Gustavo
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
Hi David, Quoting David Miller : From: "Gustavo A. R. Silva" Date: Wed, 31 Jan 2018 18:24:07 -0600 Cast to s64 some variables and a macro in order to give the compiler complete information about the proper arithmetic to use. Notice that these elements are used in contexts that expect expressions of type s64 (64 bits, signed). Currently such expression are being evaluated using 32-bit arithmetic. Addresses-Coverity-ID: 200687 Addresses-Coverity-ID: 200688 Addresses-Coverity-ID: 200689 Signed-off-by: Gustavo A. R. Silva Sorry, I'm not applying this, the domain of the input values means that the calculation cannot overflow. Yep, that's fine. Thanks for the feedback. -- Gustavo
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
Hi David, Quoting David Laight : > The question you need to ask is 'can it overflow 32bit maths', otherwise > you are potentially making the system do extra work for no reason. > Yeah, I get your point and it seems that in this particular case there is no risk of a 32bit overflow, but in general and IMHO as the code evolves, the use of incorrect arithmetic may have security implications in the future, so I advocate for code correctness in this case. Even if the variable are 64bit you still need to worry (maybe less) about arithmetic overflow. The only real way to avoid overflow is to understand the domain of the values being used. Yep, that's correct. Thanks for the feedback. -- Gustavo
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
From: "Gustavo A. R. Silva" Date: Wed, 31 Jan 2018 18:24:07 -0600 > Cast to s64 some variables and a macro in order to give the > compiler complete information about the proper arithmetic to > use. Notice that these elements are used in contexts that > expect expressions of type s64 (64 bits, signed). > > Currently such expression are being evaluated using 32-bit > arithmetic. > > Addresses-Coverity-ID: 200687 > Addresses-Coverity-ID: 200688 > Addresses-Coverity-ID: 200689 > Signed-off-by: Gustavo A. R. Silva Sorry, I'm not applying this, the domain of the input values means that the calculation cannot overflow.
RE: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
> > The question you need to ask is 'can it overflow 32bit maths', otherwise > > you are potentially making the system do extra work for no reason. > > > > Yeah, I get your point and it seems that in this particular case there > is no risk of a 32bit overflow, but in general and IMHO as the code > evolves, the use of incorrect arithmetic may have security > implications in the future, so I advocate for code correctness in this > case. Even if the variable are 64bit you still need to worry (maybe less) about arithmetic overflow. The only real way to avoid overflow is to understand the domain of the values being used. David
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
On Wed, Jan 31, 2018 at 07:07:49PM -0600, Gustavo A. R. Silva wrote: > > Hi Alan, > > Quoting Alan Cox : > > >On Wed, 31 Jan 2018 18:24:07 -0600 > >"Gustavo A. R. Silva" wrote: > > > >>Cast to s64 some variables and a macro in order to give the > >>compiler complete information about the proper arithmetic to > >>use. Notice that these elements are used in contexts that > >>expect expressions of type s64 (64 bits, signed). > >> > >>Currently such expression are being evaluated using 32-bit > >>arithmetic. > > > >The question you need to ask is 'can it overflow 32bit maths', otherwise > >you are potentially making the system do extra work for no reason. > > > > Yeah, I get your point and it seems that in this particular case there is no > risk of a 32bit overflow, but in general and IMHO as the code evolves, the > use of incorrect arithmetic may have security implications in the future, so > I advocate for code correctness in this case. Hi Gustavo Is this on the hotpath? How much overhead does it add to 32 bit architectures which don't have 64 bit arithmetic in hardware? There are a lot of embedded systems which are 32 bit. Andrew
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
Hi Alan, Quoting Alan Cox : On Wed, 31 Jan 2018 18:24:07 -0600 "Gustavo A. R. Silva" wrote: Cast to s64 some variables and a macro in order to give the compiler complete information about the proper arithmetic to use. Notice that these elements are used in contexts that expect expressions of type s64 (64 bits, signed). Currently such expression are being evaluated using 32-bit arithmetic. The question you need to ask is 'can it overflow 32bit maths', otherwise you are potentially making the system do extra work for no reason. Yeah, I get your point and it seems that in this particular case there is no risk of a 32bit overflow, but in general and IMHO as the code evolves, the use of incorrect arithmetic may have security implications in the future, so I advocate for code correctness in this case. Thanks -- Gustavo
Re: [PATCH] tcp_lp: use 64-bit arithmetic instead of 32-bit
On Wed, 31 Jan 2018 18:24:07 -0600 "Gustavo A. R. Silva" wrote: > Cast to s64 some variables and a macro in order to give the > compiler complete information about the proper arithmetic to > use. Notice that these elements are used in contexts that > expect expressions of type s64 (64 bits, signed). > > Currently such expression are being evaluated using 32-bit > arithmetic. The question you need to ask is 'can it overflow 32bit maths', otherwise you are potentially making the system do extra work for no reason. Alan
