On Sun, Jul 24, 2016 at 12:04:43PM -0500, Ryan Moats wrote: > 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.
Sure, that would be a welcome patch. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
