Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose lwIP netif interface to Lua #1752

Closed
wolfgangr opened this issue Jan 21, 2017 · 13 comments
Closed

Expose lwIP netif interface to Lua #1752

wolfgangr opened this issue Jan 21, 2017 · 13 comments

Comments

@wolfgangr
Copy link

wolfgangr commented Jan 21, 2017

8<------------------------ FEATURE REQUEST ------------------------------------

Missing feature

Advanced/customized examination and modification of network configuration, using nodeMCU's lwIP network interface, from Lua land

Justification

See the lwIP documentation about the functionalities of the netif module
http://lwip.wikia.com/wiki/Network_interfaces_management

I need it as a prerequisite for developping an ethernet driver ( see issue #1725).
I suppose it will also be required for configuring customized network configuration from lua land, once drivers for other PHY than the embeded WIFI are available.
It should also enhance the manipulation of routing and other advanced network setups with the onboard WIFI from Lua land, as far as provided form lwIP.

Workarounds

Do it in C:

./app/include/lwip/netif.h
./app/include/lwip/netifapi.h
./app/include/netif/*.h
./app/lwip/api/netifapi.c
./app/lwip/core/netif.c
./app/lwip/netif/etharp.c

8<------------------------ END FEATURE REQUEST --------------------------------

Following the discussions of issue #1555 (exposing "ping"), I'd suggest to keep it in a file
app/modules/net_if.c and call the lua functions net_if.whatever().

Everybody happy with that proposal?

@wolfgangr
Copy link
Author

There are 2 layers of netif interfaces in lwip

  • The direct interface, defined in ./app/include/lwip/netif.h
  • The API interface, definded in ./app/include/lwip/netifapi.h

cmp Adam Dunkel's Initial lwIP Paper , Chapter 7 , Chapter 12.2

The general design principle used is to let as much work as possible be done within the ap-
plication process rather than in the TCP/IP process. This is important since all processes use
the TCP/IP process for their TCP/IP communication.

I think that we don't have "processes" or "threads" on the nodemcu, so we could use the "short way" and interfacing the TCP/IP direct, as available in netif.h.
However, given the above recommendation and the fact that the API interface is much simpler (it does not use callbacks), we start with the API interface in netifapi.h.

Accordingly, I'd suppose to call the function net_if_api.whatever() and the file app/modules/net_if_api.c
If we come across a functionality that does not work properly, we still can have the direct way in a seperate module net_if.whatever() in a file app/modules/net_if.c.

This approach also follows the rationale of keeping a fine granuled module footprint.

@wolfgangr
Copy link
Author

wolfgangr commented Jan 22, 2017

hm.. looks like from API i cannot even get a hold on the list of interfaces.
So I need to call into the core anyway, so mess is already under way.

I think I'll proceed without the api, keeping the name at net_if.

@wolfgangr
Copy link
Author

to have a second interface for test purposes, I enabled LOOPBACK and a LOOP interface in the network configuration:

--------------------------- app/include/lwipopts.h ----------------------------
index 23f731a..feaddcc 100755
@@ -1141,6 +1141,8 @@
  * LWIP_NETIF_LOOPBACK==1: Support sending packets with a destination IP
  * address equal to the netif IP address, looping them back up the stack.
  */
+#define LWIP_NETIF_LOOPBACK             1
+
 #ifndef LWIP_NETIF_LOOPBACK
 #define LWIP_NETIF_LOOPBACK             0
 #endif
@@ -1149,6 +1151,9 @@
  * LWIP_LOOPBACK_MAX_PBUFS: Maximum number of pbufs on queue for loopback
  * sending for each netif (0 = disabled)
  */
+
+#define LWIP_LOOPBACK_MAX_PBUFS 	1
+
 #ifndef LWIP_LOOPBACK_MAX_PBUFS
 #define LWIP_LOOPBACK_MAX_PBUFS         0
 #endif
@@ -1191,6 +1196,9 @@
 /**
  * LWIP_HAVE_LOOPIF==1: Support loop interface (127.0.0.1) and loopif.c
  */
+
+#define LWIP_HAVE_LOOPIF 		1
+
 #ifndef LWIP_HAVE_LOOPIF
 #define LWIP_HAVE_LOOPIF                0
 #endif

As a result, the output of my early test code shows at least two lines:

  • 201A8C0 = IP 192.168.0.2 for the WIFI
  • 100007F = IP 127.0.0.1 for the loopback
=net_if.list_adr()
 IP: 201A8C0 netmask: FFFFFF gw: C01A8C0  
 IP: 100007F netmask: FF gw: 100007F 

Is it OK to have it that way in a pull request to DEV?
I know that this will add some bytes to the flash code, which may not be appropriate for MASTER.
Or is there a specific way/trick to hand individual configs?

@djphoenix
Copy link
Contributor

Any kind of improvements should first go to dev, so we have a time to test them roughly, and (if it necessary) revert them before "master drop".

@marcelstoer marcelstoer changed the title Expose lwIP netif interface to LUA Expose lwIP netif interface to Lua Jan 23, 2017
@wolfgangr
Copy link
Author

wolfgangr commented Jan 23, 2017

things are getting shape...

net_if.list_adr()
ew1 - HWaddr: 60:01:94:0f:e9:29  hostname: NODE-FE929 
    IP: 192.168.1.2 netmask: 255.255.255.0 gw: 192.168.1.12 
    mtu: 1500 flags: 0xBB  UP BRCST DHCP LNKUP ETHARP IGMP 
    DHCP server: 192.168.1.12 state: 10 tries: 0 lease time: 43200
    
lo0 - HWaddr: 00:00:00:00:00:00  hostname: (null) 
    IP: 127.0.0.1 netmask: 255.0.0.0 gw: 127.0.0.1 
    mtu: 0 flags: 0x01  UP LNKDN 

@wolfgangr
Copy link
Author

wolfgangr commented Jan 23, 2017

@djphoenix

Any kind of improvements should first go to dev, so we have a time to test them roughly,

I did'nt ask for allowance of a new feature, but whether it is allowed to switch some disabled configs on for test purposes.
The main reason for turning on the LOOP interface was to have a list with more than one item to process.
I think I have learned from the last days/hours that I should reverse such test settings for a PR ;-
Never mind :-)

@devyte
Copy link

devyte commented Jan 24, 2017 via email

@wolfgangr
Copy link
Author

@devsaurus @marcelstoer ,
May I propse the following steps out of my current contribution quagmire:

It may seem odd to you to solve a problem by adding new ones, but from my point of view, this matches my current capabilities and learning needs.

The code for net_if is completely my own, so I understand what and why I have done.
I think I can easily solve/avoid most problems you raised in the pending PR #1751 for the net_if in the current stage.

The only open question at the moment is the one regarding your dislike of c_printf, because the current net_if code heavily relies on them. That's why I have been so insisting on that in my PR comments. But switching to something else (NODE_ERR(), dbg_printf) would well be manageable in the current stage.

For the following steps, I'd prefer to implement a configurable output, e.g. handing strings back to lua, handing back pointers or structures, and presumably callbacks. But I'd like to shift that to a later PR.

As I see from @devyte 's comment, there is a need for the simple informative one-way command-line peek into the netif list.

Is it possible to have two concurrent PR open from the same github account, but different branches?
If not, we might close #1757 and reopen it after the changes are made.

Is this plan OK with you?

@wolfgangr
Copy link
Author

wolfgangr commented Jan 24, 2017

Looks like the question when to print where by what utility is a major issue not yet settled.
It seems to gain momentum in the PR comment area, which may not be the prominent place it deserves.
So let me just provide a pointer for reference:
#1757 (comment)

@marcelstoer
Copy link
Member

Is it possible to have two concurrent PR open from the same github account, but different branches?

Yes, that works well and we encourage contributors to drive individual features forward in individual branches.

Looks like the question when to print where by what utility is a major issue not yet settled.

In a review comment I pointed you to https://github.com/nodemcu/nodemcu-firmware/wiki/%5BDRAFT%5D-How-to-write-a-C-module#debug-and-error-messages but I understand that the notes there are terse. Also, the code is not consistent yet across existing modules as we haven't had that template for long.

@wolfgangr
Copy link
Author

@marcelstoer

I pointed you to https://github.com/nodemcu/nodemcu-firmware/wiki/%5BDRAFT%5D-How-to-write-a-C-module#debug-and-error-messages

Yes, and this ends with

More inspiration: (...) http://blog.mclemon.io/esp8266-contributing-to-the-nodemcu-ecosystem

Which is precisely the code you lads are tearing apart in PR #1757 .
Looks like a circular reference, doesn'it? Life can be cruel sometimes ;-)
So may we agree in "things are moving ahed".
The question is not who is right and wrong, but how to get things better, I understand.

@wolfgangr
Copy link
Author

@devyte and anybody else courious:
I just pushed and tagged the code that produced the example print given above:
https://github.com/wolfgangr/nodemcu-firmware/blob/net_if-000to-stdout/app/modules/net_if.c

I think I'll rewrite this to sprintf instead of printf, so we can return the result string to lua as well.
Regarding the many open ends on my ping case, I think I'm not going to issue a PR right now, because there are too many open ends that present as moving targets to me and distract me from my real goal.

@stale
Copy link

stale bot commented Jan 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 13, 2020
@stale stale bot closed this as completed Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants