Page MenuHomeFreeBSD
Differential D39048 Diff 119115 lib/libifconfig/libifconfig_carp.c

Changeset View

Standalone View

View Options

lib/libifconfig/libifconfig_carp.c

Show All 27 Lines
* *
* $FreeBSD$ * $FreeBSD$
*/ */
#include <sys/param.h> #include <sys/param.h>
#include <sys/ioctl.h> #include <sys/ioctl.h>
#include <net/if.h> #include <net/if.h>
#include <netinet/ip_carp.h> #include <netinet/ip_carp.h>
#include <netinet/ip_carp_nl.h>
#include <netlink/netlink.h>
#include <netlink/netlink_generic.h>
#include <netlink/netlink_snl.h>
#include <netlink/netlink_snl_generic.h>
#include <string.h> #include <string.h>
#include <strings.h> #include <strings.h>
#include "libifconfig.h" #include "libifconfig.h"
#include "libifconfig_internal.h" #include "libifconfig_internal.h"
#include <stdio.h>
#define _OUT(_field) offsetof(struct ifconfig_carp, _field)
static struct snl_attr_parser ap_carp_get[] = {
{ .type = CARP_NL_VHID, .off = _OUT(carpr_vhid), .cb = snl_attr_get_uint32 },
{ .type = CARP_NL_STATE, .off = _OUT(carpr_state), .cb = snl_attr_get_uint32 },
{ .type = CARP_NL_ADVBASE, .off = _OUT(carpr_advbase), .cb = snl_attr_get_int32 },
{ .type = CARP_NL_ADVSKEW, .off = _OUT(carpr_advskew), .cb = snl_attr_get_int32 },
{ .type = CARP_NL_KEY, .off = _OUT(carpr_key), .cb = snl_attr_get_string },
};
#undef _OUT
SNL_DECLARE_GENL_PARSER(carp_get_parser, ap_carp_get);
static int
_ifconfig_carp_get(ifconfig_handle_t *h, const char *name,
struct ifconfig_carp *carp, size_t ncarp, uint32_t vhid)
{
struct snl_state ss = {};
struct snl_errmsg_data e = {};
struct snl_writer nw;
struct nlmsghdr *hdr;
kpAuthorUnsubmitted
Done
Inline Actions

We really need to put this in a common library or something, rather than repeat it everywhere we use NETLINK_GENERIC messages.

kp: We really need to put this in a common library or something, rather than repeat it everywhere…
melifaroUnsubmitted
Not Done
Inline Actions

Done as snl_get_genl_family()

melifaro: Done as `snl_get_genl_family()`
size_t i = 0;
uint32_t seq_id;
unsigned int ifindex;
int family_id;
ifconfig_error_clear(h);
ifindex = if_nametoindex(name);
if (ifindex == 0) {
ifconfig_error(h, NETLINK, ENOENT);
return (-1);
}
if (! snl_init(&ss, NETLINK_GENERIC)) {
ifconfig_error(h, NETLINK, ENOTSUP);
return (-1);
}
snl_init_writer(&ss, &nw);
family_id = snl_get_genl_family(&ss, CARP_NL_FAMILY_NAME);
if (family_id == 0) {
ifconfig_error(h, NETLINK, EPROTONOSUPPORT);
goto out;
}
hdr = snl_create_genl_msg_request(&nw, family_id, CARP_NL_CMD_GET);
hdr->nlmsg_flags |= NLM_F_DUMP;
snl_add_msg_attr_u32(&nw, CARP_NL_IFINDEX, ifindex);
if (vhid != 0)
snl_add_msg_attr_u32(&nw, CARP_NL_VHID, vhid);
hdr = snl_finalize_msg(&nw);
if (hdr == NULL) {
ifconfig_error(h, NETLINK, ENOMEM);
goto out;
}
seq_id = hdr->nlmsg_seq;
if (! snl_send_message(&ss, hdr)) {
ifconfig_error(h, NETLINK, EIO);
goto out;
}
while ((hdr = snl_read_reply_multi(&ss, seq_id, &e)) != NULL) {
if (e.error != 0) {
ifconfig_error(h, NETLINK, e.error);
break;
}
if (i >= ncarp) {
melifaroUnsubmitted
Not Done
Inline Actions

Nit: #undef _OUT for consistency

melifaro: Nit: #undef _OUT for consistency
ifconfig_error(h, NETLINK, E2BIG);
melifaroUnsubmitted
Not Done
Inline Actions
static struct snl_field_parser fp_carp_get[] = {
};
- SNL_DECLARE_PARSER(carp_get_parser, struct genlmsghdr, fp_carp_get, ap_carp_get);
+ SNL_DECLARE_GENL_PARSER(carp_get_parser, ap_carp_get);
static int
melifaro:
break;
}
memset(&carp[i], 0, sizeof(carp[0]));
if (! snl_parse_nlmsg(&ss, hdr, &carp_get_parser, &carp[i]))
continue;
i++;
carp[0].carpr_count = i;
if (i > ncarp) {
ifconfig_error(h, NETLINK, E2BIG);
break;
}
}
out:
snl_free(&ss);
return (h->error.errcode ? -1 : 0);
}
int int
ifconfig_carp_get_info(ifconfig_handle_t *h, const char *name, ifconfig_carp_set_info(ifconfig_handle_t *h, const char *name,
struct carpreq *carpr, int ncarpr) const struct ifconfig_carp *carpr)
{ {
struct ifreq ifr; struct snl_state ss = {};
struct snl_writer nw;
struct nlmsghdr *hdr;
unsigned int ifindex;
int family_id;
uint32_t seq_id;
melifaroUnsubmitted
Not Done
Inline Actions
goto out;
- hdr = snl_create_msg_request(&nw, family_id);
+ hdr = snl_create_genl_msg_request(&nw, family_id, CARP_NL_CMD_GET);
hdr->nlmsg_flags |= NLM_F_DUMP;
melifaro:
bzero(carpr, sizeof(struct carpreq) * ncarpr); ifconfig_error_clear(h);
carpr[0].carpr_count = ncarpr;
strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
ifr.ifr_data = (caddr_t)carpr;
if (ifconfig_ioctlwrap(h, AF_LOCAL, SIOCGVH, &ifr) != 0) { ifindex = if_nametoindex(name);
melifaroUnsubmitted
Not Done
Inline Actions

Nit: maybe it's worth adding handling interface as string & just use snl_add_msg_attr_u32(&nw, CARP_NL_IFNAME, name);

melifaro: Nit: maybe it's worth adding handling interface as string & just use `snl_add_msg_attr_u32(&nw…
if (ifindex == 0) {
ifconfig_error(h, NETLINK, ENOENT);
return (-1); return (-1);
} }
return (0); if (! snl_init(&ss, NETLINK_GENERIC)) {
melifaroUnsubmitted
Not Done
Inline Actions

Nit: you can skip checking the return values of snl_add_msg_attr_* and just check snl_finalize_msg() return.

melifaro: Nit: you can skip checking the return values of `snl_add_msg_attr_*` and just check…
ifconfig_error(h, NETLINK, ENOTSUP);
return (-1);
}
snl_init_writer(&ss, &nw);
family_id = snl_get_genl_family(&ss, CARP_NL_FAMILY_NAME);
if (family_id == 0) {
ifconfig_error(h, NETLINK, EPROTONOSUPPORT);
return (-1);
melifaroUnsubmitted
Not Done
Inline Actions
goto out;
}
- if (! snl_send(&ss, hdr, NLMSG_ALIGN(hdr->nlmsg_len))) {
+ if (! snl_send_message(&ss, hdr)) {
ifconfig_error(h, NETLINK, EIO);
melifaro:
}
hdr = snl_create_genl_msg_request(&nw, family_id, CARP_NL_CMD_SET);
snl_add_msg_attr_u32(&nw, CARP_NL_VHID, carpr->carpr_vhid);
melifaroUnsubmitted
Not Done
Inline Actions

I'll try to provide the iterator interface for multipart messages later on, as this pattern have to be repeated for each multipart message, which is tedious.

melifaro: I'll try to provide the iterator interface for multipart messages later on, as this pattern…
snl_add_msg_attr_u32(&nw, CARP_NL_STATE, carpr->carpr_state);
melifaroUnsubmitted
Not Done
Inline Actions
if (hdr->nlmsg_type == NLMSG_ERROR) {
- struct nlmsgerr *nlerr = (struct nlmsgerr *)(hdr + 1);
- ifconfig_error(h, NETLINK, nlerr->error);
+ struct snl_errmsg_data e = {};
+
+ snl_check_return(&ss, hdr, &e);
+
+ ifconfig_error(h, NETLINK, e.error);
+ break; ifconfig_error(h, NETLINK, nlerr->error);

Also: kernel often provides string error (available in e.error_str)

melifaro: Also: kernel often provides string error (available in e.error_str)
kpAuthorUnsubmitted
Done
Inline Actions

Is there an interface to provide that when implementing NETLINK_GENERIC? Right now the carp code returns errors by having the handler function return an error code.

I'm probably not going to handle those either, because libifconfig also doesn't expect to deal with error strings.

kp: Is there an interface to provide that when implementing NETLINK_GENERIC? Right now the carp…
melifaroUnsubmitted
Not Done
Inline Actions

You can use nlmsg_report_err_msg() (or NLMSG_REPORT_ERR_MSG() which also does NLP_LOG(LOG_DEBUG)) to specify the error.
It's also possible to report the atttribute error via nlmsg_report_err_offset() (currently used by the parsers).

Finally, you can add some attributes to the reply, using nlmsg_report_cookie().

melifaro: You can use `nlmsg_report_err_msg()` (or NLMSG_REPORT_ERR_MSG() which also does `NLP_LOG…
melifaroUnsubmitted
Not Done
Inline Actions

struct snl_errmsg_data e = {};

while ((hdr = snl_read_reply_multi(&ss, seq_id, &e)) != NULL) {

...

}

melifaro: struct snl_errmsg_data e = {}; while ((hdr = snl_read_reply_multi(&ss, seq_id, &e)) != NULL) {…
snl_add_msg_attr_s32(&nw, CARP_NL_ADVBASE, carpr->carpr_advbase);
snl_add_msg_attr_s32(&nw, CARP_NL_ADVSKEW, carpr->carpr_advskew);
snl_add_msg_attr_u32(&nw, CARP_NL_IFINDEX, ifindex);
hdr = snl_finalize_msg(&nw);
if (hdr == NULL) {
ifconfig_error(h, NETLINK, ENOMEM);
goto out;
}
seq_id = hdr->nlmsg_seq;
if (! snl_send_message(&ss, hdr)) {
ifconfig_error(h, NETLINK, EIO);
goto out;
}
struct snl_errmsg_data e = { };
if (! snl_read_reply_code(&ss, seq_id, &e))
melifaroUnsubmitted
Done
Inline Actions

Nit: this is not needed as it's the default flags for snl_create_msg_request().

melifaro: Nit: this is not needed as it's the default flags for `snl_create_msg_request()`.
ifconfig_error(h, NETLINK, e.error);
melifaroUnsubmitted
Not Done
Inline Actions

Do you want to have it as a structure?
Eventually when API evolves, some fields are always added and some gets removed.
For example, in the NETLINK_ROUTE, half of the fields in the base header are not used anymore, and some are superseded by the TLVS. Maybe it's better to do all-TLV here from day 1?

melifaro: Do you want to have it as a structure? Eventually when API evolves, some fields are always…
kpAuthorUnsubmitted
Done
Inline Actions

The thinking was that these are fields we're always going to want to supply, and we could add whatever extra we need as TLVs, but it's true that that sort of thing is hard to predict.

kp: The thinking was that these are fields we're always going to want to supply, and we could add…
out:
snl_free(&ss);
return (h->error.errcode ? -1 : 0);
}
int
melifaroUnsubmitted
Done
Inline Actions

Do we want to encode a name here an not ifindex? (and if we do, I'd suggest to have it as TLV, is it won't have IFNAMSIZ limit (and normally will consume less space).

melifaro: Do we want to encode a name here an not ifindex? (and if we do, I'd suggest to have it as TLV…
kpAuthorUnsubmitted
Done
Inline Actions

Yeah, we should just use ifindex here.

kp: Yeah, we should just use ifindex here.
ifconfig_carp_get_vhid(ifconfig_handle_t *h, const char *name,
melifaroUnsubmitted
Done
Inline Actions

Nit: recording it as binary TLV allows to change CARP_KEY_LEN semi-easily.

melifaro: Nit: recording it as binary TLV allows to change `CARP_KEY_LEN` semi-easily.
struct ifconfig_carp *carp, uint32_t vhid)
{
return (_ifconfig_carp_get(h, name, carp, 1, vhid));
}
int
ifconfig_carp_get_info(ifconfig_handle_t *h, const char *name,
struct ifconfig_carp *carp, size_t ncarp)
{
return (_ifconfig_carp_get(h, name, carp, ncarp, 0));
} }
melifaroUnsubmitted
Not Done
Inline Actions

I'll wrap this into something like snl_request_sync(ss, hdr, e)

melifaro: I'll wrap this into something like `snl_request_sync(ss, hdr, e)`