Re: bgpd: next round of kroute cleanup

2022-06-16 Thread Claudio Jeker 
On Thu, Jun 16, 2022 at 03:28:58PM +0200, Theo Buehler wrote:
> On Thu, Jun 16, 2022 at 02:36:28PM +0200, Claudio Jeker wrote:
> > This diff kills external use of prefixlen2mask() and uses inet4applymask()
> > instead.  With this the IPv4 and IPv6 code is more similar.
> > Also I feel the code is a bit easier to read.
> >
> > Also kroute{,6}_match() is changed to take a struct bgpd_addr *.
> > This is another step towards removing lots of copy paste code.
> 
> I agree that it's easier to follow and cleaner after the diff.
> The tree you used to isn't quite up to date - there were some offsets.

That is quite possible.

> ok tb

Thanks

> [...]
> 
> > +const struct in_addr   inet4allone = { INADDR_BROADCAST };
> > +const struct in6_addr  inet6allone = {{{ 0xff, 0xff, 0xff, 0xff,
> > + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > + 0xff, 0xff, 0xff, 0xff }}};
> 
> Any reason not to make these static?

Oh, yeah I should. No need to export them. 

-- 
:wq Claudio

Re: bgpd: next round of kroute cleanup

2022-06-16 Thread Theo Buehler 
On Thu, Jun 16, 2022 at 02:36:28PM +0200, Claudio Jeker wrote:
> This diff kills external use of prefixlen2mask() and uses inet4applymask()
> instead.  With this the IPv4 and IPv6 code is more similar.
> Also I feel the code is a bit easier to read.
>
> Also kroute{,6}_match() is changed to take a struct bgpd_addr *.
> This is another step towards removing lots of copy paste code.

I agree that it's easier to follow and cleaner after the diff.
The tree you used to isn't quite up to date - there were some offsets.

ok tb

[...]

> +const struct in_addr inet4allone = { INADDR_BROADCAST };
> +const struct in6_addrinet6allone = {{{ 0xff, 0xff, 0xff, 0xff,
> + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> + 0xff, 0xff, 0xff, 0xff }}};

Any reason not to make these static?

bgpd: next round of kroute cleanup

2022-06-16 Thread Claudio Jeker 
This diff kills external use of prefixlen2mask() and uses inet4applymask()
instead.  With this the IPv4 and IPv6 code is more similar.
Also I feel the code is a bit easier to read.

Also kroute{,6}_match() is changed to take a struct bgpd_addr *.
This is another step towards removing lots of copy paste code.

Next step will be to change kroute{,6}_find() to use struct bgpd_addr *.
-- 
:wq Claudio

Index: usr.sbin/bgpctl/parser.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.109
diff -u -p -r1.109 parser.c
--- usr.sbin/bgpctl/parser.c21 Mar 2022 10:16:23 -  1.109
+++ usr.sbin/bgpctl/parser.c16 Jun 2022 12:28:59 -
@@ -968,7 +968,7 @@ parse_prefix(const char *word, size_t wo
mask = 32;
if (mask > 32)
errx(1, "invalid netmask: too large");
-   addr->v4.s_addr = addr->v4.s_addr & htonl(prefixlen2mask(mask));
+   inet4applymask(&addr->v4, &addr->v4, mask);
break;
case AID_INET6:
if (mask == -1)
Index: usr.sbin/bgpd/bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.428
diff -u -p -r1.428 bgpd.h
--- usr.sbin/bgpd/bgpd.h9 Jun 2022 16:45:19 -   1.428
+++ usr.sbin/bgpd/bgpd.h16 Jun 2022 12:28:59 -
@@ -1304,7 +1304,6 @@ void   kr_show_route(struct imsg *);
 voidkr_ifinfo(char *);
 voidkr_net_reload(u_int, uint64_t, struct network_head *);
 int kr_reload(void);
-struct in6_addr*prefixlen2mask6(uint8_t prefixlen);
 int get_mpe_config(const char *, u_int *, u_int *);
 
 /* log.c */
@@ -1422,7 +1421,6 @@ intnlri_get_vpn6(u_char *, uint16_t, 
 uint8_t *, int);
 int prefix_compare(const struct bgpd_addr *,
const struct bgpd_addr *, int);
-in_addr_t   prefixlen2mask(uint8_t);
 voidinet4applymask(struct in_addr *, const struct in_addr *, int);
 voidinet6applymask(struct in6_addr *, const struct in6_addr *,
int);
Index: usr.sbin/bgpd/kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.255
diff -u -p -r1.255 kroute.c
--- usr.sbin/bgpd/kroute.c  14 Jun 2022 14:06:48 -  1.255
+++ usr.sbin/bgpd/kroute.c  16 Jun 2022 12:28:59 -
@@ -167,8 +167,9 @@ void knexthop_validate(struct 
ktable 
struct knexthop_node *);
 voidknexthop_track(struct ktable *, void *);
 voidknexthop_send_update(struct knexthop_node *);
-struct kroute_node *kroute_match(struct ktable *, in_addr_t, int);
-struct kroute6_node*kroute6_match(struct ktable *, struct in6_addr *, int);
+struct kroute_node *kroute_match(struct ktable *, struct bgpd_addr *, int);
+struct kroute6_node*kroute6_match(struct ktable *, struct bgpd_addr *,
+   int);
 voidkroute_detach_nexthop(struct ktable *,
struct knexthop_node *);
 
@@ -210,6 +211,12 @@ RB_GENERATE(kif_tree, kif_node, entry, k
 
 #define KT2KNT(x)  (&(ktable_get((x)->nhtableid)->knt))
 
+const struct in_addr   inet4allone = { INADDR_BROADCAST };
+const struct in6_addr  inet6allone = {{{ 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff }}};
+
+
 /*
  * exported functions
  */
@@ -1112,14 +1119,14 @@ kr_show_route(struct imsg *imsg)
kr = NULL;
switch (addr->aid) {
case AID_INET:
-   kr = kroute_match(kt, addr->v4.s_addr, 1);
+   kr = kroute_match(kt, addr, 1);
if (kr != NULL)
send_imsg_session(IMSG_CTL_KROUTE,
imsg->hdr.pid, kr_tofull(&kr->r),
sizeof(struct kroute_full));
break;
case AID_INET6:
-   kr6 = kroute6_match(kt, &addr->v6, 1);
+   kr6 = kroute6_match(kt, addr, 1);
if (kr6 != NULL)
send_imsg_session(IMSG_CTL_KROUTE,
imsg->hdr.pid, kr6_tofull(&kr6->r),
@@ -1800,7 +1807,7 @@ kroute_insert(struct ktable *kt, struct 
 {
struct kroute_node  *krm;
struct knexthop_node*h;
-   in_addr_tmask, ina;
+   struct in_addr   ina, inb;
 
if ((krm = RB_INSERT(kroute_tree, &kt->krt, kr)) != NULL) {
/* multipath route, add at end of list */
@@ -1812,12 +1819,14 @@ kroute_insert(struct ktable *kt, struct