colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bond: small cleanups

Browse Source 
* Use an enum instead of a string, is faster for comparisons.
* Add debug assertions
* Have NMBondMode enum correspond to Kernel numbering
This commit is contained in:
Antonio Cardace
2020-04-09 19:18:27 +02:00
parent d73a98a3e8
commit 2a5d9eb60b
3 changed files with 42 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
libnm-core/nm-core-internal.h
View File

@@ -521,14 +521,17 @@ NMConnectionMultiConnect _nm_connection_get_multi_connect (NMConnection *connect
/*****************************************************************************/ /*****************************************************************************/
typedef enum { typedef enum {
NM_BOND_MODE_UNKNOWN = 0, NM_BOND_MODE_UNKNOWN = -1,
NM_BOND_MODE_ROUNDROBIN,
NM_BOND_MODE_ACTIVEBACKUP, /* The numeric values correspond to kernel's numbering of the modes. */
NM_BOND_MODE_XOR, NM_BOND_MODE_ROUNDROBIN = 0,
NM_BOND_MODE_BROADCAST, NM_BOND_MODE_ACTIVEBACKUP = 1,
NM_BOND_MODE_8023AD, NM_BOND_MODE_XOR = 2,
NM_BOND_MODE_TLB, NM_BOND_MODE_BROADCAST = 3,
NM_BOND_MODE_ALB, NM_BOND_MODE_8023AD = 4,
NM_BOND_MODE_TLB = 5,
NM_BOND_MODE_ALB = 6,
_NM_BOND_MODE_NUM,
} NMBondMode; } NMBondMode;
NMBondMode _nm_setting_bond_mode_from_string (const char *str); NMBondMode _nm_setting_bond_mode_from_string (const char *str);

44
libnm-core/nm-setting-bond.c
View File

@@ -45,6 +45,7 @@ G_DEFINE_TYPE (NMSettingBond, nm_setting_bond, NM_TYPE_SETTING)
/*****************************************************************************/ /*****************************************************************************/
static const char *const valid_options_lst[] = { static const char *const valid_options_lst[] = {
/* mode must be the first element. nm-device-bond.c relies on that. */
NM_SETTING_BOND_OPTION_MODE, NM_SETTING_BOND_OPTION_MODE,
NM_SETTING_BOND_OPTION_MIIMON, NM_SETTING_BOND_OPTION_MIIMON,
NM_SETTING_BOND_OPTION_DOWNDELAY, NM_SETTING_BOND_OPTION_DOWNDELAY,
@@ -145,6 +146,7 @@ NM_UTILS_STRING_TABLE_LOOKUP_STRUCT_DEFINE (
nm_assert (G_N_ELEMENTS (LIST) == NM_PTRARRAY_LEN (valid_options_lst)); nm_assert (G_N_ELEMENTS (LIST) == NM_PTRARRAY_LEN (valid_options_lst));
for (i = 0; i < G_N_ELEMENTS (LIST); i++) for (i = 0; i < G_N_ELEMENTS (LIST); i++)
_nm_assert_bond_meta (&LIST[i].value); _nm_assert_bond_meta (&LIST[i].value);
nm_assert (nm_streq (valid_options_lst[0], NM_SETTING_BOND_OPTION_MODE));
} }
}, },
{ return NULL; }, { return NULL; },
@@ -204,6 +206,7 @@ gboolean
_nm_setting_bond_option_supported (const char *option, NMBondMode mode) _nm_setting_bond_option_supported (const char *option, NMBondMode mode)
{ {
nm_assert (option); nm_assert (option);
nm_assert (mode != NM_BOND_MODE_UNKNOWN);
nm_assert (_NM_INT_NOT_NEGATIVE (mode) && mode < 32); nm_assert (_NM_INT_NOT_NEGATIVE (mode) && mode < 32);
return !NM_FLAGS_ANY (_bond_option_unsupp_mode (option), BIT (mode)); return !NM_FLAGS_ANY (_bond_option_unsupp_mode (option), BIT (mode));
@@ -728,13 +731,11 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
{ {
NMSettingBond *self = NM_SETTING_BOND (setting); NMSettingBond *self = NM_SETTING_BOND (setting);
NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting); NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting);
int mode;
int miimon; int miimon;
int arp_interval; int arp_interval;
int num_grat_arp; int num_grat_arp;
int num_unsol_na; int num_unsol_na;
const char *mode_orig; const char *mode_str;
const char *mode_new;
const char *arp_ip_target = NULL; const char *arp_ip_target = NULL;
const char *lacp_rate; const char *lacp_rate;
const char *primary; const char *primary;
@@ -780,8 +781,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
*/ */
/* Verify bond mode */ /* Verify bond mode */
mode_orig = _bond_get_option (self, NM_SETTING_BOND_OPTION_MODE); mode_str = _bond_get_option (self, NM_SETTING_BOND_OPTION_MODE);
if (!mode_orig) { if (!mode_str) {
g_set_error (error, g_set_error (error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -790,13 +791,13 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
return FALSE; return FALSE;
} }
mode = nm_utils_bond_mode_string_to_int (mode_orig); bond_mode = _nm_setting_bond_mode_from_string (mode_str);
if (mode == -1) { if (bond_mode == NM_BOND_MODE_UNKNOWN) {
g_set_error (error, g_set_error (error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s' is not a valid value for '%s'"), _("'%s' is not a valid value for '%s'"),
mode_orig, mode_str,
NM_SETTING_BOND_OPTION_MODE); NM_SETTING_BOND_OPTION_MODE);
g_prefix_error (error, g_prefix_error (error,
"%s.%s: ", "%s.%s: ",
@@ -804,18 +805,17 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
NM_SETTING_BOND_OPTIONS); NM_SETTING_BOND_OPTIONS);
return FALSE; return FALSE;
} }
mode_new = nm_utils_bond_mode_int_to_string (mode);
/* Make sure mode is compatible with other settings */ /* Make sure mode is compatible with other settings */
if (NM_IN_STRSET (mode_new, "balance-alb", if (NM_IN_SET (bond_mode, NM_BOND_MODE_TLB,
"balance-tlb")) { NM_BOND_MODE_ALB)) {
if (arp_interval > 0) { if (arp_interval > 0) {
g_set_error (error, g_set_error (error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s=%s' is incompatible with '%s > 0'"), _("'%s=%s' is incompatible with '%s > 0'"),
NM_SETTING_BOND_OPTION_MODE, NM_SETTING_BOND_OPTION_MODE,
mode_new, mode_str,
NM_SETTING_BOND_OPTION_ARP_INTERVAL); NM_SETTING_BOND_OPTION_ARP_INTERVAL);
g_prefix_error (error, g_prefix_error (error,
"%s.%s: ", "%s.%s: ",
@@ -826,7 +826,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
} }
primary = _bond_get_option (self, NM_SETTING_BOND_OPTION_PRIMARY); primary = _bond_get_option (self, NM_SETTING_BOND_OPTION_PRIMARY);
if (NM_IN_STRSET (mode_new, "active-backup")) { if (bond_mode == NM_BOND_MODE_ACTIVEBACKUP) {
GError *tmp_error = NULL; GError *tmp_error = NULL;
if (primary && !nm_utils_ifname_valid_kernel (primary, &tmp_error)) { if (primary && !nm_utils_ifname_valid_kernel (primary, &tmp_error)) {
@@ -855,12 +855,12 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
if ( connection if ( connection
&& nm_connection_get_setting_infiniband (connection)) { && nm_connection_get_setting_infiniband (connection)) {
if (!NM_IN_STRSET (mode_new, "active-backup")) { if (bond_mode != NM_BOND_MODE_ACTIVEBACKUP) {
g_set_error (error, g_set_error (error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s=%s' is not a valid configuration for '%s'"), _("'%s=%s' is not a valid configuration for '%s'"),
NM_SETTING_BOND_OPTION_MODE, mode_new, NM_SETTING_INFINIBAND_SETTING_NAME); NM_SETTING_BOND_OPTION_MODE, mode_str, NM_SETTING_INFINIBAND_SETTING_NAME);
g_prefix_error (error, g_prefix_error (error,
"%s.%s: ", "%s.%s: ",
NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS); NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
@@ -967,7 +967,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
lacp_rate = _bond_get_option (self, NM_SETTING_BOND_OPTION_LACP_RATE); lacp_rate = _bond_get_option (self, NM_SETTING_BOND_OPTION_LACP_RATE);
if ( lacp_rate if ( lacp_rate
&& !nm_streq0 (mode_new, "802.3ad") && bond_mode != NM_BOND_MODE_8023AD
&& !NM_IN_STRSET (lacp_rate, "0", "slow")) { && !NM_IN_STRSET (lacp_rate, "0", "slow")) {
g_set_error (error, g_set_error (error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
@@ -996,7 +996,14 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
/* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */ /* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
if (!nm_streq0 (mode_orig, mode_new)) { if (!NM_IN_STRSET (mode_str,
"802.3ad",
"active-backup",
"balance-rr",
"balance-alb",
"balance-tlb",
"balance-xor",
"broadcast")) {
g_set_error (error, g_set_error (error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -1007,7 +1014,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
} }
/* normalize unsupported options for the current mode */ /* normalize unsupported options for the current mode */
bond_mode = _nm_setting_bond_mode_from_string (mode_new);
for (i = 0; priv->options_idx_cache[i].name; i++) { for (i = 0; priv->options_idx_cache[i].name; i++) {
n = &priv->options_idx_cache[i]; n = &priv->options_idx_cache[i];
if (!_nm_setting_bond_option_supported (n->name, bond_mode)) { if (!_nm_setting_bond_option_supported (n->name, bond_mode)) {
@@ -1015,7 +1021,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s' option is not valid with mode '%s'"), _("'%s' option is not valid with mode '%s'"),
n->name, mode_new); n->name, mode_str);
g_prefix_error (error, g_prefix_error (error,
"%s.%s: ", "%s.%s: ",
NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_SETTING_NAME,

4
src/devices/nm-device-bond.c
View File

@@ -131,8 +131,12 @@ update_connection (NMDevice *device, NMConnection *connection)
*p = '\0'; *p = '\0';
} }
if (mode == NM_BOND_MODE_UNKNOWN) {
if (value && nm_streq (*options, NM_SETTING_BOND_OPTION_MODE)) if (value && nm_streq (*options, NM_SETTING_BOND_OPTION_MODE))
mode = _nm_setting_bond_mode_from_string (value); mode = _nm_setting_bond_mode_from_string (value);
if (mode == NM_BOND_MODE_UNKNOWN)
continue;
}
if (!_nm_setting_bond_option_supported (*options, mode)) if (!_nm_setting_bond_option_supported (*options, mode))
continue; continue;