Re: crontab: support random offsets for steps
There appears to be a strong preference for a syntax like "~/10"
instead of "*/~10". The following diff implements that instead and
also supports random ranges like "1~58/10". When a high and/or low
value is used with a random range, the initial offset is a random
value less than eiter the step value or the difference of the high
and low values (whichever is smaller).
- todd
Index: usr.sbin/cron/crontab.5
===
RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
retrieving revision 1.41
diff -u -p -u -r1.41 crontab.5
--- usr.sbin/cron/crontab.5 18 Apr 2020 17:11:40 - 1.41
+++ usr.sbin/cron/crontab.5 6 May 2023 02:19:40 -
@@ -157,8 +157,7 @@ If either (or both) of the numbers on ei
.Ql ~
are omitted, the appropriate limit (low or high) for the field will be used.
.Pp
-Step values can be used in conjunction with ranges (but not random ranges
-which represent a single number).
+Step values can be used in conjunction with ranges.
Following a range with
.No / Ns Ar number
specifies skips of
@@ -173,6 +172,18 @@ Steps are also permitted after an asteri
.Dq every two hours ,
just use
.Dq */2 .
+A step value after a random range will execute the command at a random
+offset less than the step size.
+For example, to avoid a thundering herd at the top and bottom of the hour,
+.Dq 0~59/30
+.Po
+or simply
+.Dq ~/30
+.Pc
+can be used in the
+.Ar minute
+field to specify that command execution happen twice an hour at
+consistent intervals.
.Pp
An asterisk
.Pq Ql *
Index: usr.sbin/cron/entry.c
===
RCS file: /cvs/src/usr.sbin/cron/entry.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 entry.c
--- usr.sbin/cron/entry.c 21 May 2022 01:21:29 - 1.53
+++ usr.sbin/cron/entry.c 5 May 2023 21:39:30 -
@@ -456,10 +456,11 @@ get_range(bitstr_t *bits, int low, int h
/* range = number | number* "~" number* | number "-" number ["/" number]
*/
- int i, num1, num2, num3;
+ int i, num1, num2, num3, rndstep;
num1 = low;
num2 = high;
+ rndstep = 0;
if (ch == '*') {
/* '*' means [low, high] but can still be modified by /step
@@ -497,7 +498,7 @@ get_range(bitstr_t *bits, int low, int h
/* get the (optional) number following the tilde
*/
- ch = get_number(&num2, low, names, ch, file, ", \t\n");
+ ch = get_number(&num2, low, names, ch, file, "/, \t\n");
if (ch == EOF)
ch = get_char(file);
if (ch == EOF || num1 > num2) {
@@ -505,6 +506,13 @@ get_range(bitstr_t *bits, int low, int h
return (EOF);
}
+ if (ch == '/') {
+ /* randomize the step value instead of num1
+*/
+ rndstep = 1;
+ break;
+ }
+
/* get a random number in the interval [num1, num2]
*/
num3 = num1;
@@ -538,6 +546,13 @@ get_range(bitstr_t *bits, int low, int h
ch = get_number(&num3, 0, NULL, ch, file, ", \t\n");
if (ch == EOF || num3 == 0)
return (EOF);
+ if (rndstep) {
+ /*
+* use a random offset smaller than the step size
+* and the difference between high and low values.
+*/
+ num1 += arc4random_uniform(MINIMUM(num3, num2 - num1));
+ }
} else {
/* no step. default==1.
*/
Index: usr.sbin/cron/macros.h
===
RCS file: /cvs/src/usr.sbin/cron/macros.h,v
retrieving revision 1.15
diff -u -p -u -r1.15 macros.h
--- usr.sbin/cron/macros.h 12 Nov 2015 21:12:05 - 1.15
+++ usr.sbin/cron/macros.h 5 May 2023 21:38:19 -
@@ -29,6 +29,8 @@
#defineMAX_TEMPSTR 100 /* obvious */
#defineMAX_UNAME (_PW_NAME_LEN+1)/* max length of
username, should be overkill */
+#defineMINIMUM(a, b) (((a) < (b)) ? (a) : (b))
+
#defineSkip_Blanks(c, f) \
while (c == '\t' || c == ' ') \
c = get_char(f);
Re: cron: better error checking of random values
On 23-05-05 10:41AM, Todd C. Miller wrote: > On Fri, 05 May 2023 16:13:01 +1000, Mark Jamsek wrote: > > > I found kn's attempted syntax intuitive though; it feels like a natural > > extension of the existing random and step syntax. I also assumed ~/15 > > would run every 15 minutes starting with a random minute, and since > > discovering it didn't work like that, I've been carrying a simple patch > > that allows kn's syntax: > > > > ~/15 random 15 minute intervals in [0, 59] > > 1~9/10random 10 minute intervals in [1,59] > > It does seem like people prefer the ~/step syntax over my own, which > is fine. > > However, I'm unsure about the x~y/step syntax. Personally, I think > it would be more natural to treat x~y/step like x-y/step and just > use a random offset in the interval [0,step-1]. Do you have a > use-case for the way your diff handles this or did it just follow > naturally from the random value being in num1? > > Also, using the % operator with the random value results in modulo > bias, which we would like to avoid. If we use a random offset based > on the step interval instead there is no need for the modulus. Both tbh. I primarily wanted to get intervals that would not fall on common times where I already had jobs running; namely, 0th minutes (e.g., 0, 30). And I described the second example wrong, so this was most likely lost in my first response: > > 1~9/10random 10 minute intervals in [1,59] The "intervals in [1,59]" is not correct in this example. Rather, this will result in an initial random value 1 <= n <= 9, so each interval in this step /10 case will also fall in [N1,N9] (e.g., 7, 17, 27, ...) and avoid said common times. And the modulo is used to get the first point in [low,high] from which to start the step loop that fit this constraint. In other words, I wanted some control over where the random intervals landed such that being able to avoid ranges was as much a requirement, in my use case, as random steps. But I also didn't want to change the code too much so it just followed naturally. This could be done other ways, too, and I'm not presuming such a requirement is as important to anyone else. -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
fragment code cleanup for tso
Hi,
I preparation for my TSO in software diff, I would like to cleanup
the fragment code. Both are very simmilar and I like consistency.
- Use if_output_ml() to send mbuf lists to interfaces. This can
be used for TSO, fragments, ARP and ND6.
- Rename variable fml to ml. It will soon be used for TCP segments,
no need to have the f for fragment in the name.
- In pf_route6() split the if else block, then tcp_chopper() can
be easily put in there in the next diff.
- In ip_fragment() the if (hlen + firstlen < tlen) is a new safety
check. It makes the code correct for the case where the packet
was to short to be fragmented. This should not happen, but the
other functions also have this logic.
No functional change intended.
ok?
bluhm
Index: net/if.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
retrieving revision 1.694
diff -u -p -r1.694 if.c
--- net/if.c26 Apr 2023 19:54:35 - 1.694
+++ net/if.c5 May 2023 20:27:06 -
@@ -762,27 +762,6 @@ if_enqueue_ifq(struct ifnet *ifp, struct
}
void
-if_mqoutput(struct ifnet *ifp, struct mbuf_queue *mq, unsigned int *total,
-struct sockaddr *dst, struct rtentry *rt)
-{
- struct mbuf_list ml;
- struct mbuf *m;
- unsigned int len;
-
- mq_delist(mq, &ml);
- len = ml_len(&ml);
- while ((m = ml_dequeue(&ml)) != NULL)
- ifp->if_output(ifp, m, rt_key(rt), rt);
-
- /* XXXSMP we also discard if other CPU enqueues */
- if (mq_len(mq) > 0) {
- /* mbuf is back in queue. Discard. */
- atomic_sub_int(total, len + mq_purge(mq));
- } else
- atomic_sub_int(total, len);
-}
-
-void
if_input(struct ifnet *ifp, struct mbuf_list *ml)
{
ifiq_input(&ifp->if_rcv, ml);
@@ -841,6 +820,46 @@ if_input_local(struct ifnet *ifp, struct
}
return (0);
+}
+
+int
+if_output_ml(struct ifnet *ifp, struct mbuf_list *ml,
+struct sockaddr *dst, struct rtentry *rt)
+{
+ struct mbuf *m;
+ int error = 0;
+
+ while ((m = ml_dequeue(ml)) != NULL) {
+ error = ifp->if_output(ifp, m, dst, rt);
+ if (error)
+ break;
+ }
+ if (error)
+ ml_purge(ml);
+
+ return error;
+}
+
+int
+if_output_mq(struct ifnet *ifp, struct mbuf_queue *mq, unsigned int *total,
+struct sockaddr *dst, struct rtentry *rt)
+{
+ struct mbuf_list ml;
+ unsigned int len;
+ int error;
+
+ mq_delist(mq, &ml);
+ len = ml_len(&ml);
+ error = if_output_ml(ifp, &ml, dst, rt);
+
+ /* XXXSMP we also discard if other CPU enqueues */
+ if (mq_len(mq) > 0) {
+ /* mbuf is back in queue. Discard. */
+ atomic_sub_int(total, len + mq_purge(mq));
+ } else
+ atomic_sub_int(total, len);
+
+ return error;
}
int
Index: net/if_bridge.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.365
diff -u -p -r1.365 if_bridge.c
--- net/if_bridge.c 27 Feb 2023 09:35:32 - 1.365
+++ net/if_bridge.c 5 May 2023 20:38:22 -
@@ -1826,7 +1826,7 @@ bridge_fragment(struct ifnet *brifp, str
struct mbuf *m)
{
struct llc llc;
- struct mbuf_list fml;
+ struct mbuf_list ml;
int error = 0;
int hassnap = 0;
u_int16_t etype;
@@ -1884,11 +1884,11 @@ bridge_fragment(struct ifnet *brifp, str
return;
}
- error = ip_fragment(m, &fml, ifp, ifp->if_mtu);
+ error = ip_fragment(m, &ml, ifp, ifp->if_mtu);
if (error)
return;
- while ((m = ml_dequeue(&fml)) != NULL) {
+ while ((m = ml_dequeue(&ml)) != NULL) {
if (hassnap) {
M_PREPEND(m, LLC_SNAPFRAMELEN, M_DONTWAIT);
if (m == NULL) {
@@ -1908,7 +1908,7 @@ bridge_fragment(struct ifnet *brifp, str
break;
}
if (error)
- ml_purge(&fml);
+ ml_purge(&ml);
else
ipstat_inc(ips_fragmented);
Index: net/if_var.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_var.h,v
retrieving revision 1.125
diff -u -p -r1.125 if_var.h
--- net/if_var.h18 Apr 2023 22:01:24 - 1.125
+++ net/if_var.h5 May 2023 21:07:16 -
@@ -321,12 +321,14 @@ extern struct ifnet_head ifnetlist;
void if_start(struct ifnet *);
intif_enqueue(struct ifnet *, struct mbuf *);
intif_enqueue_ifq(struct ifnet *, struct mbuf *);
-void if_mqoutput(struct ifnet *, struct mbuf_queue *, unsigned int *,
- struct sockaddr *, struct rtentry *);
void if_input(struct ifnet *, struct mbuf_list *);
void if_vinput(struct ifnet *, struct mbuf
Re: passwd: fix error paths and undefined behaviour
On Fri, May 05, 2023 at 11:00:12AM -0600, Todd C. Miller wrote:
> This looks OK but I'd like to see an error message if waitpid()
> really does fail. How about something like this, which also avoid
> needing the extra variable?
Yes, looks much better!
Index: local_passwd.c
===
RCS file: /cvs/src/usr.bin/passwd/local_passwd.c,v
retrieving revision 1.63
diff -u -p -u -p -r1.63 local_passwd.c
--- local_passwd.c 10 Feb 2022 13:06:46 - 1.63
+++ local_passwd.c 5 May 2023 17:03:43 -
@@ -217,7 +217,7 @@ getnewpasswd(struct passwd *pw, login_ca
continue;
}
- if ((tries++ < pwd_tries || pwd_tries == 0) &&
+ if ((pwd_tries == 0 || tries++ < pwd_tries) &&
pwd_check(lc, p) == 0)
continue;
p = readpassphrase("Retype new password:", repeat,
sizeof(repeat),
Index: pwd_check.c
===
RCS file: /cvs/src/usr.bin/passwd/pwd_check.c,v
retrieving revision 1.17
diff -u -p -u -p -r1.17 pwd_check.c
--- pwd_check.c 28 Aug 2021 06:46:49 - 1.17
+++ pwd_check.c 5 May 2023 17:03:43 -
@@ -114,6 +114,8 @@ pwd_check(login_cap_t *lc, char *passwor
switch (child = fork()) {
case -1:
warn("fork");
+ close(pipefds[0]);
+ close(pipefds[1]);
goto out;
case 0:
(void)signal(SIGINT, SIG_DFL);
@@ -184,8 +186,10 @@ pwd_check(login_cap_t *lc, char *passwor
/* get the return value from the child */
while (waitpid(child, &res, 0) == -1) {
- if (errno != EINTR)
- break;
+ if (errno != EINTR) {
+ warn("waitpid");
+ goto out;
+ }
}
if (WIFEXITED(res) && WEXITSTATUS(res) == 0) {
free(checker);
Re: passwd: fix error paths and undefined behaviour
On Fri, 05 May 2023 17:05:05 -, Tobias Stoeckmann wrote: > On Fri, May 05, 2023 at 11:00:12AM -0600, Todd C. Miller wrote: > > This looks OK but I'd like to see an error message if waitpid() > > really does fail. How about something like this, which also avoid > > needing the extra variable? > > Yes, looks much better! OK millert@ - todd
Re: passwd: fix error paths and undefined behaviour
On Fri, 05 May 2023 16:46:51 -, Tobias Stoeckmann wrote:
> In getnewpasswd we increment "tries" every time we try to enter a new
> password. The code allows this to be repeated endlessly by defining
> passwordtries to be 0 in /etc/login.conf. But unfortunately we even
> increment the int "tries" if pwd_tries is 0, which eventually would lead
> to a signed integer overflow (and we would stop asking for passwords
> after billion times).
>
> In pwd_check we should close pipe file descriptors if fork fails. Also
> it might be possible that waitpid fails if the root user tries to
> sabotage the own passwd call. Let's just handle the error case here
> as well to avoid accessing undefined content of "res".
This looks OK but I'd like to see an error message if waitpid()
really does fail. How about something like this, which also avoid
needing the extra variable?
- todd
Index: /usr/src/usr.bin/passwd/pwd_check.c
===
RCS file: /cvs/src/usr.bin/passwd/pwd_check.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 pwd_check.c
--- /usr/src/usr.bin/passwd/pwd_check.c 28 Aug 2021 06:46:49 - 1.17
+++ /usr/src/usr.bin/passwd/pwd_check.c 5 May 2023 16:59:20 -
@@ -184,8 +184,10 @@ pwd_check(login_cap_t *lc, char *passwor
/* get the return value from the child */
while (waitpid(child, &res, 0) == -1) {
- if (errno != EINTR)
- break;
+ if (errno != EINTR) {
+ warn("waitpid");
+ goto out;
+ }
}
if (WIFEXITED(res) && WEXITSTATUS(res) == 0) {
free(checker);
passwd: fix error paths and undefined behaviour
Hi,
this patch fixes error paths and an undefined behaviour:
In getnewpasswd we increment "tries" every time we try to enter a new
password. The code allows this to be repeated endlessly by defining
passwordtries to be 0 in /etc/login.conf. But unfortunately we even
increment the int "tries" if pwd_tries is 0, which eventually would lead
to a signed integer overflow (and we would stop asking for passwords
after billion times).
In pwd_check we should close pipe file descriptors if fork fails. Also
it might be possible that waitpid fails if the root user tries to
sabotage the own passwd call. Let's just handle the error case here
as well to avoid accessing undefined content of "res".
Okay?
Tobias
Index: local_passwd.c
===
RCS file: /cvs/src/usr.bin/passwd/local_passwd.c,v
retrieving revision 1.63
diff -u -p -u -p -r1.63 local_passwd.c
--- local_passwd.c 10 Feb 2022 13:06:46 - 1.63
+++ local_passwd.c 5 May 2023 16:35:58 -
@@ -217,7 +217,7 @@ getnewpasswd(struct passwd *pw, login_ca
continue;
}
- if ((tries++ < pwd_tries || pwd_tries == 0) &&
+ if ((pwd_tries == 0 || tries++ < pwd_tries) &&
pwd_check(lc, p) == 0)
continue;
p = readpassphrase("Retype new password:", repeat,
sizeof(repeat),
Index: pwd_check.c
===
RCS file: /cvs/src/usr.bin/passwd/pwd_check.c,v
retrieving revision 1.17
diff -u -p -u -p -r1.17 pwd_check.c
--- pwd_check.c 28 Aug 2021 06:46:49 - 1.17
+++ pwd_check.c 5 May 2023 16:35:58 -
@@ -91,7 +91,7 @@ pwd_check(login_cap_t *lc, char *passwor
char *checker;
char *argp[] = { "sh", "-c", NULL, NULL};
int pipefds[2];
- pid_t child;
+ pid_t child, c;
uid_t uid;
gid_t gid;
@@ -114,6 +114,8 @@ pwd_check(login_cap_t *lc, char *passwor
switch (child = fork()) {
case -1:
warn("fork");
+ close(pipefds[0]);
+ close(pipefds[1]);
goto out;
case 0:
(void)signal(SIGINT, SIG_DFL);
@@ -183,11 +185,11 @@ pwd_check(login_cap_t *lc, char *passwor
}
/* get the return value from the child */
- while (waitpid(child, &res, 0) == -1) {
+ while ((c = waitpid(child, &res, 0)) == -1) {
if (errno != EINTR)
break;
}
- if (WIFEXITED(res) && WEXITSTATUS(res) == 0) {
+ if (c == child && WIFEXITED(res) && WEXITSTATUS(res) == 0) {
free(checker);
return (1);
}
Re: cron: better error checking of random values
On Fri, 05 May 2023 16:13:01 +1000, Mark Jamsek wrote: > I found kn's attempted syntax intuitive though; it feels like a natural > extension of the existing random and step syntax. I also assumed ~/15 > would run every 15 minutes starting with a random minute, and since > discovering it didn't work like that, I've been carrying a simple patch > that allows kn's syntax: > > ~/15random 15 minute intervals in [0, 59] > 1~9/10 random 10 minute intervals in [1,59] It does seem like people prefer the ~/step syntax over my own, which is fine. However, I'm unsure about the x~y/step syntax. Personally, I think it would be more natural to treat x~y/step like x-y/step and just use a random offset in the interval [0,step-1]. Do you have a use-case for the way your diff handles this or did it just follow naturally from the random value being in num1? Also, using the % operator with the random value results in modulo bias, which we would like to avoid. If we use a random offset based on the step interval instead there is no need for the modulus. - todd
Fix makefs(8) for creating ffs2 filesystem images
Hi,
makefs(8) creates an ffs1 filesystem by default, and the generated
filesystem image passes fsck checks. However, when creating an ffs2
filesystem using the -o version=2 option, the generated filesystem image
fails fsck checks.
This patch resolves the issue by ensuring that the superblock parameters
for the ffs2 filesystem match those set by newfs.
---
diff --git a/usr.sbin/makefs/ffs/mkfs.c b/usr.sbin/makefs/ffs/mkfs.c
index a4d57c008a2..6369af6ba4a 100644
--- a/usr.sbin/makefs/ffs/mkfs.c
+++ b/usr.sbin/makefs/ffs/mkfs.c
@@ -222,13 +222,9 @@ ffs_mkfs(const char *fsys, const fsinfo_t
*fsopts, time_t tstamp)
sblock.fs_fmask = ~(sblock.fs_fsize - 1);
sblock.fs_qbmask = ~sblock.fs_bmask;
sblock.fs_qfmask = ~sblock.fs_fmask;
- for (sblock.fs_bshift = 0, i = sblock.fs_bsize; i > 1; i >>= 1)
- sblock.fs_bshift++;
- for (sblock.fs_fshift = 0, i = sblock.fs_fsize; i > 1; i >>= 1)
- sblock.fs_fshift++;
+ sblock.fs_bshift = ilog2(sblock.fs_bsize);
+ sblock.fs_fshift = ilog2(sblock.fs_fsize);
sblock.fs_frag = numfrags(&sblock, sblock.fs_bsize);
- for (sblock.fs_fragshift = 0, i = sblock.fs_frag; i > 1; i >>= 1)
- sblock.fs_fragshift++;
if (sblock.fs_frag > MAXFRAG) {
printf("fragment size %d is too small, "
"minimum with block size %d is %d\n",
@@ -236,8 +232,12 @@ ffs_mkfs(const char *fsys, const fsinfo_t
*fsopts, time_t tstamp)
sblock.fs_bsize / MAXFRAG);
exit(21);
}
+ sblock.fs_fragshift = ilog2(sblock.fs_frag);
sblock.fs_fsbtodb = ilog2(sblock.fs_fsize / sectorsize);
sblock.fs_size = fssize = dbtofsb(&sblock, fssize);
+ sblock.fs_nspf = sblock.fs_fsize / sectorsize;
+ sblock.fs_nrpos = 1;
+ sblock.fs_cpg = 1;
if (Oflag <= 1) {
sblock.fs_magic = FS_UFS1_MAGIC;
@@ -251,13 +251,10 @@ ffs_mkfs(const char *fsys, const fsinfo_t
*fsopts, time_t tstamp)
sblock.fs_ffs1_size = sblock.fs_size;
sblock.fs_rotdelay = 0;
sblock.fs_rps = 60;
- sblock.fs_nspf = sblock.fs_fsize / sectorsize;
- sblock.fs_cpg = 1;
sblock.fs_interleave = 1;
sblock.fs_trackskew = 0;
sblock.fs_cpc = 0;
sblock.fs_postblformat = 1;
- sblock.fs_nrpos = 1;
} else {
sblock.fs_magic = FS_UFS2_MAGIC;
#if 0 /* XXX makefs is used for small filesystems. */
software tcp send offloading
Hi,
Claudio suggested to implement TCP send offloading in software as
a fallback if hardware cannot do it. So I took Jan's diff, ripped
that part out and polished it.
Result is that software TSO can be a bit faster than regular TCP.
http://bluhm.genua.de/perform/results/2023-05-04T07%3A05%3A11Z/perform.html
Left column is OpenBSD-current, middle is with this diff, but TSO
disabled, and right is software TSO enabled. So comparing middle
and right shows the effect.
http://bluhm.genua.de/perform/results/2023-05-04T07%3A05%3A11Z/patch-sys-tso-soft-disable.0/btrace/tcpbench_-S100_-t10_10.3.45.35-btrace-kstack.0.svg
http://bluhm.genua.de/perform/results/2023-05-04T07%3A05%3A11Z/patch-sys-tso-soft.0/btrace/tcpbench_-S100_-t10_10.3.45.35-btrace-kstack.0.svg
When looking at kstack you see that m_copym() load moves from
tcp_output() to ip_output() where the large packet is chopped. So
pf_test() needs less CPU cycles what explains the increased througput.
I round the large TCP packet to a multiple of the maximum segment
size with at least two segments. So I avoid residual small packets.
Implementation in pf_route() is still missing. Also IP option
handling has to be fixed and TCP output size decisions have to be
rechecked.
I just wanted to show where this is heading. When software TSO is
done, it should be easy to use network interface hardware to speed
things up.
bluhm
Index: netinet/in.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in.h,v
retrieving revision 1.142
diff -u -p -r1.142 in.h
--- netinet/in.h11 Apr 2023 00:45:09 - 1.142
+++ netinet/in.h5 May 2023 15:02:12 -
@@ -780,6 +780,7 @@ intin_canforward(struct in_addr);
int in_cksum(struct mbuf *, int);
int in4_cksum(struct mbuf *, u_int8_t, int, int);
void in_proto_cksum_out(struct mbuf *, struct ifnet *);
+int in_ifcap_cksum(struct mbuf *, struct ifnet *, int);
void in_ifdetach(struct ifnet *);
int in_mask2len(struct in_addr *);
void in_len2mask(struct in_addr *, int);
Index: netinet/ip_output.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.382
diff -u -p -r1.382 ip_output.c
--- netinet/ip_output.c 12 Aug 2022 17:04:16 - 1.382
+++ netinet/ip_output.c 5 May 2023 15:02:12 -
@@ -84,7 +84,6 @@ void ip_mloopback(struct ifnet *, struct
static __inline u_int16_t __attribute__((__unused__))
in_cksum_phdr(u_int32_t, u_int32_t, u_int32_t);
void in_delayed_cksum(struct mbuf *);
-int in_ifcap_cksum(struct mbuf *, struct ifnet *, int);
int ip_output_ipsec_lookup(struct mbuf *m, int hlen, struct inpcb *inp,
struct tdb **, int ipsecflowinfo);
@@ -104,7 +103,7 @@ ip_output(struct mbuf *m, struct mbuf *o
{
struct ip *ip;
struct ifnet *ifp = NULL;
- struct mbuf_list fml;
+ struct mbuf_list ml;
int hlen = sizeof (struct ip);
int error = 0;
struct route iproute;
@@ -469,6 +468,24 @@ sendit:
goto done;
}
+ if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO) &&
+ m->m_pkthdr.ph_mss <= mtu) {
+ error = tcp_chopper(m, &ml, ifp, m->m_pkthdr.ph_mss);
+ if (error)
+ goto done;
+
+ while ((m = ml_dequeue(&ml)) != NULL) {
+ error = ifp->if_output(ifp, m, sintosa(dst), ro->ro_rt);
+ if (error)
+ break;
+ }
+ if (error)
+ ml_purge(&ml);
+ else
+ tcpstat_inc(tcps_outswtso);
+ goto done;
+ }
+
/*
* Too large for interface; fragment if possible.
* Must be able to put at least 8 bytes per fragment.
@@ -505,17 +522,17 @@ sendit:
goto bad;
}
- error = ip_fragment(m, &fml, ifp, mtu);
+ error = ip_fragment(m, &ml, ifp, mtu);
if (error)
goto done;
- while ((m = ml_dequeue(&fml)) != NULL) {
+ while ((m = ml_dequeue(&ml)) != NULL) {
error = ifp->if_output(ifp, m, sintosa(dst), ro->ro_rt);
if (error)
break;
}
if (error)
- ml_purge(&fml);
+ ml_purge(&ml);
else
ipstat_inc(ips_fragmented);
@@ -677,16 +694,15 @@ ip_output_ipsec_send(struct tdb *tdb, st
#endif /* IPSEC */
int
-ip_fragment(struct mbuf *m0, struct mbuf_list *fml, struct ifnet *ifp,
+ip_fragment(struct mbuf *m0, struct mbuf_list *ml, struct ifnet *ifp,
u_long mtu)
{
- struct mbuf *m;
struct ip *ip;
int firstlen, hlen, tlen, len, off;
int error;
- ml_init(fml);
- ml_enqueue(fml, m0);
+ ml_init(ml);
+ ml_enqueue(ml, m0);
Re: cron: better error checking of random values
On 2023 May 05 (Fri) at 16:13:01 +1000 (+1000), Mark Jamsek wrote:
:On 23-05-04 05:40PM, Todd C. Miller wrote:
:> On Thu, 04 May 2023 21:41:26 -, Klemens Nanni wrote:
:>
:> > On Thu, May 04, 2023 at 03:30:30PM -0600, Todd C. Miller wrote:
:> > > This fixes two issues with the parsing of random values:
:> > >
:> > > 1) A random value with a step is now rejected. For example:
:> > >
:> > > ~/10* * * * echo invalid
:> >
:> > I've ben using ~/10 to randomly distribute four similar tasks so that
:> > they don't start at the same time.
:> >
:> > Is that wrong?
:>
:> I'm fairly certain that doesn't do what you think it does. When I
:> tested it "~/10" behaved the same as "~". The step value is not
:> even parsed.
:
I really dislike "previously accepted (even if behaved differently)"
configs being rejected ...
:todd is correct in that the step value is not parsed with "~/10". We
:recently discovered this when setting up Got mirrors to sync every 15
:minutes. IIRC, Lucas (or op?) asked about syncing each mirror at
:a different 15 minute interval by using the same syntax kn is using.
:
:I found kn's attempted syntax intuitive though; it feels like a natural
:extension of the existing random and step syntax. I also assumed ~/15
:would run every 15 minutes starting with a random minute, and since
:discovering it didn't work like that, I've been carrying a simple patch
:that allows kn's syntax:
:
: ~/15 random 15 minute intervals in [0, 59]
: 1~9/10 random 10 minute intervals in [1,59]
:
... but I really like this syntax and behaviour.
I haven't had a chance to review the code, but I think this would be a
better direction for us to go.
-peter
:8<
:diff refs/remotes/origin/master refs/heads/master
:commit - e253a7cc21de530da6fcf49c1279258fecade8f4
:commit + 761b09ae46431344766330cc14c958ffca5a3a0a
:blob - ab683b8476a8c862aabc53101b4080959820835a
:blob + 030ab599dcf07eb3e94efe90c118e4e9bea8f6c4
:--- usr.sbin/cron/entry.c
:+++ usr.sbin/cron/entry.c
:@@ -456,10 +456,11 @@ get_range(bitstr_t *bits, int low, int high, const cha
: /* range = number | number* "~" number* | number "-" number ["/" number]
:*/
:
:- int i, num1, num2, num3;
:+ int i, num1, num2, num3, rndstep;
:
: num1 = low;
: num2 = high;
:+ rndstep = 0;
:
: if (ch == '*') {
: /* '*' means [low, high] but can still be modified by /step
:@@ -497,7 +498,7 @@ get_range(bitstr_t *bits, int low, int high, const cha
:
: /* get the (optional) number following the tilde
:*/
:- ch = get_number(&num2, low, names, ch, file, ", \t\n");
:+ ch = get_number(&num2, low, names, ch, file, "/, \t\n");
: if (ch == EOF)
: ch = get_char(file);
: if (ch == EOF || num1 > num2) {
:@@ -509,6 +510,10 @@ get_range(bitstr_t *bits, int low, int high, const cha
:*/
: num3 = num1;
: num1 = arc4random_uniform(num2 - num3 + 1) + num3;
:+ if (ch == '/') {
:+ rndstep = 1;
:+ break;
:+ }
: /* FALLTHROUGH */
: default:
: /* not a range, it's a single number.
:@@ -538,6 +543,10 @@ get_range(bitstr_t *bits, int low, int high, const cha
: ch = get_number(&num3, 0, NULL, ch, file, ", \t\n");
: if (ch == EOF || num3 == 0)
: return (EOF);
:+ if (rndstep) {
:+ num1 %= num3;
:+ num2 = high;
:+ }
: } else {
: /* no step. default==1.
:*/
:>8
:
:> It sounds like what you want is the proposed syntax "*/~10"
:> to use a random offset.
:
:But this would be nice too! Anything that enables regular intervals from
:a random offset would satisfy a common enough use case.
:
:--
:Mark Jamsek
:GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
--
Time flies like an arrow, but fruit flies like a banana.
Re: cron: better error checking of random values
On Thu, May 04, 2023 at 05:40:10PM -0600, Todd C. Miller wrote: > I'm fairly certain that doesn't do what you think it does. When I > tested it "~/10" behaved the same as "~". The step value is not > even parsed. Oh I see, it is actually picking a random minute and ignores steps, so entries run once an hour: ~/10 * * * *-qs touch /tmp/cron-foo.$(date +\%s) ~/10 * * * *-qs touch /tmp/cron-bar.$(date +\%s) ~/10 * * * *-qs touch /tmp/cron-lol.$(date +\%s) from earlier gave me: -rw-r--r-- 1 kn wheel 0 May 5 18:08 /tmp/cron-bar.1683295681 -rw-r--r-- 1 kn wheel 0 May 5 19:08 /tmp/cron-bar.1683299281 -rw-r--r-- 1 kn wheel 0 May 5 18:34 /tmp/cron-foo.1683297241 -rw-r--r-- 1 kn wheel 0 May 5 18:11 /tmp/cron-lol.1683295861 -rw-r--r-- 1 kn wheel 0 May 5 19:11 /tmp/cron-lol.1683299461 > > It sounds like what you want is the proposed syntax "*/~10" > to use a random offset. Indeed, thanks.
