Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zebra neigh update #15179

Merged
merged 3 commits into from
Jan 23, 2024
Merged

Zebra neigh update #15179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
275edb5
*: Rename ZEBRA_NHRP_NEIGH_XXX to ZEBRA_NEIGH_XXX
donaldsharp Jan 19, 2024
a0d38a7
lib, nhrpd: Move neighbor reg/unreg to lib/zclient.c
donaldsharp Jan 19, 2024
01cb1fb
sharpd: Allow sharpd to listen to neighbor events
donaldsharp Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/user/sharp.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ keyword. At present, no sharp commands will be preserved in the config.
Install a label into the kernel that causes the specified vrf NAME table to
be used for pop and forward operations when the specified label is seen.

.. clicmd:: sharp watch [vrf VRF_NAME] neighbor

Instruct zebra to notify sharpd about neighbor events in the specified vrf.
If no vrf is specified then assume default.

.. clicmd:: sharp watch <nexthop <A.B.C.D|X:X::X:X>|import <A.B.C.D/M:X:X::X:X/M> [connected]

Instruct zebra to monitor and notify sharp when the specified nexthop is
Expand Down
10 changes: 5 additions & 5 deletions lib/log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,11 +444,11 @@ static const struct zebra_desc_table command_types[] = {
DESC_ENTRY(ZEBRA_NEIGH_DISCOVER),
DESC_ENTRY(ZEBRA_ROUTE_NOTIFY_REQUEST),
DESC_ENTRY(ZEBRA_CLIENT_CLOSE_NOTIFY),
DESC_ENTRY(ZEBRA_NHRP_NEIGH_ADDED),
DESC_ENTRY(ZEBRA_NHRP_NEIGH_REMOVED),
DESC_ENTRY(ZEBRA_NHRP_NEIGH_GET),
DESC_ENTRY(ZEBRA_NHRP_NEIGH_REGISTER),
DESC_ENTRY(ZEBRA_NHRP_NEIGH_UNREGISTER),
DESC_ENTRY(ZEBRA_NEIGH_ADDED),
DESC_ENTRY(ZEBRA_NEIGH_REMOVED),
DESC_ENTRY(ZEBRA_NEIGH_GET),
DESC_ENTRY(ZEBRA_NEIGH_REGISTER),
DESC_ENTRY(ZEBRA_NEIGH_UNREGISTER),
DESC_ENTRY(ZEBRA_NEIGH_IP_ADD),
DESC_ENTRY(ZEBRA_NEIGH_IP_DEL),
DESC_ENTRY(ZEBRA_CONFIGURE_ARP),
Expand Down
24 changes: 23 additions & 1 deletion lib/zclient.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4709,7 +4709,7 @@ static int zclient_neigh_ip_read_entry(struct stream *s, struct ipaddr *add)

int zclient_neigh_ip_encode(struct stream *s, uint16_t cmd, union sockunion *in,
union sockunion *out, struct interface *ifp,
int ndm_state)
int ndm_state, int ip_len)
{
int ret = 0;

Expand All @@ -4722,6 +4722,7 @@ int zclient_neigh_ip_encode(struct stream *s, uint16_t cmd, union sockunion *in,
sockunion_get_addrlen(out));
} else
stream_putc(s, AF_UNSPEC);
stream_putl(s, ip_len);
stream_putl(s, ifp->ifindex);
if (out)
stream_putl(s, ndm_state);
Expand All @@ -4739,6 +4740,7 @@ int zclient_neigh_ip_decode(struct stream *s, struct zapi_neigh_ip *api)
return -1;
zclient_neigh_ip_read_entry(s, &api->ip_out);

STREAM_GETL(s, api->ip_len);
STREAM_GETL(s, api->index);
STREAM_GETL(s, api->ndm_state);
return 0;
Expand Down Expand Up @@ -4885,3 +4887,23 @@ enum zclient_send_status zclient_opaque_drop_notify(struct zclient *zclient,

return zclient_send_message(zclient);
}

void zclient_register_neigh(struct zclient *zclient, vrf_id_t vrf_id, afi_t afi,
bool reg)
{
struct stream *s;

if (!zclient || zclient->sock < 0)
return;

s = zclient->obuf;
stream_reset(s);

zclient_create_header(s,
reg ? ZEBRA_NEIGH_REGISTER
: ZEBRA_NEIGH_UNREGISTER,
vrf_id);
stream_putw(s, afi);
stream_putw_at(s, 0, stream_get_endp(s));
zclient_send_message(zclient);
}
16 changes: 10 additions & 6 deletions lib/zclient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,11 @@ typedef enum {
ZEBRA_NEIGH_DISCOVER,
ZEBRA_ROUTE_NOTIFY_REQUEST,
ZEBRA_CLIENT_CLOSE_NOTIFY,
ZEBRA_NHRP_NEIGH_ADDED,
ZEBRA_NHRP_NEIGH_REMOVED,
ZEBRA_NHRP_NEIGH_GET,
ZEBRA_NHRP_NEIGH_REGISTER,
ZEBRA_NHRP_NEIGH_UNREGISTER,
ZEBRA_NEIGH_ADDED,
ZEBRA_NEIGH_REMOVED,
ZEBRA_NEIGH_GET,
ZEBRA_NEIGH_REGISTER,
ZEBRA_NEIGH_UNREGISTER,
ZEBRA_NEIGH_IP_ADD,
ZEBRA_NEIGH_IP_DEL,
ZEBRA_CONFIGURE_ARP,
Expand Down Expand Up @@ -867,6 +867,7 @@ extern const struct zclient_options zclient_options_auxiliary;

struct zapi_neigh_ip {
int cmd;
int ip_len;
struct ipaddr ip_in;
struct ipaddr ip_out;
ifindex_t index;
Expand All @@ -875,7 +876,7 @@ struct zapi_neigh_ip {
int zclient_neigh_ip_decode(struct stream *s, struct zapi_neigh_ip *api);
int zclient_neigh_ip_encode(struct stream *s, uint16_t cmd, union sockunion *in,
union sockunion *out, struct interface *ifp,
int ndm_state);
int ndm_state, int ip_len);

/*
* We reserve the top 4 bits for l2-NHG, everything else
Expand Down Expand Up @@ -1322,6 +1323,9 @@ enum zapi_opaque_registry {
*/
extern enum zclient_send_status zclient_send_hello(struct zclient *client);

extern void zclient_register_neigh(struct zclient *zclient, vrf_id_t vrf_id,
afi_t afi, bool reg);

extern enum zclient_send_status
zclient_send_neigh_discovery_req(struct zclient *zclient,
const struct interface *ifp,
Expand Down
17 changes: 10 additions & 7 deletions nhrpd/netlink_arp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ int nhrp_neighbor_operation(ZAPI_CALLBACK_ARGS)
struct zapi_neigh_ip api = {};

zclient_neigh_ip_decode(zclient->ibuf, &api);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a comment on why you are allowing only /32 is welcome.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol. That is what the original code did. I just moved it from Zebra to up into nhrpd. I don't fully understand it but if I had to guess it's a RTM_NEIGH entry where the lladr is set to a v4 address. I left it alone mainly because I didn't know and didn't want to take the time to figure it out.

if (api.ip_len != IPV4_MAX_BYTELEN && api.ip_len != 0)
return 0;

if (api.ip_in.ipa_type == AF_UNSPEC)
return 0;
sockunion_family(&addr) = api.ip_in.ipa_type;
Expand All @@ -176,12 +180,11 @@ int nhrp_neighbor_operation(ZAPI_CALLBACK_ARGS)
return 0;
debugf(NHRP_DEBUG_KERNEL,
"Netlink: %s %pSU dev %s lladdr %pSU nud 0x%x cache used %u type %u",
(cmd == ZEBRA_NHRP_NEIGH_GET)
? "who-has"
: (cmd == ZEBRA_NHRP_NEIGH_ADDED) ? "new-neigh"
: "del-neigh",
(cmd == ZEBRA_NEIGH_GET) ? "who-has"
: (cmd == ZEBRA_NEIGH_ADDED) ? "new-neigh"
: "del-neigh",
&addr, ifp->name, &lladdr, ndm_state, c->used, c->cur.type);
if (cmd == ZEBRA_NHRP_NEIGH_GET) {
if (cmd == ZEBRA_NEIGH_GET) {
if (c->cur.type >= NHRP_CACHE_CACHED) {
nhrp_cache_set_used(c, 1);
debugf(NHRP_DEBUG_KERNEL,
Expand All @@ -194,8 +197,8 @@ int nhrp_neighbor_operation(ZAPI_CALLBACK_ARGS)
netlink_update_binding(ifp, &addr, &lladdr);
}
} else {
state = (cmd == ZEBRA_NHRP_NEIGH_ADDED) ? ndm_state
: ZEBRA_NEIGH_STATE_FAILED;
state = (cmd == ZEBRA_NEIGH_ADDED) ? ndm_state
: ZEBRA_NEIGH_STATE_FAILED;
nhrp_cache_set_used(c, state == ZEBRA_NEIGH_STATE_REACHABLE);
}
return 0;
Expand Down
41 changes: 12 additions & 29 deletions nhrpd/nhrp_route.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,6 @@ static void nhrp_route_update_zebra(const struct prefix *p,
}
}

static void nhrp_zebra_register_neigh(vrf_id_t vrf_id, afi_t afi, bool reg)
{
struct stream *s;

if (!zclient || zclient->sock < 0)
return;

s = zclient->obuf;
stream_reset(s);

zclient_create_header(s, reg ? ZEBRA_NHRP_NEIGH_REGISTER :
ZEBRA_NHRP_NEIGH_UNREGISTER,
vrf_id);
stream_putw(s, afi);
stream_putw_at(s, 0, stream_get_endp(s));
zclient_send_message(zclient);
}

void nhrp_route_update_nhrp(const struct prefix *p, struct interface *ifp)
{
struct route_node *rn;
Expand Down Expand Up @@ -368,18 +350,18 @@ static void nhrp_zebra_connected(struct zclient *zclient)
ZEBRA_ROUTE_ALL, 0, VRF_DEFAULT);
zebra_redistribute_send(ZEBRA_REDISTRIBUTE_ADD, zclient, AFI_IP6,
ZEBRA_ROUTE_ALL, 0, VRF_DEFAULT);
nhrp_zebra_register_neigh(VRF_DEFAULT, AFI_IP, true);
nhrp_zebra_register_neigh(VRF_DEFAULT, AFI_IP6, true);
zclient_register_neigh(zclient, VRF_DEFAULT, AFI_IP, true);
zclient_register_neigh(zclient, VRF_DEFAULT, AFI_IP6, true);
}

static zclient_handler *const nhrp_handlers[] = {
[ZEBRA_INTERFACE_ADDRESS_ADD] = nhrp_interface_address_add,
[ZEBRA_INTERFACE_ADDRESS_DELETE] = nhrp_interface_address_delete,
[ZEBRA_REDISTRIBUTE_ROUTE_ADD] = nhrp_route_read,
[ZEBRA_REDISTRIBUTE_ROUTE_DEL] = nhrp_route_read,
[ZEBRA_NHRP_NEIGH_ADDED] = nhrp_neighbor_operation,
[ZEBRA_NHRP_NEIGH_REMOVED] = nhrp_neighbor_operation,
[ZEBRA_NHRP_NEIGH_GET] = nhrp_neighbor_operation,
[ZEBRA_NEIGH_ADDED] = nhrp_neighbor_operation,
[ZEBRA_NEIGH_REMOVED] = nhrp_neighbor_operation,
[ZEBRA_NEIGH_GET] = nhrp_neighbor_operation,
[ZEBRA_GRE_UPDATE] = nhrp_gre_update,
};

Expand Down Expand Up @@ -456,10 +438,11 @@ void nhrp_send_zebra_nbr(union sockunion *in,
return;
s = zclient->obuf;
stream_reset(s);
zclient_neigh_ip_encode(s, out ? ZEBRA_NEIGH_IP_ADD :
ZEBRA_NEIGH_IP_DEL, in, out,
ifp, out ? ZEBRA_NEIGH_STATE_REACHABLE
: ZEBRA_NEIGH_STATE_FAILED);
zclient_neigh_ip_encode(s, out ? ZEBRA_NEIGH_IP_ADD : ZEBRA_NEIGH_IP_DEL,
in, out, ifp,
out ? ZEBRA_NEIGH_STATE_REACHABLE
: ZEBRA_NEIGH_STATE_FAILED,
0);
Copy link

@pbrisset pbrisset Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add comment of the meaning of 0 (hardcoded value)
something like

0); // meaning something cool

This is the ip_len. Why are you using 0?

stream_putw_at(s, 0, stream_get_endp(s));
zclient_send_message(zclient);
}
Expand All @@ -471,8 +454,8 @@ int nhrp_send_zebra_gre_request(struct interface *ifp)

void nhrp_zebra_terminate(void)
{
nhrp_zebra_register_neigh(VRF_DEFAULT, AFI_IP, false);
nhrp_zebra_register_neigh(VRF_DEFAULT, AFI_IP6, false);
zclient_register_neigh(zclient, VRF_DEFAULT, AFI_IP, false);
zclient_register_neigh(zclient, VRF_DEFAULT, AFI_IP6, false);
zclient_stop(zclient);
zclient_free(zclient);

Expand Down
27 changes: 27 additions & 0 deletions sharpd/sharp_vty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,32 @@

DEFINE_MTYPE_STATIC(SHARPD, SRV6_LOCATOR, "SRv6 Locator");

DEFPY(watch_neighbor, watch_neighbor_cmd,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a v6 watch too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a Proof of Concept bit of code. I don't mind having it and I don't mind not having it. An end operator is not likely to ever use this.

"sharp watch [vrf NAME$vrf_name] neighbor",
"Sharp routing Protocol\n"
"Watch for changes\n"
"The vrf we would like to watch if non-default\n"
"The NAME of the vrf\n"
"Neighbor events\n")
{
struct vrf *vrf;

if (!vrf_name)
vrf_name = VRF_DEFAULT_NAME;
vrf = vrf_lookup_by_name(vrf_name);

if (!vrf) {
vty_out(vty, "The vrf NAME specified: %s does not exist\n",
vrf_name);
return CMD_WARNING;
}

sharp_zebra_register_neigh(vrf->vrf_id, AFI_IP, true);

return CMD_SUCCESS;
}


DEFPY(watch_redistribute, watch_redistribute_cmd,
"sharp watch [vrf NAME$vrf_name] redistribute " FRR_REDIST_STR_SHARPD,
"Sharp routing Protocol\n"
Expand Down Expand Up @@ -1419,6 +1445,7 @@ void sharp_vty_init(void)
install_element(ENABLE_NODE, &remove_routes_cmd);
install_element(ENABLE_NODE, &vrf_label_cmd);
install_element(ENABLE_NODE, &sharp_nht_data_dump_cmd);
install_element(ENABLE_NODE, &watch_neighbor_cmd);
install_element(ENABLE_NODE, &watch_redistribute_cmd);
install_element(ENABLE_NODE, &watch_nexthop_v6_cmd);
install_element(ENABLE_NODE, &watch_nexthop_v4_cmd);
Expand Down
43 changes: 43 additions & 0 deletions sharpd/sharp_zebra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,41 @@ static int sharp_zebra_process_srv6_locator_chunk(ZAPI_CALLBACK_ARGS)
return 0;
}

static int sharp_zebra_process_neigh(ZAPI_CALLBACK_ARGS)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function always return 0.
What is the difference between failure and success?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a historical baggage that is carried around at the moment, that needs to be cleaned up. The original code all had this idea of returning error messages. But whenever a non-zero was returned it was always ignored. When the current handler code was abstracted, by Quentin, to use arrays of function pointers it got carried over. The original coder of a lot of functionality in FRR had this idea of returning 0 or 1 for success and immediately throwing the answer away and never using it. We've cleaned up large swaths. This is just another example of something that needs to be fixed from my perspective.

{
union sockunion addr = {}, lladdr = {};
struct zapi_neigh_ip api = {};
struct interface *ifp;

zlog_debug("Received a neighbor event");
zclient_neigh_ip_decode(zclient->ibuf, &api);

if (api.ip_in.ipa_type == AF_UNSPEC)
return 0;

sockunion_family(&addr) = api.ip_in.ipa_type;
memcpy((uint8_t *)sockunion_get_addr(&addr), &api.ip_in.ip.addr,
family2addrsize(api.ip_in.ipa_type));

sockunion_family(&lladdr) = api.ip_out.ipa_type;
if (api.ip_out.ipa_type != AF_UNSPEC)
memcpy((uint8_t *)sockunion_get_addr(&lladdr),
&api.ip_out.ip.addr,
family2addrsize(api.ip_out.ipa_type));
ifp = if_lookup_by_index(api.index, vrf_id);
if (!ifp) {
zlog_debug("Failed to lookup interface for neighbor entry: %u for %u",
api.index, vrf_id);
return 0;
}

zlog_debug("Received: %s %pSU dev %s lladr %pSU",
(cmd = ZEBRA_NEIGH_ADDED) ? "NEW" : "DEL", &addr, ifp->name,
&lladdr);

return 0;
}

int sharp_zebra_send_interface_protodown(struct interface *ifp, bool down)
{
zlog_debug("Sending zebra to set %s protodown %s", ifp->name,
Expand Down Expand Up @@ -1059,6 +1094,12 @@ int sharp_zebra_send_tc_filter_rate(struct interface *ifp,
return 0;
}

void sharp_zebra_register_neigh(vrf_id_t vrf_id, afi_t afi, bool reg)
{
zclient_register_neigh(zclient, vrf_id, afi, reg);
}


static zclient_handler *const sharp_handlers[] = {
[ZEBRA_INTERFACE_ADDRESS_ADD] = interface_address_add,
[ZEBRA_INTERFACE_ADDRESS_DELETE] = interface_address_delete,
Expand All @@ -1070,6 +1111,8 @@ static zclient_handler *const sharp_handlers[] = {
[ZEBRA_OPAQUE_NOTIFY] = sharp_opq_notify_handler,
[ZEBRA_SRV6_MANAGER_GET_LOCATOR_CHUNK] =
sharp_zebra_process_srv6_locator_chunk,
[ZEBRA_NEIGH_ADDED] = sharp_zebra_process_neigh,
[ZEBRA_NEIGH_REMOVED] = sharp_zebra_process_neigh,
};

void sharp_zebra_init(void)
Expand Down
2 changes: 2 additions & 0 deletions sharpd/sharp_zebra.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,6 @@ extern int sharp_zebra_send_tc_filter_rate(struct interface *ifp,
const struct prefix *destination,
uint8_t ip_proto, uint16_t src_port,
uint16_t dst_port, uint64_t rate);

extern void sharp_zebra_register_neigh(vrf_id_t vrf_id, afi_t afi, bool reg);
#endif
30 changes: 14 additions & 16 deletions zebra/rt_netlink.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4242,11 +4242,11 @@ static int netlink_ipneigh_change(struct nlmsghdr *h, int len, ns_id_t ns_id)
* - struct ethaddr mac; (for NEW)
*/
if (h->nlmsg_type == RTM_NEWNEIGH)
cmd = ZEBRA_NHRP_NEIGH_ADDED;
cmd = ZEBRA_NEIGH_ADDED;
else if (h->nlmsg_type == RTM_GETNEIGH)
cmd = ZEBRA_NHRP_NEIGH_GET;
cmd = ZEBRA_NEIGH_GET;
else if (h->nlmsg_type == RTM_DELNEIGH)
cmd = ZEBRA_NHRP_NEIGH_REMOVED;
cmd = ZEBRA_NEIGH_REMOVED;
else {
zlog_debug("%s(): unknown nlmsg type %u", __func__,
h->nlmsg_type);
Expand All @@ -4256,20 +4256,18 @@ static int netlink_ipneigh_change(struct nlmsghdr *h, int len, ns_id_t ns_id)
/* copy LLADDR information */
l2_len = RTA_PAYLOAD(tb[NDA_LLADDR]);
}
if (l2_len == IPV4_MAX_BYTELEN || l2_len == 0) {
union sockunion link_layer_ipv4;

if (l2_len) {
sockunion_family(&link_layer_ipv4) = AF_INET;
memcpy((void *)sockunion_get_addr(&link_layer_ipv4),
RTA_DATA(tb[NDA_LLADDR]), l2_len);
} else
sockunion_family(&link_layer_ipv4) = AF_UNSPEC;
zsend_nhrp_neighbor_notify(
cmd, ifp, &ip,
netlink_nbr_entry_state_to_zclient(ndm->ndm_state),
&link_layer_ipv4);
}
union sockunion link_layer_ipv4;

if (l2_len) {
sockunion_family(&link_layer_ipv4) = AF_INET;
memcpy((void *)sockunion_get_addr(&link_layer_ipv4),
RTA_DATA(tb[NDA_LLADDR]), l2_len);
} else
sockunion_family(&link_layer_ipv4) = AF_UNSPEC;
zsend_neighbor_notify(cmd, ifp, &ip,
netlink_nbr_entry_state_to_zclient(ndm->ndm_state),
&link_layer_ipv4, l2_len);

if (h->nlmsg_type == RTM_GETNEIGH)
return 0;
Expand Down
Loading
Loading