opiproject / opi-evpn-bridge

OPI gRPC to EVPN GW FRR bridge
Apache License 2.0
14 stars 13 forks source link

`CreateLogicalBridge` what about hard coded IP `10.0.0.100` ? #83

Closed glimchb closed 1 year ago

glimchb commented 1 year ago

I see no field for IP address

ip link add vni10 type vxlan id 10 local 10.0.0.100 dstport 4789 nolearning proxy
ip link set vni10 master br-tenant up
bridge vlan add dev vni10 vid 10 pvid untagged
bridge link set dev vni10 neigh_suppress on

but

message LogicalBridgeSpec {
    // the VLAN of the L2 domain
    uint32 vlan_id         = 1 [(google.api.field_behavior) = REQUIRED];
    //VXLAN VNI for the L2 EVPN. Also used as EVPN route target
    uint32 vni             = 2;
}

see hard-coded https://github.com/opiproject/opi-evpn-bridge/blob/b998cfea409b9e05dc38e608288d9e1b8f649590/pkg/evpn/bridge.go#L72-L73

@mardim91 please advise

mardim91 commented 1 year ago

Yes @glimchb as I have explained before maybe this is something that is missing from the API as the VRF spec have VTEP IP but the LogicalBridgeSpec it doesn't. Maybe we need to add this VTEP IP field to the API or completely remove it also from the VRF spec and pass it in both cases through evpn-gw-br configuration files. I will need to discuss this with @JanScheurich before we do any changes to the API. For now when it comes to the LogicalBridges you can pass the VTEP IP through configuration in evpn-gw-br.

glimchb commented 1 year ago

thanks @mardim91 I will leave this issue open for tracking

glimchb commented 1 year ago

@JanScheurich please advise

JanScheurich commented 1 year ago

The optional VTEP IP is indeed missing from the LogicalBridgeSpec (We have it in the VrfSpec for L3 EVPN). It was simply forgotten when extending to the L2 EVPN use case. We never actually used it in our prototype because our server implementation had a default VTEP configured. Please go ahead and create an API PR to add the vtep_id_prefix as optional argument.

glimchb commented 1 year ago

opened https://github.com/opiproject/opi-api/pull/349 please review