Numan Siddique <nusid...@redhat.com> wrote on 07/24/2016 11:21:07 AM:
> From: Numan Siddique <nusid...@redhat.com> > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: nickcooper-zhangtonghao <nickcooper- > zhangtong...@opencloud.tech>, ovs dev <dev@openvswitch.org> > Date: 07/24/2016 11:21 AM > Subject: Re: [ovs-dev] [PATCH] ovn: remove the dead code. > > On Sun, Jul 24, 2016 at 6:26 AM, Ryan Moats <rmo...@us.ibm.com> wrote: > "dev" <dev-boun...@openvswitch.org> wrote on 07/21/2016 05:36:13 AM: > > > From: nickcooper-zhangtonghao <nickcooper-zhangtong...@opencloud.tech> > > To: dev@openvswitch.org > > Cc: nickcooper-zhangtonghao <nickcooper-zhangtong...@opencloud.tech> > > Date: 07/21/2016 05:36 AM > > Subject: [ovs-dev] [PATCH] ovn: remove the dead code. > > Sent by: "dev" <dev-boun...@openvswitch.org> > > > > It is not necessary to assign 0 to ip_csum. > > > > Signed-off-by: nickcooper-zhangtonghao <nickcooper- > > zhangtong...@opencloud.tech> > > --- > > ovn/controller/pinctrl.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > > index 7893872..b35576a 100644 > > --- a/ovn/controller/pinctrl.c > > +++ b/ovn/controller/pinctrl.c > > @@ -349,7 +349,6 @@ pinctrl_handle_put_dhcp_opts( > > struct ip_header *out_ip = dp_packet_l3(&pkt_out); > > out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs + > > new_l4_size); > > udp->udp_csum = 0; > > - out_ip->ip_csum = 0; > > This is not a dead code. It is required. The reason it is required > is because the DHCPv4 response packet is constructed by copying the > DHCPv4 request packet and then adding the dhcp options. So the > ip_csum needs to be set to 0 before calling "csum", otherwise, > csum() will calculate wrong checksum. because of which the VM will > reject the DHCPv4 response packet. > > You can verify yourself by applying the ovn-northd DHCP patch [1]. > > [1] - https://patchwork.ozlabs.org/patch/651958/ > > > > > out_ip->ip_csum = csum(out_ip, sizeof *out_ip); > > > > pin->packet = dp_packet_data(&pkt_out); > > lgtm... > > Acked-by: Ryan Moats <rmo...@us.ibm.com> Ok, I'll admit this is a mistaken ack, but I think adding a comment pointing out that the line is initializing the variable for the pending csum calculation makes it crystal clear for the future. Ryan _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
