echo(1): Fix some style issues
Index: echo.c === RCS file: /cvs/src/bin/echo/echo.c,v retrieving revision 1.10 diff -u -p -r1.10 echo.c --- echo.c 9 Oct 2015 01:37:06 - 1.10 +++ echo.c 21 Sep 2022 20:41:56 - @@ -30,12 +30,11 @@ * SUCH DAMAGE. */ +#include #include #include #include -#include -/* ARGSUSED */ int main(int argc, char *argv[]) { @@ -45,11 +44,10 @@ main(int argc, char *argv[]) err(1, "pledge"); /* This utility may NOT do getopt(3) option parsing. */ - if (*++argv && !strcmp(*argv, "-n")) { - ++argv; + if (*++argv && strcmp(*argv, "-n") == 0) { + argv++; nflag = 1; - } - else + } else nflag = 0; while (*argv) { @@ -57,6 +55,7 @@ main(int argc, char *argv[]) if (*++argv) putchar(' '); } + if (!nflag) putchar('\n');
Re: bgpd, improve portability by abstracting fib-priority
On Wed, Sep 21, 2022 at 07:55:24PM +0200, Claudio Jeker wrote: > Different systems need different ways to define fib-priority. > Introduce two kroute specific helper functions that are used by the parser > so that the RTP_XYZ defines no longer leak outside of kroute.c > > kr_default_prio() on OpenBSD returns RTP_BGP. On Linux that will be > RTPROT_BGP and on FreeBSD it will be 3 (for RTF_PROTO3). > > kr_check_prio() checks that the value is in the valid range. Now the error > message from parse.y is a bit less helpful but I think that is OK. ok tb > > -- > :wq Claudio > > Index: bgpd.h > === > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v > retrieving revision 1.452 > diff -u -p -r1.452 bgpd.h > --- bgpd.h31 Aug 2022 15:51:44 - 1.452 > +++ bgpd.h21 Sep 2022 17:48:53 - > @@ -1281,6 +1281,8 @@ RB_PROTOTYPE(roa_tree, roa, entry, roa_c > > /* kroute.c */ > int kr_init(int *, uint8_t); > +int kr_default_prio(void); > +int kr_check_prio(long long); > int ktable_update(u_int, char *, int); > void ktable_preload(void); > void ktable_postload(void); > Index: kroute.c > === > RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v > retrieving revision 1.299 > diff -u -p -r1.299 kroute.c > --- kroute.c 15 Sep 2022 08:20:14 - 1.299 > +++ kroute.c 21 Sep 2022 17:43:12 - > @@ -258,6 +258,20 @@ kr_init(int *fd, uint8_t fib_prio) > } > > int > +kr_default_prio(void) > +{ > + return RTP_BGP; > +} > + > +int > +kr_check_prio(long long prio) > +{ > + if (prio <= RTP_LOCAL || prio > RTP_MAX) > + return 0; > + return 1; > +} > + > +int > ktable_new(u_int rtableid, u_int rdomid, char *name, int fs) > { > struct ktable **xkrt; > Index: parse.y > === > RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v > retrieving revision 1.435 > diff -u -p -r1.435 parse.y > --- parse.y 17 Aug 2022 15:15:26 - 1.435 > +++ parse.y 21 Sep 2022 17:47:40 - > @@ -708,9 +708,8 @@ conf_main : AS as4number { > TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry); > } > | FIBPRIORITY NUMBER{ > - if ($2 <= RTP_LOCAL || $2 > RTP_MAX) { > - yyerror("fib-priority %lld must be between " > - "%u and %u", $2, RTP_LOCAL + 1, RTP_MAX); > + if (!kr_check_prio($2)) { > + yyerror("fib-priority %lld out of range", $2); > YYERROR; > } > conf->fib_priority = $2; > @@ -1046,9 +1045,8 @@ network : NETWORK prefix filter_set { > } > | NETWORK family PRIORITY NUMBER filter_set { > struct network *n; > - if ($4 <= RTP_LOCAL && $4 > RTP_MAX) { > - yyerror("priority %lld must be between " > - "%u and %u", $4, RTP_LOCAL + 1, RTP_MAX); > + if (!kr_check_prio($4)) { > + yyerror("priority %lld out of range", $4); > YYERROR; > } > > @@ -3598,7 +3596,7 @@ init_config(struct bgpd_config *c) > c->holdtime = INTERVAL_HOLD; > c->connectretry = INTERVAL_CONNECTRETRY; > c->bgpid = get_bgpid(); > - c->fib_priority = RTP_BGP; > + c->fib_priority = kr_default_prio(); > c->default_tableid = getrtable(); > if (!ktable_exists(c->default_tableid, )) > fatalx("current routing table %u does not exist", > Index: printconf.c > === > RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v > retrieving revision 1.158 > diff -u -p -r1.158 printconf.c > --- printconf.c 17 Aug 2022 09:15:06 - 1.158 > +++ printconf.c 21 Sep 2022 17:38:57 - > @@ -417,7 +417,7 @@ print_mainconf(struct bgpd_config *conf) > printf("nexthop qualify via bgp\n"); > if (conf->flags & BGPD_FLAG_NEXTHOP_DEFAULT) > printf("nexthop qualify via default\n"); > - if (conf->fib_priority != RTP_BGP) > + if (conf->fib_priority != kr_default_prio()) > printf("fib-priority %hhu\n", conf->fib_priority); > printf("\n"); > } >
bgpd, improve portability by abstracting fib-priority
Different systems need different ways to define fib-priority. Introduce two kroute specific helper functions that are used by the parser so that the RTP_XYZ defines no longer leak outside of kroute.c kr_default_prio() on OpenBSD returns RTP_BGP. On Linux that will be RTPROT_BGP and on FreeBSD it will be 3 (for RTF_PROTO3). kr_check_prio() checks that the value is in the valid range. Now the error message from parse.y is a bit less helpful but I think that is OK. -- :wq Claudio Index: bgpd.h === RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v retrieving revision 1.452 diff -u -p -r1.452 bgpd.h --- bgpd.h 31 Aug 2022 15:51:44 - 1.452 +++ bgpd.h 21 Sep 2022 17:48:53 - @@ -1281,6 +1281,8 @@ RB_PROTOTYPE(roa_tree, roa, entry, roa_c /* kroute.c */ int kr_init(int *, uint8_t); +int kr_default_prio(void); +int kr_check_prio(long long); int ktable_update(u_int, char *, int); voidktable_preload(void); voidktable_postload(void); Index: kroute.c === RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v retrieving revision 1.299 diff -u -p -r1.299 kroute.c --- kroute.c15 Sep 2022 08:20:14 - 1.299 +++ kroute.c21 Sep 2022 17:43:12 - @@ -258,6 +258,20 @@ kr_init(int *fd, uint8_t fib_prio) } int +kr_default_prio(void) +{ + return RTP_BGP; +} + +int +kr_check_prio(long long prio) +{ + if (prio <= RTP_LOCAL || prio > RTP_MAX) + return 0; + return 1; +} + +int ktable_new(u_int rtableid, u_int rdomid, char *name, int fs) { struct ktable **xkrt; Index: parse.y === RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v retrieving revision 1.435 diff -u -p -r1.435 parse.y --- parse.y 17 Aug 2022 15:15:26 - 1.435 +++ parse.y 21 Sep 2022 17:47:40 - @@ -708,9 +708,8 @@ conf_main : AS as4number { TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry); } | FIBPRIORITY NUMBER{ - if ($2 <= RTP_LOCAL || $2 > RTP_MAX) { - yyerror("fib-priority %lld must be between " - "%u and %u", $2, RTP_LOCAL + 1, RTP_MAX); + if (!kr_check_prio($2)) { + yyerror("fib-priority %lld out of range", $2); YYERROR; } conf->fib_priority = $2; @@ -1046,9 +1045,8 @@ network : NETWORK prefix filter_set { } | NETWORK family PRIORITY NUMBER filter_set { struct network *n; - if ($4 <= RTP_LOCAL && $4 > RTP_MAX) { - yyerror("priority %lld must be between " - "%u and %u", $4, RTP_LOCAL + 1, RTP_MAX); + if (!kr_check_prio($4)) { + yyerror("priority %lld out of range", $4); YYERROR; } @@ -3598,7 +3596,7 @@ init_config(struct bgpd_config *c) c->holdtime = INTERVAL_HOLD; c->connectretry = INTERVAL_CONNECTRETRY; c->bgpid = get_bgpid(); - c->fib_priority = RTP_BGP; + c->fib_priority = kr_default_prio(); c->default_tableid = getrtable(); if (!ktable_exists(c->default_tableid, )) fatalx("current routing table %u does not exist", Index: printconf.c === RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v retrieving revision 1.158 diff -u -p -r1.158 printconf.c --- printconf.c 17 Aug 2022 09:15:06 - 1.158 +++ printconf.c 21 Sep 2022 17:38:57 - @@ -417,7 +417,7 @@ print_mainconf(struct bgpd_config *conf) printf("nexthop qualify via bgp\n"); if (conf->flags & BGPD_FLAG_NEXTHOP_DEFAULT) printf("nexthop qualify via default\n"); - if (conf->fib_priority != RTP_BGP) + if (conf->fib_priority != kr_default_prio()) printf("fib-priority %hhu\n", conf->fib_priority); printf("\n"); }
Re: CVS: cvs.openbsd.org: src
This change fixes another wart in unveil/pledge which wasn't resolved in the original design. pledge allows bypass-reading of /usr/share/zoneinfo/ files for TZ=zone but absolute path support remained a wart. Once again, we have to remove a rarely used behavior of libc. During pledge and unveil propagation in programs, and even earlier with privsep development (meaning use of chroot), we added many early calls to tzset() in programs. Some programs stopped using chroot, and rely upon pledge and unveil instead. Many of those tzset() calls could potentially be removed because other libc functions can initialize late due to the zoneinfo directory bypass. When doing so, please remmber -portable versions will still need to perform the initialization calls early, and also the chroot case still needs early initialization also. Todd C. Miller wrote: > CVSROOT: /cvs > Module name: src > Changes by: mill...@cvs.openbsd.org 2022/09/21 09:57:49 > > Modified files: > lib/libc/time : localtime.c tzset.3 > > Log message: > tzset: ignore TZ if it contains an absolute path or issetugid(). > Reading time zone files from user-controlled paths can result in > pledge(2) or unveil(2) violations. We also ignore files that contain > a '.' character to avoid paths containing ".." or hidden files. > Work with and OK deraadt@ >
Re: Help on helping with virtualization?
AFAIK, the standard procedure is: - you have an itch nobody cared/had time to scratch before. - you study enough to spit at least some lines of code aiming your goals. - you puts(3) your lame-or-not code in-here. - if it's interesting enough, someone will chime in. Regards!!! El mié, 21 sept 2022 a las 14:10, Christoff Humphries () escribió: > > Hello. > > I want to help with the virtualization project to get the things that > are incomplete or missing completed (ie, the "not available at this > time" list on https://www.openbsd.org/faq/faq16.html). > > Is there a point of contact I should direct questions to, as I'll likely > have a lot of questions once start diving in to load it into my brain > and understand the code well enough to do something well and useful. > > Or should I just ask whatever questions I have here? > > Asking as I'm not sure how this project handles that stuff (open list or > pushed more to mentor/owner people for subject areas). > > Thanks! > Christoff Humphries >
unbound 1.16.3
Released today so I haven't been able to give it much testing yet... Index: doc/Changelog === RCS file: /cvs/src/usr.sbin/unbound/doc/Changelog,v retrieving revision 1.45 diff -u -p -r1.45 Changelog --- doc/Changelog 29 Aug 2022 16:05:00 - 1.45 +++ doc/Changelog 21 Sep 2022 12:41:57 - @@ -1,5 +1,5 @@ -7 February 2022: Wouter - - Fix that TCP interface does not use TLS when TLS is also configured. +21 September 2022: Wouter + - Patch for CVE-2022-3204 Non-Responsive Delegation Attack. 1 August 2022: Wouter - Fix the novel ghost domain issues CVE-2022-30698 and CVE-2022-30699. Index: config.guess === RCS file: /cvs/src/usr.sbin/unbound/config.guess,v retrieving revision 1.12 diff -u -p -r1.12 config.guess --- config.guess7 Jun 2022 15:42:53 - 1.12 +++ config.guess21 Sep 2022 12:41:57 - @@ -4,7 +4,7 @@ # shellcheck disable=SC2006,SC2268 # see below for rationale -timestamp='2022-05-25' +timestamp='2022-08-01' # This file is free software; you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -1036,7 +1036,7 @@ EOF k1om:Linux:*:*) GUESS=$UNAME_MACHINE-unknown-linux-$LIBC ;; -loongarch32:Linux:*:* | loongarch64:Linux:*:* | loongarchx32:Linux:*:*) +loongarch32:Linux:*:* | loongarch64:Linux:*:*) GUESS=$UNAME_MACHINE-unknown-linux-$LIBC ;; m32r*:Linux:*:*) Index: config.sub === RCS file: /cvs/src/usr.sbin/unbound/config.sub,v retrieving revision 1.11 diff -u -p -r1.11 config.sub --- config.sub 23 Feb 2022 12:04:05 - 1.11 +++ config.sub 21 Sep 2022 12:41:57 - @@ -4,7 +4,7 @@ # shellcheck disable=SC2006,SC2268 # see below for rationale -timestamp='2022-01-03' +timestamp='2022-08-01' # This file is free software; you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -1207,7 +1207,7 @@ case $cpu-$vendor in | k1om \ | le32 | le64 \ | lm32 \ - | loongarch32 | loongarch64 | loongarchx32 \ + | loongarch32 | loongarch64 \ | m32c | m32r | m32rle \ | m5200 | m68000 | m680[012346]0 | m68360 | m683?2 | m68k \ | m6811 | m68hc11 | m6812 | m68hc12 | m68hcs12x \ Index: configure === RCS file: /cvs/src/usr.sbin/unbound/configure,v retrieving revision 1.47 diff -u -p -r1.47 configure --- configure 29 Aug 2022 16:04:59 - 1.47 +++ configure 21 Sep 2022 12:41:57 - @@ -1,6 +1,6 @@ #! /bin/sh # Guess values for system-dependent variables and create Makefiles. -# Generated by GNU Autoconf 2.71 for unbound 1.16.2. +# Generated by GNU Autoconf 2.71 for unbound 1.16.3. # # Report bugs to https://github.com/NLnetLabs/unbound/issues>. # @@ -622,8 +622,8 @@ MAKEFLAGS= # Identity of this package. PACKAGE_NAME='unbound' PACKAGE_TARNAME='unbound' -PACKAGE_VERSION='1.16.2' -PACKAGE_STRING='unbound 1.16.2' +PACKAGE_VERSION='1.16.3' +PACKAGE_STRING='unbound 1.16.3' PACKAGE_BUGREPORT='unbound-b...@nlnetlabs.nl or https://github.com/NLnetLabs/unbound/issues' PACKAGE_URL='' @@ -1503,7 +1503,7 @@ if test "$ac_init_help" = "long"; then # Omit some internal or obsolete options to make the list less imposing. # This message is too long to be a string in the A/UX 3.1 sh. cat <<_ACEOF -\`configure' configures unbound 1.16.2 to adapt to many kinds of systems. +\`configure' configures unbound 1.16.3 to adapt to many kinds of systems. Usage: $0 [OPTION]... [VAR=VALUE]... @@ -1569,7 +1569,7 @@ fi if test -n "$ac_init_help"; then case $ac_init_help in - short | recursive ) echo "Configuration of unbound 1.16.2:";; + short | recursive ) echo "Configuration of unbound 1.16.3:";; esac cat <<\_ACEOF @@ -1812,7 +1812,7 @@ fi test -n "$ac_init_help" && exit $ac_status if $ac_init_version; then cat <<\_ACEOF -unbound configure 1.16.2 +unbound configure 1.16.3 generated by GNU Autoconf 2.71 Copyright (C) 2021 Free Software Foundation, Inc. @@ -2469,7 +2469,7 @@ cat >config.log <<_ACEOF This file contains any messages produced by compilers while running configure, to aid debugging if configure makes a mistake. -It was created by unbound $as_me 1.16.2, which was +It was created by unbound $as_me 1.16.3, which was generated by GNU Autoconf 2.71. Invocation command line was $ $0$ac_configure_args_raw @@ -3233,11 +3233,11 @@ UNBOUND_VERSION_MAJOR=1 UNBOUND_VERSION_MINOR=16 -UNBOUND_VERSION_MICRO=2 +UNBOUND_VERSION_MICRO=3 LIBUNBOUND_CURRENT=9 -LIBUNBOUND_REVISION=18
Help on helping with virtualization?
Hello. I want to help with the virtualization project to get the things that are incomplete or missing completed (ie, the "not available at this time" list on https://www.openbsd.org/faq/faq16.html). Is there a point of contact I should direct questions to, as I'll likely have a lot of questions once start diving in to load it into my brain and understand the code well enough to do something well and useful. Or should I just ask whatever questions I have here? Asking as I'm not sure how this project handles that stuff (open list or pushed more to mentor/owner people for subject areas). Thanks! Christoff Humphries
Re: iked problems with Apple clients in 7.1
On 2022/05/21 17:04, Tobias Heider wrote: > On Sat, May 21, 2022 at 12:51:19PM +0100, Stuart Henderson wrote: > > On 2022/05/21 13:44, Tobias Heider wrote: > > > On Fri, May 20, 2022 at 03:41:12PM +0100, Stuart Henderson wrote: > > > > I ran into problems with Apple clients failing to connect to > > > > iked after updating a machine to 7.1, introduced by > > > > https://github.com/openbsd/src/commit/e3f5cf2ee26929d75dc2df9e86d97c36b2a94268 > > > > > > > > spi=0xac3d46687441f957: recv IKE_SA_INIT req 0 peer > > > > rrr.rrr.rrr.rr:49436 local lll.ll.lll.lll:500, 308 bytes, policy > > > > 'default' > > > > spi=0xac3d46687441f957: send IKE_SA_INIT res 0 peer > > > > rrr.rrr.rrr.rr:49436 local lll.ll.lll.lll:500, 341 bytes > > > > spi=0xac3d46687441f957: recv IKE_AUTH req 1 peer rrr.rrr.rrr.rr:64892 > > > > local lll.ll.lll.lll:4500, 368 bytes, policy 'default' > > > > policy_test: localid mismatch > > > > spi=0xac3d46687441f957: ikev2_ike_auth_recv: no compatible policy found > > > > spi=0xac3d46687441f957: ikev2_send_auth_failed: authentication failed > > > > for > > > > spi=0xac3d46687441f957: send IKE_AUTH res 1 peer rrr.rrr.rrr.rr:64892 > > > > local lll.ll.lll.lll:4500, 80 bytes, NAT-T > > > > spi=0xac3d46687441f957: sa_free: authentication failed > > > > > > > > I don't have full details of config done on the other side nor any > > > > fruit-based phones to test from myself, did anyone already run into this > > > > and figure out a way around it? > > > > > > Hey Stuart, > > > > > > I haven't seen this before but I have a theory. > > > Based on the commit you pointed out the problem is probably the > > > `dstid kk.kkk.kkk.kkk` line which was ignored before this change. > > > > > > This should be easy to check without access to the other device if > > > you enable verbose logging on your server and look for "ikev2_pld_id" > > > above the error. I suspect that the ID sent by your apple peer might > > > actually be a different one than kk.kkk.kkk.kkk. > > > > > > Another thing you could try is just removing the dstid part and see > > > if that works. > > > > Oh sorry I wasn't clear about which one the apple was using - the one with > > "kk.kkk.kkk.kkk" is a lan-to-lan configuration (fixed IP on both endpoints) > > - > > the apple is expected to be using the first "from 0.0.0.0/0 to dynamic" one > > which doesn't have any dstid set, and that's the only one where the IP > > matches. > > Oh, makes sense. I think it may still be related to the IDs, so checking if > ikev2_pld_id matches what you expect for srcid might be a good start. > Maybe the apple client is sending something different than > "" > in their dstid. I was able to get someone to do a couple of tests (after accidentally updating that machine and running into the problem again..) - debug log from iked shows 2022-09-21T09:10:44.370Z vpn2 iked[9682]: ikev2_pld_payloads: decrypted payload IDi nextpayload NOTIFY critical 0x00 length 28 2022-09-21T09:10:44.370Z vpn2 iked[9682]: ikev2_pld_id: id FQDN/vpn2.xxx length 24 2022-09-21T09:10:44.370Z vpn2 iked[9682]: ikev2_pld_payloads: decrypted payload NOTIFY nextpayload IDr critical 0x00 length 8 2022-09-21T09:10:44.370Z vpn2 iked[9682]: ikev2_pld_notify: protoid NONE spisize 0 type INITIAL_CONTACT 2022-09-21T09:10:44.370Z vpn2 iked[9682]: ikev2_pld_payloads: decrypted payload IDr nextpayload CP critical 0x00 length 12 2022-09-21T09:10:44.370Z vpn2 iked[9682]: ikev2_pld_id: id FQDN/user length 8 ... 2022-09-21T09:10:44.371Z vpn2 iked[9682]: ikev2_resp_recv: NAT-T message received, updated SA 2022-09-21T09:10:44.371Z vpn2 iked[9682]: sa_stateok: SA_INIT flags 0x, require 0x 2022-09-21T09:10:44.371Z vpn2 iked[9682]: spi=0xbb3b1aa559598982: sa_state: SA_INIT -> EAP 2022-09-21T09:10:44.371Z vpn2 iked[9682]: policy_lookup: peerid 'vpn2.xxx' 2022-09-21T09:10:44.371Z vpn2 iked[9682]: policy_lookup: localid 'user' 2022-09-21T09:10:44.372Z vpn2 iked[9682]: policy_test: localid mismatch 2022-09-21T09:10:44.372Z vpn2 iked[9682]: spi=0xbb3b1aa559598982: ikev2_ike_auth_recv: no compatible policy found 2022-09-21T09:10:44.372Z vpn2 iked[9682]: spi=0xbb3b1aa559598982: ikev2_send_auth_failed: authentication failed for Which, given that the apple device is initiator, suggests that they're sending the IDs the wrong way round. I don't know whether that's a bug Apple-side or a config issue, either way it's going to be hard to fix as I don't have access to the client devices (or any similar device myself to tell them what to change). Reading policy.c:policy_lookup() ... 125 /* Try to find a matching policy for this message */ 126 if ((msg->msg_policy = policy_test(env, )) != NULL) { 127 log_debug("%s: setting policy '%s'", __func__, 128 msg->msg_policy->pol_name); 129 return (0); 130 } 131 132 /* No matching policy found, try the default */ 133
Re: hidmt: clickpad detection
On Tue, Sep 13, 2022 at 11:06:33PM +0200, Ulf Brosziewski wrote: Hello Ulf, > The diff below improves the way hidmt identifies clickpads, and > addresses the problems described in > https://marc.info/?l=openbsd-tech=165296530618617=2 > and > https://marc.info/?l=openbsd-tech=165980534415586=2 > > If devices don't report a HUD_BUTTON_TYPE property, the driver checks > whether they claim to have an external left button, and if they do, > hidmt doesn't treat them as clickpads. > > I think this part of the logic should be turned around: hidmt should > treat everything as clickpad except if there is no "clickpad button" > (HUP_BUTTON 1) *and* both an external left and an external right button > (HUP BUTTON 2 and 3) are being reported. Touchpads without the internal > button are still usable with the clickpad configuration, it does less > harm to err on this side. > > Tests and OKs would be welcome. Thanks for doing this! This fixes the Framework touchpad and also seems like a better heuristic than we currently have (certainly it seems better to me than the Framework-specific quirk I added!). I think it would be great to try this in snapshots for a little while to check whether it causes any issues, though I'm optimistic that it will Just Work! Laurie
Re: bgpd speed up pathid_assign() for the common case
On Mon, Sep 19, 2022 at 05:44:30PM +0200, Claudio Jeker wrote: > When running on busy bgpd servers with many clients the function > pathid_assign() consumes a lot of CPU time. The code does a lookup which > often fails and then walks the list of prefixes. In the end this is > results in two list walks. > > This complicated dance is only needed for peers that use add-path but > regular peers can skip this by assigning a per peer path_id instead. > To make this work even path_id_txs are for regular peers while the > add-path peers use even numbers. > > This should help route collectors and route reflectors that receive the > same route many times from different sources. At least as long as these > peers don't use add-path. ok Some small suggestions inline. > -- > :wq Claudio > > Index: rde.c > === > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v > retrieving revision 1.576 > diff -u -p -r1.576 rde.c > --- rde.c 12 Sep 2022 10:03:17 - 1.576 > +++ rde.c 19 Sep 2022 15:29:19 - > @@ -1607,28 +1607,38 @@ pathid_conflict(struct rib_entry *re, ui > return 0; > } > > +/* > + * Assign a send side path_id to all paths. > + */ > static uint32_t > pathid_assign(struct rde_peer *peer, uint32_t path_id, > struct bgpd_addr *prefix, uint8_t prefixlen) > { > struct rib_entry *re; > - struct prefix *p = NULL; > uint32_t path_id_tx; > > - /* > - * Assign a send side path_id to all paths. > - */ > + /* If peer has no add-path use the per peer path_id */ > + if (!peer_has_add_path(peer, prefix->aid, CAPA_AP_RECV)) > + return peer->path_id_tx; > + > + /* peer uses add-path, therefor new path_ids need to be assigned */ therefore > re = rib_get(rib_byid(RIB_ADJ_IN), prefix, prefixlen); > - if (re != NULL) > + if (re != NULL) { > + struct prefix *p; > + > p = prefix_bypeer(re, peer, path_id); > - if (p != NULL) > - path_id_tx = p->path_id_tx; > - else { > - do { > - /* assign new local path_id */ > - path_id_tx = arc4random(); > - } while (pathid_conflict(re, path_id_tx)); > + if (p != NULL) > + return p->path_id_tx; > } > + > + do { > + /* > + * Assign new local path_id, must be an odd number. > + * Even numbers are used by the per peer path_id_tx. > + */ > + path_id_tx = (arc4random() << 1) | 1; I would omit the left shift and just set the lowest bit to 1: path_id_tx = arc4random() | 1; > + } while (pathid_conflict(re, path_id_tx)); > + > return path_id_tx; > } > > Index: rde.h > === > RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v > retrieving revision 1.271 > diff -u -p -r1.271 rde.h > --- rde.h 12 Sep 2022 10:03:17 - 1.271 > +++ rde.h 19 Sep 2022 15:15:12 - > @@ -99,6 +99,7 @@ struct rde_peer { > uint32_t remote_bgpid; /* host byte order! */ > uint32_t up_nlricnt; > uint32_t up_wcnt; > + uint32_t path_id_tx; > enum peer_state state; > enum export_type export_type; > uint16_t loc_rib_id; > Index: rde_peer.c > === > RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v > retrieving revision 1.23 > diff -u -p -r1.23 rde_peer.c > --- rde_peer.c1 Sep 2022 13:23:24 - 1.23 > +++ rde_peer.c19 Sep 2022 15:26:34 - > @@ -168,6 +168,22 @@ peer_add(uint32_t id, struct peer_config > peer->flags = peer->conf.flags; > SIMPLEQ_INIT(>imsg_queue); > > + /* assign random unique transmit path id */ > + do { > + struct rde_peer *p; > + int conflict = 0; Perhaps explain the left shift with a comment matching the one in pathid_assign(). > + > + peer->path_id_tx = arc4random() << 1; > + RB_FOREACH(p, peer_tree, ) { > + if (p->path_id_tx == peer->path_id_tx) { > + conflict = 1; > + break; > + } > + } > + if (!conflict) > + break; > + } while (1); I'd make conflict a variable of function scope and set it to 0 at the start of the loop. Then you can drop the if (!conflict) check and do } while (conflict); > + > if (RB_INSERT(peer_tree, , peer) != NULL) > fatalx("rde peer table corrupted"); > >