Attached the patch with the new syntax for custom options and to use WALK_LIST.
On Sat, Jun 24, 2023 at 3:32 PM Ondrej Zajicek <[email protected]> wrote:
>
> On Sat, Jun 24, 2023 at 02:03:08AM +0200, Alexander Zubkov wrote:
> > On Fri, Jun 23, 2023, 17:47 Ondrej Zajicek <[email protected]> wrote:
> > > The only objection from me is that 'other type' option name is kind of
> > > non-descriptive, does not indicate it is related to RA options (nor it is
> > > implicated by context). I do not really have a good idea for alternative,
> > > perhaps just 'custom option'? What do you think?
> > >
> >
> > Yes, I was thinking about "custom" too. But it introduces a new keyword. So
> > I tried too choose something suitable from available keywords. But if it is
> > not a problem, I would prefer "custom" too as more descriptive.
>
> I think 'custom option' would be ok. It has two arguments, thinking about
> it, BIRD style would be more like:
>
> custom option type 10 value 12:34:56:78:12:34:56:78:12:34:56:78:12:34:56:78;
>
> instead of just:
>
> custom option 10 12:34:56:78:12:34:56:78:12:34:56:78:12:34:56:78;
>
>
> > > Could you prepare a patch for documentation?
> > >
> >
> > Sure, just will wait for the final decision about the syntax ("other" vs
> > "custom").
>
> okay.
>
>
> > > BTW, why not to use WALK_LIST() in radv_prepare_custom()?
> > > (just noticed in now)
> > >
> >
> > That is a good question. Actually I just used the structure of the similar
> > function for the predefined option and didn't thought much about it. Now
> > that you pointed that out, I remember that macro from the other parts of
> > the code. And it seems reasonable to use it here, and probably in the
> > "source" function too. I can prepare a patch for that.
>
> Okay. I see the 'source' funcions has more complex structure (two nested
> whiles), so perhaps it does not fit to them.
>
>
> --
> Elen sila lumenn' omentielvo
>
> Ondrej 'Santiago' Zajicek (email: [email protected])
> OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
> "To err is human -- to blame it on a computer is even more so."
diff --git a/proto/radv/config.Y b/proto/radv/config.Y
index 5c213d50..81dc415a 100644
--- a/proto/radv/config.Y
+++ b/proto/radv/config.Y
@@ -42,7 +42,7 @@ CF_KEYWORDS(RADV, PREFIX, INTERFACE, MIN, MAX, RA, DELAY, INTERVAL, SOLICITED,
RETRANS, TIMER, CURRENT, HOP, LIMIT, DEFAULT, VALID, PREFERRED, MULT,
LIFETIME, SKIP, ONLINK, AUTONOMOUS, RDNSS, DNSSL, NS, DOMAIN, LOCAL,
TRIGGER, SENSITIVE, PREFERENCE, LOW, MEDIUM, HIGH, PROPAGATE, ROUTE,
- ROUTES, RA_PREFERENCE, RA_LIFETIME)
+ ROUTES, RA_PREFERENCE, RA_LIFETIME, CUSTOM, OPTION, VALUE)
CF_ENUM(T_ENUM_RA_PREFERENCE, RA_PREF_, LOW, MEDIUM, HIGH)
@@ -50,6 +50,8 @@ CF_ENUM(T_ENUM_RA_PREFERENCE, RA_PREF_, LOW, MEDIUM, HIGH)
CF_GRAMMAR
+kw_sym: CUSTOM | OPTION | VALUE ;
+
proto: radv_proto ;
radv_proto_start: proto_start RADV
@@ -71,7 +73,7 @@ radv_proto_item:
| PREFIX radv_prefix { add_tail(&RADV_CFG->pref_list, NODE this_radv_prefix); }
| RDNSS { init_list(&radv_dns_list); } radv_rdnss { add_tail_list(&RADV_CFG->rdnss_list, &radv_dns_list); }
| DNSSL { init_list(&radv_dns_list); } radv_dnssl { add_tail_list(&RADV_CFG->dnssl_list, &radv_dns_list); }
- | OTHER TYPE expr BYTESTRING { radv_add_to_custom_list(&RADV_CFG->custom_list, $3, $4); }
+ | CUSTOM OPTION TYPE expr VALUE BYTESTRING { radv_add_to_custom_list(&RADV_CFG->custom_list, $4, $6); }
| TRIGGER net_ip6 { RADV_CFG->trigger = $2; }
| PROPAGATE ROUTES bool { RADV_CFG->propagate_routes = $3; }
;
@@ -136,10 +138,10 @@ radv_iface_item:
| PREFIX radv_prefix { add_tail(&RADV_IFACE->pref_list, NODE this_radv_prefix); }
| RDNSS { init_list(&radv_dns_list); } radv_rdnss { add_tail_list(&RADV_IFACE->rdnss_list, &radv_dns_list); }
| DNSSL { init_list(&radv_dns_list); } radv_dnssl { add_tail_list(&RADV_IFACE->dnssl_list, &radv_dns_list); }
- | OTHER TYPE expr BYTESTRING { radv_add_to_custom_list(&RADV_IFACE->custom_list, $3, $4); }
+ | CUSTOM OPTION TYPE expr VALUE BYTESTRING { radv_add_to_custom_list(&RADV_IFACE->custom_list, $4, $6); }
| RDNSS LOCAL bool { RADV_IFACE->rdnss_local = $3; }
| DNSSL LOCAL bool { RADV_IFACE->dnssl_local = $3; }
- | OTHER LOCAL bool { RADV_IFACE->custom_local = $3; }
+ | CUSTOM OPTION LOCAL bool { RADV_IFACE->custom_local = $4; }
;
radv_preference:
diff --git a/proto/radv/packets.c b/proto/radv/packets.c
index d1f86ec1..77c98794 100644
--- a/proto/radv/packets.c
+++ b/proto/radv/packets.c
@@ -264,9 +264,8 @@ radv_prepare_dnssl(struct radv_iface *ifa, list *dnssl_list, char **buf, char *b
static int
radv_prepare_custom(struct radv_iface *ifa, list *custom_list, char **buf, char *bufend)
{
- struct radv_custom_config *ccf = HEAD(*custom_list);
-
- while(NODE_VALID(ccf))
+ struct radv_custom_config *ccf;
+ WALK_LIST(ccf, *custom_list)
{
struct radv_opt_custom *op = (void *) *buf;
/* Add 2 octets for type and size and 8 - 1 for ceiling the division up to 8 octets */
@@ -280,7 +279,6 @@ radv_prepare_custom(struct radv_iface *ifa, list *custom_list, char **buf, char
memcpy(op->payload, ccf->payload->data, ccf->payload->length);
*buf += 8 * op->length;
- ccf = NODE_NEXT(ccf);
}
return 0;
