shell/gnrc_netif: Use netif API for ifconfig

[leandrolanzieri]

Contribution description

This PR modifies the ifconfig and txtsnd command implementations to use the new netif API introduced in #11879. Some changes had to be made in tests as now the name for GNRC interfaces returns if<n> (that's the value returned by GNRC's implementation of netif_get_name).

This may be a first step towards having a generic implementation of this command, that does not depend on the stack that is currently being used.

For now the PR includes commits of #11879, I will rebase when it gets merged, but the changes on the commands can be reviewed anyway.

Testing procedure

• Check that the commands work as expected
• Run the modified tests, they should still work.

Issues/PRs references

Depends on #11879

[miri64]

More motivation to finally continue working on #9343 and adapt it to #11879 :-)

[leandrolanzieri]

Rebased now that #11879 has been merged.

[miri64]

git mv sys/shell/commands/sc_gnrc_netif.c sys/shell/commands/sc_netif.c

?

[leandrolanzieri]

git mv sys/shell/commands/sc_gnrc_netif.c sys/shell/commands/sc_netif.c

?

Maybe it's better to rename it after all GNRC dependencies are removed

need time for testing, won't block either though

[leandrolanzieri]

Rebased to current master

[leandrolanzieri]

@miri64 @smlng @jia200x Would it make sense to modify GNRC's implementation of netif_get_name so it does not include if in the interfaces name? Maybe the impact is less: we would not need to change the tests and risk to break existing scripts that already depend on these names.

[miri64]

@miri64 @smlng @jia200x Would it make sense to modify GNRC's implementation of netif_get_name so it does not include if in the interfaces name? Maybe the impact is less: we would not need to change the tests and risk to break existing scripts that already depend on these names.

Yes, that would make sense. There is no specific reason why I added the if-prefix, other than having it more "name-like". #12667 still makes sense though, as non-GNRC stacks might use actually different names (see lwIP e.g.)

[leandrolanzieri]

@miri64 I removed the prefix from the name and reverted the changes in tests.

[leandrolanzieri]

I've adapted the GNRC netif tests according to the new names.

[tcschmidt]

@smlng can you start testing this?

[brummer-simon]

I was asked to run the gnrc_tcp tests on this PR because they rely on the output of ifconfig. From my point of view everything works as it should.

[rosetree]

The current master surrounds the interface number with 2 spaces. After this change it’s only one space.

 > ifconfig
 ifconfig
-Iface  6  HWaddr: C2:1F:EE:FD:61:CB 
+Iface 6 HWaddr: C2:1F:EE:FD:61:CB 
           L2-PDU:1500 MTU:1500  HL:64  RTR  

[leandrolanzieri]

The current master surrounds the interface number with 2 spaces. After this change it’s only one space.

 > ifconfig
 ifconfig
-Iface  6  HWaddr: C2:1F:EE:FD:61:CB 
+Iface 6 HWaddr: C2:1F:EE:FD:61:CB 
           L2-PDU:1500 MTU:1500  HL:64  RTR  

Thanks @rosetree! I added the missing spaces.

[rosetree]

At the Hack ’n’ ACK I tested the following commands, some native and some on the SAMR21 board:

• ifconfig (native + samr21)
• ifconfig 7 del <ip_addr> (samr21)
• ifconfig 7 add multicast <ip_addr> (samr21) – though I didn’t
  manage to set the ip address sucessfully
• ifconfig 6 stats ipv6 reset (native)
• ifconfig 6 stats ipv6 (native)
• ifconfig 6 stats l2 (native)
• ifconfig 6 stats (native)
• ifconfig 6 set chan 12 (samr21), error case
• ifconfig 7 set chan 12 (samr21)
• ifconfig 7 set csma_retries 7 (samr21), error case
• ifconfig 7 set csma_retries 5 (samr21)

I executed the commands on both the master branch (37dc677) and the pull request branch, saved the outputs to text files and compared them with diff. I didn’t see differences between both versions, besides from different counter values in some cases.

[PeterKietzmann]

@rosetree as discussed offline, please report about the other tests that you did (which succeeded). Furthermore, I ran gnrc_networking on a samr21-xpro using this PR and current master, and compared different outputs, all of which looked similar (besides that IPv6 address generation has changed in the meantime). In more detail I compared:

• ifconfig
• ifconfig 7 del <IPv6 addr>
• ifconfig 7 add <IPv6 addr>

-> all good

[PeterKietzmann]

@smlng hope this helps you a bit :-)

[smlng]

please rebase and squash

[leandrolanzieri]

@smlng done

[smlng]

tested ACK, though I couldn't test all possible interface variants out there

[smlng]

let's see how Murdock feels about it 😄

[leandrolanzieri]

Thanks all for reviewing and testing!