| Message ID | 20160311192921.GN2891@indiana.gru.redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Fri, Mar 11, 2016 at 11:29 AM, Thadeu Lima de Souza Cascardo <cascardo@redhat.com> wrote: > On Wed, Mar 09, 2016 at 04:40:42PM -0800, Pravin B Shelar wrote: >> Following patch fixes number of issues with compose nd, like >> setting ip packet header, set ICMP opt-len, checksum. >> >> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> >> --- >> lib/packets.c | 60 +++++++++++++++++++++++++++++++++++++++++------------------ >> 1 file changed, 42 insertions(+), 18 deletions(-) >> >> diff --git a/lib/packets.c b/lib/packets.c >> index daca1b3..6e2c68b 100644 >> --- a/lib/packets.c >> +++ b/lib/packets.c >> @@ -794,7 +794,7 @@ eth_compose(struct dp_packet *b, const struct eth_addr eth_dst, >> dp_packet_prealloc_tailroom(b, 2 + ETH_HEADER_LEN + VLAN_HEADER_LEN + size); >> dp_packet_reserve(b, 2 + VLAN_HEADER_LEN); >> eth = dp_packet_put_uninit(b, ETH_HEADER_LEN); >> - data = dp_packet_put_uninit(b, size); >> + data = dp_packet_put_zeros(b, size); >> >> eth->eth_dst = eth_dst; >> eth->eth_src = eth_src; >> @@ -845,7 +845,7 @@ packet_rh_present(struct dp_packet *packet) >> size_t remaining; >> uint8_t *data = dp_packet_l3(packet); >> >> - remaining = packet->l4_ofs - packet->l3_ofs; >> + remaining = dp_packet_size(packet) - packet->l3_ofs; >> >> if (remaining < sizeof *nh) { >> return false; >> @@ -1027,9 +1027,7 @@ packet_set_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 src[4], >> } >> >> packet_set_ipv6_tc(&nh->ip6_flow, key_tc); >> - >> packet_set_ipv6_flow_label(&nh->ip6_flow, key_fl); >> - >> nh->ip6_hlim = key_hl; >> } >> >> @@ -1116,7 +1114,8 @@ packet_set_icmp(struct dp_packet *packet, uint8_t type, uint8_t code) >> >> void >> packet_set_nd(struct dp_packet *packet, const ovs_be32 target[4], >> - const struct eth_addr sll, const struct eth_addr tll) { >> + const struct eth_addr sll, const struct eth_addr tll) >> +{ >> struct ovs_nd_msg *ns; >> struct ovs_nd_opt *nd_opt; >> int bytes_remain = dp_packet_l4_size(packet); >> @@ -1288,34 +1287,60 @@ compose_arp(struct dp_packet *b, uint16_t arp_op, >> dp_packet_set_l3(b, arp); >> } >> >> +/* This function expect packet with ehernet header with correct >> + * l3 pointer set. */ >> +static void * >> +compose_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 src[4], >> + const ovs_be32 dst[4], uint8_t key_tc, ovs_be32 key_fl, >> + uint8_t key_hl, int size) >> +{ >> + struct ip6_hdr *nh; >> + void *data; >> + >> + nh = dp_packet_l3(packet); >> + nh->ip6_vfc = 0x60; >> + nh->ip6_nxt = proto; >> + nh->ip6_plen = htons(size); >> + data = dp_packet_put_zeros(packet, size); >> + dp_packet_set_l4(packet, data); >> + packet_set_ipv6(packet, proto, src, dst, key_tc, key_fl, key_hl); >> + return data; >> +} >> + >> void >> compose_nd(struct dp_packet *b, const struct eth_addr eth_src, >> - struct in6_addr * ipv6_src, struct in6_addr * ipv6_dst) >> + struct in6_addr * ipv6_src, struct in6_addr *ipv6_dst) > > Remove the extra space before ipv6_src too. > > Otherwise, seems good to me. It passes my manual test of setting up two hosts > connected through IPv6 + VXLAN, and copying a file with scp between them. > > Also, it pass my checksum tests. I have a patch to test-csum, inline below, and > I have added a receive test VXLAN with checksummed UDP. The test packet was > checked using tcpdump and wireshark, ie, they both claim good checksum. This and > other patches I have written (some similar to yours, but besides the tests, none > needed with your patchset) are hosted here [1], in case anyone would like to > take a look, review, or use them. > > [1] http://git.cascardo.eti.br/?p=cascardo/ovs.git;a=shortlog;h=refs/heads/ipv6 > thanks for review. I have added test to check ARP/ND request messages from vswitchd.
diff --git a/tests/library.at b/tests/library.at index d82113f..114bf16 100644 --- a/tests/library.at +++ b/tests/library.at @@ -7,7 +7,7 @@ AT_CHECK([ovstest test-flows flows pcap], [0], [checked 247 packets, 0 errors AT_CLEANUP AT_SETUP([test TCP/IP checksumming]) -AT_CHECK([ovstest test-csum], [0], [....#....#....####................................#................................# +AT_CHECK([ovstest test-csum], [0], [....#....#....#####................................#................................# ]) AT_CLEANUP diff --git a/tests/test-csum.c b/tests/test-csum.c index 97638b9..36cdb2b 100644 --- a/tests/test-csum.c +++ b/tests/test-csum.c @@ -21,6 +21,7 @@ #include <inttypes.h> #include <netinet/in.h> #include <netinet/ip.h> +#include <netinet/ip6.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -209,6 +210,38 @@ test_pseudo(void) mark('#'); } +/* Check the IPv6 pseudoheader calculation. */ +static void +test_pseudov6(void) +{ + ovs_be16 csum; + /* Try an IPv6 header similar to one that the tunnel code + * might generate. */ + struct ovs_16aligned_ip6_hdr ip6 = { + .ip6_vfc = 0x60, + .ip6_hlim = 64, + .ip6_nxt = IPPROTO_UDP, + .ip6_plen = htons(1410), + .ip6_src = { .be16 = { htons(0x2001), htons(0xcafe), 0, 0, 0, 0, 0, htons(0x92) } }, + .ip6_dst = { .be16 = { htons(0x2001), htons(0xcafe), 0, 0, 0, 0, 0, htons(0x91) } } + }; + + csum = csum_finish(packet_csum_pseudoheader6(&ip6)); + assert(csum == htons(0x234a)); + + /* And also test something totally different to check for + * corner cases. */ + memset(&ip6, 0xff, sizeof ip6); + csum = csum_finish(packet_csum_pseudoheader6(&ip6)); + assert(csum == htons(0xff00)); + + memset(&ip6, 0x00, sizeof ip6); + csum = csum_finish(packet_csum_pseudoheader6(&ip6)); + assert(csum == htons(0xffff)); + + mark('#'); +} + static void test_csum_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) { @@ -273,6 +306,7 @@ test_csum_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) test_rfc1624(); test_crc32c(); test_pseudo(); + test_pseudov6(); /* Test recalc_csum16(). */ for (i = 0; i < 32; i++) {