<div dir="ltr"><div class="gmail_quote">Hi Jarno,</div><div class="gmail_quote"><br></div><div class="gmail_quote">Quick question..</div><div class="gmail_quote"><br></div><div class="gmail_quote">On 18 April 2014 13:36, Jarno Rajahalme <span dir="ltr">&lt;<a href="mailto:jrajahalme@nicira.com" target="_blank">jrajahalme@nicira.com</a>&gt;</span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">miniflow_extract() extracts packet headers directly to a miniflow,<br>

which is a compressed form of the struct flow.  This does not require<br>a large struct to be cleared to begin with, and accesses less memory.<br>These performance benefits should allow this to be used in the DPDK<br>datapath.<br>

<br>miniflow_extract() takes a miniflow as an input/output parameter.  On<br>input the buffer for values to be extracted must be properly<br>initialized.  On output the map contains ones for all the fields that<br>have been extracted.<br>

<br>Some struct flow fields are reordered to make miniflow_extract to<br>progress in the logical order.<br><br>Some explicit &quot;inline&quot; keywords are necessary for GCC to optimize this<br>properly.  Also, macros are used for same reason instead of inline<br>

functions for pushing data to the miniflow.<br><br>Signed-off-by: Jarno Rajahalme &lt;<a href="mailto:jrajahalme@nicira.com">jrajahalme@nicira.com</a>&gt;<br>---<br></blockquote><div><br></div><div>...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

@@ -358,115 +344,283 @@ void<br> flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,<br>              struct flow *flow)<br> {<br>-    struct ofpbuf b = *packet;<br>-    struct eth_header *eth;<br>+    uint32_t buf[FLOW_U32S];<br>

+    struct miniflow mf;<br><br>     COVERAGE_INC(flow_extract);<br><br>-    memset(flow, 0, sizeof *flow);<br>+    miniflow_initialize(&amp;mf, buf);<br>+    miniflow_extract(packet, md, &amp;mf);<br>+    miniflow_expand(&amp;mf, flow);<br>

+}<br><br>+/* Caller is responsible for initializing &#39;dst-&gt;values&#39; with enough storage<br>+ * (FLOW_U64S * 8 bytes is enough). */<br>+void<br>+miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,<br>

+                 struct miniflow *dst)<br>+{<br>+    void *data = ofpbuf_data(packet);<br>+    size_t size = ofpbuf_size(packet);<br>+    char *l2;<br>+    struct mf_ctx mf = { 0, dst-&gt;values, dst-&gt;values + FLOW_U32S };<br>

+    ovs_be16 dl_type;<br>+    uint8_t nw_frag, nw_tos, nw_ttl, nw_proto;<br>+<br>+    /* Metadata. */<br>     if (md) {<br>-        flow-&gt;tunnel = md-&gt;tunnel;<br>-        flow-&gt;in_port = md-&gt;in_port;<br>-        flow-&gt;skb_priority = md-&gt;skb_priority;<br>

-        flow-&gt;pkt_mark = md-&gt;pkt_mark;<br>-        flow-&gt;recirc_id = md-&gt;recirc_id;<br>-        flow-&gt;dp_hash = md-&gt;dp_hash;<br>+        if (md-&gt;tunnel.ip_dst) {<br>+            miniflow_push_words(mf, tunnel, &amp;md-&gt;tunnel,<br>

+                                sizeof md-&gt;tunnel / 4);<br>+        }<br>+        miniflow_push_uint32_check(mf, skb_priority, md-&gt;skb_priority);<br>+        miniflow_push_uint32_check(mf, pkt_mark, md-&gt;pkt_mark);<br>

+        miniflow_push_uint32_check(mf, recirc_id, md-&gt;recirc_id);<br>+        miniflow_push_uint32(mf, in_port, odp_to_u32(md-&gt;in_port.odp_port));<br>     }</blockquote><div><br></div><div>... </div><div><br></div>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">         }<br>     }<br>+    if (md) {<br>+        miniflow_push_uint32_check(mf, dp_hash, md-&gt;dp_hash);<br>

+    }<br>+ out:<br>+    dst-&gt;map = mf.map;<br> }<br></blockquote><div><br></div><div>Was there a particular reason for shifting the dp_hash part to the end of the function here, rather than having it grouped with the other metadata fields? </div>

</div></div>