buglwIP - A Lightweight TCP/IP stack - Bugs: bug #33634, ip_forward() have a faulty...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #33634: ip_forward() have a faulty behaviour.

Submitter:  hanhui <hanhui03>
Submitted:  Fri 24 Jun 2011 03:28:15 AM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Thu 21 Jul 2011 07:49:18 PM UTC, comment #17: 

Checked in nearly as you suggested, with some small changes:
- changed the names of the 2 hook functions (to let all future hooks start with LWIP_HOOK_)
- changed the name of the pbuf flags
- I didn't chagne the default behaviour, you have to set IP_FORWARD_ALLOW_TX_ON_RX_NETIF to 1 to allow forwarding on the input netif (making this the default seems too bear a high risk of flooding networks if the netif driver doesn't set the pbuf flags - I've only adapted ethernet_input, other drivers may use their own code)
- adapted ethernet_input() to set the pbuf flags
- LWIP_HOOK_IP4_INPUT: if the hook eats the packet, it takes responsibility of the pbuf (i.e. free it when done). That way, the pbuf can be just passed on to protocols without calling pbuf_ref().

I hope everything works as expected: normal traffic works for me, but I cannot test IP forwarding, currently.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 01 Jul 2011 01:28:58 AM UTC, comment #16: 

Many operating system or protocol stack have many HOOK, such as INPUT, OUTPUT, ROUTE, FORWARD...

These hooks can often be used in conjunction to handle complex logic.

For example: linux netfilter use in, out, forward hooks.

hanhui <hanhui03>
Fri 01 Jul 2011 01:14:42 AM UTC, comment #15: 

I recommend that adding IP_INPUT_HOOK, it is really very useful, many protocol stacks are also adding this feature, like BSD, if we use "COMPAT_IPFW", BSD will call ip_fw_chk_ptr() for each receives an IP packet.

hanhui <hanhui03>
Thu 30 Jun 2011 06:01:34 PM UTC, comment #14: 

BTW, I like the ip_route hook. Any objections on checking this in, anyone?

Simon Goldschmidt <goldsimon>
Group administrator
Thu 30 Jun 2011 06:01:02 PM UTC, comment #13: 

Hmm, filtering by link layer address might be a good idea for safety reasons, then...

Simon Goldschmidt <goldsimon>
Group administrator
Thu 30 Jun 2011 06:43:55 AM UTC, comment #12: 

OK, My English may not be clearly, that do discard bcast/mcast SYN is just a example of M_CAST flag, but BSD use ip and link-layer address together to determine bcast/mcast SYN, the BSD code as follow:

if (m->m_flags & (M_BCAST|M_MCAST) ||
    IN_MULTICAST(ntohl(ti->ti_dst.s_addr)))
    goto drop;

but you can temporarily not add this flag. It does not matter now.

Thanks!

hanhui <hanhui03>
Thu 30 Jun 2011 05:26:13 AM UTC, comment #11: 

Oh, and we do discard bcast/mcast SYN, but based on the IP address, not on the link-layer address.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 30 Jun 2011 05:17:39 AM UTC, comment #10: 

Alright, I only wanted to know whether the INPUT_HOOK had something to do with this bug. I like to keep things separated...

Simon Goldschmidt <goldsimon>
Group administrator
Thu 30 Jun 2011 03:24:36 AM UTC, comment #9: 

OK, I will provide patch files next time.

Many functions can be extended use LWIP_IP_INPUT_HOOK, such as : NAT, IP packet filter, Statistical analysis and so on. I think this hook is very useful, and can do something in the routing algorithm.

MCAST is not currently used, so you can temporarily not add.

However, in BSD systems, transport layer will determines the type of link-layer address.

for example: RFC1122 4.2.3.10, p. 104: discard bcast/mcast SYN.
so tcp will discard the cast/mcast SYN packet.

Of course, lwip not judge now, also did not lead problems.

You decide whether to keep MCAST flag!



hanhui <hanhui03>
Wed 29 Jun 2011 08:06:17 PM UTC, comment #8: 

Just to answer comment #6: hanhui's patch calls the hook first, so the hook's decision overrides lwIP's ip_route() decision, which I think is best for a hook...

- please provide patch files next time, that makes it much easier to see what's actually changed (without having access to the sources, which are on my home PC only)

- What is the use for LWIP_IP_INPUT_HOOK in this case? It doesn't have to do anything with IP forwarding, or does it?

- Although I dislike the idea of adding more flags to pbuf->flags (we have only 8 bits), I think preventing lwIP from forwarding broadcast packets is a good thing to do, so we should probably add such a flag. However, wouldn't one flag be enough? Would we forware multicast traffic, anyway?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 27 Jun 2011 01:20:21 PM UTC, comment #7: 

Yes, I'd like to propose an API

please see the ipv4 and pbuf.h

I add two flag in pbuf: (like BSD)

/** send/received as link-level broadcast */
#define PBUF_FLAG_BCAST     0x08U
/** send/received as link-level multicast */
#define PBUF_FLAG_MCAST     0x10U

(Simon Goldschmidt should modify etharp.c to support PBUF_FLAG_BCAST and PBUF_FLAG_MCAST flags)



(I Fixed bug #33653: ip_data.current_ip_header_tot_len calculation errors! in ipv4.c)


(file #23574, file #23575)

hanhui <hanhui03>
Mon 27 Jun 2011 08:42:16 AM UTC, comment #6: 

Yes, I'd be happy with a hook that would allow extension of the routing.  This needs some thought about how it would work though, and whether the hook function would over-ride or veto the lwIP decision, or vice versa.

Hanhui: would you like to propose an API for this?

Kieran Mansley <kieranm>
Group Member
Sun 26 Jun 2011 05:35:01 PM UTC, comment #5: 

I think defining such hook functions is the best way to solve this, since the current behaviour is what we need for wired networks.

Simon Goldschmidt <goldsimon>
Group administrator
Sat 25 Jun 2011 03:49:42 PM UTC, comment #4: 

That even if a network interface has multiple addresses in the future, it will not cause problems, because ip_input() will matching the address, if address matching successful, ip_input() will not invoke ip_forward().

hanhui <hanhui03>
Sat 25 Jun 2011 03:23:35 PM UTC, comment #3: 

I think we can use a flag in pbuf, like BSD use M_BCAST to determine whether the received packet is broadcast. If the input packet is broadcast then discard! if not, we do packet forward. we will no longer in use "network interface compare method"

I recommend that ip_forward() or ip_route() functions can be added a user-configurable hook function to expand lwip routing algorithm.

This method without increasing the code of the lwip but can be more flexible expansion.

Wireless Ad Hoc network system will be more popular in future, that Will be a variety of routing protocols, this hook will become very important in the future.

If added this route hook to LWIP, I can contribute a lwip Ad Hoc Routing Protocol (AODV)

hanhui <hanhui03>
Sat 25 Jun 2011 09:43:21 AM UTC, comment #2: 

Makes sense to me, too. Anyway, it would only make sense to send back on the input netif if we supported multiple IP (routable) addresses on one netif, which we don't (yet?).

I think to support what you want would need some more work than simply forwarding everything back to the input netif. In wired and wireless networks, forwarding everything back to the net would result in an endless loopback of packets, wouldn't it?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 24 Jun 2011 08:36:01 AM UTC, comment #1: 

I like the sound of that check.  It makes sense to me in a wired network to not send traffic back the way it has come.

Kieran Mansley <kieranm>
Group Member
Fri 24 Jun 2011 03:28:15 AM UTC, original submission:  

ip_forward() check if the output net interface is same as input net interface, then  discard! so if I use lwip in a Ad Hoc wireless net working, lwip can not forward packets for other nodes.

hanhui <hanhui03>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #23574:  ip4.c added by hanhui03 (32KiB - application/octet-stream - add ip_canforward() ; LWIP_IP_ROUTE_HOOK ; LWIP_IP_INPUT_HOOK)
file #23575:  pbuf.h added by hanhui03 (6KiB - application/octet-stream - add ip_canforward() ; LWIP_IP_ROUTE_HOOK ; LWIP_IP_INPUT_HOOK)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by hanhui03 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-07-21 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2011-06-27 hanhui03 Attached File- Added ip4.c, #23574
        Attached File- Added pbuf.h, #23575

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code