Re: [lwip-devel] Proposals for lwip

[Andrea Berlingieri]

> I'm open to adding another hook, but you'll have to do the "specify the
> hook functions at runtime" yourself. But I don't see a problem to do so:
> just define the hook to be e function that checks if a real callback has
> been registered. No need to move that into the actual lwIP code.

Ok, I've added code to do the registration of the callback as an addon in the 
contrib folder, and it
seems to work fine. You're right, if it is not of general interest there's no 
reason to add it to
the lwIP code. I'll just propose the new hook in a future patch.

> Why "lwipv6"?
> 
> I'm not interested in porting anything upstream myself right now, but if
> you want to send a patch, we could see if it's a match for lwIP that's
> good to integrate.
> 
> But I'd be certainly interested in a link to the NAT implementation,
> just to have a look ;-)

lwipv6 was the name of the project itself. It was named that beacause at the 
time it was made
(~2006) lwIP didn't support IPv6 yet, and the support for the protocol was a 
needed feature and the
main reason the fork was made in the first place. Unfortunately, I don't have a 
patch ready for the
features I listed, as the changes were made to lwip version 1.5a, or even 
earlier. A port would
require some work, as I don't think the changes would apply cleanly 8 years 
later. I can take it
upon me, but probably in the future.

If you're interested in the NAT implementation, you can find the source code 
for lwipv6 at
http://deb.debian.org/debian/pool/main/l/lwipv6/lwipv6_1.5a.orig.tar.gz. You'll 
find the NAT part
under lwip-v6/src/userfilter. In lwipv6 addresses are stored as "IPv4-mapped 
IPv6 address", as
defined in RFC 1884.

> That's available since ages: MEM_LIBC_MALLOC to use 'malloc' instead of
> 'mem_malloc' and MEMP_MEM_MALLOC to use heap allocationi instead of the
> memp pools.

I'm sorry, I must have missed it while scrolling through the options. Thank you.

> >   - I've seen that in lwip each netif is associated with a single ipv4 
> > address, a single netmask and
> >     a single gateway. Now, in lwipv6 a modification was made to support 
> > lists of ip addresses, so
> >     that a single interface could be associated with more that one ip 
> > address. Now, introducing this
> >     feature to lwip would require also a modification to the route 
> > function, although this wouldn't
> >     seem to be too complicated, at least conceptually (as, instead of 
> > looking at a single address, a
> >     search on a list would be performed). So, my next proposal would be to 
> > add this feature.
> 
> I've written before that I'm very open to this. In fact, our IPv6
> support provided this from the beginning. It might just not be easy to
> keep all apps and ports compatible to such a change...

I can work on that in the future.
 
> >   - Stack configuration (setting addresses, setting interfaces, getting 
> > interfaces infos, etc) is
> >     made through netlink in my vunetlwip module. Even ioctls are 
> > intercepted and converted to the
> >     appropriate netlink counterparts. Now, while working on the address 
> > information retrieval (the
> >     GETADDR message of the rtnetlink protocol), I couldn't be able to find 
> > the prefix len for a given
> >     ipv6 address. As of right now I simply assume that the prefix len is 64 
> > and return that, but that
> >     is a strong assumption. This is mix between a proposal to add such a 
> > field somewhere in lwip, and
> >     a request of help on how to find this information, if it is already 
> > encoded somewhere I couldn't
> >     find.
> 
> Yeah, well, after Ivan left the project, IPv6 got less attention than it
> needed. Patches are welcome :-/

Alright, I'll add a proper prefix_len field somewhere appropriate and send a 
patch for this.

> >   - In lwipv6 a modification was made to support multiple routes for a 
> > netif. That is, for each netif
> >     a list of routes was kept. It seems to me though that in lwip routing 
> > is very simple: packets are
> >     routed to the interface that has a matching ip address prefix to that 
> > of the dest ip address (if
> >     source address routing is not involved). My next proposal would be to 
> > add such a feature to lwip,
> >     as I think it may be useful, although it may make routing more 
> > complicated.
> 
> We have a hook for routing already. Such an improved implementation
> could be provided in contrib/addons as a standard addon.

I can work on that in the future. Still, I can see how to implement multiple 
routes using the
routing hook, but I was thinking more of modifying the netif struct and adding 
some functions to
add/del routes to the raw API. While this second way is a little more 
intrusive, I think it's also
more elegant than to add data stuctures to store the routes for each netif and 
the "routing API" in
addon files.
 
> >   - Now, this last one is kind of low priority. Basically, while testing 
> > with ping, I noticed that
> >     the iputils version of ping for Linux uses a 
> > socket(AF_INET,SOCK_DGRAM,IPPROTO_ICMP) call, in the
> >     case of ipv4, to create a socket to send ICMP packets. Now, lwip 
> > assumes that a socket that uses
> >     SOCK_DGRAM as a type is a UDP socket. This caused some unexpected 
> > behavior. As of right now I
> >     simply intercept the socket call and transform a 
> > socket(AF_INET,SOCK_DGRAM,IPPROTO_ICMP) call
> >     into a socket(AF_INET,SOCK_RAW,IPPROTO_ICMP) call. This kind of fixes 
> > the problem, but is not
> >     ideal. My proposal would be to add support for this case of socket, if 
> > a configuration constant
> >     has been defined (it seems to me that this kind of calls are only made 
> > and supported in Linux. In
> >     this case the kernel handles the call in a special way, and the created 
> > socket is only for ICMP
> >     echo requests: 
> > https://stackoverflow.com/questions/8290046/icmp-sockets-linux , third 
> > answer).
> >     I'm not too sure about this, maybe we can discuss it a little.
> 
>  From what you wrote: send a patch and I'm sure we can accept it.

I didn't really work on the lwIP code to fix that, but rather on my submodule 
code. However, it
should be pretty simple to fix this in the lwip code. I'll do it and include it 
in a future patch.

> > I don't have a problem with adding an option (e.g.
> > LWIP_SOCKETS_SOCKADDR_HAVE_SALEN). However, you'll probably find more
> > differences in the header files. Because of that, Joan has been working
> > on LWIP_SOCKET_EXTERNAL_HEADERS to replace our header with the original
> > OS header (Hurd in his case).
> >
> > You might still need to get rid of 'sa_len' then...
> >
> 
> In fact, I have a patch to add the LWIP_SOCKETS_SOCKADDR_HAVE_SALEN macro. 
> I'll send it soon.

I've used the LWIP_SOCKET_EXTERNAL_HEADERS and defined my own inet and sockets 
headers to deal with
this. However, I still had to modify some parts of the sockets.c file, as they 
had references to the
sa_len field of sockaddr, which I substituted with sizeof(sockaddr). I'll wait 
for Joan's patch, to
see if I can deal with this while keeping the modifications to the lwIP code at 
a minimum.