> [main]
> server_mode=yes|no

Just to add, our previous discussions on this topic concluded that server mode
is not as a good idea as it might seem. The configuration needs individual
tweaks for various things people expect from 'server mode'.

[main]
autocreateconnections=no|yes

(or another name for this particular tweak)

I also believe that in the long run, NetworkManager should internally default
to 'no' and desktops should be served with NetworkManager.conf that enables
various automatic actions.

(In reply to comment #1)
> > [main]
> > server_mode=yes|no
> 
> Just to add, our previous discussions on this topic concluded that server mode
> is not as a good idea as it might seem. The configuration needs individual
> tweaks for various things people expect from 'server mode'.
> 
> [main]
> autocreateconnections=no|yes
> 
> (or another name for this particular tweak)

I can definitely agree with this. It makes sense. Maybe we should come up with a list of the individual tweaks.

> 
> I also believe that in the long run, NetworkManager should internally default
> to 'no' and desktops should be served with NetworkManager.conf that enables
> various automatic actions.

I would find very useful. It goes in the same direction of what I was saying of changing opt-out to opt-in.

> I can definitely agree with this. It makes sense. Maybe we should come up with
> a list of the individual tweaks.

Any ideas?

(In reply to comment #3)
> > I can definitely agree with this. It makes sense. Maybe we should come up with
> > a list of the individual tweaks.
> 
> Any ideas?

I'll try to come up with some.

I think we actually already have this; put the following line into the [main] section of /etc/NetworkManager/NetworkManager.conf:

no-auto-default=*

and no auto default DHCP connection will be created.

What we *do* want to do though, is possibly create an /etc/NetworkManager/conf.d directory that other packages can drop files into, that would then get picked up into the main config.  That way you could install a "server"  package perhaps that would automatically customize the behavior at install-time, instead of having to modify it after install.

Here's the list of toggles I came up with, basically for every case I could think of where NM does something automatically that the initscripts don't. (In each case, 'true' is the current NM behavior, and 'false' is the initscripts behavior.)

autoconnect: If 'true', NMSettingConnection:autoconnect applies at all times. If 'false', it only applies at boot time. [I'm not sure there's actually a use case for this?]

carrier-detect: If 'true', device activation requires carrier (except where overridden by connection settings). If 'false', carrier is ignored. [It's a bit weird to have both this and the :carrier-detect properties, but it seemed like we think server admins want to be able to configure this globally.]

auto-ethernet: If 'true', NM automatically creates DHCP-based connections for ethernet devices with no other connections. If 'false', it doesn't. [This behavior does only apply to ethernet, right?]

auto-reload-connections: If 'true', editing a connection file causes the configuration to be reloaded. If 'false', files are only read at startup. [This is probably unnecessary given other upcoming changes.]

manage-resolv-conf: If 'true', NM adjusts /etc/resolv.conf as connections change. If 'false', it never touches /etc/resolv.conf. [Possibly to be handled by dns=... instead?]

manage-hostname: If 'true', NM sets the system hostname by reverse resolving its "best" IP address, if the system doesn't already have a hostname. If 'false', it never touches the system hostname. [Arguably unnecessary since any server should (a) always have a name in /etc/hostname anyway meaning NM would never try to set the hostname, and (b) have correct reverse DNS on its primary IP address, meaning NM would do the right thing if it did try to set the hostname.]

manage-default-route: If 'true', NM adjusts the default route automatically to match its idea of the best connection. If 'false', it adjusts it only as explicitly specified in the network config files. [Note that this 'false' behavior requires writing new code. We could alternatively say, if 'false', it sets the default route from /etc/sysconfig/network, etc, at startup and then never touches it again after that. Which also requires new code, but not as much.]


Any other automatic behaviors that I missed?

(In reply to comment #6)
> autoconnect: If 'true', NMSettingConnection:autoconnect applies at all times.
> If 'false', it only applies at boot time. [I'm not sure there's actually a use
> case for this?]

It would be nice to have a use case. And the name 'autoconnect' doesn't really say what it does. I would try to avoid this option or rework it carefully.

> carrier-detect: If 'true', device activation requires carrier (except where
> overridden by connection settings). If 'false', carrier is ignored. [It's a bit
> weird to have both this and the :carrier-detect properties, but it seemed like
> we think server admins want to be able to configure this globally.]

I could think of 'disable-carrier-detect' with reversed meaning as it sounds much more explicit. It disables carrier detect at all cases, while its absence means to leave carrier detection up to connections.

> auto-ethernet: If 'true', NM automatically creates DHCP-based connections for
> ethernet devices with no other connections. If 'false', it doesn't. [This
> behavior does only apply to ethernet, right?]

Sounds good.

> auto-reload-connections: If 'true', editing a connection file causes the
> configuration to be reloaded. If 'false', files are only read at startup.
> [This is probably unnecessary given other upcoming changes.]

Good. From our point of view this should be an explicit action and it 

> manage-resolv-conf: If 'true', NM adjusts /etc/resolv.conf as connections
> change. If 'false', it never touches /etc/resolv.conf. [Possibly to be handled
> by dns=... instead?]

I would go for dns=none|default|resolvconf|netconfig|dnsmasq or something like that. This shouldn't probably need a new directive.

> manage-hostname: If 'true', NM sets the system hostname by reverse resolving
> its "best" IP address, if the system doesn't already have a hostname. If
> 'false', it never touches the system hostname. [Arguably unnecessary since any
> server should (a) always have a name in /etc/hostname anyway meaning NM would
> never try to set the hostname, and (b) have correct reverse DNS on its primary
> IP address, meaning NM would do the right thing if it did try to set the
> hostname.]

If the existence of /etc/hostname is enough to disable hostname management, that sounds good enough for me. Otherwise, shouldn't the hostname come directly from DHCP instead of using DNS queries?

> manage-default-route: If 'true', NM adjusts the default route automatically to
> match its idea of the best connection.

Not that I really like it. You can still set the connections as non-default-route or something. I believe this needs more discussion with particular use cases involved.

> If 'false', it adjusts it only as
> explicitly specified in the network config files.

Sounds wrong. If I don't manage default route, I shouldn't touch it. Maybe 'auto-default-route' for the automatic choice?

> [Note that this 'false'
> behavior requires writing new code. We could alternatively say, if 'false', it
> sets the default route from /etc/sysconfig/network, etc, at startup and then
> never touches it again after that. Which also requires new code, but not as
> much.]

Looks a bit hacky.

> Any other automatic behaviors that I missed?

None that I remember. I am also curious about the section or sections of the configuration file. There's a big difference between pushing stuff to the main section and to a special hacky section.

(In reply to comment #7)
> I could think of 'disable-carrier-detect' with reversed meaning as it sounds
> much more explicit.

Yeah... I was trying to consistently have "true" == more automation, "false" == less automation, but especially if the DNS thing is going to be controlled by an entirely unrelated option, maybe that's not important.

> If the existence of /etc/hostname is enough to disable hostname management,
> that sounds good enough for me. Otherwise, shouldn't the hostname come directly
> from DHCP instead of using DNS queries?

Oh, maybe it does.

The problem with having this be triggered only by the existence of /etc/hostname is that there's no obvious place to document that fact... but it doesn't make a lot of sense the way I wrote it either.

> > manage-default-route: If 'true', NM adjusts the default route automatically to

> > If 'false', it adjusts it only as
> > explicitly specified in the network config files.
> 
> Sounds wrong. If I don't manage default route, I shouldn't touch it.

That would be a really useless behavior though; no other service is going to set the default route for you, so if NM doesn't do it, then you'll end up with no default route at all (or else, you'll have to write a shell script to set it yourself).

Maybe default-route=[static|dynamic] would be better

Or maybe default-route=[DEVICENAME|auto]

I guess even the initscripts change the default route automatically as devices are brought up, so maybe there's no need for this option.

> I am also curious about the section or sections of the
> configuration file. There's a big difference between pushing stuff to the main
> section and to a special hacky section.

I had been thinking they'd all go in a single section called [auto] or [automatic] or [automation] or something. But if resolv.conf-handling is going to be in [main], that makes less sense.

The presence of /etc/hostname should prevent NM from changing the hostname as long as the hostname is the same as what's in /etc/hostname.  If we can come up with a good use-case to have a dont-touch-hostname setting, then sure we can do this.  But I'm not sure there is one yet?

The problem we have with global carrier detect is that "default" isn't an accepted value for NM_SETTING_WIRED_CARRIER_DETECT.  So when the settings get to NetworkManager, we'll have no idea if the user explicitly specified eg "yes" or whether they just left it blank and let GObject fill in the default value at construct time.  We could add "default" as an option and define that to be "yes, unless changed by the global carrier-detect setting", such that if some user really wanted "yes" no matter what the global default setting was, they could set that.

For default route, that's a hard one, because most things in initscripts land change the default route automatically when a connection comes up, even sometimes if DEFROUTE=no.  PPP always sets the defualt route unless you specify "nodefaultroute" as one of the options, but of course you do that in /etc/ppp/options and not in /etc/sysconfig/network-scripts/ifcfg-ppp0 so the initscripts have no idea what's happening there.  dhclient-script does actually respect DEFROUTE=no and GATEWAYDEV, but also has a magic DHCLIENT_IGNORE_GATEWAY option too.  It's a train-wreck.

I'm not sure there's a pressing use-case for this yet like there is for DNS. We could also work around this too: if NM finds a default route when it starts (or one is added after NM starts) that does not point to a device NM manages, then perhaps we stop touching the default route until that route goes away?  I'm not sure everything has to be hard config option if we can find a reasonable way to make NM get out of the way.

(In reply to comment #9)
> The presence of /etc/hostname should prevent NM from changing the hostname as
> long as the hostname is the same as what's in /etc/hostname.

Which makes me curious about the rationale why /etc/hostname with different value shouldn't prevent NetworkManager from setting it. It might be reasonable to have this documented in the manual page together with all other files that are used by NetworkManager.

> We could add "default" as an option and define that to be
> "yes, unless changed by the global carrier-detect setting", such that if some
> user really wanted "yes" no matter what the global default setting was, they
> could set that.

Could be. With the exception that I would rephrase it a little bit so that 'default' means use the global option and the global option itself defaults to yes. Might as well be called 'global' instead of 'default' then.

But what I'm curious is whether there is a use case for that. And how does the global carrier-detect=no interact with DHCP.

> For default route...
> We could also work around this too: if NM finds a default route when it starts (or
> one is added after NM starts) that does not point to a device NM manages, then
> perhaps we stop touching the default route until that route goes away?

The default route at NetworkManager's start constitutes part of a runtime connection if it can be matched with a device somehow and should be taken over as such.

But a default route that can't be traced to a specific device should probably be handled as an existing configuration, too. Maybe a special active device-less connection? Answer to those questions will certainly lead to new questions.

> I'm not sure everything has to be hard config option if we can find a
> reasonable way to make NM get out of the way.

Agreed.

(In reply to comment #10)
> (In reply to comment #9)
> > The presence of /etc/hostname should prevent NM from changing the hostname as
> > long as the hostname is the same as what's in /etc/hostname.
> 
> Which makes me curious about the rationale why /etc/hostname with different
> value shouldn't prevent NetworkManager from setting it. It might be reasonable
> to have this documented in the manual page together with all other files that
> are used by NetworkManager.

/etc/hostname is the persistent machine hostname which is set at startup by systemd and should always be the machine hostname.  If that file is populated then NetworkManager will always use that hostname and ignore any hostname from DHCP or other sources.

Changing your hostname on-the-fly without updating /etc/hostname may cause the runtime hostname to be overwritten.  The presence of /etc/hostname *is* the indication that you want to tell NM not to touch the hostname, because you've set a persistent hostname that you want always used.  I'm not sure if there's a case to be made for temporarily having your hostname different than what's in /etc/hostname.  The hostname should almost always be set at install time and usually never changed, except occasionally.

Note that changing your hostname, especially on the fly, can make X break in unpredictable ways, or make your mail server break or your java database install (yes, really), or any number of other things that depend on the hostname but shouldn't.

Honestly I don't care much about hostname stuff, and we should probably get out of the business of setting the hostname altogether.  I'm fine with hostnamed or whatever it is, but we won't always be running on systems that have that utility and we cannot depend on it so we have to have stuff that sets it ourselves in the absence of hostnamed.

> > We could add "default" as an option and define that to be
> > "yes, unless changed by the global carrier-detect setting", such that if some
> > user really wanted "yes" no matter what the global default setting was, they
> > could set that.
> 
> Could be. With the exception that I would rephrase it a little bit so that
> 'default' means use the global option and the global option itself defaults to
> yes. Might as well be called 'global' instead of 'default' then.

I think "global" is confusing here, and that documentation of what 'default' means is enough.

> But what I'm curious is whether there is a use case for that. And how does the
> global carrier-detect=no interact with DHCP.

That's a very good question.  But in this case, if you set your server up to ignore carrier, and your connection has DHCP, and you have no cable plugged in, then perhaps you should expect DHCP to fail?  If we do the proposal where we keep a wired interface "waiting" all the time and then kick off IP configuration (or renew) when the carrier happens, then this wouldn't be a problem.  Basically the "separate carrier from connectivity" discussion.

> > For default route...
> > We could also work around this too: if NM finds a default route when it starts (or
> > one is added after NM starts) that does not point to a device NM manages, then
> > perhaps we stop touching the default route until that route goes away?
> 
> The default route at NetworkManager's start constitutes part of a runtime
> connection if it can be matched with a device somehow and should be taken over
> as such.

Agreed; though this gets into how the default route gets determined.  This data isn't currently part of the NMConnection data at all, but is part of the policy that NM keeps at runtime; obviously not persistent.

So doing this would require some modification of the policy code to read existing "default" connection/device and then preserve that until some network events change.  Unfortunately ifcfg files have no good way of expressing interface "priority" and so there's no way to make this persistent.  We could certainly create this concept and extend ifcfg file format to represent this.  This does create additional complexity with connection priority though, especially in GUI editors.

> But a default route that can't be traced to a specific device should probably
> be handled as an existing configuration, too. Maybe a special active
> device-less connection? Answer to those questions will certainly lead to new
> questions.
> 
> > I'm not sure everything has to be hard config option if we can find a
> > reasonable way to make NM get out of the way.
> 
> Agreed.

Further discussion of defualt route/priority, as an aside...  There's two cases for priority here.

1) per-connection priority (eg, always try "Red Hat Guest" first instead of "Starbucks" regardless of which one I last connected to) which really has nothing to do with the default route, but is still important in many use-cases, especially wifi

2) device-type priority (eg WiFi should always get a higher priority default route than Bluetooth/WWAN becuase I use WWAN as a failover connection if the WiFi goes down) which does have to do with the default route.


#2 explicitly does not include priority within device types (eg, always prefer eth0 over eth1), as this is already done via DEFROUTE=yes/no in the ifcfg files for that specific connection.  It's more about whether to prefer wlan0 over eth0 in certain cases.

(In reply to comment #11)
> /etc/hostname is the persistent machine hostname which is set at startup by
> systemd and should always be the machine hostname.  If that file is populated
> then NetworkManager will always use that hostname and ignore any hostname from
> DHCP or other sources.

Good. This is what I actually thought.

> Changing your hostname on-the-fly without updating /etc/hostname may cause the
> runtime hostname to be overwritten.  The presence of /etc/hostname *is* the
> indication that you want to tell NM not to touch the hostname, because you've
> set a persistent hostname that you want always used.

Correct.

> I'm not sure if there's a
> case to be made for temporarily having your hostname different than what's in
> /etc/hostname.

I don't think so. But it should be carefully documented.

> Note that changing your hostname, especially on the fly, can make X break in
> unpredictable ways, or make your mail server break or your java database
> install (yes, really), or any number of other things that depend on the
> hostname but shouldn't.

Even bigger reason to avoid disobeying /etc/hostname at all times if it's present. That's why I was curious why you previously limited it to the case where current hostname matches /etc/hostname.
 
> > Could be. With the exception that I would rephrase it a little bit so that
> > 'default' means use the global option and the global option itself defaults to
> > yes. Might as well be called 'global' instead of 'default' then.
> 
> I think "global" is confusing here, and that documentation of what 'default'
> means is enough.

No big deal.

> > But what I'm curious is whether there is a use case for that. And how does the
> > global carrier-detect=no interact with DHCP.
> 
> That's a very good question.  But in this case, if you set your server up to
> ignore carrier, and your connection has DHCP, and you have no cable plugged in,
> then perhaps you should expect DHCP to fail?

Sounds like going against the administrators' will.

I'm afraid that in the case of static IP config, carrier doesn't really matter, which makes me ask why to require it even on desktops. Remember that static configuration is explicit and if the operator set it up, he most probably wants it.

And in the case of DHCP configuration, a server administrator might probably want to detect carrier-on to start the DHCP transaction but what he might want to avoid is the reaction to the carrier-off. BTW, a new carrier-on after carrier-off might be a good time for a RENEW.

> If we do the proposal where we
> keep a wired interface "waiting" all the time and then kick off IP
> configuration (or renew) when the carrier happens, then this wouldn't be a
> problem.  Basically the "separate carrier from connectivity" discussion.

Yep.

> > > For default route...
> > > We could also work around this too: if NM finds a default route when it starts (or
> > > one is added after NM starts) that does not point to a device NM manages, then
> > > perhaps we stop touching the default route until that route goes away?
> > 
> > The default route at NetworkManager's start constitutes part of a runtime
> > connection if it can be matched with a device somehow and should be taken over
> > as such.
> 
> Agreed; though this gets into how the default route gets determined.  This data
> isn't currently part of the NMConnection data at all, but is part of the policy
> that NM keeps at runtime; obviously not persistent.
> 
> So doing this would require some modification of the policy code to read
> existing "default" connection/device and then preserve that until some network
> events change.  Unfortunately ifcfg files have no good way of expressing
> interface "priority" and so there's no way to make this persistent.  We could
> certainly create this concept and extend ifcfg file format to represent this. 
> This does create additional complexity with connection priority though,
> especially in GUI editors.

Let's get back to it when we start actually working on it.

(In reply to comment #8)
> The problem with having this be triggered only by the existence of
> /etc/hostname is that there's no obvious place to document that fact...

Actually, we should have a document of some sort (man page?) explaining exactly what sorts of automatic behavior NM does, under what circumstances. I think sysadmin types would like that.

(In reply to comment #9)
> The problem we have with global carrier detect is that "default" isn't an
> accepted value for NM_SETTING_WIRED_CARRIER_DETECT.

Sure, but if we went with Pavel's suggestion of "disable-carrier-detect=[true|false]", then that doesn't matter.

(In reply to comment #13)
> I'm afraid that in the case of static IP config, carrier doesn't really matter,
> which makes me ask why to require it even on desktops.

1) If the carrier loss was unintentional, you want to make sure the user knows about it; but this can be dealt with by separating the "low-level" connectivity state (IFF_UP, etc) from the "high-level" connectivity state (NM_STATE_CONNECTED_GLOBAL), as we'd talked about.

2) The DNS 30-second timeout problem. Although... couldn't we could just write out an empty resolv.conf?

(In reply to comment #9)
> We could also work around this too: if NM finds a default route when it
> starts (or one is added after NM starts) that does not point to a device
> NM manages, then perhaps we stop touching the default route until that
> route goes away?  I'm not sure everything has to be hard config option if
> we can find a reasonable way to make NM get out of the way.

I wasn't thinking about non-interference, I was thinking about predictability; ie, the default route is via some ethernet device that NM is managing, and the admin just wants to guarantee that the default route stays there, regardless of what happens with other connections. But maybe that isn't needed. (Since initscripts already don't guarantee that anyway.)

the danw/config branch is ready for review; it doesn't actually implement anything server-mode-ish yet, but it implements the multiple config files

* config: fix documentation of --log-domains

- indent whitespace in nm_config_get_options() is wrong

- callers of nm_logging_all_levels_to_string() leak the string, right?  Shouldn't this just be a "static GString" that we build once and return a const* pointer to the data?  Callers can then strdup() if they need to.

- same for all_domains_to_string()


* settings: use NMConfig directly rather than reparsing NetworkManager.conf

- nm_config_get_ethernet_can_auto_default() and nm_config_set_ethernet_no_auto_default() appear to only handle ARPHRD_ETHER  but the original code copied from nm-settings.c handles Infiniband too by checking  nm_utils_hwaddr_type().  Unintentional omission?


(last commit review coming...)

(In reply to comment #16)
> - nm_config_get_ethernet_can_auto_default() and
> nm_config_set_ethernet_no_auto_default() appear to only handle ARPHRD_ETHER 
> but the original code copied from nm-settings.c handles Infiniband too by
> checking  nm_utils_hwaddr_type().  Unintentional omission?

No, wired-vs-ethernet terminology mixup. I saw that nm-settings.c checked NM_IS_DEVICE_WIRED(), and was thinking that meant "only Ethernet", rather than "only Ethernet, InfiniBand, bridges and bonds".

But really, it *should* be checking NM_IS_DEVICE_ETHERNET() anyway, so I think I'm just going to fix nm-settings.c (though I also need to make sure it doesn't blow up if you ended up with an InfiniBand hwaddr in your no-auto-default).

(In reply to comment #17)
> No, wired-vs-ethernet terminology mixup. I saw that nm-settings.c checked
> NM_IS_DEVICE_WIRED(), and was thinking that meant "only Ethernet", rather than
> "only Ethernet, InfiniBand, bridges and bonds".
> 
> But really, it *should* be checking NM_IS_DEVICE_ETHERNET() anyway, so I think
> I'm just going to fix nm-settings.c (though I also need to make sure it doesn't
> blow up if you ended up with an InfiniBand hwaddr in your no-auto-default).

Thanks. This is the next step to get finally rid of NMDeviceWired.

Last patch looks good too.

OK, updated danw/config with some changes:

  1. There is now an NMConfigDevice interface which is used by
     nm_config_get_ethernet_can_auto_default() /
     nm_config_set_ethernet_no_auto_default(). (This will also be
     used by nm_config_get_ignore_carrier() later on.)

     Basically, this is just a wrapper around NMDevice to avoid
     having the config/ subdir depend on that.

     An side-effect of this change is that no-auto-default (and
     eventually ignore-carrier) now recognizes more kinds of things:

       no-auto-default= 00:11:22:33:44:55, eth0, mac:aa:bb:cc:dd:ee:ff,
         interface-name:eth1, s390-subchannels:whatever

     (whitespace added for clarity). Another side effect of this
     change is that nm_device_spec_match_list() now recognizes "*".

     (The changes related to this are mostly in a1920fe "config: add
     NMConfigDevice", and the no-auto-default bits of c6c3bd6 "settings:
     use NMConfig directly rather than reparsing NetworkManager.conf")


  2. NM no longer rewrites NetworkManager.conf to mark devices
     no-auto-default. Instead, it uses $nmstatedir/no-auto-default.state.
     Also, it no longer rereads NM.conf (or no-auto-default.state) every
     time it checks if a device is no-auto-default. Also added a test
     to test-config for this.

     (Part of c6c3bd6 "settings: use NMConfig directly rather than
     reparsing NetworkManager.conf")


  3. Made the suggested fixes to nm_logging_all_levels_to_string() and
     nm_logging_all_domains_to_string() (and the indentation fixes)

     (d416392 "config: fix documentation of --log-domains")


  4. Clarified that nm-default-wired-connection.c is ethernet-only,
     and simplified a little bit of code to not gratuitously pass the
     MAC address around to people who don't need it any more.

     (00cb808 "settings: clarify that NMDefaultWiredConnection is
     ethernet-only" and c6c3bd6 "settings: use NMConfig directly rather
     than reparsing NetworkManager.conf")

pushed a single additional commit to danw/config (updating NetworkManager.conf.5) and additionally pushed danw/ignore-carrier, which reverts the previously-added carrier-detect stuff and adds a new NM.conf ignore-carrier key instead.

dcbw: I know we talked about that branch briefly on IRC, and there were possibly some issues with bond/bridge activation, but I don't remember the details.

Oh, finally, I wrote out a sample "server.conf" file that documents the keys related to this bug, but I'm not sure where in the tree to put it (data/ I guess?) or where to install it to... do we want to create $sharedir/NetworkManager/examples/ or something?

* danw/ignore-carrier
I don't see any issues.

* danw/config
No issues encountered. Though, it was just a quick skimming over.


(In reply to comment #21)
> Oh, finally, I wrote out a sample "server.conf" file that documents the keys
> related to this bug, but I'm not sure where in the tree to put it (data/ I
> guess?) or where to install it to... do we want to create
> $sharedir/NetworkManager/examples/ or something?

Yeah, I'd use data/ dir. Or maybe create examples/configs and then install whole examples directory.

(In reply to comment #22)
> Yeah, I'd use data/ dir. Or maybe create examples/configs and then install
> whole examples directory.

Agreed. I think we should have a 'examples' directory for various stuff and that we should install that directory to /usr/share/doc. Distribution are free to put all the documentation into a separate doc package.

> config: add NMConfigDevice

That suggests we have some device-specific options (I know we do). But it would be nice if we have complete information about where the device options come from. Whether it's only NetworkManager.conf / conf.d, and so on.

I'm just curious whether we could have something like “instructions to test” and a list of what's new for the config. It could be a commit message, a README file in the src/config directory, whatever. I consider “no” a valid answer, too.

ad ignore-carrier:

Could you please look at

http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=pavlix/platform-use&id=869f5518e13d01f7b70c5972c1bbbc1749ecf197

and tell me what's needed to incorporate it into your code/branch. Otherwise both of us keep rewriting stuff that should have been already removed indefinitely.

NMPlatform will provide you with both carrier detection and information about carrier detection support. Until then, the old ways should be used. But I believe *all* carrier detection stuff should be removed from *all* NMDevice subclasses.

I don't say my patch above is perfect, so feel free to adjust it. I would be more than happy if the carrier in subclasses mess is gone before I revisit the pavlix/platform-use patchset wich was separated from the pavlix/platform for the very reason of not being able to spend so much time keeping up with the 'master' changes.

> config: fix documentation of --log-domains

While it looks like GCC automatically initializes static pointers to NULL, do you mind making that explicit?  It's not always the case depending on which compiler you use, what C standard you use, and what system you're on I think.

Also, while you're there, maybe G_UNLIKELY (!str) in both functions?


> settings: use NMConfig directly rather than reparsing NetworkManager.conf

So I'm a bit confused now.  We're using the same matching for no-auto-default as for unmanaged specs, right?  eg, nm_device_spec_match_list().  But the unmanaged specs stuff requires the "mac:" for hardware addresses, but the no-auto-default stuff doesn't bother with that.

So by my reading the no-auto-default checking in nm_config_get_ethernet_can_auto_default() fails to work, because priv->no_auto_default contains a char** of bare hardware addresses, not "mac:<addr>" and thus that'll fail to match in nm-device's spec_match_list() which calls nm_match_spec_hwaddr() which checks for "mac:".

I simply may not have dug into it enough though, but could you clarify?

Also, nm_config_get_ethernet_can_auto_default() and nm_config_set_ethernet_no_auto_default() as written don't have anything to do with ethernet; they just use the hardware address of the device but don't do any checking on the device type.  I assume that's done in nm-settings.c; but that makes the function names in nm-config.c a bit confusing since they don't actually care what device type they're called for.  And due to that, they'll have a bug if they are called for WWAN or ADSL devices or other devices that don't have a hardware address.

Next, any particular reason the no-auto-default file gets a command line option for the path?  Just because the state file does?  Honestly I'm not sure why we did a path for that, though this does help with testing eventually I'm sure.  Fine to leave it, just curious.

The rest of the changes look OK; I concentrated on the new stuff and just re-skimmed the patches I'd reviewed previously.

danw/ignore-carrier looks fine too, though we should figure out the merge strategy with pavel's code.

(In reply to comment #26)
> Could you please look at
> 
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=pavlix/platform-use&id=869f5518e13d01f7b70c5972c1bbbc1749ecf197
> 
> and tell me what's needed to incorporate it into your code/branch.

Other than reverting the previous carrier-detect stuff, the ignore-carrier branch doesn't change how carrier is monitored, it just changes how NMDevice *reacts* to changes in carrier. So it won't be trying to revert any of your changes. In that commit, I think link_connected() and link_disconnected() would need to be changed to return immediately if the device has ignore-carrier set, and act_stage3_ip4_config_start(), act_stage3_ip6_config_start(), is_available() and can_interrupt_activation() would also need to be adjusted to only consider carrier if ignore-carrier wasn't set (basically, the changes from nm-device-wired.c in my patch would move into nm-device.c in your branch).

(In reply to comment #27)
> > config: fix documentation of --log-domains
> 
> While it looks like GCC automatically initializes static pointers to NULL, do
> you mind making that explicit?  It's not always the case depending on which
> compiler you use, what C standard you use, and what system you're on I think.

I can initialize them to NULL if you like, but that behavior is actually guaranteed by the C standard, as far back as C89.

> > settings: use NMConfig directly rather than reparsing NetworkManager.conf
> 
> So I'm a bit confused now.  We're using the same matching for no-auto-default
> as for unmanaged specs, right?  eg, nm_device_spec_match_list().  But the
> unmanaged specs stuff requires the "mac:" for hardware addresses, but the
> no-auto-default stuff doesn't bother with that.

See nm_config_device_spec_match_list(); it prepends "mac:" to strings that look like MAC addresses and "interface-name:" to strings that look like interface names before passing them to nm_device_spec_match_list().

Maybe I should rename that function to make it clearer to that it's not a simple 1-1 wrapper around the NMDevice function?

> Next, any particular reason the no-auto-default file gets a command line option
> for the path?  Just because the state file does?  Honestly I'm not sure why we
> did a path for that, though this does help with testing eventually I'm sure. 

Half "because the state file does" and half "because it makes it easier to test in test-config.c". I'm willing to remove it and do that some other way if you'd rather. (Note that I used the goption flag to hide that option, so it won't actually show up in --help.)

(In reply to comment #29)
> basically, the changes from nm-device-wired.c in my patch would move into
> nm-device.c in your branch

Sure, that's where the additional repeated work comes from. If all was already moved to nm-device.c, the collaboration would work much more smoothly.

(In reply to comment #30)
> (In reply to comment #27)
> > > config: fix documentation of --log-domains
> > 
> > While it looks like GCC automatically initializes static pointers to NULL, do
> > you mind making that explicit?  It's not always the case depending on which
> > compiler you use, what C standard you use, and what system you're on I think.
> 
> I can initialize them to NULL if you like, but that behavior is actually
> guaranteed by the C standard, as far back as C89.
> 
> > > settings: use NMConfig directly rather than reparsing NetworkManager.conf
> > 
> > So I'm a bit confused now.  We're using the same matching for no-auto-default
> > as for unmanaged specs, right?  eg, nm_device_spec_match_list().  But the
> > unmanaged specs stuff requires the "mac:" for hardware addresses, but the
> > no-auto-default stuff doesn't bother with that.
> 
> See nm_config_device_spec_match_list(); it prepends "mac:" to strings that look
> like MAC addresses and "interface-name:" to strings that look like interface
> names before passing them to nm_device_spec_match_list().
> 
> Maybe I should rename that function to make it clearer to that it's not a
> simple 1-1 wrapper around the NMDevice function?
> 
> > Next, any particular reason the no-auto-default file gets a command line option
> > for the path?  Just because the state file does?  Honestly I'm not sure why we
> > did a path for that, though this does help with testing eventually I'm sure. 
> 
> Half "because the state file does" and half "because it makes it easier to test
> in test-config.c". I'm willing to remove it and do that some other way if you'd
> rather. (Note that I used the goption flag to hide that option, so it won't
> actually show up in --help.)

Thanks for clarifying, I have no further suggestions.  danw/config branch looks good.

committed

*** Bug 673537 has been marked as a duplicate of this bug. ***