This patch enables a user to set importance for a new rule via add-flow OF1.1+ in the OVS and display the same via dump-flows command OF1.1+ . The changes are made in accordance with OpenFlow 1.4 specs to implement Eviction on the basis of "importance".
Signed-off-by: Rishi Bamba <rishi.ba...@tcs.com> --- include/openflow/openflow-1.1.h | 5 +++-- lib/ofp-parse.c | 6 +++++- lib/ofp-print.c | 3 +++ lib/ofp-util.c | 5 +++++ lib/ofp-util.h | 3 +++ ofproto/ofproto-provider.h | 3 +++ ofproto/ofproto.c | 5 ++++- 7 files changed, 26 insertions(+), 4 deletions(-) diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h index f87c5cf..d881192 100644 --- a/include/openflow/openflow-1.1.h +++ b/include/openflow/openflow-1.1.h @@ -340,7 +340,7 @@ struct ofp11_flow_mod { output group. A value of OFPG11_ANY indicates no restriction. */ ovs_be16 flags; /* One of OFPFF_*. */ - uint8_t pad[2]; + ovs_be16 importance; /* Eviction Precedence */ /* Followed by an ofp11_match structure. */ /* Followed by an instruction set. */ }; @@ -451,7 +451,8 @@ struct ofp11_flow_stats { ovs_be16 idle_timeout; /* Number of seconds idle before expiration. */ ovs_be16 hard_timeout; /* Number of seconds before expiration. */ ovs_be16 flags; /* OF 1.3: Set of OFPFF*. */ - uint8_t pad2[4]; /* Align to 64-bits. */ + ovs_be16 importance; /* Eviction Precedence */ + uint8_t pad2[2]; /* Align to 64-bits. */ ovs_be64 cookie; /* Opaque controller-issued identifier. */ ovs_be64 packet_count; /* Number of packets in flow. */ ovs_be64 byte_count; /* Number of bytes in flow. */ diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index eed5a08..23626cb 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -248,6 +248,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, enum { F_OUT_PORT = 1 << 0, F_ACTIONS = 1 << 1, + F_IMPORTANCE=1 << 2, F_TIMEOUT = 1 << 3, F_PRIORITY = 1 << 4, F_FLAGS = 1 << 5, @@ -264,7 +265,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, break; case OFPFC_ADD: - fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY | F_FLAGS; + fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY | F_FLAGS | F_IMPORTANCE; break; case OFPFC_DELETE: @@ -305,6 +306,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, fm->buffer_id = UINT32_MAX; fm->out_port = OFPP_ANY; fm->flags = 0; + fm->importance = OFP_FLOW_PERMANENT; fm->out_group = OFPG11_ANY; fm->delete_reason = OFPRR_DELETE; if (fields & F_ACTIONS) { @@ -366,6 +368,8 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, error = str_to_u16(value, name, &fm->idle_timeout); } else if (fields & F_TIMEOUT && !strcmp(name, "hard_timeout")) { error = str_to_u16(value, name, &fm->hard_timeout); + } else if (fields & F_IMPORTANCE && !strcmp(name, "importance")) { + error = str_to_u16(value, name, &fm->importance); } else if (!strcmp(name, "cookie")) { char *mask = strchr(value, '/'); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 43bfa17..7f45858 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -1431,6 +1431,9 @@ ofp_print_flow_stats(struct ds *string, struct ofputil_flow_stats *fs) if (fs->flags) { ofp_print_flow_flags(string, fs->flags); } + if (fs->importance != OFP_FLOW_PERMANENT) { + ds_put_format(string, "importance=%"PRIu16", ", fs->importance); + } if (fs->idle_age >= 0) { ds_put_format(string, "idle_age=%d, ", fs->idle_age); } diff --git a/lib/ofp-util.c b/lib/ofp-util.c index c8d38e8..9c3f219 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -1700,6 +1700,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, fm->idle_timeout = ntohs(ofm->idle_timeout); fm->hard_timeout = ntohs(ofm->hard_timeout); + fm->importance = ntohs(ofm->importance); fm->buffer_id = ntohl(ofm->buffer_id); error = ofputil_port_from_ofp11(ofm->out_port, &fm->out_port); if (error) { @@ -1738,6 +1739,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, fm->new_cookie = ofm->cookie; fm->idle_timeout = ntohs(ofm->idle_timeout); fm->hard_timeout = ntohs(ofm->hard_timeout); + fm->importance = 0; fm->buffer_id = ntohl(ofm->buffer_id); fm->out_port = u16_to_ofp(ntohs(ofm->out_port)); fm->out_group = OFPG11_ANY; @@ -2248,6 +2250,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, ofm->out_port = ofputil_port_to_ofp11(fm->out_port); ofm->out_group = htonl(fm->out_group); ofm->flags = raw_flags; + ofm->importance = htons(fm->importance); ofputil_put_ofp11_match(msg, &fm->match, protocol); ofpacts_put_openflow_instructions(fm->ofpacts, fm->ofpacts_len, msg, version); @@ -2834,6 +2837,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, fs->duration_nsec = ntohl(ofs->duration_nsec); fs->idle_timeout = ntohs(ofs->idle_timeout); fs->hard_timeout = ntohs(ofs->hard_timeout); + fs->importance = ntohs(ofs->importance); if (raw == OFPRAW_OFPST13_FLOW_REPLY) { error = ofputil_decode_flow_mod_flags(ofs->flags, -1, oh->version, &fs->flags); @@ -2977,6 +2981,7 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs, ofs->priority = htons(fs->priority); ofs->idle_timeout = htons(fs->idle_timeout); ofs->hard_timeout = htons(fs->hard_timeout); + ofs->importance = htons(fs->importance); if (raw == OFPRAW_OFPST13_FLOW_REPLY) { ofs->flags = ofputil_encode_flow_mod_flags(fs->flags, version); } else { diff --git a/lib/ofp-util.h b/lib/ofp-util.h index af1a2a3..3173ab4 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -306,6 +306,7 @@ struct ofputil_flow_mod { ofp_port_t out_port; uint32_t out_group; enum ofputil_flow_mod_flags flags; + uint16_t importance; /* Eviction Precedence */ struct ofpact *ofpacts; /* Series of "struct ofpact"s. */ size_t ofpacts_len; /* Length of ofpacts, in bytes. */ @@ -355,6 +356,7 @@ struct ofputil_flow_stats { const struct ofpact *ofpacts; size_t ofpacts_len; enum ofputil_flow_mod_flags flags; + uint16_t importance; /* Eviction Precedence */ }; int ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *, @@ -391,6 +393,7 @@ struct ofputil_flow_removed { uint16_t hard_timeout; uint64_t packet_count; /* Packet count, UINT64_MAX if unknown. */ uint64_t byte_count; /* Byte count, UINT64_MAX if unknown. */ + uint16_t importance; /* Eviction Precedence */ }; enum ofperr ofputil_decode_flow_removed(struct ofputil_flow_removed *, diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 158f86e..106d4e6 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -346,6 +346,9 @@ struct rule { uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */ uint16_t idle_timeout OVS_GUARDED; /* In seconds from ->used. */ + /* "Importance" is the eviction precedence of a flow */ + uint16_t importance OVS_GUARDED; + /* Eviction groups (see comment on struct eviction_group for explanation) . * * 'eviction_group' is this rule's eviction group, or NULL if it is not in diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 5233a4d..f2835f7 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3827,6 +3827,7 @@ handle_flow_stats_request(struct ofconn *ofconn, fs.ofpacts_len = actions->ofpacts_len; fs.flags = flags; + fs.importance=rule->importance; ofputil_append_flow_stats_reply(&fs, &replies); } @@ -4237,6 +4238,7 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm, ovs_mutex_lock(&rule->mutex); rule->idle_timeout = fm->idle_timeout; rule->hard_timeout = fm->hard_timeout; + rule->importance = fm->importance; ovs_mutex_unlock(&rule->mutex); *CONST_CAST(uint8_t *, &rule->table_id) = table - ofproto->tables; @@ -4354,6 +4356,7 @@ modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm, if (fm->command == OFPFC_ADD) { rule->idle_timeout = fm->idle_timeout; rule->hard_timeout = fm->hard_timeout; + rule->importance = fm->importance; rule->flags = fm->flags & OFPUTIL_FF_STATE; rule->created = now; } @@ -4367,7 +4370,7 @@ modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm, cookies_insert(ofproto, rule); } if (fm->command == OFPFC_ADD) { - if (fm->idle_timeout || fm->hard_timeout) { + if (fm->idle_timeout || fm->hard_timeout || fm->importance) { if (!rule->eviction_group) { eviction_group_add_rule(rule); } -- 2.1.1 Thank You Regards Rishi ----- Original Message ----- From: "Ben Pfaff" <b...@nicira.com> To: "Rishi Bamba" <rishi.ba...@tcs.com> Cc: dev@openvswitch.org Sent: Monday, September 22, 2014 10:31:37 PM Subject: Re: [ovs-dev] Addition of importance parameter in flow_mod structure for implementation of eviction according to OF1.4 It would save a lot of time for both of us if you would just post your patches. On Mon, Sep 22, 2014 at 08:09:03PM +0530, Rishi Bamba wrote: > Hi Ben, > > In reference to the mail dated 15.09.2014 I would like to thank you for your > continued support to the team.Attached is the mail for reference. > > In progress to the same ,so far we have achieved the below mentioned.Wanted > to share the understanding and confirmation on the same. > > > Case 1: Flow Added with default active version on OpenvSwitch ( OF10 ) > alongwith importance > ( Eviction implementation in accordance with OF1.4 ). > > Add Flow Command(OF10) : ovs-ofctl add-flow br0 > priority=21,importance=21,action=normal > Output: No Error , rule saved but with default importance( i.e zero ) rather > than with the specified value by user. > > > Dump-Flow Command(OF 10) : ovs-ofctl dump-flows br0 > Output: cookie=0x0, duration=8.736s, table=0, n_packets=0, n_bytes=0, > idle_age=8, priority=21 actions=NORMAL > > Dump-Flow Command(OF 11+) : ovs-ofctl -O OpenFlow13 dump-flows br0 > Output: cookie=0x0, duration=116.980s, table=0, n_packets=6, n_bytes=546, > priority=21 actions=NORMAL > > The understanding is ,if the flow is added by the default active version i.e > OF10 , the added flow will have importance set as zero whether provided by > the user or not.The same is reflected in the dump flows either via OF10 or > OF11+ . If importance is zero it will not be visible in that case. > > Query 1: Is this handling correct or we need to throw a message to the user > that "importance" is supported by a particular version only rather than > making it zero. Also as we are using OF11 flow mod structure as mentioned in > previous mail, will importance parameter in add-flow should be supported on > 1.1+ or we have to make it OF1.4 specific only. > > > > Case 2: Flow Added with specific version on OpenvSwitch ( OF11+ ) alongwith > importance > (Eviction implementation in accordance with OF1.4 ). > > Add Flow Command(OF11+) : ovs-ofctl -O OpenFlow13 add-flow br0 > priority=22,importance=22,action=drop > Output: No Error , rule saved with the value provided by the user and else > zero if not provided by the user. > > > Dump-Flow Command(OF 10) : ovs-ofctl dump-flows br0 > Output: (IMPORTANCE NOT VISIBLE) > cookie=0x0, duration=47.841s, table=0, n_packets=2, n_bytes=182, idle_age=24, > priority=22 actions=drop > cookie=0x0, duration=243.481s, table=0, n_packets=6, n_bytes=546, > idle_age=152, priority=21 actions=NORMAL > > Dump-Flow Command(OF 11+) : ovs-ofctl -O OpenFlow13 dump-flows br0 > Output: (IMPORTANCE VISIBLE FOR THE FLOWS HAVING VALUE SET FOR THE FIELD) > cookie=0x0, duration=141.275s, table=0, n_packets=2, n_bytes=182, > importance=22, priority=22 actions=drop > cookie=0x0, duration=336.915s, table=0, n_packets=6, n_bytes=546, priority=21 > actions=NORMAL > > The understanding is,if the flow is added by a specific version (OF11+), the > added flow will have importance set as the value provided by the user else > zero.Now if the user dump-flows with the default version (OF10) none of the > flows importance parameter will be visible even for the set one's else if > dump-flows via OF11+ , the importance parameter will be visible for the flows > which have the parameter set of. > > Query 2: Do we need to show the importance field if a flow is added by OF11+ > and dump-flows via OF10. Currently we are not showing as mentioned above.Also > this should also be only 1.4 specific or 1.1+ is fine. > > > Note: On finalizing the the above mentioned approach we will be submitting > the patch for the same at the earliest. Need your confirmation on the above > working. > > > Thank You > Regards > Rishi > > =====-----=====-----===== > Notice: The information contained in this e-mail > message and/or attachments to it may contain > confidential or privileged information. If you are > not the intended recipient, any dissemination, use, > review, distribution, printing or copying of the > information contained in this e-mail message > and/or attachments to it are strictly prohibited. If > you have received this communication in error, > please notify us by reply e-mail or telephone and > immediately and permanently delete the message > and any attachments. Thank you > > > Date: Sun, 14 Sep 2014 13:50:23 -0700 > From: Ben Pfaff <b...@nicira.com> > To: Rishi Bamba <rishi.ba...@tcs.com> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] Addition of importance parameter in flow_mod > structure for implementation of eviction according to OF1.4 > User-Agent: Mutt/1.5.21 (2010-09-15) > > On Fri, Sep 12, 2014 at 05:55:18PM +0530, Rishi Bamba wrote: > > 1. Currently we have modified ofp11_flow_mod in openflow-1.1.h for > > addition of "ovs_be16 importance" parameter. > > > > Query: Do we need to create ofp14_flow_mod as the structure is > > currently not present in openflow-1.4.h or the enhancement to > > ofp11_flow_mod in openflow-1.1.h is justified. > > It's OK to update ofp11_flow_mod because the new member just assigns a > meaning to a field previously used for padding. > > > 2. We noticed that when using "add-flow" command with argument "-O > > OpenFlow14" a error message is thrown > > You should enable OF1.4. See the FAQ. > > Q: What versions of OpenFlow does Open vSwitch support? > > A: The following table lists the versions of OpenFlow supported by > each version of Open vSwitch: > > Open vSwitch OF1.0 OF1.1 OF1.2 OF1.3 OF1.4 OF1.5 > =============== ===== ===== ===== ===== ===== ===== > 1.9 and earlier yes --- --- --- --- --- > 1.10 yes --- [*] [*] --- --- > 1.11 yes --- [*] [*] --- --- > 2.0 yes [*] [*] [*] --- --- > 2.1 yes [*] [*] [*] --- --- > 2.2 yes [*] [*] [*] [%] [*] > 2.3 yes yes yes yes [*] [*] > > [*] Supported, with one or more missing features. > [%] Experimental, unsafe implementation. > > Open vSwitch 2.3 enables OpenFlow 1.0, 1.1, 1.2, and 1.3 by default > in ovs-vswitchd. In Open vSwitch 1.10 through 2.2, OpenFlow 1.1, > 1.2, and 1.3 must be enabled manually in ovs-vswitchd. OpenFlow > 1.4 and 1.5 are also supported, with missing features, in Open > vSwitch 2.3 and later, but not enabled by default. In any case, > the user may override the default: > > - To enable OpenFlow 1.0, 1.1, 1.2, and 1.3 on bridge br0: > > ovs-vsctl set bridge br0 > protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13 > > - To enable OpenFlow 1.0, 1.1, 1.2, 1.3, 1.4, and 1.5 on bridge br0: > > ovs-vsctl set bridge br0 > protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 > > - To enable only OpenFlow 1.0 on bridge br0: > > ovs-vsctl set bridge br0 protocols=OpenFlow10 > > All current versions of ovs-ofctl enable only OpenFlow 1.0 by > default. Use the -O option to enable support for later versions of > OpenFlow in ovs-ofctl. For example: > > ovs-ofctl -O OpenFlow13 dump-flows br0 > > (Open vSwitch 2.2 had an experimental implementation of OpenFlow > 1.4 that could cause crashes. We don't recommend enabling it.) > > OPENFLOW-1.1+ in the Open vSwitch source tree tracks support for > OpenFlow 1.1 and later features. When support for OpenFlow 1.4 and > 1.5 is solidly implemented, Open vSwitch will enable those version > by default. Also, the OpenFlow 1.5 specification is still under > development and thus subject to change. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev