Evpn refactor #6883
Merged
Evpn refactor #6883
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
polychaeta
reviewed
Aug 10, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/ba7827f09c3e4610bf5fbc4bae5225eb/raw/7b850d61546d606469371bb331553d0b7a83b352/cr_6883_1597052828.diff | git apply
diff --git a/zebra/zebra_evpn_mh.c b/zebra/zebra_evpn_mh.c
index ffd47418a..8ee2c2a96 100644
--- a/zebra/zebra_evpn_mh.c
+++ b/zebra/zebra_evpn_mh.c
@@ -59,7 +59,7 @@ DEFINE_MTYPE_STATIC(ZEBRA, ZES_VTEP, "VTEP attached to the ES");
static void zebra_evpn_es_get_one_base_evpn(void);
static int zebra_evpn_es_evi_send_to_client(struct zebra_evpn_es *es,
- zebra_evpn_t *zevpn, bool add);
+ zebra_evpn_t *zevpn, bool add);
static void zebra_evpn_local_es_del(struct zebra_evpn_es *es);
static int zebra_evpn_local_es_update(struct zebra_if *zif, uint32_t lid,
struct ethaddr *sysmac);
@@ -96,7 +96,7 @@ RB_GENERATE(zebra_es_evi_rb_head, zebra_evpn_es_evi,
* tables.
*/
static struct zebra_evpn_es_evi *zebra_evpn_es_evi_new(struct zebra_evpn_es *es,
- zebra_evpn_t *zevpn)
+ zebra_evpn_t *zevpn)
{
struct zebra_evpn_es_evi *es_evi;
@@ -116,8 +116,8 @@ static struct zebra_evpn_es_evi *zebra_evpn_es_evi_new(struct zebra_evpn_es *es,
listnode_add(es->es_evi_list, &es_evi->es_listnode);
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
- zlog_debug("es %s evi %d new",
- es_evi->es->esi_str, es_evi->zevpn->vni);
+ zlog_debug("es %s evi %d new", es_evi->es->esi_str,
+ es_evi->zevpn->vni);
return es_evi;
}
@@ -142,9 +142,9 @@ static void zebra_evpn_es_evi_re_eval_send_to_client(
old_ready = !!(es_evi->flags & ZEBRA_EVPNES_EVI_READY_FOR_BGP);
/* ES and L2-VNI have to be individually ready for BGP */
- if ((es_evi->flags & ZEBRA_EVPNES_EVI_LOCAL) &&
- (es_evi->es->flags & ZEBRA_EVPNES_READY_FOR_BGP) &&
- zebra_evpn_send_to_client_ok(es_evi->zevpn))
+ if ((es_evi->flags & ZEBRA_EVPNES_EVI_LOCAL)
+ && (es_evi->es->flags & ZEBRA_EVPNES_READY_FOR_BGP)
+ && zebra_evpn_send_to_client_ok(es_evi->zevpn))
es_evi->flags |= ZEBRA_EVPNES_EVI_READY_FOR_BGP;
else
es_evi->flags &= ~ZEBRA_EVPNES_EVI_READY_FOR_BGP;
@@ -156,10 +156,10 @@ static void zebra_evpn_es_evi_re_eval_send_to_client(
if (new_ready)
zebra_evpn_es_evi_send_to_client(es_evi->es, es_evi->zevpn,
- true /* add */);
+ true /* add */);
else
zebra_evpn_es_evi_send_to_client(es_evi->es, es_evi->zevpn,
- false /* add */);
+ false /* add */);
}
/* remove the ES-EVI from the per-L2-VNI and per-ES tables and free
@@ -171,8 +171,8 @@ static void zebra_evpn_es_evi_free(struct zebra_evpn_es_evi *es_evi)
zebra_evpn_t *zevpn = es_evi->zevpn;
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
- zlog_debug("es %s evi %d free",
- es_evi->es->esi_str, es_evi->zevpn->vni);
+ zlog_debug("es %s evi %d free", es_evi->es->esi_str,
+ es_evi->zevpn->vni);
/* remove from the ES's VNI list */
list_delete_node(es->es_evi_list, &es_evi->es_listnode);
@@ -185,8 +185,8 @@ static void zebra_evpn_es_evi_free(struct zebra_evpn_es_evi *es_evi)
}
/* find the ES-EVI in the per-L2-VNI RB tree */
-static struct zebra_evpn_es_evi *zebra_evpn_es_evi_find(
- struct zebra_evpn_es *es, zebra_evpn_t *zevpn)
+static struct zebra_evpn_es_evi *
+zebra_evpn_es_evi_find(struct zebra_evpn_es *es, zebra_evpn_t *zevpn)
{
struct zebra_evpn_es_evi es_evi;
@@ -202,24 +202,24 @@ static void zebra_evpn_local_es_evi_do_del(struct zebra_evpn_es_evi *es_evi)
return;
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
- zlog_debug("local es %s evi %d del",
- es_evi->es->esi_str, es_evi->zevpn->vni);
+ zlog_debug("local es %s evi %d del", es_evi->es->esi_str,
+ es_evi->zevpn->vni);
if (es_evi->flags & ZEBRA_EVPNES_EVI_READY_FOR_BGP) {
/* send a del only if add was sent for it earlier */
- zebra_evpn_es_evi_send_to_client(es_evi->es,
- es_evi->zevpn, false /* add */);
+ zebra_evpn_es_evi_send_to_client(es_evi->es, es_evi->zevpn,
+ false /* add */);
}
/* delete it from the EVPN's local list */
list_delete_node(es_evi->zevpn->local_es_evi_list,
- &es_evi->l2vni_listnode);
+ &es_evi->l2vni_listnode);
es_evi->flags &= ~ZEBRA_EVPNES_EVI_LOCAL;
zebra_evpn_es_evi_free(es_evi);
}
static void zebra_evpn_local_es_evi_del(struct zebra_evpn_es *es,
- zebra_evpn_t *zevpn)
+ zebra_evpn_t *zevpn)
{
struct zebra_evpn_es_evi *es_evi;
@@ -230,7 +230,7 @@ static void zebra_evpn_local_es_evi_del(struct zebra_evpn_es *es,
/* Create an ES-EVI if it doesn't already exist and tell BGP */
static void zebra_evpn_local_es_evi_add(struct zebra_evpn_es *es,
- zebra_evpn_t *zevpn)
+ zebra_evpn_t *zevpn)
{
struct zebra_evpn_es_evi *es_evi;
@@ -242,7 +242,7 @@ static void zebra_evpn_local_es_evi_add(struct zebra_evpn_es *es,
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
zlog_debug("local es %s evi %d add",
- es_evi->es->esi_str, es_evi->zevpn->vni);
+ es_evi->es->esi_str, es_evi->zevpn->vni);
es_evi->flags |= ZEBRA_EVPNES_EVI_LOCAL;
/* add to the EVPN's local list */
listnode_init(&es_evi->l2vni_listnode, es_evi);
@@ -264,9 +264,8 @@ static void zebra_evpn_es_evi_show_entry(struct vty *vty,
if (es_evi->flags & ZEBRA_EVPNES_EVI_LOCAL)
strlcat(type_str, "L", sizeof(type_str));
- vty_out(vty, "%-8d %-30s %-4s\n",
- es_evi->zevpn->vni, es_evi->es->esi_str,
- type_str);
+ vty_out(vty, "%-8d %-30s %-4s\n", es_evi->zevpn->vni,
+ es_evi->es->esi_str, type_str);
}
}
@@ -282,8 +281,8 @@ static void zebra_evpn_es_evi_show_entry_detail(struct vty *vty,
if (es_evi->flags & ZEBRA_EVPNES_EVI_LOCAL)
strlcat(type_str, "L", sizeof(type_str));
- vty_out(vty, "VNI %d ESI: %s\n",
- es_evi->zevpn->vni, es_evi->es->esi_str);
+ vty_out(vty, "VNI %d ESI: %s\n", es_evi->zevpn->vni,
+ es_evi->es->esi_str);
vty_out(vty, " Type: %s\n", type_str);
vty_out(vty, " Ready for BGP: %s\n",
(es_evi->flags &
@@ -294,11 +293,12 @@ static void zebra_evpn_es_evi_show_entry_detail(struct vty *vty,
}
static void zebra_evpn_es_evi_show_one_evpn(zebra_evpn_t *zevpn,
- struct vty *vty, json_object *json, int detail)
+ struct vty *vty, json_object *json,
+ int detail)
{
struct zebra_evpn_es_evi *es_evi;
- RB_FOREACH(es_evi, zebra_es_evi_rb_head, &zevpn->es_evi_rb_tree) {
+ RB_FOREACH (es_evi, zebra_es_evi_rb_head, &zevpn->es_evi_rb_tree) {
if (detail)
zebra_evpn_es_evi_show_entry_detail(vty, es_evi, json);
else
@@ -313,13 +313,13 @@ struct evpn_mh_show_ctx {
};
static void zebra_evpn_es_evi_show_one_evpn_hash_cb(struct hash_bucket *bucket,
- void *ctxt)
+ void *ctxt)
{
zebra_evpn_t *zevpn = (zebra_evpn_t *)bucket->data;
struct evpn_mh_show_ctx *wctx = (struct evpn_mh_show_ctx *)ctxt;
- zebra_evpn_es_evi_show_one_evpn(zevpn, wctx->vty,
- wctx->json, wctx->detail);
+ zebra_evpn_es_evi_show_one_evpn(zevpn, wctx->vty, wctx->json,
+ wctx->detail);
}
void zebra_evpn_es_evi_show(struct vty *vty, bool uj, int detail)
@@ -341,7 +341,7 @@ void zebra_evpn_es_evi_show(struct vty *vty, bool uj, int detail)
}
/* Display all L2-VNIs */
hash_iterate(zvrf->evpn_table, zebra_evpn_es_evi_show_one_evpn_hash_cb,
- &wctx);
+ &wctx);
}
void zebra_evpn_es_evi_show_vni(struct vty *vty, bool uj, vni_t vni, int detail)
@@ -381,8 +381,8 @@ void zebra_evpn_evpn_es_cleanup(zebra_evpn_t *zevpn)
struct zebra_evpn_es_evi *es_evi;
struct zebra_evpn_es_evi *es_evi_next;
- RB_FOREACH_SAFE(es_evi, zebra_es_evi_rb_head,
- &zevpn->es_evi_rb_tree, es_evi_next) {
+ RB_FOREACH_SAFE (es_evi, zebra_es_evi_rb_head, &zevpn->es_evi_rb_tree,
+ es_evi_next) {
zebra_evpn_local_es_evi_do_del(es_evi);
}
@@ -517,14 +517,15 @@ static void zebra_evpn_acc_bd_free_on_deref(struct zebra_evpn_access_bd *acc_bd)
/* called when a EVPN-L2VNI is set or cleared against a BD */
static void zebra_evpn_acc_bd_evpn_set(struct zebra_evpn_access_bd *acc_bd,
- zebra_evpn_t *zevpn, zebra_evpn_t *old_zevpn)
+ zebra_evpn_t *zevpn,
+ zebra_evpn_t *old_zevpn)
{
struct zebra_if *zif;
struct listnode *node;
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
- zlog_debug("access vlan %d l2-vni %u set",
- acc_bd->vid, zevpn ? zevpn->vni : 0);
+ zlog_debug("access vlan %d l2-vni %u set", acc_bd->vid,
+ zevpn ? zevpn->vni : 0);
for (ALL_LIST_ELEMENTS_RO(acc_bd->mbr_zifs, node, zif)) {
if (!zif->es_info.es)
@@ -604,7 +605,7 @@ void zebra_evpn_vl_vxl_deref(uint16_t vid, struct zebra_if *vxlan_zif)
/* handle EVPN add/del */
void zebra_evpn_vxl_evpn_set(struct zebra_if *zif, zebra_evpn_t *zevpn,
- bool set)
+ bool set)
{
struct zebra_l2info_vxlan *vxl;
struct zebra_evpn_access_bd *acc_bd;
@@ -701,7 +702,7 @@ static void zebra_evpn_acc_vl_show_entry_detail(struct vty *vty,
acc_bd->vxlan_zif ?
acc_bd->vxlan_zif->ifp->name : "-");
vty_out(vty, " L2-VNI: %d\n",
- acc_bd->zevpn ? acc_bd->zevpn->vni : 0);
+ acc_bd->zevpn ? acc_bd->zevpn->vni : 0);
vty_out(vty, " Member Count: %d\n",
listcount(acc_bd->mbr_zifs));
vty_out(vty, " Members: \n");
@@ -715,12 +716,10 @@ static void zebra_evpn_acc_vl_show_entry(struct vty *vty,
struct zebra_evpn_access_bd *acc_bd, json_object *json)
{
if (!json)
- vty_out(vty, "%-5u %21s %-8d %u\n",
- acc_bd->vid,
- acc_bd->vxlan_zif ?
- acc_bd->vxlan_zif->ifp->name : "-",
- acc_bd->zevpn ? acc_bd->zevpn->vni : 0,
- listcount(acc_bd->mbr_zifs));
+ vty_out(vty, "%-5u %21s %-8d %u\n", acc_bd->vid,
+ acc_bd->vxlan_zif ? acc_bd->vxlan_zif->ifp->name : "-",
+ acc_bd->zevpn ? acc_bd->zevpn->vni : 0,
+ listcount(acc_bd->mbr_zifs));
}
static void zebra_evpn_acc_vl_show_hash(struct hash_bucket *bucket, void *ctxt)
@@ -1623,7 +1622,7 @@ bool zebra_evpn_es_mac_ref(zebra_mac_t *mac, esi_t *esi)
/* Inform BGP about local ES-EVI add or del */
static int zebra_evpn_es_evi_send_to_client(struct zebra_evpn_es *es,
- zebra_evpn_t *zevpn, bool add)
+ zebra_evpn_t *zevpn, bool add)
{
struct zserv *client;
struct stream *s;
@@ -1646,9 +1645,8 @@ static int zebra_evpn_es_evi_send_to_client(struct zebra_evpn_es *es,
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
zlog_debug("send %s local es %s evi %u to %s",
- add ? "add" : "del",
- es->esi_str, zevpn->vni,
- zebra_route_string(client->proto));
+ add ? "add" : "del", es->esi_str, zevpn->vni,
+ zebra_route_string(client->proto));
client->local_es_add_cnt++;
return zserv_send_message(client, s);
@@ -1998,14 +1996,13 @@ void zebra_evpn_es_set_base_evpn(zebra_evpn_t *zevpn)
return;
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
- zlog_debug("es base vni set to %d",
- zevpn->vni);
+ zlog_debug("es base vni set to %d", zevpn->vni);
zmh_info->es_base_evpn = zevpn;
}
/* update local VTEP-IP */
- if (zmh_info->es_originator_ip.s_addr ==
- zmh_info->es_base_evpn->local_vtep_ip.s_addr)
+ if (zmh_info->es_originator_ip.s_addr
+ == zmh_info->es_base_evpn->local_vtep_ip.s_addr)
return;
zmh_info->es_originator_ip.s_addr =
@@ -2013,7 +2010,7 @@ void zebra_evpn_es_set_base_evpn(zebra_evpn_t *zevpn)
if (IS_ZEBRA_DEBUG_EVPN_MH_ES)
zlog_debug("es originator ip set to %s",
- inet_ntoa(zmh_info->es_base_evpn->local_vtep_ip));
+ inet_ntoa(zmh_info->es_base_evpn->local_vtep_ip));
/* if originator ip changes we need to update bgp */
for (ALL_LIST_ELEMENTS_RO(zmh_info->local_es_list, node, es)) {
diff --git a/zebra/zebra_evpn_mh.h b/zebra/zebra_evpn_mh.h
index ed62677e3..d7fc81620 100644
--- a/zebra/zebra_evpn_mh.h
+++ b/zebra/zebra_evpn_mh.h
@@ -201,7 +201,7 @@ extern void zebra_evpn_if_cleanup(struct zebra_if *zif);
extern void zebra_evpn_evpn_es_init(zebra_evpn_t *zevpn);
extern void zebra_evpn_evpn_es_cleanup(zebra_evpn_t *zevpn);
extern void zebra_evpn_vxl_evpn_set(struct zebra_if *zif, zebra_evpn_t *zevpn,
- bool set);
+ bool set);
extern void zebra_evpn_es_set_base_evpn(zebra_evpn_t *zevpn);
extern void zebra_evpn_es_clear_base_evpn(zebra_evpn_t *zevpn);
extern void zebra_evpn_vl_vxl_ref(uint16_t vid, struct zebra_if *vxlan_zif);
diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c
index 2dbdb4a84..fc5108781 100644
--- a/zebra/zebra_vxlan.c
+++ b/zebra/zebra_vxlan.c
@@ -73,7 +73,8 @@ static void zl3vni_print_nh(zebra_neigh_t *n, struct vty *vty,
json_object *json);
static void zl3vni_print_rmac(zebra_mac_t *zrmac, struct vty *vty,
json_object *json);
-static void zevpn_print_mac_hash_all_evpn(struct hash_bucket *bucket, void *ctxt);
+static void zevpn_print_mac_hash_all_evpn(struct hash_bucket *bucket,
+ void *ctxt);
/* l3-vni next-hop neigh related APIs */
static zebra_neigh_t *zl3vni_nh_lookup(zebra_l3vni_t *zl3vni,
@@ -167,7 +168,7 @@ static uint32_t rb_host_count(struct host_rb_tree_entry *hrbe)
* Print neighbors for all EVPN.
*/
static void zevpn_print_neigh_hash_all_evpn(struct hash_bucket *bucket,
- void **args)
+ void **args)
{
struct vty *vty;
json_object *json = NULL, *json_evpn = NULL;
@@ -234,7 +235,7 @@ static void zevpn_print_neigh_hash_all_evpn(struct hash_bucket *bucket,
* Print neighbors for all EVPNs in detail.
*/
static void zevpn_print_neigh_hash_all_evpn_detail(struct hash_bucket *bucket,
- void **args)
+ void **args)
{
struct vty *vty;
json_object *json = NULL, *json_evpn = NULL;
@@ -305,8 +306,7 @@ static void zl3vni_print_nh(zebra_neigh_t *n, struct vty *vty,
ipaddr2str(&n->ip, buf2, sizeof(buf2)));
vty_out(vty, " RMAC: %s\n",
prefix_mac2str(&n->emac, buf1, sizeof(buf1)));
- vty_out(vty, " Refcount: %d\n",
- rb_host_count(&n->host_rb));
+ vty_out(vty, " Refcount: %d\n", rb_host_count(&n->host_rb));
vty_out(vty, " Prefixes:\n");
RB_FOREACH (hle, host_rb_tree_entry, &n->host_rb)
vty_out(vty, " %s\n",
@@ -321,9 +321,10 @@ static void zl3vni_print_nh(zebra_neigh_t *n, struct vty *vty,
json_object_int_add(json, "refCount",
rb_host_count(&n->host_rb));
RB_FOREACH (hle, host_rb_tree_entry, &n->host_rb)
- json_object_array_add(json_hosts,
- json_object_new_string(prefix2str(
- &hle->p, buf2, sizeof(buf2))));
+ json_object_array_add(
+ json_hosts,
+ json_object_new_string(prefix2str(
+ &hle->p, buf2, sizeof(buf2))));
json_object_object_add(json, "prefixList", json_hosts);
}
}
@@ -370,7 +371,8 @@ static void zl3vni_print_rmac(zebra_mac_t *zrmac, struct vty *vty,
/*
* Print MACs for all EVPNs.
*/
-static void zevpn_print_mac_hash_all_evpn(struct hash_bucket *bucket, void *ctxt)
+static void zevpn_print_mac_hash_all_evpn(struct hash_bucket *bucket,
+ void *ctxt)
{
struct vty *vty;
json_object *json = NULL, *json_evpn = NULL;
@@ -409,8 +411,8 @@ static void zevpn_print_mac_hash_all_evpn(struct hash_bucket *bucket, void *ctxt
vty_out(vty,
"Flags: N=sync-neighs, I=local-inactive, P=peer-active, X=peer-proxy\n");
vty_out(vty, "%-17s %-6s %-5s %-30s %-5s %s\n", "MAC",
- "Type", "Flags", "Intf/Remote ES/VTEP",
- "VLAN", "Seq #'s");
+ "Type", "Flags", "Intf/Remote ES/VTEP", "VLAN",
+ "Seq #'s");
} else
json_object_int_add(json_evpn, "numMacs", num_macs);
}
@@ -445,7 +447,7 @@ static void zevpn_print_mac_hash_all_evpn(struct hash_bucket *bucket, void *ctxt
* Print MACs in detail for all EVPNs.
*/
static void zevpn_print_mac_hash_all_evpn_detail(struct hash_bucket *bucket,
- void *ctxt)
+ void *ctxt)
{
struct vty *vty;
json_object *json = NULL, *json_evpn = NULL;
@@ -775,8 +777,8 @@ static void zl3vni_print_hash_detail(struct hash_bucket *bucket, void *data)
zl3vni = (zebra_l3vni_t *)bucket->data;
- zebra_vxlan_print_vni(vty, zes->zvrf, zl3vni->vni,
- use_json, json_array);
+ zebra_vxlan_print_vni(vty, zes->zvrf, zl3vni->vni, use_json,
+ json_array);
if (!use_json)
vty_out(vty, "\n");
@@ -899,11 +901,14 @@ static void zevpn_build_hash_table(void)
zl3vni->mac_vlan_if = zl3vni_map_to_mac_vlan_if(zl3vni);
if (IS_ZEBRA_DEBUG_VXLAN)
- zlog_debug("create l3vni %u svi_if %s mac_vlan_if %s",
- vni, zl3vni->svi_if ? zl3vni->svi_if->name
- : "NIL",
- zl3vni->mac_vlan_if ?
- zl3vni->mac_vlan_if->name : "NIL");
+ zlog_debug(
+ "create l3vni %u svi_if %s mac_vlan_if %s",
+ vni,
+ zl3vni->svi_if ? zl3vni->svi_if->name
+ : "NIL",
+ zl3vni->mac_vlan_if
+ ? zl3vni->mac_vlan_if->name
+ : "NIL");
if (is_l3vni_oper_up(zl3vni))
zebra_vxlan_process_l3vni_oper_up(zl3vni);
@@ -917,7 +922,8 @@ static void zevpn_build_hash_table(void)
ifp->name, ifp->ifindex, vni,
inet_ntoa(vxl->vtep_ip));
- /* EVPN hash entry is expected to exist, if the BGP process is killed */
+ /* EVPN hash entry is expected to exist, if the BGP
+ * process is killed */
zevpn = zebra_evpn_lookup(vni);
if (zevpn) {
zlog_debug(
@@ -928,8 +934,8 @@ static void zevpn_build_hash_table(void)
* Inform BGP if intf is up and mapped to
* bridge.
*/
- if (if_is_operative(ifp) &&
- zif->brslave_info.br_if)
+ if (if_is_operative(ifp)
+ && zif->brslave_info.br_if)
zebra_evpn_send_add_to_client(zevpn);
/* Send Local MAC-entries to client */
@@ -946,15 +952,15 @@ static void zevpn_build_hash_table(void)
return;
}
- if (zevpn->local_vtep_ip.s_addr !=
- vxl->vtep_ip.s_addr ||
- zevpn->mcast_grp.s_addr !=
- vxl->mcast_grp.s_addr) {
+ if (zevpn->local_vtep_ip.s_addr
+ != vxl->vtep_ip.s_addr
+ || zevpn->mcast_grp.s_addr
+ != vxl->mcast_grp.s_addr) {
zebra_vxlan_sg_deref(
zevpn->local_vtep_ip,
zevpn->mcast_grp);
zebra_vxlan_sg_ref(vxl->vtep_ip,
- vxl->mcast_grp);
+ vxl->mcast_grp);
zevpn->local_vtep_ip = vxl->vtep_ip;
zevpn->mcast_grp = vxl->mcast_grp;
/* on local vtep-ip check if ES
@@ -969,7 +975,7 @@ static void zevpn_build_hash_table(void)
if (vlan_if) {
zevpn->vrf_id = vlan_if->vrf_id;
zl3vni = zl3vni_from_vrf(
- vlan_if->vrf_id);
+ vlan_if->vrf_id);
if (zl3vni)
listnode_add_sort(
zl3vni->l2vnis, zevpn);
@@ -979,8 +985,8 @@ static void zevpn_build_hash_table(void)
* Inform BGP if intf is up and mapped to
* bridge.
*/
- if (if_is_operative(ifp) &&
- zif->brslave_info.br_if)
+ if (if_is_operative(ifp)
+ && zif->brslave_info.br_if)
zebra_evpn_send_add_to_client(zevpn);
}
}
@@ -1158,8 +1164,8 @@ static int zl3vni_rmac_install(zebra_l3vni_t *zl3vni, zebra_mac_t *zrmac)
else
vid = 0;
- res = dplane_rem_mac_add(zl3vni->vxlan_if, br_ifp, vid,
- &zrmac->macaddr, zrmac->fwd_info.r_vtep_ip, 0, 0,
+ res = dplane_rem_mac_add(zl3vni->vxlan_if, br_ifp, vid, &zrmac->macaddr,
+ zrmac->fwd_info.r_vtep_ip, 0, 0,
false /*was_static*/);
if (res != ZEBRA_DPLANE_REQUEST_FAILURE)
return 0;
@@ -1187,8 +1193,8 @@ static int zl3vni_rmac_uninstall(zebra_l3vni_t *zl3vni, zebra_mac_t *zrmac)
if (IS_ZEBRA_DEBUG_VXLAN)
zlog_debug(
"RMAC %s on L3-VNI %u hash %p couldn't be uninstalled - no vxlan_if",
- prefix_mac2str(&zrmac->macaddr,
- buf, sizeof(buf)),
+ prefix_mac2str(&zrmac->macaddr, buf,
+ sizeof(buf)),
zl3vni->vni, zl3vni);
return -1;
}
@@ -1209,8 +1215,8 @@ static int zl3vni_rmac_uninstall(zebra_l3vni_t *zl3vni, zebra_mac_t *zrmac)
else
vid = 0;
- res = dplane_rem_mac_del(zl3vni->vxlan_if, br_ifp, vid,
- &zrmac->macaddr, zrmac->fwd_info.r_vtep_ip);
+ res = dplane_rem_mac_del(zl3vni->vxlan_if, br_ifp, vid, &zrmac->macaddr,
+ zrmac->fwd_info.r_vtep_ip);
if (res != ZEBRA_DPLANE_REQUEST_FAILURE)
return 0;
else
@@ -1231,7 +1237,7 @@ static int zl3vni_remote_rmac_add(zebra_l3vni_t *zl3vni,
zrmac = zl3vni_rmac_lookup(zl3vni, rmac);
if (!zrmac) {
- /* Create the RMAC entry, or update its vtep, if necessary. */
+ /* Create the RMAC entry, or update its vtep, if necessary. */
zrmac = zl3vni_rmac_add(zl3vni, rmac);
if (!zrmac) {
zlog_debug(
@@ -1276,7 +1282,7 @@ static int zl3vni_remote_rmac_add(zebra_l3vni_t *zl3vni,
/* handle rmac delete */
static void zl3vni_remote_rmac_del(zebra_l3vni_t *zl3vni, zebra_mac_t *zrmac,
- struct prefix *host_prefix)
+ struct prefix *host_prefix)
{
rb_delete_host(&zrmac->host_rb, host_prefix);
@@ -1389,7 +1395,7 @@ static int zl3vni_nh_install(zebra_l3vni_t *zl3vni, zebra_neigh_t *n)
flags |= DPLANE_NTF_ROUTER;
dplane_rem_neigh_add(zl3vni->svi_if, &n->ip, &n->emac, flags,
- false /*was_static*/);
+ false /*was_static*/);
return ret;
}
@@ -1441,12 +1447,13 @@ static int zl3vni_remote_nh_add(zebra_l3vni_t *zl3vni,
zl3vni_nh_install(zl3vni, nh);
} else if (memcmp(&nh->emac, rmac, ETH_ALEN) != 0) {
if (IS_ZEBRA_DEBUG_VXLAN)
- zlog_debug("L3VNI %u RMAC change(%s --> %s) for nexthop %s, prefix %s",
- zl3vni->vni,
- prefix_mac2str(&nh->emac, buf, sizeof(buf)),
- prefix_mac2str(rmac, buf1, sizeof(buf1)),
- ipaddr2str(vtep_ip, buf2, sizeof(buf2)),
- prefix2str(host_prefix, buf3, sizeof(buf3)));
+ zlog_debug(
+ "L3VNI %u RMAC change(%s --> %s) for nexthop %s, prefix %s",
+ zl3vni->vni,
+ prefix_mac2str(&nh->emac, buf, sizeof(buf)),
+ prefix_mac2str(rmac, buf1, sizeof(buf1)),
+ ipaddr2str(vtep_ip, buf2, sizeof(buf2)),
+ prefix2str(host_prefix, buf3, sizeof(buf3)));
memcpy(&nh->emac, rmac, ETH_ALEN);
/* install (update) the nh neigh in kernel */
@@ -1670,7 +1677,7 @@ struct interface *zl3vni_map_to_svi_if(zebra_l3vni_t *zl3vni)
struct interface *zl3vni_map_to_mac_vlan_if(zebra_l3vni_t *zl3vni)
{
- struct zebra_if *zif = NULL; /* zebra_if for vxlan_if */
+ struct zebra_if *zif = NULL; /* zebra_if for vxlan_if */
if (!zl3vni)
return NULL;
@@ -1790,7 +1797,7 @@ static int zl3vni_send_add_to_client(zebra_l3vni_t *zl3vni)
{
struct stream *s = NULL;
struct zserv *client = NULL;
- struct ethaddr svi_rmac, vrr_rmac = {.octet = {0} };
+ struct ethaddr svi_rmac, vrr_rmac = {.octet = {0}};
struct zebra_vrf *zvrf;
char buf[ETHER_ADDR_STRLEN];
char buf1[ETHER_ADDR_STRLEN];
@@ -3120,7 +3127,7 @@ static void zevpn_clear_dup_mac_hash(struct hash_bucket *bucket, void *ctxt)
}
static void zevpn_clear_dup_detect_hash_vni_all(struct hash_bucket *bucket,
- void **args)
+ void **args)
{
zebra_evpn_t *zevpn;
struct zebra_vrf *zvrf;
@@ -3145,9 +3152,9 @@ static void zevpn_clear_dup_detect_hash_vni_all(struct hash_bucket *bucket,
memset(&m_wctx, 0, sizeof(struct mac_walk_ctx));
m_wctx.zevpn = zevpn;
m_wctx.zvrf = zvrf;
- hash_iterate(zevpn->mac_table, zevpn_clear_dup_mac_hash, &m_wctx);
+ hash_iterate(zevpn->mac_table, zevpn_clear_dup_mac_hash,
+ &m_wctx);
}
-
}
int zebra_vxlan_clear_dup_detect_vni_all(struct zebra_vrf *zvrf)
@@ -3160,8 +3167,9 @@ int zebra_vxlan_clear_dup_detect_vni_all(struct zebra_vrf *zvrf)
args[0] = zvrf;
hash_iterate(zvrf->evpn_table,
- (void (*)(struct hash_bucket *, void *))
- zevpn_clear_dup_detect_hash_vni_all, args);
+ (void (*)(struct hash_bucket *,
+ void *))zevpn_clear_dup_detect_hash_vni_all,
+ args);
return 0;
}
@@ -3193,7 +3201,8 @@ int zebra_vxlan_clear_dup_detect_vni(struct zebra_vrf *zvrf, vni_t vni)
memset(&m_wctx, 0, sizeof(struct mac_walk_ctx));
m_wctx.zevpn = zevpn;
m_wctx.zvrf = zvrf;
- hash_iterate(zevpn->mac_table, zevpn_clear_dup_mac_hash, &m_wctx);
+ hash_iterate(zevpn->mac_table, zevpn_clear_dup_mac_hash,
+ &m_wctx);
}
return 0;
@@ -3585,8 +3594,7 @@ int zebra_vxlan_handle_kernel_neigh_update(struct interface *ifp,
prefix_mac2str(macaddr, buf, sizeof(buf)), ifp->name,
ifp->ifindex, state, is_ext ? "ext-learned " : "",
is_router ? "router " : "",
- local_inactive ? "local_inactive " : "",
- zevpn->vni);
+ local_inactive ? "local_inactive " : "", zevpn->vni);
/* Is this about a local neighbor or a remote one? */
if (!is_ext)
@@ -4177,7 +4185,8 @@ void zebra_vxlan_remote_vtep_add(ZAPI_HANDLER_ARGS)
if (zvtep)
zebra_evpn_vtep_install(zevpn, zvtep);
else
- flog_err(EC_ZEBRA_VTEP_ADD_FAILED,
+ flog_err(
+ EC_ZEBRA_VTEP_ADD_FAILED,
"Failed to add remote VTEP, VNI %u zevpn %p",
vni, zevpn);
}
@@ -4327,9 +4336,9 @@ int zebra_vxlan_svi_down(struct interface *ifp, struct interface *link_if)
} else {
zebra_evpn_t *zevpn = NULL;
- /* since we dont have svi corresponding to zevpn, we associate it
- * to default vrf. Note: the corresponding neigh entries on the
- * SVI would have already been deleted */
+ /* since we dont have svi corresponding to zevpn, we associate
+ * it to default vrf. Note: the corresponding neigh entries on
+ * the SVI would have already been deleted */
zevpn = zebra_evpn_from_svi(ifp, link_if);
if (zevpn) {
zevpn->vrf_id = VRF_DEFAULT;
@@ -4793,10 +4802,10 @@ int zebra_vxlan_if_update(struct interface *ifp, uint16_t chgflags)
zebra_evpn_mac_del_all(zevpn, 0, 1, DEL_LOCAL_MAC);
}
- if (zevpn->local_vtep_ip.s_addr != vxl->vtep_ip.s_addr ||
- zevpn->mcast_grp.s_addr != vxl->mcast_grp.s_addr) {
+ if (zevpn->local_vtep_ip.s_addr != vxl->vtep_ip.s_addr
+ || zevpn->mcast_grp.s_addr != vxl->mcast_grp.s_addr) {
zebra_vxlan_sg_deref(zevpn->local_vtep_ip,
- zevpn->mcast_grp);
+ zevpn->mcast_grp);
zebra_vxlan_sg_ref(vxl->vtep_ip, vxl->mcast_grp);
zevpn->local_vtep_ip = vxl->vtep_ip;
zevpn->mcast_grp = vxl->mcast_grp;
@@ -4909,10 +4918,10 @@ int zebra_vxlan_if_add(struct interface *ifp)
}
}
- if (zevpn->local_vtep_ip.s_addr != vxl->vtep_ip.s_addr ||
- zevpn->mcast_grp.s_addr != vxl->mcast_grp.s_addr) {
+ if (zevpn->local_vtep_ip.s_addr != vxl->vtep_ip.s_addr
+ || zevpn->mcast_grp.s_addr != vxl->mcast_grp.s_addr) {
zebra_vxlan_sg_deref(zevpn->local_vtep_ip,
- zevpn->mcast_grp);
+ zevpn->mcast_grp);
zebra_vxlan_sg_ref(vxl->vtep_ip, vxl->mcast_grp);
zevpn->local_vtep_ip = vxl->vtep_ip;
zevpn->mcast_grp = vxl->mcast_grp;
@@ -5398,7 +5407,7 @@ void zebra_vxlan_advertise_gw_macip(ZAPI_HANDLER_ARGS)
"EVPN gateway macip Adv %s on VNI %d , currently %s",
advertise ? "enabled" : "disabled", vni,
advertise_gw_macip_enabled(zevpn) ? "enabled"
- : "disabled");
+ : "disabled");
if (zevpn->advertise_gw_macip == advertise)
return;
If you are a new contributor to FRR, please see our contributing guidelines.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13536/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
3 Static Analyzer issues remaining.See details at |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
pjdruddy
force-pushed
the
evpn-refactor
branch
from
c20a811 to
a6300ec
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopo tests part 0 on Ubuntu 18.04 arm8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO0U18ARM8-13542/test Topology Tests failed for Topo tests part 0 on Ubuntu 18.04 arm8: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13542/artifact/TOPO0U18ARM8/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopo tests part 0 on Ubuntu 18.04 arm8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO0U18ARM8-13542/test Topology Tests failed for Topo tests part 0 on Ubuntu 18.04 arm8: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13542/artifact/TOPO0U18ARM8/ErrorLog/log_topotests.txt Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
3 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13544/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
3 Static Analyzer issues remaining.See details at |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
add tests to check IP address/MAC address associations are learned from netlink NEWNEIGH messages and are propagated to the remote PE Signed-off-by: Pat Ruddy <pat@voltanet.io>
Since the values of ifindices cannot be relied upon across distributions, simpy remove them from the VNI JSON being compared. Signed-off-by: Pat Ruddy <pat@voltanet.io>
The main zebra_vni_t hash structure has been renamed to zebra_evpn_t to allow for other transport underlays. Rename functions and variables to reflect this change. Signed-off-by: Pat Ruddy <pat@voltanet.io>
clone zebra_vxlan.c to create a file zebra_evpn_mac.c for MAC dB functions whilst retaining the history of zebra_vxlan.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
Move MAC dB specific functions to zebra_evpn_mac.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
Move MAC add code from process_remote_macip_add in zebra_vxlan.c to a generic function process_mac_remote_macip_add in zebra_evpn_mac.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract the local mac add code from zebra_vxlan_local_mac_add_update and create a new generic local mac add function zebra_evpn_add_update_local_mac Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract generic local mac add code from zebra_vxlan_local_mac_del into a new function zebra_evpn_del_local_mac in zebra_evpn_mac.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract mac_gateway add code from zevi_gw_macip_add and move it to a new generic function zebra_evpn_mac_gw_macip_add in zebra_evpn_mac.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
clone zebra_vxlan.c to create a file zebra_evpn_neigh.c for neighbor dB functions whilst retaining the history of zebra_vxlan.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
Move neighbor processing functions to new zebra_evpn_neigh.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract the neighbor part of process_remote_macip_add into a new function process_neigh_remote_macip_add in zebra_evpn_neigh.c. Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract the neighbor part of process_remote_macip_add into a new function zebra_evpn_neigh_gw_macip_add in zebra_evpn_neigh.c. Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract the neighbor uninstall part of process_remote_macip_add into a new function zebra_evpn_neigh_remote_uninstall in zebra_evpn_neigh.c. Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract the neighbor uninstall part of zebra_vxlan_handle_kernel_neigh_del into a new function zebra_evpn_neigh_del_ip in zebra_evpn_neigh.c. Signed-off-by: Pat Ruddy <pat@voltanet.io>
clone zebra_vxlan.c to create a file zebra_evpn.c for core EVPN functions whilst retaining the history of zebra_vxlan.c Signed-off-by: Pat Ruddy <pat@voltanet.io>
extract the core EVPN functions from zebra_vxlan.c and put them in a new file zebra_evpn.c. Signed-off-by: Pat Ruddy <pat@voltanet.io>
Warning logs - Logic error: Dereference of null pointer in zebra_evpn_mh.c, function zebra_evpn_es_evi_show_vni, line 360 See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13544/artifact/shared/static_analysis/report-b1eb72.html#EndPath Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
Reverting probing of neigh entry. There is a timing where probe and remote macip add request comes at the same time resulting in neigh to remain in local state event though it should be remote. In mobility case, the host moves to remote VTEP, first MAC only type-2 route is received which triggers a PROBE of neighs (associated to MAC). PROBE request can go via network port to remote VTEP. PROBE request picks up local neigh with MAC entry's outgoing port is remote VTEP tunnel port. The PROBE reply and MAC-IP (containing IP) almost comes same time at DUT. DUT first processes remote macip and installs neigh as remote. Followed by receives neigh as REACHABLE which marks neigh as LOCAL. FRR does have BPF filter which does not allow its own netlink request to receive. Otherwise frr's request to program neigh as remote can move neigh from local to remote. Though ordering can not be guranteed that REACHABLE (PROBE's repsonse) can come at anytime and move it to LOCAL. This fix would not suffice the needs of converging LOCAL inactive neighs to remove from DB. As mobility draft sugges to PROBE local neigh when MAC moves to remote but it is not working with current framework. Ticket:CM-22864 This reverts commit 44bc8ae Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
pjdruddy
force-pushed
the
evpn-refactor
branch
from
92e759d to
707b76d
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13591/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
2 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13592/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
2 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13595/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
2 Static Analyzer issues remaining.See details at |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
rzalamena
reviewed
Aug 12, 2020
There was a problem hiding this comment.
Changes look good, just a few nits here and there. I also omitted some comments on purpose to avoid polluting the PR with repeated items.
If you know, would you mind writing down what kernel versions are working for you (or that you are using)? Non working kernel versions would also help. I ask you this because of the EVPN neighbor sync quirk comment.
|
|
||
| if (!zevpn->vxlan_if) { // unexpected | ||
| if (json == NULL) | ||
| vty_out(vty, " VxLAN interface: unknown\n"); |
There was a problem hiding this comment.
I know this is unexpected, but I think it would be better to create another JSON object with just an error key otherwise user will face a JSON parsing failure. What do you think?
| if (json == NULL) { | ||
| vty_out(vty, " VxLAN interface: %s\n", zevpn->vxlan_if->name); | ||
| vty_out(vty, " VxLAN ifIndex: %u\n", zevpn->vxlan_if->ifindex); | ||
| vty_out(vty, " Local VTEP IP: %s\n", |
There was a problem hiding this comment.
How about using %pI4 instead of inet_ntoa here?
This repeats again in this file. You could eqvinox's script to do this for you as well: #6855
| for (ALL_LIST_ELEMENTS(ifp->connected, cnode, cnnode, c)) { | ||
| struct ipaddr ip; | ||
|
|
||
| memset(&ip, 0, sizeof(struct ipaddr)); |
There was a problem hiding this comment.
Two possible nits here:
- How about initializing the variable at declaration? There are possible performance benefits for this as well.
- Otherwise how about using
memset(&ip, 0, sizeof(ip))instead ofstruct ipaddr?
Same thing for the repeated occurrences of this in this file.
| stream_putw_at(s, 0, stream_get_endp(s)); | ||
|
|
||
| if (IS_ZEBRA_DEBUG_VXLAN) | ||
| zlog_debug("Send ip prefix %s %s on vrf %s", |
There was a problem hiding this comment.
You could use %pFX instead of prefix2str. It would save you two lines of code.
Take a look here for more info:
http://docs.frrouting.org/projects/dev-guide/en/latest/logging.html?highlight=printfrr#extensions
| continue; | ||
| } | ||
|
|
||
| zebra_evpn_gw_macip_del(ifp, zevpn, &ip); |
There was a problem hiding this comment.
This loop pattern is repeated and you could save a bunch of lines of code by adding a new parameter to it and do something like:
if (add_macip)
zebra_evpn_gw_macip_add(...)
else
zebra_evpn_gw_macip_del(..) Just like it was done in this function: int zebra_evpn_advertise_subnet().
Also if you are not removing items from the list you are iterating, you could use ALL_LIST_ELEMENTS_RO instead of ALL_LIST_ELEMENTS to avoid extra copy operation.
| struct interface *ifp, | ||
| vlanid_t vid) | ||
| { | ||
| struct zebra_if *zif = ifp->info; |
There was a problem hiding this comment.
Static analyser complained about possible ifp NULL access here... If you think it will never be NULL you can add assert to fix it.
| bool old_static; | ||
| bool new_static; | ||
|
|
||
| memcpy(&n->emac, macaddr, ETH_ALEN); |
There was a problem hiding this comment.
Static analyser is complaining about possible macaddr NULL here. If you think it will never be NULL you could add an assert to fix it.
Also I think you should move this block to after if (!mac) return otherwise it is wasted work and prone to static analyser warnings.
|
|
||
| set_router = !!CHECK_FLAG(n->flags, ZEBRA_NEIGH_ROUTER_FLAG); | ||
|
|
||
| /* XXX - this will change post integration with the new kernel */ |
There was a problem hiding this comment.
Maybe we should add a deprecation warning? Or check the kernel version somehow?
I fear this might break in the future without notice... or am I making it bigger than it is?
|
|
||
| typedef struct zebra_neigh_t_ zebra_neigh_t; | ||
|
|
||
| #define IS_ZEBRA_NEIGH_ACTIVE(n) (n->state == ZEBRA_NEIGH_ACTIVE) |
There was a problem hiding this comment.
I would recommend to write this as:
#define IS_FOO(n) ((n)->state == FOO)Otherwise we might have problems with more complicated macro calls (e.g. using casts, ** or &).
|
|
||
| #define IS_ZEBRA_NEIGH_INACTIVE(n) (n->state == ZEBRA_NEIGH_INACTIVE) | ||
|
|
||
| #define ZEBRA_NEIGH_SET_ACTIVE(n) n->state = ZEBRA_NEIGH_ACTIVE |
There was a problem hiding this comment.
Suggestion: If you don't expect to get the "return" value of this macro, then a do {} while (0), inline function or even expanding the macro could be better.
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13597/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
2 Static Analyzer issues remaining.See details at |
Use asserts rather thank test where the values should definitely not be NULL. Signed-off-by: Pat Ruddy <pat@voltanet.io>
pjdruddy
force-pushed
the
evpn-refactor
branch
from
39c49ec to
2bdd446
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13602/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
rzalamena
approved these changes
Aug 14, 2020
|
I didn't get time to test the whole PR, but the code review is fine and the nits can be fixed later (some of those will probably be auto fixed by Equinox's new coccinelle script). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes simply restructure the EVPN files as a precursor to implementing EVPN-MPLS based on the existing EVPN-VXLAN codebase. Thus common functions have been moved into core shared files:
zebra_evpn.c
zebra_evpn_mac.c
zebra_evpn_neigh.c
When creating these files (from the zebra_vxlan.c behemoth) the history has been conserved using git magic.
and the naming of the evpn instances which was formely based in VXLAN VNI has been shecked to the more generic zevpn.
There are no functional changes in this diffset.
The existing topotests have been expanded to cover neighbor learning.