Ben Pfaff <b...@ovn.org> wrote on 07/24/2016 12:56:29 PM: > From: Ben Pfaff <b...@ovn.org> > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: Numan Siddique <nusid...@redhat.com>, ovs dev > <dev@openvswitch.org>, nickcooper-zhangtonghao <nickcooper- > zhangtong...@opencloud.tech> > Date: 07/24/2016 12:56 PM > Subject: Re: [ovs-dev] [PATCH] ovn: remove the dead code. > > 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.
Such a patch now exists at http://patchwork.ozlabs.org/patch/652122/ _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
