> On 23 Jul 2026, at 7:41 PM, Dumitru Ceara <[email protected]> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> Hi Naveen,

Hi Dumitru,

> 
> On 7/21/26 8:45 AM, Naveen Yerramneni wrote:
>> 
>> 
>>> On 20 Jul 2026, at 9:17 PM, Naveen Yerramneni 
>>> <[email protected]> wrote:
>>> 
>>> 
>>> 
>>>> On 20 Jul 2026, at 8:43 PM, Dumitru Ceara <[email protected]> wrote:
>>>> 
>>>> !-------------------------------------------------------------------|
>>>> CAUTION: External Email
>>>> 
>>>> |-------------------------------------------------------------------!
>>>> 
>>>> On 6/29/26 7:21 AM, Naveen Yerramneni wrote:
>>>>> When a Logical_Router_Port has DHCP relay enabled and multiple IPv4
>>>>> addresses configured, ovn-northd needs to pick one of them as the
>>>>> relay agent address (giaddr).  This choice was hard-coded to
>>>>> lrp_networks.ipv4_addrs[0] which depends entirely on internal
>>>>> ordering.
>>>>> 
>>>>> The 'networks' column on Logical_Router_Port is an OVSDB set whose
>>>>> elements are sorted by strcmp() on the raw CIDR strings.
>>>>> This makes lrp_networks.ipv4_addrs[0] simply whichever CIDR sorts
>>>>> first as a string, which may not match the address intended as
>>>>> the "DHCP relay" address of the port.
>>>>> 
>>>>> Add a new key 'options:primary_ipv4' on Logical_Router_Port.  When
>>>>> set, it must be an IPv4 host address that equals one of the port's
>>>>> configured addresses; ovn-northd then uses that address as the
>>>>> giaddr in all DHCP relay flows it emits for the port.  When unset,
>>>>> behavior is unchanged: ovn-northd falls back to
>>>>> lrp_networks.ipv4_addrs[0] (whichever CIDR sorts first as a string),
>>>>> so existing deployments are unaffected on upgrade.
>>>>> 
>>>>> If 'options:primary_ipv4' is set but is not a valid IPv4 address or
>>>>> does not match any address in 'networks', ovn-northd treats it as a
>>>>> misconfiguration: it logs a warning and skips emitting DHCP relay
>>>>> flows for the port rather than silently falling back to the default
>>>>> address.
>>>>> 
>>>>> This option is intended to be reusable by any feature that needs a
>>>>> single representative IPv4 for the port.
>>>>> 
>>>>> Reported-by: Dumitru Ceara <[email protected]>
>>>>> Signed-off-by: Naveen Yerramneni <[email protected]>
>>>>> Acked-by: Aditya Mehakare <[email protected]>
>>>>> Acked-by: Huzaifa Calcuttawala <[email protected]>
>>>>> Acked-by: Lorenzo Bianconi <[email protected]>
>>>>> ---
>>>>> V2:
>>>>> - Rebase on top of latest main.
>>>>> V3:
>>>>> - Addressed comment from Lorenzo.
>>>>> ---
>>>>> 
>>>>> NEWS                |  8 ++++
>>>>> northd/northd.c     | 97 ++++++++++++++++++++++++++++++++++++---------
>>>>> ovn-nb.xml          | 29 ++++++++++++++
>>>>> tests/ovn-northd.at | 76 +++++++++++++++++++++++++++++++++++
>>>>> 4 files changed, 191 insertions(+), 19 deletions(-)
>>>>> 
>>>>> diff --git a/NEWS b/NEWS
>>>>> index 582696e2c..be51e05c2 100644
>>>>> --- a/NEWS
>>>>> +++ b/NEWS
>>>>> @@ -1,5 +1,13 @@
>>>>> Post v26.03.0
>>>>> -------------
>>>>> +   - Logical_Router_Port: Added a new "options:primary_ipv4" key that
>>>>> +     selects which of the port's IPv4 addresses is treated as the port's
>>>>> +     "primary" address.  DHCP Relay uses it as the relay agent address
>>>>> +     (giaddr) in relayed DHCP packets.  When unset, ovn-northd picks the
>>>>> +     same address it picked before (whichever CIDR in "networks" sorts
>>>>> +     first as a string), so existing deployments are unaffected;
>>>>> +     deployments that need a specific address on a multi-IP port can
>>>>> +     pin it via the new option.
>>>>>  - Added Transit Switch port support with new ovn-ic-nbctl 'tsp-add',
>>>>>    'tsp-del' and 'tsp-set-addr' commands.
>>>>>  - Added ability to set any "ipsec_*" NB_Global option to configure the
>>>>> diff --git a/northd/northd.c b/northd/northd.c
>>>>> index aa332c449..47cc6e7d3 100644
>>>>> --- a/northd/northd.c
>>>>> +++ b/northd/northd.c
>>>>> @@ -10073,6 +10073,9 @@ ls_dhcp_relay_port(const struct ovn_datapath *od)
>>>>>   return smap_get(&od->nbs->other_config, "dhcp_relay_port");
>>>>> }
>>>>> 
>>>>> +static const struct ipv4_netaddr *lrp_get_primary_ipv4_addr(
> 
> Please see lower for the reasoning, this would then have to be renamed
> to lrp_get_dhcpv4_primary_ip().
> 
>>>>> +    const struct ovn_port *op);
>>>>> +
>>>>> static void
>>>>> build_lswitch_dhcp_relay_flows(struct ovn_port *op,
>>>>>                              const struct hmap *ls_ports,
>>>>> @@ -10112,6 +10115,12 @@ build_lswitch_dhcp_relay_flows(struct ovn_port 
>>>>> *op,
>>>>>       return;
>>>>>   }
>>>>> 
>>>>> +    /* Use the LRP's primary IPv4 addr for the DHCP relay. */
>>>>> +    const struct ipv4_netaddr *lrp_ipv4_addr = 
>>>>> lrp_get_primary_ipv4_addr(rp);
>>>>> +    if (!lrp_ipv4_addr) {
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>>   char *server_ip_str = NULL;
>>>>>   uint16_t port;
>>>>>   int addr_family;
>>>>> @@ -10135,8 +10144,7 @@ build_lswitch_dhcp_relay_flows(struct ovn_port 
>>>>> *op,
>>>>>       "ip4.src == {0.0.0.0, %s/%u} && ip4.dst == 255.255.255.255 && "
>>>>>       "udp.src == 68 && udp.dst == 67",
>>>>>       op->json_key, op->lsp_addrs[0].ea_s,
>>>>> -        rp->lrp_networks.ipv4_addrs[0].network_s,
>>>>> -        rp->lrp_networks.ipv4_addrs[0].plen);
>>>>> +        lrp_ipv4_addr->network_s, lrp_ipv4_addr->plen);
>>>>>   ds_put_format(actions,
>>>>>                 "eth.dst = %s; outport = %s; next; /* DHCP_RELAY_REQ */",
>>>>>                 rp->lrp_networks.ea_s, sp->json_key);
>>>>> @@ -16833,6 +16841,57 @@ build_dhcpv6_reply_flows_for_lrouter_port(
>>>>>   }
>>>>> }
>>>>> 
>>>>> +/* Resolve the LRP's "primary" IPv4 address - the single representative
>>>>> + * IP used by features such as DHCP relay (giaddr).
>>>>> + *
> 
> Please see lower for the reasoning, I think this will only ever be used
> for DHCP.  We should rephrase this comment accordingly.
> 
>>>>> + * If 'options:primary_ipv4' is set, returns the entry from
>>>>> + * 'lrp_networks' whose address matches it.  If the option is unset,
>>>>> + * returns lrp_networks.ipv4_addrs[0] - the first address from
>>>>> + * 'networks' in OVSDB sort order (set members are sorted by strcmp()
>>>>> + * on their string form).
>>>>> + *
>>>>> + * Returns NULL when the port has no IPv4 address at all, or when
>>>>> + * 'options:primary_ipv4' is set but is not a valid IPv4 address or
>>>>> + * does not match any address on the port.  Callers must skip
>>>>> + * emitting flows that need a primary IPv4 in those cases.
>>>>> + */
>>>>> +static const struct ipv4_netaddr *
>>>>> +lrp_get_primary_ipv4_addr(const struct ovn_port *op)
>>>>> +{
>>>>> +    if (!op->lrp_networks.n_ipv4_addrs) {
>>>>> +        return NULL;
>>>>> +    }
>>>>> +
>>>>> +    const char *primary_ip = smap_get(&op->nbrp->options, 
>>>>> "primary_ipv4");
>>>>> +
>>>>> +    if (!primary_ip || !primary_ip[0]) {
>>>>> +        return &op->lrp_networks.ipv4_addrs[0];
>>>>> +    }
>>>>> +
>>>>> +    ovs_be32 primary_ip4;
>>>>> +    if (!ip_parse(primary_ip, &primary_ip4)) {
>>>>> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>>>> +        VLOG_WARN_RL(&rl,
>>>>> +                     "Logical_Router_Port %s: options:primary_ipv4=%s is 
>>>>> "
>>>>> +                     "not a valid IPv4 address.",
>>>>> +                     op->nbrp->name, primary_ip);
>>>>> +        return NULL;
>>>>> +    }
>>>>> +
>>>>> +    for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
>>>>> +        if (op->lrp_networks.ipv4_addrs[i].addr == primary_ip4) {
>>>>> +            return &op->lrp_networks.ipv4_addrs[i];
>>>>> +        }
>>>>> +    }
>>>>> +
>>>>> +    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>>>> +    VLOG_WARN_RL(&rl,
>>>>> +                 "Logical_Router_Port %s: options:primary_ipv4=%s does 
>>>>> not "
>>>>> +                 "match any address in 'networks'.",
>>>>> +                 op->nbrp->name, primary_ip);
>>>>> +    return NULL;
>>>>> +}
>>>>> +
>>>>> static void
>>>>> build_dhcp_relay_flows_for_lrouter_port(struct ovn_port *op,
>>>>>                                       struct lflow_table *lflows,
>>>>> @@ -16857,6 +16916,12 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>       return;
>>>>>   }
>>>>> 
>>>>> +    /* Use the LRP's primary IPv4 addr for the DHCP relay. */
>>>>> +    const struct ipv4_netaddr *lrp_ipv4_addr = 
>>>>> lrp_get_primary_ipv4_addr(op);
>>>>> +    if (!lrp_ipv4_addr) {
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>>   int addr_family;
>>>>>   /* currently not supporting custom port,
>>>>>    * dhcp server port is always set to 67 when installing flows */
>>>>> @@ -16880,14 +16945,12 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>       match, "inport == %s && "
>>>>>       "ip4.src == {0.0.0.0, %s/%u} && ip4.dst == 255.255.255.255 && "
>>>>>       "ip.frag == 0 && udp.src == 68 && udp.dst == 67",
>>>>> -        op->json_key,
>>>>> -        op->lrp_networks.ipv4_addrs[0].network_s,
>>>>> -        op->lrp_networks.ipv4_addrs[0].plen);
>>>>> +        op->json_key, lrp_ipv4_addr->network_s, lrp_ipv4_addr->plen);
>>>>>   ds_put_format(actions,
>>>>>                 REGBIT_DHCP_RELAY_REQ_CHK
>>>>>                 " = dhcp_relay_req_chk(%s, %s);"
>>>>>                 "next; /* DHCP_RELAY_REQ */",
>>>>> -                  op->lrp_networks.ipv4_addrs[0].addr_s, server_ip_str);
>>>>> +                  lrp_ipv4_addr->addr_s, server_ip_str);
>>>> 
>>>> I'll re-start the discussion Ales started on the v2 patch:
>>>> 
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_ovn_patch_20260622171455.1437986-2D1-2Dnaveen.yerramneni-40nutanix.com_-233713088&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=2PQjSDR7A28z1kXE1ptSm6X36oL_nCq1XxeEt7FkLmA&m=T5m-E7ByBB6C9RP60lgZfcUarUGxNSmvI-FTM_YIiLR5WXb6pyA1hW6EQVNhsl9L&s=HjCi7-gApXfOk9HyWCMbBeRdRZWyqPd4uu8271ppydQ&e=
>>>> <quote-from-v2-thread>
>>>> 
>>>>> @@ -16990,7 +17049,7 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>                 REG_DHCP_RELAY_DIP_IPV4" == %s && "
>>>>>                 "udp.src == 67 && udp.dst == 67 && "
>>>>>                 REGBIT_DHCP_RELAY_RESP_CHK" == 0",
>>>>> -                  server_ip_str, op->lrp_networks.ipv4_addrs[0].addr_s);
>>>>> +                  server_ip_str, lrp_ipv4_addr->addr_s);
>>>>> 
>>>>> Why can't we add all IPv4 addresses into the set or create multiple
>>>>> flows with different IP matches? Wouldn't that work without
>>>>> introducing another heavily specialized config option?
>>>>> 
>>>>> Reading the previous conversation I think that is
>>>>> what Dumitru suggested in the first place.
>>>> 
>>>> 
>>>> Hi Ales,
>>>> 
>>>> Thanks for the review!
>>>> 
>>>> `giaddr` is a single field in the DHCP header, and the server uses it
>>>> to choose the subnet/pool for IP allocation.  So when an LRP has
>>>> multiple IPv4 addresses on different subnets, exactly one of them has
>>>> to be stamped as `giaddr` — hence I think it is better to let the user
>>>> configure this via the new option.
>>>> 
>>>> I have updated the code to use this option to stamp `giaddr` in the
>>>> relayed packet, and to anchor the prefix-based match
>>>> (`ip4.src == {0.0.0.0, <primary>/<plen>}`) so that both
>>>> initial-assignment packets (`src == 0.0.0.0`, `dst == 255.255.255.255`)
>>>> and lease-rebind packets (`src == leased IP`, `dst == 255.255.255.255`)
>>>> on the chosen subnet are caught.
>>>> 
>>>> </quote-from-v2-thread>
>>>> 
>>>> Why can't we add all IPv4 addresses into the set?  The server will
>>>> still only use one IP address.  But we avoid the need of yet another
>>>> config knob.
>>>> 
>>>> Are we enabling any broken functionality if we do what
>>>> Ales suggested above?
>>> 
>>> 
>>> Hi Dumitru,
>>> 
>>> We can set only one IP in the DHCP packet header hence I introduced the
>>> new config option to give control to the user to select an IP that has to 
>>> be used
>>> for DHCP relay when there are multiple IPs on LRP.
>> 
>> 
>> I am referring to 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_ovn-2Dorg_ovn_blob_main_controller_pinctrl.c-23L2359&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=2PQjSDR7A28z1kXE1ptSm6X36oL_nCq1XxeEt7FkLmA&m=t5mg2g1SygVu2Yk-HCyYdF32dY9Pt_hotlETMuYNAfa_lPGTo696x4Ny8Z7VnljY&s=-7JlRKfXAfsMXKH3Vhy22GBurSlpAhMA_eznxFx5NlM&e=
>> where controller sets the giaddr in the DHCP header. This ip is passed as an 
>> arg
>> to controller through dhcp_relay_req_chk() action.
>> 
> 
> OK, I understand now.  It seems we need the option indeed then.
> 
> Let's make it clear though that it's a dhcp option so instead of
> primary_ipv4, let's call it dhcpv4_primary_ip.


Thanks for the review!
I have addressed your comments and sent v4.

> 
>> 
>>> 
>>> Thanks,
>>> Naveen
>>> 
>>>> 
>>>> Regards,
>>>> Dumitru
>>>> 
>>>>> 
>>>>>   ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 110, ds_cstr(match),
>>>>>                 ds_cstr(actions), lflow_ref,
>>>>> @@ -16904,13 +16967,11 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>       "ip4.src == {0.0.0.0, %s/%u} && ip4.dst == 255.255.255.255 && "
>>>>>       "udp.src == 68 && udp.dst == 67 && "
>>>>>       REGBIT_DHCP_RELAY_REQ_CHK,
>>>>> -        op->json_key,
>>>>> -        op->lrp_networks.ipv4_addrs[0].network_s,
>>>>> -        op->lrp_networks.ipv4_addrs[0].plen);
>>>>> +        op->json_key, lrp_ipv4_addr->network_s, lrp_ipv4_addr->plen);
>>>>>   ds_put_format(actions,
>>>>>                 "ip4.src = %s; ip4.dst = %s; udp.src = 67; next; "
>>>>>                 "/* DHCP_RELAY_REQ */",
>>>>> -                  op->lrp_networks.ipv4_addrs[0].addr_s, server_ip_str);
>>>>> +                  lrp_ipv4_addr->addr_s, server_ip_str);
>>>>> 
>>>>>   ovn_lflow_add(lflows, op->od, S_ROUTER_IN_DHCP_RELAY_REQ, 100,
>>>>>                 ds_cstr(match), ds_cstr(actions), lflow_ref,
>>>>> @@ -16924,9 +16985,7 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>       "ip4.src == {0.0.0.0, %s/%u} && ip4.dst == 255.255.255.255 && "
>>>>>       "udp.src == 68 && udp.dst == 67 && "
>>>>>       REGBIT_DHCP_RELAY_REQ_CHK" == 0",
>>>>> -        op->json_key,
>>>>> -        op->lrp_networks.ipv4_addrs[0].network_s,
>>>>> -        op->lrp_networks.ipv4_addrs[0].plen);
>>>>> +        op->json_key, lrp_ipv4_addr->network_s, lrp_ipv4_addr->plen);
>>>>>   ds_put_format(actions, "drop; /* DHCP_RELAY_REQ */");
>>>>> 
>>>>>   ovn_lflow_add(lflows, op->od, S_ROUTER_IN_DHCP_RELAY_REQ, 1,
>>>>> @@ -16939,7 +16998,7 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>   ds_put_format(
>>>>>       match, "ip4.src == %s && ip4.dst == %s && "
>>>>>       "ip.frag == 0 && udp.src == 67 && udp.dst == 67",
>>>>> -        server_ip_str, op->lrp_networks.ipv4_addrs[0].addr_s);
>>>>> +        server_ip_str, lrp_ipv4_addr->addr_s);
>>>>>   ds_put_format(actions, "next; /* DHCP_RELAY_RESP */");
>>>>>   ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 110,
>>>>>                 ds_cstr(match), ds_cstr(actions), lflow_ref,
>>>>> @@ -16951,12 +17010,12 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>   ds_put_format(
>>>>>       match, "ip4.src == %s && ip4.dst == %s && "
>>>>>       "udp.src == 67 && udp.dst == 67",
>>>>> -        server_ip_str, op->lrp_networks.ipv4_addrs[0].addr_s);
>>>>> +        server_ip_str, lrp_ipv4_addr->addr_s);
>>>>>   ds_put_format(actions,
>>>>>         REG_DHCP_RELAY_DIP_IPV4" = ip4.dst; "
>>>>>         REGBIT_DHCP_RELAY_RESP_CHK
>>>>>         " = dhcp_relay_resp_chk(%s, %s); next; /* DHCP_RELAY_RESP */",
>>>>> -          op->lrp_networks.ipv4_addrs[0].addr_s, server_ip_str);
>>>>> +          lrp_ipv4_addr->addr_s, server_ip_str);
>>>>> 
>>>>>   ovn_lflow_add(lflows, op->od, S_ROUTER_IN_DHCP_RELAY_RESP_CHK, 100,
>>>>>                 ds_cstr(match), ds_cstr(actions), lflow_ref,
>>>>> @@ -16974,11 +17033,11 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>       REG_DHCP_RELAY_DIP_IPV4" == %s && "
>>>>>       "udp.src == 67 && udp.dst == 67 && "
>>>>>       REGBIT_DHCP_RELAY_RESP_CHK,
>>>>> -        server_ip_str, op->lrp_networks.ipv4_addrs[0].addr_s);
>>>>> +        server_ip_str, lrp_ipv4_addr->addr_s);
>>>>>   ds_put_format(actions,
>>>>>                 "ip4.src = %s; udp.dst = 68; "
>>>>>                 "outport = %s; output; /* DHCP_RELAY_RESP */",
>>>>> -                  op->lrp_networks.ipv4_addrs[0].addr_s, op->json_key);
>>>>> +                  lrp_ipv4_addr->addr_s, op->json_key);
>>>>>   ovn_lflow_add(lflows, op->od, S_ROUTER_IN_DHCP_RELAY_RESP, 100,
>>>>>                 ds_cstr(match), ds_cstr(actions), lflow_ref,
>>>>>                 WITH_HINT(&op->nbrp->header_));
>>>>> @@ -16990,7 +17049,7 @@ build_dhcp_relay_flows_for_lrouter_port(struct 
>>>>> ovn_port *op,
>>>>>                 REG_DHCP_RELAY_DIP_IPV4" == %s && "
>>>>>                 "udp.src == 67 && udp.dst == 67 && "
>>>>>                 REGBIT_DHCP_RELAY_RESP_CHK" == 0",
>>>>> -                  server_ip_str, op->lrp_networks.ipv4_addrs[0].addr_s);
>>>>> +                  server_ip_str, lrp_ipv4_addr->addr_s);
>>>>>   ds_put_format(actions, "drop; /* DHCP_RELAY_RESP */");
>>>>>   ovn_lflow_add(lflows, op->od, S_ROUTER_IN_DHCP_RELAY_RESP, 1,
>>>>>                 ds_cstr(match), ds_cstr(actions), lflow_ref,
>>>>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>>>>> index 15fb1d7e8..9419a4d4b 100644
>>>>> --- a/ovn-nb.xml
>>>>> +++ b/ovn-nb.xml
>>>>> @@ -4131,6 +4131,35 @@ or
>>>>>     to <ref table="DHCP_Relay"/> table.
>>>>>   </column>
>>>>> 
>>>>> +    <column name="options" key="primary_ipv4">
>>>>> +      <p>
>>>>> +        Optional.  Selects which of the logical router port's IPv4
>>>>> +        addresses is treated as the port's <em>primary</em> IPv4 address.
>>>>> +        The value must be a bare IPv4 host address (no prefix length, 
>>>>> e.g.
>>>>> +        <code>10.0.0.1</code>) that equals one of the addresses 
>>>>> configured
>>>>> +        in <ref column="networks"/>.  If the value is not a valid IPv4
>>>>> +        address or does not match one of the addresses in
>>>>> +        <ref column="networks"/>, <code>ovn-northd</code> disables any
>>>>> +        feature that requires a primary IPv4 on this port (e.g. DHCP
>>>>> +        relay).
>>>>> +      </p>
>>>>> +      <p>
>>>>> +        This option is consumed by DHCP Relay (see
>>>>> +        <ref column="dhcp_relay"/>), where the primary IPv4 is used as
>>>>> +        the relay agent address (<code>giaddr</code>) in relayed DHCP
>>>>> +        packets.
>>>>> +      </p>
>>>>> +      <p>
>>>>> +        When unset, the primary IPv4 defaults to the first address in
>>>>> +        <ref column="networks"/> as ordered by OVSDB: the CIDR that
>>>>> +        sorts first as a string.  This default may not match the
>>>>> +        intended "primary" address, so on multi-IP ports this option
>>>>> +        should be set explicitly.  On single-IP ports the only
>>>>> +        configured address is always used as the primary and the option
>>>>> +        may be left unset.
>>>>> +      </p>
>>>>> +    </column>
>>>>> +
>>>>>   <group title="Distributed Gateway Ports">
>>>>>     <p>
>>>>>       Gateways, as documented under <code>Gateways</code> in the OVN
>>>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>>>> index 7d008a8fc..d2ae43819 100644
>>>>> --- a/tests/ovn-northd.at
>>>>> +++ b/tests/ovn-northd.at
>>>>> @@ -13647,6 +13647,82 @@ AT_CAPTURE_FILE([lflows])
>>>>> AT_CHECK([grep -e "DHCP_RELAY_" lflows | sed 's/table=../table=??/'], 
>>>>> [0], [dnl
> 
> We should use ovn_strip_lflows instead of the hand crafted sed here and
> in the other places in the test.
> 
>>>>> ])
>>>>> 
>>>>> +# Multi-IP LRP: re-enable DHCP relay on lrp1 and add a second IPv4
>>>>> +# network.  options:primary_ipv4 is unset, so ovn-northd falls back
>>>>> +# to the first address returned by OVSDB - whichever CIDR sorts first
>>>>> +# as a string.  For ["10.0.0.1/24","192.168.1.1/24"] that is
>>>>> +# 10.0.0.1/24, so DHCP relay flows are emitted using 10.0.0.1.
>>>>> +check ovn-nbctl --wait=sb set Logical_Router_port lrp1 
>>>>> dhcp_relay=$dhcp_relay
>>>>> +check ovn-nbctl --wait=sb set Logical_Router_port lrp1 \
>>>>> +    'networks=["10.0.0.1/24","192.168.1.1/24"]'
>>>>> +
>>>>> +ovn-sbctl lflow-list > lflows
>>>>> +AT_CAPTURE_FILE([lflows])
>>>>> +
>>>>> +AT_CHECK([grep -e "DHCP_RELAY_" lflows | sed 's/table=../table=??/'], 
>>>>> [0], [dnl
>>>>> +  table=??(lr_in_ip_input     ), priority=110  , match=(inport == "lrp1" 
>>>>> && ip4.src == {0.0.0.0, 10.0.0.0/24} && ip4.dst == 255.255.255.255 && 
>>>>> ip.frag == 0 && udp.src == 68 && udp.dst == 67), action=(reg9[[7]] = 
>>>>> dhcp_relay_req_chk(10.0.0.1, 172.16.1.1);next; /* DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_ip_input     ), priority=110  , match=(ip4.src == 
>>>>> 172.16.1.1 && ip4.dst == 10.0.0.1 && ip.frag == 0 && udp.src == 67 && 
>>>>> udp.dst == 67), action=(next; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_req), priority=100  , match=(inport == 
>>>>> "lrp1" && ip4.src == {0.0.0.0, 10.0.0.0/24} && ip4.dst == 255.255.255.255 
>>>>> && udp.src == 68 && udp.dst == 67 && reg9[[7]]), action=(ip4.src = 
>>>>> 10.0.0.1; ip4.dst = 172.16.1.1; udp.src = 67; next; /* DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_dhcp_relay_req), priority=1    , match=(inport == 
>>>>> "lrp1" && ip4.src == {0.0.0.0, 10.0.0.0/24} && ip4.dst == 255.255.255.255 
>>>>> && udp.src == 68 && udp.dst == 67 && reg9[[7]] == 0), action=(drop; /* 
>>>>> DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_dhcp_relay_resp_chk), priority=100  , match=(ip4.src == 
>>>>> 172.16.1.1 && ip4.dst == 10.0.0.1 && udp.src == 67 && udp.dst == 67), 
>>>>> action=(reg2 = ip4.dst; reg9[[8]] = dhcp_relay_resp_chk(10.0.0.1, 
>>>>> 172.16.1.1); next; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_resp), priority=100  , match=(ip4.src == 
>>>>> 172.16.1.1 && reg2 == 10.0.0.1 && udp.src == 67 && udp.dst == 67 && 
>>>>> reg9[[8]]), action=(ip4.src = 10.0.0.1; udp.dst = 68; outport = "lrp1"; 
>>>>> output; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_resp), priority=1    , match=(ip4.src == 
>>>>> 172.16.1.1 && reg2 == 10.0.0.1 && udp.src == 67 && udp.dst == 67 && 
>>>>> reg9[[8]] == 0), action=(drop; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(ls_in_l2_lkup      ), priority=100  , match=(inport == 
>>>>> "ls0-port1" && eth.src == 02:00:00:00:00:10 && ip4.src == {0.0.0.0, 
>>>>> 10.0.0.0/24} && ip4.dst == 255.255.255.255 && udp.src == 68 && udp.dst == 
>>>>> 67), action=(eth.dst = 02:00:00:00:00:01; outport = "lrp1-attachment"; 
>>>>> next; /* DHCP_RELAY_REQ */)
>>>>> +])
>>>>> +
>>>>> +# Pin the primary IPv4 via options:primary_ipv4.  Every DHCP relay flow
>>>>> +# should now reference 192.168.1.1/24 instead of 10.0.0.1/24.
>>>>> +check ovn-nbctl --wait=sb set Logical_Router_port lrp1 \
>>>>> +    options:primary_ipv4=192.168.1.1
>>>>> +
>>>>> +ovn-sbctl lflow-list > lflows
>>>>> +AT_CAPTURE_FILE([lflows])
>>>>> +
>>>>> +AT_CHECK([grep -e "DHCP_RELAY_" lflows | sed 's/table=../table=??/'], 
>>>>> [0], [dnl
>>>>> +  table=??(lr_in_ip_input     ), priority=110  , match=(inport == "lrp1" 
>>>>> && ip4.src == {0.0.0.0, 192.168.1.0/24} && ip4.dst == 255.255.255.255 && 
>>>>> ip.frag == 0 && udp.src == 68 && udp.dst == 67), action=(reg9[[7]] = 
>>>>> dhcp_relay_req_chk(192.168.1.1, 172.16.1.1);next; /* DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_ip_input     ), priority=110  , match=(ip4.src == 
>>>>> 172.16.1.1 && ip4.dst == 192.168.1.1 && ip.frag == 0 && udp.src == 67 && 
>>>>> udp.dst == 67), action=(next; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_req), priority=100  , match=(inport == 
>>>>> "lrp1" && ip4.src == {0.0.0.0, 192.168.1.0/24} && ip4.dst == 
>>>>> 255.255.255.255 && udp.src == 68 && udp.dst == 67 && reg9[[7]]), 
>>>>> action=(ip4.src = 192.168.1.1; ip4.dst = 172.16.1.1; udp.src = 67; next; 
>>>>> /* DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_dhcp_relay_req), priority=1    , match=(inport == 
>>>>> "lrp1" && ip4.src == {0.0.0.0, 192.168.1.0/24} && ip4.dst == 
>>>>> 255.255.255.255 && udp.src == 68 && udp.dst == 67 && reg9[[7]] == 0), 
>>>>> action=(drop; /* DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_dhcp_relay_resp_chk), priority=100  , match=(ip4.src == 
>>>>> 172.16.1.1 && ip4.dst == 192.168.1.1 && udp.src == 67 && udp.dst == 67), 
>>>>> action=(reg2 = ip4.dst; reg9[[8]] = dhcp_relay_resp_chk(192.168.1.1, 
>>>>> 172.16.1.1); next; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_resp), priority=100  , match=(ip4.src == 
>>>>> 172.16.1.1 && reg2 == 192.168.1.1 && udp.src == 67 && udp.dst == 67 && 
>>>>> reg9[[8]]), action=(ip4.src = 192.168.1.1; udp.dst = 68; outport = 
>>>>> "lrp1"; output; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_resp), priority=1    , match=(ip4.src == 
>>>>> 172.16.1.1 && reg2 == 192.168.1.1 && udp.src == 67 && udp.dst == 67 && 
>>>>> reg9[[8]] == 0), action=(drop; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(ls_in_l2_lkup      ), priority=100  , match=(inport == 
>>>>> "ls0-port1" && eth.src == 02:00:00:00:00:10 && ip4.src == {0.0.0.0, 
>>>>> 192.168.1.0/24} && ip4.dst == 255.255.255.255 && udp.src == 68 && udp.dst 
>>>>> == 67), action=(eth.dst = 02:00:00:00:00:01; outport = "lrp1-attachment"; 
>>>>> next; /* DHCP_RELAY_REQ */)
>>>>> +])
>>>>> +
>>>>> +# Setting options:primary_ipv4 to an address that does not belong to
>>>>> +# 'networks' is a misconfiguration: ovn-northd logs a warning and skips
>>>>> +# DHCP relay flow generation for the port.
>>>>> +check ovn-nbctl --wait=sb set Logical_Router_port lrp1 \
>>>>> +    options:primary_ipv4=10.99.99.99
>>>>> +
>>>>> +ovn-sbctl lflow-list > lflows
>>>>> +AT_CAPTURE_FILE([lflows])
>>>>> +
>>>>> +AT_CHECK([grep -e "DHCP_RELAY_" lflows | sed 's/table=../table=??/'], 
>>>>> [0], [dnl
>>>>> +])
>>>>> +AT_CHECK([grep -q "options:primary_ipv4=10.99.99.99 does not match any 
>>>>> address in 'networks'" northd/ovn-northd.log])
>>>>> +
>>>>> +# Reducing 'networks' back to a single IPv4 makes the option no longer
>>>>> +# mandatory; clearing it should restore DHCP relay flows that reference
>>>>> +# the remaining address.
>>>>> +check ovn-nbctl --wait=sb set Logical_Router_port lrp1 \
>>>>> +    'networks=["10.0.0.1/24"]'
>>>>> +check ovn-nbctl --wait=sb remove Logical_Router_port lrp1 options 
>>>>> primary_ipv4
>>>>> +
>>>>> +ovn-sbctl lflow-list > lflows
>>>>> +AT_CAPTURE_FILE([lflows])
>>>>> +
>>>>> +AT_CHECK([grep -e "DHCP_RELAY_" lflows | sed 's/table=../table=??/'], 
>>>>> [0], [dnl
>>>>> +  table=??(lr_in_ip_input     ), priority=110  , match=(inport == "lrp1" 
>>>>> && ip4.src == {0.0.0.0, 10.0.0.0/24} && ip4.dst == 255.255.255.255 && 
>>>>> ip.frag == 0 && udp.src == 68 && udp.dst == 67), action=(reg9[[7]] = 
>>>>> dhcp_relay_req_chk(10.0.0.1, 172.16.1.1);next; /* DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_ip_input     ), priority=110  , match=(ip4.src == 
>>>>> 172.16.1.1 && ip4.dst == 10.0.0.1 && ip.frag == 0 && udp.src == 67 && 
>>>>> udp.dst == 67), action=(next; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_req), priority=100  , match=(inport == 
>>>>> "lrp1" && ip4.src == {0.0.0.0, 10.0.0.0/24} && ip4.dst == 255.255.255.255 
>>>>> && udp.src == 68 && udp.dst == 67 && reg9[[7]]), action=(ip4.src = 
>>>>> 10.0.0.1; ip4.dst = 172.16.1.1; udp.src = 67; next; /* DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_dhcp_relay_req), priority=1    , match=(inport == 
>>>>> "lrp1" && ip4.src == {0.0.0.0, 10.0.0.0/24} && ip4.dst == 255.255.255.255 
>>>>> && udp.src == 68 && udp.dst == 67 && reg9[[7]] == 0), action=(drop; /* 
>>>>> DHCP_RELAY_REQ */)
>>>>> +  table=??(lr_in_dhcp_relay_resp_chk), priority=100  , match=(ip4.src == 
>>>>> 172.16.1.1 && ip4.dst == 10.0.0.1 && udp.src == 67 && udp.dst == 67), 
>>>>> action=(reg2 = ip4.dst; reg9[[8]] = dhcp_relay_resp_chk(10.0.0.1, 
>>>>> 172.16.1.1); next; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_resp), priority=100  , match=(ip4.src == 
>>>>> 172.16.1.1 && reg2 == 10.0.0.1 && udp.src == 67 && udp.dst == 67 && 
>>>>> reg9[[8]]), action=(ip4.src = 10.0.0.1; udp.dst = 68; outport = "lrp1"; 
>>>>> output; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(lr_in_dhcp_relay_resp), priority=1    , match=(ip4.src == 
>>>>> 172.16.1.1 && reg2 == 10.0.0.1 && udp.src == 67 && udp.dst == 67 && 
>>>>> reg9[[8]] == 0), action=(drop; /* DHCP_RELAY_RESP */)
>>>>> +  table=??(ls_in_l2_lkup      ), priority=100  , match=(inport == 
>>>>> "ls0-port1" && eth.src == 02:00:00:00:00:10 && ip4.src == {0.0.0.0, 
>>>>> 10.0.0.0/24} && ip4.dst == 255.255.255.255 && udp.src == 68 && udp.dst == 
>>>>> 67), action=(eth.dst = 02:00:00:00:00:01; outport = "lrp1-attachment"; 
>>>>> next; /* DHCP_RELAY_REQ */)
>>>>> +])
>>>>> +
>>>>> OVN_CLEANUP_NORTHD
>>>>> AT_CLEANUP
>>>>> ])
>> 
>> 
> 
> Regards,
> Dumitru


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

Reply via email to