Extensions to BGP-Signaled Pseudowires to Support Flow-Aware Transport Labels
draft-ietf-bess-fat-pw-bgp-04

I support Mirja’s question— how do I choose the flow label? Can I just use 17 for all?

Thanks for the work on this document. I have two very small editorial nits:

> Abstract
>
>  This draft defines protocol extensions required to synchronize flow
>  label states among PEs when using the BGP-based signaling procedures.

Please expand "PE".

§3:

>  A PE MAY support the configuration of the flow label (T and R bits)
>  on a per-service (e.g.  VPLS VFI) basis.

Please add a comma after "e.g."

There are a couple of instances of lower-case "should" that do not seem to be intented as 2119 keywords. Please consider using the boilerplate from RFC 8174 rather than 2119.

The security considerations could use more explanation. Please describe why the new elements and procedures in this draft do not add new security considerations.

Is there a race condition here when an endpoint changes its R bit? Like it will temporarily get packets in the wrong state, right? Problem?

I agree with Ben about the security considerations section. One or two more sentence should suffice.

I support Mirja's first comment as well.

I would be really happy if the document could say lightly more about how flow labels are supposed to be assigned. Is the assumption that all packets of an TCP flow get the same flow label assigned?

Just a comment, no action required: Given there are only 4 unsigned bits left and the registry policy is "IETF review", I actually don't think a registry is necessarily needed. The remaining bits could just be assigned by additional RFCs that update RFC4761.  

Some nits, minor comments:

1) sec 1: Please spell out "ASBR"

2) sec 3: "a PE sending a Control Flags field with T = 1 and
   receiving a Control Flags field with R = 1 MUST include a flow label
   in the Pseudowire packet."
   Must this be a MUST or could this be a SHOULD?

3) In sec 2 there is this text:
"the remaining bits, designated Z, MUST
   be set to zero when sending and MUST be ignored when receiving this
   Extended Community."
 In the IANA Consideration section there is this text:
"As per [RFC4761] and this document, the remaining bits are
   unassigned, and MUST be set to zero when sending and MUST be ignored
   when receiving the Layer2 Info Extended Community."

I think the text in section 2 is actually incorrect because it should not talked about the bits that are marked Z but rather about bits that are not assigned in the newly created registry because otherwise you'd need to update this RFC (and RFC4761) every time you assign a new bit.

Further I don't think this need to be mentioned in the IANA section and should only be correctly specified in section 2. Except you would like IANA to note this on the registration page but then you should say that...