On Fri, Dec 28, 2018 at 07:50:08PM +0100, Denis Fondras wrote:
> On Fri, Dec 28, 2018 at 06:08:16PM +0100, Klemens Nanni wrote:
> > On Fri, Dec 28, 2018 at 05:21:02PM +0100, Denis Fondras wrote:
> > > int
> > > +krVPN6_change(struct ktable *kt, struct kroute_full *kl, u_int8_t
> > > fib_prio)
> > > +{
> > > + struct kroute6_node *kr6;
> > > + struct in6_addr lo6 = IN6ADDR_LOOPBACK_INIT;
> > > + int action = RTM_ADD;
> > > + u_int32_t mplslabel = 0;
> > > + u_int16_t labelid;
> > > +
> > > + if ((kr6 = kroute6_find(kt, &kl->prefix.vpn6.addr, kl->prefixlen,
> > > + fib_prio)) != NULL)
> > > + action = RTM_CHANGE;
> > Can this be moved below the conditional returns or does kroute6_find()
> > have side effects? `actions' is not used until much later.
> >
> > > + /* nexthop to loopback -> ignore silently */
> > > + if (IN6_IS_ADDR_LOOPBACK(&kl->nexthop.v6))
> > > + return (0);
> > > +
> > > + /* only single MPLS label are supported for now */
> > > + if (kl->prefix.vpn6.labellen != 3) {
> > > + log_warnx("%s: %s/%u has not a single label", __func__,
> > > + log_addr(&kl->prefix), kl->prefixlen);
> > > + return (0);
> > > + }
> > Here.
> >
> > > + mplslabel = (kl->prefix.vpn6.labelstack[0] << 24) |
> > > + (kl->prefix.vpn6.labelstack[1] << 16) |
> > > + (kl->prefix.vpn6.labelstack[2] << 8);
> > > + mplslabel = htonl(mplslabel);
> > > +
> > > + /* for blackhole and reject routes nexthop needs to be ::1 */
> > > + if (kl->flags & (F_BLACKHOLE|F_REJECT))
> > > + bcopy(&lo6, &kl->nexthop.v6, sizeof(kl->nexthop.v6));
> > > +
> > > + labelid = rtlabel_name2id(kl->label);
> > Or even here right before it's used. `kr6' is not used above.
> >
> > > + if (action == RTM_ADD) {
>
> Yes, we can, it may save a few cycles. Here is a diff to update the current
> revision (I updated my MPLS diff with similar change).
>
> Index: kroute.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.226
> diff -u -p -r1.226 kroute.c
> --- kroute.c 6 Dec 2018 13:04:40 -0000 1.226
> +++ kroute.c 28 Dec 2018 18:46:07 -0000
> @@ -487,10 +487,6 @@ kr4_change(struct ktable *kt, struct kro
> int action = RTM_ADD;
> u_int16_t labelid;
>
> - if ((kr = kroute_find(kt, kl->prefix.v4.s_addr, kl->prefixlen,
> - fib_prio)) != NULL)
> - action = RTM_CHANGE;
> -
> /* for blackhole and reject routes nexthop needs to be 127.0.0.1 */
> if (kl->flags & (F_BLACKHOLE|F_REJECT))
> kl->nexthop.v4.s_addr = htonl(INADDR_LOOPBACK);
> @@ -501,6 +497,10 @@ kr4_change(struct ktable *kt, struct kro
>
> labelid = rtlabel_name2id(kl->label);
>
> + if ((kr = kroute_find(kt, kl->prefix.v4.s_addr, kl->prefixlen,
> + fib_prio)) != NULL)
> + action = RTM_CHANGE;
> +
> if (action == RTM_ADD) {
> if ((kr = calloc(1, sizeof(struct kroute_node))) == NULL) {
> log_warn("%s", __func__);
> @@ -545,10 +545,6 @@ kr6_change(struct ktable *kt, struct kro
> int action = RTM_ADD;
> u_int16_t labelid;
>
> - if ((kr6 = kroute6_find(kt, &kl->prefix.v6, kl->prefixlen, fib_prio)) !=
> - NULL)
> - action = RTM_CHANGE;
> -
> /* for blackhole and reject routes nexthop needs to be ::1 */
> if (kl->flags & (F_BLACKHOLE|F_REJECT))
> bcopy(&lo6, &kl->nexthop.v6, sizeof(kl->nexthop.v6));
> @@ -558,6 +554,10 @@ kr6_change(struct ktable *kt, struct kro
>
> labelid = rtlabel_name2id(kl->label);
>
> + if ((kr6 = kroute6_find(kt, &kl->prefix.v6, kl->prefixlen, fib_prio)) !=
> + NULL)
> + action = RTM_CHANGE;
> +
> if (action == RTM_ADD) {
> if ((kr6 = calloc(1, sizeof(struct kroute6_node))) == NULL) {
> log_warn("%s", __func__);
> @@ -604,10 +604,6 @@ krVPN4_change(struct ktable *kt, struct
> u_int32_t mplslabel = 0;
> u_int16_t labelid;
>
> - if ((kr = kroute_find(kt, kl->prefix.vpn4.addr.s_addr, kl->prefixlen,
> - fib_prio)) != NULL)
> - action = RTM_CHANGE;
> -
> /* nexthop within 127/8 -> ignore silently */
> if ((kl->nexthop.v4.s_addr & htonl(IN_CLASSA_NET)) ==
> htonl(INADDR_LOOPBACK & IN_CLASSA_NET))
> @@ -629,6 +625,10 @@ krVPN4_change(struct ktable *kt, struct
> /* for blackhole and reject routes nexthop needs to be 127.0.0.1 */
> if (kl->flags & (F_BLACKHOLE|F_REJECT))
> kl->nexthop.v4.s_addr = htonl(INADDR_LOOPBACK);
> +
> + if ((kr = kroute_find(kt, kl->prefix.vpn4.addr.s_addr, kl->prefixlen,
> + fib_prio)) != NULL)
> + action = RTM_CHANGE;
>
> if (action == RTM_ADD) {
> if ((kr = calloc(1, sizeof(struct kroute_node))) == NULL) {
>
OK claudio@
--
:wq Claudio
