Oh, I missed the Acked-by. Thanks, I'll fix that piece up and push to master soon.
Would you mind in future, putting a bit more whitespace around your comments and only keep the minimal context for replies? I find it a bit difficult to see comments when there is so much unrelated text from the patch. On 19 May 2014 13:51, Joe Stringer <joestrin...@nicira.com> wrote: > On 19 May 2014 13:05, Andy Zhou <az...@nicira.com> wrote: > >> On Thu, May 15, 2014 at 9:52 PM, Joe Stringer <joestrin...@nicira.com> >> wrote: >> > The userspace and kernel datapaths previously differed on their >> > treatment of the recirc_id and dp_hash fields when sending upcalls. >> > While the kernel datapath would always serialise these fields, the >> > userspace would not. When using the userspace datapath, this would cause >> > a mismatch between the odp flow key in an upcall compared to the one >> > that is serialised upon flow_dump. >> > >> > This patch brings the userspace datapath behaviour back in line with the >> > kernel datapath by always serialising recirc_id and dp_hash to odp. >> > >> > Signed-off-by: Joe Stringer <joestrin...@nicira.com> >> Acked-by: Andy Zhou <az...@nicira.com> >> > --- >> > v4: Simplify. Pass only a new "recirc" support parameter to >> > odp_flow_key_from_flow__(). >> > v3: Allow users to specify context for serialising fields. >> > Don't always match dp_hash. >> > v2: Always match dp_hash. >> > v1: Initial post. >> > --- >> > lib/dpif-netdev.c | 6 +- >> > lib/odp-util.c | 30 ++++++---- >> > lib/odp-util.h | 5 +- >> > ofproto/ofproto-dpif-upcall.c | 5 +- >> > ofproto/ofproto-dpif.c | 4 +- >> > tests/odp.at | 20 +++---- >> > tests/ofproto-dpif.at | 126 >> ++++++++++++++++++++--------------------- >> > tests/test-odp.c | 2 +- >> > 8 files changed, 104 insertions(+), 94 deletions(-) >> > >> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> > index a255a96..d3f9c7e 100644 >> > --- a/lib/dpif-netdev.c >> > +++ b/lib/dpif-netdev.c >> > @@ -1455,7 +1455,7 @@ dpif_netdev_flow_dump_next(const struct dpif >> *dpif, void *iter_, void *state_, >> > >> > ofpbuf_use_stack(&buf, &state->keybuf, sizeof state->keybuf); >> > odp_flow_key_from_flow(&buf, &netdev_flow->flow, &wc.masks, >> > - netdev_flow->flow.in_port.odp_port); >> > + netdev_flow->flow.in_port.odp_port, >> true); >> > >> > *key = ofpbuf_data(&buf); >> > *key_len = ofpbuf_size(&buf); >> > @@ -1467,7 +1467,7 @@ dpif_netdev_flow_dump_next(const struct dpif >> *dpif, void *iter_, void *state_, >> > ofpbuf_use_stack(&buf, &state->maskbuf, sizeof state->maskbuf); >> > odp_flow_key_from_mask(&buf, &wc.masks, &netdev_flow->flow, >> > odp_to_u32(wc.masks.in_port.odp_port), >> > - SIZE_MAX); >> > + SIZE_MAX, true); >> > >> > *mask = ofpbuf_data(&buf); >> > *mask_len = ofpbuf_size(&buf); >> > @@ -2063,7 +2063,7 @@ dp_netdev_output_userspace(struct dp_netdev *dp, >> struct ofpbuf *packet, >> > >> > /* Put ODP flow. */ >> > miniflow_expand(key, &flow); >> > - odp_flow_key_from_flow(buf, &flow, NULL, >> flow.in_port.odp_port); >> > + odp_flow_key_from_flow(buf, &flow, NULL, >> flow.in_port.odp_port, true); >> > upcall->key = ofpbuf_data(buf); >> > upcall->key_len = ofpbuf_size(buf); >> > >> > diff --git a/lib/odp-util.c b/lib/odp-util.c >> > index 6cff2f1..37ec2be 100644 >> > --- a/lib/odp-util.c >> > +++ b/lib/odp-util.c >> > @@ -2483,7 +2483,8 @@ ovs_to_odp_frag_mask(uint8_t nw_frag_mask) >> > static void >> > odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow, >> > const struct flow *mask, odp_port_t >> odp_in_port, >> > - size_t max_mpls_depth, bool export_mask) >> > + size_t max_mpls_depth, bool export_mask, >> > + bool recirc) >> How about keeping bool export_mask as the last parameter? >> max_mpls_depth and recirc logically belong together. > > > Sure. Would you like me to send a fresh version with this change for > review? >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
