These are chat archives for Exa-Networks/exabgp

3rd
Sep 2016
Job Snijders
@job
Sep 03 2016 14:31
hello
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:37
Hi
Job Snijders
@job
Sep 03 2016 14:37
seems I dont understand how pack works
File "/Users/job/source/exabgp/lib/exabgp/configuration/static/parser.py", line 375, in _large_community return LargeCommunity(pack('!LLL', (prefix << 64) + (affix << 32) + suffix)) error: pack expected 3 items for packing (got 1)
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:38
master ?
Job Snijders
@job
Sep 03 2016 14:38
no, large_communities
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:38
ok let me co
Job Snijders
@job
Sep 03 2016 14:38
i wouldnt pollute master with broken code ;)
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:38
I do, LOL
kidding .
Job Snijders
@job
Sep 03 2016 14:38
i pushed last version a second ago
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:38
ok pulling
Job Snijders
@job
Sep 03 2016 14:40
oh, I see, normal community fits in Long, (L), but large community should be a "long long long", but thats not LLL, that is 3 longs
so we need to make something that we have 96 bytes
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:41
grrr . git pull upstream large_bgp_communities is failing .. need to see why
Job Snijders
@job
Sep 03 2016 14:42
try git pull upstream large_communities
i removed large_bgp_communities branch and started new
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:44
ok - worked
ok I have the code - what is the issue ?
Job Snijders
@job
Sep 03 2016 14:46
we need to feed pack/unpack the correct FMT
L = long, but LLL != long long long
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:46
LLL is long long long
it will unpack three consecutive long
btw

``` slots = ['large_community','_str']

def __init__ (self, community):
    self.community = community
    self._str = "%d:%d:%d" % unpack('!LLL', self.community)

```

The slot does not match the name in the self.
it is community not large_community
Job Snijders
@job
Sep 03 2016 14:47
let me fix that
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:48
from struct import unpack
unpack('!LLL','\0'43)
(0, 0, 0)
unpack('!LLL','\0\0\0\1'*3)
(1, 1, 1)
Job Snijders
@job
Sep 03 2016 14:49
File "/Users/job/source/exabgp/lib/exabgp/configuration/static/parser.py", line 375, in _large_community return LargeCommunity(pack('!LLL', (prefix << 64) + (affix << 32) + suffix)) error: pack expected 3 items for packing (got 1)
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:49
ok let me look
return LargeCommunity(pack('!LLL',prefix,affix,suffix)
pack('!LLL',1,2,3)
Job Snijders
@job
Sep 03 2016 14:51
ah
Thomas Mangin
@thomas-mangin
Sep 03 2016 14:51
'\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x03'
fun with packing .. :-)
the number of argument is set by the content of the string
Job Snijders
@job
Sep 03 2016 15:02
can you pull again?
the output of ./qa/bin/parsing
Sat, 03 Sep 2016 17:03:11 | INFO | 27737 | parser | strings are different: Sat, 03 Sep 2016 17:03:11 | INFO | 27737 | parser | [9.9.9.9/32 next-hop 192.0.2.1 large-community [ 2914:0:666 2914:3320:123 ]] Sat, 03 Sep 2016 17:03:11 | INFO | 27737 | parser | [9.9.9.9/32 next-hop 192.0.2.1 large-community 2914:3320:123]
doing something with the length wrong somewhere i guess
Job Snijders
@job
Sep 03 2016 15:14
Thomas Mangin
@thomas-mangin
Sep 03 2016 15:17
need to look - working atm
I will a bit later on
Job Snijders
@job
Sep 03 2016 15:25
It may be that you broke attributes.conf ;)
Thomas Mangin
@thomas-mangin
Sep 03 2016 15:36
me breaking any code .. never ! :wink:
Job Snijders
@job
Sep 03 2016 16:06
alright, i think i fixed it
thomas, you got a PR, can you double check whether this is also your understanding of the draft?
Thomas Mangin
@thomas-mangin
Sep 03 2016 16:11
ok
just finished my work
Job Snijders
@job
Sep 03 2016 16:16
cool, i also submitted PR for blackhole community
Thomas Mangin
@thomas-mangin
Sep 03 2016 16:16
ok - that looks good .
in the qa/conf folder
you need to add a few files
the way it works, one file is the config, another contain the ‘raw’ packet you expect to be generated
prefixed with <order>:raw: (keep number as one for simple example)
like 1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:0038:02:0000001C40010100400200400304C0A8016480040400000064400504000000642064646464
that it the .sequence file
and a .group file which has the name of the .conf to be run by the test
then if you run:
LISTING=1 ./qa/bin/conversation
you will have all the ‘name of the test available and can do something like
SERVER=1 ./qa/bin/conversation
CLIENT=1 ./qa/bin/conversation
and it will check that ExaBGP generate the update you expect
Job Snijders
@job
Sep 03 2016 16:20
cool
i dont know how the raw packet will look
Thomas Mangin
@thomas-mangin
Sep 03 2016 16:20
or better, look with -d what ExaBGP generate, add it to the file and then make sure it is right
Job Snijders
@job
Sep 03 2016 16:20
this is the first large bgp communities implementation i have access to
Thomas Mangin
@thomas-mangin
Sep 03 2016 16:20
If you think you are right, put it and it will allow people to check for regression
if it is false when it is fixed, the test will be updated
going to read the RFC to make sure I understand it
Job Snijders
@job
Sep 03 2016 16:21
can I leave this part with you?
a double check is better anyway
Thomas Mangin
@thomas-mangin
Sep 03 2016 16:23
ok will do it
let me merge
Job Snijders
@job
Sep 03 2016 16:23
yeah and you need to update the changelog and version bump etc anyway
Thomas Mangin
@thomas-mangin
Sep 03 2016 16:33
./sbin/exabgp ./qa/conf/largecommunity.conf --decode 0000003040010100400200400304C000020140050400000064C0291800000B62000000000000029A00000B6200000CF80000007B200909090A 2>&1 | grep json
Sat, 03 Sep 2016 17:34:39 | INFO | 19383 | parser | update json { "exabgp": "3.5.0", "time": 1472920479.28, "host" : "ptr-34.212.219.82.rev.exa.net.uk", "pid" : 19383, "ppid" : 10113, "counter": 1, "type": "update", "neighbor": { "address": { "local": "127.0.0.1", "peer": "127.0.0.1" }, "asn": { "local": "65000", "peer": "65000" }, "direction": "in", "message": { "update": { "attribute": { "origin": "igp", "local-preference": 100, "large-community": [ [ 2914, 0 , 666 ], [ 2914, 3320 , 123 ] ] }, "announce": { "ipv4 unicast": { "192.0.2.1": [ { "nlri": "9.9.9.10/32" } ] } } } } } }
ok
when I blog, should I thanks you and you and your employer ?
all pushed
Thomas Mangin
@thomas-mangin
Sep 03 2016 16:39
@job still around ?
Job Snijders
@job
Sep 03 2016 17:08
yeah
was walking the dog :)
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:09
looking ...
Looks good
merging
Job Snijders
@job
Sep 03 2016 17:11
nice
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:11
done
@job can you please join the ExaBGP G+ group ?
Job Snijders
@job
Sep 03 2016 17:14
i clicked 'request to join'
so i tink you have to do something
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:16
done
you are a member
I accept everyone but it is a close community to prevent junk/spam
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:25
@job how did you find the code ?
I wish I had used more built-in for the data structure and not redefined everything as class (coming from a C++ background before Python)
Job Snijders
@job
Sep 03 2016 17:26
what annoyed me most is the code sometimes has tabs and sometimes spaces
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:26
there should be no space …
Job Snijders
@job
Sep 03 2016 17:27
i hate tabs
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:27
(at least for indenting)
Job Snijders
@job
Sep 03 2016 17:27
tabs break in email and copy+pasting code
i can send you a patch for doing everything with spaces
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:27
Ah .. I just like 3 space for tabs and my co-workers like 1, 4, and 8, so when we code tabs keeps everyone happy
Job Snijders
@job
Sep 03 2016 17:27
4 is the correct one
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:27
(but you .. sorry :p)
Job Snijders
@job
Sep 03 2016 17:27
1 is bullshit
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:27
3 is the correct one :smile:
Job Snijders
@job
Sep 03 2016 17:27
you are insane
but your IDE can adjust it to whatever you find readable
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:28
The one was someone just arguing and beeing cheeky
Job Snijders
@job
Sep 03 2016 17:28
i think the code itself should standardize on what the python base itself does
you can configure your editor to "display the indenting in steps of 3 spaces"
anyway, we can argue about it, but to me that was a bit of a challenge
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:29
I want to rewrite the reactor with Thread for IO and multiprocess for message parsing and use Queue for communication (like I done with some code at work) but I need time ..
Job Snijders
@job
Sep 03 2016 17:29
other than that, some documentation is useful
especially how to QA
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:29
documentation is the BIG down.
Ok - point taken
how hard did you find the code ?
compared to other implementations ?
Job Snijders
@job
Sep 03 2016 17:30
what helped me is that i could cheat a bit: i looked at Communities and ExtendedCommunities
without those i would be lost
well, this is the first one
next up is BIRD
in C :)
so i can report later
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:31
Yes - I do the same when I code :tongue:
bird is well structured, you should not have any problem
I like the code base
Job Snijders
@job
Sep 03 2016 17:31
cool, that is encouraging
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:32
I had to look into it to see how they done stuff and see if my implementation was compatible and I was happy all was good just reading their code
Job Snijders
@job
Sep 03 2016 17:33
one issue though, the bird people are not great maintainer
it takes them quite long before they merge things
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:35
Just contact Ondřej Filip privately - he is a nice guy - busy but if your patch is good it should be fine.
Job Snijders
@job
Sep 03 2016 17:36
yeah I know him
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:36
small word
Job Snijders
@job
Sep 03 2016 17:36
yes :)
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:37
ok - dinner time for me - grab me if you need - CU
Job Snijders
@job
Sep 03 2016 17:37
thank you for your help!
Thomas Mangin
@thomas-mangin
Sep 03 2016 17:44
no - thank you for taking the time to add this feature - we have been waiting for it for years !