[ovs-dev] [cfm 2/3] ofproto: Specialize ofproto_send_packet() for uses the callers really want.

Ben Pfaff blp at nicira.com
Wed Mar 16 14:37:47 PDT 2011


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.
---
 ofproto/ofproto.c |   46 ++++++++++++++++++++++++----------------------
 ofproto/ofproto.h |    5 ++---
 vswitchd/bridge.c |   44 ++++----------------------------------------
 3 files changed, 30 insertions(+), 65 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 147ade3..f6010ca 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1470,26 +1470,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
@@ -4470,13 +4478,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 a32b9b9..4a0464c 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010 Nicira Networks.
+ * Copyright (c) 2009, 2010, 2011 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -136,8 +136,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 6f8ca25..e09ae6d 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -325,7 +325,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 uint8_t iface_get_lacp_state(const struct iface *);
 static void iface_get_lacp_priority(struct iface *, struct lacp_info *);
 static void iface_set_lacp_defaulted(struct iface *);
@@ -3319,7 +3318,8 @@ lacp_run(struct port *port)
             iface->lacp_actor.state = iface_get_lacp_state(iface);
             compose_lacp_packet(&packet, &iface->lacp_actor,
                                 &iface->lacp_partner, ea);
-            iface_send_packet(iface, &packet);
+            ofproto_send_packet(port->bridge->ofproto, iface->dp_ifidx,
+                                0, &packet);
         } else {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 10);
             VLOG_ERR_RL(&rl, "iface %s: failed to obtain Ethernet address "
@@ -3692,9 +3692,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;
 
@@ -3710,24 +3709,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++;
@@ -4761,26 +4745,6 @@ iface_get_lacp_priority(struct iface *iface, struct lacp_info *priority)
     priority->state = 0;
 }
 
-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)
 {
-- 
1.7.1




More information about the dev mailing list