Changeset View
Standalone View
sbin/ifconfig/ifpfsync.c
Show All 23 Lines | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | ||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | ||||
* SUCH DAMAGE. | * SUCH DAMAGE. | ||||
* | * | ||||
* $FreeBSD$ | * $FreeBSD$ | ||||
*/ | */ | ||||
#include <sys/param.h> | #include <sys/param.h> | ||||
#include <sys/errno.h> | |||||
#include <sys/ioctl.h> | #include <sys/ioctl.h> | ||||
#include <sys/nv.h> | |||||
#include <sys/socket.h> | #include <sys/socket.h> | ||||
#include <net/if.h> | #include <net/if.h> | ||||
#include <netinet/in.h> | #include <netinet/in.h> | ||||
#include <net/pfvar.h> | #include <net/pfvar.h> | ||||
#include <net/if_pfsync.h> | #include <net/if_pfsync.h> | ||||
#include <net/route.h> | #include <net/route.h> | ||||
#include <arpa/inet.h> | #include <arpa/inet.h> | ||||
Show All 11 Lines | |||||
void unsetpfsync_syncdev(const char *, int, int, const struct afswtch *); | void unsetpfsync_syncdev(const char *, int, int, const struct afswtch *); | ||||
void setpfsync_syncpeer(const char *, int, int, const struct afswtch *); | void setpfsync_syncpeer(const char *, int, int, const struct afswtch *); | ||||
void unsetpfsync_syncpeer(const char *, int, int, const struct afswtch *); | void unsetpfsync_syncpeer(const char *, int, int, const struct afswtch *); | ||||
void setpfsync_syncpeer(const char *, int, int, const struct afswtch *); | void setpfsync_syncpeer(const char *, int, int, const struct afswtch *); | ||||
void setpfsync_maxupd(const char *, int, int, const struct afswtch *); | void setpfsync_maxupd(const char *, int, int, const struct afswtch *); | ||||
void setpfsync_defer(const char *, int, int, const struct afswtch *); | void setpfsync_defer(const char *, int, int, const struct afswtch *); | ||||
void pfsync_status(int); | void pfsync_status(int); | ||||
static int | |||||
pfsync_do_ioctl(int s, uint cmd, nvlist_t **nvl) | |||||
kp: Not blocking, but it might be good to think about how to move this into libpfctl. | |||||
{ | |||||
void *data; | |||||
size_t nvlen; | |||||
data = nvlist_pack(*nvl, &nvlen); | |||||
Done Inline ActionsThere's not much point to checking for allocation failures in user space. The kernel will (by default, at least) overcommit memory. So the first thing a user space applications knows about a memory shortage is not when malloc fails, but when it touches previously unused memory and gets killed by the out-of-memory killer. The ifconfig code is pretty inconsistent about checking for malloc() failure already. My personal view is that it's not worth writing error handling code that will never be exercised (and thus is almost certain to be buggy), but this isn't a blocking remark. kp: There's not much point to checking for allocation failures in user space. The kernel will (by… | |||||
ifr.ifr_cap_nv.buffer = malloc(IFR_CAP_NV_MAXBUFSIZE); | |||||
memcpy(ifr.ifr_cap_nv.buffer, data, nvlen); | |||||
ifr.ifr_cap_nv.buf_length = IFR_CAP_NV_MAXBUFSIZE; | |||||
ifr.ifr_cap_nv.length = nvlen; | |||||
free(data); | |||||
if (ioctl(s, cmd, (caddr_t)&ifr) == -1) { | |||||
Not Done Inline ActionsI'm not strictly happy with using ifr_cap_nv here, because that field is for "nv-based cap interface", and this isn't about interface capabilities. There's no obvious alternative, so perhaps we should look at adding a new name in sys/net/if.h. Let's put that aside until we've got everything else done and tested. kp: I'm not strictly happy with using ifr_cap_nv here, because that field is for "nv-based cap… | |||||
free(ifr.ifr_cap_nv.buffer); | |||||
return -1; | |||||
} | |||||
nvlist_destroy(*nvl); | |||||
Done Inline ActionsWe fail to free ifr.ifr_cap_nv.buffer here. kp: We fail to free ifr.ifr_cap_nv.buffer here. | |||||
*nvl = NULL; | |||||
*nvl = nvlist_unpack(ifr.ifr_cap_nv.buffer, ifr.ifr_cap_nv.length, 0); | |||||
if (*nvl == NULL) { | |||||
free(ifr.ifr_cap_nv.buffer); | |||||
return (EIO); | |||||
} | |||||
free(ifr.ifr_cap_nv.buffer); | |||||
return (errno); | |||||
} | |||||
static nvlist_t * | |||||
pfsync_sockaddr_to_syncpeer_nvlist(struct sockaddr_storage *sa) | |||||
{ | |||||
nvlist_t *nvl; | |||||
nvl = nvlist_create(0); | |||||
if (nvl == NULL) { | |||||
return (nvl); | |||||
} | |||||
switch (sa->ss_family) { | |||||
#ifdef INET | |||||
case AF_INET: { | |||||
struct sockaddr_in *in = (struct sockaddr_in *)sa; | |||||
nvlist_add_number(nvl, "af", in->sin_family); | |||||
nvlist_add_binary(nvl, "address", in, sizeof(*in)); | |||||
break; | |||||
} | |||||
#endif | |||||
#ifdef INET6 | |||||
case AF_INET6: { | |||||
struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)sa; | |||||
nvlist_add_number(nvl, "af", in6->sin6_family); | |||||
nvlist_add_binary(nvl, "address", in6, sizeof(*in6)); | |||||
break; | |||||
} | |||||
#endif | |||||
default: | |||||
nvlist_add_number(nvl, "af", AF_UNSPEC); | |||||
nvlist_add_binary(nvl, "address", sa, sizeof(*sa)); | |||||
break; | |||||
Not Done Inline ActionsIs this worth considering? Do we every specify something that's not IPv4 or IPv6? kp: Is this worth considering? Do we every specify something that's not IPv4 or IPv6? | |||||
Done Inline ActionsThe way pfsync was handling the case where no peer address was set (or was unset) was by leaving the address empty (that would later be filled by the multicast address in if_pfsync.c), which would result in a AF that is neither IPv4 nor IPv6. email_luiz.eng.br: The way pfsync was handling the case where no peer address was set (or was unset) was by… | |||||
} | |||||
return (nvl); | |||||
} | |||||
static int | |||||
pfsync_syncpeer_nvlist_to_sockaddr(const nvlist_t *nvl, | |||||
struct sockaddr_storage *sa) | |||||
{ | |||||
Not Done Inline ActionsThis might be something that can be usefully translated into a libpfctl library function. kp: This might be something that can be usefully translated into a libpfctl library function. | |||||
int af; | |||||
if (!nvlist_exists_number(nvl, "af")) | |||||
return (EINVAL); | |||||
if (!nvlist_exists_binary(nvl, "address")) | |||||
return (EINVAL); | |||||
af = nvlist_get_number(nvl, "af"); | |||||
switch (af) { | |||||
#ifdef INET | |||||
case AF_INET: { | |||||
struct sockaddr_in *in = (struct sockaddr_in *)sa; | |||||
size_t len; | |||||
Not Done Inline ActionsThis seems like pointless work. Can't we just nvlist_add_string(nvl, "synced", val);? kp: This seems like pointless work. Can't we just `nvlist_add_string(nvl, "synced", val);`? | |||||
Done Inline ActionsIt was a complicated way of ensuring val was not bigger than IFNAMSIZ. I added a check for the size, and adjusted the code per your suggestion. if (strlen(val) > IFNAMSIZ) errx(1, "interface name %s is too long", val); email_luiz.eng.br: It was a complicated way of ensuring val was not bigger than IFNAMSIZ.
I added a check for… | |||||
const void *addr = nvlist_get_binary(nvl, "address", &len); | |||||
in->sin_family = af; | |||||
if (len != sizeof(*in)) | |||||
return (EINVAL); | |||||
Done Inline ActionsWe prefer POSIX memset() over bzero() in new code. glebius: We prefer POSIX memset() over bzero() in new code. | |||||
memcpy(in, addr, sizeof(*in)); | |||||
break; | |||||
} | |||||
#endif | |||||
#ifdef INET6 | |||||
case AF_INET6: { | |||||
struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)sa; | |||||
size_t len; | |||||
const void *addr = nvlist_get_binary(nvl, "address", &len); | |||||
if (len != sizeof(*in6)) | |||||
return (EINVAL); | |||||
memcpy(in6, addr, sizeof(*in6)); | |||||
Not Done Inline ActionsIs it worth doing this? All we want is to set an empty string for syncdev, so why not just do that unconditionally? kp: Is it worth doing this? All we want is to set an empty string for syncdev, so why not just do… | |||||
Done Inline ActionsI will simplify it, but we still need the nvlist_exists_string() check, otherwise I get a EEXIST error, as the syncdev is returned by the SIOCGETPFSYNCNV ioctl. email_luiz.eng.br: I will simplify it, but we still need the `nvlist_exists_string()` check, otherwise I get a… | |||||
break; | |||||
} | |||||
#endif | |||||
default: | |||||
return (EINVAL); | |||||
} | |||||
return (0); | |||||
} | |||||
void | void | ||||
setpfsync_syncdev(const char *val, int d, int s, const struct afswtch *rafp) | setpfsync_syncdev(const char *val, int d, int s, const struct afswtch *rafp) | ||||
{ | { | ||||
struct pfsyncreq preq; | nvlist_t *nvl = nvlist_create(0); | ||||
Not Done Inline ActionsAlso a good libpfctl candidate. kp: Also a good libpfctl candidate. | |||||
bzero((char *)&preq, sizeof(struct pfsyncreq)); | if (strlen(val) > IFNAMSIZ) | ||||
ifr.ifr_data = (caddr_t)&preq; | errx(1, "interface name %s is too long", val); | ||||
if (ioctl(s, SIOCGETPFSYNC, (caddr_t)&ifr) == -1) | if (pfsync_do_ioctl(s, SIOCGETPFSYNCNV, &nvl) == -1) | ||||
err(1, "SIOCGETPFSYNC"); | err(1, "SIOCGETPFSYNCNV"); | ||||
strlcpy(preq.pfsyncr_syncdev, val, sizeof(preq.pfsyncr_syncdev)); | if (nvlist_exists_string(nvl, "syncdev")) | ||||
nvlist_free_string(nvl, "syncdev"); | |||||
if (ioctl(s, SIOCSETPFSYNC, (caddr_t)&ifr) == -1) | nvlist_add_string(nvl, "syncdev", val); | ||||
err(1, "SIOCSETPFSYNC"); | |||||
if (pfsync_do_ioctl(s, SIOCSETPFSYNCNV, &nvl) == -1) | |||||
err(1, "SIOCSETPFSYNCNV"); | |||||
} | } | ||||
/* ARGSUSED */ | /* ARGSUSED */ | ||||
void | void | ||||
unsetpfsync_syncdev(const char *val, int d, int s, const struct afswtch *rafp) | unsetpfsync_syncdev(const char *val, int d, int s, const struct afswtch *rafp) | ||||
{ | { | ||||
struct pfsyncreq preq; | nvlist_t *nvl = nvlist_create(0); | ||||
bzero((char *)&preq, sizeof(struct pfsyncreq)); | if (pfsync_do_ioctl(s, SIOCGETPFSYNCNV, &nvl) == -1) | ||||
Done Inline ActionsWe should try to be consistent about using {} for single line statements. At least within the same function. kp: We should try to be consistent about using {} for single line statements. At least within the… | |||||
ifr.ifr_data = (caddr_t)&preq; | err(1, "SIOCGETPFSYNCNV"); | ||||
if (ioctl(s, SIOCGETPFSYNC, (caddr_t)&ifr) == -1) | if (nvlist_exists_string(nvl, "syncdev")) | ||||
err(1, "SIOCGETPFSYNC"); | nvlist_free_string(nvl, "syncdev"); | ||||
bzero((char *)&preq.pfsyncr_syncdev, sizeof(preq.pfsyncr_syncdev)); | nvlist_add_string(nvl, "syncdev", ""); | ||||
Not Done Inline ActionsAnd we'd do case AF_INET6 here, right? kp: And we'd do case AF_INET6 here, right?
Which will come in a subsequent commit? | |||||
Done Inline ActionsThat is my current plan. email_luiz.eng.br: That is my current plan. | |||||
if (ioctl(s, SIOCSETPFSYNC, (caddr_t)&ifr) == -1) | if (pfsync_do_ioctl(s, SIOCSETPFSYNCNV, &nvl) == -1) | ||||
err(1, "SIOCSETPFSYNC"); | err(1, "SIOCSETPFSYNCNV"); | ||||
} | } | ||||
/* ARGSUSED */ | /* ARGSUSED */ | ||||
void | void | ||||
setpfsync_syncpeer(const char *val, int d, int s, const struct afswtch *rafp) | setpfsync_syncpeer(const char *val, int d, int s, const struct afswtch *rafp) | ||||
{ | { | ||||
struct pfsyncreq preq; | struct addrinfo *peerres; | ||||
struct addrinfo hints, *peerres; | struct sockaddr_storage addr; | ||||
int ecode; | int ecode; | ||||
bzero((char *)&preq, sizeof(struct pfsyncreq)); | nvlist_t *nvl = nvlist_create(0); | ||||
ifr.ifr_data = (caddr_t)&preq; | |||||
if (ioctl(s, SIOCGETPFSYNC, (caddr_t)&ifr) == -1) | if (pfsync_do_ioctl(s, SIOCGETPFSYNCNV, &nvl) == -1) | ||||
err(1, "SIOCGETPFSYNC"); | err(1, "SIOCGETPFSYNCNV"); | ||||
memset(&hints, 0, sizeof(hints)); | if ((ecode = getaddrinfo(val, NULL, NULL, &peerres)) != 0) | ||||
hints.ai_family = AF_INET; | |||||
hints.ai_socktype = SOCK_DGRAM; /*dummy*/ | |||||
if ((ecode = getaddrinfo(val, NULL, &hints, &peerres)) != 0) | |||||
errx(1, "error in parsing address string: %s", | errx(1, "error in parsing address string: %s", | ||||
gai_strerror(ecode)); | gai_strerror(ecode)); | ||||
if (peerres->ai_addr->sa_family != AF_INET) | switch (peerres->ai_family) { | ||||
errx(1, "only IPv4 addresses supported for the syncpeer"); | #ifdef INET | ||||
case AF_INET: { | |||||
struct sockaddr_in *sin = (struct sockaddr_in *) | |||||
peerres->ai_addr; | |||||
preq.pfsyncr_syncpeer.s_addr = ((struct sockaddr_in *) | if (IN_MULTICAST(ntohl(sin->sin_addr.s_addr))) | ||||
peerres->ai_addr)->sin_addr.s_addr; | errx(1, "syncpeer address cannot be multicast"); | ||||
if (ioctl(s, SIOCSETPFSYNC, (caddr_t)&ifr) == -1) | memcpy(&addr, sin, sizeof(*sin)); | ||||
err(1, "SIOCSETPFSYNC"); | break; | ||||
} | |||||
#endif | |||||
default: | |||||
errx(1, "syncpeer address %s not supported", val); | |||||
} | |||||
if (nvlist_exists_nvlist(nvl, "syncpeer")) | |||||
nvlist_free_nvlist(nvl, "syncpeer"); | |||||
nvlist_add_nvlist(nvl, "syncpeer", | |||||
pfsync_sockaddr_to_syncpeer_nvlist(&addr)); | |||||
if (pfsync_do_ioctl(s, SIOCSETPFSYNCNV, &nvl) == -1) | |||||
err(1, "SIOCSETPFSYNCNV"); | |||||
nvlist_destroy(nvl); | |||||
freeaddrinfo(peerres); | freeaddrinfo(peerres); | ||||
} | } | ||||
/* ARGSUSED */ | /* ARGSUSED */ | ||||
void | void | ||||
unsetpfsync_syncpeer(const char *val, int d, int s, const struct afswtch *rafp) | unsetpfsync_syncpeer(const char *val, int d, int s, const struct afswtch *rafp) | ||||
{ | { | ||||
struct pfsyncreq preq; | struct sockaddr_storage addr; | ||||
memset(&addr, 0, sizeof(addr)); | |||||
bzero((char *)&preq, sizeof(struct pfsyncreq)); | nvlist_t *nvl = nvlist_create(0); | ||||
ifr.ifr_data = (caddr_t)&preq; | |||||
if (ioctl(s, SIOCGETPFSYNC, (caddr_t)&ifr) == -1) | if (pfsync_do_ioctl(s, SIOCGETPFSYNCNV, &nvl) == -1) | ||||
err(1, "SIOCGETPFSYNC"); | err(1, "SIOCGETPFSYNCNV"); | ||||
preq.pfsyncr_syncpeer.s_addr = 0; | if (nvlist_exists_nvlist(nvl, "syncpeer")) | ||||
nvlist_free_nvlist(nvl, "syncpeer"); | |||||
if (ioctl(s, SIOCSETPFSYNC, (caddr_t)&ifr) == -1) | nvlist_add_nvlist(nvl, "syncpeer", | ||||
err(1, "SIOCSETPFSYNC"); | pfsync_sockaddr_to_syncpeer_nvlist(&addr)); | ||||
if (pfsync_do_ioctl(s, SIOCSETPFSYNCNV, &nvl) == -1) | |||||
err(1, "SIOCSETPFSYNCNV"); | |||||
nvlist_destroy(nvl); | |||||
Done Inline ActionsYes. That's a status flag, and it'll get reset by the kernel on every read. kp: Yes. That's a status flag, and it'll get reset by the kernel on every read.
We can set or clear… | |||||
} | } | ||||
/* ARGSUSED */ | /* ARGSUSED */ | ||||
void | void | ||||
setpfsync_maxupd(const char *val, int d, int s, const struct afswtch *rafp) | setpfsync_maxupd(const char *val, int d, int s, const struct afswtch *rafp) | ||||
{ | { | ||||
struct pfsyncreq preq; | |||||
int maxupdates; | int maxupdates; | ||||
nvlist_t *nvl = nvlist_create(0); | |||||
maxupdates = atoi(val); | maxupdates = atoi(val); | ||||
if ((maxupdates < 0) || (maxupdates > 255)) | if ((maxupdates < 0) || (maxupdates > 255)) | ||||
errx(1, "maxupd %s: out of range", val); | errx(1, "maxupd %s: out of range", val); | ||||
memset((char *)&preq, 0, sizeof(struct pfsyncreq)); | if (pfsync_do_ioctl(s, SIOCGETPFSYNCNV, &nvl) == -1) | ||||
ifr.ifr_data = (caddr_t)&preq; | err(1, "SIOCGETPFSYNCNV"); | ||||
if (ioctl(s, SIOCGETPFSYNC, (caddr_t)&ifr) == -1) | nvlist_free_number(nvl, "maxupdates"); | ||||
err(1, "SIOCGETPFSYNC"); | nvlist_add_number(nvl, "maxupdates", maxupdates); | ||||
preq.pfsyncr_maxupdates = maxupdates; | if (pfsync_do_ioctl(s, SIOCSETPFSYNCNV, &nvl) == -1) | ||||
err(1, "SIOCSETPFSYNCNV"); | |||||
if (ioctl(s, SIOCSETPFSYNC, (caddr_t)&ifr) == -1) | nvlist_destroy(nvl); | ||||
err(1, "SIOCSETPFSYNC"); | |||||
} | } | ||||
/* ARGSUSED */ | /* ARGSUSED */ | ||||
void | void | ||||
setpfsync_defer(const char *val, int d, int s, const struct afswtch *rafp) | setpfsync_defer(const char *val, int d, int s, const struct afswtch *rafp) | ||||
{ | { | ||||
struct pfsyncreq preq; | nvlist_t *nvl = nvlist_create(0); | ||||
memset((char *)&preq, 0, sizeof(struct pfsyncreq)); | if (pfsync_do_ioctl(s, SIOCGETPFSYNCNV, &nvl) == -1) | ||||
ifr.ifr_data = (caddr_t)&preq; | err(1, "SIOCGETPFSYNCNV"); | ||||
if (ioctl(s, SIOCGETPFSYNC, (caddr_t)&ifr) == -1) | nvlist_free_number(nvl, "flags"); | ||||
err(1, "SIOCGETPFSYNC"); | nvlist_add_number(nvl, "flags", d ? PFSYNCF_DEFER : 0); | ||||
preq.pfsyncr_defer = d ? PFSYNCF_DEFER : 0; | if (pfsync_do_ioctl(s, SIOCSETPFSYNCNV, &nvl) == -1) | ||||
if (ioctl(s, SIOCSETPFSYNC, (caddr_t)&ifr) == -1) | err(1, "SIOCSETPFSYNCNV"); | ||||
err(1, "SIOCSETPFSYNC"); | |||||
nvlist_destroy(nvl); | |||||
} | } | ||||
void | void | ||||
pfsync_status(int s) | pfsync_status(int s) | ||||
{ | { | ||||
struct pfsyncreq preq; | nvlist_t *nvl; | ||||
char syncdev[IFNAMSIZ]; | |||||
char syncpeer_str[NI_MAXHOST]; | |||||
struct sockaddr_storage syncpeer; | |||||
int maxupdates; | |||||
int flags; | |||||
int error; | |||||
bzero((char *)&preq, sizeof(struct pfsyncreq)); | nvl = nvlist_create(0); | ||||
ifr.ifr_data = (caddr_t)&preq; | |||||
if (ioctl(s, SIOCGETPFSYNC, (caddr_t)&ifr) == -1) | if (pfsync_do_ioctl(s, SIOCGETPFSYNCNV, &nvl) == -1) { | ||||
Not Done Inline ActionsWhat's the purpose of pfsync_kstatus? The 'k' prefix usual means this is a kernel-only structure (at least in the context of pf), so I'm a little confused at seeing that in user space. This might be a structure definition that belongs in libpfctl.h kp: What's the purpose of pfsync_kstatus?
The 'k' prefix usual means this is a kernel-only… | |||||
Not Done Inline ActionsI tried to mimic what was done with pf_kstate in pf_ioctl.c but failed to understand it was a kernel-only structure. I made this function considering that it would be useful for the other functions, but in the end it only got used once in pfsync_status. I'm considering getting rid of it and moving this code into pfsync_status directly. email_luiz.eng.br: I tried to mimic what was done with pf_kstate in pf_ioctl.c but failed to understand it was a… | |||||
nvlist_destroy(nvl); | |||||
return; | return; | ||||
} | |||||
Done Inline ActionsWe should zero out the status struct so we don't end up with random garbage if the kernel doesn't supply a syncdev. kp: We should zero out the status struct so we don't end up with random garbage if the kernel… | |||||
Done Inline ActionsSince we now zero out the status struct here, I removed the zeroing out that was done in line 367, before calling pfsync_nvstatus_to_kstatus email_luiz.eng.br: Since we now zero out the status struct here, I removed the zeroing out that was done in line… | |||||
if (preq.pfsyncr_syncdev[0] != '\0' || | memset((char *)&syncdev, 0, IFNAMSIZ); | ||||
preq.pfsyncr_syncpeer.s_addr != htonl(INADDR_PFSYNC_GROUP)) | if (nvlist_exists_string(nvl, "syncdev")) | ||||
strlcpy(syncdev, nvlist_get_string(nvl, "syncdev"), | |||||
IFNAMSIZ); | |||||
if (nvlist_exists_number(nvl, "maxupdates")) | |||||
maxupdates = nvlist_get_number(nvl, "maxupdates"); | |||||
if (nvlist_exists_number(nvl, "flags")) | |||||
flags = nvlist_get_number(nvl, "flags"); | |||||
if (nvlist_exists_nvlist(nvl, "syncpeer")) { | |||||
pfsync_syncpeer_nvlist_to_sockaddr(nvlist_get_nvlist(nvl, | |||||
"syncpeer"), | |||||
&syncpeer); | |||||
} | |||||
Not Done Inline ActionsShould we be returning an error here? It's arguable, because we're checking the result of pfsync_syncpeer_nvlist_to_sockaddr(). On the other hand, the kernel really shouldn't be sending us invalid data. Perhaps this needs an assertion. kp: Should we be returning an error here?
It's arguable, because we're checking the result of… | |||||
Not Done Inline ActionsI looked around the ifconfig code and it seems they don't use assertions. I'm inclined towards returning an error. email_luiz.eng.br: I looked around the ifconfig code and it seems they don't use assertions. I'm inclined towards… | |||||
nvlist_destroy(nvl); | |||||
if (syncdev[0] != '\0' || syncpeer.ss_family != AF_UNSPEC) | |||||
printf("\t"); | printf("\t"); | ||||
if (preq.pfsyncr_syncdev[0] != '\0') | if (syncdev[0] != '\0') | ||||
printf("pfsync: syncdev: %s ", preq.pfsyncr_syncdev); | printf("syncdev: %s ", syncdev); | ||||
if (preq.pfsyncr_syncpeer.s_addr != htonl(INADDR_PFSYNC_GROUP)) | |||||
printf("syncpeer: %s ", inet_ntoa(preq.pfsyncr_syncpeer)); | |||||
if (preq.pfsyncr_syncdev[0] != '\0' || | if (syncpeer.ss_family == AF_INET && | ||||
preq.pfsyncr_syncpeer.s_addr != htonl(INADDR_PFSYNC_GROUP)) { | ((struct sockaddr_in *)&syncpeer)->sin_addr.s_addr != | ||||
printf("maxupd: %d ", preq.pfsyncr_maxupdates); | htonl(INADDR_PFSYNC_GROUP)) { | ||||
printf("defer: %s\n", | |||||
(preq.pfsyncr_defer & PFSYNCF_DEFER) ? "on" : "off"); | struct sockaddr *syncpeer_sa = | ||||
printf("\tsyncok: %d\n", | (struct sockaddr *)&syncpeer; | ||||
(preq.pfsyncr_defer & PFSYNCF_OK) ? 1 : 0); | if ((error = getnameinfo(syncpeer_sa, syncpeer_sa->sa_len, | ||||
syncpeer_str, sizeof(syncpeer_str), NULL, 0, | |||||
Done Inline ActionsWe leak the nvlist if the ioctl fails. kp: We leak the nvlist if the ioctl fails. | |||||
NI_NUMERICHOST)) != 0) | |||||
errx(1, "getnameinfo: %s", gai_strerror(error)); | |||||
printf("syncpeer: %s ", syncpeer_str); | |||||
} | } | ||||
printf("maxupd: %d ", maxupdates); | |||||
printf("defer: %s\n", (flags & PFSYNCF_DEFER) ? "on" : "off"); | |||||
printf("\tsyncok: %d\n", (flags & PFSYNCF_OK) ? 1 : 0); | |||||
} | } | ||||
static struct cmd pfsync_cmds[] = { | static struct cmd pfsync_cmds[] = { | ||||
DEF_CMD_ARG("syncdev", setpfsync_syncdev), | DEF_CMD_ARG("syncdev", setpfsync_syncdev), | ||||
DEF_CMD("-syncdev", 1, unsetpfsync_syncdev), | DEF_CMD("-syncdev", 1, unsetpfsync_syncdev), | ||||
DEF_CMD_ARG("syncif", setpfsync_syncdev), | DEF_CMD_ARG("syncif", setpfsync_syncdev), | ||||
DEF_CMD("-syncif", 1, unsetpfsync_syncdev), | DEF_CMD("-syncif", 1, unsetpfsync_syncdev), | ||||
DEF_CMD_ARG("syncpeer", setpfsync_syncpeer), | DEF_CMD_ARG("syncpeer", setpfsync_syncpeer), | ||||
Show All 20 Lines |
Not blocking, but it might be good to think about how to move this into libpfctl.