ofproto: Specialize ofproto_send_packet() for uses the callers really want.

The callers of ofproto_send_packet() actually just want to send a packet
out on a port, possibly tagged with a VLAN, but the interface forced them
to compose a set of OpenFlow actions, which made it harder to use than
necessary.  This commit specializes the interface for the purposes that
the callers really wanted, making it easier to use.

handle_miss_upcall() can now take advantage of this function, too.

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f4b64f8814678577044bcaf91b17d17fe405c2b6..580f45da40289acf63d770d1c2cb1c11cc9159d3 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1404,26 +1404,34 @@ ofproto_port_is_floodable(struct ofproto *ofproto, uint16_t odp_port)
     return ofport && !(ofport->opp.config & OFPPC_NO_FLOOD);
 }
 
+/* Sends 'packet' out of port 'port_no' within 'p'.  If 'vlan_tci' is zero the
+ * packet will not have any 802.1Q hader; if it is nonzero, then the packet
+ * will be sent with the VLAN TCI specified by 'vlan_tci & ~VLAN_CFI'.
+ *
+ * Returns 0 if successful, otherwise a positive errno value. */
 int
-ofproto_send_packet(struct ofproto *p, const struct flow *flow,
-                    const union ofp_action *actions, size_t n_actions,
+ofproto_send_packet(struct ofproto *ofproto,
+                    uint32_t port_no, uint16_t vlan_tci,
                     const struct ofpbuf *packet)
 {
-    struct action_xlate_ctx ctx;
-    struct ofpbuf *odp_actions;
-
-    action_xlate_ctx_init(&ctx, p, flow, packet);
-    /* Always xlate packets originated in this function. */
-    ctx.check_special = false;
-    odp_actions = xlate_actions(&ctx, actions, n_actions);
-
-    /* XXX Should we translate the dpif_execute() errno value into an OpenFlow
-     * error code? */
-    dpif_execute(p->dpif, odp_actions->data, odp_actions->size, packet);
+    struct ofpbuf odp_actions;
+    int error;
 
-    ofpbuf_delete(odp_actions);
+    ofpbuf_init(&odp_actions, 32);
+    if (vlan_tci != 0) {
+        nl_msg_put_u32(&odp_actions, ODP_ACTION_ATTR_SET_DL_TCI,
+                       ntohs(vlan_tci & ~VLAN_CFI));
+    }
+    nl_msg_put_u32(&odp_actions, ODP_ACTION_ATTR_OUTPUT, port_no);
+    error = dpif_execute(ofproto->dpif, odp_actions.data, odp_actions.size,
+                         packet);
+    ofpbuf_uninit(&odp_actions);
 
-    return 0;
+    if (error) {
+        VLOG_WARN_RL(&rl, "%s: failed to send packet on port %"PRIu32" (%s)",
+                     dpif_name(ofproto->dpif), port_no, strerror(error));
+    }
+    return error;
 }
 
 /* Adds a flow to the OpenFlow flow table in 'p' that matches 'cls_rule' and
@@ -4376,13 +4384,7 @@ handle_miss_upcall(struct ofproto *p, struct dpif_upcall *upcall)
     /* Check with in-band control to see if this packet should be sent
      * to the local port regardless of the flow table. */
     if (in_band_msg_in_hook(p->in_band, &flow, upcall->packet)) {
-        struct ofpbuf odp_actions;
-
-        ofpbuf_init(&odp_actions, 32);
-        nl_msg_put_u32(&odp_actions, ODP_ACTION_ATTR_OUTPUT, ODPP_LOCAL);
-        dpif_execute(p->dpif, odp_actions.data, odp_actions.size,
-                     upcall->packet);
-        ofpbuf_uninit(&odp_actions);
+        ofproto_send_packet(p, ODPP_LOCAL, 0, upcall->packet);
     }
 
     facet = facet_lookup_valid(p, &flow);

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 93763b5c54be734cbccf23907df62e93390c2818..4baf99fe694659cd769e0549d89412c5c2235627 100644 (file)
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -132,8 +132,7 @@ void ofproto_get_snoops(const struct ofproto *, struct svec *);
 void ofproto_get_all_flows(struct ofproto *p, struct ds *);
 
 /* Functions for use by ofproto implementation modules, not by clients. */
-int ofproto_send_packet(struct ofproto *, const struct flow *,
-                        const union ofp_action *, size_t n_actions,
+int ofproto_send_packet(struct ofproto *, uint32_t port_no, uint16_t vlan_tci,
                         const struct ofpbuf *);
 void ofproto_add_flow(struct ofproto *, const struct cls_rule *,
                       const union ofp_action *, size_t n_actions);

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index b4bacc9a33f3648a8ab8fe6e149900ca05eb18d2..466346385120508c7c185d26a21ff85247105832 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -306,7 +306,6 @@ static void iface_set_ofport(const struct ovsrec_interface *, int64_t ofport);
 static void iface_update_qos(struct iface *, const struct ovsrec_qos *);
 static void iface_update_cfm(struct iface *);
 static void iface_refresh_cfm_stats(struct iface *iface);
-static void iface_send_packet(struct iface *, struct ofpbuf *packet);
 static void iface_update_carrier(struct iface *);
 static bool iface_get_carrier(const struct iface *);
 
@@ -315,7 +314,6 @@ static void shash_from_ovs_idl_map(char **keys, char **values, size_t n,
 static void shash_to_ovs_idl_map(struct shash *,
                                  char ***keys, char ***values, size_t *n);
 
-
 /* Hooks into ofproto processing. */
 static struct ofhooks bridge_ofhooks;
 \f
@@ -3427,9 +3425,8 @@ bond_send_learning_packets(struct port *port)
     ofpbuf_init(&packet, 128);
     error = n_packets = n_errors = 0;
     LIST_FOR_EACH (e, lru_node, &br->ml->lrus) {
-        union ofp_action actions[2], *a;
-        uint16_t dp_ifidx;
         tag_type tags = 0;
+        uint16_t dp_ifidx;
         struct flow flow;
         int retval;
 
@@ -3445,24 +3442,9 @@ bond_send_learning_packets(struct port *port)
             continue;
         }
 
-        /* Compose actions. */
-        memset(actions, 0, sizeof actions);
-        a = actions;
-        if (e->vlan) {
-            a->vlan_vid.type = htons(OFPAT_SET_VLAN_VID);
-            a->vlan_vid.len = htons(sizeof *a);
-            a->vlan_vid.vlan_vid = htons(e->vlan);
-            a++;
-        }
-        a->output.type = htons(OFPAT_OUTPUT);
-        a->output.len = htons(sizeof *a);
-        a->output.port = htons(odp_port_to_ofp_port(dp_ifidx));
-        a++;
-
         /* Send packet. */
         n_packets++;
-        retval = ofproto_send_packet(br->ofproto, &flow, actions, a - actions,
-                                     &packet);
+        retval = ofproto_send_packet(br->ofproto, dp_ifidx, e->vlan, &packet);
         if (retval) {
             error = retval;
             n_errors++;
@@ -3859,7 +3841,8 @@ lacp_send_pdu_cb(void *aux, const struct lacp_pdu *pdu)
 
         ofpbuf_init(&packet, ETH_HEADER_LEN + LACP_PDU_LEN);
         compose_lacp_packet(&packet, ea, pdu);
-        iface_send_packet(iface, &packet);
+        ofproto_send_packet(iface->port->bridge->ofproto,
+                            iface->dp_ifidx, 0, &packet);
         ofpbuf_uninit(&packet);
     } else {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 10);
@@ -3912,7 +3895,8 @@ port_run(struct port *port)
         if (iface->cfm) {
             struct ofpbuf *packet = cfm_run(iface->cfm);
             if (packet) {
-                iface_send_packet(iface, packet);
+                ofproto_send_packet(port->bridge->ofproto, iface->dp_ifidx,
+                                    0, packet);
                 ofpbuf_uninit(packet);
                 free(packet);
             }
@@ -4349,26 +4333,6 @@ port_update_bonding(struct port *port)
 \f
 /* Interface functions. */
 
-static void
-iface_send_packet(struct iface *iface, struct ofpbuf *packet)
-{
-    struct flow flow;
-    union ofp_action action;
-
-    memset(&action, 0, sizeof action);
-    action.output.type = htons(OFPAT_OUTPUT);
-    action.output.len  = htons(sizeof action);
-    action.output.port = htons(odp_port_to_ofp_port(iface->dp_ifidx));
-
-    flow_extract(packet, 0, ODPP_NONE, &flow);
-
-    if (ofproto_send_packet(iface->port->bridge->ofproto, &flow, &action, 1,
-                            packet)) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-        VLOG_WARN_RL(&rl, "interface %s: Failed to send packet.", iface->name);
-    }
-}
-
 static struct iface *
 iface_create(struct port *port, const struct ovsrec_interface *if_cfg)
 {