On Mon, Jun 10, 2024 at 2:29 PM Naveen Yerramneni < [email protected]> wrote:
> > > > On 10 Jun 2024, at 5:02 PM, Ales Musil <[email protected]> wrote: > > > > CAUTION: External Email > > > > > > On Mon, Jun 10, 2024 at 10:30 AM Naveen Yerramneni < > [email protected]> wrote: > > Action length is getting set incorrectly during ct_commit encode > > due to which ct action is getting skipped during phsycial flows > > creation. This issue is noticed only if ct_commit is prefixed > > with other actions. > > > > Logical Flows > > ============= > > uuid=0xb4afcd34, table=8 (ls_in_acl_eval ), priority=2001 , > match=(reg0[10] == 1 && (udp)), action=(reg8[17] = 1; ct_commit { > ct_mark.blocked = 1; }; next;) > > > > Physical Flows > > ============== > > Without fix: > > ----------- > > cookie=0xb4afcd34, duration=0.016s, table=16, n_packets=0, n_bytes=0, > idle_age=0, priority=2001,udp,reg0=0x400/0x400,metadata=0x1 > actions=load:0x1->OXM_OF_PKT_REG4[49],resubmit(,17) > > cookie=0xb4afcd34, duration=0.016s, table=16, n_packets=0, n_bytes=0, > idle_age=0, priority=2001,udp6,reg0=0x400/0x400,metadata=0x1 > actions=load:0x1->OXM_OF_PKT_REG4[49],resubmit(,17) > > > > With fix: > > -------- > > cookie=0x54dea272, duration=0.014s, table=16, n_packets=0, n_bytes=0, > idle_age=0, priority=2001,udp,reg0=0x400/0x400,metadata=0x1 > actions=load:0x1->OXM_OF_PKT_REG4[49],ct(commit,zone=NXM_NX_REG13[0..15],nat(src),exec(load:0x1->NXM_NX_CT_MARK[0])),resubmit(,17) > > cookie=0x54dea272, duration=0.014s, table=16, n_packets=0, n_bytes=0, > idle_age=0, priority=2001,udp6,reg0=0x400/0x400,metadata=0x1 > actions=load:0x1->OXM_OF_PKT_REG4[49],ct(commit,zone=NXM_NX_REG13[0..15],nat(src),exec(load:0x1->NXM_NX_CT_MARK[0])),resubmit(,17) > > > > Signed-off-by: Naveen Yerramneni <[email protected]> > > --- > > > > > > Hi Naveen, > > > > thank you for the patch. Could you also please add a test into ovn.at [ > ovn.at] "action parsing"? This should have been really caught there first. > > Hi Ales, > > Thanks. I will add the test to "action parsing”. Shall I remove the new > test that I added since it will be a duplicate ? > Yeah, I think having the "action parsing" test should be enough. > > Thanks, > Naveen > > > > > > > lib/actions.c | 3 +++ > > tests/ovn.at [ovn.at] | 41 +++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 44 insertions(+) > > > > diff --git a/lib/actions.c b/lib/actions.c > > index 361d55009..794d2e916 100644 > > --- a/lib/actions.c > > +++ b/lib/actions.c > > @@ -760,6 +760,8 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on, > > const struct ovnact_encode_params *ep OVS_UNUSED, > > struct ofpbuf *ofpacts) > > { > > + size_t ct_offset = ofpacts->size; > > + ofpbuf_pull(ofpacts, ct_offset); > > struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); > > ct->flags = NX_CT_F_COMMIT; > > ct->recirc_table = NX_CT_RECIRC_NONE; > > @@ -791,6 +793,7 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on, > > ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset); > > ct = ofpacts->header; > > ofpact_finish(ofpacts, &ct->ofpact); > > + ofpbuf_push_uninit(ofpacts, ct_offset); > > } > > > > static void > > diff --git a/tests/ovn.at [ovn.at] b/tests/ovn.at [ovn.at] > > index dc6aafd53..d4fecdb4e 100644 > > --- a/tests/ovn.at [ovn.at] > > +++ b/tests/ovn.at [ovn.at] > > @@ -2415,6 +2415,47 @@ OVN_CLEANUP([hv-1],[hv-2]) > > AT_CLEANUP > > ]) > > > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([ACLs -- stateful and drop]) > > +ovn_start > > +check ovn-nbctl ls-add sw0 > > +check ovn-nbctl lsp-add sw0 sw0-port1 > > +check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 > 192.168.0.2" > > + > > +check ovn-nbctl --label=1 acl-add sw0 from-lport 1001 'tcp' > allow-related > > +check ovn-nbctl acl-add sw0 from-lport 1001 'udp' drop > > + > > +ovn-nbctl --wait=hv sync > > + > > +net_add n1 > > +sim_add hv1 > > +as hv1 > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.1 > > +ovs-vsctl add-port br-int p1 -- \ > > + set Interface p1 external_ids:iface-id=sw0-port1 > > + > > +wait_for_ports_up > > + > > +acl_eval_table=$(ovn-debug lflow-stage-to-oftable ls_in_acl_eval) > > +acl_action_table=$(ovn-debug lflow-stage-to-oftable ls_in_acl_action) > > + > > +ovn-sbctl --uuid lflow-list sw0 >lflows > > +ovs-ofctl dump-flows br-int table=$acl_eval_table >pflows > > +AT_CAPTURE_FILE([lflows]) > > +AT_CAPTURE_FILE([pflows]) > > + > > + > > +AT_CHECK_UNQUOTED([cat pflows | grep -v NXST | grep "NXM_NX_CT_MARK" | > cut -d ' ' -f8-10 | sort], [0], [dnl > > +priority=2001,udp,reg0=0x400/0x400,metadata=0x1 > actions=load:0x1->OXM_OF_PKT_REG4[[49]],ct(commit,zone=NXM_NX_REG13[[0..15]],nat(src),exec(load:0x1->NXM_NX_CT_MARK[[0]])),resubmit(,$acl_action_table) > > +priority=2001,udp6,reg0=0x400/0x400,metadata=0x1 > actions=load:0x1->OXM_OF_PKT_REG4[[49]],ct(commit,zone=NXM_NX_REG13[[0..15]],nat(src),exec(load:0x1->NXM_NX_CT_MARK[[0]])),resubmit(,$acl_action_table) > > +]) > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > +]) > > + > > # 3 hypervisors, one logical switch, 3 logical ports per hypervisor > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([3 HVs, 1 LS, 3 lports/HV]) > > -- > > 2.36.6 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev [ > mail.openvswitch.org] > > > > > > Thanks, > > Ales > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA [redhat.com][email protected] [red.ht] > > > Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
