Re: [Bier] (Carsten) Benjamin Kaduk's Discuss on draft-ietf-bier-te-arch-10: (with DISCUSS and COMMENT)

[Toerless Eckert <[e-mail]>]

Hi Carsten,

Thanks for the suggestion Detail replies below inline.

Changes for for just your feedback are together in the commit for Ben Kaduks feedback:

http://tools.ietf.org//rfcdiff?url1=https://www.ietf.org/archive/id/draft-ietf-bier-te-arch-11.txt&url2=https://raw.githubusercontent.com/toerless/bier-te-arch/8ed27af30d4da5883aeab7d63a2c427fa2922b1a/draft-ietf-bier-te-arch.txt

Full diff from -11 to -12:

http://tools.ietf.org//rfcdiff?url1=https://www.ietf.org/archive/id/draft-ietf-bier-te-arch-11.txt&url2=https://www.ietf.org/archive/id/draft-ietf-bier-te-arch-12.txt

Cheers
    Toerless


On Wed, Dec 22, 2021 at 07:11:07PM +0100, Carsten Bormann wrote:
> On 2021-08-26, at 08:59, Benjamin Kaduk via Datatracker <[e-mail]> wrote:
> > 
> > pseudocode in Figure 6
> 
> I’m sorry, I’m not an expert about BIER-TE, so take the below with a grain of salt.
> 
> I ran into the pseudocode in a discussion we had about including pseudocode in an RFC.
> 
> The code I find is “almost C”; I don’t understand why you didn’t go the last mile so you can check what you did.

The goal of pseudocode is to compress code down to the algorithmically
important part and avoid cluttering up an RFC with all the uninteresting
syntactic decorations of existing programming languages,
such as variable definitions. 

The check you mention would only be a syntax check, so without a lot more
comprehensive C-level PoC implementation, there would be IMHO no useful
functional checking. Admittedly, a C-level PoC implementation would be nice, but
not IMHO to be printed in the RFC itself. Unless we had some standard
reference library of a virtual router with send/receive packet functions
or the like, so we could stick to just new code... Certainly interesting
line of thoughts to think more about though.

Proof in point: the "~" operator that i put into the wrong one of two lines
would have only been found by actual PoC code running, not by syntax check.

To give an IMHO stronger argument against trying to limit onesself
to just C:

Check out the pseudo code in our follow-up work, draft-eckert-bier-cgm2-rbs,
section 5. For that code i am declaring that the pseudo code uses bit instead
of byte accurate pointer address calculations and dereferncing. Simple to
declare for pseudo-code and resulting in IMHO easy to read pseudocode. Now try to
convert it into C, and you end up in a Shift&Mask nightmare or another
layer of bitstring processing library code you'd first have to explain.

> An indicator that gives away the code might have problems is the different spacing for
>        SI=GetPacketSI(Packet);
>        Offset=SI*BitStringLength;
> (no spaces around operators) vs. much, but not all, of the rest.

I tried to minimize the amount of new pseudocode ideas i introduced, so
i simply cloned the style from RFC8279, section 6.5, which introduced
this inconsistency.

But agree, no need for this inconsistency, so i inserted spaces
in the new (second) pseudocode, but keeping the dense formatting in
the first pseudocode copied from rfc8279 for easier comparison.

> But more importantly, what does this pseudocode mean:
> 
>            foreach adjacency BIFT[Index+Offset] {
> 
> Is there an “in” missing?
> I learn from the explanation that the code "loops over the adjacencies because there may be more than one adjacency for a bit”, so maybe there is a mapping from a bit position (not the bit itself, which is 1 or 0) to a list of adjacencies?

Right. That is pseudocode i made up because i did not want the syntactic
C-overhead of array start/end syntax. I used perl syntax, but i guess nobody remembers 
that language.

I changed it to bash/python "for adjacency in BIFT[Index+Offset] and added "->Adjacencies"
to make it clearer what object we're looping over.

<t>BIFT entries may contain more than one adjacency in support of specific configurations such as <xref target="hubnspoke"/>. The code therefore includes a for loop over these adjacencies.</t>

> What is the difference between commas inside and outside the {}?
> 
>                    case forward_routed({VRF},l3-neighbor):
>                        SendToL3(PacketCopy,{VRF,}l3-neighbor);

Typo on first second line. Fixed. Thanks.

You do score one point for a syntax checker here. Except that
it is really nice to be able to make up simple syntax as we go
(and admittedly, as long as it is easily understood by readers!).

> These are questions that an implementer who is tasked to look at this pseudocode will have in all likelihood, so I would expect that the WG wants this clarified.

Definitely. Thanks for the feedback. Hope the fixes make the pseudocode
still short but unambiguous to read.

Toerless

> Grüße, Carsten