Message-ID: <20180602170221.GA7561@1wt.eu>
Date:   Sat, 2 Jun 2018 19:02:21 +0200
From:   Willy Tarreau <w@....eu>
To:     Sam Patton <sam@...ancedip.org>
Cc:     netdev@...r.kernel.org
Subject: Re: ANNOUNCE: Enhanced IP v1.4

On Sat, Jun 02, 2018 at 12:17:12PM -0400, Sam Patton wrote:
> As far as application examples, check out this simple netcat-like
> program I use for testing:
> 
> https://github.com/EnIP/enhancedip/blob/master/userspace/netcat/netcat.c
> 
> Lines 61-67 show how to connect directly via an EnIP address.  The
> netcat-like application uses
> 
> a header file called eip.h.

OK so basically we need to use a new address structure (which makes sense),
however I'm not sure I understand how the system is supposed to know it has
to use EnIP instead of IPv4 if the sin_family remains the same :

...
   memset(&serv_addr, 0, sizeof(struct sockaddr_ein));
   serv_addr.sin_family= AF_INET;
   serv_addr.sin_port = htons(portno);
   serv_addr.sin_addr1=inet_addr(argv[1]);
   serv_addr.sin_addr2=inet_addr(argv[2]);

   if (connect(sockfd,(struct sockaddr *) &serv_addr,sizeof(serv_addr)) < 0) 
	error("ERROR connecting");
...

Does it solely rely on the 3rd argument to connect() to know that it
may read sin_addr2 ? If so, that sounds a bit fragile to me because I
*think* (but could be wrong) that right now any size at least as large
as sockaddr_in work for IPv4 (typically I think if you pass a struct
sockaddr_storage with its size it should work). So there would be a
risk of breaking applications if this is the case.

I suspect that using a different family would make things more robust
and more friendly to applications. But that's just a first opinion,
you have worked longer than me on this.

>  You can look at it here:
> 
> https://github.com/EnIP/enhancedip/blob/master/userspace/include/eip.h

Thank you. There's a problem with your #pragma pack there, it will
propagate to all struct declared after this file is included and
will force all subsequent structs to be packed. It would be preferable
to use __attribute__((packed)) on the struct or to use pragma pack()
safely (I've just seen it supports push/pop options).

> EnIP makes use of IPv6 AAAA records for DNS lookup.  We simply put
> 2001:0101 (which is an IPv6 experimental prefix) and
> 
> then we put the 64-bit EnIP address into the next 8 bytes of the
> address.  The remaining bytes are set to zero.

Does this require a significant amount of changes to the libc ?

> In the kernel, if you want to see how we convert the IPv6 DNS lookup
> into something connect() can manage,
> 
> check out the add_enhanced_ip() routine found here:
> 
> https://github.com/EnIP/enhancedip/blob/master/kernel/4.9.28/socket.c

Hmmm so both IPv4 and IPv6 addresses are supported and used together.
So that just makes me think that if in the end you need a little bit
of IPv6 to handle this, why not instead define a new way to map IPv6 to
Enhanced IPv4 ? I mean, you'd exclusively use AF_INET6 in applications,
within a dedicated address range, allowing you to naturally use all of
the IPv6 ecosystem in place in userland, but use IPv4 over the wire.
This way applications would possibly require even less modififications,
if any at all because they'd accept/connect/send/receive data for IPv6
address families that they all deal with nowadays, but this would rely
on an omni-present IPv4 infrastructure.

> The reason we had to do changes for openssh and not other applications
> (that use DNS) is openssh has a check to
> 
> see if the socket is using IP options.  If the socket does, sshd drops
> the connection.  I had to work around that to get openssh working

OK. They possibly do this to avoid source-routing.

> with EnIP.  The result: if you want to connect the netcat-like program
> with IP addresses you'll end up doing something like the
> 
> example above.  If you're using DNS (getaddrinfo) to connect(), it
> should just work (except for sshd as noted).
> 
> Here's the draft experimental RFC:
> https://tools.ietf.org/html/draft-chimiak-enhanced-ipv4-03

Yep I've found it on the site.

> I'll also note that I am doing this code part time as a hobby for a long
> time so I appreciate your help and support.  It would be really
> 
> great if the kernel community decided to pick this up, but if it's not a
> reality please let me know soonest so I can move on to a
> 
> different hobby.  :)

I *personally* think there is some value in what you're experimenting
with, and doing it as a hobby can leave you with the time needed to
collect pros and cons from various people. I'm just thinking that in
order to get some adoption, you need to be extremely careful not to
break any of the IPv4 applications developed in the last 38 years,
and by this, AF_INET+sizeof() scares me a little bit.

Regards,
Willy

Powered by blists - more mailing lists