On Mon, 1 Feb 2016, Donald Sharp wrote:
I was looking at the bgp code and noticed that we have different places
that we store nexthop information:
In 'struct attr'
struct in_addr nexthop; <------IPv4 Nexthop
address
In 'struct attr_extra':
#ifdef HAVE_IPV6
struct in6_addr mp_nexthop_global; <-----IPv6 Nexthop Global
address
struct in6_addr mp_nexthop_local; <-----IPv6 Nexthop Link Local
address
#endif /* HAVE_IPV6 */
#ifdef HAVE_IPV6
struct in6_addr mp_nexthop_global; <-----VPNV4 Nexthop Address
struct in6_addr mp_nexthop_local; <-----Never Used
#endif /* HAVE_IPV6 */
For the latter two you mean _in?
Does anyone know the historical reasoning for this? Does anyone mind
if I combine these data structures, in a somewhat more logical manner?
I don't know why VPNv4 doesn't re-use the same in_addr as for v6. Looks
like maybe it could?
The reason they're in attr_extra is that IPv4 paths did and likely will
continue to for at least a while, greatly outnumber IPv6 paths. With
those addresses in the main attr, the size of (struct attr) increases by
(if my counts are right):
32 bit:
- 40%, with just the 2 extra addresses (29 to 41 bytes)
- 96%, with all 4 of those (29 to 57)
64:
- 17%, with just the 2
- 53%, with all 4
I didn't fancy digging into VPNv4 to see what the hell those 2 extra
addresses were about when I tried to slim down (struct attr).
The reason I tried to slim down (struct attr) was that some people from
another free BGP implementation were going around at conferences saying
that Quagga was bloated memory wise (never mind Quagga implemented stuff
they didn't, including soft-refresh...).
I could imagine getting rid of (struct attr_extra), and making (struct
attr) more dynamically sized according to the attrs present, and
encapsulating access to its members via helpers to pull desired fields
out based on the "attrs set" bitmap. I wouldn't get /too/ radical
cleaning this up right now though, given there's a lot of other stuff
still to get in...
regards,
--
Paul Jakma [email protected] @pjakma Key ID: 64A2FF6A
Fortune:
You now have Asian Flu.
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
