| Message ID | 8832a7af843af7fe0a821e0ba9874ca6a2232ab6.1716807712.git.echaudro@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,1/7] Coverity: Fix Coverity `Unintentional integer overflow` reports. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 5/27/24 13:01, Eelco Chaudron wrote: > Fix three Unintentional integer overflow reports by adding the > ULL/LL suffix to the numerical literals used in the multiplications. > > Fixes: 5767a79a4059 ("cfm: Require ccm received in demand mode.") > Fixes: ed2300cca0d3 ("netdev-linux: Refactor put police action netlink message") > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- > lib/cfm.c | 2 +- > lib/netdev-linux.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Hi, Eelco. Thanks for the patch! Please, give the patch a more descriptive subject to reduce the probability of having patches named exactly the same in the git history in the future. Also, this patch is trying to fix two separate issues and fixes will potentially need to be backported to the different depth. So, please, split them up. This will help with naming as well. Also, please, either send all patches as separate or add a cover letter to the set. Best regards, Ilya Maximets.
On 27 May 2024, at 16:04, Ilya Maximets wrote: > On 5/27/24 13:01, Eelco Chaudron wrote: >> Fix three Unintentional integer overflow reports by adding the >> ULL/LL suffix to the numerical literals used in the multiplications. >> >> Fixes: 5767a79a4059 ("cfm: Require ccm received in demand mode.") >> Fixes: ed2300cca0d3 ("netdev-linux: Refactor put police action netlink message") >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >> --- >> lib/cfm.c | 2 +- >> lib/netdev-linux.c | 4 ++-- >> 2 files changed, 3 insertions(+), 3 deletions(-) > > > Hi, Eelco. Thanks for the patch! > > Please, give the patch a more descriptive subject to reduce the probability > of having patches named exactly the same in the git history in the future. > > Also, this patch is trying to fix two separate issues and fixes will > potentially need to be backported to the different depth. So, please, > split them up. This will help with naming as well. Will split them up, and change the name. > Also, please, either send all patches as separate or add a cover letter to > the set. ACK, had this, but forgot to add it, will do in V2. > Best regards, Ilya Maximets.
diff --git a/lib/cfm.c b/lib/cfm.c index c3742f3de..7eb080157 100644 --- a/lib/cfm.c +++ b/lib/cfm.c @@ -863,7 +863,7 @@ cfm_process_heartbeat(struct cfm *cfm, const struct dp_packet *p) rmp->num_health_ccm++; if (cfm->demand) { timer_set_duration(&cfm->demand_rx_ccm_t, - 100 * cfm->ccm_interval_ms); + 100LL * cfm->ccm_interval_ms); } } rmp->recv = true; diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 25349c605..eb0c5c624 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2915,8 +2915,8 @@ tc_add_matchall_policer(struct netdev *netdev, uint64_t kbits_rate, basic_offset = nl_msg_start_nested(&request, TCA_OPTIONS); action_offset = nl_msg_start_nested(&request, TCA_MATCHALL_ACT); nl_msg_put_act_police(&request, 0, kbits_rate, kbits_burst, - kpkts_rate * 1000, kpkts_burst * 1000, TC_ACT_UNSPEC, - false); + kpkts_rate * 1000ULL, kpkts_burst * 1000ULL, + TC_ACT_UNSPEC, false); nl_msg_end_nested(&request, action_offset); nl_msg_end_nested(&request, basic_offset);
Fix three Unintentional integer overflow reports by adding the ULL/LL suffix to the numerical literals used in the multiplications. Fixes: 5767a79a4059 ("cfm: Require ccm received in demand mode.") Fixes: ed2300cca0d3 ("netdev-linux: Refactor put police action netlink message") Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- lib/cfm.c | 2 +- lib/netdev-linux.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)