zebra: giant zapi cleanup

* Standardize arguments for ZAPI message handlers

A lot of the handler functions that are called directly from the ZAPI
input processing code take different argument sets where they don't need
to. These functions are called from only one place and all have the same
fundamental information available to them to do their work. There is no
need to specialize what information is passed to them; it is cleaner and
easier to understand when they all accept the same base set of
information and extract what they need inline.

* Eliminate the humongous switch-case in the dispatcher

After doing the above work it becomes possible to eliminate the big
switch-case statement and turn it into a lookup table keyed by ZAPI
message code. Granted the compiler usually optimized switch-case
statements into jump tables anyway but it is better to make this a
strong guarantee by doing it in the source.

* Allow batching ZAPI I/O

ZAPI's original I/O loop looks like:

zserv_read:
- Read one message
- Run message handler
- Schedule zserv_read

This has been changed to:

zserv_handle_messages:
- For each message on the input queue:
  - Pop the message
  - Run message handler

zserv_read:
- Read as many messages as available into input queue
- Schedule zserv_handle_messages
- Schedule zserv_read

This is more flexible as it allows decoupling I/O from actual state
changes inside Zebra. Theoretically it also helps cache churn by
grouping the same tasks together instead of alternating between them.

* Modify all handlers to stop accessing global client buffers

There is no reason for message handlers to look at a global data
structure that holds the last message received. Instead we can reduce
the amount of global state they need to know about by making them accept
a message stream, and not care about where it came from. This is also
necessary for the input queue changes above. It also moves us closer to
being able to unit test ZAPI message handlers.

* Modify all handlers to not return an status code

All of the ZAPI message handlers return an integer that means different
things to each of them, but nobody ever reads these integers, so this is
technical debt that we can just eliminate outright.

* Rename some functions to zread_* or zsend_* to clarify action

zserv.c functions are roughly organized into three kinds of functions:
- Functions that encode Zebra datastructures into ZAPI message
  equivalents
- Functions that build and send whole messages
- Functions that process received ZAPI messages

These are variously named with prefixes like "zserv", "zebra", "zread",
"zsend", etc. Organize them by using consistent naming prefixes;
zserv_encode_*, zsend_*, zread_* respectively for the three types given
above.

* Organize zserv.c into logical sections

Group the above functions together by type, and group the plumbing for
the I/O code together as well.

* Add struct zmsghdr

Formalize the ZAPI header by documenting it in code and providing it to
message handlers free of charge to reduce complexity.

* Clean up zserv_read

Just some simplifications of the existing code intended to make it
easier to read, understand and debug. Also improved debugging output for
ZAPI messages by leveraging zmsghdr and making log messages consistent.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

zebra/label_manager.c

@@ -50,6 +50,7 @@ DEFINE_MTYPE_STATIC(LBL_MGR, LM_CHUNK, "Label Manager Chunk");
  * it will be a proxy to relay messages to external label manager
  * This zclient thus is to connect to it
  */
+static struct stream *ibuf;
 static struct zclient *zclient;
 bool lm_is_external;
 
@@ -61,15 +62,14 @@ static void delete_label_chunk(void *val)
 static int relay_response_back(struct zserv *zserv)
 {
 	int ret = 0;
-	struct stream *src, *dst;
+	struct stream *src;
 	u_int16_t size = 0;
 	u_char marker;
 	u_char version;
 	vrf_id_t vrf_id;
 	u_int16_t resp_cmd;
 
 	src = zclient->ibuf;
-	dst = zserv->obuf;
 
 	stream_reset(src);
 
@@ -86,8 +86,8 @@ static int relay_response_back(struct zserv *zserv)
 		return -1;
 
 	/* send response back */
-	stream_copy(dst, src);
-	ret = writen(zserv->sock, dst->data, stream_get_endp(dst));
+	stream_copy(ibuf, src);
+	ret = writen(zserv->sock, src->data, stream_get_endp(src));
 	if (ret <= 0) {
 		zlog_err("%s: Error sending Label Manager response back: %s",
 			 __func__, strerror(errno));
@@ -116,9 +116,10 @@ static int lm_zclient_read(struct thread *t)
 
 static int reply_error(int cmd, struct zserv *zserv, vrf_id_t vrf_id)
 {
+	int ret;
 	struct stream *s;
 
-	s = zserv->obuf;
+	s = stream_new(ZEBRA_MAX_PACKET_SIZ);
 	stream_reset(s);
 
 	zclient_create_header(s, cmd, vrf_id);
@@ -129,7 +130,10 @@ static int reply_error(int cmd, struct zserv *zserv, vrf_id_t vrf_id)
 	/* Write packet size. */
 	stream_putw_at(s, 0, stream_get_endp(s));
 
-	return writen(zserv->sock, s->data, stream_get_endp(s));
+	ret = writen(zserv->sock, s->data, stream_get_endp(s));
+
+	stream_free(s);
+	return ret;
 }
 /**
  * Receive a request to get or release a label chunk and forward it to external
@@ -161,7 +165,7 @@ int zread_relay_label_manager_request(int cmd, struct zserv *zserv,
 		ret = relay_response_back(zserv);
 
 	/* Send request to external label manager */
-	src = zserv->ibuf;
+	src = ibuf;
 	dst = zclient->obuf;
 
 	stream_copy(dst, src);
@@ -247,6 +251,8 @@ void label_manager_init(char *lm_zserv_path)
 		lm_is_external = true;
 		lm_zclient_init(lm_zserv_path);
 	}
+
+	ibuf = stream_new(ZEBRA_MAX_PACKET_SIZ);
 }
 
 /**

zebra/redistribute.c

@@ -243,16 +243,15 @@ void redistribute_delete(struct prefix *p, struct prefix *src_p,
 	}
 }
 
-void zebra_redistribute_add(int command, struct zserv *client, int length,
-			    struct zebra_vrf *zvrf)
+void zebra_redistribute_add(ZAPI_HANDLER_ARGS)
 {
 	afi_t afi = 0;
 	int type = 0;
 	u_short instance;
 
-	STREAM_GETC(client->ibuf, afi);
-	STREAM_GETC(client->ibuf, type);
-	STREAM_GETW(client->ibuf, instance);
+	STREAM_GETC(msg, afi);
+	STREAM_GETC(msg, type);
+	STREAM_GETW(msg, instance);
 
 	if (afi == 0 || afi > AFI_MAX) {
 		zlog_warn("%s: Specified afi %d does not exist",
@@ -287,16 +286,15 @@ void zebra_redistribute_add(int command, struct zserv *client, int length,
 	return;
 }
 
-void zebra_redistribute_delete(int command, struct zserv *client, int length,
-			       struct zebra_vrf *zvrf)
+void zebra_redistribute_delete(ZAPI_HANDLER_ARGS)
 {
 	afi_t afi = 0;
 	int type = 0;
 	u_short instance;
 
-	STREAM_GETC(client->ibuf, afi);
-	STREAM_GETC(client->ibuf, type);
-	STREAM_GETW(client->ibuf, instance);
+	STREAM_GETC(msg, afi);
+	STREAM_GETC(msg, type);
+	STREAM_GETW(msg, instance);
 
 	if (afi == 0 || afi > AFI_MAX) {
 		zlog_warn("%s: Specified afi %d does not exist",
@@ -325,15 +323,13 @@ void zebra_redistribute_delete(int command, struct zserv *client, int length,
 	return;
 }
 
-void zebra_redistribute_default_add(int command, struct zserv *client,
-				    int length, struct zebra_vrf *zvrf)
+void zebra_redistribute_default_add(ZAPI_HANDLER_ARGS)
 {
 	vrf_bitmap_set(client->redist_default, zvrf_id(zvrf));
 	zebra_redistribute_default(client, zvrf_id(zvrf));
 }
 
-void zebra_redistribute_default_delete(int command, struct zserv *client,
-				       int length, struct zebra_vrf *zvrf)
+void zebra_redistribute_default_delete(ZAPI_HANDLER_ARGS)
 {
 	vrf_bitmap_unset(client->redist_default, zvrf_id(zvrf));
 }

zebra/redistribute.h

@@ -27,15 +27,12 @@
 #include "vty.h"
 #include "vrf.h"
 
-extern void zebra_redistribute_add(int, struct zserv *, int,
-				   struct zebra_vrf *zvrf);
-extern void zebra_redistribute_delete(int, struct zserv *, int,
-				      struct zebra_vrf *zvrf);
-
-extern void zebra_redistribute_default_add(int, struct zserv *, int,
-					   struct zebra_vrf *zvrf);
-extern void zebra_redistribute_default_delete(int, struct zserv *, int,
-					      struct zebra_vrf *zvrf);
+/* ZAPI command handlers */
+extern void zebra_redistribute_add(ZAPI_HANDLER_ARGS);
+extern void zebra_redistribute_delete(ZAPI_HANDLER_ARGS);
+extern void zebra_redistribute_default_add(ZAPI_HANDLER_ARGS);
+extern void zebra_redistribute_default_delete(ZAPI_HANDLER_ARGS);
+/* ----------------- */
 
 extern void redistribute_update(struct prefix *, struct prefix *,
 				struct route_entry *, struct route_entry *);

zebra/rtadv.c

@@ -801,16 +801,15 @@ static void ipv6_nd_suppress_ra_set(struct interface *ifp,
  * if the operator has explicitly enabled RA. The enable request can also
  * specify a RA interval (in seconds).
  */
-void zebra_interface_radv_set(struct zserv *client, u_short length,
-			      struct zebra_vrf *zvrf, int enable)
+static void zebra_interface_radv_set(ZAPI_HANDLER_ARGS, int enable)
 {
 	struct stream *s;
 	ifindex_t ifindex;
 	struct interface *ifp;
 	struct zebra_if *zif;
 	int ra_interval;
 
-	s = client->ibuf;
+	s = msg;
 
 	/* Get interface index and RA interval. */
 	STREAM_GETL(s, ifindex);
@@ -859,6 +858,15 @@ void zebra_interface_radv_set(struct zserv *client, u_short length,
 	return;
 }
 
+void zebra_interface_radv_disable(ZAPI_HANDLER_ARGS)
+{
+	zebra_interface_radv_set(client, hdr, msg, zvrf, 0);
+}
+void zebra_interface_radv_enable(ZAPI_HANDLER_ARGS)
+{
+	zebra_interface_radv_set(client, hdr, msg, zvrf, 1);
+}
+
 DEFUN (ipv6_nd_suppress_ra,
        ipv6_nd_suppress_ra_cmd,
        "ipv6 nd suppress-ra",

zebra/rtadv.h

@@ -103,8 +103,8 @@ typedef enum {
 extern void rtadv_init(struct zebra_ns *);
 extern void rtadv_terminate(struct zebra_ns *);
 extern void rtadv_cmd_init(void);
-extern void zebra_interface_radv_set(struct zserv *client,
-				     u_short length, struct zebra_vrf *zvrf,
-				     int enable);
+extern void zebra_interface_radv_disable(ZAPI_HANDLER_ARGS);
+extern void zebra_interface_radv_enable(ZAPI_HANDLER_ARGS);
+
 
 #endif /* _ZEBRA_RTADV_H */

zebra/zebra_mpls.c

@@ -455,16 +455,15 @@ static int fec_send(zebra_fec_t *fec, struct zserv *client)
 	rn = fec->rn;
 
 	/* Get output stream. */
-	s = client->obuf;
-	stream_reset(s);
+	s = stream_new(ZEBRA_MAX_PACKET_SIZ);
 
 	zclient_create_header(s, ZEBRA_FEC_UPDATE, VRF_DEFAULT);
 
 	stream_putw(s, rn->p.family);
 	stream_put_prefix(s, &rn->p);
 	stream_putl(s, fec->label);
 	stream_putw_at(s, 0, stream_get_endp(s));
-	return zebra_server_send_message(client);
+	return zebra_server_send_message(client, s);
 }
 
 /*

zebra/zebra_mroute.c

@@ -32,17 +32,16 @@
 #include "zebra/rt.h"
 #include "zebra/debug.h"
 
-int zebra_ipmr_route_stats(struct zserv *client, u_short length,
-			   struct zebra_vrf *zvrf)
+void zebra_ipmr_route_stats(ZAPI_HANDLER_ARGS)
 {
 	struct mcast_route_data mroute;
 	struct stream *s;
 	int suc = -1;
 
 	memset(&mroute, 0, sizeof(mroute));
-	STREAM_GET(&mroute.sg.src, client->ibuf, 4);
-	STREAM_GET(&mroute.sg.grp, client->ibuf, 4);
-	STREAM_GETL(client->ibuf, mroute.ifindex);
+	STREAM_GET(&mroute.sg.src, msg, 4);
+	STREAM_GET(&mroute.sg.grp, msg, 4);
+	STREAM_GETL(msg, mroute.ifindex);
 
 	if (IS_ZEBRA_DEBUG_KERNEL) {
 		char sbuf[40];
@@ -57,7 +56,7 @@ int zebra_ipmr_route_stats(struct zserv *client, u_short length,
 	suc = kernel_get_ipmr_sg_stats(zvrf, &mroute);
 
 stream_failure:
-	s = client->obuf;
+	s = stream_new(ZEBRA_MAX_PACKET_SIZ);
 
 	stream_reset(s);
 
@@ -68,6 +67,5 @@ int zebra_ipmr_route_stats(struct zserv *client, u_short length,
 	stream_putl(s, suc);
 
 	stream_putw_at(s, 0, stream_get_endp(s));
-	zebra_server_send_message(client);
-	return 0;
+	zebra_server_send_message(client, s);
 }

zebra/zebra_mroute.h

@@ -22,13 +22,14 @@
 #ifndef __ZEBRA_MROUTE_H__
 #define __ZEBRA_MROUTE_H__
 
+#include "zebra/zserv.h"
+
 struct mcast_route_data {
 	struct prefix_sg sg;
 	unsigned int ifindex;
 	unsigned long long lastused;
 };
 
-int zebra_ipmr_route_stats(struct zserv *client, u_short length,
-			   struct zebra_vrf *zvf);
+void zebra_ipmr_route_stats(ZAPI_HANDLER_ARGS);
 
 #endif