257 lines
12 KiB
Diff
257 lines
12 KiB
Diff
From f94b6e0f9997b5c0f9a278f1f365462dff03da51 Mon Sep 17 00:00:00 2001
|
|
From: Ilya Maximets <i.maximets@ovn.org>
|
|
Date: Tue, 1 Oct 2019 18:18:23 +0300
|
|
Subject: flow: Fix using pointer to member of packed struct icmp6_hdr.
|
|
|
|
OVS has no structure definition for ICMPv6 header with additional
|
|
data. More precisely, it has, but this structure named as
|
|
'icmp6_error_header' and only suitable to store error related
|
|
extended information. 'flow_compose_l4' stores additional
|
|
information in reserved bits by using system defined structure
|
|
'icmp6_hdr', which is marked as 'packed' and this leads to
|
|
build failure with gcc >= 9:
|
|
|
|
lib/flow.c:3041:34: error:
|
|
taking address of packed member of 'struct icmp6_hdr' may result
|
|
in an unaligned pointer value [-Werror=address-of-packed-member]
|
|
|
|
uint32_t *reserved = &icmp->icmp6_dataun.icmp6_un_data32[0];
|
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
Fix that by renaming 'icmp6_error_header' to 'icmp6_data_header'
|
|
and allowing it to store not only errors, but any type of additional
|
|
information by analogue with 'struct icmp6_hdr'.
|
|
All the usages of 'struct icmp6_hdr' replaced with this new structure.
|
|
Removed redundant conversions between network and host representations.
|
|
Now fields are always in be.
|
|
|
|
This also, probably, makes flow_compose_l4 more robust by avoiding
|
|
possible unaligned accesses to 32 bit value.
|
|
|
|
Fixes: 9b2b84973db7 ("Support for match & set ICMPv6 reserved and options type fields")
|
|
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
|
Acked-by: William Tu <u9012063@gmail.com>
|
|
Acked-by: Ben Pfaff <blp@ovn.org>
|
|
---
|
|
lib/conntrack.c | 2 +-
|
|
lib/flow.c | 77 ++++++++++++++++++++++++------------------------
|
|
lib/packets.h | 12 +++++---
|
|
ovn/controller/pinctrl.c | 6 ++--
|
|
4 files changed, 51 insertions(+), 46 deletions(-)
|
|
|
|
diff --git a/lib/conntrack.c b/lib/conntrack.c
|
|
index b56ef06ac..43fd510ef 100644
|
|
--- a/lib/conntrack.c
|
|
+++ b/lib/conntrack.c
|
|
@@ -718,7 +718,7 @@ reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn)
|
|
icmp->icmp_csum = csum(icmp, tail - (char *) icmp - pad);
|
|
} else {
|
|
struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
|
|
- struct icmp6_error_header *icmp6 = dp_packet_l4(pkt);
|
|
+ struct icmp6_data_header *icmp6 = dp_packet_l4(pkt);
|
|
struct ovs_16aligned_ip6_hdr *inner_l3_6 =
|
|
(struct ovs_16aligned_ip6_hdr *) (icmp6 + 1);
|
|
/* This call is already verified to succeed during the code path from
|
|
diff --git a/lib/flow.c b/lib/flow.c
|
|
index db84b01f2..0ba146272 100644
|
|
--- a/lib/flow.c
|
|
+++ b/lib/flow.c
|
|
@@ -399,14 +399,14 @@ parse_ethertype(const void **datap, size_t *sizep)
|
|
* and 'arp_buf[]' are filled in. If the packet is not an ND packet, 'false'
|
|
* is returned and no values are filled in on '*nd_target' or 'arp_buf[]'. */
|
|
static inline bool
|
|
-parse_icmpv6(const void **datap, size_t *sizep, const struct icmp6_hdr *icmp,
|
|
- uint32_t *rso_flags, const struct in6_addr **nd_target,
|
|
+parse_icmpv6(const void **datap, size_t *sizep,
|
|
+ const struct icmp6_data_header *icmp6,
|
|
+ ovs_be32 *rso_flags, const struct in6_addr **nd_target,
|
|
struct eth_addr arp_buf[2], uint8_t *opt_type)
|
|
{
|
|
- const uint32_t *reserved;
|
|
- if (icmp->icmp6_code != 0 ||
|
|
- (icmp->icmp6_type != ND_NEIGHBOR_SOLICIT &&
|
|
- icmp->icmp6_type != ND_NEIGHBOR_ADVERT)) {
|
|
+ if (icmp6->icmp6_base.icmp6_code != 0 ||
|
|
+ (icmp6->icmp6_base.icmp6_type != ND_NEIGHBOR_SOLICIT &&
|
|
+ icmp6->icmp6_base.icmp6_type != ND_NEIGHBOR_ADVERT)) {
|
|
return false;
|
|
}
|
|
|
|
@@ -414,12 +414,7 @@ parse_icmpv6(const void **datap, size_t *sizep, const struct icmp6_hdr *icmp,
|
|
arp_buf[1] = eth_addr_zero;
|
|
*opt_type = 0;
|
|
|
|
- reserved = data_try_pull(datap, sizep, sizeof(uint32_t));
|
|
- if (OVS_UNLIKELY(!reserved)) {
|
|
- /* Invalid ND packet. */
|
|
- return false;
|
|
- }
|
|
- *rso_flags = *reserved;
|
|
+ *rso_flags = get_16aligned_be32(icmp6->icmp6_data.be32);
|
|
|
|
*nd_target = data_try_pull(datap, sizep, sizeof **nd_target);
|
|
if (OVS_UNLIKELY(!*nd_target)) {
|
|
@@ -1027,17 +1022,18 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
|
|
miniflow_pad_to_64(mf, igmp_group_ip4);
|
|
}
|
|
} else if (OVS_LIKELY(nw_proto == IPPROTO_ICMPV6)) {
|
|
- if (OVS_LIKELY(size >= sizeof(struct icmp6_hdr))) {
|
|
+ if (OVS_LIKELY(size >= sizeof(struct icmp6_data_header))) {
|
|
const struct in6_addr *nd_target;
|
|
struct eth_addr arp_buf[2];
|
|
/* This will populate whether we received Option 1
|
|
* or Option 2. */
|
|
uint8_t opt_type;
|
|
/* This holds the ND Reserved field. */
|
|
- uint32_t rso_flags;
|
|
- const struct icmp6_hdr *icmp = data_pull(&data,
|
|
- &size,ICMP6_HEADER_LEN);
|
|
- if (parse_icmpv6(&data, &size, icmp,
|
|
+ ovs_be32 rso_flags;
|
|
+ const struct icmp6_data_header *icmp6;
|
|
+
|
|
+ icmp6 = data_pull(&data, &size, sizeof *icmp6);
|
|
+ if (parse_icmpv6(&data, &size, icmp6,
|
|
&rso_flags, &nd_target, arp_buf, &opt_type)) {
|
|
if (nd_target) {
|
|
miniflow_push_words(mf, nd_target, nd_target,
|
|
@@ -1056,16 +1052,20 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
|
|
* This will zero out the tcp_flags & pad3 field. */
|
|
miniflow_pad_to_64(mf, arp_tha);
|
|
}
|
|
- miniflow_push_be16(mf, tp_src, htons(icmp->icmp6_type));
|
|
- miniflow_push_be16(mf, tp_dst, htons(icmp->icmp6_code));
|
|
+ miniflow_push_be16(mf, tp_src,
|
|
+ htons(icmp6->icmp6_base.icmp6_type));
|
|
+ miniflow_push_be16(mf, tp_dst,
|
|
+ htons(icmp6->icmp6_base.icmp6_code));
|
|
miniflow_pad_to_64(mf, tp_dst);
|
|
/* Fill ND reserved field. */
|
|
- miniflow_push_be32(mf, igmp_group_ip4, htonl(rso_flags));
|
|
+ miniflow_push_be32(mf, igmp_group_ip4, rso_flags);
|
|
miniflow_pad_to_64(mf, igmp_group_ip4);
|
|
} else {
|
|
/* ICMPv6 but not ND. */
|
|
- miniflow_push_be16(mf, tp_src, htons(icmp->icmp6_type));
|
|
- miniflow_push_be16(mf, tp_dst, htons(icmp->icmp6_code));
|
|
+ miniflow_push_be16(mf, tp_src,
|
|
+ htons(icmp6->icmp6_base.icmp6_type));
|
|
+ miniflow_push_be16(mf, tp_dst,
|
|
+ htons(icmp6->icmp6_base.icmp6_code));
|
|
miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
|
|
miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
|
|
}
|
|
@@ -3038,15 +3038,16 @@ flow_compose_l4(struct dp_packet *p, const struct flow *flow,
|
|
igmp->igmp_code = ntohs(flow->tp_dst);
|
|
put_16aligned_be32(&igmp->group, flow->igmp_group_ip4);
|
|
} else if (flow->nw_proto == IPPROTO_ICMPV6) {
|
|
- struct icmp6_hdr *icmp = dp_packet_put_zeros(p, sizeof *icmp);
|
|
- icmp->icmp6_type = ntohs(flow->tp_src);
|
|
- icmp->icmp6_code = ntohs(flow->tp_dst);
|
|
- uint32_t *reserved = &icmp->icmp6_dataun.icmp6_un_data32[0];
|
|
- *reserved = ntohl(flow->igmp_group_ip4);
|
|
-
|
|
- if (icmp->icmp6_code == 0 &&
|
|
- (icmp->icmp6_type == ND_NEIGHBOR_SOLICIT ||
|
|
- icmp->icmp6_type == ND_NEIGHBOR_ADVERT)) {
|
|
+ struct icmp6_data_header *icmp6;
|
|
+
|
|
+ icmp6 = dp_packet_put_zeros(p, sizeof *icmp6);
|
|
+ icmp6->icmp6_base.icmp6_type = ntohs(flow->tp_src);
|
|
+ icmp6->icmp6_base.icmp6_code = ntohs(flow->tp_dst);
|
|
+ put_16aligned_be32(icmp6->icmp6_data.be32, flow->igmp_group_ip4);
|
|
+
|
|
+ if (icmp6->icmp6_base.icmp6_code == 0 &&
|
|
+ (icmp6->icmp6_base.icmp6_type == ND_NEIGHBOR_SOLICIT ||
|
|
+ icmp6->icmp6_base.icmp6_type == ND_NEIGHBOR_ADVERT)) {
|
|
struct in6_addr *nd_target;
|
|
struct ovs_nd_lla_opt *lla_opt;
|
|
|
|
@@ -3065,9 +3066,9 @@ flow_compose_l4(struct dp_packet *p, const struct flow *flow,
|
|
lla_opt->type = ND_OPT_TARGET_LINKADDR;
|
|
lla_opt->mac = flow->arp_tha;
|
|
}
|
|
- } else if (icmp->icmp6_code == 0 &&
|
|
- (icmp->icmp6_type == ICMP6_ECHO_REQUEST ||
|
|
- icmp->icmp6_type == ICMP6_ECHO_REPLY)) {
|
|
+ } else if (icmp6->icmp6_base.icmp6_code == 0 &&
|
|
+ (icmp6->icmp6_base.icmp6_type == ICMP6_ECHO_REQUEST ||
|
|
+ icmp6->icmp6_base.icmp6_type == ICMP6_ECHO_REPLY)) {
|
|
flow_compose_l7(p, l7, l7_len);
|
|
} else {
|
|
/* XXX Add inner IP packet for e.g. destination unreachable? */
|
|
@@ -3112,11 +3113,11 @@ flow_compose_l4_csum(struct dp_packet *p, const struct flow *flow,
|
|
igmp->igmp_csum = 0;
|
|
igmp->igmp_csum = csum(igmp, l4_len);
|
|
} else if (flow->nw_proto == IPPROTO_ICMPV6) {
|
|
- struct icmp6_hdr *icmp = dp_packet_l4(p);
|
|
+ struct icmp6_data_header *icmp6 = dp_packet_l4(p);
|
|
|
|
- icmp->icmp6_cksum = 0;
|
|
- icmp->icmp6_cksum = (OVS_FORCE uint16_t)
|
|
- csum_finish(csum_continue(pseudo_hdr_csum, icmp, l4_len));
|
|
+ icmp6->icmp6_base.icmp6_cksum = 0;
|
|
+ icmp6->icmp6_base.icmp6_cksum =
|
|
+ csum_finish(csum_continue(pseudo_hdr_csum, icmp6, l4_len));
|
|
}
|
|
}
|
|
}
|
|
diff --git a/lib/packets.h b/lib/packets.h
|
|
index 05040a8e0..648dbd770 100644
|
|
--- a/lib/packets.h
|
|
+++ b/lib/packets.h
|
|
@@ -961,12 +961,16 @@ struct icmp6_header {
|
|
};
|
|
BUILD_ASSERT_DECL(ICMP6_HEADER_LEN == sizeof(struct icmp6_header));
|
|
|
|
-#define ICMP6_ERROR_HEADER_LEN 8
|
|
-struct icmp6_error_header {
|
|
+#define ICMP6_DATA_HEADER_LEN 8
|
|
+struct icmp6_data_header {
|
|
struct icmp6_header icmp6_base;
|
|
- ovs_be32 icmp6_error_ext;
|
|
+ union {
|
|
+ ovs_16aligned_be32 be32[1];
|
|
+ ovs_be16 be16[2];
|
|
+ uint8_t u8[4];
|
|
+ } icmp6_data;
|
|
};
|
|
-BUILD_ASSERT_DECL(ICMP6_ERROR_HEADER_LEN == sizeof(struct icmp6_error_header));
|
|
+BUILD_ASSERT_DECL(ICMP6_DATA_HEADER_LEN == sizeof(struct icmp6_data_header));
|
|
|
|
uint32_t packet_csum_pseudoheader6(const struct ovs_16aligned_ip6_hdr *);
|
|
ovs_be16 packet_csum_upperlayer6(const struct ovs_16aligned_ip6_hdr *,
|
|
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
|
|
index af68f0f6b..d74e0bc7a 100644
|
|
--- a/ovn/controller/pinctrl.c
|
|
+++ b/ovn/controller/pinctrl.c
|
|
@@ -842,14 +842,14 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow,
|
|
}
|
|
} else {
|
|
struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh);
|
|
- struct icmp6_error_header *ih;
|
|
+ struct icmp6_data_header *ih;
|
|
uint32_t icmpv6_csum;
|
|
|
|
eh->eth_type = htons(ETH_TYPE_IPV6);
|
|
dp_packet_set_l3(&packet, nh);
|
|
nh->ip6_vfc = 0x60;
|
|
nh->ip6_nxt = IPPROTO_ICMPV6;
|
|
- nh->ip6_plen = htons(sizeof(*nh) + ICMP6_ERROR_HEADER_LEN);
|
|
+ nh->ip6_plen = htons(sizeof(*nh) + ICMP6_DATA_HEADER_LEN);
|
|
packet_set_ipv6(&packet, &ip_flow->ipv6_src, &ip_flow->ipv6_dst,
|
|
ip_flow->nw_tos, ip_flow->ipv6_label, 255);
|
|
|
|
@@ -865,7 +865,7 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow,
|
|
icmpv6_csum = packet_csum_pseudoheader6(dp_packet_l3(&packet));
|
|
ih->icmp6_base.icmp6_cksum = csum_finish(
|
|
csum_continue(icmpv6_csum, ih,
|
|
- sizeof(*nh) + ICMP6_ERROR_HEADER_LEN));
|
|
+ sizeof(*nh) + ICMP6_DATA_HEADER_LEN));
|
|
}
|
|
|
|
if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) {
|
|
--
|
|
2.14.1
|
|
|
|
|