On 4/3/23 11:47, Abhiram Sangana wrote:
>
>
>> On 29 Mar 2023, at 16:21, Dumitru Ceara <dce...@redhat.com> wrote:
>>
>> On 3/29/23 12:00, Abhiram Sangana wrote:
>>>>>> @@ -896,6 +904,26 @@ put_local_common_flows(uint32_t dp_key,
>>>>>> pb->header_.uuid.parts[0], &match, ofpacts_p,
>>>>>> &pb->header_.uuid);
>>>>>>
>>>>>> + if (zone_ids->drop) {
>>>>>> + /* Table 39, Priority 1.
>>>>>> + * =======================
>>>>>> + *
>>>>>> + * Clear the logical registers (for consistent behavior with
>>>>>> packets
>>>>>> + * that get tunneled) except MFF_LOG_ACL_DROP_ZONE. */
>>>>>> + match_init_catchall(&match);
>>>>>> + ofpbuf_clear(ofpacts_p);
>>>>>> + match_set_metadata(&match, htonll(dp_key));
>>>>>> + for (int i = 0; i < MFF_N_LOG_REGS; i++) {
>>>>>> + if ((MFF_REG0 + i) != MFF_LOG_ACL_DROP_ZONE) {
>>>>>> + put_load(0, MFF_REG0 + i, 0, 32, ofpacts_p);
>>>>>> + }
>>>>>> + }
>>>> Why do we need this? Don't we load the CT zone anyway for "to-lport"
>>>> ACLs? Can't we also load the drop zone in the same place?
>>> Yes, we are loading the drop zone (reg 9) in the same place as CT zone
>>> for “to-lport” ACLs (reg 13) but this happens before table=39 where
>>> registers 0 to 9 are cleared.
>>
>> Oh, I see now, it's because the drop zone is stored in reg9. I wanted
>> to suggest not allowing reg9 to be used as logical register anymore but
>> that's not really easy because it's used in the router pipeline:
>>
>> /* Register to store the result of check_pkt_larger action. */
>> #define REGBIT_PKT_LARGER "reg9[1]"
>> #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
>> #define REGBIT_LOOKUP_NEIGHBOR_IP_RESULT "reg9[3]"
>> #define REGBIT_DST_NAT_IP_LOCAL "reg9[4]"
>> #define REGBIT_KNOWN_ECMP_NH "reg9[5]"
>> #define REGBIT_KNOWN_LB_SESSION "reg9[6]"
>>
>> [...]
>> #define REG_ORIG_TP_DPORT_ROUTER "reg9[16..31]"
>>
>> Can we reuse one of the router-specific zones registers instead?
>>
>> #define MFF_LOG_DNAT_ZONE MFF_REG11 /* conntrack dnat zone for gateway
>> router
>> * (32 bits). */
>> #define MFF_LOG_SNAT_ZONE MFF_REG12 /* conntrack snat zone for gateway
>> router
>> * (32 bits). */
>
> Currently, we seem to be allocating SNAT and DNAT zones for
> logical switches too and loading registers 11 and 12.
>
You're right, we are. And we're also using the SNAT zone for LB
hairpin. But AFAICT we don't use the DNAT zone in the switch pipeline
(and I don't think we'll ever use it).
> bb83b543-0d9d-409b-832c-4fc235355289 is a logical_switch.
>
> $ ovn-appctl ct-zone-list
> bb83b543-0d9d-409b-832c-4fc235355289_dnat 1
> sw0p1 3
> bb83b543-0d9d-409b-832c-4fc235355289_snat 2
> bb83b543-0d9d-409b-832c-4fc235355289_drop 4
>
> cookie=0x2547c5b0, duration=27.257s, table=0, n_packets=0, n_bytes=0,
> idle_age=27, priority=100,in_port=1
> actions=load:0x3->NXM_NX_REG13[],load:0x1->NXM_NX_REG11[],load:0x2->NXM_NX_REG12[],load:0x4->NXM_NX_REG9[],load:0x1->OXM_OF_METADATA[],load:0x1->NXM_NX_REG14[],resubmit(,8)
>
> cookie=0x2547c5b0, duration=27.258s, table=38, n_packets=0, n_bytes=0,
> idle_age=27, priority=100,reg15=0x1,metadata=0x1
> actions=load:0x3->NXM_NX_REG13[],load:0x1->NXM_NX_REG11[],load:0x2->NXM_NX_REG12[],load:0x4->NXM_NX_REG9[],resubmit(,39)
>
> Is it ok to reuse these registers?
>
I think it's OK to reuse the DNAT register; Numan do you agree?
Thanks!
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
