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