zebra,pim : Fixing Review comments in PIM_MLAG

Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
This commit is contained in:
Satheesh Kumar K 2019-11-18 07:13:30 -08:00
parent 67fa73f29a
commit 1e76492b10
6 changed files with 71 additions and 84 deletions

View File

@ -40,8 +40,7 @@ char *mlag_role2str(enum mlag_role role, char *buf, size_t size)
return buf; return buf;
} }
char *zebra_mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf, char *mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf, size_t size)
size_t size)
{ {
switch (msg_type) { switch (msg_type) {
case MLAG_REGISTER: case MLAG_REGISTER:
@ -82,7 +81,7 @@ char *zebra_mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf,
} }
int zebra_mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg) int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL)
return -1; return -1;
@ -95,8 +94,7 @@ stream_failure:
return -1; return -1;
} }
int zebra_mlag_lib_decode_mroute_add(struct stream *s, int mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg)
struct mlag_mroute_add *msg)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL)
return -1; return -1;
@ -115,8 +113,7 @@ stream_failure:
return -1; return -1;
} }
int zebra_mlag_lib_decode_mroute_del(struct stream *s, int mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg)
struct mlag_mroute_del *msg)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL)
return -1; return -1;
@ -132,7 +129,7 @@ stream_failure:
return -1; return -1;
} }
int zebra_mlag_lib_decode_mlag_status(struct stream *s, struct mlag_status *msg) int mlag_lib_decode_mlag_status(struct stream *s, struct mlag_status *msg)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL)
return -1; return -1;
@ -145,7 +142,7 @@ stream_failure:
return -1; return -1;
} }
int zebra_mlag_lib_decode_vxlan_update(struct stream *s, struct mlag_vxlan *msg) int mlag_lib_decode_vxlan_update(struct stream *s, struct mlag_vxlan *msg)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL)
return -1; return -1;
@ -158,8 +155,7 @@ stream_failure:
return -1; return -1;
} }
int zebra_mlag_lib_decode_frr_status(struct stream *s, int mlag_lib_decode_frr_status(struct stream *s, struct mlag_frr_status *msg)
struct mlag_frr_status *msg)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL)
return -1; return -1;

View File

@ -119,24 +119,23 @@ struct mlag_msg {
uint16_t data_len; uint16_t data_len;
uint16_t msg_cnt; uint16_t msg_cnt;
uint8_t data[0]; uint8_t data[0];
}__attribute__((packed)); } __attribute__((packed));
extern char *mlag_role2str(enum mlag_role role, char *buf, size_t size); extern char *mlag_role2str(enum mlag_role role, char *buf, size_t size);
extern char *zebra_mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf, extern char *mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf,
size_t size); size_t size);
extern int zebra_mlag_lib_decode_mlag_hdr(struct stream *s, extern int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg);
struct mlag_msg *msg); extern int mlag_lib_decode_mroute_add(struct stream *s,
extern int zebra_mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg);
struct mlag_mroute_add *msg); extern int mlag_lib_decode_mroute_del(struct stream *s,
extern int zebra_mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg);
struct mlag_mroute_del *msg); extern int mlag_lib_decode_mlag_status(struct stream *s,
extern int zebra_mlag_lib_decode_mlag_status(struct stream *s, struct mlag_status *msg);
struct mlag_status *msg); extern int mlag_lib_decode_vxlan_update(struct stream *s,
extern int zebra_mlag_lib_decode_vxlan_update(struct stream *s, struct mlag_vxlan *msg);
struct mlag_vxlan *msg); extern int mlag_lib_decode_frr_status(struct stream *s,
extern int zebra_mlag_lib_decode_frr_status(struct stream *s, struct mlag_frr_status *msg);
struct mlag_frr_status *msg);
#ifdef __cplusplus #ifdef __cplusplus
} }
#endif #endif

View File

@ -87,22 +87,22 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
char buf[ZLOG_FILTER_LENGTH_MAX]; char buf[ZLOG_FILTER_LENGTH_MAX];
int rc = 0; int rc = 0;
rc = zebra_mlag_lib_decode_mlag_hdr(s, &mlag_msg); rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg);
if (rc) if (rc)
return (rc); return (rc);
if (PIM_DEBUG_MLAG) if (PIM_DEBUG_MLAG)
zlog_debug("%s: Received msg type: %s length: %d, bulk_cnt: %d", zlog_debug("%s: Received msg type: %s length: %d, bulk_cnt: %d",
__func__, __func__,
zebra_mlag_lib_msgid_to_str(mlag_msg.msg_type, buf, mlag_lib_msgid_to_str(mlag_msg.msg_type, buf,
sizeof(buf)), sizeof(buf)),
mlag_msg.data_len, mlag_msg.msg_cnt); mlag_msg.data_len, mlag_msg.msg_cnt);
switch (mlag_msg.msg_type) { switch (mlag_msg.msg_type) {
case MLAG_STATUS_UPDATE: { case MLAG_STATUS_UPDATE: {
struct mlag_status msg; struct mlag_status msg;
rc = zebra_mlag_lib_decode_mlag_status(s, &msg); rc = mlag_lib_decode_mlag_status(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mlagd_state_change(msg); pim_mlag_process_mlagd_state_change(msg);
@ -110,7 +110,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
case MLAG_PEER_FRR_STATUS: { case MLAG_PEER_FRR_STATUS: {
struct mlag_frr_status msg; struct mlag_frr_status msg;
rc = zebra_mlag_lib_decode_frr_status(s, &msg); rc = mlag_lib_decode_frr_status(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_peer_frr_state_change(msg); pim_mlag_process_peer_frr_state_change(msg);
@ -118,7 +118,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
case MLAG_VXLAN_UPDATE: { case MLAG_VXLAN_UPDATE: {
struct mlag_vxlan msg; struct mlag_vxlan msg;
rc = zebra_mlag_lib_decode_vxlan_update(s, &msg); rc = mlag_lib_decode_vxlan_update(s, &msg);
if (rc) if (rc)
return rc; return rc;
pim_mlag_process_vxlan_update(&msg); pim_mlag_process_vxlan_update(&msg);
@ -126,7 +126,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
case MLAG_MROUTE_ADD: { case MLAG_MROUTE_ADD: {
struct mlag_mroute_add msg; struct mlag_mroute_add msg;
rc = zebra_mlag_lib_decode_mroute_add(s, &msg); rc = mlag_lib_decode_mroute_add(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_add(msg); pim_mlag_process_mroute_add(msg);
@ -134,7 +134,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
case MLAG_MROUTE_DEL: { case MLAG_MROUTE_DEL: {
struct mlag_mroute_del msg; struct mlag_mroute_del msg;
rc = zebra_mlag_lib_decode_mroute_del(s, &msg); rc = mlag_lib_decode_mroute_del(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_del(msg); pim_mlag_process_mroute_del(msg);
@ -145,7 +145,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0; i < mlag_msg.msg_cnt; i++) {
rc = zebra_mlag_lib_decode_mroute_add(s, &msg); rc = mlag_lib_decode_mroute_add(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_add(msg); pim_mlag_process_mroute_add(msg);
@ -157,7 +157,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0; i < mlag_msg.msg_cnt; i++) {
rc = zebra_mlag_lib_decode_mroute_del(s, &msg); rc = mlag_lib_decode_mroute_del(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_del(msg); pim_mlag_process_mroute_del(msg);

View File

@ -153,7 +153,7 @@ static int zebra_mlag_client_msg_handler(struct thread *event)
wr_count = stream_fifo_count_safe(zrouter.mlag_info.mlag_fifo); wr_count = stream_fifo_count_safe(zrouter.mlag_info.mlag_fifo);
if (IS_ZEBRA_DEBUG_MLAG) if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug(":%s: Processing MLAG write, %d messages in queue", zlog_debug(":%s: Processing MLAG write, %u messages in queue",
__func__, wr_count); __func__, wr_count);
max_count = MIN(wr_count, ZEBRA_MLAG_POST_LIMIT); max_count = MIN(wr_count, ZEBRA_MLAG_POST_LIMIT);
@ -240,16 +240,17 @@ void zebra_mlag_handle_process_state(enum zebra_mlag_state state)
*/ */
static int zebra_mlag_signal_write_thread(void) static int zebra_mlag_signal_write_thread(void)
{ {
frr_with_mutex (&zrouter.mlag_info.mlag_th_mtx) { if (IS_ZEBRA_DEBUG_MLAG)
if (zrouter.mlag_info.zebra_pth_mlag) { zlog_debug(":%s: Scheduling MLAG write", __func__);
if (IS_ZEBRA_DEBUG_MLAG) /*
zlog_debug(":%s: Scheduling MLAG write", * This api will be called from Both main & MLAG Threads.
__func__); * main thread writes, "zrouter.mlag_info.th_master" only
thread_add_event(zrouter.mlag_info.th_master, * during Zebra Init/after MLAG thread is destroyed.
zebra_mlag_client_msg_handler, NULL, 0, * so it is safe to use without any locking
&zrouter.mlag_info.t_write); */
} thread_add_event(zrouter.mlag_info.th_master,
} zebra_mlag_client_msg_handler, NULL, 0,
&zrouter.mlag_info.t_write);
return 0; return 0;
} }
@ -593,13 +594,14 @@ DEFUN_HIDDEN (show_mlag,
return CMD_SUCCESS; return CMD_SUCCESS;
} }
DEFPY(test_mlag, test_mlag_cmd, DEFPY_HIDDEN(test_mlag, test_mlag_cmd,
"test zebra mlag <none$none|primary$primary|secondary$secondary>", "test zebra mlag <none$none|primary$primary|secondary$secondary>",
"Test code\n" ZEBRA_STR "Test code\n"
"Modify the Mlag state\n" ZEBRA_STR
"Mlag is not setup on the machine\n" "Modify the Mlag state\n"
"Mlag is setup to be primary\n" "Mlag is not setup on the machine\n"
"Mlag is setup to be the secondary\n") "Mlag is setup to be primary\n"
"Mlag is setup to be the secondary\n")
{ {
enum mlag_role orig = zrouter.mlag_info.role; enum mlag_role orig = zrouter.mlag_info.role;
char buf1[MLAG_ROLE_STRSIZE], buf2[MLAG_ROLE_STRSIZE]; char buf1[MLAG_ROLE_STRSIZE], buf2[MLAG_ROLE_STRSIZE];
@ -621,12 +623,8 @@ DEFPY(test_mlag, test_mlag_cmd,
if (zrouter.mlag_info.role != MLAG_ROLE_NONE) { if (zrouter.mlag_info.role != MLAG_ROLE_NONE) {
if (zrouter.mlag_info.clients_interested_cnt == 0 if (zrouter.mlag_info.clients_interested_cnt == 0
&& test_mlag_in_progress == false) { && test_mlag_in_progress == false) {
frr_with_mutex ( if (zrouter.mlag_info.zebra_pth_mlag == NULL)
&zrouter.mlag_info.mlag_th_mtx) { zebra_mlag_spawn_pthread();
if (zrouter.mlag_info.zebra_pth_mlag
== NULL)
zebra_mlag_spawn_pthread();
}
zrouter.mlag_info.clients_interested_cnt++; zrouter.mlag_info.clients_interested_cnt++;
test_mlag_in_progress = true; test_mlag_in_progress = true;
zebra_mlag_private_open_channel(); zebra_mlag_private_open_channel();
@ -649,8 +647,8 @@ void zebra_mlag_init(void)
install_element(ENABLE_NODE, &test_mlag_cmd); install_element(ENABLE_NODE, &test_mlag_cmd);
/* /*
* Intialiaze teh MLAG Global variableis * Intialiaze the MLAG Global variables
* write thread will be craeted during actual registration with MCLAG * write thread will be created during actual registration with MCLAG
*/ */
zrouter.mlag_info.clients_interested_cnt = 0; zrouter.mlag_info.clients_interested_cnt = 0;
zrouter.mlag_info.connected = false; zrouter.mlag_info.connected = false;
@ -662,7 +660,6 @@ void zebra_mlag_init(void)
zrouter.mlag_info.t_write = NULL; zrouter.mlag_info.t_write = NULL;
test_mlag_in_progress = false; test_mlag_in_progress = false;
zebra_mlag_reset_read_buffer(); zebra_mlag_reset_read_buffer();
pthread_mutex_init(&zrouter.mlag_info.mlag_th_mtx, NULL);
} }
void zebra_mlag_terminate(void) void zebra_mlag_terminate(void)
@ -692,18 +689,16 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
if (IS_ZEBRA_DEBUG_MLAG) if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug("%s: Entering..", __func__); zlog_debug("%s: Entering..", __func__);
rc = zebra_mlag_lib_decode_mlag_hdr(s, &mlag_msg); rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg);
if (rc) if (rc)
return rc; return rc;
if (IS_ZEBRA_DEBUG_MLAG) if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug("%s: Decoded msg length:%d..", __func__, zlog_debug("%s: Mlag ProtoBuf encoding of message:%s, len:%d",
__func__,
mlag_lib_msgid_to_str(mlag_msg.msg_type, buf,
sizeof(buf)),
mlag_msg.data_len); mlag_msg.data_len);
if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug("%s: Mlag ProtoBuf encoding of message:%s", __func__,
zebra_mlag_lib_msgid_to_str(mlag_msg.msg_type, buf,
sizeof(buf)));
*msg_type = mlag_msg.msg_type; *msg_type = mlag_msg.msg_type;
switch (mlag_msg.msg_type) { switch (mlag_msg.msg_type) {
case MLAG_MROUTE_ADD: { case MLAG_MROUTE_ADD: {
@ -711,7 +706,7 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
ZebraMlagMrouteAdd pay_load = ZEBRA_MLAG_MROUTE_ADD__INIT; ZebraMlagMrouteAdd pay_load = ZEBRA_MLAG_MROUTE_ADD__INIT;
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = zebra_mlag_lib_decode_mroute_add(s, &msg); rc = mlag_lib_decode_mroute_add(s, &msg);
if (rc) if (rc)
return rc; return rc;
vrf_name_len = strlen(msg.vrf_name) + 1; vrf_name_len = strlen(msg.vrf_name) + 1;
@ -743,7 +738,7 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
ZebraMlagMrouteDel pay_load = ZEBRA_MLAG_MROUTE_DEL__INIT; ZebraMlagMrouteDel pay_load = ZEBRA_MLAG_MROUTE_DEL__INIT;
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = zebra_mlag_lib_decode_mroute_del(s, &msg); rc = mlag_lib_decode_mroute_del(s, &msg);
if (rc) if (rc)
return rc; return rc;
vrf_name_len = strlen(msg.vrf_name) + 1; vrf_name_len = strlen(msg.vrf_name) + 1;
@ -783,7 +778,7 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = zebra_mlag_lib_decode_mroute_add(s, &msg); rc = mlag_lib_decode_mroute_add(s, &msg);
if (rc) { if (rc) {
cleanup = true; cleanup = true;
break; break;
@ -848,7 +843,7 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = zebra_mlag_lib_decode_mroute_del(s, &msg); rc = mlag_lib_decode_mroute_del(s, &msg);
if (rc) { if (rc) {
cleanup = true; cleanup = true;
break; break;
@ -903,8 +898,8 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
if (IS_ZEBRA_DEBUG_MLAG) if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug("%s: length of Mlag ProtoBuf encoded message:%s, %d", zlog_debug("%s: length of Mlag ProtoBuf encoded message:%s, %d",
__func__, __func__,
zebra_mlag_lib_msgid_to_str(mlag_msg.msg_type, buf, mlag_lib_msgid_to_str(mlag_msg.msg_type, buf,
sizeof(buf)), sizeof(buf)),
len); len);
hdr.type = (ZebraMlagHeader__MessageType)mlag_msg.msg_type; hdr.type = (ZebraMlagHeader__MessageType)mlag_msg.msg_type;
if (len != 0) { if (len != 0) {
@ -934,8 +929,8 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
zlog_debug( zlog_debug(
"%s: length of Mlag ProtoBuf message:%s with Header %d", "%s: length of Mlag ProtoBuf message:%s with Header %d",
__func__, __func__,
zebra_mlag_lib_msgid_to_str(mlag_msg.msg_type, buf, mlag_lib_msgid_to_str(mlag_msg.msg_type, buf,
sizeof(buf)), sizeof(buf)),
len); len);
if (hdr.data.data) if (hdr.data.data)
XFREE(MTYPE_MLAG_PBUF, hdr.data.data); XFREE(MTYPE_MLAG_PBUF, hdr.data.data);
@ -963,7 +958,7 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
if (IS_ZEBRA_DEBUG_MLAG) if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug("%s: Mlag ProtoBuf decoding of message:%s", __func__, zlog_debug("%s: Mlag ProtoBuf decoding of message:%s", __func__,
zebra_mlag_lib_msgid_to_str(msg_type, buf, 80)); mlag_lib_msgid_to_str(msg_type, buf, 80));
/* /*
* Internal MLAG Message-types & MLAG.proto message types should * Internal MLAG Message-types & MLAG.proto message types should

View File

@ -71,10 +71,8 @@ int zebra_mlag_private_write_data(uint8_t *data, uint32_t len)
static void zebra_mlag_sched_read(void) static void zebra_mlag_sched_read(void)
{ {
frr_with_mutex (&zrouter.mlag_info.mlag_th_mtx) { thread_add_read(zmlag_master, zebra_mlag_read, NULL, mlag_socket,
thread_add_read(zmlag_master, zebra_mlag_read, NULL, &zrouter.mlag_info.t_read);
mlag_socket, &zrouter.mlag_info.t_read);
}
} }
static int zebra_mlag_read(struct thread *thread) static int zebra_mlag_read(struct thread *thread)

View File

@ -100,7 +100,6 @@ struct zebra_mlag_info {
/* Threads for read/write. */ /* Threads for read/write. */
struct thread *t_read; struct thread *t_read;
struct thread *t_write; struct thread *t_write;
pthread_mutex_t mlag_th_mtx;
}; };
struct zebra_router { struct zebra_router {