Looks good. Ethan
On Tue, Sep 27, 2011 at 16:27, Ben Pfaff <[email protected]> wrote: > It's been more or less convenient to pass a dpif_upcall to send_packet_in() > in the past, because most callers had one handy. But an upcoming commit > won't have such easy access, so this commit breaks send_packet_in() into > two functions for the different types of packets to send to the controller, > each of which takes appropriate parameters instead of dpif_upcall. > --- > ofproto/ofproto-dpif.c | 63 +++++++++++++++++++++++++++++------------------ > 1 files changed, 39 insertions(+), 24 deletions(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 5136631..fb870be 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -1702,25 +1702,49 @@ port_is_lacp_current(const struct ofport *ofport_) > > /* Upcall handling. */ > > -/* Given 'upcall', of type DPIF_UC_ACTION or DPIF_UC_MISS, sends an > - * OFPT_PACKET_IN message to each OpenFlow controller as necessary according > to > - * their individual configurations. > +/* Sends an OFPT_PACKET_IN message for 'packet' of type OFPR_NO_MATCH to each > + * OpenFlow controller as necessary according to their individual > + * configurations. > + * > + * If 'clone' is true, the caller retains ownership of 'packet'. Otherwise, > + * ownership is transferred to this function. */ > +static void > +send_packet_in_miss(struct ofproto_dpif *ofproto, struct ofpbuf *packet, > + const struct flow *flow, bool clone) > +{ > + struct ofputil_packet_in pin; > + > + pin.packet = packet; > + pin.in_port = flow->in_port; > + pin.reason = OFPR_NO_MATCH; > + pin.buffer_id = 0; /* not yet known */ > + pin.send_len = 0; /* not used for flow table misses */ > + connmgr_send_packet_in(ofproto->up.connmgr, &pin, flow, > + clone ? NULL : packet); > +} > + > +/* Sends an OFPT_PACKET_IN message for 'packet' of type OFPR_ACTION to each > + * OpenFlow controller as necessary according to their individual > + * configurations. > + * > + * 'send_len' should be the number of bytes of 'packet' to send to the > + * controller, as specified in the action that caused the packet to be sent. > * > * If 'clone' is true, the caller retains ownership of 'upcall->packet'. > * Otherwise, ownership is transferred to this function. */ > static void > -send_packet_in(struct ofproto_dpif *ofproto, struct dpif_upcall *upcall, > - const struct flow *flow, bool clone) > +send_packet_in_action(struct ofproto_dpif *ofproto, struct ofpbuf *packet, > + int send_len, const struct flow *flow, bool clone) > { > struct ofputil_packet_in pin; > > - pin.packet = upcall->packet; > + pin.packet = packet; > pin.in_port = flow->in_port; > - pin.reason = upcall->type == DPIF_UC_MISS ? OFPR_NO_MATCH : OFPR_ACTION; > + pin.reason = OFPR_ACTION; > pin.buffer_id = 0; /* not yet known */ > - pin.send_len = upcall->userdata; > + pin.send_len = send_len; > connmgr_send_packet_in(ofproto->up.connmgr, &pin, flow, > - clone ? NULL : upcall->packet); > + clone ? NULL : packet); > } > > static bool > @@ -1785,7 +1809,7 @@ handle_miss_upcall(struct ofproto_dpif *ofproto, struct > dpif_upcall *upcall) > flow.in_port); > } > > - send_packet_in(ofproto, upcall, &flow, false); > + send_packet_in_miss(ofproto, upcall->packet, &flow, false); > return; > } > > @@ -1807,7 +1831,7 @@ handle_miss_upcall(struct ofproto_dpif *ofproto, struct > dpif_upcall *upcall) > * > * See the top-level comment in fail-open.c for more information. > */ > - send_packet_in(ofproto, upcall, &flow, true); > + send_packet_in_miss(ofproto, upcall->packet, &flow, true); > } > > facet_execute(ofproto, facet, upcall->packet); > @@ -1824,7 +1848,8 @@ handle_upcall(struct ofproto_dpif *ofproto, struct > dpif_upcall *upcall) > case DPIF_UC_ACTION: > COVERAGE_INC(ofproto_dpif_ctlr_action); > odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow); > - send_packet_in(ofproto, upcall, &flow, false); > + send_packet_in_action(ofproto, upcall->packet, upcall->userdata, > + &flow, false); > break; > > case DPIF_UC_SAMPLE: > @@ -2173,18 +2198,8 @@ execute_odp_actions(struct ofproto_dpif *ofproto, > const struct flow *flow, > /* As an optimization, avoid a round-trip from userspace to kernel to > * userspace. This also avoids possibly filling up kernel packet > * buffers along the way. */ > - struct dpif_upcall upcall; > - > - upcall.type = DPIF_UC_ACTION; > - upcall.packet = packet; > - upcall.key = NULL; > - upcall.key_len = 0; > - upcall.userdata = nl_attr_get_u64(odp_actions); > - upcall.sample_pool = 0; > - upcall.actions = NULL; > - upcall.actions_len = 0; > - > - send_packet_in(ofproto, &upcall, flow, false); > + send_packet_in_action(ofproto, packet, nl_attr_get_u64(odp_actions), > + flow, false); > > return true; > } else { > -- > 1.7.4.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
