sendmsg(2) with IP

Hi,

Post by Ryo Shimizu
any comment? or ok to commit?

This is a welcome change (OpenVPN on NetBSD needs this for proper
UDP operation on a system with multiple IPv4 addresses, and the lack
of kernel-side support made NetBSD the last platform where this isn't
working properly yet).

I am not qualified to review the patch, though.

gert

--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany ***@greenie.muc.de
fax: +49-89-35655025 ***@net.informatik.tu-muenchen.de

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Christos Zoulas

2017-08-01 14:24:23 UTC

Post by Ryo Shimizu
Hi,
I implemented IP_PKTINFO for sendmsg(2). For recvmsg(2) IP_PKTINFO is already
supported, but it is not supported for sending. This causes problems
described in https://mail-index.netbsd.org/tech-net/2013/09/22/msg004252.html
patch is here.
http://www.netbsd.org/~ryo/ip_pktinfo.patch
or https://github.com/ryo/netbsd-src/compare/master...ip_pktinfo
- udp_output()
- process IP_PKTINFO in cmsg
- specified ip_pktinfo->ipi_addr, check the address is bindable, and use it.
- specified ip_pktinfo->ipi_ifindex, pick up an address from the interface,
and IP_ROUTETOIFINDEX flag and imo.imo_multicast_if_index are passed to
ip_output() to output from the interface. Multicast already has similar
function, therefore I used ip_moptions argument for specifying ifindex.
- rip_output()
- just about same as udp_output() except checking pcb.
- ip_output()
- if IP_ROUTETOIFINDEX is set, checking that whether packets can be output
from the specified interface. loopback, p2p interface, or direct address
are ok. Otherwise, checking that the destination address has a gateway
address on the interface.
any comment? or ok to commit?

Why don't you share the code in udp_output and rip_output?
Otherwise LGTM.

christos

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Ryo Shimizu

2017-08-05 11:05:16 UTC

Post by Christos Zoulas

Why don't you share the code in udp_output and rip_output?
Otherwise LGTM.

Thank you for your advice.
I revised to add new function ip_pktopts() for processing control messages.

latest patch is here:
http://www.netbsd.org/~ryo/ip_pktinfo.patch2.diff
or https://github.com/ryo/netbsd-src/compare/master...ip_pktinfo

If there is no problem, I'll commit next week.

--
ryo shimizu

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Christos Zoulas

2017-08-05 12:47:22 UTC

On Aug 5, 8:05pm, ***@nerv.org (Ryo Shimizu) wrote:
-- Subject: Re: sendmsg(2) with IP_PKTINFO

| Thank you for your advice.
| I revised to add new function ip_pktopts() for processing control messages.
|
| latest patch is here:
| http://www.netbsd.org/~ryo/ip_pktinfo.patch2.diff
| or https://github.com/ryo/netbsd-src/compare/master...ip_pktinfo
|
| If there is no problem, I'll commit next week.

LGTM!

Thanks for working on this.

christos

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Roy Marples

2017-12-07 14:41:56 UTC

Hi Ryo

Post by Ryo Shimizu
I implemented IP_PKTINFO for sendmsg(2). For recvmsg(2) IP_PKTINFO is already
supported, but it is not supported for sending. This causes problems
described in https://mail-index.netbsd.org/tech-net/2013/09/22/msg004252.html
patch is here.
http://www.netbsd.org/~ryo/ip_pktinfo.patch
or https://github.com/ryo/netbsd-src/compare/master...ip_pktinfo

+ /* Checking laddr:port already in use? */
+ xinp = in_pcblookup_bind(&udbtable,
+ laddr.sin_addr, inp->inp_lport);
+ if ((xinp != NULL) && (xinp != inp)) {
+ error = EADDRINUSE;
+ goto release;
+ }
+ break;

Why is this check needed and why is it UDP specific?

In dhcpcd's case it's already bound the local address the socket and the
kernel then claims the address is already in use which is wrong.

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Ryo Shimizu

2017-12-08 05:46:24 UTC

+ /* Checking laddr:port already in use? */
+ xinp = in_pcblookup_bind(&udbtable,
+ laddr.sin_addr, inp->inp_lport);
+ if ((xinp != NULL) && (xinp != inp)) {
+ error = EADDRINUSE;
+ goto release;
+ }
+ break;
Why is this check needed and why is it UDP specific?
In dhcpcd's case it's already bound the local address the socket and the
kernel then claims the address is already in use which is wrong.

I thought that it should fail with EADDRINUSE if sendmsg with IP_PKTINFO
using address:port same as already bound on other socket.

Certainly, this behavior is different from IP6_PKTINFO and linux.
Should I fix to be able to PKTINFO with address:port that another socket bound?

--
ryo shimizu

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Roy Marples

2017-12-08 09:30:11 UTC

+ /* Checking laddr:port already in use? */
+ xinp = in_pcblookup_bind(&udbtable,
+ laddr.sin_addr, inp->inp_lport);
+ if ((xinp != NULL) && (xinp != inp)) {
+ error = EADDRINUSE;
+ goto release;
+ }
+ break;
Why is this check needed and why is it UDP specific?
In dhcpcd's case it's already bound the local address the socket and the
kernel then claims the address is already in use which is wrong.

Yes please!

Consider this use case:
https://roy.marples.name/git/dhcpcd.git/tree/src/dhcp.c#n1687
I am only using IP_PKTINFO to set the outbound interface, not influence
any address.

As things stand the kernel check noted above returns EADDRINUSE.
If I either remove the kernel check OR disable IP_PKTINFO in dhcpcd then
the message is sent.

To make the code easier to read, I bind to the unspecified
address:BOOTPC so that ICMP unreachable messages are not sent by the
kernel during DHCP init. This socket is kept open for the lifetime of
dhcpcd and just drained down.

When dhcpcd needs to unicast to the DHCP server, it opens another
socket, binds DHCPADDRESS:BOOTPC and then sends. This works fine without
IP_PKTINFO and I would like to use IP_PKTINFO just to force the outbound
interface. We're not actively listening on this socket, just sending. My
expectation is that this should work.

Can you fix this please?

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Ryo Shimizu

2017-12-08 17:19:19 UTC

Post by Roy Marples
+ /* Checking laddr:port already in use? */
+ xinp = in_pcblookup_bind(&udbtable,
+ laddr.sin_addr, inp->inp_lport);
+ if ((xinp != NULL) && (xinp != inp)) {
+ error = EADDRINUSE;
+ goto release;
+ }
+ break;
Why is this check needed and why is it UDP specific?
In dhcpcd's case it's already bound the local address the socket and the
kernel then claims the address is already in use which is wrong.

Yes please!
https://roy.marples.name/git/dhcpcd.git/tree/src/dhcp.c#n1687
I am only using IP_PKTINFO to set the outbound interface, not influence
any address.
As things stand the kernel check noted above returns EADDRINUSE.
If I either remove the kernel check OR disable IP_PKTINFO in dhcpcd then
the message is sent.
To make the code easier to read, I bind to the unspecified
address:BOOTPC so that ICMP unreachable messages are not sent by the
kernel during DHCP init. This socket is kept open for the lifetime of
dhcpcd and just drained down.
When dhcpcd needs to unicast to the DHCP server, it opens another
socket, binds DHCPADDRESS:BOOTPC and then sends. This works fine without
IP_PKTINFO and I would like to use IP_PKTINFO just to force the outbound
interface. We're not actively listening on this socket, just sending. My
expectation is that this should work.
Can you fix this please?

Ok, Please try this patch. If no problem for dhcpcd, I'll commit.

cvs -q diff -aup .
Index: ip_output.c
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/ip_output.c,v
retrieving revision 1.285
diff -a -u -p -r1.285 ip_output.c
--- ip_output.c 17 Nov 2017 07:37:12 -0000 1.285
+++ ip_output.c 8 Dec 2017 16:59:52 -0000
@@ -1411,9 +1411,8 @@ ip_pktinfo_prepare(const struct in_pktin
*/
int
ip_setpktopts(struct mbuf *control, struct ip_pktopts *pktopts, int *flags,
- struct inpcb *inp, kauth_cred_t cred, int uproto)
+ struct inpcb *inp, kauth_cred_t cred)
{
- struct inpcb *xinp;
struct cmsghdr *cm;
struct in_pktinfo *pktinfo;
int error;
@@ -1453,16 +1452,6 @@ ip_setpktopts(struct mbuf *control, stru
cred);
if (error != 0)
return error;
-
- if ((uproto == IPPROTO_UDP) &&
- !in_nullhost(pktopts->ippo_laddr.sin_addr)) {
- /* Checking laddr:port already in use? */
- xinp = in_pcblookup_bind(&udbtable,
- pktopts->ippo_laddr.sin_addr,
- inp->inp_lport);
- if ((xinp != NULL) && (xinp != inp))
- return EADDRINUSE;
- }
break;
default:
return ENOPROTOOPT;
Index: ip_var.h
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/ip_var.h,v
retrieving revision 1.120
diff -a -u -p -r1.120 ip_var.h
--- ip_var.h 10 Aug 2017 04:31:58 -0000 1.120
+++ ip_var.h 8 Dec 2017 17:00:03 -0000
@@ -215,7 +215,7 @@ void in_init(void);

int ip_ctloutput(int, struct socket *, struct sockopt *);
int ip_setpktopts(struct mbuf *, struct ip_pktopts *, int *,
- struct inpcb *, kauth_cred_t, int);
+ struct inpcb *, kauth_cred_t);
void ip_drain(void);
void ip_drainstub(void);
void ip_freemoptions(struct ip_moptions *);
Index: raw_ip.c
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/raw_ip.c,v
retrieving revision 1.166
diff -a -u -p -r1.166 raw_ip.c
--- raw_ip.c 10 Aug 2017 04:31:58 -0000 1.166
+++ raw_ip.c 8 Dec 2017 17:00:08 -0000
@@ -322,8 +322,7 @@ rip_output(struct mbuf *m, struct inpcb

/* Setup IP outgoing packet options */
memset(&pktopts, 0, sizeof(pktopts));
- error = ip_setpktopts(control, &pktopts, &flags, inp, cred,
- IPPROTO_RAW);
+ error = ip_setpktopts(control, &pktopts, &flags, inp, cred);
if (control != NULL)
m_freem(control);
if (error != 0)
Index: udp_usrreq.c
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.235
diff -a -u -p -r1.235 udp_usrreq.c
--- udp_usrreq.c 10 Aug 2017 04:31:58 -0000 1.235
+++ udp_usrreq.c 8 Dec 2017 17:00:19 -0000
@@ -804,8 +804,7 @@ udp_output(struct mbuf *m, struct inpcb

/* Setup IP outgoing packet options */
memset(&pktopts, 0, sizeof(pktopts));
- error = ip_setpktopts(control, &pktopts, &flags, inp, cred,
- IPPROTO_UDP);
+ error = ip_setpktopts(control, &pktopts, &flags, inp, cred);
if (error != 0)
goto release;

--
ryo shimizu

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Roy Marples

2017-12-08 18:15:47 UTC

Ok, Please try this patch. If no problem for dhcpcd, I'll commit.
cvs -q diff -aup .
Index: ip_output.c
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/ip_output.c,v
retrieving revision 1.285
diff -a -u -p -r1.285 ip_output.c
--- ip_output.c 17 Nov 2017 07:37:12 -0000 1.285
+++ ip_output.c 8 Dec 2017 16:59:52 -0000
@@ -1411,9 +1411,8 @@ ip_pktinfo_prepare(const struct in_pktin
*/
int
ip_setpktopts(struct mbuf *control, struct ip_pktopts *pktopts, int *flags,
- struct inpcb *inp, kauth_cred_t cred, int uproto)
+ struct inpcb *inp, kauth_cred_t cred)
{
- struct inpcb *xinp;
struct cmsghdr *cm;
struct in_pktinfo *pktinfo;
int error;
@@ -1453,16 +1452,6 @@ ip_setpktopts(struct mbuf *control, stru
cred);
if (error != 0)
return error;
-
- if ((uproto == IPPROTO_UDP) &&
- !in_nullhost(pktopts->ippo_laddr.sin_addr)) {
- /* Checking laddr:port already in use? */
- xinp = in_pcblookup_bind(&udbtable,
- pktopts->ippo_laddr.sin_addr,
- inp->inp_lport);
- if ((xinp != NULL) && (xinp != inp))
- return EADDRINUSE;
- }
break;
return ENOPROTOOPT;
Index: ip_var.h
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/ip_var.h,v
retrieving revision 1.120
diff -a -u -p -r1.120 ip_var.h
--- ip_var.h 10 Aug 2017 04:31:58 -0000 1.120
+++ ip_var.h 8 Dec 2017 17:00:03 -0000
@@ -215,7 +215,7 @@ void in_init(void);
int ip_ctloutput(int, struct socket *, struct sockopt *);
int ip_setpktopts(struct mbuf *, struct ip_pktopts *, int *,
- struct inpcb *, kauth_cred_t, int);
+ struct inpcb *, kauth_cred_t);
void ip_drain(void);
void ip_drainstub(void);
void ip_freemoptions(struct ip_moptions *);
Index: raw_ip.c
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/raw_ip.c,v
retrieving revision 1.166
diff -a -u -p -r1.166 raw_ip.c
--- raw_ip.c 10 Aug 2017 04:31:58 -0000 1.166
+++ raw_ip.c 8 Dec 2017 17:00:08 -0000
@@ -322,8 +322,7 @@ rip_output(struct mbuf *m, struct inpcb
/* Setup IP outgoing packet options */
memset(&pktopts, 0, sizeof(pktopts));
- error = ip_setpktopts(control, &pktopts, &flags, inp, cred,
- IPPROTO_RAW);
+ error = ip_setpktopts(control, &pktopts, &flags, inp, cred);
if (control != NULL)
m_freem(control);
if (error != 0)
Index: udp_usrreq.c
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.235
diff -a -u -p -r1.235 udp_usrreq.c
--- udp_usrreq.c 10 Aug 2017 04:31:58 -0000 1.235
+++ udp_usrreq.c 8 Dec 2017 17:00:19 -0000
@@ -804,8 +804,7 @@ udp_output(struct mbuf *m, struct inpcb
/* Setup IP outgoing packet options */
memset(&pktopts, 0, sizeof(pktopts));
- error = ip_setpktopts(control, &pktopts, &flags, inp, cred,
- IPPROTO_UDP);
+ error = ip_setpktopts(control, &pktopts, &flags, inp, cred);
if (error != 0)
goto release;

Works great, thanks!
Please commit and consider requesting the prior patch and this pulled up
to -8.

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Ryo Shimizu

2017-12-11 18:18:04 UTC

Post by Roy Marples
Works great, thanks!
Please commit and consider requesting the prior patch and this pulled up
to -8.

Thanks for testing.
I have commited, and sent pullup-req http://releng.netbsd.org/cgi-bin/req-8.cgi?show=445

I'll make patches for -7 and -6. Please wait a little.

--
ryo shimizu

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

Roy Marples

2017-12-12 11:23:02 UTC