On 6/9/26 12:09 PM, Naveen Yerramneni wrote:
>>> @@ -10082,12 +10083,14 @@ build_lswitch_dhcp_relay_flows(struct ovn_port
>>> *op,
>>>
>>> ds_put_format(
>>> match, "inport == %s && eth.src == %s && "
>>> - "ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && "
>>> + "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);
>>> + op->json_key, op->lsp_addrs[0].ea_s,
>>> + rp->lrp_networks.ipv4_addrs[0].network_s,
>>> + rp->lrp_networks.ipv4_addrs[0].plen);
>> Essentially here, we always just use the first network, why not add all
>> of them to the IP set? Adding the first one seems potentially incorrect
>> anyway, it might not be the one matching the DHCP_Relay configured "servers",
>> right?
>>
>> On a closer read, I see that the router port already only handles the
>> first configured network. That seems like an unfortunate decision in
>> the original implementation. Would you have time to prepare a separate
>> follow patch up for that?
> Hi Dumitru,
>
> Could you please me understand what is the exact change that you are
> proposing here ?
Hi Naveen,
Here's an example (I just modified the system test a bit) that I ran in an
OVN sandbox:
ovn-nbctl lr-add R1
ovn-nbctl ls-add sw0
ovn-nbctl ls-add sw1
ovn-nbctl ls-add sw-ext
# Add router ports with multiple subnets on each of them. <<<< relevant change!
ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 10.10.10.1/24 192.168.1.1/24
ovn-nbctl lrp-add R1 rp-sw1 00:00:03:01:02:03 20.20.20.2/24 192.168.2.1/24
ovn-nbctl lrp-add R1 rp-ext 00:00:02:01:02:03 172.16.1.254/24
dhcp_relay=$(ovn-nbctl create DHCP_Relay servers=172.16.1.1)
ovn-nbctl set Logical_Router_port rp-sw0 dhcp_relay=$dhcp_relay
ovn-nbctl set Logical_Router_port rp-sw1 dhcp_relay=$dhcp_relay
ovn-nbctl lrp-set-gateway-chassis rp-ext hv1
ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
type=router options:router-port=rp-sw0 \
-- lsp-set-addresses sw0-rp router
ovn-nbctl lsp-add sw1 sw1-rp -- set Logical_Switch_Port sw1-rp \
type=router options:router-port=rp-sw1 \
-- lsp-set-addresses sw1-rp router
ovn-nbctl set Logical_Switch sw0 other_config:dhcp_relay_port=sw0-rp
ovn-nbctl set Logical_Switch sw1 other_config:dhcp_relay_port=sw1-rp
ovn-nbctl lsp-add sw-ext ext-rp -- set Logical_Switch_Port ext-rp \
type=router options:router-port=rp-ext \
-- lsp-set-addresses ext-rp router
ovn-nbctl lsp-add-localnet-port sw-ext lnet phynet
ovn-nbctl lsp-add sw0 sw01 \
-- lsp-set-addresses sw01 "f0:00:00:01:02:03"
ovn-nbctl lsp-add sw1 sw11 \
-- lsp-set-addresses sw11 "f0:00:00:02:02:03"
Then logical flows look weird (and non-deterministic):
table=3 (lr_in_ip_input ), priority=110 , match=(inport == "rp-sw0" &&
ip4.src == {0.0.0.0, 10.10.10.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.10.10.1, 172.16.1.1);next; /* DHCP_RELAY_REQ */)
table=3 (lr_in_ip_input ), priority=110 , match=(inport == "rp-sw1" &&
ip4.src == {0.0.0.0, 192.168.2.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.2.1, 172.16.1.1);next; /* DHCP_RELAY_REQ */)
That's because in northd we do:
build_dhcp_relay_flows_for_lrouter_port()
{
...
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);
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);
ovn_lflow_add(lflows, op->od, S_ROUTER_IN_DHCP_RELAY_RESP_CHK, 100,
ds_cstr(match), ds_cstr(actions), lflow_ref,
WITH_CTRL_METER(copp_meter_get(COPP_DHCPV4_RELAY,
op->od->nbr->copp,
meter_groups)),
WITH_HINT(&op->nbrp->header_));
...
}
That's what I meant in my previous email.
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
