On Fri, Nov 09, 2018 at 06:09:34PM +0100, Jan Klemkow wrote: > On Tue, Oct 30, 2018 at 07:13:24AM +0000, Jason McIntyre wrote: > > On Mon, Oct 29, 2018 at 11:55:52PM +0100, Jan Klemkow wrote: > > > On Sun, Oct 28, 2018 at 10:58:34PM +0000, Jason McIntyre wrote: > > > > On Sun, Oct 28, 2018 at 09:40:33PM +0100, Jan Klemkow wrote: > > > > > Unlike the manpage saids or one might think , sendto(2) sets errno to > > > > > EHOSTUNREACH instead of EACCES in cases of blocking by pf(4) or not > > > > > enabled broadcasts. Finally I ran into both cases and think, its time > > > > > to fix this issue. The diff suggests a new explanation that should > > > > > cover all error cases. > > > > > > > > > > Index: /usr/src/lib/libc/sys/send.2 > > > > > =================================================================== > > > > > RCS file: /cvs/src/lib/libc/sys/send.2,v > > > > > retrieving revision 1.32 > > > > > diff -u -p -r1.32 send.2 > > > > > --- /usr/src/lib/libc/sys/send.2 5 Oct 2017 12:30:16 -0000 > > > > > 1.32 > > > > > +++ /usr/src/lib/libc/sys/send.2 28 Oct 2018 20:16:20 -0000 > > > > > @@ -161,13 +161,13 @@ The operation may succeed when buffers b > > > > > The output queue for a network interface was full. > > > > > This generally indicates that the interface has stopped sending, > > > > > but may be caused by transient congestion. > > > > > -.It Bq Er EACCES > > > > > -The > > > > > -.Dv SO_BROADCAST > > > > > -option is not set on the socket, and a broadcast address > > > > > -was given as the destination. > > > > > .It Bq Er EHOSTUNREACH > > > > > -The destination address specified an unreachable host. > > > > > +The destination address specified an host that is unreachable, > > > > > > > > unless you're going for humour, "a host" rather than "an host". > > > > > > > > i can;t comment on the accuracy, but the rest of the diff reads ok. > > > > > > No humour (here in Germany), just a spelling mistakes :-) > > > Fixed version below. > > > > morning. > > > > to be fair, some people do still unambiguously use "an" before h* words. > > so as such it is not wrong (i think). still, our pages do not use that > > format. > > > > anyway, enough of the small stuff. who wants to review this page and > > actually ok or nok it? > > Hi, > > I printed the code path below to make it easier to review the diff. > While I was following the code path again, I found an inconsistency > between udp_output() and upd6_output(). Just upd_output() turns EACCES > to EHOSTUNREACH, udp6_output does not. The Diff below fixes udp6_output > and rounds up the sendto(2) manpage. > > Bye, > Jan > > Reviewer guide from sendto(2) to EHOSTUNREACH: > > sys_sendto() -> sendit() -> sosend() ---+ > | > +--------------------------------------+ > | > +-> udp_usrreq() -> udp_output(): > | ... > | error = ip_output(m, inp->inp_options, &inp->inp_route, > | (inp->inp_socket->so_options & SO_BROADCAST), inp->inp_moptions, > | inp, ipsecflowinfo); > | if (error == EACCES) /* translate pf(4) error for userland */ > | error = EHOSTUNREACH; > | ... > | > +-> udp_usrreq() -> udp6_output(): > | ... > | error = ip6_output(m, optp, &in6p->inp_route6, > | flags, in6p->inp_moptions6, in6p); > | if (error == EACCES) /* translate pf(4) error for userland */ > | error = EHOSTUNREACH; > | ... > | > +-> tcp_usrreq() -> tcp_output(): > ... > error = ip_output(m, tp->t_inpcb->inp_options, > tp->t_inpcb->inp_route, > ip_mtudisc ? IP_MTUDISC : 0), NULL, tp->t_inpcb, 0); > ... > error = ip6_output(m, tp->t_inpcb->inp_outputopts6, > &tp->t_inpcb->inp_route6, > 0, NULL, tp->t_inpcb); > ... > if (error == EACCES) /* translate pf(4) error for userland */ > error = EHOSTUNREACH; > ... > > Index: sys/netinet6/udp6_output.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/udp6_output.c,v > retrieving revision 1.56 > diff -u -p -r1.56 udp6_output.c > --- sys/netinet6/udp6_output.c 13 Sep 2018 19:53:58 -0000 1.56 > +++ sys/netinet6/udp6_output.c 9 Nov 2018 16:31:21 -0000 > @@ -235,6 +235,8 @@ udp6_output(struct inpcb *in6p, struct m > > error = ip6_output(m, optp, &in6p->inp_route6, > flags, in6p->inp_moptions6, in6p); > + if (error == EACCES) /* translate pf(4) error for userland */ > + error = EHOSTUNREACH; > goto releaseopt; > > release: > Index: /usr/src/lib/libc/sys/send.2 > =================================================================== > RCS file: /cvs/src/lib/libc/sys/send.2,v > retrieving revision 1.32 > diff -u -p -r1.32 send.2 > --- /usr/src/lib/libc/sys/send.2 5 Oct 2017 12:30:16 -0000 1.32 > +++ /usr/src/lib/libc/sys/send.2 29 Oct 2018 22:48:24 -0000 > @@ -161,13 +161,13 @@ The operation may succeed when buffers b > The output queue for a network interface was full. > This generally indicates that the interface has stopped sending, > but may be caused by transient congestion. > -.It Bq Er EACCES > -The > -.Dv SO_BROADCAST > -option is not set on the socket, and a broadcast address > -was given as the destination. > .It Bq Er EHOSTUNREACH > -The destination address specified an unreachable host. > +The destination address specified a host that is unreachable, > +blocked by > +.Xr pf 4 > +or is broadcast communication which was not enabled > +via the socket option > +.Dv SO_BROADCAST . > .It Bq Er EINVAL > The > .Fa flags >
I really don't like this translation and therefor change of semantics of an old documented errno code. If I remember correctly this was added because of the fear that EACCES was not properly handled when used with write(2) since it is not documented there but neither is EHOSTUNREACH. I would prefer to remove this errno translation magic for pf(4) in the midlayer of the network code. As shown it is always prone for errors. -- :wq Claudio
