Re: pipex(4): use reference counters for `ifnet'

[Christiano F. Haesbaert]

You can.

The idea is that ifindex is always monotonically increased, so to actually
get a new interface you would have to have "overflowed" 65k interfaces,
which is unreal.

So if your interface is gone, you can be sure if_get will give you NULL.

On Thu, Jun 25, 2020, 18:55 Vitaliy Makkoveev 
wrote:

> Updated diff.
>
> OpenBSD uses 16 bit counter for allocate interface indexes. So we can't
> store index in session and be sure if_get(9) returned `ifnet' is our
> original `ifnet'.
>
> Now each pipex(4) session has it's own reference to `ifnet'. Also pppoe
> related sessions has the reference to it's ethernet interface.
>
> All `ifnet's are obtained by if_get(9) and we use `if_index' from stored
> reference to `ifnet'.
>
> Index: net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.90
> diff -u -p -r1.90 if_pppx.c
> --- net/if_pppx.c   24 Jun 2020 08:52:53 -  1.90
> +++ net/if_pppx.c   25 Jun 2020 16:41:25 -
> @@ -714,15 +714,15 @@ pppx_add_session(struct pppx_dev *pxd, s
> ifp->if_softc = pxi;
> /* ifp->if_rdomain = req->pr_rdomain; */
>
> -   error = pipex_link_session(session, >pxi_ifcontext);
> -   if (error)
> -   goto remove;
> -
> /* XXXSMP breaks atomicity */
> NET_UNLOCK();
> if_attach(ifp);
> NET_LOCK();
>
> +   error = pipex_link_session(session, >pxi_ifcontext);
> +   if (error)
> +   goto detach;
> +
> if_addgroup(ifp, "pppx");
> if_alloc_sadl(ifp);
>
> @@ -771,7 +771,12 @@ pppx_add_session(struct pppx_dev *pxd, s
>
> return (error);
>
> -remove:
> +detach:
> +   /* XXXSMP breaks atomicity */
> +   NET_UNLOCK();
> +   if_detach(ifp);
> +   NET_LOCK();
> +
> if (RBT_REMOVE(pppx_ifs, _ifs, pxi) == NULL)
> panic("%s: inconsistent RB tree", __func__);
> LIST_REMOVE(pxi, pxi_list);
> @@ -860,13 +865,13 @@ pppx_if_destroy(struct pppx_dev *pxd, st
> CLR(ifp->if_flags, IFF_RUNNING);
>
> pipex_unlink_session(session);
> +   pipex_rele_session(session);
>
> /* XXXSMP breaks atomicity */
> NET_UNLOCK();
> if_detach(ifp);
> NET_LOCK();
>
> -   pipex_rele_session(session);
> if (RBT_REMOVE(pppx_ifs, _ifs, pxi) == NULL)
> panic("%s: inconsistent RB tree", __func__);
> LIST_REMOVE(pxi, pxi_list);
> Index: net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 pipex.c
> --- net/pipex.c 22 Jun 2020 09:38:15 -  1.116
> +++ net/pipex.c 25 Jun 2020 16:41:25 -
> @@ -148,7 +148,7 @@ pipex_iface_init(struct pipex_iface_cont
> struct pipex_session *session;
>
> pipex_iface->pipexmode = 0;
> -   pipex_iface->ifnet_this = ifp;
> +   pipex_iface->ifnet_this = if_get(ifp->if_index);
>
> if (pipex_rd_head4 == NULL) {
> if (!rn_inithead((void **)_rd_head4,
> @@ -165,6 +165,7 @@ pipex_iface_init(struct pipex_iface_cont
> session = pool_get(_session_pool, PR_WAITOK | PR_ZERO);
> session->is_multicast = 1;
> session->pipex_iface = pipex_iface;
> +   session->ifnet_this = if_get(ifp->if_index);
> pipex_iface->multicast_session = session;
>  }
>
> @@ -194,10 +195,11 @@ pipex_iface_stop(struct pipex_iface_cont
>  void
>  pipex_iface_fini(struct pipex_iface_context *pipex_iface)
>  {
> -   pool_put(_session_pool, pipex_iface->multicast_session);
> NET_LOCK();
> pipex_iface_stop(pipex_iface);
> NET_UNLOCK();
> +   pipex_rele_session(pipex_iface->multicast_session);
> +   if_put(pipex_iface->ifnet_this);
>  }
>
>  int
> @@ -358,7 +360,7 @@ pipex_init_session(struct pipex_session
> MIN(req->pr_local_address.ss_len,
> sizeof(session->local)));
>  #ifdef PIPEX_PPPOE
> if (req->pr_protocol == PIPEX_PROTO_PPPOE)
> -   session->proto.pppoe.over_ifidx = over_ifp->if_index;
> +   session->proto.pppoe.over_iface =
> if_get(over_ifp->if_index);
>  #endif
>  #ifdef PIPEX_PPTP
> if (req->pr_protocol == PIPEX_PROTO_PPTP) {
> @@ -421,6 +423,13 @@ pipex_init_session(struct pipex_session
>  void
>  pipex_rele_session(struct pipex_session *session)
>  {
> +#ifdef PIPEX_PPPOE
> +   if (session->protocol == PIPEX_PROTO_PPPOE)
> +   if_put(session->proto.pppoe.over_iface);
> +#endif
> +   if (session->ifnet_this) {
> +   if_put(session->ifnet_this);
> +   }
> if (session->mppe_recv.old_session_keys)
> pool_put(_key_pool,
> session->mppe_recv.old_session_keys);
> pool_put(_session_pool, session);
> @@ -439,6 +448,7 @@ pipex_link_session(struct pipex_session
> return (EEXIST);
>
> session->pipex_iface = 

Re: pipex(4): use reference counters for `ifnet'

[Christiano F. Haesbaert]

On Thu, 25 Jun 2020 at 14:06, Vitaliy Makkoveev
 wrote:
>
>
>
> > On 25 Jun 2020, at 11:55, Martin Pieuchot  wrote:
> >
> > On 24/06/20(Wed) 17:10, Vitaliy Makkoveev wrote:
> >> While `mbuf' enqueued to `pipexinq' or `pipexoutq' it has the reference
> >> to corresponding pipex(4) session as `ph_cookie'. `ph_cookie' is
> >> accessed by pipexintr() and it's always defferent context from context
> >> where we destroy session. `ph_cookie' is protected only while we destroy
> >> session by pipex_timer() but this protection is not effective. While we
> >> destroy session related to pppx(4) `ph_cookie' is not potected. While we
> >> destroy session related to pppac(4) by `PIPEXSMODE' ioctl() or by
> >> closing pppac(4) device node `ph_cookie' is also not protected.
> >>
> >> I'am going to use reference counters to protect `ph_cookie' but some
> >> additional steps required to be done.
> >
> > Please no.  Store an ifidx in session instead of a pointer.  Such
> > index are guaranteed to be unique and can be used with if_get(9).
>
> This means I should do if_get(9) before each `ifnet’ usage. Also I
> should do checks and introduce error path. It's ugly.
>

My 2 cents,

Not really, this is actually easier to reason with, it produces less
dependencies between data structures and the protocol is kinda clear,
you have to account for the loss of atomicity.
Reference counting is pretty much the worst thing you can do in MP development.
Very often it is actually desirable to pay for the cost of a lookup
(which is not the case here, since it's a silly O(1)) than to pay the
human/debug cost of interlinking data structures.
Also, in an ideal world if_get() would be RCUed (or SMRed) and would
not require bumping a reference count which implies an atomic
operation, it would also remove the need for if_put() since the epoch
is "until we context switch", as the kernel is not preemptive this is
easy to do without introducing new bugs.


> Alternatively, I can pass `ifnet->if_index’ instead of `ifnet’ to
> sessinon being linked and store obtained reference up to session
> destruction. I should add one error path to pipex_link_session().
> It’s ugly but a little bit.
>
> >
> > We deliberately kept if_ref() private to keep the code base coherent.
>
> Could you explain please why if_ref() and if_get() are incoherent?

Re: pool_debug

[Christiano F. Haesbaert]

On Tue, 24 Jan 2017 at 09:14, Martin Pieuchot <m...@openbsd.org> wrote:

> On 24/01/17(Tue) 08:06, Christiano F. Haesbaert wrote:
>
> > Not sure I get it, the rwlock when is not released when you yield()). So
>
> > this will in fact context switch holding the rwlock for every pool_get().
>
> > Did I miss another a change ?
>
>
>
> That's true.  I'd like to know when that happens and where.  The diff
>
> below combined with a NET_ASSERT_UNLOCKED() in yield() allows me to
>
> find that.


>
Ah, I missed the assert, thanks.

Re: pool_debug

[Christiano F. Haesbaert]

Not sure I get it, the rwlock when is not released when you yield()). So
this will in fact context switch holding the rwlock for every pool_get().
Did I miss another a change ?




On Tue, 24 Jan 2017 at 07:48, Martin Pieuchot  wrote:

> I'd like to force a yield() for every pool_get(9) using PR_WAITOK, just
>
> like we do with malloc(9), in order to ensure that the NET_LOCK() is not
>
> held across context switches.
>
>
>
> ok?
>
>
>
> Index: kern/subr_pool.c
>
> ===
>
> RCS file: /cvs/src/sys/kern/subr_pool.c,v
>
> retrieving revision 1.204
>
> diff -u -p -r1.204 subr_pool.c
>
> --- kern/subr_pool.c21 Nov 2016 01:44:06 -  1.204
>
> +++ kern/subr_pool.c24 Jan 2017 06:29:09 -
>
> @@ -513,7 +513,7 @@ pool_get(struct pool *pp, int flags)
>
> }
>
> mtx_leave(>pr_mtx);
>
>
>
> -   if (slowdown && ISSET(flags, PR_WAITOK))
>
> +   if ((slowdown || pool_debug == 2) && ISSET(flags, PR_WAITOK))
>
> yield();
>
>
>
> if (v == NULL) {
>
>
>
>

Re: timeout_set_proc(9)

[Christiano F. Haesbaert]

On 5 October 2016 at 18:26, Ted Unangst <t...@tedunangst.com> wrote:
> Christiano F. Haesbaert wrote:
>> There is another bug, the thread runs outside of IPL_SOFTCLOCK, the
>> interrupt handler already runs at IPL_SOFTCLOCK so it did not need to raise
>> it, but the thread does.
>>
>> The mutex is not enough as it will drop before running the handler, this
>> can cause intr timeouts to interrupt proc timeouts.
>
> Is this a real bug? A proc timeout can, by definition, sleep, so it shouldn't
> be making any assumptions about running atomically. If it needs to block
> timeouts, the handler should use spl.

I agree, you can't assume it is atomic, but at this point, in this
transition phase I'd raise the ipl, since the code written under is
assumed to be running without any timeouts triggering.
The code that actually does sleep (say on a rwlock), is "new" code
that should address losing the atomicity, but again, at this point,
this does not happen and it breaks an old invariant.
If this does not hold, then we shouldn't pin the thread to cpu0
either, it should run freely.

Re: timeout_set_proc(9)

[Christiano F. Haesbaert]

Am Montag, 26. September 2016 schrieb David Gwynne :

>
> > On 26 Sep 2016, at 13:36, Ted Unangst  > wrote:
> >
> > David Gwynne wrote:
> >> +mtx_enter(_mutex);
> >> +while (!CIRCQ_EMPTY(_proc)) {
> >> +to = timeout_from_circq(CIRCQ_
> FIRST(_proc));
> >> +CIRCQ_REMOVE(>to_list);
> >   leave();
> >> +timeout_run(to);
> >   enter();
> >> +}
> >> +mtx_leave(_mutex);
> >
> > you need to drop the mutex when running the timeout. with those changes,
> looks
> > pretty good.
>
> timeout_run drops the mutex.



There is another bug, the thread runs outside of IPL_SOFTCLOCK, the
interrupt handler already runs at IPL_SOFTCLOCK so it did not need to raise
it, but the thread does.

The mutex is not enough as it will drop before running the handler, this
can cause intr timeouts to interrupt proc timeouts.

I suggest raising it on the thread startup and leaving it always up. That's
why my diff had added IPL support for tasks btw.

Re: timeout_set_proc(9)

[Christiano F. Haesbaert]

Am Samstag, 24. September 2016 schrieb David Gwynne :

> On Fri, Sep 23, 2016 at 10:16:34PM -0700, Philip Guenther wrote:
> > On Fri, 23 Sep 2016, Christiano F. Haesbaert wrote:
> > ...
> > > The diff as it is will deadlock against SCHED_LOCK.
> > > tsleep() calls sleep_setup(), which grabs SCHED_LOCK,
> > > Then sleep_setup_timeout() will grab timeout_mutex in timeout_add()
> > >
> > > On softclock() you have the opposite:
> > > Grabs timeout_mutex then does a wakeup, which grabs SCHED_LOCK.
>
> nice.
>
> >
> > Hmm, yes. And softclock_thread() has the same problem: msleep() needs to
> > grab SCHED_LOCK while the passed in mutex is held, so it'll hold
> > timeout_mutex while grabbing SCHED_LOCK too.
> >
> > I just played around with using a separate mutex for protecting
> > timeout_proc, a mutex which would be 'outside' SCHED_LOCK, unlike
> > timeout_mutex which is 'inside' SCHED_LOCK.  The catch is that supporting
> > all the functionality of timeout_add() and timeout_del() becomes ugly and
> > fragile: they would need to check whether the mutex has
> > TIMEOUT_NEEDPROCCTX set and, if so, grab the new mutex before grabbing
> > timeout_mutex. ??That's "safe" from being a lock loop from tsleep()
> because
> > the thread's p_sleep_to *can't* have that flag set, so the 'outside'
> mutex
> > wouldn't be neededbut geez is this ugly.  I'm also unsure what
> defined
> > semantics, if any, timeout_triggered() should have for NEEDPROCCTX
> > timeouts.  Should it return true once the timeout has been dequeued from
> > the timeout wheel, or should it only be set once softclock_thread is
> > actually going to run it?
> >
> >
> > ...or maybe this makes people think we should toss this out and go
> > directly to dlg@'s proposal...
>
> let's not go crazy.
>
> i believe the deadlock can be fixed by moving the wakeup outside
> the hold of timeout_mutex in timeout_add. you only need to wake up
> the thread once even if you queued multiple timeouts, cos the thread
> will loop until it completes all pending work. think of this as
> interrupt mitigation.


Yes, that was my solution as well, i don't have access to the code right
now, but I think this won't fix the msleep case guenther pointed out.



>
> it is worth noting that sleep_setup_timeout doesnt timeout_add if
> the wait is 0, so the thread doesnt recurse.
>
> Index: kern/kern_timeout.c
> ===
> RCS file: /cvs/src/sys/kern/kern_timeout.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 kern_timeout.c
> --- kern/kern_timeout.c 22 Sep 2016 12:55:24 -  1.49
> +++ kern/kern_timeout.c 24 Sep 2016 09:45:09 -
> @@ -375,6 +375,7 @@ softclock(void *arg)
> int delta;
> struct circq *bucket;
> struct timeout *to;
> +   int needsproc = 0;
>
> mtx_enter(_mutex);
> while (!CIRCQ_EMPTY(_todo)) {
> @@ -391,7 +392,7 @@ softclock(void *arg)
> CIRCQ_INSERT(>to_list, bucket);
> } else if (to->to_flags & TIMEOUT_NEEDPROCCTX) {
> CIRCQ_INSERT(>to_list, _proc);
> -   wakeup(_proc);
> +   needsproc = 1;
> } else {
>  #ifdef DEBUG
> if (delta < 0)
> @@ -401,6 +402,9 @@ softclock(void *arg)
> }
> }
> mtx_leave(_mutex);
> +
> +   if (needsproc)
> +   wakeup(_proc);
>  }
>
>  void
>

Re: timeout_set_proc(9)

[Christiano F. Haesbaert]

Am Mittwoch, 21. September 2016 schrieb Martin Pieuchot :

> On 21/09/16(Wed) 16:29, David Gwynne wrote:
> > [...]
> > the point i was trying to make was that the existing stuff (tasks,
> timeouts) can be used together to get the effect we want. my point was very
> poorly made though.
> >
> > i think your point is that you can make a clever change to timeouts and
> not have to do a ton of flow on code changes to take advantage of it.
>
> I'm trying to fix one problem at a time.
>
> > [...]
> > if timeouts are the way to schedule stuff to run in the future, then
> we're going to get head-of-line blocking problems. pending timeouts will
> end up stuck behind work that is waiting on an arbitrary lock, because
> there's an implicit single thread that will run them all. right now that is
> mitigated by timeouts being an interrupt context, we just dont put a lot of
> work like that in there right now.
>
> Really?  Is it worth than it is now with the KERNEL_LOCK()?
>
> > the benefit of taskqs is that you explicitly steer work to threads that
> can sleep independently of each other. they lack being able to schedule
> work to run in the future though.
> >
> > it turns out it isnt that hard to make taskqs use a priority queue
> internally instead of a tailq. this allows you to specify that tasks get
> executed in the future (or right now, like the current behaviour) in an
> explicit thread (or pool of threads). it does mean a lot of changes to code
> thats using timeouts now though.
>
> I agree with you, but these thoughts are IMHO too far ahead.  Everything
> is still serialized in our kernel.
>
>
The diff as it is will deadlock against SCHED_LOCK.
tsleep() calls sleep_setup(), which grabs SCHED_LOCK,
Then sleep_setup_timeout() will grab timeout_mutex in timeout_add()

On softclock() you have the opposite:
Grabs timeout_mutex then does a wakeup, which grabs SCHED_LOCK.

Re: Switch mfii(4) to msi, testing required.

[Christiano F. Haesbaert]

On Thu, Jun 06, 2013 at 12:54:31PM +0200, Christiano F. Haesbaert wrote:
 Hi,
 
 We would like to switch mfii(4) to msi, there is a family of supermicro
 X9 motherboards with incorrect ioapic routing, so they only work properly
 though msi.
 
 If you have a system with a mfii(4) device, please give this diff a spin
 and report back.
 
 So far I was able to test on:
 
 Supermicro X9DRH + Symbios Logic MegaRAID SAS2208
 Fujitsu primergy RX300 S7 + Symbios Logic MegaRAID SAS2208
 
 Index: dev/pci/mfii.c
 ===
 RCS file: /cvs/src/sys/dev/pci/mfii.c,v
 retrieving revision 1.12
 diff -d -u -p -r1.12 mfii.c
 --- dev/pci/mfii.c25 Aug 2012 07:03:04 -  1.12
 +++ dev/pci/mfii.c6 Jun 2013 10:47:23 -
 @@ -307,7 +307,7 @@ mfii_attach(struct device *parent, struc
   /* disable interrupts */
   mfii_write(sc, MFI_OMSK, 0x);
  
 - if (pci_intr_map(pa, ih) != 0) {
 + if (pci_intr_map_msi(pa, ih) != 0  pci_intr_map(pa, ih) != 0) {
   printf(: unable to map interrupt\n);
   goto pci_unmap;
   }

So far I've received only one report where this diff makes it possible
to use the Supermicro branded AOC-S2208-H8iR LSI SAS 2208, where the
apic routing is probably busted too.

We're reaching the point where we'll see more machines with wrong apic
routing since most newer chips are only being tested with MSI enabled.
This is one case of it.

I'd like to commit this after release.

Switch mfii(4) to msi, testing required.

[Christiano F. Haesbaert]

Hi,

We would like to switch mfii(4) to msi, there is a family of supermicro
X9 motherboards with incorrect ioapic routing, so they only work properly
though msi.

If you have a system with a mfii(4) device, please give this diff a spin
and report back.

So far I was able to test on:

Supermicro X9DRH + Symbios Logic MegaRAID SAS2208
Fujitsu primergy RX300 S7 + Symbios Logic MegaRAID SAS2208

Index: dev/pci/mfii.c
===
RCS file: /cvs/src/sys/dev/pci/mfii.c,v
retrieving revision 1.12
diff -d -u -p -r1.12 mfii.c
--- dev/pci/mfii.c  25 Aug 2012 07:03:04 -  1.12
+++ dev/pci/mfii.c  6 Jun 2013 10:47:23 -
@@ -307,7 +307,7 @@ mfii_attach(struct device *parent, struc
/* disable interrupts */
mfii_write(sc, MFI_OMSK, 0x);
 
-   if (pci_intr_map(pa, ih) != 0) {
+   if (pci_intr_map_msi(pa, ih) != 0  pci_intr_map(pa, ih) != 0) {
printf(: unable to map interrupt\n);
goto pci_unmap;
}

Re: faithd fcntl diff

[Christiano F. Haesbaert]

On 11 February 2013 07:05, Todd T. Fries t...@fries.net wrote:
 In light of nat64 in pf(4), what purpose does faithd(8) serve anymore?

 I played with it a bit over a decade ago, but don't recall having any use
 for it in the last number of years.

 I vote it gets tedu'ed.


I vote for it too, I remember suggesting it last year.

 Penned by David Hill on 20130209 12:53.51, we have:
 | Anyone want to OK and commit?
 |
 | On Sun, Jan 20, 2013 at 05:19:18PM -0500, David Hill wrote:
 | O_NONBLOCK is set with F_SETFL
 | 
 | Index: usr.sbin/faithd/tcp.c
 | ===
 | RCS file: /cvs/src/usr.sbin/faithd/tcp.c,v
 | retrieving revision 1.12
 | diff -N -u -p usr.sbin/faithd/tcp.c
 | --- usr.sbin/faithd/tcp.c8 Sep 2002 01:20:15 -   1.12
 | +++ usr.sbin/faithd/tcp.c20 Jan 2013 22:17:13 -
 | @@ -206,7 +206,7 @@ relay(int s_rcv, int s_snd, const char *service, int d
 |  FD_ZERO(readfds);
 |  FD_ZERO(writefds);
 |  FD_ZERO(exceptfds);
 | -fcntl(s_snd, F_SETFD, O_NONBLOCK);
 | +fcntl(s_snd, F_SETFL, O_NONBLOCK);
 |  oreadfds = readfds; owritefds = writefds; oexceptfds = exceptfds;
 |  if (s_rcv = FD_SETSIZE)
 |  exit_failure(descriptor too big);
 | 

 --
 Todd Fries .. t...@fries.net

  
 |\  1.636.410.0632 (voice)
 | Free Daemon Consulting, LLC\  1.405.227.9094 (voice)
 | http://FreeDaemonConsulting.com\  1.866.792.3418 (FAX)
 | PO Box 16169, Oklahoma City, OK 73113  \  sip:freedae...@ekiga.net
 | ..in support of free software solutions. \  sip:4052279...@ekiga.net
  \

   37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
 http://todd.fries.net/pgp.txt

Re: profiling kernels

[Christiano F. Haesbaert]

On 14 December 2012 13:40, Mike Belopuhov m...@belopuhov.com wrote:
 On 14 December 2012 13:38, Stuart Henderson s...@spacehopper.org wrote:
 Are profiling kernels known to be broken at the moment?


 profiling has never worked on MP kernels...


Hmm, that answers :)

Re: hostname.if(5) clarification

[Christiano F. Haesbaert]

On 26 November 2012 22:06, Stuart Henderson s...@spacehopper.org wrote:
 On 2012/11/26 17:40, Jason McIntyre wrote:
 anyway...i still dislike the idea of just saying order matters. also,
 could someone really expect the file to not be parsed top down

 Yes, I think they might; people are used to config files being read
 and parsed before being applied, and because this file isn't a straight
 set of commands to pass to ifconfig(8) I think it's not unreasonable
 that someone should treat it as a config file.


I never liked the way hostname.if works, but I also dislike the idea
of a new configuration file.
If we do any change, I'd vote for a script where the user enters all
ifconfig calls explicitly, we can't do this in rc.conf.local since it
is too late on the boot process.

Re: upstream vendors and why they can be really harmful

[Christiano F. Haesbaert]

Lets be honest, half the problem goes away if Lennart stops hacking.

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On Tue, Oct 30, 2012 at 04:44:36PM +0100, rustyBSD wrote:
 Le 30/10/2012 15:32, Christiano F. Haesbaert a ?crit :
  That should be an access(2) call.
 Yes.Something like this - also moved len to size_t,
 as strlen() is size_t:
 
 
 --- dired.cWed Mar 14 14:56:35 2012
 +++ dired.cTue Oct 30 16:23:00 2012
 @@ -724,9 +724,10 @@
  dired_(char *dname)
  {
  struct buffer*bp;
 -int len, i;
 +int i;
 +size_t len;
  
 -if ((fopen(dname,r)) == NULL) {
 +if (access(dname, R_OK) == -1) {
  if (errno == EACCES)
  ewprintf(Permission denied);
  return (NULL);
 

Your diff got mangled, it replaced tabs for spaces, anyway, I think we
should also check for X_OK, otherwise we can't list the directory
anyway. 

We still get the message File is not readable, but I believe it's more
useful than showing an empty directory.

Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.51
diff -d -u -p -r1.51 dired.c
--- dired.c 14 Mar 2012 13:56:35 -  1.51
+++ dired.c 3 Nov 2012 14:47:18 -
@@ -724,9 +724,10 @@ struct buffer *
 dired_(char *dname)
 {
struct buffer   *bp;
-   int  len, i;
+   int  i;
+   size_t   len;
 
-   if ((fopen(dname,r)) == NULL) {
+   if ((access(dname, R_OK | X_OK)) == -1) {
if (errno == EACCES)
ewprintf(Permission denied);
return (NULL);

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On 30 October 2012 14:36, rustyBSD rusty...@gmx.fr wrote:
 MMmhh...

 == /usr/src/usr.bin/mg/dired.c ==
 Go look the line 729:

 if ((fopen(dname,r)) == NULL) {
 ...

 Now you can cry


What is your point ?

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On 30 October 2012 14:58, Christiano F. Haesbaert
haesba...@haesbaert.org wrote:
 On 30 October 2012 14:36, rustyBSD rusty...@gmx.fr wrote:
 MMmhh...

 == /usr/src/usr.bin/mg/dired.c ==
 Go look the line 729:

 if ((fopen(dname,r)) == NULL) {
 ...

 Now you can cry


 What is your point ?

Hmm just noticed there are some calls that do not check the NULL case,
looks like you have a diff to write :).

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On 30 October 2012 15:00, Mike Belopuhov m...@belopuhov.com wrote:
 On Tue, Oct 30, 2012 at 2:58 PM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 14:36, rustyBSD rusty...@gmx.fr wrote:
 MMmhh...

 == /usr/src/usr.bin/mg/dired.c ==
 Go look the line 729:

 if ((fopen(dname,r)) == NULL) {
 ...

 Now you can cry


 What is your point ?


 you leak a FILE object and a descriptor.

Aww jesus, completely missed it !

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On 30 October 2012 15:03, Christiano F. Haesbaert
haesba...@haesbaert.org wrote:
 On 30 October 2012 15:00, Mike Belopuhov m...@belopuhov.com wrote:
 On Tue, Oct 30, 2012 at 2:58 PM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 14:36, rustyBSD rusty...@gmx.fr wrote:
 MMmhh...

 == /usr/src/usr.bin/mg/dired.c ==
 Go look the line 729:

 if ((fopen(dname,r)) == NULL) {
 ...

 Now you can cry


 What is your point ?


 you leak a FILE object and a descriptor.

 Aww jesus, completely missed it !

So that was just to check permission:

==
http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/dired.c.diff?r1=1.45;r2=1.46

From the Loganaden Velvindron:
 Make dired more sane (and emacslike):
 *  Position cursor at first filename after ..
 *  Don't reposition cursor on reopening
 *  Check for permission before attempting to open directory

I took forever to get this in. Thanks, Logan for being patient!
==

That should be an access(2) call.

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On 30 October 2012 16:45, Okan Demirmen o...@demirmen.com wrote:
 On Tue, Oct 30, 2012 at 10:32 AM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 15:03, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 15:00, Mike Belopuhov m...@belopuhov.com wrote:
 On Tue, Oct 30, 2012 at 2:58 PM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 14:36, rustyBSD rusty...@gmx.fr wrote:
 MMmhh...

 == /usr/src/usr.bin/mg/dired.c ==
 Go look the line 729:

 if ((fopen(dname,r)) == NULL) {
 ...

 Now you can cry


 What is your point ?


 you leak a FILE object and a descriptor.

 Aww jesus, completely missed it !

 So that was just to check permission:

 ==
 http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/dired.c.diff?r1=1.45;r2=1.46

 From the Loganaden Velvindron:
  Make dired more sane (and emacslike):
  *  Position cursor at first filename after ..
  *  Don't reposition cursor on reopening
  *  Check for permission before attempting to open directory

 I took forever to get this in. Thanks, Logan for being patient!
 ==

 That should be an access(2) call.


 or stat(2) due to tctu.

I believe in that case it would be the same, since there is still a
window between stat(2)/access(2) and open(2).

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On 30 October 2012 16:52, Christiano F. Haesbaert
haesba...@haesbaert.org wrote:
 On 30 October 2012 16:45, Okan Demirmen o...@demirmen.com wrote:
 On Tue, Oct 30, 2012 at 10:32 AM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 15:03, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 15:00, Mike Belopuhov m...@belopuhov.com wrote:
 On Tue, Oct 30, 2012 at 2:58 PM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 14:36, rustyBSD rusty...@gmx.fr wrote:
 MMmhh...

 == /usr/src/usr.bin/mg/dired.c ==
 Go look the line 729:

 if ((fopen(dname,r)) == NULL) {
 ...

 Now you can cry


 What is your point ?


 you leak a FILE object and a descriptor.

 Aww jesus, completely missed it !

 So that was just to check permission:

 ==
 http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/dired.c.diff?r1=1.45;r2=1.46

 From the Loganaden Velvindron:
  Make dired more sane (and emacslike):
  *  Position cursor at first filename after ..
  *  Don't reposition cursor on reopening
  *  Check for permission before attempting to open directory

 I took forever to get this in. Thanks, Logan for being patient!
 ==

 That should be an access(2) call.


 or stat(2) due to tctu.

 I believe in that case it would be the same, since there is still a
 window between stat(2)/access(2) and open(2).

I mean, considering he would open/stat/close and open again.

Re: Goodbye to you my file descriptor

[Christiano F. Haesbaert]

On 30 October 2012 16:57, Okan Demirmen o...@demirmen.com wrote:
 On Tue, Oct 30, 2012 at 11:53 AM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 16:52, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 16:45, Okan Demirmen o...@demirmen.com wrote:
 On Tue, Oct 30, 2012 at 10:32 AM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 15:03, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 15:00, Mike Belopuhov m...@belopuhov.com wrote:
 On Tue, Oct 30, 2012 at 2:58 PM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 30 October 2012 14:36, rustyBSD rusty...@gmx.fr wrote:
 MMmhh...

 == /usr/src/usr.bin/mg/dired.c ==
 Go look the line 729:

 if ((fopen(dname,r)) == NULL) {
 ...

 Now you can cry


 What is your point ?


 you leak a FILE object and a descriptor.

 Aww jesus, completely missed it !

 So that was just to check permission:

 ==
 http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/dired.c.diff?r1=1.45;r2=1.46

 From the Loganaden Velvindron:
  Make dired more sane (and emacslike):
  *  Position cursor at first filename after ..
  *  Don't reposition cursor on reopening
  *  Check for permission before attempting to open directory

 I took forever to get this in. Thanks, Logan for being patient!
 ==

 That should be an access(2) call.


 or stat(2) due to tctu.

 I believe in that case it would be the same, since there is still a
 window between stat(2)/access(2) and open(2).

 I mean, considering he would open/stat/close and open again.

 I didn't actually look at the code; I just noticed the words
 permission and access(2) and hit reply :)

 Cheers.

Today is definately not my day, forgot that stat takes a path and not an fd.

Re: mg: revert-buffer

[Christiano F. Haesbaert]

I want this baadlyyy, ill have a look.
On Oct 12, 2012 12:30 PM, Jasper Lievisse Adriaanse jas...@openbsd.org
wrote:

 Hi,

 Here's a diff that implement revert-buffer (C-x r). I've split gotoline
 into
 the 'goto-line' specifics and the code that actually jumps to the line so
 it
 can be re-used by revert-buffer to restore the current line.

 OK?

 --
 Cheers,
 Jasper

 Stay Hungry. Stay Foolish

 Index: basic.c
 ===
 RCS file: /cvs/src/usr.bin/mg/basic.c,v
 retrieving revision 1.37
 diff -p -u -r1.37 basic.c
 --- basic.c 18 Jun 2012 09:26:03 -  1.37
 +++ basic.c 12 Oct 2012 10:25:14 -
 @@ -485,7 +485,6 @@ swapmark(int f, int n)
  int
  gotoline(int f, int n)
  {
 -   struct line  *clp;
 char   buf[32], *bufp;
 const char *err;

 @@ -501,6 +500,16 @@ gotoline(int f, int n)
 return (FALSE);
 }
 }
 +   return(setlineno(n));
 +}
 +
 +/*
 + * Set the line number and switch to it.
 + */
 +int
 +setlineno(int n){
 +   struct line  *clp;
 +
 if (n = 0) {
 if (n == 0)
 n++;
 Index: buffer.c
 ===
 RCS file: /cvs/src/usr.bin/mg/buffer.c,v
 retrieving revision 1.81
 diff -p -u -r1.81 buffer.c
 --- buffer.c31 Aug 2012 18:06:42 -  1.81
 +++ buffer.c12 Oct 2012 10:25:14 -
 @@ -861,4 +861,35 @@ checkdirty(struct buffer *bp)

 return (TRUE);
  }
 -
 +
 +/*
 + * Revert the current buffer to whatever is on disk.
 + */
 +/* ARGSUSED */
 +int
 +revertbuffer(int f, int n){
 +   struct mgwin *wp = wheadp;
 +   struct buffer *bp = wp-w_bufp;
 +   char fbuf[NFILEN + 32];
 +   int lineno;
 +
 +   if (!strlen(bp-b_fname)) {
 +   ewprintf(Cannot revert buffer not associated with any
 files.);
 +   return (FALSE);
 +   }
 +
 +   snprintf(fbuf, sizeof(fbuf), Revert buffer from file %s,
 bp-b_fname);
 +
 +   if (eyorn(fbuf)) {
 +   /* Save our current line, so we can go back it after
 reloading the file. */
 +   lineno = wp-w_dotline;
 +   if (readin(bp-b_list.l_name)) {
 +   return(setlineno(lineno));
 +   } else {
 +   ewprintf(File %s no longer readable!,
 bp-b_fname);
 +   return (FALSE);
 +   }
 +   }
 +
 +   return (FALSE);
 +}
 Index: def.h
 ===
 RCS file: /cvs/src/usr.bin/mg/def.h,v
 retrieving revision 1.125
 diff -p -u -r1.125 def.h
 --- def.h   31 Aug 2012 18:06:42 -  1.125
 +++ def.h   12 Oct 2012 10:25:14 -
 @@ -414,6 +414,7 @@ int  notmodified(int, int);
  int popbuftop(struct buffer *, int);
  int getbufcwd(char *, size_t);
  int checkdirty(struct buffer *);
 +int revertbuffer(int, int);

  /* display.c */
  intvtresize(int, int, int);
 @@ -494,6 +495,7 @@ int  setmark(int, int);
  int clearmark(int, int);
  int swapmark(int, int);
  int gotoline(int, int);
 +int setlineno(int);

  /* random.c X */
  int showcpos(int, int);
 Index: funmap.c
 ===
 RCS file: /cvs/src/usr.bin/mg/funmap.c,v
 retrieving revision 1.40
 diff -p -u -r1.40 funmap.c
 --- funmap.c14 Jun 2012 17:21:22 -  1.40
 +++ funmap.c12 Oct 2012 10:25:15 -
 @@ -196,7 +196,8 @@ static struct funmap functnames[] = {
 {csprevmatch, cscope-prev-symbol,},
 {csnextfile, cscope-next-file,},
 {csprevfile, cscope-prev-file,},
 -   {cscreatelist, cscope-create-list-of-files-to-index},
 +   {cscreatelist, cscope-create-list-of-files-to-index,},
 +   {revertbuffer, revert-buffer,},
 {NULL, NULL,}
  };

 Index: keymap.c
 ===
 RCS file: /cvs/src/usr.bin/mg/keymap.c,v
 retrieving revision 1.50
 diff -p -u -r1.50 keymap.c
 --- keymap.c7 Jun 2012 15:15:04 -   1.50
 +++ keymap.c12 Oct 2012 10:25:15 -
 @@ -177,7 +177,7 @@ static PF cXcar[] = {
 nextwind,   /* o */
 prevwind,   /* p */
 rescan, /* q */
 -   rescan, /* r */
 +   revertbuffer,   /* r */
 savebuffers,/* s */
 rescan, /* t */
 undo/* u */
 Index: mg.1
 ===
 RCS file: /cvs/src/usr.bin/mg/mg.1,v
 retrieving revision 1.68
 diff -p -u -r1.68 mg.1
 --- mg.111 Jul 2012 19:56:13 -  1.68
 +++ mg.112 Oct 2012 10:25:15 -
 @@ -249,6 +249,8 @@ other-window
  other-window

Re: ftp mput recursiv upload diff for testing

[Christiano F. Haesbaert]

ok haesbaert, sorry for the slacking :=).

On Sat, Aug 11, 2012 at 05:48:13PM +0200, Alexander Bluhm wrote:
 On Mon, Jul 30, 2012 at 10:30:47AM +0200, Jan Klemkow wrote:
  Hopefully the final version.
 
 Yes, I think this is OK now.  If anybody else could have a look at
 it, I will commit it.
 
 bluhm
 
  
  Index: cmds.c
  ===
  RCS file: /cvs/src/usr.bin/ftp/cmds.c,v
  retrieving revision 1.70
  diff -u -p -r1.70 cmds.c
  --- cmds.c  5 May 2009 19:35:30 -   1.70
  +++ cmds.c  30 Jul 2012 07:58:16 -
  @@ -231,15 +231,32 @@ mput(int argc, char *argv[])
  extern int optind, optreset;
  int ch, i, restartit = 0;
  sig_t oldintr;
  -   char *cmd, *tp;
  +   char *cmd, *tp, *xargv[] = { argv[0], NULL, NULL };
  +   const char *errstr;
  +   static int depth = 0, max_depth = 0;
   
  optind = optreset = 1;
   
  -   while ((ch = getopt(argc, argv, c)) != -1) {
  +   if (depth)
  +   depth++;
  +
  +   while ((ch = getopt(argc, argv, cd:r)) != -1) {
  switch(ch) {
  case 'c':
  restartit = 1;
  break;
  +   case 'd':
  +   max_depth = strtonum(optarg, 0, INT_MAX, errstr);
  +   if (errstr != NULL) {
  +   fprintf(ttyout, bad depth value, %s: %s\n,
  +   errstr, optarg);
  +   code = -1;
  +   return;
  +   }
  +   break;
  +   case 'r':
  +   depth = 1;
  +   break;
  default:
  goto usage;
  }
  @@ -247,7 +264,8 @@ mput(int argc, char *argv[])
   
  if (argc - optind  1  !another(argc, argv, local-files)) {
   usage:
  -   fprintf(ttyout, usage: %s [-c] local-files\n, argv[0]);
  +   fprintf(ttyout, usage: %s [-cr] [-d depth] local-files\n,
  +   argv[0]);
  code = -1;
  return;
  }
  @@ -318,11 +336,13 @@ usage:
  mflag = 0;
  return;
  }
  +
  for (i = 1; i  argc; i++) {
  char **cpp;
  glob_t gl;
  int flags;
   
  +   /* Copy files without word expansion */
  if (!doglob) {
  if (mflag  confirm(argv[0], argv[i])) {
  tp = (ntflag) ? dotrans(argv[i]) : argv[i];
  @@ -348,6 +368,7 @@ usage:
  continue;
  }
   
  +   /* expanding file names */
  memset(gl, 0, sizeof(gl));
  flags = GLOB_BRACE|GLOB_NOCHECK|GLOB_QUOTE|GLOB_TILDE;
  if (glob(argv[i], flags, NULL, gl) || gl.gl_pathc == 0) {
  @@ -355,33 +376,89 @@ usage:
  globfree(gl);
  continue;
  }
  +
  +   /* traverse all expanded file names */
  for (cpp = gl.gl_pathv; cpp  *cpp != NULL; cpp++) {
  -   if (mflag  confirm(argv[0], *cpp)) {
  -   tp = (ntflag) ? dotrans(*cpp) : *cpp;
  -   tp = (mapflag) ? domap(tp) : tp;
  -   if (restartit == 1) {
  -   off_t ret;
  +   struct stat filestat;
   
  -   if (curtype != type)
  -   changetype(type, 0);
  -   ret = remotesize(tp, 0);
  -   restart_point = (ret  0) ? 0 : ret;
  -   }
  -   cmd = restartit ? APPE : ((sunique) ?
  -   STOU : STOR);
  -   sendrequest(cmd, *cpp, tp,
  -   *cpp != tp || !interactive);
  -   restart_point = 0;
  -   if (!mflag  fromatty) {
  -   if (confirm(argv[0], NULL))
  -   mflag = 1;
  +   if (!mflag)
  +   continue;
  +   if (stat(*cpp, filestat) != 0) {
  +   warn(local: %s, *cpp);
  +   continue;
  +   }
  +   if (S_ISDIR(filestat.st_mode)  depth == max_depth)
  +   continue;
  +   if (!confirm(argv[0], *cpp))
  +   continue;
  +
  +   /*
  +* If file is a directory then create a new one
  +* at the remote machine.
  +*/
  +   if (S_ISDIR(filestat.st_mode)) {
  +   xargv[1] = *cpp;
  +   makedir(2, xargv);
  +   cd(2, xargv);
  +   if 

Re: Scheduler improvements

[Christiano F. Haesbaert]

On 8 October 2012 11:24, Marc Espie es...@nerim.net wrote:
 log2 should probably be scrapped, since you're reinventing ffs.


That is actually my code, back then I failed at finding an ffs.

I'll try to have a look at the code this week.

Re: Scheduler improvements

[Christiano F. Haesbaert]

On Oct 8, 2012 6:33 PM, Alexandre Ratchov a...@caoua.org wrote:

 On Sat, Oct 06, 2012 at 10:38:34PM +0200, Gregor Best wrote:
  Hi Alexandre,
 
   [...]
   This change is unclear for me; AFAIU, it removes the mechanism
   which makes processes wake up with a priority depending on what
   they are blocked on.
   [...]
 
  Where do you see that? The code I removed/changed simply calulated the
queue from
  which to remove `p` and removed it from there (similar for insertion).
That needed
  to be changed to modify the RB tree used as a new runqueue.
 
   [...]
   For instance processes waking up from poll(2) or audio read/write
   won't be prioritized any longer. If so, this would hurt audio and
   other interactive processes but would improve cpu-intesive
   bloatware.
   [...]
 
  They weren't prioritised with the old version of this code either.
 

 AFAIU, when a process waits for an event (with tsleep(9)), it
 provides the priority of the event it's waiting for (eg an audio
 interrupt). When the event occurs, the process is put in the
 runnable queue calculated from the priority. This way, interactive
 processes (i.e. waiting for external events) are prioritized during
 the event processing.

 This mechanism ensures audio and midi are usable without having to
 manually tweak priorities. Furthermore, priorities changed
 depending on what the process is doing.

 The most notable exception to above rule is the poll(2)
 implementation, which still ignores priorities; it may need to be
 fixed one day, but this is not the most urgent problem hurting
 interactive processes.
 [A
 -- Alexandre


Ratchov is correct, when a process sleeps you specify the priority you
should have when waking up. I think george was referring to the old version
of his code.

The 4.4 bsd scheduler relies heavily on these priority boosts, i still
didnt have a look at George's code, but an equivalent mechanism is usually
needed. On cfs scheduling for example the processes forces a new delta
calculation and gets a fraction proportional to all other runnables
processes.

Re: gem(4): simplify gem_attach_pci() variant detection code a bit

[Christiano F. Haesbaert]

I prefer a switch too, it shows you acknowledged those variants, anyhow i
think the current idiom is bad and should be changed.

Re: gem(4): simplify gem_attach_pci() variant detection code a bit

[Christiano F. Haesbaert]

On Fri, Sep 28, 2012 at 02:42:18AM -0400, Brad Smith wrote:
 On Wed, Sep 26, 2012 at 03:32:37PM -0400, Brad Smith wrote:
  Simplify the gem(4) variant detection code a bit.
  
  OK?
 
 How about this..
 
 
 Index: if_gem_pci.c
 ===
 RCS file: /home/cvs/src/sys/dev/pci/if_gem_pci.c,v
 retrieving revision 1.32
 diff -u -p -r1.32 if_gem_pci.c
 --- if_gem_pci.c  3 Apr 2011 15:36:02 -   1.32
 +++ if_gem_pci.c  28 Sep 2012 05:16:00 -
 @@ -227,22 +227,19 @@ gem_attach_pci(struct device *parent, st
  
   sc-sc_pci = 1; /* X should all be done in bus_dma. */
  
 - if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_GEMNETWORK)
 + switch (PCI_PRODUCT(pa-pa_id)) {
 + case PCI_PRODUCT_SUN_GEMNETWORK:
   sc-sc_variant = GEM_SUN_GEM;
 - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_ERINETWORK)
 + break;
 + case PCI_PRODUCT_SUN_ERINETWORK:
   sc-sc_variant = GEM_SUN_ERI;
 - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC)
 - sc-sc_variant = GEM_APPLE_GMAC;
 - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC)
 - sc-sc_variant = GEM_APPLE_GMAC;
 - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC)
 - sc-sc_variant = GEM_APPLE_GMAC;
 - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC)
 - sc-sc_variant = GEM_APPLE_GMAC;
 - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC)
 - sc-sc_variant = GEM_APPLE_GMAC;
 - else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_K2_GMAC)
 + break;
 + case PCI_PRODUCT_APPLE_K2_GMAC:
   sc-sc_variant = GEM_APPLE_K2_GMAC;
 + break;
 + default:
 + sc-sc_variant = GEM_APPLE_GMAC;
 + }
  
  #define PCI_GEM_BASEADDR 0x10
   if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0,
 

Ok by me, but when I said acknowledge I meant this, I'm ok with either,
if kettenis doesn't mind :=).


Index: if_gem_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_gem_pci.c,v
retrieving revision 1.32
diff -d -u -p -r1.32 if_gem_pci.c
--- if_gem_pci.c3 Apr 2011 15:36:02 -   1.32
+++ if_gem_pci.c28 Sep 2012 07:26:23 -
@@ -227,22 +227,27 @@ gem_attach_pci(struct device *parent, st
 
sc-sc_pci = 1; /* X should all be done in bus_dma. */
 
-   if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_GEMNETWORK)
+   switch (PCI_PRODUCT(pa-pa_id)) {
+   case PCI_PRODUCT_SUN_GEMNETWORK:
sc-sc_variant = GEM_SUN_GEM;
-   else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_ERINETWORK)
+   break;
+   case PCI_PRODUCT_SUN_ERINETWORK:
sc-sc_variant = GEM_SUN_ERI;
-   else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC)
-   sc-sc_variant = GEM_APPLE_GMAC;
-   else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC)
-   sc-sc_variant = GEM_APPLE_GMAC;
-   else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC)
-   sc-sc_variant = GEM_APPLE_GMAC;
-   else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC)
-   sc-sc_variant = GEM_APPLE_GMAC;
-   else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC)
-   sc-sc_variant = GEM_APPLE_GMAC;
-   else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_K2_GMAC)
+   break;
+   case PCI_PRODUCT_APPLE_K2_GMAC:
sc-sc_variant = GEM_APPLE_K2_GMAC;
+   break;
+   case PCI_PRODUCT_APPLE_INTREPID2_GMAC:
+   case PCI_PRODUCT_APPLE_PANGEA_GMAC:
+   case PCI_PRODUCT_APPLE_SHASTA_GMAC:
+   case PCI_PRODUCT_APPLE_UNINORTHGMAC:
+   case PCI_PRODUCT_APPLE_UNINORTH2GMAC:
+   sc-sc_variant = GEM_APPLE_GMAC;
+   break;
+   default:
+   printf(: unknown variant 0x%x\n, sc-sc_variant);
+   return;
+   }
 
 #define PCI_GEM_BASEADDR   0x10
if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0,

Re: tcp ping

[Christiano F. Haesbaert]

On 14 September 2012 11:39, Stuart Henderson s...@spacehopper.org wrote:
 On 2012/09/13 13:54, Matthew Dempsky wrote:
 On Thu, Sep 13, 2012 at 12:02 PM, Ted Unangst t...@tedunangst.com wrote:
  This adds a -T portnum option to ping.  I haven't polished the output
  because I'm not sure if this is desirable or not, but I found it
  useful.  If it's not a hell no, never in base I can finish it up some.

 Is there precedent for having TCP-based ping in any other OS's ping(1)
 utility?  It seems like adding a separate tcpping(1) utility and
 leaving ping(1) for ICMP would be more appropriate.

 traceroute allows setting the protocol (though this doesn't work for
 tcp yet ;) so I think it would make some sort of  sense to do this in
 ping/ping6.


I agree, if this is getting in, it should be on ping :).

Re: fix sasyncd(8) race condition causing a segfault when getting data from the kernel

[Christiano F. Haesbaert]

I've reviewed this diff with Patrick and I believe it is correct, can
someone else have a look ?


On Tue, Sep 04, 2012 at 04:46:08PM +0200, Patrick Wildt wrote:
 When sasyncd(8) dumps data from the kernel, it does a sysctl() for the size to
 alloc,
 and a second for the actual data. In between, new data can be added/deleted.
 Therefore
 the kernel might return less data, than actually told. The case that you get
 less/more data
 isn't handled properly.
 
 In case of less data, it might happen that the child is working on
 uninitialized data.
 This might lead to a segmentation fault.
 In case of more data, the sasyncd(8) parent process handles it as he didn't
 get any data.
 
 This diff rewrites that part, so that we try thrice to get the SAs and SPDs.
 If getting the SAs
 breaks thrice, we tell the child process, that there's no data. Same for the
 SPDs.
 
 The case of less data is now handled properly. The case of more data is either
 caught
 by the for loop, or allocing twice the size of the data we should originally
 get.
 That might be helpful on bigger vpn setups, when the loop still doesn't catch
 the race.
 
 Tested with 7080 SAs. (For real.)
 
 Index: monitor.c
 ===
 RCS file: /cvs/src/usr.sbin/sasyncd/monitor.c,v
 retrieving revision 1.15
 diff -u -r1.15 monitor.c
 --- monitor.c 2 Apr 2012 18:56:15 -   1.15
 +++ monitor.c 4 Sep 2012 13:06:37 -
 @@ -342,7 +342,7 @@
  {
   u_int8_t*sadb_buf = 0, *spd_buf = 0;
   size_t   sadb_buflen = 0, spd_buflen = 0, sz;
 - int  mib[5];
 + int  i, mib[5];
   u_int32_tv;
 
   mib[0] = CTL_NET;
 @@ -352,59 +352,81 @@
   mib[4] = 0; /* Unspec SA type */
 
   /* First, fetch SADB data */
 - if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, sz, NULL, 0) == -1
 - || sz == 0) {
 - sadb_buflen = 0;
 - goto try_spd;
 - }
 -
 - sadb_buflen = sz;
 - if ((sadb_buf = malloc(sadb_buflen)) == NULL) {
 - log_err(m_priv_pfkey_snap: malloc);
 - sadb_buflen = 0;
 - goto out;
 - }
 -
 - if (sysctl(mib, sizeof mib / sizeof mib[0], sadb_buf, sz, NULL, 0)
 - == -1) {
 - log_err(m_priv_pfkey_snap: sysctl);
 - sadb_buflen = 0;
 - goto out;
 + for (i = 0; i  3; i++) {
 + if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, sz, NULL, 0)
 + == -1)
 + break;
 +
 + if (!sz)
 + break;
 +
 + /* Try to catch newly added data */
 + sz *= 2;
 +
 + if ((sadb_buf = malloc(sz)) == NULL)
 + break;
 +
 + if (sysctl(mib, sizeof mib / sizeof mib[0], sadb_buf, sz, 
 NULL, 0)
 + == -1) {
 + free(sadb_buf);
 + sadb_buf = 0;
 + /*
 +  * If new SAs were added meanwhile and the given buffer 
 is
 +  * too small, retry.
 +  */
 + if (errno == ENOMEM)
 + continue;
 + break;
 + }
 +
 + sadb_buflen = sz;
 + break;
   }
 
   /* Next, fetch SPD data */
 -  try_spd:
   mib[3] = NET_KEY_SPD_DUMP;
 
 - if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, sz, NULL, 0) == -1
 - || sz == 0) {
 - spd_buflen = 0;
 - goto out;
 - }
 + for (i = 0; i  3; i++) {
 + if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, sz, NULL, 0)
 + == -1)
 + break;
 
 - spd_buflen = sz;
 - if ((spd_buf = malloc(spd_buflen)) == NULL) {
 - log_err(m_priv_pfkey_snap: malloc);
 - spd_buflen = 0;
 - goto out;
 - }
 + if (!sz)
 + break;
 +
 + /* Try to catch newly added data */
 + sz *= 2;
 +
 + if ((spd_buf = malloc(sz)) == NULL)
 + break;
 +
 + if (sysctl(mib, sizeof mib / sizeof mib[0], spd_buf, sz, NULL, 
 0)
 + == -1) {
 + free(spd_buf);
 + spd_buf = 0;
 + /*
 +  * If new SPDs were added meanwhile and the given 
 buffer is
 +  * too small, retry.
 +  */
 + if (errno == ENOMEM)
 + continue;
 + break;
 + }
 
 - if (sysctl(mib, sizeof mib / sizeof mib[0], spd_buf, sz, NULL, 0)
 - == -1) {
 - log_err(m_priv_pfkey_snap: sysctl);
 - spd_buflen = 0;
 - goto out;
 + spd_buflen = sz;
 + break;
   }
 
 -  out:
   /* Return SADB data */
   v = 

Re: System is halted while installing on IBM x 3550 M3 server

[Christiano F. Haesbaert]

On 19 August 2012 08:52,  mu...@nitrkl.ac.in wrote:
 Dear all,

 I am using openbsd 5.1 i386 arch. While installing openbsd5.1 i get these
 error message.

 acpi0 at bios0:rev2uvm_fault (0xd07ea4,.)
 fatal page_fault (6) in supervisor mode
 trap type 6 code 0eipd02ef7ba cs 8 eflage 10006 cr2 4e4553440 cpl 90
 panic trap type 6 code=0,pc=d02ef7ba
 The operating system is halted
 Press any key to reboot.

 I have also tried amd64 arch. but it is not detecting SAS based RAID
 Driver. That why server HDD is not detected.

 Any help would be appreciable.

 Best Regards


Please paste the full dmesg, up to the panic.



 -
 This email was sent using the NIT Rourkela Webmail.
 Nation Institute of Technology - Rourkela
 http://www.nitrkl.ac.in
 This mail has been scanned by Cyberoam based UTM at NIT, Rourkela. If your 
 mail still contains virus forward it to s...@nitrkl.ac.in

Re: add WARNINGS infrastructure to ftp

[Christiano F. Haesbaert]

Lets make it a bit cleaner, while here also fix some whitespaces.

Index: bsd.own.mk
===
RCS file: /cvs/src/share/mk/bsd.own.mk,v
retrieving revision 1.116
diff -d -u -p -r1.116 bsd.own.mk
--- bsd.own.mk  12 Apr 2012 11:22:14 -  1.116
+++ bsd.own.mk  13 Aug 2012 20:14:07 -
@@ -79,10 +79,10 @@ LIBGRP?=${BINGRP}
 LIBOWN?=   ${BINOWN}
 LIBMODE?=  ${NONBINMODE}
 
-DOCDIR?=/usr/share/doc
+DOCDIR?=   /usr/share/doc
 DOCGRP?=   bin
 DOCOWN?=   root
-DOCMODE?=   ${NONBINMODE}
+DOCMODE?=  ${NONBINMODE}
 
 LKMDIR?=   /usr/lkm
 LKMGRP?=   ${BINGRP}
@@ -98,6 +98,12 @@ LOCALEDIR?=  /usr/share/locale
 LOCALEGRP?=wheel
 LOCALEOWN?=root
 LOCALEMODE?=   ${NONBINMODE}
+
+.if !defined(CDIAGFLAGS)
+CDIAGFLAGS=-Wall -Wpointer-arith -Wuninitialized -Wstrict-prototypes
+CDIAGFLAGS+=   -Wmissing-prototypes -Wunused -Wsign-compare -Wbounded
+CDIAGFLAGS+=   -Wshadow
+.endif
 
 # Shared files for system gnu configure, not used yet
 GNUSYSTEM_AUX_DIR?=${BSDSRCDIR}/share/gnu

Re: (2) HTTP basic authentication for ftp(1) (RFC 2617)

[Christiano F. Haesbaert]

On Mon, Aug 13, 2012 at 03:02:22AM +0200, Alexander Hall wrote:
 On 08/06/12 22:56, Christiano F. Haesbaert wrote:
 Please ignore the other thread, it takes ages for me to open my sent box
 over gprs, so I'm opening a new one.
 
 
 Index: fetch.c
 ===
 RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
 retrieving revision 1.105
 diff -d -u -p -r1.105 fetch.c
 --- fetch.c  30 Apr 2012 13:41:26 -  1.105
 +++ fetch.c  6 Aug 2012 20:49:51 -
 @@ -177,7 +177,7 @@ url_get(const char *origline, const char
   {
  char pbuf[NI_MAXSERV], hbuf[NI_MAXHOST], *cp, *portnum, *path, 
  ststr[4];
  char *hosttail, *cause = unknown, *newline, *host, *port, *buf = 
  NULL;
 -char *epath, *redirurl, *loctail;
 +char *epath, *redirurl, *loctail, *h, *p;
  int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
  struct addrinfo hints, *res0, *res, *ares = NULL;
  const char * volatile savefile;
 @@ -191,7 +191,7 @@ url_get(const char *origline, const char
  size_t len, wlen;
   #ifndef SMALL
  char *sslpath = NULL, *sslhost = NULL;
 -char *locbase, *full_host = NULL;
 +char *locbase, *full_host = NULL, *auth = NULL;
  const char *scheme;
  int ishttpsurl = 0;
  SSL_CTX *ssl_ctx = NULL;
 @@ -228,7 +228,20 @@ url_get(const char *origline, const char
   #endif /* !SMALL */
  } else
  errx(1, url_get: Invalid URL '%s', newline);
 -
 +#ifndef SMALL
 +/* Look for auth header */
 

Hey halex thanks for the feedback :)

 - Is this safe for urls like http://some.host/path/e:mail@me/; ?
 - Should it be?
 - Should it maybe be done on the host part after the
   host/path separation?
 
 Not sure what the rfc's says on this. Chrome seems to accept the path I 
 wrote above though.

Hmm good point, I assumed @ was not valid on an url path, I'll check if
the rfc says something, anyway, doing it after we separated the path
seems like a better solution.

 
 +if (proxyenv == NULL 
 +(!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
 +if ((p = strchr(host, '@')) != NULL) {
 +*p = 0; /* Kill @ */
 +if ((auth = calloc(1, 64)) == NULL)
 
 Why not malloc?
 
 Also, if you allocate it at runtime, why fixed at 64? Why not make it 
 sth like
 
   malloc((strlen(host) + 2) * 4 / 3)
 
 (well, or)
 
   calloc(4, (strlen(host) + 2) / 3)
 
 ?

This makes sense, lteo did some digging and it is what freebsd and
netbsd do. If I remember correctly I tried to find something about it on
the RFC.

 
 +err(1, Can't allocate memory for 
 authorization);
 +if (b64_ntop(host, strlen(host), auth, 64) == -1)
 (with appropriate fixing here  ^^)
 
 +errx(1, error in base64 encoding);
 +host = p + 1;
 +}
 +}
 +#endif /* SMALL */
  if (isfileurl) {
  path = host;
  } else {
 @@ -639,14 +652,19 @@ again:
  restart_point = 0;
  }
   #endif /* !SMALL */
 -ftp_printf(fin, ssl, GET /%s %s\r\nHost: , epath,
   #ifndef SMALL
 
 The above leaves the pretty useless residue of:
 
 #endif /* !SMALL */
 #ifndef SMALL
 
 Is this intentional?

Whoops, no, good catch.

 
 -restart_point ? HTTP/1.1\r\nConnection: close :
 -#endif /* !SMALL */
 -HTTP/1.0);
 +if (auth) {
 +ftp_printf(fin, ssl,
 +GET /%s %s\r\nAuthorization: Basic %s\r\nHost: 
 ,
 +epath, restart_point ?
 +HTTP/1.1\r\nConnection: close : HTTP/1.0,
 +auth);
 +free(auth);
 +} else
 +#endif  /* SMALL */
 +ftp_printf(fin, ssl, GET /%s %s\r\nHost: , epath,
 +HTTP/1.0);
  if (strchr(host, ':')) {
 -char *h, *p;
 -
  /*
   * strip off scoped address portion, since it's
   * local to node
 Index: ftp.1
 ===
 RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
 retrieving revision 1.82
 diff -d -u -p -r1.82 ftp.1
 --- ftp.130 Apr 2012 13:41:26 -  1.82
 +++ ftp.16 Aug 2012 20:22:14 -
 @@ -61,7 +61,8 @@
   .Op Fl o Ar output
   .Op Fl s Ar srcaddr
   .Sm off
 -.No http:// Ar host Oo : Ar port
 +.No http:// Oo Ar user : password No @
 +.Oc Ar host Oo : Ar port
   .Oc No / Ar file
   .Sm on
   .Ar ...
 @@ -71,7 +72,8 @@
   .Op Fl o Ar output
   .Op Fl s Ar srcaddr
   .Sm off
 -.No https:// Ar host Oo : Ar port
 +.No https:// Oo Ar user : password No @
 +.Oc Ar host Oo : Ar port
   .Oc No / Ar file
   .Sm on
   .Ar ...
 @@ -1278,12 +1280,12 @@ isn't defined, log in as
   .Ar user
   with a password

Re: (2) HTTP basic authentication for ftp(1) (RFC 2617)

[Christiano F. Haesbaert]

I think I've hunt this down

http://tools.ietf.org/html/rfc3986#section-3.3

If you follow the BNF for path, you have.

path - path-absolute - segment-nz - 1*pchar - \ 
unreserved / pct-encoded / sub-delims / ':' / '@'.

So it seems everything is allowed on path, I'll fix the diff.

Re: (2) HTTP basic authentication for ftp(1) (RFC 2617)

[Christiano F. Haesbaert]

On Mon, Aug 13, 2012 at 04:33:40PM +0200, Christiano F. Haesbaert wrote:
 I think I've hunt this down
 
 http://tools.ietf.org/html/rfc3986#section-3.3
 
 If you follow the BNF for path, you have.
 
 path - path-absolute - segment-nz - 1*pchar - \ 
   unreserved / pct-encoded / sub-delims / ':' / '@'.
 
 So it seems everything is allowed on path, I'll fix the diff.
 

So this addresses halex points, I tested on urls with @ and :, works
fine. I still calloc and I use an extra byte since the auth string can
be displayed on debug. 

Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.105
diff -d -u -p -r1.105 fetch.c
--- fetch.c 30 Apr 2012 13:41:26 -  1.105
+++ fetch.c 13 Aug 2012 15:33:29 -
@@ -177,7 +177,7 @@ url_get(const char *origline, const char
 {
char pbuf[NI_MAXSERV], hbuf[NI_MAXHOST], *cp, *portnum, *path, ststr[4];
char *hosttail, *cause = unknown, *newline, *host, *port, *buf = NULL;
-   char *epath, *redirurl, *loctail;
+   char *epath, *redirurl, *loctail, *h, *p;
int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
struct addrinfo hints, *res0, *res, *ares = NULL;
const char * volatile savefile;
@@ -191,7 +191,7 @@ url_get(const char *origline, const char
size_t len, wlen;
 #ifndef SMALL
char *sslpath = NULL, *sslhost = NULL;
-   char *locbase, *full_host = NULL;
+   char *locbase, *full_host = NULL, *auth = NULL;
const char *scheme;
int ishttpsurl = 0;
SSL_CTX *ssl_ctx = NULL;
@@ -250,6 +250,28 @@ url_get(const char *origline, const char
warnx(No filename after host (use -o): %s, origline);
goto cleanup_url_get;
}
+#ifndef SMALL
+   /*
+* Look for auth header in host, since now host does not
+* contain the path. Basic auth from RFC 2617, valid
+* characters for path are in RFC 3986 section 3.3.
+*/
+   if (proxyenv == NULL 
+   (!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
+   if ((p = strchr(host, '@')) != NULL) {
+   size_t authlen = (strlen(host) + 2) * 4 / 3 + 1;
+
+   *p = 0; /* Kill @ */
+   if ((auth = calloc(1, authlen)) == NULL)
+   err(1, Can't allocate memory for 
+   authorization);
+   if (b64_ntop(host, strlen(host),
+   auth, authlen) == -1)
+   errx(1, error in base64 encoding);
+   host = p + 1;
+   }
+   }
+#endif /* SMALL */
}
 
 noslash:
@@ -460,8 +482,9 @@ noslash:
if ((full_host = strdup(host)) == NULL)
errx(1, Cannot allocate memory for hostname);
if (debug)
-   fprintf(ttyout, host %s, port %s, path %s, save as %s.\n,
-   host, portnum, path, savefile);
+   fprintf(ttyout, host %s, port %s, path %s, 
+   save as %s, auth %s.\n,
+   host, portnum, path, savefile, auth);
 #endif /* !SMALL */
 
memset(hints, 0, sizeof(hints));
@@ -638,15 +661,18 @@ again:
else
restart_point = 0;
}
-#endif /* !SMALL */
+   if (auth) {
+   ftp_printf(fin, ssl,
+   GET /%s %s\r\nAuthorization: Basic %s\r\nHost: ,
+   epath, restart_point ?
+   HTTP/1.1\r\nConnection: close : HTTP/1.0,
+   auth);
+   free(auth);
+   } else
+#endif /* SMALL */
ftp_printf(fin, ssl, GET /%s %s\r\nHost: , epath,
-#ifndef SMALL
-   restart_point ? HTTP/1.1\r\nConnection: close :
-#endif /* !SMALL */
-   HTTP/1.0);
+   HTTP/1.0);
if (strchr(host, ':')) {
-   char *h, *p;
-
/*
 * strip off scoped address portion, since it's
 * local to node
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.82
diff -d -u -p -r1.82 ftp.1
--- ftp.1   30 Apr 2012 13:41:26 -  1.82
+++ ftp.1   11 Aug 2012 20:08:00 -
@@ -61,7 +61,8 @@
 .Op Fl o Ar output
 .Op Fl s Ar srcaddr
 .Sm off
-.No http:// Ar host Oo : Ar port
+.No http:// Oo Ar user : password No @
+.Oc Ar host Oo : Ar port
 .Oc No / Ar file
 .Sm on
 .Ar ...
@@ -71,7 +72,8 @@
 .Op Fl o Ar output

Re: (2) HTTP basic authentication for ftp(1) (RFC 2617)

[Christiano F. Haesbaert]

Final version, better allocation arith.

ok ?

Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.105
diff -d -u -p -r1.105 fetch.c
--- fetch.c 30 Apr 2012 13:41:26 -  1.105
+++ fetch.c 13 Aug 2012 19:13:57 -
@@ -177,7 +177,7 @@ url_get(const char *origline, const char
 {
char pbuf[NI_MAXSERV], hbuf[NI_MAXHOST], *cp, *portnum, *path, ststr[4];
char *hosttail, *cause = unknown, *newline, *host, *port, *buf = NULL;
-   char *epath, *redirurl, *loctail;
+   char *epath, *redirurl, *loctail, *h, *p;
int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
struct addrinfo hints, *res0, *res, *ares = NULL;
const char * volatile savefile;
@@ -191,7 +191,7 @@ url_get(const char *origline, const char
size_t len, wlen;
 #ifndef SMALL
char *sslpath = NULL, *sslhost = NULL;
-   char *locbase, *full_host = NULL;
+   char *locbase, *full_host = NULL, *auth = NULL;
const char *scheme;
int ishttpsurl = 0;
SSL_CTX *ssl_ctx = NULL;
@@ -250,6 +250,27 @@ url_get(const char *origline, const char
warnx(No filename after host (use -o): %s, origline);
goto cleanup_url_get;
}
+#ifndef SMALL
+   /*
+* Look for auth header in host, since now host does not
+* contain the path. Basic auth from RFC 2617, valid
+* characters for path are in RFC 3986 section 3.3.
+*/
+   if (proxyenv == NULL 
+   (!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
+   if ((p = strchr(host, '@')) != NULL) {
+   size_t authlen = (strlen(host) + 5) * 4 / 3;
+   *p = 0; /* Kill @ */
+   if ((auth = malloc(authlen)) == NULL)
+   err(1, Can't allocate memory for 
+   authorization);
+   if (b64_ntop(host, strlen(host),
+   auth, authlen) == -1)
+   errx(1, error in base64 encoding);
+   host = p + 1;
+   }
+   }
+#endif /* SMALL */
}
 
 noslash:
@@ -460,8 +481,9 @@ noslash:
if ((full_host = strdup(host)) == NULL)
errx(1, Cannot allocate memory for hostname);
if (debug)
-   fprintf(ttyout, host %s, port %s, path %s, save as %s.\n,
-   host, portnum, path, savefile);
+   fprintf(ttyout, host %s, port %s, path %s, 
+   save as %s, auth %s.\n,
+   host, portnum, path, savefile, auth);
 #endif /* !SMALL */
 
memset(hints, 0, sizeof(hints));
@@ -638,15 +660,18 @@ again:
else
restart_point = 0;
}
-#endif /* !SMALL */
+   if (auth) {
+   ftp_printf(fin, ssl,
+   GET /%s %s\r\nAuthorization: Basic %s\r\nHost: ,
+   epath, restart_point ?
+   HTTP/1.1\r\nConnection: close : HTTP/1.0,
+   auth);
+   free(auth);
+   } else
+#endif /* SMALL */
ftp_printf(fin, ssl, GET /%s %s\r\nHost: , epath,
-#ifndef SMALL
-   restart_point ? HTTP/1.1\r\nConnection: close :
-#endif /* !SMALL */
-   HTTP/1.0);
+   HTTP/1.0);
if (strchr(host, ':')) {
-   char *h, *p;
-
/*
 * strip off scoped address portion, since it's
 * local to node
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.82
diff -d -u -p -r1.82 ftp.1
--- ftp.1   30 Apr 2012 13:41:26 -  1.82
+++ ftp.1   11 Aug 2012 20:08:00 -
@@ -61,7 +61,8 @@
 .Op Fl o Ar output
 .Op Fl s Ar srcaddr
 .Sm off
-.No http:// Ar host Oo : Ar port
+.No http:// Oo Ar user : password No @
+.Oc Ar host Oo : Ar port
 .Oc No / Ar file
 .Sm on
 .Ar ...
@@ -71,7 +72,8 @@
 .Op Fl o Ar output
 .Op Fl s Ar srcaddr
 .Sm off
-.No https:// Ar host Oo : Ar port
+.No https:// Oo Ar user : password No @
+.Oc Ar host Oo : Ar port
 .Oc No / Ar file
 .Sm on
 .Ar ...
@@ -1278,12 +1280,12 @@ isn't defined, log in as
 .Ar user
 with a password of
 .Ar password .
-.It http://host[:port]/file
+.It http://[user:password@]host[:port]/file
 An HTTP URL, retrieved using the HTTP protocol.
 If
 .Ev http_proxy
 is defined, it is used as a URL to an HTTP proxy server.
-.It https://host[:port]/file
+.It 

add WARNINGS infrastructure to ftp

[Christiano F. Haesbaert]

since I'm here already...

ok ?


Index: Makefile
===
RCS file: /cvs/src/usr.bin/ftp/Makefile,v
retrieving revision 1.25
diff -d -u -p -r1.25 Makefile
--- Makefile5 May 2009 19:35:30 -   1.25
+++ Makefile13 Aug 2012 15:08:08 -
@@ -21,6 +21,17 @@ LDADD+=  -ledit -lcurses -lutil -lssl -lc
 DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBUTIL}
 LDSTATIC= ${STATIC}
 
+CDIAGFLAGS= -Wall
+#CDIAGFLAGS+=   -Werror
+CDIAGFLAGS+=-Wpointer-arith
+CDIAGFLAGS+=-Wuninitialized
+CDIAGFLAGS+=-Wstrict-prototypes
+CDIAGFLAGS+=-Wmissing-prototypes
+CDIAGFLAGS+=-Wunused
+CDIAGFLAGS+=-Wsign-compare
+CDIAGFLAGS+=-Wbounded
+CDIAGFLAGS+=-Wshadow
+
 #COPTS+= -Wall -Wconversion -Wstrict-prototypes -Wmissing-prototypes
 
 .include bsd.prog.mk

Re: add WARNINGS infrastructure to ftp

[Christiano F. Haesbaert]

On Mon, Aug 13, 2012 at 12:34:37PM -0700, Matthew Dempsky wrote:
 While I recognize there's some precedence in the tree for it already,
 this seems ugly to me.  Is there a reason different programs/libraries
 need different diagnostic flags?  Why doesn't this belong in
 bsd.own.mk or mk.conf?

How about this then ?

Index: bsd.own.mk
===
RCS file: /cvs/src/share/mk/bsd.own.mk,v
retrieving revision 1.116
diff -d -u -p -r1.116 bsd.own.mk
--- bsd.own.mk  12 Apr 2012 11:22:14 -  1.116
+++ bsd.own.mk  13 Aug 2012 19:47:45 -
@@ -99,6 +99,18 @@ LOCALEGRP?=  wheel
 LOCALEOWN?=root
 LOCALEMODE?=   ${NONBINMODE}

# Defaults diagnostic flags for WARNING=Yes.
+.if !defined(CDIAGFLAGS)
+CDIAGFLAGS= -Wall
+CDIAGFLAGS+=-Wpointer-arith
+CDIAGFLAGS+=-Wuninitialized
+CDIAGFLAGS+=-Wstrict-prototypes
+CDIAGFLAGS+=-Wmissing-prototypes
+CDIAGFLAGS+=-Wunused
+CDIAGFLAGS+=-Wsign-compare
+CDIAGFLAGS+=-Wbounded
+CDIAGFLAGS+=-Wshadow
+.endif
+
 # Shared files for system gnu configure, not used yet
 GNUSYSTEM_AUX_DIR?=${BSDSRCDIR}/share/gnu

Re: http anchor fix [Was: Re: (2) HTTP basic authentication for ftp(1) (RFC 2617)]

[Christiano F. Haesbaert]

On Tue, Aug 07, 2012 at 11:50:26AM -0400, Eric P. Mangold wrote:
 Sorry for the top-post
 
 I was wondering if some of you (active) 'ftp' maintainers could evaluate my 
 patch below.
 
 I posted this to the list ages ago, and tried following up with some former 
 committers on the ftp code, but for whatever reason I never received any 
 communication at all.
 
 It is a very simple patch, if someone could review it, and let me know if 
 this is OK or not.
 
 I would apprciate it.
 
 original message follows...
 
 Hi,
 
 I was trying to use the 'ftp' program to retrieve the following URL:
 
 https://pypi.python.org/packages/source/p/pip/pip-1.0.2.tar.gz#md5=47ec6ff3f6d962696fe08d4c8264ad49
 
 Which fails because it considers the fragment as part of the path, and
 so the server returns 404. These type of links are quite common these
 days and so it would be nice if 'ftp' would handle them.
 
 This is my first patch to OpenBSD, so please let me know if there is
 anyhthing else I can do to get this feature implemented. The patch is
 very simple, and I think, correct, as '#' is an unsafe character, and
 must always be URL-encoded if it is to appear literally in a URL and
 not be interpreted as the start of a fragment identifier.
 

I would prefer this to be done on the path processing block, if
possible. Just make sure you test the scheme for http/https, sorry for
slacking on the revision. 

Could you give it a try ?

if (isfileurl) {
path = host;
} else {
path = strchr(host, '/');   /* Find path */
if (EMPTYSTRING(path)) {
if (outfile) {  /* No slash, but */
path=strchr(host,'\0'); /* we have
outfile. */
goto noslash;
}
if (isftpurl)
goto noftpautologin;
warnx(No `/' after host (use -o): %s, origline);
goto cleanup_url_get;
}
*path++ = '\0';

 probably here

if (EMPTYSTRING(path)  !outfile) {
if (isftpurl)
goto noftpautologin;
warnx(No filename after host (use -o): %s, origline);
goto cleanup_url_get;
}

}







 Regards,
 Eric P. Mangold
 
 Index: fetch.c
 ===
 RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
 retrieving revision 1.103
 diff -u -r1.103 fetch.c
 --- fetch.c 25 Aug 2010 20:32:37 -  1.103
 +++ fetch.c 29 Oct 2011 03:34:02 -
 @@ -205,6 +205,11 @@
 if (newline == NULL)
 errx(1, Can't allocate memory to parse URL);
 if (strncasecmp(newline, HTTP_URL, sizeof(HTTP_URL) - 1) == 0) {
 +/* Remove any trailing fragment identifier from the HTTP URL.
 + * Fragments (HTTP anchors) are identified by a hash char 
 ('#'),
 + * as per RFC 3986. */
 +newline = strsep(newline, #);
 +
 host = newline + sizeof(HTTP_URL) - 1;
  #ifndef SMALL
 scheme = HTTP_URL;
 @@ -221,6 +226,11 @@
  #ifndef SMALL
 scheme = FILE_URL;
 } else if (strncasecmp(newline, HTTPS_URL, sizeof(HTTPS_URL) - 1) == 
 0) {
 +/* Remove any trailing fragment identifier from the HTTPS 
 URL.
 + * Fragments (HTTP anchors) are identified by a hash char 
 ('#'),
 + * as per RFC 3986. */
 +newline = strsep(newline, #);
 +
 host = newline + sizeof(HTTPS_URL) - 1;
 ishttpsurl = 1;
 scheme = HTTPS_URL;
 
 
 On Mon, Aug 06, 2012 at 10:56:28PM +0200, Christiano F. Haesbaert wrote:
  Please ignore the other thread, it takes ages for me to open my sent box
  over gprs, so I'm opening a new one.
  
  
  Index: fetch.c
  ===
  RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
  retrieving revision 1.105
  diff -d -u -p -r1.105 fetch.c
  --- fetch.c 30 Apr 2012 13:41:26 -  1.105
  +++ fetch.c 6 Aug 2012 20:49:51 -
  @@ -177,7 +177,7 @@ url_get(const char *origline, const char
   {
  char pbuf[NI_MAXSERV], hbuf[NI_MAXHOST], *cp, *portnum, *path, ststr[4];
  char *hosttail, *cause = unknown, *newline, *host, *port, *buf = NULL;
  -   char *epath, *redirurl, *loctail;
  +   char *epath, *redirurl, *loctail, *h, *p;
  int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
  struct addrinfo hints, *res0, *res, *ares = NULL;
  const char * volatile savefile;
  @@ -191,7 +191,7 @@ url_get(const char *origline, const char
  size_t len, wlen;
   #ifndef SMALL
  char *sslpath = NULL, *sslhost = NULL;
  -   char *locbase

Re: Reduce IPI traffic from signals

[Christiano F. Haesbaert]

Please people, test this diff, we need reports from all architectures if
possible.

We always hear how much users want to help and so on, so please, test
this diff.

On Mon, Jul 23, 2012 at 08:45:17PM +0400, Alexander Polakov wrote:
 This diff reduces IPI traffic for a case when process A is sending
 a lot of signals to process B running on a different CPU. userret()
 delivers all process signals at once, so there is no need to send
 an interrupt for every signal.
 
 The problem was noticed by rtorrent 0.9.2 users, which does exactly
 this, which led to process/system hangs and slowness.
 
 Tested and known to help on amd64 by me and dcoppa@.
 
 Index: amd64/amd64/machdep.c
 ===
 RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
 retrieving revision 1.155
 diff -u -r1.155 machdep.c
 --- amd64/amd64/machdep.c 4 Jun 2012 15:19:47 -   1.155
 +++ amd64/amd64/machdep.c 23 Jul 2012 13:49:40 -
 @@ -690,8 +690,10 @@
  void
  signotify(struct proc *p)
  {
 - aston(p);
 - cpu_unidle(p-p_cpu);
 + if (!isastset(p)) {
 + aston(p);
 + cpu_unidle(p-p_cpu);
 + }
  }
  
  #ifdef MULTIPROCESSOR
 Index: amd64/include/cpu.h
 ===
 RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
 retrieving revision 1.73
 diff -u -r1.73 cpu.h
 --- amd64/include/cpu.h   17 Apr 2012 16:02:33 -  1.73
 +++ amd64/include/cpu.h   23 Jul 2012 13:49:40 -
 @@ -213,6 +213,7 @@
  #endif
  
  #define aston(p) ((p)-p_md.md_astpending = 1)
 +#define isastset(p)  ((p)-p_md.md_astpending == 1)
  
  #define curpcb   curcpu()-ci_curpcb
  
 Index: hppa/hppa/machdep.c
 ===
 RCS file: /cvs/src/sys/arch/hppa/hppa/machdep.c,v
 retrieving revision 1.206
 diff -u -r1.206 machdep.c
 --- hppa/hppa/machdep.c   21 Jun 2012 00:56:59 -  1.206
 +++ hppa/hppa/machdep.c   23 Jul 2012 13:49:40 -
 @@ -1399,8 +1399,10 @@
  void
  signotify(struct proc *p)
  {
 - setsoftast(p);
 - cpu_unidle(p-p_cpu);
 + if (!isastset(p)) {
 + setsoftast(p);
 + cpu_unidle(p-p_cpu);
 + }
  }
  
  /*
 Index: hppa/include/intr.h
 ===
 RCS file: /cvs/src/sys/arch/hppa/include/intr.h,v
 retrieving revision 1.37
 diff -u -r1.37 intr.h
 --- hppa/include/intr.h   14 Jan 2011 13:20:06 -  1.37
 +++ hppa/include/intr.h   23 Jul 2012 13:49:40 -
 @@ -157,7 +157,8 @@
  int   hppa_ipi_broadcast(u_long);
  #endif
  
 -#define  setsoftast(p)   (p-p_md.md_astpending = 1)
 +#define  setsoftast(p)   ((p)-p_md.md_astpending = 1)
 +#define  isastset(p) ((p)-p_md.md_astpending == 1)
  
  void *softintr_establish(int, void (*)(void *), void *);
  void  softintr_disestablish(void *);
 Index: i386/i386/machdep.c
 ===
 RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
 retrieving revision 1.510
 diff -u -r1.510 machdep.c
 --- i386/i386/machdep.c   23 May 2012 08:23:43 -  1.510
 +++ i386/i386/machdep.c   23 Jul 2012 13:49:40 -
 @@ -2420,8 +2420,10 @@
  void
  signotify(struct proc *p)
  {
 - aston(p);
 - cpu_unidle(p-p_cpu);
 + if (!isastset(p)) {
 + aston(p);
 + cpu_unidle(p-p_cpu);
 + }
  }
  
  #ifdef MULTIPROCESSOR
 Index: i386/include/cpu.h
 ===
 RCS file: /cvs/src/sys/arch/i386/include/cpu.h,v
 retrieving revision 1.122
 diff -u -r1.122 cpu.h
 --- i386/include/cpu.h27 Mar 2012 06:44:01 -  1.122
 +++ i386/include/cpu.h23 Jul 2012 13:49:41 -
 @@ -226,6 +226,7 @@
  #endif
  
  #define aston(p) ((p)-p_md.md_astpending = 1)
 +#define isastset(p)  ((p)-p_md.md_astpending == 1)
  
  #define curpcb   curcpu()-ci_curpcb
  
 Index: m88k/include/cpu.h
 ===
 RCS file: /cvs/src/sys/arch/m88k/include/cpu.h,v
 retrieving revision 1.54
 diff -u -r1.54 cpu.h
 --- m88k/include/cpu.h25 Oct 2011 18:38:06 -  1.54
 +++ m88k/include/cpu.h23 Jul 2012 13:49:41 -
 @@ -256,6 +256,7 @@
   (((struct cpu_info *)(framep)-tf.tf_cpu)-ci_intrdepth  1)
  
  #define  aston(p)((p)-p_md.md_astpending = 1)
 +#define  isastset(p) ((p)-p_md.md_astpending == 1)
  
  /*
   * This is used during profiling to integrate system time.
 Index: m88k/m88k/m88k_machdep.c
 ===
 RCS file: /cvs/src/sys/arch/m88k/m88k/m88k_machdep.c,v
 retrieving revision 1.52
 diff -u -r1.52 m88k_machdep.c
 --- m88k/m88k/m88k_machdep.c  23 Mar 2012 15:51:26 -  1.52
 +++ m88k/m88k/m88k_machdep.c  23 Jul 2012 

Re: slight code refactoring in mfi(4)

[Christiano F. Haesbaert]

On 10 August 2012 09:09, David Gwynne l...@animata.net wrote:
 this moves knowledge of where the inbound doorbell on chips is out
 of code and into the structure that stores the chip differences.

 ive tested this on a perc5 (which is the xscale gen). id like a
 skinny user to give it a spin too.


Makes sense to me, I had noticed the same issue with mikeb last week.

You could kill sc_flags too now.

Re: Reduce IPI traffic from signals

[Christiano F. Haesbaert]

On 23 July 2012 18:45, Alexander Polakov p...@sdf.org wrote:
 This diff reduces IPI traffic for a case when process A is sending
 a lot of signals to process B running on a different CPU. userret()
 delivers all process signals at once, so there is no need to send
 an interrupt for every signal.

 The problem was noticed by rtorrent 0.9.2 users, which does exactly
 this, which led to process/system hangs and slowness.

 Tested and known to help on amd64 by me and dcoppa@.

I like the idea and see no harm for amd64 and i386, but I'm not sure
what the others architectures do.

I sense that we might need to implement some ratelimiting in
signotify() for stupid applications like rtorrent that loop sending
signals.

Re: Build cpu topology on amd64.

[Christiano F. Haesbaert]

Ok so here is the version with #ifndef SMALL_KERNEL, the only question
that remains is: do we keep the printf in dmesg ? or shall I take that
out ? 

I'd like to keep it so we may know if the detection is correctly just by
looking at sent dmesgs.

Index: arch/amd64/amd64/identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.36
diff -d -u -p -r1.36 identcpu.c
--- arch/amd64/amd64/identcpu.c 22 Apr 2012 19:36:09 -  1.36
+++ arch/amd64/amd64/identcpu.c 13 Jul 2012 11:45:58 -
@@ -446,4 +446,126 @@ identifycpu(struct cpu_info *ci)
sensordev_install(ci-ci_sensordev);
 #endif
}
+#ifndef SMALL_KERNEL
+   cpu_topology(ci);
+#endif
+}
+
+#ifndef SMALL_KERNEL
+/*
+ * Base 2 logarithm of an int. returns 0 for 0 (yeye, I know).
+ */
+static int
+log2(unsigned int i)
+{
+   int ret = 0;
+
+   while (i = 1)
+   ret++;
+
+   return (ret);
+}
+
+static int
+mask_width(u_int x)
+{
+   int bit;
+   int mask;
+   int powerof2;
+
+   powerof2 = ((x - 1)  x) == 0;
+   mask = (x  (1 - powerof2)) - 1;
+
+   /* fls */
+   if (mask == 0)
+   return (0);
+   for (bit = 1; mask != 1; bit++)
+   mask = (unsigned int)mask  1;
+
+   return (bit);
+}
+
+/*
+ * Build up cpu topology for given cpu, must run on the core itself.
+ */
+void
+cpu_topology(struct cpu_info *ci)
+{
+   u_int32_t eax, ebx, ecx, edx;
+   u_int32_t apicid, max_apicid, max_coreid;
+   u_int32_t smt_bits, core_bits, pkg_bits;
+   u_int32_t smt_mask, core_mask, pkg_mask;
+   
+   /* We need at least apicid at CPUID 1 */
+   CPUID(0, eax, ebx, ecx, edx);
+   if (eax  1)
+   goto no_topology;
+   
+   /* Initial apicid */
+   CPUID(1, eax, ebx, ecx, edx);
+   apicid = (ebx  24)  0xff;
+   
+   if (strcmp(cpu_vendor, AuthenticAMD) == 0) {
+   /* We need at least apicid at CPUID 0x8008 */
+   CPUID(0x8000, eax, ebx, ecx, edx);
+   if (eax  0x8008)
+   goto no_topology;
+   
+   CPUID(0x8008, eax, ebx, ecx, edx);
+   core_bits = (ecx  12)  0xf;
+   if (core_bits == 0)
+   goto no_topology;
+   /* So coreidsize 2 gives 3, 3 gives 7... */
+   core_mask = (1  core_bits) - 1;
+   /* Core id is the least significant considering mask */
+   ci-ci_core_id = apicid  core_mask;
+   /* Pkg id is the upper remaining bits */
+   ci-ci_pkg_id = apicid  ~core_mask;
+   ci-ci_pkg_id = core_bits;
+   } else if (strcmp(cpu_vendor, GenuineIntel) == 0) {
+   /* We only support leaf 1/4 detection */
+   CPUID(0, eax, ebx, ecx, edx);
+   if (eax  4)
+   goto no_topology;
+   /* Get max_apicid */
+   CPUID(1, eax, ebx, ecx, edx);
+   max_apicid = (ebx  16)  0xff;
+   /* Get max_coreid */
+   CPUID2(4, 0, eax, ebx, ecx, edx);
+   max_coreid = ((eax  26)  0x3f) + 1;
+   /* SMT */
+   smt_bits = mask_width(max_apicid / max_coreid);
+   smt_mask = (1  smt_bits) - 1;
+   /* Core */
+   core_bits = log2(max_coreid);
+   core_mask = (1  (core_bits + smt_bits)) - 1;
+   core_mask ^= smt_mask;
+   /* Pkg */
+   pkg_bits = core_bits + smt_bits;
+   pkg_mask = -1  core_bits;
+
+   ci-ci_smt_id = apicid  smt_mask;
+   ci-ci_core_id = (apicid  core_mask)  smt_bits;
+   ci-ci_pkg_id = (apicid  pkg_mask)  pkg_bits;
+   } else
+   goto no_topology;
+#ifdef DEBUG
+   printf(cpu%d: smt %u, core %u, pkg %u 
+   (apicid 0x%x, max_apicid 0x%x, max_coreid 0x%x, smt_bits 0x%x, 
smt_mask 0x%x, 
+   core_bits 0x%x, core_mask 0x%x, pkg_bits 0x%x, pkg_mask 0x%x)\n,
+   ci-ci_cpuid, ci-ci_smt_id, ci-ci_core_id, ci-ci_pkg_id,
+   apicid, max_apicid, max_coreid, smt_bits, smt_mask, core_bits,
+   core_mask, pkg_bits, pkg_mask);
+#else
+   printf(cpu%d: smt %u, core %u, package %u\n, ci-ci_cpuid,
+   ci-ci_smt_id, ci-ci_core_id, ci-ci_pkg_id);
+   
+#endif
+   return;
+   /* We can't map, so consider ci_core_id as ci_cpuid */
+no_topology:
+   ci-ci_smt_id  = 0;
+   ci-ci_core_id = ci-ci_cpuid;
+   ci-ci_pkg_id  = 0;
 }
+#endif /* SMALL_KERNEL */
Index: arch/amd64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.73
diff -d -u -p -r1.73 cpu.h
--- arch/amd64/include/cpu.h17 Apr 2012 16:02:33 -  1.73

Re: kbd: Use NULL instead of 0 for pointers

[Christiano F. Haesbaert]

I like these kind of diffs, some people don't, ok from me.

On Fri, Jul 13, 2012 at 03:37:08PM +0600, Alexandr Shadchin wrote:
 Use NULL instead of 0 for pointers
 
 -- 
 Alexandr Shadchin
 
 Index: kbd_wscons.c
 ===
 RCS file: /cvs/src/sbin/kbd/kbd_wscons.c,v
 retrieving revision 1.25
 diff -u -p -r1.25 kbd_wscons.c
 --- kbd_wscons.c  23 Jun 2008 17:41:21 -  1.25
 +++ kbd_wscons.c  13 Jul 2012 09:34:14 -
 @@ -93,13 +93,13 @@ struct nameint {
  struct nameint kbdenc_tab[] = {
   KB_ENCTAB
   ,
 - { 0, 0 }
 + { 0, NULL }
  };
  
  struct nameint kbdvar_tab[] = {
   KB_VARTAB
   ,
 - { 0, 0 }
 + { 0, NULL }
  };
  
  extern char *__progname;
 @@ -232,7 +232,7 @@ kbd_list(void)
   }
  
  #ifndef NOKVM
 - if ((kd = kvm_openfiles(NULL, NULL, NULL, O_RDONLY, errbuf)) == 0)
 + if ((kd = kvm_openfiles(NULL, NULL, NULL, O_RDONLY, errbuf)) == NULL)
   errx(1, kvm_openfiles: %s, errbuf);
  
   if (kvm_nlist(kd, nl) == -1)
 

-- 
Christiano Farina HAESBAERT
Do NOT send me html mail.

Re: Build cpu topology on amd64.

[Christiano F. Haesbaert]

On Fri, Jul 13, 2012 at 03:06:34PM +0200, Mark Kettenis wrote:
  Date: Fri, 13 Jul 2012 14:57:11 +0200
  From: Christiano F. Haesbaert haesba...@openbsd.org
  
  Ok so here is the version with #ifndef SMALL_KERNEL, the only question
  that remains is: do we keep the printf in dmesg ? or shall I take that
  out ? 
  
  I'd like to keep it so we may know if the detection is correctly just by
  looking at sent dmesgs.
 
 Can you shelve this until you:
 
 a) Have the equivalent code for i386.

Sure, that should actually be the same code, I just need to make the
identifycpu() stuff run on each cpu on i386 as in amd64.

 b) Have something that actually uses this?

That won't be so simple, but ok :)

Let me explain why, when I started all this I wanted to favor migration
from procs on the same core, and then on the same package. So you would
pay a penalty to cross cores and a double penalty to cross packages.

But this is naive and stupid, sometimes, you want procs to go as far
away as possible: think of 2 procs that trash the cachelines, Brett and
I found a good metric from a paper from Alexandra Fedorova, it involves
calculating a pain parameter, but we're far away from making that
possible and viable, we have easier/bigger gains right now doing other
stuff.

I'll hold onto it, at least it's on the mailing lists so users can play
with it :).

Cheers 

 
 Cheers,
 
 Mark
 
  Index: arch/amd64/amd64/identcpu.c
  ===
  RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
  retrieving revision 1.36
  diff -d -u -p -r1.36 identcpu.c
  --- arch/amd64/amd64/identcpu.c 22 Apr 2012 19:36:09 -  1.36
  +++ arch/amd64/amd64/identcpu.c 13 Jul 2012 11:45:58 -
  @@ -446,4 +446,126 @@ identifycpu(struct cpu_info *ci)
  sensordev_install(ci-ci_sensordev);
   #endif
  }
  +#ifndef SMALL_KERNEL
  +   cpu_topology(ci);
  +#endif
  +}
  +
  +#ifndef SMALL_KERNEL
  +/*
  + * Base 2 logarithm of an int. returns 0 for 0 (yeye, I know).
  + */
  +static int
  +log2(unsigned int i)
  +{
  +   int ret = 0;
  +
  +   while (i = 1)
  +   ret++;
  +
  +   return (ret);
  +}
  +
  +static int
  +mask_width(u_int x)
  +{
  +   int bit;
  +   int mask;
  +   int powerof2;
  +
  +   powerof2 = ((x - 1)  x) == 0;
  +   mask = (x  (1 - powerof2)) - 1;
  +
  +   /* fls */
  +   if (mask == 0)
  +   return (0);
  +   for (bit = 1; mask != 1; bit++)
  +   mask = (unsigned int)mask  1;
  +
  +   return (bit);
  +}
  +
  +/*
  + * Build up cpu topology for given cpu, must run on the core itself.
  + */
  +void
  +cpu_topology(struct cpu_info *ci)
  +{
  +   u_int32_t eax, ebx, ecx, edx;
  +   u_int32_t apicid, max_apicid, max_coreid;
  +   u_int32_t smt_bits, core_bits, pkg_bits;
  +   u_int32_t smt_mask, core_mask, pkg_mask;
  +   
  +   /* We need at least apicid at CPUID 1 */
  +   CPUID(0, eax, ebx, ecx, edx);
  +   if (eax  1)
  +   goto no_topology;
  +   
  +   /* Initial apicid */
  +   CPUID(1, eax, ebx, ecx, edx);
  +   apicid = (ebx  24)  0xff;
  +   
  +   if (strcmp(cpu_vendor, AuthenticAMD) == 0) {
  +   /* We need at least apicid at CPUID 0x8008 */
  +   CPUID(0x8000, eax, ebx, ecx, edx);
  +   if (eax  0x8008)
  +   goto no_topology;
  +   
  +   CPUID(0x8008, eax, ebx, ecx, edx);
  +   core_bits = (ecx  12)  0xf;
  +   if (core_bits == 0)
  +   goto no_topology;
  +   /* So coreidsize 2 gives 3, 3 gives 7... */
  +   core_mask = (1  core_bits) - 1;
  +   /* Core id is the least significant considering mask */
  +   ci-ci_core_id = apicid  core_mask;
  +   /* Pkg id is the upper remaining bits */
  +   ci-ci_pkg_id = apicid  ~core_mask;
  +   ci-ci_pkg_id = core_bits;
  +   } else if (strcmp(cpu_vendor, GenuineIntel) == 0) {
  +   /* We only support leaf 1/4 detection */
  +   CPUID(0, eax, ebx, ecx, edx);
  +   if (eax  4)
  +   goto no_topology;
  +   /* Get max_apicid */
  +   CPUID(1, eax, ebx, ecx, edx);
  +   max_apicid = (ebx  16)  0xff;
  +   /* Get max_coreid */
  +   CPUID2(4, 0, eax, ebx, ecx, edx);
  +   max_coreid = ((eax  26)  0x3f) + 1;
  +   /* SMT */
  +   smt_bits = mask_width(max_apicid / max_coreid);
  +   smt_mask = (1  smt_bits) - 1;
  +   /* Core */
  +   core_bits = log2(max_coreid);
  +   core_mask = (1  (core_bits + smt_bits)) - 1;
  +   core_mask ^= smt_mask;
  +   /* Pkg */
  +   pkg_bits = core_bits + smt_bits;
  +   pkg_mask = -1  core_bits;
  +
  +   ci-ci_smt_id = apicid  smt_mask;
  +   ci-ci_core_id = (apicid  core_mask)  smt_bits;
  +   ci-ci_pkg_id = (apicid  pkg_mask)  pkg_bits;
  +   } else
  +   goto

wakeup should only call need_resched if priority is lower than current one.

[Christiano F. Haesbaert]

Heya,

wakeup_n() has a inline expansion of setrunnable(), but it differs by
always calling need_resched(), this sends an ipi for *every* wakeup
channel. 

It might have something to do with aja's problem.

Index: kern_synch.c
===
RCS file: /cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.102
diff -d -u -p -r1.102 kern_synch.c
--- kern_synch.c10 Apr 2012 11:33:58 -  1.102
+++ kern_synch.c10 Jul 2012 14:27:17 -
@@ -378,7 +378,9 @@ wakeup_n(const volatile void *ident, int
p-p_stat = SRUN;
p-p_cpu = sched_choosecpu(p);
setrunqueue(p);
-   need_resched(p-p_cpu);
+   if (p-p_priority 
+   p-p_cpu-ci_schedstate.spc_curpriority)
+   need_resched(p-p_cpu);
/* END INLINE EXPANSION */
 
}

Re: wakeup should only call need_resched if priority is lower than current one.

[Christiano F. Haesbaert]

Commited, it fixes aja's problem indeed.

Now, I don't like this at all, we'll have more bugs of this nature,
can't we turn setrunnable() into an inline function and place it in a
header ?

Or just call setrunnable() directly, but I'd have to run some tests to
see the performance impact.

On Tue, Jul 10, 2012 at 04:37:13PM +0200, Christiano F. Haesbaert wrote:
 Heya,
 
 wakeup_n() has a inline expansion of setrunnable(), but it differs by
 always calling need_resched(), this sends an ipi for *every* wakeup
 channel. 
 
 It might have something to do with aja's problem.
 
 Index: kern_synch.c
 ===
 RCS file: /cvs/src/sys/kern/kern_synch.c,v
 retrieving revision 1.102
 diff -d -u -p -r1.102 kern_synch.c
 --- kern_synch.c  10 Apr 2012 11:33:58 -  1.102
 +++ kern_synch.c  10 Jul 2012 14:27:17 -
 @@ -378,7 +378,9 @@ wakeup_n(const volatile void *ident, int
   p-p_stat = SRUN;
   p-p_cpu = sched_choosecpu(p);
   setrunqueue(p);
 - need_resched(p-p_cpu);
 + if (p-p_priority 
 + p-p_cpu-ci_schedstate.spc_curpriority)
 + need_resched(p-p_cpu);
   /* END INLINE EXPANSION */
  
   }

tedu sched_peg_curproc()

[Christiano F. Haesbaert]

This isn't used, the idle thread just sets the PEG flag and goes on. 

Index: kern/kern_sched.c
===
RCS file: /cvs/src/sys/kern/kern_sched.c,v
retrieving revision 1.27
diff -d -u -p -r1.27 kern_sched.c
--- kern/kern_sched.c   10 Jul 2012 18:20:37 -  1.27
+++ kern/kern_sched.c   10 Jul 2012 19:41:55 -
@@ -545,26 +545,6 @@ sched_proc_to_cpu_cost(struct cpu_info *
return (cost);
 }
 
-/*
- * Peg a proc to a cpu.
- */
-void
-sched_peg_curproc(struct cpu_info *ci)
-{
-   struct proc *p = curproc;
-   int s;
-
-   SCHED_LOCK(s);
-   p-p_priority = p-p_usrpri;
-   p-p_stat = SRUN;
-   p-p_cpu = ci;
-   atomic_setbits_int(p-p_flag, P_CPUPEG);
-   setrunqueue(p);
-   p-p_ru.ru_nvcsw++;
-   mi_switch();
-   SCHED_UNLOCK(s);
-}
-
 #ifdef MULTIPROCESSOR
 
 void
Index: sys/sched.h
===
RCS file: /cvs/src/sys/sys/sched.h,v
retrieving revision 1.30
diff -d -u -p -r1.30 sched.h
--- sys/sched.h 16 Nov 2011 20:50:19 -  1.30
+++ sys/sched.h 10 Jul 2012 19:42:05 -
@@ -151,7 +151,6 @@ struct cpu_info *sched_choosecpu_fork(st
 void cpu_idle_enter(void);
 void cpu_idle_cycle(void);
 void cpu_idle_leave(void);
-void sched_peg_curproc(struct cpu_info *ci);
 
 #ifdef MULTIPROCESSOR
 void sched_start_secondary_cpus(void);

Re: tedu sched_peg_curproc()

[Christiano F. Haesbaert]

On 10 July 2012 22:32, Mark Kettenis mark.kette...@xs4all.nl wrote:
 Date: Tue, 10 Jul 2012 21:44:06 +0200
 From: Christiano F. Haesbaert haesba...@openbsd.org

 This isn't used, the idle thread just sets the PEG flag and goes on.

 Please don't; I have some stuff that *will* use this.


Okayz :)

 Index: kern/kern_sched.c
 ===
 RCS file: /cvs/src/sys/kern/kern_sched.c,v
 retrieving revision 1.27
 diff -d -u -p -r1.27 kern_sched.c
 --- kern/kern_sched.c 10 Jul 2012 18:20:37 -  1.27
 +++ kern/kern_sched.c 10 Jul 2012 19:41:55 -
 @@ -545,26 +545,6 @@ sched_proc_to_cpu_cost(struct cpu_info *
   return (cost);
  }

 -/*
 - * Peg a proc to a cpu.
 - */
 -void
 -sched_peg_curproc(struct cpu_info *ci)
 -{
 - struct proc *p = curproc;
 - int s;
 -
 - SCHED_LOCK(s);
 - p-p_priority = p-p_usrpri;
 - p-p_stat = SRUN;
 - p-p_cpu = ci;
 - atomic_setbits_int(p-p_flag, P_CPUPEG);
 - setrunqueue(p);
 - p-p_ru.ru_nvcsw++;
 - mi_switch();
 - SCHED_UNLOCK(s);
 -}
 -
  #ifdef MULTIPROCESSOR

  void
 Index: sys/sched.h
 ===
 RCS file: /cvs/src/sys/sys/sched.h,v
 retrieving revision 1.30
 diff -d -u -p -r1.30 sched.h
 --- sys/sched.h   16 Nov 2011 20:50:19 -  1.30
 +++ sys/sched.h   10 Jul 2012 19:42:05 -
 @@ -151,7 +151,6 @@ struct cpu_info *sched_choosecpu_fork(st
  void cpu_idle_enter(void);
  void cpu_idle_cycle(void);
  void cpu_idle_leave(void);
 -void sched_peg_curproc(struct cpu_info *ci);

  #ifdef MULTIPROCESSOR
  void sched_start_secondary_cpus(void);

tedu old comment about cpu affinity.

[Christiano F. Haesbaert]

This no longer applies, it is probably a leftover from the days when we
had a global runqueue. Discussed with blambert.

ok ?

Index: sched_bsd.c
===
RCS file: /cvs/src/sys/kern/sched_bsd.c,v
retrieving revision 1.29
diff -d -u -p -r1.29 sched_bsd.c
--- sched_bsd.c 23 Mar 2012 15:51:26 -  1.29
+++ sched_bsd.c 9 Jul 2012 08:45:32 -
@@ -464,16 +464,7 @@ resched_proc(struct proc *p, u_char pri)
 
/*
 * XXXSMP
-* Since p-p_cpu persists across a context switch,
-* this gives us *very weak* processor affinity, in
-* that we notify the CPU on which the process last
-* ran that it should try to switch.
-*
-* This does not guarantee that the process will run on
-* that processor next, because another processor might
-* grab it the next time it performs a context switch.
-*
-* This also does not handle the case where its last
+* This does not handle the case where its last
 * CPU is running a higher-priority process, but every
 * other CPU is running a lower-priority process.  There
 * are ways to handle this situation, but they're not

Build cpu topology on amd64.

[Christiano F. Haesbaert]

Heya, 

I have this rotting in my tree, since actually using it effectively is
way harder than it seems, anyhow, this correctly builds the topology in
amd64, we know 3 things about each cpu now:

- thread id (smt id)
- core id
- package id

This is not complete but is enough IMHO, it lacks x2apic detection.
I've tried to trim it up, but the mask logic is a bit cryptic.

obs: I left a print on dmesg just so that people can test, I intend to
remove if it goes in. 

an atom d270 reports the following:
cpu0: smt 0, core 0, package 0
cpu1: smt 1, core 0, package 0
cpu2: smt 0, core 1, package 0
cpu3: smt 1, core 1, package 0

a core2duo L7500:
cpu0: smt 0, core 0, package 0
cpu1: smt 0, core 1, package 0

Do we want this ? 

Index: arch/amd64/amd64/identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.36
diff -d -u -p -r1.36 identcpu.c
--- arch/amd64/amd64/identcpu.c 22 Apr 2012 19:36:09 -  1.36
+++ arch/amd64/amd64/identcpu.c 8 Jul 2012 09:03:02 -
@@ -446,4 +446,123 @@ identifycpu(struct cpu_info *ci)
sensordev_install(ci-ci_sensordev);
 #endif
}
+
+   cpu_topology(ci);
+}
+
+/*
+ * Base 2 logarithm of an int. returns 0 for 0 (yeye, I know).
+ */
+static int
+log2(unsigned int i)
+{
+   int ret = 0;
+
+   while (i = 1)
+   ret++;
+
+   return (ret);
+}
+
+static int
+mask_width(u_int x)
+{
+   int bit;
+   int mask;
+   int powerof2;
+
+   powerof2 = ((x - 1)  x) == 0;
+   mask = (x  (1 - powerof2)) - 1;
+
+   /* fls */
+   if (mask == 0)
+   return (0);
+   for (bit = 1; mask != 1; bit++)
+   mask = (unsigned int)mask  1;
+
+   return (bit);
+}
+
+/*
+ * Build up cpu topology for given cpu, must run on the core itself.
+ */
+void
+cpu_topology(struct cpu_info *ci)
+{
+   u_int32_t eax, ebx, ecx, edx;
+   u_int32_t apicid, max_apicid, max_coreid;
+   u_int32_t smt_bits, core_bits, pkg_bits;
+   u_int32_t smt_mask, core_mask, pkg_mask;
+   
+   /* We need at least apicid at CPUID 1 */
+   CPUID(0, eax, ebx, ecx, edx);
+   if (eax  1)
+   goto no_topology;
+   
+   /* Initial apicid */
+   CPUID(1, eax, ebx, ecx, edx);
+   apicid = (ebx  24)  0xff;
+   
+   if (strcmp(cpu_vendor, AuthenticAMD) == 0) {
+   /* We need at least apicid at CPUID 0x8008 */
+   CPUID(0x8000, eax, ebx, ecx, edx);
+   if (eax  0x8008)
+   goto no_topology;
+   
+   CPUID(0x8008, eax, ebx, ecx, edx);
+   core_bits = (ecx  12)  0xf;
+   if (core_bits == 0)
+   goto no_topology;
+   /* So coreidsize 2 gives 3, 3 gives 7... */
+   core_mask = (1  core_bits) - 1;
+   /* Core id is the least significant considering mask */
+   ci-ci_core_id = apicid  core_mask;
+   /* Pkg id is the upper remaining bits */
+   ci-ci_pkg_id = apicid  ~core_mask;
+   ci-ci_pkg_id = core_bits;
+   } else if (strcmp(cpu_vendor, GenuineIntel) == 0) {
+   /* We only support leaf 1/4 detection */
+   CPUID(0, eax, ebx, ecx, edx);
+   if (eax  4)
+   goto no_topology;
+   /* Get max_apicid */
+   CPUID(1, eax, ebx, ecx, edx);
+   max_apicid = (ebx  16)  0xff;
+   /* Get max_coreid */
+   CPUID2(4, 0, eax, ebx, ecx, edx);
+   max_coreid = ((eax  26)  0x3f) + 1;
+   /* SMT */
+   smt_bits = mask_width(max_apicid / max_coreid);
+   smt_mask = (1  smt_bits) - 1;
+   /* Core */
+   core_bits = log2(max_coreid);
+   core_mask = (1  (core_bits + smt_bits)) - 1;
+   core_mask ^= smt_mask;
+   /* Pkg */
+   pkg_bits = core_bits + smt_bits;
+   pkg_mask = -1  core_bits;
+
+   ci-ci_smt_id = apicid  smt_mask;
+   ci-ci_core_id = (apicid  core_mask)  smt_bits;
+   ci-ci_pkg_id = (apicid  pkg_mask)  pkg_bits;
+   } else
+   goto no_topology;
+#ifdef DEBUG
+   printf(cpu%d: smt %u, core %u, pkg %u 
+   (apicid 0x%x, max_apicid 0x%x, max_coreid 0x%x, smt_bits 0x%x, 
smt_mask 0x%x, 
+   core_bits 0x%x, core_mask 0x%x, pkg_bits 0x%x, pkg_mask 0x%x)\n,
+   ci-ci_cpuid, ci-ci_smt_id, ci-ci_core_id, ci-ci_pkg_id,
+   apicid, max_apicid, max_coreid, smt_bits, smt_mask, core_bits,
+   core_mask, pkg_bits, pkg_mask);
+#else
+   printf(cpu%d: smt %u, core %u, package %u\n, ci-ci_cpuid,
+   ci-ci_smt_id, ci-ci_core_id, ci-ci_pkg_id);
+   
+#endif
+   return;
+   /* We can't map, so consider 

Re: nc(1): Report incoming connections on nc -v -l

[Christiano F. Haesbaert]

Still waiting for another ok.

On Thu, Jun 28, 2012 at 11:36:27PM -0300, Christiano F. Haesbaert wrote:
 This looks good to me, can I get another ok ?
 
 
 On Sun, Jun 24, 2012 at 07:07:29AM -0400, Ricky Zhou wrote:
  On 2012-06-16 02:37:27 PM, Christiano F. Haesbaert wrote:
   I guess so, I don't use nc too often but it sounds reasonable to me,
   your code has a few notes though, please check inline. 
  (Sorry for the slow response, was travelling last week)
  
  Thanks for looking at the patch - here's a new one with your fixes
  included.
  
  Thanks,
  Ricky
  
  
  Index: netcat.c
  ===
  RCS file: /cvs/src/usr.bin/nc/netcat.c,v
  retrieving revision 1.107
  diff -u netcat.c
  --- netcat.c1 Apr 2012 02:58:57 -   1.107
  +++ netcat.c24 Jun 2012 09:51:19 -
  @@ -106,6 +106,7 @@
   intunix_listen(char *);
   void   set_common_sockopts(int);
   intmap_tos(char *, int *);
  +void   report_connect(const struct sockaddr *, socklen_t);
   void   usage(int);
   
   int
  @@ -364,6 +365,9 @@
  if (rv  0)
  err(1, connect);
   
  +   if (vflag)
  +   report_connect((struct sockaddr *)z, 
  len);
  +
  readwrite(s);
  } else {
  len = sizeof(cliaddr);
  @@ -371,6 +375,10 @@
  len);
  if (connfd == -1)
  err(1, accept);
  +
  +   if (vflag)
  +   report_connect((struct sockaddr 
  *)cliaddr, len);
  +
  readwrite(connfd);
  close(connfd);
  }
  @@ -957,6 +965,32 @@
  }
   
  return (0);
  +}
  +
  +void
  +report_connect(const struct sockaddr *sa, socklen_t salen)
  +{
  +   char remote_host[NI_MAXHOST];
  +   char remote_port[NI_MAXSERV];
  +   int herr;
  +   int flags = NI_NUMERICSERV;
  +   
  +   if (nflag)
  +   flags |= NI_NUMERICHOST;
  +   
  +   if ((herr = getnameinfo(sa, salen,
  +   remote_host, sizeof(remote_host),
  +   remote_port, sizeof(remote_port),
  +   flags)) != 0) {
  +   if (herr == EAI_SYSTEM)
  +   err(1, getnameinfo);
  +   else
  +   errx(1, getnameinfo: %s, gai_strerror(herr));
  +   }
  +   
  +   fprintf(stderr,
  +   Connection from %s %s 
  +   received!\n, remote_host, remote_port);
   }
   
   void

Re: nc -ul semantics

[Christiano F. Haesbaert]

How about this one ?
It's your original idea, but I don't like that extra indentation level,
we are already too deep.



Index: nc.1
===
RCS file: /cvs/src/usr.bin/nc/nc.1,v
retrieving revision 1.60
diff -d -u -p -r1.60 nc.1
--- nc.17 Feb 2012 12:11:43 -   1.60
+++ nc.17 Jul 2012 10:30:10 -
@@ -119,6 +119,10 @@ is completed.
 It is an error to use this option without the
 .Fl l
 option.
+When used together with the
+.Fl u
+option, the server socket is not connected and it receives UDP datagrams from
+multiple hosts.
 .It Fl l
 Used to specify that
 .Nm
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.108
diff -d -u -p -r1.108 netcat.c
--- netcat.c7 Jul 2012 09:36:30 -   1.108
+++ netcat.c7 Jul 2012 10:30:11 -
@@ -345,11 +345,17 @@ main(int argc, char *argv[])
if (s  0)
err(1, NULL);
/*
-* For UDP, we will use recvfrom() initially
-* to wait for a caller, then use the regular
-* functions to talk to the caller.
+* For UDP and -k, don't connect the socket, let it
+* receive datagrams from multiple socket pairs.
 */
-   if (uflag) {
+   if (uflag  kflag)
+   readwrite(s);
+   /*
+* For UDP and not -k, we will use recvfrom() initially
+* to wait for a caller, then use the regular functions
+* to talk to the caller.
+*/
+   else if (uflag  !kflag) {
int rv, plen;
char buf[16384];
struct sockaddr_storage z;

Re: nc(1): Report incoming connections on nc -v -l

[Christiano F. Haesbaert]

This looks good to me, can I get another ok ?


On Sun, Jun 24, 2012 at 07:07:29AM -0400, Ricky Zhou wrote:
 On 2012-06-16 02:37:27 PM, Christiano F. Haesbaert wrote:
  I guess so, I don't use nc too often but it sounds reasonable to me,
  your code has a few notes though, please check inline. 
 (Sorry for the slow response, was travelling last week)
 
 Thanks for looking at the patch - here's a new one with your fixes
 included.
 
 Thanks,
 Ricky
 
 
 Index: netcat.c
 ===
 RCS file: /cvs/src/usr.bin/nc/netcat.c,v
 retrieving revision 1.107
 diff -u netcat.c
 --- netcat.c  1 Apr 2012 02:58:57 -   1.107
 +++ netcat.c  24 Jun 2012 09:51:19 -
 @@ -106,6 +106,7 @@
  int  unix_listen(char *);
  void set_common_sockopts(int);
  int  map_tos(char *, int *);
 +void report_connect(const struct sockaddr *, socklen_t);
  void usage(int);
  
  int
 @@ -364,6 +365,9 @@
   if (rv  0)
   err(1, connect);
  
 + if (vflag)
 + report_connect((struct sockaddr *)z, 
 len);
 +
   readwrite(s);
   } else {
   len = sizeof(cliaddr);
 @@ -371,6 +375,10 @@
   len);
   if (connfd == -1)
   err(1, accept);
 +
 + if (vflag)
 + report_connect((struct sockaddr 
 *)cliaddr, len);
 +
   readwrite(connfd);
   close(connfd);
   }
 @@ -957,6 +965,32 @@
   }
  
   return (0);
 +}
 +
 +void
 +report_connect(const struct sockaddr *sa, socklen_t salen)
 +{
 + char remote_host[NI_MAXHOST];
 + char remote_port[NI_MAXSERV];
 + int herr;
 + int flags = NI_NUMERICSERV;
 + 
 + if (nflag)
 + flags |= NI_NUMERICHOST;
 + 
 + if ((herr = getnameinfo(sa, salen,
 + remote_host, sizeof(remote_host),
 + remote_port, sizeof(remote_port),
 + flags)) != 0) {
 + if (herr == EAI_SYSTEM)
 + err(1, getnameinfo);
 + else
 + errx(1, getnameinfo: %s, gai_strerror(herr));
 + }
 + 
 + fprintf(stderr,
 + Connection from %s %s 
 + received!\n, remote_host, remote_port);
  }
  
  void

Re: nc -ul semantics

[Christiano F. Haesbaert]

Although the idea for using -k with -u is a good one, I don't think
it's necessary, I for one think we should change semantics as the
first mail suggested.

The only real for using connected UDP sockets is to get the ICMP async
errors at the socket level, ICMP port unreachable mainly.

I see no reason why not to change the semantics to *not* use connected
UDP sockets, since the server will *never* write anything back to the
client.


On Mon, Jun 25, 2012 at 01:41:48PM +0300, Lazaros Koromilas wrote:
 On Sun, Jun 24, 2012 at 11:23:05PM +0200, Ariane van der Steldt wrote:
  On Mon, Jun 25, 2012 at 12:09:33AM +0300, dsp wrote:
   We observe the following behaviour when running nc -ul.
   The server begins on a recvfrom() and when data arrives it
   connects() the socket.
   When the client dies , the server remains in a connected state
   therefore ignoring subsequent data arriving on the port.
   Is this really the intended logic for a connectionless protocol like
   UDP? We proceeded to comment out the connect statement and we were
   able to receive data from multiple *sessions* as expected.
   Can you shed some light on the matter???
  
  Reading the man page, I think you want to add the -k option to nc.
 
 The -k option does nothing when used with -u because the readwrite()
 session cannot end.  Would the following be a reasonable change?
 It has the side effect of letting UDP packets to interleave.
 
 
 Index: nc.1
 ===
 RCS file: /cvs/src/usr.bin/nc/nc.1,v
 retrieving revision 1.60
 diff -u -p -r1.60 nc.1
 --- nc.1  7 Feb 2012 12:11:43 -   1.60
 +++ nc.1  25 Jun 2012 10:40:15 -
 @@ -119,6 +119,10 @@ is completed.
  It is an error to use this option without the
  .Fl l
  option.
 +When used together with the
 +.Fl u
 +option all UDP datagrams arriving on the port are received;
 +not just those sent by the first client to connect.
  .It Fl l
  Used to specify that
  .Nm
 Index: netcat.c
 ===
 RCS file: /cvs/src/usr.bin/nc/netcat.c,v
 retrieving revision 1.105
 diff -u -p -r1.105 netcat.c
 --- netcat.c  9 Feb 2012 06:25:35 -   1.105
 +++ netcat.c  25 Jun 2012 10:40:15 -
 @@ -364,9 +364,12 @@ main(int argc, char *argv[])
   if (rv  0)
   err(1, recvfrom);
  
 - rv = connect(s, (struct sockaddr *)z, len);
 - if (rv  0)
 - err(1, connect);
 + if (!kflag) {
 + rv = connect(s, (struct sockaddr *)z,
 + len);
 + if (rv  0)
 + err(1, connect);
 + }
  
   readwrite(s);
   } else {

Re: nc -ul semantics

[Christiano F. Haesbaert]

On Thu, Jun 28, 2012 at 11:44:36PM -0300, Christiano F. Haesbaert wrote:
 Although the idea for using -k with -u is a good one, I don't think
 it's necessary, I for one think we should change semantics as the
 first mail suggested.
 
 The only real for using connected UDP sockets is to get the ICMP async
 errors at the socket level, ICMP port unreachable mainly.
 
 I see no reason why not to change the semantics to *not* use connected
 UDP sockets, since the server will *never* write anything back to the
 client.

But then we would have to drop print connection received from...,
which is being introduced in another diff. Now I'm more inclined in
going for the -k, that way we know when to supress the message. 

 
 
 On Mon, Jun 25, 2012 at 01:41:48PM +0300, Lazaros Koromilas wrote:
  On Sun, Jun 24, 2012 at 11:23:05PM +0200, Ariane van der Steldt wrote:
   On Mon, Jun 25, 2012 at 12:09:33AM +0300, dsp wrote:
We observe the following behaviour when running nc -ul.
The server begins on a recvfrom() and when data arrives it
connects() the socket.
When the client dies , the server remains in a connected state
therefore ignoring subsequent data arriving on the port.
Is this really the intended logic for a connectionless protocol like
UDP? We proceeded to comment out the connect statement and we were
able to receive data from multiple *sessions* as expected.
Can you shed some light on the matter???
   
   Reading the man page, I think you want to add the -k option to nc.
  
  The -k option does nothing when used with -u because the readwrite()
  session cannot end.  Would the following be a reasonable change?
  It has the side effect of letting UDP packets to interleave.
  
  
  Index: nc.1
  ===
  RCS file: /cvs/src/usr.bin/nc/nc.1,v
  retrieving revision 1.60
  diff -u -p -r1.60 nc.1
  --- nc.17 Feb 2012 12:11:43 -   1.60
  +++ nc.125 Jun 2012 10:40:15 -
  @@ -119,6 +119,10 @@ is completed.
   It is an error to use this option without the
   .Fl l
   option.
  +When used together with the
  +.Fl u
  +option all UDP datagrams arriving on the port are received;
  +not just those sent by the first client to connect.
   .It Fl l
   Used to specify that
   .Nm
  Index: netcat.c
  ===
  RCS file: /cvs/src/usr.bin/nc/netcat.c,v
  retrieving revision 1.105
  diff -u -p -r1.105 netcat.c
  --- netcat.c9 Feb 2012 06:25:35 -   1.105
  +++ netcat.c25 Jun 2012 10:40:15 -
  @@ -364,9 +364,12 @@ main(int argc, char *argv[])
  if (rv  0)
  err(1, recvfrom);
   
  -   rv = connect(s, (struct sockaddr *)z, len);
  -   if (rv  0)
  -   err(1, connect);
  +   if (!kflag) {
  +   rv = connect(s, (struct sockaddr *)z,
  +   len);
  +   if (rv  0)
  +   err(1, connect);
  +   }
   
  readwrite(s);
  } else {

Re: nc(1): Report incoming connections on nc -v -l

[Christiano F. Haesbaert]

On Fri, Jun 15, 2012 at 10:43:13AM -0400, Ricky Zhou wrote:
 This patch adds a message on incoming connections when netcat is run
 with -l and -v.  Does this look like a reasonable addition?

I guess so, I don't use nc too often but it sounds reasonable to me,
your code has a few notes though, please check inline. 

 
 One thought is that report_connect might not need to err() when
 getnameinfo fails thoughts?
 
 Thanks,
 Ricky
 
 Index: netcat.c
 ===
 RCS file: /cvs/src/usr.bin/nc/netcat.c,v
 retrieving revision 1.107
 diff -u netcat.c
 --- netcat.c  1 Apr 2012 02:58:57 -   1.107
 +++ netcat.c  15 Jun 2012 12:39:49 -
 @@ -106,6 +106,7 @@
  int  unix_listen(char *);
  void set_common_sockopts(int);
  int  map_tos(char *, int *);
 +void report_connect(const struct sockaddr *, socklen_t);
  void usage(int);
 
  int
 @@ -364,6 +365,9 @@
   if (rv  0)
   err(1, connect);
 
 + if (vflag)
 + report_connect((struct sockaddr *)z, 
 len);
 +
   readwrite(s);
   } else {
   len = sizeof(cliaddr);
 @@ -371,6 +375,10 @@
   len);
   if (connfd == -1)
   err(1, accept);
 +
 + if (vflag)
 + report_connect((struct sockaddr 
 *)cliaddr, len);
 +
   readwrite(connfd);
   close(connfd);
   }
 @@ -957,6 +965,27 @@
   }
 
   return (0);
 +}
 +
 +void
 +report_connect(const struct sockaddr *sa, socklen_t salen)
 +{
 + char remote_host[NI_MAXHOST];
 + char remote_port[NI_MAXSERV];
 + int flags = NI_NUMERICSERV;
 +
 + if (nflag)
 + flags |= NI_NUMERICHOST;
 +
 + if (getnameinfo(sa, salen,
 + remote_host, sizeof(remote_host),
 + remote_port, sizeof(remote_port),
 + flags) != 0)
 + err(1, getnameinfo);

Your getnameinfo() call is not correct, you want something like this:

if ((herr = getnameinfo(addr, alen, hbuf, sizeof(hbuf),
pbuf, sizeof(pbuf), NI_NUMERICHOST|NI_NUMERICSERV)) != 0) {
if (herr == EAI_SYSTEM)
err(1, getnameinfo);
else
errx(1, getnameinfo: %s, gai_strerror(herr));

You can only trust errno if getnameinfo returned EAI_SYSTEM.


 +
 + fprintf(stderr,
 + Connection from %s %s 
 + receieved!\n, remote_host, remote_port);

Typo here: receieved.

  }
 
  void

Re: Diplay 0 connections on tcpbench after last client disconnect.

[Christiano F. Haesbaert]

On Sun, Jun 17, 2012 at 01:08:25AM +0200, Alexander Hall wrote:
 On 06/16/12 01:07, Christiano F. Haesbaert wrote:
 Hi, this behaviour is really annoying, in tcp server, when the last
 user disconnects, we don't update the status line, so it keeps showing
 1 connected user, it would be better to show Conn: 0.
 
 BEFORE
 
 sauron:haesbaert: tcpbench -s
elapsed_ms  bytes mbps   bwidth
  1000  645674612 5165.397  100.00%
 Conn:   1 Mbps: 5165.397 Peak Mbps: 5165.397 Avg Mbps: 5165.397
  2000  673254000 5386.032  100.00%
 Conn:   1 Mbps: 5386.032 Peak Mbps: 5386.032 Avg Mbps: 5386.032
 
 
 AFTER
 
 
 gimli:obj: ./tcpbench -s
elapsed_ms  bytes mbps   bwidth
  1000  314833908 2518.671  100.00%
 Conn:   1 Mbps: 2518.671 Peak Mbps: 2518.671 Avg Mbps: 2518.671
 Conn:   0 Mbps:0.000 Peak Mbps: 2518.671 Avg Mbps:0.000
 
 NAN might be more appropriate than 0.

Why is that ?

 
 
 
 
 Also, only start timer if this is the first connection, this prevents
 the display from not running while we have clients connecting (since
 it always pushes the display 1 second in the future).
 
 Are you sure about this? Looking at the code, I didn't get that
 impression (not that I claim to know much about event(3)), but
 adding a new connection every 0.5 seconds did not stop the output
 either.

You're correct, my mistake I had missed the following in
set_slice_timer():

if (evtimer_pending(mainstats.timer, NULL))
return;

Even though I think it makes more sense, no ?

 
 You might also want sth like this, for completeness:
 
 @@ -1208,6 +1210,7 @@ main(int argc, char **argv)
 } else {
 print_tcp_header();
 evtimer_set(mainstats.timer, tcp_process_slice, NULL);
 +   tcp_process_slice(0, 0, NULL);

I considered adding:
+   timerclear(tv);
+   evtimer_add(mainstats.timer, tv); 

But the output looks funky since the headers don't match the output:

./tcpbench -s
  elapsed_ms  bytes mbps   bwidth 
Conn:   0 Mbps:0.000 Peak Mbps:0.000 Avg Mbps:0.000

But I'm not too strong about it.


 }
 
 if (ptb-sflag)
 
 /Alexander
 
 
 
 Index: tcpbench.c
 ===
 RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
 retrieving revision 1.35
 diff -d -u -p -r1.35 tcpbench.c
 --- tcpbench.c   8 May 2012 01:39:58 -   1.35
 +++ tcpbench.c   15 Jun 2012 23:04:23 -
 @@ -568,7 +568,7 @@ tcp_process_slice(int fd, short event, v
  mainstats.peak_mbps = slice_mbps;
  printf(Conn: %3d Mbps: %12.3Lf Peak Mbps: %12.3Lf Avg Mbps: %12.3Lf\n,
  mainstats.nconns, slice_mbps, mainstats.peak_mbps,
 -slice_mbps / mainstats.nconns);
 +mainstats.nconns ? slice_mbps / mainstats.nconns : 0);
  mainstats.slice_bytes = 0;
 
  set_slice_timer(mainstats.nconns  0);
 @@ -657,7 +657,6 @@ tcp_server_handle_sc(int fd, short event
 
  free(sc);
  mainstats.nconns--;
 -set_slice_timer(mainstats.nconns  0);
  return;
  }
  if (ptb-vflag= 3)
 @@ -723,7 +722,8 @@ tcp_server_accept(int fd, short event, v
  event_add(sc-ev, NULL);
  TAILQ_INSERT_TAIL(sc_queue, sc, entry);
  mainstats.nconns++;
 -set_slice_timer(mainstats.nconns  0);
 +if (mainstats.nconns == 1)
 +set_slice_timer(1);
  if (ptb-vflag)
  fprintf(stderr, Accepted connection from %s, fd = %d\n,
  tmp, sc-fd);
 @@ -934,7 +934,8 @@ client_init(struct addrinfo *aitop, int
  event_add(sc-ev, NULL);
  TAILQ_INSERT_TAIL(sc_queue, sc, entry);
  mainstats.nconns++;
 -set_slice_timer(mainstats.nconns  0);
 +if (mainstats.nconns == 1)
 +set_slice_timer(1);
  }
  freeaddrinfo(aitop);
  if (aib != NULL)

Diplay 0 connections on tcpbench after last client disconnect.

[Christiano F. Haesbaert]

Hi, this behaviour is really annoying, in tcp server, when the last
user disconnects, we don't update the status line, so it keeps showing
1 connected user, it would be better to show Conn: 0.

BEFORE

sauron:haesbaert: tcpbench -s 
  elapsed_ms  bytes mbps   bwidth 
1000  645674612 5165.397  100.00% 
Conn:   1 Mbps: 5165.397 Peak Mbps: 5165.397 Avg Mbps: 5165.397
2000  673254000 5386.032  100.00% 
Conn:   1 Mbps: 5386.032 Peak Mbps: 5386.032 Avg Mbps: 5386.032


AFTER


gimli:obj: ./tcpbench -s 
  elapsed_ms  bytes mbps   bwidth 
1000  314833908 2518.671  100.00% 
Conn:   1 Mbps: 2518.671 Peak Mbps: 2518.671 Avg Mbps: 2518.671
Conn:   0 Mbps:0.000 Peak Mbps: 2518.671 Avg Mbps:0.000



Also, only start timer if this is the first connection, this prevents
the display from not running while we have clients connecting (since
it always pushes the display 1 second in the future). 


Index: tcpbench.c
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
retrieving revision 1.35
diff -d -u -p -r1.35 tcpbench.c
--- tcpbench.c  8 May 2012 01:39:58 -   1.35
+++ tcpbench.c  15 Jun 2012 23:04:23 -
@@ -568,7 +568,7 @@ tcp_process_slice(int fd, short event, v
mainstats.peak_mbps = slice_mbps;
printf(Conn: %3d Mbps: %12.3Lf Peak Mbps: %12.3Lf Avg Mbps: %12.3Lf\n,
mainstats.nconns, slice_mbps, mainstats.peak_mbps,
-   slice_mbps / mainstats.nconns); 
+   mainstats.nconns ? slice_mbps / mainstats.nconns : 0); 
mainstats.slice_bytes = 0;
 
set_slice_timer(mainstats.nconns  0);
@@ -657,7 +657,6 @@ tcp_server_handle_sc(int fd, short event
 
free(sc);
mainstats.nconns--;
-   set_slice_timer(mainstats.nconns  0);
return;
}
if (ptb-vflag = 3)
@@ -723,7 +722,8 @@ tcp_server_accept(int fd, short event, v
event_add(sc-ev, NULL);
TAILQ_INSERT_TAIL(sc_queue, sc, entry);
mainstats.nconns++;
-   set_slice_timer(mainstats.nconns  0);
+   if (mainstats.nconns == 1)
+   set_slice_timer(1);
if (ptb-vflag)
fprintf(stderr, Accepted connection from %s, fd = %d\n,
tmp, sc-fd);
@@ -934,7 +934,8 @@ client_init(struct addrinfo *aitop, int 
event_add(sc-ev, NULL);
TAILQ_INSERT_TAIL(sc_queue, sc, entry);
mainstats.nconns++;
-   set_slice_timer(mainstats.nconns  0);
+   if (mainstats.nconns == 1)
+   set_slice_timer(1);
}
freeaddrinfo(aitop);
if (aib != NULL)

Re: cwm tiling

[Christiano F. Haesbaert]

On Jun 8, 2012 9:22 PM, Jérémie Courrèges-Anglas jca+o...@wxcvbn.org
wrote:

 Antoine Jacoutot ajacou...@bsdfrog.org writes:
  I for one would love cwm to have tiling management.
  I don't care avout the alternative, they are not in base.

 Same here.


I might migrate to cwm just for the tilling.

 --
 Jérémie Courrèges-Anglas
 GPG fingerprint: 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494

Re: ftp-proxy(8): ensure nat_range_high is defined in add_nat()

[Christiano F. Haesbaert]

If no one has any objections I'd like to commit this.

On Thu, Apr 19, 2012 at 09:07:43PM -0400, Lawrence Teo wrote:
 On Wed, Apr 18, 2012 at 11:36:49PM -0400, Lawrence Teo wrote:
  This simple diff adds a check to the add_nat() function in
  ftp-proxy(8) to ensure that nat_range_high is defined before
  proceeding to create the PF NAT rule.  I think the original author
  may have intended to do this since there is an existing check for
  nat_range_low.
  
  Technically, all calls to add_nat() already use non-zero values for
  nat_range_low and nat_range_high, but I think it is still important
  to add the check as an additional safeguard in case those calls do
  change in the future.
 
 I received a reply mentioning that my original diff overran 80 columns
 columns but was otherwise ok.  Here is a revised diff that keeps the
 lines within 80 columns.
 
 Lawrence
 
 
 Index: filter.c
 ===
 RCS file: /cvs/src/usr.sbin/ftp-proxy/filter.c,v
 retrieving revision 1.17
 diff -u -p -r1.17 filter.c
 --- filter.c  6 Mar 2012 12:50:20 -   1.17
 +++ filter.c  20 Apr 2012 00:55:18 -
 @@ -71,7 +71,7 @@ add_nat(u_int32_t id, struct sockaddr *s
  u_int16_t nat_range_high)
  {
   if (!src || !dst || !d_port || !nat || !nat_range_low ||
 - (src-sa_family != nat-sa_family)) {
 + !nat_range_high || (src-sa_family != nat-sa_family)) {
   errno = EINVAL;
   return (-1);
   }

Re: ftp(1): new -s flag to specify source IP address

[Christiano F. Haesbaert]

I'm ok with the diff, can I get another ok before commiting ?

On Tue, Apr 24, 2012 at 10:53:34PM -0400, Lawrence Teo wrote:
 On Wed, Apr 18, 2012 at 11:58:26PM -0400, Lawrence Teo wrote:
  This diff adds a -s flag to ftp(1) to let the user specify the
  source IP address of the connection.  This is useful when using
  ftp(1) over VPN tunnels or when an alternate source IP is required
  to fetch a file from a FTP/HTTP/HTTPS server due to access control
  policies.
 
 I have revised this diff based on feedback from haesbaert@.  The primary
 change is to use AI_NUMERICHOST in the hints addrinfo struct that's
 passed to getaddrinfo().
 
 The resulting ftp(1) binary still passes all 48 tests from my test
 script at http://lteo.net/stuff/ftp-test
 
 Comments and more testing are welcome!
 
 Thanks,
 Lawrence
 
 
 Index: fetch.c
 ===
 RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
 retrieving revision 1.104
 diff -u -p -r1.104 fetch.c
 --- fetch.c   23 Apr 2012 21:22:02 -  1.104
 +++ fetch.c   25 Apr 2012 02:12:13 -
 @@ -179,7 +179,7 @@ url_get(const char *origline, const char
   char *hosttail, *cause = unknown, *newline, *host, *port, *buf = NULL;
   char *epath, *redirurl, *loctail;
   int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
 - struct addrinfo hints, *res0, *res;
 + struct addrinfo hints, *res0, *res, *ares = NULL;
   const char * volatile savefile;
   char * volatile proxyurl = NULL;
   char *cookie = NULL;
 @@ -198,6 +198,7 @@ url_get(const char *origline, const char
  #endif /* !SMALL */
   SSL *ssl = NULL;
   int status;
 + int save_errno;
  
   direction = received;
  
 @@ -490,6 +491,17 @@ noslash:
   goto cleanup_url_get;
   }
  
 +#ifndef SMALL
 + if (srcaddr) {
 + hints.ai_flags |= AI_NUMERICHOST;
 + error = getaddrinfo(srcaddr, NULL, hints, ares);
 + if (error) {
 + warnx(%s: %s, gai_strerror(error), srcaddr);
 + goto cleanup_url_get;
 + }
 + }
 +#endif /* !SMALL */
 +
   s = -1;
   for (res = res0; res; res = res-ai_next) {
   if (getnameinfo(res-ai_addr, res-ai_addrlen, hbuf,
 @@ -504,10 +516,28 @@ noslash:
   continue;
   }
  
 +#ifndef SMALL
 + if (srcaddr) {
 + if (ares-ai_family != res-ai_family) {
 + close(s);
 + s = -1;
 + errno = EINVAL;
 + cause = bind;
 + continue;
 + }
 + if (bind(s, ares-ai_addr, ares-ai_addrlen)  0) {
 + save_errno = errno;
 + close(s);
 + errno = save_errno;
 + s = -1;
 + cause = bind;
 + continue;
 + }
 + }
 +#endif /* !SMALL */
 +
  again:
   if (connect(s, res-ai_addr, res-ai_addrlen)  0) {
 - int save_errno;
 -
   if (errno == EINTR)
   goto again;
   save_errno = errno;
 @@ -532,6 +562,10 @@ again:
   break;
   }
   freeaddrinfo(res0);
 +#ifndef SMALL
 + if (srcaddr)
 + freeaddrinfo(ares);
 +#endif /* !SMALL */
   if (s  0) {
   warn(%s, cause);
   goto cleanup_url_get;
 Index: ftp.1
 ===
 RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
 retrieving revision 1.81
 diff -u -p -r1.81 ftp.1
 --- ftp.1 26 Jul 2010 21:31:34 -  1.81
 +++ ftp.1 25 Apr 2012 02:12:13 -
 @@ -42,10 +42,12 @@
  .Op Fl k Ar seconds
  .Op Fl P Ar port
  .Op Fl r Ar seconds
 +.Op Fl s Ar srcaddr
  .Op Ar host Op Ar port
  .Nm ftp
  .Op Fl C
  .Op Fl o Ar output
 +.Op Fl s Ar srcaddr
  .Sm off
  .No ftp:// Oo Ar user : password No @
  .Oc Ar host Oo : Ar port
 @@ -57,6 +59,7 @@
  .Op Fl C
  .Op Fl c Ar cookie
  .Op Fl o Ar output
 +.Op Fl s Ar srcaddr
  .Sm off
  .No http:// Ar host Oo : Ar port
  .Oc No / Ar file
 @@ -66,6 +69,7 @@
  .Op Fl C
  .Op Fl c Ar cookie
  .Op Fl o Ar output
 +.Op Fl s Ar srcaddr
  .Sm off
  .No https:// Ar host Oo : Ar port
  .Oc No / Ar file
 @@ -74,6 +78,7 @@
  .Nm ftp
  .Op Fl C
  .Op Fl o Ar output
 +.Op Fl s Ar srcaddr
  .Sm off
  .No file: Ar file
  .Sm on
 @@ -81,6 +86,7 @@
  .Nm ftp
  .Op Fl C
  .Op Fl o Ar output
 +.Op Fl s Ar srcaddr
  .Sm off
  .Ar host : No / Ar file Oo /
  .Oc
 @@ -220,6 +226,12 @@ if the server does not support passive c
  .It Fl r Ar seconds
  Retry to connect if failed, pausing for number of
  .Ar seconds .
 +.It Fl s Ar srcaddr
 +Use
 +.Ar srcaddr
 +on the local machine as the source 

amd64: Check cpu_vendor instead of using CPUID.

[Christiano F. Haesbaert]

There's no need for doing that somewhat strange comparison, the rest
of the code already uses cpu_vendor. 

ok ?

Index: identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.35
diff -d -u -p -r1.35 identcpu.c
--- identcpu.c  27 Mar 2012 02:23:04 -  1.35
+++ identcpu.c  22 Apr 2012 12:59:10 -
@@ -302,7 +302,6 @@ identifycpu(struct cpu_info *ci)
u_int64_t last_tsc;
u_int32_t dummy, val, pnfeatset;
u_int32_t brand[12];
-   u_int32_t vendor[4];
char mycpu_model[48];
int i, max;
char *brandstr_from, *brandstr_to;
@@ -433,11 +432,7 @@ identifycpu(struct cpu_info *ci)
 
 #endif
 
-   vendor[3] = 0;
-   CPUID(0, dummy, vendor[0], vendor[2], vendor[1]);   /* yup, 0 2 1 */
-   /* AuthenticAMD:h t u Ai t n e */
-   if (vendor[0] == 0x68747541  vendor[1] == 0x69746e65 
-   vendor[2] == 0x444d4163)/* DMAc */
+   if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);
 
if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {

Re: amd64: Check cpu_vendor instead of using CPUID.

[Christiano F. Haesbaert]

On Sun, Apr 22, 2012 at 06:36:41PM +0200, Franco Fichtner wrote:
 Just being paranoid... strncmp? 

Why ? It's a terminated string vs a string literal, what do you wanna
use as the third argument: strlen(AuthenticAmd) ? . 100% pointless.

 And how about consolidating style while at it? ! vs. == 0 - see code bits 
 below change.
 

Consolidating how ? Are you suggesting we change all strcmp calls in
kernel to use == 0 ? Please.


 Franco
 
 On 22.04.2012, at 15:12, Christiano F. Haesbaert haesba...@openbsd.org 
 wrote:
 
  There's no need for doing that somewhat strange comparison, the rest
  of the code already uses cpu_vendor. 
  
  ok ?
  
  Index: identcpu.c
  ===
  RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
  retrieving revision 1.35
  diff -d -u -p -r1.35 identcpu.c
  --- identcpu.c27 Mar 2012 02:23:04 -1.35
  +++ identcpu.c22 Apr 2012 12:59:10 -
  @@ -302,7 +302,6 @@ identifycpu(struct cpu_info *ci)
 u_int64_t last_tsc;
 u_int32_t dummy, val, pnfeatset;
 u_int32_t brand[12];
  -u_int32_t vendor[4];
 char mycpu_model[48];
 int i, max;
 char *brandstr_from, *brandstr_to;
  @@ -433,11 +432,7 @@ identifycpu(struct cpu_info *ci)
  
  #endif
  
  -vendor[3] = 0;
  -CPUID(0, dummy, vendor[0], vendor[2], vendor[1]);/* yup, 0 2 1 */
  -/* AuthenticAMD:h t u Ai t n e */
  -if (vendor[0] == 0x68747541  vendor[1] == 0x69746e65 
  -vendor[2] == 0x444d4163)/* DMAc */
  +if (!strcmp(cpu_vendor, AuthenticAMD))
 amd64_errata(ci);
  
 if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {

Re: amd64: Check cpu_vendor instead of using CPUID.

[Christiano F. Haesbaert]

On Sun, Apr 22, 2012 at 09:16:57PM +0200, Franco Fichtner wrote:
 On Apr 22, 2012, at 7:58 PM, Christiano F. Haesbaert wrote:
 
  On Sun, Apr 22, 2012 at 06:36:41PM +0200, Franco Fichtner wrote:
  Just being paranoid... strncmp? 
  
  Why ? It's a terminated string vs a string literal, what do you wanna
  use as the third argument: strlen(AuthenticAmd) ? . 100% pointless.
 
 I can see your point and yet it is being used in the line below your change. 
 Do you want to call that author's intent '100% pointless' as well just for 
 the sake of winning an argument or do you simply not care about the depth and 
 inherent wisdom of the code base you are working on?
 

You rush into conclusions, cpu_model is different, he actually wants
the first 5 bytes to evaluate to Intel, not the whole string, which
could be something like:

hw.model=Intel(R) Xeon(R) CPU E31220 @ 3.10GHz


  
  And how about consolidating style while at it? ! vs. == 0 - see code 
  bits below change.
  
  Consolidating how ? Are you suggesting we change all strcmp calls in
  kernel to use == 0 ? Please.
 
 Personally, I don't care either way, but it's bad style to ignore the context 
 and change styles. It makes the code harder to read, understand and maintain. 
 Take a look. Ok?
 

You care enough to send an email without even checking the other uses,
if you did, you'll see that !strcmp is more consistent for this case
than strncmp.

*You* are ignoring the context and trying to change styles.

  +if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);
  
if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {
 
 
 Franco

Re: queue(3) TAILQ example causes compiler warning

[Christiano F. Haesbaert]

On Fri, Mar 02, 2012 at 12:46:15AM -0500, Lawrence Teo wrote:
 The following example code in the queue(3) man page to delete all
 elements in a tail queue generates a warning in gcc and clang.
 
   while (np = TAILQ_FIRST(head)) {
   TAILQ_REMOVE(head, np, entries);
   free(np);
   }
 
 Here's a demo:
 
 ===BEGIN===
 $ cat tailq.c
 #include sys/queue.h
 #include stdlib.h
 
 struct entry {
   int foo;
   TAILQ_ENTRY(entry) entries;
 } *np;
 
 TAILQ_HEAD(tailhead, entry) head;
 
 int
 main(int argc, char *argv[])
 {
   TAILQ_INIT(head);
 
   while (np = TAILQ_FIRST(head)) {
   TAILQ_REMOVE(head, np, entries);
   free(np);
   }
 
   return 0;
 }
 $ gcc -O2 -Wall -o tailq tailq.c
 tailq.c: In function 'main':
 tailq.c:16: warning: suggest parentheses around assignment used as truth value
 ===END===
 
 The following diff fixes the example to prevent the warning from being
 triggered.
 
 Comments?
 
 Thanks,
 Lawrence
 
 
 Index: queue.3
 ===
 RCS file: /cvs/src/share/man/man3/queue.3,v
 retrieving revision 1.54
 diff -u -p -r1.54 queue.3
 --- queue.3   11 Jan 2012 19:26:34 -  1.54
 +++ queue.3   2 Mar 2012 05:29:24 -
 @@ -966,7 +966,7 @@ TAILQ_FOREACH(np, head, entries)
  for (np = n2; np != NULL; np = TAILQ_NEXT(np, entries))
   np- ...
   /* Delete. */
 -while (np = TAILQ_FIRST(head)) {
 +while ((np = TAILQ_FIRST(head)) != NULL) {
   TAILQ_REMOVE(head, np, entries);
   free(np);
  }
 

If no one opposes, I'll commit this.

Re: md5: new -C flag to skip non-existent files in checklist

[Christiano F. Haesbaert]

On 23 February 2012 08:46, Mark Kettenis mark.kette...@xs4all.nl wrote:
 Date: Thu, 23 Feb 2012 10:48:17 +0100
 From: Marc Espie es...@nerim.net

 On Thu, Feb 23, 2012 at 08:16:20AM +, Nicholas Marriott wrote:
  Hi
 
  Is this really much more useful than 2/dev/null?
 
 Note the FAILED that vanish.

 I'm not sure it's not bloat anyways.

 None of the other BSDs seem to implement an option like this (and
 md5(1) is pretty much a BSDism; Linux and Solaris don't have it).

 Our policy tends to be to not add non-standard options to our
 utilities to encourage people to write portable shell scripts.  Of
 course we make exceptions if a strong case can be made for the added
 functionality.  But personally I don't think that's the case here.


Since this is just a small specialization of the -c case, how about an
environment var ?

/me ducks

Re: Setting the IPv6 Flow Label for TCP connections

[Christiano F. Haesbaert]

On 15 February 2012 13:11, Fernando Gont ferna...@gont.com.ar wrote:
 Folks,

 There seems to be a bug in the setting of the IPv6 Flow Label for TCP
 connections.

 When an incoming connection is received, the SYN/ACK is always sent with
 the FL set to zero.

 It seems that syn_cache_respond() and syn_cache_add() should be patched,
 together with the SYN-cookies generation function (as you need to
 remember the FL you used for the SYN/ACK, since the FL is supposed to
 remain constant during the life of the connection).

 Might be able to produce a patch in a couple of weeks, but mentioned it
 in the event anyone else finds some cycles before I do.


I'm a bit ignorant about IPv6, but that is the same case for TCCLASS,
I had fixed the TOS for ipv4 and I had an untested diff for ipv6. The
thing is

On 15 February 2012 13:11, Fernando Gont ferna...@gont.com.ar wrote:
 Folks,

 There seems to be a bug in the setting of the IPv6 Flow Label for TCP
 connections.

 When an incoming connection is received, the SYN/ACK is always sent with
 the FL set to zero.

 It seems that syn_cache_respond() and syn_cache_add() should be patched,
 together with the SYN-cookies generation function (as you need to
 remember the FL you used for the SYN/ACK, since the FL is supposed to
 remain constant during the life of the connection).

 Might be able to produce a patch in a couple of weeks, but mentioned it
 in the event anyone else finds some cycles before I do.


I'm a bit ignorant about IPv6, but that is the same case for TCCLASS,
I had fixed the TOS for ipv4 and I had an untested diff for ipv6. The
thing is struct ip6_pktopts *opt is NULL in ip6_output() from
syn_cache_respond():

error = ip6_output(m, NULL /*XXX*/, (struct route_in6 *)ro, 0,
(struct ip6_moptions *)0, NULL, NULL);

I didn't want to touch it much since I'm not fully aware of the consequences.

 Thanks,
 --
 Fernando Gont
 e-mail: ferna...@gont.com.ar || fg...@si6networks.com
 PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1




 Thanks,
 --
 Fernando Gont
 e-mail: ferna...@gont.com.ar || fg...@si6networks.com
 PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

Re: fix incorrect document titles in several man pages

[Christiano F. Haesbaert]

On 11 February 2012 13:56, Jason McIntyre j...@cava.myzen.co.uk wrote:
 On Sat, Feb 11, 2012 at 10:26:18AM -0200, Christiano F. Haesbaert wrote:
 Reads fine to me, any objections ?

 jmc ?


 i had just filed this under look at after lock, but if you want to
 take it on, that's fine.


No problem, let us wait for the unlock then, I was only making sure
this wouldn't die.

 note there is no rpcauth function, so that page should probably be renamed
 (to something which does exist).

 except for that niggle, the rest of the diff looks correct.

 jmc

 On 10 February 2012 03:50, Lawrence Teo l...@lteo.net wrote:
  This diff fixes several man pages that have incorrect document titles
  (.Dt).
 
  Lawrence
 
 
  Index: src/bin/md5/sha256.1
  ===
  RCS file: /cvs/src/bin/md5/sha256.1,v
  retrieving revision 1.3
  diff -u -p -r1.3 sha256.1
  --- src/bin/md5/sha256.1 ? ? ? ?3 Sep 2010 09:53:20 - ? ? ? 1.3
  +++ src/bin/md5/sha256.1 ? ? ? ?10 Feb 2012 05:41:09 -
  @@ -19,7 +19,7 @@
  ?.\ Materiel Command, USAF, under agreement number F39502-99-1-0512.
  ?.\
  ?.Dd $Mdocdate: September 3 2010 $
  -.Dt SHA1 1
  +.Dt SHA256 1
  ?.Os
  ?.Sh NAME
  ?.Nm sha256
  Index: src/lib/libc/rpc/rpcauth.3
  ===
  RCS file: /cvs/src/lib/libc/rpc/rpcauth.3,v
  retrieving revision 1.15
  diff -u -p -r1.15 rpcauth.3
  --- src/lib/libc/rpc/rpcauth.3 ?7 Sep 2010 19:52:37 - ? ? ? 1.15
  +++ src/lib/libc/rpc/rpcauth.3 ?10 Feb 2012 05:41:09 -
  @@ -31,7 +31,7 @@
  ?.\ ? OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  ?.\
  ?.Dd $Mdocdate: September 7 2010 $
  -.Dt RPC 3
  +.Dt RPCAUTH 3
  ?.Os
  ?.Sh NAME
  ?.Nm auth_destroy ,
  Index: src/lib/libc/time/wcsftime.3
  ===
  RCS file: /cvs/src/lib/libc/time/wcsftime.3,v
  retrieving revision 1.1
  diff -u -p -r1.1 wcsftime.3
  --- src/lib/libc/time/wcsftime.3 ? ? ? ?10 Oct 2011 14:40:25 - ? ? ?1.1
  +++ src/lib/libc/time/wcsftime.3 ? ? ? ?10 Feb 2012 05:41:09 -
  @@ -14,7 +14,7 @@
  ?.\ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  ?.\
  ?.Dd $Mdocdate: October 10 2011 $
  -.Dt STRFTIME 3
  +.Dt WCSFTIME 3
  ?.Os
  ?.Sh NAME
  ?.Nm wcsftime
  Index: src/lib/libm/man/sin.3
  ===
  RCS file: /cvs/src/lib/libm/man/sin.3,v
  retrieving revision 1.13
  diff -u -p -r1.13 sin.3
  --- src/lib/libm/man/sin.3 ? ? ?7 Jul 2011 01:34:52 - ? ? ? 1.13
  +++ src/lib/libm/man/sin.3 ? ? ?10 Feb 2012 05:41:09 -
  @@ -29,7 +29,7 @@
  ?.\ ? ? from: @(#)cos.3 ? ? ? ?5.1 (Berkeley) 5/2/91
  ?.\
  ?.Dd $Mdocdate: July 7 2011 $
  -.Dt COS 3
  +.Dt SIN 3
  ?.Os
  ?.Sh NAME
  ?.Nm sin ,
  Index: src/libexec/login_krb5/login_krb5.8
  ===
  RCS file: /cvs/src/libexec/login_krb5/login_krb5.8,v
  retrieving revision 1.16
  diff -u -p -r1.16 login_krb5.8
  --- src/libexec/login_krb5/login_krb5.8 1 Feb 2012 17:32:59 - ? ? ? 
  1.16
  +++ src/libexec/login_krb5/login_krb5.8 10 Feb 2012 05:41:09 -
  @@ -15,7 +15,7 @@
  ?.\ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  ?.\
  ?.Dd $Mdocdate: February 1 2012 $
  -.Dt LOGIN_KERBEROS 8
  +.Dt LOGIN_KRB5 8
  ?.Os
  ?.Sh NAME
  ?.Nm login_krb5
  Index: src/usr.bin/bgplg/bgplgsh.8
  ===
  RCS file: /cvs/src/usr.bin/bgplg/bgplgsh.8,v
  retrieving revision 1.5
  diff -u -p -r1.5 bgplgsh.8
  --- src/usr.bin/bgplg/bgplgsh.8 13 Oct 2010 18:56:03 - ? ? ?1.5
  +++ src/usr.bin/bgplg/bgplgsh.8 10 Feb 2012 05:41:09 -
  @@ -15,7 +15,7 @@
  ?.\ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  ?.\
  ?.Dd $Mdocdate: October 13 2010 $
  -.Dt BGPLG 8
  +.Dt BGPLGSH 8
  ?.Os
  ?.Sh NAME
  ?.Nm bgplgsh

Re: pf(4) man page: clarify DIOCNATLOOK, new example program

[Christiano F. Haesbaert]

Commited, thanks :).

On 8 February 2012 01:19, Lawrence Teo l...@lteo.net wrote:
 On Mon, Feb 06, 2012 at 09:12:35PM -0500, Lawrence Teo wrote:
 This is a revised version of a diff that sthen@ and I worked on back in
 May 2011. The original thread and history is at
 http://marc.info/?t=13043899683r=1w=2 if anyone is interested.

 To summarize, the current pf(4) man page's example program uses
 DIOCNATLOOK. But Stuart pointed out in the above thread that divert-to
 has existed since OpenBSD 4.4, so the better approach is to fetch the
 destination address using getsockname() instead of DIOCNATLOOK. This
 also does not require access to /dev/pf.

 In this diff, Stuart added a paragraph to the DIOCNATLOOK description in
 the pf(4) man page to mention divert-to, while I replaced the example
 program with one that demonstrates DIOCGETLIMIT instead. I picked
 DIOCGETLIMIT because it will work as-is on any OpenBSD system
 regardless of their PF rules.

 Here's another revision that addresses jmc@'s comments (thank you
 Jason!). It uses .Dv where appropriate and adds a missing the in the
 DIOCNATLOOK paragraph. The example program remains unchanged.

 Any further comments would be appreciated.

 Thank you,
 Lawrence


 Index: pf.4
 ===
 RCS file: /cvs/src/share/man/man4/pf.4,v
 retrieving revision 1.73
 diff -u -p -r1.73 pf.4
 --- pf.423 Dec 2011 17:00:47 -  1.73
 +++ pf.48 Feb 2012 03:11:39 -
 @@ -314,6 +314,22 @@ struct pfioc_natlook {
u_int8_t direction;
  };
  .Ed
 +.Pp
 +This was primarily used to support transparent proxies with rdr-to rules.
 +New proxies should use divert-to rules instead.
 +These do not require access to the privileged
 +.Pa /dev/pf
 +device and preserve the original destination address for
 +.Xr getsockname 2 .
 +For
 +.Dv SOCK_DGRAM
 +sockets, the
 +.Xr ip 4
 +socket options
 +.Dv IP_RECVDSTADDR
 +and
 +.Dv IP_RECVDSTPORT
 +can be used to retrieve the destination address and port.
  .It Dv DIOCSETDEBUG Fa u_int32_t *level
  Set the debug level.
  See the
 @@ -990,68 +1006,81 @@ packet filtering device.
  .El
  .Sh EXAMPLES
  The following example demonstrates how to use the
 -.Dv DIOCNATLOOK
 -command to find the internal host/port of a NATed connection:
 +.Dv DIOCGETLIMIT
 +command to show the hard limit of a memory pool used by the packet filter:
  .Bd -literal
  #include sys/types.h
  #include sys/socket.h
  #include sys/ioctl.h
  #include sys/fcntl.h
  #include net/if.h
 -#include netinet/in.h
  #include net/pfvar.h
 -#include err.h
  #include stdio.h
  #include stdlib.h
 +#include string.h
 +#include err.h

 -u_int32_t
 -read_address(const char *s)
 -{
 -   int a, b, c, d;
 -
 -   sscanf(s, %i.%i.%i.%i, a, b, c, d);
 -   return htonl(a  24 | b  16 | c  8 | d);
 -}
 +static const struct {
 +   const char  *name;
 +   int index;
 +} pf_limits[] = {
 +   { states, PF_LIMIT_STATES },
 +   { src-nodes,  PF_LIMIT_SRC_NODES },
 +   { frags,  PF_LIMIT_FRAGS },
 +   { tables, PF_LIMIT_TABLES },
 +   { table-entries,  PF_LIMIT_TABLE_ENTRIES },
 +   { NULL, 0 }
 +};

  void
 -print_address(u_int32_t a)
 +usage(void)
  {
 -   a = ntohl(a);
 -   printf(%d.%d.%d.%d, a  24  255, a  16  255,
 -   a  8  255, a  255);
 +   extern char *__progname;
 +   int i;
 +
 +   fprintf(stderr, usage: %s [, __progname);
 +   for (i = 0; pf_limits[i].name; i++)
 +   fprintf(stderr, %s%s, (i  0 ? | : ),
pf_limits[i].name);
 +   fprintf(stderr, ]\en);
 +   exit(1);
  }

  int
  main(int argc, char *argv[])
  {
 -   struct pfioc_natlook nl;
 -   int dev;
 -
 -   if (argc != 5) {
 -   printf(%s gwy addr gwy port ext addr ext port\en,
 -   argv[0]);
 -   return 1;
 +   struct pfioc_limit pl;
 +   int i, dev;
 +   int pool_index = -1;
 +
 +   if (argc != 2)
 +   usage();
 +
 +   for (i = 0; pf_limits[i].name; i++)
 +   if (!strcmp(argv[1], pf_limits[i].name)) {
 +   pool_index = pf_limits[i].index;
 +   break;
 +   }
 +
 +   if (pool_index == -1) {
 +   warnx(no such memory pool: %s, argv[1]);
 +   usage();
}

dev = open(/dev/pf, O_RDWR);
if (dev == -1)
err(1, open(\e/dev/pf\e) failed);

 -   memset(nl, 0, sizeof(struct pfioc_natlook));
 -   nl.saddr.v4.s_addr  = read_address(argv[1]);
 -   nl.sport= htons(atoi(argv[2]));
 -   nl.daddr.v4.s_addr  = read_address(argv[3]);
 -   nl.dport= htons(atoi(argv[4]));
 -   nl.af   = AF_INET;
 -   nl.proto= IPPROTO_TCP;
 -   nl.direction= PF_IN;
 -
 -   if 

Re: readdir man page

[Christiano F. Haesbaert]

On 3 February 2012 02:50, Philip Guenther guent...@gmail.com wrote:
 On Thu, Feb 2, 2012 at 5:29 AM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
 On 2 February 2012 10:13, Laurence Tratt lau...@tratt.net wrote:
 To my surprise (and a couple of hours debugging later), readdir does not
 necessarily set errno even if NULL is returned. This is rather confusing
 because if NULL is returned two things might have happened:

  1) The end of the directory has been reached in a normal fashion; errno
 won't have been touched in any way.

  2) One of the errors associated with getdirentries might have occurred,
 in which case errno will have been set to a specific value.

 So any user who writes:

  if (readdir(dirp) == NULL) {
  if (errno == ...)
 ...
  }

 without setting errno to 0 beforehand will get a nasty shock from time to
 time (in my case, I was finding EAGAIN was leaking through from a failed
 flock call).

 Yep.  This is how readdir() has always behaved and was standardized by
POSIX.


 The current man page doesn't claim that errno is always set but one might
 reasonably assume that it is: certainly, it seems a horribly easy way of
 introducing bugs (at least for idiots such as myself). Similar issues
(e.g.

 I beg to disagree on this, yes, the interface is horrible:

 The readdir() function returns a pointer to the next directory entry in
 the named directory stream dirp.  It returns NULL upon reaching the
end
 of the directory or detecting an invalid seekdir() operation.

 So it says nothing about errno, but then, in the end:

The readdir() and readdir_r() functions may also fail and set errno for
 any of the errors specified for the routine getdirentries(2).

 Actually the reasonable thing to me is errno can't be trusted from
 readdir(), since may is not good enough.
 And if that is the case, errno may actually be set to anything related
 to internals.

 The use of 'may' is a standard form for the ERRORS section of
 manpages.  What you may be missing is that the 'may' applies to the
 entire second half of the sentence as a whole.  The precedence is
 similar to:
The readdir() function may also ( fail and set errno ... ).


Yes, you nailed, I didn't get that if readdir() fails it always set errno.

 I.e., if it fails, it sets errno.


 Although I like your intention, I think the problem is in the later part:

  The readdir() and readdir_r() functions may also fail and set errno for
 any of the errors specified for the routine getdirentries(2).

 Maybe this could be clarified ?

 Lets see what others think.

 I'll propose a third option: I think the directory(3) manpage would be
 clearer if the DESCRIPTION section was unwoven to have separate
 DESCRIPTION and RETURN VALUES section.  Explicit wording that errno is
 not set when end of directory is reached and that programs that want
 to tell apart end-of-directory and errors must set errno to zero
 should be added.


Agreed

 I also think readdir() should set errno if it detects an invalid
 seekdir().  EINVAL seems correct.


 Philip Guenther

Re: readdir man page

[Christiano F. Haesbaert]

On 3 February 2012 03:29, Philip Guenther guent...@gmail.com wrote:
 On Thu, 2 Feb 2012, Philip Guenther wrote:
 I also think readdir() should set errno if it detects an invalid
 seekdir().  EINVAL seems correct.

 Here's a diff for this bit.

 oks?


 Philip Guenther


 Index: gen/readdir.c
 ===
 RCS file: /cvs/src/lib/libc/gen/readdir.c,v
 retrieving revision 1.15
 diff -u -p -r1.15 readdir.c
 --- gen/readdir.c   18 Nov 2009 07:43:22 -  1.15
 +++ gen/readdir.c   3 Feb 2012 05:21:58 -
 @@ -29,6 +29,7 @@
  */

  #include dirent.h
 +#include errno.h
  #include thread_private.h

  /*
 @@ -52,12 +53,14 @@ _readdir_unlocked(DIR *dirp, struct dire
return (-1);
}
dp = (struct dirent *)(dirp-dd_buf + dirp-dd_loc);
 -   if ((long)dp  03)  /* bogus pointer check */
 -   return (-1);
 -   if (dp-d_reclen = 0 ||
 -   dp-d_reclen  dirp-dd_len + 1 - dirp-dd_loc)
 +   if ((long)dp  03 ||/* bogus pointer check */
 +   dp-d_reclen = 0 ||
 +   dp-d_reclen  dirp-dd_len + 1 - dirp-dd_loc) {
 +   errno = EINVAL;
return (-1);
 +   }
dirp-dd_loc += dp-d_reclen;
 +
/*
 * When called from seekdir(), we let it decide on
 * the end condition to avoid overshooting: the next

ok by me, the other case is assured by the above getdirentries() call.

Re: readdir man page

[Christiano F. Haesbaert]

On 2 February 2012 10:13, Laurence Tratt lau...@tratt.net wrote:
 To my surprise (and a couple of hours debugging later), readdir does not
 necessarily set errno even if NULL is returned. This is rather confusing
 because if NULL is returned two things might have happened:

  1) The end of the directory has been reached in a normal fashion; errno
 won't have been touched in any way.

  2) One of the errors associated with getdirentries might have occurred,
 in which case errno will have been set to a specific value.

 So any user who writes:

  if (readdir(dirp) == NULL) {
  if (errno == ...)
 ...
  }

 without setting errno to 0 beforehand will get a nasty shock from time to
 time (in my case, I was finding EAGAIN was leaking through from a failed
 flock call).

 The current man page doesn't claim that errno is always set but one might
 reasonably assume that it is: certainly, it seems a horribly easy way of
 introducing bugs (at least for idiots such as myself). Similar issues (e.g.

I beg to disagree on this, yes, the interface is horrible:

The readdir() function returns a pointer to the next directory entry in
 the named directory stream dirp.  It returns NULL upon reaching the end
 of the directory or detecting an invalid seekdir() operation.

So it says nothing about errno, but then, in the end:

The readdir() and readdir_r() functions may also fail and set errno for
 any of the errors specified for the routine getdirentries(2).

Actually the reasonable thing to me is errno can't be trusted from
readdir(), since may is not good enough.
And if that is the case, errno may actually be set to anything related
to internals.

 with strtoul) are documented, so it seems like a good idea to at least hint
 to users that this could happen. The following man page diff attempts to
 address this, although there may well be cleaner ways of achieving the same
 end.


 Laurie
 --
 Personal
http://tratt.net/laurie/
 The Converge programming language
 http://convergepl.org/
   https://github.com/ltratt  http://twitter.com/laurencetratt



 --- directory.3.origThu Feb  2 11:51:38 2012
 +++ directory.3 Thu Feb  2 11:58:36 2012
 @@ -109,6 +109,12 @@
  upon reaching the end of the directory or detecting an invalid
  .Fn seekdir
  operation.
 +.Va errno
 +may not be set even if
 +.Dv NULL
 +is returned and therefore must be cleared before calling
 +.Fn readdir
 +if its value is required afterwards.

Although I like your intention, I think the problem is in the later part:

  The readdir() and readdir_r() functions may also fail and set errno for
 any of the errors specified for the routine getdirentries(2).

Maybe this could be clarified ?

Lets see what others think.

  .Pp
  The
  .Fn readdir_r

Re: tcpbench: setpgid

[Christiano F. Haesbaert]

On 31 January 2012 13:50, Erik Lax e...@halon.se wrote:
 Hi,

 I noticed that tcpbench tries to setpgid() for no obvious reason (to me)
 since it's not forked anymore. Previously, 2 years ago it was fork()ed and
 utilized killpg() etc. Could this be a leftover? I'm running into issues
 spawning tcpbench since my parent process uses setsid().

 I attached a patch (against 5.0) to remove the code :)


You're correct nice catch :-).


 Regards
 Erik Lax

 Index: usr.bin/tcpbench/tcpbench.c
 ===
 RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
 retrieving revision 1.22
 diff -u -r1.22 tcpbench.c
 --- usr.bin/tcpbench/tcpbench.c 21 Jun 2011 17:31:07 -  1.22
 +++ usr.bin/tcpbench/tcpbench.c 31 Jan 2012 10:27:50 -
 @@ -703,9 +703,6 @@
struct event *ev;
nfds_t lnfds;

 -   if (setpgid(0, 0) == -1)
 -   err(1, setpgid);
 -
lnfds = 0;
for (ai = aitop; ai != NULL; ai = ai-ai_next) {
saddr_ntop(ai-ai_addr, ai-ai_addrlen, tmp, sizeof(tmp));

tcpbench: Don't use floating precision while printing PPS in UDP mode.

[Christiano F. Haesbaert]

Moin, 

I think it doesn't make sense for PPS, also, the rounding is wrong so we always 
get .000

This makes the output go from:
Elapsed:   11685 Mbps:  63.602 Peak Mbps:  63.602 Rx PPS:   5401.000

to:
Elapsed:   15000 Mbps:1227.177 Peak Mbps:1245.347 Tx PPS:  104210

ok ?

Index: tcpbench.c
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
retrieving revision 1.30
diff -d -u -p -w -r1.30 tcpbench.c
--- tcpbench.c  26 Jan 2012 17:53:53 -  1.30
+++ tcpbench.c  29 Jan 2012 02:47:38 -
@@ -571,8 +571,8 @@ static void
 udp_process_slice(int fd, short event, void *v_sc)
 {
struct statctx *sc = v_sc;
-   unsigned long long total_elapsed, since_last;
-   long double slice_mbps, pps;
+   unsigned long long total_elapsed, since_last, pps;
+   long double slice_mbps;
struct timeval t_cur, t_diff;
 
if (clock_gettime_tv(CLOCK_MONOTONIC, t_cur) == -1)
@@ -586,7 +586,7 @@ udp_process_slice(int fd, short event, v
pps = (sc-udp_slice_pkts * 1000) / since_last;
if (slice_mbps  mainstats.peak_mbps)
mainstats.peak_mbps = slice_mbps;
-   printf(Elapsed: %11llu Mbps: %11.3Lf Peak Mbps: %11.3Lf %s PPS: 
%10.3Lf\n,
+   printf(Elapsed: %11llu Mbps: %11.3Lf Peak Mbps: %11.3Lf %s PPS: 
%7llu\n,
total_elapsed, slice_mbps, mainstats.peak_mbps,
ptb-sflag ? Rx : Tx, pps);

Re: tcpbench: add timer command-line option

[Christiano F. Haesbaert]

On 26 January 2012 12:30, Henning Brauer lists-openbsdt...@bsws.de wrote:
 * Lawrence Teo l...@lteo.net [2012-01-26 03:13]:
 This diff adds a timer to tcpbench as a command-line option (-t) so
 that it is possible to stop the tcpbench client after a certain number
 of seconds. This makes it easier to use tcpbench as part of a script.

 Any comments and feedback would be appreciated.

 idea is sound, code is sane - who wants to give the second ok?


I didn't test it yet but I like the idea also, and the code looks good.

ok by me.

New puc(4) device: quad rs232 found on ebay.

[Christiano F. Haesbaert]

I bought this cheap chinese device from ebay, after tweaking a bit
with puc and some help from sthen@, it works fine, tested devices
tty02 and tty03. 

The thing is I've no idea which vendor it is, noir which device it is,
the only thing written on the card is Sun 1040. The vendor-id and
device-id are not listed in any pci database. 

Unfortunately I binned the driver cd which could contain some
information. Freebsd folks have something similar:

http://groups.google.com/group/mailing.freebsd.bugs/browse_thread/thread/41bedcffd4cbe4ef/b713700568c46002?show_docid=b713700568c46002

Should we leave the bare hexa numbers ? I'd like to get this in.

Index: pci/pucdata.c
===
RCS file: /cvs/src/sys/dev/pci/pucdata.c,v
retrieving revision 1.79
diff -d -u -p -w -r1.79 pucdata.c
--- pci/pucdata.c   2 Jan 2012 11:07:02 -   1.79
+++ pci/pucdata.c   7 Jan 2012 21:41:18 -
@@ -1892,6 +1892,16 @@ const struct puc_device_description puc_
{ PUC_COM_POW2(0), 0x10, 0x },
{ PUC_COM_POW2(0), 0x14, 0x },
},
+   },
+   {   /* Sun 1040 board */
+   {   0x5372, 0x6873, 0, 0 },
+   {   0x, 0x, 0, 0 },
+   {
+   { PUC_COM_POW2(0), 0x10, 0x },
+   { PUC_COM_POW2(0), 0x14, 0x },
+   { PUC_COM_POW2(0), 0x18, 0x },
+   { PUC_COM_POW2(0), 0x1c, 0x },
+   },
}
 };
 int puc_ndevs = nitems(puc_devs);


console is /pci@1f,0/pci@1,1/ebus@1/se@14,40:a
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2012 OpenBSD. All rights reserved.  http://www.OpenBSD.org

OpenBSD 5.0-current (GENERIC) #7: Sat Jan  7 18:34:17 BRST 2012
root@gandalf.midearth:/usr/src/sys/arch/sparc64/compile/GENERIC
real mem = 268435456 (256MB)
avail mem = 251625472 (239MB)
mainbus0 at root: Sun Ultra 5/10 UPA/PCI (UltraSPARC-IIi 400MHz)
cpu0 at mainbus0: SUNW,UltraSPARC-IIi (rev 9.1) @ 400 MHz
cpu0: physical 16K instruction (32 b/l), 16K data (32 b/l), 2048K external (64 
b/l)
psycho0 at mainbus0 addr 0xfffc4000: SUNW,sabre, impl 0, version 0, ign 7c0
psycho0: bus range 0-3, PCI bus 0
psycho0: dvma map c000-dfff
pci0 at psycho0
ppb0 at pci0 dev 1 function 1 Sun Simba PCI-PCI rev 0x13
pci1 at ppb0 bus 1
ebus0 at pci1 dev 1 function 0 Sun PCIO EBus2 rev 0x01
auxio0 at ebus0 addr 726000-726003, 728000-728003, 72a000-72a003, 
72c000-72c003, 72f000-72f003
power0 at ebus0 addr 724000-724003 ivec 0x25
SUNW,pll at ebus0 addr 504000-504002 not configured
sab0 at ebus0 addr 40-40007f ivec 0x2b: rev 3.2
sabtty0 at sab0 port 0: console
sabtty1 at sab0 port 1
comkbd0 at ebus0 addr 3083f8-3083ff ivec 0x29: no keyboard
comms0 at ebus0 addr 3062f8-3062ff ivec 0x2a
wsmouse0 at comms0 mux 0
lpt0 at ebus0 addr 3043bc-3043cb, 30015c-30015d, 70-7f ivec 0x22: polled
fdthree at ebus0 addr 3023f0-3023f7, 706000-70600f, 72-720003 ivec 0x27 
not configured
clock1 at ebus0 addr 0-1fff: mk48t59
flashprom at ebus0 addr 0-f not configured
audioce0 at ebus0 addr 20-2000ff, 702000-70200f, 704000-70400f, 
722000-722003 ivec 0x23 ivec 0x24: nvaddrs 0
audio0 at audioce0
hme0 at pci1 dev 1 function 1 Sun HME rev 0x01: ivec 0x7e1, address 
00:03:ba:08:72:5a
nsphy0 at hme0 phy 1: DP83840 10/100 PHY, rev. 1
machfb0 at pci1 dev 2 function 0 ATI Mach64 rev 0x5c
machfb0: ATY,GT-C, 1152x900
wsdisplay0 at machfb0 mux 1
wsdisplay0: screen 0 added (std, sun emulation)
pciide0 at pci1 dev 3 function 0 CMD Technology PCI0646 rev 0x03: DMA, 
channel 0 configured to native-PCI, channel 1 configured to native-PCI
pciide0: using ivec 0x7e0 for native-PCI interrupt
wd0 at pciide0 channel 0 drive 0: QUANTUM FIREBALLlct20 40
wd0: 8-sector PIO, LBA, 38172MB, 78177792 sectors
wd0(pciide0:0:0): using PIO mode 4, DMA mode 2
atapiscsi0 at pciide0 channel 1 drive 0
scsibus0 at atapiscsi0: 2 targets
cd0 at scsibus0 targ 0 lun 0: LG, CD-ROM CRD-8483B, 1.02 ATAPI 5/cdrom 
removable
cd0(pciide0:1:0): using PIO mode 4, DMA mode 2
ppb1 at pci0 dev 1 function 0 Sun Simba PCI-PCI rev 0x13
pci2 at ppb1 bus 2
pciide1 at pci2 dev 1 function 0 CMD Technology SiI3114 SATA rev 0x02: DMA
pciide1: using ivec 0x7d0 for native-PCI interrupt
pciide1: port 2: device present, speed: 1.5Gb/s
wd1 at pciide1 channel 2 drive 0: SAMSUNG HD154UI
wd1: 16-sector PIO, LBA48, 1430799MB, 2930277168 sectors
wd1(pciide1:2:0): using BIOS timings, Ultra-DMA mode 2
puc0 at pci2 dev 2 function 0 unknown vendor 0x5372 product 0x6873 rev 0x01: 
ports: 4 com
com0 at puc0 port 0 ivec 0x7d4: st16650, 32 byte fifo
com0: probed fifo depth: 0 bytes
com1 at puc0 port 1 ivec 0x7d4: ns16550a, 16 byte fifo
com2 at puc0 port 2 ivec 0x7d4: ns16550a, 16 byte fifo
com2: probed fifo depth: 0 bytes
com3 at puc0 port 3 ivec 0x7d4: ns16550a, 16 byte fifo
com3: probed fifo depth: 0 bytes
ppb2 at pci2 dev 3 function 

Re: New puc(4) device: quad rs232 found on ebay.

[Christiano F. Haesbaert]

Forgot the pcidump:

2:2:0: unknown unknown
0x: Vendor ID: 5372 Product ID: 6873
0x0004: Command: 0041 Status ID: 0400
0x0008: Class: 07 Subclass: 80 Interface: 00 Revision: 01
0x000c: BIST: 00 Header Type: 00 Latency Timer: 00 Cache Line Size: 08
0x0010: BAR io addr: 0x1030/0x0008
0x0014: BAR io addr: 0x1038/0x0008
0x0018: BAR io addr: 0x1040/0x0008
0x001c: BAR io addr: 0x1048/0x0008
0x0020: BAR io addr: 0x1050/0x0008
0x0024: BAR io addr: 0x1060/0x0010
0x0028: Cardbus CIS: 
0x002c: Subsystem Vendor ID: 1000 Product ID: 0004
0x0030: Expansion ROM Base Address: 
0x0038: 
0x003c: Interrupt Pin: 01 Line: ff Min Gnt: 00 Max Lat: 00
0x: 68735372 0441 0781 0008
0x0010: 1031 1039 1041 1049
0x0020: 1051 1061  00041000
0x0030:    01ff
0x0040:    
0x0050:    
0x0060:    
0x0070:    
0x0080:    
0x0090:    
0x00a0:    
0x00b0:    
0x00c0:    
0x00d0:    
0x00e0:    
0x00f0:    

Re: [BUG] pfctl(8) silently accept directories as config files

[Christiano F. Haesbaert]

On 2 January 2012 06:58, Henning Brauer lists-openbsdt...@bsws.de wrote:
 what next? having pfctl whine about an empty config file?

 * Stephane A. Sezer s...@cd80.net [2012-01-02 09:36]:
 Hi,

 I found a strange behavior in pfctl(8) which looks like a bug.

 When given a directory as input (either with the `-f` flag, or with the
 `include` directive in a config file), pfctl(8) does not emit any
 warning and silently accepts the given input.

 I suppose this is not the intended behavior so here is a patch that
 fixes the issue.


Even if it is the case, testing against directory doesn't seem like
the right thing, the test should be done against a regular file, what
if you pass a block device ? another check ?

You're only dealing with a very special case here, anyway, I agree
with Henning, it seems too much.


 Regards,

 --
 Stephane A. Sezer

 Index: sbin/pfctl/parse.y
 ===
 RCS file: /cvs/src/sbin/pfctl/parse.y,v
 retrieving revision 1.613
 diff -u -r1.613 parse.y
 --- sbin/pfctl/parse.y19 Dec 2011 23:26:16 -  1.613
 +++ sbin/pfctl/parse.y2 Jan 2012 08:04:12 -
 @@ -5614,37 +5614,52 @@
  pushfile(const char *name, int secret)
  {
   struct file *nfile;
 + struct stat sb;

 - if ((nfile = calloc(1, sizeof(struct file))) == NULL ||
 - (nfile-name = strdup(name)) == NULL) {
 - if (nfile)
 - free(nfile);
 - warn(malloc);
 - return (NULL);
 + if ((nfile = calloc(1, sizeof(struct file))) == NULL) {
 + warn(calloc);
 + goto err;
   }
 - if (TAILQ_FIRST(files) == NULL  strcmp(nfile-name, -) == 0) {
 +
 + if (TAILQ_FIRST(files) == NULL  strcmp(name, -) == 0) {
   nfile-stream = stdin;
 - free(nfile-name);
   if ((nfile-name = strdup(stdin)) == NULL) {
   warn(strdup);
 - free(nfile);
 - return (NULL);
 + goto err;
 + }
 + } else {
 + if ((nfile-name = strdup(name)) == NULL) {
 + warn(strdup);
 + goto err;
 + }
 + if ((nfile-stream = fopen(nfile-name, r)) == NULL) {
 + warn(%s, nfile-name);
 + goto err;
 + }
 + if (secret 
 + check_file_secrecy(fileno(nfile-stream), nfile-name))
 + goto file_err;
 + if (fstat(fileno(nfile-stream), sb) == -1) {
 + warn(fstat);
 + goto file_err;
 + }
 + if (S_ISDIR(sb.st_mode)) {
 + errno = EISDIR;
 + warn(%s, nfile-name);
 + goto file_err;
   }
 - } else if ((nfile-stream = fopen(nfile-name, r)) == NULL) {
 - warn(%s, nfile-name);
 - free(nfile-name);
 - free(nfile);
 - return (NULL);
 - } else if (secret 
 - check_file_secrecy(fileno(nfile-stream), nfile-name)) {
 - fclose(nfile-stream);
 - free(nfile-name);
 - free(nfile);
 - return (NULL);
   }
 +
   nfile-lineno = 1;
   TAILQ_INSERT_TAIL(files, nfile, entry);
   return (nfile);
 +
 +file_err:
 + fclose(nfile-stream);
 +err:
 + free(nfile-name);
 + free(nfile);
 + return (NULL);
  }

  int


 --
 Henning Brauer, h...@bsws.de, henn...@openbsd.org
 BS Web Services, http://bsws.de, Full-Service ISP
 Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully
Managed
 Henning Brauer Consulting, http://henningbrauer.com/

Re: ifconfig ieee80211 scan error to stderr

[Christiano F. Haesbaert]

On 2 December 2011 03:35, Philip Guenther guent...@gmail.com wrote:
 On Thu, Dec 1, 2011 at 6:45 PM, Christiano F. Haesbaert
 haesba...@openbsd.org wrote:
 Hi, I think we should warn() on any error, not just EPERM.
 This is more consistent with the rest of the code.

 ok ?

 I disagree with this.  The existing message is much clearer to the
 non-root mortal user that gets it and, IMO, it's clearer for the
 message to be sent to stdout with the rest of the output from the
 scan.

 As for errors other than EPERM, well, exactly what other errors *can*
 that call return in ifconfig?

Now none since the initial block guarantees no ENETDOWN, but when we
have another error in there, we'll have a silent error.


 What problem are you guys trying to solve?


Actually just code consistency.

But I've wasted too much people's time with this already, it's
bikeshed so don't bother.

Re: ifconfig ieee80211 scan error to stderr

[Christiano F. Haesbaert]

Hi, I think we should warn() on any error, not just EPERM.
This is more consistent with the rest of the code.

ok ?

Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.252
diff -d -u -p -b -r1.252 ifconfig.c
--- ifconfig.c  26 Nov 2011 23:38:18 -  1.252
+++ ifconfig.c  2 Dec 2011 02:32:48 -
@@ -2168,8 +2168,7 @@ ieee80211_listnodes(void)
strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
 
if (ioctl(s, SIOCS80211SCAN, (caddr_t)ifr) != 0) {
-   if (errno == EPERM)
-   printf(\t\tno permission to scan\n);
+   warn(SIOCS80211SCAN);
goto done;
}

Re: ifconfig ieee80211 scan error to stderr

[Christiano F. Haesbaert]

On 30 November 2011 12:39, Thomas de Grivel tho...@lowh.net wrote:
 Hi,

 Index: ifconfig.c
 ===
 RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
 retrieving revision 1.252
 diff -u -p -r1.252 ifconfig.c
 --- ifconfig.c26 Nov 2011 23:38:18 -1.252
 +++ ifconfig.c30 Nov 2011 14:35:52 -
 @@ -2169,7 +2169,7 @@ ieee80211_listnodes(void)

  if (ioctl(s, SIOCS80211SCAN, (caddr_t)ifr) != 0) {
  if (errno == EPERM)
 -printf(\t\tno permission to scan\n);
 +fprintf(stderr, %s: no permission to scan\n, name);
  goto done;
  }
 ? sbin_ifconfig.c-ieee80211-scan-stderr.diff
 Index: ifconfig.c
 ===
 RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
 retrieving revision 1.252
 diff -u -p -r1.252 ifconfig.c
 --- ifconfig.c  26 Nov 2011 23:38:18 -  1.252
 +++ ifconfig.c  30 Nov 2011 14:38:25 -
 @@ -2169,7 +2169,7 @@ ieee80211_listnodes(void)

if (ioctl(s, SIOCS80211SCAN, (caddr_t)ifr) != 0) {
if (errno == EPERM)
 -   printf(\t\tno permission to scan\n);
 +   fprintf(stderr, %s: no permission to scan\n,
name);
goto done;
}



I think this should be warnx() and not fprintf(stderr,
The rest of the code uses warnx().

Expose some scheduler statistics to userland via sysctl

[Christiano F. Haesbaert]

Moving this to tech@

Hi,

I was studying the scheduler code after watching tedu's talk, I'd like
to expose this statistics to userland so that I can try playing with
cache affinity in the future:

gimli:src: sysctl kern.schedstat

  
kern.schedstat.nmigrations=23744
kern.schedstat.noidle=0
kern.schedstat.stolen=9170
kern.schedstat.choose=834843
kern.schedstat.wasidle=808711
kern.schedstat.nomigrations=2388

Opinions ?

Index: sys/sys//sched.h
===
RCS file: /cvs/src/sys/sys/sched.h,v
retrieving revision 1.29
diff -d -u -p -w -r1.29 sched.h
--- sys/sys//sched.h7 Jul 2011 18:00:33 -   1.29
+++ sys/sys//sched.h12 Nov 2011 13:51:04 -
@@ -75,6 +75,34 @@
  * Posix defines a sched.h which may want to include sys/sched.h
  */
 
+struct schedstat {
+   u_int64_t scs_nmigrations;
+   u_int64_t scs_noidle;
+   u_int64_t scs_stolen;
+
+   u_int64_t scs_choose;
+   u_int64_t scs_wasidle;
+   u_int64_t scs_nomigrations;
+};
+
+/* These sysctl names are only really used by sysctl(8) */
+#define KERN_SCHEDSTAT_NMIGRATIONS 1
+#define KERN_SCHEDSTAT_NOIDLE  2
+#define KERN_SCHEDSTAT_STOLEN  3
+#define KERN_SCHEDSTAT_CHOOSE  4
+#define KERN_SCHEDSTAT_WASIDLE 5
+#define KERN_SCHEDSTAT_NOMIGRATIONS6
+#define KERN_SCHEDSTAT_MAXID   7
+
+#define CTL_KERN_SCHEDSTAT_NAMES { \
+   { 0, 0 },   \
+   { nmigrations, CTLTYPE_QUAD },\
+   { noidle, CTLTYPE_QUAD }, \
+   { stolen, CTLTYPE_QUAD }, \
+   { choose, CTLTYPE_QUAD }, \
+   { wasidle, CTLTYPE_QUAD },\
+   { nomigrations, CTLTYPE_QUAD }\
+}
 /*
  * CPU states.
  * XXX Not really scheduler state, but no other good place to put
Index: sys/sys//sysctl.h
===
RCS file: /cvs/src/sys/sys/sysctl.h,v
retrieving revision 1.117
diff -d -u -p -w -r1.117 sysctl.h
--- sys/sys//sysctl.h   30 Aug 2011 01:09:29 -  1.117
+++ sys/sys//sysctl.h   12 Nov 2011 13:40:45 -
@@ -189,7 +189,8 @@ struct ctlname {
 #defineKERN_CONSDEV75  /* dev_t: console terminal 
device */
 #defineKERN_NETLIVELOCKS   76  /* int: number of network 
livelocks */
 #defineKERN_POOL_DEBUG 77  /* int: enable pool_debug */
-#defineKERN_MAXID  78  /* number of valid kern ids */
+#define KERN_SCHEDSTAT 78  /* struct: sched statistics */
+#defineKERN_MAXID  79  /* number of valid kern ids */
 
 #defineCTL_KERN_NAMES { \
{ 0, 0 }, \
@@ -270,6 +271,7 @@ struct ctlname {
{ consdev, CTLTYPE_STRUCT }, \
{ netlivelocks, CTLTYPE_INT }, \
{ pool_debug, CTLTYPE_INT }, \
+   { schedstat, CTLTYPE_STRUCT }, \
 }
 
 /*
Index: sys/kern//kern_sched.c
===
RCS file: /cvs/src/sys/kern/kern_sched.c,v
retrieving revision 1.24
diff -d -u -p -w -r1.24 kern_sched.c
--- sys/kern//kern_sched.c  12 Oct 2011 18:30:09 -  1.24
+++ sys/kern//kern_sched.c  12 Nov 2011 14:41:59 -
@@ -35,6 +35,8 @@ void sched_kthreads_create(void *);
 int sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
 struct proc *sched_steal_proc(struct cpu_info *);
 
+struct schedstat schedstat;
+
 /*
  * To help choosing which cpu should run which process we keep track
  * of cpus which are currently idle and which cpus have processes
@@ -301,14 +303,6 @@ again:
return (p); 
 }
 
-uint64_t sched_nmigrations;
-uint64_t sched_noidle;
-uint64_t sched_stolen;
-
-uint64_t sched_choose;
-uint64_t sched_wasidle;
-uint64_t sched_nomigrations;
-
 struct cpu_info *
 sched_choosecpu_fork(struct proc *parent, int flags)
 {
@@ -374,7 +368,7 @@ sched_choosecpu(struct proc *p)
if (p-p_flag  P_CPUPEG)
return (p-p_cpu);
 
-   sched_choose++;
+   schedstat.scs_choose++;
 
/*
 * Look at all cpus that are currently idle and have nothing queued.
@@ -393,7 +387,7 @@ sched_choosecpu(struct proc *p)
if (cpuset_isset(set, p-p_cpu) ||
(p-p_cpu == curcpu()  p-p_cpu-ci_schedstate.spc_nrun == 0 
curproc == p)) {
-   sched_wasidle++;
+   schedstat.scs_wasidle++;
return (p-p_cpu);
}
 
@@ -411,9 +405,9 @@ sched_choosecpu(struct proc *p)
}
 
if (p-p_cpu != choice)
-   sched_nmigrations++;
+   schedstat.scs_nmigrations++;
else
-   sched_nomigrations++;
+   schedstat.scs_nomigrations++;
 
return (choice);
 }
@@ -461,7 +455,7 @@ sched_steal_proc(struct cpu_info *self)
 

Re: Expose some scheduler statistics to userland via sysctl

[Christiano F. Haesbaert]

On 17 November 2011 23:39, Ted Unangst t...@tedunangst.com wrote:
 On Fri, Nov 18, 2011, Thordur Bjornsson wrote:
 On 2011 Nov 17 (Thu) at 21:18:24 -0200 (-0200), Christiano F. Haesbaert
 wrote:
 Moving this to tech@

 Hi,

 I was studying the scheduler code after watching tedu's talk, I'd like
 to expose this statistics to userland so that I can try playing with
 cache affinity in the future:

 gimli:src: sysctl kern.schedstat

 kern.schedstat.nmigrations=23744
 kern.schedstat.noidle=0
 kern.schedstat.stolen=9170
 kern.schedstat.choose=834843
 kern.schedstat.wasidle=808711
 kern.schedstat.nomigrations=2388

 Opinions ?

 I see no point in exporting this out. They are essentially pointless
 knobs that people _will_ fiddle with without a clue.

 ???  They're readonly.


Mike just pointed out I can get those with pstat, so that solves my problem.

Still maybe there is a place for sysctl, but I'm not too strong about
it anymore.

What is the line between sysctl vs globals ?

Re: Expose some scheduler statistics to userland via sysctl

[Christiano F. Haesbaert]

On 18 November 2011 00:59, Christiano F. Haesbaert
haesba...@haesbaert.org wrote:
 On 17 November 2011 23:39, Ted Unangst t...@tedunangst.com wrote:
 On Fri, Nov 18, 2011, Thordur Bjornsson wrote:
 On 2011 Nov 17 (Thu) at 21:18:24 -0200 (-0200), Christiano F. Haesbaert
 wrote:
 Moving this to tech@

 Hi,

 I was studying the scheduler code after watching tedu's talk, I'd like
 to expose this statistics to userland so that I can try playing with
 cache affinity in the future:

 gimli:src: sysctl kern.schedstat

 kern.schedstat.nmigrations=23744
 kern.schedstat.noidle=0
 kern.schedstat.stolen=9170
 kern.schedstat.choose=834843
 kern.schedstat.wasidle=808711
 kern.schedstat.nomigrations=2388

 Opinions ?

 I see no point in exporting this out. They are essentially pointless
 knobs that people _will_ fiddle with without a clue.

 ???  They're readonly.


 Mike just pointed out I can get those with pstat, so that solves my
problem.

 Still maybe there is a place for sysctl, but I'm not too strong about
 it anymore.

 What is the line between sysctl vs globals ?


Checking again, it makes sense having the sysctl IMHO.
We have something very similar in kern.forkstat, kern.nchstats, and
the protocol stats.

Re: [panc...@nopcode.org: License]

[Christiano F. Haesbaert]

On 14 October 2011 17:19, Ted Unangst t...@tedunangst.com wrote:
 On Fri, Oct 14, 2011, Edd Barrett wrote:
 Hi guys,

 A port I am maintaining uses our file(1) implementation. A debian
 dev who was packaging this thinks he has spotted a license bug.

 Said I would forward his mail onto OpenBSD, so here it is.

 Discuss.

 What COPYRIGHT file?  There's no such file in my src/usr.bin/file.  And
 the *.c files are all 2 clause, not 3.


Actually magdir/os9 has the 4 clause license.

Re: enable aucat by default

[Christiano F. Haesbaert]

On 6 October 2011 17:26, Alexandre Ratchov a...@caoua.org wrote:
 On Thu, Oct 06, 2011 at 08:58:16PM +0200, Ingo Schwarze wrote:
 Hi Alexandre,

 Alexandre Ratchov wrote on Thu, Oct 06, 2011 at 08:29:26PM +0200:

  On the one hand, we expect audio to work by default. On the other hand
  format conversions, channel mapping, resampling and alike belong to
  the audio sub-system; until 2009, this used to be the audio(4) driver
  itself. But later, instead of extending the audio(4) driver, we put
  new audio code in aucat(1), which amongs others, has the advantage of
  running as unprivileged user rather than in supervisor mode. From this
  standpoint, there should be an instance of aucat(1) running by default
  for each instance of audio(4), ie for each sound card; [...]

 Really?

 Here is the list of daemons currently enabled by default:

   syslogd pflogd sshd sendmail inetd cron

 These are useful on almost any system, no matter whether
 server or desktop.

 Unless i misunderstand, aucat will often be useful on the desktop.
 But do you also run it on your servers in the datacenter?

 well, consider aucat a an extension of the audio(4) driver. Except
 that parts of the code are were moved in userspace.

 On servers you could disable audio, set aucat_flags=NO and recompile
 GENERIC with audio support disabled.

 Or do you argue that disabling it on servers is less work
 than enabling it on workstations?

 Not really, my point is that currently audio works by default on
 OpenBSD, on servers, laptops, PDAa. More and more code of the audio
 sub-system that would be in the kernel is being pulled in userland, so
 if we accept that audio should work by default, aucat should be
 enabled by default.

 Or do you argue that it doesn't matter running it even where
 it is not needed?


 Somewhat yes, if there are no audio devices or no audio program is
 run, aucat does nothing and shouldn't hurt.

 -- Alexandre



We could enable aucat if the user chose yes on Do you expect to run
X ? on the install script.
I think it's fair to assume that if the user is running X, he probably
wants audio.

Re: file(1): prevent printing unknown magic filename

[Christiano F. Haesbaert]

Now thinking again I see no much point in my diff, I prefer yours as
it's easier to update to a newer file(1), no point in changing only
this. 

ok from me.

On Wed, Sep 28, 2011 at 12:33:13AM -0300, Christiano F. Haesbaert wrote:
 On Thu, Sep 22, 2011 at 03:56:11PM +0100, Edd Barrett wrote:
  Hi,
  
  An upstream of one of the ports I work on (radare2) has imported our file(1)
  implementation and claims to have found bugs. This is the first:
  
  'ms-file' will only be assigned to 'fn' after a call to apprentice_load() 
  and
  ultimately load_1(), so the file_magwarn() at line 274 would report the 
  default
  filename unknown.
  
  We can trigger this behaviour by executing `file -c`:
  unknown, 0: Warning: using regular magic file `/etc/magic'
  
  Is it a bug?
  
  Index: apprentice.c
  ===
  RCS file: /cvs/src/usr.bin/file/apprentice.c,v
  retrieving revision 1.29
  diff -u -r1.29 apprentice.c
  --- apprentice.c11 Nov 2009 16:21:51 -  1.29
  +++ apprentice.c22 Sep 2011 14:27:17 -
  @@ -258,6 +258,7 @@
  return -1;
  }
   
  +   ms-file = fn;
  if (action == FILE_COMPILE) {
  rv = apprentice_load(ms, magic, nmagic, fn, action);
  if (rv != 0)
 
 Seems correct to me, but this fn seems kinda redundant down the
 stack, except for load_1() which really needs a file and not a
 directory. 
 
 So I thought we could kill this fn by assigning ms-file right in the
 beginning at file_apprentice(). Also tweak load_1() so it won't do the
 whole processing in an else. 
 
 Index: apprentice.c
 ===
 RCS file: /cvs/src/usr.bin/file/apprentice.c,v
 retrieving revision 1.29
 diff -d -u -p -w -r1.29 apprentice.c
 --- apprentice.c  11 Nov 2009 16:21:51 -  1.29
 +++ apprentice.c  28 Sep 2011 03:18:29 -
 @@ -96,11 +96,11 @@ private int parse(struct magic_set *, st
  private int parse_mime(struct magic_set *, struct magic_entry **, uint32_t *,
  const char *);
  private void eatsize(const char **);
 -private int apprentice_1(struct magic_set *, const char *, int, struct mlist 
 *);
 +private int apprentice_1(struct magic_set *, int, struct mlist *);
  private size_t apprentice_magic_strength(const struct magic *);
  private int apprentice_sort(const void *, const void *);
  private int apprentice_load(struct magic_set *, struct magic **, uint32_t *,
 -const char *, int);
 +int);
  private void byteswap(struct magic *, uint32_t);
  private void bs1(struct magic *);
  private uint16_t swap2(uint16_t);
 @@ -242,7 +242,7 @@ init_file_tables(void)
   * Handle one file or directory.
   */
  private int
 -apprentice_1(struct magic_set *ms, const char *fn, int action,
 +apprentice_1(struct magic_set *ms, int action,
  struct mlist *mlist)
  {
   struct magic *magic = NULL;
 @@ -259,19 +259,19 @@ apprentice_1(struct magic_set *ms, const
   }
  
   if (action == FILE_COMPILE) {
 - rv = apprentice_load(ms, magic, nmagic, fn, action);
 + rv = apprentice_load(ms, magic, nmagic, action);
   if (rv != 0)
   return -1;
 - rv = apprentice_compile(ms, magic, nmagic, fn);
 + rv = apprentice_compile(ms, magic, nmagic, ms-file);
   free(magic);
   return rv;
   }
  
  #ifndef COMPILE_ONLY
 - if ((rv = apprentice_map(ms, magic, nmagic, fn)) == -1) {
 + if ((rv = apprentice_map(ms, magic, nmagic, ms-file)) == -1) {
   if (ms-flags  MAGIC_CHECK)
 - file_magwarn(ms, using regular magic file `%s', fn);
 - rv = apprentice_load(ms, magic, nmagic, fn, action);
 + file_magwarn(ms, using regular magic file `%s', 
 ms-file);
 + rv = apprentice_load(ms, magic, nmagic, action);
   if (rv != 0)
   return -1;
   }
 @@ -359,7 +359,8 @@ file_apprentice(struct magic_set *ms, co
   *p++ = '\0';
   if (*fn == '\0')
   break;
 - file_err = apprentice_1(ms, fn, action, mlist);
 + ms-file = fn;
 + file_err = apprentice_1(ms, action, mlist);
   errs = MAX(errs, file_err);
   fn = p;
   }
 @@ -571,13 +572,15 @@ load_1(struct magic_set *ms, int action,
  {
   char line[BUFSIZ];
   size_t lineno = 0;
 - FILE *f = fopen(ms-file = fn, r);
 - if (f == NULL) {
 + FILE *f;
 + 
 + if ((f = fopen(fn, r)) == NULL) {
   if (errno != ENOENT)
   file_error(ms, errno, cannot read magic file `%s',
  fn);
   (*errs)++;
 - } else {
 + return;
 + } 
   /* read and parse this file */
   for (ms-line = 1; fgets(line, sizeof(line), f) != NULL; 
 ms-line

Re: fix a seg and minor improvements to config(8)

[Christiano F. Haesbaert]

Makes sense to me, ok.

Later we should fix the include orderning and change the warning
printfs to stderr. 

Can we get another ok ?

On Wed, Sep 28, 2011 at 02:37:34AM +0100, Edd Barrett wrote:
 Evening,
 
 When using `config -e`:
  * Don't print a NULL pointer if binary loaded is not a kernel.
  * Don't segfault of binary loaded is not a kernel.
  * Report non-existent kernel via a preliminary stat().
  * Make a warning look like the rest.
 
  OK?
 
 Index: exec.c
 ===
 RCS file: /cvs/src/usr.sbin/config/exec.c,v
 retrieving revision 1.7
 diff -u -r1.7 exec.c
 --- exec.c27 Oct 2009 23:59:51 -  1.7
 +++ exec.c28 Sep 2011 01:19:49 -
 @@ -26,6 +26,8 @@
  
  #include err.h
  #include sys/types.h
 +#include sys/stat.h
 +#include fcntl.h
  #include stdio.h
  
  #ifdef AOUT_SUPPORT
 @@ -109,6 +111,11 @@
  void
  loadkernel(char *file)
  {
 + struct stat st;
 +
 + if (stat(file, st) == -1)
 + err(1, cannot stat '%s', file);
 +
   current_exec = -1;
  
  #ifdef AOUT_SUPPORT
 Index: ukc.c
 ===
 RCS file: /cvs/src/usr.sbin/config/ukc.c,v
 retrieving revision 1.16
 diff -u -r1.16 ukc.c
 --- ukc.c 10 Dec 2009 22:07:19 -  1.16
 +++ ukc.c 28 Sep 2011 01:19:49 -
 @@ -114,10 +114,8 @@
   }
   }
  
 - printf(%s, adjust((caddr_t)nl[P_VERSION].n_value));
 -
   if (force == 0  outfile == NULL)
 - printf(warning: no output file specified\n);
 + printf(WARNING no output file specified\n);
  
   if (nl[IA_EXTRALOC].n_type == 0 || nl[I_NEXTRALOC].n_type == 0 ||
   nl[I_UEXTRALOC].n_type == 0 || nl[I_HISTLEN].n_type == 0 ||
 @@ -155,6 +153,8 @@
   process_history(histlen, history);
   }
  
 + printf(%s, adjust((caddr_t)nl[P_VERSION].n_value));
 +
   if (config()) {
   if (force == 0  outfile == NULL) {
   fprintf(stderr, not forced\n);
 @@ -184,7 +184,9 @@
   struct winsize w;
  #endif
  
 - cd = get_cfdata(0); /* get first item */
 + if ((cd = get_cfdata(0)) == NULL)   /* get first item */
 + errx(1, failed to get first cfdata);
 +
   while (cd-cf_attach != 0) {
   maxdev = i;
   totdev = i;
 
 -- 
 Best Regards
 Edd Barrett

Re: file(1): prevent printing unknown magic filename

[Christiano F. Haesbaert]

On Thu, Sep 22, 2011 at 03:56:11PM +0100, Edd Barrett wrote:
 Hi,
 
 An upstream of one of the ports I work on (radare2) has imported our file(1)
 implementation and claims to have found bugs. This is the first:
 
 'ms-file' will only be assigned to 'fn' after a call to apprentice_load() and
 ultimately load_1(), so the file_magwarn() at line 274 would report the 
 default
 filename unknown.
 
 We can trigger this behaviour by executing `file -c`:
 unknown, 0: Warning: using regular magic file `/etc/magic'
 
 Is it a bug?
 
 Index: apprentice.c
 ===
 RCS file: /cvs/src/usr.bin/file/apprentice.c,v
 retrieving revision 1.29
 diff -u -r1.29 apprentice.c
 --- apprentice.c  11 Nov 2009 16:21:51 -  1.29
 +++ apprentice.c  22 Sep 2011 14:27:17 -
 @@ -258,6 +258,7 @@
   return -1;
   }
  
 + ms-file = fn;
   if (action == FILE_COMPILE) {
   rv = apprentice_load(ms, magic, nmagic, fn, action);
   if (rv != 0)

Seems correct to me, but this fn seems kinda redundant down the
stack, except for load_1() which really needs a file and not a
directory. 

So I thought we could kill this fn by assigning ms-file right in the
beginning at file_apprentice(). Also tweak load_1() so it won't do the
whole processing in an else. 

Index: apprentice.c
===
RCS file: /cvs/src/usr.bin/file/apprentice.c,v
retrieving revision 1.29
diff -d -u -p -w -r1.29 apprentice.c
--- apprentice.c11 Nov 2009 16:21:51 -  1.29
+++ apprentice.c28 Sep 2011 03:18:29 -
@@ -96,11 +96,11 @@ private int parse(struct magic_set *, st
 private int parse_mime(struct magic_set *, struct magic_entry **, uint32_t *,
 const char *);
 private void eatsize(const char **);
-private int apprentice_1(struct magic_set *, const char *, int, struct mlist 
*);
+private int apprentice_1(struct magic_set *, int, struct mlist *);
 private size_t apprentice_magic_strength(const struct magic *);
 private int apprentice_sort(const void *, const void *);
 private int apprentice_load(struct magic_set *, struct magic **, uint32_t *,
-const char *, int);
+int);
 private void byteswap(struct magic *, uint32_t);
 private void bs1(struct magic *);
 private uint16_t swap2(uint16_t);
@@ -242,7 +242,7 @@ init_file_tables(void)
  * Handle one file or directory.
  */
 private int
-apprentice_1(struct magic_set *ms, const char *fn, int action,
+apprentice_1(struct magic_set *ms, int action,
 struct mlist *mlist)
 {
struct magic *magic = NULL;
@@ -259,19 +259,19 @@ apprentice_1(struct magic_set *ms, const
}
 
if (action == FILE_COMPILE) {
-   rv = apprentice_load(ms, magic, nmagic, fn, action);
+   rv = apprentice_load(ms, magic, nmagic, action);
if (rv != 0)
return -1;
-   rv = apprentice_compile(ms, magic, nmagic, fn);
+   rv = apprentice_compile(ms, magic, nmagic, ms-file);
free(magic);
return rv;
}
 
 #ifndef COMPILE_ONLY
-   if ((rv = apprentice_map(ms, magic, nmagic, fn)) == -1) {
+   if ((rv = apprentice_map(ms, magic, nmagic, ms-file)) == -1) {
if (ms-flags  MAGIC_CHECK)
-   file_magwarn(ms, using regular magic file `%s', fn);
-   rv = apprentice_load(ms, magic, nmagic, fn, action);
+   file_magwarn(ms, using regular magic file `%s', 
ms-file);
+   rv = apprentice_load(ms, magic, nmagic, action);
if (rv != 0)
return -1;
}
@@ -359,7 +359,8 @@ file_apprentice(struct magic_set *ms, co
*p++ = '\0';
if (*fn == '\0')
break;
-   file_err = apprentice_1(ms, fn, action, mlist);
+   ms-file = fn;
+   file_err = apprentice_1(ms, action, mlist);
errs = MAX(errs, file_err);
fn = p;
}
@@ -571,13 +572,15 @@ load_1(struct magic_set *ms, int action,
 {
char line[BUFSIZ];
size_t lineno = 0;
-   FILE *f = fopen(ms-file = fn, r);
-   if (f == NULL) {
+   FILE *f;
+   
+   if ((f = fopen(fn, r)) == NULL) {
if (errno != ENOENT)
file_error(ms, errno, cannot read magic file `%s',
   fn);
(*errs)++;
-   } else {
+   return;
+   } 
/* read and parse this file */
for (ms-line = 1; fgets(line, sizeof(line), f) != NULL; 
ms-line++) {
size_t len;
@@ -606,7 +609,6 @@ load_1(struct magic_set *ms, int action,
 
(void)fclose(f);
}
-}
 
 /*
  * parse a file or directory of files
@@ -614,7 +616,7 @@ load_1(struct magic_set *ms, int action,
  */
 

Re: sftp upload diff similar to scp

[Christiano F. Haesbaert]

Not sure if I got this, but why not using scp then ?

On 19 September 2011 14:52, elitter.net logana...@elitter.net wrote:
 It's annoying to upload using interactive mode.

 This diff works like scp.
 sftp localfile user@host:dir

 Index: sftp.c
 ===
 RCS file: /cvs/src/usr.bin/ssh/sftp.c,v
 retrieving revision 1.132
 diff -u -p -r1.132 sftp.c
 --- sftp.c  4 Dec 2010 00:18:01 -   1.132
 +++ sftp.c  19 Sep 2011 13:57:53 -
 @@ -25,6 +25,7 @@

  #include ctype.h
  #include errno.h
 +#include fcntl.h
  #include glob.h
  #include histedit.h
  #include paths.h
 @@ -175,7 +176,7 @@ static const struct CMD cmds[] = {
{ NULL, -1, -1  }
  };

 -int interactive_loop(struct sftp_conn *, char *file1, char *file2);
 +int interactive_loop(struct sftp_conn *, char *file1, char *file2, int);

  /* ARGSUSED */
  static void
 @@ -1834,7 +1835,7 @@ complete(EditLine *el, int ch)
  }

  int
 -interactive_loop(struct sftp_conn *conn, char *file1, char *file2)
 +interactive_loop(struct sftp_conn *conn, char *file1, char *file2, int
tflag)
  {
char *remote_path;
char *dir = NULL;
 @@ -1889,10 +1890,19 @@ interactive_loop(struct sftp_conn *conn,
}
} else {
if (file2 == NULL)
 -   snprintf(cmd, sizeof cmd, get %s, dir);
 +   if (tflag == 0)
 +   snprintf(cmd, sizeof cmd, get %s,
 +dir);
 +   else
 +   snprintf(cmd, sizeof cmd, put %s,
 +dir);
else
 -   snprintf(cmd, sizeof cmd, get %s %s, dir,
 -   file2);
 +   if (tflag == 0)
 +   snprintf(cmd, sizeof cmd, get %s
%s,
 +dir, file2);
 +   else
 +   snprintf(cmd, sizeof cmd, put %s
%s,
 +dir, file2);

err = parse_dispatch_command(conn, cmd,
remote_path, 1);
 @@ -2034,12 +2044,13 @@ usage(void)
  int
  main(int argc, char **argv)
  {
 -   int in, out, ch, err;
 +   int in, out, ch, tflag = 0, fd, err;
char *host = NULL, *userhost, *cp, *file2 = NULL;
int debug_level = 0, sshver = 2;
char *file1 = NULL, *sftp_server = NULL;
char *ssh_program = _PATH_SSH_PROGRAM, *sftp_direct = NULL;
const char *errstr;
 +   char path[MAXPATHLEN];
LogLevel ll = SYSLOG_LEVEL_INFO;
arglist args;
extern int optind;
 @@ -2163,9 +2174,22 @@ main(int argc, char **argv)
if (optind == argc || argc  (optind + 2))
usage();

 -   userhost = xstrdup(argv[optind]);
 -   file2 = argv[optind+1];
 -
 +   if (strrchr(argv[optind], '@') != NULL) {
 +   userhost = xstrdup(argv[optind]);
 +   file2 = argv[optind + 1];
 +   }
 +   else {
 +   tflag = 1;
 +   userhost = xstrdup(argv[optind + 1]);
 +   file2 = argv[optind];
 +   if ((fd = open(file2, O_RDONLY | O_NONBLOCK, 0)) 
0) {
 +   fprintf(stderr, \n%s\n,strerror(errno));
 +   _exit(1);
 +   }
 +   if (getcwd(path, sizeof(path)) != NULL 
 +   file2[0] != '/')
 +   file2 = path_append(path, file2);
 +   }
if ((host = strrchr(userhost, '@')) == NULL)
host = userhost;
else {
 @@ -2219,9 +2243,10 @@ main(int argc, char **argv)
else
fprintf(stderr, Attached to %s.\n, sftp_direct);
}
 -
 -   err = interactive_loop(conn, file1, file2);
 -
 +   if (tflag == 0)
 +   err = interactive_loop(conn, file1, file2, 0);
 +   else
 +   err = interactive_loop(conn, file2, file1, 1);
close(in);
close(out);
if (batchmode)

Re: ctags(1) and mg

[Christiano F. Haesbaert]

On 2 September 2011 14:17, Matthew Dempsky matt...@dempsky.org wrote:
 On Fri, Sep 2, 2011 at 8:55 AM, Sunil Nimmagadda
 su...@sunilnimmagadda.com wrote:
 This diff adds tags support to Mg. I am NOT an emacs user so if this
 combination is a bit odd then please excuse. It parses the tags file
 generated by ctags(1) and maintains a tree. M-. on first character of
 a word jumps to it's definition and M-* jumps back to previous location.

 I'd love to have ctags support in mg.


cscope would be cool too, if not too much bloat.

Re: passing vlan priority tag through bridge

[Christiano F. Haesbaert]

Heya, 

So here is a crude diff, the shiffting can be improved and if we wan't
this in the future we'll need a knob to enable don't touch the
vlanprio thingy.

Please it would be great if you can give this a spin Peter. I did some
basic tests with a re(4) (hw tagging) and a rl(4) (no hw tagging). 

Index: sys/net/if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.88
diff -d -u -p -w -r1.88 if_vlan.c
--- sys/net/if_vlan.c   20 Aug 2011 06:21:32 -  1.88
+++ sys/net/if_vlan.c   28 Aug 2011 05:15:37 -
@@ -225,6 +225,11 @@ vlan_start(struct ifnet *ifp)
 */
if ((p-if_capabilities  IFCAP_VLAN_HWTAGGING) 
(ifv-ifv_type == ETHERTYPE_VLAN)) {
+   /* Don't touch the priority */
+   if (m-m_flags  M_VLANPRIO) {
+   m-m_pkthdr.ether_vtag = ~EVL_VLID_MASK;
+   m-m_pkthdr.ether_vtag |= ifv-ifv_tag;
+   } else
m-m_pkthdr.ether_vtag = ifv-ifv_tag +
(ifv-ifv_prio  EVL_PRIO_BITS);
m-m_flags |= M_VLANTAG;
@@ -234,6 +239,12 @@ vlan_start(struct ifnet *ifp)
m_copydata(m, 0, ETHER_HDR_LEN, (caddr_t)evh);
evh.evl_proto = evh.evl_encap_proto;
evh.evl_encap_proto = htons(ifv-ifv_type);
+   if (m-m_flags  M_VLANPRIO)
+   /* XXX can be simpler */
+   evh.evl_tag = htons(ifv-ifv_tag +
+   (EVL_PRIOFTAG(m-m_pkthdr.ether_vtag)
+EVL_PRIO_BITS));
+   else
evh.evl_tag = htons(ifv-ifv_tag +
(ifv-ifv_prio  EVL_PRIO_BITS));
 
@@ -319,9 +330,13 @@ vlan_input(struct ether_header *eh, stru
 * reentrant!).
 */
m-m_pkthdr.rcvif = ifv-ifv_if;
+   m-m_flags |= M_VLANPRIO; /* XXX test only remove me */
if (m-m_flags  M_VLANTAG) {
m-m_flags = ~M_VLANTAG;
} else {
+   if (m-m_flags  M_VLANPRIO)
+   m-m_pkthdr.ether_vtag =
+   ntohs(*mtod(m, u_int16_t *));
eh-ether_type = mtod(m, u_int16_t *)[1];
m-m_len -= EVL_ENCAPLEN;
m-m_data += EVL_ENCAPLEN;
Index: sys/sys/mbuf.h
===
RCS file: /cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.155
diff -d -u -p -w -r1.155 mbuf.h
--- sys/sys/mbuf.h  8 Jul 2011 18:48:51 -   1.155
+++ sys/sys/mbuf.h  28 Aug 2011 03:58:05 -
@@ -71,7 +71,7 @@ struct m_hdr {
caddr_t mh_data;/* location of data */
u_int   mh_len; /* amount of data in this mbuf */
short   mh_type;/* type of data in this mbuf */
-   u_short mh_flags;   /* flags; see below */
+   u_int32_t mh_flags; /* flags; see below */
 };
 
 /* pf stuff */
@@ -171,10 +171,12 @@ struct mbuf {
 #define M_AUTH_AH  0x2000  /* header was authenticated (AH) */
 #define M_COMP 0x4000  /* header was decompressed */
 #define M_LINK00x8000  /* link layer specific flag */
+#define M_VLANPRIO 0x1 /* vlanprio bits are valid */
 
 /* flags copied when copying m_pkthdr */
 #defineM_COPYFLAGS 
(M_PKTHDR|M_EOR|M_PROTO1|M_BCAST|M_MCAST|M_CONF|M_COMP|\
-M_AUTH|M_LOOP|M_TUNNEL|M_LINK0|M_VLANTAG|M_FILDROP)
+M_AUTH|M_LOOP|M_TUNNEL|M_LINK0|M_VLANTAG|M_FILDROP|\
+M_VLANPRIO)
 
 /* Checksumming flags */
 #defineM_IPV4_CSUM_OUT 0x0001  /* IPv4 checksum needed */

Re: TOS option to tcpbench ala pf.conf

[Christiano F. Haesbaert]

Hi,

So here is the final version using -T with IPv6 with some points addressed by 
jmc@ in the manual.

ok to commit ?

Index: tcpbench.1
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.1,v
retrieving revision 1.12
diff -d -u -p -w -r1.12 tcpbench.1
--- tcpbench.1  16 Mar 2011 08:06:10 -  1.12
+++ tcpbench.1  21 Aug 2011 22:04:58 -
@@ -31,6 +31,7 @@
 .Op Fl p Ar port
 .Op Fl r Ar interval
 .Op Fl S Ar space
+.Op Fl T Ar toskeyword
 .Op Fl V Ar rtable
 .Ar hostname
 .Nm
@@ -41,6 +42,7 @@
 .Op Fl k Ar kvars
 .Op Fl p Ar port
 .Op Fl r Ar interval
+.Op Fl T Ar toskeyword
 .Op Fl S Ar space
 .Op Fl V Ar rtable
 .Ek
@@ -105,6 +107,21 @@ connections.
 It defaults to using TCP if
 .Fl u
 is not specified.
+.It Fl T Ar toskeyword
+Change the IPv4 TOS or IPv6 TCLASS value.
+.Ar toskeyword
+may be one of
+.Ar critical ,
+.Ar inetcontrol ,
+.Ar lowdelay ,
+.Ar netcontrol ,
+.Ar throughput ,
+.Ar reliability ,
+or one of the DiffServ Code Points:
+.Ar ef ,
+.Ar af11 ... af43 ,
+.Ar cs0 ... cs7 ;
+or a number in either hex or decimal.
 .It Fl u
 Use UDP instead of TCP; this must be specified on both the client
 and the server.
Index: tcpbench.c
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
retrieving revision 1.23
diff -d -u -p -w -r1.23 tcpbench.c
--- tcpbench.c  20 Aug 2011 11:11:30 -  1.23
+++ tcpbench.c  21 Aug 2011 05:42:21 -
@@ -65,6 +65,7 @@ struct {
int   Sflag;/* Socket buffer size (tcp mode) */
u_int rflag;/* Report rate (ms) */
int   sflag;/* True if server */
+   int   Tflag;/* ToS if != -1 */
int   vflag;/* Verbose */
int   uflag;/* UDP mode */
kvm_t*kvmh; /* Kvm handler */
@@ -113,7 +114,7 @@ static void client_init(struct addrinfo 
 static int clock_gettime_tv(clockid_t, struct timeval *);
 static voidudp_server_handle_sc(int, short, void *);
 static voidudp_process_slice(int, short, void *);
-
+static int map_tos(char *, int *);
 /*
  * We account the mainstats here, that is the stats
  * for all connections, all variables starting with slice
@@ -173,9 +174,10 @@ usage(void)
fprintf(stderr,
usage: tcpbench -l\n
   tcpbench [-uv] [-B buf] [-k kvars] [-n connections] [-p 
port]\n
-   [-r interval] [-S space] [-V rtable] hostname\n
+   [-r interval] [-S space] [-T toskeyword] [-V 
rtable]\n
+   hostname\n
   tcpbench -s [-uv] [-B buf] [-k kvars] [-p port]\n
-   [-r interval] [-S space] [-V rtable]\n);
+   [-r interval] [-S space] [-T toskeyword] [-V 
rtable]\n);
exit(1);
 }
 
@@ -680,6 +682,16 @@ again: 
r |= O_NONBLOCK;
if (fcntl(sock, F_SETFL, r) == -1)
err(1, fcntl(F_SETFL, O_NONBLOCK));
+   if (ptb-Tflag != -1  ss.ss_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt IP_TOS);
+   }
+   if (ptb-Tflag != -1  ss.ss_family == AF_INET6) {
+   if (setsockopt(sock, IPPROTO_IPV6, IPV6_TCLASS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt IPV_TCLASS);
+   }
/* Alloc client structure and register reading callback */
if ((sc = calloc(1, sizeof(*sc))) == NULL)
err(1, calloc);
@@ -729,6 +741,16 @@ server_init(struct addrinfo *aitop, stru
err(1, setsockopt SO_RTABLE);
}
}
+   if (ptb-Tflag != -1  ai-ai_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt IP_TOS);
+   }
+   if (ptb-Tflag != -1  ai-ai_family == AF_INET6) {
+   if (setsockopt(sock, IPPROTO_IPV6, IPV6_TCLASS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt IPV_TCLASS);
+   }
if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
on, sizeof(on)) == -1)
warn(reuse port);
@@ -821,6 +843,16 @@ client_init(struct addrinfo *aitop, int 
warn(socket);
continue;
}
+   if (ptb-Tflag != -1  ai-ai_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt 

Re: passing vlan priority tag through bridge

[Christiano F. Haesbaert]

On Fri, Aug 19, 2011 at 09:07:42AM +0200, Peter Hallin wrote:
 Hello,
 
 I have a question.
 
 We use bridging firewalls at Lund University with different vlan tags on
 respective sides of the bridges. The frames are therefore retagged
 when passing through the bridge and unforunatley the priority flag gets
 reset and always ends up as 0 on the other side.
 
 We would love to be able to let the priority flag pass the bridge and I
 wonder if this could be possible in a not so distant future.
 
 In if_vlan.c, there is a comment regarding the prio flag:
 
 /*
  * if_vlan.c - pseudo-device driver for IEEE 802.1Q virtual LANs.
  * Might be extended some day to also handle IEEE 802.1p priority
  * tagging.  This is sort of sneaky in the implementation, since
  * we need to pretend to be enough of an Ethernet implementation
  * to make arp work.  The way we do this is by telling everyone
  * that we are an Ethernet, and then catch the packets that
  * ether_output() left on our output queue when it calls
  * if_start(), rewrite them for use by the real outgoing
  * interface,
  * and ask it to send them.
   *
  * Some devices support 802.1Q tag insertion in firmware.  The
  * vlan interface behavior changes when the
  * IFCAP_VLAN_HWTAGGING
  * capability is set on the parent.  In this case,
  * vlan_start()
  * will not modify the ethernet header.
  */
 

I have a partial diff for this.

Unfortunately I couldn't test so I'll need more time.

The idea is to flag the incoming packet with a new flag M_VLANPRIO
which signals vlan(4) to not touch the vlanprio in vlan_start(). 

It's a proof-of-concept only, having something like this will probably
involve a lot of talk. 

Sorry my diff is not showable at this time.

Re: TOS option to tcpbench ala pf.conf

[Christiano F. Haesbaert]

On Fri, Aug 19, 2011 at 11:25:23PM +1000, Damien Miller wrote:
 
 Thanks, I like this. Could you add IPV6_TCLASS for IF_INET6 too?
 

So here is the version with IPV6_TCLASS and -T instead of -t, since
only traceroute uses -t, while ping and nc uses -T.

I did some basic tests but my knowledge of IPv6 is hideous, so please
test. From what I could read, IPV6_TCLASS should behave like IP_TOS.

Obs: this also includes the fix for the segfault mentioned in the
other thread. 

Index: tcpbench.1
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.1,v
retrieving revision 1.12
diff -d -u -p -w -r1.12 tcpbench.1
--- tcpbench.1  16 Mar 2011 08:06:10 -  1.12
+++ tcpbench.1  20 Aug 2011 06:41:07 -
@@ -31,6 +31,7 @@
 .Op Fl p Ar port
 .Op Fl r Ar interval
 .Op Fl S Ar space
+.Op Fl T Ar toskeyword
 .Op Fl V Ar rtable
 .Ar hostname
 .Nm
@@ -41,6 +42,7 @@
 .Op Fl k Ar kvars
 .Op Fl p Ar port
 .Op Fl r Ar interval
+.Op Fl T Ar toskeyword
 .Op Fl S Ar space
 .Op Fl V Ar rtable
 .Ek
@@ -105,6 +107,21 @@ connections.
 It defaults to using TCP if
 .Fl u
 is not specified.
+.It Fl T Ar toskeyword
+Change IPv4 TOS or IPv6 TCLASS value.
+.Ar toskeyword
+may be one of
+.Ar critical ,
+.Ar inetcontrol ,
+.Ar lowdelay ,
+.Ar netcontrol ,
+.Ar throughput ,
+.Ar reliability ,
+or one of the DiffServ Code Points:
+.Ar ef ,
+.Ar af11 ... af43 ,
+.Ar cs0 ... cs7 ;
+or one number in either hex or decimal.
 .It Fl u
 Use UDP instead of TCP; this must be specified on both the client
 and the server.
Index: tcpbench.c
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
retrieving revision 1.22
diff -d -u -p -w -r1.22 tcpbench.c
--- tcpbench.c  21 Jun 2011 17:31:07 -  1.22
+++ tcpbench.c  20 Aug 2011 06:39:02 -
@@ -65,6 +65,7 @@ struct {
int   Sflag;/* Socket buffer size (tcp mode) */
u_int rflag;/* Report rate (ms) */
int   sflag;/* True if server */
+   int   Tflag;/* ToS if != -1 */
int   vflag;/* Verbose */
int   uflag;/* UDP mode */
kvm_t*kvmh; /* Kvm handler */
@@ -113,7 +114,7 @@ static void client_init(struct addrinfo 
 static int clock_gettime_tv(clockid_t, struct timeval *);
 static voidudp_server_handle_sc(int, short, void *);
 static voidudp_process_slice(int, short, void *);
-
+static int map_tos(char *, int *);
 /*
  * We account the mainstats here, that is the stats
  * for all connections, all variables starting with slice
@@ -173,9 +174,10 @@ usage(void)
fprintf(stderr,
usage: tcpbench -l\n
   tcpbench [-uv] [-B buf] [-k kvars] [-n connections] [-p 
port]\n
-   [-r interval] [-S space] [-V rtable] hostname\n
+   [-r interval] [-S space] [-T toskeyword] [-V 
rtable]\n
+   hostname\n
   tcpbench -s [-uv] [-B buf] [-k kvars] [-p port]\n
-   [-r interval] [-S space] [-V rtable]\n);
+   [-r interval] [-S space] [-T toskeyword] [-V 
rtable]\n);
exit(1);
 }
 
@@ -642,6 +644,7 @@ again:
} else if (n == 0) {
if (ptb-vflag)
fprintf(stderr, %8d closed by remote end\n, sc-fd);
+   event_del(sc-ev);
close(sc-fd);
TAILQ_REMOVE(sc_queue, sc, entry);
free(sc);
@@ -679,6 +682,16 @@ again: 
r |= O_NONBLOCK;
if (fcntl(sock, F_SETFL, r) == -1)
err(1, fcntl(F_SETFL, O_NONBLOCK));
+   if (ptb-Tflag != -1  ss.ss_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt IP_TOS);
+   }
+   if (ptb-Tflag != -1  ss.ss_family == AF_INET6) {
+   if (setsockopt(sock, IPPROTO_IPV6, IPV6_TCLASS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt IPV_TCLASS);
+   }
/* Alloc client structure and register reading callback */
if ((sc = calloc(1, sizeof(*sc))) == NULL)
err(1, calloc);
@@ -728,6 +741,16 @@ server_init(struct addrinfo *aitop, stru
err(1, setsockopt SO_RTABLE);
}
}
+   if (ptb-Tflag != -1  ai-ai_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+   err(1, setsockopt IP_TOS);
+   }
+   if (ptb-Tflag != -1  ai-ai_family == AF_INET6) {
+   if (setsockopt(sock, IPPROTO_IPV6, IPV6_TCLASS,
+   ptb-Tflag, sizeof(ptb-Tflag)))
+  

Re: ifconfig old wpapsk entry and cleanup.

[Christiano F. Haesbaert]

keeping this up.

On Wed, Aug 17, 2011 at 12:42:02AM -0300, Christiano F. Haesbaert wrote:
 Hi
 
 Remove the old wpapsk cmd entry, remove unnecessary casts in malloc/calloc, 
 use
 timerclear macro plus minor style(9):
 
 Index: ifconfig.c
 ===
 RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
 retrieving revision 1.248
 diff -d -u -p -w -r1.248 ifconfig.c
 --- ifconfig.c9 Jul 2011 00:45:40 -   1.248
 +++ ifconfig.c11 Aug 2011 00:14:48 -
 @@ -314,9 +314,6 @@ const struct  cmd {
   { wpaprotos,  NEXTARG,0,  setifwpaprotos },
   { wpakey, NEXTARG,0,  setifwpakey },
   { -wpakey,-1, 0,  setifwpakey },
 -/*XXX delete these two after the 4.9 release */
 -/*XXX*/  { wpapsk, NEXTARG,0,  setifwpakey },
 -/*XXX*/  { -wpapsk,-1, 0,  setifwpakey },
   { chan,   NEXTARG0,   0,  setifchan },
   { -chan,  -1, 0,  setifchan },
   { scan,   NEXTARG0,   0,  setifscan },
 @@ -2838,9 +2835,9 @@ status(int link, struct sockaddr_dl *sdl
   goto proto_status;
   }
  
 - media_list = (int *)calloc(ifmr.ifm_count, sizeof(int));
 + media_list = calloc(ifmr.ifm_count, sizeof(int));
   if (media_list == NULL)
 - err(1, malloc);
 + err(1, calloc);
   ifmr.ifm_ulist = media_list;
  
   if (ioctl(s, SIOCGIFMEDIA, (caddr_t)ifmr)  0)
 @@ -3415,7 +3412,7 @@ setcarp_passwd(const char *val, int d)
  {
   struct carpreq carpr;
  
 - memset((char *)carpr, 0, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
 @@ -3440,7 +3437,7 @@ setcarp_vhid(const char *val, int d)
   if (errmsg)
   errx(1, vhid %s: %s, val, errmsg);
  
 - memset((char *)carpr, 0, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
 @@ -3465,7 +3462,7 @@ setcarp_advskew(const char *val, int d)
   if (errmsg)
   errx(1, advskew %s: %s, val, errmsg);
  
 - memset((char *)carpr, 0, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
 @@ -3489,7 +3486,7 @@ setcarp_advbase(const char *val, int d)
   if (errmsg)
   errx(1, advbase %s: %s, val, errmsg);
  
 - memset((char *)carpr, 0, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
 @@ -3509,7 +3506,7 @@ setcarppeer(const char *val, int d)
   struct addrinfo hints, *peerres;
   int ecode;
  
 - memset((char *)carpr, 0, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
 @@ -3540,13 +3537,13 @@ unsetcarppeer(const char *val, int d)
  {
   struct carpreq carpr;
  
 - bzero((char *)carpr, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
   err(1, SIOCGVH);
  
 - bzero((char *)carpr.carpr_peer, sizeof(carpr.carpr_peer));
 + bzero(carpr.carpr_peer, sizeof(carpr.carpr_peer));
  
   if (ioctl(s, SIOCSVH, (caddr_t)ifr) == -1)
   err(1, SIOCSVH);
 @@ -3559,7 +3556,7 @@ setcarp_state(const char *val, int d)
   struct carpreq carpr;
   int i;
  
 - bzero((char *)carpr, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
 @@ -3582,7 +3579,7 @@ setcarpdev(const char *val, int d)
  {
   struct carpreq carpr;
  
 - bzero((char *)carpr, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
 @@ -3599,13 +3596,13 @@ unsetcarpdev(const char *val, int d)
  {
   struct carpreq carpr;
  
 - bzero((char *)carpr, sizeof(struct carpreq));
 + bzero(carpr, sizeof(struct carpreq));
   ifr.ifr_data = (caddr_t)carpr;
  
   if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
   err(1, SIOCGVH);
  
 - bzero((char *)carpr.carpr_carpdev, sizeof(carpr.carpr_carpdev));
 + bzero(carpr.carpr_carpdev, sizeof(carpr.carpr_carpdev));
  
   if (ioctl(s, SIOCSVH, (caddr_t)ifr) == -1)
   err(1, SIOCSVH);
 @@ -3618,7 +3615,7 @@ setcarp_nodes(const char *val, int d)
   int i;
   struct carpreq carpr;
  
 - bzero((char *)carpr, sizeof(struct

TOS option to tcpbench ala pf.conf

[Christiano F. Haesbaert]

Hi,

I'm tinkering with ToS-CoS (802.1p) translation in vlan(4) so I
needed something to test, tcpbench seems to deserve a tos option.  

It uses the same map_option() from pfctl with some minor tweeks.
So it accepts decimal, hexadecimal, critical, lowdelay, af11...

Option chosen was -t, couldn't find anything related in other programs.

Index: usr.bin/tcpbench/tcpbench.c
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
retrieving revision 1.22
diff -d -u -p -w -r1.22 tcpbench.c
--- usr.bin/tcpbench/tcpbench.c 21 Jun 2011 17:31:07 -  1.22
+++ usr.bin/tcpbench/tcpbench.c 18 Aug 2011 13:45:11 -
@@ -65,6 +65,7 @@ struct {
int   Sflag;/* Socket buffer size (tcp mode) */
u_int rflag;/* Report rate (ms) */
int   sflag;/* True if server */
+   int   tflag;/* ToS if != -1 */
int   vflag;/* Verbose */
int   uflag;/* UDP mode */
kvm_t*kvmh; /* Kvm handler */
@@ -113,7 +114,7 @@ static void client_init(struct addrinfo 
 static int clock_gettime_tv(clockid_t, struct timeval *);
 static voidudp_server_handle_sc(int, short, void *);
 static voidudp_process_slice(int, short, void *);
-
+static int map_tos(char *, int *);
 /*
  * We account the mainstats here, that is the stats
  * for all connections, all variables starting with slice
@@ -173,9 +174,10 @@ usage(void)
fprintf(stderr,
usage: tcpbench -l\n
   tcpbench [-uv] [-B buf] [-k kvars] [-n connections] [-p 
port]\n
-   [-r interval] [-S space] [-V rtable] hostname\n
+   [-r interval] [-S space] [-t toskeyword] [-V 
rtable]\n
+   hostname\n
   tcpbench -s [-uv] [-B buf] [-k kvars] [-p port]\n
-   [-r interval] [-S space] [-V rtable]\n);
+   [-r interval] [-S space] [-t toskeyword] [-V 
rtable]\n);
exit(1);
 }
 
@@ -679,6 +681,11 @@ again: 
r |= O_NONBLOCK;
if (fcntl(sock, F_SETFL, r) == -1)
err(1, fcntl(F_SETFL, O_NONBLOCK));
+   if (ptb-tflag != -1  ss.ss_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-tflag, sizeof(ptb-tflag)))
+   err(1, setsockopt IP_TOS);
+   }
/* Alloc client structure and register reading callback */
if ((sc = calloc(1, sizeof(*sc))) == NULL)
err(1, calloc);
@@ -728,6 +735,11 @@ server_init(struct addrinfo *aitop, stru
err(1, setsockopt SO_RTABLE);
}
}
+   if (ptb-tflag != -1  ai-ai_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-tflag, sizeof(ptb-tflag)))
+   err(1, setsockopt IP_TOS);
+   }
if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
on, sizeof(on)) == -1)
warn(reuse port);
@@ -820,6 +832,11 @@ client_init(struct addrinfo *aitop, int 
warn(socket);
continue;
}
+   if (ptb-tflag != -1  ai-ai_family == AF_INET) {
+   if (setsockopt(sock, IPPROTO_IP, IP_TOS,
+   ptb-tflag, sizeof(ptb-tflag)))
+   err(1, setsockopt IP_TOS);
+   }
if (ptb-Vflag) {
if (setsockopt(sock, SOL_SOCKET, SO_RTABLE,
ptb-Vflag, sizeof(ptb-Vflag)) == -1) {
@@ -874,6 +891,54 @@ client_init(struct addrinfo *aitop, int 
fprintf(stderr, %u connections established\n, nconn);
 }
 
+static int
+map_tos(char *s, int *val)
+{
+   /* DiffServ Codepoints and other TOS mappings */
+   const struct toskeywords {
+   const char  *keyword;
+   int  val;
+   } *t, toskeywords[] = {
+   { af11,   IPTOS_DSCP_AF11 },
+   { af12,   IPTOS_DSCP_AF12 },
+   { af13,   IPTOS_DSCP_AF13 },
+   { af21,   IPTOS_DSCP_AF21 },
+   { af22,   IPTOS_DSCP_AF22 },
+   { af23,   IPTOS_DSCP_AF23 },
+   { af31,   IPTOS_DSCP_AF31 },
+   { af32,   IPTOS_DSCP_AF32 },
+   { af33,   IPTOS_DSCP_AF33 },
+   { af41,   IPTOS_DSCP_AF41 },
+   { af42,   IPTOS_DSCP_AF42 },
+   { af43,   IPTOS_DSCP_AF43 },
+   { critical,   

Re: TOS option to tcpbench ala pf.conf

[Christiano F. Haesbaert]

On 19 August 2011 10:25, Damien Miller d...@mindrot.org wrote:
 On Thu, 18 Aug 2011, Christiano F. Haesbaert wrote:

 Hi,

 I'm tinkering with ToS-CoS (802.1p) translation in vlan(4) so I
 needed something to test, tcpbench seems to deserve a tos option.

 It uses the same map_option() from pfctl with some minor tweeks.
 So it accepts decimal, hexadecimal, critical, lowdelay, af11...

 Option chosen was -t, couldn't find anything related in other programs.

 Thanks, I like this. Could you add IPV6_TCLASS for IF_INET6 too?


Sure, I'll just have to study a bit, I know nothing of IPv6.
Sthen pointed out that ping uses -T for tos and traceroute just
recently added -t.
Since ping uses -t for ttl, I think we should change em all to -T,
traderoute only had the -t option very recently so it wouldn't be much
problem.
Also, ping for instance won't accept hex as an option.

I'll send a diff tonight with the options on -T for tcpbench/ping/traderoute.

Small fixes for if_oerrors in vlan(4), mpe(4) and pppx

[Christiano F. Haesbaert]

Hi, vlan_start() was increasing packet counts before checking if the
packet was successfully enqueued. I made a hunt for similar errors.

Index: net/if_mpe.c
===
RCS file: /cvs/src/sys/net/if_mpe.c,v
retrieving revision 1.25
diff -d -u -p -w -r1.25 if_mpe.c
--- net/if_mpe.c28 Jan 2011 14:58:24 -  1.25
+++ net/if_mpe.c20 Aug 2011 04:06:29 -
@@ -265,7 +265,7 @@ mpeoutput(struct ifnet *ifp, struct mbuf
if (error) {
/* mbuf is already freed */
splx(s);
-   return (error);
+   goto out;
}
if_start(ifp);
splx(s);
Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.9
diff -d -u -p -w -r1.9 if_pppx.c
--- net/if_pppx.c   7 Jul 2011 20:42:56 -   1.9
+++ net/if_pppx.c   20 Aug 2011 05:37:48 -
@@ -1057,6 +1057,10 @@ pppx_if_output(struct ifnet *ifp, struct
 
s = splnet();
IFQ_ENQUEUE(ifp-if_snd, m, NULL, error);
+   if (error) {
+   splx(s);
+   goto out;
+   }
if_start(ifp);
splx(s);
 
Index: net/if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.87
diff -d -u -p -w -r1.87 if_vlan.c
--- net/if_vlan.c   18 Feb 2011 17:06:45 -  1.87
+++ net/if_vlan.c   20 Aug 2011 03:58:05 -
@@ -251,15 +251,15 @@ vlan_start(struct ifnet *ifp)
 * Send it, precisely as ether_output() would have.
 * We are already running at splnet.
 */
-   p-if_obytes += m-m_pkthdr.len;
-   if (m-m_flags  M_MCAST)
-   p-if_omcasts++;
IFQ_ENQUEUE(p-if_snd, m, NULL, error);
if (error) {
/* mbuf is already freed */
ifp-if_oerrors++;
continue;
}
+   p-if_obytes += m-m_pkthdr.len;
+   if (m-m_flags  M_MCAST)
+   p-if_omcasts++;
 
ifp-if_opackets++;
if_start(p);