On Mon, Oct 06, 2008 at 01:13:21PM +0200, Martin Mares wrote:
+ return dst - dst_start; +} + + + void
Please do not add too much whitespace.
+#define AS_TRANS 23456
This deserves a comment.
OK, i will add some.
+ { NULL, }, /* BA_EXTENDED_COMM */ + { "as4_path", -1, BAF_OPTIONAL | BAF_TRANSITIVE, EAF_TYPE_OPAQUE, 1, /* BA_AS4_PATH */ + bgp_check_as4_path, NULL }, + { "as4_aggregator", 8, BAF_OPTIONAL | BAF_TRANSITIVE, EAF_TYPE_OPAQUE, 1, /* BA_AS4_PATH */ + NULL, NULL } };
+/* BA_AS4_PATH is type EAF_TYPE_OPAQUE and not type EAF_TYPE_AS_PATH because + * EAF_TYPE_AS_PATH is supposed to have different format (2 or 4 B for each ASN) + * depending on bgp_as4_support variable. + */ +
Does this make sense? Wouldn't it be better to have a single extended attribute format and convert as necessary?
Or, if we decide that we should prefer saving space over simplicity, define a new type for the extended paths. ...
+ if ((code == BA_AGGREGATOR) && bgp_as4_support && (! p->as4_support))
What is the global setting of `bgp_as4_support' for? Shouldn't it be always a per-instance setting?
The 4B ASN support is implemented to always store AS_PATH in native representation and convert it during communication with neighbors. Although in some situations (like having more 4B-non-aware neighbors) this implementation might lead to unnecessary converting compared to lazy variant, i think it is much simpler and therefore error-proof becouse otherwise we have to do reconstruction of real 4B PATH in many places (like filter code, printing ...). Variable 'bgp_as4_support' decides whether this router support 4B ASN or not. It is possible to disable 4B ASN support (using global option 'disable_bgp_as4') (for example for testing or to workaround a bug in the neighbor implementation of 4B ASN) but it shouldn't be neccessary to change this setting. When bgs_as4_support is enabled, then native representation of AS_PATH (type EAF_TYPE_AS_PATH) is 4 B per ASN, otherwise it is 2 B per ASN. The extended attribute AS4_PATH is used only when 4B-aware and 4B-non-aware routers communicate together. If BIRD is 4B-aware, then it discards such attribute ASAP, and when BIRD is not 4B-aware, it is opaque attribute for it. Different possible implemementation would be to have two types: EAF_TYPE_AS2_PATH and EAF_TYPE_AS4_PATH (used for paths with 2 B per ASN and 4 B per ASN). This variant is probably cleaner, but more complicated to implement and does not give us much advantages. The problem with current implementation is that it is not easy to support more bgp instances with different 'bgp_as4_support' and also to change 'bgp_as4_support' during soft reconfiguration. But the option is unnecessary during a normal usage and therefore i don't see this as a big problem. -- 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."