From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.windriver.com (mail.windriver.com [147.11.1.11]) by mail.openembedded.org (Postfix) with ESMTP id 99C986B6B7 for ; Thu, 12 Feb 2015 01:30:56 +0000 (UTC) Received: from ALA-HCA.corp.ad.wrs.com (ala-hca.corp.ad.wrs.com [147.11.189.40]) by mail.windriver.com (8.14.9/8.14.5) with ESMTP id t1C1Uvm3006382 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL) for ; Wed, 11 Feb 2015 17:30:58 -0800 (PST) Received: from [128.224.162.194] (128.224.162.194) by ALA-HCA.corp.ad.wrs.com (147.11.189.40) with Microsoft SMTP Server id 14.3.174.1; Wed, 11 Feb 2015 17:30:57 -0800 Message-ID: <54DC024C.6090402@windriver.com> Date: Thu, 12 Feb 2015 09:30:52 +0800 From: Hongxu Jia User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Rob Woolley , References: <1422654909-8058-1-git-send-email-rob.woolley@windriver.com> <20150206025639.GA30800@yow-feoserver.windriver.com> <54DBF2EA.1090604@windriver.com> In-Reply-To: <54DBF2EA.1090604@windriver.com> Subject: Re: [PATCH] dhcp-client: Ignore partial checksums X-BeenThere: openembedded-core@lists.openembedded.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Patches and discussions about the oe-core layer List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Feb 2015 01:30:59 -0000 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit On 02/12/2015 08:25 AM, Rob Woolley wrote: > Hi Hongxu, > > Do you have any feedback on this patch? Are there any changes you > would like me to make? > Sorry for the late, I am fine with it. //Hongxu > Regards, > Rob > > On 02/05/2015 09:56 PM, Rob Woolley wrote: >> Hi Hongxu, >> >> Have you had a chance to review this patch? Do you have any >> questions about it? >> >> Regards, >> Rob >> >> >> On Fri, Jan 30, 2015 at 04:55:09PM -0500, Rob Woolley wrote: >>> dhclient will fail to get an IP address if run inside a guest when >>> traffic is >>> flowing over a virtual network interface. The user will see the error >>> message: >>> >>> 5 bad udp checksums in 5 packets >>> No DHCPOFFERS received. >>> Unable to obtain a lease on first try. Exiting. >>> Failed to bring up eth0. >>> >>> This is because Linux only uses partial checksums for packets that >>> go over >>> virtual network interfaces and dhclient does not like this. >>> >>> See linux kernel commit 78ea85f17b15390e30d8b47488ec7b6cf0790663 >>> ("net: skbuff: improve comment on checksumming") >>> >>> An application can detect this behaviour by checking for the >>> TP_STATUS_CSUMNOTREADY flag in the tp_status field. >>> >>> See linux kernel commit 8dc4194474159660d7f37c495e3fc3f10d0db8cc >>> ("Add optional checksum computation for recvmsg") >>> >>> An extra parameter is added to decode_udp_ip_header() in dhclient to >>> indicate >>> whether or not dhclient should ignore partial checksums. This is used >>> when the TP_STATUS_CSUMNOTREADY bit is set by the guest kernel. >>> >>> This fix has been included in Fedora and Ubuntu, however it has not >>> yet been >>> accepted by ISC upstream. Likely because it is specific to >>> behaviour in Linux >>> and other UNIX variants do not seem to be affected. >>> >>> The patch was imported from the dhcp source RPM in Fedora 21 >>> (http://pkgs.fedoraproject.org/cgit/dhcp.git/tree/dhcp-xen-checksum.patch?h=f21) >>> >>> Originally contributed to fedora-cvs-commit by David Cantrell on Jan >>> 30 2007 >>> (https://www.redhat.com/archives/fedora-cvs-commits/2007-January/msg01442.html) >>> >>> Submitted to dhcp-bugs@isc.org - [ISC-Bugs #22806] - by Michael S. >>> Tsirkin >>> (http://comments.gmane.org/gmane.comp.emulators.kvm.devel/65236) >>> (https://lists.isc.org/pipermail/dhcp-hackers/2010-April/001835.html) >>> >>> Upstream-Status: Submitted [dhcp-bugs@isc.org] >>> Signed-off-by: Rob Woolley >>> --- >>> .../dhcp/dhcp/dhcp-xen-checksum.patch | 282 >>> +++++++++++++++++++++ >>> meta/recipes-connectivity/dhcp/dhcp_4.3.1.bb | 1 + >>> 2 files changed, 283 insertions(+) >>> create mode 100644 >>> meta/recipes-connectivity/dhcp/dhcp/dhcp-xen-checksum.patch >>> >>> Index: b/meta/recipes-connectivity/dhcp/dhcp/dhcp-xen-checksum.patch >>> =================================================================== >>> --- /dev/null >>> +++ b/meta/recipes-connectivity/dhcp/dhcp/dhcp-xen-checksum.patch >>> @@ -0,0 +1,307 @@ >>> +dhcp-client: Ignore partial checksums >>> + >>> +dhclient will fail to get an IP address if run inside a guest when >>> traffic is >>> +flowing over a virtual network interface. The user will see the error >>> +message: >>> + >>> + 5 bad udp checksums in 5 packets >>> + No DHCPOFFERS received. >>> + Unable to obtain a lease on first try. Exiting. >>> + Failed to bring up eth0. >>> + >>> +This is because Linux only uses partial checksums for packets that >>> go over >>> +virtual network interfaces and dhclient does not like this. >>> + >>> + See linux kernel commit 78ea85f17b15390e30d8b47488ec7b6cf0790663 >>> + ("net: skbuff: improve comment on checksumming") >>> + >>> +An application can detect this behaviour by checking for the >>> +TP_STATUS_CSUMNOTREADY flag in the tp_status field. >>> + >>> + See linux kernel commit 8dc4194474159660d7f37c495e3fc3f10d0db8cc >>> + ("Add optional checksum computation for recvmsg") >>> + >>> +An extra parameter is added to decode_udp_ip_header() in dhclient >>> to indicate >>> +whether or not dhclient should ignore partial checksums. This is used >>> +when the TP_STATUS_CSUMNOTREADY bit is set by the guest kernel. >>> + >>> +This fix has been included in Fedora and Ubuntu, however it has not >>> yet been >>> +accepted by ISC upstream. Likely because it is specific to >>> behaviour in Linux >>> +and other UNIX variants do not seem to be affected. >>> + >>> +The patch was imported from the dhcp source RPM in Fedora 21 >>> + >>> (http://pkgs.fedoraproject.org/cgit/dhcp.git/tree/dhcp-xen-checksum.patch?h=f21) >>> + >>> +Originally contributed to fedora-cvs-commit by David Cantrell on >>> Jan 30 2007 >>> + >>> (https://www.redhat.com/archives/fedora-cvs-commits/2007-January/msg01442.html) >>> + >>> +Submitted to dhcp-bugs@isc.org - [ISC-Bugs #22806] - by Michael S. >>> Tsirkin >>> + (http://comments.gmane.org/gmane.comp.emulators.kvm.devel/65236) >>> + (https://lists.isc.org/pipermail/dhcp-hackers/2010-April/001835.html) >>> + >>> +Upstream-Status: Submitted [dhcp-bugs@isc.org] >>> +Signed-off-by: Rob Woolley >>> +-- >>> + common/bpf.c | 2 - >>> + common/dlpi.c | 2 - >>> + common/lpf.c | 83 >>> +++++++++++++++++++++++++++++++++++++++++-------------- >>> + common/nit.c | 2 - >>> + common/packet.c | 4 +- >>> + common/upf.c | 2 - >>> + includes/dhcpd.h | 2 - >>> + 7 files changed, 70 insertions(+), 27 deletions(-) >>> + >>> +diff --git a/common/bpf.c b/common/bpf.c >>> +--- a/common/bpf.c >>> ++++ b/common/bpf.c >>> +@@ -481,7 +481,7 @@ ssize_t receive_packet (interface, buf, >>> + /* Decode the IP and UDP headers... */ >>> + offset = decode_udp_ip_header(interface, interface->rbuf, >>> + interface->rbuf_offset, >>> +- from, hdr.bh_caplen, &paylen); >>> ++ from, hdr.bh_caplen, &paylen, 0); >>> + >>> + /* If the IP or UDP checksum was bad, skip the packet... */ >>> + if (offset < 0) { >>> +diff --git a/common/dlpi.c b/common/dlpi.c >>> +--- a/common/dlpi.c >>> ++++ b/common/dlpi.c >>> +@@ -691,7 +691,7 @@ ssize_t receive_packet (interface, buf, >>> + length -= offset; >>> + #endif >>> + offset = decode_udp_ip_header (interface, dbuf, bufix, >>> +- from, length, &paylen); >>> ++ from, length, &paylen, 0); >>> + >>> + /* >>> + * If the IP or UDP checksum was bad, skip the packet... >>> +diff --git a/common/lpf.c b/common/lpf.c >>> +--- a/common/lpf.c >>> ++++ b/common/lpf.c >>> +@@ -29,14 +29,15 @@ >>> + >>> + #include "dhcpd.h" >>> + #if defined (USE_LPF_SEND) || defined (USE_LPF_RECEIVE) >>> ++#include >>> + #include >>> + #include >>> + >>> + #include >>> + #include >>> + #include >>> ++#include >>> + #include >>> +-#include >>> + #include "includes/netinet/ip.h" >>> + #include "includes/netinet/udp.h" >>> + #include "includes/netinet/if_ether.h" >>> +@@ -51,6 +52,19 @@ >>> + /* Reinitializes the specified interface after an address >>> change. This >>> + is not required for packet-filter APIs. */ >>> + >>> ++#ifndef PACKET_AUXDATA >>> ++#define PACKET_AUXDATA 8 >>> ++ >>> ++struct tpacket_auxdata >>> ++{ >>> ++ __u32 tp_status; >>> ++ __u32 tp_len; >>> ++ __u32 tp_snaplen; >>> ++ __u16 tp_mac; >>> ++ __u16 tp_net; >>> ++}; >>> ++#endif >>> ++ >>> + #ifdef USE_LPF_SEND >>> + void if_reinitialize_send (info) >>> + struct interface_info *info; >>> +@@ -73,10 +87,14 @@ int if_register_lpf (info) >>> + struct interface_info *info; >>> + { >>> + int sock; >>> +- struct sockaddr sa; >>> ++ union { >>> ++ struct sockaddr_ll ll; >>> ++ struct sockaddr common; >>> ++ } sa; >>> ++ struct ifreq ifr; >>> + >>> + /* Make an LPF socket. */ >>> +- if ((sock = socket(PF_PACKET, SOCK_PACKET, >>> ++ if ((sock = socket(PF_PACKET, SOCK_RAW, >>> + htons((short)ETH_P_ALL))) < 0) { >>> + if (errno == ENOPROTOOPT || errno == EPROTONOSUPPORT || >>> + errno == ESOCKTNOSUPPORT || errno == EPFNOSUPPORT || >>> +@@ -91,11 +109,17 @@ int if_register_lpf (info) >>> + log_fatal ("Open a socket for LPF: %m"); >>> + } >>> + >>> ++ memset (&ifr, 0, sizeof ifr); >>> ++ strncpy (ifr.ifr_name, (const char *)info -> ifp, sizeof >>> ifr.ifr_name); >>> ++ ifr.ifr_name[IFNAMSIZ-1] = '\0'; >>> ++ if (ioctl (sock, SIOCGIFINDEX, &ifr)) >>> ++ log_fatal ("Failed to get interface index: %m"); >>> ++ >>> + /* Bind to the interface name */ >>> + memset (&sa, 0, sizeof sa); >>> +- sa.sa_family = AF_PACKET; >>> +- strncpy (sa.sa_data, (const char *)info -> ifp, sizeof >>> sa.sa_data); >>> +- if (bind (sock, &sa, sizeof sa)) { >>> ++ sa.ll.sll_family = AF_PACKET; >>> ++ sa.ll.sll_ifindex = ifr.ifr_ifindex; >>> ++ if (bind (sock, &sa.common, sizeof sa)) { >>> + if (errno == ENOPROTOOPT || errno == EPROTONOSUPPORT || >>> + errno == ESOCKTNOSUPPORT || errno == EPFNOSUPPORT || >>> + errno == EAFNOSUPPORT || errno == EINVAL) { >>> +@@ -177,9 +201,18 @@ static void lpf_gen_filter_setup (struct >>> + void if_register_receive (info) >>> + struct interface_info *info; >>> + { >>> ++ int val; >>> ++ >>> + /* Open a LPF device and hang it on this interface... */ >>> + info -> rfdesc = if_register_lpf (info); >>> + >>> ++ val = 1; >>> ++ if (setsockopt (info -> rfdesc, SOL_PACKET, PACKET_AUXDATA, &val, >>> ++ sizeof val) < 0) { >>> ++ if (errno != ENOPROTOOPT) >>> ++ log_fatal ("Failed to set auxiliary packet data: %m"); >>> ++ } >>> ++ >>> + #if defined (HAVE_TR_SUPPORT) >>> + if (info -> hw_address.hbuf [0] == HTYPE_IEEE802) >>> + lpf_tr_filter_setup (info); >>> +@@ -301,7 +334,6 @@ ssize_t send_packet (interface, packet, >>> + double hh [16]; >>> + double ih [1536 / sizeof (double)]; >>> + unsigned char *buf = (unsigned char *)ih; >>> +- struct sockaddr_pkt sa; >>> + int result; >>> + int fudge; >>> + >>> +@@ -322,17 +354,7 @@ ssize_t send_packet (interface, packet, >>> + (unsigned char *)raw, len); >>> + memcpy (buf + ibufp, raw, len); >>> + >>> +- /* For some reason, SOCK_PACKET sockets can't be connected, >>> +- so we have to do a sentdo every time. */ >>> +- memset (&sa, 0, sizeof sa); >>> +- sa.spkt_family = AF_PACKET; >>> +- strncpy ((char *)sa.spkt_device, >>> +- (const char *)interface -> ifp, sizeof sa.spkt_device); >>> +- sa.spkt_protocol = htons(ETH_P_IP); >>> +- >>> +- result = sendto (interface -> wfdesc, >>> +- buf + fudge, ibufp + len - fudge, 0, >>> +- (const struct sockaddr *)&sa, sizeof sa); >>> ++ result = write (interface -> wfdesc, buf + fudge, ibufp + len >>> - fudge); >>> + if (result < 0) >>> + log_error ("send_packet: %m"); >>> + return result; >>> +@@ -349,14 +371,35 @@ ssize_t receive_packet (interface, buf, >>> + { >>> + int length = 0; >>> + int offset = 0; >>> ++ int nocsum = 0; >>> + unsigned char ibuf [1536]; >>> + unsigned bufix = 0; >>> + unsigned paylen; >>> ++ unsigned char cmsgbuf[CMSG_LEN(sizeof(struct tpacket_auxdata))]; >>> ++ struct iovec iov = { >>> ++ .iov_base = ibuf, >>> ++ .iov_len = sizeof ibuf, >>> ++ }; >>> ++ struct msghdr msg = { >>> ++ .msg_iov = &iov, >>> ++ .msg_iovlen = 1, >>> ++ .msg_control = cmsgbuf, >>> ++ .msg_controllen = sizeof(cmsgbuf), >>> ++ }; >>> ++ struct cmsghdr *cmsg; >>> + >>> +- length = read (interface -> rfdesc, ibuf, sizeof ibuf); >>> ++ length = recvmsg (interface -> rfdesc, &msg, 0); >>> + if (length <= 0) >>> + return length; >>> + >>> ++ for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = >>> CMSG_NXTHDR(&msg, cmsg)) { >>> ++ if (cmsg->cmsg_level == SOL_PACKET && >>> ++ cmsg->cmsg_type == PACKET_AUXDATA) { >>> ++ struct tpacket_auxdata *aux = (void *)CMSG_DATA(cmsg); >>> ++ nocsum = aux->tp_status & TP_STATUS_CSUMNOTREADY; >>> ++ } >>> ++ } >>> ++ >>> + bufix = 0; >>> + /* Decode the physical header... */ >>> + offset = decode_hw_header (interface, ibuf, bufix, hfrom); >>> +@@ -373,7 +416,7 @@ ssize_t receive_packet (interface, buf, >>> + >>> + /* Decode the IP and UDP headers... */ >>> + offset = decode_udp_ip_header (interface, ibuf, bufix, from, >>> +- (unsigned)length, &paylen); >>> ++ (unsigned)length, &paylen, nocsum); >>> + >>> + /* If the IP or UDP checksum was bad, skip the packet... */ >>> + if (offset < 0) >>> +diff --git a/common/nit.c b/common/nit.c >>> +--- a/common/nit.c >>> ++++ b/common/nit.c >>> +@@ -363,7 +363,7 @@ ssize_t receive_packet (interface, buf, >>> + >>> + /* Decode the IP and UDP headers... */ >>> + offset = decode_udp_ip_header (interface, ibuf, bufix, >>> +- from, length, &paylen); >>> ++ from, length, &paylen, 0); >>> + >>> + /* If the IP or UDP checksum was bad, skip the packet... */ >>> + if (offset < 0) >>> +diff --git a/common/packet.c b/common/packet.c >>> +--- a/common/packet.c >>> ++++ b/common/packet.c >>> +@@ -226,7 +226,7 @@ ssize_t >>> + decode_udp_ip_header(struct interface_info *interface, >>> + unsigned char *buf, unsigned bufix, >>> + struct sockaddr_in *from, unsigned buflen, >>> +- unsigned *rbuflen) >>> ++ unsigned *rbuflen, int nocsum) >>> + { >>> + unsigned char *data; >>> + struct ip ip; >>> +@@ -337,7 +337,7 @@ decode_udp_ip_header(struct interface_in >>> + 8, IPPROTO_UDP + ulen)))); >>> + >>> + udp_packets_seen++; >>> +- if (usum && usum != sum) { >>> ++ if (!nocsum && usum && usum != sum) { >>> + udp_packets_bad_checksum++; >>> + if (udp_packets_seen > 4 && >>> + (udp_packets_seen / udp_packets_bad_checksum) < 2) { >>> +diff --git a/common/upf.c b/common/upf.c >>> +--- a/common/upf.c >>> ++++ b/common/upf.c >>> +@@ -314,7 +314,7 @@ ssize_t receive_packet (interface, buf, >>> + >>> + /* Decode the IP and UDP headers... */ >>> + offset = decode_udp_ip_header (interface, ibuf, bufix, >>> +- from, length, &paylen); >>> ++ from, length, &paylen, 0); >>> + >>> + /* If the IP or UDP checksum was bad, skip the packet... */ >>> + if (offset < 0) >>> +diff --git a/includes/dhcpd.h b/includes/dhcpd.h >>> +--- a/includes/dhcpd.h >>> ++++ b/includes/dhcpd.h >>> +@@ -2857,7 +2857,7 @@ ssize_t decode_hw_header (struct interfa >>> + unsigned, struct hardware *); >>> + ssize_t decode_udp_ip_header (struct interface_info *, unsigned >>> char *, >>> + unsigned, struct sockaddr_in *, >>> +- unsigned, unsigned *); >>> ++ unsigned, unsigned *, int); >>> + >>> + /* ethernet.c */ >>> + void assemble_ethernet_header (struct interface_info *, unsigned >>> char *, >>> +-- >>> +1.8.1.2 >>> + >>> Index: b/meta/recipes-connectivity/dhcp/dhcp_4.3.1.bb >>> =================================================================== >>> --- a/meta/recipes-connectivity/dhcp/dhcp_4.3.1.bb >>> +++ b/meta/recipes-connectivity/dhcp/dhcp_4.3.1.bb >>> @@ -6,6 +6,7 @@ SRC_URI += "file://dhcp-3.0.3-dhclient-d >>> file://fixsepbuild.patch \ >>> file://dhclient-script-drop-resolv.conf.dhclient.patch \ >>> file://replace-ifconfig-route.patch \ >>> + file://dhcp-xen-checksum.patch \ >>> " >>> SRC_URI[md5sum] = "b3a42ece3c7f2cd2e74a3e12ca881d20" >>> -- >>> _______________________________________________ >>> Openembedded-core mailing list >>> Openembedded-core@lists.openembedded.org >>> http://lists.openembedded.org/mailman/listinfo/openembedded-core >>> >