ofproto-dpif: fix an ovs crash when dpif_recv_set returns error
[cascardo/ovs.git] / ofproto / ofproto-dpif.c
index 001a2c0..d26aaf7 100644 (file)
@@ -73,11 +73,6 @@ VLOG_DEFINE_THIS_MODULE(ofproto_dpif);
 COVERAGE_DEFINE(ofproto_dpif_expired);
 COVERAGE_DEFINE(packet_in_overflow);
 
-/* Number of implemented OpenFlow tables. */
-enum { N_TABLES = 255 };
-enum { TBL_INTERNAL = N_TABLES - 1 };    /* Used for internal hidden rules. */
-BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
-
 /* No bfd/cfm status change. */
 #define NO_STATUS_CHANGE -1
 
@@ -94,6 +89,9 @@ struct rule_dpif {
     struct dpif_flow_stats stats OVS_GUARDED;
 };
 
+/* RULE_CAST() depends on this. */
+BUILD_ASSERT_DECL(offsetof(struct rule_dpif, up) == 0);
+
 static void rule_get_stats(struct rule *, uint64_t *packets, uint64_t *bytes,
                            long long int *used);
 static struct rule_dpif *rule_dpif_cast(const struct rule *);
@@ -367,18 +365,6 @@ ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto,
     ofproto_flow_mod(&ofproto->up, fm);
 }
 
-/* Resets the modified time for 'rule' or an equivalent rule. If 'rule' is not
- * in the classifier, but an equivalent rule is, unref 'rule' and ref the new
- * rule. Otherwise if 'rule' is no longer installed in the classifier,
- * reinstall it.
- *
- * Returns the rule whose modified time has been reset. */
-struct rule_dpif *
-ofproto_dpif_refresh_rule(struct rule_dpif *rule)
-{
-    return rule_dpif_cast(ofproto_refresh_rule(&rule->up));
-}
-
 /* Appends 'pin' to the queue of "packet ins" to be sent to the controller.
  * Takes ownership of 'pin' and pin->packet. */
 void
@@ -931,6 +917,11 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
 
     shash_add(&all_dpif_backers, type, backer);
 
+    backer->enable_recirc = check_recirc(backer);
+    backer->variable_length_userdata = check_variable_length_userdata(backer);
+    backer->max_mpls_depth = check_max_mpls_depth(backer);
+    backer->rid_pool = recirc_id_pool_create();
+
     error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
     if (error) {
         VLOG_ERR("failed to listen on datapath of type %s: %s",
@@ -938,10 +929,6 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
         close_dpif_backer(backer);
         return error;
     }
-    backer->enable_recirc = check_recirc(backer);
-    backer->variable_length_userdata = check_variable_length_userdata(backer);
-    backer->max_mpls_depth = check_max_mpls_depth(backer);
-    backer->rid_pool = recirc_id_pool_create();
 
     if (backer->recv_set_enable) {
         udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
@@ -950,9 +937,9 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     return error;
 }
 
-/* Tests whether 'backer''s datapath supports recirculation Only newer datapath
- * supports OVS_KEY_ATTR in OVS_ACTION_ATTR_USERSPACE actions.  We need to
- * disable some features on older datapaths that don't support this feature.
+/* Tests whether 'backer''s datapath supports recirculation.  Only newer
+ * datapaths support OVS_KEY_ATTR_RECIRC_ID in keys.  We need to disable some
+ * features on older datapaths that don't support this feature.
  *
  * Returns false if 'backer' definitely does not support recirculation, true if
  * it seems to support recirculation or if at least the error we get is
@@ -1057,11 +1044,6 @@ check_variable_length_userdata(struct dpif_backer *backer)
 
     switch (error) {
     case 0:
-        /* Variable-length userdata is supported.
-         *
-         * Purge received packets to avoid processing the nonsense packet we
-         * sent to userspace, then report success. */
-        dpif_recv_purge(backer->dpif);
         return true;
 
     case ERANGE:
@@ -1243,13 +1225,13 @@ add_internal_flows(struct ofproto_dpif *ofproto)
 
     ofpbuf_clear(&ofpacts);
     error = add_internal_miss_flow(ofproto, id++, &ofpacts,
-                              &ofproto->no_packet_in_rule);
+                                   &ofproto->no_packet_in_rule);
     if (error) {
         return error;
     }
 
     error = add_internal_miss_flow(ofproto, id++, &ofpacts,
-                              &ofproto->drop_frags_rule);
+                                   &ofproto->drop_frags_rule);
     if (error) {
         return error;
     }
@@ -1266,7 +1248,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
     match_init_catchall(&match);
     match_set_recirc_id(&match, 0);
 
-    error = ofproto_dpif_add_internal_flow(ofproto, &match, 2,  &ofpacts,
+    error = ofproto_dpif_add_internal_flow(ofproto, &match, 2, &ofpacts,
                                            &unused_rulep);
     if (error) {
         return error;
@@ -1279,7 +1261,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
      */
     ofpbuf_clear(&ofpacts);
     match_init_catchall(&match);
-    error = ofproto_dpif_add_internal_flow(ofproto, &match, 1,  &ofpacts,
+    error = ofproto_dpif_add_internal_flow(ofproto, &match, 1, &ofpacts,
                                            &unused_rulep);
 
     return error;
@@ -1359,6 +1341,12 @@ run(struct ofproto *ofproto_)
         ovs_rwlock_unlock(&ofproto->ml->rwlock);
     }
 
+    /* Always updates the ofproto->pins_seqno to avoid frequent wakeup during
+     * flow restore.  Even though nothing is processed during flow restore,
+     * all queued 'pins' will be handled immediately when flow restore
+     * completes. */
+    ofproto->pins_seqno = seq_read(ofproto->pins_seq);
+
     /* Do not perform any periodic activity required by 'ofproto' while
      * waiting for flow restore to complete. */
     if (!ofproto_get_flow_restore_wait()) {
@@ -1374,12 +1362,6 @@ run(struct ofproto *ofproto_)
         }
     }
 
-    /* Always updates the ofproto->pins_seqno to avoid frequent wakeup during
-     * flow restore.  Even though nothing is processed during flow restore,
-     * all queued 'pins' will be handled immediately when flow restore
-     * completes. */
-    ofproto->pins_seqno = seq_read(ofproto->pins_seq);
-
     if (ofproto->netflow) {
         netflow_run(ofproto->netflow);
     }
@@ -1828,14 +1810,14 @@ out:
 }
 
 static int
-get_cfm_status(const struct ofport *ofport_,
+get_cfm_status(const struct ofport *ofport_, bool force,
                struct ofproto_cfm_status *status)
 {
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
     int ret = 0;
 
     if (ofport->cfm) {
-        if (cfm_check_status_change(ofport->cfm)) {
+        if (cfm_check_status_change(ofport->cfm) || force) {
             status->faults = cfm_get_fault(ofport->cfm);
             status->flap_count = cfm_get_flap_count(ofport->cfm);
             status->remote_opstate = cfm_get_opup(ofport->cfm);
@@ -1870,13 +1852,13 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
 }
 
 static int
-get_bfd_status(struct ofport *ofport_, struct smap *smap)
+get_bfd_status(struct ofport *ofport_, bool force, struct smap *smap)
 {
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
     int ret = 0;
 
     if (ofport->bfd) {
-        if (bfd_check_status_change(ofport->bfd)) {
+        if (bfd_check_status_change(ofport->bfd) || force) {
             bfd_get_status(ofport->bfd, smap);
         } else {
             ret = NO_STATUS_CHANGE;
@@ -3137,18 +3119,18 @@ ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto,
     xin.resubmit_stats = &stats;
     xlate_actions(&xin, &xout);
 
+    execute.actions = ofpbuf_data(&xout.odp_actions);
+    execute.actions_len = ofpbuf_size(&xout.odp_actions);
+    execute.packet = packet;
+    execute.md = pkt_metadata_from_flow(flow);
+    execute.needs_help = (xout.slow & SLOW_ACTION) != 0;
+
+    /* Fix up in_port. */
     in_port = flow->in_port.ofp_port;
     if (in_port == OFPP_NONE) {
         in_port = OFPP_LOCAL;
     }
-    execute.actions = ofpbuf_data(&xout.odp_actions);
-    execute.actions_len = ofpbuf_size(&xout.odp_actions);
-    execute.packet = packet;
-    execute.md.tunnel = flow->tunnel;
-    execute.md.skb_priority = flow->skb_priority;
-    execute.md.pkt_mark = flow->pkt_mark;
     execute.md.in_port.odp_port = ofp_port_to_odp_port(ofproto, in_port);
-    execute.needs_help = (xout.slow & SLOW_ACTION) != 0;
 
     error = dpif_execute(ofproto->backer->dpif, &execute);
 
@@ -3168,24 +3150,6 @@ rule_dpif_credit_stats(struct rule_dpif *rule,
     ovs_mutex_unlock(&rule->stats_mutex);
 }
 
-bool
-rule_dpif_is_fail_open(const struct rule_dpif *rule)
-{
-    return is_fail_open_rule(&rule->up);
-}
-
-bool
-rule_dpif_is_table_miss(const struct rule_dpif *rule)
-{
-    return rule_is_table_miss(&rule->up);
-}
-
-bool
-rule_dpif_is_internal(const struct rule_dpif *rule)
-{
-    return rule_is_internal(&rule->up);
-}
-
 ovs_be64
 rule_dpif_get_flow_cookie(const struct rule_dpif *rule)
     OVS_REQUIRES(rule->up.mutex)
@@ -3427,22 +3391,6 @@ choose_miss_rule(enum ofputil_port_config config, struct rule_dpif *miss_rule,
     }
 }
 
-void
-rule_dpif_ref(struct rule_dpif *rule)
-{
-    if (rule) {
-        ofproto_rule_ref(&rule->up);
-    }
-}
-
-void
-rule_dpif_unref(struct rule_dpif *rule)
-{
-    if (rule) {
-        ofproto_rule_unref(&rule->up);
-    }
-}
-
 static void
 complete_operation(struct rule_dpif *rule)
     OVS_REQUIRES(ofproto_mutex)
@@ -4550,12 +4498,12 @@ ofproto_dpif_unixctl_init(void)
                              ofproto_unixctl_dpif_dump_flows, NULL);
 }
 
-
-/* Returns true if 'rule' is an internal rule, false otherwise. */
+/* Returns true if 'table' is the table used for internal rules,
+ * false otherwise. */
 bool
-rule_is_internal(const struct rule *rule)
+table_is_internal(uint8_t table_id)
 {
-    return rule->table_id == TBL_INTERNAL;
+    return table_id == TBL_INTERNAL;
 }
 \f
 /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
@@ -4837,7 +4785,7 @@ ofproto_dpif_free_recirc_id(struct ofproto_dpif *ofproto, uint32_t recirc_id)
 
 int
 ofproto_dpif_add_internal_flow(struct ofproto_dpif *ofproto,
-                               struct match *match, int priority,
+                               const struct match *match, int priority,
                                const struct ofpbuf *ofpacts,
                                struct rule **rulep)
 {
@@ -4869,8 +4817,8 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif *ofproto,
         return error;
     }
 
-    rule = rule_dpif_lookup_in_table(ofproto, TBL_INTERNAL, &match->flow,
-                                     &match->wc, false);
+    rule = rule_dpif_lookup_in_table(ofproto, TBL_INTERNAL, &fm.match.flow,
+                                     &fm.match.wc, false);
     if (rule) {
         *rulep = &rule->up;
     } else {