Re: [Gen-art] Genart telechat review of draft-ietf-ospf-ospfv3-segment-routing-extensions-19

2018-12-04 Thread Alissa Cooper 
Pete, thanks for your review. Acee and Peter, thanks for your responses. I 
entered a No Objection ballot. I can see Pete’s concern but I’m hopeful that 
given the existing deployment experience and the added clarification in the -20 
the concern will be mitigated in practice.

Alissa

> On Dec 3, 2018, at 5:33 PM, Pete Resnick  wrote:
> 
> Hi Acee,
> 
> On 3 Dec 2018, at 15:46, Acee Lindem (acee) wrote:
> 
>> Hi Pete,
>> 
>> While both you and I would have done it differently, the variable length SID 
>> encoding across the three LSR protocols (OSPFv2, OSPFv3, and IS-IS) has been 
>> implemented, deployed, and will not be changed during the IESG review 
>> (you'll note these SR protocol drafts have been in development for over 5 
>> years). There is, however, an update to all three which clarifies the usage 
>> of the flags. See (for example):
>> 
>>
>> https://tools.ietf.org/rfcdiff?difftype=--hwdiff=draft-ietf-ospf-ospfv3-segment-routing-extensions-20.txt
>> 
>> Thanks,
>> Acee (Document Shepherd and LSR Co-chair)
> 
> Totally understood, and that's why I said that I certainly don't want to 
> stand in the way of the doc. And I appreciate the clarification in -20; it 
> helps. My job as a GenART reviewer is to make sure that the Gen AD is aware 
> of any issues. I think it's highly unlikely that she (or anyone on the IESG) 
> would balk at this point in history.
> 
> Thanks for your (and Peter's) quick replies to these reviews.
> 
> pr
> 
> -- 
> Pete Resnick http://www.episteme.net/
> All connections to the world are tenuous at best
> 
> ___
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art

___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Re: [Gen-art] Genart telechat review of draft-ietf-ospf-ospfv3-segment-routing-extensions-19

2018-12-03 Thread Pete Resnick 

Hi Acee,

On 3 Dec 2018, at 15:46, Acee Lindem (acee) wrote:


Hi Pete,

While both you and I would have done it differently, the variable 
length SID encoding across the three LSR protocols (OSPFv2, OSPFv3, 
and IS-IS) has been implemented, deployed, and will not be changed 
during the IESG review (you'll note these SR protocol drafts have been 
in development for over 5 years). There is, however, an update to all 
three which clarifies the usage of the flags. See (for example):



https://tools.ietf.org/rfcdiff?difftype=--hwdiff=draft-ietf-ospf-ospfv3-segment-routing-extensions-20.txt

Thanks,
Acee (Document Shepherd and LSR Co-chair)


Totally understood, and that's why I said that I certainly don't want to 
stand in the way of the doc. And I appreciate the clarification in -20; 
it helps. My job as a GenART reviewer is to make sure that the Gen AD is 
aware of any issues. I think it's highly unlikely that she (or anyone on 
the IESG) would balk at this point in history.


Thanks for your (and Peter's) quick replies to these reviews.

pr

--
Pete Resnick http://www.episteme.net/
All connections to the world are tenuous at best

___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Re: [Gen-art] Genart telechat review of draft-ietf-ospf-ospfv3-segment-routing-extensions-19

2018-12-03 Thread Acee Lindem (acee) 
Hi Pete, 

On 12/3/18, 2:19 PM, "Pete Resnick"  wrote:

Reviewer: Pete Resnick
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

.

Document: draft-ietf-ospf-ospfv3-segment-routing-extensions-19
Reviewer: Pete Resnick
Review Date: 2018-12-03
IETF LC End Date: 2018-11-16
IESG Telechat date: 2018-12-06

Summary: I think this document is ready, and I certainly don't want to 
stand in
the way of it moving forward, but I do want to note the following issues I
mentioned in my previous review. The document editor notes that similar 
sorts
of things have been done in previous OSPF document without problems, but 
they
still make me nervous. Thanks to the editor for quickly addressing all of 
the
issues in my previous review.

Major/minor issues:

In 3.1:

  Length: Either 3 or 4 octets

  SID/Label: If length is set to 3, then the 20 rightmost bits
  represent a label.  If length is set to 4, then the value
  represents a 32-bit SID.

This sort of mechanism worries me. The Length is not a length, but rather a
flag. This means you can't have a general parsing implementation, as it 
would
treat the field as a length and get the left-most 24 bits when the value is 
3.
Even if the implementation chooses the right-most 24 bits, it's only 
supposed
to take the right-most 20 bits and mask off the extra 4 bits, which are not
required to be zeroed. I understand that similar things have been done 
before
without problems, but this seems like an implementation accident waiting to
happen.

In 7.1 and 7.2:

When the V-flag is set (making SID/Index/Label is a label), the value is in 
the
low 20 bits of the first 3 bytes of the field (i.e., bits 4-23). As with the
comment regarding 3.1, this seems like it has the potential for 
implementation
problems. You could explicitly say to mask of bits 0-3 and 24-31 (since 
there
is no requirement for producing implementations to clear those bits) and 
shift
the value 8 bits to the right, but this just seems like a bad way to design
this. That said, I again understand that similar things have been done 
before
without problems.

While both you and I would have done it differently, the variable length SID 
encoding across the three LSR protocols (OSPFv2, OSPFv3, and IS-IS) has been 
implemented, deployed, and will not be changed during the IESG review (you'll 
note these SR protocol drafts have been in development for over 5 years). There 
is, however, an update to all three which clarifies the usage of the flags. See 
(for example): 


https://tools.ietf.org/rfcdiff?difftype=--hwdiff=draft-ietf-ospf-ospfv3-segment-routing-extensions-20.txt

Thanks, 
Acee (Document Shepherd and LSR Co-chair) 



Nits/editorial comments:

None.



___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

[Gen-art] Genart telechat review of draft-ietf-ospf-ospfv3-segment-routing-extensions-19

2018-12-03 Thread Pete Resnick 
Reviewer: Pete Resnick
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

.

Document: draft-ietf-ospf-ospfv3-segment-routing-extensions-19
Reviewer: Pete Resnick
Review Date: 2018-12-03
IETF LC End Date: 2018-11-16
IESG Telechat date: 2018-12-06

Summary: I think this document is ready, and I certainly don't want to stand in
the way of it moving forward, but I do want to note the following issues I
mentioned in my previous review. The document editor notes that similar sorts
of things have been done in previous OSPF document without problems, but they
still make me nervous. Thanks to the editor for quickly addressing all of the
issues in my previous review.

Major/minor issues:

In 3.1:

  Length: Either 3 or 4 octets

  SID/Label: If length is set to 3, then the 20 rightmost bits
  represent a label.  If length is set to 4, then the value
  represents a 32-bit SID.

This sort of mechanism worries me. The Length is not a length, but rather a
flag. This means you can't have a general parsing implementation, as it would
treat the field as a length and get the left-most 24 bits when the value is 3.
Even if the implementation chooses the right-most 24 bits, it's only supposed
to take the right-most 20 bits and mask off the extra 4 bits, which are not
required to be zeroed. I understand that similar things have been done before
without problems, but this seems like an implementation accident waiting to
happen.

In 7.1 and 7.2:

When the V-flag is set (making SID/Index/Label is a label), the value is in the
low 20 bits of the first 3 bytes of the field (i.e., bits 4-23). As with the
comment regarding 3.1, this seems like it has the potential for implementation
problems. You could explicitly say to mask of bits 0-3 and 24-31 (since there
is no requirement for producing implementations to clear those bits) and shift
the value 8 bits to the right, but this just seems like a bad way to design
this. That said, I again understand that similar things have been done before
without problems.

Nits/editorial comments:

None.

___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art