On 07/06/2021 10:58, Dumitru Ceara wrote: > On 6/4/21 7:06 PM, Mark Gray wrote: >> ovn-trace does not set translated ip address for ct_dnat() >> actions when tracing. This causes the trace to end prematurely. > > Hi Mark, > > Thanks for the patch!
Thanks for the review. New series is at https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/383754.html >> >> This can be tested with the following or an equivalent for IPv6: >> >> ovn-nbctl ls-add sw0 >> ovn-nbctl lsp-add sw0 sw0-port1 >> ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 192.168.0.2" >> >> ovn-nbctl ls-add sw1 >> ovn-nbctl lsp-add sw1 sw1-port1 >> ovn-nbctl lsp-set-addresses sw1-port1 "50:54:00:00:00:03 11.0.0.2" >> >> ovn-nbctl lr-add lr0 >> ovn-nbctl lrp-add lr0 lrp0 00:00:00:00:ff:01 192.168.0.1/24 >> ovn-nbctl lsp-add sw0 lrp0-attachment >> ovn-nbctl lsp-set-type lrp0-attachment router >> ovn-nbctl lsp-set-addresses lrp0-attachment 00:00:00:00:ff:01 >> ovn-nbctl lsp-set-options lrp0-attachment router-port=lrp0 >> ovn-nbctl lrp-add lr0 lrp1 00:00:00:00:ff:02 11.0.0.1/24 -- >> lrp-set-gateway-chassis lrp1 chassis-1 >> ovn-nbctl lsp-add sw1 lrp1-attachment >> ovn-nbctl lsp-set-type lrp1-attachment router >> ovn-nbctl lsp-set-addresses lrp1-attachment 00:00:00:00:ff:02 >> ovn-nbctl lsp-set-options lrp1-attachment router-port=lrp1 >> >> ovn-nbctl lr-nat-add lr0 dnat 42.42.42.42 192.168.0.2 >> >> ovs-vsctl add-port br-int p1 -- \ >> set Interface p1 external_ids:iface-id=sw0-port1 >> ovs-vsctl add-port br-int p2 -- \ >> set Interface p2 external_ids:iface-id=sw1-port1 >> >> ovn-trace 'inport == "sw1-port1" && eth.src == 50:54:00:00:00:03 && eth.dst >> == 00:00:00:00:ff:02 && ip4.dst == 42.42.42.42 && ip4.src == 11.0.0.2 && >> ip.ttl == 64' > > We have some ovn-trace self tests already, in ovn-northd.at (for > configurations that don't need ovn-controller) and ovn.at (for more > complex scenario), so I think this could be added as a new test there. Good point. I didn't know there were already tests there. However, I think I have found them all so I will move them out into an ovn-trace.at file to make it easier for the next person to find them. > >> >> Signed-off-by: Mark Gray <mark.d.g...@redhat.com> >> --- >> utilities/ovn-trace.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c >> index 3b26b5af1d69..15b737b23496 100644 >> --- a/utilities/ovn-trace.c >> +++ b/utilities/ovn-trace.c >> @@ -2297,10 +2297,20 @@ execute_ct_nat(const struct ovnact_ct_nat *ct_nat, >> if (ct_nat->family == AF_INET) { >> ds_put_format(&s, "(ip4.%s="IP_FMT")", direction, >> IP_ARGS(ct_nat->ipv4)); >> + if (is_dst) { >> + ct_flow.nw_dst=ct_nat->ipv4; >> + } else { >> + ct_flow.nw_src=ct_nat->ipv4; >> + } > > Nit: needs spaces around '=' Thanks > >> } else { >> ds_put_format(&s, "(ip6.%s=", direction); >> ipv6_format_addr(&ct_nat->ipv6, &s); >> ds_put_char(&s, ')'); >> + if (is_dst) { >> + ct_flow.ipv6_dst=ct_nat->ipv6; >> + } else { >> + ct_flow.ipv6_src=ct_nat->ipv6; >> + } > > Nit: same here Thanks > >> } >> >> uint8_t state = is_dst ? CS_DST_NAT : CS_SRC_NAT; >> > > Thanks, > Dumitru > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
