On 05/18/2012 12:38 PM, Sean Bruno wrote:
> ---
> src/Makefile.am | 1 +
> src/drv_fbsd.c | 135
> +++++++++++++++++++++++++++++++++++++++++++++++++----
> src/dutil_linux.c | 28 +++++++++++
> src/dutil_linux.h | 2 +
> 4 files changed, 156 insertions(+), 10 deletions(-)
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 484ba97..e431eff 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -60,6 +60,7 @@ endif
> if NETCF_DRIVER_FBSD
> DRIVER_SOURCES = \
> $(DRIVER_SOURCES_COMMON) \
> + $(DRIVER_SOURCES_LINUX) \
Hmm. If the "Linux" sources are useful on FreeBSD, maybe they need to be
relabelled :-)
What's the proper term for "Unix-like" that doesn't step on anyone's
trademark/ego? "STARNIX" ? "UNIXLIKE" ? Or should it be split into
dutil_augeas.* and dutil_libnl.*?
(later note - looks like FreeBSD doesn't actually have libnl, and you're
just re-purposing the libnl functions in dutil_linux, or maybe not using
most of them at all? In that case, we should just leave the
libnl-related functions in dutil_linux.*, put the augeas-related ones in
dutil_augeas.*, and the "generic *nix" functions in dutil_xnix.c (unless
someone has a suggestion for a better name :-) )
> $(DRIVER_SOURCES_FBSD)
> endif
>
> diff --git a/src/drv_fbsd.c b/src/drv_fbsd.c
> index 7fd3025..1b3bb87 100644
> --- a/src/drv_fbsd.c
> +++ b/src/drv_fbsd.c
> @@ -1,9 +1,34 @@
> /*
> + * Copyright (c) 2012, Sean Bruno sbr...@freebsd.org
> + * All rights reserved.
> +
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> met:
> + * Redistributions of source code must retain the above copyright
> + notice, this list of conditions and the following disclaimer.
> + * Redistributions in binary form must reproduce the above copyright
> + notice, this list of conditions and the following disclaimer in the
> + documentation and/or other materials provided with the distribution.
> + * Neither the name of the <organization> nor the
> + names of its contributors may be used to endorse or promote products
> + derived from this software without specific prior written permission.
> +
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS" AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT 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 SUCH DAMAGE.
> */
All copyrights in netcf must be compatible with the LGPL version 2.1
(see the file COPYING). This copyright isn't compatible with the LGPL,
and will need to change before the code can be committed.
>
> #include <config.h>
> #include <internal.h>
>
> +#include <augeas.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <spawn.h>
> @@ -17,27 +42,67 @@
> #include "dutil_fbsd.h"
>
> int drv_init(struct netcf *ncf) {
> +
> + if (ALLOC(ncf->driver) < 0)
> + return -1;
> return 0;
> +
> }
This is the third version of these functions. I would say just create
them with the final version to begin with.
>
>
> void drv_close(struct netcf *ncf) {
>
> - return;
> -
> -}
> + if (ncf == NULL || ncf->driver == NULL)
> + return;
>
> -
> -void build_adapter_table(struct netcf *ncf) {
> -
> - return;
> + FREE(ncf->driver);
>
> }
>
> static int list_interface_ids(struct netcf *ncf,
> int maxnames,
> char **names, unsigned int flags
> ATTRIBUTE_UNUSED) {
> + struct augeas *aug = NULL;
> + int nint = 0, nqualified = 0, result = 0;
> + char **intf = NULL;
> +
> + aug = get_augeas(ncf);
> + ERR_BAIL(ncf);
> + nint = list_interfaces(ncf, &intf);
> + ERR_BAIL(ncf);
> + if (!names) {
> + maxnames = nint; /* if not returning list, ignore maxnames too */
> + }
> + for (result = 0; (result < nint) && (nqualified < maxnames); result++) {
> + const char *name;
> + int is_qualified = ((flags &
> (NETCF_IFACE_ACTIVE|NETCF_IFACE_INACTIVE))
> + == (NETCF_IFACE_ACTIVE|NETCF_IFACE_INACTIVE));
> +
> + name = intf[result];
> +
> + if (!is_qualified) {
> + int is_active = if_is_active(ncf, name);
> + if ((is_active && (flags & NETCF_IFACE_ACTIVE))
> + || ((!is_active) && (flags & NETCF_IFACE_INACTIVE))) {
> +
> + is_qualified = 1;
> + }
> + }
> +
> + if (is_qualified) {
> + if (names) {
> + names[nqualified] = strdup(name);
> + ERR_NOMEM(names[nqualified] == NULL, ncf);
> + }
> + nqualified++;
> + }
> + }
> + free_matches(nint, &intf);
> + return nqualified;
> + error:
> + free_matches(nint, &intf);
> return -1;
> +
That function is *so* similar to the one in drv_redhat.c, I wonder if a
common function could be made to work for both, and put into
dutil_linux.c (or dutil_augeas.c or whatever other name is settled on).
> }
>
> int drv_list_interfaces(struct netcf *ncf,
> @@ -53,41 +118,75 @@ int drv_num_of_interfaces(struct netcf *ncf, unsigned
> int flags) {
>
>
> struct netcf_if *drv_lookup_by_name(struct netcf *ncf, const char *name) {
> + int result;
> +
> ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> +error:
> + return NULL;
> }
And now all of these other stub functions are changed again, into yet
another stub. Before sending another version of patches, it would be
really good to do some "git rebase -i"'ing to re-order and squash
together a lot of this stuff.
>
> const char *drv_mac_string(struct netcf_if *nif) {
> - ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> + ERR_THROW(1 == 1, nif->ncf, EOTHER, "not implemented on this platform");
> +error:
> + return NULL;
> }
>
> int drv_if_down(struct netcf_if *nif) {
> - ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> + int result = 0;
> +
> + ERR_THROW(1 == 1, nif->ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> int drv_if_up(struct netcf_if *nif) {
> - ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> + int result = 0;
> +
> + ERR_THROW(1 == 1, nif->ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
>
> struct netcf_if *drv_define(struct netcf *ncf, const char *xml_str
> ATTRIBUTE_UNUSED) {
> + int result = 0;
> +
> ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> int drv_undefine(struct netcf_if *nif) {
> + int result = 0;
> +
> ERR_THROW(1 == 1, nif->ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
>
> char *drv_xml_desc(struct netcf_if *nif) {
> + int result = 0;
> +
> ERR_THROW(1 == 1, nif->ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> char *drv_xml_state(struct netcf_if *nif) {
> + int result = 0;
> +
> ERR_THROW(1 == 1, nif->ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> int drv_if_status(struct netcf_if *nif, unsigned int *flags
> ATTRIBUTE_UNUSED) {
> + int result = 0;
> +
> ERR_THROW(1 == 1, nif->ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> int drv_lookup_by_mac_string(struct netcf *ncf,
> @@ -95,23 +194,39 @@ int drv_lookup_by_mac_string(struct netcf *ncf,
> int maxifaces ATTRIBUTE_UNUSED,
> struct netcf_if **ifaces ATTRIBUTE_UNUSED)
> {
> + int result = 0;
> +
> ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> int
> drv_change_begin(struct netcf *ncf, unsigned int flags ATTRIBUTE_UNUSED)
> {
> + int result = 0;
> +
> ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> int
> drv_change_rollback(struct netcf *ncf, unsigned int flags ATTRIBUTE_UNUSED)
> {
> + int result = 0;
> +
> ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
>
> int
> drv_change_commit(struct netcf *ncf, unsigned int flags ATTRIBUTE_UNUSED)
> {
> + int result = 0;
> +
> ERR_THROW(1 == 1, ncf, EOTHER, "not implemented on this platform");
> +error:
> + return result;
> }
> diff --git a/src/dutil_linux.c b/src/dutil_linux.c
> index 4e05c49..e36c8e3 100644
> --- a/src/dutil_linux.c
> +++ b/src/dutil_linux.c
> @@ -45,6 +45,10 @@
>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> +#ifdef __FreeBSD__
> +#include <net/if.h> // For struct ifreq
> +#include <sys/sockio.h> // For SIOCGIADDR
> +#endif
>
> #include "safe-alloc.h"
> #include "read-file.h"
> @@ -54,14 +58,18 @@
> #include "dutil.h"
> #include "dutil_linux.h"
>
> +#ifndef __FreeBSD__
> #include <net/if.h>
> #include <netlink/socket.h>
> #include <netlink/cache.h>
> #include <netlink/route/addr.h>
> #include <netlink/route/link.h>
> +#endif
Ah, wait - if freebsd doesn't have libnl, but you are using augeas, then
we definitely need to split the augeas-related utility functions out
into dutil_augeas.*, so you can include it without including all of the
(for FreeBSD) dead code of libnl utility functions. Then you can just
create your own dutil_freebsd.* which replicate the other functions from
dutil_linux that you wanted to use. This will be much cleaner than
having dutil_linux.c thickly slathered with #ifndef __FreeBSD__ directives.
>
> +#ifndef __FreeBSD__
> /* For some reason, the headers for libnl vlan functions aren't installed */
> extern int rtnl_link_vlan_get_id(struct rtnl_link *link);
> +#endif
>
> /*
> * Executing external programs
> @@ -736,9 +744,15 @@ int if_hwaddr(struct netcf *ncf, const char *intf,
> MEMZERO(&ifr, 1);
> strncpy(ifr.ifr_name, intf, sizeof(ifr.ifr_name));
> ifr.ifr_name[sizeof(ifr.ifr_name) - 1] = '\0';
> +#ifdef __FreeBSD__
> + ret = ioctl(ncf->driver->ioctl_fd, SIOCGIFADDR, &ifr);
> + memcpy(mac,ifr.ifr_addr.sa_data,6);
> + format_mac_addr(mac,buflen, (unsigned char *)ifr.ifr_addr.sa_data,6);
> +#else
> ret = ioctl(ncf->driver->ioctl_fd, SIOCGIFHWADDR, &ifr);
> memcpy(mac,ifr.ifr_hwaddr.sa_data,6);
> format_mac_addr(mac,buflen, (unsigned char *)ifr.ifr_hwaddr.sa_data,6);
> +#endif
> return ret;
> }
>
> @@ -794,6 +808,7 @@ done:
> }
>
>
> +#ifndef __FreeBSD__
> int netlink_init(struct netcf *ncf) {
>
> ncf->driver->nl_sock = nl_handle_alloc();
> @@ -842,6 +857,7 @@ int netlink_close(struct netcf *ncf) {
> }
> return 0;
> }
> +#endif
>
>
> static void add_type_specific_info(struct netcf *ncf,
> @@ -999,9 +1015,11 @@ static void add_ethernet_info(struct netcf *ncf,
> = { doc, root, NULL, ncf };
> struct rtnl_link *filter_link = NULL;
>
> +#ifndef __FreeBSD__
> /* if interface isn't currently available, nothing to add */
> if (ifindex == RTNL_LINK_NOT_FOUND)
> return;
> +#endif
>
> filter_link = rtnl_link_alloc();
> ERR_NOMEM(filter_link == NULL, ncf);
> @@ -1046,8 +1064,10 @@ static void add_vlan_info_cb(struct nl_object *obj,
> void *arg) {
> return;
>
> l_link = rtnl_link_get_link(iflink);
> +#ifndef __FreeBSD__
> if (l_link == RTNL_LINK_NOT_FOUND)
> return;
> +#endif
>
> master_link = rtnl_link_get(nl_object_get_cache(obj), l_link);
> if (master_link == NULL)
> @@ -1071,9 +1091,11 @@ static void add_vlan_info_cb(struct nl_object *obj,
> void *arg) {
>
> /* Add in type-specific info of master interface */
> master_ifindex = rtnl_link_name2i(ncf->driver->link_cache, master_name);
> +#ifndef __FreeBSD__
> ERR_THROW((master_ifindex == RTNL_LINK_NOT_FOUND), ncf, ENETLINK,
> "couldn't find ifindex for vlan master interface `%s`",
> master_name);
> +#endif
> add_type_specific_info(ncf, master_name, master_ifindex,
> cb_data->doc, interface_node);
>
> @@ -1088,9 +1110,11 @@ static void add_vlan_info(struct netcf *ncf,
> = { doc, root, NULL, ncf };
> struct rtnl_link *filter_link = NULL;
>
> +#ifndef __FreeBSD__
> /* if interface isn't currently available, nothing to add */
> if (ifindex == RTNL_LINK_NOT_FOUND)
> return;
> +#endif
>
> filter_link = rtnl_link_alloc();
> ERR_NOMEM(filter_link == NULL, ncf);
> @@ -1165,6 +1189,7 @@ static void add_bond_info_cb(struct nl_object *obj,
>
> xmlNodePtr interface_node;
>
> +#ifndef __FreeBSD__
> /* If this is a slave link, and the master is master_ifindex, add the
> * interface info to the bond.
> */
> @@ -1172,6 +1197,7 @@ static void add_bond_info_cb(struct nl_object *obj,
> if (!(rtnl_link_get_flags(iflink) & IFF_SLAVE)
> || rtnl_link_get_master(iflink) != cb_data->master_ifindex)
> return;
> +#endif
>
> cb_data->bond = xml_node(cb_data->doc, cb_data->root, "bond");
> ERR_NOMEM(cb_data->bond == NULL, ncf);
> @@ -1203,9 +1229,11 @@ static void add_bond_info(struct netcf *ncf,
> struct nl_bond_callback_data cb_data
> = { doc, root, NULL, ifindex, ncf };
>
> +#ifndef __FreeBSD__
> /* if interface isn't currently available, nothing to add */
> if (ifindex == RTNL_LINK_NOT_FOUND)
> return;
> +#endif
>
> nl_cache_foreach(ncf->driver->link_cache, add_bond_info_cb, &cb_data);
> }
> diff --git a/src/dutil_linux.h b/src/dutil_linux.h
> index 8b10e71..2aadbb4 100644
> --- a/src/dutil_linux.h
> +++ b/src/dutil_linux.h
> @@ -23,7 +23,9 @@
> #ifndef DUTIL_LINUX_H_
> #define DUTIL_LINUX_H_
>
> +#ifndef __FreeBSD__
> #include <netlink/netlink.h>
> +#endif
>
> struct driver {
> struct augeas *augeas;
_______________________________________________
netcf-devel mailing list
netcf-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/netcf-devel
