Re: [ovs-dev] [PATCH] dpif-netdev: Fix time delta overflow in case of race for meter lock.
On 25.10.2019 17:55, William Tu wrote:
On Fri, Oct 25, 2019 at 4:44 AM Ilya Maximets wrote:
There is a race window between getting the time and getting the meter
lock. This could lead to situation where the thread with larger
current time (this thread called time_{um}sec() later than others)
will acquire meter lock first and update meter->used to the large
value. Next threads will try to calculate time delta by subtracting
the large meter->used from their lower time getting the negative value
which will be converted to a big unsigned delta.
Fix that by assuming that all these threads received packets in the
same time in this case, i.e. dropping negative delta to 0.
CC: Jarno Rajahalme
Fixes: 4b27db644a8c ("dpif-netdev: Simple DROP meter implementation.")
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363126.html
Signed-off-by: Ilya Maximets
---
LGTM.
Thanks for the fix
Acked-by: William Tu
Thanks, William! Applied to master and backported.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Fix time delta overflow in case of race for meter lock.
On Fri, Oct 25, 2019 at 4:44 AM Ilya Maximets wrote:
>
> There is a race window between getting the time and getting the meter
> lock. This could lead to situation where the thread with larger
> current time (this thread called time_{um}sec() later than others)
> will acquire meter lock first and update meter->used to the large
> value. Next threads will try to calculate time delta by subtracting
> the large meter->used from their lower time getting the negative value
> which will be converted to a big unsigned delta.
>
> Fix that by assuming that all these threads received packets in the
> same time in this case, i.e. dropping negative delta to 0.
>
> CC: Jarno Rajahalme
> Fixes: 4b27db644a8c ("dpif-netdev: Simple DROP meter implementation.")
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363126.html
> Signed-off-by: Ilya Maximets
> ---
LGTM.
Thanks for the fix
Acked-by: William Tu
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] dpif-netdev: Fix time delta overflow in case of race for meter lock.
There is a race window between getting the time and getting the meter
lock. This could lead to situation where the thread with larger
current time (this thread called time_{um}sec() later than others)
will acquire meter lock first and update meter->used to the large
value. Next threads will try to calculate time delta by subtracting
the large meter->used from their lower time getting the negative value
which will be converted to a big unsigned delta.
Fix that by assuming that all these threads received packets in the
same time in this case, i.e. dropping negative delta to 0.
CC: Jarno Rajahalme
Fixes: 4b27db644a8c ("dpif-netdev: Simple DROP meter implementation.")
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363126.html
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c | 8
1 file changed, 8 insertions(+)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4546b55e8..13586206a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5646,6 +5646,14 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
/* All packets will hit the meter at the same time. */
long_delta_t = now / 1000 - meter->used / 1000; /* msec */
+if (long_delta_t < 0) {
+/* This condition means that we have several threads fighting for a
+ meter lock, and the one who received the packets a bit later wins.
+ Assuming that all racing threads received packets at the same time
+ to avoid overflow. */
+long_delta_t = 0;
+}
+
/* Make sure delta_t will not be too large, so that bucket will not
* wrap around below. */
delta_t = (long_delta_t > (long long int)meter->max_delta_t)
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
