Re: [PATCH] ffs: always assign random inode generation numbers

[Theo de Raadt]

> On Sun, Jun 25, 2017 at 10:47:08PM -0600, Theo de Raadt wrote:
> >> :-) Speaking of signed integers, does it really need to be signed?
> >
> >Perhaps not.  Anyone know for sure?
> >
> >Of course this number should probably exclude 0 in it's range.
> Probably [0, 3] completely as those are reserved for unallocated
> entries, bad blocks, / and lost+found respectively.

it is the generation #... not the inode..

Re: [PATCH] ffs: always assign random inode generation numbers

[Klemens Nanni]

On Sun, Jun 25, 2017 at 10:47:08PM -0600, Theo de Raadt wrote:

:-) Speaking of signed integers, does it really need to be signed?


Perhaps not.  Anyone know for sure?

Of course this number should probably exclude 0 in it's range.

Probably [0, 3] completely as those are reserved for unallocated
entries, bad blocks, / and lost+found respectively.

Re: [PATCH] ffs: always assign random inode generation numbers

[Theo de Raadt]

> :-) Speaking of signed integers, does it really need to be signed?

Perhaps not.  Anyone know for sure?

Of course this number should probably exclude 0 in it's range.

Re: Add Diffie-Hellman group negotiation to iked

[Tim Stewart]

Hi,

In this message I've tried to encode everything I've done to allow
strongSwan on Android to connect with iked, including the latest patch.
I have also verified that it breaks neither initial negotiation nor
Child SA rekeying for OpenBSD, Windows, and strongSwan (on Android)
clients.

Stuart Henderson  writes:

> On 2017/05/22 01:52, Tim Stewart wrote:
>> Hello again,
>>
>> Tim Stewart  writes:
>>
>> > Tim Stewart  writes:
>> >
>> >> This patch teaches iked to reject a KE with a Notify payload of type
>> >> INVALID_KE_PAYLOAD when the KE uses a different Diffie-Hellman group
>> >> than is configured locally.  The rejection indicates the desired
>> >> group.
>> >>
>> >> In my environment, this patch allows stock strongSwan on Android from
>> >> the Google Play store to interop with iked.  strongSwan's logs show
>> >> the following once iked is patched:
>> >>
>> >>   [IKE] initiating IKE_SA android[7] to 192.0.2.1
>> >>   [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) 
>> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ]
>> >>   [ENC] parsed IKE_SA_INIT response 0 [ N(INVAL_KE) ]
>> >>   [IKE] peer didn't accept DH group ECP_256, it requested MODP_2048
>> >>   [IKE] initiating IKE_SA android[7] to 192.0.2.1
>> >>   [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) 
>> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ]
>> >>   [ENC] parsed IKE_SA_INIT response 0 [ SA KE No N(NATD_S_IP) 
>> >> N(NATD_D_IP) CERTREQ N(HASH_ALG) ]
>> >>
>> >> I'm happy to iterate on this patch to get it into proper shape for
>> >> inclusion.
>> >
>> > I discovered a bug in the previous patch that broke renegotiation of
>> > CHILD SAs.  I was ignoring "other than NONE" in the following sentence
>> > from RFC 5996 section 3.4:
>> >
>> >   If the selected proposal uses a different Diffie-Hellman group
>> >   (other than NONE), the message MUST be rejected with a Notify
>> >   payload of type INVALID_KE_PAYLOAD.
>> >
>> > The new patch below repairs the flaw.
>>
>> After re-reading relevant parts of the RFC I'm not convinced that my fix
>> (rejecting with INVALID_KE_PAYLOAD unless msg->msg_dhgroup is
>> IKEV2_XFORMDH_NONE) is correct.  It happens to resolve my local issue
>> but I think it may accidentally work due to a side effect of the code
>> path for rekeying a child SA.
>>
>> I will look at it more closely this week.

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

> Hi, I'm interested in this, but wasn't able to get strongswan to connect
> with either of your patches (and had iked exiting on one attempt, though
> I haven't been able to repeat that).

After making changes I usually rebuild all of /usr/src/sbin/iked/ (make
clean && make).  I started doing this after experiencing similar exits
and the problem went away.  That could just be a coincidence, though!

> If you have any updates please do send them here, it can be a bit slow
> getting feedback on iked diffs at times but it definitely is worth sending
> them out.

I'll summarize what I know about strongSwan (on Android) and iked
interop.  strongSwan chooses ECP_256 for its DH group guess when it
starts the IKE_SA_INIT exchange.  The negotiation gets pretty far if I
specify `group ecp256' in the ikesa directive, but there there is some
incompatibility between iked and strongSwan that causes the following:

...
ikev2_recv: IKE_AUTH request from initiator 5.6.7.8:49317 to 1.2.3.4:4500 
policy 'default' id 1, 2040 bytes
ikev2_recv: ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8
ikev2_recv: updated SA to peer 5.6.7.8:49317 local 1.2.3.4:4500
ikev2_pld_parse: header ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8 
nextpayload SK version 0x20 exchange IKE_AUTH flags 0x08 msgid 1 length 2040 
response 0
ikev2_pld_payloads: payload SK nextpayload IDi critical 0x00 length 2012
ikev2_msg_decrypt: IV length 16
ikev2_msg_decrypt: encrypted payload length 1968
ikev2_msg_decrypt: integrity checksum length 24
ikev2_msg_decrypt: integrity check failed
ikev2_resp_recv: failed to parse message
...

I suspect this is the failure many people hit if they try to configure
iked to match strongSwan on Android's built-in settings.  I don't know
what the issue is with ecp256, but I needed to use modp2048 anyway so I
decided to write this patch.

I also configured iked to match strongSwan's built-in configuration for
Child SA rekeying.  Many other cipher combinations result in
NO_PROPOSAL_CHOSEN when iked initiates the Child SA rekey and strongSwan
(usually) decides to rebuild the whole SA, which is awkward and slow
(and not always successful).  Here is the configuration I've 

[patch] dispatch.c

[Edgar Pettijohn]

Move initialization out of if/else block, unruly space, and memset 
struct beforehand.



Index: dispatch.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
retrieving revision 1.43
diff -u -p -u -r1.43 dispatch.c
--- dispatch.c12 Apr 2017 19:17:30 -1.43
+++ dispatch.c26 Jun 2017 00:49:04 -
@@ -549,18 +549,16 @@ add_timeout(time_t when, void (*where)(v
 if (free_timeouts) {
 q = free_timeouts;
 free_timeouts = q->next;
-q->func = where;
-q->what = what;
 } else {
-q = malloc(sizeof (struct dhcpd_timeout));
+q = malloc(sizeof(struct dhcpd_timeout));
 if (!q)
 fatalx("Can't allocate timeout structure!");
-q->func = where;
-q->what = what;
 }
 }
-
+memset(q, 0, sizeof *q);
+q->func = where;
 q->when = when;
+q->what = what;

 /* Now sort this timeout into the timeout list. */

Re: [PATCH] cp(1): add -v option for verbosity

[Theo de Raadt]

> Job Snijders wrote:
> > On Sun, Jun 25, 2017 at 02:06:20PM +0200, Job Snijders wrote:
> > > This patch adds a -v option to cp(1) for more verbose output.
> > 
> > NetBSD/FreeBSD/DragonFly/OSX's cp(1) with "-v" print file names without
> > the single quotes, which might indeed be more appealing to the eye:
> 
> yes, very much. if we're going to add an option on the basis that everybody
> else has it, it should definitely do the same thing.
> 
> -v seems reasonable to me. it's a common option for lots of other utilities,
> like tar, for which similar objections about lack of benefit can be made. i
> tend to monitor programs' progress using ktrace, but that doesn't mean we
> can't provide an easier way.

and I use ^T alot.  But I only want SIGINFO handlers in certain
programs (I'm unable to be exact about which ones "seem a right fit",
but can declare we don't want them everywhere).  For this case
following the -v herd seems better, and even better than nothing.

Re: [PATCH 2/3] openbgpd: Add support for 'unknown' well-known communities

[Sebastian Benoit]

ok

as wor the WELLKNOWN, what do other implementations do?
If others print it or accept it as input, lets keep it.
If not, remove it from both your diff and bgpctls output.

/Benno

Job Snijders(j...@instituut.net) on 2017.06.25 14:59:21 +0200:
> Small update.
> 
> I renamed the 'msb' argument ('most significant bits') to 'part' to
> improve readability. In Community 15562:4, '15562' is part 0 and the '4'
> is part 1. Same type of logic might be useful down the road for Large
> Communities which would have 3 parts.
> 
> - Job
> 
> diff --git usr.sbin/bgpctl/parser.c usr.sbin/bgpctl/parser.c
> index 85300d1cd32..c9d63f9ade3 100644
> --- usr.sbin/bgpctl/parser.c
> +++ usr.sbin/bgpctl/parser.c
> @@ -413,7 +413,7 @@ intparse_addr(const char *, 
> struct bgpd_addr *);
>  int   parse_asnum(const char *, size_t, u_int32_t *);
>  int   parse_number(const char *, struct parse_result *,
>enum token_type);
> -int   getcommunity(const char *);
> +int   getcommunity(const char *, int);
>  int   parse_community(const char *, struct parse_result *);
>  u_int getlargecommunity(const char *);
>  int   parse_largecommunity(const char *, struct parse_result 
> *);
> @@ -927,7 +927,7 @@ parse_number(const char *word, struct parse_result *r, 
> enum token_type type)
>  }
>  
>  int
> -getcommunity(const char *s)
> +getcommunity(const char *s, int part)
>  {
>   const char  *errstr;
>   u_int16_tuval;
> @@ -935,6 +935,9 @@ getcommunity(const char *s)
>   if (strcmp(s, "*") == 0)
>   return (COMMUNITY_ANY);
>  
> + if (part == 0 && strcmp(s, "WELLKNOWN") == 0)
> + return (COMMUNITY_WELLKNOWN);
> +
>   uval = strtonum(s, 0, USHRT_MAX, );
>   if (errstr)
>   errx(1, "Community is %s: %s", errstr, s);
> @@ -978,8 +981,8 @@ parse_community(const char *word, struct parse_result *r)
>   }
>   *p++ = 0;
>  
> - as = getcommunity(word);
> - type = getcommunity(p);
> + as = getcommunity(word, 0);
> + type = getcommunity(p, 1);
>  
>  done:
>   if (as == 0) {
> @@ -994,10 +997,6 @@ done:
>   case COMMUNITY_BLACKHOLE:
>   /* valid */
>   break;
> - default:
> - /* unknown */
> - fprintf(stderr, "Unknown well-known community\n");
> - return (0);
>   }
>  
>   if ((fs = calloc(1, sizeof(struct filter_set))) == NULL)
> diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
> index f0c96051e17..ec4ed956b60 100644
> --- usr.sbin/bgpd/parse.y
> +++ usr.sbin/bgpd/parse.y
> @@ -146,7 +146,7 @@ void   copy_filterset(struct filter_set_head 
> *,
>  void  merge_filter_lists(struct filter_head *, struct filter_head *);
>  struct filter_rule   *get_rule(enum action_types);
>  
> -int   getcommunity(char *);
> +int   getcommunity(char *, int);
>  int   parsecommunity(struct filter_community *, char *);
>  int64_t   getlargecommunity(char *);
>  int   parselargecommunity(struct filter_largecommunity *, char *);
> @@ -2963,11 +2963,13 @@ symget(const char *nam)
>  }
>  
>  int
> -getcommunity(char *s)
> +getcommunity(char *s, int part)
>  {
>   int  val;
>   const char  *errstr;
>  
> + if (part == 0 && strcmp(s, "WELLKNOWN") == 0)
> + return (COMMUNITY_WELLKNOWN);
>   if (strcmp(s, "*") == 0)
>   return (COMMUNITY_ANY);
>   if (strcmp(s, "neighbor-as") == 0)
> @@ -3017,15 +3019,11 @@ parsecommunity(struct filter_community *c, char *s)
>   }
>   *p++ = 0;
>  
> - if ((i = getcommunity(s)) == COMMUNITY_ERROR)
> + if ((i = getcommunity(s, 0)) == COMMUNITY_ERROR)
>   return (-1);
> - if (i == COMMUNITY_WELLKNOWN) {
> - yyerror("Bad community AS number");
> - return (-1);
> - }
>   as = i;
>  
> - if ((i = getcommunity(p)) == COMMUNITY_ERROR)
> + if ((i = getcommunity(p, 1)) == COMMUNITY_ERROR)
>   return (-1);
>   c->as = as;
>   c->type = i;
> 

Re: [PATCH] cp(1): add -v option for verbosity

[Ted Unangst]

Job Snijders wrote:
> On Sun, Jun 25, 2017 at 02:06:20PM +0200, Job Snijders wrote:
> > This patch adds a -v option to cp(1) for more verbose output.
> 
> NetBSD/FreeBSD/DragonFly/OSX's cp(1) with "-v" print file names without
> the single quotes, which might indeed be more appealing to the eye:

yes, very much. if we're going to add an option on the basis that everybody
else has it, it should definitely do the same thing.

-v seems reasonable to me. it's a common option for lots of other utilities,
like tar, for which similar objections about lack of benefit can be made. i
tend to monitor programs' progress using ktrace, but that doesn't mean we
can't provide an easier way.

i'd wait a day to see what other improvements can be made, then resubmit a
complete patch.

Re: [PATCH 2/3] openbgpd: Add support for 'unknown' well-known communities

[Job Snijders]

On Sun, Jun 25, 2017 at 11:41:05PM +0200, Sebastian Benoit wrote:
> ok
> 
> as wor the WELLKNOWN, what do other implementations do?

I'm not aware of other implementations that do a blanket replacement of
"65535:" with something like "WELLKNOWN:" in their CLI output.

Most implementations (after native support for a given well-known
community is added), do replace strings like "65535:666" with a human
readable version like "BLACKHOLE". Subsequently, they often accept both
the numeric and text version of the community as input. In this regard
openbsd is already aligned with the industry pattern.

> If others print it or accept it as input, lets keep it.
> If not, remove it from both your diff and bgpctls output.

ok, below:


diff --git usr.sbin/bgpctl/bgpctl.c usr.sbin/bgpctl/bgpctl.c
index 4d9701da35b..4242b3484ca 100644
--- usr.sbin/bgpctl/bgpctl.c
+++ usr.sbin/bgpctl/bgpctl.c
@@ -1548,7 +1548,7 @@ show_community(u_char *data, u_int16_t len)
printf("BLACKHOLE");
break;
default:
-   printf("WELLKNOWN:%hu", v);
+   printf("%hu:%hu", a, v);
break;
}
else
diff --git usr.sbin/bgpctl/parser.c usr.sbin/bgpctl/parser.c
index 85300d1cd32..0846436a16b 100644
--- usr.sbin/bgpctl/parser.c
+++ usr.sbin/bgpctl/parser.c
@@ -994,10 +994,6 @@ done:
case COMMUNITY_BLACKHOLE:
/* valid */
break;
-   default:
-   /* unknown */
-   fprintf(stderr, "Unknown well-known community\n");
-   return (0);
}
 
if ((fs = calloc(1, sizeof(struct filter_set))) == NULL)
diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
index f0c96051e17..7eec31f2bda 100644
--- usr.sbin/bgpd/parse.y
+++ usr.sbin/bgpd/parse.y
@@ -3019,10 +3019,6 @@ parsecommunity(struct filter_community *c, char *s)
 
if ((i = getcommunity(s)) == COMMUNITY_ERROR)
return (-1);
-   if (i == COMMUNITY_WELLKNOWN) {
-   yyerror("Bad community AS number");
-   return (-1);
-   }
as = i;
 
if ((i = getcommunity(p)) == COMMUNITY_ERROR)

Re: [PATCH] rm(1): add -v option for verbosity

[patrick keshishian]

On Sun, Jun 25, 2017 at 11:16:00PM +0200, Ingo Schwarze wrote:
> Hi,
> 
> Paul de Weerd wrote on Sun, Jun 25, 2017 at 10:46:15PM +0200:
> > On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote:
> 
> > | If you are really unsure, study the output of
> > |   $ find *
> > | first, before typing
> > |   $ rm -rf *
> > | No non-standard option is needed at all for this.
> 
> > Very bad example.  You study the output of find * and another process
> > writes a new file in the mean time.  With rm -v, you would've known.
> 
> I don't buy the talk about race conditions at all.
> 
> If other processes may be writing important files there, then for
> god's sake don't rm -rf it.  Besides, the rm -v output arrives too
> late, when the data is already gone.
> 
> If the directory is full of transient files and removing them is OK,
> than it obviously doesn't matter what exactly got removed.
> 
> Seriously, files not important enough to be kept, but so important
> that you need an audit trail concerning their removal?  That sounds
> like a very unusual use case at best.
> 
> 
> Landry's argument about the progress-meter has some merit, but it
> is clearly more important for copying than for removing, and it is
> clearly more important for copying over the network than for plain
> cp(1), so i'm not sure it is sufficient justification.

scp -v?

$ mkdir ../abc
$ scp -v ./*pdf ../abc/
Executing: cp -- ./iso-prog_lang_C-WG14-N869.pdf ../abc/
Executing: cp -- ./iso-prog_lang_C.WG14-N897.pdf ../abc/
Executing: cp -- ./n1124.pdf ../abc/
Executing: cp -- ./n1256.pdf ../abc/
Executing: cp -- ./n1570.pdf ../abc/
...
$ scp -v n1570.pdf ../abc/single_file.pdf
Executing: cp -- n1570.pdf ../abc/single_file.pdf

-pk

> Anyway, the whole thread looks awfully bikeshedish, so i guess
> i should set a better example and shut up...
> 
> Yours,
>   Ingo
> 
> 
> P.S.
> On the humorous side:
> 
> Landry mentioned about rsync:
> 
> > Technically it could be possible to replicate mv with rsync
> > --remove-source-files ... :)
> 
> Talk about the kitchen sink...
> 
> And i heard rumours that some rsync(1) command line options
> are so long that they don't fit into the command line length
> limit of some shells.  ;-)
> 

ospfd: add IMSG_IFADDRADD to deal with "sh /etc/netstart if"

[Remi Locherer]

Hi,

ospfd does not react nicely when running "sh /etc/netstart if".

This is because adding the same address again do an interface results
in RTM_DELADDR and RTM_NEWADDR. ospfd handles the former but the later.
If this happens ospfd says "interface vether0:192.168.250.1 gone".
Adjacencies on that interface are down and ospfd can not recover.

The below patch adds IMSG_IFADDRADD to deal with that. With it ospfd
logs the following after "ifconfig vether0 192.168.250.1/24" (same address
as active before):

orig_rtr_lsa: area 0.0.0.0
orig_rtr_lsa: stub net, interface iwm0
orig_rtr_lsa: stub net, interface vether0
orig_rtr_lsa: transit net, interface pair0
if_fsm: event DOWN resulted in action RESET and changing state for interface 
vether0 from DR to DOWN
interface vether0:192.168.250.1 gone
orig_rtr_lsa: area 0.0.0.0
orig_rtr_lsa: stub net, interface iwm0
orig_rtr_lsa: stub net, interface vether0
orig_rtr_lsa: transit net, interface pair0
if_fsm: event UP resulted in action START and changing state for interface 
vether0 from DOWN to WAIT
interface vether0:192.168.250.1 returned


In addition the patch also deals with a changed netmask if the address
stays the same.

A complete new address still needs a change in ospfd.conf and a reload.

Remi


Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
retrieving revision 1.107
diff -u -p -r1.107 kroute.c
--- kroute.c27 Dec 2016 09:15:16 -  1.107
+++ kroute.c25 Jun 2017 20:58:33 -
@@ -1046,6 +1046,7 @@ if_newaddr(u_short ifindex, struct socka
 {
struct kif_node *kif;
struct kif_addr *ka;
+   struct ifaddrnew ifn;
 
if (ifa == NULL || ifa->sin_family != AF_INET)
return;
@@ -1066,6 +1067,12 @@ if_newaddr(u_short ifindex, struct socka
ka->dstbrd.s_addr = INADDR_NONE;
 
TAILQ_INSERT_TAIL(>addrs, ka, entry);
+
+   ifn.addr = ka->addr;
+   ifn.mask = ka->mask;
+   ifn.dst = ka->dstbrd;
+   ifn.ifindex = ifindex;
+   main_imsg_compose_ospfe(IMSG_IFADDRADD, 0, , sizeof(ifn));
 }
 
 void
Index: ospfd.h
===
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
retrieving revision 1.97
diff -u -p -r1.97 ospfd.h
--- ospfd.h 24 Jan 2017 04:24:25 -  1.97
+++ ospfd.h 25 Jun 2017 20:58:33 -
@@ -132,6 +132,7 @@ enum imsg_type {
IMSG_RECONF_REDIST,
IMSG_RECONF_END,
IMSG_DEMOTE,
+   IMSG_IFADDRADD,
IMSG_IFADDRDEL
 };
 
@@ -358,6 +359,13 @@ struct iface {
u_int8_t linkstate;
u_int8_t priority;
u_int8_t passive;
+};
+
+struct ifaddrnew {
+   struct in_addr  addr;
+   struct in_addr  mask;
+   struct in_addr  dst;
+   unsigned intifindex;
 };
 
 struct ifaddrdel {
Index: ospfe.c
===
RCS file: /cvs/src/usr.sbin/ospfd/ospfe.c,v
retrieving revision 1.99
diff -u -p -r1.99 ospfe.c
--- ospfe.c 24 Jan 2017 04:24:25 -  1.99
+++ ospfe.c 25 Jun 2017 20:58:33 -
@@ -278,6 +278,7 @@ ospfe_dispatch_main(int fd, short event,
 {
static struct area  *narea;
static struct iface *niface;
+   struct ifaddrnew*ifn;
struct ifaddrdel*ifc;
struct imsg  imsg;
struct imsgev   *iev = bula;
@@ -345,6 +346,28 @@ ospfe_dispatch_main(int fd, short event,
" down",
iface->name);
}
+   }
+   }
+   }
+   break;
+   case IMSG_IFADDRADD:
+   if (imsg.hdr.len != IMSG_HEADER_SIZE +
+   sizeof(struct ifaddrnew))
+   fatalx("IFADDRADD imsg with wrong len");
+   ifn = imsg.data;
+
+   LIST_FOREACH(area, >area_list, entry) {
+   LIST_FOREACH(iface, >iface_list, entry) {
+   if (ifn->ifindex == iface->ifindex &&
+   ifn->addr.s_addr ==
+   iface->addr.s_addr) {
+   iface->mask = ifn->mask;
+   iface->dst = ifn->dst;
+   if_fsm(iface, IF_EVT_UP);
+   log_warnx("interface %s:%s "
+   "returned", iface->name,
+   inet_ntoa(iface->addr));
+ 

Re: [PATCH 3/3] openbgpd: Add well-known community GRACEFUL_SHUTDOWN

[Sebastian Benoit]

ok

Job Snijders(j...@instituut.net) on 2017.06.23 16:02:13 +0200:
> Dear team,
> 
> This patch adds support for the "graceful shutdown" well-known
> community as described in draft-ietf-grow-bgp-gshut.
> 
> An example implementation would be to add the following to your
> bgpd.conf:
> 
> match from any community GRACEFUL_SHUTDOWN set { localpref 0 }
> 
> Kind regards,
> 
> Job
> 
> ---
>  etc/examples/bgpd.conf| 4 
>  usr.sbin/bgpctl/bgpctl.c  | 3 +++
>  usr.sbin/bgpctl/parser.c  | 7 ++-
>  usr.sbin/bgpd/bgpd.conf.5 | 2 ++
>  usr.sbin/bgpd/bgpd.h  | 1 +
>  usr.sbin/bgpd/parse.y | 6 +-
>  6 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/etc/examples/bgpd.conf b/etc/examples/bgpd.conf
> index 2ec37b2c752..1caf200ceab 100644
> --- a/etc/examples/bgpd.conf
> +++ b/etc/examples/bgpd.conf
> @@ -87,6 +87,10 @@ allow from any inet6 prefixlen 16 - 48
>  #allow from any prefix 0.0.0.0/0
>  #allow from any prefix ::/0
>  
> +# Honor requests to gracefully shutdown BGP sessions
> +# https://tools.ietf.org/html/draft-ietf-grow-bgp-gshut
> +match from any community GRACEFUL_SHUTDOWN set { localpref 0 }
> +
>  # https://www.arin.net/announcements/2014/20140130.html
>  # This block will be subject to a minimum size allocation of /28 and a
>  # maximum size allocation of /24. ARIN should use sparse allocation when
> diff --git a/usr.sbin/bgpctl/bgpctl.c b/usr.sbin/bgpctl/bgpctl.c
> index 4d9701da35b..8baa8be0ff2 100644
> --- a/usr.sbin/bgpctl/bgpctl.c
> +++ b/usr.sbin/bgpctl/bgpctl.c
> @@ -1532,6 +1532,9 @@ show_community(u_char *data, u_int16_t len)
>   v = ntohs(v);
>   if (a == COMMUNITY_WELLKNOWN)
>   switch (v) {
> + case COMMUNITY_GRACEFUL_SHUTDOWN:
> + printf("GRACEFUL_SHUTDOWN");
> + break;
>   case COMMUNITY_NO_EXPORT:
>   printf("NO_EXPORT");
>   break;
> diff --git a/usr.sbin/bgpctl/parser.c b/usr.sbin/bgpctl/parser.c
> index 0d1e5d9fb3a..4ea16533b71 100644
> --- a/usr.sbin/bgpctl/parser.c
> +++ b/usr.sbin/bgpctl/parser.c
> @@ -953,7 +953,11 @@ parse_community(const char *word, struct parse_result *r)
>   int  as, type;
>  
>   /* Well-known communities */
> - if (strcasecmp(word, "NO_EXPORT") == 0) {
> + if (strcasecmp(word, "GRACEFUL_SHUTDOWN") == 0) {
> + as = COMMUNITY_WELLKNOWN;
> + type = COMMUNITY_GRACEFUL_SHUTDOWN;
> + goto done;
> + } else if (strcasecmp(word, "NO_EXPORT") == 0) {
>   as = COMMUNITY_WELLKNOWN;
>   type = COMMUNITY_NO_EXPORT;
>   goto done;
> @@ -991,6 +995,7 @@ done:
>   }
>   if (as == COMMUNITY_WELLKNOWN)
>   switch (type) {
> + case COMMUNITY_GRACEFUL_SHUTDOWN:
>   case COMMUNITY_NO_EXPORT:
>   case COMMUNITY_NO_ADVERTISE:
>   case COMMUNITY_NO_EXPSUBCONFED:
> diff --git a/usr.sbin/bgpd/bgpd.conf.5 b/usr.sbin/bgpd/bgpd.conf.5
> index 6cecd7a5a80..3afc54ef385 100644
> --- a/usr.sbin/bgpd/bgpd.conf.5
> +++ b/usr.sbin/bgpd/bgpd.conf.5
> @@ -1173,6 +1173,7 @@ to do wildcard matching.
>  Alternatively, well-known communities may be given by name instead and
>  include
>  .Ic BLACKHOLE ,
> +.Ic GRACEFUL_SHUTDOWN ,
>  .Ic NO_EXPORT ,
>  .Ic NO_ADVERTISE ,
>  .Ic NO_EXPORT_SUBCONFED ,
> @@ -1444,6 +1445,7 @@ is an AS number and
>  is a locally-significant number between zero and
>  .Li 65535 .
>  Alternately, well-known communities may be specified by name:
> +.Ic GRACEFUL_SHUTDOWN ,
>  .Ic NO_EXPORT ,
>  .Ic NO_ADVERTISE ,
>  .Ic NO_EXPORT_SUBCONFED ,
> diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h
> index db52f858241..ef4e30ffd94 100644
> --- a/usr.sbin/bgpd/bgpd.h
> +++ b/usr.sbin/bgpd/bgpd.h
> @@ -750,6 +750,7 @@ struct filter_peers {
>  #define  COMMUNITY_LOCAL_AS  -4
>  #define  COMMUNITY_UNSET -5
>  #define  COMMUNITY_WELLKNOWN 0x
> +#define  COMMUNITY_GRACEFUL_SHUTDOWN 0x  /* 
> draft-ietf-grow-bgp-gshut */
>  #define  COMMUNITY_BLACKHOLE 0x029A  /* RFC 7999 */
>  #define  COMMUNITY_NO_EXPORT 0xff01
>  #define  COMMUNITY_NO_ADVERTISE  0xff02
> diff --git a/usr.sbin/bgpd/parse.y b/usr.sbin/bgpd/parse.y
> index 73bdb3a0cb9..0b09f83bc0a 100644
> --- a/usr.sbin/bgpd/parse.y
> +++ b/usr.sbin/bgpd/parse.y
> @@ -2991,7 +2991,11 @@ parsecommunity(struct filter_community *c, char *s)
>   int i, as;
>  
>   /* Well-known communities */
> - if (strcasecmp(s, "NO_EXPORT") == 0) {
> + if (strcasecmp(s, "GRACEFUL_SHUTDOWN") == 0) {
> + c->as = COMMUNITY_WELLKNOWN;
> + c->type = COMMUNITY_GRACEFUL_SHUTDOWN;
> + return (0);
> + } else if (strcasecmp(s, "NO_EXPORT") == 0) {
>   c->as = 

Re: [PATCH] cp(1): add -v option for verbosity

[Job Snijders]

On Sun, Jun 25, 2017 at 02:06:20PM +0200, Job Snijders wrote:
> This patch adds a -v option to cp(1) for more verbose output.

NetBSD/FreeBSD/DragonFly/OSX's cp(1) with "-v" print file names without
the single quotes, which might indeed be more appealing to the eye:

$ touch a b; mkdir c
$ cp -v a b c
a -> c/a
b -> c/b

> +.It Fl v
> +Explain what is being done.

A more compendious option description for bin/cp/cp.1 would be: 

.It Fl v
Display the source and destination of each copied file.

The above 2 suggestions make sense to the proposed mv(1) patch too,
modulo s/copied/moved/.

Kind regards,

Job

Re: [PATCH] rm(1): add -v option for verbosity

[Ingo Schwarze]

Hi,

Paul de Weerd wrote on Sun, Jun 25, 2017 at 10:46:15PM +0200:
> On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote:

> | If you are really unsure, study the output of
> |   $ find *
> | first, before typing
> |   $ rm -rf *
> | No non-standard option is needed at all for this.

> Very bad example.  You study the output of find * and another process
> writes a new file in the mean time.  With rm -v, you would've known.

I don't buy the talk about race conditions at all.

If other processes may be writing important files there, then for
god's sake don't rm -rf it.  Besides, the rm -v output arrives too
late, when the data is already gone.

If the directory is full of transient files and removing them is OK,
than it obviously doesn't matter what exactly got removed.

Seriously, files not important enough to be kept, but so important
that you need an audit trail concerning their removal?  That sounds
like a very unusual use case at best.


Landry's argument about the progress-meter has some merit, but it
is clearly more important for copying than for removing, and it is
clearly more important for copying over the network than for plain
cp(1), so i'm not sure it is sufficient justification.

Anyway, the whole thread looks awfully bikeshedish, so i guess
i should set a better example and shut up...

Yours,
  Ingo


P.S.
On the humorous side:

Landry mentioned about rsync:

> Technically it could be possible to replicate mv with rsync
> --remove-source-files ... :)

Talk about the kitchen sink...

And i heard rumours that some rsync(1) command line options
are so long that they don't fit into the command line length
limit of some shells.  ;-)

Re: [PATCH] rm(1): add -v option for verbosity

[Job Snijders]

On Sun, Jun 25, 2017 at 04:09:11PM +0200, Job Snijders wrote:
> --- bin/rm/rm.1
> +++ bin/rm/rm.1
> @@ -95,6 +95,8 @@ that directory is skipped.
>  .It Fl r
>  Equivalent to
>  .Fl R .
> +.It Fl v
> +Explain what is being done.
   

On second thought, "Display what files were removed." would be a more
succinct description of what the "-v" option in 'rm' does.

Re: [PATCH] ffs: always assign random inode generation numbers

[Dmitry Chestnykh]

Here's another version: it starts with a random 31-bit integer and
then for subsequent allocations sets 23 random bits and incremented 8
bits from the previous generation number, so it's guaranteed to not
repeat for at least 256 allocations (suggested by Kai Antweiler).

-Dmitry

Index: src/sys/ufs/ffs/ffs_alloc.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
retrieving revision 1.108
diff -u -p -u -r1.108 ffs_alloc.c
--- src/sys/ufs/ffs/ffs_alloc.c 23 May 2016 20:47:49 -  1.108
+++ src/sys/ufs/ffs/ffs_alloc.c 25 Jun 2017 20:57:41 -
@@ -361,6 +361,7 @@ ffs_inode_alloc(struct inode *pip, mode_
struct inode *ip;
ufsino_t ino, ipref;
int cg, error;
+   int32_t ngen;

*vpp = NULL;
fs = pip->i_fs;
@@ -413,16 +414,18 @@ ffs_inode_alloc(struct inode *pip, mode_

/*
 * Set up a new generation number for this inode.
-* XXX - just increment for now, this is wrong! (millert)
-*   Need a way to preserve randomization.
 */
-   if (DIP(ip, gen) != 0)
-   DIP_ADD(ip, gen, 1);
if (DIP(ip, gen) == 0)
-   DIP_ASSIGN(ip, gen, arc4random() & INT_MAX);
+   ngen = arc4random() & INT_MAX;
+   else {
+   ngen = (arc4random() << 8) & 0x7f;
+   ngen |= (u_int8_t)(DIP(ip, gen)) + 1;
+   }
+
+   if (ngen == 0)
+   ngen = 1;

-   if (DIP(ip, gen) == 0 || DIP(ip, gen) == -1)
-   DIP_ASSIGN(ip, gen, 1); /* Shouldn't happen */
+   DIP_ASSIGN(ip, gen, ngen);

return (0);

Re: make it easier to configure IPv6 on gif(4)

[Sebastian Benoit]

ok

Stefan Sperling(s...@stsp.name) on 2017.06.24 07:09:20 +0200:
> The kernel rejects IPv6 destination addresses on point-to-point
> interfaces if the prefixlen is not 128. Because ifconfig defaults
> to prefixlen 64, configuring IPv6 on e.g. gif(4) requires an
> explicit prefix length, for instance:
> 
>ifconfig gif0 inet6 ADDR1 ADDR2 prefixlen 128
> 
> Without prefixlen we get: ifconfig: SIOCAIFADDR: Invalid argument
> 
> However, this command works for IPv4:
> 
>ifconfig gif0 ADDR1 ADDR2
> 
> The netmask isn't forced to 255.255.255.255 by the kernel, though.
> 
> Assuming the kernel is correct in enforcing IPv6 prefixlen 128,
> for whatever reason, then I suggest we make ifconfig figure out
> the required prefixlen by itself so this command works as it
> does for IPv4:
> 
>ifconfig gif0 inet6 ADDR1 ADDR2
> 
> ok?
> 
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.344
> diff -u -p -r1.344 ifconfig.c
> --- ifconfig.c8 Jun 2017 19:23:39 -   1.344
> +++ ifconfig.c24 Jun 2017 04:54:17 -
> @@ -803,9 +803,13 @@ nextarg:
>   /*
>* Aggregatable address architecture defines all prefixes
>* are 64. So, it is convenient to set prefixlen to 64 if
> -  * it is not specified.
> +  * it is not specified. If we are setting a destination
> +  * address on a point-to-point interface, 128 is required.
>*/
> - setifprefixlen("64", 0);
> + if (setipdst && (flags & IFF_POINTOPOINT))
> + setifprefixlen("128", 0);
> + else
> + setifprefixlen("64", 0);
>   /* in6_getprefix("64", MASK) if MASK is available here... */
>   }
>  
> @@ -1241,6 +1245,7 @@ void
>  setifdstaddr(const char *addr, int param)
>  {
>   setaddr++;
> + setipdst++;
>   afp->af_getaddr(addr, DSTADDR);
>  }
>  
> 

Re: [PATCH] cp(1): add -v option for verbosity

[Paul de Weerd]

On Sun, Jun 25, 2017 at 10:49:04PM +0200, Landry Breuil wrote:
| > | Alternatively one can use rsync(1), but that is not part of the base.
| > 
| > That may work for cp(1), but it's hard to replicate mv(1) behavior
| > with rsync (only metadata changes when on the same fs) or even
| > impossible to replciate rm(1) behavior.
| 
| Technically it could be possible to replicate mv with rsync
| --remove-source-files ... :)

rsync will still copy the file first, while mv can simply move the
file to another directory without the copy operation when it's on the
same filesystem (my comment about 'only metadata changes').

Cheers,

Paul

-- 
>[<++>-]<+++.>+++[<-->-]<.>+++[<+
+++>-]<.>++[<>-]<+.--.[-]
 http://www.weirdnet.nl/ 

Re: [PATCH] cp(1): add -v option for verbosity

[Landry Breuil]

On Sun, Jun 25, 2017 at 09:59:35PM +0200, Paul de Weerd wrote:
> On Sun, Jun 25, 2017 at 06:22:05PM +0200, Job Snijders wrote:
> | Dear Alexander,
> | 
> | On Sun, Jun 25, 2017 at 06:13:40PM +0200, Alexander Hall wrote:
> | > On June 25, 2017 2:06:20 PM GMT+02:00, Job Snijders  
> wrote:
> | > >This patch adds a -v option to cp(1) for more verbose output.
> | > >
> | > > $ touch a b; mkdir c
> | > > $ cp -v a b c
> | > > 'a' -> 'c/a'
> | > > 'b' -> 'c/b'
> | > > $ cp -rv c d
> | > > 'c' -> 'd/'
> | > > 'c/a' -> 'd/a'
> | > > 'c/b' -> 'd/b'
> | > 
> | > Pardon my ignorance, but why?
> | 
> | A fair question.  I myself have two use cases, but others may have their
> | own to add.
> | 
> | When a glob pattern is used, it can be beneficial to immediately observe
> | (during the execution of the command) which files have been copied.
> | 
> | When copying very large trees, the -v option provides some insight as to
> | what progress the cp operation has made so far.
> 
> I like the -v option for the above reason most (and have missed it on
> several occassions): copy, move or remove a whole bunch of files; it
> takes a while.  Is it working?  Or is NFS stalling / IO to the storage
> device otherwise acting up?
> 
> Also, when using these tools in crons, it can be very useful to have
> cron send out reports of the files copied/moved/deleted.  Note that
> directories can be altered outside of the control of said script: it
> is impossible to deterministically figure out what cp/mv/rm did after
> (or before, as the 'study `find *`' hint suggests) they are run.
> 
> | Alternatively one can use rsync(1), but that is not part of the base.
> 
> That may work for cp(1), but it's hard to replicate mv(1) behavior
> with rsync (only metadata changes when on the same fs) or even
> impossible to replciate rm(1) behavior.

Technically it could be possible to replicate mv with rsync
--remove-source-files ... :)

Re: make it easier to configure IPv6 on gif(4)

[Alexander Bluhm]

On Sat, Jun 24, 2017 at 07:09:20AM +0200, Stefan Sperling wrote:
> ok?

OK bluhm@

> 
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.344
> diff -u -p -r1.344 ifconfig.c
> --- ifconfig.c8 Jun 2017 19:23:39 -   1.344
> +++ ifconfig.c24 Jun 2017 04:54:17 -
> @@ -803,9 +803,13 @@ nextarg:
>   /*
>* Aggregatable address architecture defines all prefixes
>* are 64. So, it is convenient to set prefixlen to 64 if
> -  * it is not specified.
> +  * it is not specified. If we are setting a destination
> +  * address on a point-to-point interface, 128 is required.
>*/
> - setifprefixlen("64", 0);
> + if (setipdst && (flags & IFF_POINTOPOINT))
> + setifprefixlen("128", 0);
> + else
> + setifprefixlen("64", 0);
>   /* in6_getprefix("64", MASK) if MASK is available here... */
>   }
>  
> @@ -1241,6 +1245,7 @@ void
>  setifdstaddr(const char *addr, int param)
>  {
>   setaddr++;
> + setipdst++;
>   afp->af_getaddr(addr, DSTADDR);
>  }
>  

Re: [PATCH] rm(1): add -v option for verbosity

[Paul de Weerd]

Hi Ingo,

On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote:
| Hi,
| 
| Job Snijders wrote on Sun, Jun 25, 2017 at 02:06:16PM +0200:
| 
| > This patch adds a '-v' option to rm(1) for more verbose output.
| 
| Do not add new options to standard utilities, unless you can show
| that they are unusually useful in practice *and* practically
| every other system out there has them, with a compatible user
| interface across practically all systems.
| 
| Adding -v to rm(1) probably wouldn't make the very high bar against
| adding non-standard options even if almost everybody else had it
| (which i didn't check, to not waste time) because it's completely
| useless.
| 
| If you are really unsure, study the output of
| 
|   $ find *
| 
| first, before typing
| 
|   $ rm -rf *
| 
| No non-standard option is needed at all for this.

Very bad example.  You study the output of find * and another process
writes a new file in the mean time.  With rm -v, you would've known.

The solution here (currently) is to find * -delete -print.  That
solves it for rm (well, it moves the problem elsewhere (and is more to
type)), but doesn't help the mv or cp cases.  And I'd say that for
completeness' sake, the option should be added to all three of these
(or none, as is currently the case).

| It's also strongly in violation of the UNIX philosophy (each utility,
| do one thing, and do it well).  rm(1) removes files, and does so
| well.  rm(1) does *not* trespass on ls(1) and find(1) territory to
| list files.

Well, find(1) trespasses the rm(1) territory with its -delete option,
the only real solution to doing what you suggested.

Reporting on what you did is still doing your job.  Just a bit more
verbose (which, in certain situations, I would consider a BETTER job).

Comparing rm(1) to ls(1) is weird: ls doesn't delete files.  In this
case, rm(1) isn't "listing" the files as ls(1) does, it *reports* on
files that have been deleted.  A small, but significant difference.

|  Besides, they way your proposed rm(1) extension lists
| files is not doing it well at all.  The output is awful.

Agreed.  Just listing the filename(s) would suffice.

Paul 'WEiRD' de Weerd

-- 
>[<++>-]<+++.>+++[<-->-]<.>+++[<+
+++>-]<.>++[<>-]<+.--.[-]
 http://www.weirdnet.nl/ 

Re: pf fragment drop stale

[Alexander Bluhm]

On Fri, Jun 23, 2017 at 03:00:07PM +0200, Alexander Bluhm wrote:
> Adjusted timeouts will follow in the next diff.

To avoid that fragments for a single connection that reuse the
fragment id are reassembled into the wrong packet, throw away stale
fragments.  With the default timeout this happens after 18,000 newer
fragemnts have been seen.

ok?

bluhm

Index: net/pf_norm.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_norm.c,v
retrieving revision 1.207
diff -u -p -r1.207 pf_norm.c
--- net/pf_norm.c   24 Jun 2017 20:32:39 -  1.207
+++ net/pf_norm.c   25 Jun 2017 20:21:51 -
@@ -84,6 +84,7 @@ struct pf_frnode {
u_int8_tfn_proto;   /* protocol for fragments in fn_tree */
u_int8_tfn_direction;   /* pf packet direction */
u_int32_t   fn_fragments;   /* number of entries in fn_tree */
+   u_int32_t   fn_gen; /* fr_gen of newest entry in fn_tree */
 
RB_ENTRY(pf_frnode) fn_entry;
struct pf_frag_tree fn_tree;/* matching fragments, lookup by id */
@@ -96,6 +97,7 @@ struct pf_fragment {
TAILQ_ENTRY(pf_fragment) frag_next;
TAILQ_HEAD(pf_fragq, pf_frent) fr_queue;
int32_t fr_timeout;
+   u_int32_t   fr_gen; /* generation number (per pf_frnode) */
u_int16_t   fr_maxlen;  /* maximum length of single fragment */
struct pf_frnode *fr_node;  /* ip src/dst/proto/af for fragments */
 };
@@ -268,11 +270,13 @@ pf_free_fragment(struct pf_fragment *fra
pool_put(_frag_pl, frag);
 }
 
+#define PF_FRSTALE 200
 struct pf_fragment *
 pf_find_fragment(struct pf_frnode *key, u_int32_t id)
 {
struct pf_fragment  *frag, idkey;
struct pf_frnode*frnode;
+   u_int32_tstale;
 
frnode = RB_FIND(pf_frnode_tree, _frnode_tree, key);
if (frnode == NULL)
@@ -282,6 +286,24 @@ pf_find_fragment(struct pf_frnode *key, 
frag = RB_FIND(pf_frag_tree, >fn_tree, );
if (frag == NULL)
return (NULL);
+   /*
+* Limit the number of fragments we accept for each (proto,src,dst,af)
+* combination (aka pf_frnode), so we can deal better with a high rate
+* of fragments.
+* Store the current generation for each pf_frnode in fn_gen and on
+* lookup discard 'stale' fragments (pf_fragment, based on the fr_gen
+* member).  Instead of adding another button interpret the pf fragment
+* timeout in multiples of 200 fragments.  This way the default of 60s
+* means: pf_fragment objects older than 60*200 = 18,000 generations
+* are considered stale.
+*/
+   stale = pf_default_rule.timeout[PFTM_FRAG] * PF_FRAG_STALE;
+   if ((frnode->fn_gen - frag->fr_gen) >= stale) {
+   DPFPRINTF(LOG_NOTICE, "stale fragment %d(%p), gen %u, num %u",
+   frag->fr_id, frag, frag->fr_gen, frnode->fn_fragments);
+   pf_free_fragment(frag);
+   return (NULL);
+   }
TAILQ_REMOVE(_fragqueue, frag, frag_next);
TAILQ_INSERT_HEAD(_fragqueue, frag, frag_next);
 
@@ -369,9 +391,11 @@ pf_fillup_fragment(struct pf_frnode *key
*frnode = *key;
RB_INIT(>fn_tree);
frnode->fn_fragments = 0;
+   frnode->fn_gen = 0;
}
TAILQ_INIT(>fr_queue);
frag->fr_timeout = time_uptime;
+   frag->fr_gen = frnode->fn_gen++;
frag->fr_maxlen = frent->fe_len;
frag->fr_id = id;
frag->fr_node = frnode;
Index: net/pfvar.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar.h,v
retrieving revision 1.457
diff -u -p -r1.457 pfvar.h
--- net/pfvar.h 30 May 2017 19:40:54 -  1.457
+++ net/pfvar.h 25 Jun 2017 19:43:40 -
@@ -109,6 +109,8 @@ enum{ PFTM_TCP_FIRST_PACKET, PFTM_TCP_O
 #define PFTM_SRC_NODE_VAL  0   /* Source tracking */
 #define PFTM_TS_DIFF_VAL   30  /* Allowed TS diff */
 
+#define PF_FRAG_STALE  200 /* Limit fragments per second per connection */
+
 enum   { PF_NOPFROUTE, PF_ROUTETO, PF_DUPTO, PF_REPLYTO };
 enum   { PF_LIMIT_STATES, PF_LIMIT_SRC_NODES, PF_LIMIT_FRAGS,
  PF_LIMIT_TABLES, PF_LIMIT_TABLE_ENTRIES, PF_LIMIT_MAX };

Re: [PATCH] cp(1): add -v option for verbosity

[Paul de Weerd]

On Sun, Jun 25, 2017 at 06:22:05PM +0200, Job Snijders wrote:
| Dear Alexander,
| 
| On Sun, Jun 25, 2017 at 06:13:40PM +0200, Alexander Hall wrote:
| > On June 25, 2017 2:06:20 PM GMT+02:00, Job Snijders  
wrote:
| > >This patch adds a -v option to cp(1) for more verbose output.
| > >
| > >   $ touch a b; mkdir c
| > >   $ cp -v a b c
| > >   'a' -> 'c/a'
| > >   'b' -> 'c/b'
| > >   $ cp -rv c d
| > >   'c' -> 'd/'
| > >   'c/a' -> 'd/a'
| > >   'c/b' -> 'd/b'
| > 
| > Pardon my ignorance, but why?
| 
| A fair question.  I myself have two use cases, but others may have their
| own to add.
| 
| When a glob pattern is used, it can be beneficial to immediately observe
| (during the execution of the command) which files have been copied.
| 
| When copying very large trees, the -v option provides some insight as to
| what progress the cp operation has made so far.

I like the -v option for the above reason most (and have missed it on
several occassions): copy, move or remove a whole bunch of files; it
takes a while.  Is it working?  Or is NFS stalling / IO to the storage
device otherwise acting up?

Also, when using these tools in crons, it can be very useful to have
cron send out reports of the files copied/moved/deleted.  Note that
directories can be altered outside of the control of said script: it
is impossible to deterministically figure out what cp/mv/rm did after
(or before, as the 'study `find *`' hint suggests) they are run.

| Alternatively one can use rsync(1), but that is not part of the base.

That may work for cp(1), but it's hard to replicate mv(1) behavior
with rsync (only metadata changes when on the same fs) or even
impossible to replciate rm(1) behavior.

Cheers,

Paul 'WEiRD' de Weerd

-- 
>[<++>-]<+++.>+++[<-->-]<.>+++[<+
+++>-]<.>++[<>-]<+.--.[-]
 http://www.weirdnet.nl/ 

Re: [diffs] libcrypto: minor man page fix & question about odd function types

[Ingo Schwarze]

Hi,

Jack Burton wrote on Wed, Jun 21, 2017 at 11:45:38PM +0930:

> X509_VERIFY_PARAM_set_flags(3) states that
> X509_VERIFY_PARAM_set_flags() and X509_VERIFY_PARAM_clear_flags()
> both "return 1 for success or 0 for failure".

Which is true.

> But both those functions always return 1

In the current implementation.

> The trivial diff below amends the man page to reflect reality.

No, the diff is not needed to make the documentation match reality.
The documentation already matches reality.

Instead, the diff constitutes a change of the public interface.

The documentation of a library function does not only state
what the current implementation does.  Above all, it tells
users of the public interface which return values they must
expect, including in future versions of the library.

So what Stephen Henson did here when writing the documentation
was reserving the right to implement functionality in the future
that might possibly cause these functions to fail.

> I'm wondering whether it's perhaps worth changing the type of
> those two functions

Hell no, what a terrible idea.  Changing the prototype of a public
interface in a library requires a major bump of the library version
and potentially requires changing all application code that uses
the interface.

With the proposed change, you reward reckless or lazy programmers
who ignored the return value of these functions in their code
and you punish programmers who carefully coded according to spec -
because after your change, error handling code for failure of
these functions will no longer compile.

So even when you have strong reasons to change functionality in a
library, you generally try to avoid changing the prototypes of
established interfaces.


All that said, it may or may not have been a poor design decision
to make these functions return int rather than void, and i have
no idea whether anything that could cause them to fail might ever
be useful - but that ship has sailed, the interfaces are now public
and you need very strong reasons to change them.

Yours,
  Ingo


> Index: man/X509_VERIFY_PARAM_set_flags.3
> ===
> RCS file: /cvs/src/lib/libcrypto/man/X509_VERIFY_PARAM_set_flags.3,v
> retrieving revision 1.5
> diff -u -p -r1.5 X509_VERIFY_PARAM_set_flags.3
> --- man/X509_VERIFY_PARAM_set_flags.3 6 Jan 2017 21:30:27
> - 1.5 +++ man/X509_VERIFY_PARAM_set_flags.3   21 Jun
> 2017 13:29:13 - @@ -183,8 +183,11 @@ sets the maximum verification
> depth to That is the maximum number of untrusted CA certificates that
> can appear in a chain.
>  .Sh RETURN VALUES
> -.Fn X509_VERIFY_PARAM_set_flags ,
> -.Fn X509_VERIFY_PARAM_clear_flags ,
> +.Fn X509_VERIFY_PARAM_set_flags
> +and
> +.Fn X509_VERIFY_PARAM_clear_flags
> +always return 1.
> +.Pp
>  .Fn X509_VERIFY_PARAM_set_purpose ,
>  .Fn X509_VERIFY_PARAM_set_trust ,
>  .Fn X509_VERIFY_PARAM_add0_policy ,

Re: [PATCH] ffs: always assign random inode generation numbers

[Dmitry Chestnykh]

On Sun, Jun 25, 2017 at 8:34 PM, Ted Unangst  wrote:
>> This patch always assigns a random non-zero positive integer in [1,
>> INT_MAX] range, not equal to the previous generation number.
>
> will this cause problems if a number repeats? we've seen problems with that
> before, where you get a sequence like 4, 7, 4 and then bad things happen.

Ah, didn't think about this.

>From the overengineering department: to create completely distinct
pseudorandom generation numbers keep a key ("seed") in inode and
encrypt counter with 32-bit-block cipher. (j/k)

>From the common sense department: keep the current version, which does
simple increment, just make sure it won't overflow.

--
Dmitry Chestnykh
https://www.codingrobots.com

Re: [PATCH] ffs: always assign random inode generation numbers

[Dmitry Chestnykh]

On Sun, Jun 25, 2017 at 8:20 PM, Theo de Raadt  wrote:
> I'd really prefer to see a construct which doesn't do that.  But
> I don't know if that would result in something really complex.

Sure, will send a different version if the problem that tedu@
mentioned can be solved. Otherwise, probably it's better to keep the
current incrementing.

>> signed integer, it may overflow, causing undefined behavior
>
> The development of OpenBSD/univac is running way behind schedule.

:-) Speaking of signed integers, does it really need to be signed?

--
Dmitry Chestnykh
https://www.codingrobots.com

[patch] dhcpd.h

[Edgar Pettijohn]

Remove unused structs and line up some comments.


Index: dhcpd.h
===
RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.h,v
retrieving revision 1.64
diff -u -p -u -r1.64 dhcpd.h
--- dhcpd.h24 Apr 2017 14:58:36 -1.64
+++ dhcpd.h25 Jun 2017 18:36:47 -
@@ -54,11 +54,6 @@ struct iaddr {
 unsigned char iabuf[16];
 };

-struct iaddrlist {
-struct iaddrlist *next;
-struct iaddr addr;
-};
-
 #define DEFAULT_HASH_SIZE97

 struct hash_bucket {
@@ -78,25 +73,6 @@ struct option_data {
 u_int8_t *data;
 };

-struct string_list {
-struct string_list *next;
-char *string;
-};
-
-/* A name server, from /etc/resolv.conf. */
-struct name_server {
-struct name_server *next;
-struct sockaddr_in addr;
-time_t rcdate;
-};
-
-/* A domain search list element. */
-struct domain_search_list {
-struct domain_search_list *next;
-char *domain;
-time_t rcdate;
-};
-
 /* A dhcp packet and the pointers to its option values. */
 struct packet {
 struct dhcp_packet *raw;
@@ -107,12 +83,12 @@ struct packet {
 struct iaddr client_addr;
 struct interface_info *interface;/* Interface on which packet
was received. */
-struct hardware *haddr;/* Physical link address
-   of local sender (maybe gateway). */
+struct hardware *haddr;/* Physical link address
+  of local sender (maybe gateway). */
 struct shared_network *shared_network;
 struct option_data options[256];
-int got_requested_address;/* True if client sent the
-   dhcp-requested-address option. */
+int got_requested_address;/* True if client sent the
+   dhcp-requested-address option. */
 };

 struct hardware {
@@ -174,13 +150,12 @@ struct lease_state {
 int max_message_size;
 u_int8_t *prl;
 int prl_len;
-int got_requested_address;/* True if client sent the
-   dhcp-requested-address option. */
-int got_server_identifier;/* True if client sent the
-   dhcp-server-identifier option. */
+int got_requested_address;/* True if client sent the
+   dhcp-requested-address option. */
+int got_server_identifier;/* True if client sent the
+   dhcp-server-identifier option. */
 struct shared_network *shared_network;/* Shared network of 
interface

on which request arrived. */
-
 u_int32_t xid;
 u_int16_t secs;
 u_int16_t bootp_flags;
@@ -278,7 +253,7 @@ struct pf_cmd {
 struct interface_info {
 struct interface_info *next;/* Next interface in list... */
 struct shared_network *shared_network;
-/* Networks connected to this interface. */
+/* Networks connected to this interface. */
 struct hardware hw_address;/* Its physical address. */
 struct in_addr primary_address;/* Primary interface address. */
 char name[IFNAMSIZ];/* Its name... */
@@ -299,12 +274,6 @@ struct interface_info {
 int is_udpsock;
 ssize_t (*send_packet)(struct interface_info *, struct dhcp_packet *,
 size_t, struct in_addr, struct sockaddr_in *, struct hardware *);
-};
-
-struct hardware_link {
-struct hardware_link *next;
-char name[IFNAMSIZ];
-struct hardware address;
 };

 struct dhcpd_timeout {

Re: [PATCH] ffs: always assign random inode generation numbers

[Ted Unangst]

Dmitry Chestnykh wrote:
> Hello,
> 
> In ffs_inode_alloc(), inode generation numbers are incremented after
> being assigned an initial random value. Since the di_gen field is a
> signed integer, it may overflow, causing undefined behavior (this is
> unlikely to ever happen, though).
> 
> This patch always assigns a random non-zero positive integer in [1,
> INT_MAX] range, not equal to the previous generation number.

will this cause problems if a number repeats? we've seen problems with that
before, where you get a sequence like 4, 7, 4 and then bad things happen.


> 
> --
> Dmitry Chestnykh
> https://www.codingrobots.com
> 
> 
> Index: src/sys/ufs/ffs/ffs_alloc.c
> ===
> RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
> retrieving revision 1.108
> diff -u -p -u -r1.108 ffs_alloc.c
> --- src/sys/ufs/ffs/ffs_alloc.c 23 May 2016 20:47:49 -  1.108
> +++ src/sys/ufs/ffs/ffs_alloc.c 24 Jun 2017 19:25:04 -
> @@ -361,6 +361,7 @@ ffs_inode_alloc(struct inode *pip, mode_
> struct inode *ip;
> ufsino_t ino, ipref;
> int cg, error;
> +   int32_t ngen;
> 
> *vpp = NULL;
> fs = pip->i_fs;
> @@ -413,16 +414,12 @@ ffs_inode_alloc(struct inode *pip, mode_
> 
> /*
>  * Set up a new generation number for this inode.
> -* XXX - just increment for now, this is wrong! (millert)
> -*   Need a way to preserve randomization.
>  */
> -   if (DIP(ip, gen) != 0)
> -   DIP_ADD(ip, gen, 1);
> -   if (DIP(ip, gen) == 0)
> -   DIP_ASSIGN(ip, gen, arc4random() & INT_MAX);
> -
> -   if (DIP(ip, gen) == 0 || DIP(ip, gen) == -1)
> -   DIP_ASSIGN(ip, gen, 1); /* Shouldn't happen */
> +   do {
> +   ngen = arc4random_uniform(INT_MAX) + 1;
> +   } while (DIP(ip, gen) == ngen);
> +
> +   DIP_ASSIGN(ip, gen, ngen);
> 
> return (0);
> 

Re: [PATCH] ffs: always assign random inode generation numbers

[Theo de Raadt]

+   do {
+   ngen = arc4random_uniform(INT_MAX) + 1;
+   } while (DIP(ip, gen) == ngen);

arc4random_uniform contains a potential loop (approx 1 in 4billion)
for uniform resolution, and this wraps it with another loop (approx 1
in 4billion).

I'd really prefer to see a construct which doesn't do that.  But
I don't know if that would result in something really complex.

> signed integer, it may overflow, causing undefined behavior

The development of OpenBSD/univac is running way behind schedule.

[PATCH] ffs: always assign random inode generation numbers

[Dmitry Chestnykh]

Hello,

In ffs_inode_alloc(), inode generation numbers are incremented after
being assigned an initial random value. Since the di_gen field is a
signed integer, it may overflow, causing undefined behavior (this is
unlikely to ever happen, though).

This patch always assigns a random non-zero positive integer in [1,
INT_MAX] range, not equal to the previous generation number.

--
Dmitry Chestnykh
https://www.codingrobots.com


Index: src/sys/ufs/ffs/ffs_alloc.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
retrieving revision 1.108
diff -u -p -u -r1.108 ffs_alloc.c
--- src/sys/ufs/ffs/ffs_alloc.c 23 May 2016 20:47:49 -  1.108
+++ src/sys/ufs/ffs/ffs_alloc.c 24 Jun 2017 19:25:04 -
@@ -361,6 +361,7 @@ ffs_inode_alloc(struct inode *pip, mode_
struct inode *ip;
ufsino_t ino, ipref;
int cg, error;
+   int32_t ngen;

*vpp = NULL;
fs = pip->i_fs;
@@ -413,16 +414,12 @@ ffs_inode_alloc(struct inode *pip, mode_

/*
 * Set up a new generation number for this inode.
-* XXX - just increment for now, this is wrong! (millert)
-*   Need a way to preserve randomization.
 */
-   if (DIP(ip, gen) != 0)
-   DIP_ADD(ip, gen, 1);
-   if (DIP(ip, gen) == 0)
-   DIP_ASSIGN(ip, gen, arc4random() & INT_MAX);
-
-   if (DIP(ip, gen) == 0 || DIP(ip, gen) == -1)
-   DIP_ASSIGN(ip, gen, 1); /* Shouldn't happen */
+   do {
+   ngen = arc4random_uniform(INT_MAX) + 1;
+   } while (DIP(ip, gen) == ngen);
+
+   DIP_ASSIGN(ip, gen, ngen);

return (0);

Re: [PATCH] cp(1): add -v option for verbosity

[Job Snijders]

Dear Alexander,

On Sun, Jun 25, 2017 at 06:13:40PM +0200, Alexander Hall wrote:
> On June 25, 2017 2:06:20 PM GMT+02:00, Job Snijders  
> wrote:
> >This patch adds a -v option to cp(1) for more verbose output.
> >
> > $ touch a b; mkdir c
> > $ cp -v a b c
> > 'a' -> 'c/a'
> > 'b' -> 'c/b'
> > $ cp -rv c d
> > 'c' -> 'd/'
> > 'c/a' -> 'd/a'
> > 'c/b' -> 'd/b'
> 
> Pardon my ignorance, but why?

A fair question.  I myself have two use cases, but others may have their
own to add.

When a glob pattern is used, it can be beneficial to immediately observe
(during the execution of the command) which files have been copied.

When copying very large trees, the -v option provides some insight as to
what progress the cp operation has made so far.

Alternatively one can use rsync(1), but that is not part of the base.

> Is this a gnu thing? 

Not specifically: freebsd, netbsd, darwin and dragonfly have it too.

Kind regards,

Job

Re: [PATCH] cp(1): add -v option for verbosity

[Alexander Hall]

On June 25, 2017 2:06:20 PM GMT+02:00, Job Snijders  wrote:
>Dear team,
>
>This patch adds a -v option to cp(1) for more verbose output.
>
>   $ touch a b; mkdir c
>   $ cp -v a b c
>   'a' -> 'c/a'
>   'b' -> 'c/b'
>   $ cp -rv c d
>   'c' -> 'd/'
>   'c/a' -> 'd/a'
>   'c/b' -> 'd/b'

Pardon my ignorance, but why? Is this a gnu thing? 

/Alexander 

>
>Kind regards,
>
>Job
>
>diff --git bin/cp/cp.1 bin/cp/cp.1
>index 8573d801ca5..d4346d23f1d 100644
>--- bin/cp/cp.1
>+++ bin/cp/cp.1
>@@ -41,14 +41,14 @@
> .Nd copy files
> .Sh SYNOPSIS
> .Nm cp
>-.Op Fl fip
>+.Op Fl fipv
> .Oo
> .Fl R
> .Op Fl H | L | P
> .Oc
> .Ar source target
> .Nm cp
>-.Op Fl fip
>+.Op Fl fipv
> .Oo
> .Fl R
> .Op Fl H | L | P
>@@ -145,6 +145,8 @@ use a utility such as
> or
> .Xr tar 1
> instead.
>+.It Fl v
>+Explain what is being done.
> .El
> .Pp
> For each destination file that already exists, its contents are
>diff --git bin/cp/cp.c bin/cp/cp.c
>index 643d82ed9fa..819e02f7be8 100644
>--- bin/cp/cp.c
>+++ bin/cp/cp.c
>@@ -71,7 +71,7 @@
> PATH_T to = { to.p_path, "" };
> 
> uid_t myuid;
>-int Rflag, fflag, iflag, pflag, rflag;
>+int Rflag, fflag, iflag, pflag, rflag, vflag;
> mode_t myumask;
> 
> enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE };
>@@ -88,7 +88,7 @@ main(int argc, char *argv[])
>   char *target;
> 
>   Hflag = Lflag = Pflag = Rflag = 0;
>-  while ((ch = getopt(argc, argv, "HLPRfipr")) != -1)
>+  while ((ch = getopt(argc, argv, "HLPRfiprv")) != -1)
>   switch (ch) {
>   case 'H':
>   Hflag = 1;
>@@ -119,6 +119,9 @@ main(int argc, char *argv[])
>   case 'r':
>   rflag = 1;
>   break;
>+  case 'v':
>+  vflag = 1;
>+  break;
>   default:
>   usage();
>   break;
>@@ -394,6 +397,9 @@ copy(char *argv[], enum op type, int fts_options)
>   case S_IFLNK:
>   if (copy_link(curr, !fts_dne(curr)))
>   rval = 1;
>+  else if (vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   case S_IFDIR:
>   if (!Rflag && !rflag) {
>@@ -415,6 +421,9 @@ copy(char *argv[], enum op type, int fts_options)
>   if (mkdir(to.p_path,
>   curr->fts_statp->st_mode | S_IRWXU) < 0)
>   err(1, "%s", to.p_path);
>+  else if (vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   } else if (!S_ISDIR(to_stat.st_mode))
>   errc(1, ENOTDIR, "%s", to.p_path);
>   break;
>@@ -426,6 +435,9 @@ copy(char *argv[], enum op type, int fts_options)
>   } else
>   if (copy_file(curr, fts_dne(curr)))
>   rval = 1;
>+  if (!rval && vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   case S_IFIFO:
>   if (Rflag) {
>@@ -434,6 +446,9 @@ copy(char *argv[], enum op type, int fts_options)
>   } else
>   if (copy_file(curr, fts_dne(curr)))
>   rval = 1;
>+  if (!rval && vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   case S_IFSOCK:
>   warnc(EOPNOTSUPP, "%s", curr->fts_path);
>@@ -441,6 +456,9 @@ copy(char *argv[], enum op type, int fts_options)
>   default:
>   if (copy_file(curr, fts_dne(curr)))
>   rval = 1;
>+  else if (vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   }
>   }
>diff --git bin/cp/utils.c bin/cp/utils.c
>index 6a3c5178647..2189dd4be1f 100644
>--- bin/cp/utils.c
>+++ bin/cp/utils.c
>@@ -307,9 +307,9 @@ void
> usage(void)
> {
>   (void)fprintf(stderr,
>-  "usage: %s [-fip] [-R [-H | -L | -P]] source target\n",
>__progname);
>+  "usage: %s [-fipv] [-R [-H | -L | -P]] source target\n",
>__progname);
>   (void)fprintf(stderr,
>-  "   %s [-fip] [-R [-H | -L | -P]] source ... directory\n",
>+  " 

Re: [PATCH] rm(1): add -v option for verbosity

[Job Snijders]

Hi Ingo,

Thanks for taking the time to review this.

On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote:
> Job Snijders wrote on Sun, Jun 25, 2017 at 02:06:16PM +0200:
> 
> > This patch adds a '-v' option to rm(1) for more verbose output.
> 
> Do not add new options to standard utilities, unless you can show that
> they are unusually useful in practice *and* practically every other
> system out there has them, with a compatible user interface across
> practically all systems.

I checked a number of systems: busybox, HP-UX, and SunOS don't have -v.

FreeBSD, NetBSD, DragonFlyBSD, Slackware, Minix, Debian, CentOS, Suse,
and Darwin do have a -v option to report which files were deleted.

> Adding -v to rm(1) probably wouldn't make the very high bar against
> adding non-standard options even if almost everybody else had it
> (which i didn't check, to not waste time) because it's completely
> useless.

I appreciate the bar is high, and I doubted whether I should send the
patch at all. However, without some proposed code on the table we would
not be able to have this conversation.

> If you are really unsure, study the output of
> 
>   $ find *
> 
> first, before typing
> 
>   $ rm -rf *
> 
> No non-standard option is needed at all for this.

The 'find *' will show you what existed at the moment of executing the
'find' command, where on the other hand 'rm -rfv *' gives a report of
which files actually were succesfully deleted through the execution of
the 'rm' command.

> It's also strongly in violation of the UNIX philosophy (each utility,
> do one thing, and do it well). rm(1) removes files, and does so well.
> rm(1) does *not* trespass on ls(1) and find(1) territory to list
> files.

I'm not sure I agree with you on this point. To me ls(1) and rm(1) have
different uses, rm(1) being able to tell which files it deleted, is not
at all the same as requesting a listing of files.

> Besides, they way your proposed rm(1) extension lists files is not
> doing it well at all. The output is awful.

Yes, the output could be simplified, below a version of the patch to
align with how freebsd and netbsd do it:

$ touch f; mkdir d; touch d/f
$ rm -rfv *
d/f
d
f

Sticking to the standards is a strong argument, and I understand that
changes to core utilities like rm must be thoroughly vetted. Please
don't take offense in me attempting to propose a change.

Kind regards,

Job

diff --git bin/rm/rm.1 bin/rm/rm.1
index 5c8aefaab7d..edc68b60001 100644
--- bin/rm/rm.1
+++ bin/rm/rm.1
@@ -95,6 +95,8 @@ that directory is skipped.
 .It Fl r
 Equivalent to
 .Fl R .
+.It Fl v
+Explain what is being done.
 .El
 .Pp
 The
@@ -148,7 +150,7 @@ utility is compliant with the
 specification.
 .Pp
 The flags
-.Op Fl dP
+.Op Fl dPv
 are extensions to that specification.
 .Sh HISTORY
 An
diff --git bin/rm/rm.c bin/rm/rm.c
index 3242ef5f410..fc0904d0325 100644
--- bin/rm/rm.c
+++ bin/rm/rm.c
@@ -50,7 +50,7 @@
 
 extern char *__progname;
 
-int dflag, eval, fflag, iflag, Pflag, stdin_ok;
+int dflag, eval, fflag, iflag, Pflag, vflag, stdin_ok;
 
 intcheck(char *, char *, struct stat *);
 void   checkdot(char **);
@@ -73,7 +73,7 @@ main(int argc, char *argv[])
int ch, rflag;
 
Pflag = rflag = 0;
-   while ((ch = getopt(argc, argv, "dfiPRr")) != -1)
+   while ((ch = getopt(argc, argv, "dfiPRrv")) != -1)
switch(ch) {
case 'd':
dflag = 1;
@@ -93,6 +93,9 @@ main(int argc, char *argv[])
case 'r':   /* Compatibility. */
rflag = 1;
break;
+   case 'v':
+   vflag = 1;
+   break;
default:
usage();
}
@@ -201,8 +204,11 @@ rm_tree(char **argv)
case FTS_DP:
case FTS_DNR:
if (!rmdir(p->fts_accpath) ||
-   (fflag && errno == ENOENT))
+   (fflag && errno == ENOENT)) {
+   if (vflag)
+   (void)fprintf(stdout, "%s\n", 
p->fts_path);
continue;
+   }
break;
 
case FTS_F:
@@ -213,8 +219,11 @@ rm_tree(char **argv)
/* FALLTHROUGH */
default:
if (!unlink(p->fts_accpath) ||
-   (fflag && errno == ENOENT))
+   (fflag && errno == ENOENT)) {
+   if (vflag)
+   (void)fprintf(stdout, "%s\n", 
p->fts_path);
continue;
+   }
}
warn("%s", p->fts_path);
eval = 1;
@@ -262,7 +271,8 @@ rm_file(char **argv)
if (rval && (!fflag 

Re: [PATCH] mv(1): add -v option for verbosity

[Landry Breuil]

On Sun, Jun 25, 2017 at 02:41:28PM +0200, Mark Kettenis wrote:
> > Date: Sun, 25 Jun 2017 14:06:11 +0200
> > From: Job Snijders 
> > 
> > Hi all,
> > 
> > This patch adds a -v option to mv(1) for more verbose output.
> > 
> > $ touch a
> > $ mv -v a b
> > 'a' -> 'b'
> > $ mkdir c
> > $ mv -v b c
> > 'b' -> 'c/b'
> > $ mv -v c d
> > 'e' -> 'd'
> > 
> > And here is an example of the output of the situation mentioned in the
> > 'caveats' section in the manpage:
> > 
> > $ touch f g; mkdir -p d/f
> > $ mv -v f g d
> > mv: rename f to d/f: Is a directory
> > 'g' -> 'd/g'
> > $ echo $?
> > 1
> > 
> > Kind regards,
> 
> This is not something we want for OpenBSD.

I beg to differ. Personally, i've sometimes had to use rsync -P (or -i,
or -v, or other equivalents) to have some kind of listing/progress of a
large copy/move operation. Granted, it's not in POSIX or other
standards, and it's in GNU mv/coreutils that everyone loves to hate for
the kitchensink approach, but that shouldnt be a reason. If there's a
sensible usecase, why not ?

Landry

Re: [PATCH] rm(1): add -v option for verbosity

[Ingo Schwarze]

Hi,

Job Snijders wrote on Sun, Jun 25, 2017 at 02:06:16PM +0200:

> This patch adds a '-v' option to rm(1) for more verbose output.

Do not add new options to standard utilities, unless you can show
that they are unusually useful in practice *and* practically
every other system out there has them, with a compatible user
interface across practically all systems.

Adding -v to rm(1) probably wouldn't make the very high bar against
adding non-standard options even if almost everybody else had it
(which i didn't check, to not waste time) because it's completely
useless.

If you are really unsure, study the output of

  $ find *

first, before typing

  $ rm -rf *

No non-standard option is needed at all for this.

It's also strongly in violation of the UNIX philosophy (each utility,
do one thing, and do it well).  rm(1) removes files, and does so
well.  rm(1) does *not* trespass on ls(1) and find(1) territory to
list files.  Besides, they way your proposed rm(1) extension lists
files is not doing it well at all.  The output is awful.

Yours,
  Ingo


>   $ mkdir a; touch a/b; touch c
>   $ rm -rfv *
>   removed 'a/b'
>   removed directory 'a'
>   removed 'c'

Re: [PATCH 2/3] openbgpd: Add support for 'unknown' well-known communities

[Job Snijders]

Small update.

I renamed the 'msb' argument ('most significant bits') to 'part' to
improve readability. In Community 15562:4, '15562' is part 0 and the '4'
is part 1. Same type of logic might be useful down the road for Large
Communities which would have 3 parts.

- Job

diff --git usr.sbin/bgpctl/parser.c usr.sbin/bgpctl/parser.c
index 85300d1cd32..c9d63f9ade3 100644
--- usr.sbin/bgpctl/parser.c
+++ usr.sbin/bgpctl/parser.c
@@ -413,7 +413,7 @@ int  parse_addr(const char *, struct 
bgpd_addr *);
 int parse_asnum(const char *, size_t, u_int32_t *);
 int parse_number(const char *, struct parse_result *,
 enum token_type);
-int getcommunity(const char *);
+int getcommunity(const char *, int);
 int parse_community(const char *, struct parse_result *);
 u_int   getlargecommunity(const char *);
 int parse_largecommunity(const char *, struct parse_result 
*);
@@ -927,7 +927,7 @@ parse_number(const char *word, struct parse_result *r, enum 
token_type type)
 }
 
 int
-getcommunity(const char *s)
+getcommunity(const char *s, int part)
 {
const char  *errstr;
u_int16_tuval;
@@ -935,6 +935,9 @@ getcommunity(const char *s)
if (strcmp(s, "*") == 0)
return (COMMUNITY_ANY);
 
+   if (part == 0 && strcmp(s, "WELLKNOWN") == 0)
+   return (COMMUNITY_WELLKNOWN);
+
uval = strtonum(s, 0, USHRT_MAX, );
if (errstr)
errx(1, "Community is %s: %s", errstr, s);
@@ -978,8 +981,8 @@ parse_community(const char *word, struct parse_result *r)
}
*p++ = 0;
 
-   as = getcommunity(word);
-   type = getcommunity(p);
+   as = getcommunity(word, 0);
+   type = getcommunity(p, 1);
 
 done:
if (as == 0) {
@@ -994,10 +997,6 @@ done:
case COMMUNITY_BLACKHOLE:
/* valid */
break;
-   default:
-   /* unknown */
-   fprintf(stderr, "Unknown well-known community\n");
-   return (0);
}
 
if ((fs = calloc(1, sizeof(struct filter_set))) == NULL)
diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
index f0c96051e17..ec4ed956b60 100644
--- usr.sbin/bgpd/parse.y
+++ usr.sbin/bgpd/parse.y
@@ -146,7 +146,7 @@ void copy_filterset(struct filter_set_head 
*,
 voidmerge_filter_lists(struct filter_head *, struct filter_head *);
 struct filter_rule *get_rule(enum action_types);
 
-int getcommunity(char *);
+int getcommunity(char *, int);
 int parsecommunity(struct filter_community *, char *);
 int64_t getlargecommunity(char *);
 int parselargecommunity(struct filter_largecommunity *, char *);
@@ -2963,11 +2963,13 @@ symget(const char *nam)
 }
 
 int
-getcommunity(char *s)
+getcommunity(char *s, int part)
 {
int  val;
const char  *errstr;
 
+   if (part == 0 && strcmp(s, "WELLKNOWN") == 0)
+   return (COMMUNITY_WELLKNOWN);
if (strcmp(s, "*") == 0)
return (COMMUNITY_ANY);
if (strcmp(s, "neighbor-as") == 0)
@@ -3017,15 +3019,11 @@ parsecommunity(struct filter_community *c, char *s)
}
*p++ = 0;
 
-   if ((i = getcommunity(s)) == COMMUNITY_ERROR)
+   if ((i = getcommunity(s, 0)) == COMMUNITY_ERROR)
return (-1);
-   if (i == COMMUNITY_WELLKNOWN) {
-   yyerror("Bad community AS number");
-   return (-1);
-   }
as = i;
 
-   if ((i = getcommunity(p)) == COMMUNITY_ERROR)
+   if ((i = getcommunity(p, 1)) == COMMUNITY_ERROR)
return (-1);
c->as = as;
c->type = i;

Re: [PATCH] mv(1): add -v option for verbosity

[Mark Kettenis]

> Date: Sun, 25 Jun 2017 14:06:11 +0200
> From: Job Snijders 
> 
> Hi all,
> 
> This patch adds a -v option to mv(1) for more verbose output.
> 
>   $ touch a
>   $ mv -v a b
>   'a' -> 'b'
>   $ mkdir c
>   $ mv -v b c
>   'b' -> 'c/b'
> $ mv -v c d
> 'e' -> 'd'
> 
> And here is an example of the output of the situation mentioned in the
> 'caveats' section in the manpage:
> 
>   $ touch f g; mkdir -p d/f
>   $ mv -v f g d
>   mv: rename f to d/f: Is a directory
>   'g' -> 'd/g'
>   $ echo $?
>   1
> 
> Kind regards,

This is not something we want for OpenBSD.

> diff --git bin/mv/mv.1 bin/mv/mv.1
> index cb6c9d92673..fc8e642017e 100644
> --- bin/mv/mv.1
> +++ bin/mv/mv.1
> @@ -103,6 +103,8 @@ The
>  option overrides any previous
>  .Fl f
>  options.
> +.It Fl v
> +Explain what is being done.
>  .El
>  .Pp
>  The
> diff --git bin/mv/mv.c bin/mv/mv.c
> index 003aca59e87..fa8654b50e4 100644
> --- bin/mv/mv.c
> +++ bin/mv/mv.c
> @@ -51,7 +51,7 @@
>  
>  extern char *__progname;
>  
> -int fflg, iflg;
> +int fflg, iflg, vflg;
>  int stdin_ok;
>  
>  extern int cpmain(int argc, char **argv);
> @@ -71,7 +71,7 @@ main(int argc, char *argv[])
>   int ch;
>   char path[PATH_MAX];
>  
> - while ((ch = getopt(argc, argv, "if")) != -1)
> + while ((ch = getopt(argc, argv, "ifv")) != -1)
>   switch (ch) {
>   case 'i':
>   fflg = 0;
> @@ -81,6 +81,9 @@ main(int argc, char *argv[])
>   iflg = 0;
>   fflg = 1;
>   break;
> + case 'v':
> + vflg = 1;
> + break;
>   default:
>   usage();
>   }
> @@ -208,8 +211,11 @@ do_move(char *from, char *to)
>*  message to standard error, and do nothing more with the
>*  current source file...
>*/
> - if (!rename(from, to))
> + if (!rename(from, to)) {
> + if (vflg)
> + (void)fprintf(stdout, "'%s' -> '%s'\n", from, to);
>   return (0);
> + }
>  
>   if (errno != EXDEV) {
>   warn("rename %s to %s", from, to);
> @@ -339,6 +345,10 @@ err: if (unlink(to))
>   warn("%s: remove", from);
>   return (1);
>   }
> +
> + if (vflg)
> + (void)fprintf(stdout, "'%s' -> '%s'\n", from, to);
> +
>   return (0);
>  }
>  
> @@ -362,14 +372,17 @@ mvcopy(char *from, char *to)
>   _exit(1);
>   }
>  
> + if (vflg)
> + (void)fprintf(stdout, "'%s' -> '%s'\n", from, to);
> +
>   return (0);
>  }
>  
>  void
>  usage(void)
>  {
> - (void)fprintf(stderr, "usage: %s [-fi] source target\n", __progname);
> - (void)fprintf(stderr, "   %s [-fi] source ... directory\n",
> + (void)fprintf(stderr, "usage: %s [-fiv] source target\n", __progname);
> + (void)fprintf(stderr, "   %s [-fiv] source ... directory\n",
>   __progname);
>   exit(1);
>  }
> 
> 

Re: [PATCH 2/3] openbgpd: Add support for 'unknown' well-known communities

[Job Snijders]

On Sun, Jun 25, 2017 at 02:09:22PM +0200, Peter Hessler wrote:
> : $ bgpctl show rib community WELLKNOWN:0
> : ..
> : $ bgpctl show rib community WELLKNOWN:*
> : ..
> 
> Eh, I don't really see a reason to have syntatic sugar for
> '65535'.  In this case, I'm more likely to remember then number than
> which string to use ;).
> 
> : $ doas cat /etc/bgpd.conf | grep set
> : match from any set { community WELLKNOWN:0 community 65535:1 }
> 
> Same as before.  OK for setting 65535:1, but 'E' for
> 'WELLKNOWN:0'.
> 
> However, if we have one, then we need to have the other.

Current bgpd(8) on 6.1 uses the WELLKNOWN sugar in bgpctl output, i felt
it would be proper to allow it in bgpd.conf and bgpctl input too.

6.1 example:

[job@karen ~] $ bgpctl show rib  detail 192.147.168.0/25

BGP routing table entry for 192.147.168.0/25
Nexthop 192.147.168.249 (via 192.147.168.249) from 
192.147.168.249 (192.147.168.1)
Origin IGP, metric 0, localpref 100, weight 0, internal, valid, 
best
Last update: 00:00:08 ago
Communities: WELLKNOWN:1

Kind regards,

Job

Re: [PATCH 2/3] openbgpd: Add support for 'unknown' well-known communities

[Peter Hessler]

On 2017 Jun 23 (Fri) at 16:01:58 +0200 (+0200), Job Snijders wrote:
:Dear team,
:
:This patch makes 'unknown' well-known communities more of a first-class
:citizen.
:
:A powerful property of well-known communities is that (often) operators
:can implement the feature associated with a given well-known community
:through their local routing policy, ahead of time before their vendor
:releasing native support in the implementation. 
:
:Things that work now:
:
:   $ bgpctl show rib community 65535:0
:   ..

OK


:   $ bgpctl show rib community WELLKNOWN:0
:   ..
:   $ bgpctl show rib community WELLKNOWN:*
:   ..
:

Eh, I don't really see a reason to have syntatic sugar for '65535'.
In this case, I'm more likely to remember then number than which string
to use ;).


:   $ doas cat /etc/bgpd.conf | grep set
:   match from any set { community WELLKNOWN:0 community 65535:1 }
:

Same as before.  OK for setting 65535:1, but 'E' for 'WELLKNOWN:0'.

However, if we have one, then we need to have the other.


:Kind regards,
:
:Job
:
:---
: usr.sbin/bgpctl/parser.c | 15 +++
: usr.sbin/bgpd/parse.y| 14 ++
: 2 files changed, 13 insertions(+), 16 deletions(-)
:
:diff --git a/usr.sbin/bgpctl/parser.c b/usr.sbin/bgpctl/parser.c
:index 85300d1cd32..0d1e5d9fb3a 100644
:--- a/usr.sbin/bgpctl/parser.c
:+++ b/usr.sbin/bgpctl/parser.c
:@@ -413,7 +413,7 @@ int parse_addr(const char *, 
struct bgpd_addr *);
: intparse_asnum(const char *, size_t, u_int32_t *);
: intparse_number(const char *, struct parse_result *,
:enum token_type);
:-intgetcommunity(const char *);
:+intgetcommunity(const char *, int);
: intparse_community(const char *, struct parse_result *);
: u_int  getlargecommunity(const char *);
: intparse_largecommunity(const char *, struct parse_result 
*);
:@@ -927,7 +927,7 @@ parse_number(const char *word, struct parse_result *r, 
enum token_type type)
: }
: 
: int
:-getcommunity(const char *s)
:+getcommunity(const char *s, int msb)
: {
:   const char  *errstr;
:   u_int16_tuval;
:@@ -935,6 +935,9 @@ getcommunity(const char *s)
:   if (strcmp(s, "*") == 0)
:   return (COMMUNITY_ANY);
: 
:+  if (msb == 1 && strcmp(s, "WELLKNOWN") == 0)
:+  return (COMMUNITY_WELLKNOWN);
:+
:   uval = strtonum(s, 0, USHRT_MAX, );
:   if (errstr)
:   errx(1, "Community is %s: %s", errstr, s);
:@@ -978,8 +981,8 @@ parse_community(const char *word, struct parse_result *r)
:   }
:   *p++ = 0;
: 
:-  as = getcommunity(word);
:-  type = getcommunity(p);
:+  as = getcommunity(word, 1);
:+  type = getcommunity(p, 0);
: 
: done:
:   if (as == 0) {
:@@ -994,10 +997,6 @@ done:
:   case COMMUNITY_BLACKHOLE:
:   /* valid */
:   break;
:-  default:
:-  /* unknown */
:-  fprintf(stderr, "Unknown well-known community\n");
:-  return (0);
:   }
: 
:   if ((fs = calloc(1, sizeof(struct filter_set))) == NULL)
:diff --git a/usr.sbin/bgpd/parse.y b/usr.sbin/bgpd/parse.y
:index f0c96051e17..73bdb3a0cb9 100644
:--- a/usr.sbin/bgpd/parse.y
:+++ b/usr.sbin/bgpd/parse.y
:@@ -146,7 +146,7 @@ voidcopy_filterset(struct filter_set_head 
*,
: void   merge_filter_lists(struct filter_head *, struct filter_head *);
: struct filter_rule*get_rule(enum action_types);
: 
:-intgetcommunity(char *);
:+intgetcommunity(char *, int);
: intparsecommunity(struct filter_community *, char *);
: int64_tgetlargecommunity(char *);
: intparselargecommunity(struct filter_largecommunity *, char *);
:@@ -2963,11 +2963,13 @@ symget(const char *nam)
: }
: 
: int
:-getcommunity(char *s)
:+getcommunity(char *s, int msb)
: {
:   int  val;
:   const char  *errstr;
: 
:+  if (msb == 1 && strcmp(s, "WELLKNOWN") == 0)
:+  return (COMMUNITY_WELLKNOWN);
:   if (strcmp(s, "*") == 0)
:   return (COMMUNITY_ANY);
:   if (strcmp(s, "neighbor-as") == 0)
:@@ -3017,15 +3019,11 @@ parsecommunity(struct filter_community *c, char *s)
:   }
:   *p++ = 0;
: 
:-  if ((i = getcommunity(s)) == COMMUNITY_ERROR)
:+  if ((i = getcommunity(s, 1)) == COMMUNITY_ERROR)
:   return (-1);
:-  if (i == COMMUNITY_WELLKNOWN) {
:-  yyerror("Bad community AS number");
:-  return (-1);
:-  }
:   as = i;
: 
:-  if ((i = getcommunity(p)) == COMMUNITY_ERROR)
:+  if ((i = getcommunity(p, 0)) == COMMUNITY_ERROR)
:   return (-1);
:   c->as = as;
:   c->type = i;
:

-- 
There is nothing wrong with 

[PATCH] cp(1): add -v option for verbosity

[Job Snijders]

Dear team,

This patch adds a -v option to cp(1) for more verbose output.

$ touch a b; mkdir c
$ cp -v a b c
'a' -> 'c/a'
'b' -> 'c/b'
$ cp -rv c d
'c' -> 'd/'
'c/a' -> 'd/a'
'c/b' -> 'd/b'

Kind regards,

Job

diff --git bin/cp/cp.1 bin/cp/cp.1
index 8573d801ca5..d4346d23f1d 100644
--- bin/cp/cp.1
+++ bin/cp/cp.1
@@ -41,14 +41,14 @@
 .Nd copy files
 .Sh SYNOPSIS
 .Nm cp
-.Op Fl fip
+.Op Fl fipv
 .Oo
 .Fl R
 .Op Fl H | L | P
 .Oc
 .Ar source target
 .Nm cp
-.Op Fl fip
+.Op Fl fipv
 .Oo
 .Fl R
 .Op Fl H | L | P
@@ -145,6 +145,8 @@ use a utility such as
 or
 .Xr tar 1
 instead.
+.It Fl v
+Explain what is being done.
 .El
 .Pp
 For each destination file that already exists, its contents are
diff --git bin/cp/cp.c bin/cp/cp.c
index 643d82ed9fa..819e02f7be8 100644
--- bin/cp/cp.c
+++ bin/cp/cp.c
@@ -71,7 +71,7 @@
 PATH_T to = { to.p_path, "" };
 
 uid_t myuid;
-int Rflag, fflag, iflag, pflag, rflag;
+int Rflag, fflag, iflag, pflag, rflag, vflag;
 mode_t myumask;
 
 enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE };
@@ -88,7 +88,7 @@ main(int argc, char *argv[])
char *target;
 
Hflag = Lflag = Pflag = Rflag = 0;
-   while ((ch = getopt(argc, argv, "HLPRfipr")) != -1)
+   while ((ch = getopt(argc, argv, "HLPRfiprv")) != -1)
switch (ch) {
case 'H':
Hflag = 1;
@@ -119,6 +119,9 @@ main(int argc, char *argv[])
case 'r':
rflag = 1;
break;
+   case 'v':
+   vflag = 1;
+   break;
default:
usage();
break;
@@ -394,6 +397,9 @@ copy(char *argv[], enum op type, int fts_options)
case S_IFLNK:
if (copy_link(curr, !fts_dne(curr)))
rval = 1;
+   else if (vflag)
+   (void)fprintf(stdout, "'%s' -> '%s'\n",
+   curr->fts_path, to.p_path);
break;
case S_IFDIR:
if (!Rflag && !rflag) {
@@ -415,6 +421,9 @@ copy(char *argv[], enum op type, int fts_options)
if (mkdir(to.p_path,
curr->fts_statp->st_mode | S_IRWXU) < 0)
err(1, "%s", to.p_path);
+   else if (vflag)
+   (void)fprintf(stdout, "'%s' -> '%s'\n",
+   curr->fts_path, to.p_path);
} else if (!S_ISDIR(to_stat.st_mode))
errc(1, ENOTDIR, "%s", to.p_path);
break;
@@ -426,6 +435,9 @@ copy(char *argv[], enum op type, int fts_options)
} else
if (copy_file(curr, fts_dne(curr)))
rval = 1;
+   if (!rval && vflag)
+   (void)fprintf(stdout, "'%s' -> '%s'\n",
+   curr->fts_path, to.p_path);
break;
case S_IFIFO:
if (Rflag) {
@@ -434,6 +446,9 @@ copy(char *argv[], enum op type, int fts_options)
} else
if (copy_file(curr, fts_dne(curr)))
rval = 1;
+   if (!rval && vflag)
+   (void)fprintf(stdout, "'%s' -> '%s'\n",
+   curr->fts_path, to.p_path);
break;
case S_IFSOCK:
warnc(EOPNOTSUPP, "%s", curr->fts_path);
@@ -441,6 +456,9 @@ copy(char *argv[], enum op type, int fts_options)
default:
if (copy_file(curr, fts_dne(curr)))
rval = 1;
+   else if (vflag)
+   (void)fprintf(stdout, "'%s' -> '%s'\n",
+   curr->fts_path, to.p_path);
break;
}
}
diff --git bin/cp/utils.c bin/cp/utils.c
index 6a3c5178647..2189dd4be1f 100644
--- bin/cp/utils.c
+++ bin/cp/utils.c
@@ -307,9 +307,9 @@ void
 usage(void)
 {
(void)fprintf(stderr,
-   "usage: %s [-fip] [-R [-H | -L | -P]] source target\n", __progname);
+   "usage: %s [-fipv] [-R [-H | -L | -P]] source target\n", 
__progname);
(void)fprintf(stderr,
-   "   %s [-fip] [-R [-H | -L | -P]] source ... directory\n",
+   "   %s [-fipv] [-R [-H | -L | -P]] source ... directory\n",
__progname);
exit(1);
 }

[PATCH] rm(1): add -v option for verbosity

[Job Snijders]

Hi all,

This patch adds a '-v' option to rm(1) for more verbose output.

$ mkdir a; touch a/b; touch c
$ rm -rfv *
removed 'a/b'
removed directory 'a'
removed 'c'

Kind regards,

Job

diff --git bin/rm/rm.1 bin/rm/rm.1
index 5c8aefaab7d..7de2c7067ee 100644
--- bin/rm/rm.1
+++ bin/rm/rm.1
@@ -95,6 +95,8 @@ that directory is skipped.
 .It Fl r
 Equivalent to
 .Fl R .
+.It Fl v
+Explain what is being done.
 .El
 .Pp
 The
diff --git bin/rm/rm.c bin/rm/rm.c
index 3242ef5f410..574609c64e9 100644
--- bin/rm/rm.c
+++ bin/rm/rm.c
@@ -50,7 +50,7 @@
 
 extern char *__progname;
 
-int dflag, eval, fflag, iflag, Pflag, stdin_ok;
+int dflag, eval, fflag, iflag, Pflag, vflag, stdin_ok;
 
 intcheck(char *, char *, struct stat *);
 void   checkdot(char **);
@@ -73,7 +73,7 @@ main(int argc, char *argv[])
int ch, rflag;
 
Pflag = rflag = 0;
-   while ((ch = getopt(argc, argv, "dfiPRr")) != -1)
+   while ((ch = getopt(argc, argv, "dfiPRrv")) != -1)
switch(ch) {
case 'd':
dflag = 1;
@@ -93,6 +93,9 @@ main(int argc, char *argv[])
case 'r':   /* Compatibility. */
rflag = 1;
break;
+   case 'v':
+   vflag = 1;
+   break;
default:
usage();
}
@@ -201,8 +204,12 @@ rm_tree(char **argv)
case FTS_DP:
case FTS_DNR:
if (!rmdir(p->fts_accpath) ||
-   (fflag && errno == ENOENT))
+   (fflag && errno == ENOENT)) {
+   if (vflag)
+   (void)fprintf(stdout,
+   "removed directory '%s'\n", 
p->fts_path);
continue;
+   }
break;
 
case FTS_F:
@@ -213,8 +220,12 @@ rm_tree(char **argv)
/* FALLTHROUGH */
default:
if (!unlink(p->fts_accpath) ||
-   (fflag && errno == ENOENT))
+   (fflag && errno == ENOENT)) {
+   if (vflag)
+   (void)fprintf(stdout,
+   "removed '%s'\n", p->fts_path);
continue;
+   }
}
warn("%s", p->fts_path);
eval = 1;
@@ -262,7 +273,8 @@ rm_file(char **argv)
if (rval && (!fflag || errno != ENOENT)) {
warn("%s", f);
eval = 1;
-   }
+   } else if (vflag)
+   (void)fprintf(stdout, "removed '%s'\n", f);
}
 }
 
@@ -430,6 +442,6 @@ skip:
 void
 usage(void)
 {
-   (void)fprintf(stderr, "usage: %s [-dfiPRr] file ...\n", __progname);
+   (void)fprintf(stderr, "usage: %s [-dfiPRrv] file ...\n", __progname);
exit(1);
 }

[PATCH] mv(1): add -v option for verbosity

[Job Snijders]

Hi all,

This patch adds a -v option to mv(1) for more verbose output.

$ touch a
$ mv -v a b
'a' -> 'b'
$ mkdir c
$ mv -v b c
'b' -> 'c/b'
$ mv -v c d
'e' -> 'd'

And here is an example of the output of the situation mentioned in the
'caveats' section in the manpage:

$ touch f g; mkdir -p d/f
$ mv -v f g d
mv: rename f to d/f: Is a directory
'g' -> 'd/g'
$ echo $?
1

Kind regards,

Job

diff --git bin/mv/mv.1 bin/mv/mv.1
index cb6c9d92673..fc8e642017e 100644
--- bin/mv/mv.1
+++ bin/mv/mv.1
@@ -103,6 +103,8 @@ The
 option overrides any previous
 .Fl f
 options.
+.It Fl v
+Explain what is being done.
 .El
 .Pp
 The
diff --git bin/mv/mv.c bin/mv/mv.c
index 003aca59e87..fa8654b50e4 100644
--- bin/mv/mv.c
+++ bin/mv/mv.c
@@ -51,7 +51,7 @@
 
 extern char *__progname;
 
-int fflg, iflg;
+int fflg, iflg, vflg;
 int stdin_ok;
 
 extern int cpmain(int argc, char **argv);
@@ -71,7 +71,7 @@ main(int argc, char *argv[])
int ch;
char path[PATH_MAX];
 
-   while ((ch = getopt(argc, argv, "if")) != -1)
+   while ((ch = getopt(argc, argv, "ifv")) != -1)
switch (ch) {
case 'i':
fflg = 0;
@@ -81,6 +81,9 @@ main(int argc, char *argv[])
iflg = 0;
fflg = 1;
break;
+   case 'v':
+   vflg = 1;
+   break;
default:
usage();
}
@@ -208,8 +211,11 @@ do_move(char *from, char *to)
 *  message to standard error, and do nothing more with the
 *  current source file...
 */
-   if (!rename(from, to))
+   if (!rename(from, to)) {
+   if (vflg)
+   (void)fprintf(stdout, "'%s' -> '%s'\n", from, to);
return (0);
+   }
 
if (errno != EXDEV) {
warn("rename %s to %s", from, to);
@@ -339,6 +345,10 @@ err:   if (unlink(to))
warn("%s: remove", from);
return (1);
}
+
+   if (vflg)
+   (void)fprintf(stdout, "'%s' -> '%s'\n", from, to);
+
return (0);
 }
 
@@ -362,14 +372,17 @@ mvcopy(char *from, char *to)
_exit(1);
}
 
+   if (vflg)
+   (void)fprintf(stdout, "'%s' -> '%s'\n", from, to);
+
return (0);
 }
 
 void
 usage(void)
 {
-   (void)fprintf(stderr, "usage: %s [-fi] source target\n", __progname);
-   (void)fprintf(stderr, "   %s [-fi] source ... directory\n",
+   (void)fprintf(stderr, "usage: %s [-fiv] source target\n", __progname);
+   (void)fprintf(stderr, "   %s [-fiv] source ... directory\n",
__progname);
exit(1);
 }

Re: [PATCH 3/3] openbgpd: Add well-known community GRACEFUL_SHUTDOWN

[Peter Hessler]

OK

On 2017 Jun 23 (Fri) at 16:02:13 +0200 (+0200), Job Snijders wrote:
:Dear team,
:
:This patch adds support for the "graceful shutdown" well-known
:community as described in draft-ietf-grow-bgp-gshut.
:
:An example implementation would be to add the following to your
:bgpd.conf:
:
:match from any community GRACEFUL_SHUTDOWN set { localpref 0 }
:
:Kind regards,
:
:Job
:
:---
: etc/examples/bgpd.conf| 4 
: usr.sbin/bgpctl/bgpctl.c  | 3 +++
: usr.sbin/bgpctl/parser.c  | 7 ++-
: usr.sbin/bgpd/bgpd.conf.5 | 2 ++
: usr.sbin/bgpd/bgpd.h  | 1 +
: usr.sbin/bgpd/parse.y | 6 +-
: 6 files changed, 21 insertions(+), 2 deletions(-)
:
:diff --git a/etc/examples/bgpd.conf b/etc/examples/bgpd.conf
:index 2ec37b2c752..1caf200ceab 100644
:--- a/etc/examples/bgpd.conf
:+++ b/etc/examples/bgpd.conf
:@@ -87,6 +87,10 @@ allow from any inet6 prefixlen 16 - 48
: #allow from any prefix 0.0.0.0/0
: #allow from any prefix ::/0
: 
:+# Honor requests to gracefully shutdown BGP sessions
:+# https://tools.ietf.org/html/draft-ietf-grow-bgp-gshut
:+match from any community GRACEFUL_SHUTDOWN set { localpref 0 }
:+
: # https://www.arin.net/announcements/2014/20140130.html
: # This block will be subject to a minimum size allocation of /28 and a
: # maximum size allocation of /24. ARIN should use sparse allocation when
:diff --git a/usr.sbin/bgpctl/bgpctl.c b/usr.sbin/bgpctl/bgpctl.c
:index 4d9701da35b..8baa8be0ff2 100644
:--- a/usr.sbin/bgpctl/bgpctl.c
:+++ b/usr.sbin/bgpctl/bgpctl.c
:@@ -1532,6 +1532,9 @@ show_community(u_char *data, u_int16_t len)
:   v = ntohs(v);
:   if (a == COMMUNITY_WELLKNOWN)
:   switch (v) {
:+  case COMMUNITY_GRACEFUL_SHUTDOWN:
:+  printf("GRACEFUL_SHUTDOWN");
:+  break;
:   case COMMUNITY_NO_EXPORT:
:   printf("NO_EXPORT");
:   break;
:diff --git a/usr.sbin/bgpctl/parser.c b/usr.sbin/bgpctl/parser.c
:index 0d1e5d9fb3a..4ea16533b71 100644
:--- a/usr.sbin/bgpctl/parser.c
:+++ b/usr.sbin/bgpctl/parser.c
:@@ -953,7 +953,11 @@ parse_community(const char *word, struct parse_result *r)
:   int  as, type;
: 
:   /* Well-known communities */
:-  if (strcasecmp(word, "NO_EXPORT") == 0) {
:+  if (strcasecmp(word, "GRACEFUL_SHUTDOWN") == 0) {
:+  as = COMMUNITY_WELLKNOWN;
:+  type = COMMUNITY_GRACEFUL_SHUTDOWN;
:+  goto done;
:+  } else if (strcasecmp(word, "NO_EXPORT") == 0) {
:   as = COMMUNITY_WELLKNOWN;
:   type = COMMUNITY_NO_EXPORT;
:   goto done;
:@@ -991,6 +995,7 @@ done:
:   }
:   if (as == COMMUNITY_WELLKNOWN)
:   switch (type) {
:+  case COMMUNITY_GRACEFUL_SHUTDOWN:
:   case COMMUNITY_NO_EXPORT:
:   case COMMUNITY_NO_ADVERTISE:
:   case COMMUNITY_NO_EXPSUBCONFED:
:diff --git a/usr.sbin/bgpd/bgpd.conf.5 b/usr.sbin/bgpd/bgpd.conf.5
:index 6cecd7a5a80..3afc54ef385 100644
:--- a/usr.sbin/bgpd/bgpd.conf.5
:+++ b/usr.sbin/bgpd/bgpd.conf.5
:@@ -1173,6 +1173,7 @@ to do wildcard matching.
: Alternatively, well-known communities may be given by name instead and
: include
: .Ic BLACKHOLE ,
:+.Ic GRACEFUL_SHUTDOWN ,
: .Ic NO_EXPORT ,
: .Ic NO_ADVERTISE ,
: .Ic NO_EXPORT_SUBCONFED ,
:@@ -1444,6 +1445,7 @@ is an AS number and
: is a locally-significant number between zero and
: .Li 65535 .
: Alternately, well-known communities may be specified by name:
:+.Ic GRACEFUL_SHUTDOWN ,
: .Ic NO_EXPORT ,
: .Ic NO_ADVERTISE ,
: .Ic NO_EXPORT_SUBCONFED ,
:diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h
:index db52f858241..ef4e30ffd94 100644
:--- a/usr.sbin/bgpd/bgpd.h
:+++ b/usr.sbin/bgpd/bgpd.h
:@@ -750,6 +750,7 @@ struct filter_peers {
: #define   COMMUNITY_LOCAL_AS  -4
: #define   COMMUNITY_UNSET -5
: #define   COMMUNITY_WELLKNOWN 0x
:+#define   COMMUNITY_GRACEFUL_SHUTDOWN 0x  /* 
draft-ietf-grow-bgp-gshut */
: #define   COMMUNITY_BLACKHOLE 0x029A  /* RFC 7999 */
: #define   COMMUNITY_NO_EXPORT 0xff01
: #define   COMMUNITY_NO_ADVERTISE  0xff02
:diff --git a/usr.sbin/bgpd/parse.y b/usr.sbin/bgpd/parse.y
:index 73bdb3a0cb9..0b09f83bc0a 100644
:--- a/usr.sbin/bgpd/parse.y
:+++ b/usr.sbin/bgpd/parse.y
:@@ -2991,7 +2991,11 @@ parsecommunity(struct filter_community *c, char *s)
:   int i, as;
: 
:   /* Well-known communities */
:-  if (strcasecmp(s, "NO_EXPORT") == 0) {
:+  if (strcasecmp(s, "GRACEFUL_SHUTDOWN") == 0) {
:+  c->as = COMMUNITY_WELLKNOWN;
:+  c->type = COMMUNITY_GRACEFUL_SHUTDOWN;
:+  return (0);
:+  } else if (strcasecmp(s, "NO_EXPORT") == 0) {
:   c->as = COMMUNITY_WELLKNOWN;
:   c->type = COMMUNITY_NO_EXPORT;
:   

Re: [PATCH 1/3] openbgpd: Allow localpref of zero

[Peter Hessler]

OK

On 2017 Jun 23 (Fri) at 16:01:16 +0200 (+0200), Job Snijders wrote:
:Dear team,
:
:The lowest valid BGP LOCAL_PREF is 0, allowing bgpd to set 0 too will
:accomodate interopability.
:
:Kind regards,
:
:Job
:
:--- a/usr.sbin/bgpd/parse.y
:+++ b/usr.sbin/bgpd/parse.y
:@@ -1988,7 +1988,7 @@ filter_set_opt   : LOCALPREF NUMBER  {
:   }
:   if (($$ = calloc(1, sizeof(struct filter_set))) == NULL)
:   fatal(NULL);
:-  if ($2 > 0) {
:+  if ($2 >= 0) {
:   $$->type = ACTION_SET_LOCALPREF;
:   $$->action.metric = $2;
:   } else {
:

-- 
"A radioactive cat has eighteen half-lives."

Re: ksh(1): don't output invalid UTF-8 characters

[Anton Lindqvist]

For reference, I just committed the fix, see message below. Thanks to
all who helped out.

> CVSROOT:  /cvs
> Module name:  src
> Changes by:   an...@cvs.openbsd.org   2017/06/25 02:51:53
>
> Modified files:
>   bin/ksh: emacs.c 
>
> Log message:
> Don't output partial UTF-8 characters in ksh emacs mode. Instead, try to read 
> a
> complete UTF-8 character first. Fixes an issue while running ksh in tmux where
> UTF-8 characters inserted in columns other than the last one are discarded.
>
> With help from nicm@ and schwarze@ who also wrote the UTF-8 validation, 
> thanks!
>
> ok schwarze@