[ovs-dev] [PATCH 2/3] ofproto-dpif-xlate: Take control of the qdscp map.

Ben Pfaff blp at nicira.com
Fri Aug 2 18:23:31 UTC 2013


On Thu, Aug 01, 2013 at 03:55:46PM -0700, Ethan Jackson wrote:
> This will make locking easier in future patches.
> 
> Signed-off-by: Ethan Jackson <ethan at nicira.com>

We renamed 'priority' to 'skb_priority' in struct flow, it would be nice
for the naming here to reflect that.

Folding in the following incremental makes the code easier to read.

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 34bcf73..d7e6293 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -414,9 +414,7 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
     for (i = 0; i < n_qdscp; i++) {
         struct priority_to_dscp *pdscp;
         uint32_t priority;
-        uint8_t dscp;
 
-        dscp = (qdscp_list[i].dscp << 2) & IP_DSCP_MASK;
         if (ofproto_dpif_queue_to_priority(xport->xbridge->ofproto,
                                            qdscp_list[i].queue, &priority)) {
             continue;
@@ -429,7 +427,7 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
             pdscp = xmalloc(sizeof *pdscp);
             pdscp->priority = priority;
         }
-        pdscp->dscp = dscp;
+        pdscp->dscp = (qdscp_list[i].dscp << 2) & IP_DSCP_MASK;
 
         hmap_insert(&new, &pdscp->hmap_node, hash_int(pdscp->priority, 0));
     }

In xlate_ofport_set(), the migrate-and-swap system made sense in the
previous location for this code, where the comparison was used to decide
whether to revalidate.  Now, in the new location, it isn't used for
that, so we could instead just clear out the old data and then install
new data.  I don't know whether that would simplify much, and it would
cause extra malloc()s, but it would lead to fewer "why?" questions on
reading for people who don't know the history.



More information about the dev mailing list