Skip to content

Commit

Permalink
Merge pull request #1 from cscarpitta/fpm-protobuf
Browse files Browse the repository at this point in the history 
Address reviewers comments on the "zebra: Adding a new dataplane using protobuf" PR
  • Loading branch information
@BIoodborne
BIoodborne authored Feb 13, 2024
2 parents 4fae2bf + 2b64fc5 commit 513be69
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 112 deletions.
2 changes: 2 additions & 0 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,8 @@ AC_ARG_ENABLE([pcre2posix],
AS_HELP_STRING([--enable-pcre2posix], [enable using PCRE2 Posix libs for regex functions]))
AC_ARG_ENABLE([fpm],
AS_HELP_STRING([--enable-fpm], [enable Forwarding Plane Manager support]))
AC_ARG_ENABLE([enable_dplane_pb],
AS_HELP_STRING([--enable-dplane-pb], [enable Forwarding Plane Manager (FPM) protobuf support]))
AC_ARG_ENABLE([werror],
AS_HELP_STRING([--enable-werror], [enable -Werror (recommended for developers only)]))
AC_ARG_ENABLE([cumulus],
Expand Down
2 changes: 1 addition & 1 deletion doc/user/zebra.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,7 @@ FPM Commands


``dplane_fpm_nl`` or ``dplane_fpm_pb`` implementation
--------------------------------
-----------------------------------------------------

.. clicmd:: fpm address <A.B.C.D|X:X::X:X> [port (1-65535)]

Expand Down
39 changes: 18 additions & 21 deletions dplaneserver/dplaneserver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
#include "dplaneserver.h"
#include "fpm/fpm.pb-c.h"
#include "qpb/qpb.h"

#define FPM_HEADER_SIZE 4
DEFINE_MGROUP(DPLANESERVER, "dplaneserver");
Expand All @@ -19,7 +20,7 @@ struct Dplaneserver_data dplaneserver_data = { .bufSize = 2048,
.server_up = false };
enum fpm_msg_op fm_op;

void process_fpm_msg(struct fpm_msg_hdr_t *fpm_hdr)
void process_fpm_msg(fpm_msg_hdr_t *fpm_hdr)
{
size_t msg_len = fpm_msg_len(fpm_hdr);
Fpm__Message *msg;
Expand Down Expand Up @@ -104,7 +105,7 @@ void dplaneserver_exit(void)

int dplaneserver_read_data(void)
{
struct fpm_msg_hdr_t *fpm_hdr;
fpm_msg_hdr_t *fpm_hdr;
size_t msg_len;
size_t start = 0, left;
ssize_t read_len;
Expand All @@ -124,7 +125,7 @@ int dplaneserver_read_data(void)
}
dplaneserver_data.pos += (uint32_t)read_len;
while (true) {
fpm_hdr = (struct fpm_msg_hdr_t *)(dplaneserver_data.messageBuffer +
fpm_hdr = (fpm_msg_hdr_t *)(dplaneserver_data.messageBuffer +
start);
left = dplaneserver_data.pos - start;
if (left < FPM_MSG_HDR_LEN)
Expand All @@ -134,7 +135,7 @@ int dplaneserver_read_data(void)
if (left < msg_len)
break;
if (!fpm_msg_ok(fpm_hdr, left)) {
zlog_err("%s: fpm message header check failed");
zlog_err("%s: fpm message header check failed", __func__);
return -1;
}
process_fpm_msg(fpm_hdr);
Expand Down Expand Up @@ -219,7 +220,7 @@ int dplaneserver_poll(void)

void process_route_install_msg(Fpm__AddRoute *msg)
{
char buf[4096] = { 0 };
struct prefix prefix;

if (!msg->key) {
zlog_err("%s: ROUTE_INSTALL route key doesn't exist", __func__);
Expand All @@ -229,25 +230,21 @@ void process_route_install_msg(Fpm__AddRoute *msg)
zlog_err("%s: ROUTE_INSTALL prefix doesn't exist", __func__);
return;
}
if (IS_DPLANE_SERVER_DEBUG)
zlog_debug("%s: msg address family:%d",
__func__, msg->address_family);
if (msg->address_family == AF_INET) {
inet_ntop(AF_INET, msg->key->prefix->bytes.data, buf,
sizeof(buf));
if (IS_DPLANE_SERVER_DEBUG)
zlog_debug("%s: key ipv4 prefix:%pI4", __func__, buf);
} else if (msg->address_family == AF_INET6) {
inet_ntop(AF_INET6, msg->key->prefix->bytes.data, buf,
sizeof(buf));
if (IS_DPLANE_SERVER_DEBUG)
zlog_debug("%s: key ipv6 prefix:%pI6", __func__, buf);
} else {

if (msg->address_family != AF_INET && msg->address_family != AF_INET6) {
zlog_err("%s: not ipv4 or ipv6 address family", __func__);
return;
}
if (!qpb__l3_prefix__get(msg->key->prefix, msg->address_family,
&prefix)) {
zlog_err("%s: failed to parse route prefix", __func__);
return;
}
if (IS_DPLANE_SERVER_DEBUG)
zlog_debug("%s: key length:%d", __func__, msg->key->prefix->length);
zlog_debug("%s: msg address family: %d, key %s prefix: %pFX length: %d",
__func__, msg->address_family,
(msg->address_family == AF_INET) ? "ipv4" : "ipv6",
&prefix, msg->key->prefix->length);

json_object *json = json_object_new_object();

Expand All @@ -261,7 +258,7 @@ void process_route_install_msg(Fpm__AddRoute *msg)
(int64_t)(msg->has_route_type));
json_object_int_add(json, "routeType", (int64_t)(msg->route_type));
if (msg->key) {
json_object_string_add(json, "prefix", buf);
json_object_string_addf(json, "prefix", "%pFXh", &prefix);
json_object_int_add(json, "prefixLength",
(int64_t)(msg->key->prefix->length));
}
Expand Down
4 changes: 3 additions & 1 deletion redhat/frr.spec.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,9 @@ fi
%{_sbindir}/ripd
%{_sbindir}/bgpd
%{_sbindir}/mgmtd
%if %{with_mgmtd_test_be_client}
%{_sbindir}/mgmtd_testc
%endif
%{_sbindir}/dplaneserver
%exclude %{_sbindir}/ssd
%if %{with_watchfrr}
Expand Down Expand Up @@ -717,7 +720,6 @@ fi
%{_libdir}/frr/modules/zebra_cumulus_mlag.so
%{_libdir}/frr/modules/dplane_fpm_nl.so
%{_libdir}/frr/modules/dplane_fpm_pb.so
%{_libdir}/frr/modules/zebra_irdp.so
%{_libdir}/frr/modules/bgpd_bmp.so
%{_libdir}/libfrr_pb.so*
%{_libdir}/libfrrfpm_pb.so*
Expand Down
108 changes: 19 additions & 89 deletions zebra/dplane_fpm_pb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ static void fpm_process_event(struct event *t)

switch (event) {
case FPE_DISABLE:
zlog_info("%s: manual FPM disable event", __func__);
if (IS_ZEBRA_DEBUG_FPM)
zlog_debug("%s: manual FPM disable event", __func__);
fpc->disabled = true;
atomic_fetch_add_explicit(&fpc->counters.user_disables, 1,
memory_order_relaxed);
Expand All @@ -389,7 +390,8 @@ static void fpm_process_event(struct event *t)
break;

case FPE_RECONNECT:
zlog_info("%s: manual FPM reconnect event", __func__);
if (IS_ZEBRA_DEBUG_FPM)
zlog_debug("%s: manual FPM reconnect event", __func__);
fpc->disabled = false;
atomic_fetch_add_explicit(&fpc->counters.user_configures, 1,
memory_order_relaxed);
Expand All @@ -399,7 +401,8 @@ static void fpm_process_event(struct event *t)
fpm_reconnect(fpc);
break;
case FPE_RESET_COUNTERS:
zlog_info("%s: manual FPM counters reset event", __func__);
if (IS_ZEBRA_DEBUG_FPM)
zlog_debug("%s: manual FPM counters reset event", __func__);
memset(&fpc->counters, 0, sizeof(fpc->counters));
break;
}
Expand Down Expand Up @@ -618,85 +621,23 @@ static Fpm__Message *create_fpm_message(qpb_allocator_t *allocator,
return NULL;

fpm__message__init(msg);
switch (op) {
case DPLANE_OP_ROUTE_INSTALL:
if (op == DPLANE_OP_ROUTE_INSTALL) {
/*create add route message*/
msg->has_type = 1;
msg->type = FPM__MESSAGE__TYPE__ADD_ROUTE;
if (IS_ZEBRA_DEBUG_FPM) {
zlog_debug("fpm_pb message:");
zlog_debug("==========");
zlog_debug("has_type: %d", msg->has_type);
zlog_debug("type: %d", msg->type);
}
if (IS_ZEBRA_DEBUG_FPM)
zlog_debug("%s: Creating FPM message: has_type %d type %d",
__func__, msg->has_type, msg->type);
msg->add_route = create_route_install_message(allocator, ctx);
if (!msg->add_route)
return NULL;
break;

/* Un-handled by FPM at this time. */
case DPLANE_OP_ROUTE_UPDATE:
case DPLANE_OP_ROUTE_DELETE:
case DPLANE_OP_MAC_INSTALL:
case DPLANE_OP_MAC_DELETE:
case DPLANE_OP_NH_DELETE:
case DPLANE_OP_NH_INSTALL:
case DPLANE_OP_NH_UPDATE:
case DPLANE_OP_LSP_INSTALL:
case DPLANE_OP_LSP_UPDATE:
case DPLANE_OP_LSP_DELETE:
case DPLANE_OP_PW_INSTALL:
case DPLANE_OP_PW_UNINSTALL:
case DPLANE_OP_ADDR_INSTALL:
case DPLANE_OP_ADDR_UNINSTALL:
case DPLANE_OP_NEIGH_INSTALL:
case DPLANE_OP_NEIGH_UPDATE:
case DPLANE_OP_NEIGH_DELETE:
case DPLANE_OP_VTEP_ADD:
case DPLANE_OP_VTEP_DELETE:
case DPLANE_OP_SYS_ROUTE_ADD:
case DPLANE_OP_SYS_ROUTE_DELETE:
case DPLANE_OP_ROUTE_NOTIFY:
case DPLANE_OP_LSP_NOTIFY:
case DPLANE_OP_RULE_ADD:
case DPLANE_OP_RULE_DELETE:
case DPLANE_OP_RULE_UPDATE:
case DPLANE_OP_NEIGH_DISCOVER:
case DPLANE_OP_BR_PORT_UPDATE:
case DPLANE_OP_IPTABLE_ADD:
case DPLANE_OP_IPTABLE_DELETE:
case DPLANE_OP_IPSET_ADD:
case DPLANE_OP_IPSET_DELETE:
case DPLANE_OP_IPSET_ENTRY_ADD:
case DPLANE_OP_IPSET_ENTRY_DELETE:
case DPLANE_OP_NEIGH_IP_INSTALL:
case DPLANE_OP_NEIGH_IP_DELETE:
case DPLANE_OP_NEIGH_TABLE_UPDATE:
case DPLANE_OP_GRE_SET:
case DPLANE_OP_INTF_ADDR_ADD:
case DPLANE_OP_INTF_ADDR_DEL:
case DPLANE_OP_INTF_NETCONFIG:
case DPLANE_OP_INTF_INSTALL:
case DPLANE_OP_INTF_UPDATE:
case DPLANE_OP_INTF_DELETE:
case DPLANE_OP_TC_QDISC_INSTALL:
case DPLANE_OP_TC_QDISC_UNINSTALL:
case DPLANE_OP_TC_CLASS_ADD:
case DPLANE_OP_TC_CLASS_DELETE:
case DPLANE_OP_TC_CLASS_UPDATE:
case DPLANE_OP_TC_FILTER_ADD:
case DPLANE_OP_TC_FILTER_DELETE:
case DPLANE_OP_TC_FILTER_UPDATE:
case DPLANE_OP_NONE:
case DPLANE_OP_STARTUP_STAGE:
break;
}

return msg;
}

static Fpm__AddRoute *create_route_install_message(qpb_allocator_t *allocator,
struct zebra_dplane_ctx *ctx)
struct zebra_dplane_ctx *ctx)
{
Fpm__AddRoute *msg;
const struct prefix *p;
Expand All @@ -720,16 +661,12 @@ static Fpm__AddRoute *create_route_install_message(qpb_allocator_t *allocator,
msg->has_route_type = 1;
msg->route_type = FPM__ROUTE_TYPE__NORMAL;

if (IS_ZEBRA_DEBUG_FPM) {
zlog_debug("add route message:");
zlog_debug("==========");
zlog_debug("vrf_id: %d", msg->vrf_id);
zlog_debug("address_family: %d", msg->address_family);
zlog_debug("metric:%d", msg->metric);
zlog_debug("sub_address_family: %d", msg->sub_address_family);
zlog_debug("has_router_type: %d", msg->has_route_type);
zlog_debug("route_type:%d", msg->route_type);
}
if (IS_ZEBRA_DEBUG_FPM)
zlog_debug("%s: ADD_ROUTE message: vrf_id %d address_family %d metric %d sub_address_family %d has_route_type %d route_type %d",
__func__, msg->vrf_id, msg->address_family,
msg->metric, msg->sub_address_family,
msg->has_route_type, msg->route_type);

return msg;
}

Expand Down Expand Up @@ -861,6 +798,7 @@ static int fpm_pb_enqueue(struct fpm_pb_ctx *fpc, struct zebra_dplane_ctx *ctx)
static int fpm_pb_start(struct zebra_dplane_provider *prov)
{
struct fpm_pb_ctx *fpc;
struct sockaddr_in *sin;

fpc = dplane_provider_get_data(prov);
fpc->fthread = frr_pthread_new(NULL, prov_name, prov_name);
Expand All @@ -874,19 +812,11 @@ static int fpm_pb_start(struct zebra_dplane_provider *prov)
dplane_ctx_q_init(&fpc->ctxqueue);
pthread_mutex_init(&fpc->ctxqueue_mutex, NULL);

struct sockaddr_in *sin;
uint8_t naddr[INET6_BUFSIZ];

if (inet_pton(AF_INET, "127.0.0.1", naddr) != 1) {
zlog_warn("Invalid address: %s", "127.0.0.1");
return -1;
}

sin = (struct sockaddr_in *)&gfpc->addr;
memset(sin, 0, sizeof(*sin));
sin->sin_family = AF_INET;
sin->sin_port = htons(SOUTHBOUND_DEFAULT_PORT);
memcpy(&sin->sin_addr, naddr, sizeof(sin->sin_addr));
sin->sin_addr.s_addr = htonl(INADDR_LOOPBACK);

event_add_timer(fpc->fthread->master, fpm_connect, fpc, 0,
&fpc->t_connect);
Expand Down

0 comments on commit 513be69

Please sign in to comment.