If only I had had a test environment to test my rewrite against, oh
well.
On Wed, Mar 30, 2016 at 05:18:11PM +0100, Dimitri John Ledkov wrote:
> +Template: netcfg/use_vlan
> +Type: boolean
> +Default: false
> +# :sl6:
> +_Description: Are you configuring on an IEEE 802.1Q VLAN trunk port?
> + Virtual LAN (VLAN) is a concept of partitioning a physical network to create
> + distinct broadcast domains. Packets can be marked for different IDs by
> + tagging, so that a single interconnect (trunk) may be used to transport
> + data for various VLANs.
> + .
> + If your network interface is directly attached to a VLAN trunk port,
> + specifying a VLAN ID may be necessary to get a working connection.
s/configuring on/connected to/? It'd be good to also get the template
changes reviewed.
> +Template: netcfg/vlan_id
> +Type: string
> +# :sl6:
> +_Description: VLAN ID (1-4094):
> + VLAN IDs are divided into a normal range and an extended range:
> + .
> + Normal range IDs are 1-1005. 1 is the default native VLAN,
> + and 1002-1005 are reserved for Token Ring and FDDI VLANs.
> + Extended range IDs are 1006-4094, which are designed for service
> + providers and have fewer options.
This description is probably not very useful.
> +Template: netcfg/vlan_cmderror
> +Type: error
> +# :sl6:
> +_Description: Error setting VLAN
> + The command used to set VLAN during installation got an error,
> + please go back and try again.
s/got/returned/ I guess.
> diff --git a/netcfg.c b/netcfg.c
> index 195681b..df0bc04 100644
> --- a/netcfg.c
> +++ b/netcfg.c
> @@ -222,6 +222,11 @@ int main(int argc, char *argv[])
> else
> state = GET_METHOD;
> }
> +
> + if(netcfg_set_vlan(&interface, client) == GO_BACK){
> + state = BACKUP;
> + }
> +
> break;
> case GET_HOSTNAME_ONLY:
> if(netcfg_get_hostname(client, "netcfg/get_hostname", hostname, 0))
Please adjust to the surrounding coding style (either no curly braces or
a space before it). I would also have preferred a GET_VLAN state in the
state machine.
> diff --git a/vlan.c b/vlan.c
> new file mode 100644
> index 0000000..ec2a19b
> --- /dev/null
> +++ b/vlan.c
> @@ -0,0 +1,67 @@
> +#include <stdio.h>
> +#include <cdebconf/debconfclient.h>
> +#include <debian-installer.h>
> +#include "netcfg.h"
> +
> +#define VLAN_SUCESSED 0
SUCESSED?
> +#define VLAN_FAILED 1
> +int netcfg_set_vlan(struct netcfg_interface *interface, struct debconfclient *client){
> + char *vlaniface = NULL, *vlanid = NULL, *vlancmd = NULL;
> + int vlaniface_len, vlancmd_len;
> +
> +/*kfreebsd or hurd has different cmd to set vlan*/
I would've prefered a get_vlan_command(const char* parentif, const char*
vlanif, int vlanid) here. The attached vlan.c (untested) has it.
> +#if defined(__linux__)
> + char vlancmd_template[] = "ip link add link %s name %s type vlan id %s";
> +#elif defined(__FreeBSD_kernel__)
> + /*export 2 more to make it the same formt with Linux*/
> + char vlancmd_tmplate[] = "export NIC=%s; export VNIC=%s; export VLANID=%s;"
> + " ifconfig $VNIC create";
> +#endif
> + int vlancmd_template_len = sizeof(vlancmd_template);
> +
> + debconf_input(client, "medium", "netcfg/use_vlan");
> +
> + if (debconf_go(client) == CMD_GOBACK)
> + return GO_BACK;
> + debconf_get(client, "netcfg/use_vlan");
> +
> + if (!strcmp(client->value, "false")){
> + goto error;
> + }
> +
> + debconf_input(client, "critical", "netcfg/vlan_id");
> + debconf_get(client, "netcfg/vlan_id");
> + vlanid = client -> value;
> +
> + vlaniface_len = strlen(interface->name)+strlen(vlanid)+2;
> + vlaniface = malloc(vlaniface_len);
> + if(! vlaniface){
> + goto error;
> + }
> + snprintf(vlaniface, vlaniface_len, "%s.%s", interface->name, vlanid);
> + vlancmd_len = vlancmd_template_len + vlaniface_len*2 +1;
> + vlancmd = malloc(vlancmd_len);
> + if(! vlancmd){
> + goto error;
> + }
> + snprintf(vlancmd, vlancmd_len, vlancmd_template, interface->name, vlaniface, vlanid);
> + if(di_exec_shell_log(vlancmd)){
> + di_warning("^ Setting VLAN error: the command is \n%s", vlancmd);
> + debconf_capb(client);
> + debconf_input(client, "critical", "netcfg/vlan_cmderror");
> + debconf_go(client);
> + debconf_capb(client, "backup");
> + goto error;
> + }
> + if(interface->name){
> + free(interface->name);
> + interface->name = vlaniface;
> + }
> + free(vlancmd);
> + return VLAN_SUCESSED;
> +
> +error:
> + if(vlaniface) free(vlaniface);
> + if(vlancmd) free(vlancmd);
> + return VLAN_FAILED;
> +}
Please convert this down into a consistent coding style. (Or maybe look
at the attached file. It has been a long time ago so maybe I didn't make
much sense of it either.)
> diff --git a/write_interface.c b/write_interface.c
> index 2ab1a34..be0d78b 100644
> --- a/write_interface.c
> +++ b/write_interface.c
> @@ -44,6 +44,21 @@ static int nc_wi_loopback(const struct netcfg_interface *interface, FILE *fd)
> return 1;
> }
>
> +/* Write VLAN settings, such as: vlan_raw_device eth0
> +*/
> +static int nc_wi_vlan(const struct netcfg_interface *interface, FILE *fd)
> +{
> + char *dup_name, *strip_name;
> + dup_name = strdup(interface->name);
> + strip_name = strsep(&dup_name, ".");
> + if(strip_name != NULL){
> + fprintf(fd, "\tvlan_raw_device %s\n", strip_name);
> + }
> + free(dup_name);
> + return 1;
> +}
As you already improved in the third patch the device should get its own
struct member.
> static int nc_wi_wireless_options(const struct netcfg_interface *interface, FILE *fd)
> {
> /*
> @@ -271,7 +286,10 @@ int netcfg_write_interface(const struct netcfg_interface *interface)
> di_debug("Writing static IPv6 stanza for %s", interface->name);
> rv = nc_wi_static_ipv6(interface, fd);
> }
> -
> + if (rv && strchr(interface->name, '.')){
> + di_debug("Writing VLAN: %s", interface->name);
> + rv = nc_wi_vlan(interface, fd);
> + }
> if (rv && interface && is_wireless_iface(interface->name)) {
> di_debug("Writing wireless options for %s", interface->name);
> rv = nc_wi_wireless_options(interface, fd);
Kind regards
Philipp Kern
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <cdebconf/debconfclient.h>
#include <debian-installer.h>
#include "netcfg.h"
static char* get_vlan_command(const char* parentif, const char* vlanif, int vlanid) {
#if defined(__linux__)
const char* vlan_command = "ip link add link %s name %s type vlan id %d";
int len = strlen(vlan_command) + strlen(parentif) + strlen(vlanif) + 4 + 1;
char* buf = malloc(len);
snprintf(buf, len, vlan_command, parentif, vlanif, vlanid);
return buf;
#elif defined(__FreeBSD_kernel__)
const char* vlan_command = "ifconfig %s vlan %d vlandev %s";
int len = strlen(vlan_command) + strlen(parentif) + strlen(vlanif) + 4 + 1;
char* buf = malloc(len);
snprintf(buf, len, vlan_command, vlanif, vlanid, parentif);
return buf;
#endif
}
/* Create a new VLAN interface attached to the currently selected
* network interface.
*/
int netcfg_set_vlan (struct debconfclient *client, struct netcfg_interface *interface) {
#if defined(__linux__) || defined(__FreeBSD_kernel__)
int vlanid;
for (;;) {
debconf_get(client, "netcfg/vlan_id");
if (strlen(client->value) == 0) {
/* Empty string: no VLAN to configure. */
return 0;
}
vlanid = strtol(client->value, NULL, 10);
/* Valid range: 1-4094 (0 and 4095 are reserved.)
* 0 will be returned by strtol if the value cannot be parsed.
*/
if (vlanid < 1 || vlanid > 4094) {
di_error("VLAN ID \"%s\" is invalid.", client->value);
debconf_subst(client, "netcfg/vlan_id_invalid", "vlan_id", client->value);
debconf_input(client, "critical", "netcfg/invalid_vlan");
debconf_capb(client);
debconf_go(client);
debconf_capb(client, "backup");
debconf_input(client, "critical", "netcfg/vlan_id");
if (debconf_go(client) == GO_BACK) {
return GO_BACK;
}
} else {
break;
}
}
int vlaniflen = strlen(interface->name) + 1 + 4 + 1;
char* vlanif = malloc(vlaniflen);
snprintf(vlanif, vlaniflen, "%s.%d", interface->name, vlanid);
char *vlan_command = get_vlan_command(interface->name, vlanif, vlanid);
int rc = di_exec_shell_log(vlan_command);
if (rc != 0) {
di_error("\"%s\" failed to create VLAN interface; return code = %d", vlan_command, rc);
free(vlan_command);
debconf_input(client, "critical", "netcfg/vlan_failed");
debconf_go(client);
return GO_BACK;
}
free(vlan_command);
interface->parentif = interface->name;
interface->name = vlanif;
interface->vlanid = vlanid;
#else
/* This platform does not support VLANs. */
#endif
return 0;
}
Attachment:
signature.asc
Description: Digital signature