tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: sendmsg(2) with IP_PKTINFO



On 08/12/2017 17:19, Ryo Shimizu wrote:


+				/* 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?

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?

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;


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

Roy


Home | Main Index | Thread Index | Old Index