> On 29 Mar 2023, at 16:21, Dumitru Ceara <[email protected]> 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.

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?

Thanks,
Abhiram

> Regards,
> Dumitru
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to