Issue with unsigned integer LOCAL_PREF
Hello Bird team, Thank you for the great tool BIRD. I would like to open an issue (bird v2.0.7) about the local_pref. Today we hit an issue with one filter. if ! is_geo_region_local() then { bgp_local_pref = bgp_local_pref - 200; # If not defined, MED has a type of void instead of int if defined(bgp_med) then { bgp_med = bgp_med + 50; } else { bgp_med = 50; } } When some prefixes with the local_pref was below 200 (for example 100), the value overflow: localpref (unsigned integer 4^32) 100 - 200 = -100 = 4 ^ 32 - 100 = 4294967296 - 100 = 4294967196 We had the prefixes with this value 4294967196 X.X.X.X/32 unicast [ipv4_rr01 07:51:49.603 from 100.100.254.137] * (100/84285) [AS65030?] via 100.99.0.100 on eth1.666 Type: BGP univ BGP.origin: Incomplete BGP.as_path: 64619 65030 BGP.next_hop: y.y.y.y BGP.med: 50 BGP.local_pref: 4294967196 BGP.community: (35280,1040) (35280,2070) (35280,3110) BGP.originator_id: y.y.y.y BGP.cluster_list: 0.0.0.30 We had an exception to check the value of local_pref (on the filter) but can we have the something on the code, not to overflow the local_pref, for example SET to 0 if the value overflow? Our filter looks like this now if bgp_local_pref > 200 then bgp_local_pref = bgp_local_pref - 200; else { bgp_local_pref = 0; } Best Alexandre Corso alexandre@ac <mailto:alexandre@acorus.net>orso.fr
On Wed, Nov 25, 2020 at 11:43:29AM +0100, Alexandre Corso wrote:
Hello Bird team,
Thank you for the great tool BIRD. I would like to open an issue (bird v2.0.7) about the local_pref.
...
We had an exception to check the value of local_pref (on the filter) but can we have the something on the code, not to overflow the local_pref, for example SET to 0 if the value overflow?
Hello There are at least three reasonable behaviors for fixed-range arithmetics: 1) silent overflow (current behavior) 2) silent saturation (as you suggested) 3) explicit (logged) error Not really sure which is better. (1) is likely least astonishment (for fixed-range), (2) is most useful when overflows are intentional, (3) is most useful when overflows are unintentional. I have minor preference for (3), but no strong preference either way to change anything. Also note that in cases when fixed-range attributes have lower range than 32bits (i.e. 16bit preference and 24bit ospf_metrics), we generate explicit error on overflow on assignment. Or perhaps we should just move to arbitrary-range integers .. many new RFCs already use 64bit attributes anyways. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Hello, commercial BGP implementations (Juniper, Nokia, Cisco) usually implements silent saturation based on your point (2) in such cases, Quagga does this too (see route_value_adjust function [1]; which handles these overflows). It's not limited to locpref, also other attibutes can be modified by addition/subtraction. I suggest to follow this practice also in Bird code, as this behavior is commonly expected here. Length of relevant attributes are defined in RFC and I don't expect any move from 32b to 64b integers here - as this will break compability between speakers. - Daniel [1] https://github.com/Quagga/quagga/blob/88d6516676cbcefb6ecdc1828cf59ba3a6e5fe... On 12/1/20 5:51 PM, Ondrej Zajicek wrote:
There are at least three reasonable behaviors for fixed-range arithmetics:
1) silent overflow (current behavior)
2) silent saturation (as you suggested)
3) explicit (logged) error
Not really sure which is better. (1) is likely least astonishment (for fixed-range), (2) is most useful when overflows are intentional, (3) is most useful when overflows are unintentional.
I have minor preference for (3), but no strong preference either way to change anything. Also note that in cases when fixed-range attributes have lower range than 32bits (i.e. 16bit preference and 24bit ospf_metrics), we generate explicit error on overflow on assignment.
Or perhaps we should just move to arbitrary-range integers .. many new RFCs already use 64bit attributes anyways.
Hello, I have the same opinion. Silent saturation (with log). The goal is to have the same behavior on all BGP implementation. Alexandre
On Dec 2, 2020, at 6:45 PM, Daniel Suchy <danny@danysek.cz> wrote:
Hello, commercial BGP implementations (Juniper, Nokia, Cisco) usually implements silent saturation based on your point (2) in such cases, Quagga does this too (see route_value_adjust function [1]; which handles these overflows). It's not limited to locpref, also other attibutes can be modified by addition/subtraction.
I suggest to follow this practice also in Bird code, as this behavior is commonly expected here.
Length of relevant attributes are defined in RFC and I don't expect any move from 32b to 64b integers here - as this will break compability between speakers.
- Daniel
[1] https://github.com/Quagga/quagga/blob/88d6516676cbcefb6ecdc1828cf59ba3a6e5fe...
On 12/1/20 5:51 PM, Ondrej Zajicek wrote:
There are at least three reasonable behaviors for fixed-range arithmetics: 1) silent overflow (current behavior) 2) silent saturation (as you suggested) 3) explicit (logged) error Not really sure which is better. (1) is likely least astonishment (for fixed-range), (2) is most useful when overflows are intentional, (3) is most useful when overflows are unintentional. I have minor preference for (3), but no strong preference either way to change anything. Also note that in cases when fixed-range attributes have lower range than 32bits (i.e. 16bit preference and 24bit ospf_metrics), we generate explicit error on overflow on assignment. Or perhaps we should just move to arbitrary-range integers .. many new RFCs already use 64bit attributes anyways.
participants (3)
-
Alexandre Corso -
Daniel Suchy -
Ondrej Zajicek