Fix AS_PATH with AS_SET handling

[a16]

In the case of using the following configuration, exabgp would send 2 routes with incorrect AS_PATH attributes.

neighbor 192.168.0.100 {
        router-id 192.168.0.1;
        local-address 192.168.0.1;
        local-as 65001;
        peer-as 65100;
        hold-time 90;

        family {
                ipv4 unicast;
        }

        static {
                route 172.16.1.0/24 {
                        origin igp;
                        as-path [ ( 65002 ) ];
                        next-hop 192.168.0.1;
                }
                route 172.16.2.0/24 {
                        origin igp;
                        as-path [ ( 65002 65003 ) ];
                        next-hop 192.168.0.1;
                }
        }
}

The first route would be advertised with correct AS_PATH attribute to the neighbor, but the second one would be advertised with incorrect attributes.

Remote router> 
   Network          Next Hop            Metric LocPrf Weight Path
*> 172.16.1.0/24    192.168.0.1                            0 {65002} i
*> 172.16.2.0/24    192.168.0.1                            0 {65002} i

It should be:

Remote router> 
   Network          Next Hop            Metric LocPrf Weight Path
*> 172.16.1.0/24    192.168.0.1                            0 {65002} i
*> 172.16.2.0/24    192.168.0.1                            0 {65002,65003} i

In the latest code, It seems to return [ ] even if AS_SET exists when AS_PATH doesn't have AS_SEQ.

[thomas-mangin]

Hello Eiichiro,

What you propose seems right and does indeed correct a display issue ( and if you are using the API to get a route and re-announce it, clearly without it this will be causing grief with the generation).
But otherwise it would not affect how the data is encoded on the wire, could you please confirm that it is how you found the issue please.

Could I also be pedantic and ask you if you could provide the patch as :

string = '[ ( %s )]' % (' '.join([str(_) for _ in aset]))

As the double % makes it hard to read the line otherwise 😉

Thank you for your feedback, and the time you are spending debugging ExaBGP, this is much appreciated.

[a16]

In encoding the messages, the method def _segments in lib/exabgp/bgp/message/update/attribute/aspath.py is used and self.as_set is refered.
At this momemt, self.as_set has [] in the method. I'm not sure but the value was lost in somewhere...

[coveralls]

Coverage increased (+7.74%) to 56.19% when pulling d6d456d on a16:fix_as_path_with_as_set_handling into f6841eb on Exa-Networks:master.

[coveralls]

Coverage decreased (-0.0%) to 48.45% when pulling d6d456d on a16:fix_as_path_with_as_set_handling into f6841eb on Exa-Networks:master.

[a16]

Sorry, I made a mistake. it returned [65002L], not [].
When I put pprint.pprint(self) and print self.as_set into def pack in lib/exabgp/bgp/message/update/attribute/aspath.py and ran, it showed:

#For the first route "172.16.1.0/24":
<exabgp.bgp.message.update.attribute.aspath.ASPath object at 0x7f75ca8c86b0>
[65002L]
# => this is correct
# For the second route "172.16.2.0/24":
<exabgp.bgp.message.update.attribute.aspath.ASPath object at 0x7f75ca8c86b0>
[65002L]
# => this is incorrect, isn't it? it should be [65002L, 65003L].

It seems to refer to the same object. so I think exabgp sent the incorrect route.

On the otherhand, after applied my patch, it showed:

#For the first route "172.16.1.0/24":
<exabgp.bgp.message.update.attribute.aspath.ASPath object at 0x7f056ebf7628>
[65002L]
# For the second route "172.16.2.0/24":
<exabgp.bgp.message.update.attribute.aspath.ASPath object at 0x7f056ebf7738>
[65002L, 65003L]

Then it works correctly.

But I didn't understand the root cause...What do you think?

[thomas-mangin]

str is used for indexing so, with the current issue a bad object is used as the code thinks that that attribute was already generated and can be reused.

[thomas-mangin]

Ok - all looking good merged. Thank you for finding this !