Thanks Ryan, this is a great refactoring.
Looks good to me,
Minor issues:
1. Could you rebase the patch against master? Need to fix some new calls,
added after you posted the patch.
On Wed, May 7, 2014 at 3:14 PM, Ryan Wilson <wr...@nicira.com> wrote:
> The upcall hander keeps a hash table which hashes flow to a list of
> corresponding packets.
s/hander/handler
@@ -710,62 +687,55 @@ compose_slow_path(struct udpif *udpif, struct
> xlate_out *xout,
> odp_put_userspace_action(pid, &cookie, sizeof cookie.slow_path, buf);
> }
>
> -static struct flow_miss *
> -flow_miss_find(struct hmap *todo, const struct ofproto_dpif *ofproto,
> - const struct flow *flow, uint32_t hash)
> +static void
> +upcall_init(struct upcall *upcall, struct flow *flow, struct ofpbuf
> *packet,
> + struct ofproto_dpif *ofproto, struct dpif_upcall *dupcall,
> + odp_port_t odp_in_port)
> {
> - struct flow_miss *miss;
> -
> - HMAP_FOR_EACH_WITH_HASH (miss, hmap_node, hash, todo) {
> - if (miss->ofproto == ofproto && flow_equal(&miss->flow, flow)) {
> - return miss;
> - }
> + struct xlate_in xin;
> + struct pkt_metadata md = pkt_metadata_from_flow(flow);
>
> + flow_extract(packet, &md, &upcall->flow);
> +
>
Add a newline between local variable declaration and the code.
> +
>
> /* Do not install a flow into the datapath if:
> *
> * - The datapath already has too many flows.
> *
> - * - An earlier iteration of this loop already put the same
> flow.
> - *
> * - We received this packet via some flow installed in the
> kernel
> * already. */
> if (may_put
> - && !miss->put
> && upcall->dpif_upcall.type == DPIF_UC_MISS) {
> struct ofpbuf mask;
> bool megaflow;
>
> - miss->put = true;
> -
>
Here, the removal of 'miss->put', may cause the warning of inserting
duplicated flows (when upcalls from same flow
at handled in same batch). We think it is okay, to have this warning,
since it should be very rare and it is will not
cause duplicated flows in datapath. Let's see if there is anything shown
up during the tcp_crr test.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
