| Message ID | 20200304141030.546012-1-ihrachys@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [ovs-dev,ovn] Broadcast DHCPREPLY when BROADCAST flag is set | expand |
On 3/4/20 3:10 PM, Ihar Hrachyshka wrote: > As per RFC2131, section 4.1: > A server or relay agent sending or relaying a DHCP message directly > to a DHCP client (i.e., not to a relay agent specified in the > 'giaddr' field) SHOULD examine the BROADCAST bit in the 'flags' > field. If this bit is set to 1, the DHCP message SHOULD be sent as > an IP broadcast using an IP broadcast address (preferably 0xffffffff) > as the IP destination address and the link-layer broadcast address as > the link-layer destination address. > > This patch changes destination IP address to 255.255.255.255 when client > set BROADCAST flag in their DHCPREQUEST. Note: the offered IP address is > still part of the DHCP payload. Not a full review. Just a couple of style/patch formatting suggestions. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1801006 > > Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> > --- Since this is the second verion of the patch, subject line prefix should contain v2 tag, ex. [PATCH ovn v2]. BTW, for the next version of this patch I'd suggest using v3 (not v2). Here under the '---', should be list of changes made betwen versions. That significantly simplifies review process. For example: Version 3: * Fixed ... * Modified ... Version 2: * Fixed line length warning reproted by checkpatch. Please keep the whole version history in all the patch versions. > controller/pinctrl.c | 11 +++++ > northd/ovn-northd.c | 7 ++-- > tests/ovn.at | 98 ++++++++++++++++++++++++++++++++------------ > 3 files changed, 85 insertions(+), 31 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index d06915a65..6197ea325 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -966,6 +966,8 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const struct flow *ip_flow, > dp_packet_uninit(&packet); > } > > +#define DHCP_FLAGS_BROADCAST(flags) (((flags) & (1UL << 7)) != 0) > + > /* Called with in the pinctrl_handler thread context. */ > static void > pinctrl_handle_put_dhcp_opts( > @@ -1190,7 +1192,16 @@ pinctrl_handle_put_dhcp_opts( > > udp->udp_len = htons(new_l4_size); > > + /* Send a broadcast IP frame when BROADCAST flag is set */ We're typically want comments to be a full sentencies, i.e. start with capital letter and have a period at the end (missed). > struct ip_header *out_ip = dp_packet_l3(&pkt_out); > + uint32_t ip_dst; > + if DHCP_FLAGS_BROADCAST(dhcp_data->flags) { Despite the fact that this translates to a valid C code after the preprocessor run, it should be better it to have valid C code in a source code :), i.e. have 'if' expression parenthesised. > + ip_dst = htonl(0xffffffff); > + } else { > + ip_dst = *offer_ip; > + } > + put_16aligned_be32(&out_ip->ip_dst, ip_dst); > + > out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs + new_l4_size); > udp->udp_csum = 0; > /* Checksum needs to be initialized to zero. */ > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 3aba0487d..58dfee617 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -4271,10 +4271,9 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip, > ds_put_cstr(options_action, "); next;"); > > ds_put_format(response_action, "eth.dst = eth.src; eth.src = %s; " > - "ip4.dst = "IP_FMT"; ip4.src = %s; udp.src = 67; " > - "udp.dst = 68; outport = inport; flags.loopback = 1; " > - "output;", > - server_mac, IP_ARGS(offer_ip), server_ip); > + "ip4.src = %s; udp.src = 67; udp.dst = 68; " > + "outport = inport; flags.loopback = 1; output;", > + server_mac, server_ip); > > ds_put_format(ipv4_addr_match, > "ip4.src == "IP_FMT" && ip4.dst == {%s, 255.255.255.255}", > diff --git a/tests/ovn.at b/tests/ovn.at > index cbaa6d4a2..a7842a35e 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -4595,10 +4595,11 @@ sleep 2 > as hv1 ovs-vsctl show > > # This shell function sends a DHCP request packet > -# test_dhcp INPORT SRC_MAC DHCP_TYPE OFFER_IP REQUEST_IP ... > +# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST CIADDR OFFER_IP REQUEST_IP USE_IP ... > test_dhcp() { > - local inport=$1 src_mac=$2 dhcp_type=$3 ciaddr=$4 offer_ip=$5 request_ip=$6 use_ip=$7 > - shift; shift; shift; shift; shift; shift; shift; > + local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 ciaddr=$5 offer_ip=$6 request_ip=$7 use_ip=$8 > + shift; shift; shift; shift; shift; shift; shift; shift; > + > if test $use_ip != 0; then > src_ip=$1 > dst_ip=$2 > @@ -4607,6 +4608,7 @@ test_dhcp() { > src_ip=`ip_to_hex 0 0 0 0` > dst_ip=`ip_to_hex 255 255 255 255` > fi > + > if test $request_ip != 0; then > ip_len=0120 > udp_len=010b > @@ -4614,10 +4616,19 @@ test_dhcp() { > ip_len=011a > udp_len=0106 > fi > + > + if test $broadcast != 0; then > + flags=8000 > + reply_dst_ip=`ip_to_hex 255 255 255 255` > + else > + flags=0000 > + reply_dst_ip=${offer_ip} > + fi > + > local request=ffffffffffff${src_mac}08004510${ip_len}0000000080110000${src_ip}${dst_ip} > # udp header and dhcp header > request=${request}00440043${udp_len}0000 > - request=${request}010106006359aa7600000000${ciaddr}000000000000000000000000${src_mac} > + request=${request}010106006359aa760000${flags}${ciaddr}000000000000000000000000${src_mac} > # client hardware padding > request=${request}00000000000000000000 > # server hostname > @@ -4655,10 +4666,10 @@ test_dhcp() { > ip_len=$(printf "%x" $ip_len) > udp_len=$(printf "%x" $udp_len) > # $ip_len var will be in 3 digits i.e 134. So adding a '0' before $ip_len > - local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${offer_ip} > + local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${reply_dst_ip} > # udp header and dhcp header. > # $udp_len var will be in 3 digits. So adding a '0' before $udp_len > - reply=${reply}004300440${udp_len}0000020106006359aa7600000000${ciaddr} > + reply=${reply}004300440${udp_len}0000020106006359aa760000${flags}${ciaddr} > # your ip address; 0 for NAK > if test $dhcp_reply_type = 06; then > reply=${reply}00000000 > @@ -4729,7 +4740,7 @@ server_ip=`ip_to_hex 10 0 0 1` > ciaddr=`ip_to_hex 0 0 0 0` > request_ip=0 > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 1 f00000000001 01 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts > +test_dhcp 1 f00000000001 01 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts > > # NXT_RESUMEs should be 1. > OVS_WAIT_UNTIL([test 1 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4755,7 +4766,7 @@ server_ip=`ip_to_hex 10 0 0 1` > ciaddr=`ip_to_hex 0 0 0 0` > request_ip=$offer_ip > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts > > # NXT_RESUMEs should be 2. > OVS_WAIT_UNTIL([test 2 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4779,7 +4790,7 @@ server_ip=`ip_to_hex 10 0 0 1` > ciaddr=`ip_to_hex 0 0 0 0` > request_ip=`ip_to_hex 10 0 0 7` > expected_dhcp_opts="" > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 06 $expected_dhcp_opts > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 06 $expected_dhcp_opts > > # NXT_RESUMEs should be 3. > OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4803,7 +4814,7 @@ rm -f 2.expected > ciaddr=`ip_to_hex 0 0 0 0` > offer_ip=0 > request_ip=0 > -test_dhcp 2 f00000000002 08 $ciaddr $offer_ip $request_ip 0 1 1 > +test_dhcp 2 f00000000002 08 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`]) > @@ -4820,12 +4831,12 @@ rm -f 2.expected > # ls2-lp2 (vif4-tx.pcap) should receive the DHCPv4 request packet once. > > ciaddr=`ip_to_hex 0 0 0 0` > -test_dhcp 3 f00000000003 01 $ciaddr 0 0 4 0 > +test_dhcp 3 f00000000003 01 0 $ciaddr 0 0 4 0 > > # Send DHCPv4 packet on ls2-lp2. "router" DHCPv4 option is not defined for > # this lport. > ciaddr=`ip_to_hex 0 0 0 0` > -test_dhcp 4 f00000000004 01 $ciaddr 0 0 3 0 > +test_dhcp 4 f00000000004 01 0 $ciaddr 0 0 3 0 > > # NXT_RESUMEs should be 4. > OVS_WAIT_UNTIL([test 4 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4842,7 +4853,7 @@ request_ip=0 > src_ip=$offer_ip > dst_ip=$server_ip > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts > +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 5. > OVS_WAIT_UNTIL([test 5 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4868,7 +4879,7 @@ request_ip=0 > src_ip=$offer_ip > dst_ip=`ip_to_hex 255 255 255 255` > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts > +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 6. > OVS_WAIT_UNTIL([test 6 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4894,7 +4905,7 @@ request_ip=0 > src_ip=$offer_ip > dst_ip=`ip_to_hex 255 255 255 255` > expected_dhcp_opts="" > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > > # NXT_RESUMEs should be 7. > OVS_WAIT_UNTIL([test 7 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4920,7 +4931,7 @@ request_ip=0 > src_ip=$offer_ip > dst_ip=`ip_to_hex 255 255 255 255` > expected_dhcp_opts="" > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > > # NXT_RESUMEs should be 8. > OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4942,7 +4953,7 @@ rm -f 2.expected > ciaddr=`ip_to_hex 0 0 0 0` > src_ip=`ip_to_hex 10 0 0 6` > dst_ip=`ip_to_hex 10 0 0 4` > -test_dhcp 2 f00000000002 03 $ciaddr 0 0 1 $src_ip $dst_ip 1 > +test_dhcp 2 f00000000002 03 0 $ciaddr 0 0 1 $src_ip $dst_ip 1 > > # NXT_RESUMEs should be 8. > OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -4950,6 +4961,29 @@ OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > # vif1-tx.pcap should have received the DHCPv4 request packet > OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) > > +reset_pcap_file hv1-vif1 hv1/vif1 > +reset_pcap_file hv1-vif2 hv1/vif2 > +rm -f 1.expected > +rm -f 2.expected > + > +# Send DHCPDISCOVER with BROADCAST flag on. > +offer_ip=`ip_to_hex 10 0 0 4` > +server_ip=`ip_to_hex 10 0 0 1` > +ciaddr=`ip_to_hex 0 0 0 0` > +request_ip=0 > +expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > +test_dhcp 1 f00000000001 01 1 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts > + > +# NXT_RESUMEs should be 9. > +OVS_WAIT_UNTIL([test 9 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > + > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets > +cat 1.expected | cut -c -48 > expout > +AT_CHECK([cat 1.packets | cut -c -48], [0], [expout]) > +# Skipping the IPv4 checksum. > +cat 1.expected | cut -c 53- > expout > +AT_CHECK([cat 1.packets | cut -c 53-], [0], [expout]) > + > OVN_CLEANUP([hv1]) > > AT_CLEANUP > @@ -13206,10 +13240,11 @@ as hv1 > ovs-vsctl show > > # This shell function sends a DHCP request packet > -# test_dhcp INPORT SRC_MAC DHCP_TYPE OFFER_IP ... > +# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST OFFER_IP ... > test_dhcp() { > - local inport=$1 src_mac=$2 dhcp_type=$3 offer_ip=$4 use_ip=$5 > - shift; shift; shift; shift; shift; > + local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 offer_ip=$5 use_ip=$6 > + shift; shift; shift; shift; shift; shift; > + > if test $use_ip != 0; then > src_ip=$1 > dst_ip=$2 > @@ -13218,10 +13253,19 @@ test_dhcp() { > src_ip=`ip_to_hex 0 0 0 0` > dst_ip=`ip_to_hex 255 255 255 255` > fi > + > + if test $broadcast != 0; then > + flags=8000 > + reply_dst_ip=`ip_to_hex 255 255 255 255` > + else > + flags=0000 > + reply_dst_ip=${offer_ip} > + fi > + > local request=ffffffffffff${src_mac}0800451001100000000080110000${src_ip}${dst_ip} > # udp header and dhcp header > request=${request}0044004300fc0000 > - request=${request}010106006359aa760000000000000000000000000000000000000000${src_mac} > + request=${request}010106006359aa760000${flags}00000000000000000000000000000000${src_mac} > # client hardware padding > request=${request}00000000000000000000 > # server hostname > @@ -13245,10 +13289,10 @@ test_dhcp() { > ip_len=$(printf "%x" $ip_len) > udp_len=$(printf "%x" $udp_len) > # $ip_len var will be in 3 digits i.e 134. So adding a '0' before $ip_len > - local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${offer_ip} > + local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${reply_dst_ip} > # udp header and dhcp header. > # $udp_len var will be in 3 digits. So adding a '0' before $udp_len > - reply=${reply}004300440${udp_len}0000020106006359aa760000000000000000 > + reply=${reply}004300440${udp_len}0000020106006359aa760000${flags}00000000 > # your ip address > reply=${reply}${offer_ip} > # next server ip address, relay agent ip address, client mac address > @@ -13367,7 +13411,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > server_ip=`ip_to_hex 10 0 0 1` > server_mac=ff1000000001 > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > $expected_dhcp_opts > > # NXT_RESUMEs should be 1 in hv1. > @@ -13465,7 +13509,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > server_ip=`ip_to_hex 10 0 0 1` > server_mac=ff1000000001 > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > $expected_dhcp_opts > > # NXT_RESUMEs should be 2 in hv1. > @@ -13575,7 +13619,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > server_ip=`ip_to_hex 10 0 0 1` > server_mac=ff1000000001 > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > $expected_dhcp_opts > > # NXT_RESUMEs should be 3 in hv1. > @@ -13655,7 +13699,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > server_ip=`ip_to_hex 10 0 0 1` > server_mac=ff1000000001 > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > $expected_dhcp_opts > > # NXT_RESUMEs should be 4 in hv1. >
Thanks for the patch. Please see below for few comments. Thanks Numan On Wed, Mar 4, 2020 at 8:57 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 3/4/20 3:10 PM, Ihar Hrachyshka wrote: > > As per RFC2131, section 4.1: > > A server or relay agent sending or relaying a DHCP message directly > > to a DHCP client (i.e., not to a relay agent specified in the > > 'giaddr' field) SHOULD examine the BROADCAST bit in the 'flags' > > field. If this bit is set to 1, the DHCP message SHOULD be sent as > > an IP broadcast using an IP broadcast address (preferably 0xffffffff) > > as the IP destination address and the link-layer broadcast address as > > the link-layer destination address. > > > > This patch changes destination IP address to 255.255.255.255 when client > > set BROADCAST flag in their DHCPREQUEST. Note: the offered IP address is > > still part of the DHCP payload. > > Not a full review. Just a couple of style/patch formatting suggestions. > > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1801006 > > > > Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> > > --- > > Since this is the second verion of the patch, subject line prefix > should contain v2 tag, ex. [PATCH ovn v2]. > BTW, for the next version of this patch I'd suggest using v3 (not v2). > > Here under the '---', should be list of changes made betwen versions. > That significantly simplifies review process. For example: > > Version 3: > * Fixed ... > * Modified ... > Version 2: > * Fixed line length warning reproted by checkpatch. > > Please keep the whole version history in all the patch versions. > > > controller/pinctrl.c | 11 +++++ > > northd/ovn-northd.c | 7 ++-- > > tests/ovn.at | 98 ++++++++++++++++++++++++++++++++------------ > > 3 files changed, 85 insertions(+), 31 deletions(-) > > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index d06915a65..6197ea325 100644 > > --- a/controller/pinctrl.c > > +++ b/controller/pinctrl.c > > @@ -966,6 +966,8 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const struct flow *ip_flow, > > dp_packet_uninit(&packet); > > } > > > > +#define DHCP_FLAGS_BROADCAST(flags) (((flags) & (1UL << 7)) != 0) > > + > > /* Called with in the pinctrl_handler thread context. */ > > static void > > pinctrl_handle_put_dhcp_opts( > > @@ -1190,7 +1192,16 @@ pinctrl_handle_put_dhcp_opts( > > > > udp->udp_len = htons(new_l4_size); > > > > + /* Send a broadcast IP frame when BROADCAST flag is set */ > > We're typically want comments to be a full sentencies, i.e. start > with capital letter and have a period at the end (missed). > > > struct ip_header *out_ip = dp_packet_l3(&pkt_out); > > + uint32_t ip_dst; > > + if DHCP_FLAGS_BROADCAST(dhcp_data->flags) { > > Despite the fact that this translates to a valid C code after > the preprocessor run, it should be better it to have valid C code > in a source code :), i.e. have 'if' expression parenthesised. +1. Also compilation fails because of this code when configured this way - ./configure --enable-Werror --enable-sparse env REAL_CC="gcc" CHECK="sparse -Wsparse-error -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/include/sparse -m64 -I /usr/local/include " cgcc -target=x86_64 -DHAVE_CONFIG_H -I. -I.. -I ../include -I ../include -I ../ovn -I ./include -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/include -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/_gcc/include -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/lib -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/_gcc/lib -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/_gcc -I ../lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wswitch-bool -Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare -Wshift-negative-value -Wduplicated-cond -Wshadow -Wmultistatement-macros -Wcast-align=strict -Werror -Werror -g -O2 -MT northd/ovn-northd.o -MD -MP -MF $depbase.Tpo -c -o northd/ovn-northd.o ../northd/ovn-northd.c &&\ mv -f $depbase.Tpo $depbase.Po ../controller/pinctrl.c:1198:8: error: restricted ovs_be16 degrades to integer ../controller/pinctrl.c:1199:16: error: incorrect type in assignment (different base types) ../controller/pinctrl.c:1199:16: expected unsigned int [usertype] ip_dst ../controller/pinctrl.c:1199:16: got restricted ovs_be32 ../controller/pinctrl.c:1201:16: error: incorrect type in assignment (different base types) ../controller/pinctrl.c:1201:16: expected unsigned int [usertype] ip_dst ../controller/pinctrl.c:1201:16: got restricted ovs_be32 [usertype] ../controller/pinctrl.c:1203:41: error: incorrect type in argument 2 (different base types) ../controller/pinctrl.c:1203:41: expected restricted ovs_be32 [usertype] ../controller/pinctrl.c:1203:41: got unsigned int [usertype] ip_dst make[1]: *** [Makefile:2053: controller/pinctrl.o] Error 1 make[1]: *** Waiting for unfinished jobs.... Also you need to update the documentation in northd/ovn-northd.8.xml accordingly. It'd be here - https://github.com/ovn-org/ovn/blob/master/northd/ovn-northd.8.xml#L941 Thanks Numan > > > + ip_dst = htonl(0xffffffff); > > + } else { > > + ip_dst = *offer_ip; > > + } > > + put_16aligned_be32(&out_ip->ip_dst, ip_dst); > > + > > out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs + new_l4_size); > > udp->udp_csum = 0; > > /* Checksum needs to be initialized to zero. */ > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 3aba0487d..58dfee617 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -4271,10 +4271,9 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip, > > ds_put_cstr(options_action, "); next;"); > > > > ds_put_format(response_action, "eth.dst = eth.src; eth.src = %s; " > > - "ip4.dst = "IP_FMT"; ip4.src = %s; udp.src = 67; " > > - "udp.dst = 68; outport = inport; flags.loopback = 1; " > > - "output;", > > - server_mac, IP_ARGS(offer_ip), server_ip); > > + "ip4.src = %s; udp.src = 67; udp.dst = 68; " > > + "outport = inport; flags.loopback = 1; output;", > > + server_mac, server_ip); > > > > ds_put_format(ipv4_addr_match, > > "ip4.src == "IP_FMT" && ip4.dst == {%s, 255.255.255.255}", > > diff --git a/tests/ovn.at b/tests/ovn.at > > index cbaa6d4a2..a7842a35e 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -4595,10 +4595,11 @@ sleep 2 > > as hv1 ovs-vsctl show > > > > # This shell function sends a DHCP request packet > > -# test_dhcp INPORT SRC_MAC DHCP_TYPE OFFER_IP REQUEST_IP ... > > +# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST CIADDR OFFER_IP REQUEST_IP USE_IP ... > > test_dhcp() { > > - local inport=$1 src_mac=$2 dhcp_type=$3 ciaddr=$4 offer_ip=$5 request_ip=$6 use_ip=$7 > > - shift; shift; shift; shift; shift; shift; shift; > > + local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 ciaddr=$5 offer_ip=$6 request_ip=$7 use_ip=$8 > > + shift; shift; shift; shift; shift; shift; shift; shift; > > + > > if test $use_ip != 0; then > > src_ip=$1 > > dst_ip=$2 > > @@ -4607,6 +4608,7 @@ test_dhcp() { > > src_ip=`ip_to_hex 0 0 0 0` > > dst_ip=`ip_to_hex 255 255 255 255` > > fi > > + > > if test $request_ip != 0; then > > ip_len=0120 > > udp_len=010b > > @@ -4614,10 +4616,19 @@ test_dhcp() { > > ip_len=011a > > udp_len=0106 > > fi > > + > > + if test $broadcast != 0; then > > + flags=8000 > > + reply_dst_ip=`ip_to_hex 255 255 255 255` > > + else > > + flags=0000 > > + reply_dst_ip=${offer_ip} > > + fi > > + > > local request=ffffffffffff${src_mac}08004510${ip_len}0000000080110000${src_ip}${dst_ip} > > # udp header and dhcp header > > request=${request}00440043${udp_len}0000 > > - request=${request}010106006359aa7600000000${ciaddr}000000000000000000000000${src_mac} > > + request=${request}010106006359aa760000${flags}${ciaddr}000000000000000000000000${src_mac} > > # client hardware padding > > request=${request}00000000000000000000 > > # server hostname > > @@ -4655,10 +4666,10 @@ test_dhcp() { > > ip_len=$(printf "%x" $ip_len) > > udp_len=$(printf "%x" $udp_len) > > # $ip_len var will be in 3 digits i.e 134. So adding a '0' before $ip_len > > - local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${offer_ip} > > + local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${reply_dst_ip} > > # udp header and dhcp header. > > # $udp_len var will be in 3 digits. So adding a '0' before $udp_len > > - reply=${reply}004300440${udp_len}0000020106006359aa7600000000${ciaddr} > > + reply=${reply}004300440${udp_len}0000020106006359aa760000${flags}${ciaddr} > > # your ip address; 0 for NAK > > if test $dhcp_reply_type = 06; then > > reply=${reply}00000000 > > @@ -4729,7 +4740,7 @@ server_ip=`ip_to_hex 10 0 0 1` > > ciaddr=`ip_to_hex 0 0 0 0` > > request_ip=0 > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 1 f00000000001 01 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts > > +test_dhcp 1 f00000000001 01 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts > > > > # NXT_RESUMEs should be 1. > > OVS_WAIT_UNTIL([test 1 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4755,7 +4766,7 @@ server_ip=`ip_to_hex 10 0 0 1` > > ciaddr=`ip_to_hex 0 0 0 0` > > request_ip=$offer_ip > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts > > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts > > > > # NXT_RESUMEs should be 2. > > OVS_WAIT_UNTIL([test 2 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4779,7 +4790,7 @@ server_ip=`ip_to_hex 10 0 0 1` > > ciaddr=`ip_to_hex 0 0 0 0` > > request_ip=`ip_to_hex 10 0 0 7` > > expected_dhcp_opts="" > > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 06 $expected_dhcp_opts > > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 06 $expected_dhcp_opts > > > > # NXT_RESUMEs should be 3. > > OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4803,7 +4814,7 @@ rm -f 2.expected > > ciaddr=`ip_to_hex 0 0 0 0` > > offer_ip=0 > > request_ip=0 > > -test_dhcp 2 f00000000002 08 $ciaddr $offer_ip $request_ip 0 1 1 > > +test_dhcp 2 f00000000002 08 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`]) > > @@ -4820,12 +4831,12 @@ rm -f 2.expected > > # ls2-lp2 (vif4-tx.pcap) should receive the DHCPv4 request packet once. > > > > ciaddr=`ip_to_hex 0 0 0 0` > > -test_dhcp 3 f00000000003 01 $ciaddr 0 0 4 0 > > +test_dhcp 3 f00000000003 01 0 $ciaddr 0 0 4 0 > > > > # Send DHCPv4 packet on ls2-lp2. "router" DHCPv4 option is not defined for > > # this lport. > > ciaddr=`ip_to_hex 0 0 0 0` > > -test_dhcp 4 f00000000004 01 $ciaddr 0 0 3 0 > > +test_dhcp 4 f00000000004 01 0 $ciaddr 0 0 3 0 > > > > # NXT_RESUMEs should be 4. > > OVS_WAIT_UNTIL([test 4 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4842,7 +4853,7 @@ request_ip=0 > > src_ip=$offer_ip > > dst_ip=$server_ip > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts > > +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 5. > > OVS_WAIT_UNTIL([test 5 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4868,7 +4879,7 @@ request_ip=0 > > src_ip=$offer_ip > > dst_ip=`ip_to_hex 255 255 255 255` > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts > > +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 6. > > OVS_WAIT_UNTIL([test 6 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4894,7 +4905,7 @@ request_ip=0 > > src_ip=$offer_ip > > dst_ip=`ip_to_hex 255 255 255 255` > > expected_dhcp_opts="" > > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > > > > # NXT_RESUMEs should be 7. > > OVS_WAIT_UNTIL([test 7 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4920,7 +4931,7 @@ request_ip=0 > > src_ip=$offer_ip > > dst_ip=`ip_to_hex 255 255 255 255` > > expected_dhcp_opts="" > > -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > > +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts > > > > # NXT_RESUMEs should be 8. > > OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4942,7 +4953,7 @@ rm -f 2.expected > > ciaddr=`ip_to_hex 0 0 0 0` > > src_ip=`ip_to_hex 10 0 0 6` > > dst_ip=`ip_to_hex 10 0 0 4` > > -test_dhcp 2 f00000000002 03 $ciaddr 0 0 1 $src_ip $dst_ip 1 > > +test_dhcp 2 f00000000002 03 0 $ciaddr 0 0 1 $src_ip $dst_ip 1 > > > > # NXT_RESUMEs should be 8. > > OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -4950,6 +4961,29 @@ OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > # vif1-tx.pcap should have received the DHCPv4 request packet > > OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) > > > > +reset_pcap_file hv1-vif1 hv1/vif1 > > +reset_pcap_file hv1-vif2 hv1/vif2 > > +rm -f 1.expected > > +rm -f 2.expected > > + > > +# Send DHCPDISCOVER with BROADCAST flag on. > > +offer_ip=`ip_to_hex 10 0 0 4` > > +server_ip=`ip_to_hex 10 0 0 1` > > +ciaddr=`ip_to_hex 0 0 0 0` > > +request_ip=0 > > +expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > +test_dhcp 1 f00000000001 01 1 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts > > + > > +# NXT_RESUMEs should be 9. > > +OVS_WAIT_UNTIL([test 9 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > + > > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets > > +cat 1.expected | cut -c -48 > expout > > +AT_CHECK([cat 1.packets | cut -c -48], [0], [expout]) > > +# Skipping the IPv4 checksum. > > +cat 1.expected | cut -c 53- > expout > > +AT_CHECK([cat 1.packets | cut -c 53-], [0], [expout]) > > + > > OVN_CLEANUP([hv1]) > > > > AT_CLEANUP > > @@ -13206,10 +13240,11 @@ as hv1 > > ovs-vsctl show > > > > # This shell function sends a DHCP request packet > > -# test_dhcp INPORT SRC_MAC DHCP_TYPE OFFER_IP ... > > +# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST OFFER_IP ... > > test_dhcp() { > > - local inport=$1 src_mac=$2 dhcp_type=$3 offer_ip=$4 use_ip=$5 > > - shift; shift; shift; shift; shift; > > + local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 offer_ip=$5 use_ip=$6 > > + shift; shift; shift; shift; shift; shift; > > + > > if test $use_ip != 0; then > > src_ip=$1 > > dst_ip=$2 > > @@ -13218,10 +13253,19 @@ test_dhcp() { > > src_ip=`ip_to_hex 0 0 0 0` > > dst_ip=`ip_to_hex 255 255 255 255` > > fi > > + > > + if test $broadcast != 0; then > > + flags=8000 > > + reply_dst_ip=`ip_to_hex 255 255 255 255` > > + else > > + flags=0000 > > + reply_dst_ip=${offer_ip} > > + fi > > + > > local request=ffffffffffff${src_mac}0800451001100000000080110000${src_ip}${dst_ip} > > # udp header and dhcp header > > request=${request}0044004300fc0000 > > - request=${request}010106006359aa760000000000000000000000000000000000000000${src_mac} > > + request=${request}010106006359aa760000${flags}00000000000000000000000000000000${src_mac} > > # client hardware padding > > request=${request}00000000000000000000 > > # server hostname > > @@ -13245,10 +13289,10 @@ test_dhcp() { > > ip_len=$(printf "%x" $ip_len) > > udp_len=$(printf "%x" $udp_len) > > # $ip_len var will be in 3 digits i.e 134. So adding a '0' before $ip_len > > - local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${offer_ip} > > + local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${reply_dst_ip} > > # udp header and dhcp header. > > # $udp_len var will be in 3 digits. So adding a '0' before $udp_len > > - reply=${reply}004300440${udp_len}0000020106006359aa760000000000000000 > > + reply=${reply}004300440${udp_len}0000020106006359aa760000${flags}00000000 > > # your ip address > > reply=${reply}${offer_ip} > > # next server ip address, relay agent ip address, client mac address > > @@ -13367,7 +13411,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > > server_ip=`ip_to_hex 10 0 0 1` > > server_mac=ff1000000001 > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > > $expected_dhcp_opts > > > > # NXT_RESUMEs should be 1 in hv1. > > @@ -13465,7 +13509,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > > server_ip=`ip_to_hex 10 0 0 1` > > server_mac=ff1000000001 > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > > $expected_dhcp_opts > > > > # NXT_RESUMEs should be 2 in hv1. > > @@ -13575,7 +13619,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > > server_ip=`ip_to_hex 10 0 0 1` > > server_mac=ff1000000001 > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > > $expected_dhcp_opts > > > > # NXT_RESUMEs should be 3 in hv1. > > @@ -13655,7 +13699,7 @@ offer_ip=`ip_to_hex 10 0 0 6` > > server_ip=`ip_to_hex 10 0 0 1` > > server_mac=ff1000000001 > > expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 > > -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ > > +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ > > $expected_dhcp_opts > > > > # NXT_RESUMEs should be 4 in hv1. > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index d06915a65..6197ea325 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -966,6 +966,8 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const struct flow *ip_flow, dp_packet_uninit(&packet); } +#define DHCP_FLAGS_BROADCAST(flags) (((flags) & (1UL << 7)) != 0) + /* Called with in the pinctrl_handler thread context. */ static void pinctrl_handle_put_dhcp_opts( @@ -1190,7 +1192,16 @@ pinctrl_handle_put_dhcp_opts( udp->udp_len = htons(new_l4_size); + /* Send a broadcast IP frame when BROADCAST flag is set */ struct ip_header *out_ip = dp_packet_l3(&pkt_out); + uint32_t ip_dst; + if DHCP_FLAGS_BROADCAST(dhcp_data->flags) { + ip_dst = htonl(0xffffffff); + } else { + ip_dst = *offer_ip; + } + put_16aligned_be32(&out_ip->ip_dst, ip_dst); + out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs + new_l4_size); udp->udp_csum = 0; /* Checksum needs to be initialized to zero. */ diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 3aba0487d..58dfee617 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -4271,10 +4271,9 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip, ds_put_cstr(options_action, "); next;"); ds_put_format(response_action, "eth.dst = eth.src; eth.src = %s; " - "ip4.dst = "IP_FMT"; ip4.src = %s; udp.src = 67; " - "udp.dst = 68; outport = inport; flags.loopback = 1; " - "output;", - server_mac, IP_ARGS(offer_ip), server_ip); + "ip4.src = %s; udp.src = 67; udp.dst = 68; " + "outport = inport; flags.loopback = 1; output;", + server_mac, server_ip); ds_put_format(ipv4_addr_match, "ip4.src == "IP_FMT" && ip4.dst == {%s, 255.255.255.255}", diff --git a/tests/ovn.at b/tests/ovn.at index cbaa6d4a2..a7842a35e 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -4595,10 +4595,11 @@ sleep 2 as hv1 ovs-vsctl show # This shell function sends a DHCP request packet -# test_dhcp INPORT SRC_MAC DHCP_TYPE OFFER_IP REQUEST_IP ... +# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST CIADDR OFFER_IP REQUEST_IP USE_IP ... test_dhcp() { - local inport=$1 src_mac=$2 dhcp_type=$3 ciaddr=$4 offer_ip=$5 request_ip=$6 use_ip=$7 - shift; shift; shift; shift; shift; shift; shift; + local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 ciaddr=$5 offer_ip=$6 request_ip=$7 use_ip=$8 + shift; shift; shift; shift; shift; shift; shift; shift; + if test $use_ip != 0; then src_ip=$1 dst_ip=$2 @@ -4607,6 +4608,7 @@ test_dhcp() { src_ip=`ip_to_hex 0 0 0 0` dst_ip=`ip_to_hex 255 255 255 255` fi + if test $request_ip != 0; then ip_len=0120 udp_len=010b @@ -4614,10 +4616,19 @@ test_dhcp() { ip_len=011a udp_len=0106 fi + + if test $broadcast != 0; then + flags=8000 + reply_dst_ip=`ip_to_hex 255 255 255 255` + else + flags=0000 + reply_dst_ip=${offer_ip} + fi + local request=ffffffffffff${src_mac}08004510${ip_len}0000000080110000${src_ip}${dst_ip} # udp header and dhcp header request=${request}00440043${udp_len}0000 - request=${request}010106006359aa7600000000${ciaddr}000000000000000000000000${src_mac} + request=${request}010106006359aa760000${flags}${ciaddr}000000000000000000000000${src_mac} # client hardware padding request=${request}00000000000000000000 # server hostname @@ -4655,10 +4666,10 @@ test_dhcp() { ip_len=$(printf "%x" $ip_len) udp_len=$(printf "%x" $udp_len) # $ip_len var will be in 3 digits i.e 134. So adding a '0' before $ip_len - local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${offer_ip} + local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${reply_dst_ip} # udp header and dhcp header. # $udp_len var will be in 3 digits. So adding a '0' before $udp_len - reply=${reply}004300440${udp_len}0000020106006359aa7600000000${ciaddr} + reply=${reply}004300440${udp_len}0000020106006359aa760000${flags}${ciaddr} # your ip address; 0 for NAK if test $dhcp_reply_type = 06; then reply=${reply}00000000 @@ -4729,7 +4740,7 @@ server_ip=`ip_to_hex 10 0 0 1` ciaddr=`ip_to_hex 0 0 0 0` request_ip=0 expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 1 f00000000001 01 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts +test_dhcp 1 f00000000001 01 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts # NXT_RESUMEs should be 1. OVS_WAIT_UNTIL([test 1 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4755,7 +4766,7 @@ server_ip=`ip_to_hex 10 0 0 1` ciaddr=`ip_to_hex 0 0 0 0` request_ip=$offer_ip expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts # NXT_RESUMEs should be 2. OVS_WAIT_UNTIL([test 2 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4779,7 +4790,7 @@ server_ip=`ip_to_hex 10 0 0 1` ciaddr=`ip_to_hex 0 0 0 0` request_ip=`ip_to_hex 10 0 0 7` expected_dhcp_opts="" -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 06 $expected_dhcp_opts +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 06 $expected_dhcp_opts # NXT_RESUMEs should be 3. OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4803,7 +4814,7 @@ rm -f 2.expected ciaddr=`ip_to_hex 0 0 0 0` offer_ip=0 request_ip=0 -test_dhcp 2 f00000000002 08 $ciaddr $offer_ip $request_ip 0 1 1 +test_dhcp 2 f00000000002 08 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`]) @@ -4820,12 +4831,12 @@ rm -f 2.expected # ls2-lp2 (vif4-tx.pcap) should receive the DHCPv4 request packet once. ciaddr=`ip_to_hex 0 0 0 0` -test_dhcp 3 f00000000003 01 $ciaddr 0 0 4 0 +test_dhcp 3 f00000000003 01 0 $ciaddr 0 0 4 0 # Send DHCPv4 packet on ls2-lp2. "router" DHCPv4 option is not defined for # this lport. ciaddr=`ip_to_hex 0 0 0 0` -test_dhcp 4 f00000000004 01 $ciaddr 0 0 3 0 +test_dhcp 4 f00000000004 01 0 $ciaddr 0 0 3 0 # NXT_RESUMEs should be 4. OVS_WAIT_UNTIL([test 4 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4842,7 +4853,7 @@ request_ip=0 src_ip=$offer_ip dst_ip=$server_ip expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts +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 5. OVS_WAIT_UNTIL([test 5 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4868,7 +4879,7 @@ request_ip=0 src_ip=$offer_ip dst_ip=`ip_to_hex 255 255 255 255` expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 05 $expected_dhcp_opts +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 6. OVS_WAIT_UNTIL([test 6 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4894,7 +4905,7 @@ request_ip=0 src_ip=$offer_ip dst_ip=`ip_to_hex 255 255 255 255` expected_dhcp_opts="" -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts # NXT_RESUMEs should be 7. OVS_WAIT_UNTIL([test 7 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4920,7 +4931,7 @@ request_ip=0 src_ip=$offer_ip dst_ip=`ip_to_hex 255 255 255 255` expected_dhcp_opts="" -test_dhcp 2 f00000000002 03 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts +test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 1 $src_ip $dst_ip ff1000000001 $server_ip 06 $expected_dhcp_opts # NXT_RESUMEs should be 8. OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4942,7 +4953,7 @@ rm -f 2.expected ciaddr=`ip_to_hex 0 0 0 0` src_ip=`ip_to_hex 10 0 0 6` dst_ip=`ip_to_hex 10 0 0 4` -test_dhcp 2 f00000000002 03 $ciaddr 0 0 1 $src_ip $dst_ip 1 +test_dhcp 2 f00000000002 03 0 $ciaddr 0 0 1 $src_ip $dst_ip 1 # NXT_RESUMEs should be 8. OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -4950,6 +4961,29 @@ OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) # vif1-tx.pcap should have received the DHCPv4 request packet OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) +reset_pcap_file hv1-vif1 hv1/vif1 +reset_pcap_file hv1-vif2 hv1/vif2 +rm -f 1.expected +rm -f 2.expected + +# Send DHCPDISCOVER with BROADCAST flag on. +offer_ip=`ip_to_hex 10 0 0 4` +server_ip=`ip_to_hex 10 0 0 1` +ciaddr=`ip_to_hex 0 0 0 0` +request_ip=0 +expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 +test_dhcp 1 f00000000001 01 1 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 02 $expected_dhcp_opts + +# NXT_RESUMEs should be 9. +OVS_WAIT_UNTIL([test 9 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) + +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets +cat 1.expected | cut -c -48 > expout +AT_CHECK([cat 1.packets | cut -c -48], [0], [expout]) +# Skipping the IPv4 checksum. +cat 1.expected | cut -c 53- > expout +AT_CHECK([cat 1.packets | cut -c 53-], [0], [expout]) + OVN_CLEANUP([hv1]) AT_CLEANUP @@ -13206,10 +13240,11 @@ as hv1 ovs-vsctl show # This shell function sends a DHCP request packet -# test_dhcp INPORT SRC_MAC DHCP_TYPE OFFER_IP ... +# test_dhcp INPORT SRC_MAC DHCP_TYPE BROADCAST OFFER_IP ... test_dhcp() { - local inport=$1 src_mac=$2 dhcp_type=$3 offer_ip=$4 use_ip=$5 - shift; shift; shift; shift; shift; + local inport=$1 src_mac=$2 dhcp_type=$3 broadcast=$4 offer_ip=$5 use_ip=$6 + shift; shift; shift; shift; shift; shift; + if test $use_ip != 0; then src_ip=$1 dst_ip=$2 @@ -13218,10 +13253,19 @@ test_dhcp() { src_ip=`ip_to_hex 0 0 0 0` dst_ip=`ip_to_hex 255 255 255 255` fi + + if test $broadcast != 0; then + flags=8000 + reply_dst_ip=`ip_to_hex 255 255 255 255` + else + flags=0000 + reply_dst_ip=${offer_ip} + fi + local request=ffffffffffff${src_mac}0800451001100000000080110000${src_ip}${dst_ip} # udp header and dhcp header request=${request}0044004300fc0000 - request=${request}010106006359aa760000000000000000000000000000000000000000${src_mac} + request=${request}010106006359aa760000${flags}00000000000000000000000000000000${src_mac} # client hardware padding request=${request}00000000000000000000 # server hostname @@ -13245,10 +13289,10 @@ test_dhcp() { ip_len=$(printf "%x" $ip_len) udp_len=$(printf "%x" $udp_len) # $ip_len var will be in 3 digits i.e 134. So adding a '0' before $ip_len - local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${offer_ip} + local reply=${src_mac}${srv_mac}080045100${ip_len}000000008011XXXX${srv_ip}${reply_dst_ip} # udp header and dhcp header. # $udp_len var will be in 3 digits. So adding a '0' before $udp_len - reply=${reply}004300440${udp_len}0000020106006359aa760000000000000000 + reply=${reply}004300440${udp_len}0000020106006359aa760000${flags}00000000 # your ip address reply=${reply}${offer_ip} # next server ip address, relay agent ip address, client mac address @@ -13367,7 +13411,7 @@ offer_ip=`ip_to_hex 10 0 0 6` server_ip=`ip_to_hex 10 0 0 1` server_mac=ff1000000001 expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ $expected_dhcp_opts # NXT_RESUMEs should be 1 in hv1. @@ -13465,7 +13509,7 @@ offer_ip=`ip_to_hex 10 0 0 6` server_ip=`ip_to_hex 10 0 0 1` server_mac=ff1000000001 expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ $expected_dhcp_opts # NXT_RESUMEs should be 2 in hv1. @@ -13575,7 +13619,7 @@ offer_ip=`ip_to_hex 10 0 0 6` server_ip=`ip_to_hex 10 0 0 1` server_mac=ff1000000001 expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ $expected_dhcp_opts # NXT_RESUMEs should be 3 in hv1. @@ -13655,7 +13699,7 @@ offer_ip=`ip_to_hex 10 0 0 6` server_ip=`ip_to_hex 10 0 0 1` server_mac=ff1000000001 expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a000001 -test_dhcp 1 f00000000003 01 $offer_ip 0 $server_mac $server_ip \ +test_dhcp 1 f00000000003 01 0 $offer_ip 0 $server_mac $server_ip \ $expected_dhcp_opts # NXT_RESUMEs should be 4 in hv1.
As per RFC2131, section 4.1: A server or relay agent sending or relaying a DHCP message directly to a DHCP client (i.e., not to a relay agent specified in the 'giaddr' field) SHOULD examine the BROADCAST bit in the 'flags' field. If this bit is set to 1, the DHCP message SHOULD be sent as an IP broadcast using an IP broadcast address (preferably 0xffffffff) as the IP destination address and the link-layer broadcast address as the link-layer destination address. This patch changes destination IP address to 255.255.255.255 when client set BROADCAST flag in their DHCPREQUEST. Note: the offered IP address is still part of the DHCP payload. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1801006 Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> --- controller/pinctrl.c | 11 +++++ northd/ovn-northd.c | 7 ++-- tests/ovn.at | 98 ++++++++++++++++++++++++++++++++------------ 3 files changed, 85 insertions(+), 31 deletions(-)