From: Giuseppe Lettieri Date: Sun, 15 Sep 2013 07:54:08 +0000 (+0200) Subject: Merge branch 'mainstream' X-Git-Tag: sliver-openvswitch-2.0.90-1~14 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=57e8d2bdc174cb24e2ae474ea805c7b7308d68a5;hp=03f976b173e2e670eb2fa77eae8eacd2abf4bf84;p=sliver-openvswitch.git Merge branch 'mainstream' --- diff --git a/AUTHORS b/AUTHORS index 7642aee18..63c1ef872 100644 --- a/AUTHORS +++ b/AUTHORS @@ -215,6 +215,7 @@ Stephen Hemminger shemminger@vyatta.com Takayuki HAMA t-hama@cb.jp.nec.com Teemu Koponen koponen@nicira.com Timothy Chen tchen@nicira.com +Torbjorn Tornkvist kruskakli@gmail.com Valentin Bud valentin@hackaserver.com Vishal Swarankar vishal.swarnkar@gmail.com Vjekoslav Brajkovic balkan@cs.washington.edu @@ -223,6 +224,7 @@ YAMAMOTO Takashi yamamoto@valinux.co.jp Yeming Zhao zhaoyeming@gmail.com Ying Chen yingchen@vmware.com Yongqiang Liu liuyq7809@gmail.com +ankur dwivedi ankurengg2003@gmail.com kk yap yapkke@stanford.edu likunyun kunyunli@hotmail.com rahim entezari rahim.entezari@gmail.com diff --git a/FAQ b/FAQ index 20a3e3b45..5744d5abf 100644 --- a/FAQ +++ b/FAQ @@ -148,7 +148,7 @@ A: The following table lists the Linux kernel versions against which the 1.9.x 2.6.18 to 3.8 1.10.x 2.6.18 to 3.8 1.11.x 2.6.18 to 3.8 - 2.x 2.6.32 to 3.10 + 2.0.x 2.6.32 to 3.10 Open vSwitch userspace should also work with the Linux kernel module built into Linux 3.3 and later. diff --git a/NEWS b/NEWS index 09c98ebb7..4dd956809 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ Post-v2.0.0 --------------------- + - Log files now report times with millisecond resolution. (Previous + versions only reported whole seconds.) v2.0.0 - xx xxx xxxx diff --git a/acinclude.m4 b/acinclude.m4 index 071fe54b4..c293d337b 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -240,8 +240,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ # quoting rules. OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [[[^@]]proto_data_valid], [OVS_DEFINE([HAVE_PROTO_DATA_VALID])]) - OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [raw], - [OVS_DEFINE([HAVE_MAC_RAW])]) OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_dst(], [OVS_DEFINE([HAVE_SKB_DST_ACCESSOR_FUNCS])]) OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], diff --git a/datapath/linux/compat/include/linux/skbuff.h b/datapath/linux/compat/include/linux/skbuff.h index a48609641..9868a9864 100644 --- a/datapath/linux/compat/include/linux/skbuff.h +++ b/datapath/linux/compat/include/linux/skbuff.h @@ -95,12 +95,6 @@ static inline struct rtable *skb_rtable(const struct sk_buff *skb) #define CHECKSUM_COMPLETE CHECKSUM_HW #endif -#ifdef HAVE_MAC_RAW -#define mac_header mac.raw -#define network_header nh.raw -#define transport_header h.raw -#endif - #ifndef HAVE_SKBUFF_HEADER_HELPERS static inline unsigned char *skb_transport_header(const struct sk_buff *skb) { diff --git a/include/openflow/openflow-1.2.h b/include/openflow/openflow-1.2.h index d1e42a4db..541b1439f 100644 --- a/include/openflow/openflow-1.2.h +++ b/include/openflow/openflow-1.2.h @@ -109,18 +109,16 @@ enum oxm12_ofb_match_fields { OFPXMT12_OFB_IPV6_ND_TLL, /* Target link-layer for ND. */ OFPXMT12_OFB_MPLS_LABEL, /* MPLS label. */ OFPXMT12_OFB_MPLS_TC, /* MPLS TC. */ - /* Following added in OpenFlow 1.3 */ - OFPXMT12_OFB_MPLS_BOS, /* MPLS BoS bit. */ - OFPXMT12_OFB_PBB_ISID, /* PBB I-SID. */ - OFPXMT12_OFB_TUNNEL_ID, /* Logical Port Metadata */ - OFPXMT12_OFB_IPV6_EXTHDR, /* IPv6 Extension Header pseudo-field */ +#define OFPXMT12_MASK ((1ULL << (OFPXMT12_OFB_MPLS_TC + 1)) - 1) - /* End Marker */ - OFPXMT12_OFB_MAX, + /* Following added in OpenFlow 1.3 */ + OFPXMT13_OFB_MPLS_BOS, /* MPLS BoS bit. */ + OFPXMT13_OFB_PBB_ISID, /* PBB I-SID. */ + OFPXMT13_OFB_TUNNEL_ID, /* Logical Port Metadata */ + OFPXMT13_OFB_IPV6_EXTHDR, /* IPv6 Extension Header pseudo-field */ +#define OFPXMT13_MASK ((1ULL << (OFPXMT13_OFB_IPV6_EXTHDR + 1)) - 1) }; -#define OFPXMT12_MASK ((1ULL << OFPXMT12_OFB_MAX) - 1) - /* OXM implementation makes use of NXM as they are the same format * with different field definitions */ @@ -180,13 +178,13 @@ enum oxm12_ofb_match_fields { #define OXM_OF_IPV6_ND_TLL OXM_HEADER (OFPXMT12_OFB_IPV6_ND_TLL, 6) #define OXM_OF_MPLS_LABEL OXM_HEADER (OFPXMT12_OFB_MPLS_LABEL, 4) #define OXM_OF_MPLS_TC OXM_HEADER (OFPXMT12_OFB_MPLS_TC, 1) -#define OXM_OF_MPLS_BOS OXM_HEADER (OFPXMT12_OFB_MPLS_BOS, 1) +#define OXM_OF_MPLS_BOS OXM_HEADER (OFPXMT13_OFB_MPLS_BOS, 1) #define OXM_OF_PBB_ISID OXM_HEADER (OFPXMT12_OFB_PBB_ISID, 4) #define OXM_OF_PBB_ISID_W OXM_HEADER_W (OFPXMT12_OFB_PBB_ISID, 4) -#define OXM_OF_TUNNEL_ID OXM_HEADER (OFPXMT12_OFB_TUNNEL_ID, 8) -#define OXM_OF_TUNNEL_ID_W OXM_HEADER_W (OFPXMT12_OFB_TUNNEL_ID, 8) -#define OXM_OF_IPV6_EXTHDR OXM_HEADER (OFPXMT12_OFB_IPV6_EXTHDR, 2) -#define OXM_OF_IPV6_EXTHDR_W OXM_HEADER_W (OFPXMT12_OFB_IPV6_EXTHDR, 2) +#define OXM_OF_TUNNEL_ID OXM_HEADER (OFPXMT13_OFB_TUNNEL_ID, 8) +#define OXM_OF_TUNNEL_ID_W OXM_HEADER_W (OFPXMT13_OFB_TUNNEL_ID, 8) +#define OXM_OF_IPV6_EXTHDR OXM_HEADER (OFPXMT13_OFB_IPV6_EXTHDR, 2) +#define OXM_OF_IPV6_EXTHDR_W OXM_HEADER_W (OFPXMT13_OFB_IPV6_EXTHDR, 2) /* The VLAN id is 12-bits, so we can use the entire 16 bits to indicate * special conditions. diff --git a/lib/automake.mk b/lib/automake.mk index fce8423ab..b2d6dc24a 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -58,6 +58,8 @@ lib_libopenvswitch_a_SOURCES = \ lib/fatal-signal.h \ lib/flow.c \ lib/flow.h \ + lib/guarded-list.c \ + lib/guarded-list.h \ lib/hash.c \ lib/hash.h \ lib/hindex.c \ diff --git a/lib/classifier.c b/lib/classifier.c index 93ee977bc..36eb1f0bf 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -500,8 +500,9 @@ cls_cursor_first(struct cls_cursor *cursor) /* Returns the next matching cls_rule in 'cursor''s iteration, or a null * pointer if there are no more matches. */ struct cls_rule * -cls_cursor_next(struct cls_cursor *cursor, struct cls_rule *rule) +cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_) { + struct cls_rule *rule = CONST_CAST(struct cls_rule *, rule_); const struct cls_table *table; struct cls_rule *next; diff --git a/lib/classifier.h b/lib/classifier.h index 5a454586e..a795b4a18 100644 --- a/lib/classifier.h +++ b/lib/classifier.h @@ -46,12 +46,15 @@ extern "C" { #endif +/* Needed only for the lock annotation in struct classifier. */ +extern struct ovs_mutex ofproto_mutex; + /* A flow classifier. */ struct classifier { int n_rules; /* Total number of rules. */ struct hmap tables; /* Contains "struct cls_table"s. */ struct list tables_priority; /* Tables in descending priority order */ - struct ovs_rwlock rwlock; + struct ovs_rwlock rwlock OVS_ACQ_AFTER(ofproto_mutex); }; /* A set of rules that all have the same fields wildcarded. */ @@ -140,7 +143,7 @@ struct cls_cursor { void cls_cursor_init(struct cls_cursor *cursor, const struct classifier *cls, const struct cls_rule *match) OVS_REQ_RDLOCK(cls->rwlock); struct cls_rule *cls_cursor_first(struct cls_cursor *cursor); -struct cls_rule *cls_cursor_next(struct cls_cursor *cursor, struct cls_rule *); +struct cls_rule *cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *); #define CLS_CURSOR_FOR_EACH(RULE, MEMBER, CURSOR) \ for (ASSIGN_CONTAINER(RULE, cls_cursor_first(CURSOR), MEMBER); \ diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index e2300d6c2..6f75f57e8 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -706,7 +706,7 @@ dpif_linux_port_dump_start(const struct dpif *dpif_, void **statep) *statep = state = xmalloc(sizeof *state); dpif_linux_vport_init(&request); - request.cmd = OVS_DP_CMD_GET; + request.cmd = OVS_VPORT_CMD_GET; request.dp_ifindex = dpif->dp_ifindex; buf = ofpbuf_new(1024); @@ -963,7 +963,7 @@ dpif_linux_flow_dump_start(const struct dpif *dpif_, void **statep) *statep = state = xmalloc(sizeof *state); dpif_linux_flow_init(&request); - request.cmd = OVS_DP_CMD_GET; + request.cmd = OVS_FLOW_CMD_GET; request.dp_ifindex = dpif->dp_ifindex; buf = ofpbuf_new(1024); diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c index 9b3e7ba3b..5137d9f64 100644 --- a/lib/dynamic-string.c +++ b/lib/dynamic-string.c @@ -183,21 +183,24 @@ ds_put_printable(struct ds *ds, const char *s, size_t n) } } -/* Writes time 'when' to 'string' based on 'template', in local time or UTC - * based on 'utc'. */ +/* Writes the current time with optional millisecond resolution to 'string' + * based on 'template'. + * The current time is either localtime or UTC based on 'utc'. */ void -ds_put_strftime(struct ds *ds, const char *template, time_t when, bool utc) +ds_put_strftime_msec(struct ds *ds, const char *template, long long int when, + bool utc) { - struct tm tm; + struct tm_msec tm; if (utc) { - gmtime_r(&when, &tm); + gmtime_msec(when, &tm); } else { - localtime_r(&when, &tm); + localtime_msec(when, &tm); } for (;;) { size_t avail = ds->string ? ds->allocated - ds->length + 1 : 0; - size_t used = strftime(&ds->string[ds->length], avail, template, &tm); + size_t used = strftime_msec(&ds->string[ds->length], avail, template, + &tm); if (used) { ds->length += used; return; @@ -209,12 +212,12 @@ ds_put_strftime(struct ds *ds, const char *template, time_t when, bool utc) /* Returns a malloc()'d string for time 'when' based on 'template', in local * time or UTC based on 'utc'. */ char * -xastrftime(const char *template, time_t when, bool utc) +xastrftime_msec(const char *template, long long int when, bool utc) { struct ds s; ds_init(&s); - ds_put_strftime(&s, template, when, utc); + ds_put_strftime_msec(&s, template, when, utc); return s.string; } diff --git a/lib/dynamic-string.h b/lib/dynamic-string.h index c069586e9..227234379 100644 --- a/lib/dynamic-string.h +++ b/lib/dynamic-string.h @@ -61,10 +61,9 @@ int ds_get_line(struct ds *, FILE *); int ds_get_preprocessed_line(struct ds *, FILE *, int *line_number); int ds_get_test_line(struct ds *, FILE *); -void ds_put_strftime(struct ds *, const char *template, time_t when, bool utc) - STRFTIME_FORMAT(2); -char *xastrftime(const char *template, time_t when, bool utc) - STRFTIME_FORMAT(1); +void ds_put_strftime_msec(struct ds *, const char *template, long long int when, + bool utc); +char *xastrftime_msec(const char *template, long long int when, bool utc); char *ds_cstr(struct ds *); const char *ds_cstr_ro(const struct ds *); diff --git a/lib/guarded-list.c b/lib/guarded-list.c new file mode 100644 index 000000000..cbb203021 --- /dev/null +++ b/lib/guarded-list.c @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2013 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "guarded-list.h" + +void +guarded_list_init(struct guarded_list *list) +{ + ovs_mutex_init(&list->mutex); + list_init(&list->list); + list->n = 0; +} + +void +guarded_list_destroy(struct guarded_list *list) +{ + ovs_mutex_destroy(&list->mutex); +} + +bool +guarded_list_is_empty(const struct guarded_list *list) +{ + bool empty; + + ovs_mutex_lock(&list->mutex); + empty = list->n == 0; + ovs_mutex_unlock(&list->mutex); + + return empty; +} + +/* If 'list' has fewer than 'max' elements, adds 'node' at the end of the list + * and returns the number of elements now on the list. + * + * If 'list' already has at least 'max' elements, returns 0 without modifying + * the list. */ +size_t +guarded_list_push_back(struct guarded_list *list, + struct list *node, size_t max) +{ + size_t retval = 0; + + ovs_mutex_lock(&list->mutex); + if (list->n < max) { + list_push_back(&list->list, node); + retval = ++list->n; + } + ovs_mutex_unlock(&list->mutex); + + return retval; +} + +struct list * +guarded_list_pop_front(struct guarded_list *list) +{ + struct list *node = NULL; + + ovs_mutex_lock(&list->mutex); + if (list->n) { + node = list_pop_front(&list->list); + list->n--; + } + ovs_mutex_unlock(&list->mutex); + + return node; +} + +size_t +guarded_list_pop_all(struct guarded_list *list, struct list *elements) +{ + size_t n; + + ovs_mutex_lock(&list->mutex); + list_move(elements, &list->list); + n = list->n; + + list_init(&list->list); + list->n = 0; + ovs_mutex_unlock(&list->mutex); + + return n; +} diff --git a/lib/guarded-list.h b/lib/guarded-list.h new file mode 100644 index 000000000..625865d42 --- /dev/null +++ b/lib/guarded-list.h @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2013 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef GUARDED_LIST_H +#define GUARDED_LIST_H 1 + +#include +#include "compiler.h" +#include "list.h" +#include "ovs-thread.h" + +struct guarded_list { + struct ovs_mutex mutex; + struct list list; + size_t n; +}; + +void guarded_list_init(struct guarded_list *); +void guarded_list_destroy(struct guarded_list *); + +bool guarded_list_is_empty(const struct guarded_list *); + +size_t guarded_list_push_back(struct guarded_list *, struct list *, + size_t max); +struct list *guarded_list_pop_front(struct guarded_list *); +size_t guarded_list_pop_all(struct guarded_list *, struct list *); + +#endif /* guarded-list.h */ diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 77aa69cec..54df17ffe 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -2117,6 +2117,30 @@ ofpacts_equal(const struct ofpact *a, size_t a_len, { return a_len == b_len && !memcmp(a, b, a_len); } + +/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of + * 'ofpacts'. If found, returns its meter ID; if not, returns 0. + * + * This function relies on the order of 'ofpacts' being correct (as checked by + * ofpacts_verify()). */ +uint32_t +ofpacts_get_meter(const struct ofpact ofpacts[], size_t ofpacts_len) +{ + const struct ofpact *a; + + OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { + enum ovs_instruction_type inst; + + inst = ovs_instruction_type_from_ofpact_type(a->type); + if (a->type == OFPACT_METER) { + return ofpact_get_METER(a)->meter_id; + } else if (inst > OVSINST_OFPIT13_METER) { + break; + } + } + + return 0; +} /* Formatting ofpacts. */ diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h index a3fb60f27..0876ed702 100644 --- a/lib/ofp-actions.h +++ b/lib/ofp-actions.h @@ -530,6 +530,7 @@ bool ofpacts_output_to_group(const struct ofpact[], size_t ofpacts_len, uint32_t group_id); bool ofpacts_equal(const struct ofpact a[], size_t a_len, const struct ofpact b[], size_t b_len); +uint32_t ofpacts_get_meter(const struct ofpact[], size_t ofpacts_len); /* Formatting ofpacts. * diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index a61beb99e..e28bf0238 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -1370,7 +1370,7 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string, char *name; /* Meters require at least OF 1.3. */ - *usable_protocols &= OFPUTIL_P_OF13_UP; + *usable_protocols = OFPUTIL_P_OF13_UP; switch (command) { case -1: diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 23c7136bc..9edfe9ef0 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -4010,6 +4010,19 @@ ofputil_put_ofp11_table_stats(const struct ofp12_table_stats *in, out->matched_count = in->matched_count; } +static void +ofputil_put_ofp12_table_stats(const struct ofp12_table_stats *in, + struct ofpbuf *buf) +{ + struct ofp12_table_stats *out = ofpbuf_put(buf, in, sizeof *in); + + /* Trim off OF1.3-only capabilities. */ + out->match &= htonll(OFPXMT12_MASK); + out->wildcards &= htonll(OFPXMT12_MASK); + out->write_setfields &= htonll(OFPXMT12_MASK); + out->apply_setfields &= htonll(OFPXMT12_MASK); +} + static void ofputil_put_ofp13_table_stats(const struct ofp12_table_stats *in, struct ofpbuf *buf) @@ -4035,31 +4048,27 @@ ofputil_encode_table_stats_reply(const struct ofp12_table_stats stats[], int n, reply = ofpraw_alloc_stats_reply(request, n * sizeof *stats); - switch ((enum ofp_version) request->version) { - case OFP10_VERSION: - for (i = 0; i < n; i++) { + for (i = 0; i < n; i++) { + switch ((enum ofp_version) request->version) { + case OFP10_VERSION: ofputil_put_ofp10_table_stats(&stats[i], reply); - } - break; + break; - case OFP11_VERSION: - for (i = 0; i < n; i++) { + case OFP11_VERSION: ofputil_put_ofp11_table_stats(&stats[i], reply); - } - break; + break; - case OFP12_VERSION: - ofpbuf_put(reply, stats, n * sizeof *stats); - break; + case OFP12_VERSION: + ofputil_put_ofp12_table_stats(&stats[i], reply); + break; - case OFP13_VERSION: - for (i = 0; i < n; i++) { + case OFP13_VERSION: ofputil_put_ofp13_table_stats(&stats[i], reply); - } - break; + break; - default: - NOT_REACHED(); + default: + NOT_REACHED(); + } } return reply; diff --git a/lib/table.c b/lib/table.c index 21f490504..46281703f 100644 --- a/lib/table.c +++ b/lib/table.c @@ -221,7 +221,7 @@ table_print_table_line__(struct ds *line) static char * table_format_timestamp__(void) { - return xastrftime("%Y-%m-%d %H:%M:%S", time_wall(), true); + return xastrftime_msec("%Y-%m-%d %H:%M:%S.###", time_wall_msec(), true); } static void diff --git a/lib/timeval.c b/lib/timeval.c index 326239744..223ed3084 100644 --- a/lib/timeval.c +++ b/lib/timeval.c @@ -42,12 +42,12 @@ VLOG_DEFINE_THIS_MODULE(timeval); struct clock { clockid_t id; /* CLOCK_MONOTONIC or CLOCK_REALTIME. */ - /* Features for use by unit tests. Protected by 'rwlock'. */ - struct ovs_rwlock rwlock; - struct timespec warp; /* Offset added for unit tests. */ - bool stopped; /* Disables real-time updates if true. */ - - struct timespec cache; /* Last time read from kernel. */ + /* Features for use by unit tests. Protected by 'mutex'. */ + struct ovs_mutex mutex; + atomic_bool slow_path; /* True if warped or stopped. */ + struct timespec warp OVS_GUARDED; /* Offset added for unit tests. */ + bool stopped OVS_GUARDED; /* Disable real-time updates if true. */ + struct timespec cache OVS_GUARDED; /* Last time read from kernel. */ }; /* Our clocks. */ @@ -76,7 +76,8 @@ init_clock(struct clock *c, clockid_t id) { memset(c, 0, sizeof *c); c->id = id; - ovs_rwlock_init(&c->rwlock); + ovs_mutex_init(&c->mutex); + atomic_init(&c->slow_path, false); xclock_gettime(c->id, &c->cache); } @@ -105,14 +106,28 @@ time_init(void) static void time_timespec__(struct clock *c, struct timespec *ts) { + bool slow_path; + time_init(); - if (!c->stopped) { + atomic_read_explicit(&c->slow_path, &slow_path, memory_order_relaxed); + if (!slow_path) { xclock_gettime(c->id, ts); } else { - ovs_rwlock_rdlock(&c->rwlock); - timespec_add(ts, &c->cache, &c->warp); - ovs_rwlock_unlock(&c->rwlock); + struct timespec warp; + struct timespec cache; + bool stopped; + + ovs_mutex_lock(&c->mutex); + stopped = c->stopped; + warp = c->warp; + cache = c->cache; + ovs_mutex_unlock(&c->mutex); + + if (!stopped) { + xclock_gettime(c->id, &cache); + } + timespec_add(ts, &cache, &warp); } } @@ -320,14 +335,24 @@ timespec_add(struct timespec *sum, *sum = tmp; } +static bool +is_warped(const struct clock *c) +{ + bool warped; + + ovs_mutex_lock(&c->mutex); + warped = monotonic_clock.warp.tv_sec || monotonic_clock.warp.tv_nsec; + ovs_mutex_unlock(&c->mutex); + + return warped; +} + static void log_poll_interval(long long int last_wakeup) { long long int interval = time_msec() - last_wakeup; - if (interval >= 1000 - && !monotonic_clock.warp.tv_sec - && !monotonic_clock.warp.tv_nsec) { + if (interval >= 1000 && !is_warped(&monotonic_clock)) { const struct rusage *last_rusage = get_recent_rusage(); struct rusage rusage; @@ -451,10 +476,11 @@ timeval_stop_cb(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) { - ovs_rwlock_wrlock(&monotonic_clock.rwlock); + ovs_mutex_lock(&monotonic_clock.mutex); + atomic_store(&monotonic_clock.slow_path, true); monotonic_clock.stopped = true; xclock_gettime(monotonic_clock.id, &monotonic_clock.cache); - ovs_rwlock_unlock(&monotonic_clock.rwlock); + ovs_mutex_unlock(&monotonic_clock.mutex); unixctl_command_reply(conn, NULL); } @@ -480,9 +506,10 @@ timeval_warp_cb(struct unixctl_conn *conn, ts.tv_sec = msecs / 1000; ts.tv_nsec = (msecs % 1000) * 1000 * 1000; - ovs_rwlock_wrlock(&monotonic_clock.rwlock); + ovs_mutex_lock(&monotonic_clock.mutex); + atomic_store(&monotonic_clock.slow_path, true); timespec_add(&monotonic_clock.warp, &monotonic_clock.warp, &ts); - ovs_rwlock_unlock(&monotonic_clock.rwlock); + ovs_mutex_unlock(&monotonic_clock.mutex); unixctl_command_reply(conn, "warped"); } @@ -494,3 +521,49 @@ timeval_dummy_register(void) unixctl_command_register("time/warp", "MSECS", 1, 1, timeval_warp_cb, NULL); } + + + +/* strftime() with an extension for high-resolution timestamps. Any '#'s in + * 'format' will be replaced by subseconds, e.g. use "%S.###" to obtain results + * like "01.123". */ +size_t +strftime_msec(char *s, size_t max, const char *format, + const struct tm_msec *tm) +{ + size_t n; + + n = strftime(s, max, format, &tm->tm); + if (n) { + char decimals[4]; + char *p; + + sprintf(decimals, "%03d", tm->msec); + for (p = strchr(s, '#'); p; p = strchr(p, '#')) { + char *d = decimals; + while (*p == '#') { + *p++ = *d ? *d++ : '0'; + } + } + } + + return n; +} + +struct tm_msec * +localtime_msec(long long int now, struct tm_msec *result) +{ + time_t now_sec = now / 1000; + localtime_r(&now_sec, &result->tm); + result->msec = now % 1000; + return result; +} + +struct tm_msec * +gmtime_msec(long long int now, struct tm_msec *result) +{ + time_t now_sec = now / 1000; + gmtime_r(&now_sec, &result->tm); + result->msec = now % 1000; + return result; +} diff --git a/lib/timeval.h b/lib/timeval.h index d0962eeb9..99b3af04c 100644 --- a/lib/timeval.h +++ b/lib/timeval.h @@ -40,6 +40,11 @@ BUILD_ASSERT_DECL(TYPE_IS_SIGNED(time_t)); #define TIME_MAX TYPE_MAXIMUM(time_t) #define TIME_MIN TYPE_MINIMUM(time_t) +struct tm_msec { + struct tm tm; + int msec; +}; + time_t time_now(void); time_t time_wall(void); long long int time_msec(void); @@ -53,6 +58,10 @@ int time_poll(struct pollfd *, int n_pollfds, long long int timeout_when, long long int timespec_to_msec(const struct timespec *); long long int timeval_to_msec(const struct timeval *); +struct tm_msec *localtime_msec(long long int now, struct tm_msec *result); +struct tm_msec *gmtime_msec(long long int now, struct tm_msec *result); +size_t strftime_msec(char *s, size_t max, const char *format, + const struct tm_msec *); void xgettimeofday(struct timeval *); void xclock_gettime(clock_t, struct timespec *); diff --git a/lib/vlog.c b/lib/vlog.c index a2671121f..37806b8db 100644 --- a/lib/vlog.c +++ b/lib/vlog.c @@ -583,7 +583,7 @@ static void vlog_init__(void) { static char *program_name_copy; - time_t now; + long long int now; /* openlog() is allowed to keep the pointer passed in, without making a * copy. The daemonize code sometimes frees and replaces 'program_name', @@ -593,10 +593,10 @@ vlog_init__(void) program_name_copy = program_name ? xstrdup(program_name) : NULL; openlog(program_name_copy, LOG_NDELAY, LOG_DAEMON); - now = time_wall(); + now = time_wall_msec(); if (now < 0) { - char *s = xastrftime("%a, %d %b %Y %H:%M:%S", now, true); - VLOG_ERR("current time is negative: %s (%ld)", s, (long int) now); + char *s = xastrftime_msec("%a, %d %b %Y %H:%M:%S", now, true); + VLOG_ERR("current time is negative: %s (%lld)", s, now); free(s); } @@ -746,12 +746,12 @@ format_log_message(const struct vlog_module *module, enum vlog_level level, ds_put_cstr(s, vlog_get_module_name(module)); break; case 'd': - p = fetch_braces(p, "%Y-%m-%d %H:%M:%S", tmp, sizeof tmp); - ds_put_strftime(s, tmp, time_wall(), false); + p = fetch_braces(p, "%Y-%m-%d %H:%M:%S.###", tmp, sizeof tmp); + ds_put_strftime_msec(s, tmp, time_wall_msec(), false); break; case 'D': - p = fetch_braces(p, "%Y-%m-%d %H:%M:%S", tmp, sizeof tmp); - ds_put_strftime(s, tmp, time_wall(), true); + p = fetch_braces(p, "%Y-%m-%d %H:%M:%S.###", tmp, sizeof tmp); + ds_put_strftime_msec(s, tmp, time_wall_msec(), true); break; case 'm': /* Format user-supplied log message and trim trailing new-lines. */ diff --git a/lib/vlog.h b/lib/vlog.h index d2c6679e2..d7d63bf11 100644 --- a/lib/vlog.h +++ b/lib/vlog.h @@ -64,7 +64,7 @@ enum vlog_level vlog_get_level_val(const char *name); #define VLOG_FACILITIES \ VLOG_FACILITY(SYSLOG, "ovs|%05N|%c%T|%p|%m") \ VLOG_FACILITY(CONSOLE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m") \ - VLOG_FACILITY(FILE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m") + VLOG_FACILITY(FILE, "%D{%Y-%m-%dT%H:%M:%S.###Z}|%05N|%c%T|%p|%m") enum vlog_facility { #define VLOG_FACILITY(NAME, PATTERN) VLF_##NAME, VLOG_FACILITIES diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 2f315e608..4a370eba7 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -271,6 +271,7 @@ void connmgr_run(struct connmgr *mgr, bool (*handle_openflow)(struct ofconn *, const struct ofpbuf *ofp_msg)) + OVS_EXCLUDED(ofproto_mutex) { struct ofconn *ofconn, *next_ofconn; struct ofservice *ofservice; @@ -1667,6 +1668,7 @@ connmgr_has_in_band(struct connmgr *mgr) * In-band control has more sophisticated code that manages flows itself. */ void connmgr_flushed(struct connmgr *mgr) + OVS_EXCLUDED(ofproto_mutex) { if (mgr->fail_open) { fail_open_flushed(mgr->fail_open); @@ -1833,6 +1835,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule, enum nx_flow_update_event event, enum ofp_flow_removed_reason reason, const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid) + OVS_REQUIRES(ofproto_mutex) { enum nx_flow_monitor_flags update; struct ofconn *ofconn; @@ -1897,14 +1900,14 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule, fu.match = &match; fu.priority = rule->cr.priority; - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); fu.idle_timeout = rule->idle_timeout; fu.hard_timeout = rule->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); if (flags & NXFMF_ACTIONS) { - fu.ofpacts = rule->ofpacts; - fu.ofpacts_len = rule->ofpacts_len; + fu.ofpacts = rule->actions->ofpacts; + fu.ofpacts_len = rule->actions->ofpacts_len; } else { fu.ofpacts = NULL; fu.ofpacts_len = 0; @@ -1951,12 +1954,12 @@ ofmonitor_flush(struct connmgr *mgr) static void ofmonitor_resume(struct ofconn *ofconn) { + struct rule_collection rules; struct ofpbuf *resumed; struct ofmonitor *m; - struct list rules; struct list msgs; - list_init(&rules); + rule_collection_init(&rules); HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) { ofmonitor_collect_resume_rules(m, ofconn->monitor_paused, &rules); } diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index 72134b0a2..55d08a631 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -187,11 +187,15 @@ void ofmonitor_destroy(struct ofmonitor *); void ofmonitor_report(struct connmgr *, struct rule *, enum nx_flow_update_event, enum ofp_flow_removed_reason, - const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid); + const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid) + OVS_REQUIRES(ofproto_mutex); void ofmonitor_flush(struct connmgr *); + +struct rule_collection; void ofmonitor_collect_resume_rules(struct ofmonitor *, uint64_t seqno, - struct list *rules); -void ofmonitor_compose_refresh_updates(struct list *rules, struct list *msgs); + struct rule_collection *); +void ofmonitor_compose_refresh_updates(struct rule_collection *rules, + struct list *msgs); #endif /* connmgr.h */ diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c index 2c0a8f33d..4900c0507 100644 --- a/ofproto/fail-open.c +++ b/ofproto/fail-open.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -211,6 +211,7 @@ fail_open_wait(struct fail_open *fo) void fail_open_flushed(struct fail_open *fo) + OVS_EXCLUDED(ofproto_mutex) { int disconn_secs = connmgr_failure_duration(fo->connmgr); bool open = disconn_secs >= trigger_duration(fo); diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 54f441b06..bc1e88419 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -23,6 +23,7 @@ #include "dynamic-string.h" #include "dpif.h" #include "fail-open.h" +#include "guarded-list.h" #include "latch.h" #include "seq.h" #include "list.h" @@ -41,6 +42,7 @@ COVERAGE_DEFINE(upcall_queue_overflow); COVERAGE_DEFINE(drop_queue_overflow); COVERAGE_DEFINE(miss_queue_overflow); COVERAGE_DEFINE(fmb_queue_overflow); +COVERAGE_DEFINE(fmb_queue_revalidated); /* A thread that processes each upcall handed to it by the dispatcher thread, * forwards the upcall's packet, and then queues it to the main ofproto_dpif @@ -79,26 +81,15 @@ struct udpif { struct handler *handlers; /* Miss handlers. */ size_t n_handlers; - /* Atomic queue of unprocessed drop keys. */ - struct ovs_mutex drop_key_mutex; - struct list drop_keys OVS_GUARDED; - size_t n_drop_keys OVS_GUARDED; - - /* Atomic queue of special upcalls for ofproto-dpif to process. */ - struct ovs_mutex upcall_mutex; - struct list upcalls OVS_GUARDED; - size_t n_upcalls OVS_GUARDED; - - /* Atomic queue of flow_miss_batches. */ - struct ovs_mutex fmb_mutex; - struct list fmbs OVS_GUARDED; - size_t n_fmbs OVS_GUARDED; + /* Queues to pass up to ofproto-dpif. */ + struct guarded_list drop_keys; /* "struct drop key"s. */ + struct guarded_list upcalls; /* "struct upcall"s. */ + struct guarded_list fmbs; /* "struct flow_miss_batch"es. */ /* Number of times udpif_revalidate() has been called. */ atomic_uint reval_seq; struct seq *wait_seq; - uint64_t last_seq; struct latch exit_latch; /* Tells child threads to exit. */ }; @@ -121,13 +112,10 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif) udpif->secret = random_uint32(); udpif->wait_seq = seq_create(); latch_init(&udpif->exit_latch); - list_init(&udpif->drop_keys); - list_init(&udpif->upcalls); - list_init(&udpif->fmbs); + guarded_list_init(&udpif->drop_keys); + guarded_list_init(&udpif->upcalls); + guarded_list_init(&udpif->fmbs); atomic_init(&udpif->reval_seq, 0); - ovs_mutex_init(&udpif->drop_key_mutex); - ovs_mutex_init(&udpif->upcall_mutex); - ovs_mutex_init(&udpif->fmb_mutex); return udpif; } @@ -153,9 +141,9 @@ udpif_destroy(struct udpif *udpif) flow_miss_batch_destroy(fmb); } - ovs_mutex_destroy(&udpif->drop_key_mutex); - ovs_mutex_destroy(&udpif->upcall_mutex); - ovs_mutex_destroy(&udpif->fmb_mutex); + guarded_list_destroy(&udpif->drop_keys); + guarded_list_destroy(&udpif->upcalls); + guarded_list_destroy(&udpif->fmbs); latch_destroy(&udpif->exit_latch); seq_destroy(udpif->wait_seq); free(udpif); @@ -228,34 +216,17 @@ udpif_recv_set(struct udpif *udpif, size_t n_handlers, bool enable) } } -void -udpif_run(struct udpif *udpif) -{ - udpif->last_seq = seq_read(udpif->wait_seq); -} - void udpif_wait(struct udpif *udpif) { - ovs_mutex_lock(&udpif->drop_key_mutex); - if (udpif->n_drop_keys) { - poll_immediate_wake(); - } - ovs_mutex_unlock(&udpif->drop_key_mutex); - - ovs_mutex_lock(&udpif->upcall_mutex); - if (udpif->n_upcalls) { - poll_immediate_wake(); - } - ovs_mutex_unlock(&udpif->upcall_mutex); - - ovs_mutex_lock(&udpif->fmb_mutex); - if (udpif->n_fmbs) { + uint64_t seq = seq_read(udpif->wait_seq); + if (!guarded_list_is_empty(&udpif->drop_keys) || + !guarded_list_is_empty(&udpif->upcalls) || + !guarded_list_is_empty(&udpif->fmbs)) { poll_immediate_wake(); + } else { + seq_wait(udpif->wait_seq, seq); } - ovs_mutex_unlock(&udpif->fmb_mutex); - - seq_wait(udpif->wait_seq, udpif->last_seq); } /* Notifies 'udpif' that something changed which may render previous @@ -265,20 +236,21 @@ udpif_revalidate(struct udpif *udpif) { struct flow_miss_batch *fmb, *next_fmb; unsigned int junk; + struct list fmbs; /* Since we remove each miss on revalidation, their statistics won't be * accounted to the appropriate 'facet's in the upper layer. In most * cases, this is alright because we've already pushed the stats to the * relevant rules. However, NetFlow requires absolute packet counts on * 'facet's which could now be incorrect. */ - ovs_mutex_lock(&udpif->fmb_mutex); atomic_add(&udpif->reval_seq, 1, &junk); - LIST_FOR_EACH_SAFE (fmb, next_fmb, list_node, &udpif->fmbs) { + + guarded_list_pop_all(&udpif->fmbs, &fmbs); + LIST_FOR_EACH_SAFE (fmb, next_fmb, list_node, &fmbs) { list_remove(&fmb->list_node); flow_miss_batch_destroy(fmb); - udpif->n_fmbs--; } - ovs_mutex_unlock(&udpif->fmb_mutex); + udpif_drop_key_clear(udpif); } @@ -288,16 +260,8 @@ udpif_revalidate(struct udpif *udpif) struct upcall * upcall_next(struct udpif *udpif) { - struct upcall *next = NULL; - - ovs_mutex_lock(&udpif->upcall_mutex); - if (udpif->n_upcalls) { - udpif->n_upcalls--; - next = CONTAINER_OF(list_pop_front(&udpif->upcalls), struct upcall, - list_node); - } - ovs_mutex_unlock(&udpif->upcall_mutex); - return next; + struct list *next = guarded_list_pop_front(&udpif->upcalls); + return next ? CONTAINER_OF(next, struct upcall, list_node) : NULL; } /* Destroys and deallocates 'upcall'. */ @@ -316,16 +280,28 @@ upcall_destroy(struct upcall *upcall) struct flow_miss_batch * flow_miss_batch_next(struct udpif *udpif) { - struct flow_miss_batch *next = NULL; + int i; + + for (i = 0; i < 50; i++) { + struct flow_miss_batch *next; + unsigned int reval_seq; + struct list *next_node; + + next_node = guarded_list_pop_front(&udpif->fmbs); + if (!next_node) { + break; + } - ovs_mutex_lock(&udpif->fmb_mutex); - if (udpif->n_fmbs) { - udpif->n_fmbs--; - next = CONTAINER_OF(list_pop_front(&udpif->fmbs), - struct flow_miss_batch, list_node); + next = CONTAINER_OF(next_node, struct flow_miss_batch, list_node); + atomic_read(&udpif->reval_seq, &reval_seq); + if (next->reval_seq == reval_seq) { + return next; + } + + flow_miss_batch_destroy(next); } - ovs_mutex_unlock(&udpif->fmb_mutex); - return next; + + return NULL; } /* Destroys and deallocates 'fmb'. */ @@ -347,52 +323,13 @@ flow_miss_batch_destroy(struct flow_miss_batch *fmb) free(fmb); } -/* Discards any flow miss batches queued up in 'udpif' for 'ofproto' (because - * 'ofproto' is being destroyed). - * - * 'ofproto''s xports must already have been removed, otherwise new flow miss - * batches could still end up getting queued. */ -void -flow_miss_batch_ofproto_destroyed(struct udpif *udpif, - const struct ofproto_dpif *ofproto) -{ - struct flow_miss_batch *fmb, *next_fmb; - - ovs_mutex_lock(&udpif->fmb_mutex); - LIST_FOR_EACH_SAFE (fmb, next_fmb, list_node, &udpif->fmbs) { - struct flow_miss *miss, *next_miss; - - HMAP_FOR_EACH_SAFE (miss, next_miss, hmap_node, &fmb->misses) { - if (miss->ofproto == ofproto) { - hmap_remove(&fmb->misses, &miss->hmap_node); - miss_destroy(miss); - } - } - - if (hmap_is_empty(&fmb->misses)) { - list_remove(&fmb->list_node); - flow_miss_batch_destroy(fmb); - udpif->n_fmbs--; - } - } - ovs_mutex_unlock(&udpif->fmb_mutex); -} - /* Retreives the next drop key which ofproto-dpif needs to process. The caller * is responsible for destroying it with drop_key_destroy(). */ struct drop_key * drop_key_next(struct udpif *udpif) { - struct drop_key *next = NULL; - - ovs_mutex_lock(&udpif->drop_key_mutex); - if (udpif->n_drop_keys) { - udpif->n_drop_keys--; - next = CONTAINER_OF(list_pop_front(&udpif->drop_keys), struct drop_key, - list_node); - } - ovs_mutex_unlock(&udpif->drop_key_mutex); - return next; + struct list *next = guarded_list_pop_front(&udpif->drop_keys); + return next ? CONTAINER_OF(next, struct drop_key, list_node) : NULL; } /* Destorys and deallocates 'drop_key'. */ @@ -410,14 +347,13 @@ void udpif_drop_key_clear(struct udpif *udpif) { struct drop_key *drop_key, *next; + struct list list; - ovs_mutex_lock(&udpif->drop_key_mutex); - LIST_FOR_EACH_SAFE (drop_key, next, list_node, &udpif->drop_keys) { + guarded_list_pop_all(&udpif->drop_keys, &list); + LIST_FOR_EACH_SAFE (drop_key, next, list_node, &list) { list_remove(&drop_key->list_node); drop_key_destroy(drop_key); - udpif->n_drop_keys--; } - ovs_mutex_unlock(&udpif->drop_key_mutex); } /* The dispatcher thread is responsible for receving upcalls from the kernel, @@ -618,17 +554,16 @@ recv_upcalls(struct udpif *udpif) upcall_destroy(upcall); } } else { - ovs_mutex_lock(&udpif->upcall_mutex); - if (udpif->n_upcalls < MAX_QUEUE_LENGTH) { - n_udpif_new_upcalls = ++udpif->n_upcalls; - list_push_back(&udpif->upcalls, &upcall->list_node); - ovs_mutex_unlock(&udpif->upcall_mutex); + size_t len; + len = guarded_list_push_back(&udpif->upcalls, &upcall->list_node, + MAX_QUEUE_LENGTH); + if (len > 0) { + n_udpif_new_upcalls = len; if (n_udpif_new_upcalls >= FLOW_MISS_MAX_BATCH) { seq_change(udpif->wait_seq); } } else { - ovs_mutex_unlock(&udpif->upcall_mutex); COVERAGE_INC(upcall_queue_overflow); upcall_destroy(upcall); } @@ -725,7 +660,7 @@ execute_flow_miss(struct flow_miss *miss, struct dpif_op *ops, size_t *n_ops) xlate_actions_for_side_effects(&xin); } } - rule_dpif_release(rule); + rule_dpif_unref(rule); if (miss->xout.odp_actions.size) { LIST_FOR_EACH (packet, list_node, &miss->packets) { @@ -762,13 +697,11 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls) { struct dpif_op *opsp[FLOW_MISS_MAX_BATCH]; struct dpif_op ops[FLOW_MISS_MAX_BATCH]; - unsigned int old_reval_seq, new_reval_seq; struct upcall *upcall, *next; struct flow_miss_batch *fmb; size_t n_upcalls, n_ops, i; struct flow_miss *miss; - - atomic_read(&udpif->reval_seq, &old_reval_seq); + unsigned int reval_seq; /* Construct the to-do list. * @@ -776,6 +709,7 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls) * the packets that have the same flow in the same "flow_miss" structure so * that we can process them together. */ fmb = xmalloc(sizeof *fmb); + atomic_read(&udpif->reval_seq, &fmb->reval_seq); hmap_init(&fmb->misses); n_upcalls = 0; LIST_FOR_EACH_SAFE (upcall, next, list_node, upcalls) { @@ -808,14 +742,10 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls) drop_key->key = xmemdup(dupcall->key, dupcall->key_len); drop_key->key_len = dupcall->key_len; - ovs_mutex_lock(&udpif->drop_key_mutex); - if (udpif->n_drop_keys < MAX_QUEUE_LENGTH) { - udpif->n_drop_keys++; - list_push_back(&udpif->drop_keys, &drop_key->list_node); - ovs_mutex_unlock(&udpif->drop_key_mutex); + if (guarded_list_push_back(&udpif->drop_keys, &drop_key->list_node, + MAX_QUEUE_LENGTH)) { seq_change(udpif->wait_seq); } else { - ovs_mutex_unlock(&udpif->drop_key_mutex); COVERAGE_INC(drop_queue_overflow); drop_key_destroy(drop_key); } @@ -868,21 +798,15 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls) } dpif_operate(udpif->dpif, opsp, n_ops); - ovs_mutex_lock(&udpif->fmb_mutex); - atomic_read(&udpif->reval_seq, &new_reval_seq); - if (old_reval_seq != new_reval_seq) { - /* udpif_revalidate() was called as we were calculating the actions. - * To be safe, we need to assume all the misses need revalidation. */ - ovs_mutex_unlock(&udpif->fmb_mutex); + atomic_read(&udpif->reval_seq, &reval_seq); + if (reval_seq != fmb->reval_seq) { + COVERAGE_INC(fmb_queue_revalidated); flow_miss_batch_destroy(fmb); - } else if (udpif->n_fmbs < MAX_QUEUE_LENGTH) { - udpif->n_fmbs++; - list_push_back(&udpif->fmbs, &fmb->list_node); - ovs_mutex_unlock(&udpif->fmb_mutex); - seq_change(udpif->wait_seq); - } else { + } else if (!guarded_list_push_back(&udpif->fmbs, &fmb->list_node, + MAX_QUEUE_LENGTH)) { COVERAGE_INC(fmb_queue_overflow); - ovs_mutex_unlock(&udpif->fmb_mutex); flow_miss_batch_destroy(fmb); + } else { + seq_change(udpif->wait_seq); } } diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h index 8e8264e2e..57d462ded 100644 --- a/ofproto/ofproto-dpif-upcall.h +++ b/ofproto/ofproto-dpif-upcall.h @@ -36,7 +36,6 @@ struct udpif *udpif_create(struct dpif_backer *, struct dpif *); void udpif_recv_set(struct udpif *, size_t n_workers, bool enable); void udpif_destroy(struct udpif *); -void udpif_run(struct udpif *); void udpif_wait(struct udpif *); void udpif_revalidate(struct udpif *); @@ -105,13 +104,12 @@ struct flow_miss_batch { struct flow_miss miss_buf[FLOW_MISS_MAX_BATCH]; struct hmap misses; + + unsigned int reval_seq; }; struct flow_miss_batch *flow_miss_batch_next(struct udpif *); void flow_miss_batch_destroy(struct flow_miss_batch *); - -void flow_miss_batch_ofproto_destroyed(struct udpif *, - const struct ofproto_dpif *); /* Drop keys are odp flow keys which have drop flows installed in the kernel. * These are datapath flows which have no associated ofproto, if they did we diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index e7cec14d6..a5b6814aa 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -44,6 +44,7 @@ #include "ofproto/ofproto-dpif-mirror.h" #include "ofproto/ofproto-dpif-sflow.h" #include "ofproto/ofproto-dpif.h" +#include "ofproto/ofproto-provider.h" #include "tunnel.h" #include "vlog.h" @@ -1668,11 +1669,9 @@ compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port) static void xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule) - OVS_RELEASES(rule) { struct rule_dpif *old_rule = ctx->rule; - const struct ofpact *ofpacts; - size_t ofpacts_len; + struct rule_actions *actions; if (ctx->xin->resubmit_stats) { rule_dpif_credit_stats(rule, ctx->xin->resubmit_stats); @@ -1680,12 +1679,11 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule) ctx->recurse++; ctx->rule = rule; - rule_dpif_get_actions(rule, &ofpacts, &ofpacts_len); - do_xlate_actions(ofpacts, ofpacts_len, ctx); + actions = rule_dpif_get_actions(rule); + do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx); + rule_actions_unref(actions); ctx->rule = old_rule; ctx->recurse--; - - rule_dpif_release(rule); } static void @@ -1696,7 +1694,6 @@ xlate_table_action(struct xlate_ctx *ctx, struct rule_dpif *rule; ofp_port_t old_in_port = ctx->xin->flow.in_port.ofp_port; uint8_t old_table_id = ctx->table_id; - bool got_rule; ctx->table_id = table_id; @@ -1704,18 +1701,16 @@ xlate_table_action(struct xlate_ctx *ctx, * original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT will * have surprising behavior). */ ctx->xin->flow.in_port.ofp_port = in_port; - got_rule = rule_dpif_lookup_in_table(ctx->xbridge->ofproto, - &ctx->xin->flow, &ctx->xout->wc, - table_id, &rule); + rule_dpif_lookup_in_table(ctx->xbridge->ofproto, + &ctx->xin->flow, &ctx->xout->wc, + table_id, &rule); ctx->xin->flow.in_port.ofp_port = old_in_port; if (ctx->xin->resubmit_hook) { ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse); } - if (got_rule) { - xlate_recursively(ctx, rule); - } else if (may_packet_in) { + if (!rule && may_packet_in) { struct xport *xport; /* XXX @@ -1728,7 +1723,10 @@ xlate_table_action(struct xlate_ctx *ctx, choose_miss_rule(xport ? xport->config : 0, ctx->xbridge->miss_rule, ctx->xbridge->no_packet_in_rule, &rule); + } + if (rule) { xlate_recursively(ctx, rule); + rule_dpif_unref(rule); } ctx->table_id = old_table_id; @@ -2098,7 +2096,8 @@ static void xlate_learn_action(struct xlate_ctx *ctx, const struct ofpact_learn *learn) { - struct ofputil_flow_mod *fm; + uint64_t ofpacts_stub[1024 / 8]; + struct ofputil_flow_mod fm; struct ofpbuf ofpacts; ctx->xout->has_learn = true; @@ -2109,11 +2108,10 @@ xlate_learn_action(struct xlate_ctx *ctx, return; } - fm = xmalloc(sizeof *fm); - ofpbuf_init(&ofpacts, 0); - learn_execute(learn, &ctx->xin->flow, fm, &ofpacts); - - ofproto_dpif_flow_mod(ctx->xbridge->ofproto, fm); + ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub); + learn_execute(learn, &ctx->xin->flow, &fm, &ofpacts); + ofproto_dpif_flow_mod(ctx->xbridge->ofproto, &fm); + ofpbuf_uninit(&ofpacts); } static void @@ -2528,6 +2526,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) struct flow_wildcards *wc = &xout->wc; struct flow *flow = &xin->flow; + struct rule_actions *actions = NULL; enum slow_path_reason special; const struct ofpact *ofpacts; struct xport *in_port; @@ -2604,7 +2603,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) ofpacts = xin->ofpacts; ofpacts_len = xin->ofpacts_len; } else if (xin->rule) { - rule_dpif_get_actions(xin->rule, &ofpacts, &ofpacts_len); + actions = rule_dpif_get_actions(xin->rule); + ofpacts = actions->ofpacts; + ofpacts_len = actions->ofpacts_len; } else { NOT_REACHED(); } @@ -2690,4 +2691,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) out: ovs_rwlock_unlock(&xlate_rwlock); + + rule_actions_unref(actions); } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 95fb8744d..b91b3dffb 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -31,6 +31,7 @@ #include "dpif.h" #include "dynamic-string.h" #include "fail-open.h" +#include "guarded-list.h" #include "hmapx.h" #include "lacp.h" #include "learn.h" @@ -484,9 +485,6 @@ struct ofproto_dpif { struct classifier facets; /* Contains 'struct facet's. */ long long int consistency_rl; - /* Support for debugging async flow mods. */ - struct list completions; - struct netdev_stats stats; /* To account packets generated and consumed in * userspace. */ @@ -510,19 +508,9 @@ struct ofproto_dpif { uint64_t n_missed; /* Work queues. */ - struct ovs_mutex flow_mod_mutex; - struct list flow_mods OVS_GUARDED; - size_t n_flow_mods OVS_GUARDED; - - struct ovs_mutex pin_mutex; - struct list pins OVS_GUARDED; - size_t n_pins OVS_GUARDED; + struct guarded_list pins; /* Contains "struct ofputil_packet_in"s. */ }; -/* Defer flow mod completion until "ovs-appctl ofproto/unclog"? (Useful only - * for debugging the asynchronous flow_mod implementation.) */ -static bool clogged; - /* By default, flows in the datapath are wildcarded (megaflows). They * may be disabled with the "ovs-appctl dpif/disable-megaflows" command. */ static bool enable_megaflows = true; @@ -562,23 +550,13 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); /* Initial mappings of port to bridge mappings. */ static struct shash init_ofp_ports = SHASH_INITIALIZER(&init_ofp_ports); -/* Executes and takes ownership of 'fm'. */ +/* Executes 'fm'. The caller retains ownership of 'fm' and everything in + * it. */ void ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto, struct ofputil_flow_mod *fm) { - ovs_mutex_lock(&ofproto->flow_mod_mutex); - if (ofproto->n_flow_mods > 1024) { - ovs_mutex_unlock(&ofproto->flow_mod_mutex); - COVERAGE_INC(flow_mod_overflow); - free(fm->ofpacts); - free(fm); - return; - } - - list_push_back(&ofproto->flow_mods, &fm->list_node); - ofproto->n_flow_mods++; - ovs_mutex_unlock(&ofproto->flow_mod_mutex); + ofproto_flow_mod(&ofproto->up, fm); } /* Appends 'pin' to the queue of "packet ins" to be sent to the controller. @@ -587,18 +565,11 @@ void ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto, struct ofputil_packet_in *pin) { - ovs_mutex_lock(&ofproto->pin_mutex); - if (ofproto->n_pins > 1024) { - ovs_mutex_unlock(&ofproto->pin_mutex); + if (!guarded_list_push_back(&ofproto->pins, &pin->list_node, 1024)) { COVERAGE_INC(packet_in_overflow); free(CONST_CAST(void *, pin->packet)); free(pin); - return; } - - list_push_back(&ofproto->pins, &pin->list_node); - ofproto->n_pins++; - ovs_mutex_unlock(&ofproto->pin_mutex); } /* Factory functions. */ @@ -1031,7 +1002,6 @@ process_dpif_port_error(struct dpif_backer *backer, int error) static int dpif_backer_run_fast(struct dpif_backer *backer) { - udpif_run(backer->udpif); handle_upcalls(backer); return 0; @@ -1293,19 +1263,7 @@ construct(struct ofproto *ofproto_) classifier_init(&ofproto->facets); ofproto->consistency_rl = LLONG_MIN; - list_init(&ofproto->completions); - - ovs_mutex_init(&ofproto->flow_mod_mutex); - ovs_mutex_lock(&ofproto->flow_mod_mutex); - list_init(&ofproto->flow_mods); - ofproto->n_flow_mods = 0; - ovs_mutex_unlock(&ofproto->flow_mod_mutex); - - ovs_mutex_init(&ofproto->pin_mutex); - ovs_mutex_lock(&ofproto->pin_mutex); - list_init(&ofproto->pins); - ofproto->n_pins = 0; - ovs_mutex_unlock(&ofproto->pin_mutex); + guarded_list_init(&ofproto->pins); ofproto_dpif_unixctl_init(); @@ -1380,7 +1338,7 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id, if (rule_dpif_lookup_in_table(ofproto, &fm.match.flow, NULL, TBL_INTERNAL, rulep)) { - rule_dpif_release(*rulep); + rule_dpif_unref(*rulep); } else { NOT_REACHED(); } @@ -1423,28 +1381,16 @@ add_internal_flows(struct ofproto_dpif *ofproto) return error; } -static void -complete_operations(struct ofproto_dpif *ofproto) -{ - struct dpif_completion *c, *next; - - LIST_FOR_EACH_SAFE (c, next, list_node, &ofproto->completions) { - ofoperation_complete(c->op, 0); - list_remove(&c->list_node); - free(c); - } -} - static void destruct(struct ofproto *ofproto_) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); struct rule_dpif *rule, *next_rule; struct ofputil_packet_in *pin, *next_pin; - struct ofputil_flow_mod *fm, *next_fm; struct facet *facet, *next_facet; struct cls_cursor cursor; struct oftable *table; + struct list pins; ovs_rwlock_rdlock(&ofproto->facets.rwlock); cls_cursor_init(&cursor, &ofproto->facets, NULL); @@ -1458,42 +1404,30 @@ destruct(struct ofproto *ofproto_) xlate_remove_ofproto(ofproto); ovs_rwlock_unlock(&xlate_rwlock); - flow_miss_batch_ofproto_destroyed(ofproto->backer->udpif, ofproto); + /* Discard any flow_miss_batches queued up for 'ofproto', avoiding a + * use-after-free error. */ + udpif_revalidate(ofproto->backer->udpif); hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node); - complete_operations(ofproto); OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) { struct cls_cursor cursor; - ovs_rwlock_wrlock(&table->cls.rwlock); + ovs_rwlock_rdlock(&table->cls.rwlock); cls_cursor_init(&cursor, &table->cls, NULL); + ovs_rwlock_unlock(&table->cls.rwlock); CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, up.cr, &cursor) { - ofproto_rule_delete(&ofproto->up, &table->cls, &rule->up); + ofproto_rule_delete(&ofproto->up, &rule->up); } - ovs_rwlock_unlock(&table->cls.rwlock); - } - complete_operations(ofproto); - - ovs_mutex_lock(&ofproto->flow_mod_mutex); - LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &ofproto->flow_mods) { - list_remove(&fm->list_node); - ofproto->n_flow_mods--; - free(fm->ofpacts); - free(fm); } - ovs_mutex_unlock(&ofproto->flow_mod_mutex); - ovs_mutex_destroy(&ofproto->flow_mod_mutex); - ovs_mutex_lock(&ofproto->pin_mutex); - LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &ofproto->pins) { + guarded_list_pop_all(&ofproto->pins, &pins); + LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) { list_remove(&pin->list_node); - ofproto->n_pins--; free(CONST_CAST(void *, pin->packet)); free(pin); } - ovs_mutex_unlock(&ofproto->pin_mutex); - ovs_mutex_destroy(&ofproto->pin_mutex); + guarded_list_destroy(&ofproto->pins); mbridge_unref(ofproto->mbridge); @@ -1521,9 +1455,8 @@ run_fast(struct ofproto *ofproto_) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); struct ofputil_packet_in *pin, *next_pin; - struct ofputil_flow_mod *fm, *next_fm; - struct list flow_mods, pins; struct ofport_dpif *ofport; + struct list pins; /* Do not perform any periodic activity required by 'ofproto' while * waiting for flow restore to complete. */ @@ -1531,30 +1464,7 @@ run_fast(struct ofproto *ofproto_) return 0; } - ovs_mutex_lock(&ofproto->flow_mod_mutex); - list_move(&flow_mods, &ofproto->flow_mods); - list_init(&ofproto->flow_mods); - ofproto->n_flow_mods = 0; - ovs_mutex_unlock(&ofproto->flow_mod_mutex); - - LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &flow_mods) { - int error = ofproto_flow_mod(&ofproto->up, fm); - if (error && !VLOG_DROP_WARN(&rl)) { - VLOG_WARN("learning action failed to modify flow table (%s)", - ofperr_get_name(error)); - } - - list_remove(&fm->list_node); - free(fm->ofpacts); - free(fm); - } - - ovs_mutex_lock(&ofproto->pin_mutex); - list_move(&pins, &ofproto->pins); - list_init(&ofproto->pins); - ofproto->n_pins = 0; - ovs_mutex_unlock(&ofproto->pin_mutex); - + guarded_list_pop_all(&ofproto->pins, &pins); LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) { connmgr_send_packet_in(ofproto->up.connmgr, pin); list_remove(&pin->list_node); @@ -1577,10 +1487,6 @@ run(struct ofproto *ofproto_) struct ofbundle *bundle; int error; - if (!clogged) { - complete_operations(ofproto); - } - if (mbridge_need_revalidate(ofproto->mbridge)) { ofproto->backer->need_revalidate = REV_RECONFIGURE; ovs_rwlock_wrlock(&ofproto->ml->rwlock); @@ -1658,10 +1564,6 @@ wait(struct ofproto *ofproto_) struct ofport_dpif *ofport; struct ofbundle *bundle; - if (!clogged && !list_is_empty(&ofproto->completions)) { - poll_immediate_wake(); - } - if (ofproto_get_flow_restore_wait()) { return; } @@ -3657,7 +3559,7 @@ handle_upcalls(struct dpif_backer *backer) static int subfacet_max_idle(const struct dpif_backer *); static void update_stats(struct dpif_backer *); -static void rule_expire(struct rule_dpif *); +static void rule_expire(struct rule_dpif *) OVS_REQUIRES(ofproto_mutex); static void expire_subfacets(struct dpif_backer *, int dp_max_idle); /* This function is called periodically by run(). Its job is to collect @@ -3711,12 +3613,12 @@ expire(struct dpif_backer *backer) /* Expire OpenFlow flows whose idle_timeout or hard_timeout * has passed. */ - ovs_mutex_lock(&ofproto->up.expirable_mutex); + ovs_mutex_lock(&ofproto_mutex); LIST_FOR_EACH_SAFE (rule, next_rule, expirable, &ofproto->up.expirable) { rule_expire(rule_dpif_cast(rule)); } - ovs_mutex_unlock(&ofproto->up.expirable_mutex); + ovs_mutex_unlock(&ofproto_mutex); /* All outstanding data in existing flows has been accounted, so it's a * good time to do bond rebalancing. */ @@ -3977,33 +3879,31 @@ expire_subfacets(struct dpif_backer *backer, int dp_max_idle) * then delete it entirely. */ static void rule_expire(struct rule_dpif *rule) + OVS_REQUIRES(ofproto_mutex) { uint16_t idle_timeout, hard_timeout; - long long int now; - uint8_t reason; + long long int now = time_msec(); + int reason; - if (rule->up.pending) { - /* We'll have to expire it later. */ - return; - } + ovs_assert(!rule->up.pending); - ovs_mutex_lock(&rule->up.timeout_mutex); + /* Has 'rule' expired? */ + ovs_mutex_lock(&rule->up.mutex); hard_timeout = rule->up.hard_timeout; idle_timeout = rule->up.idle_timeout; - ovs_mutex_unlock(&rule->up.timeout_mutex); - - /* Has 'rule' expired? */ - now = time_msec(); if (hard_timeout && now > rule->up.modified + hard_timeout * 1000) { reason = OFPRR_HARD_TIMEOUT; } else if (idle_timeout && now > rule->up.used + idle_timeout * 1000) { reason = OFPRR_IDLE_TIMEOUT; } else { - return; + reason = -1; } + ovs_mutex_unlock(&rule->up.mutex); - COVERAGE_INC(ofproto_dpif_expired); - ofproto_rule_expire(&rule->up, reason); + if (reason >= 0) { + COVERAGE_INC(ofproto_dpif_expired); + ofproto_rule_expire(&rule->up, reason); + } } /* Facets. */ @@ -4195,17 +4095,22 @@ facet_is_controller_flow(struct facet *facet) if (facet) { struct ofproto_dpif *ofproto = facet->ofproto; const struct ofpact *ofpacts; + struct rule_actions *actions; struct rule_dpif *rule; size_t ofpacts_len; bool is_controller; rule_dpif_lookup(ofproto, &facet->flow, NULL, &rule); - ofpacts_len = rule->up.ofpacts_len; - ofpacts = rule->up.ofpacts; + actions = rule_dpif_get_actions(rule); + rule_dpif_unref(rule); + + ofpacts_len = actions->ofpacts_len; + ofpacts = actions->ofpacts; is_controller = ofpacts_len > 0 && ofpacts->type == OFPACT_CONTROLLER && ofpact_next(ofpacts) >= ofpact_end(ofpacts, ofpacts_len); - rule_dpif_release(rule); + rule_actions_unref(actions); + return is_controller; } return false; @@ -4299,7 +4204,7 @@ facet_check_consistency(struct facet *facet) rule_dpif_lookup(facet->ofproto, &facet->flow, NULL, &rule); xlate_in_init(&xin, facet->ofproto, &facet->flow, rule, 0, NULL); xlate_actions(&xin, &xout); - rule_dpif_release(rule); + rule_dpif_unref(rule); ok = ofpbuf_equal(&facet->xout.odp_actions, &xout.odp_actions) && facet->xout.slow == xout.slow; @@ -4397,7 +4302,7 @@ facet_revalidate(struct facet *facet) || memcmp(&facet->xout.wc, &xout.wc, sizeof xout.wc)) { facet_remove(facet); xlate_out_uninit(&xout); - rule_dpif_release(new_rule); + rule_dpif_unref(new_rule); return false; } @@ -4426,10 +4331,13 @@ facet_revalidate(struct facet *facet) facet->xout.nf_output_iface = xout.nf_output_iface; facet->xout.mirrors = xout.mirrors; facet->nf_flow.output_iface = facet->xout.nf_output_iface; + + ovs_mutex_lock(&new_rule->up.mutex); facet->used = MAX(facet->used, new_rule->up.created); + ovs_mutex_unlock(&new_rule->up.mutex); xlate_out_uninit(&xout); - rule_dpif_release(new_rule); + rule_dpif_unref(new_rule); return true; } @@ -4462,7 +4370,7 @@ flow_push_stats(struct ofproto_dpif *ofproto, struct flow *flow, xin.resubmit_stats = stats; xin.may_learn = may_learn; xlate_actions_for_side_effects(&xin); - rule_dpif_release(rule); + rule_dpif_unref(rule); } static void @@ -4546,6 +4454,7 @@ rule_dpif_fail_open(const struct rule_dpif *rule) ovs_be64 rule_dpif_get_flow_cookie(const struct rule_dpif *rule) + OVS_REQUIRES(rule->up.mutex) { return rule->up.flow_cookie; } @@ -4557,12 +4466,13 @@ rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout, ofproto_rule_reduce_timeouts(&rule->up, idle_timeout, hard_timeout); } -void -rule_dpif_get_actions(const struct rule_dpif *rule, - const struct ofpact **ofpacts, size_t *ofpacts_len) +/* Returns 'rule''s actions. The caller owns a reference on the returned + * actions and must eventually release it (with rule_actions_unref()) to avoid + * a memory leak. */ +struct rule_actions * +rule_dpif_get_actions(const struct rule_dpif *rule) { - *ofpacts = rule->up.ofpacts; - *ofpacts_len = rule->up.ofpacts_len; + return rule_get_actions(&rule->up); } /* Subfacets. */ @@ -4840,9 +4750,8 @@ bool rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, const struct flow *flow, struct flow_wildcards *wc, uint8_t table_id, struct rule_dpif **rule) - OVS_TRY_RDLOCK(true, (*rule)->up.rwlock) { - struct cls_rule *cls_rule; + const struct cls_rule *cls_rule; struct classifier *cls; bool frag; @@ -4875,11 +4784,7 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, } *rule = rule_dpif_cast(rule_from_cls_rule(cls_rule)); - if (*rule && ovs_rwlock_tryrdlock(&(*rule)->up.rwlock)) { - /* The rule is in the process of being removed. Best we can do is - * pretend it isn't there. */ - *rule = NULL; - } + rule_dpif_ref(*rule); ovs_rwlock_unlock(&cls->rwlock); return *rule != NULL; @@ -4891,34 +4796,35 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, void choose_miss_rule(enum ofputil_port_config config, struct rule_dpif *miss_rule, struct rule_dpif *no_packet_in_rule, struct rule_dpif **rule) - OVS_NO_THREAD_SAFETY_ANALYSIS { *rule = config & OFPUTIL_PC_NO_PACKET_IN ? no_packet_in_rule : miss_rule; - ovs_rwlock_rdlock(&(*rule)->up.rwlock); + rule_dpif_ref(*rule); +} + +void +rule_dpif_ref(struct rule_dpif *rule) +{ + if (rule) { + ofproto_rule_ref(&rule->up); + } } void -rule_dpif_release(struct rule_dpif *rule) - OVS_NO_THREAD_SAFETY_ANALYSIS +rule_dpif_unref(struct rule_dpif *rule) { if (rule) { - ovs_rwlock_unlock(&rule->up.rwlock); + ofproto_rule_unref(&rule->up); } } static void complete_operation(struct rule_dpif *rule) + OVS_REQUIRES(ofproto_mutex) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto); ofproto->backer->need_revalidate = REV_FLOW_TABLE; - if (clogged) { - struct dpif_completion *c = xmalloc(sizeof *c); - c->op = rule->up.pending; - list_push_back(&ofproto->completions, &c->list_node); - } else { - ofoperation_complete(rule->up.pending, 0); - } + ofoperation_complete(rule->up.pending, 0); } static struct rule_dpif *rule_dpif_cast(const struct rule *rule) @@ -4954,6 +4860,7 @@ rule_construct(struct rule *rule_) static void rule_insert(struct rule *rule_) + OVS_REQUIRES(ofproto_mutex) { struct rule_dpif *rule = rule_dpif_cast(rule_); complete_operation(rule); @@ -4961,6 +4868,7 @@ rule_insert(struct rule *rule_) static void rule_delete(struct rule *rule_) + OVS_REQUIRES(ofproto_mutex) { struct rule_dpif *rule = rule_dpif_cast(rule_); complete_operation(rule); @@ -5025,6 +4933,7 @@ rule_execute(struct rule *rule, const struct flow *flow, static void rule_modify_actions(struct rule *rule_, bool reset_counters) + OVS_REQUIRES(ofproto_mutex) { struct rule_dpif *rule = rule_dpif_cast(rule_); @@ -5339,21 +5248,32 @@ struct trace_ctx { static void trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule) { + struct rule_actions *actions; + ovs_be64 cookie; + ds_put_char_multiple(result, '\t', level); if (!rule) { ds_put_cstr(result, "No match\n"); return; } + ovs_mutex_lock(&rule->up.mutex); + cookie = rule->up.flow_cookie; + ovs_mutex_unlock(&rule->up.mutex); + ds_put_format(result, "Rule: table=%"PRIu8" cookie=%#"PRIx64" ", - rule ? rule->up.table_id : 0, ntohll(rule->up.flow_cookie)); + rule ? rule->up.table_id : 0, ntohll(cookie)); cls_rule_format(&rule->up.cr, result); ds_put_char(result, '\n'); + actions = rule_dpif_get_actions(rule); + ds_put_char_multiple(result, '\t', level); ds_put_cstr(result, "OpenFlow "); - ofpacts_format(rule->up.ofpacts, rule->up.ofpacts_len, result); + ofpacts_format(actions->ofpacts, actions->ofpacts_len, result); ds_put_char(result, '\n'); + + rule_actions_unref(actions); } static void @@ -5623,23 +5543,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow, xlate_out_uninit(&trace.xout); } - rule_dpif_release(rule); -} - -static void -ofproto_dpif_clog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) -{ - clogged = true; - unixctl_command_reply(conn, NULL); -} - -static void -ofproto_dpif_unclog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) -{ - clogged = false; - unixctl_command_reply(conn, NULL); + rule_dpif_unref(rule); } /* Runs a self-check of flow translations in 'ofproto'. Appends a message to @@ -6058,10 +5962,6 @@ ofproto_dpif_unixctl_init(void) ofproto_unixctl_fdb_flush, NULL); unixctl_command_register("fdb/show", "bridge", 1, 1, ofproto_unixctl_fdb_show, NULL); - unixctl_command_register("ofproto/clog", "", 0, 0, - ofproto_dpif_clog, NULL); - unixctl_command_register("ofproto/unclog", "", 0, 0, - ofproto_dpif_unclog, NULL); unixctl_command_register("ofproto/self-check", "[bridge]", 0, 1, ofproto_dpif_self_check, NULL); unixctl_command_register("dpif/dump-dps", "", 0, 0, diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index 7efc8d736..14a96693b 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -61,24 +61,21 @@ struct OVS_LOCKABLE rule_dpif; * actions into datapath actions. */ void rule_dpif_lookup(struct ofproto_dpif *, const struct flow *, - struct flow_wildcards *, struct rule_dpif **rule) - OVS_ACQ_RDLOCK(*rule); + struct flow_wildcards *, struct rule_dpif **rule); bool rule_dpif_lookup_in_table(struct ofproto_dpif *, const struct flow *, struct flow_wildcards *, uint8_t table_id, - struct rule_dpif **rule) - OVS_TRY_RDLOCK(true, *rule); + struct rule_dpif **rule); -void rule_dpif_release(struct rule_dpif *rule) OVS_RELEASES(rule); +void rule_dpif_ref(struct rule_dpif *); +void rule_dpif_unref(struct rule_dpif *); void rule_dpif_credit_stats(struct rule_dpif *rule , const struct dpif_flow_stats *); bool rule_dpif_fail_open(const struct rule_dpif *rule); -void rule_dpif_get_actions(const struct rule_dpif *rule, - const struct ofpact **ofpacts, - size_t *ofpacts_len); +struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *); ovs_be64 rule_dpif_get_flow_cookie(const struct rule_dpif *rule); @@ -88,8 +85,7 @@ void rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout, void choose_miss_rule(enum ofputil_port_config, struct rule_dpif *miss_rule, struct rule_dpif *no_packet_in_rule, - struct rule_dpif **rule) - OVS_ACQ_RDLOCK(*rule); + struct rule_dpif **rule); bool ofproto_has_vlan_splinters(const struct ofproto_dpif *); ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *, diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 4cbf47ff1..bbb9ba115 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -17,16 +17,32 @@ #ifndef OFPROTO_OFPROTO_PROVIDER_H #define OFPROTO_OFPROTO_PROVIDER_H 1 -/* Definitions for use within ofproto. */ +/* Definitions for use within ofproto. + * + * + * Thread-safety + * ============= + * + * Lots of ofproto data structures are only accessed from a single thread. + * Those data structures are generally not thread-safe. + * + * The ofproto-dpif ofproto implementation accesses the flow table from + * multiple threads, including modifying the flow table from multiple threads + * via the "learn" action, so the flow table and various structures that index + * it have been made thread-safe. Refer to comments on individual data + * structures for details. + */ #include "cfm.h" #include "classifier.h" +#include "guarded-list.h" #include "heap.h" #include "hindex.h" #include "list.h" #include "ofp-errors.h" #include "ofp-util.h" #include "ofproto/ofproto.h" +#include "ovs-atomic.h" #include "ovs-thread.h" #include "shash.h" #include "simap.h" @@ -38,6 +54,8 @@ struct ofputil_flow_mod; struct bfd_cfg; struct meter; +extern struct ovs_mutex ofproto_mutex; + /* An OpenFlow switch. * * With few exceptions, ofproto implementations may look at these fields but @@ -73,13 +91,11 @@ struct ofproto { struct oftable *tables; int n_tables; - struct hindex cookies; /* Rules indexed on their cookie values. */ + /* Rules indexed on their cookie values, in all flow tables. */ + struct hindex cookies OVS_GUARDED_BY(ofproto_mutex); - /* Optimisation for flow expiry. - * These flows should all be present in tables. */ - struct ovs_mutex expirable_mutex; - struct list expirable OVS_GUARDED; /* Expirable 'struct rule"s in all - tables. */ + /* List of expirable flows, in all flow tables. */ + struct list expirable OVS_GUARDED_BY(ofproto_mutex); /* Meter table. * OpenFlow meters start at 1. To avoid confusion we leave the first @@ -91,11 +107,29 @@ struct ofproto { /* OpenFlow connections. */ struct connmgr *connmgr; - /* Flow table operation tracking. */ - int state; /* Internal state. */ - struct list pending; /* List of "struct ofopgroup"s. */ - unsigned int n_pending; /* list_size(&pending). */ - struct hmap deletions; /* All OFOPERATION_DELETE "ofoperation"s. */ + /* Flow table operation tracking. + * + * 'state' is meaningful only within ofproto.c, one of the enum + * ofproto_state constants defined there. + * + * 'pending' is the list of "struct ofopgroup"s currently pending. + * + * 'n_pending' is the number of elements in 'pending'. + * + * 'deletions' contains pending ofoperations of type OFOPERATION_DELETE, + * indexed on its rule's flow.*/ + int state; + struct list pending OVS_GUARDED_BY(ofproto_mutex); + unsigned int n_pending OVS_GUARDED_BY(ofproto_mutex); + struct hmap deletions OVS_GUARDED_BY(ofproto_mutex); + + /* Delayed rule executions. + * + * We delay calls to ->ofproto_class->rule_execute() past releasing + * ofproto_mutex during a flow_mod, because otherwise a "learn" action + * triggered by the executing the packet would try to recursively modify + * the flow table and reacquire the global lock. */ + struct guarded_list rule_executes; /* Contains "struct rule_execute"s. */ /* Flow table operation logging. */ int n_add, n_delete, n_modify; /* Number of unreported ops of each kind. */ @@ -171,7 +205,29 @@ enum oftable_flags { OFTABLE_READONLY = 1 << 1 /* Don't allow OpenFlow to change this table. */ }; -/* A flow table within a "struct ofproto". */ +/* A flow table within a "struct ofproto". + * + * + * Thread-safety + * ============= + * + * A cls->rwlock read-lock holder prevents rules from being added or deleted. + * + * Adding or removing rules requires holding ofproto_mutex AND the cls->rwlock + * write-lock. + * + * cls->rwlock should be held only briefly. For extended access to a rule, + * increment its ref_count with ofproto_rule_ref(). A rule will not be freed + * until its ref_count reaches zero. + * + * Modifying a rule requires the rule's own mutex. Holding cls->rwlock (for + * read or write) does not allow the holder to modify the rule. + * + * Freeing a rule requires ofproto_mutex and the cls->rwlock write-lock. After + * removing the rule from the classifier, release a ref_count from the rule + * ('cls''s reference to the rule). + * + * Refer to the thread-safety notes on struct rule for more information.*/ struct oftable { enum oftable_flags flags; struct classifier cls; /* Contains "struct rule"s. */ @@ -215,60 +271,172 @@ struct oftable { /* An OpenFlow flow within a "struct ofproto". * * With few exceptions, ofproto implementations may look at these fields but - * should not modify them. */ + * should not modify them. + * + * + * Thread-safety + * ============= + * + * Except near the beginning or ending of its lifespan, rule 'rule' belongs to + * the classifier rule->ofproto->tables[rule->table_id].cls. The text below + * calls this classifier 'cls'. + * + * Motivation + * ---------- + * + * The thread safety rules described here for "struct rule" are motivated by + * two goals: + * + * - Prevent threads that read members of "struct rule" from reading bad + * data due to changes by some thread concurrently modifying those + * members. + * + * - Prevent two threads making changes to members of a given "struct rule" + * from interfering with each other. + * + * + * Rules + * ----- + * + * A rule 'rule' may be accessed without a risk of being freed by code that + * holds a read-lock or write-lock on 'cls->rwlock' or that owns a reference to + * 'rule->ref_count' (or both). Code that needs to hold onto a rule for a + * while should take 'cls->rwlock', find the rule it needs, increment + * 'rule->ref_count' with ofproto_rule_ref(), and drop 'cls->rwlock'. + * + * 'rule->ref_count' protects 'rule' from being freed. It doesn't protect the + * rule from being deleted from 'cls' (that's 'cls->rwlock') and it doesn't + * protect members of 'rule' from modification (that's 'rule->rwlock'). + * + * 'rule->mutex' protects the members of 'rule' from modification. It doesn't + * protect the rule from being deleted from 'cls' (that's 'cls->rwlock') and it + * doesn't prevent the rule from being freed (that's 'rule->ref_count'). + * + * Regarding thread safety, the members of a rule fall into the following + * categories: + * + * - Immutable. These members are marked 'const'. + * + * - Members that may be safely read or written only by code holding + * ofproto_mutex. These are marked OVS_GUARDED_BY(ofproto_mutex). + * + * - Members that may be safely read only by code holding ofproto_mutex or + * 'rule->mutex', and safely written only by coding holding ofproto_mutex + * AND 'rule->mutex'. These are marked OVS_GUARDED. + */ struct rule { - struct list ofproto_node; /* Owned by ofproto base code. */ - struct ofproto *ofproto; /* The ofproto that contains this rule. */ - struct cls_rule cr; /* In owning ofproto's classifier. */ + /* Where this rule resides in an OpenFlow switch. + * + * These are immutable once the rule is constructed, hence 'const'. */ + struct ofproto *const ofproto; /* The ofproto that contains this rule. */ + const struct cls_rule cr; /* In owning ofproto's classifier. */ + const uint8_t table_id; /* Index in ofproto's 'tables' array. */ + + /* Protects members marked OVS_GUARDED. + * Readers only need to hold this mutex. + * Writers must hold both this mutex AND ofproto_mutex. */ + struct ovs_mutex mutex OVS_ACQ_AFTER(ofproto_mutex); + + /* Number of references. + * The classifier owns one reference. + * Any thread trying to keep a rule from being freed should hold its own + * reference. */ + atomic_uint ref_count; + + /* Operation now in progress, if nonnull. */ + struct ofoperation *pending OVS_GUARDED_BY(ofproto_mutex); + + /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with + * a flow.. */ + ovs_be64 flow_cookie OVS_GUARDED; + struct hindex_node cookie_node OVS_GUARDED_BY(ofproto_mutex); + + /* Times. */ + long long int created OVS_GUARDED; /* Creation time. */ + long long int modified OVS_GUARDED; /* Time of last modification. */ + long long int used OVS_GUARDED; /* Last use; time created if never used. */ + enum ofputil_flow_mod_flags flags OVS_GUARDED; + + /* Timeouts. */ + uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */ + uint16_t idle_timeout OVS_GUARDED; /* In seconds from ->used. */ - struct ofoperation *pending; /* Operation now in progress, if nonnull. */ + /* 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 + * any eviction group. When 'eviction_group' is nonnull, 'evg_node' is in + * the ->eviction_group->rules hmap. */ + struct eviction_group *eviction_group OVS_GUARDED_BY(ofproto_mutex); + struct heap_node evg_node OVS_GUARDED_BY(ofproto_mutex); - ovs_be64 flow_cookie; /* Controller-issued identifier. Guarded by - rwlock. */ - struct hindex_node cookie_node; /* In owning ofproto's 'cookies' index. */ + /* OpenFlow actions. See struct rule_actions for more thread-safety + * notes. */ + struct rule_actions *actions OVS_GUARDED; - long long int created; /* Creation time. */ - long long int modified; /* Time of last modification. */ - long long int used; /* Last use; time created if never used. */ - uint8_t table_id; /* Index in ofproto's 'tables' array. */ - enum ofputil_flow_mod_flags flags; + /* In owning meter's 'rules' list. An empty list if there is no meter. */ + struct list meter_list_node OVS_GUARDED_BY(ofproto_mutex); - struct ovs_mutex timeout_mutex; - uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */ - uint16_t idle_timeout OVS_GUARDED; /* In seconds from ->used. */ + /* Flow monitors (e.g. for NXST_FLOW_MONITOR, related to struct ofmonitor). + * + * 'add_seqno' is the sequence number when this rule was created. + * 'modify_seqno' is the sequence number when this rule was last modified. + * See 'monitor_seqno' in connmgr.c for more information. */ + enum nx_flow_monitor_flags monitor_flags OVS_GUARDED_BY(ofproto_mutex); + uint64_t add_seqno OVS_GUARDED_BY(ofproto_mutex); + uint64_t modify_seqno OVS_GUARDED_BY(ofproto_mutex); - /* Eviction groups. */ - struct heap_node evg_node; /* In eviction_group's "rules" heap. */ - struct eviction_group *eviction_group; /* NULL if not in any group. */ + /* Optimisation for flow expiry. In ofproto's 'expirable' list if this + * rule is expirable, otherwise empty. */ + struct list expirable OVS_GUARDED_BY(ofproto_mutex); +}; - /* The rwlock is used to protect those elements in struct rule which are - * accessed by multiple threads. While maintaining a pointer to struct - * rule, threads are required to hold a readlock. The main ofproto code is - * guaranteed not to evict the rule, or change any of the elements "Guarded - * by rwlock" without holding the writelock. - * - * A rule will not be evicted unless both its own and its classifier's - * write locks are held. Therefore, while holding a classifier readlock, - * one can be assured that write locked rules are safe to reference. */ - struct ovs_rwlock rwlock; +void ofproto_rule_ref(struct rule *); +void ofproto_rule_unref(struct rule *); - /* Guarded by rwlock. */ - struct ofpact *ofpacts; /* Sequence of "struct ofpacts". */ - unsigned int ofpacts_len; /* Size of 'ofpacts', in bytes. */ +struct rule_actions *rule_get_actions(const struct rule *rule) + OVS_EXCLUDED(rule->mutex); +struct rule_actions *rule_get_actions__(const struct rule *rule) + OVS_REQUIRES(rule->mutex); - uint32_t meter_id; /* Non-zero OF meter_id, or zero. */ - struct list meter_list_node; /* In owning meter's 'rules' list. */ +/* A set of actions within a "struct rule". + * + * + * Thread-safety + * ============= + * + * A struct rule_actions 'actions' may be accessed without a risk of being + * freed by code that holds a read-lock or write-lock on 'rule->mutex' (where + * 'rule' is the rule for which 'rule->actions == actions') or that owns a + * reference to 'actions->ref_count' (or both). */ +struct rule_actions { + atomic_uint ref_count; + + /* These members are immutable: they do not change during the struct's + * lifetime. */ + struct ofpact *ofpacts; /* Sequence of "struct ofpacts". */ + unsigned int ofpacts_len; /* Size of 'ofpacts', in bytes. */ + uint32_t meter_id; /* Non-zero OF meter_id, or zero. */ +}; + +struct rule_actions *rule_actions_create(const struct ofpact *, size_t); +void rule_actions_ref(struct rule_actions *); +void rule_actions_unref(struct rule_actions *); - /* Flow monitors. */ - enum nx_flow_monitor_flags monitor_flags; - uint64_t add_seqno; /* Sequence number when added. */ - uint64_t modify_seqno; /* Sequence number when changed. */ +/* A set of rules to which an OpenFlow operation applies. */ +struct rule_collection { + struct rule **rules; /* The rules. */ + size_t n; /* Number of rules collected. */ - /* Optimisation for flow expiry. */ - struct list expirable; /* In ofproto's 'expirable' list if this rule - * is expirable, otherwise empty. */ + size_t capacity; /* Number of rules that will fit in 'rules'. */ + struct rule *stub[64]; /* Preallocated rules to avoid malloc(). */ }; +void rule_collection_init(struct rule_collection *); +void rule_collection_add(struct rule_collection *, struct rule *); +void rule_collection_ref(struct rule_collection *) OVS_REQUIRES(ofproto_mutex); +void rule_collection_unref(struct rule_collection *); +void rule_collection_destroy(struct rule_collection *); + /* Threshold at which to begin flow table eviction. Only affects the * ofproto-dpif implementation */ extern unsigned flow_eviction_threshold; @@ -287,21 +455,18 @@ rule_from_cls_rule(const struct cls_rule *cls_rule) return cls_rule ? CONTAINER_OF(cls_rule, struct rule, cr) : NULL; } -void ofproto_rule_expire(struct rule *rule, uint8_t reason); -void ofproto_rule_delete(struct ofproto *, struct classifier *cls, - struct rule *) OVS_REQ_WRLOCK(cls->rwlock); +void ofproto_rule_expire(struct rule *rule, uint8_t reason) + OVS_REQUIRES(ofproto_mutex); +void ofproto_rule_delete(struct ofproto *, struct rule *) + OVS_EXCLUDED(ofproto_mutex); void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout, uint16_t hard_timeout) - OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex); - -bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t out_port); + OVS_EXCLUDED(ofproto_mutex); void ofoperation_complete(struct ofoperation *, enum ofperr); -bool ofoperation_has_out_port(const struct ofoperation *, ofp_port_t out_port); -bool ofproto_rule_has_out_group(const struct rule *, uint32_t group_id); - -bool ofproto_rule_is_hidden(const struct rule *); +bool ofoperation_has_out_port(const struct ofoperation *, ofp_port_t out_port) + OVS_REQUIRES(ofproto_mutex); /* A group within a "struct ofproto". * @@ -316,7 +481,7 @@ struct ofgroup { * a group is ever write locked only while holding a write lock * on the container, the user's of groups will never face a group * in the write locked state. */ - struct ovs_rwlock rwlock; + struct ovs_rwlock rwlock OVS_ACQ_AFTER(ofproto_mutex); struct hmap_node hmap_node; /* In struct ofproto's "groups" hmap. */ struct ofproto *ofproto; /* The ofproto that contains this group. */ uint32_t group_id; @@ -1096,9 +1261,10 @@ struct ofproto_class { * * Rule destruction must not fail. */ struct rule *(*rule_alloc)(void); - enum ofperr (*rule_construct)(struct rule *rule); - void (*rule_insert)(struct rule *rule); - void (*rule_delete)(struct rule *rule); + enum ofperr (*rule_construct)(struct rule *rule) + /* OVS_REQUIRES(ofproto_mutex) */; + void (*rule_insert)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */; + void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */; void (*rule_destruct)(struct rule *rule); void (*rule_dealloc)(struct rule *rule); @@ -1107,7 +1273,8 @@ struct ofproto_class { * in '*byte_count'. UINT64_MAX indicates that the packet count or byte * count is unknown. */ void (*rule_get_stats)(struct rule *rule, uint64_t *packet_count, - uint64_t *byte_count); + uint64_t *byte_count) + /* OVS_EXCLUDED(ofproto_mutex) */; /* Applies the actions in 'rule' to 'packet'. (This implements sending * buffered packets for OpenFlow OFPT_FLOW_MOD commands.) @@ -1153,7 +1320,8 @@ struct ofproto_class { * * ->rule_modify_actions() should not modify any base members of struct * rule. */ - void (*rule_modify_actions)(struct rule *rule, bool reset_counters); + void (*rule_modify_actions)(struct rule *rule, bool reset_counters) + /* OVS_REQUIRES(ofproto_mutex) */; /* Changes the OpenFlow IP fragment handling policy to 'frag_handling', * which takes one of the following values, with the corresponding @@ -1533,12 +1701,15 @@ int ofproto_class_unregister(const struct ofproto_class *); enum { OFPROTO_POSTPONE = 1 << 16 }; BUILD_ASSERT_DECL(OFPROTO_POSTPONE < OFPERR_OFS); -int ofproto_flow_mod(struct ofproto *, struct ofputil_flow_mod *); +int ofproto_flow_mod(struct ofproto *, struct ofputil_flow_mod *) + OVS_EXCLUDED(ofproto_mutex); void ofproto_add_flow(struct ofproto *, const struct match *, unsigned int priority, - const struct ofpact *ofpacts, size_t ofpacts_len); + const struct ofpact *ofpacts, size_t ofpacts_len) + OVS_EXCLUDED(ofproto_mutex); bool ofproto_delete_flow(struct ofproto *, - const struct match *, unsigned int priority); + const struct match *, unsigned int priority) + OVS_EXCLUDED(ofproto_mutex); void ofproto_flush_flows(struct ofproto *); #endif /* ofproto/ofproto-provider.h */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 9605baab2..112790040 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -124,9 +124,7 @@ struct ofoperation { /* OFOPERATION_MODIFY, OFOPERATION_REPLACE: The old actions, if the actions * are changing. */ - struct ofpact *ofpacts; - size_t ofpacts_len; - uint32_t meter_id; + struct rule_actions *actions; /* OFOPERATION_DELETE. */ enum ofp_flow_removed_reason reason; /* Reason flow was removed. */ @@ -155,10 +153,9 @@ static void oftable_enable_eviction(struct oftable *, const struct mf_subfield *fields, size_t n_fields); -static void oftable_remove_rule(struct rule *rule) OVS_RELEASES(rule->rwlock); -static void oftable_remove_rule__(struct ofproto *ofproto, - struct classifier *cls, struct rule *rule) - OVS_REQ_WRLOCK(cls->rwlock) OVS_RELEASES(rule->rwlock); +static void oftable_remove_rule(struct rule *rule) OVS_REQUIRES(ofproto_mutex); +static void oftable_remove_rule__(struct ofproto *, struct rule *) + OVS_REQUIRES(ofproto_mutex); static void oftable_insert_rule(struct rule *); /* A set of rules within a single OpenFlow table (oftable) that have the same @@ -183,15 +180,60 @@ struct eviction_group { struct heap rules; /* Contains "struct rule"s. */ }; -static bool choose_rule_to_evict(struct oftable *table, struct rule **rulep) - OVS_TRY_WRLOCK(true, (*rulep)->rwlock); -static void ofproto_evict(struct ofproto *); +static bool choose_rule_to_evict(struct oftable *table, struct rule **rulep); +static void ofproto_evict(struct ofproto *) OVS_EXCLUDED(ofproto_mutex); static uint32_t rule_eviction_priority(struct rule *); static void eviction_group_add_rule(struct rule *); static void eviction_group_remove_rule(struct rule *); +/* Criteria that flow_mod and other operations use for selecting rules on + * which to operate. */ +struct rule_criteria { + /* An OpenFlow table or 255 for all tables. */ + uint8_t table_id; + + /* OpenFlow matching criteria. Interpreted different in "loose" way by + * collect_rules_loose() and "strict" way by collect_rules_strict(), as + * defined in the OpenFlow spec. */ + struct cls_rule cr; + + /* Matching criteria for the OpenFlow cookie. Consider a bit B in a rule's + * cookie and the corresponding bits C in 'cookie' and M in 'cookie_mask'. + * The rule will not be selected if M is 1 and B != C. */ + ovs_be64 cookie; + ovs_be64 cookie_mask; + + /* Selection based on actions within a rule: + * + * If out_port != OFPP_ANY, selects only rules that output to out_port. + * If out_group != OFPG_ALL, select only rules that output to out_group. */ + ofp_port_t out_port; + uint32_t out_group; +}; + +static void rule_criteria_init(struct rule_criteria *, uint8_t table_id, + const struct match *match, + unsigned int priority, + ovs_be64 cookie, ovs_be64 cookie_mask, + ofp_port_t out_port, uint32_t out_group); +static void rule_criteria_destroy(struct rule_criteria *); + +/* A packet that needs to be passed to rule_execute(). + * + * (We can't do this immediately from ofopgroup_complete() because that holds + * ofproto_mutex, which rule_execute() needs released.) */ +struct rule_execute { + struct list list_node; /* In struct ofproto's "rule_executes" list. */ + struct rule *rule; /* Owns a reference to the rule. */ + ofp_port_t in_port; + struct ofpbuf *packet; /* Owns the packet. */ +}; + +static void run_rule_executes(struct ofproto *) OVS_EXCLUDED(ofproto_mutex); +static void destroy_rule_executes(struct ofproto *); + /* ofport. */ -static void ofport_destroy__(struct ofport *); +static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex); static void ofport_destroy(struct ofport *); static void update_port(struct ofproto *, const char *devname); @@ -199,7 +241,6 @@ static int init_ports(struct ofproto *); static void reinit_ports(struct ofproto *); /* rule. */ -static void ofproto_rule_destroy(struct rule *); static void ofproto_rule_destroy__(struct rule *); static void ofproto_rule_send_removed(struct rule *, uint8_t reason); static bool rule_is_modifiable(const struct rule *); @@ -210,15 +251,17 @@ static enum ofperr add_flow(struct ofproto *, struct ofconn *, const struct ofp_header *); static enum ofperr modify_flows__(struct ofproto *, struct ofconn *, struct ofputil_flow_mod *, - const struct ofp_header *, struct list *); + const struct ofp_header *, + const struct rule_collection *); static void delete_flow__(struct rule *rule, struct ofopgroup *, enum ofp_flow_removed_reason) - OVS_RELEASES(rule->rwlock); + OVS_REQUIRES(ofproto_mutex); static enum ofperr add_group(struct ofproto *, struct ofputil_group_mod *); static bool handle_openflow(struct ofconn *, const struct ofpbuf *); static enum ofperr handle_flow_mod__(struct ofproto *, struct ofconn *, struct ofputil_flow_mod *, - const struct ofp_header *); + const struct ofp_header *) + OVS_EXCLUDED(ofproto_mutex); static void calc_duration(long long int start, long long int now, uint32_t *sec, uint32_t *nsec); @@ -237,6 +280,9 @@ static const struct ofproto_class **ofproto_classes; static size_t n_ofproto_classes; static size_t allocated_ofproto_classes; +/* Global lock that protects all flow table operations. */ +struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER; + unsigned flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT; unsigned n_handler_threads; enum ofproto_flow_miss_model flow_miss_model = OFPROTO_HANDLE_MISS_AUTO; @@ -422,6 +468,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type, } /* Initialize. */ + ovs_mutex_lock(&ofproto_mutex); memset(ofproto, 0, sizeof *ofproto); ofproto->ofproto_class = class; ofproto->name = xstrdup(datapath_name); @@ -446,12 +493,12 @@ ofproto_create(const char *datapath_name, const char *datapath_type, ofproto->n_tables = 0; hindex_init(&ofproto->cookies); list_init(&ofproto->expirable); - ovs_mutex_init_recursive(&ofproto->expirable_mutex); ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name); ofproto->state = S_OPENFLOW; list_init(&ofproto->pending); ofproto->n_pending = 0; hmap_init(&ofproto->deletions); + guarded_list_init(&ofproto->rule_executes); ofproto->n_add = ofproto->n_delete = ofproto->n_modify = 0; ofproto->first_op = ofproto->last_op = LLONG_MIN; ofproto->next_op_report = LLONG_MAX; @@ -461,6 +508,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type, ofproto->min_mtu = INT_MAX; ovs_rwlock_init(&ofproto->groups_rwlock); hmap_init(&ofproto->groups); + ovs_mutex_unlock(&ofproto_mutex); error = ofproto->ofproto_class->construct(ofproto); if (error) { @@ -1080,30 +1128,49 @@ ofproto_get_snoops(const struct ofproto *ofproto, struct sset *snoops) connmgr_get_snoops(ofproto->connmgr, snoops); } +static void +ofproto_rule_delete__(struct ofproto *ofproto, struct rule *rule) + OVS_REQUIRES(ofproto_mutex) +{ + struct ofopgroup *group; + + ovs_assert(!rule->pending); + + group = ofopgroup_create_unattached(ofproto); + delete_flow__(rule, group, OFPRR_DELETE); + ofopgroup_submit(group); +} + /* Deletes 'rule' from 'cls' within 'ofproto'. * - * The 'cls' argument is redundant (it is &ofproto->tables[rule->table_id].cls) - * but it allows Clang to do better checking. */ -static void -ofproto_delete_rule(struct ofproto *ofproto, struct classifier *cls, - struct rule *rule) - OVS_REQ_WRLOCK(cls->rwlock) + * Within an ofproto implementation, this function allows an ofproto + * implementation to destroy any rules that remain when its ->destruct() + * function is called. This function is not suitable for use elsewhere in an + * ofproto implementation. + * + * This function implements steps 4.4 and 4.5 in the section titled "Rule Life + * Cycle" in ofproto-provider.h. */ +void +ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule) + OVS_EXCLUDED(ofproto_mutex) { struct ofopgroup *group; + ovs_mutex_lock(&ofproto_mutex); ovs_assert(!rule->pending); - ovs_assert(cls == &ofproto->tables[rule->table_id].cls); group = ofopgroup_create_unattached(ofproto); ofoperation_create(group, rule, OFOPERATION_DELETE, OFPRR_DELETE); - ovs_rwlock_wrlock(&rule->rwlock); - oftable_remove_rule__(ofproto, cls, rule); + oftable_remove_rule__(ofproto, rule); ofproto->ofproto_class->rule_delete(rule); ofopgroup_submit(group); + + ovs_mutex_unlock(&ofproto_mutex); } static void ofproto_flush__(struct ofproto *ofproto) + OVS_EXCLUDED(ofproto_mutex) { struct oftable *table; @@ -1111,6 +1178,7 @@ ofproto_flush__(struct ofproto *ofproto) ofproto->ofproto_class->flush(ofproto); } + ovs_mutex_lock(&ofproto_mutex); OFPROTO_FOR_EACH_TABLE (table, ofproto) { struct rule *rule, *next_rule; struct cls_cursor cursor; @@ -1119,31 +1187,30 @@ ofproto_flush__(struct ofproto *ofproto) continue; } - ovs_rwlock_wrlock(&table->cls.rwlock); + ovs_rwlock_rdlock(&table->cls.rwlock); cls_cursor_init(&cursor, &table->cls, NULL); + ovs_rwlock_unlock(&table->cls.rwlock); CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cr, &cursor) { if (!rule->pending) { - ofproto_delete_rule(ofproto, &table->cls, rule); + ofproto_rule_delete__(ofproto, rule); } } - ovs_rwlock_unlock(&table->cls.rwlock); } + ovs_mutex_unlock(&ofproto_mutex); } static void delete_group(struct ofproto *ofproto, uint32_t group_id); static void ofproto_destroy__(struct ofproto *ofproto) + OVS_EXCLUDED(ofproto_mutex) { struct oftable *table; ovs_assert(list_is_empty(&ofproto->pending)); - ovs_assert(!ofproto->n_pending); - if (ofproto->meters) { - meter_delete(ofproto, 1, ofproto->meter_features.max_meters); - free(ofproto->meters); - } + destroy_rule_executes(ofproto); + guarded_list_destroy(&ofproto->rule_executes); delete_group(ofproto, OFPG_ALL); ovs_rwlock_destroy(&ofproto->groups_rwlock); @@ -1173,12 +1240,12 @@ ofproto_destroy__(struct ofproto *ofproto) free(ofproto->vlan_bitmap); - ovs_mutex_destroy(&ofproto->expirable_mutex); ofproto->ofproto_class->dealloc(ofproto); } void ofproto_destroy(struct ofproto *p) + OVS_EXCLUDED(ofproto_mutex) { struct ofport *ofport, *next_ofport; @@ -1186,6 +1253,13 @@ ofproto_destroy(struct ofproto *p) return; } + if (p->meters) { + meter_delete(p, 1, p->meter_features.max_meters); + p->meter_features.max_meters = 0; + free(p->meters); + p->meters = NULL; + } + ofproto_flush__(p); HMAP_FOR_EACH_SAFE (ofport, next_ofport, hmap_node, &p->ports) { ofport_destroy(ofport); @@ -1268,6 +1342,19 @@ ofproto_type_wait(const char *datapath_type) } } +static bool +any_pending_ops(const struct ofproto *p) + OVS_EXCLUDED(ofproto_mutex) +{ + bool b; + + ovs_mutex_lock(&ofproto_mutex); + b = !list_is_empty(&p->pending); + ovs_mutex_unlock(&ofproto_mutex); + + return b; +} + int ofproto_run(struct ofproto *p) { @@ -1281,6 +1368,8 @@ ofproto_run(struct ofproto *p) VLOG_ERR_RL(&rl, "%s: run failed (%s)", p->name, ovs_strerror(error)); } + run_rule_executes(p); + /* Restore the eviction group heap invariant occasionally. */ if (p->eviction_group_timer < time_msec()) { size_t i; @@ -1297,6 +1386,7 @@ ofproto_run(struct ofproto *p) continue; } + ovs_mutex_lock(&ofproto_mutex); HEAP_FOR_EACH (evg, size_node, &table->eviction_groups_by_size) { heap_rebuild(&evg->rules); } @@ -1310,6 +1400,7 @@ ofproto_run(struct ofproto *p) } } ovs_rwlock_unlock(&table->cls.rwlock); + ovs_mutex_unlock(&ofproto_mutex); } } @@ -1348,7 +1439,7 @@ ofproto_run(struct ofproto *p) case S_EVICT: connmgr_run(p->connmgr, NULL); ofproto_evict(p); - if (list_is_empty(&p->pending) && hmap_is_empty(&p->deletions)) { + if (!any_pending_ops(p)) { p->state = S_OPENFLOW; } break; @@ -1356,7 +1447,7 @@ ofproto_run(struct ofproto *p) case S_FLUSH: connmgr_run(p->connmgr, NULL); ofproto_flush__(p); - if (list_is_empty(&p->pending) && hmap_is_empty(&p->deletions)) { + if (!any_pending_ops(p)) { connmgr_flushed(p->connmgr); p->state = S_OPENFLOW; } @@ -1449,7 +1540,7 @@ ofproto_wait(struct ofproto *p) case S_EVICT: case S_FLUSH: connmgr_wait(p->connmgr, false); - if (list_is_empty(&p->pending) && hmap_is_empty(&p->deletions)) { + if (!any_pending_ops(p)) { poll_immediate_wake(); } break; @@ -1471,8 +1562,11 @@ ofproto_get_memory_usage(const struct ofproto *ofproto, struct simap *usage) unsigned int n_rules; simap_increase(usage, "ports", hmap_count(&ofproto->ports)); + + ovs_mutex_lock(&ofproto_mutex); simap_increase(usage, "ops", ofproto->n_pending + hmap_count(&ofproto->deletions)); + ovs_mutex_unlock(&ofproto_mutex); n_rules = 0; OFPROTO_FOR_EACH_TABLE (table, ofproto) { @@ -1687,6 +1781,34 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t ofp_port) return error; } +static int +simple_flow_mod(struct ofproto *ofproto, + const struct match *match, unsigned int priority, + const struct ofpact *ofpacts, size_t ofpacts_len, + enum ofp_flow_mod_command command) +{ + struct ofputil_flow_mod fm; + + memset(&fm, 0, sizeof fm); + fm.match = *match; + fm.priority = priority; + fm.cookie = 0; + fm.new_cookie = 0; + fm.modify_cookie = false; + fm.table_id = 0; + fm.command = command; + fm.idle_timeout = 0; + fm.hard_timeout = 0; + fm.buffer_id = UINT32_MAX; + fm.out_port = OFPP_ANY; + fm.out_group = OFPG_ANY; + fm.flags = 0; + fm.ofpacts = CONST_CAST(struct ofpact *, ofpacts); + fm.ofpacts_len = ofpacts_len; + + return handle_flow_mod__(ofproto, NULL, &fm, NULL); +} + /* Adds a flow to OpenFlow flow table 0 in 'p' that matches 'cls_rule' and * performs the 'n_actions' actions in 'actions'. The new flow will not * timeout. @@ -1702,25 +1824,34 @@ void ofproto_add_flow(struct ofproto *ofproto, const struct match *match, unsigned int priority, const struct ofpact *ofpacts, size_t ofpacts_len) + OVS_EXCLUDED(ofproto_mutex) { const struct rule *rule; + bool must_add; + /* First do a cheap check whether the rule we're looking for already exists + * with the actions that we want. If it does, then we're done. */ ovs_rwlock_rdlock(&ofproto->tables[0].cls.rwlock); rule = rule_from_cls_rule(classifier_find_match_exactly( &ofproto->tables[0].cls, match, priority)); + if (rule) { + ovs_mutex_lock(&rule->mutex); + must_add = !ofpacts_equal(rule->actions->ofpacts, + rule->actions->ofpacts_len, + ofpacts, ofpacts_len); + ovs_mutex_unlock(&rule->mutex); + } else { + must_add = true; + } ovs_rwlock_unlock(&ofproto->tables[0].cls.rwlock); - if (!rule || !ofpacts_equal(rule->ofpacts, rule->ofpacts_len, - ofpacts, ofpacts_len)) { - struct ofputil_flow_mod fm; - memset(&fm, 0, sizeof fm); - fm.match = *match; - fm.priority = priority; - fm.buffer_id = UINT32_MAX; - fm.ofpacts = xmemdup(ofpacts, ofpacts_len); - fm.ofpacts_len = ofpacts_len; - add_flow(ofproto, NULL, &fm, NULL); - free(fm.ofpacts); + /* If there's no such rule or the rule doesn't have the actions we want, + * fall back to a executing a full flow mod. We can't optimize this at + * all because we didn't take enough locks above to ensure that the flow + * table didn't already change beneath us. */ + if (must_add) { + simple_flow_mod(ofproto, match, priority, ofpacts, ofpacts_len, + OFPFC_MODIFY_STRICT); } } @@ -1728,9 +1859,11 @@ ofproto_add_flow(struct ofproto *ofproto, const struct match *match, * OFPERR_* OpenFlow error code on failure, or OFPROTO_POSTPONE if the * operation cannot be initiated now but may be retried later. * - * This is a helper function for in-band control and fail-open. */ + * This is a helper function for in-band control and fail-open and the "learn" + * action. */ int ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm) + OVS_EXCLUDED(ofproto_mutex) { return handle_flow_mod__(ofproto, NULL, fm, NULL); } @@ -1742,30 +1875,26 @@ ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm) bool ofproto_delete_flow(struct ofproto *ofproto, const struct match *target, unsigned int priority) + OVS_EXCLUDED(ofproto_mutex) { struct classifier *cls = &ofproto->tables[0].cls; struct rule *rule; + /* First do a cheap check whether the rule we're looking for has already + * been deleted. If so, then we're done. */ ovs_rwlock_rdlock(&cls->rwlock); rule = rule_from_cls_rule(classifier_find_match_exactly(cls, target, priority)); ovs_rwlock_unlock(&cls->rwlock); if (!rule) { - /* No such rule -> success. */ - return true; - } else if (rule->pending) { - /* An operation on the rule is already pending -> failure. - * Caller must retry later if it's important. */ - return false; - } else { - /* Initiate deletion -> success. */ - ovs_rwlock_wrlock(&cls->rwlock); - ofproto_delete_rule(ofproto, cls, rule); - ovs_rwlock_unlock(&cls->rwlock); - return true; } + /* Fall back to a executing a full flow mod. We can't optimize this at all + * because we didn't take enough locks above to ensure that the flow table + * didn't already change beneath us. */ + return simple_flow_mod(ofproto, target, priority, NULL, 0, + OFPFC_DELETE_STRICT) != OFPROTO_POSTPONE; } /* Starts the process of deleting all of the flows from all of ofproto's flow @@ -2256,61 +2385,134 @@ update_mtu(struct ofproto *p, struct ofport *port) } } -static void -ofproto_rule_destroy(struct rule *rule) +void +ofproto_rule_ref(struct rule *rule) { if (rule) { - rule->ofproto->ofproto_class->rule_destruct(rule); - ofproto_rule_destroy__(rule); + unsigned int orig; + + atomic_add(&rule->ref_count, 1, &orig); + ovs_assert(orig != 0); } } +void +ofproto_rule_unref(struct rule *rule) +{ + if (rule) { + unsigned int orig; + + atomic_sub(&rule->ref_count, 1, &orig); + if (orig == 1) { + rule->ofproto->ofproto_class->rule_destruct(rule); + ofproto_rule_destroy__(rule); + } else { + ovs_assert(orig != 0); + } + } +} + +struct rule_actions * +rule_get_actions(const struct rule *rule) + OVS_EXCLUDED(rule->mutex) +{ + struct rule_actions *actions; + + ovs_mutex_lock(&rule->mutex); + actions = rule_get_actions__(rule); + ovs_mutex_unlock(&rule->mutex); + + return actions; +} + +struct rule_actions * +rule_get_actions__(const struct rule *rule) + OVS_REQUIRES(rule->mutex) +{ + rule_actions_ref(rule->actions); + return rule->actions; +} + static void ofproto_rule_destroy__(struct rule *rule) + OVS_NO_THREAD_SAFETY_ANALYSIS { - cls_rule_destroy(&rule->cr); - free(rule->ofpacts); - ovs_mutex_destroy(&rule->timeout_mutex); - ovs_rwlock_destroy(&rule->rwlock); + cls_rule_destroy(CONST_CAST(struct cls_rule *, &rule->cr)); + rule_actions_unref(rule->actions); + ovs_mutex_destroy(&rule->mutex); rule->ofproto->ofproto_class->rule_dealloc(rule); } -/* This function allows an ofproto implementation to destroy any rules that - * remain when its ->destruct() function is called.. This function implements - * steps 4.4 and 4.5 in the section titled "Rule Life Cycle" in - * ofproto-provider.h. - * - * This function should only be called from an ofproto implementation's - * ->destruct() function. It is not suitable elsewhere. */ +/* Creates and returns a new 'struct rule_actions', with a ref_count of 1, + * whose actions are a copy of from the 'ofpacts_len' bytes of 'ofpacts'. */ +struct rule_actions * +rule_actions_create(const struct ofpact *ofpacts, size_t ofpacts_len) +{ + struct rule_actions *actions; + + actions = xmalloc(sizeof *actions); + atomic_init(&actions->ref_count, 1); + actions->ofpacts = xmemdup(ofpacts, ofpacts_len); + actions->ofpacts_len = ofpacts_len; + actions->meter_id = ofpacts_get_meter(ofpacts, ofpacts_len); + return actions; +} + +/* Increments 'actions''s ref_count. */ void -ofproto_rule_delete(struct ofproto *ofproto, struct classifier *cls, - struct rule *rule) - OVS_REQ_WRLOCK(cls->rwlock) +rule_actions_ref(struct rule_actions *actions) { - ofproto_delete_rule(ofproto, cls, rule); + if (actions) { + unsigned int orig; + + atomic_add(&actions->ref_count, 1, &orig); + ovs_assert(orig != 0); + } +} + +/* Decrements 'actions''s ref_count and frees 'actions' if the ref_count + * reaches 0. */ +void +rule_actions_unref(struct rule_actions *actions) +{ + if (actions) { + unsigned int orig; + + atomic_sub(&actions->ref_count, 1, &orig); + if (orig == 1) { + free(actions); + } else { + ovs_assert(orig != 0); + } + } } /* Returns true if 'rule' has an OpenFlow OFPAT_OUTPUT or OFPAT_ENQUEUE action * that outputs to 'port' (output to OFPP_FLOOD and OFPP_ALL doesn't count). */ -bool +static bool ofproto_rule_has_out_port(const struct rule *rule, ofp_port_t port) + OVS_REQUIRES(ofproto_mutex) { return (port == OFPP_ANY - || ofpacts_output_to_port(rule->ofpacts, rule->ofpacts_len, port)); + || ofpacts_output_to_port(rule->actions->ofpacts, + rule->actions->ofpacts_len, port)); } /* Returns true if 'rule' has group and equals group_id. */ -bool +static bool ofproto_rule_has_out_group(const struct rule *rule, uint32_t group_id) + OVS_REQUIRES(ofproto_mutex) { return (group_id == OFPG11_ANY - || ofpacts_output_to_group(rule->ofpacts, rule->ofpacts_len, group_id)); + || ofpacts_output_to_group(rule->actions->ofpacts, + rule->actions->ofpacts_len, group_id)); } /* Returns true if a rule related to 'op' has an OpenFlow OFPAT_OUTPUT or * OFPAT_ENQUEUE action that outputs to 'out_port'. */ bool ofoperation_has_out_port(const struct ofoperation *op, ofp_port_t out_port) + OVS_REQUIRES(ofproto_mutex) { if (ofproto_rule_has_out_port(op->rule, out_port)) { return true; @@ -2323,28 +2525,56 @@ ofoperation_has_out_port(const struct ofoperation *op, ofp_port_t out_port) case OFOPERATION_MODIFY: case OFOPERATION_REPLACE: - return ofpacts_output_to_port(op->ofpacts, op->ofpacts_len, out_port); + return ofpacts_output_to_port(op->actions->ofpacts, + op->actions->ofpacts_len, out_port); } NOT_REACHED(); } -/* Executes the actions indicated by 'rule' on 'packet' and credits 'rule''s - * statistics appropriately. - * - * 'packet' doesn't necessarily have to match 'rule'. 'rule' will be credited - * with statistics for 'packet' either way. - * - * Takes ownership of 'packet'. */ -static int -rule_execute(struct rule *rule, ofp_port_t in_port, struct ofpbuf *packet) +static void +rule_execute_destroy(struct rule_execute *e) { - struct flow flow; - union flow_in_port in_port_; + ofproto_rule_unref(e->rule); + list_remove(&e->list_node); + free(e); +} + +/* Executes all "rule_execute" operations queued up in ofproto->rule_executes, + * by passing them to the ofproto provider. */ +static void +run_rule_executes(struct ofproto *ofproto) + OVS_EXCLUDED(ofproto_mutex) +{ + struct rule_execute *e, *next; + struct list executes; + + guarded_list_pop_all(&ofproto->rule_executes, &executes); + LIST_FOR_EACH_SAFE (e, next, list_node, &executes) { + union flow_in_port in_port_; + struct flow flow; + + in_port_.ofp_port = e->in_port; + flow_extract(e->packet, 0, 0, NULL, &in_port_, &flow); + ofproto->ofproto_class->rule_execute(e->rule, &flow, e->packet); + + rule_execute_destroy(e); + } +} - in_port_.ofp_port = in_port; - flow_extract(packet, 0, 0, NULL, &in_port_, &flow); - return rule->ofproto->ofproto_class->rule_execute(rule, &flow, packet); +/* Destroys and discards all "rule_execute" operations queued up in + * ofproto->rule_executes. */ +static void +destroy_rule_executes(struct ofproto *ofproto) +{ + struct rule_execute *e, *next; + struct list executes; + + guarded_list_pop_all(&ofproto->rule_executes, &executes); + LIST_FOR_EACH_SAFE (e, next, list_node, &executes) { + ofpbuf_delete(e->packet); + rule_execute_destroy(e); + } } /* Returns true if 'rule' should be hidden from the controller. @@ -2352,7 +2582,7 @@ rule_execute(struct rule *rule, ofp_port_t in_port, struct ofpbuf *packet) * Rules with priority higher than UINT16_MAX are set up by ofproto itself * (e.g. by in-band control) and are intentionally hidden from the * controller. */ -bool +static bool ofproto_rule_is_hidden(const struct rule *rule) { return rule->cr.priority > UINT16_MAX; @@ -2495,30 +2725,6 @@ reject_slave_controller(struct ofconn *ofconn) } } -/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of - * 'ofpacts'. If found, returns its meter ID; if not, returns 0. - * - * This function relies on the order of 'ofpacts' being correct (as checked by - * ofpacts_verify()). */ -static uint32_t -find_meter(const struct ofpact ofpacts[], size_t ofpacts_len) -{ - const struct ofpact *a; - - OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { - enum ovs_instruction_type inst; - - inst = ovs_instruction_type_from_ofpact_type(a->type); - if (a->type == OFPACT_METER) { - return ofpact_get_METER(a)->meter_id; - } else if (inst > OVSINST_OFPIT13_METER) { - break; - } - } - - return 0; -} - /* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are appropriate * for a packet with the prerequisites satisfied by 'flow' in table 'table_id'. * 'flow' may be temporarily modified, but is restored at return. @@ -2537,7 +2743,7 @@ ofproto_check_ofpacts(struct ofproto *ofproto, return error; } - mid = find_meter(ofpacts, ofpacts_len); + mid = ofpacts_get_meter(ofpacts, ofpacts_len); if (mid && ofproto_get_provider_meter_id(ofproto, mid) == UINT32_MAX) { return OFPERR_OFPMMFC_INVALID_METER; } @@ -2711,12 +2917,12 @@ handle_table_stats_request(struct ofconn *ofconn, for (i = 0; i < p->n_tables; i++) { ots[i].table_id = i; sprintf(ots[i].name, "table%zu", i); - ots[i].match = htonll(OFPXMT12_MASK); - ots[i].wildcards = htonll(OFPXMT12_MASK); + ots[i].match = htonll(OFPXMT13_MASK); + ots[i].wildcards = htonll(OFPXMT13_MASK); ots[i].write_actions = htonl(OFPAT11_OUTPUT); ots[i].apply_actions = htonl(OFPAT11_OUTPUT); - ots[i].write_setfields = htonll(OFPXMT12_MASK); - ots[i].apply_setfields = htonll(OFPXMT12_MASK); + ots[i].write_setfields = htonll(OFPXMT13_MASK); + ots[i].apply_setfields = htonll(OFPXMT13_MASK); ots[i].metadata_match = htonll(UINT64_MAX); ots[i].metadata_write = htonll(UINT64_MAX); ots[i].instructions = htonl(OFPIT11_ALL); @@ -2832,6 +3038,7 @@ hash_cookie(ovs_be64 cookie) static void cookies_insert(struct ofproto *ofproto, struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { hindex_insert(&ofproto->cookies, &rule->cookie_node, hash_cookie(rule->flow_cookie)); @@ -2839,6 +3046,7 @@ cookies_insert(struct ofproto *ofproto, struct rule *rule) static void cookies_remove(struct ofproto *ofproto, struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { hindex_remove(&ofproto->cookies, &rule->cookie_node); } @@ -2846,13 +3054,14 @@ cookies_remove(struct ofproto *ofproto, struct rule *rule) static void ofproto_rule_change_cookie(struct ofproto *ofproto, struct rule *rule, ovs_be64 new_cookie) + OVS_REQUIRES(ofproto_mutex) { if (new_cookie != rule->flow_cookie) { cookies_remove(ofproto, rule); - ovs_rwlock_wrlock(&rule->rwlock); + ovs_mutex_lock(&rule->mutex); rule->flow_cookie = new_cookie; - ovs_rwlock_unlock(&rule->rwlock); + ovs_mutex_unlock(&rule->mutex); cookies_insert(ofproto, rule); } @@ -2935,169 +3144,229 @@ next_matching_table(const struct ofproto *ofproto, (TABLE) != NULL; \ (TABLE) = next_matching_table(OFPROTO, TABLE, TABLE_ID)) -/* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if - * 'table_id' is 0xff) that match 'match' in the "loose" way required for - * OpenFlow OFPFC_MODIFY and OFPFC_DELETE requests and puts them on list - * 'rules'. +/* Initializes 'criteria' in a straightforward way based on the other + * parameters. * - * If 'out_port' is anything other than OFPP_ANY, then only rules that output - * to 'out_port' are included. + * For "loose" matching, the 'priority' parameter is unimportant and may be + * supplied as 0. */ +static void +rule_criteria_init(struct rule_criteria *criteria, uint8_t table_id, + const struct match *match, unsigned int priority, + ovs_be64 cookie, ovs_be64 cookie_mask, + ofp_port_t out_port, uint32_t out_group) +{ + criteria->table_id = table_id; + cls_rule_init(&criteria->cr, match, priority); + criteria->cookie = cookie; + criteria->cookie_mask = cookie_mask; + criteria->out_port = out_port; + criteria->out_group = out_group; +} + +static void +rule_criteria_destroy(struct rule_criteria *criteria) +{ + cls_rule_destroy(&criteria->cr); +} + +void +rule_collection_init(struct rule_collection *rules) +{ + rules->rules = rules->stub; + rules->n = 0; + rules->capacity = ARRAY_SIZE(rules->stub); +} + +void +rule_collection_add(struct rule_collection *rules, struct rule *rule) +{ + if (rules->n >= rules->capacity) { + size_t old_size, new_size; + + old_size = rules->capacity * sizeof *rules->rules; + rules->capacity *= 2; + new_size = rules->capacity * sizeof *rules->rules; + + if (rules->rules == rules->stub) { + rules->rules = xmalloc(new_size); + memcpy(rules->rules, rules->stub, old_size); + } else { + rules->rules = xrealloc(rules->rules, new_size); + } + } + + rules->rules[rules->n++] = rule; +} + +void +rule_collection_ref(struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) +{ + size_t i; + + for (i = 0; i < rules->n; i++) { + ofproto_rule_ref(rules->rules[i]); + } +} + +void +rule_collection_unref(struct rule_collection *rules) +{ + size_t i; + + for (i = 0; i < rules->n; i++) { + ofproto_rule_unref(rules->rules[i]); + } +} + +void +rule_collection_destroy(struct rule_collection *rules) +{ + if (rules->rules != rules->stub) { + free(rules->rules); + } +} + +static enum ofperr +collect_rule(struct rule *rule, const struct rule_criteria *c, + struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) +{ + if (ofproto_rule_is_hidden(rule)) { + return 0; + } else if (rule->pending) { + return OFPROTO_POSTPONE; + } else { + if ((c->table_id == rule->table_id || c->table_id == 0xff) + && ofproto_rule_has_out_port(rule, c->out_port) + && ofproto_rule_has_out_group(rule, c->out_group) + && !((rule->flow_cookie ^ c->cookie) & c->cookie_mask)) { + rule_collection_add(rules, rule); + } + return 0; + } +} + +/* Searches 'ofproto' for rules that match the criteria in 'criteria'. Matches + * on classifiers rules are done in the "loose" way required for OpenFlow + * OFPFC_MODIFY and OFPFC_DELETE requests. Puts the selected rules on list + * 'rules'. * * Hidden rules are always omitted. * * Returns 0 on success, otherwise an OpenFlow error code. */ static enum ofperr -collect_rules_loose(struct ofproto *ofproto, uint8_t table_id, - const struct match *match, - ovs_be64 cookie, ovs_be64 cookie_mask, - ofp_port_t out_port, uint32_t out_group, - struct list *rules) +collect_rules_loose(struct ofproto *ofproto, + const struct rule_criteria *criteria, + struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) { struct oftable *table; - struct cls_rule cr; enum ofperr error; - error = check_table_id(ofproto, table_id); + rule_collection_init(rules); + + error = check_table_id(ofproto, criteria->table_id); if (error) { - return error; + goto exit; } - list_init(rules); - cls_rule_init(&cr, match, 0); - - if (cookie_mask == htonll(UINT64_MAX)) { + if (criteria->cookie_mask == htonll(UINT64_MAX)) { struct rule *rule; - HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie), + HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, + hash_cookie(criteria->cookie), &ofproto->cookies) { - if (table_id != rule->table_id && table_id != 0xff) { - continue; - } - if (ofproto_rule_is_hidden(rule)) { - continue; - } - if (cls_rule_is_loose_match(&rule->cr, &cr.match)) { - if (rule->pending) { - error = OFPROTO_POSTPONE; - goto exit; - } - if (rule->flow_cookie == cookie /* Hash collisions possible. */ - && ofproto_rule_has_out_port(rule, out_port) - && ofproto_rule_has_out_group(rule, out_group)) { - list_push_back(rules, &rule->ofproto_node); + if (cls_rule_is_loose_match(&rule->cr, &criteria->cr.match)) { + error = collect_rule(rule, criteria, rules); + if (error) { + break; } } } - goto exit; - } - - FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) { - struct cls_cursor cursor; - struct rule *rule; + } else { + FOR_EACH_MATCHING_TABLE (table, criteria->table_id, ofproto) { + struct cls_cursor cursor; + struct rule *rule; - ovs_rwlock_rdlock(&table->cls.rwlock); - cls_cursor_init(&cursor, &table->cls, &cr); - CLS_CURSOR_FOR_EACH (rule, cr, &cursor) { - if (rule->pending) { - ovs_rwlock_unlock(&table->cls.rwlock); - error = OFPROTO_POSTPONE; - goto exit; - } - if (!ofproto_rule_is_hidden(rule) - && ofproto_rule_has_out_port(rule, out_port) - && ofproto_rule_has_out_group(rule, out_group) - && !((rule->flow_cookie ^ cookie) & cookie_mask)) { - list_push_back(rules, &rule->ofproto_node); + ovs_rwlock_rdlock(&table->cls.rwlock); + cls_cursor_init(&cursor, &table->cls, &criteria->cr); + CLS_CURSOR_FOR_EACH (rule, cr, &cursor) { + error = collect_rule(rule, criteria, rules); + if (error) { + break; + } } + ovs_rwlock_unlock(&table->cls.rwlock); } - ovs_rwlock_unlock(&table->cls.rwlock); } exit: - cls_rule_destroy(&cr); + if (error) { + rule_collection_destroy(rules); + } return error; } -/* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if - * 'table_id' is 0xff) that match 'match' in the "strict" way required for - * OpenFlow OFPFC_MODIFY_STRICT and OFPFC_DELETE_STRICT requests and puts them - * on list 'rules'. - * - * If 'out_port' is anything other than OFPP_ANY, then only rules that output - * to 'out_port' are included. +/* Searches 'ofproto' for rules that match the criteria in 'criteria'. Matches + * on classifiers rules are done in the "strict" way required for OpenFlow + * OFPFC_MODIFY_STRICT and OFPFC_DELETE_STRICT requests. Puts the selected + * rules on list 'rules'. * * Hidden rules are always omitted. * * Returns 0 on success, otherwise an OpenFlow error code. */ static enum ofperr -collect_rules_strict(struct ofproto *ofproto, uint8_t table_id, - const struct match *match, unsigned int priority, - ovs_be64 cookie, ovs_be64 cookie_mask, - ofp_port_t out_port, uint32_t out_group, - struct list *rules) +collect_rules_strict(struct ofproto *ofproto, + const struct rule_criteria *criteria, + struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) { struct oftable *table; - struct cls_rule cr; int error; - error = check_table_id(ofproto, table_id); + rule_collection_init(rules); + + error = check_table_id(ofproto, criteria->table_id); if (error) { - return error; + goto exit; } - list_init(rules); - cls_rule_init(&cr, match, priority); - - if (cookie_mask == htonll(UINT64_MAX)) { + if (criteria->cookie_mask == htonll(UINT64_MAX)) { struct rule *rule; - HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie), + HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, + hash_cookie(criteria->cookie), &ofproto->cookies) { - if (table_id != rule->table_id && table_id != 0xff) { - continue; - } - if (ofproto_rule_is_hidden(rule)) { - continue; - } - if (cls_rule_equal(&rule->cr, &cr)) { - if (rule->pending) { - error = OFPROTO_POSTPONE; - goto exit; - } - if (rule->flow_cookie == cookie /* Hash collisions possible. */ - && ofproto_rule_has_out_port(rule, out_port) - && ofproto_rule_has_out_group(rule, out_group)) { - list_push_back(rules, &rule->ofproto_node); + if (cls_rule_equal(&rule->cr, &criteria->cr)) { + error = collect_rule(rule, criteria, rules); + if (error) { + break; } } } - goto exit; - } - - FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) { - struct rule *rule; + } else { + FOR_EACH_MATCHING_TABLE (table, criteria->table_id, ofproto) { + struct rule *rule; - ovs_rwlock_rdlock(&table->cls.rwlock); - rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls, - &cr)); - ovs_rwlock_unlock(&table->cls.rwlock); - if (rule) { - if (rule->pending) { - error = OFPROTO_POSTPONE; - goto exit; - } - if (!ofproto_rule_is_hidden(rule) - && ofproto_rule_has_out_port(rule, out_port) - && ofproto_rule_has_out_group(rule, out_group) - && !((rule->flow_cookie ^ cookie) & cookie_mask)) { - list_push_back(rules, &rule->ofproto_node); + ovs_rwlock_rdlock(&table->cls.rwlock); + rule = rule_from_cls_rule(classifier_find_rule_exactly( + &table->cls, &criteria->cr)); + ovs_rwlock_unlock(&table->cls.rwlock); + if (rule) { + error = collect_rule(rule, criteria, rules); + if (error) { + break; + } } } } exit: - cls_rule_destroy(&cr); - return 0; + if (error) { + rule_collection_destroy(rules); + } + return error; } /* Returns 'age_ms' (a duration in milliseconds), converted to seconds and @@ -3113,52 +3382,76 @@ age_secs(long long int age_ms) static enum ofperr handle_flow_stats_request(struct ofconn *ofconn, const struct ofp_header *request) + OVS_EXCLUDED(ofproto_mutex) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); struct ofputil_flow_stats_request fsr; + struct rule_criteria criteria; + struct rule_collection rules; struct list replies; - struct list rules; - struct rule *rule; enum ofperr error; + size_t i; error = ofputil_decode_flow_stats_request(&fsr, request); if (error) { return error; } - error = collect_rules_loose(ofproto, fsr.table_id, &fsr.match, - fsr.cookie, fsr.cookie_mask, - fsr.out_port, fsr.out_group, &rules); + rule_criteria_init(&criteria, fsr.table_id, &fsr.match, 0, fsr.cookie, + fsr.cookie_mask, fsr.out_port, fsr.out_group); + + ovs_mutex_lock(&ofproto_mutex); + error = collect_rules_loose(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + if (!error) { + rule_collection_ref(&rules); + } + ovs_mutex_unlock(&ofproto_mutex); + if (error) { return error; } ofpmp_init(&replies, request); - LIST_FOR_EACH (rule, ofproto_node, &rules) { + for (i = 0; i < rules.n; i++) { + struct rule *rule = rules.rules[i]; long long int now = time_msec(); struct ofputil_flow_stats fs; + long long int created, used, modified; + struct rule_actions *actions; + enum ofputil_flow_mod_flags flags; - minimatch_expand(&rule->cr.match, &fs.match); - fs.priority = rule->cr.priority; + ovs_mutex_lock(&rule->mutex); fs.cookie = rule->flow_cookie; - fs.table_id = rule->table_id; - calc_duration(rule->created, now, &fs.duration_sec, &fs.duration_nsec); - fs.idle_age = age_secs(now - rule->used); - fs.hard_age = age_secs(now - rule->modified); - ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count, - &fs.byte_count); - fs.ofpacts = rule->ofpacts; - fs.ofpacts_len = rule->ofpacts_len; - - ovs_mutex_lock(&rule->timeout_mutex); fs.idle_timeout = rule->idle_timeout; fs.hard_timeout = rule->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + created = rule->created; + used = rule->used; + modified = rule->modified; + actions = rule_get_actions__(rule); + flags = rule->flags; + ovs_mutex_unlock(&rule->mutex); - fs.flags = rule->flags; + minimatch_expand(&rule->cr.match, &fs.match); + fs.table_id = rule->table_id; + calc_duration(created, now, &fs.duration_sec, &fs.duration_nsec); + fs.priority = rule->cr.priority; + fs.idle_age = age_secs(now - used); + fs.hard_age = age_secs(now - modified); + ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count, + &fs.byte_count); + fs.ofpacts = actions->ofpacts; + fs.ofpacts_len = actions->ofpacts_len; + fs.flags = flags; ofputil_append_flow_stats_reply(&fs, &replies); + + rule_actions_unref(actions); } + + rule_collection_unref(&rules); + rule_collection_destroy(&rules); + ofconn_send_replies(ofconn, &replies); return 0; @@ -3168,22 +3461,32 @@ static void flow_stats_ds(struct rule *rule, struct ds *results) { uint64_t packet_count, byte_count; + struct rule_actions *actions; + long long int created; rule->ofproto->ofproto_class->rule_get_stats(rule, &packet_count, &byte_count); + ovs_mutex_lock(&rule->mutex); + actions = rule_get_actions__(rule); + created = rule->created; + ovs_mutex_unlock(&rule->mutex); + if (rule->table_id != 0) { ds_put_format(results, "table_id=%"PRIu8", ", rule->table_id); } - ds_put_format(results, "duration=%llds, ", - (time_msec() - rule->created) / 1000); + ds_put_format(results, "duration=%llds, ", (time_msec() - created) / 1000); ds_put_format(results, "priority=%u, ", rule->cr.priority); ds_put_format(results, "n_packets=%"PRIu64", ", packet_count); ds_put_format(results, "n_bytes=%"PRIu64", ", byte_count); cls_rule_format(&rule->cr, results); ds_put_char(results, ','); - ofpacts_format(rule->ofpacts, rule->ofpacts_len, results); + + ofpacts_format(actions->ofpacts, actions->ofpacts_len, results); + ds_put_cstr(results, "\n"); + + rule_actions_unref(actions); } /* Adds a pretty-printed description of all flows to 'results', including @@ -3234,31 +3537,43 @@ ofproto_port_get_cfm_status(const struct ofproto *ofproto, ofp_port_t ofp_port, static enum ofperr handle_aggregate_stats_request(struct ofconn *ofconn, const struct ofp_header *oh) + OVS_EXCLUDED(ofproto_mutex) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); struct ofputil_flow_stats_request request; struct ofputil_aggregate_stats stats; bool unknown_packets, unknown_bytes; + struct rule_criteria criteria; + struct rule_collection rules; struct ofpbuf *reply; - struct list rules; - struct rule *rule; enum ofperr error; + size_t i; error = ofputil_decode_flow_stats_request(&request, oh); if (error) { return error; } - error = collect_rules_loose(ofproto, request.table_id, &request.match, - request.cookie, request.cookie_mask, - request.out_port, request.out_group, &rules); + rule_criteria_init(&criteria, request.table_id, &request.match, 0, + request.cookie, request.cookie_mask, + request.out_port, request.out_group); + + ovs_mutex_lock(&ofproto_mutex); + error = collect_rules_loose(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + if (!error) { + rule_collection_ref(&rules); + } + ovs_mutex_unlock(&ofproto_mutex); + if (error) { return error; } memset(&stats, 0, sizeof stats); unknown_packets = unknown_bytes = false; - LIST_FOR_EACH (rule, ofproto_node, &rules) { + for (i = 0; i < rules.n; i++) { + struct rule *rule = rules.rules[i]; uint64_t packet_count; uint64_t byte_count; @@ -3286,6 +3601,9 @@ handle_aggregate_stats_request(struct ofconn *ofconn, stats.byte_count = UINT64_MAX; } + rule_collection_unref(&rules); + rule_collection_destroy(&rules); + reply = ofputil_encode_aggregate_stats_reply(&stats, oh); ofconn_send_reply(ofconn, reply); @@ -3394,6 +3712,7 @@ static bool is_flow_deletion_pending(const struct ofproto *ofproto, const struct cls_rule *cls_rule, uint8_t table_id) + OVS_REQUIRES(ofproto_mutex) { if (!hmap_is_empty(&ofproto->deletions)) { struct ofoperation *op; @@ -3410,32 +3729,34 @@ is_flow_deletion_pending(const struct ofproto *ofproto, return false; } -static enum ofperr -evict_rule_from_table(struct ofproto *ofproto, struct oftable *table) +static bool +should_evict_a_rule(struct oftable *table, unsigned int extra_space) + OVS_REQUIRES(ofproto_mutex) + OVS_NO_THREAD_SAFETY_ANALYSIS { - struct rule *rule; - size_t n_rules; - - ovs_rwlock_rdlock(&table->cls.rwlock); - n_rules = classifier_count(&table->cls); - ovs_rwlock_unlock(&table->cls.rwlock); - - if (n_rules < table->max_flows) { - return 0; - } else if (!choose_rule_to_evict(table, &rule)) { - return OFPERR_OFPFMFC_TABLE_FULL; - } else if (rule->pending) { - ovs_rwlock_unlock(&rule->rwlock); - return OFPROTO_POSTPONE; - } else { - struct ofopgroup *group; + return classifier_count(&table->cls) + extra_space > table->max_flows; +} - group = ofopgroup_create_unattached(ofproto); - delete_flow__(rule, group, OFPRR_EVICTION); - ofopgroup_submit(group); +static enum ofperr +evict_rules_from_table(struct ofproto *ofproto, struct oftable *table, + unsigned int extra_space) + OVS_REQUIRES(ofproto_mutex) +{ + while (should_evict_a_rule(table, extra_space)) { + struct rule *rule; - return 0; + if (!choose_rule_to_evict(table, &rule)) { + return OFPERR_OFPFMFC_TABLE_FULL; + } else if (rule->pending) { + return OFPROTO_POSTPONE; + } else { + struct ofopgroup *group = ofopgroup_create_unattached(ofproto); + delete_flow__(rule, group, OFPRR_EVICTION); + ofopgroup_submit(group); + } } + + return 0; } /* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT @@ -3446,14 +3767,14 @@ evict_rule_from_table(struct ofproto *ofproto, struct oftable *table) * error code on failure, or OFPROTO_POSTPONE if the operation cannot be * initiated now but may be retried later. * - * Upon successful return, takes ownership of 'fm->ofpacts'. On failure, - * ownership remains with the caller. + * The caller retains ownership of 'fm->ofpacts'. * * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id, * if any. */ static enum ofperr add_flow(struct ofproto *ofproto, struct ofconn *ofconn, struct ofputil_flow_mod *fm, const struct ofp_header *request) + OVS_REQUIRES(ofproto_mutex) { struct oftable *table; struct ofopgroup *group; @@ -3505,12 +3826,15 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, } else if (rule->pending) { return OFPROTO_POSTPONE; } else { - struct list rules; + struct rule_collection rules; - list_init(&rules); - list_push_back(&rules, &rule->ofproto_node); + rule_collection_init(&rules); + rule_collection_add(&rules, rule); fm->modify_cookie = true; - return modify_flows__(ofproto, ofconn, fm, request, &rules); + error = modify_flows__(ofproto, ofconn, fm, request, &rules); + rule_collection_destroy(&rules); + + return error; } } @@ -3543,7 +3867,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, } /* If necessary, evict an existing rule to clear out space. */ - error = evict_rule_from_table(ofproto, table); + error = evict_rules_from_table(ofproto, table, 1); if (error) { cls_rule_destroy(&cr); return error; @@ -3559,31 +3883,28 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, } /* Initialize base state. */ - rule->ofproto = ofproto; - cls_rule_move(&rule->cr, &cr); + *CONST_CAST(struct ofproto **, &rule->ofproto) = ofproto; + cls_rule_move(CONST_CAST(struct cls_rule *, &rule->cr), &cr); + atomic_init(&rule->ref_count, 1); rule->pending = NULL; rule->flow_cookie = fm->new_cookie; rule->created = rule->modified = rule->used = time_msec(); - ovs_mutex_init(&rule->timeout_mutex); - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_init(&rule->mutex); + ovs_mutex_lock(&rule->mutex); rule->idle_timeout = fm->idle_timeout; rule->hard_timeout = fm->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); - rule->table_id = table - ofproto->tables; + *CONST_CAST(uint8_t *, &rule->table_id) = table - ofproto->tables; rule->flags = fm->flags & OFPUTIL_FF_STATE; - - rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len); - rule->ofpacts_len = fm->ofpacts_len; - rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len); + rule->actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len); list_init(&rule->meter_list_node); rule->eviction_group = NULL; list_init(&rule->expirable); rule->monitor_flags = 0; rule->add_seqno = 0; rule->modify_seqno = 0; - ovs_rwlock_init(&rule->rwlock); /* Construct rule, initializing derived state. */ error = ofproto->ofproto_class->rule_construct(rule); @@ -3615,17 +3936,19 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, static enum ofperr modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn, struct ofputil_flow_mod *fm, const struct ofp_header *request, - struct list *rules) + const struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) { enum ofoperation_type type; struct ofopgroup *group; - struct rule *rule; enum ofperr error; + size_t i; type = fm->command == OFPFC_ADD ? OFOPERATION_REPLACE : OFOPERATION_MODIFY; group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id); error = OFPERR_OFPBRC_EPERM; - LIST_FOR_EACH (rule, ofproto_node, rules) { + for (i = 0; i < rules->n; i++) { + struct rule *rule = rules->rules[i]; struct ofoperation *op; bool actions_changed; bool reset_counters; @@ -3647,7 +3970,8 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn, } actions_changed = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len, - rule->ofpacts, rule->ofpacts_len); + rule->actions->ofpacts, + rule->actions->ofpacts_len); op = ofoperation_create(group, rule, type, 0); @@ -3655,10 +3979,10 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn, ofproto_rule_change_cookie(ofproto, rule, fm->new_cookie); } if (type == OFOPERATION_REPLACE) { - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); rule->idle_timeout = fm->idle_timeout; rule->hard_timeout = fm->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); rule->flags = fm->flags & OFPUTIL_FF_STATE; if (fm->idle_timeout || fm->hard_timeout) { @@ -3672,16 +3996,15 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn, reset_counters = (fm->flags & OFPUTIL_FF_RESET_COUNTS) != 0; if (actions_changed || reset_counters) { - op->ofpacts = rule->ofpacts; - op->ofpacts_len = rule->ofpacts_len; - op->meter_id = rule->meter_id; + struct rule_actions *new_actions; - ovs_rwlock_wrlock(&rule->rwlock); - rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len); - rule->ofpacts_len = fm->ofpacts_len; - ovs_rwlock_unlock(&rule->rwlock); + op->actions = rule->actions; + new_actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len); + + ovs_mutex_lock(&rule->mutex); + rule->actions = new_actions; + ovs_mutex_unlock(&rule->mutex); - rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len); rule->ofproto->ofproto_class->rule_modify_actions(rule, reset_counters); } else { @@ -3696,6 +4019,7 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn, static enum ofperr modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn, struct ofputil_flow_mod *fm, const struct ofp_header *request) + OVS_REQUIRES(ofproto_mutex) { if (fm->cookie_mask != htonll(0) || fm->new_cookie == htonll(UINT64_MAX)) { return 0; @@ -3712,20 +4036,26 @@ static enum ofperr modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn, struct ofputil_flow_mod *fm, const struct ofp_header *request) + OVS_REQUIRES(ofproto_mutex) { - struct list rules; + struct rule_criteria criteria; + struct rule_collection rules; int error; - error = collect_rules_loose(ofproto, fm->table_id, &fm->match, - fm->cookie, fm->cookie_mask, - OFPP_ANY, OFPG11_ANY, &rules); - if (error) { - return error; - } else if (list_is_empty(&rules)) { - return modify_flows_add(ofproto, ofconn, fm, request); - } else { - return modify_flows__(ofproto, ofconn, fm, request, &rules); + rule_criteria_init(&criteria, fm->table_id, &fm->match, 0, + fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY); + error = collect_rules_loose(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + + if (!error) { + error = (rules.n > 0 + ? modify_flows__(ofproto, ofconn, fm, request, &rules) + : modify_flows_add(ofproto, ofconn, fm, request)); } + + rule_collection_destroy(&rules); + + return error; } /* Implements OFPFC_MODIFY_STRICT. Returns 0 on success or an OpenFlow error @@ -3737,22 +4067,28 @@ static enum ofperr modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn, struct ofputil_flow_mod *fm, const struct ofp_header *request) + OVS_REQUIRES(ofproto_mutex) { - struct list rules; + struct rule_criteria criteria; + struct rule_collection rules; int error; - error = collect_rules_strict(ofproto, fm->table_id, &fm->match, - fm->priority, fm->cookie, fm->cookie_mask, - OFPP_ANY, OFPG11_ANY, &rules); - if (error) { - return error; - } else if (list_is_empty(&rules)) { - return modify_flows_add(ofproto, ofconn, fm, request); - } else { - return list_is_singleton(&rules) ? modify_flows__(ofproto, ofconn, - fm, request, &rules) - : 0; + rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority, + fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY); + error = collect_rules_strict(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + + if (!error) { + if (rules.n == 0) { + error = modify_flows_add(ofproto, ofconn, fm, request); + } else if (rules.n == 1) { + error = modify_flows__(ofproto, ofconn, fm, request, &rules); + } } + + rule_collection_destroy(&rules); + + return error; } /* OFPFC_DELETE implementation. */ @@ -3760,6 +4096,7 @@ modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn, static void delete_flow__(struct rule *rule, struct ofopgroup *group, enum ofp_flow_removed_reason reason) + OVS_REQUIRES(ofproto_mutex) { struct ofproto *ofproto = rule->ofproto; @@ -3775,16 +4112,17 @@ delete_flow__(struct rule *rule, struct ofopgroup *group, * Returns 0 on success, otherwise an OpenFlow error code. */ static enum ofperr delete_flows__(struct ofproto *ofproto, struct ofconn *ofconn, - const struct ofp_header *request, struct list *rules, + const struct ofp_header *request, + const struct rule_collection *rules, enum ofp_flow_removed_reason reason) + OVS_REQUIRES(ofproto_mutex) { - struct rule *rule, *next; struct ofopgroup *group; + size_t i; group = ofopgroup_create(ofproto, ofconn, request, UINT32_MAX); - LIST_FOR_EACH_SAFE (rule, next, ofproto_node, rules) { - ovs_rwlock_wrlock(&rule->rwlock); - delete_flow__(rule, group, reason); + for (i = 0; i < rules->n; i++) { + delete_flow__(rules->rules[i], group, reason); } ofopgroup_submit(group); @@ -3796,17 +4134,24 @@ static enum ofperr delete_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn, const struct ofputil_flow_mod *fm, const struct ofp_header *request) + OVS_REQUIRES(ofproto_mutex) { - struct list rules; + struct rule_criteria criteria; + struct rule_collection rules; enum ofperr error; - error = collect_rules_loose(ofproto, fm->table_id, &fm->match, - fm->cookie, fm->cookie_mask, - fm->out_port, fm->out_group, &rules); - return (error ? error - : !list_is_empty(&rules) ? delete_flows__(ofproto, ofconn, request, - &rules, OFPRR_DELETE) - : 0); + rule_criteria_init(&criteria, fm->table_id, &fm->match, 0, + fm->cookie, fm->cookie_mask, + fm->out_port, fm->out_group); + error = collect_rules_loose(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + + if (!error && rules.n > 0) { + error = delete_flows__(ofproto, ofconn, request, &rules, OFPRR_DELETE); + } + rule_collection_destroy(&rules); + + return error; } /* Implements OFPFC_DELETE_STRICT. */ @@ -3814,22 +4159,29 @@ static enum ofperr delete_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn, const struct ofputil_flow_mod *fm, const struct ofp_header *request) + OVS_REQUIRES(ofproto_mutex) { - struct list rules; + struct rule_criteria criteria; + struct rule_collection rules; enum ofperr error; - error = collect_rules_strict(ofproto, fm->table_id, &fm->match, - fm->priority, fm->cookie, fm->cookie_mask, - fm->out_port, fm->out_group, &rules); - return (error ? error - : list_is_singleton(&rules) ? delete_flows__(ofproto, ofconn, - request, &rules, - OFPRR_DELETE) - : 0); + rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority, + fm->cookie, fm->cookie_mask, + fm->out_port, fm->out_group); + error = collect_rules_strict(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + + if (!error && rules.n > 0) { + error = delete_flows__(ofproto, ofconn, request, &rules, OFPRR_DELETE); + } + rule_collection_destroy(&rules); + + return error; } static void ofproto_rule_send_removed(struct rule *rule, uint8_t reason) + OVS_REQUIRES(ofproto_mutex) { struct ofputil_flow_removed fr; @@ -3845,10 +4197,10 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t reason) fr.table_id = rule->table_id; calc_duration(rule->created, time_msec(), &fr.duration_sec, &fr.duration_nsec); - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); fr.idle_timeout = rule->idle_timeout; fr.hard_timeout = rule->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count, &fr.byte_count); @@ -3866,17 +4218,15 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t reason) * OpenFlow flows. */ void ofproto_rule_expire(struct rule *rule, uint8_t reason) + OVS_REQUIRES(ofproto_mutex) { struct ofproto *ofproto = rule->ofproto; - struct classifier *cls = &ofproto->tables[rule->table_id].cls; ovs_assert(reason == OFPRR_HARD_TIMEOUT || reason == OFPRR_IDLE_TIMEOUT || reason == OFPRR_DELETE || reason == OFPRR_GROUP_DELETE); - ofproto_rule_send_removed(rule, reason); - ovs_rwlock_wrlock(&cls->rwlock); - ofproto_delete_rule(ofproto, cls, rule); - ovs_rwlock_unlock(&cls->rwlock); + ofproto_rule_send_removed(rule, reason); + ofproto_rule_delete__(ofproto, rule); } /* Reduces '*timeout' to no more than 'max'. A value of zero in either case @@ -3897,26 +4247,27 @@ reduce_timeout(uint16_t max, uint16_t *timeout) void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout, uint16_t hard_timeout) - OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex) + OVS_EXCLUDED(ofproto_mutex, rule->mutex) { if (!idle_timeout && !hard_timeout) { return; } - ovs_mutex_lock(&rule->ofproto->expirable_mutex); + ovs_mutex_lock(&ofproto_mutex); if (list_is_empty(&rule->expirable)) { list_insert(&rule->ofproto->expirable, &rule->expirable); } - ovs_mutex_unlock(&rule->ofproto->expirable_mutex); + ovs_mutex_unlock(&ofproto_mutex); - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); reduce_timeout(idle_timeout, &rule->idle_timeout); reduce_timeout(hard_timeout, &rule->hard_timeout); - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); } static enum ofperr handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh) + OVS_EXCLUDED(ofproto_mutex) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); struct ofputil_flow_mod fm; @@ -3975,36 +4326,50 @@ exit: static enum ofperr handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn, struct ofputil_flow_mod *fm, const struct ofp_header *oh) + OVS_EXCLUDED(ofproto_mutex) { - if (ofproto->n_pending >= 50) { - ovs_assert(!list_is_empty(&ofproto->pending)); - return OFPROTO_POSTPONE; - } + enum ofperr error; - switch (fm->command) { - case OFPFC_ADD: - return add_flow(ofproto, ofconn, fm, oh); + ovs_mutex_lock(&ofproto_mutex); + if (ofproto->n_pending < 50) { + switch (fm->command) { + case OFPFC_ADD: + error = add_flow(ofproto, ofconn, fm, oh); + break; - case OFPFC_MODIFY: - return modify_flows_loose(ofproto, ofconn, fm, oh); + case OFPFC_MODIFY: + error = modify_flows_loose(ofproto, ofconn, fm, oh); + break; - case OFPFC_MODIFY_STRICT: - return modify_flow_strict(ofproto, ofconn, fm, oh); + case OFPFC_MODIFY_STRICT: + error = modify_flow_strict(ofproto, ofconn, fm, oh); + break; - case OFPFC_DELETE: - return delete_flows_loose(ofproto, ofconn, fm, oh); + case OFPFC_DELETE: + error = delete_flows_loose(ofproto, ofconn, fm, oh); + break; - case OFPFC_DELETE_STRICT: - return delete_flow_strict(ofproto, ofconn, fm, oh); + case OFPFC_DELETE_STRICT: + error = delete_flow_strict(ofproto, ofconn, fm, oh); + break; - default: - if (fm->command > 0xff) { - VLOG_WARN_RL(&rl, "%s: flow_mod has explicit table_id but " - "flow_mod_table_id extension is not enabled", - ofproto->name); + default: + if (fm->command > 0xff) { + VLOG_WARN_RL(&rl, "%s: flow_mod has explicit table_id but " + "flow_mod_table_id extension is not enabled", + ofproto->name); + } + error = OFPERR_OFPFMFC_BAD_COMMAND; + break; } - return OFPERR_OFPFMFC_BAD_COMMAND; + } else { + ovs_assert(!list_is_empty(&ofproto->pending)); + error = OFPROTO_POSTPONE; } + ovs_mutex_unlock(&ofproto_mutex); + + run_rule_executes(ofproto); + return error; } static enum ofperr @@ -4185,8 +4550,10 @@ static void ofproto_compose_flow_refresh_update(const struct rule *rule, enum nx_flow_monitor_flags flags, struct list *msgs) + OVS_REQUIRES(ofproto_mutex) { struct ofoperation *op = rule->pending; + const struct rule_actions *actions; struct ofputil_flow_update fu; struct match match; @@ -4199,21 +4566,20 @@ ofproto_compose_flow_refresh_update(const struct rule *rule, fu.event = (flags & (NXFMF_INITIAL | NXFMF_ADD) ? NXFME_ADDED : NXFME_MODIFIED); fu.reason = 0; - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); fu.idle_timeout = rule->idle_timeout; fu.hard_timeout = rule->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); fu.table_id = rule->table_id; fu.cookie = rule->flow_cookie; minimatch_expand(&rule->cr.match, &match); fu.match = &match; fu.priority = rule->cr.priority; + if (!(flags & NXFMF_ACTIONS)) { - fu.ofpacts = NULL; - fu.ofpacts_len = 0; + actions = NULL; } else if (!op) { - fu.ofpacts = rule->ofpacts; - fu.ofpacts_len = rule->ofpacts_len; + actions = rule->actions; } else { /* An operation is in progress. Use the previous version of the flow's * actions, so that when the operation commits we report the change. */ @@ -4223,24 +4589,19 @@ ofproto_compose_flow_refresh_update(const struct rule *rule, case OFOPERATION_MODIFY: case OFOPERATION_REPLACE: - if (op->ofpacts) { - fu.ofpacts = op->ofpacts; - fu.ofpacts_len = op->ofpacts_len; - } else { - fu.ofpacts = rule->ofpacts; - fu.ofpacts_len = rule->ofpacts_len; - } + actions = op->actions ? op->actions : rule->actions; break; case OFOPERATION_DELETE: - fu.ofpacts = rule->ofpacts; - fu.ofpacts_len = rule->ofpacts_len; + actions = rule->actions; break; default: NOT_REACHED(); } } + fu.ofpacts = actions ? actions->ofpacts : NULL; + fu.ofpacts_len = actions ? actions->ofpacts_len : 0; if (list_is_empty(msgs)) { ofputil_start_flow_update(msgs); @@ -4249,11 +4610,14 @@ ofproto_compose_flow_refresh_update(const struct rule *rule, } void -ofmonitor_compose_refresh_updates(struct list *rules, struct list *msgs) +ofmonitor_compose_refresh_updates(struct rule_collection *rules, + struct list *msgs) + OVS_REQUIRES(ofproto_mutex) { - struct rule *rule; + size_t i; - LIST_FOR_EACH (rule, ofproto_node, rules) { + for (i = 0; i < rules->n; i++) { + struct rule *rule = rules->rules[i]; enum nx_flow_monitor_flags flags = rule->monitor_flags; rule->monitor_flags = 0; @@ -4264,7 +4628,8 @@ ofmonitor_compose_refresh_updates(struct list *rules, struct list *msgs) static void ofproto_collect_ofmonitor_refresh_rule(const struct ofmonitor *m, struct rule *rule, uint64_t seqno, - struct list *rules) + struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) { enum nx_flow_monitor_flags update; @@ -4295,7 +4660,7 @@ ofproto_collect_ofmonitor_refresh_rule(const struct ofmonitor *m, } if (!rule->monitor_flags) { - list_push_back(rules, &rule->ofproto_node); + rule_collection_add(rules, rule); } rule->monitor_flags |= update | (m->flags & NXFMF_ACTIONS); } @@ -4303,7 +4668,8 @@ ofproto_collect_ofmonitor_refresh_rule(const struct ofmonitor *m, static void ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m, uint64_t seqno, - struct list *rules) + struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) { const struct ofproto *ofproto = ofconn_get_ofproto(m->ofconn); const struct ofoperation *op; @@ -4339,7 +4705,8 @@ ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m, static void ofproto_collect_ofmonitor_initial_rules(struct ofmonitor *m, - struct list *rules) + struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) { if (m->flags & NXFMF_INITIAL) { ofproto_collect_ofmonitor_refresh_rules(m, 0, rules); @@ -4348,20 +4715,22 @@ ofproto_collect_ofmonitor_initial_rules(struct ofmonitor *m, void ofmonitor_collect_resume_rules(struct ofmonitor *m, - uint64_t seqno, struct list *rules) + uint64_t seqno, struct rule_collection *rules) + OVS_REQUIRES(ofproto_mutex) { ofproto_collect_ofmonitor_refresh_rules(m, seqno, rules); } static enum ofperr handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh) + OVS_EXCLUDED(ofproto_mutex) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); struct ofmonitor **monitors; size_t n_monitors, allocated_monitors; + struct rule_collection rules; struct list replies; enum ofperr error; - struct list rules; struct ofpbuf b; size_t i; @@ -4369,6 +4738,8 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh) ofpbuf_use_const(&b, oh, ntohs(oh->length)); monitors = NULL; n_monitors = allocated_monitors = 0; + + ovs_mutex_lock(&ofproto_mutex); for (;;) { struct ofputil_flow_monitor_request request; struct ofmonitor *m; @@ -4400,20 +4771,25 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh) monitors[n_monitors++] = m; } - list_init(&rules); + rule_collection_init(&rules); for (i = 0; i < n_monitors; i++) { ofproto_collect_ofmonitor_initial_rules(monitors[i], &rules); } ofpmp_init(&replies, oh); ofmonitor_compose_refresh_updates(&rules, &replies); - ofconn_send_replies(ofconn, &replies); + ovs_mutex_unlock(&ofproto_mutex); + rule_collection_destroy(&rules); + + ofconn_send_replies(ofconn, &replies); free(monitors); return 0; error: + ovs_mutex_unlock(&ofproto_mutex); + for (i = 0; i < n_monitors; i++) { ofmonitor_destroy(monitors[i]); } @@ -4423,18 +4799,25 @@ error: static enum ofperr handle_flow_monitor_cancel(struct ofconn *ofconn, const struct ofp_header *oh) + OVS_EXCLUDED(ofproto_mutex) { struct ofmonitor *m; + enum ofperr error; uint32_t id; id = ofputil_decode_flow_monitor_cancel(oh); + + ovs_mutex_lock(&ofproto_mutex); m = ofmonitor_lookup(ofconn, id); - if (!m) { - return OFPERR_NXBRC_FM_BAD_ID; + if (m) { + ofmonitor_destroy(m); + error = 0; + } else { + error = OFPERR_NXBRC_FM_BAD_ID; } + ovs_mutex_unlock(&ofproto_mutex); - ofmonitor_destroy(m); - return 0; + return error; } /* Meters implementation. @@ -4505,6 +4888,7 @@ meter_create(const struct ofputil_meter_config *config, static void meter_delete(struct ofproto *ofproto, uint32_t first, uint32_t last) + OVS_REQUIRES(ofproto_mutex) { uint32_t mid; for (mid = first; mid <= last; ++mid) { @@ -4562,11 +4946,13 @@ handle_modify_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm) static enum ofperr handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh, struct ofputil_meter_mod *mm) + OVS_EXCLUDED(ofproto_mutex) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); uint32_t meter_id = mm->meter.meter_id; + struct rule_collection rules; + enum ofperr error = 0; uint32_t first, last; - struct list rules; if (meter_id == OFPM13_ALL) { first = 1; @@ -4580,7 +4966,8 @@ handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh, /* First delete the rules that use this meter. If any of those rules are * currently being modified, postpone the whole operation until later. */ - list_init(&rules); + rule_collection_init(&rules); + ovs_mutex_lock(&ofproto_mutex); for (meter_id = first; meter_id <= last; ++meter_id) { struct meter *meter = ofproto->meters[meter_id]; if (meter && !list_is_empty(&meter->rules)) { @@ -4588,20 +4975,25 @@ handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh, LIST_FOR_EACH (rule, meter_list_node, &meter->rules) { if (rule->pending) { - return OFPROTO_POSTPONE; + error = OFPROTO_POSTPONE; + goto exit; } - list_push_back(&rules, &rule->ofproto_node); + rule_collection_add(&rules, rule); } } } - if (!list_is_empty(&rules)) { + if (rules.n > 0) { delete_flows__(ofproto, ofconn, oh, &rules, OFPRR_METER_DELETE); } /* Delete the meters. */ meter_delete(ofproto, first, last); - return 0; +exit: + ovs_mutex_unlock(&ofproto_mutex); + rule_collection_destroy(&rules); + + return error; } static enum ofperr @@ -4630,9 +5022,12 @@ handle_meter_mod(struct ofconn *ofconn, const struct ofp_header *oh) if (mm.command != OFPMC13_DELETE) { /* Fails also when meters are not implemented by the provider. */ - if (!meter_id || meter_id > ofproto->meter_features.max_meters) { + if (meter_id == 0 || meter_id > OFPM13_MAX) { error = OFPERR_OFPMMFC_INVALID_METER; goto exit_free_bands; + } else if (meter_id > ofproto->meter_features.max_meters) { + error = OFPERR_OFPMMFC_OUT_OF_METERS; + goto exit_free_bands; } if (mm.meter.n_bands > ofproto->meter_features.max_bands) { error = OFPERR_OFPMMFC_OUT_OF_BANDS; @@ -5171,6 +5566,7 @@ handle_table_mod(struct ofconn *ofconn, const struct ofp_header *oh) static enum ofperr handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg) + OVS_EXCLUDED(ofproto_mutex) { const struct ofp_header *oh = msg->data; enum ofptype type; @@ -5330,6 +5726,7 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg) static bool handle_openflow(struct ofconn *ofconn, const struct ofpbuf *ofp_msg) + OVS_EXCLUDED(ofproto_mutex) { int error = handle_openflow__(ofconn, ofp_msg); if (error && error != OFPROTO_POSTPONE) { @@ -5348,6 +5745,7 @@ handle_openflow(struct ofconn *ofconn, const struct ofpbuf *ofp_msg) * ofoperation_create() and then submit it with ofopgroup_submit(). */ static struct ofopgroup * ofopgroup_create_unattached(struct ofproto *ofproto) + OVS_REQUIRES(ofproto_mutex) { struct ofopgroup *group = xzalloc(sizeof *group); group->ofproto = ofproto; @@ -5372,6 +5770,7 @@ ofopgroup_create_unattached(struct ofproto *ofproto) static struct ofopgroup * ofopgroup_create(struct ofproto *ofproto, struct ofconn *ofconn, const struct ofp_header *request, uint32_t buffer_id) + OVS_REQUIRES(ofproto_mutex) { struct ofopgroup *group = ofopgroup_create_unattached(ofproto); if (ofconn) { @@ -5395,6 +5794,7 @@ ofopgroup_create(struct ofproto *ofproto, struct ofconn *ofconn, * groups. */ static void ofopgroup_submit(struct ofopgroup *group) + OVS_REQUIRES(ofproto_mutex) { if (!group->n_running) { ofopgroup_complete(group); @@ -5406,6 +5806,7 @@ ofopgroup_submit(struct ofopgroup *group) static void ofopgroup_complete(struct ofopgroup *group) + OVS_REQUIRES(ofproto_mutex) { struct ofproto *ofproto = group->ofproto; @@ -5434,8 +5835,23 @@ ofopgroup_complete(struct ofopgroup *group) error = ofconn_pktbuf_retrieve(group->ofconn, group->buffer_id, &packet, &in_port); if (packet) { + struct rule_execute *re; + ovs_assert(!error); - error = rule_execute(op->rule, in_port, packet); + + ofproto_rule_ref(op->rule); + + re = xmalloc(sizeof *re); + re->rule = op->rule; + re->in_port = in_port; + re->packet = packet; + + if (!guarded_list_push_back(&ofproto->rule_executes, + &re->list_node, 1024)) { + ofproto_rule_unref(op->rule); + ofpbuf_delete(re->packet); + free(re); + } } break; } @@ -5463,7 +5879,7 @@ ofopgroup_complete(struct ofopgroup *group) if (!(op->error || ofproto_rule_is_hidden(rule) || (op->type == OFOPERATION_MODIFY - && op->ofpacts + && op->actions && rule->flow_cookie == op->flow_cookie))) { /* Check that we can just cast from ofoperation_type to * nx_flow_update_event. */ @@ -5511,15 +5927,14 @@ ofopgroup_complete(struct ofopgroup *group) } } } else { - ovs_rwlock_wrlock(&rule->rwlock); oftable_remove_rule(rule); - ofproto_rule_destroy(rule); + ofproto_rule_unref(rule); } break; case OFOPERATION_DELETE: ovs_assert(!op->error); - ofproto_rule_destroy(rule); + ofproto_rule_unref(rule); op->rule = NULL; break; @@ -5534,20 +5949,20 @@ ofopgroup_complete(struct ofopgroup *group) } } else { ofproto_rule_change_cookie(ofproto, rule, op->flow_cookie); - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); rule->idle_timeout = op->idle_timeout; rule->hard_timeout = op->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); - if (op->ofpacts) { - free(rule->ofpacts); + ovs_mutex_unlock(&rule->mutex); + if (op->actions) { + struct rule_actions *old_actions; - ovs_rwlock_wrlock(&rule->rwlock); - rule->ofpacts = op->ofpacts; - rule->ofpacts_len = op->ofpacts_len; - ovs_rwlock_unlock(&rule->rwlock); + ovs_mutex_lock(&rule->mutex); + old_actions = rule->actions; + rule->actions = op->actions; + ovs_mutex_unlock(&rule->mutex); - op->ofpacts = NULL; - op->ofpacts_len = 0; + op->actions = NULL; + rule_actions_unref(old_actions); } rule->flags = op->flags; } @@ -5590,6 +6005,7 @@ static struct ofoperation * ofoperation_create(struct ofopgroup *group, struct rule *rule, enum ofoperation_type type, enum ofp_flow_removed_reason reason) + OVS_REQUIRES(ofproto_mutex) { struct ofproto *ofproto = group->ofproto; struct ofoperation *op; @@ -5603,10 +6019,10 @@ ofoperation_create(struct ofopgroup *group, struct rule *rule, op->type = type; op->reason = reason; op->flow_cookie = rule->flow_cookie; - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); op->idle_timeout = rule->idle_timeout; op->hard_timeout = rule->hard_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); op->flags = rule->flags; group->n_running++; @@ -5621,6 +6037,7 @@ ofoperation_create(struct ofopgroup *group, struct rule *rule, static void ofoperation_destroy(struct ofoperation *op) + OVS_REQUIRES(ofproto_mutex) { struct ofopgroup *group = op->group; @@ -5631,7 +6048,7 @@ ofoperation_destroy(struct ofoperation *op) hmap_remove(&group->ofproto->deletions, &op->hmap_node); } list_remove(&op->group_node); - free(op->ofpacts); + rule_actions_unref(op->actions); free(op); } @@ -5662,13 +6079,19 @@ ofoperation_complete(struct ofoperation *op, enum ofperr error) { struct ofopgroup *group = op->group; - ovs_assert(op->rule->pending == op); ovs_assert(group->n_running > 0); ovs_assert(!error || op->type != OFOPERATION_DELETE); op->error = error; if (!--group->n_running && !list_is_empty(&group->ofproto_node)) { + /* This function can be called from ->rule_construct(), in which case + * ofproto_mutex is held, or it can be called from ->run(), in which + * case ofproto_mutex is not held. But only in the latter case can we + * arrive here, so we can safely take ofproto_mutex now. */ + ovs_mutex_lock(&ofproto_mutex); + ovs_assert(op->rule->pending == op); ofopgroup_complete(group); + ovs_mutex_unlock(&ofproto_mutex); } } @@ -5709,6 +6132,7 @@ pick_fallback_dpid(void) * or with no timeouts are not evictable.) */ static bool choose_rule_to_evict(struct oftable *table, struct rule **rulep) + OVS_REQUIRES(ofproto_mutex) { struct eviction_group *evg; @@ -5733,10 +6157,8 @@ choose_rule_to_evict(struct oftable *table, struct rule **rulep) struct rule *rule; HEAP_FOR_EACH (rule, evg_node, &evg->rules) { - if (!ovs_rwlock_trywrlock(&rule->rwlock)) { - *rulep = rule; - return true; - } + *rulep = rule; + return true; } } @@ -5752,39 +6174,13 @@ choose_rule_to_evict(struct oftable *table, struct rule **rulep) static void ofproto_evict(struct ofproto *ofproto) { - struct ofopgroup *group; struct oftable *table; - group = ofopgroup_create_unattached(ofproto); + ovs_mutex_lock(&ofproto_mutex); OFPROTO_FOR_EACH_TABLE (table, ofproto) { - while (table->eviction_fields) { - struct rule *rule; - size_t n_rules; - - ovs_rwlock_rdlock(&table->cls.rwlock); - n_rules = classifier_count(&table->cls); - ovs_rwlock_unlock(&table->cls.rwlock); - - if (n_rules <= table->max_flows) { - break; - } - - if (!choose_rule_to_evict(table, &rule)) { - break; - } - - if (rule->pending) { - ovs_rwlock_unlock(&rule->rwlock); - break; - } - - ofoperation_create(group, rule, - OFOPERATION_DELETE, OFPRR_EVICTION); - oftable_remove_rule(rule); - ofproto->ofproto_class->rule_delete(rule); - } + evict_rules_from_table(ofproto, table, 0); } - ofopgroup_submit(group); + ovs_mutex_unlock(&ofproto_mutex); } /* Eviction groups. */ @@ -5803,6 +6199,7 @@ eviction_group_priority(size_t n_rules) * adds or removes rules in 'evg'. */ static void eviction_group_resized(struct oftable *table, struct eviction_group *evg) + OVS_REQUIRES(ofproto_mutex) { heap_change(&table->eviction_groups_by_size, &evg->size_node, eviction_group_priority(heap_count(&evg->rules))); @@ -5818,6 +6215,7 @@ eviction_group_resized(struct oftable *table, struct eviction_group *evg) * - Frees 'evg'. */ static void eviction_group_destroy(struct oftable *table, struct eviction_group *evg) + OVS_REQUIRES(ofproto_mutex) { while (!heap_is_empty(&evg->rules)) { struct rule *rule; @@ -5834,6 +6232,7 @@ eviction_group_destroy(struct oftable *table, struct eviction_group *evg) /* Removes 'rule' from its eviction group, if any. */ static void eviction_group_remove_rule(struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { if (rule->eviction_group) { struct oftable *table = &rule->ofproto->tables[rule->table_id]; @@ -5853,6 +6252,7 @@ eviction_group_remove_rule(struct rule *rule) * returns the hash value. */ static uint32_t eviction_group_hash_rule(struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { struct oftable *table = &rule->ofproto->tables[rule->table_id]; const struct mf_subfield *sf; @@ -5890,6 +6290,7 @@ eviction_group_hash_rule(struct rule *rule) * if necessary. */ static struct eviction_group * eviction_group_find(struct oftable *table, uint32_t id) + OVS_REQUIRES(ofproto_mutex) { struct eviction_group *evg; @@ -5911,6 +6312,7 @@ eviction_group_find(struct oftable *table, uint32_t id) * for eviction. */ static uint32_t rule_eviction_priority(struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { long long int hard_expiration; long long int idle_expiration; @@ -5918,7 +6320,7 @@ rule_eviction_priority(struct rule *rule) uint32_t expiration_offset; /* Calculate time of expiration. */ - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); hard_expiration = (rule->hard_timeout ? rule->modified + rule->hard_timeout * 1000 : LLONG_MAX); @@ -5926,7 +6328,7 @@ rule_eviction_priority(struct rule *rule) ? rule->used + rule->idle_timeout * 1000 : LLONG_MAX); expiration = MIN(hard_expiration, idle_expiration); - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); if (expiration == LLONG_MAX) { return 0; } @@ -5950,14 +6352,15 @@ rule_eviction_priority(struct rule *rule) * The caller must ensure that 'rule' is not already in an eviction group. */ static void eviction_group_add_rule(struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { struct ofproto *ofproto = rule->ofproto; struct oftable *table = &ofproto->tables[rule->table_id]; bool has_timeout; - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); has_timeout = rule->hard_timeout || rule->idle_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); if (table->eviction_fields && has_timeout) { struct eviction_group *evg; @@ -6022,6 +6425,7 @@ oftable_set_name(struct oftable *table, const char *name) * This function configures the former policy on 'table'. */ static void oftable_disable_eviction(struct oftable *table) + OVS_REQUIRES(ofproto_mutex) { if (table->eviction_fields) { struct eviction_group *evg, *next; @@ -6048,6 +6452,7 @@ oftable_disable_eviction(struct oftable *table) static void oftable_enable_eviction(struct oftable *table, const struct mf_subfield *fields, size_t n_fields) + OVS_REQUIRES(ofproto_mutex) { struct cls_cursor cursor; struct rule *rule; @@ -6080,61 +6485,60 @@ oftable_enable_eviction(struct oftable *table, /* Removes 'rule' from the oftable that contains it. */ static void -oftable_remove_rule__(struct ofproto *ofproto, struct classifier *cls, - struct rule *rule) - OVS_REQ_WRLOCK(cls->rwlock) OVS_RELEASES(rule->rwlock) +oftable_remove_rule__(struct ofproto *ofproto, struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { - classifier_remove(cls, &rule->cr); + struct classifier *cls = &ofproto->tables[rule->table_id].cls; + + ovs_rwlock_wrlock(&cls->rwlock); + classifier_remove(cls, CONST_CAST(struct cls_rule *, &rule->cr)); + ovs_rwlock_unlock(&cls->rwlock); + cookies_remove(ofproto, rule); + eviction_group_remove_rule(rule); - ovs_mutex_lock(&ofproto->expirable_mutex); if (!list_is_empty(&rule->expirable)) { list_remove(&rule->expirable); } - ovs_mutex_unlock(&ofproto->expirable_mutex); if (!list_is_empty(&rule->meter_list_node)) { list_remove(&rule->meter_list_node); list_init(&rule->meter_list_node); } - ovs_rwlock_unlock(&rule->rwlock); } static void oftable_remove_rule(struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { - struct ofproto *ofproto = rule->ofproto; - struct oftable *table = &ofproto->tables[rule->table_id]; - - ovs_rwlock_wrlock(&table->cls.rwlock); - oftable_remove_rule__(ofproto, &table->cls, rule); - ovs_rwlock_unlock(&table->cls.rwlock); + oftable_remove_rule__(rule->ofproto, rule); } /* Inserts 'rule' into its oftable, which must not already contain any rule for * the same cls_rule. */ static void oftable_insert_rule(struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { struct ofproto *ofproto = rule->ofproto; struct oftable *table = &ofproto->tables[rule->table_id]; bool may_expire; - ovs_mutex_lock(&rule->timeout_mutex); + ovs_mutex_lock(&rule->mutex); may_expire = rule->hard_timeout || rule->idle_timeout; - ovs_mutex_unlock(&rule->timeout_mutex); + ovs_mutex_unlock(&rule->mutex); if (may_expire) { - ovs_mutex_lock(&ofproto->expirable_mutex); list_insert(&ofproto->expirable, &rule->expirable); - ovs_mutex_unlock(&ofproto->expirable_mutex); } + cookies_insert(ofproto, rule); - if (rule->meter_id) { - struct meter *meter = ofproto->meters[rule->meter_id]; + + if (rule->actions->meter_id) { + struct meter *meter = ofproto->meters[rule->actions->meter_id]; list_insert(&meter->rules, &rule->meter_list_node); } ovs_rwlock_wrlock(&table->cls.rwlock); - classifier_insert(&table->cls, &rule->cr); + classifier_insert(&table->cls, CONST_CAST(struct cls_rule *, &rule->cr)); ovs_rwlock_unlock(&table->cls.rwlock); eviction_group_add_rule(rule); } @@ -6204,6 +6608,7 @@ ofproto_get_vlan_usage(struct ofproto *ofproto, unsigned long int *vlan_bitmap) OFPROTO_FOR_EACH_TABLE (oftable, ofproto) { const struct cls_table *table; + ovs_rwlock_rdlock(&oftable->cls.rwlock); HMAP_FOR_EACH (table, hmap_node, &oftable->cls.tables) { if (minimask_get_vid_mask(&table->mask) == VLAN_VID_MASK) { const struct cls_rule *rule; @@ -6215,6 +6620,7 @@ ofproto_get_vlan_usage(struct ofproto *ofproto, unsigned long int *vlan_bitmap) } } } + ovs_rwlock_unlock(&oftable->cls.rwlock); } } diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c index 7cd048519..867012743 100644 --- a/ovsdb/ovsdb-tool.c +++ b/ovsdb/ovsdb-tool.c @@ -519,7 +519,8 @@ do_show_log(int argc, char *argv[]) date = shash_find_data(json_object(json), "_date"); if (date && date->type == JSON_INTEGER) { time_t t = json_integer(date); - char *s = xastrftime(" %Y-%m-%d %H:%M:%S", t, true); + char *s = xastrftime_msec(" %Y-%m-%d %H:%M:%S", + t * 1000LL, true); fputs(s, stdout); free(s); } diff --git a/python/ovs/vlog.py b/python/ovs/vlog.py index 25ae4ea3b..14679d9cb 100644 --- a/python/ovs/vlog.py +++ b/python/ovs/vlog.py @@ -60,7 +60,8 @@ class Vlog: if not Vlog.__inited: return - now = datetime.datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ") + dt = datetime.datetime.utcnow(); + now = dt.strftime("%Y-%m-%dT%H:%M:%S.%%iZ") % (dt.microsecond/1000) syslog_message = ("%s|%s|%s|%s" % (Vlog.__msg_num, self.name, level, message)) diff --git a/tests/ofproto.at b/tests/ofproto.at index 91ee85ab8..0e1d41ba0 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -732,13 +732,13 @@ AT_CLEANUP AT_SETUP([ofproto - flow table configuration (OpenFlow 1.2)]) OVS_VSWITCHD_START # Check the default configuration. -(mid="wild=0xffffffffff, max=1000000," +(mid="wild=0xfffffffff, max=1000000," tail=" lookup=0, matched=0 - match=0xffffffffff, instructions=0x00000007, config=0x00000003 + match=0xfffffffff, instructions=0x00000007, config=0x00000003 write_actions=0x00000000, apply_actions=0x00000000 - write_setfields=0x000000ffffffffff - apply_setfields=0x000000ffffffffff + write_setfields=0x0000000fffffffff + apply_setfields=0x0000000fffffffff metadata_match=0xffffffffffffffff metadata_write=0xffffffffffffffff" echo "OFPST_TABLE reply (OF1.2) (xid=0x2): 254 tables @@ -763,9 +763,9 @@ AT_CHECK( # Check that the configuration was updated. mv expout orig-expout (echo "OFPST_TABLE reply (OF1.2) (xid=0x2): 254 tables - 0: main : wild=0xffffffffff, max=1000000, active=0" + 0: main : wild=0xfffffffff, max=1000000, active=0" tail -n +3 orig-expout | head -7 - echo " 1: table1 : wild=0xffffffffff, max= 1024, active=0" + echo " 1: table1 : wild=0xfffffffff, max= 1024, active=0" tail -n +11 orig-expout) > expout AT_CHECK([ovs-ofctl -O OpenFlow12 dump-tables br0], [0], [expout]) OVS_VSWITCHD_STOP diff --git a/tests/vlog.at b/tests/vlog.at index 957d87227..35336ce55 100644 --- a/tests/vlog.at +++ b/tests/vlog.at @@ -9,7 +9,7 @@ AT_CHECK([$PYTHON $srcdir/test-vlog.py --log-file log_file \ AT_CHECK([diff log_file stderr_log]) -AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z|//' \ +AT_CHECK([sed -e 's/.*-.*-.*T..:..:..\....Z|//' \ -e 's/File ".*", line [[0-9]][[0-9]]*,/File , line ,/' \ stderr_log], [0], [dnl 0|module_0|EMER|emergency diff --git a/utilities/ovs-appctl.8.in b/utilities/ovs-appctl.8.in index df5630978..1cf888d38 100644 --- a/utilities/ovs-appctl.8.in +++ b/utilities/ovs-appctl.8.in @@ -158,13 +158,20 @@ The current date and time in ISO 8601 format (YYYY\-MM\-DD HH:MM:SS). .IP \fB%d{\fIformat\fB}\fR The current date and time in the specified \fIformat\fR, which takes the same format as the \fItemplate\fR argument to \fBstrftime\fR(3). +As an extension, any \fB#\fR characters in \fIformat\fR will be +replaced by fractional seconds, e.g. use \fB%H:%M:%S.###\fR for the +time to the nearest millisecond. Sub-second times are only +approximate and currently decimal places after the third will always +be reported as zero. . .IP \fB%D\fR The current UTC date and time in ISO 8601 format (YYYY\-MM\-DD HH:MM:SS). . .IP \fB%D{\fIformat\fB}\fR -The current UTC date and time in the specified \fIformat\fR, which takes -the same format as the \fItemplate\fR argument to \fBstrftime\fR(3). +The current UTC date and time in the specified \fIformat\fR, which +takes the same format as the \fItemplate\fR argument to +\fBstrftime\fR(3). Supports the same extension for sub-second +resolution as \fB%d{\fR...\fB}\fR. . .IP \fB%m\fR The message being logged. diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 95bf1bfbb..a5a5c02ce 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -1396,7 +1396,8 @@ monitor_vconn(struct vconn *vconn, bool reply_to_echo_requests) run(retval, "vconn_recv"); if (timestamp) { - char *s = xastrftime("%Y-%m-%d %H:%M:%S: ", time_wall(), true); + char *s = xastrftime_msec("%Y-%m-%d %H:%M:%S.###: ", + time_wall_msec(), true); fputs(s, stderr); free(s); } diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 5e54e0bf3..da2dc4240 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2259,11 +2259,10 @@ instant_stats_run(void) iface_refresh_cfm_stats(iface); smap_init(&smap); - if (!ofproto_port_get_bfd_status(br->ofproto, iface->ofp_port, - &smap)) { - ovsrec_interface_set_bfd_status(iface->cfg, &smap); - smap_destroy(&smap); - } + ofproto_port_get_bfd_status(br->ofproto, iface->ofp_port, + &smap); + ovsrec_interface_set_bfd_status(iface->cfg, &smap); + smap_destroy(&smap); } } }