Thanks! I added your ack and pushed this to master and branch-2.5
On 02/02/2016 04:52, "Chandran, Sugesh" <sugesh.chand...@intel.com> wrote: >Hi Daniele, >I feel this approach is fine to go ahead. At least it looks to me more >readable than the earlier one. >Once again thank you for sending out the patch. > >Acked! > >Regards >_Sugesh > > >> -----Original Message----- >> From: Daniele Di Proietto [mailto:diproiet...@vmware.com] >> Sent: Tuesday, February 2, 2016 3:28 AM >> To: dev@openvswitch.org >> Cc: Daniele Di Proietto <diproiet...@vmware.com>; Chandran, Sugesh >> <sugesh.chand...@intel.com> >> Subject: [PATCH v2] dpif-netdev: Delay packets' metadata initialization. >> >> When a group of packets arrives from a port, we loop through them to >> initialize metadata and then we loop through them again to extract the >> flow and perform the exact match classification. >> >> This commit combines the two loops into one, and initializes packet->md >> in emc_processing() to improve performance. >> >> Since emc_processing() might also be called after recirculation (in >> which case the metadata is already valid), an extra parameter is added >> to support both cases. >> >> This commits also implements simple prefetching of packet metadata, >> to further improve performance. >> >> CC: Chandran, Sugesh <sugesh.chand...@intel.com> >> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com> >> Acked-by: Andy Zhou <az...@ovn.org> >> --- >> lib/dpif-netdev.c | 61 +++++++++++++++++++++++++++++++++++++++---- >> ------------ >> lib/packets.h | 8 ++++++++ >> 2 files changed, 52 insertions(+), 17 deletions(-) >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index 9abb948..f39b125 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -478,7 +478,9 @@ static void dp_netdev_execute_actions(struct >> dp_netdev_pmd_thread *pmd, >> const struct nlattr *actions, >> size_t actions_len); >> static void dp_netdev_input(struct dp_netdev_pmd_thread *, >> - struct dp_packet **, int cnt); >> + struct dp_packet **, int cnt, odp_port_t >>port_no); >> +static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *, >> + struct dp_packet **, int cnt); >> >> static void dp_netdev_disable_upcall(struct dp_netdev *); >> static void dp_netdev_pmd_reload_done(struct dp_netdev_pmd_thread >> *pmd); >> @@ -2555,16 +2557,10 @@ dp_netdev_process_rxq_port(struct >> dp_netdev_pmd_thread *pmd, >> error = netdev_rxq_recv(rxq, packets, &cnt); >> cycles_count_end(pmd, PMD_CYCLES_POLLING); >> if (!error) { >> - int i; >> - >> *recirc_depth_get() = 0; >> >> - /* XXX: initialize md in netdev implementation. */ >> - for (i = 0; i < cnt; i++) { >> - pkt_metadata_init(&packets[i]->md, port->port_no); >> - } >> cycles_count_start(pmd); >> - dp_netdev_input(pmd, packets, cnt); >> + dp_netdev_input(pmd, packets, cnt, port->port_no); >> cycles_count_end(pmd, PMD_CYCLES_PROCESSING); >> } else if (error != EAGAIN && error != EOPNOTSUPP) { >> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); >> @@ -3284,17 +3280,21 @@ dp_netdev_queue_batches(struct dp_packet >> *pkt, >> } >> >> /* Try to process all ('cnt') the 'packets' using only the exact match >>cache >> - * 'flow_cache'. If a flow is not found for a packet 'packets[i]', the >> + * 'pmd->flow_cache'. If a flow is not found for a packet >>'packets[i]', the >> * miniflow is copied into 'keys' and the packet pointer is moved at >>the >> * beginning of the 'packets' array. >> * >> * The function returns the number of packets that needs to be >>processed in >> the >> * 'packets' array (they have been moved to the beginning of the >>vector). >> + * >> + * If 'md_is_valid' is false, the metadata in 'packets' is not valid >>and must be >> + * initialized by this function using 'port_no'. >> */ >> static inline size_t >> emc_processing(struct dp_netdev_pmd_thread *pmd, struct dp_packet >> **packets, >> size_t cnt, struct netdev_flow_key *keys, >> - struct packet_batch batches[], size_t *n_batches) >> + struct packet_batch batches[], size_t *n_batches, >> + bool md_is_valid, odp_port_t port_no) >> { >> struct emc_cache *flow_cache = &pmd->flow_cache; >> size_t i, n_missed = 0, n_dropped = 0; >> @@ -3309,10 +3309,14 @@ emc_processing(struct dp_netdev_pmd_thread >> *pmd, struct dp_packet **packets, >> } >> >> if (i != cnt - 1) { >> - /* Prefetch next packet data */ >> + /* Prefetch next packet data and metadata. */ >> OVS_PREFETCH(dp_packet_data(packets[i+1])); >> + pkt_metadata_prefetch_init(&packets[i+1]->md); >> } >> >> + if (!md_is_valid) { >> + pkt_metadata_init(&packets[i]->md, port_no); >> + } >> struct netdev_flow_key *key = &keys[n_missed]; >> miniflow_extract(packets[i], &key->mf); >> key->len = 0; /* Not computed yet. */ >> @@ -3473,9 +3477,16 @@ fast_path_processing(struct >> dp_netdev_pmd_thread *pmd, >> dp_netdev_count_packet(pmd, DP_STAT_LOST, lost_cnt); >> } >> >> +/* Packets enter the datapath from a port (or from recirculation) here. >> + * >> + * For performance reasons a caller may choose not to initialize the >> metadata >> + * in 'packets': in this case 'mdinit' is false and this function >>needs to >> + * initialize it using 'port_no'. If the metadata in 'packets' is >>already >> + * valid, 'md_is_valid' must be true and 'port_no' will be ignored. */ >> static void >> -dp_netdev_input(struct dp_netdev_pmd_thread *pmd, >> - struct dp_packet **packets, int cnt) >> +dp_netdev_input__(struct dp_netdev_pmd_thread *pmd, >> + struct dp_packet **packets, int cnt, >> + bool md_is_valid, odp_port_t port_no) >> { >> #if !defined(__CHECKER__) && !defined(_WIN32) >> const size_t PKT_ARRAY_SIZE = cnt; >> @@ -3489,7 +3500,8 @@ dp_netdev_input(struct dp_netdev_pmd_thread >> *pmd, >> size_t newcnt, n_batches, i; >> >> n_batches = 0; >> - newcnt = emc_processing(pmd, packets, cnt, keys, batches, >>&n_batches); >> + newcnt = emc_processing(pmd, packets, cnt, keys, batches, >>&n_batches, >> + md_is_valid, port_no); >> if (OVS_UNLIKELY(newcnt)) { >> fast_path_processing(pmd, packets, newcnt, keys, batches, >> &n_batches); >> } >> @@ -3503,6 +3515,21 @@ dp_netdev_input(struct dp_netdev_pmd_thread >> *pmd, >> } >> } >> >> +static void >> +dp_netdev_input(struct dp_netdev_pmd_thread *pmd, >> + struct dp_packet **packets, int cnt, >> + odp_port_t port_no) >> +{ >> + dp_netdev_input__(pmd, packets, cnt, false, port_no); >> +} >> + >> +static void >> +dp_netdev_recirculate(struct dp_netdev_pmd_thread *pmd, >> + struct dp_packet **packets, int cnt) >> +{ >> + dp_netdev_input__(pmd, packets, cnt, true, 0); >> +} >> + >> struct dp_netdev_execute_aux { >> struct dp_netdev_pmd_thread *pmd; >> }; >> @@ -3606,7 +3633,7 @@ dp_execute_cb(void *aux_, struct dp_packet >> **packets, int cnt, >> err = push_tnl_action(dp, a, packets, cnt); >> if (!err) { >> (*depth)++; >> - dp_netdev_input(pmd, packets, cnt); >> + dp_netdev_recirculate(pmd, packets, cnt); >> (*depth)--; >> } else { >> dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal); >> @@ -3637,7 +3664,7 @@ dp_execute_cb(void *aux_, struct dp_packet >> **packets, int cnt, >> } >> >> (*depth)++; >> - dp_netdev_input(pmd, packets, cnt); >> + dp_netdev_recirculate(pmd, packets, cnt); >> (*depth)--; >> } else { >> dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal); >> @@ -3695,7 +3722,7 @@ dp_execute_cb(void *aux_, struct dp_packet >> **packets, int cnt, >> } >> >> (*depth)++; >> - dp_netdev_input(pmd, packets, cnt); >> + dp_netdev_recirculate(pmd, packets, cnt); >> (*depth)--; >> >> return; >> diff --git a/lib/packets.h b/lib/packets.h >> index 834e8a4..f1445de 100644 >> --- a/lib/packets.h >> +++ b/lib/packets.h >> @@ -163,6 +163,14 @@ pkt_metadata_init(struct pkt_metadata *md, >> odp_port_t port) >> md->in_port.odp_port = port; >> } >> >> +/* This function prefetches the cachelines touched by >>pkt_metadata_init() >> + * For performance reasons the two functions should be kept in sync. */ >> +static inline void >> +pkt_metadata_prefetch_init(struct pkt_metadata *md) >> +{ >> + ovs_prefetch_range(md, offsetof(struct pkt_metadata, >>tunnel.ip_src)); >> +} >> + >> bool dpid_from_string(const char *s, uint64_t *dpidp); >> >> #define ETH_ADDR_LEN 6 >> -- >> 2.1.4 > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev