Exa-Networks / exabgp

The BGP swiss army knife of networking
Other
2.05k stars 440 forks source link

interface-set and discard are mutually exclusive #1209

Closed cwlumen closed 3 weeks ago

cwlumen commented 1 month ago

Describe the bug

When using an API call to announce a new route it appears the interface-set and the discard feature are mutually exclusive when sending updates to the process routes function. The command is accepted as valid but the route is send to the peer with both communities

A curl to my API with the new announcement in it

curl -X 'POST' -H 'Content-Type: application/json' -H 'accept: application/json' -d '{"command": "announce flow route destination 133.130.1.19/32  interface-set [ transitive:input-output:1234:10 transitive:input:1234:10 transitive:output:1234:10] discard" }' http://localhost:5001/api/v1/update_flowspec

I can see the announcement is accepted, and has both options, but when it gets announced to the peer ( the API line and actual payload) it will only have the interface-set extended community.

127.0.0.1 - - [06/Jun/2024 15:11:41] "POST /api/v1/update_flowspec HTTP/1.1" 200 -
15:11:41 | 11564  | process       | command from process routes : announce flow route destination 133.130.1.19/32  interface-set [ transitive:input-output:1234:10 transitive:input:1234:10 transitive:output:1234:10] discard 
15:11:41 | 11564  | reactor       | async | routes | announce flow route destination 133.130.1.19/32 interface-set [ transitive:input-output:1234:10 transitive:input:1234:10 transitive:output:1234:10 ] discard
15:11:41 | 11564  | configuration | . route            | 'destination' '133.130.1.19/32' 'interface-set' '[' 'transitive:input-output:1234:10' 'transitive:input:1234:10' 'transitive:output:1234:10' ']' 'discard'
15:11:41 | 11564  | api           | flow added to neighbor 133.130.1.85 local-ip 133.130.1.86 local-as 1234 peer-as 1234 router-id 10.10.20.20 family-allowed in-open, neighbor 133.130.1.89 local-ip 133.130.1.90 local-as 1234 peer-as 1234 router-id 10.10.20.20 family-allowed in-open : flow destination-ipv4 133.130.1.19/32 extended-community [ interface-set:input:1234:10 interface-set:input-output:1234:10 interface-set:output:1234:10 ]
15:11:41 | 11564  | process       | responding to routes : done
15:11:41 | 11564  | outgoing-1    | sending TCP payload (  79) FFFF FFFF FFFF FFFF FFFF FFFF FFFF FFFF 004F 0200 0000 3840 0101 0040 0200 4005 0400 0000 64C0 1018 0702 0000 04D2 C00A 0702 0000 04D2 400A 0702 0000 0D1C 800A 800E 0C00 0185 0000 0601 2085 8201 13
15:11:41 | 11564  | outgoing-1    | >> 1 UPDATE(s)

Everything works correctly when a peer is configured with both options in the config file. This announcement will have both the interface-set and the discard (rate limit) communities and send both to the router

neighbor 133.130.1.85 {
    inherit flowspec;
    description router1;
    local-address 133.130.1.86;
    flow {
        route foo {
            match {
               source 3.3.3.3/32;
            }
            scope {
                   interface-set [  non-transitive:input:1234:10 ];
            }
            then {
                discard;
            }
        }
    }
}

Order seems to be matter. If the discard keyword comes before interface-set, only the rate-limit community is sent.

curl -X 'POST' -H 'Content-Type: application/json' -H 'accept: application/json' -d '{"command": "announce flow route destination 133.130.1.219/32  discard interface-set [ transitive:input-output:1234:10 transitive:input:1234:10 transitive:output:1234:10]" }' http://localhost:5001/api/v1/update_flowspec
15:11:41 | 11564  | process       | command from process routes : announce flow route destination 133.130.1.219/32  discard interface-set [ transitive:input-output:1234:10 transitive:input:1234:10 transitive:output:1234:10] 
15:11:41 | 11564  | reactor       | async | routes | announce flow route destination 133.130.1.219/32 discard interface-set [ transitive:input-output:1234:10 transitive:input:1234:10 transitive:output:1234:10 ]
15:11:41 | 11564  | configuration | . route            | 'destination' '133.130.1.219/32' 'discard' 'interface-set' '[' 'transitive:input-output:1234:10' 'transitive:input:1234:10' 'transitive:output:1234:10' ']'
15:11:41 | 11564  | api           | flow added to neighbor 133.130.1.85 local-ip 133.130.1.86 local-as 1234 peer-as 1234 router-id 10.10.20.20 family-allowed in-open, neighbor 133.130.1.89 local-ip 133.130.1.90 local-as 1234 peer-as 1234 router-id 10.10.20.20 family-allowed in-open : flow destination-ipv4 133.130.1.219/32 extended-community rate-limit:0
15:11:41 | 11564  | process       | responding to routes : done
15:11:41 | 11564  | outgoing-1    | sending TCP payload (  63) FFFF FFFF FFFF FFFF FFFF FFFF FFFF FFFF 003F 0200 0000 2840 0101 0040 0200 4005 0400 0000 64C0 1008 8006 0000 0000 0000 800E 0C00 0185 0000 0601 2085 8201 DB
15:11:41 | 11564  | outgoing-1    | >> 1 UPDATE(s)

To Reproduce

Announce a new flow with both the discard and the interface-set arguments at the same time. Whichever one is first will be sent.

command: announce flow route destination 133.130.1.19/32  interface-set [ transitive:input-output:1234:10 transitive:input:1234:10 ] discard

Expected behavior

When making updates, both the interface-set and discard extended communities should be sent. The behavior is correct when a route is defined in the exabgp config file.

$ exabgp --version
ExaBGP : 4.2.21
Python : 3.10.7 (main, Jun  3 2024, 15:11:58) [GCC 11.4.0]
Uname  : Linux exabgp1 5.15.0-107-generic #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64
Root   : /home/nobody/ansible-env
pip3 list | grep exa
exabgp                4.2.21
thomas-mangin commented 1 month ago

I have added some test for this use case on master - not checked 4.2:

❯ ./qa/bin/functional encoding --list

The available tests are:

 0  api-add-remove            1  api-announce-processes-match 2  api-announce-star        
 3  api-announce              4  api-announcement          5  api-api                  
 6  api-attributes-path       7  api-attributes-vpn        8  api-attributes           
 9  api-broken-flow           A  api-check                 B  api-eor                  
 C  api-fast                  D  api-flow-merge            E  api-flow                 
 F  api-ipv4                  G  api-ipv6                  H  api-manual-eor           
 I  api-multi-neighbor        J  api-multiple-api          K  api-multisession         
 L  api-nexthop-self          M  api-nexthop               N  api-no-respawn           
 O  api-notification          P  api-open                  Q  api-reload               
 R  api-rib                   S  api-rr-rib                T  api-teardown             
 U  api-vpls                  V  api-vpnv4                 W  conf-addpath             
 X  conf-aggregator           Y  conf-attributes           Z  conf-ebgp                
 a  conf-extended-attributes  b  conf-flow-redirect        c  conf-flow                
 d  conf-generic-attribute    e  conf-group-limit          f  conf-group               
 g  conf-ipself4              h  conf-ipself6              i  conf-ipv46routes4family  
 j  conf-ipv46routes6family   k  conf-ipv6grouping         l  conf-l2vpn               
 m  conf-largecommunity       n  conf-name                 o  conf-new-v4              
 p  conf-new-v6               q  conf-no-asn4              r  conf-parity              
 s  conf-path-information     t  conf-prefix-sid           u  conf-split               
 v  conf-srv6-mup-v3          w  conf-srv6-mup             x  conf-template            
 y  conf-unknowncap           z  conf-vpn                  α  conf-watchdog            
❯ ./qa/bin/functional encoding --dry D
> ./qa/bin/functional encoding --server D --port 1803
> ./qa/bin/functional encoding --client D --port 1803

It is passing:

❯ ./qa/bin/functional encoding D      
 ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✓ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖

You can look at files with

./qa/bin/functional encoding --edit D

The API is using: announce flow route { match { source 2.2.2.2/32; } scope { interface-set [ non-transitive:input:3405770241:1 transitive:output:254:254 ]; } then { discard; } } which is translated to FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:004F:02:000000384001010040020040050400000064C010184702CAFFEE0140010702000000FE80FE8006000000000000800E0C000185000006022002020202 (raw:5:)

This is decoded as:

❯ ./sbin/exabgp decode 000000384001010040020040050400000064C010184702CAFFEE0140010702000000FE80FE8006000000000000800E0C000185000006022002020202 | jq
{
  "exabgp": "5.0.0",
  "time": 1717888731.297666,
  "host": "MacBook-Pro-3.local",
  "pid": 63926,
  "ppid": 81437,
  "counter": 1,
  "type": "update",
  "neighbor": {
    "address": {
      "local": "127.0.0.1",
      "peer": "127.0.0.1"
    },
    "asn": {
      "local": 65533,
      "peer": 65533
    },
    "direction": "in",
    "message": {
      "update": {
        "attribute": {
          "origin": "igp",
          "local-preference": 100,
          "extended-community": [
            {
              "value": 5116875327204835329,
              "string": "interface-set:input:3405770241:1"
            },
            {
              "value": 504966108235596030,
              "string": "interface-set:output:254:254"
            },
            {
              "value": 9225060886715039744,
              "string": "rate-limit:0"
            }
          ]
        },
        "announce": {
          "ipv4 flow": {
            "no-nexthop": [
              {
                "source-ipv4": [
                  "2.2.2.2/32"
                ],
                "string": "flow source-ipv4 2.2.2.2/32"
              }
            ]
          }
        }
      }
    }
  }
}

As there is no such thing as an HTTP/HTTPS gateway to ExaBGP, please see with the author of that code if either

I may backport the test to 4.2.

thomas-mangin commented 1 month ago

Looking more at your example, I added an extra test to have two "output" like you have ( [ transitive:input-output:1234:10 transitive:input:1234:10 transitive:output:1234:10] ) and this fails as only one is encoded, but I am not sure if this is correct or not by the RFC - I do not touch that code much nowdays. TODO ...

In all case, the discard got announced tho ...

cwlumen commented 1 month ago

The real goal is to have both an interface-set and the discard at the same time. I am not sure how the config from the config.ini file and updates that come from "process routes" function differ. It seems that any BGP update that comes from the process routes function will only send either the interface-set or the discard community, not both. Perhaps the issue is in how the process routes function takes in updates like these. If you could test sending in updates via the process routes function the issue should present itself.

process routes {
    run <xxx.py> ;
    encoder json;
}

# this will only send the interface-set
announce flow route destination 133.130.1.19/32  interface-set [ transitive:input-output:1234:10] discard
# this will only send the discard
announce flow route destination 133.130.1.19/32 discard interface-set [ transitive:input-output:1234:10]
thomas-mangin commented 1 month ago

The real goal is to have both an interface-set and the discard at the same time.

This is working on master with a test case demonstrating it.

cwlumen commented 4 weeks ago

I might be missing something, but that is not what I am seeing. Can you take a look? I remove the pip version installed, and pulled from main today and re-ran:

exabgp flowspec.ini -d
15:13:16 22652  welcome       Thank you for using ExaBGP
15:13:16 22652  version         5.0.0dev20240612151316
15:13:16 22652  location        /home/foo/git/exabgp-git
15:13:16 22652  python          3.10.7 (main, Jun  3 2024, 15:11:58) [GCC 11.4.0]
15:13:16 22652  platform        Linux exabgp1 5.15.0-107-generic #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64
15:13:20 22652  outgoing-1      >> 1 UPDATE(s)
15:13:20 22652  process         command from process routes : announce flow route destination 133.130.1.19/32  interface-set [ transitive:input:1234:10 ] discard 
15:13:20 22652  reactor         async | routes | announce flow route destination 133.130.1.19/32 interface-set [ transitive:input:1234:10 ] discard
127.0.0.1 - - [12/Jun/2024 15:13:20] "POST /api/v1/update_flowspec HTTP/1.1" 200 -
15:13:20 22652  configuration   . route            | 'destination' '133.130.1.19/32' 'interface-set' '[' 'transitive:input:1234:10' ']' 'discard'
15:13:20 22652  rib             insert flow destination-ipv4 133.130.1.19/32 extended-community interface-set:input:1234:10
15:13:20 22652  processes     flow added to neighbor 133.130.1.85 local-ip 133.130.1.86 local-as 1234 peer-as 1234 router-id 10.10.20.20 family-allowed in-open : flow destination-ipv4 133.130.1.19/32 extended-community interface-set:input:1234:10
15:13:20 22652  process         responding to routes : done
15:13:20 22652  outgoing-1      sending TCP payload (  63) FFFF FFFF FFFF FFFF FFFF FFFF FFFF FFFF 003F 0200 0000 2840 0101 0040 0200 4005 0400 0000 64C0 1008 0702 0000 04D2 400A 800E 0C00 0185 0000 0601 2085 8201 13
15:13:20 22652  outgoing-1      >> 1 UPDATE(s)
15:13:20 22652  process         command from process routes : announce flow route destination 133.130.1.219/32  discard interface-set [ transitive:input:1234:10] 
15:13:20 22652  reactor         async | routes | announce flow route destination 133.130.1.219/32 discard interface-set [ transitive:input:1234:10 ]
15:13:20 22652  configuration   . route            | 'destination' '133.130.1.219/32' 'discard' 'interface-set' '[' 'transitive:input:1234:10' ']'
127.0.0.1 - - [12/Jun/2024 15:13:20] "POST /api/v1/update_flowspec HTTP/1.1" 200 -
15:13:20 22652  rib             insert flow destination-ipv4 133.130.1.219/32 extended-community rate-limit:0
15:13:20 22652  processes     flow added to neighbor 133.130.1.85 local-ip 133.130.1.86 local-as 1234 peer-as 1234 router-id 10.10.20.20 family-allowed in-open : flow destination-ipv4 133.130.1.219/32 extended-community rate-limit:0
15:13:20 22652  process         responding to routes : done
15:13:20 22652  outgoing-1      sending TCP payload (  63) FFFF FFFF FFFF FFFF FFFF FFFF FFFF FFFF 003F 0200 0000 2840 0101 0040 0200 4005 0400 0000 64C0 1008 8006 0000 0000 0000 800E 0C00 0185 0000 0601 2085 8201 DB
15:13:20 22652  outgoing-1      >> 1 UPDATE(s)

It seems to happen at this point according to the debug, the first has lost the rate-limit, and the second has lost the interface-set. It seems like there are 4-5 steps in the debug output: "process", "reactor", configuration", "rib". The debug looks correct (both attributes are there) until we get to the rib step. The rib debug entry only shows one or the other (it looks like which ever one came first in the command).


15:13:20 22652  rib             insert flow destination-ipv4 133.130.1.19/32 extended-community interface-set:input:1234:10

15:13:20 22652  rib             insert flow destination-ipv4 133.130.1.219/32 extended-community rate-limit:0
thomas-mangin commented 4 weeks ago

Hopefully, this patch resolves the issue when {} are not used to separate the terms of the rule.

I could have saved some time by arguing that the API was not used as it should have been 😜 but as I never documented it, I have changed the code instead.

Thank you for working with me to get this problem resolve.

thomas-mangin commented 4 weeks ago

Marked as need testing but there is a test case for it so it really just need confirmation that @cwlumen is happy about the resolution.

cwlumen commented 3 weeks ago

That did it. Thanks for updating this!