stoffi92 / rfc5575bis

0 stars 0 forks source link

IESG Benjamin Kaduk: Section 4.2 #190

Closed stoffi92 closed 4 years ago

stoffi92 commented 4 years ago

Section 4.2

I think we should say something about how multi-byte values are encoded, especially given that we implicitly allow for different encodings of a given value (e.g., IP protocol could have padding, and small port numbers don't need to be encoded in 2 bytes). Even just making it more explicit that multiple encodings are allowed (and what the semantics are for any such "padding") would be worthwhile. Is there a maximum encoded length for anything other than the constraints of the 2-bit 'len' field? Interestingly, for DCSP we are much more strict -- why is the strictness needed there but not in general? Is this just to faithfully reproduce RFC 5575, as implied by the discussion in Appendix B? (I would make this a DISCUSS point if I didn't think that we're acting as if we're required to preserve the RFC 5575 behavior.)

A NLRI value not encoded as specified in Section 4.2 is considered

This is Section 4.2; perhaps "specified here" or "specified below"?

suehares commented 4 years ago

RFC7606 covers general BGP references for exceeding fields and what to do. We do not restate this because the variable length operators are specified there. I'm trying understand where more text is going to make this clearer. Section 4.2 top says a list. Section 4.3 gives example.
Yes - DSCP is more strict because we are reproducing RFC5575. RFC5575 did not have RFC7606.

Ben is an excellent reviewer - but I'm not clear what should change. Robert or Christoph - do you have any idea? Otherwise, I will ask Ben.

suehares commented 4 years ago

We should flag this as an issue - rather than a nit. I will leave it to Christoph change the status.

stoffi92 commented 4 years ago

//doc The component types using the bitmask_op are using a strict encoding size (MUST) for a consistent mapping of the bitmask bits to the value. Those that use the numeric_op allow a variable length (SHOULD). RFC7606 covers general BGP references for exceeding fields and what to do.

stoffi92 commented 4 years ago

Should we act on this? It seems consistent (bitmask_op have MUST (to have a exact matching to the bits) while numeric_op used SHOULD RFC5575 uses neither MUST nor SHOULD with any of the components). With the current solution we are consistent in itself and not breaking current deployments. -> @suehares please comment and either close or assign it back to me for changes.

suehares commented 4 years ago

3rd choice - I think the text is fine. Robert should review. I will assign it to all 3 of us.

raszuk commented 4 years ago

Christoph statements look fine to me.

Many thx, R.

On Mon, Apr 27, 2020 at 4:10 PM suehares notifications@github.com wrote:

Assigned #190 https://github.com/stoffi92/rfc5575bis/issues/190 to @raszuk https://github.com/raszuk.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/stoffi92/rfc5575bis/issues/190#event-3275969321, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIH6EBBSPJTZQ3LUHZUOYTROWG5RANCNFSM4MOZRAXQ .

stoffi92 commented 4 years ago

Nothing to do. -> Close