> On 05-Apr-2024, at 9:08 PM, Numan Siddique <[email protected]> wrote:
> 
> CAUTION: External Email
> 
> 
> On Wed, Mar 20, 2024 at 10:40 AM Naveen Yerramneni 
> <[email protected]> wrote:
> >
> >     This patch contains changes to enable DHCP Relay Agent support for 
> > overlay subnets.
> >
> >     USE CASE:
> >     ----------
> >       - Enable IP address assignment for overlay subnets from the 
> > centralized DHCP server present in the underlay network.
> >
> >     PREREQUISITES
> >     --------------
> >       - Logical Router Port IP should be assigned (statically) from the 
> > same overlay subnet which is managed by DHCP server.
> >       - LRP IP is used for GIADRR field when relaying the DHCP packets and 
> > also same IP needs to be configured as default gateway for the overlay 
> > subnet.
> >       - Overlay subnets managed by external DHCP server are expected to be 
> > directly reachable from the underlay network.
> >
> >     EXPECTED PACKET FLOW:
> >     ----------------------
> >     Following is the expected packet flow inorder to support DHCP rleay 
> > functionality in OVN.
> >       1. DHCP client originates DHCP discovery (broadcast).
> >       2. DHCP relay (running on the OVN) receives the broadcast and 
> > forwards the packet to the DHCP server by converting it to unicast.
> >          While forwarding the packet, it updates the GIADDR in DHCP header 
> > to its interface IP on which DHCP packet is received and increments hop 
> > count.
> >       3. DHCP server uses GIADDR field to decide the IP address pool from 
> > which IP has to be assigned and DHCP offer is sent to the same IP (GIADDR).
> >       4. DHCP relay agent forwards the offer to the client.
> >       5. DHCP client sends DHCP request (broadcast) packet.
> >       6. DHCP relay (running on the OVN) receives the broadcast and 
> > forwards the packet to the DHCP server by converting it to unicast.
> >          While forwarding the packet, it updates the GIADDR in DHCP header 
> > to its interface IP on which DHCP packet is received.
> >       7. DHCP Server sends the ACK packet.
> >       8. DHCP relay agent forwards the ACK packet to the client.
> >       9. All the future renew/release packets are directly exchanged 
> > between DHCP client and DHCP server.
> >
> >     OVN DHCP RELAY PACKET FLOW:
> >     ----------------------------
> >     To add DHCP Relay support on OVN, we need to replicate all the behavior 
> > described above using distributed logical switch and logical router.
> >     At, highlevel packet flow is distributed among Logical Switch and 
> > Logical Router on source node (where VM is deployed) and redirect 
> > chassis(RC) node.
> >       1. Request packet gets processed on the source node where VM is 
> > deployed and relays the packet to DHCP server.
> >       2. Response packet is first processed on RC node (which first 
> > recieves the packet from underlay network). RC node forwards the packet to 
> > the right node by filling in the dest MAC and IP.
> >
> >     OVN Packet flow with DHCP relay is explained below.
> >       1. DHCP client (VM) sends the DHCP discover packet (broadcast).
> >       2. Logical switch converts the packet to L2 unicast by setting the 
> > destination MAC to LRP's MAC
> >       3. Logical Router receives the packet and redirects it to the OVN 
> > controller.
> >       4. OVN controller updates the required information(GIADDR, HOP count) 
> > in the DHCP payload after doing the required checks. If any check fails, 
> > packet is dropped.
> >       5. Logical Router converts the packet to L3 unicast and forwards it 
> > to the server. This packets gets routed like any other packet (via RC node).
> >       6. Server replies with DHCP offer.
> >       7. RC node processes the DHCP offer and forwards it to the OVN 
> > controller.
> >       8. OVN controller does sanity checks and  updates the destination MAC 
> > (available in DHCP header), destination IP (available in DHCP header) and 
> > reinjects the packet to datapath.
> >          If any check fails, packet is dropped.
> >       9. Logical router updates the source IP and port and forwards the 
> > packet to logical switch.
> >       10. Logical switch delivers the packet to the DHCP client.
> >       11. Similar steps are performed for Request and Ack packets.
> >       12. All the future renew/release packets are directly exchanged 
> > between DHCP client and DHCP server
> >
> >     NEW OVN ACTIONS
> >     ---------------
> >       1. dhcp_relay_req_chk(<relay-ip>, <server-ip>)
> >           - This action executes on the source node on which the DHCP 
> > request originated.
> >           - This action relays the DHCP request coming from client to the 
> > server. Relay-ip is used to update GIADDR in the DHCP header.
> >       2. dhcp_relay_resp_chk(<relay-ip>, <server-ip>)
> >           - This action executes on the first node (RC node) which 
> > processes the DHCP response from the server.
> >           - This action updates  the destination MAC and destination IP so 
> > that the response can be forwarded to the appropriate node from which 
> > request was originated.
> >           - Relay-ip, server-ip are used to validate GIADDR and SERVER ID 
> > in the DHCP payload.
> >
> >     FLOWS
> >     -----
> >     Following are the flows added when DHCP Relay is configured on one 
> > overlay subnet, one additonal flow is added in ls_in_l2_lkup table for each 
> > VM part of the subnet.
> >
> >       1. table=27(ls_in_l2_lkup      ), priority=100  , match=(inport == 
> > <vm_port> && eth.src == <vm_mac> && ip4.src == 0.0.0.0 && ip4.dst == 
> > 255.255.255.255 && udp.src == 68 && udp.dst == 67),
> >          action=(eth.dst=<lrp_mac>;outport=<lrp>;next;/* DHCP_RELAY_REQ */)
> >       2. table=3 (lr_in_ip_input     ), priority=110  , match=(inport == 
> > <lrp> && ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && ip.frag == 0 
> > && udp.src == 68 && udp.dst == 67),
> >          action=(reg9[7] = dhcp_relay_req_chk(<lrp_ip>, 
> > <dhcp_server_ip>);next; /* DHCP_RELAY_REQ */)
> >       3. table=3 (lr_in_ip_input     ), priority=110  , match=(ip4.src == 
> > <dhcp_server> && ip4.dst == <lrp> && udp.src == 67 && udp.dst == 67), 
> > action=(next;/* DHCP_RELAY_RESP */)
> >       4. table=4 (lr_in_dhcp_relay_req), priority=100  , match=(inport == 
> > "lrp1" && ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && udp.src == 68 
> > && udp.dst == 67 && reg9[7]),
> >          action=(ip4.src=<lrp>;ip4.dst=<dhcp_server>;udp.src=67;next; /* 
> > DHCP_RELAY_REQ */)
> >       5. table=4 (lr_in_dhcp_relay_req), priority=1    , match=(inport == 
> > <lrp> && ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && udp.src == 68 
> > && udp.dst == 67 && reg9[7] == 0),
> >          action=(drop; /* DHCP_RELAY_REQ */)
> >       6. table=18(lr_in_dhcp_relay_resp_chk), priority=100  , 
> > match=(ip4.src == <dhcp_server> && ip4.dst == <lrp> && ip.frag == 0 && 
> > udp.src == 67 && udp.dst == 67),
> >          action=(reg2 = ip4.dst;reg9[8] = dhcp_relay_resp_chk(<lrp_ip>, 
> > <dhcp_server_ip>);next;/* DHCP_RELAY_RESP */)
> >       7. table=19(lr_in_dhcp_relay_resp), priority=100  , match=(ip4.src == 
> > <dhcp_server> && reg2 == <lrp_ip> && udp.src == 67 && udp.dst == 67 && 
> > reg9[8]),
> >          action=(ip4.src=<lrp>;udp.dst=68;outport=<lrp>;output; /* 
> > DHCP_RELAY_RESP */)
> >       8. table=19(lr_in_dhcp_relay_resp), priority=1    , match=(ip4.src == 
> > <dhcp_server> && reg2 == <lrp_ip> && udp.src == 67 && udp.dst == 67 && 
> > reg9[8] == 0), action=(drop; /* DHCP_RELAY_RESP */)
> >
> >     NEW PIPELINE STAGES
> >     -------------------
> >     Following stage is added for DHCP relay feature. Some of the flows are 
> > fitted into the existing pipeline tages.
> >       1. lr_in_dhcp_relay_req
> >           - This stage process the DHCP request packets coming from DHCP 
> > clients.
> >           - DHCP request packets for which dhcp_relay_req_chk action (which 
> > gets applied in ip input stage) is successful are forwarded to DHCP server.
> >           - DHCP request packets for which dhcp_relay_req_chk action is 
> > unsuccessful gets dropped.
> >       2. lr_in_dhcp_relay_resp_chk
> >           - This stage applied the dhcp_relay_resp_chk action for  DHCP 
> > response packets coming from the DHCP server.
> >       3. lr_in_dhcp_relay_resp
> >           - DHCP response packets for which dhcp_relay_resp_chk is 
> > sucessful are forwarded to the DHCP clients.
> >           - DHCP response packets for which dhcp_relay_resp_chk is 
> > unsucessful gets dropped.
> >
> >     REGISTRY USAGE
> >     ---------------
> >       - reg9[7] : To store the result of dhcp_relay_req_chk action.
> >       - reg9[8] : To store the result of dhcp_relay_resp_chk action.
> >       - reg2 : To store the original dest ip for DHCP response packets.
> >                This is required to properly match the packets in
> >                lr_in_dhcp_relay_resp stage since dhcp_relay_resp_chk action
> >                changes the dest ip.
> >
> >     NB SCHEMA CHANGES
> >     ----------------
> >       1. New DHCP_Relay table
> >           "DHCP_Relay": {
> >                 "columns": {
> >                     "name": {"type": "string"},
> >                     "servers": {"type": {"key": "string",
> >                                            "min": 0,
> >                                            "max": 1}},
> >                     "external_ids": {
> >                         "type": {"key": "string", "value": "string",
> >                                 "min": 0, "max": "unlimited"}}},
> >                     "options": {"type": {"key": "string", "value": "string",
> >                                 "min": 0, "max": "unlimited"}},
> >                 "isRoot": true},
> >       2. New column to Logical_Router_Port table
> >           "dhcp_relay": {"type": {"key": {"type": "uuid",
> >                                 "refTable": "DHCP_Relay",
> >                                 "refType": "strong"},
> >                                 "min": 0,
> >                                 "max": 1}},
> >
> >     Commands to enable the feature:
> >     ------------------------------
> >      ovn-nbctl create DHCP_Relay name=<name> servers=<dhcp_server_ip>
> >      ovn-nbctl set Logical_Router_port <lrp> dhcp_relay=<relay_uuid>
> >      ovn-nbctl set Logical_Switch <ls> 
> > other_config:dhcp_relay_port=<router_patch_port>
> >
> >     Example:
> >     -------
> >      ovn-nbctl ls-add ls0
> >      ovn-nbctl lsp-add ls0 vif0
> >      ovn-nbctl lsp-set-addresses vif0 <MAC> #Only MAC address has to be 
> > specified when logical ports are created.
> >      ovn-nbctl lsp-add ls0 lrp1-attachment
> >      ovn-nbctl lsp-set-type lrp1-attachment router
> >      ovn-nbctl lsp-set-addresses lrp1-attachment
> >      ovn-nbctl lsp-set-options lrp1-attachment router-port=lrp1
> >      ovn-nbctl lr-add lr0
> >      ovn-nbctl lrp-add lr0 lrp1 <MAC> <GATEWAY_IP/Prefix> #GATEWAY IP is 
> > set in GIADDR field when relaying the DHCP requests to server.
> >      ovn-nbctl lrp-add lr0 lrp-ext <MAC> <GATEWAY_IP/Prefix>
> >      ovn-nbctl ls-add ls-ext
> >      ovn-nbctl lsp-add ls-ext lrp-ext-attachment
> >      ovn-nbctl lsp-set-type lrp-ext-attachment router
> >      ovn-nbctl lsp-set-addresses lrp-ext-attachment
> >      ovn-nbctl lsp-set-options lrp-ext-attachment router-port=lrp-ext
> >      ovn-nbctl lsp-add ls-ext ln_port
> >      ovn-nbctl lsp-set-addresses ln_port unknown
> >      ovn-nbctl lsp-set-type ln_port localnet
> >      ovn-nbctl lsp-set-options ln_port network_name=physnet1
> >      # Enable DHCP Relay feature
> >      ovn-nbctl create DHCP_Relay name=dhcp_relay_test 
> > servers=<dhcp_server_ip>
> >      ovn-nbctl set Logical_Router_port lrp1 dhcp_relay=<relay_uuid>
> >      ovn-nbctl set Logical_Switch ls0 
> > other_config:dhcp_relay_port=lrp1-attachment
> >
> >     Limitations:
> >     ------------
> >       - All OVN features that needs IP address to be configured on logical 
> > port (like proxy arp, etc) will not be supported for overlay subnets on 
> > which DHCP relay is enabled.
> >
> >     References:
> >     ----------
> >       - rfc1541, rfc1542, rfc2131
> >
> > V1:
> >   - First patch.
> >
> > V2:
> >   - Addressed review comments from Numan.
> >
> > V3:
> >   - Split the patch into series.
> >   - Addressed review comments from Numan.
> >   - Updated the match condition for DHCP Relay flows.
> >
> > V4:
> >   - Fix sparse errors.
> >   - Reorder patch series.
> >
> > V5:
> >   - Fix test failures.
> >
> > Naveen Yerramneni (4):
> >   actions: DHCP Relay Agent support for overlay IPv4 subnets.
> >   controller: DHCP Relay Agent support for overlay IPv4 subnets.
> >   northd: DHCP Relay Agent support for overlay IPv4 subnets.
> >   tests: DHCP Relay Agent support for overlay IPv4 subnets.
> 
> Hi Naveen,
> 
> Thanks for the v5 and addressing the review comments.  I had a look at this 
> series and mostly it looks good to me.
> Sorry for the late response.
> 
> Can you please address the below comments.
> 
> 1.  In almost all the patches,  indentation is not proper.  Can you please 
> fix them?
>     Few examples.
>     
>    --
>    static void
> build_lswitch_dhcp_relay_flows(struct ovn_port *op,
>                            const struct hmap *ls_ports,
>                            struct lflow_table *lflows,
>                            struct ds *match,
>                            struct ds *actions)
>   ----
> 
> Please change the above to
> 
>  static void
> build_lswitch_dhcp_relay_flows(struct ovn_port *op,
>                                    const struct hmap *ls_ports,
>                                    struct lflow_table *lflows,
>                                    struct ds *match,
>                                    struct ds *actions)
>  
> ---
> 
>     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);
> 
> 
>  Please change the above to
> 
> 
>     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);
> 
> 
> Basically please align properly.
> 
> 
> 2.  Patch 1 has a lot of repetitive code.  I've some improvements here - 
> https://github.com/numansiddique/ovn/commit/a4180c2a2a441627373c839c32f34b1b5a8d8666
>  [github.com]
>      Can you please take a look and incorporate them in patch 1.
> 
> 3.  In patch 1, documentation for the new OVN actions 
> dhcp_relay_req_chk/dhcp_relay_rsp_chk is missing.  Please add in ovn-sb.xml
> 
> 4.  Remove the patch 4 altogether and move the test cases relevant to the 
> patches.  Eg.  Move the tests added in "action parsing" for the new OVN 
> actions into patch 1.
>      and rest of patch 4 to patch 3.
> 
> 5.  In patch 3, please add the documentation for the new logical flows in 
> ovn-northd.8.xml

Hi Numan,

Thanks for the review comments. I addressed them and sent v6, please take a 
look.
Sorry for the delay.

Thanks,
Naveen


> 
> Thanks
> Numan
> 
> 
> 
> 
> 
>    
> 
> 
> >
> >  controller/pinctrl.c  | 596 +++++++++++++++++++++++++++++++++++++-----
> >  include/ovn/actions.h |  27 ++
> >  lib/actions.c         | 149 +++++++++++
> >  lib/ovn-l7.h          |   2 +
> >  northd/northd.c       | 271 ++++++++++++++++++-
> >  northd/northd.h       |  41 +--
> >  ovn-nb.ovsschema      |  19 +-
> >  ovn-nb.xml            |  39 +++
> >  tests/atlocal.in [atlocal.in]      |   3 +
> >  tests/ovn-northd.at [ovn-northd.at]   |  38 +++
> >  tests/ovn.at [ovn.at]          | 258 +++++++++++++++++-
> >  tests/system-ovn.at [system-ovn.at]   | 148 +++++++++++
> >  utilities/ovn-trace.c |  67 +++++
> >  13 files changed, 1566 insertions(+), 92 deletions(-)
> >
> > --
> > 2.36.6
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev [mail.openvswitch.org]
> >

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

Reply via email to