sys/shell/lwip: add IPv4 configuration to lwip ifconfig command

[krzysztof-cabaj]

Contribution description

This PR adds to lwip ifconfig shell command, ability to configure IPv4 address, mask and gateway.

Testing procedure

Enable in example/gcoap lwip IPv4 stack: in Makefile, line 17 change:

LWIP_IPV4 ?= 0

to

LWIP_IPV4 ?= 1

Configure tap0 interface:

sudo ip tuntap add tap0 mode tap user ${USER}
sudo ip link set tap0 up

Compile example and run it with tap0 interface:

make clean
make
./bin/native/gcoap_example.elf tap0

Test new subcommands added to ifconfig command:

ifconfig help
ifconfig add4 ...

Issues/PRs references

None

[yarrick]

I think the right way to go is to add ways of setting it as an opt (continuing on #18069) so that the ifconfig command can share most of the code between the IP stacks.

[krzysztof-cabaj]

Hi!

So you idea is to add appropriate ifconfig <if_id> set <key> <value> command? Something like ifconfig 6 set addr 192.168.1.2, ifconfig 6 set mask 255.255.255.0, etc. ?

[krzysztof-cabaj]

I looked through implementation of ifconfig set from gnrc_netif.c and try to implement configuration of LWIP IPv4 in similar manner. The most recent commits contains initial version of the code - setting ip4 address.

@miri64 @yarrick could you be so kind and looked at this code and comment it before I provide full implementation of all options associated with IPv4 configuration?

[yarrick]

For IPv4 I started in 0b48789 but having separate opts for the different type of addresses like you proposed is better. (edit: or maybe since they have to be set together it makes sense to have them in one group? See netifapi_netif_set_addr in lwIP)

I sent a PR now for removing the last blockers (I think) for using the gnrc_netif shell command instead of the lwIP one.

[krzysztof-cabaj]

If I understand your idea well - you plan to use one version of the netif shell module which implements one ifconfig command for all network stacks depending on which is currently used? Good idea.

Please let me know when you end your work and I could move my configuration code.

Accordingly to one NETOPT_IPV4_ADDR I suggest split them into three, my rationale:

• You could set any (IP, mask, gw) separately - link,
• I looked at other NET_OPTs and at least most (mayby all) use simple one value to set,
• There could be a problems in ifconfig set implementation - currently all options take one value, so to set three we have to
  a) add new kind/class of function which takes three values,
  b) add some kind of value separator (for e.g. ; so user have to pass one value - 192.168.1.2;255.255.255.0;192.168,1.1), which looks strange and we have to split this later before we could use,
• Ability to set only one value for example IP or GW could be beneficial.

Of course on contrary we have to use three ifconfig set commands to fully configure IPv4 stack.

[krzysztof-cabaj]

@yarrick I checkout your branch v4_addr and try to test it using examples/gcoap. When I enable LWIP for IPv4 (set LWIP_IPV4 variable to 1) I assumed that your commands from cmds/gnrc_netif.c will be utilized. However, I still use cmds/lwip_netif.c. How can I enable your code?

Moreover, I would like to know you opinion about spliting IPv4 configuration in three NETOPTs - I provide more details: some pros and cons in the previous post.

[krzysztof-cabaj]

Can we move this PR little forward?

Sorry, for bothering - but possibility to change IPv4 address is important for my projects ... and I suspect not only for me.

I have interim solution. Could we add to master legacy style command - ifconfig add4 <interface> <IPv4>/<prefix> gw <IPv4>.
As soon as @yarrick add net opts for IPv4 - I will provide command using set style command and net opts.

@yarrick, @miri64, @benpicco what do you think about such interim solution? If I receive a green light I start cleaning my first version of the code, which provides legacy style command.

[miri64]

Sorry, not much time at the moment to give this a proper review, maybe @yarrick or @benpicco can help. However, please address the issues pointed out by the static tests regarding the coding conventions and in general keep to them.

[krzysztof-cabaj]

Current version of PR adds legacy style ifconfig command - ifconfig add4 <interface> <IPv4>/<prefix> gw <IPv4>.

@miri64 I think all mistakes associated with coding conversion are fixed. Code is rebased.

[krzysztof-cabaj]

@miri64, @yarrick, @benpicco

Could we move this PR little forward? ... and this is not April Fool's Day joke ;)

I think that this feature is not controversial, and making git cherry-pick for each new application is bothersome.

[riot-ci]

Murdock results

✔️ PASSED

71d47cd sys/shell/lwip: merge ifconfig add4 and add6 options to one-ifconfig add

Success	Failures	Total	Runtime
10026	0	10027	10m:52s

Artifacts

• Documentation preview

[benpicco]

Thank you for getting back to us and sorry for forgetting about this PR.

The code looks overall fine, intendation is a bit off in some places, but that's an easy fix :)

[krzysztof-cabaj]

@miri64, @benpicco should I fix or add some functionality to the code to finalize this PR?

[benpicco]

Looks good to me, thank you for the new feature!

[krzysztof-cabaj]

Thanks for useful comments and hints. This feature will be beneficial for my students ;)

P.S. Could you add PR to merge queue ... or we wait for more reviews?