Thanks for the reviews,
I¹m about to send a v2
On 9/19/14, 3:49 PM, "Pravin Shelar" <pshe...@nicira.com> wrote:
>On Fri, Sep 19, 2014 at 1:28 PM, Daniele Di Proietto
><ddiproie...@vmware.com> wrote:
>> If a packet didn't match a rule in the fast path classifier its memory
>>was
>> never freed. The issue was particularly clear with DPDK devices because
>>it was
>> not possible to process more than ~250000 DPDK mbufs in the slow path.
>>
>> This commit fixes the problem by:
>> * calling dpif_packet_delete() if the upcalls are disabled
>> * passing may_steal==true to dp_netdev_execute_actions() during normal
>>upcall
>> processing
>>
>> Signed-off-by: Daniele Di Proietto <ddiproie...@vmware.com>
>> ---
>> lib/dpif-netdev.c | 47 ++++++++++++++++++++++++++++++++---------------
>> 1 file changed, 32 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 7e27acf..8df3b81 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -2576,7 +2576,7 @@ fast_path_processing(struct dp_netdev_pmd_thread
>>*pmd,
>> /* We can't allow the packet batching in the next loop to
>>execute
>> * the actions. Otherwise, if there are any slow path
>>actions,
>> * we'll send the packet up twice. */
>> - dp_netdev_execute_actions(pmd, &packets[i], 1, false, md,
>> + dp_netdev_execute_actions(pmd, &packets[i], 1, true, md,
>> ofpbuf_data(&actions),
>> ofpbuf_size(&actions));
>>
>> @@ -2602,6 +2602,17 @@ fast_path_processing(struct dp_netdev_pmd_thread
>>*pmd,
>> ofpbuf_uninit(&actions);
>> ofpbuf_uninit(&put_actions);
>> fat_rwlock_unlock(&dp->upcall_rwlock);
>> + } else if (OVS_UNLIKELY(any_miss)) {
>> + int dropped_cnt = 0;
>> +
>> + for (i = 0; i < cnt; i++) {
>> + if (OVS_UNLIKELY(!rules[i] && mfs[i])) {
>> + dpif_packet_delete(packets[i]);
>> + dropped_cnt++;
>> + }
>> + }
>> +
>> + dp_netdev_count_packet(dp, DP_STAT_LOST, dropped_cnt);
>> }
>>
>> n_batches = 0;
>> @@ -2658,6 +2669,18 @@ dpif_netdev_register_upcall_cb(struct dpif
>>*dpif, upcall_callback *cb,
>> }
>>
>> static void
>> +dp_netdev_drop_packets(struct dpif_packet ** packets, int cnt, bool
>>may_steal)
>> +{
>> + int i;
>> +
>> + if (may_steal) {
>> + for (i = 0; i < cnt; i++) {
>> + dpif_packet_delete(packets[i]);
>> + }
>> + }
>> +}
>> +
>> +static void
>> dp_execute_cb(void *aux_, struct dpif_packet **packets, int cnt,
>> struct pkt_metadata *md,
>> const struct nlattr *a, bool may_steal)
>> @@ -2676,10 +2699,8 @@ dp_execute_cb(void *aux_, struct dpif_packet
>>**packets, int cnt,
>> p = dp_netdev_lookup_port(dp, u32_to_odp(nl_attr_get_u32(a)));
>> if (OVS_LIKELY(p)) {
>> netdev_send(p->netdev, pmd->core_id, packets, cnt,
>>may_steal);
>> - } else if (may_steal) {
>> - for (i = 0; i < cnt; i++) {
>> - dpif_packet_delete(packets[i]);
>> - }
>> + } else {
>> + dp_netdev_drop_packets(packets, cnt, may_steal);
>> }
>> break;
>>
>> @@ -2702,17 +2723,17 @@ dp_execute_cb(void *aux_, struct dpif_packet
>>**packets, int cnt,
>> DPIF_UC_ACTION, userdata,
>>&actions,
>> NULL);
>> if (!error || error == ENOSPC) {
>> - dp_netdev_execute_actions(pmd, &packets[i], 1,
>>false, md,
>> - ofpbuf_data(&actions),
>> + dp_netdev_execute_actions(pmd, &packets[i], 1,
>>may_steal,
>> + md,
>>ofpbuf_data(&actions),
>> ofpbuf_size(&actions));
>> - }
>> -
>> - if (may_steal) {
>> + } else if (may_steal) {
>> dpif_packet_delete(packets[i]);
>> }
>> }
>> ofpbuf_uninit(&actions);
>> fat_rwlock_unlock(&dp->upcall_rwlock);
>> + } else {
>> + dp_netdev_drop_packets(packets, cnt, may_steal);
>> }
>>
>> break;
>> @@ -2772,11 +2793,7 @@ dp_execute_cb(void *aux_, struct dpif_packet
>>**packets, int cnt,
>> break;
>> } else {
>> VLOG_WARN("Packet dropped. Max recirculation depth
>>exceeded.");
>> - if (may_steal) {
>> - for (i = 0; i < cnt; i++) {
>> - dpif_packet_delete(packets[i]);
>> - }
>> - }
>> + dp_netdev_drop_packets(packets, cnt, may_steal);
>> }
>> break;
>>
>You can move calls dp_netdev_drop_packets() at the bottom of
>dp_execute_cb()
>All successful switch-case can return immediately and error cases can
>drop packets if required.
>
>otherwise looks good.
>
>> --
>> 2.1.0.rc1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>>
>>https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman
>>/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5z%2
>>BU6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=RJWTUn8k5wT8pIg3bYcX0DQWkHIld%2BNwsgol
>>7TuQSHE%3D%0A&s=d8f26757a3a68813f4809cc2aece9235e2ad0455a421bacd321b2538f
>>03483a2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
