On 2017/7/24 9:09, Ding Tianhong wrote:
>
>
> On 2017/7/24 1:03, Cong Wang wrote:
>> On Sun, Jul 23, 2017 at 5:48 AM, liujian (CE) <liujia...@huawei.com> wrote:
>>> Hi
>>>
>>> I find it caused by below steps:
>>> 1. set tp_version to TPACKET_V3 and req->tp_block_nr to 1
>>> 2. set tp_block_nr to 0
>>> Then pg_vec was freed, and we did not delete the timer?
>>
>> Thanks for testing!
>>
>> Ah, I overlook the initialization case in my previous patch.
>>
>> How about the following one? Does it cover all the cases?
>>
>>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index 008bb34ee324..0615c2a950fa 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -4329,7 +4329,7 @@ static int packet_set_ring(struct sock *sk,
>> union tpacket_req_u *req_u,
>> register_prot_hook(sk);
>> }
>> spin_unlock(&po->bind_lock);
>> - if (closing && (po->tp_version > TPACKET_V2)) {
>> + if (pg_vec && (po->tp_version > TPACKET_V2)) {
>> /* Because we don't support block-based V3 on tx-ring */
>> if (!tx_ring)
>> prb_shutdown_retire_blk_timer(po, rb_queue);
>>
>> .
>
> Hi, Cong:
>
> It looks like could not cover the case: req->tp_block_nr = 2 ->
> reg->tp_block_nr = 1 .
>
Oh, looks like this case would never happen, so I think your solution is ok.
> what about this way:
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -4331,13 +4331,17 @@ static int packet_set_ring(struct sock *sk, union
> tpacket_req_u *req_u,
> register_prot_hook(sk);
> }
> spin_unlock(&po->bind_lock);
> - if (closing && (po->tp_version > TPACKET_V2)) {
> + if ((closing || (pg_vec && !reg->tp_block_nr))&& (po->tp_version >
> TPACKET_V2)) {
> /* Because we don't support block-based V3 on tx-ring */
> if (!tx_ring)
> prb_shutdown_retire_blk_timer(po, rb_queue);
>
>
>>
>
>
> .
>
