On Wed, Apr 24, 2024 at 9:21 AM Naveen Yerramneni <
[email protected]> wrote:

>
>
> > 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 Naveen for addressing the comments.  I applied this patch series to
main with some small changes.

Numan


> >
> > 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
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to