lwip with port forwarding on same netif (IDFGH-7397)

[mtnbkr88]

As currently written in ESP-IDF 4.4.1 (lwip 2.1.2), lwip cannot possibly forward back onto the same network (what I need) without errors. I will explain. If A is the source, B the port forwarder and C the destination, as currently written ip4_forward only modifies the destination in the packet. So a packet will be forwarded with source as A and destination as C. This is okay if the packet is going to a different network than the source. If the destination is on the same network (netif) as the source then when C receives the packet it will return directly to A. Then A will drop the packet because it does not have any known connections with C. A had a connection open to B, not C. (I verified this behavior with wireshark.) My update to lwip fixes this. If the packet is forwarded to the same network as the source, my update will have destination C return to B which will then return to A. All traffic is as expected, all connections are properly handled.

Note that IP_FORWARD, IP_NAPT and IP_FORWARD_ALLOW_TX_ON_RX_NETIF must all be enabled. I had to do manually enable IP_FORWARD_ALLOW_TX_ON_RX_NETIF in opt.h since menuconfig does not currently handle this.

@david-cermak I closed the old and created a new pull request for adding port forwarding on the same netif. All check unit tests complete with no errors.

[david-cermak]

The changes look okay to me in general, but I'm still unsure about accepting it as is, since they are very strictly tailored to your usecase so there's always a potential risk of regression for other common and more general NAPT usecases.
That's why I suggest guarding most of your changes within #if's wherever's possible.
Plus some other minor comments.

[mtnbkr88]

@david-cermak You referred to my changes as a very (very with italics!) specific use case. This functionality has been in Unix/Linux for decades (iptables) and I am actively using it in Debian Linux on the C.H.I.P. computer I'm currently running in my house. Really I'm helping lwip catch up with the big boys. You're welcome.

I made your suggested changes where it did not result in errors. The suggestion to use ip4_addr_debug_print_val fails because it is expecting a particular type of structure which the values don't have. The suggestion to use U16_t in place of d in the LWIP_DEBUGF fails because LWIP_DEBUGF can't handle type U16_t plus enable is passed in as an integer anyway.

As you requested I changed the inclusion of msport as a conditional compile in ip_napt_entry when IP_FORWARD_ALLOW_TX_ON_RX_NETIF is defined. This saves up to ~1k of RAM and excludes my additions from the compiled code for the general user who does not need this functionality.

All check unit tests pass. I also ran the following test with IP_FORWARD_ALLOW_TX_ON_RX_NETIF defined:

make clean
export EXTRA_CFLAGS="-DIP_FORWARD=1 -DESP_TEST_DEBUG=1 -DIP_NAPT=1 -DLWIP_ARCH_CC_H -DIP_FORWARD_ALLOW_TX_ON_RX_NETIF -include cc_esp_platform.h" && export CC="cc $EXTRA_CFLAGS"
make -j 4 check
results: 100%: Checks: 5, Failures: 0, Errors: 0

If you run the tests with and without IP_FORWARD_ALLOW_TX_ON_RX_NETIF defined you will clearly see the difference in the debug output.

[david-cermak]

Thanks for the updates!

[david-cermak]

@mtnbkr88 I'm sorry, I was just a little worried about the current NAPT functionality, since we don't really have a good test coverage. Using ESP32 as routing/ip-table device is not a very common use-case, but an interesting one -- appreciate your effort here!

Thanks for the contribution, I've passed this PR to an internal review and will share updates whenever available.

[mtnbkr88]

Sounds good. Thanks for approving.