> On Aug 25, 2015, at 1:03 PM, Bruce Davie <bda...@nicira.com> wrote: > > diff --git a/vtep/vtep.xml b/vtep/vtep.xml > index ff8d0fe..a554dcf 100644 > --- a/vtep/vtep.xml > +++ b/vtep/vtep.xml > @@ -367,7 +367,7 @@ > <group title="BFD Local Configuration"> > <p> > The HSC writes the key-value pairs in the > - <ref column="bfd_config_local"/> column to specifiy the local > + <ref column="bfd_config_local"/> column to specify the local
There are a few typo fixes in this patch. Normally, with a big change like this patch, I wouldn't roll those in. The main reason is that if the patch gets reverted at some point, these fixes would be lost, too. It's probably not worth re-spinning this patch, but thought I'd mention it. > + <column name="acl_bindings"> > + <p> > + Attach Access Control Lists (ACLs) to the physical port. The > + column consists of a map of VLAN tags to <ref table="ACL"/>s. If the > value of > + the VLAN tag in the map is 0, this means that the ACL is > + associated with the entire physical port. Non-zero values mean > + that the ACL is to be applied only on packets carrying that VLAN > + tag value. Switches will not necessarily support matching on the > + VLAN tag for all ACLs, and unsupported ACL bindings will cause > + errors to be reported. > + </p> This isn't related to this patch, but I didn't see what happens when "vlan_bindings" or "vlan_stats" uses a key of 0. > @@ -851,6 +870,15 @@ > One or more static routes, mapping IP prefixes to next hop IP addresses. > </column> > > + <column name="acl_binding"> > + Maps ACLs to logical router interfaces. The router interfaces > + are indicated using IP address notation, and must be the same > + interfaces created in the <ref column="switch_binding"/> > + column. For example, an ACL could be associated with the logical > + router interface with an address of 192.68.1.1 as defined in the > + example above. > + </column> The Physical_Port table has an "invalid_ACL_binding" error. Do you think it's worth adding one to the Logical_Router table, too? > + <table name="ACL_entry"> > + <p> > + Describes the individual entries that comprise an Access Control List. > + </p> > + <p> > + Each entry in the table is a single rule to match on certain > + header fields. While there are a large number of fields that can > + be matched on, most hardware cannot match on arbitrary > + combinations of fields. It is common to match on either L2 > + fields (described below in the L2 group of columns) or L3/L4 fields > + (the L3/L4 group of columns) but not both. The hardware switch > + controller may log an error if an ACL entry requires it to match > + on an incompatible mixture of fields. Do you think it's worth mentioning that all packets are allowed by default? > + <column name="sequence"> > + <p> > + The sequence number for the ACL entry for the purpose of > + ordering entries in an ACL. I think it would be helpful to indicate whether a larger number is more likely to hit or less likely. > + </column> > + <column name="ethertype"> > + <p> > + Ethertype in hexadecimal, in the form > + <var>0xAAA</var> This is very minor, but I think it may be clearer with a fourth "A". > + <column name="source_port_min"> > + <p> > + Lower end of the range of source port values. > + </p> > + </column> > + <column name="source_port_max"> > + <p> > + Upper end of the range of source port values. > + </p> > + </column> > + <column name="dest_port_min"> > + <p> > + Lower end of the range of destination port values. > + </p> > + </column> > + <column name="dest_port_max"> > + <p> > + Upper end of the range of destination port values. > + </p> For all of these, it would be good to indicate if the chosen value is included. (I assume it is.) > + <column name="tcp_flags"> > + <p> > + Integer representing the value of TCP flags to match. > + </p> > + </column> > + <column name="tcp_flags_mask"> > + <p> > + Integer representing the mask to apply when matching TCP flags. > + </p> It might be nice to include an example for "tcp_flags" and its mask. Otherwise, it looks good. Thanks, --Justin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
