document capability dc in remote(5)

2017-10-30 Thread Remi Locherer
Hi,

in 2015 remote(5) was trimmed down when tip was removed. It looks like
documentation for capability "dc" was also removed by accident. cu(1) still
supports this (src/usr.bin/cu/cu.c):

381 if (is_direct == -1 && cgetcap(cp, "dc", ':') != NULL)
382 is_direct = 1;

Below patch brings back documentation for "dc".

ok?

Remi


Index: remote.5
===
RCS file: /cvs/src/share/man/man5/remote.5,v
retrieving revision 1.26
diff -u -p -r1.26 remote.5
--- remote.521 Sep 2017 07:51:43 -  1.26
+++ remote.530 Oct 2017 22:39:44 -
@@ -75,6 +75,12 @@ The baud rate used in establishing
 a connection to the remote host.
 This is a decimal number.
 The default baud rate is 9600 baud.
+.It Sy \
+(bool)
+This host is directly connected, and
+.Xr cu 1
+should not expect carrier detect to be high, nor should it exit if
+carrier detect drops.
 .It Sy \
 (str)
 Device to open to establish a connection.



Re: [PATCH 1/2] VMD: Require interface to be defined in switches

2017-10-30 Thread Mike Larkin
On Mon, Oct 30, 2017 at 11:16:47PM +0100, Reyk Floeter wrote:
> 
> > On 27.10.2017, at 19:10, Mike Larkin  wrote:
> > 
> > On Fri, Oct 27, 2017 at 09:58:09AM +0200, Martin Pieuchot wrote:
> >> On 26/10/17(Thu) 23:47, Mike Larkin wrote:
> >>> On Fri, Oct 27, 2017 at 08:23:05AM +0200, Martin Pieuchot wrote:
>  On 26/10/17(Thu) 16:23, Carlos Cardenas wrote:
> > * Require interface name to be defined for switches in vm.conf
> > ** Requires user to create bridge(4) beforehand
> > * Remove code to create bridges on the fly
> > * Add code to ensure bridge really exists
> > * Update manpage switch and example sections
>  
>  What's the motivation to ask people to create a bridge(4) beforehand?
>  
> >>> 
> >>> This discussion came out of a previous effort to fix a bug where vmd(8) 
> >>> doesn't
> >>> deconfigure bridges (and/or undo changes to bridges) it made during
> >>> initialization. After discussing this a bit with claudio, we decided that 
> >>> rather than having vmd try to do magic stuff with creating bridges and
> >>> tearing them down behind the scenes (including corner cases where vmd is
> >>> killed or crashes/etc), it made sense instead to say "you create the 
> >>> bridge
> >>> and tell me which one you want to use". This can easily be accomplished
> >>> with a 1 (or 2) line /etc/hostname.bridgeX file.
> >>> 
> >>> We may decide later to have vmd try to create bridges again at some point
> >>> but for now, I think this is a way to make some forward progress in this
> >>> area.
> >> 
> >> I don't want to stop progress but I don't understand how creating a
> >> bridge(4) beforehand help to "deconfigure" it.  Hence my question.
> > 
> > We want vmd(8) to be out of the bridge manipulation business, except for 
> > adding
> > and removing tapX interfaces for the guest VMs it manages.
> > 
> > Right now, vmd(8) is in the bridge creation and deletion business, and it is
> > that second part that is giving us headache. Thus, we decided to remove both
> > parts since we don't have a good solution as a whole.
> > 
> 
> OK, that answers a question that I just asked in another mail.
> 
> I still don't understand why this is improving anything, but since I'm not 
> around I cannot object.
> 
> I used automatic bridge creation all the time and I never saw the theoretical 
> problem of not "deconfiguring bridges". Why does it have to? It is a 
> best-effort, it uses bridges if they already exist. And vmd already supported 
> configuration with pre-configured bridges. Or was there a (supposedly simple) 
> bug with pre-configured bridges?
> 
> Reyk
> 

This was a fix for what I outlined there as well as bizarre behaviour
during vmctl reload with bridges defined.



Re: [PATCH 1/2] VMD: Require interface to be defined in switches

2017-10-30 Thread Reyk Floeter

> On 27.10.2017, at 19:10, Mike Larkin  wrote:
> 
> On Fri, Oct 27, 2017 at 09:58:09AM +0200, Martin Pieuchot wrote:
>> On 26/10/17(Thu) 23:47, Mike Larkin wrote:
>>> On Fri, Oct 27, 2017 at 08:23:05AM +0200, Martin Pieuchot wrote:
 On 26/10/17(Thu) 16:23, Carlos Cardenas wrote:
> * Require interface name to be defined for switches in vm.conf
> ** Requires user to create bridge(4) beforehand
> * Remove code to create bridges on the fly
> * Add code to ensure bridge really exists
> * Update manpage switch and example sections
 
 What's the motivation to ask people to create a bridge(4) beforehand?
 
>>> 
>>> This discussion came out of a previous effort to fix a bug where vmd(8) 
>>> doesn't
>>> deconfigure bridges (and/or undo changes to bridges) it made during
>>> initialization. After discussing this a bit with claudio, we decided that 
>>> rather than having vmd try to do magic stuff with creating bridges and
>>> tearing them down behind the scenes (including corner cases where vmd is
>>> killed or crashes/etc), it made sense instead to say "you create the bridge
>>> and tell me which one you want to use". This can easily be accomplished
>>> with a 1 (or 2) line /etc/hostname.bridgeX file.
>>> 
>>> We may decide later to have vmd try to create bridges again at some point
>>> but for now, I think this is a way to make some forward progress in this
>>> area.
>> 
>> I don't want to stop progress but I don't understand how creating a
>> bridge(4) beforehand help to "deconfigure" it.  Hence my question.
> 
> We want vmd(8) to be out of the bridge manipulation business, except for 
> adding
> and removing tapX interfaces for the guest VMs it manages.
> 
> Right now, vmd(8) is in the bridge creation and deletion business, and it is
> that second part that is giving us headache. Thus, we decided to remove both
> parts since we don't have a good solution as a whole.
> 

OK, that answers a question that I just asked in another mail.

I still don't understand why this is improving anything, but since I'm not 
around I cannot object.

I used automatic bridge creation all the time and I never saw the theoretical 
problem of not "deconfiguring bridges". Why does it have to? It is a 
best-effort, it uses bridges if they already exist. And vmd already supported 
configuration with pre-configured bridges. Or was there a (supposedly simple) 
bug with pre-configured bridges?

Reyk



Re: add one more softnet taskq

2017-10-30 Thread Alexandr Nedvedicky
Hello,

thank you for looking at my changes. updated patch is below. The list
of changes is as follows:

o SOFTNET_TASKS   1 is defined to 1

o call to task_add():
@@ -1839,5 +1839,5 @@ void
 ip_send(struct mbuf *m)
 {
mq_enqueue(_mq, m);
-   task_add(softnettq, _task);
+   task_add(net_tq(0), _task);
 }
call to task_add() in chunk above got changed to:
KASSERT((m->m_flags & M_PKTHDR) == M_PKTHDR);
task_add(net_tq(m->m_pkthdr.ph_ifidx), _task);
where appropriate (ip_send(), ip6_send()).  the remaining calls to
task_add() where left unchanged as there is no object, where we can
reach for interface index, available. I hope that's enough to make
Peter Hessler happy.

thanks a lot
regards
sasha

8<---8<---8<--8<
diff --git a/sys/net/if.c b/sys/net/if.c
index e9e9f07add1..6ef5d2f6e3b 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -224,7 +224,9 @@ int net_livelocked(void);
 intifq_congestion;
 
 int netisr;
-struct taskq   *softnettq;
+
+#defineSOFTNET_TASKS   1
+struct taskq   *softnettq[SOFTNET_TASKS];
 
 struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
 
@@ -240,6 +242,8 @@ struct rwlock netlock = RWLOCK_INITIALIZER("netlock");
 void
 ifinit(void)
 {
+   unsigned inti;
+
/*
 * most machines boot with 4 or 5 interfaces, so size the initial map
 * to accomodate this
@@ -248,9 +252,11 @@ ifinit(void)
 
timeout_set(_tick_to, net_tick, _tick_to);
 
-   softnettq = taskq_create("softnet", 1, IPL_NET, TASKQ_MPSAFE);
-   if (softnettq == NULL)
-   panic("unable to create softnet taskq");
+   for (i = 0; i < SOFTNET_TASKS; i++) {
+   softnettq[i] = taskq_create("softnet", 1, IPL_NET, 
TASKQ_MPSAFE);
+   if (softnettq[i] == NULL)
+   panic("unable to create softnet taskq");
+   }
 
net_tick(_tick_to);
 }
@@ -725,7 +731,7 @@ if_input(struct ifnet *ifp, struct mbuf_list *ml)
 #endif
 
if (mq_enlist(>if_inputqueue, ml) == 0)
-   task_add(softnettq, ifp->if_inputtask);
+   task_add(net_tq(ifp->if_index), ifp->if_inputtask);
 }
 
 int
@@ -1025,15 +1031,15 @@ if_detach(struct ifnet *ifp)
ifp->if_watchdog = NULL;
 
/* Remove the input task */
-   task_del(softnettq, ifp->if_inputtask);
+   task_del(net_tq(ifp->if_index), ifp->if_inputtask);
mq_purge(>if_inputqueue);
 
/* Remove the watchdog timeout & task */
timeout_del(ifp->if_slowtimo);
-   task_del(softnettq, ifp->if_watchdogtask);
+   task_del(net_tq(ifp->if_index), ifp->if_watchdogtask);
 
/* Remove the link state task */
-   task_del(softnettq, ifp->if_linkstatetask);
+   task_del(net_tq(ifp->if_index), ifp->if_linkstatetask);
 
 #if NBPFILTER > 0
bpfdetach(ifp);
@@ -1583,7 +1589,7 @@ if_linkstate(struct ifnet *ifp)
 void
 if_link_state_change(struct ifnet *ifp)
 {
-   task_add(softnettq, ifp->if_linkstatetask);
+   task_add(net_tq(ifp->if_index), ifp->if_linkstatetask);
 }
 
 /*
@@ -1599,7 +1605,7 @@ if_slowtimo(void *arg)
 
if (ifp->if_watchdog) {
if (ifp->if_timer > 0 && --ifp->if_timer == 0)
-   task_add(softnettq, ifp->if_watchdogtask);
+   task_add(net_tq(ifp->if_index), ifp->if_watchdogtask);
timeout_add(ifp->if_slowtimo, hz / IFNET_SLOWHZ);
}
splx(s);
@@ -2881,3 +2887,13 @@ unhandled_af(int af)
 {
panic("unhandled af %d", af);
 }
+
+struct taskq *
+net_tq(unsigned int ifindex)
+{
+   struct taskq *t = NULL;
+
+   t = softnettq[ifindex % SOFTNET_TASKS];
+
+   return (t);
+}
diff --git a/sys/net/if.h b/sys/net/if.h
index 89867eac340..6a0770a8ea0 100644
--- a/sys/net/if.h
+++ b/sys/net/if.h
@@ -489,6 +489,7 @@ voidif_congestion(void);
 intif_congested(void);
 __dead voidunhandled_af(int);
 intif_setlladdr(struct ifnet *, const uint8_t *);
+struct taskq * net_tq(unsigned int);
 
 #endif /* _KERNEL */
 
diff --git a/sys/net/if_loop.c b/sys/net/if_loop.c
index 277e7f966a2..e9f58a4ee52 100644
--- a/sys/net/if_loop.c
+++ b/sys/net/if_loop.c
@@ -244,7 +244,7 @@ looutput(struct ifnet *ifp, struct mbuf *m, struct sockaddr 
*dst,
m->m_pkthdr.ph_family = dst->sa_family;
if (mq_enqueue(>if_inputqueue, m))
return ENOBUFS;
-   task_add(softnettq, ifp->if_inputtask);
+   task_add(net_tq(ifp->if_index), ifp->if_inputtask);
 
return (0);
 }
diff --git a/sys/net/if_pflow.c b/sys/net/if_pflow.c
index 38efb02be7e..91a61fe4c15 100644
--- a/sys/net/if_pflow.c
+++ b/sys/net/if_pflow.c
@@ -286,7 +286,7 @@ pflow_clone_destroy(struct ifnet *ifp)
if (timeout_initialized(>sc_tmo_tmpl))
timeout_del(>sc_tmo_tmpl);
pflow_flush(sc);
-   task_del(softnettq, 

isakmpd(8): honor RFC 5903

2017-10-30 Thread Patrick Wildt
Hi,

Honor RFC 5903 in isakmpd(8) as well.  The member g_xy is always the
shared secret, but so far it has always been using the length of public
points.  Since this is different now, as the shared secret for EC Groups
should only store the x point, we need another member to specify the
length of g_xy.

Note that this change breaks backwards compatibility to older peers that
don't run this change.

Similar change has been done to iked(8) recently.

OKs?  Concerns?

Patrick

diff --git a/sbin/isakmpd/dh.c b/sbin/isakmpd/dh.c
index a0da694b5fb..5d3fcfaa7d0 100644
--- a/sbin/isakmpd/dh.c
+++ b/sbin/isakmpd/dh.c
@@ -36,10 +36,13 @@ int modp_create_shared(struct group *, u_int8_t *, u_int8_t 
*);
 
 intec_init(struct group *);
 intec_getlen(struct group *);
+intec_secretlen(struct group *);
 intec_create_exchange(struct group *, u_int8_t *);
 intec_create_shared(struct group *, u_int8_t *, u_int8_t *);
 
-intec_point2raw(struct group *, const EC_POINT *, u_int8_t *, size_t);
+#define EC_POINT2RAW_FULL  0
+#define EC_POINT2RAW_XONLY 1
+intec_point2raw(struct group *, const EC_POINT *, uint8_t *, size_t, int);
 EC_POINT *
ec_raw2point(struct group *, u_int8_t *, size_t);
 
@@ -277,6 +280,7 @@ group_get(u_int32_t id)
case GROUP_ECP:
group->init = ec_init;
group->getlen = ec_getlen;
+   group->secretlen = ec_secretlen;
group->exchange = ec_create_exchange;
group->shared = ec_create_shared;
break;
@@ -305,6 +309,15 @@ dh_getlen(struct group *group)
return (group->getlen(group));
 }
 
+int
+dh_secretlen(struct group *group)
+{
+   if (group->secretlen)
+   return (group->secretlen(group));
+   else
+   return (group->getlen(group));
+}
+
 int
 dh_create_exchange(struct group *group, u_int8_t *buf)
 {
@@ -412,6 +425,20 @@ ec_getlen(struct group *group)
return ((roundup(group->spec->bits, 8) * 2) / 8);
 }
 
+/*
+ * Note that the shared secret only includes the x value:
+ *
+ * See RFC 5903, 7. ECP Key Exchange Data Formats:
+ *   The Diffie-Hellman shared secret value consists of the x value of the
+ *   Diffie-Hellman common value.
+ * See also RFC 5903, 9. Changes from RFC 4753.
+ */
+int
+ec_secretlen(struct group *group)
+{
+   return (ec_getlen(group) / 2);
+}
+
 int
 ec_create_exchange(struct group *group, u_int8_t *buf)
 {
@@ -421,7 +448,7 @@ ec_create_exchange(struct group *group, u_int8_t *buf)
bzero(buf, len);
 
return (ec_point2raw(group, EC_KEY_get0_public_key(group->ec),
-   buf, len));
+   buf, len, EC_POINT2RAW_FULL));
 }
 
 int
@@ -458,7 +485,8 @@ ec_create_shared(struct group *group, u_int8_t *secret, 
u_int8_t *exchange)
if (!EC_POINT_mul(ecgroup, secretp, NULL, exchangep, privkey, NULL))
goto done;
 
-   ret = ec_point2raw(group, secretp, secret, ec_getlen(group));
+   ret = ec_point2raw(group, secretp, secret, ec_secretlen(group),
+   EC_POINT2RAW_XONLY);
 
  done:
if (exkey != NULL)
@@ -473,7 +501,7 @@ ec_create_shared(struct group *group, u_int8_t *secret, 
u_int8_t *exchange)
 
 int
 ec_point2raw(struct group *group, const EC_POINT *point,
-u_int8_t *buf, size_t len)
+u_int8_t *buf, size_t len, int mode)
 {
const EC_GROUP  *ecgroup = NULL;
BN_CTX  *bnctx = NULL;
@@ -490,9 +518,19 @@ ec_point2raw(struct group *group, const EC_POINT *point,
goto done;
 
eclen = ec_getlen(group);
-   if (len < eclen)
+   switch (mode) {
+   case EC_POINT2RAW_XONLY:
+   xlen = eclen / 2;
+   ylen = 0;
+   break;
+   case EC_POINT2RAW_FULL:
+   xlen = ylen = eclen / 2;
+   break;
+   default:
+   goto done;
+   }
+   if (len < xlen + ylen)
goto done;
-   xlen = ylen = eclen / 2;
 
if ((ecgroup = EC_KEY_get0_group(group->ec)) == NULL)
goto done;
@@ -513,10 +551,12 @@ ec_point2raw(struct group *group, const EC_POINT *point,
if (!BN_bn2bin(x, buf + xoff))
goto done;
 
-   yoff = (ylen - BN_num_bytes(y)) + xlen;
-   bzero(buf + xlen, yoff - xlen);
-   if (!BN_bn2bin(y, buf + yoff))
-   goto done;
+   if (ylen > 0) {
+   yoff = (ylen - BN_num_bytes(y)) + xlen;
+   bzero(buf + xlen, yoff - xlen);
+   if (!BN_bn2bin(y, buf + yoff))
+   goto done;
+   }
 
ret = 0;
  done:
diff --git a/sbin/isakmpd/dh.h b/sbin/isakmpd/dh.h
index e04fdd90476..b258479fbf6 100644
--- a/sbin/isakmpd/dh.h
+++ b/sbin/isakmpd/dh.h
@@ -43,6 +43,7 @@ struct group {
 
int (*init)(struct group *);
int (*getlen)(struct group *);
+   int (*secretlen)(struct group *);
int (*exchange)(struct group 

Re: libfuse: improved command line parsing

2017-10-30 Thread Helg Bredow
On Sat, 28 Oct 2017 10:07:39 +0200
Martin Pieuchot  wrote:

> On 17/10/17(Tue) 14:26, Helg Bredow wrote:
> > [...] 
> > I've split the patch. This one improves argument and option parsing so that 
> > almost all sshfs arguments and options will now parse. It won't recognise 
> > -ocache=no since fuse_opt_match() is incorrect (fuse will treat it the same 
> > as -ocache=yes). I'll submit a patch for that some other time.
> 
> Nice.
> 
> > Running check_sym tells me:
> > 
> > No dynamic export changes
> > External reference changes:
> > added:
> > atoi
> > strchr
> > strsep
> > strstr
> > strtoul
> > 
> > I take that to mean that there is no need to bump the major or minor 
> > version for this patch. Is that correct?
> > 
> 
> That's correct.
> 
> Regarding fuse options, I'd suggest writing more tests and fuzzing this
> code.  Complex argument parsing is hard to get right.
> 

I've tried fuzzing this logic using AFL, indirectly by calling 
fuse_parse_cmdline() but I'm not sure it's that useful. It'll just end up 
supplying nonsensical arguments that never match and won't execute much of the 
other code paths. I've tested this pretty exhaustively and am confident that 
it's as stable as I can make it. I'll post the regression tests that I've been 
using in a separate email. All the changes that I'm making to fuse are a result 
of testing and not auditing the code.

I agree that this is hard to get right and it took me a few attempts to 
simplify. Your comments did prompt me to try a few more tests and I've found 
and fixed another edge case. It now prints a useful message if you don't supply 
a value to an argument that expects a value. e.g. "sshfs -p".
 
> Comments inline.

Responses and updated patch below.

> 
> > Index: fuse_opt.c
[...]
> > -   keyval = 0;
> > +#define DISCARD 0
> > +#define KEEP 1
> > +#define NEED_ANOTHER_ARG 2
> 
> I'd prefix these defines with _FOPT or w/o underscore if they are part
> of the API and move them on the top of the file since you use them in
> multiple functions.
>

Done.
 
> >  
> > -   /* check if it is a key=value entry */
> > -   idx = strcspn(val, "=");
> > -   if (idx != strlen(val)) {
> > -   idx++;
> > -   keyval = 1;
> > -   }
> > +   if (o == NULL)
> > +   return KEEP;
> 
> I know it's note your code, but one-letter variable should be avoided.
> It's hard to understand what they represent.  But let's not clutter this
> diff :)
> 

Agreed. I've been careful not to tidy the code at the same time.

> > +
> > +   keyval = 0;
> >  
> > for(; o->templ; o++) {
> > -   if ((keyval && strncmp(val, o->templ, idx) == 0) ||
> > -   (!keyval && strcmp(val, o->templ) == 0)) {
> > +   /* check key=value or -p n */
> > +   idx = strcspn(o->templ, "= ");
> > +
> > +   if (strncmp(val, o->templ, idx) == 0) {
> > +   if (o->templ[idx] == '=') {
> > +   keyval = 1;
> > +   val = [idx + 1];
> 
> How can you be sure that val[idx + 1] is valid?  For example:
> 
>   o->temp = { 'k', 'e', 'y', '=', 'v', 'a', 'l', '\0' }
>   val = { 'k', '\0' }

This example is not possible. The previous strncmp() test guarantees that both 
0->templ and val are identical up to either a '=' or ' '. val[idx + 1] will at 
worst equal '\0'.

   o->temp = { 'k', 'e', 'y', '=', 'v', 'a', 'l', '\0' }
   val = { 'k', 'e', 'y', '=', '\0' }

> 
> This looks like an overflow to me.  Well not yet, but when you
> dereference `val' below.
> 
> > +   } else if (o->templ[idx] == ' ') {
> > +   keyval = 1;
> > +   if (idx == strlen(val)) {
> > +   /* ask for next arg to be included */
> > +   return NEED_ANOTHER_ARG;
> > +   } else if (strchr(o->templ, '%') != NULL) {
> > +   val = [idx];
> 
> Same here, how can you be sure that val[idx] is valid?

Worst case it will be '\0'. Even by supplying a matching 'opt=' or '-p' I can't 
get it to crash.

> 
> > +   }
> > +   }
> > +
> > if (o->val == FUSE_OPT_KEY_DISCARD)
> > -   return (1);
> > +   return (DISCARD);
> > +
> > +   if (o->val == FUSE_OPT_KEY_KEEP)
> > +   return (KEEP);
> >  
> > if (FUSE_OPT_IS_OPT_KEY(o)) {
> > -   if (keyval)
> > -   ret = f(data, [idx], o->val, arg);
> > -   else
> > -   ret = f(data, val, o->val, arg);
> > -   }
> > +   if (f == NULL)
> > +   return KEEP;
> >  
> > -   if (o->off != ULONG_MAX && data && o->val >= 0) {
> > -  

Re: libfuse: fuse.c null checks and others

2017-10-30 Thread Helg Bredow
On Sat, 28 Oct 2017 09:24:55 +0200
Martin Pieuchot  wrote:

> On 25/10/17(Wed) 13:27, Helg Bredow wrote:
> > I've included different minor patches below as one patch. I haven't split 
> > into separate patches since the changes are not complex and easy to audit. 
> > 
> > Here's what it does:
> > 
> > Almost all functions in fuse.c do not check if the arguments are null. This 
> > patch adds null checks where appropriate.
> > 
> > Some arguments are incorrectly flagged as unused. Delete "unused" if the 
> > argument is used in the function.
> > 
> > The wrong version macro is used in dump_version() and is inconsistent with 
> > that used in fuse_version(). FUSE_USE_VERSION is intended to be defined by 
> > file system to specify which backward compatible version of libfuse they 
> > require.
> > 
> > fuse_loop_mt() is not implemented so return -1 rather than success. If a 
> > file system tries to call it then it should be obvious that it's not going 
> > to work.
> > 
> > fuse_main() declared redundant variables due to the lack of NULL checks 
> > before assignment. We now check for NULL so can safely pass NULL instead.
> > 
> > Tested with a regression test that passes all NULL arguments to exported 
> > functions.
> > 
> > Something to consider is that fuse_is_lib_option() is deprecated. Should we 
> > remove it from libfuse to stay strictly at version 26?
> 
> Testing for NULL is good.  However returning -1 in fuse_loop_mt() and
> changing the version number might break ports.  So if these changes go
> in, you should watch for regression on ports@ at least.

I've reverted fuse_loop_mt(). However, I don't feel that this is correct. If a 
file system expects it to work then it should fail to make it clear that the 
functionality is not implemented. sshfs calls it but because 
fuse_parse_cmdline() always returns 0 for multithreaded it never gets called 
(you normally need to specify -s on the command line for multithreaded fuse 
file systems to run in a single thread. I've tested to see what sshfs does when 
fuse_loop_mt() returns -1 and it simply exits with no message, just as it does 
when it returns 0.

I also reverted the version macro change. The value for both macros is the same 
(26) and they will likely stay in step if the version is incremented. It's not 
using the correct macro though.

> 
> We can remove fuse_is_lib_option() if nothing in ports use it.  A good
> start to search for it is codesearch.debian.net.  You can also ask
> porters to do a grep on unpacked port tree.
> 

I didn't know about this site, it will be useful. I was going to search the 
ports source tree but this saves me the trouble and also expands the search. 
sshfs calls this fuse_is_lib_option() so it will have to stay. However, it also 
needs work as it doesn't behave the same as the Linux version.

> Could you provide a diff including only the NULL check and the removal
> of "unused" markers?
> 

Updated patch is below. I had missed a NULL check in fuse_set_signal_handlers() 
so have added it. The signal handling only works if the file system is not 
busy, otherwise it becomes very difficult to kill. fuse_loop() also doesn't set 
the signal handlers but it should. More things to fix later.

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.31
diff -u -p -r1.31 fuse.c
--- fuse.c  25 Oct 2017 09:29:46 -  1.31
+++ fuse.c  28 Oct 2017 13:39:26 -
@@ -71,6 +71,9 @@ fuse_loop(struct fuse *fuse)
ssize_t n;
int ret;
 
+   if (fuse == NULL)
+   return (-1);
+
fuse->fc->kq = kqueue();
if (fuse->fc->kq == -1)
return (-1);
@@ -156,6 +159,9 @@ fuse_mount(const char *dir, unused struc
struct fuse_chan *fc;
const char *errcause;
 
+   if (dir == NULL)
+   return (NULL);
+
fc = calloc(1, sizeof(*fc));
if (fc == NULL)
return (NULL);
@@ -197,9 +203,9 @@ bad:
 }
 
 void
-fuse_unmount(const char *dir, unused struct fuse_chan *ch)
+fuse_unmount(const char *dir, struct fuse_chan *ch)
 {
-   if (ch->dead)
+   if (ch == NULL || ch->dead)
return;
 
if (unmount(dir, MNT_UPDATE) == -1)
@@ -207,7 +213,7 @@ fuse_unmount(const char *dir, unused str
 }
 
 int
-fuse_is_lib_option(unused const char *opt)
+fuse_is_lib_option(const char *opt)
 {
return (fuse_opt_match(fuse_core_opts, opt));
 }
@@ -215,6 +221,9 @@ fuse_is_lib_option(unused const char *op
 int
 fuse_chan_fd(struct fuse_chan *ch)
 {
+   if (ch == NULL)
+   return (-1);
+
return (ch->fd);
 }
 
@@ -233,11 +242,14 @@ fuse_loop_mt(unused struct fuse *fuse)
 struct fuse *
 fuse_new(struct fuse_chan *fc, unused struct fuse_args *args,
 const struct fuse_operations *ops, unused size_t size,
-unused void *userdata)
+void *userdata)
 {
struct fuse *fuse;

libfuse: regress test for command line parsing

2017-10-30 Thread Helg Bredow
These tests confirm that currently supported options are parsed correctly by 
libfuse. They assume that the patches I've submitted previously have been 
applied.

Review and suggestions for improvement are welcome.

Patch to Makefile is below and new tests are attached.


Index: Makefile
===
RCS file: /cvs/src/regress/lib/libfuse/Makefile,v
retrieving revision 1.1
diff -u -p -p -u -r1.1 Makefile
--- Makefile9 Aug 2013 16:20:10 -   1.1
+++ Makefile29 Oct 2017 08:44:36 -
@@ -5,6 +5,9 @@ REGRESS_TARGETS+= run-fuse-opt-add-opt-e
 REGRESS_TARGETS+= run-fuse-opt-add-arg
 REGRESS_TARGETS+= run-fuse-opt-insert-arg
 REGRESS_TARGETS+= run-fuse-opt-match
+REGRESS_TARGETS+= run-fuse-opt-parse
+REGRESS_TARGETS+= run-fuse-parse-cmdline
 
 LDFLAGS+= -lfuse
 CLEANFILES= fuse-opt-add-opt
@@ -12,6 +15,9 @@ CLEANFILES+=fuse-opt-add-opt-escaped
 CLEANFILES+=fuse-opt-add-arg
 CLEANFILES+=fuse-opt-insert-arg
 CLEANFILES+=fuse-opt-match
+CLEANFILES+=fuse-opt-parse
+CLEANFILES+=fuse-parse-cmdline
 
 .PHONY: ${REGRESS_TARGETS}
 
@@ -25,5 +31,11 @@ run-fuse-opt-insert-arg: fuse-opt-insert
./fuse-opt-insert-arg
 run-fuse-opt-match: fuse-opt-match
./fuse-opt-match
+run-fuse-opt-parse: fuse-opt-parse
+   ./fuse-opt-parse
+run-fuse-parse-cmdline: fuse-parse-cmdline
+   ./fuse-parse-cmdline
 
 .include 


fuse-opt-parse.c
Description: fuse-opt-parse.c


fuse-parse-cmdline.c
Description: fuse-parse-cmdline.c


Re: makefs(8): add EFI platform-id for UFI

2017-10-30 Thread YASUOKA Masahiko
On Mon, 30 Oct 2017 16:35:03 +0800
"Michael W. Bombardieri"  wrote:
> On Mon, Oct 30, 2017 at 05:27:29PM +0900, YASUOKA Masahiko wrote:
>> Hi,
>> 
>> I'd like to add a platform-id for EFI boot.
>> 
>> ok?
>> 
>> diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
>> b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
>> index 46ec432bc84..376c42f5dc3 100644
>> --- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
>> +++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
>> @@ -104,6 +104,8 @@ cd9660_add_boot_disk(iso9660_disk *diskStructure, const 
>> char *boot_info)
>>  new_image->system = ET_SYS_PPC;
>>  else if (strcmp(sysname, "macppc") == 0)
>>  new_image->system = ET_SYS_MAC;
>> +else if (strcmp(sysname, "efi") == 0)
>> +new_image->system = ET_SYS_EFI;
>>  else {
>>  warnx("boot disk system must be "
>>"i386, macppc, or powerpc");
> 
> Should efi be added to the warning message too, or changing it to something 
> like
> Unknown boot disk system?

Yes, it should.  Thanks,

Let me update the diff.

diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
index 46ec432bc84..125b6d69be0 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
@@ -104,9 +104,11 @@ cd9660_add_boot_disk(iso9660_disk *diskStructure, const 
char *boot_info)
new_image->system = ET_SYS_PPC;
else if (strcmp(sysname, "macppc") == 0)
new_image->system = ET_SYS_MAC;
+   else if (strcmp(sysname, "efi") == 0)
+   new_image->system = ET_SYS_EFI;
else {
warnx("boot disk system must be "
- "i386, macppc, or powerpc");
+ "i386, macppc, powerpc or efi");
free(temp);
free(new_image);
return 0;
@@ -335,12 +337,12 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
first_sector)
int used_sectors;
int num_entries = 0;
int catalog_sectors;
-   struct boot_catalog_entry *x86_head, *mac_head, *ppc_head,
+   struct boot_catalog_entry *x86_head, *mac_head, *ppc_head, *efi_head,
*valid_entry, *default_entry, *temp, *head, **headp, *next;
struct cd9660_boot_image *tmp_disk;
 
headp = NULL;
-   x86_head = mac_head = ppc_head = NULL;
+   x86_head = mac_head = ppc_head = efi_head = NULL;
 
/* If there are no boot disks, don't bother building boot information */
if (TAILQ_EMPTY(>boot_images))
@@ -413,6 +415,9 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
first_sector)
case ET_SYS_MAC:
headp = _head;
break;
+   case ET_SYS_EFI:
+   headp = _head;
+   break;
default:
warnx("%s: internal error: unknown system type",
__func__);
diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.h 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
index 43483018ef3..f34b19b9ba0 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.h
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
@@ -41,6 +41,7 @@
 #defineET_SYS_X86  0
 #defineET_SYS_PPC  1
 #defineET_SYS_MAC  2
+#defineET_SYS_EFI  0xef
 
 #define ET_BOOT_ENTRY_SIZE 0x20
 
diff --git a/usr.sbin/makefs/makefs.8 b/usr.sbin/makefs/makefs.8
index 12cf499261f..3582dda07e9 100644
--- a/usr.sbin/makefs/makefs.8
+++ b/usr.sbin/makefs/makefs.8
@@ -215,8 +215,9 @@ where
 is one of
 .Ql i386 ,
 .Ql macppc ,
+.Ql powerpc
 or
-.Ql powerpc .
+.Ql efi .
 .It Sy generic-bootimage
 Load a generic boot image into the first 32K of the CD9660 image.
 .It Sy hard-disk-boot



Re: makefs(8): add EFI platform-id for UFI

2017-10-30 Thread Michael W. Bombardieri
On Mon, Oct 30, 2017 at 05:27:29PM +0900, YASUOKA Masahiko wrote:
> Hi,
> 
> I'd like to add a platform-id for EFI boot.
> 
> ok?
> 
> diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
> b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
> index 46ec432bc84..376c42f5dc3 100644
> --- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
> +++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
> @@ -104,6 +104,8 @@ cd9660_add_boot_disk(iso9660_disk *diskStructure, const 
> char *boot_info)
>   new_image->system = ET_SYS_PPC;
>   else if (strcmp(sysname, "macppc") == 0)
>   new_image->system = ET_SYS_MAC;
> + else if (strcmp(sysname, "efi") == 0)
> + new_image->system = ET_SYS_EFI;
>   else {
>   warnx("boot disk system must be "
> "i386, macppc, or powerpc");

Should efi be added to the warning message too, or changing it to something like
Unknown boot disk system?

> @@ -335,12 +337,12 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
> first_sector)
>   int used_sectors;
>   int num_entries = 0;
>   int catalog_sectors;
> - struct boot_catalog_entry *x86_head, *mac_head, *ppc_head,
> + struct boot_catalog_entry *x86_head, *mac_head, *ppc_head, *efi_head,
>   *valid_entry, *default_entry, *temp, *head, **headp, *next;
>   struct cd9660_boot_image *tmp_disk;
>  
>   headp = NULL;
> - x86_head = mac_head = ppc_head = NULL;
> + x86_head = mac_head = ppc_head = efi_head = NULL;
>  
>   /* If there are no boot disks, don't bother building boot information */
>   if (TAILQ_EMPTY(>boot_images))
> @@ -413,6 +415,9 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
> first_sector)
>   case ET_SYS_MAC:
>   headp = _head;
>   break;
> + case ET_SYS_EFI:
> + headp = _head;
> + break;
>   default:
>   warnx("%s: internal error: unknown system type",
>   __func__);
> diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.h 
> b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> index 43483018ef3..f527bc56b46 100644
> --- a/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> +++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> @@ -41,6 +41,7 @@
>  #define  ET_SYS_X86  0
>  #define  ET_SYS_PPC  1
>  #define  ET_SYS_MAC  2
> +#define  ET_SYS_EFI  0xfe
>  
>  #define ET_BOOT_ENTRY_SIZE 0x20
>  
> 



Re: add one more softnet taskq

2017-10-30 Thread Martin Pieuchot
On 30/10/17(Mon) 10:32, Peter Hessler wrote:
> :diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
> :index 95c9194efcb..33cc3161bcb 100644
> :--- a/sys/netinet/ip_input.c
> :+++ b/sys/netinet/ip_input.c
> :@@ -1839,5 +1839,5 @@ void
> : ip_send(struct mbuf *m)
> : {
> : mq_enqueue(_mq, m);
> :-task_add(softnettq, _task);
> :+task_add(net_tq(0), _task);
> : }
> 
> I'm only singling out one of them, but the comment applies to all of
> this style.
> 
> What's the motivation to do net_tq(0) here, instead of hashing the
> if_index?

There's no such "rational", we can indeed use `ph_ifidx'.



Re: add one more softnet taskq

2017-10-30 Thread Peter Hessler
On 2017 Oct 30 (Mon) at 08:36:34 +0100 (+0100), Alexandr Nedvedicky wrote:
:Hello,
:
:patch below adds additional softnet taskq. This will allow certain degree of
:parallelism for packet processing in pf_test(). The current plan is to let
:packets received by even NICs (even ifindex) to be processed by task0, packets
:received by odd NICs (odd ifindex) by task1.
:
:big thanks should go to mpi@, who 'programmed' me to program the patch below.
:
:OK?
:
:thanks and
:regards
:sasha
:
:diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
:index 95c9194efcb..33cc3161bcb 100644
:--- a/sys/netinet/ip_input.c
:+++ b/sys/netinet/ip_input.c
:@@ -1839,5 +1839,5 @@ void
: ip_send(struct mbuf *m)
: {
:   mq_enqueue(_mq, m);
:-  task_add(softnettq, _task);
:+  task_add(net_tq(0), _task);
: }

I'm only singling out one of them, but the comment applies to all of
this style.

What's the motivation to do net_tq(0) here, instead of hashing the
if_index?


-- 
In specifications, Murphy's Law supersedes Ohm's.



Re: add one more softnet taskq

2017-10-30 Thread Martin Pieuchot
On 30/10/17(Mon) 08:36, Alexandr Nedvedicky wrote:
> Hello,
> 
> patch below adds additional softnet taskq. This will allow certain degree of
> parallelism for packet processing in pf_test(). The current plan is to let
> packets received by even NICs (even ifindex) to be processed by task0, packets
> received by odd NICs (odd ifindex) by task1.
> 
> big thanks should go to mpi@, who 'programmed' me to program the patch below.
> 
> OK?

ok with SOFTNET_TASKS defined to 1, then we can bump it when needed.

> 8<---8<---8<--8<
> diff --git a/sys/net/if.c b/sys/net/if.c
> index e9e9f07add1..d688456b677 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -224,7 +224,9 @@ int   net_livelocked(void);
>  int  ifq_congestion;
>  
>  int   netisr;
> -struct taskq *softnettq;
> +
> +#define  SOFTNET_TASKS   2
> +struct taskq *softnettq[SOFTNET_TASKS];
>  
>  struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
>  
> @@ -240,6 +242,8 @@ struct rwlock netlock = RWLOCK_INITIALIZER("netlock");
>  void
>  ifinit(void)
>  {
> + unsigned inti;
> +
>   /*
>* most machines boot with 4 or 5 interfaces, so size the initial map
>* to accomodate this
> @@ -248,9 +252,11 @@ ifinit(void)
>  
>   timeout_set(_tick_to, net_tick, _tick_to);
>  
> - softnettq = taskq_create("softnet", 1, IPL_NET, TASKQ_MPSAFE);
> - if (softnettq == NULL)
> - panic("unable to create softnet taskq");
> + for (i = 0; i < SOFTNET_TASKS; i++) {
> + softnettq[i] = taskq_create("softnet", 1, IPL_NET, 
> TASKQ_MPSAFE);
> + if (softnettq[i] == NULL)
> + panic("unable to create softnet taskq");
> + }
>  
>   net_tick(_tick_to);
>  }
> @@ -725,7 +731,7 @@ if_input(struct ifnet *ifp, struct mbuf_list *ml)
>  #endif
>  
>   if (mq_enlist(>if_inputqueue, ml) == 0)
> - task_add(softnettq, ifp->if_inputtask);
> + task_add(net_tq(ifp->if_index), ifp->if_inputtask);
>  }
>  
>  int
> @@ -1025,15 +1031,15 @@ if_detach(struct ifnet *ifp)
>   ifp->if_watchdog = NULL;
>  
>   /* Remove the input task */
> - task_del(softnettq, ifp->if_inputtask);
> + task_del(net_tq(ifp->if_index), ifp->if_inputtask);
>   mq_purge(>if_inputqueue);
>  
>   /* Remove the watchdog timeout & task */
>   timeout_del(ifp->if_slowtimo);
> - task_del(softnettq, ifp->if_watchdogtask);
> + task_del(net_tq(ifp->if_index), ifp->if_watchdogtask);
>  
>   /* Remove the link state task */
> - task_del(softnettq, ifp->if_linkstatetask);
> + task_del(net_tq(ifp->if_index), ifp->if_linkstatetask);
>  
>  #if NBPFILTER > 0
>   bpfdetach(ifp);
> @@ -1583,7 +1589,7 @@ if_linkstate(struct ifnet *ifp)
>  void
>  if_link_state_change(struct ifnet *ifp)
>  {
> - task_add(softnettq, ifp->if_linkstatetask);
> + task_add(net_tq(ifp->if_index), ifp->if_linkstatetask);
>  }
>  
>  /*
> @@ -1599,7 +1605,7 @@ if_slowtimo(void *arg)
>  
>   if (ifp->if_watchdog) {
>   if (ifp->if_timer > 0 && --ifp->if_timer == 0)
> - task_add(softnettq, ifp->if_watchdogtask);
> + task_add(net_tq(ifp->if_index), ifp->if_watchdogtask);
>   timeout_add(ifp->if_slowtimo, hz / IFNET_SLOWHZ);
>   }
>   splx(s);
> @@ -2881,3 +2887,13 @@ unhandled_af(int af)
>  {
>   panic("unhandled af %d", af);
>  }
> +
> +struct taskq *
> +net_tq(unsigned int ifindex)
> +{
> + struct taskq *t = NULL;
> +
> + t = softnettq[ifindex % SOFTNET_TASKS];
> +
> + return (t);
> +}
> diff --git a/sys/net/if.h b/sys/net/if.h
> index 89867eac340..6a0770a8ea0 100644
> --- a/sys/net/if.h
> +++ b/sys/net/if.h
> @@ -489,6 +489,7 @@ void  if_congestion(void);
>  int  if_congested(void);
>  __dead void  unhandled_af(int);
>  int  if_setlladdr(struct ifnet *, const uint8_t *);
> +struct taskq * net_tq(unsigned int);
>  
>  #endif /* _KERNEL */
>  
> diff --git a/sys/net/if_loop.c b/sys/net/if_loop.c
> index 277e7f966a2..e9f58a4ee52 100644
> --- a/sys/net/if_loop.c
> +++ b/sys/net/if_loop.c
> @@ -244,7 +244,7 @@ looutput(struct ifnet *ifp, struct mbuf *m, struct 
> sockaddr *dst,
>   m->m_pkthdr.ph_family = dst->sa_family;
>   if (mq_enqueue(>if_inputqueue, m))
>   return ENOBUFS;
> - task_add(softnettq, ifp->if_inputtask);
> + task_add(net_tq(ifp->if_index), ifp->if_inputtask);
>  
>   return (0);
>  }
> diff --git a/sys/net/if_pflow.c b/sys/net/if_pflow.c
> index 38efb02be7e..91a61fe4c15 100644
> --- a/sys/net/if_pflow.c
> +++ b/sys/net/if_pflow.c
> @@ -286,7 +286,7 @@ pflow_clone_destroy(struct ifnet *ifp)
>   if (timeout_initialized(>sc_tmo_tmpl))
>   timeout_del(>sc_tmo_tmpl);
>   pflow_flush(sc);
> - task_del(softnettq, >sc_outputtask);
> + task_del(net_tq(ifp->if_index), >sc_outputtask);
>   mq_purge(>sc_outputqueue);
>   

Re: makefs(8): add a platform-id for EFI

2017-10-30 Thread YASUOKA Masahiko
Let me update the diff and subject.

On Mon, 30 Oct 2017 17:27:29 +0900 (JST)
YASUOKA Masahiko  wrote:
> diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.h 
> b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> index 43483018ef3..f527bc56b46 100644
> --- a/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> +++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> @@ -41,6 +41,7 @@
>  #define  ET_SYS_X86  0
>  #define  ET_SYS_PPC  1
>  #define  ET_SYS_MAC  2
> +#define  ET_SYS_EFI  0xfe
>  
>  #define ET_BOOT_ENTRY_SIZE 0x20
>  
> 

The platform id must be 0xef.  Also update the man page.

ok?

Add platform-id for EFI.

diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
index 46ec432bc84..376c42f5dc3 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
@@ -104,6 +104,8 @@ cd9660_add_boot_disk(iso9660_disk *diskStructure, const 
char *boot_info)
new_image->system = ET_SYS_PPC;
else if (strcmp(sysname, "macppc") == 0)
new_image->system = ET_SYS_MAC;
+   else if (strcmp(sysname, "efi") == 0)
+   new_image->system = ET_SYS_EFI;
else {
warnx("boot disk system must be "
  "i386, macppc, or powerpc");
@@ -335,12 +337,12 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
first_sector)
int used_sectors;
int num_entries = 0;
int catalog_sectors;
-   struct boot_catalog_entry *x86_head, *mac_head, *ppc_head,
+   struct boot_catalog_entry *x86_head, *mac_head, *ppc_head, *efi_head,
*valid_entry, *default_entry, *temp, *head, **headp, *next;
struct cd9660_boot_image *tmp_disk;
 
headp = NULL;
-   x86_head = mac_head = ppc_head = NULL;
+   x86_head = mac_head = ppc_head = efi_head = NULL;
 
/* If there are no boot disks, don't bother building boot information */
if (TAILQ_EMPTY(>boot_images))
@@ -413,6 +415,9 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
first_sector)
case ET_SYS_MAC:
headp = _head;
break;
+   case ET_SYS_EFI:
+   headp = _head;
+   break;
default:
warnx("%s: internal error: unknown system type",
__func__);
diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.h 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
index 43483018ef3..f34b19b9ba0 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.h
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
@@ -41,6 +41,7 @@
 #defineET_SYS_X86  0
 #defineET_SYS_PPC  1
 #defineET_SYS_MAC  2
+#defineET_SYS_EFI  0xef
 
 #define ET_BOOT_ENTRY_SIZE 0x20
 
diff --git a/usr.sbin/makefs/makefs.8 b/usr.sbin/makefs/makefs.8
index 12cf499261f..3582dda07e9 100644
--- a/usr.sbin/makefs/makefs.8
+++ b/usr.sbin/makefs/makefs.8
@@ -215,8 +215,9 @@ where
 is one of
 .Ql i386 ,
 .Ql macppc ,
+.Ql powerpc
 or
-.Ql powerpc .
+.Ql efi .
 .It Sy generic-bootimage
 Load a generic boot image into the first 32K of the CD9660 image.
 .It Sy hard-disk-boot



makefs(8): add EFI platform-id for UFI

2017-10-30 Thread YASUOKA Masahiko
Hi,

I'd like to add a platform-id for EFI boot.

ok?

diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
index 46ec432bc84..376c42f5dc3 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
@@ -104,6 +104,8 @@ cd9660_add_boot_disk(iso9660_disk *diskStructure, const 
char *boot_info)
new_image->system = ET_SYS_PPC;
else if (strcmp(sysname, "macppc") == 0)
new_image->system = ET_SYS_MAC;
+   else if (strcmp(sysname, "efi") == 0)
+   new_image->system = ET_SYS_EFI;
else {
warnx("boot disk system must be "
  "i386, macppc, or powerpc");
@@ -335,12 +337,12 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
first_sector)
int used_sectors;
int num_entries = 0;
int catalog_sectors;
-   struct boot_catalog_entry *x86_head, *mac_head, *ppc_head,
+   struct boot_catalog_entry *x86_head, *mac_head, *ppc_head, *efi_head,
*valid_entry, *default_entry, *temp, *head, **headp, *next;
struct cd9660_boot_image *tmp_disk;
 
headp = NULL;
-   x86_head = mac_head = ppc_head = NULL;
+   x86_head = mac_head = ppc_head = efi_head = NULL;
 
/* If there are no boot disks, don't bother building boot information */
if (TAILQ_EMPTY(>boot_images))
@@ -413,6 +415,9 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
first_sector)
case ET_SYS_MAC:
headp = _head;
break;
+   case ET_SYS_EFI:
+   headp = _head;
+   break;
default:
warnx("%s: internal error: unknown system type",
__func__);
diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.h 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
index 43483018ef3..f527bc56b46 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.h
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
@@ -41,6 +41,7 @@
 #defineET_SYS_X86  0
 #defineET_SYS_PPC  1
 #defineET_SYS_MAC  2
+#defineET_SYS_EFI  0xfe
 
 #define ET_BOOT_ENTRY_SIZE 0x20
 



makefs(8) eltrito diffs 2/2

2017-10-30 Thread YASUOKA Masahiko
Hi,

> I'd like to fix two bugs which cause problems of the order of the boot
> entries.  A diff which add EFI support will follow them.

next one

ok?

Fix a bug make the boot entries' order reverse, introduced at 1.8 on NetBSD.

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/cd9660_eltorito.c.diff?r1=1.7=1.8_with_tag=MAIN

diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
index 7de2bfc83f9..46ec432bc84 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
@@ -155,11 +155,12 @@ cd9660_add_boot_disk(iso9660_disk *diskStructure, const 
char *boot_info)
break;
}
 
-   if (tmp_image == NULL) {
+   if (tmp_image == NULL)
TAILQ_INSERT_HEAD(>boot_images, new_image,
image_list);
-   } else
-   TAILQ_INSERT_BEFORE(tmp_image, new_image, image_list);
+   else
+   TAILQ_INSERT_AFTER(>boot_images, tmp_image,
+   new_image, image_list);
 
new_image->serialno = diskStructure->image_serialno++;
 



makefs(8) eltrito diffs 1/2

2017-10-30 Thread YASUOKA Masahiko
Hi,

I'd like to fix two bugs which cause problems of the order of the boot
entries.  A diff which add EFI support will follow them.

ok?


Initialize boot_catalog_entry's entry_type properly.  This had been
missing but the type was used in cd9660_setup_boot().

diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
index 409298d5ca6..7de2bfc83f9 100644
--- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
+++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
@@ -221,6 +221,7 @@ cd9660_boot_setup_validation_entry(char sys)
size_t i;
entry = cd9660_init_boot_catalog_entry();
 
+   entry->entry_type = ET_ENTRY_VE;
ve = >entry_data.VE;
 
ve->header_id[0] = 1;
@@ -255,6 +256,7 @@ cd9660_boot_setup_default_entry(struct cd9660_boot_image 
*disk)
if (default_entry == NULL)
return NULL;
 
+   default_entry->entry_type = ET_ENTRY_IE;
ie = _entry->entry_data.IE;
 
ie->boot_indicator[0] = disk->bootable;
@@ -282,6 +284,7 @@ cd9660_boot_setup_section_head(char platform)
if (entry == NULL)
return NULL;
 
+   entry->entry_type = ET_ENTRY_SH;
sh = >entry_data.SH;
/* More by default. The last one will manually be set to 0x91 */
sh->header_indicator[0] = ET_SECTION_HEADER_MORE;
@@ -298,6 +301,7 @@ cd9660_boot_setup_section_entry(struct cd9660_boot_image 
*disk)
if ((entry = cd9660_init_boot_catalog_entry()) == NULL)
return NULL;
 
+   entry->entry_type = ET_ENTRY_SE;
se = >entry_data.SE;
 
se->boot_indicator[0] = ET_BOOTABLE;



add one more softnet taskq

2017-10-30 Thread Alexandr Nedvedicky
Hello,

patch below adds additional softnet taskq. This will allow certain degree of
parallelism for packet processing in pf_test(). The current plan is to let
packets received by even NICs (even ifindex) to be processed by task0, packets
received by odd NICs (odd ifindex) by task1.

big thanks should go to mpi@, who 'programmed' me to program the patch below.

OK?

thanks and
regards
sasha

8<---8<---8<--8<
diff --git a/sys/net/if.c b/sys/net/if.c
index e9e9f07add1..d688456b677 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -224,7 +224,9 @@ int net_livelocked(void);
 intifq_congestion;
 
 int netisr;
-struct taskq   *softnettq;
+
+#defineSOFTNET_TASKS   2
+struct taskq   *softnettq[SOFTNET_TASKS];
 
 struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
 
@@ -240,6 +242,8 @@ struct rwlock netlock = RWLOCK_INITIALIZER("netlock");
 void
 ifinit(void)
 {
+   unsigned inti;
+
/*
 * most machines boot with 4 or 5 interfaces, so size the initial map
 * to accomodate this
@@ -248,9 +252,11 @@ ifinit(void)
 
timeout_set(_tick_to, net_tick, _tick_to);
 
-   softnettq = taskq_create("softnet", 1, IPL_NET, TASKQ_MPSAFE);
-   if (softnettq == NULL)
-   panic("unable to create softnet taskq");
+   for (i = 0; i < SOFTNET_TASKS; i++) {
+   softnettq[i] = taskq_create("softnet", 1, IPL_NET, 
TASKQ_MPSAFE);
+   if (softnettq[i] == NULL)
+   panic("unable to create softnet taskq");
+   }
 
net_tick(_tick_to);
 }
@@ -725,7 +731,7 @@ if_input(struct ifnet *ifp, struct mbuf_list *ml)
 #endif
 
if (mq_enlist(>if_inputqueue, ml) == 0)
-   task_add(softnettq, ifp->if_inputtask);
+   task_add(net_tq(ifp->if_index), ifp->if_inputtask);
 }
 
 int
@@ -1025,15 +1031,15 @@ if_detach(struct ifnet *ifp)
ifp->if_watchdog = NULL;
 
/* Remove the input task */
-   task_del(softnettq, ifp->if_inputtask);
+   task_del(net_tq(ifp->if_index), ifp->if_inputtask);
mq_purge(>if_inputqueue);
 
/* Remove the watchdog timeout & task */
timeout_del(ifp->if_slowtimo);
-   task_del(softnettq, ifp->if_watchdogtask);
+   task_del(net_tq(ifp->if_index), ifp->if_watchdogtask);
 
/* Remove the link state task */
-   task_del(softnettq, ifp->if_linkstatetask);
+   task_del(net_tq(ifp->if_index), ifp->if_linkstatetask);
 
 #if NBPFILTER > 0
bpfdetach(ifp);
@@ -1583,7 +1589,7 @@ if_linkstate(struct ifnet *ifp)
 void
 if_link_state_change(struct ifnet *ifp)
 {
-   task_add(softnettq, ifp->if_linkstatetask);
+   task_add(net_tq(ifp->if_index), ifp->if_linkstatetask);
 }
 
 /*
@@ -1599,7 +1605,7 @@ if_slowtimo(void *arg)
 
if (ifp->if_watchdog) {
if (ifp->if_timer > 0 && --ifp->if_timer == 0)
-   task_add(softnettq, ifp->if_watchdogtask);
+   task_add(net_tq(ifp->if_index), ifp->if_watchdogtask);
timeout_add(ifp->if_slowtimo, hz / IFNET_SLOWHZ);
}
splx(s);
@@ -2881,3 +2887,13 @@ unhandled_af(int af)
 {
panic("unhandled af %d", af);
 }
+
+struct taskq *
+net_tq(unsigned int ifindex)
+{
+   struct taskq *t = NULL;
+
+   t = softnettq[ifindex % SOFTNET_TASKS];
+
+   return (t);
+}
diff --git a/sys/net/if.h b/sys/net/if.h
index 89867eac340..6a0770a8ea0 100644
--- a/sys/net/if.h
+++ b/sys/net/if.h
@@ -489,6 +489,7 @@ voidif_congestion(void);
 intif_congested(void);
 __dead voidunhandled_af(int);
 intif_setlladdr(struct ifnet *, const uint8_t *);
+struct taskq * net_tq(unsigned int);
 
 #endif /* _KERNEL */
 
diff --git a/sys/net/if_loop.c b/sys/net/if_loop.c
index 277e7f966a2..e9f58a4ee52 100644
--- a/sys/net/if_loop.c
+++ b/sys/net/if_loop.c
@@ -244,7 +244,7 @@ looutput(struct ifnet *ifp, struct mbuf *m, struct sockaddr 
*dst,
m->m_pkthdr.ph_family = dst->sa_family;
if (mq_enqueue(>if_inputqueue, m))
return ENOBUFS;
-   task_add(softnettq, ifp->if_inputtask);
+   task_add(net_tq(ifp->if_index), ifp->if_inputtask);
 
return (0);
 }
diff --git a/sys/net/if_pflow.c b/sys/net/if_pflow.c
index 38efb02be7e..91a61fe4c15 100644
--- a/sys/net/if_pflow.c
+++ b/sys/net/if_pflow.c
@@ -286,7 +286,7 @@ pflow_clone_destroy(struct ifnet *ifp)
if (timeout_initialized(>sc_tmo_tmpl))
timeout_del(>sc_tmo_tmpl);
pflow_flush(sc);
-   task_del(softnettq, >sc_outputtask);
+   task_del(net_tq(ifp->if_index), >sc_outputtask);
mq_purge(>sc_outputqueue);
m_freem(sc->send_nam);
if (sc->so != NULL) {
@@ -1087,7 +1087,7 @@ pflow_sendout_v5(struct pflow_softc *sc)
h->time_sec = htonl(tv.tv_sec); /* XXX 2038 */
h->time_nanosec = htonl(tv.tv_nsec);
if (mq_enqueue(>sc_outputqueue, m) == 

Re: fine tuning PF_LOCK in pfioctl()

2017-10-30 Thread Alexandr Nedvedicky
Hello,


> > patch below move responsibility for PF_LOCK manipulation to particular ioctl
> > commands. The change is investment for future. It will allow us to gradually
> > move individual PF subsystems out of PF_LOCK scope.
> 
> I'm always afraid when an ioctl(2) logic contains multiple return or break
> statements and doesn't fit in a couple of lines.  When this happen I'd
> suggest to move the code to a function which can assert that the PF_LOCK()
> is held.
> 
> That said I agree that moving the lock down is some progress.  So ok
> with me.

Hrvoje is running the patch already. I'll give him some more time (like
another 10 hours or so) to run into troubles eventually. If I won't hear
back from him reporting problems with my change, I'll commit my change 
as-is.

I like your idea to move implementation of 'the big switch' cases to
standalone functions. Let's deal with it in follow up diff.

thanks and
regards
sasha