These are chat archives for Exa-Networks/exabgp

16th
Sep 2016
Job Snijders
@job
Sep 16 2016 08:56
allo
Thomas Mangin
@thomas-mangin
Sep 16 2016 08:57
halo, halo :-)
Job Snijders
@job
Sep 16 2016 08:57
alright, seems we are making good compromises in IDR on large comms
100% design by committee
Thomas Mangin
@thomas-mangin
Sep 16 2016 08:57
:-)
Job Snijders
@job
Sep 16 2016 08:57
so we have to reimplement large
Thomas Mangin
@thomas-mangin
Sep 16 2016 08:57
np - do you have a diff of what changed ?
Job Snijders
@job
Sep 16 2016 08:57
This message was deleted
Thomas Mangin
@thomas-mangin
Sep 16 2016 08:59
Thanks good ! what is happening, some time ago I complained that there was as many BGP TLV than community type - they are fixing that !!
Job Snijders
@job
Sep 16 2016 08:59
so A:B:C + A:D:E + F:H:I is packed like A:(B:C+D:E) F:(H:I)
Thomas Mangin
@thomas-mangin
Sep 16 2016 08:59
I am on a LINX board meeting (starting soon) but via video conf - will have a look
Job Snijders
@job
Sep 16 2016 08:59
i wrote draft-hares-idr-common-header.txt ;)
Thomas Mangin
@thomas-mangin
Sep 16 2016 09:00
kudos !
Let me read the doc .. should be very easy to implement
Job Snijders
@job
Sep 16 2016 09:00
but, because of the animosity between wide + large authors, i proposed the compromise that nobody is listed as author
and the chairs of the working group are the editors
with everybody equally unhappy we have a good compromise to progress this stuff
Thomas Mangin
@thomas-mangin
Sep 16 2016 09:00
LOL - remember - this is a public forum :wink:
Job Snijders
@job
Sep 16 2016 09:01
nothing to hide
technically speaking the "large" community does not need a header, but if it helps pull some extra implementations across the finish line and addresses concerns regarding using too many path attributes for the function of 'coloring', why not?
Thomas Mangin
@thomas-mangin
Sep 16 2016 09:03
Ok - so we need to create a class which will be sub-classed by more specific implementation
Job Snijders
@job
Sep 16 2016 09:03
yeah, in a way similar to extended communities
Thomas Mangin
@thomas-mangin
Sep 16 2016 09:04
Yes
Job Snijders
@job
Sep 16 2016 09:04
but instead of matching on a bitmask for transitivity, i think the direction will be to do a simple integer match
i have not written any code yet
but i figured we need to get exabgp back in the game asap
to retain the 'running code' argument for large
Thomas Mangin
@thomas-mangin
Sep 16 2016 09:08
sure - I was working like mad and able to help recently but from next week things should start to settle - quite happy to give you an hand
Job Snijders
@job
Sep 16 2016 09:08
cool
Thomas Mangin
@thomas-mangin
Sep 16 2016 09:11
The way I would do it (as you will notice I re-organised the community folder)
is to create a new section/folder for Common Header Attribute
and under it add specific implementation
Job Snijders
@job
Sep 16 2016 09:12
so common/large ?
Thomas Mangin
@thomas-mangin
Sep 16 2016 09:30
sure
Job Snijders
@job
Sep 16 2016 10:01
i'm dicking around in branch common_header
Thomas Mangin
@thomas-mangin
Sep 16 2016 10:06
:-)
Feel free to grab me if you have any question - silly or not
Thomas Mangin
@thomas-mangin
Sep 16 2016 10:52
your branch looks good.
@job do you need any help for anything ?
Job Snijders
@job
Sep 16 2016 11:21
well, i dont mind handing it over to you :D
Job Snijders
@job
Sep 16 2016 11:29
i added packet formats to the files
that should make it easier to understand
you see a diff, i am not sure how to lift the global administrator from the common header and use that in the large community
Thomas Mangin
@thomas-mangin
Sep 16 2016 11:35
ok, let me have a look
Thomas Mangin
@thomas-mangin
Sep 16 2016 11:44
push tabs/space fix upstream please pull
Thomas Mangin
@thomas-mangin
Sep 16 2016 11:55
@job ping
Job Snijders
@job
Sep 16 2016 12:01
jo
pulled
i changed something too :)
pull
Thomas Mangin
@thomas-mangin
Sep 16 2016 12:26
ok

unsure what to do here since its under common header

I need to think about it and for that I can not be listening at the same time
Job Snijders
@job
Sep 16 2016 12:35
'large communities' should be a first class citizen, even if its under common header
from config perspective
Thomas Mangin
@thomas-mangin
Sep 16 2016 12:36
Yes
The ‘internal machinery’ should be invisible to the user
so that is why this is harder than it looks. I think we need to cheat somewhere in the sub-class so it looks like different attributes
it would keep the core as it (otherwise you hard to deal with the issue you just raised)
Thomas Mangin
@thomas-mangin
Sep 16 2016 12:57
``` def _ctype (self, transitive=True):
return pack(
'!HHL',
self.COMMUNITY_TYPE if transitive else self.COMMUNITY_TYPE | self.NON_TRANSITIVE,
self.COMMUNITY_SUBTYPE
)
return self.pack .. but even that is not right
The transitivity is set on the class as a class value, so instead of having the value in common, we want it on large and not headers
@job do I make sense ?
Job Snijders
@job
Sep 16 2016 12:58
the type value in the common header dictates whether it is transitive or not
Thomas Mangin
@thomas-mangin
Sep 16 2016 12:59
ah .. then it is more complex.
I must say the draft is quite intrusive on implementation ...
Let’s me re-read it and see if we can improve it
I will do that this week-end
Job Snijders
@job
Sep 16 2016 13:01
def is_transitive(x): return 0x0001 <= x <= 0x02AA or 0x0556 <= x <= 0x07FF
Thomas Mangin
@thomas-mangin
Sep 16 2016 13:01
It would be easier to have two code, one from transitive and one for not transitive
It is moving some of the attribute logic INSIDE the TLV which I think is madness
Job Snijders
@job
Sep 16 2016 13:02
so the tricky thing, we want to prevent that a non-transitive version of "large" can exist
Thomas Mangin
@thomas-mangin
Sep 16 2016 13:02
If it is not easy to implement, it will not be done
Job Snijders
@job
Sep 16 2016 13:02
if you have a T flag in the header, you can create a header + payload which is a nontransitive version of large
Thomas Mangin
@thomas-mangin
Sep 16 2016 13:03
Let’s speak about it when I had the time to review the docs and make a mind about what is what
Job Snijders
@job
Sep 16 2016 13:04
-large- people just wanted a transitive optional attribute, jeff wants a common header, need to figure out something that prevents sitaution in which you can have a 'large' community which is of non-transitive nature
type ranges is one way
Thomas Mangin
@thomas-mangin
Sep 16 2016 13:04
you can do that by having TWO common attributes, one transitive one not .. much simpler
but again. let me read the doc correctly at least once first
Job Snijders
@job
Sep 16 2016 13:56
meh
Thomas Mangin
@thomas-mangin
Sep 16 2016 14:50
The issue is that you may want SOME data transitive and some data non-transitivite but the FLAG is for the whole attribute.
Job Snijders
@job
Sep 16 2016 14:51
you mean: s/attribute/community following the header/
Thomas Mangin
@thomas-mangin
Sep 16 2016 14:51
So the way around it is to have to different attribute code as BGP does not allow the presence of the same attribute multiple time
make more sense said that way ?
Job Snijders
@job
Sep 16 2016 14:51
what do you mean you cannot have the same attribute multiple times
Thomas Mangin
@thomas-mangin
Sep 16 2016 14:51
I mean the attribute encoding (size, flags - inc transitive, code, content)
let me find the wording for you.
Job Snijders
@job
Sep 16 2016 14:52
Communities always start with the following header:

    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |T|          Type               |          Length               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                       Global Administrator                    |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   The fields are as shown below:

        T - Transitive bit:

                Value 0: The community is transitive across ASes

                Value 1: The community is non-transitive across ASes

        Type:   The type of community following the header.  The types
             are to be defined in companion documents.  This is a 15 bit
             field.

        Length:   The number of octets occupied by this header and the
             communities under it.  This is a two octet field.

        Global Administrator:   This sub-field contains a 4-octet
             Autonomous System number assigned by IANA.
how about that
that's easy to match & act upoin
Thomas Mangin
@thomas-mangin
Sep 16 2016 14:56
RFC 4271 (BGP-4), section 6.3 states:
If any attribute appears more than once in the UPDATE message, then
the Error Subcode MUST be set to Malformed Attribute List.
So if you are creating a new “common_header” attribute
it can be either transitive or non-transitive
trying to move the transitivity within the content is an issue
or are you considering another type of transitivity ?
(still not have read the whole doc)
thomas-mangin @thomas-mangin goes away
Thomas Mangin
@thomas-mangin
Sep 16 2016 15:00
@job sorry for the noise
Job Snijders
@job
Sep 16 2016 15:01
so its not an issue ?
Thomas Mangin
@thomas-mangin
Sep 16 2016 15:01
no - you define a new “sub-transititivity” - so it is fine
what the configuration will need to do is define a type common-header and have sub conf for each created
something like common-header [ large-community:1:2:3:4 new-one:5:67:3 ]
that would work
Job Snijders
@job
Sep 16 2016 15:03
then it is not a first class citizen in the config
should just be large-community [ 2391:332:53 242:5222:111 ]
Thomas Mangin
@thomas-mangin
Sep 16 2016 15:04
it can be done as some ‘sugar'
let me code it - will do this week-end
Job Snijders
@job
Sep 16 2016 19:12
yeah indeed, stuff in parser.py