On Tue, Aug 2, 2022 at 9:05 PM Wentao Jia <wentao....@corigine.com> wrote:

> snat/dnat rules of logical router, no logical flows for established
> connection, all natted packets deliver to kernel conntrack module by
> ct commit, this is low performance and difficult to offload.
> add another logical flows without ct commit forestablished on
> pipeline stage of unsnat/undnat for logical router
>
> before patched, datapath flows for nat with ct commit
> ufid:db1fbd1b-8f16-4681-81b0-3796d60332a8,
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),packet_type(ns=0/0,id=0/0),eth(src=fa:16:3e:a1:50:79,dst=fa:16:3e:39:69:0a),eth_type(0x0800),ipv4(src=
> 192.168.200.128/255.255.255.192,dst=1.1.1.254,proto=6,tos=0/0,ttl=64,frag=no),tcp(src=0/0,dst=0/0),
> packets:2969075, bytes:4071176800, used:0.000s, dp:tc,
> actions:set(eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43)),set(ipv4(ttl=63)),ct(commit,zone=22,nat(src=1.1.1.124)),recirc(0x14b)
> ufid:e9c5df95-02df-4629-b399-ddeb5581e997,
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0x14b),dp_hash(0/0),in_port(rep0_0),packet_type(ns=0/0,id=0/0),eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43),eth_type(0x0800),ipv4(src=
> 0.0.0.0/0.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=0/0,ttl=0/0,frag=no),
> packets:2969075, bytes:4071176800, used:0.001s, offloaded:yes, dp:tc,
> actions:ct_clear,enp1s0np1
>
> after patched, there is two flows for nat, the flow with ct commit will
> be timeout and deleted after connection established. another flow without
> ct commit for established connection
> ufid:f6a591d6-de32-49cc-bf03-7a00a7601ad0,
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),packet_type(ns=0/0,id=0/0),eth(src=fa:16:3e:a1:50:79,dst=fa:16:3e:39:69:0a),eth_type(0x0800),ipv4(src=192.168.200.165,dst=1.1.1.254,proto=6,tos=0/0,ttl=64,frag=no),tcp(src=0/0,dst=0/0),
> packets:5518542, bytes:7924612730, used:0.040s, offloaded:yes, dp:tc,
> actions:set(eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43)),set(ipv4(ttl=63)),ct(zone=22,nat),recirc(0x14d)
> ufid:6af5a3ed-5920-4f5b-923e-7e2cb5fc3d6c,
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0_0),packet_type(ns=0/0,id=0/0),eth(src=00:00:00:00:00:00/00:00:00:00:00:00,dst=00:00:00:00:00:00/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.200.165,dst=
> 0.0.0.0/0.0.0.0,proto=0/0,tos=0/0,ttl=0/0,frag=no), packets:1, bytes:60,
> used:6.530s, dp:tc,
> actions:ct(commit,zone=22,nat(src=1.1.1.166)),recirc(0x14e)
> ufid:b3505ba7-9367-4533-a5df-f1f897376c54,
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0_0),packet_type(ns=0/0,id=0/0),eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43),eth_type(0x0800),ipv4(src=
> 0.0.0.0/128.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=0/0,ttl=0/0,frag=no),
> packets:5518539, bytes:7924612529, used:0.040s, offloaded:yes, dp:tc,
> actions:ct_clear,enp1s0np1
> ufid:ac4c188c-5320-4376-a53e-b1561e5ca209,
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0x14e),dp_hash(0/0),in_port(rep0_0),packet_type(ns=0/0,id=0/0),eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43),eth_type(0x0800),ipv4(src=
> 0.0.0.0/0.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=0/0,ttl=0/0,frag=no),
> packets:1, bytes:60, used:6.531s, offloaded:yes, dp:tc,
> actions:ct_clear,enp1s0np1
>
> Signed-off-by: Wentao Jia <wentao....@corigine.com>
> ---
> northd/northd.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index d31cb1688..1e7406a72 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -12867,11 +12867,8 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
>      * Undoing SNAT has to happen before DNAT processing.  This is
>      * because when the packet was DNATed in ingress pipeline, it did
>      * not know about the possibility of eventual additional SNAT in
> -    * egress pipeline. */
> -    if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) {
> -        return;
> -    }
> -
> +    * egress pipeline.
> +    */
>      bool stateless = lrouter_nat_is_stateless(nat);
>      if (od->is_gw_router) {
>          ds_clear(match);
> @@ -13036,8 +13033,7 @@ build_lrouter_out_undnat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
>      *
>      * Note that this only applies for NAT on a distributed router.
>      */
> -    if (!od->n_l3dgw_ports ||
> -        (strcmp(nat->type, "dnat") && strcmp(nat->type,
> "dnat_and_snat"))) {
> +    if (!od->n_l3dgw_ports) {
>          return;
>      }
>
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Hi,

thank you for the patch. I am afraid that in current form
the patch is not right. The changes presented here are adding additional
undnat for snat
and vice versa. I am not entirely sure how this helps with established
connections. Could you please
elaborate more on what are you trying to achieve?

Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

amu...@redhat.com    IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to