On Wed, Nov 25, 2015 at 04:19:40PM -0800, Jarno Rajahalme wrote: > > > On Nov 24, 2015, at 4:22 PM, Ben Pfaff <b...@ovn.org> wrote: > > > > On Tue, Nov 24, 2015 at 10:21:41AM -0800, Jarno Rajahalme wrote: > >> > >>> On Nov 24, 2015, at 10:15 AM, Jarno Rajahalme <ja...@ovn.org> wrote: > >>> > >>> > >>>> On Nov 24, 2015, at 9:40 AM, Jarno Rajahalme <ja...@ovn.org> wrote: > >>>> > >>>> > >>>>> On Nov 24, 2015, at 9:25 AM, Ben Pfaff <b...@ovn.org> wrote: > >>>>> > >>>>> On Thu, Nov 19, 2015 at 01:33:18PM -0800, Jarno Rajahalme wrote: > >>>>>> Currently ovs-ofctl replace-flows and diff-flows commands only support > >>>>>> flows in table 0. Extend this to cover all possible tables. > >>>>>> > >>>>>> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> > >>>>> > >>>>> There's one oddity that may deserve consideration. It depends on how > >>>>> careful we want to be. > >>>>> > >>>>> OpenFlow 1.0 does not define a way to add a flow to a particular table. > >>>>> The switch is responsible for deciding which table is most appropriate > >>>>> for a given flow. For example, a switch might have one table that > >>>>> supports wildcards and another one that is exact-match (this is in fact > >>>>> specifically envisioned by OF1.0 through its insistence that exact-match > >>>>> flows have the highest priority). > >>>>> > >>>>> This means that when talking to an OF1.0 switch, "ovs-ofctl > >>>>> replace-flows" (and friends) should ignore the table number. If > >>>>> a flow on the switch is in table 1, but the input file says it is in > >>>>> table 0 (probably because it doesn't specify a table at all), ovs-ofctl > >>>>> should do nothing, because that's the desired state. > >>>>> > >>>> > >>>> So for an OF1.0 switch without the Table ID extension we should ignore > >>>> table numbers both ways, when reading from the file and when reading > >>>> from the switch, essentially pretend that there is only one table? > >>>> > >>>>> However, for practically forever, OVS has had special extensions to > >>>>> allow control over the table in which a flow lives. This means that if > >>>>> ovs-ofctl is talking to OVS, even in OpenFlow 1.0, it should place flows > >>>>> where the user requested and should not ignore the table numbers. > >>>>> > >>>>> This distinction is reflected through ofputil_protocol values. If a > >>>>> switch supports OFPUTIL_P_OF10_STD_TID or OFPUTIL_P_OF10_NXM_TID, then > >>>>> ovs-ofctl can place flows arbitrarily; if it only supports > >>>>> OFPUTIL_P_OF10_STD (or, theoretically, only OFPUTIL_P_OF10_NXM), then it > >>>>> is just a plain OF1.0 switch and all of the tables should be treated > >>>>> alike. > >>>>> > >>>>> OF1.1+ all support placing flows where the user requests. > >>>>> > >>>>> It's probably not too hard to support this, and possibly it is > >>>>> worthwhile. > >>>>> > >>> > >>> IMO this could be cleaner if the choice of protocol is driven by the > >>> input file. If the file has any flow with a non-zero or non-all table > >>> number, then we restrict the choice of protocols to ones that support > >>> multiple tables. Sounds reasonable? > >>> > >> > >> parse_ofp_str() already does this: > >> > >> if (!strcmp(name, "table")) { > >> error = str_to_u8(value, "table", &fm->table_id); > >> if (fm->table_id != 0xff) { > >> *usable_protocols &= OFPUTIL_P_TID; > >> } > >> } > >> > >> Here even “table=0” restricts vanilla OF1.0 out, which I think is the > >> right thing to do. > >> > >> So it turns out OF1.0 without table extension is already taken care of by > >> restricting the choice of protocol. > > > > Hmm. Well, OK, we're no more wrong than we were before then. > > Sounds like the dump-flows printing out table=0 for OF1.0 should be > fixed in a separate patch? If so, you think this patch is ready to go?
I'm not sure that's the correct resolution but I do think it's reasonable to resolve it separately. Acked-by: Ben Pfaff <b...@ovn.org> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev