From b5e7e61a990fdb5c178e0ba80cb604c8eb48d27d Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Wed, 26 Feb 2014 18:08:04 -0800 Subject: [PATCH] lib: simplify flow_extract() API MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Change the flow_extract() API to accept struct pkt_metadata, instead of individual metadata fields. It will make the API more logical and easier to maintain when we need to expand metadata down the road. Signed-off-by: Andy Zhou Acked-by: Jarno Rajahalme ¬ --- lib/dpif-netdev.c | 8 +++----- lib/flow.c | 22 ++++++++++------------ lib/flow.h | 6 +++--- lib/learning-switch.c | 5 ++--- lib/odp-util.c | 9 ++++----- lib/ofp-print.c | 4 +++- lib/packets.c | 21 +++++++++++++++++++++ lib/packets.h | 10 ++++++++-- ofproto/ofproto-dpif-upcall.c | 5 +++-- ofproto/ofproto-dpif-xlate.c | 5 ++--- ofproto/ofproto-dpif.c | 9 ++++++--- ofproto/ofproto.c | 10 ++++------ tests/test-flows.c | 6 +++--- utilities/ovs-ofctl.c | 6 ++++-- 14 files changed, 76 insertions(+), 50 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index d17fa0541..5897f8bae 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1454,8 +1454,7 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) } /* Extract flow key. */ - flow_extract(execute->packet, md->skb_priority, md->pkt_mark, &md->tunnel, - (union flow_in_port *)&md->in_port, &key); + flow_extract(execute->packet, md, &key); ovs_rwlock_rdlock(&dp->port_rwlock); dp_netdev_execute_actions(dp, &key, execute->packet, md, execute->actions, @@ -1627,8 +1626,8 @@ dp_forwarder_main(void *f_) if (!error) { struct pkt_metadata md = PKT_METADATA_INITIALIZER(port->port_no); - dp_netdev_port_input(dp, &packet, &md); + dp_netdev_port_input(dp, &packet, &md); received_anything = true; } else if (error != EAGAIN && error != EOPNOTSUPP) { static struct vlog_rate_limit rl @@ -1728,8 +1727,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf *packet, if (packet->size < ETH_HEADER_LEN) { return; } - flow_extract(packet, md->skb_priority, md->pkt_mark, &md->tunnel, - (union flow_in_port *)&md->in_port, &key); + flow_extract(packet, md, &key); netdev_flow = dp_netdev_lookup_flow(dp, &key); if (netdev_flow) { struct dp_netdev_actions *actions; diff --git a/lib/flow.c b/lib/flow.c index e7fe4d349..82d672931 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -35,6 +35,7 @@ #include "ofpbuf.h" #include "openflow/openflow.h" #include "packets.h" +#include "odp-util.h" #include "random.h" #include "unaligned.h" @@ -361,8 +362,7 @@ invalid: } -/* Initializes 'flow' members from 'packet', 'skb_priority', 'tnl', and - * 'in_port'. +/* Initializes 'flow' members from 'packet' and 'md' * * Initializes 'packet' header pointers as follows: * @@ -381,8 +381,7 @@ invalid: * present and has a correct length, and otherwise NULL. */ void -flow_extract(struct ofpbuf *packet, uint32_t skb_priority, uint32_t pkt_mark, - const struct flow_tnl *tnl, const union flow_in_port *in_port, +flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md, struct flow *flow) { struct ofpbuf b = *packet; @@ -392,15 +391,14 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority, uint32_t pkt_mark, memset(flow, 0, sizeof *flow); - if (tnl) { - ovs_assert(tnl != &flow->tunnel); - flow->tunnel = *tnl; - } - if (in_port) { - flow->in_port = *in_port; + if (md) { + flow->tunnel = md->tunnel; + if (md->in_port.odp_port != ODPP_NONE) { + flow->in_port = md->in_port; + }; + flow->skb_priority = md->skb_priority; + flow->pkt_mark = md->pkt_mark; } - flow->skb_priority = skb_priority; - flow->pkt_mark = pkt_mark; packet->l2 = b.data; packet->l2_5 = NULL; diff --git a/lib/flow.h b/lib/flow.h index 3109a84a2..8165bcf79 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -32,6 +32,7 @@ struct ds; struct flow_wildcards; struct minimask; struct ofpbuf; +struct pkt_metadata; /* This sequence number should be incremented whenever anything involving flows * or the wildcarding of flows changes. This will cause build assertion @@ -72,8 +73,8 @@ struct flow_tnl { * numbers and other times datapath (dpif) port numbers. This union allows * access to both. */ union flow_in_port { - ofp_port_t ofp_port; odp_port_t odp_port; + ofp_port_t ofp_port; }; /* Maximum number of supported MPLS labels. */ @@ -173,8 +174,7 @@ struct flow_metadata { ofp_port_t in_port; /* OpenFlow port or zero. */ }; -void flow_extract(struct ofpbuf *, uint32_t priority, uint32_t mark, - const struct flow_tnl *, const union flow_in_port *in_port, +void flow_extract(struct ofpbuf *, const struct pkt_metadata *md, struct flow *); void flow_zero_wildcards(struct flow *, const struct flow_wildcards *); diff --git a/lib/learning-switch.c b/lib/learning-switch.c index 8efbce1a5..56209909f 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -556,7 +556,6 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) struct ofpbuf pkt; struct flow flow; - union flow_in_port in_port_; error = ofputil_decode_packet_in(&pi, oh); if (error) { @@ -574,8 +573,8 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) /* Extract flow data from 'opi' into 'flow'. */ ofpbuf_use_const(&pkt, pi.packet, pi.packet_len); - in_port_.ofp_port = pi.fmd.in_port; - flow_extract(&pkt, 0, 0, NULL, &in_port_, &flow); + flow_extract(&pkt, NULL, &flow); + flow.in_port.ofp_port = pi.fmd.in_port; flow.tunnel.tun_id = pi.fmd.tun_id; /* Choose output port. */ diff --git a/lib/odp-util.c b/lib/odp-util.c index e20564f29..d86dbbb36 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -2646,8 +2646,8 @@ odp_key_from_pkt_metadata(struct ofpbuf *buf, const struct pkt_metadata *md) /* Add an ingress port attribute if 'odp_in_port' is not the magical * value "ODPP_NONE". */ - if (md->in_port != ODPP_NONE) { - nl_msg_put_odp_port(buf, OVS_KEY_ATTR_IN_PORT, md->in_port); + if (md->in_port.odp_port != ODPP_NONE) { + nl_msg_put_odp_port(buf, OVS_KEY_ATTR_IN_PORT, md->in_port.odp_port); } } @@ -2662,8 +2662,7 @@ odp_key_to_pkt_metadata(const struct nlattr *key, size_t key_len, 1u << OVS_KEY_ATTR_SKB_MARK | 1u << OVS_KEY_ATTR_TUNNEL | 1u << OVS_KEY_ATTR_IN_PORT; - memset(md, 0, sizeof *md); - md->in_port = ODPP_NONE; + *md = PKT_METADATA_INITIALIZER(ODPP_NONE); NL_ATTR_FOR_EACH (nla, left, key, key_len) { uint16_t type = nl_attr_type(nla); @@ -2690,7 +2689,7 @@ odp_key_to_pkt_metadata(const struct nlattr *key, size_t key_len, wanted_attrs &= ~(1u << OVS_KEY_ATTR_TUNNEL); } } else if (type == OVS_KEY_ATTR_IN_PORT) { - md->in_port = nl_attr_get_odp_port(nla); + md->in_port.odp_port = nl_attr_get_odp_port(nla); wanted_attrs &= ~(1u << OVS_KEY_ATTR_IN_PORT); } diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 4c89b3676..06e64f662 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -46,6 +46,7 @@ #include "packets.h" #include "type-props.h" #include "unaligned.h" +#include "odp-util.h" #include "util.h" static void ofp_print_queue_name(struct ds *string, uint32_t port); @@ -58,11 +59,12 @@ char * ofp_packet_to_string(const void *data, size_t len) { struct ds ds = DS_EMPTY_INITIALIZER; + const struct pkt_metadata md = PKT_METADATA_INITIALIZER(ODPP_NONE); struct ofpbuf buf; struct flow flow; ofpbuf_use_const(&buf, data, len); - flow_extract(&buf, 0, 0, NULL, NULL, &flow); + flow_extract(&buf, &md, &flow); flow_format(&ds, &flow); if (buf.l7) { diff --git a/lib/packets.c b/lib/packets.c index 7238f42e8..3f7d6ebfd 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -29,6 +29,7 @@ #include "dynamic-string.h" #include "ofpbuf.h" #include "ovs-thread.h" +#include "odp-util.h" #include "unaligned.h" const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT; @@ -991,3 +992,23 @@ packet_format_tcp_flags(struct ds *s, uint16_t tcp_flags) ds_put_cstr(s, "[800]"); } } + +void pkt_metadata_init(struct pkt_metadata *md, const struct flow_tnl *tnl, + const uint32_t skb_priority, + const uint32_t pkt_mark, + const union flow_in_port *in_port) +{ + + tnl ? memcpy(&md->tunnel, tnl, sizeof(md->tunnel)) + : memset(&md->tunnel, 0, sizeof(md->tunnel)); + + md->skb_priority = skb_priority; + md->pkt_mark = pkt_mark; + md->in_port.odp_port = in_port ? in_port->odp_port : ODPP_NONE; +} + +void pkt_metadata_from_flow(struct pkt_metadata *md, const struct flow *flow) +{ + pkt_metadata_init(md, &flow->tunnel, flow->skb_priority, + flow->pkt_mark, &flow->in_port); +} diff --git a/lib/packets.h b/lib/packets.h index 1855a1c5c..e6b330380 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -36,11 +36,17 @@ struct pkt_metadata { struct flow_tnl tunnel; /* Encapsulating tunnel parameters. */ uint32_t skb_priority; /* Packet priority for QoS. */ uint32_t pkt_mark; /* Packet mark. */ - odp_port_t in_port; /* Input port. */ + union flow_in_port in_port; /* Input port. */ }; #define PKT_METADATA_INITIALIZER(PORT) \ - (struct pkt_metadata){ { 0, 0, 0, 0, 0, 0}, 0, 0, (PORT) } + (struct pkt_metadata){ { 0, 0, 0, 0, 0, 0}, 0, 0, {(PORT)} } + +void pkt_metadata_init(struct pkt_metadata *md, const struct flow_tnl *tnl, + const uint32_t skb_priority, + const uint32_t pkt_mark, + const union flow_in_port *in_port); +void pkt_metadata_from_flow(struct pkt_metadata *md, const struct flow *flow); bool dpid_from_string(const char *s, uint64_t *dpidp); diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index c50694b86..7dbd7f7e2 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -992,9 +992,10 @@ handle_upcalls(struct handler *handler, struct list *upcalls) type = classify_upcall(upcall); if (type == MISS_UPCALL) { uint32_t hash; + struct pkt_metadata md; - flow_extract(packet, flow.skb_priority, flow.pkt_mark, - &flow.tunnel, &flow.in_port, &miss->flow); + pkt_metadata_from_flow(&md, &flow); + flow_extract(packet, &md, &miss->flow); hash = flow_hash(&miss->flow, 0); existing_miss = flow_miss_find(&misses, ofproto, &miss->flow, diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 89d92af58..eb4931ec3 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3184,12 +3184,11 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet) struct xport *xport; struct ofpact_output output; struct flow flow; - union flow_in_port in_port_; ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output); /* Use OFPP_NONE as the in_port to avoid special packet processing. */ - in_port_.ofp_port = OFPP_NONE; - flow_extract(packet, 0, 0, NULL, &in_port_, &flow); + flow_extract(packet, NULL, &flow); + flow.in_port.ofp_port = OFPP_NONE; ovs_rwlock_rdlock(&xlate_rwlock); xport = xport_lookup(ofport); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index af21dff31..8c43ee997 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -2997,7 +2997,7 @@ ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto, execute.md.tunnel = flow->tunnel; execute.md.skb_priority = flow->skb_priority; execute.md.pkt_mark = flow->pkt_mark; - execute.md.in_port = ofp_port_to_odp_port(ofproto, in_port); + execute.md.in_port.odp_port = ofp_port_to_odp_port(ofproto, in_port); execute.needs_help = (xout.slow & SLOW_ACTION) != 0; error = dpif_execute(ofproto->backer->dpif, &execute); @@ -3784,11 +3784,14 @@ parse_flow_and_packet(int argc, const char *argv[], flow_compose(packet, flow); } else { union flow_in_port in_port = flow->in_port; + struct pkt_metadata md; /* Use the metadata from the flow and the packet argument * to reconstruct the flow. */ - flow_extract(packet, flow->skb_priority, flow->pkt_mark, NULL, - &in_port, flow); + pkt_metadata_init(&md, NULL, flow->skb_priority, + flow->pkt_mark, &in_port); + + flow_extract(packet, &md, flow); } } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 7117e1fb6..19e709173 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2713,11 +2713,10 @@ run_rule_executes(struct ofproto *ofproto) guarded_list_pop_all(&ofproto->rule_executes, &executes); LIST_FOR_EACH_SAFE (e, next, list_node, &executes) { - union flow_in_port in_port_; struct flow flow; - in_port_.ofp_port = e->in_port; - flow_extract(e->packet, 0, 0, NULL, &in_port_, &flow); + flow_extract(e->packet, NULL, &flow); + flow.in_port.ofp_port = e->in_port; ofproto->ofproto_class->rule_execute(e->rule, &flow, e->packet); rule_execute_destroy(e); @@ -2926,7 +2925,6 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh) uint64_t ofpacts_stub[1024 / 8]; struct ofpbuf ofpacts; struct flow flow; - union flow_in_port in_port_; enum ofperr error; COVERAGE_INC(ofproto_packet_out); @@ -2960,8 +2958,8 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh) } /* Verify actions against packet, then send packet if successful. */ - in_port_.ofp_port = po.in_port; - flow_extract(payload, 0, 0, NULL, &in_port_, &flow); + flow_extract(payload, NULL, &flow); + flow.in_port.ofp_port = po.in_port; error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len); if (!error) { error = p->ofproto_class->packet_out(p, payload, &flow, diff --git a/tests/test-flows.c b/tests/test-flows.c index 291003527..a49814295 100644 --- a/tests/test-flows.c +++ b/tests/test-flows.c @@ -58,7 +58,6 @@ main(int argc OVS_UNUSED, char *argv[]) struct ofp10_match extracted_match; struct match match; struct flow flow; - union flow_in_port in_port_; n++; retval = ovs_pcap_read(pcap, &packet, NULL); @@ -68,8 +67,9 @@ main(int argc OVS_UNUSED, char *argv[]) ovs_fatal(retval, "error reading pcap file"); } - in_port_.ofp_port = u16_to_ofp(1); - flow_extract(packet, 0, 0, NULL, &in_port_, &flow); + flow_extract(packet, NULL, &flow); + flow.in_port.ofp_port = u16_to_ofp(1); + match_wc_init(&match, &flow); ofputil_match_to_ofp10_match(&match, &extracted_match); diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 4ab9ca454..e62e64691 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -1864,12 +1864,13 @@ ofctl_ofp_parse_pcap(int argc OVS_UNUSED, char *argv[]) struct ofpbuf *packet; long long int when; struct flow flow; + const struct pkt_metadata md = PKT_METADATA_INITIALIZER(ODPP_NONE); error = ovs_pcap_read(file, &packet, &when); if (error) { break; } - flow_extract(packet, 0, 0, NULL, NULL, &flow); + flow_extract(packet, &md, &flow); if (flow.dl_type == htons(ETH_TYPE_IP) && flow.nw_proto == IPPROTO_TCP && (is_openflow_port(flow.tp_src, argv + 2) || @@ -3208,6 +3209,7 @@ ofctl_parse_pcap(int argc OVS_UNUSED, char *argv[]) for (;;) { struct ofpbuf *packet; struct flow flow; + const struct pkt_metadata md = PKT_METADATA_INITIALIZER(ODPP_NONE); int error; error = ovs_pcap_read(pcap, &packet, NULL); @@ -3217,7 +3219,7 @@ ofctl_parse_pcap(int argc OVS_UNUSED, char *argv[]) ovs_fatal(error, "%s: read failed", argv[1]); } - flow_extract(packet, 0, 0, NULL, NULL, &flow); + flow_extract(packet, &md, &flow); flow_print(stdout, &flow); putchar('\n'); ofpbuf_delete(packet); -- 2.43.0