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 f4b64f8..580f45d 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 93763b5..4baf99f 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 b4bacc9..4663463 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)
 {