Oh,
Just another thing, the bgp_route.c::bgp_peer_remove_private_as function
wasn't completely clear ot me. I think the logic simplifies out to at
least:
/* 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)
return;
if (!peer_af_flag_check (peer, afi, safi, PEER_FLAG_REMOVE_PRIVATE_AS))
return;
/* 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_REPLA
attr->aspath = aspath_replace_private_asns (attr->aspath, bgp->as);
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_REPLA
attr->aspath = aspath_replace_private_asns (attr->aspath, bgp->as);
else
attr->aspath = aspath_empty_get ();
}
}
Which raises the question:
If the user has not specified 'all', but has specified 'replace'
wouldn't the action of least-surprise be to replace private-ASes
regardless of whether the AS_PATH has public ones?
I.e. could it be simplied further, and just have 'replace' have an
unconditional meaning, so it's:
...
if (peer_af_flag_check (peer, afi, safi,
PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE))
{
attr->aspath = aspath_replace_private_asns (attr->aspath, bgp->as);
return;
}
if (peer_af_flag_check (peer, afi, safi, PEER_FLAG_REMOVE_PRIVATE_AS_ALL))
attr->aspath = aspath_remove_private_asns (attr->aspath);
else if (aspath_private_as_check (attr->aspath))
attr->aspath = aspath_empty_get ();
return;
}
Do we need all 2^2+1 different ways to twiddle with private-ASes? Is it
useful to have a "only do something if the whole AS_PATH is private"
variable for a new feature?
I.e. 1 flag to select between new and old behavior for
'remove-private-as', and replace can just be 'replace any private ASes'?
regards,
--
Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A
Fortune:
Isn't it conceivable to you that an intelligent person could harbor
two opposing ideas in his mind?
-- Adlai Stevenson, to reporters
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
