OK, thanks. I'll mark this series as "superseded" in patchwork.
On Tue, Oct 04, 2016 at 12:42:19AM +0000, Daniele Di Proietto wrote: > I'm rebasing the series and considering a different approach for this. > > I'll send something shortly. > > Thanks, > > Daniele > > On 03/10/2016 17:38, "Ben Pfaff" <b...@ovn.org> wrote: > > >Does this patch need anything more? I don't see it on master and it's > >still in patchwork. Same thing for patch 4/5 in this series. > > > >On Thu, Sep 01, 2016 at 01:31:06AM +0000, Daniele Di Proietto wrote: > >> > >> On 31/08/2016 11:32, "Jarno Rajahalme" <ja...@ovn.org> wrote: > >> > >> >I’d put the registers and metadata field to the ‘false’ and also maybe > >> >non-writeable fields (ether type, frags, nw_proto, etc.) in to > >> >OVS_NOT_REACHED() case, just in case. > >> > > >> > Jarno > >> > >> Agreed, done > >> > >> Thanks, > >> > >> Daniele > >> > >> > > >> >> On Aug 31, 2016, at 10:38 AM, Jesse Gross <je...@kernel.org> wrote: > >> >> > >> >> On Tue, Aug 30, 2016 at 6:47 PM, Daniele Di Proietto > >> >> <diproiet...@vmware.com> wrote: > >> >>> When translating a set action we also unwildcard the field in question. > >> >>> This is done to correctly translate set actions with the value > >> >>> identical > >> >>> to the ingress flow, like in the following example: > >> >>> > >> >>> flow table: > >> >>> > >> >>> tcp,actions=set_field:80->tcp_dst,output:5 > >> >>> > >> >>> ingress packet: > >> >>> > >> >>> ...,tcp,tcp_dst=80 > >> >>> > >> >>> datapath flow > >> >>> > >> >>> ...,tcp(dst=80) actions:5 > >> >>> > >> >>> The datapath flow must exact match the target field, because the > >> >>> actions > >> >>> do not include a set field. (Otherwise a packet coming in with a > >> >>> different tcp_dst would be matched, and its port wouldn't be altered). > >> >>> > >> >>> Tunnel attributes behave differently: at the datapath layer, before > >> >>> action processing they're cleared (we do the same at the ofproto layer > >> >>> in xlate_actions()). Therefore there's no need to unwildcard them, > >> >>> because a set action would always be detected (since we zero them at > >> >>> the > >> >>> beginning of xlate_ations()). > >> >>> > >> >>> This fixes a problem related to the handling of Geneve options. > >> >>> Unwildcarding non existing Geneve options (as done by a > >> >>> set_field:X->tun_metadata<n> on a packet coming from a non-tunnel > >> >>> interface) would be problematic for the datapaths: the ODP translation > >> >>> functions cannot express a match on non existing Geneve options (unlike > >> >>> on other attributes), and the userspace datapath wouldn't be able to > >> >>> translate them to "userspace datapath format". In both cases the > >> >>> installed flow would be deleted by revalidation at the first > >> >>> opportunity. > >> >>> > >> >>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com> > >> >> > >> >> I think there might be some more obscure ways of triggering this > >> >> problem that still exist. Basically anything that can use a register > >> >> as a target is a potential issue. For example, stack_pop, bundle, and > >> >> multipath all look like they have the same masking behavior. > >> >> > >> >> I do have a general solution in this patch (look at the bottom of > >> >> xlate_actions() where it is adjusting the wildcards): > >> >> http://openvswitch.org/pipermail/dev/2016-August/078484.html > >> >> > >> >> I didn't realize that it was addressing an existing issue though and > >> >> that patch certainly isn't suitable for anything other than master. > >> >> > >> >> I'm also not really a big fan of the way I handled it there since it's > >> >> a pretty coarse way to do it. I would be happy to drop it if we can > >> >> feel comfortable that we got all of the callsites (now and in the > >> >> future) using your method. Perhaps we can just create a helper > >> >> function with this check builtin and then use it everywhere? That > >> >> might be enough to be confident about the future. > >> >> _______________________________________________ > >> >> dev mailing list > >> >> dev@openvswitch.org > >> >> http://openvswitch.org/mailman/listinfo/dev > >> > > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
