Thanks a lot for adding more comments on mega flow implementation. Only a minor comment inline. Otherwise it looks good.
Acked-by: Andy Zhou <az...@nicira.com> On Tue, Nov 12, 2013 at 3:20 PM, Ben Pfaff <b...@nicira.com> wrote: > Signed-off-by: Ben Pfaff <b...@nicira.com> > --- > lib/dpif.c | 12 ++++++++---- > lib/dpif.h | 44 +++++++++++++++++++++++++++++++++++--------- > 2 files changed, 43 insertions(+), 13 deletions(-) > > diff --git a/lib/dpif.c b/lib/dpif.c > index 783a7cb..63a7ab6 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -883,15 +883,19 @@ dpif_flow_put__(struct dpif *dpif, const struct > dpif_flow_put *put) > > /* Adds or modifies a flow in 'dpif'. The flow is specified by the > Netlink > * attribute OVS_FLOW_ATTR_KEY with types OVS_KEY_ATTR_* in the 'key_len' > bytes > - * starting at 'key', and OVS_FLOW_ATTR_MASK with types of OVS_KEY_ATTR_* > in the > - * 'mask_len' bytes starting at 'mask'. The associated actions are > specified by > - * the Netlink attributes with types OVS_ACTION_ATTR_* in the > 'actions_len' > - * bytes starting at 'actions'. > + * starting at 'key', and OVS_FLOW_ATTR_MASK with types of OVS_KEY_ATTR_* > in > + * the 'mask_len' bytes starting at 'mask'. The associated actions are > + * specified by the Netlink attributes with types OVS_ACTION_ATTR_* in the > + * 'actions_len' bytes starting at 'actions'. > * > * - If the flow's key does not exist in 'dpif', then the flow will be > added if > * 'flags' includes DPIF_FP_CREATE. Otherwise the operation will fail > with > * ENOENT. > * > + * The datapath may reject attempts to insert overlapping flows with > EINVAL, > or EEXIST > + * but clients should not rely on this: avoiding overlapping flows is > + * primarily the client's responsibility. > + * > * If the operation succeeds, then 'stats', if nonnull, will be zeroed. > * > * - If the flow's key does exist in 'dpif', then the flow's actions will > be > diff --git a/lib/dpif.h b/lib/dpif.h > index 6db6ad3..499ee79 100644 > --- a/lib/dpif.h > +++ b/lib/dpif.h > @@ -103,12 +103,12 @@ > * Flow Table > * ========== > * > - * The flow table is a hash table of "flow entries". Each flow entry > contains: > + * The flow table is a collection of "flow entries". Each flow entry > contains: > * > * - A "flow", that is, a summary of the headers in an Ethernet > packet. The > - * flow is the hash key and thus must be unique within the flow > table. > - * Flows are fine-grained entities that include L2, L3, and L4 > headers. A > - * single TCP connection consists of two flows, one in each > direction. > + * flow must be unique within the flow table. Flows are fine-grained > + * entities that include L2, L3, and L4 headers. A single TCP > connection > + * consists of two flows, one in each direction. > * > * In Open vSwitch userspace, "struct flow" is the typical way to > describe > * a flow, but the datapath interface uses a different data format to > @@ -117,11 +117,37 @@ > * "struct ovs_key_*" in include/linux/openvswitch.h for details. > * lib/odp-util.h defines several functions for working with these > flows. > * > - * (In case you are familiar with OpenFlow, datapath flows are > analogous > - * to OpenFlow flow matches. The most important difference is that > - * OpenFlow allows fields to be wildcarded and prioritized, whereas a > - * datapath's flow table is a hash table so every flow must be > - * exact-match, thus without priorities.) > + * - A "mask" that, for each bit in the flow, specifies whether the > datapath > + * should consider the corresponding flow bit when deciding whether a > + * given packet matches the flow entry. The original datapath > design did > + * not support matching: every flow entry was exact match. With the > + * addition of a mask, the interface supports datapaths with a > spectrum of > + * wildcard matching capabilities, from those that only support exact > + * matches to those that support bitwise wildcarding on the entire > flow > + * key, as well as datapaths with capabilities somewhere in between. > + * > + * Datapaths do not provide a way to query their wildcarding > capabilities, > + * nor is it expected that the client should attempt to probe for the > + * details of their support. Instead, a client installs flows with > masks > + * that wildcard as many bits as acceptable. The datapath then > actually > + * wildcards as many of those bits as it can and changes the > wildcard bits > + * that it does not support into exact match bits. A datapath that > can > + * wildcard any bit, for example, would install the supplied mask, an > + * exact-match only datapath would install an exact-match mask > regardless > + * of what mask the client supplied, and a datapath in the middle of > the > + * spectrum would selectively change some wildcard bits into exact > match > + * bits. > + * > + * Regardless of the requested or installed mask, the datapath > retains the > + * original flow supplied by the client. (It does not, for example, > "zero > + * out" the wildcarded bits.) This allows the client to > unambiguously > + * identify the flow entry in later flow table operations. > + * > + * The flow table does not have priorities; that is, all flow > entries have > + * equal priority. Detecting overlapping flow entries is expensive > in > + * general, so the datapath is not required to do it. It is > primarily the > + * client's responsibility not to install flow entries whose flow > and mask > + * combinations overlap. > * > * - A list of "actions" that tell the datapath what to do with packets > * within a flow. Some examples of actions are > OVS_ACTION_ATTR_OUTPUT, > -- > 1.7.10.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
