On Fri, Apr 15, 2016 at 03:17:00AM +0100, Dimitri John Ledkov wrote: > --- > > Changes since last version: > - Corrected Philipp's name > - Adjusted vlan_failed text message > - Adding a di_error if vlan_id is preseeded and not supported > - Added NetworkManager vlan type > - Sent templates for review
>From a patch point of view I'm mostly happy. But this should incorporate the newly suggested templates as well. Plus there are a few nits below. > +netcfg (1.139) UNRELEASED; urgency=medium > + > + * Add vlan support, based on YunQiang Su patch and Philipp Kern's > + rewrite, with own additions. Closes: #433568 Please capitalize VLAN and put Closes into parantheses. Suggested patch description: Add VLAN support. This is based on YunQiang Su patch and Philipp Kern's rewrite, with own additions. Closes: #433568 > + > + -- Dimitri John Ledkov <x...@ubuntu.com> Mon, 04 Apr 2016 12:18:08 +0100 > + > netcfg (1.138) unstable; urgency=medium > > [ Hendrik Brueckner ] > diff --git a/debian/netcfg-common.templates b/debian/netcfg-common.templates > index 2b77936..ebbfbb3 100644 > --- a/debian/netcfg-common.templates > +++ b/debian/netcfg-common.templates > @@ -26,6 +26,34 @@ _Description: Domain name: > If you are setting up a home network, you can make something up, but make > sure you use the same domain name on all your computers. > > +Template: netcfg/use_vlan > +Type: boolean > +Default: false > +# :sl6: > +_Description: Are you connected to 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 connected to a VLAN trunk port, > + specifying a VLAN ID may be necessary to get a working connection. > + > +Template: netcfg/vlan_id > +Type: string > +# :sl6: > +_Description: VLAN ID (1-4094): > + If your network interface is directly connected to a VLAN trunk port, > + specifying a VLAN ID may be necessary to get a working connection. > + > +Template: netcfg/vlan_failed > +Type: error > +# :sl6: > +_Description: Error setting up VLAN > + The command used to set up VLAN during the installation returned an > + error, please check installer logs, or go back and try another > + configuration. > + > Template: netcfg/get_nameservers > Type: string > # :sl1: > @@ -371,4 +399,3 @@ _Choices: ${essid_list} Enter ESSID manually > # :sl1: > _Description: Wireless network: > Select the wireless network to use during the installation process. > - > diff --git a/dhcp.c b/dhcp.c > index fe06950..9476bac 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -459,7 +459,7 @@ int netcfg_activate_dhcp (struct debconfclient *client, > struct netcfg_interface > kill_dhcp_client(); > loop_setup(); > > - interface_up(interface->name); > + netcfg_interface_up(interface); > > for (;;) { > di_debug("State is now %i", state); > diff --git a/netcfg-common.c b/netcfg-common.c > index c6d1d8d..ac2c6df 100644 > --- a/netcfg-common.c > +++ b/netcfg-common.c > @@ -267,7 +267,7 @@ int is_raw_80211(const char *iface) > > #if defined(__s390__) > // Layer 3 qeth on s390(x) cannot do arping to test gateway reachability. > -int is_layer3_qeth(const char *iface) > +int is_layer3_qeth(const struct netcfg_interface *interface) > { > const int bufsize = 1024; > int retval = 0; > @@ -277,6 +277,12 @@ int is_layer3_qeth(const char *iface) > ssize_t slen; > char* driver; > int fd; > + char* iface; > + > + if (interface->parentif) > + iface = interface->parentif; > + else > + iface = interface->name; > > // This is sufficient for both /driver and /layer2. > len = strlen(SYSCLASSNET) + strlen(iface) + strlen("/device/driver") + 1; > @@ -329,7 +335,7 @@ out: > return retval; > } > #else > -int is_layer3_qeth(const char *iface __attribute__((unused))) > +int is_layer3_qeth(const struct netcfg_interface *interface > __attribute__((unused))) > { > return 0; > } > @@ -1338,6 +1344,24 @@ void interface_down (const char *if_name) > } > } > > +void netcfg_interface_up (const struct netcfg_interface *iface) > +{ > + if (iface->parentif) > + interface_up (iface->parentif); > + > + if (iface->name) > + interface_up (iface->name); > +} > + > +void netcfg_interface_down (const struct netcfg_interface *iface) > +{ > + if (iface->name) > + interface_down (iface->name); > + > + if (iface->parentif) > + interface_down (iface->parentif); > +} > + > void parse_args (int argc, char ** argv) > { > if (argc == 2) { > @@ -1528,7 +1552,7 @@ int netcfg_detect_link(struct debconfclient *client, > const struct netcfg_interfa > if (ethtool_lite(if_name) == 1) /* ethtool-lite's CONNECTED */ { > di_info("Found link on %s", if_name); > > - if (!empty_str(gateway) && !is_wireless_iface(if_name) && > !is_layer3_qeth(if_name)) { > + if (!empty_str(gateway) && !is_wireless_iface(if_name) && > !is_layer3_qeth(interface)) { > for (count = 0; count < gw_tries; count++) { > if (di_exec_shell_log(arping) == 0) > break; > @@ -1564,6 +1588,8 @@ void netcfg_interface_init(struct netcfg_interface > *iface) > iface->v6_stateless_config = -1; > iface->loopback = -1; > iface->mode = MANAGED; > + iface->parentif = NULL; > + iface->vlanid = -1; > } > > /* Parse an IP address (v4 or v6), with optional CIDR netmask, into > diff --git a/netcfg.c b/netcfg.c > index 195681b..c918641 100644 > --- a/netcfg.c > +++ b/netcfg.c > @@ -67,6 +67,7 @@ int main(int argc, char *argv[]) > GET_METHOD, > GET_DHCP, > GET_STATIC, > + GET_VLAN, > WCONFIG, > WCONFIG_ESSID, > WCONFIG_SECURITY_TYPE, > @@ -216,13 +217,19 @@ int main(int argc, char *argv[]) > state = BACKUP; > else if (! interface.name || ! num_interfaces) > state = GET_HOSTNAME_ONLY; > - else { > - if (is_wireless_iface (interface.name)) > - state = WCONFIG; > - else > - state = GET_METHOD; > - } > + else if (is_wireless_iface (interface.name)) > + state = WCONFIG; > + else > + state = GET_VLAN; > + break; > + > + case GET_VLAN: > + if (netcfg_set_vlan(client, &interface) == GO_BACK) > + state = BACKUP; > + else > + state = GET_METHOD; > break; > + > case GET_HOSTNAME_ONLY: > if(netcfg_get_hostname(client, "netcfg/get_hostname", hostname, > 0)) > state = BACKUP; > @@ -231,6 +238,7 @@ int main(int argc, char *argv[]) > state = QUIT; > } > break; > + > case GET_METHOD: > if ((res = netcfg_get_method(client)) == GO_BACK) > state = (num_interfaces == 1) ? BACKUP : GET_INTERFACE; > diff --git a/netcfg.h b/netcfg.h > index 00a2cea..8b45776 100644 > --- a/netcfg.h > +++ b/netcfg.h > @@ -151,6 +151,11 @@ struct netcfg_interface { > /* WPA */ > wpa_t wpa_supplicant_status; > char *passphrase; > + > + /* VLAN */ > + char *parentif; > + int vlanid; > + > }; > > /* Somewhere we can store both in_addr and in6_addr; convenient for all those > @@ -221,6 +226,9 @@ extern void deconfigure_network(struct netcfg_interface > *iface); > extern void interface_up (const char *if_name); > extern void interface_down (const char *if_name); > > +extern void netcfg_interface_up (const struct netcfg_interface *iface); > +extern void netcfg_interface_down (const struct netcfg_interface *iface); > + > extern void loop_setup(void); > extern int get_hostname_from_dns(const struct netcfg_interface *interface, > char *hostname, const size_t max_hostname_len); > > @@ -270,4 +278,7 @@ extern void cleanup_dhcpv6_client(void); > extern int start_dhcpv6_client(struct debconfclient *client, const struct > netcfg_interface *interface); > extern int netcfg_autoconfig(struct debconfclient *client, struct > netcfg_interface *interface); > > +/* vlan.c */ > +extern int netcfg_set_vlan(struct debconfclient *client, struct > netcfg_interface *interface); > + > #endif /* _NETCFG_H_ */ > diff --git a/nm-conf.c b/nm-conf.c > index aeab031..66d549b 100644 > --- a/nm-conf.c > +++ b/nm-conf.c > @@ -32,11 +32,17 @@ static void get_uuid(char* target) > > static void nm_write_connection(FILE *config_file, nm_connection connection) > { > + static char *type = NM_DEFAULT_WIRED; > + if (connection.type == WIFI) { > + type = NM_DEFAULT_WIRELESS; > + } > + if (connection.type == VLAN) { > + type = NM_DEFAULT_VLAN; > + } > fprintf(config_file, "\n%s\n", NM_SETTINGS_CONNECTION); > fprintf(config_file, "id=%s\n", connection.id); > fprintf(config_file, "uuid=%s\n", connection.uuid); > - fprintf(config_file, "type=%s\n", (connection.type == WIFI) ? > - NM_DEFAULT_WIRELESS : NM_DEFAULT_WIRED); > + fprintf(config_file, "type=%s\n", type); > } > > #ifdef WIRELESS > @@ -71,6 +77,15 @@ static void nm_write_wired_specific_options(FILE > *config_file, > } > } > > +static void nm_write_vlan_specific_options(FILE *config_file, > + struct nm_config_info *nmconf) > +{ > + nm_vlan vlan = nmconf->vlan; > + fprintf(config_file, "\n%s\n", NM_SETTINGS_VLAN); > + fprintf(config_file, "parent=%s\n", vlan.parent); > + fprintf(config_file, "parent=%i\n", vlan.id); id=%d, no? Would be nice to have this tested once, if it actually produces a configuration that works. ;-) > +} > + > #ifdef WIRELESS > static void nm_write_wireless_security(FILE *config_file, > nm_wireless_security > wireless_security) > @@ -176,6 +191,9 @@ static void nm_write_connection_type(struct > nm_config_info nmconf) > if (nmconf.connection.type == WIFI) { > fprintf(f, "connection type: wireless\n"); > } > + else if (nmconf.connection.type == VLAN) { > + fprintf(f, "connection type: vlan\n"); > + } > else { > fprintf(f, "connection type: wired\n"); > } > @@ -229,6 +247,9 @@ void nm_write_configuration(struct nm_config_info nmconf) > if (nmconf.connection.type == WIRED) { > nm_write_wired_specific_options(config_file, &nmconf); > } > + else if (nmconf.connection.type == VLAN) { > + nm_write_vlan_specific_options(config_file, &nmconf); > + } > #ifdef WIRELESS > else { > nm_write_wireless_specific_options(config_file, &nmconf); > @@ -420,6 +441,15 @@ static void nm_get_ipv6(struct netcfg_interface *niface, > nm_ipvX *ipv6) > > } > > +static void nm_get_vlan(struct netcfg_interface *niface, nm_connection > *connection, nm_vlan *nmvlan) > +{ > + if (niface->vlanid > 0) { > + connection->type = VLAN; > + nmvlan->id = niface->vlanid; > + nmvlan->parent = niface->parentif; > + } > +} > + > /* Extract all configs for a wireless interface, from both global netcfg > * values and other resources. */ > #ifdef WIRELESS > @@ -444,6 +474,7 @@ static void nm_get_wired_config(struct netcfg_interface > *niface, struct nm_confi > nm_get_wired_specific_options(niface, &(nmconf->wired)); > nm_get_ipv4(niface, &(nmconf->ipv4)); > nm_get_ipv6(niface, &(nmconf->ipv6)); > + nm_get_vlan(niface, &(nmconf->connection), &(nmconf->vlan)); > } > > /* Getting configurations for NM relies on netcfrg global variables. */ Would this also need to write out wired_specific_options for the parentif? (The only one being the MAC address...) I'm not sure if it'd need to be a separate file then. Probably? |: Kind regards and thanks Philipp Kern