2016-06-13 13:47 GMT-07:00 Jesse Gross <je...@kernel.org>: > On Mon, Jun 13, 2016 at 1:16 PM, Daniele Di Proietto > <diproiet...@ovn.org> wrote: > > > > 2016-06-09 17:46 GMT-07:00 Jesse Gross <je...@kernel.org>: > >> > >> 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 <je...@kernel.org> > >> --- > >> 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(). > > I did consider that but the problem is that some callers still want > the port to be in OpenFlow format, such as ofproto_trace(). Even > though this is just masking the whole field, I consider the operation > here to be "converting" it to ODP format and so more appropriate for > the upcall code. > > The other area where we have this type of split between ODP/OpenFlow > format is with tunnel metadata. That has a flag to indicate which > format it is in which is nice. As part of some larger work I'm > considering making that flag apply to both input port and tunnel > metadata, which might have some benefits for both pieces. If I end up > doing that then I'll move this into the xlate code so that it is > transparent. > > Makes sense, thanks
> > Acked-by: Daniele Di Proietto <diproiet...@vmware.com> > > Thanks, I'll this series to master and the first patch to branch-2.5 > in a minute. > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev