[ovs-dev] Enforce datapath and port key constraints in vxlan mode

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index baaddb73e..644f04aa5 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1370,7 +1370,8 @@  ovn_datapath_allocate_key(struct northd_context *ctx,
 }
 
 static void
-ovn_datapath_assign_requested_tnl_id(struct hmap *dp_tnlids,
+ovn_datapath_assign_requested_tnl_id(struct northd_context *ctx,
+                                     struct hmap *dp_tnlids,
                                      struct ovn_datapath *od)
 {
     const struct smap *other_config = (od->nbs
@@ -1378,6 +1379,13 @@  ovn_datapath_assign_requested_tnl_id(struct hmap *dp_tnlids,
                                        : &od->nbr->options);
     uint32_t tunnel_key = smap_get_int(other_config, "requested-tnl-key", 0);
     if (tunnel_key) {
+        if (is_vxlan_mode(ctx->ovnsb_idl) && tunnel_key >= 1 << 12) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+            VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for datapath %s is "
+                         "incompatible with VXLAN", tunnel_key,
+                         od->nbs ? od->nbs->name : od->nbr->name);
+            return;
+        }
         if (ovn_add_tnlid(dp_tnlids, tunnel_key)) {
             od->tunnel_key = tunnel_key;
         } else {
@@ -1407,10 +1415,10 @@  build_datapaths(struct northd_context *ctx, struct hmap *datapaths,
     struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids);
     struct ovn_datapath *od, *next;
     LIST_FOR_EACH (od, list, &both) {
-        ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od);
+        ovn_datapath_assign_requested_tnl_id(ctx, &dp_tnlids, od);
     }
     LIST_FOR_EACH (od, list, &nb_only) {
-        ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od);
+        ovn_datapath_assign_requested_tnl_id(ctx, &dp_tnlids, od);
     }
 
     /* Keep nonconflicting tunnel IDs that are already assigned. */
@@ -3815,27 +3823,40 @@  ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key)
 }
 
 static void
-ovn_port_assign_requested_tnl_id(struct ovn_port *op)
+ovn_port_assign_requested_tnl_id(struct northd_context *ctx,
+                                 struct ovn_port *op)
 {
     const struct smap *options = (op->nbsp
                                   ? &op->nbsp->options
                                   : &op->nbrp->options);
     uint32_t tunnel_key = smap_get_int(options, "requested-tnl-key", 0);
-    if (tunnel_key && !ovn_port_add_tnlid(op, tunnel_key)) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-        VLOG_WARN_RL(&rl, "Logical %s port %s requests same tunnel key "
-                     "%"PRIu32" as another LSP or LRP",
-                     op->nbsp ? "switch" : "router",
-                     op_get_name(op), tunnel_key);
+    if (tunnel_key) {
+        if (is_vxlan_mode(ctx->ovnsb_idl) &&
+                tunnel_key >= OVN_VXLAN_MIN_MULTICAST) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+            VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for port %s "
+                         "is incompatible with VXLAN",
+                         tunnel_key, op_get_name(op));
+            return;
+        }
+        if (!ovn_port_add_tnlid(op, tunnel_key)) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+            VLOG_WARN_RL(&rl, "Logical %s port %s requests same tunnel key "
+                         "%"PRIu32" as another LSP or LRP",
+                         op->nbsp ? "switch" : "router",
+                         op_get_name(op), tunnel_key);
+        }
     }
 }
 
 static void
-ovn_port_allocate_key(struct hmap *ports, struct ovn_port *op)
+ovn_port_allocate_key(struct northd_context *ctx, struct hmap *ports,
+                      struct ovn_port *op)
 {
     if (!op->tunnel_key) {
+        uint8_t key_bits = is_vxlan_mode(ctx->ovnsb_idl)? 12 : 16;
         op->tunnel_key = ovn_allocate_tnlid(&op->od->port_tnlids, "port",
-                                            1, (1u << 15) - 1,
+                                            1, (1u << (key_bits - 1)) - 1,
                                             &op->od->port_key_hint);
         if (!op->tunnel_key) {
             if (op->sb) {
@@ -3875,10 +3896,10 @@  build_ports(struct northd_context *ctx,
     /* Assign explicitly requested tunnel ids first. */
     struct ovn_port *op, *next;
     LIST_FOR_EACH (op, list, &both) {
-        ovn_port_assign_requested_tnl_id(op);
+        ovn_port_assign_requested_tnl_id(ctx, op);
     }
     LIST_FOR_EACH (op, list, &nb_only) {
-        ovn_port_assign_requested_tnl_id(op);
+        ovn_port_assign_requested_tnl_id(ctx, op);
     }
 
     /* Keep nonconflicting tunnel IDs that are already assigned. */
@@ -3890,10 +3911,10 @@  build_ports(struct northd_context *ctx,
 
     /* Assign new tunnel ids where needed. */
     LIST_FOR_EACH_SAFE (op, next, list, &both) {
-        ovn_port_allocate_key(ports, op);
+        ovn_port_allocate_key(ctx, ports, op);
     }
     LIST_FOR_EACH_SAFE (op, next, list, &nb_only) {
-        ovn_port_allocate_key(ports, op);
+        ovn_port_allocate_key(ctx, ports, op);
     }
 
     /* For logical ports that are in both databases, update the southbound
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index ff92c989c..80704d282 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -8334,10 +8334,18 @@  relation OvnMaxDpKeyLocal[integer]
 OvnMaxDpKeyLocal[oVN_MAX_DP_VXLAN_KEY()] :- IsVxlanMode[true].
 OvnMaxDpKeyLocal[oVN_MAX_DP_KEY() - oVN_MAX_DP_GLOBAL_NUM()] :- IsVxlanMode[false].
 
-function get_dp_tunkey(map: Map<string,string>, key: string): Option<integer> {
+relation OvnPortKeyBits[bit<32>]
+OvnPortKeyBits[12] :- IsVxlanMode[true].
+OvnPortKeyBits[16] :- IsVxlanMode[false].
+
+relation OvnDpKeyBits[bit<32>]
+OvnDpKeyBits[12] :- IsVxlanMode[true].
+OvnDpKeyBits[24] :- IsVxlanMode[false].
+
+function get_dp_tunkey(map: Map<string,string>, key: string, bits: bit<32>): Option<integer> {
     map.get(key)
        .and_then(parse_dec_u64)
-       .and_then(|x| if (x > 0 and x < (2<<24)) {
+       .and_then(|x| if (x > 0 and x < (1<<bits)) {
                          Some{x}
                      } else {
                          None
@@ -8347,11 +8355,13 @@  function get_dp_tunkey(map: Map<string,string>, key: string): Option<integer> {
 // Tunnel keys requested by datapaths.
 relation RequestedTunKey(datapath: uuid, tunkey: integer)
 RequestedTunKey(uuid, tunkey) :-
+    OvnDpKeyBits[bits],
     ls in &nb::Logical_Switch(._uuid = uuid),
-    Some{var tunkey} = get_dp_tunkey(ls.other_config, "requested-tnl-key").
+    Some{var tunkey} = get_dp_tunkey(ls.other_config, "requested-tnl-key", bits).
 RequestedTunKey(uuid, tunkey) :-
+    OvnDpKeyBits[bits],
     lr in nb::Logical_Router(._uuid = uuid),
-    Some{var tunkey} = get_dp_tunkey(lr.options, "requested-tnl-key").
+    Some{var tunkey} = get_dp_tunkey(lr.options, "requested-tnl-key", bits).
 Warning[message] :-
     RequestedTunKey(datapath, tunkey),
     var count = datapath.group_by((tunkey)).size(),
@@ -8413,13 +8423,13 @@  TunKeyAllocation(datapath, tunkey) :-
 /*
  * Port id allocation:
  *
- * Port IDs in a per-datapath space in the range 1...2**15-1
+ * Port IDs in a per-datapath space in the range 1...2**(bits-1)-1, where
+ * bits is the number of bits available for port keys (default: 16, vxlan: 12)
  */
-
-function get_port_tunkey(map: Map<string,string>, key: string): Option<integer> {
+function get_port_tunkey(map: Map<string,string>, key: string, bits: bit<32>): Option<integer> {
     map.get(key)
        .and_then(parse_dec_u64)
-       .and_then(|x| if (x > 0 and x < (2<<15)) {
+       .and_then(|x| if (x > 0 and x < (1<<bits)) {
                          Some{x}
                      } else {
                          None
@@ -8429,15 +8439,17 @@  function get_port_tunkey(map: Map<string,string>, key: string): Option<integer>
 // Tunnel keys requested by port bindings.
 relation RequestedPortTunKey(datapath: uuid, port: uuid, tunkey: integer)
 RequestedPortTunKey(datapath, port, tunkey) :-
+    OvnPortKeyBits[bits],
     sp in &SwitchPort(),
     var datapath = sp.sw._uuid,
     var port = sp.lsp._uuid,
-    Some{var tunkey} = get_port_tunkey(sp.lsp.options, "requested-tnl-key").
+    Some{var tunkey} = get_port_tunkey(sp.lsp.options, "requested-tnl-key", bits).
 RequestedPortTunKey(datapath, port, tunkey) :-
+    OvnPortKeyBits[bits],
     rp in &RouterPort(),
     var datapath = rp.router._uuid,
     var port = rp.lrp._uuid,
-    Some{var tunkey} = get_port_tunkey(rp.lrp.options, "requested-tnl-key").
+    Some{var tunkey} = get_port_tunkey(rp.lrp.options, "requested-tnl-key", bits).
 Warning[message] :-
     RequestedPortTunKey(datapath, port, tunkey),
     var count = port.group_by((datapath, tunkey)).size(),
diff --git a/tests/ovn.at b/tests/ovn.at
index 30625ec37..6c0d44de1 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -3690,6 +3690,55 @@  done
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([VXLAN check port/datapath key space limits])
+
+ovn_start
+net_add net
+check ovs-vsctl add-br br-phys
+check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovn_attach net br-phys 192.168.0.1 24 vxlan
+check ovn-nbctl --wait=sb sync
+
+check ovn-nbctl ls-add ls-bad -- \
+    set Logical_Switch ls-bad other_config:requested-tnl-key=5000
+check ovn-nbctl lsp-add ls-bad lsp-bad -- \
+    set logical_switch_port lsp-bad options:requested-tnl-key=5000
+check ovn-nbctl --wait=sb sync
+
+check ovs-vsctl add-port br-int vif-bad -- \
+    set Interface vif-bad external-ids:iface-id=lsp-bad
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lsp-bad` = xup])
+
+# 5000 is higher than 1 << 12, so OVN should ignore the key requests
+AT_CHECK([ovn-sbctl get Datapath_Binding ls-bad tunnel_key], [0], [dnl
+1
+])
+AT_CHECK([ovn-sbctl get Port_Binding lsp-bad tunnel_key], [0], [dnl
+1
+])
+
+check ovn-nbctl ls-add ls-good -- \
+    set Logical_Switch ls-good other_config:requested-tnl-key=1000
+check ovn-nbctl lsp-add ls-good lsp-good -- \
+    set logical_switch_port lsp-good options:requested-tnl-key=1000
+check ovn-nbctl --wait=sb sync
+
+check ovs-vsctl add-port br-int vif-good -- \
+    set Interface vif-good external-ids:iface-id=lsp-good
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lsp-good` = xup])
+
+# 1000 is lower than 1 << 12, so OVN should honor the key requests
+AT_CHECK([ovn-sbctl get Datapath_Binding ls-good tunnel_key], [0], [dnl
+1000
+])
+AT_CHECK([ovn-sbctl get Port_Binding lsp-good tunnel_key], [0], [dnl
+1000
+])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([2 HVs, 1 LS, no switching between multiple localnet ports with different tags])
 ovn_start