Re: bgpd, Adj-RIB-Out support

2018-10-31 Thread David Higgs
On Wed, Oct 31, 2018 at 6:58 PM Sebastian Benoit  wrote:



On a phone, saw some typos and such, sorry no diff.

[benoit@border2:~]$ cat before
> RDE memory statistics
> 715727 IPv4 unicast network entries using 27.3M of memory
>  58953 IPv6 unicast network entries using 3.1M of memory
>1549171 rib entries using 94.6M of memory
>2897130 prefix entries using 265M of memory
> 562423 BGP path attribute entries using 60.1M of memory
> 149579 BGP AS-PATH attribute entries using 6.1M of memory,
>and holding 562423 references
>  37007 BGP attributes entries using 1.4M of memory
>and holding 880668 references


Inconsistent comma usage above


>  37006 BGP attributes using 912K of memory
>  61964 as-set elements in 58064 tables using 950K of memory
> 103150 prefix-set elments using 4.3M of memory


elments => elements

RIB using 459M of memory
> Sets using 5.2M of memory
>
> RDE hash statistics
> path hash: size 131072, 562423 entires
> min 0 max 20 avg/std-dev = 4.291/2.364
> aspath hash: size 131072, 149579 entires
> min 0 max 8 avg/std-dev = 1.141/0.835
> attr hash: size 16384, 37007 entires
> min 0 max 10 avg/std-dev = 2.259/1.378
>

entires => entries

—david


Re: bgpd, Adj-RIB-Out support

2018-10-31 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.10.31 16:24:49 +0100:
> This diff introduces a real Adj-RIB-Out. It is the minimal change to
> introduce the new RIB. This removes the update_rib introduced before 6.4
> lock because that is now replaced with a real RIB.
> The code used by bgpctl show rib is now a fair amount easier since now one
> RIB runner can be used for all cases.
> path_update() is now returning if a prefix was not modified which is
> needed in the update path to suppress unneeded updates.
> The softreconfig out case becomes simpler because there is no more the
> need to run with both filters and check results.
> Last the shutdown code of the RDE had to be adjusted a bit to still work
> with the Adj-RIB-Out.
> 
> This may cause memory consumption to go up but my testing has shown that
> the result is actually not significant. Needs the commits from earlier
> today to apply.
> -- 
> :wq Claudio

i dont see a problem with the diff other than the memory usage (and denis
remarks).

so if we decide that the memory is worth it, ok benno@

not sure if double the usage is acceptable though.

 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.444
> diff -u -p -r1.444 rde.c
> --- rde.c 31 Oct 2018 14:50:07 -  1.444
> +++ rde.c 31 Oct 2018 15:09:39 -
> @@ -1395,7 +1395,7 @@ rde_update_update(struct rde_peer *peer,
>  
>   /* add original path to the Adj-RIB-In */
>   if (path_update([RIB_ADJ_IN].rib, peer, in, prefix, prefixlen,
> - vstate))
> + vstate) == 1)
>   peer->prefix_cnt++;
>  
>   /* max prefix checker */
> @@ -2124,16 +2124,17 @@ rde_reflector(struct rde_peer *peer, str
>   * control specific functions
>   */
>  static void
> -rde_dump_rib_as(struct prefix *p, struct rde_aspath *asp,
> -struct nexthop *nexthop, pid_t pid, int flags)
> +rde_dump_rib_as(struct prefix *p, struct rde_aspath *asp, pid_t pid, int 
> flags)
>  {
>   struct ctl_show_rib  rib;
>   struct ibuf *wbuf;
>   struct attr *a;
> + struct nexthop  *nexthop;
>   void*bp;
>   time_t   staletime;
>   u_int8_t l;
>  
> + nexthop = prefix_nexthop(p);
>   bzero(, sizeof(rib));
>   rib.lastchange = p->lastchange;
>   rib.local_pref = asp->lpref;
> @@ -2208,70 +2209,37 @@ rde_dump_rib_as(struct prefix *p, struct
>  }
>  
>  static void
> -rde_dump_filterout(struct rde_peer *peer, struct prefix *p,
> -struct ctl_show_rib_request *req)
> -{
> - struct filterstate   state;
> - enum filter_actions  a;
> -
> - if (up_test_update(peer, p) != 1)
> - return;
> -
> - rde_filterstate_prep(, prefix_aspath(p), prefix_nexthop(p),
> - prefix_nhflags(p));
> - a = rde_filter(out_rules, peer, p, );
> -
> - if (a == ACTION_ALLOW)
> - rde_dump_rib_as(p, , state.nexthop, req->pid,
> - req->flags);
> -
> - rde_filterstate_clean();
> -}
> -
> -static void
>  rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
>  {
> - struct rde_peer *peer;
>   struct rde_aspath   *asp;
>  
> - if (req->flags & F_CTL_ADJ_OUT) {
> - if (p->re->active != p)
> - /* only consider active prefix */
> - return;
> - if (req->peerid) {
> - if ((peer = peer_get(req->peerid)) != NULL)
> - rde_dump_filterout(peer, p, req);
> - return;
> - }
> - } else {
> - asp = prefix_aspath(p);
> - if (req->peerid && req->peerid != prefix_peer(p)->conf.id)
> - return;
> - if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
> - return;
> - if ((req->flags & F_CTL_INVALID) &&
> - (asp->flags & F_ATTR_PARSE_ERR) == 0)
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_AS &&
> - !aspath_match(asp->aspath->data, asp->aspath->len,
> - >as, 0))
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
> - !community_match(asp, req->community.as,
> - req->community.type))
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
> - !community_ext_match(asp, >extcommunity, 0))
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_LARGECOMMUNITY &&
> - !community_large_match(asp, req->large_community.as,
> - req->large_community.ld1, req->large_community.ld2))
> - return;
> - if (!ovs_match(p, req->flags))
> - return;
> - 

Re: bgpd, Adj-RIB-Out support

2018-10-31 Thread Sebastian Benoit
Denis Fondras(open...@ledeuns.net) on 2018.10.31 21:02:17 +0100:
> On Wed, Oct 31, 2018 at 04:24:49PM +0100, Claudio Jeker wrote:
> > This diff introduces a real Adj-RIB-Out. It is the minimal change to
> > introduce the new RIB. This removes the update_rib introduced before 6.4
> > lock because that is now replaced with a real RIB.
> > The code used by bgpctl show rib is now a fair amount easier since now one
> > RIB runner can be used for all cases.
> > path_update() is now returning if a prefix was not modified which is
> > needed in the update path to suppress unneeded updates.
> > The softreconfig out case becomes simpler because there is no more the
> > need to run with both filters and check results.
> > Last the shutdown code of the RDE had to be adjusted a bit to still work
> > with the Adj-RIB-Out.
> > 
> > This may cause memory consumption to go up but my testing has shown that
> > the result is actually not significant. Needs the commits from earlier
> > today to apply.
> 
> On my Route Server, it is quite a big change (in percentage).
> * Before :
> RDE memory statistics
>  11561 IPv4 unicast network entries using 452K of memory
>131 IPv6 unicast network entries using 7.2K of memory
>  23370 rib entries using 1.4M of memory
>  23517 prefix entries using 1.8M of memory
>   4894 BGP path attribute entries using 344K of memory
>and holding 23517 references
>   1720 BGP AS-PATH attribute entries using 76.4K of memory
>and holding 4894 references
>   1061 BGP attributes entries using 41.4K of memory
>and holding 10565 references
>   1060 BGP attributes using 25.6K of memory
> 101809 as-set elements in 58041 tables using 2.1M of memory
> 114429 prefix-set elments using 4.7M of memory
> RIB using 4.1M of memory
> Sets using 6.9M of memory
> 
> RDE hash statistics
> path hash: size 131072, 4894 entires
> min 0 max 3 avg/std-dev = 0.037/0.000
> aspath hash: size 131072, 1720 entires
> min 0 max 2 avg/std-dev = 0.013/0.000
> attr hash: size 16384, 1061 entires
> min 0 max 2 avg/std-dev = 0.065/0.000
> 
> * After:
> RDE memory statistics
>  11560 IPv4 unicast network entries using 452K of memory
>145 IPv6 unicast network entries using 7.9K of memory
>  34991 rib entries using 2.1M of memory
>  69844 prefix entries using 5.3M of memory
>   4929 BGP path attribute entries using 347K of memory
>and holding 69844 references
>   1727 BGP AS-PATH attribute entries using 76.6K of memory
>and holding 4929 references
>   1066 BGP attributes entries using 41.6K of memory
>and holding 10643 references
>   1065 BGP attributes using 25.6K of memory
> 101809 as-set elements in 58041 tables using 2.1M of memory
> 114429 prefix-set elments using 4.7M of memory
> RIB using 8.4M of memory
> Sets using 6.9M of memory
> 
> RDE hash statistics
> path hash: size 131072, 4929 entires
> min 0 max 3 avg/std-dev = 0.038/0.000
> aspath hash: size 131072, 1727 entires
> min 0 max 2 avg/std-dev = 0.013/0.000
> attr hash: size 16384, 1066 entires
> min 0 max 2 avg/std-dev = 0.065/0.000
> 

ok this is a more interesting router:

[benoit@border2:~]$ cat before  
RDE memory statistics
715727 IPv4 unicast network entries using 27.3M of memory
 58953 IPv6 unicast network entries using 3.1M of memory
   1549171 rib entries using 94.6M of memory
   2897130 prefix entries using 265M of memory
562423 BGP path attribute entries using 60.1M of memory
149579 BGP AS-PATH attribute entries using 6.1M of memory,
   and holding 562423 references
 37007 BGP attributes entries using 1.4M of memory
   and holding 880668 references
 37006 BGP attributes using 912K of memory
 61964 as-set elements in 58064 tables using 950K of memory
103150 prefix-set elments using 4.3M of memory
RIB using 459M of memory
Sets using 5.2M of memory

RDE hash statistics
path hash: size 131072, 562423 entires
min 0 max 20 avg/std-dev = 4.291/2.364
aspath hash: size 131072, 149579 entires
min 0 max 8 avg/std-dev = 1.141/0.835
attr hash: size 16384, 37007 entires
min 0 max 10 avg/std-dev = 2.259/1.378

[benoit@border2:~]$ cat now 
   
RDE memory statistics
   2323535 unspec network entries using 0B of memory
716246 IPv6 unicast network entries using 38.3M of memory
 58933 IPv4 vpn network entries using 4.0M of memory
   4806161 rib entries using 293M of memory
   4806161 prefix entries using 440M of memory
529410 BGP path attribute entries using 56.5M of memory
  1945 BGP AS-PATH attribute entries using 134K of memory,
   and holding 5807738 references
529410 BGP attributes entries using 20.2M of memory
  

Re: top: merge duplicate code into helper functions

2018-10-31 Thread Klemens Nanni
On Tue, Oct 30, 2018 at 02:24:46PM -0600, Todd C. Miller wrote:
> I really dislike the side-effect in filteruser(), see below.
> Otherwise looks good.
New diff free of side-effects in filteruser(), rest unchanged.

> > +static int
> > +filteruser(char buf[])
> > +{
> > +   char *bufp = buf;
> > +   uid_t *uidp;
> > +
> > +   if (bufp[0] == '-') {
> > +   bufp++[0] = ' ';
> 
> Why is this needed, can't you just do "bufp++"?
Done.

The caller rundisplay() now checks for leading dash (again) and just
prints `tempbuf' or `tempbuf + 1' accordingly as done before without
replacing the character.

OK?

Index: top.c
===
RCS file: /cvs/src/usr.bin/top/top.c,v
retrieving revision 1.94
diff -u -p -r1.94 top.c
--- top.c   5 Oct 2018 18:56:57 -   1.94
+++ top.c   31 Oct 2018 22:21:38 -
@@ -83,7 +83,7 @@ int no_command = Yes;
 int old_system = No;
 int old_threads = No;
 int show_args = No;
-pid_t hlpid = -1;
+pid_t hlpid = (pid_t)-1;
 int combine_cpus = 0;
 
 #if Default_TOPN == Infinity
@@ -131,6 +131,46 @@ usage(void)
__progname);
 }
 
+static int
+filteruser(char buf[])
+{
+   char *bufp = buf;
+   uid_t *uidp;
+
+   if (bufp[0] == '-') {
+   bufp++;
+   uidp = 
+   ps.uid = (pid_t)-1;
+   } else {
+   uidp = 
+   ps.huid = (pid_t)-1;
+   }
+
+   return uid_from_user(bufp, uidp);
+}
+
+static int
+filterpid(char buf[], int hl)
+{
+   const char *errstr;
+   int pid;
+
+   pid = strtonum(buf, 0, INT_MAX, );
+   if (errstr != NULL || !find_pid(pid))
+   return -1;
+
+   if (hl == Yes)
+   hlpid = (pid_t)pid;
+   else {
+   if (ps.system == No)
+   old_system = No;
+   ps.pid = (pid_t)pid;
+   ps.system = Yes;
+   }
+
+   return 0;
+}
+
 static void
 parseargs(int ac, char **av)
 {
@@ -150,32 +190,16 @@ parseargs(int ac, char **av)
break;
 
case 'U':   /* display only username's processes */
-   if (optarg[0] == '-') {
-   if (uid_from_user(optarg+1, ) == -1)
-   new_message(MT_delayed,
-   "%s: unknown user", optarg);
-   else
-   ps.uid = (uid_t)-1;
-   } else if (uid_from_user(optarg, ) == -1)
+   if (filteruser(optarg) == -1)
new_message(MT_delayed, "%s: unknown user",
optarg);
-   else
-   ps.huid = (uid_t)-1;
break;
 
-   case 'p': { /* display only process id */
-   const char *errstr;
-
-   i = strtonum(optarg, 0, INT_MAX, );
-   if (errstr != NULL || !find_pid(i))
+   case 'p':   /* display only process id */
+   if (filterpid(optarg, No) == -1)
new_message(MT_delayed, "%s: unknown pid",
optarg);
-   else {
-   ps.pid = (pid_t)i;
-   ps.system = Yes;
-   }
break;
-   }
 
case 'S':   /* show system processes */
ps.system = !ps.system;
@@ -802,20 +826,12 @@ rundisplay(void)
tempbuf[1] == '\0') {
ps.uid = (uid_t)-1;
ps.huid = (uid_t)-1;
-   } else if (tempbuf[0] == '-') {
-   if (uid_from_user(tempbuf+1, ) 
== -1) {
-   new_message(MT_standout,
-   " %s: unknown user", 
tempbuf+1);
-   no_command = Yes;
-   } else {
-   ps.uid = (uid_t)-1;
-   }
-   } else if (uid_from_user(tempbuf, ) == 
-1) {
-   new_message(MT_standout,
-   " %s: unknown user", 
tempbuf);
-   no_command = Yes;
-   } else {
-   ps.huid = (uid_t)-1;
+   } else if (filteruser(tempbuf) == -1) {
+   new_message(MT_standout,
+   " %s: unknown user",
+  

Re: bgpd, Adj-RIB-Out support

2018-10-31 Thread Sebastian Benoit
Sebastian Benoit(be...@openbsd.org) on 2018.10.31 22:26:51 +0100:
> two full views v4 + v6, the usual testbox:

forget this one, the machine is not really doing much with the Adj-RIB-Out.



Re: bgpd, Adj-RIB-Out support

2018-10-31 Thread Sebastian Benoit
Denis Fondras(open...@ledeuns.net) on 2018.10.31 21:02:17 +0100:
> On Wed, Oct 31, 2018 at 04:24:49PM +0100, Claudio Jeker wrote:
> > This diff introduces a real Adj-RIB-Out. It is the minimal change to
> > introduce the new RIB. This removes the update_rib introduced before 6.4
> > lock because that is now replaced with a real RIB.
> > The code used by bgpctl show rib is now a fair amount easier since now one
> > RIB runner can be used for all cases.
> > path_update() is now returning if a prefix was not modified which is
> > needed in the update path to suppress unneeded updates.
> > The softreconfig out case becomes simpler because there is no more the
> > need to run with both filters and check results.
> > Last the shutdown code of the RDE had to be adjusted a bit to still work
> > with the Adj-RIB-Out.
> > 
> > This may cause memory consumption to go up but my testing has shown that
> > the result is actually not significant. Needs the commits from earlier
> > today to apply.
> 
> On my Route Server, it is quite a big change (in percentage).
> * Before :
> RDE memory statistics
>  11561 IPv4 unicast network entries using 452K of memory
>131 IPv6 unicast network entries using 7.2K of memory
>  23370 rib entries using 1.4M of memory
>  23517 prefix entries using 1.8M of memory
>   4894 BGP path attribute entries using 344K of memory
>and holding 23517 references
>   1720 BGP AS-PATH attribute entries using 76.4K of memory
>and holding 4894 references
>   1061 BGP attributes entries using 41.4K of memory
>and holding 10565 references
>   1060 BGP attributes using 25.6K of memory
> 101809 as-set elements in 58041 tables using 2.1M of memory
> 114429 prefix-set elments using 4.7M of memory
> RIB using 4.1M of memory
> Sets using 6.9M of memory
> 
> RDE hash statistics
> path hash: size 131072, 4894 entires
> min 0 max 3 avg/std-dev = 0.037/0.000
> aspath hash: size 131072, 1720 entires
> min 0 max 2 avg/std-dev = 0.013/0.000
> attr hash: size 16384, 1061 entires
> min 0 max 2 avg/std-dev = 0.065/0.000
> 
> * After:
> RDE memory statistics
>  11560 IPv4 unicast network entries using 452K of memory
>145 IPv6 unicast network entries using 7.9K of memory
>  34991 rib entries using 2.1M of memory
>  69844 prefix entries using 5.3M of memory
>   4929 BGP path attribute entries using 347K of memory
>and holding 69844 references
>   1727 BGP AS-PATH attribute entries using 76.6K of memory
>and holding 4929 references
>   1066 BGP attributes entries using 41.6K of memory
>and holding 10643 references
>   1065 BGP attributes using 25.6K of memory
> 101809 as-set elements in 58041 tables using 2.1M of memory
> 114429 prefix-set elments using 4.7M of memory
> RIB using 8.4M of memory
> Sets using 6.9M of memory
> 
> RDE hash statistics
> path hash: size 131072, 4929 entires
> min 0 max 3 avg/std-dev = 0.038/0.000
> aspath hash: size 131072, 1727 entires
> min 0 max 2 avg/std-dev = 0.013/0.000
> attr hash: size 16384, 1066 entires
> min 0 max 2 avg/std-dev = 0.065/0.000
> 
> I need to test it on a router with a fullview.


two full views v4 + v6, the usual testbox:

before:

[benoit@fw-ipv6onlyorg:/usr/src/usr.sbin/bgpd]$ cat  /tmp/old  
RDE memory statistics
714558 IPv4 unicast network entries using 27.3M of memory
 58836 IPv6 unicast network entries using 3.1M of memory
   1546598 rib entries using 94.4M of memory
   3093193 prefix entries using 236M of memory
413229 BGP path attribute entries using 28.4M of memory
   and holding 3093193 references
108607 BGP AS-PATH attribute entries using 4.3M of memory
   and holding 413229 references
 43526 BGP attributes entries using 1.7M of memory
   and holding 1563107 references
 43525 BGP attributes using 1.1M of memory
  1055 as-set elements in 1 tables using 32B of memory
12 prefix-set elments using 504B of memory
RIB using 396M of memory
Sets using 536B of memory

RDE hash statistics
path hash: size 131072, 413229 entires
min 0 max 14 avg/std-dev = 3.153/1.749
aspath hash: size 131072, 108607 entires
min 0 max 7 avg/std-dev = 0.829/0.560
attr hash: size 16384, 43526 entires
min 0 max 11 avg/std-dev = 2.657/1.394


after:

[benoit@fw-ipv6onlyorg:/usr/src/usr.sbin/bgpd]$ cat  /tmp/new2 
RDE memory statistics
714521 IPv4 unicast network entries using 27.3M of memory
 58831 IPv6 unicast network entries using 3.1M of memory
   1546515 rib entries using 94.4M of memory
   3093028 prefix entries using 236M of memory
391788 BGP path attribute entries using 26.9M of memory
   and holding 3093028 references
111348 BGP AS-PATH attribute entries using 4.5M of 

Re: bgpd, Adj-RIB-Out support

2018-10-31 Thread Denis Fondras
On Wed, Oct 31, 2018 at 04:24:49PM +0100, Claudio Jeker wrote:
> This diff introduces a real Adj-RIB-Out. It is the minimal change to
> introduce the new RIB. This removes the update_rib introduced before 6.4
> lock because that is now replaced with a real RIB.
> The code used by bgpctl show rib is now a fair amount easier since now one
> RIB runner can be used for all cases.
> path_update() is now returning if a prefix was not modified which is
> needed in the update path to suppress unneeded updates.
> The softreconfig out case becomes simpler because there is no more the
> need to run with both filters and check results.
> Last the shutdown code of the RDE had to be adjusted a bit to still work
> with the Adj-RIB-Out.
> 
> This may cause memory consumption to go up but my testing has shown that
> the result is actually not significant. Needs the commits from earlier
> today to apply.

On my Route Server, it is quite a big change (in percentage).
* Before :
RDE memory statistics
 11561 IPv4 unicast network entries using 452K of memory
   131 IPv6 unicast network entries using 7.2K of memory
 23370 rib entries using 1.4M of memory
 23517 prefix entries using 1.8M of memory
  4894 BGP path attribute entries using 344K of memory
   and holding 23517 references
  1720 BGP AS-PATH attribute entries using 76.4K of memory
   and holding 4894 references
  1061 BGP attributes entries using 41.4K of memory
   and holding 10565 references
  1060 BGP attributes using 25.6K of memory
101809 as-set elements in 58041 tables using 2.1M of memory
114429 prefix-set elments using 4.7M of memory
RIB using 4.1M of memory
Sets using 6.9M of memory

RDE hash statistics
path hash: size 131072, 4894 entires
min 0 max 3 avg/std-dev = 0.037/0.000
aspath hash: size 131072, 1720 entires
min 0 max 2 avg/std-dev = 0.013/0.000
attr hash: size 16384, 1061 entires
min 0 max 2 avg/std-dev = 0.065/0.000

* After:
RDE memory statistics
 11560 IPv4 unicast network entries using 452K of memory
   145 IPv6 unicast network entries using 7.9K of memory
 34991 rib entries using 2.1M of memory
 69844 prefix entries using 5.3M of memory
  4929 BGP path attribute entries using 347K of memory
   and holding 69844 references
  1727 BGP AS-PATH attribute entries using 76.6K of memory
   and holding 4929 references
  1066 BGP attributes entries using 41.6K of memory
   and holding 10643 references
  1065 BGP attributes using 25.6K of memory
101809 as-set elements in 58041 tables using 2.1M of memory
114429 prefix-set elments using 4.7M of memory
RIB using 8.4M of memory
Sets using 6.9M of memory

RDE hash statistics
path hash: size 131072, 4929 entires
min 0 max 3 avg/std-dev = 0.038/0.000
aspath hash: size 131072, 1727 entires
min 0 max 2 avg/std-dev = 0.013/0.000
attr hash: size 16384, 1066 entires
min 0 max 2 avg/std-dev = 0.065/0.000

I need to test it on a router with a fullview.

Comments below.

> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.444
> diff -u -p -r1.444 rde.c
> --- rde.c 31 Oct 2018 14:50:07 -  1.444
> +++ rde.c 31 Oct 2018 15:09:39 -
> @@ -1395,7 +1395,7 @@ rde_update_update(struct rde_peer *peer,
>  
>   /* add original path to the Adj-RIB-In */
>   if (path_update([RIB_ADJ_IN].rib, peer, in, prefix, prefixlen,
> - vstate))
> + vstate) == 1)
>   peer->prefix_cnt++;
>  
>   /* max prefix checker */
> @@ -2124,16 +2124,17 @@ rde_reflector(struct rde_peer *peer, str
>   * control specific functions
>   */
>  static void
> -rde_dump_rib_as(struct prefix *p, struct rde_aspath *asp,
> -struct nexthop *nexthop, pid_t pid, int flags)
> +rde_dump_rib_as(struct prefix *p, struct rde_aspath *asp, pid_t pid, int 
> flags)
>  {
>   struct ctl_show_rib  rib;
>   struct ibuf *wbuf;
>   struct attr *a;
> + struct nexthop  *nexthop;
>   void*bp;
>   time_t   staletime;
>   u_int8_t l;
>  
> + nexthop = prefix_nexthop(p);
>   bzero(, sizeof(rib));
>   rib.lastchange = p->lastchange;
>   rib.local_pref = asp->lpref;
> @@ -2208,70 +2209,37 @@ rde_dump_rib_as(struct prefix *p, struct
>  }
>  
>  static void
> -rde_dump_filterout(struct rde_peer *peer, struct prefix *p,
> -struct ctl_show_rib_request *req)
> -{
> - struct filterstate   state;
> - enum filter_actions  a;
> -
> - if (up_test_update(peer, p) != 1)
> - return;
> -
> - rde_filterstate_prep(, prefix_aspath(p), prefix_nexthop(p),
> - prefix_nhflags(p));
> - a = 

small timespec_get(3) man page typo

2018-10-31 Thread Hiltjo Posthuma
Hi,

Below is a patch to fix a small typo in the man page for timespec_get(3).


diff --git a/lib/libc/gen/timespec_get.3 b/lib/libc/gen/timespec_get.3
index 1092344a666..c698d10926a 100644
--- a/lib/libc/gen/timespec_get.3
+++ b/lib/libc/gen/timespec_get.3
@@ -40,7 +40,7 @@
 .Fn timespec_get "struct timespec *ts" "int base"
 .Sh DESCRIPTION
 The
-.Fn timspec_get
+.Fn timespec_get
 function sets the interval pointed to by
 .Fa ts
 to hold the current calendar time based on the specified time base in

-- 
Kind regards,
Hiltjo



bgpd, Adj-RIB-Out support

2018-10-31 Thread Claudio Jeker
This diff introduces a real Adj-RIB-Out. It is the minimal change to
introduce the new RIB. This removes the update_rib introduced before 6.4
lock because that is now replaced with a real RIB.
The code used by bgpctl show rib is now a fair amount easier since now one
RIB runner can be used for all cases.
path_update() is now returning if a prefix was not modified which is
needed in the update path to suppress unneeded updates.
The softreconfig out case becomes simpler because there is no more the
need to run with both filters and check results.
Last the shutdown code of the RDE had to be adjusted a bit to still work
with the Adj-RIB-Out.

This may cause memory consumption to go up but my testing has shown that
the result is actually not significant. Needs the commits from earlier
today to apply.
-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.444
diff -u -p -r1.444 rde.c
--- rde.c   31 Oct 2018 14:50:07 -  1.444
+++ rde.c   31 Oct 2018 15:09:39 -
@@ -1395,7 +1395,7 @@ rde_update_update(struct rde_peer *peer,
 
/* add original path to the Adj-RIB-In */
if (path_update([RIB_ADJ_IN].rib, peer, in, prefix, prefixlen,
-   vstate))
+   vstate) == 1)
peer->prefix_cnt++;
 
/* max prefix checker */
@@ -2124,16 +2124,17 @@ rde_reflector(struct rde_peer *peer, str
  * control specific functions
  */
 static void
-rde_dump_rib_as(struct prefix *p, struct rde_aspath *asp,
-struct nexthop *nexthop, pid_t pid, int flags)
+rde_dump_rib_as(struct prefix *p, struct rde_aspath *asp, pid_t pid, int flags)
 {
struct ctl_show_rib  rib;
struct ibuf *wbuf;
struct attr *a;
+   struct nexthop  *nexthop;
void*bp;
time_t   staletime;
u_int8_t l;
 
+   nexthop = prefix_nexthop(p);
bzero(, sizeof(rib));
rib.lastchange = p->lastchange;
rib.local_pref = asp->lpref;
@@ -2208,70 +2209,37 @@ rde_dump_rib_as(struct prefix *p, struct
 }
 
 static void
-rde_dump_filterout(struct rde_peer *peer, struct prefix *p,
-struct ctl_show_rib_request *req)
-{
-   struct filterstate   state;
-   enum filter_actions  a;
-
-   if (up_test_update(peer, p) != 1)
-   return;
-
-   rde_filterstate_prep(, prefix_aspath(p), prefix_nexthop(p),
-   prefix_nhflags(p));
-   a = rde_filter(out_rules, peer, p, );
-
-   if (a == ACTION_ALLOW)
-   rde_dump_rib_as(p, , state.nexthop, req->pid,
-   req->flags);
-
-   rde_filterstate_clean();
-}
-
-static void
 rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
 {
-   struct rde_peer *peer;
struct rde_aspath   *asp;
 
-   if (req->flags & F_CTL_ADJ_OUT) {
-   if (p->re->active != p)
-   /* only consider active prefix */
-   return;
-   if (req->peerid) {
-   if ((peer = peer_get(req->peerid)) != NULL)
-   rde_dump_filterout(peer, p, req);
-   return;
-   }
-   } else {
-   asp = prefix_aspath(p);
-   if (req->peerid && req->peerid != prefix_peer(p)->conf.id)
-   return;
-   if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
-   return;
-   if ((req->flags & F_CTL_INVALID) &&
-   (asp->flags & F_ATTR_PARSE_ERR) == 0)
-   return;
-   if (req->type == IMSG_CTL_SHOW_RIB_AS &&
-   !aspath_match(asp->aspath->data, asp->aspath->len,
-   >as, 0))
-   return;
-   if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
-   !community_match(asp, req->community.as,
-   req->community.type))
-   return;
-   if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
-   !community_ext_match(asp, >extcommunity, 0))
-   return;
-   if (req->type == IMSG_CTL_SHOW_RIB_LARGECOMMUNITY &&
-   !community_large_match(asp, req->large_community.as,
-   req->large_community.ld1, req->large_community.ld2))
-   return;
-   if (!ovs_match(p, req->flags))
-   return;
-   rde_dump_rib_as(p, asp, prefix_nexthop(p), req->pid,
-   req->flags);
-   }
+   if (req->peerid && req->peerid != prefix_peer(p)->conf.id)
+   return;
+
+   asp = prefix_aspath(p);
+   if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
+   return;
+   if ((req->flags & F_CTL_INVALID) &&
+   

Re: smtpd(8) match mail-from entire domain

2018-10-31 Thread Gilles Chehade
Sorry for the delay, was catching up.

On Thu, Oct 25, 2018 at 10:32:32PM +0200, Martijn van Duren wrote:
> Back in the old days of the ancient syntax smtpd.conf(5) contained
> the following section:
> sender [!] 
>   If specified, the rule will only be matched if the sender
>   email address is found in the table senders.  The table
>   may contain complete email addresses or apply to an
>   entire domain if prefixed with ???@???.
> 
> This almost worked for me, except when adding @. in my
> sqlite backend (haven't tested with different backends). I reported
> this way back in 2016 and left it at that, but today I had a machine
> at my $DAYJOB that got an annoying amount of spam from a single
> domain that varied in user component and source ip. So filtering on
> domain would've helped a lot.
> 

There was a bug in the mailaddr matching which got fixed a while ago
so this should not be a problem with the smtpd shipped with 6.4


> The following diff implements what the old sender said it would do
> for mail-from and rcpt-to.
> 
> So far only lightly tested on a private server.
> 
> thoughts?
> 

Have you checked that it still doesn't work ??

I've been using the following for many many many months:

 match from any mail-from "@qq.com" for any reject

So as far as I know there's no need for your diff...

   $ nc localhost 25
   220 poolp.org ESMTP OpenSMTPD
   helo localhost
   250 poolp.org Hello localhost [127.0.0.1], pleased to meet you
   mail from:
   250 2.0.0: Ok
   rcpt to:
   550 Invalid recipient
   ^C

The diff would be wrong anyway but that's another story


> Index: ruleset.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/ruleset.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 ruleset.c
> --- ruleset.c 16 Jun 2018 19:41:26 -  1.36
> +++ ruleset.c 25 Oct 2018 20:18:53 -
> @@ -179,6 +179,13 @@ ruleset_match_smtp_mail_from(struct rule
>   table = table_find(env, r->table_smtp_mail_from, NULL);
>   if ((ret = ruleset_match_table_lookup(table, key, K_MAILADDR)) < 0)
>   return -1;
> + if (ret == 0) {
> + if ((key = strchr(key, '@')) == NULL)
> + return 0;
> + ret = ruleset_match_table_lookup(table, key, K_MAILADDR);
> + if (ret < 0)
> + return -1;
> + }
>  
>   return r->flag_smtp_mail_from < 0 ? !ret : ret;
>  }
> @@ -199,6 +206,13 @@ ruleset_match_smtp_rcpt_to(struct rule *
>   table = table_find(env, r->table_smtp_rcpt_to, NULL);
>   if ((ret = ruleset_match_table_lookup(table, key, K_MAILADDR)) < 0)
>   return -1;
> + if (ret == 0) {
> + if ((key = strchr(key, '@')) == NULL)
> + return 0;
> + ret = ruleset_match_table_lookup(table, key, K_MAILADDR);
> + if (ret < 0)
> + return -1;
> + }
>  
>   return r->flag_smtp_rcpt_to < 0 ? !ret : ret;
>  }
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.206
> diff -u -p -r1.206 smtpd.conf.5
> --- smtpd.conf.5  8 Oct 2018 06:10:17 -   1.206
> +++ smtpd.conf.5  25 Oct 2018 20:18:53 -
> @@ -531,6 +531,11 @@ Specify that session's HELO / EHLO shoul
>  .Xc
>  Specify that transactions's MAIL FROM should match the string or list table
>  .Ar sender .
> +The
> +.Ar sender
> +may contain complete email addresses or apply to an entire domain if prefixed
> +with
> +.Sq @ .
>  .It Xo
>  .Op Ic \&!
>  .Cm rcpt\-to
> @@ -538,6 +543,11 @@ Specify that transactions's MAIL FROM sh
>  .Xc
>  Specify that transaction's RCPT TO should match the string or list table
>  .Ar recipient .
> +The
> +.Ar recipient 
> +may contain complete email addresses or apply to an entire domain if prefixed
> +with
> +.Sq @ .
>  .It Xo
>  .Op Ic \&!
>  .Cm tag Ar tag
> 

-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg



Re: bgpd, remove some no longer needed linked lists

2018-10-31 Thread Denis Fondras
On Mon, Oct 29, 2018 at 11:08:12AM +0100, Claudio Jeker wrote:
> On Mon, Oct 29, 2018 at 10:58:36AM +0100, Claudio Jeker wrote:
> > With the last commit the rde_aspath no longer needs to have a list of
> > prefixes and also the peer no longer need to have a list of aspaths.
> > This diff removes both and replaces the list of prefixes with a refcount.
> > The big benefit of this is that struct rde_aspath can now be shared
> > between sessions and ribs. This is an important set for Adj-RIB-Out
> > support.
> > 
> 
> And here is the bgpctl diff to print out the number of references hold on
> the rde_aspath structs.
> 

OK denis@

> -- 
> :wq Claudio
> 
> Index: bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.220
> diff -u -p -r1.220 bgpctl.c
> --- bgpctl.c  26 Oct 2018 16:54:53 -  1.220
> +++ bgpctl.c  29 Oct 2018 09:43:34 -
> @@ -1864,6 +1864,8 @@ show_rib_memory_msg(struct imsg *imsg)
>   printf("%10lld BGP path attribute entries using %s of memory\n",
>   (long long)stats.path_cnt, fmt_mem(stats.path_cnt *
>   sizeof(struct rde_aspath)));
> + printf("\t   and holding %lld references\n",
> + (long long)stats.path_refs);
>   printf("%10lld BGP AS-PATH attribute entries using "
>   "%s of memory,\n\t   and holding %lld references\n",
>   (long long)stats.aspath_cnt, fmt_mem(stats.aspath_size),
> 



Re: libunwind: fix register numbering on OpenBSD/i386 (again)

2018-10-31 Thread Mark Kettenis
> Date: Wed, 31 Oct 2018 10:14:29 +0100
> From: Patrick Wildt 
> 
> On Wed, Oct 31, 2018 at 10:03:57AM +0100, Mark Kettenis wrote:
> > > Date: Wed, 31 Oct 2018 09:54:35 +0100
> > > From: Patrick Wildt 
> > > 
> > > Hi,
> > > 
> > > during the libunwind update to 6.0 we lost a particular patchset that
> > > fixes register numbering for OpenBSD/i386, thus breaking exception
> > > handling on that platform.  Looks like no one noticed until now.
> > > 
> > > ok?
> > 
> > I don't think so.  The code in src/Registers.hpp effectively swaps the
> > two registers on !__APPLE__ platforms.  Is that not working?
> 
> Good point, I missed that.  Well, there's something buggy.  Will dig
> more and come back again.

According to http://bluhm.genua.de/regress/results/regress.html the
misc/exception regress test still passes on i386.



Re: libunwind: fix register numbering on OpenBSD/i386 (again)

2018-10-31 Thread Patrick Wildt
On Wed, Oct 31, 2018 at 10:03:57AM +0100, Mark Kettenis wrote:
> > Date: Wed, 31 Oct 2018 09:54:35 +0100
> > From: Patrick Wildt 
> > 
> > Hi,
> > 
> > during the libunwind update to 6.0 we lost a particular patchset that
> > fixes register numbering for OpenBSD/i386, thus breaking exception
> > handling on that platform.  Looks like no one noticed until now.
> > 
> > ok?
> 
> I don't think so.  The code in src/Registers.hpp effectively swaps the
> two registers on !__APPLE__ platforms.  Is that not working?

Good point, I missed that.  Well, there's something buggy.  Will dig
more and come back again.



Re: libunwind: fix register numbering on OpenBSD/i386 (again)

2018-10-31 Thread Mark Kettenis
> Date: Wed, 31 Oct 2018 09:54:35 +0100
> From: Patrick Wildt 
> 
> Hi,
> 
> during the libunwind update to 6.0 we lost a particular patchset that
> fixes register numbering for OpenBSD/i386, thus breaking exception
> handling on that platform.  Looks like no one noticed until now.
> 
> ok?

I don't think so.  The code in src/Registers.hpp effectively swaps the
two registers on !__APPLE__ platforms.  Is that not working?


> diff --git a/lib/libunwind/include/libunwind.h 
> b/lib/libunwind/include/libunwind.h
> index 29cf62e4335..4ff0c01b4e2 100644
> --- a/lib/libunwind/include/libunwind.h
> +++ b/lib/libunwind/include/libunwind.h
> @@ -164,8 +164,13 @@ enum {
>UNW_X86_ECX = 1,
>UNW_X86_EDX = 2,
>UNW_X86_EBX = 3,
> +#ifdef __OpenBSD__
> +  UNW_X86_ESP = 4,
> +  UNW_X86_EBP = 5,
> +#else
>UNW_X86_EBP = 4,
>UNW_X86_ESP = 5,
> +#endif
>UNW_X86_ESI = 6,
>UNW_X86_EDI = 7
>  };
> 
> 



Re: libunwind: fix register numbering on OpenBSD/i386 (again)

2018-10-31 Thread Robert Nagy
On 31/10/18 09:54 +0100, Patrick Wildt wrote:
> Hi,
> 
> during the libunwind update to 6.0 we lost a particular patchset that
> fixes register numbering for OpenBSD/i386, thus breaking exception
> handling on that platform.  Looks like no one noticed until now.
> 
> ok?
> 
> Patrick
> 
> diff --git a/lib/libunwind/include/libunwind.h 
> b/lib/libunwind/include/libunwind.h
> index 29cf62e4335..4ff0c01b4e2 100644
> --- a/lib/libunwind/include/libunwind.h
> +++ b/lib/libunwind/include/libunwind.h
> @@ -164,8 +164,13 @@ enum {
>UNW_X86_ECX = 1,
>UNW_X86_EDX = 2,
>UNW_X86_EBX = 3,
> +#ifdef __OpenBSD__
> +  UNW_X86_ESP = 4,
> +  UNW_X86_EBP = 5,
> +#else
>UNW_X86_EBP = 4,
>UNW_X86_ESP = 5,
> +#endif
>UNW_X86_ESI = 6,
>UNW_X86_EDI = 7
>  };
> 

I think kettenis told me to remove this because they fixed it elsewhere.



libunwind: fix register numbering on OpenBSD/i386 (again)

2018-10-31 Thread Patrick Wildt
Hi,

during the libunwind update to 6.0 we lost a particular patchset that
fixes register numbering for OpenBSD/i386, thus breaking exception
handling on that platform.  Looks like no one noticed until now.

ok?

Patrick

diff --git a/lib/libunwind/include/libunwind.h 
b/lib/libunwind/include/libunwind.h
index 29cf62e4335..4ff0c01b4e2 100644
--- a/lib/libunwind/include/libunwind.h
+++ b/lib/libunwind/include/libunwind.h
@@ -164,8 +164,13 @@ enum {
   UNW_X86_ECX = 1,
   UNW_X86_EDX = 2,
   UNW_X86_EBX = 3,
+#ifdef __OpenBSD__
+  UNW_X86_ESP = 4,
+  UNW_X86_EBP = 5,
+#else
   UNW_X86_EBP = 4,
   UNW_X86_ESP = 5,
+#endif
   UNW_X86_ESI = 6,
   UNW_X86_EDI = 7
 };



Re: Qcow2: Clean up logging/error handling

2018-10-31 Thread Mike Larkin
On Tue, Oct 30, 2018 at 10:41:21PM -0700, o...@eigenstate.org wrote:
> > On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote:
> >> On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin  
> >> wrote:
> >> 
> >> > > -  if (disk->base->clustersz != disk->clustersz) {
> >> > > -  log_warn("%s: all disks must share clustersize",
> >> > > +  fatal("%s: could not open %s", basepath, 
> >> > > __func__);
> >> > 
> >> > is this right?
> >> > 
> >> > > +  if (qc2_open(disk->base, fds + 1, nfd - 1) == -1)
> >> > > +  fatalx("%s: could not open %s", basepath, 
> >> > > __func__);
> >> > 
> >> > same
> >> > 
> >> > > +  if (disk->base->clustersz != disk->clustersz)
> >> > > +  fatalx("%s: all disk parts must share 
> >> > > clustersize",
> >> > >__func__);
> >> > > -  goto error;
> >> > > -  }
> >> > > -  }
> >> 
> >> Good question. I think from earlier discussion, we wanted to fail if we 
> >> could
> >> not open a disk image -- especially since these ones are actually *parts* 
> >> of
> >> a disk image, meaning that we've got a corrupt image.
> >> 
> >> I can easily change these back to warns + error returns.
> >> 
> >> -- 
> >> Ori Bernstein
> > 
> > I was referring to the argument order.
> 
> Oh, that's obviously wrong. Updated.
> 

with that fix, go for it. we can hack on it further in-tree.

> diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
> index 6e7ed279d6b..0860ad0e7d2 100644
> --- usr.sbin/vmd/config.c
> +++ usr.sbin/vmd/config.c
> @@ -284,8 +284,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, 
> uint32_t peerid, uid_t uid)
>*/
>   if (kernfd == -1 && !vmboot &&
>   (kernfd = open(VM_DEFAULT_BIOS, O_RDONLY)) == -1) {
> - log_warn("%s: can't open %s", __func__,
> - VM_DEFAULT_BIOS);
> + log_warn("can't open %s", VM_DEFAULT_BIOS);
>   errno = VMD_BIOS_MISSING;
>   goto fail;
>   }
> @@ -305,8 +304,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, 
> uint32_t peerid, uid_t uid)
>   /* Stat cdrom to ensure it is a regular file */
>   if ((cdromfd =
>   open(vcp->vcp_cdrom, O_RDONLY)) == -1) {
> - log_warn("%s: can't open cdrom %s", __func__,
> - vcp->vcp_cdrom);
> + log_warn("can't open cdrom %s", vcp->vcp_cdrom);
>   errno = VMD_CDROM_MISSING;
>   goto fail;
>   }
> @@ -325,14 +323,14 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, 
> uint32_t peerid, uid_t uid)
>   for (i = 0 ; i < vcp->vcp_ndisks; i++) {
>   if (strlcpy(path, vcp->vcp_disks[i], sizeof(path))
>  >= sizeof(path))
> - log_warnx("%s, disk path too long", __func__);
> + log_warnx("disk path %s too long", vcp->vcp_disks[i]);
>   memset(vmc->vmc_diskbases, 0, sizeof(vmc->vmc_diskbases));
>   oflags = O_RDWR|O_EXLOCK|O_NONBLOCK;
>   aflags = R_OK|W_OK;
>   for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) {
>   /* Stat disk[i] to ensure it is a regular file */
>   if ((diskfds[i][j] = open(path, oflags)) == -1) {
> - log_warn("%s: can't open disk %s", __func__,
> + log_warn("can't open disk %s",
>   vcp->vcp_disks[i]);
>   errno = VMD_DISK_MISSING;
>   goto fail;
> @@ -487,7 +485,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, 
> uint32_t peerid, uid_t uid)
>  
>   fail:
>   saved_errno = errno;
> - log_warnx("%s: failed to start vm %s", __func__, vcp->vcp_name);
> + log_warnx("failed to start vm %s", vcp->vcp_name);
>  
>   if (kernfd != -1)
>   close(kernfd);
> diff --git usr.sbin/vmd/vioqcow2.c usr.sbin/vmd/vioqcow2.c
> index 28b087df39a..af762fcdf45 100644
> --- usr.sbin/vmd/vioqcow2.c
> +++ usr.sbin/vmd/vioqcow2.c
> @@ -102,9 +102,9 @@ struct qcdisk {
>  extern char *__progname;
>  
>  static off_t xlate(struct qcdisk *, off_t, int *);
> -static int copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
> +static void copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
> +static void inc_refs(struct qcdisk *, off_t, int);
>  static off_t mkcluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
> -static int inc_refs(struct qcdisk *, off_t, int);
>  static int qc2_open(struct qcdisk *, int *, size_t);
>  static ssize_t qc2_pread(void *, char *, size_t, off_t);
>  static ssize_t qc2_pwrite(void *, char *, size_t, off_t);
> @@ -126,7 +126,7 @@ virtio_init_qcow2(struct