Re: mp-safe carp_iamatch6()

[Hrvoje Popovski]

On 22.11.2017. 15:48, Martin Pieuchot wrote:
> Hrvoje Popovski reported the following panic when testing my diff to
> unlock protocol inputs function:
> 
>  panic() at panic+0x128
>  __assert(814d1114,800022755d80,809ce000,800022755e20)
>  carp_ourether(ff0003c0c290,809ce000) at carp_ourether
>  nd6_ns_input(20,0,809c9800) at nd6_ns_input+0x4cf
>  icmp6_input(18,81a95af0,1,3a) at icmp6_input+0x3d4
>  
> ip_deliver(800022756020,80002275602c,80019080,817ee880)
>  ip6intr() at ip6intr+0x7b
> 
> The problem comes from carp_iamatch6() which is not yet MP-safe.  Since
> it is now identical to carp_iamatch(), let's use this one instead.


Hi,

with this diff i can't trigger panic as before.

Thank you ...

macppc: default to MBR for new installs

[Stefan Sperling]

This flips the default response for the macppc disk layout question
from HFS to MBR.

I use an MBR on all my macppc machines. Booting OpenBSD is much simpler
this way. I don't see why I cannot just hit enter for this question on
new installs. I'd rather let Mac software archaeologists who wish to
dual-boot an obsolete Mac OS with OpenBSD do the extra work.

There is a follow-up question in md_prep_MBR which prints a warning
and confirms this choice again. So I don't expect this change will
cause anyone to overwrite an HFS partition table by accident.

OK?

Index: install.md
===
RCS file: /cvs/src/distrib/macppc/ramdisk/install.md,v
retrieving revision 1.71
diff -u -p -r1.71 install.md
--- install.md  28 Jul 2017 18:15:44 -  1.71
+++ install.md  22 Nov 2017 22:11:54 -
@@ -144,7 +144,7 @@ md_prep_disklabel() {
PARTTABLE=
while [[ -z $PARTTABLE ]]; do
resp=MBR
-   disk_has $_disk hfs && ask "Use HFS or MBR partition table?" HFS
+   disk_has $_disk hfs && ask "Use HFS or MBR partition table?" MBR
case $resp in
[mM]*)  md_prep_MBR $_disk && PARTTABLE=MBR ;;
[hH]*)  md_prep_HFS $_disk && PARTTABLE=HFS ;;

less: use monotonic clock for line number echo

[Scott Cheloha]

Hi,

The "Calculating line numbers" echo in less(1) is meant to
fire after an interval has elapsed, so we should use the
monotonic clock to measure.

Currently, if the system time is changed during the execution
of find_linenum() the echo fires too quickly or not at all.

--
Scott Cheloha

Index: usr.bin/less/linenum.c
===
RCS file: /cvs/src/usr.bin/less/linenum.c,v
retrieving revision 1.16
diff -u -p -r1.16 linenum.c
--- usr.bin/less/linenum.c  30 Dec 2016 19:52:43 -  1.16
+++ usr.bin/less/linenum.c  22 Nov 2017 22:11:52 -
@@ -33,6 +33,10 @@
  * called to make sure we cache line numbers often enough.
  */
 
+#include 
+
+#include 
+
 #include "less.h"
 
 /*
@@ -197,14 +201,30 @@ add_lnum(off_t linenum, off_t pos)
 }
 
 static int loopcount;
-static time_t startime;
+static struct timespec timeout;
+
+static void
+timeout_set(int seconds)
+{
+   clock_gettime(CLOCK_MONOTONIC, );
+   timeout.tv_sec += seconds;
+}
+
+static int
+timeout_elapsed(void)
+{
+   struct timespec now;
+
+   clock_gettime(CLOCK_MONOTONIC, );
+   return timespeccmp(, , >=);
+}
 
 static void
 longish(void)
 {
if (loopcount >= 0 && ++loopcount > 100) {
loopcount = 0;
-   if (time(NULL) >= startime + LONGTIME) {
+   if (timeout_elapsed()) {
ierror("Calculating line numbers", NULL);
loopcount = -1;
}
@@ -274,7 +294,7 @@ find_linenum(off_t pos)
 * The decision is based on which way involves
 * traversing fewer bytes in the file.
 */
-   startime = time(NULL);
+   timeout_set(LONGTIME);
if (p ==  || pos - p->prev->pos < p->pos - pos) {
/*
 * Go forward.

Re: mp-safe carp_iamatch6()

[Alexander Bluhm]

On Wed, Nov 22, 2017 at 03:48:43PM +0100, Martin Pieuchot wrote:
> Hrvoje Popovski reported the following panic when testing my diff to
> unlock protocol inputs function:
> 
>  panic() at panic+0x128
>  __assert(814d1114,800022755d80,809ce000,800022755e20)
>  carp_ourether(ff0003c0c290,809ce000) at carp_ourether
>  nd6_ns_input(20,0,809c9800) at nd6_ns_input+0x4cf
>  icmp6_input(18,81a95af0,1,3a) at icmp6_input+0x3d4
>  
> ip_deliver(800022756020,80002275602c,80019080,817ee880)
>  ip6intr() at ip6intr+0x7b
> 
> The problem comes from carp_iamatch6() which is not yet MP-safe.  Since
> it is now identical to carp_iamatch(), let's use this one instead.
> 
> ok?

OK bluhm@

> Index: netinet/ip_carp.h
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 ip_carp.h
> --- netinet/ip_carp.h 30 May 2017 12:09:27 -  1.43
> +++ netinet/ip_carp.h 22 Nov 2017 14:42:37 -
> @@ -199,7 +199,6 @@ void   carp_carpdev_state(void *);
>  void  carp_group_demote_adj(struct ifnet *, int, char *);
>  int   carp6_proto_input(struct mbuf **, int *, int, int);
>  int   carp_iamatch(struct ifnet *);
> -int   carp_iamatch6(struct ifnet *);
>  struct ifnet *carp_ourether(void *, u_int8_t *);
>  int   carp_output(struct ifnet *, struct mbuf *, struct sockaddr *,
>struct rtentry *);
> Index: netinet/ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.319
> diff -u -p -r1.319 ip_carp.c
> --- netinet/ip_carp.c 21 Nov 2017 09:08:55 -  1.319
> +++ netinet/ip_carp.c 22 Nov 2017 14:42:37 -
> @@ -1352,22 +1352,6 @@ carp_iamatch(struct ifnet *ifp)
>   return (match);
>  }
>  
> -#ifdef INET6
> -int
> -carp_iamatch6(struct ifnet *ifp)
> -{
> - struct carp_softc *sc = ifp->if_softc;
> - struct carp_vhost_entry *vhe = SRPL_FIRST_LOCKED(>carp_vhosts);
> -
> - KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
> -
> - if (vhe->state == MASTER)
> - return (1);
> -
> - return (0);
> -}
> -#endif /* INET6 */
> -
>  struct ifnet *
>  carp_ourether(void *v, u_int8_t *ena)
>  {
> Index: netinet6/in6.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 in6.c
> --- netinet6/in6.c4 Nov 2017 13:11:54 -   1.218
> +++ netinet6/in6.c22 Nov 2017 14:43:08 -
> @@ -1287,7 +1287,7 @@ in6_ifawithscope(struct ifnet *oifp, str
>* Never use a carp address of an interface which is not
>* the master.
>*/
> - if (ifp->if_type == IFT_CARP && !carp_iamatch6(ifp))
> + if (ifp->if_type == IFT_CARP && !carp_iamatch(ifp))
>   continue;
>  #endif
>  
> Index: netinet6/nd6_nbr.c
> ===
> RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
> retrieving revision 1.121
> diff -u -p -r1.121 nd6_nbr.c
> --- netinet6/nd6_nbr.c11 Aug 2017 21:24:20 -  1.121
> +++ netinet6/nd6_nbr.c22 Nov 2017 14:42:36 -
> @@ -218,7 +218,7 @@ nd6_ns_input(struct mbuf *m, int off, in
>   /* (1) and (3) check. */
>   ifa = _ifpwithaddr(ifp, )->ia_ifa;
>  #if NCARP > 0
> - if (ifp->if_type == IFT_CARP && ifa && !carp_iamatch6(ifp))
> + if (ifp->if_type == IFT_CARP && ifa && !carp_iamatch(ifp))
>   ifa = NULL;
>  #endif
>  
> @@ -668,7 +668,7 @@ nd6_na_input(struct mbuf *m, int off, in
>* Ignore NAs silently for carp addresses if we're not
>* the CARP master.
>*/
> - if (ifp->if_type == IFT_CARP && !carp_iamatch6(ifp))
> + if (ifp->if_type == IFT_CARP && !carp_iamatch(ifp))
>   goto freeit;
>  #endif
>   log(LOG_ERR,
> @@ -1014,7 +1014,7 @@ nd6_na_output(struct ifnet *ifp, struct 
>  
>  #if NCARP > 0
>   /* Do not send NAs for carp addresses if we're not the CARP master. */
> - if (ifp->if_type == IFT_CARP && !carp_iamatch6(ifp))
> + if (ifp->if_type == IFT_CARP && !carp_iamatch(ifp))
>   goto bad;
>  #endif
>  

armv7 stand diff for clang

[Mark Kettenis]

Needed to build efiboot with clang.

ok?


Index: sys/arch/armv7/stand/efiboot/Makefile
===
RCS file: /cvs/src/sys/arch/armv7/stand/efiboot/Makefile,v
retrieving revision 1.7
diff -u -p -r1.7 Makefile
--- sys/arch/armv7/stand/efiboot/Makefile   4 Jul 2017 14:54:15 -   
1.7
+++ sys/arch/armv7/stand/efiboot/Makefile   22 Nov 2017 20:46:57 -
@@ -32,6 +32,7 @@ SRCS+=ufs.c
 
 .PATH: ${S}/lib/libkern/arch/arm ${S}/lib/libkern
 SRCS+= divsi3.S divdi3.c moddi3.c qdivrem.c strlcpy.c strlen.c
+SRCS+= __aeabi_ldivmod.S
 
 .PATH: ${S}/lib/libz
 SRCS+= adler32.c crc32.c inflate.c inftrees.c

Re: Regulators as sensors?

[STeve Andre']

On 11/22/17 13:06, Anders Andersson wrote:

On Tue, Nov 21, 2017 at 11:19 PM, STeve Andre'  wrote:

On 11/21/17 16:31, Mark Kettenis wrote:


The diff below exposes voltage regulators as sensors.  This makes it
easy to look at the current settings of these regulators.  The
downside is that these aren't really sensors as the voltages are not
actually measured.

The functionality is optional; callers can pass NULL in the
regulator_register() if the regulators aren't particularly
interesting.

This is what it looks like on the rk3399-firefly:

milhaud$ sysctl hw.sensors
hw.sensors.rktemp0.temp0=23.89 degC (CPU)
hw.sensors.rktemp0.temp1=28.75 degC (GPU)
hw.sensors.rkpmic0.volt0=0.90 VDC (vdd_cpu_l)
hw.sensors.rkpmic0.volt1=1.80 VDC (vcc1v8_dvp)
hw.sensors.rkpmic0.volt2=1.80 VDC (vcc1v8_pmu)
hw.sensors.rkpmic0.volt3=3.00 VDC (vcc_sd)
hw.sensors.rkpmic0.volt4=1.80 VDC (vcca1v8_codec)
hw.sensors.rkpmic0.volt5=3.00 VDC (vcc_3v0)

thoughts?



As someone who does hardware stuff, having easy access to these sensorts
can't hurt, and might be useful in some situations.  I've measured voltages
before and found during extreme temperature conditions things changed. So
it's possibly useful and doesn't cost much.


This reply illustrates the problem, and I think it won't be the last
time someone misunderstands the feature.

They are *not* sensors, so they will not vary under load. They don't
reflect the actual voltage, so they are useless for checking if the
hardware works as it should. I bet that a lot of people will still
assume that they can be used as such, leading people to believe that
everything is OK with their hardware when it's not.




That's true.  Hardware often lies.  That's why I like the ability to 
monitor stuff.


--STeve Andre'

Re: Regulators as sensors?

[Anders Andersson]

On Tue, Nov 21, 2017 at 11:19 PM, STeve Andre'  wrote:
> On 11/21/17 16:31, Mark Kettenis wrote:
>>
>> The diff below exposes voltage regulators as sensors.  This makes it
>> easy to look at the current settings of these regulators.  The
>> downside is that these aren't really sensors as the voltages are not
>> actually measured.
>>
>> The functionality is optional; callers can pass NULL in the
>> regulator_register() if the regulators aren't particularly
>> interesting.
>>
>> This is what it looks like on the rk3399-firefly:
>>
>> milhaud$ sysctl hw.sensors
>> hw.sensors.rktemp0.temp0=23.89 degC (CPU)
>> hw.sensors.rktemp0.temp1=28.75 degC (GPU)
>> hw.sensors.rkpmic0.volt0=0.90 VDC (vdd_cpu_l)
>> hw.sensors.rkpmic0.volt1=1.80 VDC (vcc1v8_dvp)
>> hw.sensors.rkpmic0.volt2=1.80 VDC (vcc1v8_pmu)
>> hw.sensors.rkpmic0.volt3=3.00 VDC (vcc_sd)
>> hw.sensors.rkpmic0.volt4=1.80 VDC (vcca1v8_codec)
>> hw.sensors.rkpmic0.volt5=3.00 VDC (vcc_3v0)
>>
>> thoughts?
>
>
> As someone who does hardware stuff, having easy access to these sensorts
> can't hurt, and might be useful in some situations.  I've measured voltages
> before and found during extreme temperature conditions things changed. So
> it's possibly useful and doesn't cost much.

This reply illustrates the problem, and I think it won't be the last
time someone misunderstands the feature.

They are *not* sensors, so they will not vary under load. They don't
reflect the actual voltage, so they are useless for checking if the
hardware works as it should. I bet that a lot of people will still
assume that they can be used as such, leading people to believe that
everything is OK with their hardware when it's not.

Re: openssl s_time, speed: use monotime for absolute interval measurement

[Scott Cheloha]

Whoops, ignore that last patch, it lacked the
static changes in apps_posix.c

--
Scott Cheloha

Index: usr.bin/openssl/apps_posix.c
===
RCS file: /cvs/src/usr.bin/openssl/apps_posix.c,v
retrieving revision 1.2
diff -u -p -r1.2 apps_posix.c
--- usr.bin/openssl/apps_posix.c13 Sep 2015 12:41:01 -  1.2
+++ usr.bin/openssl/apps_posix.c22 Nov 2017 16:39:42 -
@@ -116,31 +116,48 @@
  * Functions that need to be overridden by non-POSIX operating systems.
  */
 
-#include 
+#include 
+#include 
 
-#include 
+#include 
 
 #include "apps.h"
 
-double
-app_tminterval(int stop, int usertime)
+static double
+real_interval(int stop)
 {
-   double ret = 0;
-   struct tms rus;
-   clock_t now = times();
-   static clock_t tmstart;
-
-   if (usertime)
-   now = rus.tms_utime;
-
-   if (stop == TM_START)
-   tmstart = now;
-   else {
-   long int tck = sysconf(_SC_CLK_TCK);
-   ret = (now - tmstart) / (double) tck;
+   static struct timespec start;
+   struct timespec elapsed, now;
+
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (stop) {
+   timespecsub(, , );
+   return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
}
+   start = now;
+   return 0.0;
+}
 
-   return (ret);
+static double
+user_interval(int stop)
+{
+   static struct timeval start;
+   struct timeval elapsed;
+   struct rusage now;
+
+   getrusage(RUSAGE_SELF, );
+   if (stop) {
+   timersub(_utime, , );
+   return elapsed.tv_sec + elapsed.tv_usec / 100.0;
+   }
+   start = now.ru_utime;
+   return 0.0;
+}
+
+double
+app_tminterval(int stop, int usertime)
+{
+   return (usertime) ? user_interval(stop) : real_interval(stop);
 }
 
 int
Index: usr.bin/openssl/s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.18
diff -u -p -r1.18 s_time.c
--- usr.bin/openssl/s_time.c2 Nov 2017 00:31:49 -   1.18
+++ usr.bin/openssl/s_time.c22 Nov 2017 16:39:42 -
@@ -63,11 +63,13 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -248,7 +250,7 @@ s_time_main(int argc, char **argv)
double totalTime = 0.0;
int nConn = 0;
SSL *scon = NULL;
-   time_t finishtime;
+   struct timespec finishtime, now;
int ret = 1;
char buf[1024 * 8];
int ver;
@@ -330,10 +332,12 @@ s_time_main(int argc, char **argv)
/* Loop and time how long it takes to make connections */
 
bytes_read = 0;
-   finishtime = time(NULL) + s_time_config.maxtime;
+   clock_gettime(CLOCK_MONOTONIC, );
+   finishtime.tv_sec += s_time_config.maxtime;
tm_Time_F(START);
for (;;) {
-   if (finishtime < time(NULL))
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (timespeccmp(, , <))
break;
if ((scon = doConnection(NULL)) == NULL)
goto end;
@@ -383,7 +387,7 @@ s_time_main(int argc, char **argv)
nConn, totalTime, ((double) nConn / totalTime), bytes_read);
printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
nConn,
-   (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+   (long long)(now.tv_sec - finishtime.tv_sec + s_time_config.maxtime),
bytes_read / nConn);
 
/*
@@ -422,14 +426,16 @@ next:
nConn = 0;
totalTime = 0.0;
 
-   finishtime = time(NULL) + s_time_config.maxtime;
+   clock_gettime(CLOCK_MONOTONIC, );
+   finishtime.tv_sec += s_time_config.maxtime;
 
printf("starting\n");
bytes_read = 0;
tm_Time_F(START);
 
for (;;) {
-   if (finishtime < time(NULL))
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (timespeccmp(, , <))
break;
if ((doConnection(scon)) == NULL)
goto end;
@@ -475,7 +481,7 @@ next:
printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes 
read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
nConn,
-   (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+   (long long)(now.tv_sec - finishtime.tv_sec + s_time_config.maxtime),
bytes_read / nConn);
 
ret = 0;

Re: openssl s_time, speed: use monotime for absolute interval measurement

[Scott Cheloha]

On Sat, Nov 18, 2017 at 05:27:14PM +0100, Jeremie Courreges-Anglas wrote:
> On Sat, Nov 11 2017, Scott Cheloha  wrote:
> > [...]
> 
> I doubt that timersub/timespecsub are a big problem to add to -portable,
> they're just macros.  clock_gettime and getrusage seem to already be
> used in libressl(-portable), there's autoconf glue for the former at
> least.

Sounds good to me.

> > Thoughts and feedback?
> 
> More comment inline.
> 
> > [...]
> >
> >  double
> > -app_tminterval(int stop, int usertime)
> > +real_interval(int new)
> 
> I suggest you keep app_tminterval() as the entry point here.  As
> mentioned by the comment, non-POSIX systems need to implement the same
> interface, and indeed apps/openssl/apps_win.c in libressl-portable also
> provides app_tminterval().

Okay, retained.

> > [...]
> >
> > +   static struct timespec elapsed, now, start;
> 
> Only "start" need to be static here.
> 
> > [...]
> >
> > +double
> > +user_interval(int new)
> > +{
> > +   static struct timeval elapsed, start;
> > +   static struct rusage now;
> 
> Same here.

Fixed and fixed.

> > [...]
> >
> > -   if (usertime == 0 && !mr)
> > +   if (usertime == 0 && !mr) {
> > BIO_printf(bio_err, "You have chosen to measure elapsed time 
> > instead of user CPU time.\n");
> > +   interval_function = real_interval;
> 
> It seems a bit overengineered to use a function pointer for this.  No
> need to change this code if you just keep the app_tminterval()
> interface.

Yep.  Keeping the interface eliminates the need for changes in
speed.c and apps.h.

Also, you need  to use timersub, timespecsub, etc, so
I added that to s_time.c and apps_posix.c.

--
Scott Cheloha

Index: usr.bin/openssl/apps_posix.c
===
RCS file: /cvs/src/usr.bin/openssl/apps_posix.c,v
retrieving revision 1.2
diff -u -p -r1.2 apps_posix.c
--- usr.bin/openssl/apps_posix.c13 Sep 2015 12:41:01 -  1.2
+++ usr.bin/openssl/apps_posix.c21 Nov 2017 23:48:12 -
@@ -116,31 +116,48 @@
  * Functions that need to be overridden by non-POSIX operating systems.
  */
 
-#include 
+#include 
+#include 
 
-#include 
+#include 
 
 #include "apps.h"
 
-double
-app_tminterval(int stop, int usertime)
+static double
+real_interval(int stop)
 {
-   double ret = 0;
-   struct tms rus;
-   clock_t now = times();
-   static clock_t tmstart;
-
-   if (usertime)
-   now = rus.tms_utime;
-
-   if (stop == TM_START)
-   tmstart = now;
-   else {
-   long int tck = sysconf(_SC_CLK_TCK);
-   ret = (now - tmstart) / (double) tck;
+   static struct timespec start;
+   static struct timespec elapsed, now;
+
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (stop) {
+   timespecsub(, , );
+   return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
}
+   start = now;
+   return 0.0;
+}
 
-   return (ret);
+static double
+user_interval(int stop)
+{
+   static struct timeval start;
+   static struct timeval elapsed;
+   static struct rusage now;
+
+   getrusage(RUSAGE_SELF, );
+   if (stop) {
+   timersub(_utime, , );
+   return elapsed.tv_sec + elapsed.tv_usec / 100.0;
+   }
+   start = now.ru_utime;
+   return 0.0;
+}
+
+double
+app_tminterval(int stop, int usertime)
+{
+   return (usertime) ? user_interval(stop) : real_interval(stop);
 }
 
 int
Index: usr.bin/openssl/s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.18
diff -u -p -r1.18 s_time.c
--- usr.bin/openssl/s_time.c2 Nov 2017 00:31:49 -   1.18
+++ usr.bin/openssl/s_time.c21 Nov 2017 23:48:12 -
@@ -63,11 +63,13 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -248,7 +250,7 @@ s_time_main(int argc, char **argv)
double totalTime = 0.0;
int nConn = 0;
SSL *scon = NULL;
-   time_t finishtime;
+   struct timespec finishtime, now;
int ret = 1;
char buf[1024 * 8];
int ver;
@@ -330,10 +332,12 @@ s_time_main(int argc, char **argv)
/* Loop and time how long it takes to make connections */
 
bytes_read = 0;
-   finishtime = time(NULL) + s_time_config.maxtime;
+   clock_gettime(CLOCK_MONOTONIC, );
+   finishtime.tv_sec += s_time_config.maxtime;
tm_Time_F(START);
for (;;) {
-   if (finishtime < time(NULL))
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (timespeccmp(, , <))
break;
if ((scon = doConnection(NULL)) == NULL)
goto end;
@@ -383,7 +387,7 @@ s_time_main(int argc, char **argv)
nConn, totalTime, ((double) nConn / totalTime), bytes_read);
 

Re: Add Diffie-Hellman group negotiation to iked

[Patrick Wildt]

On 2017/06/25 21:44, Tim Stewart wrote:
> My first patch did, in fact, break Child SAs rekeying.  I have a new
> patch at the end of this message that simply restricts DH group
> negotiation to IKE SAs (I *think* that DH group guessing only applies to
> IKE SAs, and perhaps only the IKE_SA_INIT exchange, but I'm still
> working through the RFC).  This may not be the ultimate solution, but it
> does allow us to move forward.

Reading RFC 7296 it looks like throwing INVALID_KE_PAYLOAD is fine for
both establishing the IKE SA and rekeying the Child SAs.  If we select a
proposal from the msg that uses a different DH group than the one that's
used in the KEi (in the same msg) we need to throw INVALID_KE_PAYLOAD.

Since all messages subsequent to the initial exchange must be encrypted,
the INVALID_KE_PAYLOAD message on rekeying Child SAs must be encrypted.
Apparently with the previous diff the Child SA rekeying failed.  This is
because the code sends the INVALID_KE_PAYLOAD response unencrypted.

Also I have found inconsistencies in handling INVALID_KE_PAYLOAD with us
acting as initiator.  I will take a look at both cases and will follow
up.

Patrick

Azalia Apollo Lake Cosmetic Patch

[Kevin Chadwick]

Before realising the motherboard switch was set to i2s and wondering
why I had no codecs I cooked up this patch. Almost entirely
cosmetic, Change it, Take it or leave it, obviously.

Note APPLE comment in azalia_codec.c added simply because quirk
present for CS4206 and CS4208 and so guessed at relevance for 4207.

Thanks


Index: src/sys/dev/pci/azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.238
diff -u -p -u -p -r1.238 azalia.c
--- src/sys/dev/pci/azalia.c22 Sep 2017 06:33:44 -  1.238
+++ src/sys/dev/pci/azalia.c22 Nov 2017 15:42:47 -
@@ -458,6 +458,7 @@ azalia_configure_pci(azalia_t *az)
case PCI_PRODUCT_INTEL_C600_HDA:
case PCI_PRODUCT_INTEL_C610_HDA:
case PCI_PRODUCT_INTEL_BSW_HDA:
+   case PCI_PRODUCT_INTEL_APOLLO_LAKE_HDA:
reg = azalia_pci_read(az->pc, az->tag,
INTEL_PCIE_NOSNOOP_REG);
reg &= INTEL_PCIE_NOSNOOP_MASK;
Index: src/sys/dev/pci/azalia_codec.c
===
RCS file: /cvs/src/sys/dev/pci/azalia_codec.c,v
retrieving revision 1.172
diff -u -p -u -p -r1.172 azalia_codec.c
--- src/sys/dev/pci/azalia_codec.c  28 Mar 2017 04:54:44 -  1.172
+++ src/sys/dev/pci/azalia_codec.c  22 Nov 2017 15:45:45 -
@@ -72,6 +72,13 @@ azalia_codec_init_vtbl(codec_t *this)
AZ_QRK_GPIO_UNMUTE_3;
}
break;
+case 0x10134207:
+   this->name = "Cirrus Logic CS4207";
+   if (this->subid == 0x72708086) {/* APPLE_MBA? */
+   this->qrks |= AZ_QRK_GPIO_UNMUTE_0 |
+   AZ_QRK_GPIO_UNMUTE_1;
+   }
+   break;
case 0x10134208:
this->name = "Cirrus Logic CS4208";
if (this->subid == 0x72708086) {/* APPLE_MBA6_1 */
Index: src/sys/dev/pci/pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1825
diff -u -p -u -p -r1.1825 pcidevs
--- src/sys/dev/pci/pcidevs 28 Aug 2017 09:43:18 -  1.1825
+++ src/sys/dev/pci/pcidevs 22 Nov 2017 15:48:08 -
@@ -4557,6 +4557,7 @@ product INTEL CORE7G_U_GT2_2  0x591d  HD G
 product INTEL CORE7G_Y_GT2 0x591e  HD Graphics 615
 product INTEL CORE7G_U_GT3_15W 0x5926  Iris Plus Graphics 640
 product INTEL CORE7G_U_GT3_28W 0x5927  Iris Plus Graphics 650
+product INTEL APOLLO_LAKE_HDA  0x5a98  Apollo Lake HD Audio
 product INTEL 5100_HB  0x65c0  5100 Host
 product INTEL 5100_PCIE_2  0x65e2  5100 PCIE
 product INTEL 5100_PCIE_3  0x65e3  5100 PCIE

race-less nd6_timer

[Martin Pieuchot]

Hrvoje Popovski the great, found another crazy race related with NDP:

 # ifconfig vlan300 destroy
 kernel: protection fault trap, code=0
 Stopped at  nd6_ns_output+0x30: cmpb$0xff,0(%r13)
 ddb{0}> trace
 nd6_ns_output(ff0786bfe210,81434800,...) at nd6_ns_output+0x30
 nd6_llinfo_timer(ff0786bfe210) at nd6_llinfo_timer+0x1a8
 softclock_thread(0) at softclock_thread+0xc6
 end trace frame: 0x0, count: -3
 
What happened in this case is a complicated use-after-free.  The 'softclock'
thread was sleeping on the NET_LOCK() trying to execute the already schedule
per-ND timer, while the route entry was freed by ifconfig(8).

The problems is in nd6_rtrequest(). For the RTM_DELETE case, we do not
check if timeout_del(9) did not remove anything:

970:TAILQ_REMOVE(_list, ln, ln_list);
971:nd6_llinfo_settimer(ln, -1);


There are multiple ways to fix this problem:

 1/ Use the new proposed API timeout_barrier(9) from dlg@.  However this
introduces a new sleeping point and requires a NET_LOCK()/UNLOCK()
dance.

 2/ Refcount `ln' or abuse the `rt' refcounting.  But I'd prefer to
reduce the number of references.

 3/ Move the timeout outside of the short lived structure.

Diff below implements 3/ because it seems the simplest approach to
me and reduce differences with ARP a bit further.  This pave the way
for further MP in the L2 layer.  The idea is that a single global list
will have to be protected and no refcounting should be necessary.

Briefly tested here, I appreciate more tests and reviews.

Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.221
diff -u -p -r1.221 nd6.c
--- netinet6/nd6.c  31 Oct 2017 22:05:13 -  1.221
+++ netinet6/nd6.c  22 Nov 2017 15:01:05 -
@@ -67,7 +67,8 @@
 #define ND6_RECALC_REACHTM_INTERVAL (60 * 120) /* 2 hours */
 
 /* timer values */
-time_t nd6_expire_time = -1;   /* at which time_uptime nd6_expire runs */
+intnd6_timer_next  = -1;   /* at which time_uptime nd6_timer runs */
+time_t nd6_expire_next = -1;   /* at which time_uptime nd6_expire runs */
 intnd6_delay   = 5;/* delay first probe time 5 second */
 intnd6_umaxtries   = 3;/* maximum unicast query */
 intnd6_mmaxtries   = 3;/* maximum multicast query */
@@ -90,13 +91,15 @@ int nd6_inuse, nd6_allocated;
 
 int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
 
+void nd6_timer(void *);
 void nd6_slowtimo(void *);
 void nd6_expire(void *);
 void nd6_expire_timer(void *);
 void nd6_invalidate(struct rtentry *);
 void nd6_free(struct rtentry *);
-void nd6_llinfo_timer(void *);
+void nd6_llinfo_timer(struct rtentry *);
 
+struct timeout nd6_timer_to;
 struct timeout nd6_slowtimo_ch;
 struct timeout nd6_expire_timeout;
 struct task nd6_expire_task;
@@ -120,6 +123,7 @@ nd6_init(void)
nd6_init_done = 1;
 
/* start timer */
+   timeout_set_proc(_timer_to, nd6_timer, _timer_to);
timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
timeout_set(_expire_timeout, nd6_expire_timer, NULL);
@@ -297,45 +301,57 @@ skip1:
  * ND6 timer routine to handle ND6 entries
  */
 void
-nd6_llinfo_settimer(struct llinfo_nd6 *ln, int secs)
+nd6_llinfo_settimer(struct llinfo_nd6 *ln, unsigned int secs)
 {
-   if (secs < 0) {
-   ln->ln_rt->rt_expire = 0;
-   timeout_del(>ln_timer_ch);
-   } else {
-   ln->ln_rt->rt_expire = time_uptime + secs;
-   timeout_add_sec(>ln_timer_ch, secs);
+   time_t expire = time_uptime + secs;
+
+   ln->ln_rt->rt_expire = expire;
+   if (!timeout_pending(_timer_to) || expire < nd6_timer_next) {
+   nd6_timer_next = expire;
+   timeout_add_sec(_timer_to, secs);
}
 }
 
 void
-nd6_llinfo_timer(void *arg)
+nd6_timer(void *arg)
 {
-   struct llinfo_nd6 *ln;
-   struct rtentry *rt;
-   struct sockaddr_in6 *dst;
-   struct ifnet *ifp;
-   struct nd_ifinfo *ndi = NULL;
+   struct llinfo_nd6 *ln, *nln;
+   time_t expire = time_uptime + nd6_gctimer;
+   int secs;
 
NET_LOCK();
+   TAILQ_FOREACH_SAFE(ln, _list, ln_list, nln) {
+   struct rtentry *rt = ln->ln_rt;
 
-   ln = (struct llinfo_nd6 *)arg;
+   if (rt->rt_expire && rt->rt_expire <= time_uptime)
+   nd6_llinfo_timer(rt);
 
-   if ((rt = ln->ln_rt) == NULL)
-   panic("ln->ln_rt == NULL");
-   if ((ifp = if_get(rt->rt_ifidx)) == NULL) {
-   NET_UNLOCK();
-   return;
+   if (rt->rt_expire && rt->rt_expire < expire)
+   expire = rt->rt_expire;
}
-   ndi = ND_IFINFO(ifp);
-   dst = satosin6(rt_key(rt));
 
-   /* sanity check */
-   if (rt->rt_llinfo != NULL && (struct llinfo_nd6 

mp-safe carp_iamatch6()

[Martin Pieuchot]

Hrvoje Popovski reported the following panic when testing my diff to
unlock protocol inputs function:

 panic() at panic+0x128
 __assert(814d1114,800022755d80,809ce000,800022755e20)
 carp_ourether(ff0003c0c290,809ce000) at carp_ourether
 nd6_ns_input(20,0,809c9800) at nd6_ns_input+0x4cf
 icmp6_input(18,81a95af0,1,3a) at icmp6_input+0x3d4
 ip_deliver(800022756020,80002275602c,80019080,817ee880)
 ip6intr() at ip6intr+0x7b

The problem comes from carp_iamatch6() which is not yet MP-safe.  Since
it is now identical to carp_iamatch(), let's use this one instead.

ok?

Index: netinet/ip_carp.h
===
RCS file: /cvs/src/sys/netinet/ip_carp.h,v
retrieving revision 1.43
diff -u -p -r1.43 ip_carp.h
--- netinet/ip_carp.h   30 May 2017 12:09:27 -  1.43
+++ netinet/ip_carp.h   22 Nov 2017 14:42:37 -
@@ -199,7 +199,6 @@ void carp_carpdev_state(void *);
 voidcarp_group_demote_adj(struct ifnet *, int, char *);
 int carp6_proto_input(struct mbuf **, int *, int, int);
 int carp_iamatch(struct ifnet *);
-int carp_iamatch6(struct ifnet *);
 struct ifnet   *carp_ourether(void *, u_int8_t *);
 int carp_output(struct ifnet *, struct mbuf *, struct sockaddr *,
 struct rtentry *);
Index: netinet/ip_carp.c
===
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.319
diff -u -p -r1.319 ip_carp.c
--- netinet/ip_carp.c   21 Nov 2017 09:08:55 -  1.319
+++ netinet/ip_carp.c   22 Nov 2017 14:42:37 -
@@ -1352,22 +1352,6 @@ carp_iamatch(struct ifnet *ifp)
return (match);
 }
 
-#ifdef INET6
-int
-carp_iamatch6(struct ifnet *ifp)
-{
-   struct carp_softc *sc = ifp->if_softc;
-   struct carp_vhost_entry *vhe = SRPL_FIRST_LOCKED(>carp_vhosts);
-
-   KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
-
-   if (vhe->state == MASTER)
-   return (1);
-
-   return (0);
-}
-#endif /* INET6 */
-
 struct ifnet *
 carp_ourether(void *v, u_int8_t *ena)
 {
Index: netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.218
diff -u -p -r1.218 in6.c
--- netinet6/in6.c  4 Nov 2017 13:11:54 -   1.218
+++ netinet6/in6.c  22 Nov 2017 14:43:08 -
@@ -1287,7 +1287,7 @@ in6_ifawithscope(struct ifnet *oifp, str
 * Never use a carp address of an interface which is not
 * the master.
 */
-   if (ifp->if_type == IFT_CARP && !carp_iamatch6(ifp))
+   if (ifp->if_type == IFT_CARP && !carp_iamatch(ifp))
continue;
 #endif
 
Index: netinet6/nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.121
diff -u -p -r1.121 nd6_nbr.c
--- netinet6/nd6_nbr.c  11 Aug 2017 21:24:20 -  1.121
+++ netinet6/nd6_nbr.c  22 Nov 2017 14:42:36 -
@@ -218,7 +218,7 @@ nd6_ns_input(struct mbuf *m, int off, in
/* (1) and (3) check. */
ifa = _ifpwithaddr(ifp, )->ia_ifa;
 #if NCARP > 0
-   if (ifp->if_type == IFT_CARP && ifa && !carp_iamatch6(ifp))
+   if (ifp->if_type == IFT_CARP && ifa && !carp_iamatch(ifp))
ifa = NULL;
 #endif
 
@@ -668,7 +668,7 @@ nd6_na_input(struct mbuf *m, int off, in
 * Ignore NAs silently for carp addresses if we're not
 * the CARP master.
 */
-   if (ifp->if_type == IFT_CARP && !carp_iamatch6(ifp))
+   if (ifp->if_type == IFT_CARP && !carp_iamatch(ifp))
goto freeit;
 #endif
log(LOG_ERR,
@@ -1014,7 +1014,7 @@ nd6_na_output(struct ifnet *ifp, struct 
 
 #if NCARP > 0
/* Do not send NAs for carp addresses if we're not the CARP master. */
-   if (ifp->if_type == IFT_CARP && !carp_iamatch6(ifp))
+   if (ifp->if_type == IFT_CARP && !carp_iamatch(ifp))
goto bad;
 #endif
 

[PATCH] Enable PCMCIA ne(4) on macppc

[Donovan Watteau]

Hi,

I have an old SMC 8041TX EZCard that happens to work in the PCMCIA
slot of my PowerBook G4, when it's enabled with the following diff.

dmesg also included.

Index: GENERIC
===
RCS file: /cvs/src/sys/arch/macppc/conf/GENERIC,v
retrieving revision 1.262
diff -p -u -r1.262 GENERIC
--- GENERIC 23 Oct 2017 03:13:58 -  1.262
+++ GENERIC 22 Nov 2017 01:06:04 -
@@ -335,6 +335,7 @@ cardbus*at cardslot?
 pcmcia*at cardslot?
 cbb*   at pci?
 cardslot*  at cbb?
+ne*at pcmcia?  # PCMCIA based NE2000 ethernet
 ep*at pcmcia?  # PCMCIA based 3C5xx ethernet
 an*at pcmcia?  # Cisco/Aironet
 ath*   at cardbus? # Atheros AR5k (802.11a/b/g)

[ using 1138284 bytes of bsd ELF symbol table ]
console out [ATY,Jasper_A] console in [keyboard], using USB
using parent ATY,JasperParent:: memaddr b800, size 800 : consaddr 
b8008000 : ioaddr b002, size 2: width 1280 linebytes 1280 height 854 
depth 8
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2017 OpenBSD. All rights reserved.  https://www.OpenBSD.org

OpenBSD 6.2-current (GENERIC) #0: Wed Nov 22 02:35:56 CET 2017
ts...@macg4.my.domain:/usr/src/sys/arch/macppc/compile/GENERIC
real mem = 1073741824 (1024MB)
avail mem = 1028153344 (980MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root: model PowerBook5,6
cpu0 at mainbus0: 7447A (Revision 0x102): 1666 MHz: 512KB L2 cache
mem0 at mainbus0
spdmem0 at mem0: 1GB DDR SDRAM non-parity PC2700CL2.5
memc0 at mainbus0: uni-n rev 0xd2
"hw-clock" at memc0 not configured
kiic0 at memc0 offset 0xf8001000
iic0 at kiic0
adt0 at iic0 addr 0x2e: adt7467 rev 0x71
lmtemp0 at iic0 addr 0x49: ds1775
asms0 at iic0 addr 0x58: rev 1.34, version 0.1
mpcpcibr0 at mainbus0 pci: uni-north
pci0 at mpcpcibr0 bus 0
pchb0 at pci0 dev 11 function 0 "Apple UniNorth AGP" rev 0x00
agp at pchb0 not configured
radeondrm0 at pci0 dev 16 function 0 "ATI Radeon Mobility M10" rev 0x00
drm0 at radeondrm0
radeondrm0: irq 48
mpcpcibr1 at mainbus0 pci: uni-north
pci1 at mpcpcibr1 bus 0
macobio0 at pci1 dev 23 function 0 "Apple Intrepid" rev 0x00
openpic0 at macobio0 offset 0x4: version 0x4614 feature 3f0302 LE
macgpio0 at macobio0 offset 0x50
"modem-reset" at macgpio0 offset 0x1d not configured
"modem-power" at macgpio0 offset 0x1c not configured
"accelerometer-1" at macgpio0 offset 0x13 not configured
"accelerometer-2" at macgpio0 offset 0x14 not configured
"headphone-mute" at macgpio0 offset 0x1f not configured
"amp-mute" at macgpio0 offset 0x20 not configured
"hw-reset" at macgpio0 offset 0x25 not configured
"linein-detect" at macgpio0 offset 0xc not configured
"headphone-detect" at macgpio0 offset 0x17 not configured
dfs0 at macgpio0 offset 0x1b: speeds: 1666, 833 MHz
macgpio1 at macgpio0 offset 0x9: irq 47
"programmer-switch" at macgpio0 offset 0x11 not configured
"gpio4" at macgpio0 offset 0x1e not configured
"escc-legacy" at macobio0 offset 0x12000 not configured
zs0 at macobio0 offset 0x13000: irq 22,23
zstty0 at zs0 channel 0
zstty1 at zs0 channel 1
snapper0 at macobio0 offset 0x0: irq 30,1,2
"timer" at macobio0 offset 0x15000 not configured
adb0 at macobio0 offset 0x16000
apm0 at adb0: battery flags 0x1, 0% charged
piic0 at adb0
iic1 at piic0
"backlight" at macobio0 offset 0xf300 not configured
kiic1 at macobio0 offset 0x18000
iic2 at kiic1
wdc0 at macobio0 offset 0x2 irq 24: DMA
atapiscsi0 at wdc0 channel 0 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0:  ATAPI 5/cdrom 
removable
cd0(wdc0:0:0): using BIOS timings, DMA mode 2
audio0 at snapper0
bwi0 at pci1 dev 18 function 0 "Broadcom BCM4306" rev 0x03: irq 52, address XXX
cbb0 at pci1 dev 19 function 0 "TI PCI1510 CardBus" rev 0x00: irq 53
ohci0 at pci1 dev 26 function 0 "Apple Intrepid USB" rev 0x00: irq 29, version 
1.0, legacy support
ohci1 at pci1 dev 27 function 0 "NEC USB" rev 0x43: irq 63, version 1.0
ohci2 at pci1 dev 27 function 1 "NEC USB" rev 0x43: irq 63, version 1.0
ehci0 at pci1 dev 27 function 2 "NEC USB" rev 0x04: irq 63
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 configuration 1 interface 0 "NEC EHCI root hub" rev 2.00/1.00 
addr 1
cardslot0 at cbb0 slot 0 flags 0
cardbus0 at cardslot0: bus 1 device 0 cacheline 0x8, lattimer 0x20
pcmcia0 at cardslot0
usb1 at ohci0: USB revision 1.0
uhub1 at usb1 configuration 1 interface 0 "Apple OHCI root hub" rev 1.00/1.00 
addr 1
usb2 at ohci1: USB revision 1.0
uhub2 at usb2 configuration 1 interface 0 "NEC OHCI root hub" rev 1.00/1.00 
addr 1
usb3 at ohci2: USB revision 1.0
uhub3 at usb3 configuration 1 interface 0 "NEC OHCI root hub" rev 1.00/1.00 
addr 1
mpcpcibr2 at mainbus0 pci: uni-north
pci2 at mpcpcibr2 bus 0
kauaiata0 at pci2 dev 13 function 0 "Apple Intrepid ATA" rev 0x00
wdc1 at kauaiata0 

Re: cvsweb: cvs annotate error

[Stuart Henderson]

On 2017/11/22 14:48, Alexander Bluhm wrote:
> On Wed, Nov 22, 2017 at 03:30:09PM +0800, Michael W. Bombardieri wrote:
> > Sorry if this is the wrong list, but I found a couple of branch
> > revisions in file src/sys/dev/midi.c which produce an error when
> > clicking "annotate" link in cvsweb.
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/midi.c?annotate=1.40.6.1
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/midi.c?annotate=1.42.4.1
> > 
> > The error is "Error: Error occured during annotate: error".
> > Posting it here in case someone has seen it before.
> 
> Seems to be a cvs mirror problem.  When I run it against my local
> cvs repository:
> 
> $ cvs -R annotate -r1.40.6.1 sys/dev/midi.c  
> Annotations for sys/dev/midi.c
> ***
> cvs [annotate aborted]: unexpected EOF reading RCS file 
> /data/mirror/openbsd/cvs/src/sys/dev/midi.c,v
> 
> When I use cvs.openbsd.org as server it works.
> 
> bluhm

It's not a problem with a particular mirror - it's a cvsync problem.

Re: avoid pcb lookup in pf forward

[Alexandr Nedvedicky]

Hello,

On Wed, Nov 22, 2017 at 01:45:39PM +0100, Alexander Bluhm wrote:
> On Wed, Nov 22, 2017 at 09:49:06AM +0100, Alexandr Nedvedicky wrote:
> > >   /* if reassembled packet passed, create new fragments */
> > > - if (pf_status.reass && action == PF_PASS && pd.m && fwdir == PF_FWD) {
> > > + if (pf_status.reass && action == PF_PASS && pd.m && fwdir == PF_FWD &&
> > > + pd.af == AF_INET6) {
> > >   struct m_tag*mtag;
> > >  
> > >   if ((mtag = m_tag_find(pd.m, PACKET_TAG_PF_REASSEMBLED, NULL)))
> > 
> > I wonder whether the test 'fwdir == PF_FWD' is correct. I need to think more
> > about what happens to reassembled packets in PF_OUT direction. I suggest to
> > deal with it in separate thread.
> 
> IPv6 routers are not allowed to fragment.  We have end to end path
> MTU discovery.  As pf analyzes reassembled packets, we have to
> fragment them to the same size after forwarding.  This is only done
> for IPv6 to preserve PMTU.
> 
> For non-forwarded outgoing packets we use the regular fragment code
> in ip6_output().  ip_output() will handle all IPv4 fragments.

thank you for putting my feet back to ground. Now I see it, the
local outbound packets are inspected by pf_test() first and then get
fragmented.

regards
sasha

Re: cvsweb: cvs annotate error

[Alexander Bluhm]

On Wed, Nov 22, 2017 at 03:30:09PM +0800, Michael W. Bombardieri wrote:
> Sorry if this is the wrong list, but I found a couple of branch
> revisions in file src/sys/dev/midi.c which produce an error when
> clicking "annotate" link in cvsweb.
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/midi.c?annotate=1.40.6.1
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/midi.c?annotate=1.42.4.1
> 
> The error is "Error: Error occured during annotate: error".
> Posting it here in case someone has seen it before.

Seems to be a cvs mirror problem.  When I run it against my local
cvs repository:

$ cvs -R annotate -r1.40.6.1 sys/dev/midi.c  
Annotations for sys/dev/midi.c
***
cvs [annotate aborted]: unexpected EOF reading RCS file 
/data/mirror/openbsd/cvs/src/sys/dev/midi.c,v

When I use cvs.openbsd.org as server it works.

bluhm

Re: sbunlock() w/o KERNEL_LOCK()

[Alexander Bluhm]

On Wed, Nov 22, 2017 at 01:01:25PM +0100, Martin Pieuchot wrote:
> When we converted most of the splsoftnet() to NET_LOCK() we ended up
> with some "lock" inversions between the socket buffer "lock" and the
> NET_LOCK().  At that time the KERNEL_LOCK() was serializing access to
> `sb_flags'.
> 
> We're now moving towards running sockets functions w/o KERNEL_LOCK()
> in protocol input paths.  So I'd like to assert that the socket lock
> is held when sb_flags are modified.
> 
> There's currently one exception to this rule, FIFO kqueue filters,
> that I'll address in a later diff.
> 
> ok?

OK bluhm@

> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.207
> diff -u -p -r1.207 uipc_socket.c
> --- kern/uipc_socket.c4 Nov 2017 14:13:53 -   1.207
> +++ kern/uipc_socket.c22 Nov 2017 11:22:02 -
> @@ -453,7 +453,7 @@ restart:
>   (atomic || space < so->so_snd.sb_lowat))) {
>   if ((so->so_state & SS_NBIO) || (flags & MSG_DONTWAIT))
>   snderr(EWOULDBLOCK);
> - sbunlock(>so_snd);
> + sbunlock(so, >so_snd);
>   error = sbwait(so, >so_snd);
>   so->so_state &= ~SS_ISSENDING;
>   if (error)
> @@ -497,7 +497,7 @@ restart:
>  
>  release:
>   so->so_state &= ~SS_ISSENDING;
> - sbunlock(>so_snd);
> + sbunlock(so, >so_snd);
>  out:
>   sounlock(s);
>   m_freem(top);
> @@ -736,7 +736,7 @@ restart:
>   }
>   SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1");
>   SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1");
> - sbunlock(>so_rcv);
> + sbunlock(so, >so_rcv);
>   error = sbwait(so, >so_rcv);
>   sounlock(s);
>   if (error)
> @@ -957,7 +957,7 @@ dontblock:
>   SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 2");
>   error = sbwait(so, >so_rcv);
>   if (error) {
> - sbunlock(>so_rcv);
> + sbunlock(so, >so_rcv);
>   sounlock(s);
>   return (0);
>   }
> @@ -993,7 +993,7 @@ dontblock:
>   }
>   if (orig_resid == uio->uio_resid && orig_resid &&
>   (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) == 0) {
> - sbunlock(>so_rcv);
> + sbunlock(so, >so_rcv);
>   sounlock(s);
>   goto restart;
>   }
> @@ -1004,7 +1004,7 @@ dontblock:
>   if (flagsp)
>   *flagsp |= flags;
>  release:
> - sbunlock(>so_rcv);
> + sbunlock(so, >so_rcv);
>   sounlock(s);
>   return (error);
>  }
> @@ -1049,7 +1049,7 @@ sorflush(struct socket *so)
>   /* with SB_NOINTR and M_WAITOK sblock() must not fail */
>   KASSERT(error == 0);
>   socantrcvmore(so);
> - sbunlock(sb);
> + sbunlock(so, sb);
>   aso.so_proto = pr;
>   aso.so_rcv = *sb;
>   memset(sb, 0, sizeof (*sb));
> @@ -1110,7 +1110,7 @@ sosplice(struct socket *so, int fd, off_
>   }
>   if (so->so_sp->ssp_socket)
>   sounsplice(so, so->so_sp->ssp_socket, 1);
> - sbunlock(>so_rcv);
> + sbunlock(so, >so_rcv);
>   return (0);
>   }
>  
> @@ -1139,7 +1139,7 @@ sosplice(struct socket *so, int fd, off_
>   return (error);
>   }
>   if ((error = sblock(so, >so_snd, M_WAITOK)) != 0) {
> - sbunlock(>so_rcv);
> + sbunlock(so, >so_rcv);
>   FRELE(fp, curproc);
>   return (error);
>   }
> @@ -1183,8 +1183,8 @@ sosplice(struct socket *so, int fd, off_
>   }
>  
>   release:
> - sbunlock(>so_snd);
> - sbunlock(>so_rcv);
> + sbunlock(sosp, >so_snd);
> + sbunlock(so, >so_rcv);
>   FRELE(fp, curproc);
>   return (error);
>  }
> Index: kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.86
> diff -u -p -r1.86 uipc_socket2.c
> --- kern/uipc_socket2.c   11 Aug 2017 21:24:19 -  1.86
> +++ kern/uipc_socket2.c   22 Nov 2017 11:22:37 -
> @@ -342,7 +342,6 @@ sblock(struct socket *so, struct sockbuf
>  {
>   int error, prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
>  
> - KERNEL_ASSERT_LOCKED();
>   soassertlocked(so);
>  
>   if ((sb->sb_flags & SB_LOCK) == 0) {
> @@ -363,9 +362,9 @@ sblock(struct socket *so, struct sockbuf
>  }
>  
>  void
> -sbunlock(struct sockbuf *sb)
> +sbunlock(struct socket *so, struct sockbuf *sb)
>  {
> - KERNEL_ASSERT_LOCKED();
> + soassertlocked(so);
>  
>   sb->sb_flags &= ~SB_LOCK;
>   if (sb->sb_flags & SB_WANT) {

Re: ip_deliver() w/o KERNEL_LOCK

[Alexander Bluhm]

On Wed, Nov 22, 2017 at 12:19:49PM +0100, Martin Pieuchot wrote:
> On 22/11/17(Wed) 11:05, Martin Pieuchot wrote:
> > ip_deliver() dispatches incoming packets to their corresponding protocol
> > input function.  It doesn't need the KERNEL_LOCK(), so remove the assert
> > and mark the dispatch tables as 'const' all over the kernel.
> > 
> > ok?

OK bluhm@

> I missed netinet6/ bits in the previous diff.
> 
> Index: kern/uipc_domain.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_domain.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 uipc_domain.c
> --- kern/uipc_domain.c29 Oct 2017 14:56:36 -  1.54
> +++ kern/uipc_domain.c22 Nov 2017 09:57:06 -
> @@ -76,7 +76,7 @@ void
>  domaininit(void)
>  {
>   struct domain *dp;
> - struct protosw *pr;
> + const struct protosw *pr;
>   static struct timeout pffast_timeout;
>   static struct timeout pfslow_timeout;
>   int i;
> @@ -118,11 +118,11 @@ pffinddomain(int family)
>   return (NULL);
>  }
>  
> -struct protosw *
> +const struct protosw *
>  pffindtype(int family, int type)
>  {
>   struct domain *dp;
> - struct protosw *pr;
> + const struct protosw *pr;
>  
>   dp = pffinddomain(family);
>   if (dp == NULL)
> @@ -134,12 +134,12 @@ pffindtype(int family, int type)
>   return (NULL);
>  }
>  
> -struct protosw *
> +const struct protosw *
>  pffindproto(int family, int protocol, int type)
>  {
>   struct domain *dp;
> - struct protosw *pr;
> - struct protosw *maybe = NULL;
> + const struct protosw *pr;
> + const struct protosw *maybe = NULL;
>  
>   if (family == 0)
>   return (NULL);
> @@ -164,7 +164,7 @@ net_sysctl(int *name, u_int namelen, voi
>  size_t newlen, struct proc *p)
>  {
>   struct domain *dp;
> - struct protosw *pr;
> + const struct protosw *pr;
>   int error, family, protocol;
>  
>   /*
> @@ -218,7 +218,7 @@ void
>  pfctlinput(int cmd, struct sockaddr *sa)
>  {
>   struct domain *dp;
> - struct protosw *pr;
> + const struct protosw *pr;
>   int i;
>  
>   NET_ASSERT_LOCKED();
> @@ -235,7 +235,7 @@ pfslowtimo(void *arg)
>  {
>   struct timeout *to = (struct timeout *)arg;
>   struct domain *dp;
> - struct protosw *pr;
> + const struct protosw *pr;
>   int i;
>  
>   for (i = 0; (dp = domains[i]) != NULL; i++) {
> @@ -251,7 +251,7 @@ pffasttimo(void *arg)
>  {
>   struct timeout *to = (struct timeout *)arg;
>   struct domain *dp;
> - struct protosw *pr;
> + const struct protosw *pr;
>   int i;
>  
>   for (i = 0; (dp = domains[i]) != NULL; i++) {
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.207
> diff -u -p -r1.207 uipc_socket.c
> --- kern/uipc_socket.c4 Nov 2017 14:13:53 -   1.207
> +++ kern/uipc_socket.c22 Nov 2017 09:59:21 -
> @@ -111,7 +111,7 @@ int
>  socreate(int dom, struct socket **aso, int type, int proto)
>  {
>   struct proc *p = curproc;   /* XXX */
> - struct protosw *prp;
> + const struct protosw *prp;
>   struct socket *so;
>   int error, s;
>  
> @@ -633,7 +633,7 @@ soreceive(struct socket *so, struct mbuf
>   struct mbuf *cm;
>   u_long len, offset, moff;
>   int flags, error, s, type, uio_error = 0;
> - struct protosw *pr = so->so_proto;
> + const struct protosw *pr = so->so_proto;
>   struct mbuf *nextrecord;
>   size_t resid, orig_resid = uio->uio_resid;
>  
> @@ -1012,7 +1012,7 @@ release:
>  int
>  soshutdown(struct socket *so, int how)
>  {
> - struct protosw *pr = so->so_proto;
> + const struct protosw *pr = so->so_proto;
>   int s, error = 0;
>  
>   s = solock(so);
> @@ -1040,7 +1040,7 @@ void
>  sorflush(struct socket *so)
>  {
>   struct sockbuf *sb = >so_rcv;
> - struct protosw *pr = so->so_proto;
> + const struct protosw *pr = so->so_proto;
>   struct socket aso;
>   int error;
>  
> Index: netinet/in_proto.c
> ===
> RCS file: /cvs/src/sys/netinet/in_proto.c,v
> retrieving revision 1.87
> diff -u -p -r1.87 in_proto.c
> --- netinet/in_proto.c17 Nov 2017 18:22:52 -  1.87
> +++ netinet/in_proto.c22 Nov 2017 09:53:08 -
> @@ -174,7 +174,7 @@
>  
>  u_char ip_protox[IPPROTO_MAX];
>  
> -struct protosw inetsw[] = {
> +const struct protosw inetsw[] = {
>  {
>.pr_domain = ,
>.pr_init   = ip_init,
> Index: netinet/ip_input.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.333
> diff -u -p -r1.333 ip_input.c
> --- netinet/ip_input.c20 Nov 2017 10:35:24 -  1.333
> +++ netinet/ip_input.c22 Nov 2017 09:55:39 -

Re: Implement __cxa_thread_atexit

[Joerg Sonnenberger]

On Mon, Nov 20, 2017 at 10:07:33PM +0100, Mark Kettenis wrote:
> > Date: Sun, 19 Nov 2017 23:13:05 +0100
> > From: Joerg Sonnenberger 
> > 
> > On Sun, Nov 19, 2017 at 11:05:31PM +0100, Joerg Sonnenberger wrote:
> > > On Sun, Nov 19, 2017 at 10:04:45PM +0100, Mark Kettenis wrote:
> > > > Here is an update diff that implements __cxa_thread_atexit which is
> > > > emitted by clang (and modern gcc) to implement certain aspects of
> > > > C++11 thread_local.
> > > 
> > > Note that without providing __cxa_thread_atexit, gcc will not detect it
> > > and try to provide its own.
> > 
> > __cxa_thread_atexit_impl I mean, sorry.
> 
> GCC 7.2 also checks for __cxa_thread_atexit.
> 
> GCC 6.4 and 4.9 only check for __cxa_thread_atexit_impl.  The
> consequence is that code compiled with ports GCC will indeed use GCC's
> implementation instead of the one in libc.  That means it doesn't
> prevent the unloading of shared libraries.  And with the current diff
> static linking will fail if __cxa_thread_atexit is used.  If ports
> people deem fixing this is important, I can follow what FreeBSD did
> and put the implementation in a separate file and/or provide an alias.

It has also a good chance of breaking static linkage. Providing an alias
is likely the easiest solution. Taking from experience :)

Joerg

Re: avoid pcb lookup in pf forward

[Alexander Bluhm]

On Wed, Nov 22, 2017 at 09:49:06AM +0100, Alexandr Nedvedicky wrote:
> > /* if reassembled packet passed, create new fragments */
> > -   if (pf_status.reass && action == PF_PASS && pd.m && fwdir == PF_FWD) {
> > +   if (pf_status.reass && action == PF_PASS && pd.m && fwdir == PF_FWD &&
> > +   pd.af == AF_INET6) {
> > struct m_tag*mtag;
> >  
> > if ((mtag = m_tag_find(pd.m, PACKET_TAG_PF_REASSEMBLED, NULL)))
> 
> I wonder whether the test 'fwdir == PF_FWD' is correct. I need to think more
> about what happens to reassembled packets in PF_OUT direction. I suggest to
> deal with it in separate thread.

IPv6 routers are not allowed to fragment.  We have end to end path
MTU discovery.  As pf analyzes reassembled packets, we have to
fragment them to the same size after forwarding.  This is only done
for IPv6 to preserve PMTU.

For non-forwarded outgoing packets we use the regular fragment code
in ip6_output().  ip_output() will handle all IPv4 fragments.

bluhm

sbunlock() w/o KERNEL_LOCK()

[Martin Pieuchot]

When we converted most of the splsoftnet() to NET_LOCK() we ended up
with some "lock" inversions between the socket buffer "lock" and the
NET_LOCK().  At that time the KERNEL_LOCK() was serializing access to
`sb_flags'.

We're now moving towards running sockets functions w/o KERNEL_LOCK()
in protocol input paths.  So I'd like to assert that the socket lock
is held when sb_flags are modified.

There's currently one exception to this rule, FIFO kqueue filters,
that I'll address in a later diff.

ok?

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.207
diff -u -p -r1.207 uipc_socket.c
--- kern/uipc_socket.c  4 Nov 2017 14:13:53 -   1.207
+++ kern/uipc_socket.c  22 Nov 2017 11:22:02 -
@@ -453,7 +453,7 @@ restart:
(atomic || space < so->so_snd.sb_lowat))) {
if ((so->so_state & SS_NBIO) || (flags & MSG_DONTWAIT))
snderr(EWOULDBLOCK);
-   sbunlock(>so_snd);
+   sbunlock(so, >so_snd);
error = sbwait(so, >so_snd);
so->so_state &= ~SS_ISSENDING;
if (error)
@@ -497,7 +497,7 @@ restart:
 
 release:
so->so_state &= ~SS_ISSENDING;
-   sbunlock(>so_snd);
+   sbunlock(so, >so_snd);
 out:
sounlock(s);
m_freem(top);
@@ -736,7 +736,7 @@ restart:
}
SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1");
SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1");
-   sbunlock(>so_rcv);
+   sbunlock(so, >so_rcv);
error = sbwait(so, >so_rcv);
sounlock(s);
if (error)
@@ -957,7 +957,7 @@ dontblock:
SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 2");
error = sbwait(so, >so_rcv);
if (error) {
-   sbunlock(>so_rcv);
+   sbunlock(so, >so_rcv);
sounlock(s);
return (0);
}
@@ -993,7 +993,7 @@ dontblock:
}
if (orig_resid == uio->uio_resid && orig_resid &&
(flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) == 0) {
-   sbunlock(>so_rcv);
+   sbunlock(so, >so_rcv);
sounlock(s);
goto restart;
}
@@ -1004,7 +1004,7 @@ dontblock:
if (flagsp)
*flagsp |= flags;
 release:
-   sbunlock(>so_rcv);
+   sbunlock(so, >so_rcv);
sounlock(s);
return (error);
 }
@@ -1049,7 +1049,7 @@ sorflush(struct socket *so)
/* with SB_NOINTR and M_WAITOK sblock() must not fail */
KASSERT(error == 0);
socantrcvmore(so);
-   sbunlock(sb);
+   sbunlock(so, sb);
aso.so_proto = pr;
aso.so_rcv = *sb;
memset(sb, 0, sizeof (*sb));
@@ -1110,7 +1110,7 @@ sosplice(struct socket *so, int fd, off_
}
if (so->so_sp->ssp_socket)
sounsplice(so, so->so_sp->ssp_socket, 1);
-   sbunlock(>so_rcv);
+   sbunlock(so, >so_rcv);
return (0);
}
 
@@ -1139,7 +1139,7 @@ sosplice(struct socket *so, int fd, off_
return (error);
}
if ((error = sblock(so, >so_snd, M_WAITOK)) != 0) {
-   sbunlock(>so_rcv);
+   sbunlock(so, >so_rcv);
FRELE(fp, curproc);
return (error);
}
@@ -1183,8 +1183,8 @@ sosplice(struct socket *so, int fd, off_
}
 
  release:
-   sbunlock(>so_snd);
-   sbunlock(>so_rcv);
+   sbunlock(sosp, >so_snd);
+   sbunlock(so, >so_rcv);
FRELE(fp, curproc);
return (error);
 }
Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.86
diff -u -p -r1.86 uipc_socket2.c
--- kern/uipc_socket2.c 11 Aug 2017 21:24:19 -  1.86
+++ kern/uipc_socket2.c 22 Nov 2017 11:22:37 -
@@ -342,7 +342,6 @@ sblock(struct socket *so, struct sockbuf
 {
int error, prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
 
-   KERNEL_ASSERT_LOCKED();
soassertlocked(so);
 
if ((sb->sb_flags & SB_LOCK) == 0) {
@@ -363,9 +362,9 @@ sblock(struct socket *so, struct sockbuf
 }
 
 void
-sbunlock(struct sockbuf *sb)
+sbunlock(struct socket *so, struct sockbuf *sb)
 {
-   KERNEL_ASSERT_LOCKED();
+   soassertlocked(so);
 
sb->sb_flags &= ~SB_LOCK;
if (sb->sb_flags & SB_WANT) {
Index: sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.77
diff -u -p -r1.77 socketvar.h
--- sys/socketvar.h 4 Nov 

Re: ip_deliver() w/o KERNEL_LOCK

[Martin Pieuchot]

On 22/11/17(Wed) 11:05, Martin Pieuchot wrote:
> ip_deliver() dispatches incoming packets to their corresponding protocol
> input function.  It doesn't need the KERNEL_LOCK(), so remove the assert
> and mark the dispatch tables as 'const' all over the kernel.
> 
> ok?

I missed netinet6/ bits in the previous diff.

Index: kern/uipc_domain.c
===
RCS file: /cvs/src/sys/kern/uipc_domain.c,v
retrieving revision 1.54
diff -u -p -r1.54 uipc_domain.c
--- kern/uipc_domain.c  29 Oct 2017 14:56:36 -  1.54
+++ kern/uipc_domain.c  22 Nov 2017 09:57:06 -
@@ -76,7 +76,7 @@ void
 domaininit(void)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
static struct timeout pffast_timeout;
static struct timeout pfslow_timeout;
int i;
@@ -118,11 +118,11 @@ pffinddomain(int family)
return (NULL);
 }
 
-struct protosw *
+const struct protosw *
 pffindtype(int family, int type)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
 
dp = pffinddomain(family);
if (dp == NULL)
@@ -134,12 +134,12 @@ pffindtype(int family, int type)
return (NULL);
 }
 
-struct protosw *
+const struct protosw *
 pffindproto(int family, int protocol, int type)
 {
struct domain *dp;
-   struct protosw *pr;
-   struct protosw *maybe = NULL;
+   const struct protosw *pr;
+   const struct protosw *maybe = NULL;
 
if (family == 0)
return (NULL);
@@ -164,7 +164,7 @@ net_sysctl(int *name, u_int namelen, voi
 size_t newlen, struct proc *p)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int error, family, protocol;
 
/*
@@ -218,7 +218,7 @@ void
 pfctlinput(int cmd, struct sockaddr *sa)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
 
NET_ASSERT_LOCKED();
@@ -235,7 +235,7 @@ pfslowtimo(void *arg)
 {
struct timeout *to = (struct timeout *)arg;
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
 
for (i = 0; (dp = domains[i]) != NULL; i++) {
@@ -251,7 +251,7 @@ pffasttimo(void *arg)
 {
struct timeout *to = (struct timeout *)arg;
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
 
for (i = 0; (dp = domains[i]) != NULL; i++) {
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.207
diff -u -p -r1.207 uipc_socket.c
--- kern/uipc_socket.c  4 Nov 2017 14:13:53 -   1.207
+++ kern/uipc_socket.c  22 Nov 2017 09:59:21 -
@@ -111,7 +111,7 @@ int
 socreate(int dom, struct socket **aso, int type, int proto)
 {
struct proc *p = curproc;   /* XXX */
-   struct protosw *prp;
+   const struct protosw *prp;
struct socket *so;
int error, s;
 
@@ -633,7 +633,7 @@ soreceive(struct socket *so, struct mbuf
struct mbuf *cm;
u_long len, offset, moff;
int flags, error, s, type, uio_error = 0;
-   struct protosw *pr = so->so_proto;
+   const struct protosw *pr = so->so_proto;
struct mbuf *nextrecord;
size_t resid, orig_resid = uio->uio_resid;
 
@@ -1012,7 +1012,7 @@ release:
 int
 soshutdown(struct socket *so, int how)
 {
-   struct protosw *pr = so->so_proto;
+   const struct protosw *pr = so->so_proto;
int s, error = 0;
 
s = solock(so);
@@ -1040,7 +1040,7 @@ void
 sorflush(struct socket *so)
 {
struct sockbuf *sb = >so_rcv;
-   struct protosw *pr = so->so_proto;
+   const struct protosw *pr = so->so_proto;
struct socket aso;
int error;
 
Index: netinet/in_proto.c
===
RCS file: /cvs/src/sys/netinet/in_proto.c,v
retrieving revision 1.87
diff -u -p -r1.87 in_proto.c
--- netinet/in_proto.c  17 Nov 2017 18:22:52 -  1.87
+++ netinet/in_proto.c  22 Nov 2017 09:53:08 -
@@ -174,7 +174,7 @@
 
 u_char ip_protox[IPPROTO_MAX];
 
-struct protosw inetsw[] = {
+const struct protosw inetsw[] = {
 {
   .pr_domain   = ,
   .pr_init = ip_init,
Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.333
diff -u -p -r1.333 ip_input.c
--- netinet/ip_input.c  20 Nov 2017 10:35:24 -  1.333
+++ netinet/ip_input.c  22 Nov 2017 09:55:39 -
@@ -151,7 +151,7 @@ void save_rte(struct mbuf *, u_char *, s
 void
 ip_init(void)
 {
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
const u_int16_t defbaddynamicports_tcp[] = DEFBADDYNAMICPORTS_TCP;
const u_int16_t defbaddynamicports_udp[] = 

ip_deliver() w/o KERNEL_LOCK

[Martin Pieuchot]

ip_deliver() dispatches incoming packets to their corresponding protocol
input function.  It doesn't need the KERNEL_LOCK(), so remove the assert
and mark the dispatch tables as 'const' all over the kernel.

ok?

Index: kern/uipc_domain.c
===
RCS file: /cvs/src/sys/kern/uipc_domain.c,v
retrieving revision 1.54
diff -u -p -r1.54 uipc_domain.c
--- kern/uipc_domain.c  29 Oct 2017 14:56:36 -  1.54
+++ kern/uipc_domain.c  22 Nov 2017 09:57:06 -
@@ -76,7 +76,7 @@ void
 domaininit(void)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
static struct timeout pffast_timeout;
static struct timeout pfslow_timeout;
int i;
@@ -118,11 +118,11 @@ pffinddomain(int family)
return (NULL);
 }
 
-struct protosw *
+const struct protosw *
 pffindtype(int family, int type)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
 
dp = pffinddomain(family);
if (dp == NULL)
@@ -134,12 +134,12 @@ pffindtype(int family, int type)
return (NULL);
 }
 
-struct protosw *
+const struct protosw *
 pffindproto(int family, int protocol, int type)
 {
struct domain *dp;
-   struct protosw *pr;
-   struct protosw *maybe = NULL;
+   const struct protosw *pr;
+   const struct protosw *maybe = NULL;
 
if (family == 0)
return (NULL);
@@ -164,7 +164,7 @@ net_sysctl(int *name, u_int namelen, voi
 size_t newlen, struct proc *p)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int error, family, protocol;
 
/*
@@ -218,7 +218,7 @@ void
 pfctlinput(int cmd, struct sockaddr *sa)
 {
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
 
NET_ASSERT_LOCKED();
@@ -235,7 +235,7 @@ pfslowtimo(void *arg)
 {
struct timeout *to = (struct timeout *)arg;
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
 
for (i = 0; (dp = domains[i]) != NULL; i++) {
@@ -251,7 +251,7 @@ pffasttimo(void *arg)
 {
struct timeout *to = (struct timeout *)arg;
struct domain *dp;
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
 
for (i = 0; (dp = domains[i]) != NULL; i++) {
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.207
diff -u -p -r1.207 uipc_socket.c
--- kern/uipc_socket.c  4 Nov 2017 14:13:53 -   1.207
+++ kern/uipc_socket.c  22 Nov 2017 09:59:21 -
@@ -111,7 +111,7 @@ int
 socreate(int dom, struct socket **aso, int type, int proto)
 {
struct proc *p = curproc;   /* XXX */
-   struct protosw *prp;
+   const struct protosw *prp;
struct socket *so;
int error, s;
 
@@ -633,7 +633,7 @@ soreceive(struct socket *so, struct mbuf
struct mbuf *cm;
u_long len, offset, moff;
int flags, error, s, type, uio_error = 0;
-   struct protosw *pr = so->so_proto;
+   const struct protosw *pr = so->so_proto;
struct mbuf *nextrecord;
size_t resid, orig_resid = uio->uio_resid;
 
@@ -1012,7 +1012,7 @@ release:
 int
 soshutdown(struct socket *so, int how)
 {
-   struct protosw *pr = so->so_proto;
+   const struct protosw *pr = so->so_proto;
int s, error = 0;
 
s = solock(so);
@@ -1040,7 +1040,7 @@ void
 sorflush(struct socket *so)
 {
struct sockbuf *sb = >so_rcv;
-   struct protosw *pr = so->so_proto;
+   const struct protosw *pr = so->so_proto;
struct socket aso;
int error;
 
Index: netinet/in_proto.c
===
RCS file: /cvs/src/sys/netinet/in_proto.c,v
retrieving revision 1.87
diff -u -p -r1.87 in_proto.c
--- netinet/in_proto.c  17 Nov 2017 18:22:52 -  1.87
+++ netinet/in_proto.c  22 Nov 2017 09:53:08 -
@@ -174,7 +174,7 @@
 
 u_char ip_protox[IPPROTO_MAX];
 
-struct protosw inetsw[] = {
+const struct protosw inetsw[] = {
 {
   .pr_domain   = ,
   .pr_init = ip_init,
Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.333
diff -u -p -r1.333 ip_input.c
--- netinet/ip_input.c  20 Nov 2017 10:35:24 -  1.333
+++ netinet/ip_input.c  22 Nov 2017 09:55:39 -
@@ -151,7 +151,7 @@ void save_rte(struct mbuf *, u_char *, s
 void
 ip_init(void)
 {
-   struct protosw *pr;
+   const struct protosw *pr;
int i;
const u_int16_t defbaddynamicports_tcp[] = DEFBADDYNAMICPORTS_TCP;
const u_int16_t defbaddynamicports_udp[] = DEFBADDYNAMICPORTS_UDP;
@@ -613,13 +613,11 @@ ip_local(struct mbuf **mp, int *offp, in
 int
 ip_deliver(struct mbuf **mp, 

Re: avoid pcb lookup in pf forward

[Alexandr Nedvedicky]

Hello,

your change looks good to me as-is.

Though the patch itself drags my attention to line here in pf_test():

> @@ -7072,7 +7083,8 @@ done:
>  
>  #ifdef INET6
>   /* if reassembled packet passed, create new fragments */
> - if (pf_status.reass && action == PF_PASS && pd.m && fwdir == PF_FWD) {
> + if (pf_status.reass && action == PF_PASS && pd.m && fwdir == PF_FWD &&
> + pd.af == AF_INET6) {
>   struct m_tag*mtag;
>  
>   if ((mtag = m_tag_find(pd.m, PACKET_TAG_PF_REASSEMBLED, NULL)))

I wonder whether the test 'fwdir == PF_FWD' is correct. I need to think more
about what happens to reassembled packets in PF_OUT direction. I suggest to
deal with it in separate thread.

I'm OK with your patch as-is.

thanks and
regards
sasha