Hi,
Is it just me, or does this patch leak attr->aspath? I've highlighted
the bits that made me ask that. Though, to be fair, if so then it looks
like the old remove-private-as also leaked.
I'd suggest separating lifetime management from the manipulation. Have
aspath_replace_private_asns and aspath_remove_private_asns just act on
the given aspath, and have the caller handle lifetime.
regards,
Paul
On Fri, 11 Dec 2015, Donald Sharp wrote:
+/* Replace all private ASNs with our own ASN */
+struct aspath *
+aspath_replace_private_asns (struct aspath *aspath, as_t asn)
+{
+ struct aspath *new;
+ struct assegment *seg;
+
+ new = aspath_dup(aspath);
^^^^^^^^^^^^^^^^^^^^^^^^^
+ seg = new->segments;
+
+ while (seg)
+ {
+ int i;
+
+ for (i = 0; i < seg->length; i++)
+ {
+ if (BGP_AS_IS_PRIVATE(seg->as[i]))
+ seg->as[i] = asn;
+ }
+ seg = seg->next;
+ }
+
+ aspath_str_update(new);
+ return new;
+}
+
+/* Remove all private ASNs */
+struct aspath *
+aspath_remove_private_asns (struct aspath *aspath)
+{
+ struct aspath *new;
+ struct assegment *seg;
+ struct assegment *new_seg;
+ struct assegment *last_new_seg;
+ int i;
+ int j;
+ int public = 0;
+
+ new = XCALLOC (MTYPE_AS_PATH, sizeof (struct aspath));
+
+ new_seg = NULL;
+ last_new_seg = NULL;
+ seg = aspath->segments;
+ while (seg)
+ {
+ public = 0;
+ for (i = 0; i < seg->length; i++)
+ {
+ // ASN is public
+ if (!BGP_AS_IS_PRIVATE(seg->as[i]))
+ {
+ public++;
+ }
+ }
+
+ // The entire segment is private so skip it
+ if (!public)
+ {
+ seg = seg->next;
+ continue;
+ }
+
+ // The entire segment is public so copy it
+ else if (public == seg->length)
+ {
+ new_seg = assegment_dup (seg);
+ }
+
+ // The segment is a mix of public and private ASNs. Copy as many spots as
+ // there are public ASNs then come back and fill in only the public ASNs.
+ else
+ {
+ new_seg = assegment_new (seg->type, public);
+ j = 0;
+ for (i = 0; i < seg->length; i++)
+ {
+ // ASN is public
+ if (!BGP_AS_IS_PRIVATE(seg->as[i]))
+ {
+ new_seg->as[j] = seg->as[i];
+ j++;
+ }
+ }
+ }
+
+ // This is the first segment so set the aspath segments pointer to this
one
+ if (!last_new_seg)
+ new->segments = new_seg;
+ else
+ last_new_seg->next = new_seg;
+
+ last_new_seg = new_seg;
+ seg = seg->next;
+ }
+
+ aspath_str_update(new);
+ return new;
+}
+
+/* If this is an EBGP peer with remove-private-AS */
+static void
+bgp_peer_remove_private_as(struct bgp *bgp, afi_t afi, safi_t safi,
+ struct peer *peer, struct attr *attr)
+{
+ if (peer->sort == BGP_PEER_EBGP &&
+ peer_af_flag_check (peer, afi, safi, PEER_FLAG_REMOVE_PRIVATE_AS))
+ {
+ // Take action on the entire aspath
+ if (peer_af_flag_check (peer, afi, safi,
PEER_FLAG_REMOVE_PRIVATE_AS_ALL))
+ {
+ if (peer_af_flag_check (peer, afi, safi,
PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE))
+ attr->aspath = aspath_replace_private_asns (attr->aspath, bgp->as);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+ // The entire aspath consists of private ASNs so create an empty
aspath
+ else if (aspath_private_as_check (attr->aspath))
+ attr->aspath = aspath_empty_get ();
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+ // There are some public and some private ASNs, remove the private
ASNs
+ else
+ attr->aspath = aspath_remove_private_asns (attr->aspath);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ }
+
+ // 'all' was not specified so the entire aspath must be private ASNs
+ // for us to do anything
+ else if (aspath_private_as_check (attr->aspath))
+ {
+ if (peer_af_flag_check (peer, afi, safi,
PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE))
+ attr->aspath = aspath_replace_private_asns (attr->aspath, bgp->as);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ else
+ attr->aspath = aspath_empty_get ();
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ }
-
- /* If this is EBGP peer and remove-private-AS is set. */
- if (rsclient->sort == BGP_PEER_EBGP
- && peer_af_flag_check (rsclient, afi, safi, PEER_FLAG_REMOVE_PRIVATE_AS)
- && aspath_private_as_check (attr->aspath))
- attr->aspath = aspath_empty_get ();
+ bgp_peer_remove_private_as(bgp, afi, safi, rsclient, attr);
regards,
--
Paul Jakma | [email protected] | @pjakma | Key ID: 0xD86BF79464A2FF6A
Fortune:
It is impossible to travel faster than light, and certainly not desirable,
as one's hat keeps blowing off.
-- Woody Allen
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
