Adding serializable enums (or enums with backing types and specified representations). #615

akeep · 2018-05-02T22:11:33Z

Hey All,

Following up on the discussion in #611, this pull request attempts to capture the proposal in the P4-16 specification document.

jafingerhut · 2018-05-02T23:01:18Z

p4-16/spec/P4-16-spec.mdk

+E e1 = E.e2;
+E e2;
+
+x  = (bit<8>) e1; // sets x to 1


Should that be "sets x to 0"?

I may be wrong in my comment, but note that it is fairly confusing to have variables with the exact same names as the enum member names, e.g. e1, e2 are used as both in this example code.

No, e1 = E.e2, which is 1. I should, however, chose different names for variables e1 and e2 (that was a really confusing choice on my part!)

jafingerhut · 2018-05-02T23:14:15Z

p4-16/spec/P4-16-spec.mdk

+
+1. if an integer $x \notin N$ then $\forall e \in S$, `(E)x != e`
+1. $\forall x \in [0, 2^W]$, `(bit<W>)(E)x == x`
+1. $\forall e \in S$, `(E)(bit<W>)e == e`


So if I roughly translate this into English, if an enum type with a backing type bit<W>, where the numeric values defined by the enum definition do not include all possible 2^W values of the backing type, then it is guaranteed that variables of that enum type might not be equal to any of the named values in the enum type?

Is the comment in the last 'else' branch below correct, given this proposal?

enum bit<2> MyPartialEnum_t { VALUE_A = 2w0, VALUE_B = 2w1, VALUE_C = 2w2 } MyPartialEnum_t x; if (x == MyPartialEnum_t.VALUE_A) { // some code here } else if (x == MyPartialEnum_t.VALUE_B) { // some code here } else if (x == MyPartialEnum_t.VALUE_C) { // some code here } else { // A P4 compiler MUST ASSUME that this branch can be executed // some code here }

I think that's the idea. Although a more realistic scenario would probably look like this:

enum bit<16> EtherType { VLAN = 0x8100, IPV4 = 0x0800, IPV6 = 0x86dd } header ethernet { ... EtherType etherType; ... } parser my_parser(...) { state parse_ethernet { packet.extract(hdr.ethernet); transition select(hdr.ethernet.etherType) { EtherType.VLAN : parse_vlan; EtherType.IPV4 : parse_ipv4; EtherType.IPV6: parse_ipv6; default: reject; } }

I think it's important to have the guarantee that if the etherType extracted from the packet is not one of 0x8100 (VLAN), 0x0800 (IPV4) or 0x86dd (IPV6), then the default transition will be taken.

All of the guarantees listed here are achieved by a "trivial" implementation where an enum with an underlying type is completely taken care of at compile time and doesn't exist at runtime. I think that's what makes the enum useful: it makes it more convenient to write programs and generate runtime APIs, and it doesn't have any packet-processing runtime cost.

That being said I'm fine with introducing extern methods to safely cast enums or to check if a value is valid, as @jamescoole-cisco suggested, as long as they are optional when it comes to actually using enums in a P4 program.

Should one (or both) of these examples be added as explanation to the math?

It's not obvious that the first example is unique to enums with backing types. If uninitialized enum variables are intended to be "undefined" only within valid entries of the enum, it might be good to state that explicitly.

However, if that's the case, (compiler) implementations will already need to initialize non-backed enum variable storage in most cases, so why not require the same for enums with backing types?

@jamescoole-cisco I actually wrote a change to the P4_16 language spec, committed after v1.0.0 was published, that says something about exactly this case, but had forgotten I wrote that part. See #611 (comment) for details. Unless that text is changed further in the spec, P4_16 compilers are already free to make uninitialized enum and error values unequal to any of their named members.

Given my previous comment, I am not against having example code like these in the spec, but if so, they should be clear that the examples apply for enum's with or without underlying types.

I think having no code examples would also be fine, but adding a sentence that mentions that all values with type enum can be not-equal to any of their named members, with a cross-reference to the section "## Operations on headers { #sec-ops-on-hdrs }" (Madoko syntax copied from the .mdk file)

I agree it would be good to copy or move the discussion of how undefined both types of enums can be into section, since it applies in situations besides header fields. But IDK if the example is necessary to get the point across.

jafingerhut · 2018-05-02T23:21:12Z

p4-16/spec/P4-16-spec.mdk

+symbolic value `E.e2`, which corresponds to the numeric value `1`.
+
+Note that while it is always safe to cast from an `enum` to its numeric type,
+it may not be th case that every numeric value specified by the backing type


typo: "th" -> "the" ?

jamescoole-cisco · 2018-05-02T23:18:11Z

p4-16/spec/P4-16-spec.mdk

+for each entry in the `enum` and provides a backing type: `bit<16>`.
+Implementations are expected to raise an error if the numeric representation
+for an enumeration entry falls outside the representation range of the backing
+type.


It seems like the issue of multiple entries having the same representation should be discussed somewhere in here. For example, whether this is allowed:

enum bit<16> SomeEnum { Case1 = 0x0001, Case2 = 0x0001, // related, can I say instead "= Case1"? Case3 = 0x0002 }

Good point. Given the contents of the rest of the proposal, it seems that this should not be allowed.

+1 to disallow this

jamescoole-cisco · 2018-05-03T00:40:11Z

p4-16/spec/P4-16-spec.mdk

+sets `e` to an unknown value, since there is no symbol corresponding to the
+numeric value `5`.  While the value of `e` is unspecified, it must not
+correspond to any symbol defined within enum `E`.  This is because casting from
+a numeric type to an `enum` type is required obey the following semantics:


Some of this has been discussed before and was in the proposal, but I'm not sure where it ended up.

It seems that providing something like bool is_valid<Enum, Back>(Back b) (or bool is_valid<Enum>(Enum e)), either as part of the language or an extern, would be useful.

If a user wants to check that an unsafe cast will (or did) succeed, they're still going to even if these aren't provided, but the code they write will need to be updated whenever the enum definition changes.

antoninbas

LGTM, I'm approving this PR with the understanding that all review comments will be addressed.

antoninbas · 2018-05-03T17:03:38Z

p4-16/spec/P4-16-spec.mdk

+introduces a new enumeration type, which contains five constants---e.g.,
+`EtherType.IPV4`.  This `enum` declaration specifies the numeric representation
+for each entry in the `enum` and provides a backing type: `bit<16>`.
+Implementations are expected to raise an error if the numeric representation


I would explicitly say "Compiler implementations"

antoninbas · 2018-05-03T17:04:23Z

p4-16/spec/P4-16-spec.mdk

+`EtherType.IPV4`.  This `enum` declaration specifies the numeric representation
+for each entry in the `enum` and provides a backing type: `bit<16>`.
+Implementations are expected to raise an error if the numeric representation
+for an enumeration entry falls outside the representation range of the backing


Nit: maybe you should use "underlying type" consistently instead of "backing type"?

antoninbas · 2018-05-03T17:04:44Z

p4-16/spec/P4-16-spec.mdk

+for each entry in the `enum` and provides a backing type: `bit<16>`.
+Implementations are expected to raise an error if the numeric representation
+for an enumeration entry falls outside the representation range of the backing
+type.


+1 to disallow this

antoninbas · 2018-05-03T17:05:44Z

p4-16/spec/P4-16-spec.mdk

+sets `e` to an unknown value, since there is no symbol corresponding to the
+numeric value `5`.  While the value of `e` is unspecified, it must not
+correspond to any symbol defined within enum `E`.  This is because casting from
+a numeric type to an `enum` type is required obey the following semantics:


s/required obey/required to obey/ ?

antoninbas · 2018-05-03T17:17:25Z

p4-16/spec/P4-16-spec.mdk

+
+1. if an integer $x \notin N$ then $\forall e \in S$, `(E)x != e`
+1. $\forall x \in [0, 2^W]$, `(bit<W>)(E)x == x`
+1. $\forall e \in S$, `(E)(bit<W>)e == e`


I think that's the idea. Although a more realistic scenario would probably look like this:

enum bit<16> EtherType { VLAN = 0x8100, IPV4 = 0x0800, IPV6 = 0x86dd } header ethernet { ... EtherType etherType; ... } parser my_parser(...) { state parse_ethernet { packet.extract(hdr.ethernet); transition select(hdr.ethernet.etherType) { EtherType.VLAN : parse_vlan; EtherType.IPV4 : parse_ipv4; EtherType.IPV6: parse_ipv6; default: reject; } }

I think it's important to have the guarantee that if the etherType extracted from the packet is not one of 0x8100 (VLAN), 0x0800 (IPV4) or 0x86dd (IPV6), then the default transition will be taken.

All of the guarantees listed here are achieved by a "trivial" implementation where an enum with an underlying type is completely taken care of at compile time and doesn't exist at runtime. I think that's what makes the enum useful: it makes it more convenient to write programs and generate runtime APIs, and it doesn't have any packet-processing runtime cost.

That being said I'm fine with introducing extern methods to safely cast enums or to check if a value is valid, as @jamescoole-cisco suggested, as long as they are optional when it comes to actually using enums in a P4 program.

jamescoole-cisco · 2018-05-03T19:00:26Z

p4-16/spec/P4-16-spec.mdk

 specified, so their "size" in bits is not specified (it is
 target-specific).

+It is also possible to specify an `enum` with an underlying representation.
+This requires the programmer provide both the numeric type and an associated


Should "the numeric type" be "a fixed-width integer type"?

mihaibudiu · 2018-05-03T22:42:09Z

p4-16/spec/P4-16-spec.mdk

+representations ($n$), the following properties must hold:
+
+1. if an integer $x \notin N$ then $\forall e \in S$, `(E)x != e`
+1. $\forall x \in [0, 2^W]$, `(bit<W>)(E)x == x`


I don' t know why you want this property to hold. Who cares about it?

Do you mean to say the property holds by definition of an enum with underlying type and doesn't need to be spelled out explicitly?

@mbudiu-vmw If we are going to the trouble of specifying an underlying type that is bit<W>, are you saying you think a compiler would gain some kind of efficiency advantage in a target by violating that property?

More directly related to your question, if implementations guaranteed the property, it certainly makes explaining exactly how an enum with an underlying type works -- it is really a bit<W> "under the hood", and casts between bit<W> and the enum type never change the underlying bit pattern. Done. End of story. No weird corner cases to think about, except that such an enum value might not be equal to any of the named enum member values.

If that is the model, then we should say so explicitly in the spec.
This will preclude other implementations, and will also make it easier for people to understand what is going on. Do not leave freedom to the compiler writers if you think you will be sorry about it later. Like people designing C did with respect to integer overflow.

This was requested in the issue #611 related to this and I basically pulled the text from there (with a lite-rewording & turning some text in math). Where do we stand on this?

Personally, I don't feel strongly about it one way or the other.

mihaibudiu · 2018-05-03T22:43:09Z

p4-16/spec/P4-16-spec.mdk

+~ End P4Example
+
+sets `e` to an unknown value, since there is no symbol corresponding to the
+numeric value `5`.  While the value of `e` is unspecified, it must not


I think it is simpler to keep this to be unspecified, and make no such promises.

Strongly disagree. I believe that would greatly limit how these enums can be used. Please see the parser example I gave below in a different comment.

This property is important for programmers expectations. Without it the example that @antoninbas showed in the comments above is not something that a programmer could rely on working across P4 implementations.

Ah, good catch, I missed the unspecified in this one. That said, the While the value ... has now been removed.

akeep · 2018-05-04T15:18:39Z

Hey All,

I've put in a new version that tries to address most of the feedback. It seemed like the discussion between @mbudiu-vmw and @antoninbas was not quite concluded and I did not make changes related to those conversations (though I did comment on them inline).

I also did not add the examples @jafingerhut and @antoninbas noted with regard to the serialization rules, but I'm happy to add them if you all think it will help add clarity.

Thanks for the all the feedback, let me know what you think!

-andy:)

mihaibudiu · 2018-05-04T15:46:12Z

My proposal is to actually rephrase this to make it look more like a bit type with a bunch of constant values. Then the compiler does not have a choice: it will always implement such an enum with the underlying type. Also, the semantics of values that are not one of the constants is much simpler to describe. And you get all the properties that you are asking for for free.

So enum bit<32> X { A = 1, B = 2 } is in fact just a bit<32>, with a bunch of named constants X.A and X.B. Then we don't even have the requirement for A and B to be distinct, they can have any values you want. The compiler will treat them just as constants. Unlike a bit<32>, but much like a newtype (at least as it is today) this enum will have no arithmetic operations except tests for equality, assignments and casts from/to bit<32>. The most important gain is that now the semantics of a cast is very simple to describe: the same underlying bit representation is kept.

What do you think? So I think that the syntax is unchanged, you get all the properties you wanted to get, but it is much simpler for a programmer to understand what such an enum is doing. All the business about undefined values is gone.

antoninbas · 2018-05-04T16:44:55Z

@mbudiu-vmw that works for me

…'s request to make the enum with underlying type more like a newtype based on the underlying type.

akeep · 2018-05-07T18:05:39Z

@antoninbas, @jamescoole-cisco, @jafingerhut I've added the examples with slight modification and moved the commentary around undefined values up into the operations section.

@mbudiu-vmw I've tried to add some sentences to the enum definition section to talk about it more like a newtype based on the bit<W> underlying type.

mihaibudiu · 2018-05-07T18:10:13Z

p4-16/spec/P4-16-spec.mdk

+
+Any `enum` that contains an unspecifed value, whether as the result of a cast
+to an `enum` with an underlying type, parse into the field of an `enum` with an
+underlying type, or simply the declaration of any `enum` without a specified


I think this explanation can now be also simplified by reducing it to the case of bitstrings.

I'm not sure what you mean by this.

These should be changed to "unnamed" as well. This whole paragraph is really unnecessary.
A question is whether we allow matching against raw constants:

select (hdr.ethernet.etherType) { 0x8000: something }

mihaibudiu · 2018-05-07T18:10:29Z

p4-16/spec/P4-16-spec.mdk

@@ -2076,6 +2076,14 @@ enum bit<16> EtherType {
 introduces a new enumeration type, which contains five constants---e.g.,
 `EtherType.IPV4`.  This `enum` declaration specifies the fixed-width integer representation
 for each entry in the `enum` and provides an underlying type: `bit<16>`.
+This type of `enum` declaration can be thought of as declaring a new `bit<16>`


Do we allow signed enum values as well?

I've used unsigned in all of the examples, but I don't think there is any particular reason to restrict it to this. Do you have an opinion? (I'm happy to add a case for it.)

jafingerhut · 2018-05-07T19:53:24Z

p4-16/spec/P4-16-spec.mdk

+E b;
+
+x = (bit<8>) a; // sets x to 1
+b = (E) x;      // sets e to E.e2


typo: "sets e" -> "sets b"

jafingerhut · 2018-05-07T19:59:12Z

p4-16/spec/P4-16-spec.mdk

+sets `e` to an unknown value, since there is no symbol corresponding to the
+fixed-width integer value `5`.  While the value of `e` is unspecified, it must not
+correspond to any symbol defined within enum `E`.  This is because casting from
+a fixed-width integer type to an `enum` type is required to obey the following semantics:


I know you have the later condition that (bit)(E)x == x for all possible 2^W values of x of type bit<W>. Assuming we keep that condition (seems reasonable to me), it isn't really true that the assignment to e in the example code above assigns e an "unknown" value? I mean, it must be a value not equal to any named member of type E, I agree, but it must also be able to be cast back to a bit<8> type value of 5, and no other, so we know a lot more about it than that it is simply unknown.

I mention this simply because uninitialized values in P4_16 are, I believe, "more unknown" than the value of e in this sample code.

What would you refer to this value as, if not unknown?

…ked for

mihaibudiu · 2018-05-09T16:13:39Z

p4-16/spec/P4-16-spec.mdk

+}
+~ End P4Example
+
+Note that each symbolic value in the enum must map to a unique fixed-width integer


Did we decide to keep the "unique" requirement?

Can case names be used while defining the enum?

This is particularly useful if aliases are allowed:

enum bit<32> Authors { Clemens = 0, Twain = Clemens // is Clemens defined here? is Authors.Clemens? }

Not clear, a declaration is available only after it has been "completed". Not clear whether the Clemens declaration ends with the comma or with the closing brace.

I'd argue that since it's useful, it should be allowed. But there should be some language to say one way or another.

mihaibudiu · 2018-05-09T16:15:24Z

p4-16/spec/P4-16-spec.mdk

+~ End P4Example
+
+sets `e` to an unnamed value, since there is no symbol corresponding to the
+fixed-width integer value `5`.  While the value of `e` is unspecified, it must not


I would say here that the value of e is unnamed. It certainly is not unspecified, in fact it is 5. The phrase about "must not correspond" is no longer necessary.

mihaibudiu · 2018-05-09T16:16:03Z

p4-16/spec/P4-16-spec.mdk

+sets `e` to an unnamed value, since there is no symbol corresponding to the
+fixed-width integer value `5`.  While the value of `e` is unspecified, it must not
+correspond to any symbol defined within enum `E`.  This is because casting from
+a fixed-width integer type to an `enum` type is required to obey the following semantics:


The phrase starting with "This is because" is probably unnecessary now.

mihaibudiu · 2018-05-09T16:17:01Z

p4-16/spec/P4-16-spec.mdk

+correspond to any symbol defined within enum `E`.  This is because casting from
+a fixed-width integer type to an `enum` type is required to obey the following semantics:
+
+Given an enum type $E$ with underlying type `bit<W>` and the set of mappings


These are no longer requirements, they are consequences of the definition. They will always hold.

I removed all of the language around this.

mihaibudiu · 2018-05-09T16:19:19Z

p4-16/spec/P4-16-spec.mdk

+
+Any `enum` that contains an unspecifed value, whether as the result of a cast
+to an `enum` with an underlying type, parse into the field of an `enum` with an
+underlying type, or simply the declaration of any `enum` without a specified


These should be changed to "unnamed" as well. This whole paragraph is really unnecessary.
A question is whether we allow matching against raw constants:

select (hdr.ethernet.etherType) { 0x8000: something }

mihaibudiu · 2018-05-09T16:21:20Z

p4-16/spec/P4-16-spec.mdk

+use the specified underlying type as the serialization data type and
+representation.
+
+In addition to these operations a P4 implementation may choose to implement


I don't know if we need to put this into the spec; it can be in the architecture definition file of the targets which do implement such functions. Also, "is_valid" should be probably called "is_named".

I went ahead and removed these, I agree having them as part of an architecture description is probably right.

Not sure why I cannot separately reply to the question about matching against raw constants, but I would say this:

I don't think we should allow this because I think the enum should be a distinct type that can be cast to the underlying type, but not treated exactly the same as the underlying type. I think if you really want to use the integer value (which really defeats the whole purpose of putting in an enum in the first place to me), it would be:

select ((bit<16>)hdr.ethernet.etherType) { 0x8000: something }

I think forcing the programmer to be explicit about this is in keeping with the requirement for explicit casts in other places in the language.

I've not yet written anything up about this, I think that is the one outstanding issue.

akeep · 2018-05-10T15:38:02Z

Hey All,

I think I've addressed everything with the exception of whether an enum value should be matchable in a select as the underlying representation. As mentioned in a comment above, I'm not a fan of that, but I've not added any text to address it yet.

mihaibudiu · 2018-05-15T00:49:21Z

p4-16/spec/P4-16-spec.mdk

@@ -2026,12 +2026,22 @@ An enumeration type is defined using the following syntax:
 ~ Begin P4Grammar
 enumDeclaration
    : optAnnotations ENUM name '{' identifierList '}'
+    | optAnnotations ENUM typeRef name '{' specifiedIdentifierList '}'


We could make this even more specific:

optAnnotations ENUM BIT '<' INTEGER '>' name

You noted this here, but then noted later that the typeRef here worked, do you have a preference?

mihaibudiu · 2018-05-16T00:34:59Z

p4-16/spec/P4-16-spec.mdk

+}
+~ End P4Example
+
+Additionally, within an `enum` declaration, earlier symbols may be referred to in later


Can we leave this out of the spec for now?
If anyone asks for this feature we should clarify.
But it will be easier to implement without it.

mihaibudiu · 2018-05-16T00:36:36Z

p4-16/spec/P4-16-spec.mdk

+fixed-width integer representation for `E.e2`, `1`.  The variable `b` is then set to the
+symbolic value `E.e2`, which corresponds to the fixed-width integer value `1`.
+
+Note that while it is always safe to cast from an `enum` to its fixed-width integer type,


Maybe "safe" is not the right word here; with our current definition both casts are actually "safe", but the second one does not create a "named" value.

mihaibudiu · 2018-05-16T00:37:37Z

p4-16/spec/P4-16-spec.mdk

+}
+~ End P4Example
+
+Any `enum` that contains an unnamed value, whether as the result of a cast


We should say here "variable with an enum type". The "enum" itself is the type declaration.

mihaibudiu · 2018-05-16T00:38:05Z

p4-16/spec/P4-16-spec.mdk

+Any `enum` that contains an unnamed value, whether as the result of a cast
+to an `enum` with an underlying type, parse into the field of an `enum` with an
+underlying type, or simply the declaration of any `enum` without a specified
+initial value might not be equal to any of the values defined for that


Actually I think that "might" is too weak: it "will not" be equal for sure.

I think the original intent of the sentence is that a variable of an enum type that contains at least one unnamed value might not be equal to any of the named values.

We could strengthen the sentence to "When a variable of an enum type with unnamed values is currently equal to one of those unnamed values, it will not be equal to any of the named values." But perhaps that would be redundant at this point in the text (I haven't carefully looked at the rest of the changes compared to this sentence.)

I just changed "might" to "will", I think that is accurate.

mihaibudiu · 2018-05-16T00:38:15Z

p4-16/spec/P4-16-spec.mdk

+to an `enum` with an underlying type, parse into the field of an `enum` with an
+underlying type, or simply the declaration of any `enum` without a specified
+initial value might not be equal to any of the values defined for that
+type.  Such an unspecified value should still lead to predictable


unspecified -> unnamed

mihaibudiu · 2018-05-16T00:39:08Z

p4-16/spec/grammar.mdk

@@ -334,6 +334,7 @@ structField

 enumDeclaration
    : optAnnotations ENUM name '{' identifierList '}'
+    | optAnnotations ENUM typeRef name '{' specifiedIdentifierList '}'


I have implemented this grammar change in the compiler and it will work out (using "bit" in fact) - there are no ambiguities.

I've left this alone for now, if you prefer bit, let me know.

akeep · 2018-05-21T22:04:04Z

Added unsigned to the description of the type you can use as the base type for a serializable enum type and updated the grammar to reflect this restriction.

akeep · 2018-05-23T14:10:09Z

Hey All,

I'm not planning on making any additional changes, since I think I've addressed @mbudiu-vmw requests and the requests that came up during the call on Monday. If there are needed changes please let me know soon, I'll be traveling over the weekend and my opportunities to handle changes are going to be limited.

mihaibudiu

Does anyone object merging this PR?

Adding enums with backing types.

33ee2b4

akeep mentioned this pull request May 2, 2018

Serializable Enums #611

Closed

cc10512 requested review from mihaibudiu, jnfoster, antoninbas and jafingerhut May 2, 2018 22:50

jafingerhut reviewed May 2, 2018

View reviewed changes

jamescoole-cisco reviewed May 2, 2018

View reviewed changes

jamescoole-cisco reviewed May 3, 2018

View reviewed changes

antoninbas approved these changes May 3, 2018

View reviewed changes

jamescoole-cisco reviewed May 3, 2018

View reviewed changes

mihaibudiu requested changes May 3, 2018

View reviewed changes

Addressed the resolved issues in pull request p4lang#615

34c7fc1

Added examples to the rules on enum, and tried to address @mbudiu-vmw…

8b0a64c

…'s request to make the enum with underlying type more like a newtype based on the underlying type.

mihaibudiu approved these changes May 7, 2018

View reviewed changes

jafingerhut reviewed May 7, 2018

View reviewed changes

Fixing unknown to unnamed and the set e comment to set b that Andy as…

83b1efa

…ked for

mihaibudiu requested changes May 9, 2018

View reviewed changes

Revisions to cover (most) of the comments from Mihai and James

4e90588

mihaibudiu requested changes May 16, 2018

View reviewed changes

akeep added 2 commits May 21, 2018 10:06

addressing latest round of change requests

e5be6fa

Updated explicitly require a bit<W> type for the serializable enum.

8b9f2af

mihaibudiu approved these changes May 23, 2018

View reviewed changes

jafingerhut approved these changes May 23, 2018

View reviewed changes

mihaibudiu merged commit 8ae0876 into p4lang:master May 25, 2018

akeep deleted the master branch May 1, 2021 01:20

Adding serializable enums (or enums with backing types and specified representations). #615

Adding serializable enums (or enums with backing types and specified representations). #615

Conversation

akeep commented May 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jafingerhut May 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescoole-cisco May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamescoole-cisco May 3, 2018 • edited Loading

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jafingerhut May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akeep commented May 4, 2018

mihaibudiu commented May 4, 2018

antoninbas commented May 4, 2018

akeep commented May 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akeep commented May 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jafingerhut May 2, 2018 •

edited

Loading

jamescoole-cisco May 4, 2018 •

edited

Loading

jamescoole-cisco May 3, 2018 •

edited

Loading

jafingerhut May 3, 2018 •

edited

Loading