Re: vmd: reset queue_size if queue_select is invalid

[Mike Larkin]

On Wed, Jul 26, 2017 at 09:37:30PM -0700, Nick Owens wrote:
> hello tech@,
> 
> here is a diff that will follow the virtio spec a little closer, and
> allows 9front's (http://9front.org) virtio-blk driver to correctly find
> the number of queues. i know that virtio-blk only has one queue, but
> the virtio probing code is shared between virtio-blk and virtio-scsi.
> 
> without this change, the size of the first queue is used for all
> subsequently probed queues.
> 
> for completeness i've changed rng and net to do the same as blk.
> 
> some bits from the spec:
> 
> 4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
> corresponding to the current queue_select is unavailable."
> 
> 4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
> queue_select. Read the virtqueue size from queue_size. This controls
> how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
> virtqueue does not exist."
> 

Thanks. committed.

Sorry it took so long.

-ml

PS, the diff got mangled below, but I recreated it by hand.



> Index: virtio.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> retrieving revision 1.49
> diff -u -p -u -p -r1.49 virtio.c
> --- virtio.c  30 May 2017 17:56:47 -  1.49
> +++ virtio.c  27 Jul 2017 04:35:46 -
> @@ -150,8 +150,10 @@ void
>  viornd_update_qs(void)
>  {
>   /* Invalid queue? */
> - if (viornd.cfg.queue_select > 0)
> + if (viornd.cfg.queue_select > 0) {
> + viornd.cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   viornd.cfg.queue_address =
> viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
>  vioblk_update_qs(struct vioblk_dev *dev)
>  {
>   /* Invalid queue? */
> - if (dev->cfg.queue_select > 0)
> + if (dev->cfg.queue_select > 0) {
> + dev->cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> @@ -1037,8 +1041,10 @@ void
>  vionet_update_qs(struct vionet_dev *dev)
>  {
>   /* Invalid queue? */
> - if (dev->cfg.queue_select > 1)
> + if (dev->cfg.queue_select > 1) {
> + dev->cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> 

Update inaccurate comment in rasops(9)

[Frederic Cambus]

Hi tech@,

Update inaccurate comment: rasops_copycols() doesn't use bcopy()
anymore, but either memmove() or slow_bcopy().

Comments? OK?

Index: sys/dev/rasops/rasops.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.45
diff -u -p -r1.45 rasops.c
--- sys/dev/rasops/rasops.c 16 May 2017 02:22:51 -  1.45
+++ sys/dev/rasops/rasops.c 1 Aug 2017 21:32:14 -
@@ -660,8 +660,8 @@ rasops_copyrows(void *cookie, int src, i
 /*
  * Copy columns. This is slow, and hard to optimize due to alignment,
  * and the fact that we have to copy both left->right and right->left.
- * We simply cop-out here and use bcopy(), since it handles all of
- * these cases anyway.
+ * We simply cop-out here and use either memmove() or slow_bcopy(),
+ * since they handle all of these cases anyway.
  */
 int
 rasops_copycols(void *cookie, int row, int src, int dst, int num)

Re: so{s,g}etopt() & solock

[Alexander Bluhm]

On Wed, Aug 02, 2017 at 11:03:13AM +0200, Martin Pieuchot wrote:
> Diff below moves the socket lock "above" sosetopt(), sogetopt() and
> sosplice().  While this adds a lot of lock/unlock dances in NFS, they
> will be merge in a later diff.
> 
> sosetopt() modifies a socket fields so it needs the lock.  sogetopt()
> do not always need it, but it makes the code simpler to always grab
> it and the code section is really small anyway.
> 
> ok?

OK bluhm@

> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.198
> diff -u -p -r1.198 uipc_socket.c
> --- kern/uipc_socket.c27 Jul 2017 12:05:36 -  1.198
> +++ kern/uipc_socket.c2 Aug 2017 08:21:45 -
> @@ -1073,7 +1073,9 @@ sosplice(struct socket *so, int fd, off_
>   struct file *fp;
>   struct socket   *sosp;
>   struct sosplice *sp;
> - int  s, error = 0;
> + int  error = 0;
> +
> + soassertlocked(so);
>  
>   if (sosplice_taskq == NULL)
>   sosplice_taskq = taskq_create("sosplice", 1, IPL_SOFTNET, 0);
> @@ -1097,17 +1099,14 @@ sosplice(struct socket *so, int fd, off_
>  
>   /* If no fd is given, unsplice by removing existing link. */
>   if (fd < 0) {
> - s = solock(so);
>   /* Lock receive buffer. */
>   if ((error = sblock(so, >so_rcv,
>   (so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) {
> - sounlock(s);
>   return (error);
>   }
>   if (so->so_sp->ssp_socket)
>   sounsplice(so, so->so_sp->ssp_socket, 1);
>   sbunlock(>so_rcv);
> - sounlock(s);
>   return (0);
>   }
>  
> @@ -1129,17 +1128,14 @@ sosplice(struct socket *so, int fd, off_
>   pool_put(_pool, sp);
>   }
>  
> - s = solock(so);
>   /* Lock both receive and send buffer. */
>   if ((error = sblock(so, >so_rcv,
>   (so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) {
> - sounlock(s);
>   FRELE(fp, curproc);
>   return (error);
>   }
>   if ((error = sblock(so, >so_snd, M_WAITOK)) != 0) {
>   sbunlock(>so_rcv);
> - sounlock(s);
>   FRELE(fp, curproc);
>   return (error);
>   }
> @@ -1185,7 +1181,6 @@ sosplice(struct socket *so, int fd, off_
>   release:
>   sbunlock(>so_snd);
>   sbunlock(>so_rcv);
> - sounlock(s);
>   FRELE(fp, curproc);
>   return (error);
>  }
> @@ -1565,15 +1560,15 @@ sowwakeup(struct socket *so)
>  int
>  sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
>  {
> - int s, error = 0;
> + int error = 0;
>   struct mbuf *m = m0;
>  
> + soassertlocked(so);
> +
>   if (level != SOL_SOCKET) {
>   if (so->so_proto && so->so_proto->pr_ctloutput) {
> - s = solock(so);
>   error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
>   level, optname, m0);
> - sounlock(s);
>   return (error);
>   }
>   error = ENOPROTOOPT;
> @@ -1647,14 +1642,11 @@ sosetopt(struct socket *so, int level, i
>   error = EINVAL;
>   goto bad;
>   }
> - s = solock(so);
>   if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
>   sbreserve(so, >so_snd, cnt)) {
> - sounlock(s);
>   error = ENOBUFS;
>   goto bad;
>   }
> - sounlock(s);
>   so->so_snd.sb_wat = cnt;
>   break;
>  
> @@ -1663,14 +1655,11 @@ sosetopt(struct socket *so, int level, i
>   error = EINVAL;
>   goto bad;
>   }
> - s = solock(so);
>   if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
>   sbreserve(so, >so_rcv, cnt)) {
> - sounlock(s);
>   error = ENOBUFS;
>   goto bad;
>   }
> - sounlock(s);
>   so->so_rcv.sb_wat = cnt;
>   break;
>  
> @@ -1724,10 +1713,8 @@ sosetopt(struct socket *so, int level, i
>   struct domain *dom = so->so_proto->pr_domain;
>  
>   level = 

Re: inform userland on ifconfig $if -inet6

[Alexander Bluhm]

On Fri, Aug 04, 2017 at 07:51:22PM +, Florian Obser wrote:
> Yes, I was thinking about that, too. it's a bit more complex. I think
> it's worth it, especially if we ever have a autoconf4 flag.  The
> trouble with the SIOCIFAFDETACH that have to track if a flag changes.
> this applies the heavy hammer of just checking if if_flags or
> if_xflags changed.
> Am I overengineering this?

I think this approach fine.
Please remove the error = 0; line, here it is 0 already.
OK bluhm@

> diff --git if.c if.c
> index ed95b15b8c9..3d9af2acf6e 100644
> --- if.c
> +++ if.c
> @@ -1828,7 +1828,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, 
> struct proc *p)
>   struct if_afreq *ifar;
>   char ifdescrbuf[IFDESCRSIZE];
>   char ifrtlabelbuf[RTLABEL_LEN];
> - int s, error = 0;
> + int s, error = 0, oif_xflags;
>   size_t bytesdone;
>   short oif_flags;
>   const char *label;
> @@ -1865,23 +1865,29 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, 
> struct proc *p)
>   ifar = (struct if_afreq *)data;
>   if ((ifp = ifunit(ifar->ifar_name)) == NULL)
>   return (ENXIO);
> + oif_flags = ifp->if_flags;
> + oif_xflags = ifp->if_xflags;
> + error = 0;
>   switch (ifar->ifar_af) {
>   case AF_INET:
>   /* attach is a noop for AF_INET */
>   if (cmd == SIOCIFAFDETACH)
>   in_ifdetach(ifp);
> - return (0);
> + break;
>  #ifdef INET6
>   case AF_INET6:
>   if (cmd == SIOCIFAFATTACH)
>   error = in6_ifattach(ifp);
>   else
>   in6_ifdetach(ifp);
> - return (error);
> + break;
>  #endif /* INET6 */
>   default:
>   return (EAFNOSUPPORT);
>   }
> + if (oif_flags != ifp->if_flags || oif_xflags != ifp->if_xflags)
> + rtm_ifchg(ifp);
> + return (error);
>   }
>  
>   ifp = ifunit(ifr->ifr_name);
> 
> 
> -- 
> I'm not entirely sure you are real.

Re: nd6 address expiration & NET_LOCK() contention

[Alexander Bluhm]

On Wed, Aug 02, 2017 at 11:24:10AM +0200, Martin Pieuchot wrote:
> Florian killed the default router and prefix lists from the kernel.  So
> nd6_timer() is now only doing address expiration based on pltime/vltime.
> 
> Diff below renames the function and kill outdated comments to reflect
> the reality.
> 
> Now, this timer wakes a task every second.  And even if there's nothing
> to do the task tries to grab the NET_LOCK().  Being scheduled on `systq'
> this task creates contention with the `softnettq' and any userland
> program trying to grab the lock.  So this diff also moves this task to
> the `softnettq'.
> 
> ok?

OK bluhm@

> Index: netinet6/nd6.c
> ===
> RCS file: /cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.211
> diff -u -p -r1.211 nd6.c
> --- netinet6/nd6.c12 Jul 2017 16:53:58 -  1.211
> +++ netinet6/nd6.c2 Aug 2017 09:18:13 -
> @@ -90,15 +90,15 @@ int   nd6_inuse, nd6_allocated;
>  int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
>  
>  void nd6_slowtimo(void *);
> -void nd6_timer_work(void *);
> -void nd6_timer(void *);
> +void nd6_expire(void *);
> +void nd6_expire_timer(void *);
>  void nd6_invalidate(struct rtentry *);
>  struct llinfo_nd6 *nd6_free(struct rtentry *, int);
>  void nd6_llinfo_timer(void *);
>  
>  struct timeout nd6_slowtimo_ch;
> -struct timeout nd6_timer_ch;
> -struct task nd6_timer_task;
> +struct timeout nd6_expire_timeout;
> +struct task nd6_expire_task;
>  
>  void
>  nd6_init(void)
> @@ -114,17 +114,15 @@ nd6_init(void)
>   pool_init(_pool, sizeof(struct llinfo_nd6), 0,
>   IPL_SOFTNET, 0, "nd6", NULL);
>  
> - /* initialization of the default router list */
> -
> - task_set(_timer_task, nd6_timer_work, NULL);
> + task_set(_expire_task, nd6_expire, NULL);
>  
>   nd6_init_done = 1;
>  
>   /* start timer */
>   timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL);
>   timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
> - timeout_set(_timer_ch, nd6_timer, NULL);
> - timeout_add_sec(_timer_ch, nd6_prune);
> + timeout_set(_expire_timeout, nd6_expire_timer, NULL);
> + timeout_add_sec(_expire_timeout, nd6_prune);
>  
>  }
>  
> @@ -420,24 +418,19 @@ nd6_llinfo_timer(void *arg)
>  }
>  
>  /*
> - * ND6 timer routine to expire default route list and prefix list
> + * Expire interface addresses.
>   */
>  void
> -nd6_timer_work(void *null)
> +nd6_expire(void *unused)
>  {
>   struct ifnet *ifp;
>   int s;
>  
> + KERNEL_LOCK();
>   NET_LOCK(s);
>  
> - timeout_add_sec(_timer_ch, nd6_prune);
> + timeout_add_sec(_expire_timeout, nd6_prune);
>  
> - /*
> -  * expire interface addresses.
> -  * in the past the loop was inside prefix expiry processing.
> -  * However, from a stricter spec-conformance standpoint, we should
> -  * rather separate address lifetimes and prefix lifetimes.
> -  */
>   TAILQ_FOREACH(ifp, , if_list) {
>   struct ifaddr *ifa, *nifa;
>   struct in6_ifaddr *ia6;
> @@ -462,12 +455,13 @@ nd6_timer_work(void *null)
>   }
>  
>   NET_UNLOCK(s);
> + KERNEL_UNLOCK();
>  }
>  
>  void
> -nd6_timer(void *ignored_arg)
> +nd6_expire_timer(void *unused)
>  {
> - task_add(systq, _timer_task);
> + task_add(softnettq, _expire_task);
>  }
>  
>  /*

Re: cleanup nd6_free next pointer

[Florian Obser]

OK florian@

On Fri, Aug 04, 2017 at 09:21:31PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> Since we have TAILQ_FOREACH_SAFE in nd6_purge() it is not necessary
> that nd6_free() advances the next pointer itself.  Make nd6_free()
> a void function and remove unused parameter gc.
> 
> ok?
> 
> bluhm
> 
> Index: netinet6/nd6.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.211
> diff -u -p -r1.211 nd6.c
> --- netinet6/nd6.c12 Jul 2017 16:53:58 -  1.211
> +++ netinet6/nd6.c4 Aug 2017 19:10:45 -
> @@ -93,7 +93,7 @@ void nd6_slowtimo(void *);
>  void nd6_timer_work(void *);
>  void nd6_timer(void *);
>  void nd6_invalidate(struct rtentry *);
> -struct llinfo_nd6 *nd6_free(struct rtentry *, int);
> +void nd6_free(struct rtentry *);
>  void nd6_llinfo_timer(void *);
>  
>  struct timeout nd6_slowtimo_ch;
> @@ -369,7 +369,7 @@ nd6_llinfo_timer(void *arg)
>   ln->ln_hold = NULL;
>   }
>   }
> - (void)nd6_free(rt, 0);
> + nd6_free(rt);
>   ln = NULL;
>   }
>   break;
> @@ -384,7 +384,7 @@ nd6_llinfo_timer(void *arg)
>   case ND6_LLINFO_PURGE:
>   /* Garbage Collection(RFC 2461 5.3) */
>   if (!ND6_LLINFO_PERMANENT(ln)) {
> - (void)nd6_free(rt, 1);
> + nd6_free(rt);
>   ln = NULL;
>   }
>   break;
> @@ -409,7 +409,7 @@ nd6_llinfo_timer(void *arg)
>   nd6_ns_output(ifp, >sin6_addr,
>   >sin6_addr, ln, 0);
>   } else {
> - (void)nd6_free(rt, 0);
> + nd6_free(rt);
>   ln = NULL;
>   }
>   break;
> @@ -493,7 +493,7 @@ nd6_purge(struct ifnet *ifp)
>   rt->rt_gateway->sa_family == AF_LINK) {
>   sdl = satosdl(rt->rt_gateway);
>   if (sdl->sdl_index == ifp->if_index)
> - nln = nd6_free(rt, 0);
> + nd6_free(rt);
>   }
>   }
>  }
> @@ -655,10 +655,10 @@ nd6_invalidate(struct rtentry *rt)
>   * make it global, unless you have a strong reason for the change, and are 
> sure
>   * that the change is safe.
>   */
> -struct llinfo_nd6 *
> -nd6_free(struct rtentry *rt, int gc)
> +void
> +nd6_free(struct rtentry *rt)
>  {
> - struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo, *next;
> + struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo;
>   struct in6_addr in6 = satosin6(rt_key(rt))->sin6_addr;
>   struct ifnet *ifp;
>  
> @@ -677,12 +677,6 @@ nd6_free(struct rtentry *rt, int gc)
>   }
>   }
>  
> - /*
> -  * Before deleting the entry, remember the next entry as the
> -  * return value.
> -  */
> - next = TAILQ_NEXT(ln, ln_list);
> -
>   nd6_invalidate(rt);
>  
>   /*
> @@ -694,8 +688,6 @@ nd6_free(struct rtentry *rt, int gc)
>   rtdeletemsg(rt, ifp, ifp->if_rdomain);
>  
>   if_put(ifp);
> -
> - return (next);
>  }
>  
>  /*
> @@ -,7 +1103,7 @@ nd6_cache_lladdr(struct ifnet *ifp, stru
>   return;
>   if ((rt->rt_flags & (RTF_GATEWAY | RTF_LLINFO)) != RTF_LLINFO) {
>  fail:
> - (void)nd6_free(rt, 0);
> + nd6_free(rt);
>   rtfree(rt);
>   return;
>   }
> 

-- 
I'm not entirely sure you are real.

Re: inform userland on ifconfig $if -inet6

[Florian Obser]

On Fri, Aug 04, 2017 at 06:04:26PM +0200, Alexander Bluhm wrote:
> On Fri, Aug 04, 2017 at 03:38:40PM +, Florian Obser wrote:
> > When we disable INET6 on an interface that also removes the autoconf6
> > flag.  Notify userland about this via the route socket, otherwise
> > slaacd gets confused about the state the interface is in.
> 
> The other rtm_ifchg() calls are in net/if.c.  Should this one
> not also be there in the SIOCIFAFDETACH case?
> 
> bluhm

Yes, I was thinking about that, too. it's a bit more complex. I think
it's worth it, especially if we ever have a autoconf4 flag.  The
trouble with the SIOCIFAFDETACH that have to track if a flag changes.
this applies the heavy hammer of just checking if if_flags or
if_xflags changed.
Am I overengineering this?

diff --git if.c if.c
index ed95b15b8c9..3d9af2acf6e 100644
--- if.c
+++ if.c
@@ -1828,7 +1828,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, 
struct proc *p)
struct if_afreq *ifar;
char ifdescrbuf[IFDESCRSIZE];
char ifrtlabelbuf[RTLABEL_LEN];
-   int s, error = 0;
+   int s, error = 0, oif_xflags;
size_t bytesdone;
short oif_flags;
const char *label;
@@ -1865,23 +1865,29 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, 
struct proc *p)
ifar = (struct if_afreq *)data;
if ((ifp = ifunit(ifar->ifar_name)) == NULL)
return (ENXIO);
+   oif_flags = ifp->if_flags;
+   oif_xflags = ifp->if_xflags;
+   error = 0;
switch (ifar->ifar_af) {
case AF_INET:
/* attach is a noop for AF_INET */
if (cmd == SIOCIFAFDETACH)
in_ifdetach(ifp);
-   return (0);
+   break;
 #ifdef INET6
case AF_INET6:
if (cmd == SIOCIFAFATTACH)
error = in6_ifattach(ifp);
else
in6_ifdetach(ifp);
-   return (error);
+   break;
 #endif /* INET6 */
default:
return (EAFNOSUPPORT);
}
+   if (oif_flags != ifp->if_flags || oif_xflags != ifp->if_xflags)
+   rtm_ifchg(ifp);
+   return (error);
}
 
ifp = ifunit(ifr->ifr_name);


-- 
I'm not entirely sure you are real.

Re: Fix clang's lgamma(3) builtin

[Patrick Wildt]

On Fri, Aug 04, 2017 at 06:04:59PM +0200, Mark Kettenis wrote:
> So the regress failure is a clang bug.  Already fixed upstream:
> 
>   https://reviews.llvm.org/D29778
> 
> ok?

Sure.

> 
> 
> Index: gnu/llvm/tools/clang/include/clang/Basic/Builtins.def
> ===
> RCS file: /cvs/src/gnu/llvm/tools/clang/include/clang/Basic/Builtins.def,v
> retrieving revision 1.1.1.4
> diff -u -p -r1.1.1.4 Builtins.def
> --- gnu/llvm/tools/clang/include/clang/Basic/Builtins.def 14 Mar 2017 
> 08:07:54 -  1.1.1.4
> +++ gnu/llvm/tools/clang/include/clang/Basic/Builtins.def 4 Aug 2017 
> 14:24:47 -
> @@ -1086,9 +1086,11 @@ LIBBUILTIN(ilogb, "id", "fne", "math.h",
>  LIBBUILTIN(ilogbf, "if", "fne", "math.h", ALL_LANGUAGES)
>  LIBBUILTIN(ilogbl, "iLd", "fne", "math.h", ALL_LANGUAGES)
>  
> -LIBBUILTIN(lgamma, "dd", "fne", "math.h", ALL_LANGUAGES)
> -LIBBUILTIN(lgammaf, "ff", "fne", "math.h", ALL_LANGUAGES)
> -LIBBUILTIN(lgammal, "LdLd", "fne", "math.h", ALL_LANGUAGES)
> +// POSIX math.h declares a global, signgam, that lgamma writes to, so these
> +// shouldn't have "e" or "c" attributes
> +LIBBUILTIN(lgamma, "dd", "fn", "math.h", ALL_LANGUAGES)
> +LIBBUILTIN(lgammaf, "ff", "fn", "math.h", ALL_LANGUAGES)
> +LIBBUILTIN(lgammal, "LdLd", "fn", "math.h", ALL_LANGUAGES)
>  
>  LIBBUILTIN(llrint, "LLid", "fne", "math.h", ALL_LANGUAGES)
>  LIBBUILTIN(llrintf, "LLif", "fne", "math.h", ALL_LANGUAGES)
> 

cleanup nd6_free next pointer

[Alexander Bluhm]

Hi,

Since we have TAILQ_FOREACH_SAFE in nd6_purge() it is not necessary
that nd6_free() advances the next pointer itself.  Make nd6_free()
a void function and remove unused parameter gc.

ok?

bluhm

Index: netinet6/nd6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.211
diff -u -p -r1.211 nd6.c
--- netinet6/nd6.c  12 Jul 2017 16:53:58 -  1.211
+++ netinet6/nd6.c  4 Aug 2017 19:10:45 -
@@ -93,7 +93,7 @@ void nd6_slowtimo(void *);
 void nd6_timer_work(void *);
 void nd6_timer(void *);
 void nd6_invalidate(struct rtentry *);
-struct llinfo_nd6 *nd6_free(struct rtentry *, int);
+void nd6_free(struct rtentry *);
 void nd6_llinfo_timer(void *);
 
 struct timeout nd6_slowtimo_ch;
@@ -369,7 +369,7 @@ nd6_llinfo_timer(void *arg)
ln->ln_hold = NULL;
}
}
-   (void)nd6_free(rt, 0);
+   nd6_free(rt);
ln = NULL;
}
break;
@@ -384,7 +384,7 @@ nd6_llinfo_timer(void *arg)
case ND6_LLINFO_PURGE:
/* Garbage Collection(RFC 2461 5.3) */
if (!ND6_LLINFO_PERMANENT(ln)) {
-   (void)nd6_free(rt, 1);
+   nd6_free(rt);
ln = NULL;
}
break;
@@ -409,7 +409,7 @@ nd6_llinfo_timer(void *arg)
nd6_ns_output(ifp, >sin6_addr,
>sin6_addr, ln, 0);
} else {
-   (void)nd6_free(rt, 0);
+   nd6_free(rt);
ln = NULL;
}
break;
@@ -493,7 +493,7 @@ nd6_purge(struct ifnet *ifp)
rt->rt_gateway->sa_family == AF_LINK) {
sdl = satosdl(rt->rt_gateway);
if (sdl->sdl_index == ifp->if_index)
-   nln = nd6_free(rt, 0);
+   nd6_free(rt);
}
}
 }
@@ -655,10 +655,10 @@ nd6_invalidate(struct rtentry *rt)
  * make it global, unless you have a strong reason for the change, and are sure
  * that the change is safe.
  */
-struct llinfo_nd6 *
-nd6_free(struct rtentry *rt, int gc)
+void
+nd6_free(struct rtentry *rt)
 {
-   struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo, *next;
+   struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo;
struct in6_addr in6 = satosin6(rt_key(rt))->sin6_addr;
struct ifnet *ifp;
 
@@ -677,12 +677,6 @@ nd6_free(struct rtentry *rt, int gc)
}
}
 
-   /*
-* Before deleting the entry, remember the next entry as the
-* return value.
-*/
-   next = TAILQ_NEXT(ln, ln_list);
-
nd6_invalidate(rt);
 
/*
@@ -694,8 +688,6 @@ nd6_free(struct rtentry *rt, int gc)
rtdeletemsg(rt, ifp, ifp->if_rdomain);
 
if_put(ifp);
-
-   return (next);
 }
 
 /*
@@ -,7 +1103,7 @@ nd6_cache_lladdr(struct ifnet *ifp, stru
return;
if ((rt->rt_flags & (RTF_GATEWAY | RTF_LLINFO)) != RTF_LLINFO) {
 fail:
-   (void)nd6_free(rt, 0);
+   nd6_free(rt);
rtfree(rt);
return;
}

Re: vmd: reset queue_size if queue_select is invalid

[Mike Larkin]

On Thu, Aug 03, 2017 at 04:18:54PM -0700, Nick Owens wrote:
> ping?
> 

Sorry this got buried. Should have time later today (yes, really) :)

-ml

> On Thu, Jul 27, 2017 at 10:20 AM, Mike Larkin  wrote:
> > On Wed, Jul 26, 2017 at 09:37:30PM -0700, Nick Owens wrote:
> >> hello tech@,
> >>
> >> here is a diff that will follow the virtio spec a little closer, and
> >> allows 9front's (http://9front.org) virtio-blk driver to correctly find
> >> the number of queues. i know that virtio-blk only has one queue, but
> >> the virtio probing code is shared between virtio-blk and virtio-scsi.
> >>
> >> without this change, the size of the first queue is used for all
> >> subsequently probed queues.
> >>
> >> for completeness i've changed rng and net to do the same as blk.
> >>
> >> some bits from the spec:
> >>
> >> 4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
> >> corresponding to the current queue_select is unavailable."
> >>
> >> 4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
> >> queue_select. Read the virtqueue size from queue_size. This controls
> >> how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
> >> virtqueue does not exist."
> >>
> >
> > vmd diffs are always welcome, thanks. I'll take a look at this later today.
> >
> > -ml
> >
> >
> >> Index: virtio.c
> >> ===
> >> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> >> retrieving revision 1.49
> >> diff -u -p -u -p -r1.49 virtio.c
> >> --- virtio.c  30 May 2017 17:56:47 -  1.49
> >> +++ virtio.c  27 Jul 2017 04:35:46 -
> >> @@ -150,8 +150,10 @@ void
> >>  viornd_update_qs(void)
> >>  {
> >>   /* Invalid queue? */
> >> - if (viornd.cfg.queue_select > 0)
> >> + if (viornd.cfg.queue_select > 0) {
> >> + viornd.cfg.queue_size = 0;
> >>   return;
> >> + }
> >>
> >>   /* Update queue address/size based on queue select */
> >>   viornd.cfg.queue_address =
> >> viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
> >>  vioblk_update_qs(struct vioblk_dev *dev)
> >>  {
> >>   /* Invalid queue? */
> >> - if (dev->cfg.queue_select > 0)
> >> + if (dev->cfg.queue_select > 0) {
> >> + dev->cfg.queue_size = 0;
> >>   return;
> >> + }
> >>
> >>   /* Update queue address/size based on queue select */
> >>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> >> @@ -1037,8 +1041,10 @@ void
> >>  vionet_update_qs(struct vionet_dev *dev)
> >>  {
> >>   /* Invalid queue? */
> >> - if (dev->cfg.queue_select > 1)
> >> + if (dev->cfg.queue_select > 1) {
> >> + dev->cfg.queue_size = 0;
> >>   return;
> >> + }
> >>
> >>   /* Update queue address/size based on queue select */
> >>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> >>

Re: clang ld.so regress failures

[Alexander Bluhm]

On Thu, Aug 03, 2017 at 01:37:12PM +0200, Mark Kettenis wrote:
> With these fixes all ld.so regress tests pass again on amd64.

Also works on i386.

> ok?

OK bluhm@

> Index: constructor/libaa/aa.C
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/constructor/libaa/aa.C,v
> retrieving revision 1.2
> diff -u -p -r1.2 aa.C
> --- constructor/libaa/aa.C27 Sep 2016 06:52:50 -  1.2
> +++ constructor/libaa/aa.C3 Aug 2017 11:18:57 -
> @@ -5,7 +5,7 @@
>   */
>  
>  #include "aa.h"
> -int a;
> +volatile int a;
>  
>  
>  AA::AA(char *arg)
> Index: constructor/libab/Makefile
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/constructor/libab/Makefile,v
> retrieving revision 1.3
> diff -u -p -r1.3 Makefile
> --- constructor/libab/Makefile27 Sep 2016 18:14:22 -  1.3
> +++ constructor/libab/Makefile3 Aug 2017 11:18:57 -
> @@ -7,6 +7,8 @@ CPPFLAGS=-I${.CURDIR}/../libaa
>  LDADD=-L../libaa
>  LDADD+=-laa
>  
> +CC=${CXX}
> +
>  regress: all
>  
>  .include 
> Index: init-env/libaa/Makefile
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/init-env/libaa/Makefile,v
> retrieving revision 1.2
> diff -u -p -r1.2 Makefile
> --- init-env/libaa/Makefile   5 Sep 2016 07:20:34 -   1.2
> +++ init-env/libaa/Makefile   3 Aug 2017 11:18:57 -
> @@ -3,6 +3,8 @@
>  LIB= aa
>  SRCS=aa.C
>  
> +CC=${CXX}
> +
>  regress: all
>  
>  .include 
> Index: initfirst/test2/libaa/aa.C
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libaa/aa.C,v
> retrieving revision 1.1
> diff -u -p -r1.1 aa.C
> --- initfirst/test2/libaa/aa.C29 Nov 2011 04:36:15 -  1.1
> +++ initfirst/test2/libaa/aa.C3 Aug 2017 11:18:57 -
> @@ -4,7 +4,7 @@
>   * $OpenBSD: aa.C,v 1.1 2011/11/29 04:36:15 kurt Exp $
>   */
>  
> -#include 
> +#include 
>  
>  class AA
>  {
> @@ -15,12 +15,12 @@ class AA
>  
>  AA::AA()
>  {
> -   std::cout << "A";
> +   std::printf("A");
>  }
>  
>  AA::~AA()
>  {
> -   std::cout << "a";
> +   std::printf("a");
>  }
>  
>  AA a;
> Index: initfirst/test2/libab/ab.C
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libab/ab.C,v
> retrieving revision 1.1
> diff -u -p -r1.1 ab.C
> --- initfirst/test2/libab/ab.C29 Nov 2011 04:36:15 -  1.1
> +++ initfirst/test2/libab/ab.C3 Aug 2017 11:18:57 -
> @@ -4,7 +4,7 @@
>   * $OpenBSD: ab.C,v 1.1 2011/11/29 04:36:15 kurt Exp $
>   */
>  
> -#include 
> +#include 
>  
>  class AB
>  {
> @@ -15,12 +15,12 @@ class AB
>  
>  AB::AB()
>  {
> -   std::cout << "B";
> +   std::printf("B");
>  }
>  
>  AB::~AB()
>  {
> -   std::cout << "b";
> +   std::printf("b");
>  }
>  
>  AB b;
> Index: initfirst/test2/libac/ac.C
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libac/ac.C,v
> retrieving revision 1.1
> diff -u -p -r1.1 ac.C
> --- initfirst/test2/libac/ac.C29 Nov 2011 04:36:15 -  1.1
> +++ initfirst/test2/libac/ac.C3 Aug 2017 11:18:57 -
> @@ -4,7 +4,7 @@
>   * $OpenBSD: ac.C,v 1.1 2011/11/29 04:36:15 kurt Exp $
>   */
>  
> -#include 
> +#include 
>  
>  class AC
>  {
> @@ -15,12 +15,12 @@ class AC
>  
>  AC::AC()
>  {
> -   std::cout << "C";
> +   std::printf("C");
>  }
>  
>  AC::~AC()
>  {
> -   std::cout << "c";
> +   std::printf("c");
>  }
>  
>  AC c;
> Index: initfirst/test2/libad/ad.C
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libad/ad.C,v
> retrieving revision 1.1
> diff -u -p -r1.1 ad.C
> --- initfirst/test2/libad/ad.C29 Nov 2011 04:36:15 -  1.1
> +++ initfirst/test2/libad/ad.C3 Aug 2017 11:18:57 -
> @@ -4,7 +4,7 @@
>   * $OpenBSD: ad.C,v 1.1 2011/11/29 04:36:15 kurt Exp $
>   */
>  
> -#include 
> +#include 
>  
>  class AD
>  {
> @@ -15,12 +15,12 @@ class AD
>  
>  AD::AD()
>  {
> -   std::cout << "D";
> +   std::printf("D");
>  }
>  
>  AD::~AD()
>  {
> -   std::cout << "d";
> +   std::printf("d");
>  }
>  
>  AD d;
> Index: initfirst/test2/libae/ae.C
> ===
> RCS file: /cvs/src/regress/libexec/ld.so/initfirst/test2/libae/ae.C,v
> retrieving revision 1.1
> diff -u -p -r1.1 ae.C
> --- initfirst/test2/libae/ae.C29 Nov 2011 04:36:15 -  1.1
> +++ initfirst/test2/libae/ae.C3 Aug 2017 11:18:57 -
> @@ -4,7 +4,7 @@
>   * $OpenBSD: ae.C,v 1.1 2011/11/29 04:36:15 kurt Exp $
>   */
>  
> -#include 
> +#include 
>  
>  class AE
>  {
> @@ -15,12 +15,12 @@ class AE
>  
>  AE::AE()
>  {
> -   std::cout << "E";
> +   std::printf("E");
>  }
>  
>  

Fix clang's lgamma(3) builtin

[Mark Kettenis]

So the regress failure is a clang bug.  Already fixed upstream:

  https://reviews.llvm.org/D29778

ok?


Index: gnu/llvm/tools/clang/include/clang/Basic/Builtins.def
===
RCS file: /cvs/src/gnu/llvm/tools/clang/include/clang/Basic/Builtins.def,v
retrieving revision 1.1.1.4
diff -u -p -r1.1.1.4 Builtins.def
--- gnu/llvm/tools/clang/include/clang/Basic/Builtins.def   14 Mar 2017 
08:07:54 -  1.1.1.4
+++ gnu/llvm/tools/clang/include/clang/Basic/Builtins.def   4 Aug 2017 
14:24:47 -
@@ -1086,9 +1086,11 @@ LIBBUILTIN(ilogb, "id", "fne", "math.h",
 LIBBUILTIN(ilogbf, "if", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(ilogbl, "iLd", "fne", "math.h", ALL_LANGUAGES)
 
-LIBBUILTIN(lgamma, "dd", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(lgammaf, "ff", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(lgammal, "LdLd", "fne", "math.h", ALL_LANGUAGES)
+// POSIX math.h declares a global, signgam, that lgamma writes to, so these
+// shouldn't have "e" or "c" attributes
+LIBBUILTIN(lgamma, "dd", "fn", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(lgammaf, "ff", "fn", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(lgammal, "LdLd", "fn", "math.h", ALL_LANGUAGES)
 
 LIBBUILTIN(llrint, "LLid", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(llrintf, "LLif", "fne", "math.h", ALL_LANGUAGES)

Re: inform userland on ifconfig $if -inet6

[Alexander Bluhm]

On Fri, Aug 04, 2017 at 03:38:40PM +, Florian Obser wrote:
> When we disable INET6 on an interface that also removes the autoconf6
> flag.  Notify userland about this via the route socket, otherwise
> slaacd gets confused about the state the interface is in.

The other rtm_ifchg() calls are in net/if.c.  Should this one
not also be there in the SIOCIFAFDETACH case?

bluhm

> diff --git netinet6/in6_ifattach.c netinet6/in6_ifattach.c
> index 89acde9c6a4..65eac6dfa56 100644
> --- netinet6/in6_ifattach.c
> +++ netinet6/in6_ifattach.c
> @@ -560,6 +560,8 @@ in6_ifdetach(struct ifnet *ifp)
>   rtfree(rt);
>   }
>  
> - if (ifp->if_xflags & IFXF_AUTOCONF6)
> + if (ifp->if_xflags & IFXF_AUTOCONF6) {
>   ifp->if_xflags &= ~IFXF_AUTOCONF6;
> + rtm_ifchg(ifp);
> + }
>  }
> 
> 
> -- 
> I'm not entirely sure you are real.

inform userland on ifconfig $if -inet6

[Florian Obser]

When we disable INET6 on an interface that also removes the autoconf6
flag.  Notify userland about this via the route socket, otherwise
slaacd gets confused about the state the interface is in.

OK?

diff --git netinet6/in6_ifattach.c netinet6/in6_ifattach.c
index 89acde9c6a4..65eac6dfa56 100644
--- netinet6/in6_ifattach.c
+++ netinet6/in6_ifattach.c
@@ -560,6 +560,8 @@ in6_ifdetach(struct ifnet *ifp)
rtfree(rt);
}
 
-   if (ifp->if_xflags & IFXF_AUTOCONF6)
+   if (ifp->if_xflags & IFXF_AUTOCONF6) {
ifp->if_xflags &= ~IFXF_AUTOCONF6;
+   rtm_ifchg(ifp);
+   }
 }


-- 
I'm not entirely sure you are real.

[patch] puc(4) add ASIX AX99100 support

[SASANO Takayoshi]

hello, here is AX99100 4port serial PCI-Express controller support.
applying this patch, AX99100 is recognized as follows:

puc0 at pci3 dev 0 function 0 "ASIX AX99100" rev 0x00: ports: 1 com
com4 at puc0 port 0 apic 5 int 17: st16650, 32 byte fifo
puc1 at pci3 dev 0 function 1 "ASIX AX99100" rev 0x00: ports: 1 com
com5 at puc1 port 0 apic 5 int 18: st16650, 32 byte fifo
puc2 at pci3 dev 0 function 2 "ASIX AX99100" rev 0x00: ports: 1 com
com6 at puc2 port 0 apic 5 int 19: st16650, 32 byte fifo
puc3 at pci3 dev 0 function 3 "ASIX AX99100" rev 0x00: ports: 1 com
com7 at puc3 port 0 apic 5 int 16: st16650, 32 byte fifo

ok?
-- 
SASANO Takayoshi (JG1UAA) 

Index: pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1823
diff -u -p -r1.1823 pcidevs
--- pcidevs 16 Jul 2017 13:41:58 -  1.1823
+++ pcidevs 4 Aug 2017 12:53:12 -
@@ -966,6 +966,7 @@ product ARECA ARC1880   0x1880  ARC-1880
 
 /* ASIX Electronics products */
 product ASIX AX88140A  0x1400  AX88140A/88141
+product ASIX AX99100   0x9100  AX99100
 
 /* ASMedia products */
 product ASMEDIA ASM1061_SATA   0x0611  ASM1061 SATA
Index: pcidevs.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs.h,v
retrieving revision 1.1817
diff -u -p -r1.1817 pcidevs.h
--- pcidevs.h   16 Jul 2017 13:42:20 -  1.1817
+++ pcidevs.h   4 Aug 2017 12:53:13 -
@@ -971,6 +971,7 @@
 
 /* ASIX Electronics products */
 #definePCI_PRODUCT_ASIX_AX88140A   0x1400  /* 
AX88140A/88141 */
+#definePCI_PRODUCT_ASIX_AX991000x9100  /* AX99100 */
 
 /* ASMedia products */
 #definePCI_PRODUCT_ASMEDIA_ASM1061_SATA0x0611  /* 
ASM1061 SATA */
Index: pcidevs_data.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs_data.h,v
retrieving revision 1.1811
diff -u -p -r1.1811 pcidevs_data.h
--- pcidevs_data.h  16 Jul 2017 13:42:20 -  1.1811
+++ pcidevs_data.h  4 Aug 2017 12:53:14 -
@@ -2152,6 +2152,10 @@ static const struct pci_known_product pc
"AX88140A/88141",
},
{
+   PCI_VENDOR_ASIX, PCI_PRODUCT_ASIX_AX99100,
+   "AX99100",
+   },
+   {
PCI_VENDOR_ASMEDIA, PCI_PRODUCT_ASMEDIA_ASM1061_SATA,
"ASM1061 SATA",
},
Index: pucdata.c
===
RCS file: /cvs/src/sys/dev/pci/pucdata.c,v
retrieving revision 1.105
diff -u -p -r1.105 pucdata.c
--- pucdata.c   6 May 2017 01:54:31 -   1.105
+++ pucdata.c   4 Aug 2017 12:53:14 -
@@ -2200,6 +2200,13 @@ const struct puc_device_description puc_
{ PUC_COM_POW2(0), 0x14, 0x },
}
},
+   {   /* "ASIX AX99100", */
+   {   PCI_VENDOR_ASIX, PCI_PRODUCT_ASIX_AX99100,  0, 0},
+   {   0x, 0x, 0, 0},
+   {
+   { PUC_COM_POW2(0), 0x10, 0x },
+   }
+   },
{   /* "NetMos NM9820 UART" */
{   PCI_VENDOR_NETMOS, PCI_PRODUCT_NETMOS_NM9820,   0, 0},
{   0x, 0x, 0, 0},

Re: SO_TIMESTAMP

[Alexander Bluhm]

On Fri, Aug 04, 2017 at 12:36:36PM +, Florian Obser wrote:
> We do have SO_TIMESTAMP since some time and there is other code in the
> kernel that uses it without the #ifdef guard.
> 
> OK?

OK bluhm@

> 
> diff --git netinet/ip_input.c netinet/ip_input.c
> index ee74eeadc4c..30fa3597fbb 100644
> --- netinet/ip_input.c
> +++ netinet/ip_input.c
> @@ -1711,7 +1711,6 @@ void
>  ip_savecontrol(struct inpcb *inp, struct mbuf **mp, struct ip *ip,
>  struct mbuf *m)
>  {
> -#ifdef SO_TIMESTAMP
>   if (inp->inp_socket->so_options & SO_TIMESTAMP) {
>   struct timeval tv;
>  
> @@ -1721,7 +1720,7 @@ ip_savecontrol(struct inpcb *inp, struct mbuf **mp, 
> struct ip *ip,
>   if (*mp)
>   mp = &(*mp)->m_next;
>   }
> -#endif
> +
>   if (inp->inp_flags & INP_RECVDSTADDR) {
>   *mp = sbcreatecontrol((caddr_t) >ip_dst,
>   sizeof(struct in_addr), IP_RECVDSTADDR, IPPROTO_IP);
> diff --git netinet6/ip6_input.c netinet6/ip6_input.c
> index ed8702fa71a..3da635a70ab 100644
> --- netinet6/ip6_input.c
> +++ netinet6/ip6_input.c
> @@ -942,7 +942,6 @@ ip6_savecontrol(struct inpcb *in6p, struct mbuf *m, 
> struct mbuf **mp)
>  {
>   struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
>  
> -#ifdef SO_TIMESTAMP
>   if (in6p->inp_socket->so_options & SO_TIMESTAMP) {
>   struct timeval tv;
>  
> @@ -952,7 +951,6 @@ ip6_savecontrol(struct inpcb *in6p, struct mbuf *m, 
> struct mbuf **mp)
>   if (*mp)
>   mp = &(*mp)->m_next;
>   }
> -#endif
>  
>   /* RFC 2292 sec. 5 */
>   if ((in6p->inp_flags & IN6P_PKTINFO) != 0) {
> 
> 
> -- 
> I'm not entirely sure you are real.

Re: [patch] remove smtpd filter code

[Gilles Chehade]

On Fri, Aug 04, 2017 at 02:56:21PM +0200, Gilles Chehade wrote:
> On Fri, Aug 04, 2017 at 01:13:06PM +0200, Eric Faurot wrote:
> > Hi,
> > 
> > Experimental support for filters has been removed some time ago from
> > the config parser.  Now we want to get rid of the remaining code.
> > It's not that trivial, so we proceed in several steps.
> > 
> > The first (and trickiest) one is to bypass the filter code for
> > incoming smtp sessions, so that filter.c can be unhooked.
> > This is what the following diff does:
> > 
> > - drop filter configuration,
> > - drop filter events,
> > - simulate a positive reply for all filter queries,
> > - write message content directly to the file.
> > 
> > There should be no functionnal change.
> > 
> 
> this should be tested by many people right away to spot subtle regressions
> 

Just a clarification because i've received the same question three times
since this mail :-)

We're not killing filters, most of the implementation is correct and the
filters support is pretty much functional at the exception of some cases
which are very unfortunately show stoppers...

We're removing the filter code from the SMTP state machine to plug it at
a different spot and fix the current limitations. Most of the code is to
be reused, this is not a rewrite but a refactor.

-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg

Re: [patch] remove smtpd filter code

[Gilles Chehade]

On Fri, Aug 04, 2017 at 01:13:06PM +0200, Eric Faurot wrote:
> Hi,
> 
> Experimental support for filters has been removed some time ago from
> the config parser.  Now we want to get rid of the remaining code.
> It's not that trivial, so we proceed in several steps.
> 
> The first (and trickiest) one is to bypass the filter code for
> incoming smtp sessions, so that filter.c can be unhooked.
> This is what the following diff does:
> 
> - drop filter configuration,
> - drop filter events,
> - simulate a positive reply for all filter queries,
> - write message content directly to the file.
> 
> There should be no functionnal change.
> 

this should be tested by many people right away to spot subtle regressions

ok gilles@


> Index: pony.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/pony.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 pony.c
> --- pony.c9 Jan 2017 09:53:23 -   1.17
> +++ pony.c3 Aug 2017 09:57:22 -
> @@ -60,7 +60,7 @@ pony_imsg(struct mproc *p, struct imsg *
>   case IMSG_CONF_START:
>   return;
>   case IMSG_CONF_END:
> - filter_configure();
> + smtp_configure();
>   return;
>   case IMSG_CTL_VERBOSE:
>   m_msg(, imsg);
> @@ -148,7 +148,6 @@ pony(void)
>   mda_postfork();
>   mta_postfork();
>   smtp_postfork();
> - filter_postfork();
>  
>   /* do not purge listeners and pki, they are purged
>* in smtp_configure()
> Index: smtp_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.304
> diff -u -p -r1.304 smtp_session.c
> --- smtp_session.c19 Jun 2017 08:35:56 -  1.304
> +++ smtp_session.c4 Aug 2017 10:32:41 -
> @@ -69,9 +69,6 @@ enum session_flags {
>   SF_BOUNCE   = 0x0010,
>   SF_VERIFIED = 0x0020,
>   SF_BADINPUT = 0x0080,
> - SF_FILTERCONN   = 0x0100,
> - SF_FILTERDATA   = 0x0200,
> - SF_FILTERTX = 0x0400,
>  };
>  
>  enum message_flags {
> @@ -116,7 +113,7 @@ struct smtp_tx {
>  
>   size_t   datain;
>   size_t   odatalen;
> - struct io   *oev;
> + FILE*ofile;
>   int  hdrdone;
>   int  rcvcount;
>   int  dataeom;
> @@ -167,7 +164,6 @@ static void smtp_connected(struct smtp_s
>  static void smtp_send_banner(struct smtp_session *);
>  static void smtp_tls_verified(struct smtp_session *);
>  static void smtp_io(struct io *, int, void *);
> -static void smtp_data_io(struct io *, int, void *);
>  static void smtp_data_io_done(struct smtp_session *);
>  static void smtp_enter_state(struct smtp_session *, int);
>  static void smtp_reply(struct smtp_session *, char *, ...);
> @@ -194,11 +190,6 @@ static void smtp_queue_commit(struct smt
>  static void smtp_queue_rollback(struct smtp_session *);
>  
>  static void smtp_filter_connect(struct smtp_session *, struct sockaddr *);
> -static void smtp_filter_rset(struct smtp_session *);
> -static void smtp_filter_disconnect(struct smtp_session *);
> -static void smtp_filter_tx_begin(struct smtp_session *);
> -static void smtp_filter_tx_commit(struct smtp_session *);
> -static void smtp_filter_tx_rollback(struct smtp_session *);
>  static void smtp_filter_eom(struct smtp_session *);
>  static void smtp_filter_helo(struct smtp_session *);
>  static void smtp_filter_mail(struct smtp_session *);
> @@ -728,12 +719,10 @@ smtp_session_imsg(struct mproc *p, struc
>   break;
>  
>   case LKA_PERMFAIL:
> - smtp_filter_tx_rollback(s);
>   smtp_tx_free(s->tx);
>   smtp_reply(s, "%d %s", 530, "Sender rejected");
>   break;
>   case LKA_TEMPFAIL:
> - smtp_filter_tx_rollback(s);
>   smtp_tx_free(s->tx);
>   smtp_reply(s, "421 %s: Temporary Error",
>   esc_code(ESC_STATUS_TEMPFAIL, 
> ESC_OTHER_MAIL_SYSTEM_STATUS));
> @@ -785,7 +774,6 @@ smtp_session_imsg(struct mproc *p, struc
>   smtp_reply(s, "250 %s: Ok",
>   esc_code(ESC_STATUS_OK, ESC_OTHER_STATUS));
>   } else {
> - smtp_filter_tx_rollback(s);
>   smtp_tx_free(s->tx);
>   smtp_reply(s, "421 %s: Temporary Error",
>   esc_code(ESC_STATUS_TEMPFAIL, 
> ESC_OTHER_MAIL_SYSTEM_STATUS));
> @@ -813,7 +801,7 @@ smtp_session_imsg(struct mproc *p, struc
>   log_debug("smtp: %p: fd %d from queue", s, imsg->fd);
>  
>   tree_xset(_filter, s->id, s);
> - filter_build_fd_chain(s->id, imsg->fd);
> +  

SO_TIMESTAMP

[Florian Obser]

We do have SO_TIMESTAMP since some time and there is other code in the
kernel that uses it without the #ifdef guard.

OK?

diff --git netinet/ip_input.c netinet/ip_input.c
index ee74eeadc4c..30fa3597fbb 100644
--- netinet/ip_input.c
+++ netinet/ip_input.c
@@ -1711,7 +1711,6 @@ void
 ip_savecontrol(struct inpcb *inp, struct mbuf **mp, struct ip *ip,
 struct mbuf *m)
 {
-#ifdef SO_TIMESTAMP
if (inp->inp_socket->so_options & SO_TIMESTAMP) {
struct timeval tv;
 
@@ -1721,7 +1720,7 @@ ip_savecontrol(struct inpcb *inp, struct mbuf **mp, 
struct ip *ip,
if (*mp)
mp = &(*mp)->m_next;
}
-#endif
+
if (inp->inp_flags & INP_RECVDSTADDR) {
*mp = sbcreatecontrol((caddr_t) >ip_dst,
sizeof(struct in_addr), IP_RECVDSTADDR, IPPROTO_IP);
diff --git netinet6/ip6_input.c netinet6/ip6_input.c
index ed8702fa71a..3da635a70ab 100644
--- netinet6/ip6_input.c
+++ netinet6/ip6_input.c
@@ -942,7 +942,6 @@ ip6_savecontrol(struct inpcb *in6p, struct mbuf *m, struct 
mbuf **mp)
 {
struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
 
-#ifdef SO_TIMESTAMP
if (in6p->inp_socket->so_options & SO_TIMESTAMP) {
struct timeval tv;
 
@@ -952,7 +951,6 @@ ip6_savecontrol(struct inpcb *in6p, struct mbuf *m, struct 
mbuf **mp)
if (*mp)
mp = &(*mp)->m_next;
}
-#endif
 
/* RFC 2292 sec. 5 */
if ((in6p->inp_flags & IN6P_PKTINFO) != 0) {


-- 
I'm not entirely sure you are real.

[patch] remove smtpd filter code

[Eric Faurot]

Hi,

Experimental support for filters has been removed some time ago from
the config parser.  Now we want to get rid of the remaining code.
It's not that trivial, so we proceed in several steps.

The first (and trickiest) one is to bypass the filter code for
incoming smtp sessions, so that filter.c can be unhooked.
This is what the following diff does:

- drop filter configuration,
- drop filter events,
- simulate a positive reply for all filter queries,
- write message content directly to the file.

There should be no functionnal change.

Eric.

Index: pony.c
===
RCS file: /cvs/src/usr.sbin/smtpd/pony.c,v
retrieving revision 1.17
diff -u -p -r1.17 pony.c
--- pony.c  9 Jan 2017 09:53:23 -   1.17
+++ pony.c  3 Aug 2017 09:57:22 -
@@ -60,7 +60,7 @@ pony_imsg(struct mproc *p, struct imsg *
case IMSG_CONF_START:
return;
case IMSG_CONF_END:
-   filter_configure();
+   smtp_configure();
return;
case IMSG_CTL_VERBOSE:
m_msg(, imsg);
@@ -148,7 +148,6 @@ pony(void)
mda_postfork();
mta_postfork();
smtp_postfork();
-   filter_postfork();
 
/* do not purge listeners and pki, they are purged
 * in smtp_configure()
Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.304
diff -u -p -r1.304 smtp_session.c
--- smtp_session.c  19 Jun 2017 08:35:56 -  1.304
+++ smtp_session.c  4 Aug 2017 10:32:41 -
@@ -69,9 +69,6 @@ enum session_flags {
SF_BOUNCE   = 0x0010,
SF_VERIFIED = 0x0020,
SF_BADINPUT = 0x0080,
-   SF_FILTERCONN   = 0x0100,
-   SF_FILTERDATA   = 0x0200,
-   SF_FILTERTX = 0x0400,
 };
 
 enum message_flags {
@@ -116,7 +113,7 @@ struct smtp_tx {
 
size_t   datain;
size_t   odatalen;
-   struct io   *oev;
+   FILE*ofile;
int  hdrdone;
int  rcvcount;
int  dataeom;
@@ -167,7 +164,6 @@ static void smtp_connected(struct smtp_s
 static void smtp_send_banner(struct smtp_session *);
 static void smtp_tls_verified(struct smtp_session *);
 static void smtp_io(struct io *, int, void *);
-static void smtp_data_io(struct io *, int, void *);
 static void smtp_data_io_done(struct smtp_session *);
 static void smtp_enter_state(struct smtp_session *, int);
 static void smtp_reply(struct smtp_session *, char *, ...);
@@ -194,11 +190,6 @@ static void smtp_queue_commit(struct smt
 static void smtp_queue_rollback(struct smtp_session *);
 
 static void smtp_filter_connect(struct smtp_session *, struct sockaddr *);
-static void smtp_filter_rset(struct smtp_session *);
-static void smtp_filter_disconnect(struct smtp_session *);
-static void smtp_filter_tx_begin(struct smtp_session *);
-static void smtp_filter_tx_commit(struct smtp_session *);
-static void smtp_filter_tx_rollback(struct smtp_session *);
 static void smtp_filter_eom(struct smtp_session *);
 static void smtp_filter_helo(struct smtp_session *);
 static void smtp_filter_mail(struct smtp_session *);
@@ -728,12 +719,10 @@ smtp_session_imsg(struct mproc *p, struc
break;
 
case LKA_PERMFAIL:
-   smtp_filter_tx_rollback(s);
smtp_tx_free(s->tx);
smtp_reply(s, "%d %s", 530, "Sender rejected");
break;
case LKA_TEMPFAIL:
-   smtp_filter_tx_rollback(s);
smtp_tx_free(s->tx);
smtp_reply(s, "421 %s: Temporary Error",
esc_code(ESC_STATUS_TEMPFAIL, 
ESC_OTHER_MAIL_SYSTEM_STATUS));
@@ -785,7 +774,6 @@ smtp_session_imsg(struct mproc *p, struc
smtp_reply(s, "250 %s: Ok",
esc_code(ESC_STATUS_OK, ESC_OTHER_STATUS));
} else {
-   smtp_filter_tx_rollback(s);
smtp_tx_free(s->tx);
smtp_reply(s, "421 %s: Temporary Error",
esc_code(ESC_STATUS_TEMPFAIL, 
ESC_OTHER_MAIL_SYSTEM_STATUS));
@@ -813,7 +801,7 @@ smtp_session_imsg(struct mproc *p, struc
log_debug("smtp: %p: fd %d from queue", s, imsg->fd);
 
tree_xset(_filter, s->id, s);
-   filter_build_fd_chain(s->id, imsg->fd);
+   smtp_filter_fd(s->id, imsg->fd);
return;
 
case IMSG_QUEUE_ENVELOPE_SUBMIT:
@@ -869,7 +857,6 @@ smtp_session_imsg(struct mproc *p, struc
m_end();
s = tree_xpop(_queue_commit, reqid);
if