Re: bgpd refactor aspath_match a bit
On Tue, Nov 27, 2018 at 06:55:51PM +0100, Job Snijders wrote:
> On Tue, Nov 27, 2018 at 06:23:53PM +0100, Claudio Jeker wrote:
> > On Tue, Nov 27, 2018 at 04:21:53PM +0100, Job Snijders wrote:
> > > On Fri, Nov 23, 2018 at 03:55:18PM +0100, Claudio Jeker wrote:
> > > > For origin validation I chacked the source_as in struct rde_aspath
> > > > this is not really the right place. It should be in struct aspath
> > > > since that holds all the ASPATH related stuff. Change this, move
> > > > aspath_match out of util.c back into rde_attr.c and adjust code to use
> > > > the cached value also in match from any source-as XYZ rules.
> > > > This last bit causes a minor behavioural change since the old code
> > > > extracted the last non AS_SET asnumber. The new code follows the ROA
> > > > RFC and returns the rightmost AS for AS_SEQUENCE, the local AS for
> > > > empty paths and AS_NONE (which is 0) for everything else.
> > > > So now 'match from any source-as 0' will return all paths that do not
> > > > have a final AS_SEQUENCE segment.
> > > >
> > > > The reason for this change is that I don't want to have two different
> > > > behaviours for what we call source-as (the one in roa-set and the one
> > > > on a
> > > > filter).
> > >
> > > Something is off, it seems 'source-as 0' is matching anything that has
> > > an AS_SET attribute set:
> > >
> > > $ bgpctl show rib source-as 0 | head
> > > flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
> > >S = Stale, E = Error
> > > origin validation state: N = not-found, V = valid, ! = invalid
> > > origin: i = IGP, e = EGP, ? = Incomplete
> > >
> > > flags ovs destination gateway lpref med aspath
> > > origin
> > > I*> N 5.39.176.0/21192.147.168.1 100 0 2914 8530
> > > { 198753 } ?
> > > I*> N 5.101.110.0/24 192.147.168.1 100 0 2914
> > > 14061 { 46652 } i
> > > I*> N 5.175.0.0/19 192.147.168.1 100 0 2914 1299
> > > 20773 { 8972 } i
> > > I*> N 8.41.202.0/24192.147.168.1 100 0 2914
> > > 13789 30372 { 40179 } i
> > >
> > > Similarly, this should return at least 5.39.176.0/21:
> > >
> > > $ bgpctl show rib source-as 8530
> > > flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
> > >S = Stale, E = Error
> > > origin validation state: N = not-found, V = valid, ! = invalid
> > > origin: i = IGP, e = EGP, ? = Incomplete
> > >
> > > flags ovs destination gateway lpref med aspath
> > > origin
> > > I*> N 80.87.16.0/20192.147.168.1 100 0 2914 8530
> > > ?
> > > I*> N 87.236.128.0/21 192.147.168.1 100 0 2914 8530
> > > ?
> > > I*> N 88.151.152.0/21 192.147.168.1 100 0 2914 8530
> > > ?
> > > I*> N 89.38.120.0/21 192.147.168.1 100 0 2914 8530
> > > i
> > > I*> N 93.115.176.0/20 192.147.168.1 100 0 2914 8530
> > > i
> > > I*> N 185.52.144.0/22 192.147.168.1 100 0 2914 8530
> > > ?
> > >
> >
> > I implemented source-as the way ROA is defining it. So anything which ends
> > with a AS_SET will return AS_NONE (which is 0). OpenBGPD has no way to
> > have an AS_PATH that has a real 0 in the AS_PATH (those UPDATES are
> > treated as withdraw). Because of this also the 5.39.176.0/21 is no longer
> > matching in 'bgpctl show rib source-as 8530'.
>
> I'm not sure it should behave that way.
>
> 'bgpctl show rib source-as 8530' really ought to return prefixes like
> 80.87.16.0/20 but also 5.39.176.0/21.
But isn't this different from other implementations? At least I would
expect that the AS-path regex '8530$' would not match on the AS_SET path
either. My issue is that we have 'source-as' in roa-set, origin-set and on
filters in bgpd.conf plus the source-as used by bgpctl. Depending on
context they behave differently. So if AS 8530 is in the roa-set
and I do bgpctl show rib source-as 8530 the result will be different to
what would match in the roa-set.
We already had a lot of confusion about announce and that is why I decided
to make them behave the same.
> > I'm a bit on the edge here about where to go and currently prefer to
> > follow a RFC (which in this case is RFC6811).
> >
> > o Route Origin ASN: The origin AS number derived from a Route as
> > follows:
> >
> > * the rightmost AS in the final segment of the AS_PATH attribute
> > in the Route if that segment is of type AS_SEQUENCE, or
> >
> > * the BGP speaker's own AS number if that segment is of type
> >AS_CONFED_SEQUENCE or AS_CONFED_SET or if the AS_PATH is empty,
> >or
> >
> > * the distinguished value "NONE" if the final segment of the
> >AS_PATH attribute is of any other type.
> >
> > As mentioned above I found it strange when behaviour is different because
> > of where it is used.
>
> RFC 6811
Re: cleanup decision process
On Thu, Nov 22, 2018 at 04:52:08PM +0100, Claudio Jeker wrote:
> Since a while announcements added by bgpctl will overwrite the ones from
> the config and no longer live next to each other. Because of this step 13
> in the decision process is no longer needed.
>
> OK?
Ping
--
:wq Claudio
Index: bgpd.8
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v
retrieving revision 1.59
diff -u -p -r1.59 bgpd.8
--- bgpd.8 4 Nov 2018 14:34:00 - 1.59
+++ bgpd.8 22 Nov 2018 14:50:47 -
@@ -112,10 +112,6 @@ If it is not present then a length of 0
.It
The path coming from the peer with the lowest IP address is selected.
IPv4 sessions will be preferred over IPv6 ones.
-.It
-In case of locally announced prefixes
-.Nm
-will prefer statically set prefixes over dynamically inserted ones.
.El
.Pp
Attributes set by filters can be used to tip the decision process to prefer
Index: rde_decide.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_decide.c,v
retrieving revision 1.72
diff -u -p -r1.72 rde_decide.c
--- rde_decide.c27 Sep 2018 15:53:14 - 1.72
+++ rde_decide.c22 Nov 2018 14:46:42 -
@@ -229,16 +229,7 @@ prefix_cmp(struct prefix *p1, struct pre
return (-memcmp(&peer1->remote_addr, &peer2->remote_addr,
sizeof(peer1->remote_addr)));
- /* 13. for announced prefixes prefer dynamic routes */
- if ((asp1->flags & F_ANN_DYNAMIC) != (asp2->flags & F_ANN_DYNAMIC)) {
- if (asp1->flags & F_ANN_DYNAMIC)
- return (1);
- else
- return (-1);
- }
-
fatalx("Uh, oh a politician in the decision process");
- /* NOTREACHED */
}
/*
Re: top: allow reverse sort order
On Wed, Nov 28, 2018 at 12:07:37AM +0100, Klemens Nanni wrote:
> On Tue, Nov 27, 2018 at 03:52:52PM -0600, Scott Cheloha wrote:
> > > > static int
> > > > +getorder(char *field)
> > > > +{
> > > > + rev_order = field[0] == '-';
> > > > +
> > > > + return string_index(rev_order ? field + 1 : field,
> > > > statics.order_names);
> > > > +}
> > > > +
> >
> > You need to check that the field has an ordering before potentially
> > modifying rev_order, otherwise a bogus input will have a side effect.
> >
> > For example, with the current code the input "o -notafield" reverses
> > the ordering.
> Good catch.
>
> Updated diff to fix that, rest unchanged.
>
> Note how an empty field is silently treated as the default field
> "state", but that's an independent issue I'd like to address in a
> separate diff for string_index().
>
> OK?
ok cheloha@
> Index: display.c
> ===
> RCS file: /cvs/src/usr.bin/top/display.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 display.c
> --- display.c 17 Nov 2018 23:10:08 - 1.57
> +++ display.c 27 Nov 2018 21:09:56 -
> @@ -817,7 +817,8 @@ show_help(void)
> "I | i- toggle the display of idle processes\n"
> "k [-sig] pid - send signal `-sig' to process `pid'\n"
> "n|# count- show `count' processes\n"
> - "o field - specify sort order (size, res, cpu, time, pri, pid,
> command)\n"
> + "o [-]field - specify sort order (size, res, cpu, time, pri, pid,
> command)\n"
> + " (o -field sorts in reverse)\n"
> "P pid- highlight process `pid' (P+ switches highlighting
> off)\n"
> "p pid- display process by `pid' (p+ selects all
> processes)\n"
> "q- quit\n"
> Index: machine.c
> ===
> RCS file: /cvs/src/usr.bin/top/machine.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 machine.c
> --- machine.c 17 Nov 2018 23:10:08 - 1.95
> +++ machine.c 27 Nov 2018 21:09:56 -
> @@ -602,6 +602,8 @@ static unsigned char sorted_state[] =
> 1 /* zombie*/
> };
>
> +extern int rev_order;
> +
> /*
> * proc_compares - comparison functions for "qsort"
> */
> @@ -631,6 +633,17 @@ static unsigned char sorted_state[] =
> #define ORDERKEY_CMD \
> if ((result = strcmp(p1->p_comm, p2->p_comm)) == 0)
>
> +/* remove one level of indirection and set sort order */
> +#define SETORDER do { \
> + if (rev_order) { \
> + p1 = *(struct kinfo_proc **) pp2; \
> + p2 = *(struct kinfo_proc **) pp1; \
> + } else { \
> + p1 = *(struct kinfo_proc **) pp1; \
> + p2 = *(struct kinfo_proc **) pp2; \
> + } \
> + } while (0)
> +
> /* compare_cpu - the comparison function for sorting by cpu percentage */
> static int
> compare_cpu(const void *v1, const void *v2)
> @@ -640,9 +653,7 @@ compare_cpu(const void *v1, const void *
> struct kinfo_proc *p1, *p2;
> int result;
>
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>
> ORDERKEY_PCTCPU
> ORDERKEY_CPUTIME
> @@ -663,9 +674,7 @@ compare_size(const void *v1, const void
> struct kinfo_proc *p1, *p2;
> int result;
>
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>
> ORDERKEY_MEM
> ORDERKEY_RSSIZE
> @@ -686,9 +695,7 @@ compare_res(const void *v1, const void *
> struct kinfo_proc *p1, *p2;
> int result;
>
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>
> ORDERKEY_RSSIZE
> ORDERKEY_MEM
> @@ -709,9 +716,7 @@ compare_time(const void *v1, const void
> struct kinfo_proc *p1, *p2;
> int result;
>
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>
> ORDERKEY_CPUTIME
> ORDERKEY_PCTCPU
> @@ -732,9 +737,7 @@ compare_prio(const void *v1, const void
> struct kinfo_proc *p1, *p2;
> int result;
>
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>
> ORDERKEY_PRIO
> ORDERKEY_PCTCPU
> @@ -754,9 +757,7 @@ compare_pid(const void *v1, const void *
> struct kinfo_proc *p1, *p2;
> int result;
>
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>
> ORDERKEY_PID
> ORDERK
free(9) sizes for hooks
Trivial sizes, ok?
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.567
diff -u -p -r1.567 if.c
--- net/if.c12 Nov 2018 23:41:22 - 1.567
+++ net/if.c28 Nov 2018 20:57:21 -
@@ -1088,9 +1088,9 @@ if_detach(struct ifnet *ifp)
}
}
- free(ifp->if_addrhooks, M_TEMP, 0);
- free(ifp->if_linkstatehooks, M_TEMP, 0);
- free(ifp->if_detachhooks, M_TEMP, 0);
+ free(ifp->if_addrhooks, M_TEMP, sizeof(*ifp->if_addrhooks));
+ free(ifp->if_linkstatehooks, M_TEMP, sizeof(*ifp->if_linkstatehooks));
+ free(ifp->if_detachhooks, M_TEMP, sizeof(*ifp->if_detachhooks));
for (i = 0; (dp = domains[i]) != NULL; i++) {
if (dp->dom_ifdetach && ifp->if_afdata[dp->dom_family])
free(9) sizes for autoconf
This one is a bit tricky. In the indirect case, config_make_softc()
allocates a chunk corresponding to a given `cf->cf_attach->ca_devsize'.
ok?
Index: kern/subr_autoconf.c
===
RCS file: /cvs/src/sys/kern/subr_autoconf.c,v
retrieving revision 1.92
diff -u -p -r1.92 subr_autoconf.c
--- kern/subr_autoconf.c14 Mar 2016 23:08:06 - 1.92
+++ kern/subr_autoconf.c18 Nov 2018 16:07:04 -
@@ -154,13 +154,15 @@ mapply(struct matchinfo *m, struct cfdat
pri);
if (pri > m->pri) {
- if (m->indirect && m->match)
- free(m->match, M_DEVBUF, 0);
+ if (m->indirect && m->match) {
+ cf = ((struct device *)m->match)->dv_cfdata;
+ free(m->match, M_DEVBUF, cf->cf_attach->ca_devsize);
+ }
m->match = match;
m->pri = pri;
} else {
if (m->indirect)
- free(match, M_DEVBUF, 0);
+ free(match, M_DEVBUF, cf->cf_attach->ca_devsize);
}
}
@@ -471,7 +473,7 @@ config_make_softc(struct device *parent,
old != 0 ? "expand" : "creat");
if (old != 0) {
bcopy(cd->cd_devs, nsp, old * sizeof(void *));
- free(cd->cd_devs, M_DEVBUF, 0);
+ free(cd->cd_devs, M_DEVBUF, old * sizeof(void *));
}
cd->cd_devs = nsp;
}
@@ -613,7 +615,7 @@ config_detach(struct device *dev, int fl
if (cd->cd_devs[i] != NULL)
break;
if (i == cd->cd_ndevs) {/* nothing found; deallocate */
- free(cd->cd_devs, M_DEVBUF, 0);
+ free(cd->cd_devs, M_DEVBUF, i * sizeof(void *));
cd->cd_devs = NULL;
cd->cd_ndevs = 0;
cf->cf_unit = 0;
Re: free(9) sizes for hooks
On Wed, Nov 28, 2018 at 07:01:16PM -0200, Martin Pieuchot wrote:
> Trivial sizes, ok?
ok claudio@
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.567
> diff -u -p -r1.567 if.c
> --- net/if.c 12 Nov 2018 23:41:22 - 1.567
> +++ net/if.c 28 Nov 2018 20:57:21 -
> @@ -1088,9 +1088,9 @@ if_detach(struct ifnet *ifp)
> }
> }
>
> - free(ifp->if_addrhooks, M_TEMP, 0);
> - free(ifp->if_linkstatehooks, M_TEMP, 0);
> - free(ifp->if_detachhooks, M_TEMP, 0);
> + free(ifp->if_addrhooks, M_TEMP, sizeof(*ifp->if_addrhooks));
> + free(ifp->if_linkstatehooks, M_TEMP, sizeof(*ifp->if_linkstatehooks));
> + free(ifp->if_detachhooks, M_TEMP, sizeof(*ifp->if_detachhooks));
>
> for (i = 0; (dp = domains[i]) != NULL; i++) {
> if (dp->dom_ifdetach && ifp->if_afdata[dp->dom_family])
>
--
:wq Claudio
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
Le 2018-10-05 23:42, Stuart Henderson a écrit :
On 2018/10/05 18:38, Alexander Bluhm wrote:
IPv6 Source selection is a mess!
> ICMP6 messages
> are generated with a source of, I think, the local address associated with
> the route to the recipient,
It is not that simple. Look at in6_ifawithscope() in
sys/netinet6/in6.c.
I know that's used for newly generated packets, but I'm not sure it's
the
case for icmp6, I haven't tried modifying the kernel to confirm for
sure
that this is the code generating the address in this case, but it seems
likely, in icmp6.c:
/*
1112 * If the incoming packet was addressed directly to us
(i.e. unicast),
1113 * use dst as the src for the reply.
1114 * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would be
VERY rare,
1115 * but is possible (for example) when we encounter an
error while
1116 * forwarding procedure destined to a duplicated address
of ours.
1117 */
1118 rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
1119 if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
1120 !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
1121 IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED))
{
1122 src = &t;
1123 }
I don't think this is the proper code extract because the traceroute6
packet
is not addressed directly to us.
It's addressed to another global unicast address and appears to time out
because the TTL gets decremented.
So I think the code that gets executed in this case is :
1127 if (src == NULL) {
1128 /*
1129 * This case matches to multicasts, our anycast, or
unicasts
1130 * that we do not own. Select a source address
based on the
1131 * source address of the erroneous packet.
1132 */
1133 rt = rtalloc(sin6tosa(&sa6_src), RT_RESOLVE,
rtableid);
1134 if (!rtisvalid(rt)) {
1135 char addr[INET6_ADDRSTRLEN];
1136
1137 nd6log((LOG_DEBUG,
1138 "%s: source can't be determined:
dst=%s\n",
1139 __func__, inet_ntop(AF_INET6,
&sa6_src.sin6_addr,
1140 addr, sizeof(addr;
1141 rtfree(rt);
1142 goto bad;
1143 }
1144 src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
1145
1146 }
I was thinking of replacing lines 1144 and 1145 with something along the
lines of
src = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
if (src=NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
Sorry if style is wrong, I'm not used to it, but you get the idea.
I have just looked at FreeBSD in6_ifawithifp(), it is quite simple.
Perhaps this is a way to go.
The code in FreeBSD's icmp6.c matching the above calls
in6ifa_ifwithaddr
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2113
Again, I think the branch hat gets executed is at
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2137
which calls
2147in6_splitscope(&ip6->ip6_src, &dst6, &scopeid);
2148error = in6_selectsrc_addr(M_GETFIB(m), &dst6,
2149scopeid, NULL, &src6, &hlim);
I can't find the in6_splitscope function in OpenBSD, so I guess there's
more to it than just copy-paste...
Re: fuse_parse_cmd_line.3 patch
Edgar Pettijohn III wrote: > remove a bunch of `_' where there shouldn't be `_'s please resend without nonbreaking spaces breaking the diff.
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
Le 2018-11-28 22:46, Arnaud BRAND a écrit :
Le 2018-10-05 23:42, Stuart Henderson a écrit :
On 2018/10/05 18:38, Alexander Bluhm wrote:
IPv6 Source selection is a mess!
> ICMP6 messages
> are generated with a source of, I think, the local address associated with
> the route to the recipient,
It is not that simple. Look at in6_ifawithscope() in
sys/netinet6/in6.c.
I know that's used for newly generated packets, but I'm not sure it's
the
case for icmp6, I haven't tried modifying the kernel to confirm for
sure
that this is the code generating the address in this case, but it
seems
likely, in icmp6.c:
/*
1112 * If the incoming packet was addressed directly to us
(i.e. unicast),
1113 * use dst as the src for the reply.
1114 * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would be
VERY rare,
1115 * but is possible (for example) when we encounter an
error while
1116 * forwarding procedure destined to a duplicated address
of ours.
1117 */
1118 rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
1119 if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
1120 !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
1121
IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
1122 src = &t;
1123 }
I don't think this is the proper code extract because the traceroute6
packet
is not addressed directly to us.
It's addressed to another global unicast address and appears to time
out
because the TTL gets decremented.
So I think the code that gets executed in this case is :
1127 if (src == NULL) {
1128 /*
1129 * This case matches to multicasts, our anycast,
or unicasts
1130 * that we do not own. Select a source address
based on the
1131 * source address of the erroneous packet.
1132 */
1133 rt = rtalloc(sin6tosa(&sa6_src), RT_RESOLVE,
rtableid);
1134 if (!rtisvalid(rt)) {
1135 char addr[INET6_ADDRSTRLEN];
1136
1137 nd6log((LOG_DEBUG,
1138 "%s: source can't be determined:
dst=%s\n",
1139 __func__, inet_ntop(AF_INET6,
&sa6_src.sin6_addr,
1140 addr, sizeof(addr;
1141 rtfree(rt);
1142 goto bad;
1143 }
1144 src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
1145
1146 }
I was thinking of replacing lines 1144 and 1145 with something along
the lines of
src = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
if (src=NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
Sorry if style is wrong, I'm not used to it, but you get the idea.
Currently building a test kernel with this diff.
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.227
diff -u -p -u -p -r1.227 icmp6.c
--- netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227
+++ netinet6/icmp6.c28 Nov 2018 22:55:04 -
@@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
struct icmp6_hdr *icmp6;
struct in6_addr t, *src = NULL;
struct sockaddr_in6 sa6_src, sa6_dst;
+ struct in6_ifaddr *ifa;
u_int rtableid;
CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <=
MHLEN);
@@ -1141,7 +1142,9 @@ icmp6_reflect(struct mbuf *m, size_t off
rtfree(rt);
goto bad;
}
- src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
+ ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t,
rtableid);
+ if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr);
+ if (src == NULL) src =
&ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
}
ip6->ip6_src = *src;
But I have no -current machine in IPv6 paths to test it.
I'll have to backport on -stable next week if I get a chance.
I have just looked at FreeBSD in6_ifawithifp(), it is quite simple.
Perhaps this is a way to go.
The code in FreeBSD's icmp6.c matching the above calls
in6ifa_ifwithaddr
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2113
Again, I think the branch hat gets executed is at
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2137
which calls
2147in6_splitscope(&ip6->ip6_src, &dst6, &scopeid);
2148error = in6_selectsrc_addr(M_GETFIB(m), &dst6,
2149scopeid, NULL, &src6, &hlim);
I can't find the in6_splitscope function in OpenBSD, so I guess
there's more to it than just copy-paste...
Actually in6_selectsrc_addr seems to do something similar to what
in6_ifawithscope does.
So this part seems correct.
What seems a litt
Re: traceroute6 and ospf6d (icmp6 source addresses and link-locals)
I have setup a test environment where I can reproduce the issue with
GENERIC#481.
I can confirm that the issue is fixed by the proposed patch.
traceroute6 to/from/through the patched machine got expected result and
did not crash the machine.
Anyone else would like to try ?
Or propose improvements ?
Le 2018-11-29 00:02, Arnaud BRAND a écrit :
Le 2018-11-28 22:46, Arnaud BRAND a écrit :
Le 2018-10-05 23:42, Stuart Henderson a écrit :
On 2018/10/05 18:38, Alexander Bluhm wrote:
IPv6 Source selection is a mess!
> ICMP6 messages
> are generated with a source of, I think, the local address associated with
> the route to the recipient,
It is not that simple. Look at in6_ifawithscope() in
sys/netinet6/in6.c.
I know that's used for newly generated packets, but I'm not sure it's
the
case for icmp6, I haven't tried modifying the kernel to confirm for
sure
that this is the code generating the address in this case, but it
seems
likely, in icmp6.c:
/*
1112 * If the incoming packet was addressed directly to us
(i.e. unicast),
1113 * use dst as the src for the reply.
1114 * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would
be
VERY rare,
1115 * but is possible (for example) when we encounter an
error while
1116 * forwarding procedure destined to a duplicated address
of ours.
1117 */
1118 rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
1119 if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
1120 !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
1121
IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
1122 src = &t;
1123 }
I don't think this is the proper code extract because the traceroute6
packet
is not addressed directly to us.
It's addressed to another global unicast address and appears to time
out
because the TTL gets decremented.
So I think the code that gets executed in this case is :
1127 if (src == NULL) {
1128 /*
1129 * This case matches to multicasts, our anycast,
or unicasts
1130 * that we do not own. Select a source address
based on the
1131 * source address of the erroneous packet.
1132 */
1133 rt = rtalloc(sin6tosa(&sa6_src), RT_RESOLVE,
rtableid);
1134 if (!rtisvalid(rt)) {
1135 char addr[INET6_ADDRSTRLEN];
1136
1137 nd6log((LOG_DEBUG,
1138 "%s: source can't be determined:
dst=%s\n",
1139 __func__, inet_ntop(AF_INET6,
&sa6_src.sin6_addr,
1140 addr, sizeof(addr;
1141 rtfree(rt);
1142 goto bad;
1143 }
1144 src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
1145
1146 }
I was thinking of replacing lines 1144 and 1145 with something along
the lines of
src = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
if (src=NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
Sorry if style is wrong, I'm not used to it, but you get the idea.
Currently building a test kernel with this diff.
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.227
diff -u -p -u -p -r1.227 icmp6.c
--- netinet6/icmp6.c9 Nov 2018 14:14:32 - 1.227
+++ netinet6/icmp6.c28 Nov 2018 22:55:04 -
@@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
struct icmp6_hdr *icmp6;
struct in6_addr t, *src = NULL;
struct sockaddr_in6 sa6_src, sa6_dst;
+ struct in6_ifaddr *ifa;
u_int rtableid;
CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <=
MHLEN);
@@ -1141,7 +1142,9 @@ icmp6_reflect(struct mbuf *m, size_t off
rtfree(rt);
goto bad;
}
- src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
+ ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t,
rtableid);
+ if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr);
+ if (src == NULL) src =
&ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
}
ip6->ip6_src = *src;
But I have no -current machine in IPv6 paths to test it.
I'll have to backport on -stable next week if I get a chance.
I have just looked at FreeBSD in6_ifawithifp(), it is quite simple.
Perhaps this is a way to go.
The code in FreeBSD's icmp6.c matching the above calls
in6ifa_ifwithaddr
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2113
Again, I think the branch hat gets executed is at
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2137
which calls
2147in6_splitscope(&ip6->ip6_src, &dst6, &scopeid);
2148
Idea forOpenBGPD RDE MP ... would this work to make RDE MP safe ?
Hello, before I begin... im just a sysadmin not a programmer I appreciate the work you are doing on OpenBGPd :) and I use it and im very happy I saw Claudes presentation on openBGPd recently and how there was some work on MP for BGPd.. and I was wondering about an idea (I thought it was a simple thing which may be too simple ... please bear in mind Im not a professional programmer, basically I was wondering could there be a simple way of removing risk of having an MP RDE, from my understanding, one of the concerns about RDE being MP is a withdraw being received on process running on a busy processor , and a subsequent announce being received by a process running on a lighty loaded processor .. the announce being processed first and then the withdraw being processed afterwards... (there by withdrawing the route even though the announce was received after the withdraw) there are probably others... To get around the concern above.. could the Ip space be carved up between different processes so it is always the same process dealing with all messages to do with an address space) lets say with 2 core and 4 core Box for example Could you split the RDE into 2 RDE processes which would only process half of the IP space ? so for ip v4 0.0.0.0/0 (route decision engine on a single core (current setup) on a 2 core system 0.0.0.0/1 would be managed by RDE0 128.0.0.0/1 would be managed by RDE1 on a 4 core system 0.0.0.0/2 RDE0 64.0.0.0/2RDE1 128.0.0.0/2 RDE2 192.0.0.0/2 RDE3 on an 8 core system 0.0.0.0/3 RDE0 32.0.0.0/3 RDE1 64.0.0.0/3 RDE2 96.0.0.0/3 RDE3 128.0.0.0/3RDE4 160.0.0.0/3 RDE5 192.0.0.0/3 RDE6 224.0.0.0/3 RDE7 you can use the spare cycles on this one to generate your favourite concurrency or help SETI :P or something :) so each process may not be equally loaded but they would be faster over all with the increased parallelism ... and because each RDE is operating on prefixes that are only in a certain range that doesn't overlap other RDE Process work ... there would never be a race condition or nasty MP introduced bugs also there would need to be some work on the session engine to pass theBGP messages to the correct RDE process easily (the decision process would be similar to a routing decision in the kernel LPM with 2 or 4 or 8 routes ) IPv6 I get would need a more nuanced approach as in split the subset 2000::/3 as opposed to 0:0/128 It would be a long time before I would be able to submit a patch worthy of consideration... but is this Idea worth pursuing ? is there a fundamental flaw in this idea / approach Thanks for your time and consideration... Tom Smyth
Re: pvclock(4)
So far I only got positive reports. Where are the problems? ;) Otherwise: OK? Reyk Hi, my kvm/quest/openbsd-amd64 can not boot, after upgrade to today current (28-nov-2018). It work before upgrade (OpenBSD 6.4-current (GENERIC.MP) #447: Sun Nov 18 17:25:58 MST 2018) Host: 4.18.0-3-amd64 #1 SMP Debian 4.18.20-2 (2018-11-23) x86_64 GNU/Linux qemu: QEMU emulator version 2.12.0 (Debian 1:2.12+dfsg-3+b1) panic photo: https://ibb.co/gjWBhL4 panic photo: https://ibb.co/HDrJ7Q0 panic photo: https://ibb.co/g96J5s7 Thanks. -- Key fingerprint: CDB3 6C62 254B C088 1E5D DD32 182C 97DB CF2C 80AC
Re: pvclock(4)
Hi, > Am 29.11.2018 um 05:27 schrieb johnw : > > >> So far I only got positive reports. Where are the problems? ;) > >> Otherwise: OK? > >> Reyk > > Hi, my kvm/quest/openbsd-amd64 can not boot, after upgrade to today current > (28-nov-2018). > thanks for reporting. Do you have a full dmesg for me? Reyk > It work before upgrade (OpenBSD 6.4-current (GENERIC.MP) #447: Sun Nov 18 > 17:25:58 MST 2018) > > Host: 4.18.0-3-amd64 #1 SMP Debian 4.18.20-2 (2018-11-23) x86_64 GNU/Linux > qemu: QEMU emulator version 2.12.0 (Debian 1:2.12+dfsg-3+b1) > panic photo: https://ibb.co/gjWBhL4 > panic photo: https://ibb.co/HDrJ7Q0 > panic photo: https://ibb.co/g96J5s7 > > Thanks. > > -- > Key fingerprint: CDB3 6C62 254B C088 1E5D DD32 182C 97DB CF2C 80AC
[patch] sh.1 getopts inaccurate
hi
I'm not familiar with getopts, first time I use it and the man page was
confusing. It says "a colon following an option indicates it may take an
argument".
I understand this as: if I use the string "s:" then I can use "-s" or "-s
something" as it **may** take an argument.
But if you use "s:", getopts reports an error "-s requires argument".
I propose to modify that sentence (not sure for the s in requires).
Index: sh.1
===
RCS file: /data/cvs/src/bin/ksh/sh.1,v
retrieving revision 1.149
diff -u -p -r1.149 sh.1
--- sh.128 Sep 2018 18:32:39 - 1.149
+++ sh.129 Nov 2018 06:44:50 -
@@ -495,7 +495,7 @@ to the index of the next variable to be
The string
.Ar optstring
contains a list of acceptable options;
-a colon following an option indicates it may take an argument.
+a colon following an option indicates it requires an argument.
If an option not recognised by
.Ar optstring
is found,
Example to reproduce it:
#!/bin/sh
while getopts s:? args
do
case "$args" in
s) VALUE=${OPTARG} ;;
*) exit 0 ;;
esac
done
echo $VALUE
solene@t480 ~/notes/perso $ ./bug.sh -s
./bug.sh[9]: -`s' requires argument
solene@t480 ~/notes/perso $ ./bug.sh -s value
value
Re: [patch] sh.1 getopts inaccurate
On Wed, Nov 28, 2018 at 10:52 PM Solene Rapenne wrote: > I'm not familiar with getopts, first time I use it and the man page was > confusing. It says "a colon following an option indicates it may take an > argument". > I understand this as: if I use the string "s:" then I can use "-s" or "-s > something" as it **may** take an argument. > > But if you use "s:", getopts reports an error "-s requires argument". > > I propose to modify that sentence (not sure for the s in requires). > > Index: sh.1 > === > RCS file: /data/cvs/src/bin/ksh/sh.1,v > retrieving revision 1.149 > diff -u -p -r1.149 sh.1 > --- sh.128 Sep 2018 18:32:39 - 1.149 > +++ sh.129 Nov 2018 06:44:50 - > @@ -495,7 +495,7 @@ to the index of the next variable to be > The string > .Ar optstring > contains a list of acceptable options; > -a colon following an option indicates it may take an argument. > +a colon following an option indicates it requires an argument. > If an option not recognised by > .Ar optstring > is found, > "requires" is correct there. ok guenther@
Re: [patch] sh.1 getopts inaccurate
I also agree.
> hi
>
> I'm not familiar with getopts, first time I use it and the man page was
> confusing. It says "a colon following an option indicates it may take an
> argument".
> I understand this as: if I use the string "s:" then I can use "-s" or "-s
> something" as it **may** take an argument.
>
> But if you use "s:", getopts reports an error "-s requires argument".
>
> I propose to modify that sentence (not sure for the s in requires).
>
> Index: sh.1
> ===
> RCS file: /data/cvs/src/bin/ksh/sh.1,v
> retrieving revision 1.149
> diff -u -p -r1.149 sh.1
> --- sh.1 28 Sep 2018 18:32:39 - 1.149
> +++ sh.1 29 Nov 2018 06:44:50 -
> @@ -495,7 +495,7 @@ to the index of the next variable to be
> The string
> .Ar optstring
> contains a list of acceptable options;
> -a colon following an option indicates it may take an argument.
> +a colon following an option indicates it requires an argument.
> If an option not recognised by
> .Ar optstring
> is found,
>
>
> Example to reproduce it:
>
> #!/bin/sh
> while getopts s:? args
> do
> case "$args" in
> s) VALUE=${OPTARG} ;;
> *) exit 0 ;;
> esac
> done
> echo $VALUE
>
>
> solene@t480 ~/notes/perso $ ./bug.sh -s
> ./bug.sh[9]: -`s' requires argument
> solene@t480 ~/notes/perso $ ./bug.sh -s value
> value
>
