Also, one additional issue which I think the document should address.

Since this document only introduces one dataplane option.  But it requires
that control plane signal options in the order that the receiver would like
to
see them, and the sender must honor that.  How does this work as additional
options get defined?  Does it mean that sending hardware must be built
so it can encapsulate in any order, but receiving hardware can be built in
a vendor specific fashion?  This is perhaps the most concerning issue about
this document for me because it means that every vendor might pick their
favorite order for receiving, and every sender is required to implement all
options.  Further, if a systems vendor has silicon from multiple vendors,
their
own control plane may be forced to operate differently.

On Sat, Jul 18, 2020 at 11:40 PM Anoop Ghanwani <[email protected]>
wrote:

> I have reviewed the doc and have the following comments.
>
> Anoop
>
> ==
> authors:
> Jorge is listed as working at Juniper but with a Nokia address. :)
>
> Throughout the document:
> Mixed use of GENEVE and Geneve.
> Suggest making it consistent (should be Geneve per the Geneve draft).
> There are also several occurrences of GENVE.  (A typo.)
>
> pg 1, abstract
> >>
>
> EVPN control plane can also be used by a Network Virtualization
>    Endpoints (NVEs) to express Geneve tunnel option TLV(s)supported in
>    transmission and/or reception of Geneve encapsulated data packets.
>
> >>
> to
> >>
>
> EVPN control plane can also be used by Network Virtualization
>    Edges (NVEs) to express Geneve tunnel option TLV(s) supported in
>    the transmission and/or reception of Geneve encapsulated data packets.
>
> >>
> (remove 'a', Endpoints -> Edges, add missing space after TLV(s), add 'the'
> before transmission)
>
> sec 1, pg 2
> >>
>
> The NVO3 working group have been working on
>    different dataplane encapsulations.  The Generic Network
>    Virtualizationi Encapsulation [I-D.ietf-nvo3-geneve 
> <https://tools.ietf.org/html/draft-ietf-bess-evpn-geneve-01#ref-I-D.ietf-nvo3-geneve>]
>  have been
>    recently recommended to be the proposed standard for network
>    virtualization overlay encapsulation.
>
> >>
>
> to
>
> >>
>
> The NVO3 working group has evaluated
>    different dataplane encapsulations.  The Generic Network
>    Virtualization Encapsulation [I-D.ietf-nvo3-geneve 
> <https://tools.ietf.org/html/draft-ietf-bess-evpn-geneve-01#ref-I-D.ietf-nvo3-geneve>]
>  has been
>    recently recommended to be the proposed standard for NVO3
>
>    encapsulation.
>
> >>
>
> (fixed typos and grammar.)
>
>
> control plane determines ->
>
> control plane determine
>
>
> pg 3
>
> Network Virtualization Edge devices (NVEs) ->
>
> Network Virtualization Edges (NVEs)
>
>
> >>
>
>    This EVPN control plane extension will allow a Network Virtualization
>    Edge (NVE) to express what Geneve option TLV types it is capable to
>
>    receive or to send over the Geneve tunnel to its peers.
>
> >>
>
> to
>
> >>
>
>    This EVPN control plane extension will allow an NVE
>
>    to express what Geneve option TLV types it is capable of
>    receiving from, or sending to, over the Geneve tunnel with
>
>    its peers.
>
> >>
>
>
> sec 3 seems to be a mix of abbreviations and terminology.  Would adjust the 
> title to cover both or split in 2 sections, one for each.
>
>
> sec 3, pg 3
>
> NVO3 Network Virtualization Overlays over Layer 3 ->
>
> NVO3: Network Virtualization Overlays over Layer 3.
>
>
> pg 4.
>
> >>>
>
> 4 <https://tools.ietf.org/html/draft-ietf-bess-evpn-geneve-01#section-4>.  
> GENEVE extensions
>
>    This document adds some extensions to the [I-D.ietf-nvo3-geneve 
> <https://tools.ietf.org/html/draft-ietf-bess-evpn-geneve-01#ref-I-D.ietf-nvo3-geneve>]
>    encapsulation that are relevant to the operation of EVPN.
> 4.1 <https://tools.ietf.org/html/draft-ietf-bess-evpn-geneve-01#section-4.1>. 
>  Ethernet option TLV
>
> >>>
>
> If there is only 1 extension, then I would say "document adds an 
> extension...".
>
>
> pg 4
>
> For GENVE encapsulation we need a bit to for this purpose. ->
>
> For Geneve, we need to add a bit for this purpose.
>
>
> pg5
>
> >>
>
>    Where: - Option Class is set to Ethernet (new Option Class requested
>    to IANA) - Type is set to EVPN-OPTION (new type requested to IANA)
>    and C bit must be set.  - B bit is set to 1 for BUM traffic.  - L bit
>    is set to 1 for Leaf-Indication.  - Source-ID is a 24-bit value that
>    encodes the ESI-label value signaled on the EVPN Autodiscovery per-ES
>    routes, as described in [RFC7432 <https://tools.ietf.org/html/rfc7432>] 
> for multi-homing and [RFC8317 <https://tools.ietf.org/html/rfc8317>] for
>    leaf-to-leaf BUM filtering.  The ESI-label value is encoded in the
>    high-order 20 bits of the Source-ESI field.
>
> >>
>
> This could benefit from some formatting.  Very hard to read, as is.
>
> R bit is not defined.
>
> EVPN-OPTION appears to be missing from IANA section.
>
>
>  "Local- bias" -> "Local-bias
>
> ESI- labels -> ESI-labels
>
> (extra space)
>
>
> >>
>
>    The egress NVEs that make use of ESIs in the data path (because they
>    have a local multi-homed ES or support [RFC8317 
> <https://tools.ietf.org/html/rfc8317>]
>
> >>
>
> Missing a closing parenthesis.
>
>
> >>>
>
> The use of the B bit is only relevant
>    to GENEVE packets with Protocol Type 0x6558 (Bridged Ethernet).
>
> >>>
>
> The entire option is relevant only for bridged ethernet, right?  What should
>
> an implementation do if it receives a packet where this is in conflict?
>
>
> pg 6
>
> >>>
>
> Where, an NVE receiving the above sub-TLV, will send GENEVE packets
>    to the originator NVE with with only the option TLVs the receiver NVE
>    is capable of receiving, and following the same order.
>
> >>>
>
> This statement could be made clearer.  I think it's combining control
>
> plane and data plane when describing originator or receiver.  Also, there
>
> are 2 "withs".
>
>
> >>>
>
> Also the high order bit in the type, is the critical bit, MUST be set 
> accordingly.
>
> >>>
>
> Please elaborate on what "set accordingly" means.  Use appropriate reference 
> to Geneve doc if needed.
>
>
> >>>
>
> In the datapath, NVE2, NVE3 and NVE4
>    only encapsulate overlay packets with the Geneve option TLV(s) that
>    NVE1 is capable of receiving.
>
> >>>
>
> to
>
> >>>
>
> In the datapath, NVE2, NVE3 and NVE4 may
> only encapsulate overlay packets with the Geneve option TLV(s) that
> NVE1 is capable of receiving, and in case more than one option TLV is
>
> being used, they must be in the order specified by NVE1.
>
> >>>
>
>
> sec 7
>
> >>
>
> Security considerations described in [RFC7432 
> <https://tools.ietf.org/html/rfc7432>] and in
>    [I-D.ietf-bess-evpn-overlay 
> <https://tools.ietf.org/html/draft-ietf-bess-evpn-geneve-01#ref-I-D.ietf-bess-evpn-overlay>]
>  are equally applicable.
>
> >>
>
> Also security considerations in the Geneve doc especially
>
> since securing options in Geneve has been a tricky topic.
>
>
> On Mon, Jul 13, 2020 at 4:46 AM <[email protected]> wrote:
>
>> Hello WG,
>>
>>
>>
>> This email starts a two weeks Working Group Last Call on
>> draft-ietf-bess-evpn-geneve-01 [1].
>>
>>
>>
>> This poll runs until * the 27th Of July *.
>>
>>
>>
>> We are also polling for knowledge of any undisclosed IPR that applies to
>> this Document, to ensure that IPR has been disclosed in compliance with
>> IETF IPR rules (see RFCs 3979, 4879, 3669 and 5378 for more details).
>>
>> If you are listed as an Author or a Contributor of this Document please
>> respond to this email and indicate whether or not you are aware of any
>> relevant undisclosed IPR. The Document won't progress without answers from
>> all the Authors and Contributors.
>>
>> There is currently no IPR disclosed.
>>
>>
>>
>> If you are not listed as an Author or a Contributor, then please
>> explicitly respond only if you are aware of any IPR that has not yet been
>> disclosed in conformance with IETF rules.
>>
>>
>>
>> We are also polling for any existing implementation as per [2].
>>
>>
>>
>> Thank you,
>>
>> Stephane & Matthew
>>
>>
>>
>> [1] https://datatracker.ietf.org/doc/draft-ietf-bess-evpn-geneve/
>>
>> [2]
>> https://mailarchive.ietf.org/arch/msg/bess/cG3X1tTqb_vPC4rg56SEdkjqDpw
>>
>>
>> _______________________________________________
>> BESS mailing list
>> [email protected]
>> https://www.ietf.org/mailman/listinfo/bess
>>
>
_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess

Reply via email to