[PATCH] adding custom options in radv protocol, strict ipv6 regex

Alexander Zubkov green at qrator.net
Fri Jul 7 00:55:12 CEST 2023


Hi,

And the final patch for the bytestring documentation. Also slightly
modified radv documentation patch - added a semicolon in the end of
the example.
I actually would prefer the "binary" name for the type more than
"bytestring". Or maybe you have something else on your mind. So if you
would also like not to name it "bytestring", I can alter the pathes
for it.

On Fri, Jun 30, 2023 at 1:21 AM Alexander Zubkov <green at qrator.net> wrote:
>
> Patch for RAdv documentation for a new custom option.
>
> I was also thinking about the new bytestring type. I needed tho change
> BYTESTRING -> BYTETEXT to avoid collision. But probably the better
> variant would be to name the new type for example "binary", it might
> sound better. What do you think? As far as I see, the name
> "bytestring" has not been used yet outside of the code.
>
> Also found a small issue with my patches in "conf/confbase.Y" when
> compiling with relatively old gcc. It complains on the value
> definitions inside the switch statement:
>
>        case T_STRING:
>          int len = strlen(val.val.s);
>          struct bytestring *bs = cfg_allocz(sizeof(*bs) + len);
>
> I can prepare a new set of patches or you can fix it ad-hoc.
>
> On Thu, Jun 29, 2023 at 1:59 PM Alexander Zubkov <green at qrator.net> wrote:
> >
> > On Tue, Jun 27, 2023 at 2:13 AM Alexander Zubkov <green at qrator.net> wrote:
> > >
> > > On Mon, Jun 26, 2023 at 5:54 PM Alexander Zubkov <green at qrator.net> wrote:
> > > >
> > > > On Mon, Jun 26, 2023 at 5:43 PM Ondrej Zajicek <santiago at crfreenet.org> wrote:
> > > > >
> > > > > On Mon, Jun 26, 2023 at 03:24:47AM +0200, Alexander Zubkov wrote:
> > > > > > On Sat, Jun 24, 2023 at 3:16 PM Ondrej Zajicek <santiago at crfreenet.org> wrote:
> > > > > > >
> > > > > > > On Sat, Jun 24, 2023 at 02:20:03AM +0200, Alexander Zubkov wrote:
> > > > > > > > > Yes, the original idea there was to add bytestring as a data type, make
> > > > > > > > > hex() a regular (filter) function instead of special function-like
> > > > > > > > > syntax, and add equivalent of 'expr' grammar term for other data types.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I see. I think I can look into preparing a patch for that too.
> > > > > > > > But for such variant I would suggest using function names like
> > > > > > > > "from_hex/base64" instead of "hex/base64", or something including
> > > > > > > > bytestring reference: "bs_hex". Because the simple variants could be
> > > > > > > > misleading when used not only in the limited set of scopes.
> > > > > > > > they can be thought of converting to hex/base64 representation too. Or they
> > > > > > > > could collide with "hex" function to convert from string to int, which
> > > > > > > > someone would need to implement in the future.
> > > > > > >
> > > > > > > Yes, that is true.
> > > > > > >
> > > > > > > You can try it if you are brave enough to add new f_val type.
> > > > > >
> > > > > > Take a look at the patch, please. Waiting for the critics and
> > > > > > improvement suggestions.
> > > > >
> > > > > Hi
> > > > >
> > > > > It looks pretty good. First, could you split it to at least four patches?
> > > >
> > > > Sure. I'll provide split patches later.
> > > >
> > > > >
> > > > > 1) unrelated changes, like the newline-in-string-constant
> > > > > 2) preparatory changes (functions in lib/bytestring.c, change to BYTESTRING lexer)
> > > > > 3) adding bytestring type to filter code (including FI_FROM_HEX inst)
> > >
> > > Added patches up to this point. There are also some fixes and
> > > modification. For example, I noted that 'bytestring' symbol for the
> > > type name conflicts with lexer's BYTESTRING id. So I had to rename
> > > lexer's BYTESTRING to BYTETEXT (like it is done for strings).
> > > For the following patches it is better to decide the structure of the
> > > new *eval* functions.
> > >
> > > > > 4) change to parser related to f_eval_val(), bytestring nonterminal and so on.
> >
> > Final patches to modify current f_eval_int() to generic approach. And
> > for nonterminal bytestring.
> > Again, waiting for comments/suggestions. Also feel free, of course, to
> > fix naming/etc when applying.
> > Next, I will be able to move forward with patches to the documentation.
> >
> > > > >
> > > > > Some more comments:
> > > > >
> > > > > > It was needed to add another function like f_eval_int(), so I decided
> > > > > > to do some more generic approach and replaced all occurences of
> > > > > > f_eval_int() with it.
> > > > >
> > > > > That is good approach, although it would be probably better to call this
> > > > > function like cf_eval(), associated macro as cf_eval_val, and keep some
> > > > > inline functions like cf_eval_int(), cf_eval_bs() and so on.
> > > > >
> > > > > Or perhaps cf_eval() could return f_val as return value, and have
> > > > > shorthand functions like:
> > > > >
> > > > > static inline cf_eval_int(..) { return cf_eval(.., T_INT).i; }
> > >
> > > I actually tried first to return the struct instead of modifying it by
> > > a reference. But for that we need to have "struct f_val" known in
> > > filter/filter.h, which is defined in filter/data.h. But that causes
> > > some circular dependencies problem. I didn't dig deep into it, but
> > > maybe it is possible to solve the conflict in a clean way.
> >
> > Looked at it again. It seems safe to include filter/data.h into
> > filter/filter.h. I probably had problems when trying to incude it
> > somewhere else, until it all finalized into filter.h
> >
> > >
> > > > >
> > > > > I will give more comments later.
> > > > >
> > > > > --
> > > > > Elen sila lumenn' omentielvo
> > > > >
> > > > > Ondrej 'Santiago' Zajicek (email: santiago at 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."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: doc-radv-custom-option.patch
Type: text/x-patch
Size: 1866 bytes
Desc: not available
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20230707/577d9e15/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: doc-bytestring-type.patch
Type: text/x-patch
Size: 3532 bytes
Desc: not available
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20230707/577d9e15/attachment-0001.bin>


More information about the Bird-users mailing list