2016-06-09 17:46 GMT-07:00 Jesse Gross <[email protected]>: > When we generate wildcards for upcalled flows, the flows and therefore > the wildcards, are in OpenFlow format. These are mostly the same but > one exception is the input port. We work around this problem by simply > performing an exact match on the input port when generating netlink > formatted keys. (This does not lose any information in practice because > action translation also always exact matches on input port.) > > While this works fine for kernel based flows, it misses the userspace > datapath, which directly consumes the OFP format mask for the input > port. The effect of this is that the in_port mask is sometimes only > the lower 16 bits of the field. (This is because OFP format is a 16-bit > value stored in a 32-bit field. The full width of the field is initialized > with an exact match mask but certain operations result in cleaving this > down to 16 bits.) In practice this does not cause a problem because > datapath > port numbers are almost always in the lower 16 bits of the range anyways. > > This moves the masking of the datapath format field to translation so that > all datapaths see the same result. This also makes more sense conceptually > as the input port in the flow is also in ODP format at this stage. > > Signed-off-by: Jesse Gross <[email protected]> > --- > ofproto/ofproto-dpif-upcall.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index a18fc5a..9400ef9 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -1083,7 +1083,16 @@ upcall_xlate(struct udpif *udpif, struct upcall > *upcall, > > upcall->dump_seq = seq_read(udpif->dump_seq); > upcall->reval_seq = seq_read(udpif->reval_seq); > + > xlate_actions(&xin, &upcall->xout); > + if (wc) { > + /* Convert the input port wildcard from OFP to ODP format. > There's no > + * real way to do this for arbitrary bitmasks since the numbering > spaces > + * aren't the same. However, flow translation always exact > matches the > + * whole thing, so we can do the same here. */ > + WC_MASK_FIELD(wc, in_port.odp_port); > + } > + >
I guess the above could be done in xlate_wc_finish(). Either way this looks good to me. Acked-by: Daniele Di Proietto <[email protected]> Thanks > upcall->xout_initialized = true; > > if (!upcall->xout.slow) { > @@ -1489,7 +1498,7 @@ ukey_create_from_upcall(struct upcall *upcall, > struct flow_wildcards *wc) > atomic_read_relaxed(&enable_megaflows, &megaflow); > ofpbuf_use_stack(&maskbuf, &maskstub, sizeof maskstub); > if (megaflow) { > - odp_parms.odp_in_port = ODPP_NONE; > + odp_parms.odp_in_port = wc->masks.in_port.odp_port; > odp_parms.key_buf = &keybuf; > > odp_flow_key_from_mask(&odp_parms, &maskbuf); > -- > 2.5.0 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
