Re: [patch] improve strptime(3) %z timezone parsing
On Fri, 10 May 2019 23:06:21 +0200, Ingo Schwarze wrote: > The following patch preserves the parsing behaviour > and correctly stores the number of seconds into tm_gmtoff. That one looks correct. OK millert@ - todd
Re: enable pfctl to flush all rules and tables
Hello,
Petr Hoffmann pointed out three nits off-list.
we are better to use errx() instead of fprintf() + exit here:
+pfctl_get_anchors(int dev, int opts)
+{
+
+ if (pfra.pfra == NULL)
+ errx(1,
+ "%s failed to allocate main anchor, can't continue\n",
+ __func__);
+
+ SLIST_INSERT_HEAD(, pfra.pfra, pfra_sle);
+
+ opts |= PF_OPT_VERBOSE;
+ if (pfctl_walk_anchors(dev, opts, "", , pfctl_walk_get))
+ errx(1,
+ "%s failed to retrieve list of anchors, can't continue\n",
And there was a forgotten else statement here:
+pfctl_walk_get(int opts, struct pfioc_ruleset *pr, void *warg)
+{
+
+ if (tail_pfra->pfra != NULL)
+ SLIST_INSERT_AFTER(tail_pfra->pfra, pfra, pfra_sle);
+ else
+ tail_pfra->pfra = pfra;
+
complete updated diff is attached.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index f56f6f9e90b..918832b73cf 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -54,6 +54,8 @@
#include
+#include
+
#include "pfctl_parser.h"
#include "pfctl.h"
@@ -63,7 +65,7 @@ intpfctl_disable(int, int);
voidpfctl_clear_queues(struct pf_qihead *);
voidpfctl_clear_stats(int, const char *, int);
voidpfctl_clear_interface_flags(int, int);
-voidpfctl_clear_rules(int, int, char *);
+int pfctl_clear_rules(int, int, char *);
voidpfctl_clear_src_nodes(int, int);
voidpfctl_clear_states(int, const char *, int);
struct addrinfo *
@@ -106,6 +108,16 @@ const char *pfctl_lookup_option(char *, const char **);
void pfctl_state_store(int, const char *);
void pfctl_state_load(int, const char *);
void pfctl_reset(int, int);
+intpfctl_walk_show(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_get(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_anchors(int, int, char *, void *,
+int(*)(int, struct pfioc_ruleset *, void *));
+struct pfr_anchors *
+ pfctl_get_anchors(int, int);
+intpfctl_recurse(int, int, int(*nuke)(int, int, struct pfr_anchoritem *));
+intpfctl_call_clearrules(int, int, struct pfr_anchoritem *);
+intpfctl_call_cleartables(int, int, struct pfr_anchoritem *);
+intpfctl_call_clearanchors(int, int, struct pfr_anchoritem *);
const char *clearopt;
char *rulesopt;
@@ -125,6 +137,7 @@ char*state_kill[2];
int dev = -1;
int first_title = 1;
int labels = 0;
+int exit_val = 0;
#define INDENT(d, o) do {\
if (o) {\
@@ -234,7 +247,6 @@ static const char *optiopt_list[] = {
struct pf_qihead qspecs = TAILQ_HEAD_INITIALIZER(qspecs);
struct pf_qihead rootqs = TAILQ_HEAD_INITIALIZER(rootqs);
-
__dead void
usage(void)
{
@@ -251,6 +263,40 @@ usage(void)
exit(1);
}
+void
+pfctl_err(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verr(1, fmt, ap);
+ else
+ vwarn(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
+void
+pfctl_errx(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verrx(1, fmt, ap);
+ else
+ vwarnx(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
int
pfctl_enable(int dev, int opts)
{
@@ -289,10 +335,10 @@ pfctl_clear_stats(int dev, const char *iface, int opts)
memset(, 0, sizeof(pi));
if (iface != NULL && strlcpy(pi.pfiio_name, iface,
sizeof(pi.pfiio_name)) >= sizeof(pi.pfiio_name))
- errx(1, "invalid interface: %s", iface);
+ pfctl_errx(opts, 1, "invalid interface: %s", iface);
if (ioctl(dev, DIOCCLRSTATUS, ))
- err(1, "DIOCCLRSTATUS");
+ pfctl_err(opts, 1, "DIOCCLRSTATUS");
if ((opts & PF_OPT_QUIET) == 0) {
fprintf(stderr, "pf: statistics cleared");
if (iface != NULL)
@@ -311,32 +357,36 @@ pfctl_clear_interface_flags(int dev, int opts)
pi.pfiio_flags = PFI_IFLAG_SKIP;
if (ioctl(dev, DIOCCLRIFFLAG, ))
- err(1, "DIOCCLRIFFLAG");
+ pfctl_err(opts, 1, "DIOCCLRIFFLAG");
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "pf: interface flags reset\n");
}
}
-void
+int
pfctl_clear_rules(int dev, int opts, char *anchorname)
{
- struct pfr_buffer t;
+ struct pfr_buffer t;
+ int rv = 0;
memset(, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
if (pfctl_add_trans(,
Re: [patch] improve strptime(3) %z timezone parsing
Hi,
> That one looks correct. OK millert@
Committed, thanks for checking!
While here, i noticed ugly preprocessor macros.
Let's make our future life easier by unifdefing a bit.
When compiling with -g0, there is no object change.
Note that if TM_ZONE is not defined, wcsftime.c doesn't
currently even compile. Talk about code in the tree that
is never tested.
OK?
Ingo
Index: localtime.c
===
RCS file: /cvs/src/lib/libc/time/localtime.c,v
retrieving revision 1.59
diff -u -p -r1.59 localtime.c
--- localtime.c 19 Sep 2016 12:48:21 - 1.59
+++ localtime.c 10 May 2019 22:17:14 -
@@ -46,7 +46,7 @@
** in which Daylight Saving Time is never observed.
** 4. They might reference tzname[0] after setting to a time zone
** in which Standard Time is never observed.
-** 5. They might reference tm.TM_ZONE after calling offtime.
+** 5. They might reference tm.tm_zone after calling offtime.
** What's best to do in the above cases is open to debate;
** for now, we just set things up so that in any of the five cases
** WILDABBR is used. Another possibility: initialize tzname[0] to the
@@ -214,14 +214,8 @@ DEF_WEAK(tzname);
static struct tm tm;
-#ifdef USG_COMPAT
long timezone = 0;
intdaylight = 0;
-#endif /* defined USG_COMPAT */
-
-#ifdef ALTZONE
-time_t altzone = 0;
-#endif /* defined ALTZONE */
static long
detzcode(const char *codep)
@@ -255,13 +249,8 @@ settzname(void)
tzname[0] = wildabbr;
tzname[1] = wildabbr;
-#ifdef USG_COMPAT
daylight = 0;
timezone = 0;
-#endif /* defined USG_COMPAT */
-#ifdef ALTZONE
- altzone = 0;
-#endif /* defined ALTZONE */
if (sp == NULL) {
tzname[0] = tzname[1] = (char *)gmt;
return;
@@ -273,16 +262,10 @@ settzname(void)
const struct ttinfo *ttisp = >ttis[sp->types[i]];
tzname[ttisp->tt_isdst] = >chars[ttisp->tt_abbrind];
-#ifdef USG_COMPAT
if (ttisp->tt_isdst)
daylight = 1;
if (!ttisp->tt_isdst)
timezone = -(ttisp->tt_gmtoff);
-#endif /* defined USG_COMPAT */
-#ifdef ALTZONE
- if (ttisp->tt_isdst)
- altzone = -(ttisp->tt_gmtoff);
-#endif /* defined ALTZONE */
}
/*
** Finally, scrub the abbreviations.
@@ -1274,9 +1257,7 @@ localsub(const time_t *timep, long offse
result = timesub(, ttisp->tt_gmtoff, sp, tmp);
tmp->tm_isdst = ttisp->tt_isdst;
tzname[tmp->tm_isdst] = >chars[ttisp->tt_abbrind];
-#ifdef TM_ZONE
- tmp->TM_ZONE = >chars[ttisp->tt_abbrind];
-#endif /* defined TM_ZONE */
+ tmp->tm_zone = >chars[ttisp->tt_abbrind];
return result;
}
@@ -1325,21 +1306,19 @@ gmtsub(const time_t *timep, long offset,
}
_THREAD_PRIVATE_MUTEX_UNLOCK(gmt);
result = timesub(timep, offset, gmtptr, tmp);
-#ifdef TM_ZONE
/*
** Could get fancy here and deliver something such as
** "UTC+" or "UTC-" if offset is non-zero,
** but this is no time for a treasure hunt.
*/
if (offset != 0)
- tmp->TM_ZONE = wildabbr;
+ tmp->tm_zone = wildabbr;
else {
if (gmtptr == NULL)
- tmp->TM_ZONE = (char *)gmt;
+ tmp->tm_zone = (char *)gmt;
else
- tmp->TM_ZONE = gmtptr->chars;
+ tmp->tm_zone = gmtptr->chars;
}
-#endif /* defined TM_ZONE */
return result;
}
@@ -1508,9 +1487,7 @@ timesub(const time_t *timep, long offset
idays -= ip[tmp->tm_mon];
tmp->tm_mday = (int) (idays + 1);
tmp->tm_isdst = 0;
-#ifdef TM_GMTOFF
- tmp->TM_GMTOFF = offset;
-#endif /* defined TM_GMTOFF */
+ tmp->tm_gmtoff = offset;
return tmp;
}
Index: private.h
===
RCS file: /cvs/src/lib/libc/time/private.h,v
retrieving revision 1.38
diff -u -p -r1.38 private.h
--- private.h 24 Oct 2015 18:13:18 - 1.38
+++ private.h 10 May 2019 22:17:14 -
@@ -9,12 +9,9 @@
*/
/* OpenBSD defaults */
-#define TM_GMTOFF tm_gmtoff
-#define TM_ZONEtm_zone
#define PCTS 1
#define ALL_STATE 1
#define STD_INSPIRED 1
-#define USG_COMPAT 1
/*
** This header is for use ONLY with the time conversion code.
Index: strftime.c
===
RCS file: /cvs/src/lib/libc/time/strftime.c,v
retrieving revision 1.30
diff -u -p -r1.30 strftime.c
--- strftime.c 21 Sep 2016 04:38:57 - 1.30
+++ strftime.c 10 May 2019 22:17:14 -
@@ -456,12 +456,9 @@ label:
Re: [patch] improve strptime(3) %z timezone parsing
On Fri, 10 May 2019 16:52:35 +0200, Ingo Schwarze wrote: > I failed to find any users that do *not* expect seconds. > So my conclusion is that the documentation is right and > what the code in strptime.c does is wrong. Yes, tm_gmtoff is in seconds. > Here is a patch to fix the code. OK millert@ for that part. > The change to %Z is exactly what Hiltjo sent. > The current code for %z is unnecessarily complicated. > Rather than fixing it, i simply rewrote it from scratch. > I like it when a bugfix results in -28 +11 LOC and better readability. I don't believe this handles the "[+-]hh" form. - todd
Re: [patch] improve strptime(3) %z timezone parsing
Ingo Schwarze wrote: > Ouch. No, it does not. Thanks for spotting the regression. > > The following patch preserves the parsing behaviour > and correctly stores the number of seconds into tm_gmtoff. oops, missed that case too. this looks correct.
Re: [patch] improve strptime(3) %z timezone parsing
Hi Todd,
Todd C. Miller wrote on Fri, May 10, 2019 at 02:08:45PM -0600:
> On Fri, 10 May 2019 16:52:35 +0200, Ingo Schwarze wrote:
>> Here is a patch to fix the code.
> OK millert@ for that part.
Thanks, committed.
>> The change to %Z is exactly what Hiltjo sent.
>> The current code for %z is unnecessarily complicated.
>> Rather than fixing it, i simply rewrote it from scratch.
>> I like it when a bugfix results in -28 +11 LOC and better readability.
> I don't believe this handles the "[+-]hh" form.
Ouch. No, it does not. Thanks for spotting the regression.
The following patch preserves the parsing behaviour
and correctly stores the number of seconds into tm_gmtoff.
OK?
Ingo
$ ./z +1
NULL
$ ./z +02
7200
$ ./z +02x
7200 (rest "x")
$ ./z +02:
7200
$ ./z +02:x
7200 (rest "x")
$ ./z +02:1
NULL
$ ./z +02:01
7260
$ ./z +02:013
7260 (rest "3")
Index: strptime.c
===
RCS file: /cvs/src/lib/libc/time/strptime.c,v
retrieving revision 1.27
diff -u -p -r1.27 strptime.c
--- strptime.c 10 May 2019 20:24:58 - 1.27
+++ strptime.c 10 May 2019 20:43:35 -
@@ -519,32 +519,17 @@ literal:
}
return NULL;
}
- offs = 0;
- for (i = 0; i < 4; ) {
- if (isdigit(*bp)) {
- offs = offs * 10 + (*bp++ - '0');
- i++;
- continue;
- }
- if (i == 2 && *bp == ':') {
- bp++;
- continue;
- }
- break;
- }
- switch (i) {
- case 2:
- offs *= 100;
- break;
- case 4:
- i = offs % 100;
- if (i >= 60)
- return NULL;
- /* Convert minutes into decimal */
- offs = (offs / 100) * 100 + (i * 50) / 30;
- break;
- default:
+ if (!isdigit(bp[0]) || !isdigit(bp[1]))
return NULL;
+ offs = ((bp[0]-'0') * 10 + (bp[1]-'0')) * SECSPERHOUR;
+ bp += 2;
+ if (*bp == ':')
+ bp++;
+ if (isdigit(*bp)) {
+ offs += (*bp++ - '0') * 10 * SECSPERMIN;
+ if (!isdigit(*bp))
+ return NULL;
+ offs += (*bp++ - '0') * SECSPERMIN;
}
if (neg)
offs = -offs;
Re: [patch] improve strptime(3) %z timezone parsing
On Sat, 11 May 2019 00:30:35 +0200, Ingo Schwarze wrote: > While here, i noticed ugly preprocessor macros. > Let's make our future life easier by unifdefing a bit. > When compiling with -g0, there is no object change. No objection, OK millert@ - todd
Proper prototype for upgrade() in boot code
See subject -- :wq Claudio Index: cmd.h === RCS file: /cvs/src/sys/stand/boot/cmd.h,v retrieving revision 1.17 diff -u -p -r1.17 cmd.h --- cmd.h 8 Apr 2019 13:55:46 - 1.17 +++ cmd.h 8 May 2019 23:12:10 - @@ -60,5 +60,5 @@ int read_conf(void); int bootparse(int); void boot(dev_t); -int upgrade(); +int upgrade(void); int docmd(void); /* No longer static: needed by regress test */
fix tcpdump localtime caching
The comment above priv_localtime() says, the obtained localtime (from the
privileged process) is cached for about one minute. However, since the
according if statement compares the wrong variable, the caching doesn't
happen. This bug is there since the very first file version (from 15+
years ago).
Regards
Holger
Index: usr.sbin/tcpdump/privsep.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 privsep.c
--- usr.sbin/tcpdump/privsep.c 18 Mar 2019 00:09:22 - 1.53
+++ usr.sbin/tcpdump/privsep.c 10 May 2019 13:17:42 -
@@ -727,7 +727,7 @@ priv_localtime(const time_t *t)
static struct tm *gt = NULL;
static char zone[PATH_MAX];
- if (gt != NULL) {
+ if (t != NULL) {
gt = gmtime(t);
gt0.tm_sec = gt->tm_sec;
gt0.tm_zone = gt->tm_zone;
ssl(8), fix text about web browsers and SAN
it's standard behaviour for web browsers to not use hostnames in Subject at all but require SAN. current ssl(8) text suggests "some new" and "deprecated" rather than "stopped supporting". comments/ok? Index: ssl.8 === RCS file: /cvs/src/share/man/man8/ssl.8,v retrieving revision 1.67 diff -u -p -r1.67 ssl.8 --- ssl.8 25 Mar 2019 18:36:58 - 1.67 +++ ssl.8 10 May 2019 11:48:41 - @@ -94,9 +94,9 @@ You can also sign the key yourself, usin -out /etc/ssl/server.crt .Ed .Pp -Note that some new browsers have deprecated using the common name of a -certificate and require that subject alt names are provided. -This may require the use of +Note that standard web browsers do not use the common name of a subject, +but instead require that subject alt names are provided. +This requires the use of .Ar -extfile Pa server.ext when self-signing. .Bd -literal -offset indent
Re: ssl(8), fix text about web browsers and SAN
I was just stumbling over this as well when I did the relayd: SNI diff. OK reyk On Fri, May 10, 2019 at 1:50 PM Stuart Henderson wrote: > it's standard behaviour for web browsers to not use hostnames in > Subject at all but require SAN. current ssl(8) text suggests "some new" > and "deprecated" rather than "stopped supporting". > > comments/ok? > > > Index: ssl.8 > === > RCS file: /cvs/src/share/man/man8/ssl.8,v > retrieving revision 1.67 > diff -u -p -r1.67 ssl.8 > --- ssl.8 25 Mar 2019 18:36:58 - 1.67 > +++ ssl.8 10 May 2019 11:48:41 - > @@ -94,9 +94,9 @@ You can also sign the key yourself, usin >-out /etc/ssl/server.crt > .Ed > .Pp > -Note that some new browsers have deprecated using the common name of a > -certificate and require that subject alt names are provided. > -This may require the use of > +Note that standard web browsers do not use the common name of a subject, > +but instead require that subject alt names are provided. > +This requires the use of > .Ar -extfile Pa server.ext > when self-signing. > .Bd -literal -offset indent > >
Re: ssl(8), fix text about web browsers and SAN
Stuart Henderson wrote: > it's standard behaviour for web browsers to not use hostnames in > Subject at all but require SAN. current ssl(8) text suggests "some new" > and "deprecated" rather than "stopped supporting". > > comments/ok? I was trying to avoid argument "but my browser still works" :) but I agree this wording is closer to reality. ok.
Re: SMR lists for bridge(4)
On 08/05/19(Wed) 17:09, Martin Pieuchot wrote:
> Diff below pushes the KERNEL_LOCK() further down into bridge(4).
>
> With it bridge_enqueue() now only takes the lock for rules. Rules could
> easily be protected by a mutex but I wanted to keep this change small.
>
> The list of interface and span interfaces are now using SMR list.
> Removing a `bif' from such list now calls smr_barrier() before tearing
> down the object and freeing the memory. That means we might call
> if_enqueue() on an interface while it is being removed/destroyed.
> This is OK because if_detach() will purge the queue.
>
> If my use of SMR list is correct I'll convert bridge_input() next.
>
> Tests, reviews and oks welcome!
Updated diff including some feedbacks from visa@.
Index: net/bridgectl.c
===
RCS file: /cvs/src/sys/net/bridgectl.c,v
retrieving revision 1.18
diff -u -p -r1.18 bridgectl.c
--- net/bridgectl.c 28 Apr 2019 22:15:57 - 1.18
+++ net/bridgectl.c 10 May 2019 14:41:17 -
@@ -723,8 +723,12 @@ u_int8_t
bridge_filterrule(struct brl_head *h, struct ether_header *eh, struct mbuf *m)
{
struct brl_node *n;
- u_int8_t flags;
+ u_int8_t action, flags;
+ if (SIMPLEQ_EMPTY(h))
+ return (BRL_ACTION_PASS);
+
+ KERNEL_LOCK();
SIMPLEQ_FOREACH(n, h, brl_next) {
if (!bridge_arpfilter(n, eh, m))
continue;
@@ -753,13 +757,16 @@ bridge_filterrule(struct brl_head *h, st
goto return_action;
}
}
+ KERNEL_UNLOCK();
return (BRL_ACTION_PASS);
return_action:
#if NPF > 0
pf_tag_packet(m, n->brl_tag, -1);
#endif
- return (n->brl_action);
+ action = n->brl_action;
+ KERNEL_UNLOCK();
+ return (action);
}
int
@@ -781,6 +788,9 @@ bridge_addrule(struct bridge_iflist *bif
else
n->brl_tag = 0;
#endif
+
+ KERNEL_ASSERT_LOCKED();
+
if (out) {
n->brl_flags &= ~BRL_FLAG_IN;
n->brl_flags |= BRL_FLAG_OUT;
@@ -797,6 +807,8 @@ void
bridge_flushrule(struct bridge_iflist *bif)
{
struct brl_node *p;
+
+ KERNEL_ASSERT_LOCKED();
while (!SIMPLEQ_EMPTY(>bif_brlin)) {
p = SIMPLEQ_FIRST(>bif_brlin);
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.330
diff -u -p -r1.330 if_bridge.c
--- net/if_bridge.c 10 May 2019 12:41:30 - 1.330
+++ net/if_bridge.c 10 May 2019 14:49:05 -
@@ -137,7 +137,6 @@ int bridge_ipsec(struct ifnet *, struct
#endif
int bridge_clone_create(struct if_clone *, int);
intbridge_clone_destroy(struct ifnet *);
-intbridge_delete(struct bridge_softc *, struct bridge_iflist *);
#defineETHERADDR_IS_IP_MCAST(a) \
/* struct etheraddr *a; */ \
@@ -173,8 +172,8 @@ bridge_clone_create(struct if_clone *ifc
sc->sc_brtmax = BRIDGE_RTABLE_MAX;
sc->sc_brttimeout = BRIDGE_RTABLE_TIMEOUT;
timeout_set(>sc_brtimeout, bridge_rtage, sc);
- SLIST_INIT(>sc_iflist);
- SLIST_INIT(>sc_spanlist);
+ SMR_SLIST_INIT(>sc_iflist);
+ SMR_SLIST_INIT(>sc_spanlist);
mtx_init(>sc_mtx, IPL_MPFLOOR);
for (i = 0; i < BRIDGE_RTABLE_SIZE; i++)
LIST_INIT(>sc_rts[i]);
@@ -220,9 +219,9 @@ bridge_clone_destroy(struct ifnet *ifp)
bridge_stop(sc);
bridge_rtflush(sc, IFBF_FLUSHALL);
- while ((bif = SLIST_FIRST(>sc_iflist)) != NULL)
+ while ((bif = SMR_SLIST_FIRST_LOCKED(>sc_iflist)) != NULL)
bridge_ifremove(bif);
- while ((bif = SLIST_FIRST(>sc_spanlist)) != NULL)
+ while ((bif = SMR_SLIST_FIRST_LOCKED(>sc_spanlist)) != NULL)
bridge_spanremove(bif);
bstp_destroy(sc->sc_stp);
@@ -241,26 +240,6 @@ bridge_clone_destroy(struct ifnet *ifp)
}
int
-bridge_delete(struct bridge_softc *sc, struct bridge_iflist *bif)
-{
- int error;
-
- if (bif->bif_flags & IFBIF_STP)
- bstp_delete(bif->bif_stp);
-
- bif->ifp->if_bridgeidx = 0;
- error = ifpromisc(bif->ifp, 0);
- hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie);
-
- if_ih_remove(bif->ifp, bridge_input, NULL);
- bridge_rtdelete(sc, bif->ifp, 0);
- bridge_flushrule(bif);
- free(bif, M_DEVBUF, sizeof *bif);
-
- return (error);
-}
-
-int
bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
{
struct bridge_softc *sc = (struct bridge_softc *)ifp->if_softc;
@@ -299,7 +278,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
}
/* If it's in the span list, it can't be a member. */
- SLIST_FOREACH(bif, >sc_spanlist, bif_next) {
+ SMR_SLIST_FOREACH_LOCKED(bif, >sc_spanlist,
Re: [patch] improve strptime(3) %z timezone parsing
Ingo Schwarze wrote: > Now let's get to the more serious part. > Hiltjo observed that %Z and %z produce wrong results. > > First of all, neither POSIX nor XPG define tm_gmtoff nor %Z nor %z: > > https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/time.h.html > https://pubs.opengroup.org/onlinepubs/9699919799/functions/strptime.html > > So i think the best way to find out what tm_gmtoff should be is to > understand how programs in our own tree use it. > > Here is a (hopefully) complete list of the users in OpenBSD base. > The following files expect it to contain seconds: > - lib/libc/time/strftime.c > - lib/libc/time/wcsftime.c > - usr.sbin/smtpd/to.c > - usr.sbin/snmpd/mib.c > - usr.sbin/cron/cron.c > - usr.bin/ftp/util.c > - usr.bin/cvs/entries.c > - usr.bin/rcs/rcstime.c > - gnu/usr.bin/perl/time64.c > > I failed to find any users that do *not* expect seconds. > So my conclusion is that the documentation is right and > what the code in strptime.c does is wrong. > > Here is a patch to fix the code. this looks good to me. ok.
Re: [patch] improve strptime(3) %z timezone parsing
Hi Ted,
Ted Unangst wrote on Thu, May 09, 2019 at 04:16:40PM -0400:
> Ingo Schwarze wrote:
>> I'm not mixing anything else into this diff. The other bugs should
>> be handled separately.
> Works for me. (with additional comment removal)
Thanks for checking, committed.
Now let's get to the more serious part.
Hiltjo observed that %Z and %z produce wrong results.
First of all, neither POSIX nor XPG define tm_gmtoff nor %Z nor %z:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/time.h.html
https://pubs.opengroup.org/onlinepubs/9699919799/functions/strptime.html
So i think the best way to find out what tm_gmtoff should be is to
understand how programs in our own tree use it.
Here is a (hopefully) complete list of the users in OpenBSD base.
The following files expect it to contain seconds:
- lib/libc/time/strftime.c
- lib/libc/time/wcsftime.c
- usr.sbin/smtpd/to.c
- usr.sbin/snmpd/mib.c
- usr.sbin/cron/cron.c
- usr.bin/ftp/util.c
- usr.bin/cvs/entries.c
- usr.bin/rcs/rcstime.c
- gnu/usr.bin/perl/time64.c
I failed to find any users that do *not* expect seconds.
So my conclusion is that the documentation is right and
what the code in strptime.c does is wrong.
Here is a patch to fix the code.
OK?
The change to %Z is exactly what Hiltjo sent.
The current code for %z is unnecessarily complicated.
Rather than fixing it, i simply rewrote it from scratch.
I like it when a bugfix results in -28 +11 LOC and better readability.
Yours,
Ingo
Index: strptime.c
===
RCS file: /cvs/src/lib/libc/time/strptime.c,v
retrieving revision 1.26
diff -u -p -r1.26 strptime.c
--- strptime.c 10 May 2019 12:49:16 - 1.26
+++ strptime.c 10 May 2019 14:40:49 -
@@ -497,7 +497,7 @@ literal:
ep = _find_string(bp, , nast, NULL, 4);
if (ep != NULL) {
#ifdef TM_GMTOFF
- tm->TM_GMTOFF = -5 - i;
+ tm->TM_GMTOFF = (-5 - i) * SECSPERHOUR;
#endif
#ifdef TM_ZONE
tm->TM_ZONE = (char *)nast[i];
@@ -509,7 +509,7 @@ literal:
if (ep != NULL) {
tm->tm_isdst = 1;
#ifdef TM_GMTOFF
- tm->TM_GMTOFF = -4 - i;
+ tm->TM_GMTOFF = (-4 - i) * SECSPERHOUR;
#endif
#ifdef TM_ZONE
tm->TM_ZONE = (char *)nadt[i];
@@ -519,33 +519,16 @@ literal:
}
return NULL;
}
- offs = 0;
- for (i = 0; i < 4; ) {
- if (isdigit(*bp)) {
- offs = offs * 10 + (*bp++ - '0');
- i++;
- continue;
- }
- if (i == 2 && *bp == ':') {
- bp++;
- continue;
- }
- break;
- }
- switch (i) {
- case 2:
- offs *= 100;
- break;
- case 4:
- i = offs % 100;
- if (i >= 60)
- return NULL;
- /* Convert minutes into decimal */
- offs = (offs / 100) * 100 + (i * 50) / 30;
- break;
- default:
+ if (!isdigit(bp[0]) || !isdigit(bp[1]))
return NULL;
- }
+ offs = ((bp[0]-'0') * 10 + (bp[1]-'0')) * SECSPERHOUR;
+ bp += 2;
+ if (*bp == ':')
+ bp++;
+ if (!isdigit(bp[0]) || !isdigit(bp[1]))
+ return NULL;
+ offs += ((bp[0]-'0') * 10 + (bp[1]-'0')) * SECSPERMIN;
+ bp += 2;
if (neg)
offs = -offs;
tm->tm_isdst = 0; /* XXX */
Re: Proper prototype for upgrade() in boot code
> Date: Fri, 10 May 2019 14:47:54 +0200 > From: Claudio Jeker > > See subject ok kettenis@ > -- > :wq Claudio > > Index: cmd.h > === > RCS file: /cvs/src/sys/stand/boot/cmd.h,v > retrieving revision 1.17 > diff -u -p -r1.17 cmd.h > --- cmd.h 8 Apr 2019 13:55:46 - 1.17 > +++ cmd.h 8 May 2019 23:12:10 - > @@ -60,5 +60,5 @@ int read_conf(void); > int bootparse(int); > void boot(dev_t); > > -int upgrade(); > +int upgrade(void); > int docmd(void); /* No longer static: needed by regress test */ > >
bgpd refactor UPDATE attribute writer
This change is from a much larger patch I'm working on. This cleans up
up_generate_attr() from a hardcoded implementation to a loop-switch
construct. This way attributes are always dumped in ascending order as
suggested by the RFC and adding special attributes is simpler than in the
current way. There is an exception whit the MP attributes because those
are added at a later point.
Works for me(tm)
--
:wq Claudio
Index: rde_update.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
retrieving revision 1.108
diff -u -p -r1.108 rde_update.c
--- rde_update.c21 Jan 2019 02:07:56 - 1.108
+++ rde_update.c10 May 2019 11:51:28 -
@@ -295,96 +295,155 @@ up_generate_attr(u_char *buf, int len, s
struct filterstate *state, u_int8_t aid)
{
struct rde_aspath *asp = >aspath;
- struct attr *oa, *newaggr = NULL;
+ struct attr *oa = NULL, *newaggr = NULL;
u_char *pdata;
u_int32_ttmp32;
in_addr_tnexthop;
int flags, r, neednewpath = 0;
u_int16_twlen = 0, plen;
- u_int8_t l;
- u_int16_tnlen = 0;
- u_char *ndata;
-
- /* origin */
- if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
- ATTR_ORIGIN, >origin, 1)) == -1)
- return (-1);
- wlen += r; len -= r;
-
- /* aspath */
- if (!peer->conf.ebgp ||
- peer->conf.flags & PEERFLAG_TRANS_AS)
- pdata = aspath_prepend(asp->aspath, peer->conf.local_as, 0,
- );
- else
- pdata = aspath_prepend(asp->aspath, peer->conf.local_as, 1,
- );
-
- if (!rde_as4byte(peer))
- pdata = aspath_deflate(pdata, , );
-
- if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
- ATTR_ASPATH, pdata, plen)) == -1)
- return (-1);
- wlen += r; len -= r;
- free(pdata);
-
- switch (aid) {
- case AID_INET:
- nexthop = up_get_nexthop(peer, state);
- if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
- ATTR_NEXTHOP, , 4)) == -1)
- return (-1);
- wlen += r; len -= r;
- break;
- default:
- break;
- }
-
- /*
-* The old MED from other peers MUST not be announced to others
-* unless the MED is originating from us or the peer is an IBGP one.
-* Only exception are routers with "transparent-as yes" set.
-*/
- if (asp->flags & F_ATTR_MED && (!peer->conf.ebgp ||
- asp->flags & F_ATTR_MED_ANNOUNCE ||
- peer->conf.flags & PEERFLAG_TRANS_AS)) {
- tmp32 = htonl(asp->med);
- if ((r = attr_write(buf + wlen, len, ATTR_OPTIONAL,
- ATTR_MED, , 4)) == -1)
- return (-1);
- wlen += r; len -= r;
- }
-
- if (!peer->conf.ebgp) {
- /* local preference, only valid for ibgp */
- tmp32 = htonl(asp->lpref);
- if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
- ATTR_LOCALPREF, , 4)) == -1)
- return (-1);
- wlen += r; len -= r;
- }
-
- /*
-* dump all other path attributes. Following rules apply:
-* 1. well-known attrs: ATTR_ATOMIC_AGGREGATE and ATTR_AGGREGATOR
-* pass unmodified (enforce flags to correct values)
-* Actually ATTR_AGGREGATOR may be deflated for OLD 2-byte peers.
-* 2. non-transitive attrs: don't re-announce to ebgp peers
-* 3. transitive known attrs: announce unmodified
-* 4. transitive unknown attrs: set partial bit and re-announce
-*/
- for (l = 0; l < asp->others_len; l++) {
- if ((oa = asp->others[l]) == NULL)
+ u_int8_t oalen = 0, type;
+
+ if (asp->others_len > 0)
+ oa = asp->others[oalen++];
+
+ /* dump attributes in ascending order */
+ for (type = ATTR_ORIGIN; type < 255; type++) {
+ r = 0;
+
+ while (oa && oa->type < type) {
+ if (oalen < asp->others_len)
+ oa = asp->others[oalen++];
+ else
+ oa = NULL;
+ }
+
+ switch (type) {
+ /*
+* Attributes stored in rde_aspath
+*/
+ case ATTR_ORIGIN:
+ if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
+ ATTR_ORIGIN, >origin, 1)) == -1)
+ return (-1);
break;
- switch (oa->type) {
+ case ATTR_ASPATH:
+ if (!peer->conf.ebgp ||
switch(4): port protection man page updates
Hi, These are the manpage updates to go with the port protection diffs for switch(4). Thanks, Ayaka Index: sbin/ifconfig/ifconfig.8 === RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v retrieving revision 1.334 diff -u -p -u -r1.334 ifconfig.8 --- sbin/ifconfig/ifconfig.819 Apr 2019 04:30:57 - 1.334 +++ sbin/ifconfig/ifconfig.810 May 2019 16:02:29 - @@ -1626,6 +1626,20 @@ Set the port number for the port named .Ar interface . The default value is the interface index of the .Ar interface . +.It Cm protected Ar interface ids +Put +.Ar interface +in protected domains. +.Ar ids +is a comma delimited list of domain IDs, between 1 and 31, to put the +interface in. +Interfaces that are part of a protected domain cannot forward traffic to any +other interface in that domain. +Interfaces do not belong to any protected domain by default. +.It Cm -protected Ar interface +Remove +.Ar interface +from all protected domains. .It Cm up Start the switch processing packets. .El Index: share/man/man4/bridge.4 === RCS file: /cvs/src/share/man/man4/bridge.4,v retrieving revision 1.76 diff -u -p -u -r1.76 bridge.4 --- share/man/man4/bridge.4 24 Oct 2017 09:36:13 - 1.76 +++ share/man/man4/bridge.4 10 May 2019 16:02:30 - @@ -561,6 +561,11 @@ to the value in The value in .Va ifbr_path_cost must be greater than or equal to one. +.It Dv SIOCBRDGSIFPROT Fa "struct ifbreq *" +Set the protection domain membership of the interface named in +.Va ifbr_ifsname +to the value in +.Va ifbr_protected . .El .Sh ERRORS If the Index: share/man/man4/switch.4 === RCS file: /cvs/src/share/man/man4/switch.4,v retrieving revision 1.8 diff -u -p -u -r1.8 switch.4 --- share/man/man4/switch.4 24 Oct 2017 09:36:13 - 1.8 +++ share/man/man4/switch.4 10 May 2019 16:02:30 - @@ -92,6 +92,8 @@ and .Dv SIOCBRDGDEL .It .Dv SIOCBRDGGIFFLGS +.It +.Dv SIOCBRDGSIFPROT .El .Pp The following
Re: switch(4): port protection man page updates
On Fri, May 10, 2019 at 09:18:06AM -0700, Ayaka Koshibe wrote: > These are the manpage updates to go with the port protection diffs for > switch(4). Wording is identical to that in ifconfig(8), fine with me. Thanks for taking care of bridge(4) as well. OK kn
