Thanks Numan, it looks good to me!

Acked-by: Mark Michelson <[email protected]>

On Mon, Jan 26, 2026 at 11:31 PM <[email protected]> wrote:
>
> From: Numan Siddique <[email protected]>
>
> Similar to router ports and localnet ports, we need to skip
> switch ports from conntrack checks.  Otherwise the packets
> received from remote ports of a spine switch will enter the peer
> switch's ingress pipeline and will be marked as inv when sent
> to conntrack and dropped.
>
> Signed-off-by: Numan Siddique <[email protected]>
> ---
>  northd/northd.c     | 16 ++++++++++++++++
>  northd/northd.h     |  3 ++-
>  tests/ovn-northd.at | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index adaa94e859..e998c80817 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -561,6 +561,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct 
> uuid *key,
>      sset_init(&od->router_ips);
>      od->ls_peers = VECTOR_EMPTY_INITIALIZER(struct ovn_datapath *);
>      od->router_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
> +    od->switch_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
>      od->l3dgw_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
>      od->localnet_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
>      od->lb_with_stateless_mode = false;
> @@ -589,6 +590,7 @@ ovn_datapath_destroy(struct ovn_datapath *od)
>          ovn_destroy_tnlids(&od->port_tnlids);
>          destroy_ipam_info(&od->ipam_info);
>          vector_destroy(&od->router_ports);
> +        vector_destroy(&od->switch_ports);
>          vector_destroy(&od->ls_peers);
>          vector_destroy(&od->localnet_ports);
>          vector_destroy(&od->l3dgw_ports);
> @@ -1616,6 +1618,10 @@ join_logical_ports_lsp(struct hmap *ports,
>          vector_push(&od->localnet_ports, &op);
>      }
>
> +    if (lsp_is_switch(nbsp)) {
> +        vector_push(&od->switch_ports, &op);
> +    }
> +
>      if (lsp_is_vtep(nbsp)) {
>          od->has_vtep_lports = true;
>      }
> @@ -6210,6 +6216,11 @@ build_ls_stateful_rec_pre_acls(
>                                       S_SWITCH_IN_PRE_ACL, 
> S_SWITCH_OUT_PRE_ACL,
>                                       110, lflows, lflow_ref);
>          }
> +        VECTOR_FOR_EACH (&od->switch_ports, op) {
> +            skip_port_from_conntrack(od, op, true,
> +                                     S_SWITCH_IN_PRE_ACL, 
> S_SWITCH_OUT_PRE_ACL,
> +                                     110, lflows, lflow_ref);
> +        }
>          struct ovn_port *lp;
>          VECTOR_FOR_EACH (&od->localnet_ports, lp) {
>              skip_port_from_conntrack(od, lp, true, S_SWITCH_IN_PRE_ACL,
> @@ -6420,6 +6431,11 @@ build_ls_stateful_rec_pre_lb(const struct 
> ls_stateful_record *ls_stateful_rec,
>                                   S_SWITCH_IN_PRE_LB, S_SWITCH_OUT_PRE_LB,
>                                   110, lflows, lflow_ref);
>      }
> +    VECTOR_FOR_EACH (&od->switch_ports, op) {
> +        skip_port_from_conntrack(od, op, true,
> +                                 S_SWITCH_IN_PRE_LB, S_SWITCH_OUT_PRE_LB,
> +                                 110, lflows, lflow_ref);
> +    }
>
>      /* Localnet ports have no need for going through conntrack, unless
>       * the logical switch has a load balancer. Then, conntrack is necessary
> diff --git a/northd/northd.h b/northd/northd.h
> index e4d7de9f9e..eb5c15f34f 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -419,7 +419,8 @@ struct ovn_datapath {
>
>      /* Logical switch data. */
>      struct vector router_ports; /* Vector of struct ovn_port *. */
> -
> +    struct vector switch_ports; /* Vector of struct ovn_port * of
> +                                 * type 'switch'. */
>      struct hmap port_tnlids;
>      uint32_t port_key_hint;
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 0bcb5e90b3..3fa9d4b9e3 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -19337,3 +19337,38 @@ AT_CHECK([grep "lr_in_policy[[^_]]" lr0flows | 
> ovn_strip_lflows | sort], [0], [d
>  OVN_CLEANUP_NORTHD
>  AT_CLEANUP
>  ])
> +
> +AT_SETUP([Conntrack skip for switch ports connected to spine switch])
> +ovn_start
> +
> +check ovn-nbctl ls-add spine
> +check ovn-nbctl ls-add ls1
> +
> +# Connect ls1 to spine.
> +check ovn-nbctl lsp-add spine spine-to-ls1
> +check ovn-nbctl lsp-add ls1 ls1-to-spine
> +check ovn-nbctl lsp-set-type spine-to-ls1 switch peer=ls1-to-spine
> +check ovn-nbctl lsp-set-type ls1-to-spine switch peer=spine-to-ls1
> +
> +check ovn-nbctl lsp-add ls1 ls1-p1 -- \
> +lsp-set-addresses ls1-p1 "f0:00:00:01:02:01 172.16.1.1"
> +
> +check ovn-nbctl acl-add ls1 from-lport 1003 "ip4" allow-related
> +
> +
> +check ovn-nbctl --wait=sb sync
> +
> +AT_CHECK([ovn-sbctl dump-flows ls1 > ls1flows])
> +AT_CAPTURE_FILE([ls1flows])
> +
> +AT_CHECK([grep "ls1-to-spine" ls1flows | ovn_strip_lflows | sort], [0], [dnl
> +  table=??(ls_in_lookup_fdb   ), priority=100  , match=(inport == 
> "ls1-to-spine"), action=(reg0[[11]] = lookup_fdb(inport, eth.src); next;)
> +  table=??(ls_in_pre_acl      ), priority=110  , match=(ip && inport == 
> "ls1-to-spine"), action=(next;)
> +  table=??(ls_in_pre_lb       ), priority=110  , match=(ip && inport == 
> "ls1-to-spine"), action=(next;)
> +  table=??(ls_in_put_fdb      ), priority=100  , match=(inport == 
> "ls1-to-spine" && reg0[[11]] == 0), action=(put_fdb(inport, eth.src); next;)
> +  table=??(ls_out_pre_acl     ), priority=110  , match=(ip && outport == 
> "ls1-to-spine"), action=(next;)
> +  table=??(ls_out_pre_lb      ), priority=110  , match=(ip && outport == 
> "ls1-to-spine"), action=(next;)
> +])
> +
> +OVN_CLEANUP_NORTHD
> +AT_CLEANUP
> --
> 2.52.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to