[v2] set simple network setup via the system configuration submenu

This patch allows the setup of simple /etc/network/interfaces via the
configuration menus instead of using an overlay

* supports manual ipv4 configuration
* supports dhcp configuration

Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>

---

This patch is here to avoid having to do an overlay for the most common
cases (ipv4 with fixed IP or DHCP)

It can be made more complex (second network if, support ipv6) depending on
what people want/need, but I want to keep it simple. The point is to avoid
having to tweak overlays to change stuff that everybody needs to change for
prototyping

When networkd is enabled, this option will be deactivated. Networkd support
is tricky to get right on first approximation. It can be added later if it
is deemed usefull
---

v1->v2 :
* moved to TARGET_FINALIZE
* removed support for lo. It should always be on.
* reworked default parameters
---
 support/scripts/generate-interfaces-ifconfig.sh | 70 +++++++++++++++++++++++++
 system/Config.in                                | 66 +++++++++++++++++++++++
 system/system.mk                                |  5 ++
 3 files changed, 141 insertions(+)
 create mode 100755 support/scripts/generate-interfaces-ifconfig.sh

Hi,

2014-10-20 16:14 GMT+02:00 Jérémy Rosen <jeremy.rosen@openwide.fr>:
> This patch allows the setup of simple /etc/network/interfaces via the
> configuration menus instead of using an overlay
>
> * supports manual ipv4 configuration
> * supports dhcp configuration
>
> Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
>
> ---
>
> This patch is here to avoid having to do an overlay for the most common
> cases (ipv4 with fixed IP or DHCP)
>
> It can be made more complex (second network if, support ipv6) depending on
> what people want/need, but I want to keep it simple. The point is to avoid
> having to tweak overlays to change stuff that everybody needs to change for
> prototyping
>
> When networkd is enabled, this option will be deactivated. Networkd support
> is tricky to get right on first approximation. It can be added later if it
> is deemed usefull

I'm willing to add networkd support once your work has been merged ;)

Just for reference, my notes on what needs to be done:
* no loopback setup, systemd does it on its own
* creation of a ".network" file - .ini file (equivalent to
/etc/network/interfaces), requires conversion of the netmask to cidr
format (255.255.255.0 => 24 a.s.o.)
* -optionally- disable "predictable" network interface renaming
("eth0"=>"enp2s0") [0] so that one can actually rely on eth0's
existence

Did I miss something here?


[0] http://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/


> ---
>
> v1->v2 :
> * moved to TARGET_FINALIZE
> * removed support for lo. It should always be on.
> * reworked default parameters
> ---
>  support/scripts/generate-interfaces-ifconfig.sh | 70 +++++++++++++++++++++++++
>  system/Config.in                                | 66 +++++++++++++++++++++++
>  system/system.mk                                |  5 ++
>  3 files changed, 141 insertions(+)
>  create mode 100755 support/scripts/generate-interfaces-ifconfig.sh
>
> diff --git a/support/scripts/generate-interfaces-ifconfig.sh b/support/scripts/generate-interfaces-ifconfig.sh
> new file mode 100755
> index 0000000..873e824
> --- /dev/null
> +++ b/support/scripts/generate-interfaces-ifconfig.sh
> @@ -0,0 +1,70 @@
> +#!/bin/sh
> +
> [...]
> +
> +function do_generate_interfaces

That's a "bashism", which breaks the script when /bin/sh is linked to
ash,dash,.... Either set the hashbang to "#!/bin/bash" or use
"do_generate_interfaces ()" instead of "function
do_generate_interfaces".

> +{
> +       echo "# interface file auto-generated by buildroot"
> +       echo
> +       echo "auto lo"
> +       echo "iface lo inet loopback"
> +       echo
> +
> +       if [ $BR2_SIMPLE_NETWORK ] ; then
> +               if [ -z $BR2_SIMPLE_NETWORK_NAME ] ; then

It's been already discussed a bit in the v1 thread, but I'd recommend
to quote variables that are string-type in kconfig.

The shell expands the expression here (word-splitting, globbing) and
the effective command might be unexpected. It'd usually be caused by
misconfiguration (*), which I wouldn't care about in this script, but
why let the shell expand a value that you want to use as-is?

(*): Example: BR2_SIMPLE_NETWORK_NAME="-a 2" => "[ -z -a 2 ]", returns
0 in bash, but 2 in [d]ash...

As a not-so-simple corner case, for systemd-networkd, it would be
legal to set BR2_SIMPLE_NETWORK_NAME="eth*", which is just a matter of
finding the right directory to break the "[]"-statement. Just talking
about the "[]" here, overall it would still work in your script,
because "[ -z ... ]" returns nonzero on syntax error (and screams to
stderr!) and you're checking for 0.

> +                       echo ERROR no name specified for first network interface
> +                       exit 1
> +               fi
> +               echo "auto  $BR2_SIMPLE_NETWORK_NAME"
> +               if [ $BR2_SIMPLE_NETWORK_IPV4_DHCP ] ; then
> +                       echo "iface $BR2_SIMPLE_NETWORK_NAME inet dhcp"
> +               elif [ $BR2_SIMPLE_NETWORK_IPV4_MANUAL ] ; then
> +                       echo "iface $BR2_SIMPLE_NETWORK_NAME inet static"
> +
> +                       if [ -z $BR2_SIMPLE_NETWORK_IPV4_ADDRESS ] ; then
> +                               echo ERROR BR2_SIMPLE_NETWORK_IPV4_ADDRESS not set 1>&2
> +                               exit 1
> +                       fi
> +                       echo "  address $BR2_SIMPLE_NETWORK_IPV4_ADDRESS"
> +
> +
> +                       if [ -z $BR2_SIMPLE_NETWORK_IPV4_NETMASK ] ; then
> +                               echo ERROR BR2_SIMPLE_NETWORK_IPV4_NETMASK not set 1>&2
> +                               exit 1
> +                       fi
> +                       echo "  netmask $BR2_SIMPLE_NETWORK_IPV4_NETMASK"
> +
> +                       if [ $BR2_SIMPLE_NETWORK_IPV4_BROADCAST ] ; then
> +                               echo "  broadcast $BR2_SIMPLE_NETWORK_IPV4_BROADCAST"
> +                       fi
> +                       if [ $BR2_SIMPLE_NETWORK_IPV4_GATEWAY ] ; then
> +                               echo "  gateway $BR2_SIMPLE_NETWORK_IPV4_GATEWAY"
> +                       fi
> +               fi
> +       fi
> +}

Please consider splitting up data validation (basically everything
that leads to "exit 1") and output generation. It adds a few
duplicated checks, but it'll be easier to add support for networkd.

> +
> +mkdir -p $TARGET_DIR/etc/network/
> +do_generate_interfaces > $TARGET_DIR/etc/network/interfaces
>
> [snip]

Hey, thanks for the review, Bash and Makefile are the two programming
languages I was never able to really master

----- Mail original -----
> Hi,
> 
> 2014-10-20 16:14 GMT+02:00 Jérémy Rosen <jeremy.rosen@openwide.fr>:
> > This patch allows the setup of simple /etc/network/interfaces via
> > the
> > configuration menus instead of using an overlay
> >
> > * supports manual ipv4 configuration
> > * supports dhcp configuration
> >
> > Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
> >
> > ---
> >
> > This patch is here to avoid having to do an overlay for the most
> > common
> > cases (ipv4 with fixed IP or DHCP)
> >
> > It can be made more complex (second network if, support ipv6)
> > depending on
> > what people want/need, but I want to keep it simple. The point is
> > to avoid
> > having to tweak overlays to change stuff that everybody needs to
> > change for
> > prototyping
> >
> > When networkd is enabled, this option will be deactivated. Networkd
> > support
> > is tricky to get right on first approximation. It can be added
> > later if it
> > is deemed usefull
> 
> I'm willing to add networkd support once your work has been merged ;)
> 

I've already been working on it, it's indeed quite easy but there is 
a tricky question that needs resolving, so I didn't include it in this
patch. Basically I have a problem with dealing with default values with
static IP settings

* The script need to fail if no value is provided for address
* The script needs to build correctly with default values (autobuilder)
* There is no "correct" default values to use
* We should not let an unconfigured Adress on the network.

Right now, ADDRESS defaults to 0.0.0.0 which is interpreted by net-utils
as "incorrect address, don't configure the interface" (so it ends up
enabled but with no IP address) I'm happy with that, it's ok with my 
requirement

But I could not find an equivalent setting for networkd. 0.0.0.0 is 
interpreted as "automatically find an IP address" and I couldn't find
any documentation on how it does that (does it check the network for
collision ? It seems to use the netmask to choose the base address and
it makes sure there is no conflict with other interfaces on the same
machine, but that's all I could find)

So right now I'm open to suggestions on that particular problem, generating
the actual .network file shouldn't be very hard (I temptatively call it
busybox.networks rather than <name>.network to be sure to overwrite it 
on the next iteration of the script)


> Just for reference, my notes on what needs to be done:
> * no loopback setup, systemd does it on its own

yes, I found that out, it's simple to deal with since loopback is not a config
option for net-utils anymore

> * creation of a ".network" file - .ini file (equivalent to
> /etc/network/interfaces), requires conversion of the netmask to cidr
> format (255.255.255.0 => 24 a.s.o.)

I found how to do that in shell script somewher on stackoverflow, so
not a problem

> * -optionally- disable "predictable" network interface renaming
> ("eth0"=>"enp2s0") [0] so that one can actually rely on eth0's
> existence
> 

that's a good idea, but predicatable interface names seems to already
be disabled on buildroot builds. I wasn't able to find out why and I
didn't dig since I was dropping networkd support at that point. It
seems that there is something weird there...

> Did I miss something here?
> 
> 
> [0]
> http://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
> 
> 
> > ---
> >
> > v1->v2 :
> > * moved to TARGET_FINALIZE
> > * removed support for lo. It should always be on.
> > * reworked default parameters
> > ---
> >  support/scripts/generate-interfaces-ifconfig.sh | 70
> >  +++++++++++++++++++++++++
> >  system/Config.in                                | 66
> >  +++++++++++++++++++++++
> >  system/system.mk                                |  5 ++
> >  3 files changed, 141 insertions(+)
> >  create mode 100755 support/scripts/generate-interfaces-ifconfig.sh
> >
> > diff --git a/support/scripts/generate-interfaces-ifconfig.sh
> > b/support/scripts/generate-interfaces-ifconfig.sh
> > new file mode 100755
> > index 0000000..873e824
> > --- /dev/null
> > +++ b/support/scripts/generate-interfaces-ifconfig.sh
> > @@ -0,0 +1,70 @@
> > +#!/bin/sh
> > +
> > [...]
> > +
> > +function do_generate_interfaces
> 
> That's a "bashism", which breaks the script when /bin/sh is linked to
> ash,dash,.... Either set the hashbang to "#!/bin/bash" or use
> "do_generate_interfaces ()" instead of "function
> do_generate_interfaces".
> 

will fix

> > +{
> > +       echo "# interface file auto-generated by buildroot"
> > +       echo
> > +       echo "auto lo"
> > +       echo "iface lo inet loopback"
> > +       echo
> > +
> > +       if [ $BR2_SIMPLE_NETWORK ] ; then
> > +               if [ -z $BR2_SIMPLE_NETWORK_NAME ] ; then
> 
> It's been already discussed a bit in the v1 thread, but I'd recommend
> to quote variables that are string-type in kconfig.
> 

will do

> The shell expands the expression here (word-splitting, globbing) and
> the effective command might be unexpected. It'd usually be caused by
> misconfiguration (*), which I wouldn't care about in this script, but
> why let the shell expand a value that you want to use as-is?
> 
> (*): Example: BR2_SIMPLE_NETWORK_NAME="-a 2" => "[ -z -a 2 ]",
> returns
> 0 in bash, but 2 in [d]ash...
> 
> As a not-so-simple corner case, for systemd-networkd, it would be
> legal to set BR2_SIMPLE_NETWORK_NAME="eth*", which is just a matter
> of
> finding the right directory to break the "[]"-statement. Just talking
> about the "[]" here, overall it would still work in your script,
> because "[ -z ... ]" returns nonzero on syntax error (and screams to
> stderr!) and you're checking for 0.
> 
> > +                       echo ERROR no name specified for first
> > network interface
> > +                       exit 1
> > +               fi
> > +               echo "auto  $BR2_SIMPLE_NETWORK_NAME"
> > +               if [ $BR2_SIMPLE_NETWORK_IPV4_DHCP ] ; then
> > +                       echo "iface $BR2_SIMPLE_NETWORK_NAME inet
> > dhcp"
> > +               elif [ $BR2_SIMPLE_NETWORK_IPV4_MANUAL ] ; then
> > +                       echo "iface $BR2_SIMPLE_NETWORK_NAME inet
> > static"
> > +
> > +                       if [ -z $BR2_SIMPLE_NETWORK_IPV4_ADDRESS ]
> > ; then
> > +                               echo ERROR
> > BR2_SIMPLE_NETWORK_IPV4_ADDRESS not set 1>&2
> > +                               exit 1
> > +                       fi
> > +                       echo "  address
> > $BR2_SIMPLE_NETWORK_IPV4_ADDRESS"
> > +
> > +
> > +                       if [ -z $BR2_SIMPLE_NETWORK_IPV4_NETMASK ]
> > ; then
> > +                               echo ERROR
> > BR2_SIMPLE_NETWORK_IPV4_NETMASK not set 1>&2
> > +                               exit 1
> > +                       fi
> > +                       echo "  netmask
> > $BR2_SIMPLE_NETWORK_IPV4_NETMASK"
> > +
> > +                       if [ $BR2_SIMPLE_NETWORK_IPV4_BROADCAST ] ;
> > then
> > +                               echo "  broadcast
> > $BR2_SIMPLE_NETWORK_IPV4_BROADCAST"
> > +                       fi
> > +                       if [ $BR2_SIMPLE_NETWORK_IPV4_GATEWAY ] ;
> > then
> > +                               echo "  gateway
> > $BR2_SIMPLE_NETWORK_IPV4_GATEWAY"
> > +                       fi
> > +               fi
> > +       fi
> > +}
> 
> Please consider splitting up data validation (basically everything
> that leads to "exit 1") and output generation. It adds a few
> duplicated checks, but it'll be easier to add support for networkd.
> 

I support networkd with a separate generation script at this point,
but merging the two might be a good idea, i'll look into that...



Thx for the review, i'll wait a couple of days for more feedback as
usual, then i'll respin

> > +
> > +mkdir -p $TARGET_DIR/etc/network/
> > +do_generate_interfaces > $TARGET_DIR/etc/network/interfaces
> >
> > [snip]
> 
> --
> André
>

Dear Jérémy Rosen,

On Mon, 20 Oct 2014 16:14:46 +0200, Jérémy Rosen wrote:
> This patch allows the setup of simple /etc/network/interfaces via the
> configuration menus instead of using an overlay
> 
> * supports manual ipv4 configuration
> * supports dhcp configuration
> 
> Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
> 
> ---
> 
> This patch is here to avoid having to do an overlay for the most common
> cases (ipv4 with fixed IP or DHCP)
> 
> It can be made more complex (second network if, support ipv6) depending on
> what people want/need, but I want to keep it simple. The point is to avoid
> having to tweak overlays to change stuff that everybody needs to change for
> prototyping
> 
> When networkd is enabled, this option will be deactivated. Networkd support
> is tricky to get right on first approximation. It can be added later if it
> is deemed usefull

I certainly don't like to push back stuff, but I'm wondering if this
isn't too much complexity compared to just having people use an overlay
containing their own /etc/network/interfaces. We will never be able to
handle all the possible cases, with multiple network interfaces, hook
scripts called before/after bringing up/down interfaces, etc.

What is the opinion of other Buildroot developers?

Best regards,

Thomas

----- Mail original -----

[<snip>]

> 
> I certainly don't like to push back stuff, but I'm wondering if this
> isn't too much complexity compared to just having people use an
> overlay
> containing their own /etc/network/interfaces. We will never be able
> to
> handle all the possible cases, with multiple network interfaces, hook
> scripts called before/after bringing up/down interfaces, etc.
> 
> What is the opinion of other Buildroot developers?
> 


just to clarify...

the point of the patch is not to allow all possible configurations that
network/interfaces can handle, but just allow to easily set an IP address
or enable DHCP without having to add an overlay. For anything more
complex an overlay is the way to go.

I originally thought that putting more stuff in would be a good
idea but I have revised my position on that and now I think it
should be limited to the minimal required subset

* static ipv4
* ipv4 dhcp
* networkd support

if ipv6 becomes popular enough it could trivially be added but 
let's keep this for a followup


Regards

Jérémy

Jeremy, All,

On 2014-10-21 10:50 +0200, Jeremy Rosen spake thusly:
> [<snip>]
> > I certainly don't like to push back stuff, but I'm wondering if this
> > isn't too much complexity compared to just having people use an
> > overlay
> > containing their own /etc/network/interfaces. We will never be able
> > to
> > handle all the possible cases, with multiple network interfaces, hook
> > scripts called before/after bringing up/down interfaces, etc.
> > 
> > What is the opinion of other Buildroot developers?
> 
> just to clarify...
> 
> the point of the patch is not to allow all possible configurations that
> network/interfaces can handle, but just allow to easily set an IP address
> or enable DHCP without having to add an overlay. For anything more
> complex an overlay is the way to go.
> 
> I originally thought that putting more stuff in would be a good
> idea but I have revised my position on that and now I think it
> should be limited to the minimal required subset
> 
> * static ipv4
> * ipv4 dhcp
> * networkd support
> 
> if ipv6 becomes popular enough it could trivially be added but 
> let's keep this for a followup

I've meant to review this earlier, but then I always backed off because
I'm not too happy with this kind of stuff, without being able to
pinpoint the exact underlying reason. Call it gut feelings if you
want...

Although I like it when we present user-friendly configuration options,
and this certainly is to some extent, I worry about the option-creep we
are going to have with this kind of options.

I too am also responsible for this option-creep, so I'm noone to really
blame any one trying for such a change... ;-)

Still, I would like to see where we going with that kind of setup, and
how far we should go (or rather, where we should stop).

Certainly, this IPv4-only options are not too intrusive, yet they only
cover a basic setup.

Now, for some actual, high-level review (mostly about cosmetics):

  - multiple spearation lines, or spaces; this breaks the current flow of
    the "code", and is not in-line with the rest of the file. Please use
    only one line to separate /sections/, and do not double spaces,
    especially in the help texts.

  - I would make all into a single choice, with three entries:
    - don't generate network settings
    - use DHCP
    - use static

  - Also, we already have a /etc/network/interfaces file in the skeleton,
    with lo in it; I'd prefer you keep it that way, and not generate the
    lo entry, and just append the 'eth0' entry.

Regards,
Yann E. MORIN.

Yann, all...


> 
> I've meant to review this earlier, but then I always backed off
> because
> I'm not too happy with this kind of stuff, without being able to
> pinpoint the exact underlying reason. Call it gut feelings if you
> want...
> 
> Although I like it when we present user-friendly configuration
> options,
> and this certainly is to some extent, I worry about the option-creep
> we
> are going to have with this kind of options.
> 
> I too am also responsible for this option-creep, so I'm noone to
> really
> blame any one trying for such a change... ;-)
> 
> Still, I would like to see where we going with that kind of setup,
> and
> how far we should go (or rather, where we should stop).
> 
> Certainly, this IPv4-only options are not too intrusive, yet they
> only
> cover a basic setup.
> 

For me it's more a case of being able to quickly test stuff with BR
without having to go through the hassle of adding an ovelray etc...
clone, configure, build, flash. Setting the network is a really
basic thing you always need to do (it's not very different from
setting the hostname, which is already in the system submenu)

It's really a shame that I missed posting that as a subject for 
ELCE, but at least you're opening the real debate :) 

This remarks don't block me working on the patch, it only blocks
merging the patch so i'll repost a v3 when it is ready anyway,
but it would be nice to have this discussion going on...

> Now, for some actual, high-level review (mostly about cosmetics):
> 
>   - multiple spearation lines, or spaces; this breaks the current
>   flow of
>     the "code", and is not in-line with the rest of the file. Please
>     use
>     only one line to separate /sections/, and do not double spaces,
>     especially in the help texts.
> 

will do

>   - I would make all into a single choice, with three entries:
>     - don't generate network settings
>     - use DHCP
>     - use static
> 

sounds good, will do...

>   - Also, we already have a /etc/network/interfaces file in the
>   skeleton,
>     with lo in it; I'd prefer you keep it that way, and not generate
>     the
>     lo entry, and just append the 'eth0' entry.
> 

that's very complicated because skeleton is copied very early in the 
build process whereas my script is called in FINALIZE. So if there
are multiple builds without clean between them, the naïve approch
would end up with multiple eth0 lines.

I could mark the autogenerated section, cut it out and replace it
but that sounds very complicated/error prone for just the lo line.

I personally think that the most consistant way to do it is to 
remove /etc/network/interfaces from skeleton entirely and move 
the responsability for that file to the script.

* none : generate lo only
* dchp : generate lo and eth0-dhcp
* static :  generate lo and eth0-static

that would be simpler, easier to maintain (all network conf stuff
in one place) and users can still override it easily with overlays

The only problematic case would be users that use a custom skeleton
to override the default one, but they should probably migrate to
overlays anyway.

So, I will probably remove the file from skeleton unless someone
objects to it since that sounds like the best approch to me...
(if the skeleton reorg make it into buildroot first I will also
adapt accordingly)

As usual I will wait a couple of days before a respin 

Thx for the comments

Regards
Jeremy

> Regards,
> Yann E. MORIN.
> 
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> |  conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
> |               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/
> |  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
> |   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
>

On 21/10/14 10:36, Thomas Petazzoni wrote:
> I certainly don't like to push back stuff, but I'm wondering if this
> isn't too much complexity compared to just having people use an overlay
> containing their own /etc/network/interfaces. We will never be able to
> handle all the possible cases, with multiple network interfaces, hook
> scripts called before/after bringing up/down interfaces, etc.

 Personally I think that the network interfaces is at least as relevant as the
getty, hostname, timezone, default shell, and other system configuration.
Similar to the getty configuration, it's good to have something that gives you a
working system with a simple config without having to create custom files. And
at least for the network interface, the simple configuration will often be
sufficient. getty  on the other hand becomes irrelevant when you create a custom
inittab (i.e. always for a real product).

 So I'm definitely in favour of having something like this. On condition that it
is sufficiently simple, and doesn't attempt to cover all cases.


 Regards,
 Arnout

2014/10/22 Jeremy Rosen <jeremy.rosen@openwide.fr>:
> 
> 
> Hey, thanks for the review, Bash and Makefile are the two programming
> languages I was never able to really master
> 
> ----- Mail original -----
>> Hi,
>>
>> 2014-10-20 16:14 GMT+02:00 Jérémy Rosen <jeremy.rosen@openwide.fr>:
>>> This patch allows the setup of simple /etc/network/interfaces via
>>> the
>>> configuration menus instead of using an overlay
>>>
>>> * supports manual ipv4 configuration
>>> * supports dhcp configuration
>>>
>>> Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
>>>
>>> ---
>>>
>>> This patch is here to avoid having to do an overlay for the most
>>> common
>>> cases (ipv4 with fixed IP or DHCP)
>>>
>>> It can be made more complex (second network if, support ipv6)
>>> depending on
>>> what people want/need, but I want to keep it simple. The point is
>>> to avoid
>>> having to tweak overlays to change stuff that everybody needs to
>>> change for
>>> prototyping
>>>
>>> When networkd is enabled, this option will be deactivated. Networkd
>>> support
>>> is tricky to get right on first approximation. It can be added
>>> later if it
>>> is deemed usefull
>>
>> I'm willing to add networkd support once your work has been merged ;)
>>
> 
> I've already been working on it, it's indeed quite easy but there is 
> a tricky question that needs resolving, so I didn't include it in this
> patch. Basically I have a problem with dealing with default values with
> static IP settings
> 
> * The script need to fail if no value is provided for address
> * The script needs to build correctly with default values (autobuilder)
> * There is no "correct" default values to use
> * We should not let an unconfigured Adress on the network.
> 
> Right now, ADDRESS defaults to 0.0.0.0 which is interpreted by net-utils
> as "incorrect address, don't configure the interface" (so it ends up
> enabled but with no IP address) I'm happy with that, it's ok with my 
> requirement
> 
> But I could not find an equivalent setting for networkd.

In the .network file, omit the "[Address]" section / "Address=" line.
The interface gets upped, but no ip address gets assigned.
I've attached some ".network" file examples below.

> 0.0.0.0 is 
> interpreted as "automatically find an IP address" and I couldn't find
> any documentation on how it does that (does it check the network for
> collision ? It seems to use the netmask to choose the base address and
> it makes sure there is no conflict with other interfaces on the same
> machine, but that's all I could find)
> 

That's all it does: ensure that each automatic address is unique on a single
system, i.e. doesn't clash with any configured addr or any addr that has
already been assigned.

It might be useful for virtual ethernet setups (veth and whatnot), but IMO
that's out of scope for simple network setup, so my suggestion would be to
handle "0.0.0.0" as <not configured> => no "Address=" in the .network file.
It also makes BR2_SIMPLE_NETWORK_IPV4_IPV4_ADDRESS semantically equivalent
for both output formats.

Alternatives, all with the disadvantage of adding unnecessary complexity:
* introduce a dummy value, e.g. "-"
* add another choice to the "Configuration type" prompt,
  BR2_SIMPLE_NETWORK_IPV4_NONE (or similar)


> So right now I'm open to suggestions on that particular problem, generating
> the actual .network file shouldn't be very hard (I temptatively call it
> busybox.networks rather than <name>.network to be sure to overwrite it 
> on the next iteration of the script)
> 

Not sure if it's a binding convention, but these files are usually prefixed
with a number 01..99, e.g. "50-buildroot.network".

> 
>> Just for reference, my notes on what needs to be done:
>> * no loopback setup, systemd does it on its own
> 
> yes, I found that out, it's simple to deal with since loopback is not a config
> option for net-utils anymore
> 
>> * creation of a ".network" file - .ini file (equivalent to
>> /etc/network/interfaces), requires conversion of the netmask to cidr
>> format (255.255.255.0 => 24 a.s.o.)
> 
> I found how to do that in shell script somewher on stackoverflow, so
> not a problem
> 
>> * -optionally- disable "predictable" network interface renaming
>> ("eth0"=>"enp2s0") [0] so that one can actually rely on eth0's
>> existence
>>
> 
> that's a good idea, but predicatable interface names seems to already
> be disabled on buildroot builds. I wasn't able to find out why and I
> didn't dig since I was dropping networkd support at that point. It
> seems that there is something weird there...
> 

It's not disabled, but under certain circumstances it's not "active", for example
if the kernel-provided name is deemed to be stable, e.g. qemu/kvm with virtio netdev. 
You can provoke it by plugging in an usb wlan adapter, which appears as "wlp0s20u10"/...
in 'ip link'/'ifconfig'.
On x86 consumer-grade hardware, the wired interfaces get always renamed, too.

There are several ways to disable it, but the best solution -for me- is
"ln -s /dev/null $(TARGET_DIR)/etc/systemd/network/99-default.link", which still allows
custom interface renaming in /etc/systemd/network/ while disabling the default policy.

---
.network file examples:

# "0.0.0.0"
[Match]
Name=eth0

[Network]
Description=buildroot-configured interface
(EOF)

# any other manual ip addr
[Match]
Name=eth0

[Network]
Description=buildroot-configured interface

[Address]
Address=192.168.1.2/24
Broadcast=192.168.1.255 ## if configured

[Route]
Gateway=192.168.1.1 ## if configured
(EOF)

# dhcp
[Match]
Name=eth0

[Network]
Description=buildroot-configured interface
DHCP=v4
(EOF)

Notes:
* the "Gateway=" line can also be moved to the "[Network]" section
  (+ drop the "[Route]" section)

* "Address=" should have its own section because of "Broadcast="

André, all

----- Mail original -----
> 2014/10/22 Jeremy Rosen <jeremy.rosen@openwide.fr>:
> > 
> > 
> > Hey, thanks for the review, Bash and Makefile are the two
> > programming
> > languages I was never able to really master
> > 
> > ----- Mail original -----
> >> Hi,
> >>
> >> 2014-10-20 16:14 GMT+02:00 Jérémy Rosen
> >> <jeremy.rosen@openwide.fr>:
> >>> This patch allows the setup of simple /etc/network/interfaces via
> >>> the
> >>> configuration menus instead of using an overlay
> >>>
> >>> * supports manual ipv4 configuration
> >>> * supports dhcp configuration
> >>>
> >>> Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
> >>>
> >>> ---
> >>>
> >>> This patch is here to avoid having to do an overlay for the most
> >>> common
> >>> cases (ipv4 with fixed IP or DHCP)
> >>>
> >>> It can be made more complex (second network if, support ipv6)
> >>> depending on
> >>> what people want/need, but I want to keep it simple. The point is
> >>> to avoid
> >>> having to tweak overlays to change stuff that everybody needs to
> >>> change for
> >>> prototyping
> >>>
> >>> When networkd is enabled, this option will be deactivated.
> >>> Networkd
> >>> support
> >>> is tricky to get right on first approximation. It can be added
> >>> later if it
> >>> is deemed usefull
> >>
> >> I'm willing to add networkd support once your work has been merged
> >> ;)
> >>
> > 
> > I've already been working on it, it's indeed quite easy but there
> > is
> > a tricky question that needs resolving, so I didn't include it in
> > this
> > patch. Basically I have a problem with dealing with default values
> > with
> > static IP settings
> > 
> > * The script need to fail if no value is provided for address
> > * The script needs to build correctly with default values
> > (autobuilder)
> > * There is no "correct" default values to use
> > * We should not let an unconfigured Adress on the network.
> > 
> > Right now, ADDRESS defaults to 0.0.0.0 which is interpreted by
> > net-utils
> > as "incorrect address, don't configure the interface" (so it ends
> > up
> > enabled but with no IP address) I'm happy with that, it's ok with
> > my
> > requirement
> > 
> > But I could not find an equivalent setting for networkd.
> 
> In the .network file, omit the "[Address]" section / "Address=" line.
> The interface gets upped, but no ip address gets assigned.
> I've attached some ".network" file examples below.
> 

Awesome, i'll look into it.

> > 0.0.0.0 is
> > interpreted as "automatically find an IP address" and I couldn't
> > find
> > any documentation on how it does that (does it check the network
> > for
> > collision ? It seems to use the netmask to choose the base address
> > and
> > it makes sure there is no conflict with other interfaces on the
> > same
> > machine, but that's all I could find)
> > 
> 
> That's all it does: ensure that each automatic address is unique on a
> single
> system, i.e. doesn't clash with any configured addr or any addr that
> has
> already been assigned.
> 
> It might be useful for virtual ethernet setups (veth and whatnot),
> but IMO
> that's out of scope for simple network setup, so my suggestion would
> be to
> handle "0.0.0.0" as <not configured> => no "Address=" in the .network
> file.
> It also makes BR2_SIMPLE_NETWORK_IPV4_IPV4_ADDRESS semantically
> equivalent
> for both output formats.
> 

yes, that's how i'll do it.

> Alternatives, all with the disadvantage of adding unnecessary
> complexity:
> * introduce a dummy value, e.g. "-"
> * add another choice to the "Configuration type" prompt,
>   BR2_SIMPLE_NETWORK_IPV4_NONE (or similar)
> 

There is already a "none" case where eht0 is not configured at all.
This is only to be able to build with the autobuilder. The autobuilder
might randomly select a static configuration and the build should not
fail in that case. That configuration (setting to manual but leaving
an address of 0.0.0.0) makes no sense from a user point of view.

A user that doesn't want eth0 would select the "do not configure the
interface" option that wouldn't generate any config file

> 
> > So right now I'm open to suggestions on that particular problem,
> > generating
> > the actual .network file shouldn't be very hard (I temptatively
> > call it
> > busybox.networks rather than <name>.network to be sure to overwrite
> > it
> > on the next iteration of the script)
> > 
> 
> Not sure if it's a binding convention, but these files are usually
> prefixed
> with a number 01..99, e.g. "50-buildroot.network".
> 

it seems so. I now generate 80-buildroot.network (80 is used by all
the .network files I have found lying around)

> > 
> >> Just for reference, my notes on what needs to be done:
> >> * no loopback setup, systemd does it on its own
> > 
> > yes, I found that out, it's simple to deal with since loopback is
> > not a config
> > option for net-utils anymore
> > 
> >> * creation of a ".network" file - .ini file (equivalent to
> >> /etc/network/interfaces), requires conversion of the netmask to
> >> cidr
> >> format (255.255.255.0 => 24 a.s.o.)
> > 
> > I found how to do that in shell script somewher on stackoverflow,
> > so
> > not a problem
> > 
> >> * -optionally- disable "predictable" network interface renaming
> >> ("eth0"=>"enp2s0") [0] so that one can actually rely on eth0's
> >> existence
> >>
> > 
> > that's a good idea, but predicatable interface names seems to
> > already
> > be disabled on buildroot builds. I wasn't able to find out why and
> > I
> > didn't dig since I was dropping networkd support at that point. It
> > seems that there is something weird there...
> > 
> 
> It's not disabled, but under certain circumstances it's not "active",
> for example
> if the kernel-provided name is deemed to be stable, e.g. qemu/kvm
> with virtio netdev.
> You can provoke it by plugging in an usb wlan adapter, which appears
> as "wlp0s20u10"/...
> in 'ip link'/'ifconfig'.
> On x86 consumer-grade hardware, the wired interfaces get always
> renamed, too.
> 
> There are several ways to disable it, but the best solution -for me-
> is
> "ln -s /dev/null $(TARGET_DIR)/etc/systemd/network/99-default.link",
> which still allows
> custom interface renaming in /etc/systemd/network/ while disabling
> the default policy.
> 

I'll have a look at that. On the raspberry pi (my testing hw) it is
named eth0 so testing this is tricky. I probably can't go further than
"I test that the link is correctly set, not that networkd reacts 
correctly"


At this point I have too many changes to v3 to wait much longer, so
I'll integrate networkd and resubmit today

Regards

Jeremy

> ---
> .network file examples:
> 
> # "0.0.0.0"
> [Match]
> Name=eth0
> 
> [Network]
> Description=buildroot-configured interface
> (EOF)
> 
> # any other manual ip addr
> [Match]
> Name=eth0
> 
> [Network]
> Description=buildroot-configured interface
> 
> [Address]
> Address=192.168.1.2/24
> Broadcast=192.168.1.255 ## if configured
> 
> [Route]
> Gateway=192.168.1.1 ## if configured
> (EOF)
> 
> # dhcp
> [Match]
> Name=eth0
> 
> [Network]
> Description=buildroot-configured interface
> DHCP=v4
> (EOF)
> 
> Notes:
> * the "Gateway=" line can also be moved to the "[Network]" section
>   (+ drop the "[Route]" section)
> 
> * "Address=" should have its own section because of "Broadcast="
> 
> --
> André
> 
>