* Re: [PATCH 06/11] icmp6: Parse extra options
@ 2022-04-19 11:37 Andrew Zaborowski
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Zaborowski @ 2022-04-19 11:37 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 3005 bytes --]
Hi Denis,
On Mon, 18 Apr 2022 at 21:39, Denis Kenzior <denkenz(a)gmail.com> wrote:
> On 4/11/22 09:20, Andrew Zaborowski wrote:
> > + if (((opts[3] >> 3) & 3) == 2)
> > + break;
>
> Can we use bit_field() from useful.h instead?
Sure.
>
> > +
> > + /*
> > + * RFC 4191 Section 3.1:
> > + * "The Router Preference and Lifetime values in a ::/0
> > + * Route Information Option override the preference and
> > + * lifetime values in the Router Advertisement header."
> > + *
> > + * Don't count ::/0 routes.
> > + */
> > + if (opts[2] == 0)
> > + break;
> > +
> > + n_routes += 1;
> > break;
> > }
> >
> > @@ -739,6 +780,10 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
> > r->other = true;
> >
> > r->pref = (ra->nd_ra_flags_reserved >> 3) & 0x3;
> > + /*
> > + * "If the Reserved (10) value is received, the receiver MUST
> > + * treat the value as if it were (00).
> > + */
>
> This might warrant a separate commit
Ok.
>
> > if (r->pref == 0x2) /* If invalid, reset to medium */
> > r->pref = 0;
> >
>
> <snip>
>
> > @@ -765,17 +810,69 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
> > break;
> > case ND_OPT_PREFIX_INFORMATION:
> > {
> > - struct route_info *i = &r->prefixes[n_prefixes];
> > + struct route_info *i = &r->routes[n_routes];
> >
> > if (!(opts[3] & ND_OPT_PI_FLAG_ONLINK))
> > break;
> >
> > i->prefix_len = opts[2];
> > + i->onlink = true;
> > i->valid_lifetime = l_get_be32(opts + 4);
> > i->preferred_lifetime = l_get_be32(opts + 8);
> > memcpy(i->address, opts + 16, 16);
> >
> > - n_prefixes += 1;
> > + n_routes += 1;
> > + break;
> > + }
> > + case ND_OPT_RTR_ADV_INTERVAL:
> > + if (l < 8)
> > + break;
> > +
> > + r->max_rtr_adv_interval_ms = l_get_be32(opts + 4);
> > + break;
> > + case ND_OPT_ROUTE_INFORMATION:
> > + {
> > + struct route_info *i = &r->routes[n_routes++];
> > +
> > + if (((opts[3] >> 3) & 3) == 2)
>
> bit_field()?
>
> Also, you access this value at least twice below, so maybe just use a variable
> and assign it.
Ok.
Best regards
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 06/11] icmp6: Parse extra options
@ 2022-04-18 19:39 Denis Kenzior
0 siblings, 0 replies; 3+ messages in thread
From: Denis Kenzior @ 2022-04-18 19:39 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 4211 bytes --]
Hi Andrew,
On 4/11/22 09:20, Andrew Zaborowski wrote:
> Parse Route Information and New Advertisement Interval options. Route
> Information options (RIOs) are supported by radvd and a few clients that
> I checked.
> ---
> ell/icmp6-private.h | 6 ++-
> ell/icmp6.c | 121 +++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 113 insertions(+), 14 deletions(-)
>
<snip>
> @@ -719,7 +732,35 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
> return NULL;
>
> if (opts[3] & ND_OPT_PI_FLAG_ONLINK)
> - n_prefixes += 1;
> + n_routes += 1;
> + break;
> + case ND_OPT_ROUTE_INFORMATION:
> + if (l < 8)
> + return NULL;
> +
> + if (opts[2] > 128 || opts[2] > (l - 8) * 8)
> + return NULL;
> +
> + /*
> + * RFC 4191 Section 2.3:
> + * "If the Reserved (10) value is received, the Route
> + * Information Option MUST be ignored."
> + */
> + if (((opts[3] >> 3) & 3) == 2)
> + break;
Can we use bit_field() from useful.h instead?
> +
> + /*
> + * RFC 4191 Section 3.1:
> + * "The Router Preference and Lifetime values in a ::/0
> + * Route Information Option override the preference and
> + * lifetime values in the Router Advertisement header."
> + *
> + * Don't count ::/0 routes.
> + */
> + if (opts[2] == 0)
> + break;
> +
> + n_routes += 1;
> break;
> }
>
> @@ -739,6 +780,10 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
> r->other = true;
>
> r->pref = (ra->nd_ra_flags_reserved >> 3) & 0x3;
> + /*
> + * "If the Reserved (10) value is received, the receiver MUST
> + * treat the value as if it were (00).
> + */
This might warrant a separate commit
> if (r->pref == 0x2) /* If invalid, reset to medium */
> r->pref = 0;
>
<snip>
> @@ -765,17 +810,69 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
> break;
> case ND_OPT_PREFIX_INFORMATION:
> {
> - struct route_info *i = &r->prefixes[n_prefixes];
> + struct route_info *i = &r->routes[n_routes];
>
> if (!(opts[3] & ND_OPT_PI_FLAG_ONLINK))
> break;
>
> i->prefix_len = opts[2];
> + i->onlink = true;
> i->valid_lifetime = l_get_be32(opts + 4);
> i->preferred_lifetime = l_get_be32(opts + 8);
> memcpy(i->address, opts + 16, 16);
>
> - n_prefixes += 1;
> + n_routes += 1;
> + break;
> + }
> + case ND_OPT_RTR_ADV_INTERVAL:
> + if (l < 8)
> + break;
> +
> + r->max_rtr_adv_interval_ms = l_get_be32(opts + 4);
> + break;
> + case ND_OPT_ROUTE_INFORMATION:
> + {
> + struct route_info *i = &r->routes[n_routes++];
> +
> + if (((opts[3] >> 3) & 3) == 2)
bit_field()?
Also, you access this value at least twice below, so maybe just use a variable
and assign it.
> + break;
> +
> + /*
> + * RFC 4191 Section 3.1:
> + * "The Router Preference and Lifetime values in a ::/0
> + * Route Information Option override the preference and
> + * lifetime values in the Router Advertisement header."
> + */
> + if (opts[2] == 0) {
> + if (r->lifetime == 0 && l_get_be32(opts + 4)) {
> + /*
> + * A ::/0 route received from a
> + * non-default router? Should issue
> + * a warning?
> + */
> + break;
> + }
> +
> + r->pref = (opts[3] >> 3) & 3;
> + r->lifetime = l_get_be16(opts + 4) ? 0xffff :
> + l_get_be16(opts + 6);
> + break;
> + }
> +
> + /*
> + * Don't check or warn if the route lifetime is longer
> + * than the router lifetime because that refers to its
> + * time as the default router. It may be configured to
> + * route packets for us for specific prefixes without
> + * being a default router.
> + */
> + i->prefix_len = opts[2];
> + i->onlink = false;
> + i->preference = (opts[3] >> 3) & 3;
> + i->valid_lifetime = l_get_be32(opts + 4);
> + memcpy(i->address, opts + 8, (i->prefix_len + 7) / 8);
> +
> + n_routes += 1;
> break;
> }
> }
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 06/11] icmp6: Parse extra options
@ 2022-04-11 14:20 Andrew Zaborowski
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Zaborowski @ 2022-04-11 14:20 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 6735 bytes --]
Parse Route Information and New Advertisement Interval options. Route
Information options (RIOs) are supported by radvd and a few clients that
I checked.
---
ell/icmp6-private.h | 6 ++-
ell/icmp6.c | 121 +++++++++++++++++++++++++++++++++++++++-----
2 files changed, 113 insertions(+), 14 deletions(-)
diff --git a/ell/icmp6-private.h b/ell/icmp6-private.h
index 2433087..77df0b8 100644
--- a/ell/icmp6-private.h
+++ b/ell/icmp6-private.h
@@ -22,6 +22,7 @@
struct route_info {
uint8_t address[16];
+ bool onlink : 1;
uint8_t prefix_len;
uint8_t preference;
uint32_t preferred_lifetime;
@@ -36,8 +37,9 @@ struct l_icmp6_router {
uint64_t start_time;
uint16_t lifetime;
uint32_t mtu;
- uint32_t n_prefixes;
- struct route_info *prefixes;
+ uint32_t max_rtr_adv_interval_ms;
+ uint32_t n_routes;
+ struct route_info *routes;
};
struct l_icmp6_router *_icmp6_router_new();
diff --git a/ell/icmp6.c b/ell/icmp6.c
index 3fa8876..4128f01 100644
--- a/ell/icmp6.c
+++ b/ell/icmp6.c
@@ -51,6 +51,11 @@
#include "icmp6.h"
#include "icmp6-private.h"
+/* RFC4191 */
+#ifndef ND_OPT_ROUTE_INFORMATION
+#define ND_OPT_ROUTE_INFORMATION 24
+#endif
+
#define CLIENT_DEBUG(fmt, args...) \
l_util_debug(client->debug_handler, client->debug_data, \
"%s:%i " fmt, __func__, __LINE__, ## args)
@@ -328,16 +333,24 @@ static void icmp6_client_setup_routes(struct l_icmp6_client *client)
l_rtnl_route_add(client->rtnl, client->ifindex, rt,
NULL, NULL, NULL);
- for (i = 0; i < ra->n_prefixes; i++) {
- struct route_info *info = &ra->prefixes[i];
+ for (i = 0; i < ra->n_routes; i++) {
+ char prefix_buf[INET6_ADDRSTRLEN];
+ struct route_info *info = &ra->routes[i];
if (info->valid_lifetime == 0)
continue;
- if (!inet_ntop(AF_INET6, info->address, buf, sizeof(buf)))
+ if (!inet_ntop(AF_INET6, info->address, prefix_buf,
+ sizeof(prefix_buf)))
continue;
- rt = l_rtnl_route_new_prefix(buf, info->prefix_len);
+ if (info->onlink)
+ rt = l_rtnl_route_new_prefix(prefix_buf,
+ info->prefix_len);
+ else
+ rt = l_rtnl_route_new_static(buf, prefix_buf,
+ info->prefix_len);
+
if (!rt)
continue;
@@ -670,7 +683,7 @@ struct l_icmp6_router *_icmp6_router_new()
void _icmp6_router_free(struct l_icmp6_router *r)
{
- l_free(r->prefixes);
+ l_free(r->routes);
l_free(r);
}
@@ -682,7 +695,7 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
struct l_icmp6_router *r;
const uint8_t *opts;
uint32_t opts_len;
- uint32_t n_prefixes = 0;
+ uint32_t n_routes = 0;
if (ra->nd_ra_type != ND_ROUTER_ADVERT)
return NULL;
@@ -719,7 +732,35 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
return NULL;
if (opts[3] & ND_OPT_PI_FLAG_ONLINK)
- n_prefixes += 1;
+ n_routes += 1;
+ break;
+ case ND_OPT_ROUTE_INFORMATION:
+ if (l < 8)
+ return NULL;
+
+ if (opts[2] > 128 || opts[2] > (l - 8) * 8)
+ return NULL;
+
+ /*
+ * RFC 4191 Section 2.3:
+ * "If the Reserved (10) value is received, the Route
+ * Information Option MUST be ignored."
+ */
+ if (((opts[3] >> 3) & 3) == 2)
+ break;
+
+ /*
+ * RFC 4191 Section 3.1:
+ * "The Router Preference and Lifetime values in a ::/0
+ * Route Information Option override the preference and
+ * lifetime values in the Router Advertisement header."
+ *
+ * Don't count ::/0 routes.
+ */
+ if (opts[2] == 0)
+ break;
+
+ n_routes += 1;
break;
}
@@ -729,8 +770,8 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
r = _icmp6_router_new();
memcpy(r->address, src, sizeof(r->address));
- r->prefixes = l_new(struct route_info, n_prefixes);
- r->n_prefixes = n_prefixes;
+ r->routes = l_new(struct route_info, n_routes);
+ r->n_routes = n_routes;
if (ra->nd_ra_flags_reserved & ND_RA_FLAG_MANAGED)
r->managed = true;
@@ -739,6 +780,10 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
r->other = true;
r->pref = (ra->nd_ra_flags_reserved >> 3) & 0x3;
+ /*
+ * "If the Reserved (10) value is received, the receiver MUST
+ * treat the value as if it were (00).
+ */
if (r->pref == 0x2) /* If invalid, reset to medium */
r->pref = 0;
@@ -747,7 +792,7 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
opts = (uint8_t *) (ra + 1);
opts_len = len - sizeof(struct nd_router_advert);
- n_prefixes = 0;
+ n_routes = 0;
while (opts_len) {
uint8_t t = opts[0];
@@ -765,17 +810,69 @@ struct l_icmp6_router *_icmp6_router_parse(const struct nd_router_advert *ra,
break;
case ND_OPT_PREFIX_INFORMATION:
{
- struct route_info *i = &r->prefixes[n_prefixes];
+ struct route_info *i = &r->routes[n_routes];
if (!(opts[3] & ND_OPT_PI_FLAG_ONLINK))
break;
i->prefix_len = opts[2];
+ i->onlink = true;
i->valid_lifetime = l_get_be32(opts + 4);
i->preferred_lifetime = l_get_be32(opts + 8);
memcpy(i->address, opts + 16, 16);
- n_prefixes += 1;
+ n_routes += 1;
+ break;
+ }
+ case ND_OPT_RTR_ADV_INTERVAL:
+ if (l < 8)
+ break;
+
+ r->max_rtr_adv_interval_ms = l_get_be32(opts + 4);
+ break;
+ case ND_OPT_ROUTE_INFORMATION:
+ {
+ struct route_info *i = &r->routes[n_routes++];
+
+ if (((opts[3] >> 3) & 3) == 2)
+ break;
+
+ /*
+ * RFC 4191 Section 3.1:
+ * "The Router Preference and Lifetime values in a ::/0
+ * Route Information Option override the preference and
+ * lifetime values in the Router Advertisement header."
+ */
+ if (opts[2] == 0) {
+ if (r->lifetime == 0 && l_get_be32(opts + 4)) {
+ /*
+ * A ::/0 route received from a
+ * non-default router? Should issue
+ * a warning?
+ */
+ break;
+ }
+
+ r->pref = (opts[3] >> 3) & 3;
+ r->lifetime = l_get_be16(opts + 4) ? 0xffff :
+ l_get_be16(opts + 6);
+ break;
+ }
+
+ /*
+ * Don't check or warn if the route lifetime is longer
+ * than the router lifetime because that refers to its
+ * time as the default router. It may be configured to
+ * route packets for us for specific prefixes without
+ * being a default router.
+ */
+ i->prefix_len = opts[2];
+ i->onlink = false;
+ i->preference = (opts[3] >> 3) & 3;
+ i->valid_lifetime = l_get_be32(opts + 4);
+ memcpy(i->address, opts + 8, (i->prefix_len + 7) / 8);
+
+ n_routes += 1;
break;
}
}
--
2.32.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-19 11:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 11:37 [PATCH 06/11] icmp6: Parse extra options Andrew Zaborowski
-- strict thread matches above, loose matches on Subject: below --
2022-04-18 19:39 Denis Kenzior
2022-04-11 14:20 Andrew Zaborowski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.