Hi Tom
thanks for your email
On Apr 25, 2011, at 11:35 AM, Tom Zhang wrote:
> Hi,
>
> This is Tom. When I am using pf_ring for fast packet capture, I found two
> possible bugs. Please take a look together.
>
> 1, in skb_ring_handler() of kernel pf_ring.c file
> FlowSlot *theSlot = get_slot(pfr, pfr->slots_info->insert_idx);
> // race condition here.
> if((theSlot == NULL) || (theSlot->slot_state == 0 /* Not full
> */)) {
> /* We've found the ring where the packet can be stored */
> add_skb_to_ring(skb, pfr, &hdr,
> is_ip_pkt, displ,
> channel_id, num_rx_channels);
> rc = 1; /* Ring found: we've done our job */
> break;
> }
>
> This part of code has problems under SMP. Since there is no lock
> ring_index_lock, the softirq may run into this part of code simultineously.
> One core is changing the insert_index, but the other core already read the
> slot, but find the slot_state is 1. I have got the packet loss and add a
> prink here.
>
> The fix should remove this check. Since in add_skb_to_ring, there is write
> lock ring_index_lock inside for race condition.
>
You're right but in the current SVN code, the problem is gone (I think) as we
have changed the logic. Please resync and let me know
> 2, in pfring_read() function of userland pfring.c file, line 1014
> queuedPkts caculation when counter wrapping.
> //64bits case
> queuedPkts = 0xFFFFFFFFFFFFFFFF -+ ring->slots_info->tot_insert -
> ring->slots_info->tot_read;
>
> Tot_insert and tot_read is not the insert_index and read_index. Its boundary
> is not ring->slots_info->tot_slots.
>
> Though 64bits overflow may cost one hundreds of years, but the code should
> keep right.
This code is also gone in SVN
Regards Luca
>
> Thanks,
> Tom
>
> _______________________________________________
> Ntop-dev mailing list
> [email protected]
> http://listgateway.unipi.it/mailman/listinfo/ntop-dev
---
If you can not measure it, you can not improve it - Lord Kelvin
_______________________________________________
Ntop-dev mailing list
[email protected]
http://listgateway.unipi.it/mailman/listinfo/ntop-dev
