> > >> >> So with the LB series, I need to store the value of ct_label and ct_mark >> in a register and then load it from there when I finally do ct_commit. >> ct_label is 32 bits or one register and ct_mark is 128 bits or 4 registers >> for a total of 5 registers. We do not have that many registers at all. With >> this patch, one can set ct_mark or ct_label to any value in logical flows >> which means that I am forced to use 5 registers. What I am saying is that >> if we get rid of ct_mark from this patch and make ct_label to only be set >> to one value, I can confidently use a single register for all of this. >> >> I guess what you are saying is that even though this patch is general >> purpose, we only use one bit of ct_label for ACL. But in the code of >> ovn-controller, it no longer will be general purpose and I will only have >> to read one bit from the set logical flow for ct_label and ignore ct_mark. >> That would look odd, no? >> > > 1) General capabilities of the logical flow syntax. That's what this > patch adds. It doesn't really say anything about how it's used. I *think* > you're saying it looks like this could be used in a way that would be > painful for implementing multiple stateful services. It's great to > consider the needs of ovn-northd today, but the syntax is a more general > capability. >
Correct. > Someone could throw ovn-northd away and write their own logical flows, > perhaps. > Right. Or 2 weeks from now, some other developer can implement a feature that sets a logical flow as ct_commit (ct_label = 0x3002, ct_mark = 0x3333) in ovn-northd. So my suggestion was don't make it general purpose, but only limit setting ct_label to one specific value as that is all is what is needed right now. > 2) How we structure our logical flows in ovn-northd. I think that's what > you're most concerned with. > > Right now the next patch in this series makes use of this logical flow > capability to do: > > ct_commit(ct_label=1) > ... or ... > ct_commit(ct_label=0) > > I believe you were splitting stateful services up to share a ct_commit in > a later table, so we could change that to something like: > > reg0[0] = 1; > > and in the next table: > > ct_commit(ct_label=reg0[0]) > Correct. > > We could even give "reg0[0]" a nicer name by adding a new symbol like > "ct_drop_connection". > Yes. > > This is all in logical flows, not code in ovn-controller. I think this > current patch is all that's needed for ovn-controller, so it knows how to > handle this action in logical flows. > If we go by symbols, then ovn-controller will read the symbol, check whether it is a valid symbol and then convert it to a integer and then do openflow action. > > If I'm still missing something, maybe we can hop on the phone or something > tomorrow and recap the conversation on the mailing list? > I think we are close to be on the same page. But, if my answers in this mail confused this more, or if you think of a better idea or a different perspective, I am happy to jump on a call. > > -- > Russell Bryant > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
