Re: preparations for IP_SENDSRCADDR / (un)tangle pcb_bind...
On Tue, Aug 06, 2013 at 09:24:13PM +0200, Christopher Zimmermann wrote:
> Hi,
>
> I'm currently working towards an IP_SENDSRCADDR implementation.
> As a first step I moved the calls to in_pcbrehash() from
> in_pcb(dis)connect() and in_pcbbind() to the call sites of those
> functions. This should save some calls to in_pcbrehash() in the
> in_pcbconnect() codepath.
> Also I improved code sharing between the IPv4 and IPv6 stacks by
> implementing a generic in_pcbsetport used by both stacks.
> The next step will be to replace in_pcbconnect() by in_selectsrc()
> and in_pcbsetport() in udp_output() like the IPv6 stack does it.
> The splsoftnet() lock could then be removed. Also implementing
> IP_SENDSRCADDR will become trivial because in_selectsrc() won't
> modify the struct inpcb like in_pcbconnect() does at the moment.
>
> Not much testing yet. Just surfing the web and sending this mail...
>
> I'd really like some feedback on whether I'm on the right track or
> doing something stupid here. I have no experience in the OpenBSD
> IP-stack yet.
>
>
Moving the calls of of in_pcbrehash() to outside of in_pcb.c is wrong.
This is an internal function for the specific way fast PCB lookups are
done at the moment. Having that all over the place in higher level code
where someone does a bind or connect call is not an option IMO.
This needs to be done differently, sorry.
--
:wq Claudio
> Christopher
>
>
>
> Index: netinet/in_pcb.c
> ===
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.139
> diff -u -p -r1.139 in_pcb.c
> --- netinet/in_pcb.c 1 Jun 2013 13:25:40 - 1.139
> +++ netinet/in_pcb.c 6 Aug 2013 18:25:54 -
> @@ -218,7 +218,6 @@ in_pcbbind(struct inpcb *inp, struct mbu
> {
> struct socket *so = inp->inp_socket;
> struct inpcbtable *table = inp->inp_table;
> - u_int16_t *lastport = &inp->inp_table->inpt_lastport;
> struct sockaddr_in *sin;
> u_int16_t lport = 0;
> int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
> @@ -293,71 +292,121 @@ in_pcbbind(struct inpcb *inp, struct mbu
> inp->inp_laddr = sin->sin_addr;
> }
> if (lport == 0) {
> - u_int16_t first, last;
> - int count;
> + error = in_pcbsetport(inp, p);
> + if (error != 0)
> + return error;
> + }
> + else
> + inp->inp_lport = lport;
> + return (0);
> +}
>
> - if (inp->inp_flags & INP_HIGHPORT) {
> - first = ipport_hifirstauto; /* sysctl */
> - last = ipport_hilastauto;
> - } else if (inp->inp_flags & INP_LOWPORT) {
> - if ((error = suser(p, 0)))
> - return (EACCES);
> - first = IPPORT_RESERVED-1; /* 1023 */
> - last = 600;/* not IPPORT_RESERVED/2 */
> - } else {
> - first = ipport_firstauto; /* sysctl */
> - last = ipport_lastauto;
> - }
> +int
> +in_pcbsetport(struct inpcb *inp, struct proc *p)
> +{
> + struct socket *so = inp->inp_socket;
> + struct inpcbtable *table = inp->inp_table;
> + u_int16_t first, last;
> + u_int16_t *lastport = &inp->inp_table->inpt_lastport;
> + u_int16_t lport = 0;
> + int count;
> + int error;
> + int wild;
> + u_int rtableid;
> + void *faddrp, *laddrp;
> +#ifdef INET6
> + extern struct in6_addr zeroin6_addr;
>
> - /*
> - * Simple check to ensure all ports are not used up causing
> - * a deadlock here.
> - *
> - * We split the two cases (up and down) so that the direction
> - * is not being tested on each round of the loop.
> - */
> + if (sotopf(so) == PF_INET6) {
> + wild = INPLOOKUP_IPV6;
> + rtableid = 0;
> + faddrp = &zeroin6_addr;
> + laddrp = &inp->inp_laddr6;
> + }
> + else
> +#endif /* INET6 */
> + {
> + wild = 0;
> + rtableid = inp->inp_rtableid;
> + faddrp = &zeroin_addr;
> + laddrp = &inp->inp_laddr;
> + }
>
> - if (first > last) {
> - /*
> - * counting down
> - */
> - count = first - last;
> - if (count)
> - *lastport = first - arc4random_uniform(count);
> + if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) == 0 &&
> + ((so->so_proto->pr_flags & PR_CONNREQUIRED) == 0 ||
> + (so->so_options & SO_ACCEPTCONN) == 0))
> + wild |= INPLOOKUP_WILDCARD;
>
> - do {
> - if (count-- < 0)/* completely used? */
> -
include netinet/in_var.h in arch/dev
Hi, I have just removed a bunch of useless include netinet/in_var.h from the machine independent drivers. I suspect that they are also not needed in the architecture specific network drivers. Unfortunately I don't have any of these machines. So if you have access to one of macppc mvme68k octeon sgi sparc vax could you please test if this diff compiles. ok? bluhm Index: arch/macppc/dev/if_mc.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/macppc/dev/if_mc.c,v retrieving revision 1.14 diff -u -p -r1.14 if_mc.c --- arch/macppc/dev/if_mc.c 21 Apr 2010 03:03:26 - 1.14 +++ arch/macppc/dev/if_mc.c 7 Aug 2013 00:57:57 - @@ -56,7 +56,6 @@ #include #include #include -#include #include #endif Index: arch/mvme68k/dev/if_ie.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/mvme68k/dev/if_ie.c,v retrieving revision 1.40 diff -u -p -r1.40 if_ie.c --- arch/mvme68k/dev/if_ie.c10 Oct 2012 04:52:16 - 1.40 +++ arch/mvme68k/dev/if_ie.c7 Aug 2013 00:57:57 - @@ -120,7 +120,6 @@ Mode of operation: #ifdef INET #include #include -#include #include #include #endif Index: arch/mvme88k/dev/if_ie.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/mvme88k/dev/if_ie.c,v retrieving revision 1.45 diff -u -p -r1.45 if_ie.c --- arch/mvme88k/dev/if_ie.c10 Oct 2012 04:52:16 - 1.45 +++ arch/mvme88k/dev/if_ie.c7 Aug 2013 00:57:57 - @@ -119,7 +119,6 @@ Mode of operation: #ifdef INET #include #include -#include #include #include #endif Index: arch/octeon/dev/if_cnmac.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/octeon/dev/if_cnmac.c,v retrieving revision 1.10 diff -u -p -r1.10 if_cnmac.c --- arch/octeon/dev/if_cnmac.c 12 Apr 2013 15:22:26 - 1.10 +++ arch/octeon/dev/if_cnmac.c 7 Aug 2013 00:57:57 - @@ -66,7 +66,6 @@ #include #include -#include #include #include Index: arch/sgi/dev/if_iec.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sgi/dev/if_iec.c,v retrieving revision 1.8 diff -u -p -r1.8 if_iec.c --- arch/sgi/dev/if_iec.c 22 May 2012 19:24:59 - 1.8 +++ arch/sgi/dev/if_iec.c 7 Aug 2013 00:57:57 - @@ -101,7 +101,6 @@ #ifdef INET #include #include -#include #include #endif Index: arch/sgi/dev/if_mec.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sgi/dev/if_mec.c,v retrieving revision 1.25 diff -u -p -r1.25 if_mec.c --- arch/sgi/dev/if_mec.c 3 Oct 2012 22:46:09 - 1.25 +++ arch/sgi/dev/if_mec.c 7 Aug 2013 00:57:57 - @@ -85,7 +85,6 @@ #ifdef INET #include #include -#include #include #endif Index: arch/sgi/hpc/if_sq.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sgi/hpc/if_sq.c,v retrieving revision 1.8 diff -u -p -r1.8 if_sq.c --- arch/sgi/hpc/if_sq.c28 May 2012 17:03:35 - 1.8 +++ arch/sgi/hpc/if_sq.c7 Aug 2013 00:57:57 - @@ -57,7 +57,6 @@ #ifdef INET #include #include -#include #include #endif Index: arch/sparc/dev/be.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sparc/dev/be.c,v retrieving revision 1.43 diff -u -p -r1.43 be.c --- arch/sparc/dev/be.c 28 Nov 2008 02:44:17 - 1.43 +++ arch/sparc/dev/be.c 7 Aug 2013 00:57:57 - @@ -46,7 +46,6 @@ #ifdef INET #include #include -#include #include #include #endif Index: arch/sparc/dev/hme.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sparc/dev/hme.c,v retrieving revision 1.62 diff -u -p -r1.62 hme.c --- arch/sparc/dev/hme.c13 Aug 2009 17:01:31 - 1.62 +++ arch/sparc/dev/hme.c7 Aug 2013 00:57:57 - @@ -59,7 +59,6 @@ #ifdef INET #include #include -#include #include #include #include Index: arch/sparc/dev/if_gem_sbus.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sparc/dev/if_gem_sbus.c,v retrieving revision 1.1 diff -u -p -r1.1 if_gem_sbus.c --- arch/sparc/dev/if_gem_sbus.c13 Jul 2009 19:53:58 - 1.1 +++ arch/sparc/dev/if_gem_sbus.c7 Aug 2013 00:57:57 - @@ -48,7 +48,6 @@ #ifdef INET #include #include -#include #include #include #endif Index: arch/sparc/dev/if_ie.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sparc/dev/if_ie.c,v retrieving revision 1.42 diff -u -p -r1.42 if_ie.c --- arch/sparc/dev/if_i
preparations for IP_SENDSRCADDR / (un)tangle pcb_bind...
Hi,
I'm currently working towards an IP_SENDSRCADDR implementation.
As a first step I moved the calls to in_pcbrehash() from
in_pcb(dis)connect() and in_pcbbind() to the call sites of those
functions. This should save some calls to in_pcbrehash() in the
in_pcbconnect() codepath.
Also I improved code sharing between the IPv4 and IPv6 stacks by
implementing a generic in_pcbsetport used by both stacks.
The next step will be to replace in_pcbconnect() by in_selectsrc()
and in_pcbsetport() in udp_output() like the IPv6 stack does it.
The splsoftnet() lock could then be removed. Also implementing
IP_SENDSRCADDR will become trivial because in_selectsrc() won't
modify the struct inpcb like in_pcbconnect() does at the moment.
Not much testing yet. Just surfing the web and sending this mail...
I'd really like some feedback on whether I'm on the right track or
doing something stupid here. I have no experience in the OpenBSD
IP-stack yet.
Christopher
Index: netinet/in_pcb.c
===
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.139
diff -u -p -r1.139 in_pcb.c
--- netinet/in_pcb.c1 Jun 2013 13:25:40 - 1.139
+++ netinet/in_pcb.c6 Aug 2013 18:25:54 -
@@ -218,7 +218,6 @@ in_pcbbind(struct inpcb *inp, struct mbu
{
struct socket *so = inp->inp_socket;
struct inpcbtable *table = inp->inp_table;
- u_int16_t *lastport = &inp->inp_table->inpt_lastport;
struct sockaddr_in *sin;
u_int16_t lport = 0;
int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
@@ -293,71 +292,121 @@ in_pcbbind(struct inpcb *inp, struct mbu
inp->inp_laddr = sin->sin_addr;
}
if (lport == 0) {
- u_int16_t first, last;
- int count;
+ error = in_pcbsetport(inp, p);
+ if (error != 0)
+ return error;
+ }
+ else
+ inp->inp_lport = lport;
+ return (0);
+}
- if (inp->inp_flags & INP_HIGHPORT) {
- first = ipport_hifirstauto; /* sysctl */
- last = ipport_hilastauto;
- } else if (inp->inp_flags & INP_LOWPORT) {
- if ((error = suser(p, 0)))
- return (EACCES);
- first = IPPORT_RESERVED-1; /* 1023 */
- last = 600;/* not IPPORT_RESERVED/2 */
- } else {
- first = ipport_firstauto; /* sysctl */
- last = ipport_lastauto;
- }
+int
+in_pcbsetport(struct inpcb *inp, struct proc *p)
+{
+ struct socket *so = inp->inp_socket;
+ struct inpcbtable *table = inp->inp_table;
+ u_int16_t first, last;
+ u_int16_t *lastport = &inp->inp_table->inpt_lastport;
+ u_int16_t lport = 0;
+ int count;
+ int error;
+ int wild;
+ u_int rtableid;
+ void *faddrp, *laddrp;
+#ifdef INET6
+ extern struct in6_addr zeroin6_addr;
- /*
-* Simple check to ensure all ports are not used up causing
-* a deadlock here.
-*
-* We split the two cases (up and down) so that the direction
-* is not being tested on each round of the loop.
-*/
+ if (sotopf(so) == PF_INET6) {
+ wild = INPLOOKUP_IPV6;
+ rtableid = 0;
+ faddrp = &zeroin6_addr;
+ laddrp = &inp->inp_laddr6;
+ }
+ else
+#endif /* INET6 */
+ {
+ wild = 0;
+ rtableid = inp->inp_rtableid;
+ faddrp = &zeroin_addr;
+ laddrp = &inp->inp_laddr;
+ }
- if (first > last) {
- /*
-* counting down
-*/
- count = first - last;
- if (count)
- *lastport = first - arc4random_uniform(count);
+ if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) == 0 &&
+ ((so->so_proto->pr_flags & PR_CONNREQUIRED) == 0 ||
+(so->so_options & SO_ACCEPTCONN) == 0))
+ wild |= INPLOOKUP_WILDCARD;
- do {
- if (count-- < 0)/* completely used? */
- return (EADDRNOTAVAIL);
- --*lastport;
- if (*lastport > first || *lastport < last)
- *lastport = first;
- lport = htons(*lastport);
- } while (in_baddynamic(*lastport,
so->so_proto->pr_protocol) ||
- in_pcblookup(table, &zeroin_addr, 0,
- &inp->inp_laddr, lport, wild, inp->inp_rtable
Re: make IPv6 macros look nicer
On Tue, Aug 6, 2013 at 11:40 AM, Christopher Zimmermann wrote: > I'm currently working on the OpenBSD ip-stack for the first time. > Here's a small cosmetic improvement. > OK? No, because the system doesn't build with that. Philip Guenther
make IPv6 macros look nicer
Hi, I'm currently working on the OpenBSD ip-stack for the first time. Here's a small cosmetic improvement. OK? Christopher Index: netinet6/in6.h === RCS file: /cvs/src/sys/netinet6/in6.h,v retrieving revision 1.65 diff -u -p -r1.65 in6.h --- netinet6/in6.h 26 Jun 2013 09:12:40 - 1.65 +++ netinet6/in6.h 6 Aug 2013 18:26:05 - @@ -214,37 +214,37 @@ extern const struct in6_addr in6addr_lin * Unspecified */ #define IN6_IS_ADDR_UNSPECIFIED(a) \ - ((*(const u_int32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[4]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[8]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[12]) == 0)) + (((a)->s6_addr32[0] == 0) &&\ +((a)->s6_addr32[1] == 0) &&\ +((a)->s6_addr32[2] == 0) &&\ +((a)->s6_addr32[3] == 0)) /* * Loopback */ #define IN6_IS_ADDR_LOOPBACK(a)\ - ((*(const u_int32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[4]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[8]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[12]) == ntohl(1))) + (((a)->s6_addr32[0] == 0) &&\ +((a)->s6_addr32[1] == 0) &&\ +((a)->s6_addr32[2] == 0) &&\ +((a)->s6_addr32[3] == ntohl(1))) /* * IPv4 compatible */ #define IN6_IS_ADDR_V4COMPAT(a)\ - ((*(const u_int32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[4]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[8]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[12]) != 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[12]) != ntohl(1))) + (((a)->s6_addr32[0] == 0) &&\ +((a)->s6_addr32[1] == 0) &&\ +((a)->s6_addr32[2] == 0) &&\ +((a)->s6_addr32[3] != 0) &&\ +((a)->s6_addr32[3] != ntohl(1))) /* * Mapped */ #define IN6_IS_ADDR_V4MAPPED(a) \ - ((*(const u_int32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[4]) == 0) && \ -(*(const u_int32_t *)(const void *)(&(a)->s6_addr[8]) == ntohl(0x))) + (((a)->s6_addr32[0] == 0) &&\ +((a)->s6_addr32[1] == 0) &&\ +((a)->s6_addr32[2] == ntohl(0x))) /* * KAME Scope Values -- http://gmerlin.de OpenPGP: http://gmerlin.de/christopher.pub 1917 680A 723C BF3D 2CA3 0E44 7E24 D19F 34B8 2A2A
Re: include netinet/in_var.h in dev
On 6 August 2013 03:54, Alexander Bluhm wrote: > Hi, > > For an upcoming change in in6_var.h I would like to minimize the > impact. Most network drivers include netinet/in_var.h, but apparently > they don't have to. Can we remove these includes? > > compiled on amd64 and i386 > > ok? > > bluhm > should be fine. ok mikeb
Re: Inline assembly is hard...
On Tue, Aug 06, 2013 at 12:37:49AM +0200, Mark Kettenis wrote: > The atomic_setbits_int() and atomic_clearbits_int() implementations > use stwcx., which touches the condition code register. This means we > need to tell GCC that it gets clobbered. Fixes issues with > uvm_page_physload() which currently gets miscompiled. > > ok? > > > Index: atomic.h > === > RCS file: /cvs/src/sys/arch/powerpc/include/atomic.h,v > retrieving revision 1.4 > diff -u -p -r1.4 atomic.h > --- atomic.h 23 Mar 2011 16:54:36 - 1.4 > +++ atomic.h 5 Aug 2013 22:34:00 - > @@ -17,7 +17,7 @@ atomic_setbits_int(__volatile unsigned i > " or %0, %1, %0 \n" > " stwcx. %0, 0, %2 \n" > " bne-1b \n" > - " sync" : "=&r" (tmp) : "r" (v), "r" (uip) : "memory"); > + " sync" : "=&r" (tmp) : "r" (v), "r" (uip) : "cc", "memory"); > } > > static __inline void > @@ -30,7 +30,7 @@ atomic_clearbits_int(__volatile unsigned > " andc%0, %0, %1 \n" > " stwcx. %0, 0, %2 \n" > " bne-1b \n" > - " sync" : "=&r" (tmp) : "r" (v), "r" (uip) : "memory"); > + " sync" : "=&r" (tmp) : "r" (v), "r" (uip) : "cc", "memory"); > } > > #endif /* defined(_KERNEL) */ Obviously okay.
