Thanks Ben
On 3/18/16, 11:16 AM, "dev on behalf of Ben Pfaff" <dev-boun...@openvswitch.org on behalf of b...@ovn.org> wrote: >Hi Darrell. > >The commit message should begin with a brief summary, no more than about >75 columns wide, and that should be the entire first paragraph of the >commit message. When it's longer than that, the subject of the email >message becomes too wide. I’ll adjust Each patch should have had “RFC” in the commit message as well > >I think that the commit message needs to be more extensive, too. It >should explain the higher-level goal here. I’ll include more high level text in the commit message instead of just the cover letter > >The vocabulary here is somewhat confusing. It refers to "physical >endpoints" a lot. Are there logical endpoints as well? If not, is the >word "physical" helpful? Could you Agreed; its not not clear In the subsequent non-RFC patch series covering physical-logical separation, I used the following text "Physical endpoints are defined here as endpoints at the border of a physical network. This presently applies to localnet and gateways.” I’m consolidating 2 non-RFC patch series and will send out today/tomorrow > > >> + <column name="chassis_port"> >> + The port on the transport node the physical endpoint resides on. >> + Context is the associated transport node. >> + </column> > >I don't know what is supposed to go in chassis_port. I see that it's >optional--in what case is it not populated? Will specify as "real interface, such as eth0, used to connect to a physical provider network” ? > >> + <column name="type"> >> + The encapsulation type of the physical endpoint. Types include >> + single vlan for now, but later support for mpls label stack, >> + IP tunnel outer header and dual vlan. Note that encapsulations >> + are often directionally different, meaning the encapsulation >> + expected on ingress is different from the encapsulation >> + for egressing packets; mpls labels and IP tunnels being >> + two examples. >> + </column> >> + > >I don't think we should include "mpls_label_stack" as an option if we're >not going to fully define what it means. I had put it there as a hint how it could be used in future I’ll give an example for mpls using a single label that differentiates ingress and egress cases and see it helps; I'll just mention that it can be extended > >I don't understand what a type of "later" means. Thanks That should not be there > >> + <column name="ingress_encap"> >> + The ingress encapsulation type of the physical endpoint. >> + </column> >> + >> + <column name="egress_encap"> >> + The egress encapsulation type of the physical endpoint. >> + </column> > >The above documentation, and the schema itself, do not define the >possible ingress and egress encapsulation types, so I wouldn't have any >idea what's supposed to go here. I’ll be more clear the xml file and only include in the schema what is presently used in the code > >> @@ -1278,6 +1328,7 @@ tcp.flags = RST; >> </column> >> >> <column name="tag"> >> + This column will be deprecated. >> If set, indicates that the port represents a connection to a >> specific >> VLAN on a locally accessible network. The VLAN ID is used to match >> incoming traffic and is also added to outgoing traffic. > >It's confusing to say that a column "will be" deprecated. Is it >deprecated or not? What's the replacement? I won’t be adding any such comments in the xml file going forward I’ll remove the field from the xml file when it time to deprecate it > >> @@ -1286,15 +1337,18 @@ tcp.flags = RST; >> >> <group title="VTEP Options"> >> <p> >> + Will be deprecated. >> These options apply to logical ports with <ref column="type"/> of >> <code>vtep</code>. >> </p> >> >> <column name="options" key="vtep-physical-switch"> >> + This column "may" be deprecated. >> Required. The name of the VTEP gateway. >> </column> >> >> <column name="options" key="vtep-logical-switch"> >> + This column "may" be deprecated. >> Required. A logical switch name connected by the VTEP gateway. >> Must >> be set when <ref column="type"/> is <code>vtep</code>. >> </column> > >I'm even more confused when it says that a column "may" be deprecated. agreed >_______________________________________________ >dev mailing list >dev@openvswitch.org >http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
