From 5a51b2cd3483c6c1719e5ef7091f558d49431351 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Tue, 25 Mar 2014 15:26:23 -0700 Subject: [PATCH] lib/ofpbuf: Remove 'l7' pointer. Now that we don't need to parse TCP flags from the packet after extraction, we usually do not need the 'l7' pointer any more. When needed, ofpbuf_get_tcp|udp|sctp|icmp_payload() or ofpbuf_get_l4_size() can be used instead. Removal of 'l7' was requested by Pravin for the DPDK datapath work, as it simplifies packet parsing a bit. Signed-off-by: Jarno Rajahalme Signed-off-by: Ben Pfaff --- lib/bfd.c | 9 ++++- lib/flow.c | 100 ++++++++++++++++-------------------------------- lib/ofp-print.c | 26 ++++++------- lib/ofpbuf.c | 8 +--- lib/ofpbuf.h | 39 ++++++++++++++++++- lib/packets.c | 11 ++++-- lib/pcap-file.c | 7 ++-- 7 files changed, 103 insertions(+), 97 deletions(-) diff --git a/lib/bfd.c b/lib/bfd.c index d46f99219..cc9e4535c 100644 --- a/lib/bfd.c +++ b/lib/bfd.c @@ -654,6 +654,11 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, enum flags flags; uint8_t version; struct msg *msg; + const uint8_t *l7 = ofpbuf_get_udp_payload(p); + + if (!l7) { + return; /* No UDP payload. */ + } /* This function is designed to follow section RFC 5880 6.8.6 closely. */ @@ -668,11 +673,11 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, goto out; } - msg = ofpbuf_at(p, (uint8_t *)p->l7 - (uint8_t *)p->data, BFD_PACKET_LEN); + msg = ofpbuf_at(p, l7 - (uint8_t *)p->data, BFD_PACKET_LEN); if (!msg) { VLOG_INFO_RL(&rl, "%s: Received too-short BFD control message (only " "%"PRIdPTR" bytes long, at least %d required).", - bfd->name, (uint8_t *) ofpbuf_tail(p) - (uint8_t *) p->l7, + bfd->name, (uint8_t *) ofpbuf_tail(p) - l7, BFD_PACKET_LEN); goto out; } diff --git a/lib/flow.c b/lib/flow.c index f6ec135fd..4e8cc0e07 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -69,31 +69,6 @@ pull_ip(struct ofpbuf *packet) return NULL; } -static struct tcp_header * -pull_tcp(struct ofpbuf *packet) -{ - if (packet->size >= TCP_HEADER_LEN) { - struct tcp_header *tcp = packet->data; - int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4; - if (tcp_len >= TCP_HEADER_LEN && packet->size >= tcp_len) { - return ofpbuf_pull(packet, tcp_len); - } - } - return NULL; -} - -static struct udp_header * -pull_udp(struct ofpbuf *packet) -{ - return ofpbuf_try_pull(packet, UDP_HEADER_LEN); -} - -static struct sctp_header * -pull_sctp(struct ofpbuf *packet) -{ - return ofpbuf_try_pull(packet, SCTP_HEADER_LEN); -} - static struct icmp_header * pull_icmp(struct ofpbuf *packet) { @@ -258,46 +233,46 @@ parse_ipv6(struct ofpbuf *packet, struct flow *flow) } static void -parse_tcp(struct ofpbuf *packet, struct ofpbuf *b, struct flow *flow) +parse_tcp(struct ofpbuf *b, struct flow *flow) { - const struct tcp_header *tcp = pull_tcp(b); - if (tcp) { + if (b->size >= TCP_HEADER_LEN) { + const struct tcp_header *tcp = b->data; + flow->tp_src = tcp->tcp_src; flow->tp_dst = tcp->tcp_dst; flow->tcp_flags = tcp->tcp_ctl & htons(0x0fff); - packet->l7 = b->data; } } static void -parse_udp(struct ofpbuf *packet, struct ofpbuf *b, struct flow *flow) +parse_udp(struct ofpbuf *b, struct flow *flow) { - const struct udp_header *udp = pull_udp(b); - if (udp) { + if (b->size >= UDP_HEADER_LEN) { + const struct udp_header *udp = b->data; + flow->tp_src = udp->udp_src; flow->tp_dst = udp->udp_dst; - packet->l7 = b->data; } } static void -parse_sctp(struct ofpbuf *packet, struct ofpbuf *b, struct flow *flow) +parse_sctp(struct ofpbuf *b, struct flow *flow) { - const struct sctp_header *sctp = pull_sctp(b); - if (sctp) { + if (b->size >= SCTP_HEADER_LEN) { + const struct sctp_header *sctp = b->data; + flow->tp_src = sctp->sctp_src; flow->tp_dst = sctp->sctp_dst; - packet->l7 = b->data; } } -static bool +static void parse_icmpv6(struct ofpbuf *b, struct flow *flow) { const struct icmp6_hdr *icmp = pull_icmpv6(b); if (!icmp) { - return false; + return; } /* The ICMPv6 type and code fields use the 16-bit transport port @@ -312,7 +287,7 @@ parse_icmpv6(struct ofpbuf *b, struct flow *flow) nd_target = ofpbuf_try_pull(b, sizeof *nd_target); if (!nd_target) { - return false; + return; } flow->nd_target = *nd_target; @@ -351,15 +326,14 @@ parse_icmpv6(struct ofpbuf *b, struct flow *flow) } } - return true; + return; invalid: memset(&flow->nd_target, 0, sizeof(flow->nd_target)); memset(flow->arp_sha, 0, sizeof(flow->arp_sha)); memset(flow->arp_tha, 0, sizeof(flow->arp_tha)); - return false; - + return; } /* Initializes 'flow' members from 'packet' and 'md' @@ -376,9 +350,6 @@ invalid: * * - packet->l4 to just past the IPv4 header, if one is present and has a * correct length, and otherwise NULL. - * - * - packet->l7 to just past the TCP/UDP/SCTP/ICMP header, if one is - * present and has a correct length, and otherwise NULL. */ void flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md, @@ -402,7 +373,6 @@ flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md, packet->l2_5 = NULL; packet->l3 = NULL; packet->l4 = NULL; - packet->l7 = NULL; if (b.size < sizeof *eth) { return; @@ -448,17 +418,16 @@ flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md, if (!(nh->ip_frag_off & htons(IP_FRAG_OFF_MASK))) { if (flow->nw_proto == IPPROTO_TCP) { - parse_tcp(packet, &b, flow); + parse_tcp(&b, flow); } else if (flow->nw_proto == IPPROTO_UDP) { - parse_udp(packet, &b, flow); + parse_udp(&b, flow); } else if (flow->nw_proto == IPPROTO_SCTP) { - parse_sctp(packet, &b, flow); + parse_sctp(&b, flow); } else if (flow->nw_proto == IPPROTO_ICMP) { const struct icmp_header *icmp = pull_icmp(&b); if (icmp) { flow->tp_src = htons(icmp->icmp_type); flow->tp_dst = htons(icmp->icmp_code); - packet->l7 = b.data; } } } @@ -470,15 +439,13 @@ flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md, packet->l4 = b.data; if (flow->nw_proto == IPPROTO_TCP) { - parse_tcp(packet, &b, flow); + parse_tcp(&b, flow); } else if (flow->nw_proto == IPPROTO_UDP) { - parse_udp(packet, &b, flow); + parse_udp(&b, flow); } else if (flow->nw_proto == IPPROTO_SCTP) { - parse_sctp(packet, &b, flow); + parse_sctp(&b, flow); } else if (flow->nw_proto == IPPROTO_ICMPV6) { - if (parse_icmpv6(&b, flow)) { - packet->l7 = b.data; - } + parse_icmpv6(&b, flow); } } else if (flow->dl_type == htons(ETH_TYPE_ARP) || flow->dl_type == htons(ETH_TYPE_RARP)) { @@ -1270,7 +1237,7 @@ flow_set_mpls_lse(struct flow *flow, int idx, ovs_be32 lse) flow->mpls_lse[idx] = lse; } -static void +static uint8_t * flow_compose_l4(struct ofpbuf *b, const struct flow *flow) { if (!(flow->nw_frag & FLOW_NW_FRAG_ANY) @@ -1282,21 +1249,21 @@ flow_compose_l4(struct ofpbuf *b, const struct flow *flow) tcp->tcp_src = flow->tp_src; tcp->tcp_dst = flow->tp_dst; tcp->tcp_ctl = TCP_CTL(ntohs(flow->tcp_flags), 5); - b->l7 = ofpbuf_tail(b); + return ofpbuf_tail(b); } else if (flow->nw_proto == IPPROTO_UDP) { struct udp_header *udp; udp = ofpbuf_put_zeros(b, sizeof *udp); udp->udp_src = flow->tp_src; udp->udp_dst = flow->tp_dst; - b->l7 = ofpbuf_tail(b); + return ofpbuf_tail(b); } else if (flow->nw_proto == IPPROTO_SCTP) { struct sctp_header *sctp; sctp = ofpbuf_put_zeros(b, sizeof *sctp); sctp->sctp_src = flow->tp_src; sctp->sctp_dst = flow->tp_dst; - b->l7 = ofpbuf_tail(b); + return ofpbuf_tail(b); } else if (flow->nw_proto == IPPROTO_ICMP) { struct icmp_header *icmp; @@ -1304,7 +1271,7 @@ flow_compose_l4(struct ofpbuf *b, const struct flow *flow) icmp->icmp_type = ntohs(flow->tp_src); icmp->icmp_code = ntohs(flow->tp_dst); icmp->icmp_csum = csum(icmp, ICMP_HEADER_LEN); - b->l7 = ofpbuf_tail(b); + return ofpbuf_tail(b); } else if (flow->nw_proto == IPPROTO_ICMPV6) { struct icmp6_hdr *icmp; @@ -1336,9 +1303,10 @@ flow_compose_l4(struct ofpbuf *b, const struct flow *flow) } icmp->icmp6_cksum = (OVS_FORCE uint16_t) csum(icmp, (char *)ofpbuf_tail(b) - (char *)icmp); - b->l7 = ofpbuf_tail(b); + return ofpbuf_tail(b); } } + return NULL; } /* Puts into 'b' a packet that flow_extract() would parse as having the given @@ -1389,6 +1357,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow) ip->ip_csum = csum(ip, sizeof *ip); } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { struct ovs_16aligned_ip6_hdr *nh; + uint8_t *l7; nh = ofpbuf_put_zeros(b, sizeof *nh); put_16aligned_be32(&nh->ip6_flow, htonl(6 << 28) | @@ -1401,10 +1370,9 @@ flow_compose(struct ofpbuf *b, const struct flow *flow) b->l4 = ofpbuf_tail(b); - flow_compose_l4(b, flow); + l7 = flow_compose_l4(b, flow); - nh->ip6_plen = - b->l7 ? htons((uint8_t *) b->l7 - (uint8_t *) b->l4) : htons(0); + nh->ip6_plen = l7 ? htons(l7 - (uint8_t *) b->l4) : htons(0); } else if (flow->dl_type == htons(ETH_TYPE_ARP) || flow->dl_type == htons(ETH_TYPE_RARP)) { struct arp_eth_header *arp; diff --git a/lib/ofp-print.c b/lib/ofp-print.c index da8940551..599dc322b 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -62,25 +62,23 @@ ofp_packet_to_string(const void *data, size_t len) const struct pkt_metadata md = PKT_METADATA_INITIALIZER(0); struct ofpbuf buf; struct flow flow; + size_t l4_size; ofpbuf_use_const(&buf, data, len); flow_extract(&buf, &md, &flow); flow_format(&ds, &flow); - if (buf.l7) { - if (flow.nw_proto == IPPROTO_TCP) { - struct tcp_header *th = buf.l4; - ds_put_format(&ds, " tcp_csum:%"PRIx16, - ntohs(th->tcp_csum)); - } else if (flow.nw_proto == IPPROTO_UDP) { - struct udp_header *uh = buf.l4; - ds_put_format(&ds, " udp_csum:%"PRIx16, - ntohs(uh->udp_csum)); - } else if (flow.nw_proto == IPPROTO_SCTP) { - struct sctp_header *sh = buf.l4; - ds_put_format(&ds, " sctp_csum:%"PRIx32, - ntohl(sh->sctp_csum)); - } + l4_size = ofpbuf_get_l4_size(&buf); + + if (flow.nw_proto == IPPROTO_TCP && l4_size >= TCP_HEADER_LEN) { + struct tcp_header *th = buf.l4; + ds_put_format(&ds, " tcp_csum:%"PRIx16, ntohs(th->tcp_csum)); + } else if (flow.nw_proto == IPPROTO_UDP && l4_size >= UDP_HEADER_LEN) { + struct udp_header *uh = buf.l4; + ds_put_format(&ds, " udp_csum:%"PRIx16, ntohs(uh->udp_csum)); + } else if (flow.nw_proto == IPPROTO_SCTP && l4_size >= SCTP_HEADER_LEN) { + struct sctp_header *sh = buf.l4; + ds_put_format(&ds, " sctp_csum:%"PRIx32, ntohl(sh->sctp_csum)); } ds_put_char(&ds, '\n'); diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c index c0af06878..9f22836ad 100644 --- a/lib/ofpbuf.c +++ b/lib/ofpbuf.c @@ -30,7 +30,7 @@ ofpbuf_use__(struct ofpbuf *b, void *base, size_t allocated, b->allocated = allocated; b->source = source; b->size = 0; - b->l2 = b->l2_5 = b->l3 = b->l4 = b->l7 = NULL; + b->l2 = b->l2_5 = b->l3 = b->l4 = NULL; list_poison(&b->list_node); b->private_p = NULL; } @@ -182,9 +182,6 @@ ofpbuf_clone_with_headroom(const struct ofpbuf *buffer, size_t headroom) if (buffer->l4) { new_buffer->l4 = (char *) buffer->l4 + data_delta; } - if (buffer->l7) { - new_buffer->l7 = (char *) buffer->l7 + data_delta; - } return new_buffer; } @@ -279,9 +276,6 @@ ofpbuf_resize__(struct ofpbuf *b, size_t new_headroom, size_t new_tailroom) if (b->l4) { b->l4 = (char *) b->l4 + data_delta; } - if (b->l7) { - b->l7 = (char *) b->l7 + data_delta; - } } } diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h index 96b5479fd..bda514d89 100644 --- a/lib/ofpbuf.h +++ b/lib/ofpbuf.h @@ -20,6 +20,7 @@ #include #include #include "list.h" +#include "packets.h" #include "util.h" #ifdef __cplusplus @@ -48,7 +49,6 @@ struct ofpbuf { void *l2_5; /* MPLS label stack */ void *l3; /* Network-level header. */ void *l4; /* Transport-level header. */ - void *l7; /* Application data. */ struct list list_node; /* Private list element for use by owner. */ void *private_p; /* Private pointer for use by owner. */ @@ -212,6 +212,43 @@ static inline bool ofpbuf_equal(const struct ofpbuf *a, const struct ofpbuf *b) return a->size == b->size && memcmp(a->data, b->data, a->size) == 0; } +static inline size_t ofpbuf_get_l4_size(const struct ofpbuf *b) +{ + return b->l4 ? (const char *)ofpbuf_tail(b) - (const char *)b->l4 : 0; +} + +static inline const void *ofpbuf_get_tcp_payload(const struct ofpbuf *b) +{ + size_t l4_size = ofpbuf_get_l4_size(b); + + if (OVS_LIKELY(l4_size >= TCP_HEADER_LEN)) { + struct tcp_header *tcp = b->l4; + int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4; + + if (OVS_LIKELY(tcp_len >= TCP_HEADER_LEN && tcp_len <= l4_size)) { + return (const char *)tcp + tcp_len; + } + } + return NULL; +} + +static inline const void *ofpbuf_get_udp_payload(const struct ofpbuf *b) +{ + return OVS_LIKELY(ofpbuf_get_l4_size(b) >= UDP_HEADER_LEN) + ? (const char *)b->l4 + UDP_HEADER_LEN : NULL; +} + +static inline const void *ofpbuf_get_sctp_payload(const struct ofpbuf *b) +{ + return OVS_LIKELY(ofpbuf_get_l4_size(b) >= SCTP_HEADER_LEN) + ? (const char *)b->l4 + SCTP_HEADER_LEN : NULL; +} + +static inline const void *ofpbuf_get_icmp_payload(const struct ofpbuf *b) +{ + return OVS_LIKELY(ofpbuf_get_l4_size(b) >= ICMP_HEADER_LEN) + ? (const char *)b->l4 + ICMP_HEADER_LEN : NULL; +} #ifdef __cplusplus } diff --git a/lib/packets.c b/lib/packets.c index 65ba3f685..3edfe01a7 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -619,12 +619,13 @@ packet_set_ipv4_addr(struct ofpbuf *packet, { struct ip_header *nh = packet->l3; ovs_be32 old_addr = get_16aligned_be32(addr); + size_t l4_size = ofpbuf_get_l4_size(packet); - if (nh->ip_proto == IPPROTO_TCP && packet->l7) { + if (nh->ip_proto == IPPROTO_TCP && l4_size >= TCP_HEADER_LEN) { struct tcp_header *th = packet->l4; th->tcp_csum = recalc_csum32(th->tcp_csum, old_addr, new_addr); - } else if (nh->ip_proto == IPPROTO_UDP && packet->l7) { + } else if (nh->ip_proto == IPPROTO_UDP && l4_size >= UDP_HEADER_LEN ) { struct udp_header *uh = packet->l4; if (uh->udp_csum) { @@ -727,11 +728,13 @@ static void packet_update_csum128(struct ofpbuf *packet, uint8_t proto, ovs_16aligned_be32 addr[4], const ovs_be32 new_addr[4]) { - if (proto == IPPROTO_TCP && packet->l7) { + size_t l4_size = ofpbuf_get_l4_size(packet); + + if (proto == IPPROTO_TCP && l4_size >= TCP_HEADER_LEN) { struct tcp_header *th = packet->l4; th->tcp_csum = recalc_csum128(th->tcp_csum, addr, new_addr); - } else if (proto == IPPROTO_UDP && packet->l7) { + } else if (proto == IPPROTO_UDP && l4_size >= UDP_HEADER_LEN) { struct udp_header *uh = packet->l4; if (uh->udp_csum) { diff --git a/lib/pcap-file.c b/lib/pcap-file.c index e2fd20397..b449eff8b 100644 --- a/lib/pcap-file.c +++ b/lib/pcap-file.c @@ -303,15 +303,16 @@ tcp_reader_run(struct tcp_reader *r, const struct flow *flow, uint32_t hash; uint32_t seq; uint8_t flags; + const char *l7 = ofpbuf_get_tcp_payload(packet); if (flow->dl_type != htons(ETH_TYPE_IP) || flow->nw_proto != IPPROTO_TCP - || !packet->l7) { + || !l7) { return NULL; } tcp = packet->l4; flags = TCP_FLAGS(tcp->tcp_ctl); - l7_length = (char *) ofpbuf_tail(packet) - (char *) packet->l7; + l7_length = (char *) ofpbuf_tail(packet) - l7; seq = ntohl(get_16aligned_be32(&tcp->tcp_seq)); /* Construct key. */ @@ -347,7 +348,7 @@ tcp_reader_run(struct tcp_reader *r, const struct flow *flow, * continually expanding it. */ ofpbuf_shift(payload, (char *) payload->base - (char *) payload->data); - ofpbuf_put(payload, packet->l7, l7_length); + ofpbuf_put(payload, l7, l7_length); stream->seq_no += l7_length; return payload; } else { -- 2.47.0