[ovs-dev] [PATCH ovn v2] pinctrl: Support DHCPRELEASE and DHCPINFORM in native OVN dhcp responder.

Numan Siddique numans at ovn.org
Tue Jun 23 17:23:02 UTC 2020


On Tue, Jun 23, 2020 at 6:19 PM Mark Michelson <mmichels at redhat.com> wrote:

> Acked-by: Mark Michelson <mmichels at redhat.com>
>
>
Thank you Lorenzo and Mark for the reviews. I applied this patch to master.

Thanks
Numan


> On 6/22/20 11:31 AM, numans at ovn.org wrote:
> > From: Numan Siddique <numans at ovn.org>
> >
> > Right now we ignore these dhcp packets. This patch adds the support
> > as per RFC 2131.
> >
> > Signed-off-by: Numan Siddique <numans at ovn.org>
> > ---
> >
> > v1 -> v2
> > ---
> >    * Rebased to latest master to resolve conflicts.
> >
> >   NEWS                 |   2 +
> >   controller/pinctrl.c | 125 ++++++++++++++++++++++++++++++++++---------
> >   lib/ovn-l7.h         |  12 +++++
> >   tests/ovn.at         | 125 ++++++++++++++++++++++++++++++++++++++++---
> >   4 files changed, 233 insertions(+), 31 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index c6bb9b2fb..2f5bff5b1 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,5 +1,7 @@
> >   Post-v20.06.0
> >   --------------------------
> > +   - Added DHCPINFORM and DHCPRELEASE support in native
> > +     OVN DHCPv4 responder.
> >
> >   OVN v20.06.0
> >   --------------------------
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index bb90edd1f..b2c656efb 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -1682,11 +1682,13 @@ static void
> >   pinctrl_handle_put_dhcp_opts(
> >       struct rconn *swconn,
> >       struct dp_packet *pkt_in, struct ofputil_packet_in *pin,
> > -    struct ofpbuf *userdata, struct ofpbuf *continuation)
> > +    struct flow *in_flow, struct ofpbuf *userdata,
> > +    struct ofpbuf *continuation)
> >   {
> >       enum ofp_version version = rconn_get_version(swconn);
> >       enum ofputil_protocol proto =
> ofputil_protocol_from_ofp_version(version);
> >       struct dp_packet *pkt_out_ptr = NULL;
> > +    struct ofpbuf *dhcp_inform_reply_buf = NULL;
> >       uint32_t success = 0;
> >
> >       /* Parse result field. */
> > @@ -1810,22 +1812,15 @@ pinctrl_handle_put_dhcp_opts(
> >           VLOG_WARN_RL(&rl, "Missing DHCP message type");
> >           goto exit;
> >       }
> > -    if (*in_dhcp_msg_type != DHCP_MSG_DISCOVER &&
> > -        *in_dhcp_msg_type != DHCP_MSG_REQUEST) {
> > -        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> > -        VLOG_WARN_RL(&rl, "Invalid DHCP message type: %d",
> *in_dhcp_msg_type);
> > -        goto exit;
> > -    }
> >
> > -    uint8_t msg_type;
> > -    if (*in_dhcp_msg_type == DHCP_MSG_DISCOVER) {
> > +    struct ofpbuf *reply_dhcp_opts_ptr = userdata;
> > +    uint8_t msg_type = 0;
> > +
> > +    switch (*in_dhcp_msg_type) {
> > +    case DHCP_MSG_DISCOVER:
> >           msg_type = DHCP_MSG_OFFER;
> > -    } else {
> > -        /* This is a DHCPREQUEST. If the client has requested an IP that
> > -         * does not match the offered IP address, reply with a NAK. The
> > -         * requested IP address may be supplied either via Requested IP
> Address
> > -         * (opt 50) or via ciaddr, depending on the client's state.
> > -         */
> > +        break;
> > +    case DHCP_MSG_REQUEST: {
> >           msg_type = DHCP_MSG_ACK;
> >           if (request_ip != *offer_ip) {
> >               static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
> 5);
> > @@ -1834,12 +1829,81 @@ pinctrl_handle_put_dhcp_opts(
> >                            IP_ARGS(*offer_ip));
> >               msg_type = DHCP_MSG_NAK;
> >           }
> > +        break;
> > +    }
> > +    case OVN_DHCP_MSG_RELEASE: {
> > +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(20, 40);
> > +        const struct eth_header *l2 = dp_packet_eth(pkt_in);
> > +        VLOG_INFO_RL(&rl, "DHCPRELEASE "ETH_ADDR_FMT " "IP_FMT"",
> > +                     ETH_ADDR_ARGS(l2->eth_src),
> > +                     IP_ARGS(in_dhcp_data->ciaddr));
> > +        break;
> > +    }
> > +    case OVN_DHCP_MSG_INFORM: {
> > +        /* RFC 2131 section 3.4.
> > +         * Remove all the offer ip related dhcp options and
> > +         * all the time related dhcp options.
> > +         * Loop through the dhcp option defined in the userdata buffer
> > +         * and copy all the options into dhcp_inform_reply_buf skipping
> > +         * the not required ones.
> > +         * */
> > +        msg_type = DHCP_MSG_ACK;
> > +        in_dhcp_ptr = userdata->data;
> > +        end = (const char *)userdata->data + userdata->size;
> > +
> > +        /* The buf size cannot be greater > userdata->size. */
> > +        dhcp_inform_reply_buf = ofpbuf_new(userdata->size);
> > +
> > +        reply_dhcp_opts_ptr = dhcp_inform_reply_buf;
> > +        while (in_dhcp_ptr < end) {
> > +            const struct dhcp_opt_header *in_dhcp_opt =
> > +                (const struct dhcp_opt_header *)in_dhcp_ptr;
> > +
> > +            switch (in_dhcp_opt->code) {
> > +            case OVN_DHCP_OPT_CODE_NETMASK:
> > +            case OVN_DHCP_OPT_CODE_LEASE_TIME:
> > +            case OVN_DHCP_OPT_CODE_T1:
> > +            case OVN_DHCP_OPT_CODE_T2:
> > +                break;
> > +            default:
> > +                /* Copy the dhcp option to reply_dhcp_opts_ptr. */
> > +                ofpbuf_put(reply_dhcp_opts_ptr, in_dhcp_opt,
> > +                           in_dhcp_opt->len + sizeof *in_dhcp_opt);
> > +                break;
> > +            }
> > +
> > +            in_dhcp_ptr += sizeof *in_dhcp_opt;
> > +            if (in_dhcp_ptr > end) {
> > +                break;
> > +            }
> > +            in_dhcp_ptr += in_dhcp_opt->len;
> > +            if (in_dhcp_ptr > end) {
> > +                break;
> > +            }
> > +        }
> > +
> > +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(20, 40);
> > +        VLOG_INFO_RL(&rl, "DHCPINFORM from "ETH_ADDR_FMT " "IP_FMT"",
> > +                     ETH_ADDR_ARGS(in_flow->dl_src),
> > +                     IP_ARGS(in_flow->nw_src));
> > +
> > +        break;
> > +    }
> > +    default: {
> > +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> > +        VLOG_WARN_RL(&rl, "Invalid DHCP message type: %d",
> *in_dhcp_msg_type);
> > +        goto exit;
> > +    }
> > +    }
> > +
> > +    if (!msg_type) {
> > +        goto exit;
> >       }
> >
> >       /* Frame the DHCP reply packet
> > -     * Total DHCP options length will be options stored in the userdata
> +
> > -     * 16 bytes. Note that the DHCP options stored in userdata are not
> included
> > -     * in DHCPNAK messages.
> > +     * Total DHCP options length will be options stored in the
> > +     * reply_dhcp_opts_ptr + 16 bytes. Note that the DHCP options
> stored in
> > +     * reply_dhcp_opts_ptr are not included in DHCPNAK messages.
> >        *
> >        * --------------------------------------------------------------
> >        *| 4 Bytes (dhcp cookie) | 3 Bytes (option type) | DHCP options |
> > @@ -1849,7 +1913,7 @@ pinctrl_handle_put_dhcp_opts(
> >        */
> >       uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + 16;
> >       if (msg_type != DHCP_MSG_NAK) {
> > -        new_l4_size += userdata->size;
> > +        new_l4_size += reply_dhcp_opts_ptr->size;
> >       }
> >       size_t new_packet_size = pkt_in->l4_ofs + new_l4_size;
> >
> > @@ -1874,12 +1938,18 @@ pinctrl_handle_put_dhcp_opts(
> >       struct dhcp_header *dhcp_data = dp_packet_put(
> >           &pkt_out, dp_packet_pull(pkt_in, DHCP_HEADER_LEN),
> DHCP_HEADER_LEN);
> >       dhcp_data->op = DHCP_OP_REPLY;
> > -    dhcp_data->yiaddr = (msg_type == DHCP_MSG_NAK) ? 0 : *offer_ip;
> > +
> > +    if (*in_dhcp_msg_type != OVN_DHCP_MSG_INFORM) {
> > +        dhcp_data->yiaddr = (msg_type == DHCP_MSG_NAK) ? 0 : *offer_ip;
> > +    } else {
> > +        dhcp_data->yiaddr = 0;
> > +    }
> > +
> >       dp_packet_put(&pkt_out, &magic_cookie, sizeof(ovs_be32));
> >
> >       uint16_t out_dhcp_opts_size = 12;
> >       if (msg_type != DHCP_MSG_NAK) {
> > -      out_dhcp_opts_size += userdata->size;
> > +      out_dhcp_opts_size += reply_dhcp_opts_ptr->size;
> >       }
> >       uint8_t *out_dhcp_opts = dp_packet_put_zeros(&pkt_out,
> >                                                    out_dhcp_opts_size);
> > @@ -1890,8 +1960,9 @@ pinctrl_handle_put_dhcp_opts(
> >       out_dhcp_opts += 3;
> >
> >       if (msg_type != DHCP_MSG_NAK) {
> > -      memcpy(out_dhcp_opts, userdata->data, userdata->size);
> > -      out_dhcp_opts += userdata->size;
> > +        memcpy(out_dhcp_opts, reply_dhcp_opts_ptr->data,
> > +               reply_dhcp_opts_ptr->size);
> > +        out_dhcp_opts += reply_dhcp_opts_ptr->size;
> >       }
> >
> >       /* Padding */
> > @@ -1939,6 +2010,10 @@ exit:
> >       if (pkt_out_ptr) {
> >           dp_packet_uninit(pkt_out_ptr);
> >       }
> > +
> > +    if (dhcp_inform_reply_buf) {
> > +        ofpbuf_delete(dhcp_inform_reply_buf);
> > +    }
> >   }
> >
> >   static bool
> > @@ -2644,8 +2719,8 @@ process_packet_in(struct rconn *swconn, const
> struct ofp_header *msg)
> >           break;
> >
> >       case ACTION_OPCODE_PUT_DHCP_OPTS:
> > -        pinctrl_handle_put_dhcp_opts(swconn, &packet, &pin, &userdata,
> > -                                     &continuation);
> > +        pinctrl_handle_put_dhcp_opts(swconn, &packet, &pin, &headers,
> > +                                     &userdata, &continuation);
> >           break;
> >
> >       case ACTION_OPCODE_ND_NA:
> > diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> > index 22a2153de..4bfa902bd 100644
> > --- a/lib/ovn-l7.h
> > +++ b/lib/ovn-l7.h
> > @@ -36,6 +36,14 @@ struct gen_opts_map {
> >
> >   #define DHCP_BROADCAST_FLAG 0x8000
> >
> > +/* These are not defined in ovs/lib/dhcp.h and hence defined here with
> > + * OVN_DHCP_OPT_CODE_<opt_name>.
> > + */
> > +#define OVN_DHCP_OPT_CODE_NETMASK      1
> > +#define OVN_DHCP_OPT_CODE_LEASE_TIME   51
> > +#define OVN_DHCP_OPT_CODE_T1           58
> > +#define OVN_DHCP_OPT_CODE_T2           59
> > +
> >   #define DHCP_OPTION(NAME, CODE, TYPE) \
> >       {.name = NAME, .code = CODE, .type = TYPE}
> >
> > @@ -168,6 +176,10 @@ struct dhcp_opt6_header {
> >       ovs_be16 size;
> >   };
> >
> > +/* These are not defined in ovs/lib/dhcp.h, hence defining here. */
> > +#define OVN_DHCP_MSG_RELEASE        7
> > +#define OVN_DHCP_MSG_INFORM         8
> > +
> >   /* Supported DHCPv6 Message Types */
> >   #define DHCPV6_MSG_TYPE_SOLICIT     1
> >   #define DHCPV6_MSG_TYPE_ADVT        2
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 8ee348397..e05896ecf 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -5227,6 +5227,12 @@ test_dhcp() {
> >       done
> >       if test $offer_ip != 0; then
> >           local srv_mac=$1 srv_ip=$2 dhcp_reply_type=$3
> expected_dhcp_opts=$4
> > +        local offered_ip=$offer_ip
> > +        if [[ "$dhcp_type" == "08" ]]; then
> > +            # DHCP ACK for DHCP INFORM should not have any offer ip.
> > +            offered_ip=00000000
> > +        fi
> > +
> >           # total IP length will be the IP length of the request packet
> >           # (which is 272 in our case) + 8 (padding bytes) +
> (expected_dhcp_opts / 2)
> >           ip_len=`expr 280 + ${#expected_dhcp_opts} / 2`
> > @@ -5242,7 +5248,7 @@ test_dhcp() {
> >           if test $dhcp_reply_type = 06; then
> >               reply=${reply}00000000
> >           else
> > -            reply=${reply}${offer_ip}
> > +            reply=${reply}${offered_ip}
> >           fi
> >           # next server ip address, relay agent ip address, client mac
> address
> >           reply=${reply}0000000000000000${src_mac}
> > @@ -5382,7 +5388,7 @@ rm -f 2.expected
> >   ciaddr=`ip_to_hex 0 0 0 0`
> >   offer_ip=0
> >   request_ip=0
> > -test_dhcp 2 f00000000002 08 0 $ciaddr $offer_ip $request_ip 0 1 1
> > +test_dhcp 2 f00000000002 09 0 $ciaddr $offer_ip $request_ip 0 1 1
> >
> >   # NXT_RESUMEs should be 4.
> >   OVS_WAIT_UNTIL([test 4 = `cat ofctl_monitor*.log | grep -c
> NXT_RESUME`])
> > @@ -5557,6 +5563,113 @@ reset_pcap_file hv1-vif2 hv1/vif2
> >   rm -f 1.expected
> >   rm -f 2.expected
> >
> > +# Send DHCPRELEASE.
> > +offer_ip=0
> > +server_ip=`ip_to_hex 10 0 0 1`
> > +ciaddr=`ip_to_hex 10 0 0 6`
> > +request_ip=0
> > +expected_dhcp_opts=0
> > +test_dhcp 2 f00000000002 07 0 $ciaddr $offer_ip $request_ip 0
> ff1000000001
> > +
> > +# NXT_RESUMEs should be 10.
> > +OVS_WAIT_UNTIL([test 10 = $(cat ofctl_monitor*.log | grep -c
> NXT_RESUME)])
> > +
> > +# There is no reply for this. Check for the INFO log in
> ovn-controller.log
> > +AT_CHECK([test 1 = $(cat hv1/ovn-controller.log | \
> > +grep "DHCPRELEASE f0:00:00:00:00:02 10.0.0.6" -c)])
> > +
> > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap >
> 2.packets
> > +AT_CHECK([cat 2.packets], [0], [])
> > +
> > +reset_pcap_file hv1-vif1 hv1/vif1
> > +reset_pcap_file hv1-vif2 hv1/vif2
> > +rm -f 1.expected
> > +rm -f 2.expected
> > +
> > +# Send DHCPINFORM
> > +offer_ip=`ip_to_hex 10 0 0 6`
> > +server_ip=`ip_to_hex 10 0 0 1`
> > +ciaddr=$offer_ip
> > +request_ip=0
> > +src_ip=$offer_ip
> > +dst_ip=$server_ip
> > +# In the expected_dhcp_opts we should not see 330400000e10 which is
> > +# dhcp lease time option and 0104ffffff00 which is subnet mask option.
> > +expected_dhcp_opts=03040a00000136040a000001
> > +test_dhcp 2 f00000000002 08 0 $ciaddr $offer_ip $request_ip 1 $src_ip
> $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts
> > +
> > +# NXT_RESUMEs should be 11.
> > +OVS_WAIT_UNTIL([test 11 = $(cat ofctl_monitor*.log | grep -c
> NXT_RESUME)])
> > +
> > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap >
> 2.packets
> > +cat 2.expected | cut -c -48 > expout
> > +AT_CHECK([cat 2.packets | cut -c -48], [0], [expout])
> > +# Skipping the IPv4 checksum.
> > +cat 2.expected | cut -c 53- > expout
> > +AT_CHECK([cat 2.packets | cut -c 53-], [0], [expout])
> > +
> > +# Now add the dhcp option T1 to the dhcp options.
> > +ovn-nbctl set dhcp_options ${d1} options:T1=4000
> > +
> > +reset_pcap_file hv1-vif1 hv1/vif1
> > +reset_pcap_file hv1-vif2 hv1/vif2
> > +rm -f 1.expected
> > +rm -f 2.expected
> > +
> > +# Send DHCPREQUEST to make sure that T1 is in the reply dhcp options.
> > +offer_ip=`ip_to_hex 10 0 0 6`
> > +server_ip=`ip_to_hex 10 0 0 1`
> > +ciaddr=$offer_ip
> > +request_ip=0
> > +src_ip=$offer_ip
> > +dst_ip=$server_ip
> > +# In the expected_dhcp_opts we should not see 330400000e10 which is
> > +# dhcp lease time option.
> >
> +expected_dhcp_opts=3a0400000fa0330400000e100104ffffff0003040a00000136040a000001
> > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 1 $src_ip
> $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts
> > +
> > +# NXT_RESUMEs should be 12.
> > +OVS_WAIT_UNTIL([test 12 = $(cat ofctl_monitor*.log | grep -c
> NXT_RESUME)])
> > +
> > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap >
> 2.packets
> > +cat 2.expected | cut -c -48 > expout
> > +AT_CHECK([cat 2.packets | cut -c -48], [0], [expout])
> > +# Skipping the IPv4 checksum.
> > +cat 2.expected | cut -c 53- > expout
> > +AT_CHECK([cat 2.packets | cut -c 53-], [0], [expout])
> > +
> > +reset_pcap_file hv1-vif1 hv1/vif1
> > +reset_pcap_file hv1-vif2 hv1/vif2
> > +rm -f 1.expected
> > +rm -f 2.expected
> > +
> > +# Now send DHCPINFORM again.
> > +offer_ip=`ip_to_hex 10 0 0 6`
> > +server_ip=`ip_to_hex 10 0 0 1`
> > +ciaddr=00000000
> > +request_ip=0
> > +src_ip=$offer_ip
> > +dst_ip=$server_ip
> > +# In the expected_dhcp_opts we should not see 330400000e10 which is
> > +# dhcp lease time option and 0104ffffff00 which is subnet mask option.
> > +expected_dhcp_opts=03040a00000136040a000001
> > +test_dhcp 2 f00000000002 08 0 $ciaddr $offer_ip $request_ip 1 $src_ip
> $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts
> > +
> > +# NXT_RESUMEs should be 13.
> > +OVS_WAIT_UNTIL([test 13 = $(cat ofctl_monitor*.log | grep -c
> NXT_RESUME)])
> > +
> > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap >
> 2.packets
> > +cat 2.expected | cut -c -48 > expout
> > +AT_CHECK([cat 2.packets | cut -c -48], [0], [expout])
> > +# Skipping the IPv4 checksum.
> > +cat 2.expected | cut -c 53- > expout
> > +AT_CHECK([cat 2.packets | cut -c 53-], [0], [expout])
> > +
> > +reset_pcap_file hv1-vif1 hv1/vif1
> > +reset_pcap_file hv1-vif2 hv1/vif2
> > +rm -f 1.expected
> > +rm -f 2.expected
> > +
> >   # Set tftp server option (IPv4 address) for ls1
> >   echo "------ Set tftp server (IPv4 address) --------"
> >   ovn-nbctl dhcp-options-set-options $d1 server_id=10.0.0.1 \
> > @@ -5573,8 +5686,8 @@ request_ip=$offer_ip
> >
>  expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a00000142040a0a0a0a
> >   test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0
> ff1000000001 $server_ip 05 $expected_dhcp_opts
> >
> > -# NXT_RESUMEs should be 10.
> > -OVS_WAIT_UNTIL([test 10 = `cat ofctl_monitor*.log | grep -c
> NXT_RESUME`])
> > +# NXT_RESUMEs should be 14.
> > +OVS_WAIT_UNTIL([test 14 = `cat ofctl_monitor*.log | grep -c
> NXT_RESUME`])
> >
> >   $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap >
> 2.packets
> >   cat 2.expected | cut -c -48 > expout
> > @@ -5604,8 +5717,8 @@ request_ip=$offer_ip
> >
>  expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a0000014210746573745f746674705f736572766572
> >   test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0
> ff1000000001 $server_ip 05 $expected_dhcp_opts
> >
> > -# NXT_RESUMEs should be 11.
> > -OVS_WAIT_UNTIL([test 11 = `cat ofctl_monitor*.log | grep -c
> NXT_RESUME`])
> > +# NXT_RESUMEs should be 15.
> > +OVS_WAIT_UNTIL([test 15 = `cat ofctl_monitor*.log | grep -c
> NXT_RESUME`])
> >
> >   $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap >
> 2.packets
> >   cat 2.expected | cut -c -48 > expout
> >
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list