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 <santi...@crfreenet.org> wrote:
>
> On Sat, Jun 24, 2023 at 02:03:08AM +0200, Alexander Zubkov wrote:
> > On Fri, Jun 23, 2023, 17:47 Ondrej Zajicek <santi...@crfreenet.org> 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: santi...@crfreenet.org)
> 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;

Reply via email to