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
"action parsing"? This should have been really caught there first.
lib/actions.c | 3 +++
> tests/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 b/tests/ovn.at
> index dc6aafd53..d4fecdb4e 100644
> --- a/tests/ovn.at
> +++ b/tests/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
>
>
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
