[ovs-dev] [PATCH v2] dpif-netdev: Delay packets' metadata initialization.
Daniele Di Proietto
diproiettod at vmware.com
Wed Feb 3 02:30:12 UTC 2016
Thanks!
I added your ack and pushed this to master and branch-2.5
On 02/02/2016 04:52, "Chandran, Sugesh" <sugesh.chandran at 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:diproiettod at vmware.com]
>> Sent: Tuesday, February 2, 2016 3:28 AM
>> To: dev at openvswitch.org
>> Cc: Daniele Di Proietto <diproiettod at vmware.com>; Chandran, Sugesh
>> <sugesh.chandran at 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.chandran at intel.com>
>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>> Acked-by: Andy Zhou <azhou at 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
>
More information about the dev
mailing list