[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#433568: [PATCH] Add vlan support, based on YunQiang Su patch and Philipp Kern's rewrite, with own additions. Closes: #433568



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 <xnox@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


Reply to: