Exa-Networks / exabgp

The BGP swiss army knife of networking
Other
2.1k stars 447 forks source link

Support for Redirect IPv6 extended communities (for Flowspec Redirect-IPv6) #927

Open pguibert6WIND opened 5 years ago

pguibert6WIND commented 5 years ago

Hello,

Is the RFC 5701 supported ? If not, this would be good to have in order to have support for Redirect-IPv6 from [0]

Thanks, Philippe [0] https://tools.ietf.org/html/draft-ietf-idr-flow-spec-v6-09

pguibert6WIND commented 5 years ago

Actually, when configuring redirect-to-nexthop-ietf 33::2, I was happy to see an ipv6 ext comunity forged. However I have a doubt on which type and subtype to apply when looking at [0], 4. IPv6 Flow Specification Traffic Filtering Action changes : Redirect-IPv6 (0x800B) On exabgp code, I find in lib/exabgp/bgp/message/update/attribute/community/extended/ following value in function TrafficNextHopIPv6IETF:

pguibert6WIND commented 5 years ago

which draft overrides the other one : draft-ietf-idr-flow-spec-v6-09 or draft-ietf-idr-flowspec-redirect-ip ?

thomas-mangin commented 5 years ago

I do not believe that https://tools.ietf.org/html/rfc5701 was implemented. What you are finding is the Flowspec next hop redirect RFC. It should however be quite straight forward to implement.

thomas-mangin commented 5 years ago

I have not implemented redirect-to-nexthop-ietf - as far as the author @elindsey tested it is working - https://github.com/Exa-Networks/exabgp/pull/923

pguibert6WIND commented 5 years ago

I can however see a 20 byte ipv6 ext community forged, but with wrong type and subtype. I am on head of exabgp. I would be happy to know the exact status with @elindsey

elindsey commented 5 years ago

I can however see a 20 byte ipv6 ext community forged, but with wrong type and subtype.

It is the correct type and subtype, but you're looking for a different feature. 🙂

The patchset recently added is for redirect to next hop (draft-ietf-idr-flowspec-redirect-ip), this is for explicitly writing the next hop ip address. This is action type 0x000c, as documented here https://www.iana.org/assignments/bgp-extended-communities/bgp-extended-communities.xhtml (search for 0x000c).

What you're asking about is redirect ipv6, action type 0x800b. This is for redirecting traffic into a VRF, picked by a specific ipv6 address being in its import policy. This is not implemented.

In general, rfcs that say nexthop are going to be a simple next hop IP rewrite. Anything that says redirect without "nexthop" in the name is going to be some form of local VRF redirection.

elindsey commented 5 years ago

As for rfc5701 specifically - support for it was added to be able to send the ipv6 extended community for the 0x000c next hop action. I don't believe anything else in the exa codebase currently uses it, but it would be easy to modify if you want to support additional ipv6 extended communities.

pguibert6WIND commented 5 years ago

Thanks @elindsey for the comments; actually the draft from https://www.iana.org/assignments/bgp-extended-communities/bgp-extended-communities.xhtml was not explicitly mentioning the usage of ext ipv6 community. Thanks for clarification.

I would be ok to specify the command. what about the following : redirect-to-nexthop-vrf-rt-ipv6 <IPV6>:<AS2B>

for the implementation, I also would be happy to help for that; alittle guidance could be helpful however. thanks,

elindsey commented 5 years ago

I don't speak for exa or Thomas, so anything he says overrides me - but I'll voice my thoughts

what about the following : redirect-to-nexthop-vrf-rt-ipv6 <IPV6>:<AS2B>

I would first see if this functionality can be attached to the existing redirect config directive, which handles the original rfc 5575 0x8008 action. If possible I would extend that same config directive to now handle the ipv6:asn notation, ie. redirect ipv6:asn. It's possible that redirect is too overloaded to disambiguate it though, you'll have to look at the parser.

redirect-to-nexthop/redirect-to-nexthop-ietf exists because of the unfortunate competing drafts and to disambiguate from the vrf forms. That doesn't apply here, so hopefully you won't need to go down the route of adding a new config directive.

This recent pull request is a useful reference.

This is where the redirect directive is parsed, and where I would look to see if you can add ipv6:asn parsing.

This commented out stub was going to add support for an ipv4:asn vrf redirect and may be relevant.

pguibert6WIND commented 5 years ago

I have to admit that the format is equal to me, provided that the feature can be put into exabgp. so your counter proposal would suit me. thanks.

thomas-mangin commented 5 years ago

adding the new syntax to redirect is fine and surely the best way forward. I just need the time to implement it .. "just".

thomas-mangin commented 5 years ago

@elindsey thank you for your help figuring out what the type and sub-type needed to be :p I noticed that there was a bug with your code (as the registration of the two differents attributes (normal and the v6 you added) was done within the same namespace. I fixed it with the patch above moving registered_extended into the sub-classes to give them both distinct registration.

@pguibert6WIND can you please try and tell us if it is working as you expect. I have no way to test. If it is ok, then I will add some code for self-testing the generation and decoding of the attributes. The code is untested though.

elindsey commented 5 years ago

Ahh, thanks for the fix. 🙂

I think the 0x0002 in the patch is incorrect. RFC 5701 specifies a route target format of type 0x0002, but draft-ietf-idr-flow-spec-v6-08 is the actual flowspec redirect ipv6 action that happens to reuse that format for a different purpose with type 0x800B. 0x80 is from the experimental use range, which is why I assume it's not in the IANA registry - this redirect ipv6 draft doesn't look finished or adopted.

@pguibert6WIND have you seen any vendor gear that claims support for draft-ietf-idr-flow-spec-v6-08? I haven't been able to turn up any.

thomas-mangin commented 5 years ago

@elindsey happy to change the 0x0002 to whatever is seen in production ...

thomas-mangin commented 5 years ago

@elindsey perhaps I am mixing redirect with adding an IPv6 RT and Origin support ..It has been quite a few years since I last looked at the community RFCs.

pguibert6WIND commented 5 years ago

Hi @thomas-mangin , so previously, I made a patch to replace the route ttarget redirect IP with route target redirect VRF like that one:

 class TrafficNextHopIPv6IETF (ExtendedCommunityIPv6):
-       COMMUNITY_TYPE = 0x00
-       COMMUNITY_SUBTYPE = 0x0C
+       COMMUNITY_TYPE = 0x80
+       COMMUNITY_SUBTYPE = 0x0B

the behaviour is ok from my side. now with the patch you submitted, I made change to the exabgp config:

-redirect-to-nexthop-ietf 33::2;
+redirect 33::2;

and I tried to run exabgp, but with no success.

18:38:34 | 14264  | configuration | not reloaded, no change found in the configuration
18:38:34 | 14264  | configuration | problem parsing configuration file line 15
18:38:34 | 14264  | configuration | error message: illegal IP address string passed to inet_pton

did I make someting wrong?

pguibert6WIND commented 5 years ago

adding to this, currently it is possible to have the following as for simpson draft:

redirect IPV6;

My expectation is that redirect either takes an IP (v4 or v6 like for simpson draft with IP included in NLRI) or an IPv6 route target ( the new draft for redirect vrf).

thomas-mangin commented 5 years ago

Need to look how I can do the parsing ..

thomas-mangin commented 5 years ago

redirect IPV6:asn should now work as you want

pguibert6WIND commented 5 years ago

Hi Thomas,

use case 1 : simpson way: nexthop IPV6 added in NLRI IPv6 : NOK ( it was working fine before patches).

route {
match {
source 1001::2/128;
destination 3003::3/128;
packet-length <200;
}
then {
redirect 50::2;
rate-limit 55;
}
}

16:07:44 | 20468 | configuration | problem parsing configuration file line 15 16:07:44 | 20468 | configuration | error message: illegal IP address string passed to inet_pton

use case 2 : draft ietf flowspec ipv6 way: nexthop IPV6 encoded with AS2B in IPv6 ext comm : NOK ( it has never worked).

route {
match {
 source 192:168::1:5/128;
 }
 then {
 rate-limit 55;
redirect 33::2:0;
 }
 }
16:09:24 | 22149  | configuration | problem parsing configuration file line 14
16:09:24 | 22149  | configuration | error message: inet_aton() takes exactly 1 argument (2 given)
thomas-mangin commented 5 years ago

Thanks .. will investigate

thomas-mangin commented 5 years ago

not sure what I should do to close this ...

pguibert6WIND commented 5 years ago

Hi Thomas, please give me a bit more time to clarify better the need.