On Wed, 2011-11-16 at 16:21 +0800, Weiping Pan wrote: > On 11/16/2011 01:35 PM, Dan Williams wrote: > > On Fri, 2011-10-21 at 09:52 +0800, Weiping Pan wrote: > >> The example of ifcfg-vlan is as followed: > >> > >> VLAN=yes > >> TYPE=vlan > >> DEVICE=vlan43 > >> PHYSDEV=eth9 > > Ideally here we also accept a MAC address or NM connection UUID as > > PHYSDEV so you don't have to manually keep all the interface names in > > sync. > > > Yes. I will put it in TODO list. > >> REORDER_HDR=0 > >> VLAN_FLAGS=GVRP,LOOSE_BINDING > >> VLAN_INGRESS_PRIORITY_MAP=0:1,2:5 > >> VLAN_EGRESS_PRIORITY_MAP=12:3,14:7 > > Are these new variables? ifup (F16) seems to think the only ones that > > exist are VLAN (yes/no), REORDER_HDR, and GVRP. > > > But vconfig supports ingress/egress priority mapping, > so I think NetworkManager should support them, too.
Yes, no objection to the support, I was just curious about it as I couldn't find anywhere in the existing ifcfg scripts that this value was defined. So having NM define that as an ifcfg variable is fine. > >> ONBOOT=yes > >> BOOTPROTO=static > >> IPADDR=192.168.43.149 > >> NETMASK=255.255.255.0 > >> > >> And we try to make it compitable with the format used by initscripts, > >> and there is no need to change anything in ifcfg-eth9. > >> > >> Pay attention the format of DEVICE here is 'vlan+id', and id is 0-4095. > >> > >> Signed-off-by: Weiping Pan<[email protected]> > >> --- > >> libnm-util/Makefile.am | 2 + > >> libnm-util/libnm-util.ver | 11 + > >> libnm-util/nm-connection.c | 25 ++- > >> libnm-util/nm-connection.h | 2 + > >> libnm-util/nm-setting-vlan.c | 351 > >> ++++++++++++++++++++ > >> libnm-util/nm-setting-vlan.h | 93 +++++ > >> src/settings/plugins/ifcfg-rh/common.h | 1 + > >> src/settings/plugins/ifcfg-rh/reader.c | 143 ++++++++- > >> .../network-scripts/ifcfg-test-vlan-interface | 11 +- > >> 9 files changed, 622 insertions(+), 17 deletions(-) > >> create mode 100644 libnm-util/nm-setting-vlan.c > >> create mode 100644 libnm-util/nm-setting-vlan.h > >> > >> diff --git a/libnm-util/Makefile.am b/libnm-util/Makefile.am > >> index 1529b4a..a6cf684 100644 > >> --- a/libnm-util/Makefile.am > >> +++ b/libnm-util/Makefile.am > >> @@ -17,6 +17,7 @@ libnm_util_include_HEADERS = \ > >> nm-setting-bluetooth.h \ > >> nm-setting-connection.h \ > >> nm-setting-ip4-config.h \ > >> + nm-setting-vlan.h \ > >> nm-setting-ip6-config.h \ > >> nm-setting-ppp.h \ > >> nm-setting-pppoe.h \ > >> @@ -46,6 +47,7 @@ libnm_util_la_csources = \ > >> nm-setting-bluetooth.c \ > >> nm-setting-connection.c \ > >> nm-setting-ip4-config.c \ > >> + nm-setting-vlan.c \ > >> nm-setting-ip6-config.c \ > >> nm-setting-ppp.c \ > >> nm-setting-pppoe.c \ > >> diff --git a/libnm-util/libnm-util.ver b/libnm-util/libnm-util.ver > >> index 53c2482..9f3033e 100644 > >> --- a/libnm-util/libnm-util.ver > >> +++ b/libnm-util/libnm-util.ver > >> @@ -30,6 +30,7 @@ global: > >> nm_connection_get_setting_wired; > >> nm_connection_get_setting_wireless; > >> nm_connection_get_setting_wireless_security; > >> + nm_connection_get_setting_vlan; > >> nm_connection_get_type; > >> nm_connection_get_uuid; > >> nm_connection_is_type; > >> @@ -449,6 +450,16 @@ global: > >> nm_utils_wifi_find_next_channel; > >> nm_utils_wifi_freq_to_channel; > >> nm_utils_wifi_is_channel_valid; > >> + nm_setting_vlan_error_get_type; > >> + nm_setting_vlan_error_quark; > >> + nm_setting_vlan_get_type; > >> + nm_setting_vlan_new; > >> + nm_setting_vlan_get_interface_name; > >> + nm_setting_vlan_get_vlan_slave; > >> + nm_setting_vlan_get_vlan_id; > >> + nm_setting_vlan_get_vlan_flags; > >> + nm_setting_vlan_get_vlan_priority_ingress_map; > >> + nm_setting_vlan_get_vlan_priority_egress_map; > > We probably want to kill the second 'vlan' here since these functions > > are already part of the vlan setting, we can just name them: > > > > nm_setting_vlan_get_slave; > > nm_setting_vlan_get_id; > > nm_setting_vlan_get_flags; > > nm_setting_vlan_get_priority_ingress_map; > > nm_setting_vlan_get_priority_egress_map; > > > Ok. I will remove the duplicate "vlan". > > >> local: > >> *; > >> }; > >> diff --git a/libnm-util/nm-connection.c b/libnm-util/nm-connection.c > >> index e3bbeae..2137352 100644 > >> --- a/libnm-util/nm-connection.c > >> +++ b/libnm-util/nm-connection.c > >> @@ -44,6 +44,7 @@ > >> #include "nm-setting-wireless-security.h" > >> #include "nm-setting-vpn.h" > >> #include "nm-setting-olpc-mesh.h" > >> +#include "nm-setting-vlan.h" > >> > >> #include "nm-setting-serial.h" > >> #include "nm-setting-gsm.h" > >> @@ -135,7 +136,7 @@ static guint signals[LAST_SIGNAL] = { 0 }; > >> > >> static GHashTable *registered_settings = NULL; > >> > >> -#define DEFAULT_MAP_SIZE 16 > >> +#define DEFAULT_MAP_SIZE 17 > >> > >> static struct SettingInfo { > >> const char *name; > >> @@ -242,6 +243,11 @@ register_default_settings (void) > >> NM_SETTING_WIMAX_ERROR, > >> 1, TRUE); > >> > >> + register_one_setting (NM_SETTING_VLAN_SETTING_NAME, > >> + NM_TYPE_SETTING_VLAN, > >> + NM_SETTING_VLAN_ERROR, > >> + 1, TRUE); > >> + > >> register_one_setting (NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, > >> NM_TYPE_SETTING_WIRELESS_SECURITY, > >> NM_SETTING_WIRELESS_SECURITY_ERROR, > >> @@ -1543,6 +1549,23 @@ nm_connection_get_setting_wireless_security > >> (NMConnection *connection) > >> return (NMSettingWirelessSecurity *) nm_connection_get_setting > >> (connection, NM_TYPE_SETTING_WIRELESS_SECURITY); > >> } > >> > >> +/** > >> + * nm_connection_get_setting_vlan: > >> + * @connection: the #NMConnection > >> + * > >> + * A shortcut to return any #NMSettingVLAN the connection might contain. > >> + * > >> + * Returns: (transfer none): an #NMSettingVLAN if the connection contains > >> one, otherwise NULL > >> + **/ > >> +NMSettingVLAN * > >> +nm_connection_get_setting_vlan (NMConnection *connection) > >> +{ > >> + g_return_val_if_fail (connection != NULL, NULL); > >> + g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); > >> + > >> + return (NMSettingVLAN *) nm_connection_get_setting (connection, > >> NM_TYPE_SETTING_VLAN); > >> +} > >> + > >> /*************************************************************/ > >> > >> static void > >> diff --git a/libnm-util/nm-connection.h b/libnm-util/nm-connection.h > >> index 069dc84..e573b8c 100644 > >> --- a/libnm-util/nm-connection.h > >> +++ b/libnm-util/nm-connection.h > >> @@ -45,6 +45,7 @@ > >> #include<nm-setting-wired.h> > >> #include<nm-setting-wireless.h> > >> #include<nm-setting-wireless-security.h> > >> +#include<nm-setting-vlan.h> > >> > >> G_BEGIN_DECLS > >> > >> @@ -196,6 +197,7 @@ NMSettingWimax * > >> nm_connection_get_setting_wimax (NMConnec > >> NMSettingWired * nm_connection_get_setting_wired > >> (NMConnection *connection); > >> NMSettingWireless * nm_connection_get_setting_wireless > >> (NMConnection *connection); > >> NMSettingWirelessSecurity *nm_connection_get_setting_wireless_security > >> (NMConnection *connection); > >> +NMSettingVLAN * nm_connection_get_setting_vlan > >> (NMConnection *connection); > >> > >> G_END_DECLS > >> > >> diff --git a/libnm-util/nm-setting-vlan.c b/libnm-util/nm-setting-vlan.c > >> new file mode 100644 > >> index 0000000..7f104b3 > >> --- /dev/null > >> +++ b/libnm-util/nm-setting-vlan.c > >> @@ -0,0 +1,351 @@ > >> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- > >> */ > >> + > >> +/* > >> + * Weiping Pan<[email protected]> > >> + * > >> + * This library is free software; you can redistribute it and/or > >> + * modify it under the terms of the GNU Lesser General Public > >> + * License as published by the Free Software Foundation; either > >> + * version 2 of the License, or (at your option) any later version. > >> + * > >> + * This library is distributed in the hope that it will be useful, > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > >> + * Lesser General Public License for more details. > >> + * > >> + * You should have received a copy of the GNU Lesser General Public > >> + * License along with this library; if not, write to the > >> + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, > >> + * Boston, MA 02110-1301 USA. > >> + * > >> + * (C) Copyright 2011 Red Hat, Inc. > >> + */ > >> +#include<stdio.h> > >> +#include<string.h> > >> + > >> +#include<dbus/dbus-glib.h> > >> +#include "nm-setting-vlan.h" > >> +#include "nm-param-spec-specialized.h" > >> +#include "nm-utils.h" > >> +#include "nm-dbus-glib-types.h" > >> + > >> +/** > >> + * SECTION:nm-setting-vlan > >> + * @short_description: Describes connection properties for VLAN devices > >> + * @include: nm-setting-vlan.h > >> + * > >> + * The #NMSettingVLAN object is a #NMSetting subclass that describes > >> properties > >> + * necessary for connection to VLAN devices. > >> + **/ > >> + > >> +/** > >> + * nm_setting_vlan_error_quark: > >> + * Registers an error quark for #NMSettingVLAN if necessary. > >> + * Returns: the error quark used for #NMSettingVLAN errors. > >> + **/ > >> +GQuark > >> +nm_setting_vlan_error_quark (void) > >> +{ > >> + static GQuark quark; > >> + > >> + if (G_UNLIKELY (!quark)) > >> + quark = g_quark_from_static_string > >> ("nm-setting-vlan-error-quark"); > >> + return quark; > >> +} > >> + > >> +/* This should really be standard. */ > >> +#define ENUM_ENTRY(NAME, DESC) { NAME, "" #NAME "", DESC } > >> + > >> +GType > >> +nm_setting_vlan_error_get_type (void) > >> +{ > >> + static GType etype = 0; > >> + > >> + if (etype == 0) { > >> + static const GEnumValue values[] = { > >> + /* Unknown error. */ > >> + ENUM_ENTRY (NM_SETTING_VLAN_ERROR_UNKNOWN, > >> "UnknownError"), > >> + /* The specified property was invalid. */ > >> + ENUM_ENTRY (NM_SETTING_VLAN_ERROR_INVALID_PROPERTY, > >> "InvalidProperty"), > >> + /* The specified property was missing and is required. > >> */ > >> + ENUM_ENTRY (NM_SETTING_VLAN_ERROR_MISSING_PROPERTY, > >> "MissingProperty"), > >> + { 0, 0, 0 } > >> + }; > >> + etype = g_enum_register_static ("NMSettingVLANError", values); > >> + } > >> + return etype; > >> +} > >> + > >> +G_DEFINE_TYPE (NMSettingVLAN, nm_setting_vlan, NM_TYPE_SETTING) > > Lets actually use NMSettingVlan here instead of VLAN to keep consistency > > with others like NMSettingWimax, NMSettingWifi, etc. I made a mistake > > with NMSettingVPN... > Ok, I will use "vlan" and "XXXVlan" to keep consistency in the future. > >> +#define NM_SETTING_VLAN_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), > >> NM_TYPE_SETTING_VLAN, NMSettingVLANPrivate)) > >> + > >> +typedef struct { > >> + char *interface_name; > >> + char *vlan_slave; > >> + guint32 vlan_id; > >> + guint32 vlan_flags; > >> + char *vlan_priority_ingress_map; > >> + char *vlan_priority_egress_map; > > I'll comment on this later, but we probably want to store these > > differently; instead of a string, if the format will always be > > "<uint>:<uint>" it might make sense to store them that way so that at > > least we can manipulate them as tuples rather than having to parse > > strings in the future. Will the ingress/egress mappings always be > > <uint>:<uint> or are there other possibilities? > The ingress/egress mappings will always be <uint>:<uint>. Ok, great. Makes our life simpler. > >> +} NMSettingVLANPrivate; > >> + > >> +enum { > >> + PROP_0, > >> + PROP_VLAN_DEVICE, > >> + PROP_VLAN_SLAVE, > >> + PROP_VLAN_ID, > >> + PROP_VLAN_FLAGS, > >> + PROP_VLAN_NAME_TYPE, > >> + PROP_VLAN_PRIORITY_INGRESS_MAP, > >> + PROP_VLAN_PRIORITY_EGRESS_MAP, > >> + LAST_PROP > >> +}; > >> + > >> +/** > >> + * nm_setting_vlan_new: > >> + * > >> + * Creates a new #NMSettingVLAN object with default values. > >> + * > >> + * Returns: (transfer full): the new empty #NMSettingVLAN object > >> + **/ > >> +NMSetting * > >> +nm_setting_vlan_new (void) > >> +{ > >> + return (NMSetting *)g_object_new(NM_TYPE_SETTING_VLAN, NULL); > >> +} > >> + > >> +/** > >> + * nm_setting_vlan_get_interface_name: > >> + * @setting: the #NMSettingVLAN > >> + * > >> + * Returns: the #NMSettingVLAN:interface_name property of the setting > >> + **/ > >> +const char * > >> +nm_setting_vlan_get_interface_name(NMSettingVLAN *setting) > >> +{ > >> + g_return_val_if_fail (NM_IS_SETTING_VLAN(setting), NULL); > >> + return NM_SETTING_VLAN_GET_PRIVATE(setting)->interface_name; > >> +} > >> + > >> +/** > >> + * nm_setting_vlan_get_vlan_slave: > >> + * @setting: the #NMSettingVLAN > >> + * > >> + * Returns: the #NMSettingVLAN:vlan_slave property of the setting > >> + **/ > >> +const char * > >> +nm_setting_vlan_get_vlan_slave(NMSettingVLAN *setting) > >> +{ > >> + g_return_val_if_fail (NM_IS_SETTING_VLAN(setting), NULL); > >> + return NM_SETTING_VLAN_GET_PRIVATE(setting)->vlan_slave; > >> +} > >> + > >> +/** > >> + * nm_setting_vlan_get_vlan_id: > >> + * @setting: the #NMSettingVLAN > >> + * > >> + * Returns: the #NMSettingVLAN:vlan_id property of the setting > >> + **/ > >> +guint32 > >> +nm_setting_vlan_get_vlan_id(NMSettingVLAN *setting) > >> +{ > >> + g_return_val_if_fail (NM_IS_SETTING_VLAN(setting), 0); > >> + return NM_SETTING_VLAN_GET_PRIVATE(setting)->vlan_id; > >> +} > >> + > >> +/** > >> + * nm_setting_vlan_get_vlan_flags: > >> + * @setting: the #NMSettingVLAN > >> + * > >> + * Returns: the #NMSettingVLAN:vlan_flags property of the setting > >> + **/ > >> +guint32 > >> +nm_setting_vlan_get_vlan_flags(NMSettingVLAN *setting) > >> +{ > >> + g_return_val_if_fail (NM_IS_SETTING_VLAN(setting), 0); > >> + return NM_SETTING_VLAN_GET_PRIVATE(setting)->vlan_flags; > >> +} > >> + > >> +/** > >> + * nm_setting_vlan_get_vlan_priority_ingress_map: > >> + * @setting: the #NMSettingVLAN > >> + * > >> + * Returns: the #NMSettingVLAN:vlan_priority_ingress_map property of the > >> setting > >> + **/ > >> +const char * > >> +nm_setting_vlan_get_vlan_priority_ingress_map(NMSettingVLAN *setting) > >> +{ > >> + g_return_val_if_fail (NM_IS_SETTING_VLAN(setting), NULL); > >> + return NM_SETTING_VLAN_GET_PRIVATE(setting)->vlan_priority_ingress_map; > >> +} > >> + > >> +/** > >> + * nm_setting_vlan_get_vlan_priority_egress_map: > >> + * @setting: the #NMSettingVLAN > >> + * > >> + * Returns: the #NMSettingVLAN:vlan_priority_egress_map property of the > >> setting > >> + **/ > >> +const char * > >> +nm_setting_vlan_get_vlan_priority_egress_map(NMSettingVLAN *setting) > >> +{ > >> + g_return_val_if_fail (NM_IS_SETTING_VLAN(setting), NULL); > >> + return NM_SETTING_VLAN_GET_PRIVATE(setting)->vlan_priority_egress_map; > >> +} > >> + > >> +static void > >> +nm_setting_vlan_init (NMSettingVLAN *setting) > >> +{ > >> + NMSettingVLANPrivate *priv = NULL; > >> + > >> + priv = NM_SETTING_VLAN_GET_PRIVATE(setting); > >> + priv->vlan_id = 0; > >> + priv->vlan_flags = 0; > >> + > >> + priv->vlan_priority_ingress_map = NULL; > > The private structure actually gets memset() to 0, so we don't need to > > explicitly initialize anything. Properties can also get explicitly set > > to their default value during construction if you pass G_PARAM_CONSTRUCT > > as one of the flags when calling g_param_spec_xxxx(). > > > Ok. > >> + > >> + priv->vlan_priority_egress_map = NULL; > >> + g_object_set (setting, NM_SETTING_NAME, NM_SETTING_VLAN_SETTING_NAME, > >> NULL); > >> + > >> + return; > >> +} > >> + > >> +static void > >> +finalize (GObject *object) > >> +{ > >> + NMSettingVLAN *setting = NM_SETTING_VLAN (object); > >> + NMSettingVLANPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting); > >> + > >> + g_free(priv->interface_name); > >> + g_free(priv->vlan_slave); > >> + g_free(priv->vlan_priority_ingress_map); > >> + g_free(priv->vlan_priority_egress_map); > >> + return; > >> +} > >> + > >> +static void > >> +set_property (GObject *object, guint prop_id, > >> + const GValue *value, GParamSpec *pspec) > >> +{ > >> + NMSettingVLAN *setting = NM_SETTING_VLAN (object); > >> + NMSettingVLANPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting); > >> + > >> + switch (prop_id) { > >> + case PROP_VLAN_DEVICE: > >> + g_free (priv->interface_name); > >> + priv->interface_name = g_value_dup_string (value); > >> + break; > >> + case PROP_VLAN_SLAVE: > >> + g_free (priv->vlan_slave); > >> + priv->vlan_slave = g_value_dup_string (value); > >> + break; > >> + case PROP_VLAN_ID: > >> + priv->vlan_id = g_value_get_uint(value); > >> + break; > >> + case PROP_VLAN_FLAGS: > >> + priv->vlan_flags |= g_value_get_uint(value); > > You probably just want "priv->vlan_flags = xxx" here instead of OR-ing > > with the existing value. > > > Ok. > >> + break; > >> + case PROP_VLAN_PRIORITY_INGRESS_MAP: > >> + priv->vlan_priority_ingress_map = g_value_dup_string(value); > >> + break; > >> + case PROP_VLAN_PRIORITY_EGRESS_MAP: > >> + priv->vlan_priority_egress_map = g_value_dup_string(value); > >> + break; > >> + default: > >> + break; > >> + } > >> +} > >> + > >> +static void > >> +get_property (GObject *object, guint prop_id, > >> + GValue *value, GParamSpec *pspec) > >> +{ > >> + NMSettingVLAN *setting = NM_SETTING_VLAN (object); > >> + > >> + switch (prop_id) { > >> + case PROP_VLAN_DEVICE: > >> + g_value_set_string(value, > >> nm_setting_vlan_get_interface_name(setting)); > >> + break; > >> + case PROP_VLAN_SLAVE: > >> + g_value_set_string(value, > >> nm_setting_vlan_get_vlan_slave(setting)); > >> + break; > >> + case PROP_VLAN_ID: > >> + g_value_set_uint(value, nm_setting_vlan_get_vlan_id(setting)); > >> + break; > >> + case PROP_VLAN_FLAGS: > >> + g_value_set_uint(value, > >> nm_setting_vlan_get_vlan_flags(setting)); > >> + break; > >> + case PROP_VLAN_PRIORITY_INGRESS_MAP: > >> + g_value_set_string(value, > >> nm_setting_vlan_get_vlan_priority_ingress_map(setting)); > >> + break; > >> + case PROP_VLAN_PRIORITY_EGRESS_MAP: > >> + g_value_set_string(value, > >> nm_setting_vlan_get_vlan_priority_egress_map(setting)); > >> + break; > >> + default: > >> + break; > >> + } > >> +} > >> + > >> +static void > >> +nm_setting_vlan_class_init (NMSettingVLANClass *setting_class) > >> +{ > >> + GObjectClass *object_class = G_OBJECT_CLASS (setting_class); > >> + g_type_class_add_private (setting_class, sizeof (NMSettingVLANPrivate)); > >> + > >> + /* virtual methods */ > >> + object_class->set_property = set_property; > >> + object_class->get_property = get_property; > >> + object_class->finalize = finalize; > >> + > >> + /* Properties */ > >> + g_object_class_install_property > >> + (object_class, PROP_VLAN_DEVICE, > >> + g_param_spec_string( NM_SETTING_VLAN_INTERFACE_NAME, > >> + "InterfaceName", > >> + "The vlan device name in kernel", > >> + NULL, > >> + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | > >> NM_SETTING_PARAM_SERIALIZE)); > >> + > >> + g_object_class_install_property > >> + (object_class, PROP_VLAN_SLAVE, > >> + g_param_spec_string( NM_SETTING_VLAN_VLAN_SLAVE, > >> + "vlan slave", > >> + "The underlying physical ethernet > >> device", > >> + NULL, > >> + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | > >> NM_SETTING_PARAM_SERIALIZE)); > >> + > >> + g_object_class_install_property > >> + (object_class, PROP_VLAN_ID, > >> + g_param_spec_uint (NM_SETTING_VLAN_VLAN_ID, > >> + "vlan id", > >> + "vlan id", > >> + 0, > >> + G_MAXUINT32, > >> + 0, > >> + G_PARAM_READWRITE | > >> G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE)); > >> + > >> + g_object_class_install_property > >> + (object_class, PROP_VLAN_FLAGS, > >> + g_param_spec_uint (NM_SETTING_VLAN_VLAN_FLAGS, > >> + "vlan flags", > >> + "vlan flags", > >> + 0, > >> + G_MAXUINT32, > >> + 0, > >> + G_PARAM_READWRITE | > >> G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE)); > >> + > >> + g_object_class_install_property > >> + (object_class, PROP_VLAN_PRIORITY_INGRESS_MAP, > >> + g_param_spec_string( > >> NM_SETTING_VLAN_VLAN_INGRESS_PRIORITY_MAP, > >> + "vlan ingress priority mapping", > >> + "vlan ingress priority mapping", > >> + NULL, > >> + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | > >> NM_SETTING_PARAM_SERIALIZE)); > >> + > >> + g_object_class_install_property > >> + (object_class, PROP_VLAN_PRIORITY_EGRESS_MAP, > >> + g_param_spec_string( > >> NM_SETTING_VLAN_VLAN_EGRESS_PRIORITY_MAP, > >> + "vlan egress priority mapping", > >> + "vlan egress priority mapping", > >> + NULL, > >> + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | > >> NM_SETTING_PARAM_SERIALIZE)); > >> +} > >> diff --git a/libnm-util/nm-setting-vlan.h b/libnm-util/nm-setting-vlan.h > >> new file mode 100644 > >> index 0000000..c354db8 > >> --- /dev/null > >> +++ b/libnm-util/nm-setting-vlan.h > >> @@ -0,0 +1,93 @@ > >> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- > >> */ > >> + > >> +/* > >> + * Weiping Pan<[email protected]> > >> + * > >> + * This library is free software; you can redistribute it and/or > >> + * modify it under the terms of the GNU Lesser General Public > >> + * License as published by the Free Software Foundation; either > >> + * version 2 of the License, or (at your option) any later version. > >> + * > >> + * This library is distributed in the hope that it will be useful, > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > >> + * Lesser General Public License for more details. > >> + * > >> + * You should have received a copy of the GNU Lesser General Public > >> + * License along with this library; if not, write to the > >> + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, > >> + * Boston, MA 02110-1301 USA. > >> + * > >> + * (C) Copyright 2011 Red Hat, Inc. > >> + */ > >> + > >> +#ifndef NM_SETTING_VLAN_H > >> +#define NM_SETTING_VLAN_H > >> + > >> +#include "nm-setting.h" > >> +#include<linux/if_vlan.h> > >> + > >> +G_BEGIN_DECLS > >> + > >> +#define NM_TYPE_SETTING_VLAN (nm_setting_vlan_get_type ()) > >> +#define NM_SETTING_VLAN(obj) (G_TYPE_CHECK_INSTANCE_CAST > >> ((obj), NM_TYPE_SETTING_VLAN, NMSettingVLAN)) > >> +#define NM_SETTING_VLAN_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST > >> ((klass), NM_TYPE_SETTING_VLANCONFIG, NMSettingVLANClass)) > >> +#define NM_IS_SETTING_VLAN(obj) (G_TYPE_CHECK_INSTANCE_TYPE > >> ((obj), NM_TYPE_SETTING_VLAN)) > >> +#define NM_IS_SETTING_VLAN_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), > >> NM_TYPE_SETTING_VLAN)) > >> +#define NM_SETTING_VLAN_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS > >> ((obj), NM_TYPE_SETTING_VLAN, NMSettingVLANClass)) > >> + > >> +#define NM_SETTING_VLAN_SETTING_NAME "VLAN" > > Lowercase here, ie 'vlan', for consistency with the rest of the > > settings. > > > >> +/** > >> + * NMSettingVlanError: > >> + * @NM_SETTING_VLAN_ERROR_UNKNOWN: unknown or unclassified error > >> + * @NM_SETTING_VLAN_ERROR_INVALID_PROPERTY: the property was invalid > >> + * @NM_SETTING_VLAN_ERROR_MISSING_PROPERTY: the property was missing and > >> is > >> + * required > >> + */ > >> +typedef enum { > >> + NM_SETTING_VLAN_ERROR_UNKNOWN = 0, > >> + NM_SETTING_VLAN_ERROR_INVALID_PROPERTY, > >> + NM_SETTING_VLAN_ERROR_MISSING_PROPERTY > >> +} NMSettingVlanError; > >> + > >> +#define NM_TYPE_SETTING_VLAN_ERROR (nm_setting_vlan_error_get_type ()) > >> +GType nm_setting_vlan_error_get_type (void); > >> + > >> +#define NM_SETTING_VLAN_ERROR nm_setting_vlan_error_quark () > >> +GQuark nm_setting_vlan_error_quark (void); > >> + > >> +GType nm_setting_vlan_get_type (void); > >> + > >> +#define NM_SETTING_VLAN_INTERFACE_NAME "interface-name" > >> +#define NM_SETTING_VLAN_VLAN_SLAVE "vlan-slave" > >> +#define NM_SETTING_VLAN_VLAN_ID "vlan-id" > >> +#define NM_SETTING_VLAN_VLAN_FLAGS "vlan-flags" > >> +#define NM_SETTING_VLAN_VLAN_INGRESS_PRIORITY_MAP > >> "vlan-priority-ingress-map" > >> +#define NM_SETTING_VLAN_VLAN_EGRESS_PRIORITY_MAP > >> "vlan-priority-egress-map" > > Same thing as with the accessor functions for these, lets just name them > > as follows since they are already namespaced as part of the VLAN > > setting: > > > > #define NM_SETTING_VLAN_SLAVE "slave" > > #define NM_SETTING_VLAN_ID "id" > > #define NM_SETTING_VLAN_FLAGS "flags" > > > > We should also probably have an enum for the 'flags' somewhere, ie > > called NMVlanFlags; I see later patches use VLAN_FLAG_REORDER_HDR but I > > can't find a definition of that anywhere? And lets make it a typedef > > too so we don't have to keep writing 'enum NMVlanFlags' everywhere. > > > The definition is in /usr/include/linux/if_vlan.h. > 33 enum vlan_flags { > 34 VLAN_FLAG_REORDER_HDR = 0x1, > 35 VLAN_FLAG_GVRP = 0x2, > 36 VLAN_FLAG_LOOSE_BINDING = 0x4, > 37 }; Ok, but that's kernel headers and we're letting that leak into the NM public API. I don't expect this to change in the future, but since it's a "private" kernel header (things like this have changed before, ex WEXT -> nl80211 for wifi) we should probably define an NM-specific enum type for these flags and mirror the kernel definitions. I'd rather to do that now than have to deal with the kernel stuff changing in the future. > So we can make use of it instead of creating another 'enum NMVlanFlags'. > >> +typedef struct { > >> + NMSetting parent; > >> +} NMSettingVLAN; > >> + > >> +typedef struct { > >> + NMSettingClass parent; > >> + > >> + /* Padding for future expansion */ > >> + void (*_reserved1) (void); > >> + void (*_reserved2) (void); > >> + void (*_reserved3) (void); > >> + void (*_reserved4) (void); > >> +} NMSettingVLANClass; > >> + > >> +NMSetting *nm_setting_vlan_new(void); > >> +const char *nm_setting_vlan_get_interface_name(NMSettingVLAN *setting); > >> +const char *nm_setting_vlan_get_vlan_slave(NMSettingVLAN *setting); > >> +guint32 nm_setting_vlan_get_vlan_id(NMSettingVLAN *setting); > >> +guint32 nm_setting_vlan_get_vlan_flags(NMSettingVLAN *setting); > > Once we get an enum for flags, we should return that enum type from > > nm_setting_vlan_get_vlan_flags() too. > But this flag is the OR of three different enum type, so maybe > nm_setting_vlan_get_vlan_flags() > should return gunit32. True. Leave it as a guint32 then. Dan > >> +const char *nm_setting_vlan_get_vlan_priority_ingress_map(NMSettingVLAN > >> *setting); > >> +const char *nm_setting_vlan_get_vlan_priority_egress_map(NMSettingVLAN > >> *setting); > >> + > >> +G_END_DECLS > >> + > >> +#endif /* NM_SETTING_VLAN_H */ > >> diff --git a/src/settings/plugins/ifcfg-rh/common.h > >> b/src/settings/plugins/ifcfg-rh/common.h > >> index 74a77c4..90f9584 100644 > >> --- a/src/settings/plugins/ifcfg-rh/common.h > >> +++ b/src/settings/plugins/ifcfg-rh/common.h > >> @@ -44,6 +44,7 @@ > >> #define TYPE_ETHERNET "Ethernet" > >> #define TYPE_WIRELESS "Wireless" > >> #define TYPE_BRIDGE "Bridge" > >> +#define TYPE_VLAN "VLAN" > >> > >> #define SECRET_FLAG_AGENT "user" > >> #define SECRET_FLAG_NOT_SAVED "ask" > >> diff --git a/src/settings/plugins/ifcfg-rh/reader.c > >> b/src/settings/plugins/ifcfg-rh/reader.c > >> index 910cca3..7ba9509 100644 > >> --- a/src/settings/plugins/ifcfg-rh/reader.c > >> +++ b/src/settings/plugins/ifcfg-rh/reader.c > >> @@ -46,6 +46,7 @@ > >> #include<NetworkManager.h> > >> #include<nm-setting-connection.h> > >> #include<nm-setting-ip4-config.h> > >> +#include<nm-setting-vlan.h> > >> #include<nm-setting-ip6-config.h> > >> #include<nm-setting-wired.h> > >> #include<nm-setting-wireless.h> > >> @@ -3298,6 +3299,130 @@ wired_connection_from_ifcfg (const char *file, > >> return connection; > >> } > >> > >> +static NMSetting * > >> +make_vlan_setting (shvarFile *ifcfg, > >> + const char *file, > >> + gboolean nm_controlled, > >> + char **unmanaged, > >> + NMSetting8021x **s_8021x, > >> + GError **error) > >> +{ > >> + NMSettingVLAN *s_vlan = NULL; > >> + char *value = NULL; > >> + char *interface_name = NULL; > >> + char *vlan_slave = NULL; > >> + char *p = NULL; > >> + guint32 vlan_id = 0; > >> + guint32 vlan_flags = 0; > >> + char *vlan_priority_ingress_map = NULL; > >> + char *vlan_priority_egress_map = NULL; > >> + > >> + > >> + s_vlan = NM_SETTING_VLAN(nm_setting_vlan_new()); > >> + > >> + interface_name = svGetValue (ifcfg, "DEVICE", FALSE); > >> + if (!interface_name) > >> + goto error_return; > >> + else { > >> + if (!g_str_has_prefix(interface_name, "vlan")) > >> + goto free_interface_name; > >> + > >> + g_object_set(s_vlan, NM_SETTING_VLAN_INTERFACE_NAME, > >> interface_name, NULL); > >> + > >> + p = interface_name + 4; > >> + vlan_id = g_ascii_strtoull(p, NULL, 10); > >> + if (vlan_id>= 0&& vlan_id< 4096) > >> + g_object_set(s_vlan, NM_SETTING_VLAN_VLAN_ID, vlan_id, > >> NULL); > >> + else > >> + goto free_interface_name; > >> + } > >> + > >> + vlan_slave = svGetValue (ifcfg, "PHYSDEV", FALSE); > >> + if (vlan_slave) { > >> + g_object_set(s_vlan, NM_SETTING_VLAN_VLAN_SLAVE, vlan_slave, > >> NULL); > >> + g_free(vlan_slave); > >> + } > >> + > >> + value = svGetValue (ifcfg, "REORDER_HDR", FALSE); > >> + if (value) > >> + vlan_flags |= VLAN_FLAG_REORDER_HDR; > > I couldn't find the definition of VLAN_FLAG_REORDER_HDR; did I just miss > > that? > > > The definition is in /usr/include/linux/if_vlan.h. > >> + g_free(value); > >> + > >> + value = svGetValue (ifcfg, "VLAN_FLAGS", FALSE); > >> + if (g_strcmp0("VLAN_FLAG_GVRP", value)) { > >> + vlan_flags |= VLAN_FLAG_GVRP; > >> + } else if (g_strcmp0("VLAN_FLAG_LOOSE_BINDING", value)) { > >> + vlan_flags |= VLAN_FLAG_LOOSE_BINDING; > >> + } > >> + g_object_set(s_vlan, NM_SETTING_VLAN_VLAN_FLAGS, vlan_flags, NULL); > >> + g_free(value); > >> + > >> + vlan_priority_ingress_map = svGetValue (ifcfg, > >> "VLAN_INGRESS_PRIORITY_MAP", FALSE); > >> + if (vlan_priority_ingress_map) { > >> + g_object_set(s_vlan, NM_SETTING_VLAN_VLAN_INGRESS_PRIORITY_MAP, > >> vlan_priority_ingress_map, NULL); > >> + g_free(vlan_priority_ingress_map); > >> + } > > So like I was talking about earlier, it's probably just easier to parse > > the ingress/egress priorities when reading the ifcfg file in, rather > > than having them as a string. That probably means adding list iterator > > functions like we have for IP addresses, nameservers, etc, and creating > > a small opaque structure for them (though we could just store them > > internally as a 32-bit number and pack both of the<uint>:<uint> into > > that, and save a lot of allocation). So we'd then get something like: > > > > guint32 nm_setting_vlan_get_num_ingress_priorities (NMSettingVlan > > *setting); > > gboolean nm_setting_vlan_get_ingress_priority (NMSettingVlan > > *setting, guint32 i, guint32 *out_a, guint32 *out_b); > > gboolean nm_setting_vlan_add_ingress_priority (NMSettingVlan > > *setting, guint32 a, b); > > void nm_setting_vlan_remove_ingress_priority (NMSettingVlan > > *setting, guint32 i); > > void nm_setting_vlan_clear_ingress_priorities (NMSettingVlan > > *setting); > > > > for both ingress and egress; that'll be a lot easier to work with > > internally in NM since we'll only have to parse it once and then we can > > just iterate through and pull them out. > > > > It's a bit more complicated than that for the actual GObject properties > > though (and thus D-Bus marshalling). I vote we keep it simple and make > > the GObject property type DBUS_TYPE_G_LIST_OF_STRING. This makes it > > easier to use from D-Bus, but it also means we need converters for the > > get_property() and set_property() functions in nm-setting-vlan.c. These > > would take a GSList<string> and parse each "<uint>:<uint>" element into > > the right integers to pass to nm_setting_vlan_add_ingress_priority(). > > Then in set_property() we'd do the reverse and build up a GSList of > > allocated strings using the ingress/egress priorites and use that in the > > call to g_value_set_boxed (). There are some examples of that in > > various places, like the IPv4 config where we box/unbox (ie > > marshall/demarshall) the property. I can explain more here if you want. > > > > Thanks, > > Dan > > > Ok, thanks > Weiping Pan > >> + vlan_priority_egress_map = svGetValue (ifcfg, > >> "VLAN_EGRESS_PRIORITY_MAP", FALSE); > >> + if (vlan_priority_egress_map) { > >> + g_object_set(s_vlan, NM_SETTING_VLAN_VLAN_EGRESS_PRIORITY_MAP, > >> vlan_priority_egress_map, NULL); > >> + g_free(vlan_priority_egress_map); > >> + } > >> + return (NMSetting *) s_vlan; > >> + > >> +free_interface_name: > >> + g_free(interface_name); > >> +error_return: > >> + g_object_unref(s_vlan); > >> + return NULL; > >> +} > >> + > >> +static NMConnection * > >> +vlan_connection_from_ifcfg (const char *file, > >> + shvarFile *ifcfg, > >> + gboolean nm_controlled, > >> + char **unmanaged, > >> + GError **error) > >> +{ > >> + NMConnection *connection = NULL; > >> + NMSetting *con_setting = NULL; > >> + NMSetting *vlan_setting = NULL; > >> + NMSetting8021x *s_8021x = NULL; > >> + > >> + g_return_val_if_fail (file != NULL, NULL); > >> + g_return_val_if_fail (ifcfg != NULL, NULL); > >> + > >> + connection = nm_connection_new (); > >> + if (!connection) { > >> + g_set_error (error, IFCFG_PLUGIN_ERROR, 0, > >> + "Failed to allocate new connection for %s.", file); > >> + return NULL; > >> + } > >> + > >> + con_setting = make_connection_setting (file, ifcfg, > >> NM_SETTING_VLAN_SETTING_NAME, NULL); > >> + if (!con_setting) { > >> + g_set_error (error, IFCFG_PLUGIN_ERROR, 0, > >> + "Failed to create connection setting."); > >> + g_object_unref (connection); > >> + return NULL; > >> + } > >> + nm_connection_add_setting (connection, con_setting); > >> + > >> + vlan_setting = make_vlan_setting (ifcfg, file, nm_controlled, > >> unmanaged,&s_8021x, error); > >> + if (!vlan_setting) { > >> + g_object_unref (connection); > >> + return NULL; > >> + } > >> + nm_connection_add_setting (connection, vlan_setting); > >> + > >> + if (!nm_connection_verify (connection, error)) { > >> + g_object_unref (connection); > >> + return NULL; > >> + } > >> + > >> + return connection; > >> +} > >> + > >> static gboolean > >> is_wireless_device (const char *iface) > >> { > >> @@ -3342,7 +3467,6 @@ is_wireless_device (const char *iface) > >> enum { > >> IGNORE_REASON_NONE = 0x00, > >> IGNORE_REASON_BRIDGE = 0x01, > >> - IGNORE_REASON_VLAN = 0x02, > >> }; > >> > >> NMConnection * > >> @@ -3456,7 +3580,7 @@ connection_from_file (const char *filename, > >> g_free (lower); > >> } > >> > >> - /* Ignore BRIDGE= and VLAN= connections for now too (rh #619863) */ > >> + /* Ignore BRIDGE= connections for now too (rh #619863) */ > >> tmp = svGetValue (parsed, "BRIDGE", FALSE); > >> if (tmp) { > >> g_free (tmp); > >> @@ -3464,24 +3588,17 @@ connection_from_file (const char *filename, > >> ignore_reason = IGNORE_REASON_BRIDGE; > >> } > >> > >> - if (nm_controlled) { > >> - tmp = svGetValue (parsed, "VLAN", FALSE); > >> - if (tmp) { > >> - g_free (tmp); > >> - nm_controlled = FALSE; > >> - ignore_reason = IGNORE_REASON_VLAN; > >> - } > >> - } > >> - > >> /* Construct the connection */ > >> if (!strcasecmp (type, TYPE_ETHERNET)) > >> connection = wired_connection_from_ifcfg (filename, parsed, > >> nm_controlled, unmanaged,&error); > >> else if (!strcasecmp (type, TYPE_WIRELESS)) > >> connection = wireless_connection_from_ifcfg (filename, parsed, > >> nm_controlled, unmanaged,&error); > >> - else if (!strcasecmp (type, TYPE_BRIDGE)) { > >> + else if (!strcasecmp (type, TYPE_BRIDGE)) > >> g_set_error (&error, IFCFG_PLUGIN_ERROR, 0, > >> "Bridge connections are not yet supported"); > >> - } else { > >> + else if (!strcasecmp (type, TYPE_VLAN)) > >> + connection = vlan_connection_from_ifcfg (filename, parsed, > >> nm_controlled, unmanaged,&error); > >> + else { > >> g_set_error (&error, IFCFG_PLUGIN_ERROR, 0, > >> "Unknown connection type '%s'", type); > >> } > >> diff --git > >> a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface > >> > >> b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface > >> index 6c84185..d6d3f99 100644 > >> --- > >> a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface > >> +++ > >> b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface > >> @@ -1,7 +1,12 @@ > >> -DEVICE=eth1.43 > >> VLAN=yes > >> +TYPE=vlan > >> +DEVICE=vlan43 > >> +PHYSDEV=eth9 > >> +REORDER_HDR=0 > >> +VLAN_FLAGS=GVRP,LOOSE_BINDING > >> +VLAN_INGRESS_PRIORITY_MAP=0:1,2:5 > >> +VLAN_EGRESS_PRIORITY_MAP=12:3,14:7 > >> ONBOOT=yes > >> -BOOTPROTO=none > >> +BOOTPROTO=static > >> IPADDR=192.168.43.149 > >> NETMASK=255.255.255.0 > >> - > > > _______________________________________________ networkmanager-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/networkmanager-list
